From b79134acbe89fad0ca0ad8b0c6fb1c130f72a641 Mon Sep 17 00:00:00 2001 From: Agent Manager Date: Wed, 5 Aug 2026 19:41:57 +0000 Subject: [PATCH] codex: capture the pin in a shared folder, so a restart can resume MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A codex pane sharing its folder with another live codex session never got a conversation pin at all: the watcher tick checked folderIsShared FIRST and skipped tryCaptureCodexId entirely, so codexSessionId/codexRollout stayed empty for the life of the pane. resumeCmd then correctly refuses `resume --last` in a shared folder and runs bare `codex`, so every restart came back empty with the conversation still on disk. Observed live: am-image-inputs-aee9c3 ran ~3h in /data/workspaces/agent-manager alongside agent-manager-5-b0fa07, logged [codex] am-image-inputs-aee9c3: folder shared with another live session — not following thread resets here twice, and restarted into an empty pane while its 277-line rollout (019fd29a-97e5-7d11-b36e-3ce290b0a0c1) sat unreferenced. The guard was written for reset-following, where a new conversation in a shared folder is genuinely unattributable — our /clear, or a sibling's? The INITIAL capture has no such ambiguity, and tryCaptureCodexId already carries the checks that make it safe: the claimed set, the cwd match, born-after-launch, and the subagent filter. Split the two: capture always runs, reset-following still stops at a shared folder. - codexCandidate() is split out of tryCaptureCodexId (mirrors claudeCandidate) and takes a `bornBefore` cap, so a sibling's LATER conversation stays out of reach while our own launch-window one is claimable. - codexCaptureMode() is the pure policy: follow / window / pinned / expired. - 'expired' logs once, so a pane that really found no rollout says so instead of restarting empty in silence. Codex launches within ~15s of each other in one folder remain ambiguous — the existing born-after-launch slack — and unlike claude (#23, #35) codex has no breadcrumb to disambiguate. Noted, not fixed here. server/test/repin.test.mjs: 34 checks, up from 21. Full npm test green (5 suites, 50 checks). Verified the new checks have teeth — dropping the bornBefore cap fails 2. Co-Authored-By: Claude Opus 5 --- server/src/runner.js | 85 +++++++++++++++++++++++++++++--------- server/test/repin.test.mjs | 53 ++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 19 deletions(-) diff --git a/server/src/runner.js b/server/src/runner.js index b5aa781..b4f5d91 100644 --- a/server/src/runner.js +++ b/server/src/runner.js @@ -751,9 +751,13 @@ function firstLine(p) { } finally { fs.closeSync(fd); } } -function tryCaptureCodexId(sessionId, workdir, sinceMs) { +// The newest rollout written in `workdir` that no other session has pinned, as +// { id, p }. `bornBefore` caps how late a conversation may have been BORN and +// still be taken as ours; in a shared folder that cap is what keeps a sibling's +// later reset out of reach (see codexCaptureMode). +// Exported for server/test/repin.test.mjs. +export function codexCandidate(sessionId, workdir, sinceMs, { bornBefore = Infinity } = {}) { const claimed = new Set(list().filter((s) => s.id !== sessionId && s.codexSessionId).map((s) => s.codexSessionId)); - const pinned = (list().find((s) => s.id === sessionId) || {}).codexSessionId; for (const c of codexRolloutsSince(sinceMs)) { const m = c.p.match(/rollout-.*-([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})\.jsonl$/); if (!m || claimed.has(m[1])) continue; @@ -764,20 +768,28 @@ function tryCaptureCodexId(sessionId, workdir, sinceMs) { // A sibling's ongoing conversation in the same folder gets fresh writes // (mtime) during our capture window — require the rollout to have been // CREATED after this launch so we never claim someone else's thread. - const created = Date.parse(mp.timestamp || meta.timestamp || '') || 0; - if (created && created < sinceMs - 15_000) continue; + const born = Date.parse(mp.timestamp || meta.timestamp || '') || 0; + if (born && born < sinceMs - 15_000) continue; + if (born && born > bornBefore) continue; // Skip Codex's internal guardian/subagent rollouts — they share the cwd but // aren't this agent's conversation, so pinning one would break resume and // the Overview digest. if (mp.thread_source === 'subagent' || (mp.source && mp.source.subagent)) continue; - // The watcher re-runs for the life of the pane, so the usual outcome is - // "still the same conversation" — don't rewrite sessions.json for that. - if (m[1] === pinned) return true; - if (pinned) console.warn(`[codex] re-pinning ${sessionId}: ${pinned} -> ${m[1]} (conversation was replaced)`); - update(sessionId, { codexSessionId: m[1], codexRollout: c.p }); - return true; + return { id: m[1], p: c.p }; } - return false; + return null; +} + +function tryCaptureCodexId(sessionId, workdir, sinceMs, opts) { + const c = codexCandidate(sessionId, workdir, sinceMs, opts); + if (!c) return false; + const pinned = (list().find((s) => s.id === sessionId) || {}).codexSessionId; + // The watcher re-runs for the life of the pane, so the usual outcome is + // "still the same conversation" — don't rewrite sessions.json for that. + if (c.id === pinned) return true; + if (pinned) console.warn(`[codex] re-pinning ${sessionId}: ${pinned} -> ${c.id} (conversation was replaced)`); + update(sessionId, { codexSessionId: c.id, codexRollout: c.p }); + return true; } // A pin captured before subagents were filtered out (or one whose rollout was @@ -793,11 +805,38 @@ function pinIsStale(session) { } catch { return false; } } +// How late after launch a conversation may be born and still be taken as ours +// when the folder is shared. Codex writes its session_meta line as the TUI +// starts — 3.5s after launch in the session that motivated this — so two +// minutes is ample margin, while a sibling's reset minutes or hours in stays +// out of reach. +const CODEX_PIN_WINDOW_MS = 120_000; + +// What the watcher may do on this tick: +// 'follow' — capture, and keep following resets: this pane owns the folder. +// 'window' — unpinned in a shared folder. Capture, but only a conversation +// born in our launch window. This case used to do NOTHING, which +// meant a codex pane sharing a folder never got a pin at all: its +// conversation was on disk, and every restart came back empty, +// because resumeCmd correctly refuses `resume --last` there. +// 'pinned' — pinned in a shared folder: a NEW conversation here is +// unattributable (our reset, or a sibling's?), so don't follow it. +// 'expired' — shared, unpinned, window gone: nothing safe left to take. +// Exported for server/test/repin.test.mjs. +export function codexCaptureMode({ shared, pinned, nowMs, sinceMs }) { + if (!shared) return 'follow'; + if (pinned) return 'pinned'; + return nowMs <= sinceMs + CODEX_PIN_WINDOW_MS ? 'window' : 'expired'; +} + // Same staleness problem as Claude's pin, same remedy: codex starts a fresh // conversation — and a fresh rollout file — when the thread is reset, so a pin // captured once at launch stops describing the live conversation. Keep watching // for as long as the pane is alive and follow the newest rollout this folder // produces. tryCaptureCodexId only writes when the id actually changes. +// +// A shared folder narrows that to the initial capture only — see +// codexCaptureMode for which part is safe there and which isn't. function scheduleCodexCapture(session, workdir) { if (session.codexSessionId && pinIsStale(session)) { session = update(session.id, { codexSessionId: undefined, codexRollout: undefined }) || session; @@ -805,17 +844,25 @@ function scheduleCodexCapture(session, workdir) { const prev = codexCapturing.get(session.id); if (prev) clearTimeout(prev); const since = Date.now() - 2000; - let warnedShared = false; + let warnedShared = false, warnedExpired = false; const tick = () => { if (!isRunning(session.id)) { codexCapturing.delete(session.id); return; } - if (folderIsShared(session.id, workdir, 'codex')) { - if (!warnedShared) { - warnedShared = true; - console.warn(`[codex] ${session.id}: folder shared with another live session — not following thread resets here`); - } - } else { - tryCaptureCodexId(session.id, workdir, since); + const mode = codexCaptureMode({ + shared: folderIsShared(session.id, workdir, 'codex'), + pinned: !!(list().find((s) => s.id === session.id) || {}).codexSessionId, + nowMs: Date.now(), + sinceMs: since, + }); + if (mode === 'follow') tryCaptureCodexId(session.id, workdir, since); + else if (mode === 'window') { + tryCaptureCodexId(session.id, workdir, since, { bornBefore: since + CODEX_PIN_WINDOW_MS }); + } else if (mode === 'pinned' && !warnedShared) { + warnedShared = true; + console.warn(`[codex] ${session.id}: folder shared with another live session — not following thread resets here`); + } else if (mode === 'expired' && !warnedExpired) { + warnedExpired = true; + console.warn(`[codex] ${session.id}: no rollout born in this folder within ${CODEX_PIN_WINDOW_MS / 1000}s of launch — staying unpinned, so a restart will start a fresh conversation`); } const t = setTimeout(tick, REPIN_MS); if (t.unref) t.unref(); diff --git a/server/test/repin.test.mjs b/server/test/repin.test.mjs index dcc2222..1e84500 100644 --- a/server/test/repin.test.mjs +++ b/server/test/repin.test.mjs @@ -15,6 +15,7 @@ fs.mkdirSync(DATA, { recursive: true }); process.env.CLAUDE_CONFIG_DIR = CFG; process.env.DATA_DIR = DATA; +process.env.CODEX_HOME = path.join(TMP, 'codex'); const sessions = await import('../src/sessions.js'); const runner = await import('../src/runner.js'); @@ -109,6 +110,58 @@ check('malformed session_id rejected', verdict(crumb({ payload: { session_id: 'not-a-uuid', cwd: WORKDIR } }), facts()).repin, null); check('null crumb rejected', verdict(null, facts()).repin, null); +// ---------- codex: the pin a folder-sharing pane never got ---------- +// Codex has no breadcrumb, so a shared folder used to skip capture ENTIRELY and +// the pane stayed unpinned for life — conversation on disk, every restart empty. +// The initial capture is safe there; following a later reset is not. +const R = (n) => `019fd29a-97e5-7d11-b36e-3ce290b0a00${n}`; +function rollout(id, { cwd = WORKDIR, bornMs, mtimeMs, subagent = false }) { + const dir = path.join(process.env.CODEX_HOME, 'sessions', '2026', '08', '05'); + fs.mkdirSync(dir, { recursive: true }); + const p = path.join(dir, `rollout-2026-08-05T15-46-14-${id}.jsonl`); + const payload = { cwd, timestamp: new Date(bornMs).toISOString() }; + if (subagent) payload.thread_source = 'subagent'; + fs.writeFileSync(p, JSON.stringify({ type: 'session_meta', payload }) + '\n'); + const t = (mtimeMs ?? bornMs) / 1000; + fs.utimesSync(p, t, t); + return p; +} +const WINDOW = { bornBefore: SINCE + 120_000 }; + +console.log('\ncodex: the conversation born in this launch window is ours'); +rollout(R(1), { bornMs: NOW }); +check('own rollout captured', runner.codexCandidate('c1', WORKDIR, SINCE)?.id, R(1)); + +console.log('\ncodex: whose folder and whose birth, not whose mtime'); +rollout(R(2), { cwd: path.join(cfg.WORKSPACES_DIR, 'proj-b'), bornMs: NOW + 1000, mtimeMs: NOW + 120_000 }); +check('other folder ignored', runner.codexCandidate('c1', WORKDIR, SINCE)?.id, R(1)); +rollout(R(3), { bornMs: NOW - 3600_000, mtimeMs: NOW + 180_000 }); +check('pre-launch rollout rejected despite a fresh mtime', + runner.codexCandidate('c1', WORKDIR, SINCE)?.id, R(1)); +rollout(R(4), { bornMs: NOW + 5000, mtimeMs: NOW + 200_000, subagent: true }); +check('subagent rollout skipped', runner.codexCandidate('c1', WORKDIR, SINCE)?.id, R(1)); + +console.log('\ncodex: a later conversation is a reset to follow, or a sibling to leave alone'); +rollout(R(5), { bornMs: NOW + 240_000 }); +check('followed when the folder is ours', runner.codexCandidate('c1', WORKDIR, SINCE)?.id, R(5)); +check('out of reach in a shared folder', runner.codexCandidate('c1', WORKDIR, SINCE, WINDOW)?.id, R(1)); + +console.log('\ncodex: a rollout another session pinned is left to it'); +const rivalCodex = sessions.create({ name: 'rival-codex', cli: 'codex', path: 'proj-a' }); +sessions.update(rivalCodex.id, { codexSessionId: R(1) }); +check('claimed rollout skipped', runner.codexCandidate('c1', WORKDIR, SINCE, WINDOW), null); + +console.log('\ncodex: what the watcher may do on a tick'); +const mode = (o) => runner.codexCaptureMode({ nowMs: NOW, sinceMs: SINCE, ...o }); +check('sole pane captures and follows resets', mode({ shared: false, pinned: false }), 'follow'); +check('sole pane keeps following once pinned', mode({ shared: false, pinned: true }), 'follow'); +check('shared and unpinned still captures', mode({ shared: true, pinned: false }), 'window'); +check('shared and pinned stops at the pin', mode({ shared: true, pinned: true }), 'pinned'); +check('window is open at its last instant', + mode({ shared: true, pinned: false, nowMs: SINCE + 120_000 }), 'window'); +check('shared, unpinned, window gone', + mode({ shared: true, pinned: false, nowMs: SINCE + 120_001 }), 'expired'); + // ---------- hook installer: merge, never replace; idempotent; refuse corrupt ---------- console.log('\ninstaller merges into existing settings and is idempotent'); const settings = path.join(CFG, 'settings.json');