fix(FN-5994): address verification review feedback
Harden command normalization edge cases and abort loop compaction on timeout before falling through to stuck-task requeue. Fusion-Task-Id: FN-5994
This commit is contained in:
@@ -3457,10 +3457,12 @@ describe("TaskExecutor loop recovery", () => {
|
||||
const compactRetVal = overrides && "compactResult" in overrides ? overrides.compactResult : defaultResult;
|
||||
const compact = vi.fn(async () => compactRetVal);
|
||||
const steer = vi.fn(async () => {});
|
||||
const abort = vi.fn(async () => {});
|
||||
|
||||
return {
|
||||
prompt: vi.fn(async () => {}),
|
||||
dispose: vi.fn(),
|
||||
abort,
|
||||
subscribe: vi.fn(),
|
||||
setThinkingLevel: vi.fn(),
|
||||
steer,
|
||||
@@ -3598,6 +3600,7 @@ describe("TaskExecutor loop recovery", () => {
|
||||
await vi.advanceTimersByTimeAsync(60000);
|
||||
|
||||
await expect(resultPromise).resolves.toBe(false);
|
||||
expect(mockSession.abort).toHaveBeenCalled();
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
expect.stringContaining("Context compaction timed out"),
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { runVerificationCommand, normalizeVerificationCommand, type RunVerificationOptions } from "../run-verification-tool.js";
|
||||
|
||||
// Some tests use platform-appropriate shell syntax. On Windows, sh-style
|
||||
@@ -24,7 +24,7 @@ const itPosix = onPosix ? it : it.skip;
|
||||
// so we fall back to os.tmpdir() which is always C:\Users\…\Temp there.
|
||||
describe("runVerificationCommand", { timeout: 30000 }, () => {
|
||||
const tempDir = onPosix ? "/tmp" : tmpdir();
|
||||
const workspaceRoot = join(process.cwd(), "../..");
|
||||
const workspaceRoot = fileURLToPath(new URL("../../../../", import.meta.url));
|
||||
|
||||
describe("command normalization", () => {
|
||||
it("rewrites package test -- --run filters to direct vitest with package-relative files", () => {
|
||||
@@ -54,6 +54,33 @@ describe("runVerificationCommand", { timeout: 30000 }, () => {
|
||||
const command = "pnpm --filter @fusion/dashboard test";
|
||||
expect(normalizeVerificationCommand(command, workspaceRoot)).toEqual({ command, warnings: [] });
|
||||
});
|
||||
|
||||
it("leaves commands with unterminated shell quotes unchanged", () => {
|
||||
const command = "pnpm --filter @fusion/dashboard test -- --run 'src/__tests__/routes-tasks.test.ts";
|
||||
expect(normalizeVerificationCommand(command, workspaceRoot)).toEqual({ command, warnings: [] });
|
||||
});
|
||||
|
||||
it("preserves pnpm global flags that precede --filter", () => {
|
||||
const result = normalizeVerificationCommand(
|
||||
"pnpm -w --filter @fusion/dashboard test -- --run packages/dashboard/src/__tests__/routes-tasks.test.ts",
|
||||
workspaceRoot,
|
||||
);
|
||||
|
||||
expect(result.command).toBe(
|
||||
"pnpm -w --filter @fusion/dashboard exec vitest run src/__tests__/routes-tasks.test.ts --silent=passed-only --reporter=dot",
|
||||
);
|
||||
});
|
||||
|
||||
it("verifies the CLI package directory through package.json before rewriting", () => {
|
||||
const result = normalizeVerificationCommand(
|
||||
"pnpm --filter @runfusion/fusion test -- --run packages/cli/src/__tests__/cli.test.ts",
|
||||
workspaceRoot,
|
||||
);
|
||||
|
||||
expect(result.command).toBe(
|
||||
"pnpm --filter @runfusion/fusion exec vitest run src/__tests__/cli.test.ts --silent=passed-only --reporter=dot",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("basic command execution", () => {
|
||||
|
||||
@@ -13421,6 +13421,14 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
|
||||
let compactionTimedOut = false;
|
||||
let compactionTimer: ReturnType<typeof setTimeout> | undefined;
|
||||
const abortActiveSession = () => {
|
||||
const sessionWithAbort = activeEntry.session as unknown as { abort?: () => Promise<void> };
|
||||
if (typeof sessionWithAbort.abort === "function") {
|
||||
void sessionWithAbort.abort().catch((err: unknown) => {
|
||||
executorLog.warn(`${taskId} loop compaction abort after timeout failed: ${err instanceof Error ? err.message : String(err)}`);
|
||||
});
|
||||
}
|
||||
};
|
||||
let compactResult: Awaited<ReturnType<typeof compactSessionContext>> | null;
|
||||
try {
|
||||
compactResult = await Promise.race([
|
||||
@@ -13428,6 +13436,7 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
new Promise<null>((resolve) => {
|
||||
compactionTimer = setTimeout(() => {
|
||||
compactionTimedOut = true;
|
||||
abortActiveSession();
|
||||
resolve(null);
|
||||
}, LOOP_COMPACTION_TIMEOUT_MS);
|
||||
}),
|
||||
|
||||
@@ -36,7 +36,9 @@ const DEFAULT_TIMEOUT_PACKAGE_SEC = 300;
|
||||
const DEFAULT_TIMEOUT_WORKSPACE_SEC = 900;
|
||||
const MAX_TIMEOUT_SEC = 1800;
|
||||
|
||||
function shellSplit(input: string): string[] {
|
||||
const packageDirCache = new Map<string, string | null>();
|
||||
|
||||
function shellSplit(input: string): string[] | null {
|
||||
const tokens: string[] = [];
|
||||
let current = "";
|
||||
let quote: "'" | "\"" | null = null;
|
||||
@@ -75,6 +77,7 @@ function shellSplit(input: string): string[] {
|
||||
}
|
||||
|
||||
if (escaped) current += "\\";
|
||||
if (quote !== null) return null;
|
||||
if (current.length > 0) tokens.push(current);
|
||||
return tokens;
|
||||
}
|
||||
@@ -85,7 +88,8 @@ function shellQuote(value: string): string {
|
||||
}
|
||||
|
||||
function findWorkspacePackageDir(rootDir: string, packageName: string): string | null {
|
||||
if (packageName === "@runfusion/fusion") return "packages/cli";
|
||||
const cacheKey = `${rootDir}\0${packageName}`;
|
||||
if (packageDirCache.has(cacheKey)) return packageDirCache.get(cacheKey) ?? null;
|
||||
|
||||
const lastSegment = packageName.split("/").pop();
|
||||
const candidates = [
|
||||
@@ -97,7 +101,10 @@ function findWorkspacePackageDir(rootDir: string, packageName: string): string |
|
||||
if (!candidate) continue;
|
||||
try {
|
||||
const pkg = JSON.parse(readFileSync(join(rootDir, candidate, "package.json"), "utf8")) as { name?: string };
|
||||
if (pkg.name === packageName) return candidate;
|
||||
if (pkg.name === packageName) {
|
||||
packageDirCache.set(cacheKey, candidate);
|
||||
return candidate;
|
||||
}
|
||||
} catch {
|
||||
// Keep looking.
|
||||
}
|
||||
@@ -123,13 +130,17 @@ function findWorkspacePackageDir(rootDir: string, packageName: string): string |
|
||||
const child = join(current.dir, entry.name);
|
||||
try {
|
||||
const pkg = JSON.parse(readFileSync(join(rootDir, child, "package.json"), "utf8")) as { name?: string };
|
||||
if (pkg.name === packageName) return child;
|
||||
if (pkg.name === packageName) {
|
||||
packageDirCache.set(cacheKey, child);
|
||||
return child;
|
||||
}
|
||||
} catch {
|
||||
if (current.depth < 3) queue.push({ dir: child, depth: current.depth + 1 });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
packageDirCache.set(cacheKey, null);
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -152,6 +163,7 @@ function toPackageRelativeFilter(token: string, rootDir: string, packageDir: str
|
||||
export function normalizeVerificationCommand(command: string, rootDir: string): { command: string; warnings: string[] } {
|
||||
const tokens = shellSplit(command);
|
||||
const warnings: string[] = [];
|
||||
if (!tokens) return { command, warnings };
|
||||
if (tokens[0] !== "pnpm") return { command, warnings };
|
||||
|
||||
const filterIndex = tokens.findIndex((token) => token === "--filter" || token === "-F");
|
||||
@@ -182,8 +194,10 @@ export function normalizeVerificationCommand(command: string, rootDir: string):
|
||||
|
||||
const hasReporter = vitestArgs.some((token) => token === "--reporter" || token.startsWith("--reporter="));
|
||||
const hasSilent = vitestArgs.some((token) => token === "--silent" || token.startsWith("--silent="));
|
||||
const pnpmGlobalFlags = tokens.slice(1, filterIndex);
|
||||
const normalizedTokens = [
|
||||
"pnpm",
|
||||
...pnpmGlobalFlags,
|
||||
"--filter",
|
||||
packageName,
|
||||
"exec",
|
||||
|
||||
Reference in New Issue
Block a user