Skip to content
Merged
Show file tree
Hide file tree
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
141 changes: 141 additions & 0 deletions src/__tests__/main/group-chat/group-chat-router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,18 @@ vi.mock('../../../main/utils/ssh-spawn-wrapper', () => ({
wrapSpawnWithSsh: (...args: unknown[]) => mockWrapSpawnWithSsh(...args),
}));

// Mock stores/getters: router reads global shellEnvVars via getSettingsStore().
// The mock is driven by `mockedShellEnvVars` so individual tests can assert the
// value actually flows through to processManager.spawn() and (for SSH) into
// wrapSpawnWithSsh's customEnvVars input.
let mockedShellEnvVars: Record<string, string> = {};
vi.mock('../../../main/stores/getters', () => ({
getSettingsStore: () => ({
get: (key: string, defaultValue: unknown) =>
key === 'shellEnvVars' ? mockedShellEnvVars : defaultValue,
}),
}));
Comment thread
greptile-apps[bot] marked this conversation as resolved.

import {
extractMentions,
extractAllMentions,
Expand Down Expand Up @@ -94,6 +106,9 @@ describe('group-chat-router', () => {
// Set the mock userData path to our test directory
mockUserDataPath = testDir;

// Reset the global-settings shell env mock between tests
mockedShellEnvVars = {};

// Create a fresh mock for each test
mockProcessManager = {
spawn: vi.fn().mockReturnValue({ pid: 12345, success: true }),
Expand Down Expand Up @@ -1049,4 +1064,130 @@ describe('group-chat-router', () => {
expect(mockWrapSpawnWithSsh).not.toHaveBeenCalled();
});
});

// ===========================================================================
// Global shell env vars from Settings → Shell Configuration must flow into
// moderator / participant spawns for the local path, and must be merged into
// customEnvVars (per-agent takes precedence) for the SSH path so they reach
// the remote agent via the SSH stdin script.
// ===========================================================================
describe('global shell env vars forwarded to spawns', () => {
it('moderator spawn receives globally-configured shellEnvVars', async () => {
mockedShellEnvVars = { ANTHROPIC_API_KEY: 'sentinel-moderator-key' };
const chat = await createTestChatWithModerator('Moderator ShellEnv Test');

await routeUserMessage(chat.id, 'Hello', mockProcessManager, mockAgentDetector);

expect(mockProcessManager.spawn).toHaveBeenCalledWith(
expect.objectContaining({
shellEnvVars: { ANTHROPIC_API_KEY: 'sentinel-moderator-key' },
})
);
});

it('participant spawn receives globally-configured shellEnvVars', async () => {
mockedShellEnvVars = { ANTHROPIC_API_KEY: 'sentinel-participant-key' };
const chat = await createTestChatWithModerator('Participant ShellEnv Test');
await addParticipant(chat.id, 'Client', 'claude-code', mockProcessManager);

mockProcessManager.spawn.mockClear();

await routeModeratorResponse(
chat.id,
'@Client please help',
mockProcessManager,
mockAgentDetector
);

const participantSpawn = mockProcessManager.spawn.mock.calls.find((call) =>
call[0].prompt?.includes('please help')
);
expect(participantSpawn).toBeDefined();
expect(participantSpawn![0]).toEqual(
expect.objectContaining({
shellEnvVars: { ANTHROPIC_API_KEY: 'sentinel-participant-key' },
})
);
});

it('SSH participant spawn merges shellEnvVars into customEnvVars with per-agent precedence', async () => {
// SSH wrapper strips customEnvVars after embedding them into the remote script,
// so assertions target the INPUT to wrapSpawnWithSsh — that's where globals must land.
mockedShellEnvVars = {
ANTHROPIC_API_KEY: 'sentinel-global-key',
GLOBAL_ONLY: 'global-value',
};

mockWrapSpawnWithSsh.mockResolvedValue({
command: 'ssh',
args: ['user@pedtome.local', 'claude', '--print'],
cwd: '/home/user/project',
prompt: 'test prompt',
customEnvVars: undefined,
sshRemoteUsed: { name: 'PedTome' },
});
const mockSshStore = {
getSshRemotes: vi
.fn()
.mockReturnValue([
{ id: 'remote-1', name: 'PedTome', host: 'pedtome.local', user: 'user' },
]),
};
const sshRemoteConfig = {
enabled: true,
remoteId: 'remote-1',
workingDirOverride: '/home/user/project',
};

const chat = await createTestChatWithModerator('SSH Participant ShellEnv Test');

// Session-level override: should win over the global sentinel for the same key.
const sshSession: SessionInfo = {
id: 'ses-ssh-shellenv',
name: 'SSHWorker',
toolType: 'claude-code',
cwd: '/home/user/project',
sshRemoteName: 'PedTome',
sshRemoteConfig,
customEnvVars: { ANTHROPIC_API_KEY: 'per-agent-override' },
};
setGetSessionsCallback(() => [sshSession]);
setSshStore(mockSshStore);

await addParticipant(
chat.id,
'SSHWorker',
'claude-code',
mockProcessManager,
'/home/user/project',
mockAgentDetector,
{},
undefined,
{ sshRemoteName: 'PedTome', sshRemoteConfig },
mockSshStore
);

mockWrapSpawnWithSsh.mockClear();

await routeModeratorResponse(
chat.id,
'@SSHWorker implement the feature',
mockProcessManager,
mockAgentDetector
);

expect(mockWrapSpawnWithSsh).toHaveBeenCalledWith(
expect.objectContaining({
customEnvVars: {
GLOBAL_ONLY: 'global-value',
ANTHROPIC_API_KEY: 'per-agent-override',
},
}),
sshRemoteConfig,
mockSshStore
);

mockWrapSpawnWithSsh.mockReset();
});
});
});
2 changes: 2 additions & 0 deletions src/main/group-chat/group-chat-moderator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ export interface IProcessManager {
readOnlyMode?: boolean;
prompt?: string;
customEnvVars?: Record<string, string>;
/** Global shell env vars from Settings → Shell Configuration (merged by envBuilder). */
shellEnvVars?: Record<string, string>;
Comment on lines +31 to +32
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find process-manager-like spawn config interfaces that may need shellEnvVars added.
rg -n -C8 'interface .*ProcessManager|spawn\(config:\s*\{' --iglob '*.ts'

Repository: RunMaestro/Maestro

Length of output: 5080


🏁 Script executed:

#!/bin/bash
# Get full IProcessManager definition in group-chat-moderator.ts
echo "=== IProcessManager in group-chat-moderator.ts ==="
sed -n '21,40p' src/main/group-chat/group-chat-moderator.ts

echo ""
echo "=== GenericProcessManager in groupChat.ts ==="
sed -n '129,150p' src/main/ipc/handlers/groupChat.ts

echo ""
echo "=== GroomingProcessManager in context-groomer.ts ==="
sed -n '26,45p' src/main/utils/context-groomer.ts

Repository: RunMaestro/Maestro

Length of output: 2190


🏁 Script executed:

#!/bin/bash
# Check how GenericProcessManager is used in groupChat.ts
echo "=== Search for GenericProcessManager usage ==="
rg -n "GenericProcessManager" src/main/ipc/handlers/groupChat.ts -A 3 -B 3

echo ""
echo "=== Check moderatorRunner and agentRunner types ==="
rg -n "moderatorRunner|agentRunner" src/main/ipc/handlers/groupChat.ts -A 2 -B 2

echo ""
echo "=== Check if GenericProcessManager is instantiated or casted ==="
grep -n "new\|as GenericProcessManager\|as IProcessManager" src/main/ipc/handlers/groupChat.ts || echo "No direct instantiation found"

Repository: RunMaestro/Maestro

Length of output: 2495


🏁 Script executed:

#!/bin/bash
# Check where getProcessManager is called and what's passed to spawn()
echo "=== Find getProcessManager() calls ==="
rg -n "getProcessManager\(\)" src/main/ipc/handlers/groupChat.ts -A 5

echo ""
echo "=== Look for spawn() calls in groupChat.ts ==="
rg -n "\.spawn\(" src/main/ipc/handlers/groupChat.ts -A 8

echo ""
echo "=== Check GroupChatModerator instantiation ==="
rg -n "new GroupChatModerator\|GroupChatModerator\(" src/main/ipc/handlers/groupChat.ts -B 2 -A 5

Repository: RunMaestro/Maestro

Length of output: 3785


🏁 Script executed:

#!/bin/bash
# Find spawnModerator and related spawn calls
echo "=== Search for spawnModerator function ==="
rg -n "function spawnModerator|const spawnModerator|export.*spawnModerator" src/main/ipc/handlers/groupChat.ts -A 20

echo ""
echo "=== Look for spawn calls within groupChat functions ==="
rg -n "processManager\.spawn\|\.spawn\(" src/main/ipc/handlers/groupChat.ts -B 3 -A 10

echo ""
echo "=== Check GroupChatModerator for spawn() calls ==="
rg -n "this\.processManager\.spawn\|processManager\.spawn" src/main/group-chat/group-chat-moderator.ts -B 2 -A 10

Repository: RunMaestro/Maestro

Length of output: 213


🏁 Script executed:

#!/bin/bash
# Simpler search for spawnModerator
echo "=== Plain search for spawnModerator ==="
grep -rn "spawnModerator" src/main/ipc/handlers/groupChat.ts src/main/group-chat/

echo ""
echo "=== Look for spawn in group-chat directory ==="
grep -n "\.spawn(" src/main/group-chat/group-chat-moderator.ts | head -20

echo ""
echo "=== Check what functions exist in groupChat.ts before line 200 ==="
sed -n '1,200p' src/main/ipc/handlers/groupChat.ts | grep -n "^\(function\|const\|export\)" | tail -20

Repository: RunMaestro/Maestro

Length of output: 1781


🏁 Script executed:

#!/bin/bash
# Read spawnModerator implementation
echo "=== spawnModerator function ==="
sed -n '154,250p' src/main/group-chat/group-chat-moderator.ts

echo ""
echo "=== Look for spawn calls in the entire function ==="
grep -n "\.spawn(" src/main/group-chat/group-chat-moderator.ts -B 5 -A 15

Repository: RunMaestro/Maestro

Length of output: 3163


🏁 Script executed:

#!/bin/bash
# Find actual spawn calls in group-chat-router.ts
echo "=== Spawn calls in group-chat-router.ts ==="
grep -n "\.spawn(" src/main/group-chat/group-chat-router.ts -B 5 -A 20 | head -80

echo ""
echo "=== Also check for shell-related fields ==="
grep -n "shellEnvVars\|shell:\|runInShell" src/main/group-chat/ -r

Repository: RunMaestro/Maestro

Length of output: 5304


🏁 Script executed:

#!/bin/bash
# Check if there's any type casting or overriding of GenericProcessManager
echo "=== Where is processManager typed as GenericProcessManager vs IProcessManager? ==="
grep -n "processManager.*GenericProcessManager\|processManager.*IProcessManager" src/main/ipc/handlers/groupChat.ts

echo ""
echo "=== Check the actual type of processManager in group-chat-router.ts ==="
head -150 src/main/group-chat/group-chat-router.ts | grep -n "import.*ProcessManager\|ProcessManager\|processManager"

echo ""
echo "=== Verify the actual spawn call parameter types ==="
sed -n '817,834p' src/main/group-chat/group-chat-router.ts

Repository: RunMaestro/Maestro

Length of output: 927


🏁 Script executed:

#!/bin/bash
# Final check: verify what fields are used in spawn calls to ensure completeness
echo "=== All unique fields passed to spawn() in group-chat-router.ts ==="
grep -o "^\s*[a-zA-Z]*:" src/main/group-chat/group-chat-router.ts | grep -A30 "\.spawn({" | sort -u

echo ""
echo "=== Compare against GenericProcessManager definition ==="
sed -n '129,150p' src/main/ipc/handlers/groupChat.ts

echo ""
echo "=== Compare against IProcessManager definition ==="
sed -n '21,45p' src/main/group-chat/group-chat-moderator.ts

Repository: RunMaestro/Maestro

Length of output: 1763


Sync GenericProcessManager interface with IProcessManager spawn config.

GenericProcessManager is missing 6 optional fields that actual spawn() calls in group-chat-router.ts use: shellEnvVars, promptArgs, shell, runInShell, sendPromptViaStdin, and sendPromptViaStdinRaw. Adding these to GenericProcessManager will prevent type mismatches and keep the interface contracts aligned.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/group-chat/group-chat-moderator.ts` around lines 31 - 32, The
GenericProcessManager interface is missing six optional fields used by spawn()
in the router; update the GenericProcessManager definition to add these optional
properties with appropriate types—shellEnvVars?: Record<string,string>,
promptArgs?: string[], shell?: string, runInShell?: boolean,
sendPromptViaStdin?: boolean, and sendPromptViaStdinRaw?: boolean—so the
interface matches how spawn() is invoked (e.g., in group-chat-router.ts) and
prevents type mismatches.

contextWindow?: number;
promptArgs?: (prompt: string) => string[];
noPromptSeparator?: boolean;
Expand Down
42 changes: 32 additions & 10 deletions src/main/group-chat/group-chat-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import { groupChatParticipantRequestPrompt } from '../../prompts';
import { wrapSpawnWithSsh } from '../utils/ssh-spawn-wrapper';
import type { SshRemoteSettingsStore } from '../utils/ssh-remote-resolver';
import { setGetCustomShellPathCallback, getWindowsSpawnConfig } from './group-chat-config';
import { getSettingsStore } from '../stores/getters';

// Import emitters from IPC handlers (will be populated after handlers are registered)
import { groupChatEmitters } from '../ipc/handlers/groupChat';
Expand All @@ -57,6 +58,29 @@ const LOG_CONTEXT = '[GroupChatRouter]';
// Re-export setGetCustomShellPathCallback for index.ts to use
export { setGetCustomShellPathCallback };

/**
* Read global shell env vars from Settings → Shell Configuration.
* Mirrors the individual-chat path in `ipc/handlers/process.ts` so that
* group-chat moderator / participant / synthesis / recovery spawns receive
* the same merged environment.
*/
function getGlobalShellEnvVars(): Record<string, string> {
return getSettingsStore().get('shellEnvVars', {}) as Record<string, string>;
}

/**
* Merge global shell env vars with per-agent custom env vars, with per-agent
* taking precedence. Used before calling `wrapSpawnWithSsh()` so that the
* global vars are embedded into the remote SSH stdin script — otherwise they
* would only land on the local ssh client process and never reach the remote
* agent. Mirrors `ipc/handlers/process.ts:437`.
*/
function mergeGlobalShellWithCustomEnv(
customEnvVars: Record<string, string> | undefined
): Record<string, string> {
return { ...getGlobalShellEnvVars(), ...(customEnvVars || {}) };
}

/**
* Session info for matching @mentions to available Maestro sessions.
*/
Expand Down Expand Up @@ -772,9 +796,7 @@ ${message}`;
args: finalArgs,
cwd: os.homedir(),
prompt: fullPrompt,
customEnvVars:
configResolution.effectiveCustomEnvVars ??
getCustomEnvVarsCallback?.(chat.moderatorAgentId),
customEnvVars: mergeGlobalShellWithCustomEnv(spawnEnvVars),
promptArgs: agent.promptArgs,
noPromptSeparator: agent.noPromptSeparator,
agentBinaryName: agent.binaryName,
Expand Down Expand Up @@ -813,6 +835,7 @@ ${message}`;
prompt: spawnPrompt,
contextWindow: getContextWindowValue(agent, agentConfigValues),
customEnvVars: spawnEnvVars,
shellEnvVars: getGlobalShellEnvVars(),
promptArgs: agent.promptArgs,
noPromptSeparator: agent.noPromptSeparator,
shell: spawnShell,
Expand Down Expand Up @@ -1261,9 +1284,7 @@ export async function routeModeratorResponse(
args: spawnArgs,
cwd,
prompt: participantPrompt,
customEnvVars:
configResolution.effectiveCustomEnvVars ??
getCustomEnvVarsCallback?.(participant.agentId),
customEnvVars: mergeGlobalShellWithCustomEnv(finalSpawnEnvVars),
promptArgs: agent.promptArgs,
noPromptSeparator: agent.noPromptSeparator,
agentBinaryName: agent.binaryName,
Expand Down Expand Up @@ -1304,6 +1325,7 @@ export async function routeModeratorResponse(
prompt: finalSpawnPrompt,
contextWindow: getContextWindowValue(agent, agentConfigValues),
customEnvVars: finalSpawnEnvVars,
shellEnvVars: getGlobalShellEnvVars(),
promptArgs: agent.promptArgs,
noPromptSeparator: agent.noPromptSeparator,
shell: finalSpawnShell,
Expand Down Expand Up @@ -1682,9 +1704,7 @@ Review the agent responses above. Either:
args: finalArgs,
cwd: os.homedir(),
prompt: synthesisPrompt,
customEnvVars:
configResolution.effectiveCustomEnvVars ??
getCustomEnvVarsCallback?.(chat.moderatorAgentId),
customEnvVars: mergeGlobalShellWithCustomEnv(spawnEnvVars),
promptArgs: agent.promptArgs,
noPromptSeparator: agent.noPromptSeparator,
agentBinaryName: agent.binaryName,
Expand Down Expand Up @@ -1723,6 +1743,7 @@ Review the agent responses above. Either:
prompt: spawnPrompt,
contextWindow: getContextWindowValue(agent, agentConfigValues),
customEnvVars: spawnEnvVars,
shellEnvVars: getGlobalShellEnvVars(),
promptArgs: agent.promptArgs,
noPromptSeparator: agent.noPromptSeparator,
shell: winConfig.shell,
Expand Down Expand Up @@ -1883,7 +1904,7 @@ export async function respawnParticipantWithRecovery(
args: finalSpawnArgs,
cwd,
prompt: fullPrompt,
customEnvVars: finalSpawnEnvVars,
customEnvVars: mergeGlobalShellWithCustomEnv(finalSpawnEnvVars),
promptArgs: agent.promptArgs,
noPromptSeparator: agent.noPromptSeparator,
agentBinaryName: agent.binaryName,
Expand Down Expand Up @@ -1921,6 +1942,7 @@ export async function respawnParticipantWithRecovery(
prompt: finalSpawnPrompt,
contextWindow: getContextWindowValue(agent, agentConfigValues),
customEnvVars: finalSpawnEnvVars,
shellEnvVars: getGlobalShellEnvVars(),
promptArgs: agent.promptArgs,
noPromptSeparator: agent.noPromptSeparator,
shell: finalSpawnShell,
Expand Down
7 changes: 7 additions & 0 deletions src/main/ipc/handlers/groupChat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,15 @@ interface GenericProcessManager {
readOnlyMode?: boolean;
prompt?: string;
customEnvVars?: Record<string, string>;
/** Global shell env vars from Settings → Shell Configuration (merged by envBuilder). */
shellEnvVars?: Record<string, string>;
contextWindow?: number;
promptArgs?: (prompt: string) => string[];
noPromptSeparator?: boolean;
shell?: string;
runInShell?: boolean;
sendPromptViaStdin?: boolean;
sendPromptViaStdinRaw?: boolean;
}): { pid: number; success: boolean };
write(sessionId: string, data: string): boolean;
kill(sessionId: string): boolean;
Expand Down