FN-6486: rescue quarantined flaky tests

Rescue the same-day core and CLI flaky quarantines by fixing their fixture seams instead of appeasing timeouts.

- Make the core concurrent-write lock helper release synchronously inside its child process so package load cannot delay the transient lock release.
- Close real CLI TaskStore fixtures before removing temp roots and switch mock cleanup to non-hoisted unmocking.
- Remove both test files from package quarantine excludes and clear the quarantine ledger while documenting the rescue pattern.

Files changed:
 docs/testing.md                                         |  2 ++
 packages/cli/src/__tests__/extension-task-tools.test.ts | 17 +++++++++++++----
 packages/cli/vitest.config.ts                           |  4 +++-
 .../core/src/__tests__/store-concurrent-writes.test.ts  |  8 +++++++-
 packages/core/vitest.config.ts                          |  4 +++-
 scripts/lib/test-quarantine.json                        | 13 +------------
 6 files changed, 29 insertions(+), 19 deletions(-)

Fusion-Task-Id: FN-6486

Fusion-Task-Lineage: c90358c4-0549-4c68-8d17-2e2a336433b5
This commit is contained in:
gsxdsm
2026-06-15 08:06:53 -07:00
parent 84830595f2
commit a38752c6bb
6 changed files with 29 additions and 19 deletions

View File

