diff --git a/packages/client/ui-chat/src/client/apply.ts b/packages/client/ui-chat/src/client/apply.ts index fcbc44d9be..5f2f20cd72 100644 --- a/packages/client/ui-chat/src/client/apply.ts +++ b/packages/client/ui-chat/src/client/apply.ts @@ -297,14 +297,24 @@ export function apply(ctx: Context): void { read: () => chatScrollPositions.get(sessionId) ?? null, }, forkAt: (seq) => { + // The Workspace navigation owns the branch policy: whether this page may name the child and + // when the child becomes presentable. This page supplies the branch point and its analytics. const turn = [...chat.getSnapshot().timeline.turns.values()].find(turn => turn.end?.seq === seq) const messageId = turn?.data.get('turn-tail')?.closing?.finalNode.messageId - ctx.sessions.fork({ sessionId, atSeq: seq, increaseTitle: true, onCreated: (childId) => { - ctx.get('productAnalytics')?.track('branch_session_click', { session_id: childId, parent_session_id: sessionId, ...messageId === undefined ? {} : { parent_message_id: messageId }, click_position: 'footer' }) - } }) - .then((childId) => { ctx.uiWorkspace.openSession(childId) }) - .catch(() => { - // Fork or child-title failure leaves the source view unchanged. + void ctx.uiWorkspace.forkSession(sessionId, (childId) => { + ctx.get('productAnalytics')?.track('branch_session_click', { + session_id: childId, + parent_session_id: sessionId, + ...messageId === undefined ? {} : { parent_message_id: messageId }, + click_position: 'footer', + }) + }, { atSeq: seq, present: 'selected' }) + .catch((error: unknown) => { + // Fork or child-title failure leaves the source view unchanged, and states why on + // the Session's own composer: a refusal this page cannot retry is not silent. + ctx.uiConversation.noticeBranchRefusal( + sessionId, error, reason => t('message.branchRefused', { reason }), + ) }) }, } diff --git a/packages/client/ui-chat/tests/apply-inject.client.spec.tsx b/packages/client/ui-chat/tests/apply-inject.client.spec.tsx index 58d5a6f75f..bb4548a3f2 100644 --- a/packages/client/ui-chat/tests/apply-inject.client.spec.tsx +++ b/packages/client/ui-chat/tests/apply-inject.client.spec.tsx @@ -91,7 +91,15 @@ async function bench(initialSettings?: ChatSettings, withBrowserRegistry = true, openSession(ROOT) }), openSession, - openListedBranch: vi.fn((_childId: SessionId) => {}), + // The branch surface delegates naming and presentation to Workspace navigation. + forkSession: vi.fn(async ( + _sessionId: SessionId, + onCreated?: (childId: SessionId) => void, + _options?: { readonly atSeq?: number; readonly present?: 'selected' | 'listed' }, + ) => { + onCreated?.(ROOT) + return ROOT + }), } runtime.ctx.provide('uiWorkspace', uiWorkspace as never) const session = sessionFakeFor() @@ -251,7 +259,7 @@ describe('Chat inject API', () => { } }) - it('loads older history and forks through the Session Controller', async () => { + it('delegates the branch point, its analytics, and the child presentation to Workspace navigation', async () => { const b = await bench() const { injected } = b.chatViewApi(b.rootReference) injected.loadOlder() @@ -262,8 +270,11 @@ describe('Chat inject API', () => { const track = vi.fn() b.runtime.ctx.provide('productAnalytics', { track } as never) - const forkCreated = vi.spyOn(b.runtime.sessions, 'fork').mockImplementation(async (input) => { input.onCreated?.(ROOT); return ROOT }) injected.forkAt(17) + // The page owns only its branch point and analytics; naming and presentation belong to Workspace. + expect(b.uiWorkspace.forkSession).toHaveBeenCalledWith(ROOT, expect.any(Function) as (childId: SessionId) => void, { + atSeq: 17, present: 'selected', + }) expect(track).toHaveBeenCalledWith('branch_session_click', { session_id: ROOT, parent_session_id: ROOT, click_position: 'footer' }) await b.runtime.sessions.replaceEvents(ROOT, [ { type: 'event', event: { type: 'turn/start', seq: SessionSeq(10), time: 10, data: { turn: 1 } } }, @@ -275,20 +286,12 @@ describe('Chat inject API', () => { ]) injected.forkAt(17) expect(track).toHaveBeenLastCalledWith('branch_session_click', { session_id: ROOT, parent_session_id: ROOT, parent_message_id: 'reply', click_position: 'footer' }) - forkCreated.mockRestore() - injected.forkAt(17) - await vi.waitFor(() => { - expect(b.openSession).toHaveBeenCalledWith(ROOT) - }) - expect(b.runtime.sessions.calls).toContainEqual({ - method: 'fork', args: [{ sessionId: ROOT, atSeq: 17, increaseTitle: true, onCreated: expect.any(Function) as (childId: SessionId) => void }], - }) - - const fork = vi.spyOn(b.runtime.sessions, 'fork').mockRejectedValueOnce(new Error('fork failed')) + // A rejected branch is reported on this Session's composer; the page adds no retry of its own. + vi.spyOn(b.uiWorkspace, 'forkSession').mockRejectedValueOnce(new Error('fork failed')) injected.forkAt(18) await vi.waitFor(() => { - expect(fork).toHaveBeenCalledWith({ - sessionId: ROOT, atSeq: 18, increaseTitle: true, onCreated: expect.any(Function) as (childId: SessionId) => void, + expect(b.uiWorkspace.forkSession).toHaveBeenLastCalledWith(ROOT, expect.any(Function) as (childId: SessionId) => void, { + atSeq: 18, present: 'selected', }) }) await b.runtime.dispose() @@ -311,7 +314,7 @@ describe('Chat inject API', () => { const { injected } = b.chatViewApi(b.rootReference) // The Host's own message reaches the reader with the stable code beside it, and the Session it was // asked of is the one that shows it. - vi.spyOn(b.runtime.sessions, 'fork').mockRejectedValueOnce({ + vi.spyOn(b.uiWorkspace, 'forkSession').mockRejectedValueOnce({ rpcError: { code: 'boundary-not-committed', message: 'boundary is not committed' }, }) injected.forkAt(18) @@ -321,7 +324,7 @@ describe('Chat inject API', () => { ) }) // A rejection that named no code keeps its own message and gains no placeholder. - vi.spyOn(b.runtime.sessions, 'fork').mockRejectedValueOnce(new Error('fork child rename failed')) + vi.spyOn(b.uiWorkspace, 'forkSession').mockRejectedValueOnce(new Error('fork child rename failed')) injected.forkAt(19) await vi.waitFor(() => { expect(noticeOf(ROOT)).toBe('未能完成分支：fork child rename failed') @@ -329,29 +332,6 @@ describe('Chat inject API', () => { await b.runtime.dispose() }) - it('leaves a branch unnamed and opens it once listed while access excludes renaming the Session', async () => { - const b = await bench() - const source = vi.spyOn(b.runtime.ctx.remoteAccess, 'source') - source.mockImplementation((_kind, _id, endpoint) => createSnapshotStore(endpoint !== 'session/rename')) - const { injected } = b.chatViewApi(b.rootReference) - injected.forkAt(17) - // Naming a branch renames it, which this page may not do: asking for the rename would fail the - // fork after the Host already made the child, so the page forks an unnamed branch. - await vi.waitFor(() => { - expect(b.runtime.sessions.calls).toContainEqual({ - method: 'fork', args: [{ sessionId: ROOT, atSeq: 17, increaseTitle: false, onCreated: expect.any(Function) as (childId: SessionId) => void }], - }) - }) - // The child's own list item is what says the branch is this page's own, so the page asks the - // Workspace navigation to open it then — that service waits for the item rather than assembling - // a view of a child the page cannot yet describe. - expect(b.uiWorkspace.openListedBranch).toHaveBeenCalledWith(ROOT) - expect(b.runtime.sessions.calls).not.toContainEqual({ method: 'open', args: [ROOT] }) - expect(source).toHaveBeenCalledWith('session', ROOT, 'session/rename') - source.mockRestore() - await b.runtime.dispose() - }) - it('addresses file paths under the Session\'s scope and opens them in the right Sidebar', async () => { const b = await bench() const { injected } = b.chatViewApi(b.rootReference) diff --git a/packages/client/ui-workspace/src/client/index.ts b/packages/client/ui-workspace/src/client/index.ts index 94199e9111..d6fdfbf086 100644 --- a/packages/client/ui-workspace/src/client/index.ts +++ b/packages/client/ui-workspace/src/client/index.ts @@ -58,7 +58,7 @@ import { SessionIdMenuItem } from './session-actions/SessionId.tsx' import { WorkspacePicker } from './WorkspacePicker.tsx' import { en, ja, zh, type WorkspaceKey } from './locales.ts' -export type { StartSessionOptions, UiWorkspace } from './navigation.ts' +export type { ForkSessionOptions, StartSessionOptions, UiWorkspace } from './navigation.ts' export type { DirectoryFlowOwnerProps, DirectoryFlowSlotName, DirectoryPickingHooks, DirectoryPickingInjected, MenuOpenState, RowToast, SessionRenameTarget, SessionRowOwnerProps, UseMenuOpenState, WorkspaceBrowserInjected, diff --git a/packages/client/ui-workspace/src/client/navigation.ts b/packages/client/ui-workspace/src/client/navigation.ts index 12619113b5..637537308c 100644 --- a/packages/client/ui-workspace/src/client/navigation.ts +++ b/packages/client/ui-workspace/src/client/navigation.ts @@ -59,24 +59,18 @@ export interface UiWorkspace { options?: Pick, ): Promise /** - * Fork a Session. Pages whose access snapshot includes renaming it increment the child title without - * selecting it; other pages leave the title unchanged and open the child once it is listed. + * Fork a Session. Pages whose access snapshot includes renaming it increment the child title; other + * pages leave the title unchanged and present the child only once the Host lists it. * @param sessionId - source Session. - * @param onCreated - observer before the optional child-title update. - * @param options - optional permission to start source migration; omission preserves the Host default. + * @param onCreated - observer after creation, before the optional child-title update. + * @param options - branch boundary, migration permission, and how the child is presented. * @returns the child SessionId after creation and any permitted inherited-title increment. */ forkSession( sessionId: SessionId, onCreated?: (childId: SessionId) => void, - options?: Pick[0], 'allowMigration'>, + options?: ForkSessionOptions, ): Promise - /** - * Open a created Session once its id is known to the list. Later navigation or plugin disposal - * cancels the wait; a completed refresh without that id or a failed refresh leaves selection intact. - * @param childId - Session the Host created. - */ - openListedBranch(childId: SessionId): void /** * Resolve the Session for a new conversation in a Workspace: navigation names the first reusable * blank member — listed in the Workspace, not archived, and listing no preset or the requested one — @@ -159,6 +153,89 @@ declare module '@deepseek-ai/cordis' { } } +/** What one branch fork asks of the Workspace navigation service. */ +export interface ForkSessionOptions extends Pick[0], 'atSeq' | 'allowMigration'> { + /** + * How the caller presents the child. `selected` opens it immediately when the page may rename the + * source Session and otherwise once the Host lists it; `listed` (the default, the row-action path) + * never selects it immediately and opens it once listed only when the page could not name it. + */ + readonly present?: 'selected' | 'listed' +} + +/** Where one wait for a listed branch reveals it, and where a failed list refresh is reported. */ +interface ListedBranchNavigation { + /** Supersession signal of the page navigation this open belongs to. */ + readonly signal: AbortSignal + /** + * Reveal one listed Session as the main selection. + * @param sessionId - Session the Host listed. + * @param signal - the navigation signal; a superseded navigation must not reveal. + */ + reveal(sessionId: SessionId, signal: AbortSignal): void + /** + * Report a failed refresh that may have kept the child out of the list. + * @param error - the refresh rejection. + */ + fail(error: unknown): void +} + +/** + * Open a created Session once the Host lists it. + * + * A page that may not rename the Session it branched presents the child only once the Host's own + * summary of it is on the list; opening sooner assembles a view of a branch it cannot describe. The + * wait ends on a list change, a completed refresh, supersession, or disposal; a failed refresh is + * reported through the navigation's failure seat and leaves the selection as it was. + * @param navigation - Supersession signal plus the reveal and failure seats. + * @param sessions - Session controller holding the list and refresh. + * @param childId - Session the Host created. + */ +function openListedBranch( + navigation: ListedBranchNavigation, + sessions: ISessions, + childId: SessionId, +): void { + const { signal } = navigation + if (signal.aborted) return + let stop = (): void => {} + let settled = false + const finish = (): void => { + settled = true + stop() + signal.removeEventListener('abort', finish) + } + const open = (): void => { + if (settled || signal.aborted || sessions.list.getSnapshot().byId[childId] === undefined) return + finish() + try { + navigation.reveal(childId, signal) + } catch (error: unknown) { + // oxlint-disable-next-line typescript/no-unnecessary-condition -- reveal may synchronously abort navigation before throwing. + if (!signal.aborted) navigation.fail(error) + } + } + stop = sessions.list.subscribe(open) + signal.addEventListener('abort', finish, { once: true }) + // Registration may synchronously publish the row; always recheck after the disposer is owned. + // oxlint-disable-next-line typescript/no-unnecessary-condition -- subscribe may synchronously call open and finish. + if (settled) { + stop() + signal.removeEventListener('abort', finish) + return + } + open() + // oxlint-disable-next-line typescript/no-unnecessary-condition -- open synchronously settles a listed child. + if (settled) return + void sessions.refresh().then(() => { + open() + finish() + }, (error: unknown) => { + if (!settled && !signal.aborted) navigation.fail(error) + finish() + }) +} + /** Structured directory failure exposed to directory UI consumers. */ export class DirectoryBrowseError extends Error { override readonly name = 'DirectoryBrowseError' @@ -331,22 +408,31 @@ class UiWorkspaceService extends Service implements UiWorkspace { async forkSession( sessionId: SessionId, onCreated?: (childId: SessionId) => void, - options?: Pick[0], 'allowMigration'>, + options: ForkSessionOptions = {}, ): Promise { // Naming a branch renames the child. A page whose access snapshot excludes renaming the source // Session leaves the name to the Host — asking for it would fail the fork it just made. const namesBranch = this.ctx.remoteAccess.source('session', sessionId, 'session/rename').getSnapshot() const childId = await this.sessions.fork({ sessionId, + ...options.atSeq === undefined ? {} : { atSeq: options.atSeq }, increaseTitle: namesBranch, ...onCreated === undefined ? {} : { onCreated }, - ...options?.allowMigration === undefined ? {} : { allowMigration: options.allowMigration }, + ...options.allowMigration === undefined ? {} : { allowMigration: options.allowMigration }, }) - if (!namesBranch) { - // A page that may not rename the Session it branched presents the child only once the Host's own - // summary of it is on the list; opening sooner assembles a view of a branch it cannot describe. - this.openListedBranch(childId) + if (namesBranch) { + // A branch surface asks to land on the child its page could describe; the row action leaves the + // selection alone. + if (options.present === 'selected') this.openSession(childId) + return childId } + // A page that may not rename the Session it branched presents the child only once the Host's own + // summary of it is on the list; opening sooner assembles a view of a branch it cannot describe. + openListedBranch({ + signal: AbortSignal.any([this.ctx.layout.beginNavigation(), this.lifetime.signal]), + reveal: (listedId, signal) => { this.replaceMain(listedId, signal, 'reveal') }, + fail: (error) => { this.notify({ kind: 'createFailed', message: creationFailureMessage(error) }) }, + }, this.sessions, childId) return childId } diff --git a/packages/client/ui-workspace/tests/apply.client.spec.ts b/packages/client/ui-workspace/tests/apply.client.spec.ts index c95600c880..26c61ec049 100644 --- a/packages/client/ui-workspace/tests/apply.client.spec.ts +++ b/packages/client/ui-workspace/tests/apply.client.spec.ts @@ -1002,6 +1002,13 @@ describe('ui-workspace apply', () => { await vi.waitFor(() => { expect(b.retain).toHaveBeenCalledWith('forked', { source: 'mainView' }) }) }) + /** Fork as a branch surface does on a page that may branch but not rename the source Session. */ + async function forkReaderBranch(b: Awaited>, childId: SessionId): Promise { + provideReaderAnswers(b) + b.fork.mockResolvedValueOnce(childId) + await b.ctx.uiWorkspace.forkSession(sid('session'), undefined, { present: 'selected' }) + } + it('opens a row published synchronously during subscription and detaches the wait', async () => { const b = await bench() await b.ctx.plugin({ inject: [...inject], apply }).await() @@ -1012,7 +1019,7 @@ describe('ui-workspace apply', () => { b.setSummary('raced', { id: sid('raced') }) return stop } }) - b.ctx.uiWorkspace.openListedBranch(sid('raced')) + await forkReaderBranch(b, sid('raced')) expect(b.retain).toHaveBeenCalledWith('raced', { source: 'mainView' }) b.setSummary('raced', { title: 'later' }) expect(b.retain.mock.calls.filter(([id]) => id === 'raced')).toHaveLength(1) @@ -1026,12 +1033,12 @@ describe('ui-workspace apply', () => { b.refresh.mockImplementation(() => new Promise(() => {})) const fiber = b.ctx.plugin({ inject: [...inject], apply }) await fiber.await() - b.ctx.uiWorkspace.openListedBranch(sid('late')) + await forkReaderBranch(b, sid('late')) navigation.abort() b.setSummary('late', { id: sid('late') }) expect(b.retain).not.toHaveBeenCalledWith('late', { source: 'mainView' }) Object.assign(b.ctx.layout, { beginNavigation: () => new AbortController().signal }) - b.ctx.uiWorkspace.openListedBranch(sid('later')) + await forkReaderBranch(b, sid('later')) await fiber.dispose() b.setSummary('later', { id: sid('later') }) expect(b.retain).not.toHaveBeenCalledWith('later', { source: 'mainView' }) @@ -1041,12 +1048,12 @@ describe('ui-workspace apply', () => { const b = await bench() declare(b.slots, 'shell.overlay') await b.ctx.plugin({ inject: [...inject], apply }).await() - b.ctx.uiWorkspace.openListedBranch(sid('missing')) + await forkReaderBranch(b, sid('missing')) await Promise.resolve() b.setSummary('missing', { id: sid('missing') }) expect(b.retain).not.toHaveBeenCalledWith('missing', { source: 'mainView' }) b.refresh.mockRejectedValueOnce(new Error('list refused')) - b.ctx.uiWorkspace.openListedBranch(sid('refused')) + await forkReaderBranch(b, sid('refused')) await Promise.resolve() const toast = faceOf(entry(b.slots, 'shell.overlay', 'workspace.row-toast')) as RowToastInjected expect(toast.hooks.toast.getSnapshot()).toMatchObject({ kind: 'createFailed', message: 'list refused' }) @@ -1058,7 +1065,7 @@ describe('ui-workspace apply', () => { const navigation = new AbortController() navigation.abort() Object.assign(b.ctx.layout, { beginNavigation: () => navigation.signal }) - b.ctx.uiWorkspace.openListedBranch(sid('cancelled')) + await forkReaderBranch(b, sid('cancelled')) expect(b.refresh).not.toHaveBeenCalled() expect(b.retain).not.toHaveBeenCalledWith('cancelled', { source: 'mainView' }) }) @@ -1074,7 +1081,7 @@ describe('ui-workspace apply', () => { if (cancel) navigation.abort() throw new Error('branch opening refused') }) - b.ctx.uiWorkspace.openListedBranch(sid('broken')) + await forkReaderBranch(b, sid('broken')) const toast = faceOf(entry(b.slots, 'shell.overlay', 'workspace.row-toast')) as RowToastInjected expect(toast.hooks.toast.getSnapshot()).toEqual(cancel ? null : { kind: 'createFailed', message: 'branch opening refused', seq: 1 }) @@ -1089,7 +1096,7 @@ describe('ui-workspace apply', () => { Object.assign(b.ctx.layout, { beginNavigation: () => navigation.signal }) const refreshed = Promise.withResolvers() b.refresh.mockImplementationOnce(() => refreshed.promise) - b.ctx.uiWorkspace.openListedBranch(sid('late-refresh')) + await forkReaderBranch(b, sid('late-refresh')) if (end === 'opened') b.setSummary('late-refresh', { id: sid('late-refresh') }) else navigation.abort() refreshed.reject(new Error('late refresh refused')) diff --git a/packages/extensions/cordis-client-runner/src/client/api-catalog.ts b/packages/extensions/cordis-client-runner/src/client/api-catalog.ts index 6646483753..b7525caad1 100644 --- a/packages/extensions/cordis-client-runner/src/client/api-catalog.ts +++ b/packages/extensions/cordis-client-runner/src/client/api-catalog.ts @@ -348,9 +348,9 @@ export const SERVICE_API: readonly ServiceApiEntry[] = [ throws: ['on failure; a refused creation is also shown through the Workspace notice unless a later navigation or disposal superseded the request.'], }, { - signature: 'forkSession( sessionId: SessionId, onCreated?: (childId: SessionId) => void, options?: Pick[0], \'allowMigration\'>, ): Promise', - description: 'Fork a Session. Pages whose access snapshot includes renaming it increment the child title without selecting it; other pages leave the title unchanged and open the child once it is listed.', - parameters: [{ name: 'sessionId', description: 'source Session.' }, { name: 'onCreated', description: 'observer before the optional child-title update.' }, { name: 'options', description: 'optional permission to start source migration; omission preserves the Host default.' }], + signature: 'forkSession( sessionId: SessionId, onCreated?: (childId: SessionId) => void, options?: ForkSessionOptions, ): Promise', + description: 'Fork a Session. Pages whose access snapshot includes renaming it increment the child title; other pages leave the title unchanged and present the child only once the Host lists it.', + parameters: [{ name: 'sessionId', description: 'source Session.' }, { name: 'onCreated', description: 'observer after creation, before the optional child-title update.' }, { name: 'options', description: 'branch boundary, migration permission, and how the child is presented.' }], returns: 'the child SessionId after creation and any permitted inherited-title increment.', }, { @@ -631,6 +631,10 @@ export const TYPE_API: readonly TypeApiEntry[] = [ name: 'FontSizes', declaration: 'export type FontSizes = Readonly>;', }, + { + name: 'ForkSessionOptions', + declaration: 'export interface ForkSessionOptions extends Pick[0], \'atSeq\' | \'allowMigration\'> {\n readonly present?: \'selected\' | \'listed\';\n}', + }, { name: 'GlobalStandardProps', declaration: 'export interface GlobalStandardProps {\n}',