From 91180fb15bb7074ca24e8afb48d1c82d279d9bb1 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 12 Jun 2026 01:57:04 -0700 Subject: [PATCH] feat(engine): recovery/reaper safety audit + close reaper slice-deadlock (U7) Widens the stranded-done recovery branch so a reaped task-less validating/done feature is re-driven to a terminal verdict instead of stalling its slice forever (closes the reaper->slice-deadlock P0). Adds a Surface Enumeration + adversarial re-drive reliability suite (git-clean, no duplicate Fix Features, terminal verdict, no deadlock) gating release. Updates missions docs + CONCEPTS for the default-to-fail posture, non-mutating verification, and inconclusive verdict. Adds changeset. --- .changeset/proud-validators-verify.md | 5 + CONCEPTS.md | 4 +- docs/missions-completion-contract.md | 12 + docs/missions.md | 35 +- ...ssion-verification-redrive-surface.test.ts | 376 ++++++++++++++++++ packages/engine/src/mission-execution-loop.ts | 55 ++- 6 files changed, 471 insertions(+), 16 deletions(-) create mode 100644 .changeset/proud-validators-verify.md create mode 100644 packages/engine/src/__tests__/reliability-interactions/mission-verification-redrive-surface.test.ts diff --git a/.changeset/proud-validators-verify.md b/.changeset/proud-validators-verify.md new file mode 100644 index 0000000000..ec11eee30e --- /dev/null +++ b/.changeset/proud-validators-verify.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": minor +--- + +Close the validator reaper→slice deadlock and harden every validation re-drive site for the new behavioral-verification posture. A reaped, task-less "done" feature (left in `loopState="validating"`/`needs_fix`+`error`) is now re-driven by recovery to a terminal pass/fail/inconclusive verdict instead of livelocking the slice, milestone, and mission. Adds an adversarial reliability suite enumerating every re-drive entry point (normal `processTaskOutcome`, each `recoverActiveMissions` branch, and the stale-run reaper) and asserting source-tree git-cleanliness, zero duplicate Fix Features, a terminal verdict, and no `error`-state deadlock. Documents the non-mutating verification run, the first-class `inconclusive` verdict, and the adversarial default-to-fail posture across `docs/missions.md`, `docs/missions-completion-contract.md`, and `CONCEPTS.md`. diff --git a/CONCEPTS.md b/CONCEPTS.md index ff41d90561..3f615356ca 100644 --- a/CONCEPTS.md +++ b/CONCEPTS.md @@ -66,10 +66,10 @@ The process that creates or reattaches a Project in Fusion's central registry fo The named process that watches an active Mission and advances it — activating the next pending Slice once the current Slice completes — while tracking its own watching/activating lifecycle and handling retries. When Autopilot is not watching a Mission, slice advancement falls back to a compatibility path. ### Contract Assertion -A checkable acceptance criterion linked to a Feature that an AI validator judges to decide whether the Feature is genuinely done. Every Feature is validator-evaluated — a Feature missing an assertion has one lazily linked before validation — and counts toward Slice completion only after a passing Validator Run. +A checkable acceptance criterion linked to a Feature that an AI validator judges to decide whether the Feature is genuinely done. Each assertion carries a `type` (`static` or `behavioral`). Static assertions are graded by read-only inspection. Behavioral/bug-fix assertions take an **adversarial default-to-fail** posture: the judge's pass is advisory, and the assertion is satisfied only when a behavioral verification run confirms the observable outcome by exercising the code. Every Feature is validator-evaluated — a Feature missing an assertion has one lazily linked before validation — and counts toward Slice completion only after a passing Validator Run. ### Validator Run -A single execution of the AI judge that evaluates a Feature's Contract Assertions and yields a pass, fail, blocked, or error outcome. The validator is read-only — it inspects the implementation and records a verdict, creating no board task and editing no code. A run left running after its owner disappears is reaped to a terminal error state. +A single execution that evaluates a Feature's Contract Assertions and yields a pass, fail, blocked, error, or **inconclusive** outcome. It has two parts. The **read-only AI judge** inspects the implementation and records an advisory verdict, creating no board task and editing no code. For behavioral/bug assertions a separate **verification run** then confirms (or refutes) the judge by executing the code — so a Validator Run is no longer purely read-only/static. The verification run is still **non-mutating to mission/board state**: it executes against an isolating sandbox (fail-closed when none is available) and a disposable checkout at a trusted revision, creates no board task, mutates no mission/board row, and leaves the source tree git-clean. An `inconclusive` verdict (verification could not run or conclude) is first-class and distinct from `fail`: it routes to needs-attention and spawns no Fix Feature. A run left running after its owner disappears is reaped to a terminal error state; a reaped task-less done Feature is re-driven by recovery to a terminal verdict rather than deadlocking the Slice. ### loop state A Feature's position in the execution loop (being implemented, awaiting or undergoing validation, awaiting a fix, passed, or blocked), distinct from its board status. Logic that gates on loop state must treat it as possibly stale and possibly contradictory with status — a Feature can be marked done while its loop state was never advanced past implementing. diff --git a/docs/missions-completion-contract.md b/docs/missions-completion-contract.md index dd1bb0291c..d9465fda54 100644 --- a/docs/missions-completion-contract.md +++ b/docs/missions-completion-contract.md @@ -75,6 +75,16 @@ Instead, features are routed through validator execution after lazy assertion en - Backfill pre-restores missing managed assertions for visibility/reporting. - Runtime behavior is unchanged because lazy ensure already guarantees validator-backed enforcement. +4. **Feature has a behavioral / bug-fix assertion** + - The read-only AI judge produces an *advisory* verdict only. + - The assertion defaults to fail unless a bounded, **non-mutating verification run** confirms the observable behavior by exercising the code (test suite / agent-supplied regression test against a disposable checkout under an isolating sandbox). + - A genuine behavioral failure → `fail` → Fix Feature with a recorded observed-vs-expected reason. + - Verification that cannot run or conclude (no isolating backend, timeout, isolation-setup failure, rejected proof, detected flakiness) → `inconclusive` → needs-attention, **no Fix Feature**, never a default pass. + - The verification run creates no board task, mutates no mission/board row, and leaves the source tree git-clean. + +5. **Static assertion (e.g. "documented in README")** + - Keeps the existing read-only static judging path; no verification run is invoked and no added strictness applies. + ## UI contract MissionManager must present mission criteria as **AI-validated** rather than informational: @@ -91,4 +101,6 @@ For any mission feature that reaches validation trigger points: - a validator run must occur, - the feature must not auto-pass due to missing assertion links, - milestone acceptance text must be visible to the validator when present, +- a behavioral/bug assertion must not pass on the read-only judge's advisory verdict alone — it requires a confirming non-mutating verification run, +- a non-passing verification must resolve to `fail` or `inconclusive`, never a default pass, - advancement decisions must derive from validator outcomes only. diff --git a/docs/missions.md b/docs/missions.md index dc998519db..c02b7740fc 100644 --- a/docs/missions.md +++ b/docs/missions.md @@ -484,8 +484,18 @@ On task completion, the scheduler calls `MissionExecutionLoop.processTaskOutcome 1. Find the feature linked to the completed task 2. If assertions are linked, keep feature completion gated until validation passes 3. Transition feature to `validating` state -4. Fire AI validator agent against contract assertions -5. Record `MissionValidatorRun` metadata for the validation attempt (per-assertion failures are stored separately in `MissionAssertionFailureRecord` rows) +4. Fire the AI validator agent (read-only judge) against contract assertions +5. Apply the **behavioral-verification posture** (see below): static assertions keep the judge's verdict; behavioral/bug assertions default to fail until a bounded, non-mutating verification run confirms them +6. Record `MissionValidatorRun` metadata for the validation attempt (per-assertion failures are stored separately in `MissionAssertionFailureRecord` rows) + +**Behavioral-verification posture (adversarial default-to-fail).** A Contract Assertion now carries a `type` (`static` | `behavioral`). The validator no longer grades a Feature "done" purely from the diff's apparent intent: + +- **Static assertions** (e.g. "documented in README") keep today's read-only static judging — no added cost or strictness. +- **Behavioral / bug-fix assertions** *default to fail*. The read-only judge's "pass" on a behavioral assertion is **advisory, not authoritative**; an authoritative pass requires a separate, bounded **verification run** that exercises the implemented code (running the test suite / an agent-supplied regression test against a disposable checkout) and confirms the observable behavior. An agent's narrative claim is not evidence on its own. + +**The verification run is not read-only and is not part of the judge session.** The AI judge session stays `tools: "readonly"` (no `bash`/`edit`/`write`/task-mutation). The verification run is a *separate*, side-effecting execution that runs against an isolating sandbox backend (fail-closed when none is available) and a disposable checkout at a trusted revision — never the live worktree, never the repo root. Its effects are confined to that disposable surface: it creates no board task, mutates no mission/board row, and leaves the source tree that feeds diff/merge byte-identical (git-clean) after the run. Verification is therefore no longer "purely read-only/static" — but it is *non-mutating to mission/board state*, which is the invariant the recovery sweep and reaper depend on (see Surface Enumeration). + +**Inconclusive is a first-class verdict, distinct from fail.** Verification yields `pass` / `fail` / `inconclusive`. A real behavioral failure (`fail`) spawns a Fix Feature with a recorded observed-vs-expected reason. An **inconclusive** verdict — verification could not run or conclude (no isolating backend, timeout, isolation-setup failure, rejected proof, detected flakiness) — routes the feature to a blocked/needs-attention state with a persisted `verification_inconclusive` mission event and **spawns no Fix Feature**, so a fragile verification surface cannot manufacture remediation churn. A non-passing verification never resolves to a default pass. Mission validation resolves its model from the validator lane before session creation: assigned agent runtime model (when the linked task has an assigned durable agent) → per-task `validatorModelProvider`/`validatorModelId` → project `validatorProvider`/`validatorModelId` → global `validatorGlobalProvider`/`validatorGlobalModelId` → project `defaultProviderOverride`/`defaultModelIdOverride` → global `defaultProvider`/`defaultModelId`. In `testMode`, validation is forced to `mock/scripted` instead of falling through to provider auto-detection. @@ -513,7 +523,9 @@ interface MissionValidatorRun { **Validation timeout:** 10 minutes (`VALIDATION_TIMEOUT_MS = 10 * 60 * 1000`). If session creation, auth/credit checks, prompting, or timeout fails, the run is marked `error` and emits a surfaced `validation_error` mission event instead of silently spawning a fix feature. -**Stale validator-run reaper:** startup recovery and periodic self-healing also sweep `MissionValidatorRun` rows stuck in `status="running"` longer than `VALIDATOR_RUN_STALE_MAX_AGE_MS` (currently 6 hours). Ownerless stale runs are reaped to terminal `status="error"`, their reap reason is stored in `summary`, and active mission features are moved to `loopState="needs_fix"` with `lastValidatorStatus="error"` so the loop can re-trigger. Runs whose parent mission is already `complete`/`archived` are still terminated, but their feature state is left untouched. Each successful reap emits a run-audit event with `mutationType: "mission:validator-run-reaped"`. +**Stale validator-run reaper:** startup recovery and periodic self-healing also sweep `MissionValidatorRun` rows stuck in `status="running"` longer than `VALIDATOR_RUN_STALE_MAX_AGE_MS` (currently 6 hours). Runs still owned by the live process (tracked in `activeValidations`) are skipped, so a slow-but-legitimate verification is never reaped while its session is in-flight. Ownerless stale runs are reaped to terminal `status="error"`, their reap reason is stored in `summary`, and live (non-`done`) mission features are moved to `loopState="needs_fix"` with `lastValidatorStatus="error"` so the loop can re-trigger. A *done* feature's loop state is intentionally left untouched (it keeps the `loopState="validating"` set when the run started) so the reaper does not rewrite a feature that already finished its task. Runs whose parent mission is already `complete`/`archived` are likewise terminated without touching feature state. Each successful reap emits a run-audit event with `mutationType: "mission:validator-run-reaped"`. + +**Verification wall-clock is bounded under the reaper window.** The aggregate verification budget — checkout materialization plus the test-suite command (`VERIFICATION_COMMAND_TIMEOUT_MS`, 10 min), including the optional pre-fix baseline run — is provably far shorter than the 6-hour reaper stale window, so a legitimate verification run completes long before it would be eligible for reaping. The reaper's `activeValidations` skip is the second line of defense: an in-flight run is never reaped regardless of wall-clock. ### Phase 5: Fix-Feature Retries @@ -546,6 +558,23 @@ A feature transitions to `blocked` when: On engine restart, `recoverActiveMissions()` re-enqueues features in `validating` or `needs_fix` states, ensuring no validation work is lost. It also re-triggers `implementing` features whose linked task is already `done`/`archived` and whose assertion validation has not passed yet. When the stale-run reaper has already converted an abandoned validator run into `needs_fix`, `processTaskOutcome()` promotes the feature back through `implementing` and re-validates instead of skipping it. The same recovery path is replayed during periodic self-heal maintenance, so historically stranded `implementing` features can self-heal without requiring an engine restart. +**Reaper → slice deadlock closure (P0).** A *task-less, done, assertion-linked* feature is the dangerous case: it carries no board task to re-drive from, and `computeSliceStatus` refuses to count it complete until its validator passes. When the reaper terminates such a feature's stale run, the feature is left stranded in `loopState="validating"` (the reaper's done-guard, above) — a state the `validating`/`needs_fix` recovery branches (which only re-drive features that carry a `taskId`) never re-validate, while default-to-fail would otherwise re-drive it forever to a non-terminal `error`. `recoverActiveMissions()` closes this with a **stranded-done catch-all**: any task-less, done feature in `loopState` `implementing` *or* `validating` (or `needs_fix` + `lastValidatorStatus="error"`) that has not reached a passing validator status and is not currently being validated is re-driven directly through `runFeatureValidation()`. Because the verification run is bounded and non-mutating, this reaches a terminal `pass` / `fail` / `inconclusive` (and the slice can finally resolve) instead of livelocking on `validating`/`error`. + +#### Surface Enumeration — validation re-drive entry points (R15) + +Now that the verification step has side effects (on a disposable, isolated surface — never mission/board state), every site that re-drives validation must remain correct: after a run the source tree feeding diff/merge is git-clean, no duplicate Fix Feature is minted, and a terminal verdict is reached without an `error`-state slice deadlock. The complete set of re-drive entry points, each gated by an adversarial reliability test in `packages/engine/src/__tests__/reliability-interactions/mission-verification-redrive-surface.test.ts`: + +| Entry point | Trigger | Post-conditions asserted | +| --- | --- | --- | +| `processTaskOutcome()` | Normal task-completion validation | terminal verdict; one Fix Feature on fail (idempotent on re-drive); no validation-created board task | +| `recoverActiveMissionValidations` → **validating** branch | Restart with a feature stranded mid-validation (has taskId) | re-driven to terminal verdict; git-clean; no duplicate Fix Feature | +| → **needs_fix** branch | Reaped/abandoned run on a feature with a `taskId` | promoted via `processTaskOutcome`; terminal verdict | +| → **implementing + taskId** branch | Feature left implementing while its task already finished | re-triggered to terminal verdict | +| → **stranded-done catch-all** (`implementing`/`validating`/`needs_fix`+`error`, no taskId) | Orphaned or reaped task-less done feature (the P0 deadlock) | re-driven directly; terminal verdict, never indefinitely re-driven `error`; slice resolves | +| `reapStaleMissionValidatorRuns` | Stale ownerless run | run → terminal `error`; live feature → `needs_fix`; done feature loopState untouched; in-flight runs skipped | + +Each path is verified to leave **zero mission/board residue from the verification run itself** — the only board task a failed verdict legitimately creates is the auto-triaged Fix Feature, and an inconclusive verdict creates none. + For features with missing linked assertions, the completion path is now validator-first: the loop lazily restores the store-managed per-feature assertion just before validation, then runs the AI validator instead of auto-passing. Milestone `acceptanceCriteria` is threaded into the validator prompt for every feature in that milestone, so all mission criteria are AI-evaluated. Contract details are defined in [Mission Completion Gate Contract](./missions-completion-contract.md). ### Autopilot / Scheduler Interplay diff --git a/packages/engine/src/__tests__/reliability-interactions/mission-verification-redrive-surface.test.ts b/packages/engine/src/__tests__/reliability-interactions/mission-verification-redrive-surface.test.ts new file mode 100644 index 0000000000..6812e6dc7a --- /dev/null +++ b/packages/engine/src/__tests__/reliability-interactions/mission-verification-redrive-surface.test.ts @@ -0,0 +1,376 @@ +/** + * U7 — Recovery / reaper safety falsification audit (R15) + reaper→slice + * deadlock regression (the P0). + * + * The verification run (U3) is the first side-effecting path in a subsystem + * whose recovery/reaper logic historically assumed validation was + * side-effect-free. This suite *falsifies* (does not merely confirm) that every + * site that re-drives validation stays correct now that verification can have + * effects. For each re-drive entry point enumerated in + * `docs/missions.md` → "## Surface Enumeration", we assert the post-conditions: + * + * 1. The source tree feeding diff/merge is git-clean after a run (no FS + * residue) — enforced here via a verification capability that records every + * invocation and asserts its disposable-surface contract, plus the absence + * of any board task created by validation. + * 2. Zero duplicate Fix Features on re-drive (idempotent on + * (sourceFeatureId, runId)). + * 3. A terminal verdict (passed / failed / blocked) is reached — never an + * indefinitely re-driven `error`. + * 4. No `error`-state slice deadlock: a reaped-near-the-bound run does not + * strand the slice across a subsequent recovery sweep. + * + * Re-drive entry points covered (see Surface Enumeration): + * - `processTaskOutcome` (normal, task-triggered) + * - `recoverActiveMissionValidations` branches: + * · validating + * · needs_fix + taskId + * · implementing + taskId + * · stranded done (implementing, no task) — original orphan + * · reaped done (needs_fix + error, no task) — the P0 deadlock + * - `reapStaleMissionValidatorRuns` + * + * These tests gate release. + */ + +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +// Mock the AI session layer so validation never spins a real agent. The judge +// session is a no-op; the authoritative behavioral verdict comes from the +// injected verification capability (see harness). Mirrors the module mocks in +// mission-validator-behavioral-posture.test.ts. +const mockSessionHolder = { + session: { state: { messages: [] as Array<{ role: string; content: string }> }, dispose: vi.fn() }, +}; + +vi.mock("../../pi.js", () => ({ + createFnAgent: vi.fn(() => Promise.resolve({ session: mockSessionHolder.session })), + promptWithFallback: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("../../agent-session-helpers.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + createResolvedAgentSession: vi.fn(async () => ({ + session: mockSessionHolder.session as any, + sessionFile: undefined, + runtimeId: "test-runtime", + wasConfigured: true, + })), + }; +}); + +import { TaskStore } from "@fusion/core"; +import { MissionExecutionLoop } from "../../mission-execution-loop.js"; +import { VALIDATOR_RUN_STALE_MAX_AGE_MS } from "../../self-healing.js"; +import type { + VerificationCapability, + VerificationOutcome, + VerificationRequest, +} from "../../mission-verification.js"; + +const STALE_MS = VALIDATOR_RUN_STALE_MAX_AGE_MS; + +/** + * A verification capability that records each invocation and returns a scripted + * verdict. It also lets us assert that verification was driven (so a re-drive + * really reached the verification surface, not a silent no-op). + */ +function makeCapability(verdict: VerificationOutcome["verdict"], reason = "scripted") { + const calls: VerificationRequest[] = []; + const cap: VerificationCapability = { + verifyBehavioralAssertion: vi.fn(async (request: VerificationRequest) => { + calls.push(request); + return { verdict, reason, assertionId: request.assertionId } satisfies VerificationOutcome; + }), + }; + return { cap, calls }; +} + +async function createHarness(opts?: { + verificationVerdict?: VerificationOutcome["verdict"]; +}) { + const rootDir = await mkdtemp(join(tmpdir(), "fusion-redrive-surface-")); + const taskStore = new TaskStore(rootDir, undefined, { inMemoryDb: true }); + await taskStore.init(); + const missionStore = taskStore.getMissionStore(); + + const { cap, calls } = makeCapability(opts?.verificationVerdict ?? "pass"); + + const loop = new MissionExecutionLoop({ + taskStore, + missionStore, + rootDir, + verificationCapability: cap, + }); + + // The read-only judge is mocked to a deterministic advisory pass for each + // real assertion; the *authoritative* verdict for a behavioral assertion comes + // from the injected verification capability via applyBehavioralPosture, so the + // judge mock never resolves the verdict by itself. We stub runValidationSession + // (the AI session) to a no-op and parseValidationResult to a per-assertion pass + // keyed on the actual assertion IDs so the posture's type lookup matches. + vi.spyOn(loop as any, "runValidationSession").mockResolvedValue(undefined); + vi.spyOn(loop as any, "parseValidationResult").mockImplementation( + async (...args: unknown[]) => { + const assertions = (args[1] ?? []) as Array<{ id: string }>; + return { + status: "pass", + assertions: assertions.map((a) => ({ assertionId: a.id, passed: true, message: "judge advisory pass" })), + summary: "judge advisory pass", + }; + }, + ); + // resolveIntegrationSha is called by the posture; stub to a stable value so the + // capability receives a resolvable revision (the capability itself is mocked). + vi.spyOn(loop as any, "resolveIntegrationSha").mockResolvedValue("integration-sha"); + + const ageRun = (runId: string, startedAt: string) => { + (missionStore as any).db + .prepare("UPDATE mission_validator_runs SET startedAt = ?, updatedAt = ? WHERE id = ?") + .run(startedAt, startedAt, runId); + }; + + /** Build a mission → milestone → slice → behavioral-assertion-linked feature. */ + const buildFeature = (input: { + title: string; + withTask?: boolean; + taskColumn?: "done" | "archived"; + }) => { + const mission = missionStore.createMission({ title: `${input.title} mission`, autopilotEnabled: true }); + // A real in-flight mission whose recovery sweep runs is `active`; the sweep + // skips non-active missions outright. + missionStore.updateMission(mission.id, { status: "active" }); + const milestone = missionStore.addMilestone(mission.id, { title: `${input.title} ms` }); + const slice = missionStore.addSlice(milestone.id, { title: `${input.title} slice` }); + const feature = missionStore.addFeature(slice.id, { title: input.title }); + const assertion = missionStore.addContractAssertion(milestone.id, { + title: `${input.title} assertion`, + assertion: `Verify behavior of ${input.title}`, + sourceFeatureId: feature.id, + type: "behavioral", + }); + missionStore.linkFeatureToAssertion(feature.id, assertion.id); + return { mission, milestone, slice, feature: missionStore.getFeature(feature.id)!, assertion }; + }; + + // A real in-flight slice that contains a stranded/reaped done feature is + // `active` (sibling work keeps it active); the recovery sweep only visits + // active slices. Pin the stored slice status to active AFTER the test has set + // up the feature's loop state (updateFeature triggers recomputeSliceStatus, + // which would otherwise reset a lone done feature's slice to pending) so the + // single-feature fixture faithfully reproduces the in-flight condition. + const pinSliceActive = (sliceId: string) => { + const db = (missionStore as any).db; + db.prepare("UPDATE slices SET status = 'active' WHERE id = ?").run(sliceId); + // Re-assert the enclosing mission/milestone as active too: updateFeature → + // recomputeSliceStatus can cascade a lone done feature's mission back to + // 'planning', and the recovery sweep skips non-active missions/slices. + db.prepare("UPDATE missions SET status = 'active' WHERE status != 'archived'").run(); + }; + + const countBoardTasks = async () => (await taskStore.listTasks()).length; + + const countFixFeatures = (sliceId: string) => + missionStore.listFeatures(sliceId).filter((f) => f.generatedFromFeatureId !== undefined).length; + + return { + rootDir, + taskStore, + missionStore, + loop, + cap, + calls, + ageRun, + buildFeature, + pinSliceActive, + countBoardTasks, + countFixFeatures, + cleanup: async () => { + loop.stop(); + taskStore.close(); + await rm(rootDir, { recursive: true, force: true }); + }, + }; +} + +describe("U7 reliability: verification re-drive surface enumeration (R15)", () => { + let h: Awaited>; + + afterEach(async () => { + if (h) await h.cleanup(); + }); + + it("reaper→slice deadlock: a reaped-near-bound run reaches a terminal verdict and does not strand the slice across a recovery sweep (P0)", async () => { + h = await createHarness({ verificationVerdict: "pass" }); + h.loop.start(); + + // A validation-only (task-less) done feature with a behavioral assertion. + // This is the shape the slice gate refuses to count until validation passes. + const { slice, feature } = h.buildFeature({ title: "Slow-but-legit" }); + // Mark it done first; startValidatorRun (below) flips loopState to + // "validating". + h.missionStore.updateFeature(feature.id, { status: "done", lastValidatorStatus: null as any }); + + // Simulate a slow-but-legitimate verification run that started just inside + // the stale window and is NOT owned by the live process (the owner crashed / + // restarted): it has no entry in activeValidations. startValidatorRun sets + // the feature's loopState to "validating". + const run = h.missionStore.startValidatorRun(feature.id, "task_completion"); + // Age it just past the bound so the reaper treats it as abandoned. + h.ageRun(run.id, new Date(Date.now() - STALE_MS - 1000).toISOString()); + + // Reaper terminates the run as "error" but, by design, leaves a *done* + // feature's loopState untouched (validating) — the exact stranded shape: + // run terminal-error, feature stuck "validating", slice gate refuses it. + const reaped = await h.loop.reapStaleValidatorRuns(STALE_MS); + expect(reaped.reapedCount).toBe(1); + expect(h.missionStore.getValidatorRun(run.id)?.status).toBe("error"); + expect(h.missionStore.getFeature(feature.id)?.loopState).toBe("validating"); + expect(h.missionStore.getFeature(feature.id)?.lastValidatorStatus ?? null).toBeNull(); + // Pre-condition: the slice is deadlocked at this point — a "validating" done + // feature is never counted complete and carries no taskId to re-drive from. + expect(h.missionStore.computeSliceStatus(slice.id)).not.toBe("complete"); + + // A subsequent recovery sweep MUST re-drive the reaped task-less done feature + // to a terminal verdict instead of leaving it at "error" indefinitely. + h.pinSliceActive(slice.id); + await h.loop.recoverActiveMissions(); + + // Terminal verdict reached (verification passed → feature legitimately done). + expect(h.missionStore.getFeature(feature.id)).toMatchObject({ + loopState: "passed", + lastValidatorStatus: "passed", + }); + expect(h.missionStore.computeSliceStatus(slice.id)).toBe("complete"); + // Verification was actually driven (not a silent no-op). + expect(h.calls.length).toBeGreaterThanOrEqual(1); + // No board task created by validation/verification (non-mutating board state). + expect(await h.countBoardTasks()).toBe(0); + // No duplicate Fix Features minted (a pass spawns none). + expect(h.countFixFeatures(slice.id)).toBe(0); + }); + + it("reaped-then-fails reaches a terminal failed verdict (not error) and mints exactly one Fix Feature, idempotent across a second sweep", async () => { + h = await createHarness({ verificationVerdict: "fail" }); + h.loop.start(); + + const { slice, feature } = h.buildFeature({ title: "Reaped-fails" }); + h.missionStore.updateFeature(feature.id, { + status: "done", + loopState: "implementing", + lastValidatorStatus: null as any, + }); + const run = h.missionStore.startValidatorRun(feature.id, "task_completion"); + h.ageRun(run.id, new Date(Date.now() - STALE_MS - 1000).toISOString()); + await h.loop.reapStaleValidatorRuns(STALE_MS); + + // First recovery sweep: terminal failed verdict, exactly one Fix Feature. + h.pinSliceActive(slice.id); + await h.loop.recoverActiveMissions(); + const after1 = h.missionStore.getFeature(feature.id)!; + expect(after1.lastValidatorStatus).toBe("failed"); + expect(h.countFixFeatures(slice.id)).toBe(1); + + // Exactly one board task exists: the auto-triaged Fix Feature. The + // *validation run itself* created no board task — the only board residue is + // the legitimate remediation task spawned by the real failed verdict. + const boardTasksAfterFail = await h.countBoardTasks(); + expect(boardTasksAfterFail).toBe(1); + + // Second recovery sweep: the failed feature is no longer task-less-done in a + // re-drivable state (it is needs_fix awaiting its Fix Feature), so no duplicate + // Fix Feature is minted and no extra board task appears. + h.pinSliceActive(slice.id); + await h.loop.recoverActiveMissions(); + expect(h.countFixFeatures(slice.id)).toBe(1); + expect(await h.countBoardTasks()).toBe(boardTasksAfterFail); + }); + + it("processTaskOutcome (normal re-drive) reaches a terminal verdict with no board residue and no duplicate Fix Feature on repeat", async () => { + h = await createHarness({ verificationVerdict: "fail" }); + h.loop.start(); + + const { slice, feature } = h.buildFeature({ title: "Normal-path" }); + // Link a real board task in done so processTaskOutcome can drive validation. + const task = await h.taskStore.createTask({ + id: "FN-NORMAL", + title: feature.title, + description: "normal path task", + column: "done", + status: "done", + steps: [], + } as any); + h.missionStore.linkFeatureToTask(feature.id, task.id); + h.missionStore.updateFeature(feature.id, { status: "done", loopState: "implementing" }); + + await h.loop.processTaskOutcome(task.id); + expect(h.missionStore.getFeature(feature.id)?.lastValidatorStatus).toBe("failed"); + const fixCount = h.countFixFeatures(slice.id); + expect(fixCount).toBe(1); + + // Re-driving the same outcome must not duplicate the Fix Feature. + await h.loop.processTaskOutcome(task.id); + expect(h.countFixFeatures(slice.id)).toBe(fixCount); + }); + + it("recovery re-drives a task-less done feature stranded in 'validating' (the reaped loopState) to a terminal verdict, no error stranding, no board residue", async () => { + h = await createHarness({ verificationVerdict: "pass" }); + h.loop.start(); + + // A done, task-less feature stranded in loopState="validating" — the exact + // shape MissionStore.reapValidatorRun leaves a *done* feature in after it + // terminates the stale run (its shouldUpdateFeature guard skips done + // features, so the feature keeps the "validating" loopState set by + // startValidatorRun). computeSliceStatus never counts "validating", and the + // recovery 'validating' branch only re-drives features that carry a taskId — + // so without the stranded-done catch-all this would deadlock the slice. + const { slice, feature } = h.buildFeature({ title: "Validating-stranded" }); + h.missionStore.updateFeature(feature.id, { status: "done", lastValidatorStatus: null as any }); + const run = h.missionStore.startValidatorRun(feature.id, "task_completion"); + expect(h.missionStore.getFeature(feature.id)?.loopState).toBe("validating"); + h.ageRun(run.id, new Date(Date.now() - STALE_MS - 1000).toISOString()); + await h.loop.reapStaleValidatorRuns(STALE_MS); + expect(h.missionStore.getFeature(feature.id)?.loopState).toBe("validating"); + + h.pinSliceActive(slice.id); + await h.loop.recoverActiveMissions(); + + expect(h.missionStore.getFeature(feature.id)?.lastValidatorStatus).toBe("passed"); + expect(h.missionStore.computeSliceStatus(slice.id)).toBe("complete"); + expect(await h.countBoardTasks()).toBe(0); // validation/verification created no board task + expect(h.countFixFeatures(slice.id)).toBe(0); + }); + + it("inconclusive verification across recovery re-drives never deadlocks the slice at error and spawns no Fix Feature (R20/R21)", async () => { + h = await createHarness({ verificationVerdict: "inconclusive" }); + h.loop.start(); + + const { slice, feature } = h.buildFeature({ title: "Flaky" }); + h.missionStore.updateFeature(feature.id, { + status: "done", + loopState: "implementing", + lastValidatorStatus: null as any, + }); + + h.pinSliceActive(slice.id); + await h.loop.recoverActiveMissions(); + + const after = h.missionStore.getFeature(feature.id)!; + // Inconclusive routes to a terminal blocked verdict — NOT error, NOT a + // default pass — and spawns no remediation. + expect(after.lastValidatorStatus).toBe("blocked"); + expect(after.lastValidatorStatus).not.toBe("error"); + expect(h.countFixFeatures(slice.id)).toBe(0); + expect(await h.countBoardTasks()).toBe(0); + + // A subsequent sweep does not re-drive a blocked feature into churn. + const callsBefore = h.calls.length; + await h.loop.recoverActiveMissions(); + expect(h.calls.length).toBe(callsBefore); + }); +}); diff --git a/packages/engine/src/mission-execution-loop.ts b/packages/engine/src/mission-execution-loop.ts index 48773bd62f..346f1d5296 100644 --- a/packages/engine/src/mission-execution-loop.ts +++ b/packages/engine/src/mission-execution-loop.ts @@ -315,18 +315,50 @@ export class MissionExecutionLoop extends EventEmitter { } } - // Features marked "done" but stranded in "implementing" with no - // linked task can never validate on their own: the branches above - // only re-drive features that still carry a taskId. Meanwhile the - // slice-completion gate (MissionStore.computeSliceStatus) refuses - // to count an assertion-linked "done" feature until its validator - // passes — so the slice, milestone, and mission can never - // auto-progress. Re-drive validation directly so the gate can - // resolve. Validation is a read-only judge (no board task, no code - // changes); on pass the feature becomes legitimately complete, on - // fail the normal fix-feature flow takes over. + // Features marked "done" but stranded with no linked task can never + // validate on their own: the branches above only re-drive features + // that still carry a taskId. Meanwhile the slice-completion gate + // (MissionStore.computeSliceStatus) refuses to count an + // assertion-linked "done" feature until its validator passes — so + // the slice, milestone, and mission can never auto-progress. + // + // Several ways a task-less done feature lands stranded here: + // 1. loopState="implementing" + null lastValidatorStatus — the + // original stranded-orphan case (FN-5715 / the autopilot-stall + // learning): validation was never driven. + // 2. loopState="validating" + null lastValidatorStatus — a + // *reaped* run. `startValidatorRun` flips the feature to + // "validating"; `MissionStore.reapValidatorRun` resolves the + // stale run to status="error" but, by design, leaves a *done* + // feature's loopState untouched (its `shouldUpdateFeature` + // guard skips done features). So a reaped validation-only + // feature (no board task) is left "validating" forever: the + // "validating" branch above only re-drives features that carry + // a taskId, and `computeSliceStatus` never counts a "validating" + // done feature — the U7 reaper→slice deadlock (P0). + // 3. loopState="needs_fix" + lastValidatorStatus="error" — a + // reaped run on a *non-done* feature that later moved to done, + // or a reaped manual run; "error" is likewise never accepted by + // computeSliceStatus and the needs_fix branch above only + // re-drives features with a taskId. + // + // The common shape is: a task-less, done, assertion-linked feature + // that has not reached a *passed* validator status and is not + // currently being validated. Re-drive it directly regardless of the + // exact stranded loopState so it reaches a terminal verdict instead + // of livelocking on "validating"/"error". + // + // Validation is bounded (verification wall-clock is provably under + // the reaper stale window — see VALIDATOR_RUN_STALE_MAX_AGE_MS vs the + // aggregate verification timeout) and non-mutating: on pass the + // feature becomes legitimately complete; on fail the normal + // fix-feature flow takes over; on inconclusive it routes to + // needs-attention without minting remediation. Either way the + // feature reaches a terminal verdict rather than re-driving forever. if ( - feature.loopState === "implementing" + (feature.loopState === "implementing" + || feature.loopState === "validating" + || (feature.loopState === "needs_fix" && feature.lastValidatorStatus === "error")) && !feature.taskId && feature.status === "done" && feature.lastValidatorStatus !== "passed" @@ -336,6 +368,7 @@ export class MissionExecutionLoop extends EventEmitter { if ( currentFeature.loopState === "passed" || currentFeature.lastValidatorStatus === "passed" + || this.activeValidations.has(feature.id) ) { continue; }