diff --git a/packages/acp-bridge/src/bridge.test.ts b/packages/acp-bridge/src/bridge.test.ts index eb333f6a588..458d7560ac9 100644 --- a/packages/acp-bridge/src/bridge.test.ts +++ b/packages/acp-bridge/src/bridge.test.ts @@ -1393,6 +1393,40 @@ describe('createAcpSessionBridge', () => { await bridge.shutdown(); }); + it('wraps a Goal control request in the envelope the agent reads', async () => { + // The agent's `sessionGoalControl` handler reads `params['request']`; this + // method is its only producer, and a flattened envelope makes every + // POST /session/:id/goal fail with "Invalid or missing Goal control + // request" while the route and agent tests stay green. + const snapshot = { v: 2, activity: 'idle', goal: null }; + const handle = makeChannel({ + extMethodImpl: async (method) => + method === SERVE_CONTROL_EXT_METHODS.sessionGoalControl + ? { snapshot } + : {}, + }); + const bridge = makeBridge({ channelFactory: async () => handle.channel }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + const request = { action: 'create' as const, objective: 'ship it' }; + + await expect( + bridge.controlSessionGoal(session.sessionId, request), + ).resolves.toEqual({ snapshot }); + expect(handle.agent.extMethodCalls).toContainEqual({ + method: SERVE_CONTROL_EXT_METHODS.sessionGoalControl, + params: { sessionId: session.sessionId, request }, + }); + + await expect( + bridge.controlSessionGoal( + '11111111-2222-3333-4444-555555555555', + request, + ), + ).rejects.toBeInstanceOf(SessionNotFoundError); + + await bridge.shutdown(); + }); + it('serves completed MCP status without restarting an idle channel', async () => { const makeMcpChannel = () => makeChannel({ @@ -29635,6 +29669,103 @@ describe('createAcpSessionBridge — mid-turn message queue (enqueueMidTurnMessa await bridge.shutdown(); }); + /** + * A Goal turn runs inside the child via `prompt()` directly, so the bridge + * never sees a `session/prompt` RPC for it and `pendingPromptCount` stays 0 + * for its whole duration. The child still drains this queue between tool + * batches, so the session is busy: without the `goalTurnActive` check every + * mid-turn insert during a Goal turn would be refused as idle — while the + * client enables the affordance precisely because a Goal turn is non-idle. + */ + it('accepts a rejectIfIdle insert while a child-driven Goal turn runs', async () => { + const handle = makeChannel({}); + const bridge = makeBridge({ channelFactory: async () => handle.channel }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + await handle.agentConnection.extNotification('_qwencode/start_turn', { + sessionId: session.sessionId, + source: 'goal', + }); + + expect( + bridge.enqueueMidTurnMessage( + session.sessionId, + 'insert me', + { clientId: session.clientId }, + 'goal-insert', + { rejectIfIdle: true }, + ), + ).toEqual({ accepted: true, messageId: 'goal-insert' }); + // Queued for the child's drain, NOT promoted into a prompt of its own. + expect(bridge.getPendingPrompts(session.sessionId)).toEqual([]); + expect( + bridge.getMidTurnMessages(session.sessionId, { + clientId: session.clientId, + }).messages, + ).toEqual([ + expect.objectContaining({ messageId: 'goal-insert', text: 'insert me' }), + ]); + + await bridge.shutdown(); + }); + + it('promotes what the ending Goal turn never drained', async () => { + let release: (() => void) | undefined; + const handle = makeChannel({ + promptImpl: async () => { + await new Promise((res) => { + release = res; + }); + return { stopReason: 'end_turn' }; + }, + }); + const bridge = makeBridge({ channelFactory: async () => handle.channel }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + await handle.agentConnection.extNotification('_qwencode/start_turn', { + sessionId: session.sessionId, + source: 'goal', + }); + expect( + bridge.enqueueMidTurnMessage( + session.sessionId, + 'never drained', + { clientId: session.clientId }, + 'goal-undrained', + { rejectIfIdle: true }, + ), + ).toEqual({ accepted: true, messageId: 'goal-undrained' }); + + // A Goal turn owns no prompt slot, so its end is the only signal that can + // settle what its last drain missed. + await handle.agentConnection.extNotification('_qwencode/end_turn', { + sessionId: session.sessionId, + reason: 'end_turn', + source: 'goal', + promptId: `${session.sessionId}########1`, + }); + + await vi.waitFor(() => + expect(bridge.getPendingPrompts(session.sessionId)).toEqual([ + expect.objectContaining({ + promptId: 'goal-undrained', + text: 'never drained', + }), + ]), + ); + expect( + bridge.getMidTurnMessages(session.sessionId, { + clientId: session.clientId, + }).messages, + ).toEqual([]); + + release?.(); + await vi.waitFor(() => + expect(bridge.getPendingPrompts(session.sessionId)).toEqual([]), + ); + await bridge.shutdown(); + }); + it('rejects a whitespace-only message even while busy', async () => { const { factory, release } = hangingPromptFactory(); const bridge = makeBridge({ channelFactory: factory }); @@ -30699,10 +30830,13 @@ describe('createAcpSessionBridge — mid-turn message queue (enqueueMidTurnMessa const admission = bridge.enqueueMidTurnMessage( session.sessionId, 'leftover', + { clientId: session.clientId }, + 'leftover-public', + { rejectIfIdle: true }, ); expect(admission).toEqual({ accepted: true, - messageId: expect.any(String), + messageId: 'leftover-public', }); releases[0]!(); await t1; @@ -30712,6 +30846,7 @@ describe('createAcpSessionBridge — mid-turn message queue (enqueueMidTurnMessa expect.objectContaining({ promptId: admission.messageId, text: 'leftover', + originatorClientId: session.clientId, }), ]); releases[1]!(); @@ -31100,12 +31235,16 @@ describe('createAcpSessionBridge — mid-turn message queue (enqueueMidTurnMessa .catch(() => {}); await new Promise((r) => setTimeout(r, 10)); - const admission = bridge.enqueueMidTurnMessage(session.sessionId, 'hi', { - clientId: session.clientId, - }); + const admission = bridge.enqueueMidTurnMessage( + session.sessionId, + 'hi', + { clientId: session.clientId }, + 'public-mid-turn', + { rejectIfIdle: true }, + ); expect(admission).toEqual({ accepted: true, - messageId: expect.any(String), + messageId: 'public-mid-turn', }); // Subscribe before the drain so the live injection frame is captured. The @@ -32034,6 +32173,31 @@ describe('createAcpSessionBridge — mid-turn message queue (enqueueMidTurnMessa await bridge.shutdown(); }); + it('rejects a public enqueue on idle only when rejectIfIdle is set', async () => { + let promptCalls = 0; + const handle = makeChannel({ + promptImpl: async () => { + promptCalls++; + return { stopReason: 'end_turn' }; + }, + }); + const bridge = makeBridge({ channelFactory: async () => handle.channel }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + expect( + bridge.enqueueMidTurnMessage( + session.sessionId, + 'public message', + { clientId: session.clientId }, + 'public-idle', + { rejectIfIdle: true }, + ), + ).toEqual({ accepted: false }); + expect(promptCalls).toBe(0); + expect(bridge.getPendingPrompts(session.sessionId)).toEqual([]); + await bridge.shutdown(); + }); + it('still queues a queueOnly enqueue while the session is busy', async () => { const release = deferred(); const prompts: string[] = []; diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index 5155d452ecf..70a75ce7ee8 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -1136,6 +1136,13 @@ interface SessionEntry { * an originator clientId is known. Used by the session reaper to avoid * killing sessions mid-prompt. */ promptActive: boolean; + /** + * True while a child-driven Goal turn is running. Maintained by the + * `_qwencode/start_turn` / `_qwencode/end_turn` (source `goal`) + * notifications in `BridgeClient`; OR-ed into `hasActivePrompt` + * summaries because Goal turns never flip `promptActive`. + */ + goalTurnActive?: boolean; /** Terminal error from the prior turn, cleared when the next turn starts. */ turnError?: { message: string; @@ -3619,7 +3626,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { ...(entry.sourceType ? { sourceType: entry.sourceType } : {}), ...(entry.sourceId !== undefined ? { sourceId: entry.sourceId } : {}), clientCount: entry.clientIds.size, - hasActivePrompt: entry.promptActive, + hasActivePrompt: entry.promptActive || entry.goalTurnActive === true, isWaitingForPermission, isWaitingForUserQuestion, pendingInteractionCount: entry.pendingInteractions.size, @@ -4018,6 +4025,9 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { // Child-side automatic title updates change persisted catalog // metadata the bridge never sees; forward the catalog-clock mark. markSessionCatalogChanged, + // A Goal turn drains the mid-turn queue but owns no prompt slot, so + // nothing else would settle what its last drain missed. + settleMidTurnQueueAfterGoalTurn, ); const rawConnection = new ClientSideConnection( () => @@ -6622,7 +6632,8 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { // Late attachers get the same ACP state the original restore // caller saw; spawn-only sessions don't carry a state payload. state: existing.restoreState ?? {}, - hasActivePrompt: existing.promptActive, + hasActivePrompt: + existing.promptActive || existing.goalTurnActive === true, ...replayFields, ...(historyAnchorRecordId !== undefined ? { historyAnchorRecordId } @@ -6768,7 +6779,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { attached: true, clientId, createdAt: entry.createdAt, - hasActivePrompt: entry.promptActive, + hasActivePrompt: entry.promptActive || entry.goalTurnActive === true, ...(waiterReplayFields ?? {}), }; } @@ -7213,7 +7224,8 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { ? { sourceId: racedEntry.sourceId } : {}), state: racedEntry.restoreState ?? {}, - hasActivePrompt: racedEntry.promptActive, + hasActivePrompt: + racedEntry.promptActive || racedEntry.goalTurnActive === true, ...replayFieldsFor(racedEntry, action, liveReplayMode), }; } @@ -7313,7 +7325,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { ...(artifactRestoreWarnings.length > 0 ? { artifactWarnings: artifactRestoreWarnings } : {}), - hasActivePrompt: entry.promptActive, + hasActivePrompt: entry.promptActive || entry.goalTurnActive === true, ...replayFieldsFor(entry, action, liveReplayMode), }; })().finally(async () => { @@ -7667,6 +7679,61 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { }); }; + /** + * Hand back every mid-turn message the turn that just ended never drained: + * `queueOnly` callers drive their own follow-through, everything else starts + * through the normal prompt path. + */ + const settleUndrainedMidTurnMessages = ( + entry: SessionEntry, + messages: readonly MidTurnQueueEntry[], + ) => { + for (const message of messages) { + if (message.queueOnly) { + try { + message.onSettledWithoutDrain?.(); + } catch (error) { + writeStderrLine( + `[mid-turn] session=${JSON.stringify(entry.sessionId)} failed to hand undrained queue-only message ${JSON.stringify(message.messageId)} back to its caller: ${JSON.stringify(error instanceof Error ? error.message : String(error))}`, + ); + } + continue; + } + promoteMidTurnMessage( + entry, + message.messageId, + message.text, + message.originatorClientId, + message.content, + ); + } + }; + + /** + * Close the Goal turn's drain window. A Goal turn drains the mid-turn queue + * from inside the child, so a message enqueued after its last drain would + * otherwise sit in the queue with nothing scheduled to consume it — the same + * race the prompt settle already closes. Promoting is the supported path + * while a Goal is still active: the child's `claimGoalTurn` makes the + * promoted prompt wait for the permit and run as the next Goal turn. + */ + const settleMidTurnQueueAfterGoalTurn = (sessionId: string) => { + const entry = byId.get(sessionId); + if (!entry) return; + // A prompt owns the queue and settles it on its own terminal; a Goal turn + // that started again already re-armed the child's drain. + if ( + entry.goalTurnActive === true || + entry.pendingPromptCount > 0 || + entry.closing + ) { + return; + } + const undrained = entry.midTurnMessageQueue.splice(0); + if (undrained.length === 0) return; + settleUndrainedMidTurnMessages(entry, undrained); + }; + const bridgeApi: AcpSessionBridge = { setLiveScreenContextCaptureHandler(handler) { liveScreenContextCaptureHandler = handler; @@ -7715,7 +7782,8 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { attachCount: entry.attachCount, pendingPromptCount: entry.pendingPromptCount, pendingPermissionCount: entry.pendingPermissionIds.size, - hasActivePrompt: entry.promptActive, + hasActivePrompt: + entry.promptActive || entry.goalTurnActive === true, lastEventId: entry.events.lastEventId, ...(entry.sessionLastSeenAt !== undefined ? { lastSeenAt: entry.sessionLastSeenAt } @@ -7979,7 +8047,8 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { ...(existing.sourceId !== undefined ? { sourceId: existing.sourceId } : {}), - hasActivePrompt: existing.promptActive, + hasActivePrompt: + existing.promptActive || existing.goalTurnActive === true, }; } // Coalesce: if another caller is already mid-spawn for this same @@ -8055,7 +8124,9 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { ...session, attached: true, clientId, - hasActivePrompt: attachedEntry.promptActive, + hasActivePrompt: + attachedEntry.promptActive || + attachedEntry.goalTurnActive === true, }; } } @@ -8594,6 +8665,10 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { return copy; })(); entry.promptActive = true; + // The child serializes Goal turns against RPC prompts, so a + // still-set flag here means the goal end_turn signal was + // lost; self-heal rather than pin the session active. + entry.goalTurnActive = false; entry.activePromptId = pendingEntry.promptId; delete entry.cancelBroadcastWithoutPrompt; delete entry.turnError; @@ -8863,25 +8938,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { // caller synchronously reserves the next FIFO slot, then ordinary // promotions follow it without exposing the fallback as queued. releasePromptSlot(); - for (const message of undrainedMessages) { - if (message.queueOnly) { - try { - message.onSettledWithoutDrain?.(); - } catch (error) { - writeStderrLine( - `[mid-turn] session=${JSON.stringify(entry.sessionId)} failed to hand undrained queue-only message ${JSON.stringify(message.messageId)} back to its caller: ${JSON.stringify(error instanceof Error ? error.message : String(error))}`, - ); - } - continue; - } - promoteMidTurnMessage( - entry, - message.messageId, - message.text, - message.originatorClientId, - message.content, - ); - } + settleUndrainedMidTurnMessages(entry, undrainedMessages); // DAEMON-005: deferred close-on-prompt-complete. Lives here (not // in `promptPromise.finally`) so the terminal broadcast — the // `result.then` registered above on this same promise — runs @@ -10134,6 +10191,19 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { ); }, + async controlSessionGoal(sessionId, request, context) { + const entry = byId.get(sessionId); + if (!entry) throw new SessionNotFoundError(sessionId); + const info = channelInfoForEntry(entry); + if (!info || info.isDying) throw new SessionNotFoundError(sessionId); + resolveTrustedClientId(entry, context?.clientId); + return requestSessionStatus( + sessionId, + SERVE_CONTROL_EXT_METHODS.sessionGoalControl, + { request }, + ); + }, + async clearSessionGoal(sessionId) { return requestSessionStatus<{ cleared: boolean; condition?: string }>( sessionId, @@ -11058,11 +11128,18 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { const messageId = requestedMessageId ?? randomUUID(); // If the turn settled while the POST was in flight, start it through the // normal prompt path. A client-supplied id keeps retries idempotent. - if (entry.pendingPromptCount === 0) { - // `queueOnly` callers (live steering) drive the next turn themselves: - // a promoted message would run as a bare prompt with no collector - // forwarding its response to them or arming a deadline. - if (options?.queueOnly) { + // A child-driven Goal turn never crosses the `session/prompt` RPC + // boundary, so `pendingPromptCount` stays 0 for its whole duration — + // but the child drains THIS queue between tool batches from inside that + // turn, so the session is genuinely busy and the message belongs in the + // queue. Without `goalTurnActive` here every mid-turn insert during a + // Goal turn is rejected as idle even though the client enables the + // affordance (Goal turns are non-idle in `hasActivePrompt` summaries). + if (entry.pendingPromptCount === 0 && entry.goalTurnActive !== true) { + // Both modes refuse new ownership once idle. `queueOnly` callers (live + // steering) additionally drive the next turn themselves: a promoted + // message would have no collector forwarding its response or deadline. + if (options?.queueOnly || options?.rejectIfIdle) { writeStderrLine( `[mid-turn] session=${JSON.stringify(entry.sessionId)} rejected id ${JSON.stringify(messageId)}: session idle`, ); diff --git a/packages/acp-bridge/src/bridgeClient.test.ts b/packages/acp-bridge/src/bridgeClient.test.ts index dc4f252a972..ca0473bd0dd 100644 --- a/packages/acp-bridge/src/bridgeClient.test.ts +++ b/packages/acp-bridge/src/bridgeClient.test.ts @@ -213,6 +213,96 @@ describe('BridgeClient — background notification turn boundary', () => { }); }); + it('marks the session active for a goal-turn start signal', async () => { + const sessionId = 'session-goal'; + const publish = vi.fn(); + const entry = { sessionId, events: { publish }, goalTurnActive: false }; + const noFlow = () => { + throw new Error('test: permission flow should not run'); + }; + const client = new BridgeClient( + ((id: string) => (id === sessionId ? entry : undefined)) as never, + noFlow as never, + { request: noFlow } as never, + 0, + Infinity, + ); + + await client.extNotification('_qwencode/start_turn', { + sessionId, + source: 'goal', + }); + + expect(entry.goalTurnActive).toBe(true); + expect(publish).not.toHaveBeenCalled(); + + await client.extNotification('_qwencode/end_turn', { + sessionId, + reason: 'end_turn', + source: 'goal', + promptId: 'session-goal########1', + }); + + expect(entry.goalTurnActive).toBe(false); + }); + + it('publishes a real turn_complete for a goal-turn end signal', async () => { + const sessionId = 'session-goal'; + const publish = vi.fn().mockReturnValue(true); + const entry = { sessionId, events: { publish } }; + const noFlow = () => { + throw new Error('test: permission flow should not run'); + }; + const client = new BridgeClient( + ((id: string) => (id === sessionId ? entry : undefined)) as never, + noFlow as never, + { request: noFlow } as never, + 0, + Infinity, + ); + + await client.extNotification('_qwencode/end_turn', { + sessionId, + reason: 'end_turn', + source: 'goal', + promptId: 'session-goal########3', + }); + + expect(publish).toHaveBeenCalledWith({ + type: 'turn_complete', + promptId: 'session-goal########3', + data: { + sessionId, + stopReason: 'end_turn', + promptId: 'session-goal########3', + }, + }); + }); + + it('drops a goal-turn end signal without a promptId', async () => { + const sessionId = 'session-goal'; + const publish = vi.fn(); + const entry = { sessionId, events: { publish } }; + const noFlow = () => { + throw new Error('test: permission flow should not run'); + }; + const client = new BridgeClient( + ((id: string) => (id === sessionId ? entry : undefined)) as never, + noFlow as never, + { request: noFlow } as never, + 0, + Infinity, + ); + + await client.extNotification('_qwencode/end_turn', { + sessionId, + reason: 'end_turn', + source: 'goal', + }); + + expect(publish).not.toHaveBeenCalled(); + }); + it('drops malformed or foreign end-turn signals', async () => { const publish = vi.fn(); const entry = { sessionId: 'owned', events: { publish } }; diff --git a/packages/acp-bridge/src/bridgeClient.ts b/packages/acp-bridge/src/bridgeClient.ts index 8e4a29996a3..b0b424f16bb 100644 --- a/packages/acp-bridge/src/bridgeClient.ts +++ b/packages/acp-bridge/src/bridgeClient.ts @@ -636,6 +636,14 @@ export interface BridgeClientSessionEntry { settledMidTurnMessageIds: string[]; /** Complete prompts waiting behind the currently running prompt. */ pendingPromptList: PendingPromptEntry[]; + /** + * True while a child-driven Goal turn is running. Set by the + * `_qwencode/start_turn` notification and cleared by the matching + * `_qwencode/end_turn`; OR-ed into `hasActivePrompt` summaries so + * live-state consumers (sidebar activity, daemon status) see Goal turns + * that never cross the bridge's `session/prompt` RPC boundary. + */ + goalTurnActive?: boolean; /** Bridge prompt that owns the child Guard wait for this FIFO. */ todoStopGuardAwaitingQueuedPromptOwnerPromptId?: string; /** True while a prompt is executing for this session. */ @@ -829,6 +837,14 @@ export class BridgeClient implements Client { * optional so existing direct constructors stay source-compatible. */ private readonly onSessionCatalogChanged?: () => void, + /** + * Invoked after a child-driven Goal turn clears `goalTurnActive`. The + * bridge settles whatever the ending turn's last mid-turn drain missed — + * a Goal turn owns no prompt slot, so its terminal is the only signal. + * Trailing and optional so existing direct constructors stay + * source-compatible. + */ + private readonly onGoalTurnEnded?: (sessionId: string) => void, ) {} async requestPermission( @@ -1929,7 +1945,7 @@ export class BridgeClient implements Client { * `qwen/notify/session/prompt-suggestion` (followup assist), * `qwen/notify/session/artifact-event` (hook artifacts), * `qwen/notify/session/terminal-sequence`, and - * `_qwencode/end_turn` (background-notification turns), and + * `_qwencode/end_turn` (background-notification and goal turns), and * `qwen/notify/session/mcp-budget-event` — each translated into a * session-scoped SSE frame. Unknown methods are dropped silently for * forward-compat. @@ -1961,21 +1977,56 @@ export class BridgeClient implements Client { } return; } + if (method === '_qwencode/start_turn') { + const sessionId = params['sessionId']; + if ( + typeof sessionId !== 'string' || + sessionId.length === 0 || + params['source'] !== 'goal' + ) { + return; + } + const entry = this.resolveEntry(sessionId); + if (!entry || !this.ownsSession(sessionId)) return; + entry.goalTurnActive = true; + return; + } if (method === '_qwencode/end_turn') { const sessionId = params['sessionId']; const reason = params['reason']; + const source = params['source']; if ( typeof sessionId !== 'string' || sessionId.length === 0 || typeof reason !== 'string' || reason.length === 0 || reason.length > 128 || - params['source'] !== 'background_notification' + (source !== 'background_notification' && source !== 'goal') ) { return; } const entry = this.resolveEntry(sessionId); if (!entry || !this.ownsSession(sessionId)) return; + if (source === 'goal') { + entry.goalTurnActive = false; + // Before the promptId validation below: a malformed id costs the + // session its `turn_complete`, but the queue must still be settled. + this.onGoalTurnEnded?.(sessionId); + const promptId = params['promptId']; + if ( + typeof promptId !== 'string' || + promptId.length === 0 || + promptId.length > 256 + ) { + return; + } + entry.events.publish({ + type: 'turn_complete', + promptId, + data: { sessionId, stopReason: reason, promptId }, + }); + return; + } entry.events.publish({ type: 'background_notification_turn_complete', data: { sessionId, reason }, diff --git a/packages/acp-bridge/src/bridgeTypes.ts b/packages/acp-bridge/src/bridgeTypes.ts index 3332371c2d8..1e2d087a056 100644 --- a/packages/acp-bridge/src/bridgeTypes.ts +++ b/packages/acp-bridge/src/bridgeTypes.ts @@ -6,7 +6,9 @@ import type { ApprovalMode, + GoalControlRequest, GoalSnapshotV2, + GoalStateResponse, SessionGroupPresetColor, TurnResultCode, TurnResultErrorPayload, @@ -1661,6 +1663,13 @@ export interface AcpSessionBridge { sessionId: string, ): Promise<{ cleared: boolean; condition?: string }>; + /** Atomically apply a typed Goal lifecycle control in a live session. */ + controlSessionGoal( + sessionId: string, + request: GoalControlRequest, + context?: BridgeClientRequestContext, + ): Promise; + /** * Read a live session's Goal state. Throws `SessionNotFoundError` when the * session is not resident because this route addresses the selected runtime. @@ -1849,9 +1858,12 @@ export interface AcpSessionBridge { * authorized against the session like `/prompt` and `/btw` — throws * `InvalidClientIdError` when the id is not bound to the session, and * `SessionNotFoundError` for unknown ids. Ownership is session-wide. - * With `options.queueOnly` an idle session rejects instead of promoting. If - * a busy session settles before draining the message, - * `onSettledWithoutDrain` lets the caller drive the next turn itself. + * With `options.rejectIfIdle` an idle session rejects instead of taking + * ownership. A message accepted while busy keeps the ordinary public queue + * semantics: it is echoed when drained and promoted if the turn settles + * first. `options.queueOnly` is reserved for internal live steering; if a + * busy session settles before draining one of those messages, + * `onSettledWithoutDrain` lets that internal caller drive the next turn. * `options.content` carries image blocks with the message; * an empty `message` is admitted when media blocks are present. */ @@ -1861,6 +1873,7 @@ export interface AcpSessionBridge { context?: BridgeClientRequestContext, messageId?: string, options?: { + rejectIfIdle?: boolean; queueOnly?: boolean; onSettledWithoutDrain?: () => void; content?: readonly BridgePromptContentBlock[]; diff --git a/packages/acp-bridge/src/status.ts b/packages/acp-bridge/src/status.ts index 51cb8ac6035..7a49486b3c8 100644 --- a/packages/acp-bridge/src/status.ts +++ b/packages/acp-bridge/src/status.ts @@ -175,6 +175,7 @@ export const SERVE_CONTROL_EXT_METHODS = { workspaceMemoryDream: 'qwen/control/workspace/memory/dream', // Runtime MCP server mutation ext-methods sessionTaskCancel: 'qwen/control/session/task/cancel', + sessionGoalControl: 'qwen/control/session/goal/control', sessionGoalClear: 'qwen/control/session/goal/clear', /** * Read a live session's `/goal` state. The active goal lives only in the diff --git a/packages/acp-bridge/src/transcript-replay.test.ts b/packages/acp-bridge/src/transcript-replay.test.ts index 522260986a2..94d21b07ed3 100644 --- a/packages/acp-bridge/src/transcript-replay.test.ts +++ b/packages/acp-bridge/src/transcript-replay.test.ts @@ -89,14 +89,30 @@ describe('createTranscriptReplayMachine', () => { ).toEqual([]); }); + it('replays user-initiated Goal controls as user messages', () => { + const projected = updates( + createTranscriptReplayMachine(), + goalStateRecord('goal-create', 'create', GOAL), + ); + + expect(projected[0]).toMatchObject({ + sessionUpdate: 'user_message_chunk', + content: { type: 'text', text: `/goal ${GOAL.objective}` }, + _meta: { + source: 'goal_control', + 'qwen.session.recordId': 'goal-create', + }, + }); + }); + it('projects goal_state through v2-first metadata', () => { const projected = updates( createTranscriptReplayMachine(), goalStateRecord('goal-create', 'create', GOAL), ); - expect(projected).toHaveLength(1); - expect(projected[0]?._meta).toMatchObject({ + expect(projected).toHaveLength(2); + expect(projected[1]?._meta).toMatchObject({ goalState: { v: 2, goal: GOAL, activity: 'idle' }, goalStatus: { kind: 'set', condition: GOAL.objective }, 'qwen.session.recordId': 'goal-create', @@ -115,7 +131,7 @@ describe('createTranscriptReplayMachine', () => { goalStateRecord('goal-clear', 'clear', null), ); - expect(projected[0]?._meta).toMatchObject({ + expect(projected[1]?._meta).toMatchObject({ goalState: { v: 2, goal: null, activity: 'idle' }, goalStatus: { kind: 'cleared', condition: GOAL.objective }, 'qwen.session.recordId': 'goal-clear', @@ -182,7 +198,7 @@ describe('createTranscriptReplayMachine', () => { expect( updates(machine, goalStateRecord('goal-create', 'create', GOAL)), - ).toHaveLength(1); + ).toHaveLength(2); const turned: GoalRecord = { ...GOAL, @@ -302,7 +318,7 @@ describe('createTranscriptReplayMachine', () => { expect( updates(machine, goalStateRecord('goal-create', 'create', GOAL)), - ).toHaveLength(1); + ).toHaveLength(2); const turnedOnce: GoalRecord = { ...GOAL, diff --git a/packages/acp-bridge/src/transcript-replay.ts b/packages/acp-bridge/src/transcript-replay.ts index 3b8553c55a6..b8a6c8b645e 100644 --- a/packages/acp-bridge/src/transcript-replay.ts +++ b/packages/acp-bridge/src/transcript-replay.ts @@ -939,9 +939,26 @@ class DefaultTranscriptReplayMachine implements TranscriptReplayMachine { payload, this.goalState?.goal ?? null, ); + const goalControlCommand = projectGoalControlCommand( + payload.cause, + payload.snapshot, + ); this.goalState = payload.snapshot; this.goalCause = payload.cause; if (bookkeepingOnly) return; + if (goalControlCommand) { + yield emit( + createTranscriptMessageUpdate({ + role: 'user', + text: goalControlCommand, + ...meta, + extra: { + source: 'goal_control', + 'qwen.session.recordId': record.uuid, + }, + }), + ); + } const { type: _type, ...goalStatus } = projection.goalStatus; yield emit( createTranscriptMessageUpdate({ @@ -1121,6 +1138,40 @@ class DefaultTranscriptReplayMachine implements TranscriptReplayMachine { } } +function projectGoalControlCommand( + cause: GoalStateCause, + snapshot: GoalSnapshotV2, +): string | undefined { + switch (cause) { + case 'create': + case 'replace': + return snapshot.goal ? `/goal ${snapshot.goal.objective}` : undefined; + case 'edit': + return snapshot.goal + ? `/goal edit ${snapshot.goal.objective}` + : undefined; + case 'pause': + case 'resume': + case 'clear': + return `/goal ${cause}`; + case 'turn_finished': + case 'checkpoint': + case 'verifier_accept': + case 'verifier_reject': + case 'complete': + case 'blocked': + case 'usage_limited': + case 'migrated': + return undefined; + default: + return assertNever(cause); + } +} + +function assertNever(value: never): never { + throw new Error(`Unsupported Goal state cause: ${String(value)}`); +} + function parseTranscriptGoalStatus( value: unknown, ): TranscriptGoalStatus | undefined { diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index b3d6350c3ad..3276722c7f7 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -222,6 +222,19 @@ vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => ({ GoalPersistenceUnavailableError: ( await importOriginal() ).GoalPersistenceUnavailableError, + parseGoalControlRequest: ( + await importOriginal() + ).parseGoalControlRequest, + // The real classes for the same reason as above: `mapGoalControlError` + // narrows on them with `instanceof`, and a stand-in (or an omission, which + // resolves to undefined) makes every conflict/transition branch throw before + // it can be asserted. + GoalConflictError: ( + await importOriginal() + ).GoalConflictError, + GoalInvalidTransitionError: ( + await importOriginal() + ).GoalInvalidTransitionError, SessionIdCaseConflictError: ( await importOriginal() ).SessionIdCaseConflictError, @@ -945,6 +958,8 @@ import { APPROVAL_MODES, ToolNames, GoalPersistenceUnavailableError, + GoalConflictError, + GoalInvalidTransitionError, } from '@qwen-code/qwen-code-core'; import { ndJsonStream } from '@qwen-code/acp-bridge/ndJsonStream'; import { SESSION_SOURCE_META_KEY } from '@qwen-code/acp-bridge'; @@ -3891,6 +3906,7 @@ describe('QwenAgent MCP SSE/HTTP support', () => { getHookSystem: vi.fn().mockReturnValue(undefined), getDisableAllHooks: vi.fn().mockReturnValue(true), hasHooksForEvent: vi.fn().mockReturnValue(false), + isTrustedFolder: vi.fn().mockReturnValue(true), }; } @@ -9851,6 +9867,158 @@ describe('QwenAgent MCP SSE/HTTP support', () => { await agentPromise; }); + it('allows reducing Goal work in an untrusted workspace but rejects starting it', async () => { + const sessionId = '11111111-1111-1111-1111-111111111111'; + const innerConfig = await setupSessionMocks(sessionId); + const snapshot = goalSnapshot({ objective: 'ship it', turnCount: 1 }); + const dispatch = vi.fn().mockResolvedValue({ snapshot }); + Object.assign(innerConfig, { + isTrustedFolder: vi.fn().mockReturnValue(false), + getGoalRuntimeReady: vi.fn().mockResolvedValue({ + getSnapshot: () => snapshot, + dispatch, + }), + }); + + const agentPromise = runAcpAgent( + mockConfig, + makeSessionSettings(), + mockArgv, + ); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + const agent = capturedAgentFactory!({ + get closed() { + return mockConnectionState.promise; + }, + }) as AgentLike; + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + await expect( + agent.extMethod(SERVE_CONTROL_EXT_METHODS.sessionGoalControl, { + sessionId, + request: { + action: 'pause', + expectedGoalId: 'goal-1', + expectedRevision: 1, + }, + }), + ).resolves.toEqual({ snapshot }); + // Every action that starts or expands Goal work is gated, not just create: + // dropping any one of them restarts work in an untrusted workspace. + for (const request of [ + { action: 'create' as const, objective: 'new work' }, + { + action: 'replace' as const, + objective: 'new work', + expectedGoalId: 'goal-1', + expectedRevision: 1, + }, + { + action: 'edit' as const, + objective: 'revised work', + expectedGoalId: 'goal-1', + expectedRevision: 1, + }, + { + action: 'resume' as const, + expectedGoalId: 'goal-1', + expectedRevision: 1, + }, + ]) { + await expect( + agent.extMethod(SERVE_CONTROL_EXT_METHODS.sessionGoalControl, { + sessionId, + request, + }), + ).rejects.toMatchObject({ + code: -32003, + data: { errorKind: 'untrusted_workspace', httpStatus: 403 }, + }); + } + expect(dispatch).toHaveBeenCalledOnce(); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('maps a Goal control dispatch failure onto its wire contract', async () => { + // The client's 409 resync reads `data.errorKind` and `data.current`: a + // refactor that drops `current`, swaps the `instanceof` order, or changes + // the code breaks resync silently. The only other coverage here is the + // success path and the untrusted gate, and the gate throws before this + // mapping is reachable. + const sessionId = '11111111-1111-1111-1111-111111111111'; + const innerConfig = await setupSessionMocks(sessionId); + const current = goalSnapshot({ objective: 'ship it', revision: 4 }); + const persistFallback = goalSnapshot({ objective: 'from the runtime' }); + const dispatch = vi.fn(); + Object.assign(innerConfig, { + isTrustedFolder: vi.fn().mockReturnValue(true), + getGoalRuntime: vi.fn().mockReturnValue({ + getSnapshot: () => persistFallback, + dispatch, + }), + getGoalRuntimeReady: vi.fn().mockResolvedValue({ + getSnapshot: () => persistFallback, + dispatch, + }), + }); + + const agentPromise = runAcpAgent( + mockConfig, + makeSessionSettings(), + mockArgv, + ); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + const agent = capturedAgentFactory!({ + get closed() { + return mockConnectionState.promise; + }, + }) as AgentLike; + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + + const control = (request: Record) => + agent.extMethod(SERVE_CONTROL_EXT_METHODS.sessionGoalControl, { + sessionId, + request, + }); + const pause = { + action: 'pause', + expectedGoalId: 'goal-1', + expectedRevision: 1, + }; + + // A CAS miss carries the daemon's own snapshot so the client can resync + // against it rather than re-reading. + dispatch.mockRejectedValueOnce(new GoalConflictError(current)); + await expect(control(pause)).rejects.toMatchObject({ + code: -32009, + data: { errorKind: 'goal_conflict', current }, + }); + + // Same code, different kind: the two are distinguished only by errorKind. + dispatch.mockRejectedValueOnce( + new GoalInvalidTransitionError('cannot pause a completed goal', current), + ); + await expect(control(pause)).rejects.toMatchObject({ + code: -32009, + message: 'cannot pause a completed goal', + data: { errorKind: 'goal_invalid_transition', current }, + }); + + // Anything else is a persistence failure, and its `current` comes from the + // runtime — the failure carries no snapshot of its own. + dispatch.mockRejectedValueOnce(new Error('disk full')); + await expect(control(pause)).rejects.toMatchObject({ + code: -32603, + message: 'disk full', + data: { errorKind: 'goal_persist_failed', current: persistFallback }, + }); + + mockConnectionState.resolve(); + await agentPromise; + }); + it('returns cleared false when no session goal is active', async () => { const sessionId = '11111111-1111-1111-1111-111111111111'; const innerConfig = await setupSessionMocks(sessionId); diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 5d5909e2daa..5dfeaa1ae4b 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -99,8 +99,14 @@ import { extractDaemonTraceContext, withDaemonSpan, emptyGoalSnapshot, + GoalConflictError, + GoalInvalidTransitionError, GoalPersistenceUnavailableError, + parseGoalControlRequest, + type GoalControlRequest, + type GoalRuntime, type GoalSnapshotV2, + type GoalStateResponse, type AgentParams, ApprovalMode, type Config, @@ -424,6 +430,68 @@ const ACP_REASONING_EFFORT_NAMES: Record = { // Must be less than WORKSPACE_MEMORY_REMEMBER_TIMEOUT_MS (300s) in bridge.ts. const WORKSPACE_MEMORY_REMEMBER_CHILD_TIMEOUT_MS = 295_000; +function currentGoalSnapshot( + config: Config, + runtime?: GoalRuntime, +): GoalSnapshotV2 { + try { + return (runtime ?? config.getGoalRuntime()).getSnapshot(); + } catch { + return emptyGoalSnapshot(); + } +} + +function mapGoalControlError( + error: unknown, + config: Config, + runtime?: GoalRuntime, +): RequestError { + if (error instanceof GoalConflictError) { + return new RequestError(-32009, error.message, { + errorKind: 'goal_conflict', + current: error.current, + }); + } + if (error instanceof GoalInvalidTransitionError) { + return new RequestError(-32009, error.message, { + errorKind: 'goal_invalid_transition', + current: error.current, + }); + } + return new RequestError( + -32603, + error instanceof Error ? error.message : 'Goal persistence failed', + { + errorKind: 'goal_persist_failed', + current: currentGoalSnapshot(config, runtime), + }, + ); +} + +async function dispatchGoalControl( + config: Config, + request: GoalControlRequest, +): Promise { + const requiresTrustedWorkspace = + request.action === 'create' || + request.action === 'replace' || + request.action === 'edit' || + request.action === 'resume'; + if (requiresTrustedWorkspace && !config.isTrustedFolder()) { + throw new RequestError(-32003, 'Workspace is not trusted.', { + errorKind: 'untrusted_workspace', + httpStatus: 403, + }); + } + let runtime: GoalRuntime | undefined; + try { + runtime = await config.getGoalRuntimeReady(); + return await runtime.dispatch(request); + } catch (error) { + throw mapGoalControlError(error, config, runtime); + } +} + const TURN_STATUS_SCAN_PAGE_LIMIT = 500; const TURN_STATUS_SCAN_MAX_PAGES = 10; @@ -11114,6 +11182,28 @@ class QwenAgent implements Agent { snapshot: response.snapshot, }; } + case SERVE_CONTROL_EXT_METHODS.sessionGoalControl: { + const sessionId = params['sessionId']; + if (typeof sessionId !== 'string' || sessionId.length === 0) { + throw RequestError.invalidParams( + undefined, + 'Invalid or missing sessionId', + ); + } + const request = parseGoalControlRequest(params['request']); + if (!request) { + throw RequestError.invalidParams( + undefined, + 'Invalid or missing Goal control request', + ); + } + const session = this.sessionOrThrow(sessionId); + const response = await dispatchGoalControl( + session.getConfig(), + request, + ); + return { snapshot: response.snapshot }; + } case SERVE_CONTROL_EXT_METHODS.sessionGoalGet: { const sessionId = params['sessionId']; if (typeof sessionId !== 'string' || sessionId.length === 0) { diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index 7d030f26dac..91a03d63810 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -17709,6 +17709,62 @@ describe('Session', () => { ).not.toHaveBeenCalled(); }); + it('notifies the bridge that the Goal turn ended', async () => { + const permit: core.GoalTurnPermit = { + goalId: 'goal-1', + revision: 1, + turnId: 'turn-end-signal', + }; + mockGoalRuntime.getSnapshot.mockReturnValue({ + v: 2, + activity: 'running', + goal: { + goalId: 'goal-1', + revision: 1, + objective: 'check weather', + status: 'active', + evidenceCursor: { recordId: 'cursor-1' }, + turnCount: 0, + activeTimeMs: 0, + createdAt: 1234, + updatedAt: 1234, + }, + }); + mockGoalRuntime.permitForTurn.mockImplementation((turnKey: string) => + turnKey === 'goal-runtime:turn-end-signal' ? permit : undefined, + ); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValue(createEmptyStream()); + + expect(boundGoalHost).toBeDefined(); + await boundGoalHost!.startGoalTurn({ + permit, + continuationContext: 'check weather', + }); + + await vi.waitFor(() => { + expect(mockClient.extNotification).toHaveBeenCalledWith( + '_qwencode/end_turn', + { + sessionId: 'test-session-id', + reason: 'end_turn', + source: 'goal', + promptId: expect.stringMatching( + /^test-session-id########\d+$/, + ) as unknown as string, + }, + ); + }); + expect(mockClient.extNotification).toHaveBeenCalledWith( + '_qwencode/start_turn', + { + sessionId: 'test-session-id', + source: 'goal', + }, + ); + }); + it('settles a Goal turn whose prompt rejects before the turn body runs', async () => { // `prompt()` rejects ahead of the try whose finally settles the turn // when `assertCanStartTurn` throws — a session that began closing diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index 73e1fd40cc3..c60c5998bd8 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -2208,8 +2208,10 @@ export class Session implements SessionContext { this.goalProcessing = true; this.activeGoalTurn = turn; const parts = buildGoalContinuationParts(turn); + let result: PromptResponse | undefined; + await this.#emitGoalStartTurn(); try { - await this.prompt( + result = await this.prompt( { sessionId: this.sessionId, prompt: parts.map((part) => ({ @@ -2239,6 +2241,7 @@ export class Session implements SessionContext { }`, ); } finally { + await this.#emitGoalEndTurn(result); if (this.activeGoalTurn === turn) this.activeGoalTurn = undefined; this.goalProcessing = false; void this.#drainCronQueue(); @@ -8489,6 +8492,40 @@ export class Session implements SessionContext { } } + /** + * Goal turns run inside this child via `prompt()` directly, so the daemon + * bridge never observes a `session/prompt` RPC boundary for them and would + * otherwise publish no `turn_complete` — leaving SSE clients (Web Shell, + * SDK) with a streaming state that never settles. + */ + async #emitGoalStartTurn(): Promise { + try { + await this.client.extNotification('_qwencode/start_turn', { + sessionId: this.sessionId, + source: 'goal', + }); + } catch (error) { + debugLogger.debug( + `Goal start-turn extNotification dropped: ${this.#formatError(error)}`, + ); + } + } + + async #emitGoalEndTurn(result: PromptResponse | undefined): Promise { + try { + await this.client.extNotification('_qwencode/end_turn', { + sessionId: this.sessionId, + reason: result?.stopReason ?? 'cancelled', + source: 'goal', + promptId: this.config.getSessionId() + '########' + String(this.turn), + }); + } catch (error) { + debugLogger.debug( + `Goal end-turn extNotification dropped: ${this.#formatError(error)}`, + ); + } + } + async sendAvailableCommandsUpdate(): Promise { try { await this.sendAvailableCommandsUpdateOrThrow(); diff --git a/packages/cli/src/acp-integration/session/history-replay-page.test.ts b/packages/cli/src/acp-integration/session/history-replay-page.test.ts index 7b82a6046bf..2f75c51d9d4 100644 --- a/packages/cli/src/acp-integration/session/history-replay-page.test.ts +++ b/packages/cli/src/acp-integration/session/history-replay-page.test.ts @@ -603,7 +603,11 @@ describe('history replay page', () => { return 'next-cursor'; }, }); - expect(firstPage.updates).toHaveLength(2); + expect(firstPage.updates).toHaveLength(3); + expect(firstPage.updates[0]).toMatchObject({ + sessionUpdate: 'user_message_chunk', + content: { type: 'text', text: `/goal ${goal.objective}` }, + }); expect(nextReplay).toMatchObject({ goalCause: 'verifier_reject' }); const recommittedGoal = { diff --git a/packages/cli/src/serve/routes/goals.test.ts b/packages/cli/src/serve/routes/goals.test.ts index fcff6888d12..0360cacee1a 100644 --- a/packages/cli/src/serve/routes/goals.test.ts +++ b/packages/cli/src/serve/routes/goals.test.ts @@ -64,6 +64,20 @@ const activeGoal = ( }; }; +const goalWithStatus = ( + condition: string, + status: 'paused' | 'blocked' | 'usage_limited' | 'complete', +): BridgeSessionGoal => { + const base = activeGoal(condition); + return { + ...base, + snapshot: { + ...base.snapshot, + goal: { ...base.snapshot.goal!, status }, + }, + }; +}; + const noGoal: BridgeSessionGoal = { snapshot: { v: 2, activity: 'idle', goal: null }, active: null, @@ -165,6 +179,7 @@ describe('GET /goals', () => { iterations: 0, setAt: 2000, hasActivePrompt: true, + snapshot: goals['s2'].snapshot, }, { sessionId: 's1', @@ -174,10 +189,54 @@ describe('GET /goals', () => { setAt: 1000, lastReason: 'two tests still fail', hasActivePrompt: false, + snapshot: goals['s1'].snapshot, }, ]); }); + it.each(['paused', 'blocked', 'usage_limited'] as const)( + 'lists a %s goal so its controls stay reachable', + async (status) => { + // A stopped goal is exactly the one the user needs to find in order to + // resume it; listing only active goals hides it from the Goals page. + const goals: Record = { + s1: goalWithStatus('resume me', status), + }; + const app = makeApp({ + listWorkspaceSessions: () => [summary('s1')], + getSessionGoal: async (id) => goals[id], + }); + + const res = await request(app).get('/goals'); + + expect(res.status).toBe(200); + expect(res.body.goals).toHaveLength(1); + expect(res.body.goals[0]).toMatchObject({ + sessionId: 's1', + condition: 'resume me', + }); + }, + ); + + it('filters out a completed goal', async () => { + // Without the exclusion a finished goal is listed forever. + const goals: Record = { + s1: goalWithStatus('already done', 'complete'), + s2: activeGoal('still running'), + }; + const app = makeApp({ + listWorkspaceSessions: () => [summary('s1'), summary('s2')], + getSessionGoal: async (id) => goals[id], + }); + + const res = await request(app).get('/goals'); + + expect(res.status).toBe(200); + expect( + res.body.goals.map((goal: { sessionId: string }) => goal.sessionId), + ).toEqual(['s2']); + }); + it('drops a session whose probe rejects rather than failing the whole list', async () => { vi.mocked(writeStderrLine).mockClear(); const app = makeApp({ @@ -199,6 +258,7 @@ describe('GET /goals', () => { iterations: 0, setAt: 1000, hasActivePrompt: false, + snapshot: activeGoal('keep going').snapshot, }, ]); diff --git a/packages/cli/src/serve/routes/goals.ts b/packages/cli/src/serve/routes/goals.ts index e33aa52d5a5..9b248ddd2a6 100644 --- a/packages/cli/src/serve/routes/goals.ts +++ b/packages/cli/src/serve/routes/goals.ts @@ -18,9 +18,8 @@ * (up to `PROBE_CONCURRENCY`), so a wedged child costs one timeout rather than * one per session. * - * Read-only: clearing a goal stays on `POST /session/:id/goal/clear`, and - * setting one stays a prompt (`/goal ` updates the owning runtime, - * which schedules the first Goal turn). + * Controls use the canonical `POST /session/:id/goal` route. This listing stays + * read-only and only projects each live runtime's current snapshot. */ import type { Application } from 'express'; @@ -86,7 +85,7 @@ async function allSettledWithLimit( return results; } -/** One row of the Goals page. */ +/** One non-terminal Goal shown on the Goals page. */ interface GoalView { sessionId: string; /** The session's label, when it has one — otherwise the client shows the id. */ @@ -102,6 +101,7 @@ interface GoalView { * that the goal specifically is running. */ hasActivePrompt: boolean; + snapshot: BridgeSessionGoal['snapshot']; } export function registerGoalsRoutes( @@ -145,17 +145,19 @@ export function registerGoalsRoutes( continue; } const { session, goal } = outcome.value; - if (!goal.active) continue; + const record = goal.snapshot.goal; + if (!record || record.status === 'complete') continue; goals.push({ sessionId: session.sessionId, displayName: session.displayName ?? null, - condition: goal.active.condition, - iterations: goal.active.iterations, - setAt: goal.active.setAt, - ...(goal.active.lastReason !== undefined - ? { lastReason: goal.active.lastReason } + condition: record.objective, + iterations: record.turnCount, + setAt: record.createdAt, + ...(record.lastReason !== undefined + ? { lastReason: record.lastReason } : {}), hasActivePrompt: session.hasActivePrompt, + snapshot: goal.snapshot, }); } if (dropped.length > 0) { diff --git a/packages/cli/src/serve/routes/session.ts b/packages/cli/src/serve/routes/session.ts index f841dfcc569..010253e727b 100644 --- a/packages/cli/src/serve/routes/session.ts +++ b/packages/cli/src/serve/routes/session.ts @@ -30,6 +30,7 @@ import { type SessionGroupColor, type SessionGroupPresetColor, type SessionArchiveState, + parseGoalControlRequest, } from '@qwen-code/qwen-code-core'; import type { SessionArtifactInput } from '@qwen-code/acp-bridge/sessionArtifacts'; import { @@ -4357,6 +4358,46 @@ export function registerSessionRoutes( ), ); + app.post( + '/session/:id/goal', + mutate({ strict: true }), + withOwnerMutableSession( + 'POST /session/:id/goal', + async (req, res, sessionId, runtime) => { + const request = parseGoalControlRequest(safeBody(req)); + if (!request) { + res.status(400).json({ + error: 'Invalid Goal control request', + code: 'invalid_goal_control_request', + }); + return; + } + const clientId = parseClientIdHeader(req, res); + if (clientId === null) return; + res + .status(200) + .json( + await runtime.bridge.controlSessionGoal( + sessionId, + request, + clientId === undefined ? undefined : { clientId }, + ), + ); + }, + ), + ); + + app.get( + '/session/:id/goal', + withOwnerReadSession( + 'GET /session/:id/goal', + async (_req, res, sessionId, runtime) => { + const goal = await runtime.bridge.getSessionGoal(sessionId); + res.status(200).json({ snapshot: goal.snapshot }); + }, + ), + ); + app.post( '/session/:id/goal/clear', mutate({ strict: true }), @@ -6250,7 +6291,10 @@ export function registerSessionRoutes( trimmed, clientId !== undefined ? { clientId } : undefined, typeof messageId === 'string' ? messageId : undefined, - mediaBlocks ? { content: mediaBlocks } : undefined, + { + rejectIfIdle: true, + ...(mediaBlocks ? { content: mediaBlocks } : {}), + }, ); res.status(200).json(result); }, diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index eeddb2c6f8d..0aa05c9261f 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -89,6 +89,8 @@ import { type PrepareExtensionInstallOptions, type PreparedExtensionMutation, type SessionListItem, + type GoalControlRequest, + type GoalSnapshotV2, } from '@qwen-code/qwen-code-core'; import * as qwenCore from '@qwen-code/qwen-code-core'; import type { DaemonStatusProvider } from '@qwen-code/acp-bridge'; @@ -797,6 +799,7 @@ interface FakeBridgeOpts { message: string, context?: BridgeClientRequestContext, messageId?: string, + options?: Parameters[4], ) => { accepted: boolean; messageId?: string }; removeMidTurnImpl?: ( sessionId: string, @@ -915,6 +918,12 @@ interface FakeBridgeOpts { clearSessionGoalImpl?: ( sessionId: string, ) => Promise<{ cleared: boolean; condition?: string }>; + controlSessionGoalImpl?: ( + sessionId: string, + request: GoalControlRequest, + context?: BridgeClientRequestContext, + ) => Promise<{ snapshot: GoalSnapshotV2 }>; + getSessionGoalImpl?: AcpSessionBridge['getSessionGoal']; continueSessionImpl?: (sessionId: string) => Promise<{ accepted: boolean; interruption: 'none' | 'interrupted_prompt' | 'interrupted_turn'; @@ -1116,6 +1125,7 @@ interface FakeBridge extends AcpSessionBridge { message: string; context?: BridgeClientRequestContext; messageId?: string; + options?: Parameters[4]; }>; removeMidTurnCalls: Array<{ sessionId: string; @@ -1202,6 +1212,11 @@ interface FakeBridge extends AcpSessionBridge { taskKind: 'agent' | 'shell' | 'monitor'; }>; clearSessionGoalCalls: string[]; + controlSessionGoalCalls: Array<{ + sessionId: string; + request: GoalControlRequest; + context?: BridgeClientRequestContext; + }>; continueSessionCalls: string[]; continueSessionContexts: Array; sessionHooksCalls: string[]; @@ -1381,6 +1396,7 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { const sessionTranscriptCalls: FakeBridge['sessionTranscriptCalls'] = []; const cancelSessionTaskCalls: FakeBridge['cancelSessionTaskCalls'] = []; const clearSessionGoalCalls: string[] = []; + const controlSessionGoalCalls: FakeBridge['controlSessionGoalCalls'] = []; const continueSessionCalls: string[] = []; const continueSessionContexts: Array = []; @@ -1722,6 +1738,34 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { opts.cancelSessionTaskImpl ?? (async () => ({ cancelled: true })); const clearSessionGoalImpl = opts.clearSessionGoalImpl ?? (async () => ({ cleared: true })); + const controlSessionGoalImpl = + opts.controlSessionGoalImpl ?? + (async (_sessionId, request) => ({ + snapshot: { + v: 2 as const, + activity: 'idle' as const, + goal: + request.action === 'create' + ? null + : { + goalId: request.expectedGoalId, + revision: request.expectedRevision, + objective: 'ship it', + status: 'active' as const, + evidenceCursor: { recordId: null }, + turnCount: 0, + activeTimeMs: 0, + createdAt: 1, + updatedAt: 1, + }, + }, + })); + const getSessionGoalImpl = + opts.getSessionGoalImpl ?? + (async () => ({ + snapshot: { v: 2 as const, activity: 'idle' as const, goal: null }, + active: null, + })); const continueSessionImpl = opts.continueSessionImpl ?? (async () => ({ accepted: false, interruption: 'none' as const })); @@ -1963,6 +2007,7 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { sessionTranscriptCalls, cancelSessionTaskCalls, clearSessionGoalCalls, + controlSessionGoalCalls, continueSessionCalls, continueSessionContexts, sessionHooksCalls, @@ -2255,6 +2300,17 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { clearSessionGoalCalls.push(sessionId); return clearSessionGoalImpl(sessionId); }, + async controlSessionGoal(sessionId, request, context) { + controlSessionGoalCalls.push({ + sessionId, + request, + ...(context ? { context } : {}), + }); + return controlSessionGoalImpl(sessionId, request, context); + }, + async getSessionGoal(sessionId) { + return getSessionGoalImpl(sessionId); + }, async continueSession(sessionId, context) { continueSessionCalls.push(sessionId); continueSessionContexts.push(context); @@ -2374,7 +2430,13 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { ...(messageId ? { messageId } : {}), ...(options ? { options } : {}), }); - return enqueueMidTurnImpl(sessionId, message, context, messageId); + return enqueueMidTurnImpl( + sessionId, + message, + context, + messageId, + options, + ); }, removeMidTurnMessage(sessionId, messageId, context) { removeMidTurnCalls.push({ @@ -9390,6 +9452,86 @@ describe('createServeApp', () => { expect(bridge.clearSessionGoalCalls).toEqual(['s-1']); }); + it('reads and controls the canonical session Goal', async () => { + const snapshot: GoalSnapshotV2 = { + v: 2, + activity: 'idle', + goal: { + goalId: 'goal-1', + revision: 3, + objective: 'ship it', + status: 'active', + evidenceCursor: { recordId: null }, + turnCount: 1, + activeTimeMs: 1000, + createdAt: 1, + updatedAt: 2, + }, + }; + const bridge = fakeBridge({ + getSessionGoalImpl: async () => ({ snapshot, active: null }), + controlSessionGoalImpl: async () => ({ snapshot }), + knownClientIds: ['client-1'], + }); + const tokenOpts: ServeOptions = { ...baseOpts, token: 'secret' }; + const app = createServeApp( + { ...tokenOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + + const read = await request(app) + .get('/session/s-1/goal') + .set('Host', `127.0.0.1:${tokenOpts.port}`) + .set('Authorization', 'Bearer secret'); + const controlled = await request(app) + .post('/session/s-1/goal') + .set('Host', `127.0.0.1:${tokenOpts.port}`) + .set('Authorization', 'Bearer secret') + .set('X-Qwen-Client-Id', 'client-1') + .send({ + action: 'pause', + expectedGoalId: 'goal-1', + expectedRevision: 3, + }); + + expect(read.status).toBe(200); + expect(read.body).toEqual({ snapshot }); + expect(controlled.status).toBe(200); + expect(controlled.body).toEqual({ snapshot }); + expect(bridge.controlSessionGoalCalls).toEqual([ + { + sessionId: 's-1', + request: { + action: 'pause', + expectedGoalId: 'goal-1', + expectedRevision: 3, + }, + context: { clientId: 'client-1' }, + }, + ]); + }); + + it('rejects an invalid Goal control before bridge dispatch', async () => { + const bridge = fakeBridge(); + const tokenOpts: ServeOptions = { ...baseOpts, token: 'secret' }; + const app = createServeApp( + { ...tokenOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + + const res = await request(app) + .post('/session/s-1/goal') + .set('Host', `127.0.0.1:${tokenOpts.port}`) + .set('Authorization', 'Bearer secret') + .send({ action: 'pause', expectedGoalId: 'goal-1' }); + + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_goal_control_request'); + expect(bridge.controlSessionGoalCalls).toEqual([]); + }); + it('maps goal clear bridge errors', async () => { const bridge = fakeBridge({ clearSessionGoalImpl: async (sessionId) => { @@ -9901,6 +10043,7 @@ describe('createServeApp', () => { message: 'hello', context: { clientId: 'client-9' }, messageId: 'client-mid-1', + options: { rejectIfIdle: true }, }, ]); }); @@ -9917,10 +10060,38 @@ describe('createServeApp', () => { ); expect(res.status).toBe(200); expect(bridge.enqueueMidTurnCalls).toEqual([ - { sessionId: 's-1', message: 'hi', context: { clientId: 'client-9' } }, + { + sessionId: 's-1', + message: 'hi', + context: { clientId: 'client-9' }, + options: { rejectIfIdle: true }, + }, ]); }); + it('rejects an in-flight enqueue that reaches an idle session', async () => { + const bridge = fakeBridge({ + enqueueMidTurnImpl: ( + _sessionId, + _message, + _context, + _messageId, + options, + ) => (options?.rejectIfIdle ? { accepted: false } : { accepted: true }), + }); + + const res = await midTurnPost(midTurnApp(bridge), 's-1', { + message: 'late steering', + messageId: 'late-steering-1', + }); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ accepted: false }); + expect(bridge.enqueueMidTurnCalls[0]?.options).toEqual({ + rejectIfIdle: true, + }); + }); + it.each([[''], [123], ['x'.repeat(129)]])( '400 when `messageId` is invalid: %j', async (messageId) => { @@ -9973,6 +10144,7 @@ describe('createServeApp', () => { sessionId: 's-1', message: 'see this', options: { + rejectIfIdle: true, content: [{ type: 'image', data: 'aW1n', mimeType: 'image/png' }], }, }, @@ -9999,7 +10171,7 @@ describe('createServeApp', () => { { sessionId: 's-1', message: 'read this', - options: { content: [resource] }, + options: { rejectIfIdle: true, content: [resource] }, }, ]); }); @@ -10101,6 +10273,7 @@ describe('createServeApp', () => { sessionId: 's-1', message: 'see this', options: { + rejectIfIdle: true, content: [ { type: 'image', diff --git a/packages/cli/src/serve/server/error-response.test.ts b/packages/cli/src/serve/server/error-response.test.ts index e3fe841d72b..05b00ce960c 100644 --- a/packages/cli/src/serve/server/error-response.test.ts +++ b/packages/cli/src/serve/server/error-response.test.ts @@ -138,6 +138,61 @@ describe('sendBridgeError session writer errors', () => { }); }); + it('maps an untrusted workspace bridge error to 403', () => { + const { response, status, json } = responseMock(); + const error = Object.assign(new Error('Workspace is not trusted'), { + data: { errorKind: 'untrusted_workspace', httpStatus: 403 }, + }); + + sendBridgeError(response, error); + + expect(status).toHaveBeenCalledWith(403); + expect(json).toHaveBeenCalledWith({ + error: 'Workspace is not trusted', + code: 'untrusted_workspace', + }); + }); + + it.each([ + ['goal_conflict', 409], + ['goal_invalid_transition', 409], + ['goal_persist_failed', 500], + ] as const)('maps %s to %i', (kind, expectedStatus) => { + // A persistence failure is not retryable; surfacing it as a 409 sends the + // client back to re-sync `current` and retry a write that cannot succeed, + // and the inverse turns an ordinary conflict into a 500. + const { response, status, json } = responseMock(); + const error = Object.assign(new Error('goal control failed'), { + data: { errorKind: kind }, + }); + + sendBridgeError(response, error); + + expect(status).toHaveBeenCalledWith(expectedStatus); + expect(json).toHaveBeenCalledWith({ + error: 'goal control failed', + code: kind, + }); + }); + + it('forwards the current Goal snapshot on a conflict', () => { + // The client re-syncs from `current` before retrying; dropping it leaves it + // retrying against the revision the daemon just rejected. + const { response, json } = responseMock(); + const current = { v: 2, activity: 'idle', goal: null }; + const error = Object.assign(new Error('goal revision changed'), { + data: { errorKind: 'goal_conflict', current }, + }); + + sendBridgeError(response, error); + + expect(json).toHaveBeenCalledWith({ + error: 'goal revision changed', + code: 'goal_conflict', + current, + }); + }); + it.each([ ['invalid_session_attachment_reference', 400], ['session_attachment_gone', 410], diff --git a/packages/cli/src/serve/server/error-response.ts b/packages/cli/src/serve/server/error-response.ts index 89b4b9d93c5..523faf7f20e 100644 --- a/packages/cli/src/serve/server/error-response.ts +++ b/packages/cli/src/serve/server/error-response.ts @@ -662,6 +662,26 @@ export function sendBridgeError( }); return; } + if (kind === 'untrusted_workspace') { + res.status(403).json({ + error: errorMessage(err), + code: kind, + }); + return; + } + if ( + kind === 'goal_conflict' || + kind === 'goal_invalid_transition' || + kind === 'goal_persist_failed' + ) { + const d = data as { current?: unknown }; + res.status(kind === 'goal_persist_failed' ? 500 : 409).json({ + error: errorMessage(err), + code: kind, + ...(d.current !== undefined ? { current: d.current } : {}), + }); + return; + } if (kind === 'branch_point_invalid') { res.status(409).json({ error: errorMessage(err), diff --git a/packages/cli/src/serve/server/telemetry-catalog.test.ts b/packages/cli/src/serve/server/telemetry-catalog.test.ts index 4c8c7a21f40..f04e17b2e91 100644 --- a/packages/cli/src/serve/server/telemetry-catalog.test.ts +++ b/packages/cli/src/serve/server/telemetry-catalog.test.ts @@ -98,7 +98,7 @@ describe('legacy session telemetry route drift guard', () => { .map(({ method, path }) => `${method} ${path}`) .sort(); - expect(registered).toHaveLength(59); + expect(registered).toHaveLength(61); expect(registered).toEqual(catalog); }); }); diff --git a/packages/cli/src/serve/server/telemetry.test.ts b/packages/cli/src/serve/server/telemetry.test.ts index ea675d074ef..3c351646d96 100644 --- a/packages/cli/src/serve/server/telemetry.test.ts +++ b/packages/cli/src/serve/server/telemetry.test.ts @@ -1062,17 +1062,17 @@ describe('daemonTelemetryMiddleware — recordRequest seam', () => { }); describe('legacy session telemetry route catalog', () => { - it('contains 59 unique routes with the audited 57/2 attribution split', () => { + it('contains 61 unique routes with the audited 59/2 attribution split', () => { const keys = legacySessionTelemetryRoutes.map( ({ method, path }) => `${method} ${path}`, ); - expect(keys).toHaveLength(59); - expect(new Set(keys).size).toBe(59); + expect(keys).toHaveLength(61); + expect(new Set(keys).size).toBe(61); expect( legacySessionTelemetryRoutes.filter( ({ attribution }) => attribution === 'handler_resolved', ), - ).toHaveLength(57); + ).toHaveLength(59); expect( legacySessionTelemetryRoutes.filter( ({ attribution }) => attribution === 'pre_resolved', diff --git a/packages/cli/src/serve/server/telemetry.ts b/packages/cli/src/serve/server/telemetry.ts index 3702373f211..a8765e1ee7e 100644 --- a/packages/cli/src/serve/server/telemetry.ts +++ b/packages/cli/src/serve/server/telemetry.ts @@ -181,6 +181,18 @@ export const legacySessionTelemetryRoutes = [ attribution: 'handler_resolved', route: 'POST /session/:id/tasks/:taskId/cancel', }, + { + method: 'POST', + path: '/session/:id/goal', + attribution: 'handler_resolved', + route: 'POST /session/:id/goal', + }, + { + method: 'GET', + path: '/session/:id/goal', + attribution: 'handler_resolved', + route: 'GET /session/:id/goal', + }, { method: 'POST', path: '/session/:id/goal/clear', diff --git a/packages/core/src/goals/goal-protocol.ts b/packages/core/src/goals/goal-protocol.ts index ed5d2bcf6a1..1fddb0eb177 100644 --- a/packages/core/src/goals/goal-protocol.ts +++ b/packages/core/src/goals/goal-protocol.ts @@ -120,6 +120,13 @@ export interface GoalSnapshotV2 { v: typeof GOAL_STATE_VERSION; goal: GoalRecord | null; activity: GoalActivity; + clearedGoal?: GoalOrder; +} + +export interface GoalOrder { + goalId: string; + revision: number; + updatedAt: number; } /** diff --git a/packages/core/src/goals/goal-reducer.test.ts b/packages/core/src/goals/goal-reducer.test.ts index 8e602b8e6ff..7c5a1f3bff9 100644 --- a/packages/core/src/goals/goal-reducer.test.ts +++ b/packages/core/src/goals/goal-reducer.test.ts @@ -625,6 +625,23 @@ describe('goal reducer', () => { }, ); + it('parses clear snapshots with their cleared goal order', () => { + const value = { + v: 2, + goal: null, + activity: 'idle', + clearedGoal: { goalId: 'g-1', revision: 3, updatedAt: 42 }, + } as const; + + expect(parseGoalSnapshotV2(value)).toEqual(value); + expect( + parseGoalSnapshotV2({ + ...value, + clearedGoal: { ...value.clearedGoal, revision: 0 }, + }), + ).toBeUndefined(); + }); + it.each(['evidence_catalog', 'checkpoint_request'] as const)( 'round-trips a %s limitKind through a persisted snapshot', (limitKind) => { diff --git a/packages/core/src/goals/goal-reducer.ts b/packages/core/src/goals/goal-reducer.ts index 270d374a4f9..c11c7b07503 100644 --- a/packages/core/src/goals/goal-reducer.ts +++ b/packages/core/src/goals/goal-reducer.ts @@ -268,25 +268,48 @@ export function parseGoalSnapshotV2( ): GoalSnapshotV2 | undefined { if ( !isRecord(value) || - !hasOnlyKeys(value, ['v', 'goal', 'activity']) || + !hasOnlyKeys(value, ['v', 'goal', 'activity', 'clearedGoal']) || value['v'] !== GOAL_STATE_VERSION || !isGoalActivity(value['activity']) ) { return undefined; } if (value['goal'] === null) { + const clearedGoal = parseGoalOrder(value['clearedGoal']); + if (value['clearedGoal'] !== undefined && !clearedGoal) return undefined; return { v: GOAL_STATE_VERSION, goal: null, activity: value['activity'], + ...(clearedGoal ? { clearedGoal } : {}), }; } + if (value['clearedGoal'] !== undefined) return undefined; const goal = parseGoalRecord(value['goal']); return goal ? { v: GOAL_STATE_VERSION, goal, activity: value['activity'] } : undefined; } +function parseGoalOrder(value: unknown): GoalSnapshotV2['clearedGoal'] { + if ( + !isRecord(value) || + !hasOnlyKeys(value, ['goalId', 'revision', 'updatedAt']) || + typeof value['goalId'] !== 'string' || + !value['goalId'] || + !isNonNegativeInteger(value['revision']) || + value['revision'] === 0 || + !isFiniteNumber(value['updatedAt']) + ) { + return undefined; + } + return { + goalId: value['goalId'], + revision: value['revision'], + updatedAt: value['updatedAt'], + }; +} + export function parseGoalStateCause( value: unknown, ): GoalStateCause | undefined { diff --git a/packages/core/src/goals/goal-runtime.test.ts b/packages/core/src/goals/goal-runtime.test.ts index 50db7af7904..488d6c304f6 100644 --- a/packages/core/src/goals/goal-runtime.test.ts +++ b/packages/core/src/goals/goal-runtime.test.ts @@ -3817,6 +3817,11 @@ describe('goal runtime', () => { expect(host.preemptGoalTurn).toHaveBeenCalledOnce(); expect(host.started).toHaveLength(2); expect(runtime.getSnapshot().goal).toBeNull(); + expect(runtime.getSnapshot().clearedGoal).toEqual({ + goalId: replaced.snapshot.goal!.goalId, + revision: 1, + updatedAt: replaced.snapshot.goal!.updatedAt, + }); }); it('defensively copies response, subscriber, and getter snapshots', async () => { diff --git a/packages/core/src/goals/goal-runtime.ts b/packages/core/src/goals/goal-runtime.ts index 9e460af861b..de3f15915cd 100644 --- a/packages/core/src/goals/goal-runtime.ts +++ b/packages/core/src/goals/goal-runtime.ts @@ -1419,6 +1419,15 @@ export function createGoalRuntime( v: GOAL_STATE_VERSION, goal: nextGoal, activity: 'idle', + ...(request.action === 'clear' && snapshot.goal + ? { + clearedGoal: { + goalId: snapshot.goal.goalId, + revision: snapshot.goal.revision, + updatedAt: snapshot.goal.updatedAt, + }, + } + : {}), }; try { await options.journal.recordGoalState(recordUuid, { diff --git a/packages/sdk-typescript/src/daemon/DaemonClient.ts b/packages/sdk-typescript/src/daemon/DaemonClient.ts index 501b53eec24..fc8a351cc9e 100644 --- a/packages/sdk-typescript/src/daemon/DaemonClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonClient.ts @@ -121,6 +121,8 @@ import type { DaemonWorkspaceRemovalResult, DaemonWorkspaceUpdate, HeartbeatResult, + GoalControlRequest, + GoalStateResponse, PermissionResponse, PromptContentBlock, PromptResult, @@ -3016,23 +3018,37 @@ export class DaemonClient { ); } - async sessionGoalClear( + sessionGoalClear( sessionId: string, clientId?: string, ): Promise<{ cleared: boolean; condition?: string }> { - return await this.fetchWithTimeout( - `${this.baseUrl}/session/${urlEncode(sessionId)}/goal/clear`, - { - method: 'POST', - headers: this.headers({ 'Content-Type': 'application/json' }, clientId), - body: JSON.stringify({}), - }, - async (res) => { - if (!res.ok) { - throw await this.failOnError(res, 'POST /session/:id/goal/clear'); - } - return (await res.json()) as { cleared: boolean; condition?: string }; - }, + return this.jsonRequest<{ cleared: boolean; condition?: string }>( + `/session/${urlEncode(sessionId)}/goal/clear`, + 'POST /session/:id/goal/clear', + { method: 'POST', body: {}, clientId }, + ); + } + + sessionGoal( + sessionId: string, + clientId?: string, + ): Promise { + return this.jsonRequest( + `/session/${urlEncode(sessionId)}/goal`, + 'GET /session/:id/goal', + { clientId }, + ); + } + + sessionGoalControl( + sessionId: string, + request: GoalControlRequest, + clientId?: string, + ): Promise { + return this.jsonRequest( + `/session/${urlEncode(sessionId)}/goal`, + 'POST /session/:id/goal', + { method: 'POST', body: request, clientId }, ); } diff --git a/packages/sdk-typescript/src/daemon/DaemonSessionClient.ts b/packages/sdk-typescript/src/daemon/DaemonSessionClient.ts index f86635d3d3a..86443702df1 100644 --- a/packages/sdk-typescript/src/daemon/DaemonSessionClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonSessionClient.ts @@ -50,6 +50,8 @@ import type { DaemonSessionTaskStatus, DaemonSessionTasksStatus, HeartbeatResult, + GoalControlRequest, + GoalStateResponse, PermissionResponse, PromptContentBlock, PromptResult, @@ -614,50 +616,43 @@ export class DaemonSessionClient { * policy. Forwards the bound `clientId` so identified clients update * their per-client timestamp instead of just the session-wide one. */ - async heartbeat(): Promise { - return await this.client.heartbeat(this.sessionId, this.clientId); + heartbeat(): Promise { + return this.client.heartbeat(this.sessionId, this.clientId); } - async artifacts(): Promise { - return await this.client.listSessionArtifacts( - this.sessionId, - this.clientId, - ); + artifacts(): Promise { + return this.client.listSessionArtifacts(this.sessionId, this.clientId); } - async addArtifact( + addArtifact( artifact: DaemonSessionArtifactInput, ): Promise { - return await this.client.addSessionArtifact( + return this.client.addSessionArtifact( this.sessionId, artifact, this.clientId, ); } - async removeArtifact( + removeArtifact( artifactId: string, ): Promise { - return await this.client.removeSessionArtifact( + return this.client.removeSessionArtifact( this.sessionId, artifactId, this.clientId, ); } - async setModel(modelId: string): Promise { - return await this.client.setSessionModel( - this.sessionId, - modelId, - this.clientId, - ); + setModel(modelId: string): Promise { + return this.client.setSessionModel(this.sessionId, modelId, this.clientId); } - async setConfigOption( + setConfigOption( configId: 'reasoning_effort', value: string, ): Promise { - return await this.client.setSessionConfigOption( + return this.client.setSessionConfigOption( this.sessionId, configId, value, @@ -665,17 +660,17 @@ export class DaemonSessionClient { ); } - async getRewindSnapshots(): Promise<{ + getRewindSnapshots(): Promise<{ snapshots: DaemonRewindSnapshotInfo[]; }> { - return await this.client.getRewindSnapshots(this.sessionId); + return this.client.getRewindSnapshots(this.sessionId); } - async rewind( + rewind( promptId: string, opts?: { rewindFiles?: boolean }, ): Promise { - return await this.client.rewindSession(this.sessionId, promptId, { + return this.client.rewindSession(this.sessionId, promptId, { clientId: this.clientId, ...(opts?.rewindFiles !== undefined ? { rewindFiles: opts.rewindFiles } @@ -683,8 +678,8 @@ export class DaemonSessionClient { }); } - async fork(directive: string): Promise { - return await this.client.forkSession( + fork(directive: string): Promise { + return this.client.forkSession( this.sessionId, { directive }, this.clientId, @@ -699,10 +694,8 @@ export class DaemonSessionClient { * child both run to completion regardless (no cross-process abort * plumbing in v1). */ - async recap(opts?: { - signal?: AbortSignal; - }): Promise { - return await this.client.recapSession(this.sessionId, { + recap(opts?: { signal?: AbortSignal }): Promise { + return this.client.recapSession(this.sessionId, { ...(opts?.signal ? { signal: opts.signal } : {}), ...(this.clientId ? { clientId: this.clientId } : {}), }); @@ -718,11 +711,11 @@ export class DaemonSessionClient { }); } - async btw( + btw( question: string, opts?: { signal?: AbortSignal }, ): Promise { - return await this.client.btwSession(this.sessionId, question, { + return this.client.btwSession(this.sessionId, question, { ...(opts?.signal ? { signal: opts.signal } : {}), ...(this.clientId ? { clientId: this.clientId } : {}), }); @@ -734,7 +727,7 @@ export class DaemonSessionClient { * create/attach. Accepted requests become daemon-owned even when the active * turn settles while the request is in flight. */ - async enqueueMidTurnMessage( + enqueueMidTurnMessage( message: string, opts?: { signal?: AbortSignal; @@ -742,7 +735,7 @@ export class DaemonSessionClient { content?: PromptContentBlock[]; }, ): Promise { - return await this.client.enqueueMidTurnMessage(this.sessionId, message, { + return this.client.enqueueMidTurnMessage(this.sessionId, message, { ...(opts?.signal ? { signal: opts.signal } : {}), ...(opts?.messageId ? { messageId: opts.messageId } : {}), ...(opts?.content && opts.content.length > 0 @@ -752,10 +745,10 @@ export class DaemonSessionClient { }); } - async removeMidTurnMessage( + removeMidTurnMessage( messageId: string, ): Promise { - return await this.client.removeMidTurnMessage(this.sessionId, messageId, { + return this.client.removeMidTurnMessage(this.sessionId, messageId, { ...(this.clientId ? { clientId: this.clientId } : {}), }); } @@ -818,10 +811,10 @@ export class DaemonSessionClient { }; } - async removePendingPrompt( + removePendingPrompt( promptId: string, ): Promise { - return await this.client.removePendingPrompt(this.sessionId, promptId, { + return this.client.removePendingPrompt(this.sessionId, promptId, { ...(this.clientId ? { clientId: this.clientId } : {}), }); } @@ -832,54 +825,47 @@ export class DaemonSessionClient { * automatically forwards the client id bound when the session was created * or attached. */ - async shellCommand( + shellCommand( command: string, signal?: AbortSignal, ): Promise { - return await this.client.shellCommand(this.sessionId, command, { + return this.client.shellCommand(this.sessionId, command, { ...(signal ? { signal } : {}), ...(this.clientId ? { clientId: this.clientId } : {}), }); } - async context(): Promise { - return await this.client.sessionContext(this.sessionId, this.clientId); + context(): Promise { + return this.client.sessionContext(this.sessionId, this.clientId); } - async status(): Promise { - return await this.client.sessionStatus(this.sessionId, this.clientId); + status(): Promise { + return this.client.sessionStatus(this.sessionId, this.clientId); } - async contextUsage( + contextUsage( opts: { detail?: boolean } = {}, ): Promise { - return await this.client.sessionContextUsage( - this.sessionId, - opts, - this.clientId, - ); + return this.client.sessionContextUsage(this.sessionId, opts, this.clientId); } - async supportedCommands(): Promise { - return await this.client.sessionSupportedCommands( - this.sessionId, - this.clientId, - ); + supportedCommands(): Promise { + return this.client.sessionSupportedCommands(this.sessionId, this.clientId); } - async tasks(): Promise { - return await this.client.sessionTasks(this.sessionId, this.clientId); + tasks(): Promise { + return this.client.sessionTasks(this.sessionId, this.clientId); } - async lspStatus(): Promise { - return await this.client.sessionLspStatus(this.sessionId, this.clientId); + lspStatus(): Promise { + return this.client.sessionLspStatus(this.sessionId, this.clientId); } - async cancelTask( + cancelTask( taskId: string, kind: DaemonSessionTaskStatus['kind'], ): Promise<{ cancelled: boolean }> { - return await this.client.sessionTaskCancel( + return this.client.sessionTaskCancel( this.sessionId, taskId, kind, @@ -887,12 +873,24 @@ export class DaemonSessionClient { ); } - async clearGoal(): Promise<{ cleared: boolean; condition?: string }> { - return await this.client.sessionGoalClear(this.sessionId, this.clientId); + clearGoal(): Promise<{ cleared: boolean; condition?: string }> { + return this.client.sessionGoalClear(this.sessionId, this.clientId); + } + + goal(): Promise { + return this.client.sessionGoal(this.sessionId, this.clientId); + } + + controlGoal(request: GoalControlRequest): Promise { + return this.client.sessionGoalControl( + this.sessionId, + request, + this.clientId, + ); } - async stats(): Promise { - return await this.client.sessionStats(this.sessionId, this.clientId); + stats(): Promise { + return this.client.sessionStats(this.sessionId, this.clientId); } async respondToPermission( diff --git a/packages/sdk-typescript/src/daemon/index.ts b/packages/sdk-typescript/src/daemon/index.ts index 56899b4a79a..b32b7c0c915 100644 --- a/packages/sdk-typescript/src/daemon/index.ts +++ b/packages/sdk-typescript/src/daemon/index.ts @@ -344,6 +344,14 @@ export type { KnownDaemonEvent, } from './events.js'; export type { + GoalActivity, + GoalControlRequest, + GoalLimitKind, + GoalRecord, + GoalSnapshotV2, + GoalStateResponse, + GoalStatus, + TranscriptCursor, DaemonAgentLevel, DaemonAgentMutationResult, DaemonGeneratedAgentContent, diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index a9520856eef..9b1d23b0e3a 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -15,6 +15,70 @@ export type DaemonMode = 'http-bridge' | 'native'; +/** Goal v2 wire types, duplicated here to keep the SDK independent of Core. */ +export type GoalStatus = + | 'active' + | 'paused' + | 'blocked' + | 'usage_limited' + | 'complete'; + +export type GoalActivity = 'idle' | 'running' | 'verifying'; + +export interface TranscriptCursor { + recordId: string | null; +} + +/** + * Why the runtime stopped a Goal at one of its enumerated bounds. Set alongside + * `lastReason` — that stays the human-readable half, this is the half a client + * may key behavior off (an evidence-limited Goal cannot be resumed). + */ +export type GoalLimitKind = 'evidence_catalog' | 'checkpoint_request'; + +export interface GoalRecord { + goalId: string; + revision: number; + objective: string; + status: GoalStatus; + evidenceCursor: TranscriptCursor; + turnCount: number; + activeTimeMs: number; + createdAt: number; + updatedAt: number; + lastReason?: string; + limitKind?: GoalLimitKind; +} + +export interface GoalSnapshotV2 { + v: 2; + goal: GoalRecord | null; + activity: GoalActivity; + clearedGoal?: { + goalId: string; + revision: number; + updatedAt: number; + }; +} + +export type GoalControlRequest = + | { action: 'create'; objective: string } + | { + action: 'replace' | 'edit'; + objective: string; + expectedGoalId: string; + expectedRevision: number; + } + | { + action: 'pause' | 'resume' | 'clear'; + expectedGoalId: string; + expectedRevision: number; + }; + +export interface GoalStateResponse { + snapshot: GoalSnapshotV2; +} + export interface DaemonProtocolVersions { current: string; supported: string[]; diff --git a/packages/sdk-typescript/test/unit/DaemonClient.test.ts b/packages/sdk-typescript/test/unit/DaemonClient.test.ts index b0a9464552a..d62ef63dfbd 100644 --- a/packages/sdk-typescript/test/unit/DaemonClient.test.ts +++ b/packages/sdk-typescript/test/unit/DaemonClient.test.ts @@ -24,6 +24,9 @@ import { import type { BranchSessionRequest, DaemonCapabilities, + GoalControlRequest, + GoalSnapshotV2, + GoalStateResponse, DaemonSessionContextStatus, DaemonSessionLspStatus, DaemonSessionOrganizationResult, @@ -37,6 +40,22 @@ import type { DaemonWorkspaceSkillsStatus, } from '../../src/daemon/types.js'; +const GOAL_SNAPSHOT: GoalSnapshotV2 = { + v: 2, + activity: 'running', + goal: { + goalId: 'goal-1', + revision: 3, + objective: 'ship it', + status: 'active', + evidenceCursor: { recordId: 'record-1' }, + turnCount: 2, + activeTimeMs: 4000, + createdAt: 1000, + updatedAt: 2000, + }, +}; + function jsonResponse(status: number, body: unknown): Response { return new Response(JSON.stringify(body), { status, @@ -121,6 +140,59 @@ function recordingFetch( } describe('DaemonClient', () => { + describe('session Goal lifecycle', () => { + it('reads and controls the authoritative snapshot with client identity', async () => { + const response: GoalStateResponse = { snapshot: GOAL_SNAPSHOT }; + const { fetch, calls } = recordingFetch(() => + jsonResponse(200, response), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + const request: GoalControlRequest = { + action: 'pause', + expectedGoalId: 'goal-1', + expectedRevision: 3, + }; + + await expect( + client.sessionGoal('session/1', 'client-1'), + ).resolves.toEqual(response); + await expect( + client.sessionGoalControl('session/1', request, 'client-1'), + ).resolves.toEqual(response); + + expect(calls.map(({ url, method }) => ({ url, method }))).toEqual([ + { url: 'http://daemon/session/session%2F1/goal', method: 'GET' }, + { url: 'http://daemon/session/session%2F1/goal', method: 'POST' }, + ]); + expect(calls.map((call) => call.headers['x-qwen-client-id'])).toEqual([ + 'client-1', + 'client-1', + ]); + expect(JSON.parse(calls[1]!.body!)).toEqual(request); + }); + + it('preserves a Goal conflict body through DaemonHttpError', async () => { + const conflict = { + error: 'Goal revision is stale', + code: 'goal_conflict', + current: GOAL_SNAPSHOT, + }; + const { fetch } = recordingFetch(() => jsonResponse(409, conflict)); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + + const error = await client + .sessionGoalControl('s-1', { + action: 'pause', + expectedGoalId: 'goal-1', + expectedRevision: 2, + }) + .catch((reason: unknown) => reason); + + expect(error).toBeInstanceOf(DaemonHttpError); + expect(error).toMatchObject({ status: 409, body: conflict }); + }); + }); + describe('normalizePendingPromptLimit', () => { it('defaults undefined to 5', () => { expect(normalizePendingPromptLimit(undefined)).toBe(5); diff --git a/packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts b/packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts index d25d2a54c67..34d7d7f7a4a 100644 --- a/packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts +++ b/packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts @@ -13,6 +13,10 @@ import { DaemonSessionClient, type DaemonSessionSubscribeOptions, } from '../../src/daemon/DaemonSessionClient.js'; +import type { + GoalControlRequest, + GoalSnapshotV2, +} from '../../src/daemon/types.js'; import { AutoReconnectTransport } from '../../src/daemon/AutoReconnectTransport.js'; import { DaemonTransportClosedError, @@ -21,6 +25,22 @@ import { type DaemonTransportType, } from '../../src/daemon/DaemonTransport.js'; +const GOAL_SNAPSHOT: GoalSnapshotV2 = { + v: 2, + activity: 'idle', + goal: { + goalId: 'goal-1', + revision: 4, + objective: 'ship it', + status: 'paused', + evidenceCursor: { recordId: null }, + turnCount: 1, + activeTimeMs: 2000, + createdAt: 1000, + updatedAt: 3000, + }, +}; + function jsonResponse(status: number, body: unknown): Response { return new Response(JSON.stringify(body), { status, @@ -139,6 +159,42 @@ function turnCompleteFrame(promptId: string): string { } describe('DaemonSessionClient', () => { + it('binds Goal reads and controls to the session and client identity', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(200, { snapshot: GOAL_SNAPSHOT }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + const session = new DaemonSessionClient({ + client, + session: { + sessionId: 's-1', + workspaceCwd: '/work/a', + attached: true, + clientId: 'client-1', + }, + }); + const request: GoalControlRequest = { + action: 'resume', + expectedGoalId: 'goal-1', + expectedRevision: 4, + }; + + await expect(session.goal()).resolves.toEqual({ snapshot: GOAL_SNAPSHOT }); + await expect(session.controlGoal(request)).resolves.toEqual({ + snapshot: GOAL_SNAPSHOT, + }); + + expect(calls.map(({ url, method }) => ({ url, method }))).toEqual([ + { url: 'http://daemon/session/s-1/goal', method: 'GET' }, + { url: 'http://daemon/session/s-1/goal', method: 'POST' }, + ]); + expect(calls.map((call) => call.headers['x-qwen-client-id'])).toEqual([ + 'client-1', + 'client-1', + ]); + expect(JSON.parse(calls[1]!.body!)).toEqual(request); + }); + it('creates or attaches a daemon session and exposes session metadata', async () => { const { fetch, calls } = recordingFetch(() => jsonResponse(200, { diff --git a/packages/web-shell/client/App.test.tsx b/packages/web-shell/client/App.test.tsx index 9d5dfc4d7f3..f670cf584cb 100644 --- a/packages/web-shell/client/App.test.tsx +++ b/packages/web-shell/client/App.test.tsx @@ -12,6 +12,7 @@ import { type DaemonSessionStatsStatus, type DaemonSettingDescriptor, type DaemonWorkspaceGitStatus, + type GoalSnapshotV2, } from '@qwen-code/sdk/daemon'; import type { WebShellApi } from './App'; import type { Message } from './adapters/types'; @@ -48,8 +49,30 @@ type MockConnection = { gitStatus?: DaemonWorkspaceGitStatus; voiceTarget?: VoiceWorkspaceTarget; voiceStatusRevision?: VoiceStatusRevision; + goalState?: GoalSnapshotV2; }; +function activeGoalSnapshot( + objective = 'ship it', + revision = 1, +): GoalSnapshotV2 { + return { + v: 2, + activity: 'idle', + goal: { + goalId: 'goal-1', + revision, + objective, + status: 'active', + evidenceCursor: { recordId: null }, + turnCount: 2, + activeTimeMs: 1_000, + createdAt: 123, + updatedAt: 456, + }, + }; +} + type ChatEditorTestProps = { onSubmit: ( text: string, @@ -244,6 +267,17 @@ const { }), submitPermission: vi.fn().mockResolvedValue(true), clearGoal: vi.fn().mockResolvedValue(undefined), + getGoal: vi.fn().mockResolvedValue({ + snapshot: { v: 2, activity: 'idle', goal: null }, + }), + controlGoal: vi.fn().mockResolvedValue({ + snapshot: { v: 2, activity: 'idle', goal: null }, + }), + applyGoalSnapshot: vi.fn((sessionId: string, snapshot: unknown) => { + if (mockConnection.sessionId === sessionId) { + mockConnection.goalState = snapshot as never; + } + }), forkSession: vi.fn().mockResolvedValue({ launched: false }), sendShellCommand: vi.fn().mockResolvedValue(undefined), cancel: vi.fn().mockResolvedValue(undefined), @@ -297,6 +331,9 @@ const { updateScheduledTask: vi.fn(), deleteScheduledTask: vi.fn(), deleteModel: vi.fn().mockResolvedValue(undefined), + controlGoal: vi.fn().mockResolvedValue({ + snapshot: { v: 2, activity: 'idle', goal: null }, + }), }, mockMcp: { initialize: vi.fn().mockResolvedValue({ accepted: true }), @@ -333,6 +370,7 @@ const { streamingState: 'idle' as StreamingState, blocks: [] as unknown[], messages: [] as unknown[], + queuedPromptHoldHistory: [] as boolean[], chatEditorRenderCount: 0, latestChatEditorProps: null as ChatEditorTestProps | null, latestToastHostElevated: false, @@ -559,15 +597,20 @@ vi.mock('./hooks/useAnimationFrameValue', () => ({ })); vi.mock('./hooks/useQueuedPrompts', () => ({ - useQueuedPrompts: () => ({ - queuedPrompts: [], - queuedTexts, - enqueuePrompt: rawEnqueuePrompt, - removeQueuedPrompt: vi.fn(), - editQueuedPrompt: vi.fn(), - editLastQueuedPrompt, - clearQueuedPrompts, - }), + useQueuedPrompts: (args: { holdQueuedPromptsLocally?: boolean }) => { + testState.queuedPromptHoldHistory.push( + args.holdQueuedPromptsLocally === true, + ); + return { + queuedPrompts: [], + queuedTexts, + enqueuePrompt: rawEnqueuePrompt, + removeQueuedPrompt: vi.fn(), + editQueuedPrompt: vi.fn(), + editLastQueuedPrompt, + clearQueuedPrompts, + }; + }, })); vi.mock('./utils/systemInfo', () => ({ @@ -4672,6 +4715,9 @@ beforeEach(() => { }; mockConnection.gitBranch = undefined; mockConnection.gitStatus = undefined; + // A loaded session always carries a Goal snapshot; tests that exercise the + // hydration window (goalState still unknown) set it back to undefined. + mockConnection.goalState = { v: 2, activity: 'idle', goal: null }; testState.ownerVersion = 0; mockWorkspace.capabilities = { workspaces: [{ id: 'primary', cwd: '/workspace', primary: true }], @@ -4717,6 +4763,7 @@ beforeEach(() => { testState.streamingState = 'idle'; testState.blocks = []; testState.messages = []; + testState.queuedPromptHoldHistory = []; testState.chatEditorRenderCount = 0; testState.latestChatEditorProps = null; testState.latestToastHostElevated = false; @@ -4810,6 +4857,12 @@ beforeEach(() => { }); mockSessionActions.submitPermission.mockResolvedValue(undefined); mockSessionActions.clearGoal.mockResolvedValue(undefined); + mockSessionActions.getGoal.mockResolvedValue({ + snapshot: { v: 2, activity: 'idle', goal: null }, + }); + mockSessionActions.controlGoal.mockResolvedValue({ + snapshot: { v: 2, activity: 'idle', goal: null }, + }); mockSessionActions.forkSession.mockResolvedValue({ launched: false }); mockSessionActions.sendShellCommand.mockResolvedValue(undefined); mockSessionActions.cancel.mockResolvedValue(undefined); @@ -4844,6 +4897,9 @@ beforeEach(() => { modifiedMs: 0, }); mockWorkspaceActions.loadProviders.mockResolvedValue({ current: null }); + mockWorkspaceActions.controlGoal.mockResolvedValue({ + snapshot: { v: 2, activity: 'idle', goal: null }, + }); mockWorkspaceActions.loadPreflight.mockResolvedValue(null); mockWorkspaceActions.loadEnv.mockResolvedValue(null); mockCollectSystemInfo.mockImplementation(() => ({ @@ -5407,6 +5463,21 @@ describe('App shell command queueing', () => { ); }); + it('runs an idle shell command immediately while a Goal is active', async () => { + mockConnection.goalState = activeGoalSnapshot('keep working'); + renderApp({}); + await flush(); + + await act(async () => { + testState.latestChatEditorProps?.onSubmit('!pwd'); + await vi.waitFor(() => { + expect(mockSessionActions.sendShellCommand).toHaveBeenCalledWith('pwd'); + }); + }); + + expect(rawEnqueuePrompt).not.toHaveBeenCalled(); + }); + it('blocks duplicate ! submission while session creation is in flight', async () => { mockConnection.sessionId = undefined; let resolveCreate!: () => void; @@ -10741,29 +10812,23 @@ describe('App session callbacks', () => { expect(onSessionIdChange).not.toHaveBeenCalled(); }); - it('preserves active goal for the same session and clears it after session changes', async () => { + it('exposes canonical goal state to custom footers and clears it after session changes', async () => { + const snapshots: unknown[] = []; const activeGoals: unknown[] = []; + mockConnection.goalState = activeGoalSnapshot(); const { rerender } = renderApp({ renderFooter: (props) => { + snapshots.push(props.goalSnapshot); activeGoals.push(props.activeGoal); return null; }, }); await flush(); - await act(async () => { - window.dispatchEvent( - new CustomEvent('web-shell-goal-status-active', { - detail: { - active: true, - condition: 'ship it', - setAt: 123, - }, - }), - ); - await Promise.resolve(); + expect(snapshots.at(-1)).toMatchObject({ + v: 2, + goal: { goalId: 'goal-1', objective: 'ship it' }, }); - expect(activeGoals.at(-1)).toMatchObject({ condition: 'ship it', setAt: 123, @@ -10772,29 +10837,121 @@ describe('App session callbacks', () => { mockConnection.errorStatus = 404; rerender({ renderFooter: (props) => { + snapshots.push(props.goalSnapshot); activeGoals.push(props.activeGoal); return null; }, }); await flush(); + expect(snapshots.at(-1)).toMatchObject({ + goal: { goalId: 'goal-1' }, + }); expect(activeGoals.at(-1)).toMatchObject({ condition: 'ship it', setAt: 123, }); mockConnection.sessionId = 'session-2'; + mockConnection.goalState = undefined; rerender({ renderFooter: (props) => { + snapshots.push(props.goalSnapshot); activeGoals.push(props.activeGoal); return null; }, }); await flush(); + expect(snapshots.at(-1)).toBeNull(); expect(activeGoals.at(-1)).toBeNull(); }); + it('refuses /language ui while a Goal owns the session', async () => { + // The daemon sync is what makes the agent answer in the new language; if + // it is skipped the chrome switches alone and the agent keeps replying in + // the old one for the rest of the Goal run. + const onToast = vi.fn(); + mockConnection.goalState = activeGoalSnapshot('keep working'); + renderApp({ onToast }); + await flush(); + + let accepted: boolean | undefined; + await act(async () => { + accepted = testState.latestChatEditorProps?.onSubmit('/language ui zh'); + await flush(); + }); + + expect(accepted).toBe(false); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + expect(onToast).toHaveBeenCalledWith( + 'error', + "Slash commands can't be queued while a turn is running.", + ); + }); + + it('holds a composer prompt while Goal state is still hydrating', async () => { + // The session load clears `loadingTranscript` before its `goal()` fetch + // resolves, so the composer is writable while the Goal state is unknown. + // The queue-hold gate already fails closed on that state; a direct submit + // must too, or a prompt typed in that window is sent straight into a Goal + // the client has not learned about yet. + mockConnection.goalState = undefined; + renderApp(); + await flush(); + + let accepted: boolean | undefined; + await act(async () => { + accepted = testState.latestChatEditorProps?.onSubmit( + 'hello during hydration', + ); + await flush(); + }); + + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + expect(rawEnqueuePrompt).toHaveBeenCalledTimes(1); + expect(rawEnqueuePrompt.mock.calls[0]?.[0]).toBe('hello during hydration'); + expect(accepted).toBe(true); + }); + + it('holds queued prompts on the first render of an active Goal', async () => { + mockConnection.goalState = activeGoalSnapshot(); + + renderApp(); + await flush(); + + expect(testState.queuedPromptHoldHistory.length).toBeGreaterThan(0); + expect(testState.queuedPromptHoldHistory).not.toContain(false); + }); + + it('restores the Goal snapshot when the same session learns its workspace', async () => { + const snapshots: unknown[] = []; + mockConnection.workspaceCwd = undefined; + mockConnection.goalState = activeGoalSnapshot(); + const { rerender } = renderApp({ + renderFooter: (props) => { + snapshots.push(props.goalSnapshot); + return null; + }, + }); + await flush(); + + act(() => { + mockConnection.workspaceCwd = '/tmp/project'; + rerender({ + renderFooter: (props) => { + snapshots.push(props.goalSnapshot); + return null; + }, + }); + }); + await flush(); + + expect(snapshots.at(-1)).toMatchObject({ + goal: { goalId: 'goal-1', objective: 'ship it' }, + }); + }); + it('gates direct submissions and dispatches compatible submit events', async () => { const onSubmitBefore = vi.fn().mockResolvedValue(undefined); const onSessionChange = vi.fn(); @@ -10936,6 +11093,28 @@ describe('App session callbacks', () => { expect(testState.latestChatEditorProps?.isPreparing).toBe(false); }); + it('keeps a daemon-bound draft when onSubmitBefore rejects', async () => { + // The direct-submission path (streaming idle) must leave the composer + // untouched when the host refuses the prompt: clearing or committing the + // editor on rejection silently discards what the user typed. + const onSubmitBefore = vi.fn().mockRejectedValue(new Error('host says no')); + const { container } = renderApp({ onSubmitBefore }); + await flush(); + + await clickSubmit(container); + await flush(); + + expect(onSubmitBefore).toHaveBeenCalledWith({ + sessionId: 'session-1', + prompt: 'hello', + }); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + expect(editorCommit).not.toHaveBeenCalled(); + expect(editorClear).not.toHaveBeenCalled(); + expect(testState.prompt).toBe('hello'); + expect(testState.latestChatEditorProps?.isPreparing).toBe(false); + }); + it('cancels an approved direct submission after a session transition', async () => { let approve: (() => void) | undefined; const onSubmitBefore = vi.fn( @@ -13862,7 +14041,7 @@ describe('App session callbacks', () => { expect(editorCommit).not.toHaveBeenCalled(); }); - it('keeps daemon-bound slash command drafts when onSubmitBefore rejects', async () => { + it('keeps goal controls on the control plane instead of prompt admission', async () => { const onSubmitBefore = vi.fn().mockRejectedValue(new Error('blocked')); const { container } = renderApp({ onSubmitBefore }); await flush(); @@ -13871,13 +14050,14 @@ describe('App session callbacks', () => { await clickSubmit(container); await flush(); - expect(onSubmitBefore).toHaveBeenCalledWith({ - sessionId: 'session-1', - prompt: '/goal ship it', + await vi.waitFor(() => { + expect(mockSessionActions.controlGoal).toHaveBeenCalledWith({ + action: 'create', + objective: 'ship it', + }); }); + expect(onSubmitBefore).not.toHaveBeenCalled(); expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); - expect(editorCommit).not.toHaveBeenCalled(); - expect(editorClear).not.toHaveBeenCalled(); }); it('refreshes background tasks after /fork launches', async () => { @@ -19894,169 +20074,684 @@ describe('App /goal command', () => { expect(rawEnqueuePrompt).not.toHaveBeenCalled(); }); - it('still sends /goal as a prompt rather than opening the page', async () => { + it('creates a goal through the canonical control plane without sending a prompt', async () => { const { container } = renderApp(); await flush(); + mockSessionActions.getGoal.mockClear(); + mockSessionActions.controlGoal.mockResolvedValueOnce({ + snapshot: activeGoalSnapshot('ship it'), + }); testState.prompt = '/goal ship it'; await clickSubmit(container); - await flush(); + await vi.waitFor(() => { + expect(mockSessionActions.controlGoal).toHaveBeenCalledWith({ + action: 'create', + objective: 'ship it', + }); + }); expect(container.querySelector('[data-testid="goals-page"]')).toBeNull(); - expect(mockSessionActions.sendPrompt).toHaveBeenCalled(); + expect(mockSessionActions.getGoal).toHaveBeenCalledTimes(1); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + expect(mockStore.appendLocalUserMessage).toHaveBeenCalledWith( + '/goal ship it', + ); }); - it('still routes /goal clear through the daemon clear path', async () => { + it('refuses a composer control while another goal control is in flight', async () => { + // The strip disables its buttons while a control runs; the composer has no + // disabled state, so without this refusal both controls read the same + // snapshot, stamp the same expected revision, and the daemon rejects the + // loser with a 409 surfaced as "Failed to …the goal". + const pendingControl = deferred<{ + snapshot: ReturnType; + }>(); + mockSessionActions.controlGoal.mockReturnValueOnce(pendingControl.promise); const { container } = renderApp(); await flush(); - testState.prompt = '/goal clear'; + testState.prompt = '/goal ship it'; await clickSubmit(container); - await flush(); - - expect(container.querySelector('[data-testid="goals-page"]')).toBeNull(); - expect(mockSessionActions.clearGoal).toHaveBeenCalled(); - }); - - it('starts a goal in a fresh session from the Goals page', async () => { - const { container } = renderApp(); - await flush(); + await vi.waitFor(() => { + expect(mockSessionActions.controlGoal).toHaveBeenCalledTimes(1); + }); - testState.prompt = '/goal'; + testState.prompt = '/goal ship something else'; await clickSubmit(container); await flush(); - const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; - if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); - mockSessionActions.clearSession.mockClear(); - mockSessionActions.sendPrompt.mockClear(); + expect(mockSessionActions.controlGoal).toHaveBeenCalledTimes(1); + expect(mockStore.appendLocalUserMessage).toHaveBeenCalledTimes(1); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); await act(async () => { - await onCreateGoal('all tests pass'); + pendingControl.resolve({ snapshot: activeGoalSnapshot('ship it') }); + await flush(); }); - - // A goal takes over its session's turns, so it starts in a NEW one - // (clearSession is how createNewSession starts one) rather than hijacking - // the conversation the user was already having. - expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); - expect(mockSessionActions.sendPrompt).toHaveBeenCalledWith( - '/goal all tests pass', - expect.anything(), - ); }); - it('keeps the Goals page mounted across createNewSession, not just after it', async () => { - // `createNewSession` switches to the chat view itself, before any await. That - // silently defeated the deferred switch below: by the time `sendPrompt` - // rejected, the Goals page — and the form that renders the error — was already - // gone, dumping the user in an empty chat with no explanation. The handler - // passes `keepView` so the page survives until the prompt is admitted. + it('creates a goal as the first command while the new session is still committing', async () => { + mockConnection.sessionId = undefined; + mockSessionActions.createSession.mockResolvedValueOnce({ + sessionId: 'session-created', + }); + mockWorkspaceActions.controlGoal.mockResolvedValueOnce({ + snapshot: activeGoalSnapshot('first objective'), + }); const { container } = renderApp(); await flush(); - testState.prompt = '/goal'; + testState.prompt = '/goal first objective'; await clickSubmit(container); - await flush(); - - const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; - if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); - mockSessionActions.sendPrompt.mockRejectedValueOnce( - new Error('daemon says no'), - ); - - await act(async () => { - await expect(onCreateGoal('all tests pass')).rejects.toThrow( - 'daemon says no', + await vi.waitFor(() => { + expect(mockWorkspaceActions.controlGoal).toHaveBeenCalledWith( + 'session-created', + { action: 'create', objective: 'first objective' }, ); }); - // createNewSession ran (a fresh session was started) … - expect(mockSessionActions.clearSession).toHaveBeenCalled(); - // … and the Goals page is STILL up, so the rejection has somewhere to land. - expect( - container.querySelector('[data-testid="goals-page"]'), - ).not.toBeNull(); + expect(mockSessionActions.createSession).toHaveBeenCalledOnce(); + expect(mockSessionActions.attachSession).toHaveBeenCalledOnce(); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); }); - it('keeps the Goals page open when the goal prompt is rejected', async () => { + it('re-syncs canonical Goal state after creating it in an allocated session', async () => { + const active = activeGoalSnapshot('first objective'); + mockConnection.sessionId = undefined; + mockSessionActions.createSession.mockResolvedValueOnce({ + sessionId: 'session-created', + }); + mockSessionActions.attachSession.mockImplementationOnce(async () => { + mockConnection.sessionId = 'session-created'; + mockConnection.goalState = { v: 2, activity: 'idle', goal: null }; + }); + mockWorkspaceActions.controlGoal.mockResolvedValueOnce({ + snapshot: active, + }); + mockSessionActions.getGoal.mockImplementationOnce(async () => { + mockConnection.goalState = active; + return { snapshot: active }; + }); const { container } = renderApp(); await flush(); - testState.prompt = '/goal'; + testState.prompt = '/goal first objective'; await clickSubmit(container); - await flush(); - - const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; - if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); - mockSessionActions.sendPrompt.mockRejectedValueOnce( - new Error('daemon says no'), - ); - - await act(async () => { - await expect(onCreateGoal('all tests pass')).rejects.toThrow( - 'daemon says no', - ); + await vi.waitFor(() => { + expect(mockSessionActions.getGoal).toHaveBeenCalledOnce(); }); - // Switching to the chat first would unmount the page, leaving the rejection - // with nowhere to render: the user would land in an empty session with no - // explanation. expect( - container.querySelector('[data-testid="goals-page"]'), - ).not.toBeNull(); + mockWorkspaceActions.controlGoal.mock.invocationCallOrder[0], + ).toBeLessThan(mockSessionActions.getGoal.mock.invocationCallOrder[0]!); + expect(mockConnection.goalState).toBe(active); }); - it('switches to the chat view only after the goal prompt is admitted', async () => { + it('installs the allocated-session Goal before its re-sync resolves', async () => { + // `workspaceActions.controlGoal` does not write `connection.goalState`, so + // without installing the create response the state stays goal-less for a + // whole round trip (up to the action timeout if the GET stalls): the hold + // gate reads false and a prompt typed in that window bypasses the Goal + // queue entirely. + const active = activeGoalSnapshot('first objective'); + mockConnection.sessionId = undefined; + mockSessionActions.createSession.mockResolvedValueOnce({ + sessionId: 'session-created', + }); + mockSessionActions.attachSession.mockImplementationOnce(async () => { + mockConnection.sessionId = 'session-created'; + mockConnection.goalState = { v: 2, activity: 'idle', goal: null }; + }); + mockWorkspaceActions.controlGoal.mockResolvedValueOnce({ + snapshot: active, + }); + // The re-sync never resolves: the create response has to stand on its own. + mockSessionActions.getGoal.mockImplementationOnce( + () => new Promise(() => {}), + ); const { container } = renderApp(); await flush(); - testState.prompt = '/goal'; + testState.prompt = '/goal first objective'; await clickSubmit(container); + await vi.waitFor(() => { + expect(mockSessionActions.applyGoalSnapshot).toHaveBeenCalledWith( + 'session-created', + active, + ); + }); await flush(); - const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; - if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); + expect(mockConnection.goalState).toBe(active); + expect(testState.queuedPromptHoldHistory.at(-1)).toBe(true); + // The App's own snapshot drives the strip; asserting only the connection + // state would re-read what this test's mock wrote. + expect( + container.querySelector('[data-testid="goal-status-strip"]'), + ).not.toBeNull(); + expect(container.textContent).toContain('first objective'); + rawEnqueuePrompt.mockClear(); + mockSessionActions.sendPrompt.mockClear(); + testState.prompt = 'bypass me'; await act(async () => { - await onCreateGoal('all tests pass'); + testState.latestChatEditorProps?.onSubmit('bypass me'); + await flush(); }); - expect(container.querySelector('[data-testid="goals-page"]')).toBeNull(); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + expect(rawEnqueuePrompt.mock.calls[0]?.[0]).toBe('bypass me'); }); - it("opens a goal's session in the chat view", async () => { - // The goal's session transcript IS its history, so the Goals page has to be - // able to hand off to it. Nothing exercised this wiring before. - const { container } = renderApp(); + it('keeps a session-less /goal control in the composer', async () => { + // Returning true wipes the composer, so a control that cannot run without + // a session has to be refused before that happens — the async path would + // otherwise clear the text and leave only a toast. + mockConnection.sessionId = undefined; + renderApp(); await flush(); - testState.prompt = '/goal'; - await clickSubmit(container); + let accepted: boolean | undefined; + act(() => { + accepted = testState.latestChatEditorProps?.onSubmit( + '/goal clear', + undefined, + undefined, + editorCommit, + ); + }); await flush(); - expect( - container.querySelector('[data-testid="goals-page"]'), - ).not.toBeNull(); - const onOpenSession = testState.latestGoalsProps?.onOpenSession; - if (!onOpenSession) throw new Error('onOpenSession was not captured'); - mockSessionActions.loadSession.mockClear(); + expect(accepted).toBe(false); + expect(editorCommit).not.toHaveBeenCalled(); + expect(mockSessionActions.controlGoal).not.toHaveBeenCalled(); + expect(mockSessionActions.createSession).not.toHaveBeenCalled(); + }); - await act(async () => { - onOpenSession('goal-session-9'); + it('reports an objective-less /goal set through the i18n layer', async () => { + // `formatError` prefers `error.message`, so a hardcoded English string in + // the parser would reach the toast untranslated. Localized copy comes from + // the dictionaries, which the zh-CN mount below exercises. + const onToast = vi.fn(); + const { rerender } = renderApp({ onToast }); + await flush(); + + let accepted: boolean | undefined; + act(() => { + accepted = testState.latestChatEditorProps?.onSubmit( + '/goal set', + undefined, + undefined, + editorCommit, + ); }); await flush(); - // Pin the session id, not the options bag — main added a `{ workspaceCwd }` - // second argument and will likely keep evolving it; the id is what this test - // is about. - expect(mockSessionActions.loadSession.mock.calls[0][0]).toBe( - 'goal-session-9', + expect(accepted).toBe(false); + expect(editorCommit).not.toHaveBeenCalled(); + expect(onToast).toHaveBeenCalledWith( + 'error', + '/goal set requires an objective.', ); - // It must leave the Goals page, or the user loads a transcript they cannot see. + + act(() => rerender({ onToast, language: 'zh-CN' })); + await flush(); + act(() => { + testState.latestChatEditorProps?.onSubmit( + '/goal edit', + undefined, + undefined, + editorCommit, + ); + }); + await flush(); + + expect(onToast).toHaveBeenLastCalledWith( + 'error', + '/goal edit 需要提供目标内容。', + ); + }); + + it('keeps /goal attachments in the composer instead of discarding them', async () => { + renderApp(); + await flush(); + testState.prompt = '/goal inspect this screenshot'; + const images = [{ data: 'abc', media_type: 'image/png' }]; + + let accepted: boolean | undefined; + act(() => { + accepted = testState.latestChatEditorProps?.onSubmit( + testState.prompt, + images, + undefined, + editorCommit, + ); + }); + await flush(); + + expect(accepted).toBe(false); + expect(mockSessionActions.controlGoal).not.toHaveBeenCalled(); + expect(mockWorkspaceActions.controlGoal).not.toHaveBeenCalled(); + expect(editorCommit).not.toHaveBeenCalled(); + expect(testState.prompt).toBe('/goal inspect this screenshot'); + }); + + it('drops a lazy /goal create when another session wins allocation', async () => { + mockConnection.sessionId = undefined; + mockSessionActions.createSession.mockResolvedValueOnce({ + sessionId: 'session-created', + }); + const attach = deferred(); + mockSessionActions.attachSession.mockReturnValueOnce(attach.promise); + const { container, rerender } = renderApp(); + await flush(); + + testState.prompt = '/goal first objective'; + void clickSubmit(container); + await vi.waitFor(() => { + expect(mockSessionActions.attachSession).toHaveBeenCalledOnce(); + }); + + act(() => { + testState.ownerVersion += 1; + mockConnection.sessionId = 'other-session'; + rerender({}); + }); + await act(async () => { + attach.resolve(); + await attach.promise; + }); + await flush(); + + expect(mockWorkspaceActions.controlGoal).not.toHaveBeenCalled(); + expect(mockStore.appendLocalUserMessage).not.toHaveBeenCalledWith( + '/goal first objective', + ); + }); + + it('refuses non-set Goal controls without allocating a session', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + mockConnection.sessionId = undefined; + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal clear'; + await clickSubmit(container); + await flush(); + + expect(mockSessionActions.createSession).not.toHaveBeenCalled(); + expect(mockSessionActions.attachSession).not.toHaveBeenCalled(); + expect(mockSessionActions.controlGoal).not.toHaveBeenCalled(); + expect(mockStore.appendLocalUserMessage).not.toHaveBeenCalledWith( + '/goal clear', + ); + }); + + it('replaces an existing goal with compare-and-swap identity', async () => { + const current = activeGoalSnapshot('old objective', 7); + mockConnection.goalState = current; + mockSessionActions.getGoal.mockResolvedValue({ snapshot: current }); + mockSessionActions.controlGoal.mockResolvedValueOnce({ + snapshot: activeGoalSnapshot('new objective', 8), + }); + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal new objective'; + await clickSubmit(container); + await vi.waitFor(() => { + expect(mockSessionActions.controlGoal).toHaveBeenCalledWith({ + action: 'replace', + objective: 'new objective', + expectedGoalId: 'goal-1', + expectedRevision: 7, + }); + }); + + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + }); + + it('clears a goal directly without a confirmation or legacy clear call', async () => { + const current = activeGoalSnapshot('ship it', 4); + mockConnection.goalState = current; + mockSessionActions.getGoal.mockResolvedValue({ snapshot: current }); + const { container } = renderApp(); + await flush(); + mockSessionActions.controlGoal.mockResolvedValueOnce({ + snapshot: { v: 2, activity: 'idle', goal: null }, + }); + + testState.prompt = '/goal clear'; + await clickSubmit(container); + await vi.waitFor(() => { + expect(mockSessionActions.controlGoal).toHaveBeenCalledWith({ + action: 'clear', + expectedGoalId: 'goal-1', + expectedRevision: 4, + }); + }); + + expect(mockSessionActions.clearGoal).not.toHaveBeenCalled(); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + }); + + it('applies explicit controls immediately while a turn is running', async () => { + const current = activeGoalSnapshot('ship it', 5); + mockConnection.goalState = current; + mockSessionActions.getGoal.mockResolvedValue({ snapshot: current }); + const { container, rerender } = renderApp(); + await flush(); + act(() => { + testState.streamingState = 'responding'; + rerender({}); + }); + + testState.prompt = '/goal pause'; + await clickSubmit(container); + await vi.waitFor(() => { + expect(mockSessionActions.controlGoal).toHaveBeenCalledWith({ + action: 'pause', + expectedGoalId: 'goal-1', + expectedRevision: 5, + }); + }); + + expect(rawEnqueuePrompt).not.toHaveBeenCalled(); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + }); + + it('starts a canonical goal in a fresh session from the Goals page', async () => { + const onSessionIdChange = vi.fn(); + const { container } = renderApp({ onSessionIdChange }); + await flush(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + + const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; + if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); + mockSessionActions.clearSession.mockClear(); + mockSessionActions.getGoal.mockClear(); + mockSessionActions.controlGoal.mockResolvedValueOnce({ + snapshot: activeGoalSnapshot('all tests pass'), + }); + + await act(async () => { + await onCreateGoal('all tests pass'); + }); + + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); + expect(mockSessionActions.controlGoal).toHaveBeenCalledWith({ + action: 'create', + objective: 'all tests pass', + }); + // Order is the invariant the flow exists for: dispatching the create + // before the allocation completes would start the Goal inside the + // conversation the user is leaving. + expect( + mockSessionActions.clearSession.mock.invocationCallOrder[0], + ).toBeLessThan(mockSessionActions.controlGoal.mock.invocationCallOrder[0]!); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + expect(onSessionIdChange).not.toHaveBeenCalledWith(undefined); expect(container.querySelector('[data-testid="goals-page"]')).toBeNull(); }); - it("reports a failure to open a goal's session instead of swallowing it", async () => { + it.each(['resolve', 'reject'] as const)( + 'ignores a stale Goal edit %s after the session changes', + async (outcome) => { + const goalA = activeGoalSnapshot('session A objective', 5); + const goalB = { + ...activeGoalSnapshot('session B objective', 1), + goal: { + ...activeGoalSnapshot('session B objective', 1).goal!, + goalId: 'goal-b', + }, + }; + const pending = deferred<{ snapshot: typeof goalA }>(); + mockConnection.goalState = goalA; + mockSessionActions.getGoal.mockResolvedValue({ snapshot: goalA }); + mockSessionActions.controlGoal.mockReturnValueOnce(pending.promise); + const { container, rerender } = renderApp(); + await flush(); + + const editA = container.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Edit goal"]', + ); + if (!editA) throw new Error('session A edit control was not rendered'); + act(() => editA.click()); + const saveA = [ + ...document.querySelectorAll('button'), + ].find((button) => button.textContent === 'Save'); + if (!saveA) throw new Error('session A save control was not rendered'); + act(() => saveA.click()); + await vi.waitFor(() => { + expect(mockSessionActions.controlGoal).toHaveBeenCalledTimes(1); + }); + + act(() => { + testState.ownerVersion += 1; + mockConnection.sessionId = 'session-b'; + mockConnection.goalState = goalB; + rerender({}); + }); + // Session A's dialog must be gone before B's is opened: left open, it + // re-syncs its textarea from B's objective and the user edits B believing + // it is still A. (The same-session replacement case, which only the + // goalId-keyed reset effect covers, is pinned below.) + expect(document.querySelector('textarea')).toBeNull(); + const editB = container.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Edit goal"]', + ); + if (!editB) throw new Error('session B edit control was not rendered'); + act(() => editB.click()); + expect(document.querySelector('textarea')).not.toBeNull(); + + await act(async () => { + if (outcome === 'resolve') pending.resolve({ snapshot: goalA }); + else pending.reject(new Error('session A edit failed')); + await Promise.resolve(); + }); + + expect(document.querySelector('textarea')).not.toBeNull(); + expect(document.querySelector('[role="alert"]')).toBeNull(); + // The stale resolution must not install session A's goal over B's: the + // strip and the dialog would then describe the wrong session's goal. + expect(container.textContent).toContain('session B objective'); + expect(container.textContent).not.toContain('session A objective'); + expect( + document.querySelector('textarea')?.value, + ).toBe('session B objective'); + }, + ); + + it('keeps a Goal control busy latch owned by its own session', async () => { + // Session A's control settles after the user moved to B. Releasing the + // latch unconditionally re-enables B's strip mid-flight, and a second click + // dispatches a duplicate control that dies in the daemon's CAS. + const goalA = activeGoalSnapshot('goal A', 5); + const goalB = { + ...activeGoalSnapshot('goal B', 1), + goal: { + ...activeGoalSnapshot('goal B', 1).goal!, + goalId: 'goal-b', + }, + }; + const pendingA = deferred<{ snapshot: typeof goalA }>(); + const pendingB = deferred<{ snapshot: typeof goalB }>(); + mockConnection.goalState = goalA; + mockSessionActions.getGoal + .mockReturnValueOnce(pendingA.promise) + .mockReturnValueOnce(pendingB.promise); + const { container, rerender } = renderApp(); + await flush(); + + const pauseA = container.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + ); + if (!pauseA) throw new Error('session A pause control was not rendered'); + act(() => pauseA.click()); + + act(() => { + testState.ownerVersion += 1; + mockConnection.sessionId = 'session-b'; + mockConnection.goalState = goalB; + rerender({}); + }); + const pauseB = container.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + ); + if (!pauseB) throw new Error('session B pause control was not rendered'); + act(() => pauseB.click()); + expect( + container.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + )?.disabled, + ).toBe(true); + + await act(async () => { + pendingA.resolve({ snapshot: goalA }); + await flush(); + }); + + expect( + container.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + )?.disabled, + ).toBe(true); + + await act(async () => { + pendingB.resolve({ snapshot: goalB }); + await flush(); + }); + }); + + it('keeps the busy latch when an allocated-session create settles late', async () => { + // `createGoalForAllocatedSession` shares the latch with `controlCurrentGoal` + // but used to release it unconditionally, so a create that settles after + // the user moved on re-enabled the strip under the new session's control. + const created = activeGoalSnapshot('first objective'); + const goalB = activeGoalSnapshot('goal B', 1); + mockConnection.sessionId = undefined; + mockSessionActions.createSession.mockResolvedValueOnce({ + sessionId: 'session-created', + }); + const pendingCreate = deferred<{ snapshot: typeof created }>(); + const pendingPause = deferred<{ snapshot: typeof goalB }>(); + mockWorkspaceActions.controlGoal.mockReturnValueOnce(pendingCreate.promise); + const { container, rerender } = renderApp(); + await flush(); + + testState.prompt = '/goal first objective'; + await clickSubmit(container); + await vi.waitFor(() => { + expect(mockWorkspaceActions.controlGoal).toHaveBeenCalledTimes(1); + }); + + // The user leaves for a session that already has a Goal and pauses it. + mockSessionActions.getGoal.mockReturnValueOnce(pendingPause.promise); + act(() => { + testState.ownerVersion += 1; + mockConnection.sessionId = 'session-b'; + mockConnection.goalState = goalB; + rerender({}); + }); + const pause = container.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + ); + if (!pause) throw new Error('session B pause control was not rendered'); + act(() => pause.click()); + expect( + container.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + )?.disabled, + ).toBe(true); + + await act(async () => { + pendingCreate.resolve({ snapshot: created }); + await flush(); + }); + + expect( + container.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + )?.disabled, + ).toBe(true); + + await act(async () => { + pendingPause.resolve({ snapshot: goalB }); + await flush(); + }); + }); + + it('closes the Goal edit dialog when the same session replaces its goal', async () => { + // Only the goalId-keyed reset effect can close it here — the session key is + // unchanged — and an open dialog would re-sync its textarea to the new + // goal's objective while the user believes they are editing the old one. + const goalA = activeGoalSnapshot('goal A', 5); + const goalB = { + ...activeGoalSnapshot('goal B', 1), + goal: { + ...activeGoalSnapshot('goal B', 1).goal!, + goalId: 'goal-b', + }, + }; + mockConnection.goalState = goalA; + mockSessionActions.getGoal.mockResolvedValue({ snapshot: goalA }); + const { container, rerender } = renderApp(); + await flush(); + + const edit = container.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Edit goal"]', + ); + if (!edit) throw new Error('edit control was not rendered'); + act(() => edit.click()); + expect(document.querySelector('textarea')).not.toBeNull(); + + act(() => { + mockConnection.goalState = goalB; + rerender({}); + }); + + expect(document.querySelector('textarea')).toBeNull(); + }); + + it('rejects a Goal edit when the same session replaces the goal', async () => { + const goalA = activeGoalSnapshot('goal A', 5); + const goalB = { + ...activeGoalSnapshot('goal B', 1), + goal: { + ...activeGoalSnapshot('goal B', 1).goal!, + goalId: 'goal-b', + }, + }; + const pendingGoal = deferred<{ snapshot: typeof goalB }>(); + mockConnection.goalState = goalA; + mockSessionActions.getGoal.mockReturnValueOnce(pendingGoal.promise); + const { container, rerender } = renderApp(); + await flush(); + + const edit = container.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Edit goal"]', + ); + if (!edit) throw new Error('edit control was not rendered'); + act(() => edit.click()); + const save = [ + ...document.querySelectorAll('button'), + ].find((button) => button.textContent === 'Save'); + if (!save) throw new Error('save control was not rendered'); + act(() => save.click()); + act(() => { + mockConnection.goalState = goalB; + rerender({}); + }); + await act(async () => pendingGoal.resolve({ snapshot: goalB })); + + expect(mockSessionActions.controlGoal).not.toHaveBeenCalled(); + }); + + it('keeps the Goals page open when canonical creation is rejected', async () => { const { container } = renderApp(); await flush(); @@ -20064,48 +20759,117 @@ describe('App /goal command', () => { await clickSubmit(container); await flush(); - const onOpenSession = testState.latestGoalsProps?.onOpenSession; - if (!onOpenSession) throw new Error('onOpenSession was not captured'); - mockSessionActions.loadSession.mockRejectedValueOnce( - new Error('session is gone'), + const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; + if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); + mockSessionActions.controlGoal.mockRejectedValueOnce( + new Error('daemon says no'), ); - const consoleError = vi - .spyOn(console, 'error') - .mockImplementation(() => {}); await act(async () => { - onOpenSession('goal-session-9'); + await expect(onCreateGoal('all tests pass')).rejects.toThrow( + 'daemon says no', + ); + }); + + expect( + container.querySelector('[data-testid="goals-page"]'), + ).not.toBeNull(); + }); + + it('reuses the empty session left by a rejected canonical creation', async () => { + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + + const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; + if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); + mockSessionActions.clearSession.mockClear(); + mockSessionActions.controlGoal.mockRejectedValueOnce( + new Error('daemon says no'), + ); + + await act(async () => { + await expect(onCreateGoal('all tests pass')).rejects.toThrow( + 'daemon says no', + ); + }); + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); + + await act(async () => { + await onCreateGoal('all tests pass'); + }); + + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); + expect(mockSessionActions.controlGoal).toHaveBeenLastCalledWith({ + action: 'create', + objective: 'all tests pass', + }); + }); + + it('forgets a rejected goal session after leaving the Goals page', async () => { + const pendingCreate = deferred<{ + snapshot: ReturnType; + }>(); + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; + if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); + mockSessionActions.clearSession.mockClear(); + mockSessionActions.controlGoal.mockReturnValueOnce(pendingCreate.promise); + + let firstCreate!: Promise; + act(() => { + firstCreate = onCreateGoal('all tests pass'); + }); + await vi.waitFor(() => { + expect(mockSessionActions.controlGoal).toHaveBeenCalledOnce(); + }); + act(() => { + container + .querySelector('[data-testid="goals-page"] button') + ?.click(); + }); + await flush(); + expect(container.querySelector('[data-testid="goals-page"]')).toBeNull(); + pendingCreate.reject(new Error('daemon says no')); + await act(async () => { + await expect(firstCreate).rejects.toThrow('daemon says no'); }); + + testState.prompt = '/goal'; + await clickSubmit(container); await flush(); + const retryCreate = testState.latestGoalsProps?.onCreateGoal; + if (!retryCreate) throw new Error('onCreateGoal was not recaptured'); + await act(async () => { + await retryCreate('all tests pass'); + }); - // `loadSidebarSession` rethrows, so the handler's own `.catch` is the only - // thing standing between a dead session and an unhandled rejection. It has - // to route the failure to `reportError` (console + toast), not swallow it. - expect(consoleError).toHaveBeenCalledWith( - '[web-shell]', - expect.stringContaining('session is gone'), - expect.anything(), - ); - consoleError.mockRestore(); + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(2); }); - it('reuses the empty session a failed goal attempt left behind', async () => { - // `sendPrompt` creates the daemon session lazily, so a prompt that fails - // after admission leaves a created-but-empty session. The form keeps the - // condition and invites a retry; if that retry started ANOTHER new session, - // every failed attempt would strand a blank chat in the sidebar. + it('forgets a stranded session recorded before the user leaves the page', async () => { + // Reject FIRST, leave the page second: the `[mainView]` cleanup effect is + // the only thing that forgets the stranded session in that order, and + // without it a later create reuses a session the user has since turned into + // a real conversation. const { container } = renderApp(); await flush(); testState.prompt = '/goal'; await clickSubmit(container); await flush(); - const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); - mockSessionActions.clearSession.mockClear(); - mockSessionActions.sendPrompt.mockRejectedValueOnce( + mockSessionActions.controlGoal.mockRejectedValueOnce( new Error('daemon says no'), ); @@ -20116,38 +20880,43 @@ describe('App /goal command', () => { }); expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); - // Retry: the session from the failed attempt is still current and empty, so - // it is reused rather than abandoned. No second clearSession. + act(() => { + container + .querySelector('[data-testid="goals-page"] button') + ?.click(); + }); + await flush(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + const retryCreate = testState.latestGoalsProps?.onCreateGoal; + if (!retryCreate) throw new Error('onCreateGoal was not recaptured'); await act(async () => { - await onCreateGoal('all tests pass'); + await retryCreate('all tests pass'); }); - expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); - expect(mockSessionActions.sendPrompt).toHaveBeenLastCalledWith( - '/goal all tests pass', - expect.anything(), - ); + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(2); }); - it('forgets the stranded session once the user leaves the Goals page', async () => { - // The stranded session is only a scratch session while the Goals page is - // up. Leave, and the composer can talk to it — reusing it for a later goal - // would drop the goal loop on top of a real conversation, which is the very - // thing starting a fresh session exists to prevent. + it('starts a fresh session for a goal created after a successful one', async () => { + // A failed create records its session so the retry can reuse it. The + // success path has to forget it again — otherwise the NEXT create reuses + // the session the running Goal now owns and degrades into a CAS replace + // against it. const { container } = renderApp(); await flush(); testState.prompt = '/goal'; await clickSubmit(container); await flush(); - const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); - mockSessionActions.clearSession.mockClear(); - mockSessionActions.sendPrompt.mockRejectedValueOnce( + mockSessionActions.controlGoal.mockRejectedValueOnce( new Error('daemon says no'), ); + await act(async () => { await expect(onCreateGoal('all tests pass')).rejects.toThrow( 'daemon says no', @@ -20155,72 +20924,257 @@ describe('App /goal command', () => { }); expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); - // Leave the Goals page via its Back button, then use the session from the - // composer — it is now a real conversation, not a scratch session. - const back = container.querySelector( - '[data-testid="goals-page"] button[aria-label="back"]', - ); - if (!back) throw new Error('Back button not found'); + // The retry reuses the stranded session rather than piling up a blank one. + mockSessionActions.controlGoal.mockResolvedValue({ + snapshot: activeGoalSnapshot('all tests pass'), + }); await act(async () => { - back.dispatchEvent(new MouseEvent('click', { bubbles: true })); + await onCreateGoal('all tests pass'); }); - await flush(); - expect(container.querySelector('[data-testid="goals-page"]')).toBeNull(); + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); - testState.prompt = 'hello from the composer'; + testState.prompt = '/goal'; await clickSubmit(container); await flush(); + const nextCreate = testState.latestGoalsProps?.onCreateGoal; + if (!nextCreate) throw new Error('onCreateGoal was not recaptured'); + await act(async () => { + await nextCreate('and lint is clean'); + }); + + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(2); + }); + + it('does not reuse a session selected while goal creation is in flight', async () => { + const pendingCreate = deferred<{ + snapshot: ReturnType; + }>(); + const { container, rerender } = renderApp(); + await flush(); - // Re-open Goals and set a goal: it must NOT reuse the session the user has - // since been talking to. testState.prompt = '/goal'; await clickSubmit(container); await flush(); - const onCreateGoalAgain = testState.latestGoalsProps?.onCreateGoal; - if (!onCreateGoalAgain) throw new Error('onCreateGoal was not captured'); + const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; + if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); mockSessionActions.clearSession.mockClear(); + mockSessionActions.controlGoal.mockReturnValueOnce(pendingCreate.promise); + + let firstCreate!: Promise; + act(() => { + firstCreate = onCreateGoal('all tests pass'); + }); + await vi.waitFor(() => { + expect(mockSessionActions.controlGoal).toHaveBeenCalledTimes(1); + }); + act(() => { + testState.ownerVersion += 1; + mockConnection.sessionId = 'conversation-session'; + rerender({}); + }); + pendingCreate.reject(new Error('daemon says no')); await act(async () => { - await onCreateGoalAgain('all tests pass'); + await expect(firstCreate).rejects.toThrow('daemon says no'); }); - expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); + const retryCreate = testState.latestGoalsProps?.onCreateGoal; + if (!retryCreate) throw new Error('onCreateGoal was not recaptured'); + mockSessionActions.controlGoal.mockResolvedValueOnce({ + snapshot: activeGoalSnapshot('all tests pass'), + }); + await act(async () => { + await retryCreate('all tests pass'); + }); + + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(2); }); - it('starts a fresh session again once a goal has actually been sent', async () => { - // The reuse above is only for a session stranded by a failure. Once a goal - // lands, that session belongs to it, and the next goal must not be dropped - // on top of the running one. + it('locks goal controls while the current snapshot refresh is in flight', async () => { + const current = activeGoalSnapshot('ship it', 5); + const pendingGoal = deferred<{ snapshot: typeof current }>(); + mockConnection.goalState = current; + mockSessionActions.getGoal.mockReturnValueOnce(pendingGoal.promise); + mockSessionActions.controlGoal.mockResolvedValueOnce({ snapshot: current }); const { container } = renderApp(); await flush(); - testState.prompt = '/goal'; - await clickSubmit(container); + const pause = container.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + ); + if (!pause) throw new Error('pause control was not rendered'); + act(() => pause.click()); + + expect(pause.disabled).toBe(true); + act(() => pause.click()); + expect(mockSessionActions.getGoal).toHaveBeenCalledTimes(1); + + await act(async () => pendingGoal.resolve({ snapshot: current })); + await vi.waitFor(() => { + expect(mockSessionActions.controlGoal).toHaveBeenCalledTimes(1); + }); + }); + + it('does not dispatch a Goal control after the session changes during refresh', async () => { + const current = activeGoalSnapshot('ship it', 5); + const pendingGoal = deferred<{ snapshot: typeof current }>(); + mockConnection.goalState = current; + mockSessionActions.getGoal.mockReturnValueOnce(pendingGoal.promise); + const { container, rerender } = renderApp(); await flush(); - const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; - if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); + const pause = container.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + ); + if (!pause) throw new Error('pause control was not rendered'); + act(() => pause.click()); + act(() => { + testState.ownerVersion += 1; + mockConnection.sessionId = 'session-b'; + rerender({}); + }); + await act(async () => pendingGoal.resolve({ snapshot: current })); - mockSessionActions.clearSession.mockClear(); - mockSessionActions.sendPrompt.mockRejectedValueOnce( - new Error('daemon says no'), + expect(mockSessionActions.controlGoal).not.toHaveBeenCalled(); + }); + + it('releases Goal control busy state after a same-session reattach', async () => { + const current = activeGoalSnapshot('ship it', 5); + const pendingControl = deferred<{ snapshot: typeof current }>(); + mockConnection.goalState = current; + mockSessionActions.getGoal.mockResolvedValue({ snapshot: current }); + mockSessionActions.controlGoal.mockReturnValueOnce(pendingControl.promise); + const { container, rerender } = renderApp(); + await flush(); + + const pause = container.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', ); - await act(async () => { - await expect(onCreateGoal('first goal')).rejects.toThrow( - 'daemon says no', - ); + if (!pause) throw new Error('pause control was not rendered'); + act(() => pause.click()); + await vi.waitFor(() => + expect(mockSessionActions.controlGoal).toHaveBeenCalledOnce(), + ); + act(() => { + testState.ownerVersion += 1; + rerender({}); + }); + await act(async () => pendingControl.resolve({ snapshot: current })); + + expect( + container.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + )?.disabled, + ).toBe(false); + }); + + it('reports an edit failure after the edited Goal disappears', async () => { + const current = activeGoalSnapshot('ship it', 5); + const pendingGoal = deferred<{ + snapshot: { v: 2; activity: 'idle'; goal: null }; + }>(); + const consoleError = vi + .spyOn(console, 'error') + .mockImplementation(() => {}); + mockConnection.goalState = current; + mockSessionActions.getGoal.mockReturnValueOnce(pendingGoal.promise); + const { container, rerender } = renderApp(); + await flush(); + + act(() => { + container + .querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Edit goal"]', + ) + ?.click(); }); + const save = [ + ...document.querySelectorAll('button'), + ].find((button) => button.textContent === 'Save'); + if (!save) throw new Error('save control was not rendered'); + act(() => save.click()); + act(() => { + mockConnection.goalState = { v: 2, activity: 'idle', goal: null }; + rerender({}); + }); + await act(async () => + pendingGoal.resolve({ + snapshot: { v: 2, activity: 'idle', goal: null }, + }), + ); + + expect(consoleError).toHaveBeenCalledWith( + '[web-shell]', + expect.stringContaining('goal'), + expect.any(Error), + ); + consoleError.mockRestore(); + }); + + it("opens a goal's session in the chat view", async () => { + // The goal's session transcript IS its history, so the Goals page has to be + // able to hand off to it. Nothing exercised this wiring before. + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + expect( + container.querySelector('[data-testid="goals-page"]'), + ).not.toBeNull(); + + const onOpenSession = testState.latestGoalsProps?.onOpenSession; + if (!onOpenSession) throw new Error('onOpenSession was not captured'); + mockSessionActions.loadSession.mockClear(); + await act(async () => { - await onCreateGoal('first goal'); + onOpenSession('goal-session-9'); }); - expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); + await flush(); + + // Pin the session id, not the options bag — main added a `{ workspaceCwd }` + // second argument and will likely keep evolving it; the id is what this test + // is about. + expect(mockSessionActions.loadSession.mock.calls[0][0]).toBe( + 'goal-session-9', + ); + // It must leave the Goals page, or the user loads a transcript they cannot see. + expect(container.querySelector('[data-testid="goals-page"]')).toBeNull(); + }); + + it("reports a failure to open a goal's session instead of swallowing it", async () => { + const { container } = renderApp(); + await flush(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + + const onOpenSession = testState.latestGoalsProps?.onOpenSession; + if (!onOpenSession) throw new Error('onOpenSession was not captured'); + mockSessionActions.loadSession.mockRejectedValueOnce( + new Error('session is gone'), + ); + const consoleError = vi + .spyOn(console, 'error') + .mockImplementation(() => {}); - // A brand-new goal after a successful send: fresh session again. await act(async () => { - await onCreateGoal('second goal'); + onOpenSession('goal-session-9'); }); - expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(2); + await flush(); + + // `loadSidebarSession` rethrows, so the handler's own `.catch` is the only + // thing standing between a dead session and an unhandled rejection. It has + // to route the failure to `reportError` (console + toast), not swallow it. + expect(consoleError).toHaveBeenCalledWith( + '[web-shell]', + expect.stringContaining('session is gone'), + expect.anything(), + ); + consoleError.mockRestore(); }); it('does not drop the goal into the current session when the new session fails', async () => { @@ -20236,13 +21190,13 @@ describe('App /goal command', () => { mockSessionActions.clearSession.mockRejectedValueOnce( new Error('daemon unreachable'), ); - mockSessionActions.sendPrompt.mockClear(); + mockSessionActions.controlGoal.mockClear(); await act(async () => { await onCreateGoal('all tests pass'); }); - expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + expect(mockSessionActions.controlGoal).not.toHaveBeenCalled(); }); }); @@ -20253,6 +21207,11 @@ describe('App manual-run orchestration (scheduled tasks)', () => { async function openRunHandler( container: HTMLElement, ): Promise<(prompt: string, sessionId: string | null) => Promise> { + mockConnection.goalState ??= { + v: 2, + activity: 'idle', + goal: null, + }; testState.prompt = '/schedule'; await clickSubmit(container); await flush(); @@ -20304,6 +21263,81 @@ describe('App manual-run orchestration (scheduled tasks)', () => { }); }); + it('rejects an unbound run before admission while Goal is active', async () => { + mockConnection.goalState = activeGoalSnapshot('keep working'); + const { container } = renderApp(); + await flush(); + const run = await openRunHandler(container); + + await act(async () => { + await expect(run('do the thing', null)).rejects.toThrow(/Goal is active/); + }); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + }); + + it('rejects an unbound run while Goal state is hydrating', async () => { + mockConnection.goalState = undefined; + const { container } = renderApp(); + await flush(); + const run = await openRunHandler(container); + mockConnection.goalState = undefined; + + await act(async () => { + await expect(run('do the thing', null)).rejects.toThrow(/Goal is active/); + }); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + }); + + it('starts an unbound manual run when no session is attached yet', async () => { + // Session-less means no Goal can exist and `sendPrompt` allocates a session + // itself, so gating the run on an unknown Goal state here would make every + // Run now on a fresh workspace fail. + admitOnSend(); + const { container, rerender } = renderApp(); + await flush(); + const run = await openRunHandler(container); + act(() => { + mockConnection.sessionId = undefined; + mockConnection.goalState = undefined; + rerender({}); + }); + + await act(async () => { + await expect(run('do the thing', null)).resolves.toBeUndefined(); + }); + expect(mockSessionActions.sendPrompt).toHaveBeenCalled(); + }); + + it('waits for bound-session Goal hydration before admitting a run', async () => { + const { container, rerender } = renderApp(); + await flush(); + const run = await openRunHandler(container); + act(() => { + mockConnection.goalState = undefined; + rerender({}); + }); + + let runError: unknown; + act(() => { + void run('do the thing', 'session-1').catch((error) => { + runError = error; + }); + }); + await flush(); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + + act(() => { + mockConnection.goalState = activeGoalSnapshot('keep working'); + rerender({}); + }); + await vi.waitFor(() => { + expect((runError as Error | undefined)?.message).toMatch( + /Goal is active/, + ); + }); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + }); + it('fires a bound run immediately when its session is already active', async () => { admitOnSend(); const { container } = renderApp(); diff --git a/packages/web-shell/client/App.tsx b/packages/web-shell/client/App.tsx index c78c83d57c9..e02f8708d1e 100644 --- a/packages/web-shell/client/App.tsx +++ b/packages/web-shell/client/App.tsx @@ -48,8 +48,10 @@ import type { DaemonSessionArtifact, DaemonWorkspaceCapability, DaemonWorkspaceGitStatus, + GoalSnapshotV2, } from '@qwen-code/sdk/daemon'; +import { isGoalGateBlocked as isGoalGateBlockedFor } from './utils/goalGate'; import { type SessionGitIntent } from './components/GitModePopover'; import { SESSION_LIST_PAGE_SIZE, @@ -90,6 +92,9 @@ import type { import type { PromptFile, PromptImage } from './adapters/promptTypes'; import type { AttachmentPreviewRequest } from './adapters/messageTypes'; import { StatusBar, type StatusBarHandle } from './components/StatusBar'; +import { GoalStatusStrip } from './components/GoalStatusStrip'; +import composerStatusStyles from './components/ComposerStatusStack.module.css'; +import { GoalEditDialog } from './components/dialogs/GoalEditDialog'; import { StreamingStatus } from './components/StreamingStatus'; import { ToastHost, @@ -160,11 +165,8 @@ import { } from './utils/splitUrl'; import { ScheduledTasksDialog } from './components/dialogs/ScheduledTasksDialog'; import { GoalsDialog } from './components/dialogs/GoalsDialog'; -import { - goalArgOf, - isGoalClearCommand, - isGoalClearKeyword, -} from './utils/goalCondition'; +import { parseWebShellGoalCommand } from './utils/goalCondition'; +import { buildGoalControlRequest } from './utils/goalControlRequest'; import { ExtensionsManagerPage } from './components/extensions/ExtensionsManagerPage'; import { PluginManagerPage } from './components/plugins/PluginManagerPage'; import { ChannelsManagerPage } from './components/channels/ChannelsManagerPage'; @@ -255,11 +257,6 @@ import { } from './components/messages/StatusMessage'; import type { SerializedMcpStatusMessage } from './components/messages/McpStatusMessage'; import { McpManagerPage } from './components/mcp/McpManagerPage'; -import { - GOAL_STATUS_ACTIVE_EVENT, - parseGoalStatusMessage, - serializeGoalStatusMessage, -} from './components/messages/GoalStatusMessage'; import { BtwMessage } from './components/messages/BtwMessage'; import { createAndAttachSessionForPrompt, @@ -489,11 +486,6 @@ function normalizeHiddenCommand(command: string): string { return command.trim().replace(/^\/+/, '').toLowerCase(); } -interface ActiveGoalStatus { - condition: string; - setAt: number; -} - interface SendPromptOptionsWithRetry { optimisticUserMessage?: boolean; images?: PromptImage[]; @@ -751,40 +743,6 @@ function retryTranscriptIdentityMatches( ); } -type GoalStatusTranscriptBlock = DaemonTranscriptBlock & { - text: string; - source?: string; - data?: unknown; -}; - -function parseGoalStatusFromBlock(block: DaemonTranscriptBlock) { - const statusBlock = block as GoalStatusTranscriptBlock; - if (statusBlock.source !== 'goal') return null; - return ( - parseGoalStatusMessage(statusBlock.data) ?? - parseGoalStatusMessage(statusBlock.text) - ); -} - -function getLatestActiveGoalFromBlocks( - blocks: readonly DaemonTranscriptBlock[], -): ActiveGoalStatus | null { - for (let i = blocks.length - 1; i >= 0; i--) { - const block = blocks[i]; - if (block.kind !== 'status') continue; - const status = parseGoalStatusFromBlock(block); - if (!status) continue; - if (status.kind === 'set' || status.kind === 'checking') { - return { - condition: status.condition, - setAt: status.setAt ?? block.serverTimestamp ?? block.createdAt, - }; - } - return null; - } - return null; -} - interface LocalAnchoredMessage { anchorAfterId?: string; anchorIndex: number; @@ -4193,13 +4151,30 @@ export function App({ useEffect(() => { assignComposerRef(composerRef, editorRef.current ?? emptyComposerApi); }, [composerRef]); - const [activeGoal, setActiveGoal] = useState(null); - useLayoutEffect(() => setActiveGoal(null), [logicalSessionKey]); + const [goalSnapshot, setGoalSnapshot] = useState(null); + const goalSnapshotRef = useRef(null); + goalSnapshotRef.current = goalSnapshot; + const [goalControlBusy, setGoalControlBusy] = useState(false); + // Which control operation owns the busy latch, mirroring ChatPane's twin. A + // finishing operation must not release the latch under a newer one that is + // still in flight, or the strip re-enables mid-control and a second dispatch + // races the first against the same expected revision. + const goalControlOpSeqRef = useRef(0); + const goalControlOwnerRef = useRef< + { opId: number; sessionId: string | undefined } | undefined + >(undefined); + const [goalEditOpen, setGoalEditOpen] = useState(false); + const [goalEditError, setGoalEditError] = useState(null); + useLayoutEffect(() => { + setGoalSnapshot(null); + goalControlOwnerRef.current = undefined; + setGoalControlBusy(false); + setGoalEditOpen(false); + setGoalEditError(null); + }, [logicalSessionKey]); const [isCreatingMissingSession, setIsCreatingMissingSession] = useState(false); const creatingMissingSessionRef = useRef(false); - const activeGoalRef = useRef(null); - activeGoalRef.current = activeGoal; const { followupState, onAcceptFollowup, @@ -5375,6 +5350,16 @@ export function App({ }, []); const connectionRef = useRef(connection); connectionRef.current = connection; + /** + * Whether a local action must be held back because a Goal owns the session. + * Reads the latest connection through the ref so callers get the gate as of + * call time; the fail-closed hydration convention lives in the shared + * predicate, which every Goal gate in the client shares. + */ + const isGoalGateBlocked = useCallback( + () => isGoalGateBlockedFor(connectionRef.current), + [], + ); const refreshActiveSessionDisplayName = useCallback(async () => { const activeConnection = connectionRef.current; if (!activeConnection.sessionId || !activeConnection.workspaceCwd) return; @@ -5523,28 +5508,25 @@ export function App({ const onSessionCreatedRef = useRef(onSessionCreated); onSessionCreatedRef.current = onSessionCreated; /** - * The session a failed `/goal` submit left behind. + * The session a failed Goal creation left behind. * - * Setting a goal starts a fresh session and then sends `/goal ` - * into it, but the daemon session is not created by the "new session" step — - * `ensureSessionForPrompt` creates it lazily *inside* `sendPrompt`. So a - * prompt that fails leaves a session that exists but never got its goal. + * Creating a Goal from the Goals page allocates a fresh session and then + * installs the Goal in it; the daemon session is created lazily, so an + * attempt that fails leaves a session that exists but never got its Goal. * - * The Goals form keeps the condition and lets the user retry. Without this - * ref every retry would abandon that session and create another, piling up - * blank chats in the sidebar. Remembering it lets the retry reuse it — no - * session is ever deleted. + * The form keeps the condition and lets the user retry. Without this ref + * every retry would abandon that session and create another, piling up blank + * chats in the sidebar. Remembering it lets the retry reuse it — no session + * is ever deleted. * * Only valid while the Goals page stays mounted. The moment the user leaves, * that session is reachable from the composer and may stop being a scratch - * session, so the effect below forgets it: a later goal then starts a fresh + * session, so the effect below forgets it: a later Goal then starts a fresh * session rather than landing on top of a conversation. */ const strandedGoalSessionRef = useRef(undefined); useEffect(() => { - if (mainView !== 'goals') { - strandedGoalSessionRef.current = undefined; - } + if (mainView !== 'goals') strandedGoalSessionRef.current = undefined; }, [mainView]); const ensureSessionForPrompt = useCallback(() => { const currentSessionId = connectionRef.current.sessionId; @@ -6125,7 +6107,11 @@ export function App({ [pushToast], ); const handleFailedPromptRetry = useCallback(() => { - if (sessionWriteBlockedRef.current || promptPreparationOwnerRef.current) { + if ( + sessionWriteBlockedRef.current || + promptPreparationOwnerRef.current || + isGoalGateBlocked() + ) { return; } let failed = failedPromptRef.current; @@ -6261,6 +6247,7 @@ export function App({ t, updateFailedPrompt, updateUnknownPromptAdmission, + isGoalGateBlocked, ]); const canMutateMidTurn = connection.capabilities?.features.includes( @@ -6277,6 +6264,7 @@ export function App({ queuedTexts, enqueuePrompt: rawEnqueuePrompt, removeQueuedPrompt, + insertQueuedPrompt, editQueuedPrompt, editLastQueuedPrompt, clearQueuedPrompts, @@ -6291,6 +6279,10 @@ export function App({ canInjectMidTurnMedia, workspaceFileActions: artifactWorkspaceActions, streamingState, + holdQueuedPromptsLocally: + connection.sessionId !== undefined && + (connection.goalState === undefined || + connection.goalState.goal?.status === 'active'), sessionActions, store, editorRef, @@ -7072,7 +7064,7 @@ export function App({ reloadWorkspaceSettings(), ]); }; - if (streamingStateRef.current !== 'idle') { + if (streamingStateRef.current !== 'idle' || isGoalGateBlocked()) { handleLanguageChange(previousLanguage); blockLocalCommandDuringTurn(); return; @@ -7095,6 +7087,7 @@ export function App({ selectedLanguage, sessionActions, sessionOwnerGuard, + isGoalGateBlocked, ], ); @@ -7565,43 +7558,29 @@ export function App({ ]); useEffect(() => { - const nextGoal = getLatestActiveGoalFromBlocks(blocks); - setActiveGoal((current) => { - if (!nextGoal) return current ? null : current; - if ( - current?.condition === nextGoal.condition && - current.setAt === nextGoal.setAt - ) { - return current; - } - return nextGoal; - }); - }, [blocks]); + setGoalSnapshot(connection.goalState ?? null); + }, [connection.goalState, connection.sessionId, logicalSessionKey]); + const connectionGoalComplete = + connection.goalState?.goal?.status === 'complete'; useEffect(() => { - const onGoalStatusActive = (event: Event) => { - const detail = ( - event as CustomEvent<{ - active?: boolean; - condition?: string; - setAt?: number; - }> - ).detail; - if (!detail?.active) { - setActiveGoal(null); - return; - } - if (!detail.condition) return; - setActiveGoal({ - condition: detail.condition, - setAt: detail.setAt ?? Date.now(), - }); - }; + setGoalEditOpen(false); + setGoalEditError(null); + }, [ + connection.goalState?.goal?.goalId, + connection.sessionId, + connectionGoalComplete, + ]); - window.addEventListener(GOAL_STATUS_ACTIVE_EVENT, onGoalStatusActive); - return () => - window.removeEventListener(GOAL_STATUS_ACTIVE_EVENT, onGoalStatusActive); - }, []); + const activeGoal = + goalSnapshot?.goal && goalSnapshot.goal.status !== 'complete' + ? { + condition: goalSnapshot.goal.objective, + setAt: goalSnapshot.goal.createdAt, + } + : null; + const liveGoalSnapshot = + goalSnapshot?.goal?.status === 'complete' ? null : goalSnapshot; // Auto-recap: fire when the user returns after being away ≥ 3 minutes const hiddenAtRef = useRef(null); @@ -8466,6 +8445,13 @@ export function App({ const enqueueManualRun = useCallback( (prompt: string): Promise => new Promise((resolve, reject) => { + // Session-less means no Goal can exist (and `sendPrompt` allocates a + // session itself), so gate on the shared predicate rather than on a + // bare `goalState === undefined`, which also fires with no session. + if (isGoalGateBlocked()) { + reject(new Error(t('scheduledTasks.error.goalActive'))); + return; + } let admitted = false; const admit = () => { if (admitted) return; @@ -8483,7 +8469,7 @@ export function App({ }, ); }), - [sendPrompt], + [isGoalGateBlocked, sendPrompt, t], ); // Enqueue the pending bound run once its session is the current, fully-loaded // one — driven both by the effect below (when the session switch changes a @@ -8497,7 +8483,8 @@ export function App({ if ( !pending || conn.sessionId !== pending.sessionId || - conn.loadingTranscript + conn.loadingTranscript || + conn.goalState === undefined ) { return; } @@ -8580,6 +8567,7 @@ export function App({ connection.sessionId, connection.loadingTranscript, connection.catchingUp, + connection.goalState, tryFireBoundRun, ]); @@ -8626,61 +8614,103 @@ export function App({ [handleOpenMonitorDetails, handleOpenShellDetails, openTasksPanel], ); - const dispatchGoalSet = useCallback( - (condition: string, setAt: number) => { - setActiveGoal({ condition, setAt }); - store.dispatch([ - { - type: 'status', - text: serializeGoalStatusMessage({ - kind: 'set', - condition, - setAt, - }), - }, - ]); - }, - [store], - ); + const refreshGoal = useCallback(async () => { + const owner = sessionOwnerGuard.capture(); + const response = await sessionActions.getGoal(); + if (owner.isCurrent()) setGoalSnapshot(response.snapshot); + return response.snapshot; + }, [sessionActions, sessionOwnerGuard]); - const dispatchGoalCleared = useCallback( - (goal: ActiveGoalStatus | null) => { - if (!goal) return; - store.dispatch([ - { - type: 'status', - text: serializeGoalStatusMessage({ - kind: 'cleared', - condition: goal.condition, - durationMs: Date.now() - goal.setAt, - }), - }, - ]); - setActiveGoal(null); + const controlCurrentGoal = useCallback( + async ( + action: 'create' | 'replace' | 'edit' | 'pause' | 'resume' | 'clear', + objective?: string, + ) => { + const busyOwner = sessionOwnerGuard.capture(); + const busySessionId = connectionRef.current.sessionId; + const expectedGoalId = goalSnapshotRef.current?.goal?.goalId; + const opId = ++goalControlOpSeqRef.current; + goalControlOwnerRef.current = { opId, sessionId: busySessionId }; + setGoalControlBusy(true); + try { + const snapshot = await refreshGoal(); + const goal = snapshot.goal; + if ( + (action === 'replace' || action === 'edit') && + goal?.goalId !== expectedGoalId + ) { + throw new Error(t('goals.error.goalUnavailable')); + } + const request = buildGoalControlRequest(action, goal, objective, { + emptyObjective: t('goals.error.emptyCondition'), + goalUnavailable: t('goals.error.goalUnavailable'), + }); + + if (!busyOwner.isCurrent()) { + throw new Error(t('goals.error.goalUnavailable')); + } + const owner = sessionOwnerGuard.capture(); + try { + const response = await sessionActions.controlGoal(request); + if (owner.isCurrent()) setGoalSnapshot(response.snapshot); + return response.snapshot; + } catch (error) { + if (owner.isCurrent()) await refreshGoal().catch(() => undefined); + throw error; + } + } finally { + // A newer operation (or a session change) owns the latch now; leave it + // to whoever owns it rather than releasing it under them. + if (goalControlOwnerRef.current?.opId === opId) { + goalControlOwnerRef.current = undefined; + if (connectionRef.current.sessionId === busySessionId) { + setGoalControlBusy(false); + } + } + } }, - [store], + [refreshGoal, sessionActions, sessionOwnerGuard, t], ); - const handleBusyGoalClear = useCallback( - (text: string) => { - if (sessionWriteBlocked) return false; - if (!requireActiveSessionForLocalCommand()) return false; - const owner = sessionOwnerGuard.capture(); - store.appendLocalUserMessage(text); - sessionActions.clearGoal().catch((error: unknown) => { - if (!owner.isCurrent()) return; - reportError(error, 'Failed to clear /goal'); - }); - return true; + const createGoalForAllocatedSession = useCallback( + async (sessionId: string, objective: string) => { + const opId = ++goalControlOpSeqRef.current; + goalControlOwnerRef.current = { opId, sessionId }; + setGoalControlBusy(true); + try { + const response = await workspaceActions.controlGoal(sessionId, { + action: 'create', + objective, + }); + // The workspace-scoped control does not write `connection.goalState` + // the way `sessionActions.controlGoal` does, so install the create + // response directly. Until it lands, `holdQueuedPromptsLocally` reads + // false and the sync effect re-derives the local snapshot to null — a + // prompt typed in that window would go straight to the daemon instead + // of the Goal queue, and no Goal strip would render. + sessionActions.applyGoalSnapshot(sessionId, response.snapshot); + if ( + !connectionRef.current.sessionId || + connectionRef.current.sessionId === sessionId + ) { + setGoalSnapshot(response.snapshot); + } + if (connectionRef.current.sessionId === sessionId) { + await refreshGoal(); + } + return response.snapshot; + } finally { + // Same ownership rule as `controlCurrentGoal`: a create that settles + // after the user switched sessions must not release a latch a newer + // control now holds, or the strip re-enables mid-control and a second + // dispatch loses the daemon's CAS with a 409. + if (goalControlOwnerRef.current?.opId === opId) { + goalControlOwnerRef.current = undefined; + setGoalControlBusy(false); + } + } }, - [ - reportError, - requireActiveSessionForLocalCommand, - sessionWriteBlocked, - sessionActions, - sessionOwnerGuard, - store, - ], + [refreshGoal, sessionActions, workspaceActions], ); const loadRewindSnapshots = useCallback( @@ -8706,72 +8736,127 @@ export function App({ ); const handleGoalSlashCommand = useCallback( - ( - text: string, - images?: PromptImage[], - files?: PromptFile[], - opts?: { - sendToDaemon?: boolean; - commitComposerAccepted?: ComposerSubmitCommit; - }, - ) => { - const goalArg = goalArgOf(text); - const sendToDaemon = opts?.sendToDaemon ?? true; - const sendGoalPrompt = () => { - const owner = { current: sessionOwnerGuard.capture() }; - const deferComposerCommit = - Boolean(onSubmitBeforeRef.current) || - createSessionPromiseRef.current !== null; - const clearComposerOnPromptStart = - !connectionRef.current.sessionId || deferComposerCommit; - sendPrompt(text, images, files, { - ownerRef: owner, - clearComposerOnPromptStart, - commitComposerAccepted: clearComposerOnPromptStart - ? opts?.commitComposerAccepted - : undefined, - }).catch((error: unknown) => { - if (!owner.current.isCurrent()) return; - reportError(error, 'Failed to send /goal command'); - }); - return clearComposerOnPromptStart ? false : true; - }; + (text: string, hasAttachments: boolean) => { + if (hasAttachments) { + pushToast('error', t('goals.error.attachmentsUnsupported')); + return false; + } + const operation = parseWebShellGoalCommand(text); + if (operation.kind === 'status') { + openGoals(); + return true; + } + if (operation.kind === 'error') { + pushToast( + 'error', + t('goals.error.requiresObjective', { keyword: operation.keyword }), + ); + return false; + } + // Returning true wipes the composer, so the preconditions that can be + // checked here must be checked before that happens — a control typed + // without a session would otherwise lose its text to a toast. + if (!connectionRef.current.sessionId && operation.kind !== 'set') { + pushToast('error', t('localCommand.noSession')); + return false; + } + // The strip disables its buttons while a control is in flight; the + // composer has no disabled state, so it has to refuse here. Two controls + // read the same snapshot and stamp the same `expectedGoalId`/ + // `expectedRevision`, and the daemon rejects the loser with a 409. + if (goalControlOwnerRef.current) { + pushToast('error', t('goals.error.controlBusy')); + return false; + } - if (goalArg && isGoalClearKeyword(goalArg)) { - if (!sendToDaemon) { - store.appendLocalUserMessage(text); - dispatchGoalCleared(activeGoalRef.current); - return true; + void (async () => { + const sourceOwner = sessionOwnerGuard.capture(); + const sourceSessionId = connectionRef.current.sessionId; + let allocatedSessionId: string | undefined; + if (!connectionRef.current.sessionId) { + if (operation.kind !== 'set') { + throw new Error(t('localCommand.noSession')); + } + allocatedSessionId = await ensureSessionForPrompt(); } - return handleBusyGoalClear(text); - } else if (goalArg) { - if (!sendToDaemon) { - store.appendLocalUserMessage(text); - dispatchGoalSet(goalArg, Date.now()); - return true; + const currentSessionId = connectionRef.current.sessionId; + const ownAllocationSucceeded = + sourceSessionId === undefined && + allocatedSessionId !== undefined && + (currentSessionId === undefined || + currentSessionId === allocatedSessionId); + if ( + (!sourceOwner.isCurrent() && !ownAllocationSucceeded) || + (sourceSessionId !== undefined + ? currentSessionId !== sourceSessionId + : currentSessionId !== undefined && + currentSessionId !== allocatedSessionId) + ) { + return; } - return sendGoalPrompt(); - } - - // Bare `/goal` opens the Goals page instead of asking the daemon to print - // its status as text — the same move `/schedule` makes. Nothing is sent, - // so the composer is cleared by returning true. - openGoals(); + if (!connectionRef.current.sessionId && !allocatedSessionId) { + throw new Error(t('localCommand.noSession')); + } + store.appendLocalUserMessage(text); + const action = operation.kind === 'set' ? 'replace' : operation.kind; + const objective = + operation.kind === 'set' || operation.kind === 'edit' + ? operation.objective + : undefined; + if (allocatedSessionId && operation.kind === 'set') { + await createGoalForAllocatedSession( + allocatedSessionId, + operation.objective, + ); + } else { + await controlCurrentGoal(action, objective); + } + })().catch((error: unknown) => { + reportError(error, `Failed to ${operation.kind} /goal`); + }); return true; }, [ - dispatchGoalCleared, - dispatchGoalSet, - handleBusyGoalClear, + controlCurrentGoal, + createGoalForAllocatedSession, + ensureSessionForPrompt, openGoals, + pushToast, reportError, - sendPrompt, sessionOwnerGuard, store, - connectionRef, + t, ], ); + const runGoalControl = useCallback( + (action: 'pause' | 'resume' | 'clear') => { + void controlCurrentGoal(action).catch((error: unknown) => { + reportError(error, t(`goals.error.${action}Failed`)); + }); + }, + [controlCurrentGoal, reportError, t], + ); + + const handleGoalEditSave = useCallback( + (objective: string) => { + const owner = sessionOwnerGuard.capture(); + setGoalEditError(null); + void controlCurrentGoal('edit', objective) + .then(() => { + if (owner.isCurrent()) setGoalEditOpen(false); + }) + .catch((error: unknown) => { + if (!owner.isCurrent()) return; + setGoalEditError( + error instanceof Error ? error.message : String(error), + ); + reportError(error, t('goals.error.editFailed')); + }); + }, + [controlCurrentGoal, reportError, sessionOwnerGuard, t], + ); + const hiddenCommands = useMemo( () => new Set( @@ -8815,7 +8900,8 @@ export function App({ pushToast('warning', t('editor.connectionDisconnected')); return false; } - const promptBlocked = streamingStateRef.current !== 'idle'; + const promptBlocked = + streamingStateRef.current !== 'idle' || isGoalGateBlocked(); const submitPromptFromEditor = ( promptText: string, promptImages: PromptImage[] | undefined, @@ -8998,21 +9084,12 @@ export function App({ return true; } if (cmd === 'goal') { - // A bare `/goal` just opens the Goals page; it neither sends a - // prompt nor touches the session, so it works mid-turn too. - if (!goalArgOf(text)) { - openGoals(); - return true; - } - if (promptBlocked) { - if (isGoalClearCommand(text)) { - return handleBusyGoalClear(text); - } - return blockLocalCommandDuringTurn(); - } - return handleGoalSlashCommand(text, images, files, { - commitComposerAccepted, - }); + return handleGoalSlashCommand( + text, + (images?.length ?? 0) > 0 || + (files?.length ?? 0) > 0 || + (metadata?.inputAnnotations?.length ?? 0) > 0, + ); } if (cmd === 'theme') { const themeArg = text.slice(match[0].length).trim().toLowerCase(); @@ -9073,8 +9150,14 @@ export function App({ } const nextLanguage = normalizeLanguage(languageArg); const owner = { current: sessionOwnerGuard.capture() }; + // The daemon sync is what keeps the agent answering in the + // language the chrome just switched to, so when it cannot run + // (turn in flight, or a Goal owning the session) refuse the + // command instead of switching the UI alone — the language + // picker treats the identical condition the same way. + if (promptBlocked) return blockLocalCommandDuringTurn(); handleLanguageChange(nextLanguage); - if (!promptBlocked) { + { const deferComposerCommit = Boolean(onSubmitBeforeRef.current) || createSessionPromiseRef.current !== null; @@ -9824,7 +9907,7 @@ export function App({ } else if (text.startsWith('!')) { const cmd = text.slice(1).trim(); if (!cmd) return false; - if (promptBlocked) { + if (streamingStateRef.current !== 'idle') { queuedShellCommandsRef.current.push(cmd); pushToast('info', t('queue.shellQueued')); return true; @@ -9919,7 +10002,6 @@ export function App({ closeMobileDrawer, openPanel, openScheduledTasks, - openGoals, createNewSession, ensureSessionForPrompt, finishPromptPreparation, @@ -9928,7 +10010,6 @@ export function App({ gitDiffWorkspaceCwd, sessionWorktree, gitHubPrsSupported, - handleBusyGoalClear, handleGoalSlashCommand, handleThemeChange, handleSetMode, @@ -9957,6 +10038,7 @@ export function App({ workspaceActions, updateFailedPrompt, updateUnknownPromptAdmission, + isGoalGateBlocked, ], ); @@ -10083,7 +10165,11 @@ export function App({ ); const handleRetry = useCallback(() => { - if (sessionWriteBlockedRef.current || promptPreparationOwnerRef.current) { + if ( + sessionWriteBlockedRef.current || + promptPreparationOwnerRef.current || + isGoalGateBlocked() + ) { return; } if ( @@ -10279,6 +10365,7 @@ export function App({ store, t, updateUnknownPromptAdmission, + isGoalGateBlocked, ]); useEffect(() => { @@ -10668,7 +10755,7 @@ export function App({ const handleFastModelSelect = useCallback( (modelId: string) => { - if (streamingState !== 'idle') { + if (streamingState !== 'idle' || isGoalGateBlocked()) { blockLocalCommandDuringTurn(); return; } @@ -10724,6 +10811,7 @@ export function App({ reloadWorkspaceSettings, modelSettingScope, sessionOwnerGuard, + isGoalGateBlocked, ], ); @@ -11329,6 +11417,19 @@ export function App({ /> )} + {goalEditOpen && goalSnapshot?.goal && ( + { + if (goalControlBusy) return; + setGoalEditOpen(false); + setGoalEditError(null); + }} + /> + )} {showAuthDialog && ( { - // Setting a goal registers the Stop hook AND kicks off - // the first turn, so it has to travel the prompt path. - // Start a FRESH session so the goal loop doesn't take - // over the conversation the user was already having. - // - // Unless a previous attempt in this same visit to the - // page already made one and then failed to send: that - // session never got its goal and is still current, so - // reuse it. Creating another would strand it, and a user - // retrying a few times would end up with a column of - // blank chats in the sidebar. - // - // Leaving the page forgets it (see the effect on - // `strandedGoalSessionRef`), so this can never reuse a - // session the user has since talked to. const stranded = strandedGoalSessionRef.current; const canReuseStranded = stranded !== undefined && connectionRef.current.sessionId === stranded; if (!canReuseStranded) { - // `keepView`: createNewSession switches to the chat by - // default, which would unmount this form before the - // prompt is even sent and leave a later rejection with - // nowhere to render — the exact failure the deferred - // switch below exists to prevent. + strandedGoalSessionRef.current = undefined; const created = await createNewSession(undefined, { keepView: true, }); - // createNewSession already surfaced the failure; don't - // drop the goal into the wrong (still-current) session. - // `false` keeps the form open with the typed condition - // still in it — returning normally would read as - // "created" and reset it. if (!created) return false; - onSessionIdChange?.(undefined); } - // Switch to the chat only once the prompt is admitted. - // Switching first unmounts the Goals page, and a later - // rejection would then have nowhere to render: the user - // would land in an empty session with no explanation. - // Letting this reject keeps the error in the form the - // user is looking at. - const owner = { - current: sessionOwnerGuard.capture(), - }; + const allocationOwner = sessionOwnerGuard.capture(); + const sourceSessionId = + connectionRef.current.sessionId; + const allocatedSessionId = + await ensureSessionForPrompt(); + const currentSessionId = + connectionRef.current.sessionId; + const ownAllocationSucceeded = + sourceSessionId === undefined && + allocatedSessionId !== undefined && + (currentSessionId === undefined || + currentSessionId === allocatedSessionId); + if ( + (!allocationOwner.isCurrent() && + !ownAllocationSucceeded) || + (sourceSessionId !== undefined + ? currentSessionId !== sourceSessionId + : currentSessionId !== undefined && + currentSessionId !== allocatedSessionId) + ) { + return false; + } + if ( + !connectionRef.current.sessionId && + !allocatedSessionId + ) { + return false; + } + const owner = sessionOwnerGuard.capture(); try { - await sendPrompt( - `/goal ${condition}`, - undefined, - undefined, - { - clearComposerOnPromptStart: true, - ownerRef: owner, - }, - ); - if (!owner.current.isCurrent()) return false; + if (allocatedSessionId) { + await createGoalForAllocatedSession( + allocatedSessionId, + condition, + ); + } else { + await controlCurrentGoal('create', condition); + } } catch (error) { - // `sendPrompt` creates the session lazily, so by now - // one may exist even though the prompt never landed. - // Remember it so the retry reuses it rather than - // stranding it. - if (owner.current.isCurrent()) { + if ( + owner.isCurrent() && + mainViewRef.current === 'goals' + ) { strandedGoalSessionRef.current = + allocatedSessionId ?? connectionRef.current.sessionId; } throw error; } + if (!owner.isCurrent()) return false; strandedGoalSessionRef.current = undefined; setMainView('chat'); }} @@ -12179,6 +12274,7 @@ export function App({ onError={reportError} onImageIngestionNotice={pushToast} onSlashCommand={onSlashCommand} + onOpenGoals={openGoals} onRightPanelOpen={handleTurnOutputOpen} onOpenMonitor={openMonitorPanel} onPaneArtifactsChange={handlePaneArtifactsChange} @@ -12626,15 +12722,38 @@ export function App({ )} )} - + {(queuedPrompts.length > 0 || + liveGoalSnapshot?.goal) && ( +
+ + {liveGoalSnapshot?.goal && ( + { + setGoalEditError(null); + setGoalEditOpen(true); + }} + onPause={() => runGoalControl('pause')} + onResume={() => runGoalControl('resume')} + onClear={() => runGoalControl('clear')} + /> + )} +
+ )} {CustomComposerHeader && (
diff --git a/packages/web-shell/client/components/ChatPane.test.tsx b/packages/web-shell/client/components/ChatPane.test.tsx index 72612bf94cc..35835bfb1d4 100644 --- a/packages/web-shell/client/components/ChatPane.test.tsx +++ b/packages/web-shell/client/components/ChatPane.test.tsx @@ -35,7 +35,8 @@ let latestOnSubmit: | (( text: string, images?: unknown, - commit?: () => void, + files?: unknown, + commitAccepted?: () => void, metadata?: unknown, ) => boolean) | undefined; @@ -53,6 +54,7 @@ let sendPromptAdmit: (() => void) | undefined; const clearFollowup = vi.fn(); const insertText = vi.fn(); const transcriptDispatch = vi.fn(); +const appendLocalUserMessage = vi.fn(); const sendPrompt = vi.fn(async () => ({}) as any); const submitPermission = vi.fn(async () => true); const cancel = vi.fn(async () => {}); @@ -60,6 +62,8 @@ const setApprovalMode = vi.fn(async (mode: string) => ({ mode })); const setModel = vi.fn(async () => ({}) as any); const loadArtifacts = vi.fn(async () => ({ artifacts: [] })); const getTasks = vi.fn(); +const getGoal = vi.fn(); +const controlGoal = vi.fn(); const readAttachment = vi.fn(); const daemonActions = { sendPrompt, @@ -69,6 +73,8 @@ const daemonActions = { setModel, loadArtifacts, getTasks, + getGoal, + controlGoal, readAttachment, }; const enqueuePrompt = vi.fn(() => true); @@ -78,6 +84,7 @@ const editLastQueuedPrompt = vi.fn(() => false); const clearQueuedPrompts = vi.fn(() => false); let queuedPromptsMock: any[] = []; let queuedTextsMock: string[] = []; +let ownerVersion = 0; const latestComposerCoreOptions = vi.hoisted(() => ({ current: null as Record | null, @@ -108,6 +115,7 @@ vi.mock('@qwen-code/webui/daemon-react-sdk', () => ({ }), useTranscriptStore: () => ({ dispatch: transcriptDispatch, + appendLocalUserMessage, }), usePromptStatus: () => 'idle', useOptionalWorkspace: () => undefined, @@ -119,7 +127,10 @@ vi.mock('@qwen-code/webui/daemon-react-sdk', () => ({ }), useWorkspaceEventSignals: () => ({ artifactsVersion: 0 }), useDaemonSessionOwnerGuard: () => ({ - capture: () => ({ isCurrent: () => true }), + capture: () => { + const captured = ownerVersion; + return { isCurrent: () => ownerVersion === captured }; + }, }), })); @@ -330,6 +341,7 @@ vi.mock('./QueuedPromptDisplay', () => ({
{String(props.prompts.length)}
@@ -378,6 +390,11 @@ beforeEach(() => { workspaceCwd: '/w', loadingTranscript: false, catchingUp: false, + // A loaded session always carries a Goal snapshot (the load falls back to + // an idle one when the fetch fails), and the Goal gates fail CLOSED on an + // absent one — leaving it out here would model a session that is still + // hydrating, not a Goal-less one. + goalState: { v: 2, activity: 'idle', goal: null }, }; streamingStateValue = 'idle'; pendingPermission = null; @@ -391,10 +408,13 @@ beforeEach(() => { sendPromptAdmit = undefined; queuedPromptsMock = []; queuedTextsMock = []; + ownerVersion = 0; sendPrompt.mockReset(); loadArtifacts.mockReset(); loadArtifacts.mockResolvedValue({ artifacts: [] }); getTasks.mockReset(); + getGoal.mockReset(); + controlGoal.mockReset(); readAttachment.mockReset(); readAttachment.mockResolvedValue({ data: 'eyJoaSI6IuS9oOWlvSJ9', @@ -417,6 +437,7 @@ beforeEach(() => { editLastQueuedPrompt.mockClear(); clearQueuedPrompts.mockClear(); transcriptDispatch.mockClear(); + appendLocalUserMessage.mockClear(); catalogController.invalidateWorkspace.mockClear(); catalogController.promptAdmitted.mockClear(); catalogController.promptAdmissionUncertain.mockClear(); @@ -467,7 +488,651 @@ function testid(id: string): HTMLElement | null { return container!.querySelector(`[data-testid="${id}"]`); } +function deferred() { + let resolve!: (value: T) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((done, fail) => { + resolve = done; + reject = fail; + }); + return { promise, reject, resolve }; +} + describe('ChatPane', () => { + it.each([ + [ + 'images', + [{ data: 'image-data', media_type: 'image/png' }], + undefined, + undefined, + ], + ['files', undefined, [{ name: 'notes.txt' }], undefined], + [ + 'input annotations', + undefined, + undefined, + { + inputAnnotations: [ + { + start: 15, + end: 22, + text: '@notes', + type: 'file', + data: { path: 'notes.txt' }, + }, + ], + }, + ], + ])( + 'rejects /goal with %s and preserves the draft', + (_kind, images, files, metadata) => { + const onError = vi.fn(); + render({ onError }); + let returned: boolean | undefined; + + act(() => { + returned = latestOnSubmit!( + '/goal set inspect the attachment', + images, + files, + undefined, + metadata, + ); + }); + + expect(returned).toBe(false); + expect(onError).toHaveBeenCalledWith( + expect.any(Error), + 'Remove attachments before using /goal.', + ); + expect(controlGoal).not.toHaveBeenCalled(); + expect(transcriptDispatch).not.toHaveBeenCalled(); + }, + ); + + it('lets the host slash handler intercept /goal before the control plane', () => { + // The prop contract says the host handler runs before Web Shell handles a + // slash command; the main composer honours that for /goal, so the pane has + // to as well or an override silently applies on one surface only. + const onSlashCommand = vi.fn(() => true); + render({ onSlashCommand, onOpenGoals: vi.fn() }); + let returned: boolean | undefined; + + act(() => { + returned = latestOnSubmit!('/goal pause'); + }); + + expect(returned).toBe(true); + expect(onSlashCommand).toHaveBeenCalled(); + expect(getGoal).not.toHaveBeenCalled(); + expect(controlGoal).not.toHaveBeenCalled(); + }); + + it('does not swallow a bare /goal when the pane has no goals view', () => { + // The side-task pane passes no `onOpenGoals`; consuming the text there + // opens nothing and shows nothing. + const onError = vi.fn(); + render({ onError }); + let returned: boolean | undefined; + + act(() => { + returned = latestOnSubmit!('/goal'); + }); + + expect(returned).toBe(false); + expect(onError).toHaveBeenCalledWith( + expect.any(Error), + 'The goals view is not available on this surface.', + ); + }); + + it('reports an objective-less /goal set without consuming it', () => { + const onError = vi.fn(); + render({ onError, onOpenGoals: vi.fn() }); + let returned: boolean | undefined; + + act(() => { + returned = latestOnSubmit!('/goal set'); + }); + + expect(returned).toBe(false); + expect(onError).toHaveBeenCalledWith( + expect.any(Error), + '/goal set requires an objective.', + ); + expect(controlGoal).not.toHaveBeenCalled(); + }); + + it('offers Insert only while a turn is running', () => { + // Between two Goal turns streaming is idle while the hold keeps queued + // prompts visible. `insertQueuedPrompt` no-ops at idle, so the affordance + // has to disappear with it rather than render a button that does nothing. + queuedPromptsMock = [{ id: 1, text: 'held while the Goal runs' } as never]; + connectionState.goalState = { + v: 2, + activity: 'idle', + goal: { + goalId: 'goal-1', + revision: 1, + objective: 'ship it', + status: 'active', + evidenceCursor: { recordId: 'record-1' }, + turnCount: 1, + activeTimeMs: 10, + createdAt: 1, + updatedAt: 1, + }, + }; + streamingStateValue = 'idle'; + render(); + + expect(testid('pane-queue')?.dataset['canInsertMidTurn']).toBe('false'); + + act(() => { + streamingStateValue = 'responding'; + rerender(); + }); + + expect(testid('pane-queue')?.dataset['canInsertMidTurn']).toBe('true'); + }); + + it('preserves a /goal command the pane connection cannot deliver', () => { + // App.tsx applies the broken-connection guard before any slash handling and + // keeps the text in the composer. Without the same ordering here the branch + // consumes the text, writes a transcript entry, and only then fails inside + // `requireSessionForAction` — the typed control is gone. + const onError = vi.fn(); + connectionState = { ...connectionState, status: 'error' }; + render({ onError }); + let returned: boolean | undefined; + + act(() => { + returned = latestOnSubmit!('/goal pause'); + }); + + expect(returned).toBe(false); + expect(controlGoal).not.toHaveBeenCalled(); + expect(getGoal).not.toHaveBeenCalled(); + expect(appendLocalUserMessage).not.toHaveBeenCalled(); + }); + + it('keeps goal controls locked when the goal is replaced mid-control', async () => { + const goalA = { + v: 2 as const, + activity: 'running' as const, + goal: { + goalId: 'goal-a', + revision: 5, + objective: 'ship it', + status: 'active' as const, + evidenceCursor: { recordId: 'record-1' }, + turnCount: 1, + activeTimeMs: 10, + createdAt: 1, + updatedAt: 1, + }, + }; + const goalB = { + ...goalA, + goal: { + ...goalA.goal, + goalId: 'goal-b', + revision: 1, + objective: 'replaced by another client', + updatedAt: 2, + }, + }; + const pendingControl = deferred<{ snapshot: typeof goalA }>(); + connectionState.goalState = goalA; + getGoal.mockResolvedValue({ snapshot: goalA }); + controlGoal.mockReturnValueOnce(pendingControl.promise); + render(); + + const pause = container!.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + ); + if (!pause) throw new Error('pause control was not rendered'); + act(() => pause.click()); + await vi.waitFor(() => expect(controlGoal).toHaveBeenCalledOnce()); + + // Another client replaces the goal while the pause is still in flight. + act(() => { + connectionState = { ...connectionState, goalState: goalB }; + rerender(); + }); + const pauseAfterReplace = container!.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + ); + expect(pauseAfterReplace?.disabled).toBe(true); + act(() => pauseAfterReplace?.click()); + expect(controlGoal).toHaveBeenCalledOnce(); + + await act(async () => pendingControl.resolve({ snapshot: goalB })); + expect( + container!.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + )?.disabled, + ).toBe(false); + }); + + it('locks goal controls while the current snapshot refresh is in flight', async () => { + const current = { + v: 2 as const, + activity: 'running' as const, + goal: { + goalId: 'goal-1', + revision: 5, + objective: 'ship it', + status: 'active' as const, + evidenceCursor: { recordId: 'record-1' }, + turnCount: 1, + activeTimeMs: 10, + createdAt: 1, + updatedAt: 1, + }, + }; + connectionState.goalState = current; + let resolveGoal: + | ((value: { snapshot: typeof current }) => void) + | undefined; + getGoal.mockReturnValue( + new Promise((resolve) => { + resolveGoal = resolve; + }), + ); + controlGoal.mockResolvedValue({ snapshot: current }); + render(); + + const pause = container!.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + ); + if (!pause) throw new Error('pause control was not rendered'); + act(() => pause.click()); + + expect(pause.disabled).toBe(true); + act(() => pause.click()); + expect(getGoal).toHaveBeenCalledTimes(1); + + await act(async () => { + resolveGoal?.({ snapshot: current }); + }); + expect(controlGoal).toHaveBeenCalledTimes(1); + }); + + it('builds the control request from the freshly fetched Goal', async () => { + // `expectedGoalId`/`expectedRevision` must come from the getGoal round trip, + // not from the possibly-stale snapshot in connection state, or every + // control races the daemon's CAS. + const stale = { + v: 2 as const, + activity: 'running' as const, + goal: { + goalId: 'goal-1', + revision: 5, + objective: 'ship it', + status: 'active' as const, + evidenceCursor: { recordId: 'record-1' }, + turnCount: 1, + activeTimeMs: 10, + createdAt: 1, + updatedAt: 1, + }, + }; + const fresh = { + ...stale, + goal: { ...stale.goal, revision: 9 }, + }; + connectionState.goalState = stale; + getGoal.mockResolvedValue({ snapshot: fresh }); + controlGoal.mockResolvedValue({ snapshot: fresh }); + render({ onOpenGoals: vi.fn() }); + + act(() => { + container! + .querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + )! + .click(); + }); + await vi.waitFor(() => expect(controlGoal).toHaveBeenCalledTimes(1)); + + expect(controlGoal).toHaveBeenCalledWith({ + action: 'pause', + expectedGoalId: 'goal-1', + expectedRevision: 9, + }); + + // `/goal set` maps to a versioned replace against the same fresh snapshot. + act(() => { + latestOnSubmit!('/goal set ship the other thing'); + }); + await vi.waitFor(() => expect(controlGoal).toHaveBeenCalledTimes(2)); + expect(controlGoal).toHaveBeenLastCalledWith({ + action: 'replace', + objective: 'ship the other thing', + expectedGoalId: 'goal-1', + expectedRevision: 9, + }); + expect(appendLocalUserMessage).toHaveBeenCalledWith( + '/goal set ship the other thing', + ); + }); + + it('closes the pane Goal edit dialog when its session changes', async () => { + // Left open, the dialog re-syncs its textarea from the new session's + // objective and the user edits that Goal believing it is the old one. + const goalA = { + v: 2 as const, + activity: 'running' as const, + goal: { + goalId: 'goal-a', + revision: 5, + objective: 'session A objective', + status: 'active' as const, + evidenceCursor: { recordId: 'record-1' }, + turnCount: 1, + activeTimeMs: 10, + createdAt: 1, + updatedAt: 1, + }, + }; + connectionState.goalState = goalA; + getGoal.mockResolvedValue({ snapshot: goalA }); + render(); + + act(() => { + container! + .querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Edit goal"]', + )! + .click(); + }); + expect(document.querySelector('textarea')).not.toBeNull(); + + act(() => { + connectionState = { + ...connectionState, + goalState: { + ...goalA, + goal: { ...goalA.goal, goalId: 'goal-b', objective: 'goal B' }, + }, + }; + rerender(); + }); + + expect(document.querySelector('textarea')).toBeNull(); + }); + + it('does not dispatch a Goal control after the pane session changes during refresh', async () => { + const current = { + v: 2 as const, + activity: 'running' as const, + goal: { + goalId: 'goal-1', + revision: 5, + objective: 'ship it', + status: 'active' as const, + evidenceCursor: { recordId: 'record-1' }, + turnCount: 1, + activeTimeMs: 10, + createdAt: 1, + updatedAt: 1, + }, + }; + const pendingGoal = deferred<{ snapshot: typeof current }>(); + const onError = vi.fn(); + connectionState.goalState = current; + getGoal.mockReturnValueOnce(pendingGoal.promise); + render({ onError }); + + const pause = container!.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + ); + if (!pause) throw new Error('pause control was not rendered'); + act(() => pause.click()); + act(() => { + ownerVersion += 1; + connectionState = { ...connectionState, sessionId: 'sess-2' }; + rerender({ onError }); + }); + await act(async () => pendingGoal.resolve({ snapshot: current })); + + expect(controlGoal).not.toHaveBeenCalled(); + // The operation was dropped on purpose; reporting it would show a failure + // toast for a control the user's own session switch cancelled. + expect(onError).not.toHaveBeenCalled(); + }); + + it('releases Goal control busy state after a same-session reattach', async () => { + const current = { + v: 2 as const, + activity: 'running' as const, + goal: { + goalId: 'goal-1', + revision: 5, + objective: 'ship it', + status: 'active' as const, + evidenceCursor: { recordId: 'record-1' }, + turnCount: 1, + activeTimeMs: 10, + createdAt: 1, + updatedAt: 1, + }, + }; + const pendingControl = deferred<{ snapshot: typeof current }>(); + connectionState.goalState = current; + getGoal.mockResolvedValue({ snapshot: current }); + controlGoal.mockReturnValueOnce(pendingControl.promise); + render(); + + const pause = container!.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + ); + if (!pause) throw new Error('pause control was not rendered'); + act(() => pause.click()); + await vi.waitFor(() => expect(controlGoal).toHaveBeenCalledOnce()); + act(() => { + ownerVersion += 1; + rerender(); + }); + await act(async () => pendingControl.resolve({ snapshot: current })); + + expect( + container!.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Pause goal"]', + )?.disabled, + ).toBe(false); + }); + + it('reports an edit failure after the edited Goal disappears', async () => { + const current = { + v: 2 as const, + activity: 'running' as const, + goal: { + goalId: 'goal-1', + revision: 5, + objective: 'ship it', + status: 'active' as const, + evidenceCursor: { recordId: 'record-1' }, + turnCount: 1, + activeTimeMs: 10, + createdAt: 1, + updatedAt: 1, + }, + }; + const pendingGoal = deferred<{ + snapshot: { v: 2; activity: 'idle'; goal: null }; + }>(); + const onError = vi.fn(); + connectionState.goalState = current; + getGoal.mockReturnValueOnce(pendingGoal.promise); + render({ onError }); + + act(() => { + container! + .querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Edit goal"]', + ) + ?.click(); + }); + const save = [ + ...document.querySelectorAll('button'), + ].find((button) => button.textContent === 'Save'); + if (!save) throw new Error('save control was not rendered'); + act(() => save.click()); + act(() => { + connectionState = { + ...connectionState, + goalState: { v: 2, activity: 'idle', goal: null }, + }; + rerender({ onError }); + }); + await act(async () => + pendingGoal.resolve({ + snapshot: { v: 2, activity: 'idle', goal: null }, + }), + ); + + expect(onError).toHaveBeenCalledWith( + // The guard that produces this message is the only protection the + // pause/resume/clear flows have against dereferencing a null goal, so + // pin the message rather than "some Error". + expect.objectContaining({ message: 'The goal is no longer available.' }), + 'Failed to edit the goal', + ); + }); + + it.each(['resolve', 'reject'] as const)( + 'ignores a stale Goal edit %s after the pane session changes', + async (outcome) => { + const goalA = { + v: 2 as const, + activity: 'running' as const, + goal: { + goalId: 'goal-a', + revision: 5, + objective: 'session A objective', + status: 'active' as const, + evidenceCursor: { recordId: 'record-a' }, + turnCount: 1, + activeTimeMs: 10, + createdAt: 1, + updatedAt: 1, + }, + }; + const goalB = { + ...goalA, + goal: { + ...goalA.goal, + goalId: 'goal-b', + revision: 1, + objective: 'session B objective', + }, + }; + let resolveEdit!: (value: { snapshot: typeof goalA }) => void; + let rejectEdit!: (error: Error) => void; + const edit = new Promise<{ snapshot: typeof goalA }>( + (resolve, reject) => { + resolveEdit = resolve; + rejectEdit = reject; + }, + ); + connectionState.goalState = goalA; + getGoal.mockResolvedValue({ snapshot: goalA }); + controlGoal.mockReturnValueOnce(edit); + render(); + + const editA = container!.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Edit goal"]', + ); + if (!editA) throw new Error('session A edit control was not rendered'); + act(() => editA.click()); + const saveA = [ + ...document.querySelectorAll('button'), + ].find((button) => button.textContent === 'Save'); + if (!saveA) throw new Error('session A save control was not rendered'); + act(() => saveA.click()); + await vi.waitFor(() => expect(controlGoal).toHaveBeenCalledTimes(1)); + + act(() => { + ownerVersion += 1; + connectionState = { + ...connectionState, + sessionId: 'sess-2', + goalState: goalB, + }; + rerender(); + }); + const editB = container!.querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Edit goal"]', + ); + if (!editB) throw new Error('session B edit control was not rendered'); + expect(editB.disabled).toBe(false); + act(() => editB.click()); + expect(document.querySelector('textarea')).not.toBeNull(); + + await act(async () => { + if (outcome === 'resolve') resolveEdit({ snapshot: goalA }); + else rejectEdit(new Error('session A edit failed')); + await Promise.resolve(); + }); + + expect(document.querySelector('textarea')).not.toBeNull(); + expect(document.querySelector('[role="alert"]')).toBeNull(); + }, + ); + + it('rejects a Goal edit when the same session replaces the goal', async () => { + const goalA = { + v: 2 as const, + activity: 'running' as const, + goal: { + goalId: 'goal-a', + revision: 5, + objective: 'goal A', + status: 'active' as const, + evidenceCursor: { recordId: 'record-a' }, + turnCount: 1, + activeTimeMs: 10, + createdAt: 1, + updatedAt: 1, + }, + }; + const goalB = { + ...goalA, + goal: { ...goalA.goal, goalId: 'goal-b', objective: 'goal B' }, + }; + const pendingGoal = deferred<{ snapshot: typeof goalB }>(); + const onError = vi.fn(); + connectionState.goalState = goalA; + getGoal.mockReturnValueOnce(pendingGoal.promise); + render({ onError }); + + act(() => { + container! + .querySelector( + '[data-testid="goal-status-strip"] button[aria-label="Edit goal"]', + ) + ?.click(); + }); + const save = [ + ...document.querySelectorAll('button'), + ].find((button) => button.textContent === 'Save'); + if (!save) throw new Error('save control was not rendered'); + act(() => save.click()); + act(() => { + connectionState = { ...connectionState, goalState: goalB }; + rerender({ onError }); + }); + await act(async () => pendingGoal.resolve({ snapshot: goalB })); + + expect(controlGoal).not.toHaveBeenCalled(); + expect(onError).toHaveBeenCalledWith( + expect.any(Error), + 'Failed to edit the goal', + ); + }); + it('opens a pane monitor in the shared right panel', async () => { connectionState.capabilities = { features: ['session_monitor_tool_correlation'], @@ -950,6 +1615,42 @@ describe('ChatPane', () => { expect(enqueuePrompt).not.toHaveBeenCalled(); }); + it('holds an idle prompt while the Goal state is still hydrating', () => { + // The session load clears `loadingTranscript` before its `goal()` fetch + // resolves, so the composer is writable with no snapshot yet. The daemon + // has no server-side prompt gate for an active Goal, so a direct send in + // that window bypasses the Goal queue outright — fail closed, exactly as + // the local hold does. + connectionState = { ...connectionState, goalState: undefined }; + render(); + + act(() => + testid('pane-submit')!.dispatchEvent( + new MouseEvent('click', { bubbles: true }), + ), + ); + + expect(sendPrompt).not.toHaveBeenCalled(); + expect(enqueuePrompt).toHaveBeenCalled(); + + // ...and the gate reopens once the snapshot lands Goal-less — the window + // is a hold, not a lock. + act(() => { + connectionState = { + ...connectionState, + goalState: { v: 2, activity: 'idle', goal: null }, + }; + rerender(); + }); + act(() => + testid('pane-submit')!.dispatchEvent( + new MouseEvent('click', { bubbles: true }), + ), + ); + + expect(sendPrompt).toHaveBeenCalledTimes(1); + }); + it('lets the host handle a slash command', () => { const onSlashCommand = vi.fn(() => true); render({ onSlashCommand }); @@ -1738,6 +2439,7 @@ describe('ChatPane', () => { }); it('enables mid-turn queue mutations only when advertised', () => { + queuedPromptsMock = [{ id: 1, text: 'queued next' }]; connectionState.capabilities = { features: ['session_mid_turn_message_mutation'], }; @@ -1747,6 +2449,7 @@ describe('ChatPane', () => { }); it('disables mid-turn queue mutations when not advertised', () => { + queuedPromptsMock = [{ id: 1, text: 'queued next' }]; render(); expect(testid('pane-queue')?.dataset.canMutateMidTurn).toBe('false'); }); diff --git a/packages/web-shell/client/components/ChatPane.tsx b/packages/web-shell/client/components/ChatPane.tsx index cb0be82f82f..6990fc29599 100644 --- a/packages/web-shell/client/components/ChatPane.tsx +++ b/packages/web-shell/client/components/ChatPane.tsx @@ -17,6 +17,7 @@ import { useActions, useConnection, useDaemonFollowupSuggestion, + useDaemonSessionOwnerGuard, useStreamingState, useTranscriptHistory, useTranscriptStore, @@ -61,6 +62,9 @@ import { } from '../utils/todos'; import { findMonitorTaskForTool } from '../utils/monitorTasks'; import { invokeSlashCommandHandler } from '../utils/slash-command-action'; +import { parseWebShellGoalCommand } from '../utils/goalCondition'; +import { buildGoalControlRequest } from '../utils/goalControlRequest'; +import { isGoalGateBlocked } from '../utils/goalGate'; import type { WebShellSlashCommandHandler } from '../App'; import { getModelDisplayName } from '../utils/modelDisplay'; import { @@ -83,6 +87,9 @@ import { MessageList } from './MessageList'; import { StreamingStatus } from './StreamingStatus'; import { ChatEditor, type ComposerToolbarAction } from './ChatEditor'; import { QueuedPromptDisplay } from './QueuedPromptDisplay'; +import { GoalStatusStrip } from './GoalStatusStrip'; +import composerStatusStyles from './ComposerStatusStack.module.css'; +import { GoalEditDialog } from './dialogs/GoalEditDialog'; import { ToolApproval } from './messages/ToolApproval'; import { AskUserQuestion } from './messages/AskUserQuestion'; import type { @@ -182,6 +189,7 @@ export interface ChatPaneProps { onImageIngestionNotice?: (tone: 'warning' | 'error', message: string) => void; /** Host slash-command callback shared with the main chat composer. */ onSlashCommand?: WebShellSlashCommandHandler; + onOpenGoals?: () => void; onRightPanelOpen?: (request: TurnOutputOpenRequest) => void; onOpenMonitor?: ( task: DaemonSessionMonitorTaskStatus, @@ -223,6 +231,7 @@ export function ChatPane({ onError, onImageIngestionNotice, onSlashCommand, + onOpenGoals, onRightPanelOpen, onOpenMonitor, onPaneArtifactsChange, @@ -241,6 +250,7 @@ export function ChatPane({ useWebShellCustomization(); const connection = useConnection(); const actions = useActions(); + const sessionOwnerGuard = useDaemonSessionOwnerGuard(); const workspace = useWorkspace(); const attachmentWorkspaceTarget = useArtifactWorkspaceTarget( connection.workspaceCwd, @@ -253,6 +263,39 @@ export function ChatPane({ const transcriptHistory = useTranscriptHistory(); const store = useTranscriptStore(); const streamingState = useStreamingState(); + const [goalControlBusy, setGoalControlBusy] = useState(false); + const goalControlOpSeqRef = useRef(0); + const goalControlOwnerRef = useRef< + { opId: number; sessionId: string | undefined } | undefined + >(undefined); + const [goalEditOpen, setGoalEditOpen] = useState(false); + const [goalEditError, setGoalEditError] = useState(null); + const connectionRef = useRef(connection); + connectionRef.current = connection; + const connectionGoalComplete = + connection.goalState?.goal?.status === 'complete'; + const liveGoalSnapshot = connectionGoalComplete + ? undefined + : connection.goalState; + useEffect(() => { + const owner = goalControlOwnerRef.current; + // Release the busy latch only when no control operation owns it, or when + // its owner belongs to a session we have left (that operation's `finally` + // can no longer release it here). Releasing it while an operation is still + // in flight — which a server-side goal replacement would otherwise do — + // re-enables the strip and lets a second control dispatch against the same + // expected revision, so one of the two loses with a 409. + if (!owner || owner.sessionId !== connection.sessionId) { + goalControlOwnerRef.current = undefined; + setGoalControlBusy(false); + } + setGoalEditOpen(false); + setGoalEditError(null); + }, [ + connection.goalState?.goal?.goalId, + connection.sessionId, + connectionGoalComplete, + ]); const { artifacts } = useSessionArtifacts(); const openSubagentDetails = useCallback( (tool: ACPToolCall) => { @@ -538,6 +581,7 @@ export function ChatPane({ queuedTexts, enqueuePrompt, removeQueuedPrompt, + insertQueuedPrompt, editQueuedPrompt, editLastQueuedPrompt, clearQueuedPrompts, @@ -551,6 +595,7 @@ export function ChatPane({ canInjectMidTurnMedia, workspaceFileActions: attachmentWorkspaceTarget?.actions, streamingState, + holdQueuedPromptsLocally: isGoalGateBlocked(connection), sessionActions: actions, store, editorRef, @@ -570,6 +615,86 @@ export function ChatPane({ return undefined; }, [messages, isResponding]); + const controlGoal = useCallback( + async ( + action: 'replace' | 'edit' | 'pause' | 'resume' | 'clear', + objective?: string, + ) => { + const busyOwner = sessionOwnerGuard.capture(); + const busySessionId = connectionRef.current.sessionId; + const expectedGoalId = connectionRef.current.goalState?.goal?.goalId; + const opId = ++goalControlOpSeqRef.current; + goalControlOwnerRef.current = { opId, sessionId: busySessionId }; + setGoalControlBusy(true); + try { + const snapshot = (await actions.getGoal()).snapshot; + const goal = snapshot.goal; + if ( + (action === 'replace' || action === 'edit') && + goal?.goalId !== expectedGoalId + ) { + throw new Error(t('goals.error.goalUnavailable')); + } + const request = buildGoalControlRequest(action, goal, objective, { + emptyObjective: t('goals.error.emptyCondition'), + goalUnavailable: t('goals.error.goalUnavailable'), + }); + if (!busyOwner.isCurrent()) { + throw new Error(t('goals.error.goalUnavailable')); + } + try { + return await actions.controlGoal(request); + } catch (error) { + await actions.getGoal().catch(() => undefined); + throw error; + } + } finally { + // A newer operation (or a session change) owns the latch now; leave it + // to whoever owns it rather than releasing it under them. + if (goalControlOwnerRef.current?.opId === opId) { + goalControlOwnerRef.current = undefined; + if (connectionRef.current.sessionId === busySessionId) { + setGoalControlBusy(false); + } + } + } + }, + [actions, sessionOwnerGuard, t], + ); + + const runGoalControl = useCallback( + (action: 'pause' | 'resume' | 'clear') => { + const owner = sessionOwnerGuard.capture(); + void controlGoal(action).catch((error: unknown) => { + // A control dropped because the pane moved to another session is not a + // failure the user needs to see — `handleGoalEditSave` and the main + // composer swallow the same race. + if (!owner.isCurrent()) return; + reportError(error, t(`goals.error.${action}Failed`)); + }); + }, + [controlGoal, reportError, sessionOwnerGuard, t], + ); + + const handleGoalEditSave = useCallback( + (objective: string) => { + const owner = sessionOwnerGuard.capture(); + setGoalEditError(null); + void controlGoal('edit', objective) + .then(() => { + if (owner.isCurrent()) setGoalEditOpen(false); + }) + .catch((error: unknown) => { + if (!owner.isCurrent()) return; + setGoalEditError( + error instanceof Error ? error.message : String(error), + ); + reportError(error, t('goals.error.editFailed')); + }); + }, + [controlGoal, reportError, sessionOwnerGuard, t], + ); + const handleSubmit = useCallback( ( text: string, @@ -582,12 +707,69 @@ export function ChatPane({ if (!trimmed && (images?.length ?? 0) === 0 && (files?.length ?? 0) === 0) return false; if (admissionPayloadLocked) return false; + // The host handler is documented as running before Web Shell handles a + // slash command, so it gets `/goal` first here exactly as it does in the + // main composer — otherwise an override works on one surface only. if ( trimmed && invokeSlashCommandHandler(text, onSlashCommandRef.current, reportError) ) { return true; } + if (/^\/goal(?:\s|$)/i.test(trimmed)) { + // The same guard App.tsx applies before any slash handling: a control + // that cannot reach the daemon must leave the text in the composer + // instead of consuming it, appending a transcript entry, and failing + // later at `requireSessionForAction` with only a toast. + if ( + shouldBlockComposerSubmit({ + connectionStatus: connection.status, + hasSession: Boolean(connection.sessionId), + }) + ) { + return false; + } + if ( + (images?.length ?? 0) > 0 || + (files?.length ?? 0) > 0 || + (metadata?.inputAnnotations?.length ?? 0) > 0 + ) { + const message = t('goals.error.attachmentsUnsupported'); + reportError(new Error(message), message); + return false; + } + const operation = parseWebShellGoalCommand(trimmed); + if (operation.kind === 'status') { + // A pane without a Goals surface (the side-task pane passes no + // handler) would otherwise consume the text and open nothing. + if (!onOpenGoals) { + reportError( + new Error(t('goals.error.goalsUnavailable')), + t('goals.error.goalsUnavailable'), + ); + return false; + } + onOpenGoals(); + return true; + } + if (operation.kind === 'error') { + const message = t('goals.error.requiresObjective', { + keyword: operation.keyword, + }); + reportError(new Error(message), message); + return false; + } + const action = operation.kind === 'set' ? 'replace' : operation.kind; + const objective = + operation.kind === 'set' || operation.kind === 'edit' + ? operation.objective + : undefined; + store.appendLocalUserMessage(text); + void controlGoal(action, objective).catch((error: unknown) => { + reportError(error, `Failed to ${operation.kind} /goal`); + }); + return true; + } if ( shouldBlockComposerSubmit({ connectionStatus: connection.status, @@ -607,7 +789,17 @@ export function ChatPane({ onFirstPromptAdmitted(trimmed); } }; - if (streamingStateRef.current === 'idle') { + // Fail CLOSED on a hydrating `goalState`, exactly as the local hold + // above does: the load makes the composer writable before `goal()` + // resolves, and the daemon has no server-side prompt gate for an active + // Goal, so a direct send in that window bypasses the Goal queue. + if ( + streamingStateRef.current === 'idle' && + !isGoalGateBlocked({ + sessionId: connection.sessionId, + goalState: connection.goalState, + }) + ) { const admissionOwner = admissionOwnerRef.current; let admissionStarted = false; let admitted = false; @@ -680,13 +872,20 @@ export function ChatPane({ admissionPayloadLocked, catalogOwnerCwd, clearFollowup, + // The whole snapshot, not just the status: the gate distinguishes an + // absent (hydrating) snapshot from a Goal-less one, and both read as an + // undefined status. + connection.goalState, connection.sessionId, connection.status, + controlGoal, enqueuePrompt, onFirstPromptAdmitted, onImageIngestionNotice, + onOpenGoals, reportError, sessionCatalogController, + store, t, ], ); @@ -934,6 +1133,19 @@ export function ChatPane({ data-testid="chat-pane" aria-label={headerLabel} > + {goalEditOpen && connection.goalState?.goal && ( + { + if (goalControlBusy) return; + setGoalEditOpen(false); + setGoalEditError(null); + }} + /> + )} {!embedded && (
- + {(queuedPrompts.length > 0 || liveGoalSnapshot?.goal) && ( +
+ + {liveGoalSnapshot?.goal && ( + { + setGoalEditError(null); + setGoalEditOpen(true); + }} + onPause={() => runGoalControl('pause')} + onResume={() => runGoalControl('resume')} + onClear={() => runGoalControl('clear')} + /> + )} +
+ )} {unknownPromptAdmission && (
:global([data-web-shell-queued-prompts]), +.root > [data-web-shell-goal-status] { + width: 100%; + margin: 0; + border: 0; + border-radius: 0; + background: transparent; +} + +.root + > :global([data-web-shell-queued-prompts]) + + [data-web-shell-goal-status] { + border-top: 1px solid color-mix(in srgb, var(--border) 74%, transparent); +} diff --git a/packages/web-shell/client/components/GoalStatusStrip.module.css b/packages/web-shell/client/components/GoalStatusStrip.module.css new file mode 100644 index 00000000000..a5923875381 --- /dev/null +++ b/packages/web-shell/client/components/GoalStatusStrip.module.css @@ -0,0 +1,114 @@ +.root { + container-type: inline-size; + display: flex; + align-items: center; + gap: 10px; + width: calc(100% - 32px); + min-width: 0; + min-height: 42px; + box-sizing: border-box; + margin: 0 auto -8px; + padding: 8px 12px; + border: 1px solid var(--border); + border-radius: 12px 12px 0 0; + background: var(--background); + color: var(--muted-foreground); +} + +.target { + flex: 0 0 auto; + color: color-mix(in srgb, var(--foreground) 62%, var(--muted-foreground)); +} + +.summary { + display: flex; + align-items: baseline; + gap: 6px; + min-width: 0; + flex: 1 1 auto; + font-size: 13px; + line-height: 1.25; +} + +.status { + flex: 0 0 auto; + color: var(--foreground); + font-weight: 600; +} + +.activity, +.objective { + color: var(--muted-foreground); +} + +.activity, +.separator, +.elapsed { + flex: 0 0 auto; +} + +.objective { + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + font-weight: 500; +} + +.separator, +.elapsed { + color: color-mix(in srgb, var(--muted-foreground) 78%, transparent); +} + +.actions { + display: inline-flex; + align-items: center; + gap: 2px; + flex: 0 0 auto; +} + +.action { + appearance: none; + display: inline-flex; + align-items: center; + justify-content: center; + width: 30px; + height: 28px; + padding: 0; + border: 0; + border-radius: 7px; + background: transparent; + color: var(--muted-foreground); + cursor: pointer; +} + +.action:hover:not(:disabled), +.action:focus-visible:not(:disabled) { + outline: none; + background: var(--muted); + color: var(--foreground); +} + +.action:focus-visible:not(:disabled) { + box-shadow: 0 0 0 2px color-mix(in srgb, var(--primary) 45%, transparent); +} + +.action:disabled { + opacity: 0.45; + cursor: default; +} + +/* + * `.root` establishes the container, so it can only style its DESCENDANTS from + * here — an element is never its own container query target. Compacting the + * strip's own gap/padding would need a separate outer element carrying + * `container-type`; until then keep this block to what actually resolves, + * rather than leaving half a responsive rule that silently does nothing. + */ +@container (max-width: 620px) { + .activity, + .separator, + .elapsed { + display: none; + } +} diff --git a/packages/web-shell/client/components/GoalStatusStrip.test.tsx b/packages/web-shell/client/components/GoalStatusStrip.test.tsx new file mode 100644 index 00000000000..a6996847b6e --- /dev/null +++ b/packages/web-shell/client/components/GoalStatusStrip.test.tsx @@ -0,0 +1,194 @@ +// @vitest-environment jsdom +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { GoalSnapshotV2 } from '@qwen-code/sdk/daemon'; +import { I18nProvider } from '../i18n'; +import { GOAL_EVIDENCE_LIMIT_REASONS } from '../utils/goalGate'; +import { GoalStatusStrip, getGoalActiveTimeMs } from './GoalStatusStrip'; + +function snapshot( + status: NonNullable['status'], +): GoalSnapshotV2 { + return { + v: 2, + activity: status === 'active' ? 'running' : 'idle', + goal: { + goalId: 'goal-1', + revision: 2, + objective: 'ship every surface', + status, + evidenceCursor: { recordId: null }, + turnCount: 3, + activeTimeMs: 4000, + createdAt: 1000, + updatedAt: 5000, + }, + }; +} + +describe('GoalStatusStrip', () => { + let container: HTMLDivElement; + let root: Root; + + beforeEach(() => { + container = document.createElement('div'); + document.body.append(container); + root = createRoot(container); + }); + + afterEach(() => { + act(() => root.unmount()); + container.remove(); + }); + + function render(status: NonNullable['status']) { + const handlers = { + onEdit: vi.fn(), + onPause: vi.fn(), + onResume: vi.fn(), + onClear: vi.fn(), + }; + act(() => { + root.render( + + + , + ); + }); + return handlers; + } + + it('shows pause for an active Goal and wires actions', () => { + const handlers = render('active'); + expect(container.textContent).toContain('In progress'); + expect(container.textContent).toContain('ship every surface'); + + act(() => { + container + .querySelector('[aria-label="Edit goal"]')! + .click(); + container + .querySelector('[aria-label="Pause goal"]')! + .click(); + container + .querySelector('[aria-label="Clear goal"]')! + .click(); + }); + + expect(handlers.onEdit).toHaveBeenCalledOnce(); + expect(handlers.onPause).toHaveBeenCalledOnce(); + expect(handlers.onClear).toHaveBeenCalledOnce(); + expect(container.querySelector('[aria-label="Resume goal"]')).toBeNull(); + }); + + it('shows resume for recoverable stopped states and hides completed Goals', () => { + render('blocked'); + expect( + container.querySelector('[aria-label="Resume goal"]'), + ).not.toBeNull(); + expect(container.querySelector('[aria-label="Pause goal"]')).toBeNull(); + + act(() => { + root.render( + + + , + ); + }); + expect( + container.querySelector('[data-testid="goal-status-strip"]'), + ).toBeNull(); + }); + + it('hides resume for an evidence-limited Goal', () => { + // The reducer refuses to resume a Goal stopped at an evidence bound, so + // offering the control only earns the user an invalid-transition 409. + const limited = snapshot('usage_limited'); + act(() => { + root.render( + + + , + ); + }); + + expect(container.querySelector('[aria-label="Resume goal"]')).toBeNull(); + expect( + container.querySelector('[data-testid="goal-status-strip"]'), + ).not.toBeNull(); + }); + + it('hides resume for a Goal evidence-limited before `limitKind` existed', () => { + // The sentinel prose shipped before the `limitKind` field did, so a Goal + // persisted in that window restores as `usage_limited` with no `limitKind` + // at all. The reducer still refuses it; a gate keyed off `limitKind` alone + // offered a Resume button that could only ever earn a 409. + const limited = snapshot('usage_limited'); + for (const lastReason of GOAL_EVIDENCE_LIMIT_REASONS) { + act(() => { + root.render( + + + , + ); + }); + expect(container.querySelector('[aria-label="Resume goal"]')).toBeNull(); + } + }); + + it('still offers resume for an ordinary usage-limited stop', () => { + // Reverse control for the test above: operational stops carry prose in + // `lastReason` too and the reducer resumes them, so the fallback must not + // widen into "any usage_limited Goal with a reason". + const limited = snapshot('usage_limited'); + act(() => { + root.render( + + + , + ); + }); + expect( + container.querySelector('[aria-label="Resume goal"]'), + ).not.toBeNull(); + }); + + it('adds current active time only while active', () => { + expect(getGoalActiveTimeMs(snapshot('active'), 8000)).toBe(7000); + expect(getGoalActiveTimeMs(snapshot('paused'), 8000)).toBe(4000); + }); +}); diff --git a/packages/web-shell/client/components/GoalStatusStrip.tsx b/packages/web-shell/client/components/GoalStatusStrip.tsx new file mode 100644 index 00000000000..990b9887b93 --- /dev/null +++ b/packages/web-shell/client/components/GoalStatusStrip.tsx @@ -0,0 +1,129 @@ +import { useEffect, useState } from 'react'; +import type { GoalSnapshotV2 } from '@qwen-code/sdk/daemon'; +import { Pause, Pencil, Play, Target, Trash2 } from 'lucide-react'; +import { useI18n } from '../i18n'; +import { formatRuntime } from '../utils/formatRuntime'; +import { canResumeGoal } from '../utils/goalGate'; +import styles from './GoalStatusStrip.module.css'; + +const TICK_INTERVAL_MS = 1000; + +export interface GoalStatusStripProps { + snapshot: GoalSnapshotV2; + busy?: boolean; + onEdit: () => void; + onPause: () => void; + onResume: () => void; + onClear: () => void; +} + +export function getGoalActiveTimeMs( + snapshot: GoalSnapshotV2, + now: number, +): number { + const goal = snapshot.goal; + if (!goal) return 0; + return ( + goal.activeTimeMs + + (goal.status === 'active' ? Math.max(0, now - goal.updatedAt) : 0) + ); +} + +export function GoalStatusStrip({ + snapshot, + busy = false, + onEdit, + onPause, + onResume, + onClear, +}: GoalStatusStripProps) { + const { t } = useI18n(); + const goal = snapshot.goal; + const [now, setNow] = useState(() => Date.now()); + + useEffect(() => { + if (goal?.status !== 'active') return; + const id = window.setInterval(() => setNow(Date.now()), TICK_INTERVAL_MS); + return () => window.clearInterval(id); + }, [goal?.status]); + + if (!goal || goal.status === 'complete') return null; + + const canPause = goal.status === 'active'; + // An evidence-limited stop is terminal for resume: the reducer rejects it + // with an invalid-transition 409, so the control must not be offered. The + // reducer's own rule lives in `canResumeGoal` -- keying off `limitKind` + // alone here missed Goals persisted before that field existed. + const canResume = canResumeGoal(goal); + + return ( +
+
+ ); +} diff --git a/packages/web-shell/client/components/MessageItem.tsx b/packages/web-shell/client/components/MessageItem.tsx index 003cda4dbd9..caf916fa74b 100644 --- a/packages/web-shell/client/components/MessageItem.tsx +++ b/packages/web-shell/client/components/MessageItem.tsx @@ -34,7 +34,6 @@ interface MessageItemProps { onImagePreview?: (src: string, alt?: string) => void; onAttachmentPreview?: (file: AttachmentPreviewRequest) => void; workspaceCwd?: string; - isLatest?: boolean; showRetryHint?: boolean; onRetryClick?: () => void; sendFailed?: boolean; @@ -55,7 +54,6 @@ export const MessageItem = memo(function MessageItem({ onImagePreview, onAttachmentPreview, workspaceCwd, - isLatest = false, showRetryHint = false, onRetryClick, sendFailed = false, @@ -151,7 +149,6 @@ export const MessageItem = memo(function MessageItem({ onShowContextDetail={onShowContextDetail} onImagePreview={onImagePreview} onAttachmentPreview={onAttachmentPreview} - isLatest={isLatest} showRetryHint={showRetryHint && message.retryable === true} onRetryClick={onRetryClick} /> @@ -295,7 +292,6 @@ function areMessageItemPropsEqual( if (prev.onImagePreview !== next.onImagePreview) return false; if (prev.onAttachmentPreview !== next.onAttachmentPreview) return false; if (prev.workspaceCwd !== next.workspaceCwd) return false; - if (prev.isLatest !== next.isLatest) return false; if (prev.showRetryHint !== next.showRetryHint) return false; if (prev.onRetryClick !== next.onRetryClick) return false; if (prev.sendFailed !== next.sendFailed) return false; diff --git a/packages/web-shell/client/components/MessageList.tsx b/packages/web-shell/client/components/MessageList.tsx index 584cfc6680d..5713a44faa9 100644 --- a/packages/web-shell/client/components/MessageList.tsx +++ b/packages/web-shell/client/components/MessageList.tsx @@ -4844,10 +4844,7 @@ export const MessageList = memo( const renderVirtualItem = useCallback( (index: number) => { - const renderDisplayItem = ( - displayItem: DisplayItem, - isLatest: boolean, - ): ReactNode => { + const renderDisplayItem = (displayItem: DisplayItem): ReactNode => { if (displayItem.type === 'parallel_agents') { return ( @@ -4957,7 +4954,6 @@ export const MessageList = memo( onImagePreview={onImagePreview} onAttachmentPreview={onAttachmentPreview} workspaceCwd={workspaceCwd} - isLatest={isLatest} showRetryHint={showRetryHint} onRetryClick={onRetryClick} sendFailed={ @@ -4998,7 +4994,7 @@ export const MessageList = memo( const item = visibleItems[itemIndex]; if (!item) return null; - return renderDisplayItem(item, itemIndex === visibleItems.length - 1); + return renderDisplayItem(item); }, [ hasHeader, diff --git a/packages/web-shell/client/components/QueuedPromptDisplay.test.tsx b/packages/web-shell/client/components/QueuedPromptDisplay.test.tsx index 980e8c49dd9..99d9d1fd59b 100644 --- a/packages/web-shell/client/components/QueuedPromptDisplay.test.tsx +++ b/packages/web-shell/client/components/QueuedPromptDisplay.test.tsx @@ -39,6 +39,7 @@ function setup( ) { const handlers = { onDelete: vi.fn(), + onInsert: vi.fn(), onEdit: vi.fn(), }; const prompts: QueuedPrompt[] = overrides.prompts @@ -199,6 +200,83 @@ describe('QueuedPromptDisplay', () => { expect(container.textContent).not.toContain('插入'); }); + it('shows an explicit insert action for a locally held message', () => { + const { container } = setup({ + prompts: [{ id: 1, text: '等待主动插入' }], + }); + expect(container.textContent).toContain('插入'); + }); + + it('hides insert when mid-turn mutation is unavailable', () => { + const { container } = setup({ + prompts: [{ id: 1, text: '等待主动插入' }], + canMutateMidTurn: false, + }); + expect( + container.querySelector(`[aria-label="${t('queue.insert')}"]`), + ).toBeNull(); + }); + + it('hides insert when there is no running turn', () => { + const { container } = setup({ + prompts: [{ id: 1, text: '等待主动插入' }], + canInsertMidTurn: false, + }); + expect( + container.querySelector(`[aria-label="${t('queue.insert')}"]`), + ).toBeNull(); + }); + + it('hides insert for prompts with input annotations', () => { + const { container } = setup({ + prompts: [ + { + id: 1, + text: 'inspect this file', + inputAnnotations: [ + { + type: 'reference', + start: 8, + end: 17, + text: 'this file', + reference: { + id: 'file-1', + kind: 'data-table', + label: 'File', + value: '/tmp/a.ts', + serialized: 'this file', + }, + }, + ], + }, + ], + }); + expect( + container.querySelector(`[aria-label="${t('queue.insert')}"]`), + ).toBeNull(); + }); + + it('hides insert for prompts with file attachments', () => { + const { container } = setup({ + prompts: [ + { + id: 1, + text: 'inspect this file', + files: [ + { + name: 'a.ts', + media_type: 'text/typescript', + text: 'export {};', + }, + ], + }, + ], + }); + expect( + container.querySelector(`[aria-label="${t('queue.insert')}"]`), + ).toBeNull(); + }); + it('allows deleting but not editing a summary-only server row', () => { const { container } = setup({ prompts: [ @@ -450,11 +528,15 @@ describe('QueuedPromptDisplay', () => { expect(handlers.onDelete).toHaveBeenCalledWith(42); }); - it('does not render an insert action for a command prompt', () => { + it('disables the insert action for a command prompt', () => { const { container } = setup({ prompts: [{ id: 1, text: '/help me' }], }); - expect(container.querySelectorAll('button')).toHaveLength(2); - expect(container.textContent).not.toContain('插入'); + expect(container.querySelectorAll('button')).toHaveLength(3); + const insert = container.querySelector( + `[aria-label="${t('queue.insert')}"]`, + ); + expect(insert?.disabled).toBe(true); + expect(insert?.title).toBe(t('queue.insertCommandDisabled')); }); }); diff --git a/packages/web-shell/client/components/QueuedPromptDisplay.tsx b/packages/web-shell/client/components/QueuedPromptDisplay.tsx index efc4af8a8b8..3c7bb527a78 100644 --- a/packages/web-shell/client/components/QueuedPromptDisplay.tsx +++ b/packages/web-shell/client/components/QueuedPromptDisplay.tsx @@ -10,8 +10,10 @@ import type { DaemonInputAnnotation } from '@qwen-code/sdk/daemon'; import { Fragment } from 'react'; import deleteIconUrl from '../assets/icons/delete.svg'; import editIconUrl from '../assets/icons/edit.svg'; +import insertIconUrl from '../assets/icons/insert.svg'; import queueIconUrl from '../assets/icons/queue.svg'; import type { getTranslator } from '../i18n'; +import { isCommandPrompt } from '../utils/localCommandQueue'; import { useWebShellCustomization, type UserMessageContentParser, @@ -134,6 +136,7 @@ export interface QueuedPrompt { midTurnState?: 'submitting' | 'queued'; midTurnMessageId?: string; midTurnFailedAction?: 'delete' | 'edit'; + isInserting?: boolean; isEditing?: boolean; isRemoving?: boolean; payloadCompleteness?: 'complete' | 'summary-only'; @@ -143,7 +146,9 @@ export function QueuedPromptDisplay({ prompts, t, canMutateMidTurn = false, + canInsertMidTurn = true, onDelete, + onInsert, onEdit, onImagePreview, onAttachmentPreview, @@ -151,7 +156,9 @@ export function QueuedPromptDisplay({ prompts: readonly QueuedPrompt[]; t: ReturnType; canMutateMidTurn?: boolean; + canInsertMidTurn?: boolean; onDelete: (id: number) => void; + onInsert: (id: number) => void; onEdit: (id: number) => void; onImagePreview?: (src: string, alt?: string) => void; onAttachmentPreview?: (file: AttachmentPreviewRequest) => void; @@ -172,10 +179,11 @@ export function QueuedPromptDisplay({ latestPrompt.serverState !== 'running' && !latestPrompt.isEditing && !latestPrompt.isRemoving && + !latestPrompt.isInserting && latestPrompt.payloadCompleteness !== 'summary-only'; return ( -
+
{prompts.map((prompt) => { const preview = truncateQueuedPromptParts( getQueuedPromptParts(prompt, parseUserMessageContent), @@ -202,17 +210,29 @@ export function QueuedPromptDisplay({ const isSummaryOnly = prompt.payloadCompleteness === 'summary-only'; const showActions = !isMidTurnPending || canMutateMidTurn; const isRemoving = prompt.isRemoving === true; + const isInserting = prompt.isInserting === true; + const canInsert = + canMutateMidTurn && + canInsertMidTurn && + prompt.serverState === undefined && + prompt.serverPromptId === undefined && + !isMidTurnPending && + imageCount === 0 && + fileCount === 0 && + (prompt.inputAnnotations?.length ?? 0) === 0; const hasStateSpinner = isSubmitting || prompt.midTurnState === 'submitting' || prompt.isEditing === true || - isRemoving; + isRemoving || + isInserting; const isBusy = isSubmitting || isRunning || isMidTurnLocked || prompt.isEditing === true || - isRemoving; + isRemoving || + isInserting; const isEditDisabled = isBusy || isSummaryOnly; let editTitle = t('queue.editTip'); if (isEditDisabled) { @@ -333,7 +353,8 @@ export function QueuedPromptDisplay({ isQueued || isMidTurnPending || prompt.isEditing || - isRemoving ? ( + isRemoving || + isInserting ? ( ) : null} {showActions ? ( <> + {canInsert && ( + + )} )} - {goalLabel && - (onOpenGoals ? ( - - ) : ( - - {goalLabel} - - ))}
); diff --git a/packages/web-shell/client/components/WebShellTranscript.dom.test.tsx b/packages/web-shell/client/components/WebShellTranscript.dom.test.tsx index a10af04eef1..34605490035 100644 --- a/packages/web-shell/client/components/WebShellTranscript.dom.test.tsx +++ b/packages/web-shell/client/components/WebShellTranscript.dom.test.tsx @@ -370,15 +370,11 @@ describe('WebShellTranscript DOM integration', () => { expect(container.textContent).toContain('Hidden reasoning'); }); - it('suppresses session and goal events while preserving their text', () => { + it('suppresses session events while preserving their text', () => { const sessionEvents: unknown[] = []; - const goalEvents: unknown[] = []; const onSession = (event: Event) => sessionEvents.push((event as CustomEvent).detail); - const onGoal = (event: Event) => - goalEvents.push((event as CustomEvent).detail); window.addEventListener('qwen:open-session', onSession); - window.addEventListener('web-shell-goal-status-active', onGoal); const { container } = render( { expect(container.querySelector('a[role="button"]')).toBeNull(); expect(container.textContent).toContain('All checks pass'); expect(sessionEvents).toEqual([]); - expect(goalEvents).toEqual([]); window.removeEventListener('qwen:open-session', onSession); - window.removeEventListener('web-shell-goal-status-active', onGoal); }); it('mounts a themed scoped portal root and removes it on unmount', () => { diff --git a/packages/web-shell/client/components/dialogs/GoalEditDialog.test.tsx b/packages/web-shell/client/components/dialogs/GoalEditDialog.test.tsx new file mode 100644 index 00000000000..2f428afbcb8 --- /dev/null +++ b/packages/web-shell/client/components/dialogs/GoalEditDialog.test.tsx @@ -0,0 +1,133 @@ +// @vitest-environment jsdom +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { I18nProvider } from '../../i18n'; +import { WebShellPortalRootContext } from '../../portalRoot'; +import { ThemeProvider } from '../../themeContext'; +import { GoalEditDialog } from './GoalEditDialog'; + +describe('GoalEditDialog', () => { + let container: HTMLDivElement; + let portalRoot: HTMLDivElement; + let root: Root; + + beforeEach(() => { + container = document.createElement('div'); + portalRoot = document.createElement('div'); + document.body.append(container, portalRoot); + root = createRoot(container); + }); + + afterEach(() => { + act(() => root.unmount()); + container.remove(); + portalRoot.remove(); + }); + + it('mounts in the Web Shell portal and locks actions while saving', () => { + const onSave = vi.fn(); + const onClose = vi.fn(); + act(() => { + root.render( + + + + + + + , + ); + }); + + expect(container.querySelector('[role="dialog"]')).toBeNull(); + const dialog = portalRoot.querySelector('[role="dialog"]')!; + expect(dialog.getAttribute('aria-label')).toBe('Edit goal'); + expect(dialog.querySelector('textarea')?.value).toBe( + 'ship every surface', + ); + expect( + Array.from(dialog.querySelectorAll('button')).every( + (button) => button.disabled, + ), + ).toBe(true); + }); + + const renderDialog = (objective: string, onSave = vi.fn()) => { + act(() => { + root.render( + + + + + + + , + ); + }); + return portalRoot.querySelector( + '[role="dialog"] textarea', + )!; + }; + + const type = (textarea: HTMLTextAreaElement, text: string) => { + act(() => { + const setter = Object.getOwnPropertyDescriptor( + HTMLTextAreaElement.prototype, + 'value', + )!.set!; + setter.call(textarea, text); + textarea.dispatchEvent(new Event('input', { bubbles: true })); + }); + }; + + it('keeps a typed draft when the Goal objective changes underneath', () => { + // The parents pass live Goal state, so a concurrent edit from another + // client (or the refresh a failed save triggers) arrives as a new prop + // while the user is editing — the textarea holds the only copy. + const textarea = renderDialog('ship every surface'); + type(textarea, 'my typed edit'); + + renderDialog('concurrent edit from another client'); + + expect( + portalRoot.querySelector('[role="dialog"] textarea') + ?.value, + ).toBe('my typed edit'); + }); + + it('adopts a Goal objective change while the field is pristine', () => { + renderDialog('ship every surface'); + + renderDialog('concurrent edit from another client'); + + expect( + portalRoot.querySelector('[role="dialog"] textarea') + ?.value, + ).toBe('concurrent edit from another client'); + }); + + it('saves the typed draft, not the refreshed objective', () => { + const onSave = vi.fn(); + const textarea = renderDialog('ship every surface', onSave); + type(textarea, 'my typed edit'); + renderDialog('concurrent edit from another client', onSave); + + const save = Array.from( + portalRoot.querySelectorAll('[role="dialog"] button'), + ).find((button) => button.textContent === 'Save')!; + act(() => save.click()); + + expect(onSave).toHaveBeenCalledWith('my typed edit'); + }); +}); diff --git a/packages/web-shell/client/components/dialogs/GoalEditDialog.tsx b/packages/web-shell/client/components/dialogs/GoalEditDialog.tsx new file mode 100644 index 00000000000..2dacf5aa732 --- /dev/null +++ b/packages/web-shell/client/components/dialogs/GoalEditDialog.tsx @@ -0,0 +1,94 @@ +import { useEffect, useState } from 'react'; +import { useI18n } from '../../i18n'; +import { DialogShell } from './DialogShell'; +import styles from './GoalsDialog.module.css'; + +interface GoalEditDialogProps { + objective: string; + saving: boolean; + error?: string | null; + onSave: (objective: string) => void; + onClose: () => void; +} + +export function GoalEditDialog({ + objective, + saving, + error, + onSave, + onClose, +}: GoalEditDialogProps) { + const { t } = useI18n(); + const [value, setValue] = useState(objective); + const [edited, setEdited] = useState(false); + const [localError, setLocalError] = useState(null); + + // Both mount sites pass live Goal state, so the objective changes under an + // open dialog whenever another client edits the Goal or a failed save + // refreshes the snapshot. Adopt those refreshes only while the field is + // still pristine: once the user has typed, the textarea holds the only copy + // of that draft. + useEffect(() => { + if (edited) return; + setValue(objective); + }, [objective, edited]); + + const submit = () => { + if (saving) return; + const trimmed = value.trim(); + if (!trimmed) { + setLocalError(t('goals.error.emptyCondition')); + return; + } + setLocalError(null); + onSave(trimmed); + }; + + return ( + !saving && onClose()} + > +
+