From 47e793527482ea6d89e80aa825652fb8361cf355 Mon Sep 17 00:00:00 2001 From: Michael Grundberg Date: Mon, 21 Sep 2026 13:36:27 +0200 Subject: [PATCH 1/2] fix(session): resume the conversation when respawning a dead pane A CLI that launches with `--session-id ` refuses an id that is already in use (claude: `Error: Session ID ... is already in use.`), and every session whose agent has been prompted owns a transcript under that id. The dead-pane respawn in `_setupOrAttachMuxSession()` passed the bare launch line, so recovering such a session relaunched a CLI that died on startup, the pane went dead again at once, and the conversation was stranded behind a tab that looked merely idle. `restartCli()` has pinned a resume id against this since the custom-model work, and its comment states the assumption that made the other path look safe: "Unlike the dead-pane respawn, this one kills a WORKING pane whose conversation already has a transcript". A pane whose agent exited has a transcript too. Both relaunch paths now build options through `_buildRespawnPaneOptionsWithResumePin()`, and so does the create-path fallback after a failed respawn, which otherwise met the same refusal that made it the fallback. Four gates guard the pin, each standing for a way of resuming the WRONG conversation or of making a working relaunch fail. A remote or docker session is never pinned. Unlike `restartCli()`, whose route refuses both, the dead-pane respawn is reached by every session shape. Their pane commands already render a self-healing `--session-id || --resume`, and both flip to resume-first once the resume id differs; the conversation lives on the far side, so a local id resolves to nothing there and the `--session-id` fallback then collides with the transcript the far side does hold. The id comes from the conversation CHAIN rather than `_claudeSessionId`, which also holds history-correlated guesses keyed on the working directory. `_recordClaudeSessionInChain()` refuses those so they cannot "write a foreign conversation into this pane's permanent record", and launching from one is worse than the display bug that rule prevents. The chain tail also outranks the launch seed, which is written once at construction and never moves off a `/clear`. A pin no transcript backs is dropped, because the fallback branch keeps `--session-id ` and would collide. A synthetic `restored-` id from socket discovery is dropped too, and logged: it fails claude's `uuid` token pattern, so the renderer would emit the unpinned command while the caller believed otherwise. Tests cover each gate and the rendered command. Four of them fail against the unfixed source; the remote and docker ones were separately checked against a build with only that guard removed, since they pass on master for the wrong reason. Co-Authored-By: Claude Opus 5 (1M context) --- src/session.ts | 119 +++++++-- src/utils/claude-transcript.ts | 57 +++++ test/respawn-session-id-collision.test.ts | 282 ++++++++++++++++++++++ 3 files changed, 441 insertions(+), 17 deletions(-) create mode 100644 src/utils/claude-transcript.ts create mode 100644 test/respawn-session-id-collision.test.ts diff --git a/src/session.ts b/src/session.ts index fdad22f8..b3226332 100644 --- a/src/session.ts +++ b/src/session.ts @@ -62,6 +62,8 @@ import { type SessionWriteOptions, } from './types.js'; import { resolveAndClaimOmpSessionId } from './utils/omp-session-resolver.js'; +import { claudeTranscriptExists } from './utils/claude-transcript.js'; +import { matchesPattern } from './config/cli-registry/patterns.js'; import { probeDockerCliVersion } from './docker-hosts.js'; import { probeRemoteCliVersion } from './remote-hosts.js'; import type { TerminalMultiplexer, MuxSession } from './mux-interface.js'; @@ -1749,7 +1751,7 @@ export class Session extends EventEmitter { // `options.respawnPaneOptions` was built eagerly before this dead-pane // check ran, so it still carries the pre-pin ompConfig; rebuild it. this._pinOmpRespawnId(); - const newPid = await mux.respawnPane(this._buildRespawnPaneOptions()); + const newPid = await mux.respawnPane(await this._buildRespawnPaneOptionsWithResumePin()); if (!newPid) { console.error('[Session] Failed to respawn pane, will create new session'); needsNewSession = true; @@ -1765,7 +1767,17 @@ export class Session extends EventEmitter { if (isRestored) { console.log('[Session] Attaching to existing mux session:', this._muxSession!.muxName); } else { - // Create a new mux session + // Create a new mux session. When this is the FALLBACK after a failed + // respawn, the eagerly-built create options still carry the unpinned + // launch seed, so a session whose transcript exists would meet the same + // `--session-id ... already in use` refusal the respawn just lost to — + // the recovery of last resort failing for the very reason it was needed. + // A genuinely new session has no chain and no transcript, so the pin + // resolves to its own id and the command shape is unchanged. + if (needsNewSession) { + const pinned = (await this._buildRespawnPaneOptionsWithResumePin()).resumeSessionId; + if (pinned) options.createSessionOptions.resumeSessionId = pinned; + } this._muxSession = await mux.createSession(options.createSessionOptions); console.log('[Session] Created mux session:', this._muxSession.muxName); // No extra sleep — createSession() already waits for tmux readiness @@ -1878,21 +1890,7 @@ export class Session extends EventEmitter { } this._pinOmpRespawnId(); - const options = this._buildRespawnPaneOptions(); - // Unlike the dead-pane respawn, this one kills a WORKING pane whose conversation - // already has a transcript, and a CLI that launches with `--session-id ` refuses - // an id that is already in use (claude: `Error: Session ID ... is already in use.`), - // which turned an endpoint switch into a dead pane and a lost session. A launch that - // declares a `fallback` chain renders `resume || new` once a resume id is set, the - // same `--resume || --session-id ` shape the docker and remote pane commands - // already use, so pin the live conversation id for THIS respawn only. The registry - // shape is the gate, not the CLI's name: an entry whose resume id is minted by the - // CLI itself (codex/pi/omp/grok) never declares that chain, and its resume field is - // read from its own `Config` rather than this top-level one anyway. - if (!options.resumeSessionId && getCli(this.mode)?.launch.chain === 'fallback') { - options.resumeSessionId = this._claudeSessionId ?? this.id; - } - const newPid = await mux.respawnPane(options); + const newPid = await mux.respawnPane(await this._buildRespawnPaneOptionsWithResumePin()); if (!newPid) { console.error('[Session] restartCli: respawnPane failed for', this._muxSession.muxName); return false; @@ -1947,6 +1945,93 @@ export class Session extends EventEmitter { return this._withCustomModelLaunchModel(options); } + /** + * Respawn options for a pane whose command is being REPLACED, with the + * conversation pinned so the relaunch resumes rather than collides. + * + * A CLI that launches with `--session-id ` refuses an id that is already + * in use (claude: `Error: Session ID ... is already in use.`), and every + * session whose agent has been prompted owns a transcript under that id. So + * relaunching such a pane with the bare launch line fails, the pane dies + * again immediately, and the user's conversation is stranded. A launch that + * declares a `fallback` chain renders `resume || new` once a resume id is + * set, which is the shape that survives both cases. + * + * Four conditions gate the pin, each protecting against a way of resuming the + * WRONG conversation or of making a working relaunch fail. + * + * ⚠️ **A remote or docker session is never pinned.** Unlike `restartCli()`, + * whose route refuses both, the dead-pane respawn is reached by every session + * shape. Their pane commands (`buildRemoteLaunchCommand`, + * `claudeDockerPaneCommand`) already render a SELF-HEALING + * `--session-id || --resume `, and both flip to resume-first the + * moment the resume id differs from the session id. The conversation lives on + * the far side, so a local id pinned onto it resolves to nothing there, the + * resume fails, and the `--session-id` fallback then collides with the + * transcript the far side really does hold — both branches fail and the pane + * dies. `_pinOmpRespawnId()` refuses remote for the same reason. + * + * ⚠️ **The id comes from the conversation CHAIN, not from + * `_claudeSessionId`.** That field holds either a first-hand id from the + * CLI's own hook payload or a history correlation, which is a guess keyed on + * the working directory. `_recordClaudeSessionInChain()` refuses a guess + * precisely so it cannot "write a foreign conversation into this pane's + * permanent record", and launching from one would do worse than the display + * bug that rule exists to prevent: the relaunched CLI would open and WRITE to + * a conversation that was never this pane's. The chain's tail is the live + * conversation and is hook-vouched, so it also outranks the launch seed, + * which is written once at construction and never moves off a `/clear`. + * + * ⚠️ **A pin that no transcript backs is dropped.** When the pinned id + * differs from the session id, the fallback branch still carries + * `--session-id `; if the resume finds nothing, that fallback + * collides and the pane dies exactly as it did before this pinning existed. + * The create route pre-validates a resume id for the same reason, though it + * additionally requires the transcript be substantial — here mere existence + * is the question, because a one-line transcript still makes `--session-id` + * collide. + * + * The registry shape is the last gate, not the CLI's name: an entry whose + * resume id is minted by the CLI itself (codex/pi/omp/grok) declares no + * `fallback` chain and reads its resume field from its own `Config`. + * + * `reattachRemote()` deliberately does NOT call this. It re-runs the remote + * session command, which attaches to the durable remote tmux with the agent + * still inside it and renders no local `--session-id` to collide. + */ + private async _buildRespawnPaneOptionsWithResumePin(): Promise { + const options = this._buildRespawnPaneOptions(); + if (this._remote || this._docker) return options; + if (getCli(this.mode)?.launch.chain !== 'fallback') return options; + + const chainTail = this._claudeSessionChain[this._claudeSessionChain.length - 1]; + const pin = chainTail ?? options.resumeSessionId ?? this.id; + // A session Codeman DISCOVERED on the socket rather than created carries a + // synthetic `restored-` id, which fails claude's `uuid` token + // pattern. The renderer would silently drop the resume flag and emit the + // unpinned command, so say so here rather than letting the caller believe + // the pane was pinned. Such a pane keeps the pre-existing behaviour. + const resumeIdPattern = getCli(this.mode)?.launch.params?.resumeId; + if (resumeIdPattern?.type === 'token' && !matchesPattern(resumeIdPattern.pattern, pin)) { + console.log(`[Session] Not pinning resume id ${pin} for relaunch: the CLI cannot accept that id shape`); + return options; + } + // Pinning the session's own id renders the self-healing + // `--session-id || --resume `, which needs no transcript to be + // correct: it starts fresh when there is none and resumes when there is. + if (pin !== this.id && !(await claudeTranscriptExists(pin, this._claudeConfigDir()))) { + console.log(`[Session] Not pinning resume id ${pin} for relaunch: no transcript on disk`); + return options; + } + options.resumeSessionId = pin; + return options; + } + + /** The session's Claude config dir when it has been relocated (#255), else undefined. */ + private _claudeConfigDir(): string | undefined { + return this._envOverrides?.CLAUDE_CONFIG_DIR; + } + /** * Force the custom-model selection's `launchModel` (pi/omp `custom/`, grok's * `[model.]` block name) onto the CLI's `model` launch param. Where that param diff --git a/src/utils/claude-transcript.ts b/src/utils/claude-transcript.ts new file mode 100644 index 00000000..6c6c4b49 --- /dev/null +++ b/src/utils/claude-transcript.ts @@ -0,0 +1,57 @@ +/** + * @fileoverview Does a Claude conversation transcript exist on this host? + * + * Claude writes one `.jsonl` per conversation under + * `/projects//`. Two launch decisions turn on whether + * such a file exists: `--resume ` needs one, and `--session-id ` is + * REFUSED when one exists (`Error: Session ID ... is already in use.`). + * + * The project directory name is derived from the working directory, and a case + * that has been moved or renamed leaves its transcript under the OLD name, so + * the search is across every project directory rather than the one that matches + * the pane's cwd today. + * + * ⚠️ Existence is the whole question here, with no size floor. The create route + * additionally requires ~4 KB before it will resume, which is a "is this + * conversation worth resuming" judgement; for a relaunch the question is the + * opposite one — a one-line transcript still makes `--session-id` collide. + * + * @dependencies none + * @consumedby session (relaunch resume pinning) + * + * @module utils/claude-transcript + */ + +import { readdir, stat } from 'node:fs/promises'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; + +/** `/projects`, honouring a session's relocated `CLAUDE_CONFIG_DIR`. */ +export function claudeProjectsDir(configDir?: string): string { + return join(configDir || join(homedir(), '.claude'), 'projects'); +} + +/** + * True when a transcript for `conversationId` exists under any project + * directory. Returns false for a missing projects dir or an unreadable one: + * the caller's fallback is to skip the resume, which is the safe direction. + */ +export async function claudeTranscriptExists(conversationId: string, configDir?: string): Promise { + if (!conversationId) return false; + const projectsDir = claudeProjectsDir(configDir); + let projectDirs: string[]; + try { + projectDirs = await readdir(projectsDir); + } catch { + return false; + } + for (const projectDir of projectDirs) { + try { + await stat(join(projectsDir, projectDir, `${conversationId}.jsonl`)); + return true; + } catch { + // Not in this project directory; keep looking. + } + } + return false; +} diff --git a/test/respawn-session-id-collision.test.ts b/test/respawn-session-id-collision.test.ts new file mode 100644 index 00000000..34b3f24a --- /dev/null +++ b/test/respawn-session-id-collision.test.ts @@ -0,0 +1,282 @@ +/** + * @fileoverview Relaunching a pane must resume its conversation, not collide + * with it, and must not resume somebody else's. + * + * A CLI that launches with `--session-id ` refuses an id that is already in + * use (claude: `Error: Session ID ... is already in use.`), and every session + * whose agent has been prompted owns a transcript under that id. A relaunch + * that passes the bare launch line therefore dies on startup, the pane goes + * dead again at once, and the conversation is stranded. + * + * `restartCli()` has pinned a resume id for this reason since the custom-model + * work. The dead-pane respawn in `_setupOrAttachMuxSession()` did not, and its + * comment said so explicitly — "Unlike the dead-pane respawn, this one kills a + * WORKING pane whose conversation already has a transcript". That assumption is + * what these tests refute: a pane whose agent exited has a transcript too. + * + * The pin is gated four ways, and most of these tests are about the gates + * rather than the pin, because each gate stands for a way of resuming the WRONG + * conversation or of making a working relaunch fail. + * + * Port: N/A + */ +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { Session } from '../src/session.js'; +import { getCli } from '../src/config/cli-registry/registry.js'; +import { buildSpawnCommandFromRegistry } from '../src/session-cli-registry-bridge.js'; +import type { MuxSession, RespawnPaneOptions, TerminalMultiplexer } from '../src/mux-interface.js'; + +/** Captures the options each respawn is invoked with. */ +function recordingMux() { + const calls: RespawnPaneOptions[] = []; + const mux = { + isAvailable: () => true, + muxSessionExists: () => true, + isPaneDead: () => true, + // Called by the PTY-exit handler during teardown. Absent, it throws + // asynchronously after the test body has already passed, which vitest + // reports as an unhandled error rather than a failure. + setAttached: () => {}, + respawnPane: async (options: RespawnPaneOptions) => { + calls.push(options); + return 4242; + }, + }; + return { mux: mux as unknown as TerminalMultiplexer, calls }; +} + +const muxSession = (muxName = 'codeman-aaaa') => ({ muxName, sessionId: 'aaaa' }) as unknown as MuxSession; + +const CONVERSATION = 'aaaabbbb-cccc-dddd-eeee-ffff00001111'; + +let configDir: string; + +/** A relocated Claude config dir, so the transcript gate reads a real fixture. */ +beforeEach(() => { + configDir = mkdtempSync(join(tmpdir(), 'codeman-transcript-')); +}); + +afterEach(() => { + rmSync(configDir, { recursive: true, force: true }); +}); + +/** Write the `.jsonl` Claude would have written for a conversation. */ +function giveTranscript(conversationId: string): void { + const projectDir = join(configDir, 'projects', '-tmp-case'); + mkdirSync(projectDir, { recursive: true }); + writeFileSync(join(projectDir, `${conversationId}.jsonl`), '{"type":"user"}\n'); +} + +function localSession(extra: Record = {}, mux?: TerminalMultiplexer) { + return new Session({ + workingDir: '/tmp', + mode: 'claude', + useMux: true, + mux, + muxSession: muxSession(), + envOverrides: { CLAUDE_CONFIG_DIR: configDir }, + ...extra, + }); +} + +describe('pinning a conversation onto a relaunch', () => { + it('pins the chain tail on the DEAD-PANE respawn, which is the bug', async () => { + // The path a recovered `/exit`ed session takes, and the one that was + // missing the pin. Driven through `startInteractive()` rather than asserted + // from source: a comment claiming a thing happens is exactly what was wrong + // here before. + giveTranscript(CONVERSATION); + const { mux, calls } = recordingMux(); + const session = localSession({ claudeSessionChain: [CONVERSATION] }, mux); + + await session.startInteractive(); + try { + expect(calls).toHaveLength(1); + expect(calls[0].resumeSessionId).toBe(CONVERSATION); + } finally { + await session.stop(); + } + }); + + it('pins the chain tail on a custom-model restart too', async () => { + giveTranscript(CONVERSATION); + const { mux, calls } = recordingMux(); + const session = localSession({ claudeSessionChain: [CONVERSATION] }, mux); + + expect(await session.restartCli()).toBe(true); + + expect(calls[0].resumeSessionId).toBe(CONVERSATION); + }); + + it('prefers the live chain tail over the launch seed', async () => { + // `_resumeSessionId` is written once at construction and never moves, so a + // `/clear` after launch leaves it pointing at the predecessor. Resuming + // that would reopen an abandoned conversation and strand the live one. + giveTranscript(CONVERSATION); + const { mux, calls } = recordingMux(); + const session = localSession( + { resumeSessionId: 'bbbbcccc-dddd-eeee-ffff-000011112222', claudeSessionChain: [CONVERSATION] }, + mux + ); + + expect(await session.restartCli()).toBe(true); + + expect(calls[0].resumeSessionId).toBe(CONVERSATION); + }); + + it('falls back to the launch seed when no conversation was ever recorded', async () => { + const seed = 'bbbbcccc-dddd-eeee-ffff-000011112222'; + giveTranscript(seed); + const { mux, calls } = recordingMux(); + const session = localSession({ resumeSessionId: seed }, mux); + + expect(await session.restartCli()).toBe(true); + + expect(calls[0].resumeSessionId).toBe(seed); + }); + + it('never resumes an id the conversation chain did not vouch for', async () => { + // `_claudeSessionId` also holds history-CORRELATED guesses, keyed on the + // working directory, which the chain deliberately refuses. Launching from + // one would open and WRITE to a conversation that was never this pane's — + // worse than the display bug that rule exists to prevent. + giveTranscript(CONVERSATION); + const { mux, calls } = recordingMux(); + const session = localSession({}, mux); + session.adoptClaudeSessionId(CONVERSATION); // no firstHand flag: a guess + expect(session.claudeSessionId).toBe(CONVERSATION); + + expect(await session.restartCli()).toBe(true); + + expect(calls[0].resumeSessionId).not.toBe(CONVERSATION); + expect(calls[0].resumeSessionId).toBe(session.id); + }); + + it('drops a pin that no transcript backs', async () => { + // A divergent pin renders `--resume || --session-id `, so a + // resume that finds nothing falls back onto the colliding form and the pane + // dies exactly as it did before any of this. No transcript, no pin. + const { mux, calls } = recordingMux(); + const session = localSession({ claudeSessionChain: [CONVERSATION] }, mux); + + expect(await session.restartCli()).toBe(true); + + expect(calls[0].resumeSessionId).toBeUndefined(); + }); + + it('pins nothing for a remote session, whose conversation lives elsewhere', async () => { + // The dead-pane respawn is reached by every session shape, unlike + // `restartCli()` whose route refuses remote. A local id pinned onto a + // remote pane resolves to nothing there, and the `--session-id` fallback + // then collides with the transcript the remote host really does hold. + giveTranscript(CONVERSATION); + const { mux, calls } = recordingMux(); + const session = localSession( + { + remote: { hostId: 'h1', label: 'box', host: 'box', username: 'dev', remotePath: '/tmp' }, + claudeSessionChain: [CONVERSATION], + }, + mux + ); + + await session.startInteractive(); + try { + expect(calls[0].resumeSessionId).toBeUndefined(); + } finally { + await session.stop(); + } + }); + + it('pins nothing for a docker case, whose pane execs into the container', async () => { + giveTranscript(CONVERSATION); + const { mux, calls } = recordingMux(); + const session = localSession( + { + docker: { hostId: 'd1', label: 'ctr', containerName: 'ctr' }, + claudeSessionChain: [CONVERSATION], + }, + mux + ); + + await session.startInteractive(); + try { + expect(calls[0].resumeSessionId).toBeUndefined(); + } finally { + await session.stop(); + } + }); + + it('pins nothing for a CLI that mints its own resume id', async () => { + // codex/pi/omp/grok declare no `fallback` chain and read their resume id + // from their own config, so a top-level pin would be meaningless at best. + const { mux, calls } = recordingMux(); + const session = localSession({ mode: 'codex' }, mux); + + expect(await session.restartCli()).toBe(true); + + expect(calls[0].resumeSessionId).toBeUndefined(); + }); + + it('pins nothing for a remote reattach, which relaunches no CLI', async () => { + // `reattachRemote()` re-runs the remote session command, which attaches to + // the durable remote tmux with the agent still running inside it. + giveTranscript(CONVERSATION); + const { mux, calls } = recordingMux(); + const session = localSession( + { + remote: { hostId: 'h1', label: 'box', host: 'box', username: 'dev', remotePath: '/tmp' }, + claudeSessionChain: [CONVERSATION], + }, + mux + ); + + expect(await session.reattachRemote()).toBe(true); + + expect(calls[0].resumeSessionId).toBeUndefined(); + }); +}); + +describe('what the pin renders', () => { + // The rendered command is what actually runs, and it is where each gate's + // reason shows. Asserting here rather than counting call sites in the source + // is what would have caught the divergent-pin and synthetic-id cases. + const SID = '0f9c2b14-1111-2222-3333-444455556666'; + const entry = getCli('claude'); + const render = (resumeSessionId?: string) => { + if (!entry) throw new Error('no registry entry for claude'); + return buildSpawnCommandFromRegistry(entry, { + mode: 'claude', + sessionId: SID, + claudeCliVersion: null, + resumeSessionId, + }); + }; + + it('renders the colliding bare form with no pin — the bug itself', () => { + expect(render()).toContain(`--session-id "${SID}"`); + expect(render()).not.toContain('--resume'); + }); + + it('renders a self-healing resume-or-new when the pin is the session id', () => { + expect(render(SID)).toBe( + `claude --dangerously-skip-permissions --resume "${SID}" || claude --dangerously-skip-permissions --session-id "${SID}"` + ); + }); + + it('keeps the SESSION id in the fallback branch when the pin diverges', () => { + // Which is why a pin with no transcript behind it has to be dropped: the + // fallback is the colliding form, so a failed resume dies twice. + expect(render(CONVERSATION)).toContain(`--resume "${CONVERSATION}"`); + expect(render(CONVERSATION)).toContain(`--session-id "${SID}"`); + }); + + it('drops a synthetic discovered id, which fails the uuid token pattern', () => { + // `reconcileSessions()` mints `restored-` for a tmux session + // Codeman found but does not own. The renderer emits the unpinned command, + // so those panes keep the pre-existing behaviour. + expect(render('restored-40568a29')).not.toContain('--resume'); + }); +}); From 1cb0441bd8b88ad98d0b6d9dab2942a09fafbd72 Mon Sep 17 00:00:00 2001 From: Michael Grundberg Date: Mon, 21 Sep 2026 18:21:21 +0200 Subject: [PATCH 2/2] fix(session): degrade the resume pin to the session id, not to nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A single pin that failed its transcript gate returned the options untouched, so `resumeSessionId` fell back to `_resumeSessionId` — undefined for an ordinary session — and the renderer emitted the bare `claude --dangerously-skip-permissions --session-id ""`. Every session prompted before its first `/clear` owns a transcript under that id, so the dropped pin handed back exactly the refusal this branch removes, with no `||` branch to catch it. It was also a regression against master on the `restartCli()` path, which pinned `_claudeSessionId ?? this.id` and, since the constructor seeds that field, could never land unpinned. The pin now walks three candidates in priority order — the conversation chain's tail, the launch seed, then the session's own id — and takes the first one a transcript backs. A candidate that misses is passed over rather than ending the walk. Falling off the end pins nothing, which also settles the second half of the problem: the old code skipped the transcript check whenever the pin was the session's own id, so a genuinely new pane rendered the two-branch form after all. That costs a brand-new session claude's "No conversation found" line in its scrollback, and `wrapWithNice()` prefixes only the first branch of the rendered `a || b`, so the branch that actually runs loses its priority for the life of the session. With no transcript anywhere the bare `--session-id` is the correct command, so the comment claiming an unchanged shape is now true. The transcript lookup reads the server process's own `CLAUDE_CONFIG_DIR` when a session declares none. A pane inherits the server environment through tmux, so on an install that exports it the CLI writes its transcripts there and every lookup under `~/.claude` was a false negative — which under the old code meant the colliding command. `claudeCredentialsPath()` and `realClaudeConfigDir()` resolve the same directory the same way. The header sentence calling a skipped resume "the safe direction" described the opposite of what happens at this call site, and says so now. The create-path fallback writes `_resumeSessionId` alongside the create options. That branch leaves `isRestored` false, so `_claudeSessionId` is recomputed from the launch fields and settled on `this.id` while the CLI resumed the chain tail; the response viewer, Read My Mind and the unified-list alias map read that field until the next first-hand hook. Four new tests: a chain tail with no transcript while the session id has one, no transcript anywhere, the create path's alias, and the process-env lookup. All four fail against the previous commit. Two existing tests move with the gate — the guess-refusal test now backs the session's own id, and the custom-model restart test gives its working pane the transcript that makes `--session-id` collide in the first place, alongside a new one pinning the no-transcript case. CLAUDE.md described the pin as a `restartCli()`-only thing sourced from the live conversation id. All three halves of that moved here. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 2 +- src/session.ts | 86 ++++++++----- src/utils/claude-transcript.ts | 26 +++- test/respawn-session-id-collision.test.ts | 139 ++++++++++++++++++++-- test/session-custom-model-restart.test.ts | 48 +++++++- 5 files changed, 256 insertions(+), 45 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6dd34e47..fad5aeaf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -229,7 +229,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **DeepSeek web UI** (`POST`/`GET`/`DELETE /api/deepseek/web`, `deepseek-web-server.ts`): the Run menu's "DeepSeek web UI..." entry supervises ONE background `dsh web` child process, deliberately **NOT a shell session**. The session version worked and was still wrong in use: it put a terminal tab on screen next to the web tab the user actually asked for, every single time, and nothing about a long-lived HTTP server needs to be a tab. ⚠️ What a session gave for free now has to be paid for explicitly, and every piece is load-bearing: **exactly one** server (a second click REUSES it rather than racing it for a port, which two sessions structurally could not do), **restarted when the browser authority changes** (`--trusted-host` fences dsh's `/api` against the browser authority, and a Codeman reachable at both loopback and a tailnet name has two, so whoever asks last wins: the asker is by definition the origin about to load the page), **killed on shutdown** (`stopDeepSeekWeb()` in the server teardown, because the child is detached so its whole plugin tree can be signalled at once, which also means it would OUTLIVE Codeman and hold its port against the next start), and **failures returned to the caller**, since with no tab there is nowhere for a stack trace to land. ⚠️ The port search starts at dsh's own default 3080 and walks 40, never fixed: that default is precisely the port most likely to be taken already by the user's own `dsh web`, and hardcoding it killed this feature with EADDRINUSE once. Free-port detection BINDS rather than connects (a connect probe cannot tell "free" from "listening but not answering yet"), so it is racy by nature and the caller still waits for the server to really answer before reporting success. ⚠️ Both `POST` and `DELETE` sit at the **same privilege bar as the profile installer** (`canUsernameRunPrivilegedCommands`) even though the action reads as "open a page": booting a dsh profile executes the plugin code in it, and the server is a single shared instance, so stopping it in multi-user mode takes it out from under other users' tabs. -**Custom Model Endpoint Profiles** (opt-in, `customModelEndpointsEnabled`, SYNCED, default OFF; `docs/custom-model-endpoints.md`, design doc `docs/custom-model-endpoints-plan.md`; full stack — settings-panel CRUD + the Run-menu picker, on top of the backend below): points a session at a user-configured custom OpenAI-compatible endpoint — local (llama.cpp, DGX Spark, Strix Halo) or cloud (Azure AI Foundry, OpenRouter) — instead of its harness's native cloud backend. Endpoints are a read/write-array store (`custom-model-hosts.ts`, `~/.codeman/custom-model-hosts.json`) discovered via `GET /v1/models`; `CustomModelHost.authStyle` is `'bearer'` (default, `Authorization: Bearer`) or `'api-key'` (Azure's convention) — **never both**, live-tested against a real server: sending both headers on one request reliably hangs it indefinitely, reproduced 3×. ⚠️ The actual per-CLI redirect is `capabilities.customModelInjection` on the CLI registry (four kinds: `env` for claude/gemini/deepseek, `configContentEnv` reusing opencode's existing `OPENCODE_CONFIG_CONTENT`, `configDir` for codex/pi/grok/omp — writes an isolated per-session config file, NEVER the user's real `~/.codex`/`~/.pi`/`~/.omp`/grok config — and `unsupported` for antigravity, which has no known mechanism), computed by the pure `custom-model-injection.ts` (mirrors `session-cli-builder.ts`'s no-IO discipline). ⚠️ `PI_CONFIG_DIR` does NOTHING for pi or omp (grepped pi's entire bundled JS source — the string appears nowhere); both hardcode `~/.pi/agent/models.json` / `~/.omp/agent/models.yml` with no dedicated override, so the real redirect for both is the child process's own **`HOME`**, and both need `models` as an ARRAY of `{id}` objects (an object keyed by id silently loads zero models). Grok's real mechanism turned out to be a `config.toml` `[model.]` block redirected via `GROK_HOME` — its original env-var-based recipe was flat-out wrong (produced "Not signed in" against a real binary), not just unverified. ⚠️ **Two launch paths, chosen by mechanism, not preference — see the second paragraph below for why**: opencode/codex/gemini/pi/grok/deepseek/omp apply the selection ONE-SHOT, before the session/process ever exists, with no restart at all; claude alone still applies a selection by **restarting the session's CLI process in place** via `Session.restartCli()` — a de-restricted `reattachRemote()` reusing the same `respawn-pane -k` primitive local/remote respawns already share — because every one of these harnesses reads its endpoint config at process start, never per-turn, so there is no live hot-swap; `Session.setCustomModel()` undoes the PREVIOUS selection's env keys (and deletes its old `configDir`) before merging the new ones in, so switching endpoints or clearing back to native cloud never leaves a stale key behind. ⚠️ Deleting a key from `_envOverrides` is NOT enough on its own: `tmux setenv` persists at the tmux-session level and is inherited by `respawn-pane` (measured: `setenv FOO bar` survived two successive `respawn-pane -k`), so the retired keys are queued (`_pendingEnvUnsets`) and ride `RespawnPaneOptions.unsetEnvKeys` into `applyEnvOverrides()`, which `setenv -u`s them BEFORE re-applying the live overrides. ⚠️ `restartCli()` kills a WORKING pane, so a CLI whose launch declares a `fallback` chain (claude) gets the live conversation id pinned as `resumeSessionId` for that one respawn: `--session-id ` refuses an id that already has a transcript (`Session ID ... is already in use`), and without the `--resume || --session-id ` shape the docker/remote pane commands already use, applying a model killed the pane and lost the session. ⚠️ pi, omp and grok need the config file AND a `model` launch param (`custom/` for pi/omp, grok's `[model.codeman-custom]` block name): that is the registry's `customModelInjection.launchModel` template, applied onto the respawn options through `legacyConfigField` by `_withCustomModelLaunchModel()`, never by id, and a model id the CLI's `model` token pattern cannot carry is refused with a 400 rather than silently dropped by the argv engine. ⚠️ Remote (SSH) and Docker sessions are REFUSED (400): their `restartCli()` reattaches a durable tmux rather than restarting the agent and the env lands on the local pane, so they used to report `restarted:true` and change nothing. The selection survives a Codeman restart as the disk-only `__customModel` (bookkeeping: env KEYS, config dir, launch model; never the values, which carry the API key and are re-derived from the endpoint store on recovery), the config dir is removed with the session, and every secret-bearing file (`custom-model-hosts.json`, the per-session config dir) is written 0600. ⚠️ **Security**: every env var this feature can redirect (`ANTHROPIC_BASE_URL`, `GOOGLE_GEMINI_BASE_URL`, `CODEX_HOME`, `GROK_HOME`, `HOME` for pi/omp, `OPENCODE_CONFIG_CONTENT`, etc.) is in that CLI's `privilegedEnvKeys` — several of these were reachable via the generic `envOverrides` field's prefix allowlist BEFORE this feature existed (the env allowlist is global and prefix-based, not per-CLI-scoped), so building this surfaced and closed a pre-existing gap rather than opening a new one. `ANTHROPIC_*` is deliberately NOT in claude's `allowedPrefixes` at all — Anthropic-traffic redirection can only happen through this feature's own admin-configured, SSRF-guarded route, never a plain client-supplied `envOverrides`. **Confidence, verified end-to-end against a real llama-swap server via the DYNAMIC `scripts/test-local-llm-harnesses.ts`** (reads the live CLI registry, so a registry change needs zero script edits): claude/opencode/pi/grok/omp **PASS**; codex config structure is correct, and codex only speaks the Responses API since Feb 2026 (`wire_api = "responses"`) — re-verified live against a llama-swap deployment that DOES answer `/v1/responses` (an earlier test's harder failure against a different deployment does not reproduce everywhere): a plain, no-tool-call chat turn gets a real reply, but a real tool-call attempt came back as `agent_message` TEXT (the tool-call JSON printed as the answer) rather than an executable `function_call` item — confirmed via `codex exec --json`'s raw event stream. Tool execution is what makes codex a coding agent, so it remains not usable for real work either way, just with a more precise failure mode than a flat protocol break; gemini fails with `Invalid auth method selected` (an undocumented `GATEWAY` AuthType gemini-cli selects once `GOOGLE_GEMINI_BASE_URL` is set — unresolved after real investigation); deepseek's originally-reported `HTTP_404` is root-caused and fixed — its bundled `@deepseek-ai/dsh-llm-deepseek` module builds `${DEEPSEEK_BASE_URL}/chat/completions` with no `/v1` of its own (confirmed by installing the real package and reading its source), so a new `appendV1Suffix` flag on its registry entry (alone — claude/gemini must not get it) runs `endpoint.baseUrl` through `withV1Suffix()` before writing it, live-confirmed against llama-swap (`.../chat/completions` 404s, `.../v1/chat/completions` succeeds) though not yet re-run through an actual `dsh` binary, which isn't installable in this environment; antigravity has no known mechanism at all. See the confidence table in `docs/custom-model-endpoints-plan.md` for the full detail on each. ⚠️ **The Run-menu picker generates entries from `window.__codemanCustomModelClis`** (`server.ts`, injected at page render from `enabledClis().filter(kind==='agent' && customModelInjection.kind!=='unsupported')`, JSON-escaped against a literal `` via the exported `escapeScriptJson()` since `label` is a user-`clis.json`-settable string unlike the neighbouring booleans-only `__codemanCliAvailable`), never a hardcoded per-CLI id list in the frontend — the same "no branching on CLI id outside stock.ts" discipline the registry itself enforces. One entry per (capable, INSTALLED CLI, saved endpoint) pair, e.g. "Claude Code (llama.cpp)", filtered through `isCliAvailable()` like the stock entries. Clicking one calls `selectCustomModelEntry(mode, endpointId)` (`session-ui.js`), which re-fetches the endpoint (never trusts anything cached from the dropdown's render — the 5-minute sweep below or a settings edit may have changed it since) and decides the model: exactly one discovered model launches straight away, two or more open `#customModelPickModal` to ask, with `defaultModelId` marked but never auto-chosen (asking exists so ONE launch can deliberately differ from the saved default). ⚠️ **The picker promotes exactly one row to the top**: whichever model llama-swap reports `ready` right now (tagged "Currently loaded", queried via `GET /api/model-endpoints/:id/running-status`, client-side bounded to ~800ms via `Promise.race` so a sleeping/firewalled endpoint cannot leave the modal invisible for the route's own 5s server-side timeout) beats a merely remembered choice, and — only when nothing is currently loaded — the model actually launched last for this exact (harness, endpoint) pair (tagged "Last used", read from the per-device `codeman:customModelLastUsed::` localStorage key). Neither tag reorders past the top, and the "Default" pill is a SEPARATE span rather than a third value of the same slot, so a promoted row that is also the endpoint's `defaultModelId` shows both (on a single-purpose GPU box that is the common case; an exclusive slot silently dropped the Default marking for exactly that row). "Last used" is written by `_runCustomModelEntryViaRestart` (claude) and `_quickStartWithCustomModelConfirm` (every one-shot launch; `runCustomModelEntry` itself only dispatches between the two) only once the model is actually applied — never on the mere click — because a context-window-warning decline means this exact model cannot work with this CLI at all, and promoting a model that cannot launch would be actively wrong, not just premature. Either way the actual launch (`runCustomModelEntry`) routes through `run()` itself via a temporary `_runMode` swap — never `setRunMode()`, which would persist it as the user's new default — rather than a parallel dispatch table, which is what gives a custom-model launch the same `_runInFlight` lock every other Run click gets and means a CLI whose injection recipe lands later needs no update here. It then GETs `/api/sessions/:id/wait?until=idle&timeout=20000` on that session BEFORE applying — measured live, a freshly launched CLI reports itself `busy` for its own startup (boot spinner, workspace-trust check) well before the apply call would otherwise reach it, and the apply route's `isBusy()` guard correctly can't tell that apart from a real turn in progress, so every fresh launch failed with `SESSION_BUSY` until this wait was added. A timeout there is a normal 200 per the wait endpoint's own contract, never an error, so a session still busy after 20s just reaches the apply call anyway and gets that route's own honest error. It then calls `POST /api/sessions/:id/custom-model` on the session `run()` produced, guarded by snapshotting `activeSessionId` before the call and requiring it to have actually changed after — every `run*()` handles its own failure internally and returns normally rather than throwing, so a declined/failed launch must not silently re-point and restart whatever session was already open. ⚠️ The apply call reads the response body itself (`_api()`) rather than `_apiJson()`, which unwraps success but silently discards a failure body — losing the one thing (`error`) that distinguishes "still busy", "remote/Docker session" and everything else the route can report (⚠️ neither route validates `modelId` against the endpoint's discovered list, deliberately: discovery can be up to 5 minutes stale, so a 400 there would refuse a launch that works — a typo'd id fails on the CLI's own first request instead); the resulting toast is `type: 'error'` with an explicit `duration: 0` (no auto-dismiss, an explicit close button) at that one call site — not a blanket sticky-error default, which stacked unbounded on `.toast-container` with no cap or eviction — precisely so a message worth diagnosing survives long enough to be read instead of vanishing on the usual 3s timer. Entries are hidden for a remote/docker active case (the apply route refuses both) and for an endpoint with no discovered models at all (nothing to launch with). ⚠️ **Every saved endpoint's models also re-discover themselves automatically**, a `this.cleanup.setInterval` in `server.ts` (`CUSTOM_MODEL_REDISCOVER_INTERVAL_MS`, 5 minutes, off under `testMode` like the Codex plan-usage poll beside it) calling the exported `refreshAllCustomModelHosts()` (`custom-model-routes.ts`) — one endpoint unreachable on a cycle never blocks the others, and a read-modify-write PER HOST (re-reading the store before each splice, keyed by id) means an admin's concurrent edit or delete wins over a sweep that started before it, never the reverse. +**Custom Model Endpoint Profiles** (opt-in, `customModelEndpointsEnabled`, SYNCED, default OFF; `docs/custom-model-endpoints.md`, design doc `docs/custom-model-endpoints-plan.md`; full stack — settings-panel CRUD + the Run-menu picker, on top of the backend below): points a session at a user-configured custom OpenAI-compatible endpoint — local (llama.cpp, DGX Spark, Strix Halo) or cloud (Azure AI Foundry, OpenRouter) — instead of its harness's native cloud backend. Endpoints are a read/write-array store (`custom-model-hosts.ts`, `~/.codeman/custom-model-hosts.json`) discovered via `GET /v1/models`; `CustomModelHost.authStyle` is `'bearer'` (default, `Authorization: Bearer`) or `'api-key'` (Azure's convention) — **never both**, live-tested against a real server: sending both headers on one request reliably hangs it indefinitely, reproduced 3×. ⚠️ The actual per-CLI redirect is `capabilities.customModelInjection` on the CLI registry (four kinds: `env` for claude/gemini/deepseek, `configContentEnv` reusing opencode's existing `OPENCODE_CONFIG_CONTENT`, `configDir` for codex/pi/grok/omp — writes an isolated per-session config file, NEVER the user's real `~/.codex`/`~/.pi`/`~/.omp`/grok config — and `unsupported` for antigravity, which has no known mechanism), computed by the pure `custom-model-injection.ts` (mirrors `session-cli-builder.ts`'s no-IO discipline). ⚠️ `PI_CONFIG_DIR` does NOTHING for pi or omp (grepped pi's entire bundled JS source — the string appears nowhere); both hardcode `~/.pi/agent/models.json` / `~/.omp/agent/models.yml` with no dedicated override, so the real redirect for both is the child process's own **`HOME`**, and both need `models` as an ARRAY of `{id}` objects (an object keyed by id silently loads zero models). Grok's real mechanism turned out to be a `config.toml` `[model.]` block redirected via `GROK_HOME` — its original env-var-based recipe was flat-out wrong (produced "Not signed in" against a real binary), not just unverified. ⚠️ **Two launch paths, chosen by mechanism, not preference — see the second paragraph below for why**: opencode/codex/gemini/pi/grok/deepseek/omp apply the selection ONE-SHOT, before the session/process ever exists, with no restart at all; claude alone still applies a selection by **restarting the session's CLI process in place** via `Session.restartCli()` — a de-restricted `reattachRemote()` reusing the same `respawn-pane -k` primitive local/remote respawns already share — because every one of these harnesses reads its endpoint config at process start, never per-turn, so there is no live hot-swap; `Session.setCustomModel()` undoes the PREVIOUS selection's env keys (and deletes its old `configDir`) before merging the new ones in, so switching endpoints or clearing back to native cloud never leaves a stale key behind. ⚠️ Deleting a key from `_envOverrides` is NOT enough on its own: `tmux setenv` persists at the tmux-session level and is inherited by `respawn-pane` (measured: `setenv FOO bar` survived two successive `respawn-pane -k`), so the retired keys are queued (`_pendingEnvUnsets`) and ride `RespawnPaneOptions.unsetEnvKeys` into `applyEnvOverrides()`, which `setenv -u`s them BEFORE re-applying the live overrides. ⚠️ `restartCli()` relaunches a CLI in an existing pane, so a CLI whose launch declares a `fallback` chain (claude) gets a conversation pinned as `resumeSessionId` for that one respawn: `--session-id ` refuses an id that already has a transcript (`Session ID ... is already in use`), and without the `--resume || --session-id ` shape the docker/remote pane commands already use, applying a model killed the pane and lost the session. ⚠️ **The dead-pane respawn needs the same pin and shares it** (`_buildRespawnPaneOptionsWithResumePin()` in `session.ts`, used by `restartCli()`, the dead-pane respawn in `_setupOrAttachMuxSession()` and the create-path fallback after a failed respawn): a pane whose agent EXITED owns a transcript too, so recovering one with the bare launch line hit the same refusal and the conversation was stranded behind a tab that looked merely idle. The pin walks three candidates in order — the conversation chain's tail, the launch seed, then the session's own id — and takes the first one a transcript backs, never `_claudeSessionId` (which also holds history-correlated GUESSES keyed on the working directory, and launching from one would open and write to a conversation that was never this pane's). ⚠️ A candidate no transcript backs is passed over, and falling off the end of the walk pins NOTHING: a divergent pin leaves `--session-id ` in the fallback branch, where a failed resume collides all over again, while pinning an id with no transcript prints claude's "No conversation found" into a brand-new session's scrollback and costs the running branch its `nice` priority (`wrapWithNice()` prefixes only the first branch of an `a || b`). ⚠️ Remote and docker sessions are never pinned: their pane commands are already self-healing, the conversation lives on the far side, and a local id resolves to nothing there. ⚠️ pi, omp and grok need the config file AND a `model` launch param (`custom/` for pi/omp, grok's `[model.codeman-custom]` block name): that is the registry's `customModelInjection.launchModel` template, applied onto the respawn options through `legacyConfigField` by `_withCustomModelLaunchModel()`, never by id, and a model id the CLI's `model` token pattern cannot carry is refused with a 400 rather than silently dropped by the argv engine. ⚠️ Remote (SSH) and Docker sessions are REFUSED (400): their `restartCli()` reattaches a durable tmux rather than restarting the agent and the env lands on the local pane, so they used to report `restarted:true` and change nothing. The selection survives a Codeman restart as the disk-only `__customModel` (bookkeeping: env KEYS, config dir, launch model; never the values, which carry the API key and are re-derived from the endpoint store on recovery), the config dir is removed with the session, and every secret-bearing file (`custom-model-hosts.json`, the per-session config dir) is written 0600. ⚠️ **Security**: every env var this feature can redirect (`ANTHROPIC_BASE_URL`, `GOOGLE_GEMINI_BASE_URL`, `CODEX_HOME`, `GROK_HOME`, `HOME` for pi/omp, `OPENCODE_CONFIG_CONTENT`, etc.) is in that CLI's `privilegedEnvKeys` — several of these were reachable via the generic `envOverrides` field's prefix allowlist BEFORE this feature existed (the env allowlist is global and prefix-based, not per-CLI-scoped), so building this surfaced and closed a pre-existing gap rather than opening a new one. `ANTHROPIC_*` is deliberately NOT in claude's `allowedPrefixes` at all — Anthropic-traffic redirection can only happen through this feature's own admin-configured, SSRF-guarded route, never a plain client-supplied `envOverrides`. **Confidence, verified end-to-end against a real llama-swap server via the DYNAMIC `scripts/test-local-llm-harnesses.ts`** (reads the live CLI registry, so a registry change needs zero script edits): claude/opencode/pi/grok/omp **PASS**; codex config structure is correct, and codex only speaks the Responses API since Feb 2026 (`wire_api = "responses"`) — re-verified live against a llama-swap deployment that DOES answer `/v1/responses` (an earlier test's harder failure against a different deployment does not reproduce everywhere): a plain, no-tool-call chat turn gets a real reply, but a real tool-call attempt came back as `agent_message` TEXT (the tool-call JSON printed as the answer) rather than an executable `function_call` item — confirmed via `codex exec --json`'s raw event stream. Tool execution is what makes codex a coding agent, so it remains not usable for real work either way, just with a more precise failure mode than a flat protocol break; gemini fails with `Invalid auth method selected` (an undocumented `GATEWAY` AuthType gemini-cli selects once `GOOGLE_GEMINI_BASE_URL` is set — unresolved after real investigation); deepseek's originally-reported `HTTP_404` is root-caused and fixed — its bundled `@deepseek-ai/dsh-llm-deepseek` module builds `${DEEPSEEK_BASE_URL}/chat/completions` with no `/v1` of its own (confirmed by installing the real package and reading its source), so a new `appendV1Suffix` flag on its registry entry (alone — claude/gemini must not get it) runs `endpoint.baseUrl` through `withV1Suffix()` before writing it, live-confirmed against llama-swap (`.../chat/completions` 404s, `.../v1/chat/completions` succeeds) though not yet re-run through an actual `dsh` binary, which isn't installable in this environment; antigravity has no known mechanism at all. See the confidence table in `docs/custom-model-endpoints-plan.md` for the full detail on each. ⚠️ **The Run-menu picker generates entries from `window.__codemanCustomModelClis`** (`server.ts`, injected at page render from `enabledClis().filter(kind==='agent' && customModelInjection.kind!=='unsupported')`, JSON-escaped against a literal `` via the exported `escapeScriptJson()` since `label` is a user-`clis.json`-settable string unlike the neighbouring booleans-only `__codemanCliAvailable`), never a hardcoded per-CLI id list in the frontend — the same "no branching on CLI id outside stock.ts" discipline the registry itself enforces. One entry per (capable, INSTALLED CLI, saved endpoint) pair, e.g. "Claude Code (llama.cpp)", filtered through `isCliAvailable()` like the stock entries. Clicking one calls `selectCustomModelEntry(mode, endpointId)` (`session-ui.js`), which re-fetches the endpoint (never trusts anything cached from the dropdown's render — the 5-minute sweep below or a settings edit may have changed it since) and decides the model: exactly one discovered model launches straight away, two or more open `#customModelPickModal` to ask, with `defaultModelId` marked but never auto-chosen (asking exists so ONE launch can deliberately differ from the saved default). ⚠️ **The picker promotes exactly one row to the top**: whichever model llama-swap reports `ready` right now (tagged "Currently loaded", queried via `GET /api/model-endpoints/:id/running-status`, client-side bounded to ~800ms via `Promise.race` so a sleeping/firewalled endpoint cannot leave the modal invisible for the route's own 5s server-side timeout) beats a merely remembered choice, and — only when nothing is currently loaded — the model actually launched last for this exact (harness, endpoint) pair (tagged "Last used", read from the per-device `codeman:customModelLastUsed::` localStorage key). Neither tag reorders past the top, and the "Default" pill is a SEPARATE span rather than a third value of the same slot, so a promoted row that is also the endpoint's `defaultModelId` shows both (on a single-purpose GPU box that is the common case; an exclusive slot silently dropped the Default marking for exactly that row). "Last used" is written by `_runCustomModelEntryViaRestart` (claude) and `_quickStartWithCustomModelConfirm` (every one-shot launch; `runCustomModelEntry` itself only dispatches between the two) only once the model is actually applied — never on the mere click — because a context-window-warning decline means this exact model cannot work with this CLI at all, and promoting a model that cannot launch would be actively wrong, not just premature. Either way the actual launch (`runCustomModelEntry`) routes through `run()` itself via a temporary `_runMode` swap — never `setRunMode()`, which would persist it as the user's new default — rather than a parallel dispatch table, which is what gives a custom-model launch the same `_runInFlight` lock every other Run click gets and means a CLI whose injection recipe lands later needs no update here. It then GETs `/api/sessions/:id/wait?until=idle&timeout=20000` on that session BEFORE applying — measured live, a freshly launched CLI reports itself `busy` for its own startup (boot spinner, workspace-trust check) well before the apply call would otherwise reach it, and the apply route's `isBusy()` guard correctly can't tell that apart from a real turn in progress, so every fresh launch failed with `SESSION_BUSY` until this wait was added. A timeout there is a normal 200 per the wait endpoint's own contract, never an error, so a session still busy after 20s just reaches the apply call anyway and gets that route's own honest error. It then calls `POST /api/sessions/:id/custom-model` on the session `run()` produced, guarded by snapshotting `activeSessionId` before the call and requiring it to have actually changed after — every `run*()` handles its own failure internally and returns normally rather than throwing, so a declined/failed launch must not silently re-point and restart whatever session was already open. ⚠️ The apply call reads the response body itself (`_api()`) rather than `_apiJson()`, which unwraps success but silently discards a failure body — losing the one thing (`error`) that distinguishes "still busy", "remote/Docker session" and everything else the route can report (⚠️ neither route validates `modelId` against the endpoint's discovered list, deliberately: discovery can be up to 5 minutes stale, so a 400 there would refuse a launch that works — a typo'd id fails on the CLI's own first request instead); the resulting toast is `type: 'error'` with an explicit `duration: 0` (no auto-dismiss, an explicit close button) at that one call site — not a blanket sticky-error default, which stacked unbounded on `.toast-container` with no cap or eviction — precisely so a message worth diagnosing survives long enough to be read instead of vanishing on the usual 3s timer. Entries are hidden for a remote/docker active case (the apply route refuses both) and for an endpoint with no discovered models at all (nothing to launch with). ⚠️ **Every saved endpoint's models also re-discover themselves automatically**, a `this.cleanup.setInterval` in `server.ts` (`CUSTOM_MODEL_REDISCOVER_INTERVAL_MS`, 5 minutes, off under `testMode` like the Codex plan-usage poll beside it) calling the exported `refreshAllCustomModelHosts()` (`custom-model-routes.ts`) — one endpoint unreachable on a cycle never blocks the others, and a read-modify-write PER HOST (re-reading the store before each splice, keyed by id) means an admin's concurrent edit or delete wins over a sweep that started before it, never the reverse. **Everything below landed after the initial backend + picker cut, each confirmed live against a real llama-swap deployment.** ⚠️ **llama-swap runs one model at a time, and switching can disrupt ANOTHER live session** — before applying, both apply routes call llama-swap's own `GET /running` (feature-detected via `getLlamaSwapStatus()`, `custom-model-routes.ts`; a plain llama.cpp/OpenAI-compatible server has no such endpoint and is simply never checked). If a different model is loaded and ready AND another live session's own selection is using it, the apply returns `{requiresConfirmation, currentlyLoadedModel, affectedSessions}` instead of switching silently; retrying with `confirmedSwap: true` skips the check, and switching with nothing else affected proceeds immediately. ⚠️ **The swap question and the context-floor question below have SEPARATE flags** (`confirmedSwap`, `confirmedContext`), because the context check runs first and while both shared one `confirmed` a user who clicked past a too-small context silently consented to evicting another session's model too; the legacy `confirmed` still means both, since it shipped in the HTTP-API-only cut. llama-swap also has no dedicated "switch model" endpoint — the only thing that actually starts a swap is a real inference request naming the model (confirmed live: applying a selection alone never reached llama-swap's own logs, since nothing had asked it to load anything) — so both routes also fire `triggerLlamaSwapLoad()`, a fire-and-forget `POST /v1/chat/completions` with `max_tokens: 1`, whenever the target model isn't already loaded and ready. ⚠️ **That launch-time check cannot catch a swap caused by a DIFFERENT session's LATER, ordinary use** — confirmed live: a second Codex session picking a different model launched with no warning at all (nothing conflicted at that exact instant), yet it silently evicted the first session's model regardless, since llama-swap has no push notification of its own. `detectCustomModelSwapDisplacements()` (`custom-model-routes.ts`) is a separate periodic sweep (`server.ts`, `CUSTOM_MODEL_SWAP_CHECK_INTERVAL_MS` = 20s) that compares each live custom-model session's own `modelId` against what `/running` actually reports loaded, broadcasting a `custom-model:swapped-out` SSE event — shown as a global toast, never tied to the displaced session's own tab, since the whole point is telling the user before they type into it — the first time a mismatch appears, via a caller-owned de-dupe `Set` cleared once that session's own model is loaded and ready again so a later, genuinely new displacement notifies again rather than staying silently un-notified forever after the first one. ⚠️ **Context length is read from the REAL launch command, never `/props`** — `/props?model=`'s `default_generation_settings.n_ctx` was confirmed live to report a `--fit-ctx`-launched backend's theoretical/trained maximum rather than the real runtime-configured size (a measured 154112-vs-16384 discrepancy, caught only because the unfixed value still overflowed), so discovery parses the actual configured size straight out of `/running`'s own `cmd` field instead (`parseCtxFromCmd`: `--fit-ctx ` first, then plain llama.cpp `-c`/`--ctx-size`), falling back to `/props` only when `cmd` states no recognizable flag at all. ⚠️ **Claude alone gets a context-window FLOOR check, on top of the ceiling `contextLengthVar` already fixes** — `exceedsSafeContextFloor()` (gated on the registry declaring `contextLengthVar`, so a no-op for every other CLI by construction) compares a model's discovered context against `CLAUDE_MIN_SAFE_CONTEXT_TOKENS` (40000): confirmed live, twice, that Claude Code's own system prompt and tool schemas cost roughly 36.4K tokens on the very first message, before any conversation history exists to compact, so a smaller real context fails outright regardless of what `CLAUDE_CODE_MAX_CONTEXT_TOKENS` says (that var only controls when HISTORY gets compacted, and there is none yet on message one). Below the floor, the apply returns `{requiresContextWarning, modelId, contextLength, minSafeContextTokens}` instead of launching, shown as an in-app dialog naming the actual fix: give the model an explicit larger `-c`/`--ctx-size` in llama-swap's config instead of relying on `--fit-ctx` auto-fit, which optimizes for the biggest MODEL that fits rather than the biggest CONTEXT. ⚠️ **A fresh, isolated `CLAUDE_CONFIG_DIR` looks like a brand-new Claude Code profile and replays its ENTIRE first-run sequence on every launch** — the theme picker, the security-notes screen, the per-project "trust this folder?" dialog, and (running with a bypass-permissions flag) a one-time warning about it, confirmed live, none of which a real, already-onboarded profile shows again. `skipFirstRunPrompts` (claude's entry only, requires `apiKeyTrustFile` since it reuses the same file) pre-seeds that same "already been through this" state: `hasCompletedOnboarding` and this session's own `projects[workingDir].hasTrustDialogAccepted` merge into the same `.claude.json` the API-key trust file already writes to, and `skipDangerousModePermissionPrompt` merges into `settings.json` (a different file, same corrupt-tolerant merge). ⚠️ **The loading banner shows the REAL backend log line, not a guess, and has no countdown or auto-timeout at all.** `getLatestLlamaSwapLogLine()` holds one `GET /api/events` SSE connection open per endpoint (confirmed live to stay open indefinitely — read past 220KB over 8s with no `done`; idle-closed after 30s via `pruneIdleLlamaSwapLogTails`, same 20s sweep as the swap-displacement check above), parsing `logData` frames and keeping only `source: "upstream"` (the real `llama-server` process's own stdout) lines, never `source: "proxy"` (llama-swap's own request-access log). ⚠️ `GET /logs` — the endpoint this feature's own first cut targeted, since the name suggested it — was confirmed live to carry ONLY the proxy log and never a single backend line, even seconds after a real, verified model swap; caught and corrected by a live check before merge, not after. The banner itself dropped its size-scaled expected-time estimate and matching auto-timeout (a guess dressed up as a fact that could kill a genuinely slow load partway through on slower hardware) for a generic hardware/model-size disclaimer plus a user-driven **Cancel** button (`_showCenterStatus`'s `onCancel` option, a real button distinct from the plain "×" close glyph an `'error'`-type banner gets) that ends the wait and closes the session on the user's own call rather than a guessed deadline. diff --git a/src/session.ts b/src/session.ts index b3226332..ced7862c 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1772,11 +1772,22 @@ export class Session extends EventEmitter { // launch seed, so a session whose transcript exists would meet the same // `--session-id ... already in use` refusal the respawn just lost to — // the recovery of last resort failing for the very reason it was needed. - // A genuinely new session has no chain and no transcript, so the pin - // resolves to its own id and the command shape is unchanged. + // A genuinely new session has no transcript under any of its candidate + // ids, so nothing is pinned and its command shape is unchanged. + // + // `_resumeSessionId` is written alongside, not just the create options: + // this branch leaves `isRestored` false, so the block that sets + // `_claudeSessionId` below reads that field and would otherwise settle on + // `this.id` while the CLI resumes the chain tail. The response viewer, + // Read My Mind and the unified-list alias map all read `_claudeSessionId` + // until the next first-hand hook, so the two have to name the same + // conversation. if (needsNewSession) { const pinned = (await this._buildRespawnPaneOptionsWithResumePin()).resumeSessionId; - if (pinned) options.createSessionOptions.resumeSessionId = pinned; + if (pinned) { + options.createSessionOptions.resumeSessionId = pinned; + this._resumeSessionId = pinned; + } } this._muxSession = await mux.createSession(options.createSessionOptions); console.log('[Session] Created mux session:', this._muxSession.muxName); @@ -1957,8 +1968,11 @@ export class Session extends EventEmitter { * declares a `fallback` chain renders `resume || new` once a resume id is * set, which is the shape that survives both cases. * - * Four conditions gate the pin, each protecting against a way of resuming the - * WRONG conversation or of making a working relaunch fail. + * Three candidates are tried in priority order — the conversation chain's + * tail, the launch seed, then the session's own id — and the first one a + * transcript backs is pinned. Four conditions gate that walk, each protecting + * against a way of resuming the WRONG conversation or of making a working + * relaunch fail. * * ⚠️ **A remote or docker session is never pinned.** Unlike `restartCli()`, * whose route refuses both, the dead-pane respawn is reached by every session @@ -1971,7 +1985,7 @@ export class Session extends EventEmitter { * transcript the far side really does hold — both branches fail and the pane * dies. `_pinOmpRespawnId()` refuses remote for the same reason. * - * ⚠️ **The id comes from the conversation CHAIN, not from + * ⚠️ **The candidates come from the conversation CHAIN, never from * `_claudeSessionId`.** That field holds either a first-hand id from the * CLI's own hook payload or a history correlation, which is a guess keyed on * the working directory. `_recordClaudeSessionInChain()` refuses a guess @@ -1979,13 +1993,24 @@ export class Session extends EventEmitter { * permanent record", and launching from one would do worse than the display * bug that rule exists to prevent: the relaunched CLI would open and WRITE to * a conversation that was never this pane's. The chain's tail is the live - * conversation and is hook-vouched, so it also outranks the launch seed, - * which is written once at construction and never moves off a `/clear`. + * conversation and is hook-vouched, so it leads the walk, ahead of the launch + * seed, which is written once at construction and never moves off a `/clear`. + * + * ⚠️ **Every candidate must be backed by a transcript, the session's own id + * included, and a candidate that has none is passed over rather than ending + * the walk.** A pin that differs from the session id leaves + * `--session-id ` in the fallback branch, so a resume that finds + * nothing collides there and the pane dies exactly as it did before this + * pinning existed. Pinning `this.id` renders the self-healing + * `--resume || --session-id `, which is correct whether or not a + * transcript exists, but a pane that has none pays for the shape twice: + * claude prints "No conversation found" into the scrollback of a session that + * is brand new, and `wrapWithNice()` prefixes only the FIRST branch of the + * rendered `a || b`, so the branch that actually runs loses its priority for + * the life of the session. Falling off the end of the walk therefore pins + * nothing, which is the right answer: with no transcript anywhere there is + * nothing for the bare `--session-id ` to collide with. * - * ⚠️ **A pin that no transcript backs is dropped.** When the pinned id - * differs from the session id, the fallback branch still carries - * `--session-id `; if the resume finds nothing, that fallback - * collides and the pane dies exactly as it did before this pinning existed. * The create route pre-validates a resume id for the same reason, though it * additionally requires the transcript be substantial — here mere existence * is the question, because a one-line transcript still makes `--session-id` @@ -2002,28 +2027,29 @@ export class Session extends EventEmitter { private async _buildRespawnPaneOptionsWithResumePin(): Promise { const options = this._buildRespawnPaneOptions(); if (this._remote || this._docker) return options; - if (getCli(this.mode)?.launch.chain !== 'fallback') return options; + const entry = getCli(this.mode); + if (entry?.launch.chain !== 'fallback') return options; + const resumeIdPattern = entry.launch.params?.resumeId; + const configDir = this._claudeConfigDir(); const chainTail = this._claudeSessionChain[this._claudeSessionChain.length - 1]; - const pin = chainTail ?? options.resumeSessionId ?? this.id; - // A session Codeman DISCOVERED on the socket rather than created carries a - // synthetic `restored-` id, which fails claude's `uuid` token - // pattern. The renderer would silently drop the resume flag and emit the - // unpinned command, so say so here rather than letting the caller believe - // the pane was pinned. Such a pane keeps the pre-existing behaviour. - const resumeIdPattern = getCli(this.mode)?.launch.params?.resumeId; - if (resumeIdPattern?.type === 'token' && !matchesPattern(resumeIdPattern.pattern, pin)) { - console.log(`[Session] Not pinning resume id ${pin} for relaunch: the CLI cannot accept that id shape`); + const candidates = [chainTail, options.resumeSessionId, this.id].filter((v): v is string => !!v); + for (const candidate of candidates) { + // A session Codeman DISCOVERED on the socket rather than created carries a + // synthetic `restored-` id, which fails claude's `uuid` token + // pattern. The renderer would silently drop the resume flag and emit the + // unpinned command, so say so here rather than letting the caller believe + // the pane was pinned. + if (resumeIdPattern?.type === 'token' && !matchesPattern(resumeIdPattern.pattern, candidate)) { + console.log(`[Session] Not pinning resume id ${candidate} for relaunch: the CLI cannot accept that id shape`); + continue; + } + if (!(await claudeTranscriptExists(candidate, configDir))) continue; + options.resumeSessionId = candidate; return options; } - // Pinning the session's own id renders the self-healing - // `--session-id || --resume `, which needs no transcript to be - // correct: it starts fresh when there is none and resumes when there is. - if (pin !== this.id && !(await claudeTranscriptExists(pin, this._claudeConfigDir()))) { - console.log(`[Session] Not pinning resume id ${pin} for relaunch: no transcript on disk`); - return options; - } - options.resumeSessionId = pin; + // Nothing on disk to collide with, so the bare `--session-id ` the + // unpinned options already carry is the correct command. return options; } diff --git a/src/utils/claude-transcript.ts b/src/utils/claude-transcript.ts index 6c6c4b49..8ce01f8b 100644 --- a/src/utils/claude-transcript.ts +++ b/src/utils/claude-transcript.ts @@ -16,6 +16,11 @@ * conversation worth resuming" judgement; for a relaunch the question is the * opposite one — a one-line transcript still makes `--session-id` collide. * + * ⚠️ A false answer is not the conservative one. Skipping a resume leaves the + * relaunch on `--session-id `, which is safe only when no transcript backs + * that id either, so a lookup that misses the real config dir turns a + * recoverable pane into the collision this module exists to prevent. + * * @dependencies none * @consumedby session (relaunch resume pinning) * @@ -26,15 +31,28 @@ import { readdir, stat } from 'node:fs/promises'; import { homedir } from 'node:os'; import { join } from 'node:path'; -/** `/projects`, honouring a session's relocated `CLAUDE_CONFIG_DIR`. */ +/** + * `/projects`, honouring a session's relocated `CLAUDE_CONFIG_DIR` + * (#255) and, failing that, the server process's own. + * + * ⚠️ The process env is not optional here. A pane inherits the server's + * environment through tmux, so on an install that exports `CLAUDE_CONFIG_DIR` + * the CLI writes its transcripts there and a lookup under `~/.claude` answers + * "no transcript" for every conversation on the host. `claudeCredentialsPath()` + * (claude-credentials.ts) and `realClaudeConfigDir()` + * (custom-model-injection-apply.ts) resolve the same directory the same way. + */ export function claudeProjectsDir(configDir?: string): string { - return join(configDir || join(homedir(), '.claude'), 'projects'); + const fromEnv = typeof process.env.CLAUDE_CONFIG_DIR === 'string' && process.env.CLAUDE_CONFIG_DIR.trim(); + return join(configDir || fromEnv || join(homedir(), '.claude'), 'projects'); } /** * True when a transcript for `conversationId` exists under any project - * directory. Returns false for a missing projects dir or an unreadable one: - * the caller's fallback is to skip the resume, which is the safe direction. + * directory. Returns false for a missing projects dir or an unreadable one, + * which leaves the caller unpinned: safe where nothing else can collide with + * the bare `--session-id`, and the reason the caller walks its candidates down + * to the session's own id rather than treating one false answer as final. */ export async function claudeTranscriptExists(conversationId: string, configDir?: string): Promise { if (!conversationId) return false; diff --git a/test/respawn-session-id-collision.test.ts b/test/respawn-session-id-collision.test.ts index 34b3f24a..90591e22 100644 --- a/test/respawn-session-id-collision.test.ts +++ b/test/respawn-session-id-collision.test.ts @@ -14,9 +14,13 @@ * WORKING pane whose conversation already has a transcript". That assumption is * what these tests refute: a pane whose agent exited has a transcript too. * - * The pin is gated four ways, and most of these tests are about the gates - * rather than the pin, because each gate stands for a way of resuming the WRONG - * conversation or of making a working relaunch fail. + * The pin walks three candidates — the conversation chain's tail, the launch + * seed, then the session's own id — and takes the first one a transcript backs. + * Most of these tests are about the four gates on that walk rather than about + * the pin, because each gate stands for a way of resuming the WRONG + * conversation or of making a working relaunch fail. Two more are about what + * the walk does when a candidate misses: it carries on to the next, and pinning + * nothing is the right answer only once every candidate has missed. * * Port: N/A */ @@ -27,7 +31,13 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { Session } from '../src/session.js'; import { getCli } from '../src/config/cli-registry/registry.js'; import { buildSpawnCommandFromRegistry } from '../src/session-cli-registry-bridge.js'; -import type { MuxSession, RespawnPaneOptions, TerminalMultiplexer } from '../src/mux-interface.js'; +import { claudeTranscriptExists } from '../src/utils/claude-transcript.js'; +import type { + CreateSessionOptions, + MuxSession, + RespawnPaneOptions, + TerminalMultiplexer, +} from '../src/mux-interface.js'; /** Captures the options each respawn is invoked with. */ function recordingMux() { @@ -50,6 +60,28 @@ function recordingMux() { const muxSession = (muxName = 'codeman-aaaa') => ({ muxName, sessionId: 'aaaa' }) as unknown as MuxSession; +/** + * A mux whose `respawnPane` fails, which is what sends + * `_setupOrAttachMuxSession()` down its create-a-new-session fallback — the + * path that has to pin too, since it meets the same refusal the respawn just + * lost to. + */ +function failingRespawnMux() { + const calls: CreateSessionOptions[] = []; + const mux = { + isAvailable: () => true, + muxSessionExists: () => true, + isPaneDead: () => true, + setAttached: () => {}, + respawnPane: async () => 0, + createSession: async (options: CreateSessionOptions) => { + calls.push(options); + return muxSession('codeman-recreated'); + }, + }; + return { mux: mux as unknown as TerminalMultiplexer, calls }; +} + const CONVERSATION = 'aaaabbbb-cccc-dddd-eeee-ffff00001111'; let configDir: string; @@ -146,6 +178,10 @@ describe('pinning a conversation onto a relaunch', () => { giveTranscript(CONVERSATION); const { mux, calls } = recordingMux(); const session = localSession({}, mux); + // Backed, so the walk reaching it pins it. Without this the session would + // land unpinned for want of a transcript rather than for refusing the + // guess, and the test would pass while proving nothing. + giveTranscript(session.id); session.adoptClaudeSessionId(CONVERSATION); // no firstHand flag: a guess expect(session.claudeSessionId).toBe(CONVERSATION); @@ -155,18 +191,73 @@ describe('pinning a conversation onto a relaunch', () => { expect(calls[0].resumeSessionId).toBe(session.id); }); - it('drops a pin that no transcript backs', async () => { - // A divergent pin renders `--resume || --session-id `, so a - // resume that finds nothing falls back onto the colliding form and the pane - // dies exactly as it did before any of this. No transcript, no pin. + it('degrades to the session id rather than to the colliding bare command', async () => { + // The chain tail is gone from disk but the session's own id is not, which + // is every session prompted before its first `/clear`. Dropping the pin + // outright hands back `--session-id ` alone — the very refusal + // this whole mechanism removes — so the walk carries on to the next + // candidate instead of stopping at the first miss. const { mux, calls } = recordingMux(); const session = localSession({ claudeSessionChain: [CONVERSATION] }, mux); + giveTranscript(session.id); + + expect(await session.restartCli()).toBe(true); + + expect(calls[0].resumeSessionId).toBe(session.id); + }); + + it('pins nothing at all when no candidate has a transcript', async () => { + // Falling off the end of the walk is the one case where the bare + // `--session-id ` is right: nothing on disk can collide with it, + // and pinning anyway would cost a brand-new pane claude's "No conversation + // found" line plus the `nice` priority on the branch that actually runs. + // The walk only ever ADDS a pin, so a session launched as a resume keeps + // the seed its options already carried — see the custom-model restart + // tests, which cover that case. + const { mux, calls } = recordingMux(); + const session = localSession({}, mux); expect(await session.restartCli()).toBe(true); expect(calls[0].resumeSessionId).toBeUndefined(); }); + it('pins the create-path fallback after a failed respawn', async () => { + // The recovery of last resort would otherwise meet the same refusal that + // made it the fallback, since the create options were built eagerly from + // the unpinned launch seed. + giveTranscript(CONVERSATION); + const { mux, calls } = failingRespawnMux(); + const session = localSession({ claudeSessionChain: [CONVERSATION] }, mux); + + await session.startInteractive(); + try { + expect(calls).toHaveLength(1); + expect(calls[0].resumeSessionId).toBe(CONVERSATION); + } finally { + await session.stop(); + } + }); + + it('leaves the session naming the conversation the create path resumed', async () => { + // That path leaves `isRestored` false, so `_claudeSessionId` is recomputed + // from the launch fields and lands on `this.id` unless the pin is written + // back to `_resumeSessionId` as well. The response viewer, Read My Mind and + // the unified-list alias map read that field until the next first-hand + // hook, so a mismatch points all three at a conversation claude never + // opened. + giveTranscript(CONVERSATION); + const { mux } = failingRespawnMux(); + const session = localSession({ claudeSessionChain: [CONVERSATION] }, mux); + + await session.startInteractive(); + try { + expect(session.claudeSessionId).toBe(CONVERSATION); + } finally { + await session.stop(); + } + }); + it('pins nothing for a remote session, whose conversation lives elsewhere', async () => { // The dead-pane respawn is reached by every session shape, unlike // `restartCli()` whose route refuses remote. A local id pinned onto a @@ -280,3 +371,35 @@ describe('what the pin renders', () => { expect(render('restored-40568a29')).not.toContain('--resume'); }); }); + +describe('where the transcript lookup reads', () => { + it("honours the server process's own CLAUDE_CONFIG_DIR", async () => { + // A pane inherits the server environment through tmux, so on an install + // that exports this the CLI writes its transcripts there. Reading `~/.claude` + // regardless answers "no transcript" for every conversation on the host, + // and under the walk above that means the colliding bare command. + giveTranscript(CONVERSATION); + const before = process.env.CLAUDE_CONFIG_DIR; + process.env.CLAUDE_CONFIG_DIR = configDir; + try { + expect(await claudeTranscriptExists(CONVERSATION)).toBe(true); + } finally { + if (before === undefined) delete process.env.CLAUDE_CONFIG_DIR; + else process.env.CLAUDE_CONFIG_DIR = before; + } + }); + + it("prefers the session's own relocated dir over the process one", async () => { + // A session pointed at a separate Claude account (#255) reads its own tree, + // not the server's. + giveTranscript(CONVERSATION); + const before = process.env.CLAUDE_CONFIG_DIR; + process.env.CLAUDE_CONFIG_DIR = join(configDir, 'nowhere'); + try { + expect(await claudeTranscriptExists(CONVERSATION, configDir)).toBe(true); + } finally { + if (before === undefined) delete process.env.CLAUDE_CONFIG_DIR; + else process.env.CLAUDE_CONFIG_DIR = before; + } + }); +}); diff --git a/test/session-custom-model-restart.test.ts b/test/session-custom-model-restart.test.ts index 6f0a3841..61ee1ef3 100644 --- a/test/session-custom-model-restart.test.ts +++ b/test/session-custom-model-restart.test.ts @@ -12,7 +12,8 @@ * 2. Applying to a local claude session killed the pane: the relaunch was * `claude --session-id ` and Claude refuses an id that already has a * transcript, so it needs the `--resume || --session-id ` shape the - * docker and remote pane commands use, i.e. a pinned resume id. + * docker and remote pane commands use, i.e. a pinned resume id. The pin is + * gated on that transcript existing, so these tests write one. * 3. pi/omp/grok wrote their config file and then launched without the `--model` * that selects it, so the file was ignored. * @@ -20,7 +21,7 @@ * spying on `respawnPane` to read the options the relaunch would get. * Port: N/A. */ -import { mkdirSync, rmSync } from 'node:fs'; +import { mkdirSync, rmSync, writeFileSync } from 'node:fs'; import { homedir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, expect, it, vi } from 'vitest'; @@ -30,13 +31,29 @@ import { TmuxManager } from '../src/tmux-manager.js'; import type { MuxSession, SessionMode } from '../src/types.js'; const workingDir = join(homedir(), 'codeman-cases', 'custom-model-restart'); +const projectsDir = join(homedir(), '.claude', 'projects', '-custom-model-restart'); const sessions: Session[] = []; afterEach(() => { for (const s of sessions.splice(0)) s.stop(); rmSync(workingDir, { recursive: true, force: true }); + // Only the directory these tests create. `setup.ts` gives each test file a + // temp HOME, but a wider sweep here would delete a real `~/.claude` the day + // that stops being true. + rmSync(projectsDir, { recursive: true, force: true }); }); +/** + * Write the transcript Claude would have written for a conversation. A pane + * `restartCli()` relaunches is a WORKING one, so its conversation has a + * transcript on disk; that is both what makes the bare `--session-id` collide + * and what the pin is now gated on. + */ +function giveTranscript(conversationId: string): void { + mkdirSync(projectsDir, { recursive: true }); + writeFileSync(join(projectsDir, `${conversationId}.jsonl`), '{"type":"user"}\n'); +} + function liveSession(mode: SessionMode, extra: Record = {}) { mkdirSync(workingDir, { recursive: true }); const mux = new TmuxManager(); @@ -118,6 +135,7 @@ describe('clearing a selection unsets what it injected', () => { describe('restartCli() must not kill a working pane', () => { it('claude: pins the live conversation id so the relaunch renders --resume || --session-id ', async () => { const { session, respawn } = liveSession('claude'); + giveTranscript(session.id); await session.restartCli(); const options = respawn.mock.calls[0][0]; expect(options.resumeSessionId).toBe(session.claudeSessionId); @@ -126,7 +144,33 @@ describe('restartCli() must not kill a working pane', () => { expect(session.toState().resumeSessionId).toBeUndefined(); }); + it('claude: pins nothing when the pane has no transcript, because nothing can collide', async () => { + // A pane that was launched and never prompted. `--session-id ` is + // accepted on an id no transcript holds, so pinning would buy nothing and + // cost two things: claude prints "No conversation found" into a pane with + // no history, and `wrapWithNice()` prefixes only the first branch of the + // rendered `a || b`, so the branch that actually runs loses its priority + // for the life of the session. + const { session, respawn } = liveSession('claude'); + await session.restartCli(); + expect(respawn.mock.calls[0][0].resumeSessionId).toBeUndefined(); + }); + it('claude: an explicit resume id from a resume-from-history launch wins over the pin', async () => { + const RESUMED = '01a060f0-0361-7f91-abde-b283020db0d7'; + const { session, respawn } = liveSession('claude', { resumeSessionId: RESUMED }); + giveTranscript(RESUMED); + await session.restartCli(); + expect(respawn.mock.calls[0][0].resumeSessionId).toBe(RESUMED); + }); + + it('claude: a launch seed survives the pin walk even with its transcript gone', async () => { + // The walk only ever ADDS a pin. The seed is what the session was created + // with and every respawn has always carried it, so a transcript deleted + // under a running session leaves the relaunch on + // `--resume || --session-id ` — the resume fails and the + // fallback runs, which is safe precisely because nothing is on disk to + // collide with. const RESUMED = '01a060f0-0361-7f91-abde-b283020db0d7'; const { session, respawn } = liveSession('claude', { resumeSessionId: RESUMED }); await session.restartCli();