fix: correct workspace review fingerprint range and land-intent resolve gating
Two product defects surfaced by workspace-e2e's remaining failures. 1. A merge-boundary fence silently did not apply. captureWorkspaceReviewEvidence computes a repository's file list over baseCommitSha..<resolved task branch>, but computeReviewDiffFingerprint hardcoded baseRef..HEAD. For a workspace entry whose checkout sits on the integration branch those are different ranges, so the fingerprint did not describe the files captured beside it: a diverged checkout hard-failed an approved repository as content-changed, and a checkout at the base produced an empty diff -> undefined fingerprint -> the repo dropped out of mergeBoundaryFingerprints, so BOTH the approval-missing and content-changed fences stopped applying to it at all. computeReviewDiffFingerprint now takes an optional headRef; workspace evidence passes the resolved task branch. The singular-review caller, whose worktree IS the branch, keeps the ambient HEAD default. 2. Land intents were recorded and resolved under different conditions. landOneRepo records an intent only when ctx.workspaceLand is set, which landWorkspaceTask passes only for remote targets, but the resolve side was gated on durableLandLease alone. A local-only land therefore resolved an intent that was never recorded, got "missing", and failed a fully-landed repo as a partial land AFTER its integration ref had advanced. Resolve now uses the same condition as record. The approveWorkspaceReview helper's "reviewStep called exactly once" constant only held because defect 1 suppressed a repository; it now derives the expected count from the same production capture the review loop uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
gsxdsm
7
.changeset/workspace-local-only-land-fingerprint.md
Normal file
7
.changeset/workspace-local-only-land-fingerprint.md
Normal file
@@ -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.
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<string> => "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<void> {
|
||||
/*
|
||||
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 `<harnessRoot>/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);
|
||||
|
||||
359
packages/engine/src/__tests__/zz-probe-stuck.test.ts
Normal file
359
packages/engine/src/__tests__/zz-probe-stuck.test.ts
Normal file
@@ -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> = {}): 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 "<task-branch>" 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 "<task-branch>" 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<string, unknown> = {}) {
|
||||
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<Task>) => {
|
||||
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> | void = async () => {}) {
|
||||
let started!: () => void;
|
||||
const startedPromise = new Promise<void>((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<string, unknown> = {},
|
||||
beforeAbort?: (live: Task) => void,
|
||||
) {
|
||||
const store = createMutableStore(task, settings);
|
||||
let releasePrompt!: () => void;
|
||||
const promptRelease = new Promise<void>((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<string, unknown> = {}) {
|
||||
const store = createMutableStore(task, {
|
||||
runStepsInNewSessions: true,
|
||||
maxParallelSteps: 2,
|
||||
...settings,
|
||||
});
|
||||
let release!: () => void;
|
||||
mockExecuteAll.mockReturnValue(new Promise<void>((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<void>((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;
|
||||
});
|
||||
});
|
||||
@@ -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;
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<string | undefined> {
|
||||
/**
|
||||
* 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<string | undefined> {
|
||||
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;
|
||||
|
||||
@@ -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 });
|
||||
|
||||
Reference in New Issue
Block a user