sysadminandClaude Opus 4.8
4a2fae8495
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]>
5.1 KiB
5.1 KiB
Sanctioned Recovery Playbooks & Controls (Phase 2 #644)
Overview
Stale runtimes, worktree binding mismatches, and un-reconciled merged branches previously required expert manual shell recovery. Manual process kills (pkill -f mcp_server.py) are strictly forbidden and classified as runtime contamination (#630).
Phase 2 introduces sanctioned recovery playbooks and controls into the Web Console:
- Diagnose: Surface stale runtimes, worktree binding errors, contamination markers, and worktree anomalies via health & inventory APIs.
- Preview: Render mutation ledgers and exact confirmation phrases for recovery playbooks.
- Confirm & Apply: Execute sanctioned recovery actions through gated, audited paths.
- Verify: Revalidate control-plane state post-recovery before claiming clean status.
Recovery Playbook Taxonomy
| Playbook ID | Action ID | Minimum Role | Target / Scope | Description |
|---|---|---|---|---|
clear_stale_binding |
system.clear_stale_binding |
Operator | Active worktree binding | Clear provably missing or superseded GITEA_ACTIVE_WORKTREE binding (#702). |
rebind_session_worktree |
system.rebind_session_worktree |
Operator | Session worktree | Rebind or synchronize session worktree to verified lease worktree (#864). |
reconcile_cleanups |
system.reconcile_cleanups |
Controller | Worktree hygiene | Execute reconciler cleanup preview and apply for merged/superseded PR branches. |
sanctioned_restart |
system.restart_namespace |
Admin | MCP Namespace | Restart MCP daemon gracefully via host supervisor (#642). |
Wizard Workflow (Diagnose → Preview → Confirm → Verify)
1. Diagnose (GET /api/v1/system/recovery/diagnose)
Runs control-plane diagnostics:
- Stale Runtime: Mismatch between running daemon HEAD, local checkout HEAD, and remote-tracking HEAD.
- Worktree Binding: Missing path (
provably_stale_missing_path), unverified inherited binding (unverified_inherited), or superseded binding (superseded_by_session_lease). - Contamination: Checks for live contamination markers from unmanaged process kills.
- Worktree Anomalies: Scans
branches/directory for un-reconciled cleanups or missing preserved worktrees.
Returns RecoveryDiagnosis with eligible playbooks.
2. Preview (POST /api/v1/system/recovery/preview)
Takes playbook_id and optional target/params.
Returns:
- Mutation Ledger: Step-by-step sequence of actions.
- Confirmation Phrase: Exact phrase required to authorize execution (e.g.,
confirm clear_stale_binding). - Authorization Decision: RBAC check against the operator's principal.
3. Apply (POST /api/v1/system/recovery/apply)
Requires playbook_id and matching confirmation phrase. Gates run in this order, and each fails closed before anything is mutated:
- RBAC and execution phase (
console_authz.authorize(..., for_execution=True)). The phase branch only applies whenfor_executionis set. WhileACTIVE_PHASEis1, every phase-2 recovery action is refused withphase_not_active, so no recovery playbook writes yet. Preview reports the same decision underexecution_authorization/execution_blocked_reason. - Confirmation phrase (
confirmation_matches). - Contamination rules (#630): the live marker is read from the session inventory and assessed under the gated task key
console_recovery_apply. A contaminated runtime must be cleared through the reconciler cleanup playbook, which is the one playbook exempted from this gate because it is the designated remedy. The marker is also forwarded tosanctioned_restart.execute_restart, so a restart cannot launder a contaminated runtime.
Apply then executes the sanctioned recovery logic against the live process environment — not a copy — and records an audit entry in console_audit. A playbook that leaves the binding unchanged reports performed: false; binding_before, binding_after, and binding_changed are returned so a no-op cannot read as success.
Apply does not enforce master parity. Parity is reported by Diagnose (#610) as evidence for the operator; it is not a precondition of this endpoint.
4. Verify (POST /api/v1/system/recovery/verify)
Re-evaluates control-plane diagnostics post-recovery and reports clean, stale_runtime_clean, binding_clean, binding_classification, and contamination_clean. It reports; it does not assert or block. State is read fresh rather than from the mapping a mutation just wrote. An unverified_inherited binding is reported as not clean, because unproven is not clean.
Safety & Governance Principles
- No Manual
pkill: Direct process killing remains forbidden and is recorded as contamination. - Auditability: Every recovery preview and execution is logged in the console audit trail.
- Master Parity & Dual Control: High-privilege recovery actions require controller/admin roles and explicit confirmation phrases.