BitSquare/Codeman
2
0
Fork 0
mirror of https://github.com/Ark0N/Codeman.git synced 2026-09-30 12:39:42 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(session): degrade the resume pin to the session id, not to nothing

Browse Source 
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 "<this.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) <noreply@anthropic.com>
This commit is contained in:
Michael Grundberg
2026-09-21 18:21:21 +02:00
co-authored by Claude Opus 5
parent 47e7935274
commit 1cb0441bd8
5 changed files with 256 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CLAUDE.md
+1 -1
View File
File diff suppressed because one or more lines are too long
src/session.ts
+56 -30
View File
@@ -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 <this.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 <id> || --session-id <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 <this.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 <this.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<import('./mux-interface.js').RespawnPaneOptions> {
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-<fragment>` 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-<fragment>` 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 <id> || --resume <id>`, 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 <this.id>` the
// unpinned options already carry is the correct command.
return options;
}
src/utils/claude-transcript.ts
+22 -4
View File
@@ -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 <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';
/** `<config dir>/projects`, honouring a session's relocated `CLAUDE_CONFIG_DIR`. */
/**
* `<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<boolean> {
if (!conversationId) return false;
test/respawn-session-id-collision.test.ts
+131 -8
View File
@@ -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 <pin> || --session-id <this.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 <this.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 <this.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;
}
});
});
test/session-custom-model-restart.test.ts
+46 -2
View File
@@ -12,7 +12,8 @@
* 2. Applying to a local claude session killed the pane: the relaunch was
* `claude --session-id <id>` and Claude refuses an id that already has a
* transcript, so it needs the `--resume <id> || --session-id <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<string, unknown> = {}) {
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 <id> || --session-id <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 <this.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 <seed> || --session-id <this.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();