Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions docs/design/2026-09-01-web-shell-clear-manual-title.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Preserve manual titles across `/clear`

`/clear` creates a deferred successor session. Remember the current title only
when its persisted provenance is `manual`, then rename the successor before it
attaches and before its first prompt.

The existing session catalog is the durable source of title provenance after a
reload. Live rename events provide the same provenance without another read.
`/new`, `/reset`, session navigation, and workspace changes discard the carry.

Automatic and legacy titles with unknown provenance are never carried.
93 changes: 91 additions & 2 deletions packages/acp-bridge/src/bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28997,15 +28997,104 @@ describe('createAcpSessionBridge', () => {
(e) => e.type === 'session_metadata_updated',
);
expect(metaEvent).toBeDefined();
expect((metaEvent?.data as { displayName: string }).displayName).toBe(
'Test Session',
expect(metaEvent?.data).toMatchObject({
displayName: 'Test Session',
titleSource: 'manual',
});

await bridge.closeSession(session.sessionId);
await drain;
await bridge.shutdown();
});

it('uses automatic provenance for programmatic renames', async () => {
const titleUpdates: unknown[] = [];
const bridge = makeBridge({
channelFactory: async () =>
makeChannel({
extMethodImpl: (method, params) => {
if (method === SERVE_CONTROL_EXT_METHODS.sessionTitle) {
titleUpdates.push(params);
}
return { persisted: true };
},
}).channel,
});
const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A });
const events: BridgeEvent[] = [];
const sub = bridge.subscribeEvents(session.sessionId);
const drain = (async () => {
for await (const event of sub) events.push(event);
})();
await new Promise((resolve) => setImmediate(resolve));

bridge.updateSessionMetadata(session.sessionId, {
displayName: 'Voice chat',
titleSource: 'auto',
});

await vi.waitFor(() => expect(titleUpdates).toHaveLength(1));
expect(titleUpdates[0]).toMatchObject({
displayName: 'Voice chat',
titleSource: 'auto',
});
await vi.waitFor(() =>
expect(
events.find((event) => event.type === 'session_metadata_updated')
?.data,
).toMatchObject({
displayName: 'Voice chat',
titleSource: 'auto',
}),
);

await bridge.closeSession(session.sessionId);
await drain;
await bridge.shutdown();
});

it('rejects an empty displayName instead of clearing only the live entry', async () => {
const bridge = makeBridge({
channelFactory: async () => makeChannel().channel,
});
const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A });

bridge.updateSessionMetadata(session.sessionId, {
displayName: 'Payments bug',
});

const events: BridgeEvent[] = [];
const sub = bridge.subscribeEvents(session.sessionId);
const drain = (async () => {
for await (const ev of sub) events.push(ev);
})();
await new Promise((r) => setImmediate(r));

// A clear is never persisted (the `sessionTitle` persist skips
// falsy names), so accepting it would let the stale manual record
// resurface through the session-list merge and the `/clear` carry.
expect(() =>
bridge.updateSessionMetadata(session.sessionId, { displayName: '' }),
).toThrow(InvalidSessionMetadataError);
expect(() =>
bridge.updateSessionMetadata(session.sessionId, {
displayName: ' ',
}),
).toThrow(InvalidSessionMetadataError);

await new Promise((r) => setImmediate(r));
expect(bridge.getSessionSummary(session.sessionId)).toMatchObject({
displayName: 'Payments bug',
});
expect(
events.filter((e) => e.type === 'session_metadata_updated'),
).toHaveLength(0);

await bridge.closeSession(session.sessionId);
await drain;
await bridge.shutdown();
});

