feat(agent-chat): warm Stop + approval dock polish (steer UI flag-gated)#5477
Open
ardaerzin wants to merge 4 commits into
Open
feat(agent-chat): warm Stop + approval dock polish (steer UI flag-gated)#5477ardaerzin wants to merge 4 commits into
ardaerzin wants to merge 4 commits into
Conversation
…killing or leaking Today "Stop" either just aborts the client stream (the runner keeps running and billing) or, behind a flag, kills the whole session. Neither cancels the turn cleanly. This adds a cooperative cancel: Runner: on a user Stop the control-plane `cancel` command drops the alive lock and the heartbeat aborts the run signal. run-turn now races that abort to a distinct `stopReason:"cancelled"` (instead of falling through to the error catch), settles every still-open tool call with a new INTERRUPTED_BY_USER sentinel — no fake success, no orphaned "running" part — and treats continuity like a pause (invalidate, don't advance the ledger). Keep-warm-on-cancel is deferred to v2 (cold replay resumes fine and it avoids the unverified Claude-ACP warm-cancel question); the session stays open, so a follow-up prompt cold-resumes. FE: handleStop sends the `cancel` command by default (turn stops, session stays open) instead of leaving the runner running; the hard kill stays opt-in behind NEXT_PUBLIC_AGENT_CHAT_STOP_KILLS_SESSION. Runner tsc clean; 78 files / 1245 unit tests pass (incl. a new cancel test).
Adds "reject-with-message" from the approval proposal: a denial can carry an instruction that reruns as the next turn, so the agent hears "no — do this instead" rather than a bare refusal (the redirect case, e.g. "write to staging, not prod"). Pure FE, reusing tested paths — no runner/wire change: the ApprovalDock gets a subtle "Redirect" popover (progressive disclosure — the instruction field only appears on click; bare Deny stays one-click) that answers the gate as denied AND carries the note. handleApprovalResponse then denies via addToolApprovalResponse and sends the note through the existing queue `submit`, which holds it until the paused turn settles and drives the next turn. (v2: fuse the note into the denial itself as model-facing guidance rather than a follow-up turn.) tsc/prettier/eslint clean.
Approval dock: inline Redirect affordance (replaces the body-portal popover that lingered across sessions), a single primary action, a subtle filled redirect field with a neutral hover/focus border, and the action row / always-allow toggle / redirect panel animate as one coordinated collapse-expand swap; the field auto-focuses on open. Steer (deny + redirect) is UI-complete but gated OFF behind NEXT_PUBLIC_AGENT_CHAT_STEER: the redirect can only run as a follow-up turn today (the harness continues the original prompt on reject and exposes no reject-with-feedback channel), so the control is hidden until the runner-level reject-and-redirect lands. The implementation ships intact behind the flag.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe runner now treats user stops as cancelled turns, settles open tool calls with an interruption sentinel, and avoids cancelled ledger updates. Agent Chat sends cooperative cancellations and supports feature-gated approval redirection. Playground configuration spacing is tightened. ChangesTurn cancellation and approval steering
Playground spacing adjustment
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant AgentConversation
participant Runner
participant ToolCalls
User->>AgentConversation: select Stop
AgentConversation->>Runner: commandSessionStream cancellation
Runner->>ToolCalls: settle open calls as INTERRUPTED_BY_USER
Runner-->>AgentConversation: cancelled turn
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ardaerzin
marked this pull request as ready for review
July 24, 2026 10:06
Contributor
Author
|
@coderabbitai review |
dosubot
Bot
added
size:L
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b11f23f-32df-434d-adac-ad2cfc7b841b
📒 Files selected for processing (8)
services/runner/src/engines/sandbox_agent/run-turn.tsservices/runner/src/tracing/otel.tsservices/runner/tests/unit/session-keepalive-approval.test.tsweb/oss/src/components/AgentChatSlice/AgentConversation.tsxweb/oss/src/components/AgentChatSlice/assets/constants.tsweb/oss/src/components/AgentChatSlice/components/ApprovalDock.tsxweb/oss/src/lib/helpers/dynamicEnv.tsweb/packages/agenta-entity-ui/src/DrillInView/components/PlaygroundConfigSection.tsx
Comment on lines
+1861
to
+1863
| // pt-0 (not py-3) tightens the gap between the region header and the first section | ||
| // row; pb-3 keeps the bottom breathing room. The loading fallback below matches. | ||
| return <div className="px-4 pb-3 pt-1">{props.defaultRender()}</div> | ||
| return <div className="px-4 pb-3 pt-0">{props.defaultRender()}</div> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the loading fallback spacing synchronized.
The agent content now uses pt-0, but the loading fallback at Lines [1898-1901] still uses pt-1 despite claiming to mirror this wrapper. This causes the content to shift vertically when loading completes. Update the fallback to pt-0 as well.
Contributor
Railway Preview Environment
|
ardaerzin
added a commit
that referenced
this pull request
Jul 24, 2026
Stop is verified warm (§5 acceptance passes). Steer's 'message as model feedback' (§3) is NOT reachable pure-FE — both harnesses continue the original turn on reject and the ACP reply carries no text; the redirect needs runner path A or ACP-chain patch C. UI shipped flag-gated (#5477). Batch surfacing is a small runner change (#5470), not blocked by #5391.
ardaerzin
added a commit
that referenced
this pull request
Jul 24, 2026
…/Stop/steer) (#5444) * docs(agent-workflows): reject-siblings approval policy proposal (Deny/Stop/steer) * docs(agent-workflows): add task-5 implementation findings Stop is verified warm (§5 acceptance passes). Steer's 'message as model feedback' (§3) is NOT reachable pure-FE — both harnesses continue the original turn on reject and the ACP reply carries no text; the redirect needs runner path A or ACP-chain patch C. UI shipped flag-gated (#5477). Batch surfacing is a small runner change (#5470), not blocked by #5391.
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
1 participant
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.
What
cancelcommand) instead of only aborting the client stream — the turn ends cleanly and the session stays open for a follow-up prompt. Open tool calls settle with a newINTERRUPTED_BY_USERsentinel; continuity treats a cancel like a pause (invalidate, don't advance the ledger). Hard-kill stays opt-in behindNEXT_PUBLIC_AGENT_CHAT_STOP_KILLS_SESSION.NEXT_PUBLIC_AGENT_CHAT_STEER(see below).pt-0).Why steer is gated (not shipped live)
On both harnesses (Claude + pi), denying a gate makes the model continue the original turn and react to a bare "permission denied" before the user's redirect lands — the ACP permission reply is a closed outcome (
selected/cancelled) with no text field, and the harness continues on reject (claude-agent-acp returns{behavior:"deny", message:"User refused permission"}and keeps going). So a pure-FE steer can only send the note as a follow-up turn, and the model flails on the bare denial first (retrying the blocked action → a new gate that traps the note, or reasoning around it).The clean fix is runner-level (reject-and-redirect) or a per-harness ACP-chain patch to carry the deny
message. Until then the control is hidden; the implementation ships intact behind the flag. Design, evidence, and the A-vs-C options: #5444.Test plan
services/runner/tests/unit/session-keepalive-approval.test.ts).NEXT_PUBLIC_AGENT_CHAT_STEER=true): Redirect opens inline, coordinated animation, auto-focus, neutral border; switch sessions → no lingering popover.