test(scrutiny): fix missing MissionExecutionLoop mock in serve.test.ts

Add MissionExecutionLoop mock to @fusion/engine vi.mock block in
serve.test.ts. The mock provides start, stop, processTaskOutcome,
and recoverActiveMissions methods to match the actual class interface.

Also adds the scrutiny synthesis report for milestone execution-loop
which identifies 3 blocking issues in FEAT-004:
- parseValidationResult stub always returns pass
- notifyValidationComplete passes featureId instead of taskId
- recoverActiveMissions doesn't perform state transitions

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
This commit is contained in:
gsxdsm
2026-04-11 18:37:49 -07:00
parent bc3688badf
commit 89abece8cb
3 changed files with 153 additions and 0 deletions

View File

@@ -0,0 +1,62 @@
{
"featureId": "FEAT-004",
"reviewedAt": "2026-04-11T18:30:00.000Z",
"commitId": "46fff0d3",
"transcriptSkeletonReviewed": true,
"diffReviewed": true,
"status": "fail",
"codeReview": {
"summary": "MissionExecutionLoop class is wired into dashboard.ts, serve.ts, InProcessRuntime, and scheduler. However, there are three blocking functional issues: (1) parseValidationResult is a stub that always returns status='pass', so validation failure/blocked handling is never exercised; (2) notifyValidationComplete passes featureId to handleTaskCompletion(taskId) which expects a taskId, causing MissionAutopilot to receive wrong input; (3) recoverActiveMissions only logs recovery intent but does not actually transition validating features back to implementing state, leaving them stuck.",
"issues": [
{
"file": "packages/engine/src/mission-execution-loop.ts",
"line": 317,
"severity": "blocking",
"description": "parseValidationResult() is a stub that always returns { status: 'pass', assertions: all passed }. The comment explicitly says 'For now, return a default pass result since we don't have the actual parsing logic implemented'. This means validation failures are never detected, handleValidationFail is never called, and fix features are never generated. The VAL-EL-006 (validation failure creates fix feature) and VAL-EL-007 (validation blocked) outcomes are never exercised in practice."
},
{
"file": "packages/engine/src/mission-execution-loop.ts",
"line": 480,
"severity": "blocking",
"description": "notifyValidationComplete callback passes featureId to missionAutopilot.handleTaskCompletion(featureId), but handleTaskCompletion expects a taskId parameter. Inside MissionAutopilot.handleTaskCompletion, it calls getFeatureByTaskId(taskId) which will receive a featureId and return null/undefined, causing the function to early-return without doing anything. This breaks the autopilot coordination for loop states (VAL-EL-012)."
},
{
"file": "packages/engine/src/mission-execution-loop.ts",
"line": 134,
"severity": "blocking",
"description": "recoverActiveMissions() only logs the features it finds in 'validating' or 'needs_fix' states but never calls any state transition or re-triggers validation. Features in 'validating' state remain stuck in that state with no path to recovery unless a new task completion event fires. This violates VAL-EL-009 which requires recovery to 're-enqueue pending validations'."
},
{
"file": "packages/engine/src/mission-execution-loop.ts",
"line": 492,
"severity": "non_blocking",
"description": "When parseValidationResult returns 'fail' (which never happens due to the stub), createGeneratedFixFeature is called with runId='unknown' fallback if runId is falsy. The store's createGeneratedFixFeature validates that runId exists and throws if not found. This would cause an unhandled error in the catch block of handleValidationFail, though this path is never reached due to the stub."
},
{
"file": "packages/engine/src/mission-execution-loop.ts",
"line": 1,
"severity": "non_blocking",
"description": "MissionExecutionLoop extends EventEmitter and emits events (validation:passed, validation:failed, etc.) but there are no test files referencing these events. AGENTS.md specifies 'write tests BEFORE implementation (TDD)' but handoff shows tests.added: []. The referenced test file mission-execution-loop.test.ts does not exist."
}
]
},
"sharedStateObservations": [
{
"area": "conventions",
"observation": "AGENTS.md specifies TDD with tests written before implementation and references packages/engine/src/mission-execution-loop.test.ts as a key file to create. However, no test file was created and tests.added is empty in the handoff. The validation contract for VAL-EL-005 through VAL-EL-008 requires testing validation pass/fail/blocked flows, but there are no unit tests for any of these outcomes.",
"evidence": "handoff.tests.added: []; AGENTS.md Testing section says 'write tests BEFORE implementation (TDD)'; validation-contract.md VAL-EL-* items list 'vitest' as the Tool but no test file exists at packages/engine/src/mission-execution-loop.test.ts"
},
{
"area": "skills",
"observation": "The backend-worker skill procedure was followed according to skillFeedback.followedProcedure: true. However, the worker did not implement the AI response parsing logic (parseValidationResult stub) and did not wire the notifyValidationComplete callback correctly to MissionAutopilot. The backend-worker skill does not document how to extract structured JSON from AI agent sessions, which may have been a knowledge gap.",
"evidence": "parseValidationResult() always returns pass with a comment 'For now, return a default pass result since we don't have the actual parsing logic implemented'; transcript shows worker created the file but did not iterate on the parsing logic"
},
{
"area": "knowledge",
"observation": "The worker did not implement actual AI response parsing for the validator output. The extractResponseText() and parseValidatorResponse() methods that appear in the later version of the file (fn-1587 worktree) were not part of the committed implementation. The parsing logic is non-trivial - it requires extracting JSON from potentially multi-part AI responses. This knowledge is not documented in .factory/library/",
"evidence": "parseValidationResult at line 290-307 returns hardcoded pass result with comment acknowledging it's unimplemented; the worktree version (fusion/fn-1587) shows extractResponseText and parseValidatorResponse methods that handle JSON extraction from session state"
}
],
"addressesFailureFrom": null,
"summary": "FEAT-004 wiring is structurally complete (dashboard.ts, serve.ts, InProcessRuntime, scheduler integration all present) but has three blocking functional issues: (1) validation result parsing is a stub so failures/blocked never trigger fix feature generation; (2) notifyValidationComplete passes featureId to handleTaskCompletion(taskId) which expects taskId, breaking autopilot coordination; (3) recoverActiveMissions doesn't actually transition validating features. Additionally, no unit tests were written despite AGENTS.md requiring TDD. The implementation would not satisfy VAL-EL-005 through VAL-EL-009, VAL-EL-012 in an actual integration test."
}

