From 6255df4db63dfbb7e968c6ecd370dd2b894d5889 Mon Sep 17 00:00:00 2001 From: Harsh Date: Fri, 6 Mar 2026 11:20:27 +0530 Subject: [PATCH] fix: harden OpenCode session id handling and title reuse selection --- .../src/__tests__/session-manager.test.ts | 21 +++++++++++++++ packages/core/src/session-manager.ts | 22 +++++++++++---- .../plugins/agent-opencode/src/index.test.ts | 27 +++++++++++++++++++ packages/plugins/agent-opencode/src/index.ts | 20 +++++++++++--- 4 files changed, 82 insertions(+), 8 deletions(-) diff --git a/packages/core/src/__tests__/session-manager.test.ts b/packages/core/src/__tests__/session-manager.test.ts index b32f2c00a..ef447d5fb 100644 --- a/packages/core/src/__tests__/session-manager.test.ts +++ b/packages/core/src/__tests__/session-manager.test.ts @@ -1450,6 +1450,27 @@ describe("kill", () => { const deleteLog = readFileSync(deleteLogPath, "utf-8"); expect(deleteLog).toContain("session delete ses_purge"); }); + + it("skips purge when mapped OpenCode session id is invalid", async () => { + const deleteLogPath = join(tmpDir, "opencode-delete-kill-invalid.log"); + const mockBin = installMockOpencode("[]", deleteLogPath); + process.env.PATH = `${mockBin}:${originalPath ?? ""}`; + + writeMetadata(sessionsDir, "app-1", { + worktree: "/tmp/ws", + branch: "main", + status: "working", + project: "my-app", + agent: "opencode", + opencodeSessionId: "ses bad id", + runtimeHandle: JSON.stringify(makeHandle("rt-1")), + }); + + const sm = createSessionManager({ config, registry: mockRegistry }); + await sm.kill("app-1", { purgeOpenCode: true }); + + expect(existsSync(deleteLogPath)).toBe(false); + }); }); describe("cleanup", () => { diff --git a/packages/core/src/session-manager.ts b/packages/core/src/session-manager.ts index 42d31eb87..e78935daf 100644 --- a/packages/core/src/session-manager.ts +++ b/packages/core/src/session-manager.ts @@ -64,6 +64,14 @@ import { const execFileAsync = promisify(execFile); const OPENCODE_DISCOVERY_TIMEOUT_MS = 2_000; const OPENCODE_INTERACTIVE_DISCOVERY_TIMEOUT_MS = 10_000; +const OPENCODE_SESSION_ID_RE = /^ses_[A-Za-z0-9_-]+$/; + +function asValidOpenCodeSessionId(value: unknown): string | undefined { + if (typeof value !== "string") return undefined; + const trimmed = value.trim(); + if (trimmed.length === 0) return undefined; + return OPENCODE_SESSION_ID_RE.test(trimmed) ? trimmed : undefined; +} function errorIncludesSessionNotFound(err: unknown): boolean { if (!(err instanceof Error)) return false; @@ -73,6 +81,8 @@ function errorIncludesSessionNotFound(err: unknown): boolean { } async function deleteOpenCodeSession(sessionId: string): Promise { + const validatedSessionId = asValidOpenCodeSessionId(sessionId); + if (!validatedSessionId) return; const retryDelaysMs = [0, 200, 600]; let lastError: unknown; for (const delayMs of retryDelaysMs) { @@ -80,7 +90,9 @@ async function deleteOpenCodeSession(sessionId: string): Promise { await new Promise((resolve) => setTimeout(resolve, delayMs)); } try { - await execFileAsync("opencode", ["session", "delete", sessionId], { timeout: 30_000 }); + await execFileAsync("opencode", ["session", "delete", validatedSessionId], { + timeout: 30_000, + }); return; } catch (err) { if (errorIncludesSessionNotFound(err)) { @@ -118,8 +130,8 @@ async function discoverOpenCodeSessionIdsByTitle( }); return candidates - .map((entry) => entry["id"]) - .filter((id): id is string => typeof id === "string" && id.length > 0); + .map((entry) => asValidOpenCodeSessionId(entry["id"])) + .filter((id): id is string => typeof id === "string"); } catch { return []; } @@ -377,8 +389,8 @@ export function createSessionManager(deps: SessionManagerDeps): SessionManager { const ids: string[] = []; const maybeAdd = (id: string, raw: Record | null) => { if (!matchesCriteria(id, raw)) return; - const mapped = raw?.["opencodeSessionId"]; - if (typeof mapped !== "string" || mapped.length === 0) return; + const mapped = asValidOpenCodeSessionId(raw?.["opencodeSessionId"]); + if (!mapped) return; ids.push(mapped); }; diff --git a/packages/plugins/agent-opencode/src/index.test.ts b/packages/plugins/agent-opencode/src/index.test.ts index ad401fbea..9a02ddebe 100644 --- a/packages/plugins/agent-opencode/src/index.test.ts +++ b/packages/plugins/agent-opencode/src/index.test.ts @@ -485,6 +485,33 @@ describe("detectActivity", () => { }); }); +describe("getActivityState", () => { + const agent = create(); + + it("returns null when opencode session list output is malformed JSON", async () => { + mockExecFileAsync.mockImplementation((cmd: string) => { + if (cmd === "tmux") return Promise.resolve({ stdout: "/dev/ttys003\n", stderr: "" }); + if (cmd === "ps") { + return Promise.resolve({ + stdout: " PID TT ARGS\n 789 ttys003 opencode\n", + stderr: "", + }); + } + if (cmd === "opencode") return Promise.resolve({ stdout: "not json", stderr: "" }); + return Promise.reject(new Error("unexpected")); + }); + + const state = await agent.getActivityState( + makeSession({ + runtimeHandle: makeTmuxHandle(), + metadata: { opencodeSessionId: "ses_abc123" }, + }), + ); + + expect(state).toBeNull(); + }); +}); + // ========================================================================= // getSessionInfo // ========================================================================= diff --git a/packages/plugins/agent-opencode/src/index.ts b/packages/plugins/agent-opencode/src/index.ts index aa28c7cc6..c3abe09af 100644 --- a/packages/plugins/agent-opencode/src/index.ts +++ b/packages/plugins/agent-opencode/src/index.ts @@ -20,13 +20,27 @@ interface OpenCodeSessionListEntry { updated?: string; } +const OPENCODE_SESSION_ID_RE = /^ses_[A-Za-z0-9_-]+$/; + +function asValidOpenCodeSessionId(value: unknown): string | undefined { + if (typeof value !== "string") return undefined; + const trimmed = value.trim(); + if (trimmed.length === 0) return undefined; + return OPENCODE_SESSION_ID_RE.test(trimmed) ? trimmed : undefined; +} + function parseSessionList(raw: string): OpenCodeSessionListEntry[] { - const parsed: unknown = JSON.parse(raw); + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return []; + } if (!Array.isArray(parsed)) return []; return parsed.filter((item): item is OpenCodeSessionListEntry => { if (!item || typeof item !== "object") return false; const record = item as Record; - return typeof record["id"] === "string"; + return asValidOpenCodeSessionId(record["id"]) !== undefined; }); } @@ -99,7 +113,7 @@ function createOpenCodeAgent(): Agent { const runCommand = promptValue ? ["opencode", "run", ...runOptions, promptValue].join(" ") : ["opencode", "run", ...runOptions, "--command", "true"].join(" "); - const continueSession = `"$(opencode session list --format json | node -e ${shellEscape("let input='';process.stdin.on('data',c=>input+=c).on('end',()=>{const title=process.argv[1];const rows=JSON.parse(input);if(!Array.isArray(rows))process.exit(1);const matches=rows.filter((r)=>r&&r.title===title&&typeof r.id==='string');if(matches.length===0)process.exit(1);process.stdout.write(matches[0].id);});")} ${shellEscape(`AO:${config.sessionId}`)})"`; + const continueSession = `"$(opencode session list --format json | node -e ${shellEscape("let input='';process.stdin.on('data',c=>input+=c).on('end',()=>{const title=process.argv[1];const rows=JSON.parse(input);if(!Array.isArray(rows))process.exit(1);const matches=rows.filter((r)=>r&&r.title===title&&typeof r.id==='string').sort((a,b)=>{const aUpdated=Date.parse(typeof a.updated==='string'?a.updated:'');const bUpdated=Date.parse(typeof b.updated==='string'?b.updated:'');const aScore=Number.isNaN(aUpdated)?0:aUpdated;const bScore=Number.isNaN(bUpdated)?0:bUpdated;return bScore-aScore;});if(matches.length===0)process.exit(1);process.stdout.write(matches[0].id);});")} ${shellEscape(`AO:${config.sessionId}`)})"`; const continueCommand = ["opencode", "--session", continueSession, ...sharedOptions].join( " ", );