From 1d8a80ad259343e60d1a1dd8e5b3d61f7e2c0b19 Mon Sep 17 00:00:00 2001 From: cannabinoids <13259089+cannabinoids@users.noreply.github.com> Date: Tue, 18 Aug 2026 11:23:33 -0600 Subject: [PATCH] fix: hold the wording path to the same bar as the sentinel path Rebased onto main after 1782034. That commit raised the idle thresholds and added CONTINUATION_PATTERNS, which covers progress reports -- the case that killed the four-agent research team. Three things it does not cover remain. The wording path still ends a run on two agents, whatever the team size. `hasTwoRecentCompletionSignals` returns true for any two distinct agents, so once a trio is quiet past TWO_SIGNAL_IDLE_THRESHOLD_MS the third can still be cut off by the other two -- exactly what the comment above the call warns about. It now requires every active agent, matching the sentinel path. Two guards on the heuristic path in isCompletionStatement, after the explicit CLOSING_PATTERNS check so an unambiguous sign-off still wins outright. A sign-off is short, and it does not quote the machinery: on a review task pointed at collab-poll.sh both agents quoted its own ---STATUS:{ACTIVE,QUIET,DONE,WAITING} sentinel inside long analysis messages, and that ended the session. CONTINUATION_PATTERNS does not catch it, because nothing about those messages announces further work. Completion is also counted only from the last message the user sent. `ensemble steer` exists so a user can redirect a running team, but a sentinel from before the redirect still counted, so a team could disband while an agent was answering the new instruction -- seen live with a 15s gap between the sentinel and the interjection. "Wrap it up" followed by sentinels still disbands. Nine specs added, five in premature-disband.test.ts and four in ensemble.test.ts. All nine fail on main and pass with the change. Co-Authored-By: Claude Opus 5 --- services/ensemble-service.ts | 53 ++++++++++++++++++--- tests/ensemble.test.ts | 81 +++++++++++++++++++++++++++++++++ tests/premature-disband.test.ts | 29 ++++++++++++ 3 files changed, 156 insertions(+), 7 deletions(-) diff --git a/services/ensemble-service.ts b/services/ensemble-service.ts index 7892a79..04e5746 100644 --- a/services/ensemble-service.ts +++ b/services/ensemble-service.ts @@ -55,6 +55,10 @@ const COMPLETION_SIGNAL_WINDOW_MS = 180_000 // unaffected: a team that is really done still ends within seconds. const SINGLE_SIGNAL_IDLE_THRESHOLD_MS = 480_000 const TWO_SIGNAL_IDLE_THRESHOLD_MS = 300_000 +/** A sign-off is short; long analysis prose that mentions "done" is not one. */ +const MAX_COMPLETION_SIGNAL_LENGTH = 400 +/** Machinery an agent can discuss without meaning "I am finished". */ +const ORCHESTRATION_VOCABULARY = /---STATUS|COLLAB_DONE|team-say|team-read|DONE protocol|\bsentinel\b/i const MIN_MESSAGES_BEFORE_AUTO_DISBAND = 10 // Explicit sentinel: when every active agent sends this exact marker as a full // message, the team auto-disbands immediately — no idle wait, no minimum @@ -127,6 +131,14 @@ interface CompletionSignal { */ function isCompletionStatement(content: string): boolean { if (CLOSING_PATTERNS.some(pattern => pattern.test(content))) return true + // Below this line only the heuristic path remains, so the shape of the message + // matters. A sign-off is short: a long analysis that happens to contain "done" + // is an agent working, not an agent finishing. + if (content.length > MAX_COMPLETION_SIGNAL_LENGTH) return false + // And an agent describing the orchestration is not reporting on itself. Seen on + // a review task pointed at collab-poll.sh: both agents quoted the script's own + // ---STATUS:{ACTIVE,QUIET,DONE,WAITING} sentinel, which ended the run. + if (ORCHESTRATION_VOCABULARY.test(content)) return false if (!COMPLETION_PATTERNS.some(pattern => pattern.test(content))) return false return !CONTINUATION_PATTERNS.some(pattern => pattern.test(content)) } @@ -270,7 +282,21 @@ class EnsembleService { } private shouldAutoDisband(team: EnsembleTeam): boolean { - const messages = getMessages(team.id) + const allMessages = getMessages(team.id) + // Completion counts only from the last message the user sent. `ensemble steer` + // exists so a user can redirect a running team, so a sentinel or sign-off from + // before that redirect no longer describes the work: an agent that had already + // finished is not finished with the new instruction. Without this the team can + // disband while an agent is mid-answer to the user. + const lastUserMessageMs = allMessages + .filter(message => message.from === 'user' && message.timestamp) + .reduce((latest, message) => Math.max(latest, new Date(message.timestamp).getTime() || 0), 0) + const messages = lastUserMessageMs + ? allMessages.filter(message => { + const ts = message.timestamp ? new Date(message.timestamp).getTime() : NaN + return Number.isNaN(ts) || ts >= lastUserMessageMs + }) + : allMessages const nonEnsembleMessages = messages.filter(message => message.from !== 'ensemble') const lastMessage = nonEnsembleMessages[nonEnsembleMessages.length - 1] if (!lastMessage) return false @@ -317,7 +343,7 @@ class EnsembleService { // third agent mid-task. The exact sentinel above is the fast path; these // patterns are only a safety net for teams that go quiet without sending it. if (idleForMs <= TWO_SIGNAL_IDLE_THRESHOLD_MS) return false - if (this.hasTwoRecentCompletionSignals(completionSignals)) return true + if (this.hasRecentCompletionSignalsFromAll(completionSignals, activeAgentNames.size)) return true if (idleForMs <= SINGLE_SIGNAL_IDLE_THRESHOLD_MS) return false return completionSignals.length >= 1 } @@ -326,11 +352,22 @@ class EnsembleService { return isCompletionStatement(content) } - private hasTwoRecentCompletionSignals(signals: CompletionSignal[]): boolean { - for (let i = 0; i < signals.length; i++) { - for (let j = i + 1; j < signals.length; j++) { - if (signals[j].timestamp - signals[i].timestamp > COMPLETION_SIGNAL_WINDOW_MS) break - if (signals[i].agentName !== signals[j].agentName) return true + /** + * True when EVERY active agent produced a completion signal inside one + * COMPLETION_SIGNAL_WINDOW_MS window — the same bar the sentinel path uses. + * Two agents agreeing is not the team agreeing: in a trio that ends the run + * for the third, which is what the comment above this call warns about. + */ + private hasRecentCompletionSignalsFromAll( + signals: CompletionSignal[], activeAgentCount: number, + ): boolean { + if (activeAgentCount < 2) return false + for (let start = 0; start < signals.length; start++) { + const seen = new Set([signals[start].agentName]) + for (let end = start + 1; end < signals.length; end++) { + if (signals[end].timestamp - signals[start].timestamp > COMPLETION_SIGNAL_WINDOW_MS) break + seen.add(signals[end].agentName) + if (seen.size >= activeAgentCount) return true } } return false @@ -1225,4 +1262,6 @@ export const __testing = { SINGLE_SIGNAL_IDLE_THRESHOLD_MS, COMPLETION_PATTERNS, CONTINUATION_PATTERNS, + MAX_COMPLETION_SIGNAL_LENGTH, + ORCHESTRATION_VOCABULARY, } diff --git a/tests/ensemble.test.ts b/tests/ensemble.test.ts index e8469af..fccdf03 100644 --- a/tests/ensemble.test.ts +++ b/tests/ensemble.test.ts @@ -69,6 +69,23 @@ function fillerMessages(count: number, teamId = 'team-1'): EnsembleMessage[] { ) } +/** + * Filler older than the messages a wording-path test cares about. The wording path + * only fires while idle sits between TWO_SIGNAL_IDLE_THRESHOLD_MS and + * SINGLE_SIGNAL_IDLE_THRESHOLD_MS, so the signals under test must be the newest + * messages in the feed. + */ +function olderFiller(count: number, teamId = 'team-1'): EnsembleMessage[] { + return Array.from({ length: count }, (_, i) => + makeMessage({ + teamId, + from: i % 2 === 0 ? 'codex-1' : 'claude-2', + content: `analysis step ${i}`, + timestamp: `2026-03-18T11:50:${String(i).padStart(2, '0')}.000Z`, + }), + ) +} + /** A three-agent team, for checks that must not assume a pair. */ function makeTrioTeam(): EnsembleTeam { return makeTeam({ @@ -318,6 +335,70 @@ describe('shouldAutoDisband() — tested via checkIdleTeams()', () => { expect(appendedMessages.some(m => m.content.includes('Auto-disband'))).toBe(false) }) + it('does NOT auto-disband a trio when only two of three used completion wording', async () => { + // The sentinel path already requires every agent. The wording path did not, so a + // quiet trio could still lose its third agent to the other two agreeing. + const team = makeTrioTeam() + const messages: EnsembleMessage[] = [ + ...olderFiller(8), + makeMessage({ from: 'codex-1', teamId: 'team-1', content: 'Ik ben klaar', timestamp: '2026-03-18T11:58:00.000Z' }), + makeMessage({ from: 'claude-2', teamId: 'team-1', content: 'Ik ben ook klaar', timestamp: '2026-03-18T11:58:30.000Z' }), + ] + + const { mod, appendedMessages } = await setupServiceWithMocks(team, messages) + await mod.checkIdleTeams() + + expect(appendedMessages.some(m => m.content.includes('Auto-disband'))).toBe(false) + }) + + it('auto-disbands a trio once all three used completion wording', async () => { + const team = makeTrioTeam() + const messages: EnsembleMessage[] = [ + ...olderFiller(8), + makeMessage({ from: 'codex-1', teamId: 'team-1', content: 'Ik ben klaar', timestamp: '2026-03-18T11:58:00.000Z' }), + makeMessage({ from: 'claude-2', teamId: 'team-1', content: 'Ik ben ook klaar', timestamp: '2026-03-18T11:58:20.000Z' }), + makeMessage({ from: 'grok-3', teamId: 'team-1', content: 'Ik ben klaar', timestamp: '2026-03-18T11:58:40.000Z' }), + ] + + const { mod, appendedMessages } = await setupServiceWithMocks(team, messages) + await mod.checkIdleTeams() + + expect(appendedMessages.some(m => m.content.includes('Auto-disband'))).toBe(true) + }) + + it('voids done sentinels sent before the user steered the team', async () => { + // An agent that had already signalled is not finished with the new instruction: + // the team must not disband while it answers. + const team = makeTeam() + const messages: EnsembleMessage[] = [ + ...fillerMessages(8), + makeMessage({ from: 'codex-1', teamId: 'team-1', content: '<>', timestamp: '2026-03-18T12:04:20.000Z' }), + makeMessage({ from: 'claude-2', teamId: 'team-1', content: '<>', timestamp: '2026-03-18T12:04:30.000Z' }), + makeMessage({ from: 'user', teamId: 'team-1', content: 'One more thing: check the retry path', timestamp: '2026-03-18T12:04:45.000Z' }), + makeMessage({ from: 'claude-2', teamId: 'team-1', content: 'ack: checking the retry path now', timestamp: '2026-03-18T12:04:50.000Z' }), + ] + + const { mod, appendedMessages } = await setupServiceWithMocks(team, messages) + await mod.checkIdleTeams() + + expect(appendedMessages.some(m => m.content.includes('Auto-disband'))).toBe(false) + }) + + it('still disbands on sentinels sent after the user steered the team', async () => { + const team = makeTeam() + const messages: EnsembleMessage[] = [ + ...fillerMessages(8), + makeMessage({ from: 'user', teamId: 'team-1', content: 'wrap it up please', timestamp: '2026-03-18T12:04:20.000Z' }), + makeMessage({ from: 'codex-1', teamId: 'team-1', content: '<>', timestamp: '2026-03-18T12:04:40.000Z' }), + makeMessage({ from: 'claude-2', teamId: 'team-1', content: '<>', timestamp: '2026-03-18T12:04:50.000Z' }), + ] + + const { mod, appendedMessages } = await setupServiceWithMocks(team, messages) + await mod.checkIdleTeams() + + expect(appendedMessages.some(m => m.content.includes('Auto-disband'))).toBe(true) + }) + it('auto-disbands when two different agents send completion signals and the team went quiet', async () => { const team = makeTeam() const messages: EnsembleMessage[] = [ diff --git a/tests/premature-disband.test.ts b/tests/premature-disband.test.ts index 8aa70d8..c243855 100644 --- a/tests/premature-disband.test.ts +++ b/tests/premature-disband.test.ts @@ -60,3 +60,32 @@ it('idle thresholds leave room for an agent that is reading', () => { ) }) }) + +// A message about the orchestration is not a message about the agent. Seen on a +// review task pointed at scripts/collab-poll.sh: both agents quoted its own +// ---STATUS:{ACTIVE,QUIET,DONE,WAITING} sentinel, and the run ended mid-analysis. +describe('describing the machinery is not finishing', () => { + it('quoting the DONE sentinel of a script under review is not an ending', () => { + assert.strictEqual( + hasCompletionSignal('collab-poll.sh emits TSV lines terminated by a ---STATUS:{ACTIVE,QUIET,DONE,WAITING} sentinel'), + false, + ) + }) + + it('talking about the DONE protocol is not an ending', () => { + assert.strictEqual(hasCompletionSignal('I will follow the DONE protocol once we agree'), false) + }) + + it('announcing a team-say is not an ending', () => { + assert.strictEqual(hasCompletionSignal('I will send the done sentinel via team-say when you agree'), false) + }) + + it('a long analysis that merely contains finishing words is not an ending', () => { + const analysis = 'Exchange 2: the two scripts disagree about what finished means, and the cursor is done differently in each. '.padEnd(600, 'x') + assert.strictEqual(hasCompletionSignal(analysis), false) + }) + + it('but a short genuine sign-off still counts', () => { + assert.strictEqual(hasCompletionSignal('Ik ben klaar'), true) + }) +})