Skip to content

Commit 337935f

Browse files
brookscclaude
andcommitted
chore: merge task/orc-8b22e6 — add unit tests for TODOs johannesjo#9, johannesjo#10, johannesjo#11
- userEdited suppression guard in processAutoFireTick (+ questionActive compat) - /tmp MCP config cleanup test in coordinator.test.ts - coordinator checkbox re-enable test in tasks.test.ts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 parents 54ceeb3 + 847d029 commit 337935f

5 files changed

Lines changed: 123 additions & 18 deletions

File tree

TODOS.md

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,6 @@
1111

1212
## Missing unit tests
1313

14-
### 9. Edit suppression — autofire must not fire when user edits staged text
15-
16-
**File:** `src/components/PromptInput.tsx` (autofire tick), `src/components/autofire-tick.ts` (if extracted), `src/components/PromptInput.test.ts`
17-
**What's missing:** No test verifies that setting `userEdited: true` on a staged notification prevents the autofire tick from firing.
18-
**Done when:** A test in `PromptInput.test.ts` (or a new `autofire-tick.test.ts`) stages a notification, sets `userEdited: true`, advances the timer, and asserts the prompt was NOT auto-sent.
19-
20-
### 10. /tmp config cleanup — sub-task MCP config deleted on close
21-
22-
**File:** `electron/mcp/coordinator.ts` (`cleanupTask`), `electron/mcp/coordinator.test.ts`
23-
**What's missing:** `cleanupTask` should delete the per-task config written to `/tmp/parallel-code-subtask-<id>.json` but no test verifies deletion.
24-
**Done when:** A test in `coordinator.test.ts` calls `close_task`, then asserts `fs.existsSync(configPath)` is false.
25-
26-
### 11. Coordinator checkbox re-enables after coordinator task closes
27-
28-
**File:** `src/components/NewTaskDialog.tsx` (`hasActiveCoordinator()`), `src/store/tasks.test.ts`
29-
**What's missing:** `hasActiveCoordinator()` derives from `store.tasks` but no test verifies that removing a coordinator task causes it to return false.
30-
**Done when:** A test in `tasks.test.ts` creates a coordinator task, closes it (removes from store), and asserts `hasActiveCoordinator()` returns false.
31-
3214
### 12. selectMcpJsonDir — Docker mode writes .mcp.json to worktree
3315

3416
**File:** `electron/ipc/register.ts` (`selectMcpJsonDir`), `electron/ipc/register.test.ts`

