FN-6860: rescue dashboard quarantine coverage
Rescue dashboard process lifecycle coverage and clear stale quarantine state. - Track dev server lifecycle work so stale probes and output parsing cannot race cleanup. - Assert duplicate URL detection suppression and fallback probe cleanup across failure, restart, stop, and cleanup paths. - Remove rescued dashboard tests from quarantine config and ledger, and document the loaded-shard proof. Files changed: docs/testing.md | 4 ++ .../src/__tests__/dev-server-process.test.ts | 28 +++++++- packages/dashboard/src/dev-server-process.ts | 74 +++++++++++++++++----- packages/dashboard/vitest.config.ts | 5 +- scripts/lib/test-quarantine.json | 13 +--- 5 files changed, 95 insertions(+), 29 deletions(-) Fusion-Task-Id: FN-6860 Fusion-Task-Lineage: abd22a00-e08e-4f65-bebc-c663c715b40e
This commit is contained in:
@@ -228,6 +228,10 @@ FNXC:CoreTests 2026-06-19-15:05: Merge verification re-observed store-concurrent
|
||||
|
||||
**2026-06-19 dashboard session-cross-tab rescue (FN-6742):** `packages/dashboard/src/__tests__/session-cross-tab.test.ts` was rescued before its 2026-07-03 deletion deadline. The loaded `dashboard-api-quality-backfill` shard reproduced the original `fusion-test-workers-*` `ENOTEMPTY` cleanup failure with the quarantine exclude temporarily removed, while the test's assertions retained value by failing when the expected lock holder was mutated from `tab-a` to `tab-z`. The fix keeps the test unquarantined by disposing the created API router, stopping `AiSessionStore` scheduled cleanup, closing the real `TaskStore`/SQLite handles, hiding route EventEmitter hooks not used by this harness, and draining four bounded check-phase turns before deleting the temp root. The ledger and `packages/dashboard/vitest.config.ts` exclude were updated in lockstep; later loaded runs no longer failed this file, and unrelated dashboard loaded-suite failures are tracked separately rather than weakening this test.
|
||||
|
||||
<!-- FNXC:DashboardTests 2026-06-21-12:55: FN-6860 found dashboard quarantine ledger/config drift after earlier rescues: session-cross-tab was still ledger-only, while dev-server-process remained excluded. Treat dashboard rescue closure as a loaded-shard proof plus same-commit ledger/config convergence; stale ledger-only entries should be removed after loaded proof, not re-quarantined. -->
|
||||
|
||||
**2026-06-21 dashboard quarantine lockstep cleanup (FN-6860):** `packages/dashboard/src/__tests__/dev-server-process.test.ts` and `packages/dashboard/src/__tests__/session-cross-tab.test.ts` were cleared from the deletion ratchet after repeated `dashboard-api-quality-backfill` loaded-shard runs passed with the excludes removed. `dev-server-process` kept its process-lifecycle regression value by tracking lifecycle generations, disposed state, active stdout/stderr line work, and fallback probe work before close/failure cleanup resolves; its tests now assert duplicate URL detection is suppressed and probe timers are cleared on failure/restart/cleanup. `session-cross-tab` needed no code change in this batch because it was already active in Vitest config; the stale ledger-only entry was removed after loaded-shard proof. Closure evidence is the grouped rescued-file lane, two full `test:quality:api:backfill` runs, ledger/config empty-state convergence, lint, typecheck, gate, and build, with no timeout/retry/worker appeasement.
|
||||
|
||||
<!-- FNXC:WorkflowNodeEditorTests 2026-06-19-18:24: FN-6744 proved WorkflowNodeEditor duplicate-merge coverage still catches a real product race: the palette can be used after workflow IR loads but before React Flow nodes exist. Rescue this class by checking seam conflicts against the authoritative loaded IR during initial canvas materialization, then prove desktop and mobile conflict surfaces under the loaded dashboard components-b lane; do not add waits, retries, worker reductions, or timeout appeasement. -->
|
||||
|
||||
**2026-06-19 dashboard WorkflowNodeEditor rescue (FN-6744):** `packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx` was rescued before its 2026-07-03 deletion deadline. The original duplicate-merge test passed in isolation but was load-sensitive because `handleInsertFragment` derived existing seams only from transient React Flow nodes; a fast palette click could arrive after `activeWorkflow.ir` loaded but before the canvas nodes materialized, allowing an invalid duplicate merge seam instead of showing the conflict alert. The fix keeps the test unquarantined by treating IR merge nodes as the merge seam and by unioning seams from the loaded IR only during initial canvas materialization, preserving post-load canvas-state semantics. Regression coverage now exercises both desktop and mobile fragment insertion surfaces and asserts the conflict affordance appears without growing the rendered graph. The ledger and `packages/dashboard/vitest.config.ts` exclude were removed in lockstep; targeted file runs, repeated `test:quality:app:components-b`, lint, gate, typecheck, and build are the closure evidence. A broader `@fusion/dashboard test` run currently fails unrelated Command Center ProductivityArea mock drift tracked by FN-6754, so do not re-quarantine WorkflowNodeEditor for that lane.
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
import { mkdtempSync, readFileSync, rmSync } from "node:fs";
|
||||
import os from "node:os";
|
||||
import { join } from "node:path";
|
||||
import type { ChildProcess } from "node:child_process";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { DevServerProcessManager } from "../dev-server-process.js";
|
||||
import { loadDevServerStore, resetDevServerStore } from "../dev-server-store.js";
|
||||
@@ -26,6 +27,11 @@ function isProcessAlive(pid: number): boolean {
|
||||
}
|
||||
}
|
||||
|
||||
type DevServerProcessManagerInternals = {
|
||||
childProcess: ChildProcess | null;
|
||||
handleFailure(error: Error): Promise<void>;
|
||||
};
|
||||
|
||||
describe("DevServerProcessManager", () => {
|
||||
const tempDirs: string[] = [];
|
||||
const managers: DevServerProcessManager[] = [];
|
||||
@@ -219,14 +225,17 @@ describe("DevServerProcessManager", () => {
|
||||
|
||||
it("clears fallback probe timer when URL is detected from logs", async () => {
|
||||
const { root, store, manager } = await createManager({ probeDelayMs: 2_000, probeTimeoutMs: 5 });
|
||||
const detectedEvents: unknown[] = [];
|
||||
manager.on("url-detected", (payload) => detectedEvents.push(payload));
|
||||
|
||||
await manager.start(
|
||||
"node -e \"console.log('ready at http://localhost:4321');process.stdin.resume();process.stdin.on('end',()=>process.exit(0))\"",
|
||||
"node -e \"console.log('ready at http://localhost:4321');console.log('ready again at http://localhost:4321');process.stdin.resume();process.stdin.on('end',()=>process.exit(0))\"",
|
||||
root,
|
||||
);
|
||||
|
||||
await waitFor(() => store.getState().detectedPort === 4321);
|
||||
expect(manager.hasPendingProbeTimer()).toBe(false);
|
||||
expect(detectedEvents).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("clears fallback probe timer on stop", async () => {
|
||||
@@ -251,6 +260,20 @@ describe("DevServerProcessManager", () => {
|
||||
expect(manager.hasPendingProbeTimer()).toBe(false);
|
||||
});
|
||||
|
||||
it("clears fallback probe timer when the child process reports failure", async () => {
|
||||
const { root, store, manager } = await createManager({ probeDelayMs: 2_000, probeTimeoutMs: 5 });
|
||||
|
||||
await manager.start("node -e \"setTimeout(() => process.exit(0), 50)\"", root);
|
||||
expect(manager.hasPendingProbeTimer()).toBe(true);
|
||||
|
||||
const internals = manager as unknown as DevServerProcessManagerInternals;
|
||||
internals.childProcess?.emit("error", new Error("synthetic process failure"));
|
||||
|
||||
await waitFor(() => store.getState().status === "failed");
|
||||
|
||||
expect(manager.hasPendingProbeTimer()).toBe(false);
|
||||
});
|
||||
|
||||
it("restarts with a fresh fallback probe timer", async () => {
|
||||
const { root, manager } = await createManager({ probeDelayMs: 2_000, probeTimeoutMs: 5 });
|
||||
|
||||
@@ -260,6 +283,8 @@ describe("DevServerProcessManager", () => {
|
||||
await manager.restart();
|
||||
|
||||
expect(manager.hasPendingProbeTimer()).toBe(true);
|
||||
await manager.stop();
|
||||
expect(manager.hasPendingProbeTimer()).toBe(false);
|
||||
});
|
||||
|
||||
it("cleanup() kills process and clears listeners", async () => {
|
||||
@@ -272,6 +297,7 @@ describe("DevServerProcessManager", () => {
|
||||
manager.cleanup();
|
||||
await waitFor(() => manager.isRunning() === false);
|
||||
|
||||
expect(manager.hasPendingProbeTimer()).toBe(false);
|
||||
expect(manager.listenerCount("output")).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -83,6 +83,9 @@ export class DevServerProcessManager extends EventEmitter {
|
||||
private hasDetectedUrl = false;
|
||||
private closePromise: Promise<DevServerState> | null = null;
|
||||
private resolveClosePromise: ((state: DevServerState) => void) | null = null;
|
||||
private lifecycleId = 0;
|
||||
private isDisposed = false;
|
||||
private readonly activeLifecycleWork = new Set<Promise<void>>();
|
||||
|
||||
private readonly stopTimeoutMs: number;
|
||||
private readonly probeDelayMs: number;
|
||||
@@ -118,6 +121,9 @@ export class DevServerProcessManager extends EventEmitter {
|
||||
throw new Error("cwd is required");
|
||||
}
|
||||
|
||||
this.lifecycleId += 1;
|
||||
this.isDisposed = false;
|
||||
const lifecycleId = this.lifecycleId;
|
||||
this.hasDetectedUrl = false;
|
||||
await this.store.updateState({
|
||||
status: "starting",
|
||||
@@ -157,14 +163,17 @@ export class DevServerProcessManager extends EventEmitter {
|
||||
|
||||
const handleLine = async (line: string, stream: "stdout" | "stderr"): Promise<void> => {
|
||||
const trimmed = line.replace(/\r$/, "");
|
||||
if (!trimmed) {
|
||||
if (!trimmed || !this.isCurrentLifecycle(lifecycleId)) {
|
||||
return;
|
||||
}
|
||||
|
||||
await this.store.appendLog(trimmed);
|
||||
if (!this.isCurrentLifecycle(lifecycleId)) {
|
||||
return;
|
||||
}
|
||||
const payload = { line: trimmed, stream, timestamp: new Date().toISOString() };
|
||||
this.emit("output", payload);
|
||||
void this.handleDetectionFromLine(trimmed);
|
||||
await this.handleDetectionFromLine(trimmed, lifecycleId);
|
||||
};
|
||||
|
||||
this.attachOutput(child.stdout, "stdout", handleLine);
|
||||
@@ -187,7 +196,7 @@ export class DevServerProcessManager extends EventEmitter {
|
||||
});
|
||||
|
||||
this.portProbeTimer = setTimeout(() => {
|
||||
void this.runFallbackProbe();
|
||||
this.trackLifecycleWork(this.runFallbackProbe(lifecycleId));
|
||||
}, this.probeDelayMs);
|
||||
|
||||
return runningState;
|
||||
@@ -245,6 +254,8 @@ export class DevServerProcessManager extends EventEmitter {
|
||||
}
|
||||
|
||||
cleanup(): void {
|
||||
this.lifecycleId += 1;
|
||||
this.isDisposed = true;
|
||||
this.clearTimers();
|
||||
|
||||
if (this.childProcess && typeof this.childProcess.pid === "number") {
|
||||
@@ -274,7 +285,7 @@ export class DevServerProcessManager extends EventEmitter {
|
||||
pending = lines.pop() ?? "";
|
||||
|
||||
for (const line of lines) {
|
||||
void onLine(line, source);
|
||||
this.trackLifecycleWork(onLine(line, source));
|
||||
}
|
||||
});
|
||||
|
||||
@@ -282,7 +293,7 @@ export class DevServerProcessManager extends EventEmitter {
|
||||
if (pending.length > 0) {
|
||||
const line = pending;
|
||||
pending = "";
|
||||
void onLine(line, source);
|
||||
this.trackLifecycleWork(onLine(line, source));
|
||||
}
|
||||
};
|
||||
|
||||
@@ -290,8 +301,8 @@ export class DevServerProcessManager extends EventEmitter {
|
||||
stream.on("close", flushPending);
|
||||
}
|
||||
|
||||
private async handleDetectionFromLine(line: string): Promise<void> {
|
||||
if (this.hasDetectedUrl) {
|
||||
private async handleDetectionFromLine(line: string, lifecycleId = this.lifecycleId): Promise<void> {
|
||||
if (this.hasDetectedUrl || !this.isCurrentLifecycle(lifecycleId)) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -300,26 +311,28 @@ export class DevServerProcessManager extends EventEmitter {
|
||||
return;
|
||||
}
|
||||
|
||||
await this.persistDetection(detected);
|
||||
await this.persistDetection(detected, lifecycleId);
|
||||
}
|
||||
|
||||
private async runFallbackProbe(): Promise<void> {
|
||||
this.portProbeTimer = null;
|
||||
private async runFallbackProbe(lifecycleId = this.lifecycleId): Promise<void> {
|
||||
if (this.isCurrentLifecycle(lifecycleId)) {
|
||||
this.portProbeTimer = null;
|
||||
}
|
||||
|
||||
if (this.hasDetectedUrl || !this.isRunning()) {
|
||||
if (this.hasDetectedUrl || !this.isRunning() || !this.isCurrentLifecycle(lifecycleId)) {
|
||||
return;
|
||||
}
|
||||
|
||||
const detected = await probeFallbackPorts(DEFAULT_PROBE_HOST, this.probeTimeoutMs);
|
||||
if (!detected || this.hasDetectedUrl || !this.isRunning()) {
|
||||
if (!detected || this.hasDetectedUrl || !this.isRunning() || !this.isCurrentLifecycle(lifecycleId)) {
|
||||
return;
|
||||
}
|
||||
|
||||
await this.persistDetection(detected);
|
||||
await this.persistDetection(detected, lifecycleId);
|
||||
}
|
||||
|
||||
private async persistDetection(detected: PortDetectionResult): Promise<void> {
|
||||
if (this.hasDetectedUrl) {
|
||||
private async persistDetection(detected: PortDetectionResult, lifecycleId = this.lifecycleId): Promise<void> {
|
||||
if (this.hasDetectedUrl || !this.isCurrentLifecycle(lifecycleId)) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -333,6 +346,9 @@ export class DevServerProcessManager extends EventEmitter {
|
||||
detectedUrl: detected.url,
|
||||
detectedPort: detected.port,
|
||||
});
|
||||
if (!this.isCurrentLifecycle(lifecycleId)) {
|
||||
return;
|
||||
}
|
||||
|
||||
const payload: UrlDetectedEventPayload = {
|
||||
url: updated.detectedUrl ?? detected.url,
|
||||
@@ -348,6 +364,7 @@ export class DevServerProcessManager extends EventEmitter {
|
||||
|
||||
private async handleClose(code: number): Promise<void> {
|
||||
this.clearTimers();
|
||||
await this.waitForActiveLifecycleWork();
|
||||
|
||||
const updated = await this.store.updateState({
|
||||
status: "stopped",
|
||||
@@ -365,6 +382,7 @@ export class DevServerProcessManager extends EventEmitter {
|
||||
|
||||
private async handleFailure(error: Error): Promise<void> {
|
||||
this.clearTimers();
|
||||
await this.waitForActiveLifecycleWork();
|
||||
|
||||
const updated = await this.store.updateState({
|
||||
status: "failed",
|
||||
@@ -379,6 +397,32 @@ export class DevServerProcessManager extends EventEmitter {
|
||||
this.emit("failed", { error: error.message });
|
||||
}
|
||||
|
||||
private isCurrentLifecycle(lifecycleId: number): boolean {
|
||||
return !this.isDisposed && lifecycleId === this.lifecycleId;
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:DevServerProcess 2026-06-21-12:35:
|
||||
Loaded dashboard API shards can close a child process while stdout parsing, URL persistence, or fallback probing is still settling. Track lifecycle work and invalidate stale callbacks so every stop, close, failure, restart, and cleanup path clears the probe timer without leaving late store writes or process-handle work racing test fixture removal.
|
||||
*/
|
||||
private trackLifecycleWork(promise: Promise<void>): void {
|
||||
this.activeLifecycleWork.add(promise);
|
||||
void promise.then(
|
||||
() => {
|
||||
this.activeLifecycleWork.delete(promise);
|
||||
},
|
||||
() => {
|
||||
this.activeLifecycleWork.delete(promise);
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
private async waitForActiveLifecycleWork(): Promise<void> {
|
||||
while (this.activeLifecycleWork.size > 0) {
|
||||
await Promise.allSettled([...this.activeLifecycleWork]);
|
||||
}
|
||||
}
|
||||
|
||||
private clearProbeTimer(): void {
|
||||
if (this.portProbeTimer) {
|
||||
clearTimeout(this.portProbeTimer);
|
||||
|
||||
@@ -293,8 +293,11 @@ Keep QuickEntryBox out of this list so the dashboard app lanes exercise Enter, S
|
||||
|
||||
FNXC:DashboardTestQuarantine 2026-06-21-06:50:
|
||||
FN-6722 workspace verification observed dev-server-process time out only in the broad dashboard API backfill shard while the isolated file passed immediately. Quarantine the process/timer race under the deletion ratchet instead of widening waits or changing unrelated Command Center behavior.
|
||||
|
||||
FNXC:DashboardTestQuarantine 2026-06-21-12:42:
|
||||
FN-6860 rescued dev-server-process by settling stdout detection and fallback-probe lifecycle work before stop/close/failure teardown, then removed its ledger/config quarantine entry. The same loaded API shard also confirmed FN-6742's session-cross-tab rescue still holds, so its stale ledger-only entry was removed to restore lockstep.
|
||||
*/
|
||||
const quarantinedDashboardTests: string[] = ["src/__tests__/dev-server-process.test.ts"];
|
||||
const quarantinedDashboardTests: string[] = [];
|
||||
|
||||
const qualityApiTests = [
|
||||
// Critical HTTP/server behavior: auth, task/project/settings mutation,
|
||||
|
||||
@@ -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/dashboard/src/__tests__/dev-server-process.test.ts",
|
||||
"reason": "FN-6722 broad workspace `pnpm test` observed `clears fallback probe timer when URL is detected from logs` time out only in the dashboard-api-quality-backfill shard; isolated rerun passed, so quarantine the timer/process race on sight instead of widening waits or changing unrelated Command Center behavior.",
|
||||
"quarantinedAt": "2026-06-21"
|
||||
},
|
||||
{
|
||||
"file": "packages/dashboard/src/__tests__/session-cross-tab.test.ts",
|
||||
"reason": "FN-6690 local workspace `pnpm test` observed ENOTEMPTY while removing the test's temp .fusion directory in dashboard-api-quality-backfill shard; isolated rerun passed, indicating cleanup flake rather than a lazy-view CSS regression.",
|
||||
"quarantinedAt": "2026-06-19"
|
||||
}
|
||||
]
|
||||
"entries": []
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user