feat(transport): add transport-neutral MCP bind seam (Closes #931) #966
Labels
Clear labels
allocator
anti-stomp
architecture
bug
chore
codex
concurrency
contamination
control-plane
dashboard
database
design
documentation
enhancement
gitea
glitchtip
important
incident
incident-bridge
integration
jenkins
labels
leases
mcp
mcp-health
mcp-menu
multi-project
mutating
nice-to-have
observability
portability
preflight
protected-branch
queue
read-only
reconnect
recovery
refactor
release
reliability
resumable-review
reviewer
roadmap
safety
security
self-hosted
sentry
stale-runtime
status:blocked
status:in-progress
status:pr-open
status:ready
terminal-lock
testing
tracker
type:bug
type:feature
type:feature
type:guardrail
visibility
workflow
workflow-hardening
workflow-hardening
Controller-owned work allocator
Prevent concurrent LLM session stomping
Architecture / structural design
OpenAI Codex client / workflow session surface
Concurrent session safety
Workflow or session contamination incident
MCP control-plane coordination and allocation authority
MCP operational dashboard/queue view
Internal coordination storage (SQLite/Postgres)
Design / investigation, no implementation
Docs / runbooks
New feature or improvement
Gitea MCP workflow
GlitchTip integration
Operational or process incident requiring durable audit trail
Sentry-to-Gitea incident bridging
Integration testing
Jenkins integration
Label taxonomy management
Lease adopt/release/expire lifecycle
MCP server / tooling
MCP namespace and runtime health
MCP menu surface
Work spanning multiple monitoring projects or Gitea repos
Mutating action; requires gating
Observability, metrics, traces, error reporting
Cross-platform / portability
Shared preflight gates before mutation
Protected branch / stable-branch policy concern
Work queue visibility and allocation
Read-only, no mutation
MCP client reconnect/reload recovery path
Recovery paths for stale/foreign leases
Code refactor / restructure
Release / versioning
Reliability / failure handling
Persist and resume prepared review verdicts across sessions
Reviewer workflow tooling
Roadmap / umbrella issue
Safety rails and fail-closed mutation guards
Security / trust boundary
Self-hosted infrastructure integration
Sentry error monitoring integration
Stale backend daemon / runtime-vs-master parity failures
Issue is blocked
Issue is being worked on
Issue has an open pull request
Issue is ready for work
Terminal review lock (#332) path
Tests / test coverage
Issue tracker hygiene / meta
Bug or defect
Feature or enhancement
Feature or enhancement
Safety gate or guardrail
Workflow state visibility for LLMs/operators
Cross-tool workflow
LLM workflow coordination hardening
LLM workflow coordination hardening
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: Scaled-Tech-Consulting/Gitea-Tools#966
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #931
Problem and security boundary
The server bound one transport, by name, at the bottom of the module:
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 totransport: 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.pyis 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.sseis 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_TRANSPORTSis 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.Nonemeans unbound.assert_transport_bound()fails closed before tool service.native_runtime_status()andmutation_provenance_fields()exposebound_transport.Entrypoint (
gitea_mcp_server.py)Both former literals are gone.
mcp.runis now served exactly the transport that was bound, read back through the one accessor.Durable provenance —
mutation_provenance_fields()gainsbound_transport, which reaches the durable decision lock throughmcp_session_state.save_stateand the audit records that already spread those fields (gitea_mcp_server.pydecision-lock payload andlive_mutationsentry).assess_transport_for_auth_mintreports the bound transport through that one accessor; its verdict is unchanged.#956 anchors — this change shifts lines that
docs/remote-mcp/threat-model-anchors.jsoncites, so the fixture and the prose are re-anchored, and the two boundary claims #931 makes false (the "literalstdiobind" in B1, and the stdio contract atmcp_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
GITEA_MCP_TRANSPORTresolves tostdio, 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_runtimeandassert_keychain_access_allowedare untouched.How invalid identifiers fail closed
UnsanctionedRuntimeErrorat bind time, with the offending value and#931in the reason. Nothing is bound, sobound_transport()staysNoneandis_native_mcp_transport()stays false.resolve_configured_transportreportssupported: falseand keeps the offending name.assert_transport_bound()refuses beforemcp.run, so the server does not reach tool service.""and can never match a permitted member.Explicit non-goals (untouched)
Test commands and results
All runs from the
branches/worktree, never a temporary directory.Base/head failure comparison
Baseline run from a dedicated
branches/worktree at the pinned base,branches/baseline-931-9b80e75c:Compared by failing test ID, not by count:
The failing sets are identical — 28 IDs on both sides. No failure added, none fixed. The
+42passed are exactly this PR's new module.The two
tests/test_mcp_server.pyfailures 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' != Noneattests/test_mcp_server.py:1820TestPreflightVerification::test_declared_clean_task_worktree_ignores_control_checkout_violation—AssertionError: False is not trueattests/test_mcp_server.py:4344Neither touches transport. Tool inventory:
tests/test_allocator_inventory_mcp.py,tests/test_pr_queue_inventory.py,tests/test_reviewer_inventory_worktree.pyare 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
9b80e75ca3f441fec2fb077a1b5f874faa0912e2(master)2fb835a1aa4cb6260445f2ef3f10057aabd2fbe7c1626081(the seam) and2fb835a1(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 atc1626081is valid at head)Changed files
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 head92615f47, and worktreebranches/issue-949-fleet-inventoryare 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:
mcp_transport_config.SUPPORTED_TRANSPORTSis the only enumeration — confirm viagrep -n '"stdio"' *.py, which should hit only that module.bind_native_mcp_transportsits after the entrypoint/mode/path checks, so a rebind attempt from a non-entrypoint still fails on provenance first.tests/test_issue_931_transport_bind_seam.py::_ProductionBinddrives 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.streamable-httpbefore #938 exists is the right call, and whether excludingsseshould 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.
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:
2fb835a1aatarget_branch: master
target_branch_sha:
9b80e75ca3last_activity: 2026-07-28T15:34:37Z
expires_at: 2026-07-28T15:44:37Z
blocker: none
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:
2fb835a1aatarget_branch: master
target_branch_sha:
9b80e75ca3last_activity: 2026-07-28T15:49:09Z
expires_at: 2026-07-28T15:59:09Z
blocker: none
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.runstarting an unauthenticated listener, then push tofeat/issue-931-transport-neutral-bind-seamand hand back for re-review.NEXT_PROMPT:
WHAT_HAPPENED:
Independent review at head
2fb835a1aain 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-httpwhilemcp.runis 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_transportis read only by reporting sites, so no decision anywhere depends on it.ISSUE:
#931
HEAD_SHA:
2fb835a1aaREVIEW_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 livemaster. 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-httpputs an unauthenticated listener one environment variable awaymcp.runnow receives the bound value:streamable-httpis inSUPPORTED_TRANSPORTS, soGITEA_MCP_TRANSPORT=streamable-httpbinds and that string reachesFastMCP.run, which dispatches torun_streamable_http_asyncand starts a uvicorn HTTP server over the entire tool surface.Executed at this head, not inferred:
FastMCP settings here:
host=127.0.0.1,port=8000,auth=None.FASTMCP_HOSTdoes 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() == Truefor 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_transportat this head is a reporting site:mcp_session_state.save_state, theassess_transport_for_auth_mintreport 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
carrier-pigeonrefused, "is not a registered MCP transport (#931)";bound_transportstays None; serve gate refusesmcp_transport_config.py:40; all other hits are comment, docstring, CLI text or error prosesave_statesetsbound_transport; provenance spread reaches the decision-lock payload andlive_mutationstransportvalues untouchedOrdering confirmed by reading
bind_native_mcp_transport: resolution and permitted-set validation both complete before any_NATIVE_RUNTIMEfield 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_RUNTIMEstays 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_statusexposes the environment key name, never a value.#956 anchors: five of 58 chosen at random resolve directly against this head, and
generated_against_commitc1626081appears 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_seamscans 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.bound_transport()returns"test"under the pytest runtime, so a durable record can carry a value outsideSUPPORTED_TRANSPORTS.Design judgments
streamable-httpas 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.
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:
2fb835a1aatarget_branch: master
target_branch_sha:
9b80e75ca3last_activity: 2026-07-28T15:52:46Z
expires_at: 2026-07-28T16:02:46Z
blocker: manual-release
Canonical PR State
STATE:
changes-addressed-awaiting-independent-re-review
WHO_IS_NEXT:
reviewer
NEXT_ACTION:
Re-review PR #966 at head
0104a76eeain 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:
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:
0104a76eeaREVIEW_STATUS:
review 635 REQUEST_CHANGES by sysadmin at head
2fb835a1remains 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-reviewMERGE_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
0104a76eVALIDATION:
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
9b80e75cfrom 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
EXECUTABLE_TRANSPORTS,assess_transport_execution,authorize_transport_execution, entrypoint now calls the guardtest_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_ownerassess_serve_authorizationreadsbound_transportand is the serve decisiontest_serve_decision_consumes_bound_transport,test_serve_verdict_reports_the_bound_transport,test_no_mutation_is_authorized_after_the_denialNon-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.runsite and that it routes through the guard; andtest_test_mode_runtime_is_not_servablecovers the pytest-only record being outside both the recognized and executable sets.Scope: two commits,
a143cd06(the gate) and0104a76e(#956 restamp, no.pytouched). Files changed since2fb835a1: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.py178 to 195 and 519 to 583); no other anchor moved.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:
0104a76eeatarget_branch: master
target_branch_sha:
9b80e75ca3last_activity: 2026-07-28T16:35:00Z
expires_at: 2026-07-28T16:45:00Z
blocker: none
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:
0104a76eeatarget_branch: master
target_branch_sha:
9b80e75ca3last_activity: 2026-07-28T16:45:00Z
expires_at: 2026-07-28T16:55:00Z
blocker: none
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:
0104a76eeatarget_branch: master
target_branch_sha:
9b80e75ca3last_activity: 2026-07-28T16:58:18Z
expires_at: 2026-07-28T17:08:18Z
blocker: none
Canonical PR State
STATE:
approved
WHO_IS_NEXT:
merger
NEXT_ACTION:
Merge PR #966 into
masterat head0104a76eea863eeb32ef42f245ddaa51ed2b197funder the merger role and profile, and let the merge transition close issue #931.NEXT_PROMPT:
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.runwas handed whatever was bound, soGITEA_MCP_TRANSPORT=streamable-httpwould 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_TRANSPORTSis a strict subset ofSUPPORTED_TRANSPORTScontaining only the local transport;assess_transport_executionreturns a structured verdict;mcp_daemon_guard.assess_serve_authorizationis the decision that consumesbound_transport; andauthorize_transport_executionenforces 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 atgitea_mcp_server.py:24753, the refusal is raised while the argument tomcp.runis still being evaluated somcp.runis 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:
0104a76eeaREVIEW_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
0104a76efrom 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 base9b80e75c: 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 head0104a76e. 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 livemaster. 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 andmcp_transport_config. The single occurrence of the word socket anywhere in the added lines is inside the test module's own tripwire.uvicornappears only in the pre-existingwebui/application, which this PR does not touch, and in one test docstring.EXECUTABLE_TRANSPORTSstill 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:24753is the onlymcp.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_asyncand direct uvicorn use. None exists outsidewebui/. Every other.run(hit in the tree issubprocess.runfor git.mcp_server.pyis the process the fleet actually runs, and it has no runner of its own: it compiles and executesgitea_mcp_server.pyinto its own__main__namespace, so the real production path does reach line 24753.Executed against the real entrypoint, three configurations:
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.TransportExecutionErrorcannot 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 nameUnsanctionedRuntimeErrorall live in tool functions and ingitea_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_authorizationcallsbound_transport()and is whatauthorize_transport_executionconsults. I confirmed consumption four ways, with reviewer-owned patches:bound_transportto the remote identifier flips the verdict from allowed to denied with blockertransport_listener_not_commissionedand owner#938; restoring the accessor restores the verdict.bound_transportto none yields blockertransport_not_bound.GITEA_MCP_TRANSPORTafter 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
carrier-pigeonrefused at line 24728;bound_transportstays nonemcp_transport_config.py, at the constant and in its module docstring; every other hit is a testmutation_provenance_fields()["bound_transport"]is the bound value while the trust-classtransportfield keepsnative_mcpUnsanctionedRuntimeError, notTransportExecutionError, with the unchanged "No MCP transport is bound" textIsolation 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 forbind_native_mcp_transport, 519 to 583 forassert_keychain_access_allowed), andgenerated_against_commitadvanced fromc1626081toa143cd06. The generation-commit semantics are correct:a143cd06is the commit that shifted the lines, and the final commit0104a76touches 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 ata143cd06and 58 of 58 at0104a76e.Non-blocking observations
test_no_mutation_is_authorized_after_the_denialdoes 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 denialassert_sanctioned_mutation_runtimestill 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.test_every_run_call_in_production_goes_through_the_guardglobs*.pyat the repository root only and keys on the literal textmcp.run(. A serve site added inside a subpackage, or a differently named runner such asrun_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.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_boundin 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-httpbefore #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 toEXECUTABLE_TRANSPORTSalongside the authentication boundary #938 must bring. Excludingsseremains 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.
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:
0104a76eeatarget_branch: master
target_branch_sha:
9b80e75ca3last_activity: 2026-07-28T17:00:15Z
expires_at: 2026-07-28T17:10:15Z
blocker: manual-release
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:
0104a76eeatarget_branch: master
target_branch_sha:
9b80e75ca3last_activity: 2026-07-28T18:35:56Z
expires_at: 2026-07-28T18:45:56Z
blocker: none
Stale #332 review-decision lock cleanup (#594)
Status: APPLIED
sysadminprgs-merger2026-07-28T18:36:55.423069+00:00approveon PR feat(transport): add transport-neutral MCP bind seam (Closes #931) (#966)closed(merged=True)17ba1ff035ee3154a0f2dcacbefe107457cca33f2prgs-reviewerManual deletion of session-state files is not the workflow.
This path only clears a lock when the referenced PR is merged/closed.