From 79ac07cd42a93fca02aad7b43f3e8866746113a2 Mon Sep 17 00:00:00 2001 From: Jeffrey Tseng Date: Sat, 15 Aug 2026 16:49:34 -0700 Subject: [PATCH 1/4] fix(coding-agent): harden session graph persistence Reject malformed session graphs and update session files atomically without corrupting the active manager. --- .../coding-agent/src/core/session-manager.ts | 257 ++++++++++++------ .../session-manager/file-operations.test.ts | 153 +++++++++-- 2 files changed, 292 insertions(+), 118 deletions(-) diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index 0f4db72b6..3306f7e95 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -6,6 +6,7 @@ import { closeSync, createReadStream, existsSync, + fstatSync, mkdirSync, openSync, readdirSync, @@ -432,7 +433,7 @@ function applyPlanLifecycleEntry(state: PlanLifecycleState, entry: SessionEntry) return state; } -function validateSessionPlanLifecycle(entries: SessionEntry[]): void { +function validateSessionEntries(entries: SessionEntry[]): void { const seenIds = new Set(); for (const entry of entries) { if (seenIds.has(entry.id)) { @@ -440,10 +441,6 @@ function validateSessionPlanLifecycle(entries: SessionEntry[]): void { } seenIds.add(entry.id); } - const hasPlanLifecycleEntries = entries.some( - (entry) => - entry.type === "collaboration_mode_change" || entry.type === "plan" || entry.type === "plan_resolution", - ); const stateById = new Map(); for (const entry of entries) { const parentState = @@ -455,15 +452,41 @@ function validateSessionPlanLifecycle(entries: SessionEntry[]): void { } : stateById.get(entry.parentId); if (!parentState) { - if (hasPlanLifecycleEntries) { - throw new Error(`Session entry ${entry.id} has a missing parent in a plan-aware session.`); - } - continue; + throw new Error(`Session entry ${entry.id} has a missing parent ${String(entry.parentId)}.`); } stateById.set(entry.id, applyPlanLifecycleEntry(parentState, entry)); } } +interface SessionEntryIndex { + byId: Map; + labelsById: Map; + labelTimestampsById: Map; + leafId: string | null; +} + +function buildSessionEntryIndex(fileEntries: FileEntry[]): SessionEntryIndex { + const byId = new Map(); + const labelsById = new Map(); + const labelTimestampsById = new Map(); + let leafId: string | null = null; + for (const entry of fileEntries) { + if (entry.type === "session") continue; + byId.set(entry.id, entry); + leafId = entry.id; + if (entry.type === "label") { + if (entry.label) { + labelsById.set(entry.targetId, entry.label); + labelTimestampsById.set(entry.targetId, entry.timestamp); + } else { + labelsById.delete(entry.targetId); + labelTimestampsById.delete(entry.targetId); + } + } + } + return { byId, labelsById, labelTimestampsById, leafId }; +} + function getSessionContextSettings( path: SessionEntry[], ): Pick { @@ -633,54 +656,101 @@ function parseSessionEntryLine(line: string): FileEntry | null { return entry; } +function parseStrictSessionEntryLine( + line: string, + lineNumber: number, + allowIncompleteFinalAppend: boolean, +): FileEntry | null { + if (!line.trim()) return null; + let parsed: unknown; + try { + parsed = JSON.parse(line); + } catch { + if (allowIncompleteFinalAppend) return null; + throw new Error(`Session contains a malformed JSONL record at line ${lineNumber}.`); + } + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { + throw new Error(`Session contains an invalid JSONL object at line ${lineNumber}.`); + } + const entry = parsed as FileEntry; + if (entry.type === "plan") { + const plan = parsePlanArtifact(entry.plan); + if (!plan) throw new Error("Session contains an invalid persisted plan artifact."); + return { ...entry, plan }; + } + return entry.type === "session" ? normalizeSessionHeader(entry) : entry; +} + +function loadEntriesFromDescriptor(fd: number): FileEntry[] { + const entries: FileEntry[] = []; + const decoder = new StringDecoder("utf8"); + const buffer = Buffer.allocUnsafe(SESSION_READ_BUFFER_SIZE); + let pending = ""; + let lineNumber = 1; + + while (true) { + const bytesRead = readSync(fd, buffer, 0, buffer.length, null); + if (bytesRead === 0) break; + + pending += decoder.write(buffer.subarray(0, bytesRead)); + let lineStart = 0; + let newlineIndex = pending.indexOf("\n", lineStart); + while (newlineIndex !== -1) { + const entry = parseStrictSessionEntryLine(pending.slice(lineStart, newlineIndex), lineNumber, false); + if (entry) entries.push(entry); + lineNumber++; + lineStart = newlineIndex + 1; + newlineIndex = pending.indexOf("\n", lineStart); + } + pending = pending.slice(lineStart); + } + + pending += decoder.end(); + // A process can terminate after writing only part of the final append. Recover + // that one case only after a complete header has already been read. Complete + // malformed records, including a malformed final record ending in LF, fail. + const finalEntry = parseStrictSessionEntryLine(pending, lineNumber, entries[0]?.type === "session"); + if (finalEntry) entries.push(finalEntry); + + // Validate session header + if (entries.length === 0) return entries; + const header = entries[0]; + if (header.type !== "session" || typeof (header as { id?: unknown }).id !== "string") { + return []; + } + + return entries; +} + /** Exported for testing */ export function loadEntriesFromFile(filePath: string): FileEntry[] { const resolvedFilePath = normalizePath(filePath); if (!existsSync(resolvedFilePath)) return []; - const entries: FileEntry[] = []; const fd = openSync(resolvedFilePath, "r"); try { - const decoder = new StringDecoder("utf8"); - const buffer = Buffer.allocUnsafe(SESSION_READ_BUFFER_SIZE); - let pending = ""; - - while (true) { - const bytesRead = readSync(fd, buffer, 0, buffer.length, null); - if (bytesRead === 0) break; - - pending += decoder.write(buffer.subarray(0, bytesRead)); - let lineStart = 0; - let newlineIndex = pending.indexOf("\n", lineStart); - while (newlineIndex !== -1) { - const entry = parseSessionEntryLine(pending.slice(lineStart, newlineIndex)); - if (entry) entries.push(entry); - lineStart = newlineIndex + 1; - newlineIndex = pending.indexOf("\n", lineStart); - } - pending = pending.slice(lineStart); - } - - pending += decoder.end(); - const finalEntry = parseSessionEntryLine(pending); - if (finalEntry) entries.push(finalEntry); + return loadEntriesFromDescriptor(fd); } finally { closeSync(fd); } +} - // Validate session header - if (entries.length === 0) return entries; - const header = entries[0]; - if (header.type !== "session" || typeof (header as { id?: unknown }).id !== "string") { - return []; +function sessionFileEndsWithLineFeed(filePath: string): boolean { + const fd = openSync(filePath, "r"); + try { + const stats = fstatSync(fd); + if (stats.size === 0) return true; + const finalByte = Buffer.allocUnsafe(1); + return readSync(fd, finalByte, 0, 1, stats.size - 1) === 1 && finalByte[0] === 0x0a; + } finally { + closeSync(fd); } - - return entries; } /** * Inspect a physical line while searching for the first parsed session entry. - * Blank and malformed lines are skipped to match loadEntriesFromFile(). + * Blank and malformed lines are skipped only for bounded header discovery. + * The authoritative full load rejects every complete malformed record. * Returns undefined to keep scanning, null for a parsed non-header entry, or the header. */ function parseSessionHeaderCandidate(line: string): SessionHeader | null | undefined { @@ -1015,41 +1085,72 @@ export class SessionManager { this._setSessionFile(sessionFile); } + private _rewriteFileEntries(sessionFile: string, fileEntries: FileEntry[]): void { + if (!this.persist) return; + const fd = openSync(sessionFile, "w"); + try { + for (const entry of fileEntries) { + writeFileSync(fd, `${JSON.stringify(entry)}\n`); + } + } finally { + closeSync(fd); + } + } + private _setSessionFile(sessionFile: string, preloadedFileEntries?: FileEntry[]): void { - this.sessionFile = resolvePath(sessionFile); - if (existsSync(this.sessionFile)) { - this.fileEntries = preloadedFileEntries ?? loadEntriesFromFile(this.sessionFile); + const nextSessionFile = resolvePath(sessionFile); + const targetExists = existsSync(nextSessionFile); + if (preloadedFileEntries !== undefined || targetExists) { + const nextFileEntries = preloadedFileEntries ?? loadEntriesFromFile(nextSessionFile); // If file was empty, initialize it with a valid session header. If it was // non-empty but did not parse as a Diffex session, fail without modifying it. - if (this.fileEntries.length === 0) { - const explicitPath = this.sessionFile; - if (statSync(explicitPath).size > 0) { - throw new Error(`Session file is not a valid Diffex session: ${explicitPath}`); + if (nextFileEntries.length === 0) { + if (statSync(nextSessionFile).size > 0) { + throw new Error(`Session file is not a valid Diffex session: ${nextSessionFile}`); } - this.newSession(); - this.sessionFile = explicitPath; - this._rewriteFile(); + const nextSessionId = createSessionId(); + const header: SessionHeader = { + type: "session", + version: CURRENT_SESSION_VERSION, + id: nextSessionId, + timestamp: new Date().toISOString(), + cwd: this.cwd, + }; + const initializedEntries: FileEntry[] = [header]; + const index = buildSessionEntryIndex(initializedEntries); + this._rewriteFileEntries(nextSessionFile, initializedEntries); + this.sessionFile = nextSessionFile; + this.fileEntries = initializedEntries; + this.sessionId = nextSessionId; + this.byId = index.byId; + this.labelsById = index.labelsById; + this.labelTimestampsById = index.labelTimestampsById; + this.leafId = index.leafId; this.flushed = true; return; } - const header = this.fileEntries.find((e) => e.type === "session") as SessionHeader | undefined; - this.sessionId = header?.id ?? createSessionId(); - - if (migrateToCurrentVersion(this.fileEntries)) { - this._rewriteFile(); + const header = nextFileEntries.find((entry) => entry.type === "session") as SessionHeader | undefined; + const nextSessionId = header?.id ?? createSessionId(); + const migrated = migrateToCurrentVersion(nextFileEntries); + validateSessionEntries(nextFileEntries.filter((entry): entry is SessionEntry => entry.type !== "session")); + const index = buildSessionEntryIndex(nextFileEntries); + if (migrated || (targetExists && !sessionFileEndsWithLineFeed(nextSessionFile))) { + this._rewriteFileEntries(nextSessionFile, nextFileEntries); } - validateSessionPlanLifecycle( - this.fileEntries.filter((entry): entry is SessionEntry => entry.type !== "session"), - ); - this._buildIndex(); + this.sessionFile = nextSessionFile; + this.fileEntries = nextFileEntries; + this.sessionId = nextSessionId; + this.byId = index.byId; + this.labelsById = index.labelsById; + this.labelTimestampsById = index.labelTimestampsById; + this.leafId = index.leafId; this.flushed = true; } else { - const explicitPath = this.sessionFile; this.newSession(); - this.sessionFile = explicitPath; // preserve explicit path from --session flag + this.sessionFile = nextSessionFile; // preserve explicit path from --session flag } } @@ -1082,36 +1183,16 @@ export class SessionManager { } private _buildIndex(): void { - this.byId.clear(); - this.labelsById.clear(); - this.labelTimestampsById.clear(); - this.leafId = null; - for (const entry of this.fileEntries) { - if (entry.type === "session") continue; - this.byId.set(entry.id, entry); - this.leafId = entry.id; - if (entry.type === "label") { - if (entry.label) { - this.labelsById.set(entry.targetId, entry.label); - this.labelTimestampsById.set(entry.targetId, entry.timestamp); - } else { - this.labelsById.delete(entry.targetId); - this.labelTimestampsById.delete(entry.targetId); - } - } - } + const index = buildSessionEntryIndex(this.fileEntries); + this.byId = index.byId; + this.labelsById = index.labelsById; + this.labelTimestampsById = index.labelTimestampsById; + this.leafId = index.leafId; } private _rewriteFile(): void { if (!this.persist || !this.sessionFile) return; - const fd = openSync(this.sessionFile, "w"); - try { - for (const entry of this.fileEntries) { - writeFileSync(fd, `${JSON.stringify(entry)}\n`); - } - } finally { - closeSync(fd); - } + this._rewriteFileEntries(this.sessionFile, this.fileEntries); } isPersisted(): boolean { diff --git a/packages/coding-agent/test/session-manager/file-operations.test.ts b/packages/coding-agent/test/session-manager/file-operations.test.ts index 99a4b61b9..bdc49c475 100644 --- a/packages/coding-agent/test/session-manager/file-operations.test.ts +++ b/packages/coding-agent/test/session-manager/file-operations.test.ts @@ -49,10 +49,10 @@ describe("loadEntriesFromFile", () => { expect(loadEntriesFromFile(file)).toEqual([]); }); - it("returns empty array for malformed JSON", () => { + it("rejects malformed nonblank JSON", () => { const file = join(tempDir, "malformed.jsonl"); writeFileSync(file, "not json\n"); - expect(loadEntriesFromFile(file)).toEqual([]); + expect(() => loadEntriesFromFile(file)).toThrow("malformed JSONL record at line 1"); }); it("loads valid session file", () => { @@ -68,22 +68,19 @@ describe("loadEntriesFromFile", () => { expect(entries[1].type).toBe("message"); }); - it("skips malformed lines but keeps valid ones", () => { + it("rejects malformed records inside an otherwise valid session", () => { const file = join(tempDir, "mixed.jsonl"); writeFileSync( file, '{"type":"session","id":"abc","timestamp":"2025-01-01T00:00:00Z","cwd":"/tmp"}\n' + "not valid json\n" + - "null\n[]\n" + '{"type":"message","id":"1","parentId":null,"timestamp":"2025-01-01T00:00:01Z","message":{"role":"user","content":"hi","timestamp":1}}\n', ); - const entries = loadEntriesFromFile(file); - expect(entries).toHaveLength(2); + expect(() => loadEntriesFromFile(file)).toThrow("malformed JSONL record at line 2"); }); it.each([ ["leading blank lines", "\n \n", "leading-blank"], - ["leading malformed lines", "not json\n{broken json\n", "leading-malformed"], ["a multi-buffer header", "", "a".repeat(8192)], ])("reads cwd from a session with %s", (_description, prefix, sessionId) => { const file = join(tempDir, "header.jsonl"); @@ -95,30 +92,57 @@ describe("loadEntriesFromFile", () => { expect(sessionManager.getCwd()).toBe(storedCwd); }); - it("opens compatible sessions beyond the discovery scan limit", () => { + it("opens compatible sessions with headers beyond the discovery scan limit", () => { const storedCwd = join(tempDir, "stored-project"); const overrideCwd = join(tempDir, "override-project"); - const cases = [ - { name: "large-header", id: "a".repeat(HEADER_SCAN_LIMIT_BYTES + 1), prefix: "" }, - { - name: "large-prefix", - id: "large-prefix", - prefix: `${"x".repeat(HEADER_SCAN_LIMIT_BYTES + 1)}\n`, - }, - ]; - - for (const { name, id, prefix } of cases) { - const file = join(tempDir, `${name}.jsonl`); - writeSessionHeader(file, storedCwd, id, prefix); - for (const cwdOverride of [undefined, overrideCwd]) { - const sessionManager = SessionManager.open(file, tempDir, cwdOverride); - expect(sessionManager.getSessionId()).toBe(id); - expect(sessionManager.getCwd()).toBe(cwdOverride ?? storedCwd); - } + const id = "a".repeat(HEADER_SCAN_LIMIT_BYTES + 1); + const file = join(tempDir, "large-header.jsonl"); + writeSessionHeader(file, storedCwd, id); + for (const cwdOverride of [undefined, overrideCwd]) { + const sessionManager = SessionManager.open(file, tempDir, cwdOverride); + expect(sessionManager.getSessionId()).toBe(id); + expect(sessionManager.getCwd()).toBe(cwdOverride ?? storedCwd); } }); - it("opens session files larger than Node's max string length", () => { + it("rejects malformed records before a valid header", () => { + const file = join(tempDir, "leading-malformed.jsonl"); + writeSessionHeader(file, tempDir, "session-id", "not json\n"); + + expect(() => SessionManager.open(file, tempDir)).toThrow("malformed JSONL record at line 1"); + }); + + it("recovers from an incomplete final append without accepting interior corruption", () => { + const file = join(tempDir, "incomplete-final-append.jsonl"); + const completePrefix = + '{"type":"session","version":3,"id":"abc","timestamp":"2025-01-01T00:00:00Z","cwd":"/tmp"}\n' + + '{"type":"message","id":"1","parentId":null,"timestamp":"2025-01-01T00:00:01Z","message":{"role":"user","content":"hi","timestamp":1}}\n'; + const partialAppend = '{"type":"message","id":"partial"'; + writeFileSync(file, completePrefix + partialAppend); + + expect(loadEntriesFromFile(file)).toHaveLength(2); + const recovered = SessionManager.open(file, tempDir); + expect(recovered.getEntries()).toHaveLength(1); + recovered.appendMessage({ role: "user", content: "after recovery", timestamp: 2 }); + expect(SessionManager.open(file, tempDir).getEntries()).toHaveLength(2); + expect(readFileSync(file, "utf8")).not.toContain(partialAppend); + + const malformedFile = join(tempDir, "complete-malformed-final-record.jsonl"); + writeFileSync(malformedFile, `${completePrefix}${partialAppend}\n`); + expect(() => loadEntriesFromFile(malformedFile)).toThrow("malformed JSONL record at line 3"); + }); + + it("rejects complete non-object JSONL records", () => { + const file = join(tempDir, "non-object-record.jsonl"); + writeFileSync( + file, + '{"type":"session","version":3,"id":"abc","timestamp":"2025-01-01T00:00:00Z","cwd":"/tmp"}\nnull\n', + ); + + expect(() => loadEntriesFromFile(file)).toThrow("invalid JSONL object at line 2"); + }); + + it("does not treat sparse-file holes as blank JSONL records", () => { const file = join(tempDir, "large.jsonl"); writeFileSync( file, @@ -141,10 +165,7 @@ describe("loadEntriesFromFile", () => { '{"type":"message","id":"1","parentId":null,"timestamp":"2025-01-01T00:00:01Z","message":{"role":"user","content":"hi","timestamp":1}}\n', ); - const sessionManager = SessionManager.open(file, tempDir); - expect(sessionManager.getSessionId()).toBe("abc"); - expect(sessionManager.getEntries()).toHaveLength(1); - expect(sessionManager.buildSessionContext().messages).toEqual([{ role: "user", content: "hi", timestamp: 1 }]); + expect(() => SessionManager.open(file, tempDir)).toThrow("malformed JSONL record at line 2"); }); }); @@ -483,6 +504,78 @@ describe("SessionManager.setSessionFile with corrupted files", () => { expect(() => SessionManager.open(duplicateFile, tempDir)).toThrow("duplicate entry ID duplicate"); }); + it("rejects current-format orphan entries before plan mode can make the session unloadable", () => { + const orphanFile = join(tempDir, "orphan-entry.jsonl"); + const entries = [ + { + type: "session", + version: 3, + id: "orphan-session", + timestamp: "2025-01-01T00:00:00Z", + cwd: tempDir, + }, + { + type: "custom", + id: "root", + parentId: null, + timestamp: "2025-01-01T00:00:01Z", + customType: "root", + }, + { + type: "custom", + id: "orphan", + parentId: "missing", + timestamp: "2025-01-01T00:00:02Z", + customType: "orphan", + }, + ]; + writeFileSync(orphanFile, `${entries.map((entry) => JSON.stringify(entry)).join("\n")}\n`); + + expect(() => SessionManager.open(orphanFile, tempDir)).toThrow( + "Session entry orphan has a missing parent missing.", + ); + }); + + it("leaves the current session intact when switching to an invalid target", () => { + const session = SessionManager.inMemory(tempDir); + const currentEntryId = session.appendMessage({ role: "user", content: "current", timestamp: 1 }); + const currentSessionId = session.getSessionId(); + const currentEntries = session.getEntries(); + const targets = [ + { + name: "malformed", + content: + '{"type":"session","version":3,"id":"malformed-target","timestamp":"2025-01-01T00:00:00Z","cwd":"/tmp"}\n' + + "not json\n", + }, + { + name: "duplicate", + content: + '{"type":"session","version":3,"id":"duplicate-target","timestamp":"2025-01-01T00:00:00Z","cwd":"/tmp"}\n' + + '{"type":"custom","id":"duplicate","parentId":null,"timestamp":"2025-01-01T00:00:01Z","customType":"first"}\n' + + '{"type":"custom","id":"duplicate","parentId":null,"timestamp":"2025-01-01T00:00:02Z","customType":"second"}\n', + }, + { + name: "lifecycle", + content: + '{"type":"session","version":3,"id":"lifecycle-target","timestamp":"2025-01-01T00:00:00Z","cwd":"/tmp"}\n' + + '{"type":"plan","id":"invalid-plan","parentId":null,"timestamp":"2025-01-01T00:00:01Z","plan":{"id":"plan","title":"Invalid","markdown":"No mode"}}\n', + }, + ]; + + for (const target of targets) { + const targetFile = join(tempDir, `${target.name}.jsonl`); + writeFileSync(targetFile, target.content); + + expect(() => session.setSessionFile(targetFile)).toThrow(); + expect(session.getSessionFile()).toBeUndefined(); + expect(session.getSessionId()).toBe(currentSessionId); + expect(session.getEntries()).toEqual(currentEntries); + expect(session.getLeafId()).toBe(currentEntryId); + expect(session.getEntry(currentEntryId)).toEqual(currentEntries[0]); + } + }); + it("preserves explicit session file path when recovering from corrupted file", () => { const explicitPath = join(tempDir, "my-session.jsonl"); writeFileSync(explicitPath, ""); From c0ce5bb35f0c8f0ab943e8a30031f2789e0a27dc Mon Sep 17 00:00:00 2001 From: Jeffrey Tseng Date: Sat, 15 Aug 2026 16:49:42 -0700 Subject: [PATCH 2/4] fix(coding-agent): append exact plan continuations Require accepted plans to persist their trusted implementation prompt through a dedicated append path. --- .../coding-agent/src/core/agent-session.ts | 13 +++- .../coding-agent/src/core/session-manager.ts | 40 +++++++++++- .../session-manager/file-operations.test.ts | 65 +++++++++++++++++++ 3 files changed, 113 insertions(+), 5 deletions(-) diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 0db73b943..a48b3b567 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -868,9 +868,16 @@ export class AgentSession { event.message.role === "toolResult" ) { // Regular LLM message - persist as SessionMessageEntry - this.sessionManager.appendMessage(event.message); - if (event.message.role === "user" && event.message === this._activePlanContinuation?.userMessage) { - this._activePlanContinuation.persisted = true; + const activePlanContinuation = this._activePlanContinuation; + if ( + event.message.role === "user" && + activePlanContinuation !== undefined && + event.message === activePlanContinuation.userMessage + ) { + this.sessionManager.appendPlanContinuationMessage(event.message); + activePlanContinuation.persisted = true; + } else { + this.sessionManager.appendMessage(event.message); } } // Other message types (bashExecution, compactionSummary, branchSummary) are persisted elsewhere diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index 3306f7e95..3c0da7d15 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -27,7 +27,13 @@ import { createCompactionSummaryMessage, createCustomMessage, } from "./messages.ts"; -import { type CollaborationMode, type PlanArtifact, type PlanResolution, parsePlanArtifact } from "./plan-mode.ts"; +import { + type CollaborationMode, + createPlanImplementationPrompt, + type PlanArtifact, + type PlanResolution, + parsePlanArtifact, +} from "./plan-mode.ts"; export const CURRENT_SESSION_VERSION = 3; @@ -428,11 +434,20 @@ function applyPlanLifecycleEntry(state: PlanLifecycleState, entry: SessionEntry) }; } if (entry.type === "message" && entry.message.role === "user" && state.pendingPlanContinuation) { + if (!isExactPlanContinuationMessage(entry.message, state.pendingPlanContinuation)) { + throw new Error(`User message ${entry.id} does not match the pending plan continuation.`); + } return { ...state, pendingPlanContinuation: undefined }; } return state; } +function isExactPlanContinuationMessage(message: Message, plan: PlanArtifact): boolean { + if (message.role !== "user" || !Array.isArray(message.content) || message.content.length !== 1) return false; + const content = message.content[0]; + return content.type === "text" && content.text === createPlanImplementationPrompt(plan); +} + function validateSessionEntries(entries: SessionEntry[]): void { const seenIds = new Set(); for (const entry of entries) { @@ -1269,7 +1284,7 @@ export class SessionManager { * so it is easier to find them. * These need to be appended via appendCompaction() and appendBranchSummary() methods. */ - appendMessage(message: Message | CustomMessage | BashExecutionMessage): string { + private _appendMessage(message: Message | CustomMessage | BashExecutionMessage): string { const entry: SessionMessageEntry = { type: "message", id: generateId(this.byId), @@ -1281,6 +1296,27 @@ export class SessionManager { return entry.id; } + appendMessage(message: Message | CustomMessage | BashExecutionMessage): string { + if (message.role === "user" && this.buildSessionContext().pendingPlanContinuation) { + throw new Error( + "A plan continuation is pending. Persist it with appendPlanContinuationMessage before appending another user message.", + ); + } + return this._appendMessage(message); + } + + /** Persist the exact one-shot user message for the currently accepted plan. */ + appendPlanContinuationMessage(message: Message): string { + const pendingPlanContinuation = this.buildSessionContext().pendingPlanContinuation; + if (!pendingPlanContinuation) { + throw new Error("No plan continuation is pending."); + } + if (!isExactPlanContinuationMessage(message, pendingPlanContinuation)) { + throw new Error("The user message does not match the pending plan continuation."); + } + return this._appendMessage(message); + } + /** Append a thinking level change as child of current leaf, then advance leaf. Returns entry id. */ appendThinkingLevelChange(thinkingLevel: string): string { const entry: ThinkingLevelChangeEntry = { diff --git a/packages/coding-agent/test/session-manager/file-operations.test.ts b/packages/coding-agent/test/session-manager/file-operations.test.ts index bdc49c475..ccc56d935 100644 --- a/packages/coding-agent/test/session-manager/file-operations.test.ts +++ b/packages/coding-agent/test/session-manager/file-operations.test.ts @@ -3,6 +3,7 @@ import { appendFileSync, closeSync, mkdirSync, openSync, readFileSync, rmSync, w import { tmpdir } from "os"; import { join } from "path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { createPlanImplementationPrompt } from "../../src/core/plan-mode.ts"; import { findMostRecentSession, loadEntriesFromFile, SessionManager } from "../../src/core/session-manager.ts"; const HEADER_SCAN_LIMIT_BYTES = 1024 * 1024; @@ -380,6 +381,39 @@ describe("SessionManager plan persistence", () => { pendingPlan: undefined, }); }); + + it("only consumes an accepted plan through the exact continuation append API", () => { + const plan = { + id: "plan-continuation", + title: "Use the trusted continuation", + markdown: "## Steps\n\n1. Implement it.", + }; + const session = SessionManager.inMemory(tempDir); + session.appendCollaborationModeChange("plan"); + session.appendPlan(plan); + session.appendPlanResolution(plan.id, "implement"); + + expect(() => session.appendMessage({ role: "user", content: "unrelated prompt", timestamp: Date.now() })).toThrow( + "appendPlanContinuationMessage", + ); + expect(session.buildSessionContext().pendingPlanContinuation).toEqual(plan); + + expect(() => + session.appendPlanContinuationMessage({ + role: "user", + content: [{ type: "text", text: "wrong continuation" }], + timestamp: Date.now(), + }), + ).toThrow("does not match the pending plan continuation"); + expect(session.buildSessionContext().pendingPlanContinuation).toEqual(plan); + + session.appendPlanContinuationMessage({ + role: "user", + content: [{ type: "text", text: createPlanImplementationPrompt(plan) }], + timestamp: Date.now(), + }); + expect(session.buildSessionContext().pendingPlanContinuation).toBeUndefined(); + }); }); describe("SessionManager.setSessionFile with corrupted files", () => { @@ -536,6 +570,37 @@ describe("SessionManager.setSessionFile with corrupted files", () => { ); }); + it("rejects a persisted user message that bypasses the accepted-plan continuation API", () => { + const sessionFile = join(tempDir, "bypassed-plan-continuation.jsonl"); + const timestamp = "2025-01-01T00:00:00Z"; + const plan = { id: "accepted-plan", title: "Accepted", markdown: "1. Implement this." }; + const entries = [ + { type: "session", version: 3, id: "bypassed-continuation", timestamp, cwd: tempDir }, + { type: "collaboration_mode_change", id: "mode", parentId: null, timestamp, mode: "plan" }, + { type: "plan", id: "plan", parentId: "mode", timestamp, plan }, + { + type: "plan_resolution", + id: "resolution", + parentId: "plan", + timestamp, + planId: plan.id, + resolution: "implement", + }, + { + type: "message", + id: "untrusted-user", + parentId: "resolution", + timestamp, + message: { role: "user", content: "unrelated", timestamp: 1 }, + }, + ]; + writeFileSync(sessionFile, `${entries.map((entry) => JSON.stringify(entry)).join("\n")}\n`); + + expect(() => SessionManager.open(sessionFile, tempDir)).toThrow( + "User message untrusted-user does not match the pending plan continuation.", + ); + }); + it("leaves the current session intact when switching to an invalid target", () => { const session = SessionManager.inMemory(tempDir); const currentEntryId = session.appendMessage({ role: "user", content: "current", timestamp: 1 }); From 3d297ec19cefbaaf61b2905b022f5f89c7ccaf4a Mon Sep 17 00:00:00 2001 From: Jeffrey Tseng Date: Sat, 15 Aug 2026 16:49:53 -0700 Subject: [PATCH 3/4] test(coding-agent): cover persisted plan lifecycles Exercise plan validation and continuation recovery across persisted session branches and reloads. --- .../suite/session-manager-plan-mode.test.ts | 255 ++++++++++++++++++ 1 file changed, 255 insertions(+) create mode 100644 packages/coding-agent/test/suite/session-manager-plan-mode.test.ts diff --git a/packages/coding-agent/test/suite/session-manager-plan-mode.test.ts b/packages/coding-agent/test/suite/session-manager-plan-mode.test.ts new file mode 100644 index 000000000..1ca372810 --- /dev/null +++ b/packages/coding-agent/test/suite/session-manager-plan-mode.test.ts @@ -0,0 +1,255 @@ +import { writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { createPlanImplementationPrompt, type PlanArtifact } from "../../src/core/plan-mode.ts"; +import { SessionManager } from "../../src/core/session-manager.ts"; +import { createHarness } from "./harness.ts"; + +describe("plan-mode session context", () => { + it("rejects malformed persisted plans and normalizes newly appended artifacts", async () => { + const harness = await createHarness(); + try { + const sessionPath = join(harness.tempDir, "malformed-plan.jsonl"); + const timestamp = new Date().toISOString(); + writeFileSync( + sessionPath, + `${[ + { + type: "session", + version: 3, + id: "malformed-plan-session", + timestamp, + cwd: harness.tempDir, + }, + { + type: "collaboration_mode_change", + id: "mode-entry", + parentId: null, + timestamp, + mode: "plan", + }, + { + type: "plan", + id: "malformed-entry", + parentId: "mode-entry", + timestamp, + plan: { id: "malformed-plan", title: "Bad plan", markdown: 42 }, + }, + { + type: "message", + id: "child-entry", + parentId: "malformed-entry", + timestamp, + message: { role: "user", content: "This child must not become a default-mode root.", timestamp: 1 }, + }, + ] + .map((entry) => JSON.stringify(entry)) + .join("\n")}\n`, + ); + + expect(() => SessionManager.open(sessionPath)).toThrow("invalid persisted plan artifact"); + + const session = SessionManager.inMemory(); + session.appendCollaborationModeChange("plan"); + session.appendPlan({ id: "valid-plan", title: " Valid\n plan ", markdown: " 1. Preserve this. " }); + expect(session.buildSessionContext().pendingPlan).toEqual({ + id: "valid-plan", + title: "Valid plan", + markdown: "1. Preserve this.", + }); + expect(() => + session.appendPlan({ id: "unsafe plan id", title: "Unsafe", markdown: "1. Reject this." }), + ).toThrow("Invalid plan artifact"); + } finally { + harness.cleanup(); + } + }); + + it("rejects an orphan bridge left by a syntactically malformed plan line", async () => { + const harness = await createHarness(); + try { + const sessionPath = join(harness.tempDir, "malformed-plan-orphan.jsonl"); + const timestamp = new Date().toISOString(); + const header = JSON.stringify({ + type: "session", + version: 3, + id: "malformed-plan-orphan-session", + timestamp, + cwd: harness.tempDir, + }); + const mode = JSON.stringify({ + type: "collaboration_mode_change", + id: "mode-entry", + parentId: null, + timestamp, + mode: "plan", + }); + const child = JSON.stringify({ + type: "message", + id: "child-entry", + parentId: "malformed-plan-entry", + timestamp, + message: { role: "user", content: "Do not restore this child in default mode.", timestamp: 1 }, + }); + writeFileSync(sessionPath, `${header}\n${mode}\n{"type":"plan","id":"malformed-plan-entry"\n${child}\n`); + + expect(() => SessionManager.open(sessionPath)).toThrow("malformed JSONL record at line 3"); + } finally { + harness.cleanup(); + } + }); + + it("rejects a valid plan artifact persisted outside plan mode", async () => { + const harness = await createHarness(); + try { + const sessionPath = join(harness.tempDir, "plan-without-mode.jsonl"); + const timestamp = new Date().toISOString(); + writeFileSync( + sessionPath, + `${[ + { + type: "session", + version: 3, + id: "plan-without-mode-session", + timestamp, + cwd: harness.tempDir, + }, + { + type: "plan", + id: "plan-entry", + parentId: null, + timestamp, + plan: { id: "valid-plan", title: "Valid plan", markdown: "1. Must not fail open." }, + }, + ] + .map((entry) => JSON.stringify(entry)) + .join("\n")}\n`, + ); + + expect(() => SessionManager.open(sessionPath)).toThrow("outside plan mode"); + } finally { + harness.cleanup(); + } + }); + + it("rejects a persisted resolution that does not match the pending plan", async () => { + const harness = await createHarness(); + try { + const sessionPath = join(harness.tempDir, "mismatched-resolution.jsonl"); + const timestamp = new Date().toISOString(); + writeFileSync( + sessionPath, + `${[ + { + type: "session", + version: 3, + id: "mismatched-resolution-session", + timestamp, + cwd: harness.tempDir, + }, + { + type: "collaboration_mode_change", + id: "mode-entry", + parentId: null, + timestamp, + mode: "plan", + }, + { + type: "plan", + id: "plan-entry", + parentId: "mode-entry", + timestamp, + plan: { id: "pending-plan", title: "Pending", markdown: "1. Remain pending." }, + }, + { + type: "plan_resolution", + id: "resolution-entry", + parentId: "plan-entry", + timestamp, + planId: "different-plan", + resolution: "implement", + }, + ] + .map((entry) => JSON.stringify(entry)) + .join("\n")}\n`, + ); + + expect(() => SessionManager.open(sessionPath)).toThrow("does not match the pending plan"); + } finally { + harness.cleanup(); + } + }); + + it("enforces plan lifecycle transitions through public append methods", () => { + const session = SessionManager.inMemory(); + const plan: PlanArtifact = { id: "plan-lifecycle", title: "Lifecycle", markdown: "1. Enforce it." }; + + expect(() => session.appendPlan(plan)).toThrow("only be appended in plan mode"); + session.appendCollaborationModeChange("plan"); + expect(() => session.appendPlanResolution(plan.id, "implement")).toThrow("does not match"); + session.appendPlan(plan); + expect(() => session.appendCollaborationModeChange("default")).toThrow("Resolve the pending plan"); + expect(() => session.appendPlanResolution("another-plan", "implement")).toThrow("does not match"); + session.appendPlanResolution(plan.id, "implement"); + + expect(session.buildSessionContext()).toMatchObject({ + collaborationMode: "default", + pendingPlan: undefined, + pendingPlanContinuation: plan, + }); + expect(() => session.appendCollaborationModeChange("plan")).toThrow("pending plan continuation"); + session.appendPlanContinuationMessage({ + role: "user", + content: [{ type: "text", text: createPlanImplementationPrompt(plan) }], + timestamp: 1, + }); + expect(session.buildSessionContext().pendingPlanContinuation).toBeUndefined(); + }); + + it("restores pending plans from the active branch across compaction and resolution", () => { + const session = SessionManager.inMemory(); + const plan: PlanArtifact = { + id: "plan-1", + title: "Persisted plan", + markdown: "1. Preserve the artifact.", + }; + const modeEntryId = session.appendCollaborationModeChange("plan"); + const planEntryId = session.appendPlan(plan); + session.appendCompaction("Compacted planning context", modeEntryId, 100); + + expect(session.buildSessionContext()).toMatchObject({ collaborationMode: "plan", pendingPlan: plan }); + + const resolutionEntryId = session.appendPlanResolution(plan.id, "implement"); + expect(session.buildSessionContext()).toMatchObject({ + collaborationMode: "default", + pendingPlan: undefined, + pendingPlanContinuation: plan, + }); + session.appendPlanContinuationMessage({ + role: "user", + content: [{ type: "text", text: createPlanImplementationPrompt(plan) }], + timestamp: 1, + }); + expect(session.buildSessionContext().pendingPlanContinuation).toBeUndefined(); + session.branch(resolutionEntryId); + expect(session.buildSessionContext().pendingPlanContinuation).toEqual(plan); + + session.branch(planEntryId); + expect(session.buildSessionContext()).toMatchObject({ collaborationMode: "plan", pendingPlan: plan }); + }); + + it("derives default mode from a discarded plan resolution", () => { + const session = SessionManager.inMemory(); + const plan: PlanArtifact = { + id: "plan-discard-1", + title: "Discarded plan", + markdown: "1. Discard this plan.", + }; + session.appendCollaborationModeChange("plan"); + session.appendPlan(plan); + session.appendPlanResolution(plan.id, "discard"); + + expect(session.buildSessionContext()).toMatchObject({ collaborationMode: "default", pendingPlan: undefined }); + expect(session.getEntries().filter((entry) => entry.type === "collaboration_mode_change")).toHaveLength(1); + }); +}); From ce961c6f594fa000c606986438d7de10b2661ba6 Mon Sep 17 00:00:00 2001 From: Jeffrey Tseng Date: Sun, 16 Aug 2026 09:28:14 -0700 Subject: [PATCH 4/4] fix(coding-agent): harden persisted plan lifecycle state - Malformed or orphaned plan entries could leave a resumed session claiming a pending plan that no longer existed - The strict entry parser also called the profile-only normalizeSessionHeader helper, so it is dropped here for the same reason as the lenient parser --- packages/coding-agent/src/core/session-manager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index 3c0da7d15..5ceeefbad 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -693,7 +693,7 @@ function parseStrictSessionEntryLine( if (!plan) throw new Error("Session contains an invalid persisted plan artifact."); return { ...entry, plan }; } - return entry.type === "session" ? normalizeSessionHeader(entry) : entry; + return entry; } function loadEntriesFromDescriptor(fd: number): FileEntry[] {