Skip to content

fix(mcp): route call-time ErrNoToken through the auth gate (closes #376) - #377

Merged
initializ-mk merged 1 commit into
mainfrom
fix/mcp-authgate-calltime-notoken
Aug 1, 2026
Merged

fix(mcp): route call-time ErrNoToken through the auth gate (closes #376)#377
initializ-mk merged 1 commit into
mainfrom
fix/mcp-authgate-calltime-notoken

Conversation

@initializ-mk

Copy link
Copy Markdown
Contributor

Closes #376.

Problem

A delegated (type=user) MCP agent whose user has no grant failed the tool call hard instead of parking on the auth gate — no mcp_auth_required, task completes, LLM narrates the raw error:

tool error tool=atlassian-write__searchJiraIssuesUsingJql
  mcp atlassian-write/searchJiraIssuesUsingJql: mcp: no stored token — login required:
  no platform grant for subject "mk@initializ.io" ... (#317)
mcp_tool_result ok=false reason=no_token duration_ms=65   # no park, no gate event

The auth gate (#330) was consulted only for resolveClient (establish) errors in mcp_tool.go::Execute. But transport_http attaches the bearer per request (authFn runs inside Send, every frame including tools/call), so a streamable-HTTP MCP server that initializes fine and only 403s on the actual call surfaces ErrNoToken from client.CallTool — which the gate never inspected. (Diagnostic tell: the error carried no initialize:/connect: withPhase prefix, proving it bypassed establish.)

Fix

Execute now wraps the whole resolve→call sequence: an ErrNoToken from either the establish or the CallTool half parks via authGate.Await, and on a granted resume retries resolve→call. Same bounded, one-shot gate semantics as before — the establish-time behavior is unchanged, the call-time path is newly covered.

Tests

go test ./... green (forge-core), gofmt + golangci-lint clean.

Downstream

Restores the clean park + auto-resume path for the platform consent flow (initializ/agent-builder emits/consumes mcp_auth_required; initializ/console-next#67 surfaces it). The console's proactive "connect your account" banner (#67) was a partial mitigation; this is the server-side root fix.

The delegated-consent auth gate (#330) was consulted only for
resolveClient (connection-establish) errors. But per-request-auth
transports (transport_http attaches the bearer on every Send,
including tools/call) 403 at CALL time, not at initialize — so a
type=user server whose user has no grant surfaced ErrNoToken from
client.CallTool, which the gate never inspected. The call failed hard
with reason=no_token, no mcp_auth_required event fired, and the
platform consent flow had nothing to trigger on (field: Atlassian
agent, forge#376).

Execute now wraps the whole resolve→call sequence in the gate: an
ErrNoToken from either half parks via authGate.Await and, on a granted
resume, retries resolve→call. Same bounded, one-shot semantics as
before; the diagnostic phase prefix is irrelevant now since both paths
are covered.

Tests: call-time no-token parks + retries to success; gate give-up
still fails as no_token; a non-auth CallTool error never parks; nil
gate still surfaces ErrNoToken.

@initializ-mk initializ-mk left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the full resolve→call rework against the branch source — correct, minimal, well-tested. Approve-worthy. This is a genuine bug fix and the root-cause analysis in the description matches the code.

The bug is real, the diagnosis is right

The auth gate (#330) previously wrapped only resolveClient — the establish half. But transport_http attaches the bearer per request (authFn runs inside Send on every frame, including tools/call), so a streamable-HTTP server that initializes fine and only 403s on the actual call surfaces mcp.ErrNoToken from client.CallTool — which the old code reached only after the gate check. Hard fail, reason=no_token, no park, no mcp_auth_required. The "no initialize:/connect: withPhase prefix" tell is a legitimate proof it bypassed establish.

The fix is structurally sound

  • One-shot semantics preserved — the gate branch is a plain if, not a loop, so a park happens at most once; a second ErrNoToken on retry falls straight through to emit+return as no_token.
  • Retry re-resolves intentionally — on a granted resume resolveAndCall re-runs resolveClient so the per-subject pool picks up the now-granted connection, then calls. For the fixed-client case (resolver == nil) the re-resolve just returns m.client — harmless.
  • Retry is safe against double-executionErrNoToken is a 403, so the first CallTool did no work before failing; re-issuing can't double-apply a write. Strictly improves on before, where a call-time no-token never retried at all.
  • Establish-time path unchanged — net behavior identical to the old code.

Tests cover all four branches

CallTimeNoToken_Parks, GateGivesUp, CallTimeError_NoSpuriousPark, NoGate_NoTokenSurfaces. The gateStub.onAwait hook flipping the mock client to success cleanly models "grant now exists." Interface signature matches (Await(ctx, server string) error). CI fully green.

Non-blocking observations (no fix required)

  • res.Content on a (nil, nil) return would nil-deref in flattenContent — but this is pre-existing (the old code had the identical shape) and real MCP clients never return (nil, nil). Noted only for completeness.
  • No test asserts the "retry still returns ErrNoToken" grant-race case doesn't double-park — the non-looping structure makes that obvious, so it's a nicety, not a gap.

Good fix. LGTM.

}

res, err := resolveAndCall()
if err != nil && m.authGate != nil && errors.Is(err, mcp.ErrNoToken) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the crux of the fix and it's correct. Gating resolveAndCall (resolve→call) rather than just resolveClient means an ErrNoToken from either half now parks: the establish case (unchanged) and the call-time case (newly covered — the #376 regression, where per-request-auth transports 403 on the tools/call frame).

One-shot semantics are intact — this is a single if, not a loop, so a grant-race that still yields ErrNoToken on the retry (line 229) falls through to emit+return as no_token rather than parking twice. And the retry re-running resolveClient is the right call: a per-subject pool needs to re-pick the now-granted connection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP auth gate bypassed when ErrNoToken surfaces at tool-call time (per-request auth), not connection-establish

1 participant