Skip to content

feat(auth): scope renewable outside-agent OAuth grants - #558

Merged
lukemaj merged 5 commits into
mainfrom
feat/553-outside-oauth
Oct 19, 2026
Merged

lukemaj merged 5 commits into
mainfrom
feat/553-outside-oauth

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

What: Outside agents can be approved until revoked, with project-id and named-operation limits enforced by the server.
Why: Unattended services otherwise lose sign-in after 30 days, and outside agents can currently reach every project.
So what: Ready for human review against main, with current CI and independent review passed; no merge or deployment is authorized.

Closes #553, including the scoping acceptance criteria.

Approval retains the 30-day default and adds Until revoked, an optional immutable project id, and named operations. Renewable grants return one-hour access tokens and rotating refresh tokens bound to the client and resource. Refresh authority has a separate signing domain; only the current and previous hashes are persisted. Renewal copies the signed ceiling and the existing session owns revocation. The declaration boundary rejects unapproved tools, other projects, remote forwarding and borrowed caller contexts before handlers. Settle permits only settling.

#323's paired-client protocol, peer links, #552 and #554 are unchanged. Renewable outside grants use an absolute deadline to avoid #389's existing year-10000 TTL overflow; the peer helper and existing grants are untouched. This supports the Objective by allowing unattended services to drive Drafters within an approved project.

Elon record

Requirements and who asked: Lukasz requested #553 plus comment 6275891763, self-renewal with immediate revocation, fixed server-side project/operation limits, unchanged 30-day defaults, red tests first, shortened clock, isolated worktree and different-model review.
Deleted: Global TTL changes, monthly reapproval, a separate non-OAuth credential mechanism, and work owned by #323/#552/#554/#379.
Bottleneck: OAuth lacked refresh grants and MCP declarations lacked project/operation limits; browser proof also exposed #389's existing permanent-TTL overflow.
Checked myself: Read the Issue and required comment in full, checked #323's mcp-client exclusion and upstream history/open PRs/Issues, ran red/green authorization and clock tests, and exercised UI approval/revocation and real MCP HTTP requests against synthetic worktree state.

Proof

  • Red: missing refresh_token in HTTP exchange; 13 denial cases failed against main's original declaration code while 7 positive cases passed; absolute-expiry regression failed before its fix.
  • Green: 9 focused files, 185 tests passed. HTTP OAuth/default TTL, rotation/replay rejection, client/resource binding, scope-widening rejection, retained runtime/project/operation ceilings, 31-day clock advance, revocation, database close/reopen, absolute-expiry listing and revocation, read/write fences.
  • Focused lint passed. Scoped server and web typechecks exited 0 (Effect suggestions only). The bounded retry correction was checked with 45 focused auth tests and the full nine-file 185-test selection passing. No repo-wide checks.
  • Authorized browser proof: approved a synthetic service with Until revoked, wedding-proof project and named operations. Real MCP HTTP client read its project, launched an idle thread, read/interrupted/settled it, renewed and kept its restrictions. Other-project reads/launches and unapproved tools were refused before and after renewal.
  • Existing synthetic refresh authority survived the isolated dev watcher's automatic reload during implementation; disk-backed close/reopen also has deterministic coverage. No live application/server restart was performed.
  • Settings → Connections listed the corrected grant. Revoke immediately produced MCP HTTP 401 and refresh HTTP 400 invalid_grant.
  • Web/desktop share the approval page. Mobile has no separate native OAuth approval form. Authorization is provider-independent; the synthetic thread was idle and started no model turn. Origins remain request-derived, and restricted grants cannot forward to other environments.

Focused commands:

node_modules/.bin/vp test run apps/server/src/auth/McpOAuth.test.ts apps/server/src/auth/SessionStore.test.ts apps/server/src/auth/EnvironmentAuth.test.ts apps/server/src/auth/EnvironmentAuthAdmin.test.ts apps/server/src/mcp/outsideGrant.test.ts apps/server/src/mcp/McpToolAccess.test.ts apps/server/src/peer/PeerLinks.test.ts packages/contracts/src/auth.test.ts packages/contracts/src/environmentHttp.test.ts
# Test Files 9 passed (9), Tests 185 passed (185)
node_modules/.bin/vp test run apps/server/src/auth/SessionStore.test.ts apps/server/src/auth/McpOAuth.test.ts
# Test Files 2 passed (2), Tests 45 passed (45), after bounded retry correction
node_modules/.bin/vp exec tsc-rs --noEmit -p apps/server/tsconfig.json
node_modules/.bin/vp exec tsc-rs --noEmit -p apps/web/tsconfig.json
# Both exit 0, suggestions only

