Skip to content

fix(auth): treat corrupted auth.json as empty instead of crashing - #29562

Closed
Tushar49 wants to merge 8 commits into
anomalyco:devfrom
Tushar49:Tushar49-patch-2
Closed

fix(auth): treat corrupted auth.json as empty instead of crashing#29562
Tushar49 wants to merge 8 commits into
anomalyco:devfrom
Tushar49:Tushar49-patch-2

Conversation

@Tushar49

@Tushar49 Tushar49 commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown

Closes #29628, #18351

Issue for this PR

Refs #27530 (sub-cause: corrupted auth.json from interrupted writes — the issue thread also tracks provider-API 5xx, #-in-instructions parser crash, and SSL/config failures, all of which are out of scope here)

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

When auth.json is left in a corrupt state (whitespace-only or otherwise unparseable — typically because a process was killed mid-write), opencode currently crashes at startup with the cryptic:

Error: 4 of 5 requests failed:
  config.providers: Unexpected server error
  provider.list: Unexpected server error
  app.agents: Unexpected server error
  config.get: Unexpected server error

No log file is created (the failure happens before logging is wired). The community workaround on issue #27530 is to nuke ~/.local/share/opencode/ (or its Windows equivalent) — destroying every saved session. 10+ users have hit this across Windows, NixOS, Pop!_OS, macOS, and WSL.

Root cause: FileSystem.readJson in packages/core/src/filesystem.ts called JSON.parse synchronously inside an Effect.fn generator. A SyntaxError from JSON.parse becomes an Effect defect, not a typed failure. Auth.all then wrapped the call with Effect.orElseSucceed which only catches typed failures — the defect bypassed it and crashed the fiber. Every config-dependent request (Config.loadInstanceState → Auth.all → readJson) failed with the generic "Unexpected server error".

Changes

  • packages/core/src/filesystem.ts — wrap JSON.parse in Effect.try returning a typed FileSystemError, matching the existing pattern at line 81 (readDirectoryEntries). Effect.orElseSucceed / Effect.tapError in callers can now actually intercept parse failures.
  • packages/opencode/src/auth/index.ts — log a warning only on FileSystemError (parse failure) so first-run users with no auth.json stay silent and only actual corruption is surfaced. Apply the same recovery + log to the OPENCODE_AUTH_CONTENT env-var branch and run it through Record.filterMap so both branches return the same Record<string, Info> shape.
  • packages/opencode/test/auth/auth.test.ts — 5 new it.live tests covering: half-written JSON, binary garbage, single-key get() lookup on corrupted file, set() recovery after corruption, and positive verification that valid OPENCODE_AUTH_CONTENT is honored end-to-end.

Out of scope (intentional, to keep this PR small):

  • Atomic writeJson to prevent future corruption — being addressed independently in fix(storage): write json files atomically #28010.
  • Provider-startup graceful degradation when provider APIs return 5xx (sub-cause [b] in the issue thread) — bigger architecture change.
  • # character in instructions file crashing the parser (sub-cause [c]) — separate bug.

How did you verify your code works?

  • bun typecheck from packages/core — clean (tsgo exit 0)
  • bun typecheck from packages/opencode — clean
  • bun test test/filesystem/ from packages/core — 26/26 pass (no regressions)
  • bun test test/auth/ test/filesystem/ from packages/opencode — 33/33 pass (was 28; +5 new tests)
  • Manual repro: seeded auth.json with 300 spaces — before: crash with "4 of 5 requests failed"; after: opencode starts, logs failed to parse auth file at <path>, treating as empty; run 'opencode auth login' to recover (...) and continues.

Screenshots / recordings

Not a UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Tushar49 added 3 commits May 27, 2026 15:47
@Tushar49
fix(core): convert FileSystem.readJson parse errors to typed failures
df14b58 
JSON.parse throwing inside Effect.fn becomes an Effect defect that bypasses Effect.orElseSucceed and similar typed-error recovery patterns. Wrap parse in Effect.try returning a FileSystemError so callers can recover from corrupted JSON files (e.g. half-written auth.json on interrupted writes).

