diff --git a/src/codex-transcript.ts b/src/codex-transcript.ts index fc9251c3..9f360ee3 100644 --- a/src/codex-transcript.ts +++ b/src/codex-transcript.ts @@ -42,7 +42,9 @@ import { open, readdir, stat } from 'node:fs/promises'; import { homedir } from 'node:os'; -import { basename, join } from 'node:path'; +import { join } from 'node:path'; + +import { LRUMap } from './utils/lru-map.js'; /** Covers `session_meta` (~19 KiB) plus the first user message (~69 KiB behind it). */ const HEAD_BYTES = 131072; @@ -51,16 +53,19 @@ const HEAD_BYTES = 131072; const TAIL_BYTES = 65536; /** - * Newest rollouts to report. A store this size is already far more than any list - * shows, and the cap keeps one enormous `~/.codex` from stalling a request. + * Newest rollouts to REPORT. Counted in emitted rows, not files scanned: the + * store is mostly sub-agent threads this never returns, so capping files first + * would spend the budget on rows nobody sees. */ const MAX_ROLLOUTS = 400; /** - * How many of those also get a tail read for `lastPrompt`. The head read is - * cached forever (see below) but the tail cannot be, because appending to a - * rollout is exactly what changes it, so this is the one genuinely per-request - * cost and it stays bounded. + * How many emitted rows also get a tail read for `lastPrompt`. The head read is + * cached (see below) but the tail cannot be, because appending to a rollout is + * exactly what changes it, so this is the one genuinely per-request cost and it + * stays bounded. Counted in emitted rows for the same reason as above — against + * file index a store of sub-agent threads spends the whole budget before the + * first row that needed it. */ const MAX_TAIL_READS = 100; @@ -73,6 +78,13 @@ const MIN_ROLLOUT_BYTES = 100; export interface CodexHistorySession { /** The rollout's own thread id — the token `codex resume ` expects. */ sessionId: string; + /** + * `session_meta.originator`, which codex stamps from + * CODEX_INTERNAL_ORIGINATOR_OVERRIDE — `codeman_` for every pane + * Codeman spawns. The only link between a FRESH codex pane and the rollout it + * is writing, since such a pane knows no thread id of its own. + */ + originator?: string; workingDir: string; sizeBytes: number; /** ISO timestamp, from the file's own mtime. */ @@ -87,17 +99,40 @@ interface RolloutIdentity { cwd?: string; /** `'subagent'` marks a thread codex spawned for itself. */ threadSource?: string; + /** `codeman_` for a pane Codeman spawned; codex's own default otherwise. */ + originator?: string; firstPrompt?: string; } /** * `session_meta` is written once and never rewritten — the same fact - * `readCodexRolloutMetaCached()` in session-routes.ts relies on — and the first - * user message cannot change either. So a path's identity is cached for the life - * of the process, and a rescan costs a `stat` per file plus head reads for + * `readCodexRolloutMetaCached()` in session-routes.ts relies on — so a path's + * identity is cached, and a rescan costs a `stat` per file plus head reads for * rollouts this process has not seen before. + * + * ⚠️ The first user message is NOT written up front: codex writes it when the + * user submits. Caching before then pins `firstPrompt: undefined` for the life + * of the process, and every scan of the home screen, the command palette and the + * search-index refresh can land in that window — so the row reads as having no + * prompt until a restart. `shouldCacheIdentity()` is the guard. + * + * Bounded, unlike a plain Map: this process runs for days and every sub-agent + * rollout adds an entry. Same reason and same size as `codexRolloutMetaCache`. */ -const identityCache = new Map(); +const identityCache = new LRUMap({ maxSize: 4096 }); + +/** + * Is this identity settled enough to keep? + * + * A known `firstPrompt` settles it. So does a head read that FILLED its window, + * which means the prompt is genuinely not in the first `HEAD_BYTES` rather than + * not written yet. A short file with no prompt is the ambiguous case — codex is + * still to write one — so that one is re-read next scan. + */ +function shouldCacheIdentity(identity: RolloutIdentity, fileSize: number): boolean { + if (!identity.threadId) return false; + return identity.firstPrompt !== undefined || fileSize >= HEAD_BYTES; +} function codexSessionsRoot(): string { const home = process.env.CODEX_HOME || join(homedir(), '.codex'); @@ -211,6 +246,7 @@ function parseIdentity(head: string): RolloutIdentity { session_id?: string; cwd?: string; thread_source?: string; + originator?: string; type?: string; role?: string; content?: unknown; @@ -228,6 +264,7 @@ function parseIdentity(head: string): RolloutIdentity { out.threadId ??= p.id || p.session_id; out.cwd ??= p.cwd; out.threadSource ??= p.thread_source; + out.originator ??= p.originator; } else if (entry.type === 'turn_context' && p) { out.cwd ??= p.cwd; } @@ -294,34 +331,29 @@ async function listRollouts(root: string): Promise { - const files = (await listRollouts(codexSessionsRoot())).slice(0, MAX_ROLLOUTS); + const files = await listRollouts(codexSessionsRoot()); const out: CodexHistorySession[] = []; - for (const [index, file] of files.entries()) { + for (const file of files) { + if (out.length >= MAX_ROLLOUTS) break; + let identity = identityCache.get(file.path); if (!identity) { identity = parseIdentity(await readHead(file.path, HEAD_BYTES)); - // A rollout still being created may not have flushed session_meta yet; - // caching that would pin an empty identity for the life of the process. - if (identity.threadId) identityCache.set(file.path, identity); + if (shouldCacheIdentity(identity, file.size)) identityCache.set(file.path, identity); } if (!identity.threadId || identity.threadSource === 'subagent') continue; - - // The filename ends in the thread id, so a rollout whose head window was too - // small to reach session_meta still yields an id worth resuming. - const fromName = basename(file.path) - .replace(/\.jsonl$/, '') - .split('-') - .slice(-5) - .join('-'); - const sessionId = identity.threadId || fromName; + // A row with no directory has nowhere to resume INTO, and emitting an empty + // one makes a click post `workingDir: ''`. omp drops such a row; so does this. + if (!identity.cwd) continue; const lastPrompt = - index < MAX_TAIL_READS ? parseLastPrompt(await readTail(file.path, file.size, TAIL_BYTES)) : undefined; + out.length < MAX_TAIL_READS ? parseLastPrompt(await readTail(file.path, file.size, TAIL_BYTES)) : undefined; out.push({ - sessionId, - workingDir: identity.cwd || '', + sessionId: identity.threadId, + originator: identity.originator, + workingDir: identity.cwd, sizeBytes: file.size, lastModified: new Date(file.mtimeMs).toISOString(), firstPrompt: identity.firstPrompt, @@ -332,6 +364,29 @@ export async function scanCodexSessionsHistory(): Promise return out; } +/** + * Which codex thread each Codeman-spawned pane is writing, keyed by Codeman + * session id. + * + * Codeman spawns every codex pane with + * CODEX_INTERNAL_ORIGINATOR_OVERRIDE=codeman_, and codex stamps that + * into `session_meta.originator`. That is the ONLY link between a fresh codex + * pane and the rollout it is writing: such a pane knows no thread id of its own, + * so it cannot be folded into its own Past-Sessions row from its own side. + * + * Newest wins. `/new` typed inside the codex TUI leaves several rollouts sharing + * one originator, and the pane is on the most recent — so this expects `rows` + * newest-first, as `scanCodexSessionsHistory()` returns them. + */ +export function codexThreadBySessionId(rows: CodexHistorySession[]): Map { + const out = new Map(); + for (const row of rows) { + const owner = /^codeman_(.+)$/.exec(row.originator ?? '')?.[1]; + if (owner && !out.has(owner)) out.set(owner, row.sessionId); + } + return out; +} + /** Test seam: drop the per-path identity cache. */ export function __clearCodexIdentityCache(): void { identityCache.clear(); diff --git a/src/services/unified-session-service.ts b/src/services/unified-session-service.ts index 6af01801..9bd447b3 100644 --- a/src/services/unified-session-service.ts +++ b/src/services/unified-session-service.ts @@ -2,12 +2,16 @@ * @fileoverview Pure merge/filter logic for the unified session list (COD-121). * * Combines four read-only views of a session — live (in-memory `Session`), - * persisted (`state.json`), transcript history (`~/.claude/projects`), and the - * lifecycle audit log — plus mux process stats, into one de-duplicated list - * keyed by sessionId. Transcript-history rows are keyed by the Claude - * conversation UUID (the `.jsonl` filename stem), which diverges from the - * Codeman id for resumed sessions — an alias map (claudeSessionId → Codeman id, - * built from the live/persisted views) folds them into the owning session item. + * persisted (`state.json`), transcript history, and the lifecycle audit log — + * plus mux process stats, into one de-duplicated list keyed by sessionId. + * + * Transcript history is not one source but three, because the CLIs keep their + * conversations in their own stores: Claude's `~/.claude/projects`, omp's + * `~/.omp/agent/sessions` and codex's `~/.codex/sessions`. Each row is keyed by + * whatever id that CLI names the conversation with, which diverges from the + * Codeman id for a resumed session and for every non-Claude one — an alias map + * (claudeSessionId → Codeman id, built from the live/persisted views) folds them + * into the owning session item. * Higher-precedence sources overwrite scalar fields when present * (history < lifecycle < persisted < live), while the `sources` array * always accumulates every contributing view. A "meaningfulness floor" drops diff --git a/src/session.ts b/src/session.ts index 1598df90..ca5da833 100644 --- a/src/session.ts +++ b/src/session.ts @@ -703,13 +703,20 @@ export class Session extends EventEmitter { this._wireActivityAt = config.lastActivityAt || Date.now(); this._wireActivitySettleUntil = config.lastActivityAt ? Date.now() + WIRE_ACTIVITY_SETTLE_MS : 0; // Set claudeSessionId — when resuming, the Claude conversation ID is the resumed one. - // For omp, `claudeSessionId` doubles as the generic "external transcript id" - // alias key mergeUnifiedSessions() folds a history row into its owning - // session by: omp mints its OWN uuid, unrelated to this Codeman id, so - // without this an omp conversation's Past-Sessions row (keyed by omp's - // id) would never merge with its own live/persisted row (keyed by this - // id) — it would just show up a second time. - this._claudeSessionId = config.resumeSessionId || config.ompConfig?.resumeSessionId || this.id; + // For omp and codex, `claudeSessionId` doubles as the generic "external + // transcript id" alias key mergeUnifiedSessions() folds a history row into + // its owning session by: each mints its OWN thread id, unrelated to this + // Codeman id, so without this the conversation's Past-Sessions row (keyed by + // that thread id) would never merge with its own live/persisted row (keyed + // by this id) — it would just show up a second time. For codex a duplicate + // is worse than cosmetic: the stale row still resumes, so clicking it starts + // a SECOND `codex resume` on a thread already open in another pane. + // + // This covers a RESUMED codex session, which knows its thread id up front. A + // fresh one learns its id only once codex writes the rollout, so it is folded + // from the other side — see the originator stamping in `gatherUnifiedInputs()`. + this._claudeSessionId = + config.resumeSessionId || config.ompConfig?.resumeSessionId || config.codexConfig?.resumeSessionId || this.id; // Restored from state.json on boot recovery. start() resets _claudeSessionId // to the launch id even when re-attaching to a mux session whose CLI has // moved on (a `/clear` before the restart), so this anchor is what lets the @@ -1988,8 +1995,12 @@ export class Session extends EventEmitter { // this to omp's own session uuid — that already-resolved id must win // over the generic `this.id` fallback, or this line clobbers it back // to the Codeman id - // on every single respawn. - this._claudeSessionId = this._resumeSessionId || this._ompConfig?.resumeSessionId || this.id; + // on every single respawn. codex needs the same fallback for the same + // reason: its thread id lives in `_codexConfig`, so without it every + // respawn drops a resumed codex session's alias and its Past-Sessions + // row springs back as a duplicate that still resumes. + this._claudeSessionId = + this._resumeSessionId || this._ompConfig?.resumeSessionId || this._codexConfig?.resumeSessionId || this.id; // For NEW mux sessions: wait for readiness then clean buffer // For RESTORED mux sessions: don't do anything - client will fetch buffer on tab switch @@ -2090,10 +2101,11 @@ export class Session extends EventEmitter { // Set claudeSessionId — when resuming, the Claude conversation ID is the resumed one. // Mirrors the mux branch above and must not clobber it: this line runs // unconditionally after both the mux and direct-PTY paths, so it also needs - // the ompConfig fallback or it stomps the mux branch's correctly-resolved - // OMP alias back to this.id on every mux/plain-reattach boot recovery - // (the "third reset point" — see DECISIONS.md). - this._claudeSessionId = this._resumeSessionId || this._ompConfig?.resumeSessionId || this.id; + // the ompConfig and codexConfig fallbacks or it stomps the mux branch's + // correctly-resolved OMP/codex alias back to this.id on every mux/plain- + // reattach boot recovery (the "third reset point" — see DECISIONS.md). + this._claudeSessionId = + this._resumeSessionId || this._ompConfig?.resumeSessionId || this._codexConfig?.resumeSessionId || this.id; this._pid = this.ptyProcess.pid; console.log('[Session] Interactive PTY spawned with PID:', this._pid); diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index aa2cbad0..09c232cf 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -2990,11 +2990,16 @@ Object.assign(CodemanApp.prototype, { // as a duplicate — click it 3 times, see the same name 3 times. Claude // rows are left alone: `sessionId` there is a claudeSessionId, which // usually has no live/persisted Codeman session of its own to delete. - // Gated on `continuesSomething`: for codex/gemini/antigravity (no - // continuation wired above), this is really a FRESH session with no + // Gated on `continuesSomething`: for gemini/antigravity, and for a codex + // row carrying no `resumeId`, this is really a FRESH session with no // relation to the old row's conversation, so retiring it would discard // the old conversation with no recovery — worse than the duplicate row // this guard exists to prevent for the modes that DO continue. + // + // A codex row that DOES continue passes this gate, but the DELETE is a + // no-op for it: `sessionId` there is codex's thread id and no Codeman + // session carries that id. Its duplicate is cleared from the other side + // instead, by the alias fold in gatherUnifiedInputs()/Session. if (effectiveMode !== 'claude' && continuesSomething && sessionId !== newSessionId) { fetch(`/api/sessions/${sessionId}?killMux=true`, { method: 'DELETE' }).catch(() => {}); } diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index d3e37eec..4fd732db 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -145,7 +145,7 @@ import { import { LRUMap } from '../../utils/lru-map.js'; import { findLatestOmpSessionId } from '../../utils/omp-session-resolver.js'; import { scanOmpSessionsHistory } from '../../omp-transcript.js'; -import { scanCodexSessionsHistory } from '../../codex-transcript.js'; +import { scanCodexSessionsHistory, codexThreadBySessionId } from '../../codex-transcript.js'; import { getLastTranscriptResponse, isExternalCliTranscriptMode, @@ -4062,6 +4062,13 @@ export function registerSessionRoutes( // Persisted sessions (state.json). resumeSessionId is the Claude // conversation UUID a resumed session continues — feed it to the merge's // alias map so its transcript row folds into this session. + // + // codex keeps its thread id in `codexConfig` instead, and state.json stores + // that, so read it here as well. Without it a resumed codex session that has + // been demoted to a persisted-only record loses its alias and duplicates: the + // originator fallback below cannot rescue that one, because a RESUMED rollout + // keeps the original session_meta (see findActiveCodexFile) and so still + // names whichever pane first created the thread. const persisted: PersistedSessionInput[] = Object.values(ctx.store.getState().sessions).map((p) => ({ id: p.id, name: p.name, @@ -4070,7 +4077,7 @@ export function registerSessionRoutes( workingDir: p.workingDir, createdAt: p.createdAt, lastActivityAt: p.lastActivityAt, - claudeSessionId: p.resumeSessionId, + claudeSessionId: p.resumeSessionId || p.codexConfig?.resumeSessionId, pinned: p.pinned, pinnedAt: p.pinnedAt, })); @@ -4144,7 +4151,8 @@ export function registerSessionRoutes( // session record does. `resumeId` is the rollout's own thread id, which is // what `codex resume` takes; see codex-transcript.ts. try { - for (const h of await scanCodexSessionsHistory()) { + const codexRows = await scanCodexSessionsHistory(); + for (const h of codexRows) { history.push({ sessionId: h.sessionId, workingDir: h.workingDir, @@ -4156,6 +4164,26 @@ export function registerSessionRoutes( resumeId: h.sessionId, }); } + + // Fold a FRESH codex pane into its own rollout row. A resumed one already + // folds, because Session sets `claudeSessionId` from the resume id it was + // given; a fresh one has no thread id until codex writes the rollout, so + // the link has to come from the other side. Codeman spawns every codex pane + // with CODEX_INTERNAL_ORIGINATOR_OVERRIDE=codeman_, and codex + // stamps that into session_meta.originator, so the rollout names the pane. + // + // Newest rollout wins: `/new` typed inside the TUI leaves several rollouts + // carrying the same originator, and the pane is on the most recent one. + // Rows arrive newest-first, so the first match is it. + // + // Never overwrites an id a session already knows — that one came from the + // resume path and is authoritative. + const codexThreads = codexThreadBySessionId(codexRows); + for (const row of [...live, ...persisted]) { + if (row.claudeSessionId && row.claudeSessionId !== row.id) continue; + const threadId = codexThreads.get(row.id); + if (threadId) row.claudeSessionId = threadId; + } } catch { // Best-effort, same as the two scans above. } diff --git a/test/codex-resume-alias-survives-start.test.ts b/test/codex-resume-alias-survives-start.test.ts new file mode 100644 index 00000000..903e4f06 --- /dev/null +++ b/test/codex-resume-alias-survives-start.test.ts @@ -0,0 +1,82 @@ +/** + * @fileoverview A resumed codex session must keep its thread-id alias across + * `start()`, not just at construction. + * + * `claudeSessionId` doubles as the generic "external transcript id" the unified + * list folds a Past-Sessions row into its owning session by. For codex that id + * is the rollout's thread id, and losing it is not cosmetic: the stale row stays + * in PAST and still resumes, so clicking it starts a SECOND `codex resume` on a + * thread already open in the live pane. + * + * The bug this pins: the alias was wired into the constructor only. `start()` + * recomputes `claudeSessionId` at two further points — the mux branch and the + * unconditional "third reset point" that runs after both the mux and direct-PTY + * paths — and both listed only Claude's `resumeSessionId` and omp's. For codex + * both are undefined, so every mux reattach and every boot recovery reset the + * alias back to the Codeman id and the duplicate came back. The existing comment + * at the third reset point already warned that omitting omp's fallback there + * "stomps the mux branch's correctly-resolved OMP alias"; codex needed the same. + * + * Mirrors `test/omp-fresh-run-no-resume.test.ts`, which drives a real `Session` + * against the in-memory tmux layer that vitest substitutes. + */ +import { mkdirSync, rmSync } from 'node:fs'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; + +import { Session } from '../src/session.js'; +import { TmuxManager } from '../src/tmux-manager.js'; + +describe('codex: a resumed thread id survives start()', () => { + const workingDir = join(homedir(), 'codeman-cases', 'codex-resume-alias'); + const THREAD_ID = '01a060f0-0361-7f91-abde-b283020db0d7'; + const sessions: Session[] = []; + + afterEach(() => { + for (const s of sessions.splice(0)) s.stop(); + rmSync(workingDir, { recursive: true, force: true }); + }); + + function makeSession(useMux: boolean): Session { + mkdirSync(workingDir, { recursive: true }); + const session = new Session({ + workingDir, + mode: 'codex', + codexConfig: { resumeSessionId: THREAD_ID }, + mux: new TmuxManager(), + useMux, + }); + sessions.push(session); + return session; + } + + it('carries the thread id from construction', () => { + expect(makeSession(true).claudeSessionId).toBe(THREAD_ID); + }); + + it('still carries it after starting under mux', async () => { + const session = makeSession(true); + await session.startInteractive(); + expect(session.claudeSessionId).toBe(THREAD_ID); + }); + + it('refuses to start without mux at all, so the mux path is the only one to cover', async () => { + // codex declares `requiresMux`, so there is no direct-PTY codex session for + // the third reset point to run against on its own — the assertion above is + // the whole surface. + await expect(makeSession(false).startInteractive()).rejects.toThrow(/require tmux/i); + }); + + it('a fresh codex session keeps the Codeman id, having no thread of its own', async () => { + mkdirSync(workingDir, { recursive: true }); + const session = new Session({ workingDir, mode: 'codex', mux: new TmuxManager(), useMux: true }); + sessions.push(session); + + await session.startInteractive(); + + // Nothing to alias to yet — codex has not written the rollout. Such a + // session is folded from the other side, by originator (codexThreadBySessionId). + expect(session.claudeSessionId).toBe(session.id); + }); +}); diff --git a/test/codex-transcript.test.ts b/test/codex-transcript.test.ts index ca4584ad..255b8c01 100644 --- a/test/codex-transcript.test.ts +++ b/test/codex-transcript.test.ts @@ -16,25 +16,29 @@ * outnumbered the threads a person can actually resume. */ import { describe, expect, it, beforeEach, afterEach } from 'vitest'; -import { mkdtemp, mkdir, writeFile, rm, utimes } from 'node:fs/promises'; +import { appendFile, mkdtemp, mkdir, writeFile, rm, utimes } from 'node:fs/promises'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; -import { scanCodexSessionsHistory, __clearCodexIdentityCache } from '../src/codex-transcript.js'; +import { + scanCodexSessionsHistory, + codexThreadBySessionId, + __clearCodexIdentityCache, +} from '../src/codex-transcript.js'; let home: string; let prevCodexHome: string | undefined; /** A rollout's opening line, as codex writes it. */ -const sessionMeta = (opts: { id: string; cwd: string; threadSource?: string }) => +const sessionMeta = (opts: { id: string; cwd?: string; threadSource?: string; originator?: string }) => JSON.stringify({ timestamp: '2026-09-02T07:05:37.421Z', type: 'session_meta', payload: { id: opts.id, session_id: opts.id, - cwd: opts.cwd, - originator: 'codex-tui', + ...(opts.cwd ? { cwd: opts.cwd } : {}), + originator: opts.originator ?? 'codex-tui', ...(opts.threadSource ? { thread_source: opts.threadSource } : {}), // The real thing embeds full base instructions here; padded so the file // clears the size floor and exercises the head window. @@ -194,6 +198,76 @@ describe('scanCodexSessionsHistory', () => { expect(rows[0].firstPrompt).toBe('still found me'); }); + it('reports the originator, which is how a fresh pane finds its own rollout', async () => { + await writeRollout('bbbbbbbb-bbbb-7bbb-8bbb-bbbbbbbbbbbb', [ + sessionMeta({ + id: 'bbbbbbbb-bbbb-7bbb-8bbb-bbbbbbbbbbbb', + cwd: '/repo/nine', + originator: 'codeman_2f1c9a44-1111-2222-3333-444455556666', + }), + itemCompletedUser('hello'), + ]); + + const rows = await scanCodexSessionsHistory(); + expect(rows[0].originator).toBe('codeman_2f1c9a44-1111-2222-3333-444455556666'); + }); + + it('drops a rollout that records no working directory', async () => { + // Emitting workingDir: '' would make a click post an empty directory. + await writeRollout('cccccccc-cccc-7ccc-8ccc-cccccccccccc', [ + sessionMeta({ id: 'cccccccc-cccc-7ccc-8ccc-cccccccccccc' }), + itemCompletedUser('nowhere to resume into'), + ]); + + expect(await scanCodexSessionsHistory()).toEqual([]); + }); + + it('picks up a prompt written after an earlier scan saw none', async () => { + // The bug this pins: the identity cache was written as soon as the thread id + // was known, but codex writes the first UserMessage only when the user + // submits. Any scan in that window — the home screen, the command palette, + // the search-index refresh — pinned `firstPrompt: undefined` until restart. + const id = 'dddddddd-dddd-7ddd-8ddd-dddddddddddd'; + const path = await writeRollout(id, [sessionMeta({ id, cwd: '/repo/ten' })]); + + const before = await scanCodexSessionsHistory(); + expect(before).toHaveLength(1); + expect(before[0].firstPrompt).toBeUndefined(); + + await appendFile(path, itemCompletedUser('the prompt, typed a moment later') + '\n', 'utf-8'); + + const after = await scanCodexSessionsHistory(); + expect(after[0].firstPrompt).toBe('the prompt, typed a moment later'); + }); + + it('does not spend the lastPrompt budget on rollouts it never returns', async () => { + // The budget used to count file index, so a store whose newest files are all + // sub-agent threads exhausted it before the first row that needed it. + for (let i = 0; i < 3; i++) { + await writeRollout( + `eeeeeeee-eeee-7eee-8eee-00000000000${i}`, + [ + sessionMeta({ id: `eeeeeeee-eeee-7eee-8eee-00000000000${i}`, cwd: '/repo/sub', threadSource: 'subagent' }), + itemCompletedUser('subagent work'), + ], + new Date('2026-09-05T00:00:00Z') + ); + } + await writeRollout( + 'ffffffff-ffff-7fff-8fff-ffffffffffff', + [ + sessionMeta({ id: 'ffffffff-ffff-7fff-8fff-ffffffffffff', cwd: '/repo/real' }), + itemCompletedUser('opening'), + itemCompletedUser('the latest thing asked'), + ], + new Date('2026-09-04T00:00:00Z') + ); + + const rows = await scanCodexSessionsHistory(); + expect(rows).toHaveLength(1); + expect(rows[0].lastPrompt).toBe('the latest thing asked'); + }); + it('collapses a long prompt to a single capped line', async () => { await writeRollout('aaaaaaaa-aaaa-7aaa-8aaa-aaaaaaaaaaaa', [ sessionMeta({ id: 'aaaaaaaa-aaaa-7aaa-8aaa-aaaaaaaaaaaa', cwd: '/repo/eight' }), @@ -206,3 +280,25 @@ describe('scanCodexSessionsHistory', () => { expect(rows[0].firstPrompt!.endsWith('…')).toBe(true); }); }); + +describe('codexThreadBySessionId', () => { + const row = (sessionId: string, originator?: string) => + ({ sessionId, originator, workingDir: '/w', sizeBytes: 1, lastModified: '2026-09-02T00:00:00.000Z' }) as never; + + it('maps a Codeman-spawned pane to the thread it is writing', () => { + const map = codexThreadBySessionId([row('thread-a', 'codeman_sess-1')]); + expect(map.get('sess-1')).toBe('thread-a'); + }); + + it('ignores a rollout codex started on its own', () => { + expect(codexThreadBySessionId([row('thread-a', 'codex-tui')]).size).toBe(0); + expect(codexThreadBySessionId([row('thread-a', undefined)]).size).toBe(0); + }); + + it('keeps the newest rollout when a pane has several', () => { + // `/new` inside the codex TUI leaves the pane's originator on more than one + // rollout; the pane is on the most recent, and rows arrive newest-first. + const map = codexThreadBySessionId([row('thread-new', 'codeman_sess-1'), row('thread-old', 'codeman_sess-1')]); + expect(map.get('sess-1')).toBe('thread-new'); + }); +}); diff --git a/test/resume-history-mode-fidelity.test.ts b/test/resume-history-mode-fidelity.test.ts index 85619ae4..3f71d27e 100644 --- a/test/resume-history-mode-fidelity.test.ts +++ b/test/resume-history-mode-fidelity.test.ts @@ -90,7 +90,7 @@ describe('resumeHistorySession: row retirement is gated on actual continuation', fetchMock = stubFetch('new-session-id'); }); - it.each(['codex', 'gemini', 'antigravity'])( + it.each(['gemini', 'antigravity'])( 'does NOT retire the old row for %s (no continuation is wired for it)', async (mode) => { const app = makeApp(); @@ -104,6 +104,44 @@ describe('resumeHistorySession: row retirement is gated on actual continuation', } ); + // codex continues only when the row carried its thread id. A row without one + // is a live session's row, whose sessionId is Codeman's own uuid — sending + // THAT to `codex resume` asks for a thread that does not exist, so it must + // stay a fresh session and must not retire the row it came from. + it('does NOT continue or retire a codex row that carries no resumeId', async () => { + const app = makeApp(); + await app.resumeHistorySession.call(app, 'codeman-uuid', '/repo', 'w1-repo', 'codex'); + + expect(createBody(fetchMock)).toMatchObject({ mode: 'codex' }); + expect(createBody(fetchMock).codexConfig).toBeUndefined(); + expect(deleteCalls(fetchMock)).toEqual([]); + }); + + it('resumes a codex row by the thread id the row carried', async () => { + const app = makeApp(); + await app.resumeHistorySession.call( + app, + '01a060f0-0361-7f91-abde-b283020db0d7', + '/repo', + 'w1-repo', + 'codex', + '01a060f0-0361-7f91-abde-b283020db0d7' + ); + + expect(createBody(fetchMock)).toMatchObject({ + mode: 'codex', + codexConfig: { resumeSessionId: '01a060f0-0361-7f91-abde-b283020db0d7' }, + }); + }); + + it('ignores a resumeId on a row that is not codex', async () => { + const app = makeApp(); + await app.resumeHistorySession.call(app, 'old-id', '/repo', 'w1-repo', 'gemini', 'some-thread-id'); + + expect(createBody(fetchMock).codexConfig).toBeUndefined(); + expect(deleteCalls(fetchMock)).toEqual([]); + }); + it.each([ ['opencode', 'openCodeConfig'], ['pi', 'piConfig'], diff --git a/test/services/unified-session-service.test.ts b/test/services/unified-session-service.test.ts index a9f97573..82e67097 100644 --- a/test/services/unified-session-service.test.ts +++ b/test/services/unified-session-service.test.ts @@ -14,6 +14,70 @@ import { } from '../../src/services/unified-session-service.js'; describe('mergeUnifiedSessions', () => { + // A codex conversation showing twice is worse than cosmetic: the stale PAST row + // still resumes, so clicking it starts a SECOND `codex resume` on a thread + // already open in another pane. Both folds below are what prevent that. + it('folds a RESUMED codex session into its own rollout row', () => { + // Session sets claudeSessionId from codexConfig.resumeSessionId, so the live + // row already names the thread the rollout is keyed by. + const merged = mergeUnifiedSessions({ + live: [{ id: 'codeman-uuid', status: 'idle', mode: 'codex', claudeSessionId: 'codex-thread-id' }], + history: [ + { + sessionId: 'codex-thread-id', + workingDir: '/w', + sizeBytes: 4000, + lastModified: '2026-09-02T00:00:00.000Z', + mode: 'codex', + resumeId: 'codex-thread-id', + }, + ], + }); + expect(merged).toHaveLength(1); + expect(merged[0].sessionId).toBe('codeman-uuid'); + expect([...merged[0].sources].sort()).toEqual(['history', 'live']); + }); + + it('folds a FRESH codex session once its rollout has been matched by originator', () => { + // A fresh pane knows no thread id, so gatherUnifiedInputs() stamps one onto + // the live row from session_meta.originator (see codexThreadBySessionId). + // This is that stamped row. + const merged = mergeUnifiedSessions({ + live: [{ id: 'codeman-uuid', status: 'busy', mode: 'codex', claudeSessionId: 'fresh-thread-id' }], + persisted: [{ id: 'codeman-uuid', status: 'idle', mode: 'codex', claudeSessionId: 'fresh-thread-id' }], + history: [ + { + sessionId: 'fresh-thread-id', + workingDir: '/w', + sizeBytes: 900, + lastModified: '2026-09-02T00:00:00.000Z', + mode: 'codex', + resumeId: 'fresh-thread-id', + }, + ], + }); + expect(merged).toHaveLength(1); + expect(merged[0].sessionId).toBe('codeman-uuid'); + expect(merged[0].status).toBe('busy'); + }); + + it('leaves an unrelated codex rollout as its own row', () => { + const merged = mergeUnifiedSessions({ + live: [{ id: 'codeman-uuid', status: 'idle', mode: 'codex', claudeSessionId: 'thread-one' }], + history: [ + { + sessionId: 'thread-two', + workingDir: '/w', + sizeBytes: 4000, + lastModified: '2026-09-02T00:00:00.000Z', + mode: 'codex', + resumeId: 'thread-two', + }, + ], + }); + expect(merged.map((m) => m.sessionId).sort()).toEqual(['codeman-uuid', 'thread-two']); + }); + it("carries a transcript row's own resume token, and stamps none on a live row", () => { // codex names a thread by an id in its rollout, not by Codeman's session id. // The scanner sets `resumeId`; a live session never does, which is what stops