Skip to content

Commit 2718dd3

Browse files
committed
Do not overwrite unreadable run.json on rename
The default resume picker is still running and cancelled only; document --force for failed and done. Silence tests pin a file log sink so a console leak fails. renameSession throws instead of clobbering corrupt state, and the TUI does not persist a snapshot after that failure.
1 parent 2f3dbf2 commit 2718dd3

9 files changed

Lines changed: 203 additions & 112 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,11 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
1616
### Fixed
1717

1818
- Failed sessions with an `error` string in `run.json` are valid resume
19-
candidates, not corrupt files. A truly unreadable session id prints one
20-
recovery line; parse diagnostics go to the structured log, not the
21-
terminal.
19+
candidates, not corrupt files. The default picker still shows only
20+
running and cancelled sessions; `--force` includes failed and done. A
21+
truly unreadable session id prints one recovery line; parse diagnostics
22+
go to the structured log, not the terminal. Renaming a session does not
23+
overwrite an unreadable `run.json`.
2224
- `ask_operator` no longer pre-authorizes a model-authored shell command when
2325
the operator picks any option, including Reject. Clarification choices
2426
cannot mint shell grants.

docs/PRODUCT.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,9 @@ Opens a picker of saved conversations for the working directory. Plain
8181
is the direct, explicit resume path.
8282

8383
A session that ended in `failed` (including one that recorded an `error`
84-
string in `run.json`) is a failed session, not a corrupt one — it still
85-
appears in the picker. Passing a corrupt session id prints one short
84+
string in `run.json`) is a failed session, not a corrupt one. The default
85+
picker still shows only running and cancelled sessions; pass `--force` to
86+
include failed and done. Passing a corrupt session id prints one short
8687
recovery line instead of dumping the file path and parse details.
8788

8889
## Safety Model

src/config.test.ts

Lines changed: 23 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import { generateSessionId, initSessionDir, sessionDir } from "./session/index.j
3131
import { saveState } from "./session/state.js";
3232
import { filterMcpServersForConnect } from "./trust/project-trust.js";
3333
import { createExaMCPServerConfig } from "./mcp/exa.js";
34+
import { withFileLogSink } from "../tests/helpers/file-log-sink.js";
3435

3536
const BUILTIN_EXA_MCP = createExaMCPServerConfig();
3637

@@ -528,28 +529,18 @@ describe("loadConfig", () => {
528529
);
529530
}
530531

