feat(auth): tolerate AUTH_STORE_READ_FAILED in logout --user (2/4) by scottlovegrove · Pull Request #26 · Doist/cli-core

scottlovegrove · 2026-05-16T12:17:00Z

Stack position: 2 / 4 — base is #25 (feat/keyring-primitive). Replaces part of #24.

Summary

A TokenStore may legitimately throw CliError('AUTH_STORE_READ_FAILED', …) from active(ref) when a matching record exists but the token itself can't be read (e.g. an OS keyring backing the store is offline). Without special handling, logout --user <ref> would abort even though clear(ref) doesn't need the token.

Add AUTH_STORE_READ_FAILED to AuthErrorCode.
attachLogoutCommand catches it on the explicit-ref pre-flight and proceeds with clear(ref). Other typed errors (notably ACCOUNT_NOT_FOUND from a genuine miss) still propagate.
attachStatusCommand / attachTokenViewCommand propagate the error unchanged — they can't render without the token.
Extract accountNotFoundError(ref) helper in auth/user-flag.ts so PR 3's setDefault can share the construction.

Why now

The concrete thrower is in PR 3 (createKeyringTokenStore), but this attacher behavioural change is logically a contract addition — any TokenStore consumer can use it — and reviews cleaner on its own.

Test plan

npm run type-check, npm run check, npm test (307 passing, +3 new attacher tests)
Once PR 3 lands, smoke logout --user <ref> against a createKeyringTokenStore with DBUS_SESSION_BUS_ADDRESS unset.

🤖 Generated with Claude Code

doistbot

This PR usefully enhances the authentication flow by introducing the AUTH_STORE_READ_FAILED error code to allow logout --user <ref> to proceed even when the token itself cannot be read. The changes logically isolate the new contract addition, making the TokenStore more robust and smoothly paving the way for the upcoming keyring implementation. A few adjustments are noted, such as ensuring the onCleared callback retains the requested reference when an account is unreadable, utilizing the existing buildStore helper to avoid test duplication, adding missing test coverage for the token-view command, and keeping the accountNotFoundError helper file-local to prevent a dead export.

_{Share Feedback • Review Logs}

scottlovegrove · 2026-05-16T13:48:26Z

@doistbot /review

doistbot

This PR effectively introduces handling for AUTH_STORE_READ_FAILED in the authentication flow, ensuring logout --user can proceed smoothly even when the backing token store is unreadable. The targeted error recovery and extracted helpers create a highly resilient contract for upcoming keyring integrations. A few adjustments are needed to accurately reflect the pre-flight error handling in the contract documentation, streamline the conditional logic for snapshot retrieval, and strengthen the assertions in the error propagation tests.

_{Share Feedback • Review Logs}

scottlovegrove · 2026-05-16T14:18:22Z

@doistbot /review

doistbot

This PR introduces the AUTH_STORE_READ_FAILED error code to allow the logout command to successfully clear records even when the underlying token is unreadable. It is a thoughtful enhancement to the authentication contract that improves the robustness of the CLI against offline keyrings. A few minor additions have been flagged for completeness, specifically updating the TokenStore.active() JSDoc to document the new failure mode and ensuring test coverage includes both the token-view error propagation and plain logouts without hooks.

_{Share Feedback • Review Logs}

doistbot · 2026-05-16T14:21:28Z

        })
    })
