feat(auth): add account list/use Commander attachers by scottlovegrove · Pull Request #45 · Doist/cli-core

scottlovegrove · 2026-05-22T21:45:04Z

Summary

Adds attachAccountListCommand + attachAccountUseCommand under the @doist/cli-core/auth subpath, giving consumers account list / account use <ref> for multi-account credential management.
These consume TokenStore.list() / setDefault() — required on the contract since 0.12.0 but with zero consumer inside cli-core until now (the smell the roadmap flagged).
Mirrors the existing attachStatusCommand / attachLogoutCommand patterns: optional renderText/renderJson overrides with sensible defaults, hand-rolled json/ndjson/human branching, generics + .js extensions + type (not interface).

Behavior

account list — --json emits { accounts: [{ account, isDefault, … }], default: <ref|null> } where default is the default entry's account.id; --ndjson streams one per-account object per line (no envelope); human prints <label ?? id> (id:<id>) with a (default) marker. Renderers optional (unlike status) so a CLI gets working output with zero config.
account use <ref> — positional <ref> → store.setDefault(ref), then ✓ Default account set to <ref> (human) / { ok: true, default: <ref> } (--json) / silent (--ndjson, matching logout). ACCOUNT_NOT_FOUND propagates unchanged. Optional onDefaultSet follow-up hook.

Design notes

