Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/planning.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ Each task in the swarm follows a per-task state machine. The Architect advances
| `pre_check_passed` | Automated gates passed | `pre_check_batch` returns `gates_passed: true` |
| `reviewer_run` | Human-style review complete | Reviewer delegation returns APPROVED |
| `tests_run` | Verification tests passed | Test engineer delegation returns PASS |
| `rework_required` | Current-generation verification failed and same-task repair is required | Stage A fails, or reviewer/test engineer returns a negative or malformed verdict |
| `rework_required` | Current-generation verification failed and same-task repair is required | Stage A fails, or reviewer/test engineer returns a negative or malformed verdict. A TESTED `SKIPPED` verdict (tests not run) is the exception: it stays Stage B eligible for test-gate re-dispatch instead of entering `rework_required` (#2756) |
| `blocked` | Task ended without completion and no verification debt remains | `update_task_status(status: 'blocked')` commits the terminal transaction |
| `closed` | A session ended with unfinished work; this is not successful completion | `/swarm close` commits a plan-bound `task_closed` transition |
| `complete` | Task fully complete | `update_task_status(status: 'completed')` called |
Expand Down
20 changes: 20 additions & 0 deletions docs/releases/pending/2756-skipped-verdict-stage-b.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Stop test_runner's dead-end scope advice and score TESTED SKIPPED as retryable, not failed

Issue: #2756

## What changed

- **`test_runner` remediation text** (`src/tools/test-runner.ts`): the guard that rejects `scope:"convention"`/`"graph"`/`"impact"` without `files`/`targets` no longer recommends `scope:"all"` — the exact scope its sibling guard blocks for agent use (env-gated `SWARM_ALLOW_FULL_SUITE`) and the `test_engineer` prompt prohibits. The `message` field now directs the caller to pass a non-empty `files` array (or `targets` for framework-native names) with a concrete example, and both response fields (`message` and `error`) enumerate every scope the guard covers. Both guards' rejection semantics are unchanged.
- **Foreground Stage B verdict settlement** (`src/hooks/delegation-gate.ts`): a `[TESTED] | task-N | SKIPPED | ...` structured verdict (tests were NOT run — prohibited scope, framework detection none, missing test file) no longer emits `stage_b_failed`. The task stays in its Stage B eligible state (`reviewer_run`/`pre_check_passed`), the reviewer's APPROVED gate proof is preserved, and the `stageBCompletion` entry is untouched, so the architect can re-dispatch the test gate instead of forcing a coder rework of correct code. A warn log records the skip for the orchestrator. Genuine `FAIL` verdicts and `REVIEWED` rejections keep the existing `stage_b_failed` → `rework_required` semantics.
- **Background Stage B ingestion** (`src/background/stage-b-gates.ts`): `structuredStageBVerdict` now returns `'skip'` for TESTED SKIPPED; `ingestBackgroundStageBCompletion` consumes the record with the new `skipped: true` result flag and fires no transition and no proof clearing. `StageBIngestionResult` gains the optional `skipped` field.
- **Background advisory** (`src/background/completion-observer.ts`): a skipped ingestion publishes `skipped (tests not run) — re-dispatch the test gate; reviewer proof preserved; task remains Stage B eligible` instead of the generic `ingestion failed`, so operators can distinguish a retryable skip from a hard failure.

## Why

An agent that followed the tool's own advice (`scope:"all"`) could not succeed — the recommended scope is blocked — and models without a natural `files:` habit (observed: Kimi K2.7 Code, 10–11 identical calls) looped until the repetition breaker fired. The prompt's SKIP CONDITION 1 then legitimately produced a `[TESTED] ... SKIPPED` verdict, which the gate scored as a code failure: `rework_required` plus deletion of the reviewer's approval for code that was correct and passing (`python -m pytest` green). Together with #2755 (no autonomous exit from `rework_required`, fixed by PR #2760's audited recovery tool), a single tool-argument mistake stranded tasks that only a human could free. This fix removes the wrongful entry: tests-not-run is retryable state, not failure.

## Tests

