fix(cli): await browser scenario cleanup on interrupts
Translate browser scenario CLI interrupts into cancellation and await owned provider cleanup. Preserve authenticated observations and cancelled steps with conventional exit codes 130/143. Refs #1771.
This commit is contained in:
Ysopwnable
GitHub
parent
0bdc7a40ac
commit
aaeadf2d4c
|
|
@ -2,6 +2,7 @@ import { Cli } from "incur";
|
|||
import { z } from "zod";
|
||||
|
||||
import { captureBrowserScenario } from "./application/BrowserScenarioCaptureService.js";
|
||||
import { withCommandCancellation } from "./cli/commandCancellation.js";
|
||||
import { CLI_COMMANDS } from "./cliCommandNames.js";
|
||||
import { parseCliJsonInput, resolveCliJsonPaths } from "./cliJsonInput.js";
|
||||
import { logCliCommand } from "./cliLogging.js";
|
||||
|
|
@ -31,58 +32,63 @@ export const registerBrowserScenarioCommands = (
|
|||
.describe("Inline browser scenario JSON or JSON file path"),
|
||||
}),
|
||||
run: ({ args }) =>
|
||||
logCliCommand(logger, CLI_COMMANDS.captureBrowserScenario, async () => {
|
||||
const input = await parseCliJsonInput(args.inputJson, OPERATION);
|
||||
if (!input.ok) return input.error;
|
||||
const scenario = browserScenarioSchema.safeParse(
|
||||
resolveCliJsonPaths(input.value, [["browser", "executable_path"]]),
|
||||
);
|
||||
if (!scenario.success)
|
||||
return cliError(
|
||||
new AnalysisInputError(
|
||||
OPERATION,
|
||||
{ cause: scenario.error },
|
||||
projectInputIssues(scenario.error.issues, input.value),
|
||||
),
|
||||
withCommandCancellation((signal) =>
|
||||
logCliCommand(logger, CLI_COMMANDS.captureBrowserScenario, async () => {
|
||||
const input = await parseCliJsonInput(args.inputJson, OPERATION);
|
||||
if (!input.ok) return input.error;
|
||||
const scenario = browserScenarioSchema.safeParse(
|
||||
resolveCliJsonPaths(input.value, [["browser", "executable_path"]]),
|
||||
);
|
||||
// Load Playwright only for this command, not during CLI startup.
|
||||
const { createBrowserScenarioProvider } =
|
||||
await import("./composition/browserScenario.js");
|
||||
const provider = createBrowserScenarioProvider(environment);
|
||||
let result:
|
||||
| Awaited<ReturnType<typeof captureBrowserScenario>>
|
||||
| undefined;
|
||||
try {
|
||||
result = await captureBrowserScenario(provider, scenario.data);
|
||||
return result.ok ? result.value : cliError(result.error);
|
||||
} finally {
|
||||
if (!scenario.success)
|
||||
return cliError(
|
||||
new AnalysisInputError(
|
||||
OPERATION,
|
||||
{ cause: scenario.error },
|
||||
projectInputIssues(scenario.error.issues, input.value),
|
||||
),
|
||||
);
|
||||
// Load Playwright only for this command, not during CLI startup.
|
||||
const { createBrowserScenarioProvider } =
|
||||
await import("./composition/browserScenario.js");
|
||||
const provider = createBrowserScenarioProvider(environment);
|
||||
let result:
|
||||
| Awaited<ReturnType<typeof captureBrowserScenario>>
|
||||
| undefined;
|
||||
try {
|
||||
await provider?.close?.();
|
||||
} catch (cause: unknown) {
|
||||
// Failed captures already retain their primary error and partial
|
||||
// observations. A cleanup retry must not replace that outcome.
|
||||
if (result?.ok)
|
||||
throw new BrowserObservationError(OPERATION, "cleanup_failed", {
|
||||
cause,
|
||||
cleanup:
|
||||
cause instanceof AnalysisError && cause.cleanup !== undefined
|
||||
? cause.cleanup
|
||||
: {
|
||||
reason:
|
||||
cause instanceof Error
|
||||
? cause.message
|
||||
: String(cause),
|
||||
resources:
|
||||
cause instanceof AnalysisError &&
|
||||
cause.cleanupResources.length > 0
|
||||
? cause.cleanupResources
|
||||
: ["browser_transport"],
|
||||
},
|
||||
partialObservation: result.value,
|
||||
});
|
||||
result = await captureBrowserScenario(provider, scenario.data, {
|
||||
signal,
|
||||
});
|
||||
return result.ok ? result.value : cliError(result.error);
|
||||
} finally {
|
||||
try {
|
||||
await provider?.close?.();
|
||||
} catch (cause: unknown) {
|
||||
// Failed captures already retain their primary error and partial
|
||||
// observations. A cleanup retry must not replace that outcome.
|
||||
if (result?.ok)
|
||||
throw new BrowserObservationError(OPERATION, "cleanup_failed", {
|
||||
cause,
|
||||
cleanup:
|
||||
cause instanceof AnalysisError &&
|
||||
cause.cleanup !== undefined
|
||||
? cause.cleanup
|
||||
: {
|
||||
reason:
|
||||
cause instanceof Error
|
||||
? cause.message
|
||||
: String(cause),
|
||||
resources:
|
||||
cause instanceof AnalysisError &&
|
||||
cause.cleanupResources.length > 0
|
||||
? cause.cleanupResources
|
||||
: ["browser_transport"],
|
||||
},
|
||||
partialObservation: result.value,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}),
|
||||
}),
|
||||
),
|
||||
});
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,84 @@
|
|||
import { resolve } from "node:path";
|
||||
import { expect } from "vitest";
|
||||
|
||||
import { browserScenarioCaptureSchema } from "../../../src/domain/browserScenarioCapture.js";
|
||||
import { parseEvidence } from "../../../src/domain/evidence.js";
|
||||
import { processTest } from "../../support/process/processFixture.js";
|
||||
|
||||
// Actual CLI subprocess and OS signals with a controlled browser session.
|
||||
// This verifies adapter wiring and awaited cleanup, without launching Chromium.
|
||||
processTest
|
||||
.skipIf(process.platform === "win32")
|
||||
.for(["SIGINT", "SIGTERM"] as const)(
|
||||
"awaits browser scenario cleanup and retains observations after %s",
|
||||
async (signal, { processes }) => {
|
||||
const child = processes.spawn(
|
||||
process.execPath,
|
||||
[
|
||||
"--import",
|
||||
resolve("tests/fixtures/browserScenarioCancellation.mjs"),
|
||||
resolve("scripts/rea.mjs"),
|
||||
"capture-browser-scenario",
|
||||
JSON.stringify({
|
||||
browser: { mode: "launch", executable_path: "/controlled/chromium" },
|
||||
start_url: { url: "https://app.example.test/" },
|
||||
actions: [
|
||||
{
|
||||
step_id: "wait",
|
||||
action: "wait_for_timeout",
|
||||
duration_ms: 60_000,
|
||||
},
|
||||
],
|
||||
storage: {},
|
||||
capture: { after_each_step: ["url"], at_end: [], events: [] },
|
||||
}),
|
||||
"--json",
|
||||
],
|
||||
{ env: { PATH: process.env.PATH ?? "" } },
|
||||
);
|
||||
child.stdin.end();
|
||||
let stdout = "",
|
||||
stderr = "",
|
||||
requested = false;
|
||||
child.stdout.on("data", (chunk: Buffer) => (stdout += chunk.toString()));
|
||||
child.stderr.on("data", (chunk: Buffer) => {
|
||||
stderr += chunk.toString();
|
||||
if (!requested && stderr.includes("REA_CONTROLLED_BROWSER_READY"))
|
||||
requested = child.kill(signal);
|
||||
});
|
||||
const deadline = setTimeout(() => child.kill("SIGKILL"), 20_000);
|
||||
try {
|
||||
const exited = await new Promise<{
|
||||
code: number | null;
|
||||
signal: NodeJS.Signals | null;
|
||||
}>((resolveExit, reject) => {
|
||||
child.once("error", reject);
|
||||
child.once("close", (code, exitSignal) =>
|
||||
resolveExit({ code, signal: exitSignal }),
|
||||
);
|
||||
});
|
||||
expect(requested, stderr).toBe(true);
|
||||
expect(exited, stderr).toEqual({
|
||||
code: signal === "SIGINT" ? 130 : 143,
|
||||
signal: null,
|
||||
});
|
||||
expect(stderr).toContain("signal_present=1");
|
||||
expect(stderr).toContain("REA_CONTROLLED_CLOSE_CALLBACK");
|
||||
expect(stderr).toContain("REA_CONTROLLED_PROFILE_CALLBACK");
|
||||
const document: unknown = JSON.parse(stdout);
|
||||
const evidence = parseEvidence(document);
|
||||
expect(evidence).toEqual(document);
|
||||
const capture = browserScenarioCaptureSchema.parse(
|
||||
evidence.normalized_result,
|
||||
);
|
||||
expect(capture.browser.cleanup).toBe("terminated-owned-process");
|
||||
expect(capture.steps.map(({ status }) => status)).toEqual([
|
||||
"completed",
|
||||
"cancelled",
|
||||
]);
|
||||
expect(capture.completeness.status).toBe("incomplete");
|
||||
} finally {
|
||||
clearTimeout(deadline);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
|
@ -0,0 +1,119 @@
|
|||
import assert from "node:assert/strict";
|
||||
import { dirname, resolve } from "node:path";
|
||||
import { registerHooks } from "node:module";
|
||||
import { pathToFileURL } from "node:url";
|
||||
|
||||
const root = resolve(dirname(process.argv[1]), "..");
|
||||
const load = (path) => import(pathToFileURL(root + "/dist/" + path).href);
|
||||
const { PlaywrightBrowserScenarioProvider } = await load(
|
||||
"browser/PlaywrightBrowserScenarioProvider.js",
|
||||
);
|
||||
const { PlaywrightScenarioBrowserCleanupOwner } = await load(
|
||||
"browser/PlaywrightScenarioBrowser.js",
|
||||
);
|
||||
const { browserStepArtifactsSchema } = await load(
|
||||
"domain/browserScenarioCaptureValues.js",
|
||||
);
|
||||
const { sanitizeBrowserUrl } = await load("domain/browserObservation.js");
|
||||
const { AnalysisCancelledError } = await load("domain/analysisErrorCore.js");
|
||||
|
||||
// No Chromium process or profile is created. The real provider and cleanup
|
||||
// owner manage a controlled session; only the public CLI receives OS signals.
|
||||
class ControlledSession {
|
||||
mode = "launch";
|
||||
processOwnership = "provider-owned";
|
||||
product = "Controlled fixture";
|
||||
version = "1";
|
||||
initialUrl = "https://app.example.test/";
|
||||
cleanup = new PlaywrightScenarioBrowserCleanupOwner({
|
||||
closeBrowser: async () => {
|
||||
process.stderr.write("REA_CONTROLLED_CLOSE_CALLBACK\n");
|
||||
},
|
||||
removeProfile: async () => {
|
||||
assert.equal(this.cleanup.browserClosed, true);
|
||||
process.stderr.write("REA_CONTROLLED_PROFILE_CALLBACK\n");
|
||||
},
|
||||
});
|
||||
currentUrl() {
|
||||
return this.initialUrl;
|
||||
}
|
||||
sanitizeUrl(value) {
|
||||
return sanitizeBrowserUrl(value);
|
||||
}
|
||||
setStep() {}
|
||||
nextEventSequence() {
|
||||
return 1;
|
||||
}
|
||||
lastEventSequence() {
|
||||
return 0;
|
||||
}
|
||||
events() {
|
||||
return { retained: 0, dropped: 0, items: [] };
|
||||
}
|
||||
async perform(action, signal) {
|
||||
assert.equal(action.action, "wait_for_timeout");
|
||||
let timer, abort;
|
||||
try {
|
||||
await new Promise((resolveWait, reject) => {
|
||||
timer = setTimeout(resolveWait, 60000);
|
||||
abort = () =>
|
||||
reject(new AnalysisCancelledError("capture_browser_scenario"));
|
||||
signal?.addEventListener("abort", abort, { once: true });
|
||||
process.stderr.write(
|
||||
"REA_CONTROLLED_BROWSER_READY signal_present=" +
|
||||
Number(signal !== undefined) +
|
||||
"\n",
|
||||
);
|
||||
if (signal?.aborted) abort();
|
||||
});
|
||||
} finally {
|
||||
clearTimeout(timer);
|
||||
signal?.removeEventListener("abort", abort);
|
||||
}
|
||||
}
|
||||
async capture(requested) {
|
||||
const artifact = (kind) =>
|
||||
!requested.has(kind)
|
||||
? { state: "not_requested" }
|
||||
: kind === "url"
|
||||
? { state: "captured", value: this.sanitizeUrl(this.initialUrl) }
|
||||
: { state: "missing", reason: "controlled fixture unavailable" };
|
||||
return browserStepArtifactsSchema.parse(
|
||||
Object.fromEntries(
|
||||
["screenshot", "dom", "accessibility", "url", "history", "storage"].map(
|
||||
(kind) => [kind, artifact(kind)],
|
||||
),
|
||||
),
|
||||
);
|
||||
}
|
||||
async close() {
|
||||
await this.cleanup.close();
|
||||
return "terminated-owned-process";
|
||||
}
|
||||
redactError(error) {
|
||||
return error instanceof Error
|
||||
? error.message
|
||||
: "controlled fixture failure";
|
||||
}
|
||||
}
|
||||
|
||||
export const createBrowserScenarioProvider = () =>
|
||||
new PlaywrightBrowserScenarioProvider(async () => new ControlledSession());
|
||||
|
||||
const composition = pathToFileURL(
|
||||
root + "/dist/composition/browserScenario.js",
|
||||
).href;
|
||||
registerHooks({
|
||||
load(url, context, nextLoad) {
|
||||
if (url === composition)
|
||||
return {
|
||||
format: "module",
|
||||
shortCircuit: true,
|
||||
source:
|
||||
"export { createBrowserScenarioProvider } from " +
|
||||
JSON.stringify(import.meta.url) +
|
||||
";",
|
||||
};
|
||||
return nextLoad(url, context);
|
||||
},
|
||||
});
|
||||
Loading…
Reference in New Issue