electron/mcp/coordinator.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1984,6 +1984,7 @@ describe('Coordinator close with active sub-tasks', () => {
19841984
const task = coordinator.getTask('task-1');
19851985
const configPath = task?.mcpConfigPath;
19861986
expect(configPath).toBeDefined();
1987+
expect(configPath).toMatch(/parallel-code-subtask-task-1\.json$/);
19871988

19881989
mockUnlinkSync.mockClear();
19891990
await coordinator.closeTask('task-1');
@@ -1993,6 +1994,22 @@ describe('Coordinator close with active sub-tasks', () => {
19931994
expect(unlinkCall).toBeDefined();
19941995
});
19951996

1997+
it('does not call unlinkSync for tasks created without MCP server info', async () => {
1998+
// No setMCPServerInfo call — task gets no mcpConfigPath
1999+
await coordinator.createTask({ name: 'test', prompt: 'do work', coordinatorTaskId: 'coord-1' });
2000+
2001+
expect(coordinator.getTask('task-1')?.mcpConfigPath).toBeUndefined();
2002+
2003+
mockUnlinkSync.mockClear();
2004+
await coordinator.closeTask('task-1');
2005+
2006+
// unlinkSync should NOT have been called with a parallel-code path
2007+
const parallelCodeCalls = mockUnlinkSync.mock.calls.filter(
2008+
(c) => typeof c[0] === 'string' && (c[0] as string).includes('parallel-code-subtask'),
2009+
);
2010+
expect(parallelCodeCalls).toHaveLength(0);
2011+
});
2012+
19962013
it('task is removed from map even when docker exec sub-tasks are active', async () => {
19972014
mockCreateBackendTask.mockResolvedValue({
19982015
id: 'task-1',

src/components/PromptInput.test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,61 @@ const pastNow = 2_000; // past autoFireAt=1000
1414
const noPromptTail = 'agent is thinking...';
1515
const promptTail = 'agent output ❯ ';
1616

17+
const stagedEdited: StagedNotification = {
18+
...staged,
19+
userEdited: true,
20+
};
21+
22+
describe('processAutoFireTick — userEdited suppression', () => {
23+
it('returns paused when userEdited=true with controlledBy coordinator and prompt visible', () => {
24+
const result = processAutoFireTick({
25+
staged: stagedEdited,
26+
now: pastNow,
27+
controlledBy: 'coordinator',
28+
questionActive: false,
29+
tail: promptTail,
30+
currentMissCount: 0,
31+
});
32+
expect(result.outcome).toBe('paused');
33+
});
34+
35+
it('returns paused when userEdited=true with controlledBy undefined and prompt visible', () => {
36+
const result = processAutoFireTick({
37+
staged: stagedEdited,
38+
now: pastNow,
39+
controlledBy: undefined,
40+
questionActive: false,
41+
tail: promptTail,
42+
currentMissCount: 0,
43+
});
44+
expect(result.outcome).toBe('paused');
45+
});
46+
47+
it('returns paused when userEdited=true with controlledBy human', () => {
48+
const result = processAutoFireTick({
49+
staged: stagedEdited,
50+
now: pastNow,
51+
controlledBy: 'human',
52+
questionActive: false,
53+
tail: promptTail,
54+
currentMissCount: 0,
55+
});
56+
expect(result.outcome).toBe('paused');
57+
});
58+
59+
it('fires when userEdited=false with controlledBy coordinator and prompt visible (regression guard)', () => {
60+
const result = processAutoFireTick({
61+
staged,
62+
now: pastNow,
63+
controlledBy: 'coordinator',
64+
questionActive: false,
65+
tail: promptTail,
66+
currentMissCount: 0,
67+
});
68+
expect(result.outcome).toBe('fire');
69+
});
70+
});
71+
1772
describe('processAutoFireTick — controlledBy: human', () => {
1873
it('returns paused without touching the miss counter when controlledBy is human', () => {
1974
const result = processAutoFireTick({

src/components/autofire-tick.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@ export function processAutoFireTick(params: {
3737
return { outcome: 'paused' };
3838
}
3939

40+
// User has manually edited the prompt — suppress autofire indefinitely until
41+
// they press Enter themselves.
42+
if (params.staged.userEdited) {
43+
return { outcome: 'paused' };
44+
}
45+
4046
const tailSnippet = params.tail.slice(-PROMPT_MARKER_SCAN_CHARS);
4147
const hasPrompt = /[]/.test(tailSnippet);
4248
if (!hasPrompt) {

src/store/tasks.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,3 +210,48 @@ describe('MCP_TaskCreated IPC handler', () => {
210210
expect(mockTasks['sub-task-1'].controlledBy).toBeDefined();
211211
});
212212
});
213+
214+
describe('hasActiveCoordinator condition — coordinator task removal', () => {
215+
it('condition is false when the store has no tasks', () => {
216+
const result = Object.values(mockTasks).some((t) => t.coordinatorMode && !t.closingStatus);
217+
expect(result).toBe(false);
218+
});
219+
220+
it('condition is true when a coordinator task exists in the store', () => {
221+
mockTasks['coord-1'] = {
222+
coordinatorMode: true,
223+
closingStatus: undefined,
224+
projectId: 'proj-1',
225+
agentIds: [],
226+
shellAgentIds: [],
227+
};
228+
const result = Object.values(mockTasks).some((t) => t.coordinatorMode && !t.closingStatus);
229+
expect(result).toBe(true);
230+
});
231+
232+
it('condition is false after the coordinator task is removed from the store', () => {
233+
mockTasks['coord-1'] = {
234+
coordinatorMode: true,
235+
closingStatus: undefined,
236+
projectId: 'proj-1',
237+
agentIds: [],
238+
shellAgentIds: [],
239+
};
240+
// Simulate task removal (what happens when close completes)
241+
delete mockTasks['coord-1'];
242+
const result = Object.values(mockTasks).some((t) => t.coordinatorMode && !t.closingStatus);
243+
expect(result).toBe(false);
244+
});
245+
246+
it('condition is false when the coordinator task has closingStatus set (is being closed)', () => {
247+
mockTasks['coord-1'] = {
248+
coordinatorMode: true,
249+
closingStatus: 'closing',
250+
projectId: 'proj-1',
251+
agentIds: [],
252+
shellAgentIds: [],
253+
};
254+
const result = Object.values(mockTasks).some((t) => t.coordinatorMode && !t.closingStatus);
255+
expect(result).toBe(false);
256+
});
257+
});

0 commit comments

Comments
 (0)