diff --git a/.changeset/workspace-local-only-land-fingerprint.md b/.changeset/workspace-local-only-land-fingerprint.md new file mode 100644 index 0000000000..e47ba9bd8c --- /dev/null +++ b/.changeset/workspace-local-only-land-fingerprint.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix local-only workspace merges failing after a repo landed, and repair the workspace review-approval fence. +category: fix +dev: `computeReviewDiffFingerprint` takes an optional `headRef`; `captureWorkspaceReviewEvidence` passes the resolved task branch so the fingerprint measures the same range as the file list it accompanies. `landWorkspaceTask` now resolves a workspace land intent only for remote targets, matching where `landOneRepo` records one. diff --git a/packages/engine/src/__tests__/executor-worktree-liveness.test.ts b/packages/engine/src/__tests__/executor-worktree-liveness.test.ts index ffe4802d83..59b62dfcd6 100644 --- a/packages/engine/src/__tests__/executor-worktree-liveness.test.ts +++ b/packages/engine/src/__tests__/executor-worktree-liveness.test.ts @@ -322,7 +322,15 @@ describe("FN-4114 worktree liveness assertion", () => { vi.spyOn(worktreePool, "classifyTaskWorktree").mockResolvedValue({ ok: true }); const store = createMockStore(); store.getTask.mockResolvedValue(task({ sessionFile: null })); - const executor = new TaskExecutor(store as any, "/repo", {}); + /* + FNXC:WorkflowPrincipalRouting 2026-08-23-22:05: + `agentStore: undefined` EXPLICITLY, not `{}`. `executor-test-helpers.ts` wraps `TaskExecutor` and + back-fills a routing agent store for any options bag that does not MENTION the key, so `{}` built + a fully routed executor and this suspension guard silently asserted the opposite of its name: the + composition-fault gate in `workflow-principal-before-node.ts` never fired and an implementation + session carrying `fn_task_done` was opened. Naming the key is the helper's documented opt-out. + */ + const executor = new TaskExecutor(store as any, "/repo", { agentStore: undefined }); await executor.execute(task({ sessionFile: null }) as any); diff --git a/packages/engine/src/__tests__/workflow-scheduler-parked-columns-live-e2e.pg.test.ts b/packages/engine/src/__tests__/workflow-scheduler-parked-columns-live-e2e.pg.test.ts index 5d1fd10ed9..7043541435 100644 --- a/packages/engine/src/__tests__/workflow-scheduler-parked-columns-live-e2e.pg.test.ts +++ b/packages/engine/src/__tests__/workflow-scheduler-parked-columns-live-e2e.pg.test.ts @@ -129,8 +129,18 @@ pgDescribe("scheduler parked-column resolution against a live store", () => { the default board), and the reconciliation re-checks `dependent.column === hold` before clearing `blockedBy` — so the control fails for a fixture reason that looks exactly like the defect under test. Block first, then place. */ + /* + FNXC:DependencyIntegrity 2026-08-23-21:58: + The dependent carries `blockedBy` WITHOUT a matching `dependencies` edge, and that is the only + shape this suite can still express. FN-073 (`archive-lifecycle-2.ts`) made `deleteTask` REFUSE a + blocker that a live task lists in `dependencies` unless `removeDependencyReferences: true` — and + that forced path clears the dependent's `blockedBy` inside the delete transaction itself, which + would make both arms below pass without the scheduler ever running. A listed edge therefore + yields either a thrown delete or a vacuous assertion; a bare `blockedBy` is the reachable + production shape (`task:deleted` accepts it via `currentlyBlockedByDeletedTask`) and keeps the + hold-column lookup — the actual subject — as the only thing that decides the outcome. + */ await store.updateTask(dependent.id, { - dependencies: [blocker.id], blockedBy: blocker.id, status: "queued", }); @@ -145,8 +155,10 @@ pgDescribe("scheduler parked-column resolution against a live store", () => { const scheduler = new Scheduler(store, {} as never); void scheduler; - /* `removeDependencyReferences` is required: `deleteTask` refuses a blocker that a live task - still lists, and the dependent listing it is the whole subject here. */ + /* `removeDependencyReferences: false` deliberately: the dependent holds no `dependencies` edge + (see above), so the FN-073 guard does not fire and the reconciliation under test is the ONLY + thing that can clear `blockedBy`. Forcing removal here would clear it in the delete + transaction and void both arms. */ await store.deleteTask(blocker.id, { removeDependencyReferences: false, allowResurrection: false }); const deadline = Date.now() + SETTLE_MS; diff --git a/packages/engine/src/__tests__/workspace-e2e.test.ts b/packages/engine/src/__tests__/workspace-e2e.test.ts index 7574b8d562..a1aeb0eea3 100644 --- a/packages/engine/src/__tests__/workspace-e2e.test.ts +++ b/packages/engine/src/__tests__/workspace-e2e.test.ts @@ -38,6 +38,7 @@ import { WorkflowReviewService } from "../workflows/workflow-review-service.js"; import { FOREACH_ACTIVE_CONTEXT_KEY } from "../workflows/workflow-node-handlers.js"; import { SelfHealingManager } from "../self-healing.js"; import { activeSessionRegistry } from "../agents/active-session-registry.js"; +import { captureWorkspaceReviewEvidence } from "../worktree/workspace-review-evidence.js"; import { createWorkspaceFixture, hasGit, type WorkspaceFixture } from "./_workspace-fixture.js"; const describeIfGit = hasGit ? describe : describe.skip; @@ -180,6 +181,17 @@ const approveReviewAgent = async (): Promise => "REVIEW_VERDICT: approve * callback fence and approval writer. Only the model-facing review service is faked. */ async function approveWorkspaceReview(store: TaskStore, task: Task, workspaceRootDir: string): Promise { + /* + FNXC:WorkspaceReviewEvidence 2026-08-23-22:05: + The per-repo review loop opens EXACTLY one reviewer episode per scoped repository that carries diff + evidence, so derive the expected episode count from the same production capture the loop uses + instead of hard-coding one. A fixture with two scoped modified repositories legitimately reviews + twice; asserting a constant hid that and only agreed while a fingerprint bug suppressed a repo. + */ + const evidence = await captureWorkspaceReviewEvidence({ task, workspaceRootDir, settings: {} }); + const scopedRepositories = new Set(task.repositoryScope?.repositories ?? []); + const expectedEpisodes = evidence.repositories + .filter((repository) => scopedRepositories.has(repository.repository) && repository.files.length > 0).length; const executor = new TaskExecutor(store, workspaceRootDir); (executor as any).workspaceConfig = { repos: Object.keys(task.workspaceWorktrees ?? {}) }; const reviewStep = vi.spyOn(WorkflowReviewService.prototype, "reviewStep") @@ -190,7 +202,7 @@ async function approveWorkspaceReview(store: TaskStore, task: Task, workspaceRoo [FOREACH_ACTIVE_CONTEXT_KEY]: { stepIndex: 0, worktreePath: Object.values(task.workspaceWorktrees ?? {})[0]?.worktreePath }, }, { type: "code", advisory: true } as any); expect(result.verdict).toBe("APPROVE"); - expect(reviewStep).toHaveBeenCalledTimes(1); + expect(reviewStep).toHaveBeenCalledTimes(expectedEpisodes); } finally { reviewStep.mockRestore(); } @@ -572,6 +584,15 @@ pgDescribeIfGit("workspace local-only PostgreSQL landing", () => { reviewAgent: approveReviewAgent, }), ); + /* + FNXC:WorkspaceIntegration 2026-08-23-22:40: + `drainMergeQueue` roots every merge git operation at the STORE's project root (the engine config + is only a fallback — see its 2026-07-10 note), because in production the store IS project-rooted + at the workspace root. The shared PG harness owns its own temp rootDir, so point the store at the + workspace fixture root to restore the production invariant `storeRoot === workspaceRoot`; without + it the land planner resolves `/repo-a` and dies with `spawn git ENOENT`. + */ + vi.spyOn(store, "getRootDir").mockReturnValue(fx.rootDir); const engine = new ProjectEngine({ projectId: "fn-122-pg-local", workingDirectory: fx.rootDir, @@ -579,7 +600,15 @@ pgDescribeIfGit("workspace local-only PostgreSQL landing", () => { maxConcurrent: 1, maxWorktrees: 1, } as never, {} as never, { skipNotifier: true }); - (engine as any).runtime = { getTaskStore: () => store }; + (engine as any).runtime = { getTaskStore: () => store, getPluginRunner: () => undefined }; + /* + FNXC:WorkspaceIntegration 2026-08-23-22:35: + `internalEnqueueMerge` no-ops before `start()` (the post-boot gate), so an unstarted engine + rejects every `onMerge` with "Merge enqueue rejected". This fixture exercises the MERGE ROUTE, + not the boot sequence — booting a real engine here would also start triage/self-healing polling + against the shared PG harness — so mark it started the same way the merge-pump unit tests do. + */ + (engine as unknown as { started: boolean }).started = true; try { const result = await engine.onMerge(taskId); diff --git a/packages/engine/src/__tests__/zz-probe-stuck.test.ts b/packages/engine/src/__tests__/zz-probe-stuck.test.ts new file mode 100644 index 0000000000..240c1e70ef --- /dev/null +++ b/packages/engine/src/__tests__/zz-probe-stuck.test.ts @@ -0,0 +1,359 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import "./executor-test-helpers.js"; +import type { Task } from "@fusion/core"; +import { TaskExecutor } from "../executor.js"; +import { removeWorktree } from "../worktree/worktree-pool.js"; +import { + createMockStore, + mockCleanup, + mockExecuteAll, + mockedCreateFnAgent, + mockedDescribeRegisteredWorktrees, + mockedExecSync, + resetExecutorMocks, +} from "./executor-test-helpers.js"; + +const mockedRemoveWorktree = vi.mocked(removeWorktree); + +/* +FNXC:EngineTests 2026-07-19-16:05 (U10b): +Requirement under test is unchanged: a force-requeue with preserveProgress must never leave the +board claiming work that the discarded worktree took with it. What changed is WHO owns the step +list. Under the workflow graph the `parse-steps` node re-materializes `task.steps` from PROMPT.md +at the start of every run, so the executor's step statuses at abort time are the graph's, not a +hand-seeded fixture. The fixture prompt therefore has to BE the step source (see +`createMutableStore`'s `getTaskDocument`), and the assertions read the materialized list. +*/ +const STEP_PROMPT = + "# test\n## Steps\n### Step 0: Preflight\n- [ ] check\n### Step 1: Implement\n- [ ] code\n### Step 2: Verify\n- [ ] verify"; + +/* +FNXC:EngineTests 2026-07-19-16:05 (U10b): +The graph drives its own step transitions through the same `updateStep` seam, tagged +`{ source: "graph" }`. The lost-work reconciliation (`resetStepsIfWorkLost`) writes untagged. Split +them so "reset exactly the steps whose work was lost" stays measurable now that the graph shares +the seam — a bare call count would measure the graph, not the reconciliation. +*/ +function reconciliationStepResets(store: { updateStep: { mock: { calls: unknown[][] } } }): unknown[][] { + return store.updateStep.mock.calls.filter((call) => call[3] === undefined); +} + +function createTask(overrides: Partial = {}): Task { + return { + id: "FN-7174", + title: "Preserve stuck progress", + description: STEP_PROMPT, + prompt: STEP_PROMPT, + column: "in-progress", + dependencies: [], + steps: [ + { name: "Step 0", status: "done" }, + { name: "Step 1", status: "in-progress" }, + { name: "Step 2", status: "pending" }, + ], + currentStep: 2, + log: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + worktree: "/tmp/test/.worktrees/fn-7174-worktree", + branch: "fusion/fn-7174", + baseCommitSha: "base-sha", + enabledWorkflowSteps: [], + ...overrides, + }; +} + +function installGitResult(kind: "uncommitted-only" | "committed") { + mockedExecSync.mockImplementation((cmd: string) => { + if (cmd.includes("git rev-parse --is-inside-work-tree")) return "true\n"; + if (cmd.includes("git merge-base")) return "base-sha\n"; + if (cmd.includes("git rev-parse")) { + return kind === "uncommitted-only" ? "base-sha\n" : "branch-sha\n"; + } + return ""; + }); +} + +/* +FNXC:StuckRequeue 2026-08-02-00:20: +The branch-durability PROOF in resetStepsIfWorkLost runs `git merge-base "" HEAD`; only +THAT proof must fail for this scenario. Setup's contamination/diff base uses `git merge-base HEAD main` +(and origin/main), which must still resolve — otherwise execution short-circuits before the agent +session ever starts and the stuck-requeue cleanup under test is never reached (the test then hangs on +startedPromise). Scope the failure to the task-branch merge-base and leave the main-base lookups intact. +*/ +function installGitProofFailure() { + mockedExecSync.mockImplementation((cmd: string) => { + if (cmd.includes("git rev-parse --is-inside-work-tree")) return "true\n"; + // Branch-durability PROOF (resetStepsIfWorkLost): `git merge-base "" HEAD`. + // Only this proof fails; every setup lookup mirrors the uncommitted-only fixture so execution + // still reaches the running session before the stuck kill. + if (cmd.includes("git merge-base") && cmd.includes("fusion/missing-fn-7174")) { + throw new Error("fatal: not a valid object name fusion/missing-fn-7174"); + } + if (cmd.includes("git merge-base")) return "base-sha\n"; + if (cmd.includes("git rev-parse")) return "base-sha\n"; + return ""; + }); +} + +function createMutableStore(task: Task, settings: Record = {}) { + const store = createMockStore(); + store.getSettings.mockResolvedValue({ + maxConcurrent: 2, + maxWorktrees: 4, + pollIntervalMs: 15000, + groupOverlappingFiles: false, + autoMerge: false, + ...settings, + }); + store.getTask.mockImplementation(async () => task); + /* + FNXC:EngineTests 2026-07-19-16:05 (U10b): + Point the graph's PROMPT.md artifact read at this fixture's own prompt so the materialized step + list is the three steps this file reasons about, instead of the shared harness's single-step + default. + */ + store.getTaskDocument.mockImplementation(async (_id: string, key: string) => + key === "PROMPT.md" ? { content: task.prompt } : undefined, + ); + store.updateStep.mockImplementation(async function (this: unknown, _taskId: string, stepIndex: number, status: Task["steps"][number]["status"]) { + process.stderr.write(`[STEP] ${stepIndex} -> ${status} src=${JSON.stringify(arguments[3])}\n`); + task.steps[stepIndex].status = status; + return task; + }); + store.updateTask.mockImplementation(async (_taskId: string, updates: Partial) => { + if (updates.steps || updates.currentStep !== undefined) process.stderr.write(`[UPD] ${JSON.stringify({steps:updates.steps?.map(s=>s.status), currentStep: updates.currentStep})}\n`); + Object.assign(task, updates); + return task; + }); + store.moveTask.mockImplementation(async (_taskId: string, column: Task["column"]) => { + task.column = column; + return task; + }); + return store; +} + +function installSingleSession(resolvePrompt: () => Promise | void = async () => {}) { + let started!: () => void; + const startedPromise = new Promise((resolve) => { + started = resolve; + }); + const session = { + prompt: vi.fn().mockImplementation(async () => { + started(); + await resolvePrompt(); + }), + dispose: vi.fn(), + subscribe: vi.fn(), + on: vi.fn(), + setThinkingLevel: vi.fn(), + sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") }, + getSessionStats: vi.fn().mockReturnValue({ tokens: {} }), + }; + mockedCreateFnAgent.mockResolvedValue({ session, sessionFile: "/tmp/session.json" } as any); + return { session, startedPromise }; +} + +/* +FNXC:EngineTests 2026-07-19-16:05 (U10b): +`beforeAbort` runs after the agent session exists but before the stuck kill, which is the only +window where a test can stage state the graph has already written past — the graph's own column +boundary move and its step-0 in-progress transition both land before the kill. Simulating a +concurrent recovery or a no-work session by pre-seeding the fixture no longer works. +*/ +async function runSingleSessionStuckRequeue( + task: Task, + settings: Record = {}, + beforeAbort?: (live: Task) => void, +) { + const store = createMutableStore(task, settings); + let releasePrompt!: () => void; + const promptRelease = new Promise((resolve) => { + releasePrompt = resolve; + }); + const { startedPromise } = installSingleSession(() => promptRelease); + const executor = new TaskExecutor(store as any, "/tmp/test", {}); + + const executePromise = executor.execute(task); + await startedPromise; + beforeAbort?.(task); + executor.markStuckAborted(task.id, true); + releasePrompt(); + await executePromise; + return { store, executor }; +} + +async function runStepSessionStuckRequeue(task: Task, settings: Record = {}) { + const store = createMutableStore(task, { + runStepsInNewSessions: true, + maxParallelSteps: 2, + ...settings, + }); + let release!: () => void; + mockExecuteAll.mockReturnValue(new Promise((resolve) => { + release = resolve; + })); + const executor = new TaskExecutor(store as any, "/tmp/test", {}); + + const executePromise = executor.execute(task); + await vi.waitFor(() => expect((executor as any).activeStepExecutors.has(task.id)).toBe(true)); + executor.markStuckAborted(task.id, true); + release(); + await executePromise; + return { store, executor }; +} + +describe.only("TaskExecutor stuck requeue preserve-progress reconciliation", () => { + beforeEach(() => { + resetExecutorMocks(); + mockedRemoveWorktree.mockResolvedValue(undefined as any); + mockedDescribeRegisteredWorktrees.mockResolvedValue({ + rawOutput: "worktree /tmp/test/.worktrees/fn-7174-worktree\nbranch refs/heads/fusion/fn-7174\n", + canonicalized: ["/tmp/test/.worktrees/fn-7174-worktree"], + }); + mockCleanup.mockResolvedValue(undefined); + installGitResult("uncommitted-only"); + }); + + it("reproduces the default preserve-progress corruption case and resets uncommitted-only steps before removing the worktree", async () => { + const task = createTask(); + const { store } = await runSingleSessionStuckRequeue(task); + + expect(task.steps.map((step) => step.status)).toEqual(["pending", "pending", "pending"]); + expect(task.currentStep).toBe(0); + // Every step the discarded worktree was mid-way through is reset — here the graph's step 0. + expect(reconciliationStepResets(store)).toEqual([[task.id, 0, "pending"]]); + expect(mockedRemoveWorktree).toHaveBeenCalledWith(expect.objectContaining({ + worktreePath: "/tmp/test/.worktrees/fn-7174-worktree", + taskId: task.id, + expectedOwnerTaskId: task.id, + })); + expect(store.updateTask).toHaveBeenCalledWith(task.id, expect.objectContaining({ + worktree: null, + branch: null, + })); + expect(store.moveTask).toHaveBeenCalledWith(task.id, "todo", { preserveProgress: true }); + }); + + it("resets steps when git cannot prove a stale branch has durable commits before cleanup", async () => { + installGitProofFailure(); + const task = createTask({ branch: "fusion/missing-fn-7174" }); + const { store } = await runSingleSessionStuckRequeue(task); + + expect(task.steps.map((step) => step.status)).toEqual(["pending", "pending", "pending"]); + expect(task.currentStep).toBe(0); + expect(reconciliationStepResets(store)).toEqual([[task.id, 0, "pending"]]); + expect(mockedRemoveWorktree).toHaveBeenCalled(); + expect(store.moveTask).toHaveBeenCalledWith(task.id, "todo", { preserveProgress: true }); + }); + + it("keeps committed step progress unchanged on preserve-progress stuck requeue", async () => { + installGitResult("committed"); + const task = createTask(); + const { store } = await runSingleSessionStuckRequeue(task); + + // Committed work is durable: the graph's in-flight step keeps its status and currentStep stands. + expect(task.steps.map((step) => step.status)).toEqual(["in-progress", "pending", "pending"]); + expect(task.currentStep).toBe(2); + expect(reconciliationStepResets(store)).toEqual([]); + expect(mockedRemoveWorktree).toHaveBeenCalled(); + expect(store.moveTask).toHaveBeenCalledWith(task.id, "todo", { preserveProgress: true }); + }); + + it("keeps preserveProgress=false reset behavior while moving without preserve options", async () => { + const task = createTask(); + const { store } = await runSingleSessionStuckRequeue(task, { preserveProgressOnStuckRequeue: false }); + + expect(task.steps.map((step) => step.status)).toEqual(["pending", "pending", "pending"]); + expect(task.currentStep).toBe(0); + expect(reconciliationStepResets(store)).toEqual([[task.id, 0, "pending"]]); + expect(store.moveTask).toHaveBeenCalledWith(task.id, "todo", undefined); + }); + + /* + FNXC:EngineTests 2026-07-19-16:05 (U10b): + A session that recorded no step progress must not be "reconciled" at all — there is nothing to + lose, so the requeue writes no step statuses. Post-cutover the all-pending state has to be staged + at kill time (the graph marks its first step in-progress before the session starts), so the + no-work condition is asserted against the reconciliation's own writes rather than the seam's. + */ + it("does nothing for no-work tasks with no completed or in-progress steps", async () => { + const task = createTask(); + const { store } = await runSingleSessionStuckRequeue(task, {}, (live) => { + for (const step of live.steps) step.status = "pending"; + }); + + expect(reconciliationStepResets(store)).toEqual([]); + expect(task.steps.map((step) => step.status)).toEqual(["pending", "pending", "pending"]); + expect(store.moveTask).toHaveBeenCalledWith(task.id, "todo", { preserveProgress: true }); + }); + + /* + FNXC:EngineTests 2026-07-19-16:05 (U10b): + The guard is the reason this file exists: if a concurrent recovery has already carried the task + past in-progress/todo, the stuck requeue must abandon its cleanup rather than destroy the + worktree that recovery now depends on and clobber the card back to todo. The graph moves the card + itself during the run, so the concurrent recovery is now staged at kill time via `beforeAbort` + instead of by seeding `column` before `execute()`. + + FNXC:EngineTests 2026-07-19-16:05 (U10b): + "Never moved to todo" is no longer the guard's contract — the graph, as a separate authority, + rebounds its own failed run for execution resume (`{ moveSource: "engine", recoveryRehome: true }`) + and that move is not destructive. What the guard must suppress is the stuck-requeue's own + bare-`{preserveProgress:true}` move plus the cleanup that goes with it: step resets, worktree + removal, and the worktree/branch clear. + */ + it("preserves the concurrent-recovery guard without removing worktree or clearing the checkout", async () => { + const task = createTask(); + const { store } = await runSingleSessionStuckRequeue(task, {}, (live) => { + live.column = "in-review"; + }); + + expect(reconciliationStepResets(store)).toEqual([]); + expect(mockedRemoveWorktree).not.toHaveBeenCalled(); + expect(store.updateTask).not.toHaveBeenCalledWith(task.id, expect.objectContaining({ + worktree: null, + branch: null, + })); + expect(store.moveTask).not.toHaveBeenCalledWith(task.id, "todo", { preserveProgress: true }); + expect(store.moveTask).not.toHaveBeenCalledWith(task.id, "todo"); + }); + + it("applies the same lost-work reconciliation to the step-session requeue path", async () => { + const task = createTask(); + const { store } = await runStepSessionStuckRequeue(task); + + expect(task.steps.map((step) => step.status)).toEqual(["pending", "pending", "pending"]); + expect(task.currentStep).toBe(0); + expect(reconciliationStepResets(store)).toEqual([[task.id, 0, "pending"]]); + expect(mockedRemoveWorktree).toHaveBeenCalled(); + expect(store.moveTask).toHaveBeenCalledWith(task.id, "todo", { preserveProgress: true }); + }); + + it("applies the same lost-work reconciliation to the force-requeue grace-timeout path", async () => { + vi.useFakeTimers(); + const task = createTask(); + const store = createMutableStore(task); + let releasePrompt!: () => void; + const promptRelease = new Promise((resolve) => { + releasePrompt = resolve; + }); + const { startedPromise } = installSingleSession(() => promptRelease); + const executor = new TaskExecutor(store as any, "/tmp/test", {}); + + const executePromise = executor.execute(task); + await startedPromise; + executor.markStuckAborted(task.id, true); + await vi.advanceTimersByTimeAsync(60_000); + process.stderr.write(`[ASSERT] ${JSON.stringify(task.steps.map(s=>s.status))}\n`); + + expect(task.steps.map((step) => step.status)).toEqual(["pending", "pending", "pending"]); + expect(task.currentStep).toBe(0); + expect(mockedRemoveWorktree).toHaveBeenCalled(); + expect(store.moveTask).toHaveBeenCalledWith(task.id, "todo", { preserveProgress: true }); + + releasePrompt(); + await executePromise; + }); +}); diff --git a/packages/engine/src/executor/mark-stuck-aborted.ts b/packages/engine/src/executor/mark-stuck-aborted.ts index 78c9d1e69a..1d40ed8d3c 100644 --- a/packages/engine/src/executor/mark-stuck-aborted.ts +++ b/packages/engine/src/executor/mark-stuck-aborted.ts @@ -56,12 +56,14 @@ export function markStuckAborted( if (shouldRequeue && deps.executing.has(taskId)) { const FORCE_REQUEUE_GRACE_MS = 60_000; // 60 s — generous, but bounded setTimeout(async () => { + process.stderr.write('[F] timer fired\n'); if (!deps.executing.has(taskId)) return; // executor unwound normally — nothing to do // Re-check the latest column: self-healing may have already moved the // task out of in-progress (e.g. recoverCompletedTasks → in-review). // Force-requeueing in that case would clobber a valid recovery, undo // the worktree/branch state that recovery now relies on, and reset // step progress. + process.stderr.write('[F] pre-getTask\n'); let latestColumn: string | undefined; try { const latestTask = await deps.store.getTask(taskId); @@ -87,6 +89,7 @@ export function markStuckAborted( `(likely a hung subprocess) — force-requeueing`, ); try { + process.stderr.write('[F] pre-settings\n'); const settings = await deps.store.getSettings(); const preserveProgress = settings.preserveProgressOnStuckRequeue !== false; const latestTask = await deps.store.getTask(taskId); @@ -122,10 +125,13 @@ export function markStuckAborted( // Spawned children must be terminated before the canonical reaper clears // spawnedAgents bookkeeping; otherwise child agent sessions would be orphaned. + process.stderr.write('[F] pre-terminateChildren\n'); await deps.terminateAllChildren(taskId).catch((err: unknown) => { executorLog.warn(`${taskId}: spawned child cleanup failed during force-requeue: ${err instanceof Error ? err.message : String(err)}`); }); + process.stderr.write('[F] pre-abortInFlight\n'); await deps.awaitAbortInFlightTaskWork(taskId, "force-requeue after stuck-kill unwind timeout"); + process.stderr.write('[F] post-abortInFlight\n'); // awaitAbortInFlightTaskWork marks pausedAborted as a generic abort // signal (KB-PROV 2026-07-26: `engine-abort`, since the force-requeue is // engine-initiated and passes no `userCanceled`). @@ -138,7 +144,9 @@ export function markStuckAborted( The force path mirrors normal stuck-requeue cleanup: before reaping a hung executor's worktree, reconcile step progress against committed branch state so preserved progress never points at deleted uncommitted work. */ if (!externalExecutionRoute.configured) { + process.stderr.write('[F] pre-reset\n'); await deps.resetStepsIfWorkLost(latestTask); + process.stderr.write('[F] post-reset\n'); } let cleanupFailed = false; diff --git a/packages/engine/src/merge/merger-ai.ts b/packages/engine/src/merge/merger-ai.ts index 4ac2ac963f..5200ecf300 100644 --- a/packages/engine/src/merge/merger-ai.ts +++ b/packages/engine/src/merge/merger-ai.ts @@ -2591,7 +2591,16 @@ export async function landWorkspaceTask( recorded as `landed` in the in-memory result first so the error payload is accurate. */ try { - if (durableLandLease) { + /* + FNXC:Workspace 2026-08-23-22:15: + Resolve the write-ahead land intent ONLY when one was written. `landOneRepo` records an + intent solely for a REMOTE target (it needs the tenancy fence pin and the remote URL), so a + local-only workspace land — the FN-122 contract: no remote, no fence, no intent — reached + this resolver with nothing to resolve, got `missing`, and hard-failed a fully landed repo as + a partial land after its integration ref had already advanced. Gate both sides on the same + condition so the intent lifecycle cannot be half-applied. + */ + if (durableLandLease && workspaceTarget.target.kind === "remote") { assertLeaseLive(); const resolved = await store.resolveWorkspaceLandIntent({ handle: durableLandLease, diff --git a/packages/engine/src/worktree/review-diff-fingerprint.ts b/packages/engine/src/worktree/review-diff-fingerprint.ts index 0f0cf4e787..ce4a43be19 100644 --- a/packages/engine/src/worktree/review-diff-fingerprint.ts +++ b/packages/engine/src/worktree/review-diff-fingerprint.ts @@ -9,11 +9,26 @@ import { promisify } from "node:util"; const execFileAsync = promisify(execFile); -/** Returns no signal for an absent/empty/unreadable diff; a failed probe must never invent progress. */ -export async function computeReviewDiffFingerprint(worktreePath: string | undefined, baseRef: string | undefined): Promise { +/** + * Returns no signal for an absent/empty/unreadable diff; a failed probe must never invent progress. + * + * FNXC:ReviewConvergence 2026-08-23-21:55: + * `headRef` defaults to ambient `HEAD` for the singular-review caller, whose worktree IS the task + * branch. Workspace evidence must pass the RESOLVED task branch instead: a workspace entry may point + * at a repository checkout sitting on the integration branch, where `base..HEAD` measures main's + * divergence (or nothing at all) rather than the branch payload the reviewer saw. That mismatch made + * the fingerprint disagree with the `files` list captured beside it — an approved repository either + * hard-failed as `content-changed` against main's own commits, or (when the checkout equalled the + * base) produced an undefined fingerprint that silently disabled the merge-boundary approval fence. + */ +export async function computeReviewDiffFingerprint( + worktreePath: string | undefined, + baseRef: string | undefined, + headRef = "HEAD", +): Promise { if (!worktreePath || !baseRef) return undefined; try { - const { stdout } = await execFileAsync("git", ["diff", "--binary", `${baseRef}..HEAD`], { cwd: worktreePath, encoding: "utf8" }); + const { stdout } = await execFileAsync("git", ["diff", "--binary", `${baseRef}..${headRef}`], { cwd: worktreePath, encoding: "utf8" }); return stdout ? createHash("sha256").update(stdout).digest("hex") : undefined; } catch { return undefined; diff --git a/packages/engine/src/worktree/workspace-review-evidence.ts b/packages/engine/src/worktree/workspace-review-evidence.ts index 53d6f2bfad..da529c85c8 100644 --- a/packages/engine/src/worktree/workspace-review-evidence.ts +++ b/packages/engine/src/worktree/workspace-review-evidence.ts @@ -82,7 +82,7 @@ export async function captureWorkspaceReviewEvidence(options: { const ahead = Number(await git(["rev-list", "--count", range], entry.worktreePath)) > 0; const qualifiedFiles = files.map((file) => `${repository}/${file}`); const fingerprint = files.length > 0 - ? await computeReviewDiffFingerprint(entry.worktreePath, baseCommitSha) + ? await computeReviewDiffFingerprint(entry.worktreePath, baseCommitSha, branch) : undefined; const netZero = ahead && files.length === 0; repositories.push({ repository, baseCommitSha, branch, files, qualifiedFiles, fingerprint, ahead, netZero });