Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/main/group-chat/group-chat-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,10 @@ ${participantContext}${availableSessionsContext}
${historyContext}

## User Request${readOnly ? ' (READ-ONLY MODE - do not make changes)' : ''}:
${message}${imageContext}`;
${message}${imageContext}

## 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.'}`;
Comment on lines +484 to +485
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.


// Get the base args from the agent configuration
const args = [...agent.args];
Expand Down Expand Up @@ -516,7 +519,9 @@ ${message}${imageContext}`;
console.log(`[GroupChat:Debug] Tool Type: ${chat.moderatorAgentId}`);
console.log(`[GroupChat:Debug] CWD: ${os.homedir()}`);
console.log(`[GroupChat:Debug] Command: ${command}`);
console.log(`[GroupChat:Debug] ReadOnly: true`);
console.log(
`[GroupChat:Debug] ReadOnly: true (moderator always read-only), participants readOnly: ${readOnly ?? false}`
);

// Spawn the moderator process in batch mode
try {
Expand Down