feat: implement ExitPlanMode HITL for Tool Permission Model#1589
feat: implement ExitPlanMode HITL for Tool Permission Model#1589quay-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 <[email protected]>
- 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 <[email protected]>
✅ Deploy Preview for cheerful-kitten-f556a0 canceled.
|
|
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}
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 <[email protected]>
Claude Code ReviewSummaryClean, well-scoped implementation that correctly generalizes AskUserQuestion's HITL machinery to cover ExitPlanMode. The DRY refactor — extracting FindingsBlockerNone. CriticalNone. MajorNone. Minor1. Exact string match in adapter.py inconsistent with normalized approach everywhere else if current_tool_display_name == "ExitPlanMode":…uses an exact case-sensitive comparison while the backend ( def _normalize_tool_name(name: str) -> str:
return "".join(c for c in name.lower() if c.isalpha())
if _normalize_tool_name(current_tool_display_name) == "exitplanmode":2. 3. Removed comment in 4. Nit
Positive Highlights
Recommendations
Amber · standards loaded from CLAUDE.md, specs/standards/backend/, specs/standards/frontend/, specs/standards/security/ |
2 participants
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