diff --git a/src/agents/agentPair.test.ts b/src/agents/agentPair.test.ts index e8fbfe8..e7987d5 100644 --- a/src/agents/agentPair.test.ts +++ b/src/agents/agentPair.test.ts @@ -17,6 +17,7 @@ import { getSessionHistory, clearAllSessions, calculateConfidence, + isDegenerateWorkerResult, updateConfidenceTracker, needsConfidenceIntervention, getConfidenceSummary, @@ -139,6 +140,36 @@ describe('agentPair', () => { }); }); + describe('isDegenerateWorkerResult (INT-2521)', () => { + const wr = (o: Partial): WorkerResult => ({ + success: true, summary: '', filesChanged: [], commands: [], output: '', ...o, + }); + it('flags a success run with 0 files, 0 commands, empty output', () => { + expect(isDegenerateWorkerResult(wr({ output: '' }))).toBe(true); + expect(isDegenerateWorkerResult(wr({ output: ' \n ' }))).toBe(true); + }); + it('is NOT degenerate when files changed', () => { + expect(isDegenerateWorkerResult(wr({ filesChanged: ['a.ts'] }))).toBe(false); + }); + it('is NOT degenerate when commands ran', () => { + expect(isDegenerateWorkerResult(wr({ commands: ['npm test'] }))).toBe(false); + }); + it('is NOT degenerate on a real analysis run (substantial output)', () => { + expect(isDegenerateWorkerResult(wr({ output: 'x'.repeat(250) }))).toBe(false); + }); + it('is NOT degenerate on a legitimate no-edit outcome with a substantive summary', () => { + // e.g. "no change needed" — the value is in the summary, not a diff. + expect(isDegenerateWorkerResult(wr({ + summary: 'Investigated — no change needed; the bug is already fixed on main, verified by reading auth.ts.', + }))).toBe(false); + }); + it('is NOT degenerate when the worker failed, errored, or halted', () => { + expect(isDegenerateWorkerResult(wr({ success: false }))).toBe(false); + expect(isDegenerateWorkerResult(wr({ error: 'boom' }))).toBe(false); + expect(isDegenerateWorkerResult(wr({ haltReason: 'stuck' }))).toBe(false); + }); + }); + describe('Worker Result Handling', () => { it('should save worker result', () => { const session = createPairSession({ diff --git a/src/agents/agentPair.ts b/src/agents/agentPair.ts index 95294dc..28505d5 100644 --- a/src/agents/agentPair.ts +++ b/src/agents/agentPair.ts @@ -461,6 +461,19 @@ const UNCERTAINTY_WORDS = [ 'seems like', 'appears to', 'not tested', 'untested', ]; +/** + * A degenerate no-op run: the worker reported success but changed 0 files, ran 0 + * commands, AND produced no substantive report (trivial summary AND output) — it + * did nothing. Requiring an empty summary too spares legitimate no-edit outcomes + * (analysis / "no change needed") whose value is in a real summary; only a pure + * empty no-op is flagged. Callers escalate instead of re-revising to STUCK. (INT-2521) + */ +export function isDegenerateWorkerResult(r: WorkerResult): boolean { + return !!r.success && (r.filesChanged?.length ?? 0) === 0 && (r.commands?.length ?? 0) === 0 + && !r.error && !r.haltReason + && (r.summary?.trim().length ?? 0) < 40 && (r.output?.trim().length ?? 0) < 100; +} + /** * Calculate confidence as a 0-100 percentage based on worker result */ diff --git a/src/agents/pairPipeline.test.ts b/src/agents/pairPipeline.test.ts index 03d1106..09a1adc 100644 --- a/src/agents/pairPipeline.test.ts +++ b/src/agents/pairPipeline.test.ts @@ -156,6 +156,27 @@ describe('PairPipeline model selection', () => { expect(runDocumenter).toHaveBeenCalled(); }); + it('a degenerate no-op worker does not pass even when it self-reports high confidence (INT-2521)', async () => { + // 0 files, 0 commands, empty output, but confidencePercent 90 — the degenerate + // check must HALT it independently of the confidence threshold, not fake-pass. + runWorker.mockResolvedValueOnce({ + success: true, summary: 'done', filesChanged: [], commands: [], output: '', confidencePercent: 90, + }); + const { PairPipeline } = await import('./pairPipeline.js'); + const pipeline = new PairPipeline({ + stages: ['worker', 'reviewer'], + maxIterations: 1, + roles: { worker: { enabled: true, timeoutMs: 0 }, reviewer: { enabled: true, timeoutMs: 0 } }, + }); + + const result = await pipeline.run(task(), process.cwd()); + + expect(result.success).toBe(false); + // The degenerate HALT fires before the reviewer — it must NOT silently reach a + // reviewer that could approve the empty diff. + expect(runReviewer).not.toHaveBeenCalled(); + }); + it('falls back to role models when no jobProfile matches', async () => { const { PairPipeline } = await import('./pairPipeline.js'); const pipeline = new PairPipeline({ diff --git a/src/agents/pairPipeline.ts b/src/agents/pairPipeline.ts index 0499832..b09cf47 100644 --- a/src/agents/pairPipeline.ts +++ b/src/agents/pairPipeline.ts @@ -1026,34 +1026,34 @@ export class PairPipeline extends EventEmitter { // ========== HALT CHECK (confidence too low) ========== if (context.workerResult) { const confidence = agentPair.calculateConfidence(context.workerResult); - if (confidence < CONFIDENCE_THRESHOLDS.HALT) { - const haltReason = context.workerResult.haltReason - || `Low confidence: ${confidence}%`; - console.warn(`[${context.taskPrefix}] HALT triggered: confidence=${confidence}%, reason=${haltReason}`); - - this.emit('halt', { - confidence, - haltReason, - sessionId: context.session.id, - iteration: context.currentIteration, - context, - }); - - // Inject revise to retry. Low confidence is a subjective signal, so it - // travels through the reviewer channel (dropped on a fresh-context reset). + // A degenerate no-op HALTs INDEPENDENTLY of self-reported confidence — a + // worker can claim 90% while changing nothing, so it must not slip past. (INT-2521) + const degenerate = agentPair.isDegenerateWorkerResult(context.workerResult); + if (degenerate || confidence < CONFIDENCE_THRESHOLDS.HALT) { + const haltReason = degenerate ? 'Worker produced no changes' + : (context.workerResult.haltReason || `Low confidence: ${confidence}%`); + console.warn(`[${context.taskPrefix}] HALT triggered: confidence=${confidence}%, degenerate=${degenerate}, reason=${haltReason}`); + this.emit('halt', { confidence, haltReason, sessionId: context.session.id, iteration: context.currentIteration, context }); + // Degenerate no-op → escalate the next attempt (stronger effort/model) vs the same empty run to STUCK. (INT-2521) + if (degenerate && !context.workerEscalation) { + context.workerEscalation = buildRepeatEscalation({ + workerCfg: this.config.roles?.worker, + currentIteration: context.currentIteration, + currentModel: this.getModelForRole('worker', context.task), + currentEffort: this.getEffortForTask(context.task), + }); + } context.reviewResult = { decision: 'revise', - feedback: `[HALT] Confidence too low (${confidence}%). ${haltReason}`, - issues: [haltReason], + feedback: degenerate + ? 'Your previous attempt produced ZERO file changes and ran no commands — you did not implement anything. Read the relevant files and make concrete edits now.' + : `[HALT] Confidence too low (${confidence}%). ${haltReason}`, + issues: [degenerate ? 'Worker produced no changes' : haltReason], suggestions: ['Review task requirements', 'Provide additional context', 'Break into sub-tasks'], }; context.feedbackSource = 'review'; agentPair.trackFailure(context.session.id); - this.emit('iteration:fail', { - iteration: context.currentIteration, - stage: 'worker', - context, - }); + this.emit('iteration:fail', { iteration: context.currentIteration, stage: 'worker', context }); agentPair.updateSessionStatus(context.session.id, 'revising'); continue; }