Part of fix for #27530.
@Tushar49
fix(auth): log and recover from corrupted auth file instead of crashing
795fd74 
Add Effect.logWarning before falling back to empty credentials so users see why their auth file was ignored (the original symptom is the cryptic '4 of 5 requests failed' with no log file). Apply the same recovery path to OPENCODE_AUTH_CONTENT, and run both branches through Record.filterMap so they share the same Record<string, Info> shape.
@Tushar49
test(auth): cover corrupted auth.json recovery paths
791ba92 
Five new it.live tests verifying Auth.all/get/set survive: half-written JSON, binary garbage, single-key lookup on corrupted file, set() recovery after corruption, and invalid OPENCODE_AUTH_CONTENT.
@Tushar49 Tushar49 mentioned this pull request May 27, 2026
Closed
Tushar49 added 3 commits May 27, 2026 16:04
@Tushar49
fix(core): normalize filesystem.ts line endings to LF
1081364 
Previous web-upload introduced CRLF line endings on every line, making the diff appear as a full-file rewrite. Reupload the same content with LF line endings so the actual change (the Effect.try wrap on JSON.parse) is the only diff.
@Tushar49
fix(auth): only warn on parse failures, not missing files
536db7d 
Per PR review: previous tapError fired Effect.logWarning for any readJson error including the normal first-run case where auth.json doesn't exist yet. Narrow the warning to FileSystemError (which is now produced by the JSON.parse wrap) so missing-file is silent and only actual corruption is logged. Also LF line-ending fix.
@Tushar49
test(auth): tighten env-var test to positive case + LF endings
84e3d37 
Replace the negative bad-OPENCODE_AUTH_CONTENT test (which would have passed even on the old code: bad env -> catch swallows -> fall through -> missing-file orElseSucceed -> {}) with a positive test verifying valid env content is honored end-to-end. Also LF line-ending fix on the whole test file.
@github-actions

github-actions Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@Tushar49

Tushar49 commented May 27, 2026

Copy link
Copy Markdown
Author

Issue #27530

@Tushar49 Tushar49 mentioned this pull request May 27, 2026
Closed
@Tushar49

Tushar49 commented May 29, 2026

Copy link
Copy Markdown
Author

Friendly nudge — rebased on latest dev, CI green. Closes #29628. Treats corrupted auth.json as empty (with a warning log) instead of crashing the CLI, so users can recover by re-authenticating instead of manually deleting files. Could @adamdotdevin or @fwang give this a look when convenient? Happy to iterate on any feedback. Thanks!

@edevil

edevil commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

This PR also fixes #18351 ("readJson() lacks error handling for malformed JSON"), which is currently open with no associated PR.

The change to packages/core/src/filesystem.ts — wrapping JSON.parse in Effect.try and surfacing a typed FileSystemError instead of letting the parse throw as an uncatchable Effect defect — is exactly what #18351 asks for, and it satisfies its acceptance criteria (meaningful error tied to the file/method, original cause preserved).

It also resolves a separate crash path: opencode mcp list / mcp auth dies with JSON Parse error: Unrecognized token u0000 when ~/.local/share/opencode/mcp-auth.json is corrupt. McpAuth.all() already guards with .pipe(Effect.catch(() => Effect.succeed({}))), but because the old readJson threw the parse error as a defect (not the typed error channel), Effect.catch never caught it. With this fix the fallback works and the command degrades gracefully. (Tracked in #18351; symptom also reported in #25008.)

Consider adding Closes #18351 to the description.

@Tushar49

Tushar49 commented Jun 2, 2026

Copy link
Copy Markdown
Author

Thanks @edevil — really appreciate the cross-link. Updating the PR description to add Closes #18351 so it auto-closes on merge. You're right that the MCP-auth crash path is the same root cause: readJson was throwing the JSON.parse defect outside the Effect error channel, so every caller's Effect.catch fallback (Auth.all and McpAuth.all alike) was bypassed. With the parse now routed through Effect.try → typed FileSystemError, both auth files degrade to empty instead of killing the CLI on startup.

@danielxxomg

danielxxomg commented Jun 5, 2026

Copy link
Copy Markdown

I was looking into this issue and noticed that the fix still seems relevant, but the current dev branch has changed since this PR was opened.

It looks like readJson / writeJson moved from packages/core/src/filesystem.ts to packages/core/src/fs-util.ts, so the same fix would now need to be applied there instead. The auth/index.ts changes seem like they should still map cleanly to the current code.

Would it make sense to rebase this against the current dev layout, or would maintainers prefer a fresh follow-up PR?

@github-actions

github-actions Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Automated PR Cleanup

Thank you for contributing to opencode.

Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions.

This PR was closed because it matched the following cleanup criteria:

  • The PR was created more than 1 month ago
  • The PR had fewer than 2 positive reactions
  • Positive reactions are counted as thumbs-up, heart, celebration, or rocket reactions on the PR

PRs created within the last month are not affected by this cleanup.

If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate.

Thanks again for taking the time to contribute.

@github-actions github-actions Bot closed this Jun 27, 2026
@github-project-automation github-project-automation Bot moved this from Backlog to Done in OSS Contribution Tracker Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

automated-pr-cleanup

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

auth.json corruption causes startup crash (sub-cause of #27530)

3 participants

@Tushar49 @edevil @danielxxomg