Lint ran only the 12 changed TypeScript/TSX files and exited 0. Test machine bestia, Node 26.8.2; no runtime installation or production credential use. Full CI remains GitHub-owned.

Refresh retry behavior

Concurrent SDK refreshes and immediate retries return the same successor for a fixed five-second window. The successor nonce uses a separate HMAC domain; the server stores only the current and immediate predecessor hashes and the original retry deadline. Retrying does not slide that deadline. Reuse at or after five seconds, or of an older token, revokes the existing session and all its access/refresh authority.

The pinned MCP SDK's real refreshAuthorization runs twice concurrently against the real OAuth HTTP routes in the tests, preserving the ceiling. A disk reopen at four seconds retains the same successor; retry at five seconds revokes, and an older-than-immediate token revokes even within the window. These cover both independent review findings without requiring SDK users to write a refresh lock. Final-head Claude Opus 5.5 independent review passed, with no P1/P2 findings; the reviewer inspected code and author logs without running additional tests. The fork registry and allowlist now assign the three new carried upstream edits to #553, and the fork stack check passes locally.

UI evidence

Before:
Before

After:
After

Final head: d1703f980eac5dfd81fa473635e084693c2882e8, based on main c8b69c643bfe2fb031ffcd096376a6cd35a9dac8. Independent Claude Opus 5.5 review at medium effort passed on that exact head; the newest review/independent status is success from lukemaj. GitHub CI attempt 2 passed on this unchanged head, including all eight server shards and the aggregate Check. The earlier Grok replay idle assertion passed on the single failed-job rerun, with no source/assertion/timeout change; its original cause remains unknown. Transfer measurements remain within all ceilings with zero measured increase. All applicable checks pass; deployment/native-only jobs are skipped by CI policy. Versions remain unchanged under the fork's AGENTS.md. Website impact: none, this changes the app's OAuth approval route; user guidance and the feature map are updated.

Implemented with GPT-6.1-Sol, medium effort, through the Codex harness.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L labels Oct 19, 2026
@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent review of f3e109bb5f8419773b7961f17132852bf52e1b2d (Claude Opus 5.5, claude-opus-5-5, medium effort): one P2 finding, so review/independent is set to failure.
Why: A replayed (already rotated) refresh token is rejected but does not revoke the grant, so a stolen refresh token used first keeps an "Until revoked" grant alive indefinitely while the real service is locked out.
So what: The author fixes the P2 and requests a fresh review on the new head; no user decision needed.

P2: refresh-token reuse does not revoke the grant

  • Location: apps/server/src/auth/SessionStore.ts, refreshMcpOAuthToken, the if (updated.length !== 1) return yield* new MalformedSessionTokenError({}) branch after the compare-and-swap UPDATE auth_mcp_refresh ... AND token_hash = ....
  • Trigger: An attacker obtains the service's current refresh token (it is a bearer secret for a public client with token_endpoint_auth_method: none and no sender constraint) and refreshes first. The attacker's chain rotates and stays valid. The legitimate service then presents the now-stale token, which has a valid signature, matching client/resource binding and a live session, but a hash mismatch. It gets invalid_grant, and nothing else changes.
  • Effect: The attacker keeps renewing for as long as the grant lives, which for this feature is until manual revocation. If the operator responds by re-approving the service instead of finding and revoking the old row in Settings → Connections, the attacker's grant stays live next to the new one. RFC 9700 §4.14.2 allows public-client refresh tokens only when sender-constrained or rotated, and rotation there includes revoking the active token when a stale one is presented, because the server cannot tell which party is the legitimate one.
  • Evidence: In SessionStore.test.ts ("outside OAuth ... rotation"), replaying first.refreshToken after renewal is asserted to fail, but the test never checks renewed.refreshToken or renewed.token afterwards. They stay valid under the current code; the session is only cut by the explicit store.revoke(issued.sessionId) that follows.
  • Fix: When the signature, binding and session check pass but the hash CAS updates no row, revoke the session (or at least delete its auth_mcp_refresh row and revoke) before returning invalid_grant. Extend the test so that after replaying the old token both renewed.token and renewed.refreshToken fail. Concurrent refreshes from one client would then also revoke the grant. RFC 9700 accepts that tradeoff, and the user docs already tell clients to store each replacement token.

