diff --git a/.changeset/fix-branch-group-name-collision-triage.md b/.changeset/fix-branch-group-name-collision-triage.md new file mode 100644 index 0000000000..2c503d15bc --- /dev/null +++ b/.changeset/fix-branch-group-name-collision-triage.md @@ -0,0 +1,9 @@ +--- +"@runfusion/fusion": patch +--- + +Fix mission triage silently stranding features when two missions share a base branch. + +`branch_groups.branchName` is globally unique, but `ensureBranchGroupForSource` only checked for an existing group by `(sourceType, sourceId)`. When a second mission's shared-branch triage resolved to a base branch (e.g. `main`) that another mission already owned a branch group for, `createBranchGroup` threw `UNIQUE constraint failed: branch_groups.branchName`. That error escaped `triageFeature` and was swallowed by both of its callers (the validation-failure auto-triage and the startup/maintenance reconcile sweep), leaving the mission's `defined` features — including auto-generated fix features — permanently un-triaged and the mission unable to progress. + +`ensureBranchGroupForSource` now reuses an existing open group for the same branch name (matching the established `getBranchGroupByBranchName(...) ?? ensureBranchGroupForSource(...)` idiom) instead of colliding on the unique constraint. diff --git a/.changeset/fix-stranded-done-feature-recovery.md b/.changeset/fix-stranded-done-feature-recovery.md new file mode 100644 index 0000000000..222c1d437d --- /dev/null +++ b/.changeset/fix-stranded-done-feature-recovery.md @@ -0,0 +1,9 @@ +--- +"@runfusion/fusion": patch +--- + +Fix missions stalling when a feature is marked `done` but stranded mid-loop. + +A mission feature could be left `status: "done"` while its `loopState` never advanced past `"implementing"` and it had no linked board task (so it was never validated). The slice-completion gate (`MissionStore.computeSliceStatus`) correctly refuses to count an assertion-linked `done` feature until its validator passes, but nothing re-drove a task-less feature, so the slice — and the whole mission — could never auto-progress. + +Active-mission recovery now detects these stranded `done` features and re-runs assertion validation directly (no board task), so the gate can resolve: on pass the feature becomes legitimately complete, on fail the normal fix-feature flow takes over. The feature-validation path was extracted into a shared `runFeatureValidation` helper used by both task-completion and recovery. diff --git a/.changeset/vitest-autokill-guard.md b/.changeset/vitest-autokill-guard.md new file mode 100644 index 0000000000..841ae063cd --- /dev/null +++ b/.changeset/vitest-autokill-guard.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": patch +--- + +Fix the vitest memory-pressure auto-kill firing on a garbage metric and killing innocent processes. The guard probed `os.availableMemory` (which does not exist) and silently fell back to `os.freemem()`, which on macOS reads ~99% used on an idle machine — so with the toggle on, every vitest process was SIGKILLed every 30 seconds regardless of real memory pressure. It now reads `process.availableMemory()` (Node 22+) and refuses to auto-kill when only the unreliable freemem fallback is available. Kill targeting is also fixed: `pgrep -f vitest` matches full command lines (wrapper shells, monitors, editors that merely mention vitest); the TUI auto-kill/manual kill and the dashboard `POST /api/kill-vitest` + system-stats count now filter matches to actual node processes via a shared `findVitestProcessIds` helper. diff --git a/CONCEPTS.md b/CONCEPTS.md index 58705bab96..a0cd3849c4 100644 --- a/CONCEPTS.md +++ b/CONCEPTS.md @@ -15,6 +15,40 @@ The named persistence pattern for a user preference on the dashboard: a device-l ### Supported Locale A language tag in the closed set Fusion ships translations for. Any external tag (browser, environment, flag) is normalized into this set or rejected — never passed through raw. Chinese tags route by script and region so Traditional-script users are never silently served Simplified, and the two Chinese variants never collapse into a generic base tag. +## Missions + +### Relationships + +A Mission owns an ordered list of Milestones; a Milestone owns an ordered list of Slices; a Slice owns a set of Features. Status rolls **up**, not down: a Slice's status is derived from its Features, a Milestone's from its Slices, and a Mission's from its Milestones. Autopilot acts at the Slice boundary — it advances a Mission by activating the next Slice once the current one is complete. + +### Mission +A unit of autonomous, multi-step work the system plans and then drives to completion on its own, decomposed into Milestones. A Mission may run under Autopilot or be advanced manually. + +### Milestone +An ordered phase of a Mission, containing Slices and optionally depending on earlier Milestones. A Milestone is complete only when all of its Slices are complete. + +### Slice +A vertically-scoped, independently-completable chunk of a Milestone, containing Features. A Slice's status is derived from its Features and reaches *complete* only when every Feature counts as done — which, for a Feature carrying Contract Assertions, requires a passing Validator Run. + +### Feature +The smallest unit of mission work: a single deliverable evaluated against its Contract Assertions. A Feature carries both a board status (its workflow column, e.g. done) and a loop state (its execution phase); the two are distinct and can legitimately disagree mid-flight, but a done Feature that never reached a terminal loop state is an invariant violation that will stall its Slice. + +### Fix Feature +A Feature auto-generated from a failed Validator Run to carry the remediation work for the assertions that failed, linked back to the Feature it descends from. + +## Mission execution + +### Autopilot +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. + +### 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. + +### 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. ## Merge lifecycle diff --git a/docs/solutions/logic-errors/branch-group-name-collision-strands-mission-triage.md b/docs/solutions/logic-errors/branch-group-name-collision-strands-mission-triage.md new file mode 100644 index 0000000000..d970439557 --- /dev/null +++ b/docs/solutions/logic-errors/branch-group-name-collision-strands-mission-triage.md @@ -0,0 +1,83 @@ +--- +title: "Branch-group name collision silently strands mission triage" +date: 2026-06-03 +category: docs/solutions/logic-errors +module: "core/store (branch_groups) + engine mission triage" +problem_type: logic_error +component: database +symptoms: + - "Mission's defined features (incl. auto-generated fix features) are never triaged into tasks and the mission stops progressing" + - "No Fix: tasks exist and no triage audit event is emitted, despite repeated startups" + - "Engine log shows 'UNIQUE constraint failed: branch_groups.branchName' (only in stdout — never persisted)" + - "Triage works for one mission but fails for another that shares the same base branch" +root_cause: logic_error +resolution_type: code_fix +severity: high +related_components: + - "packages/core/src/store.ts (ensureBranchGroupForSource, createBranchGroup, getBranchGroupByBranchName)" + - "packages/core/src/mission-store.ts (triageFeature)" + - "packages/engine/src/mission-execution-loop.ts (handleValidationFail auto-triage)" + - "packages/engine/src/scheduler.ts (reconcileAllMissionFeatures)" +tags: + - mission-system + - branch-groups + - triage + - unique-constraint + - swallowed-error + - idempotency +--- + +# Branch-group name collision silently strands mission triage + +## Problem + +`MissionStore.triageFeature` throws `UNIQUE constraint failed: branch_groups.branchName` for a mission whose shared-branch base collides with a branch group another mission already owns. The throw is swallowed by both triage callers, so the mission's `defined` features — including auto-generated **Fix** features from failed validations — are never turned into tasks and the mission silently stops progressing. + +## Symptoms + +- A mission stops advancing; `defined`/Fix features accumulate in active slices and never become tasks. +- No `Fix:` tasks exist and no triage audit event (`mission:stranded-feature-triaged`) is emitted, even across many engine restarts. +- The only trace is in engine **stdout** (never persisted): `Error triaging fix feature …: UNIQUE constraint failed: branch_groups.branchName` and `Failed to triage stranded feature … during reconciliation: …`. +- Triage succeeds for one mission but consistently fails for another — the one whose shared base resolves to a branch name (e.g. `main`) already claimed by the first mission's branch group. + +## What Didn't Work + +- **Reasoning from code alone** suggested `triageFeature` looked robust (the branch-assignment helpers don't obviously throw), which nearly led to dismissing the triage-throw hypothesis. The error sites are also silent (logged, not persisted), so the audit/activity tables showed nothing. +- The breakthrough was **reproducing against a `VACUUM INTO` snapshot of the live mission DB**: instantiating a real `TaskStore`, pulling its `MissionStore`, and calling `triageFeature` on a stuck fix feature surfaced the exact exception and stack immediately. + +## Solution + +`ensureBranchGroupForSource` was only idempotent by `(sourceType, sourceId)`, but `branch_groups.branchName` is globally **UNIQUE**. When the source had no group yet and another source already owned a group with that branch name, `createBranchGroup` violated the unique constraint and threw. + +Reuse an existing open group for the same branch name before creating one (the idiom already used in `register-task-workflow-routes.ts`): + +```ts +// packages/core/src/store.ts — ensureBranchGroupForSource +const existing = this.getBranchGroupBySource(sourceType, sourceId); +if (existing) return existing; + +// branch_groups.branchName is globally UNIQUE — one open group per branch. +// Reuse it instead of colliding on the constraint. +const existingByBranch = this.getBranchGroupByBranchName(init.branchName); +if (existingByBranch) return existingByBranch; + +return this.createBranchGroup({ sourceType, sourceId, ...init }); +``` + +The low-level `createBranchGroup` still enforces uniqueness (unchanged). + +## Why This Works + +The mission had an empty `branchStrategy`, so `missionBranchStrategyDefaults(undefined)` returned `assignmentMode: "shared"`, and the shared base fell through to `settings.defaultBranch = "main"`. Triaging any `defined` feature then called `ensureBranchGroupForSource("mission", missionId, { branchName: "main" })`; a different mission already owned the `"main"` group, so the insert threw. The error escaped `triageFeature` into its two callers — the validation-failure auto-triage (`mission-execution-loop.ts`) and the reconcile sweep (`scheduler.ts`) — both of which catch-and-log without persisting, so features stayed `defined` forever. Reusing the existing open group removes the only failing operation; verified against the live snapshot (`triageFeature` threw before, returned `status: triaged` with a new task after). + +## Prevention + +- **An "ensure"-named helper keyed on one identity can still violate a UNIQUE constraint on a *different* column.** Make idempotency cover every uniqueness dimension the table enforces — here, both `(sourceType, sourceId)` and the unique `branchName`. +- **Swallowed errors in triage/reconcile paths cause silent stalls.** When a catch-and-continue site guards a step that work depends on (triage, validation, advancement), emit a persisted signal (audit event / mission event), not just a stdout log — otherwise the failure is invisible in the DB and impossible to diagnose post-hoc. +- **When a state machine stalls with no error, snapshot the live DB read-only (`VACUUM INTO` / `?mode=ro`) and drive the real code path against it.** Code-reading alone misled this investigation; the exact exception came from reproduction. +- Known limitation / follow-up: this reuses an *open* same-name group; a *closed/finalized* group on the same branch would still hit the UNIQUE constraint (branch-name retirement is a separate, arguably by-design concern). + +## Related Issues + +- `docs/solutions/logic-errors/mission-autopilot-stalled-by-stranded-done-feature.md` — sibling mission-stall learning (PR #1345). Same family: a mission silently wedges and an error/edge in a triage/recovery path is the cause. Both reinforce "swallowed triage-path errors → silent mission stalls." +- PR #1348 — the fix for this bug. diff --git a/docs/solutions/logic-errors/mission-autopilot-stalled-by-stranded-done-feature.md b/docs/solutions/logic-errors/mission-autopilot-stalled-by-stranded-done-feature.md new file mode 100644 index 0000000000..5238346833 --- /dev/null +++ b/docs/solutions/logic-errors/mission-autopilot-stalled-by-stranded-done-feature.md @@ -0,0 +1,124 @@ +--- +title: "Mission autopilot stalls forever on a done+implementing feature with no task" +date: 2026-06-03 +category: docs/solutions/logic-errors +module: "engine/mission-execution-loop + core/mission-store" +problem_type: logic_error +component: background_job +symptoms: + - "A mission silently stops advancing — no error, no crash, just no progress" + - "Autopilot cycles watching to activating to watching indefinitely in mission_events, never advancing the milestone" + - "A slice stays stuck active even though all of its features report status=done" + - "Wedged feature shows the contradictory combo: status=done plus loopState=implementing plus null lastValidatorStatus plus a linked assertion plus no taskId" +root_cause: missing_workflow_step +resolution_type: code_fix +severity: high +related_components: + - "packages/engine/src/mission-execution-loop.ts (recoverActiveMissions, runFeatureValidation)" + - "packages/core/src/mission-store.ts (computeSliceStatus)" +tags: + - mission-system + - autopilot + - recovery + - slice-completion + - assertion-validation + - loop-state +--- + +# Mission autopilot stalls forever on a done+implementing feature with no task + +## Problem + +A mission feature could be left `status="done"` while its `loopState` stayed `"implementing"`, with no linked board task (`taskId`) and never validated (`lastValidatorStatus` null). The slice-completion gate correctly refuses to count an unvalidated, assertion-linked `done` feature, so the slice — and therefore the milestone and the whole mission — could never auto-progress. The mission stalled silently and indefinitely. + +## Symptoms + +- A mission stops advancing entirely — no error, no crash, just no forward motion. +- Autopilot cycles `watching → activating → watching` forever in `mission_events`, never advancing the milestone. +- A slice stays `active` even though every feature in it reports `status="done"`. +- The wedged features carry the contradictory combination: `status="done"` + `loopState="implementing"` + `lastValidatorStatus=null` + at least one linked assertion + no `taskId`. + +## What Didn't Work + +The first hypothesis came from reading code alone: an early `return` in the scheduler — the `reconciliation.kind === "blocked"` branch in `handleMissionTaskMove` — looked like it could swallow the transition before the completion handler ran. Plausible on inspection, but **not** what wedged this mission. + +The real cause only surfaced by inspecting the live per-project DB read-only (`file:.../.fusion/fusion.db?mode=ro`) and looking at the actual stored feature rows. The diagnosis was then confirmed by contrast: an already-**completed** older mission also had many `done`+`implementing` features, but with **zero** assertions — so the gate let them through. That isolated the *assertion gate* as the active ingredient, not the `done`+`implementing` pairing by itself. + +Lesson: reasoning from code alone pointed at the wrong early-return; observed data found the orphan state. + +## Solution + +Two independent, individually-correct facts interlocked into a deadlock: + +1. **The slice gate is strict (by design).** `MissionStore.computeSliceStatus` (`packages/core/src/mission-store.ts:3866-3880`, added by FN-5715) refuses to count an assertion-linked `done` feature toward slice completion unless its validator passed *or* its `loopState` is idle/undefined. +2. **The recovery sweep had a gap.** `MissionExecutionLoop.recoverActiveMissions` only re-drove `implementing` features that still carried a `taskId` (`feature.loopState === "implementing" && feature.taskId`). A task-less stranded `done` feature matched none of the recovery branches (`validating` / `needs_fix` / `implementing && taskId`), so it could never be validated. + +The fix adds a recovery branch for the orphan and extracts the validation path into a shared helper. Validation is a read-only judge (no board task created, no code edited), so it is safe to run directly from the recovery sweep. + +```ts +// packages/engine/src/mission-execution-loop.ts — recoverActiveMissions, +// after the existing implementing+taskId branch +if ( + feature.loopState === "implementing" + && !feature.taskId + && feature.status === "done" + && feature.lastValidatorStatus !== "passed" + && !this.activeValidations.has(feature.id) +) { + const currentFeature = this.missionStore.getFeature(feature.id) ?? feature; + // Live re-check: skip if it has since passed (avoids racing a concurrent pass) + if ( + currentFeature.loopState === "passed" + || currentFeature.lastValidatorStatus === "passed" + ) { + continue; + } + recoveredCount++; + await this.runFeatureValidation(currentFeature); +} +``` + +The validation execution path was lifted out of `processTaskOutcome` into a reusable private method (behavior-preserving for the existing task-completion path): + +```ts +// processTaskOutcome's inline block becomes a single call: +await this.runFeatureValidation(feature); + +// shared helper used by both task-completion and recovery: +private async runFeatureValidation(feature: MissionFeature): Promise { + const assertions = this.missionStore.listAssertionsForFeature(feature.id); + if (assertions.length === 0) { + await this.handleValidationPass(feature.id, undefined, "No assertions linked"); + return; + } + this.activeValidations.add(feature.id); + try { + const run = this.missionStore.startValidatorRun(feature.id, "task_completion"); + const result = await this.runValidation(feature, assertions, run); + // dispatch pass / fail / blocked / error as before + } finally { + this.activeValidations.delete(feature.id); + } +} +``` + +Shipped in PR #1345 (commit `c2604d5`). Tests added in `packages/engine/src/__tests__/mission-execution-loop.test.ts`; full mission-execution-loop suite plus self-healing/validator-reaper suites stayed green. + +## Why This Works + +The mission stalled because the validator never ran → `lastValidatorStatus` stayed null → `computeSliceStatus` never let the slice reach `complete` → the milestone never completed → autopilot looped forever. The gate was right to block; the bug was that nothing ever *satisfied* the gate for a task-less feature. Re-driving validation gives the orphan a terminal validator status either way: on pass it becomes legitimately complete and the slice resolves; on fail the existing fix-feature flow takes over. The live `getFeature` re-check before validating avoids racing a concurrent pass. + +## Prevention + +- **Treat `loopState` as possibly-stale and possibly-contradictory with `status`.** The `done` + non-terminal-`loopState` pairing is an invariant violation worth asserting/reconciling at write time, not just tolerating downstream. Any logic that *gates* on `loopState` inherits this fragility. +- **Recovery/self-healing sweeps keyed on `taskId` must handle the task-less orphan.** Conditions like `loopState === "implementing" && feature.taskId` silently skip any feature missing the key. Enumerate the orphan states explicitly. +- **When two individually-correct rules can interlock into a deadlock** (a strict gate + an incomplete recovery sweep), add an explicit reconciliation path rather than weakening the gate. +- **Diagnostic tip:** when a state machine stalls with no error, inspect the live DB read-only (`?mode=ro`) and read the actual stored values; contrast a wedged instance against a healthy/completed one to isolate the active ingredient. Code-reading alone misdirected this investigation. + +## Related Issues + +- `docs/missions-completion-contract.md` — the canonical FN-5715 completion-gate contract. It already covers (a) zero-assertion features going to `loopState="passed"` and (b) `taskId == null` features being re-triaged, but does **not** yet cover this specific orphan: `done` + `implementing` + no `taskId` + never validated. This learning extends that contract; the invariant belongs folded into its "Slice Status / Autopilot Advance" and "Validator/loop behavior" sections. +- `docs/missions.md:297` — documents stranded-feature (`taskId == null`) reconciliation and the `mission:stranded-feature-triaged` audit event. +- FN-5721 (#1183) — "Implement mission completion gate contract" (FN-5715 enforcement baseline); closest companion issue. +- FN-5901 — "reap stale mission validator runs": the sibling self-healing pattern for stale *validator* runs. This fix is the analogous self-heal for stranded *implementing* features. (session history) +- FN-5902 (in flight as of 2026-06-02) — "make ALL mission validation AI-run; eliminate zero-assertion auto-pass". Touches the same validation pipeline (`mission-execution-loop.ts` auto-pass branch); changing zero-assertion behavior interacts with this gate. (session history) diff --git a/packages/cli/src/commands/dashboard-tui/__tests__/available-memory.test.ts b/packages/cli/src/commands/dashboard-tui/__tests__/available-memory.test.ts new file mode 100644 index 0000000000..1dd934f926 --- /dev/null +++ b/packages/cli/src/commands/dashboard-tui/__tests__/available-memory.test.ts @@ -0,0 +1,54 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import os from "node:os"; +import { getAvailableMemoryInfo } from "../controller.js"; + +type ProcessWithAvailableMemory = NodeJS.Process & { availableMemory?: () => number }; + +describe("getAvailableMemoryInfo", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("reports a reliable reading from process.availableMemory when present", () => { + const proc = process as ProcessWithAvailableMemory; + if (typeof proc.availableMemory !== "function") { + // Older runtime without the API — covered by the fallback test below. + return; + } + const spy = vi.spyOn(proc, "availableMemory").mockReturnValue(123_456_789); + + expect(getAvailableMemoryInfo()).toEqual({ bytes: 123_456_789, reliable: true }); + expect(spy).toHaveBeenCalled(); + }); + + it("falls back to os.freemem and flags the reading unreliable when the API is missing", () => { + const proc = process as ProcessWithAvailableMemory; + const original = proc.availableMemory; + // Simulate a runtime without process.availableMemory (Node < 22). The + // freemem fallback must be flagged unreliable: on macOS freemem reads + // ~99% used on an idle machine, and treating it as a pressure signal made + // the vitest auto-kill fire every 30s (2026-06-03 incident). + Reflect.deleteProperty(proc, "availableMemory"); + const freememSpy = vi.spyOn(os, "freemem").mockReturnValue(42); + try { + expect(getAvailableMemoryInfo()).toEqual({ bytes: 42, reliable: false }); + } finally { + if (original) proc.availableMemory = original; + freememSpy.mockRestore(); + } + }); + + it("falls back unreliable when process.availableMemory throws", () => { + const proc = process as ProcessWithAvailableMemory; + if (typeof proc.availableMemory !== "function") return; + vi.spyOn(proc, "availableMemory").mockImplementation(() => { + throw new Error("not supported"); + }); + const freememSpy = vi.spyOn(os, "freemem").mockReturnValue(7); + try { + expect(getAvailableMemoryInfo()).toEqual({ bytes: 7, reliable: false }); + } finally { + freememSpy.mockRestore(); + } + }); +}); diff --git a/packages/cli/src/commands/dashboard-tui/controller.ts b/packages/cli/src/commands/dashboard-tui/controller.ts index 1b0460bc14..a84ce30e5c 100644 --- a/packages/cli/src/commands/dashboard-tui/controller.ts +++ b/packages/cli/src/commands/dashboard-tui/controller.ts @@ -1,24 +1,39 @@ import os from "node:os"; import v8 from "node:v8"; -import { execFile } from "node:child_process"; import { appendFileSync } from "node:fs"; +import { findVitestProcessIds } from "@fusion/core"; // `os.freemem()` on macOS only counts truly-free pages and excludes the large // "inactive"/cached pool that the OS will reclaim on demand — so total-free -// reads ~95%+ used on an otherwise-idle machine. `os.availableMemory()` (Node -// 22+) reports memory the OS considers available, matching Activity Monitor's -// notion of "used". Fall back to freemem on older runtimes. -function getAvailableMemory(): number { - const fn = (os as unknown as { availableMemory?: () => number }).availableMemory; - if (typeof fn === "function") { +// reads ~95%+ used on an otherwise-idle machine. `process.availableMemory()` +// (Node 22+ — NOT `os.availableMemory`, which does not exist and silently +// fell through to the freemem trap this function was written to avoid) +// reports memory the OS considers available, matching Activity Monitor's +// notion of "used". The freemem fallback is flagged unreliable so pressure- +// triggered actions can refuse to fire on a garbage ratio: with freemem, an +// idle 256GB Mac reads ~99% used and the vitest auto-kill fired every 30s +// regardless of real pressure (2026-06-03 incident). +interface AvailableMemoryReading { + bytes: number; + /** False when only `os.freemem()` was available — unusable as a pressure signal. */ + reliable: boolean; +} + +export function getAvailableMemoryInfo(): AvailableMemoryReading { + const processFn = (process as unknown as { availableMemory?: () => number }).availableMemory; + if (typeof processFn === "function") { try { - const v = fn.call(os); - if (Number.isFinite(v) && v >= 0) return v; + const v = processFn.call(process); + if (Number.isFinite(v) && v >= 0) return { bytes: v, reliable: true }; } catch { // fall through } } - return os.freemem(); + return { bytes: os.freemem(), reliable: false }; +} + +function getAvailableMemory(): number { + return getAvailableMemoryInfo().bytes; } const TUI_DEBUG_LOG = process.env.FUSION_TUI_DEBUG_LOG; @@ -302,8 +317,10 @@ export class DashboardTUI { if (this.autoKillVitestOnPressure) { const total = os.totalmem(); - const free = getAvailableMemory(); - if (total > 0) { + const { bytes: free, reliable } = getAvailableMemoryInfo(); + // Without a reliable availability reading the ratio is garbage (freemem + // on macOS ≈ always >90% used) — never SIGKILL on a garbage signal. + if (total > 0 && reliable) { const usedRatio = (total - free) / total; // 30s minimum gap between auto-kills — vitest restart and OS reclaim // both take a few seconds; firing every 2s would flap. @@ -328,24 +345,13 @@ export class DashboardTUI { * gone by the time we send the signal). */ async killVitestProcesses(): Promise<{ killed: number; pids: number[] }> { - // pgrep is POSIX-only; Windows path is a no-op above. - if (process.platform === "win32") { - return { killed: 0, pids: [] }; - } - const selfPid = process.pid; - // execFile (not execSync) so the TUI render loop stays responsive while - // pgrep walks the process table — that walk can take 100ms+ on a busy - // machine and previously froze the UI on every memory-pressure check. - const stdout: string = await new Promise((resolve) => { - execFile("pgrep", ["-f", "vitest"], { encoding: "utf8" }, (err, out) => { - // pgrep exits non-zero when no matches — treat as empty result. - resolve(err ? "" : (typeof out === "string" ? out : "")); - }); - }); - const pids = stdout - .split("\n") - .map((s) => Number.parseInt(s.trim(), 10)) - .filter((n) => Number.isFinite(n) && n > 0 && n !== selfPid); + // findVitestProcessIds is pgrep-based (POSIX-only; no-op on Windows) and + // uses async execFile so the TUI render loop stays responsive while the + // process table is walked. Crucially it filters matches to actual node + // processes: a bare `pgrep -f vitest` also matches wrapper shells whose + // command line mentions vitest, monitors, and editors — SIGKILLing those + // took out unrelated process trees (2026-06-03 incident). + const pids = await findVitestProcessIds(); let killed = 0; for (const pid of pids) { diff --git a/packages/core/src/__tests__/branch-group-store.test.ts b/packages/core/src/__tests__/branch-group-store.test.ts index ae26efd00d..17d641af37 100644 --- a/packages/core/src/__tests__/branch-group-store.test.ts +++ b/packages/core/src/__tests__/branch-group-store.test.ts @@ -67,6 +67,34 @@ describe("TaskStore branch groups", () => { expect(second.autoMerge).toBe(true); }); + it("reuses an existing open group with the same branchName across sources instead of throwing", () => { + // Regression: branch_groups.branchName is globally UNIQUE. When one mission + // already owns an open group for a shared base branch, a second source whose + // triage resolves to the same branch must reuse that group rather than crash + // on the UNIQUE constraint. (Mission triage discards the result and only needs + // it not to throw; a thrown error there silently strands "defined" features.) + const owner = store.createBranchGroup({ sourceType: "mission", sourceId: "M-OWNER", branchName: "main" }); + + let reusedByMission!: ReturnType; + expect(() => { + reusedByMission = store.ensureBranchGroupForSource("mission", "M-OTHER", { + branchName: "main", + autoMerge: true, + }); + }).not.toThrow(); + expect(reusedByMission.id).toBe(owner.id); + + // Invariant holds across the other source types that share this helper. + const reusedByNewTask = store.ensureBranchGroupForSource("new-task", "shared/main", { branchName: "main" }); + expect(reusedByNewTask.id).toBe(owner.id); + + const reusedByPlanning = store.ensureBranchGroupForSource("planning", "PS-main", { branchName: "main" }); + expect(reusedByPlanning.id).toBe(owner.id); + + // No duplicate rows were created for the shared branch. + expect(store.listBranchGroups().filter((g) => g.branchName === "main")).toHaveLength(1); + }); + it("supports new-task branch group sources and round-trips through lookups", () => { const group = store.ensureBranchGroupForSource("new-task", "shared/onboarding", { branchName: "shared/onboarding", diff --git a/packages/core/src/__tests__/vitest-processes.test.ts b/packages/core/src/__tests__/vitest-processes.test.ts new file mode 100644 index 0000000000..f3e92f59ca --- /dev/null +++ b/packages/core/src/__tests__/vitest-processes.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, it, vi } from "vitest"; +import type { execFile as nodeExecFile } from "node:child_process"; +import { findVitestProcessIds } from "../vitest-processes.js"; + +type ExecFileCallback = (err: Error | null, stdout: string, stderr: string) => void; + +function makeExecFileMock(responses: { pgrep?: string; ps?: string; pgrepError?: boolean }) { + const calls: Array<{ cmd: string; args: string[] }> = []; + const impl = ((cmd: string, args: string[], _opts: unknown, cb: ExecFileCallback) => { + calls.push({ cmd, args }); + if (cmd === "pgrep") { + if (responses.pgrepError) { + cb(new Error("pgrep: no matches"), "", ""); + } else { + cb(null, responses.pgrep ?? "", ""); + } + return {} as never; + } + if (cmd === "ps") { + cb(null, responses.ps ?? "", ""); + return {} as never; + } + cb(new Error(`unexpected command ${cmd}`), "", ""); + return {} as never; + }) as unknown as typeof nodeExecFile; + return { impl, calls }; +} + +describe("findVitestProcessIds", () => { + it("returns only pids whose executable is node — wrapper shells and monitors are spared", async () => { + const { impl, calls } = makeExecFileMock({ + // pgrep -f vitest matches the runner, two workers, a zsh wrapper whose + // command line contains "npx vitest run", and a watch loop grepping for + // "node (vitest". + pgrep: "101\n102\n103\n104\n105\n", + ps: [ + " 101 /opt/homebrew/bin/node", + " 102 node", + " 103 /usr/local/bin/node", + " 104 zsh", + " 105 /bin/zsh", + ].join("\n"), + }); + + const pids = await findVitestProcessIds({ execFileImpl: impl }); + + expect(pids).toEqual([101, 102, 103]); + expect(calls[0]).toEqual({ cmd: "pgrep", args: ["-f", "vitest"] }); + expect(calls[1]?.cmd).toBe("ps"); + expect(calls[1]?.args).toEqual(["-o", "pid=,comm=", "-p", "101,102,103,104,105"]); + }); + + it("always excludes the calling process and any caller-supplied pids", async () => { + const self = process.pid; + const { impl } = makeExecFileMock({ + pgrep: `${self}\n201\n202\n`, + ps: [` ${self} node`, " 201 node", " 202 node"].join("\n"), + }); + + const pids = await findVitestProcessIds({ execFileImpl: impl, excludePids: [202] }); + + expect(pids).toEqual([201]); + }); + + it("returns empty when pgrep finds nothing (non-zero exit)", async () => { + const { impl, calls } = makeExecFileMock({ pgrepError: true }); + + const pids = await findVitestProcessIds({ execFileImpl: impl }); + + expect(pids).toEqual([]); + // ps must not run with an empty pid list. + expect(calls.map((c) => c.cmd)).toEqual(["pgrep"]); + }); + + it("returns empty on win32 without spawning anything", async () => { + const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + try { + const { impl, calls } = makeExecFileMock({ pgrep: "999\n", ps: " 999 node" }); + const pids = await findVitestProcessIds({ execFileImpl: impl }); + expect(pids).toEqual([]); + expect(calls).toEqual([]); + } finally { + platformSpy.mockRestore(); + } + }); +}); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 32b3516cc5..a8199b9b4b 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -326,6 +326,10 @@ export { type MergeTargetResolution, type MergeTargetResolverOptions, } from "./task-merge.js"; +export { + findVitestProcessIds, + type FindVitestProcessIdsOptions, +} from "./vitest-processes.js"; export { countRecentIdenticalStallEntries, getInReviewStallReason, diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index c948c29a63..b4bab176d2 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -4434,6 +4434,17 @@ export class TaskStore extends EventEmitter { return existing; } + // `branch_groups.branchName` is globally UNIQUE — a branch is represented by + // exactly one open group. If another source already owns an open group for + // this branch, reuse it rather than calling createBranchGroup and violating + // the UNIQUE constraint. Without this, two missions whose shared base resolves + // to the same branch (e.g. "main") collide: the throw escapes triageFeature + // and is swallowed by its callers, silently stranding "defined" features. + const existingByBranch = this.getBranchGroupByBranchName(init.branchName); + if (existingByBranch) { + return existingByBranch; + } + return this.createBranchGroup({ sourceType, sourceId, diff --git a/packages/core/src/vitest-processes.ts b/packages/core/src/vitest-processes.ts new file mode 100644 index 0000000000..a2f5e01802 --- /dev/null +++ b/packages/core/src/vitest-processes.ts @@ -0,0 +1,87 @@ +import { execFile as nodeExecFile } from "node:child_process"; + +/** + * Locate running vitest processes safely. + * + * `pgrep -f vitest` matches FULL command lines, so a bare pattern also matches + * innocent bystanders whose argv merely mentions vitest: + * - wrapper shells (`zsh -c '... npx vitest run ...'`) — killing these + * strands the `$?` handler so failures look like silent truncation, + * - monitoring/grep one-liners that mention vitest, + * - editors or tools opened on `vitest.config.ts`. + * Root cause of the 2026-06-03 incident where the memory-pressure auto-kill + * SIGKILLed unrelated process trees every 30s. + * + * This helper filters pgrep candidates to processes whose executable (`comm`) + * is actually node, so only the vitest runner and its workers are reported. + */ + +export interface FindVitestProcessIdsOptions { + /** PIDs to exclude in addition to the calling process. */ + excludePids?: number[]; + /** Test seam — injected execFile. */ + execFileImpl?: typeof nodeExecFile; +} + +function execToStdout( + execFileImpl: typeof nodeExecFile, + cmd: string, + args: string[], +): Promise { + return new Promise((resolve) => { + execFileImpl(cmd, args, { encoding: "utf8" }, (err, out) => { + // pgrep/ps exit non-zero when nothing matches — treat as empty result. + resolve(err ? "" : (typeof out === "string" ? out : "")); + }); + }); +} + +function parsePids(stdout: string): number[] { + return stdout + .split(/\r?\n/) + .map((line) => Number.parseInt(line.trim(), 10)) + .filter((pid) => Number.isFinite(pid) && pid > 0); +} + +/** Keep only pids whose executable is node (vitest runner + pool workers). */ +async function filterToNodeProcesses( + execFileImpl: typeof nodeExecFile, + pids: number[], +): Promise { + if (pids.length === 0) return []; + const stdout = await execToStdout(execFileImpl, "ps", [ + "-o", + "pid=,comm=", + "-p", + pids.join(","), + ]); + const nodePids: number[] = []; + for (const line of stdout.split(/\r?\n/)) { + const trimmed = line.trim(); + if (!trimmed) continue; + const spaceIdx = trimmed.indexOf(" "); + if (spaceIdx <= 0) continue; + const pid = Number.parseInt(trimmed.slice(0, spaceIdx), 10); + if (!Number.isFinite(pid) || pid <= 0) continue; + const comm = trimmed.slice(spaceIdx + 1).trim(); + const executable = comm.split("/").pop() ?? comm; + if (executable === "node" || executable === "node.exe" || executable === "nodejs") { + nodePids.push(pid); + } + } + return nodePids; +} + +export async function findVitestProcessIds( + options: FindVitestProcessIdsOptions = {}, +): Promise { + // pgrep/ps are POSIX-only; Windows callers treat this as a no-op. + if (process.platform === "win32") return []; + + const execFileImpl = options.execFileImpl ?? nodeExecFile; + const excluded = new Set([process.pid, ...(options.excludePids ?? [])]); + + const candidates = parsePids(await execToStdout(execFileImpl, "pgrep", ["-f", "vitest"])); + const nodePids = await filterToNodeProcesses(execFileImpl, candidates); + return nodePids.filter((pid) => !excluded.has(pid)); +} diff --git a/packages/dashboard/src/__tests__/routes-system.test.ts b/packages/dashboard/src/__tests__/routes-system.test.ts index 7babdd990b..512181ad76 100644 --- a/packages/dashboard/src/__tests__/routes-system.test.ts +++ b/packages/dashboard/src/__tests__/routes-system.test.ts @@ -406,7 +406,13 @@ describe("GET /api/system-stats", () => { }); mockExecFile.mockImplementation((...callArgs: unknown[]) => { + const [file] = callArgs as [string]; const cb = callArgs[callArgs.length - 1] as (err: unknown, stdout?: string, stderr?: string) => void; + if (file === "ps") { + // comm filter pass: all candidates are real node processes. + cb(null, ` ${process.pid} node\n 111 /opt/homebrew/bin/node\n 222 node\n`, ""); + return; + } cb(null, `${process.pid}\n111\n222\n`, ""); }); @@ -584,10 +590,18 @@ describe("POST /api/kill-vitest", () => { it("kills all matched vitest pids except the current dashboard process", async () => { const store = createMockStore(); - mockExecFile.mockImplementationOnce((...callArgs: unknown[]) => { - const cb = callArgs[callArgs.length - 1] as (err: unknown, stdout?: string, stderr?: string) => void; - cb(null, `${process.pid}\n1001\n1002\nnot-a-pid\n`, ""); - }); + mockExecFile + .mockImplementationOnce((...callArgs: unknown[]) => { + // pgrep -f vitest: matches the dashboard itself, two node processes, + // a wrapper shell whose command line mentions vitest, and garbage. + const cb = callArgs[callArgs.length - 1] as (err: unknown, stdout?: string, stderr?: string) => void; + cb(null, `${process.pid}\n1001\n1002\n1003\nnot-a-pid\n`, ""); + }) + .mockImplementationOnce((...callArgs: unknown[]) => { + // ps comm filter: 1003 is a zsh wrapper and must be spared. + const cb = callArgs[callArgs.length - 1] as (err: unknown, stdout?: string, stderr?: string) => void; + cb(null, ` ${process.pid} node\n 1001 /opt/homebrew/bin/node\n 1002 node\n 1003 zsh\n`, ""); + }); const killSpy = vi.spyOn(process, "kill").mockImplementation(() => true); const res = await REQUEST(buildApp(store), "POST", "/api/kill-vitest"); diff --git a/packages/dashboard/src/routes.ts b/packages/dashboard/src/routes.ts index e1bcde6aaa..8289fb18db 100644 --- a/packages/dashboard/src/routes.ts +++ b/packages/dashboard/src/routes.ts @@ -22,6 +22,7 @@ import { MemoryBackendError, RoutineStore, discoverPiExtensions, + findVitestProcessIds, getFusionAgentDir, getLegacyPiAgentDir, isWebhookTrigger, @@ -1496,22 +1497,13 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout }; const getVitestProcessIds = async (): Promise => { - // execFile (not execSync) so the dashboard's event loop stays responsive - // while pgrep walks the process table — that walk can take 100ms+ on a - // busy machine and previously froze every concurrent request. - const { execFile } = await import("node:child_process"); - - const stdout: string = await new Promise((resolve) => { - execFile("pgrep", ["-f", "vitest"], { encoding: "utf8" }, (err, out) => { - // pgrep exits non-zero when no matches — treat as empty result. - resolve(err ? "" : (typeof out === "string" ? out : "")); - }); - }); - - return stdout - .split(/\r?\n/) - .map((line) => Number.parseInt(line.trim(), 10)) - .filter((pid) => Number.isInteger(pid) && pid > 0 && pid !== process.pid); + // Async pgrep/ps via findVitestProcessIds so the dashboard's event loop + // stays responsive while the process table is walked. The helper filters + // matches to actual node processes — a bare `pgrep -f vitest` also matches + // wrapper shells, monitors, and editors whose command line merely mentions + // vitest, and SIGKILLing those took out unrelated process trees + // (2026-06-03 incident). + return findVitestProcessIds(); }; /** diff --git a/packages/engine/src/__tests__/mission-execution-loop.test.ts b/packages/engine/src/__tests__/mission-execution-loop.test.ts index bd09d6e125..d1bb8d7dc2 100644 --- a/packages/engine/src/__tests__/mission-execution-loop.test.ts +++ b/packages/engine/src/__tests__/mission-execution-loop.test.ts @@ -556,6 +556,87 @@ describe("MissionExecutionLoop", () => { }); }); + describe("recoverActiveMissions stranded done features", () => { + function wireHierarchy(slice: Slice, features: MissionFeature[]) { + missionStore.getMissionWithHierarchy = vi.fn((id: string) => { + const mission = missionStore.getMission(id); + if (!mission) return undefined; + return { + ...mission, + milestones: [ + { + ...createMockMilestone({ missionId: id }), + slices: [{ ...slice, features }], + }, + ], + }; + }) as any; + } + + it("re-validates a done feature stranded in 'implementing' with no linked task", async () => { + // Regression: a feature marked "done" whose loopState never left + // "implementing" (and which was never validated and has no board task) + // can never validate on its own — the prior recovery loop only re-drove + // implementing features that still had a taskId. The slice-completion + // gate then refuses to count it, wedging the whole mission. Recovery + // must re-drive validation so the slice can eventually complete. + const mission = createMockMission({ id: "M-STRAND", status: "active" }); + missionStore._setMission(mission); + + const slice = createMockSlice({ id: "SL-STRAND", milestoneId: "MS-001", status: "active" }); + const orphan = createMockFeature({ + id: "F-STRAND", + sliceId: "SL-STRAND", + status: "done", + loopState: "implementing", + lastValidatorStatus: undefined, + taskId: undefined, + }); + (missionStore as any)._addFeatureWithManagedAssertion(orphan); + wireHierarchy(slice, [missionStore.getFeature("F-STRAND") as MissionFeature]); + + loop = new MissionExecutionLoop({ + taskStore: taskStore as any, + missionStore: missionStore as any, + rootDir: "/tmp", + }); + loop.start(); + + const result = await loop.recoverActiveMissions(); + + expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-STRAND", "task_completion"); + expect(result.recoveredCount).toBeGreaterThanOrEqual(1); + }); + + it("leaves an already-validated done feature untouched", async () => { + const mission = createMockMission({ id: "M-OK", status: "active" }); + missionStore._setMission(mission); + + const slice = createMockSlice({ id: "SL-OK", milestoneId: "MS-001", status: "active" }); + const validated = createMockFeature({ + id: "F-OK", + sliceId: "SL-OK", + status: "done", + loopState: "passed", + lastValidatorStatus: "passed", + taskId: undefined, + }); + (missionStore as any)._addFeatureWithManagedAssertion(validated); + wireHierarchy(slice, [missionStore.getFeature("F-OK") as MissionFeature]); + + loop = new MissionExecutionLoop({ + taskStore: taskStore as any, + missionStore: missionStore as any, + rootDir: "/tmp", + }); + loop.start(); + + await loop.recoverActiveMissions(); + + expect(missionStore.startValidatorRun).not.toHaveBeenCalled(); + }); + }); + describe("reapStaleValidatorRuns", () => { it("reaps stale runs across trigger types and records audit metadata", async () => { vi.useFakeTimers(); diff --git a/packages/engine/src/mission-execution-loop.ts b/packages/engine/src/mission-execution-loop.ts index c321662830..438433992b 100644 --- a/packages/engine/src/mission-execution-loop.ts +++ b/packages/engine/src/mission-execution-loop.ts @@ -293,6 +293,42 @@ export class MissionExecutionLoop extends EventEmitter { loopLog.error(`Recovery failed for implementing feature ${feature.id}:`, err); } } + + // 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. + if ( + feature.loopState === "implementing" + && !feature.taskId + && feature.status === "done" + && feature.lastValidatorStatus !== "passed" + && !this.activeValidations.has(feature.id) + ) { + const currentFeature = this.missionStore.getFeature(feature.id) ?? feature; + if ( + currentFeature.loopState === "passed" + || currentFeature.lastValidatorStatus === "passed" + ) { + continue; + } + try { + loopLog.warn( + `Recovery: re-validating stranded "done" feature ${feature.id} ` + + `(loopState=${feature.loopState}, no linked task) so its slice can complete`, + ); + recoveredCount++; + await this.runFeatureValidation(currentFeature); + } catch (err) { + loopLog.error(`Recovery failed for stranded done feature ${feature.id}:`, err); + } + } } } } @@ -354,46 +390,59 @@ export class MissionExecutionLoop extends EventEmitter { return; } - // Lazily guarantee a linked assertion before validation so every feature - // is evaluated by the validator even when legacy data is missing links. - let assertions = this.missionStore.listAssertionsForFeature(feature.id); - if (assertions.length === 0) { - loopLog.log(`Feature ${feature.id} has no linked assertions; lazily ensuring store-managed assertion linkage`); - assertions = this.missionStore.ensureFeatureAssertionLinked(feature.id); - } - - // Mark feature as being validated - this.activeValidations.add(feature.id); - - try { - loopLog.log(`Running internal validation for feature ${feature.id} — no board task created (policy: docs/missions.md)`); - - // Start the validator run (no board task per docs/missions.md) - const run = this.missionStore.startValidatorRun(feature.id, "task_completion"); - loopLog.log(`Started validator run ${run.id} for feature ${feature.id}`); - - // Run the validation - const result = await this.runValidation(feature, assertions, run); - - // Handle the result - if (result.status === "pass") { - await this.handleValidationPass(feature.id, run.id, result.summary); - } else if (result.status === "fail") { - await this.handleValidationFail(feature.id, run.id, result); - } else if (result.status === "blocked") { - await this.handleValidationBlocked(feature.id, run.id, result.blockedReason); - } else if (result.status === "error") { - await this.handleValidationError(feature.id, run.id, result.summary); - } - } finally { - this.activeValidations.delete(feature.id); - } + await this.runFeatureValidation(feature); } catch (err) { loopLog.error(`Error processing task outcome for ${taskId}:`, err); // Don't crash the loop - log and continue } } + /** + * Run assertion validation for a feature and apply the outcome. + * + * Shared by processTaskOutcome (task-triggered) and recoverActiveMissions + * (self-healing for features stranded mid-loop with no board task). Callers + * are responsible for confirming the feature is eligible to validate; this + * method handles lazy assertion linkage, validator run bookkeeping, and + * dispatch of the validation result. + */ + private async runFeatureValidation(feature: MissionFeature): Promise { + // Lazily guarantee a linked assertion before validation so every feature + // is evaluated by the validator even when legacy data is missing links. + let assertions = this.missionStore.listAssertionsForFeature(feature.id); + if (assertions.length === 0) { + loopLog.log(`Feature ${feature.id} has no linked assertions; lazily ensuring store-managed assertion linkage`); + assertions = this.missionStore.ensureFeatureAssertionLinked(feature.id); + } + + // Mark feature as being validated + this.activeValidations.add(feature.id); + + try { + loopLog.log(`Running internal validation for feature ${feature.id} — no board task created (policy: docs/missions.md)`); + + // Start the validator run (no board task per docs/missions.md) + const run = this.missionStore.startValidatorRun(feature.id, "task_completion"); + loopLog.log(`Started validator run ${run.id} for feature ${feature.id}`); + + // Run the validation + const result = await this.runValidation(feature, assertions, run); + + // Handle the result + if (result.status === "pass") { + await this.handleValidationPass(feature.id, run.id, result.summary); + } else if (result.status === "fail") { + await this.handleValidationFail(feature.id, run.id, result); + } else if (result.status === "blocked") { + await this.handleValidationBlocked(feature.id, run.id, result.blockedReason); + } else if (result.status === "error") { + await this.handleValidationError(feature.id, run.id, result.summary); + } + } finally { + this.activeValidations.delete(feature.id); + } + } + /** * Run the validation AI session for a feature. *