mirror/rea
1
0
Fork 0
mirror of https://github.com/morluto/rea synced 2026-10-25 22:08:11 -04:00
Code Issues Packages Projects Releases Wiki Activity

fix(browser): preserve cleanup failure diagnostics

Browse Source 
Preserve Electron cleanup observer reasons and typed primary failures through the shared error projection. Keep ownership and partial Evidence semantics intact.

Fixes #4403. Refs #4227, #4282.
This commit is contained in:
Ysopwnable
2026-10-16 14:51:25 +08:00
committed by GitHub
parent 2c7583599b
commit 5a77acfbcd
4 changed files with 202 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/browser/PlaywrightElectronActiveProvider.test.ts
+92 -21
View File
@@ -9,6 +9,7 @@ import {
} from "../../tests/fixtures/electron/fuseImage.js";
import * as processOwnership from "../process/ProcessOwnership.js";
import * as ownershipObservation from "../process/ProcessOwnershipObservation.js";
import * as electronActions from "./PlaywrightElectronActiveActions.js";
import { electronActiveObservationInputSchema } from "../domain/javascript/electronActiveObservation.js";
import type { OwnedProcessGroup } from "../process/ProcessOwnership.js";
@@ -97,32 +98,90 @@ const emptyHookSnapshot = () => ({
hook_error: false,
});
it("does not signal a Windows Electron PID when lineage is unavailable", async () => {
const terminateTree = vi.fn(async () => ({
cleaned: true as const,
signaled: true,
}));
const result = await cleanupElectronProcesses(
ownership,
{
status: "unavailable",
observedAt: "2026-10-10T00:00:00.000Z",
runId: ownership.runId,
launcherPid: ownership.leaderPid,
processGroupId: ownership.processGroupId,
reason: "owned launcher is not live",
},
{ platform: "win32", terminateTree },
);
it.each([
"owned launcher is not live",
"process table permission denied",
"process ownership did not match",
])(
"does not signal a Windows Electron PID when lineage is unavailable: %s",
async (reason) => {
const terminateTree = vi.fn(async () => ({
cleaned: true as const,
signaled: true,
}));
const result = await cleanupElectronProcesses(
ownership,
{
status: "unavailable",
observedAt: "2026-10-10T00:00:00.000Z",
runId: ownership.runId,
launcherPid: ownership.leaderPid,
processGroupId: ownership.processGroupId,
reason,
},
{ platform: "win32", terminateTree },
);
expect(terminateTree).not.toHaveBeenCalled();
expect(result).toEqual({
expect(terminateTree).not.toHaveBeenCalled();
expect(result).toEqual({
cleaned: false,
reason: `${reason}; helper cleanup was not proven`,
});
},
);
it("does not signal an unobserved Windows Electron lineage", async () => {
const terminateTree = vi.fn();
expect(
await cleanupElectronProcesses(ownership, undefined, {
platform: "win32",
terminateTree,
}),
).toEqual({
cleaned: false,
reason:
"owned Electron lineage was unavailable; helper cleanup was not proven",
"owned Electron lineage was not observed; helper cleanup was not proven",
});
expect(terminateTree).not.toHaveBeenCalled();
});
it.each([0, 1])(
"preserves a Windows termination failure at tree index %i",
async (failedIndex) => {
const failure = { cleaned: false as const, reason: "termination denied" };
const terminateTree = vi
.fn<typeof processOwnership.cleanupWindowsProcessTree>()
.mockResolvedValue({ cleaned: true, signaled: true });
terminateTree.mockImplementationOnce(async () =>
failedIndex === 0 ? failure : { cleaned: true, signaled: true },
);
if (failedIndex === 1) terminateTree.mockResolvedValueOnce(failure);
const result = await cleanupElectronProcesses(
ownership,
{
status: "verified",
observedAt: "2026-10-10T00:00:00.000Z",
lineage: {
runId: ownership.runId,
launcherPid: ownership.leaderPid,
launcherParentPid: 1,
processGroupId: ownership.processGroupId,
descendants: [
{
pid: 4343,
parentPid: ownership.leaderPid,
processGroupId: ownership.processGroupId,
},
],
},
},
{ platform: "win32", terminateTree },
);
expect(result).toBe(failure);
expect(terminateTree).toHaveBeenCalledTimes(failedIndex + 1);
},
);
it("signals a Windows Electron tree only after lineage is verified", async () => {
const terminateTree = vi.fn(async () => ({
cleaned: true as const,
@@ -157,6 +216,18 @@ it("signals a Windows Electron tree only after lineage is verified", async () =>
it("retains the partial capture without taskkill when Windows lineage is unavailable", async () => {
// Exercise the Windows decision branch; this is not native Windows coverage.
// The injected unavailable observation keeps this independent of host PID reuse.
vi.spyOn(
ownershipObservation,
"observeOwnedProcessLineage",
).mockResolvedValue({
status: "unavailable",
observedAt: "2026-10-10T00:00:00.000Z",
runId: "fixture-run",
launcherPid: 424242,
processGroupId: 424242,
reason: "process table permission denied",
});
const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
Object.defineProperty(process, "platform", { value: "win32" });
const terminateTree = vi
@@ -198,7 +269,7 @@ it("retains the partial capture without taskkill when Windows lineage is unavail
reason: "cleanup_failed",
cleanup: {
reason:
"owned Electron lineage was unavailable; helper cleanup was not proven",
"process table permission denied; helper cleanup was not proven",
},
partialObservation: {
kind: "electron-active-observation",
src/browser/PlaywrightElectronActiveProvider.ts
+10 -17
View File
@@ -292,14 +292,17 @@ export const cleanupElectronProcesses = async (
terminateTree: cleanupWindowsProcessTree,
},
): Promise<ProcessCleanupResult> => {
if (lineage?.status !== "verified") {
if (host.platform !== "win32") {
const root = await cleanupOwnedProcessGroup(ownership);
if (!root.cleaned) return root;
}
return {
cleaned: false,
reason: `${lineage?.reason ?? "owned Electron lineage was not observed"}; helper cleanup was not proven`,
};
}
if (host.platform === "win32") {
// An empty Windows process table is not ownership proof for this PID.
if (lineage?.status !== "verified")
return {
cleaned: false,
reason:
"owned Electron lineage was unavailable; helper cleanup was not proven",
};
const root = await host.terminateTree(ownership.leaderPid);
if (!root.cleaned) return root;
for (const descendant of lineage.lineage.descendants) {
@@ -308,16 +311,6 @@ export const cleanupElectronProcesses = async (
}
return root;
}
if (lineage?.status !== "verified") {
const root = await cleanupOwnedProcessGroup(ownership);
return root.cleaned
? {
cleaned: false,
reason:
"owned Electron lineage was unavailable; helper cleanup was not proven",
}
: root;
}
const groupIds = selectCapturedProcessGroupIds(
ownership.leaderPid,
lineage.lineage.descendants.map(({ pid, processGroupId }) => ({
src/domain/analysisErrorProjection.ts
+24 -20
View File
@@ -110,7 +110,7 @@ const underlyingErrorCode = (
if (error instanceof BrowserObservationError)
return error.userCategory === "cancelled"
? "cancelled"
: browserErrorCode(error.reason);
: BROWSER_ERROR_CODES[error.reason];
if (error instanceof ArtifactOperationError)
return artifactOperationCode(error);
if (error instanceof BinaryTargetError)
@@ -126,22 +126,25 @@ const underlyingErrorCode = (
return staticErrorCode(error._tag);
};
const browserErrorCode = (
reason: BrowserObservationError["reason"],
): AnalysisErrorProjection["code"] => {
if (reason === "cancelled") return "cancelled";
if (reason === "timeout") return "provider_timeout";
if (reason === "payload_limit") return "truncated";
if (
reason === "target_not_found" ||
reason === "target_not_allowed" ||
reason === "service_worker_blocked" ||
reason === "target_changed"
)
return "target_unavailable";
if (reason === "endpoint_unreachable" || reason === "disconnected")
return "provider_unavailable";
return "unreadable_output";
const BROWSER_ERROR_CODES: Readonly<
Record<BrowserObservationError["reason"], AnalysisErrorProjection["code"]>
> = {
endpoint_unreachable: "provider_unavailable",
invalid_endpoint_response: "unreadable_output",
target_not_found: "target_unavailable",
target_not_allowed: "target_unavailable",
service_worker_blocked: "target_unavailable",
target_changed: "target_unavailable",
protocol_error: "unreadable_output",
disconnected: "provider_unavailable",
payload_limit: "truncated",
window_not_found: "unreadable_output",
window_metadata_missing: "unreadable_output",
process_ownership_unavailable: "unreadable_output",
cleanup_failed: "execution_failure",
cancelled: "cancelled",
timeout: "provider_timeout",
secret_unavailable: "unreadable_output",
};
const artifactOperationCode = (
@@ -439,8 +442,9 @@ const providerErrorDetails = (
const primaryBrowserFailureReason = (
error: BrowserObservationError,
): string | undefined => {
if (!(error.cause instanceof AggregateError)) return undefined;
const primary = error.cause.errors[0];
const aggregate = error.cause instanceof AggregateError;
const primary =
error.cause instanceof AggregateError ? error.cause.errors[0] : error.cause;
if (primary instanceof BrowserObservationError) return primary.reason;
if (primary instanceof AnalysisCancelledError) return "cancelled";
if (primary instanceof AnalysisTimeoutError) return "timeout";
@@ -448,7 +452,7 @@ const primaryBrowserFailureReason = (
return typeof primary._tag === "string"
? primary._tag
: "execution_failure";
return "execution_failure";
return aggregate ? "execution_failure" : undefined;
};
const lifecycleErrorDetails = (
tests/conformance/contracts/analysisErrorProjection.test.ts
+76
View File
@@ -7,9 +7,85 @@ import { EvidenceFileError } from "../../../src/domain/evidenceErrors.js";
import { HopperRemoteError } from "../../../src/domain/hopperErrors.js";
import { UnknownRegistryError } from "../../../src/domain/unknownRegistryError.js";
import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js";
import { BrowserObservationError } from "../../../src/domain/browserObservationError.js";
import {
AnalysisCancelledError,
AnalysisTimeoutError,
} from "../../../src/domain/analysisErrorCore.js";
import type { AnalysisError } from "../../../src/domain/analysisErrorBase.js";
describe("analysis error projection contract", () => {
it("keeps cleanup failure distinct from unreadable collected output", () => {
const projected = projectAnalysisError(
new BrowserObservationError(
"capture_electron_scenario",
"cleanup_failed",
{
cause: new Error("lineage could not be inspected"),
cleanup: {
reason: "process table permission denied",
resources: ["owned_process_group"],
},
capturedOutput: {
stdout: "completed observation",
stderr: "",
truncated: false,
},
},
),
);
expect(projected).toMatchObject({
code: "cleanup_incomplete",
details: {
cleanup_reason: "process table permission denied",
execution_failure: "execution_failure",
captured_output: { stdout: "completed observation", truncated: false },
},
});
expect(projected.details).not.toHaveProperty("primary_reason");
expect(analysisErrorProjectionSchema.safeParse(projected).success).toBe(
true,
);
});
it.each([
[new AnalysisCancelledError("capture_electron_scenario"), "cancelled"],
[new AnalysisTimeoutError("capture_electron_scenario", 10), "timeout"],
[
new BrowserObservationError("capture_electron_scenario", "disconnected"),
"disconnected",
],
] as const)(
"preserves direct and aggregated primary failures: %s",
(primary, reason) => {
for (const cause of [
primary,
new AggregateError([primary, new Error("close failed")]),
]) {
const projected = projectAnalysisError(
new BrowserObservationError(
"capture_electron_scenario",
"cleanup_failed",
{
cause,
cleanup: {
reason: "close failed",
resources: ["owned_process_group"],
},
},
),
);
expect(projected).toMatchObject({
code: "cleanup_incomplete",
details: { primary_reason: reason, cleanup_reason: "close failed" },
});
expect(analysisErrorProjectionSchema.safeParse(projected).success).toBe(
true,
);
}
},
);
it("accepts every closed error-reason variant", () => {
const variants: AnalysisError[] = [
...(