diff --git a/docs/releases/pending/fix-plan-critic-task-attribution-2757.md b/docs/releases/pending/fix-plan-critic-task-attribution-2757.md new file mode 100644 index 000000000..a74be9ec1 --- /dev/null +++ b/docs/releases/pending/fix-plan-critic-task-attribution-2757.md @@ -0,0 +1,26 @@ +# Plan-critic task attribution fix (#2757) + +## What changed + +Plan-level critic-family dispatches no longer acquire a durable per-task gate +merely because review prose mentions a plan task ID. `critic`, +`critic_sounding_board`, `critic_drift_verifier`, `critic_hallucination_verifier`, +and `critic_architecture_supervisor` now require structured task attribution or +an exact task marker at launch, background pending capture, and foreground +settlement. A non-strict named ID cannot shadow a valid numeric marker. Reviewer +and test-engineer plan-aware routing is unchanged, including explicit task +routing for large plans. + +The architect delegation contract now instructs task-scoped dispatches to keep +the numeric plan ID consistent across the `TASK:` line and `task_id` argument. + +## Recovery + +Existing projects with already-orphaned critic gate evidence should use the +audited `repair_gate_evidence` recovery path. The fix does not rewrite durable +evidence automatically. + +## Migration + +No configuration change is required. Task-scoped critic dispatches must carry a +structured task ID or an exact task marker such as `TASK: 1.1`. diff --git a/src/agents/architect.ts b/src/agents/architect.ts index af574a4f9..f15101212 100644 --- a/src/agents/architect.ts +++ b/src/agents/architect.ts @@ -737,6 +737,14 @@ Mutation delegations are performed by calling the **Task** tool. Read-only advis All delegations MUST follow the receiving agent's INPUT FORMAT exactly. Do NOT invent fields, omit required fields, or force one agent's schema onto another. Every delegation MUST begin with the agent name, include \`TASK:\`, and include \`SKILLS:\` when that agent prompt supports skills. Do NOT add conversational preamble before the agent prefix. Begin directly with the agent name. +TASK ATTRIBUTION: For task-scoped delegations, put the exact numeric plan task ID +alone on a standalone \`TASK:\` line (for example, \`TASK: 1.1\`) and put the objective +on the following line. When the Task arguments support an explicit field, set +\`task_id\` to the same numeric value as a tool argument (not as prompt prose). Keep +the numeric ID consistent across the TASK line, \`task_id\`, and any acceptance text. +Plan-level critics and other project-wide reviews must omit task attribution rather +than guessing from ambient prose or session state. + {{AGENT_PREFIX}}[agent] TASK: [single objective] [agent-specific fields required by that agent's INPUT FORMAT] diff --git a/src/hooks/delegation-gate.ts b/src/hooks/delegation-gate.ts index 13433888b..2a415a2ca 100644 --- a/src/hooks/delegation-gate.ts +++ b/src/hooks/delegation-gate.ts @@ -128,9 +128,11 @@ import { toTaskIdPlanContextOptions, } from './plan-task-id-context.js'; import { + EXPLICIT_TASK_ID_FIELDS, resolveDelegatedPlanTaskId, resolveTaskId, TASK_ID_RESOLUTION_LIMITS, + type TaskIdPolicy, } from './task-id-resolver.js'; export { resolveDelegatedPlanTaskId } from './task-id-resolver.js'; @@ -2666,6 +2668,34 @@ const TASK_GATE_AGENTS = new Set([ 'sme', ]); +const EXPLICIT_TASK_EVIDENCE_AGENTS = new Set([ + 'critic', + 'critic_sounding_board', + 'critic_drift_verifier', + 'critic_hallucination_verifier', + 'critic_architecture_supervisor', +]); + +function isExplicitTaskEvidenceAgent(targetAgent: string): boolean { + return EXPLICIT_TASK_EVIDENCE_AGENTS.has(stripKnownSwarmPrefix(targetAgent)); +} + +type EvidenceTaskResolutionOptions = { + policy?: TaskIdPolicy; + allowSessionFallback?: boolean; +}; + +function evidenceTaskResolutionOptions( + targetAgent: string, + allowSessionFallback?: boolean, +): EvidenceTaskResolutionOptions | undefined { + if (isExplicitTaskEvidenceAgent(targetAgent)) { + return { policy: 'attribution', allowSessionFallback: false }; + } + if (allowSessionFallback === false) return { allowSessionFallback: false }; + return undefined; +} + export function canRunWhileTaskAwaitsCompletion(input: { directory: string | undefined; normalizedTool: string; @@ -3026,10 +3056,10 @@ async function getEvidenceTaskId( } /** - * Resolves the correct task ID for evidence recording by chaining: - * 1. Explicit task_id in direct args (structured field) - * 2. Prompt-text extraction via resolveDelegatedPlanTaskId (plan-aware) - * 3. Session-state fallback via getEvidenceTaskId + * Resolves the correct task ID for evidence recording by chaining the selected + * resolver policy with an optional session-state fallback. Most roles retain + * plan-aware prompt resolution; task-gated critic roles select attribution + * policy and disable the fallback so only structured IDs or exact markers bind. * * This fixes parallel evidence recording where multiple reviewer/test_engineer * agents are dispatched for different tasks from the same architect session. @@ -3039,7 +3069,7 @@ async function resolveEvidenceTaskId( args: Record | undefined, session: AgentSessionState, directory: string, - options: { allowSessionFallback?: boolean } = {}, + options: EvidenceTaskResolutionOptions = {}, ): Promise { // Shared bounded resolution first; session fallback is allowed only when the // resolver had no safe plan context and therefore made no authoritative @@ -3058,22 +3088,58 @@ async function resolveEvidenceTaskId( if (args) { try { - const resolution = resolveTaskId(args, { - policy: 'plan', - ...(planTaskIdContext - ? toTaskIdPlanContextOptions(planTaskIdContext) - : {}), - }); + const policy = options.policy ?? 'plan'; + // A plan over the shared bounded-ID limit can still authorize an + // explicitly attributed critic task. The full plan has already been + // loaded above, so defer numeric membership validation to the existing + // full-plan check below instead of handing the bounded resolver an + // over-limit context that intentionally rejects numeric markers. + const planContextOptions = + policy === 'attribution' && planTaskIdContext?.status === 'over_limit' + ? {} + : planTaskIdContext + ? toTaskIdPlanContextOptions(planTaskIdContext) + : {}; + const resolutionOptions = { + policy, + ...planContextOptions, + // Durable critic gates accept only structured IDs or a bare TASK + // marker; quoted and example text is not dispatch attribution. + standaloneTaskMarkerOnly: policy === 'attribution', + }; + const resolution = resolveTaskId(args, resolutionOptions); if (resolution.status === 'resolved') { + let resolvedTaskId = resolution.taskId; + if (policy === 'attribution' && !isStrictTaskId(resolvedTaskId)) { + // The generic attribution resolver intentionally accepts safe named + // IDs for non-gate consumers. Durable task-gate evidence is stricter: + // retry marker-only attribution so a named explicit value cannot + // shadow a valid numeric TASK marker, then fail closed otherwise. + const markerOnlyArgs = { ...args }; + for (const field of EXPLICIT_TASK_ID_FIELDS) { + delete markerOnlyArgs[field]; + } + const markerResolution = resolveTaskId( + markerOnlyArgs, + resolutionOptions, + ); + if ( + markerResolution.status !== 'resolved' || + !isStrictTaskId(markerResolution.taskId) + ) { + return null; + } + resolvedTaskId = markerResolution.taskId; + } if ( planTaskIdContext?.status === 'over_limit' && !plan?.phases.some((phase) => - phase?.tasks?.some((task) => task?.id === resolution.taskId), + phase?.tasks?.some((task) => task?.id === resolvedTaskId), ) ) { return null; } - return resolution.taskId; + return resolvedTaskId; } if ( resolution.status === 'invalid' || @@ -4306,12 +4372,15 @@ export function createDelegationGateHook( args, stageBSession, directory, - activePrReviewBinding ? { allowSessionFallback: false } : undefined, + evidenceTaskResolutionOptions( + targetAgent, + activePrReviewBinding ? false : undefined, + ), ); const candidateTaskIds = new Set(); if (resolvedTaskId) candidateTaskIds.add(resolvedTaskId); const dispatchPlan = await loadPlanJsonOnly(directory); - if (dispatchPlan) { + if (dispatchPlan && !isExplicitTaskEvidenceAgent(targetAgent)) { const knownIds = new Set( dispatchPlan.phases.flatMap((phase) => phase.tasks.map((task) => task.id), @@ -5349,10 +5418,13 @@ export function createDelegationGateHook( } if (subagentSessionId) { const mergedArgs = { ...(storedArgs ?? {}), ...directArgs }; + const normalizedSubagentType = + stripKnownSwarmPrefix(subagentType); const evidenceTaskId = await resolveEvidenceTaskId( mergedArgs, session, directory, + evidenceTaskResolutionOptions(normalizedSubagentType), ); const scope = session.declaredCoderScope && @@ -5388,9 +5460,7 @@ export function createDelegationGateHook( evidenceTaskId, workspace: fallbackWorkspace, taskChangeContext, - workflowGeneration: TASK_GATE_AGENTS.has( - stripKnownSwarmPrefix(subagentType), - ) + workflowGeneration: TASK_GATE_AGENTS.has(normalizedSubagentType) ? stageBDispatchGenerationsByCallID .get(input.callID) ?.get(evidenceTaskId ?? '') @@ -6208,10 +6278,12 @@ export function createDelegationGateHook( let coderSettleTaskId: string | null = null; try { const mergedArgs = { ...(storedArgs ?? {}), ...directArgs }; + const targetAgentForEvidence = stripKnownSwarmPrefix(subagentType); let evidenceTaskId = await resolveEvidenceTaskId( mergedArgs, session, directory, + evidenceTaskResolutionOptions(targetAgentForEvidence), ); // Issue #2214 belt: the toolBefore scope preflight may have // resolved the task via sources resolveEvidenceTaskId lacks @@ -6239,8 +6311,6 @@ export function createDelegationGateHook( 'explorer', 'sme', ]; - const targetAgentForEvidence = - stripKnownSwarmPrefix(subagentType); if (gateAgents.includes(targetAgentForEvidence)) { if ( targetAgentForEvidence === 'reviewer' || diff --git a/src/hooks/task-id-resolver.ts b/src/hooks/task-id-resolver.ts index 7e1154ad0..722bc8a24 100644 --- a/src/hooks/task-id-resolver.ts +++ b/src/hooks/task-id-resolver.ts @@ -32,6 +32,12 @@ export type TaskIdResolution = export interface ResolveTaskIdOptions { policy: TaskIdPolicy; + /** + * Restrict text-derived attribution to bare, standalone TASK lines. This is + * used for durable critic evidence, where quoted/example text is not proof + * that a critic was dispatched for a task. Structured ID fields still win. + */ + standaloneTaskMarkerOnly?: boolean; knownPlanTaskIds?: ReadonlySet; /** The caller observed a valid plan whose task-ID cardinality exceeded the bound. */ planContextOverLimit?: boolean; @@ -45,7 +51,7 @@ export type TaskIdPlanContextOptions = Pick< >; const TEXT_FIELDS = ['prompt', 'description', 'task', 'input'] as const; -const EXPLICIT_FIELDS = [ +export const EXPLICIT_TASK_ID_FIELDS = [ 'plan_task_id', 'planTaskId', 'task_id', @@ -63,10 +69,58 @@ const ATTRIBUTION_ID_MARKER = // separately handles numeric IDs embedded in prose. const ATTRIBUTION_TASK_MARKER = /\bTASK\s*[:=]\s*([A-Za-z0-9][A-Za-z0-9._-]*)[ \t]*(?=\r?$)/gim; +const ATTRIBUTION_TASK_MARKER_STANDALONE = + /^TASK\s*[:=]\s*([A-Za-z0-9][A-Za-z0-9._-]*)[ \t]*\r?$/gim; const ATTRIBUTION_ID_MARKER_RAW = /\b(?:task_id|task-id|taskId)\s*[:=][ \t]*([^\s]*)/gi; const ATTRIBUTION_TASK_MARKER_RAW = /\bTASK\s*[:=][ \t]*([^\s]+)[ \t]*(?=\r?$)/gim; +const ATTRIBUTION_TASK_MARKER_RAW_STANDALONE = + /^TASK\s*[:=][ \t]*([^\s]+)[ \t]*\r?$/gim; + +/** Remove Markdown code/quote blocks before treating free text as evidence. */ +function stripUntrustedAttributionMarkdown(text: string): string { + const lines = text.split(/\r?\n/); + const kept: string[] = []; + let fence: { marker: '`' | '~'; length: number } | undefined; + let inBlockQuote = false; + + for (const line of lines) { + if (fence) { + const close = line.match(/^[ \t]{0,3}(`+|~+)[ \t]*$/); + if ( + close && + close[1][0] === fence.marker && + close[1].length >= fence.length + ) { + fence = undefined; + } + continue; + } + + const open = line.match(/^[ \t]{0,3}(`{3,}|~{3,})(.*)$/); + if (open && !(open[1][0] === '`' && open[2].includes('`'))) { + fence = { marker: open[1][0] as '`' | '~', length: open[1].length }; + continue; + } + + if (/^[ \t]{0,3}>/.test(line)) { + inBlockQuote = true; + continue; + } + if (inBlockQuote) { + if (line.trim() === '') { + inBlockQuote = false; + } else { + // Markdown allows lazy continuation lines within a block quote. + continue; + } + } + kept.push(line); + } + + return kept.join('\n'); +} function isSafeAttributionId(value: string): boolean { return ( @@ -177,7 +231,7 @@ export function resolveTaskId( } const explicit = new Set(); - for (const field of EXPLICIT_FIELDS) { + for (const field of EXPLICIT_TASK_ID_FIELDS) { const raw = input[field]; if (raw === undefined || raw === null) continue; if (typeof raw !== 'string') return { status: 'invalid', input: field }; @@ -270,12 +324,15 @@ export function resolveTaskId( const textSelection = select(textCandidates, 'text'); if (textSelection) return textSelection; } else { + const markerTextFields = options.standaloneTaskMarkerOnly + ? textFields.map(stripUntrustedAttributionMarkdown) + : textFields; let hasInvalidRawMarker = false; - for (const rawMarker of [ - ATTRIBUTION_ID_MARKER_RAW, - ATTRIBUTION_TASK_MARKER_RAW, - ]) { - for (const text of textFields) { + const rawMarkers = options.standaloneTaskMarkerOnly + ? [ATTRIBUTION_TASK_MARKER_RAW_STANDALONE] + : [ATTRIBUTION_ID_MARKER_RAW, ATTRIBUTION_TASK_MARKER_RAW]; + for (const rawMarker of rawMarkers) { + for (const text of markerTextFields) { rawMarker.lastIndex = 0; for (const match of text.matchAll(rawMarker)) { const value = match[1]; @@ -296,8 +353,11 @@ export function resolveTaskId( } if (hasInvalidRawMarker) return { status: 'invalid', input: 'marker' }; const marked = new Set(); - for (const marker of [ATTRIBUTION_ID_MARKER, ATTRIBUTION_TASK_MARKER]) { - for (const text of textFields) { + const markers = options.standaloneTaskMarkerOnly + ? [ATTRIBUTION_TASK_MARKER_STANDALONE] + : [ATTRIBUTION_ID_MARKER, ATTRIBUTION_TASK_MARKER]; + for (const marker of markers) { + for (const text of markerTextFields) { marker.lastIndex = 0; for (const match of text.matchAll(marker)) { const value = match[1]; diff --git a/tests/integration/pr-workflow-taskless-reentry-real-host.test.ts b/tests/integration/pr-workflow-taskless-reentry-real-host.test.ts index c1adcc1c2..43183a7ad 100644 --- a/tests/integration/pr-workflow-taskless-reentry-real-host.test.ts +++ b/tests/integration/pr-workflow-taskless-reentry-real-host.test.ts @@ -6,8 +6,11 @@ import { createDelegationGateHook } from '../../src/hooks/delegation-gate.js'; import { _test_exports, activatePrWorkflow, + hasActivePrReviewReentryAuthorization, + readPrReviewReentryBindingContext, } from '../../src/hooks/pr-workflow-gate.js'; import { issuePrReviewReentryAuthorization } from '../../src/pr-review/authorization.js'; +import { readReviewRouteReceipt } from '../../src/review/routing-enforcement.js'; import { ensureAgentSession, resetSwarmState, @@ -18,6 +21,7 @@ import { bootKnowledgeHost, createKnowledgeProject, } from '../helpers/knowledge-real-host.js'; +import { safeRmRecursive } from '../helpers/safe-test-dir.js'; const SESSION_ID = 'pr-workflow-taskless-reentry'; const HEAD_SHA = 'abc123'; @@ -154,6 +158,127 @@ describe('PR workflow taskless re-entry stays standalone-only', () => { } }); + test('plan-free re-entry ignores stale session task IDs and consumes authorization', async () => { + const staleSessionID = 'pr-workflow-stale-task-reentry'; + const staleTaskId = '1.1'; + const planFreeDirectory = createKnowledgeProject(); + try { + // This fixture deliberately does not boot the host or write an approved plan. + // Declare the temp directory as its own project root so an ancestor .swarm + // (for example, the developer profile) cannot block route-receipt writes. + mkdirSync(path.join(planFreeDirectory, '.git'), { recursive: true }); + await activatePrWorkflow(planFreeDirectory, staleSessionID, 'PR_REVIEW', { + prHeadSha: HEAD_SHA, + }); + expect( + await readPrReviewReentryBindingContext( + planFreeDirectory, + staleSessionID, + ), + ).toMatchObject({ prHeadSha: HEAD_SHA }); + const issued = await issuePrReviewReentryAuthorization( + planFreeDirectory, + staleSessionID, + { prHeadSha: HEAD_SHA, role: 'reviewer' }, + ); + expect(issued.role).toBe('reviewer'); + expect( + await hasActivePrReviewReentryAuthorization( + planFreeDirectory, + staleSessionID, + { role: 'reviewer' }, + ), + ).toBe(true); + + const session = ensureAgentSession( + staleSessionID, + 'architect', + planFreeDirectory, + ); + expect(session.taskWorkflowStates.size).toBe(0); + session.currentTaskId = staleTaskId; + + const hook = createDelegationGateHook( + { hooks: { delegation_gate: true } } as PluginConfig, + planFreeDirectory, + ); + const args = { + subagent_type: 'reviewer', + prompt: + 'Review the bound PR without a plan task.\nACCEPTANCE: report the review result.', + }; + await hook.toolBefore( + { + tool: 'Task', + sessionID: staleSessionID, + callID: 'stale-task-reviewer', + }, + { args }, + ); + expect('task_id' in args).toBe(false); + expect( + await hasActivePrReviewReentryAuthorization( + planFreeDirectory, + staleSessionID, + { role: 'reviewer' }, + ), + ).toBe(false); + expect( + await readPrReviewReentryBindingContext( + planFreeDirectory, + staleSessionID, + ), + ).toMatchObject({ prHeadSha: HEAD_SHA }); + expect( + await readReviewRouteReceipt({ + projectRoot: planFreeDirectory, + sessionId: staleSessionID, + taskId: staleTaskId, + }), + ).toBeNull(); + expect( + await readReviewRouteReceipt({ + projectRoot: planFreeDirectory, + sessionId: staleSessionID, + taskId: 'unresolved-stale-task-reviewer', + }), + ).toMatchObject({ taskId: 'unresolved-stale-task-reviewer' }); + await hook.toolAfter( + { + tool: 'Task', + sessionID: staleSessionID, + callID: 'stale-task-reviewer', + args, + }, + { + status: 'completed', + text: `[REVIEWED] | task-${staleTaskId} | APPROVED | stale-task bait`, + }, + ); + + expect(session.currentTaskId).toBe(staleTaskId); + expect(session.taskWorkflowStates.size).toBe(0); + expect( + await readPrReviewReentryBindingContext( + planFreeDirectory, + staleSessionID, + ), + ).toMatchObject({ prHeadSha: HEAD_SHA }); + await expect( + hook.toolBefore( + { + tool: 'Task', + sessionID: staleSessionID, + callID: 'stale-task-reviewer-replay', + }, + { args }, + ), + ).rejects.toThrow(/TASK_WORKFLOW_STAGE_A_REQUIRED/); + } finally { + safeRmRecursive(planFreeDirectory); + } + }); + test('taskless re-entry refuses to guess when task workflow state exists', async () => { await activatePrWorkflow(directory, SESSION_ID, 'PR_REVIEW', { prHeadSha: HEAD_SHA, @@ -271,7 +396,7 @@ describe('PR workflow taskless re-entry stays standalone-only', () => { } finally { await Promise.allSettled([...swarmState.pendingRehydrations]); swarmState.pendingRehydrations.clear(); - rmSync(pendingDirectory, { recursive: true, force: true }); + safeRmRecursive(pendingDirectory); } }); }); diff --git a/tests/unit/agents/architect-task-attribution-prompt.test.ts b/tests/unit/agents/architect-task-attribution-prompt.test.ts new file mode 100644 index 000000000..9783ca986 --- /dev/null +++ b/tests/unit/agents/architect-task-attribution-prompt.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'bun:test'; +import { createArchitectAgent } from '../../../src/agents/architect'; +import { resolveTaskId } from '../../../src/hooks/task-id-resolver'; + +describe('Architect prompt — task attribution guidance', () => { + it('requires numeric task identity on task-scoped delegations', () => { + const prompt = createArchitectAgent('test-model').config.prompt ?? ''; + + expect(prompt).toContain('TASK ATTRIBUTION'); + expect(prompt).toContain( + 'alone on a standalone `TASK:` line (for example, `TASK: 1.1`)', + ); + expect(prompt).not.toContain('TASK: 1.1 —'); + expect(prompt).toContain( + 'task_id` to the same numeric value as a tool argument', + ); + expect(prompt).toContain('Plan-level critics'); + + const shippedExample = /for example, `(TASK: \d+\.\d+(?:\.\d+)*)`/.exec( + prompt, + )?.[1]; + expect(shippedExample).toBe('TASK: 1.1'); + expect( + resolveTaskId( + { prompt: shippedExample }, + { policy: 'attribution', knownPlanTaskIds: new Set(['1.1']) }, + ), + ).toEqual({ status: 'resolved', taskId: '1.1', source: 'marker' }); + }); +}); diff --git a/tests/unit/hooks/delegation-gate-critic-task-attribution-2757.test.ts b/tests/unit/hooks/delegation-gate-critic-task-attribution-2757.test.ts new file mode 100644 index 000000000..5ab587e55 --- /dev/null +++ b/tests/unit/hooks/delegation-gate-critic-task-attribution-2757.test.ts @@ -0,0 +1,487 @@ +/** Regression coverage for issue #2757's plan-critic task attribution dead-end. */ + +import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { mkdirSync, writeFileSync } from 'node:fs'; +import * as path from 'node:path'; +import { createBackgroundCompletionObserver } from '../../../src/background/completion-observer'; +import { findByCorrelationId } from '../../../src/background/pending-delegations'; +import type { Plan } from '../../../src/config/plan-schema'; +import { closeProjectDb } from '../../../src/db/project-db'; +import { + getTaskWorkflowSnapshot, + hasPassedAllGates, + readTaskEvidence, + transitionTaskWorkflowEvidence, +} from '../../../src/gate-evidence'; +import { createDelegationGateHook } from '../../../src/hooks/delegation-gate'; +import { ensureAgentSession, resetSwarmState } from '../../../src/state'; +import { createIsolatedTestEnv } from '../../helpers/isolated-test-env'; +import { safeRmRecursive } from '../../helpers/safe-test-dir'; +import { canonicalMkdtemp } from '../../helpers/tmpdir'; +import { makeConfig } from './_delegation-gate-helpers'; + +const TASK_ID = '1.1'; + +function makePlan(): Plan { + return { + schema_version: '1.0.0', + title: 'Critic attribution regression', + swarm: 'mega', + current_phase: 1, + phases: [ + { + id: 1, + name: 'Implementation', + status: 'pending', + tasks: [ + { + id: TASK_ID, + phase: 1, + status: 'pending', + size: 'small', + description: 'Implement the issue fix', + depends: [], + files_touched: ['src/example.ts'], + }, + ], + }, + ], + }; +} + +function writePlan(directory: string): void { + writeFileSync( + path.join(directory, '.swarm', 'plan.json'), + JSON.stringify(makePlan(), null, 2), + 'utf8', + ); +} + +async function seedStageA(sessionID: string): Promise { + const accepted = await transitionTaskWorkflowEvidence(tmpDir, TASK_ID, { + type: 'accepted_mutation', + agentType: 'coder', + expectedGeneration: 0, + transitionId: 'seed-coder', + }); + const generation = getTaskWorkflowSnapshot(accepted).generation; + await transitionTaskWorkflowEvidence(tmpDir, TASK_ID, { + type: 'stage_a_passed', + expectedGeneration: generation, + transitionId: 'seed-stage-a', + }); + const session = ensureAgentSession(sessionID, 'architect', tmpDir); + session.currentTaskId = TASK_ID; + session.taskWorkflowStates.set(TASK_ID, 'pre_check_passed'); + return generation; +} + +async function settleCritic( + hook: ReturnType, + sessionID: string, + callID: string, + args: Record, +): Promise { + await hook.toolBefore({ tool: 'Task', sessionID, callID }, { args }); + await hook.toolAfter( + { tool: 'Task', sessionID, callID, args }, + { output: 'VERDICT: APPROVED\nThe review is complete.' }, + ); +} + +let tmpDir: string; +let isolatedEnv: ReturnType | undefined; + +beforeEach(() => { + // Keep user-scoped stores under a test-owned app-data root. The delegation + // hook exercises durable evidence and route state, so this also prevents a + // test run from touching the developer's real config/data directories. + isolatedEnv = createIsolatedTestEnv(); + resetSwarmState(); + tmpDir = canonicalMkdtemp('dg-critic-attribution-'); + mkdirSync(path.join(tmpDir, '.opencode'), { recursive: true }); + mkdirSync(path.join(tmpDir, '.swarm'), { recursive: true }); + writePlan(tmpDir); +}); + +afterEach(() => { + resetSwarmState(); + closeProjectDb(tmpDir); + safeRmRecursive(tmpDir); + isolatedEnv?.cleanup(); + isolatedEnv = undefined; +}); + +describe('delegation-gate — regression: plan-level critic attribution (#2757)', () => { + it('does not create task evidence from a text-only plan critic prompt', async () => { + const sessionID = 'plan-critic-unbound'; + const session = ensureAgentSession(sessionID, 'architect', tmpDir); + // The session may be actively working on 1.1, but plan-level critic + // dispatches still need explicit attribution rather than this fallback. + session.currentTaskId = TASK_ID; + session.taskWorkflowStates.set(TASK_ID, 'pre_check_passed'); + const hook = createDelegationGateHook(makeConfig(), tmpDir); + + // Before the fix, plan-aware text extraction treated the sole `1.1` in + // this plan-level prompt as task attribution, then recorded a per-task + // critic requirement that could outlive the coder mutation. + await settleCritic(hook, sessionID, 'critic-unbound', { + subagent_type: 'critic', + prompt: + 'MODE: CRITIC-GATE\nReview the complete plan before implementation. The plan includes 1.1.', + }); + + expect(await readTaskEvidence(tmpDir, TASK_ID)).toBeNull(); + }); + + it('keeps an unbound background plan critic out of pending gate ingestion', async () => { + const sessionID = 'background-plan-critic-unbound'; + const session = ensureAgentSession(sessionID, 'architect', tmpDir); + // A current task must not turn free-form plan prose into a Stage-B binding. + session.currentTaskId = TASK_ID; + session.taskWorkflowStates.set(TASK_ID, 'pre_check_passed'); + const hook = createDelegationGateHook( + makeConfig({ + hooks: { + background_subagents: true, + background_pending_timeout_minutes: 30, + }, + }), + tmpDir, + ); + const args = { + subagent_type: 'critic', + background: true, + prompt: + 'MODE: CRITIC-GATE\nReview the whole plan before work begins. The only task is 1.1.', + }; + + await hook.toolBefore( + { tool: 'Task', sessionID, callID: 'background-critic-unbound' }, + { args }, + ); + await hook.toolAfter( + { + tool: 'Task', + sessionID, + callID: 'background-critic-unbound', + args, + }, + { + state: 'running', + output: + 'Background critic started', + metadata: { background: true, jobId: 'background-critic-job' }, + }, + ); + + const observer = createBackgroundCompletionObserver({ + config: { enabled: true }, + directory: tmpDir, + }); + await observer.event({ + event: { + type: 'message.part.updated', + properties: { + part: { + type: 'text', + synthetic: true, + sessionID, + text: + '\n' + + 'VERDICT: APPROVED\n', + }, + }, + }, + }); + + const record = findByCorrelationId(tmpDir, 'background-critic-child'); + expect(record?.status).toBe('completed'); + expect(record?.planTaskId).toBeNull(); + expect(record?.evidenceTaskId).toBeNull(); + expect(record?.workflowGeneration).toBeUndefined(); + expect(await readTaskEvidence(tmpDir, TASK_ID)).toBeNull(); + }); + + it('records critic evidence when the dispatch carries explicit task_id attribution', async () => { + const sessionID = 'critic-explicit-task'; + ensureAgentSession(sessionID, 'architect', tmpDir); + const hook = createDelegationGateHook(makeConfig(), tmpDir); + + await settleCritic(hook, sessionID, 'critic-explicit', { + subagent_type: 'critic', + task_id: TASK_ID, + prompt: + 'MODE: CRITIC-GATE\nReview the implementation plan before execution.', + }); + + const evidence = await readTaskEvidence(tmpDir, TASK_ID); + expect(evidence?.required_gates).toContain('critic'); + expect(evidence?.gates.critic).toBeDefined(); + }); + + it('does not let a named ID shadow a valid TASK marker (F3)', async () => { + const sessionID = 'critic-mixed-attribution'; + ensureAgentSession(sessionID, 'architect', tmpDir); + const hook = createDelegationGateHook(makeConfig(), tmpDir); + + await settleCritic(hook, sessionID, 'critic-mixed-attribution', { + subagent_type: 'critic', + task_id: 'runtime-session-handle', + prompt: `MODE: CRITIC-GATE\nTASK: ${TASK_ID}`, + }); + + const evidence = await readTaskEvidence(tmpDir, TASK_ID); + expect(evidence?.required_gates).toContain('critic'); + expect(evidence?.gates.critic).toBeDefined(); + }); + + for (const [context, prompt] of [ + ['fenced code', `\`\`\`text\nTASK: ${TASK_ID}\n\`\`\``], + ['Markdown blockquote', `> TASK: ${TASK_ID}`], + ['quoted text', `"TASK: ${TASK_ID}"`], + ['prose-suffixed marker', `Review this prompt: TASK: ${TASK_ID}`], + ['free-text task_id marker', `task_id: ${TASK_ID}`], + ] as const) { + it(`does not record critic evidence from ${context} TASK text (F7)`, async () => { + const sessionID = `critic-untrusted-${context.replaceAll(' ', '-')}`; + ensureAgentSession(sessionID, 'architect', tmpDir); + const hook = createDelegationGateHook(makeConfig(), tmpDir); + + await settleCritic(hook, sessionID, sessionID, { + subagent_type: 'critic', + prompt, + }); + + expect(await readTaskEvidence(tmpDir, TASK_ID)).toBeNull(); + }); + } + + it('records critic evidence from a bare standalone TASK marker (F7)', async () => { + const sessionID = 'critic-bare-task-marker'; + ensureAgentSession(sessionID, 'architect', tmpDir); + const hook = createDelegationGateHook(makeConfig(), tmpDir); + + await settleCritic(hook, sessionID, sessionID, { + subagent_type: 'critic', + prompt: `Review the exact task.\nTASK: ${TASK_ID}`, + }); + + const evidence = await readTaskEvidence(tmpDir, TASK_ID); + expect(evidence?.required_gates).toContain('critic'); + expect(evidence?.gates.critic).toBeDefined(); + }); + + it('re-satisfies explicit critic evidence after an accepted coder mutation (F14)', async () => { + const sessionID = 'critic-explicit-rerun'; + ensureAgentSession(sessionID, 'architect', tmpDir); + const hook = createDelegationGateHook(makeConfig(), tmpDir); + + await settleCritic(hook, sessionID, 'critic-before-mutation', { + subagent_type: 'critic', + task_id: TASK_ID, + prompt: 'MODE: CRITIC-GATE\nReview the explicit task before execution.', + }); + const beforeMutation = await readTaskEvidence(tmpDir, TASK_ID); + expect(beforeMutation?.gates.critic).toBeDefined(); + + const accepted = await transitionTaskWorkflowEvidence(tmpDir, TASK_ID, { + type: 'accepted_mutation', + agentType: 'coder', + expectedGeneration: getTaskWorkflowSnapshot(beforeMutation).generation, + transitionId: 'coder-after-explicit-critic', + }); + const generation = getTaskWorkflowSnapshot(accepted).generation; + expect(accepted.required_gates).toContain('critic'); + expect(accepted.gates.critic).toBeUndefined(); + + await transitionTaskWorkflowEvidence(tmpDir, TASK_ID, { + type: 'stage_a_passed', + expectedGeneration: generation, + transitionId: 'stage-a-before-explicit-critic-rerun', + }); + await settleCritic(hook, sessionID, 'critic-after-mutation', { + subagent_type: 'critic', + task_id: TASK_ID, + prompt: + 'MODE: CRITIC-GATE\nRe-review the explicit task after the coder mutation.', + }); + + const afterRerun = await readTaskEvidence(tmpDir, TASK_ID); + expect(afterRerun?.gates.critic).toBeDefined(); + }); + + it('does not leave an unsatisfiable critic requirement after a coder mutation', async () => { + const sessionID = 'plan-critic-orphan'; + ensureAgentSession(sessionID, 'architect', tmpDir); + const hook = createDelegationGateHook(makeConfig(), tmpDir); + + // Before the fix, this plan-level critic settlement created `critic` as a + // task requirement. A succeeding coder mutation intentionally clears its + // proof, but cannot satisfy the stale requirement again. + await settleCritic(hook, sessionID, 'critic-orphan', { + subagent_type: 'critic', + prompt: + 'MODE: CRITIC-GATE\nReview the whole plan. Its only implementation task is 1.1.', + }); + const accepted = await transitionTaskWorkflowEvidence(tmpDir, TASK_ID, { + type: 'accepted_mutation', + agentType: 'coder', + expectedGeneration: 0, + transitionId: 'coder-after-plan-critic', + }); + const generation = getTaskWorkflowSnapshot(accepted).generation; + await transitionTaskWorkflowEvidence(tmpDir, TASK_ID, { + type: 'stage_a_passed', + expectedGeneration: generation, + transitionId: 'stage-a-after-plan-critic', + }); + await transitionTaskWorkflowEvidence(tmpDir, TASK_ID, { + type: 'stage_b_completed', + gate: 'reviewer', + sessionId: sessionID, + routeComplete: false, + expectedGeneration: generation, + transitionId: 'reviewer-after-plan-critic', + }); + await transitionTaskWorkflowEvidence(tmpDir, TASK_ID, { + type: 'stage_b_completed', + gate: 'test_engineer', + sessionId: sessionID, + routeComplete: true, + expectedGeneration: generation, + transitionId: 'test-engineer-after-plan-critic', + }); + + const evidence = await readTaskEvidence(tmpDir, TASK_ID); + expect(evidence?.required_gates).not.toContain('critic'); + expect(evidence?.gates.critic).toBeUndefined(); + expect(await hasPassedAllGates(tmpDir, TASK_ID)).toBe(true); + }); + + it('preserves TASK-line routing for reviewer and test_engineer', async () => { + const sessionID = 'stage-b-task-line'; + const generation = await seedStageA(sessionID); + const hook = createDelegationGateHook(makeConfig(), tmpDir); + + const reviewerArgs = { + subagent_type: 'reviewer', + prompt: `TASK: ${TASK_ID}\nACCEPTANCE: review the exact task and report APPROVED`, + }; + await hook.toolBefore( + { tool: 'Task', sessionID, callID: 'reviewer-task-line' }, + { args: reviewerArgs }, + ); + await hook.toolAfter( + { + tool: 'Task', + sessionID, + callID: 'reviewer-task-line', + args: reviewerArgs, + }, + { + output: `[REVIEWED] | task-${TASK_ID} | APPROVED | exact task approved`, + }, + ); + + const testEngineerArgs = { + subagent_type: 'test_engineer', + prompt: `TASK: ${TASK_ID}\nACCEPTANCE: test the exact task and report PASS`, + }; + await hook.toolBefore( + { tool: 'Task', sessionID, callID: 'test-engineer-task-line' }, + { args: testEngineerArgs }, + ); + await hook.toolAfter( + { + tool: 'Task', + sessionID, + callID: 'test-engineer-task-line', + args: testEngineerArgs, + }, + { output: `[TESTED] | task-${TASK_ID} | PASS | exact task passed` }, + ); + + const evidence = await readTaskEvidence(tmpDir, TASK_ID); + expect(evidence?.gates.reviewer).toBeDefined(); + expect(evidence?.gates.test_engineer).toBeDefined(); + expect(getTaskWorkflowSnapshot(evidence)).toMatchObject({ + generation, + state: 'tests_run', + authoritative: true, + }); + expect(await hasPassedAllGates(tmpDir, TASK_ID)).toBe(true); + }); + + it('preserves ambient plan-text routing for reviewer and test_engineer (F17)', async () => { + const sessionID = 'stage-b-plan-text'; + await seedStageA(sessionID); + const session = ensureAgentSession(sessionID, 'architect', tmpDir); + // Force this regression through the plan-text parser rather than the + // session fallback that is intentionally available to non-critic gates. + session.currentTaskId = null; + session.lastCoderDelegationTaskId = null; + const hook = createDelegationGateHook(makeConfig(), tmpDir); + const reviewerArgs = { + subagent_type: 'reviewer', + prompt: + 'Implementation plan excerpt:\n- [ ] 1.1: Review the implementation plan.\nACCEPTANCE: DONE = review complete.', + }; + await hook.toolBefore( + { tool: 'Task', sessionID, callID: 'reviewer-plan-text' }, + { args: reviewerArgs }, + ); + await hook.toolAfter( + { + tool: 'Task', + sessionID, + callID: 'reviewer-plan-text', + args: reviewerArgs, + }, + { + output: `[REVIEWED] | task-${TASK_ID} | APPROVED | plan text preserved`, + }, + ); + const testEngineerArgs = { + subagent_type: 'test_engineer', + prompt: + 'Implementation plan excerpt:\n- [ ] 1.1: Test the implementation plan.', + }; + await hook.toolBefore( + { tool: 'Task', sessionID, callID: 'test-engineer-plan-text' }, + { args: testEngineerArgs }, + ); + await hook.toolAfter( + { + tool: 'Task', + sessionID, + callID: 'test-engineer-plan-text', + args: testEngineerArgs, + }, + { output: `[TESTED] | task-${TASK_ID} | PASS | plan text preserved` }, + ); + + const evidence = await readTaskEvidence(tmpDir, TASK_ID); + expect(evidence?.gates.reviewer).toBeDefined(); + expect(evidence?.gates.test_engineer).toBeDefined(); + }); + + it('requires explicit attribution for every critic-family role (F15)', async () => { + const roles = [ + 'critic_drift_verifier', + 'critic_hallucination_verifier', + 'critic_architecture_supervisor', + ] as const; + for (const role of roles) { + const sessionID = `unbound-${role}`; + ensureAgentSession(sessionID, 'architect', tmpDir); + const hook = createDelegationGateHook(makeConfig(), tmpDir); + await settleCritic(hook, sessionID, `call-${role}`, { + subagent_type: role, + prompt: `MODE: CRITIC-GATE\nReview the plan; task 1.1 is mentioned in prose.`, + }); + } + + expect(await readTaskEvidence(tmpDir, TASK_ID)).toBeNull(); + }); +}); diff --git a/tests/unit/hooks/delegation-gate-critic-task-attribution-large-plan.test.ts b/tests/unit/hooks/delegation-gate-critic-task-attribution-large-plan.test.ts new file mode 100644 index 000000000..1a2502c54 --- /dev/null +++ b/tests/unit/hooks/delegation-gate-critic-task-attribution-large-plan.test.ts @@ -0,0 +1,119 @@ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { _internals as delegationGateInternals } from '../../../src/hooks/delegation-gate'; +import { TASK_ID_RESOLUTION_LIMITS } from '../../../src/hooks/task-id-resolver'; +import { ensureAgentSession, resetSwarmState } from '../../../src/state'; +import { safeRmRecursive } from '../../helpers/safe-test-dir'; +import { canonicalMkdtemp } from '../../helpers/tmpdir'; + +const { resolveEvidenceTaskId } = delegationGateInternals; +const LARGE_PLAN_TASK_ID = `1.${TASK_ID_RESOLUTION_LIMITS.maxKnownIds + 1}`; + +function makeLargePlan(): Record { + return { + schema_version: '1.0.0', + title: 'Large plan critic attribution', + swarm: 'test', + current_phase: 1, + phases: [ + { + id: 1, + name: 'Implementation', + status: 'in_progress', + tasks: Array.from( + { length: TASK_ID_RESOLUTION_LIMITS.maxKnownIds + 1 }, + (_, index) => ({ + id: `1.${index + 1}`, + phase: 1, + status: 'pending', + size: 'small', + description: `Task ${index + 1}`, + depends: [], + files_touched: [], + }), + ), + }, + ], + }; +} + +let directory: string; + +beforeEach(() => { + resetSwarmState(); + directory = canonicalMkdtemp('critic-large-plan-'); + fs.mkdirSync(path.join(directory, '.swarm'), { recursive: true }); + fs.writeFileSync( + path.join(directory, '.swarm', 'plan.json'), + JSON.stringify(makeLargePlan()), + 'utf8', + ); +}); + +afterEach(() => { + resetSwarmState(); + safeRmRecursive(directory); +}); + +describe('critic task attribution with over-limit plans', () => { + test('preserves explicit and exact-marker IDs from the full plan', async () => { + const session = ensureAgentSession( + 'critic-large-plan', + 'architect', + directory, + ); + session.currentTaskId = '1.1'; + const options = { + policy: 'attribution' as const, + allowSessionFallback: false, + }; + + expect( + await resolveEvidenceTaskId( + { task_id: LARGE_PLAN_TASK_ID }, + session, + directory, + options, + ), + ).toBe(LARGE_PLAN_TASK_ID); + expect( + await resolveEvidenceTaskId( + { prompt: `TASK: ${LARGE_PLAN_TASK_ID}` }, + session, + directory, + options, + ), + ).toBe(LARGE_PLAN_TASK_ID); + }); + + test('rejects explicit and marked IDs absent from the full plan', async () => { + const session = ensureAgentSession( + 'critic-large-plan-foreign', + 'architect', + directory, + ); + session.currentTaskId = '1.1'; + const options = { + policy: 'attribution' as const, + allowSessionFallback: false, + }; + + expect( + await resolveEvidenceTaskId( + { task_id: '9.9' }, + session, + directory, + options, + ), + ).toBeNull(); + expect( + await resolveEvidenceTaskId( + { prompt: 'TASK: 9.9' }, + session, + directory, + options, + ), + ).toBeNull(); + }); +}); diff --git a/tests/unit/hooks/delegation-gate-critic-task-attribution-public.test.ts b/tests/unit/hooks/delegation-gate-critic-task-attribution-public.test.ts new file mode 100644 index 000000000..2ca906cf0 --- /dev/null +++ b/tests/unit/hooks/delegation-gate-critic-task-attribution-public.test.ts @@ -0,0 +1,273 @@ +/** Public-hook guardrails for issue #2757 critic task attribution. */ + +import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { mkdirSync, writeFileSync } from 'node:fs'; +import * as path from 'node:path'; +import { createBackgroundCompletionObserver } from '../../../src/background/completion-observer'; +import { findByCorrelationId } from '../../../src/background/pending-delegations'; +import type { Plan } from '../../../src/config/plan-schema'; +import { closeProjectDb } from '../../../src/db/project-db'; +import { + getTaskWorkflowSnapshot, + hasPassedAllGates, + readTaskEvidence, + transitionTaskWorkflowEvidence, +} from '../../../src/gate-evidence'; +import { createDelegationGateHook } from '../../../src/hooks/delegation-gate'; +import { ensureAgentSession, resetSwarmState } from '../../../src/state'; +import { createIsolatedTestEnv } from '../../helpers/isolated-test-env'; +import { safeRmRecursive } from '../../helpers/safe-test-dir'; +import { canonicalMkdtemp } from '../../helpers/tmpdir'; +import { makeConfig } from './_delegation-gate-helpers'; + +const TASK_ONE = '1.1'; +const TASK_TWO = '1.2'; +const LARGE_TASK = '1.1025'; +const LARGE_MARKER_TASK = '1.1024'; + +function makeTask(id: string): Plan['phases'][number]['tasks'][number] { + return { + id, + phase: 1, + status: 'pending', + size: 'small', + description: `Implement ${id}`, + depends: [], + files_touched: [`src/task-${id.replaceAll('.', '-')}.ts`], + }; +} + +function makePlan(taskIds: string[]): Plan { + return { + schema_version: '1.0.0', + title: 'Critic attribution public-hook coverage', + swarm: 'mega', + current_phase: 1, + phases: [ + { + id: 1, + name: 'Implementation', + status: 'pending', + tasks: taskIds.map(makeTask), + }, + ], + }; +} + +function writePlan(directory: string, plan: Plan): void { + writeFileSync( + path.join(directory, '.swarm', 'plan.json'), + JSON.stringify(plan, null, 2), + 'utf8', + ); +} + +async function settleCritic( + hook: ReturnType, + sessionID: string, + callID: string, + args: Record, +): Promise { + await hook.toolBefore({ tool: 'Task', sessionID, callID }, { args }); + await hook.toolAfter( + { tool: 'Task', sessionID, callID, args }, + { state: 'completed', output: 'VERDICT: APPROVED\nReview complete.' }, + ); +} + +async function seedStageA(sessionID: string, taskId: string): Promise { + const accepted = await transitionTaskWorkflowEvidence(tmpDir, taskId, { + type: 'accepted_mutation', + agentType: 'coder', + expectedGeneration: 0, + transitionId: `seed-coder:${taskId}`, + }); + const generation = getTaskWorkflowSnapshot(accepted).generation; + await transitionTaskWorkflowEvidence(tmpDir, taskId, { + type: 'stage_a_passed', + expectedGeneration: generation, + transitionId: `seed-stage-a:${taskId}`, + }); + const session = ensureAgentSession(sessionID, 'architect', tmpDir); + session.currentTaskId = taskId; + session.taskWorkflowStates.set(taskId, 'pre_check_passed'); + return generation; +} + +let tmpDir: string; +let isolatedEnv: ReturnType | undefined; + +beforeEach(() => { + isolatedEnv = createIsolatedTestEnv(); + resetSwarmState(); + tmpDir = canonicalMkdtemp('dg-critic-public-'); + mkdirSync(path.join(tmpDir, '.opencode'), { recursive: true }); + mkdirSync(path.join(tmpDir, '.swarm'), { recursive: true }); +}); + +afterEach(() => { + resetSwarmState(); + closeProjectDb(tmpDir); + safeRmRecursive(tmpDir); + isolatedEnv?.cleanup(); + isolatedEnv = undefined; +}); + +describe('delegation-gate critic attribution public boundaries', () => { + it('requires explicit sounding-board attribution and preserves generation binding', async () => { + writePlan(tmpDir, makePlan([TASK_ONE, TASK_TWO])); + const unboundSession = ensureAgentSession( + 'sounding-board-unbound', + 'architect', + tmpDir, + ); + unboundSession.currentTaskId = TASK_ONE; + unboundSession.taskWorkflowStates.set(TASK_ONE, 'pre_check_passed'); + const hook = createDelegationGateHook(makeConfig(), tmpDir); + + await settleCritic(hook, 'sounding-board-unbound', 'unbound', { + subagent_type: 'critic_sounding_board', + prompt: 'Review the whole plan; its only implementation task is 1.1.', + }); + expect(await readTaskEvidence(tmpDir, TASK_ONE)).toBeNull(); + + const structuredGeneration = await seedStageA( + 'sounding-board-structured', + TASK_ONE, + ); + await settleCritic(hook, 'sounding-board-structured', 'structured', { + subagent_type: 'critic_sounding_board', + task_id: TASK_ONE, + prompt: 'Retry the exact task and return its approved verdict.', + }); + const structuredEvidence = await readTaskEvidence(tmpDir, TASK_ONE); + expect(structuredEvidence?.gates.critic_sounding_board?.agent).toBe( + 'critic_sounding_board', + ); + expect(getTaskWorkflowSnapshot(structuredEvidence)).toMatchObject({ + generation: structuredGeneration, + authoritative: true, + }); + + const markerGeneration = await seedStageA( + 'sounding-board-prefixed', + TASK_TWO, + ); + await settleCritic(hook, 'sounding-board-prefixed', 'marker', { + subagent_type: 'mega_critic_sounding_board', + prompt: `TASK: ${TASK_TWO}`, + }); + const markerEvidence = await readTaskEvidence(tmpDir, TASK_TWO); + expect(markerEvidence?.gates.critic_sounding_board?.agent).toBe( + 'critic_sounding_board', + ); + expect(getTaskWorkflowSnapshot(markerEvidence)).toMatchObject({ + generation: markerGeneration, + authoritative: true, + }); + }); + + it('records an explicitly bound background critic through completion ingestion', async () => { + writePlan(tmpDir, makePlan([TASK_ONE])); + const sessionID = 'background-critic-explicit'; + const hook = createDelegationGateHook( + makeConfig({ + hooks: { background_subagents: true }, + }), + tmpDir, + ); + const args = { + subagent_type: 'mega_critic', + task_id: TASK_ONE, + background: true, + prompt: 'Review this exact task and return a trusted verdict.', + }; + + await hook.toolBefore( + { tool: 'Task', sessionID, callID: 'background-call' }, + { args }, + ); + await hook.toolAfter( + { + tool: 'Task', + sessionID, + callID: 'background-call', + args, + }, + { + state: 'running', + output: + 'started', + metadata: { background: true, jobId: 'background-critic-job' }, + }, + ); + + const pending = findByCorrelationId(tmpDir, 'background-critic-child'); + expect(pending).toMatchObject({ + planTaskId: TASK_ONE, + evidenceTaskId: TASK_ONE, + workflowGeneration: 0, + status: 'pending', + }); + + const observer = createBackgroundCompletionObserver({ + config: { enabled: true }, + directory: tmpDir, + }); + await observer.event({ + event: { + type: 'message.part.updated', + properties: { + part: { + type: 'text', + synthetic: true, + sessionID, + text: + '\n' + + 'VERDICT: APPROVED\n', + }, + }, + }, + }); + + const completed = findByCorrelationId(tmpDir, 'background-critic-child'); + expect(completed?.status).toBe('consumed'); + expect(completed?.planTaskId).toBe(TASK_ONE); + expect(completed?.evidenceTaskId).toBe(TASK_ONE); + const evidence = await readTaskEvidence(tmpDir, TASK_ONE); + expect(evidence?.gates.critic).toBeDefined(); + expect(await hasPassedAllGates(tmpDir, TASK_ONE)).toBe(true); + }); + + it('uses real launch and settlement boundaries for over-limit plans', async () => { + const taskIds = Array.from( + { length: 1025 }, + (_, index) => `1.${index + 1}`, + ); + writePlan(tmpDir, makePlan(taskIds)); + const hook = createDelegationGateHook(makeConfig(), tmpDir); + + await settleCritic(hook, 'large-plan-valid', 'large-valid', { + subagent_type: 'critic', + task_id: LARGE_TASK, + prompt: 'Review the explicitly selected task.', + }); + const explicitEvidence = await readTaskEvidence(tmpDir, LARGE_TASK); + expect(explicitEvidence?.gates.critic).toBeDefined(); + expect(await hasPassedAllGates(tmpDir, LARGE_TASK)).toBe(true); + + await settleCritic(hook, 'large-plan-marker', 'large-marker', { + subagent_type: 'critic', + prompt: `TASK: ${LARGE_MARKER_TASK}`, + }); + const markerEvidence = await readTaskEvidence(tmpDir, LARGE_MARKER_TASK); + expect(markerEvidence?.gates.critic).toBeDefined(); + + await settleCritic(hook, 'large-plan-foreign', 'large-foreign', { + subagent_type: 'critic', + task_id: '9.9', + prompt: 'Review the explicitly selected task.', + }); + expect(await readTaskEvidence(tmpDir, '9.9')).toBeNull(); + }); +});