From 852416110e34946d7677b13f633bc20237395b8e Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 6 Oct 2026 05:44:04 +0000 Subject: [PATCH] An expired session exits 3 with one line saying to run quak login (closes #164) A 401 from the server that ends a command reaches run in src/cli-run.ts as an ApiError; run prints one line saying to run "quak login" and exits 3. quak backup meets it on its first refresh, before it touches any file. For the commands that load the saved session, a missing or corrupt session file keeps its message and also exits 3. Judgement call: quak logout is unchanged; it handles its own errors and deletes the session file whatever the server answers. Judgement call: quak backup still prints its two progress lines before the error line. Model: opus-5-5 --- README.md | 18 ++++++-- TODO.md | 10 +++++ src/cli-commands.ts | 22 +++++----- src/cli-run.ts | 20 ++++++--- test/cli/commands.test.ts | 86 +++++++++++++++++++++++++++++++-------- test/cli/run.test.ts | 23 +++++++++++ 6 files changed, 142 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 354c53d..f8825a6 100644 --- a/README.md +++ b/README.md @@ -516,8 +516,17 @@ The CLI stores the snapshot at the platform-appropriate data directory via `0600`. The key material is stored in cleartext in the JSON; treat this file as you would treat the password itself. A missing file is reported as "not logged in"; a file that exists but is corrupt is reported as such, naming the bad -field. Both exit with status 1, except that `quak logout` with no file says -there is no session and exits 0. +field. When the refresh a command starts with gets HTTP 401 from the server, +because it no longer accepts the saved session's token, the command prints one +line, `quak: the saved session is no longer valid; run "quak login"`, with no +stack trace. All three exit with status 3, which means the user must run +`quak login` again. `quak logout` is the exception: with no file it says there +is no session and exits 0, and it handles a corrupt file or a failed server call +as described below. `quak backup` meets an expired session on the refresh that +starts every run, before it touches any file. A session that stops working +partway through a backup instead fails each remaining file into `failures.json`, +so that run exits 1 and the next one stops at its refresh with status 3. No +command but `quak login` ever prompts. `quak logout` ends the session on the server, so the token in `session.json` stops working even in a copy of the file, and then deletes the file. If the @@ -551,8 +560,9 @@ library. The read commands — `collections`, `files`, `get`, `get-thumb`, `helper fix-missing-thumbnails` — force a fresh server round-trip before they answer, so they report current account state rather than whatever the cache last held. If that round-trip fails, the command prints the error on one line and -exits 1. `--cache-dir` overrides where the cache lives; without it each account -gets its own directory under the per-user cache path. +exits 1, or 3 when the server no longer accepts the saved session (see "Session +handling"). `--cache-dir` overrides where the cache lives; without it each +account gets its own directory under the per-user cache path. `get` and `get-thumb` resolve the file by ID directly, so `--collection` is accepted for backward compatibility but ignored. For a live photo, `get` writes diff --git a/TODO.md b/TODO.md index c8baf5c..72da5df 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,16 @@ declares one. # Completed Steps +- 2026-10-06: When the server answers HTTP 401 and that ends a command that + loads the saved session, because the server no longer accepts its token, the + command prints one line, + `quak: the saved session is no longer valid; run "quak login"`, and exits 3 + (issue 164). `quak backup` meets that 401 on the refresh it starts with, + before it touches any file. For the same commands, a missing or corrupt + session file keeps its message and also exits 3, so a cron job can tell that + the user must log in again. `quak logout` is unchanged. `run` in + `src/cli-run.ts` recognises the 401, which reaches it unchanged. + - 2026-10-06: `quak backup` writes the account and album records `backup-metadata` writes (issue 166): `account.json` with the account's `email` and `userID`, and in each album's JSON its `ownerID`, `isShared`, diff --git a/src/cli-commands.ts b/src/cli-commands.ts index 3ea8f34..30aa4ee 100644 --- a/src/cli-commands.ts +++ b/src/cli-commands.ts @@ -73,7 +73,9 @@ export const saveSession = ( ); }; -// The saved client, or undefined after telling the user why there is none. +// The saved client, or undefined after telling the user why there is none. The +// command then exits 3, the code for "log in again", as `run` in `cli-run.ts` +// does when the server no longer accepts the saved session. const requireSession = (ctx: CliContext): Client | undefined => { let client: Client | null; try { @@ -150,7 +152,7 @@ export const loginCommand = async (ctx: CliContext): Promise => { export const whoamiCommand = async (ctx: CliContext): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; const info = client.whoami(); ctx.stdout.write(JSON.stringify(info) + "\n"); return 0; @@ -201,7 +203,7 @@ export const collectionsCommand = async ( ): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; const lib = await openReadLibrary(ctx, client); try { // Force a server round-trip and list in enumeration order (issue #36 @@ -243,7 +245,7 @@ export const filesCommand = async ( ): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; const collectionID = Number(opts.collection); if (!Number.isFinite(collectionID)) { ctx.stderr.write("Invalid collection ID\n"); @@ -285,7 +287,7 @@ export const getCommand = async ( ): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; const fileID = Number(fileIDStr); if (!Number.isFinite(fileID)) { ctx.stderr.write("Invalid file ID\n"); @@ -343,7 +345,7 @@ export const getThumbCommand = async ( ): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; const fileID = Number(fileIDStr); if (!Number.isFinite(fileID)) { ctx.stderr.write("Invalid file ID\n"); @@ -380,7 +382,7 @@ export const backupMetadataCommand = async ( ): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; const lib = await openReadLibrary(ctx, client); try { // Refresh first so the dump holds current account state, not what the @@ -403,7 +405,7 @@ export const backupCommand = async ( ): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; ctx.stderr.write("Starting backup...\n"); // The precache is off: the backup fetches what it needs, and must not @@ -453,7 +455,7 @@ export const listMissingThumbnailsCommand = async ( ): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; const lib = await openReadLibrary(ctx, client); try { // Refresh first so files added since the cache was written are @@ -491,7 +493,7 @@ export const fixMissingThumbnailsCommand = async ( ): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; const lib = await openReadLibrary(ctx, client); try { // Refresh first so files added since the cache was written are found; diff --git a/src/cli-run.ts b/src/cli-run.ts index c668b49..027bc40 100644 --- a/src/cli-run.ts +++ b/src/cli-run.ts @@ -1,12 +1,15 @@ // Runs one CLI command for `bin/quak.ts` and exits with its code. import type { Writable } from "node:stream"; +import { ApiError } from "./api/client.js"; // Run a command and exit with its code once stdout/stderr have drained. // Exiting before the drain can truncate piped output, and the library can keep // the event loop alive after a command returns, so a plain return could hang. // An error the command throws is printed as one `quak: MESSAGE` line, without -// the stack trace, and exits 1. +// the stack trace, and exits 1. A 401 from the server means it no longer +// accepts the saved session: that prints one line saying to log in again and +// exits 3, as a missing or corrupt session file does. export const run = async ( command: Promise, stdout: Writable, @@ -17,10 +20,17 @@ export const run = async ( try { code = await command; } catch (err) { - stderr.write( - `quak: ${err instanceof Error ? err.message : String(err)}\n`, - ); - code = 1; + if (err instanceof ApiError && err.status === 401) { + stderr.write( + `quak: the saved session is no longer valid; run "quak login"\n`, + ); + code = 3; + } else { + stderr.write( + `quak: ${err instanceof Error ? err.message : String(err)}\n`, + ); + code = 1; + } } const pending = [stdout, stderr].filter((s) => s.writableLength > 0); if (pending.length === 0) { diff --git a/test/cli/commands.test.ts b/test/cli/commands.test.ts index fa24325..633cc35 100644 --- a/test/cli/commands.test.ts +++ b/test/cli/commands.test.ts @@ -230,20 +230,30 @@ describe("session file", () => { expect(JSON.parse(readFileSync(path, "utf-8"))).toEqual(snapshot); }); - it("a missing session exits 1 with 'Not logged in'", async () => { + it("a missing session exits 3 with 'Not logged in' from every command that needs one", async () => { const ctx = { ...context(), loadSession }; - expect(await whoamiCommand(ctx)).toBe(1); - expect(stderr.text).toBe( + const dir = join(root, "backup"); + expect(await whoamiCommand(ctx)).toBe(3); + expect(await collectionsCommand(ctx, {})).toBe(3); + expect(await filesCommand(ctx, { collection: "1" })).toBe(3); + expect(await getCommand(ctx, "100", {})).toBe(3); + expect(await getThumbCommand(ctx, "100", {})).toBe(3); + expect(await backupMetadataCommand(ctx, dir, {})).toBe(3); + expect(await backupCommand(ctx, dir, {})).toBe(3); + expect(await listMissingThumbnailsCommand(ctx, {})).toBe(3); + expect(await fixMissingThumbnailsCommand(ctx, {})).toBe(3); + const notLoggedIn = `Not logged in. Run "quak login" first.\n` + - `Session file: ${join(ctx.sessionDir, "session.json")}\n`, - ); + `Session file: ${join(ctx.sessionDir, "session.json")}\n`; + expect(stderr.text).toBe(notLoggedIn.repeat(9)); expect(stdout.text).toBe(""); + expect(existsSync(dir)).toBe(false); }); - it("a corrupt session exits 1 and says it is corrupt", async () => { + it("a corrupt session exits 3 and says it is corrupt", async () => { const ctx = { ...context(), loadSession }; saveSession(ctx.sessionDir, snapshot); - expect(await collectionsCommand(ctx, {})).toBe(1); + expect(await collectionsCommand(ctx, {})).toBe(3); expect(stderr.text).toContain("is corrupt"); expect(stderr.text).toContain( `Run "quak logout" and then "quak login" to replace it.\n`, @@ -748,15 +758,12 @@ describe("backup", () => { ); }); - it("exits 1 with the error on one line when the refresh fails", async () => { - const client = { - ...fakeClient(), - collectionsSince: async () => { - throw new Error("HTTP 401 from server"); - }, - } as unknown as Client; - const dir = join(root, "backup"); - // Through `run`, as `bin/quak.ts` does, which prints a thrown error. + // Runs `backup` through `run`, as `bin/quak.ts` does, which prints a thrown + // error; returns the exit code and what `run` printed. + const backupThroughRun = async ( + ctx: CliContext, + dir: string, + ): Promise<{ code: number; runText: string }> => { const runStderr = new PassThrough(); let runText = ""; runStderr.on("data", (chunk: Buffer) => { @@ -764,14 +771,57 @@ describe("backup", () => { }); const code = await new Promise((resolve) => { void run( - backupCommand(context(client), dir, {}), + backupCommand(ctx, dir, {}), new PassThrough(), runStderr, resolve, ); }); + return { code, runText }; + }; + + it("exits 1 with the error on one line when the refresh fails", async () => { + const client = { + ...fakeClient(), + collectionsSince: async () => { + throw new Error("HTTP 503 from server"); + }, + } as unknown as Client; + const dir = join(root, "backup"); + const { code, runText } = await backupThroughRun(context(client), dir); expect(code).toBe(1); - expect(runText).toBe("quak: HTTP 401 from server\n"); + expect(runText).toBe("quak: HTTP 503 from server\n"); + expect(stderr.text).toBe("Starting backup...\nRefreshing library...\n"); + expect(existsSync(dir)).toBe(false); + }); + + // A real saved session, read back by `loadSession`, whose server answers + // every request with 401, as it does once it no longer accepts the token. + // The context's prompts throw, so a prompt would end the run with another + // line and exit 1. + it("exits 3 with one line saying to log in again when the server answers 401", async () => { + const key = toBase64(new Uint8Array(32)); + saveSession(join(root, "session"), { + email: "cli@example.com", + userID: USER_ID, + token: "expired", + masterKey: key, + secretKey: key, + publicKey: key, + }); + const unauthorized = async (): Promise => + new Response(null, { status: 401 }); + const ctx = { + ...context(), + loadSession: (path: string) => + loadSession(path, { fetch: unauthorized }), + }; + const dir = join(root, "backup"); + const { code, runText } = await backupThroughRun(ctx, dir); + expect(code).toBe(3); + expect(runText).toBe( + `quak: the saved session is no longer valid; run "quak login"\n`, + ); expect(stderr.text).toBe("Starting backup...\nRefreshing library...\n"); expect(existsSync(dir)).toBe(false); }); diff --git a/test/cli/run.test.ts b/test/cli/run.test.ts index 29d0a92..cfad348 100644 --- a/test/cli/run.test.ts +++ b/test/cli/run.test.ts @@ -5,6 +5,7 @@ import { PassThrough } from "node:stream"; import { describe, it, expect } from "vitest"; +import { ApiError } from "../../src/api/client.js"; import { run } from "../../src/cli-run.js"; // A stream whose written text is kept in `text`; writes finish at once, so @@ -55,4 +56,26 @@ describe("run", () => { stderr: "quak: offline\n", }); }); + + it("on a 401 from the server says to run quak login, on one line, and exits 3", async () => { + const result = await runToExit( + Promise.reject(new ApiError("unauthorized", 401)), + ); + expect(result).toEqual({ + code: 3, + stdout: "", + stderr: `quak: the saved session is no longer valid; run "quak login"\n`, + }); + }); + + it("prints another HTTP error as it is and exits 1", async () => { + const result = await runToExit( + Promise.reject(new ApiError("forbidden", 403)), + ); + expect(result).toEqual({ + code: 1, + stdout: "", + stderr: "quak: forbidden\n", + }); + }); });