From c826e08824548f06a91e8edf415b198db34b9787 Mon Sep 17 00:00:00 2001 From: Test User Date: Sun, 13 Sep 2026 21:08:51 -0500 Subject: [PATCH 1/3] fix(workflow): architect-only audited exit from rework_required (#2755) --- .../rework-recovery-escape-hatch-2755.md | 29 ++ scripts/check-core-events-usage.ts | 4 + scripts/retention-registry.data.ts | 2 +- src/gate-evidence.ts | 14 +- src/hooks/delegation-gate.ts | 13 +- src/tools/index.ts | 4 + src/tools/manifest.ts | 2 + src/tools/recover-rework-task.ts | 83 +++++ src/tools/tool-metadata.ts | 5 + src/workflow/rework-recovery.ts | 177 ++++++++++ src/workflow/stage-a-repair.ts | 4 +- .../rework-recovery-supervised.test.ts | 183 +++++++++++ ...delegation-gate-rework-remediation.test.ts | 128 ++++++++ tests/unit/tools/recover-rework-task.test.ts | 223 +++++++++++++ tests/unit/workflow/rework-recovery.test.ts | 306 ++++++++++++++++++ 15 files changed, 1172 insertions(+), 5 deletions(-) create mode 100644 docs/releases/pending/rework-recovery-escape-hatch-2755.md create mode 100644 src/tools/recover-rework-task.ts create mode 100644 src/workflow/rework-recovery.ts create mode 100644 tests/unit/gate-evidence/rework-recovery-supervised.test.ts create mode 100644 tests/unit/hooks/delegation-gate-rework-remediation.test.ts create mode 100644 tests/unit/tools/recover-rework-task.test.ts create mode 100644 tests/unit/workflow/rework-recovery.test.ts diff --git a/docs/releases/pending/rework-recovery-escape-hatch-2755.md b/docs/releases/pending/rework-recovery-escape-hatch-2755.md new file mode 100644 index 000000000..acb128f3e --- /dev/null +++ b/docs/releases/pending/rework-recovery-escape-hatch-2755.md @@ -0,0 +1,29 @@ +# rework_required gains an architect-only audited exit (issue #2755) + +- **New `recover_rework_task` tool (architect-only).** A task stranded at + `rework_required` by a Stage B verdict that did not require a code change + (e.g. a SKIPPED verdict scored from a tool-argument mistake) can now be + returned to Stage B dispatch without re-running the coder. The tool writes + a supervised `stage_a_passed` for the task's current generation and appends + a `stage_a_repair` audit event (action `rework_recovered`, with the + sanitized reason) to `.swarm/events.jsonl`; the durable transition id is + prefixed `rework-recovery:` so a supervised recovery stays distinguishable + from a mechanical Stage A pass. +- **Fail-closed preconditions.** Requires the active session to be the + architect, the exact plan task id, durable workflow state exactly + `rework_required`, and green pre-check proof for the wedged generation + (both secretscan and sast_scan bundles green and newer than the failure + transition — the same #2665 bar the Stage A wedge repair uses). Every + refusal is a distinct typed error (`RECOVER_REWORK_*`, `PLAN_*`). +- **Mechanical guardrail unchanged.** The reducer admits `stage_a_passed` + from `rework_required` only when the new `supervisedRecovery` event flag is + set, which only this tool sets. The guardrails recorder and the + `/swarm recover` wedge scan are untouched: a genuine code defect still + requires an accepted coder mutation before Stage A passes again, and the + automatic repair scan still skips `rework_required` tasks. +- **Truthful dispatch remediation.** The `TASK_WORKFLOW_STAGE_A_REQUIRED` + refusal for reviewer/test_engineer dispatch now branches on state: from + `rework_required` it names `recover_rework_task` (and the coder repair loop + for real defects) instead of the human-only `/swarm recover`; the + attribution-wedge guidance (`coder_delegated` after `/swarm reset-session`) + keeps the `/swarm recover` advice where it actually applies. diff --git a/scripts/check-core-events-usage.ts b/scripts/check-core-events-usage.ts index bdf8bf65f..96bffb9c7 100644 --- a/scripts/check-core-events-usage.ts +++ b/scripts/check-core-events-usage.ts @@ -94,6 +94,10 @@ export const CORE_EVENTS_MENTION_ALLOWLIST: Readonly< reason: 'tool description/help strings describing where the audit event lands (documentation text, no I/O) — mirrors the approve-plan-critic entry for the #2703 sibling gate', cls: 'prompt-doc', }, + 'src/tools/recover-rework-task.ts': { + reason: 'tool description/result strings describing where the audit event lands (documentation text, no I/O — the append itself goes through appendStageARepairEvent/appendCoreEventSync) — the #2755 sibling of the approve-* entries', + cls: 'prompt-doc', + }, 'src/services/diagnose-service.ts': { reason: 'user-facing diagnostic output strings naming the checked store (the reads themselves go through the seam API)', cls: 'prompt-doc', diff --git a/scripts/retention-registry.data.ts b/scripts/retention-registry.data.ts index e16d182fa..6da852bf0 100644 --- a/scripts/retention-registry.data.ts +++ b/scripts/retention-registry.data.ts @@ -1693,7 +1693,7 @@ export const RETENTION_REGISTRY: readonly RetentionRow[] = [ bound: 'retryHistory ≤3 (schema :303); per-task file; evidence/ archived+cleaned at close', scope: 'per-key', keyspaceBound: - 'FINITE BY REAPER, not by key domain: one key per taskId — a flat .swarm/evidence/{taskId}.json (src/gate-evidence.ts:786 getEvidencePath) whose taskId is only shape-validated (src/validation/task-id.ts:69-114), so the domain is open. The GLOBAL deleter is the same one the task-evidence-trajectory row cites: "evidence" is in ACTIVE_STATE_DIRS_TO_CLEAN (src/commands/close/constants.ts:253-269) and the close clean loop recursively removes the whole tree (src/commands/close/clean-stage.ts:176-190), taking every {taskId}.json with it. Note the per-file retryHistory ≤3 cap is NOT the keyspace bound — it caps one key\'s history and says nothing about how many keys exist. CAVEAT: archive-first-gated (src/commands/close/clean-stage.ts:176-185) and untouched by /swarm reset and /swarm reset-session, so an unclosed session holds one file per distinct taskId.', + 'FINITE BY REAPER, not by key domain: one key per taskId — a flat .swarm/evidence/{taskId}.json (src/gate-evidence.ts:798 getEvidencePath) whose taskId is only shape-validated (src/validation/task-id.ts:69-114), so the domain is open. The GLOBAL deleter is the same one the task-evidence-trajectory row cites: "evidence" is in ACTIVE_STATE_DIRS_TO_CLEAN (src/commands/close/constants.ts:253-269) and the close clean loop recursively removes the whole tree (src/commands/close/clean-stage.ts:176-190), taking every {taskId}.json with it. Note the per-file retryHistory ≤3 cap is NOT the keyspace bound — it caps one key\'s history and says nothing about how many keys exist. CAVEAT: archive-first-gated (src/commands/close/clean-stage.ts:176-185) and untouched by /swarm reset and /swarm reset-session, so an unclosed session holds one file per distinct taskId.', citation: 'src/gate-evidence.ts:303; src/commands/close/constants.ts:253-269 ACTIVE_STATE_DIRS_TO_CLEAN', }, readBound: { pattern: 'full-file', bound: 'single per-task JSON', sync: true, citation: 'src/gate-evidence.ts:1054-1089' }, diff --git a/src/gate-evidence.ts b/src/gate-evidence.ts index 546b21cf1..e6c8a0828 100644 --- a/src/gate-evidence.ts +++ b/src/gate-evidence.ts @@ -241,6 +241,14 @@ export type TaskWorkflowTransitionEvent = } | { type: 'stage_a_passed'; + /** + * Architect-supervised recovery only (issue #2755): the recover_rework_task + * tool sets this to admit stage_a_passed from rework_required when the + * Stage B verdict did not require a code change. Mechanical emitters + * (the guardrails recorder, stage-a-repair) never set it, so they still + * fail closed with TASK_WORKFLOW_CODER_MUTATION_REQUIRED from that state. + */ + supervisedRecovery?: boolean; expectedGeneration: number; transitionId?: string; } @@ -647,7 +655,11 @@ export function reduceTaskWorkflowSnapshot( case 'stage_a_passed': if ( current.state !== 'coder_delegated' && - current.state !== 'pre_check_passed' + current.state !== 'pre_check_passed' && + !( + current.state === 'rework_required' && + event.supervisedRecovery === true + ) ) { throw new Error( `TASK_WORKFLOW_CODER_MUTATION_REQUIRED: cannot pass Stage A from ${current.state}`, diff --git a/src/hooks/delegation-gate.ts b/src/hooks/delegation-gate.ts index 655ec8f57..2851e0a7c 100644 --- a/src/hooks/delegation-gate.ts +++ b/src/hooks/delegation-gate.ts @@ -4455,10 +4455,21 @@ export function createDelegationGateHook( ); return; } + // Two distinct remediation fragments (never merged into one + // literal): rework_required has an architect-legal autonomous + // exit (issue #2755), every other pre-Stage-A state still gets + // the attribution-wedge guidance where /swarm recover applies. + const stageARemediation = + workflow.state === 'rework_required' + ? 'Remediation: if the Stage B verdict did not require a code change, call recover_rework_task for task ' + + taskId + + ' (architect-only, audited; requires a green pre_check_batch run taken after the verdict), then re-dispatch. ' + + 'Otherwise delegate the coder to repair the code, re-run pre_check_batch, and re-dispatch once stage_a_passed has fired.' + : `Remediation: run pre_check_batch on the task's changed files first. If pre_check_batch passes but the task remains coder_delegated (typical after /swarm reset-session), run /swarm recover ${taskId} to repair Stage A attribution, then re-dispatch.`; throw new Error( `TASK_WORKFLOW_STAGE_A_REQUIRED: cannot dispatch ${targetAgent} for task ${taskId} from ${workflow.state}. ` + `Stage B (${targetAgent}) requires the task to be at pre_check_passed (or later) — a state written only by the stage_a_passed transition, which is emitted when pre_check_batch completes with the task correctly attributed. ` + - `Remediation: run pre_check_batch on the task's changed files first. If pre_check_batch passes but the task remains coder_delegated (typical after /swarm reset-session), run /swarm recover ${taskId} to repair Stage A attribution, then re-dispatch.` + + stageARemediation + (targetAgent === 'reviewer' || targetAgent === 'test_engineer' ? ` For PR-review re-entry outside the task workflow, issue a one-use authorization with authorize_pr_review_reentry immediately before the Task dispatch.` : ''), diff --git a/src/tools/index.ts b/src/tools/index.ts index ed6a9137a..170458bae 100644 --- a/src/tools/index.ts +++ b/src/tools/index.ts @@ -245,6 +245,10 @@ export { executeRecordRecurrenceSweep, record_recurrence_sweep, } from './record-recurrence-sweep'; +export { + executeRecoverReworkTask, + recover_rework_task, +} from './recover-rework-task'; export { executeRunPrFeedbackStageA, run_pr_feedback_stage_a, diff --git a/src/tools/manifest.ts b/src/tools/manifest.ts index b9cec0668..c99cb72fa 100644 --- a/src/tools/manifest.ts +++ b/src/tools/manifest.ts @@ -108,6 +108,7 @@ import { record_implementation_review } from './record-implementation-review'; import { record_issue_publication } from './record-issue-publication'; import { record_issue_reproduction } from './record-issue-reproduction'; import { record_recurrence_sweep } from './record-recurrence-sweep'; +import { recover_rework_task } from './recover-rework-task'; import { repair_gate_evidence } from './repair-gate-evidence'; import { repair_knowledge_receipt_ledger } from './repair-knowledge-receipt-ledger'; import { repo_map } from './repo-map'; @@ -198,6 +199,7 @@ export const TOOL_MANIFEST = defineHandlers({ submit_pr_review_result: () => submit_pr_review_result, approve_plan_critic: () => approve_plan_critic, approve_retry_sounding_board: () => approve_retry_sounding_board, + recover_rework_task: () => recover_rework_task, prepare_pr_workflow_checkout: () => prepare_pr_workflow_checkout, record_implementation_review: () => record_implementation_review, record_issue_publication: () => record_issue_publication, diff --git a/src/tools/recover-rework-task.ts b/src/tools/recover-rework-task.ts new file mode 100644 index 000000000..05dee8c80 --- /dev/null +++ b/src/tools/recover-rework-task.ts @@ -0,0 +1,83 @@ +import { z } from 'zod'; +import { forceRecoverReworkTask } from '../workflow/rework-recovery.js'; +import { createSwarmTool } from './create-tool.js'; + +const RecoverReworkTaskArgsSchema = z + .object({ + task_id: z + .string() + .trim() + .min(1) + .describe( + 'Exact plan task id of the rework_required task (e.g. "1.1"). Must exist in the current plan and have durable workflow evidence at state rework_required.', + ), + reason: z + .string() + .trim() + .min(1) + .max(500) + .describe( + 'Why the Stage B verdict did not require a code change (e.g. "test_engineer returned SKIPPED for a tool-argument error while pytest passes"). Audited to .swarm/events.jsonl.', + ), + }) + .strict(); + +export async function executeRecoverReworkTask( + args: unknown, + directory: string, + context: { sessionID?: string } = {}, +): Promise { + const parsed = RecoverReworkTaskArgsSchema.safeParse(args); + if (!parsed.success) { + return JSON.stringify({ + success: false, + message: `Invalid recover_rework_task call: ${parsed.error.issues + .map((issue) => `${issue.path.join('.')}: ${issue.message}`) + .join('; ')}`, + }); + } + if (!context.sessionID?.trim()) { + return JSON.stringify({ + success: false, + message: 'recover_rework_task requires an active sessionID', + }); + } + try { + const summary = await forceRecoverReworkTask(directory, context.sessionID, { + taskId: parsed.data.task_id, + reason: parsed.data.reason, + }); + return JSON.stringify({ + success: true, + task_id: summary.taskId, + generation: summary.generation, + state: summary.state, + transition_id: summary.transitionId, + recorded_at: summary.recordedAt, + method: 'supervised_recovery', + message: + `Task ${summary.taskId} recovered from rework_required: a supervised stage_a_passed was written at generation ${summary.generation} ` + + '(transition id ' + + summary.transitionId + + '). Reviewer/test_engineer dispatch is permitted again — re-run the Stage B gates that the failed verdict cleared. ' + + 'The recovery is audited to .swarm/events.jsonl (stage_a_repair action rework_recovered). ' + + 'Use it ONLY when the Stage B verdict did not require a code change; a genuine defect still needs the coder repair loop.', + }); + } catch (error) { + return JSON.stringify({ + success: false, + message: error instanceof Error ? error.message : String(error), + }); + } +} + +export const recover_rework_task: ReturnType = + createSwarmTool({ + description: + 'Recover a task wedged at rework_required when the Stage B verdict did NOT require a code change (issue #2755): writes a supervised stage_a_passed for the current generation so reviewer/test_engineer can be re-dispatched, without re-running the coder. Requires the exact plan task id, an active architect session, durable workflow state exactly rework_required, and green post-failure pre-check evidence (a passing pre_check_batch run after the verdict). Fail-closed otherwise; the mechanical stage_a_passed path still requires an accepted coder mutation. A reason is required and audited to .swarm/events.jsonl (stage_a_repair action rework_recovered). Prefer the coder repair loop when the code actually has a defect.', + args: { + task_id: RecoverReworkTaskArgsSchema.shape.task_id, + reason: RecoverReworkTaskArgsSchema.shape.reason, + }, + execute: executeRecoverReworkTask, + }); diff --git a/src/tools/tool-metadata.ts b/src/tools/tool-metadata.ts index c109cfdef..3a00029dc 100644 --- a/src/tools/tool-metadata.ts +++ b/src/tools/tool-metadata.ts @@ -255,6 +255,11 @@ export const TOOL_METADATA = { 'record a MANUAL critic_sounding_board gate entry to unblock the coder retry circuit-breaker gate when the sounding board returned APPROVED but the mechanical recorder missed it (issue #2703)', agents: ['architect'], }, + recover_rework_task: { + description: + 'recover a task wedged at rework_required when the reviewer/test_engineer verdict did not require a code change: write a supervised stage_a_passed so those gates can be re-dispatched without re-running the coder (issue #2755)', + agents: ['architect'], + }, prepare_pr_workflow_checkout: { description: 'prepare an auditable PR workflow checkout or restore its exact original branch/HEAD and preserved stash after terminal cleanup', diff --git a/src/workflow/rework-recovery.ts b/src/workflow/rework-recovery.ts new file mode 100644 index 000000000..9363e3f75 --- /dev/null +++ b/src/workflow/rework-recovery.ts @@ -0,0 +1,177 @@ +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { stripKnownSwarmPrefix } from '../config/schema.js'; +import { + getTaskWorkflowSnapshot, + readTaskEvidence, + transitionTaskWorkflowEvidence, +} from '../gate-evidence.js'; +import { loadPlanJsonOnly } from '../plan/manager.js'; +import { ensureAgentSession } from '../state.js'; +import { + appendStageARepairEvent, + hasGreenPostSettlementPreCheck, +} from './stage-a-repair.js'; + +export interface ReworkRecoverySummary { + taskId: string; + generation: number; + state: string; + transitionId: string; + recordedAt: string; +} + +/** + * Architect-supervised recovery from `rework_required` (issue #2755). + * + * The mechanical `stage_a_passed` transition deliberately fails closed from + * `rework_required` (`TASK_WORKFLOW_CODER_MUTATION_REQUIRED`): a genuine code + * defect must go back through the coder. But a Stage B verdict that failed + * WITHOUT a code defect (e.g. a tool-argument mistake scored as a verdict — + * sibling issue #2756) strands a task whose code is correct, reviewed, and + * green on Stage A checks, with no agent-legal exit; the only other remedy + * ever offered is the human-only `/swarm recover`, whose repair leg skips + * this state entirely. + * + * This is the audited escape hatch the #2703 precedent + * (`approve_retry_sounding_board`) established for exactly this class: it + * writes the SAME durable transition the mechanical path would have written, + * admitted ONLY by the `supervisedRecovery` flag the reducer accepts from + * `rework_required`, and records a distinguishable `stage_a_repair` audit + * event (action `rework_recovered`). Every precondition fails closed; the + * mechanical path and the `/swarm recover` auto-scan are untouched. + */ +export async function forceRecoverReworkTask( + directory: string, + sessionID: string, + options: { taskId: string; reason?: string }, +): Promise { + // Defense-in-depth mirroring forceRecordRetrySoundingBoardApproval: the + // recover_rework_task tool is registered for the architect only, but + // require the ACTIVE session to be the architect so a non-architect + // context cannot self-unblock the workflow state machine. + const session = ensureAgentSession(sessionID, undefined, directory); + if ( + !session || + !session.agentName || + stripKnownSwarmPrefix(session.agentName) !== 'architect' + ) { + throw new Error( + 'RECOVER_REWORK_ARCHITECT_REQUIRED: recover_rework_task requires an active architect session. ' + + 'The rework escape hatch is architect-only; a coder/reviewer/test_engineer cannot self-unblock a task.', + ); + } + + const taskId = + typeof options.taskId === 'string' ? options.taskId.trim() : ''; + if (!taskId) { + throw new Error( + 'RECOVER_REWORK_UNKNOWN_TASK: recover_rework_task requires the exact plan task id (e.g. "1.1").', + ); + } + + const plan = await loadPlanJsonOnly(directory); + if (!plan) { + // Same missing/corrupt distinction as forceRecordPlanCriticApproval. + const planPath = path.join(directory, '.swarm', 'plan.json'); + if (fs.existsSync(planPath)) { + throw new Error( + 'PLAN_CORRUPT: .swarm/plan.json exists but could not be parsed ' + + '(corrupt or schema-invalid). Repair or re-save the plan before ' + + 'recovering a rework_required task.', + ); + } + throw new Error( + 'PLAN_NOT_FOUND: no .swarm/plan.json — cannot recover a rework_required ' + + 'task without a plan. Save a plan first.', + ); + } + const knownTaskIds = new Set( + plan.phases.flatMap((phase) => phase.tasks.map((task) => task.id)), + ); + if (!knownTaskIds.has(taskId)) { + throw new Error( + `RECOVER_REWORK_UNKNOWN_TASK: task ${taskId} is not in the current plan. Refusing to recover a foreign task id.`, + ); + } + + const evidence = await readTaskEvidence(directory, taskId); + const workflow = getTaskWorkflowSnapshot(evidence); + if (!evidence || !workflow.authoritative) { + throw new Error( + `RECOVER_REWORK_NO_WORKFLOW: no durable task workflow evidence exists for task ${taskId} — there is no rework state to recover.`, + ); + } + if (workflow.state !== 'rework_required') { + throw new Error( + `RECOVER_REWORK_STATE_REQUIRED: task ${taskId} is at ${workflow.state}, not rework_required. ` + + 'This recovery applies only to a task whose Stage B verdict moved it to rework_required without a code change. ' + + 'From coder_delegated, run pre_check_batch; for a genuine code defect, delegate the coder to repair it first.', + ); + } + + // Green pre-check proof for the wedged generation: both a secretscan and a + // sast_scan bundle whose latest entries are green AND newer than the + // transition that wedged the task. `workflow.updatedAt` on rework_required + // IS that wedge moment (the stage_b_failed / stage_a_failed), which is + // strictly newer than the accepted_mutation that created this generation — + // so bundles newer than the wedge are necessarily newer than the mutation. + // Same fail-closed trade-offs as the #2665 wedge repair: a project with + // SAST disabled never persists a sast_scan bundle and cannot use this path. + const wedgeMs = Date.parse(workflow.updatedAt); + if (!Number.isFinite(wedgeMs)) { + throw new Error( + 'RECOVER_REWORK_GREEN_PRECHECK_REQUIRED: cannot prove pre-check recency ' + + `(workflow updatedAt "${workflow.updatedAt}" is not a parseable timestamp). Re-run pre_check_batch on the task's changed files, then retry.`, + ); + } + const greenness = await hasGreenPostSettlementPreCheck(directory, wedgeMs); + if (!greenness.green) { + throw new Error( + `RECOVER_REWORK_GREEN_PRECHECK_REQUIRED: refusing to mark Stage A passed without proof (${greenness.reason}). ` + + "Re-run pre_check_batch on the task's changed files and retry once it is green.", + ); + } + + const sanitizedReason = + typeof options.reason === 'string' && options.reason.trim().length > 0 + ? options.reason.trim().slice(0, 500) + : undefined; + const recordedAt = new Date().toISOString(); + // Deterministic per generation: a repeat call after success can never reach + // this line (the state precondition above fails closed on pre_check_passed). + const transitionId = `rework-recovery:${taskId}:gen${workflow.generation}`; + + // The supervised stage_a_passed the mechanical path refuses to emit from + // rework_required. expectedGeneration fails the write closed if a + // concurrent accepted_mutation/repair_idle rotated the generation between + // the read above and this write. + const updated = await transitionTaskWorkflowEvidence(directory, taskId, { + type: 'stage_a_passed', + supervisedRecovery: true, + expectedGeneration: workflow.generation, + transitionId, + }); + const updatedWorkflow = getTaskWorkflowSnapshot(updated); + + // Best-effort audit event through the shared #2665 stage_a_repair wrapper + // (appendCoreEventSync seam, criticalWarn on failure): the durable + // transition above is authoritative; this event is the human-readable + // trail distinguishing a supervised recovery from a mechanical pass. + await appendStageARepairEvent(directory, { + action: 'rework_recovered', + taskId, + transitionId, + generation: workflow.generation, + sessionId: sessionID, + ...(sanitizedReason ? { reason: sanitizedReason } : {}), + }); + + return { + taskId, + generation: updatedWorkflow.generation, + state: updatedWorkflow.state, + transitionId, + recordedAt, + }; +} diff --git a/src/workflow/stage-a-repair.ts b/src/workflow/stage-a-repair.ts index ba28dbae7..dd5915942 100644 --- a/src/workflow/stage-a-repair.ts +++ b/src/workflow/stage-a-repair.ts @@ -51,7 +51,7 @@ export interface StageARepairResult { * failure surfaced via criticalWarn so a silently missing audit line is * visible. */ -async function appendStageARepairEvent( +export async function appendStageARepairEvent( directory: string, payload: Record, ): Promise { @@ -105,7 +105,7 @@ export type PreCheckGreennessResult = * persists its bundle under bucket `sast_scan` (see `src/tools/sast-scan.ts`) * with individual entries tagged `type: 'sast'`. */ -async function hasGreenPostSettlementPreCheck( +export async function hasGreenPostSettlementPreCheck( directory: string, settledAfterMs: number | null, ): Promise { diff --git a/tests/unit/gate-evidence/rework-recovery-supervised.test.ts b/tests/unit/gate-evidence/rework-recovery-supervised.test.ts new file mode 100644 index 000000000..bd477b451 --- /dev/null +++ b/tests/unit/gate-evidence/rework-recovery-supervised.test.ts @@ -0,0 +1,183 @@ +import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { mkdtempSync, rmSync } from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { + readTaskEvidenceRaw, + transitionTaskWorkflowEvidence, +} from '../../../src/gate-evidence'; +import { canonicalTmpDir } from '../../helpers/tmpdir.js'; + +/** + * Issue #2755: the `stage_a_passed` reducer case accepts `rework_required` + * ONLY when the event carries `supervisedRecovery: true` (the + * recover_rework_task escape hatch). The mechanical path — the guardrails + * toolAfter recorder and the stage-a-repair wedge scan, neither of which sets + * the flag — must still fail closed with + * TASK_WORKFLOW_CODER_MUTATION_REQUIRED so a genuine code defect keeps + * requiring an accepted coder mutation. + */ +describe('gate-evidence supervised stage_a_passed (issue #2755)', () => { + let tempDir: string; + + beforeEach(() => { + tempDir = mkdtempSync(path.join(canonicalTmpDir(), 'rework-supervised-')); + }); + + afterEach(() => { + rmSync(tempDir, { recursive: true, force: true }); + }); + + async function seedReworkRequired(taskId: string): Promise { + await transitionTaskWorkflowEvidence(tempDir, taskId, { + type: 'accepted_mutation', + agentType: 'coder', + expectedGeneration: 0, + transitionId: 'mut-1', + }); + await transitionTaskWorkflowEvidence(tempDir, taskId, { + type: 'stage_a_passed', + expectedGeneration: 1, + transitionId: 'stage-a-1', + }); + await transitionTaskWorkflowEvidence(tempDir, taskId, { + type: 'stage_b_failed', + gate: 'test_engineer', + expectedGeneration: 1, + transitionId: 'stage-b-fail-1', + }); + } + + it('admits a supervised stage_a_passed from rework_required without rotating the generation', async () => { + await seedReworkRequired('1.1'); + await transitionTaskWorkflowEvidence(tempDir, '1.1', { + type: 'stage_a_passed', + supervisedRecovery: true, + expectedGeneration: 1, + transitionId: 'rework-recovery:1.1:gen1', + }); + const evidence = readTaskEvidenceRaw(tempDir, '1.1'); + expect(evidence?.workflow).toMatchObject({ + state: 'pre_check_passed', + generation: 1, + lastOutcome: 'stage_a_passed', + lastTransitionId: 'rework-recovery:1.1:gen1', + }); + }); + + it('preserves retry history across the supervised recovery', async () => { + await seedReworkRequired('1.2'); + const before = readTaskEvidenceRaw(tempDir, '1.2')?.workflow; + expect(before?.state).toBe('rework_required'); + expect(before?.retryCount).toBe(1); + await transitionTaskWorkflowEvidence(tempDir, '1.2', { + type: 'stage_a_passed', + supervisedRecovery: true, + expectedGeneration: before?.generation ?? 1, + transitionId: 'rework-recovery:1.2:gen1', + }); + const after = readTaskEvidenceRaw(tempDir, '1.2')?.workflow; + // A spurious-verdict history must stay visible to the retry circuit + // (plan-critic R1 semantics): no special clearing on the supervised path. + expect(after?.retryCount).toBe(before?.retryCount); + expect(after?.retryHistory).toEqual(before?.retryHistory); + expect(after?.retryEpoch).toBe(before?.retryEpoch); + }); + + it('still rejects the mechanical stage_a_passed from rework_required (regression: issue #2755 dead end preserved by design)', async () => { + await seedReworkRequired('1.3'); + await expect( + transitionTaskWorkflowEvidence(tempDir, '1.3', { + type: 'stage_a_passed', + expectedGeneration: 1, + transitionId: 'mechanical-retry', + }), + ).rejects.toThrow(/TASK_WORKFLOW_CODER_MUTATION_REQUIRED/); + expect(readTaskEvidenceRaw(tempDir, '1.3')?.workflow?.state).toBe( + 'rework_required', + ); + }); + + it('rejects a supervised stage_a_passed from every state that is not rework_required/coder_delegated/pre_check_passed', async () => { + // idle: nothing has happened yet. + await expect( + transitionTaskWorkflowEvidence(tempDir, '1.4', { + type: 'stage_a_passed', + supervisedRecovery: true, + expectedGeneration: 0, + transitionId: 'supervised-from-idle', + }), + ).rejects.toThrow(/TASK_WORKFLOW_CODER_MUTATION_REQUIRED/); + + // tests_run: Stage B already completed; recovery does not apply. + await seedReworkRequired('1.5'); + await transitionTaskWorkflowEvidence(tempDir, '1.5', { + type: 'stage_a_passed', + supervisedRecovery: true, + expectedGeneration: 1, + transitionId: 'rr', + }); + await transitionTaskWorkflowEvidence(tempDir, '1.5', { + type: 'stage_b_completed', + gate: 'reviewer', + sessionId: 'rev-session', + routeComplete: true, + expectedGeneration: 1, + transitionId: 'rev-done', + }); + expect(readTaskEvidenceRaw(tempDir, '1.5')?.workflow?.state).toBe( + 'tests_run', + ); + await expect( + transitionTaskWorkflowEvidence(tempDir, '1.5', { + type: 'stage_a_passed', + supervisedRecovery: true, + expectedGeneration: 1, + transitionId: 'supervised-from-tests-run', + }), + ).rejects.toThrow(/TASK_WORKFLOW_CODER_MUTATION_REQUIRED/); + }); + + it('rejects a supervised stage_a_passed from terminal states', async () => { + await seedReworkRequired('1.6'); + await transitionTaskWorkflowEvidence(tempDir, '1.6', { + type: 'task_blocked', + expectedGeneration: 1, + transitionId: 'blocked-1', + }); + await expect( + transitionTaskWorkflowEvidence(tempDir, '1.6', { + type: 'stage_a_passed', + supervisedRecovery: true, + expectedGeneration: 1, + transitionId: 'supervised-from-blocked', + }), + ).rejects.toThrow(/TASK_WORKFLOW_TERMINAL/); + }); + + it('still admits the plain stage_a_passed from coder_delegated and pre_check_passed', async () => { + await transitionTaskWorkflowEvidence(tempDir, '1.7', { + type: 'accepted_mutation', + agentType: 'coder', + expectedGeneration: 0, + transitionId: 'mut-a', + }); + await transitionTaskWorkflowEvidence(tempDir, '1.7', { + type: 'stage_a_passed', + expectedGeneration: 1, + transitionId: 'stage-a-plain', + }); + expect(readTaskEvidenceRaw(tempDir, '1.7')?.workflow?.state).toBe( + 'pre_check_passed', + ); + // Idempotent re-fire from pre_check_passed stays legal (unchanged behavior). + await transitionTaskWorkflowEvidence(tempDir, '1.7', { + type: 'stage_a_passed', + expectedGeneration: 1, + transitionId: 'stage-a-plain-2', + }); + expect(readTaskEvidenceRaw(tempDir, '1.7')?.workflow?.state).toBe( + 'pre_check_passed', + ); + }); +}); diff --git a/tests/unit/hooks/delegation-gate-rework-remediation.test.ts b/tests/unit/hooks/delegation-gate-rework-remediation.test.ts new file mode 100644 index 000000000..be72bc216 --- /dev/null +++ b/tests/unit/hooks/delegation-gate-rework-remediation.test.ts @@ -0,0 +1,128 @@ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs/promises'; +import type { PluginConfig } from '../../../src/config'; +import { closeAllProjectDbs } from '../../../src/db/project-db.js'; +import { transitionTaskWorkflowEvidence } from '../../../src/gate-evidence'; +import { createDelegationGateHook } from '../../../src/hooks/delegation-gate'; +import { resetSwarmState } from '../../../src/state'; +import { createIsolatedTestEnv } from '../../helpers/isolated-test-env.js'; +import { canonicalMkdtemp } from '../../helpers/tmpdir.js'; + +const config = { + max_iterations: 5, + qa_retry_limit: 3, + inject_phase_reminders: true, + hooks: { delegation_gate: true }, +} as PluginConfig; + +let tmpDir = ''; +let isolatedEnv: ReturnType | undefined; + +beforeEach(async () => { + isolatedEnv = createIsolatedTestEnv(); + resetSwarmState(); + tmpDir = canonicalMkdtemp('dg-rework-remediation-'); + await fs.mkdir(`${tmpDir}/.swarm`, { recursive: true }); + await fs.mkdir(`${tmpDir}/.opencode`, { recursive: true }); +}); + +afterEach(async () => { + resetSwarmState(); + closeAllProjectDbs(); + await fs.rm(tmpDir, { recursive: true, force: true }); + isolatedEnv?.cleanup(); + isolatedEnv = undefined; +}); + +async function seedCoderDelegated(taskId: string): Promise { + await transitionTaskWorkflowEvidence(tmpDir, taskId, { + type: 'accepted_mutation', + agentType: 'coder', + expectedGeneration: 0, + transitionId: `seed-coder:${taskId}`, + }); +} + +async function seedReworkRequired(taskId: string): Promise { + await transitionTaskWorkflowEvidence(tmpDir, taskId, { + type: 'accepted_mutation', + agentType: 'coder', + expectedGeneration: 0, + transitionId: `seed-coder:${taskId}`, + }); + await transitionTaskWorkflowEvidence(tmpDir, taskId, { + type: 'stage_a_passed', + expectedGeneration: 1, + transitionId: `seed-stage-a:${taskId}`, + }); + await transitionTaskWorkflowEvidence(tmpDir, taskId, { + type: 'stage_b_failed', + gate: 'test_engineer', + expectedGeneration: 1, + transitionId: `seed-stage-b-fail:${taskId}`, + }); +} + +async function dispatchReviewerAndGetError( + callID: string, + agent: 'reviewer' | 'test_engineer', +): Promise { + const hook = createDelegationGateHook(config, tmpDir); + const thrown = await hook + .toolBefore( + { tool: 'Task', sessionID: 'rework-remediation-session', callID }, + { + args: { + subagent_type: agent, + task_id: '1.1', + prompt: + 'TASK: 1.1\nACCEPTANCE: Verify the exact task and report a bound positive verdict.', + }, + }, + ) + .catch((error: unknown) => error); + expect(thrown).toBeInstanceOf(Error); + return thrown as Error; +} + +/** + * Issue #2755 AC4: the TASK_WORKFLOW_STAGE_A_REQUIRED remediation is + * state-specific. The rework_required branch must name the architect-legal + * autonomous move (recover_rework_task) and must NOT offer the human-only + * /swarm recover; the pre-Stage-A attribution-wedge branch keeps the + * /swarm recover guidance where it actually applies. + */ +describe('delegation gate rework_required remediation text (issue #2755)', () => { + test('from rework_required the refusal names recover_rework_task and never /swarm recover', async () => { + await seedReworkRequired('1.1'); + const error = await dispatchReviewerAndGetError( + 'call-rework-reviewer', + 'reviewer', + ); + expect(error.message).toContain('TASK_WORKFLOW_STAGE_A_REQUIRED'); + expect(error.message).toContain('from rework_required'); + expect(error.message).toContain('recover_rework_task'); + expect(error.message).not.toContain('/swarm recover'); + }); + + test('from rework_required the same holds for test_engineer dispatch', async () => { + await seedReworkRequired('1.1'); + const error = await dispatchReviewerAndGetError( + 'call-rework-te', + 'test_engineer', + ); + expect(error.message).toContain('recover_rework_task'); + expect(error.message).not.toContain('/swarm recover'); + }); + + test('from coder_delegated the attribution-wedge guidance with /swarm recover is preserved', async () => { + await seedCoderDelegated('1.1'); + const error = await dispatchReviewerAndGetError( + 'call-wedge-reviewer', + 'reviewer', + ); + expect(error.message).toContain('TASK_WORKFLOW_STAGE_A_REQUIRED'); + expect(error.message).toContain('/swarm recover 1.1'); + expect(error.message).not.toContain('recover_rework_task'); + }); +}); diff --git a/tests/unit/tools/recover-rework-task.test.ts b/tests/unit/tools/recover-rework-task.test.ts new file mode 100644 index 000000000..d70afbe7c --- /dev/null +++ b/tests/unit/tools/recover-rework-task.test.ts @@ -0,0 +1,223 @@ +import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { mkdirSync, writeFileSync } from 'node:fs'; +import * as path from 'node:path'; +import { + getTaskWorkflowSnapshot, + readTaskEvidence, + transitionTaskWorkflowEvidence, +} from '../../../src/gate-evidence'; +import { ensureAgentSession, resetSwarmState } from '../../../src/state'; +import { + executeRecoverReworkTask, + recover_rework_task, +} from '../../../src/tools/recover-rework-task'; +import { createSafeTestDir } from '../../helpers/safe-test-dir'; + +/** + * Issue #2755: the recover_rework_task tool wrapper — argument validation, + * session requirement, JSON contract, and refusal surfacing through the + * registered tool surface. + */ +describe('recover_rework_task tool (issue #2755)', () => { + let directory = ''; + let cleanup = (): void => {}; + + beforeEach(() => { + resetSwarmState(); + ({ dir: directory, cleanup } = createSafeTestDir('recover-rework-tool-')); + }); + + afterEach(() => { + resetSwarmState(); + cleanup(); + }); + + function writeMinimalPlan(taskId: string): void { + const plan = { + schema_version: '1.0.0', + title: 'recover-rework-tool test plan', + swarm: 'local', + current_phase: 1, + phases: [ + { + id: 1, + name: 'Phase 1', + status: 'in_progress', + tasks: [ + { + id: taskId, + phase: 1, + status: 'in_progress', + size: 'small', + description: `recover-rework-tool task ${taskId}`, + depends: [], + files_touched: [], + }, + ], + }, + ], + }; + mkdirSync(path.join(directory, '.swarm'), { recursive: true }); + writeFileSync( + path.join(directory, '.swarm', 'plan.json'), + JSON.stringify(plan, null, 2), + 'utf-8', + ); + } + + async function writeGreenBundles(taskId: string): Promise { + // Derived from the durable wedge anchor (+60s) — deterministic and + // strictly newer than the stage_b_failed that wedged the task. + const workflow = getTaskWorkflowSnapshot( + await readTaskEvidence(directory, taskId), + ); + const fresh = new Date( + Date.parse(workflow.updatedAt) + 60_000, + ).toISOString(); + const bundles = { + secretscan: { + task_id: 'secretscan', + type: 'secretscan', + verdict: 'pass', + summary: 'seeded green secretscan bundle', + findings_count: 0, + files_scanned: 3, + skipped_files: 0, + incomplete_files: 0, + incomplete_paths: [], + }, + sast_scan: { + task_id: 'sast_scan', + type: 'sast', + verdict: 'pass', + summary: 'seeded green sast bundle', + findings: [], + engine: 'tier_a', + files_scanned: 3, + findings_count: 0, + findings_by_severity: { critical: 0, high: 0, medium: 0, low: 0 }, + }, + } as const; + for (const [bucket, entry] of Object.entries(bundles)) { + const dir = path.join(directory, '.swarm', 'evidence', bucket); + mkdirSync(dir, { recursive: true }); + writeFileSync( + path.join(dir, 'evidence.json'), + JSON.stringify( + { + schema_version: '1.0.0', + task_id: bucket, + entries: [{ ...entry, timestamp: fresh, agent: bucket }], + created_at: fresh, + updated_at: fresh, + }, + null, + 2, + ), + 'utf-8', + ); + } + } + + async function seedReworkRequired(taskId: string): Promise { + await transitionTaskWorkflowEvidence(directory, taskId, { + type: 'accepted_mutation', + agentType: 'coder', + expectedGeneration: 0, + transitionId: 'mut-1', + }); + await transitionTaskWorkflowEvidence(directory, taskId, { + type: 'stage_a_passed', + expectedGeneration: 1, + transitionId: 'stage-a-1', + }); + await transitionTaskWorkflowEvidence(directory, taskId, { + type: 'stage_b_failed', + gate: 'test_engineer', + expectedGeneration: 1, + transitionId: 'stage-b-fail-1', + }); + } + + it('rejects invalid arguments with a success:false JSON message', async () => { + const raw = await executeRecoverReworkTask({}, directory, { + sessionID: 's-1', + }); + const parsed = JSON.parse(raw) as { success: boolean; message: string }; + expect(parsed.success).toBe(false); + expect(parsed.message).toContain('Invalid recover_rework_task call'); + + const noReason = await executeRecoverReworkTask( + { task_id: '1.1' }, + directory, + { sessionID: 's-1' }, + ); + expect((JSON.parse(noReason) as { success: boolean }).success).toBe(false); + }); + + it('requires an active sessionID', async () => { + const raw = await executeRecoverReworkTask( + { task_id: '1.1', reason: 'why' }, + directory, + {}, + ); + const parsed = JSON.parse(raw) as { success: boolean; message: string }; + expect(parsed.success).toBe(false); + expect(parsed.message).toContain('requires an active sessionID'); + }); + + it('surfaces a fail-closed refusal through the JSON contract', async () => { + writeMinimalPlan('1.1'); + await seedReworkRequired('1.1'); + // No green bundles seeded: greenness precondition must refuse. + ensureAgentSession('tool-arch-1', 'architect', directory); + const raw = await executeRecoverReworkTask( + { task_id: '1.1', reason: 'not green yet' }, + directory, + { sessionID: 'tool-arch-1' }, + ); + const parsed = JSON.parse(raw) as { success: boolean; message: string }; + expect(parsed.success).toBe(false); + expect(parsed.message).toContain('RECOVER_REWORK_GREEN_PRECHECK_REQUIRED'); + }); + + it('returns the full success contract on the happy path', async () => { + writeMinimalPlan('1.2'); + await seedReworkRequired('1.2'); + await writeGreenBundles('1.2'); + ensureAgentSession('tool-arch-2', 'architect', directory); + const raw = await executeRecoverReworkTask( + { + task_id: '1.2', + reason: 'SKIPPED verdict for a tool-argument error while pytest passes', + }, + directory, + { sessionID: 'tool-arch-2' }, + ); + const parsed = JSON.parse(raw) as { + success: boolean; + task_id: string; + generation: number; + state: string; + transition_id: string; + method: string; + message: string; + }; + expect(parsed.success).toBe(true); + expect(parsed.task_id).toBe('1.2'); + expect(parsed.generation).toBe(1); + expect(parsed.state).toBe('pre_check_passed'); + expect(parsed.transition_id).toBe('rework-recovery:1.2:gen1'); + expect(parsed.method).toBe('supervised_recovery'); + expect(parsed.message).toContain( + 'Reviewer/test_engineer dispatch is permitted again', + ); + expect(parsed.message).toContain('rework_recovered'); + }); + + it('is registered with an architect-only grant and a two-arg execute (invariant 11)', () => { + expect(recover_rework_task.description).toContain('rework_required'); + expect(typeof recover_rework_task.execute).toBe('function'); + expect(recover_rework_task.execute.length).toBeGreaterThanOrEqual(2); + }); +}); diff --git a/tests/unit/workflow/rework-recovery.test.ts b/tests/unit/workflow/rework-recovery.test.ts new file mode 100644 index 000000000..1bc087e20 --- /dev/null +++ b/tests/unit/workflow/rework-recovery.test.ts @@ -0,0 +1,306 @@ +import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { + existsSync, + mkdirSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import * as path from 'node:path'; +import { resetSwarmState } from '../../../src/state'; +import { forceRecoverReworkTask } from '../../../src/workflow/rework-recovery'; +import { createSafeTestDir } from '../../helpers/safe-test-dir'; + +/** + * Issue #2755: forceRecoverReworkTask is the architect-only audited escape + * hatch from `rework_required`. Every precondition fails closed with a + * distinct error; the success path writes a supervised stage_a_passed and a + * distinguishable stage_a_repair audit event. + */ +describe('forceRecoverReworkTask (issue #2755)', () => { + let directory = ''; + let cleanup = (): void => {}; + + beforeEach(() => { + resetSwarmState(); + ({ dir: directory, cleanup } = createSafeTestDir('rework-recovery-')); + }); + + afterEach(() => { + resetSwarmState(); + cleanup(); + }); + + function writeMinimalPlan(taskId: string): void { + const plan = { + schema_version: '1.0.0', + title: 'rework-recovery test plan', + swarm: 'local', + current_phase: 1, + phases: [ + { + id: 1, + name: 'Phase 1', + status: 'in_progress', + tasks: [ + { + id: taskId, + phase: 1, + status: 'in_progress', + size: 'small', + description: `rework-recovery task ${taskId}`, + depends: [], + files_touched: [], + }, + ], + }, + ], + }; + mkdirSync(path.join(directory, '.swarm'), { recursive: true }); + writeFileSync( + path.join(directory, '.swarm', 'plan.json'), + JSON.stringify(plan, null, 2), + 'utf-8', + ); + } + + function writeBundle( + bucket: 'secretscan' | 'sast_scan', + timestamp: string, + ): void { + const entry = + bucket === 'secretscan' + ? { + task_id: bucket, + type: 'secretscan', + timestamp, + agent: bucket, + verdict: 'pass', + summary: 'seeded green secretscan bundle', + findings_count: 0, + files_scanned: 3, + skipped_files: 0, + incomplete_files: 0, + incomplete_paths: [], + } + : { + task_id: bucket, + type: 'sast', + timestamp, + agent: bucket, + verdict: 'pass', + summary: 'seeded green sast bundle', + findings: [], + engine: 'tier_a', + files_scanned: 3, + findings_count: 0, + findings_by_severity: { critical: 0, high: 0, medium: 0, low: 0 }, + }; + const bundle = { + schema_version: '1.0.0', + task_id: bucket, + entries: [entry], + created_at: timestamp, + updated_at: timestamp, + }; + const dir = path.join(directory, '.swarm', 'evidence', bucket); + mkdirSync(dir, { recursive: true }); + writeFileSync( + path.join(dir, 'evidence.json'), + JSON.stringify(bundle, null, 2), + 'utf-8', + ); + } + + async function writeGreenBundles(taskId: string): Promise { + // Strictly newer than the workflow anchor by construction: derive the + // bundle timestamp from the durable wedge transition's updatedAt (+60s) + // instead of the wall clock, so the fixture is deterministic and the + // greenness predicate's recency floor is genuinely crossed. + const gateEvidence = await import('../../../src/gate-evidence'); + const anchor = gateEvidence.getTaskWorkflowSnapshot( + await gateEvidence.readTaskEvidence(directory, taskId), + ).updatedAt; + const fresh = new Date(Date.parse(anchor) + 60_000).toISOString(); + writeBundle('secretscan', fresh); + writeBundle('sast_scan', fresh); + } + + async function seedWorkflow( + taskId: string, + steps: Array<'mutate' | 'stageA' | 'stageBFail'>, + ): Promise { + const { transitionTaskWorkflowEvidence } = await import( + '../../../src/gate-evidence' + ); + let generation = 0; + for (const step of steps) { + if (step === 'mutate') { + await transitionTaskWorkflowEvidence(directory, taskId, { + type: 'accepted_mutation', + agentType: 'coder', + expectedGeneration: generation, + transitionId: `mut-${taskId}-${generation}`, + }); + generation += 1; + } else if (step === 'stageA') { + await transitionTaskWorkflowEvidence(directory, taskId, { + type: 'stage_a_passed', + expectedGeneration: generation, + transitionId: `stage-a-${taskId}-${generation}`, + }); + } else { + await transitionTaskWorkflowEvidence(directory, taskId, { + type: 'stage_b_failed', + gate: 'test_engineer', + expectedGeneration: generation, + transitionId: `stage-b-fail-${taskId}-${generation}`, + }); + } + } + } + + function readEvents(): string { + const eventsPath = path.join(directory, '.swarm', 'events.jsonl'); + return existsSync(eventsPath) ? readFileSync(eventsPath, 'utf-8') : ''; + } + + it('refuses a non-architect session and leaves durable state untouched', async () => { + writeMinimalPlan('1.1'); + await seedWorkflow('1.1', ['mutate', 'stageA', 'stageBFail']); + const { ensureAgentSession } = await import('../../../src/state'); + ensureAgentSession('coder-session-1', 'coder', directory); + await expect( + forceRecoverReworkTask(directory, 'coder-session-1', { + taskId: '1.1', + reason: 'coder cannot self-unblock', + }), + ).rejects.toThrow(/RECOVER_REWORK_ARCHITECT_REQUIRED/); + const { readTaskEvidence, getTaskWorkflowSnapshot } = await import( + '../../../src/gate-evidence' + ); + expect( + getTaskWorkflowSnapshot(await readTaskEvidence(directory, '1.1')).state, + ).toBe('rework_required'); + }); + + it('refuses an unknown task id and a task outside the current plan', async () => { + writeMinimalPlan('1.1'); + const { ensureAgentSession } = await import('../../../src/state'); + ensureAgentSession('arch-1', 'architect', directory); + await expect( + forceRecoverReworkTask(directory, 'arch-1', { taskId: ' ' }), + ).rejects.toThrow(/RECOVER_REWORK_UNKNOWN_TASK/); + await expect( + forceRecoverReworkTask(directory, 'arch-1', { + taskId: '9.9', + reason: 'foreign task', + }), + ).rejects.toThrow(/RECOVER_REWORK_UNKNOWN_TASK/); + }); + + it('refuses without a plan on disk', async () => { + const { ensureAgentSession } = await import('../../../src/state'); + ensureAgentSession('arch-2', 'architect', directory); + await expect( + forceRecoverReworkTask(directory, 'arch-2', { + taskId: '1.1', + reason: 'no plan', + }), + ).rejects.toThrow(/PLAN_NOT_FOUND/); + }); + + it('refuses a task that is not at rework_required', async () => { + writeMinimalPlan('1.2'); + await seedWorkflow('1.2', ['mutate', 'stageA']); + const { ensureAgentSession } = await import('../../../src/state'); + ensureAgentSession('arch-3', 'architect', directory); + await expect( + forceRecoverReworkTask(directory, 'arch-3', { + taskId: '1.2', + reason: 'wrong state', + }), + ).rejects.toThrow( + /RECOVER_REWORK_STATE_REQUIRED: task 1.2 is at pre_check_passed/, + ); + }); + + it('refuses without green pre-check evidence, including the SAST-disabled shape (plan-critic R1)', async () => { + writeMinimalPlan('1.3'); + await seedWorkflow('1.3', ['mutate', 'stageA', 'stageBFail']); + // Secretscan only: a project with SAST disabled never persists a + // sast_scan bundle; the recovery must fail closed (#2665 trade-off). + // The bundle timestamp is derived from the durable wedge anchor + // (deterministic, lint-clean) rather than the wall clock. + const { readTaskEvidence, getTaskWorkflowSnapshot } = await import( + '../../../src/gate-evidence' + ); + const wedgeIso = getTaskWorkflowSnapshot( + await readTaskEvidence(directory, '1.3'), + ).updatedAt; + writeBundle( + 'secretscan', + new Date(Date.parse(wedgeIso) + 60_000).toISOString(), + ); + const { ensureAgentSession } = await import('../../../src/state'); + ensureAgentSession('arch-4', 'architect', directory); + await expect( + forceRecoverReworkTask(directory, 'arch-4', { + taskId: '1.3', + reason: 'green bundles absent', + }), + ).rejects.toThrow( + /RECOVER_REWORK_GREEN_PRECHECK_REQUIRED.*no_pre_check_bundles/, + ); + expect( + getTaskWorkflowSnapshot(await readTaskEvidence(directory, '1.3')).state, + ).toBe('rework_required'); + }); + + it('recovers to pre_check_passed at an unchanged generation with an audited, distinguishable transition', async () => { + writeMinimalPlan('1.4'); + await seedWorkflow('1.4', ['mutate', 'stageA', 'stageBFail']); + await writeGreenBundles('1.4'); + const { ensureAgentSession } = await import('../../../src/state'); + ensureAgentSession('arch-5', 'architect', directory); + const summary = await forceRecoverReworkTask(directory, 'arch-5', { + taskId: '1.4', + reason: 'SKIPPED verdict for a tool-argument error while pytest passes', + }); + expect(summary.state).toBe('pre_check_passed'); + expect(summary.generation).toBe(1); + expect(summary.transitionId).toBe('rework-recovery:1.4:gen1'); + + const { readTaskEvidence, getTaskWorkflowSnapshot } = await import( + '../../../src/gate-evidence' + ); + const snapshot = getTaskWorkflowSnapshot( + await readTaskEvidence(directory, '1.4'), + ); + expect(snapshot.state).toBe('pre_check_passed'); + expect(snapshot.generation).toBe(1); + expect(snapshot.lastTransitionId).toBe('rework-recovery:1.4:gen1'); + expect(snapshot.lastOutcome).toBe('stage_a_passed'); + + const events = readEvents(); + const auditLine = events + .split('\n') + .find( + (line) => + line.includes('"stage_a_repair"') && + line.includes('"rework_recovered"'), + ); + expect(auditLine).toBeDefined(); + expect(auditLine).toContain('"taskId":"1.4"'); + expect(auditLine).toContain('"sessionId":"arch-5"'); + expect(auditLine).toContain('SKIPPED verdict for a tool-argument error'); + + // A repeat call cannot fire again: the state precondition fails closed. + await expect( + forceRecoverReworkTask(directory, 'arch-5', { + taskId: '1.4', + reason: 'second attempt', + }), + ).rejects.toThrow(/RECOVER_REWORK_STATE_REQUIRED/); + }); +}); From c1d78017ef3e83280487882004de7c77b06b15ad Mon Sep 17 00:00:00 2001 From: Test User Date: Mon, 14 Sep 2026 09:10:17 -0500 Subject: [PATCH 2/3] fix(workflow): address PR #2760 review findings (audit honesty, supervised marker, pins, tests) --- .../rework-recovery-escape-hatch-2755.md | 16 ++- scripts/registry-citation-baseline.json | 35 ----- scripts/retention-registry.data.ts | 16 +-- src/commands/recover.ts | 6 + src/commands/reset-session.ts | 5 +- src/gate-evidence.ts | 40 +++++- src/hooks/delegation-gate.ts | 2 +- src/tools/recover-rework-task.ts | 11 +- src/workflow/rework-recovery.ts | 50 ++++++- src/workflow/stage-a-repair.ts | 9 +- .../rework-recovery-supervised.test.ts | 105 ++++++++++++++ tests/unit/tools/recover-rework-task.test.ts | 42 ++++++ tests/unit/workflow/rework-recovery.test.ts | 136 ++++++++++++++++++ 13 files changed, 411 insertions(+), 62 deletions(-) diff --git a/docs/releases/pending/rework-recovery-escape-hatch-2755.md b/docs/releases/pending/rework-recovery-escape-hatch-2755.md index acb128f3e..dd51187f5 100644 --- a/docs/releases/pending/rework-recovery-escape-hatch-2755.md +++ b/docs/releases/pending/rework-recovery-escape-hatch-2755.md @@ -13,8 +13,20 @@ architect, the exact plan task id, durable workflow state exactly `rework_required`, and green pre-check proof for the wedged generation (both secretscan and sast_scan bundles green and newer than the failure - transition — the same #2665 bar the Stage A wedge repair uses). Every - refusal is a distinct typed error (`RECOVER_REWORK_*`, `PLAN_*`). + transition — the same #2665 bar the Stage A wedge repair uses; bundle + recency is global, not correlated to the task's changed files). Every + refusal is a distinct typed error (`RECOVER_REWORK_*`, `PLAN_*`). Known + limitation: a project with `gates.sast_scan.enabled` disabled never + persists a sast_scan bundle, so `recover_rework_task` cannot succeed + there by design — use the coder repair loop instead (or re-enable SAST). +- **Durable distinguishability.** The supervised pass also persists a + `supervisedRecovery` marker in the task's workflow evidence (mirroring the + `forcedCompletion` precedent: preserved across same-generation + transitions, cleared when `repair_idle` opens a new generation), so a + supervised recovery stays distinguishable from a mechanical Stage A pass + even after later transitions overwrite the transition id; the append + outcome of the `.swarm/events.jsonl` audit event is surfaced in the tool + result (`audit_event_recorded`) instead of being asserted. - **Mechanical guardrail unchanged.** The reducer admits `stage_a_passed` from `rework_required` only when the new `supervisedRecovery` event flag is set, which only this tool sets. The guardrails recorder and the diff --git a/scripts/registry-citation-baseline.json b/scripts/registry-citation-baseline.json index 461837d6c..63539b779 100644 --- a/scripts/registry-citation-baseline.json +++ b/scripts/registry-citation-baseline.json @@ -589,41 +589,6 @@ "kind": "out-of-range", "note": "Pre-existing debt, not approved drift: \"writeCouncilEvidence\" exists in src/council/council-evidence-writer.ts but not inside src/council/council-evidence-writer.ts:96 (task-workflow-evidence.writerCitations[1])." }, - { - "rowId": "task-workflow-evidence", - "file": "src/gate-evidence.ts", - "identifier": "readTaskEvidence", - "kind": "out-of-range", - "note": "Pre-existing debt, not approved drift: \"readTaskEvidence\" exists in src/gate-evidence.ts but not inside src/gate-evidence.ts:1054 (task-workflow-evidence.readerCitations[0])." - }, - { - "rowId": "task-workflow-evidence", - "file": "src/gate-evidence.ts", - "identifier": "readTaskEvidenceRaw", - "kind": "out-of-range", - "note": "Pre-existing debt, not approved drift: \"readTaskEvidenceRaw\" exists in src/gate-evidence.ts but not inside :1071 (task-workflow-evidence.readerCitations[0])." - }, - { - "rowId": "task-workflow-evidence", - "file": "src/gate-evidence.ts", - "identifier": "recordAgentDispatch", - "kind": "out-of-range", - "note": "Pre-existing debt, not approved drift: \"recordAgentDispatch\" exists in src/gate-evidence.ts but not inside :1010 (task-workflow-evidence.writerCitations[0])." - }, - { - "rowId": "task-workflow-evidence", - "file": "src/gate-evidence.ts", - "identifier": "recordGateEvidence", - "kind": "out-of-range", - "note": "Pre-existing debt, not approved drift: \"recordGateEvidence\" exists in src/gate-evidence.ts but not inside :956 (task-workflow-evidence.writerCitations[0])." - }, - { - "rowId": "task-workflow-evidence", - "file": "src/gate-evidence.ts", - "identifier": "transitionTaskWorkflowEvidence", - "kind": "out-of-range", - "note": "Pre-existing debt, not approved drift: \"transitionTaskWorkflowEvidence\" exists in src/gate-evidence.ts but not inside src/gate-evidence.ts:879 (task-workflow-evidence.writerCitations[0])." - }, { "rowId": "telemetry-jsonl", "file": "src/services/cost-accounting.ts", diff --git a/scripts/retention-registry.data.ts b/scripts/retention-registry.data.ts index 6da852bf0..6e6383c1b 100644 --- a/scripts/retention-registry.data.ts +++ b/scripts/retention-registry.data.ts @@ -1675,14 +1675,14 @@ export const RETENTION_REGISTRY: readonly RetentionRow[] = [ canonicalRoot: 'project-swarm', writerModules: ['src/gate-evidence.ts', 'src/council/council-evidence-writer.ts'], writerCitations: [ - 'src/gate-evidence.ts:879 transitionTaskWorkflowEvidence / :956 recordGateEvidence / :1010 recordAgentDispatch — locked read-modify-write, atomic write', + 'src/gate-evidence.ts:984 transitionTaskWorkflowEvidence / :1094 recordGateEvidence / :1152 recordAgentDispatch — locked read-modify-write, atomic write', 'src/council/council-evidence-writer.ts:96 writeCouncilEvidence — gates.council section under withTaskEvidenceLock', ], readerCitations: [ - 'src/gate-evidence.ts:1054 readTaskEvidence — FULL-FILE fail-open, async; :1071 readTaskEvidenceRaw — strict, sync', + 'src/gate-evidence.ts:1196 readTaskEvidence — FULL-FILE fail-open, async; :1272 readTaskEvidenceRaw — strict, sync', 'src/council/council-evidence-writer.ts:207 hasCouncilEvidenceAttempt', ], - schemaVersion: 'workflow WAL states; unrecognized states degrade to null (documented :1048-1052)', + schemaVersion: 'workflow WAL states; unrecognized states degrade to null (documented :1183-1188)', stateClass: 'authoritative', privacyClass: 'mixed', directFileExemption: { @@ -1690,15 +1690,15 @@ export const RETENTION_REGISTRY: readonly RetentionRow[] = [ reviewedIssue: 2036, }, writeLimits: { - bound: 'retryHistory ≤3 (schema :303); per-task file; evidence/ archived+cleaned at close', + bound: 'retryHistory ≤3 (schema :347); per-task file; evidence/ archived+cleaned at close', scope: 'per-key', keyspaceBound: - 'FINITE BY REAPER, not by key domain: one key per taskId — a flat .swarm/evidence/{taskId}.json (src/gate-evidence.ts:798 getEvidencePath) whose taskId is only shape-validated (src/validation/task-id.ts:69-114), so the domain is open. The GLOBAL deleter is the same one the task-evidence-trajectory row cites: "evidence" is in ACTIVE_STATE_DIRS_TO_CLEAN (src/commands/close/constants.ts:253-269) and the close clean loop recursively removes the whole tree (src/commands/close/clean-stage.ts:176-190), taking every {taskId}.json with it. Note the per-file retryHistory ≤3 cap is NOT the keyspace bound — it caps one key\'s history and says nothing about how many keys exist. CAVEAT: archive-first-gated (src/commands/close/clean-stage.ts:176-185) and untouched by /swarm reset and /swarm reset-session, so an unclosed session holds one file per distinct taskId.', - citation: 'src/gate-evidence.ts:303; src/commands/close/constants.ts:253-269 ACTIVE_STATE_DIRS_TO_CLEAN', + 'FINITE BY REAPER, not by key domain: one key per taskId — a flat .swarm/evidence/{taskId}.json (src/gate-evidence.ts:832 getEvidencePath) whose taskId is only shape-validated (src/validation/task-id.ts:69-114), so the domain is open. The GLOBAL deleter is the same one the task-evidence-trajectory row cites: "evidence" is in ACTIVE_STATE_DIRS_TO_CLEAN (src/commands/close/constants.ts:253-269) and the close clean loop recursively removes the whole tree (src/commands/close/clean-stage.ts:176-190), taking every {taskId}.json with it. Note the per-file retryHistory ≤3 cap is NOT the keyspace bound — it caps one key\'s history and says nothing about how many keys exist. CAVEAT: archive-first-gated (src/commands/close/clean-stage.ts:176-185) and untouched by /swarm reset and /swarm reset-session, so an unclosed session holds one file per distinct taskId.', + citation: 'src/gate-evidence.ts:347; src/commands/close/constants.ts:253-269 ACTIVE_STATE_DIRS_TO_CLEAN', }, - readBound: { pattern: 'full-file', bound: 'single per-task JSON', sync: true, citation: 'src/gate-evidence.ts:1054-1089' }, + readBound: { pattern: 'full-file', bound: 'single per-task JSON', sync: true, citation: 'src/gate-evidence.ts:1196-1224' }, lockModel: 'withTaskEvidenceLock (evidence/{taskId}.json key) — proper-lockfile, 60 s timeout, backoff+jitter', - crashBehavior: 'atomic write; WAL PREPARED fencing (assertTaskEvidenceWriteAllowed :117)', + crashBehavior: 'atomic write; WAL PREPARED fencing (assertTaskEvidenceWriteAllowed :152)', closePolicy: 'cleaned — evidence/ dir lifecycle', resetPolicy: 'not reset', legacyCompatibility: 'unknown workflow states read as null (graceful degrade)', diff --git a/src/commands/recover.ts b/src/commands/recover.ts index 3cbdbf881..3df56241b 100644 --- a/src/commands/recover.ts +++ b/src/commands/recover.ts @@ -56,6 +56,12 @@ function renderStageARepairOutcome(outcome: StageARepairOutcome): string { case 'repaired': return `✅ Task ${outcome.taskId}: Stage A repaired — stage_a_passed written at generation ${outcome.generation} without re-running the coder`; case 'skipped_not_wedged': + // Issue #2755: rework_required wedges have their own architect-legal + // exit that this scan deliberately does not touch — point at it instead + // of reporting a bare dead end. + if (outcome.state === 'rework_required') { + return `⏭️ Task ${outcome.taskId}: workflow state is rework_required — not a coder_delegated Stage A wedge, nothing to repair here. If the Stage B verdict did not require a code change, have the architect run the recover_rework_task tool for this task; otherwise delegate the coder to repair first.`; + } return `⏭️ Task ${outcome.taskId}: workflow state is ${outcome.state} with pre_check proof present or not settled — nothing to repair`; case 'skipped_not_green': return `⏭️ Task ${outcome.taskId}: no green post-settlement pre-check evidence (${outcome.reason === 'no_pre_check_bundles' ? 'missing or non-green secretscan/SAST bundle — run pre_check_batch first' : 'latest pre-check run failed or predates the settlement'}) — refusing to mark Stage A passed without proof`; diff --git a/src/commands/reset-session.ts b/src/commands/reset-session.ts index 64840ae23..f9ba5fc38 100644 --- a/src/commands/reset-session.ts +++ b/src/commands/reset-session.ts @@ -788,6 +788,9 @@ export async function handleResetSessionCommand( 'intentionally survive this reset. If a task is blocked with ' + 'TASK_RETRY_CRITIC_REQUIRED waiting for a critic_sounding_board ' + 'APPROVED verdict that was already obtained, have the architect ' + - 'record it with the approve_retry_sounding_board tool.', + 'record it with the approve_retry_sounding_board tool. A task left ' + + 'at rework_required also survives this reset; when the Stage B ' + + 'verdict did not require a code change, the architect can exit it ' + + 'with the recover_rework_task tool (issue #2755).', ].join('\n'); } diff --git a/src/gate-evidence.ts b/src/gate-evidence.ts index e6c8a0828..d2f390f97 100644 --- a/src/gate-evidence.ts +++ b/src/gate-evidence.ts @@ -100,6 +100,19 @@ export interface TaskWorkflowMetadata { * the same answer those files would have given anyway. */ forcedCompletion?: boolean; + /** + * True when this task's current generation entered `pre_check_passed` via the + * architect-supervised `recover_rework_task` escape hatch (issue #2755) rather + * than the mechanical pre_check_batch recorder. + * + * Same rationale as `forcedCompletion`: the reducer consumes the event-scoped + * `supervisedRecovery` flag transiently, so without a persisted marker the + * evidence file becomes byte-identical to a mechanically-earned Stage A pass + * as soon as the next transition overwrites `lastTransitionId`. Preserved + * across subsequent transitions in the same generation, cleared by + * `repair_idle` (which opens a new generation for genuinely new work). + */ + supervisedRecovery?: boolean; } export interface TaskWorkflowSnapshot extends TaskWorkflowMetadata { @@ -337,6 +350,7 @@ const TaskWorkflowMetadataSchema = z.object({ lastTransitionId: z.string().min(1).nullable().optional().default(null), updatedAt: z.string(), forcedCompletion: z.boolean().optional(), + supervisedRecovery: z.boolean().optional(), }); const TaskEvidenceSchema = z.object({ @@ -604,6 +618,12 @@ export function reduceTaskWorkflowSnapshot( // completion stays visible downstream. Cleared by repair_idle, which opens a new // generation for genuinely new work. ...(current.forcedCompletion === true ? { forcedCompletion: true } : {}), + // Same durability contract for the supervised Stage A entry mode (issue + // #2755): preserved across same-generation transitions, set by the + // supervised stage_a_passed below, stripped when a new generation opens. + ...(current.supervisedRecovery === true + ? { supervisedRecovery: true } + : {}), }; switch (event.type) { @@ -643,6 +663,8 @@ export function reduceTaskWorkflowSnapshot( retryCount: Math.min(current.retryCount + 1, 3), retryHistory: [...current.retryHistory, outcome].slice(-3), retryEpoch: current.retryEpoch || current.generation + 1, + // New generation: any prior Stage A entry mode no longer applies. + supervisedRecovery: undefined, }; } return { @@ -651,6 +673,7 @@ export function reduceTaskWorkflowSnapshot( state: 'coder_delegated', // A mutation is a repair attempt, not proof that prior rejections were // resolved. Preserve the task-level circuit history across generations. + supervisedRecovery: undefined, }; case 'stage_a_passed': if ( @@ -667,6 +690,12 @@ export function reduceTaskWorkflowSnapshot( } return { ...base, + // Persist the supervised entry mode so the evidence file stays + // distinguishable from a mechanically-earned Stage A pass after + // later transitions overwrite lastTransitionId (issue #2755 review). + ...(event.supervisedRecovery === true + ? { supervisedRecovery: true as const } + : {}), state: 'pre_check_passed', }; case 'stage_a_failed': @@ -750,9 +779,14 @@ export function reduceTaskWorkflowSnapshot( state: 'closed', }; case 'repair_idle': { - // A repair reopens the task for new work, so a prior forced completion no - // longer describes this generation. Drop the field rather than carrying it. - const { forcedCompletion: _cleared, ...withoutForced } = base; + // A repair reopens the task for new work, so prior forced-completion and + // supervised-recovery markers no longer describe this generation. Drop + // them rather than carrying them forward. + const { + forcedCompletion: _cleared, + supervisedRecovery: _clearedMarker, + ...withoutForced + } = base; return { ...withoutForced, generation: current.generation + 1, diff --git a/src/hooks/delegation-gate.ts b/src/hooks/delegation-gate.ts index 2851e0a7c..7cc6f088d 100644 --- a/src/hooks/delegation-gate.ts +++ b/src/hooks/delegation-gate.ts @@ -4463,7 +4463,7 @@ export function createDelegationGateHook( workflow.state === 'rework_required' ? 'Remediation: if the Stage B verdict did not require a code change, call recover_rework_task for task ' + taskId + - ' (architect-only, audited; requires a green pre_check_batch run taken after the verdict), then re-dispatch. ' + + " (architect-only, audited; requires green secretscan AND sast_scan evidence newer than the verdict — normally from a fresh pre_check_batch run; unavailable when SAST is disabled), then re-dispatch. " + 'Otherwise delegate the coder to repair the code, re-run pre_check_batch, and re-dispatch once stage_a_passed has fired.' : `Remediation: run pre_check_batch on the task's changed files first. If pre_check_batch passes but the task remains coder_delegated (typical after /swarm reset-session), run /swarm recover ${taskId} to repair Stage A attribution, then re-dispatch.`; throw new Error( diff --git a/src/tools/recover-rework-task.ts b/src/tools/recover-rework-task.ts index 05dee8c80..bd75d4878 100644 --- a/src/tools/recover-rework-task.ts +++ b/src/tools/recover-rework-task.ts @@ -8,6 +8,7 @@ const RecoverReworkTaskArgsSchema = z .string() .trim() .min(1) + .max(64) .describe( 'Exact plan task id of the rework_required task (e.g. "1.1"). Must exist in the current plan and have durable workflow evidence at state rework_required.', ), @@ -47,6 +48,9 @@ export async function executeRecoverReworkTask( taskId: parsed.data.task_id, reason: parsed.data.reason, }); + const auditNote = summary.auditEventRecorded + ? 'The recovery is audited to .swarm/events.jsonl (stage_a_repair action rework_recovered).' + : 'WARNING: the audit event could NOT be appended to .swarm/events.jsonl (see the plugin log); the durable workflow evidence still records the supervised recovery via the rework-recovery: transition id.'; return JSON.stringify({ success: true, task_id: summary.taskId, @@ -54,14 +58,15 @@ export async function executeRecoverReworkTask( state: summary.state, transition_id: summary.transitionId, recorded_at: summary.recordedAt, + audit_event_recorded: summary.auditEventRecorded, method: 'supervised_recovery', message: `Task ${summary.taskId} recovered from rework_required: a supervised stage_a_passed was written at generation ${summary.generation} ` + '(transition id ' + summary.transitionId + '). Reviewer/test_engineer dispatch is permitted again — re-run the Stage B gates that the failed verdict cleared. ' + - 'The recovery is audited to .swarm/events.jsonl (stage_a_repair action rework_recovered). ' + - 'Use it ONLY when the Stage B verdict did not require a code change; a genuine defect still needs the coder repair loop.', + auditNote + + ' Use it ONLY when the Stage B verdict did not require a code change; a genuine defect still needs the coder repair loop.', }); } catch (error) { return JSON.stringify({ @@ -74,7 +79,7 @@ export async function executeRecoverReworkTask( export const recover_rework_task: ReturnType = createSwarmTool({ description: - 'Recover a task wedged at rework_required when the Stage B verdict did NOT require a code change (issue #2755): writes a supervised stage_a_passed for the current generation so reviewer/test_engineer can be re-dispatched, without re-running the coder. Requires the exact plan task id, an active architect session, durable workflow state exactly rework_required, and green post-failure pre-check evidence (a passing pre_check_batch run after the verdict). Fail-closed otherwise; the mechanical stage_a_passed path still requires an accepted coder mutation. A reason is required and audited to .swarm/events.jsonl (stage_a_repair action rework_recovered). Prefer the coder repair loop when the code actually has a defect.', + 'Recover a task wedged at rework_required when the Stage B verdict did NOT require a code change (issue #2755): writes a supervised stage_a_passed for the current generation so reviewer/test_engineer can be re-dispatched, without re-running the coder. Requires the exact plan task id, an active architect session, durable workflow state exactly rework_required, and green pre-check proof: green secretscan AND sast_scan evidence bundles newer than the failing verdict (normally from a fresh pre_check_batch run; bundle recency is global, not file-correlated; projects with SAST disabled cannot use this path). Fail-closed otherwise; the mechanical stage_a_passed path still requires an accepted coder mutation. A reason is required and audited to .swarm/events.jsonl (stage_a_repair action rework_recovered). Prefer the coder repair loop when the code actually has a defect.', args: { task_id: RecoverReworkTaskArgsSchema.shape.task_id, reason: RecoverReworkTaskArgsSchema.shape.reason, diff --git a/src/workflow/rework-recovery.ts b/src/workflow/rework-recovery.ts index 9363e3f75..d93e6fab2 100644 --- a/src/workflow/rework-recovery.ts +++ b/src/workflow/rework-recovery.ts @@ -3,10 +3,11 @@ import * as path from 'node:path'; import { stripKnownSwarmPrefix } from '../config/schema.js'; import { getTaskWorkflowSnapshot, - readTaskEvidence, + readTaskEvidenceState, transitionTaskWorkflowEvidence, } from '../gate-evidence.js'; import { loadPlanJsonOnly } from '../plan/manager.js'; +import { sanitizeDiagnosticText } from '../scope/path-identity.js'; import { ensureAgentSession } from '../state.js'; import { appendStageARepairEvent, @@ -19,6 +20,13 @@ export interface ReworkRecoverySummary { state: string; transitionId: string; recordedAt: string; + /** + * Whether the stage_a_repair audit event actually landed in + * `.swarm/events.jsonl`. The durable transition is authoritative regardless; + * this surfaces the best-effort append honestly instead of asserting an + * audit record that may not exist (issue #2755 review, FB-001). + */ + auditEventRecorded: boolean; } /** @@ -95,7 +103,18 @@ export async function forceRecoverReworkTask( ); } - const evidence = await readTaskEvidence(directory, taskId); + // Discriminated evidence read (issue #2755 review, FB-004): readTaskEvidence + // collapses missing and unparseable files to null, which would misreport a + // corrupt evidence file as "no evidence exists" and steer the architect away + // from evidence repair. Mirrors the plan missing/corrupt distinction below. + const evidenceRead = await readTaskEvidenceState(directory, taskId); + if (evidenceRead.kind === 'unparseable') { + throw new Error( + `RECOVER_REWORK_EVIDENCE_CORRUPT: the workflow evidence file for task ${taskId} exists but could not be parsed (${evidenceRead.evidencePath}). ` + + 'Repair the evidence first (repair_gate_evidence or /swarm doctor); recovery cannot classify a corrupt evidence state.', + ); + } + const evidence = evidenceRead.kind === 'ok' ? evidenceRead.evidence : null; const workflow = getTaskWorkflowSnapshot(evidence); if (!evidence || !workflow.authoritative) { throw new Error( @@ -122,20 +141,34 @@ export async function forceRecoverReworkTask( if (!Number.isFinite(wedgeMs)) { throw new Error( 'RECOVER_REWORK_GREEN_PRECHECK_REQUIRED: cannot prove pre-check recency ' + - `(workflow updatedAt "${workflow.updatedAt}" is not a parseable timestamp). Re-run pre_check_batch on the task's changed files, then retry.`, + `(workflow updatedAt "${workflow.updatedAt}" is not a parseable timestamp — the evidence file for task ${taskId} is likely corrupt). ` + + 'Repair the evidence (repair_gate_evidence or /swarm doctor); re-running pre_check_batch cannot fix an unparseable timestamp.', ); } const greenness = await hasGreenPostSettlementPreCheck(directory, wedgeMs); if (!greenness.green) { + // FB-007: in a SAST-disabled config this refusal is permanent (no + // sast_scan bundle is ever persisted), so the generic retry advice would + // dead-end; name the config cause and the working alternative. + const sastHint = + greenness.reason === 'no_pre_check_bundles' + ? ' Note: if gates.sast_scan.enabled is false, no sast_scan bundle is ever persisted and this recovery is unavailable by design — enable gates.sast_scan.enabled and run sast_scan, or use the coder repair loop.' + : ''; throw new Error( `RECOVER_REWORK_GREEN_PRECHECK_REQUIRED: refusing to mark Stage A passed without proof (${greenness.reason}). ` + - "Re-run pre_check_batch on the task's changed files and retry once it is green.", + "The bar is a green secretscan AND sast_scan evidence pair newer than the failing verdict — normally provided by a fresh pre_check_batch run (bundle proof is global, not correlated to the task's changed files)." + + sastHint, ); } + // FB-005: the reason reaches the human-readable audit trail; apply the + // repo's untrusted-diagnostic scrubber so control/bidi characters cannot + // spoof audit renders (sibling stage-a-repair sanitizes its repair-failed + // messages the same way). Also makes the release fragment's "sanitized + // reason" claim true. const sanitizedReason = typeof options.reason === 'string' && options.reason.trim().length > 0 - ? options.reason.trim().slice(0, 500) + ? sanitizeDiagnosticText(options.reason.trim(), 500) : undefined; const recordedAt = new Date().toISOString(); // Deterministic per generation: a repeat call after success can never reach @@ -157,8 +190,10 @@ export async function forceRecoverReworkTask( // Best-effort audit event through the shared #2665 stage_a_repair wrapper // (appendCoreEventSync seam, criticalWarn on failure): the durable // transition above is authoritative; this event is the human-readable - // trail distinguishing a supervised recovery from a mechanical pass. - await appendStageARepairEvent(directory, { + // trail distinguishing a supervised recovery from a mechanical pass. The + // append outcome is surfaced to the caller (FB-001) instead of asserting + // an audit record that may not exist. + const auditEventRecorded = await appendStageARepairEvent(directory, { action: 'rework_recovered', taskId, transitionId, @@ -173,5 +208,6 @@ export async function forceRecoverReworkTask( state: updatedWorkflow.state, transitionId, recordedAt, + auditEventRecorded, }; } diff --git a/src/workflow/stage-a-repair.ts b/src/workflow/stage-a-repair.ts index dd5915942..a6824ee32 100644 --- a/src/workflow/stage-a-repair.ts +++ b/src/workflow/stage-a-repair.ts @@ -49,24 +49,29 @@ export interface StageARepairResult { * (which owns `.swarm` creation, lock retry, and torn-tail framing — its * single atomic append replaces the former EBUSY/EPERM one-retry), final * failure surfaced via criticalWarn so a silently missing audit line is - * visible. + * visible. Returns whether the audit line actually landed, so callers on + * supervised-write paths can surface audit honesty instead of asserting it + * (issue #2755 review: the recover_rework_task tool must not claim an audit + * record exists when the append failed). */ export async function appendStageARepairEvent( directory: string, payload: Record, -): Promise { +): Promise { try { appendCoreEventSync(directory, { type: 'stage_a_repair', timestamp: new Date().toISOString(), ...payload, }); + return true; } catch (error) { logger.criticalWarn( `[stage-a-repair] audit event write failed: ${ error instanceof Error ? error.message : String(error) }`, ); + return false; } } diff --git a/tests/unit/gate-evidence/rework-recovery-supervised.test.ts b/tests/unit/gate-evidence/rework-recovery-supervised.test.ts index bd477b451..db4f13f9c 100644 --- a/tests/unit/gate-evidence/rework-recovery-supervised.test.ts +++ b/tests/unit/gate-evidence/rework-recovery-supervised.test.ts @@ -155,6 +155,111 @@ describe('gate-evidence supervised stage_a_passed (issue #2755)', () => { ).rejects.toThrow(/TASK_WORKFLOW_TERMINAL/); }); + it('rejects a supervised stage_a_passed from reviewer_run (FB-003: mutation-probe gap)', async () => { + // Seed to reviewer_run: supervised recovery, then the reviewer gate + // completes without completing the route. + await seedReworkRequired('1.8'); + await transitionTaskWorkflowEvidence(tempDir, '1.8', { + type: 'stage_a_passed', + supervisedRecovery: true, + expectedGeneration: 1, + transitionId: 'rr-1.8', + }); + await transitionTaskWorkflowEvidence(tempDir, '1.8', { + type: 'stage_b_completed', + gate: 'reviewer', + sessionId: 'rev-session', + routeComplete: false, + expectedGeneration: 1, + transitionId: 'rev-partial', + }); + expect(readTaskEvidenceRaw(tempDir, '1.8')?.workflow?.state).toBe( + 'reviewer_run', + ); + // The supervised admission must NOT widen into reviewer_run. + await expect( + transitionTaskWorkflowEvidence(tempDir, '1.8', { + type: 'stage_a_passed', + supervisedRecovery: true, + expectedGeneration: 1, + transitionId: 'supervised-from-reviewer-run', + }), + ).rejects.toThrow(/TASK_WORKFLOW_CODER_MUTATION_REQUIRED/); + }); + + it('persists supervisedRecovery on the supervised pass, only on the supervised pass, and clears it on generation rotation (FB-002)', async () => { + // Mechanical pass from coder_delegated: no marker. + await transitionTaskWorkflowEvidence(tempDir, '2.1', { + type: 'accepted_mutation', + agentType: 'coder', + expectedGeneration: 0, + transitionId: 'mut-2.1', + }); + await transitionTaskWorkflowEvidence(tempDir, '2.1', { + type: 'stage_a_passed', + expectedGeneration: 1, + transitionId: 'mech-2.1', + }); + expect( + readTaskEvidenceRaw(tempDir, '2.1')?.workflow?.supervisedRecovery, + ).toBeUndefined(); + + // Supervised pass from rework_required: marker persisted and durable + // across a subsequent same-generation transition. + await seedReworkRequired('2.2'); + await transitionTaskWorkflowEvidence(tempDir, '2.2', { + type: 'stage_a_passed', + supervisedRecovery: true, + expectedGeneration: 1, + transitionId: 'rr-2.2', + }); + expect( + readTaskEvidenceRaw(tempDir, '2.2')?.workflow?.supervisedRecovery, + ).toBe(true); + await transitionTaskWorkflowEvidence(tempDir, '2.2', { + type: 'stage_b_completed', + gate: 'reviewer', + sessionId: 'rev', + routeComplete: false, + expectedGeneration: 1, + transitionId: 'rev-2.2', + }); + expect( + readTaskEvidenceRaw(tempDir, '2.2')?.workflow?.supervisedRecovery, + ).toBe(true); + + // Generation rotation (accepted_mutation) clears the marker. + await transitionTaskWorkflowEvidence(tempDir, '2.2', { + type: 'accepted_mutation', + agentType: 'coder', + expectedGeneration: 1, + transitionId: 'mut-2.2', + }); + expect( + readTaskEvidenceRaw(tempDir, '2.2')?.workflow?.supervisedRecovery, + ).toBeUndefined(); + + // repair_idle also clears it (mirrors forcedCompletion semantics). + await seedReworkRequired('2.3'); + await transitionTaskWorkflowEvidence(tempDir, '2.3', { + type: 'stage_a_passed', + supervisedRecovery: true, + expectedGeneration: 1, + transitionId: 'rr-2.3', + }); + expect( + readTaskEvidenceRaw(tempDir, '2.3')?.workflow?.supervisedRecovery, + ).toBe(true); + await transitionTaskWorkflowEvidence(tempDir, '2.3', { + type: 'repair_idle', + expectedGeneration: 1, + transitionId: 'repair-2.3', + }); + expect( + readTaskEvidenceRaw(tempDir, '2.3')?.workflow?.supervisedRecovery, + ).toBeUndefined(); + }); + it('still admits the plain stage_a_passed from coder_delegated and pre_check_passed', async () => { await transitionTaskWorkflowEvidence(tempDir, '1.7', { type: 'accepted_mutation', diff --git a/tests/unit/tools/recover-rework-task.test.ts b/tests/unit/tools/recover-rework-task.test.ts index d70afbe7c..d31292faf 100644 --- a/tests/unit/tools/recover-rework-task.test.ts +++ b/tests/unit/tools/recover-rework-task.test.ts @@ -6,6 +6,7 @@ import { readTaskEvidence, transitionTaskWorkflowEvidence, } from '../../../src/gate-evidence'; +import { TOOL_METADATA } from '../../../src/tools/tool-metadata'; import { ensureAgentSession, resetSwarmState } from '../../../src/state'; import { executeRecoverReworkTask, @@ -200,6 +201,7 @@ describe('recover_rework_task tool (issue #2755)', () => { generation: number; state: string; transition_id: string; + audit_event_recorded: boolean; method: string; message: string; }; @@ -208,6 +210,7 @@ describe('recover_rework_task tool (issue #2755)', () => { expect(parsed.generation).toBe(1); expect(parsed.state).toBe('pre_check_passed'); expect(parsed.transition_id).toBe('rework-recovery:1.2:gen1'); + expect(parsed.audit_event_recorded).toBe(true); expect(parsed.method).toBe('supervised_recovery'); expect(parsed.message).toContain( 'Reviewer/test_engineer dispatch is permitted again', @@ -215,9 +218,48 @@ describe('recover_rework_task tool (issue #2755)', () => { expect(parsed.message).toContain('rework_recovered'); }); + it('rejects an oversized task_id instead of echoing it (FB-014)', async () => { + const raw = await executeRecoverReworkTask( + { task_id: `9.9${'X'.repeat(80)}`, reason: 'too long' }, + directory, + { sessionID: 's-1' }, + ); + const parsed = JSON.parse(raw) as { success: boolean; message: string }; + expect(parsed.success).toBe(false); + expect(parsed.message).toContain('Invalid recover_rework_task call'); + }); + + it('surfaces audit_event_recorded:false with a WARNING when the audit append fails (FB-001)', async () => { + writeMinimalPlan('1.3'); + await seedReworkRequired('1.3'); + await writeGreenBundles('1.3'); + ensureAgentSession('tool-arch-3', 'architect', directory); + // Force the events append to fail: the canonical .swarm/events.jsonl + // path is occupied by a directory, so appendCoreEventSync cannot write + // (EISDIR class — same probe the review used). + mkdirSync(path.join(directory, '.swarm', 'events.jsonl'), { + recursive: true, + }); + const raw = await executeRecoverReworkTask( + { task_id: '1.3', reason: 'audit append will fail' }, + directory, + { sessionID: 'tool-arch-3' }, + ); + const parsed = JSON.parse(raw) as { + success: boolean; + audit_event_recorded: boolean; + message: string; + }; + expect(parsed.success).toBe(true); + expect(parsed.audit_event_recorded).toBe(false); + expect(parsed.message).toContain('WARNING'); + expect(parsed.message).not.toContain('is audited to .swarm/events.jsonl'); + }); + it('is registered with an architect-only grant and a two-arg execute (invariant 11)', () => { expect(recover_rework_task.description).toContain('rework_required'); expect(typeof recover_rework_task.execute).toBe('function'); expect(recover_rework_task.execute.length).toBeGreaterThanOrEqual(2); + expect(TOOL_METADATA.recover_rework_task.agents).toEqual(['architect']); }); }); diff --git a/tests/unit/workflow/rework-recovery.test.ts b/tests/unit/workflow/rework-recovery.test.ts index 1bc087e20..853de478b 100644 --- a/tests/unit/workflow/rework-recovery.test.ts +++ b/tests/unit/workflow/rework-recovery.test.ts @@ -270,6 +270,7 @@ describe('forceRecoverReworkTask (issue #2755)', () => { expect(summary.state).toBe('pre_check_passed'); expect(summary.generation).toBe(1); expect(summary.transitionId).toBe('rework-recovery:1.4:gen1'); + expect(summary.auditEventRecorded).toBe(true); const { readTaskEvidence, getTaskWorkflowSnapshot } = await import( '../../../src/gate-evidence' @@ -303,4 +304,139 @@ describe('forceRecoverReworkTask (issue #2755)', () => { }), ).rejects.toThrow(/RECOVER_REWORK_STATE_REQUIRED/); }); + + it('persists the supervisedRecovery marker in the durable evidence (FB-002)', async () => { + writeMinimalPlan('1.5'); + await seedWorkflow('1.5', ['mutate', 'stageA', 'stageBFail']); + await writeGreenBundles('1.5'); + const { ensureAgentSession } = await import('../../../src/state'); + ensureAgentSession('arch-6', 'architect', directory); + await forceRecoverReworkTask(directory, 'arch-6', { + taskId: '1.5', + reason: 'supervised recovery marker pin', + }); + const { readTaskEvidence } = await import('../../../src/gate-evidence'); + expect( + (await readTaskEvidence(directory, '1.5'))?.workflow?.supervisedRecovery, + ).toBe(true); + }); + + it('refuses stale green bundles (FB-010: recency floor pinned at the call site)', async () => { + writeMinimalPlan('1.6'); + await seedWorkflow('1.6', ['mutate', 'stageA', 'stageBFail']); + // Green bundles dated BEFORE the wedge anchor: recency must refuse even + // though the verdicts are green. + const { readTaskEvidence, getTaskWorkflowSnapshot } = await import( + '../../../src/gate-evidence' + ); + const anchor = getTaskWorkflowSnapshot( + await readTaskEvidence(directory, '1.6'), + ).updatedAt; + const stale = new Date(Date.parse(anchor) - 60_000).toISOString(); + writeBundle('secretscan', stale); + writeBundle('sast_scan', stale); + const { ensureAgentSession } = await import('../../../src/state'); + ensureAgentSession('arch-7', 'architect', directory); + await expect( + forceRecoverReworkTask(directory, 'arch-7', { + taskId: '1.6', + reason: 'stale bundles', + }), + ).rejects.toThrow( + /RECOVER_REWORK_GREEN_PRECHECK_REQUIRED.*pre_check_failed_or_stale/, + ); + }); + + it('distinguishes corrupt evidence from missing evidence (FB-004)', async () => { + writeMinimalPlan('1.7'); + await seedWorkflow('1.7', ['mutate', 'stageA', 'stageBFail']); + const evidencePath = path.join(directory, '.swarm', 'evidence', '1.7.json'); + writeFileSync(evidencePath, '{ not valid json', 'utf-8'); + const { ensureAgentSession } = await import('../../../src/state'); + ensureAgentSession('arch-8', 'architect', directory); + await expect( + forceRecoverReworkTask(directory, 'arch-8', { + taskId: '1.7', + reason: 'corrupt evidence', + }), + ).rejects.toThrow(/RECOVER_REWORK_EVIDENCE_CORRUPT/); + }); + + it('refuses a plan task with no durable evidence as NO_WORKFLOW (FB-013)', async () => { + writeMinimalPlan('1.8'); + const { ensureAgentSession } = await import('../../../src/state'); + ensureAgentSession('arch-9', 'architect', directory); + await expect( + forceRecoverReworkTask(directory, 'arch-9', { + taskId: '1.8', + reason: 'no evidence yet', + }), + ).rejects.toThrow(/RECOVER_REWORK_NO_WORKFLOW/); + }); + + it('refuses PLAN_CORRUPT for an unparseable plan (FB-013)', async () => { + mkdirSync(path.join(directory, '.swarm'), { recursive: true }); + writeFileSync( + path.join(directory, '.swarm', 'plan.json'), + '{ not json', + 'utf-8', + ); + const { ensureAgentSession } = await import('../../../src/state'); + ensureAgentSession('arch-10', 'architect', directory); + await expect( + forceRecoverReworkTask(directory, 'arch-10', { + taskId: '1.1', + reason: 'corrupt plan', + }), + ).rejects.toThrow(/PLAN_CORRUPT/); + }); + + it('refuses an unparseable workflow updatedAt as corrupt evidence (FB-013)', async () => { + writeMinimalPlan('1.9'); + // Hand-write schema-valid evidence with a garbage updatedAt: the + // TaskWorkflowMetadata schema only requires a string, so corruption can + // reach the recency anchor. + await seedWorkflow('1.9', ['mutate', 'stageA', 'stageBFail']); + const evidencePath = path.join(directory, '.swarm', 'evidence', '1.9.json'); + const raw = JSON.parse(readFileSync(evidencePath, 'utf-8') ?? '{}') as { + workflow?: { updatedAt?: string }; + }; + expect(raw.workflow).toBeDefined(); + raw.workflow!.updatedAt = 'garbage-not-a-date'; + writeFileSync(evidencePath, JSON.stringify(raw), 'utf-8'); + const { ensureAgentSession } = await import('../../../src/state'); + ensureAgentSession('arch-11', 'architect', directory); + await expect( + forceRecoverReworkTask(directory, 'arch-11', { + taskId: '1.9', + reason: 'corrupt updatedAt', + }), + ).rejects.toThrow( + /RECOVER_REWORK_GREEN_PRECHECK_REQUIRED.*not a parseable timestamp/, + ); + }); + + it('accepts a multi-swarm prefixed architect and still refuses a prefixed non-architect (FB-012)', async () => { + writeMinimalPlan('1.11'); + await seedWorkflow('1.11', ['mutate', 'stageA', 'stageBFail']); + await writeGreenBundles('1.11'); + const { ensureAgentSession } = await import('../../../src/state'); + ensureAgentSession('pref-1', 'local_architect', directory); + const summary = await forceRecoverReworkTask(directory, 'pref-1', { + taskId: '1.11', + reason: 'prefixed architect acceptance pin', + }); + expect(summary.state).toBe('pre_check_passed'); + + // Prefixed non-architect: still refused. + writeMinimalPlan('1.12'); + await seedWorkflow('1.12', ['mutate', 'stageA', 'stageBFail']); + ensureAgentSession('pref-2', 'mega_coder', directory); + await expect( + forceRecoverReworkTask(directory, 'pref-2', { + taskId: '1.12', + reason: 'prefixed non-architect', + }), + ).rejects.toThrow(/RECOVER_REWORK_ARCHITECT_REQUIRED/); + }); }); From 67431fc21f327c73428798c6efe8441648ee39b6 Mon Sep 17 00:00:00 2001 From: Test User Date: Mon, 14 Sep 2026 10:32:12 -0500 Subject: [PATCH 3/3] style: biome ci import order and formatting (quality-gate fix) --- src/hooks/delegation-gate.ts | 2 +- tests/unit/tools/recover-rework-task.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hooks/delegation-gate.ts b/src/hooks/delegation-gate.ts index 7cc6f088d..e54e6dc18 100644 --- a/src/hooks/delegation-gate.ts +++ b/src/hooks/delegation-gate.ts @@ -4463,7 +4463,7 @@ export function createDelegationGateHook( workflow.state === 'rework_required' ? 'Remediation: if the Stage B verdict did not require a code change, call recover_rework_task for task ' + taskId + - " (architect-only, audited; requires green secretscan AND sast_scan evidence newer than the verdict — normally from a fresh pre_check_batch run; unavailable when SAST is disabled), then re-dispatch. " + + ' (architect-only, audited; requires green secretscan AND sast_scan evidence newer than the verdict — normally from a fresh pre_check_batch run; unavailable when SAST is disabled), then re-dispatch. ' + 'Otherwise delegate the coder to repair the code, re-run pre_check_batch, and re-dispatch once stage_a_passed has fired.' : `Remediation: run pre_check_batch on the task's changed files first. If pre_check_batch passes but the task remains coder_delegated (typical after /swarm reset-session), run /swarm recover ${taskId} to repair Stage A attribution, then re-dispatch.`; throw new Error( diff --git a/tests/unit/tools/recover-rework-task.test.ts b/tests/unit/tools/recover-rework-task.test.ts index d31292faf..e54b22488 100644 --- a/tests/unit/tools/recover-rework-task.test.ts +++ b/tests/unit/tools/recover-rework-task.test.ts @@ -6,12 +6,12 @@ import { readTaskEvidence, transitionTaskWorkflowEvidence, } from '../../../src/gate-evidence'; -import { TOOL_METADATA } from '../../../src/tools/tool-metadata'; import { ensureAgentSession, resetSwarmState } from '../../../src/state'; import { executeRecoverReworkTask, recover_rework_task, } from '../../../src/tools/recover-rework-task'; +import { TOOL_METADATA } from '../../../src/tools/tool-metadata'; import { createSafeTestDir } from '../../helpers/safe-test-dir'; /**