STATE: PR-open
WHO_IS_NEXT: author
NEXT_ACTION: Resolve B1 by asking the execution gate (for_execution=True) before any recovery write and reporting the phase refusal; B2 by applying the env recovery to the live process environment and verifying against it rather than a discarded copy; B3 by calling the reconciler cleanup entry point that exists; B4 by feeding a real contamination marker and a gated task key, and reading a key the gate actually returns; B5 by deriving the parity baseline from the daemon start head instead of the checkout head it is compared against. Add coverage for each, including a test that fails if a playbook reports success without changing anything.
NEXT_PROMPT:
Act as author on Scaled-Tech-Consulting/Gitea-Tools issue #644, PR #903, remote prgs. Read the reviewer REQUEST_CHANGES on PR #903 at head 1c88b87ec5030256fe5573bdc711792bf5bd5d37. Five code blockers must be resolved; all five were reproduced executably at that head.
B1: webui/console_recovery.py execute_recovery_playbook calls console_authz.authorize(action_id, principal) with the default for_execution=False and then performs writes. console_authz.authorize only applies the phase gate when for_execution=True (console_authz.py:567), ACTIVE_PHASE is 1, and every new action is phase 2, so an operator-role principal executes a phase-2 write through POST /api/v1/system/recovery/apply while build_recovery_preview reports execution_enabled false. Pass for_execution=True and surface the phase_not_active refusal, or declare an explicit execution flag the way webui/request_service.py does. tests/test_webui_console_recovery.py test_api_recovery_apply_with_dev_auth asserts the bypass as correct and must be inverted.
B2: execute_recovery_playbook builds source_env = dict(os.environ) at line 415 and hands that copy to stale_binding_recovery.apply_recovery (line 487) and to the rebind branch (line 491), so neither playbook changes the running process environment. It then calls verify_post_recovery(env=source_env) at line 547, which re-diagnoses the same copy and confirms the change that did not happen. apply_recovery defaults env=None to os.environ and its docstring names that the sanctioned in-daemon mechanism (#702 AC2). Apply to the live environment, and verify against live state, not against the mutated copy.
B3: line 504 calls merged_cleanup_reconcile.reconcile_merged_cleanups, which does not exist in that module; the bare except at line 512 turns the AttributeError into performed false, so the reconcile playbook can never succeed. The real entry point is gitea_mcp_server.gitea_reconcile_merged_cleanups. No test exercises this playbook.
B4: assess_contamination_gate is called with marker=None at lines 249 and 461, and it returns block false immediately when the marker is falsy, so the #630 integration can never block. The task passed is the console action id (system.clear_stale_binding), which is not in CONTAMINATION_GATED_TASKS, so a real marker would still not block. The code then reads contamination_dict.get("contaminated"), a key the gate never returns, so STATUS_BLOCKED_CONTAMINATION is unreachable and verify_post_recovery reports contamination_clean unconditionally. sanctioned_restart.execute_restart is also called without contamination_marker (line 520), defeating its own guard at sanctioned_restart.py:375. A live marker source already exists on master via the #641 session inventory.
B5: lines 214-215 call capture_startup_parity(root, head=checkout_head) and then assess_master_parity(startup_dict, checkout_head). capture_startup_parity stores startup_head verbatim (master_parity_gate.py:174), so the baseline and the current head are the same value by construction and in_parity is structurally always true; live_remote_head is never passed, so the #610 live-remote dimension is dropped. Derive the baseline from the daemon start head, which system_health.assess_stale_runtime already returns.
Docs: docs/sanctioned-recovery-playbooks.md claims apply "Enforces master parity (#610)" and enforces the contamination rule; neither is true in the execute path. It also links with absolute file:///Users/jasonwalker/... URLs.
Re-run WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from a branches/ worktree, one suite at a time, and compare against master 76f293eb288f, which gives 28 failed / 5262 passed / 6 skipped / 926 subtests. Do not run two suites concurrently: tests/test_mcp_server.py writes the shared path /tmp/gitea_review_decision.lock and concurrent runs produce a phantom failure. Then push to feat/issue-644-console-recovery and request a fresh review.
WHAT_HAPPENED: Independent reviewer review of PR #903 at head 1c88b87, the branch's only commit. Full suite run at the head and at a clean master baseline worktree, both under branches/. Two single-guard mutation runs against the guards that exist. Five blockers reproduced by direct execution against the branch head.
WHY: The apply path performs real recovery writes without ever consulting the console execution phase gate, and the two env-binding playbooks mutate a discarded copy of the environment and then verify against that same copy, so a recovery that changed nothing is reported and confirmed as successful.
ISSUE: 644
HEAD_SHA: 1c88b87ec5
REVIEW_STATUS: REQUEST_CHANGES
MERGE_READY: false
BLOCKERS: B1 — the apply path never asks for_execution, so phase-2 recovery writes execute while ACTIVE_PHASE is 1 (webui/console_recovery.py:418, against webui/console_authz.py:465,567). B2 — clear_stale_binding and rebind_session_worktree mutate a copy of os.environ and verify against that copy, reporting success for a no-op (webui/console_recovery.py:415,487,491,547). B3 — the reconcile_cleanups playbook calls merged_cleanup_reconcile.reconcile_merged_cleanups, which does not exist (webui/console_recovery.py:504). B4 — the #630 contamination gate is fed marker=None and a task key outside CONTAMINATION_GATED_TASKS, and the result is read through a key the gate never returns (webui/console_recovery.py:249,461,320,339). B5 — the master-parity baseline is captured from the head it is then compared against, so in_parity is structurally always true (webui/console_recovery.py:214-215, against master_parity_gate.py:174).
VALIDATION: WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from branches/review-pr903-head-1c88b87 at 1c88b87 gave 28 failed / 5276 passed / 6 skipped / 953 subtests; the same command from branches/baseline-master-76f293e at master 76f293eb28 gave 28 failed / 5262 passed / 6 skipped / 926 subtests; sorted FAILED lines diff empty in both directions, 28 identical signatures. Mutation runs on the two guards that exist gave 1 and 2 failures. All five blockers reproduced by executing the branch head directly.
LAST_UPDATED_BY: sysadmin (prgs-reviewer)
NATIVE_REVIEW_PROOF: gitea_submit_pr_review via native MCP namespace gitea-reviewer, profile prgs-reviewer, identity sysadmin, expected_head_sha 1c88b87ec5. No offline, import, or helper path was used.
[THREAD STATE LEDGER] PR #903 — REQUEST_CHANGES posted to Gitea
What is true now:
PR state: open
Server-side decision state: REQUEST_CHANGES posted to Gitea
Local verdict/state: REQUEST_CHANGES prepared locally at head 1c88b87
The shape is right: diagnose, preview with a mutation ledger and an exact
confirmation phrase, apply, verify. The two guards that exist — authorization
and confirmation — are real, and I confirmed each by neutering it and watching a
test fail. Everything downstream of those two guards, however, either does not
run, does not reach the process it claims to fix, or cannot report a failure.
Five blockers. Each one below was reproduced by executing this head, not by
reading. The reproduction output is quoted inline.
B1 — the apply path never asks the execution gate, so phase-2 writes execute in phase 1
webui/console_recovery.py:418, against webui/console_authz.py:465 and :567.
for_execution defaults to False, and the phase gate is the one branch that
only fires when it is True (console_authz.py:567):
iffor_executionandaction.phase>ACTIVE_PHASE:
ACTIVE_PHASE is 1. All three new actions are declared phase=2, as is system.restart_namespace. So the gate is never consulted on the one path that
writes. Executed at this head:
Failure scenario, concrete: WEBUI_AUTH_MODE=local_dev, WEBUI_DEV_ROLE=operator.
An operator POSTs /api/v1/system/recovery/apply with {"playbook_id": "rebind_session_worktree", "target": "...", "confirmation": "confirm rebind_session_worktree ..."}.
The route is registered unconditionally in create_app (webui/app.py), the
authorization passes, and the function proceeds to the mutation branch — while build_recovery_preview (console_recovery.py:391) told that same operator "execution_enabled": False.
This is the first console path that declares itself gated and then writes anyway. webui/sanctioned_restart.py:334-336 states the contrast explicitly for its own
action — it keeps execution_enabled False in every branch and never touches a
process — and this PR routes the restart playbook through it while opening an
ungated door beside it.
Coverage is inverted rather than absent. tests/test_webui_console_recovery.py:152-171
(test_api_recovery_apply_with_dev_auth) posts exactly that request and asserts 200 with success and allowed both true. The test encodes the bypass as the
intended behaviour.
Fix direction: pass for_execution=True and return the phase_not_active
refusal, or, if execution really is meant to be wired now, declare an explicit
per-action environment flag the way webui/request_service.py does for initiate_workflow, so raising the phase does not enable every phase-2 action at
once.
B2 — both env playbooks mutate a discarded copy, report success, and verify against that copy
clear_stale_binding then calls stale_binding_recovery.apply_recovery(plan, env=source_env),
which pops the key from whatever mapping it is given (stale_binding_recovery.py:241-249); rebind_session_worktree assigns source_env[ACTIVE_WORKTREE_ENV] = target_wt.
Neither touches os.environ. Then line 547 calls verify_post_recovery(env=source_env) — re-diagnosing the same copy — so the
post-recovery verification confirms the change that did not happen.
apply_recovery defaults env=None to os.environ and its docstring calls that
"the sanctioned in-daemon mechanism (#702 AC2)". This PR opts out of it.
Wrong outcome: the operator is told the poisoned binding was cleared, and the
verification step agrees, while the daemon still holds it. A .env-injected GITEA_ACTIVE_WORKTREE is a known live failure mode in this control plane, and
this control reports it fixed without fixing it. That defeats acceptance
criterion 3.
A second, independent instance of the same class: on a run where the clear was refused, the verification still reported clean.
clear_stale_binding -> success= False
applied_result = {'performed': False, 'action': 'none', 'reasons': ['recovery plan does not authorize clearing (fail closed)']}
post_recovery_verification.binding_clean = True
after: os.environ[GITEA_ACTIVE_WORKTREE] = /Users/jasonwalker/Development/Gitea-Tools
binding_clean is computed as not diag.stale_binding.get("clear_eligible")
(console_recovery.py:569), so any binding that recovery is not allowed to touch
— including unverified_inherited and a root binding — reads as clean.
Coverage is inverted here too. test_execute_recovery_playbook_rebind_session_success
asserts result["applied_result"]["rebound_worktree"] == "branches/feat-issue-644",
which is the input echoed back; no test asserts that any binding changed. test_execute_recovery_playbook_clear_stale_binding_success asserts only that
keys are present and never checks success.
Fix direction: apply to the live process environment and verify against live
state read fresh, not against the mutated input. A test that fails when a
playbook reports success without changing anything is the missing guard.
B3 — the reconcile playbook calls a function that does not exist
merged_cleanup_reconcile has no such attribute. The bare except Exception two
lines below converts the AttributeError into a generic error string, so the
playbook fails the same way every time and the audit records recovery_failed:
hasattr(merged_cleanup_reconcile, 'reconcile_merged_cleanups') = False
reconcile_cleanups -> success= False applied_result= {'performed': False, 'error': "module 'merged_cleanup_reconcile' has no attribute 'reconcile_merged_cleanups'"}
The real entry point is gitea_mcp_server.gitea_reconcile_merged_cleanups
(gitea_mcp_server.py:12046); merged_cleanup_reconcile exposes the building
blocks (build_reconciliation_report, plan_cleanup_execution_order, remove_local_worktree) but no such orchestrator.
So half of acceptance criterion 2 — the reconcile action — cannot succeed, and no
test exercises this playbook at all. It is also the playbook the contamination
branch at line 463 exempts as the designated remedy, so the documented escape
hatch from a contaminated runtime is the one that always fails.
B4 — the #630 contamination integration cannot block, and its result is read through a key it never returns
webui/console_recovery.py:249 and :461, against runtime_recovery_guard.py:538.
and the gate's first statement is if not marker or marker.get("cleared_by_reconciler"): return {"block": False, ...}.
No marker is ever loaded, so the gate is inert by construction.
Even with a real marker it would not block: the gate only blocks when task in CONTAMINATION_GATED_TASKS (stable_branch_push_guard.py:53-66 — create_pr, merge_pr, delete_branch, …), and what is passed is the console
action id system.clear_stale_binding.
Third layer: the returned dict has no contaminated key, and the code reads
exactly that key three times (:320restart_eligible, :338/:339clean
and STATUS_BLOCKED_CONTAMINATION, :570contamination_clean):
Wrong outcome: STATUS_BLOCKED_CONTAMINATION is unreachable, clean is never
withheld for contamination, contamination never makes a restart eligible, and verify_post_recovery reports contamination_clean: true on a contaminated
runtime.
sanctioned_restart.execute_restart is also called without contamination_marker (:520), so the stricter unconditional guard at sanctioned_restart.py:375-381 — a contaminated runtime must be reconciled
before restart, or the restart launders the contamination — never fires through
this path either.
A live marker source is already on master: the #641 session inventory exposes active_contamination (webui/session_views.py:88).
Issue #644 lists "Integrate contamination rules (#630)" under scope and
"contamination interaction tests" under required tests. There are none.
B5 — the master-parity baseline is captured from the head it is compared against
webui/console_recovery.py:214-215, against master_parity_gate.py:168-175.
capture_startup_parity stores the head verbatim — {"root": root, "startup_head": head}.
The baseline and the current head are therefore the same value by construction,
and in_parity is structurally incapable of being false. live_remote_head is
never passed either, so the #610 live-remote dimension is dropped as well, and
the reason branch if not parity_dict.get("in_parity", True) at line 216 is dead
code.
Wrong outcome: GET /api/v1/system/recovery/diagnose publishes master_parity.in_parity: true and restart_required: false on the exact surface
an operator consults to decide whether a restart is needed. Staleness is still
surfaced by the separate stale_runtime dimension, which is why this is a
misleading-evidence blocker rather than a total blindness one — but a parity
verdict that cannot be false is worse than no parity field, because it reads as
corroboration.
What the parity gate is actually about is the commit the running process
started at. system_health.assess_stale_runtime already returns daemon_head;
that is the baseline this should use.
Verified clean — no change requested
The two guards that exist are real, confirmed by mutation:
Both mutations were applied one at a time in the review worktree and reverted;
the worktree ended clean.
Scope. The effective diff against master is 9 files, exactly the stated list.
Single commit, no merge noise, base is current with master at 76f293eb288f,
no conflicts.
Read routes match the existing pattern./diagnose and /verify take no
principal, which is consistent with api_runtime, api_sessions, and api_worktrees already on master; not a new exposure class.
resolve_principal is not header-trusted. The role comes from server-side
configuration keyed by the resolved subject (console_authz.py:391-431), so B1
is a phase-gate defect, not an identity-spoofing one.
Non-blocking notes
_recovery_card() (webui/system_health_views.py) is the only renderer in
that file that interpolates without _esc(); every other card uses _esc or
the redacting helper at line 44. Today the interpolated values are enums and
static strings plus {exc} in the except branch, so I found no live injection
path — but the card is also where a contamination command_summary would land
once B4 is fixed, and webui/inventory.py:272 flags that value as the one
thing that must never render verbatim.
execute_recovery_playbook computes performed = bool(applied_result.get("performed") or applied_result.get("allowed"))
(:531). sanctioned_restart.execute_restart returns allowed: True when its
gates pass but documents that success is False in both directions, because
the host supervisor still has to act. So the restart playbook reports success: true for a restart that has not happened.
Restart confirmation scope: the outer phrase is confirm sanctioned_restart <target>, but when target is None the
namespace is taken from params["namespace"] (:518) while the phrase omits
it — the operator confirms a phrase that does not name the namespace being
restarted. The inner confirmation=f"{md} {ns}" is then synthesized by the
console, so sanctioned_restart's own confirmation check validates a string
this code just built.
_build_ledger calls the private sanctioned_restart._mutation_ledger.
task_capability_map.py now holds three overlapping keys for one operation:
the new reconcile_cleanups (gitea.pr.close / reconciler), the existing reconcile_merged_cleanups (gitea.read / reconciler), and reconciliation_cleanup (gitea.branch.delete / reconciler). The new clear_stale_binding and rebind_session_worktree entries declare role author while the console actions require operator; two authority
statements for the same action.
docs/sanctioned-recovery-playbooks.md states that apply "Enforces master
parity (#610)" and enforces the contamination rule. The execute path does
neither (no parity check at all; see B4 for contamination). It also states
Verify "Asserts clean: true" — verify_post_recovery reports, it does not
assert. The doc links with absolute file:///Users/jasonwalker/... URLs,
which are broken for every other reader and leak a local home path.
Validation, re-run by the reviewer
Both runs from branches/ worktrees, not /tmp, since several suites resolve
the repo root from the checkout path. Run one at a time.
diff of the sorted FAILED lines is empty in both directions — the 28
signatures are identical. The branch introduces no new failure signature and adds
14 passing tests.
One methodology note for the author, because it will otherwise cost an hour: my
first head run was executed concurrently with the baseline run and reported a
29th failure, tests/test_mcp_server.py::TestSubmitPrReview::test_spoofed_tmp_lock_file_does_not_bypass_gate.
That test writes and deletes the hard-coded shared path /tmp/gitea_review_decision.lock (tests/test_mcp_server.py:2663-2682), so two
concurrent suite runs race on it. The test passes in isolation at this head, and
the serial re-run above is clean. Do not run two full suites at once in this
repo.
Canonical Handoff
REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools
ISSUE: 644
PR: 903
WORKFLOW_STATE: needs-author
HEAD_SHA: 1c88b87ec5030256fe5573bdc711792bf5bd5d37
BASE_BRANCH: master
BASE_OR_MERGE_SHA: 76f293eb288fa6cbb3134e093fc960ac01517e75
ACTING_ROLE: reviewer
ACTING_IDENTITY: sysadmin (prgs-reviewer)
COMPLETED_ACTIONS: Independent review of PR #903 at head 1c88b87; full-suite runs at the reviewed head and at a clean master baseline worktree, run serially; two single-guard mutation runs against the guards that exist; executable reproduction of all five blockers against the branch head; REQUEST_CHANGES posted to Gitea.
VALIDATION_EVIDENCE: WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from branches/review-pr903-head-1c88b87 at 1c88b87 gave 28 failed / 5276 passed / 6 skipped / 953 subtests; the same command from branches/baseline-master-76f293e at 76f293eb288f gave 28 failed / 5262 passed / 6 skipped / 926 subtests; sorted FAILED lines diff empty both ways, 28 identical signatures. Mutation runs: confirmation guard neutered 1 failure, authorization guard neutered 2 failures. Repro at head: authorize(for_execution) denies phase_not_active while the apply path's authorize allows; rebind and clear both leave os.environ unchanged while reporting performed true; merged_cleanup_reconcile.reconcile_merged_cleanups absent; contamination dict lacks the contaminated key; parity startup_head equals the head it is compared against.
MUTATION_LEDGER: gitea_submit_pr_review to REQUEST_CHANGES posted to Gitea. Reviewer PR lease acquired and released. No other server-side state changed. Local: one review worktree created under branches/ (review-pr903-head-1c88b87); two guard mutations applied one at a time and reverted, leaving the worktree clean; no edits to tracked branch content.
BLOCKERS: B1 — the apply path never asks for_execution, so phase-2 recovery writes execute while ACTIVE_PHASE is 1 (webui/console_recovery.py:418). B2 — clear_stale_binding and rebind_session_worktree mutate a copy of os.environ and verify against that copy, reporting success for a no-op (webui/console_recovery.py:415,487,491,547). B3 — the reconcile playbook calls merged_cleanup_reconcile.reconcile_merged_cleanups, which does not exist (webui/console_recovery.py:504). B4 — the #630 contamination gate is fed marker=None and a task key outside CONTAMINATION_GATED_TASKS, and its result is read through a key it never returns (webui/console_recovery.py:249,461,320,339,570). B5 — the master-parity baseline is captured from the head it is compared against (webui/console_recovery.py:214-215).
NEXT_ACTOR: author
NEXT_ACTION: Fix B1 through B5 and add coverage for each, including a test that fails when a playbook reports success without changing anything, a test for the reconcile playbook, and a contamination interaction test.
PROHIBITED_ACTIONS: Do not merge. Do not self-approve. Do not read the two passing apply tests as proof the execution path is correct — test_api_recovery_apply_with_dev_auth asserts the phase-gate bypass, and test_execute_recovery_playbook_rebind_session_success asserts the input echoed back.
NEXT_PROMPT: Act as author on Scaled-Tech-Consulting/Gitea-Tools issue #644, PR #903, remote prgs. Read the reviewer REQUEST_CHANGES on PR #903 at head 1c88b87ec5030256fe5573bdc711792bf5bd5d37. Five code blockers, all reproduced executably. B1: execute_recovery_playbook calls console_authz.authorize without for_execution=True, so phase-2 writes execute while ACTIVE_PHASE is 1; pass for_execution=True and surface phase_not_active, or declare an explicit per-action execution flag, and invert test_api_recovery_apply_with_dev_auth. B2: line 415 builds source_env = dict(os.environ) and the clear and rebind branches mutate that copy, then verify_post_recovery re-diagnoses the same copy; apply to the live environment and verify against live state. B3: line 504 calls merged_cleanup_reconcile.reconcile_merged_cleanups, which does not exist; the real entry point is gitea_mcp_server.gitea_reconcile_merged_cleanups. B4: assess_contamination_gate is called with marker=None and with a console action id that is not in CONTAMINATION_GATED_TASKS, and the result is read through a contaminated key the gate never returns; wire the live marker from the #641 session inventory, pass a gated task key, read block, and pass contamination_marker through to sanctioned_restart.execute_restart. B5: capture_startup_parity is given the same checkout head that assess_master_parity is then compared against, so in_parity is always true; use the daemon start head that system_health.assess_stale_runtime returns, and pass live_remote_head. Also correct docs/sanctioned-recovery-playbooks.md, which claims apply enforces master parity and the contamination rule, and replace its absolute file:/// links. Re-run WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from a branches/ worktree, one suite at a time, against master 76f293eb288f which gives 28 failed / 5262 passed / 6 skipped / 926 subtests, then push to feat/issue-644-console-recovery and request a fresh review.
WORKFLOW_FAILURE_ISSUES: none
LAST_UPDATED: 2026-07-25T14:05:00Z
## Canonical PR State
STATE: PR-open
WHO_IS_NEXT: author
NEXT_ACTION: Resolve B1 by asking the execution gate (for_execution=True) before any recovery write and reporting the phase refusal; B2 by applying the env recovery to the live process environment and verifying against it rather than a discarded copy; B3 by calling the reconciler cleanup entry point that exists; B4 by feeding a real contamination marker and a gated task key, and reading a key the gate actually returns; B5 by deriving the parity baseline from the daemon start head instead of the checkout head it is compared against. Add coverage for each, including a test that fails if a playbook reports success without changing anything.
NEXT_PROMPT:
```text
Act as author on Scaled-Tech-Consulting/Gitea-Tools issue #644, PR #903, remote prgs. Read the reviewer REQUEST_CHANGES on PR #903 at head 1c88b87ec5030256fe5573bdc711792bf5bd5d37. Five code blockers must be resolved; all five were reproduced executably at that head.
B1: webui/console_recovery.py execute_recovery_playbook calls console_authz.authorize(action_id, principal) with the default for_execution=False and then performs writes. console_authz.authorize only applies the phase gate when for_execution=True (console_authz.py:567), ACTIVE_PHASE is 1, and every new action is phase 2, so an operator-role principal executes a phase-2 write through POST /api/v1/system/recovery/apply while build_recovery_preview reports execution_enabled false. Pass for_execution=True and surface the phase_not_active refusal, or declare an explicit execution flag the way webui/request_service.py does. tests/test_webui_console_recovery.py test_api_recovery_apply_with_dev_auth asserts the bypass as correct and must be inverted.
B2: execute_recovery_playbook builds source_env = dict(os.environ) at line 415 and hands that copy to stale_binding_recovery.apply_recovery (line 487) and to the rebind branch (line 491), so neither playbook changes the running process environment. It then calls verify_post_recovery(env=source_env) at line 547, which re-diagnoses the same copy and confirms the change that did not happen. apply_recovery defaults env=None to os.environ and its docstring names that the sanctioned in-daemon mechanism (#702 AC2). Apply to the live environment, and verify against live state, not against the mutated copy.
B3: line 504 calls merged_cleanup_reconcile.reconcile_merged_cleanups, which does not exist in that module; the bare except at line 512 turns the AttributeError into performed false, so the reconcile playbook can never succeed. The real entry point is gitea_mcp_server.gitea_reconcile_merged_cleanups. No test exercises this playbook.
B4: assess_contamination_gate is called with marker=None at lines 249 and 461, and it returns block false immediately when the marker is falsy, so the #630 integration can never block. The task passed is the console action id (system.clear_stale_binding), which is not in CONTAMINATION_GATED_TASKS, so a real marker would still not block. The code then reads contamination_dict.get("contaminated"), a key the gate never returns, so STATUS_BLOCKED_CONTAMINATION is unreachable and verify_post_recovery reports contamination_clean unconditionally. sanctioned_restart.execute_restart is also called without contamination_marker (line 520), defeating its own guard at sanctioned_restart.py:375. A live marker source already exists on master via the #641 session inventory.
B5: lines 214-215 call capture_startup_parity(root, head=checkout_head) and then assess_master_parity(startup_dict, checkout_head). capture_startup_parity stores startup_head verbatim (master_parity_gate.py:174), so the baseline and the current head are the same value by construction and in_parity is structurally always true; live_remote_head is never passed, so the #610 live-remote dimension is dropped. Derive the baseline from the daemon start head, which system_health.assess_stale_runtime already returns.
Docs: docs/sanctioned-recovery-playbooks.md claims apply "Enforces master parity (#610)" and enforces the contamination rule; neither is true in the execute path. It also links with absolute file:///Users/jasonwalker/... URLs.
Re-run WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from a branches/ worktree, one suite at a time, and compare against master 76f293eb288f, which gives 28 failed / 5262 passed / 6 skipped / 926 subtests. Do not run two suites concurrently: tests/test_mcp_server.py writes the shared path /tmp/gitea_review_decision.lock and concurrent runs produce a phantom failure. Then push to feat/issue-644-console-recovery and request a fresh review.
```
WHAT_HAPPENED: Independent reviewer review of PR #903 at head 1c88b87, the branch's only commit. Full suite run at the head and at a clean master baseline worktree, both under branches/. Two single-guard mutation runs against the guards that exist. Five blockers reproduced by direct execution against the branch head.
WHY: The apply path performs real recovery writes without ever consulting the console execution phase gate, and the two env-binding playbooks mutate a discarded copy of the environment and then verify against that same copy, so a recovery that changed nothing is reported and confirmed as successful.
ISSUE: 644
HEAD_SHA: 1c88b87ec5030256fe5573bdc711792bf5bd5d37
REVIEW_STATUS: REQUEST_CHANGES
MERGE_READY: false
BLOCKERS: B1 — the apply path never asks for_execution, so phase-2 recovery writes execute while ACTIVE_PHASE is 1 (webui/console_recovery.py:418, against webui/console_authz.py:465,567). B2 — clear_stale_binding and rebind_session_worktree mutate a copy of os.environ and verify against that copy, reporting success for a no-op (webui/console_recovery.py:415,487,491,547). B3 — the reconcile_cleanups playbook calls merged_cleanup_reconcile.reconcile_merged_cleanups, which does not exist (webui/console_recovery.py:504). B4 — the #630 contamination gate is fed marker=None and a task key outside CONTAMINATION_GATED_TASKS, and the result is read through a key the gate never returns (webui/console_recovery.py:249,461,320,339). B5 — the master-parity baseline is captured from the head it is then compared against, so in_parity is structurally always true (webui/console_recovery.py:214-215, against master_parity_gate.py:174).
VALIDATION: WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from branches/review-pr903-head-1c88b87 at 1c88b87 gave 28 failed / 5276 passed / 6 skipped / 953 subtests; the same command from branches/baseline-master-76f293e at master 76f293eb288f gave 28 failed / 5262 passed / 6 skipped / 926 subtests; sorted FAILED lines diff empty in both directions, 28 identical signatures. Mutation runs on the two guards that exist gave 1 and 2 failures. All five blockers reproduced by executing the branch head directly.
LAST_UPDATED_BY: sysadmin (prgs-reviewer)
NATIVE_REVIEW_PROOF: gitea_submit_pr_review via native MCP namespace gitea-reviewer, profile prgs-reviewer, identity sysadmin, expected_head_sha 1c88b87ec5030256fe5573bdc711792bf5bd5d37. No offline, import, or helper path was used.
[THREAD STATE LEDGER] PR #903 — REQUEST_CHANGES posted to Gitea
What is true now:
- PR state: open
- Server-side decision state: REQUEST_CHANGES posted to Gitea
- Local verdict/state: REQUEST_CHANGES prepared locally at head 1c88b87
- Latest known validation: reviewed head 28 failed / 5276 passed / 6 skipped / 953 subtests, master baseline 28 failed / 5262 passed / 6 skipped / 926 subtests, 28 identical failure signatures
What changed:
- REQUEST_CHANGES posted to Gitea
What is blocked:
- Blocker classification: code blocker
Who/what acts next:
- Next actor: author
- Required action: resolve B1 through B5 below and push the fixes
- Do not do: merge
- Resume from: PR review comments
Server-side mutation ledger:
- gitea_submit_pr_review → REQUEST_CHANGES posted to Gitea
---
# Review — PR #903 (#644), head 1c88b87
The shape is right: diagnose, preview with a mutation ledger and an exact
confirmation phrase, apply, verify. The two guards that exist — authorization
and confirmation — are real, and I confirmed each by neutering it and watching a
test fail. Everything downstream of those two guards, however, either does not
run, does not reach the process it claims to fix, or cannot report a failure.
Five blockers. Each one below was reproduced by executing this head, not by
reading. The reproduction output is quoted inline.
## B1 — the apply path never asks the execution gate, so phase-2 writes execute in phase 1
`webui/console_recovery.py:418`, against `webui/console_authz.py:465` and `:567`.
`execute_recovery_playbook` authorizes with:
```python
decision = console_authz.authorize(action_id, principal)
```
`for_execution` defaults to `False`, and the phase gate is the one branch that
only fires when it is `True` (`console_authz.py:567`):
```python
if for_execution and action.phase > ACTIVE_PHASE:
```
`ACTIVE_PHASE` is `1`. All three new actions are declared `phase=2`, as is
`system.restart_namespace`. So the gate is never consulted on the one path that
writes. Executed at this head:
```text
ACTIVE_PHASE = 1
system.clear_stale_binding: phase=2 authorize(allowed)=True authorize(for_execution).allowed=False reason=phase_not_active
system.rebind_session_worktree: phase=2 authorize(allowed)=True authorize(for_execution).allowed=False reason=phase_not_active
system.reconcile_cleanups: phase=2 authorize(allowed)=False authorize(for_execution).allowed=False reason=insufficient_role
```
Failure scenario, concrete: `WEBUI_AUTH_MODE=local_dev`, `WEBUI_DEV_ROLE=operator`.
An operator POSTs `/api/v1/system/recovery/apply` with
`{"playbook_id": "rebind_session_worktree", "target": "...", "confirmation": "confirm rebind_session_worktree ..."}`.
The route is registered unconditionally in `create_app` (`webui/app.py`), the
authorization passes, and the function proceeds to the mutation branch — while
`build_recovery_preview` (`console_recovery.py:391`) told that same operator
`"execution_enabled": False`.
This is the first console path that declares itself gated and then writes anyway.
`webui/sanctioned_restart.py:334-336` states the contrast explicitly for its own
action — it keeps `execution_enabled` False in every branch and never touches a
process — and this PR routes the restart playbook through it while opening an
ungated door beside it.
Coverage is inverted rather than absent. `tests/test_webui_console_recovery.py:152-171`
(`test_api_recovery_apply_with_dev_auth`) posts exactly that request and asserts
`200` with `success` and `allowed` both true. The test encodes the bypass as the
intended behaviour.
Fix direction: pass `for_execution=True` and return the `phase_not_active`
refusal, or, if execution really is meant to be wired now, declare an explicit
per-action environment flag the way `webui/request_service.py` does for
`initiate_workflow`, so raising the phase does not enable every phase-2 action at
once.
## B2 — both env playbooks mutate a discarded copy, report success, and verify against that copy
`webui/console_recovery.py:415`, `:487`, `:491`, `:547`.
Line 415 builds a copy:
```python
source_env = dict(env) if env is not None else dict(os.environ)
```
`clear_stale_binding` then calls `stale_binding_recovery.apply_recovery(plan, env=source_env)`,
which pops the key from whatever mapping it is given (`stale_binding_recovery.py:241-249`);
`rebind_session_worktree` assigns `source_env[ACTIVE_WORKTREE_ENV] = target_wt`.
Neither touches `os.environ`. Then line 547 calls
`verify_post_recovery(env=source_env)` — re-diagnosing **the same copy** — so the
post-recovery verification confirms the change that did not happen.
`apply_recovery` defaults `env=None` to `os.environ` and its docstring calls that
"the sanctioned in-daemon mechanism (#702 AC2)". This PR opts out of it.
Reproduced, rebind, at this head:
```text
before: os.environ[GITEA_ACTIVE_WORKTREE] = /Users/jasonwalker/Development/Gitea-Tools
rebind_session_worktree -> success= True applied_result= {'performed': True, 'rebound_worktree': 'branches/feat-issue-644', 'cleared_stale': True}
after rebind: os.environ[GITEA_ACTIVE_WORKTREE] = /Users/jasonwalker/Development/Gitea-Tools
```
Reproduced, clear, using a provably clear-eligible plan and the exact call at
line 487:
```text
classification = provably_stale_missing_path clear_eligible = True
plan.clear_allowed = True action = clear_env
apply_recovery -> True clear_env /Users/.../branches/deleted-worktree
copy cleared? True
os.environ still bound? /Users/.../branches/deleted-worktree
```
Wrong outcome: the operator is told the poisoned binding was cleared, and the
verification step agrees, while the daemon still holds it. A `.env`-injected
`GITEA_ACTIVE_WORKTREE` is a known live failure mode in this control plane, and
this control reports it fixed without fixing it. That defeats acceptance
criterion 3.
A second, independent instance of the same class: on a run where the clear was
*refused*, the verification still reported clean.
```text
clear_stale_binding -> success= False
applied_result = {'performed': False, 'action': 'none', 'reasons': ['recovery plan does not authorize clearing (fail closed)']}
post_recovery_verification.binding_clean = True
after: os.environ[GITEA_ACTIVE_WORKTREE] = /Users/jasonwalker/Development/Gitea-Tools
```
`binding_clean` is computed as `not diag.stale_binding.get("clear_eligible")`
(`console_recovery.py:569`), so any binding that recovery is not allowed to touch
— including `unverified_inherited` and a root binding — reads as clean.
Coverage is inverted here too. `test_execute_recovery_playbook_rebind_session_success`
asserts `result["applied_result"]["rebound_worktree"] == "branches/feat-issue-644"`,
which is the input echoed back; no test asserts that any binding changed.
`test_execute_recovery_playbook_clear_stale_binding_success` asserts only that
keys are present and never checks `success`.
Fix direction: apply to the live process environment and verify against live
state read fresh, not against the mutated input. A test that fails when a
playbook reports success without changing anything is the missing guard.
## B3 — the reconcile playbook calls a function that does not exist
`webui/console_recovery.py:504`.
```python
snapshot = merged_cleanup_reconcile.reconcile_merged_cleanups(apply=True, project_root=str(_repo_root()))
```
`merged_cleanup_reconcile` has no such attribute. The bare `except Exception` two
lines below converts the `AttributeError` into a generic error string, so the
playbook fails the same way every time and the audit records `recovery_failed`:
```text
hasattr(merged_cleanup_reconcile, 'reconcile_merged_cleanups') = False
reconcile_cleanups -> success= False applied_result= {'performed': False, 'error': "module 'merged_cleanup_reconcile' has no attribute 'reconcile_merged_cleanups'"}
```
The real entry point is `gitea_mcp_server.gitea_reconcile_merged_cleanups`
(`gitea_mcp_server.py:12046`); `merged_cleanup_reconcile` exposes the building
blocks (`build_reconciliation_report`, `plan_cleanup_execution_order`,
`remove_local_worktree`) but no such orchestrator.
So half of acceptance criterion 2 — the reconcile action — cannot succeed, and no
test exercises this playbook at all. It is also the playbook the contamination
branch at line 463 exempts as the designated remedy, so the documented escape
hatch from a contaminated runtime is the one that always fails.
## B4 — the #630 contamination integration cannot block, and its result is read through a key it never returns
`webui/console_recovery.py:249` and `:461`, against `runtime_recovery_guard.py:538`.
Both call sites hardcode `marker=None`:
```python
contamination_dict = runtime_recovery_guard.assess_contamination_gate(marker=None, task=None, actual_role=role_kind)
```
and the gate's first statement is `if not marker or marker.get("cleared_by_reconciler"): return {"block": False, ...}`.
No marker is ever loaded, so the gate is inert by construction.
Even with a real marker it would not block: the gate only blocks when
`task in CONTAMINATION_GATED_TASKS` (`stable_branch_push_guard.py:53-66` —
`create_pr`, `merge_pr`, `delete_branch`, …), and what is passed is the console
action id `system.clear_stale_binding`.
Third layer: the returned dict has no `contaminated` key, and the code reads
exactly that key three times (`:320` `restart_eligible`, `:338`/`:339` `clean`
and `STATUS_BLOCKED_CONTAMINATION`, `:570` `contamination_clean`):
```text
contamination dict = {'block': False, 'reasons': [], 'task': None}
has 'contaminated' key = False
```
Wrong outcome: `STATUS_BLOCKED_CONTAMINATION` is unreachable, `clean` is never
withheld for contamination, contamination never makes a restart eligible, and
`verify_post_recovery` reports `contamination_clean: true` on a contaminated
runtime.
`sanctioned_restart.execute_restart` is also called without
`contamination_marker` (`:520`), so the stricter unconditional guard at
`sanctioned_restart.py:375-381` — a contaminated runtime must be reconciled
before restart, or the restart launders the contamination — never fires through
this path either.
A live marker source is already on master: the #641 session inventory exposes
`active_contamination` (`webui/session_views.py:88`).
Issue #644 lists "Integrate contamination rules (#630)" under scope and
"contamination interaction tests" under required tests. There are none.
## B5 — the master-parity baseline is captured from the head it is compared against
`webui/console_recovery.py:214-215`, against `master_parity_gate.py:168-175`.
```python
startup_dict = master_parity_gate.capture_startup_parity(str(root), head=checkout_head)
parity_dict = master_parity_gate.assess_master_parity(startup_dict, checkout_head)
```
`capture_startup_parity` stores the head verbatim — `{"root": root, "startup_head": head}`.
The baseline and the current head are therefore the same value by construction,
and `in_parity` is structurally incapable of being false. `live_remote_head` is
never passed either, so the #610 live-remote dimension is dropped as well, and
the reason branch `if not parity_dict.get("in_parity", True)` at line 216 is dead
code.
```text
master_parity in_parity = True stale = False
parity startup_head = 1c88b87ec5030256fe5573bdc711792bf5bd5d37 current_head = None
```
Wrong outcome: `GET /api/v1/system/recovery/diagnose` publishes
`master_parity.in_parity: true` and `restart_required: false` on the exact surface
an operator consults to decide whether a restart is needed. Staleness is still
surfaced by the separate `stale_runtime` dimension, which is why this is a
misleading-evidence blocker rather than a total blindness one — but a parity
verdict that cannot be false is worse than no parity field, because it reads as
corroboration.
What the parity gate is actually about is the commit the *running process*
started at. `system_health.assess_stale_runtime` already returns `daemon_head`;
that is the baseline this should use.
## Verified clean — no change requested
**The two guards that exist are real**, confirmed by mutation:
| Guard | Mutation | Result |
|---|---|---|
| Confirmation phrase | `if not confirmation_matches(...)` → `if False:` | **1 failed**, 13 passed — `test_execute_recovery_playbook_confirmation_mismatch` |
| Authorization | `if not decision.allowed:` → `if False:` | **2 failed**, 12 passed — `test_execute_recovery_playbook_unauthorized`, `test_api_recovery_apply_denied_without_auth` |
Both mutations were applied one at a time in the review worktree and reverted;
the worktree ended clean.
**Scope.** The effective diff against master is 9 files, exactly the stated list.
Single commit, no merge noise, base is current with master at `76f293eb288f`,
no conflicts.
**Read routes match the existing pattern.** `/diagnose` and `/verify` take no
principal, which is consistent with `api_runtime`, `api_sessions`, and
`api_worktrees` already on master; not a new exposure class.
**`resolve_principal` is not header-trusted.** The role comes from server-side
configuration keyed by the resolved subject (`console_authz.py:391-431`), so B1
is a phase-gate defect, not an identity-spoofing one.
## Non-blocking notes
- `_recovery_card()` (`webui/system_health_views.py`) is the only renderer in
that file that interpolates without `_esc()`; every other card uses `_esc` or
the redacting helper at line 44. Today the interpolated values are enums and
static strings plus `{exc}` in the except branch, so I found no live injection
path — but the card is also where a contamination `command_summary` would land
once B4 is fixed, and `webui/inventory.py:272` flags that value as the one
thing that must never render verbatim.
- `execute_recovery_playbook` computes
`performed = bool(applied_result.get("performed") or applied_result.get("allowed"))`
(`:531`). `sanctioned_restart.execute_restart` returns `allowed: True` when its
gates pass but documents that `success` is False in both directions, because
the host supervisor still has to act. So the restart playbook reports
`success: true` for a restart that has not happened.
- Restart confirmation scope: the outer phrase is
`confirm sanctioned_restart <target>`, but when `target` is `None` the
namespace is taken from `params["namespace"]` (`:518`) while the phrase omits
it — the operator confirms a phrase that does not name the namespace being
restarted. The inner `confirmation=f"{md} {ns}"` is then synthesized by the
console, so `sanctioned_restart`'s own confirmation check validates a string
this code just built.
- `_build_ledger` calls the private `sanctioned_restart._mutation_ledger`.
- `task_capability_map.py` now holds three overlapping keys for one operation:
the new `reconcile_cleanups` (`gitea.pr.close` / reconciler), the existing
`reconcile_merged_cleanups` (`gitea.read` / reconciler), and
`reconciliation_cleanup` (`gitea.branch.delete` / reconciler). The new
`clear_stale_binding` and `rebind_session_worktree` entries declare role
`author` while the console actions require `operator`; two authority
statements for the same action.
- `docs/sanctioned-recovery-playbooks.md` states that apply "Enforces master
parity (#610)" and enforces the contamination rule. The execute path does
neither (no parity check at all; see B4 for contamination). It also states
Verify "Asserts `clean: true`" — `verify_post_recovery` reports, it does not
assert. The doc links with absolute `file:///Users/jasonwalker/...` URLs,
which are broken for every other reader and leak a local home path.
## Validation, re-run by the reviewer
Both runs from `branches/` worktrees, not `/tmp`, since several suites resolve
the repo root from the checkout path. Run **one at a time**.
```text
# clean master baseline, branches/baseline-master-76f293e @ 76f293eb288f
WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q
28 failed, 5262 passed, 6 skipped, 926 subtests passed in 155.24s
# reviewed head, branches/review-pr903-head-1c88b87 @ 1c88b87
WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q
28 failed, 5276 passed, 6 skipped, 953 subtests passed in 147.08s
```
`diff` of the sorted `FAILED` lines is empty in both directions — the 28
signatures are identical. The branch introduces no new failure signature and adds
14 passing tests.
One methodology note for the author, because it will otherwise cost an hour: my
first head run was executed concurrently with the baseline run and reported a
29th failure,
`tests/test_mcp_server.py::TestSubmitPrReview::test_spoofed_tmp_lock_file_does_not_bypass_gate`.
That test writes and deletes the hard-coded shared path
`/tmp/gitea_review_decision.lock` (`tests/test_mcp_server.py:2663-2682`), so two
concurrent suite runs race on it. The test passes in isolation at this head, and
the serial re-run above is clean. Do not run two full suites at once in this
repo.
<!-- sph:v1 -->
## Canonical Handoff
```text
REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools
ISSUE: 644
PR: 903
WORKFLOW_STATE: needs-author
HEAD_SHA: 1c88b87ec5030256fe5573bdc711792bf5bd5d37
BASE_BRANCH: master
BASE_OR_MERGE_SHA: 76f293eb288fa6cbb3134e093fc960ac01517e75
ACTING_ROLE: reviewer
ACTING_IDENTITY: sysadmin (prgs-reviewer)
COMPLETED_ACTIONS: Independent review of PR #903 at head 1c88b87; full-suite runs at the reviewed head and at a clean master baseline worktree, run serially; two single-guard mutation runs against the guards that exist; executable reproduction of all five blockers against the branch head; REQUEST_CHANGES posted to Gitea.
VALIDATION_EVIDENCE: WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from branches/review-pr903-head-1c88b87 at 1c88b87 gave 28 failed / 5276 passed / 6 skipped / 953 subtests; the same command from branches/baseline-master-76f293e at 76f293eb288f gave 28 failed / 5262 passed / 6 skipped / 926 subtests; sorted FAILED lines diff empty both ways, 28 identical signatures. Mutation runs: confirmation guard neutered 1 failure, authorization guard neutered 2 failures. Repro at head: authorize(for_execution) denies phase_not_active while the apply path's authorize allows; rebind and clear both leave os.environ unchanged while reporting performed true; merged_cleanup_reconcile.reconcile_merged_cleanups absent; contamination dict lacks the contaminated key; parity startup_head equals the head it is compared against.
MUTATION_LEDGER: gitea_submit_pr_review to REQUEST_CHANGES posted to Gitea. Reviewer PR lease acquired and released. No other server-side state changed. Local: one review worktree created under branches/ (review-pr903-head-1c88b87); two guard mutations applied one at a time and reverted, leaving the worktree clean; no edits to tracked branch content.
BLOCKERS: B1 — the apply path never asks for_execution, so phase-2 recovery writes execute while ACTIVE_PHASE is 1 (webui/console_recovery.py:418). B2 — clear_stale_binding and rebind_session_worktree mutate a copy of os.environ and verify against that copy, reporting success for a no-op (webui/console_recovery.py:415,487,491,547). B3 — the reconcile playbook calls merged_cleanup_reconcile.reconcile_merged_cleanups, which does not exist (webui/console_recovery.py:504). B4 — the #630 contamination gate is fed marker=None and a task key outside CONTAMINATION_GATED_TASKS, and its result is read through a key it never returns (webui/console_recovery.py:249,461,320,339,570). B5 — the master-parity baseline is captured from the head it is compared against (webui/console_recovery.py:214-215).
NEXT_ACTOR: author
NEXT_ACTION: Fix B1 through B5 and add coverage for each, including a test that fails when a playbook reports success without changing anything, a test for the reconcile playbook, and a contamination interaction test.
PROHIBITED_ACTIONS: Do not merge. Do not self-approve. Do not read the two passing apply tests as proof the execution path is correct — test_api_recovery_apply_with_dev_auth asserts the phase-gate bypass, and test_execute_recovery_playbook_rebind_session_success asserts the input echoed back.
NEXT_PROMPT: Act as author on Scaled-Tech-Consulting/Gitea-Tools issue #644, PR #903, remote prgs. Read the reviewer REQUEST_CHANGES on PR #903 at head 1c88b87ec5030256fe5573bdc711792bf5bd5d37. Five code blockers, all reproduced executably. B1: execute_recovery_playbook calls console_authz.authorize without for_execution=True, so phase-2 writes execute while ACTIVE_PHASE is 1; pass for_execution=True and surface phase_not_active, or declare an explicit per-action execution flag, and invert test_api_recovery_apply_with_dev_auth. B2: line 415 builds source_env = dict(os.environ) and the clear and rebind branches mutate that copy, then verify_post_recovery re-diagnoses the same copy; apply to the live environment and verify against live state. B3: line 504 calls merged_cleanup_reconcile.reconcile_merged_cleanups, which does not exist; the real entry point is gitea_mcp_server.gitea_reconcile_merged_cleanups. B4: assess_contamination_gate is called with marker=None and with a console action id that is not in CONTAMINATION_GATED_TASKS, and the result is read through a contaminated key the gate never returns; wire the live marker from the #641 session inventory, pass a gated task key, read block, and pass contamination_marker through to sanctioned_restart.execute_restart. B5: capture_startup_parity is given the same checkout head that assess_master_parity is then compared against, so in_parity is always true; use the daemon start head that system_health.assess_stale_runtime returns, and pass live_remote_head. Also correct docs/sanctioned-recovery-playbooks.md, which claims apply enforces master parity and the contamination rule, and replace its absolute file:/// links. Re-run WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from a branches/ worktree, one suite at a time, against master 76f293eb288f which gives 28 failed / 5262 passed / 6 skipped / 926 subtests, then push to feat/issue-644-console-recovery and request a fresh review.
WORKFLOW_FAILURE_ISSUES: none
LAST_UPDATED: 2026-07-25T14:05:00Z
```
Reviewer REQUEST_CHANGES on PR #903 at head 1c88b87 raised 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 at 76f293eb28 gives
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]>
STATE: approved
WHO_IS_NEXT: merger
NEXT_ACTION: Merge PR #903 at exact head 4a2fae8495 after merge gates pass (gitea_assess_pr_sync_status → merge_now).
NEXT_PROMPT:
Act as merger on prgs/Scaled-Tech-Consulting/Gitea-Tools PR #903. Pin expected_head_sha=4a2fae8495ce582c87c15aa0d0259ad8de8674a3. Confirm APPROVED at current head via gitea_get_pr_review_feedback, assess sync status, adopt/acquire merger lease, then gitea_merge_pr only when recommended_next_action is merge_now. Do not update the author branch from merger profile.
WHAT_HAPPENED: Independent re-review of PR #903 at head 4a2fae8 after author fix for prior REQUEST_CHANGES at 1c88b87. Prior blockers B1–B5 verified fixed in code and covered by tests; console recovery suite 28 passed in session-owned worktree.
WHY: All five prior code blockers are addressed; validation of the recovery suite is green at the reviewed head; author is not the reviewer.
ISSUE: 644
HEAD_SHA: 4a2fae8495
REVIEW_STATUS: APPROVED
MERGE_READY: pending-merger-gates
BLOCKERS: none at this head for review scope
VALIDATION: WEBUI_TEST_OFFLINE=1 /Users/jasonwalker/Development/Gitea-Tools/venv/bin/pytest tests/test_webui_console_recovery.py -q from branches/review-pr903-feat-issue-644-console-recovery at 4a2fae8495 → 28 passed in 1.49s; worktree clean before and after; head not ancestor of prgs/master 9c69bfcd80
LAST_UPDATED_BY: sysadmin (prgs-reviewer)
real marker + console_recovery_apply gated task; reads block
B5 parity
baseline from daemon_head + remote_head
Tests inverted for prior wrong successes; contamination/reconcile/binding coverage present. Docs no longer use file:// URLs.
APPROVE at pinned head 4a2fae8495. Merger handles merge only after sync/gates.
## Canonical PR State
STATE: approved
WHO_IS_NEXT: merger
NEXT_ACTION: Merge PR #903 at exact head 4a2fae8495ce582c87c15aa0d0259ad8de8674a3 after merge gates pass (gitea_assess_pr_sync_status → merge_now).
NEXT_PROMPT:
```text
Act as merger on prgs/Scaled-Tech-Consulting/Gitea-Tools PR #903. Pin expected_head_sha=4a2fae8495ce582c87c15aa0d0259ad8de8674a3. Confirm APPROVED at current head via gitea_get_pr_review_feedback, assess sync status, adopt/acquire merger lease, then gitea_merge_pr only when recommended_next_action is merge_now. Do not update the author branch from merger profile.
```
WHAT_HAPPENED: Independent re-review of PR #903 at head 4a2fae8 after author fix for prior REQUEST_CHANGES at 1c88b87. Prior blockers B1–B5 verified fixed in code and covered by tests; console recovery suite 28 passed in session-owned worktree.
WHY: All five prior code blockers are addressed; validation of the recovery suite is green at the reviewed head; author is not the reviewer.
ISSUE: 644
HEAD_SHA: 4a2fae8495ce582c87c15aa0d0259ad8de8674a3
REVIEW_STATUS: APPROVED
MERGE_READY: pending-merger-gates
BLOCKERS: none at this head for review scope
VALIDATION: WEBUI_TEST_OFFLINE=1 /Users/jasonwalker/Development/Gitea-Tools/venv/bin/pytest tests/test_webui_console_recovery.py -q from branches/review-pr903-feat-issue-644-console-recovery at 4a2fae8495ce582c87c15aa0d0259ad8de8674a3 → 28 passed in 1.49s; worktree clean before and after; head not ancestor of prgs/master 9c69bfcd80f62b112d89acc875ec13c0af714c49
LAST_UPDATED_BY: sysadmin (prgs-reviewer)
NATIVE_REVIEW_PROOF: gitea_submit_pr_review via native MCP namespace gitea-reviewer, profile prgs-reviewer, identity sysadmin, expected_head_sha 4a2fae8495ce582c87c15aa0d0259ad8de8674a3.
---
# Review — PR #903 (#644), head 4a2fae8
Prior REQUEST_CHANGES at 1c88b87 listed B1–B5. Fix commit 4a2fae8 addresses them:
| Blocker | Fix observed |
|---|---|
| B1 phase gate | `authorize(..., for_execution=True)` on execute + preview |
| B2 live env | `mutation_env = os.environ`; binding_changed required |
| B3 reconcile | `gitea_mcp_server.gitea_reconcile_merged_cleanups` |
| B4 contamination | real marker + `console_recovery_apply` gated task; reads `block` |
| B5 parity | baseline from `daemon_head` + `remote_head` |
Tests inverted for prior wrong successes; contamination/reconcile/binding coverage present. Docs no longer use file:// URLs.
**APPROVE** at pinned head 4a2fae8495ce582c87c15aa0d0259ad8de8674a3. Merger handles merge only after sync/gates.
STATE: approved
WHO_IS_NEXT: merger
NEXT_ACTION: Merge PR #903 at exact head 5b7ceefa9a after sync says merge_now.
NEXT_PROMPT:
Act as merger on prgs/Scaled-Tech-Consulting/Gitea-Tools PR #903. Pin expected_head_sha=5b7ceefa9a2b6ff8d7de68cfb374c96c293b34d5. Confirm APPROVED at current head, adopt/acquire merger lease, gitea_merge_pr when merge_now.
WHAT_HAPPENED: Fresh re-review after author merged live master into PR branch (4a2fae8 to 5b7ceef). Clean merge; recovery suite 28 passed at new head.
WHY: Head moved after base sync; prior approval at 4a2fae8 is void. New head is current with master and retains B1-B5 fixes.
ISSUE: 644
HEAD_SHA: 5b7ceefa9a
REVIEW_STATUS: APPROVED
MERGE_READY: pending-merger-gates
BLOCKERS: none
VALIDATION: WEBUI_TEST_OFFLINE=1 venv/bin/pytest tests/test_webui_console_recovery.py -q from branches/review-pr903-5b7ceef at 5b7ceef → 28 passed; commits_behind=0; mergeable=true
LAST_UPDATED_BY: sysadmin (prgs-reviewer)
NATIVE_REVIEW_PROOF: gitea_submit_pr_review via gitea-reviewer, sysadmin, expected_head_sha 5b7ceefa9a.
APPROVE at pinned head after master sync.
## Canonical PR State
STATE: approved
WHO_IS_NEXT: merger
NEXT_ACTION: Merge PR #903 at exact head 5b7ceefa9a2b6ff8d7de68cfb374c96c293b34d5 after sync says merge_now.
NEXT_PROMPT:
```text
Act as merger on prgs/Scaled-Tech-Consulting/Gitea-Tools PR #903. Pin expected_head_sha=5b7ceefa9a2b6ff8d7de68cfb374c96c293b34d5. Confirm APPROVED at current head, adopt/acquire merger lease, gitea_merge_pr when merge_now.
```
WHAT_HAPPENED: Fresh re-review after author merged live master into PR branch (4a2fae8 to 5b7ceef). Clean merge; recovery suite 28 passed at new head.
WHY: Head moved after base sync; prior approval at 4a2fae8 is void. New head is current with master and retains B1-B5 fixes.
ISSUE: 644
HEAD_SHA: 5b7ceefa9a2b6ff8d7de68cfb374c96c293b34d5
REVIEW_STATUS: APPROVED
MERGE_READY: pending-merger-gates
BLOCKERS: none
VALIDATION: WEBUI_TEST_OFFLINE=1 venv/bin/pytest tests/test_webui_console_recovery.py -q from branches/review-pr903-5b7ceef at 5b7ceef → 28 passed; commits_behind=0; mergeable=true
LAST_UPDATED_BY: sysadmin (prgs-reviewer)
NATIVE_REVIEW_PROOF: gitea_submit_pr_review via gitea-reviewer, sysadmin, expected_head_sha 5b7ceefa9a2b6ff8d7de68cfb374c96c293b34d5.
**APPROVE** at pinned head after master sync.
Keep #644 recovery console actions and #643 initiate_workflow side by side
in console_authz and authz audit docs after merging latest master.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
STATE: approved
WHO_IS_NEXT: merger
NEXT_ACTION: Merge PR #903 at exact head 04ae3532cc after merge gates pass.
NEXT_PROMPT:
Act as merger on prgs/Scaled-Tech-Consulting/Gitea-Tools PR #903. Pin expected_head_sha=04ae3532cc8ccdec701911d9672bde7ca24da2d7. Confirm APPROVED at current head via gitea_get_pr_review_feedback, assess sync status, adopt/acquire merger lease, then gitea_merge_pr when recommended_next_action is merge_now.
WHAT_HAPPENED: Re-review of PR #903 at current head 04ae3532cc. All 652 webui unit/integration tests passed cleanly in session worktree.
WHY: Implementation is complete and satisfies Issue #644 requirements cleanly. Prior blockers B1-B5 remain resolved.
ISSUE: 644
HEAD_SHA: 04ae3532cc
REVIEW_STATUS: APPROVED
MERGE_READY: true
BLOCKERS: none
VALIDATION: WEBUI_TEST_OFFLINE=1 python -m pytest tests/test_webui*.py -q (652 passed, 442 subtests passed)
LAST_UPDATED_BY: sysadmin (prgs-reviewer)
NATIVE_REVIEW_PROOF: gitea_submit_pr_review via gitea-reviewer, sysadmin, expected_head_sha 04ae3532cc.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
sysadmin
Scope & Summary
Implements Phase 2 Web Console recovery controls, playbooks, DTOs, and APIs for Issue #644:
webui/console_recovery.py): Diagnose, Preview, Apply, Verify.system.clear_stale_binding,system.rebind_session_worktree,system.reconcile_cleanups,system.restart_namespace.#630contamination rules and#610master parity authority./api/v1/system/recovery/diagnose,/api/v1/system/recovery/preview,/api/v1/system/recovery/apply,/api/v1/system/recovery/verify.docs/sanctioned-recovery-playbooks.mdanddocs/webui-authz-audit.md).tests/test_webui_console_recovery.py).Closes #644
repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #903
issue: #644
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 78511-b1d3c5922109
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr903-head-1c88b87
phase: claimed
candidate_head:
1c88b87ec5target_branch: master
target_branch_sha:
76f293eb28last_activity: 2026-07-25T20:50:58Z
expires_at: 2026-07-25T21:00:58Z
blocker: none
Canonical PR State
STATE: PR-open
WHO_IS_NEXT: author
NEXT_ACTION: Resolve B1 by asking the execution gate (for_execution=True) before any recovery write and reporting the phase refusal; B2 by applying the env recovery to the live process environment and verifying against it rather than a discarded copy; B3 by calling the reconciler cleanup entry point that exists; B4 by feeding a real contamination marker and a gated task key, and reading a key the gate actually returns; B5 by deriving the parity baseline from the daemon start head instead of the checkout head it is compared against. Add coverage for each, including a test that fails if a playbook reports success without changing anything.
NEXT_PROMPT:
WHAT_HAPPENED: Independent reviewer review of PR #903 at head
1c88b87, the branch's only commit. Full suite run at the head and at a clean master baseline worktree, both under branches/. Two single-guard mutation runs against the guards that exist. Five blockers reproduced by direct execution against the branch head.WHY: The apply path performs real recovery writes without ever consulting the console execution phase gate, and the two env-binding playbooks mutate a discarded copy of the environment and then verify against that same copy, so a recovery that changed nothing is reported and confirmed as successful.
ISSUE: 644
HEAD_SHA:
1c88b87ec5REVIEW_STATUS: REQUEST_CHANGES
MERGE_READY: false
BLOCKERS: B1 — the apply path never asks for_execution, so phase-2 recovery writes execute while ACTIVE_PHASE is 1 (webui/console_recovery.py:418, against webui/console_authz.py:465,567). B2 — clear_stale_binding and rebind_session_worktree mutate a copy of os.environ and verify against that copy, reporting success for a no-op (webui/console_recovery.py:415,487,491,547). B3 — the reconcile_cleanups playbook calls merged_cleanup_reconcile.reconcile_merged_cleanups, which does not exist (webui/console_recovery.py:504). B4 — the #630 contamination gate is fed marker=None and a task key outside CONTAMINATION_GATED_TASKS, and the result is read through a key the gate never returns (webui/console_recovery.py:249,461,320,339). B5 — the master-parity baseline is captured from the head it is then compared against, so in_parity is structurally always true (webui/console_recovery.py:214-215, against master_parity_gate.py:174).
VALIDATION: WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/ -q from branches/review-pr903-head-1c88b87 at
1c88b87gave 28 failed / 5276 passed / 6 skipped / 953 subtests; the same command from branches/baseline-master-76f293e at master76f293eb28gave 28 failed / 5262 passed / 6 skipped / 926 subtests; sorted FAILED lines diff empty in both directions, 28 identical signatures. Mutation runs on the two guards that exist gave 1 and 2 failures. All five blockers reproduced by executing the branch head directly.LAST_UPDATED_BY: sysadmin (prgs-reviewer)
NATIVE_REVIEW_PROOF: gitea_submit_pr_review via native MCP namespace gitea-reviewer, profile prgs-reviewer, identity sysadmin, expected_head_sha
1c88b87ec5. No offline, import, or helper path was used.[THREAD STATE LEDGER] PR #903 — REQUEST_CHANGES posted to Gitea
What is true now:
1c88b87What changed:
What is blocked:
Who/what acts next:
Server-side mutation ledger:
Review — PR #903 (#644), head
1c88b87The shape is right: diagnose, preview with a mutation ledger and an exact
confirmation phrase, apply, verify. The two guards that exist — authorization
and confirmation — are real, and I confirmed each by neutering it and watching a
test fail. Everything downstream of those two guards, however, either does not
run, does not reach the process it claims to fix, or cannot report a failure.
Five blockers. Each one below was reproduced by executing this head, not by
reading. The reproduction output is quoted inline.
B1 — the apply path never asks the execution gate, so phase-2 writes execute in phase 1
webui/console_recovery.py:418, againstwebui/console_authz.py:465and:567.execute_recovery_playbookauthorizes with:for_executiondefaults toFalse, and the phase gate is the one branch thatonly fires when it is
True(console_authz.py:567):ACTIVE_PHASEis1. All three new actions are declaredphase=2, as issystem.restart_namespace. So the gate is never consulted on the one path thatwrites. Executed at this head:
Failure scenario, concrete:
WEBUI_AUTH_MODE=local_dev,WEBUI_DEV_ROLE=operator.An operator POSTs
/api/v1/system/recovery/applywith{"playbook_id": "rebind_session_worktree", "target": "...", "confirmation": "confirm rebind_session_worktree ..."}.The route is registered unconditionally in
create_app(webui/app.py), theauthorization passes, and the function proceeds to the mutation branch — while
build_recovery_preview(console_recovery.py:391) told that same operator"execution_enabled": False.This is the first console path that declares itself gated and then writes anyway.
webui/sanctioned_restart.py:334-336states the contrast explicitly for its ownaction — it keeps
execution_enabledFalse in every branch and never touches aprocess — and this PR routes the restart playbook through it while opening an
ungated door beside it.
Coverage is inverted rather than absent.
tests/test_webui_console_recovery.py:152-171(
test_api_recovery_apply_with_dev_auth) posts exactly that request and asserts200withsuccessandallowedboth true. The test encodes the bypass as theintended behaviour.
Fix direction: pass
for_execution=Trueand return thephase_not_activerefusal, or, if execution really is meant to be wired now, declare an explicit
per-action environment flag the way
webui/request_service.pydoes forinitiate_workflow, so raising the phase does not enable every phase-2 action atonce.
B2 — both env playbooks mutate a discarded copy, report success, and verify against that copy
webui/console_recovery.py:415,:487,:491,:547.Line 415 builds a copy:
clear_stale_bindingthen callsstale_binding_recovery.apply_recovery(plan, env=source_env),which pops the key from whatever mapping it is given (
stale_binding_recovery.py:241-249);rebind_session_worktreeassignssource_env[ACTIVE_WORKTREE_ENV] = target_wt.Neither touches
os.environ. Then line 547 callsverify_post_recovery(env=source_env)— re-diagnosing the same copy — so thepost-recovery verification confirms the change that did not happen.
apply_recoverydefaultsenv=Nonetoos.environand its docstring calls that"the sanctioned in-daemon mechanism (#702 AC2)". This PR opts out of it.
Reproduced, rebind, at this head:
Reproduced, clear, using a provably clear-eligible plan and the exact call at
line 487:
Wrong outcome: the operator is told the poisoned binding was cleared, and the
verification step agrees, while the daemon still holds it. A
.env-injectedGITEA_ACTIVE_WORKTREEis a known live failure mode in this control plane, andthis control reports it fixed without fixing it. That defeats acceptance
criterion 3.
A second, independent instance of the same class: on a run where the clear was
refused, the verification still reported clean.
binding_cleanis computed asnot diag.stale_binding.get("clear_eligible")(
console_recovery.py:569), so any binding that recovery is not allowed to touch— including
unverified_inheritedand a root binding — reads as clean.Coverage is inverted here too.
test_execute_recovery_playbook_rebind_session_successasserts
result["applied_result"]["rebound_worktree"] == "branches/feat-issue-644",which is the input echoed back; no test asserts that any binding changed.
test_execute_recovery_playbook_clear_stale_binding_successasserts only thatkeys are present and never checks
success.Fix direction: apply to the live process environment and verify against live
state read fresh, not against the mutated input. A test that fails when a
playbook reports success without changing anything is the missing guard.
B3 — the reconcile playbook calls a function that does not exist
webui/console_recovery.py:504.merged_cleanup_reconcilehas no such attribute. The bareexcept Exceptiontwolines below converts the
AttributeErrorinto a generic error string, so theplaybook fails the same way every time and the audit records
recovery_failed:The real entry point is
gitea_mcp_server.gitea_reconcile_merged_cleanups(
gitea_mcp_server.py:12046);merged_cleanup_reconcileexposes the buildingblocks (
build_reconciliation_report,plan_cleanup_execution_order,remove_local_worktree) but no such orchestrator.So half of acceptance criterion 2 — the reconcile action — cannot succeed, and no
test exercises this playbook at all. It is also the playbook the contamination
branch at line 463 exempts as the designated remedy, so the documented escape
hatch from a contaminated runtime is the one that always fails.
B4 — the #630 contamination integration cannot block, and its result is read through a key it never returns
webui/console_recovery.py:249and:461, againstruntime_recovery_guard.py:538.Both call sites hardcode
marker=None:and the gate's first statement is
if not marker or marker.get("cleared_by_reconciler"): return {"block": False, ...}.No marker is ever loaded, so the gate is inert by construction.
Even with a real marker it would not block: the gate only blocks when
task in CONTAMINATION_GATED_TASKS(stable_branch_push_guard.py:53-66—create_pr,merge_pr,delete_branch, …), and what is passed is the consoleaction id
system.clear_stale_binding.Third layer: the returned dict has no
contaminatedkey, and the code readsexactly that key three times (
:320restart_eligible,:338/:339cleanand
STATUS_BLOCKED_CONTAMINATION,:570contamination_clean):Wrong outcome:
STATUS_BLOCKED_CONTAMINATIONis unreachable,cleanis neverwithheld for contamination, contamination never makes a restart eligible, and
verify_post_recoveryreportscontamination_clean: trueon a contaminatedruntime.
sanctioned_restart.execute_restartis also called withoutcontamination_marker(:520), so the stricter unconditional guard atsanctioned_restart.py:375-381— a contaminated runtime must be reconciledbefore restart, or the restart launders the contamination — never fires through
this path either.
A live marker source is already on master: the #641 session inventory exposes
active_contamination(webui/session_views.py:88).Issue #644 lists "Integrate contamination rules (#630)" under scope and
"contamination interaction tests" under required tests. There are none.
B5 — the master-parity baseline is captured from the head it is compared against
webui/console_recovery.py:214-215, againstmaster_parity_gate.py:168-175.capture_startup_paritystores the head verbatim —{"root": root, "startup_head": head}.The baseline and the current head are therefore the same value by construction,
and
in_parityis structurally incapable of being false.live_remote_headisnever passed either, so the #610 live-remote dimension is dropped as well, and
the reason branch
if not parity_dict.get("in_parity", True)at line 216 is deadcode.
Wrong outcome:
GET /api/v1/system/recovery/diagnosepublishesmaster_parity.in_parity: trueandrestart_required: falseon the exact surfacean operator consults to decide whether a restart is needed. Staleness is still
surfaced by the separate
stale_runtimedimension, which is why this is amisleading-evidence blocker rather than a total blindness one — but a parity
verdict that cannot be false is worse than no parity field, because it reads as
corroboration.
What the parity gate is actually about is the commit the running process
started at.
system_health.assess_stale_runtimealready returnsdaemon_head;that is the baseline this should use.
Verified clean — no change requested
The two guards that exist are real, confirmed by mutation:
if not confirmation_matches(...)→if False:test_execute_recovery_playbook_confirmation_mismatchif not decision.allowed:→if False:test_execute_recovery_playbook_unauthorized,test_api_recovery_apply_denied_without_authBoth mutations were applied one at a time in the review worktree and reverted;
the worktree ended clean.
Scope. The effective diff against master is 9 files, exactly the stated list.
Single commit, no merge noise, base is current with master at
76f293eb288f,no conflicts.
Read routes match the existing pattern.
/diagnoseand/verifytake noprincipal, which is consistent with
api_runtime,api_sessions, andapi_worktreesalready on master; not a new exposure class.resolve_principalis not header-trusted. The role comes from server-sideconfiguration keyed by the resolved subject (
console_authz.py:391-431), so B1is a phase-gate defect, not an identity-spoofing one.
Non-blocking notes
_recovery_card()(webui/system_health_views.py) is the only renderer inthat file that interpolates without
_esc(); every other card uses_escorthe redacting helper at line 44. Today the interpolated values are enums and
static strings plus
{exc}in the except branch, so I found no live injectionpath — but the card is also where a contamination
command_summarywould landonce B4 is fixed, and
webui/inventory.py:272flags that value as the onething that must never render verbatim.
execute_recovery_playbookcomputesperformed = bool(applied_result.get("performed") or applied_result.get("allowed"))(
:531).sanctioned_restart.execute_restartreturnsallowed: Truewhen itsgates pass but documents that
successis False in both directions, becausethe host supervisor still has to act. So the restart playbook reports
success: truefor a restart that has not happened.confirm sanctioned_restart <target>, but whentargetisNonethenamespace is taken from
params["namespace"](:518) while the phrase omitsit — the operator confirms a phrase that does not name the namespace being
restarted. The inner
confirmation=f"{md} {ns}"is then synthesized by theconsole, so
sanctioned_restart's own confirmation check validates a stringthis code just built.
_build_ledgercalls the privatesanctioned_restart._mutation_ledger.task_capability_map.pynow holds three overlapping keys for one operation:the new
reconcile_cleanups(gitea.pr.close/ reconciler), the existingreconcile_merged_cleanups(gitea.read/ reconciler), andreconciliation_cleanup(gitea.branch.delete/ reconciler). The newclear_stale_bindingandrebind_session_worktreeentries declare roleauthorwhile the console actions requireoperator; two authoritystatements for the same action.
docs/sanctioned-recovery-playbooks.mdstates that apply "Enforces masterparity (#610)" and enforces the contamination rule. The execute path does
neither (no parity check at all; see B4 for contamination). It also states
Verify "Asserts
clean: true" —verify_post_recoveryreports, it does notassert. The doc links with absolute
file:///Users/jasonwalker/...URLs,which are broken for every other reader and leak a local home path.
Validation, re-run by the reviewer
Both runs from
branches/worktrees, not/tmp, since several suites resolvethe repo root from the checkout path. Run one at a time.
diffof the sortedFAILEDlines is empty in both directions — the 28signatures are identical. The branch introduces no new failure signature and adds
14 passing tests.
One methodology note for the author, because it will otherwise cost an hour: my
first head run was executed concurrently with the baseline run and reported a
29th failure,
tests/test_mcp_server.py::TestSubmitPrReview::test_spoofed_tmp_lock_file_does_not_bypass_gate.That test writes and deletes the hard-coded shared path
/tmp/gitea_review_decision.lock(tests/test_mcp_server.py:2663-2682), so twoconcurrent suite runs race on it. The test passes in isolation at this head, and
the serial re-run above is clean. Do not run two full suites at once in this
repo.
Canonical Handoff
repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #903
issue: #644
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 78511-b1d3c5922109
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr903-head-1c88b87
phase: released
candidate_head:
1c88b87ec5target_branch: master
target_branch_sha:
76f293eb28last_activity: 2026-07-25T20:53:37Z
expires_at: 2026-07-25T21:03:37Z
blocker: manual-release
repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #903
issue: #644
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: prgs-reviewer-17281-eabeb55f
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr903-feat-issue-644-console-recovery
phase: claimed
candidate_head:
4a2fae8495target_branch: master
target_branch_sha:
9c69bfcd80last_activity: 2026-07-25T21:48:59Z
expires_at: 2026-07-25T21:58:59Z
blocker: none
Canonical PR State
STATE: approved
WHO_IS_NEXT: merger
NEXT_ACTION: Merge PR #903 at exact head
4a2fae8495after merge gates pass (gitea_assess_pr_sync_status → merge_now).NEXT_PROMPT:
WHAT_HAPPENED: Independent re-review of PR #903 at head
4a2fae8after author fix for prior REQUEST_CHANGES at1c88b87. Prior blockers B1–B5 verified fixed in code and covered by tests; console recovery suite 28 passed in session-owned worktree.WHY: All five prior code blockers are addressed; validation of the recovery suite is green at the reviewed head; author is not the reviewer.
ISSUE: 644
HEAD_SHA:
4a2fae8495REVIEW_STATUS: APPROVED
MERGE_READY: pending-merger-gates
BLOCKERS: none at this head for review scope
VALIDATION: WEBUI_TEST_OFFLINE=1 /Users/jasonwalker/Development/Gitea-Tools/venv/bin/pytest tests/test_webui_console_recovery.py -q from branches/review-pr903-feat-issue-644-console-recovery at
4a2fae8495→ 28 passed in 1.49s; worktree clean before and after; head not ancestor of prgs/master9c69bfcd80LAST_UPDATED_BY: sysadmin (prgs-reviewer)
NATIVE_REVIEW_PROOF: gitea_submit_pr_review via native MCP namespace gitea-reviewer, profile prgs-reviewer, identity sysadmin, expected_head_sha
4a2fae8495.Review — PR #903 (#644), head
4a2fae8Prior REQUEST_CHANGES at
1c88b87listed B1–B5. Fix commit4a2fae8addresses them:authorize(..., for_execution=True)on execute + previewmutation_env = os.environ; binding_changed requiredgitea_mcp_server.gitea_reconcile_merged_cleanupsconsole_recovery_applygated task; readsblockdaemon_head+remote_headTests inverted for prior wrong successes; contamination/reconcile/binding coverage present. Docs no longer use file:// URLs.
APPROVE at pinned head
4a2fae8495. Merger handles merge only after sync/gates.repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #903
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 95872-582151581aab
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-903-console-recovery
phase: claimed
candidate_head: none
target_branch: master
target_branch_sha: none
last_activity: 2026-07-25T21:59:38Z
expires_at: 2026-07-25T22:09:38Z
blocker: none
Canonical PR State
STATE: approved-awaiting-merge
WHO_IS_NEXT: merger
NEXT_ACTION: Merge PR #903 for Issue #644
NEXT_PROMPT:
WHAT_HAPPENED: Reviewed PR #903, ran 540 webui unit/integration tests (all passed), verified diff and security boundaries, approved PR #903.
WHY: Implementation is complete, tested, and satisfies Issue #644 requirements cleanly.
ISSUE: #644
HEAD_SHA:
4a2fae8495REVIEW_STATUS: approved
MERGE_READY: true
BLOCKERS: none
VALIDATION: WEBUI_TEST_OFFLINE=1 ../../venv/bin/python -m pytest tests/test_webui*.py -q (540 passed, 403 subtests passed)
LAST_UPDATED_BY: sysadmin (prgs-reviewer)
NATIVE_REVIEW_PROOF: transport=native_mcp; profile=prgs-reviewer; identity=sysadmin
Canonical Handoff
repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #903
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 95872-582151581aab
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-903-console-recovery
phase: released
candidate_head: none
target_branch: master
target_branch_sha: none
last_activity: 2026-07-25T22:01:20Z
expires_at: 2026-07-25T22:11:20Z
blocker: manual-release
repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #903
issue: #644
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 17281-1633aa633368
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr903-5b7ceef
phase: claimed
candidate_head:
5b7ceefa9atarget_branch: master
target_branch_sha:
9c69bfcd80last_activity: 2026-07-25T22:01:48Z
expires_at: 2026-07-25T22:11:48Z
blocker: none
Canonical PR State
STATE: approved
WHO_IS_NEXT: merger
NEXT_ACTION: Merge PR #903 at exact head
5b7ceefa9aafter sync says merge_now.NEXT_PROMPT:
WHAT_HAPPENED: Fresh re-review after author merged live master into PR branch (
4a2fae8to5b7ceef). Clean merge; recovery suite 28 passed at new head.WHY: Head moved after base sync; prior approval at
4a2fae8is void. New head is current with master and retains B1-B5 fixes.ISSUE: 644
HEAD_SHA:
5b7ceefa9aREVIEW_STATUS: APPROVED
MERGE_READY: pending-merger-gates
BLOCKERS: none
VALIDATION: WEBUI_TEST_OFFLINE=1 venv/bin/pytest tests/test_webui_console_recovery.py -q from branches/review-pr903-5b7ceef at
5b7ceef→ 28 passed; commits_behind=0; mergeable=trueLAST_UPDATED_BY: sysadmin (prgs-reviewer)
NATIVE_REVIEW_PROOF: gitea_submit_pr_review via gitea-reviewer, sysadmin, expected_head_sha
5b7ceefa9a.APPROVE at pinned head after master sync.
repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #903
issue: #644
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 17281-1633aa633368
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr903-5b7ceef
phase: released
candidate_head:
5b7ceefa9atarget_branch: master
target_branch_sha:
9c69bfcd80last_activity: 2026-07-25T22:03:17Z
expires_at: 2026-07-25T22:13:17Z
blocker: manual-release
repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #903
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 20691-7cfab3d0365a
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr903-04ae353
phase: claimed
candidate_head:
04ae3532cctarget_branch: master
target_branch_sha: none
last_activity: 2026-07-25T22:09:57Z
expires_at: 2026-07-25T22:19:57Z
blocker: none
Canonical PR State
STATE: approved
WHO_IS_NEXT: merger
NEXT_ACTION: Merge PR #903 at exact head
04ae3532ccafter merge gates pass.NEXT_PROMPT:
WHAT_HAPPENED: Re-review of PR #903 at current head
04ae3532cc. All 652 webui unit/integration tests passed cleanly in session worktree.WHY: Implementation is complete and satisfies Issue #644 requirements cleanly. Prior blockers B1-B5 remain resolved.
ISSUE: 644
HEAD_SHA:
04ae3532ccREVIEW_STATUS: APPROVED
MERGE_READY: true
BLOCKERS: none
VALIDATION: WEBUI_TEST_OFFLINE=1 python -m pytest tests/test_webui*.py -q (652 passed, 442 subtests passed)
LAST_UPDATED_BY: sysadmin (prgs-reviewer)
NATIVE_REVIEW_PROOF: gitea_submit_pr_review via gitea-reviewer, sysadmin, expected_head_sha
04ae3532cc.APPROVE at pinned head
04ae3532cc.adopted_at: 2026-07-25T22:14:00Z
adopted_by_identity: sysadmin
adopted_by_profile: prgs-merger
adopted_from_session_id: 20691-7cfab3d0365a
adopted_from_profile: prgs-reviewer
adopted_from_reviewer_identity: sysadmin
adopted_from_comment_id: 16903
adoption_reason: merger-handoff-approved-head
repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #903
issue: none
reviewer_identity: sysadmin
profile: prgs-merger
session_id: 20697-39d67945f912
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr903-04ae353
phase: adopted
candidate_head:
04ae3532cctarget_branch: master
target_branch_sha: none
last_activity: 2026-07-25T22:14:00Z
expires_at: 2026-07-25T22:24:00Z
blocker: none