Skip to content

Commit 6bab08a

Browse files
Merge pull request #726 from corbitsdev/cl-7287-hide-unreadable-session-diagnostics-from-corbits-resume
Treat failed run.json with error as valid session state
2 parents 9c6219e + 42a44cd commit 6bab08a

15 files changed

Lines changed: 523 additions & 81 deletions

File tree

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,13 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
1313

1414
## [Unreleased]
1515

16+
### Fixed
17+
18+
- 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.
22+
1623
## [0.3.10] - 2026-08-30
1724

1825
### Fixed

docs/PRODUCT.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,11 @@ Opens a picker of saved conversations for the working directory. Plain
8080
`corbits` always starts a fresh conversation; `corbits resume <session-id>`
8181
is the direct, explicit resume path.
8282

83+
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
86+
recovery line instead of dumping the file path and parse details.
87+
8388
## Safety Model
8489

8590
- **Tiered permission gate** — Read-only tools (`read_file`, `search_files`, `grep`, `list_dir`) run freely. Every consequential tool (`write_file`, `edit_file`, `run_shell`, …) is gated. The operator can Allow Once or Allow Always (scoped to a file, a directory, or a command shape); "Allow Always" choices persist per working directory so repeat actions don't interrupt flow.
@@ -128,7 +133,9 @@ The exact turn thresholds are model-family-dependent (tighter for models with ob
128133

129134
**What the user sees:** `Ctrl+C` mid-run, network error, or crash. The last state is persisted.
130135

131-
**Recovery:** `corbits resume` reloads `RunState` and continues.
136+
**Recovery:** `corbits resume` reloads `RunState` and continues. Failed
137+
sessions remain failed (still listed); a corrupt id gets a short recovery
138+
line instead of a path dump.
132139

133140
## Configuration
134141

src/config.test.ts

Lines changed: 139 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
buildProviderCatalog,
1111
catalogEntryAsProviderSettings,
1212
CliHelpError,
13+
CliUserError,
1314
CLI_HELP_TEXT,
1415
KEYLESS_API_KEY,
1516
loadConfig,
@@ -26,7 +27,7 @@ import {
2627
type Settings,
2728
} from "./config/settings.js";
2829
import { OPENCODE_GO_BASE_URL } from "../packages/opencode-go/src/index.js";
29-
import { generateSessionId, initSessionDir } from "./session/index.js";
30+
import { generateSessionId, initSessionDir, sessionDir } from "./session/index.js";
3031
import { saveState } from "./session/state.js";
3132
import { filterMcpServersForConnect } from "./trust/project-trust.js";
3233
import { createExaMCPServerConfig } from "./mcp/exa.js";
@@ -467,6 +468,94 @@ describe("loadConfig", () => {
467468
}
468469
});
469470

