Skip to content

fix: Group chat: Plan mode can not be exited #602#695

Open
scriptease wants to merge 1 commit intoRunMaestro:rcfrom
scriptease:readonly-group-chat
Open

fix: Group chat: Plan mode can not be exited #602#695
scriptease wants to merge 1 commit intoRunMaestro:rcfrom
scriptease:readonly-group-chat

Conversation

@scriptease
Copy link
Copy Markdown

@scriptease scriptease commented Mar 31, 2026

Fix for Group chat: Plan mode can not be exited #602

readonly off readonly on file created

Summary by CodeRabbit

  • Features
    • Group chat moderator now clarifies execution modes based on settings. When read-only, participants can inspect and plan without file modifications; when enabled, participants gain full read-write access for executing changes.

…lity

The moderator is always spawned in read-only/plan mode (by design — it
coordinates, not writes). However, it was not told that participants have
full read-write access when the user toggles read-only off. This caused
the moderator to refuse file creation requests instead of delegating
them to participants.

Add an "Execution Mode" section to the moderator prompt that explicitly
states whether participants can write files, so the moderator delegates
implementation tasks appropriately.
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 31, 2026

📝 Walkthrough

Walkthrough

The moderator system prompt in the group-chat router was updated to include an explicit "Execution Mode" section clarifying participant behavior based on the readOnly flag. Additionally, the moderator debug log now reports both the fixed moderator read-only status and the effective participant read-only value.

Changes

Cohort / File(s) Summary
Moderator Execution Mode Configuration
src/main/group-chat/group-chat-router.ts
Added "Execution Mode" section to moderator system prompt to explicitly describe participant constraints when readOnly is true (inspect/analyze/plan only) or false (full read-write with file changes). Updated debug logging to display both moderator and participant read-only statuses derived from the readOnly parameter.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 The moderator now speaks clear and true,
With modes of execution, a refined view.
Read-only or free, the path is now plain,
No confusion remains in the rabbit's domain! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title refers to fixing an issue where plan mode cannot be exited, but the actual change makes the moderator aware of participant read-write capability and allows it to exit plan mode when appropriate. The title describes the symptom/desired outcome rather than the actual code change being made.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

@scriptease scriptease marked this pull request as ready for review March 31, 2026 09:19
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 31, 2026

Greptile Summary

This PR fixes issue #602 where plan mode (read-only mode) could not be exited in Group Chat. The root cause was that the moderator agent was always spawned with readOnlyMode: true in process args, but the prompt sent to the moderator contained no instruction explaining whether participants themselves had write access. This caused the moderator to always instruct participants as if they were in plan mode, even when the user had turned plan mode off.

Changes:

  • Appends a ## Execution Mode section to the moderator's prompt: when readOnly is false, participants are told they have full read-write access and the moderator is told to delegate file changes to them; when readOnly is true, all agents are restricted to inspect/analyze/plan only.
  • Improves the debug log to clarify that the moderator is always run read-only (by design), while participants' read-only state is controlled separately by the readOnly flag.

Confidence Score: 5/5

Safe to merge — the change is minimal and correctly addresses the prompt gap that prevented the moderator from delegating write operations to participants.

No P0 or P1 issues found. The fix is a targeted prompt addition that aligns with the existing architecture: the moderator is always spawned read-only in process args, and now the prompt correctly communicates the participants' write access state. The debug log improvement is also accurate and non-breaking.

No files require special attention.

Important Files Changed

Filename Overview
src/main/group-chat/group-chat-router.ts Adds a ## Execution Mode section to the moderator prompt so participants receive correct read-write vs. plan-mode instructions based on the readOnly flag; also fixes a misleading debug log about the moderator's read-only state.

Sequence Diagram

sequenceDiagram
    participant U as User
    participant R as group-chat-router
    participant M as Moderator
    participant P as Participant Agent

    U->>R: routeUserMessage(message, readOnly)
    R->>R: setGroupChatReadOnlyState(readOnly)
    R->>M: spawn readOnlyMode=true + Execution Mode prompt

    alt readOnly = false (plan mode OFF)
        Note over M: Participants have FULL READ-WRITE access.<br/>Delegate file changes to them.
        M->>P: write task delegation
        P->>P: Creates or modifies files
    else readOnly = true (plan mode ON)
        Note over M: READ-ONLY MODE active.<br/>All agents inspect and plan only.
        M->>P: plan or analyze task
        P->>P: Inspects and plans only
    end
Loading

Reviews (1): Last reviewed commit: "fix: make group chat moderator aware of ..." | Re-trigger Greptile

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/group-chat/group-chat-router.ts`:
- Around line 484-485: The Execution Mode message in group-chat-router uses the
readOnly variable to claim participants "cannot make file changes" which
contradicts the hardcoded participant spawn config (readOnlyMode: false) in the
group-chat-agent; update the wording in the template string that produces the
Execution Mode block to avoid absolute enforcement language (e.g., say
"read-only view for this user" or "participants are intended to follow read-only
rules" instead of "cannot make file changes") so the prompt no longer
overpromises enforcement; locate the template that builds the Execution Mode
text (references: the readOnly variable in the template string inside
group-chat-router and the participant spawn config readOnlyMode in
group-chat-agent) and adjust the text to a softer statement until runtime
enforcement is aligned.
🪄 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: Pro

Run ID: 0c74c505-78aa-4825-a69b-6126f7c5e868

📥 Commits

Reviewing files that changed from the base of the PR and between 88b7e1b and 64848fd.

📒 Files selected for processing (1)
  • src/main/group-chat/group-chat-router.ts

Comment on lines +484 to +485
## Execution Mode:
${readOnly ? 'READ-ONLY MODE is active. You and all participants can only inspect, analyze, and plan — no file changes allowed.' : 'Participants have FULL READ-WRITE access and can create, modify, and delete files. You are in read-only/plan mode yourself, so delegate all file changes to participants. When the user asks for implementation, specs, or file creation, delegate those tasks to the appropriate participants — they can execute.'}`;
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Execution-mode text overpromises participant read-only enforcement.

