Skip to content

fix: preserve keychain read failures - #240

Merged
skvark merged 2 commits into
mainfrom
agent/preserve-keychain-read-errors
Jul 30, 2026
Merged

fix: preserve keychain read failures#240
skvark merged 2 commits into
mainfrom
agent/preserve-keychain-read-errors

Conversation

@skvark

@skvark skvark commented Jul 30, 2026

Copy link
Copy Markdown
Member

Summary

  • preserve keychain access failures instead of converting them to missing credentials
  • apply the same behavior to token reads, client-registration reads, and conditional token clearing
  • keep genuine missing entries as null and continue refusing implicit plaintext fallback
  • add regression coverage and document the current dependency limitation

Why

This is the downstream half of Brooooooklyn/keyring-node#136.

@napi-rs/keyring@1.3.0 currently collapses keychain read failures and missing entries to the same null result. The upstream PR changes that boundary so only a genuine missing entry returns null; access and platform failures throw.

Our migration layer was independently catching those thrown failures and returning null again. This PR removes that second error-collapsing boundary so a fixed keyring release will produce actionable storage guidance instead of an unauthenticated result.

Important

This PR is safe to merge before the upstream release. With @napi-rs/keyring@1.3.0, the changed read paths remain inactive because the dependency still returns null; therefore this PR alone does not resolve the reported macOS behavior for installed users.

The release and dependency-bump work is tracked separately in #241.

Validation

  • targeted storage tests: 50 passed
  • full test suite: 2,505 passed
  • build passed
  • built CLI unauthenticated smoke passed
  • built MCP registration smoke passed
  • lint completed with one unrelated pre-existing warning

Authenticated live smoke coverage reached the backend successfully but was subsequently rate-limited with HTTP 429.

Keep missing credentials distinct from keychain access failures in the migrating storage layer. This prepares the CLI to surface errors once the upstream @napi-rs/keyring read fix is released.
@skvark skvark added the bug Something isn't working label Jul 30, 2026 — with ChatGPT Codex Connector
@skvark
skvark marked this pull request as ready for review July 30, 2026 07:00
Document that the downstream error handling is safe to merge now but remains inactive for read failures until keyring-node#136 is released and adopted.
@skvark
skvark merged commit c9165d2 into main Jul 30, 2026
8 checks passed
@skvark skvark mentioned this pull request Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working skip-changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant