Skip to content

Commit 2f14d3d

Browse files
committed
fix(coordinator): address PR125 polish nits
1 parent 5af7cf1 commit 2f14d3d

6 files changed

Lines changed: 73 additions & 7 deletions

File tree

electron/mcp/agent-args.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,22 @@ describe('MCP agent launch args', () => {
2828
]);
2929
});
3030

31+
it('quotes Codex inline config env keys so non-bare TOML keys remain valid', () => {
32+
const override = buildCodexMcpConfigOverride({
33+
mcpServers: {
34+
'parallel-code': {
35+
command: 'node',
36+
args: [],
37+
env: {
38+
'TOKEN.WITH.DOTS': 'token-1',
39+
},
40+
},
41+
},
42+
});
43+
44+
expect(override).toContain('"TOKEN.WITH.DOTS" = "token-1"');
45+
});
46+
3147
it('uses --mcp-config for Claude-compatible agents', () => {
3248
expect(buildMcpLaunchArgs('claude', '/tmp/config.json', config)).toEqual([
3349
'--mcp-config',

electron/mcp/agent-args.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ function tomlStringArray(values: string[]): string {
2424

2525
function tomlStringMap(values: Record<string, string>): string {
2626
return `{ ${Object.entries(values)
27-
.map(([key, value]) => `${key} = ${tomlString(value)}`)
27+
.map(([key, value]) => `${tomlString(key)} = ${tomlString(value)}`)
2828
.join(', ')} }`;
2929
}
3030

src/components/NewTaskDialog.tsx

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ import { ProjectSelect } from './ProjectSelect';
3232
import { SymlinkDirPicker } from './SymlinkDirPicker';
3333
import type { AgentDef } from '../ipc/types';
3434
import { DEFAULT_DOCKER_IMAGE, PROJECT_DOCKERFILE_RELATIVE_PATH } from '../lib/docker';
35+
import {
36+
clampCoordinatorConcurrentTasks,
37+
DEFAULT_COORDINATOR_CONCURRENT_TASKS,
38+
MAX_COORDINATOR_CONCURRENT_TASKS,
39+
MIN_COORDINATOR_CONCURRENT_TASKS,
40+
} from '../lib/coordinator-limits';
3541

3642
interface NewTaskDialogProps {
3743
open: boolean;
@@ -65,7 +71,9 @@ export function NewTaskDialog(props: NewTaskDialogProps) {
6571
} | null>(null);
6672
const [coordinatorMode, setCoordinatorMode] = createSignal(false);
6773
const [propagateSkipPermissions, setPropagateSkipPermissions] = createSignal(false);
68-
const [maxConcurrentTasks, setMaxConcurrentTasks] = createSignal(3);
74+
const [maxConcurrentTasks, setMaxConcurrentTasks] = createSignal(
75+
DEFAULT_COORDINATOR_CONCURRENT_TASKS,
76+
);
6977
const hasActiveCoordinator = () =>
7078
Object.values(store.tasks).some(
7179
(t) => t.coordinatorMode && !t.closingStatus && t.projectId === selectedProjectId(),
@@ -553,7 +561,9 @@ export function NewTaskDialog(props: NewTaskDialogProps) {
553561
: undefined,
554562
coordinatorMode: coordinatorMode() || undefined,
555563
propagateSkipPermissions: coordinatorMode() ? propagateSkipPermissions() : undefined,
556-
maxConcurrentTasks: coordinatorMode() ? maxConcurrentTasks() : undefined,
564+
maxConcurrentTasks: coordinatorMode()
565+
? clampCoordinatorConcurrentTasks(maxConcurrentTasks())
566+
: undefined,
557567
});
558568
// Drop flow: prefill prompt without auto-sending
559569
if (isFromDrop && p) {
@@ -1137,12 +1147,12 @@ export function NewTaskDialog(props: NewTaskDialogProps) {
11371147
Max concurrent sub-tasks:
11381148
<input
11391149
type="number"
1140-
min="1"
1141-
max="20"
1150+
min={MIN_COORDINATOR_CONCURRENT_TASKS}
1151+
max={MAX_COORDINATOR_CONCURRENT_TASKS}
11421152
value={maxConcurrentTasks()}
11431153
onInput={(e) => {
11441154
const v = parseInt(e.currentTarget.value, 10);
1145-
if (!isNaN(v) && v >= 1) setMaxConcurrentTasks(v);
1155+
if (!isNaN(v)) setMaxConcurrentTasks(clampCoordinatorConcurrentTasks(v));
11461156
}}
11471157
style={{
11481158
width: '60px',

src/lib/coordinator-limits.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import {
4+
clampCoordinatorConcurrentTasks,
5+
DEFAULT_COORDINATOR_CONCURRENT_TASKS,
6+
} from './coordinator-limits';
7+
8+
describe('clampCoordinatorConcurrentTasks', () => {
9+
it('keeps values within the coordinator concurrency range', () => {
10+
expect(clampCoordinatorConcurrentTasks(0)).toBe(1);
11+
expect(clampCoordinatorConcurrentTasks(3)).toBe(3);
12+
expect(clampCoordinatorConcurrentTasks(21)).toBe(20);
13+
});
14+
15+
it('falls back to the default for invalid values', () => {
16+
expect(clampCoordinatorConcurrentTasks(Number.NaN)).toBe(DEFAULT_COORDINATOR_CONCURRENT_TASKS);
17+
expect(clampCoordinatorConcurrentTasks(Number.POSITIVE_INFINITY)).toBe(
18+
DEFAULT_COORDINATOR_CONCURRENT_TASKS,
19+
);
20+
});
21+
});

src/lib/coordinator-limits.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
export const MIN_COORDINATOR_CONCURRENT_TASKS = 1;
2+
export const MAX_COORDINATOR_CONCURRENT_TASKS = 20;
3+
export const DEFAULT_COORDINATOR_CONCURRENT_TASKS = 3;
4+
5+
export function clampCoordinatorConcurrentTasks(value: number): number {
6+
if (!Number.isFinite(value)) return DEFAULT_COORDINATOR_CONCURRENT_TASKS;
7+
return Math.min(
8+
MAX_COORDINATOR_CONCURRENT_TASKS,
9+
Math.max(MIN_COORDINATOR_CONCURRENT_TASKS, Math.trunc(value)),
10+
);
11+
}

src/store/tasks.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ import { parseGitHubUrl, taskNameFromGitHubUrl } from '../lib/github-url';
2929
import type { Agent, Task, GitIsolationMode } from './types';
3030
import type { DockerSource } from '../lib/docker';
3131
import { COORDINATOR_PREAMBLE } from './coordinator-preamble';
32+
import {
33+
clampCoordinatorConcurrentTasks,
34+
DEFAULT_COORDINATOR_CONCURRENT_TASKS,
35+
} from '../lib/coordinator-limits';
3236
import { getCoordinatorChildren, isCoordinatedChild } from './sidebar-order';
3337

3438
function initTaskInStore(
@@ -257,7 +261,11 @@ export async function createTask(opts: CreateTaskOptions): Promise<string> {
257261
opts.coordinatorMode && effectivePrompt
258262
? COORDINATOR_PREAMBLE.replace(
259263
/\{\{MAX_CONCURRENT\}\}/g,
260-
String(opts.maxConcurrentTasks ?? 3),
264+
String(
265+
clampCoordinatorConcurrentTasks(
266+
opts.maxConcurrentTasks ?? DEFAULT_COORDINATOR_CONCURRENT_TASKS,
267+
),
268+
),
261269
) +
262270
`Use \`${opts.baseBranch}\` as the baseBranch for all sub-tasks.\n\n` +
263271
effectivePrompt

0 commit comments

Comments
 (0)