diff --git a/docs/releases/pending/guardrails-coder-mutation-guidance-2758.md b/docs/releases/pending/guardrails-coder-mutation-guidance-2758.md new file mode 100644 index 000000000..1061bb68f --- /dev/null +++ b/docs/releases/pending/guardrails-coder-mutation-guidance-2758.md @@ -0,0 +1,20 @@ +# Correct Stage A coder-mutation guidance + +## What + +Stage A guardrails now distinguish a missing accepted coder mutation from a +genuine attribution-recovery failure, including the audited no-change recovery +path for tasks whose Stage B proof shows that no code change was required. + +## Why + +When a task is `rework_required` because Stage B did not require a code change +and a fresh green `pre_check_batch` proof exists, the architect may use the +audited architect-only `recover_rework_task` path with the exact task ID and +reason. Genuine defects still require coder repair; block only when neither +path applies. Attribution failures retain `/swarm recover` guidance. + +## Migration + +No migration is required. This is an internal guardrail message correction; +workflow persistence and command behavior are unchanged. diff --git a/src/hooks/guardrails/index.ts b/src/hooks/guardrails/index.ts index 158f11efd..8842f5224 100644 --- a/src/hooks/guardrails/index.ts +++ b/src/hooks/guardrails/index.ts @@ -183,13 +183,18 @@ export function emitDurableAttributionAdvisory( } /** - * Stage A workflow-transition write errors that indicate an attribution or - * correlation miss — the post-reset wedge signature. These escalate beyond a - * log line because every silent one is exactly how tasks wedge at - * coder_delegated with no diagnostic. Duplicate transitions never throw - * (isDuplicateTransition returns the existing evidence), so any throw here is - * abnormal; TASK_WORKFLOW_TERMINAL and fencing codes stay warn-only because a - * late gate result after close/settlement is expected churn, not a wedge. + * Stage A workflow-transition write errors are split by the remediation they + * require. Coder-mutation-required is a reducer precondition: the architect + * must dispatch a coder for a real change before another Stage A result can + * be recorded. Stage-A-required is the attribution-recovery category. It is + * defensive at these two Stage A catches (the wrapped stage_a_passed and + * stage_a_failed writes currently cannot throw it), but remains live in the + * Stage B/delegation recovery surfaces. + * + * Duplicate transitions never throw (isDuplicateTransition returns the + * existing evidence), so any throw here is abnormal; TASK_WORKFLOW_TERMINAL + * and fencing codes stay warn-only because a late gate result after + * close/settlement is expected churn, not a wedge. * * `TASK_WORKFLOW_GENERATION_MISMATCH` is deliberately EXCLUDED: it is a CAS * fencing code that fires during ordinary concurrent/parallel-lane operation @@ -198,8 +203,11 @@ export function emitDurableAttributionAdvisory( * turn a routine race into a false "run /swarm recover" advisory during * normal, non-reset operation. */ -export const STAGE_A_ATTRIBUTION_MISS_CODES = new Set([ +export const STAGE_A_CODER_MUTATION_REQUIRED_CODES = new Set([ 'TASK_WORKFLOW_CODER_MUTATION_REQUIRED', +]); + +export const STAGE_A_ATTRIBUTION_MISS_CODES = new Set([ 'TASK_WORKFLOW_STAGE_A_REQUIRED', ]); @@ -1260,7 +1268,15 @@ export function createGuardrailsHooks( emitStageARoute('pre_check_failed', taskId); } catch (err) { const code = stageAWriteErrorCode(err); - if (code && STAGE_A_ATTRIBUTION_MISS_CODES.has(code)) { + if (code && STAGE_A_CODER_MUTATION_REQUIRED_CODES.has(code)) { + logger.criticalWarn( + `[guardrails] Stage A failure write failed for task ${taskId}: ${code}. An accepted coder mutation is required before Stage A can be recorded again. If the task is rework_required because Stage B did not require a code change, the architect-only recover_rework_task path is valid after fresh green pre_check_batch proof; for a genuine defect, dispatch a coder for a real code change, and mark the task blocked only when neither path applies.`, + ); + pushAdvisory( + session, + `STAGE A WRITE FAILED (${code}) for task ${taskId}: pre_check_batch failed but an accepted coder mutation is required before Stage A can be recorded again. If the task is rework_required because Stage B did not require a code change, the architect-only recover_rework_task path is valid after fresh green pre_check_batch proof; for a genuine defect, dispatch a coder for a real code change, and mark the task blocked only when neither path applies.`, + ); + } else if (code && STAGE_A_ATTRIBUTION_MISS_CODES.has(code)) { logger.criticalWarn( `[guardrails] Stage A failure write failed for task ${taskId}: ${code}. Run /swarm recover ${taskId}.`, ); @@ -1313,10 +1329,11 @@ export function createGuardrailsHooks( emitStageARoute('valid_pass', taskId); } catch (err) { // Duplicate transitions return existing evidence without - // throwing, so any error here is abnormal. Attribution-miss + // throwing, so any error here is abnormal. These category-specific // codes are exactly how tasks silently wedge at - // coder_delegated post-reset — escalate them to a visible - // advisory instead of swallowing (TASK_WORKFLOW_TERMINAL and + // coder_delegated post-reset — escalate both category-specific + // routing codes to a visible advisory instead of swallowing + // (TASK_WORKFLOW_TERMINAL and // WAL-fencing codes stay log-only: late gate results after // close/settlement are expected churn, not a wedge). const code = stageAWriteErrorCode(err); @@ -1325,7 +1342,15 @@ export function createGuardrailsHooks( // a late result that must not advance the task. emitStageARoute('late_result', taskId); } - if (code && STAGE_A_ATTRIBUTION_MISS_CODES.has(code)) { + if (code && STAGE_A_CODER_MUTATION_REQUIRED_CODES.has(code)) { + logger.criticalWarn( + `[guardrails] Stage A write failed for task ${taskId}: ${code} — an accepted coder mutation is required before Stage A can be recorded again. If the task is rework_required because Stage B did not require a code change, the architect-only recover_rework_task path is valid after fresh green pre_check_batch proof; for a genuine defect, dispatch a coder for a real code change, and mark the task blocked only when neither path applies.`, + ); + pushAdvisory( + session, + `STAGE A WRITE FAILED (${code}) for task ${taskId}: pre_check_batch passed but an accepted coder mutation is required before Stage A can be recorded again. If the task is rework_required because Stage B did not require a code change, the architect-only recover_rework_task path is valid after fresh green pre_check_batch proof; for a genuine defect, dispatch a coder for a real code change, and mark the task blocked only when neither path applies.`, + ); + } else if (code && STAGE_A_ATTRIBUTION_MISS_CODES.has(code)) { logger.criticalWarn( `[guardrails] Stage A write failed for task ${taskId}: ${code} — pre_check_batch result was NOT attributed. Run /swarm recover ${taskId}.`, ); diff --git a/tests/unit/hooks/guardrails-durable-stage-a-attribution.test.ts b/tests/unit/hooks/guardrails-durable-stage-a-attribution.test.ts index eadf7f2de..6a74a88a7 100644 --- a/tests/unit/hooks/guardrails-durable-stage-a-attribution.test.ts +++ b/tests/unit/hooks/guardrails-durable-stage-a-attribution.test.ts @@ -14,7 +14,7 @@ * of a swallowed warn; * - the normal correlated flow is unchanged. */ -import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import { afterEach, beforeEach, describe, expect, spyOn, test } from 'bun:test'; import type { GuardrailsConfig } from '../../../src/config/schema'; import { getTaskWorkflowSnapshot, @@ -27,6 +27,7 @@ import { resetSwarmState, swarmState, } from '../../../src/state'; +import * as logger from '../../../src/utils/logger'; import { createSafeTestDir } from '../../helpers/safe-test-dir'; // Deterministic fixture instant (explicit-arg Date constructor, not a raw @@ -351,7 +352,7 @@ describe('durable Stage A attribution', () => { ).toBe(true); }); - test('attribution-miss write failure escalates to a visible advisory', async () => { + test('coder-mutation write failure escalates to a visible advisory', async () => { // Correlated task with NO durable coder_delegated evidence: the Stage A // pass transition throws TASK_WORKFLOW_CODER_MUTATION_REQUIRED — the // exact error class previously swallowed into an invisible warn. @@ -378,6 +379,63 @@ describe('durable Stage A attribution', () => { ).toBe(true); }); + test('rework_required pass identifies the required mutation and recovery guidance', async () => { + await transitionTaskWorkflowEvidence(directory, '9.10', { + type: 'accepted_mutation', + agentType: 'coder', + expectedGeneration: 0, + transitionId: 'coder:setup-9.10', + }); + await transitionTaskWorkflowEvidence(directory, '9.10', { + type: 'stage_a_failed', + expectedGeneration: 1, + transitionId: 'stage-a:setup-9.10', + }); + expect( + getTaskWorkflowSnapshot(await readTaskEvidence(directory, '9.10')).state, + ).toBe('rework_required'); + ensureAgentSession('architect').currentTaskId = '9.10'; + + const hooks = createGuardrailsHooks(directory, defaultConfig()); + const criticalWarnSpy = spyOn(logger, 'criticalWarn').mockImplementation( + () => {}, + ); + + try { + await hooks.toolBefore( + { tool: 'pre_check_batch', sessionID: 'architect', callID: 'c5' }, + { args: {} }, + ); + await hooks.toolAfter( + { tool: 'pre_check_batch', sessionID: 'architect', callID: 'c5' }, + { title: '', output: PASS_PAYLOAD, metadata: null }, + ); + + const messages = + swarmState.agentSessions.get('architect')?.pendingAdvisoryMessages ?? + []; + const advisory = messages.find((message) => + message.includes('TASK_WORKFLOW_CODER_MUTATION_REQUIRED'), + ); + expect(advisory).toBeDefined(); + expect(advisory).toMatch(/accepted coder mutation.*before Stage A/); + expect(advisory).toMatch( + /recover_rework_task.*dispatch a coder.*mark the task blocked/, + ); + expect(advisory).not.toContain('NOT attributed'); + expect(advisory).not.toContain('/swarm recover'); + expect(criticalWarnSpy).toHaveBeenCalledTimes(1); + expect(criticalWarnSpy.mock.calls[0]?.[0]).toMatch( + /recover_rework_task.*mark the task blocked/, + ); + expect(criticalWarnSpy.mock.calls[0]?.[0]).not.toMatch( + /NOT attributed|\/swarm recover/, + ); + } finally { + criticalWarnSpy.mockRestore(); + } + }); + test('normal correlated flow is unchanged (no fallback needed)', async () => { await settleTask('3.1'); const session = ensureAgentSession('architect'); diff --git a/tests/unit/hooks/guardrails-stage-a-coder-mutation-failure.test.ts b/tests/unit/hooks/guardrails-stage-a-coder-mutation-failure.test.ts new file mode 100644 index 000000000..6d1c73e42 --- /dev/null +++ b/tests/unit/hooks/guardrails-stage-a-coder-mutation-failure.test.ts @@ -0,0 +1,112 @@ +/** Regression coverage for the failure-side coder-mutation workflow guard. */ +import { afterEach, beforeEach, describe, expect, spyOn, test } from 'bun:test'; +import type { GuardrailsConfig } from '../../../src/config/schema'; +import { + getTaskWorkflowSnapshot, + readTaskEvidence, +} from '../../../src/gate-evidence'; +import { createGuardrailsHooks } from '../../../src/hooks/guardrails'; +import { + ensureAgentSession, + resetSwarmState, + swarmState, +} from '../../../src/state'; +import * as logger from '../../../src/utils/logger'; +import { createSafeTestDir } from '../../helpers/safe-test-dir'; + +const FAIL_PAYLOAD = JSON.stringify({ + gates_passed: false, + total_duration_ms: 1, + batch_status: 'completed', + lint: { ran: true, duration_ms: 1 }, + secretscan: { + ran: true, + duration_ms: 1, + result: { + count: 1, + findings: ['test-secret'], + files_scanned: 1, + incomplete_files: 0, + incomplete_paths: [], + }, + }, + sast_scan: { ran: true, duration_ms: 1, result: { verdict: 'pass' } }, + quality_budget: { ran: false, duration_ms: 0 }, +}); + +function defaultConfig(): GuardrailsConfig { + return { + enabled: true, + max_tool_calls: 200, + max_duration_minutes: 30, + idle_timeout_minutes: 60, + max_repetitions: 10, + max_consecutive_errors: 5, + warning_threshold: 0.75, + }; +} + +let cleanup: () => void; +let directory: string; + +beforeEach(() => { + ({ dir: directory, cleanup } = createSafeTestDir( + 'guardrails-coder-mutation', + )); + resetSwarmState(); +}); + +afterEach(() => { + cleanup(); + resetSwarmState(); +}); + +describe('coder-mutation-required Stage A failure guidance', () => { + test('does not mislabel a failed Stage A write as an attribution miss', async () => { + ensureAgentSession('architect').currentTaskId = '9.11'; + expect( + getTaskWorkflowSnapshot(await readTaskEvidence(directory, '9.11')).state, + ).toBe('idle'); + const hooks = createGuardrailsHooks(directory, defaultConfig()); + const criticalWarnSpy = spyOn(logger, 'criticalWarn').mockImplementation( + () => {}, + ); + + try { + await hooks.toolBefore( + { tool: 'pre_check_batch', sessionID: 'architect', callID: 'c-fail' }, + { args: {} }, + ); + await hooks.toolAfter( + { tool: 'pre_check_batch', sessionID: 'architect', callID: 'c-fail' }, + { title: '', output: FAIL_PAYLOAD, metadata: null }, + ); + + const messages = + swarmState.agentSessions.get('architect')?.pendingAdvisoryMessages ?? + []; + const advisory = messages.find((message) => + message.includes('TASK_WORKFLOW_CODER_MUTATION_REQUIRED'), + ); + expect(advisory).toBeDefined(); + expect(advisory).toMatch(/accepted coder mutation.*before Stage A/); + expect(advisory).toMatch( + /recover_rework_task.*dispatch a coder.*mark the task blocked/, + ); + expect(advisory).not.toContain('NOT attributed'); + expect(advisory).not.toContain('/swarm recover'); + expect(criticalWarnSpy).toHaveBeenCalledTimes(1); + expect(criticalWarnSpy.mock.calls[0]?.[0]).toMatch( + /recover_rework_task.*mark the task blocked/, + ); + expect(criticalWarnSpy.mock.calls[0]?.[0]).not.toContain( + 'NOT attributed', + ); + expect(criticalWarnSpy.mock.calls[0]?.[0]).not.toContain( + '/swarm recover', + ); + } finally { + criticalWarnSpy.mockRestore(); + } + }); +}); diff --git a/tests/unit/hooks/stage-a-attribution-classification-preserving.test.ts b/tests/unit/hooks/stage-a-attribution-classification-preserving.test.ts new file mode 100644 index 000000000..010e8a7d5 --- /dev/null +++ b/tests/unit/hooks/stage-a-attribution-classification-preserving.test.ts @@ -0,0 +1,30 @@ +/** Preserves genuine Stage A attribution/recovery classification semantics. */ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import * as guardrails from '../../../src/hooks/guardrails/index'; +import { STAGE_A_ATTRIBUTION_MISS_CODES } from '../../../src/hooks/guardrails/index'; +import { createSafeTestDir } from '../../helpers/safe-test-dir'; + +let cleanup: () => void; + +beforeEach(() => { + ({ cleanup } = createSafeTestDir('stage-a-attribution-class')); +}); + +afterEach(() => { + cleanup(); +}); + +function optionalCoderMutationCodes(): Set { + const exported = (guardrails as unknown as Record)[ + 'STAGE_A_CODER_MUTATION_REQUIRED_CODES' + ]; + return exported instanceof Set ? new Set(exported as Set) : new Set(); +} + +describe('preserving Stage A attribution classification', () => { + test('keeps TASK_WORKFLOW_STAGE_A_REQUIRED in attribution recovery only', () => { + const attributionCode = 'TASK_WORKFLOW_STAGE_A_REQUIRED'; + expect(STAGE_A_ATTRIBUTION_MISS_CODES.has(attributionCode)).toBe(true); + expect(optionalCoderMutationCodes().has(attributionCode)).toBe(false); + }); +}); diff --git a/tests/unit/hooks/stage-a-error-classification.test.ts b/tests/unit/hooks/stage-a-error-classification.test.ts index 2cbd0cfe0..336b0adca 100644 --- a/tests/unit/hooks/stage-a-error-classification.test.ts +++ b/tests/unit/hooks/stage-a-error-classification.test.ts @@ -3,16 +3,18 @@ * (TASK_WORKFLOW_STAGE_A_REQUIRED post-reset wedge). * * The escalation path in guardrails/index.ts classifies reducer throw codes: - * attribution-miss codes escalate to a visible advisory, everything else stays - * log-only. This test mechanically pins that classification to the actual - * reducer: every TASK_WORKFLOW_* error literal emitted by src/gate-evidence.ts - * must be either explicitly classified or explicitly allowlisted. A new - * reducer error code added without touching the classification fails here, so - * the class cannot silently return as an unclassified swallow. + * attribution-miss and coder-mutation-required codes have distinct visible + * guidance, while everything else stays log-only. This test mechanically pins + * that classification to the actual reducer: every TASK_WORKFLOW_* error + * literal emitted by src/gate-evidence.ts must be either explicitly classified + * or explicitly allowlisted. A new reducer error code added without touching + * the classification fails here, so the class cannot silently return as an + * unclassified swallow. */ import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; import * as fs from 'node:fs'; import * as path from 'node:path'; +import * as guardrails from '../../../src/hooks/guardrails/index'; import { STAGE_A_ATTRIBUTION_MISS_CODES, stageAWriteErrorCode, @@ -47,6 +49,13 @@ const REDUCER_ALLOWLIST = new Set([ 'TASK_WORKFLOW_GENERATION_MISMATCH', ]); +function getCoderMutationCodes(): Set { + const exported = (guardrails as unknown as Record)[ + 'STAGE_A_CODER_MUTATION_REQUIRED_CODES' + ]; + return exported instanceof Set ? new Set(exported as Set) : new Set(); +} + function extractReducerErrorCodes(): string[] { const source = fs.readFileSync( path.resolve(import.meta.dir, '../../../src/gate-evidence.ts'), @@ -61,7 +70,9 @@ function extractReducerErrorCodes(): string[] { describe('Stage A error classification guardrail', () => { test('every reducer workflow error code is classified or allowlisted', () => { + const coderMutationCodes = getCoderMutationCodes(); const classified = new Set([ + ...coderMutationCodes, ...STAGE_A_ATTRIBUTION_MISS_CODES, ...REDUCER_ALLOWLIST, ]); @@ -71,11 +82,34 @@ describe('Stage A error classification guardrail', () => { expect(unclassified).toEqual([]); }); + test('coder-mutation and attribution classifications are explicit and disjoint', () => { + const coderMutationCodes = getCoderMutationCodes(); + expect(coderMutationCodes).toEqual( + new Set(['TASK_WORKFLOW_CODER_MUTATION_REQUIRED']), + ); + expect(STAGE_A_ATTRIBUTION_MISS_CODES).toEqual( + new Set(['TASK_WORKFLOW_STAGE_A_REQUIRED']), + ); + for (const code of coderMutationCodes) { + expect(STAGE_A_ATTRIBUTION_MISS_CODES.has(code)).toBe(false); + } + }); + + test('Stage A required remains the attribution-recovery classification', () => { + const code = stageAWriteErrorCode( + new Error('TASK_WORKFLOW_STAGE_A_REQUIRED: attribution is missing'), + ); + expect(code).toBe('TASK_WORKFLOW_STAGE_A_REQUIRED'); + expect(STAGE_A_ATTRIBUTION_MISS_CODES.has(code as string)).toBe(true); + expect(getCoderMutationCodes().has(code as string)).toBe(false); + }); + test('classification set only contains codes the reducer can actually throw', () => { const reducerCodes = new Set(extractReducerErrorCodes()); - const stale = [...STAGE_A_ATTRIBUTION_MISS_CODES].filter( - (code) => !reducerCodes.has(code), - ); + const stale = [ + ...STAGE_A_ATTRIBUTION_MISS_CODES, + ...getCoderMutationCodes(), + ].filter((code) => !reducerCodes.has(code)); expect(stale).toEqual([]); });