- `tests/unit/tools/test-runner-scope-advice.test.ts` — guard-2 response fields direct to files/targets, enumerate all three guarded scopes, and never recommend the blocked scope; guard-1 block characterized as unchanged.
- `tests/unit/hooks/delegation-gate-stage-b-skipped.test.ts` — SKIPPED leaves state `reviewer_run` with durable reviewer proof and the reviewer completion entry intact; FAIL and REVIEWED REJECTED still go `rework_required` with proof cleared.
- `tests/unit/background/stage-b-gates-skipped-verdict.test.ts` — SKIPPED ingest returns `skipped: true`, no state mutation, proof preserved; FAIL and unparseable output keep the fail-closed rejection.
16 changes: 9 additions & 7 deletions src/background/completion-observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -523,13 +523,15 @@ export function createBackgroundCompletionObserver(opts: {
directory,
record,
terminal.eventId,
legacyTransferPending
? 'ingestion failed; legacy coder settlement transfer is pending; durable reconciliation will retry'
: legacyTransferRequiresManualRecovery
? 'ingestion failed; legacy coder settlement requires manual recovery; run /swarm recover for this task (or /swarm reset-session)'
: applied.stale
? 'stale'
: 'ingestion failed',
applied.skipped
? 'skipped (tests not run) — re-dispatch the test gate; reviewer proof preserved; task remains Stage B eligible'
Comment thread
zaxbysauce marked this conversation as resolved.
: legacyTransferPending
? 'ingestion failed; legacy coder settlement transfer is pending; durable reconciliation will retry'
: legacyTransferRequiresManualRecovery
? 'ingestion failed; legacy coder settlement requires manual recovery; run /swarm recover for this task (or /swarm reset-session)'
: applied.stale
? 'stale'
: 'ingestion failed',
);
// Maintenance point P2b (issue #2104): the ingestion
// rejection has just been durably recorded — reconcile now
Expand Down
24 changes: 22 additions & 2 deletions src/background/stage-b-gates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ function structuredStageBVerdict(
role: StageBStateRole,
text: string,
taskId: string,
): 'pass' | 'fail' | null {
): 'pass' | 'fail' | 'skip' | null {
const escapedTaskId = taskId.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
const pattern =
role === 'reviewer'
Expand All @@ -97,7 +97,11 @@ function structuredStageBVerdict(
);
const match = pattern.exec(text);
if (!match) return null;
return match[1] === 'APPROVED' || match[1] === 'PASS' ? 'pass' : 'fail';
if (match[1] === 'APPROVED' || match[1] === 'PASS') return 'pass';
// SKIPPED means the tests were not run (issue #2756): a tool-argument
// outcome the caller can retry, not a code failure.
if (match[1] === 'SKIPPED') return 'skip';
return 'fail';
}

function normalizeAttributionPath(file: string): string | null {
Expand Down Expand Up @@ -218,6 +222,10 @@ export interface StageBIngestionResult {
ok: boolean;
consumed: boolean;
stale?: boolean;
/** True when the gate reported a not-run outcome (TESTED SKIPPED): no
* transition fired, no proof was cleared, and the task remains Stage B
* eligible for re-dispatch (issue #2756). */
skipped?: boolean;
reason?: string;
}

Expand Down Expand Up @@ -624,6 +632,18 @@ export async function ingestBackgroundStageBCompletion(args: {
const verdict = stageBRole
? structuredStageBVerdict(stageBRole, args.result.text ?? '', taskId)
: null;
if (stageBRole && verdict === 'skip') {
// TESTED SKIPPED = tests not run (issue #2756): consume the record so
// the observer publishes the skip advisory and no retry loop forms,
// but fire NO stage_b_failed transition and clear no gate proof —
// the task stays Stage B eligible for a test-gate re-dispatch.
return {
ok: false,
consumed: true,
skipped: true,
reason: `background ${stageBRole} skipped task ${taskId} — tests not run; re-dispatch the test gate (task stays Stage B eligible; reviewer proof preserved)`,
};
}
if (stageBRole && verdict !== 'pass') {
const rejected = await transitionTaskWorkflowEvidence(
args.directory,
Expand Down
15 changes: 15 additions & 0 deletions src/hooks/delegation-gate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5948,6 +5948,21 @@ export function createDelegationGateHook(
const verdictEntry = attributionResult.verdicts.get(taskId);
const dispatchCtxForVerdict =
stageBDispatchContextByCallID.get(input.callID);
// A SKIPPED TESTED verdict means the tests were not run
// (e.g. prohibited scope, framework detection none) — a
// tool-argument outcome, not a code failure. Leave the
// task in its Stage B eligible state with the reviewer
// proof intact so the architect can re-dispatch the test
// gate instead of forcing a coder rework (issue #2756).
if (
dispatchCtxForVerdict?.expectedVerdictKind === 'TESTED' &&
verdictEntry?.verdict === 'SKIPPED'
) {
logger.warn(
`[delegation-gate] Stage B test gate SKIPPED (tests not run) for task ${taskId} from call ${input.callID} — leaving state ${state} for test-gate re-dispatch; reviewer proof preserved`,
);
continue;
}
const positiveVerdict =
dispatchCtxForVerdict?.expectedVerdictKind === 'TESTED'
? verdictEntry?.verdict === 'PASS'
Expand Down
4 changes: 2 additions & 2 deletions src/tools/test-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3164,9 +3164,9 @@ export const test_runner: ReturnType<typeof tool> = createSwarmTool({
framework: 'none',
scope,
error:
'scope "convention" and "graph" require explicit files or targets array - omitting both causes unsafe full-project discovery',
'scope "convention", "graph", and "impact" require explicit files or targets array - omitting both causes unsafe full-project discovery',
message:
'When using scope "convention" or "graph", you must provide a non-empty "files" or "targets" array. Use scope "all" for full project test suite without specifying files.',
'When using scope "convention", "graph", or "impact", you must provide a non-empty "files" array (or "targets" for framework-native test names). Example: { scope: "convention", files: ["tests/test_calc.py"] }',
outcome: 'error',
resolution: makeResolution(scope, scope, [], [], 'skip', workingDir),
};
Expand Down
Loading
Loading