fix: resolve full-suite CI failures across engine + dashboard (shards 1-4)#1947
Open
gsxdsm wants to merge 20 commits into
Open
fix: resolve full-suite CI failures across engine + dashboard (shards 1-4)#1947gsxdsm wants to merge 20 commits into
gsxdsm wants to merge 20 commits into
Conversation
…-validation-trigger-gap mocks recoverActiveMissions (mission-execution-loop.ts:263) calls missionStore.reconcileSupersededGeneratedFixFeatures per slice; the 5 MissionExecutionLoop-backed mocks here omitted it, so recovery threw (TypeError) at the slice loop and aborted before processTaskOutcome / ensureFeatureAssertionLinked / startValidatorRun ran — 4 tests failed. Add a no-op stub (matches mission-execution-loop.test.ts reference) with an FNXC:MissionReconcile note. No-op is correct: supersession is not exercised by these tests.
…/7340/7156/7342/6825/7265) Update five dashboard app test files whose assertions drifted from intentional product changes that landed on main without updating them: - graph-workflow-header: FN-7057 treats stale/missing workflow ids as the default workflow, so FN-unknown now shows under the default selection. - EngineControlMenu.css: FN-7340 added a 768px range-thumb touch-target block; narrow the popover-breakpoint assertion to that selector. - MissionManager.delete-confirm: FN-7156 removed first-mission auto-select; explicitly select the mission before the detail-delete flow. - board-mobile-initial-render: FN-7342 preserves board column scroll during stabilization; FN-6825 renders the workflow toolbar on options, not callbacks. - workflow-auto-layout: FN-7265 removed the stepwise review node (per-step review lives in the foreach); the connected run ends at completion-summary. No assertion was loosened or deleted to force a pass; each change cites the breaking commit via an FNXC comment. packages/dashboard is private (no changeset).
… fail-closed getTask)
…erger-merge-details assertions (FN-7503)
…to isolate git worktree prune plumbing
…des for sub-repo worktrees (FN-7360)
…d-signal snapshot (FN-7577)
…kflow mocks (FN-7229 retry-cap + workflow verdict wiring)
…rkflowOptionalSteps mock
…ock (3167dbc); align stepwise/graph/prompt-override tests (FN-7265/7335)
…rtbeat/triage tests (FN-7503)
…ack call (specifyTask no longer loops)
…ree exec counts + sync conflict mapping
…ion-executor logging/terminal-activity assertions
…vePlanningMcpServers helper
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds proven landed-commit recovery for workspace partial-land retries, strips shared branch overrides during sub-repo worktree acquisition, and updates dashboard and engine tests and mocks to match revised workflow, logging, and startup behavior. ChangesWorkspace recovery and worktree setup
Dashboard and engine test alignment
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThis PR fixes full-suite CI failures across the engine and dashboard. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "fix(engine): require exact trailer line,..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/engine/src/__tests__/triage-planning-prompt-single-source.test.ts (1)
21-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the
ModelFallbackExhaustedErrormock to a shared test helper.The same inline
class ModelFallbackExhaustedError extends Error {}plusformatModelMarkerDetailsstub is duplicated verbatim across this file,triage-soft-delete-write-abort.test.ts, andtriage-split-into-subtasks-delete.test.ts. This PR already established the pattern of extracting shared mock/assertion logic into a helper (agent-log-assertions.ts'sexpectAppendAgentLog) for exactly this kind of cross-test duplication.♻️ Example: shared pi.js mock helper
// packages/engine/src/__tests__/pi-mock-helpers.ts export class MockModelFallbackExhaustedError extends Error {} export function basePiMockExports() { return { ModelFallbackExhaustedError: MockModelFallbackExhaustedError, formatModelMarkerDetails: vi.fn((model: string) => model), }; }Then in each test file:
- class ModelFallbackExhaustedError extends Error {} - return { - ModelFallbackExhaustedError, - createFnAgent: mockCreateFnAgent, - describeModel: vi.fn().mockReturnValue("mock-model"), - formatModelMarkerDetails: vi.fn((model: string) => model), - promptWithFallback: vi.fn().mockResolvedValue(undefined), - }; + return { + ...basePiMockExports(), + createFnAgent: mockCreateFnAgent, + describeModel: vi.fn().mockReturnValue("mock-model"), + promptWithFallback: vi.fn().mockResolvedValue(undefined), + };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/__tests__/triage-planning-prompt-single-source.test.ts` around lines 21 - 34, The `vi.mock("../pi.js")` setup is duplicating the same `ModelFallbackExhaustedError` class and `formatModelMarkerDetails` stub across multiple triage tests. Extract those shared exports into a reusable test helper (similar to `agent-log-assertions.ts`’s shared assertion helper) and have this file’s `vi.mock("../pi.js")` consume that helper so the mock shape stays consistent across `triage-planning-prompt-single-source.test.ts`, `triage-soft-delete-write-abort.test.ts`, and `triage-split-into-subtasks-delete.test.ts`.packages/dashboard/app/components/__tests__/board-no-legacy-flash.test.tsx (1)
285-297: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest only waits for the fetch call, not for the rejection to actually be handled.
waitFor(() => expect(fetchBoardWorkflows).toHaveBeenCalled())resolves as soon as the mock is invoked, not after its rejection has propagated throughuseBoardWorkflows's.catch. Since the subsequent assertion (expectSkeleton) is a no-change assertion (skeleton was already shown pre-call), this test can pass even if the non-authoritative-failure catch branch never executes, weakening its ability to catch a future regression where a rejected fetch does fall back to legacy.Consider waiting on a signal that the rejection was actually processed (e.g., flushing microtasks via
await Promise.resolve()after the call, or asserting on a state change that only happens after the catch runs) to make this a stronger regression guard.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/__tests__/board-no-legacy-flash.test.tsx` around lines 285 - 297, The test in the Board/ListView non-legacy flash case only waits for fetchBoardWorkflows to be called, so it does not prove the rejected promise was handled by useBoardWorkflows. Update this test to wait for the rejection path to settle after the mock invocation, using a microtask flush or another post-catch signal, so the assertion verifies the non-authoritative failure behavior instead of just the initial call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/engine/src/merger-ai.ts`:
- Around line 1236-1247: Recovery fallback can still leave the repo stranded
because a failed git rev-parse is swallowed and later records status as landed
with an undefined landedSha. In merger-ai.ts, update the recovery path in the
logic around recoveredLandedSha so that a failed refs/heads/${integrationBranch}
lookup is treated explicitly: either throw a retryable error like
WorkspacePartialLandError or emit a clear failure/audit signal and avoid pushing
a landed record without a SHA. Ensure the repos.push path only marks
alreadyLanded when a valid landedSha was recovered.
- Around line 1236-1247: The recovery path in merger-ai.ts is using the
integration branch tip as the landed SHA when entry.landedSha is missing, which
can point past the actual squash commit. Update the fallback logic in the AI
merge workspace flow so it preserves the original landed squash commit SHA
associated with the task, not refs/heads/${integrationBranch}. Make sure the
value stored in finalizeMerged via mergeDetails.commitSha and
workspaceLandedShas remains the task’s landed commit, since task-revert.ts uses
it as the range endpoint.
---
Nitpick comments:
In `@packages/dashboard/app/components/__tests__/board-no-legacy-flash.test.tsx`:
- Around line 285-297: The test in the Board/ListView non-legacy flash case only
waits for fetchBoardWorkflows to be called, so it does not prove the rejected
promise was handled by useBoardWorkflows. Update this test to wait for the
rejection path to settle after the mock invocation, using a microtask flush or
another post-catch signal, so the assertion verifies the non-authoritative
failure behavior instead of just the initial call.
In `@packages/engine/src/__tests__/triage-planning-prompt-single-source.test.ts`:
- Around line 21-34: The `vi.mock("../pi.js")` setup is duplicating the same
`ModelFallbackExhaustedError` class and `formatModelMarkerDetails` stub across
multiple triage tests. Extract those shared exports into a reusable test helper
(similar to `agent-log-assertions.ts`’s shared assertion helper) and have this
file’s `vi.mock("../pi.js")` consume that helper so the mock shape stays
consistent across `triage-planning-prompt-single-source.test.ts`,
`triage-soft-delete-write-abort.test.ts`, and
`triage-split-into-subtasks-delete.test.ts`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0117936a-816d-4237-b933-26194c9aeb90
📒 Files selected for processing (47)
.changeset/fn-workspace-landedsha-recovery.md.changeset/fn-worktree-subrepo-branch-strip.mddocs/signals-connectors.mdpackages/dashboard/app/__tests__/graph-workflow-header.test.tsxpackages/dashboard/app/components/__tests__/AppModals.test.tsxpackages/dashboard/app/components/__tests__/ChangesDiffModal.test.tsxpackages/dashboard/app/components/__tests__/EngineControlMenu.css.test.tspackages/dashboard/app/components/__tests__/MissionManager.delete-confirm.test.tsxpackages/dashboard/app/components/__tests__/TaskDetailModal.summary-tab.test.tsxpackages/dashboard/app/components/__tests__/board-mobile-initial-render.test.tsxpackages/dashboard/app/components/__tests__/board-no-legacy-flash.test.tsxpackages/dashboard/app/components/__tests__/workflow-auto-layout.test.tspackages/dashboard/src/__tests__/chat-routes.rooms.test.tspackages/dashboard/src/__tests__/register-git-github.backfill.test.tspackages/dashboard/src/__tests__/routes-agent-import.test.tspackages/dashboard/src/__tests__/routes-run-cited-goals.test.tspackages/dashboard/src/__tests__/routes-sandbox-audit.test.tspackages/dashboard/src/__tests__/session-resume-history.test.tspackages/dashboard/src/routes/__tests__/task-create-workflow-route.test.tspackages/engine/src/__tests__/agent-log-assertions.tspackages/engine/src/__tests__/ce-workflow-step-executor.test.tspackages/engine/src/__tests__/executor-step-session.test.tspackages/engine/src/__tests__/executor-test-helpers.tspackages/engine/src/__tests__/heartbeat-executor.test.tspackages/engine/src/__tests__/heartbeat-session-prompt.test.tspackages/engine/src/__tests__/invariant-wrong-checkout-completion.test.tspackages/engine/src/__tests__/mcp-surface-coverage.test.tspackages/engine/src/__tests__/merger-merge-details.test.tspackages/engine/src/__tests__/pi-create-fn-agent.test.tspackages/engine/src/__tests__/planner-overseer-intervention-wiring.test.tspackages/engine/src/__tests__/project-engine.test.tspackages/engine/src/__tests__/reliability-interactions/executor-liveness-gate.test.tspackages/engine/src/__tests__/reliability-interactions/mission-validation-trigger-gap.test.tspackages/engine/src/__tests__/reliability-interactions/post-finalize-verification-noop-status-write.test.tspackages/engine/src/__tests__/reliability-interactions/worktrunk-self-healing.test.tspackages/engine/src/__tests__/step-session-executor.test.tspackages/engine/src/__tests__/stepwise-workflow-parity.test.tspackages/engine/src/__tests__/triage-planning-prompt-single-source.test.tspackages/engine/src/__tests__/triage-soft-delete-write-abort.test.tspackages/engine/src/__tests__/triage-split-into-subtasks-delete.test.tspackages/engine/src/__tests__/workflow-graph-optional-step-fix.test.tspackages/engine/src/__tests__/workflow-prompt-overrides-resolution.test.tspackages/engine/src/__tests__/worktree-acquisition-backend.test.tspackages/engine/src/__tests__/worktree-acquisition-worktrunk.test.tspackages/engine/src/__tests__/worktree-backend.test.tspackages/engine/src/merger-ai.tspackages/engine/src/worktree-acquisition.ts
…ptile P1) findProvenLandedCommit returns the task's own trailer commit (or recorded landedSha when still an ancestor) instead of rev-parse on the integration tip, so an intervening sub-repo land can't attribute a later unrelated commit. Regression: intervening commit after lost persist recovers tipAfterFirst.
…nded commit (Greptile P1) findProvenLandedCommit now keeps --grep as a prefilter but verifies each candidate carries an actual 'Fusion-Task-Id: <taskId>' trailer line via git show -s --format=%B, so a later commit that merely mentions the trailer text in its body cannot be selected. Regression covers a body-mention intervening commit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the failing full-suite CI run on
main(run 28874651861) — all 4 test shards were red. ~32 test files failing across engine + dashboard (src + app), rooted in ~13 distinct causes from recent main commits. All resolved; the merge gate and full engine/dashboard suites are green locally.Root causes & fixes
Engine (shards 1 & 2)
appendAgentLog6th timing arg (FN-7503,2797803c0) —agent-logger.tsnow passes an optional{durationMs,timeToFirstTokenMs}6th arg; many executor/heartbeat/merger tests asserted the old 5-arg form. Added a shared timing-tolerant helperagent-log-assertions.ts(assertstaskId/text/type, tolerant of the timing object) and applied it across affected files — so future timing fields won't re-break every executor test.reconcileSupersededGeneratedFixFeatures(mission) —mission-execution-loop.tscalls a method the test's missionStore mock lacked; added a no-op stub (the realMissionStorealready implements it).ModelFallbackExhaustedError/proseSignalsClearApproval/extractJsonObjectCandidatesmissing fromvi.mock— converted stale hand-written mocks (../pi.js,../reviewer.jsinexecutor-test-helpers.ts) toimportOriginal-spread so real exports carry through.merger-ai.ts—landWorkspaceTasknow recovers the integration-tip sha aslandedShawhen the A1 trailer-fallback proved a sub-repo landed but its sha was never persisted, sofinalizeWorkspaceTaskcan build merge proof (was stranding partial-land retries in-review).worktree-acquisition.ts—acquireWorkspaceRepoWorktreestrips the shared projectintegrationBranch/baseBranchoverrides before forwarding toacquireTaskWorktree(FN-7360'sfreshStartPointwas resolving an absent shared branch).git symbolic-refexec — updated worktree exec-count assertions for the newresolveIntegrationBranchcall.Dashboard API (shard 3)
store.on('task:moved')(FN-7337) —createServernow registers the listener; backed the 4 affected MockStores with EventEmitter (shared root cause across chat-routes.rooms, register-git-github, routes-run-cited-goals, routes-sandbox-audit).routes-agent-import— core mock converted toimportOriginal-spread (was missing FN-7444 planning-deepening constants).session-resume-history— engine mock missingresolveMcpServersForStore.task-create-workflow-route—builtin:legacy-codingdefaultSteps now includeplan-review(FN-7224/7226).[GitLab Parity Inventory]cross-link indocs/signals-connectors.md.Dashboard app (shard 4)
fetchWorkflowOptionalStepsmock.MCP coverage
mcp-surface-coverageforwarding needle updated for FN-7446'sresolvePlanningMcpServershelper.Approach notes
Verification
custom-providers-openai-completionsimport error is stale localpi-ai@0.79.9vs the lockfile's0.80.3— CI's fresh install resolves/compat; it passed in the original CI run).dashboard-api-quality-backfill): 242 files / 3185 tests / 0 failures.dashboard-app-quality-backfill): all targeted files green (37 + 95 tests).pnpm test:gate): engine-core 326 + ci-shape 63, plus nohup/4040/appeasement/changeset-format checks — all pass.@runfusion/fusionbehavior fixes (workspace landedSha, sub-repo worktree branch-strip).Summary by CodeRabbit