fix(webui): arm the recovery gates and make the playbooks reach the process (#644)
Reviewer REQUEST_CHANGES on PR #903 at head1c88b87raised five blockers, all reproduced by executing that head. The shared shape: a write path that declared itself gated, audited, and verified, but never armed the gate, mutated a copy of the state it claimed to fix, and then verified against that same copy. B1 - the apply path never asked the execution gate. execute_recovery_playbook called console_authz.authorize with the default for_execution=False, and the phase branch only fires when it is True. ACTIVE_PHASE is 1 and every new action is phase 2, so an operator executed a phase-2 write through POST /api/v1/system/recovery/apply while build_recovery_preview reported execution_enabled false. The call now passes for_execution=True and surfaces the phase_not_active refusal. Preview reports the same decision under execution_authorization / execution_blocked_reason instead of a hardcoded False it could not explain. B2 - both env playbooks mutated a discarded copy and verified against it. source_env = dict(os.environ) meant clear_stale_binding and rebind_session_worktree never touched the running process, and verify_post_recovery(env=source_env) re-diagnosed the same copy, confirming a change that had not happened. Mutations now target the live mapping (apply_recovery's sanctioned env=None -> os.environ path, #702 AC2) and verification re-reads state rather than the mutated input. binding_before / binding_after / binding_changed are returned, and a playbook that changed nothing reports performed: false. verify_post_recovery no longer reads an unverified_inherited binding as clean, because unproven is not clean. B3 - the reconcile playbook called a function that does not exist. merged_cleanup_reconcile.reconcile_merged_cleanups is absent from that module and a bare except turned the AttributeError into a generic failure, so the playbook could never succeed. It now calls gitea_mcp_server.gitea_reconcile_merged_cleanups, the real orchestrator, imported lazily; failures carry error_type. task_capability_map declared gitea.pr.close for reconcile_cleanups while the entry point gates on gitea.read; the two authority statements are reconciled to the one that is enforced. B4 - the #630 contamination integration could not block. assess_contamination_gate was fed marker=None, which short-circuits to block: False on its first statement; the task passed was a console action id outside CONTAMINATION_GATED_TASKS; and the result was read through a "contaminated" key the gate never returns, making STATUS_BLOCKED_CONTAMINATION unreachable. The live marker now comes from the #641 session inventory reader, the gated task key console_recovery_apply is added to CONTAMINATION_GATED_TASKS, every read uses the "block" key the gate actually returns, and the marker is forwarded to sanctioned_restart.execute_restart so a restart cannot launder a contaminated runtime. The reconciler cleanup playbook stays exempt as the designated remedy. B5 - the parity baseline was captured from the head it was compared against. capture_startup_parity(root, head=checkout_head) stores the head verbatim, so in_parity was structurally incapable of being false, and live_remote_head was never passed. The baseline is now the daemon start head that assess_stale_runtime already returns, and the #610 live-remote dimension is restored. Also: _recovery_card was the one renderer in system_health_views.py interpolating without _esc(), and it is where a marker's operator-supplied command_summary lands once B4 is wired; it now escapes, including the except branch. Docs no longer claim apply enforces master parity or that verify asserts clean: true, and the absolute file:///Users/... links are relative. Tests: the two that asserted the defects as intended are inverted - test_api_recovery_apply_with_dev_auth asserted the phase-gate bypass, and the rebind test asserted the input echoed back. Added coverage per blocker, including a no-op detection test that fails when a playbook reports success without changing anything, the previously untested reconcile playbook, contamination block and remedy-exemption tests, and parity baseline/live-remote tests. Both new guards were mutation-verified: disarming for_execution fails 2 tests, restoring the env copy fails 2 tests. Validation: WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from branches/feat-issue-644 gives 27 failed / 5291 passed / 6 skipped / 953 subtests; the same command from branches/baseline-master-76f293e at76f293eb28gives 28 failed / 5262 passed / 6 skipped / 926 subtests. Suites run one at a time. comm of the sorted FAILED lines shows no new signature at the head. The single absent signature, test_workspace_guard_alignment.py:: TestRuntimeContextGuardAlignment::test_declared_branches_worktree_passes_when_mcp_root_differs, is suite-order dependent: that file passes 9/9 in isolation at both revisions. Closes #644 Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
@@ -274,18 +274,24 @@ def _recovery_card() -> str:
|
||||
try:
|
||||
from webui import console_recovery
|
||||
diag = console_recovery.diagnose_recovery()
|
||||
status_badge = f"<span class='status-pill {diag.status}'>{diag.status}</span>"
|
||||
# Every other card in this file escapes at the interpolation boundary.
|
||||
# This one did not, and it is where a #630 marker's operator-supplied
|
||||
# command_summary lands once the contamination gate is wired.
|
||||
status_badge = (
|
||||
f"<span class='status-pill {_esc(diag.status)}'>{_esc(diag.status)}</span>"
|
||||
)
|
||||
playbook_lis = ""
|
||||
for pb in diag.playbooks:
|
||||
elig = "eligible" if pb.eligible else "disabled"
|
||||
playbook_lis += (
|
||||
f"<li><strong>{pb.label}</strong> (<code>{pb.playbook_id}</code>) — "
|
||||
f"<span class='badge {elig}'>{elig}</span>: {pb.description} "
|
||||
f"<em class='muted'>({pb.reason})</em></li>"
|
||||
f"<li><strong>{_esc(pb.label)}</strong> "
|
||||
f"(<code>{_esc(pb.playbook_id)}</code>) — "
|
||||
f"<span class='badge {elig}'>{elig}</span>: {_esc(pb.description)} "
|
||||
f"<em class='muted'>({_esc(pb.reason)})</em></li>"
|
||||
)
|
||||
reasons_html = ""
|
||||
if diag.reasons:
|
||||
items = "".join(f"<li>{r}</li>" for r in diag.reasons)
|
||||
items = "".join(f"<li>{_esc(r)}</li>" for r in diag.reasons)
|
||||
reasons_html = f"<ul class='reasons'>{items}</ul>"
|
||||
else:
|
||||
reasons_html = "<p class='clean-note'>No recovery actions currently required. Control plane is healthy.</p>"
|
||||
@@ -308,7 +314,7 @@ def _recovery_card() -> str:
|
||||
return (
|
||||
"<section class='health-card'>"
|
||||
"<h3>Sanctioned Recovery Controls (Phase 2 #644)</h3>"
|
||||
f"<p class='error'>Recovery diagnostics unavailable: {exc}</p>"
|
||||
f"<p class='error'>Recovery diagnostics unavailable: {_esc(exc)}</p>"
|
||||
"</section>"
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user