qwen-code/packages/web-shell/client/utils/goalControlRequest.ts
qqqys 2870eeedf9
feat(web-shell): adopt canonical Goal v3 controls (#9393)
* 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 <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>
Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com>
2026-08-20 12:19:05 +00:00

52 lines
1.5 KiB
TypeScript

/**
* @license
* Copyright 2026 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/
import type { GoalControlRequest, GoalRecord } from '@qwen-code/sdk/daemon';
export type GoalControlAction =
| 'create'
| 'replace'
| 'edit'
| 'pause'
| 'resume'
| 'clear';
/**
* Build the versioned control request for `action` against the goal the daemon
* just reported.
*
* Shared by the main composer and the split-view pane: both read a fresh
* snapshot, then stamp `expectedGoalId`/`expectedRevision` from it. Keeping the
* construction in one place is what stops the two surfaces from drifting apart
* on the optimistic-concurrency contract.
*/
export function buildGoalControlRequest(
action: GoalControlAction,
goal: GoalRecord | null | undefined,
objective: string | undefined,
errors: { emptyObjective: string; goalUnavailable: string },
): GoalControlRequest {
if (action === 'create' || action === 'replace') {
if (!objective) throw new Error(errors.emptyObjective);
// Replacing nothing is a create; the daemon rejects a replace that names a
// goal it no longer holds.
return goal
? {
action: 'replace',
objective,
expectedGoalId: goal.goalId,
expectedRevision: goal.revision,
}
: { action: 'create', objective };
}
if (!goal) throw new Error(errors.goalUnavailable);
return {
action,
...(action === 'edit' ? { objective: objective ?? goal.objective } : {}),
expectedGoalId: goal.goalId,
expectedRevision: goal.revision,
} as GoalControlRequest;
}