feat(monitor): mid-flight steering — inject user messages into a running agent #227
No reviewers
Labels
No labels
area:agents
area:dashboard
area:database
area:design
area:design-review
area:flows
area:infra
area:meta
area:security
area:sessions
area:webhook
area:workdir
security
type:bug
type:chore
type:meta
type:user-story
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
charles/claude-hooks!227
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "boss/224"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Closes #224.
agent-runnerfrom one-shotquery({ prompt: string })to the Claude Agent SDK's streaming-input form (query({ prompt: AsyncIterable<SDKUserMessage> })) so operator messages can flow into a live conversation at the next turn boundary. Session-chain + resume semantics are unchanged.apps/server/src/steer.tsowns the per-taskSteerChannelregistry — bounded FIFO (max 10, overflow drops oldest), 6 msgs/min/task rate limit, 8 KB size cap, and a hard refusal forforeman(its own chat surface at/foreman/chat).POST /task/:id/steer(auth-gated, M18-8). Maps outcomes to202 queued / 409 conflict / 413 too large / 429 rate-limited / 400 foreman refused.steer_queued+steer_deliveredSSE envelopes on/events; shared type added asSseSteerEvent. Transcript events persist in the task history so a reload keeps them visible.steer_deliveredevent lands. Cancel stays a separate, terminal action.--color-role-operator(new token, alias of--ch-color-warning) and an "operator · queued/delivered" label so human turns are visually distinct from agent output.Test plan
just qa— typecheck, lint, format, full test suite (server 830 pass, web 187 pass).SteerChannel(FIFO, overflow-drops-oldest, close-then-drain, onDelivered hook) andqueueSteer(size cap, rate limit, foreman refusal, queued happy path, dropped-oldest warning bit).agent-runner.test.ts— posted steer message lands in the next iterator yield.POST /task/:id/steerroute tests cover 400 (bad body / foreman), 409 (unregistered task), 413 (oversize), 429 (rate-limit exhausted), and the 202 happy path withonQueuedcallback invocation.Switches `agent-runner` from one-shot `query({ prompt: string })` to the Claude Agent SDK's streaming-input form so operator messages can flow into a live conversation at the next turn boundary. A new per-task `SteerChannel` (bounded FIFO, rate-limited, foreman-refused) sits between the HTTP `POST /task/:id/steer` route and the SDK iterator. The Monitor task-detail page gains an Interrupt + Steer composer while the task is running; operator-injected messages render with a dedicated `--color-role-operator` accent so the transcript distinguishes human turns from agent output. Cancel stays a separate, terminal action. Closes #224 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>CI still pending at review time (run #1870, sha
9667a9c). Stepping off the review request — will be re-dispatched automatically when CI completes.Review — mid-flight steering (#224)
CI: ✅ green (run #1870, 4 m 3 s).
All acceptance criteria are met. Architecture is clean:
SteerChannel/ registry in a dedicated module, streaming-input iterator wraps the initial prompt + channel, session-resume retry correctly re-registers a fresh channel on the second attempt. Test coverage is thorough — SteerChannel semantics,queueSteerpolicy, iterator contract, and HTTP route surface. Two issues below.Issue 1 — Hardcoded
6in rate-limit error message (maintenance bug)File:
apps/server/src/main.ts—handleTaskSteer, therate_limitedbranch of the switch:STEER_RATE_LIMITis not in the import from./steer(onlyMAX_STEER_BYTESandqueueSteerare). The literal${6}will silently drift if the constant is ever changed.Fix: add
STEER_RATE_LIMITto the import and use it in the template literal:Issue 2 — Empty message maps to misleading 409 Conflict
File:
apps/server/src/steer.ts—queueSteer:The route pre-validates
typeof body.message !== "string"(→ 400) but notbody.message.length === 0, so an empty string passes the route guard and reachesqueueSteer, which returnsoutcome: "conflict". The caller then emits409 Conflictwith the body"task has already settled or is not registered"— a misleading message that tells the API operator the task settled when the real problem is the empty payload.The
SteerComposerUI pre-checkstrimmed.length === 0before callingmutate, so this path isn't reachable from the UI. But it's reachable viacurl/ any direct API client and produces confusing diagnostics.Fix: add the empty-string guard at the route level, alongside the size check that already lives there:
This keeps the route as the authoritative input validator and avoids co-opting the
conflictoutcome for a logically unrelated case. The existingqueueSteerempty-string path can stay as a belt-and-suspenders guard but the route catches it first.Both issues are small fixes. Happy to approve once they're addressed.
9667a9c59060d59c7c2a60d59c7c2ad0870d7b4bRound 2 review — mid-flight steering (#224)
CI: ✅ green (run #1879, 4m16s).
Neither of the two findings from round 1 has been addressed in the new commit (
d0870d7). Re-raising both below — same fix guidance as before.Issue 1 — Hardcoded
6in rate-limit error message (still unresolved)File:
apps/server/src/main.ts—handleTaskSteer,rate_limitedbranch:The import is still
import { MAX_STEER_BYTES, queueSteer } from "./steer"—STEER_RATE_LIMITis not imported. The literal${6}will silently drift if the constant is ever changed.Fix: add
STEER_RATE_LIMITto the import and use it in the template:Issue 2 — Empty message maps to misleading 409 Conflict (still unresolved)
File:
apps/server/src/steer.ts—queueSteer, empty-string branch:File:
apps/server/src/main.ts—handleTaskSteer, route-level guard:The route guard only rejects non-strings. An empty string passes through to
queueSteer, which returnsoutcome: "conflict"→ 409 with the body"task has already settled or is not registered"— still a misleading diagnostic for an empty-payload mistake.Fix: add the empty-string check at the route level, right after the
typeofguard:The
queueSteerbelt-and-suspenders check can stay; the route just needs to catch this first so the HTTP response is accurate.Two small fixes; happy to approve immediately once they're in.
Both round-2 findings addressed in
afb29e3:STEER_RATE_LIMITis now imported alongsideMAX_STEER_BYTESandqueueSteerinapps/server/src/main.ts, and the 429 response template uses the constant (no more${6}literal).handleTaskSteernow short-circuits empty-stringmessageat the route with400 { error: "message must be non-empty" }, beforequeueSteerever sees it. The belt-and-suspenders guard inqueueSteerstays put.Regression tests: new
main.test.tscase asserts the empty-body 400, and the existing 429 test now checksbody.errorcontains the liveSTEER_RATE_LIMITvalue so future drift trips the test.just qaclean (835 server pass, was 830).Round 3 review — mid-flight steering (#224)
CI: ✅ green (run #1885, 4m14s).
Both findings from round 2 are resolved in commit
afb29e3:6) —STEER_RATE_LIMITis now in the import from./steerand the template literal inhandleTaskSteer'srate_limitedbranch uses it correctly. ✅handleTaskSteer:if (body.message.length === 0)returns400 { error: "message must be non-empty" }beforequeueSteeris ever reached. ✅All acceptance criteria met, CI green, no outstanding issues. Approved.