Skip to content

Commit cac3feb

Browse files
alari76claude
andauthored
feat: ground-truth child completion verification (Joe 4/5) (#504)
* feat: realtime blocked-child notifications for the orchestrator When a child session blocks on a tool approval or question, the parent orchestrator now gets an immediate push notification with the requestId and the exact API call to respond, instead of the child silently dying at timeout. Adds a 'blocked' child status, stops auto-denying pending prompts when the last client leaves an agent session, and replaces the headless-agent fast-deny with the normal prompt flow (5-min timeout remains the backstop). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: persistent notification outbox with replay when the orchestrator returns Notifications to the orchestrator (Agent Joe) were silently dropped when its Claude process was not running — terminal child-session events and monitor alerts vanished, so Joe woke up with no idea what happened while it was down. - New OrchestratorOutbox: JSON-file-backed queue (survives restarts, capped at 200 items) with a 60s background flusher that replays queued items as a single digest message once the orchestrator session is alive again. Flush is gated on the rate-limit circuit breaker and clears the queue before sending to avoid double-delivery. - sendOrchestratorNotification now enqueues to the outbox when the parent is unreachable and returns true (the outbox owns delivery from that point); false only when queueing itself fails. - OrchestratorMonitor.deliverToOrchestrator enqueues instead of dropping when the orchestrator process is down. - Flusher wired up at server boot in ws-server.ts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: pausable child timeouts, broader allowlist, worktree status in spawn response Blocked children used to burn their whole timeout waiting on an approval the orchestrator never saw — with PR1's realtime blocked notifications in place, the timeout now pauses while a child waits for an answer. - Working-time clock pauses when a child blocks on an approval/question and resumes (with the remaining budget) once the prompt is answered; a separate 30-min blocked-time cap still terminates abandoned children. - DEFAULT_TIMEOUT_MS raised from 10 to 30 minutes; spawn API now accepts and validates timeoutMs (1 min – 4 h), documented in Joe's CLAUDE.md. - AGENT_CHILD_ALLOWED_TOOLS broadened: python3, pytest, sed, rg, jq, mkdir, cp, mv, touch (rm/sudo/docker still require approval). - ChildSession now reports worktree status ('active'/'failed'/'none') and worktreePath so the orchestrator knows when isolation failed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: verify child completion against ground truth instead of transcript sniffing Completion checks used to grep the child transcript for keywords like "pull request" — false-positive on mentions, false-negative on terse output — and treated >100 chars of output as success on process exit. - isFinalStepMissing asks the real systems: gh pr list --head <branch> for 'pr' policy, git ls-remote --heads origin <branch> for 'merge' (run in the child's worktree, injectable exec for tests). Transcript sniffing remains only as a fallback when the command itself fails. - Children are nudged once when the final step is missing; if it still doesn't land, the terminal notification carries a "Completion not verified" note so the orchestrator knows to follow up. - Exit-path status now derives from ground truth, not output length; removed the dead supersededMsgs tracking. - Joe's CLAUDE.md template: drop the "poll children every 30 min" cron example — the server pushes blocked/terminal notifications and owns completion verification now. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 722338d commit cac3feb

3 files changed

Lines changed: 305 additions & 80 deletions

File tree

server/orchestrator-children.test.ts

Lines changed: 181 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,22 @@ function makeMockSessions(worktreeSucceeds = true) {
7070
} as any
7171
}
7272

73+
/**
74+
* Build a manager with a stubbed exec so ground-truth checks (gh / git)
75+
* never spawn real processes. The default stub reports the final step as
76+
* done ('[{"number": 1}]' parses as a non-empty PR list, and is non-empty
77+
* output for git ls-remote). Override `exec` to simulate a missing step.
78+
*/
79+
function makeManager(
80+
sessions: any,
81+
opts: { notify?: any; exec?: any } = {},
82+
): OrchestratorChildManager {
83+
return new OrchestratorChildManager(sessions, {
84+
exec: opts.exec ?? vi.fn(async () => '[{"number": 1}]'),
85+
...(opts.notify ? { notify: opts.notify } : {}),
86+
})
87+
}
88+
7389
// ---------------------------------------------------------------------------
7490
// Tests
7591
// ---------------------------------------------------------------------------
@@ -89,7 +105,7 @@ describe('OrchestratorChildManager', () => {
89105
describe('spawn', () => {
90106
beforeEach(() => {
91107
sessions = makeMockSessions()
92-
manager = new OrchestratorChildManager(sessions)
108+
manager = makeManager(sessions)
93109
})
94110

95111
it('creates a child that transitions from starting to running', async () => {
@@ -128,7 +144,7 @@ describe('OrchestratorChildManager', () => {
128144

129145
it('falls back gracefully when worktree creation fails', async () => {
130146
sessions = makeMockSessions(false)
131-
manager = new OrchestratorChildManager(sessions)
147+
manager = makeManager(sessions)
132148

133149
const child = await manager.spawn(makeRequest({ useWorktree: true }))
134150

@@ -146,7 +162,7 @@ describe('OrchestratorChildManager', () => {
146162

147163
it('reports worktree status "failed" when worktree creation fails', async () => {
148164
sessions = makeMockSessions(false)
149-
manager = new OrchestratorChildManager(sessions)
165+
manager = makeManager(sessions)
150166

151167
const child = await manager.spawn(makeRequest({ useWorktree: true }))
152168

@@ -187,7 +203,7 @@ describe('OrchestratorChildManager', () => {
187203
describe('status tracking', () => {
188204
beforeEach(() => {
189205
sessions = makeMockSessions()
190-
manager = new OrchestratorChildManager(sessions)
206+
manager = makeManager(sessions)
191207
})
192208

193209
it('marks child as completed when result event fires', async () => {
@@ -230,10 +246,11 @@ describe('OrchestratorChildManager', () => {
230246
expect(child.error).toBe('Claude returned an error')
231247
})
232248

233-
it('marks child as completed on exit with sufficient output', async () => {
249+
it('marks child as completed on exit when ground truth confirms the final step', async () => {
250+
// Default makeManager exec stub reports an existing PR for the branch.
234251
sessions.get = vi.fn(() => ({
235252
claudeProcess: null,
236-
outputHistory: [{ type: 'output', data: 'A'.repeat(200) }],
253+
outputHistory: [{ type: 'output', data: 'brief' }],
237254
pendingToolApprovals: new Map(),
238255
pendingControlRequests: new Map(),
239256
}))
@@ -249,7 +266,8 @@ describe('OrchestratorChildManager', () => {
249266
})
250267
})
251268

252-
it('marks child as failed on exit without sufficient output', async () => {
269+
it('marks child as failed on exit when the final step never landed', async () => {
270+
manager = makeManager(sessions, { exec: vi.fn(async () => '[]') })
253271
sessions.get = vi.fn(() => ({
254272
claudeProcess: null,
255273
outputHistory: [{ type: 'output', data: 'short' }],
@@ -266,7 +284,26 @@ describe('OrchestratorChildManager', () => {
266284
await vi.waitFor(() => {
267285
expect(child.status).toBe('failed')
268286
})
269-
expect(child.error).toContain('without sufficient output')
287+
expect(child.error).toContain('before the final step')
288+
})
289+
290+
it('marks commit-only child as failed on exit with no output at all', async () => {
291+
sessions.get = vi.fn(() => ({
292+
claudeProcess: null,
293+
outputHistory: [],
294+
pendingToolApprovals: new Map(),
295+
pendingControlRequests: new Map(),
296+
}))
297+
298+
const child = await manager.spawn(makeRequest({ completionPolicy: 'commit-only' }))
299+
300+
for (const listener of sessions._exitListeners) {
301+
listener(child.id, 1, null, false)
302+
}
303+
304+
await vi.waitFor(() => {
305+
expect(child.status).toBe('failed')
306+
})
270307
})
271308

272309
it('keeps monitoring when exit has willRestart=true', async () => {
@@ -295,6 +332,127 @@ describe('OrchestratorChildManager', () => {
295332
})
296333
})
297334

335+
// -------------------------------------------------------------------------
336+
// Ground-truth final-step verification
337+
// -------------------------------------------------------------------------
338+
339+
describe('ground-truth final-step verification', () => {
340+
const aliveSession = (output: string) => ({
341+
claudeProcess: { isAlive: vi.fn(() => true), stop: vi.fn() },
342+
outputHistory: output ? [{ type: 'output', data: output }] : [],
343+
pendingToolApprovals: new Map(),
344+
pendingControlRequests: new Map(),
345+
})
346+
347+
beforeEach(() => {
348+
sessions = makeMockSessions()
349+
})
350+
351+
it('checks gh pr list (in the worktree) for pr policy', async () => {
352+
const exec = vi.fn(async () => '[{"number": 7}]')
353+
manager = makeManager(sessions, { exec })
354+
sessions.get = vi.fn(() => aliveSession('done'))
355+
356+
const child = await manager.spawn(makeRequest({ completionPolicy: 'pr' }))
357+
for (const cb of sessions._resultListeners) cb(child.id, false)
358+
359+
await vi.waitFor(() => {
360+
expect(child.status).toBe('completed')
361+
})
362+
expect(exec).toHaveBeenCalledWith(
363+
'gh',
364+
['pr', 'list', '--head', 'fix/login-bug', '--state', 'all', '--json', 'number', '--limit', '1'],
365+
'/repos/myproject-wt-child123',
366+
)
367+
expect(child.error).toBeNull()
368+
})
369+
370+
it('nudges once when no PR exists, then completes with an unverified note', async () => {
371+
const exec = vi.fn(async () => '[]')
372+
manager = makeManager(sessions, { exec })
373+
sessions.get = vi.fn(() => aliveSession('made the changes and committed'))
374+
375+
const child = await manager.spawn(makeRequest({ completionPolicy: 'pr' }))
376+
377+
// First result: PR missing → nudge, keep monitoring
378+
for (const cb of sessions._resultListeners) cb(child.id, false)
379+
await vi.waitFor(() => {
380+
expect(sessions._sentInputs.some((p: string) => p.includes('no Pull Request exists'))).toBe(true)
381+
})
382+
expect(child.status).toBe('running')
383+
384+
// Second result: still no PR → no second nudge, terminal with note
385+
for (const cb of sessions._resultListeners) cb(child.id, false)
386+
await vi.waitFor(() => {
387+
expect(child.status).toBe('completed')
388+
})
389+
expect(child.error).toContain('Completion not verified')
390+
const nudges = sessions._sentInputs.filter((p: string) => p.includes('no Pull Request exists'))
391+
expect(nudges.length).toBe(1)
392+
})
393+
394+
it('checks git ls-remote for merge policy and nudges when the branch is not on the remote', async () => {
395+
const exec = vi.fn(async () => '')
396+
manager = makeManager(sessions, { exec })
397+
sessions.get = vi.fn(() => aliveSession('committed everything'))
398+
399+
const child = await manager.spawn(makeRequest({ completionPolicy: 'merge' }))
400+
for (const cb of sessions._resultListeners) cb(child.id, false)
401+
402+
await vi.waitFor(() => {
403+
expect(sessions._sentInputs.some((p: string) => p.includes('has not been pushed'))).toBe(true)
404+
})
405+
expect(exec).toHaveBeenCalledWith(
406+
'git',
407+
['ls-remote', '--heads', 'origin', 'fix/login-bug'],
408+
'/repos/myproject-wt-child123',
409+
)
410+
})
411+
412+
it('treats a non-empty ls-remote as pushed for merge policy', async () => {
413+
const exec = vi.fn(async () => 'abc123\trefs/heads/fix/login-bug\n')
414+
manager = makeManager(sessions, { exec })
415+
sessions.get = vi.fn(() => aliveSession('pushed'))
416+
417+
const child = await manager.spawn(makeRequest({ completionPolicy: 'merge' }))
418+
for (const cb of sessions._resultListeners) cb(child.id, false)
419+
420+
await vi.waitFor(() => {
421+
expect(child.status).toBe('completed')
422+
})
423+
expect(child.error).toBeNull()
424+
})
425+
426+
it('falls back to transcript sniffing when the ground-truth command fails', async () => {
427+
const exec = vi.fn(async () => { throw new Error('gh: command not found') })
428+
manager = makeManager(sessions, { exec })
429+
sessions.get = vi.fn(() => aliveSession('Opened a pull request with the changes.'))
430+
431+
const child = await manager.spawn(makeRequest({ completionPolicy: 'pr' }))
432+
for (const cb of sessions._resultListeners) cb(child.id, false)
433+
434+
await vi.waitFor(() => {
435+
expect(child.status).toBe('completed')
436+
})
437+
expect(child.error).toBeNull()
438+
})
439+
440+
it('never verifies remotely for commit-only policy', async () => {
441+
const exec = vi.fn(async () => '[]')
442+
manager = makeManager(sessions, { exec })
443+
sessions.get = vi.fn(() => aliveSession('committed locally'))
444+
445+
const child = await manager.spawn(makeRequest({ completionPolicy: 'commit-only' }))
446+
for (const cb of sessions._resultListeners) cb(child.id, false)
447+
448+
await vi.waitFor(() => {
449+
expect(child.status).toBe('completed')
450+
})
451+
expect(exec).not.toHaveBeenCalled()
452+
expect(child.error).toBeNull()
453+
})
454+
})
455+
298456
// -------------------------------------------------------------------------
299457
// Default allowlist
300458
// -------------------------------------------------------------------------
@@ -325,7 +483,7 @@ describe('OrchestratorChildManager', () => {
325483
beforeEach(() => {
326484
vi.useFakeTimers()
327485
sessions = makeMockSessions()
328-
manager = new OrchestratorChildManager(sessions)
486+
manager = makeManager(sessions)
329487
})
330488

331489
afterEach(() => {
@@ -410,6 +568,9 @@ describe('OrchestratorChildManager', () => {
410568
})
411569

412570
it('resumes the working clock after unblock with the remaining budget', async () => {
571+
// Ground truth reports no PR → after unblock, the child gets nudged and
572+
// monitoring continues on the resumed clock (~30s of budget left).
573+
manager = makeManager(sessions, { exec: vi.fn(async () => '[]') })
413574
const pendingApprovals = new Map([['req-1', { toolInput: { command: 'git push' } }]])
414575
const makeSession = (pending: Map<string, unknown>) => ({
415576
claudeProcess: { isAlive: vi.fn(() => true), stop: vi.fn() },
@@ -429,9 +590,10 @@ describe('OrchestratorChildManager', () => {
429590
pendingApprovals.clear()
430591
sessions.get = vi.fn(() => makeSession(new Map()))
431592
for (const cb of sessions._resultListeners) cb(child.id, false)
432-
// Result with empty output → nudge consumed? No — process alive, PR policy,
433-
// no "pull request" text → nudge fires and monitoring continues with the
434-
// resumed clock. Remaining working budget is ~30s.
593+
// Let the async ground-truth check + nudge settle, then burn the
594+
// remaining ~30s of working budget.
595+
await vi.advanceTimersByTimeAsync(0)
596+
expect(sessions._sentInputs.some((p: string) => p.includes('no Pull Request exists'))).toBe(true)
435597
vi.advanceTimersByTime(31_000)
436598

437599
await vi.waitFor(() => {
@@ -448,7 +610,7 @@ describe('OrchestratorChildManager', () => {
448610
describe('list and get', () => {
449611
beforeEach(() => {
450612
sessions = makeMockSessions()
451-
manager = new OrchestratorChildManager(sessions)
613+
manager = makeManager(sessions)
452614
})
453615

454616
it('lists children and retrieves by ID', async () => {
@@ -478,7 +640,7 @@ describe('OrchestratorChildManager', () => {
478640
describe('prompt generation', () => {
479641
beforeEach(() => {
480642
sessions = makeMockSessions()
481-
manager = new OrchestratorChildManager(sessions)
643+
manager = makeManager(sessions)
482644
})
483645

484646
it('includes worktree environment section when worktree succeeds', async () => {
@@ -510,7 +672,7 @@ describe('OrchestratorChildManager', () => {
510672

511673
it('includes create-branch step when NOT in worktree', async () => {
512674
sessions = makeMockSessions(false)
513-
manager = new OrchestratorChildManager(sessions)
675+
manager = makeManager(sessions)
514676
await manager.spawn(makeRequest({ useWorktree: true, completionPolicy: 'pr' }))
515677

516678
const prompt = sessions._sentInputs[0]
@@ -542,7 +704,7 @@ describe('OrchestratorChildManager', () => {
542704
beforeEach(() => {
543705
sessions = makeMockSessions()
544706
notify = vi.fn(() => true)
545-
manager = new OrchestratorChildManager(sessions, { notify })
707+
manager = makeManager(sessions, { notify })
546708
})
547709

548710
it('fires exactly one notification when a child times out', async () => {
@@ -673,7 +835,7 @@ describe('OrchestratorChildManager', () => {
673835
it('does NOT stamp terminalNotifiedAt when delivery fails (returns false)', async () => {
674836
// Notify reports the parent is unreachable on the first attempt.
675837
notify = vi.fn(() => false)
676-
manager = new OrchestratorChildManager(sessions, { notify })
838+
manager = makeManager(sessions, { notify })
677839

678840
sessions.get = vi.fn(() => ({
679841
claudeProcess: { isAlive: vi.fn(() => false), stop: vi.fn() },
@@ -729,7 +891,7 @@ describe('OrchestratorChildManager', () => {
729891

730892
it('does NOT stamp terminalNotifiedAt when notify throws', async () => {
731893
notify = vi.fn(() => { throw new Error('boom') })
732-
manager = new OrchestratorChildManager(sessions, { notify })
894+
manager = makeManager(sessions, { notify })
733895

734896
sessions.get = vi.fn(() => ({
735897
claudeProcess: { isAlive: vi.fn(() => false), stop: vi.fn() },
@@ -772,7 +934,7 @@ describe('OrchestratorChildManager', () => {
772934
beforeEach(() => {
773935
sessions = makeMockSessions()
774936
notify = vi.fn(() => true)
775-
manager = new OrchestratorChildManager(sessions, { notify })
937+
manager = makeManager(sessions, { notify })
776938
})
777939

778940
it('subscribes to session prompt events on construction', () => {

0 commit comments

Comments
 (0)