mirror of
https://github.com/morluto/rea.git
synced 2026-10-19 04:19:54 -05:00
fix(providers): preserve live cleanup ownership through retries
Share exact command/helper cleanup adoption across Java probes. Preserve complete live resource labels and explicit unknowns through inspection leases, and retain unresolved ownership when a cleanup capability is unavailable. Validation: focused cleanup, provider/doctor and Kuna consumer checks; shared Ghidra lifecycle and public doctor regressions. Native Windows/Linux provider behavior remains unverified.
This commit is contained in:
1 parent
3ad1032b11
commit
964decd989
15 files changed
+288
-74
No files matched your search
@@ -205,7 +205,14 @@ export interface AnalysisClientContext {
|
||||
|
||||
/** One lifecycle owner for descriptor cleanup and abortable inspection work. */
|
||||
export interface FileCloseResourceOwner {
|
||||
retain(owner: { close(): Promise<void> }, resource: string): void;
|
||||
retain(
|
||||
owner: {
|
||||
close(): Promise<void>;
|
||||
/** Complete labels authored by this exact owner; the display label is only a fallback. */
|
||||
readonly cleanupResources?: readonly string[] | undefined;
|
||||
},
|
||||
resource: string,
|
||||
): void;
|
||||
run<T>(operation: () => Promise<T>): Promise<T>;
|
||||
}
|
||||
|
||||
|
||||
@@ -510,7 +510,7 @@ export const systemDoctorHost = (
|
||||
resources.retain({
|
||||
kind: "lease",
|
||||
resource,
|
||||
lease: new AnalysisCleanupLease(() => owner.close(), [resource]),
|
||||
lease: AnalysisCleanupLease.forCloseOwner(owner, resource),
|
||||
}),
|
||||
run: (operation) => resources.run(operation),
|
||||
};
|
||||
|
||||
@@ -114,7 +114,7 @@ export class AnalysisProviderRegistry {
|
||||
this.#profileFiles.retain({
|
||||
kind: "lease",
|
||||
resource,
|
||||
lease: new AnalysisCleanupLease(() => owner.close(), [resource]),
|
||||
lease: AnalysisCleanupLease.forCloseOwner(owner, resource),
|
||||
}),
|
||||
run: (operation) => this.#profileFiles.run(operation),
|
||||
};
|
||||
|
||||
@@ -111,3 +111,34 @@ it("resolves every live retry error bound to one lease", async () => {
|
||||
expect(firstError.cause).toBe(firstCause);
|
||||
expect(secondError.cause).toBe(secondCause);
|
||||
});
|
||||
|
||||
it("keeps explicit unknown labels separate from an absent owner label", () => {
|
||||
const unknown = AnalysisCleanupLease.forCloseOwner(
|
||||
{
|
||||
close: async () => {},
|
||||
get cleanupResources() {
|
||||
return undefined;
|
||||
},
|
||||
},
|
||||
"display-only",
|
||||
);
|
||||
const absent = AnalysisCleanupLease.forCloseOwner(
|
||||
{ close: async () => {} },
|
||||
"owned-path",
|
||||
);
|
||||
expect(unknown.cleanupResources).toBeUndefined();
|
||||
expect(absent.cleanupResources).toEqual(["owned-path"]);
|
||||
});
|
||||
|
||||
it.each([false, true])(
|
||||
"preserves unknown resource labels in pending lease groups (known peer: %s)",
|
||||
(knownPeer) => {
|
||||
const unknown = new AnalysisCleanupLease(async () => {});
|
||||
const error = new ProviderCleanupError("fixture", [], {});
|
||||
const peers = knownPeer
|
||||
? [new AnalysisCleanupLease(async () => {}, ["known"]), unknown]
|
||||
: [unknown];
|
||||
const group = new AnalysisCleanupLeaseGroup(error, peers);
|
||||
expect(group.cleanupResources).toBeUndefined();
|
||||
},
|
||||
);
|
||||
@@ -1,4 +1,7 @@
|
||||
import type { AnalysisError } from "../domain/analysisErrorBase.js";
|
||||
import type {
|
||||
AnalysisError,
|
||||
AnalysisCleanupResourceOwner,
|
||||
} from "../domain/analysisErrorBase.js";
|
||||
|
||||
/** Link one resource's confirmed release to its originating analysis error. */
|
||||
export class AnalysisCleanupLease {
|
||||
@@ -11,14 +14,46 @@ export class AnalysisCleanupLease {
|
||||
return this.#released;
|
||||
}
|
||||
|
||||
readonly cleanupResources: readonly string[] | undefined;
|
||||
readonly #resourceOwner: AnalysisCleanupResourceOwner | undefined;
|
||||
readonly #resources: readonly string[] | undefined;
|
||||
|
||||
get cleanupResources(): readonly string[] | undefined {
|
||||
return this.#resourceOwner === undefined
|
||||
? this.#resources
|
||||
: this.#resourceOwner.cleanupResources;
|
||||
}
|
||||
|
||||
/** Adopt exact close authority while retaining its live labels across retries. */
|
||||
static forCloseOwner(
|
||||
owner: {
|
||||
close(): Promise<void>;
|
||||
readonly cleanupResources?: readonly string[] | undefined;
|
||||
},
|
||||
resource: string,
|
||||
): AnalysisCleanupLease {
|
||||
return new AnalysisCleanupLease(
|
||||
() => owner.close(),
|
||||
!("cleanupResources" in owner)
|
||||
? [resource]
|
||||
: {
|
||||
get cleanupResources() {
|
||||
return owner.cleanupResources;
|
||||
},
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
constructor(
|
||||
private readonly cleanup: () => Promise<void>,
|
||||
resources?: readonly string[],
|
||||
resources?: readonly string[] | AnalysisCleanupResourceOwner,
|
||||
) {
|
||||
this.cleanupResources =
|
||||
resources === undefined ? undefined : [...resources];
|
||||
if (resources !== undefined && "cleanupResources" in resources) {
|
||||
this.#resourceOwner = resources;
|
||||
this.#resources = undefined;
|
||||
} else {
|
||||
this.#resourceOwner = undefined;
|
||||
this.#resources = resources === undefined ? undefined : [...resources];
|
||||
}
|
||||
}
|
||||
|
||||
async release(): Promise<void> {
|
||||
|
||||
@@ -205,11 +205,14 @@ export class ArtifactReaderFailure extends Error {
|
||||
/** Give otherwise opaque cleanup failures a concrete owned resource. */
|
||||
static cleanupObservation(
|
||||
cause: unknown,
|
||||
resource: string,
|
||||
resource: string | readonly string[],
|
||||
): AnalysisCleanupObservation {
|
||||
if (cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined)
|
||||
return cause.cleanup;
|
||||
return { reason: errorMessage(cause), resources: [resource] };
|
||||
return {
|
||||
reason: errorMessage(cause),
|
||||
resources: typeof resource === "string" ? [resource] : [...resource],
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -179,7 +179,12 @@ export class ArtifactResourceScope {
|
||||
}
|
||||
if (failures.length === 0) return;
|
||||
const observations = failures.map(({ owner, cause }) =>
|
||||
ArtifactReaderFailure.cleanupObservation(cause, owner.resource),
|
||||
ArtifactReaderFailure.cleanupObservation(
|
||||
cause,
|
||||
owner.kind === "lease" && owner.lease instanceof AnalysisCleanupLease
|
||||
? (owner.lease.cleanupResources ?? owner.resource)
|
||||
: owner.resource,
|
||||
),
|
||||
);
|
||||
const resources = [
|
||||
...new Set(observations.flatMap(({ resources }) => resources)),
|
||||
|
||||
@@ -1,12 +1,11 @@
|
||||
import { retainOwnedCommandCleanup } from "../providers/OwnedCommandCleanup.js";
|
||||
import { analysisClientFailureForOperation } from "../providers/AnalysisClientFailure.js";
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { AnalysisError } from "../domain/analysisErrorBase.js";
|
||||
import { AnalysisCancelledError } from "../domain/analysisErrorCore.js";
|
||||
import { ProviderAdapterError } from "../domain/providerAdapterError.js";
|
||||
import { ProviderCleanupError } from "../domain/providerCleanupError.js";
|
||||
import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js";
|
||||
import { jsonValueSchema } from "../domain/jsonValue.js";
|
||||
import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js";
|
||||
import type { FileCloseResourceOwner } from "../application/AnalysisProvider.js";
|
||||
import { DarwinProcessOwnershipInspectionError } from "../process/DarwinProcessRunTokenReader.js";
|
||||
import {
|
||||
@@ -207,14 +206,16 @@ const observeGhidraJava = async (
|
||||
Object.defineProperty(interpreted.error, "cancellation", {
|
||||
value: commandFailure.cancellation,
|
||||
});
|
||||
const cleanups = retainOwnedCommandCleanup(
|
||||
cause,
|
||||
options.resourceOwner,
|
||||
"ghidra",
|
||||
"ghidra:java",
|
||||
);
|
||||
let primary: AnalysisError | undefined;
|
||||
if (interpreted !== undefined && !interpreted.ok) {
|
||||
// A completed, invalid report remains the primary fact across cleanup-time cancellation.
|
||||
if (
|
||||
commandFailure?.cleanupOwner === undefined &&
|
||||
preparation?.cleanupFailure === undefined
|
||||
)
|
||||
return interpreted;
|
||||
if (cleanups.length === 0) return interpreted;
|
||||
primary = probeFailureError(interpreted.error, cause);
|
||||
} else if (
|
||||
commandFailure?.reason === "cancelled" &&
|
||||
@@ -264,53 +265,6 @@ const observeGhidraJava = async (
|
||||
} else if (cause instanceof AnalysisError) {
|
||||
primary = cause;
|
||||
}
|
||||
const cleanups: AnalysisError[] = [];
|
||||
if (preparation?.cleanupFailure !== undefined) {
|
||||
const cleanup = preparation.cleanupFailure;
|
||||
const lease = new AnalysisCleanupLease(async () => {
|
||||
if (!cleanup.cleanupIncomplete) return;
|
||||
if (preparation.cleanupOwner === undefined) throw cleanup;
|
||||
await preparation.cleanupOwner.close();
|
||||
if (cleanup.cleanupIncomplete) throw cleanup;
|
||||
}, cleanup.cleanupResources);
|
||||
options.resourceOwner.retain(
|
||||
{ close: () => lease.release() },
|
||||
cleanup.cleanupResources[0] ??
|
||||
"Java prerequisite ownership preparation",
|
||||
);
|
||||
cleanups.push(cleanup);
|
||||
}
|
||||
if (commandFailure?.cleanupOwner !== undefined) {
|
||||
const supervisor = commandFailure.cleanupOwner;
|
||||
const cleanup = new ProviderCleanupError(
|
||||
"ghidra",
|
||||
commandFailure.resources,
|
||||
{
|
||||
reason:
|
||||
commandFailure.cleanupFailure ??
|
||||
"Java prerequisite cleanup remains incomplete",
|
||||
...(commandFailure.cleanupResult === undefined
|
||||
? {}
|
||||
: {
|
||||
process_cleanup: jsonValueSchema.parse(
|
||||
commandFailure.cleanupResult,
|
||||
),
|
||||
}),
|
||||
},
|
||||
{ cause, operation: "ghidra:java" },
|
||||
);
|
||||
const lease = new AnalysisCleanupLease(async () => {
|
||||
const stopped = await supervisor.stop();
|
||||
if (stopped.status === "incomplete") throw cleanup;
|
||||
}, commandFailure.resources);
|
||||
cleanup.bindCleanupResourceOwner(lease);
|
||||
lease.bindCleanupError(cleanup);
|
||||
options.resourceOwner.retain(
|
||||
{ close: () => lease.release() },
|
||||
commandFailure.resources[0] ?? "Java prerequisite process",
|
||||
);
|
||||
cleanups.push(cleanup);
|
||||
}
|
||||
if (primary === undefined) {
|
||||
const code =
|
||||
commandFailure?.reason === "timeout"
|
||||
|
||||
@@ -113,7 +113,7 @@ export class GhidraProvider implements AnalysisProviderCandidate {
|
||||
this.#inspectionResources.retain({
|
||||
kind: "lease",
|
||||
resource,
|
||||
lease: new AnalysisCleanupLease(() => owner.close(), [resource]),
|
||||
lease: AnalysisCleanupLease.forCloseOwner(owner, resource),
|
||||
}),
|
||||
run: (operation) => this.#inspectionResources.run(operation),
|
||||
};
|
||||
|
||||
@@ -92,9 +92,17 @@ export const createKunaFileCloseScope = (
|
||||
return {
|
||||
owner: {
|
||||
retain: (owner, resource) => {
|
||||
const lease = new AnalysisCleanupLease(() => owner.close(), [resource]);
|
||||
const lease = AnalysisCleanupLease.forCloseOwner(owner, resource);
|
||||
leases.set(owner, lease);
|
||||
parent.retain({ close: () => lease.release() }, resource);
|
||||
parent.retain(
|
||||
{
|
||||
close: () => lease.release(),
|
||||
get cleanupResources() {
|
||||
return lease.cleanupResources ?? [resource];
|
||||
},
|
||||
},
|
||||
resource,
|
||||
);
|
||||
},
|
||||
run: (operation) => parent.run(operation),
|
||||
},
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js";
|
||||
import { z } from "zod";
|
||||
|
||||
import {
|
||||
@@ -1382,7 +1383,7 @@ export class KunaSessionClient implements AnalysisClient {
|
||||
this.#fileCloseResources.retain({
|
||||
kind: "lease",
|
||||
resource,
|
||||
lease: { release: () => owner.close() },
|
||||
lease: AnalysisCleanupLease.forCloseOwner(owner, resource),
|
||||
}),
|
||||
run: <T>(operation: () => Promise<T>) =>
|
||||
this.#fileCloseResources.run(operation),
|
||||
|
||||
@@ -26,7 +26,7 @@ export class AnalysisClientCreation {
|
||||
this.#inspectionResources.retain({
|
||||
kind: "lease",
|
||||
resource,
|
||||
lease: new AnalysisCleanupLease(() => owner.close(), [resource]),
|
||||
lease: AnalysisCleanupLease.forCloseOwner(owner, resource),
|
||||
}),
|
||||
run: (operation) => this.#inspectionResources.run(operation),
|
||||
};
|
||||
|
||||
@@ -0,0 +1,86 @@
|
||||
import { jsonValueSchema } from "../domain/jsonValue.js";
|
||||
import type { AnalysisError } from "../domain/analysisErrorBase.js";
|
||||
import type { FileCloseResourceOwner } from "../application/AnalysisProvider.js";
|
||||
import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js";
|
||||
import { ProviderCleanupError } from "../domain/providerCleanupError.js";
|
||||
import { DarwinProcessOwnershipInspectionError } from "../process/DarwinProcessRunTokenReader.js";
|
||||
import { OwnedCommandFailure } from "../process/OwnedCommand.js";
|
||||
|
||||
/** Transfer exact helper and command cleanup capabilities to the caller's scope. */
|
||||
export const retainOwnedCommandCleanup = (
|
||||
cause: unknown,
|
||||
resourceOwner: FileCloseResourceOwner,
|
||||
providerId: string,
|
||||
operation: string,
|
||||
): readonly AnalysisError[] => {
|
||||
const commandFailure =
|
||||
cause instanceof OwnedCommandFailure ? cause : undefined;
|
||||
const preparation =
|
||||
commandFailure?.preparationFailure ??
|
||||
(cause instanceof DarwinProcessOwnershipInspectionError
|
||||
? cause
|
||||
: undefined);
|
||||
const cleanups: AnalysisError[] = [];
|
||||
if (preparation?.cleanupFailure !== undefined) {
|
||||
const cleanup = preparation.cleanupFailure;
|
||||
const lease = new AnalysisCleanupLease(async () => {
|
||||
if (!cleanup.cleanupIncomplete) return;
|
||||
if (preparation.cleanupOwner === undefined) throw cleanup;
|
||||
await preparation.cleanupOwner.close();
|
||||
if (cleanup.cleanupIncomplete) throw cleanup;
|
||||
}, cleanup);
|
||||
resourceOwner.retain(
|
||||
{
|
||||
close: () => lease.release(),
|
||||
get cleanupResources() {
|
||||
return cleanup.cleanupResources;
|
||||
},
|
||||
},
|
||||
cleanup.cleanupResources[0] ??
|
||||
"Provider prerequisite ownership preparation",
|
||||
);
|
||||
cleanups.push(cleanup);
|
||||
}
|
||||
if (
|
||||
commandFailure !== undefined &&
|
||||
(commandFailure.cleanupOwner !== undefined ||
|
||||
commandFailure.cleanupFailure !== null)
|
||||
) {
|
||||
const supervisor = commandFailure.cleanupOwner;
|
||||
const cleanup = new ProviderCleanupError(
|
||||
providerId,
|
||||
commandFailure.resources,
|
||||
{
|
||||
reason:
|
||||
commandFailure.cleanupFailure ??
|
||||
"Provider prerequisite cleanup remains incomplete",
|
||||
...(commandFailure.cleanupResult === undefined
|
||||
? {}
|
||||
: {
|
||||
process_cleanup: jsonValueSchema.parse(
|
||||
commandFailure.cleanupResult,
|
||||
),
|
||||
}),
|
||||
},
|
||||
{ cause, operation },
|
||||
);
|
||||
const lease = new AnalysisCleanupLease(async () => {
|
||||
if (supervisor === undefined) throw cleanup;
|
||||
const stopped = await supervisor.stop();
|
||||
if (stopped.status === "incomplete") throw cleanup;
|
||||
}, commandFailure.resources);
|
||||
cleanup.bindCleanupResourceOwner(lease);
|
||||
lease.bindCleanupError(cleanup);
|
||||
resourceOwner.retain(
|
||||
{
|
||||
close: () => lease.release(),
|
||||
get cleanupResources() {
|
||||
return cleanup.cleanupResources;
|
||||
},
|
||||
},
|
||||
commandFailure.resources[0] ?? "Provider prerequisite process",
|
||||
);
|
||||
cleanups.push(cleanup);
|
||||
}
|
||||
return cleanups;
|
||||
};
|
||||
@@ -0,0 +1,87 @@
|
||||
import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js";
|
||||
import { DarwinProcessOwnershipInspectionError } from "../../../src/process/DarwinProcessRunTokenReader.js";
|
||||
import { expect, it } from "vitest";
|
||||
import { OwnedCommandFailure } from "../../../src/process/OwnedCommand.js";
|
||||
import { retainOwnedCommandCleanup } from "../../../src/providers/OwnedCommandCleanup.js";
|
||||
import { createInspectionResources } from "../../fixtures/inspectionResources.js";
|
||||
import { ArtifactResourceRetryFailure } from "../../../src/artifacts/ArtifactResourceScope.js";
|
||||
|
||||
it("retains every owned label without inventing a missing cleanup capability", async () => {
|
||||
// Model unavailable capabilities only; these labels name no real resources.
|
||||
const resources = createInspectionResources();
|
||||
const labels = ["controlled-run", "controlled-leader", "controlled-group"];
|
||||
const cause = new OwnedCommandFailure(
|
||||
"process",
|
||||
"cleanup unresolved",
|
||||
null,
|
||||
"cleanup capability unavailable",
|
||||
undefined,
|
||||
labels,
|
||||
);
|
||||
const [cleanup] = retainOwnedCommandCleanup(
|
||||
cause,
|
||||
resources.owner,
|
||||
"jadx",
|
||||
"inspect_jadx_configuration",
|
||||
);
|
||||
expect(cleanup?.cleanupResources).toEqual(labels);
|
||||
for (let attempt = 0; attempt < 2; attempt++) {
|
||||
const error: unknown = await resources
|
||||
.close()
|
||||
.catch((failure: unknown) => failure);
|
||||
expect(error).toBeInstanceOf(ArtifactResourceRetryFailure);
|
||||
if (!(error instanceof ArtifactResourceRetryFailure))
|
||||
throw new Error("Expected retained ownership");
|
||||
expect(error.cleanup?.resources).toEqual(labels);
|
||||
expect(cleanup?.cleanupIncomplete).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it("reports only live helper peers across partial cleanup and retry", async () => {
|
||||
const resources = createInspectionResources();
|
||||
let labels = ["helper-one", "helper-two"];
|
||||
const cleanup = new ProviderCleanupError("process-ownership", labels, {
|
||||
reason: "mixed helper cleanup",
|
||||
}).bindCleanupResourceOwner({
|
||||
get cleanupResources() {
|
||||
return labels;
|
||||
},
|
||||
});
|
||||
const preparation = new DarwinProcessOwnershipInspectionError(
|
||||
"controlled preparation cleanup",
|
||||
{
|
||||
cleanupFailure: cleanup,
|
||||
cleanupOwner: {
|
||||
close: async () => {
|
||||
if (labels.length > 1) {
|
||||
labels = ["helper-two"];
|
||||
throw cleanup;
|
||||
}
|
||||
labels = [];
|
||||
cleanup.resolveCleanup();
|
||||
},
|
||||
},
|
||||
},
|
||||
);
|
||||
retainOwnedCommandCleanup(
|
||||
preparation,
|
||||
resources.owner,
|
||||
"jadx",
|
||||
"inspect_jadx_configuration",
|
||||
);
|
||||
try {
|
||||
const failed: unknown = await resources
|
||||
.close()
|
||||
.catch((cause: unknown) => cause);
|
||||
expect(failed).toBeInstanceOf(ArtifactResourceRetryFailure);
|
||||
if (!(failed instanceof ArtifactResourceRetryFailure))
|
||||
throw new Error("Expected mixed cleanup");
|
||||
expect(failed.cleanup?.resources).toEqual(["helper-two"]);
|
||||
await resources.close();
|
||||
expect(cleanup.cleanupIncomplete).toBe(false);
|
||||
} finally {
|
||||
labels = [];
|
||||
cleanup.resolveCleanup();
|
||||
await resources.close();
|
||||
}
|
||||
});
|
||||
+1
-4
@@ -10,10 +10,7 @@ export const createInspectionResources = () => {
|
||||
scope.retain({
|
||||
kind: "lease",
|
||||
resource,
|
||||
lease: new AnalysisCleanupLease(
|
||||
() => resourceOwner.close(),
|
||||
[resource],
|
||||
),
|
||||
lease: AnalysisCleanupLease.forCloseOwner(resourceOwner, resource),
|
||||
}),
|
||||
run: (operation) => scope.run(operation),
|
||||
};
|
||||
|
||||
Reference in new issue
Block a user