fix(cli): tailor UNAUTHORIZED guidance by credential + scope (TNT-142)#102
Merged
Conversation
handleError rewrote every UNAUTHORIZED into "Session expired. Run 'me
login'" and cleared the stored token, gated only on a sessionServer
opt — it never checked which credential was actually used. So an
agent/PAT caller (ME_API_KEY) hitting a 401 was told to run `me login`
(irrelevant; clearTokens is a no-op on an env key), and a logged-in
human with a stale/typo'd active space got silently logged out: a
space-scoped 401 ("unknown space", deliberately generic to avoid
enumeration) is indistinguishable from an expired session.
Add a pure, tested `describeAuthError(error, creds, scope)` that picks
the message and whether to clear the session, by credential type and
command scope (account = user RPC, space = memory RPC):
- api key: never "me login", never clear a token; point at ME_API_KEY
(and, for space commands, the active space + `me space list`).
- session/account: genuine expiry — clear the token, prompt re-login
(unchanged).
- session/space: ambiguous — keep the token, mention both the space
(`me space list`) and a possible re-login.
handleError now takes `{ creds, scope }` (default account). Call sites
updated by the rule "builds a memory client → scope: space"; the
previously-unhandled memory commands (memory.*, import*, pack) now also
get the key+space hint instead of the raw "Invalid credentials".
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes misleading CLI handling of UNAUTHORIZED (HTTP 401) by tailoring guidance based on credential type (session vs ME_API_KEY) and command scope (account/user-RPC vs space/memory-RPC), avoiding unnecessary session clearing when a 401 could be caused by an invalid/unknown active space.
Changes:
- Adds
describeAuthError(error, creds, scope)to convert 401s into actionable messages and decide whether to clear the stored session token. - Updates
handleErrorto use{ creds, scope }(defaultaccount) and only clear tokens on unambiguous session-expiry cases. - Updates CLI command call sites to pass the correct scope, and adds unit tests covering the new auth-description logic.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/util.ts | Introduces AuthScope + describeAuthError, and updates handleError to tailor 401 handling and token clearing. |
| packages/cli/util.test.ts | Adds unit tests for describeAuthError branches (api key vs session; account vs space). |
| packages/cli/commands/whoami.ts | Passes resolved credentials into handleError for account-scoped user-RPC errors. |
| packages/cli/commands/space.ts | Passes { creds } for account-scoped space commands and { creds, scope: "space" } for invite (memory-RPC) commands. |
| packages/cli/commands/pack.ts | Routes pack errors through handleError with space scope to improve 401 guidance. |
| packages/cli/commands/memory.ts | Routes memory RPC errors through handleError with space scope. |
| packages/cli/commands/import.ts | Routes import RPC errors through handleError with space scope. |
| packages/cli/commands/import-git.ts | Routes git-import RPC errors through handleError with space scope. |
| packages/cli/commands/group.ts | Updates group commands to pass { creds, scope: "space" } into handleError. |
| packages/cli/commands/apikey.ts | Passes { creds } into handleError for account-scoped apiKey user-RPC calls. |
| packages/cli/commands/agent.ts | Uses account scope for user-RPC calls and space scope for memory-RPC calls. |
| packages/cli/commands/access.ts | Updates access commands to pass { creds, scope: "space" } into handleError. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: John Pruitt <jgpruitt@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes TNT-142. The CLI's
handleErrorrewrote anyUNAUTHORIZED(HTTP 401) into "Session expired. Run 'me login' to sign in again." and cleared the stored token — gated only on asessionServeropt, never checking which credential was actually used. Two real defects:ME_API_KEY(agent key or PAT),me loginis irrelevant and there's no session token (clearTokensis a no-op on the env key). Surfaced while testing feat(auth): broaden agent API-key utility in the CLI; authorize per-method on the user RPC (TNT-139) #101 / TNT-139, which broadened agent-key use across CLI commands:me group mine→ "Session expired" even though the real cause was an unresolved active space.UNAUTHORIZED("Invalid credentials") when the space doesn't resolve — deliberately generic to avoid space enumeration, and indistinguishable from a real credential failure. For a logged-in human a stale/typo'd active space therefore wiped their session and told them to re-login.The CLI can't tell "unknown space" from "bad credential" apart on the wire, so the guidance is tailored by credential type and command scope instead.
Changes
util.ts: new pure, exporteddescribeAuthError(error, creds, scope)→{ message, clearSession } | null(nullfor non-401s, so the raw server message is kept):ME_API_KEY; never clear a session.ME_API_KEYinvalid or active space'<slug>'missing/inaccessible →me space list.me space list) andme login; do not clear the token.handleErroropts changed{ sessionServer }→{ creds?, scope? }(defaultaccount); clears the token only whenclearSessionis true.scope: "space": all ofgroup/access, the memory-client handlers inagent/space, account scope forwhoami/apikey/rest. The previously-unhandled memory commands (memory.*,import*,pack) now also get the key+space hint instead of the raw "Invalid credentials".util.test.tsgains 6describeAuthErrorcases (all 4 branches, slug present/absent, non-401 passthrough).Server-side behavior is unchanged — the intentionally-generic 401 stays (enumeration safety); the CLI message now covers both causes.
Verification
./bun run check— typecheck + lint + unit all green (the one lint warning inimport-git-hook.tsis pre-existing and untouched).ME_API_KEY+ badME_SPACE→ space/key hint (not "Session expired");me whoamistill works; session + bad space → token not cleared.