diff --git a/src/main/claude/claude-transcript-conversation-name.test.ts b/src/main/claude/claude-transcript-conversation-name.test.ts index 3f5cf360c34..6c35a3fd204 100644 --- a/src/main/claude/claude-transcript-conversation-name.test.ts +++ b/src/main/claude/claude-transcript-conversation-name.test.ts @@ -3,6 +3,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { + claudeConversationNameReporterDeps, readClaudeTranscriptConversationName, reportPersistedClaudeConversationName } from './claude-transcript-conversation-name' @@ -69,6 +70,73 @@ describe('readClaudeTranscriptConversationName', () => { }) }) +describe('readClaudeTranscriptConversationName clearing', () => { + // These write REAL transcript records. The reporter tests below stub this + // reader, so they cannot see anything it decides — including whether an + // emptied custom title should fall back or wipe the name. + it('falls back to the generated title when the user empties their own', async () => { + const path = await transcript([ + { type: 'ai-title', aiTitle: 'Lease probe flake', sessionId: 'session-1' }, + { type: 'custom-title', customTitle: 'My own name', sessionId: 'session-1' }, + { type: 'custom-title', customTitle: '', sessionId: 'session-1' } + ]) + + // The CLI still shows the ai-title here; reporting `cleared` would wipe a + // name the user can see. + await expect(readClaudeTranscriptConversationName(path)).resolves.toEqual({ + kind: 'named', + title: 'Lease probe flake' + }) + }) + + it('reports cleared only when nothing remains to fall back to', async () => { + const path = await transcript([ + { type: 'custom-title', customTitle: 'My own name', sessionId: 'session-1' }, + { type: 'custom-title', customTitle: '', sessionId: 'session-1' } + ]) + + await expect(readClaudeTranscriptConversationName(path)).resolves.toEqual({ + kind: 'cleared' + }) + }) + + it.each([ + ['an absent field', { type: 'custom-title', sessionId: 'session-1' }], + ['a null field', { type: 'custom-title', customTitle: null, sessionId: 'session-1' }], + ['a renamed field', { type: 'custom-title', title: '', sessionId: 'session-1' }] + ])('refuses to read %s as a deliberate clear', async (_label, record) => { + const path = await transcript([ + { type: 'ai-title', aiTitle: 'Lease probe flake', sessionId: 'session-1' }, + record + ]) + + // Fails closed, like isCodexThreadReadablyUnnamed: a shape this build cannot + // read is not evidence the user removed anything. + await expect(readClaudeTranscriptConversationName(path)).resolves.toEqual({ + kind: 'named', + title: 'Lease probe flake' + }) + }) + + it('reports unknown, not cleared, for a tail with no title record at all', async () => { + const path = await transcript([{ type: 'user', sessionId: 'session-1' }]) + + await expect(readClaudeTranscriptConversationName(path)).resolves.toEqual({ kind: 'unknown' }) + }) + + it('still prefers a custom title the user actually set', async () => { + const path = await transcript([ + { type: 'ai-title', aiTitle: 'Lease probe flake', sessionId: 'session-1' }, + { type: 'custom-title', customTitle: 'My own name', sessionId: 'session-1' } + ]) + + await expect(readClaudeTranscriptConversationName(path)).resolves.toEqual({ + kind: 'named', + title: 'My own name' + }) + }) +}) + describe('reportPersistedClaudeConversationName', () => { const session = { providerSessionId: 'provider-1', claudeConfigDir: '/home/dev/.claude' } @@ -199,3 +267,32 @@ describe('reportPersistedClaudeConversationName', () => { expect(readTranscriptConversationName).not.toHaveBeenCalled() }) }) + +describe('claudeConversationNameReporterDeps', () => { + it('maps the adapter’s onNamingError onto the reporter’s onError', () => { + const onNamingError = vi.fn() + + // The adapter names this hook differently. Handing its object over whole + // worked only by accident and would break the day it is narrowed. + const mapped = claudeConversationNameReporterDeps({ onNamingError }) + mapped.onError?.('scope', new Error('boom')) + + expect(onNamingError).toHaveBeenCalledWith('scope', expect.any(Error)) + }) + + it('carries the read and both name hooks through', () => { + const readTranscriptConversationName = vi.fn(async () => ({ kind: 'unknown' as const })) + const onConversationName = vi.fn() + const onConversationNameCleared = vi.fn() + + const mapped = claudeConversationNameReporterDeps({ + readTranscriptConversationName, + onConversationName, + onConversationNameCleared + }) + + expect(mapped.readTranscriptConversationName).toBe(readTranscriptConversationName) + expect(mapped.onConversationName).toBe(onConversationName) + expect(mapped.onConversationNameCleared).toBe(onConversationNameCleared) + }) +}) diff --git a/src/main/claude/claude-transcript-conversation-name.ts b/src/main/claude/claude-transcript-conversation-name.ts index 673a96bc3b8..16e8265b40c 100644 --- a/src/main/claude/claude-transcript-conversation-name.ts +++ b/src/main/claude/claude-transcript-conversation-name.ts @@ -8,8 +8,12 @@ // with the attach it runs alongside, on files that reach many megabytes. // // Bounded means a title older than the tail limit is NOT found. That reads as -// "no name yet", never as "this conversation has no name" — and once any read -// succeeds the name is on the durable record, so the scan is not repeated. +// "no name yet", never as "this conversation has no name". +// +// This runs on EVERY acquisition, deliberately: it is how a rename made in the +// CLI reaches Orca at all, so gating it on "already named" would freeze the +// first name forever. It both fills and clears, and clearing an already-cleared +// record is a no-op, so the repeat costs a bounded read and nothing else. import { normalizeTitleText, parseJsonObject } from '../ai-vault/session-scanner-values' import { claudeTranscriptTailLines } from './claude-transcript-tail-scan' @@ -27,14 +31,21 @@ export type ClaudeTranscriptConversationName = * The transcript's stored name. * * A user's own `custom-title` outranks the generated `ai-title`, matching the - * precedence the CLI itself applies. Read newest-first, so the first record of - * each kind is the current one — and an emptied `custom-title` is the user - * deleting the name, which must not read the same as never having had one. + * precedence the CLI itself applies — including when the custom slot is EMPTY, + * which falls back to the generated name rather than reading as no name at all. + * Only an emptied custom slot with nothing to fall back to is a clear. + * + * Read newest-first, so the first record of each type is the current one. + * + * Fails closed on a shape it cannot read: a `custom-title` whose field is absent, + * null, or renamed by a future CLI is skipped, never taken as a deliberate clear. + * Wrongly clearing destroys a name the user can still see in their CLI. */ export async function readClaudeTranscriptConversationName( transcriptPath: string ): Promise { let generated: string | null = null + let customCleared = false for await (const line of claudeTranscriptTailLines(transcriptPath)) { if (!line.includes('-title')) { continue @@ -43,15 +54,49 @@ export async function readClaudeTranscriptConversationName( if (!record) { continue } - if (record.type === 'custom-title') { - const title = normalizeTitleText(String(record.customTitle ?? '')) - return title ? { kind: 'named', title } : { kind: 'cleared' } + if (record.type === 'custom-title' && !customCleared) { + if (typeof record.customTitle !== 'string') { + continue + } + const title = normalizeTitleText(record.customTitle) + if (title) { + return { kind: 'named', title } + } + customCleared = true + continue } if (record.type === 'ai-title' && !generated) { generated = normalizeTitleText(String(record.aiTitle ?? '')) || null } } - return generated ? { kind: 'named', title: generated } : { kind: 'unknown' } + if (generated) { + return { kind: 'named', title: generated } + } + return customCleared ? { kind: 'cleared' } : { kind: 'unknown' } +} + +/** The adapter's own dep bag, which names this reporter's error hook differently. + * Mapped rather than passed by reference: an undeclared key survives only while + * the object happens to be handed over whole, and typecheck cannot see it go. */ +export type ClaudeConversationNameReporterSource = ClaudeConversationNameReporter & { + onNamingError?: (scope: string, error: unknown) => void +} + +export function claudeConversationNameReporterDeps( + source: ClaudeConversationNameReporterSource +): ClaudeConversationNameReporter { + return { + ...(source.readTranscriptConversationName + ? { readTranscriptConversationName: source.readTranscriptConversationName } + : {}), + ...(source.onConversationName ? { onConversationName: source.onConversationName } : {}), + ...(source.onConversationNameCleared + ? { onConversationNameCleared: source.onConversationNameCleared } + : {}), + ...((source.onError ?? source.onNamingError) + ? { onError: (source.onError ?? source.onNamingError)! } + : {}) + } } export type ClaudeConversationNameReporter = { @@ -75,8 +120,9 @@ export function reportPersistedClaudeConversationName( session: | { providerSessionId: string; claudeConfigDir: string; namingAttempted?: boolean } | undefined, - deps: ClaudeConversationNameReporter + source: ClaudeConversationNameReporterSource ): void { + const deps = claudeConversationNameReporterDeps(source) const read = deps.readTranscriptConversationName if (!session || !read || !deps.onConversationName) { return diff --git a/src/main/codex/codex-structured-session-conversation-name.test.ts b/src/main/codex/codex-structured-session-conversation-name.test.ts index c2a184fde07..4bc53bd9f6b 100644 --- a/src/main/codex/codex-structured-session-conversation-name.test.ts +++ b/src/main/codex/codex-structured-session-conversation-name.test.ts @@ -467,19 +467,20 @@ describe('Codex sub-agent threads survive the naming window', () => { codex.connections[0]!.handlers.onServerRequest?.({ id: 91, method: 'item/commandExecution/requestApproval', - params: { threadId: SUBAGENT_THREAD, command: 'pnpm test' } + // `itemId` is what makes this a durable PROMPT rather than a request the + // registry declines; without it the assertion below would pass on a + // different refusal and prove nothing about reaching the user. + params: { threadId: SUBAGENT_THREAD, itemId: 'item-subagent-1', command: 'pnpm test' } }) await settle() - // Auto-refusing here denies a tool the user's OWN agent asked to run, with a - // reason that is untrue. Asserted on the MESSAGE: -32001 is also the code the - // normal prompt path uses when a journal admission fails, so the code alone - // would not tell the two apart. - const refusals = codex.replies.filter((reply) => - String(reply.message ?? '').includes('conversation-naming turn') - ) - expect(refusals).toEqual([]) - expect(emittedThreads(events)).toContain(SUBAGENT_THREAD) + expect(codex.replies).toEqual([]) + // It reaches the user as a prompt, which is the behaviour the narrowed gate + // restored — not merely "some event was emitted". + const prompts = events.filter((event) => (event as { type?: string }).type === 'prompt') + expect(prompts).toEqual([ + expect.objectContaining({ threadId: SUBAGENT_THREAD, codexItemId: 'item-subagent-1' }) + ]) }) it('still keeps the naming thread out once its id is known', async () => { diff --git a/src/main/runtime/structured-claude-runtime-adapter.ts b/src/main/runtime/structured-claude-runtime-adapter.ts index 12bc2c0fb0c..d6ef9c3e964 100644 --- a/src/main/runtime/structured-claude-runtime-adapter.ts +++ b/src/main/runtime/structured-claude-runtime-adapter.ts @@ -93,7 +93,6 @@ export function createStructuredClaudeRuntimeAdapter( ...(deps.onConversationNameCleared ? { onConversationNameCleared: deps.onConversationNameCleared } : {}), - ...(deps.onNamingError ? { onError: deps.onNamingError } : {}), readTranscriptConversationName: async ({ providerSessionId, claudeConfigDir }) => { const transcriptPath = await resolveSessionFilePath('claude', providerSessionId, { claudeProjectsDir: join(claudeConfigDir, 'projects')