mirror of
https://github.com/morluto/rea
synced 2026-10-25 22:08:11 -04:00
fix(javascript): retain primary interruption through worker cleanup
This commit is contained in:
@@ -246,6 +246,10 @@ waiting before receiving the reply, follow [MCP cancellation recovery](mcp-contr
|
||||
to find the record. CLI failure output contains the complete partial Evidence.
|
||||
Owned analyzer cleanup is verified before private files are released; uncertain
|
||||
cleanup retains ownership for retry and is reported separately.
|
||||
Typed cancellation or another primary interruption remains attached while
|
||||
cleanup is incomplete. Confirmed release restores that primary classification,
|
||||
including the caller's exact cancellation reason, while retaining partial
|
||||
application Evidence and historical cleanup diagnostics.
|
||||
|
||||
Private worker storage failures retain the source identity, worker phase and
|
||||
reported I/O message as execution failures; they are distinct from malformed
|
||||
|
||||
@@ -23,6 +23,7 @@ import type { JavaScriptSourceSelection } from "../../domain/javascript/javascri
|
||||
import type { ElectronBoundarySummary } from "../../domain/javascript/javascriptApplicationAnalysis.js";
|
||||
import { parseOwnedJavaScriptApplicationAnalysisSteps } from "../../domain/javascript/javascriptApplicationAnalysis.js";
|
||||
import { AnalysisError } from "../../domain/analysisErrorBase.js";
|
||||
import { analysisErrorWithCleanupFailure } from "../../domain/analysisErrorCleanup.js";
|
||||
import { ArtifactOperationError } from "../../domain/artifactOperationError.js";
|
||||
import type { ArtifactInventorySnapshot } from "../../domain/artifactInventorySnapshot.js";
|
||||
import type { ArtifactInventoryPartialObservation } from "../../domain/artifactPartialObservation.js";
|
||||
@@ -232,7 +233,7 @@ export const reconstructJavaScriptArtifact = async (
|
||||
}
|
||||
}
|
||||
const analysis = outcome.analysis;
|
||||
let interruption = cleanupFailure ?? outcome.interruption;
|
||||
let interruption = outcome.interruption;
|
||||
// Source facts are owned now. Finish their projection cooperatively so a
|
||||
// later cancellation can retain Evidence instead of discarding those facts.
|
||||
const checkpointCompletedAnalysis = async (): Promise<void> => {
|
||||
@@ -251,7 +252,7 @@ export const reconstructJavaScriptArtifact = async (
|
||||
message: string,
|
||||
): Promise<void> => {
|
||||
await checkpointCompletedAnalysis();
|
||||
if (interruption == null)
|
||||
if (interruption == null && cleanupFailure === undefined)
|
||||
await progress?.report({ phase, completed: 0, total: 1, message });
|
||||
await checkpointCompletedAnalysis();
|
||||
};
|
||||
@@ -373,7 +374,17 @@ export const reconstructJavaScriptArtifact = async (
|
||||
limitations: analysis.limitations,
|
||||
};
|
||||
await checkpointCompletedAnalysis();
|
||||
if (interruption !== null && interruption !== undefined) {
|
||||
const failure =
|
||||
cleanupFailure === undefined
|
||||
? interruption
|
||||
: interruption === null
|
||||
? cleanupFailure
|
||||
: analysisErrorWithCleanupFailure(
|
||||
interruption,
|
||||
cleanupFailure,
|
||||
OPERATION,
|
||||
).bindCleanupDependencies([interruption, cleanupFailure]);
|
||||
if (failure !== null) {
|
||||
const { electron_summary: summary, ...application } = completed;
|
||||
const partial = await completeJavaScriptAnalysisSteps(
|
||||
parseOwnedJavaScriptApplicationAnalysisSteps({
|
||||
@@ -387,7 +398,7 @@ export const reconstructJavaScriptArtifact = async (
|
||||
input,
|
||||
partial,
|
||||
);
|
||||
throw interruption.retainPartialObservation(evidence);
|
||||
throw failure.retainPartialObservation(evidence);
|
||||
}
|
||||
reconstructionOutcome = { kind: "completed", result: completed };
|
||||
} catch (cause: unknown) {
|
||||
|
||||
@@ -1194,6 +1194,7 @@ export class JavaScriptAnalysisWorker {
|
||||
},
|
||||
{
|
||||
operation: OPERATION,
|
||||
...(original === undefined ? {} : { cleanupPrimary: original }),
|
||||
cause: new Error(
|
||||
reason,
|
||||
original === undefined ? undefined : { cause: original },
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { setImmediate } from "node:timers";
|
||||
import { mkdir, open, realpath, symlink, writeFile } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { expect, it, onTestFinished } from "vitest";
|
||||
import { basename, join } from "node:path";
|
||||
import { expect, it, onTestFinished, vi } from "vitest";
|
||||
import { z } from "zod";
|
||||
import {
|
||||
ArtifactResourceScope,
|
||||
@@ -13,6 +13,87 @@ import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js";
|
||||
import { AnalysisOutputError } from "../../../src/domain/analysisErrorCore.js";
|
||||
import { analyzeJavaScriptApplication } from "../../support/javascriptApplicationScope.js";
|
||||
import { PrivateRuntimeRoot } from "../../../src/process/PrivateRuntimeRoot.js";
|
||||
import { JavaScriptAnalysisWorker } from "../../../src/javascript/analysis/JavaScriptAnalysisWorker.js";
|
||||
|
||||
it.each(["worker-phase", "final-cleanup", "no-cancellation"] as const)(
|
||||
"preserves the primary through failed worker cleanup and exact-owner retry: %s",
|
||||
async (timing) => {
|
||||
const root = await createTestTempDirectory("rea-js-primary-cleanup-");
|
||||
await writeFile(join(root, "main.js"), "export const observed = 7;");
|
||||
const controller = new AbortController();
|
||||
const reason = { request: "cancelled while worker resources remain owned" };
|
||||
let denyRemoval = true;
|
||||
let runtimePath: string | undefined;
|
||||
const originalClose = PrivateRuntimeRoot.prototype.close;
|
||||
const close = vi
|
||||
.spyOn(PrivateRuntimeRoot.prototype, "close")
|
||||
.mockImplementation(async function (this: PrivateRuntimeRoot) {
|
||||
if (basename(this.path).startsWith("rea-javascript-analysis-")) {
|
||||
runtimePath = this.path;
|
||||
if (timing === "final-cleanup") controller.abort(reason);
|
||||
if (denyRemoval)
|
||||
throw new Error(
|
||||
"controlled JavaScript worker root removal failure",
|
||||
);
|
||||
}
|
||||
await originalClose.call(this);
|
||||
});
|
||||
// Exercise physical finalization even after the one healthy source finishes.
|
||||
const reuse = vi
|
||||
.spyOn(JavaScriptAnalysisWorker.prototype, "canReuse")
|
||||
.mockReturnValue(false);
|
||||
const scope = new ArtifactResourceScope();
|
||||
onTestFinished(async () => {
|
||||
denyRemoval = false;
|
||||
try {
|
||||
await scope.close();
|
||||
} finally {
|
||||
close.mockRestore();
|
||||
reuse.mockRestore();
|
||||
}
|
||||
});
|
||||
const result = await analyzeOwnedApplication(
|
||||
{ input_path: root, format: "directory" },
|
||||
scope,
|
||||
{
|
||||
signal: controller.signal,
|
||||
progress: {
|
||||
async report(event) {
|
||||
if (
|
||||
timing === "worker-phase" &&
|
||||
event.phase.startsWith("javascript_worker_")
|
||||
)
|
||||
controller.abort(reason);
|
||||
},
|
||||
},
|
||||
},
|
||||
);
|
||||
if (result.ok) throw new Error("Expected cleanup failure");
|
||||
expect(runtimePath).toBeDefined();
|
||||
expect(result.error.cleanupIncomplete).toBe(true);
|
||||
expect(result.error.cleanupResources).toContain(runtimePath);
|
||||
expect(projectAnalysisError(result.error)).toMatchObject({
|
||||
code: "cleanup_incomplete",
|
||||
details: { partial_observation: expect.any(Object) },
|
||||
});
|
||||
if (timing !== "no-cancellation") {
|
||||
expect(result.error.cleanupPrimary).toBeDefined();
|
||||
expect(result.error.cleanupPrimary?.cause).toBe(reason);
|
||||
}
|
||||
denyRemoval = false;
|
||||
await scope.close();
|
||||
expect(result.error.cleanupIncomplete).toBe(false);
|
||||
expect(result.error.cleanupResources).toEqual([]);
|
||||
const projected = projectAnalysisError(result.error);
|
||||
expect(projected.code).toBe(
|
||||
timing === "no-cancellation" ? "execution_failure" : "cancelled",
|
||||
);
|
||||
expect(projected.details).toMatchObject({
|
||||
partial_observation: expect.any(Object),
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it.each(["during cleanup", "on the next control turn"] as const)(
|
||||
"retains the final source when cancellation arrives %s after analysis",
|
||||
|
||||
Reference in New Issue
Block a user