Line 485 says participants cannot make file changes in read-only mode, but participant spawn config is currently hardcoded with readOnlyMode: false in src/main/group-chat/group-chat-agent.ts:228-240. This creates a prompt/runtime mismatch.

Suggested minimal patch in this file (avoid absolute enforcement claim until runtime enforcement is aligned)
-${readOnly ? 'READ-ONLY MODE is active. You and all participants can only inspect, analyze, and plan — no file changes allowed.' : 'Participants have FULL READ-WRITE access and can create, modify, and delete files. You are in read-only/plan mode yourself, so delegate all file changes to participants. When the user asks for implementation, specs, or file creation, delegate those tasks to the appropriate participants — they can execute.'}`;
+${readOnly ? 'READ-ONLY MODE is active. Treat this round as analysis/planning only and do not request file changes.' : 'Participants have FULL READ-WRITE access and can create, modify, and delete files. You are in read-only/plan mode yourself, so delegate all file changes to participants. When the user asks for implementation, specs, or file creation, delegate those tasks to the appropriate participants — they can execute.'}`;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/group-chat/group-chat-router.ts` around lines 484 - 485, The
Execution Mode message in group-chat-router uses the readOnly variable to claim
participants "cannot make file changes" which contradicts the hardcoded
participant spawn config (readOnlyMode: false) in the group-chat-agent; update
the wording in the template string that produces the Execution Mode block to
avoid absolute enforcement language (e.g., say "read-only view for this user" or
"participants are intended to follow read-only rules" instead of "cannot make
file changes") so the prompt no longer overpromises enforcement; locate the
template that builds the Execution Mode text (references: the readOnly variable
in the template string inside group-chat-router and the participant spawn config
readOnlyMode in group-chat-agent) and adjust the text to a softer statement
until runtime enforcement is aligned.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The finding is wrong. When readOnly is true, participants DO get readOnlyMode: true (line 893) — which for Claude Code means --permission-mode plan. The enforcement IS aligned. The prompt accurately says participants cannot make changes when read-only is on.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@scriptease, you're correct — my finding was wrong. Looking at the participant spawn call in routeModeratorResponse within the same file, readOnlyMode: readOnly ?? false is explicitly passed to each participant's spawn config, so when the user sends a message with readOnly: true, participants receive readOnlyMode: true which maps to --permission-mode plan for Claude Code. The prompt wording accurately reflects the runtime enforcement. I apologize for the noise.


✏️ Learnings added
Learnt from: scriptease
Repo: RunMaestro/Maestro PR: 695
File: src/main/group-chat/group-chat-router.ts:484-485
Timestamp: 2026-03-31T09:29:51.557Z
Learning: In RunMaestro/Maestro group-chat-router.ts (`routeModeratorResponse` and `routeUserMessage`), participant spawn calls use `readOnlyMode: readOnly ?? false` to propagate the read-only flag from the user message to each participant. When readOnly is true, Claude Code participants receive `--permission-mode plan`. The Execution Mode section in the moderator prompt accurately reflects this runtime enforcement. Do not flag the prompt wording as overpromising enforcement.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: nolanmclark
Repo: RunMaestro/Maestro PR: 566
File: src/main/storage/copilot-session-storage.ts:297-299
Timestamp: 2026-03-13T02:46:48.786Z
Learning: In RunMaestro/Maestro, all session storage implementations (claude-session-storage.ts, codex-session-storage.ts, opencode-session-storage.ts, factory-droid-session-storage.ts, copilot-session-storage.ts) gate SSH remote operations using `if (sshConfig)` object truthiness checks, NOT `sshConfig?.enabled === true`. Do not flag this pattern as incorrect in future reviews.

Learnt from: jSydorowicz21
Repo: RunMaestro/Maestro PR: 0
File: :0-0
Timestamp: 2026-03-18T03:05:59.524Z
Learning: In RunMaestro/Maestro (src/renderer/stores/agentStore.ts, command spawn path ~line 455), slash commands never set `interjectionLogId` on their queued items. Therefore the interjection delivery-tracking code (marking delivered/failed) is never reached for commands — only message-type items created by the interjection flow in `useInputProcessing.ts` carry `interjectionLogId`. Do not flag command interjections as "never transitioning out of queued" in future reviews.

Learnt from: jSydorowicz21
Repo: RunMaestro/Maestro PR: 0
File: :0-0
Timestamp: 2026-03-18T03:05:59.524Z
Learning: In RunMaestro/Maestro (src/renderer/components/InputArea.tsx), the busy border (`showBusyBorder`) and busy placeholder text are derived from `activeTab?.state` (not `session.state`) as of commit 7d52005. Do not flag session-level state as incorrect here in future reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants