fix(grok): show Grok plan-mode plans and handle plan approvals over ACP#4233
fix(grok): show Grok plan-mode plans and handle plan approvals over ACP#4233Emmanuek5 wants to merge 5 commits into
Conversation
When Grok finishes planning it sends a reverse request _x.ai/exit_plan_mode to the ACP client. T3 Code had no handler, so the request failed method-not-found and Grok reported "Plan approval could not be completed because the client disconnected" - the plan never displayed and plan mode stayed stuck. GrokAdapter now registers handlers for x.ai/exit_plan_mode and _x.ai/exit_plan_mode: the first presentation is captured as a turn.proposed.completed event (same plan-card flow as Claude/Cursor) with a revise-and-wait response so Grok ends its turn; once a plan is captured, the next non-plan turn (the implementation turn) answers "approved" so Grok exits plan mode and builds. The adapter also tracks current_mode_update and nudges Grok into plan mode when a turn arrives with interactionMode "plan" while the session is not in plan mode. grok-4.5 sometimes batches the plan-file write and exit_plan_mode in a single response; the CLI then races its own plan read and planContent arrives empty. The adapter remembers the plan-file path from the enter_plan_mode tool output and recovers the plan by polling the file. Verified end to end against the real grok CLI (plan captured on turn 1, approved + implemented on turn 2, current_mode_update plan -> default).
github-actions
Bot
added
size:L
![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: b1798b070f
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (ctx?.planCaptured === true && ctx.activeInteractionMode !== "plan") { | ||
| ctx.planCaptured = false; | ||
| return makeXAiExitPlanModeApprovedResponse(); |
There was a problem hiding this comment.
When Grok enters plan mode during a normal (default) turn, the first exit_plan_mode request sets planCaptured, but activeInteractionMode remains default. If Grok issues another exit_plan_mode request in that same prompt—for example after processing the rejection feedback and revising the plan—this condition immediately returns approved, allowing implementation before the user has reviewed or accepted the proposed-plan card. Track the turn that captured the plan (or a later user submission) rather than using only the interaction-mode value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 28fd2a6 and refined in 0e7464e: plan captures now record a monotonic prompt serial, and approval requires at least one new non-plan user prompt after the capture. A plan re-presented by Grok within the same prompt is captured again, never auto-approved. Regression tests: same-prompt double presentation (both captured) plus unit coverage of shouldAutoApproveGrokPlan.
There was a problem hiding this comment.
Pull request overview
Fixes Grok “plan mode” integration over ACP by handling the reverse _x.ai/exit_plan_mode extension request, surfacing the plan as a proposed-plan card in the UI, and correctly responding with approval on the subsequent implementation turn.
Changes:
- Add ACP extension schema/helpers for
exit_plan_mode(plain + wrapped) plus response builders. - Update
GrokAdapterto capture plans viaturn.proposed.completed, track plan/interaction mode state, and recover missingplanContentby reading the plan file path fromenter_plan_mode. - Extend the ACP mock agent and add tests covering capture → refine → approve and the empty-
planContentrecovery path.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/server/src/provider/Layers/GrokAdapter.ts | Registers/handles Grok exit_plan_mode, tracks plan-mode state, and recovers plans from disk when needed. |
| apps/server/src/provider/Layers/GrokAdapter.test.ts | Adds adapter-level tests for plan capture/refine/approve sequencing and plan-file recovery. |
| apps/server/src/provider/acp/XAiAcpExtension.ts | Introduces schema + helpers for Grok’s exit_plan_mode reverse request and response builders. |
| apps/server/src/provider/acp/XAiAcpExtension.test.ts | Adds unit tests for exit_plan_mode decoding, plan extraction, and response shapes. |
| apps/server/scripts/acp-mock-agent.ts | Adds mock emission of _x.ai/exit_plan_mode (including empty planContent + plan-file-path race simulation). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Grok interprets the response outcome as: `approved` exits plan mode and | ||
| * tells the agent to implement the plan; `abandoned` keeps plan mode active | ||
| * and tells the agent the user does not want to exit; any other outcome is | ||
| * treated as "revise" — the tool completes with "The user wants to revise the | ||
| * plan." plus the feedback text, and plan mode stays active. | ||
| */ |
There was a problem hiding this comment.
Fixed in 28fd2a6 - the doc now states that abandoned exists in Grok's protocol but is never sent by T3 Code, so the type models only the produced outcomes.
|
|
||
| it("extracts the plan from wrapped _x.ai exit_plan_mode payloads", () => { | ||
| const decoded = decodeXAiExitPlanModeRequest({ | ||
| method: "x.ai/exit_plan_mode", |
There was a problem hiding this comment.
Fixed in 28fd2a6 - the wrapped-payload test now uses the _x.ai/exit_plan_mode method.
| // Re-read per attempt: the enter_plan_mode update that | ||
| // carries the path is processed on a concurrent fiber. | ||
| const planFilePath = ctx.planFilePath; | ||
| if (planFilePath) { |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review This PR introduces significant new Grok plan-mode handling capability (620+ new lines), not a simple fix. Additionally, there's an unresolved P1 review comment identifying a potential bug where plans could be auto-approved before user review. Both the scope and the open correctness concern warrant human review. You can customize Macroscope's approvability policy. Learn more. |
- Only auto-approve a captured plan when exit_plan_mode arrives on a later non-plan turn: planCapture records the capturing turn id, so a plan re-presented within the same turn (e.g. right after revise feedback) is captured again instead of approved on the user's behalf (Codex P1 / Bugbot high). - Guard plan-file recovery against stale reads: contents matching the previously captured plan are treated as a rewrite in flight and only used as a fallback after polling (Bugbot medium). - Stop waiting early when no plan-file path is known instead of holding the exit_plan_mode response for the full poll window (Bugbot medium). - Validate the agent-supplied plan_file_path (absolute, NUL-free) before reading it from disk (Copilot). - Clarify the exit_plan_mode response doc (abandoned exists in Grok's protocol but is never sent) and use the _x.ai method in the wrapped payload test (Copilot). - Run the plan-mode adapter tests with TestClock.withLive: the recovery loop sleeps on the live clock, matching the other prompt-flow tests. Re-verified end to end against the real grok CLI, including the batched write+exit_plan_mode race (plan recovered from the plan file, captured once, approved on the implementation turn, file edited).
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
A bare leading backslash is a relative path on POSIX, so treating it as absolute let a malformed agent-supplied plan_file_path resolve against the server working directory. Reject it.
| if ( | ||
| planFilePath.length > 0 && | ||
| !planFilePath.includes("\0") && | ||
| /^(?:[A-Za-z]:[\\/]|\/)/.test(planFilePath) |
There was a problem hiding this comment.
Windows absolute paths rejected
Low Severity
The absolute-path check now requires a drive letter or a leading /, so Windows UNC paths and current-drive roots that start with \ are dropped. When planContent is empty, plan-file recovery never runs and the UI falls back to a placeholder plan.
Reviewed by Cursor Bugbot for commit f392222. Configure here.
There was a problem hiding this comment.
Deliberate trade-off after the sibling Macroscope finding (backslash roots are relative on POSIX): Grok stores plan files under the user's home directory on a lettered drive, so UNC/current-drive-relative roots do not occur in practice, and this path only gates the empty-planContent recovery fallback - rejecting it degrades to the placeholder card, never breaks approval. Happy to add platform-aware UNC support if maintainers prefer.
Gating approval on a different turn id broke the implement click that steers into the still-running capturing turn (steers reuse the active turn id), leaving Grok stuck in plan mode. Track a monotonic prompt serial instead: approval requires a plan captured earlier plus at least one new non-plan user prompt since the capture - a new turn and a steer both qualify, while Grok re-presenting on its own within the same prompt still gets captured. Decision extracted as shouldAutoApproveGrokPlan with unit coverage.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0e7464e. Configure here.
Two follow-up Bugbot findings on the prompt-serial gate: - promptSerial now increments only after prompt preparation succeeds, right before the RPC dispatches. A prompt that failed validation or was interrupted before reaching Grok no longer counts as the fresh user prompt that unlocks plan auto-approval. - The exit_plan_mode handler snapshots the serial at request entry and records that value on capture, so an implement steer landing while the capture is being processed (e.g. during plan-file recovery polling) still counts as arriving after the capture and can approve.
Problem
Grok's plan mode was broken in T3 Code. When the Grok CLI finishes planning it intercepts its
exit_plan_modetool and sends a reverse request_x.ai/exit_plan_mode({sessionId, toolCallId, planContent}) to the ACP client for approval. T3 Code had no handler for it, so the request failed with method-not-found, which Grok surfaces as:The plan never displayed in the UI and the Grok session stayed stuck in plan mode.
Wire protocol (discovered by live-probing
grok agent stdio){outcome: "approved"}→ Grok exits plan mode (current_mode_update→default) and implements the plan."rejected") with afeedbackstring → "The user wants to revise the plan." + feedback; the turn ends and plan mode stays active._x.ai/toggle_plan_modeis rejected). Plan mode is only entered via the agent's ownenter_plan_modetool.Fix
XAiAcpExtension.ts: schema for the request (plain + wrapped forms) and response builders.GrokAdapter.ts:x.ai/exit_plan_mode/_x.ai/exit_plan_mode.turn.proposed.completedwith the plan markdown (same proposed-plan card flow as Claude/Cursor) and answers with a revise-and-wait response so Grok ends its turn cleanly.interactionModeis not"plan"(the implementation turn started from the plan card) answersapproved, so Grok exits plan mode and builds. Plan-mode refinement turns capture the revised plan again.current_mode_updateand, when a turn arrives withinteractionMode: "plan"while the Grok session is not in plan mode, prepends an instruction steering Grok intoenter_plan_mode- previously T3 Code's plan toggle silently did nothing for Grok.exit_plan_modein one batched response; the CLI's plan-file read then races its own write andplanContentarrives null. The adapter remembers the plan-file path from theenter_plan_modetool output (rawOutput.Entered.plan_file_path) and poll-reads the file whenplanContentis empty.acp-mock-agent.ts+ tests: mock support for the ext request (including the empty-planContentrecovery path), adapter tests for capture → refine → approve, and schema unit tests.Verification
current_mode_updateflippedplan→default, and Grok edited the target file. (Test removed before commit - it requires an authenticated local Grok install.)vp test run src/provider/acp/XAiAcpExtension.test.ts- 14/14 pass..shmock wrapper fails to spawn for all pre-existing GrokAdapter tests too - so they rely on CI).tsgo --noEmitandvp checkclean.Note
Medium Risk
Touches Grok session lifecycle and auto-approval of plan exit over ACP; incorrect gating could approve or block plans wrongly, though logic is covered by new tests and path validation limits file-read abuse.
Overview
Fixes broken Grok plan mode where unhandled
_x.ai/exit_plan_modereverse requests left sessions stuck and plans invisible in the UI.ACP extension and adapter: Adds
XAiExitPlanModeRequestparsing and approved/rejected response builders inXAiAcpExtension.ts.GrokAdapterregisters handlers forx.ai/exit_plan_modeand_x.ai/exit_plan_mode, emitsturn.proposed.completedwith plan markdown (same card flow as Claude/Cursor), and answers with a revise-and-wait response so Grok ends the turn cleanly.shouldAutoApproveGrokPlangates auto-approval on a captured plan plus a newer non-plan user prompt (promptSerial/planCapture), so same-turn re-presentations and plan-mode refinement turns capture again instead of approving on the user's behalf.Plan recovery and steering: Session state tracks
planFilePathfromEnterPlanModetool updates and poll-reads the plan file whenplanContentis empty (CLI write/read race). When the thread is in plan mode but the Grok session is not, prompts get a leading instruction to callenter_plan_mode.extractGrokPlanFilePathonly accepts absolute, NUL-free paths.Tests: Mock agent env flags for exit-plan-mode scenarios; adapter tests for capture → refine → approve, same-turn double presentation, and file recovery; unit tests for extension helpers and approval logic.
Reviewed by Cursor Bugbot for commit 8f927f2. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add Grok plan-mode plan capture and approval handling over ACP
GrokAdapternow intercepts_x.ai/exit_plan_modeextension requests and captures the proposed plan markdown, emitting aturn.proposed.completedevent and responding with a rejected outcome to keep Grok in plan mode until the user acts.shouldAutoApproveGrokPlanhelper.planContentis missing from the request, the adapter recovers the plan by reading the file path previously reported viaEnterPlanModetool call updates.XAiAcpExtension.tsparse both bare and wrappedexit_plan_moderequests and construct approval or capture responses.T3_ACP_EMIT_XAI_EXIT_PLAN_MODEand related flags to simulate Grok plan-mode exit flows for local testing.Macroscope summarized 8f927f2.