fix(session): pin immutable MCP context; ban cross-host profile substitution (Closes #714) #715

Open
jcwalker3 wants to merge 3 commits from fix/issue-714-session-context-immutability into master
Owner

Summary

Closes #714.

Makes the MCP session profile, remote, host, repository, namespace, and authenticated identity immutable (or fail closed on drift). Capability resolution and mutation gates no longer silently substitute a more privileged or cross-host profile (for example mdcps-reviewer becoming prgs-author).

Root cause

_ensure_matching_profile and _try_auto_switch_for_operation auto-activated any configured profile that held the required permission, without regard to remote or host. After pinning mdcps-reviewer for dadeschools, resolving comment_issue switched the process to prgs-author.

Changes

  • New session_context_binding.py: bind, seed, and assess session context; filter profiles by remote host; identity expected-versus-actual checks.
  • Remove silent auto-switch; evaluate only the active profile for the requested remote.
  • Explicit gitea_activate_profile re-binds the session context.
  • whoami, runtime context, and capability resolver report the same context proof.
  • Mutation gates and audit records include the bound session context; cross-host, identity mismatch, and drift fail closed before writes.

Tests

  • New tests/test_issue_714_session_context_immutability.py reproduces the pin then drift then resolver sequence and remaining acceptance criteria.
  • Updated tests/test_operation_scoped_roles.py for fail-closed (no auto-switch); explicit activate still works.

Dependent work

  • #711 is the next dependent issue (MDCPS close path, worktree_path, reconciler routing).
  • MDCPS reconciliation remains prohibited until #714 and #711 are both merged and verified.

Verification

  • Targeted pytest: 90 tests passed in the worktree (issue 714, operation-scoped roles, role session router, runtime clarity, permission reports, workspace guards, session state).
  • Control checkout left on clean master (pre-existing untracked only).
  • Registered worktree: branches/fix-issue-714-session-context-immutability.

Do not self-review or merge.

## Summary Closes #714. Makes the MCP session profile, remote, host, repository, namespace, and authenticated identity immutable (or fail closed on drift). Capability resolution and mutation gates no longer silently substitute a more privileged or cross-host profile (for example `mdcps-reviewer` becoming `prgs-author`). ### Root cause `_ensure_matching_profile` and `_try_auto_switch_for_operation` auto-activated any configured profile that held the required permission, without regard to remote or host. After pinning `mdcps-reviewer` for `dadeschools`, resolving `comment_issue` switched the process to `prgs-author`. ### Changes - New `session_context_binding.py`: bind, seed, and assess session context; filter profiles by remote host; identity expected-versus-actual checks. - Remove silent auto-switch; evaluate only the active profile for the requested remote. - Explicit `gitea_activate_profile` re-binds the session context. - `whoami`, runtime context, and capability resolver report the same context proof. - Mutation gates and audit records include the bound session context; cross-host, identity mismatch, and drift fail closed before writes. ### Tests - New `tests/test_issue_714_session_context_immutability.py` reproduces the pin then drift then resolver sequence and remaining acceptance criteria. - Updated `tests/test_operation_scoped_roles.py` for fail-closed (no auto-switch); explicit activate still works. ### Dependent work - **#711** is the next dependent issue (MDCPS close path, worktree_path, reconciler routing). - **MDCPS reconciliation remains prohibited until #714 and #711 are both merged and verified.** ### Verification - Targeted pytest: 90 tests passed in the worktree (issue 714, operation-scoped roles, role session router, runtime clarity, permission reports, workspace guards, session state). - Control checkout left on clean `master` (pre-existing untracked only). - Registered worktree: `branches/fix-issue-714-session-context-immutability`. Do not self-review or merge.
jcwalker3 added 1 commit 2026-07-14 22:06:22 -05:00
Capability resolution and mutation gates no longer auto-switch profiles.
Session profile/remote/host/identity are bound after pin or first seed,
and drift or cross-host resolution fails closed before writes.

Co-Authored-By: Grok <[email protected]>
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #715
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 54162-69761428cb3b
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-715-drift-fix
phase: claimed
candidate_head: none
target_branch: master
target_branch_sha: none
last_activity: 2026-07-15T06:01:30Z
expires_at: 2026-07-15T08:01:30Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #715 issue: none reviewer_identity: sysadmin profile: prgs-reviewer session_id: 54162-69761428cb3b worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-715-drift-fix phase: claimed candidate_head: none target_branch: master target_branch_sha: none last_activity: 2026-07-15T06:01:30Z expires_at: 2026-07-15T08:01:30Z blocker: none
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #715
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 54162-69761428cb3b
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-715-drift-fix
phase: released
candidate_head: none
target_branch: master
target_branch_sha: none
last_activity: 2026-07-15T06:13:58Z
expires_at: 2026-07-15T08:13:58Z
blocker: manual-release

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #715 issue: none reviewer_identity: sysadmin profile: prgs-reviewer session_id: 54162-69761428cb3b worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-715-drift-fix phase: released candidate_head: none target_branch: master target_branch_sha: none last_activity: 2026-07-15T06:13:58Z expires_at: 2026-07-15T08:13:58Z blocker: manual-release
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #715
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 54162-5bf29bfb7884
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-715-drift-fix
phase: claimed
candidate_head: none
target_branch: master
target_branch_sha: none
last_activity: 2026-07-15T06:14:11Z
expires_at: 2026-07-15T08:14:11Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #715 issue: none reviewer_identity: sysadmin profile: prgs-reviewer session_id: 54162-5bf29bfb7884 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-715-drift-fix phase: claimed candidate_head: none target_branch: master target_branch_sha: none last_activity: 2026-07-15T06:14:11Z expires_at: 2026-07-15T08:14:11Z blocker: none
sysadmin requested changes 2026-07-15 01:14:36 -05:00
sysadmin left a comment
Owner

Canonical PR State

STATE: OPEN
WHO_IS_NEXT: author
NEXT_ACTION: Fix test pollution in session context tests
NEXT_PROMPT:

The drift protection logic works correctly in the live server, but the PR introduces significant test pollution. The `_SESSION_CONTEXT` global variable in `session_context_binding.py` bleeds across tests in the same process because `session_ctx.clear_session_context()` is not called in the `setUp` of most test classes. This causes false-positive "drift block" failures when tests change profiles or identities (41 test failures across the suite). Please add `clear_session_context()` to a base test class or all relevant test classes, verify the full test suite passes reliably, and push the fix.

WHAT_HAPPENED: The PR was reviewed and the core drift-protection logic works successfully (failed closed properly in manual cross-host mutation testing). However, the automated test suite fails due to context leakage across test boundaries.
WHY: _SESSION_CONTEXT is not cleared between tests in several classes (e.g., TestResolveTaskCapability, TestReviewerAuthorMutationBlocks).
ISSUE: #714
HEAD_SHA: 632c568865
REVIEW_STATUS: CHANGES_REQUESTED
MERGE_READY: false
BLOCKERS: Test pollution breaking the automated test suite.
VALIDATION: Manual testing of cross-host mutation bypass successfully blocked. pytest tests/ fails with 41 errors.
LAST_UPDATED_BY: sysadmin

Review Details

The implementation correctly prevents cross-host profile drift and enforces session context immutability. The manual testing confirms that mutation attempts with a cross-host or mismatched profile now fail closed rather than silently substituting a different profile (e.g., prgs-reviewer attempting to mutate dadeschools is correctly blocked and does not switch to mdcps-reviewer).

However, the PR introduces significant test pollution that breaks the broader test suite. The session_context_binding.py module uses a global _SESSION_CONTEXT dictionary. While TestOperationScopedRoles correctly calls session_ctx.clear_session_context() in setUp() and tearDown(), this was omitted in several other test classes. As a result, the bound context bleeds across tests in the same process, causing false positive "drift block" failures in tests that change profiles or identities.

This causes 41 test failures across the suite, including in:

  • TestResolveTaskCapability
  • TestReviewerAuthorMutationBlocks
  • TestCreateIssue
  • TestCreatePR
  • etc.

Please add session_ctx.clear_session_context() to the setUp()/tearDown() of a base test class or ensure it is cleared in all relevant test classes so that the test suite passes reliably.

## Canonical PR State STATE: OPEN WHO_IS_NEXT: author NEXT_ACTION: Fix test pollution in session context tests NEXT_PROMPT: ```text The drift protection logic works correctly in the live server, but the PR introduces significant test pollution. The `_SESSION_CONTEXT` global variable in `session_context_binding.py` bleeds across tests in the same process because `session_ctx.clear_session_context()` is not called in the `setUp` of most test classes. This causes false-positive "drift block" failures when tests change profiles or identities (41 test failures across the suite). Please add `clear_session_context()` to a base test class or all relevant test classes, verify the full test suite passes reliably, and push the fix. ``` WHAT_HAPPENED: The PR was reviewed and the core drift-protection logic works successfully (failed closed properly in manual cross-host mutation testing). However, the automated test suite fails due to context leakage across test boundaries. WHY: `_SESSION_CONTEXT` is not cleared between tests in several classes (e.g., `TestResolveTaskCapability`, `TestReviewerAuthorMutationBlocks`). ISSUE: #714 HEAD_SHA: 632c568865cab3de7a8d9970463ec182578d0a86 REVIEW_STATUS: CHANGES_REQUESTED MERGE_READY: false BLOCKERS: Test pollution breaking the automated test suite. VALIDATION: Manual testing of cross-host mutation bypass successfully blocked. `pytest tests/` fails with 41 errors. LAST_UPDATED_BY: sysadmin ## Review Details The implementation correctly prevents cross-host profile drift and enforces session context immutability. The manual testing confirms that mutation attempts with a cross-host or mismatched profile now fail closed rather than silently substituting a different profile (e.g., `prgs-reviewer` attempting to mutate `dadeschools` is correctly blocked and does not switch to `mdcps-reviewer`). However, the PR introduces significant test pollution that breaks the broader test suite. The `session_context_binding.py` module uses a global `_SESSION_CONTEXT` dictionary. While `TestOperationScopedRoles` correctly calls `session_ctx.clear_session_context()` in `setUp()` and `tearDown()`, this was omitted in several other test classes. As a result, the bound context bleeds across tests in the same process, causing false positive "drift block" failures in tests that change profiles or identities. This causes 41 test failures across the suite, including in: - `TestResolveTaskCapability` - `TestReviewerAuthorMutationBlocks` - `TestCreateIssue` - `TestCreatePR` - etc. Please add `session_ctx.clear_session_context()` to the `setUp()`/`tearDown()` of a base test class or ensure it is cleared in all relevant test classes so that the test suite passes reliably.
jcwalker3 added 1 commit 2026-07-15 01:26:52 -05:00
Author
Owner

Canonical Issue State

STATE: OPEN
WHO_IS_NEXT: reviewer
NEXT_ACTION: Independently re-review PR #715 at head 29ffe1407f.
NEXT_PROMPT:

Independently re-review PR #715 at 29ffe1407fa9620b4a75035bb9be3a254e74a674. Verify the immutable session-context lifecycle, the pytest-only before/finally isolation boundary, concurrency safety, the preserved cross-host/repository/identity fail-closed gates, and the full-suite result. Do not rely on the author's conclusions.

WHAT_HAPPENED: The author remediated review #437 on the existing PR branch, committed 29ffe1407f, and pushed normally without force.
WHY: Pytest reused the process-local session binding across independent tests, and the mutable check-then-set dictionary allowed competing first calls to overwrite the established context.
RELATED_PRS: #715
BLOCKERS: Formal review #437 remains REQUEST_CHANGES pending independent re-review; it was not dismissed or rewritten.
VALIDATION: Issue #714 plus operation-scoped roles: 23 passed. Previously failing modules: 333 passed. Isolation tests: 2 passed in forward order and 2 passed in reverse order. Full suite python3 -m pytest tests/: 2710 passed, 6 skipped, 1 warning in 30.83s.
LAST_UPDATED_BY: jcwalker3 / prgs-author

Lifecycle and isolation design

  • The first whoami/capability/runtime/mutation gate atomically binds profile, remote, host, org/repository, identity, role, and PID.
  • Ordinary calls and environment changes cannot rebind it. A new PID re-seeds; explicit gitea_activate_profile is the sanctioned logical-session transition.
  • The shared autouse fixture resets before each pytest item and in a finally block after it, so failures and teardown exceptions cannot pollute later tests. Ad hoc class cleanup was removed.
  • The reset helper is private, requires pytest's per-test marker, is not an MCP tool, and is rejected outside a pytest boundary. Production never clears context between calls.
  • The binding is a frozen dataclass behind an RLock; first bind and sanctioned rebind are atomic, snapshots are detached, and parallel/interleaved seed calls cannot overwrite an established binding.
  • Unsupported permissions and namespace mismatches still fail before writes. Configured identity mismatch, profile drift, cross-host, org, and repository mismatch remain blocked. Legacy env-only profiles may establish only their first remote/host/repository pin.

The only warning is the existing Starlette/httpx deprecation in tests/test_webui_audit.py. The author did not self-review or merge.

## Canonical Issue State STATE: OPEN WHO_IS_NEXT: reviewer NEXT_ACTION: Independently re-review PR #715 at head 29ffe1407fa9620b4a75035bb9be3a254e74a674. NEXT_PROMPT: ```text Independently re-review PR #715 at 29ffe1407fa9620b4a75035bb9be3a254e74a674. Verify the immutable session-context lifecycle, the pytest-only before/finally isolation boundary, concurrency safety, the preserved cross-host/repository/identity fail-closed gates, and the full-suite result. Do not rely on the author's conclusions. ``` WHAT_HAPPENED: The author remediated review #437 on the existing PR branch, committed 29ffe1407fa9620b4a75035bb9be3a254e74a674, and pushed normally without force. WHY: Pytest reused the process-local session binding across independent tests, and the mutable check-then-set dictionary allowed competing first calls to overwrite the established context. RELATED_PRS: #715 BLOCKERS: Formal review #437 remains REQUEST_CHANGES pending independent re-review; it was not dismissed or rewritten. VALIDATION: Issue #714 plus operation-scoped roles: 23 passed. Previously failing modules: 333 passed. Isolation tests: 2 passed in forward order and 2 passed in reverse order. Full suite `python3 -m pytest tests/`: 2710 passed, 6 skipped, 1 warning in 30.83s. LAST_UPDATED_BY: jcwalker3 / prgs-author ### Lifecycle and isolation design - The first whoami/capability/runtime/mutation gate atomically binds profile, remote, host, org/repository, identity, role, and PID. - Ordinary calls and environment changes cannot rebind it. A new PID re-seeds; explicit `gitea_activate_profile` is the sanctioned logical-session transition. - The shared autouse fixture resets before each pytest item and in a `finally` block after it, so failures and teardown exceptions cannot pollute later tests. Ad hoc class cleanup was removed. - The reset helper is private, requires pytest's per-test marker, is not an MCP tool, and is rejected outside a pytest boundary. Production never clears context between calls. - The binding is a frozen dataclass behind an `RLock`; first bind and sanctioned rebind are atomic, snapshots are detached, and parallel/interleaved seed calls cannot overwrite an established binding. - Unsupported permissions and namespace mismatches still fail before writes. Configured identity mismatch, profile drift, cross-host, org, and repository mismatch remain blocked. Legacy env-only profiles may establish only their first remote/host/repository pin. The only warning is the existing Starlette/httpx deprecation in `tests/test_webui_audit.py`. The author did not self-review or merge.
jcwalker3 added 1 commit 2026-07-15 14:52:05 -05:00
The formal REQUEST_CHANGES review on PR #715 was resolved by the test
isolation commit (29ffe14); the full suite is green. Auditing the required
session-lifecycle coverage surfaced three gaps that the suite did not prove:

- Every cross-host test pinned mdcps-reviewer and denied prgs. The reverse
  direction (a pinned MDCPS profile must never serve a prgs request, nor be
  swapped for prgs-author) was unproven.
- test_parallel_calls_cannot_overwrite_established_binding binds first and
  then races, so concurrent *initialization* from an unbound context — N
  threads racing to first-bind, exactly one winner, no torn context — was
  unproven.
- assess_session_context checks repository and org drift, but no test bound a
  repository/org and asserted a same-host mismatch fails closed.

Adds three tests covering those cases. Each was mutation-verified: disabling
the first-bind-wins guard, the cross-host denial, and the repository/org drift
checks makes the corresponding test fail.

Additive only — no production code changed and no existing security assertion
weakened, skipped, or rewritten.

Focused: 20 passed. Full suite: 2713 passed, 6 skipped, 1 pre-existing warning,
161 subtests passed in 23.05s.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Author
Owner

Canonical Issue State

ISSUE: #714
PR: #715
STATE: OPEN
WHO_IS_NEXT: reviewer
NEXT_ACTION: Independent prgs-reviewer re-review of PR #715 at head d936da5c87
NEXT_PROMPT:

REVIEWER task — Gitea-Tools PR #715 (Issue #714), repository Scaled-Tech-Consulting/Gitea-Tools, remote prgs.

Use the native gitea-reviewer namespace with the prgs-reviewer profile. Acquire your own reviewer lease; do not adopt the author session.

Pin and verify head d936da5c872fb4bad31536d80b7c5f57462b05a9 before analysis. Review 437 (REQUEST_CHANGES) was recorded at head 632c5688 and predates two pushes; it does not describe this head.

Scope: Issue #714 acceptance criteria 1-8 (cross-host MCP profile drift and capability substitution fail closed).

Verify specifically:
- Repeated calls after pinning mdcps-reviewer never report prgs-author.
- A dadeschools request cannot resolve through a prgs profile, and a prgs request cannot resolve through an MDCPS profile.
- Unsupported capability returns a structured denial without profile substitution.
- Identity, repository, org, host, and profile mismatch each fail closed before mutation.
- _reset_session_context_for_testing cannot be reached by a production caller.
- Concurrent initialization yields one winning binding; interleaved calls cannot overwrite an established binding.
- Existing static-profile namespaces remain functional.

Run the full suite from a reviewer worktree using the project venv interpreter (bare python3 is aliased to 3.13 and has no pytest):
  venv/bin/python -m pytest tests/ -q -s
Author measured 2713 pass, 6 skip, 1 pre-existing warning, 161 subtests, 23.05s.

Do not merge. Record a formal verdict via the review API only.

WHAT_HAPPENED: Author remediated the test-context contamination cited by review 437 and pushed two commits. 29ffe14 centralized _SESSION_CONTEXT isolation at the test boundary; d936da5 adds three tests closing coverage gaps found while auditing the required session-lifecycle properties. The full suite now reports zero failures where review 437 measured 41.
WHY: session_context_binding._SESSION_CONTEXT is a process-global binding that only TestOperationScopedRoles reset, so one test's binding stayed live for later tests in the same process; tests that legitimately change profile or identity were then measured against a stale predecessor binding and rejected as drift. The drift protection was correct — the test boundary was not re-establishing a clean session.
RELATED_PRS: #715 (this PR, Closes #714); #711 is the dependent follow-on (MDCPS close path, worktree_path, reconciler routing) and remains outstanding; MDCPS reconciliation stays prohibited until #714 and #711 are both landed and verified. PR #710 is out of scope and untouched.
HEAD_SHA: d936da5c87
PRIOR_HEAD_SHA: 29ffe1407f
REVIEWED_HEAD_SHA: 632c568865
REVIEW_STATUS: CHANGES_REQUESTED
MERGE_READY: false
BLOCKERS: none
VALIDATION: Full suite 2713 pass, 6 skip, 1 pre-existing warning, 161 subtests in 23.05s
LAST_UPDATED_BY: jcwalker3

[THREAD STATE LEDGER]

What is true now

Server-side decision state: PR #715 carries one REQUEST_CHANGES review verdict (review_id 437) recorded via the review API by sysadmin at head 632c5688. It is undismissed and stale against the current head; approval_at_current_head is false and no APPROVE verdict exists for any head.
Local verdict/state: Author remediation is complete at head d936da5c872fb4bad31536d80b7c5f57462b05a9. This session holds the prgs-author profile, which forbids gitea.pr.approve, gitea.pr.merge, and gitea.pr.request_changes; no review verdict was prepared or posted from this session.

What changed

Two pushes have landed on fix/issue-714-session-context-immutability since review 437 was recorded:

  • 29ffe14 — centralized test-boundary isolation of _SESSION_CONTEXT (resolves the contamination review 437 identifies).
  • d936da5 — this session: three additional tests closing coverage gaps found while auditing the required session-lifecycle properties. Tests only; no production code changed.

Server-side mutation ledger: 1 commit (d936da5), 1 non-force push (29ffe14..d936da5, fast-forward), 1 issue comment. No review verdict, no merge, no label change, no issue state change.

What is blocked

Blocker classification: no blocker

The 41 suite failures cited by review 437 no longer reproduce at the current head. The full suite reports zero failures and zero errors. The one remaining warning is a pre-existing StarletteDeprecationWarning in tests/test_webui_audit.py (httpx/starlette testclient), unrelated to Issue #714 and present independently of this branch.

Who/what acts next

Next actor: prgs-reviewer
Required action: Independent re-review of PR #715 at head d936da5c872fb4bad31536d80b7c5f57462b05a9, against Issue #714 acceptance criteria 1-8. Review 437 predates two pushes and does not describe this head.
Do not do: Do not treat review 437 as current. Do not re-review from the author session or the control checkout. Do not merge on the strength of this comment — it records author remediation only, not a review outcome. Do not include PR #710 or any eAgenda scope.

[/THREAD STATE LEDGER]

Author remediation — Issue #714 / PR #715

Cause of the contamination

session_context_binding._SESSION_CONTEXT is a process-global binding. Review 437 (head 632c5688) found it leaking across test boundaries: only TestOperationScopedRoles reset it, so a binding established by one test stayed live for every later test in the same process. Tests that legitimately change profile or identity were then measured against a stale predecessor binding and rejected as drift — 41 false-positive "drift block" failures. The drift protection itself was correct; the fault was that no test boundary re-established a clean session.

Lifecycle design

One atomic binding per logical MCP session:

  • _SessionContext is a frozen, slotted dataclass — a binding is replaced as a whole value, never partially mutated, so no caller can observe a half-initialized context.
  • seed_session_context_if_unbound binds only when the process has no current context (first-bind-wins). A changed environment or an interleaved call is not a session boundary and cannot displace an established binding. A new or forked process is recognized by PID.
  • bind_session_context is the single sanctioned re-bind, reachable only through explicit gitea_activate_profile — the legitimate new-logical-session boundary.
  • get_session_context returns a detached snapshot, so a caller mutating the returned dict cannot reach the binding.
  • assess_session_context compares live profile, remote, host, identity, repository, and org against the binding and fails closed on any drift before a mutation is attempted; require_bound=True also fails closed when unbound.

Reset-boundary security design

Reset is _reset_session_context_for_testing: private, and it raises RuntimeError unless PYTEST_CURRENT_TEST is present in the environment. It is therefore unreachable as a production bypass — a normal MCP caller cannot invoke it to clear a binding mid-request and re-bind to another profile. Production sessions transition only via process start / PID change or the explicit activation re-bind. Test isolation is centralized in the existing autouse _reset_mutation_authority fixture in tests/conftest.py, which resets before the test and again in a finally, so a failure in setup, test body, assertion, or teardown cannot leak a binding into the next test. No per-test resets were scattered across modules.

Concurrency treatment

_SESSION_CONTEXT is guarded by a module-level threading.RLock. Read, first-bind, assess, and re-bind all take the lock, and the stored value is immutable. Concurrent initialization resolves to exactly one winning binding, and every racing caller returns that same complete binding rather than its own proposed one. Interleaved calls cannot overwrite an established binding, and no caller can observe a torn or partially initialized context.

Files changed in this commit

  • tests/test_issue_714_session_context_immutability.py (+106, tests only)

The isolation fix itself landed earlier on this branch in 29ffe14 (tests/conftest.py, session_context_binding.py, gitea_mcp_server.py, and test modules). Auditing the required coverage against that head surfaced three properties the suite asserted nowhere, which d936da5 closes:

  1. Reverse cross-host denial — every existing cross-host test pinned mdcps-reviewer and denied prgs. test_prgs_request_cannot_resolve_through_mdcps_profile proves the mirror direction: a pinned MDCPS profile never serves a prgs request and is never swapped for prgs-author.
  2. Concurrent initialization — the existing parallel test binds first and then races, so racing first-binds from an unbound context were unproven. test_concurrent_initialization_has_single_winning_binding races 8 threads from unbound and asserts a single winner with no spliced identity.
  3. Repository/org driftassess_session_context checks these, but no test bound a repository/org and asserted that a same-host mismatch fails closed. test_repository_mismatch_blocks_before_mutation covers repository and org drift independently of host.

Each new test was mutation-verified: disabling the first-bind-wins guard, the cross-host denial, or the repository/org drift checks each makes the corresponding test fail. They are not vacuous.

Exact verification

Run from the PR #715 worktree only, using the project venv interpreter (venv/bin/python; a shell alias points bare python3 at 3.13, which has no pytest installed):

  1. python -m pytest tests/test_issue_714_session_context_immutability.py -q -s
    20 pass, 0 fail, 0 skip, 0 error — 1.09s
  2. python -m pytest tests/test_resolve_task_capability.py tests/test_operation_scoped_roles.py tests/test_mcp_server.py -q -s (modules named in review 437)
    240 pass, 0 fail, 0 skip, 0 error — 5.87s
    Classes named in review 437 (TestResolveTaskCapability, TestReviewerAuthorMutationBlocks, TestCreateIssue, TestCreatePR)
    62 pass, 2654 deselected — 2.35s
  3. python -m pytest tests/test_issue_714_session_context_immutability.py -q -s -k "concurrent or parallel or boundary or reset or drift or mismatch or contaminat or escape or survives"
    12 pass, 8 deselected — 0.47s
  4. python -m pytest tests/ -q -s
    2713 pass, 6 skip, 0 fail, 0 error, 1 warning, 161 subtests pass — 23.05s

The 41 failures reported in review 437 are gone. The prior head 29ffe14 measured 2710 pass / 6 skip; the +3 delta is exactly the three tests added here.

Security readback

  • Production session bindings remain immutable; the only sanctioned transitions are process start / PID change and explicit gitea_activate_profile.
  • No security assertion was weakened, removed, skipped, or rewritten. This commit adds tests only and changes no production code.
  • No mutation gate was bypassed. Cross-host, identity, repository, org, host, and profile mismatches each fail closed before any write is attempted.
  • The reset mechanism is unavailable to normal production callers (private, plus PYTEST_CURRENT_TEST required, else RuntimeError).
  • The Gitea-Tools control checkout was untouched: still on master at 1eafb757, carrying only its pre-existing untracked files.
  • No eAgenda repository or worktree was accessed or modified; eAgenda PR #194 untouched.
  • PR #710 untouched.

Request

Requesting independent prgs-reviewer re-review of PR #715 at head d936da5c872fb4bad31536d80b7c5f57462b05a9. Author identity posted no review verdict and performed no merge.

## Canonical Issue State ISSUE: #714 PR: #715 STATE: OPEN WHO_IS_NEXT: reviewer NEXT_ACTION: Independent prgs-reviewer re-review of PR #715 at head d936da5c872fb4bad31536d80b7c5f57462b05a9 NEXT_PROMPT: ```text REVIEWER task — Gitea-Tools PR #715 (Issue #714), repository Scaled-Tech-Consulting/Gitea-Tools, remote prgs. Use the native gitea-reviewer namespace with the prgs-reviewer profile. Acquire your own reviewer lease; do not adopt the author session. Pin and verify head d936da5c872fb4bad31536d80b7c5f57462b05a9 before analysis. Review 437 (REQUEST_CHANGES) was recorded at head 632c5688 and predates two pushes; it does not describe this head. Scope: Issue #714 acceptance criteria 1-8 (cross-host MCP profile drift and capability substitution fail closed). Verify specifically: - Repeated calls after pinning mdcps-reviewer never report prgs-author. - A dadeschools request cannot resolve through a prgs profile, and a prgs request cannot resolve through an MDCPS profile. - Unsupported capability returns a structured denial without profile substitution. - Identity, repository, org, host, and profile mismatch each fail closed before mutation. - _reset_session_context_for_testing cannot be reached by a production caller. - Concurrent initialization yields one winning binding; interleaved calls cannot overwrite an established binding. - Existing static-profile namespaces remain functional. Run the full suite from a reviewer worktree using the project venv interpreter (bare python3 is aliased to 3.13 and has no pytest): venv/bin/python -m pytest tests/ -q -s Author measured 2713 pass, 6 skip, 1 pre-existing warning, 161 subtests, 23.05s. Do not merge. Record a formal verdict via the review API only. ``` WHAT_HAPPENED: Author remediated the test-context contamination cited by review 437 and pushed two commits. `29ffe14` centralized `_SESSION_CONTEXT` isolation at the test boundary; `d936da5` adds three tests closing coverage gaps found while auditing the required session-lifecycle properties. The full suite now reports zero failures where review 437 measured 41. WHY: `session_context_binding._SESSION_CONTEXT` is a process-global binding that only `TestOperationScopedRoles` reset, so one test's binding stayed live for later tests in the same process; tests that legitimately change profile or identity were then measured against a stale predecessor binding and rejected as drift. The drift protection was correct — the test boundary was not re-establishing a clean session. RELATED_PRS: #715 (this PR, Closes #714); #711 is the dependent follow-on (MDCPS close path, worktree_path, reconciler routing) and remains outstanding; MDCPS reconciliation stays prohibited until #714 and #711 are both landed and verified. PR #710 is out of scope and untouched. HEAD_SHA: d936da5c872fb4bad31536d80b7c5f57462b05a9 PRIOR_HEAD_SHA: 29ffe1407fa9620b4a75035bb9be3a254e74a674 REVIEWED_HEAD_SHA: 632c568865cab3de7a8d9970463ec182578d0a86 REVIEW_STATUS: CHANGES_REQUESTED MERGE_READY: false BLOCKERS: none VALIDATION: Full suite 2713 pass, 6 skip, 1 pre-existing warning, 161 subtests in 23.05s LAST_UPDATED_BY: jcwalker3 [THREAD STATE LEDGER] ### What is true now Server-side decision state: PR #715 carries one REQUEST_CHANGES review verdict (review_id 437) recorded via the review API by sysadmin at head `632c5688`. It is undismissed and stale against the current head; approval_at_current_head is false and no APPROVE verdict exists for any head. Local verdict/state: Author remediation is complete at head `d936da5c872fb4bad31536d80b7c5f57462b05a9`. This session holds the `prgs-author` profile, which forbids gitea.pr.approve, gitea.pr.merge, and gitea.pr.request_changes; no review verdict was prepared or posted from this session. ### What changed Two pushes have landed on `fix/issue-714-session-context-immutability` since review 437 was recorded: - `29ffe14` — centralized test-boundary isolation of `_SESSION_CONTEXT` (resolves the contamination review 437 identifies). - `d936da5` — this session: three additional tests closing coverage gaps found while auditing the required session-lifecycle properties. Tests only; no production code changed. Server-side mutation ledger: 1 commit (`d936da5`), 1 non-force push (`29ffe14..d936da5`, fast-forward), 1 issue comment. No review verdict, no merge, no label change, no issue state change. ### What is blocked Blocker classification: no blocker The 41 suite failures cited by review 437 no longer reproduce at the current head. The full suite reports zero failures and zero errors. The one remaining warning is a pre-existing `StarletteDeprecationWarning` in `tests/test_webui_audit.py` (httpx/starlette testclient), unrelated to Issue #714 and present independently of this branch. ### Who/what acts next Next actor: prgs-reviewer Required action: Independent re-review of PR #715 at head `d936da5c872fb4bad31536d80b7c5f57462b05a9`, against Issue #714 acceptance criteria 1-8. Review 437 predates two pushes and does not describe this head. Do not do: Do not treat review 437 as current. Do not re-review from the author session or the control checkout. Do not merge on the strength of this comment — it records author remediation only, not a review outcome. Do not include PR #710 or any eAgenda scope. [/THREAD STATE LEDGER] ## Author remediation — Issue #714 / PR #715 ### Cause of the contamination `session_context_binding._SESSION_CONTEXT` is a process-global binding. Review 437 (head `632c5688`) found it leaking across test boundaries: only `TestOperationScopedRoles` reset it, so a binding established by one test stayed live for every later test in the same process. Tests that legitimately change profile or identity were then measured against a stale predecessor binding and rejected as drift — 41 false-positive "drift block" failures. The drift protection itself was correct; the fault was that no test boundary re-established a clean session. ### Lifecycle design One atomic binding per logical MCP session: - `_SessionContext` is a frozen, slotted dataclass — a binding is replaced as a whole value, never partially mutated, so no caller can observe a half-initialized context. - `seed_session_context_if_unbound` binds only when the process has no current context (first-bind-wins). A changed environment or an interleaved call is not a session boundary and cannot displace an established binding. A new or forked process is recognized by PID. - `bind_session_context` is the single sanctioned re-bind, reachable only through explicit `gitea_activate_profile` — the legitimate new-logical-session boundary. - `get_session_context` returns a detached snapshot, so a caller mutating the returned dict cannot reach the binding. - `assess_session_context` compares live profile, remote, host, identity, repository, and org against the binding and fails closed on any drift before a mutation is attempted; `require_bound=True` also fails closed when unbound. ### Reset-boundary security design Reset is `_reset_session_context_for_testing`: private, and it raises `RuntimeError` unless `PYTEST_CURRENT_TEST` is present in the environment. It is therefore unreachable as a production bypass — a normal MCP caller cannot invoke it to clear a binding mid-request and re-bind to another profile. Production sessions transition only via process start / PID change or the explicit activation re-bind. Test isolation is centralized in the existing autouse `_reset_mutation_authority` fixture in `tests/conftest.py`, which resets before the test and again in a `finally`, so a failure in setup, test body, assertion, or teardown cannot leak a binding into the next test. No per-test resets were scattered across modules. ### Concurrency treatment `_SESSION_CONTEXT` is guarded by a module-level `threading.RLock`. Read, first-bind, assess, and re-bind all take the lock, and the stored value is immutable. Concurrent initialization resolves to exactly one winning binding, and every racing caller returns that same complete binding rather than its own proposed one. Interleaved calls cannot overwrite an established binding, and no caller can observe a torn or partially initialized context. ### Files changed in this commit - `tests/test_issue_714_session_context_immutability.py` (+106, tests only) The isolation fix itself landed earlier on this branch in `29ffe14` (`tests/conftest.py`, `session_context_binding.py`, `gitea_mcp_server.py`, and test modules). Auditing the required coverage against that head surfaced three properties the suite asserted nowhere, which `d936da5` closes: 1. **Reverse cross-host denial** — every existing cross-host test pinned `mdcps-reviewer` and denied `prgs`. `test_prgs_request_cannot_resolve_through_mdcps_profile` proves the mirror direction: a pinned MDCPS profile never serves a `prgs` request and is never swapped for `prgs-author`. 2. **Concurrent initialization** — the existing parallel test binds first and then races, so racing *first-binds* from an unbound context were unproven. `test_concurrent_initialization_has_single_winning_binding` races 8 threads from unbound and asserts a single winner with no spliced identity. 3. **Repository/org drift** — `assess_session_context` checks these, but no test bound a repository/org and asserted that a same-host mismatch fails closed. `test_repository_mismatch_blocks_before_mutation` covers repository and org drift independently of host. Each new test was mutation-verified: disabling the first-bind-wins guard, the cross-host denial, or the repository/org drift checks each makes the corresponding test fail. They are not vacuous. ### Exact verification Run from the PR #715 worktree only, using the project venv interpreter (`venv/bin/python`; a shell alias points bare `python3` at 3.13, which has no pytest installed): 1. `python -m pytest tests/test_issue_714_session_context_immutability.py -q -s` → **20 pass, 0 fail, 0 skip, 0 error — 1.09s** 2. `python -m pytest tests/test_resolve_task_capability.py tests/test_operation_scoped_roles.py tests/test_mcp_server.py -q -s` (modules named in review 437) → **240 pass, 0 fail, 0 skip, 0 error — 5.87s** Classes named in review 437 (`TestResolveTaskCapability`, `TestReviewerAuthorMutationBlocks`, `TestCreateIssue`, `TestCreatePR`) → **62 pass, 2654 deselected — 2.35s** 3. `python -m pytest tests/test_issue_714_session_context_immutability.py -q -s -k "concurrent or parallel or boundary or reset or drift or mismatch or contaminat or escape or survives"` → **12 pass, 8 deselected — 0.47s** 4. `python -m pytest tests/ -q -s` → **2713 pass, 6 skip, 0 fail, 0 error, 1 warning, 161 subtests pass — 23.05s** The 41 failures reported in review 437 are gone. The prior head `29ffe14` measured 2710 pass / 6 skip; the +3 delta is exactly the three tests added here. ### Security readback - Production session bindings remain immutable; the only sanctioned transitions are process start / PID change and explicit `gitea_activate_profile`. - No security assertion was weakened, removed, skipped, or rewritten. This commit adds tests only and changes no production code. - No mutation gate was bypassed. Cross-host, identity, repository, org, host, and profile mismatches each fail closed before any write is attempted. - The reset mechanism is unavailable to normal production callers (private, plus `PYTEST_CURRENT_TEST` required, else `RuntimeError`). - The Gitea-Tools control checkout was untouched: still on `master` at `1eafb757`, carrying only its pre-existing untracked files. - No eAgenda repository or worktree was accessed or modified; eAgenda PR #194 untouched. - PR #710 untouched. ### Request Requesting independent `prgs-reviewer` re-review of PR #715 at head `d936da5c872fb4bad31536d80b7c5f57462b05a9`. Author identity posted no review verdict and performed no merge.
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #715
issue: #714
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 25509-f2c2f35d026b
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-pr715-d936da5
phase: claimed
candidate_head: d936da5c87
target_branch: master
target_branch_sha: none
last_activity: 2026-07-15T20:05:21Z
expires_at: 2026-07-15T22:05:21Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #715 issue: #714 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 25509-f2c2f35d026b worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-pr715-d936da5 phase: claimed candidate_head: d936da5c872fb4bad31536d80b7c5f57462b05a9 target_branch: master target_branch_sha: none last_activity: 2026-07-15T20:05:21Z expires_at: 2026-07-15T22:05:21Z blocker: none
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #715
issue: #714
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 25509-f2c2f35d026b
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-pr715-d936da5
phase: decision
candidate_head: d936da5c87
target_branch: master
target_branch_sha: none
last_activity: 2026-07-15T20:08:19Z
expires_at: 2026-07-15T22:08:19Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #715 issue: #714 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 25509-f2c2f35d026b worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-pr715-d936da5 phase: decision candidate_head: d936da5c872fb4bad31536d80b7c5f57462b05a9 target_branch: master target_branch_sha: none last_activity: 2026-07-15T20:08:19Z expires_at: 2026-07-15T22:08:19Z blocker: none
You are not authorized to merge this pull request.
This pull request can be merged automatically.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/issue-714-session-context-immutability:fix/issue-714-session-context-immutability
git checkout fix/issue-714-session-context-immutability
Sign in to join this conversation.
No Reviewers
No labels
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

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