@@ -160,6 +160,8 @@ Legitimate legacy exceptions must be recorded in `scripts/lib/test-timeout-appea
**Non-CLI quarantine sweep pattern (FN-6433):** for engine/core/dashboard batches, first remove quarantine excludes only in temporary local configs and run the exact quarantined files together so suite-load coupling is visible before editing the ledger. Rescue is valid when the grouped package lane proves the invariant now holds (for example, FN-6433 fixed engine cross-file interference by replacing broad `activeSessionRegistry.clear()` cleanup with path-scoped unregistering) or when a prior shared-fixture fix is demonstrated under package load. Delete duplicate/low-value files under the ratchet when another deterministic suite owns the same invariant. Finish by making `scripts/lib/test-quarantine.json` and every package Vitest exclude array converge in one commit, then prove the empty/non-empty state with package lanes, `pnpm test:gate`, `pnpm test`, `pnpm build`, and the bounded temp-leak output from `pnpm test`.
**2026-06-15 rescue batch (FN-6486):** two same-day quarantines were rescued before their 2026-06-29 deletion deadline. `store-concurrent-writes.test.ts` kept its WAL/`transactionImmediate` regression value by making the external lock helper's timed release use synchronous `Atomics.wait` inside the child process, removing event-loop timer scheduling as the load-only flake source without widening retry windows. `extension-task-tools.test.ts` kept its worktree-root task-tool coverage by closing each real `TaskStore` fixture before temp-root removal and using non-hoisted mock cleanup. The reusable pattern is to remove scheduler/resource leaks in the helper or fixture seam, then prove the rescue with repeated exact-file runs plus package lanes, not with timeout bumps, retries, assertion loosening, or worker changes.
**Gate eviction:** a flake inside the merge gate cannot block all merges while red — it is evicted by removing its line from the `engine-core` allow-list (no quarantine entry needed unless it should also leave the non-blocking tier).
**Gate admission:** the mirror operation — add the test's path to the `engine-core` `include` array in `packages/engine/vitest.config.ts`, citing the evidence of value (a real regression it caught) in the PR. Keep the project under its ~60s wall-clock budget.

View File

@@ -3,6 +3,9 @@ import { describe, expect, it, vi, beforeEach, afterEach } from "vitest";
FNXC:CliTests 2026-06-14-01:25:
FN-6430 requires rescued CLI suites to run on the default timeout after shared HOME isolation, not via the older file-wide 20s timeout.
Keep this worktree-root regression slice fast by relying on module resets and bounded temp fixtures.
FNXC:CliTests 2026-06-15-07:44:
FN-6486 rescues this load-only timeout by closing each real TaskStore before removing its temp root and by using non-hoisted mock cleanup. The suite keeps the worktree-root regression coverage without widening timeouts, adding retries, or changing package worker settings.
*/
import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
@@ -30,7 +33,7 @@ describe("extension task tools resolve repo root from worktrees", () => {
afterEach(() => {
vi.restoreAllMocks();
vi.unmock("@fusion/core");
vi.doUnmock("@fusion/core");
});
it("exports getProjectRootFromWorktree from @fusion/core", () => {
@@ -40,10 +43,11 @@ describe("extension task tools resolve repo root from worktrees", () => {
it("uses canonical project root for fn_task_show and fn_task_list from worktree cwd", async () => {
const repoRoot = await mkdtemp(join(tmpdir(), "fn-4904-cli-"));
const worktreeRoot = join(repoRoot, ".worktrees", "feature");
let store: TaskStore | undefined;
try {
await mkdir(join(repoRoot, ".fusion"), { recursive: true });
const store = new TaskStore(repoRoot);
store = new TaskStore(repoRoot);
await store.init();
const created = await store.createTask({ description: "Task from canonical root" });
@@ -74,6 +78,7 @@ describe("extension task tools resolve repo root from worktrees", () => {
expect(show.content[0].text).toContain("Task from canonical root");
expect(list.content[0].text).toContain(created.id);
} finally {
store?.close();
await rm(repoRoot, { recursive: true, force: true });
}
});
@@ -81,6 +86,7 @@ describe("extension task tools resolve repo root from worktrees", () => {
it("uses canonical project root for task tools from AI merge temp linked worktrees", async () => {
const repoRoot = await mkdtemp(join(tmpdir(), "fn-6079-cli-"));
const mergeRoot = await mkdtemp(join(tmpdir(), "fusion-ai-merge-fn-6079-"));
let store: TaskStore | undefined;
try {
git(repoRoot, "init -q -b main");
git(repoRoot, "config user.email test@example.com");
@@ -89,7 +95,7 @@ describe("extension task tools resolve repo root from worktrees", () => {
git(repoRoot, "add -A");
git(repoRoot, "commit -q -m base");
const store = new TaskStore(repoRoot);
store = new TaskStore(repoRoot);
await store.init();
const created = await store.createTask({ description: "Task visible from merge worktree" });
git(repoRoot, `worktree add --detach ${JSON.stringify(mergeRoot)} HEAD`);
@@ -116,6 +122,7 @@ describe("extension task tools resolve repo root from worktrees", () => {
expect(show.content[0].text).toContain("Task visible from merge worktree");
expect(list.content[0].text).toContain(created.id);
} finally {
store?.close();
try {
git(repoRoot, `worktree remove --force ${JSON.stringify(mergeRoot)}`);
} catch {
@@ -129,10 +136,11 @@ describe("extension task tools resolve repo root from worktrees", () => {
it("falls back when getProjectRootFromWorktree is unavailable in no-task context", async () => {
const repoRoot = await mkdtemp(join(tmpdir(), "fn-4927-cli-"));
const worktreeRoot = join(repoRoot, ".worktrees", "ambient");
let store: TaskStore | undefined;
try {
await mkdir(join(repoRoot, ".fusion"), { recursive: true });
const store = new TaskStore(repoRoot);
store = new TaskStore(repoRoot);
await store.init();
const created = await store.createTask({ description: "Ambient tool check" });
@@ -169,6 +177,7 @@ describe("extension task tools resolve repo root from worktrees", () => {
expect(show.content[0]?.text).toContain(created.id);
expect(warnSpy).toHaveBeenCalledTimes(1);
} finally {
store?.close();
await rm(repoRoot, { recursive: true, force: true });
}
});

View File

@@ -21,8 +21,10 @@ const quarantinedCliTests: string[] = [
FNXC:CliTests 2026-06-15-04:07:
FN-6483 observed extension-task-tools timing out only under the full @runfusion/fusion package lane while passing standalone immediately afterward.
Quarantine the suite for the 14-day deletion ratchet instead of appeasing the load-sensitive timeout with wider test timeouts, retries, or worker changes.
FNXC:CliTests 2026-06-15-07:46:
FN-6486 rescued extension-task-tools by closing real TaskStore fixtures and replacing hoisted mock cleanup, then removed the quarantine in lockstep with scripts/lib/test-quarantine.json. Keep this array empty unless a future observed CLI flake is mirrored in the ledger in the same commit.
*/
"src/__tests__/extension-task-tools.test.ts",
];
export default defineConfig({

View File

@@ -36,7 +36,13 @@ async function holdWriteLock(
process.exit(0);
};
if (${JSON.stringify(releaseMode)} === "timer") {
setTimeout(release, ${holdMs});
/*
FNXC:CoreTests 2026-06-15-07:38:
FN-6486 rescues this WAL lock-recovery regression by removing the helper's event-loop timer dependency. Under package-lane load, a delayed setTimeout could keep the external writer lock past the recovery window and mimic a product failure; a synchronous child-process sleep preserves the transient lock invariant without widening test or SQLite retry timeouts.
*/
const signal = new Int32Array(new SharedArrayBuffer(4));
Atomics.wait(signal, 0, 0, ${holdMs});
release();
} else {
process.stdin.setEncoding("utf8");
process.stdin.on("data", (chunk) => {

View File

@@ -14,8 +14,10 @@ const quarantinedCoreTests = [
FNXC:CoreTests 2026-06-15-03:13:
FN-6481 observed the disk-backed concurrent write test fail in the changed-package workspace lane with a transient SQLite BEGIN IMMEDIATE lock after the gate had already passed. Quarantine the flaky file instead of widening lock-recovery timeouts or weakening assertions.
FNXC:CoreTests 2026-06-15-07:39:
FN-6486 rescued store-concurrent-writes by making the transient lock helper release independent of event-loop timer scheduling, then removed the quarantine in lockstep with scripts/lib/test-quarantine.json. Keep this array empty unless a future observed flake is mirrored in the ledger in the same commit.
*/
"src/__tests__/store-concurrent-writes.test.ts",
];
export default defineConfig({

View File

@@ -1,15 +1,4 @@
{
"$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/core/src/__tests__/store-concurrent-writes.test.ts",
"reason": "FN-6481 pnpm test 2026-06-15 failed unrelated core SQLite lock recovery concurrency test with `SQLite BEGIN IMMEDIATE failed after 7 attempts: database is locked`; quarantine on sight per flaky-test policy.",
"quarantinedAt": "2026-06-15"
},
{
"file": "packages/cli/src/__tests__/extension-task-tools.test.ts",
"reason": "FN-6483: @runfusion/fusion package lane under load timed out after 5000ms in uses canonical project root for fn_task_show and fn_task_list from worktree cwd during the 2026-06-15 verification rerun, matching the FN-6482 reported load-only CLI timeout signature. The same file passed standalone immediately afterward (4 tests passed in 5.90s), so quarantine per the deletion ratchet rather than widening timeouts or changing worker knobs.",
"quarantinedAt": "2026-06-15"
}
]
"entries": []
}