FN-6241: rescue quarantined engine tests
Rescue quarantined engine tests by adding deterministic seams and fake-timer flushing.\n\n- Add an injectable staged-files reader for squash file-scope invariant checks while preserving the production git-backed default.\n- Update file-scope invariant tests to use the deterministic staged-files seam instead of child_process mocking.\n- Stabilize project engine reconciliation retry coverage by avoiding mixed real timers in fake-timer tests.\n- Remove the rescued engine tests from Vitest excludes and the quarantine ledger.\n\nFiles changed:\n .../__tests__/merger-file-scope-invariant.test.ts | 39 ++++++++++++----------\n .../src/__tests__/project-engine-manager.test.ts | 18 +++++-----\n packages/engine/src/merger.ts | 22 ++++++++----\n packages/engine/vitest.config.ts | 3 --\n scripts/lib/test-quarantine.json | 10 ------\n 5 files changed, 48 insertions(+), 44 deletions(-) Fusion-Task-Id: FN-6241 Fusion-Task-Lineage: c1c0d40c-7581-4bcf-8244-688ea04d12b9
This commit is contained in:
@@ -49,21 +49,18 @@ function createMergeResult(): MergeResult {
|
||||
};
|
||||
}
|
||||
|
||||
let stagedFilesReader: (cwd: string) => Promise<string[]> = vi.fn(async () => []);
|
||||
|
||||
function mockStagedFiles(files: string[]) {
|
||||
stagedFilesReader = vi.fn(async (_cwd: string) => files);
|
||||
}
|
||||
|
||||
describe("assertSquashOverlapsFileScope", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockStagedFiles([]);
|
||||
});
|
||||
|
||||
function mockStagedFiles(files: string[]) {
|
||||
mockedExecSync.mockImplementation((cmd: any) => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr === "git diff --cached --name-only") {
|
||||
return files.join("\n");
|
||||
}
|
||||
return "";
|
||||
});
|
||||
}
|
||||
|
||||
it("passes without logging when no declared scope exists", async () => {
|
||||
const store = createInvariantStore([]);
|
||||
mockStagedFiles(["packages/engine/src/merger.ts"]);
|
||||
@@ -72,6 +69,7 @@ describe("assertSquashOverlapsFileScope", () => {
|
||||
store: store as never,
|
||||
taskId: "FN-4073",
|
||||
rootDir: "/tmp/root",
|
||||
stagedFilesReader,
|
||||
task: await (store as any).getTask("FN-4073"),
|
||||
})).resolves.toBeUndefined();
|
||||
|
||||
@@ -86,6 +84,7 @@ describe("assertSquashOverlapsFileScope", () => {
|
||||
store: store as never,
|
||||
taskId: "FN-4073",
|
||||
rootDir: "/tmp/root",
|
||||
stagedFilesReader,
|
||||
task: await (store as any).getTask("FN-4073"),
|
||||
})).resolves.toBeUndefined();
|
||||
|
||||
@@ -103,6 +102,7 @@ describe("assertSquashOverlapsFileScope", () => {
|
||||
store: store as never,
|
||||
taskId: "FN-4073",
|
||||
rootDir: "/tmp/root",
|
||||
stagedFilesReader,
|
||||
task: await (store as any).getTask("FN-4073"),
|
||||
})).resolves.toBeUndefined();
|
||||
});
|
||||
@@ -115,6 +115,7 @@ describe("assertSquashOverlapsFileScope", () => {
|
||||
store: store as never,
|
||||
taskId: "FN-4073",
|
||||
rootDir: "/tmp/root",
|
||||
stagedFilesReader,
|
||||
task: await (store as any).getTask("FN-4073"),
|
||||
})).rejects.toMatchObject({
|
||||
name: "FileScopeViolationError",
|
||||
@@ -132,6 +133,7 @@ describe("assertSquashOverlapsFileScope", () => {
|
||||
store: store as never,
|
||||
taskId: "FN-4073",
|
||||
rootDir: "/tmp/root",
|
||||
stagedFilesReader,
|
||||
task: await (store as any).getTask("FN-4073"),
|
||||
})).resolves.toBeUndefined();
|
||||
});
|
||||
@@ -144,6 +146,7 @@ describe("assertSquashOverlapsFileScope", () => {
|
||||
store: store as never,
|
||||
taskId: "FN-4073",
|
||||
rootDir: "/tmp/root",
|
||||
stagedFilesReader,
|
||||
task: await (store as any).getTask("FN-4073"),
|
||||
})).rejects.toMatchObject({
|
||||
name: "FileScopeViolationError",
|
||||
@@ -151,12 +154,6 @@ describe("assertSquashOverlapsFileScope", () => {
|
||||
} satisfies Partial<FileScopeViolationError>);
|
||||
});
|
||||
|
||||
// Skipped: flakes under workspace-concurrent runs because the
|
||||
// vi.mock("node:child_process") implementation occasionally doesn't take
|
||||
// effect, letting `git diff --cached --name-only` reach the real git binary
|
||||
// (which reports staged files unrelated to the test scope and trips the
|
||||
// FileScopeViolationError). The same logic is covered by the existing
|
||||
// real-git fixture tests in reliability-interactions/workflow-and-file-scope.
|
||||
it("accepts declared scope as a single changeset file when staged matches exactly", async () => {
|
||||
const store = createInvariantStore([".changeset/fn-4767-pr-flow.md"]);
|
||||
mockStagedFiles([".changeset/fn-4767-pr-flow.md"]);
|
||||
@@ -165,11 +162,11 @@ describe("assertSquashOverlapsFileScope", () => {
|
||||
store: store as never,
|
||||
taskId: "FN-4073",
|
||||
rootDir: "/tmp/root",
|
||||
stagedFilesReader,
|
||||
task: await (store as any).getTask("FN-4073"),
|
||||
})).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
// Skipped: same flake mode as the test above.
|
||||
it("accepts declared scope as a changeset glob when staged file matches", async () => {
|
||||
const store = createInvariantStore([".changeset/*.md"]);
|
||||
mockStagedFiles([".changeset/fn-4767-pr-flow.md"]);
|
||||
@@ -178,6 +175,7 @@ describe("assertSquashOverlapsFileScope", () => {
|
||||
store: store as never,
|
||||
taskId: "FN-4073",
|
||||
rootDir: "/tmp/root",
|
||||
stagedFilesReader,
|
||||
task: await (store as any).getTask("FN-4073"),
|
||||
})).resolves.toBeUndefined();
|
||||
});
|
||||
@@ -190,6 +188,7 @@ describe("assertSquashOverlapsFileScope", () => {
|
||||
store: store as never,
|
||||
taskId: "FN-4073",
|
||||
rootDir: "/tmp/root",
|
||||
stagedFilesReader,
|
||||
task: await (store as any).getTask("FN-4073"),
|
||||
})).resolves.toBeUndefined();
|
||||
|
||||
@@ -214,6 +213,7 @@ describe("assertSquashOverlapsFileScope", () => {
|
||||
store: store as never,
|
||||
taskId: "FN-4073",
|
||||
rootDir: "/tmp/root",
|
||||
stagedFilesReader,
|
||||
task: await (store as any).getTask("FN-4073"),
|
||||
})).resolves.toBeUndefined();
|
||||
|
||||
@@ -230,6 +230,7 @@ describe("assertSquashOverlapsFileScope", () => {
|
||||
describe("enforceSquashFileScopeInvariant audit emission", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockStagedFiles(["packages/core/src/store.ts"]);
|
||||
});
|
||||
|
||||
it("emits run_audit event on file-scope violation but continues", async () => {
|
||||
@@ -245,6 +246,7 @@ describe("enforceSquashFileScopeInvariant audit emission", () => {
|
||||
store: store as never,
|
||||
taskId: "FN-4073",
|
||||
rootDir: "/tmp/root",
|
||||
stagedFilesReader,
|
||||
task: await (store as any).getTask("FN-4073"),
|
||||
resetLabel: "file-scope invariant violation",
|
||||
auditor: auditor as any,
|
||||
@@ -281,6 +283,7 @@ describe("enforceSquashFileScopeInvariant audit emission", () => {
|
||||
store: store as never,
|
||||
taskId: "FN-4073",
|
||||
rootDir: "/tmp/root",
|
||||
stagedFilesReader,
|
||||
task: await (store as any).getTask("FN-4073"),
|
||||
resetLabel: "file-scope invariant violation",
|
||||
auditor: auditor as any,
|
||||
@@ -302,6 +305,7 @@ describe("enforceSquashFileScopeInvariant audit emission", () => {
|
||||
store: store as never,
|
||||
taskId: "FN-4073",
|
||||
rootDir: "/tmp/root",
|
||||
stagedFilesReader,
|
||||
task: await (store as any).getTask("FN-4073"),
|
||||
resetLabel: "file-scope invariant violation",
|
||||
auditor: auditor as any,
|
||||
@@ -328,6 +332,7 @@ describe("enforceSquashFileScopeInvariant audit emission", () => {
|
||||
store: store as never,
|
||||
taskId: "FN-4073",
|
||||
rootDir: "/tmp/root",
|
||||
stagedFilesReader,
|
||||
task: await (store as any).getTask("FN-4073"),
|
||||
resetLabel: "file-scope invariant violation",
|
||||
})).resolves.toBeUndefined();
|
||||
|
||||
@@ -432,8 +432,13 @@ describe("ProjectEngineManager", () => {
|
||||
});
|
||||
|
||||
describe("startReconciliation / stopReconciliation", () => {
|
||||
async function flushReconciliationWork(): Promise<void> {
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
await Promise.resolve();
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers({ shouldAdvanceTime: true });
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -547,9 +552,6 @@ describe("ProjectEngineManager", () => {
|
||||
manager.stopReconciliation();
|
||||
});
|
||||
|
||||
// Flake under full reliability-suite load: 30s timeout, but passes in ~46ms
|
||||
// standalone. Setinterval-driven reconciliation appears to race with vitest
|
||||
// fake-timer contention when other reliability-pool files are co-resident.
|
||||
it("retries failed project starts on subsequent reconciliation ticks", async () => {
|
||||
// Track how many times start() is called to fail only the FIRST set
|
||||
let startCallCount = 0;
|
||||
@@ -577,9 +579,9 @@ describe("ProjectEngineManager", () => {
|
||||
// Start reconciliation (runs immediate tick which fails all 3)
|
||||
manager.startReconciliation(1000);
|
||||
|
||||
// Wait for the immediate tick to complete
|
||||
await vi.advanceTimersByTimeAsync(100);
|
||||
await new Promise((resolve) => setTimeout(resolve, 10)); // Let promises settle
|
||||
// Wait for the immediate tick to complete without mixing real timers into
|
||||
// this fake-timer block.
|
||||
await flushReconciliationWork();
|
||||
|
||||
// After immediate tick: all should have failed
|
||||
expect(manager.getEngine("proj_aaa")).toBeUndefined();
|
||||
@@ -588,7 +590,7 @@ describe("ProjectEngineManager", () => {
|
||||
|
||||
// First scheduled tick (after 1000ms): should retry and succeed
|
||||
await vi.advanceTimersByTimeAsync(1000);
|
||||
await new Promise((resolve) => setTimeout(resolve, 10)); // Let promises settle
|
||||
await flushReconciliationWork();
|
||||
|
||||
expect(manager.getEngine("proj_aaa")).toBeDefined();
|
||||
expect(manager.getEngine("proj_bbb")).toBeDefined();
|
||||
|
||||
@@ -4969,18 +4969,31 @@ export class FileScopeViolationError extends Error {
|
||||
}
|
||||
}
|
||||
|
||||
export type StagedFilesReader = (cwd: string) => Promise<string[]>;
|
||||
|
||||
async function readStagedFileNames(cwd: string): Promise<string[]> {
|
||||
const { stdout } = await execAsync("git diff --cached --name-only", {
|
||||
cwd,
|
||||
encoding: "utf-8",
|
||||
});
|
||||
return stdout.split("\n").map((line) => line.trim()).filter(Boolean);
|
||||
}
|
||||
|
||||
export async function assertSquashOverlapsFileScope(params: {
|
||||
store: TaskStore;
|
||||
taskId: string;
|
||||
rootDir: string;
|
||||
task: Task;
|
||||
/** Test seam for deterministic file-scope invariant coverage. Production
|
||||
* callers use the default real-git staged-file reader. */
|
||||
stagedFilesReader?: StagedFilesReader;
|
||||
/** U7 (R10): when the merge trait's `fileScope: "custom"` mode is active,
|
||||
* these glob/path rules replace the task's File Scope section as the
|
||||
* declared scope. `scopeOverride` is a documented no-op only under
|
||||
* `fileScope: "off"` (handled by the caller, which skips this assert). */
|
||||
customScopeRules?: string[];
|
||||
}): Promise<void> {
|
||||
const { store, taskId, rootDir, task, customScopeRules } = params;
|
||||
const { store, taskId, rootDir, task, customScopeRules, stagedFilesReader = readStagedFileNames } = params;
|
||||
const hasCustomRules = Array.isArray(customScopeRules) && customScopeRules.length > 0;
|
||||
|
||||
if (!hasCustomRules && task.scopeOverride === true) {
|
||||
@@ -5011,11 +5024,7 @@ export async function assertSquashOverlapsFileScope(params: {
|
||||
return;
|
||||
}
|
||||
|
||||
const { stdout } = await execAsync("git diff --cached --name-only", {
|
||||
cwd: rootDir,
|
||||
encoding: "utf-8",
|
||||
});
|
||||
const stagedFiles = stdout.split("\n").map((line) => line.trim()).filter(Boolean);
|
||||
const stagedFiles = await stagedFilesReader(rootDir);
|
||||
const hasOverlap = stagedFiles.some((file) => matchesScope(file, declaredScope));
|
||||
if (!hasOverlap) {
|
||||
throw new FileScopeViolationError(taskId, stagedFiles, declaredScope);
|
||||
@@ -5039,6 +5048,7 @@ export async function enforceSquashFileScopeInvariant(params: {
|
||||
rootDir: string;
|
||||
task: Task;
|
||||
resetLabel: string;
|
||||
stagedFilesReader?: StagedFilesReader;
|
||||
auditor?: RunAuditor;
|
||||
}): Promise<void> {
|
||||
// U7 (R10): resolve the file-scope enforcement mode from the merge trait
|
||||
|
||||
@@ -86,7 +86,6 @@ export default defineConfig({
|
||||
exclude: [
|
||||
"node_modules/**",
|
||||
"dist/**",
|
||||
"src/__tests__/merger-file-scope-invariant.test.ts",
|
||||
],
|
||||
},
|
||||
},
|
||||
@@ -103,8 +102,6 @@ export default defineConfig({
|
||||
"src/**/*.slow.test.ts",
|
||||
"node_modules/**",
|
||||
"dist/**",
|
||||
"src/__tests__/merger-file-scope-invariant.test.ts",
|
||||
"src/__tests__/project-engine-manager.test.ts",
|
||||
"src/__tests__/merger-ai-cleanup-active-session.test.ts",
|
||||
"src/__tests__/merger-ai-cleanup.test.ts",
|
||||
"src/__tests__/merger-ai.test.ts",
|
||||
|
||||
@@ -1,16 +1,6 @@
|
||||
{
|
||||
"$comment": "Flaky-test quarantine ledger (deletion ratchet — see AGENTS.md 'Flaky tests: quarantine on sight' and docs/testing.md 'Quarantine ledger and the deletion ratchet'). A test observed failing without a corresponding real bug is quarantined ON SIGHT: add an entry here AND a matching one-line `exclude` entry in that package's vitest config, in the same commit. Every entry needs a non-empty `reason` (link the failing run) and a `quarantinedAt` date — the entry expires 14 days later, at which point the test file is DELETED unless someone rescues it with evidence it catches real regressions plus a root-cause fix (never appeasement). There is deliberately no loader module and no automation around this file: it is a dated record, the vitest config exclude is the mechanism, and the sweep is policy executed by whoever touches the suite.",
|
||||
"entries": [
|
||||
{
|
||||
"file": "packages/engine/src/__tests__/project-engine-manager.test.ts",
|
||||
"reason": "Flake: setInterval-driven reconciliation races with vitest fake-timer contention under full reliability-suite load. Test passes standalone (~46ms) but times out (30s) when reliability-pool files are co-resident. FN-6206.",
|
||||
"quarantinedAt": "2026-06-10"
|
||||
},
|
||||
{
|
||||
"file": "packages/engine/src/__tests__/merger-file-scope-invariant.test.ts",
|
||||
"reason": "Flake: vi.mock('node:child_process') occasionally doesn't take under workspace-concurrent runs, letting real git binary leak and report staged files unrelated to test scope (trips FileScopeViolationError). Same logic covered by real-git fixture tests in reliability-interactions/workflow-and-file-scope. FN-6206.",
|
||||
"quarantinedAt": "2026-06-10"
|
||||
},
|
||||
{
|
||||
"file": "packages/engine/src/__tests__/merger-ai-cleanup-active-session.test.ts",
|
||||
"reason": "Flake: pruneExistingAiMergeWorktrees skips active-session paths — active-session temp AI merge dir was unexpectedly pruned during pnpm --filter @fusion/engine test in FN-6206 verification, while the same file passed standalone. Root cause suspected: realpathSync resolution mismatch or readdirSync mock interaction with activeSessionRegistry singleton under concurrent engine suite load. Discovered during FN-6206.",
|
||||
|
||||
Reference in New Issue
Block a user