From 10bb708eb7402f693bab40702a3cf60ad508a784 Mon Sep 17 00:00:00 2001 From: imccyu <276526105+imccyu@users.noreply.github.com> Date: Mon, 27 Jul 2026 05:07:59 +0800 Subject: [PATCH] fix: review-bot findings on the provider-hosted shell - Keep ConversationSession mounted for blank sessions (chrome-less) so the draft-persistence mirror stays bound in the hero; hero typing reaches the chat store again. - Restore the baselines-ready gate in AppFrame: empty boot snapshots no longer flash the New Workspace hero before either baseline lands. - Commit ordinary sends through the machine (send-committed event + Shell.commitSend): undo can no longer resurrect already-sent content on the default-sink path. - Give the production InputMachine a real wall clock so the typing-run merge window actually expires. - Coalesce concurrent connectWorkspace creates per workspace: the summary has no cwd until the host frame lands, so a second New Session inside that window minted a duplicate hidden blank session. --- .../runtime/src/client/workspaces/service.ts | 63 ++++++++++++++++++- .../src/client/input/contract.ts | 2 + .../src/client/input/facade.ts | 13 +++- .../ui-conversation/src/client/input/hub.ts | 3 +- .../src/client/input/machine.ts | 14 +++++ .../src/client/skeleton/ConversationRoot.tsx | 6 +- .../ui-conversation/tests/skeleton.spec.tsx | 8 +-- .../client/ui-layout/src/client/AppFrame.tsx | 30 ++++++--- .../client/ui-layout/tests/app-frame.spec.tsx | 10 ++- 9 files changed, 128 insertions(+), 21 deletions(-) diff --git a/packages/client/runtime/src/client/workspaces/service.ts b/packages/client/runtime/src/client/workspaces/service.ts index 31d71bb3c9..c4a01fe664 100644 --- a/packages/client/runtime/src/client/workspaces/service.ts +++ b/packages/client/runtime/src/client/workspaces/service.ts @@ -27,6 +27,10 @@ export class WorkspacesService { readonly list: SnapshotStore /** Workspace baseline and frame owner. */ private readonly manager: WorkspaceManager + /** In-flight blank-session creates keyed by workspace (connectWorkspace coalescing). */ + private readonly connecting = new Map>() + /** Guards the runtime-owned one-shot initial-selection subscription. */ + private initialSelectionStarted = false /** * @param ctx - client root context. @@ -59,6 +63,11 @@ export class WorkspacesService { async connectWorkspace(workspaceId: WorkspaceId): Promise { const workspace = this.list.getSnapshot().items.find(item => item.workspaceId === workspaceId) if (workspace === undefined) throw new Error(`workspaces.connectWorkspace: unknown workspace ${workspaceId}`) + // Coalesce concurrent connects: a create's summary lands without cwd + // until the host frame arrives, so a second call inside that window + // would miss the reuse scan and mint another hidden blank session. + const inflight = this.connecting.get(workspaceId) + if (inflight !== undefined) return inflight // Reuse: blank && same canonical cwd (workspace.path is the host realpath // canon; summary cwd is the session header passthrough of the same canon). const sessions = this.sessions.list.getSnapshot() @@ -66,7 +75,59 @@ export class WorkspacesService { const summary = sessions.byId[id] if (summary !== undefined && summary.blank && summary.cwd === workspace.path) return summary.id } - return this.sessions.create({ workspaceId }) + const attempt = this.sessions.create({ workspaceId }) + .finally(() => { this.connecting.delete(workspaceId) }) + this.connecting.set(workspaceId, attempt) + return attempt + } + + /** + * Follow the first complete Workspace/Session baseline and select a default + * session exactly once. A restored current session wins; otherwise the most + * recent Workspace is connected (reusing or creating its blank session). + * Later explicit clears stay cleared instead of retriggering this startup + * policy. A failed connect may retry on the next baseline projection. + * @returns disposer for the baseline subscription; late work cannot navigate after disposal. + */ + startInitialSelection(): () => void { + if (this.initialSelectionStarted) { + throw new Error('workspaces.startInitialSelection: already started') + } + this.initialSelectionStarted = true + let state: 'waiting' | 'connecting' | 'done' = 'waiting' + let disposed = false + const reconcile = (): void => { + if (disposed || state !== 'waiting') return + const workspace = this.list.getSnapshot() + if (!workspace.baselinesReady) return + const current = this.sessions.list.getSnapshot().current + const target = workspace.recentWorkspaceId + if (current !== undefined || target === undefined) { + state = 'done' + return + } + state = 'connecting' + void this.connectWorkspace(target).then( + (sessionId) => { + if (disposed) return + if (this.sessions.list.getSnapshot().current === undefined) { + this.sessions.open(sessionId) + } + state = 'done' + }, + (reason: unknown) => { + if (disposed) return + state = 'waiting' + console.warn('initial workspace selection failed:', reason) + }, + ) + } + const unsubscribe = this.list.subscribe(reconcile) + reconcile() + return () => { + disposed = true + unsubscribe() + } } /** diff --git a/packages/client/ui-conversation/src/client/input/contract.ts b/packages/client/ui-conversation/src/client/input/contract.ts index 3361f8f1e1..75a0e6b8e4 100644 --- a/packages/client/ui-conversation/src/client/input/contract.ts +++ b/packages/client/ui-conversation/src/client/input/contract.ts @@ -250,6 +250,8 @@ export type InputEvent = | { readonly type: 'adjudicated'; readonly attempt: SubmitAttempt; readonly outcome: PickOutcome } | { readonly type: 'adjudication-failed'; readonly attempt: SubmitAttempt; readonly message: string } | { readonly type: 'submit-settled'; readonly attempt: SubmitAttempt; readonly ok: boolean; readonly outcome?: SubmitOutcome; readonly message?: string } + /** An ordinary (default-sink) send was accepted: clear the draft as a COMMIT — undo must not resurrect sent content (mirrors the command submit-settled success arm). */ + | { readonly type: 'send-committed' } | { readonly type: 'release' } /** diff --git a/packages/client/ui-conversation/src/client/input/facade.ts b/packages/client/ui-conversation/src/client/input/facade.ts index 0530f2ecaa..f3f6dd7451 100644 --- a/packages/client/ui-conversation/src/client/input/facade.ts +++ b/packages/client/ui-conversation/src/client/input/facade.ts @@ -71,7 +71,9 @@ export class SessionInputShell implements SessionInput { submit: (mode) => { this.submit(mode) }, } - private readonly core = new InputMachine() + // Real wall clock: the typing-run merge window must actually expire in + // production (the machine's no-clock default is a constant for pure tests). + private readonly core = new InputMachine({ now: () => Date.now() }) private noticeSeq = 0 private lastDraft = '' private disposed = false @@ -95,6 +97,15 @@ export class SessionInputShell implements SessionInput { this.run(this.core.dispatch({ type: 'draft-changed', draft: text, ...(editRange !== undefined ? { editRange } : {}) })) } + /** + * Clear the draft as a successful-send commit: no undo unit is recorded and + * the undo history is cut, so Ctrl/Cmd-Z cannot resurrect sent content + * (the command path gets the same discipline from submit-settled success). + */ + commitSend(): void { + this.run(this.core.dispatch({ type: 'send-committed' })) + } + /** * Insert a newline at the selection as one machine transaction (the * execCommand path is gone — a second undo history would fork). diff --git a/packages/client/ui-conversation/src/client/input/hub.ts b/packages/client/ui-conversation/src/client/input/hub.ts index 93e0b6b411..2ae474be31 100644 --- a/packages/client/ui-conversation/src/client/input/hub.ts +++ b/packages/client/ui-conversation/src/client/input/hub.ts @@ -116,7 +116,8 @@ export class InputHub implements InputService { private sink(session: Session, text: string, mode: 'queue' | 'steer'): void { if (text === '') return const shell = this.shells.get(session.sessionId) - shell?.setDraft('') + // Commit, not an editable clear: undo must not resurrect sent content. + shell?.commitSend() void session.prompt([{ type: 'text', text }], mode).then( (result) => { if (!result.ok && shell?.snapshot.draft === '') shell.setDraft(text) diff --git a/packages/client/ui-conversation/src/client/input/machine.ts b/packages/client/ui-conversation/src/client/input/machine.ts index 8567ea4ad8..6d039fd4cd 100644 --- a/packages/client/ui-conversation/src/client/input/machine.ts +++ b/packages/client/ui-conversation/src/client/input/machine.ts @@ -167,6 +167,7 @@ export class InputMachine { case 'adjudicated': return this.onAdjudicated(ev.attempt, ev.outcome) case 'adjudication-failed': return this.onAdjudicationFailed(ev.attempt, ev.message) case 'submit-settled': return this.onSubmitSettled(ev) + case 'send-committed': return this.onSendCommitted() case 'release': return this.onRelease() default: return unreachable(ev) } @@ -542,6 +543,19 @@ export class InputMachine { return [{ type: 'notice', level: 'error', text }] } + /** Ordinary send accepted: clear as a commit (no undo unit; sent content + * must not be resurrectable — same discipline as submit-settled success). */ + private onSendCommitted(): InputEffect[] { + this.claim = undefined + this.occurrences = [] + this.adopt('') + this.log = [] + this.redoStack = [] + this.typingRun = undefined + this.paste = undefined + return [] + } + private onRelease(): InputEffect[] { if (this.inflight !== undefined) { this.inflight.controller.abort() diff --git a/packages/client/ui-conversation/src/client/skeleton/ConversationRoot.tsx b/packages/client/ui-conversation/src/client/skeleton/ConversationRoot.tsx index c62290c36f..5f9e91a049 100644 --- a/packages/client/ui-conversation/src/client/skeleton/ConversationRoot.tsx +++ b/packages/client/ui-conversation/src/client/skeleton/ConversationRoot.tsx @@ -77,7 +77,11 @@ export function ConversationRoot({ return (
- {!hero && renderSlot('conversation.session', {})} + {/* Mounted for every real session, hero included: ConversationSession + renders no chrome while blank but owns the draft-persistence mirror + bind — unmounting it in the hero would lose pre-first-send text on + a refresh or scope rebuild. */} + {sessionId !== undefined && renderSlot('conversation.session', {})} {renderSlotChain( 'conversation.composer', { interactions: pending }, diff --git a/packages/client/ui-conversation/tests/skeleton.spec.tsx b/packages/client/ui-conversation/tests/skeleton.spec.tsx index 0e3337d4ae..623ee93202 100644 --- a/packages/client/ui-conversation/tests/skeleton.spec.tsx +++ b/packages/client/ui-conversation/tests/skeleton.spec.tsx @@ -162,12 +162,12 @@ describe('ConversationRoot resident composer', () => { // Hero chrome present, view ring absent. expect(b.view.getByText("Let's start building")).toBeTruthy() expect(b.view.queryByTestId('view-chat')).toBeNull() - // The same machine-backed textarea is live in the hero. The chat-store - // mirror binds with ConversationSession (unmounted in hero), so the - // draft's truth here is the machine itself. + // The same machine-backed textarea is live in the hero, and the + // persistence mirror stays bound (ConversationSession mounts chrome-less + // for blank sessions): hero typing reaches the chat store. const box = b.view.getByRole('textbox') fireEvent.change(box, { target: { value: 'draft in hero' } }) - expect((box as HTMLTextAreaElement).value).toBe('draft in hero') + expect(b.chat.store.getSnapshot().draft).toBe('draft in hero') // Picker: open through the chip; a pick switches to the other // workspace's blank session (draft carry is apply-layer wiring). fireEvent.click(b.view.getByRole('button', { name: 'Choose workspace' })) diff --git a/packages/client/ui-layout/src/client/AppFrame.tsx b/packages/client/ui-layout/src/client/AppFrame.tsx index b234de4547..6dcf97d397 100644 --- a/packages/client/ui-layout/src/client/AppFrame.tsx +++ b/packages/client/ui-layout/src/client/AppFrame.tsx @@ -85,7 +85,12 @@ export function AppFrame({ useStore, actions, renderSlot, + useWorkspaces, }: AppFrameProps) { + // Baseline gate: before both object-layer baselines land, empty snapshots + // are indistinguishable from a genuine no-session state — rendering the + // conversation shell then would flash the New Workspace hero on boot. + const baselinesReady = useWorkspaces(s => s.baselinesReady) const panels = useStore((s) => s) const frameRef = useRef(null) const [viewport, setViewport] = useState(() => window.innerWidth) @@ -151,13 +156,24 @@ export function AppFrame({ width: cols.sidebar, })}
- <> - {/* Both column occupants stay at fixed tree positions. The - conversation is session-maybe; the strict details entry - naturally renders empty while no session is current. */} - {renderSlot('conversation', {})} - {renderSlot('details', {})} - + {baselinesReady + ? ( + <> + {/* Both column occupants stay at fixed tree positions. The + conversation is session-maybe; the strict details entry + naturally renders empty while no session is current. */} + {renderSlot('conversation', {})} + {renderSlot('details', {})} + + ) + : ( + <> + +
Loading workspaces and sessions…
+ + + + )} {/* The collapsed rail is fixed-width: no resize handle while closed. */} {panels.sidebar > 0 && } {cols.details > 0 && } diff --git a/packages/client/ui-layout/tests/app-frame.spec.tsx b/packages/client/ui-layout/tests/app-frame.spec.tsx index 05bd6fac19..f69eedeb80 100644 --- a/packages/client/ui-layout/tests/app-frame.spec.tsx +++ b/packages/client/ui-layout/tests/app-frame.spec.tsx @@ -160,13 +160,11 @@ describe('AppFrame', () => { expect(slotCalls.map((c) => c.key)).toContain('conversation') }) - it('renders both column occupants before baselines settle (no loading gate)', () => { - // The loading branch is gone: fixed tree positions from first paint, the - // occupants render their own pending states. + it('keeps the loading branch until both object-layer baselines are ready', () => { baselinesReady.current = false - const { slotCalls } = mountFrame() - expect(slotCalls.map((c) => c.key)).toContain('conversation') - expect(slotCalls.map((c) => c.key)).toContain('details') + const { slotCalls, getByRole } = mountFrame() + expect(getByRole('status').textContent).toContain('Loading workspaces and sessions') + expect(slotCalls.map((c) => c.key)).not.toContain('conversation') }) it('sidebar slot receives live concession output as owner props', () => {