531-
const chunks: string[] = [];
532-
const orig = process.stderr.write.bind(process.stderr);
533-
process.stderr.write = ((chunk: string | Uint8Array, ...rest: unknown[]) => {
534-
chunks.push(typeof chunk === "string" ? chunk : Buffer.from(chunk).toString());
535-
return orig(chunk, ...(rest as []));
536-
}) as typeof process.stderr.write;
537532
let config: Awaited<ReturnType<typeof loadConfig>>;
538-
try {
533+
const logged = await withFileLogSink(async () => {
539534
config = await loadConfig(["resume", targetId, "--force", "--cwd", cwd], {
540535
globalSettingsPath: globalPath,
541536
home,
542537
});
543-
} finally {
544-
process.stderr.write = orig;
545-
}
546-
assertConfigured(config);
547-
expect(config.sessionId).toBe(targetId);
548-
expect(config.task).toBe("target failed session");
549-
const text = chunks.join("");
550-
expect(text).not.toContain("ignoring unreadable");
551-
expect(text).not.toContain(home);
552-
expect(text).not.toContain("invalid shape");
538+
});
539+
assertConfigured(config!);
540+
expect(config!.sessionId).toBe(targetId);
541+
expect(config!.task).toBe("target failed session");
542+
expect(logged).not.toContain("unreadable session state");
543+
expect(logged).not.toContain(home);
553544
} finally {
554545
await rm(cwd, { recursive: true, force: true });
555546
await rm(home, { recursive: true, force: true });
@@ -654,25 +645,20 @@ describe("loadConfig", () => {
654645
const globalPath = await writeGlobalSettings(cwd);
655646
const sessionId = generateSessionId();
656647
await initSessionDir(cwd, sessionId, home);
657-
await writeFile(join(sessionDir(cwd, sessionId, home), "run.json"), "{ not json");
658-
659-
const chunks: string[] = [];
660-
const orig = process.stderr.write.bind(process.stderr);
661-
process.stderr.write = ((chunk: string | Uint8Array, ...rest: unknown[]) => {
662-
chunks.push(typeof chunk === "string" ? chunk : Buffer.from(chunk).toString());
663-
return orig(chunk, ...(rest as []));
664-
}) as typeof process.stderr.write;
648+
const runPath = join(sessionDir(cwd, sessionId, home), "run.json");
649+
await writeFile(runPath, "{ not json");
650+
665651
let thrown: unknown;
666-
try {
667-
await loadConfig(["resume", sessionId, "--force", "--cwd", cwd], {
668-
globalSettingsPath: globalPath,
669-
home,
670-
});
671-
} catch (err) {
672-
thrown = err;
673-
} finally {
674-
process.stderr.write = orig;
675-
}
652+
const logged = await withFileLogSink(async () => {
653+
try {
654+
await loadConfig(["resume", sessionId, "--force", "--cwd", cwd], {
655+
globalSettingsPath: globalPath,
656+
home,
657+
});
658+
} catch (err) {
659+
thrown = err;
660+
}
661+
});
676662

677663
expect(thrown).toBeInstanceOf(CliUserError);
678664
const message = thrown instanceof Error ? thrown.message : String(thrown);
@@ -687,9 +673,8 @@ describe("loadConfig", () => {
687673
if (thrown instanceof CliUserError) {
688674
expect(thrown.exitCode).toBe(1);
689675
}
690-
const text = chunks.join("");
691-
expect(text).not.toContain("ignoring unreadable");
692-
expect(text).not.toContain(home);
676+
expect(logged).toContain(runPath);
677+
expect(logged).toContain("corrupt JSON");
693678
} finally {
694679
await rm(cwd, { recursive: true, force: true });
695680
await rm(home, { recursive: true, force: true });

src/session/index.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,10 @@ export async function renameSession(
293293
}
294294
await migrateLegacySessionIfNeeded(cwd, sessionId, home);
295295
const existing = await loadState(cwd, sessionId, home);
296-
if (existing.kind !== "ok") {
296+
if (existing.kind === "unreadable") {
297+
throw new Error("Session state is unreadable");
298+
}
299+
if (existing.kind === "missing") {
297300
let startedAt = Date.now();
298301
try {
299302
const dirStat = await stat(sessionDir(cwd, sessionId, home));

src/session/list-sessions.test.ts

Lines changed: 19 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { join } from "node:path";
44
import { tmpdir } from "node:os";
55

66
import { generateSessionId, initSessionDir, listSessions, sessionDir } from "./index.js";
7+
import { withFileLogSink } from "../../tests/helpers/file-log-sink.js";
78

89
let cwd = "";
910
let home = "";
@@ -94,8 +95,10 @@ test("listSessions skips a session whose run.json is unreadable", async () => {
9495
const sessionId = generateSessionId();
9596
await initSessionDir(cwd, sessionId, home);
9697
await writeFile(join(sessionDir(cwd, sessionId, home), "run.json"), "{ not json");
97-
const listed = await listSessions(cwd, home);
98-
expect(listed.find((s) => s.sessionId === sessionId)).toBeUndefined();
98+
await withFileLogSink(async () => {
99+
const listed = await listSessions(cwd, home);
100+
expect(listed.find((s) => s.sessionId === sessionId)).toBeUndefined();
101+
});
99102
});
100103

101104
test("listSessions stays silent when many sibling run.json files are unreadable", async () => {
@@ -110,29 +113,25 @@ test("listSessions stays silent when many sibling run.json files are unreadable"
110113
startedAt: 1_700_000_000_000,
111114
}),
112115
);
116+
const unreadablePaths: string[] = [];
113117
for (let i = 0; i < 8; i++) {
114118
const id = generateSessionId();
115119
await initSessionDir(cwd, id, home);
116-
await writeFile(join(sessionDir(cwd, id, home), "run.json"), '{ "turnsUsed": ');
120+
const runPath = join(sessionDir(cwd, id, home), "run.json");
121+
unreadablePaths.push(runPath);
122+
await writeFile(runPath, '{ "turnsUsed": ');
117123
}
118124

119-
const chunks: string[] = [];
120-
const orig = process.stderr.write.bind(process.stderr);
121-
process.stderr.write = ((chunk: string | Uint8Array, ...rest: unknown[]) => {
122-
chunks.push(typeof chunk === "string" ? chunk : Buffer.from(chunk).toString());
123-
return orig(chunk, ...(rest as []));
124-
}) as typeof process.stderr.write;
125125
let listed: Awaited<ReturnType<typeof listSessions>> = [];
126-
try {
126+
const logged = await withFileLogSink(async () => {
127127
listed = await listSessions(cwd, home);
128-
} finally {
129-
process.stderr.write = orig;
130-
}
128+
});
131129

132130
expect(listed.map((s) => s.sessionId)).toEqual([validId]);
133-
const text = chunks.join("");
134-
expect(text).not.toContain("ignoring unreadable");
135-
expect(text).not.toContain(home);
131+
expect(logged).toContain("corrupt JSON");
132+
for (const runPath of unreadablePaths) {
133+
expect(logged).toContain(runPath);
134+
}
136135
});
137136

138137
test("listSessions includes a failed run that recorded an error", async () => {
@@ -194,23 +193,13 @@ test("listSessions stays silent when many sibling runs failed with an error", as
194193
);
195194
}
196195

197-
const chunks: string[] = [];
198-
const orig = process.stderr.write.bind(process.stderr);
199-
process.stderr.write = ((chunk: string | Uint8Array, ...rest: unknown[]) => {
200-
chunks.push(typeof chunk === "string" ? chunk : Buffer.from(chunk).toString());
201-
return orig(chunk, ...(rest as []));
202-
}) as typeof process.stderr.write;
203196
let listed: Awaited<ReturnType<typeof listSessions>> = [];
204-
try {
197+
const logged = await withFileLogSink(async () => {
205198
listed = await listSessions(cwd, home);
206-
} finally {
207-
process.stderr.write = orig;
208-
}
199+
});
209200

210201
expect(listed.map((s) => s.sessionId).sort()).toEqual([...ids].sort());
211202
expect(listed.every((s) => s.status === "failed")).toBe(true);
212-
const text = chunks.join("");
213-
expect(text).not.toContain("ignoring unreadable");
214-
expect(text).not.toContain(home);
215-
expect(text).not.toContain("invalid shape");
203+
expect(logged).not.toContain("unreadable session state");
204+
expect(logged).not.toContain(home);
216205
});

src/session/rename-session.test.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import { afterEach, beforeEach, expect, test } from "bun:test";
2+
import { mkdir, readFile, rm, writeFile } from "node:fs/promises";
3+
import { join } from "node:path";
4+
import { tmpdir } from "node:os";
5+
6+
import { generateSessionId, initSessionDir, renameSession, sessionDir } from "./index.js";
7+
import { loadState } from "./state.js";
8+
9+
let cwd = "";
10+
let home = "";
11+
12+
beforeEach(async () => {
13+
const stamp = `${Date.now()}-${Math.random().toString(16).slice(2)}`;
14+
cwd = join(tmpdir(), `corbits-rename-session-${stamp}`);
15+
home = join(tmpdir(), `corbits-rename-home-${stamp}`);
16+
await mkdir(cwd, { recursive: true });
17+
await mkdir(home, { recursive: true });
18+
});
19+
20+
afterEach(async () => {
21+
await rm(cwd, { recursive: true, force: true });
22+
await rm(home, { recursive: true, force: true });
23+
});
24+
25+
test("renameSession updates task on a readable run.json and preserves other fields", async () => {
26+
const sessionId = generateSessionId();
27+
await initSessionDir(cwd, sessionId, home);
28+
await writeFile(
29+
join(sessionDir(cwd, sessionId, home), "run.json"),
30+
JSON.stringify({
31+
status: "done",
32+
turnsUsed: 4,
33+
task: "old name",
34+
startedAt: 1_700_000_000_000,
35+
finishedAt: 1_700_000_100_000,
36+
model: "provider:model",
37+
}),
38+
);
39+
40+
await renameSession(cwd, sessionId, "new name", home);
41+
42+
const loaded = await loadState(cwd, sessionId, home);
43+
expect(loaded).toEqual({
44+
kind: "ok",
45+
state: {
46+
status: "done",
47+
turnsUsed: 4,
48+
task: "new name",
49+
startedAt: 1_700_000_000_000,
50+
finishedAt: 1_700_000_100_000,
51+
model: "provider:model",
52+
},
53+
});
54+
});
55+
56+
test("renameSession creates a running record when run.json is missing", async () => {
57+
const sessionId = generateSessionId();
58+
await initSessionDir(cwd, sessionId, home);
59+
60+
await renameSession(cwd, sessionId, "named session", home);
61+
62+
const loaded = await loadState(cwd, sessionId, home);
63+
expect(loaded.kind).toBe("ok");
64+
if (loaded.kind !== "ok") return;
65+
expect(loaded.state.status).toBe("running");
66+
expect(loaded.state.turnsUsed).toBe(0);
67+
expect(loaded.state.task).toBe("named session");
68+
expect(loaded.state.startedAt).toBeGreaterThan(0);
69+
});
70+
71+
test("renameSession throws on unreadable run.json and leaves the bytes unchanged", async () => {
72+
const sessionId = generateSessionId();
73+
await initSessionDir(cwd, sessionId, home);
74+
const path = join(sessionDir(cwd, sessionId, home), "run.json");
75+
const corrupt = "{ not json";
76+
await writeFile(path, corrupt);
77+
78+
let thrown: unknown;
79+
try {
80+
await renameSession(cwd, sessionId, "should not land", home);
81+
} catch (err) {
82+
thrown = err;
83+
}
84+
expect(thrown).toBeInstanceOf(Error);
85+
expect(thrown instanceof Error ? thrown.message : "").toBe("Session state is unreadable");
86+
expect(await readFile(path, "utf8")).toBe(corrupt);
87+
});

0 commit comments

Comments
 (0)