+
+    it('surfaces AUTH_STORE_READ_FAILED end-to-end (does not collapse to ACCOUNT_NOT_FOUND)', async () => {


[P2] This test successfully asserts error propagation for status, but the analogous test for token-view mentioned in the PR conversation appears to have been omitted from the diff. Please ensure token-view.test.ts is staged and included.

Held — earlier in this PR's review history we explicitly dropped the analogous token-view test as redundant with the status propagation test (both attachers go through the same requireSnapshotForRef with no local catch, so they share a code path and a regression would surface in either).

A `TokenStore` may legitimately throw `CliError('AUTH_STORE_READ_FAILED', …)` from `active(ref)` when a matching record exists but the token itself can't be read — e.g. an OS keyring backing the store is offline. Without special handling, that would abort `logout --user <ref>` even though local clear doesn't need the token; only the server-side `revokeToken` hook does, and the hook is harmlessly skipped when the snapshot is unavailable. Changes: - Add `AUTH_STORE_READ_FAILED` to `AuthErrorCode`. - `attachLogoutCommand` catches it on the explicit-ref pre-flight and proceeds with `clear(ref)`. Other typed errors (notably `ACCOUNT_NOT_FOUND` from a genuine ref miss) still propagate. - `attachStatusCommand` / `attachTokenViewCommand` propagate the error unchanged — they can't render without the token. - Add a `ref` field to `AttachLogoutContext` (and the revoke context). It is set to the `--user <ref>` argument or `undefined` when absent, so a consumer's `onCleared` callback can distinguish "nothing was stored" (`account: null`, `ref: undefined`) from "cleared an unreadable record by ref" (`account: null`, `ref: "me"`). - Keep `accountNotFoundError(ref)` as a file-local helper in `auth/user-flag.ts` so a future caller can lift it without duplicating the error wording. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

## [0.14.0](v0.13.0...v0.14.0) (2026-05-16) ### Features * **auth:** tolerate AUTH_STORE_READ_FAILED in logout --user ([#26](#26)) ([23652bc](23652bc))

doist-release-bot · 2026-05-16T14:29:26Z

🎉 This PR is included in version 0.14.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

scottlovegrove self-assigned this May 16, 2026

This was referenced May 16, 2026

feat(auth): add createKeyringTokenStore multi-account TokenStore (3/4) #27

Merged

feat(auth): add keyring-backed multi-account TokenStore #24

Closed

doistbot reviewed May 16, 2026

View reviewed changes

Comment thread src/auth/logout.ts Outdated

Comment thread src/auth/logout.test.ts Outdated

Comment thread src/auth/status.test.ts Outdated

Comment thread src/auth/user-flag.ts Outdated

Comment thread src/auth/status.test.ts

scottlovegrove force-pushed the feat/auth-store-read-failed branch from 644b8c5 to 2cd6aaf Compare May 16, 2026 12:35

scottlovegrove force-pushed the feat/keyring-primitive branch from 165fb07 to 5b0cfd7 Compare May 16, 2026 13:50

scottlovegrove force-pushed the feat/auth-store-read-failed branch from 2cd6aaf to 5092110 Compare May 16, 2026 13:50

doistbot reviewed May 16, 2026

View reviewed changes

Comment thread README.md Outdated

Comment thread src/auth/logout.ts Outdated

Comment thread src/auth/status.test.ts Outdated

Base automatically changed from feat/keyring-primitive to main May 16, 2026 13:57

scottlovegrove force-pushed the feat/auth-store-read-failed branch 3 times, most recently from e5b268a to 93bb598 Compare May 16, 2026 14:10

scottlovegrove commented May 16, 2026

View reviewed changes

Comment thread src/auth/user-flag.ts Outdated

scottlovegrove force-pushed the feat/auth-store-read-failed branch 2 times, most recently from 3c50122 to fe9ab44 Compare May 16, 2026 14:15

doistbot reviewed May 16, 2026

View reviewed changes

scottlovegrove force-pushed the feat/auth-store-read-failed branch from fe9ab44 to 3e86be6 Compare May 16, 2026 14:23

scottlovegrove merged commit 23652bc into main May 16, 2026
4 checks passed

scottlovegrove deleted the feat/auth-store-read-failed branch May 16, 2026 14:28

doist-release-bot Bot added a commit that referenced this pull request May 16, 2026

chore(release): 0.14.0 [skip ci]

5424c65

## [0.14.0](v0.13.0...v0.14.0) (2026-05-16) ### Features * **auth:** tolerate AUTH_STORE_READ_FAILED in logout --user ([#26](#26)) ([23652bc](23652bc))

doist-release-bot Bot added the released label May 16, 2026

Conversation

scottlovegrove commented May 16, 2026

Summary

Why now

Test plan

Uh oh!

doistbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

scottlovegrove commented May 16, 2026

Uh oh!

doistbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

scottlovegrove commented May 16, 2026

Uh oh!

doistbot left a comment

Choose a reason for hiding this comment

Uh oh!

doistbot May 16, 2026

Choose a reason for hiding this comment

Uh oh!

scottlovegrove May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

doist-release-bot Bot commented May 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

scottlovegrove May 16, 2026 •

edited

Loading