From edd0794148b448e46d4ea681b6f820f553da0435 Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Fri, 21 Aug 2026 10:42:45 +0200 Subject: [PATCH] fix(session-context): report a read failure instead of throwing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sessionReadFailure` exists, by its own docstring, for "a session file that is discovered but then unreadable, which a session removed between discovery and the read produces". It could not be reached for that case: `readTextIfExists` checks existence and then reads, and returns null only for absence — a file that exists but cannot be read rejects, and the rejection escaped `readClaudeCodeSession`, `readCursorSession` and `readCopilotSession` instead of becoming a result. The antigravity and pi readers already spell this `.catch(() => null)`; these three did not. A permission-denied transcript, or one deleted between discovery and the read, therefore crashed the command rather than reporting which session failed. Fire-tested per site, both directions, under WSL — chmod is a no-op on Windows and root ignores the mode bits, so these cases are non-root POSIX only and report as passing without ever running anywhere else. Also switches the three transcript parses to `safeParse`, the convention CLAUDE.md states. Behaviour is unchanged: the surrounding try/catch already caught the throw, and still guards `Bun.JSONL.parse`. Found by CodeRabbit on #576 and deferred out of it: the review ran against that branch before it was rebased, so it covered files belonging to #574, already merged. Signed-off-by: Rhuan Barreto --- src/helpers/session-context-copilot.ts | 7 +++-- src/helpers/session-context.ts | 20 +++++++++--- tests/helpers/session-context-copilot.test.ts | 21 +++++++++++++ tests/helpers/session-context-cursor.test.ts | 25 +++++++++++++++ tests/helpers/session-context.test.ts | 31 ++++++++++++++++++- 5 files changed, 97 insertions(+), 7 deletions(-) diff --git a/src/helpers/session-context-copilot.ts b/src/helpers/session-context-copilot.ts index f91d16a8..8e1daaf0 100644 --- a/src/helpers/session-context-copilot.ts +++ b/src/helpers/session-context-copilot.ts @@ -196,12 +196,15 @@ export async function readCopilotSession( path: eventsFile, }; - const raw = await readTextIfExists(eventsFile); + // A file that exists but cannot be read rejects rather than yielding null. + const raw = await readTextIfExists(eventsFile).catch(() => null); if (raw === null) return noTranscript; let rawEntries: z.infer; try { - rawEntries = CopilotEventsSchema.parse(Bun.JSONL.parse(raw)); + const parsed = CopilotEventsSchema.safeParse(Bun.JSONL.parse(raw)); + if (!parsed.success) return noTranscript; + rawEntries = parsed.data; } catch { return noTranscript; } diff --git a/src/helpers/session-context.ts b/src/helpers/session-context.ts index f69fa83e..c8f6033f 100644 --- a/src/helpers/session-context.ts +++ b/src/helpers/session-context.ts @@ -283,12 +283,18 @@ export async function readClaudeCodeSession( } const sessionFile = join(projectsDir, targetName); - const raw = await readTextIfExists(sessionFile); + // `.catch` as well as the null: readTextIfExists rejects for a file that + // exists but cannot be read, which is the case sessionReadFailure names. + const raw = await readTextIfExists(sessionFile).catch(() => null); if (raw === null) return sessionReadFailure(sessionFile); let entries: TranscriptEntry[]; try { - entries = z.array(TranscriptEntrySchema).parse(Bun.JSONL.parse(raw)); + const parsed = z + .array(TranscriptEntrySchema) + .safeParse(Bun.JSONL.parse(raw)); + if (!parsed.success) return sessionReadFailure(sessionFile); + entries = parsed.data; } catch { return sessionReadFailure(sessionFile); } @@ -433,12 +439,18 @@ export async function readCursorSession( targetDir.name, `${targetDir.name}.jsonl` ); - const raw = await readTextIfExists(sessionFile); + // `.catch` as well as the null: readTextIfExists rejects for a file that + // exists but cannot be read, which is the case sessionReadFailure names. + const raw = await readTextIfExists(sessionFile).catch(() => null); if (raw === null) return sessionReadFailure(sessionFile); let entries: TranscriptEntry[]; try { - entries = z.array(TranscriptEntrySchema).parse(Bun.JSONL.parse(raw)); + const parsed = z + .array(TranscriptEntrySchema) + .safeParse(Bun.JSONL.parse(raw)); + if (!parsed.success) return sessionReadFailure(sessionFile); + entries = parsed.data; } catch { return sessionReadFailure(sessionFile); } diff --git a/tests/helpers/session-context-copilot.test.ts b/tests/helpers/session-context-copilot.test.ts index a6cc13f1..79db6cb2 100644 --- a/tests/helpers/session-context-copilot.test.ts +++ b/tests/helpers/session-context-copilot.test.ts @@ -2,6 +2,7 @@ // Copyright 2026 Archgate import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { + chmodSync, mkdirSync, mkdtempSync, rmSync, @@ -60,6 +61,26 @@ describe("readCopilotSession", () => { } } + // events.jsonl exists but cannot be read: the reader reports "no transcript" + // rather than letting the rejection escape. Non-root POSIX only. + test.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "reports no transcript when events.jsonl cannot be read", + async () => { + const sessionId = `copilot-${uniqueId}-locked`; + makeSession(sessionId, projectRoot, ['{"type":"user.message"}']); + const file = join(stateDir, sessionId, "events.jsonl"); + chmodSync(file, 0o000); + try { + const result = await readCopilotSession(projectRoot); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("expected a failure result"); + expect(result.error).toContain("no conversation transcript"); + } finally { + chmodSync(file, 0o600); + } + } + ); + test("returns data for most recent session matching project", async () => { const sessionId = `copilot-${uniqueId}-1`; makeSession(sessionId, projectRoot, [ diff --git a/tests/helpers/session-context-cursor.test.ts b/tests/helpers/session-context-cursor.test.ts index ceb2d7e8..e5110654 100644 --- a/tests/helpers/session-context-cursor.test.ts +++ b/tests/helpers/session-context-cursor.test.ts @@ -14,6 +14,7 @@ import { mkdtempSync, rmSync, symlinkSync, + chmodSync, writeFileSync, } from "node:fs"; import * as os from "node:os"; @@ -65,6 +66,30 @@ describe("readCursorSession", () => { writeFileSync(join(sessionDir, `${sessionId}.jsonl`), lines.join("\n")); } + // Discovery finds the file, the read then fails — permission-denied here, + // a session removed mid-read in practice. Non-root POSIX only: chmod is a + // no-op on Windows and root ignores the mode bits. + test.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "reports a read failure when the transcript cannot be read", + async () => { + makeSession("session-locked", ['{"role":"user"}']); + const file = join( + transcriptsDir, + "session-locked", + "session-locked.jsonl" + ); + chmodSync(file, 0o000); + try { + const result = await readCursorSession(projectRoot); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("expected a failure result"); + expect(result.error).toBe("Failed to read session file"); + } finally { + chmodSync(file, 0o600); + } + } + ); + test("returns data for most recent session", async () => { makeSession("session-abc", [ JSON.stringify({ diff --git a/tests/helpers/session-context.test.ts b/tests/helpers/session-context.test.ts index 11ad1a0d..fc6d867e 100644 --- a/tests/helpers/session-context.test.ts +++ b/tests/helpers/session-context.test.ts @@ -9,7 +9,13 @@ import { test, type Mock, } from "bun:test"; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { + chmodSync, + mkdirSync, + mkdtempSync, + rmSync, + writeFileSync, +} from "node:fs"; import * as os from "node:os"; import { join } from "node:path"; @@ -171,6 +177,29 @@ describe("readClaudeCodeSession", () => { ); } + // A session file can exist at discovery and be unreadable at the read: + // removed in between, or permission-denied. readTextIfExists rejects + // rather than yielding null there, which is the case sessionReadFailure + // names. Only reproducible as a non-root POSIX user — chmod is a no-op on + // Windows and root ignores the mode bits. + test.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "reports a read failure for a session file that cannot be read", + async () => { + const file = join(projectsDir, "session.jsonl"); + writeFileSync(file, '{"type":"user"}'); + chmodSync(file, 0o000); + try { + const result = await readClaudeCodeSession(projectRoot); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("expected a failure result"); + expect(result.error).toBe("Failed to read session file"); + } finally { + // Restore before afterEach's rmSync, which cannot remove it otherwise. + chmodSync(file, 0o600); + } + } + ); + test("returns data with correct transcript when JSONL exists", async () => { writeSession([ { type: "user", message: { role: "user", content: "hello" } },