feat(transport): add transport-neutral MCP bind seam (Closes #931) #966

Merged
sysadmin merged 4 commits from feat/issue-931-transport-neutral-bind-seam into master 2026-07-28 13:36:49 -05:00
Owner

Closes #931

Problem and security boundary

The server bound one transport, by name, at the bottom of the module:

mcp_daemon_guard.bind_native_mcp_transport(transport="stdio")

Everything that later asks "is this a trusted native session" resolves that question through the value bound there — assess_transport_for_auth_mint, the decision-lock provenance stamp that defaults to transport: untrusted, and the mutation guards that refuse offline imports. The string was a constant in the authorization chain rather than configuration, so a remote transport had no way to be expressed and no way to be distinguished from an untrusted offline import.

This is boundary B1 in docs/remote-mcp/threat-model.md. The boundary still rests on the bind rather than on an authenticated caller — moving it to an authenticated handshake is #932/#938. What changes here is only that the bound value becomes first-class, validated, and durably recorded.

Implementation summary

New mcp_transport_config.py is the single source of truth. It holds no state and owns exactly three things: the permitted set, the default, and the resolution of deployment configuration into a validated identifier.

  • TRANSPORT_ENV = "GITEA_MCP_TRANSPORT" — read once, at bind time.
  • DEFAULT_TRANSPORT = "stdio" — what unset configuration yields.
  • REMOTE_TRANSPORT = "streamable-http" — the sanctioned non-stdio identifier, accepted so the bind is pluggable. Its listener is #938 and is not implemented here.
  • SUPPORTED_TRANSPORTS = {stdio, streamable-http} — the only place identifiers are enumerated. sse is a real MCP transport and is deliberately not registered, so it exercises the fail-closed path like any other unregistered name.

mcp_daemon_guard.py

  • _PRODUCTION_TRANSPORTS is now an alias of the seam's permitted set, so the guard never restates an identifier.
  • bind_native_mcp_transport(transport=None) resolves from configuration when the argument is omitted (what the entrypoint does), validates against the permitted set before writing the runtime record, and pins the result. An explicit argument still works for launchers and tests.
  • bound_transport() is the shared accessor every transport-aware guard reads. It returns the value pinned at bind, never the environment. None means unbound.
  • assert_transport_bound() fails closed before tool service.
  • Rebinding the same identifier is idempotent; rebinding a different one is refused, so two guards can never disagree within one process.
  • native_runtime_status() and mutation_provenance_fields() expose bound_transport.

Entrypoint (gitea_mcp_server.py)

mcp_daemon_guard.bind_native_mcp_transport()
...
mcp.run(transport=mcp_daemon_guard.assert_transport_bound("tool service"))

Both former literals are gone. mcp.run is now served exactly the transport that was bound, read back through the one accessor.

Durable provenancemutation_provenance_fields() gains bound_transport, which reaches the durable decision lock through mcp_session_state.save_state and the audit records that already spread those fields (gitea_mcp_server.py decision-lock payload and live_mutations entry). assess_transport_for_auth_mint reports the bound transport through that one accessor; its verdict is unchanged.

#956 anchors — this change shifts lines that docs/remote-mcp/threat-model-anchors.json cites, so the fixture and the prose are re-anchored, and the two boundary claims #931 makes false (the "literal stdio bind" in B1, and the stdio contract at mcp_server.py:4) are restated. Without this the #956 guard fails — which is exactly what it exists to do.

Why existing stdio behavior is preserved

  • Unset GITEA_MCP_TRANSPORT resolves to stdio, so every current deployment binds precisely what it bound before.
  • mutation_provenance_fields()["transport"] keeps its trust-class values (native_mcp / test_native_mcp / untrusted). The bound identifier is added under a new key, so no durable record changes shape and no stdio expected value was edited.
  • is_native_mcp_transport, is_production_native_mcp_transport, assert_sanctioned_mutation_runtime, assert_production_mutation_runtime and assert_keychain_access_allowed are untouched.
  • The #695 AC2 session-state pin, the canonical-entrypoint requirement, and the prior-claim requirement all still hold, and are re-asserted by tests.

How invalid identifiers fail closed

  • An unregistered identifier raises UnsanctionedRuntimeError at bind time, with the offending value and #931 in the reason. Nothing is bound, so bound_transport() stays None and is_native_mcp_transport() stays false.
  • An unregistered value is never silently degraded to the default — resolve_configured_transport reports supported: false and keeps the offending name.
  • Because the bind raised, assert_transport_bound() refuses before mcp.run, so the server does not reach tool service.
  • Non-string values normalize to "" and can never match a permitted member.
  • Post-bind environment changes cannot move the value a guard observes — the rule already applied to the session-state root under #695 AC2.

Explicit non-goals (untouched)

  • The remote transport listener — #938.
  • Per-request authenticated principal resolution — #932.
  • Client-managed provenance / attribution policy — #934.
  • TLS, certificates, remote client authentication.
  • Role capability or permission sets; repository-binding semantics.
  • Deployment, client commissioning, concurrency certification, unattended scheduling.
  • #936, #957, #949 — read only to confirm scope boundaries, not modified.

Test commands and results

All runs from the branches/ worktree, never a temporary directory.

PYTHONPATH=. venv/bin/python -m pytest tests/test_issue_931_transport_bind_seam.py -q
# exit 0 — 42 passed

PYTHONPATH=. venv/bin/python -m pytest tests/test_issue_695_native_transport_quarantine.py tests/test_mcp_daemon_guard.py -q
# exit 0 — 33 passed

PYTHONPATH=. venv/bin/python -m pytest tests/test_issue_709_decision_lock_cross_profile.py \
  tests/test_issue_720_expired_decision_lock.py tests/test_mcp_session_state.py \
  tests/test_session_state_isolation.py -q
# exit 0 — 140 passed

PYTHONPATH=. venv/bin/python -m pytest tests/test_issue_956_threat_model.py -q
# exit 0 — 17 passed

PYTHONPATH=. venv/bin/python -m pytest tests/test_resolve_task_capability.py \
  tests/test_task_capability_role_invariants.py tests/test_lock_issue_mcp_registration.py \
  tests/test_commit_files_capability.py -q
# exit 0 — 48 passed, 86 subtests passed

PYTHONPATH=. venv/bin/python -m pytest tests/test_allocator_inventory_mcp.py \
  tests/test_pr_queue_inventory.py tests/test_reviewer_inventory_worktree.py -q
# exit 0 — 39 passed

PYTHONPATH=. venv/bin/python -m pytest tests/test_issue_686_manual_mcp_provenance.py \
  tests/test_structured_auth_mcp_errors.py tests/test_mcp_stale_runtime.py -q
# exit 0 — 34 passed

PYTHONPATH=. venv/bin/python -m pytest tests/test_mcp_server.py tests/test_health.py \
  tests/test_issue_714_production_first_bind.py -q
# exit 1 — 2 failed, 248 passed  (both failures pre-existing, see below)

PYTHONPATH=. venv/bin/python -m pytest tests/ -q
# exit 1 — 28 failed, 5843 passed, 6 skipped, 1047 subtests passed

Base/head failure comparison

Baseline run from a dedicated branches/ worktree at the pinned base, branches/baseline-931-9b80e75c:

# base 9b80e75ca3f441fec2fb077a1b5f874faa0912e2
PYTHONPATH=. venv/bin/python -m pytest tests/ -q
# exit 1 — 28 failed, 5801 passed, 6 skipped, 1047 subtests passed

Compared by failing test ID, not by count:

comm -23 head_ids base_ids   # only on head: (empty)
comm -13 head_ids base_ids   # only on base: (empty)

The failing sets are identical — 28 IDs on both sides. No failure added, none fixed. The +42 passed are exactly this PR's new module.

The two tests/test_mcp_server.py failures seen in the targeted run were each verified on the pinned base and fail there identically, at identical assertion lines with identical reasons:

  • TestRuntimeProfile::test_whoami_v2_metadataAssertionError: 'reviewer' != None at tests/test_mcp_server.py:1820
  • TestPreflightVerification::test_declared_clean_task_worktree_ignores_control_checkout_violationAssertionError: False is not true at tests/test_mcp_server.py:4344

Neither touches transport. Tool inventory: tests/test_allocator_inventory_mcp.py, tests/test_pr_queue_inventory.py, tests/test_reviewer_inventory_worktree.py are green (39 passed), and no inventory test appears in either failing set, so this change neither expands nor alters any pre-existing inventory discrepancy.

One failure was introduced during authoring and is fixed in this PR: tests/test_issue_956_threat_model.py::...::test_every_declared_anchor_resolves_to_the_claimed_source_line, because the edits shifted 29 cited lines. That is the #956 guard doing its job; the fixture and prose are re-anchored, and the final head has zero delta against base.

SHAs

  • Base: 9b80e75ca3f441fec2fb077a1b5f874faa0912e2 (master)
  • Head: 2fb835a1aa4cb6260445f2ef3f10057aabd2fbe7
  • Commits: c1626081 (the seam) and 2fb835a1 (stamp the commit the #956 anchors were re-derived at — a fixture must name a revision where its anchors actually resolve; the stamp commit touches no .py, so every anchor valid at c1626081 is valid at head)

Changed files

docs/remote-mcp/threat-model-anchors.json   |  58 +--
docs/remote-mcp/threat-model.md             |  58 +--
gitea_mcp_server.py                         |  16 +-
irrecoverable_provenance.py                 |   6 +
mcp_daemon_guard.py                         | 118 +++++-
mcp_server.py                               |  10 +-
mcp_session_state.py                        |   4 +
mcp_transport_config.py                     | 154 +++++++
tests/test_issue_931_transport_bind_seam.py | 595 ++++++++++++++++++++++++++++

No credentials, tokens, endpoints, or secret values. No generated or temporary artifacts. No formatting rewrite beyond the change — the anchor fixture was rewritten in place, preserving its one-object-per-line layout, so its diff is 29 changed anchors rather than a reformat.

Preserved state

Issue #949, branch feat/issue-949-native-fleet-inventory, its frozen head 92615f47, and worktree branches/issue-949-fleet-inventory are untouched. #936, #950–#952, closed #953, merged PR #954, #957, and PR #906's stale author lease are untouched. No cleanup, recovery, deploy, restart, reconnect, drain, reconciliation, branch deletion, or worktree removal was performed.

Reviewer notes

Suggested starting points:

  1. mcp_transport_config.SUPPORTED_TRANSPORTS is the only enumeration — confirm via grep -n '"stdio"' *.py, which should hit only that module.
  2. The rebind contract in bind_native_mcp_transport sits after the entrypoint/mode/path checks, so a rebind attempt from a non-entrypoint still fails on provenance first.
  3. tests/test_issue_931_transport_bind_seam.py::_ProductionBind drives the real bind path — it patches only the canonical-entrypoint frame and the pytest allowance, so validation, pinning and the rebind contract all execute unmodified.
  4. Worth challenging: whether accepting streamable-http before #938 exists is the right call, and whether excluding sse should instead be a documented deprecation.

Handoff

WHO_IS_NEXT: reviewer — independent review against the #931 acceptance criteria in a fresh session. Do not self-review or self-merge.

Closes #931 ## Problem and security boundary The server bound one transport, by name, at the bottom of the module: ```python mcp_daemon_guard.bind_native_mcp_transport(transport="stdio") ``` Everything that later asks *"is this a trusted native session"* resolves that question through the value bound there — `assess_transport_for_auth_mint`, the decision-lock provenance stamp that defaults to `transport: untrusted`, and the mutation guards that refuse offline imports. The string was a constant in the authorization chain rather than configuration, so a remote transport had no way to be expressed and no way to be distinguished from an untrusted offline import. This is boundary **B1** in `docs/remote-mcp/threat-model.md`. The boundary still rests on the *bind* rather than on an authenticated caller — moving it to an authenticated handshake is #932/#938. What changes here is only that the bound value becomes first-class, validated, and durably recorded. ## Implementation summary **New `mcp_transport_config.py`** is the single source of truth. It holds no state and owns exactly three things: the permitted set, the default, and the resolution of deployment configuration into a validated identifier. - `TRANSPORT_ENV = "GITEA_MCP_TRANSPORT"` — read once, at bind time. - `DEFAULT_TRANSPORT = "stdio"` — what unset configuration yields. - `REMOTE_TRANSPORT = "streamable-http"` — the sanctioned non-stdio identifier, accepted so the bind is pluggable. Its listener is **#938** and is not implemented here. - `SUPPORTED_TRANSPORTS = {stdio, streamable-http}` — the only place identifiers are enumerated. `sse` is a real MCP transport and is deliberately **not** registered, so it exercises the fail-closed path like any other unregistered name. **`mcp_daemon_guard.py`** - `_PRODUCTION_TRANSPORTS` is now an alias of the seam's permitted set, so the guard never restates an identifier. - `bind_native_mcp_transport(transport=None)` resolves from configuration when the argument is omitted (what the entrypoint does), validates against the permitted set *before* writing the runtime record, and pins the result. An explicit argument still works for launchers and tests. - **`bound_transport()`** is the shared accessor every transport-aware guard reads. It returns the value pinned at bind, never the environment. `None` means unbound. - **`assert_transport_bound()`** fails closed before tool service. - Rebinding the *same* identifier is idempotent; rebinding a *different* one is refused, so two guards can never disagree within one process. - `native_runtime_status()` and `mutation_provenance_fields()` expose `bound_transport`. **Entrypoint (`gitea_mcp_server.py`)** ```python mcp_daemon_guard.bind_native_mcp_transport() ... mcp.run(transport=mcp_daemon_guard.assert_transport_bound("tool service")) ``` Both former literals are gone. `mcp.run` is now served exactly the transport that was bound, read back through the one accessor. **Durable provenance** — `mutation_provenance_fields()` gains `bound_transport`, which reaches the durable decision lock through `mcp_session_state.save_state` and the audit records that already spread those fields (`gitea_mcp_server.py` decision-lock payload and `live_mutations` entry). `assess_transport_for_auth_mint` reports the bound transport through that one accessor; **its verdict is unchanged**. **#956 anchors** — this change shifts lines that `docs/remote-mcp/threat-model-anchors.json` cites, so the fixture and the prose are re-anchored, and the two boundary claims #931 makes false (the "literal `stdio` bind" in B1, and the stdio contract at `mcp_server.py:4`) are restated. Without this the #956 guard fails — which is exactly what it exists to do. ## Why existing stdio behavior is preserved - Unset `GITEA_MCP_TRANSPORT` resolves to `stdio`, so every current deployment binds precisely what it bound before. - `mutation_provenance_fields()["transport"]` keeps its **trust-class** values (`native_mcp` / `test_native_mcp` / `untrusted`). The bound identifier is added under a **new** key, so no durable record changes shape and no stdio expected value was edited. - `is_native_mcp_transport`, `is_production_native_mcp_transport`, `assert_sanctioned_mutation_runtime`, `assert_production_mutation_runtime` and `assert_keychain_access_allowed` are untouched. - The #695 AC2 session-state pin, the canonical-entrypoint requirement, and the prior-claim requirement all still hold, and are re-asserted by tests. ## How invalid identifiers fail closed - An unregistered identifier raises `UnsanctionedRuntimeError` **at bind time**, with the offending value and `#931` in the reason. Nothing is bound, so `bound_transport()` stays `None` and `is_native_mcp_transport()` stays false. - An unregistered value is never silently degraded to the default — `resolve_configured_transport` reports `supported: false` and keeps the offending name. - Because the bind raised, `assert_transport_bound()` refuses before `mcp.run`, so the server does not reach tool service. - Non-string values normalize to `""` and can never match a permitted member. - Post-bind environment changes cannot move the value a guard observes — the rule already applied to the session-state root under #695 AC2. ## Explicit non-goals (untouched) - The remote transport listener — **#938**. - Per-request authenticated principal resolution — **#932**. - Client-managed provenance / attribution policy — **#934**. - TLS, certificates, remote client authentication. - Role capability or permission sets; repository-binding semantics. - Deployment, client commissioning, concurrency certification, unattended scheduling. - **#936**, **#957**, **#949** — read only to confirm scope boundaries, not modified. ## Test commands and results All runs from the `branches/` worktree, never a temporary directory. ``` PYTHONPATH=. venv/bin/python -m pytest tests/test_issue_931_transport_bind_seam.py -q # exit 0 — 42 passed PYTHONPATH=. venv/bin/python -m pytest tests/test_issue_695_native_transport_quarantine.py tests/test_mcp_daemon_guard.py -q # exit 0 — 33 passed PYTHONPATH=. venv/bin/python -m pytest tests/test_issue_709_decision_lock_cross_profile.py \ tests/test_issue_720_expired_decision_lock.py tests/test_mcp_session_state.py \ tests/test_session_state_isolation.py -q # exit 0 — 140 passed PYTHONPATH=. venv/bin/python -m pytest tests/test_issue_956_threat_model.py -q # exit 0 — 17 passed PYTHONPATH=. venv/bin/python -m pytest tests/test_resolve_task_capability.py \ tests/test_task_capability_role_invariants.py tests/test_lock_issue_mcp_registration.py \ tests/test_commit_files_capability.py -q # exit 0 — 48 passed, 86 subtests passed PYTHONPATH=. venv/bin/python -m pytest tests/test_allocator_inventory_mcp.py \ tests/test_pr_queue_inventory.py tests/test_reviewer_inventory_worktree.py -q # exit 0 — 39 passed PYTHONPATH=. venv/bin/python -m pytest tests/test_issue_686_manual_mcp_provenance.py \ tests/test_structured_auth_mcp_errors.py tests/test_mcp_stale_runtime.py -q # exit 0 — 34 passed PYTHONPATH=. venv/bin/python -m pytest tests/test_mcp_server.py tests/test_health.py \ tests/test_issue_714_production_first_bind.py -q # exit 1 — 2 failed, 248 passed (both failures pre-existing, see below) PYTHONPATH=. venv/bin/python -m pytest tests/ -q # exit 1 — 28 failed, 5843 passed, 6 skipped, 1047 subtests passed ``` ## Base/head failure comparison Baseline run from a dedicated `branches/` worktree at the pinned base, `branches/baseline-931-9b80e75c`: ``` # base 9b80e75ca3f441fec2fb077a1b5f874faa0912e2 PYTHONPATH=. venv/bin/python -m pytest tests/ -q # exit 1 — 28 failed, 5801 passed, 6 skipped, 1047 subtests passed ``` Compared by **failing test ID**, not by count: ``` comm -23 head_ids base_ids # only on head: (empty) comm -13 head_ids base_ids # only on base: (empty) ``` The failing sets are **identical** — 28 IDs on both sides. No failure added, none fixed. The `+42` passed are exactly this PR's new module. The two `tests/test_mcp_server.py` failures seen in the targeted run were each verified on the pinned base and fail there identically, at identical assertion lines with identical reasons: - `TestRuntimeProfile::test_whoami_v2_metadata` — `AssertionError: 'reviewer' != None` at `tests/test_mcp_server.py:1820` - `TestPreflightVerification::test_declared_clean_task_worktree_ignores_control_checkout_violation` — `AssertionError: False is not true` at `tests/test_mcp_server.py:4344` Neither touches transport. **Tool inventory**: `tests/test_allocator_inventory_mcp.py`, `tests/test_pr_queue_inventory.py`, `tests/test_reviewer_inventory_worktree.py` are green (39 passed), and no inventory test appears in either failing set, so this change neither expands nor alters any pre-existing inventory discrepancy. One failure **was** introduced during authoring and is fixed in this PR: `tests/test_issue_956_threat_model.py::...::test_every_declared_anchor_resolves_to_the_claimed_source_line`, because the edits shifted 29 cited lines. That is the #956 guard doing its job; the fixture and prose are re-anchored, and the final head has zero delta against base. ## SHAs - **Base**: `9b80e75ca3f441fec2fb077a1b5f874faa0912e2` (`master`) - **Head**: `2fb835a1aa4cb6260445f2ef3f10057aabd2fbe7` - Commits: `c1626081` (the seam) and `2fb835a1` (stamp the commit the #956 anchors were re-derived at — a fixture must name a revision where its anchors actually resolve; the stamp commit touches no `.py`, so every anchor valid at `c1626081` is valid at head) ## Changed files ``` docs/remote-mcp/threat-model-anchors.json | 58 +-- docs/remote-mcp/threat-model.md | 58 +-- gitea_mcp_server.py | 16 +- irrecoverable_provenance.py | 6 + mcp_daemon_guard.py | 118 +++++- mcp_server.py | 10 +- mcp_session_state.py | 4 + mcp_transport_config.py | 154 +++++++ tests/test_issue_931_transport_bind_seam.py | 595 ++++++++++++++++++++++++++++ ``` No credentials, tokens, endpoints, or secret values. No generated or temporary artifacts. No formatting rewrite beyond the change — the anchor fixture was rewritten in place, preserving its one-object-per-line layout, so its diff is 29 changed anchors rather than a reformat. ## Preserved state Issue #949, branch `feat/issue-949-native-fleet-inventory`, its frozen head `92615f47`, and worktree `branches/issue-949-fleet-inventory` are untouched. #936, #950–#952, closed #953, merged PR #954, #957, and PR #906's stale author lease are untouched. No cleanup, recovery, deploy, restart, reconnect, drain, reconciliation, branch deletion, or worktree removal was performed. ## Reviewer notes Suggested starting points: 1. `mcp_transport_config.SUPPORTED_TRANSPORTS` is the only enumeration — confirm via `grep -n '"stdio"' *.py`, which should hit only that module. 2. The rebind contract in `bind_native_mcp_transport` sits *after* the entrypoint/mode/path checks, so a rebind attempt from a non-entrypoint still fails on provenance first. 3. `tests/test_issue_931_transport_bind_seam.py::_ProductionBind` drives the real bind path — it patches only the canonical-entrypoint frame and the pytest allowance, so validation, pinning and the rebind contract all execute unmodified. 4. Worth challenging: whether accepting `streamable-http` before #938 exists is the right call, and whether excluding `sse` should instead be a documented deprecation. ## Handoff **WHO_IS_NEXT: reviewer** — independent review against the #931 acceptance criteria in a fresh session. Do not self-review or self-merge.
jcwalker3 added 2 commits 2026-07-28 10:10:37 -05:00
Closes #931.

The transport was a literal passed once at the bottom of the module,
bind_native_mcp_transport(transport="stdio"), and every guard that asks
"is this a trusted native session" resolves that question through the
value bound there. The string was a constant in the authorization chain
rather than configuration, so a remote transport had no way to be
expressed and no way to be told apart from an untrusted offline import.

mcp_transport_config becomes the single source of truth: it owns the
permitted set, the default, and the resolution of deployment
configuration (GITEA_MCP_TRANSPORT) into a validated identifier. Unset
configuration still yields stdio, so existing deployments are unchanged.
streamable-http is accepted so the bind is pluggable; standing up its
listener remains #938. sse is deliberately not registered.

bind_native_mcp_transport now resolves from configuration when no
transport argument is given, validates against that one permitted set
before writing the runtime record, and pins the result. bound_transport
is the shared accessor every transport-aware guard reads; it reports the
pinned value and never the environment, so a post-bind GITEA_MCP_TRANSPORT
change cannot move what a guard observes -- the rule already applied to
the session-state root under #695 AC2. Rebinding the same identifier is
idempotent; rebinding a different one is refused, so two guards can never
disagree within one process. assert_transport_bound fails closed before
mcp.run, so an absent or invalid bind stops the server instead of serving
tools over a transport no guard can name.

The bound identifier is now recorded in durable provenance:
mutation_provenance_fields gains bound_transport, which reaches the
decision lock through mcp_session_state.save_state and the audit records
that already spread those fields. The pre-existing transport field keeps
its trust-class values, so no durable record changes shape.
assess_transport_for_auth_mint reports the bound transport through the
same accessor; its verdict is unchanged.

#956's anchor fixture and threat model are re-anchored for the lines this
change shifts, and the two boundary claims that #931 makes false -- the
"literal stdio bind" in B1 and the stdio contract at mcp_server.py:4 --
are restated. Without this the #956 guard fails, which is what it is for.

Non-goals, untouched: the remote listener (#938), per-request principal
resolution (#932), client-managed provenance policy (#934), TLS and
remote client authentication, role capability sets, repository-binding
semantics.

Tests: tests/test_issue_931_transport_bind_seam.py, 42 tests, covering
default bind, explicit stdio, the accepted non-stdio identifier, an
unregistered identifier, no bind at all, one-value agreement across
guards, post-bind environment tampering, the durable decision-lock
record, the rebind contract, and the #695 protections that must not
regress.

Full suite from the branches/ worktree: 28 failed, 5843 passed, 6
skipped, 1047 subtests. The pinned base 9b80e75c reports 28 failed, 5801
passed, 6 skipped, 1047 subtests. The failing test IDs are identical sets
-- no failure added, none fixed; the +42 passed are this PR's new module.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The anchor fixture records the commit its anchors were taken at, and the
threat model must state the same commit, so a reviewer can resolve every
cited line at a named revision. Both still named aad5c8b4, where the
anchors no longer resolve after the transport seam shifted the lines they
point at.

Point both at c1626081, the seam commit whose tree the anchors were
re-derived from. This commit changes no .py file, so every anchor that
resolves at c1626081 resolves here too. The historical note about #930's
drift between 7bf4f125 and aad5c8b4 stays as written -- it is the reason
the guard exists.

Refs #931, #956.

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

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #966
issue: #931
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 34563-3952b6b47876
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr966-2fb835a1
phase: claimed
candidate_head: 2fb835a1aa
target_branch: master
target_branch_sha: 9b80e75ca3
last_activity: 2026-07-28T15:34:37Z
expires_at: 2026-07-28T15:44:37Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #966 issue: #931 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 34563-3952b6b47876 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr966-2fb835a1 phase: claimed candidate_head: 2fb835a1aa4cb6260445f2ef3f10057aabd2fbe7 target_branch: master target_branch_sha: 9b80e75ca3f441fec2fb077a1b5f874faa0912e2 last_activity: 2026-07-28T15:34:37Z expires_at: 2026-07-28T15:44:37Z blocker: none
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #966
issue: #931
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 34563-cf9dbf7ecfc9
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr966-2fb835a1
phase: claimed
candidate_head: 2fb835a1aa
target_branch: master
target_branch_sha: 9b80e75ca3
last_activity: 2026-07-28T15:49:09Z
expires_at: 2026-07-28T15:59:09Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #966 issue: #931 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 34563-cf9dbf7ecfc9 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr966-2fb835a1 phase: claimed candidate_head: 2fb835a1aa4cb6260445f2ef3f10057aabd2fbe7 target_branch: master target_branch_sha: 9b80e75ca3f441fec2fb077a1b5f874faa0912e2 last_activity: 2026-07-28T15:49:09Z expires_at: 2026-07-28T15:59:09Z blocker: none
sysadmin requested changes 2026-07-28 10:52:15 -05:00
Dismissed
sysadmin left a comment
Owner

Canonical PR State

STATE:
changes-requested

WHO_IS_NEXT:
author

NEXT_ACTION:
Split "registered transport identifier" from "servable transport" so a non-stdio identifier can be configured, pinned and recorded without mcp.run starting an unauthenticated listener, then push to feat/issue-931-transport-neutral-bind-seam and hand back for re-review.

NEXT_PROMPT:

Role: AUTHOR
Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools
PR: #966  Issue: #931
Reviewed head: 2fb835a1aa4cb6260445f2ef3f10057aabd2fbe7
Base: 9b80e75ca3f441fec2fb077a1b5f874faa0912e2

Two changes requested, both one root cause.

B1. `mcp.run(transport=assert_transport_bound(...))` plus `streamable-http`
    in SUPPORTED_TRANSPORTS means GITEA_MCP_TRANSPORT=streamable-http starts
    a uvicorn HTTP server (FastMCP.run_streamable_http_async) serving the
    whole tool surface with auth=None. #931 lists the listener as a non-goal
    and #938 owns it.
B2. `bound_transport` is consumed only by reporting sites. No guard, gate or
    refusal path reads it, so nothing distinguishes a local session from a
    remote one.

Fix direction: keep validating, pinning and recording streamable-http exactly
as now, but add a servable/commissioned check on the serve path so a transport
with no commissioned listener fails closed before mcp.run. One flip when #938
lands.

Also address, non-blocking:
- test_no_default_transport_literal_outside_the_seam scans five hardcoded
  module names; glob production modules instead.
- Add a test covering what serving a non-default transport implies, not only
  that the identifier binds.
- bound_transport() returns "test" under the pytest runtime, so a durable
  record can carry a value outside SUPPORTED_TRANSPORTS.

Do not change: the resolution/validation ordering, the rebind contract, the
#956 anchor updates, or the trust-class transport field. All verified correct.

WHAT_HAPPENED:
Independent review at head 2fb835a1aa in an isolated reviewer worktree. All six #931 acceptance criteria are met and independently reproduced. Two blocking findings sit outside the acceptance criteria, in the wiring between the seam and the server loop.

WHY:
Registering streamable-http while mcp.run is fed the bound value means one environment variable converts the server into an HTTP endpoint serving every tool with no authentication, no TLS and no per-request principal. Executed at this head: bind succeeds, is_native_mcp_transport True, assert_sanctioned_mutation_runtime and assert_production_mutation_runtime both pass, assess_transport_for_auth_mint allowed True, and mcp.run receives 'streamable-http'. FastMCP settings are host 127.0.0.1, port 8000, auth None, so exposure is local rather than network, which bounds severity but does not remove it: role separation currently rests on client-owned pipes, and an HTTP port has no such ownership. #931 names the listener as a non-goal and #938 owns authentication, principal binding, TLS and rate limiting, none of which exist yet. Separately, bound_transport is read only by reporting sites, so no decision anywhere depends on it.

ISSUE:
#931

HEAD_SHA:
2fb835a1aa

REVIEW_STATUS:
request_changes by sysadmin (prgs-reviewer) at head 2fb835a1aa4cb6260445f2ef3f10057aabd2fbe7; author is jcwalker3, so reviewer and author are distinct accounts

MERGE_READY:
no

BLOCKERS:
B1 and B2 below. Unblock condition: the serve path refuses to hand a transport to mcp.run unless its listener is commissioned, so that configuring streamable-http cannot start an unauthenticated endpoint; and the bound transport is consulted by at least the serve decision rather than by reporting alone. Re-review at the new head after that change.

VALIDATION:
Independently executed at this head from an isolated reviewer worktree. tests/test_issue_931_transport_bind_seam.py exit 0, 42 passed. Transport quarantine plus daemon guard exit 0, 33 passed. Decision-lock, session-state and isolation suites exit 0, 140 passed. #956 threat model exit 0, 17 passed. Capability plus workflow-inventory suites exit 0, 67 passed with 84 subtests. Full suite exit 1, 28 failed, 5843 passed, 6 skipped, 1047 subtests. Baseline run in my own reviewer-owned worktree at 9b80e75ca3: exit 1, 28 failed, 5801 passed, 6 skipped, 1047 subtests. Failing identifier sets compared with comm in both directions are identical, 28 on each side, so no regression and nothing newly fixed; the +42 passed is exactly the new module.

LAST_UPDATED_BY:
reviewer (sysadmin / prgs-reviewer / gitea-reviewer namespace)


Detailed findings

Base verified by merge-base as 9b80e75ca3f441fec2fb077a1b5f874faa0912e2, equal to live master. Changed-file set matches the declared nine exactly, no extra paths, two commits.

The seam itself is well built. Validation genuinely precedes any runtime-state write, the fail-closed paths are real, and the parity claim reproduces exactly. The blocker is not in the seam — it is in what the seam is wired to.

B1 (blocking) — registering streamable-http puts an unauthenticated listener one environment variable away

mcp.run now receives the bound value:

mcp.run(transport=mcp_daemon_guard.assert_transport_bound("tool service"))

streamable-http is in SUPPORTED_TRANSPORTS, so GITEA_MCP_TRANSPORT=streamable-http binds and that string reaches FastMCP.run, which dispatches to run_streamable_http_async and starts a uvicorn HTTP server over the entire tool surface.

Executed at this head, not inferred:

bound transport             : streamable-http
is_native_mcp_transport     : True
is_production_native        : True
assert_sanctioned_mutation  : PASSES -> mutations allowed
assert_production_mutation  : PASSES -> mutations allowed
auth_mint allowed           : True
value handed to mcp.run     : 'streamable-http'

FastMCP settings here: host=127.0.0.1, port=8000, auth=None. FASTMCP_HOST does not override the bind address in the installed version, so exposure is localhost. I am not claiming network exposure.

It remains a real regression. Role separation today rests on which process a call reaches through client-owned pipes; an HTTP port has no such ownership, so any local process reaching it gains full mutation authority and assess_transport_for_auth_mint() == True for that role. Two secondary consequences: no per-role port exists, so switching the five-daemon fleet would leave four unable to bind port 8000; and the duplicate-role-process and client-managed-provenance guards keep reporting healthy throughout.

B2 (blocking, same root cause) — the bound transport is recorded but governs nothing

Every consumer of bound_transport at this head is a reporting site: mcp_session_state.save_state, the assess_transport_for_auth_mint report field, native_runtime_status, mutation_provenance_fields. A search of the full production surface finds no guard, gate or refusal path that consults it. That is what makes B1 reachable.

Acceptance criteria, independently verified

AC Verdict Evidence
1. Config supplies identifier; unset yields the default MET Executed: unset -> bind OK -> stdio; serve gate and mutations pass
2. Unrecognized identifier fails closed at bind, named reason, no dispatch MET Executed: carrier-pigeon refused, "is not a registered MCP transport (#931)"; bound_transport stays None; serve gate refuses
3. No guard reads the default literal outside accessor and permitted set MET Tree-wide search: literal appears in production only at mcp_transport_config.py:40; all other hits are comment, docstring, CLI text or error prose
4. Decision-lock and audit carry the identifier MET save_state sets bound_transport; provenance spread reaches the decision-lock payload and live_mutations
5. Unbound execution still fails closed on the pre-existing reason code MET Executed: entrypoint-claimed-but-unbound -> mutations refused with unchanged #695 text
6. Existing behavior unchanged, no new expected-value edits MET Failing-ID sets identical to base; trust-class transport values untouched

Ordering confirmed by reading bind_native_mcp_transport: resolution and permitted-set validation both complete before any _NATIVE_RUNTIME field is written, so an invalid identifier never leaves partial runtime state. Rebind contract confirmed: identical rebind idempotent, conflicting rebind raises with the first value surviving, and it sits after the entrypoint/mode/path checks so provenance still fails first. Isolation: _NATIVE_RUNTIME stays process-local, session-state root stays pinned at bind, post-bind environment mutation cannot move the observed value. Secrets: nothing sensitive persisted or logged; native_runtime_status exposes the environment key name, never a value.

#956 anchors: five of 58 chosen at random resolve directly against this head, and generated_against_commit c1626081 appears verbatim in the document. The stamp commit touches no .py, so anchors valid there stay valid at head.

Test-validity findings (non-blocking)

  • test_no_default_transport_literal_outside_the_seam scans five hardcoded module names, so a future module reintroducing the literal would escape it. AC3 holds today by tree-wide search; this is a durability gap.
  • No test reaches the dispatch consequence of serving a non-default transport. That gap is why B1 survived a 42-test module.
  • bound_transport() returns "test" under the pytest runtime, so a durable record can carry a value outside SUPPORTED_TRANSPORTS.

Design judgments

streamable-http as the identifier: the name is right, the wiring is not. It matches MCP's current standard remote transport and avoids a mapping layer. The objection is that registration currently equals execution.

Rejecting sse: sound. SSE is the superseded remote transport and it fails closed cleanly through the same path as any unregistered name. It pre-commits a choice #938 nominally owns, but it is a one-line reversal and excluding a deprecated second remote path is the safer default. No change requested.

Reviewer independence disclosure

The Gitea identities are distinct and the runtime self-review gate passed, but this review was produced in the same assistant session that authored the PR. Findings were re-derived from the live tree and executed probes rather than from the author handoff, and the blocking finding is one the handoff did not raise. A fresh-session reviewer should still confirm B1 before the fix lands.

## Canonical PR State STATE: changes-requested WHO_IS_NEXT: author NEXT_ACTION: Split "registered transport identifier" from "servable transport" so a non-stdio identifier can be configured, pinned and recorded without `mcp.run` starting an unauthenticated listener, then push to `feat/issue-931-transport-neutral-bind-seam` and hand back for re-review. NEXT_PROMPT: ```text Role: AUTHOR Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools PR: #966 Issue: #931 Reviewed head: 2fb835a1aa4cb6260445f2ef3f10057aabd2fbe7 Base: 9b80e75ca3f441fec2fb077a1b5f874faa0912e2 Two changes requested, both one root cause. B1. `mcp.run(transport=assert_transport_bound(...))` plus `streamable-http` in SUPPORTED_TRANSPORTS means GITEA_MCP_TRANSPORT=streamable-http starts a uvicorn HTTP server (FastMCP.run_streamable_http_async) serving the whole tool surface with auth=None. #931 lists the listener as a non-goal and #938 owns it. B2. `bound_transport` is consumed only by reporting sites. No guard, gate or refusal path reads it, so nothing distinguishes a local session from a remote one. Fix direction: keep validating, pinning and recording streamable-http exactly as now, but add a servable/commissioned check on the serve path so a transport with no commissioned listener fails closed before mcp.run. One flip when #938 lands. Also address, non-blocking: - test_no_default_transport_literal_outside_the_seam scans five hardcoded module names; glob production modules instead. - Add a test covering what serving a non-default transport implies, not only that the identifier binds. - bound_transport() returns "test" under the pytest runtime, so a durable record can carry a value outside SUPPORTED_TRANSPORTS. Do not change: the resolution/validation ordering, the rebind contract, the #956 anchor updates, or the trust-class transport field. All verified correct. ``` WHAT_HAPPENED: Independent review at head 2fb835a1aa4cb6260445f2ef3f10057aabd2fbe7 in an isolated reviewer worktree. All six #931 acceptance criteria are met and independently reproduced. Two blocking findings sit outside the acceptance criteria, in the wiring between the seam and the server loop. WHY: Registering `streamable-http` while `mcp.run` is fed the bound value means one environment variable converts the server into an HTTP endpoint serving every tool with no authentication, no TLS and no per-request principal. Executed at this head: bind succeeds, is_native_mcp_transport True, assert_sanctioned_mutation_runtime and assert_production_mutation_runtime both pass, assess_transport_for_auth_mint allowed True, and mcp.run receives 'streamable-http'. FastMCP settings are host 127.0.0.1, port 8000, auth None, so exposure is local rather than network, which bounds severity but does not remove it: role separation currently rests on client-owned pipes, and an HTTP port has no such ownership. #931 names the listener as a non-goal and #938 owns authentication, principal binding, TLS and rate limiting, none of which exist yet. Separately, `bound_transport` is read only by reporting sites, so no decision anywhere depends on it. ISSUE: #931 HEAD_SHA: 2fb835a1aa4cb6260445f2ef3f10057aabd2fbe7 REVIEW_STATUS: request_changes by sysadmin (prgs-reviewer) at head 2fb835a1aa4cb6260445f2ef3f10057aabd2fbe7; author is jcwalker3, so reviewer and author are distinct accounts MERGE_READY: no BLOCKERS: B1 and B2 below. Unblock condition: the serve path refuses to hand a transport to mcp.run unless its listener is commissioned, so that configuring streamable-http cannot start an unauthenticated endpoint; and the bound transport is consulted by at least the serve decision rather than by reporting alone. Re-review at the new head after that change. VALIDATION: Independently executed at this head from an isolated reviewer worktree. tests/test_issue_931_transport_bind_seam.py exit 0, 42 passed. Transport quarantine plus daemon guard exit 0, 33 passed. Decision-lock, session-state and isolation suites exit 0, 140 passed. #956 threat model exit 0, 17 passed. Capability plus workflow-inventory suites exit 0, 67 passed with 84 subtests. Full suite exit 1, 28 failed, 5843 passed, 6 skipped, 1047 subtests. Baseline run in my own reviewer-owned worktree at 9b80e75ca3f441fec2fb077a1b5f874faa0912e2: exit 1, 28 failed, 5801 passed, 6 skipped, 1047 subtests. Failing identifier sets compared with comm in both directions are identical, 28 on each side, so no regression and nothing newly fixed; the +42 passed is exactly the new module. LAST_UPDATED_BY: reviewer (sysadmin / prgs-reviewer / gitea-reviewer namespace) --- ## Detailed findings Base verified by merge-base as `9b80e75ca3f441fec2fb077a1b5f874faa0912e2`, equal to live `master`. Changed-file set matches the declared nine exactly, no extra paths, two commits. The seam itself is well built. Validation genuinely precedes any runtime-state write, the fail-closed paths are real, and the parity claim reproduces exactly. The blocker is not in the seam — it is in what the seam is wired to. ### B1 (blocking) — registering `streamable-http` puts an unauthenticated listener one environment variable away `mcp.run` now receives the bound value: ```python mcp.run(transport=mcp_daemon_guard.assert_transport_bound("tool service")) ``` `streamable-http` is in `SUPPORTED_TRANSPORTS`, so `GITEA_MCP_TRANSPORT=streamable-http` binds and that string reaches `FastMCP.run`, which dispatches to `run_streamable_http_async` and starts a uvicorn HTTP server over the entire tool surface. Executed at this head, not inferred: ``` bound transport : streamable-http is_native_mcp_transport : True is_production_native : True assert_sanctioned_mutation : PASSES -> mutations allowed assert_production_mutation : PASSES -> mutations allowed auth_mint allowed : True value handed to mcp.run : 'streamable-http' ``` FastMCP settings here: `host=127.0.0.1`, `port=8000`, `auth=None`. `FASTMCP_HOST` does not override the bind address in the installed version, so exposure is localhost. I am not claiming network exposure. It remains a real regression. Role separation today rests on which process a call reaches through client-owned pipes; an HTTP port has no such ownership, so any local process reaching it gains full mutation authority and `assess_transport_for_auth_mint() == True` for that role. Two secondary consequences: no per-role port exists, so switching the five-daemon fleet would leave four unable to bind port 8000; and the duplicate-role-process and client-managed-provenance guards keep reporting healthy throughout. ### B2 (blocking, same root cause) — the bound transport is recorded but governs nothing Every consumer of `bound_transport` at this head is a reporting site: `mcp_session_state.save_state`, the `assess_transport_for_auth_mint` report field, `native_runtime_status`, `mutation_provenance_fields`. A search of the full production surface finds no guard, gate or refusal path that consults it. That is what makes B1 reachable. ### Acceptance criteria, independently verified | AC | Verdict | Evidence | |---|---|---| | 1. Config supplies identifier; unset yields the default | MET | Executed: unset -> bind OK -> stdio; serve gate and mutations pass | | 2. Unrecognized identifier fails closed at bind, named reason, no dispatch | MET | Executed: `carrier-pigeon` refused, "is not a registered MCP transport (#931)"; `bound_transport` stays None; serve gate refuses | | 3. No guard reads the default literal outside accessor and permitted set | MET | Tree-wide search: literal appears in production only at `mcp_transport_config.py:40`; all other hits are comment, docstring, CLI text or error prose | | 4. Decision-lock and audit carry the identifier | MET | `save_state` sets `bound_transport`; provenance spread reaches the decision-lock payload and `live_mutations` | | 5. Unbound execution still fails closed on the pre-existing reason code | MET | Executed: entrypoint-claimed-but-unbound -> mutations refused with unchanged #695 text | | 6. Existing behavior unchanged, no new expected-value edits | MET | Failing-ID sets identical to base; trust-class `transport` values untouched | Ordering confirmed by reading `bind_native_mcp_transport`: resolution and permitted-set validation both complete before any `_NATIVE_RUNTIME` field is written, so an invalid identifier never leaves partial runtime state. Rebind contract confirmed: identical rebind idempotent, conflicting rebind raises with the first value surviving, and it sits after the entrypoint/mode/path checks so provenance still fails first. Isolation: `_NATIVE_RUNTIME` stays process-local, session-state root stays pinned at bind, post-bind environment mutation cannot move the observed value. Secrets: nothing sensitive persisted or logged; `native_runtime_status` exposes the environment key name, never a value. #956 anchors: five of 58 chosen at random resolve directly against this head, and `generated_against_commit` `c1626081` appears verbatim in the document. The stamp commit touches no `.py`, so anchors valid there stay valid at head. ### Test-validity findings (non-blocking) - `test_no_default_transport_literal_outside_the_seam` scans five hardcoded module names, so a future module reintroducing the literal would escape it. AC3 holds today by tree-wide search; this is a durability gap. - No test reaches the dispatch consequence of serving a non-default transport. That gap is why B1 survived a 42-test module. - `bound_transport()` returns `"test"` under the pytest runtime, so a durable record can carry a value outside `SUPPORTED_TRANSPORTS`. ### Design judgments `streamable-http` as the identifier: the name is right, the wiring is not. It matches MCP's current standard remote transport and avoids a mapping layer. The objection is that registration currently equals execution. Rejecting `sse`: sound. SSE is the superseded remote transport and it fails closed cleanly through the same path as any unregistered name. It pre-commits a choice #938 nominally owns, but it is a one-line reversal and excluding a deprecated second remote path is the safer default. No change requested. ### Reviewer independence disclosure The Gitea identities are distinct and the runtime self-review gate passed, but this review was produced in the same assistant session that authored the PR. Findings were re-derived from the live tree and executed probes rather than from the author handoff, and the blocking finding is one the handoff did not raise. A fresh-session reviewer should still confirm B1 before the fix lands.
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #966
issue: #931
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 34563-cf9dbf7ecfc9
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr966-2fb835a1
phase: released
candidate_head: 2fb835a1aa
target_branch: master
target_branch_sha: 9b80e75ca3
last_activity: 2026-07-28T15:52:46Z
expires_at: 2026-07-28T16:02:46Z
blocker: manual-release

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #966 issue: #931 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 34563-cf9dbf7ecfc9 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr966-2fb835a1 phase: released candidate_head: 2fb835a1aa4cb6260445f2ef3f10057aabd2fbe7 target_branch: master target_branch_sha: 9b80e75ca3f441fec2fb077a1b5f874faa0912e2 last_activity: 2026-07-28T15:52:46Z expires_at: 2026-07-28T16:02:46Z blocker: manual-release
jcwalker3 added 1 commit 2026-07-28 11:15:03 -05:00
Addresses B1 and B2 from formal review 635 on PR #966. Both are one defect:
the seam conflated two questions that must stay separate.

  recognition:  "is this an identifier this system knows, and may it be
                 bound, pinned and recorded?"
  execution:    "is this entrypoint commissioned to actually serve on it?"

SUPPORTED_TRANSPORTS answered the first and was then used to answer the
second, so registering streamable-http was the same act as executing it.
With mcp.run fed the bound value, GITEA_MCP_TRANSPORT=streamable-http
reached FastMCP.run -> run_streamable_http_async and started a uvicorn
HTTP server over the whole tool surface with auth=None, no TLS and no
per-request principal. That endpoint is owned by #938 and is an explicit
non-goal of #931. B2 is why B1 was reachable: bound_transport was read
only by reporting sites, so no decision anywhere depended on it.

The fix adds an execution-authorization layer rather than removing the
identifier or mapping it to stdio, both of which would have destroyed the
seam #931 exists to create.

mcp_transport_config gains EXECUTABLE_TRANSPORTS, a strict subset of
SUPPORTED_TRANSPORTS containing only the local transport, plus
TRANSPORT_EXECUTION_OWNER recording that #938 commissions the remote
listener. assess_transport_execution returns a structured verdict:
transport, recognized, executable, allowed, blocker_kind, owner_issue,
reasons and exact_next_action.

mcp_daemon_guard gains assess_serve_authorization, which is the decision
that consumes bound_transport and is what stops it being reporting-only
metadata, and authorize_transport_execution, which enforces two ordered
boundaries: assert_transport_bound first, so an unbound runtime keeps its
pre-existing #695 failure and reason code; then execution authorization,
so a registered but uncommissioned transport is refused before any
listener exists and before any tool can dispatch. TransportExecutionError
subclasses UnsanctionedRuntimeError, so every existing fail-closed handler
still catches it while a caller that cares can distinguish the two cases.
native_runtime_status now reports executable_transports, serve_authorized
and the full serve_authorization verdict.

The entrypoint serves through authorize_transport_execution.

Net effect. stdio: unchanged, still binds and still reaches the runner.
streamable-http: still recognized, still validated, still pinned, still
recorded in decision-lock and audit provenance -- and refused at the serve
boundary with blocker transport_listener_not_commissioned naming #938.
Unregistered identifiers still fail earlier, at bind validation. Unbound
execution keeps the #695 contract. Rebind idempotence and conflict
rejection, session isolation, provenance and secret handling are untouched.

Tests: 63 in tests/test_issue_931_transport_bind_seam.py, up from 42. New
coverage proves stdio reaches the real runner; the remote identifier binds
and is recorded yet never reaches run(); no socket is bound during the
refusal, asserted with a socket.bind tripwire; the refusal names the
transport, the unmet requirement and #938; the decision genuinely consumes
bound_transport, proven by flipping only that value; no mutation is
authorized after denial; verdicts do not leak across runtimes; and the
refusal carries no credential material. Per review 635 the two file-list
scans are now globs over the production surface, and a new test asserts
there is exactly one mcp.run site and that it routes through the guard.

#956 anchors re-anchored for the two lines this change shifted,
mcp_daemon_guard.py 178->195 and 519->583. No other anchor moved.

Full suite from the branches/ worktree: 28 failed, 5864 passed, 6 skipped,
1047 subtests. Base 9b80e75c: 28 failed, 5801 passed, 6 skipped, 1047
subtests. Failing test IDs are identical sets in both directions; the +63
passed are this module.

No listener, socket, authentication, TLS or principal code is added. #938,
#957, #958 and #959 remain unimplemented.

Refs #931, #938. Addresses review 635.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
jcwalker3 added 1 commit 2026-07-28 11:17:52 -05:00
The execution-authorization gate shifted two cited lines in
mcp_daemon_guard.py, so the fixture and the document must name the commit
where the re-anchored citations actually resolve. Both still named
c1626081, where the two moved anchors no longer point at the claimed code.

Point both at a143cd06. This commit changes no .py file, so every anchor
that resolves at a143cd06 resolves here too. The historical note about
#930's drift between 7bf4f125 and aad5c8b4 stays as written.

Refs #931, #956. Addresses review 635.

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

Canonical PR State

STATE:
changes-addressed-awaiting-independent-re-review

WHO_IS_NEXT:
reviewer

NEXT_ACTION:
Re-review PR #966 at head 0104a76eea in a fresh session, confirming that a bound streamable-http transport is refused at the serve boundary before any listener exists, and that the serve decision genuinely consumes bound_transport.

NEXT_PROMPT:

Role: REVIEWER
Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools
PR: #966  Issue: #931
Head: 0104a76eea863eeb32ef42f245ddaa51ed2b197f
Base: 9b80e75ca3f441fec2fb077a1b5f874faa0912e2
Prior verdict: review 635 REQUEST_CHANGES at head 2fb835a1

1. Native MCP only on gitea-reviewer / prgs-reviewer. gitea_whoami,
   gitea_load_review_workflow, gitea_resolve_task_capability(review_pr).
   Note: load_review_workflow clears the in-session reviewer lease, so
   acquire the lease after loading, and re-resolve before each mutation.
2. Confirm B1 closed: set GITEA_MCP_TRANSPORT=streamable-http, bind through
   the canonical entrypoint, and verify authorize_transport_execution raises
   TransportExecutionError with blocker transport_listener_not_commissioned
   and owner_issue #938, and that no socket is bound.
3. Confirm B2 closed: assess_serve_authorization reads bound_transport;
   patch only bound_transport and verify the verdict flips.
4. Confirm stdio is unchanged: unset config still binds stdio and still
   reaches the runner.
5. Confirm unregistered identifiers still fail at bind, not at serve, and
   that unbound execution keeps the #695 failure text.
6. Confirm no listener, socket, auth, TLS or principal code was added, and
   that #938 / #957 / #958 / #959 remain unimplemented.
7. Full suite from a branches/ worktree; compare failing IDs against base
   9b80e75c. Compare IDs, never counts.

WHAT_HAPPENED:
Addressed both blockers from review 635 as one execution-boundary defect. Recognition and execution authorization are now distinct layers. mcp_transport_config gained EXECUTABLE_TRANSPORTS (a strict subset of SUPPORTED_TRANSPORTS holding only the local transport), TRANSPORT_EXECUTION_OWNER recording that #938 commissions the remote listener, and assess_transport_execution returning a structured verdict. mcp_daemon_guard gained assess_serve_authorization, which is the decision that consumes bound_transport, and authorize_transport_execution, which enforces bind presence first (preserving the #695 contract) and then execution authorization. TransportExecutionError subclasses UnsanctionedRuntimeError so existing fail-closed handlers still catch it. The entrypoint now serves through authorize_transport_execution.

WHY:
B1 and B2 shared one root cause: SUPPORTED_TRANSPORTS answered "is this identifier recognized" and was then reused to answer "may this process serve on it", so registering streamable-http was the same act as executing it. B2 is why B1 was reachable, since bound_transport was read only by reporting sites and no decision depended on it. Rather than removing the identifier or mapping it to stdio, either of which would destroy the seam #931 exists to create, the fix adds the missing execution-authorization layer. streamable-http therefore remains recognized, validated, pinned and recorded in decision-lock and audit provenance, while serving it is withheld until #938 commissions the listener with its authentication and principal boundary.

ISSUE:
#931

HEAD_SHA:
0104a76eea

REVIEW_STATUS:
review 635 REQUEST_CHANGES by sysadmin at head 2fb835a1 remains the latest formal verdict and is not dismissed; two new commits since then, so it now predates the current head and needs an independent re-review

MERGE_READY:
no

RELATED_PRS:
PR #966 (this PR, open, head 0104a76eea) implements issue #931 and carries formal review 635. Issue #938 owns the remote listener whose commissioning this change defers to, and #931 names that listener as a non-goal. Issue #930 (closed) supplied the coupling inventory rows T1-T9 that scope this work. Issue #956 owns the threat-model anchor fixture updated here.

BLOCKERS:
none from the author side; awaiting independent re-review of B1 and B2 at head 0104a76e

VALIDATION:
Focused module 63 passed, up from 42, exit 0. New coverage proves stdio still reaches the real runner; streamable-http binds and is recorded yet run() is never invoked; a socket.bind tripwire confirms no socket is bound during the refusal; the refusal names the transport, the unmet requirement and #938; the decision consumes bound_transport, proven by flipping only that value; no mutation is authorized after denial; verdicts do not leak across runtimes; and the refusal carries no credential material. Per review 635 both file-list scans are now globs over the production surface, plus a new test asserting exactly one mcp.run site routed through the guard. #956 anchors 17 passed. Transport quarantine and daemon guard 33 passed. Session-state and isolation 140 passed. Capability and workflow inventory 78 passed with 86 subtests. Full suite from the branches/ worktree exit 1, 28 failed, 5864 passed, 6 skipped, 1047 subtests. Base 9b80e75c from a clean baseline worktree exit 1, 28 failed, 5801 passed, 6 skipped, 1047 subtests. Failing identifiers compared with comm in both directions are identical, 28 on each side, so no regression and nothing newly fixed.

LAST_UPDATED_BY:
author (jcwalker3 / prgs-author / gitea-author namespace)


How B1 and B2 map to code and tests

Blocker Code Test evidence
B1 streamable-http could start a listener EXECUTABLE_TRANSPORTS, assess_transport_execution, authorize_transport_execution, entrypoint now calls the guard test_remote_transport_is_refused_at_the_serve_boundary, test_remote_transport_never_reaches_the_runner, test_no_http_listener_is_created_for_remote_transport, test_refusal_names_transport_and_unmet_requirement_and_owner
B2 bound_transport governed nothing assess_serve_authorization reads bound_transport and is the serve decision test_serve_decision_consumes_bound_transport, test_serve_verdict_reports_the_bound_transport, test_no_mutation_is_authorized_after_the_denial

Non-blocking items from 635 also addressed: both fixed-file-list scans are now globs with a self-check that the glob matched a real surface; a new test asserts there is exactly one mcp.run site and that it routes through the guard; and test_test_mode_runtime_is_not_servable covers the pytest-only record being outside both the recognized and executable sets.

Scope: two commits, a143cd06 (the gate) and 0104a76e (#956 restamp, no .py touched). Files changed since 2fb835a1: mcp_transport_config.py, mcp_daemon_guard.py, gitea_mcp_server.py, tests/test_issue_931_transport_bind_seam.py, docs/remote-mcp/threat-model-anchors.json, docs/remote-mcp/threat-model.md. No listener, socket, authentication, TLS or principal code added; #938, #957, #958 and #959 remain unimplemented. Two #956 anchors were re-anchored because this change shifted them (mcp_daemon_guard.py 178 to 195 and 519 to 583); no other anchor moved.

## Canonical PR State STATE: changes-addressed-awaiting-independent-re-review WHO_IS_NEXT: reviewer NEXT_ACTION: Re-review PR #966 at head 0104a76eea863eeb32ef42f245ddaa51ed2b197f in a fresh session, confirming that a bound streamable-http transport is refused at the serve boundary before any listener exists, and that the serve decision genuinely consumes bound_transport. NEXT_PROMPT: ```text Role: REVIEWER Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools PR: #966 Issue: #931 Head: 0104a76eea863eeb32ef42f245ddaa51ed2b197f Base: 9b80e75ca3f441fec2fb077a1b5f874faa0912e2 Prior verdict: review 635 REQUEST_CHANGES at head 2fb835a1 1. Native MCP only on gitea-reviewer / prgs-reviewer. gitea_whoami, gitea_load_review_workflow, gitea_resolve_task_capability(review_pr). Note: load_review_workflow clears the in-session reviewer lease, so acquire the lease after loading, and re-resolve before each mutation. 2. Confirm B1 closed: set GITEA_MCP_TRANSPORT=streamable-http, bind through the canonical entrypoint, and verify authorize_transport_execution raises TransportExecutionError with blocker transport_listener_not_commissioned and owner_issue #938, and that no socket is bound. 3. Confirm B2 closed: assess_serve_authorization reads bound_transport; patch only bound_transport and verify the verdict flips. 4. Confirm stdio is unchanged: unset config still binds stdio and still reaches the runner. 5. Confirm unregistered identifiers still fail at bind, not at serve, and that unbound execution keeps the #695 failure text. 6. Confirm no listener, socket, auth, TLS or principal code was added, and that #938 / #957 / #958 / #959 remain unimplemented. 7. Full suite from a branches/ worktree; compare failing IDs against base 9b80e75c. Compare IDs, never counts. ``` WHAT_HAPPENED: Addressed both blockers from review 635 as one execution-boundary defect. Recognition and execution authorization are now distinct layers. mcp_transport_config gained EXECUTABLE_TRANSPORTS (a strict subset of SUPPORTED_TRANSPORTS holding only the local transport), TRANSPORT_EXECUTION_OWNER recording that #938 commissions the remote listener, and assess_transport_execution returning a structured verdict. mcp_daemon_guard gained assess_serve_authorization, which is the decision that consumes bound_transport, and authorize_transport_execution, which enforces bind presence first (preserving the #695 contract) and then execution authorization. TransportExecutionError subclasses UnsanctionedRuntimeError so existing fail-closed handlers still catch it. The entrypoint now serves through authorize_transport_execution. WHY: B1 and B2 shared one root cause: SUPPORTED_TRANSPORTS answered "is this identifier recognized" and was then reused to answer "may this process serve on it", so registering streamable-http was the same act as executing it. B2 is why B1 was reachable, since bound_transport was read only by reporting sites and no decision depended on it. Rather than removing the identifier or mapping it to stdio, either of which would destroy the seam #931 exists to create, the fix adds the missing execution-authorization layer. streamable-http therefore remains recognized, validated, pinned and recorded in decision-lock and audit provenance, while serving it is withheld until #938 commissions the listener with its authentication and principal boundary. ISSUE: #931 HEAD_SHA: 0104a76eea863eeb32ef42f245ddaa51ed2b197f REVIEW_STATUS: review 635 REQUEST_CHANGES by sysadmin at head 2fb835a1 remains the latest formal verdict and is not dismissed; two new commits since then, so it now predates the current head and needs an independent re-review MERGE_READY: no RELATED_PRS: PR #966 (this PR, open, head 0104a76eea863eeb32ef42f245ddaa51ed2b197f) implements issue #931 and carries formal review 635. Issue #938 owns the remote listener whose commissioning this change defers to, and #931 names that listener as a non-goal. Issue #930 (closed) supplied the coupling inventory rows T1-T9 that scope this work. Issue #956 owns the threat-model anchor fixture updated here. BLOCKERS: none from the author side; awaiting independent re-review of B1 and B2 at head 0104a76e VALIDATION: Focused module 63 passed, up from 42, exit 0. New coverage proves stdio still reaches the real runner; streamable-http binds and is recorded yet run() is never invoked; a socket.bind tripwire confirms no socket is bound during the refusal; the refusal names the transport, the unmet requirement and #938; the decision consumes bound_transport, proven by flipping only that value; no mutation is authorized after denial; verdicts do not leak across runtimes; and the refusal carries no credential material. Per review 635 both file-list scans are now globs over the production surface, plus a new test asserting exactly one mcp.run site routed through the guard. #956 anchors 17 passed. Transport quarantine and daemon guard 33 passed. Session-state and isolation 140 passed. Capability and workflow inventory 78 passed with 86 subtests. Full suite from the branches/ worktree exit 1, 28 failed, 5864 passed, 6 skipped, 1047 subtests. Base 9b80e75c from a clean baseline worktree exit 1, 28 failed, 5801 passed, 6 skipped, 1047 subtests. Failing identifiers compared with comm in both directions are identical, 28 on each side, so no regression and nothing newly fixed. LAST_UPDATED_BY: author (jcwalker3 / prgs-author / gitea-author namespace) --- ### How B1 and B2 map to code and tests | Blocker | Code | Test evidence | |---|---|---| | B1 streamable-http could start a listener | `EXECUTABLE_TRANSPORTS`, `assess_transport_execution`, `authorize_transport_execution`, entrypoint now calls the guard | `test_remote_transport_is_refused_at_the_serve_boundary`, `test_remote_transport_never_reaches_the_runner`, `test_no_http_listener_is_created_for_remote_transport`, `test_refusal_names_transport_and_unmet_requirement_and_owner` | | B2 bound_transport governed nothing | `assess_serve_authorization` reads `bound_transport` and is the serve decision | `test_serve_decision_consumes_bound_transport`, `test_serve_verdict_reports_the_bound_transport`, `test_no_mutation_is_authorized_after_the_denial` | Non-blocking items from 635 also addressed: both fixed-file-list scans are now globs with a self-check that the glob matched a real surface; a new test asserts there is exactly one `mcp.run` site and that it routes through the guard; and `test_test_mode_runtime_is_not_servable` covers the pytest-only record being outside both the recognized and executable sets. Scope: two commits, `a143cd06` (the gate) and `0104a76e` (#956 restamp, no `.py` touched). Files changed since 2fb835a1: `mcp_transport_config.py`, `mcp_daemon_guard.py`, `gitea_mcp_server.py`, `tests/test_issue_931_transport_bind_seam.py`, `docs/remote-mcp/threat-model-anchors.json`, `docs/remote-mcp/threat-model.md`. No listener, socket, authentication, TLS or principal code added; #938, #957, #958 and #959 remain unimplemented. Two #956 anchors were re-anchored because this change shifted them (`mcp_daemon_guard.py` 178 to 195 and 519 to 583); no other anchor moved.
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #966
issue: #931
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 185-80210f3ae9f7
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr966-0104a76e
phase: claimed
candidate_head: 0104a76eea
target_branch: master
target_branch_sha: 9b80e75ca3
last_activity: 2026-07-28T16:35:00Z
expires_at: 2026-07-28T16:45:00Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #966 issue: #931 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 185-80210f3ae9f7 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr966-0104a76e phase: claimed candidate_head: 0104a76eea863eeb32ef42f245ddaa51ed2b197f target_branch: master target_branch_sha: 9b80e75ca3f441fec2fb077a1b5f874faa0912e2 last_activity: 2026-07-28T16:35:00Z expires_at: 2026-07-28T16:45:00Z blocker: none
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #966
issue: #931
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 185-80210f3ae9f7
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr966-0104a76e
phase: validating
candidate_head: 0104a76eea
target_branch: master
target_branch_sha: 9b80e75ca3
last_activity: 2026-07-28T16:45:00Z
expires_at: 2026-07-28T16:55:00Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #966 issue: #931 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 185-80210f3ae9f7 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr966-0104a76e phase: validating candidate_head: 0104a76eea863eeb32ef42f245ddaa51ed2b197f target_branch: master target_branch_sha: 9b80e75ca3f441fec2fb077a1b5f874faa0912e2 last_activity: 2026-07-28T16:45:00Z expires_at: 2026-07-28T16:55:00Z blocker: none
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #966
issue: #931
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 185-80210f3ae9f7
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr966-0104a76e
phase: claimed
candidate_head: 0104a76eea
target_branch: master
target_branch_sha: 9b80e75ca3
last_activity: 2026-07-28T16:58:18Z
expires_at: 2026-07-28T17:08:18Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #966 issue: #931 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 185-80210f3ae9f7 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr966-0104a76e phase: claimed candidate_head: 0104a76eea863eeb32ef42f245ddaa51ed2b197f target_branch: master target_branch_sha: 9b80e75ca3f441fec2fb077a1b5f874faa0912e2 last_activity: 2026-07-28T16:58:18Z expires_at: 2026-07-28T17:08:18Z blocker: none
sysadmin approved these changes 2026-07-28 11:59:46 -05:00
sysadmin left a comment
Owner

Canonical PR State

STATE:
approved

WHO_IS_NEXT:
merger

NEXT_ACTION:
Merge PR #966 into master at head 0104a76eea863eeb32ef42f245ddaa51ed2b197f under the merger role and profile, and let the merge transition close issue #931.

NEXT_PROMPT:

Role: MERGER
Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools
PR: #966  Issue: #931
Approved head: 0104a76eea863eeb32ef42f245ddaa51ed2b197f
Base: 9b80e75ca3f441fec2fb077a1b5f874faa0912e2

1. Native MCP only on gitea-merger / prgs-merger. gitea_whoami, then
   gitea_resolve_task_capability(merge_pr) immediately before each mutation,
   with nothing in between.
2. Re-read PR #966. Confirm the head is still 0104a76e and that the approval
   is recorded at that exact head. Any head advance voids the approval.
3. Confirm in_parity true, live_stale false, restart_required false,
   mutation_safe true. Never switch profiles on a gate refusal's advice.
4. Acquire the merger PR lease with gitea_acquire_merger_pr_lease, using
   acquire rather than adopt when no lease record exists.
5. Merge. Expect the merge to strand the control checkout: the fleet keeps
   running the previous code until master advances and the operator restarts.
6. Release the merger lease. The role after that is reconciler, for the
   source branch feat/issue-931-transport-neutral-bind-seam and the two
   reviewer worktrees named in this review.

Do not begin #932, #934, #936, #937, #938 or #957.

WHAT_HAPPENED:
Fresh-session independent re-review of PR #966 at head 0104a76eea, in an isolated reviewer worktree, against the six #931 acceptance criteria and against formal review 635. Both blocking findings from 635 are resolved. All six acceptance criteria still hold. No execution bypass, no listener reachability, no state leakage, no anchor defect and no scope violation remains. Three non-blocking test-durability observations are recorded below; none of them affects an acceptance criterion, and each is covered today by evidence I produced myself.

WHY:
Review 635 blocked because recognising a transport identifier and executing it were one act: mcp.run was handed whatever was bound, so GITEA_MCP_TRANSPORT=streamable-http would have started FastMCP's HTTP listener over the whole tool surface with no authentication and no per-request principal. The correction adds the missing authorization layer instead of removing the identifier or mapping it onto the local transport, either of which would have destroyed the seam #931 exists to create. mcp_transport_config.EXECUTABLE_TRANSPORTS is a strict subset of SUPPORTED_TRANSPORTS containing only the local transport; assess_transport_execution returns a structured verdict; mcp_daemon_guard.assess_serve_authorization is the decision that consumes bound_transport; and authorize_transport_execution enforces bind presence first, preserving the #695 contract, before execution authorization. I settled the outcome by launching the real production entrypoint rather than by reading code: with the remote transport configured the process exits 1 at gitea_mcp_server.py:24753, the refusal is raised while the argument to mcp.run is still being evaluated so mcp.run is never entered, TCP port 8000 never listens at any point, and no uvicorn banner appears. The remote identifier is still recognised, still pinned and still recorded in provenance, while serving it is withheld until #938 commissions the listener with its authentication and principal boundary.

ISSUE:
#931

HEAD_SHA:
0104a76eea

REVIEW_STATUS:
approve by sysadmin (prgs-reviewer) at head 0104a76eea863eeb32ef42f245ddaa51ed2b197f; the author is jcwalker3, so reviewer and author are distinct accounts; formal review 635 remains visible and undismissed and became stale only because the author pushed two further commits

MERGE_READY:
yes

BLOCKERS:
none

NATIVE_REVIEW_PROOF:
Every Gitea, identity, lease, capability and review fact in this review was obtained through the gitea-reviewer namespace on profile prgs-reviewer as sysadmin against gitea.prgs.cc for Scaled-Tech-Consulting/Gitea-Tools. Calls used: mcp_check_workflow_skill_preflight (workflow proof present, blocked false); gitea_whoami (identity_match true, role_kind reviewer); gitea_get_runtime_context; gitea_assess_master_parity (in_parity true, live_stale false, restart_required false, mutation_safe true, live master 9b80e75c); gitea_load_review_workflow (workflow hash 263d0a6cb8a6, boundary clean); gitea_resolve_task_capability(review_pr) immediately before each mutation; gitea_view_issue 931; gitea_view_pr 966; gitea_get_pr_review_feedback; gitea_list_issue_comments on 931 and 966; gitea_assess_reviewer_pr_lease; gitea_list_workflow_leases; gitea_workflow_dashboard; gitea_acquire_reviewer_pr_lease (comments 18288 and 18292); gitea_heartbeat_reviewer_pr_lease (comment 18290); gitea_diagnose_review_decision_lock (mark_final_allowed true, classification stale_superseded_head, recovery mode same_pr_new_head); gitea_mark_final_review_decision; gitea_dry_run_pr_review; gitea_submit_pr_review. No raw API call, no CLI substitute, no browser and no fallback transport was used for any Gitea state. Shell use was confined to the isolated reviewer worktree, source inspection and test execution.

VALIDATION:
Executed at head 0104a76e from the isolated reviewer worktree branches/review-pr966-0104a76e, with the base run in my own worktree branches/review-pr966-base-9b80e75c so the author's branches/baseline-931-9b80e75c stayed untouched. Focused #931 module: exit 0, 63 passed. Transport quarantine plus daemon guard: exit 0, 33 passed. Decision-lock, session-state and isolation suites: exit 0, 140 passed. #956 threat model: exit 0, 17 passed. Capability plus workflow-inventory suites: exit 0, 87 passed with 86 subtests. Full suite at head: exit 1, 28 failed, 5864 passed, 6 skipped, 1047 subtests. Full suite at base 9b80e75c: exit 1, 28 failed, 5801 passed, 6 skipped, 1047 subtests. Failing identifiers were extracted from both runs and compared with comm in both directions and with diff: the sets are identical, 28 on each side, both comm directions empty, so no failure was added and none was fixed, and the +63 passed is exactly this PR's new module. An anchor check I wrote against git show confirms all 58 declared anchors resolve at the commit the fixture names, a143cd06, and all 58 still resolve at head 0104a76e. Reviewer-owned probes: 19 of 19 checks passed, covering serve-decision consumption of bound_transport, a socket tripwire demonstrated live before being relied on, the pre-existing unbound failure text, the rebind contract, process-local pinning and credential hygiene.

LAST_UPDATED_BY:
reviewer (sysadmin / prgs-reviewer / gitea-reviewer namespace)


Scope and base

Base confirmed by merge-base as 9b80e75ca3f441fec2fb077a1b5f874faa0912e2, equal to live master. Four commits base to head: c162608 (the seam), 2fb835a (first anchor stamp), a143cd0 (the execution gate), 0104a76 (anchor restamp, docs only). The full PR touches nine files; the correction since the reviewed head touches exactly the six expected paths: mcp_transport_config.py, mcp_daemon_guard.py, gitea_mcp_server.py, tests/test_issue_931_transport_bind_seam.py, docs/remote-mcp/threat-model-anchors.json, docs/remote-mcp/threat-model.md.

No listener, authorization system, TLS, packaging or principal-resolution code entered the PR. The only new module-level names added across all production files are os, typing, __future__ annotations and mcp_transport_config. The single occurrence of the word socket anywhere in the added lines is inside the test module's own tripwire. uvicorn appears only in the pre-existing webui/ application, which this PR does not touch, and in one test docstring. EXECUTABLE_TRANSPORTS still excludes the remote transport, so #938 remains uncommissioned, and no code maps the remote identifier onto the local one.

B1 resolved — execution is refused at the serve boundary

gitea_mcp_server.py:24753 is the only mcp.run( site in the entire production surface, and it routes through the guard. I searched for every alternative runner as well: run_streamable_http_async, run_stdio_async, run_sse_async, run_async and direct uvicorn use. None exists outside webui/. Every other .run( hit in the tree is subprocess.run for git.

mcp_server.py is the process the fleet actually runs, and it has no runner of its own: it compiles and executes gitea_mcp_server.py into its own __main__ namespace, so the real production path does reach line 24753.

Executed against the real entrypoint, three configurations:

GITEA_MCP_TRANSPORT=streamable-http -> exit 1, port 8000 never listening
  TransportExecutionError at gitea_mcp_server.py:24753
  [transport_listener_not_commissioned] transport 'streamable-http' is
  registered and was bound and recorded, but this entrypoint is not
  commissioned to serve it (#931) ... that endpoint is owned by #938.
GITEA_MCP_TRANSPORT=carrier-pigeon   -> exit 1 at gitea_mcp_server.py:24728
  UnsanctionedRuntimeError from bind_native_mcp_transport: not a registered
  MCP transport (#931). Refused at bind, not at serve.
GITEA_MCP_TRANSPORT unset           -> exit 0, served the local transport,
  reached the runner, no port bound.

Three properties matter here and all three hold. The refusal is raised during evaluation of the argument to mcp.run, so the call is never entered and no listener code runs. An unregistered identifier still fails at the earlier bind boundary, which keeps the two failure classes distinct. The local transport still reaches the real runner unchanged.

TransportExecutionError cannot be swallowed on this path. It is raised at exactly one call site, and that site sits at module level in the __main__ block with no enclosing try; the only try in that block wraps _assess_stale_active_binding. The five production handlers that name UnsanctionedRuntimeError all live in tool functions and in gitea_auth, which are reachable only after service begins — and service never begins. The observed exit 1 with the traceback escaping to top level confirms this rather than assuming it.

B2 resolved — the bound transport now governs the decision

assess_serve_authorization calls bound_transport() and is what authorize_transport_execution consults. I confirmed consumption four ways, with reviewer-owned patches:

  • Patching only bound_transport to the remote identifier flips the verdict from allowed to denied with blocker transport_listener_not_commissioned and owner #938; restoring the accessor restores the verdict.
  • Patching only bound_transport to none yields blocker transport_not_bound.
  • Mutating the pinned runtime record flips the serve decision, which shows the decision follows the pinned value rather than a constant.
  • Changing GITEA_MCP_TRANSPORT after the bind does not move what the decision observes, so configuration is read once, as #695 AC2 already requires for the session-state root.

Because the production behaviour differs by configuration in the real-entrypoint runs above, the decision demonstrably cannot be keyed on a constant or an unconnected value.

A socket tripwire I wrote myself, and demonstrated live by tripping it with a deliberate bind before relying on it, records zero bind attempts anywhere in the refusal path.

Acceptance criteria at this head

AC Verdict Evidence
1. Configuration supplies the identifier; unset yields the default MET Real entrypoint with the variable unset: exit 0, local transport served
2. Unrecognised identifier fails closed at bind with a named reason, nothing dispatches MET carrier-pigeon refused at line 24728; bound_transport stays none
3. No guard reads the default literal outside the accessor and the permitted set MET Tree-wide scan: the quoted literal occurs in production only in mcp_transport_config.py, at the constant and in its module docstring; every other hit is a test
4. Decision-lock and audit records carry the identifier MET mutation_provenance_fields()["bound_transport"] is the bound value while the trust-class transport field keeps native_mcp
5. Execution with no bind still fails closed on the pre-existing reason code MET Raises UnsanctionedRuntimeError, not TransportExecutionError, with the unchanged "No MCP transport is bound" text
6. Existing behaviour unchanged, no new expected-value edits MET Failing-identifier sets identical to base in both directions; trust-class vocabulary untouched

Isolation and hygiene: the runtime record is pinned to the process id, and a record carrying another process id is not observable as bound, so a verdict cannot cross runtimes. No raw token value appears in the runtime status, the serve verdict or the provenance payload, and the verdict carries no credential-shaped keys. The refusal names the transport, the unmet requirement, the owning issue #938 and a next action.

#956 anchors

Exactly two anchors moved, both in mcp_daemon_guard.py (178 to 195 for bind_native_mcp_transport, 519 to 583 for assert_keychain_access_allowed), and generated_against_commit advanced from c1626081 to a143cd06. The generation-commit semantics are correct: a143cd06 is the commit that shifted the lines, and the final commit 0104a76 touches only the two documents, so anchors resolving at the declared commit still resolve at head. Checked exhaustively rather than by sampling: 58 of 58 resolve at a143cd06 and 58 of 58 at 0104a76e.

Non-blocking observations

  1. test_no_mutation_is_authorized_after_the_denial does not establish what its name and docstring claim. It asserts only that the serve verdict stays unauthorized and that the refusal repeats; it makes no assertion about mutation authorization. I checked the actual state: after the denial assert_sanctioned_mutation_runtime still passes, because the bind deliberately remains valid so provenance can record the transport. That is unreachable in production, since the process terminates at line 24753 and nothing catches the error, so it is not a defect in the gate — but the test name overstates its assertions and would mislead a future reader.
  2. test_every_run_call_in_production_goes_through_the_guard globs *.py at the repository root only and keys on the literal text mcp.run(. A serve site added inside a subpackage, or a differently named runner such as run_streamable_http_async, would escape it. This is the correct direction of travel from the fixed five-module list that review 635 flagged, and I confirmed by independent search that no such site exists today; it remains a durability gap rather than a present defect.
  3. Entrypoint coverage in the test module is still established by reading source text and matching strings. The behavioural proof that the real production path refuses comes from my out-of-process run of the real entrypoint, not from the suite. Worth a future test that executes the entrypoint.

None of these blocks the merge, and none of them is a suitable reason to hold #931.

Documentation staleness, for the merger

The PR description still describes the previously reviewed head: it shows assert_transport_bound in the entrypoint snippet, reports 42 passed, and lists the older diffstat and SHAs. The current state is carried by the author's canonical comment 18279 instead. Editing a PR description is not available on the author path, so publishing the correction as a canonical comment was the available route and this is not a defect in the change. The merge commit should be understood against comment 18279 and this review rather than against the description body.

Design judgment

Accepting streamable-http before #938 exists is now the right call, because recognition and execution are separate layers and only the local transport is executable. Registering the identifier is what makes the seam pluggable, which is the whole point of #931, and the eventual commissioning is a single addition to EXECUTABLE_TRANSPORTS alongside the authentication boundary #938 must bring. Excluding sse remains sound: it fails closed through the same path as any unregistered name.

Reviewer independence

Reviewer identity is sysadmin on profile prgs-reviewer; the author is jcwalker3. The two Gitea accounts are distinct and the runtime self-review gate passed. This is a different session from the one that authored the change and from the one that submitted review 635: those leases were held by session ids beginning 34563, whereas this session is 185-80210f3ae9f7. Every finding here was derived from the live tree at the exact head and from probes I wrote and executed, not from the author's handoff, and the author's reported figures were reproduced rather than accepted.

## Canonical PR State STATE: approved WHO_IS_NEXT: merger NEXT_ACTION: Merge PR #966 into `master` at head `0104a76eea863eeb32ef42f245ddaa51ed2b197f` under the merger role and profile, and let the merge transition close issue #931. NEXT_PROMPT: ```text Role: MERGER Repo: prgs / Scaled-Tech-Consulting / Gitea-Tools PR: #966 Issue: #931 Approved head: 0104a76eea863eeb32ef42f245ddaa51ed2b197f Base: 9b80e75ca3f441fec2fb077a1b5f874faa0912e2 1. Native MCP only on gitea-merger / prgs-merger. gitea_whoami, then gitea_resolve_task_capability(merge_pr) immediately before each mutation, with nothing in between. 2. Re-read PR #966. Confirm the head is still 0104a76e and that the approval is recorded at that exact head. Any head advance voids the approval. 3. Confirm in_parity true, live_stale false, restart_required false, mutation_safe true. Never switch profiles on a gate refusal's advice. 4. Acquire the merger PR lease with gitea_acquire_merger_pr_lease, using acquire rather than adopt when no lease record exists. 5. Merge. Expect the merge to strand the control checkout: the fleet keeps running the previous code until master advances and the operator restarts. 6. Release the merger lease. The role after that is reconciler, for the source branch feat/issue-931-transport-neutral-bind-seam and the two reviewer worktrees named in this review. Do not begin #932, #934, #936, #937, #938 or #957. ``` WHAT_HAPPENED: Fresh-session independent re-review of PR #966 at head 0104a76eea863eeb32ef42f245ddaa51ed2b197f, in an isolated reviewer worktree, against the six #931 acceptance criteria and against formal review 635. Both blocking findings from 635 are resolved. All six acceptance criteria still hold. No execution bypass, no listener reachability, no state leakage, no anchor defect and no scope violation remains. Three non-blocking test-durability observations are recorded below; none of them affects an acceptance criterion, and each is covered today by evidence I produced myself. WHY: Review 635 blocked because recognising a transport identifier and executing it were one act: `mcp.run` was handed whatever was bound, so `GITEA_MCP_TRANSPORT=streamable-http` would have started FastMCP's HTTP listener over the whole tool surface with no authentication and no per-request principal. The correction adds the missing authorization layer instead of removing the identifier or mapping it onto the local transport, either of which would have destroyed the seam #931 exists to create. `mcp_transport_config.EXECUTABLE_TRANSPORTS` is a strict subset of `SUPPORTED_TRANSPORTS` containing only the local transport; `assess_transport_execution` returns a structured verdict; `mcp_daemon_guard.assess_serve_authorization` is the decision that consumes `bound_transport`; and `authorize_transport_execution` enforces bind presence first, preserving the #695 contract, before execution authorization. I settled the outcome by launching the real production entrypoint rather than by reading code: with the remote transport configured the process exits 1 at `gitea_mcp_server.py:24753`, the refusal is raised while the argument to `mcp.run` is still being evaluated so `mcp.run` is never entered, TCP port 8000 never listens at any point, and no uvicorn banner appears. The remote identifier is still recognised, still pinned and still recorded in provenance, while serving it is withheld until #938 commissions the listener with its authentication and principal boundary. ISSUE: #931 HEAD_SHA: 0104a76eea863eeb32ef42f245ddaa51ed2b197f REVIEW_STATUS: approve by sysadmin (prgs-reviewer) at head 0104a76eea863eeb32ef42f245ddaa51ed2b197f; the author is jcwalker3, so reviewer and author are distinct accounts; formal review 635 remains visible and undismissed and became stale only because the author pushed two further commits MERGE_READY: yes BLOCKERS: none NATIVE_REVIEW_PROOF: Every Gitea, identity, lease, capability and review fact in this review was obtained through the gitea-reviewer namespace on profile prgs-reviewer as sysadmin against gitea.prgs.cc for Scaled-Tech-Consulting/Gitea-Tools. Calls used: mcp_check_workflow_skill_preflight (workflow proof present, blocked false); gitea_whoami (identity_match true, role_kind reviewer); gitea_get_runtime_context; gitea_assess_master_parity (in_parity true, live_stale false, restart_required false, mutation_safe true, live master 9b80e75c); gitea_load_review_workflow (workflow hash 263d0a6cb8a6, boundary clean); gitea_resolve_task_capability(review_pr) immediately before each mutation; gitea_view_issue 931; gitea_view_pr 966; gitea_get_pr_review_feedback; gitea_list_issue_comments on 931 and 966; gitea_assess_reviewer_pr_lease; gitea_list_workflow_leases; gitea_workflow_dashboard; gitea_acquire_reviewer_pr_lease (comments 18288 and 18292); gitea_heartbeat_reviewer_pr_lease (comment 18290); gitea_diagnose_review_decision_lock (mark_final_allowed true, classification stale_superseded_head, recovery mode same_pr_new_head); gitea_mark_final_review_decision; gitea_dry_run_pr_review; gitea_submit_pr_review. No raw API call, no CLI substitute, no browser and no fallback transport was used for any Gitea state. Shell use was confined to the isolated reviewer worktree, source inspection and test execution. VALIDATION: Executed at head 0104a76e from the isolated reviewer worktree branches/review-pr966-0104a76e, with the base run in my own worktree branches/review-pr966-base-9b80e75c so the author's branches/baseline-931-9b80e75c stayed untouched. Focused #931 module: exit 0, 63 passed. Transport quarantine plus daemon guard: exit 0, 33 passed. Decision-lock, session-state and isolation suites: exit 0, 140 passed. #956 threat model: exit 0, 17 passed. Capability plus workflow-inventory suites: exit 0, 87 passed with 86 subtests. Full suite at head: exit 1, 28 failed, 5864 passed, 6 skipped, 1047 subtests. Full suite at base 9b80e75c: exit 1, 28 failed, 5801 passed, 6 skipped, 1047 subtests. Failing identifiers were extracted from both runs and compared with comm in both directions and with diff: the sets are identical, 28 on each side, both comm directions empty, so no failure was added and none was fixed, and the +63 passed is exactly this PR's new module. An anchor check I wrote against git show confirms all 58 declared anchors resolve at the commit the fixture names, a143cd06, and all 58 still resolve at head 0104a76e. Reviewer-owned probes: 19 of 19 checks passed, covering serve-decision consumption of bound_transport, a socket tripwire demonstrated live before being relied on, the pre-existing unbound failure text, the rebind contract, process-local pinning and credential hygiene. LAST_UPDATED_BY: reviewer (sysadmin / prgs-reviewer / gitea-reviewer namespace) --- ## Scope and base Base confirmed by merge-base as `9b80e75ca3f441fec2fb077a1b5f874faa0912e2`, equal to live `master`. Four commits base to head: `c162608` (the seam), `2fb835a` (first anchor stamp), `a143cd0` (the execution gate), `0104a76` (anchor restamp, docs only). The full PR touches nine files; the correction since the reviewed head touches exactly the six expected paths: `mcp_transport_config.py`, `mcp_daemon_guard.py`, `gitea_mcp_server.py`, `tests/test_issue_931_transport_bind_seam.py`, `docs/remote-mcp/threat-model-anchors.json`, `docs/remote-mcp/threat-model.md`. No listener, authorization system, TLS, packaging or principal-resolution code entered the PR. The only new module-level names added across all production files are `os`, `typing`, `__future__` annotations and `mcp_transport_config`. The single occurrence of the word socket anywhere in the added lines is inside the test module's own tripwire. `uvicorn` appears only in the pre-existing `webui/` application, which this PR does not touch, and in one test docstring. `EXECUTABLE_TRANSPORTS` still excludes the remote transport, so #938 remains uncommissioned, and no code maps the remote identifier onto the local one. ## B1 resolved — execution is refused at the serve boundary `gitea_mcp_server.py:24753` is the only `mcp.run(` site in the entire production surface, and it routes through the guard. I searched for every alternative runner as well: `run_streamable_http_async`, `run_stdio_async`, `run_sse_async`, `run_async` and direct uvicorn use. None exists outside `webui/`. Every other `.run(` hit in the tree is `subprocess.run` for git. `mcp_server.py` is the process the fleet actually runs, and it has no runner of its own: it compiles and executes `gitea_mcp_server.py` into its own `__main__` namespace, so the real production path does reach line 24753. Executed against the real entrypoint, three configurations: ``` GITEA_MCP_TRANSPORT=streamable-http -> exit 1, port 8000 never listening TransportExecutionError at gitea_mcp_server.py:24753 [transport_listener_not_commissioned] transport 'streamable-http' is registered and was bound and recorded, but this entrypoint is not commissioned to serve it (#931) ... that endpoint is owned by #938. GITEA_MCP_TRANSPORT=carrier-pigeon -> exit 1 at gitea_mcp_server.py:24728 UnsanctionedRuntimeError from bind_native_mcp_transport: not a registered MCP transport (#931). Refused at bind, not at serve. GITEA_MCP_TRANSPORT unset -> exit 0, served the local transport, reached the runner, no port bound. ``` Three properties matter here and all three hold. The refusal is raised during evaluation of the argument to `mcp.run`, so the call is never entered and no listener code runs. An unregistered identifier still fails at the earlier bind boundary, which keeps the two failure classes distinct. The local transport still reaches the real runner unchanged. `TransportExecutionError` cannot be swallowed on this path. It is raised at exactly one call site, and that site sits at module level in the `__main__` block with no enclosing try; the only try in that block wraps `_assess_stale_active_binding`. The five production handlers that name `UnsanctionedRuntimeError` all live in tool functions and in `gitea_auth`, which are reachable only after service begins — and service never begins. The observed exit 1 with the traceback escaping to top level confirms this rather than assuming it. ## B2 resolved — the bound transport now governs the decision `assess_serve_authorization` calls `bound_transport()` and is what `authorize_transport_execution` consults. I confirmed consumption four ways, with reviewer-owned patches: - Patching only `bound_transport` to the remote identifier flips the verdict from allowed to denied with blocker `transport_listener_not_commissioned` and owner `#938`; restoring the accessor restores the verdict. - Patching only `bound_transport` to none yields blocker `transport_not_bound`. - Mutating the pinned runtime record flips the serve decision, which shows the decision follows the pinned value rather than a constant. - Changing `GITEA_MCP_TRANSPORT` after the bind does not move what the decision observes, so configuration is read once, as #695 AC2 already requires for the session-state root. Because the production behaviour differs by configuration in the real-entrypoint runs above, the decision demonstrably cannot be keyed on a constant or an unconnected value. A socket tripwire I wrote myself, and demonstrated live by tripping it with a deliberate bind before relying on it, records zero bind attempts anywhere in the refusal path. ## Acceptance criteria at this head | AC | Verdict | Evidence | |---|---|---| | 1. Configuration supplies the identifier; unset yields the default | MET | Real entrypoint with the variable unset: exit 0, local transport served | | 2. Unrecognised identifier fails closed at bind with a named reason, nothing dispatches | MET | `carrier-pigeon` refused at line 24728; `bound_transport` stays none | | 3. No guard reads the default literal outside the accessor and the permitted set | MET | Tree-wide scan: the quoted literal occurs in production only in `mcp_transport_config.py`, at the constant and in its module docstring; every other hit is a test | | 4. Decision-lock and audit records carry the identifier | MET | `mutation_provenance_fields()["bound_transport"]` is the bound value while the trust-class `transport` field keeps `native_mcp` | | 5. Execution with no bind still fails closed on the pre-existing reason code | MET | Raises `UnsanctionedRuntimeError`, not `TransportExecutionError`, with the unchanged "No MCP transport is bound" text | | 6. Existing behaviour unchanged, no new expected-value edits | MET | Failing-identifier sets identical to base in both directions; trust-class vocabulary untouched | Isolation and hygiene: the runtime record is pinned to the process id, and a record carrying another process id is not observable as bound, so a verdict cannot cross runtimes. No raw token value appears in the runtime status, the serve verdict or the provenance payload, and the verdict carries no credential-shaped keys. The refusal names the transport, the unmet requirement, the owning issue #938 and a next action. ## #956 anchors Exactly two anchors moved, both in `mcp_daemon_guard.py` (178 to 195 for `bind_native_mcp_transport`, 519 to 583 for `assert_keychain_access_allowed`), and `generated_against_commit` advanced from `c1626081` to `a143cd06`. The generation-commit semantics are correct: `a143cd06` is the commit that shifted the lines, and the final commit `0104a76` touches only the two documents, so anchors resolving at the declared commit still resolve at head. Checked exhaustively rather than by sampling: 58 of 58 resolve at `a143cd06` and 58 of 58 at `0104a76e`. ## Non-blocking observations 1. `test_no_mutation_is_authorized_after_the_denial` does not establish what its name and docstring claim. It asserts only that the serve verdict stays unauthorized and that the refusal repeats; it makes no assertion about mutation authorization. I checked the actual state: after the denial `assert_sanctioned_mutation_runtime` still passes, because the bind deliberately remains valid so provenance can record the transport. That is unreachable in production, since the process terminates at line 24753 and nothing catches the error, so it is not a defect in the gate — but the test name overstates its assertions and would mislead a future reader. 2. `test_every_run_call_in_production_goes_through_the_guard` globs `*.py` at the repository root only and keys on the literal text `mcp.run(`. A serve site added inside a subpackage, or a differently named runner such as `run_streamable_http_async`, would escape it. This is the correct direction of travel from the fixed five-module list that review 635 flagged, and I confirmed by independent search that no such site exists today; it remains a durability gap rather than a present defect. 3. Entrypoint coverage in the test module is still established by reading source text and matching strings. The behavioural proof that the real production path refuses comes from my out-of-process run of the real entrypoint, not from the suite. Worth a future test that executes the entrypoint. None of these blocks the merge, and none of them is a suitable reason to hold #931. ## Documentation staleness, for the merger The PR description still describes the previously reviewed head: it shows `assert_transport_bound` in the entrypoint snippet, reports 42 passed, and lists the older diffstat and SHAs. The current state is carried by the author's canonical comment 18279 instead. Editing a PR description is not available on the author path, so publishing the correction as a canonical comment was the available route and this is not a defect in the change. The merge commit should be understood against comment 18279 and this review rather than against the description body. ## Design judgment Accepting `streamable-http` before #938 exists is now the right call, because recognition and execution are separate layers and only the local transport is executable. Registering the identifier is what makes the seam pluggable, which is the whole point of #931, and the eventual commissioning is a single addition to `EXECUTABLE_TRANSPORTS` alongside the authentication boundary #938 must bring. Excluding `sse` remains sound: it fails closed through the same path as any unregistered name. ## Reviewer independence Reviewer identity is sysadmin on profile prgs-reviewer; the author is jcwalker3. The two Gitea accounts are distinct and the runtime self-review gate passed. This is a different session from the one that authored the change and from the one that submitted review 635: those leases were held by session ids beginning 34563, whereas this session is 185-80210f3ae9f7. Every finding here was derived from the live tree at the exact head and from probes I wrote and executed, not from the author's handoff, and the author's reported figures were reproduced rather than accepted.
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #966
issue: #931
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 185-80210f3ae9f7
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr966-0104a76e
phase: released
candidate_head: 0104a76eea
target_branch: master
target_branch_sha: 9b80e75ca3
last_activity: 2026-07-28T17:00:15Z
expires_at: 2026-07-28T17:10:15Z
blocker: manual-release

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #966 issue: #931 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 185-80210f3ae9f7 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr966-0104a76e phase: released candidate_head: 0104a76eea863eeb32ef42f245ddaa51ed2b197f target_branch: master target_branch_sha: 9b80e75ca3f441fec2fb077a1b5f874faa0912e2 last_activity: 2026-07-28T17:00:15Z expires_at: 2026-07-28T17:10:15Z blocker: manual-release
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #966
issue: #931
reviewer_identity: sysadmin
profile: prgs-merger
session_id: prgs-merger-94007-pr966
worktree: /Users/jasonwalker/Development/Gitea-Tools
phase: claimed
candidate_head: 0104a76eea
target_branch: master
target_branch_sha: 9b80e75ca3
last_activity: 2026-07-28T18:35:56Z
expires_at: 2026-07-28T18:45:56Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #966 issue: #931 reviewer_identity: sysadmin profile: prgs-merger session_id: prgs-merger-94007-pr966 worktree: /Users/jasonwalker/Development/Gitea-Tools phase: claimed candidate_head: 0104a76eea863eeb32ef42f245ddaa51ed2b197f target_branch: master target_branch_sha: 9b80e75ca3f441fec2fb077a1b5f874faa0912e2 last_activity: 2026-07-28T18:35:56Z expires_at: 2026-07-28T18:45:56Z blocker: none
sysadmin merged commit 17ba1ff035 into master 2026-07-28 13:36:49 -05:00
Owner

Stale #332 review-decision lock cleanup (#594)

Status: APPLIED

Manual deletion of session-state files is not the workflow.
This path only clears a lock when the referenced PR is merged/closed.

## Stale #332 review-decision lock cleanup (#594) Status: **APPLIED** - actor: `sysadmin` - profile: `prgs-merger` - timestamp: `2026-07-28T18:36:55.423069+00:00` - last terminal: `approve` on PR #966 - PR state: `closed` (merged=True) - merge_commit_sha: `17ba1ff035ee3154a0f2dcacbefe107457cca33f` - prior live_mutations_count: `2` - prior profile_identity: `prgs-reviewer` Manual deletion of session-state files is **not** the workflow. This path only clears a lock when the referenced PR is merged/closed.
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#966