471+
test("resume <id> --force reopens a failed session that recorded an error", async () => {
472+
const cwd = await emptyCwd();
473+
const home = await mkdtemp(join(tmpdir(), "ic-resume-home-"));
474+
try {
475+
const globalPath = await writeGlobalSettings(cwd);
476+
const sessionId = generateSessionId();
477+
await initSessionDir(cwd, sessionId, home);
478+
await saveState(
479+
cwd,
480+
sessionId,
481+
{
482+
status: "failed",
483+
turnsUsed: 4,
484+
task: "ship resume after failure",
485+
startedAt: Date.now() - 1_000,
486+
finishedAt: Date.now(),
487+
error: "Cycle commit failed\nhook dump: pre-commit rejected",
488+
},
489+
home,
490+
);
491+
const config = await loadConfig(["resume", sessionId, "--force", "--cwd", cwd], {
492+
globalSettingsPath: globalPath,
493+
home,
494+
});
495+
assertConfigured(config);
496+
expect(config.resumeMode).toBe("id");
497+
expect(config.sessionId).toBe(sessionId);
498+
expect(config.skipInitialTask).toBe(true);
499+
expect(config.task).toBe("ship resume after failure");
500+
expect(config.force).toBe(true);
501+
} finally {
502+
await rm(cwd, { recursive: true, force: true });
503+
await rm(home, { recursive: true, force: true });
504+
}
505+
});
506+
507+
test("resume <id> --force among failed siblings stays silent and reopens the target", async () => {
508+
const cwd = await emptyCwd();
509+
const home = await mkdtemp(join(tmpdir(), "ic-resume-home-"));
510+
try {
511+
const globalPath = await writeGlobalSettings(cwd);
512+
const targetId = generateSessionId();
513+
for (let i = 0; i < 6; i++) {
514+
const id = i === 0 ? targetId : generateSessionId();
515+
await initSessionDir(cwd, id, home);
516+
await saveState(
517+
cwd,
518+
id,
519+
{
520+
status: "failed",
521+
turnsUsed: 2,
522+
task: i === 0 ? "target failed session" : `sibling failed ${i}`,
523+
startedAt: Date.now() - 1_000 - i,
524+
finishedAt: Date.now() - i,
525+
error: "Cycle commit failed\nhook dump: pre-commit rejected",
526+
},
527+
home,
528+
);
529+
}
530+
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;
537+
let config: Awaited<ReturnType<typeof loadConfig>>;
538+
try {
539+
config = await loadConfig(["resume", targetId, "--force", "--cwd", cwd], {
540+
globalSettingsPath: globalPath,
541+
home,
542+
});
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");
553+
} finally {
554+
await rm(cwd, { recursive: true, force: true });
555+
await rm(home, { recursive: true, force: true });
556+
}
557+
});
558+
470559
test("--resume opens the picker", async () => {
471560
const cwd = await emptyCwd();
472561
const home = await mkdtemp(join(tmpdir(), "ic-resume-home-"));
@@ -558,6 +647,55 @@ describe("loadConfig", () => {
558647
}
559648
});
560649

650+
test("resume <id> of an unreadable session throws a short recovery line", async () => {
651+
const cwd = await emptyCwd();
652+
const home = await mkdtemp(join(tmpdir(), "ic-resume-home-"));
653+
try {
654+
const globalPath = await writeGlobalSettings(cwd);
655+
const sessionId = generateSessionId();
656+
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;
665+
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+
}
676+
677+
expect(thrown).toBeInstanceOf(CliUserError);
678+
const message = thrown instanceof Error ? thrown.message : String(thrown);
679+
expect(message).toBe(
680+
`Session ${sessionId} is unreadable. Use \`corbits resume\` to choose another.`,
681+
);
682+
expect(message).not.toMatch(/No session/);
683+
expect(message).not.toContain("ignoring unreadable");
684+
expect(message).not.toContain("invalid shape");
685+
expect(message).not.toContain(home);
686+
expect(message.split("\n")).toHaveLength(1);
687+
if (thrown instanceof CliUserError) {
688+
expect(thrown.exitCode).toBe(1);
689+
}
690+
const text = chunks.join("");
691+
expect(text).not.toContain("ignoring unreadable");
692+
expect(text).not.toContain(home);
693+
} finally {
694+
await rm(cwd, { recursive: true, force: true });
695+
await rm(home, { recursive: true, force: true });
696+
}
697+
});
698+
561699
test("resume rejects a non-id positional instead of treating it as last", async () => {
562700
const cwd = await emptyCwd();
563701
try {

src/config/index.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { resolve } from "node:path";
33
import type { InferenceSource } from "@intx/types/runtime";
44
import { generateSessionId, isSessionId, migrateLegacySessionIfNeeded } from "../session/index.js";
55
import { loadState } from "../session/state.js";
6+
import { COMMAND_NAME } from "../branding.js";
67

78
import { isDirectorId } from "../agent/directors/registry.js";
89
import { DIRECTOR_IDS, type DirectorId } from "../agent/directors/types.js";
@@ -525,6 +526,19 @@ export class CliHelpError extends Error {
525526
}
526527
}
527528

529+
/**
530+
* Thrown for a recoverable operator mistake. Entry points must print
531+
* `message` to stderr and exit 1 — not dump a stack.
532+
*/
533+
export class CliUserError extends Error {
534+
readonly exitCode = 1 as const;
535+
536+
constructor(message: string) {
537+
super(message);
538+
this.name = "CliUserError";
539+
}
540+
}
541+
528542
export interface LoadConfigOptions {
529543
// Override the global settings file location (for tests / non-standard homes).
530544
globalSettingsPath?: string;
@@ -838,12 +852,18 @@ export async function loadConfig(
838852
} else if (resumeMode === "id") {
839853
const id = resumeSessionId!;
840854
await migrateLegacySessionIfNeeded(cwd, id, options.home);
841-
const state = await loadState(cwd, id, options.home);
842-
if (state === null) {
855+
const loaded = await loadState(cwd, id, options.home);
856+
if (loaded.kind === "unreadable") {
857+
throw new CliUserError(
858+
`Session ${id} is unreadable. Use \`${COMMAND_NAME} resume\` to choose another.`,
859+
);
860+
}
861+
if (loaded.kind === "missing") {
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
}
866+
const state = loaded.state;
847867
sessionId = id;
848868
skipInitialTask = true;
849869
if (task.length === 0) resumeTask = state.task;

src/index.ts

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { primeCrashReporting, writeCrashReport, type CrashKind } from "./crash/r
44
import { getActiveRun, markCrashed } from "./session/active-run.js";
55
import { getActiveDisposeHost } from "./session/active-host.js";
66
import { saveCrashState } from "./session/state.js";
7-
import { loadConfig, CliHelpError } from "./config/index.js";
7+
import { loadConfig, CliHelpError, CliUserError } from "./config/index.js";
88
import { ensureTelemetrySettings, globalSettingsPath } from "./config/settings.js";
99
import { installFileLogSink } from "./logging/sink.js";
1010
import { flushPerfToOtel } from "./perf/index.js";
@@ -286,6 +286,24 @@ export function installSignalHandlers(): void {
286286
}
287287
}
288288

289+
export function cliCaughtExit(err: unknown): {
290+
stream: "stdout" | "stderr";
291+
text: string;
292+
code: number;
293+
} {
294+
if (err instanceof CliHelpError) {
295+
return { stream: "stdout", text: `${err.message}\n`, code: err.exitCode };
296+
}
297+
if (err instanceof CliUserError) {
298+
return { stream: "stderr", text: `${err.message}\n`, code: err.exitCode };
299+
}
300+
return {
301+
stream: "stderr",
302+
text: `${err instanceof Error ? (err.stack ?? err.message) : String(err)}\n`,
303+
code: 1,
304+
};
305+
}
306+
289307
if (import.meta.main) {
290308
installCrashHandlers();
291309
installSignalHandlers();
@@ -294,14 +312,10 @@ if (import.meta.main) {
294312
try {
295313
code = await main(process.argv.slice(2));
296314
} catch (err: unknown) {
297-
// Help is an intentional early exit, not a crash — stdout + 0.
298-
if (err instanceof CliHelpError) {
299-
process.stdout.write(`${err.message}\n`);
300-
code = err.exitCode;
301-
} else {
302-
process.stderr.write(`${err instanceof Error ? (err.stack ?? err.message) : String(err)}\n`);
303-
code = 1;
304-
}
315+
const exit = cliCaughtExit(err);
316+
const dest = exit.stream === "stdout" ? process.stdout : process.stderr;
317+
dest.write(exit.text);
318+
code = exit.code;
305319
}
306320
process.exit(code);
307321
}

src/session/index.ts

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -240,20 +240,23 @@ export async function listSessions(
240240
const summaries: SessionSummary[] = [];
241241
for (const entry of entries) {
242242
await migrateLegacySessionIfNeeded(cwd, entry, home);
243-
const state = await loadState(cwd, entry, home);
244-
if (state !== null) {
243+
const loaded = await loadState(cwd, entry, home);
244+
if (loaded.kind === "ok") {
245245
summaries.push({
246246
sessionId: entry,
247-
task: state.task,
248-
startedAt: state.startedAt,
249-
status: state.status,
247+
task: loaded.state.task,
248+
startedAt: loaded.state.startedAt,
249+
status: loaded.state.status,
250250
});
251251
continue;
252252
}
253-
// A session directory with context/ but no readable run.json never
254-
// reached its first saveState call (see src/tui/runner.ts's early
255-
// "running" write) and therefore isn't actually running: report it as
256-
// crashed rather than fabricating liveness.
253+
if (loaded.kind === "unreadable") {
254+
continue;
255+
}
256+
// Missing run.json: a session directory with context/ never reached its
257+
// first saveState call (see src/tui/runner.ts's early "running" write)
258+
// and therefore isn't actually running: report it as crashed rather
259+
// than fabricating liveness.
257260
try {
258261
const dirStat = await stat(sessionDir(cwd, entry, home));
259262
await stat(sessionContextDir(cwd, entry, home));
@@ -290,7 +293,7 @@ export async function renameSession(
290293
}
291294
await migrateLegacySessionIfNeeded(cwd, sessionId, home);
292295
const existing = await loadState(cwd, sessionId, home);
293-
if (existing === null) {
296+
if (existing.kind !== "ok") {
294297
let startedAt = Date.now();
295298
try {
296299
const dirStat = await stat(sessionDir(cwd, sessionId, home));
@@ -311,7 +314,7 @@ export async function renameSession(
311314
);
312315
return;
313316
}
314-
await saveState(cwd, sessionId, { ...existing, task: trimmed }, home);
317+
await saveState(cwd, sessionId, { ...existing.state, task: trimmed }, home);
315318
}
316319

317320
export { projectKeyFor, projectSessionsRoot, projectsRoot, projectRootFor } from "./project-key.js";

0 commit comments

Comments
 (0)