it('keeps the optimistic update and logs a generic persistence failure', async () => {
const stderrSpy = vi
.spyOn(process.stderr, 'write')
Expand Down
33 changes: 31 additions & 2 deletions packages/acp-bridge/src/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11363,6 +11363,16 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge {
}
}
if (metadata.displayName !== undefined) {
if (
metadata.titleSource !== undefined &&
metadata.titleSource !== 'manual' &&
metadata.titleSource !== 'auto'
) {
throw new InvalidSessionMetadataError(
'titleSource',
'must be either `manual` or `auto`',
);
}
if (
typeof metadata.displayName !== 'string' ||
metadata.displayName.length > MAX_DISPLAY_NAME_LENGTH
Expand All @@ -11378,7 +11388,22 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge {
'must not contain control characters',
);
}
// An empty name would only clear the live entry: the `sessionTitle`
// persist below runs for truthy names, so no tombstone reaches the
// transcript. The persisted manual record would then resurface
// through the session-list merge (`live.displayName ??
// existing.displayName`) and be carried into a `/clear` successor as
// if the clear never happened. Reject the clear instead of serving a
// name the catalog no longer backs. Mirrors the workspace-scoped
// metadata route, which rejects empty names for the same reason.
if (metadata.displayName.trim() === '') {
throw new InvalidSessionMetadataError(
'displayName',
'must not be empty',
);
}
const nextDisplayName = metadata.displayName || undefined;
const titleSource = metadata.titleSource ?? 'manual';
if (entry.displayName !== nextDisplayName) {
entry.displayName = nextDisplayName;
// The catalog exposes display names; an actual rename is a
Expand All @@ -11397,7 +11422,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge {
.extMethod(SERVE_CONTROL_EXT_METHODS.sessionTitle, {
sessionId,
displayName: nextDisplayName,
titleSource: 'manual',
titleSource,
})
.then((res: unknown) => {
const r = res as { persisted?: boolean } | undefined;
Expand All @@ -11418,7 +11443,11 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge {
try {
entry.events.publish({
type: 'session_metadata_updated',
data: { sessionId, displayName: entry.displayName },
data: {
sessionId,
displayName: entry.displayName,
...(entry.displayName ? { titleSource } : {}),
},
Comment thread
yiliang114 marked this conversation as resolved.
...(metadataOriginatorClientId
? { originatorClientId: metadataOriginatorClientId }
: {}),
Expand Down
2 changes: 2 additions & 0 deletions packages/acp-bridge/src/bridgeTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,7 @@ export interface BridgeSessionSummary {
createdAt: string;
updatedAt?: string;
displayName?: string;
titleSource?: 'manual' | 'auto';
/** Id of the session that spawned this one (via `create_sub_session`), or
* absent for a top-level session. Lets a UI link a sub-session back to its
* parent. Immutable — set when the session is created. */
Expand Down Expand Up @@ -811,6 +812,7 @@ export interface SessionPrIssueInfo {

export interface SessionMetadataUpdate {
displayName?: string;
titleSource?: 'manual' | 'auto';
/** Issues are daemon-derived, never client-bound — the input omits them. */
pr?: Omit<SessionPrInfo, 'issues'>;
/** Full binding list after the update (return value only; ignored on input). */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,9 @@ function toStandaloneSummary(
createdAt: item.startTime,
updatedAt: new Date(item.mtime).toISOString(),
...(displayName ? { displayName } : {}),
...(item.customTitle && item.titleSource
? { titleSource: item.titleSource }
: {}),
sourceType: STANDALONE_SESSION_SOURCE_TYPE,
context: { kind: 'standalone' },
...(source.metadata.parentSessionId !== undefined
Expand Down
15 changes: 12 additions & 3 deletions packages/cli/src/serve/create-sub-session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,11 @@ function makeFakeBridge(opts?: {
}> = [];
const prompts: Array<{ sessionId: string; promptId?: string; text: string }> =
[];
const names: Array<{ sessionId: string; displayName?: string }> = [];
const names: Array<{
sessionId: string;
displayName?: string;
titleSource?: 'manual' | 'auto';
}> = [];
const closes: string[] = [];
const relocations: Array<{
sessionId: string;
Expand Down Expand Up @@ -131,9 +135,12 @@ function makeFakeBridge(opts?: {
},
updateSessionMetadata: (
sessionId: string,
metadata: { displayName?: string },
metadata: {
displayName?: string;
titleSource?: 'manual' | 'auto';
},
) => {
names.push({ sessionId, displayName: metadata.displayName });
names.push({ sessionId, ...metadata });
return metadata;
},
getSessionLastEventId: () => 0,
Expand Down Expand Up @@ -339,6 +346,7 @@ describe('sub-session launcher', () => {
]);
expect(fake.prompts[0]!.text).toBe('do the thing');
expect(fake.names[0]!.displayName).toContain('my task');
expect(fake.names[0]!.titleSource).toBe('auto');
// 'sent' returns immediately but starts a background subscription to hold
// the concurrency slot until the sub-session's turn finishes (so the cap
// stays meaningful). The subscription is fire-and-forget — the launch
Expand Down Expand Up @@ -386,6 +394,7 @@ describe('sub-session launcher', () => {
sourceId: 'scheduled_task_run:task-1',
});
expect(fake.names[0]?.displayName).toBe('Hourly review');
expect(fake.names[0]?.titleSource).toBe('auto');
});

it('rejects a scheduled-task run when prompt admission fails', async () => {
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/serve/create-sub-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -911,6 +911,7 @@ export function createSubSessionLauncher(
info.name ?? info.prompt,
!isScheduledTaskRunSource(info),
),
titleSource: 'auto',
});
} catch (err) {
log.debug('sub-session: updateSessionMetadata failed', sessionId, err);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ describe('LiveSessionCoordinator', () => {
});
expect(harness.bridge.updateSessionMetadata).toHaveBeenCalledWith(
'live-new',
{ displayName: 'Voice chat' },
{ displayName: 'Voice chat', titleSource: 'auto' },
);
expect(harness.host.setCallState).toHaveBeenLastCalledWith(1, 'listening');

Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/serve/live/live-session-coordinator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ export class LiveSessionCoordinator {
try {
context.runtime?.bridge.updateSessionMetadata(
context.coordinator.sessionId,
{ displayName: 'Voice chat' },
{ displayName: 'Voice chat', titleSource: 'auto' },
);
} catch {
/* the session remains usable when a title write fails */
Expand Down
25 changes: 21 additions & 4 deletions packages/cli/src/serve/routes/scheduled-tasks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ interface StubBridge {
ensureDefaultSessionPersisted(sessionId: string): Promise<void>;
updateSessionMetadata(
sessionId: string,
metadata: { displayName?: string },
metadata: {
displayName?: string;
titleSource?: 'manual' | 'auto';
},
): unknown;
getSessionSummary(sessionId: string): {
sessionId: string;
Expand Down Expand Up @@ -95,7 +98,11 @@ interface StubBridge {
prompts: Array<{ sessionId: string; text: string }>;
closed: string[];
persisted: string[];
named: Array<{ sessionId: string; displayName?: string }>;
named: Array<{
sessionId: string;
displayName?: string;
titleSource?: 'manual' | 'auto';
}>;
failNext: boolean;
persistenceError?: Error;
}
Expand Down Expand Up @@ -469,6 +476,7 @@ describe('scheduled-tasks routes', () => {
displayName: expect.stringMatching(
/^Review PRs · \d{2}-\d{2} \d{2}:\d{2}$/,
),
titleSource: 'auto',
});
expect(h.bridge.prompts).toHaveLength(1);
expect(h.bridge.prompts[0]).toMatchObject({ sessionId: childSessionId });
Expand Down Expand Up @@ -1416,13 +1424,18 @@ describe('scheduled-tasks routes', () => {
prompt: 'summarize the day',
});
expect(h.bridge.named).toEqual([
{ sessionId: named.body.sessionId, displayName: 'Digest' },
{
sessionId: named.body.sessionId,
displayName: 'Digest',
titleSource: 'auto',
},
]);

const unnamed = await create({ cron: '0 9 * * *', prompt: 'do the thing' });
expect(h.bridge.named[1]).toEqual({
sessionId: unnamed.body.sessionId,
displayName: 'do the thing',
titleSource: 'auto',
});
});

Expand Down Expand Up @@ -2036,7 +2049,9 @@ describe('scheduled-tasks routes', () => {
});
const id = created.body.id as string;
const sid = created.body.sessionId as string;
expect(h.bridge.named).toEqual([{ sessionId: sid, displayName: 'Old' }]);
expect(h.bridge.named).toEqual([
{ sessionId: sid, displayName: 'Old', titleSource: 'auto' },
]);

// Renaming the task re-labels its session.
const rename = await request(h.app)
Expand All @@ -2046,6 +2061,7 @@ describe('scheduled-tasks routes', () => {
expect(h.bridge.named).toContainEqual({
sessionId: sid,
displayName: 'New',
titleSource: 'auto',
});

// A bare cron edit does NOT re-touch the session name.
Expand All @@ -2060,6 +2076,7 @@ describe('scheduled-tasks routes', () => {
expect(h.bridge.named).toContainEqual({
sessionId: sid,
displayName: 'p',
titleSource: 'auto',
});
});

Expand Down
8 changes: 7 additions & 1 deletion packages/cli/src/serve/routes/scheduled-tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,10 @@ export interface ScheduledTasksSessionBridge {
* session list (rather than a bare id). Best-effort. */
updateSessionMetadata(
sessionId: string,
metadata: { displayName?: string },
metadata: {
displayName?: string;
titleSource?: 'manual' | 'auto';
},
): unknown;
getSessionSummary(sessionId: string): {
workspaceCwd: string;
Expand Down Expand Up @@ -550,6 +553,7 @@ async function dispatchTaskToFreshSession(
task.name ?? task.prompt,
triggeredAt,
),
titleSource: 'auto',
});
} catch {
// The prompt can still run with the generated session id as its label.
Expand Down Expand Up @@ -1076,6 +1080,7 @@ function registerScheduledTaskCrudRoutes(
displayName: scheduledTaskSessionName(
nameResult.value ?? prompt,
),
titleSource: 'auto',
}),
);
} catch {
Expand Down Expand Up @@ -1544,6 +1549,7 @@ function registerScheduledTaskCrudRoutes(
displayName: scheduledTaskSessionName(
updated.name ?? updated.prompt,
),
titleSource: 'auto',
});
} catch {
// non-critical — the schedule change already persisted
Expand Down
Loading
Loading