diff --git a/bridge/CLAUDE.md b/bridge/CLAUDE.md index 51f21aa..051635a 100644 --- a/bridge/CLAUDE.md +++ b/bridge/CLAUDE.md @@ -34,6 +34,19 @@ session's checkout — see `headlessScratchCwd`. History a CLI offers no switch skip is redirected per spawn instead (`HeadlessCommand.scratchEnv`, a fresh dir the runner deletes after), so never list a var that also carries credentials. +**An agent's native session id is not stable across a resume**, so nothing may +read a change of it as "a new conversation started". Measured on Claude Code: +`--resume` copies the transcript into a NEW file and appends under a fresh id, +so the same thread comes back wearing a different name. Every guard against +re-naming a session is otherwise per-run — `SessionNamer` and `TitleAttempts` +both die with the PTY — which is why the winning signal is also written to the +session row as `autoTitleRank`, and why `SessionManager.noteConversationStart` +records AT LAUNCH that a run continues the previous conversation. The first +identity report spends that claim; every rotation after it is a real `/clear`. +Get either half wrong and a stop/start pays another model spawn and renames the +session — and not back to the same title, since the transcript read returns the +LAST few messages, so the new name describes wherever the work had drifted to. + The `agent:tools` advert (and the loopback `tools:list` reply) carries TWO arrays, and the split is load-bearing. `tools[]` is the PATH probe — what this machine can actually launch. `agents[]` (`agent-catalog.ts`, projected from the diff --git a/bridge/src/agent-core.ts b/bridge/src/agent-core.ts index 4f8aa8d..c4d65af 100644 --- a/bridge/src/agent-core.ts +++ b/bridge/src/agent-core.ts @@ -2589,7 +2589,7 @@ export async function buildAgentCore(opts: BuildAgentCoreOptions): Promise sessions?.applyAutoName(id, name), + applyAutoName: (id, name, rank) => sessions?.applyAutoName(id, name, rank), }); // agy fires no hook on a `/rename`, so it would not reach the sidebar until @@ -2909,6 +2909,12 @@ export async function buildAgentCore(opts: BuildAgentCoreOptions): Promise(); + /** Sessions launched to CONTINUE their previous conversation, until the agent + * reports the identity it continued under. Per-run and therefore in memory: + * a launch always precedes the report, and a bridge that died between them + * killed the run too. See noteConversationStart. */ + private readonly awaitingResumedIdentity = new Set(); /** Live and recovered `worktree.setup` state, keyed by session id. Runtime * only — it reaches the wire through toWire and never sessions.json, which * is also why every mutation here emits with notifyObservers() rather than @@ -911,6 +932,10 @@ export class SessionManager { if (!trimmed) throw new Error("name cannot be empty"); entry.name = trimmed; entry.manuallyRenamed = true; + // The name is the user's now, so it no longer describes any signal. Kept + // honest rather than load-bearing: manuallyRenamed already refuses every + // auto-name, and a rank left behind would outlive the title it described. + entry.autoTitleRank = undefined; this.changed(); } @@ -932,15 +957,54 @@ export class SessionManager { return !!entry && !entry.manuallyRenamed; } - applyAutoName(id: string, name: string): void { + /** + * Whether this slot already holds a name no model call should try to improve + * on. The DURABLE twin of SessionNamer.hasFinalTitle, which holds nothing once + * the PTY has exited — so this is the only thing that still knows, after a + * restart, that the name on the row was generated rather than defaulted. + */ + hasFinalAutoTitle(id: string): boolean { + const entry = this.entries.get(id); + return titleRankValue(entry?.autoTitleRank) >= titleRankValue("self"); + } + + applyAutoName(id: string, name: string, rank?: TitleRank): void { const entry = this.entries.get(id); if (!entry || entry.manuallyRenamed) return; + // Precedence, restated here because SessionNamer's copy only spans one run: + // the first-message read repeats the SAME opening prompt every turn and the + // OSC signal is terminal chrome, so after a restart either would rename a + // session we had already titled back to something worse. + if (titleRankValue(rank) < titleRankValue(entry.autoTitleRank)) return; const trimmed = name.trim(); - if (!trimmed || trimmed === entry.name) return; + if (!trimmed) return; + // The rank travels with the name it describes, so an unchanged name still + // writes when the signal behind it strengthened. + if (trimmed === entry.name && rank === entry.autoTitleRank) return; entry.name = trimmed; + entry.autoTitleRank = rank; this.changed(); } + /** + * Record whether this launch continues the slot's previous conversation, and + * release the generated title when it does not. + * + * A continuation cannot be recognized from the agent's session id afterwards: + * measured on Claude Code, `--resume` copies the transcript into a new file + * and appends under a FRESH id, so the same thread comes back wearing a + * different name. Only the launch knows, which is why it is recorded here and + * consumed by the first report in setAgentSession. + */ + private noteConversationStart(entry: PersistedEntry, resumed: boolean): void { + if (resumed) { + this.awaitingResumedIdentity.add(entry.id); + return; + } + this.awaitingResumedIdentity.delete(entry.id); + entry.autoTitleRank = undefined; + } + /** * Record the agent's last-active native conversation id for a slot * (overwrite-latest). Called from the /session-title pipeline every turn. @@ -950,6 +1014,15 @@ export class SessionManager { setAgentSession(id: string, agentSessionId: string, agentTranscriptPath?: string): void { const entry = this.entries.get(id); if (!entry) return; + // A conversation change releases the slot's title — the name describes what + // was being worked on, not the slot. Ordered BEFORE the unchanged early-out + // below so a resume that comes back under the SAME id still consumes the + // claim; left armed, the `/clear` after it would read as that resume. + if (!this.awaitingResumedIdentity.delete(id) + && entry.agentSessionId !== undefined + && entry.agentSessionId !== agentSessionId) { + entry.autoTitleRank = undefined; + } // Keep a previously captured path if this report omits one for the SAME // session — an agent's SessionStart can fire before the transcript path is // known, then a later turn-end report supplies it (antigravity's @@ -1076,6 +1149,7 @@ export class SessionManager { this.tm.forget(id); this.entries.delete(id); this.resumableCache.delete(id); + this.awaitingResumedIdentity.delete(id); this.setups.delete(id); } @@ -1787,10 +1861,12 @@ export class SessionManager { const chatAlreadyRunning = this.runningChat.has(id); this.runningChat.add(id); const chatTool = entry.tool ?? "codex"; + const resumeId = entry.conversationStart === "fork" ? undefined : this.resumeIdFor(chatTool, entry); + this.noteConversationStart(entry, resumeId !== undefined); this.opts.onStartChat?.({ sessionId: id, tool: chatTool, - resumeId: entry.conversationStart === "fork" ? undefined : this.resumeIdFor(chatTool, entry), + resumeId, config: entry.config, initialPrompt: this.forkInitialPrompt(entry, initialPrompt), }); @@ -1880,6 +1956,9 @@ export class SessionManager { // than spawning a default shell. throw new Error("agent.tool or agent.command not configured"); } + // After the throw: a launch that never happened has started no conversation + // and must not release the title of the one still recorded here. + this.noteConversationStart(entry, resumeArgs.length > 0); // One-shot first prompt, appended LAST (after resume tokens — codex's // `resume ` is a subcommand and the positional prompt must follow it). diff --git a/bridge/src/session-namer.ts b/bridge/src/session-namer.ts index 50d436a..728874d 100644 --- a/bridge/src/session-namer.ts +++ b/bridge/src/session-namer.ts @@ -1,7 +1,10 @@ import type { ResolvedTitle } from "./agents/types"; export interface AutoNameSink { - applyAutoName(id: string, name: string): void; + /** The signal the name came from, absent for the OSC fallback. Passed on so + * the sink can apply the same precedence DURABLY: this class holds nothing + * after the PTY exits, and the name outlives it on the session entry. */ + applyAutoName(id: string, name: string, rank?: TitleRank): void; } const MAX_TITLE_LEN = 60; @@ -39,6 +42,17 @@ const RANK_ORDER: Record = { manual: 2, }; +/** Every rank, for the persisted schema that has to name them. Derived from + * RANK_ORDER rather than restated, because `Record` is what + * makes forgetting a new rank a build error. */ +export const TITLE_RANKS = Object.keys(RANK_ORDER) as [TitleRank, ...TitleRank[]]; + +/** Comparable strength of a title signal. The OSC fallback carries no rank and + * ranks below every structured one — it is terminal chrome, not a title. */ +export function titleRankValue(rank?: TitleRank): number { + return rank ? RANK_ORDER[rank] : -1; +} + interface Signals { /** Sanitized at ingest, so `title` is exactly the name that will be applied. * Title and rank travel together: a rank stored without the title it @@ -82,7 +96,7 @@ export class SessionNamer { */ onStructuredTitle(id: string, title: string, rank: TitleRank): void { const held = this.signals.get(id)?.structured; - if (held && RANK_ORDER[rank] < RANK_ORDER[held.rank]) return; + if (held && titleRankValue(rank) < titleRankValue(held.rank)) return; // Normalize at ingest, so the stored title IS the name that will be applied // and `flush` never has to re-derive it. One that sanitizes away applies // nothing, and latching the slot on it would block every later title on @@ -105,7 +119,7 @@ export class SessionNamer { */ hasFinalTitle(id: string): boolean { const held = this.signals.get(id)?.structured; - return !!held && RANK_ORDER[held.rank] >= RANK_ORDER.self; + return titleRankValue(held?.rank) >= titleRankValue("self"); } onOscTitle(id: string, title: string): void { @@ -118,7 +132,7 @@ export class SessionNamer { const s = this.signals.get(id); if (!s) continue; const name = s.structured?.title ?? sanitizeTitle(s.osc ?? ""); - if (name) this.sink.applyAutoName(id, name); + if (name) this.sink.applyAutoName(id, name, s.structured?.rank); } this.dirty.clear(); } diff --git a/bridge/tests/session-manager-autoname.test.ts b/bridge/tests/session-manager-autoname.test.ts index 5c59d7e..fda262a 100644 --- a/bridge/tests/session-manager-autoname.test.ts +++ b/bridge/tests/session-manager-autoname.test.ts @@ -91,3 +91,140 @@ describe("applyAutoName / manuallyRenamed", () => { expect(sm.get("b")!.name).toBe("Hand named"); }); }); + +// The bug: every guard against re-naming a session lived in memory, so a +// stop/start renamed a session we had already titled — and not back to the same +// title, because the transcript read returns the LAST few messages, so the new +// name described wherever the conversation had drifted to. +describe("a generated title survives a restart", () => { + const TP = "tx.jsonl"; + + /** A claude-code session whose conversation is resumable: `start()` resumes it + * only while the transcript it names still exists. */ + function resumableSession(sm: SessionManager, store: string) { + const s = sm.create(undefined, { tool: "claude-code" }); + const path = join(store, TP); + writeFileSync(path, "{}"); + sm.setAgentSession(s.id, "sess-1", path); + sm.applyAutoName(s.id, "Fix S3 retry backoff", "self"); + return s; + } + + test("only a name worth protecting is final", () => { + const sm = mk(newStore()); + const s = sm.create(); + expect(sm.hasFinalAutoTitle(s.id)).toBe(false); + sm.applyAutoName(s.id, "open this file for me", "first-message"); + expect(sm.hasFinalAutoTitle(s.id)).toBe(false); + sm.applyAutoName(s.id, "Fix S3 retry backoff", "self"); + expect(sm.hasFinalAutoTitle(s.id)).toBe(true); + }); + + test("the rank is written beside the name and reloads with it", () => { + const store = newStore(); + const sm = mk(store); + const s = sm.create(); + sm.applyAutoName(s.id, "Fix S3 retry backoff", "self"); + sm.flushNow(); + + const reloaded = mk(store); + expect(reloaded.get(s.id)!.name).toBe("Fix S3 retry backoff"); + expect(reloaded.hasFinalAutoTitle(s.id)).toBe(true); + }); + + // The rename a restart used to perform BEFORE the model spawn: the native read + // repeats the opening prompt on every turn, and after a reload nothing in + // memory ranked it below the name it was about to replace. + test("the opening prompt cannot displace it after a reload", () => { + const store = newStore(); + const sm = mk(store); + const s = sm.create(); + sm.applyAutoName(s.id, "Fix S3 retry backoff", "self"); + sm.flushNow(); + + const reloaded = mk(store); + reloaded.applyAutoName(s.id, "hey can you look at the retry code", "first-message"); + reloaded.applyAutoName(s.id, "bash - claude", undefined); // OSC chrome + expect(reloaded.get(s.id)!.name).toBe("Fix S3 retry backoff"); + }); + + test("a rename typed at the agent does displace it, and then holds", () => { + const sm = mk(newStore()); + const s = sm.create(); + sm.applyAutoName(s.id, "Fix S3 retry backoff", "self"); + sm.applyAutoName(s.id, "Release blockers", "manual"); + expect(sm.get(s.id)!.name).toBe("Release blockers"); + sm.applyAutoName(s.id, "Fix S3 retry backoff", "self"); + expect(sm.get(s.id)!.name).toBe("Release blockers"); + }); + + test("a manual rename releases the rank along with the name", () => { + const sm = mk(newStore()); + const s = sm.create(); + sm.applyAutoName(s.id, "Fix S3 retry backoff", "self"); + sm.rename(s.id, "Mine"); + expect(sm.hasFinalAutoTitle(s.id)).toBe(false); + }); + + // Claude's --resume copies the transcript into a new file and appends under a + // FRESH id, so the same thread comes back under a different name. Reading that + // rotation as a new conversation is what spent a model call on every restart. + test("a resumed launch keeps the title through the id it comes back under", () => { + const store = newStore(); + const sm = mk(store); + const s = resumableSession(sm, store); + + sm.start(s.id); + sm.setAgentSession(s.id, "sess-2", join(store, TP)); + expect(sm.hasFinalAutoTitle(s.id)).toBe(true); + expect(sm.get(s.id)!.name).toBe("Fix S3 retry backoff"); + }); + + test("a launch that resumes nothing starts a conversation the name is not about", () => { + const store = newStore(); + const sm = mk(store); + const s = sm.create(undefined, { tool: "claude-code" }); + sm.setAgentSession(s.id, "sess-dead", "/no/such/file.jsonl"); + sm.applyAutoName(s.id, "Fix S3 retry backoff", "self"); + + sm.start(s.id); // transcript gone → no resume argv → fresh conversation + expect(sm.hasFinalAutoTitle(s.id)).toBe(false); + }); + + test("a new conversation under the running agent releases it", () => { + const store = newStore(); + const sm = mk(store); + const s = resumableSession(sm, store); + + sm.start(s.id); + sm.setAgentSession(s.id, "sess-2", join(store, TP)); // the resume's own id + sm.setAgentSession(s.id, "sess-3", join(store, TP)); // `/clear` + expect(sm.hasFinalAutoTitle(s.id)).toBe(false); + }); + + // An agent whose resume keeps the id reports one that matches. The claim has + // to be spent on that report all the same, or the NEXT rotation — a real + // `/clear` — is mistaken for this launch's resume and keeps a stale name. + test("a resume reporting the same id still spends the claim", () => { + const store = newStore(); + const sm = mk(store); + const s = resumableSession(sm, store); + + sm.start(s.id); + sm.setAgentSession(s.id, "sess-1", join(store, TP)); + sm.setAgentSession(s.id, "sess-2", join(store, TP)); + expect(sm.hasFinalAutoTitle(s.id)).toBe(false); + }); + + // A row written before the rank existed cannot say which signal named it, and + // guessing "generated" would freeze it against ever being named properly. + test("a legacy row is not treated as already titled", () => { + const store = newStore(); + const dir = join(store, "agents", "p1"); mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, "sessions.json"), JSON.stringify({ + version: 1, + sessions: [{ id: "a", name: "Session 3", createdAt: 1, lastUsedAt: 1, archived: false }], + })); + expect(mk(store).hasFinalAutoTitle("a")).toBe(false); + }); +}); diff --git a/bridge/tests/session-namer.test.ts b/bridge/tests/session-namer.test.ts index ec53976..a82f395 100644 --- a/bridge/tests/session-namer.test.ts +++ b/bridge/tests/session-namer.test.ts @@ -227,3 +227,32 @@ describe("SessionNamer", () => { expect(s.calls).toEqual([["a", "First name"], ["a", "Second name"]]); }); }); + +// The rank has to reach the sink, because the sink is the only half of this +// precedence that outlives the PTY: SessionManager writes it beside the name so +// a restarted session still knows which signal named it. +describe("the rank travels with the name", () => { + function rankedSink() { + const calls: Array<[string, string, string | undefined]> = []; + return { + calls, + applyAutoName: (id: string, name: string, rank?: string) => calls.push([id, name, rank]), + }; + } + + test("a structured title is applied with the rank it was ingested at", () => { + const s = rankedSink(); + const n = new SessionNamer(s, { debounceMs: 0 }); + n.onStructuredTitle("a", "Fix session auto-naming", "self"); + n.flush(); + expect(s.calls).toEqual([["a", "Fix session auto-naming", "self"]]); + }); + + test("the OSC fallback carries none — it is terminal chrome, not a title", () => { + const s = rankedSink(); + const n = new SessionNamer(s, { debounceMs: 0 }); + n.onOscTitle("a", "osc filler"); + n.flush(); + expect(s.calls).toEqual([["a", "osc filler", undefined]]); + }); +});