Repository navigation
Conversation
…ueing
The session and note chat composers could only queue a follow-up while a
session ran. Project notes recently gained live delivery through the
`continue_repo_session` MCP tool; this gives the composer the same path.
Capability flag. A new `get_session_live_delivery` command (Tauri + web)
reports `{ supported, reason }` for a session, computed by the backend from
the same gate `route_continue_session` applies before delivering to a
running row: the row is `running`, this process owns it (not another Staged
instance sharing the database), and its ACP connection has handed out a
live turn-control handle. Read without the startup wait, so a row still
connecting is "queue only" with a reason. The chat pane asks on each
500 ms poll tick until the answer is yes and drops the flag whenever the
status leaves `running`; nothing is duplicated client-side.
Send-now command. `send_session_message_now` wraps the shared
`continue_session_for_store` body — steer into the turn in flight, else
interrupt and re-prompt, resume a stopped session, refuse a queued one —
and returns the same `delivery` / `reason` spelling as the MCP tool. The
web arm runs detached like `resume_session`, since a resume may be in
flight when the client disconnects.
UI. `composerSendControl` derives the submit control: a plain "Send"
(start/resume) when the session is idle, a plain "Queue" when it runs but
cannot take a message now, and a split button once the backend says it
can — the primary segment performs the remembered mode (Queue / Send) and
a dropdown (existing dropdown-menu component, accent button styling)
switches it. Live "Send" success lets the poll pick up the persisted user
row; `blocked` / `rejected_queued` / `not_resumable` surface the reason and
restore the draft rather than silently re-queueing, because a steer the
agent never acknowledged may already have landed.
Not-running sessions. While a session is queued, completed, failed, or
cancelled the composer renders exactly what it did before (single
button); "Send" on a stopped session resumes it, which is what
`continue_repo_session` does for those states. "Send" is never offered
for a queued session, matching the tool's refusal.
Persistence. The mode is a global preference, `composer-send-mode` in the
existing Tauri-store-backed preferences, defaulting to `queue`; malformed
values fall back to the default.
Tests: live-delivery flag for connected / connecting / other-instance /
not-running rows and the delivery reply shape (Rust); mode persistence,
default, and malformed-value fallback across native and web store clients;
the single-vs-split render decision in `composerSendControl`.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…s four findings Follow-ups to b51a832 (Queue / Send split button), addressing the code review of that commit. Double-submit kept the first draft and lost the second. `handleSend` cleared the input before anything checked `sending`, and only the inner send functions refused re-entry, so a second Cmd+Enter during an in-flight live send (steer acknowledgement, interrupt-and-reprompt, or a resume can take seconds) emptied the box and threw the new draft away. The `sending` guard now runs first in `handleSend`, before the draft is taken out; the inner guards stay as a second line. "Send" on a session that stopped since the last poll no longer resumes it on the backend. `send_session_message_now` resumed through the claimed path, which applies no branch override and no follow-up config, so a race in the note chat pane could land the resume on the session's own branch. `route_continue_session` now takes a `StoppedSessionPolicy`: the MCP tool keeps `Resume` (unchanged behaviour), while the command's new body, `send_session_message_now_for_store`, routes with `Refuse` and answers a stopped session — found stopped up front or after the row settles under the wait — with a new `not_running` outcome, row untouched. The pane treats `not_running` by falling through to `sendMessage`, the idle path that passes the pane's `branchId` and follow-up selection. The command no longer needs the action executor, registry, or app handle; the web arm stays detached so a client disconnect between interrupt and re-prompt cannot strand the turn. The `delivery` switch is exhaustive. A `default:` arm reports `Not delivered: <kind>` and restores the draft, with the codebase's `const _: never` check so a kind added to `ContinueOutcome::delivery()` without updating the TS union fails type-check rather than silently restoring the draft. A `blocked` answer also forgets the live-delivery flag, since the connection may have gone under the request. Live-delivery polling stops on a permanent "no". `SessionLiveDelivery` gains `retryable`: true while the row is running, ours, and its connection has not handed out a handle yet (or lost it), false when another Staged instance owns the row or it is not running, and always false alongside `supported: true`. The pane keeps `LiveDeliveryKnowledge` (`supported`, `settled`) instead of a boolean, asks only while unsettled, and forgets it whenever the status leaves `running`, the pane changes session, or the pane itself starts a new run. The pure helpers live in `liveDelivery.ts`. Tests: the four Rust capability tests assert `retryable`; new tests cover `not_running` for a stopped session (and that the tool still claims it) and for a session that stops during the wait; `not_running` spelling and reason on both the outcome and the wire reply; vitest covers the ask/settle decisions of the pane helper. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Follow-up to 7c8431d. When `send_session_message_now` answers `not_running`, the chat pane falls back to `sendMessage`, the idle resume path. That arm returned straight after the call, so a resume that failed — the worktree is gone, or the queue drain re-ran the session between the `not_running` answer and `resumeSession` and the backend refused an already-running row — reported the error but lost the draft, unlike every other non-delivered arm, which puts `inputText` and `replyImageIds` back. `sendMessage` now resolves a boolean: true once the backend accepted the message (even if the pane moved to another session meanwhile), false when the guard refused or the resume threw, having set `error`. The `not_running` arm returns on true and otherwise breaks to the shared draft restore, which does not touch `error`, so the reason stays visible. The idle path in `handleSend` restores the draft on false the same way; the "Continue where you left off" button and the note follow-up ignore the result, having no draft to restore. The `not_resumable` arm is unreachable from the command (the `Refuse` policy returns before the resume gate) and is kept with a comment saying it is defensive, since the shared TS union still spells the kind. No pure helper changed, so no vitest was added; `pnpm check` and the full vitest suite pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
…ading Follow-up to 5f3e1b1. `sendMessage`'s doc comment promised that `error` says why whenever it resolves `false`, but the guard-refusal branch set nothing. The no-session case is reachable: Cmd+Enter before the session row has loaded went through `handleSend`, which restored the draft in silence. That case now sets `error` ("No session to send to yet"). The other refusal — another send already holds `sending` — stays silent, since the spinner already explains the wait, and the doc comment now states exactly that guarantee. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
…nd failures beside it Fixes scenario 8 of the manual QA of the Queue / Send composer. d5bdcb4 made `sendMessage` set `error` to "No session to send to yet" when Cmd+Enter ran before the session row had loaded. That text lives in the transcript slot behind `{#if loading}`, so it was invisible exactly while it applied, and once `loadSession` finished the stale error replaced the whole transcript with a red banner until the next send cleared it. Submit is disabled while the row loads. `ChatComposer` takes `loading`; the pane passes `loading || !session`. The submit control (and the split button's mode menu) is disabled with the same styling it uses while `sending`, titled "Loading session…", and `handleSend` returns before it takes the draft out of the box, so Cmd+Enter is a no-op that keeps the draft. Typing during load is unchanged. The decision is the pure `composerSubmitBlock` helper in `composerSendControl.ts` (loading wins over sending over an empty draft, since it is the only block the control's own label does not explain), covered by vitest. `sendMessage`'s no-session guard stays as a defensive branch returning `false`, but sets nothing: with submit disabled its other trigger is a targeted send whose pane has moved on, and a message there would be about a session the pane no longer shows. Composer-level failures move out of the transcript slot. Every failure of an action that starts at the composer — "Failed to send", "Not delivered", the `rejected_queued` text, "Failed to queue", the queued-message send and remove failures, and "Failed to cancel" — used to go into `error`, which replaces the transcript with a centered card and, via `reopenMode`, turns the next reopen into a full reload. They now go into a pane-local `sendError`, rendered by `ChatComposer` as a `role="alert"` line above the input, sharing the attachment error's style (renamed `.composer-error`). It is cleared when the next composer action starts (so a successful send dismisses it), when the draft is edited, and when the pane moves to another session. Starting an action also clears a stale load `error`, as the send paths did before, so a transcript retained across a failed reopen comes back on the next send. With that, `loadSession` is the only writer of `error` apart from the shared clear, so a successful load can no longer leave a pre-load error on screen: nothing can set one before the row has loaded. Tests: `composerSubmitBlock` for loading / sending / empty / allowed. `pnpm check` and the full vitest suite (1718 tests) pass; no Rust touched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
…sabled Closes the three leftovers from the review of 0a6bbe5 and the targeted QA re-run, all in the chat pane and its composer. A failed Stop was invisible. While a workflow runs with no transcript yet (`isPipelineRunning && grouped.length === 0`) the pipeline stop area stands in for `ChatComposer`, so a `cancelSession` failure set `sendError` on a component that was not mounted. The error line is now its own `ComposerError` component (same markup, `role="alert"`, same danger style), rendered by the composer for the attachment and send errors and by the stop area above its button, which becomes a column so the line sits over the right-aligned Stop. Stale lines. `handleCancel` and `handleDeleteQueuedMessage` now call `beginComposerAction()` after their guards, as `handleSendQueuedMessage` does, so a successful cancel or remove drops the previous failure line and a repeated identical failure remounts it (and is announced again). Attaching, dropping, or removing an image is treated as a draft edit and clears `sendError` alongside `attachError`. Disabled title after a failed load. The composer's `loading` prop was the pane's `loading || !session`, so once `getSession` threw or answered "Session not found" the control stayed disabled titled "Loading session…" while the transcript card said the load had failed. The prop is now `sessionBlock: 'loading' | 'unavailable' | null`, computed by the pure `composerSessionBlock` (loading wins while the fetch is in flight, since a reopen keeps the row null until it settles; a settled fetch with no row is "unavailable"). `composerSubmitBlock` takes it in place of the boolean and `composerSubmitBlockTitle` gives "Loading session…" or "Session unavailable", leaving the Send / Queue title alone for blocks the control already explains. `handleSend` returns on any session block, as before. Left alone, as requested: the one-poll-tick "Queue" flicker after a failed `not_running` fallback, which is correct in the real race. Tests: `composerSessionBlock` for loading / unavailable / loaded, `composerSubmitBlock` for the new input, and the block titles. `pnpm check` and the full vitest suite (1724 tests) pass; no Rust touched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
…it accent
The accent split button's two segments were separated by a 2 CSS px seam
(4 device px at 2x) of composer background, against the 1 CSS px border the
Rebase and Create PR outline pairs show. `buttonVariants` paints every
button with a 1px transparent border and `bg-clip-padding`, so the
primary's transparent right border and the trigger's left border sat side
by side, both compositing over `--bg-chrome`. The "35% white hairline" was
therefore `#fafafa` beside `#f7f7f7` (light) and `#1e1a23` beside `#18151d`
(dark): a crack of page colour, not a tint.
Two tokens change in `ChatComposer.svelte`, as recommended in the seam
note. The `DropdownMenu.Trigger` gains `-ml-px`, the same overlap
`BranchCardHeaderInfo.svelte` and `BranchCardPrButton.svelte` put on their
trigger wrapper, so the trigger's left border covers the primary's
transparent right border and one border column remains. The scoped
`border-left-color` mixes `--bg-primary` into `var(--ui-accent)` instead of
`transparent`, so that remaining column is an opaque tint of the fill
(`#6fb2ff` light, `#4777b6` dark) rather than a wash over the chrome
background. The comment on the rule now explains the two-column problem
and why the mix target is the accent colour. The control is 1 CSS px
narrower (91.3 px against 92.3 px); the fills are unchanged.
Measured with a Playwright + Chrome fixture using this branch's built
Tailwind CSS (the earlier `/tmp/split-seam` harness was gone), scanning
the row a quarter of the way down the control, clear of the label and
chevron. The before numbers reproduce the note's exactly.
before: 2 CSS px at 1x, 4 device px at 2x; #f7f7f7 + #fafafa (light),
#18151d + #1e1a23 (dark)
after: 1 CSS px at 1x, 2 device px at 2x; #6fb2ff (light),
#4777b6 (dark); primary.right - trigger.left = 1
`-ml-px` is present in the built CSS. `pnpm check` and the full vitest
suite (1724 tests) pass.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…border-box height Follow-up to 59308ae. The accent hairline between the split button's two segments poked past the blue fills at the top and bottom. Every button paints a 1px transparent border with bg-clip-padding, so each fill is inset 1px from its border box, while the trigger's tinted border-left spanned the whole border box and mitred into the transparent top and bottom borders. Measured on the saved real-app 2x captures (light and dark) and the fixture harness, scanning the two seam columns against the fill columns 4 CSS px to either side: before: outer seam column 1 device px past the fills at each end, plus a half-tone pixel beyond that (the mitre); inner column flush with a half-tone pixel beyond. 1x: flush, with a half-tone pixel at each end. after: 0 device px overshoot top and bottom at 1x and 2x, light and dark; the rows beyond the fills are plain background. The trigger keeps -ml-px but drops its left border, so its fill paints over the primary's transparent right-border column and the two fills meet with no seam. The divider is a 1px absolutely positioned ::before with top and bottom 0: its containing block is the trigger's padding box, which is exactly the fill's extent (a 1px inset there would notch the line by 1 CSS px at each end, which the harness confirmed). The colour is the same opaque accent tint as before. The comment on the rule now describes this mechanism. Also evaluated and rejected: an inset box-shadow through Tailwind's --tw-inset-shadow variable measures identically and composes with the focus ring, but couples the divider to a private variable and to some box-shadow utility being present at rest; a 1px background-image strip also measures identically but reads less plainly and rounds the dark tint by one unit. Verified: seam 1 CSS px (2 device px at 2x), same x as before; primary and trigger fill extents and the control width (83.06 px typed) unchanged; keyboard focus ring on the trigger renders as before (screenshot). Also captured once in the running app through the /tmp/staged-live scaffold (light, 2x, prebuilt patched binary, new dist): seam columns rows 3..70 against fill rows 3..70, where the previous capture had 2..71. pnpm check and the full vitest suite (1724 tests) pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8c2575a69
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+488
to
+490
| export function setComposerSendMode(mode: ComposerSendMode): void { | ||
| preferences.composerSendMode = mode; | ||
| setStoreValue(COMPOSER_SEND_MODE_STORE_KEY, mode); |
There was a problem hiding this comment.
When multiple Staged windows are open, changing this supposedly global mode updates only the current window’s Svelte state and persisted store; unlike project-status preferences, there is no listenToStoreKeyChange subscription to update the other window. A peer window can therefore retain send after the user selected queue elsewhere and unexpectedly deliver into or interrupt a running turn (or retain queue and defer a message), so store-change events should normalize and apply this preference in every client.
Useful? React with 👍 / 👎.
…dows Addresses the PR #1012 review comment on `setComposerSendMode`. The mode is one global preference, but only the window that picked it updated: `project-status-options` subscribes to `store://change`, the composer mode did not. A second window kept its old mode, so a stale `send` could deliver into (or interrupt) a running turn the user had switched to queueing behind, and a stale `queue` could defer a message meant to land now. The backend already forwards every store key's change to Tauri windows and web tabs, so this is frontend-only. The subscribe-and-refresh machinery the project statuses had is now the `syncedPreference(key, apply)` helper in preferences.svelte.ts, used by both keys: one `store://change` listener per key, a read on registration and on every web reconnect (`onEstablished`), and a revision counter so a notification or local edit wins over an older read still in flight. `apply` normalizes, so a deleted or malformed mode falls back to `queue` in every client, as it already did on load. Receiving an event never writes back, so there is no echo loop. The revision guard also closes a startup race the composer mode had on its own: the mode is read at the end of `initPreferences`, after diff theme loading, so a mode picked in that window was overwritten by the late read. Project statuses behave as before. Tests, native and web clients: a mode picked in one client reaches the other with no echo write; deletion and malformed values fall back to queue; a reconnect refresh recovers a missed change; a mode picked while the initial read is in flight survives it. The listener-count assertion is now one per synced key. The three sync tests fail against the old code. `pnpm check` and the full vitest suite (1732 tests) pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The session and note chat composers could only queue a follow-up while a session was running. This PR adds a Send now path next to queueing. It uses the same delivery route that project notes already get through the
continue_repo_sessionMCP tool.Backend (
session_commands.rs,web_server.rs)get_session_live_deliveryreturns{ supported, reason, retryable }. It applies the same check asroute_continue_session: the row isrunning, this Staged process owns it, and its ACP connection has handed out a live turn-control handle. It doesn't wait for startup, so a session that is still connecting reports "queue only" (retryable).send_session_message_nowsteers the message into the turn in progress, or interrupts and re-prompts if steering isn't possible. Its reply uses the samedelivery/reasonwording as the MCP tool. It routes with a newStoppedSessionPolicy::Refuse, so a session that stopped since the last poll getsnot_runningand its row is left alone. The MCP tool keepsResume. The web handler runs detached, so a client disconnect can't strand a turn halfway through an interrupt and re-prompt.Frontend
composerSendControldecides what the submit button shows:composer-send-mode(defaultqueue).liveDelivery.ts: the pane re-checks live delivery on each 500 ms poll until it gets a final answer. It clears that answer when the status leavesrunningor the pane switches session.blocked,rejected_queuedornot_resumable, the draft is put back in the box and the reason is shown. It is not quietly re-queued, because a steer the agent never acknowledged may already have landed. Anot_runningreply falls back to the normal idle resume path, which applies the pane's branch and follow-up settings; if that resume fails, the draft is restored too.ComposerErrorline next to the composer instead of replacing the transcript, and a failed Stop is visible even when the pipeline stop area is shown in place of the composer.Tests
retryable);not_runningfor a session that was already stopped and for one that stops during the wait; the reply format.composerSendControl(single vs split button, submit-block reasons and titles),liveDelivery(when to ask and when the answer is final), and saving/loading the send-mode preference, including the default and a malformed value.🤖 Generated with Claude Code