From 2870eeedf974e6913b55991f128f6b6e7d2989af Mon Sep 17 00:00:00 2001 From: qqqys Date: Thu, 20 Aug 2026 12:19:05 +0000 Subject: [PATCH 1/4] feat(web-shell): adopt canonical Goal v3 controls (#9393) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(web-shell): adopt canonical Goal v3 controls Route WebShell Goal lifecycle through the canonical Goal v3 control plane instead of model-bound chat commands. Goals can be created before the first message, then inspected, edited, paused, resumed, replaced and cleared directly. - core: extend the Goal reducer/runtime and protocol with the v3 control actions and a revisioned snapshot. - serve/acp: expose typed Goal state and controls over the daemon HTTP routes and the ACP bridge, with a shared error taxonomy. - sdk/webui: surface the same revisioned snapshot and control actions so every consumer reads one source of truth. - web-shell: render the active Goal as a compact composer row sized to match the queued-message row, drop the clear confirmation, and hold ordinary messages in a local FIFO queue while a Goal runs — only the explicit Insert action enters the active turn. The TUI keeps its existing presentation and command flow. Token-budget UI and desktop-shell adoption are intentionally out of scope. * test(serve): count the two Goal routes in the telemetry guards The PR registers `POST /session/:id/goal` and `GET /session/:id/goal`, both `handler_resolved`, taking the legacy session telemetry catalog from 59 routes to 61 and the attribution split from 57/2 to 59/2. Three hardcoded totals in telemetry.test.ts and one in the drift guard still asserted the old counts, so the Test job failed even though the guard's real check — registered Express routes equal the catalog — passed. Co-Authored-By: Claude Opus 5 * fix(webui): keep the Goal snapshot authoritative across replace, stale reads, and load Three ordering defects let a stale Goal frame win over the daemon's actual state, each of which then flips `holdQueuedPromptsLocally`, the Goal strip, and the manual-run gate off a goal that is not really there. - `selectGoalState` recorded an ordering identity only for a cleared goal, and `goal-runtime` attaches `clearedGoal` to a clear but not to a replace — so a pre-replace frame of the replaced goal passed every guard and reinstalled it over its replacement. Carry a bounded ledger of superseded goal identities (cleared and replaced alike) forward onto each accepted snapshot and reject frames at or behind one. - `getGoal()` reconciled a bare-null READ snapshot against whatever state existed at resolution time. A read the daemon answered while goal-less can land after a concurrent create, and with no `clearedGoal` tombstone it read as "clear whatever is current", wiping the new goal. Stamp the read with the goal observed at issue time: a bare-null response may only clear that goal. - The session-load path installed its snapshot behind a reference-equality guard instead of `selectGoalState`, so a frame arriving inside the load window discarded the authoritative response — and when none arrived, the raw install registered no tombstone and a later stale frame resurrected a cleared goal. Reconcile instead, and keep the synthesized empty snapshot for a failed fetch only while no state is known. Each fix is pinned by a test that fails when the fix is reverted. Co-Authored-By: Claude Opus 5 (1M context) * test(webui): pin the known-Goal-state branch of a failed session-load fetch Only the fresh-connection branch (synthesize an idle snapshot) was covered, so a simplification that always synthesizes on rejection would ship green and replace a live goal with idle on a transient `goal()` failure. Co-Authored-By: Claude Opus 5 (1M context) * fix(web-shell): keep Goal controls and drafts owned by the surface that started them Four Goal-surface defects, all of the same shape — state applied to whatever form, latch, or composer exists when an operation settles rather than to the one that started it: - `GoalEditDialog` live-synced its textarea to the `objective` prop, so a concurrent edit from another client (or the refresh a failed save triggers) silently overwrote the user's typed draft — the only copy. Adopt prop refreshes only while the field is still pristine. - `GoalsDialog` left its form dismissible while a submit was in flight, so closing it mid-request handed that request's `resetForm()`/`setFormError` to the next goal's form. Pass `dismissible={!submitting}`, matching `GoalEditDialog`. - ChatPane's `/goal` branch ran before the broken-connection guard applied further down the same `handleSubmit`, so a control typed while the pane was disconnected was consumed, written to the transcript, and then failed with only a toast. Apply the guard inside the branch and keep the text. - ChatPane's goal-control busy latch was session-keyed, so a server-side goal replacement released it mid-operation and a second control could dispatch against the same expected revision (one loses with a 409). Key the latch to the operation and release it only from its owner. Each fix is pinned by a test that fails when the fix is reverted. Co-Authored-By: Claude Opus 5 (1M context) * fix(web-shell): fail Goal gates closed while hydrating, and stop losing held prompts - The `goalSnapshotRef` gates (`promptBlocked`, retry, language change, fast-model select) failed OPEN while `connection.goalState` was still hydrating: the session load clears `loadingTranscript` before its `goal()` fetch resolves, so a prompt typed in that window was submitted straight into a Goal the client had not learned about — while the queue-hold gate, `enqueueManualRun` and `tryFireBoundRun` all failed closed on the identical state. They now share one `isGoalGateBlocked()` predicate that treats an unknown Goal state on a real session as blocked. - Creating a Goal in an allocated session went through the workspace-scoped control, which — unlike `sessionActions.controlGoal` — never wrote `connection.goalState`; the only compensation was a conditional re-sync one round trip later. Until it landed the hold gate read false and no Goal strip rendered. A new `applyGoalSnapshot` session action installs the create response into the connection state, reconciled like any other snapshot. - Locally held Goal prompts were stashed under `(workspaceCwd, sessionId)` and could only be relocated when the session just left was the same one, so a workspace resolving while the user was on another session orphaned the stash under a key nothing looks up again — silently losing typed text. Any stash whose session half matches is now relocated and restored in queue order. Each fix is pinned by a test that fails when the fix is reverted. The App test harness now defaults to a hydrated (goal-less) snapshot, matching a loaded session; the tests that exercise the hydration window set it back to undefined. Co-Authored-By: Claude Opus 5 (1M context) * fix(acp): publish turn_complete when a Goal turn ends Goal turns are driven inside the ACP child via its own prompt() call, so the daemon bridge never sees a session/prompt RPC boundary for them and published no turn terminal on SSE. Web Shell (and any SSE client) only settles its streaming state on turn_complete/turn_error/prompt_cancelled, so after a Goal turn the spinner spun forever and locally queued messages never drained — even though the daemon itself was already idle. The child now sends the existing `_qwencode/end_turn` notification with `source: 'goal'` and a turn-scoped promptId when a Goal turn settles, and the bridge translates that into a real `turn_complete` frame — the same contract the Web Shell goal e2e mock already assumed. * fix(web-shell): serialize held-prompt release, drop the dead insert abort registry - Releasing several locally held Goal prompts fired every submission at once, so a prompt awaiting media uploads could be overtaken by a later plain one and reach the daemon's queue out of order. `submitPendingPrompt` now returns its admission promise and the release chains them; the first release stays synchronous. - `explicitInsertAbortControllersRef` was populated and cleaned up but never read: nothing aborted its controllers, so the signal plumbing and the `abort.signal.aborted` recovery branches were unreachable. An explicit insert is meant to outlive an owner rotation and settle into the queue of the session it was started from (two tests pin exactly that), so the registry, the signal and the dead branches are gone rather than given a consumer. - Both mount sites rendered the Insert button enabled while streaming was idle and a Goal was active — precisely the state `insertQueuedPrompt` no-ops in. `canInsertMidTurn` now tracks the hook: the affordance appears only while a turn is running. Test coverage the review probes found missing, each verified by reverting the line it gates: the images and slash-command insert guards, the `isInserting` reset on both settle-unaccepted paths, the release order, the Insert affordance, and the held-prompt stash handoff (a prompt already dealt with must not come back from a stale owner key). Co-Authored-By: Claude Opus 5 (1M context) * fix(web-shell): localize Goal command errors and stop Goal surfaces swallowing input - `parseWebShellGoalCommand` returned an English sentence that `formatError` prefers over any localized fallback. It now returns the offending keyword and both composers render `goals.error.requiresObjective` from the dictionaries; the manual-run rejection gets `scheduledTasks.error.goalActive` for the same reason. - `handleGoalSlashCommand` returned true — wiping the composer — before the preconditions it checks asynchronously held, so a `/goal clear` typed without a session lost its text to a toast. The cheap preconditions are checked first and refuse the submit instead. - `enqueueManualRun` treated an unknown Goal state as "Goal is active", which also fires when no session is attached and no Goal can exist, so Run now on a fresh workspace always failed. It shares the hydration-aware gate now. - ChatPane consulted the host slash handler after its `/goal` intercept, so a host override applied in the main composer but not in a pane; and a bare `/goal` in a pane without a Goals view consumed the text silently. The pane now matches the composer's ordering and refuses what it cannot open. - GoalsDialog offered Edit on a completed Goal, which the reducer rejects, and fell back to the stale snapshot when the edited session left the list — turning the friendly "no longer available" path into a raw conflict error. - The versioned control request is built by one shared helper instead of two copies that had already drifted. Co-Authored-By: Claude Opus 5 (1M context) * refactor(web-shell): retire the dead Goal footer paths and restore the stranded-session rationale - `GOAL_STATUS_ACTIVE_EVENT` lost its only production listener in this PR while `GoalStatusMessage` kept dispatching it and two test files kept asserting the dispatch — a contract that looks live but is dead. The dispatch, the `activateFooter` prop, the `isLatest` plumbing that fed it, and the assertions are gone. - The built-in StatusBar now received `activeGoal={null}` permanently, so its goal pill and `onOpenGoals` wiring could never fire while StatusBar.test.tsx kept them green. The pill, its props, its elapsed-time ticker and that pill-only test file are removed; the composer status stack is the goal surface. Custom footers keep their `activeGoal` prop, which is still fed. - `/language ui` skipped its daemon sync when `promptBlocked`, silently switching the chrome while the agent kept answering in the old language for the rest of a Goal run. It now refuses with the same feedback the language picker gives for the identical condition. - The `@container` block in GoalStatusStrip styled `.root`, which cannot match its own container query, so half the responsive rule never applied. Trimmed to the descendant rules that do, with the reason recorded. - Restored the rationale comment above `strandedGoalSessionRef` — the mechanism it documents is unchanged and still live. Co-Authored-By: Claude Opus 5 (1M context) * fix(sdk): expose limitKind, drop the unused expected-version type, cover the Goal wire seams - `GoalRecord` in the SDK omitted `limitKind`, the field that decides whether a stopped Goal can be resumed. The daemon sends it, the webui mapper now parses it (rejecting unknown values), and both Resume controls hide for an evidence-limited Goal instead of offering a click that always 409s. - `GoalExpectedVersion` was added to the published SDK with zero read sites — the request type inlines the two fields — so it is removed before release turns it into a shape we must support forever. - Tests for the wire seams the review found uncovered, each verified by reverting the line it gates: the goal error-kind → HTTP mapping and its `current` forwarding, `GET /goals` listing paused/blocked/usage_limited and filtering complete, the untrusted-workspace gate for every work-expanding action (not just create), and `bridge.controlSessionGoal`'s `{ request }` envelope — the only producer of the shape the agent's handler reads. - acpAgent's core mock now carries the real `GoalConflictError` / `GoalInvalidTransitionError`, without which every `instanceof` branch in `mapGoalControlError` throws before it can be asserted. Co-Authored-By: Claude Opus 5 (1M context) * test(web-shell): close the Goal coverage gaps the review probes found Each test below fails when the line it gates is reverted (probe-verified): - App: the direct-submission `onSubmitBefore` rejection keeps the draft; a session-less stranded session is forgotten when the user leaves the Goals page; a create after a successful one starts a fresh session; the allocated create is dispatched only after the allocation completes; the Goal edit dialog closes when the same session's goal is replaced; a stale edit resolution does not install its snapshot over the session now displayed; the control busy latch stays owned by the session that started it; and the allocated-session create is observed through the strip the App renders rather than through the value the test's own mock wrote. - ChatPane: the control request is built from the freshly fetched Goal (both the pause payload and the `/goal set` → replace mapping); the edit dialog closes when the pane's goal changes; the "no longer available" message is pinned instead of `expect.any(Error)`; and a control dropped because the pane moved to another session no longer reports a failure toast — matching how the edit-save and main-composer paths already treat that race. - e2e/mock: the mock's `GET /session/:id/status` returns the flat `DaemonSessionSummary` the daemon really sends (the envelope left every field the client reads undefined), its clear response carries the `clearedGoal` tombstone so the anti-resurrection path is reachable, and the first Goal scenario re-checks that no prompt was sent after the create resolves. - Restored the one-shot run-now rationale comments in ScheduledTasksDialog and its tests; the behavior they explain is unchanged. Co-Authored-By: Claude Opus 5 (1M context) * fix(web-shell): drop the imports the retired Goal paths stranded `ed422f9a6c` removed the dead `GOAL_STATUS_ACTIVE_EVENT` contract and the StatusBar goal pill together with the tests that covered them, but left four imports behind with no remaining reference. `lint:ci` runs eslint with `--max-warnings 0`, so `@typescript-eslint/no-unused-vars` failed the required `Test (ubuntu-latest, Node 22.x)` gate on this head. - `StatusBar.tsx`: `useEffect`/`useState` fed only the goal pill's elapsed-time ticker, which went with the pill. - `SystemMessage.test.tsx`: `TranscriptRenderModeProvider` and `serializeGoalStatusMessage` were used only by the removed "goal status activation" describe block. No behavior change and no test weakened — the covered production paths were deleted in the same commit that stranded these imports. Co-Authored-By: Claude Opus 5 * fix(acp): surface Goal-turn activity in hasActivePrompt summaries Goal turns never flip the bridge's promptActive flag because they run inside the ACP child without a session/prompt RPC, so live-state reported hasActivePrompt: false while a Goal was executing and the Web Shell sidebar showed no activity indicator for that session. The child now sends `_qwencode/start_turn` (source `goal`) when a Goal turn begins; the bridge tracks it as a separate goalTurnActive flag and ORs it into every hasActivePrompt summary. The existing goal end_turn signal clears the flag, the start/end pair now lives in the goal queue drain so a prompt that rejects early cannot leak the flag, and an RPC prompt start self-heals a stale flag since the child serializes the two. * fix(acp): let a mid-turn insert reach a running Goal turn A Goal turn runs inside the ACP child via `prompt()` directly, so the bridge never sees a `session/prompt` RPC for it and `pendingPromptCount` stays 0 for its whole duration. `POST /session/:id/mid-turn-message` passes `rejectIfIdle: true` and the admission gate reads only that counter, so every explicit Insert during a Goal turn was answered `accepted: false` — while the Web Shell enables the affordance precisely because `c9597f6e` made a Goal turn non-idle in `hasActivePrompt`. The client returned the row to its hold and reported `insertFailed`; the media variant deleted the uploaded attachments with it. The e2e mock answers `accepted: true` unconditionally, so nothing caught it. The session is genuinely busy during a Goal turn — the child drains this same queue between tool batches from inside it — so admission now treats `goalTurnActive` as busy and the message is queued for that drain. A Goal turn owns no prompt slot, so nothing settled what its last drain missed: the goal `end_turn` signal now closes that window the way the prompt terminal already does (`queueOnly` callers get `onSettledWithoutDrain`, everything else is promoted). Promotion 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. Both tests fail when their line is reverted (probe-verified). Co-Authored-By: Claude Opus 5 * fix(webui): stamp the session-load Goal read the way `getGoal` does The R1-10 fix landed only in `getGoal()`: the session-load path still routed a fulfilled bare-null `goal()` read straight through `selectGoalState`. The load issues that read while the session is goal-less, so the answer carries no `clearedGoal` tombstone — and `selectGoalState` then derives the clear target from whatever the store holds when it is applied. A goal created inside the load window (the Web Shell allocates a session, then creates the goal on it, and the load's `Promise.allSettled` is gated on its slowest sibling request) was therefore accepted as the thing being cleared, and its identity was written into `clearedGoalOrder`. From there `isSupersededGoalFrame`'s `<=` tie rejected every later frame of that goal at the same revision — including the `refreshGoal()` that follows the create. The daemon held a live goal while the store reported goal-less, `holdQueuedPromptsLocally` read false so ordinary prompts bypassed the Goal queue, and the tombstone survived reloads. `getGoal`'s guard moves into a shared `selectGoalStateFromRead`, and the load path stamps its read with `goalStateAtLoadStart` — the goal it observed when the read was issued, which it already captures. The provider test mirrors actions.test.ts's stale-read case and fails when the stamp is replaced by an apply-time read (probe-verified). Co-Authored-By: Claude Opus 5 * fix(web-shell): close two Goal-queue leaks in the composer and queue Two Goal-queue leaks the review found on this head. `ChatPane`'s composer submit read `goalState?.goal?.status !== 'active'`, which is TRUE while the snapshot is still hydrating — the session load clears `loadingTranscript` before its `goal()` resolves, and the daemon has no server-side prompt gate for an active Goal, so a prompt typed in that window went straight to the daemon and bypassed the queue. Every other gate in the client already fails closed on the same window. The predicate now lives in `utils/goalGate`, shared by App's `isGoalGateBlocked` and both of ChatPane's gates, so they cannot drift apart again. The pane fixture gains the Goal snapshot a loaded session always carries — without it the fixture models a hydrating session, not a Goal-less one. `useQueuedPrompts` captured the stash owner key once when an explicit insert started, but the workspace half of that key can resolve mid-flight and the owner-change effect relocates the whole stash onto the new key, deleting the old one. The accepted `midTurnState: 'queued'` write then hit a key nobody holds and was silently dropped: the row came back from the stash still `isInserting`, and release/edit/delete/clear all skip such a row — bricked until a reload. The key is now resolved at use time by the session half (the same uniqueness invariant the relocation itself relies on), and the four inline owner-match copies collapse into one helper that compares that half rather than the whole key. Both new tests fail when the line they gate is reverted (probe-verified). Co-Authored-By: Claude Opus 5 * fix(web-shell): re-check the Goal hold on every release-chain link The chain that drains locally-held prompts is built synchronously when the hold lifts, but each link runs only after the previous admission settles, and `submitPendingPrompt` never consults the hold. Pausing a Goal (which starts the drain) and resuming it inside the chain-length x admission-latency window left the remaining links POSTing against an active Goal — the queue emptied against the user's change of mind, which is the contract this PR exists to hold. Each link now re-checks the hold (and the write block) and returns its row to held instead of sending it; the next inactive transition re-drains in order. The revert is inline rather than through `setQueuedPromptFlags` because that callback is declared below this effect, so naming it in the dep array would read it before its initializer. Also pins `mapGoalControlError` at the ext-method layer, which had no coverage: `sessionGoalControl`'s only tests were the success path and the untrusted gate, and the gate throws before the mapping is reachable. The new case drives real `GoalConflictError` / `GoalInvalidTransitionError` / persistence rejections through `extMethod` and asserts the code plus intact `data.errorKind` and `data.current` — the payload the client's 409 resync depends on. Both fail when the line they gate is reverted (probe-verified). Co-Authored-By: Claude Opus 5 * refactor(web-shell): stamp GoalsDialog's CAS fields through the shared builder `versionedRequest` was a private second writer of `expectedGoalId` / `expectedRevision` — the exact drift `buildGoalControlRequest` was introduced to prevent. The dialog's two call sites now go through the utility; behaviour is unchanged for them (both always hold a goal, so the builder's `goalUnavailable` guard is unreachable there, and `edit` falls back to the goal's own objective as before). Co-Authored-By: Claude Opus 5 * fix(web-shell): stop offering Goal controls the reducer will reject Clears the three Critical findings from round 2 of the review on #9393. R2-1 / R2-2 — both resume gates (`GoalStatusStrip.tsx`, `GoalsDialog.tsx`) keyed off `goal.limitKind === undefined` alone, while core's `isEvidenceLimited` (`goal-reducer.ts`) also treats a Goal as evidence-limited when `lastReason` is one of the two sentinel strings. Those sentinels shipped before the `limitKind` field did, so a Goal persisted in that window restores as `usage_limited` with no `limitKind` at all: the UI offered a Resume button that `reduceGoalControl` is guaranteed to answer with `GoalInvalidTransitionError`. Both gates now go through one `canResumeGoal` in `utils/goalGate.ts` — the module whose documented invariant is that every client Goal gate runs through a shared predicate so none can drift. It states the reducer's rule directly (complete/active refuse; `usage_limited` refuses when evidence-limited) rather than approximating it, and it keeps the evidence check scoped to `usage_limited` exactly as the reducer does, so a `paused` Goal carrying stale sentinel prose is not stranded without a Resume control. The Web Shell client bundles for the browser and cannot import `@qwen-code/qwen-code-core`, so the two sentinel strings are duplicated. A comment asking the next person to keep them in sync is not a mechanism: `goalGate.test.ts` reads `packages/core/src/goals/goal-protocol.ts` and fails if either literal moves, or if core grows a third sentinel branch. R2-3 — a session switch mid-drain silently destroyed queued prompts. The serial release chain stamps the whole batch `serverState: 'submitting'` up front and then releases it one link at a time (a prompt carrying media awaits its uploads, so that window is seconds, not microseconds). Two things went wrong inside it: the chain carried no owner token, so remaining links called `submitPrompt` against the old session, which throws before POSTing and was swallowed by the chain's own `.catch`; and the owner-change stash only saves rows matching `isLocallyHeldPrompt` (`serverState === undefined`), so every chain-marked row was excluded from it. Both typed prompts were gone with nothing restored to the editor. Each link now bails when the owner token it was built for is no longer current, and a new `unreleasedPromptIdsRef` tracks exactly the rows the chain stamped but never handed to `submitPendingPrompt`. The owner-change effect stashes those alongside the genuinely held rows, dropping the optimistic stamp on the way in so the next drain re-releases them in order. Deliberately narrower than the finding's suggested fix on one point: the row whose admission is actually in flight is NOT stashed. Restoring it would flip the behaviour pinned by 'ignores an old submit response after an S1 to S2 to S1 owner change' and 'fences an old submit before the replacement owner rerenders', which deliberately fence and drop an in-flight submission across an owner change — that POST may well have landed, so resurrecting the row risks a duplicate. The rows the chain never POSTed have no such ambiguity: nothing exists for them on the daemon, so stashing them cannot duplicate anything. Reversing the in-flight contract is a design call for the reviewer, not a drive-by. Also fixes R2-12 in passing, since the new tests need it: `MockGoal`'s snapshot `status` union omitted `'complete'` (a TS2322 against this file's own later usage) and had no `limitKind`. Verification: `npx vitest run client/hooks/ client/utils/ client/components/GoalStatusStrip.test.tsx client/components/dialogs/GoalsDialog.test.tsx client/components/ChatPane.test.tsx` in `packages/web-shell` — 1011 passed (994 on the stashed tree, so all 17 new tests pass), with an identical 17 pre-existing failures in the untouched `hooks/useMessages.test.ts` on both trees. eslint clean on all eight touched files. Every fix is mutation-verified. Dropping the sentinel fallback, widening it to any `lastReason`, or unscoping it from `usage_limited` each turns a distinct test red at both the unit and the component level; removing the chain owner guard, the unreleased-row half of the stash filter, or the stamp reset each turns the new mid-drain test red. Co-Authored-By: Claude Opus 5 * fix(web-shell): stop Goal controls and the release chain racing each other Four round-3 Criticals, each reproduced first and then mutation-verified. R3-4 / R3-19 — `goalControlBusy` was a shared latch with no owner. A create in an allocated session released it unconditionally in `finally`, and the composer `/goal` path never consulted it at all, so two controls could read the same snapshot, stamp the same expectedGoalId/expectedRevision, and let the daemon reject the loser with a 409. Ported ChatPane's twin pattern — `goalControlOpSeqRef`/`goalControlOwnerRef`, released only by the operation that still owns it — to both `controlCurrentGoal` and `createGoalForAllocatedSession`, and made `handleGoalSlashCommand` refuse (keeping the composer text) while a control is in flight. R3-24 — a chain link deleted its id from `unreleasedPromptIdsRef` before the owner check. The owner token is replaced in the render body while the stash is a passive effect flushed after commit, so a link firing in that window left a row that was neither locally held nor unreleased, and the stash dropped it: the typed prompt vanished with no POST, no abort, no toast. Delete moved below the owner check, as the review suggested. R3-20 — a queue clear mid-drain aborts only the in-flight link's controller; the chain's pending links have no controller yet, so they went on to POST prompts the user had explicitly cleared. The link now bails when its row has left the queue. Verification: 775 passing across App / useQueuedPrompts.dom / useQueuedPrompts.midTurnReconcile / ChatPane / GoalStatusStrip / GoalsDialog; full web-shell 3906 passing with the same 18 failures the branch has without this change (useMessages background-agent reconciliation, build-artifact — confirmed by stash-and-rerun). tsc unchanged at 66 pre-existing errors from unbuilt workspace deps. ESLint and Prettier clean on the touched files. Mutation-verified: each of the four fixes reverted in isolation turns exactly its own new test red. * test(cli): align merged replay expectations * fix(web-shell): keep a prompt typed mid-drain behind the release chain R3-2: the serial release chain preserved order only inside the batch it drained. It exists because the prompt at its head may await media uploads for seconds; a prompt typed inside that window went straight through `submitPendingPrompt`, so the daemon admitted it ahead of the older held rows it was typed after -- and while link 1's upload was still running it could overtake link 1 itself and start the turn. The chain is now published on `releaseChainRef` (owner-pinned, retired once its newest tail settles), and `enqueuePrompt`'s ordinary path appends to that tail instead of POSTing past it. The waiting row is registered in `unreleasedPromptIdsRef`, so it is stamped-but-not-POSTed exactly like the chain's own undrained rows and the owner-change stash saves its text rather than losing it. The per-link guards (owner pinned at build time, row-still-present, hold/write-block revert) move into a shared `releaseChainedPrompt` so both the drain and the appended send carry identical semantics. A re-drain for a live owner now extends the existing chain instead of racing it. Test: `holds a prompt typed mid-drain behind the release chain` -- two held prompts with link 1's admission hung, a third typed mid-drain; asserts it does not POST while the chain is in flight and that final order is ['first with media', 'second plain', 'typed during drain']. Mutation-verified: disabling the append arm turns the test red with the reported symptom (the mid-drain prompt POSTs immediately, order inverted). Verification: `npx tsc --noEmit` clean; `npx vitest run` in packages/web-shell -> 190 files / 3986 tests passed. --------- Co-authored-by: Claude Opus 5 Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com> --- packages/acp-bridge/src/bridge.test.ts | 174 +- packages/acp-bridge/src/bridge.ts | 141 +- packages/acp-bridge/src/bridgeClient.test.ts | 90 + packages/acp-bridge/src/bridgeClient.ts | 55 +- packages/acp-bridge/src/bridgeTypes.ts | 19 +- packages/acp-bridge/src/status.ts | 1 + .../acp-bridge/src/transcript-replay.test.ts | 26 +- packages/acp-bridge/src/transcript-replay.ts | 51 + .../cli/src/acp-integration/acpAgent.test.ts | 168 ++ packages/cli/src/acp-integration/acpAgent.ts | 90 + .../acp-integration/session/Session.test.ts | 56 + .../src/acp-integration/session/Session.ts | 39 +- .../session/history-replay-page.test.ts | 6 +- packages/cli/src/serve/routes/goals.test.ts | 60 + packages/cli/src/serve/routes/goals.ts | 22 +- packages/cli/src/serve/routes/session.ts | 46 +- packages/cli/src/serve/server.test.ts | 179 +- .../src/serve/server/error-response.test.ts | 55 + .../cli/src/serve/server/error-response.ts | 20 + .../serve/server/telemetry-catalog.test.ts | 2 +- .../cli/src/serve/server/telemetry.test.ts | 8 +- packages/cli/src/serve/server/telemetry.ts | 12 + packages/core/src/goals/goal-protocol.ts | 7 + packages/core/src/goals/goal-reducer.test.ts | 17 + packages/core/src/goals/goal-reducer.ts | 25 +- packages/core/src/goals/goal-runtime.test.ts | 5 + packages/core/src/goals/goal-runtime.ts | 9 + .../sdk-typescript/src/daemon/DaemonClient.ts | 44 +- .../src/daemon/DaemonSessionClient.ts | 126 +- packages/sdk-typescript/src/daemon/index.ts | 8 + packages/sdk-typescript/src/daemon/types.ts | 64 + .../test/unit/DaemonClient.test.ts | 72 + .../test/unit/DaemonSessionClient.test.ts | 56 + packages/web-shell/client/App.test.tsx | 1542 ++++++++++++++--- packages/web-shell/client/App.tsx | 703 ++++---- .../web-shell/client/assets/icons/insert.svg | 1 + .../client/components/ChatPane.test.tsx | 707 +++++++- .../web-shell/client/components/ChatPane.tsx | 254 ++- .../components/ComposerStatusStack.module.css | 24 + .../components/GoalStatusStrip.module.css | 114 ++ .../components/GoalStatusStrip.test.tsx | 194 +++ .../client/components/GoalStatusStrip.tsx | 129 ++ .../client/components/MessageItem.tsx | 4 - .../client/components/MessageList.tsx | 8 +- .../components/QueuedPromptDisplay.test.tsx | 88 +- .../client/components/QueuedPromptDisplay.tsx | 64 +- .../web-shell/client/components/SplitView.tsx | 3 + .../client/components/StatusBar.test.tsx | 115 -- .../web-shell/client/components/StatusBar.tsx | 65 +- .../WebShellTranscript.dom.test.tsx | 8 +- .../dialogs/GoalEditDialog.test.tsx | 133 ++ .../components/dialogs/GoalEditDialog.tsx | 94 + .../components/dialogs/GoalsDialog.test.tsx | 333 +++- .../client/components/dialogs/GoalsDialog.tsx | 234 ++- .../dialogs/ScheduledTasksDialog.test.tsx | 4 +- .../dialogs/ScheduledTasksDialog.tsx | 6 +- .../components/messages/GoalStatusMessage.tsx | 21 - .../messages/SystemMessage.test.tsx | 34 - .../components/messages/SystemMessage.tsx | 4 +- packages/web-shell/client/customization.tsx | 8 +- .../web-shell/client/e2e/utils/mockDaemon.ts | 202 +++ .../client/e2e/web-shell.goal.spec.ts | 200 +++ .../hooks/useQueuedPrompts.dom.test.tsx | 853 ++++++++- ...useQueuedPrompts.midTurnReconcile.test.tsx | 381 +++- .../client/hooks/useQueuedPrompts.ts | 713 +++++++- packages/web-shell/client/i18n.tsx | 68 + .../web-shell/client/midTurnDedup.test.ts | 30 + packages/web-shell/client/midTurnDedup.ts | 7 +- .../client/utils/goalCondition.test.ts | 21 + .../web-shell/client/utils/goalCondition.ts | 31 + .../client/utils/goalControlRequest.ts | 52 + .../web-shell/client/utils/goalGate.test.ts | 175 ++ packages/web-shell/client/utils/goalGate.ts | 91 + .../session/DaemonSessionProvider.test.tsx | 342 ++++ .../daemon/session/DaemonSessionProvider.tsx | 77 +- .../webui/src/daemon/session/actions.test.ts | 209 ++- packages/webui/src/daemon/session/actions.ts | 92 + .../webui/src/daemon/session/mappers.test.ts | 482 ++++++ packages/webui/src/daemon/session/mappers.ts | 275 +++ packages/webui/src/daemon/session/types.ts | 16 + .../webui/src/daemon/workspace/actions.ts | 52 +- .../daemon/workspace/goals.actions.test.ts | 67 +- packages/webui/src/daemon/workspace/types.ts | 11 +- 83 files changed, 9945 insertions(+), 1149 deletions(-) create mode 100644 packages/web-shell/client/assets/icons/insert.svg create mode 100644 packages/web-shell/client/components/ComposerStatusStack.module.css create mode 100644 packages/web-shell/client/components/GoalStatusStrip.module.css create mode 100644 packages/web-shell/client/components/GoalStatusStrip.test.tsx create mode 100644 packages/web-shell/client/components/GoalStatusStrip.tsx delete mode 100644 packages/web-shell/client/components/StatusBar.test.tsx create mode 100644 packages/web-shell/client/components/dialogs/GoalEditDialog.test.tsx create mode 100644 packages/web-shell/client/components/dialogs/GoalEditDialog.tsx create mode 100644 packages/web-shell/client/e2e/web-shell.goal.spec.ts create mode 100644 packages/web-shell/client/utils/goalControlRequest.ts create mode 100644 packages/web-shell/client/utils/goalGate.test.ts create mode 100644 packages/web-shell/client/utils/goalGate.ts diff --git a/packages/acp-bridge/src/bridge.test.ts b/packages/acp-bridge/src/bridge.test.ts index eb333f6a58..458d7560ac 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 5155d452ec..70a75ce7ee 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 dc4f252a97..ca0473bd0d 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 8e4a29996a..b0b424f16b 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 3332371c2d..1e2d087a05 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 51cb8ac603..7a49486b3c 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 522260986a..94d21b07ed 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 3b8553c55a..b8a6c8b645 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 b3d6350c3a..3276722c7f 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 5d5909e2da..5dfeaa1ae4 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 7d030f26da..91a03d6381 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 73e1fd40cc..c60c5998bd 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 7b82a6046b..2f75c51d9d 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 fcff6888d1..0360cacee1 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 e33aa52d5a..9b248ddd2a 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 f841dfcc56..010253e727 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 eeddb2c6f8..0aa05c9261 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 e3fe841d72..05b00ce960 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 89b4b9d93c..523faf7f20 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 4c8c7a21f4..f04e17b2e9 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 ea675d074e..3c351646d9 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 3702373f21..a8765e1ee7 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 ed5d2bcf6a..1fddb0eb17 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 8e602b8e6f..7c5a1f3bff 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 270d374a4f..c11c7b0750 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 50db7af790..488d6c304f 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 9e460af861..de3f15915c 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 501b53eec2..fc8a351cc9 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 f86635d3d3..86443702df 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); } - async stats(): Promise { - return await this.client.sessionStats(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, + ); + } + + 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 56899b4a79..b32b7c0c91 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 a9520856ee..9b1d23b0e3 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 b0a9464552..d62ef63dfb 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 d25d2a54c6..34d7d7f7a4 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 9d5dfc4d7f..f670cf584c 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,19 +20074,305 @@ 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 vi.waitFor(() => { + expect(mockSessionActions.controlGoal).toHaveBeenCalledWith({ + action: 'create', + objective: 'ship it', + }); + }); + + expect(container.querySelector('[data-testid="goals-page"]')).toBeNull(); + expect(mockSessionActions.getGoal).toHaveBeenCalledTimes(1); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + expect(mockStore.appendLocalUserMessage).toHaveBeenCalledWith( + '/goal ship it', + ); + }); + + 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 ship it'; await clickSubmit(container); + await vi.waitFor(() => { + expect(mockSessionActions.controlGoal).toHaveBeenCalledTimes(1); + }); + + testState.prompt = '/goal ship something else'; + await clickSubmit(container); await flush(); - expect(container.querySelector('[data-testid="goals-page"]')).toBeNull(); - expect(mockSessionActions.sendPrompt).toHaveBeenCalled(); + expect(mockSessionActions.controlGoal).toHaveBeenCalledTimes(1); + expect(mockStore.appendLocalUserMessage).toHaveBeenCalledTimes(1); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + + await act(async () => { + pendingControl.resolve({ snapshot: activeGoalSnapshot('ship it') }); + await flush(); + }); }); - it('still routes /goal clear through the daemon clear path', async () => { + 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 first objective'; + await clickSubmit(container); + await vi.waitFor(() => { + expect(mockWorkspaceActions.controlGoal).toHaveBeenCalledWith( + 'session-created', + { action: 'create', objective: 'first objective' }, + ); + }); + + expect(mockSessionActions.createSession).toHaveBeenCalledOnce(); + expect(mockSessionActions.attachSession).toHaveBeenCalledOnce(); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + }); + + 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 first objective'; + await clickSubmit(container); + await vi.waitFor(() => { + expect(mockSessionActions.getGoal).toHaveBeenCalledOnce(); + }); + + expect( + mockWorkspaceActions.controlGoal.mock.invocationCallOrder[0], + ).toBeLessThan(mockSessionActions.getGoal.mock.invocationCallOrder[0]!); + expect(mockConnection.goalState).toBe(active); + }); + + 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 first objective'; + await clickSubmit(container); + await vi.waitFor(() => { + expect(mockSessionActions.applyGoalSnapshot).toHaveBeenCalledWith( + 'session-created', + active, + ); + }); + await flush(); + + 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 () => { + testState.latestChatEditorProps?.onSubmit('bypass me'); + await flush(); + }); + + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + expect(rawEnqueuePrompt.mock.calls[0]?.[0]).toBe('bypass me'); + }); + + 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(); + + let accepted: boolean | undefined; + act(() => { + accepted = testState.latestChatEditorProps?.onSubmit( + '/goal clear', + undefined, + undefined, + editorCommit, + ); + }); + await flush(); + + expect(accepted).toBe(false); + expect(editorCommit).not.toHaveBeenCalled(); + expect(mockSessionActions.controlGoal).not.toHaveBeenCalled(); + expect(mockSessionActions.createSession).not.toHaveBeenCalled(); + }); + + 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(); + + expect(accepted).toBe(false); + expect(editorCommit).not.toHaveBeenCalled(); + expect(onToast).toHaveBeenCalledWith( + 'error', + '/goal set requires an objective.', + ); + + 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(); @@ -19914,11 +20380,403 @@ describe('App /goal command', () => { await clickSubmit(container); await flush(); - expect(container.querySelector('[data-testid="goals-page"]')).toBeNull(); - expect(mockSessionActions.clearGoal).toHaveBeenCalled(); + expect(mockSessionActions.createSession).not.toHaveBeenCalled(); + expect(mockSessionActions.attachSession).not.toHaveBeenCalled(); + expect(mockSessionActions.controlGoal).not.toHaveBeenCalled(); + expect(mockStore.appendLocalUserMessage).not.toHaveBeenCalledWith( + '/goal clear', + ); }); - it('starts a goal in a fresh session from the Goals page', async () => { + 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.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(); + + testState.prompt = '/goal'; + await clickSubmit(container); + await flush(); + + const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; + if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); + mockSessionActions.controlGoal.mockRejectedValueOnce( + new Error('daemon says no'), + ); + + await act(async () => { + 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(); @@ -19929,99 +20787,329 @@ describe('App /goal command', () => { const onCreateGoal = testState.latestGoalsProps?.onCreateGoal; if (!onCreateGoal) throw new Error('onCreateGoal was not captured'); mockSessionActions.clearSession.mockClear(); - mockSessionActions.sendPrompt.mockClear(); + mockSessionActions.controlGoal.mockRejectedValueOnce( + new Error('daemon says no'), + ); await act(async () => { - await onCreateGoal('all tests pass'); + await expect(onCreateGoal('all tests pass')).rejects.toThrow( + 'daemon says no', + ); }); - - // 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. - 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.sendPrompt.mockRejectedValueOnce( - new Error('daemon says no'), - ); - - await act(async () => { - await expect(onCreateGoal('all tests pass')).rejects.toThrow( - 'daemon says no', - ); - }); - - // 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(); - }); - - it('keeps the Goals page open when the goal prompt is rejected', 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.sendPrompt.mockRejectedValueOnce( - new Error('daemon says no'), - ); - - await act(async () => { - await expect(onCreateGoal('all tests pass')).rejects.toThrow( - 'daemon says no', - ); - }); - - // 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(); - }); - - it('switches to the chat view only after the goal prompt is admitted', 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'); 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'); + }); + + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(2); + }); + + 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.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); + + 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 retryCreate('all tests pass'); + }); + + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(2); + }); + + 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.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); + + // The retry reuses the stranded session rather than piling up a blank one. + mockSessionActions.controlGoal.mockResolvedValue({ + snapshot: activeGoalSnapshot('all tests pass'), + }); + await act(async () => { + await onCreateGoal('all tests pass'); + }); + expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); + + 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(); + + 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).toHaveBeenCalledTimes(1); + }); + + act(() => { + testState.ownerVersion += 1; + mockConnection.sessionId = 'conversation-session'; + rerender({}); + }); + pendingCreate.reject(new Error('daemon says no')); + await act(async () => { + await expect(firstCreate).rejects.toThrow('daemon says no'); + }); + + 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('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(); + + 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 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 })); + + 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"]', + ); + 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 () => { @@ -20089,140 +21177,6 @@ describe('App /goal command', () => { consoleError.mockRestore(); }); - 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. - 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( - new Error('daemon says no'), - ); - - await act(async () => { - await expect(onCreateGoal('all tests pass')).rejects.toThrow( - 'daemon says no', - ); - }); - 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. - await act(async () => { - await onCreateGoal('all tests pass'); - }); - - expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); - expect(mockSessionActions.sendPrompt).toHaveBeenLastCalledWith( - '/goal all tests pass', - expect.anything(), - ); - }); - - 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. - 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( - new Error('daemon says no'), - ); - await act(async () => { - await expect(onCreateGoal('all tests pass')).rejects.toThrow( - 'daemon says no', - ); - }); - 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'); - await act(async () => { - back.dispatchEvent(new MouseEvent('click', { bubbles: true })); - }); - await flush(); - expect(container.querySelector('[data-testid="goals-page"]')).toBeNull(); - - testState.prompt = 'hello from the composer'; - await clickSubmit(container); - 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'); - mockSessionActions.clearSession.mockClear(); - - await act(async () => { - await onCreateGoalAgain('all tests pass'); - }); - - expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); - }); - - 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. - 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( - new Error('daemon says no'), - ); - await act(async () => { - await expect(onCreateGoal('first goal')).rejects.toThrow( - 'daemon says no', - ); - }); - await act(async () => { - await onCreateGoal('first goal'); - }); - expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); - - // A brand-new goal after a successful send: fresh session again. - await act(async () => { - await onCreateGoal('second goal'); - }); - expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(2); - }); - it('does not drop the goal into the current session when the new session fails', async () => { const { container } = renderApp(); await flush(); @@ -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 c78c83d57c..e02f8708d1 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, - }), - }, - ]); + 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 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 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 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); + } + } }, - [store], - ); - - 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; - }, - [ - 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; - }; - - if (goalArg && isGoalClearKeyword(goalArg)) { - if (!sendToDaemon) { - store.appendLocalUserMessage(text); - dispatchGoalCleared(activeGoalRef.current); - return true; - } - return handleBusyGoalClear(text); - } else if (goalArg) { - if (!sendToDaemon) { - store.appendLocalUserMessage(text); - dispatchGoalSet(goalArg, Date.now()); - return true; - } - return sendGoalPrompt(); + (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; } - // 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(); + 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(); + } + 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; + } + 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 72612bf94c..35835bfb1d 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 cb0be82f82..6990fc2959 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 0000000000..a592387538 --- /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 0000000000..a6996847b6 --- /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 0000000000..990b9887b9 --- /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 003cda4dbd..caf916fa74 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 584cfc6680..5713a44faa 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 980e8c49dd..99d9d1fd59 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 efc4af8a8b..3c7bb527a7 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 a10af04eef..3460549003 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 0000000000..2f428afbcb --- /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 0000000000..2dacf5aa73 --- /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()} + > +
+