fix(maestro-p): finalize interactive plan-mode turns on ExitPlanMode

[pedramamini]

Closes #1117

Problem

With Token Source = Interactive, Maestro routes the agent turn through maestro-p, which drives the Claude TUI (instead of claude --print). When the agent is in read-only / plan mode, Maestro forwards --permission-mode plan to the TUI.

In plan mode, the model ends its turn by calling the ExitPlanMode tool, which parks the TUI on a blocking "Ready to code? 1. Yes / 2. No, keep planning" approval dialog. maestro-p is a headless driver with no human to answer it, so the model never emits a stop_reason: end_turn assistant message.

maestro-p's run loop only finalizes a turn on end_turn, so that completion signal never arrives. The idle watchdog then kills the turn after --max-wait (~302s) with error: timeout / exitCode 3, and because that is an error path it drops the assistant text it had already captured, surfacing to the user as response: undefined. This matches the report in #1117 exactly (the plan/answer was generated, e.g. the tab-namer captured "Texas", but the main agent path returned nothing).

The headless claude --print --permission-mode plan path is unaffected because print mode emits the plan and exits with no interactive dialog.

Fix

Treat an ExitPlanMode tool call as a terminal completion for the turn:

• New pure helper extractExitPlanText(message) (src/maestro-p/plan-mode.ts) detects an ExitPlanMode tool_use block and returns its plan body (carried in the tool_use input.plan, not in a text block).
• In the run-mode loop (src/maestro-p/index.ts), when an assistant message calls ExitPlanMode, fold the plan body into the aggregated result and finalize() with exitCode 0, mirroring the semantics of claude --print --permission-mode plan (the plan is the deliverable for a read-only turn).

Tests

Added src/__tests__/maestro-p/plan-mode.test.ts (7 cases: plan extracted, missing/non-string plan body, normal end_turn, non-ExitPlanMode tool_use, block-position independence, malformed input). Full maestro-p suite passes (224 tests).

Notes

• No change to the --print / API path or to normal end_turn turns.
• Read-only intent is preserved: the turn still does not execute anything; the user receives the plan and can follow up to proceed.

Summary by CodeRabbit

• New Features

  • Added capability to extract and signal plan completion from tool calls.

• Tests

  • Added comprehensive test suite covering plan extraction with multiple scenarios and defensive handling for edge cases.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 22f38c71-c7a9-4649-932f-f2b570ea62b3

📥 Commits

Reviewing files that changed from the base of the PR and between 8a4cccf and 5bcc50f.

📒 Files selected for processing (3)

• src/__tests__/maestro-p/plan-mode.test.ts
• src/maestro-p/index.ts
• src/maestro-p/plan-mode.ts

---

📝 Walkthrough

Walkthrough

Adds a new plan-mode.ts module exporting extractExitPlanText, which inspects assistant messages for an ExitPlanMode tool call and returns the plan string or null. The function is integrated into processEntry in index.ts as an early-finalization path that skips the normal end_turn grace timer, with a Vitest suite covering all extraction behaviors.

Changes

Plan-mode ExitPlanMode detection and early finalization

Layer / File(s)	Summary
extractExitPlanText helper src/maestro-p/plan-mode.ts	Defines EXIT_PLAN_MODE_TOOL constant and extractExitPlanText(message: unknown): string | null, which scans the content array of an assistant message for a tool_use block named ExitPlanMode and returns input.plan (or '' if absent), and null for non-plan turns.
processEntry plan-mode wiring src/maestro-p/index.ts	Imports extractExitPlanText and extends the assistant-message branch of processEntry: calls the helper, appends the plan to aggregatedText with deduplication, invokes finalize({ isError: false, exitCode: 0 }), and returns early to bypass the end_turn grace-timer path.
extractExitPlanText tests src/__tests__/maestro-p/plan-mode.test.ts	Vitest suite with cases for normal extraction, missing plan body, non-string plan, end_turn messages, non-ExitPlanMode tool calls, positional robustness, and malformed/null inputs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 Hop hop, the plan arrives at last,
No more timeouts, no grace-timer blast!
ExitPlanMode rings the finish bell,
finalize fires—all is well.
The rabbit cheers: the bug is past! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: finalizing interactive plan-mode turns when ExitPlanMode is called, which directly addresses the PR's core objective.
Linked Issues check	✅ Passed	The PR successfully addresses issue #1117 by implementing ExitPlanMode detection to finalize plan-mode turns without requiring end_turn, preventing the 302-second timeout and exitCode 3 error.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to implementing ExitPlanMode detection and plan extraction; no unrelated modifications to other systems or functionality were introduced.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/1117-maestro-p-plan-mode-timeout

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

Fixes a hang-then-timeout on every interactive plan-mode turn: maestro-p now treats an ExitPlanMode tool call as a terminal completion rather than waiting for end_turn, which the TUI never sends because it parks on a blocking approval dialog the headless driver cannot dismiss.

• src/maestro-p/plan-mode.ts adds a single pure helper, extractExitPlanText, that detects the ExitPlanMode tool_use block and returns the plan body from input.plan (not from a text block), returning null for ordinary turns.
• src/maestro-p/index.ts wires the helper into the processEntry loop: on a non-null return it folds the plan body into aggregatedText and calls finalize(exitCode 0) immediately, bypassing the idle watchdog that was previously killing the turn at --max-wait.
• Seven new tests cover the main scenarios, including block-position independence and malformed input.

Confidence Score: 4/5

Safe to merge; the change is well-scoped and only affects the interactive plan-mode path.

The fix is narrow, well-scoped to the interactive plan-mode path, and backed by comprehensive tests. The single concern is a substring deduplication check that could silently drop the plan body in a contrived edge case.

The deduplication guard on line 467 of src/maestro-p/index.ts is the only place that warrants a second look.

Important Files Changed

Filename	Overview
src/maestro-p/plan-mode.ts	New pure helper that detects an ExitPlanMode tool_use block and extracts the plan body; well-guarded against malformed input, returns null for normal turns and a string (possibly empty) for ExitPlanMode turns.
src/maestro-p/index.ts	Integrates plan-mode detection into the run-mode processEntry loop; calls finalize(exitCode 0) immediately on ExitPlanMode so the idle watchdog never fires - the substring dedup check on planText has a minor edge-case correctness risk.
src/tests/maestro-p/plan-mode.test.ts	Seven test cases covering the happy path, missing/non-string plan body, normal end_turn, non-ExitPlanMode tool_use, block-position independence, and malformed input - comprehensive coverage.

_{Reviews (1): Last reviewed commit: "fix(maestro-p): finalize interactive pla..." | Re-trigger Greptile}

[greptile-apps]

Substring deduplication can silently drop the plan body

aggregatedText.includes(planText) is a substring match against all text accumulated earlier in the turn. If the model previously wrote any text that happens to contain planText as a literal substring (e.g. the same phrasing appears in a prior reasoning block), the guard evaluates to true and the plan is never appended - the user receives only the pre-ExitPlanMode text. Because collectAssistantText only collects text-type blocks and the plan lives in tool_use.input.plan, this overlap is unlikely today, but it's a silent correctness trap. A boolean flag (planTextAppended) would be more reliable.