fix(mcp): invalidate review session state on cross-profile activation (Closes #690) #924

2026-07-25T18:26:03-05:00

jcwalker3 commented

2026-07-25 18:26:03 -05:00

Summary

Hardens formal-review runs against cross-role profile activation mid-session (#690), spawned from the PR #688 / issue #687 formal-review recovery path.

A mid-run profile switch (e.g. reviewer → author → reviewer) previously left workflow-load proof, reviewer lease binding, the review decision lock, live namespace health, and preflight identity/capability stamps intact, so a formal verdict could be recorded under contaminated session state (wrong role stamp, stale lease, inherited capability).

Changes

gitea_activate_profile now treats a cross-profile switch as a session boundary and invalidates, in memory and in durable session state keyed by either profile identity:
- preflight identity/capability stamps,
- review workflow-load proof,
- the review decision lock (incl. final_review_decision_ready),
- the reviewer PR session lease,
- live namespace-health assessments.
  The activation result reports review_state_invalidation + re_preflight_required with an exact next action.
Full reviewer preflight required after any switch before a formal verdict: gitea_whoami, gitea_load_review_workflow, gitea_resolve_task_capability(review_pr), live head re-pin, lease re-acquire/adopt. Switching back to the prior profile cannot resurrect the stale run (durable state cleared for both identities).
Namespace provenance (#690 AC4): optional launcher-declared GITEA_MCP_NAMESPACE is reported as namespace_provenance by gitea_whoami, gitea_get_runtime_context, and gitea_resolve_task_capability; a declared namespace that disagrees with a task's required namespace (review_pr → gitea-reviewer, merge_pr → gitea-merger) fails closed. Undeclared = unknown, never treated as proof.
Docs: supported pattern is separate session/namespace per role (dual-namespace), not in-process profile hopping mid-review; profile switching is the operator-approved exception with the re-preflight cost.

Acceptance criteria coverage

✅ Cross-role activation strictly isolated: review state invalidated unless fully re-preflighted.
✅ Workflow-load proof, lease binding, decision markers, capability state invalidated on profile change.
✅ Full reviewer preflight required post-switch (existing gates fail closed on cleared state).
✅ Namespace provenance reported; fail closed on task/namespace mismatch.
✅ Tests: mid-run switch without re-preflight blocks verdict gate; clean re-preflight allows; switch-back cannot resurrect; same-profile re-activation preserves state; namespace mismatch/unknown cases; whoami provenance; resolver fail-closed on mismatch.
✅ Docs updated (docs/gitea-execution-profiles.md, docs/mcp-namespace-health.md).

Test evidence

New: tests/test_issue_690_profile_switch_review_guard.py — 7/7 pass.
Targeted sweep (namespace/review-workflow/decision-lock/runtime/session/capability/preflight/profile, 728 tests): 724 pass; 4 failures reproduce identically on unmodified master (pre-existing baseline, verified by running the same tests on the master checkout).
Full suite comparison vs master baseline: see follow-up comment.

Non-goals respected

No #687 branch-delete/migration work; no process-kill recovery; #332 terminal-mutation accounting untouched.

Handoff

WHO_IS_NEXT: reviewer — please review against #690 ACs. Active-config proof: this PR was authored through the live gitea-author namespace (gitea_whoami verified jcwalker3 / prgs-author).

## Summary Hardens formal-review runs against cross-role profile activation mid-session (#690), spawned from the PR #688 / issue #687 formal-review recovery path. A mid-run profile switch (e.g. reviewer → author → reviewer) previously left workflow-load proof, reviewer lease binding, the review decision lock, live namespace health, and preflight identity/capability stamps intact, so a formal verdict could be recorded under contaminated session state (wrong role stamp, stale lease, inherited capability). ## Changes - **`gitea_activate_profile`** now treats a cross-profile switch as a session boundary and invalidates, in memory **and** in durable session state keyed by *either* profile identity: - preflight identity/capability stamps, - review workflow-load proof, - the review decision lock (incl. `final_review_decision_ready`), - the reviewer PR session lease, - live namespace-health assessments. The activation result reports `review_state_invalidation` + `re_preflight_required` with an exact next action. - **Full reviewer preflight required after any switch** before a formal verdict: `gitea_whoami`, `gitea_load_review_workflow`, `gitea_resolve_task_capability(review_pr)`, live head re-pin, lease re-acquire/adopt. Switching back to the prior profile cannot resurrect the stale run (durable state cleared for both identities). - **Namespace provenance (#690 AC4):** optional launcher-declared `GITEA_MCP_NAMESPACE` is reported as `namespace_provenance` by `gitea_whoami`, `gitea_get_runtime_context`, and `gitea_resolve_task_capability`; a declared namespace that disagrees with a task's required namespace (`review_pr` → `gitea-reviewer`, `merge_pr` → `gitea-merger`) fails closed. Undeclared = `unknown`, never treated as proof. - **Docs:** supported pattern is separate session/namespace per role (dual-namespace), not in-process profile hopping mid-review; profile switching is the operator-approved exception with the re-preflight cost. ## Acceptance criteria coverage 1. ✅ Cross-role activation strictly isolated: review state invalidated unless fully re-preflighted. 2. ✅ Workflow-load proof, lease binding, decision markers, capability state invalidated on profile change. 3. ✅ Full reviewer preflight required post-switch (existing gates fail closed on cleared state). 4. ✅ Namespace provenance reported; fail closed on task/namespace mismatch. 5. ✅ Tests: mid-run switch without re-preflight blocks verdict gate; clean re-preflight allows; switch-back cannot resurrect; same-profile re-activation preserves state; namespace mismatch/unknown cases; whoami provenance; resolver fail-closed on mismatch. 6. ✅ Docs updated (`docs/gitea-execution-profiles.md`, `docs/mcp-namespace-health.md`). ## Test evidence - New: `tests/test_issue_690_profile_switch_review_guard.py` — **7/7 pass**. - Targeted sweep (namespace/review-workflow/decision-lock/runtime/session/capability/preflight/profile, 728 tests): 724 pass; 4 failures reproduce **identically on unmodified master** (pre-existing baseline, verified by running the same tests on the master checkout). - Full suite comparison vs master baseline: see follow-up comment. ## Non-goals respected - No #687 branch-delete/migration work; no process-kill recovery; #332 terminal-mutation accounting untouched. ## Handoff **WHO_IS_NEXT:** reviewer — please review against #690 ACs. Active-config proof: this PR was authored through the live `gitea-author` namespace (`gitea_whoami` verified `jcwalker3` / `prgs-author`).

jcwalker3 added 1 commit 2026-07-25 18:26:04 -05:00

fix(mcp): invalidate review session state on cross-profile activation (Closes #690 ) 2b3f5baaeb

A mid-run profile switch (reviewer -> author -> reviewer) left workflow-load
proof, reviewer lease binding, the review decision lock, live namespace
health, and preflight identity/capability stamps intact, so a formal verdict
could be recorded under contaminated session state.

- gitea_activate_profile now invalidates all review-critical session state
  on a cross-profile switch, in memory and in durable state keyed by either
  profile identity, and reports the invalidation + re-preflight requirement.
- Full reviewer preflight (whoami, load_review_workflow,
  resolve_task_capability(review_pr), head re-pin, lease re-acquire) is
  required before any formal verdict after a switch; switching back cannot
  resurrect the stale run.
- Namespace provenance: optional launcher-declared GITEA_MCP_NAMESPACE is
  reported by whoami/runtime context/capability resolution, and a declared
  namespace that disagrees with a task's required namespace fails closed.
- Docs: supported pattern is separate session/namespace per role, not
  in-process profile hopping mid-review.

sysadmin referenced this pull request

2026-07-26 08:46:27 -05:00

fix(author bootstrap): restore missing runtime identity and session helpers (Closes #943) #944

sysadmin referenced this pull request

2026-07-26 08:48:10 -05:00

fix(author bootstrap): restore missing runtime identity and session helpers #943

sysadmin commented

2026-07-27 15:03:11 -05:00

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #924
issue: #690
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 18879-9e0ffc453847
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr924-2b3f5ba-s2
phase: claimed
candidate_head: 2b3f5baaeb
target_branch: master
target_branch_sha: c30b381eb2
last_activity: 2026-07-27T20:03:11Z
expires_at: 2026-07-27T20:13:11Z
blocker: none

repo: Scaled-Tech-Consulting/Gitea-Tools pr: #924 issue: #690 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 18879-9e0ffc453847 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr924-2b3f5ba-s2 phase: claimed candidate_head: 2b3f5baaeb7d4b87a4f0158005fe667289f8161d target_branch: master target_branch_sha: c30b381eb2756c4ba9cbb2bf7c3f2d6f31b701e7 last_activity: 2026-07-27T20:03:11Z expires_at: 2026-07-27T20:13:11Z blocker: none

sysadmin requested changes 2026-07-27 15:06:58 -05:00

sysadmin left a comment

REQUEST_CHANGES — PR #924 at head `2b3f5baaeb7d4b87a4f0158005fe667289f8161d`

Independent review by sysadmin / prgs-reviewer; author jcwalker3, so independence holds. Validated in a fresh session-owned worktree branches/review-pr924-2b3f5ba-s2, detached at exactly this head, clean before and after. Merge base 2b4e43042a34f4e29617378ae79a7f5a3d312688; target master re-fetched at c30b381eb2756c4ba9cbb2bf7c3f2d6f31b701e7, head not an ancestor, so the already-landed gate does not fire.

The premise is right and most of the execution is good. A cross-profile activation genuinely is a session boundary, and clearing workflow-load proof, the reviewer lease, preflight stamps and namespace health under both the before and after identities is the correct treatment — the before/after loop closes the reviewer → author → reviewer resurrection path properly. AC4's namespace provenance is a clean addition: it is reported rather than trusted, unknown when undeclared, and the resolver's not ns_provenance.get("mismatch") conjunct fails closed on a declared mismatch.

One blocker, and it is in the same statement list.

B1 — BLOCKER: clearing the decision lock discards terminal-mutation history, so a profile switch re-admits a second verdict on the same PR at the same head

_invalidate_review_state_on_profile_switch calls _save_review_decision_lock(None). That writer does not clear selectively — on None it clears the durable record and sets _REVIEW_DECISION_LOCK = None outright, discarding live_mutations and last_terminal along with the authority fields.

live_mutations is the sole input to the duplicate-verdict refusal. stale_review_decision_lock.prior_live_mutations_block_boundary opens with:

if not lock:
    return False

so a cleared lock disables the guard rather than failing closed. Both consumers depend on it — gitea_mcp_server.py:6294 and :7274.

Reproduced with the real predicate, a lock shaped as it exists immediately after a reviewer records a terminal verdict on PR 999 at this head:

BEFORE profile switch  -> second verdict blocked: True
AFTER  profile switch  -> second verdict blocked: False

cross-PR isolation still intact on an unswitched lock (PR 1000): False

So the sequence is: post REQUEST_CHANGES on PR X at head H → activate another profile and return → run the full reviewer preflight this PR documents as the remedy → mark_final on PR X at head H succeeds. #332's one-terminal-mutation rule and #620/#693's per-(PR, head) refinement are both defeated, and the PR's own prescribed recovery path is the thing that walks through the hole.

The repository already states the principle being violated, at the writer this change calls — gitea_mcp_server.py, _save_review_decision_lock: "#720: durable decision locks are recovery-critical terminal provenance, not generic TTL session cache." Provenance of what already happened is not contaminated by a role switch; a later profile cannot un-post a review that Gitea has already recorded.

Fix direction: separate authority state from history state. A role switch revokes may I act — workflow-load proof, lease binding, preflight stamps, final_review_decision_ready — and it should. It must not revoke what already happened — live_mutations, last_terminal, locked_head_sha. Clear the former and preserve the latter, rather than nulling the whole record.

Test-coverage note, stated precisely. _seed_contaminated_review_state seeds {"session_profile", "final_review_decision_ready", "ready_pr_number": 688} with no live_mutations — so test_clean_repreflight_after_switch_allows_gate exercises a prepared but unconsumed verdict, and asserting the gate reopens there is legitimate. I am not claiming that test encodes the defect. The gap is that no test seeds a consumed terminal mutation across a switch, which is why B1 is invisible to a suite that passes 7/7. The test at line 119 asserting "review_decision_lock" appears in invalidated will keep passing under the corrected behaviour, since the lock's authority fields would still be invalidated.

Non-blocking observations

The durable cleanup loop wraps clear_state in except Exception: pass. In-memory state is already reset by then, so the failure mode is a surviving durable record whose in-memory counterpart is gone — the opposite skew from B1, and a state the invalidated list would nonetheless report as cleared. A recorded reason would make that visible.
_PROFILE_SWITCH_INVALIDATION is a module global that is never reset except by the test conftest, so gitea_get_runtime_context reports the most recent switch for the remainder of the process even after a full clean re-preflight. As diagnostics that is defensible, but nothing distinguishes "switch happened and was remediated" from "switch happened", and re_preflight_required: True stays pinned either way.
tests/conftest.py gains GITEA_MCP_NAMESPACE to the unset list and resets the new global. Both are correct and prevent cross-test leakage of the new state.

Validation

worktree : branches/review-pr924-2b3f5ba-s2 (detached at 2b3f5ba, clean before and after)
baseline : branches/baseline-pr924-2b4e430-s2 (detached at 2b4e4304, merge base)

head : venv/bin/python -m pytest tests/test_issue_690_profile_switch_review_guard.py -q
       -> 7 passed

head : venv/bin/python -m pytest tests/ -q --tb=no -k "profile or decision_lock or namespace or review_workflow or 690 or 693 or 620"
       -> 1 failed, 483 passed, 5038 deselected, 81 subtests
base : same command, in the baseline worktree
       -> 1 failed, 476 passed, 5038 deselected, 81 subtests

failing test id sets: comm -23 and comm -13 both empty -> IDENTICAL

The single failure is tests/test_mcp_server.py::TestRuntimeProfile::test_whoami_v2_metadata. Because this PR adds a namespace_provenance key to the gitea_whoami payload, that failure looked like a likely regression and was checked rather than assumed: it reproduces at the merge base with the identical test id, so it is pre-existing. The +7 passes over base are exactly this PR's new tests.

Official validation status: baseline-equivalent failure accepted. No other validation failure was observed in this session. No full-suite run was performed at this head and none is claimed. No file in either worktree was edited; the B1 reproduction invoked the predicate in-process and wrote nothing.

Canonical PR State

STATE: PR #924 is open at head 2b3f5baaeb and now carries a formal REQUEST_CHANGES verdict from sysadmin recorded at that exact head. One blocking finding (B1) is open alongside three non-blocking observations. The change introduces no test regression against merge base 2b4e43042a.

WHO_IS_NEXT: author

NEXT_ACTION: Author jcwalker3 must preserve terminal-mutation history across a profile switch while continuing to invalidate authority state, add a regression test that seeds a consumed terminal mutation across a switch, push, and publish a new head-pinned handoff for a fresh independent review.

NEXT_PROMPT:

Address the REQUEST_CHANGES review on PR #924 (Closes #690) in
Scaled-Tech-Consulting/Gitea-Tools on remote prgs.

Invoke the canonical gitea-workflow skill first. Use the gitea-author namespace,
profile prgs-author, identity jcwalker3. Reviewed head was
2b3f5baaeb7d4b87a4f0158005fe667289f8161d; merge base 2b4e43042a34f4e29617378ae79a7f5a3d312688.

B1 (blocker) — _invalidate_review_state_on_profile_switch calls
_save_review_decision_lock(None), which clears the whole record including
live_mutations and last_terminal. stale_review_decision_lock
.prior_live_mutations_block_boundary returns False on a falsy lock, so the
duplicate-verdict guard is disabled rather than failing closed; its two consumers
are gitea_mcp_server.py:6294 and :7274. Reproduced with the real predicate: a lock
holding one terminal mutation for PR 999 at the reviewed head blocks a second
verdict (True), and the same query after the switch clears it does not (False).
The result is that posting REQUEST_CHANGES on a PR, switching profile and back,
then running the full re-preflight this PR prescribes, admits a second terminal
verdict on the same PR at the same head, defeating #332 and #620/#693.

Fix: separate authority state from history state. Keep invalidating workflow-load
proof, reviewer lease binding, preflight identity/capability stamps and
final_review_decision_ready. Preserve live_mutations, last_terminal and
locked_head_sha across the switch instead of nulling the record. Note the comment
already on _save_review_decision_lock: "#720: durable decision locks are
recovery-critical terminal provenance, not generic TTL session cache."

Test gap: _seed_contaminated_review_state seeds no live_mutations, so no test
covers a consumed terminal mutation across a switch. Add one that seeds a terminal
mutation for a PR at a head, performs reviewer -> author -> reviewer, completes a
clean re-preflight, and asserts mark_final on that same PR and head is still
refused. The existing assertion that "review_decision_lock" appears in the
invalidated list should continue to pass under the corrected behaviour.

Not in scope for the fix: the before/after identity loop that clears durable state
under both profile identities is correct and closes the switch-back resurrection
path; the AC4 namespace provenance work is sound and fails closed on a declared
mismatch. Do not rework either. The single failing test
test_whoami_v2_metadata reproduces on the merge base and is not caused by this PR.

Do not self-review and do not self-merge.

WHAT_HAPPENED: An independent review at the exact head read all six changed files, traced the invalidation statement list to the decision-lock writer and then to the duplicate-verdict predicate and its two consumers, and reproduced the resulting behaviour change by invoking the real predicate against a lock shaped as it exists after a terminal verdict. The seed helper used by the new tests was read to establish precisely what the suite does and does not cover, and the claim that a specific test encodes the defect was checked and withdrawn. Focused and neighbouring suites were run at the head and at the merge base in separate worktrees; the one failure was specifically investigated because the PR touches the payload it asserts on, and was found to reproduce at base.

WHY: #690 correctly treats a cross-profile activation as a session boundary, and revoking authority to act under a contaminated role is the right response. Terminal-mutation history is a different kind of state: it records what Gitea already accepted, and no later role switch can make that untrue. Clearing it alongside the authority fields turns the documented remediation path into a way to obtain a second verdict on the same PR at the same head, so the change weakens an existing guarantee while strengthening the one it targets.

ISSUE: #690

HEAD_SHA: 2b3f5baaeb

REVIEW_STATUS: REQUEST_CHANGES recorded at 2b3f5baaeb by sysadmin

MERGE_READY: no

BLOCKERS: code blocker

VALIDATION: Reviewed in /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr924-2b3f5ba-s2, created fresh this session, detached at 2b3f5baaeb, verified clean by git status --porcelain --untracked-files=all before and after. Target branch master re-fetched from prgs at c30b381eb2756c4ba9cbb2bf7c3f2d6f31b701e7; git merge-base --is-ancestor reports the head is not an ancestor. Diff against merge base 2b4e43042a is 6 files, +506/-1. B1 was established by reading _save_review_decision_lock, which on a None argument clears durable state and sets the module lock to None without field selection, and stale_review_decision_lock.prior_live_mutations_block_boundary, which returns False when the lock is falsy; its consumers were located at gitea_mcp_server.py:6294 and :7274. It was then reproduced by invoking the real predicate with a lock carrying one live_mutations entry for PR 999 at the reviewed head: blocked True before the switch, blocked False after, with cross-PR isolation unaffected. The new test module's seed helper was read and confirmed to set no live_mutations, so the claim that test_clean_repreflight_after_switch_allows_gate encodes the defect was checked and withdrawn; the accurate finding is absent coverage of a consumed terminal mutation. Focused suite at head: 7 passed. Neighbouring sweep at head: 1 failed, 483 passed, 5038 deselected, 81 subtests. Same sweep at the merge base in branches/baseline-pr924-2b4e430-s2, run in a command block carrying its own cd, pwd and git rev-parse HEAD: 1 failed, 476 passed, 5038 deselected, 81 subtests. The failing id sets were compared with comm in both directions and found identical; the single failure test_whoami_v2_metadata was investigated specifically because this PR adds a namespace_provenance key to the gitea_whoami payload it asserts on, and reproduces unchanged at base. No full-suite run was performed at this head and none is claimed. No file was edited in either worktree. Pushes during validation: none.

LAST_UPDATED_BY: sysadmin / prgs-reviewer / gitea-reviewer namespace, reviewer lease session 18879-9e0ffc453847

## REQUEST_CHANGES — PR #924 at head `2b3f5baaeb7d4b87a4f0158005fe667289f8161d` Independent review by `sysadmin` / `prgs-reviewer`; author `jcwalker3`, so independence holds. Validated in a fresh session-owned worktree `branches/review-pr924-2b3f5ba-s2`, detached at exactly this head, clean before and after. Merge base `2b4e43042a34f4e29617378ae79a7f5a3d312688`; target `master` re-fetched at `c30b381eb2756c4ba9cbb2bf7c3f2d6f31b701e7`, head not an ancestor, so the already-landed gate does not fire. The premise is right and most of the execution is good. A cross-profile activation genuinely is a session boundary, and clearing workflow-load proof, the reviewer lease, preflight stamps and namespace health under **both** the before and after identities is the correct treatment — the before/after loop closes the reviewer → author → reviewer resurrection path properly. AC4's namespace provenance is a clean addition: it is reported rather than trusted, `unknown` when undeclared, and the resolver's `not ns_provenance.get("mismatch")` conjunct fails closed on a declared mismatch. One blocker, and it is in the same statement list. ### B1 — BLOCKER: clearing the decision lock discards terminal-mutation history, so a profile switch re-admits a second verdict on the same PR at the same head `_invalidate_review_state_on_profile_switch` calls `_save_review_decision_lock(None)`. That writer does not clear selectively — on `None` it clears the durable record and sets `_REVIEW_DECISION_LOCK = None` outright, discarding `live_mutations` and `last_terminal` along with the authority fields. `live_mutations` is the sole input to the duplicate-verdict refusal. `stale_review_decision_lock.prior_live_mutations_block_boundary` opens with: ```python if not lock: return False ``` so a cleared lock disables the guard rather than failing closed. Both consumers depend on it — `gitea_mcp_server.py:6294` and `:7274`. Reproduced with the real predicate, a lock shaped as it exists immediately after a reviewer records a terminal verdict on PR 999 at this head: ```text BEFORE profile switch -> second verdict blocked: True AFTER profile switch -> second verdict blocked: False cross-PR isolation still intact on an unswitched lock (PR 1000): False ``` So the sequence is: post `REQUEST_CHANGES` on PR X at head H → activate another profile and return → run the full reviewer preflight this PR documents as the remedy → `mark_final` on PR X at head H succeeds. #332's one-terminal-mutation rule and #620/#693's per-(PR, head) refinement are both defeated, and the PR's own prescribed recovery path is the thing that walks through the hole. The repository already states the principle being violated, at the writer this change calls — `gitea_mcp_server.py`, `_save_review_decision_lock`: *"#720: durable decision locks are recovery-critical terminal provenance, not generic TTL session cache."* Provenance of what already happened is not contaminated by a role switch; a later profile cannot un-post a review that Gitea has already recorded. **Fix direction: separate authority state from history state.** A role switch revokes *may I act* — workflow-load proof, lease binding, preflight stamps, `final_review_decision_ready` — and it should. It must not revoke *what already happened* — `live_mutations`, `last_terminal`, `locked_head_sha`. Clear the former and preserve the latter, rather than nulling the whole record. **Test-coverage note, stated precisely.** `_seed_contaminated_review_state` seeds `{"session_profile", "final_review_decision_ready", "ready_pr_number": 688}` with **no `live_mutations`** — so `test_clean_repreflight_after_switch_allows_gate` exercises a *prepared but unconsumed* verdict, and asserting the gate reopens there is legitimate. I am not claiming that test encodes the defect. The gap is that no test seeds a **consumed** terminal mutation across a switch, which is why B1 is invisible to a suite that passes 7/7. The test at line 119 asserting `"review_decision_lock"` appears in `invalidated` will keep passing under the corrected behaviour, since the lock's authority fields would still be invalidated. ### Non-blocking observations * The durable cleanup loop wraps `clear_state` in `except Exception: pass`. In-memory state is already reset by then, so the failure mode is a surviving durable record whose in-memory counterpart is gone — the opposite skew from B1, and a state the `invalidated` list would nonetheless report as cleared. A recorded reason would make that visible. * `_PROFILE_SWITCH_INVALIDATION` is a module global that is never reset except by the test conftest, so `gitea_get_runtime_context` reports the most recent switch for the remainder of the process even after a full clean re-preflight. As diagnostics that is defensible, but nothing distinguishes "switch happened and was remediated" from "switch happened", and `re_preflight_required: True` stays pinned either way. * `tests/conftest.py` gains `GITEA_MCP_NAMESPACE` to the unset list and resets the new global. Both are correct and prevent cross-test leakage of the new state. ### Validation ```text worktree : branches/review-pr924-2b3f5ba-s2 (detached at 2b3f5ba, clean before and after) baseline : branches/baseline-pr924-2b4e430-s2 (detached at 2b4e4304, merge base) head : venv/bin/python -m pytest tests/test_issue_690_profile_switch_review_guard.py -q -> 7 passed head : venv/bin/python -m pytest tests/ -q --tb=no -k "profile or decision_lock or namespace or review_workflow or 690 or 693 or 620" -> 1 failed, 483 passed, 5038 deselected, 81 subtests base : same command, in the baseline worktree -> 1 failed, 476 passed, 5038 deselected, 81 subtests failing test id sets: comm -23 and comm -13 both empty -> IDENTICAL ``` The single failure is `tests/test_mcp_server.py::TestRuntimeProfile::test_whoami_v2_metadata`. Because this PR adds a `namespace_provenance` key to the `gitea_whoami` payload, that failure looked like a likely regression and was checked rather than assumed: it reproduces at the merge base with the identical test id, so it is pre-existing. The `+7` passes over base are exactly this PR's new tests. Official validation status: `baseline-equivalent failure accepted`. No other validation failure was observed in this session. No full-suite run was performed at this head and none is claimed. No file in either worktree was edited; the B1 reproduction invoked the predicate in-process and wrote nothing. ## Canonical PR State STATE: PR #924 is open at head 2b3f5baaeb7d4b87a4f0158005fe667289f8161d and now carries a formal REQUEST_CHANGES verdict from sysadmin recorded at that exact head. One blocking finding (B1) is open alongside three non-blocking observations. The change introduces no test regression against merge base 2b4e43042a34f4e29617378ae79a7f5a3d312688. WHO_IS_NEXT: author NEXT_ACTION: Author jcwalker3 must preserve terminal-mutation history across a profile switch while continuing to invalidate authority state, add a regression test that seeds a consumed terminal mutation across a switch, push, and publish a new head-pinned handoff for a fresh independent review. NEXT_PROMPT: ```text Address the REQUEST_CHANGES review on PR #924 (Closes #690) in Scaled-Tech-Consulting/Gitea-Tools on remote prgs. Invoke the canonical gitea-workflow skill first. Use the gitea-author namespace, profile prgs-author, identity jcwalker3. Reviewed head was 2b3f5baaeb7d4b87a4f0158005fe667289f8161d; merge base 2b4e43042a34f4e29617378ae79a7f5a3d312688. B1 (blocker) — _invalidate_review_state_on_profile_switch calls _save_review_decision_lock(None), which clears the whole record including live_mutations and last_terminal. stale_review_decision_lock .prior_live_mutations_block_boundary returns False on a falsy lock, so the duplicate-verdict guard is disabled rather than failing closed; its two consumers are gitea_mcp_server.py:6294 and :7274. Reproduced with the real predicate: a lock holding one terminal mutation for PR 999 at the reviewed head blocks a second verdict (True), and the same query after the switch clears it does not (False). The result is that posting REQUEST_CHANGES on a PR, switching profile and back, then running the full re-preflight this PR prescribes, admits a second terminal verdict on the same PR at the same head, defeating #332 and #620/#693. Fix: separate authority state from history state. Keep invalidating workflow-load proof, reviewer lease binding, preflight identity/capability stamps and final_review_decision_ready. Preserve live_mutations, last_terminal and locked_head_sha across the switch instead of nulling the record. Note the comment already on _save_review_decision_lock: "#720: durable decision locks are recovery-critical terminal provenance, not generic TTL session cache." Test gap: _seed_contaminated_review_state seeds no live_mutations, so no test covers a consumed terminal mutation across a switch. Add one that seeds a terminal mutation for a PR at a head, performs reviewer -> author -> reviewer, completes a clean re-preflight, and asserts mark_final on that same PR and head is still refused. The existing assertion that "review_decision_lock" appears in the invalidated list should continue to pass under the corrected behaviour. Not in scope for the fix: the before/after identity loop that clears durable state under both profile identities is correct and closes the switch-back resurrection path; the AC4 namespace provenance work is sound and fails closed on a declared mismatch. Do not rework either. The single failing test test_whoami_v2_metadata reproduces on the merge base and is not caused by this PR. Do not self-review and do not self-merge. ``` WHAT_HAPPENED: An independent review at the exact head read all six changed files, traced the invalidation statement list to the decision-lock writer and then to the duplicate-verdict predicate and its two consumers, and reproduced the resulting behaviour change by invoking the real predicate against a lock shaped as it exists after a terminal verdict. The seed helper used by the new tests was read to establish precisely what the suite does and does not cover, and the claim that a specific test encodes the defect was checked and withdrawn. Focused and neighbouring suites were run at the head and at the merge base in separate worktrees; the one failure was specifically investigated because the PR touches the payload it asserts on, and was found to reproduce at base. WHY: #690 correctly treats a cross-profile activation as a session boundary, and revoking authority to act under a contaminated role is the right response. Terminal-mutation history is a different kind of state: it records what Gitea already accepted, and no later role switch can make that untrue. Clearing it alongside the authority fields turns the documented remediation path into a way to obtain a second verdict on the same PR at the same head, so the change weakens an existing guarantee while strengthening the one it targets. ISSUE: #690 HEAD_SHA: 2b3f5baaeb7d4b87a4f0158005fe667289f8161d REVIEW_STATUS: REQUEST_CHANGES recorded at 2b3f5baaeb7d4b87a4f0158005fe667289f8161d by sysadmin MERGE_READY: no BLOCKERS: code blocker VALIDATION: Reviewed in /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr924-2b3f5ba-s2, created fresh this session, detached at 2b3f5baaeb7d4b87a4f0158005fe667289f8161d, verified clean by git status --porcelain --untracked-files=all before and after. Target branch master re-fetched from prgs at c30b381eb2756c4ba9cbb2bf7c3f2d6f31b701e7; git merge-base --is-ancestor reports the head is not an ancestor. Diff against merge base 2b4e43042a34f4e29617378ae79a7f5a3d312688 is 6 files, +506/-1. B1 was established by reading _save_review_decision_lock, which on a None argument clears durable state and sets the module lock to None without field selection, and stale_review_decision_lock.prior_live_mutations_block_boundary, which returns False when the lock is falsy; its consumers were located at gitea_mcp_server.py:6294 and :7274. It was then reproduced by invoking the real predicate with a lock carrying one live_mutations entry for PR 999 at the reviewed head: blocked True before the switch, blocked False after, with cross-PR isolation unaffected. The new test module's seed helper was read and confirmed to set no live_mutations, so the claim that test_clean_repreflight_after_switch_allows_gate encodes the defect was checked and withdrawn; the accurate finding is absent coverage of a consumed terminal mutation. Focused suite at head: 7 passed. Neighbouring sweep at head: 1 failed, 483 passed, 5038 deselected, 81 subtests. Same sweep at the merge base in branches/baseline-pr924-2b4e430-s2, run in a command block carrying its own cd, pwd and git rev-parse HEAD: 1 failed, 476 passed, 5038 deselected, 81 subtests. The failing id sets were compared with comm in both directions and found identical; the single failure test_whoami_v2_metadata was investigated specifically because this PR adds a namespace_provenance key to the gitea_whoami payload it asserts on, and reproduces unchanged at base. No full-suite run was performed at this head and none is claimed. No file was edited in either worktree. Pushes during validation: none. LAST_UPDATED_BY: sysadmin / prgs-reviewer / gitea-reviewer namespace, reviewer lease session 18879-9e0ffc453847

sysadmin commented

2026-07-27 15:07:22 -05:00

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #924
issue: #690
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 18879-9e0ffc453847
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr924-2b3f5ba-s2
phase: released
candidate_head: 2b3f5baaeb
target_branch: master
target_branch_sha: c30b381eb2
last_activity: 2026-07-27T20:07:22Z
expires_at: 2026-07-27T20:17:22Z
blocker: manual-release

repo: Scaled-Tech-Consulting/Gitea-Tools pr: #924 issue: #690 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 18879-9e0ffc453847 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr924-2b3f5ba-s2 phase: released candidate_head: 2b3f5baaeb7d4b87a4f0158005fe667289f8161d target_branch: master target_branch_sha: c30b381eb2756c4ba9cbb2bf7c3f2d6f31b701e7 last_activity: 2026-07-27T20:07:22Z expires_at: 2026-07-27T20:17:22Z blocker: manual-release

sysadmin referenced this pull request

2026-07-27 19:43:12 -05:00

fix(author bootstrap): restore missing runtime identity and session helpers (Closes #943) #944

sysadmin referenced this pull request

2026-07-27 19:46:58 -05:00

fix(author bootstrap): restore missing runtime identity and session helpers #943

jcwalker3 added 1 commit 2026-07-28 08:38:10 -05:00

Merge branch 'master' into fix/issue-690-review-profile-switch-guard a633db6186

This pull request has changes conflicting with the target branch.