fix(oauth): degrade to re-auth on corrupt credential cache instead of crashing

[KrasimirKralev]

Problem

When a cached OAuth credential file under an explicit tokenCacheDir (or a legacy per-server cache during migration) is present but corrupt — truncated or malformed JSON, e.g. from an interrupted write — DirectoryPersistence's readers let the JSON.parse SyntaxError propagate. Through CompositePersistence that crashes the MCP connection instead of treating the file as "no usable credentials" and degrading to re-auth.

Root cause

DirectoryPersistence.readTokens() and readClientInfo() call readJsonFile, which maps only ENOENT to undefined and re-throws everything else (including SyntaxError).

This is asymmetric with the rest of the codebase, which already treats a corrupt credential file as recoverable:

• VaultPersistence / readVaultState (oauth-vault.ts) catches SyntaxError and rebuilds (needsRepair)
• daemon/host.ts and server-proxy.ts wrap their reads in catch-all handlers

These DirectoryPersistence readers were the lone exceptions.

Fix

Route the credential readers (readTokens, readClientInfo) through a small private helper (readJsonOrUndefined) that maps only SyntaxError → undefined; genuine I/O faults still propagate. Mirrors the existing VaultPersistence pattern (if (!(error instanceof SyntaxError)) throw error).

OAuth state is deliberately excluded. A corrupt state.txt must fail the flow closed: oauth.ts only enforces the callback state check when expectedState is truthy, so degrading state to undefined would skip the CSRF check (thanks @chatgpt-codex-connector for catching this on the first pass). readState() therefore keeps throwing on corrupt input. readJsonFile itself is also left untouched, since the vault relies on it throwing SyntaxError to distinguish corrupt-from-missing for its repair path.

Verification

Run locally (Node 22, vitest):

• New regression test in tests/oauth-persistence.test.ts writes corrupt tokens.json / client.json / state.txt and asserts readTokens() / readClientInfo() resolve to undefined while readState() rejects with SyntaxError (CSRF fail-closed).
  • The credential-degrade assertions fail on main (SyntaxError: Unterminated string in JSON); they pass with the fix.
• Full oauth-persistence suite: 35/35 passing.
• tsgo --noEmit, oxfmt --check, oxlint --type-aware: clean.

Fixes #207.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 175e5b10f1

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[KrasimirKralev]

Good catch, thanks — addressed in 94e66b1.

You're right that readState() must not degrade. oauth.ts only enforces the state check when expectedState is truthy, so a corrupt state.txt returning undefined would skip the CSRF check on the callback. I've narrowed the corrupt-tolerance to the credential caches only (readTokens / readClientInfo, which degrade to re-auth); readState() keeps throwing so the flow fails closed. The regression test now asserts readState() rejects with SyntaxError while tokens/client resolve to undefined.

[clawsweeper]

ClawSweeper review: did not complete due to Codex infrastructure failure. Reviewed June 16, 2026, 9:04 AM ET / 13:04 UTC.

Summary
Review failed before ClawSweeper could summarize the requested change.

Reproducibility: unclear. The review failed before ClawSweeper could establish a reproduction path.

Review metrics: none identified.

Merge readiness
Not assessed.
Failure reason: timeout.

This is a ClawSweeper/Codex infrastructure failure, not a PR readiness or patch-quality verdict.
Keep any merge decision on the normal maintainer review path until ClawSweeper can complete a fresh review.

Risk before merge

• [P1] No close action taken because the review did not complete.

Maintainer options:

1. Decide the mitigation before merge
   Retry the Codex review after fixing the execution failure.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

• Review did not complete, so no work-lane recommendation was made.

Review details

Best possible solution:

Retry the Codex review after fixing the execution failure.

Do we have a high-confidence way to reproduce the issue?

Unclear. The review failed before ClawSweeper could establish a reproduction path.

Is this the best way to solve the issue?

Unclear. Retry the review first so ClawSweeper can evaluate the actual issue and fix direction.

AGENTS.md: unclear because the file could not be read completely.

Codex review notes: model internal, reasoning high; reviewed against 37391ce70b3f.

Evidence reviewed

What I checked:

• failure reason: timeout.
• codex failure detail: Codex review failed for this PR: Codex process timed out after 1200000ms.
• codex stderr: No stderr captured.
• codex stdout: enclaw/mcporter --json number,title,state,url,author,body,labels,comments'","aggregated_output":"{"author":{"id":"U_kgDOD7QqeQ","is_bot":false,"login":"KrasimirKralev","name":"Krasimir Kralev"},"body":"### Bug type\n\nError handling / resilience — a present-but-corrupt credential cache file crashes an MCP connection instead of degrading to re-auth.\n\n### Summary\n\nDirectoryPersistence (used whenever a server sets tokenCacheDir, and during the best-effort legacy-cache migration) reads OAuth artifacts through readJsonFile, whose catch only handles ENOENT:\n\n```ts\n// src/fs-json.ts:14\nexport async function readJsonFile<T = unknown>(filePath: string): Promise<T | undefined> {\n try {\n const content = await fs.readFile(filePath, 'utf8');\n return JSON.parse(content) as T;\n } catch (error) {\n if ((error as NodeJS.ErrnoException).code === 'ENOENT') {\n return undefi.
• process error code: ETIMEDOUT.

Likely related people:

• unknown: Codex failed before it could trace repository history. (role: review did not complete; confidence: low)

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

[vincentkoc]

Maintainer verification for bea0ed6:

• Corrupt token and client caches degrade to re-auth; corrupt OAuth state remains fail-closed.
• Stale-client I/O failures still close the callback server.
• Focused OAuth coverage, format/lint/typecheck, and the full suite passed: 779 passed, 3 skipped.
• Fresh local Codex review found no actionable findings.

@codex review