## Summary
Wave 18 continues the package code-organization program after wave 17
domain folders (U4 Slice A from
`docs/plans/2026-07-14-001-refactor-package-code-organization-plan.md`).
### What changed
Peel **pure, behavior-preserving** helpers out of
`packages/engine/src/executor.ts` into domain modules under
`packages/engine/src/executor/`, with **stable re-exports** from
`executor.ts` so deep imports and `vi.mock("../executor.js")` keep
working.
| New module | Symbols |
|------------|---------|
| `executor/task-done-refusal.ts` | `evaluateTaskDoneRefusal`,
`determineRevisionResetStart`, skip-bypass refusal helper |
| `executor/workflow-feedback-paths.ts` |
`extractReferencedPathsFromWorkflowFeedback`,
`isAlwaysAllowedScopeLeakPath`, `workflowPathMatchesDeclaredScope` |
| `executor/workflow-step-verdict.ts` |
`FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE`, `parseWorkflowStepVerdict`
/ `parseWorkflowStepOutput`, step outcome types |
| `executor/await-input-parse.ts` | `parseAwaitInputSentinel`,
`parseAwaitInputQuestionToolCall` |
| `executor/no-commit-eligibility.ts` | `getNoCommitEligibilityReason`
(+ prompt heuristics) |
`executor.ts` live LOC ~**22817 → ~22427** (first pure-peel batch; more
peels needed to approach the 2k cap).
### Shims
- `old path` `executor.ts` public exports → `new path` `executor/*.ts` →
delete-when consumer deep-imports are re-pointed (not this PR)
### Test plan
- [x] `@fusion/engine` typecheck
- [x] Oracle: task-done refusal, skip-bypass, workflow malformed
verdict, scope-leak allowlist, executor-step-session, executor-prompt
- [x] `vitest --project=engine-core` (merge-gate curated suite)
- [ ] CI merge gate
**Stack:** wave17 (merged) → **this PR**
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Improved recognition of workflow outcomes from structured and
conversational responses.
* Added support for extracting questions from await-input responses and
tool calls.
* Improved workflow feedback handling for referenced files and declared
scope patterns.
* Added clearer guidance for task execution, approvals, verification,
and available tools.
* **Bug Fixes**
* Prevented completion when required review approvals are missing or
revisions remain pending.
* Improved handling of workflows that legitimately require no code
changes.
* Added clearer refusal messages and more reliable revision restarts.
* Sanitized repository paths in Git remediation instructions.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
145 lines
5.5 KiB
TypeScript
145 lines
5.5 KiB
TypeScript
import { beforeEach, describe, expect, it, vi } from "vitest";
|
|
import "./executor-test-helpers.js";
|
|
/*
|
|
FNXC:CodeOrganization 2026-08-03-14:10:
|
|
captureBaseCommitSha peeled to executor/worktree-git-refs.ts (U4 Slice B).
|
|
Gate suite calls the free function with an injected store — no TaskExecutor method.
|
|
*/
|
|
import { captureBaseCommitSha } from "../executor/worktree-git-refs.js";
|
|
import { executorLog } from "../logger.js";
|
|
import type { Task } from "@fusion/core";
|
|
import { createMockStore, mockedExec, mockedExecSync, resetExecutorMocks } from "./executor-test-helpers.js";
|
|
|
|
function makeTask(overrides: Partial<Task> = {}): Task {
|
|
return {
|
|
id: "FN-4383",
|
|
title: "Test",
|
|
description: "Test",
|
|
column: "in-progress",
|
|
dependencies: [],
|
|
steps: [],
|
|
currentStep: 0,
|
|
log: [],
|
|
createdAt: new Date().toISOString(),
|
|
updatedAt: new Date().toISOString(),
|
|
...overrides,
|
|
} as Task;
|
|
}
|
|
|
|
describe("captureBaseCommitSha", () => {
|
|
beforeEach(() => {
|
|
resetExecutorMocks();
|
|
});
|
|
|
|
it("captures merge-base for fresh worktree", async () => {
|
|
mockedExec.mockImplementation(((cmd: any, _opts: any, cb: any) => {
|
|
cb(null, cmd.includes("merge-base") ? "abc1234\n" : "");
|
|
return {} as any;
|
|
}) as any);
|
|
const store = createMockStore();
|
|
const audit = { git: vi.fn().mockResolvedValue(undefined) };
|
|
|
|
await captureBaseCommitSha(store, makeTask(), "/tmp/test/.worktrees/fn-4383", audit);
|
|
|
|
expect(store.updateTask).toHaveBeenCalledWith("FN-4383", { baseCommitSha: "abc1234" });
|
|
expect(audit.git).toHaveBeenCalledWith(expect.objectContaining({ metadata: { purpose: "base", preserved: false } }));
|
|
});
|
|
|
|
it("preserves existing valid baseCommitSha across resumed sessions", async () => {
|
|
mockedExecSync.mockReturnValue("");
|
|
const store = createMockStore();
|
|
const audit = { git: vi.fn().mockResolvedValue(undefined) };
|
|
|
|
await captureBaseCommitSha(
|
|
store,
|
|
makeTask({ baseCommitSha: "old123" }),
|
|
"/tmp/test/.worktrees/fn-4383",
|
|
audit,
|
|
{ isResume: true },
|
|
);
|
|
|
|
expect(store.updateTask).not.toHaveBeenCalled();
|
|
expect(audit.git).toHaveBeenCalledWith(expect.objectContaining({ metadata: { purpose: "base", preserved: true } }));
|
|
});
|
|
|
|
it("recaptures baseCommitSha on non-resume acquisitions even when stored value is ancestor (FN-4417)", async () => {
|
|
// FN-4417 regression: on a fresh pool acquisition the branch was just
|
|
// force-reset to current main, so any stored baseCommitSha is stale
|
|
// relative to the new merge-base. Preserving it would re-introduce the
|
|
// false-positive contamination cascade.
|
|
mockedExecSync.mockReturnValue(""); // is-ancestor would succeed if asked
|
|
mockedExec.mockImplementation(((cmd: any, _opts: any, cb: any) => {
|
|
cb(null, cmd.includes("merge-base") ? "freshmainSHA\n" : "");
|
|
return {} as any;
|
|
}) as any);
|
|
const store = createMockStore();
|
|
const audit = { git: vi.fn().mockResolvedValue(undefined) };
|
|
|
|
await captureBaseCommitSha(
|
|
store,
|
|
makeTask({ baseCommitSha: "stale_main_sha" }),
|
|
"/tmp/test/.worktrees/fn-4383",
|
|
audit,
|
|
{ isResume: false },
|
|
);
|
|
|
|
expect(store.updateTask).toHaveBeenCalledWith("FN-4383", { baseCommitSha: "freshmainSHA" });
|
|
expect(audit.git).toHaveBeenCalledWith(
|
|
expect.objectContaining({ metadata: { purpose: "base", preserved: false } }),
|
|
);
|
|
// Critically: is-ancestor must NOT have been the deciding factor.
|
|
// Even if it would have passed, non-resume always recaptures.
|
|
});
|
|
|
|
it("recaptures when existing baseCommitSha is not ancestor", async () => {
|
|
mockedExecSync.mockImplementation(() => {
|
|
throw new Error("not ancestor");
|
|
});
|
|
mockedExec.mockImplementation(((cmd: any, _opts: any, cb: any) => {
|
|
cb(null, cmd.includes("merge-base") ? "new456\n" : "");
|
|
return {} as any;
|
|
}) as any);
|
|
const store = createMockStore();
|
|
const audit = { git: vi.fn().mockResolvedValue(undefined) };
|
|
|
|
await captureBaseCommitSha(store, makeTask({ baseCommitSha: "stale999" }), "/tmp/test/.worktrees/fn-4383", audit);
|
|
|
|
expect(store.updateTask).toHaveBeenCalledWith("FN-4383", { baseCommitSha: "new456" });
|
|
});
|
|
|
|
it("preserves prior merge base on resume for FN-4309/FN-4383 multi-session regression", async () => {
|
|
mockedExecSync.mockReturnValue("");
|
|
const store = createMockStore();
|
|
const audit = { git: vi.fn().mockResolvedValue(undefined) };
|
|
|
|
await captureBaseCommitSha(
|
|
store,
|
|
makeTask({ baseCommitSha: "merge_base_sha" }),
|
|
"/tmp/test/.worktrees/fn-4383",
|
|
audit,
|
|
{ isResume: true },
|
|
);
|
|
|
|
expect(store.updateTask).not.toHaveBeenCalled();
|
|
expect(audit.git).toHaveBeenCalledWith(expect.objectContaining({ metadata: { purpose: "base", preserved: true } }));
|
|
});
|
|
|
|
it("falls back to HEAD when merge-base fails", async () => {
|
|
mockedExec.mockImplementation(((cmd: any, _opts: any, cb: any) => {
|
|
if (String(cmd).includes("merge-base")) {
|
|
cb(new Error("merge-base failed"), "", "merge-base failed");
|
|
return {} as any;
|
|
}
|
|
cb(null, "head777\n", "");
|
|
return {} as any;
|
|
}) as any);
|
|
const store = createMockStore();
|
|
const audit = { git: vi.fn().mockResolvedValue(undefined) };
|
|
|
|
await captureBaseCommitSha(store, makeTask(), "/tmp/test/.worktrees/fn-4383", audit);
|
|
|
|
expect(store.updateTask).toHaveBeenCalledWith("FN-4383", { baseCommitSha: "head777" });
|
|
expect(vi.mocked(executorLog.warn)).toHaveBeenCalledWith(expect.stringContaining("falling back to HEAD"));
|
|
});
|
|
});
|