From 25b5fac68a7deee2c445dc6db6592d234be6703d Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 10 Sep 2026 19:15:44 -0700 Subject: [PATCH] feat(native-chat): record the provider's name for an agent session (#19908) * refactor(ai-vault): move the surrogate-safe slice to shared, one implementation `sliceAtCodeUnitLimit` lived in src/main/ai-vault, and src/shared never imports src/main, so a shared consumer could not reach it. Rather than add a second copy, it moves to src/shared and ai-vault imports and re-exports it, leaving every existing importer of that module untouched. Separated from the feature that needs it: this is the only change here to a subsystem the rest of the branch does not touch. * feat(native-chat): give an agent session one place to hold its name Adds the normalized conversation name and the record field that stores it, so Orca has a durable note that it already named a session and does not name it again on a later acquisition. No name is generated yet, and nothing displays this field: the AI Vault path stays the home for the name a user sees. - `agent-session-conversation-name` bounds and flattens the text once, at 200 characters, cutting on a character boundary. - `AgentSessionRecord.conversationName` carries it, validated on load. - `setAgentSessionRecordConversationName` sets or clears it, unfenced: the name is a durable note, not ownership, so writing it never contends with the writer lease. * refactor(runtime): move reserve-owner orchestration next to its admission logic Pure move, no behavior change. `reserveOwner`'s transaction body sequenced decisions that all live in agent-session-reservation-admission and then applied the winning one; it now sits there as `commitAgentSessionReservation`, and the store keeps the transaction boundary and a one-line delegation. Takes agent-session-record-store.ts from 300/300 to 279/300, which is what the conversation-name field needs to land. Every existing test passes unedited. The module header said nothing in it mutates; that is now qualified rather than left false, since the committer writes the state it is handed. * feat(runtime): let the store set a session's conversation name `setConversationName` is the one writer for the record field, so a producer never reaches the record shape itself. It normalizes at the boundary, so no caller can persist a name the loader would then reject as unreadable. Unfenced by design: the name is durable memory that Orca already named this session, not ownership, so naming never contends with the writer lease. No name generation and no provider code yet. * refactor(runtime): reuse the shared default chat label on a replacement tab The replacement tab open-coded `'Claude Chat' : 'Codex Chat'` while every other publisher already calls `defaultAgentChatLabel`. One writer for the placeholder. * fix(native-chat): reject noncanonical stored conversation names --------- Co-authored-by: Merge Sim --- .../session-scanner-text-normalization.ts | 13 +- ...nt-session-conversation-name-store.test.ts | 104 +++++++++++ ...t-session-record-conversation-name.test.ts | 129 ++++++++++++++ .../agent-session-record-conversation-name.ts | 27 +++ .../runtime/agent-session-record-store.ts | 41 ++--- .../agent-session-reservation-admission.ts | 37 +++- ...structured-conversation-tab-replacement.ts | 3 +- .../agent-session-conversation-name.test.ts | 162 ++++++++++++++++++ src/shared/agent-session-conversation-name.ts | 67 ++++++++ src/shared/agent-session-record.ts | 5 + src/shared/surrogate-safe-text-slice.ts | 19 ++ 11 files changed, 566 insertions(+), 41 deletions(-) create mode 100644 src/main/runtime/agent-session-conversation-name-store.test.ts create mode 100644 src/main/runtime/agent-session-record-conversation-name.test.ts create mode 100644 src/main/runtime/agent-session-record-conversation-name.ts create mode 100644 src/shared/agent-session-conversation-name.test.ts create mode 100644 src/shared/agent-session-conversation-name.ts create mode 100644 src/shared/surrogate-safe-text-slice.ts diff --git a/src/main/ai-vault/session-scanner-text-normalization.ts b/src/main/ai-vault/session-scanner-text-normalization.ts index 99f73a93d02..50fc54ab7aa 100644 --- a/src/main/ai-vault/session-scanner-text-normalization.ts +++ b/src/main/ai-vault/session-scanner-text-normalization.ts @@ -1,3 +1,7 @@ +import { sliceAtCodeUnitLimit } from '../../shared/surrogate-safe-text-slice' + +export { sliceAtCodeUnitLimit } + const SESSION_TITLE_TEXT_LIMIT = 96 const SESSION_PREVIEW_TEXT_LIMIT = 220 const ELLIPSIS = '...' @@ -42,15 +46,6 @@ export function normalizePreviewText(value: string): string | null { return finalizeNormalizedText(normalizeStringText(value, SESSION_PREVIEW_TEXT_LIMIT)) } -/** Cut to `limit` UTF-16 code units without splitting a trailing surrogate pair. */ -export function sliceAtCodeUnitLimit(value: string, limit: number): string { - if (value.length <= limit) { - return value - } - const end = limit > 0 && isHighSurrogate(value.charCodeAt(limit - 1)) ? limit - 1 : limit - return value.slice(0, end) -} - function normalizeContentText(value: unknown, limit: number): string | null { if (typeof value === 'string') { return finalizeNormalizedText(normalizeStringText(value, limit)) diff --git a/src/main/runtime/agent-session-conversation-name-store.test.ts b/src/main/runtime/agent-session-conversation-name-store.test.ts new file mode 100644 index 00000000000..ce5d3fd2621 --- /dev/null +++ b/src/main/runtime/agent-session-conversation-name-store.test.ts @@ -0,0 +1,104 @@ +// The name is durable state on the record: the store is the only thing that writes it. +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import type { AgentSessionExecutionLocation } from '../../shared/agent-session-record' +import { AgentSessionRecordStore } from './agent-session-record-store' +import type { AgentSessionReserveRequest } from './agent-session-reservation-admission' + +const NOW = 1_800_000_000_000 +const SESSION = 'session-alpha' +const NATIVE: AgentSessionExecutionLocation = { + executionHostId: 'local', + wslDistro: null, + workspaceId: 'workspace-1', + workspaceKind: 'git-worktree' +} + +let counter = 0 +/** Same shape the store's own suite uses: `-<32 hex>`. */ +function operationId(): string { + counter += 1 + return `${NOW}-${String(counter) + .padStart(32, '0') + .replaceAll(/[^0-9a-f]/g, '0')}` +} + +const reserveRequest = (): AgentSessionReserveRequest => ({ + sessionId: SESSION, + location: NATIVE, + provider: 'claude', + accountHome: { variable: 'CLAUDE_CONFIG_DIR', path: '/home/dev/.claude-work' }, + runtimeKind: 'native', + expectedFence: null, + spawnToken: 'spawn-a', + claimKeyId: 'key-1', + handoffOperationId: null, + probe: { outcome: 'indeterminate', reason: 'no answer' }, + operation: { callerKey: 'client-1', operationId: operationId(), fingerprint: 'fp-1' }, + now: NOW +}) + +let directory: string + +beforeEach(async () => { + directory = await mkdtemp(join(tmpdir(), 'orca-conversation-name-store-')) +}) +afterEach(async () => { + await rm(directory, { recursive: true, force: true }) +}) + +async function reservedStore(): Promise { + const store = await AgentSessionRecordStore.open({ directory, hostId: 'local' }) + await store.reserveOwner(reserveRequest()) + return store +} + +describe('AgentSessionRecordStore.setConversationName', () => { + it('stores the name and survives a reload, so the record is where it lives', async () => { + const store = await reservedStore() + + await store.setConversationName(SESSION, 'Fix the lease probe') + + const reloaded = await AgentSessionRecordStore.open({ directory, hostId: 'local' }) + expect(reloaded.getRecord(SESSION)?.conversationName).toBe('Fix the lease probe') + }) + + it('normalizes at the boundary, so no caller can persist an invalid record', async () => { + const store = await reservedStore() + + await store.setConversationName(SESSION, `Fix\nthe ${'x'.repeat(400)}`) + + const name = store.getRecord(SESSION)?.conversationName ?? '' + expect(name).toHaveLength(200) + expect(name.startsWith('Fix the ')).toBe(true) + // A reload validates every record; an over-long name would be dropped as unreadable. + const reloaded = await AgentSessionRecordStore.open({ directory, hostId: 'local' }) + expect(reloaded.getRecord(SESSION)?.conversationName).toBe(name) + }) + + it('clears the name with null', async () => { + const store = await reservedStore() + await store.setConversationName(SESSION, 'Fix the lease probe') + + await store.setConversationName(SESSION, null) + + expect(store.getRecord(SESSION)?.conversationName).toBeUndefined() + }) + + it('does not need the lease: an unfenced rename never contends with the writer', async () => { + const store = await reservedStore() + + // No fence argument exists to pass, and no fence error is raised. + await expect(store.setConversationName(SESSION, 'Fix the lease probe')).resolves.toBeDefined() + }) + + it('refuses a session it has no record for', async () => { + const store = await reservedStore() + + await expect(store.setConversationName('missing', 'A name')).rejects.toThrow( + 'agent_session_identity_required' + ) + }) +}) diff --git a/src/main/runtime/agent-session-record-conversation-name.test.ts b/src/main/runtime/agent-session-record-conversation-name.test.ts new file mode 100644 index 00000000000..3c5c55d83c6 --- /dev/null +++ b/src/main/runtime/agent-session-record-conversation-name.test.ts @@ -0,0 +1,129 @@ +import { describe, expect, it } from 'vitest' +import { isAgentSessionRecord } from '../../shared/agent-session-record' +import { agentSessionRecordFixture } from '../../shared/agent-session-record.test-fixture' +import { setAgentSessionRecordConversationName } from './agent-session-record-conversation-name' + +const NOW = 9_000 + +describe('agent session record conversationName validation', () => { + it('accepts a record carrying a bounded name', () => { + expect( + isAgentSessionRecord({ + ...agentSessionRecordFixture(), + conversationName: 'Fix the lease probe' + }) + ).toBe(true) + }) + + it('accepts a record with no name at all', () => { + expect(isAgentSessionRecord(agentSessionRecordFixture())).toBe(true) + }) + + it('rejects a name past the stored maximum', () => { + expect( + isAgentSessionRecord({ ...agentSessionRecordFixture(), conversationName: 'a'.repeat(201) }) + ).toBe(false) + }) + + it('rejects a name that is not a string', () => { + expect(isAgentSessionRecord({ ...agentSessionRecordFixture(), conversationName: 42 })).toBe( + false + ) + expect(isAgentSessionRecord({ ...agentSessionRecordFixture(), conversationName: '' })).toBe( + false + ) + }) + + it('rejects persisted names that bypassed canonical normalization', () => { + expect( + isAgentSessionRecord({ + ...agentSessionRecordFixture(), + conversationName: 'Fix\u202Egnp.exe probe' + }) + ).toBe(false) + expect( + isAgentSessionRecord({ ...agentSessionRecordFixture(), conversationName: 'Fix\nthe probe' }) + ).toBe(false) + }) +}) + +describe('setAgentSessionRecordConversationName', () => { + it('sets the name and stamps the update', () => { + const next = setAgentSessionRecordConversationName( + agentSessionRecordFixture(), + 'Fix the lease probe', + NOW + ) + + expect(next.conversationName).toBe('Fix the lease probe') + expect(next.updatedAt).toBe(NOW) + expect(isAgentSessionRecord(next)).toBe(true) + }) + + it('normalizes on the way in so the record stays valid whatever the caller sent', () => { + const next = setAgentSessionRecordConversationName( + agentSessionRecordFixture(), + `Fix\nthe probe`, + NOW + ) + + expect(next.conversationName).toBe('Fix the probe') + expect(isAgentSessionRecord(next)).toBe(true) + }) + + it('bounds an over-long name rather than storing a record the validator would reject', () => { + const next = setAgentSessionRecordConversationName( + agentSessionRecordFixture(), + 'a'.repeat(1000), + NOW + ) + + expect(next.conversationName).toHaveLength(200) + expect(isAgentSessionRecord(next)).toBe(true) + }) + + it('clears the name via null, deleting the key rather than storing an empty string', () => { + const named = setAgentSessionRecordConversationName( + agentSessionRecordFixture(), + 'Fix the probe', + NOW + ) + + const cleared = setAgentSessionRecordConversationName(named, null, NOW + 1) + + expect(Object.hasOwn(cleared, 'conversationName')).toBe(false) + expect(cleared.updatedAt).toBe(NOW + 1) + expect(isAgentSessionRecord(cleared)).toBe(true) + }) + + it('treats a name that normalizes to nothing as a clear', () => { + const named = setAgentSessionRecordConversationName( + agentSessionRecordFixture(), + 'Fix the probe', + NOW + ) + + expect( + Object.hasOwn( + setAgentSessionRecordConversationName(named, ' ', NOW + 1), + 'conversationName' + ) + ).toBe(false) + }) + + it('returns the same object when the name is unchanged, so no write is provoked', () => { + const named = setAgentSessionRecordConversationName( + agentSessionRecordFixture(), + 'Fix the probe', + NOW + ) + + expect(setAgentSessionRecordConversationName(named, 'Fix the probe', NOW + 1)).toBe(named) + }) + + it('returns the same object when clearing a record that has no name', () => { + const record = agentSessionRecordFixture() + + expect(setAgentSessionRecordConversationName(record, null, NOW)).toBe(record) + }) +}) diff --git a/src/main/runtime/agent-session-record-conversation-name.ts b/src/main/runtime/agent-session-record-conversation-name.ts new file mode 100644 index 00000000000..db4ad25d10a --- /dev/null +++ b/src/main/runtime/agent-session-record-conversation-name.ts @@ -0,0 +1,27 @@ +import { normalizeAgentSessionConversationName } from '../../shared/agent-session-conversation-name' +import type { AgentSessionRecord } from '../../shared/agent-session-record' + +/** + * Set or clear the conversation name on one record. + * + * Deliberately unfenced: the name is a durable note, not ownership, so writing it never contends + * with the writer lease. Normalizing here — the only writer of the field — keeps the record's own + * validator satisfied no matter which caller supplied the text. + */ +export function setAgentSessionRecordConversationName( + record: AgentSessionRecord, + name: string | null, + now: number +): AgentSessionRecord { + const normalized = name === null ? null : normalizeAgentSessionConversationName(name) + if ((record.conversationName ?? null) === normalized) { + return record + } + const next = { ...record, updatedAt: now } + if (normalized === null) { + delete next.conversationName + return next + } + next.conversationName = normalized + return next +} diff --git a/src/main/runtime/agent-session-record-store.ts b/src/main/runtime/agent-session-record-store.ts index 4325410ed81..1bfc9bbcb3c 100644 --- a/src/main/runtime/agent-session-record-store.ts +++ b/src/main/runtime/agent-session-record-store.ts @@ -1,9 +1,9 @@ import { setVisibleSessionId } from './agent-session-visible-tab-index' import { commitConversationCommandRecord } from './agent-session-conversation-command-record' +import { setAgentSessionRecordConversationName } from './agent-session-record-conversation-name' /** Durable single-writer session records and their operation ledger. */ import { - agentSessionOperationKey, settleAgentSessionOperation, type AgentSessionOperationDecision, type AgentSessionOperationOutcome, @@ -51,10 +51,7 @@ import { type AgentSessionReservationProcesslessProof } from './agent-session-processless-reservation' import { - admitPendingAgentSessionReservationReplay, - applyAgentSessionReservation, - evaluateAgentSessionReserveOperation, - requireAgentSessionRecordForReplay, + commitAgentSessionReservation, type AgentSessionReserveRequest, type AgentSessionReserveResult } from './agent-session-reservation-admission' @@ -145,6 +142,13 @@ export class AgentSessionRecordStore { ) } + /** Unfenced on purpose: the name is a durable note, so writing it never contends with the + * writer lease. `null` clears it. */ + setConversationName = (sessionId: string, name: string | null): Promise => + this.mutate(sessionId, (record) => + setAgentSessionRecordConversationName(record, name, Date.now()) + ) + /** A record this build cannot validate: readable as present, never grantable as a writer. */ isSessionUnreadable(sessionId: string): boolean { return this.state.unreadableRecords.has(sessionId) @@ -165,31 +169,10 @@ export class AgentSessionRecordStore { ) } - /** - * Compare-and-swap reservation plus its client-operation row, committed together. A replayed - * operation returns the recorded outcome and never reaches the reservation. - */ async reserveOwner(request: AgentSessionReserveRequest): Promise { - return this.transact(() => { - const decision = evaluateAgentSessionReserveOperation(this.state, request) - if (decision.decision === 'refused') { - throw new Error(decision.code) - } - if (decision.decision === 'replay') { - let record = requireAgentSessionRecordForReplay(this.state, decision.row, request.sessionId) - if (decision.row.outcome.status === 'pending' && request.handoffOperationId !== null) { - record = admitPendingAgentSessionReservationReplay(record, request) - } - return { record, disposition: 'replayed' as const, operationRow: decision.row } - } - const result = applyAgentSessionReservation(this.state, request, AGENT_SESSION_LEASE_TTL_MS) - this.state.operations.set( - agentSessionOperationKey(request.operation.callerKey, request.operation.operationId), - decision.row - ) - this.state.records.set(result.record.sessionId, result.record) - return { ...result, operationRow: decision.row } - }) + return this.transact(() => + commitAgentSessionReservation(this.state, request, AGENT_SESSION_LEASE_TTL_MS) + ) } async commitProcessIdentity( diff --git a/src/main/runtime/agent-session-reservation-admission.ts b/src/main/runtime/agent-session-reservation-admission.ts index 82176a05297..662ea481921 100644 --- a/src/main/runtime/agent-session-reservation-admission.ts +++ b/src/main/runtime/agent-session-reservation-admission.ts @@ -2,11 +2,15 @@ * Reservation admission: what a reserve request means against the persisted state. * * Pure over a store snapshot so the compare-and-swap, the idempotency replay, and the - * location-immutability check can be reasoned about without touching the disk. The store applies - * the result inside one transaction; nothing here mutates. + * location-immutability check can be reasoned about without touching the disk. + * + * `commitAgentSessionReservation` is the one exception and the only writer here: it sequences + * those decisions and applies the winning one to the state it was handed. The store calls it + * inside a transaction, which is what makes the record and its operation row land together. */ import { + agentSessionOperationKey, evaluateAgentSessionOperation, pruneAgentSessionOperationRows, type AgentSessionOperationDecision, @@ -265,3 +269,32 @@ function createAgentSessionRecord( } } } + +/** + * Compare-and-swap reservation plus its client-operation row, committed together. A replayed + * operation returns the recorded outcome and never reaches the reservation. + */ +export function commitAgentSessionReservation( + state: AgentSessionStoreState, + request: AgentSessionReserveRequest, + leaseTtlMs: number +): AgentSessionReserveResult { + const decision = evaluateAgentSessionReserveOperation(state, request) + if (decision.decision === 'refused') { + throw new Error(decision.code) + } + if (decision.decision === 'replay') { + let record = requireAgentSessionRecordForReplay(state, decision.row, request.sessionId) + if (decision.row.outcome.status === 'pending' && request.handoffOperationId !== null) { + record = admitPendingAgentSessionReservationReplay(record, request) + } + return { record, disposition: 'replayed' as const, operationRow: decision.row } + } + const result = applyAgentSessionReservation(state, request, leaseTtlMs) + state.operations.set( + agentSessionOperationKey(request.operation.callerKey, request.operation.operationId), + decision.row + ) + state.records.set(result.record.sessionId, result.record) + return { ...result, operationRow: decision.row } +} diff --git a/src/main/runtime/structured-conversation-tab-replacement.ts b/src/main/runtime/structured-conversation-tab-replacement.ts index 94d9bdf52d9..7384a374c91 100644 --- a/src/main/runtime/structured-conversation-tab-replacement.ts +++ b/src/main/runtime/structured-conversation-tab-replacement.ts @@ -1,3 +1,4 @@ +import { defaultAgentChatLabel } from '../../shared/agent-session-chat-label' import type { RuntimeMobileSessionTabsSnapshot } from '../../shared/runtime-types' import type { ConversationReplacement } from '../native-chat/agent-session-wire/structured-conversation-command' @@ -30,7 +31,7 @@ export function replaceConversationInSnapshot( id, sessionId: replacement.sessionId, agent: replacement.agent, - title: replacement.agent === 'claude' ? 'Claude Chat' : 'Codex Chat', + title: defaultAgentChatLabel(replacement.agent), replacesSessionId: replacement.sourceSessionId } : tab diff --git a/src/shared/agent-session-conversation-name.test.ts b/src/shared/agent-session-conversation-name.test.ts new file mode 100644 index 00000000000..aa824e5f042 --- /dev/null +++ b/src/shared/agent-session-conversation-name.test.ts @@ -0,0 +1,162 @@ +import { describe, expect, it } from 'vitest' +import { + AGENT_SESSION_CONVERSATION_NAME_MAX_LENGTH, + isAgentSessionConversationName, + normalizeAgentSessionConversationName +} from './agent-session-conversation-name' + +describe('normalizeAgentSessionConversationName', () => { + it('keeps a plain single-line name unchanged', () => { + expect(normalizeAgentSessionConversationName('Fix the flaky lease probe')).toBe( + 'Fix the flaky lease probe' + ) + }) + + it('flattens whitespace so a multi-line name cannot break the tab strip', () => { + expect(normalizeAgentSessionConversationName('Fix the\nlease\tprobe ')).toBe( + 'Fix the lease probe' + ) + }) + + it('rejects an empty or whitespace-only name rather than blanking the label', () => { + expect(normalizeAgentSessionConversationName('')).toBeNull() + expect(normalizeAgentSessionConversationName(' \n ')).toBeNull() + }) + + it('rejects anything that is not a string', () => { + expect(normalizeAgentSessionConversationName(undefined)).toBeNull() + expect(normalizeAgentSessionConversationName(null)).toBeNull() + expect(normalizeAgentSessionConversationName(42)).toBeNull() + expect(normalizeAgentSessionConversationName({ title: 'x' })).toBeNull() + }) + + it('bounds a pasted essay to the stored maximum', () => { + const normalized = normalizeAgentSessionConversationName('a'.repeat(1000)) + expect(normalized).toHaveLength(AGENT_SESSION_CONVERSATION_NAME_MAX_LENGTH) + expect(isAgentSessionConversationName(normalized)).toBe(true) + }) +}) + +describe('isAgentSessionConversationName', () => { + it('accepts only bounded canonical names', () => { + expect(isAgentSessionConversationName('Fix the probe')).toBe(true) + expect(isAgentSessionConversationName('')).toBe(false) + expect(isAgentSessionConversationName('a'.repeat(201))).toBe(false) + expect(isAgentSessionConversationName(' Fix the probe ')).toBe(false) + expect(isAgentSessionConversationName('Fix\nthe probe')).toBe(false) + expect(isAgentSessionConversationName('Fix\u202Egnp.exe probe')).toBe(false) + expect(isAgentSessionConversationName(7)).toBe(false) + }) +}) + +describe('normalizeAgentSessionConversationName hostile text', () => { + it('strips control characters and bidi overrides', () => { + // U+202E renders what follows right-to-left, so a tab could show a label + // that reads as text the name does not contain. + expect(normalizeAgentSessionConversationName('Fix\u202Egnp.exe probe')).toBe( + 'Fix gnp.exe probe' + ) + expect(normalizeAgentSessionConversationName('Fix\u0007the probe')).toBe('Fix the probe') + expect(normalizeAgentSessionConversationName('Fix\u200Bthe probe')).toBe('Fix the probe') + expect(normalizeAgentSessionConversationName('\u202E\u200B ')).toBeNull() + }) + + it('never truncates through a surrogate pair', () => { + const name = `${'a'.repeat(AGENT_SESSION_CONVERSATION_NAME_MAX_LENGTH - 1)}\u{1F600}tail` + + const normalized = normalizeAgentSessionConversationName(name) + + // A raw slice would leave the emoji's lone high surrogate, which renders as + // U+FFFD on every surface that shows the name. + expect(normalized).toBe('a'.repeat(AGENT_SESSION_CONVERSATION_NAME_MAX_LENGTH - 1)) + expect(normalized).not.toContain('\uFFFD') + }) + + it('keeps a legitimate non-ASCII name intact', () => { + expect(normalizeAgentSessionConversationName('R\u00E9sum\u00E9 du fil \u2615')).toBe( + 'R\u00E9sum\u00E9 du fil \u2615' + ) + }) +}) + +describe('normalizeAgentSessionConversationName joiners', () => { + // U+200C/U+200D carry meaning: stripping them as "format characters" splits a + // family emoji into three people and breaks Persian and Hindi orthography. + // The naming prompt asks for the user's own language, so this is normal input. + const ZWJ = '\u200D' + const ZWNJ = '\u200C' + + it.each([ + ['family emoji', `Fix \u{1F468}${ZWJ}\u{1F469}${ZWJ}\u{1F467} layout`], + ['flag emoji', `Ship \u{1F3F3}\uFE0F${ZWJ}\u{1F308} theme`], + ['profession emoji', `Add \u{1F469}${ZWJ}\u{1F4BB} avatar`], + ['Persian ZWNJ', `می${ZWNJ}خواهم تست`], + ['Hindi ZWNJ conjunct', `क्${ZWNJ}ष ठीक`] + ])('keeps the joiners in a %s name', (_label, name) => { + expect(normalizeAgentSessionConversationName(name)).toBe(name) + }) + + // Kept from the hardening: allowing the joiners must not readmit these. + it.each([ + ['bidi override', 'Fix\u202Egnp.exe probe', 'Fix gnp.exe probe'], + ['isolate pair', 'Fix\u2066the\u2069 probe', 'Fix the probe'], + ['Arabic letter mark', 'Fix\u061Cthe probe', 'Fix the probe'], + ['soft hyphen', 'Fix\u00ADthe probe', 'Fix the probe'], + ['word joiner', 'Fix\u2060the probe', 'Fix the probe'], + ['zero-width space', 'Fix\u200Bthe probe', 'Fix the probe'], + ['byte order mark', 'Fix\uFEFFthe probe', 'Fix the probe'] + ])('still strips a %s', (_label, name, expected) => { + expect(normalizeAgentSessionConversationName(name)).toBe(expected) + }) + + // Each row is a `\p{Cf}` run the hand-written enumeration this replaces let + // through, so the name normalized to a non-empty label that renders as nothing. + it.each([ + ['invisible maths operators', '\u2061\u2062\u2063\u2064'], + ['tag characters', '\u{E0020}\u{E0041}\u{E007F}'], + ['a Mongolian vowel separator', '\u180E'], + ['interlinear annotation marks', '\uFFF9\uFFFA\uFFFB'], + ['deprecated format characters', '\u206A\u206B\u206C\u206D\u206E\u206F'], + ['Arabic number signs', '\u0600\u0601\u06DD'], + ['the joiners themselves', `${ZWNJ}${ZWJ}`], + ['a bidi and zero-width mix', '\u202E\u200B\u2060'], + // A joiner survives the collapsing run, so it splits that run in two and + // each half becomes its own space; the guard has to read the spaces too. + ['joiners split by a tab', `${ZWJ}\t${ZWJ}`], + ['joiners split by a newline', `${ZWJ}\n${ZWJ}`], + ['joiners split by a byte order mark', `${ZWJ}\uFEFF${ZWJ}`], + ['joiners split by zero-width spaces', `\u200B${ZWJ}\u200B${ZWJ}`], + ['joiners split by literal spaces', ` ${ZWJ} ${ZWJ} `], + ['joiners split by a bidi override', `\u202E${ZWJ}\u202E${ZWJ}`], + ['mixed joiners split by a tab', `${ZWJ}\t${ZWNJ}`], + ['joiners wrapped in tag characters', `\u{E0020}${ZWJ}\u{E0041}${ZWJ}\u{E007F}`] + ])('rejects a name that is only %s', (_label, name) => { + expect(normalizeAgentSessionConversationName(name)).toBeNull() + }) + + it('drops a tag-character payload hidden after a real title', () => { + // Tag characters mirror ASCII, so this run decodes to readable text that no + // surface draws — it reached the user's own Codex history via thread/name/set. + const hidden = Array.from('ransom', (c) => + String.fromCodePoint(0xe0000 + c.charCodeAt(0)) + ).join('') + + const normalized = normalizeAgentSessionConversationName(`Fix login bug${hidden}`) + + expect(normalized).toBe('Fix login bug') + expect(Array.from(normalized ?? '', (c) => c.codePointAt(0) ?? 0).every((c) => c < 0x7f)).toBe( + true + ) + }) + + it('never ends a truncated name on a dangling joiner', () => { + const name = `${'A'.repeat(197)}\u{1F468}${ZWJ}\u{1F469}${ZWJ}\u{1F467}` + + const normalized = normalizeAgentSessionConversationName(name) + + // The cut lands mid-sequence; the joiner it strands attaches to nothing. + expect(normalized?.endsWith(ZWJ)).toBe(false) + expect(normalized).toBe(`${'A'.repeat(197)}\u{1F468}`) + expect(normalized).not.toContain('\uFFFD') + }) +}) diff --git a/src/shared/agent-session-conversation-name.ts b/src/shared/agent-session-conversation-name.ts new file mode 100644 index 00000000000..039721bfee2 --- /dev/null +++ b/src/shared/agent-session-conversation-name.ts @@ -0,0 +1,67 @@ +// The conversation name Orca recorded for one structured chat, normalized once +// at the single boundary that writes it. +// +// The text is free-form and provider-supplied, so it is bounded and flattened +// here rather than trusted: a name carrying a newline or a bidi override is not +// something the record should ever hold. + +import { sliceAtCodeUnitLimit } from './surrogate-safe-text-slice' + +/** Well past any provider's own cap, short enough that a pasted essay cannot enter the record. */ +export const AGENT_SESSION_CONVERSATION_NAME_MAX_LENGTH = 200 + +/** Whitespace, plus the C0/C1 controls, bidi controls and zero-width marks `\s` + * misses. A bidi override renders a label that reads as text the name does not + * contain, and a zero-width run renders as nothing at all. Subtracted from all + * of `\p{Cf}` rather than enumerated, so a format character Unicode adds later + * is covered with no list to remember; U+200C/U+200D are the one exception, + * being load-bearing in Persian, Hindi and every multi-part emoji. Accepted + * cost: the U+E0020-E007F tag sequences go too, so the England, Scotland and + * Wales flags degrade — far cheaper than an invisible payload in a label. + * Deliberately NOT reached: blank-RENDERING letters and marks such as U+2800, + * U+3164 and U+115F, which are Lo/So/Mn rather than any invisible category. A + * name made only of those is accepted and looks empty; Braille and the Hangul + * jamo fillers carry meaning in real text, so stripping them would cost more. */ +const UNRENDERABLE_RUN = /(?:[\s\p{Cc}\p{Zl}\p{Zp}]|(?![\u200C\u200D])\p{Cf})+/gu + +/** The joiners outlive the run above by design; alone — or separated only by the + * spaces that run collapsed to — they are still a blank label. */ +const BLANK_ONLY = /^[\s\u200C\u200D]+$/u + +/** A surrogate with no partner — a provider that truncated an emoji, usually. + * Under `u` this class matches ONLY unpaired ones, so astral characters keep + * both halves; left in, each renders as U+FFFD on every surface. */ +const LONE_SURROGATE = /[\uD800-\uDFFF]/gu + +/** A cut inside an emoji sequence strands the joiner that attached it. */ +const TRAILING_DANGLE = /[\s\u200C\u200D]+$/u + +export function normalizeAgentSessionConversationName(value: unknown): string | null { + if (typeof value !== 'string') { + return null + } + // Surrogates first, so the gap one leaves collapses with the run around it. + const collapsed = value.replace(LONE_SURROGATE, '').replace(UNRENDERABLE_RUN, ' ').trim() + if (!collapsed || BLANK_ONLY.test(collapsed)) { + return null + } + if (collapsed.length <= AGENT_SESSION_CONVERSATION_NAME_MAX_LENGTH) { + return collapsed + } + // Cut on a character boundary: a raw slice can strand a lone high surrogate, + // which every surface then renders as U+FFFD. + const truncated = sliceAtCodeUnitLimit( + collapsed, + AGENT_SESSION_CONVERSATION_NAME_MAX_LENGTH + ).replace(TRAILING_DANGLE, '') + return truncated || null +} + +export function isAgentSessionConversationName(value: unknown): value is string { + return ( + typeof value === 'string' && + value.length > 0 && + value.length <= AGENT_SESSION_CONVERSATION_NAME_MAX_LENGTH && + normalizeAgentSessionConversationName(value) === value + ) +} diff --git a/src/shared/agent-session-record.ts b/src/shared/agent-session-record.ts index 961c88e3aac..a8add5c1128 100644 --- a/src/shared/agent-session-record.ts +++ b/src/shared/agent-session-record.ts @@ -1,4 +1,5 @@ import { isAgentSessionRewindRecord, type AgentSessionRewindRecord } from './agent-session-rewind' +import { isAgentSessionConversationName } from './agent-session-conversation-name' /** * Durable agent-session record and its single-writer lease. * @@ -132,6 +133,8 @@ export type AgentSessionRecord = { options?: Record rewind?: AgentSessionRewindRecord conversationCommand?: AgentSessionConversationCommandRecord + /** The name Orca gave this conversation, so a later acquisition need not name it again. */ + conversationName?: string launchArgs?: AgentSessionLaunchArgs lease: AgentSessionLease createdAt: number @@ -345,6 +348,8 @@ export function isAgentSessionRecord(value: unknown): value is AgentSessionRecor (record.rewind === undefined || isAgentSessionRewindRecord(record.rewind)) && (record.conversationCommand === undefined || isAgentSessionConversationCommandRecord(record.conversationCommand)) && + (record.conversationName === undefined || + isAgentSessionConversationName(record.conversationName)) && (record.launchArgs === undefined || isAgentSessionLaunchArgs(record.launchArgs)) && !Object.hasOwn(record, 'launchEnv') && isAgentSessionLease(record.lease) && diff --git a/src/shared/surrogate-safe-text-slice.ts b/src/shared/surrogate-safe-text-slice.ts new file mode 100644 index 00000000000..6e9e2d8d030 --- /dev/null +++ b/src/shared/surrogate-safe-text-slice.ts @@ -0,0 +1,19 @@ +// Cutting text to a length bound without splitting a character in half. +// +// JavaScript string length counts UTF-16 code units, so a raw `slice` at a +// bound can land between the two halves of an astral character — an emoji, or +// most CJK extension characters — and leave a lone surrogate that every surface +// renders as U+FFFD. + +/** Cut to `limit` UTF-16 code units without splitting a trailing surrogate pair. */ +export function sliceAtCodeUnitLimit(value: string, limit: number): string { + if (value.length <= limit) { + return value + } + const end = limit > 0 && isHighSurrogate(value.charCodeAt(limit - 1)) ? limit - 1 : limit + return value.slice(0, end) +} + +function isHighSurrogate(code: number): boolean { + return code >= 0xd800 && code <= 0xdbff +}