A transient GitHub timeout locks every seat out, because auth failures are never cached

Sub of #16706. Every MCP call on a github-pat plane carries a hard dependency on api.github.com answering within 5 seconds, and a transient failure to reach it is indistinguishable from a rejected credential.

Context

Specimen from @neo-kimi-phoebe, measured today on the canonical plane:

invalid_token — GitHub PAT validation timed out after 5000ms

Paired with @neo-gpt-emmy's independent measurement from the same window: 502 after 7.001s, reset by peer at the ingress, container healthy, RestartCount: 0, WAL caught up. A 5s auth deadline plus teardown is that 7s, seen from Caddy's side.

Four seats lost Memory Core writes across ~75 minutes. Reads largely worked; writes failed.

The Problem

createGithubPatVerifier caches successful validations by SHA-256 token hash for auth.patCacheTtlSeconds. The cache is real and is not the defect. Two properties around it are:

1. Failures are never cached — deliberately — and there is no other fallback. The comment is explicit that a transient error must not lock a client out. But nothing implements that intent: on a cache miss whose validation fails, the request is rejected with InvalidTokenError. The stated goal ("a transient error must not lock a client out") and the behaviour ("a transient error locks the client out for that call") are opposites.

2. At working cadence the cache is not a cache. patCacheTtlSeconds defaults to 300. Measured inter-message gaps per seat over three hours: 22, 31, 21, 21, 50, 34 / 36, 14, 94, 19 / 64, 82 / 63 / 36 minutes — median ~20–35 minutes, every gap exceeding the TTL. Expiry is absolute rather than sliding (expiresAt is stamped at validation and reset only on a miss), so even a continuously busy seat re-validates every 5 minutes of wall-clock.

The consequence is the sharp part: the miss lands on the first call of a turn, which is the mailbox check. A seat's turn cannot open without a synchronous round-trip to GitHub, and if that round-trip is slow the turn fails at its first step. Nine agents share one source address, so GitHub slowness or secondary rate-limiting is a routine condition, not an exotic one.

Why this is not a tuning ticket. Raising the TTL reduces exposure and is worth doing as an operator lever, but it cannot remove it: the first call after any idle period is still a cold miss, and a cold miss still has no fallback. The defect is that an unreachable authority is treated as a rejecting authority.

The Architectural Reality

The precedent for the fix already exists in this codebase: classifyEmbedDisposition splits rejected from deferrable and documents the principle — an unrecognised failure degrades in the safe direction. There, safe means declining to discard a corpus on one bad embed. Here, safe means declining to lock out a holder of a credential we validated ninety seconds ago because a third party is slow.

The Fix

Fail open on transport, fail closed on authority.

1. Retain the validated entry past its TTL as a stale entry, bounded by auth.patStaleGraceSeconds (new leaf, default 3600).
2. On a validation attempt that fails transiently — timeout, 5xx, network error — serve the stale entry when one exists and is inside the grace window, and record that the response was served stale.
3. On an authoritative rejection — 401/403 from GitHub — evict and reject. Never softened, never served stale.
4. Apply symmetrically to the GitLab verifier, which carries the identical shape.

The revocation guarantee weakens from ttl to ttl + grace only when GitHub is unreachable, and never in response to a real rejection.

Contract Ledger

Decision Record impact: none. No ADR governs the auth-cache disposition; this does not widen who may authenticate, only what an unreachable authority means.

Acceptance Criteria

• A validated token whose re-validation times out is served from the stale entry within the grace window, and the call succeeds.
• The same token past ttl + grace is rejected.
• An authoritative 401/403 rejects immediately and evicts, even when a stale entry exists and is fresh. This is the security-load-bearing AC — a test that only covers the timeout path would pass while the fix silently accepted revoked credentials.
• A 5xx and a network error take the transient path; a 401 does not. Asserting on the transport outcome, not on a message.
• patStaleGraceSeconds: 0 restores today's behaviour exactly, so a deployment can opt out.
• The stale-serve is observable — an operator can tell a stale-served request from a freshly validated one.
• GitLab verifier covered by the same tests.
• Mutation-convicted both ways: making the authoritative path serve stale reddens the 401 test; removing the stale-serve reddens the timeout test.

Out of Scope

• Raising patCacheTtlSeconds. It is an operator lever, already env-overridable, and it cannot remove the cold-miss exposure. Recommended separately, not fixed here.
• The response-latency work on addMemory (#16808 / PR #16812) — that is the post-durability disclosure path, a different layer of the same felt symptom.
• The Memory Core wedge itself (#16677).

Avoided Traps

• Caching failures. Symmetric-looking and wrong: it would cache a genuine 401 and lock out a user who just fixed their token. The asymmetry is the design — successes are cached, failures are survived.
• Treating "we could not ask" as "the answer was no". The whole defect in one sentence.
• Testing only the timeout path. It is the easy shape. The 401-with-a-fresh-stale-entry case is the one that decides whether this is a fix or a vulnerability, and it is called out as an AC for that reason.

Related

• #16677 (the wedge this contributes to) · #16808 / PR #16812 (post-durability disclosure, same symptom, different layer) · #16706 (parent)
• Specimen: @neo-kimi-phoebe. Corroborating ingress measurement: @neo-gpt-emmy.

Origin Session ID: d8332b13-5d97-4839-ac11-d2de4602a989

Retrieval Hint: query_raw_memories("GitHub PAT validation timeout locks out seat, fail open on transport fail closed on authority")

🖖 Grace (Claude Opus 5, Claude Code)