feat(account): account command group (cli-core attachers)#79
Conversation
Wire cli-core's account attachers into a new `ol account` group: list / use / current / remove. The multi-account storage (users[] config + keyring) and the `--user` selector / store-wrap shipped in #77; this surfaces it to users. - `ol account` defaults to `list`; default account marked (accessible-aware) - `ol account current` honours `--user` and reports OUTLINE_API_TOKEN / legacy single-user sources via onNotAuthenticated - machine output drops the OAuth client id - SKILL_CONTENT gains an Accounts section (SKILL.md regenerated) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
doistbot
left a comment
doistbot
left a comment
There was a problem hiding this comment.
This PR successfully introduces the ol account command group, cleanly wrapping the underlying multi-account storage capabilities and providing a solid foundation for the next phase of the cli-core migration. The implementation effectively leverages our existing storage plumbing to make multi-user workflows significantly more intuitive. There are a few implementation details to refine, primarily around standardizing the JSON output shapes for machine consumers alongside their SKILL_CONTENT documentation, correcting the authentication fallback precedence, and avoiding duplicate configuration reads. Additionally, extracting some duplicated logic, utilizing Commander's native default routing, and simplifying the test mocks while expanding coverage for keyring fallbacks and --user overrides will help ensure the new commands are fully robust.
- Fix auth precedence: prefer a stored v2 account over a lingering legacy
token. `current` now only reports the legacy source when no v2 record
backs the resolved account (was short-circuiting on any legacy session).
- Single discriminated `--json` envelope for `current`:
{source:"stored", account} | {source:"env"} | {source:"legacy"};
SKILL_CONTENT updated + SKILL.md regenerated.
- Resolve the ambient source once in the store wrapper and stash it, so the
miss path no longer re-reads config via a second isLegacyAuthActive probe.
- Dedup the clear-result handling into a shared `logClearResult` in auth.ts,
reused by `auth logout` and `account remove`.
- Drop redundant `?? account.id` (label is a required string).
- Tests: add --user override + keyring-fallback-warning coverage; legacy
case no longer depends on isLegacyAuthActive.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@doistbot /review |
doistbot
left a comment
doistbot
left a comment
There was a problem hiding this comment.
This PR successfully introduces the ol account command group to expose multi-account management capabilities to users. While the new commands effectively leverage existing attachers to enhance the CLI experience, there are a few opportunities to streamline the internal integration and test suite. Specifically, adjustments are needed to centralize the auth-source resolution logic to prevent duplication and redundant store reads, improve the accuracy of legacy account detection, and extract shared utilities out of sibling command modules. Further refinements to replace private Commander API usage and strengthen the tests with simpler stubs and comprehensive JSON coverage will help solidify the implementation.
- Add `resolveActiveAccountSource` to auth-provider as the authoritative env/stored/legacy classifier. It resolves the v2 account by ref via the bare keyring store, so a renamed legacy snapshot sharing a UUID with a v2 record can no longer be misreported as `stored` (fixes the UUID-equality P1). `account current` now consumes it instead of re-deriving the cascade from a `store.list()` membership check — one store read on the normal path. - Drop the `_defaultCommandName` private-field cast: bare `ol account` now delegates to `list` via a parent `.action()` (public commander API). - Move `logTokenStorageResult` / `logClearResult` into `src/lib/auth-output.ts` so `account.ts` no longer imports a sibling command module. - Tests: per-test stubs with explicit arg assertions; `current` stubs `resolveActiveAccountSource`; added --json/--ndjson coverage for the env and legacy source branches and a bad-`--user` ACCOUNT_NOT_FOUND case. The stored-vs-legacy precedence (incl. the renamed-legacy regression) is now covered directly in auth-provider.test.ts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`current` was wrapping the store in an `activeAccount` adapter + closure-stashed source just to feed cli-core's generic account attacher, which only models stored accounts. Resolve the source directly in the command action instead — same behaviour and discriminated `--json` envelope, ~26 fewer lines, and no store-wrapping indirection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
🎉 This PR is included in version 1.9.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
2 participants
Summary
Surfaces the multi-account storage (already shipped in #77 —
users[]config, keyring,--userselector,withUserRefAwarestore-wrap) to users via a newol accountcommand group, wiring cli-core's four generic account attachers.ol account list(and bareol account, via default subcommand) — lists stored accounts with an accessibility-aware default marker;--jsonemits a{ accounts, default }envelope,--ndjsonstreams one per line.ol account use <id|name>— sets the default account used when--useris omitted.ol account current— shows the active account, honoursol --user <ref>, and reports theOUTLINE_API_TOKENenv source and legacy single-user sessions viaonNotAuthenticated(rather than rendering them as a blank stored account).ol account remove <id|name>— clears the keyring + config entry and surfaces any keyring-fallback warning.{ id, label, teamName, baseUrl, isDefault }).Reuses existing plumbing:
createOutlineTokenStore,withUserRefAware,matchOutlineAccount,logTokenStorageResult. No changes to the env/legacy resolution logic beyond a read-only wrapper forcurrent.Test plan
npm run type-checknpm run test— 200/200 pass, incl. newsrc/commands/account.test.ts(list/use/current/remove, env-token, legacy, empty, not-authenticated)npm run format:checkcleannpm run check:skill-syncin syncnode dist/index.js account --helprenders all four subcommands + examplesol account,ol account use,ol --user <ref> account current,OUTLINE_API_TOKEN=… ol account current🤖 Generated with Claude Code