Cleared in the inspected scope

  • Refresh domain separation: refresh tokens are signed over mcp-refresh.<payload> and stored hashes over mcp-refresh-hash.<token>. Session payloads are base64url without ., so the signing domains cannot collide. Only the current refresh hash is persisted, bound to session_id, client id and resource. Rotation is a single compare-and-swap UPDATE, so a race has exactly one winner.
  • Ceilings: renewal re-verifies the original signed claims (verify, which covers revocation and expiry) and copies scopes, rtc, plk and mcp unchanged. scope on refresh is rejected, and resource must match.
  • Bypass paths: mcp-client tokens are rejected outside /mcp (rejectMcpClientAudience) for HTTP and WebSocket. Every MCP tool goes through Declaration.declare, which now runs checkOutsideGrant before McpRouting.withContext or a handler. The operation map is an allowlist, so other tools are denied. environmentId and contextThreadId are denied, and contextCapabilities is inert without contextThreadId. Launch requires the approved projectId, which rules out scratch and omitted projects. Thread tools require an explicit threadId whose shell is in the project and not deleted. Organize permits only settle.
  • Defaults and persistence: 30 days stays the default (renewable only for duration: "until-revoked"). auth_mcp_refresh lives in the persisted database, and the signing secret comes from the secret store. The session row's 9999-12-31T23:59:59.999Z deadline sorts lexically and avoids fix(auth): permanent peer expiry can fall outside lexical active-session queries #389's year-10000 overflow. SessionRenewal still excludes mcp-client (Paired desktop and browser sessions renew instead of closing at 30 days #323 unchanged), and the peer helper is untouched.

Optional, not blocking

  • ConnectAgentSurface.tsx adds raw <input type="radio">/checkbox controls even though the file already uses the Radio primitive. The project's components/ui convention prefers the primitives.
  • A refresh request that repeats the originally granted scope is rejected with invalid_grant. RFC 6749 §6 allows an identical scope. The MCP TypeScript SDK does not send scope on refresh, so this is only an interoperability note.

Proof limits

I read the full diff, #553 and comment 6275891763, and the consumers named above. I ran no tests. I inspected the author's logs: /tmp/553-head-tests.log (9 files, 183 passed), /tmp/553-final-layer-tests.log (34 passed), and /tmp/553-type-final.log (suggestions only). I did not re-run the browser or MCP HTTP proof, and I did not view the screenshots.

Reviewed by Claude Opus 5.5 (claude-opus-5-5) through the Claude Code harness.

@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

What: Fixed the review's P2 in 76be06eb1bf761d68e30c4478606a4e98688bdc0: reusing a rotated, correctly signed and client/resource-bound refresh token now revokes the existing session.
Why: Rejecting only the stale request left the successor's access and refresh authority usable by whichever bearer rotated first; the original test checked rejection without checking the successor.
So what: Fresh Claude Opus 5.5 medium review will examine the new head. PR stays draft pending that verdict; no user decision or live change.

Finding accepted and independently checked against the compare-and-swap failure branch. The new regression failed before the fix because the successor access token still authenticated. After the fix it proves stale refresh rejection, successor access/refresh rejection, and absence from active clients. A separate concurrent-refresh test proves one successful rotation followed by revocation of the raced grant. The manual-revocation test remains separate and retains wrong-client/resource denials followed by a successful legitimate renewal. User guidance now tells clients to serialize renewal and never reuse replaced tokens.

Focused proof: vp test run apps/server/src/auth/SessionStore.test.ts apps/server/src/auth/McpOAuth.test.ts: 2 files,45tests passed. Scoped server typecheck exit0, changed-file lint exit0. The nine-file current-head rerun is running; its output is /tmp/553-replay-final-tests.log. Earlier full focused183-test run and UI/MCP/screenshot proof remain in the PR.

Optional notes are deferred: no demonstrated client failure requires changing the refresh scope-input policy, and switching the new native form inputs to primitives is outside this security correction.

Requirements and who asked: #553's renewable OAuth grant, with the independent review's P2 requiring compromised rotation authority to be cut off.
Deleted: Treating stale-token rejection alone as sufficient proof; no new credential store, retry/grace mechanism, or change to #323.
Bottleneck: Final exact-head review of the replay correction.
Checked myself: Read the full review, reproduced the successor-token failure red, inspected session revocation ownership, ran replay/concurrency/manual-revocation tests, scoped types and lint, and verified main is still the task base.

@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent review of 76be06eb1bf761d68e30c4478606a4e98688bdc0 (Claude Opus 5.5, claude-opus-5-5, medium effort). The replay fix works as intended, but it adds one new P2, so review/independent is set to failure.
Why: The MCP TypeScript SDK pinned in this repo refreshes once per concurrent 401, with no single-flight. Two parallel calls made after the one-hour access token expires now present the same refresh token, and the second presentation revokes the whole "Until revoked" grant.
So what: Either the author makes an immediate concurrent retry safe, or Lukasz accepts the documented "refresh one at a time" limit and the outreach service serializes its refreshes. That limit is a user decision; after a fix, a fresh review runs on the new head.

P2: concurrent refresh from the stock MCP SDK revokes the grant

  • Location: apps/server/src/auth/SessionStore.ts, refreshMcpOAuthToken, the if (updated.length !== 1) { yield* revoke(refresh.claims.sid); ... } branch. The PR's concurrent outside OAuth refreshes revoke the raced grant test asserts this outcome.
  • Trigger: An unattended client built on @modelcontextprotocol/sdk@1.29.0 (the version in this repo's node_modules) has two requests in flight when its one-hour access token expires. Client request() calls can run concurrently, and a t3_thread_wait long-poll alongside a read is a typical case. In dist/esm/client/streamableHttp.js send(), each 401 independently calls auth(). In auth.js authInternal, each auth() reads the same provider.tokens().refresh_token and calls refreshAuthorization. Nothing deduplicates these refreshes.
  • Effect: The first refresh rotates the token. The second has a valid signature and binding but a stale hash, so the server runs revoke(sid), and the winner's new access and refresh tokens die too. The SDK then treats the InvalidGrantError by calling invalidateCredentials('tokens') and falling into startAuthorization/redirectToAuthorization, which an unattended service cannot complete. The grant is dead until Lukasz approves it again. This breaks Outside agents can keep their MCP sign-in until revoked #553's outcome ("keeps working ... without user action"), and nothing malicious is needed to trigger it. In the previous round I accepted this tradeoff on the assumption that clients refresh one at a time. The pinned SDK shows that assumption does not hold for the reference client, so this finding corrects my earlier acceptance.
  • Current mitigation: docs/user/outside-agents.md says "Refresh one request at a time: reusing a replaced refresh token revokes the sign-in." That leaves every client to wrap the SDK's auth provider in its own single-flight lock, and the stock SDK does not do this.
  • Fix options: (a) Make an immediate retry idempotent. Derive the successor refresh token deterministically from the presented token, for example a nonce of HMAC("mcp-refresh-nonce." + presented). Keep the previous hash together with a rotation time. A presentation of the immediately previous token within a short window (seconds) then returns the same successor and a fresh access token. Reuse outside the window, or of any older token, still revokes, which keeps the RFC 9700 §4.14.2 theft detection that fixed the prior P2. (b) Keep the current rule as a documented contract, record the decision in Outside agents can keep their MCP sign-in until revoked #553, and prove that the outreach client serializes its refreshes. Rejecting the duplicate without revoking does not help the SDK, because its invalidateCredentials('tokens') can wipe the winner's saved tokens.
  • Proof needed: Two concurrent refreshes with the same token, both within the window, leave the session active, and both callers receive the same successor refresh token. A replay of the old token after the window, or of an older token, still revokes, so the existing replay test stays green.

Prior P2 (replay without revocation): fixed

refreshMcpOAuthToken now revokes the session when the signature, client/resource binding and live-session verify all pass but the compare-and-swap updates no row. revoke (SessionStore.ts:1210) marks the row revoked, drops connected state and emits removal, so verify rejects both the replacement access token and the replacement refresh token. The updated test checks current.token, current.refreshToken and listActive. A wrong client id, wrong resource or bad signature returns invalid_grant without revoking, so a third party cannot revoke a grant by presenting a token it does not hold. Manual revoke is unchanged.

Grant ceiling boundaries re-checked on this head: cleared

  • Domain separation: Refresh tokens are signed over mcp-refresh.<payload>, stored hashes use mcp-refresh-hash.<token>, and session signatures cover a dotless base64url payload. A refresh token fails verify, and an access token fails the refresh signature check. Only the current hash is persisted, keyed by session_id.
  • Fixed ceiling on renewal: Renewal re-verifies the signed original claims and copies scopes, rtc, plk and mcp unchanged. scope on refresh is rejected, and resource must equal urls.resource and the bound resource. Access tokens last one hour.
  • Read and write bypasses: Every handler goes through Declaration.declare, and routedHandlers provides CurrentTool, so checkOutsideGrant runs before McpRouting.withContext and before any handler or forwarding. The operation map is an allowlist. environmentId and contextThreadId are refused. t3_thread_organize is limited to settle. Launch and project read require the exact projectId, so scratch or an omitted project id is refused. Thread tools require an explicit threadId whose live shell belongs to the project. The read, send, wait and interrupt inputs carry no other thread or project reference. The launch workspaceStrategy is bound to the project's worktrees. delegatedFrom is pre-existing behaviour and only takes effect alongside linkOrigin.
  • Max date, default and persistence: The absolute 9999-12-31T23:59:59.999Z deadline lists and revokes (tested), and the 30-day default is unchanged unless duration: "until-revoked". auth_mcp_refresh lives in the persisted database, and the signing secret comes from the secret store. SessionRenewal (Paired desktop and browser sessions renew instead of closing at 30 days #323) and the peer helper are untouched.

Optional, still not blocking

Native <input type="radio">/checkbox controls in ConnectAgentSurface.tsx, and rejecting an identical scope on refresh. Both remain deferred, as agreed.

Proof limits

I read #553, comment 6275891763, the full PR diff at 76be06eb, the prior review, the consumers named above, and the pinned SDK's client/auth.js and client/streamableHttp.js. I ran no tests and no SDK reproduction. The P2 rests on reading the SDK source, not on an executed client. Author logs: /tmp/553-replay-final-tests.log (185 passed), /tmp/553-replay-race-tests.log (45 passed), /tmp/553-replay-types.log and /tmp/553-final-web-types.log (suggestions only), /tmp/553-replay-lint.log (no errors shown). I did not re-run the browser or MCP HTTP proof or view the screenshots.

Reviewed by Claude Opus 5.5 (claude-opus-5-5) through the Claude Code harness.

@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

What: Fixed the concurrent-SDK-refresh P2 in d1703f980eac5dfd81fa473635e084693c2882e8 without removing stale-token theft detection.
Why: The pinned SDK refreshes each concurrent failed request independently; immediate reuse must return its original successor instead of revoking an unattended grant.
So what: Fresh Claude Opus 5.5 medium review is requested on this exact head; the PR remains draft pending its verdict.

Finding accepted after reading the pinned SDK's client/streamableHttp.js and client/auth.js. The previous concurrency test assumed a client-side refresh lock and protected the wrong outcome. Its replacement failed red on the preceding head and now checks both successful callers, the exact same successor refresh token, live access and unchanged project/operation limits.

The successor nonce is derived with a separate HMAC domain from the presented token. We persist only the current and immediate predecessor hashes plus a fixed five-second deadline. A duplicate within that deadline returns the same successor; it neither rotates again nor extends the deadline. Older-token reuse or reuse at/after the deadline still revokes the shared session. This state survives reopening the database, and the schema adds the two nullable retry fields when an earlier candidate table exists.

Proof on this candidate:

  • Nine focused files,185tests passed (/tmp/553-grace-final-tests.log).
  • The real pinned SDK's refreshAuthorization runs twice concurrently against the real OAuth HTTP routes, returns identical successors and retains the approved ceiling. The test adapter calls the existing native web handler, with no manual Effect runtime.
  • Disk close/reopen at four seconds retains the same successor. Another retry at five seconds revokes, proving the deadline did not slide. An older-than-immediate predecessor also revokes inside the window. Manual revoke, wrong-client/resource denials and31-day renewal remain covered.
  • Scoped server typecheck exit0 (/tmp/553-grace-types2.log), changed-file lint exit0 (/tmp/553-grace-lint2.log). Existing web typecheck and UI screenshots remain applicable; no UI code changed in this correction.
  • Fork CI's three unregistered carried edits are now assigned to Outside agents can keep their MCP sign-in until revoked #553 in the fork registry and allowlist. node scripts/fork-features.mjs check and bash scripts/fork-check.sh --upstream upstream pass locally.

Requirements and who asked: #553's unattended self-renewal plus independent review's replay and SDK-concurrency findings.
Deleted: Requiring each SDK user to write a refresh lock; storing successor credentials; an extending grace window; dropping reuse detection.
Bottleneck: Final exact-head review and GitHub checks.
Checked myself: Pinned SDK source, red concurrent refresh test, actual SDK OAuth route calls, deterministic clock boundaries, older replay, database reopen, scoped checks and fork ownership gate. No live data, live restart, deploy or merge.

@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent review of d1703f980eac5dfd81fa473635e084693c2882e8 (Claude Opus 5.5, claude-opus-5-5, medium effort): PASS, no P1 or P2 findings, review/independent set to success.
Why: The immediate-retry window fixes the concurrent-refresh revocation without reopening the replay finding: only the immediate predecessor, inside a fixed persisted five-second window, gets the same successor, and every other reuse still revokes.
So what: Merge stays with Lukasz once CI is green on this head; no other decision is needed.

Prior findings, adjudicated on this head

  • P2 replay left the attacker's successor live (comment 6276452298): still fixed. In SessionStore.ts refreshMcpOAuthToken, a failed compare-and-swap now goes to the retry SELECT, which matches only when token_hash equals the successor derived from the presented token, previous_hash equals the presented token's hash, and previous_valid_until > now. Any older token, or a retry at or after the deadline, misses and runs revoke(sid). verify runs first, so a revoked session cannot retry. Wrong client, wrong resource, bad signature and malformed tokens return before any revoke, so a party without the token cannot revoke the grant.
  • P2 stock SDK concurrency revoked the grant (comment 6276554583): fixed. The successor nonce is HMAC("mcp-refresh-nonce." + presented), so two concurrent presentations of the same token produce the same refresh token. The SQLite UPDATE ... RETURNING has one winner, and the loser's SELECT sees the committed row. The McpOAuth test drives the pinned @modelcontextprotocol/sdk refreshAuthorization twice through Promise.all against the real token route and gets one successor.

Security tradeoff, accepted

A thief who presents the same stale token within five seconds of the real rotation receives the same successor instead of triggering revocation. Because successors are deterministic, both holders then share one chain. The next time either one presents a superseded token more than five seconds after a rotation, the grant is revoked. The thief cannot read the legitimate client's timing, so detection is delayed by at most the window, not lost. That is the RFC 9700 §4.14.2 tradeoff proposed in the prior review and documented in docs/user/outside-agents.md.

Checked and cleared

  • Domain separation: the mcp-refresh., mcp-refresh-hash. and mcp-refresh-nonce. prefixes differ at byte 11 or 12, and session and asset verifiers only sign dotless payloads. The nonce is visible inside the successor's payload, but it is never accepted as a signature anywhere. No refresh secret is stored, only HMAC hashes.
  • No widening or extension: the retry path copies the signed claims (scopes, runtime ceiling, project and operation limits, absolute session deadline) unchanged, the access token stays one hour, and the retry path performs no UPDATE, so the deadline cannot slide. The disk-reopen test at 4 s gets the same successor, and at 5 s it revokes, including the retried access token.
  • Schema: CREATE TABLE IF NOT EXISTS with nullable previous_hash and previous_valid_until, plus a guarded ALTER inside one transaction for databases created by earlier heads of this branch.
  • Grant gate unchanged since the last review: checkOutsideGrant runs in Declaration.declare before McpRouting.withContext and every handler. The operation allowlist, the environmentId/contextThreadId denial, settle-only organize, the exact projectId for launch and project read, and the live thread-shell project check are all unchanged. The 30-day default, the 9999-12-31T23:59:59.999Z deadline listing and revoke, SessionRenewal (Paired desktop and browser sessions renew instead of closing at 30 days #323), the peer helper (fix(auth): permanent peer expiry can fall outside lexical active-session queries #389) and Opt-in tool allowlist for delegated and launched threads #552/Thread reads expose quota waits, scheduled recovery and successor runs #554/spec: native cross-machine control across linked environments #379 code are untouched by this commit.
  • Fork registry: outside-agent-oauth-grants lists the two new files, the three newly carried upstream files (McpOAuth.test.ts, mcpOAuthHttp.ts, ConnectAgentSurface.tsx, each present on upstream/main and listed exactly once in scripts/fork-upstream-edits.txt), and the eight shared files that other features already claim. The Fork Stack Model check passes on this head.

Optional, still deferred

Native <input type="radio">/checkbox controls in ConnectAgentSurface.tsx, and rejecting an identical scope on refresh. No real client is shown to hit either one.

Proof limits

I read #553, comment 6275891763, both prior reviews, the full diff c8b69c643b..d1703f980e with the 76be06eb..d1703f98 delta line by line, and the consumers named above. I ran no tests and no SDK reproduction myself. Author logs, which I did not tie to this SHA myself: /tmp/553-grace-final-tests.log (9 files, 185 passed), /tmp/553-grace-final-auth.log (45 passed), /tmp/553-grace-types2.log and /tmp/553-final-web-types.log (0 error TS, suggestions only), /tmp/553-grace-lint2.log (no errors), /tmp/553-final-fork-check.log (OK). CI on this head was still pending when I posted. I did not re-run the browser or MCP HTTP proof or view the screenshots.

Reviewed by Claude Opus 5.5 (claude-opus-5-5) through the Claude Code harness.

@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.9 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: c8b69c6 · PR result: d1703f9 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

What: PR #558 hit the previously reported Grok fast-wake idle assertion at unchanged head d1703f980eac5dfd81fa473635e084693c2882e8; its focused local replay passes.
Why: Server shard 1 still saw run ordinal 3 running with no pending requests at await_thread_idle; the cause remains unknown.
So what: One failed-job rerun is requested on the same SHA. PR #558 remains draft; no source, assertion, timeout or retry-policy change.

Failed job: OrchestratorV2ScenarioStepError, grok_background_bash_fast_wake/grok, await_thread_idle, ordinal 3 running, requests empty. 823 passed, 1 failed, 9 skipped. Aggregate Check fails solely because test_server failed. This matches the failure signature already recorded at #382 (comment), without establishing a shared cause. #553 does not change replay fixtures, scenario waiting, or provider adapters.

Unchanged local command: node_modules/.bin/vp test run apps/server/src/orchestration-v2/testkit/OrchestratorReplayFixtures.integration.test.ts -t grok_background_bash_fast_wake, exit 0, 1 passed, 146 filtered/skipped, 7.55s. Local log /tmp/553-ci-replay-target.log; CI log /tmp/553-ci-server1.log. This isolated pass does not replace full CI or prove a flake. Main baseline c8b69 CI passed. Transfer impact passes all ceilings with zero measured increase but correctly says source CI failed.

Requirements and who asked: User #553 requires focused proof and an independent different-model review; repository CI supplies complete checks.
Deleted: CI bypass, changes to unrelated replay code, weakened assertions/timeouts, repeated reruns and unsupported flake attribution.
Bottleneck: The unchanged failed-job rerun result.
Checked myself: Exact failed job and aggregate logs, full transfer comment, unchanged local replay result, current clean Git/SHA, main baseline CI, and prior matching #382 report.

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

Labels

size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Outside agents can keep their MCP sign-in until revoked

1 participant