Test isolation: intra-file global-state leak in review-mutation ledger fails test_unknown_profile_blocks on master #590

Closed
opened 2026-07-09 12:49:05 -05:00 by jcwalker3 · 7 comments
Owner

Problem

tests/test_mcp_server.py::TestMergePR::test_unknown_profile_blocks fails on unmodified master (currently 27d75f), not only on feature branches. It reproduces when that test file is run alone, which points to an intra-file global-state leak in the review-mutation ledger rather than a cross-file ordering artifact.

Evidence

Surfaced during validation of PR #586 (feat/issue-582-issue-content-preflight). Per that PR's validation note:

tests/test_mcp_server.py::TestMergePR::test_unknown_profile_blocks fails on this branch AND on unmodified master (reproduces with that file alone) — a pre-existing intra-file global-state leak in the review-mutation ledger, untouched by this change. Diagnosed for separate tracking; not introduced here.

The review-mutation budget/ledger carries state across tests within the file; an earlier test consumes or mutates it, so test_unknown_profile_blocks observes a dirtied ledger and asserts against the wrong state.

Repro

python -m pytest tests/test_mcp_server.py::TestMergePR::test_unknown_profile_blocks

Fails on a clean checkout of master with no local modifications.

Required behavior

  • Each test in tests/test_mcp_server.py starts from a clean review-mutation ledger.
  • Reset/isolate the ledger global state via fixture teardown (or per-test construction) so test_unknown_profile_blocks passes deterministically regardless of run order or sibling tests.
  • Add a guard/regression test proving the ledger is reset between tests.

Acceptance criteria

  • test_unknown_profile_blocks passes in isolation AND as part of the full test_mcp_server.py run, on clean master.
  • No shared mutable review-mutation ledger state persists across tests in the file.
  • The full suite remains green after the isolation fix.

Source pointers

  • PR #586 (validation note diagnosing this leak, not introduced there)
  • Issue #582 (the content-gate work whose validation surfaced it)
  • Ledger runtime behavior context: #332 (closed — hard-stop after terminal review mutation)
## Problem `tests/test_mcp_server.py::TestMergePR::test_unknown_profile_blocks` fails on **unmodified master** (currently 27d75f), not only on feature branches. It reproduces when that test file is run alone, which points to an intra-file global-state leak in the review-mutation ledger rather than a cross-file ordering artifact. ## Evidence Surfaced during validation of PR #586 (`feat/issue-582-issue-content-preflight`). Per that PR's validation note: > `tests/test_mcp_server.py::TestMergePR::test_unknown_profile_blocks` fails on this branch AND on unmodified master (reproduces with that file alone) — a pre-existing intra-file global-state leak in the review-mutation ledger, untouched by this change. Diagnosed for separate tracking; not introduced here. The review-mutation budget/ledger carries state across tests within the file; an earlier test consumes or mutates it, so `test_unknown_profile_blocks` observes a dirtied ledger and asserts against the wrong state. ## Repro ``` python -m pytest tests/test_mcp_server.py::TestMergePR::test_unknown_profile_blocks ``` Fails on a clean checkout of master with no local modifications. ## Required behavior - Each test in `tests/test_mcp_server.py` starts from a clean review-mutation ledger. - Reset/isolate the ledger global state via fixture teardown (or per-test construction) so `test_unknown_profile_blocks` passes deterministically regardless of run order or sibling tests. - Add a guard/regression test proving the ledger is reset between tests. ## Acceptance criteria - `test_unknown_profile_blocks` passes in isolation AND as part of the full `test_mcp_server.py` run, on clean master. - No shared mutable review-mutation ledger state persists across tests in the file. - The full suite remains green after the isolation fix. ## Source pointers - PR #586 (validation note diagnosing this leak, not introduced there) - Issue #582 (the content-gate work whose validation surfaced it) - Ledger runtime behavior context: #332 (closed — hard-stop after terminal review mutation)
jcwalker3 added the bugmcptesting labels 2026-07-09 12:51:46 -05:00
jcwalker3 added the status:in-progress label 2026-07-09 13:17:22 -05:00
Author
Owner

Issue claim heartbeat

<!-- gitea-issue-claim-heartbeat:v1 --> **Issue claim heartbeat** - kind: claim - issue: #590 - branch: fix/issue-590-ledger-isolation - phase: claimed - profile: prgs-author - pr: none - blocker: none - next_action: create worktree and begin implementation
Author
Owner

Claiming #590 as author (jcwalker3).

Branch: fix/issue-590-ledger-isolation @ 0d8b000
Worktree: branches/fix-issue-590-ledger-isolation

Root cause: patch.dict(os.environ, clear=True) dropped GITEA_MCP_SESSION_STATE_DIR, so tests re-adopted the host ~/.cache/gitea-tools/session-state review-mutation ledger and hit the #332 hard-stop before the empty-profile gate.

Fix: pin default_state_dir/DEFAULT_STATE_DIR to a per-test temp dir in conftest (still honors explicit env overrides); clear decision lock in TestMergePR.setUp; add regression tests.

Claiming #590 as author (jcwalker3). **Branch:** `fix/issue-590-ledger-isolation` @ `0d8b000` **Worktree:** `branches/fix-issue-590-ledger-isolation` **Root cause:** `patch.dict(os.environ, clear=True)` dropped `GITEA_MCP_SESSION_STATE_DIR`, so tests re-adopted the host `~/.cache/gitea-tools/session-state` review-mutation ledger and hit the #332 hard-stop before the empty-profile gate. **Fix:** pin `default_state_dir`/`DEFAULT_STATE_DIR` to a per-test temp dir in conftest (still honors explicit env overrides); clear decision lock in `TestMergePR.setUp`; add regression tests.
Author
Owner

Lock adopted for #590 on fix/issue-590-ledger-isolation (@ 0d8b000). Opening PR.

Lock adopted for #590 on `fix/issue-590-ledger-isolation` (@ `0d8b000`). Opening PR.
jcwalker3 added status:pr-open and removed status:in-progress labels 2026-07-09 13:22:08 -05:00
Owner

Canonical Issue State

STATE:
blocked

WHO_IS_NEXT:
user

NEXT_ACTION:
Unblock formal review of PR #592 by clearing the stale prgs-reviewer #332 durable terminal ledger (last mutation: approve on already-merged #586), then re-run formal APPROVE.

NEXT_PROMPT:

See PR #592 comment 8458 for full diagnosis and operator unblock steps. Technical review of fix/issue-590-ledger-isolation @ b810e512fd9f50b7c0106305914ad79553faee12 is green; formal APPROVE blocked by durable #332 ledger, not by PR #592 code findings.

WHAT_HAPPENED:
Reviewer completed technical validation of PR #592 for this issue. Formal review submission blocked by #332 hard-stop from durable prgs-reviewer session-state after prior approve of merged PR #586. Findings recorded on PR #592 comment 8458.

WHY:
Durable review-decision lock retained terminal mutations across sessions after merge; #590 test isolation PR is otherwise merge-ready from code review perspective but cannot receive a formal verdict until the operator ledger is reset.

RELATED_DISCUSSION:
PR #592 comment 8458

RELATED_PRS:

  • #592 (open fix for this issue)
  • #586 (merged; source of last terminal approve in durable ledger)

BRANCH:
fix/issue-590-ledger-isolation

HEAD_SHA:
b810e512fd

VALIDATION:
See PR #592 comment 8458 (207 + 32 tests green; dry-run would_perform; mark_ready blocked by #332).

BLOCKERS:
Stale durable #332 ledger for prgs-reviewer. Unblock: supported clear of review_decision_lock-prgs-reviewer, then formal re-review of #592.

LAST_UPDATED_BY:
sysadmin / prgs-reviewer / 2026-07-09

## Canonical Issue State STATE: blocked WHO_IS_NEXT: user NEXT_ACTION: Unblock formal review of PR #592 by clearing the stale prgs-reviewer #332 durable terminal ledger (last mutation: approve on already-merged #586), then re-run formal APPROVE. NEXT_PROMPT: ```text See PR #592 comment 8458 for full diagnosis and operator unblock steps. Technical review of fix/issue-590-ledger-isolation @ b810e512fd9f50b7c0106305914ad79553faee12 is green; formal APPROVE blocked by durable #332 ledger, not by PR #592 code findings. ``` WHAT_HAPPENED: Reviewer completed technical validation of PR #592 for this issue. Formal review submission blocked by #332 hard-stop from durable prgs-reviewer session-state after prior approve of merged PR #586. Findings recorded on PR #592 comment 8458. WHY: Durable review-decision lock retained terminal mutations across sessions after merge; #590 test isolation PR is otherwise merge-ready from code review perspective but cannot receive a formal verdict until the operator ledger is reset. RELATED_DISCUSSION: PR #592 comment 8458 RELATED_PRS: - #592 (open fix for this issue) - #586 (merged; source of last terminal approve in durable ledger) BRANCH: fix/issue-590-ledger-isolation HEAD_SHA: b810e512fd9f50b7c0106305914ad79553faee12 VALIDATION: See PR #592 comment 8458 (207 + 32 tests green; dry-run would_perform; mark_ready blocked by #332). BLOCKERS: Stale durable #332 ledger for prgs-reviewer. Unblock: supported clear of review_decision_lock-prgs-reviewer, then formal re-review of #592. LAST_UPDATED_BY: sysadmin / prgs-reviewer / 2026-07-09
Owner