ndjson for list is true streaming (one object per account); per-account shape is identical to the json accounts[] entries.
default is derived from account.id (the contract's stable index key); stores with non-id ref-matching can override via renderJson. Documented in JSDoc.
The roadmap's "consolidate attacher context types" cleanup is deferred to a follow-up PR (non-breaking type-alias refactor; better factored once these two shapes exist).

Test plan

npm run build (tsc)
npm test — 465 pass, incl. 21 new cases in src/auth/account.test.ts
npm run check (oxlint + oxfmt) on the changed source

🤖 Generated with Claude Code

Adds attachAccountListCommand + attachAccountUseCommand under the /auth subpath, giving consumers `account list` / `account use <ref>` for multi-account management. They consume TokenStore.list() / setDefault(), which were required since 0.12.0 but had no consumer inside cli-core. Mirrors the existing attachStatusCommand / attachLogoutCommand patterns: optional renderText/renderJson overrides with sensible defaults, json envelope { accounts, default } with ndjson one-object-per-line streaming, and the logout-style success envelope (ndjson silent) for `use`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

doistbot

Adding the new account list and use commands provides a solid foundation for multi-account credential management. These additions effectively bridge the gap for consumers leveraging the existing TokenStore contract. A few adjustments are needed to polish the implementation, such as updating the README to document the new exports, ensuring the JSON payload contract is consistent and validated across commands, and replacing the buffered NDJSON path with true streaming. Additionally, reusing the existing emitView utility for output formatting and refining the test suite to cover custom overrides while trimming redundant cases will help ensure long-term maintainability.

_{Share Feedback • Review Logs}

doistbot · 2026-05-22T21:49:54Z

@@ -1,4 +1,10 @@
 export type { AuthErrorCode } from './errors.js'
+export { attachAccountListCommand, attachAccountUseCommand } from './account.js'


[P0] AGENTS.md strictly requires updating README.md in the same commit when adding a new export to a sub-path entry. The new attachAccountListCommand and attachAccountUseCommand exports must be documented in the README before the PR lands.

Done in daa9b9f — added both attachers + AttachAccountListContext to the "What's in it" auth row, and a new "Account selection (account list / account use)" usage block.

doistbot · 2026-05-22T21:49:54Z

+            const defaultRef: AccountRef | null =
+                accounts.find((entry) => entry.isDefault)?.account.id ?? null
+            if (view.json || view.ndjson) {
+                const payloads = accounts.map(({ account, isDefault }) =>


[P2] renderJson results aren't validated before they go into accounts. If a renderer returns undefined (or another non-serializable array element), --json will silently emit null inside accounts[], while --ndjson throws because formatNdjson rejects it. Please validate each per-account payload before building the envelope so both machine modes fail consistently instead of corrupting the JSON output on one path.

Holding off here for consistency: attachStatusCommand also passes its renderJson output straight to formatJson/formatNdjson with no validation, so guarding only this attacher would make the auth family inconsistent — a renderer returning undefined is a consumer programming error in either mode. If we want a uniform guard it's better as a small shared helper applied across all the attachers in a separate change. Happy to do that sweep if you'd prefer.

doistbot · 2026-05-22T21:49:54Z

+            const accounts = await options.store.list()
+            const defaultRef: AccountRef | null =
+                accounts.find((entry) => entry.isDefault)?.account.id ?? null
+            if (view.json || view.ndjson) {


[P2] This file now hand-rolls the json/ndjson/human switch twice (list here and use below), even though src/options.ts already has emitView for the shared JSON-vs-human path. If you keep list’s multi-record NDJSON case and use’s silent---ndjson case as early returns, the remaining branches can reuse emitView instead of maintaining another local copy of the canonical output dispatch.

Done in daa9b9f — both commands now use emitView for the json/human branches. list early-returns the multi-record --ndjson case first and builds the json payload lazily (only under --json) so renderJson stays machine-mode-only; use early-returns the silent --ndjson case, then emitView covers json+human.

doistbot · 2026-05-22T21:49:54Z

+            }
+            await options.store.setDefault(ref)
+            if (view.json) {
+                console.log(formatJson({ ok: true, default: ref }))


[P2] default means different things across the two new machine APIs. account list --json emits the resolved account.id, but account use --json echoes the raw <ref> the user typed. Reusing the same field name for both makes the JSON contract inconsistent and breaks simple round-tripping/comparisons. Please either emit the same canonical value in both commands, or rename this field to something input-specific like requestedRef.

Done in daa9b9f — account use --json now re-reads store.list() after setDefault and emits the resolved default account.id, so default is the same canonical value in both commands and round-trips. The human line still echoes the raw <ref> typed (conversational), but the machine field is canonical. Documented in the JSDoc + README.

doistbot · 2026-05-22T21:49:54Z

+            const defaultRef: AccountRef | null =
+                accounts.find((entry) => entry.isDefault)?.account.id ?? null
+            if (view.json || view.ndjson) {
+                const payloads = accounts.map(({ account, isDefault }) =>


[P2] This NDJSON path still builds the full payloads array and then a second full joined string via formatNdjson(...) before writing anything. That adds unnecessary memory/CPU overhead and loses the per-account streaming this command is documented to provide. Split the --ndjson branch from --json and emit each rendered entry as it's produced instead of buffering the whole list.

Done in daa9b9f — the --ndjson branch now loops and emits each entry as its own console.log(formatNdjson([payload])) line instead of building the whole array + a joined string. Empty list still emits nothing.

doistbot · 2026-05-22T21:49:54Z

+        )
+    })
+
+    it('streams one object per account under --ndjson with no envelope', async () => {


[P2] This only checks the default NDJSON payload. The new contract here is that renderJson shapes each NDJSON line the same way it shapes accounts[] under --json, but that path isn't exercised anywhere in this file. Add an --ndjson case with a custom renderJson so a regression in the override path can't slip through.

Done in daa9b9f — added a test "shapes each --ndjson line via renderJson, matching the --json accounts entries" that drives --ndjson with a custom renderJson and asserts each line equals the corresponding accounts[] entry shape.

doistbot · 2026-05-22T21:49:54Z

+        expect(logSpy).not.toHaveBeenCalled()
+    })
+
+    it('echoes the raw user-supplied ref, not a resolved id', async () => {


[P3] This doesn't verify anything beyond the previous emits the success envelope under --json test: same command, same ref, same assertion. Either drop it, or rewrite it with setup that makes the "raw user-supplied ref, not a resolved id" distinction observable.

Done in daa9b9f — replaced it. With the canonical-id change, use bob@b --json now emits { ok: true, default: "2" } (a2's id) while the human line echoes bob@b, so the new test "emits the canonical resolved id under --json, not the requested ref" exercises a real requested-vs-resolved distinction.

- Document the new exports in README (What's in it + Account selection usage block) per the AGENTS.md README-maintenance rule. - account list --ndjson now streams one object per account instead of buffering a joined string, matching the documented streaming contract. - account use --json now emits the resolved default account.id (re-read from list() after setDefault) so it round-trips against account list --json; the human line still echoes the raw ref. - Adopt the shared emitView helper for the json/human branches. - Add an --ndjson + custom renderJson test; replace the redundant raw-ref test with one asserting the canonical-vs-requested distinction. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

doistbot reviewed May 22, 2026

View reviewed changes

		@@ -1,4 +1,10 @@
		export type { AuthErrorCode } from './errors.js'
		export { attachAccountListCommand, attachAccountUseCommand } from './account.js'

Conversation

scottlovegrove commented May 22, 2026

Summary

Behavior

Design notes

Test plan

Uh oh!

doistbot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants