mirror of
https://github.com/stablyai/orca.git
synced 2026-09-30 00:03:15 +00:00
fix(native-chat): make conversation naming a durable, thread-scoped decision
Twelve review findings, most of which collapse into two causes. "Have we named this conversation" was live-session state. Both providers rebuild their session object on every acquisition, so the flag reset and the next message re-titled the chat — re-imposing a name the user had cleared, and paying for a model call to do it. The marker now lives on the record beside the name, so an eviction, a restart and a re-acquisition all see the same answer. The record gained a clear path too: a name deleted in another client no longer lingers here and keeps rendering. Both once-per-conversation tests now construct a SECOND session, which is what re-acquisition does and what the old same-object tests could not catch. The naming turn's isolation was per-frame-kind and time-boxed. Server requests bypassed the gate entirely, so an approval from the naming turn became a durable prompt in the user's chat for a command they never asked for, pending forever once the turn was abandoned; it is refused now, which also settles the turn. Unhandled frames bypassed it too. And the gate closed when the collector settled, including on its own timeout, so a late frame carrying the title arrived with the gate down. The throwaway thread id is retained for the life of the session instead, which makes the leak structurally impossible rather than a matter of timing. Also: a revealed or reopened chat showed the placeholder forever, because only the startup sweep passed a title and an unchanged name never fans out — every publication now labels itself from the record. The re-read before `thread/name/set` failed OPEN on a reply shape this build cannot parse, and would have overwritten a name a person chose; it fails closed. The collector waited on a `turn/failed` that does not exist, so a refused turn stalled for its full timeout; it settles on a non-retryable `error`, matched on thread id — a retryable one explicitly does not interrupt the turn. The throwaway thread is deleted when the app-server persisted it despite `ephemeral`, verified against codex 0.153.4, which refuses to delete a truly ephemeral one and so is not asked to. The Claude title read moved onto the bounded reverse tail scan the transcript readers already use, extracted so both share it. And every naming failure now logs, so a host whose app-server refuses to name threads is distinguishable from a model that declined. `republishStatus` was a no-op once the status summary stopped carrying a name; removed with its comment. Prompt-text extraction moved inside the protective promise and guards its input, because only the RPC send path validates blocks and a throw there turns a delivered message into a failed one. The session-id pattern forbids ':', so a prefixed key cannot reach the host's map; the tab id and the record it is labelled from are now derived from one stripped value so they cannot disagree.
This commit is contained in:
@@ -24,3 +24,32 @@ describe('createClaudeControlSurface stopTask', () => {
|
||||
expect(stopTask).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
})
|
||||
|
||||
describe('createClaudeControlSurface generateSessionTitle', () => {
|
||||
it('answers null when the CLI exposes no title request at all', async () => {
|
||||
// The real degradation path: the shipped Query declaration omits the method,
|
||||
// and an older CLI genuinely does not have it. The surface still exposes the
|
||||
// method so callers never probe, and answers null rather than throwing.
|
||||
const surface = createClaudeControlSurface({} as unknown as Query)
|
||||
|
||||
await expect(surface.generateSessionTitle('fix the lease probe')).resolves.toBeNull()
|
||||
})
|
||||
|
||||
it('asks the CLI to persist the title and trims what comes back', async () => {
|
||||
const generateSessionTitle = vi.fn(async () => ' Lease probe flake ')
|
||||
const surface = createClaudeControlSurface({ generateSessionTitle } as unknown as Query)
|
||||
|
||||
await expect(
|
||||
surface.generateSessionTitle('fix the lease probe', { persist: true })
|
||||
).resolves.toBe('Lease probe flake')
|
||||
expect(generateSessionTitle).toHaveBeenCalledWith('fix the lease probe', { persist: true })
|
||||
})
|
||||
|
||||
it('treats a blank title as no title', async () => {
|
||||
const surface = createClaudeControlSurface({
|
||||
generateSessionTitle: async () => ' '
|
||||
} as unknown as Query)
|
||||
|
||||
await expect(surface.generateSessionTitle('fix the lease probe')).resolves.toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -59,19 +59,28 @@ describe('startClaudeConversationNaming', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('asks only once per session', async () => {
|
||||
it('asks only once across a re-acquisition, which builds a NEW session object', async () => {
|
||||
const generateSessionTitle = vi.fn(async () => null)
|
||||
const session = sessionWith(generateSessionTitle)
|
||||
// Passing the same object twice could only prove the in-memory flag; an
|
||||
// eviction hands the next send a fresh session, which is the real case.
|
||||
let attempted = false
|
||||
const deps = {
|
||||
onConversationName: vi.fn(),
|
||||
readNamingState: () => ({ conversationName: null, namingAttempted: attempted }),
|
||||
markNamingAttempted: () => {
|
||||
attempted = true
|
||||
}
|
||||
}
|
||||
|
||||
startClaudeConversationNaming(SESSION, session, USER_TURN, { onConversationName: vi.fn() })
|
||||
startClaudeConversationNaming(SESSION, sessionWith(generateSessionTitle), USER_TURN, deps)
|
||||
await settle()
|
||||
startClaudeConversationNaming(SESSION, session, USER_TURN, { onConversationName: vi.fn() })
|
||||
startClaudeConversationNaming(SESSION, sessionWith(generateSessionTitle), USER_TURN, deps)
|
||||
await settle()
|
||||
|
||||
expect(generateSessionTitle).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('reports nothing when the CLI exposes no title request', async () => {
|
||||
it('reports nothing when the title request answers null', async () => {
|
||||
const onConversationName = vi.fn()
|
||||
|
||||
startClaudeConversationNaming(SESSION, sessionWith(vi.fn(async () => null)), USER_TURN, {
|
||||
@@ -139,7 +148,7 @@ describe('startClaudeConversationNaming across re-acquisitions', () => {
|
||||
|
||||
startClaudeConversationNaming(SESSION, reacquired, USER_TURN, {
|
||||
onConversationName: vi.fn(),
|
||||
readConversationName: () => 'Lease probe flake'
|
||||
readNamingState: () => ({ conversationName: 'Lease probe flake', namingAttempted: true })
|
||||
})
|
||||
await settle()
|
||||
|
||||
@@ -152,7 +161,7 @@ describe('startClaudeConversationNaming across re-acquisitions', () => {
|
||||
|
||||
startClaudeConversationNaming(SESSION, sessionWith(generateSessionTitle), USER_TURN, {
|
||||
onConversationName,
|
||||
readConversationName: () => null
|
||||
readNamingState: () => ({ conversationName: null, namingAttempted: false })
|
||||
})
|
||||
await settle()
|
||||
|
||||
|
||||
@@ -9,10 +9,6 @@
|
||||
// Deliberately NOT Codex's imperative-verb style: Claude's own titling is a short
|
||||
// noun phrase in sentence case, and the SDK call already produces that. Passing
|
||||
// the user's text as the description and nothing else keeps it that way.
|
||||
//
|
||||
// Asked at most once per NAMED CONVERSATION, not once per session object: Claude
|
||||
// rebuilds its session on every acquisition, so the in-memory flag alone would
|
||||
// retitle the chat — and pay for it — on the second message after every eviction.
|
||||
|
||||
import type { AgentJournalMessageItem } from '../../shared/agent-session-journal-types'
|
||||
import { agentSessionNamingPromptText } from '../native-chat/agent-session-wire/agent-session-naming-prompt-text'
|
||||
@@ -21,17 +17,23 @@ import type { ClaudeSession } from './claude-structured-session-state'
|
||||
export type ClaudeConversationNamingDeps = {
|
||||
requestTimeoutMs?: number
|
||||
onConversationName?: (sessionId: string, conversationName: string) => void
|
||||
/** The name already recorded for this session, if any. Claude's session object
|
||||
* is rebuilt on every acquisition, so the in-memory one-shot flag alone would
|
||||
* re-title the conversation once per eviction cycle. */
|
||||
readConversationName?: (sessionId: string) => string | null
|
||||
/** The durable naming state. Claude rebuilds its session object on every
|
||||
* acquisition, so an in-memory flag alone would retitle the conversation —
|
||||
* and pay for it — on the second message after every eviction. */
|
||||
readNamingState?: (sessionId: string) => {
|
||||
conversationName: string | null
|
||||
namingAttempted: boolean
|
||||
}
|
||||
markNamingAttempted?: (sessionId: string) => void
|
||||
onError?: (scope: string, error: unknown) => void
|
||||
}
|
||||
|
||||
/**
|
||||
* Names the session once, off the turn's critical path.
|
||||
*
|
||||
* One attempt only: a CLI that exposes no title request, or a turn the model
|
||||
* declined to name, must not be re-asked on every later turn.
|
||||
* Asked at most once per CONVERSATION rather than once per session object, and
|
||||
* every step runs inside the promise: this sits on the send path, and nothing
|
||||
* here may turn a delivered message into a reported failure.
|
||||
*/
|
||||
export function startClaudeConversationNaming(
|
||||
sessionId: string,
|
||||
@@ -42,33 +44,25 @@ export function startClaudeConversationNaming(
|
||||
if (session.namingAttempted || !deps.onConversationName) {
|
||||
return
|
||||
}
|
||||
// The durable record outlives the session object; a conversation named on any
|
||||
// earlier acquisition is never renamed, and never paid for twice.
|
||||
if (deps.readConversationName?.(sessionId)) {
|
||||
session.namingAttempted = true
|
||||
return
|
||||
}
|
||||
const description = agentSessionNamingPromptText(body)
|
||||
if (!description) {
|
||||
return
|
||||
}
|
||||
session.namingAttempted = true
|
||||
// Started inside a promise so NOTHING here can reach the caller. This runs on
|
||||
// the send path, and an integration fake without the method turned a delivered
|
||||
// message into a reported failure — a synchronous throw from any cause would do
|
||||
// the same. A chat with no name beats a send that claims it failed.
|
||||
void Promise.resolve()
|
||||
.then(() =>
|
||||
session.connection.generateSessionTitle(description, {
|
||||
.then(async () => {
|
||||
const durable = deps.readNamingState?.(sessionId)
|
||||
if (durable?.conversationName || durable?.namingAttempted) {
|
||||
return
|
||||
}
|
||||
const description = agentSessionNamingPromptText(body)
|
||||
if (!description) {
|
||||
return
|
||||
}
|
||||
deps.markNamingAttempted?.(sessionId)
|
||||
const title = await session.connection.generateSessionTitle(description, {
|
||||
persist: true,
|
||||
...(deps.requestTimeoutMs ? { timeoutMs: deps.requestTimeoutMs } : {})
|
||||
})
|
||||
)
|
||||
.then((title) => {
|
||||
if (title) {
|
||||
deps.onConversationName?.(sessionId, title)
|
||||
}
|
||||
})
|
||||
// A session without a name is the state this started in; never surface it.
|
||||
.catch(() => undefined)
|
||||
.catch((error: unknown) => deps.onError?.('claude-conversation-naming', error))
|
||||
}
|
||||
|
||||
@@ -230,7 +230,10 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda
|
||||
if (outcome.state === 'accepted') {
|
||||
// The accepted user message is the only text this session is sure the CLI
|
||||
// received, and the first thing worth naming the conversation after.
|
||||
startClaudeConversationNaming(input.sessionId, session, input.body, this.deps)
|
||||
startClaudeConversationNaming(input.sessionId, session, input.body, {
|
||||
...this.deps,
|
||||
...(this.deps.onNamingError ? { onError: this.deps.onNamingError } : {})
|
||||
})
|
||||
}
|
||||
return outcome
|
||||
}
|
||||
|
||||
@@ -76,8 +76,13 @@ export type ClaudeStructuredSessionAdapterDeps = {
|
||||
}) => Promise<void>
|
||||
/** Claude named (or the user renamed) the conversation behind this session. */
|
||||
onConversationName?: (sessionId: string, conversationName: string) => void
|
||||
/** The name already recorded for a session, so a re-acquisition does not retitle it. */
|
||||
readConversationName?: (sessionId: string) => string | null
|
||||
/** The durable naming state, so a re-acquisition does not retitle. */
|
||||
readNamingState?: (sessionId: string) => {
|
||||
conversationName: string | null
|
||||
namingAttempted: boolean
|
||||
}
|
||||
markNamingAttempted?: (sessionId: string) => void
|
||||
onNamingError?: (scope: string, error: unknown) => void
|
||||
/** The name Claude already persisted for this provider session, if any. Its
|
||||
* stream carries no title frame, so the transcript is the only source. */
|
||||
readTranscriptConversationName?: (input: {
|
||||
@@ -136,7 +141,8 @@ export type ClaudeSession = {
|
||||
/** Provider uuid of the most recently admitted turn, if one is active. */
|
||||
activeTurnId?: string
|
||||
backgroundTasks: ClaudeBackgroundTaskTracker
|
||||
/** One naming attempt per session; see claude-conversation-name-turn. */
|
||||
/** Guards a second attempt within this live session only; the durable marker
|
||||
* on the record is what survives eviction. See claude-conversation-name-turn. */
|
||||
namingAttempted: boolean
|
||||
/** Monotonic fence advanced when a dispatch starts, including unresolved dispatches. */
|
||||
dispatchSequence: number
|
||||
|
||||
@@ -3,41 +3,47 @@
|
||||
//
|
||||
// Claude's stream-json protocol carries no title frame, so the transcript is the
|
||||
// only place a name it generated (or a name the user set from the CLI) survives.
|
||||
// The records are read through the AI Vault session parser rather than a second
|
||||
// one, so `custom-title` / `ai-title` keep meaning exactly what they mean there.
|
||||
// `custom-title` and `ai-title` are appended, last-wins records, so the file is
|
||||
// read backwards in bounded chunks: a full parse on every acquisition competes
|
||||
// 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.
|
||||
|
||||
import { createInterface } from 'node:readline'
|
||||
import { openTranscriptReadStream } from '../native-chat/wsl-transcript-fs-access'
|
||||
import {
|
||||
consumeClaudeSessionLine,
|
||||
createClaudeSessionParseState
|
||||
} from '../ai-vault/session-scanner-primary-parsers'
|
||||
import { normalizeTitleText, parseJsonObject } from '../ai-vault/session-scanner-values'
|
||||
import { claudeTranscriptTailLines } from './claude-transcript-tail-scan'
|
||||
|
||||
/**
|
||||
* The transcript's stored name, or null when it holds none.
|
||||
* The transcript's stored name, or null when its tail holds none.
|
||||
*
|
||||
* A user's own `custom-title` outranks the generated `ai-title`, matching the
|
||||
* precedence the CLI itself applies when it shows the session's name.
|
||||
* precedence the CLI itself applies. Read newest-first, so the first record of
|
||||
* each kind is the current one.
|
||||
*/
|
||||
export async function readClaudeTranscriptConversationName(
|
||||
transcriptPath: string
|
||||
): Promise<string | null> {
|
||||
const state = createClaudeSessionParseState({
|
||||
path: transcriptPath,
|
||||
mtimeMs: 0,
|
||||
modifiedAt: new Date(0).toISOString()
|
||||
})
|
||||
const stream = openTranscriptReadStream(transcriptPath, { encoding: 'utf-8' }, 'scan')
|
||||
const lines = createInterface({ input: stream, crlfDelay: Infinity })
|
||||
try {
|
||||
for await (const line of lines) {
|
||||
consumeClaudeSessionLine(state, line)
|
||||
let generated: string | null = null
|
||||
for await (const line of claudeTranscriptTailLines(transcriptPath)) {
|
||||
if (!line.includes('-title')) {
|
||||
continue
|
||||
}
|
||||
const record = parseJsonObject(line)
|
||||
if (!record) {
|
||||
continue
|
||||
}
|
||||
if (record.type === 'custom-title') {
|
||||
const title = normalizeTitleText(String(record.customTitle ?? ''))
|
||||
if (title) {
|
||||
return title
|
||||
}
|
||||
}
|
||||
if (record.type === 'ai-title' && !generated) {
|
||||
generated = normalizeTitleText(String(record.aiTitle ?? '')) || null
|
||||
}
|
||||
} finally {
|
||||
lines.close()
|
||||
stream.destroy()
|
||||
}
|
||||
return state.accumulator.title || state.generatedTitle || null
|
||||
return generated
|
||||
}
|
||||
|
||||
export type ClaudeConversationNameReporter = {
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
// Reading a Claude transcript backwards, in bounded chunks.
|
||||
//
|
||||
// These files reach many megabytes on a long conversation, and the records Orca
|
||||
// wants from them — the leaf uuid, the session's name — are appended, so the tail
|
||||
// holds them. Reading the whole file to find a trailing record costs a full parse
|
||||
// on every acquisition, competing with the attach it runs alongside.
|
||||
//
|
||||
// The bound is deliberate: a record older than the limit is NOT found. Every
|
||||
// caller here treats "not found" as "no answer yet", never as a negative fact.
|
||||
|
||||
import { open } from 'node:fs/promises'
|
||||
|
||||
const TRANSCRIPT_TAIL_CHUNK_BYTES = 64 * 1024
|
||||
export const TRANSCRIPT_TAIL_READ_LIMIT_BYTES = 4 * 1024 * 1024
|
||||
|
||||
/**
|
||||
* Every non-empty line of the transcript's tail, newest first, up to the limit.
|
||||
*
|
||||
* A chunk boundary can split a line, so the leading partial of each chunk is
|
||||
* carried into the next (earlier) one rather than parsed as a whole line.
|
||||
*/
|
||||
export async function* claudeTranscriptTailLines(
|
||||
transcriptPath: string
|
||||
): AsyncGenerator<string, void, undefined> {
|
||||
const file = await open(transcriptPath, 'r')
|
||||
try {
|
||||
const { size } = await file.stat()
|
||||
let position = size
|
||||
let suffix = ''
|
||||
let scanned = 0
|
||||
while (position > 0 && scanned < TRANSCRIPT_TAIL_READ_LIMIT_BYTES) {
|
||||
const length = Math.min(TRANSCRIPT_TAIL_CHUNK_BYTES, position)
|
||||
position -= length
|
||||
scanned += length
|
||||
const buffer = Buffer.alloc(length)
|
||||
await file.read(buffer, 0, length, position)
|
||||
const lines = `${buffer.toString('utf8')}${suffix}`.split(/\r?\n/)
|
||||
suffix = position > 0 ? (lines.shift() ?? '') : ''
|
||||
for (let index = lines.length - 1; index >= 0; index -= 1) {
|
||||
const line = lines[index]?.trim()
|
||||
if (line) {
|
||||
yield line
|
||||
}
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
await file.close()
|
||||
}
|
||||
}
|
||||
@@ -1,10 +1,7 @@
|
||||
import { open } from 'node:fs/promises'
|
||||
import type { AgentSessionProviderHandleLink } from '../../shared/agent-session-provider-handle'
|
||||
import { claudeTranscriptTailLines } from './claude-transcript-tail-scan'
|
||||
import { claudeProviderHandleLink } from './claude-structured-owner-identity'
|
||||
|
||||
const TRANSCRIPT_TAIL_CHUNK_BYTES = 64 * 1024
|
||||
const TRANSCRIPT_TAIL_READ_LIMIT_BYTES = 4 * 1024 * 1024
|
||||
|
||||
type TranscriptLeafCandidate = { leafUuid: string; authoritative: boolean }
|
||||
|
||||
function validLeafUuid(value: unknown): string | null {
|
||||
@@ -41,40 +38,15 @@ function readLeafCandidate(line: string): TranscriptLeafCandidate | null {
|
||||
}
|
||||
|
||||
export async function readClaudeTranscriptLeafUuid(transcriptPath: string): Promise<string | null> {
|
||||
const file = await open(transcriptPath, 'r')
|
||||
try {
|
||||
const { size } = await file.stat()
|
||||
let position = size
|
||||
let suffix = ''
|
||||
let fallback: string | null = null
|
||||
let scanned = 0
|
||||
while (position > 0 && scanned < TRANSCRIPT_TAIL_READ_LIMIT_BYTES) {
|
||||
const length = Math.min(TRANSCRIPT_TAIL_CHUNK_BYTES, position)
|
||||
position -= length
|
||||
scanned += length
|
||||
const buffer = Buffer.alloc(length)
|
||||
await file.read(buffer, 0, length, position)
|
||||
const lines = `${buffer.toString('utf8')}${suffix}`.split(/\r?\n/)
|
||||
suffix = position > 0 ? (lines.shift() ?? '') : ''
|
||||
for (let index = lines.length - 1; index >= 0; index -= 1) {
|
||||
const line = lines[index]?.trim()
|
||||
if (!line) {
|
||||
continue
|
||||
}
|
||||
const candidate = readLeafCandidate(line)
|
||||
if (!candidate) {
|
||||
continue
|
||||
}
|
||||
if (candidate.authoritative) {
|
||||
return candidate.leafUuid
|
||||
}
|
||||
fallback ??= candidate.leafUuid
|
||||
}
|
||||
let fallback: string | null = null
|
||||
for await (const line of claudeTranscriptTailLines(transcriptPath)) {
|
||||
const candidate = readLeafCandidate(line)
|
||||
if (candidate?.authoritative) {
|
||||
return candidate.leafUuid
|
||||
}
|
||||
return fallback
|
||||
} finally {
|
||||
await file.close()
|
||||
fallback ??= candidate?.leafUuid ?? fallback
|
||||
}
|
||||
return fallback
|
||||
}
|
||||
|
||||
export type ClaudeTuiChildExit = {
|
||||
|
||||
@@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest'
|
||||
import type { CodexAppServerConnection } from './codex-app-server-connection'
|
||||
import {
|
||||
createCodexNamingTurnCollector,
|
||||
isTerminalCodexTurnError,
|
||||
generateAndSetCodexConversationName,
|
||||
readCodexGeneratedTitle
|
||||
} from './codex-conversation-name-generation'
|
||||
@@ -10,22 +11,38 @@ const THREAD = 'thread-user'
|
||||
const NAMING = 'thread-naming'
|
||||
|
||||
/** An app-server that answers the naming flow's four requests. */
|
||||
function fakeConnection(options: { nameOnReRead?: string | null; answer?: string | null } = {}) {
|
||||
function fakeConnection(
|
||||
options: {
|
||||
nameOnReRead?: string | null
|
||||
answer?: string | null
|
||||
/** The app-server ignored `ephemeral` and persisted the throwaway thread. */
|
||||
persistDespiteEphemeral?: boolean
|
||||
/** A `thread/read` reply shape this build may or may not understand. */
|
||||
threadReadReply?: unknown
|
||||
} = {}
|
||||
) {
|
||||
const calls: { method: string; params: Record<string, unknown> }[] = []
|
||||
const connection: Pick<CodexAppServerConnection, 'request'> = {
|
||||
request: vi.fn(async (method: string, params?: Record<string, unknown>) => {
|
||||
calls.push({ method, params: params ?? {} })
|
||||
if (method === 'thread/start') {
|
||||
return { thread: { id: NAMING, ephemeral: params?.ephemeral === true } }
|
||||
}
|
||||
if (method === 'thread/read') {
|
||||
return {
|
||||
thread: {
|
||||
id: THREAD,
|
||||
...(options.nameOnReRead ? { name: options.nameOnReRead } : {})
|
||||
id: NAMING,
|
||||
...(options.persistDespiteEphemeral ? {} : { ephemeral: params?.ephemeral === true })
|
||||
}
|
||||
}
|
||||
}
|
||||
if (method === 'thread/read') {
|
||||
return options.threadReadReply !== undefined
|
||||
? options.threadReadReply
|
||||
: {
|
||||
thread: {
|
||||
id: THREAD,
|
||||
...(options.nameOnReRead ? { name: options.nameOnReRead } : {})
|
||||
}
|
||||
}
|
||||
}
|
||||
return {}
|
||||
})
|
||||
}
|
||||
@@ -181,3 +198,75 @@ describe('generateAndSetCodexConversationName', () => {
|
||||
expect(calls.some((call) => call.method === 'thread/name/set')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('naming-turn cleanup and fail-closed re-read', () => {
|
||||
it('deletes a throwaway thread the app-server persisted despite ephemeral', async () => {
|
||||
const { connection, calls } = fakeConnection({ persistDespiteEphemeral: true })
|
||||
|
||||
await run(connection, '{"title":"Fix flaky lease probe"}')
|
||||
|
||||
// Otherwise every named chat leaves a junk thread and rollout file behind.
|
||||
expect(calls.find((call) => call.method === 'thread/delete')?.params).toEqual({
|
||||
threadId: NAMING
|
||||
})
|
||||
})
|
||||
|
||||
it('does not try to delete a genuinely ephemeral thread', async () => {
|
||||
const { connection, calls } = fakeConnection()
|
||||
|
||||
await run(connection, '{"title":"Fix flaky lease probe"}')
|
||||
|
||||
// The app-server refuses to delete one, so attempting it would log a failure
|
||||
// on every successful naming.
|
||||
expect(calls.some((call) => call.method === 'thread/delete')).toBe(false)
|
||||
})
|
||||
|
||||
// NOTE: a name under an unrecognised KEY on an otherwise-readable reply is not
|
||||
// detectable — by construction this build does not know the key. What fails
|
||||
// closed is an unrecognised reply SHAPE, which is the case it can decide.
|
||||
it.each([
|
||||
['a reply with no thread object', { threadId: 'thread-user' }],
|
||||
['a reply that is not an object', 'thread-user']
|
||||
])(
|
||||
'refuses to overwrite a name it cannot positively read as absent: %s',
|
||||
async (_label, reply) => {
|
||||
const { connection, calls } = fakeConnection({ threadReadReply: reply })
|
||||
|
||||
await expect(run(connection, '{"title":"Fix flaky lease probe"}')).resolves.toBeNull()
|
||||
|
||||
// Fails CLOSED. Skipping a name is a non-event; clobbering a rename is not.
|
||||
expect(calls.some((call) => call.method === 'thread/name/set')).toBe(false)
|
||||
}
|
||||
)
|
||||
|
||||
it('still names a thread a readable reply shows as unnamed', async () => {
|
||||
const { connection, calls } = fakeConnection()
|
||||
|
||||
await expect(run(connection, '{"title":"Fix flaky lease probe"}')).resolves.toBe(
|
||||
'Fix flaky lease probe'
|
||||
)
|
||||
|
||||
expect(calls.find((call) => call.method === 'thread/name/set')?.params).toEqual({
|
||||
threadId: THREAD,
|
||||
name: 'Fix flaky lease probe'
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('isTerminalCodexTurnError', () => {
|
||||
it('ends the turn on a non-retryable error', () => {
|
||||
expect(isTerminalCodexTurnError('error', { threadId: 't', willRetry: false })).toBe(true)
|
||||
})
|
||||
|
||||
it('does NOT end the turn on a retryable one', () => {
|
||||
// A retryable error explicitly does not interrupt the turn; settling here
|
||||
// would abandon a naming turn that was about to succeed.
|
||||
expect(isTerminalCodexTurnError('error', { threadId: 't', willRetry: true })).toBe(false)
|
||||
expect(isTerminalCodexTurnError('error', { threadId: 't', will_retry: true })).toBe(false)
|
||||
})
|
||||
|
||||
it('ignores anything that is not an error frame', () => {
|
||||
expect(isTerminalCodexTurnError('turn/started', { willRetry: false })).toBe(false)
|
||||
expect(isTerminalCodexTurnError('error', null)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -25,7 +25,7 @@
|
||||
import type { CodexAppServerConnection } from './codex-app-server-connection'
|
||||
import { readCodexThreadId, readCodexThreadName } from './codex-structured-thread-facts'
|
||||
|
||||
/** Upstream's own bound for a thread name; keeps the tab strip readable. */
|
||||
/** Short enough to read as a tab label at a glance; also the schema's own cap. */
|
||||
export const CODEX_CONVERSATION_NAME_MAX_LENGTH = 36
|
||||
|
||||
export const CODEX_CONVERSATION_NAME_SCHEMA = {
|
||||
@@ -82,6 +82,25 @@ export type CodexNamingTurnCollector = {
|
||||
answer: Promise<string | null>
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether an `error` frame ends the turn. There is no `turn/failed`; a refused or
|
||||
* rate-limited turn arrives as `error`. A RETRYABLE one explicitly does not
|
||||
* interrupt the turn, so settling on it would abandon a naming turn that was
|
||||
* about to succeed.
|
||||
*/
|
||||
export function isTerminalCodexTurnError(method: string, params: unknown): boolean {
|
||||
if (method !== 'error') {
|
||||
return false
|
||||
}
|
||||
if (typeof params !== 'object' || params === null) {
|
||||
return false
|
||||
}
|
||||
return (
|
||||
(params as { willRetry?: unknown; will_retry?: unknown }).willRetry !== true &&
|
||||
(params as { will_retry?: unknown }).will_retry !== true
|
||||
)
|
||||
}
|
||||
|
||||
function agentMessageText(params: unknown): string | null {
|
||||
if (typeof params !== 'object' || params === null) {
|
||||
return null
|
||||
@@ -109,10 +128,7 @@ export function createCodexNamingTurnCollector(timeoutMs: number): CodexNamingTu
|
||||
handle: (method, params) => {
|
||||
if (method === 'item/completed') {
|
||||
latest = agentMessageText(params) ?? latest
|
||||
} else if (method === 'turn/completed' || method === 'error') {
|
||||
// `error` is how the app-server reports a refused or rate-limited turn;
|
||||
// there is no `turn/failed`. Without it a failed turn would hold this
|
||||
// open for the full timeout.
|
||||
} else if (method === 'turn/completed' || isTerminalCodexTurnError(method, params)) {
|
||||
settle(latest)
|
||||
}
|
||||
},
|
||||
@@ -138,6 +154,38 @@ export function readCodexGeneratedTitle(answer: string | null): string | null {
|
||||
return typeof title === 'string' && title.trim() ? title.trim() : null
|
||||
}
|
||||
|
||||
/**
|
||||
* True only when the reply is one this build understands AND carries no name.
|
||||
* An unrecognised shape is not evidence of an unnamed thread.
|
||||
*/
|
||||
export function isCodexThreadReadablyUnnamed(read: unknown): boolean {
|
||||
if (typeof read !== 'object' || read === null) {
|
||||
return false
|
||||
}
|
||||
const thread = (read as { thread?: unknown }).thread
|
||||
if (typeof thread !== 'object' || thread === null) {
|
||||
return false
|
||||
}
|
||||
// A thread reply this build can read always names the thread it describes.
|
||||
if (typeof (thread as { id?: unknown }).id !== 'string') {
|
||||
return false
|
||||
}
|
||||
return readCodexThreadName(read) === null
|
||||
}
|
||||
|
||||
/** Whether the app-server confirmed the thread it opened is throwaway. */
|
||||
export function readCodexThreadIsEphemeral(opened: unknown): boolean {
|
||||
if (typeof opened !== 'object' || opened === null) {
|
||||
return false
|
||||
}
|
||||
const thread = (opened as { thread?: unknown }).thread
|
||||
return (
|
||||
typeof thread === 'object' &&
|
||||
thread !== null &&
|
||||
(thread as { ephemeral?: unknown }).ephemeral === true
|
||||
)
|
||||
}
|
||||
|
||||
export type CodexConversationNameGeneration = {
|
||||
connection: Pick<CodexAppServerConnection, 'request'>
|
||||
cwd: string
|
||||
@@ -151,6 +199,8 @@ export type CodexConversationNameGeneration = {
|
||||
* arriving after this flow gives up are dropped rather than journaled. */
|
||||
retainNamingThread: (namingThreadId: string) => void
|
||||
closeNamingTurn: () => void
|
||||
/** Diagnostics only; naming never surfaces to the user. */
|
||||
onError?: (scope: string, error: unknown) => void
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -163,6 +213,7 @@ export async function generateAndSetCodexConversationName(
|
||||
const { connection, timeoutMs } = input
|
||||
const collector = input.openNamingTurn()
|
||||
let answer: string | null
|
||||
let disposableThreadId: string | null = null
|
||||
try {
|
||||
const opened = await connection.request(
|
||||
'thread/start',
|
||||
@@ -170,13 +221,21 @@ export async function generateAndSetCodexConversationName(
|
||||
{ timeoutMs }
|
||||
)
|
||||
const namingThreadId = readCodexThreadId(opened)
|
||||
// Never the user's own thread: an app-server that ignored `ephemeral` would
|
||||
// otherwise have this turn's prompt and JSON answer land in their transcript
|
||||
// and their history, which is the one outcome this whole path exists to avoid.
|
||||
// An app-server that ignored `ephemeral` hands back a NEW PERSISTED thread,
|
||||
// not the user's — so the id check below is not what protects them; the
|
||||
// delete in the finally block is. The check covers only a reply that names
|
||||
// the session's own thread, which would put this turn straight into the
|
||||
// user's chat.
|
||||
if (!namingThreadId || namingThreadId === input.threadId) {
|
||||
return null
|
||||
}
|
||||
input.retainNamingThread(namingThreadId)
|
||||
// Only when `ephemeral` was NOT honoured. A truly ephemeral thread refuses
|
||||
// deletion ("thread is not persisted and cannot be deleted"), so attempting
|
||||
// it unconditionally would log a failure on every successful naming.
|
||||
if (!readCodexThreadIsEphemeral(opened)) {
|
||||
disposableThreadId = namingThreadId
|
||||
}
|
||||
await connection.request(
|
||||
'turn/start',
|
||||
{
|
||||
@@ -190,6 +249,14 @@ export async function generateAndSetCodexConversationName(
|
||||
answer = await collector.answer
|
||||
} finally {
|
||||
input.closeNamingTurn()
|
||||
// Set only when the app-server persisted the thread despite `ephemeral`.
|
||||
// Without this, every named chat would leave a junk thread and rollout file
|
||||
// in the user's Codex history that Orca never shows and never reclaims.
|
||||
if (disposableThreadId) {
|
||||
await connection
|
||||
.request('thread/delete', { threadId: disposableThreadId }, { timeoutMs })
|
||||
.catch((error: unknown) => input.onError?.('delete-naming-thread', error))
|
||||
}
|
||||
}
|
||||
const title = readCodexGeneratedTitle(answer)
|
||||
if (!title) {
|
||||
@@ -202,7 +269,10 @@ export async function generateAndSetCodexConversationName(
|
||||
{ threadId: input.threadId },
|
||||
{ timeoutMs }
|
||||
)
|
||||
if (readCodexThreadName(current)) {
|
||||
// Fails CLOSED. Only a reply this build can positively read as unnamed permits
|
||||
// the write: a shape it does not recognise would otherwise read as "unnamed"
|
||||
// and clobber a name a person chose. Skipping a name is a non-event.
|
||||
if (!isCodexThreadReadablyUnnamed(current)) {
|
||||
return null
|
||||
}
|
||||
await connection.request(
|
||||
|
||||
@@ -2,12 +2,16 @@
|
||||
// the answer goes. The generation flow itself lives beside this.
|
||||
|
||||
import type { AgentJournalMessageItem } from '../../shared/agent-session-journal-types'
|
||||
import { agentSessionNamingPromptText } from '../native-chat/agent-session-wire/agent-session-naming-prompt-text'
|
||||
import {
|
||||
createCodexNamingTurnCollector,
|
||||
generateAndSetCodexConversationName
|
||||
} from './codex-conversation-name-generation'
|
||||
import { agentSessionNamingPromptText } from '../native-chat/agent-session-wire/agent-session-naming-prompt-text'
|
||||
import type { CodexSession } from './codex-structured-session-state'
|
||||
import { readCodexThreadId, readCodexThreadName } from './codex-structured-thread-facts'
|
||||
import type {
|
||||
CodexSession,
|
||||
CodexStructuredSessionAdapterDeps
|
||||
} from './codex-structured-session-state'
|
||||
|
||||
/** A naming turn outlives no session: past this the chat keeps its placeholder. */
|
||||
const NAMING_TURN_TIMEOUT_MS = 60_000
|
||||
@@ -18,41 +22,56 @@ export type CodexConversationNamingInput = {
|
||||
body: AgentJournalMessageItem
|
||||
requestTimeoutMs?: number
|
||||
onConversationName?: (sessionId: string, conversationName: string) => void
|
||||
/** Durable "we already asked", so an eviction or restart does not re-ask. */
|
||||
readNamingAttempted?: (sessionId: string) => boolean
|
||||
markNamingAttempted?: (sessionId: string) => void
|
||||
onError?: (scope: string, error: unknown) => void
|
||||
}
|
||||
|
||||
/**
|
||||
* Names the thread once per session, off the turn's critical path.
|
||||
* Names the thread once, off the turn's critical path.
|
||||
*
|
||||
* One attempt only: a thread the model declined to name, or one a person
|
||||
* deliberately cleared, must not be re-asked on every later turn.
|
||||
* Asked at most once per CONVERSATION, not once per session object: the durable
|
||||
* marker means a thread the model declined to name, and a name a person
|
||||
* deliberately cleared, are not re-asked after an eviction or a restart.
|
||||
*
|
||||
* Everything runs inside the promise, including reading the user's text: this
|
||||
* sits on the send path, and nothing here may turn a delivered message into a
|
||||
* reported failure.
|
||||
*/
|
||||
export function startCodexConversationNaming(input: CodexConversationNamingInput): void {
|
||||
const { session, sessionId } = input
|
||||
if (session.namingAttempted || session.conversationName || !input.onConversationName) {
|
||||
return
|
||||
}
|
||||
const prompt = agentSessionNamingPromptText(input.body)
|
||||
if (!prompt) {
|
||||
return
|
||||
}
|
||||
session.namingAttempted = true
|
||||
void generateAndSetCodexConversationName({
|
||||
connection: session.connection,
|
||||
cwd: session.cwd,
|
||||
threadId: session.threadId,
|
||||
prompt,
|
||||
...(input.requestTimeoutMs ? { timeoutMs: input.requestTimeoutMs } : {}),
|
||||
openNamingTurn: () => {
|
||||
const collector = createCodexNamingTurnCollector(NAMING_TURN_TIMEOUT_MS)
|
||||
session.naming = collector
|
||||
return collector
|
||||
},
|
||||
retainNamingThread: (namingThreadId) => session.namingThreadIds.add(namingThreadId),
|
||||
closeNamingTurn: () => {
|
||||
session.naming = null
|
||||
}
|
||||
})
|
||||
.then((name) => {
|
||||
void Promise.resolve()
|
||||
.then(async () => {
|
||||
if (input.readNamingAttempted?.(sessionId)) {
|
||||
return
|
||||
}
|
||||
const prompt = agentSessionNamingPromptText(input.body)
|
||||
if (!prompt) {
|
||||
return
|
||||
}
|
||||
input.markNamingAttempted?.(sessionId)
|
||||
const name = await generateAndSetCodexConversationName({
|
||||
connection: session.connection,
|
||||
cwd: session.cwd,
|
||||
threadId: session.threadId,
|
||||
prompt,
|
||||
...(input.requestTimeoutMs ? { timeoutMs: input.requestTimeoutMs } : {}),
|
||||
...(input.onError ? { onError: input.onError } : {}),
|
||||
openNamingTurn: () => {
|
||||
const collector = createCodexNamingTurnCollector(NAMING_TURN_TIMEOUT_MS)
|
||||
session.naming = collector
|
||||
return collector
|
||||
},
|
||||
retainNamingThread: (namingThreadId) => session.namingThreadIds.add(namingThreadId),
|
||||
closeNamingTurn: () => {
|
||||
session.naming = null
|
||||
}
|
||||
})
|
||||
// `thread/name/set` echoes back as `thread/name/updated`, but only while
|
||||
// this session still holds the connection; report directly so a name set
|
||||
// just before a close is not lost.
|
||||
@@ -61,8 +80,44 @@ export function startCodexConversationNaming(input: CodexConversationNamingInput
|
||||
input.onConversationName?.(sessionId, name)
|
||||
}
|
||||
})
|
||||
// A thread with no name is the state this started in; never surface it.
|
||||
.catch(() => {
|
||||
.catch((error: unknown) => {
|
||||
session.naming = null
|
||||
input.onError?.('codex-conversation-naming', error)
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Records a name Codex reported for THIS session's thread, or the clearing of it.
|
||||
*
|
||||
* Codex broadcasts `thread/name/updated` for every thread it has stored, so a
|
||||
* frame naming another thread must not relabel this chat. A frame for this thread
|
||||
* carrying no name is a deletion: leaving the old one would keep rendering a name
|
||||
* the user removed.
|
||||
*/
|
||||
export function captureCodexConversationName(
|
||||
sessionId: string,
|
||||
session: CodexSession,
|
||||
method: string,
|
||||
params: unknown,
|
||||
deps: Pick<CodexStructuredSessionAdapterDeps, 'onConversationName' | 'onConversationNameCleared'>
|
||||
): void {
|
||||
if (method !== 'thread/name/updated') {
|
||||
return
|
||||
}
|
||||
if ((readCodexThreadId(params) ?? session.threadId) !== session.threadId) {
|
||||
return
|
||||
}
|
||||
const conversationName = readCodexThreadName(params)
|
||||
if (!conversationName) {
|
||||
if (session.conversationName !== null) {
|
||||
session.conversationName = null
|
||||
deps.onConversationNameCleared?.(sessionId)
|
||||
}
|
||||
return
|
||||
}
|
||||
if (conversationName === session.conversationName) {
|
||||
return
|
||||
}
|
||||
session.conversationName = conversationName
|
||||
deps.onConversationName?.(sessionId, conversationName)
|
||||
}
|
||||
|
||||
@@ -36,8 +36,11 @@ import {
|
||||
deliverCodexUnhandledFrame
|
||||
} from './codex-structured-provider-events'
|
||||
import { isCodexNamingFrame } from './codex-conversation-name-generation'
|
||||
import { startCodexConversationNaming } from './codex-conversation-name-turn'
|
||||
import { readCodexThreadId, readCodexThreadName } from './codex-structured-thread-facts'
|
||||
import {
|
||||
captureCodexConversationName,
|
||||
startCodexConversationNaming
|
||||
} from './codex-conversation-name-turn'
|
||||
import { readCodexThreadId } from './codex-structured-thread-facts'
|
||||
import { CodexStructuredTurnCancellation } from './codex-structured-turn-cancellation'
|
||||
import { createCodexStructuredNotificationRetry } from './codex-structured-notification-retry'
|
||||
import { acquireCodexStructuredSession } from './codex-structured-session-acquire'
|
||||
@@ -121,41 +124,19 @@ export class CodexStructuredSessionAdapter implements StructuredAgentSessionAdap
|
||||
if (this.turnCancellation.handleNotification(sessionId, session, method, params)) {
|
||||
return { accepted: true }
|
||||
}
|
||||
// Before every other handler: a naming turn runs on a throwaway thread over
|
||||
// this same connection, and the item translator journals items from ANY
|
||||
// Before anything can journal it: a naming turn runs on a throwaway thread
|
||||
// over this same connection, and the item translator journals items from ANY
|
||||
// thread. Routed here, its prompt and its JSON answer never reach the chat.
|
||||
if (isCodexNamingFrame(session, readCodexThreadId(params))) {
|
||||
session.naming?.handle(method, params)
|
||||
return { accepted: true }
|
||||
}
|
||||
this.captureConversationName(sessionId, session, method, params)
|
||||
captureCodexConversationName(sessionId, session, method, params, this.deps)
|
||||
return deliverCodexNotification(sessionId, session, method, params, (current, event) =>
|
||||
this.emit(current, event)
|
||||
)
|
||||
}
|
||||
|
||||
/** Codex broadcasts `thread/name/updated` for every thread it has stored, so a
|
||||
* frame naming another thread must not relabel this session's chat. */
|
||||
private captureConversationName(
|
||||
sessionId: string,
|
||||
session: CodexSession,
|
||||
method: string,
|
||||
params: unknown
|
||||
): void {
|
||||
if (method !== 'thread/name/updated') {
|
||||
return
|
||||
}
|
||||
if ((readCodexThreadId(params) ?? session.threadId) !== session.threadId) {
|
||||
return
|
||||
}
|
||||
const conversationName = readCodexThreadName(params)
|
||||
if (!conversationName || conversationName === session.conversationName) {
|
||||
return
|
||||
}
|
||||
session.conversationName = conversationName
|
||||
this.deps.onConversationName?.(sessionId, conversationName)
|
||||
}
|
||||
|
||||
/** Journal first so observers never see an event ahead of its durable row. */
|
||||
private emit(
|
||||
session: CodexSession,
|
||||
@@ -225,7 +206,14 @@ export class CodexStructuredSessionAdapter implements StructuredAgentSessionAdap
|
||||
...(this.deps.requestTimeoutMs ? { requestTimeoutMs: this.deps.requestTimeoutMs } : {}),
|
||||
...(this.deps.onConversationName
|
||||
? { onConversationName: this.deps.onConversationName }
|
||||
: {})
|
||||
: {}),
|
||||
...(this.deps.readNamingAttempted
|
||||
? { readNamingAttempted: this.deps.readNamingAttempted }
|
||||
: {}),
|
||||
...(this.deps.markNamingAttempted
|
||||
? { markNamingAttempted: this.deps.markNamingAttempted }
|
||||
: {}),
|
||||
...(this.deps.onNamingError ? { onError: this.deps.onNamingError } : {})
|
||||
})
|
||||
}
|
||||
return outcome
|
||||
|
||||
@@ -188,7 +188,10 @@ const USER_TURN = {
|
||||
blocks: [{ type: 'text', text: 'fix the flaky lease probe' }]
|
||||
} as const
|
||||
|
||||
async function dispatchedAdapter(codex: ReturnType<typeof namingCodex>) {
|
||||
async function dispatchedAdapter(
|
||||
codex: ReturnType<typeof namingCodex>,
|
||||
naming: { readNamingAttempted?: () => boolean; markNamingAttempted?: () => void } = {}
|
||||
) {
|
||||
const onConversationName = vi.fn()
|
||||
const events: unknown[] = []
|
||||
const adapter = new CodexStructuredSessionAdapter({
|
||||
@@ -202,7 +205,8 @@ async function dispatchedAdapter(codex: ReturnType<typeof namingCodex>) {
|
||||
openConnection: codex.openConnection,
|
||||
readProcessStartTime: async () => 1_700_000_000_000,
|
||||
onEvent: (event) => events.push(event),
|
||||
onConversationName
|
||||
onConversationName,
|
||||
...naming
|
||||
})
|
||||
await adapter.acquire({ identity, fence: 7, spawnToken: 'spawn-9' })
|
||||
await adapter.dispatch({
|
||||
@@ -249,6 +253,30 @@ describe('Codex conversation-name generation', () => {
|
||||
expect(JSON.stringify(events)).not.toContain('Fix lease probe')
|
||||
})
|
||||
|
||||
it('asks only once across a re-acquisition, which builds a NEW session', async () => {
|
||||
// A second acquisition rebuilds the session object, so the in-memory flag
|
||||
// resets. Only the durable marker stops the user's next message paying for
|
||||
// a second naming turn — and re-imposing a name they may have cleared.
|
||||
let attempted = false
|
||||
const naming = {
|
||||
readNamingAttempted: () => attempted,
|
||||
markNamingAttempted: () => {
|
||||
attempted = true
|
||||
}
|
||||
}
|
||||
const first = namingCodex({ answer: 'I could not think of one' })
|
||||
await dispatchedAdapter(first, naming)
|
||||
await settle()
|
||||
const second = namingCodex({ answer: 'I could not think of one' })
|
||||
await dispatchedAdapter(second, naming)
|
||||
await settle()
|
||||
|
||||
const ephemeralStarts = (codex: ReturnType<typeof namingCodex>) =>
|
||||
codex.calls.filter((call) => call.method === 'thread/start' && call.params.ephemeral === true)
|
||||
expect(ephemeralStarts(first)).toHaveLength(1)
|
||||
expect(ephemeralStarts(second)).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('asks only once per session, even when the first attempt produced no name', async () => {
|
||||
// A model that declines to answer leaves `conversationName` null, so the
|
||||
// one-shot flag is the ONLY thing stopping a second attempt. With a name set
|
||||
|
||||
@@ -45,6 +45,11 @@ export type CodexStructuredSessionAdapterDeps = {
|
||||
onEvent?: (event: CodexStructuredSessionEvent) => void
|
||||
/** Codex named (or renamed) the thread behind this session. */
|
||||
onConversationName?: (sessionId: string, conversationName: string) => void
|
||||
/** Codex reports the thread has no name any more. */
|
||||
onConversationNameCleared?: (sessionId: string) => void
|
||||
readNamingAttempted?: (sessionId: string) => boolean
|
||||
markNamingAttempted?: (sessionId: string) => void
|
||||
onNamingError?: (scope: string, error: unknown) => void
|
||||
openConnection?: typeof openCodexAppServerConnection
|
||||
readProcessStartTime?: (pid: number) => Promise<number | null>
|
||||
mintLinkId?: () => string
|
||||
@@ -76,8 +81,9 @@ export type CodexSession = {
|
||||
/** Every throwaway thread this session opened for naming. Retained for the
|
||||
* session's life: an abandoned turn is never cancelled and can still emit. */
|
||||
namingThreadIds: Set<string>
|
||||
/** One naming attempt per session: a thread the model declined to name, or one
|
||||
* a person deliberately cleared, must not be re-asked on every later turn. */
|
||||
/** Guards a SECOND attempt within this live session only. The durable answer
|
||||
* to "have we asked" lives on the record; this just fences concurrent sends
|
||||
* before that write lands. */
|
||||
namingAttempted: boolean
|
||||
prompts: CodexAcquisitionWindow['prompts']
|
||||
options: Map<string, string>
|
||||
|
||||
@@ -42,3 +42,20 @@ describe('agentSessionNamingPromptText', () => {
|
||||
expect(text).toHaveLength(2_000)
|
||||
})
|
||||
})
|
||||
|
||||
describe('agentSessionNamingPromptText hostile input', () => {
|
||||
it.each([
|
||||
['absent blocks', {}],
|
||||
['null blocks', { blocks: null }],
|
||||
['a non-array blocks', { blocks: 'fix the probe' }],
|
||||
['an absent body', undefined]
|
||||
])('reports null rather than throwing on %s', (_label, body) => {
|
||||
// This runs on the send path. Only the RPC send schema guarantees an array;
|
||||
// journal-replay and resend callers do not pass through it, and a throw here
|
||||
// would turn a delivered message into a reported failure.
|
||||
expect(() =>
|
||||
agentSessionNamingPromptText(body as unknown as AgentJournalMessageItem)
|
||||
).not.toThrow()
|
||||
expect(agentSessionNamingPromptText(body as unknown as AgentJournalMessageItem)).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -11,6 +11,12 @@ const MAX_NAMING_PROMPT_LENGTH = 2_000
|
||||
* path would leak a filesystem location into a generated label.
|
||||
*/
|
||||
export function agentSessionNamingPromptText(body: AgentJournalMessageItem): string | null {
|
||||
// `blocks` reaches here as provider/journal data, not something the type system
|
||||
// verified: only the RPC send path runs it through a schema. A non-array here
|
||||
// would throw on the send path and turn a delivered message into a failed one.
|
||||
if (!Array.isArray(body?.blocks)) {
|
||||
return null
|
||||
}
|
||||
const text = (body.blocks as NativeChatBlock[])
|
||||
.filter((block): block is Extract<NativeChatBlock, { type: 'text' }> => block.type === 'text')
|
||||
.map((block) => block.text)
|
||||
|
||||
+86
-30
@@ -5,73 +5,129 @@ import { StructuredAgentSessionConversationNames } from './structured-agent-sess
|
||||
const SESSION = 'session-1'
|
||||
|
||||
function harness(
|
||||
options: {
|
||||
stored?: string | undefined
|
||||
setConversationName?: (
|
||||
sessionId: string,
|
||||
conversationName: string,
|
||||
now: number
|
||||
) => Promise<AgentSessionRecord>
|
||||
} = {}
|
||||
stored: { conversationName?: string; conversationNamingAttempted?: boolean } = {},
|
||||
options: { failWrites?: boolean } = {}
|
||||
) {
|
||||
const stored = { conversationName: options.stored }
|
||||
const setConversationName =
|
||||
options.setConversationName ??
|
||||
vi.fn(async (_sessionId: string, conversationName: string) => {
|
||||
stored.conversationName = conversationName
|
||||
const record = { ...stored }
|
||||
const applyConversationNaming = vi.fn(
|
||||
async (_sessionId: string, change: { conversationName?: string | null; attempted?: true }) => {
|
||||
if (options.failWrites) {
|
||||
throw new Error('agent_session_identity_required')
|
||||
}
|
||||
if (change.conversationName === null) {
|
||||
delete record.conversationName
|
||||
} else if (change.conversationName !== undefined) {
|
||||
record.conversationName = change.conversationName
|
||||
}
|
||||
if (change.attempted) {
|
||||
record.conversationNamingAttempted = true
|
||||
}
|
||||
return {} as AgentSessionRecord
|
||||
})
|
||||
}
|
||||
)
|
||||
const onChanged = vi.fn()
|
||||
const onError = vi.fn()
|
||||
const names = new StructuredAgentSessionConversationNames({
|
||||
store: {
|
||||
getRecord: () => stored as AgentSessionRecord,
|
||||
setConversationName: setConversationName as never
|
||||
getRecord: () => record as AgentSessionRecord,
|
||||
applyConversationNaming: applyConversationNaming as never
|
||||
},
|
||||
now: () => 5,
|
||||
onChanged
|
||||
onChanged,
|
||||
onError
|
||||
})
|
||||
return { names, onChanged, setConversationName, stored }
|
||||
return { names, onChanged, onError, applyConversationNaming, record }
|
||||
}
|
||||
|
||||
describe('StructuredAgentSessionConversationNames', () => {
|
||||
it('persists a published name and announces the change once', async () => {
|
||||
const { names, onChanged, setConversationName, stored } = harness()
|
||||
const { names, onChanged, record } = harness()
|
||||
|
||||
await names.publish(SESSION, ' Fix the\nlease probe ')
|
||||
|
||||
expect(setConversationName).toHaveBeenCalledWith(SESSION, 'Fix the lease probe', 5)
|
||||
expect(stored.conversationName).toBe('Fix the lease probe')
|
||||
expect(record.conversationName).toBe('Fix the lease probe')
|
||||
expect(onChanged).toHaveBeenCalledExactlyOnceWith(SESSION, 'Fix the lease probe')
|
||||
})
|
||||
|
||||
it('does not rewrite or re-announce a name the record already holds', async () => {
|
||||
const { names, onChanged, setConversationName } = harness({ stored: 'Fix the lease probe' })
|
||||
const { names, onChanged, applyConversationNaming } = harness({
|
||||
conversationName: 'Fix the lease probe'
|
||||
})
|
||||
|
||||
await names.publish(SESSION, 'Fix the lease probe')
|
||||
|
||||
expect(setConversationName).not.toHaveBeenCalled()
|
||||
expect(applyConversationNaming).not.toHaveBeenCalled()
|
||||
expect(onChanged).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('ignores a report that carries no usable name', async () => {
|
||||
const { names, onChanged, setConversationName } = harness()
|
||||
const { names, onChanged, applyConversationNaming } = harness()
|
||||
|
||||
await names.publish(SESSION, '')
|
||||
await names.publish(SESSION, null)
|
||||
await names.publish(SESSION, { name: 'nope' })
|
||||
|
||||
expect(setConversationName).not.toHaveBeenCalled()
|
||||
expect(applyConversationNaming).not.toHaveBeenCalled()
|
||||
expect(onChanged).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps a store failure off the caller and announces nothing', async () => {
|
||||
const { names, onChanged } = harness({
|
||||
setConversationName: vi.fn(async () => {
|
||||
throw new Error('agent_session_identity_required')
|
||||
})
|
||||
it('clears a name the provider says is gone, and announces the clear', async () => {
|
||||
const { names, onChanged, record } = harness({ conversationName: 'Fix the lease probe' })
|
||||
|
||||
await names.clear(SESSION)
|
||||
|
||||
// A name the user deleted elsewhere must not linger here and keep rendering.
|
||||
expect(record.conversationName).toBeUndefined()
|
||||
expect(onChanged).toHaveBeenCalledExactlyOnceWith(SESSION, null)
|
||||
})
|
||||
|
||||
it('does nothing when clearing a conversation that has no name', async () => {
|
||||
const { names, onChanged, applyConversationNaming } = harness()
|
||||
|
||||
await names.clear(SESSION)
|
||||
|
||||
expect(applyConversationNaming).not.toHaveBeenCalled()
|
||||
expect(onChanged).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('records the attempt durably, and reads it back', async () => {
|
||||
const { names, record } = harness()
|
||||
|
||||
expect(names.read(SESSION).namingAttempted).toBe(false)
|
||||
await names.markAttempted(SESSION)
|
||||
|
||||
expect(record.conversationNamingAttempted).toBe(true)
|
||||
expect(names.read(SESSION).namingAttempted).toBe(true)
|
||||
})
|
||||
|
||||
it('reads the stored name and attempted marker together', () => {
|
||||
const { names } = harness({
|
||||
conversationName: 'Fix the lease probe',
|
||||
conversationNamingAttempted: true
|
||||
})
|
||||
|
||||
expect(names.read(SESSION)).toEqual({
|
||||
conversationName: 'Fix the lease probe',
|
||||
namingAttempted: true
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps a store failure off the caller, announces nothing, and reports it', async () => {
|
||||
const { names, onChanged, onError } = harness({}, { failWrites: true })
|
||||
|
||||
await expect(names.publish(SESSION, 'Fix the lease probe')).resolves.toBeUndefined()
|
||||
|
||||
expect(onChanged).not.toHaveBeenCalled()
|
||||
// Silent to the user, never silent to the log: a host whose store refuses
|
||||
// the write must not look like a model that declined to name anything.
|
||||
expect(onError).toHaveBeenCalledWith('apply-name', expect.any(Error))
|
||||
})
|
||||
|
||||
it('reports a failed attempt marker rather than swallowing it', async () => {
|
||||
const { names, onError } = harness({}, { failWrites: true })
|
||||
|
||||
await names.markAttempted(SESSION)
|
||||
|
||||
expect(onError).toHaveBeenCalledWith('mark-attempted', expect.any(Error))
|
||||
})
|
||||
})
|
||||
|
||||
+63
-14
@@ -1,42 +1,91 @@
|
||||
// Where a provider's name for a conversation becomes the structured chat's label.
|
||||
//
|
||||
// Providers PUSH: Codex reports the thread's name when it opens one and again on
|
||||
// every rename, and Claude's name is read out of its transcript once a session
|
||||
// is live. Nothing polls, so the name has one path in and one way out — the
|
||||
// durable record — and only a name that actually CHANGED notifies. A re-read on
|
||||
// every attach must not re-publish an unchanged label.
|
||||
// every rename or clear, and Claude's name is read out of its transcript once a
|
||||
// session is live. Nothing polls, so the name has one path in and one way out —
|
||||
// the durable record.
|
||||
//
|
||||
// The record is also where "we already asked" lives. Both providers rebuild their
|
||||
// session object on every acquisition, so an in-memory flag would re-ask after
|
||||
// every eviction: re-imposing a name the user cleared, and paying for it again.
|
||||
//
|
||||
// Nothing here may fail an attach or a turn: the name is display metadata, so a
|
||||
// store write that loses a race with a close is dropped rather than retried.
|
||||
// write that loses a race with a close is logged and dropped, never retried.
|
||||
|
||||
import { normalizeAgentSessionConversationName } from '../../../shared/agent-session-conversation-name'
|
||||
import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store'
|
||||
|
||||
export type StructuredAgentSessionNamingState = {
|
||||
conversationName: string | null
|
||||
namingAttempted: boolean
|
||||
}
|
||||
|
||||
export type StructuredAgentSessionConversationNameDeps = {
|
||||
store: Pick<AgentSessionRecordStore, 'getRecord' | 'setConversationName'>
|
||||
store: Pick<AgentSessionRecordStore, 'getRecord' | 'applyConversationNaming'>
|
||||
now: () => number
|
||||
onChanged: (sessionId: string, conversationName: string) => void
|
||||
onChanged: (sessionId: string, conversationName: string | null) => void
|
||||
/** Diagnostics only. Naming never surfaces to the user, but a host whose
|
||||
* app-server refuses to name threads must not be indistinguishable from a
|
||||
* model that simply declined. */
|
||||
onError?: (scope: string, error: unknown) => void
|
||||
}
|
||||
|
||||
export class StructuredAgentSessionConversationNames {
|
||||
constructor(private readonly deps: StructuredAgentSessionConversationNameDeps) {}
|
||||
|
||||
/** Records a name a provider published. Ignores anything that is not a usable name. */
|
||||
/** What a provider needs to know before deciding whether to ask for a name. */
|
||||
read = (sessionId: string): StructuredAgentSessionNamingState => {
|
||||
const record = this.deps.store.getRecord(sessionId)
|
||||
return {
|
||||
conversationName: record?.conversationName ?? null,
|
||||
namingAttempted: record?.conversationNamingAttempted === true
|
||||
}
|
||||
}
|
||||
|
||||
/** Records a name a provider published, or clears it when the provider says so. */
|
||||
publish = async (sessionId: string, reported: unknown): Promise<void> => {
|
||||
const conversationName = normalizeAgentSessionConversationName(reported)
|
||||
if (!conversationName) {
|
||||
return
|
||||
}
|
||||
// Read first so an unchanged name costs no durable transaction and no fan-out.
|
||||
if (this.deps.store.getRecord(sessionId)?.conversationName === conversationName) {
|
||||
await this.apply(sessionId, { conversationName }, conversationName)
|
||||
}
|
||||
|
||||
/** The provider reports this conversation has no name any more. */
|
||||
clear = async (sessionId: string): Promise<void> => {
|
||||
if (this.read(sessionId).conversationName === null) {
|
||||
return
|
||||
}
|
||||
await this.apply(sessionId, { conversationName: null }, null)
|
||||
}
|
||||
|
||||
/** Durably marks that a naming attempt happened, so no later session repeats it. */
|
||||
markAttempted = async (sessionId: string): Promise<void> => {
|
||||
if (this.read(sessionId).namingAttempted) {
|
||||
return
|
||||
}
|
||||
try {
|
||||
await this.deps.store.setConversationName(sessionId, conversationName, this.deps.now())
|
||||
} catch {
|
||||
// The record is gone or reconciling. A label is never worth surfacing a failure for.
|
||||
await this.deps.store.applyConversationNaming(sessionId, { attempted: true }, this.deps.now())
|
||||
} catch (error) {
|
||||
this.deps.onError?.('mark-attempted', error)
|
||||
}
|
||||
}
|
||||
|
||||
private apply = async (
|
||||
sessionId: string,
|
||||
change: { conversationName: string | null },
|
||||
next: string | null
|
||||
): Promise<void> => {
|
||||
// Read first so an unchanged name costs no durable transaction and no fan-out.
|
||||
if (this.read(sessionId).conversationName === next) {
|
||||
return
|
||||
}
|
||||
this.deps.onChanged(sessionId, conversationName)
|
||||
try {
|
||||
await this.deps.store.applyConversationNaming(sessionId, change, this.deps.now())
|
||||
} catch (error) {
|
||||
this.deps.onError?.('apply-name', error)
|
||||
return
|
||||
}
|
||||
this.deps.onChanged(sessionId, next)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -329,8 +329,9 @@ export class StructuredAgentSessionHost {
|
||||
) => this.backgroundTasks.publish(sessionId, state)
|
||||
unsubscribe = (sessionId: string, id: string): void => this.subscribers.close(sessionId, id)
|
||||
|
||||
/** Re-projects one session after its RECORD changed; journal writes publish themselves. */
|
||||
republishStatus = (sessionId: string): void => this.statusFeed.publish(sessionId)
|
||||
/** The provider's name for one conversation, for a surface publishing its tab. */
|
||||
readConversationName = (sessionId: string): string | null =>
|
||||
this.deps.store.getRecord(sessionId)?.conversationName ?? null
|
||||
|
||||
/** Every session's projected status for session lists; unlike `subscribe`, retains nothing. */
|
||||
subscribeStatus = (
|
||||
|
||||
@@ -49,7 +49,11 @@ describe('agent session conversation name', () => {
|
||||
const store = await AgentSessionRecordStore.open({ directory, hostId: 'local' })
|
||||
await store.reserveOwner(request())
|
||||
|
||||
await store.setConversationName(SESSION, 'Fix the lease probe', NOW + 1)
|
||||
await store.applyConversationNaming(
|
||||
SESSION,
|
||||
{ conversationName: 'Fix the lease probe' },
|
||||
NOW + 1
|
||||
)
|
||||
|
||||
const reopened = await AgentSessionRecordStore.open({ directory, hostId: 'local' })
|
||||
expect(reopened.getRecord(SESSION)?.conversationName).toBe('Fix the lease probe')
|
||||
@@ -58,9 +62,17 @@ describe('agent session conversation name', () => {
|
||||
it('leaves the record untouched when the name it is given is the stored one', async () => {
|
||||
const store = await AgentSessionRecordStore.open({ directory, hostId: 'local' })
|
||||
await store.reserveOwner(request())
|
||||
const named = await store.setConversationName(SESSION, 'Fix the lease probe', NOW + 1)
|
||||
const named = await store.applyConversationNaming(
|
||||
SESSION,
|
||||
{ conversationName: 'Fix the lease probe' },
|
||||
NOW + 1
|
||||
)
|
||||
|
||||
const again = await store.setConversationName(SESSION, 'Fix the lease probe', NOW + 2)
|
||||
const again = await store.applyConversationNaming(
|
||||
SESSION,
|
||||
{ conversationName: 'Fix the lease probe' },
|
||||
NOW + 2
|
||||
)
|
||||
|
||||
expect(again.updatedAt).toBe(named.updatedAt)
|
||||
})
|
||||
@@ -68,9 +80,9 @@ describe('agent session conversation name', () => {
|
||||
it('refuses a name for a session that has no record', async () => {
|
||||
const store = await AgentSessionRecordStore.open({ directory, hostId: 'local' })
|
||||
|
||||
await expect(store.setConversationName('missing-session', 'name', NOW)).rejects.toThrow(
|
||||
'agent_session_identity_required'
|
||||
)
|
||||
await expect(
|
||||
store.applyConversationNaming('missing-session', { conversationName: 'name' }, NOW)
|
||||
).rejects.toThrow('agent_session_identity_required')
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -1,18 +1,57 @@
|
||||
import type { AgentSessionRecord } from '../../shared/agent-session-record'
|
||||
|
||||
/**
|
||||
* Records the provider's name for a conversation.
|
||||
* Records the provider's name for a conversation, or clears it.
|
||||
*
|
||||
* Unfenced on purpose: a name is display metadata, not ownership, so a reader
|
||||
* that learned it must not have to win the lease to keep it. An unchanged name
|
||||
* that learned it must not have to win the lease to keep it. An unchanged value
|
||||
* is returned as-is, so a re-read costs no durable write.
|
||||
*
|
||||
* `null` clears: a user who deletes the name in another client must not have it
|
||||
* linger here and keep rendering.
|
||||
*/
|
||||
export function setAgentSessionRecordConversationName(
|
||||
record: AgentSessionRecord,
|
||||
conversationName: string,
|
||||
conversationName: string | null,
|
||||
now: number
|
||||
): AgentSessionRecord {
|
||||
if (conversationName === null) {
|
||||
if (record.conversationName === undefined) {
|
||||
return record
|
||||
}
|
||||
const { conversationName: _cleared, ...rest } = record
|
||||
return { ...rest, updatedAt: now }
|
||||
}
|
||||
return record.conversationName === conversationName
|
||||
? record
|
||||
: { ...record, conversationName, updatedAt: now }
|
||||
}
|
||||
|
||||
/** Marks that a naming attempt has been made, so no later session repeats it. */
|
||||
export function markAgentSessionRecordConversationNamingAttempted(
|
||||
record: AgentSessionRecord,
|
||||
now: number
|
||||
): AgentSessionRecord {
|
||||
return record.conversationNamingAttempted === true
|
||||
? record
|
||||
: { ...record, conversationNamingAttempted: true, updatedAt: now }
|
||||
}
|
||||
|
||||
/** What one naming update changes: the name, the attempted marker, or both. */
|
||||
export type AgentSessionConversationNamingChange = {
|
||||
/** A string sets the name; `null` clears it; omitted leaves it alone. */
|
||||
conversationName?: string | null
|
||||
attempted?: true
|
||||
}
|
||||
|
||||
export function applyAgentSessionRecordConversationNaming(
|
||||
record: AgentSessionRecord,
|
||||
change: AgentSessionConversationNamingChange,
|
||||
now: number
|
||||
): AgentSessionRecord {
|
||||
const named =
|
||||
change.conversationName === undefined
|
||||
? record
|
||||
: setAgentSessionRecordConversationName(record, change.conversationName, now)
|
||||
return change.attempted ? markAgentSessionRecordConversationNamingAttempted(named, now) : named
|
||||
}
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
/** Durable single-writer session records and their operation ledger. */
|
||||
|
||||
import {
|
||||
agentSessionOperationKey,
|
||||
settleAgentSessionOperation,
|
||||
type AgentSessionOperationDecision,
|
||||
type AgentSessionOperationOutcome,
|
||||
@@ -47,7 +46,10 @@ import {
|
||||
isAgentSessionClaimKeyVerifiable,
|
||||
retireAgentSessionClaimKey
|
||||
} from './agent-session-claim-key-retention'
|
||||
import { setAgentSessionRecordConversationName } from './agent-session-record-conversation-name'
|
||||
import {
|
||||
applyAgentSessionRecordConversationNaming,
|
||||
type AgentSessionConversationNamingChange
|
||||
} from './agent-session-record-conversation-name'
|
||||
import {
|
||||
listVisibleAgentSessionIds,
|
||||
setAgentSessionTabVisibility,
|
||||
@@ -59,10 +61,7 @@ import {
|
||||
type AgentSessionReservationProcesslessProof
|
||||
} from './agent-session-processless-reservation'
|
||||
import {
|
||||
admitPendingAgentSessionReservationReplay,
|
||||
applyAgentSessionReservation,
|
||||
evaluateAgentSessionReserveOperation,
|
||||
requireAgentSessionRecordForReplay,
|
||||
commitAgentSessionReservation,
|
||||
type AgentSessionReserveRequest,
|
||||
type AgentSessionReserveResult
|
||||
} from './agent-session-reservation-admission'
|
||||
@@ -162,26 +161,9 @@ export class AgentSessionRecordStore {
|
||||
* operation returns the recorded outcome and never reaches the reservation.
|
||||
*/
|
||||
async reserveOwner(request: AgentSessionReserveRequest): Promise<AgentSessionReserveResult> {
|
||||
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(
|
||||
@@ -310,13 +292,13 @@ export class AgentSessionRecordStore {
|
||||
replaceSessionOptions = (args: AgentSessionOptionsReplacement): Promise<AgentSessionRecord> =>
|
||||
this.mutate(args.sessionId, (record) => replaceAgentSessionRecordOptions(record, args))
|
||||
|
||||
setConversationName = (
|
||||
applyConversationNaming = (
|
||||
sessionId: string,
|
||||
conversationName: string,
|
||||
change: AgentSessionConversationNamingChange,
|
||||
now: number
|
||||
): Promise<AgentSessionRecord> =>
|
||||
this.mutate(sessionId, (record) =>
|
||||
setAgentSessionRecordConversationName(record, conversationName, now)
|
||||
applyAgentSessionRecordConversationNaming(record, change, now)
|
||||
)
|
||||
|
||||
async retireClaimKey(keyId: string, now: number): Promise<void> {
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
* the result inside one transaction; nothing here mutates.
|
||||
*/
|
||||
|
||||
import { agentSessionOperationKey } from '../../shared/agent-session-operation-ledger'
|
||||
import {
|
||||
evaluateAgentSessionOperation,
|
||||
pruneAgentSessionOperationRows,
|
||||
@@ -216,3 +217,35 @@ function createAgentSessionRecord(
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The whole reservation transition: adjudicate the operation, replay a recorded
|
||||
* outcome, or apply a fresh reservation and record its ledger row.
|
||||
*
|
||||
* Lives beside the pieces it orchestrates rather than in the store, which owns
|
||||
* durability and serialization rather than reservation policy.
|
||||
*/
|
||||
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 }
|
||||
}
|
||||
|
||||
@@ -49,6 +49,10 @@ export class OrcaRuntimeWithRestoreStructuredAgentSessionTabsOnce extends OrcaRu
|
||||
if (session.agent !== 'codex' && session.agent !== 'claude') {
|
||||
continue
|
||||
}
|
||||
// Belt-and-braces: a session id cannot contain ':' (SESSION_ID_PATTERN in
|
||||
// agent-session-record), so a prefixed key cannot reach the host's map.
|
||||
// Stripped here rather than at the lookup so the tab id and the record it
|
||||
// is labelled from can never disagree.
|
||||
let sessionId = session.sessionId
|
||||
while (sessionId.startsWith('agent-session:')) {
|
||||
sessionId = sessionId.slice('agent-session:'.length)
|
||||
@@ -75,10 +79,18 @@ export class OrcaRuntimeWithRestoreStructuredAgentSessionTabsOnce extends OrcaRu
|
||||
if (typeof host?.setSessionTabVisibility === 'function') {
|
||||
await host.setSessionTabVisibility(input.sessionId, true)
|
||||
}
|
||||
// Every publication labels itself from the record, so a revealed or reopened
|
||||
// chat shows the name it already has. Only the startup sweep passes a title,
|
||||
// and an unchanged name never fans out, so nothing else would repair it.
|
||||
const title =
|
||||
input.title?.trim() ||
|
||||
(typeof host?.readConversationName === 'function'
|
||||
? (host.readConversationName(input.sessionId) ?? '')
|
||||
: '')
|
||||
const existing = this.mobileSessionTabsByWorktree.get(input.workspaceId)
|
||||
const id = `agent-session:${input.sessionId}`
|
||||
if (existing?.tabs.some((tab) => tab.id === id)) {
|
||||
const conversationName = input.title?.trim()
|
||||
const conversationName = title
|
||||
if (!input.activate) {
|
||||
// Republishing an already-open tab is how a restored session hands over
|
||||
// the name it was persisted with; the rest of the snapshot is unchanged.
|
||||
@@ -121,7 +133,7 @@ export class OrcaRuntimeWithRestoreStructuredAgentSessionTabsOnce extends OrcaRu
|
||||
const tab: RuntimeMobileSessionAgentTab = {
|
||||
type: 'agent-session',
|
||||
id,
|
||||
title: input.title?.trim() || defaultAgentChatLabel(input.agent),
|
||||
title: title || defaultAgentChatLabel(input.agent),
|
||||
sessionId: input.sessionId,
|
||||
agent: input.agent,
|
||||
isActive: input.activate
|
||||
|
||||
@@ -462,6 +462,48 @@ describe('structured session cold restoration', () => {
|
||||
expect(again.snapshotVersion).toBe(settled.snapshotVersion)
|
||||
})
|
||||
|
||||
it('labels a revealed chat with the name its record already holds', async () => {
|
||||
const runtime = new OrcaRuntimeService()
|
||||
// Reveal, re-open and create publish no title. Only the startup sweep did,
|
||||
// and resume reports the same name so the unchanged-name short circuit means
|
||||
// nothing ever repairs the label afterwards.
|
||||
setStructuredAgentSessionHost({
|
||||
setSessionTabVisibility: async () => undefined,
|
||||
readConversationName: () => 'Fix the lease probe'
|
||||
} as never)
|
||||
|
||||
await runtime.publishStructuredAgentSessionTab({
|
||||
workspaceId: 'workspace-1',
|
||||
sessionId: 'revealed-codex',
|
||||
agent: 'codex',
|
||||
activate: true
|
||||
})
|
||||
|
||||
const snapshot = await runtime.listMobileSessionTabs('id:workspace-1')
|
||||
expect(snapshot.tabs[0]).toMatchObject({
|
||||
id: 'agent-session:revealed-codex',
|
||||
title: 'Fix the lease probe'
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps the placeholder for a chat whose record holds no name', async () => {
|
||||
const runtime = new OrcaRuntimeService()
|
||||
setStructuredAgentSessionHost({
|
||||
setSessionTabVisibility: async () => undefined,
|
||||
readConversationName: () => null
|
||||
} as never)
|
||||
|
||||
await runtime.publishStructuredAgentSessionTab({
|
||||
workspaceId: 'workspace-1',
|
||||
sessionId: 'unnamed-codex',
|
||||
agent: 'codex',
|
||||
activate: true
|
||||
})
|
||||
|
||||
const snapshot = await runtime.listMobileSessionTabs('id:workspace-1')
|
||||
expect(snapshot.tabs[0]).toMatchObject({ title: 'Codex Chat' })
|
||||
})
|
||||
|
||||
it('commits the host close when the renderer already removed the structured tab', async () => {
|
||||
const runtime = new OrcaRuntimeService()
|
||||
runtime.setNotifier({
|
||||
|
||||
@@ -81,7 +81,8 @@ export type StructuredAgentSessionRuntimeDeps = {
|
||||
onConversationName?: (input: {
|
||||
sessionId: string
|
||||
workspaceId: string
|
||||
conversationName: string
|
||||
/** Null when the provider cleared the name; the tab returns to its placeholder. */
|
||||
conversationName: string | null
|
||||
}) => void
|
||||
handoffTransport?: StructuredAgentSessionHandoffTransport
|
||||
reapOrphanChildren?: typeof stopOrphanAgentSessionChildren
|
||||
@@ -224,12 +225,15 @@ async function install(deps: StructuredAgentSessionRuntimeDeps): Promise<Install
|
||||
store,
|
||||
now: () => Date.now(),
|
||||
onChanged: (sessionId, conversationName) => {
|
||||
host?.republishStatus(sessionId)
|
||||
// The tab snapshot is the only carrier: the status summary has no name
|
||||
// field, so re-projecting it would broadcast a byte-identical summary.
|
||||
const workspaceId = store.getRecord(sessionId)?.location.workspaceId
|
||||
if (workspaceId) {
|
||||
deps.onConversationName?.({ sessionId, workspaceId, conversationName })
|
||||
}
|
||||
}
|
||||
},
|
||||
onError: (scope, error) =>
|
||||
console.warn(`[agent-session] conversation naming failed (${scope})`, error)
|
||||
})
|
||||
const codex = new CodexStructuredSessionAdapter({
|
||||
resolveLaunch: createCodexStructuredLaunchResolver({
|
||||
@@ -242,6 +246,11 @@ async function install(deps: StructuredAgentSessionRuntimeDeps): Promise<Install
|
||||
...(deps.readProcessStartTime ? { readProcessStartTime: deps.readProcessStartTime } : {}),
|
||||
onConversationName: (sessionId, conversationName) =>
|
||||
void conversationNames.publish(sessionId, conversationName),
|
||||
onConversationNameCleared: (sessionId) => void conversationNames.clear(sessionId),
|
||||
readNamingAttempted: (sessionId) => conversationNames.read(sessionId).namingAttempted,
|
||||
markNamingAttempted: (sessionId) => void conversationNames.markAttempted(sessionId),
|
||||
onNamingError: (scope, error) =>
|
||||
console.warn(`[agent-session] conversation naming failed (${scope})`, error),
|
||||
onEvent: (event) => {
|
||||
if (event.type !== 'ended' || !('cause' in event) || event.cause !== 'unexpected-exit') {
|
||||
return
|
||||
@@ -285,6 +294,10 @@ async function install(deps: StructuredAgentSessionRuntimeDeps): Promise<Install
|
||||
host?.publishBackgroundTaskState(sessionId, state),
|
||||
onConversationName: (sessionId, conversationName) =>
|
||||
void conversationNames.publish(sessionId, conversationName),
|
||||
readNamingState: (sessionId) => conversationNames.read(sessionId),
|
||||
markNamingAttempted: (sessionId) => void conversationNames.markAttempted(sessionId),
|
||||
onNamingError: (scope, error) =>
|
||||
console.warn(`[agent-session] conversation naming failed (${scope})`, error),
|
||||
...(deps.openClaudeConnection ? { openClaudeConnection: deps.openClaudeConnection } : {}),
|
||||
...(deps.readProcessStartTime ? { readProcessStartTime: deps.readProcessStartTime } : {})
|
||||
})
|
||||
|
||||
@@ -36,6 +36,12 @@ export type StructuredClaudeRuntimeAdapterDeps = {
|
||||
state: AgentSessionBackgroundTaskState | null
|
||||
) => void
|
||||
onConversationName?: (sessionId: string, conversationName: string) => void
|
||||
readNamingState?: (sessionId: string) => {
|
||||
conversationName: string | null
|
||||
namingAttempted: boolean
|
||||
}
|
||||
markNamingAttempted?: (sessionId: string) => void
|
||||
onNamingError?: (scope: string, error: unknown) => void
|
||||
}
|
||||
|
||||
export function createStructuredClaudeRuntimeAdapter(
|
||||
@@ -80,7 +86,9 @@ export function createStructuredClaudeRuntimeAdapter(
|
||||
: null
|
||||
},
|
||||
...(deps.onConversationName ? { onConversationName: deps.onConversationName } : {}),
|
||||
readConversationName: (sessionId) => store.getRecord(sessionId)?.conversationName ?? null,
|
||||
...(deps.readNamingState ? { readNamingState: deps.readNamingState } : {}),
|
||||
...(deps.markNamingAttempted ? { markNamingAttempted: deps.markNamingAttempted } : {}),
|
||||
...(deps.onNamingError ? { onNamingError: deps.onNamingError } : {}),
|
||||
readTranscriptConversationName: async ({ providerSessionId, claudeConfigDir }) => {
|
||||
const transcriptPath = await resolveSessionFilePath('claude', providerSessionId, {
|
||||
claudeProjectsDir: join(claudeConfigDir, 'projects')
|
||||
|
||||
@@ -129,6 +129,11 @@ export type AgentSessionRecord = {
|
||||
/** Name the PROVIDER gave this conversation. A user's own rename lives on the
|
||||
* client tab and always outranks it; nothing here may overwrite that. */
|
||||
conversationName?: string
|
||||
/** Set once Orca has asked a provider to name this conversation. Durable on
|
||||
* purpose: the session object is rebuilt on every acquisition, so an eviction
|
||||
* or a restart would otherwise re-ask — re-imposing a name the user cleared,
|
||||
* and paying for it again. */
|
||||
conversationNamingAttempted?: boolean
|
||||
launchArgs?: AgentSessionLaunchArgs
|
||||
lease: AgentSessionLease
|
||||
createdAt: number
|
||||
@@ -341,6 +346,8 @@ export function isAgentSessionRecord(value: unknown): value is AgentSessionRecor
|
||||
(record.options === undefined || isAgentSessionOptions(record.options)) &&
|
||||
(record.conversationName === undefined ||
|
||||
isAgentSessionConversationName(record.conversationName)) &&
|
||||
(record.conversationNamingAttempted === undefined ||
|
||||
typeof record.conversationNamingAttempted === 'boolean') &&
|
||||
(record.launchArgs === undefined || isAgentSessionLaunchArgs(record.launchArgs)) &&
|
||||
!Object.hasOwn(record, 'launchEnv') &&
|
||||
isAgentSessionLease(record.lease) &&
|
||||
|
||||
Reference in New Issue
Block a user