Canonical Issue State

STATE: BLOCKED
WHO_IS_NEXT: user
NEXT_ACTION: Clear or restart prgs-reviewer durable review-decision lock, then submit formal APPROVE on PR #592 @ b810e512fd.
NEXT_PROMPT:

After clearing durable review_decision_lock for prgs-reviewer, formally approve Scaled-Tech-Consulting/Gitea-Tools PR #592 (Issue #590) at head b810e512fd9f50b7c0106305914ad79553faee12 via canonical MCP reviewer tools. Technical validation already green; formal submit was blocked only by #332 durable terminal mutation from PR #586.

WHAT_HAPPENED: Reviewer re-validated PR #592 against Issue #590 acceptance criteria. Isolation fix is correct and fully tested. Formal Gitea review verdict could not be submitted because durable #332 hard-stop still holds prior approve on PR #586 in host session-state for prgs-reviewer. Full technical write-up is on PR #592 comment_id 8472.
WHY: Not a regression in the #590 fix. Operator session tooling gate (#332 + durable #559 lock) blocks multi-PR terminal review mutations until lock is cleared/restarted.
RELATED_PRS: #592
BLOCKERS: UNBLOCK when review_decision_lock-prgs-reviewer.json no longer hard-stops mark_final_review_decision for a PR other than the last terminal-approved PR (currently last terminal is approve on PR #586).
VALIDATION:

  • test_mcp_server.py: 207 passed
  • isolation + hard-stop suites: 33 passed
  • git diff --check clean
  • production code unchanged
    LAST_UPDATED_BY: sysadmin (prgs-reviewer)
## Canonical Issue State STATE: BLOCKED WHO_IS_NEXT: user NEXT_ACTION: Clear or restart prgs-reviewer durable review-decision lock, then submit formal APPROVE on PR #592 @ b810e512fd9f50b7c0106305914ad79553faee12. NEXT_PROMPT: ```text After clearing durable review_decision_lock for prgs-reviewer, formally approve Scaled-Tech-Consulting/Gitea-Tools PR #592 (Issue #590) at head b810e512fd9f50b7c0106305914ad79553faee12 via canonical MCP reviewer tools. Technical validation already green; formal submit was blocked only by #332 durable terminal mutation from PR #586. ``` WHAT_HAPPENED: Reviewer re-validated PR #592 against Issue #590 acceptance criteria. Isolation fix is correct and fully tested. Formal Gitea review verdict could not be submitted because durable #332 hard-stop still holds prior approve on PR #586 in host session-state for prgs-reviewer. Full technical write-up is on PR #592 comment_id 8472. WHY: Not a regression in the #590 fix. Operator session tooling gate (#332 + durable #559 lock) blocks multi-PR terminal review mutations until lock is cleared/restarted. RELATED_PRS: #592 BLOCKERS: UNBLOCK when review_decision_lock-prgs-reviewer.json no longer hard-stops mark_final_review_decision for a PR other than the last terminal-approved PR (currently last terminal is approve on PR #586). VALIDATION: - test_mcp_server.py: 207 passed - isolation + hard-stop suites: 33 passed - git diff --check clean - production code unchanged LAST_UPDATED_BY: sysadmin (prgs-reviewer)
Owner

Canonical Issue State

STATE: BLOCKED
WHO_IS_NEXT: user
NEXT_ACTION: Fast-forward operator root Gitea-Tools to prgs/master (ac531dda05 includes PR #595), fully restart gitea-reviewer MCP, confirm gitea_cleanup_stale_review_decision_lock is available, then formal APPROVE PR #592.
NEXT_PROMPT:

After root is on master with PR #595 and gitea-reviewer MCP is restarted with gitea_cleanup_stale_review_decision_lock available, formal APPROVE PR #592 at b810e512fd9f50b7c0106305914ad79553faee12 for Issue #590. Full blocker detail: PR #592 comment 8608.

WHAT_HAPPENED: Formal APPROVE of PR #592 blocked at MCP runtime prerequisite — tool from #595 not loaded because root checkout still pre-#595.
WHY: MCP entrypoint uses root source; restart without advancing root cannot load cleanup tool.
RELATED_PRS: #592, #595
BLOCKERS: UNBLOCK when MCP source HEAD includes #595 and cleanup tool is listed in gitea-reviewer tools.
VALIDATION: whoami ok; PR head still b810e512; formal review not submitted
LAST_UPDATED_BY: sysadmin (prgs-reviewer)

## Canonical Issue State STATE: BLOCKED WHO_IS_NEXT: user NEXT_ACTION: Fast-forward operator root Gitea-Tools to prgs/master (ac531dda05858d17360aab17744e1a489e7d0aba includes PR #595), fully restart gitea-reviewer MCP, confirm gitea_cleanup_stale_review_decision_lock is available, then formal APPROVE PR #592. NEXT_PROMPT: ```text After root is on master with PR #595 and gitea-reviewer MCP is restarted with gitea_cleanup_stale_review_decision_lock available, formal APPROVE PR #592 at b810e512fd9f50b7c0106305914ad79553faee12 for Issue #590. Full blocker detail: PR #592 comment 8608. ``` WHAT_HAPPENED: Formal APPROVE of PR #592 blocked at MCP runtime prerequisite — tool from #595 not loaded because root checkout still pre-#595. WHY: MCP entrypoint uses root source; restart without advancing root cannot load cleanup tool. RELATED_PRS: #592, #595 BLOCKERS: UNBLOCK when MCP source HEAD includes #595 and cleanup tool is listed in gitea-reviewer tools. VALIDATION: whoami ok; PR head still b810e512; formal review not submitted LAST_UPDATED_BY: sysadmin (prgs-reviewer)
Owner

Canonical Issue State

STATE: open — PR #592 formal APPROVE retry stalled on MCP infra, not on the #590 isolation fix quality
WHO_IS_NEXT: user
NEXT_ACTION: After root is on prgs/master with #595 and gitea-reviewer is restarted, complete formal APPROVE of PR #592 at head b810e512fd (run cleanup for stale #332 lock if still present)
NEXT_PROMPT:

Retry formal review of PR #592 / Issue #590 after gitea-reviewer runs master with #595. Head b810e512fd9f50b7c0106305914ad79553faee12. APPROVE. Use gitea_cleanup_stale_review_decision_lock if #332 lock for PR #586 remains. Do not merge.

WHAT_HAPPENED: Formal APPROVE path stopped because live reviewer MCP lacks post-#595 cleanup tool; root HEAD 6913ac9 behind prgs/master ac531dd. Full diagnosis on PR #592 comment 8619 and #591 comment 8631.
WHY: Host MCP runtime staleness (#591), not a regression in the #590 isolation fix itself. Prior technical validation for #592 remains the prepared basis for APPROVE once infra is current.
RELATED_PRS: #592, #595, #591/#593
BLOCKERS: stale gitea-reviewer runtime and root HEAD lag; unblock when gitea_cleanup_stale_review_decision_lock is available on live gitea-reviewer and formal APPROVE of #592 is submitted (mergeable may also need recheck)
VALIDATION: live PR open at expected head; whoami sysadmin/prgs-reviewer; cleanup tool missing from live catalog
LAST_UPDATED_BY: prgs-reviewer (sysadmin) 2026-07-09

## Canonical Issue State STATE: open — PR #592 formal APPROVE retry stalled on MCP infra, not on the #590 isolation fix quality WHO_IS_NEXT: user NEXT_ACTION: After root is on prgs/master with #595 and gitea-reviewer is restarted, complete formal APPROVE of PR #592 at head b810e512fd9f50b7c0106305914ad79553faee12 (run cleanup for stale #332 lock if still present) NEXT_PROMPT: ```text Retry formal review of PR #592 / Issue #590 after gitea-reviewer runs master with #595. Head b810e512fd9f50b7c0106305914ad79553faee12. APPROVE. Use gitea_cleanup_stale_review_decision_lock if #332 lock for PR #586 remains. Do not merge. ``` WHAT_HAPPENED: Formal APPROVE path stopped because live reviewer MCP lacks post-#595 cleanup tool; root HEAD 6913ac9 behind prgs/master ac531dd. Full diagnosis on PR #592 comment 8619 and #591 comment 8631. WHY: Host MCP runtime staleness (#591), not a regression in the #590 isolation fix itself. Prior technical validation for #592 remains the prepared basis for APPROVE once infra is current. RELATED_PRS: #592, #595, #591/#593 BLOCKERS: stale gitea-reviewer runtime and root HEAD lag; unblock when gitea_cleanup_stale_review_decision_lock is available on live gitea-reviewer and formal APPROVE of #592 is submitted (mergeable may also need recheck) VALIDATION: live PR open at expected head; whoami sysadmin/prgs-reviewer; cleanup tool missing from live catalog LAST_UPDATED_BY: prgs-reviewer (sysadmin) 2026-07-09
Sign in to join this conversation.
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Scaled-Tech-Consulting/Gitea-Tools#590