auth attachers: accept global --user ref via a getRequestedUserRef callback
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 0
- Forks
- 0
- Avg merge
- 9h 16m
- Merged PRs (30d)
- 15
Description
Problem
cli-core's auth attachers (attachLoginCommand, attachLogoutCommand, attachStatusCommand, attachTokenViewCommand) all declare a per-command --user <ref> flag via attachUserFlag. Two consumers (todoist-cli and twist-cli) also want the global form — td --user 42 task list, tw --user 42 auth status — accepted before any subcommand. cli-core gives us the parsing primitives (parseGlobalArgs, stripUserFlag), but the integration with the auth attachers is reinvented in every consumer.
In todoist-cli (PR #341) and twist-cli (PR #228, expanded by PR #231) the same ~50-LOC dance is hand-rolled:
parseGlobalArgs(process.argv)to extract--userstripUserFlag(process.argv.slice(2))so commander doesn't reject--userat the root- Cache the parsed value (
getRequestedUserRef()accessor in a localglobal-args.ts) - Wrap the
TokenStorewith awithUserRefAware(store, requestedRef)adapter that substitutes the cached ref when commander didn't see one - Add an existence pre-check in the wrapper so
tw --user <wrong> auth logoutsurfacesACCOUNT_NOT_FOUNDinstead of cli-core's silentclear()no-op
The root cause: cli-core's attachLogoutCommand deliberately preflights store.active(ref) to convert a ref miss into ACCOUNT_NOT_FOUND — but only when ref !== undefined (where ref = extractUserRef(cmd)). When the global flag has been stripped from argv before commander runs, cmd.user === undefined, so the preflight is skipped and the consumer has to compensate. The preflight gating is correct given the information available to cli-core; cli-core just has no way of knowing about a ref that was never on the option list.
This is now the rule-of-three smell triggering on count two. A third consumer would rediscover the same pattern from scratch.
Proposed change
Add an optional getRequestedUserRef callback to each auth attacher's options:
type AttachLogoutCommandOptions<TAccount> = {
store: TokenStore<TAccount>
/** ... existing fields ... */
/**
* Optional source of the global `--user <ref>` value when the consumer
* strips it from argv before commander runs. cli-core's preflight uses
* this as the fallback for `ref` so a non-matching global ref surfaces
* as `ACCOUNT_NOT_FOUND` instead of a silent `clear()` no-op.
*/
getRequestedUserRef?: () => AccountRef | undefined
}
Same option on AttachStatusCommandOptions, AttachTokenViewCommandOptions, and (optionally) AttachLoginCommandOptions for consistency.
Internally, every place that currently reads extractUserRef(cmd) becomes:
const ref = extractUserRef(cmd) ?? options.getRequestedUserRef?.()
requireSnapshotForRef then sees a real ref on the global-flag path, the preflight catches the miss, and the consumer no longer needs to wrap the store at all.
Alternative (rejected): cli-core owns argv massaging
A more opinionated attachGlobalUserFlag(program) that strips argv + caches at startup. Cleaner for consumers (one call instead of three) but cli-core would mutate process.argv — surprising for a library and harder to test in isolation. The callback shape above keeps cli-core pure.
Downstream simplification
Once this lands, both consumer CLIs can drop ~30 LOC and one whole file each.
twist-cli (Doist/twist-cli)
- Delete
src/commands/auth/store-wrap.tsentirely (thewithUserRefAwarewrapper). - Update
src/commands/auth/index.tsto stop wrapping the store; passgetRequestedUserRefstraight into each attacher's options:const store = createTwistTokenStore() const userRef = getRequestedUserRef() attachTwistLoginCommand(auth, store) attachTwistLogoutCommand(auth, store, { getRequestedUserRef: () => userRef }) attachTwistStatusCommand(auth, store, { getRequestedUserRef: () => userRef }) attachTokenViewCommand(tokenCmd, { name: 'view', store, envVarName: TOKEN_ENV_VAR, getRequestedUserRef: () => userRef }) - Delete the existence-check tests in
src/commands/auth/auth.test.ts(blocks tw --user <wrong> auth logout with ACCOUNT_NOT_FOUND ...) — cli-core would own that assertion via its own attacher tests. src/index.tsargv strip +getRequestedUserRef()accessor insrc/lib/global-args.tsstay (still needed to extract the value from argv pre-commander).
todoist-cli (Doist/todoist-cli)
- Delete
src/commands/auth/store-wrap.tsentirely. - Update
src/commands/auth/index.ts: stop buildingrefAware, passgetRequestedUserRefcallback into each attacher instead. - Delete the
withUserRefAwaretests undersrc/lib/auth-store.test.tsand the--user from global argscases insrc/commands/auth/auth.test.ts. - todoist's argv strip in
src/index.tsstays.
Acceptance criteria
- New
getRequestedUserRef?: () => AccountRef | undefinedoption onAttachLogoutCommandOptions,AttachStatusCommandOptions,AttachTokenViewCommandOptions,AttachLoginCommandOptions. - Each attacher resolves
ref = extractUserRef(cmd) ?? options.getRequestedUserRef?.()before callingrequireSnapshotForRef/ the store methods. requireSnapshotForRefalready throwsACCOUNT_NOT_FOUNDfor explicit non-null refs that resolve tonull— no change needed there.- Cli-core tests cover the new flag for at least logout (the preflight-driven
ACCOUNT_NOT_FOUNDpath) and one read path (status or token view). - No breaking changes to existing consumers — the option is purely additive.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the implementations of attachLoginCommand, attachLogoutCommand, attachStatusCommand, and attachTokenViewCommand, tracing extractUserRef and requireSnapshotForRef. Review the existing cli-core attacher tests, then cover the global-ref logout preflight and one read path; done means all four option types accept the callback without breaking existing consumers.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- cli
- Issue type
- Feature
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 70/100