feat: implement ExitPlanMode HITL for Tool Permission Model#1589
Open
quay-devel wants to merge 5 commits into
Open
feat: implement ExitPlanMode HITL for Tool Permission Model#1589quay-devel wants to merge 5 commits into
quay-devel wants to merge 5 commits into
Conversation
Add ExitPlanMode as a human-in-the-loop tool alongside AskUserQuestion, enabling plan approval workflows in ACP sessions. This implements the spec from PR ambient-code#1586 (closes ambient-code#1583). Runner: - Add ExitPlanMode to BUILTIN_FRONTEND_TOOLS halt set - Enrich ExitPlanMode tool args with plan file content from .claude/plans/ - Complete Tier 1 tool allowlist (NotebookEdit, WebFetch, TodoWrite, etc.) Backend: - Generalize isAskUserQuestionToolCall → isHITLToolCall to detect both AskUserQuestion and ExitPlanMode for status derivation and compaction - Add ExitPlanMode test cases for waiting_input detection Frontend: - Generalize HITL detection in use-agent-status and stream-message - Add ExitPlanModeMessage component with approve/reject/request-changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Extract shared hitl-tools.ts with normalizeToolName, isHITLTool, isAskUserQuestionTool, isExitPlanModeTool, and hasToolResult helpers - Remove duplicated hasResult and tool detection functions from ask-user-question.tsx, exit-plan-mode.tsx, stream-message.tsx, and use-agent-status.ts - Add 100KB size guard to _read_plan_file to prevent oversized events - Log JSON errors during ExitPlanMode plan enrichment instead of silently swallowing them - Use stable composite key for allowedPrompts list rendering Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
✅ Deploy Preview for cheerful-kitten-f556a0 canceled.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds unified HITL (AskUserQuestion + ExitPlanMode) detection and handling across backend, frontend, and runner: backend generalizes HITL detection and preserves state, frontend centralizes helpers, updates status logic, and adds ExitPlanMode UI, and the runner enriches ExitPlanMode calls and expands allowed tools. ChangesHITL Tool Support
Sequence DiagramsequenceDiagram
participant Claude as Claude (AI)
participant Adapter as Adapter
participant Backend as Backend (WebSocket)
participant Frontend as Frontend
participant User as User
Claude->>Adapter: Invoke ExitPlanMode (tool call)
Adapter->>Adapter: Read latest .claude/plans/*.md
Adapter->>Adapter: Inject planContent into arguments
Adapter->>Backend: Emit TOOL_CALL_START (ExitPlanMode)
Backend->>Backend: isHITLToolCall() matches ExitPlanMode
Backend->>Frontend: Stream event: TOOL_CALL_START / waiting_input
Frontend->>Frontend: useAgentStatus() detects HITL tool
Frontend->>Frontend: stream-message checks isExitPlanModeTool()
Frontend->>Frontend: Render ExitPlanModeMessage with plan content
Frontend->>User: Display "Plan Review" UI (approve/reject/changes)
User->>Frontend: Click approve/reject + optional feedback
Frontend->>Backend: Submit tool result via onSubmitAnswer
Backend->>Backend: Snapshot TOOL_CALL_START to preserve waiting_input
Backend->>Claude: Resume execution with user result
🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/frontend/src/components/session/exit-plan-mode.tsx`:
- Around line 35-36: The variables planContent and allowedPrompts currently use
TypeScript assertions only; add runtime guards so planContent is set to
input.planContent if typeof input.planContent === "string" otherwise "" and set
allowedPrompts to Array.isArray(input.allowedPrompts) ? input.allowedPrompts as
AllowedPrompt[] : [] (or validate each element) before using ReactMarkdown and
.map; update the initialization of planContent and allowedPrompts in
exit-plan-mode.tsx to perform these checks so ReactMarkdown always gets a string
and .map runs on a real array.
In `@components/runners/ambient-runner/ag_ui_claude_sdk/adapter.py`:
- Around line 105-108: The truncation checks byte length but slices by character
count, which can exceed _PLAN_FILE_MAX_BYTES for multi-byte UTF-8 chars; fix by
performing the truncation in bytes: read the file text into content, encode to
bytes (e.g., content_bytes = content.encode("utf-8")), if len(content_bytes) >
_PLAN_FILE_MAX_BYTES then slice the bytes to _PLAN_FILE_MAX_BYTES, decode back
to a string with a safe error handler (e.g., errors="ignore" or "replace") and
append the "\n\n[truncated]" marker before returning; update the logic around
plan_files, content, and _PLAN_FILE_MAX_BYTES accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 767b3627-8743-45b6-a69e-396e31eb5da9
📒 Files selected for processing (10)
components/backend/websocket/agui_proxy.gocomponents/backend/websocket/agui_store.gocomponents/backend/websocket/agui_store_test.gocomponents/frontend/src/components/session/ask-user-question.tsxcomponents/frontend/src/components/session/exit-plan-mode.tsxcomponents/frontend/src/components/ui/stream-message.tsxcomponents/frontend/src/hooks/use-agent-status.tscomponents/frontend/src/lib/hitl-tools.tscomponents/runners/ambient-runner/ag_ui_claude_sdk/adapter.pycomponents/runners/ambient-runner/ambient_runner/bridges/claude/mcp.py
- Add runtime type guards for planContent (typeof string) and allowedPrompts (Array.isArray) in ExitPlanModeMessage to prevent runtime errors from unexpected backend data - Fix byte-accurate truncation in _read_plan_file: slice encoded bytes instead of character count to respect the 100KB limit for multi-byte UTF-8 content Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.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
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.
Summary
Implements the Tool Permission Model spec from PR #1586, adding
ExitPlanModeas a HITL (human-in-the-loop) tool that halts the event stream and waits for user approval — the same mechanism already used byAskUserQuestion.ExitPlanModeadded toBUILTIN_FRONTEND_TOOLShalt set; plan file content injected into tool args; Tier 1 allowlist completed with all missing toolsisAskUserQuestionToolCall→isHITLToolCallto detect both tools for status derivation and snapshot compaction; new test cases for ExitPlanModehitl-tools.ts; newExitPlanModeMessagecomponent with approve/reject/request-changes actionsCloses #1583
Spec: #1586
Test plan
go test ./websocket/...)npm run build)npx vitest run)panic()in Go code, noanytypes in TypeScript🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements