Skip to content

Commit ed630ab

Browse files
committed
Treat failed run.json with error as valid session state
loadState used to treat any parsed error field as a corrupt file, so a valid failed run with an error string was unreadable. Parse is now a tagged result, resume-by-id of a truly unreadable file throws one recovery line, and diagnostics go to structured log instead of stderr.
1 parent 2503cc6 commit ed630ab

6 files changed

Lines changed: 220 additions & 16 deletions

File tree

src/config.test.ts

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {
2626
type Settings,
2727
} from "./config/settings.js";
2828
import { OPENCODE_GO_BASE_URL } from "../packages/opencode-go/src/index.js";
29-
import { generateSessionId, initSessionDir } from "./session/index.js";
29+
import { generateSessionId, initSessionDir, sessionDir } from "./session/index.js";
3030
import { saveState } from "./session/state.js";
3131
import { filterMcpServersForConnect } from "./trust/project-trust.js";
3232
import { createExaMCPServerConfig } from "./mcp/exa.js";
@@ -467,6 +467,42 @@ describe("loadConfig", () => {
467467
}
468468
});
469469

470+
test("resume <id> --force reopens a failed session that recorded an error", async () => {
471+
const cwd = await emptyCwd();
472+
const home = await mkdtemp(join(tmpdir(), "ic-resume-home-"));
473+
try {
474+
const globalPath = await writeGlobalSettings(cwd);
475+
const sessionId = generateSessionId();
476+
await initSessionDir(cwd, sessionId, home);
477+
await saveState(
478+
cwd,
479+
sessionId,
480+
{
481+
status: "failed",
482+
turnsUsed: 4,
483+
task: "ship resume after failure",
484+
startedAt: Date.now() - 1_000,
485+
finishedAt: Date.now(),
486+
error: "Cycle commit failed\nhook dump: pre-commit rejected",
487+
},
488+
home,
489+
);
490+
const config = await loadConfig(["resume", sessionId, "--force", "--cwd", cwd], {
491+
globalSettingsPath: globalPath,
492+
home,
493+
});
494+
assertConfigured(config);
495+
expect(config.resumeMode).toBe("id");
496+
expect(config.sessionId).toBe(sessionId);
497+
expect(config.skipInitialTask).toBe(true);
498+
expect(config.task).toBe("ship resume after failure");
499+
expect(config.force).toBe(true);
500+
} finally {
501+
await rm(cwd, { recursive: true, force: true });
502+
await rm(home, { recursive: true, force: true });
503+
}
504+
});
505+
470506
test("--resume opens the picker", async () => {
471507
const cwd = await emptyCwd();
472508
const home = await mkdtemp(join(tmpdir(), "ic-resume-home-"));
@@ -558,6 +594,50 @@ describe("loadConfig", () => {
558594
}
559595
});
560596

597+
test("resume <id> of an unreadable session throws a short recovery line", async () => {
598+
const cwd = await emptyCwd();
599+
const home = await mkdtemp(join(tmpdir(), "ic-resume-home-"));
600+
try {
601+
const globalPath = await writeGlobalSettings(cwd);
602+
const sessionId = generateSessionId();
603+
await initSessionDir(cwd, sessionId, home);
604+
await writeFile(join(sessionDir(cwd, sessionId, home), "run.json"), "{ not json");
605+
606+
const chunks: string[] = [];
607+
const orig = process.stderr.write.bind(process.stderr);
608+
process.stderr.write = ((chunk: string | Uint8Array, ...rest: unknown[]) => {
609+
chunks.push(typeof chunk === "string" ? chunk : Buffer.from(chunk).toString());
610+
return orig(chunk, ...(rest as []));
611+
}) as typeof process.stderr.write;
612+
let thrown: unknown;
613+
try {
614+
await loadConfig(["resume", sessionId, "--force", "--cwd", cwd], {
615+
globalSettingsPath: globalPath,
616+
home,
617+
});
618+
} catch (err) {
619+
thrown = err;
620+
} finally {
621+
process.stderr.write = orig;
622+
}
623+
624+
expect(thrown).toBeInstanceOf(Error);
625+
const message = thrown instanceof Error ? thrown.message : String(thrown);
626+
expect(message).toMatch(new RegExp(`session ${sessionId} is unreadable`, "i"));
627+
expect(message).not.toMatch(/No session/);
628+
expect(message).not.toContain("ignoring unreadable");
629+
expect(message).not.toContain("invalid shape");
630+
expect(message).not.toContain(home);
631+
expect(message.split("\n")).toHaveLength(1);
632+
const text = chunks.join("");
633+
expect(text).not.toContain("ignoring unreadable");
634+
expect(text).not.toContain(home);
635+
} finally {
636+
await rm(cwd, { recursive: true, force: true });
637+
await rm(home, { recursive: true, force: true });
638+
}
639+
});
640+
561641
test("resume rejects a non-id positional instead of treating it as last", async () => {
562642
const cwd = await emptyCwd();
563643
try {

src/config/index.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,15 @@
1-
import { resolve } from "node:path";
1+
import { join, resolve } from "node:path";
2+
import { stat } from "node:fs/promises";
23

34
import type { InferenceSource } from "@intx/types/runtime";
4-
import { generateSessionId, isSessionId, migrateLegacySessionIfNeeded } from "../session/index.js";
5+
import {
6+
generateSessionId,
7+
isSessionId,
8+
migrateLegacySessionIfNeeded,
9+
sessionDir,
10+
} from "../session/index.js";
511
import { loadState } from "../session/state.js";
12+
import { COMMAND_NAME } from "../branding.js";
613

714
import { isDirectorId } from "../agent/directors/registry.js";
815
import { DIRECTOR_IDS, type DirectorId } from "../agent/directors/types.js";
@@ -840,8 +847,20 @@ export async function loadConfig(
840847
await migrateLegacySessionIfNeeded(cwd, id, options.home);
841848
const state = await loadState(cwd, id, options.home);
842849
if (state === null) {
850+
let runJsonPresent = false;
851+
try {
852+
await stat(join(sessionDir(cwd, id, options.home), "run.json"));
853+
runJsonPresent = true;
854+
} catch {
855+
// Missing run.json: treat as no session for this project.
856+
}
857+
if (runJsonPresent) {
858+
throw new Error(
859+
`Session ${id} is unreadable. Use \`${COMMAND_NAME} resume\` to choose another.`,
860+
);
861+
}
843862
throw new Error(
844-
`No session ${id} for this project. Sessions are stored under ~/.corbits/projects/<project-key>/ (this checkout's git toplevel). Use \`corbits resume\` to choose one.`,
863+
`No session ${id} for this project. Sessions are stored under ~/.corbits/projects/<project-key>/ (this checkout's git toplevel). Use \`${COMMAND_NAME} resume\` to choose one.`,
845864
);
846865
}
847866
sessionId = id;

src/session/index.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,14 @@ export async function listSessions(
250250
});
251251
continue;
252252
}
253+
// Present but unreadable run.json is not a picker candidate — diagnostics
254+
// already went to the structured log from loadState.
255+
try {
256+
await stat(join(sessionDir(cwd, entry, home), "run.json"));
257+
continue;
258+
} catch {
259+
// Missing run.json: fall through to the context-only crashed fallback.
260+
}
253261
// A session directory with context/ but no readable run.json never
254262
// reached its first saveState call (see src/tui/runner.ts's early
255263
// "running" write) and therefore isn't actually running: report it as

src/session/list-sessions.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,48 @@ test("listSessions ignores a leftover goal.json from a pre-removal session", asy
8989
expect(row).toBeDefined();
9090
expect(row?.task).toBe("pre-removal session");
9191
});
92+
93+
test("listSessions skips a session whose run.json is unreadable", async () => {
94+
const sessionId = generateSessionId();
95+
await initSessionDir(cwd, sessionId, home);
96+
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();
99+
});
100+
101+
test("listSessions stays silent when many sibling run.json files are unreadable", async () => {
102+
const validId = generateSessionId();
103+
await initSessionDir(cwd, validId, home);
104+
await writeFile(
105+
join(sessionDir(cwd, validId, home), "run.json"),
106+
JSON.stringify({
107+
status: "running",
108+
turnsUsed: 1,
109+
task: "keep me",
110+
startedAt: 1_700_000_000_000,
111+
}),
112+
);
113+
for (let i = 0; i < 8; i++) {
114+
const id = generateSessionId();
115+
await initSessionDir(cwd, id, home);
116+
await writeFile(join(sessionDir(cwd, id, home), "run.json"), '{ "turnsUsed": ');
117+
}
118+
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;
125+
let listed: Awaited<ReturnType<typeof listSessions>> = [];
126+
try {
127+
listed = await listSessions(cwd, home);
128+
} finally {
129+
process.stderr.write = orig;
130+
}
131+
132+
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);
136+
});

src/session/state.ts

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@ import { mkdir, writeFile, readFile, rename } from "node:fs/promises";
22
import { dirname, join } from "node:path";
33

44
import { type } from "arktype";
5+
import { getLogger } from "@intx/log";
56

67
import { sessionDir } from "./index.js";
78
import { clearActiveRun, getTestWriteGate, isCrashed } from "./active-run.js";
8-
import { COMMAND_NAME } from "../branding.js";
9+
import { LOG_NAMESPACE_ROOT } from "../branding.js";
10+
11+
const log = getLogger([LOG_NAMESPACE_ROOT, "session", "state"]);
912

1013
const ConnectedMcpServerSchema = type({
1114
name: "string",
@@ -50,11 +53,10 @@ export async function atomicWrite(path: string, content: string): Promise<void>
5053
}
5154

5255
// A corrupt or shape-invalid state file means resume is silently starting over
53-
// and prior progress is being discarded. Surface it rather than swallowing it.
56+
// and prior progress is being discarded. Log it rather than printing a path and
57+
// parse dump on the terminal the TUI is about to own.
5458
export function warnUnreadableState(path: string, reason: string): void {
55-
process.stderr.write(
56-
`${COMMAND_NAME}: ignoring unreadable state at ${path} (${reason}); starting fresh\n`,
57-
);
59+
log.warn("unreadable session state at {path}: {reason}", { path, reason });
5860
}
5961

6062
// Concurrent saveState calls for the same session (a straggler progress
@@ -159,11 +161,14 @@ export async function saveCrashState(
159161
await atomicWrite(path, JSON.stringify(state, null, 2));
160162
}
161163

162-
// Returns the parsed state, or the arktype error summary when the shape is
163-
// invalid, so callers can surface a specific reason rather than "invalid shape".
164-
function parseRunState(data: unknown): RunState | { error: string } {
164+
type ParseRunStateResult = { ok: true; state: RunState } | { ok: false; reason: string };
165+
166+
// Tagged so a valid RunState.error string cannot be mistaken for a parse failure.
167+
function parseRunState(data: unknown): ParseRunStateResult {
165168
const result = RunStateSchema(data);
166-
return result instanceof type.errors ? { error: result.summary } : result;
169+
return result instanceof type.errors
170+
? { ok: false, reason: result.summary }
171+
: { ok: true, state: result };
167172
}
168173

169174
export async function loadState(
@@ -176,11 +181,11 @@ export async function loadState(
176181
try {
177182
const raw = await readFile(path, "utf8");
178183
const parsed = parseRunState(JSON.parse(raw));
179-
if ("error" in parsed) {
180-
warnUnreadableState(path, `invalid shape: ${parsed.error}`);
184+
if (!parsed.ok) {
185+
warnUnreadableState(path, `invalid shape: ${parsed.reason}`);
181186
return null;
182187
}
183-
return parsed;
188+
return parsed.state;
184189
} catch (err) {
185190
if (err instanceof SyntaxError) {
186191
warnUnreadableState(path, "corrupt JSON");

src/state.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,30 @@ describe("state persistence", () => {
4444
expect(loaded).toEqual(baseRunState);
4545
});
4646

47+
test("loadState returns a failed run that recorded an error string", async () => {
48+
const state: RunState = {
49+
...baseRunState,
50+
status: "failed",
51+
finishedAt: 1_700_000_005_000,
52+
error: "Cycle commit failed\nhook dump: pre-commit rejected",
53+
};
54+
await saveState(cwd, SESSION_ID, state, home);
55+
const loaded = await loadState(cwd, SESSION_ID, home);
56+
expect(loaded).toEqual(state);
57+
});
58+
59+
test("loadState returns a crashed run that recorded an error string", async () => {
60+
const state: RunState = {
61+
...baseRunState,
62+
status: "crashed",
63+
finishedAt: 1_700_000_005_000,
64+
error: "uncaughtException: boom",
65+
};
66+
await saveState(cwd, SESSION_ID, state, home);
67+
const loaded = await loadState(cwd, SESSION_ID, home);
68+
expect(loaded).toEqual(state);
69+
});
70+
4771
test("saveState round-trips optional fields", async () => {
4872
const state: RunState = {
4973
...baseRunState,
@@ -78,6 +102,29 @@ describe("state persistence", () => {
78102
expect(result).toBeNull();
79103
});
80104

105+
test("loadState does not print unreadable-state diagnostics to stderr", async () => {
106+
const stateDir = dir();
107+
const { mkdir } = await import("node:fs/promises");
108+
await mkdir(stateDir, { recursive: true });
109+
await writeFile(join(stateDir, "run.json"), '{ "turnsUsed": ');
110+
111+
const chunks: string[] = [];
112+
const orig = process.stderr.write.bind(process.stderr);
113+
process.stderr.write = ((chunk: string | Uint8Array, ...rest: unknown[]) => {
114+
chunks.push(typeof chunk === "string" ? chunk : Buffer.from(chunk).toString());
115+
return orig(chunk, ...(rest as []));
116+
}) as typeof process.stderr.write;
117+
try {
118+
expect(await loadState(cwd, SESSION_ID, home)).toBeNull();
119+
} finally {
120+
process.stderr.write = orig;
121+
}
122+
const text = chunks.join("");
123+
expect(text).not.toContain("ignoring unreadable");
124+
expect(text).not.toContain(home);
125+
expect(text).not.toContain("invalid shape");
126+
});
127+
81128
// ---------------------------------------------------------------------------
82129
// 4. Valid JSON but wrong shape returns null via the validators
83130
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)