diff --git a/packages/cli/commands/access.ts b/packages/cli/commands/access.ts index 18a4051d..dc1a71a5 100644 --- a/packages/cli/commands/access.ts +++ b/packages/cli/commands/access.ts @@ -74,7 +74,7 @@ function createAccessGrantCommand(): Command { }, ); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds, scope: "space" }); } }, ); @@ -111,7 +111,7 @@ function createAccessRmGrantCommand(): Command { } }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -162,7 +162,7 @@ function createAccessListCommand(): Command { ); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds, scope: "space" }); } }); } diff --git a/packages/cli/commands/agent.ts b/packages/cli/commands/agent.ts index e25ad19b..77085868 100644 --- a/packages/cli/commands/agent.ts +++ b/packages/cli/commands/agent.ts @@ -53,7 +53,7 @@ function createAgentListCommand(): Command { ); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds }); } }); } @@ -78,7 +78,7 @@ function createAgentCreateCommand(): Command { ); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds }); } }); } @@ -102,7 +102,7 @@ function createAgentRenameCommand(): Command { clack.log.success(`Renamed agent → '${newName}'`); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds }); } }); } @@ -127,7 +127,7 @@ function createAgentDeleteCommand(): Command { else clack.log.warn("Agent not found."); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds }); } }); } @@ -154,7 +154,7 @@ function createAgentAddCommand(): Command { clack.log.info("Mint a key with 'me apikey create'."); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -194,7 +194,7 @@ function createAgentSpacesCommand(): Command { ); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds }); } }); } @@ -226,7 +226,7 @@ function createAgentGroupsCommand(): Command { ); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds, scope: "space" }); } }); } diff --git a/packages/cli/commands/apikey.ts b/packages/cli/commands/apikey.ts index 3cb55d66..a3a6ea8d 100644 --- a/packages/cli/commands/apikey.ts +++ b/packages/cli/commands/apikey.ts @@ -90,7 +90,7 @@ function createApiKeyCreateCommand(): Command { ); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds }); } }, ); @@ -122,7 +122,7 @@ function createApiKeyListCommand(): Command { ); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds }); } }); } @@ -152,7 +152,7 @@ function createApiKeyGetCommand(): Command { console.log(` Expires: ${apiKey.expiresAt ?? "(never)"}`); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds }); } }); } @@ -188,7 +188,7 @@ function createApiKeyDeleteCommand(): Command { else clack.log.warn("API key not found."); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds }); } }); } diff --git a/packages/cli/commands/group.ts b/packages/cli/commands/group.ts index 45434d78..88f1e459 100644 --- a/packages/cli/commands/group.ts +++ b/packages/cli/commands/group.ts @@ -88,7 +88,7 @@ function createGroupListCommand(): Command { ); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -111,7 +111,7 @@ function createGroupCreateCommand(): Command { clack.log.success(`Created group '${name}' (${id})`); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -136,7 +136,7 @@ function createGroupRenameCommand(): Command { clack.log.success(`Renamed group → '${newName}'`); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -162,7 +162,7 @@ function createGroupDeleteCommand(): Command { else clack.log.warn("Group not found."); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -195,7 +195,7 @@ function createGroupAddCommand(): Command { ); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -224,7 +224,7 @@ function createGroupRemoveCommand(): Command { else clack.log.warn("Member not in group."); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -260,7 +260,7 @@ function createGroupMembersCommand(): Command { ); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -293,7 +293,7 @@ function createGroupMineCommand(): Command { ); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds, scope: "space" }); } }); } diff --git a/packages/cli/commands/import-git.ts b/packages/cli/commands/import-git.ts index acc80d41..3aacf3e5 100644 --- a/packages/cli/commands/import-git.ts +++ b/packages/cli/commands/import-git.ts @@ -196,7 +196,7 @@ export async function runGitImport( range = `${highWater}..${rev}`; } } catch (error) { - handleError(error, fmt); + handleError(error, fmt, { creds, scope: "space" }); } } @@ -245,7 +245,7 @@ export async function runGitImport( } } catch (error) { progress?.stop(); - handleError(error, fmt); + handleError(error, fmt, { creds, scope: "space" }); } // Dedup on the commit sha (the (tree, name) key), not the random id. diff --git a/packages/cli/commands/import.ts b/packages/cli/commands/import.ts index 8c4ee9fa..ab64e8d2 100644 --- a/packages/cli/commands/import.ts +++ b/packages/cli/commands/import.ts @@ -215,7 +215,7 @@ export async function runAgentImport( ); } catch (error) { progress?.stop(); - handleError(error, fmt); + handleError(error, fmt, { creds, scope: "space" }); } finally { progress?.stop(); } diff --git a/packages/cli/commands/memory.ts b/packages/cli/commands/memory.ts index 3ad11add..cc7dd930 100644 --- a/packages/cli/commands/memory.ts +++ b/packages/cli/commands/memory.ts @@ -248,7 +248,7 @@ function createMemoryCreateCommand(): Command { if (memory.name) console.log(` Name: ${memory.name}`); }); } catch (error) { - handleError(error, fmt); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -308,7 +308,7 @@ function createMemoryGetCommand(): Command { console.log(`\n${header}\n\n${rendered.trimEnd()}\n`); } catch (error) { - handleError(error, fmt); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -464,7 +464,7 @@ function createMemorySearchCommand(): Command { ); }); } catch (error) { - handleError(error, fmt); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -544,7 +544,7 @@ function createMemoryUpdateCommand(): Command { clack.log.success(`Updated memory ${memory.id}`); }); } catch (error) { - handleError(error, fmt); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -595,7 +595,7 @@ function createMemoryDeleteCommand(): Command { // Couldn't probe the subtree — fall through to the original error. } } - handleError(error, fmt); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -657,7 +657,7 @@ function createMemoryDeltreeCommand(): Command { ); }); } catch (error) { - handleError(error, fmt); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -678,7 +678,7 @@ function createMemoryEditCommand(): Command { try { await editMemory(client, id); } catch (error) { - handleError(error, fmt); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -712,7 +712,7 @@ function createMemoryCountCommand(): Command { } }); } catch (error) { - handleError(error, fmt); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -748,7 +748,7 @@ function createMemoryTreeCommand(): Command { } }); } catch (error) { - handleError(error, fmt); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -822,7 +822,7 @@ function createMemoryMoveCommand(): Command { ); }); } catch (error) { - handleError(error, fmt); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -896,7 +896,7 @@ function createMemoryCopyCommand(): Command { ); }); } catch (error) { - handleError(error, fmt); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -1086,7 +1086,7 @@ function createMemoryExportCommand(): Command { } } } catch (error) { - handleError(error, fmt); + handleError(error, fmt, { creds, scope: "space" }); } }); } diff --git a/packages/cli/commands/pack.ts b/packages/cli/commands/pack.ts index 459f519f..a5c60bdd 100644 --- a/packages/cli/commands/pack.ts +++ b/packages/cli/commands/pack.ts @@ -353,7 +353,7 @@ function createPackInstallCommand(): Command { } }); } catch (error) { - handleError(error, fmt); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -419,7 +419,7 @@ function createPackListCommand(): Command { ); }); } catch (error) { - handleError(error, fmt); + handleError(error, fmt, { creds, scope: "space" }); } }); } diff --git a/packages/cli/commands/space.ts b/packages/cli/commands/space.ts index b455f4e9..980f71ec 100644 --- a/packages/cli/commands/space.ts +++ b/packages/cli/commands/space.ts @@ -144,7 +144,7 @@ function createSpaceListCommand(): Command { }, ); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds }); } }); } @@ -169,7 +169,7 @@ function createSpaceUseCommand(): Command { clack.log.success(`Active space: ${space.name} (${space.slug})`); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds }); } }); } @@ -195,7 +195,7 @@ function createSpaceCreateCommand(): Command { clack.log.info("It is now your active space."); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds }); } }); } @@ -227,7 +227,7 @@ function createSpaceRenameCommand(): Command { ); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds }); } }); } @@ -282,7 +282,7 @@ function createSpaceDeleteCommand(): Command { } }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds }); } }); } @@ -340,7 +340,7 @@ function createSpaceInviteListCommand(): Command { ); }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -367,7 +367,7 @@ function createSpaceInviteRevokeCommand(): Command { } }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds, scope: "space" }); } }); } @@ -420,7 +420,7 @@ function createSpaceInviteCommand(): Command { } }); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds, scope: "space" }); } }); invite.addCommand(createSpaceInviteListCommand()); diff --git a/packages/cli/commands/whoami.ts b/packages/cli/commands/whoami.ts index 44005194..3117042b 100644 --- a/packages/cli/commands/whoami.ts +++ b/packages/cli/commands/whoami.ts @@ -45,7 +45,7 @@ export function createWhoamiCommand(): Command { }, ); } catch (error) { - handleError(error, fmt, { sessionServer: creds.server }); + handleError(error, fmt, { creds }); } }); } diff --git a/packages/cli/util.test.ts b/packages/cli/util.test.ts index a6e20b5c..ae537b8f 100644 --- a/packages/cli/util.test.ts +++ b/packages/cli/util.test.ts @@ -4,12 +4,21 @@ * Tests the space-model resolution functions with mocked clients. */ import { describe, expect, mock, test } from "bun:test"; -import type { MemoryClient, UserClient } from "@memory.build/client"; +import { + type MemoryClient, + RpcError, + type UserClient, +} from "@memory.build/client"; +import type { ResolvedCredentials } from "./credentials.ts"; // Dynamic import to avoid pulling in @clack/prompts at top level (it touches // process.stdin). -const { resolveSpacePrincipalId, resolveAgentId, shellTildeExpansionHint } = - await import("./util.ts"); +const { + resolveSpacePrincipalId, + resolveAgentId, + shellTildeExpansionHint, + describeAuthError, +} = await import("./util.ts"); const UUID = "019d694f-79f6-7595-8faf-b70b01c11f98"; @@ -156,3 +165,90 @@ describe("shellTildeExpansionHint", () => { ).toBeNull(); }); }); + +// ============================================================================= +// describeAuthError +// ============================================================================= + +const SERVER = "https://api.example.com"; +const unauthorized = () => + new RpcError(-32000, "Invalid credentials", { code: "UNAUTHORIZED" }); +const creds = ( + over: Partial = {}, +): ResolvedCredentials => ({ + server: SERVER, + loggedIn: false, + ...over, +}); + +describe("describeAuthError", () => { + test("returns null for a non-UNAUTHORIZED error (falls back to raw message)", () => { + const notFound = new RpcError(-32000, "Nope", { code: "NOT_FOUND" }); + expect( + describeAuthError(notFound, creds({ apiKey: "me.k" }), "space"), + ).toBeNull(); + expect(describeAuthError(new Error("boom"), creds(), "account")).toBeNull(); + }); + + test("api key, account scope: blames the key, never clears a session", () => { + const r = describeAuthError( + unauthorized(), + creds({ apiKey: "me.k", activeSpace: "abc123" }), + "account", + ); + expect(r).not.toBeNull(); + expect(r?.clearSession).toBe(false); + expect(r?.message).toContain("ME_API_KEY"); + expect(r?.message).not.toContain("me login"); + // Account scope doesn't mention the space even when one is set. + expect(r?.message).not.toContain("abc123"); + }); + + test("api key, space scope: mentions the active space and 'me space list'", () => { + const r = describeAuthError( + unauthorized(), + creds({ apiKey: "me.k", activeSpace: "abc123" }), + "space", + ); + expect(r?.clearSession).toBe(false); + expect(r?.message).toContain("ME_API_KEY"); + expect(r?.message).toContain("'abc123'"); + expect(r?.message).toContain("me space list"); + expect(r?.message).not.toContain("me login"); + }); + + test("session, account scope: genuine expiry — clears the token, prompts login", () => { + const r = describeAuthError( + unauthorized(), + creds({ loggedIn: true }), + "account", + ); + expect(r?.clearSession).toBe(true); + expect(r?.message).toBe( + "Session expired. Run 'me login' to sign in again.", + ); + }); + + test("session, space scope: ambiguous — keeps the token, mentions space and login", () => { + const r = describeAuthError( + unauthorized(), + creds({ loggedIn: true, activeSpace: "abc123" }), + "space", + ); + // Must NOT clear the session over what may just be a stale active space. + expect(r?.clearSession).toBe(false); + expect(r?.message).toContain("'abc123'"); + expect(r?.message).toContain("me space list"); + expect(r?.message).toContain("me login"); + }); + + test("omits the slug when no active space is set", () => { + const r = describeAuthError( + unauthorized(), + creds({ apiKey: "me.k" }), + "space", + ); + expect(r?.message).not.toContain("''"); + expect(r?.message).toContain("me space list"); + }); +}); diff --git a/packages/cli/util.ts b/packages/cli/util.ts index 1c9f3e6c..549e3a21 100644 --- a/packages/cli/util.ts +++ b/packages/cli/util.ts @@ -254,18 +254,81 @@ function isUnauthorized(error: unknown): boolean { return isAppErrorCode(error, "UNAUTHORIZED"); } +/** + * Which credential surface a command authenticates against — drives the + * `UNAUTHORIZED` guidance: + * - `account`: a user-RPC call (e.g. whoami, agent.*, apiKey.*, space.*). A 401 here + * is unambiguously a credential problem. + * - `space`: a memory-RPC call (e.g. group.*, access.*, memory.*, principal.*, grant.*, invite.*). A 401 here is + * a bad credential and for an unknown/unset space (it resolves the space + * before validating the credential, and keeps the message generic to avoid + * space enumeration). So the message must mention the space, and we must not + * clear a session token over what may just be a stale active space. + */ +export type AuthScope = "account" | "space"; + +/** + * Classify an `UNAUTHORIZED` RPC error into an actionable message and whether the + * stored session token should be cleared. Returns `null` for any other error, so + * the caller falls back to the raw server message. + * + * The CLI can't tell a bad credential from an unknown space apart on the wire + * (both are a generic 401), so the guidance is tailored by *credential type* and + * *command scope* instead: + * - api key (ME_API_KEY): never a session, so never "run me login" and never + * clear a token. Point at ME_API_KEY (and, for space commands, the space). + * - session: only the account-scoped 401 is a true session expiry (clear the + * token, prompt re-login). A space-scoped 401 is ambiguous — keep the token + * and mention both the space and a possible re-login. + */ +export function describeAuthError( + error: unknown, + creds: ResolvedCredentials, + scope: AuthScope, +): { message: string; clearSession: boolean } | null { + if (!isUnauthorized(error)) return null; + + const space = creds.activeSpace ? ` '${creds.activeSpace}'` : ""; + + if (creds.apiKey) { + // Api-key auth — there is no session to expire or clear. + const message = + scope === "space" + ? `Not authorized. Check that ME_API_KEY is valid, and that the active space${space} exists and is accessible — run 'me space list'.` + : "Not authorized. Your ME_API_KEY is invalid or expired."; + return { message, clearSession: false }; + } + + if (scope === "space") { + // Ambiguous: a bad space slug looks identical to an expired session. Don't + // clear the token over a stale active space. + return { + message: `Not authorized. The active space${space} may not exist or be accessible (run 'me space list'), or your login may have expired (run 'me login').`, + clearSession: false, + }; + } + + // Account-scoped session call — a genuine session expiry. + return { + message: "Session expired. Run 'me login' to sign in again.", + clearSession: true, + }; +} + /** * Handle an error from an RPC call. Formats per output mode and exits. * - * Pass `opts.sessionServer` for commands that authenticate with a session - * token. When the server returns UNAUTHORIZED we clear the stored token for - * that server (so the next command says "Not logged in") and replace the - * generic message with an actionable "Run 'me login' to sign in again." hint. + * Pass `opts.creds` for commands that authenticate (every RPC command). On an + * `UNAUTHORIZED` the message is tailored by credential type and `opts.scope` + * (default `account`) — see {@link describeAuthError}. A genuine session expiry + * (account-scoped, session auth) clears the stored token so the next command + * says "Not logged in"; api-key auth and ambiguous space-scoped 401s leave it + * intact. */ export function handleError( error: unknown, fmt: OutputFormat, - opts?: { sessionServer?: string }, + opts?: { creds?: ResolvedCredentials; scope?: AuthScope }, ): never { let msg = error instanceof RpcError @@ -275,10 +338,13 @@ export function handleError( : String(error); let code: string | undefined; - if (opts?.sessionServer && isUnauthorized(error)) { - clearTokens(opts.sessionServer); - msg = "Session expired. Run 'me login' to sign in again."; - code = "UNAUTHORIZED"; + if (opts?.creds) { + const auth = describeAuthError(error, opts.creds, opts.scope ?? "account"); + if (auth) { + if (auth.clearSession) clearTokens(opts.creds.server); + msg = auth.message; + code = "UNAUTHORIZED"; + } } if (fmt === "text") {