View File

@@ -0,0 +1,75 @@
{
"milestone": "execution-loop",
"round": 1,
"status": "fail",
"validatorsRun": {
"test": {
"passed": false,
"command": "pnpm test",
"exitCode": 1,
"note": "Test suite failed due to missing MissionExecutionLoop mock in serve.test.ts. Fix applied to add the mock, which resolves the immediate test failure. However, the broader test suite has pre-existing failures in merger.test.ts (buffer handling tests) that are unrelated to execution-loop feature."
},
"typecheck": {
"passed": true,
"command": "pnpm build",
"exitCode": 0
},
"lint": {
"passed": false,
"command": "pnpm lint",
"exitCode": 1,
"note": "Lint has 4217 pre-existing errors (mostly @typescript-eslint/no-explicit-any in test files and no-undef in .mjs scripts). These errors predate the execution-loop work and are not introduced by FEAT-004. Only 16 auto-fixable errors were fixed."
}
},
"reviewsSummary": {
"total": 1,
"passed": 0,
"failed": 1,
"failedFeatures": ["FEAT-004"]
},
"blockingIssues": [
{
"featureId": "FEAT-004",
"severity": "blocking",
"file": "packages/engine/src/mission-execution-loop.ts",
"line": 317,
"description": "parseValidationResult() is a stub that always returns { status: 'pass', assertions: all passed }. The comment explicitly says 'For now, return a default pass result since we don't have the actual parsing logic implemented'. This means validation failures are never detected, handleValidationFail is never called, and fix features are never generated. Violates VAL-EL-006 (validation failure creates fix feature) and VAL-EL-007 (validation blocked)."
},
{
"featureId": "FEAT-004",
"severity": "blocking",
"file": "packages/engine/src/mission-execution-loop.ts",
"line": 480,
"description": "notifyValidationComplete callback passes featureId to missionAutopilot.handleTaskCompletion(featureId), but handleTaskCompletion expects a taskId parameter. Inside MissionAutopilot.handleTaskCompletion, it calls getFeatureByTaskId(taskId) which receives a featureId and returns null, causing early-return without action. This breaks autopilot coordination for loop states (VAL-EL-012)."
},
{
"featureId": "FEAT-004",
"severity": "blocking",
"file": "packages/engine/src/mission-execution-loop.ts",
"line": 134,
"description": "recoverActiveMissions() only logs features in validating/needs_fix states but performs no state transitions or re-triggers. Features in 'validating' state remain stuck with no recovery path unless a new task completion event fires. Violates VAL-EL-009 which requires recovery to 're-enqueue pending validations'."
}
],
"appliedUpdates": [],
"suggestedGuidanceUpdates": [
{
"target": "AGENTS.md",
"suggestion": "Clarify TDD enforcement: AGENTS.md specifies 'write tests BEFORE implementation (TDD)' but no test file was created for MissionExecutionLoop despite the requirement. Consider requiring workers to show test file creation in the same commit as implementation, or adding a validation step that checks for test files.",
"evidence": "FEAT-004 handoff shows tests.added: [] despite AGENTS.md requiring TDD; mission-execution-loop.test.ts referenced in feature spec but never created",
"isSystemic": true
},
{
"target": "skills/backend-worker",
"suggestion": "Add guidance on AI response parsing: The worker skill procedure was followed correctly but the backend-worker skill does not document how to extract structured JSON from AI agent sessions. The parseValidationResult stub was left unimplemented due to this knowledge gap. Consider adding a library entry or skill section on 'Extracting structured data from AI sessions'.",
"evidence": "parseValidationResult at mission-execution-loop.ts:317 returns hardcoded pass; comment acknowledges 'don't have actual parsing logic'; parseValidatorResponse methods in fn-1587 worktree not committed",
"isSystemic": false
}
],
"rejectedObservations": [
{
"observation": "runId='unknown' fallback in createGeneratedFixFeature call would throw (line 492)",
"reason": "Non-blocking: This code path is unreachable because parseValidationResult always returns 'pass', so the catch block with createGeneratedFixFeature is never executed. The issue would only manifest if parseValidationResult were fixed, but it would also require fix in the runId fallback logic."
}
],
"previousRound": null
}