fix(providers): retain composite cleanup failures and probe child liveness

This commit is contained in:
morluto 2026-10-14 09:47:26 +08:00
parent fab33a66c6
commit 31b99306aa
3 changed files with 259 additions and 33 deletions

View File

@ -116,9 +116,7 @@ describe("BinarySession open liveness", () => {
expect(deep.created).toHaveLength(2);
});
it("falls back to the health probe when the child reports no operation health", async () => {
// Without an operation-health hook there is no child-liveness signal,
// so the probe keeps the previous `health` admission behavior.
it("probes instantiated children through health when snapshots are absent", async () => {
const { session, deep } = openSession({ operationHealth: false });
expect((await session.open(process.execPath)).ok).toBe(true);
expect((await session.execute("address_name", {})).ok).toBe(true);
@ -126,9 +124,33 @@ describe("BinarySession open liveness", () => {
const second = await session.open(process.execPath);
expect(second.ok).toBe(true);
// The composite's `health` answers ok, so the live client is reused.
// The composite probes this already-instantiated child rather than
// answering health without contacting it.
expect(deep.created).toHaveLength(1);
expect(deep.created[0]?.closeCalled).toBe(false);
expect(deep.created[0]?.healthCalls).toBe(1);
});
it("retires a dead no-snapshot child after its health probe fails", async () => {
const { session, deep } = openSession({ operationHealth: false });
expect((await session.open(process.execPath)).ok).toBe(true);
expect((await session.execute("address_name", {})).ok).toBe(true);
const dead = deep.created[0];
dead?.kill();
expect((await session.open(process.execPath)).ok).toBe(true);
expect(dead?.healthCalls).toBe(1);
expect(dead?.closeCalled).toBe(true);
expect(deep.created).toHaveLength(1);
expect((await session.execute("address_name", {})).ok).toBe(true);
expect(deep.created).toHaveLength(2);
});
it("does not instantiate a child to answer health before first use", async () => {
const { session, deep } = openSession({ operationHealth: false });
expect((await session.open(process.execPath)).ok).toBe(true);
expect((await session.open(process.execPath)).ok).toBe(true);
expect(deep.created).toEqual([]);
});
it("reports cancellation without touching the session when aborted mid-probe", async () => {
@ -167,6 +189,7 @@ describe("BinarySession open liveness", () => {
interface KillableChild extends AnalysisClient {
kill(state?: "exited" | "unreachable" | "unknown" | "not_started"): void;
closeCalled: boolean;
healthCalls: number;
}
const deepIdentity: ProviderIdentity = {
@ -269,10 +292,12 @@ const killableDeepProvider = (options: {
createClient: (): KillableChild => {
let alive = true;
let closeCalled = false;
let healthCalls = 0;
let deadState: "exited" | "unreachable" | "unknown" | "not_started" =
"exited";
const client: KillableChild = {
execute: (operation) => {
if (operation === "health") healthCalls += 1;
return alive
? Promise.resolve(
ok(
@ -313,6 +338,9 @@ const killableDeepProvider = (options: {
get closeCalled() {
return closeCalled;
},
get healthCalls() {
return healthCalls;
},
kill: (
state:
| "exited"

View File

@ -0,0 +1,129 @@
import { expect, it, vi } from "vitest";
import {
createAnalysisExecution,
type AnalysisClient,
type AnalysisOperation,
type AnalysisProvider,
type CapabilityDescriptor,
type ProviderIdentity,
} from "../AnalysisProvider.js";
import { ProviderCleanupError } from "../../domain/providerCleanupError.js";
import { err, ok } from "../../domain/result.js";
import { CompositeProvider } from "./CompositeProvider.js";
const identity = (id: string): ProviderIdentity => ({
id,
name: id,
version: "1",
});
const operationCapability = (
provider: ProviderIdentity,
operation: Exclude<AnalysisOperation, "health">,
): CapabilityDescriptor => ({
provider,
operation,
available: true,
reason: null,
effects: {
mutatesArtifact: false,
launchesProcess: false,
mayShowUi: false,
mayAccessNetwork: false,
mayWriteFilesystem: false,
changesPermissions: false,
requiresRoot: false,
},
limitations: [],
});
const failingProvider = (
id: string,
operation: Exclude<AnalysisOperation, "health">,
resource: string,
) => {
const providerIdentity = identity(id);
const cleanupError = new ProviderCleanupError(id, [resource], {
reason: `${id} cleanup is unresolved`,
child_marker: `${id}-detail`,
});
const close = vi.fn(async () => err(cleanupError));
const provider: AnalysisProvider = {
identity: () => providerIdentity,
capabilities: () => [operationCapability(providerIdentity, operation)],
createClient: (): AnalysisClient => ({
execute: (name) =>
Promise.resolve(
ok(createAnalysisExecution(`${id}:${name}`, providerIdentity)),
),
close,
}),
};
return { provider, close, cleanupError };
};
const executableTarget = {
path: "/tmp/fixture",
sha256: "a".repeat(64),
kind: "executable",
format: "elf",
architecture: "x86_64",
availableArchitectures: ["x86_64"],
} as const;
it("retains every failed child resource across composite cleanup retries", async () => {
const left = failingProvider("provider-left", "address_name", "left-db");
const right = failingProvider("provider-right", "procedure_info", "right-db");
const client = new CompositeProvider([
left.provider,
right.provider,
]).createClient(executableTarget);
expect((await client.execute("address_name", {})).ok).toBe(true);
expect((await client.execute("procedure_info", {})).ok).toBe(true);
for (let attempt = 0; attempt < 2; attempt += 1) {
const closed = await client.close();
expect(closed.ok).toBe(false);
if (closed.ok) throw new Error("Expected composite cleanup failure");
expect(closed.error).toMatchObject({
cleanupIncomplete: true,
cleanupResources: ["left-db", "right-db"],
diagnostics: {
failures: [
{ provider_id: "provider-left", resources: ["left-db"] },
{ provider_id: "provider-right", resources: ["right-db"] },
],
},
});
expect(closed.error).toBeInstanceOf(ProviderCleanupError);
expect(closed.error.diagnostics).toMatchObject({
failures: [
{
diagnostics: { reason: "provider-left cleanup is unresolved" },
},
{
diagnostics: { reason: "provider-right cleanup is unresolved" },
},
],
});
}
expect(left.close).toHaveBeenCalledTimes(2);
expect(right.close).toHaveBeenCalledTimes(2);
});
it("preserves one child's typed cleanup error unchanged", async () => {
const child = failingProvider("provider-only", "address_name", "one-db");
const client = new CompositeProvider([child.provider]).createClient(
executableTarget,
);
expect((await client.execute("address_name", {})).ok).toBe(true);
const closed = await client.close();
expect(closed.ok).toBe(false);
if (closed.ok) throw new Error("Expected child cleanup failure");
expect(closed.error).toBe(child.cleanupError);
});

View File

@ -1,4 +1,6 @@
import { AnalysisCapabilityUnavailableError } from "../../domain/analysisErrorCore.js";
import { ProviderCleanupError } from "../../domain/providerCleanupError.js";
import { ProviderAdapterError } from "../../domain/providerAdapterError.js";
import { err, ok } from "../../domain/result.js";
import type { BinaryTarget } from "../../domain/binaryTargetTypes.js";
import type { AnalysisProfileCommitment } from "../../domain/analysisProfile.js";
@ -7,6 +9,7 @@ import type {
AnalysisClientContext,
AnalysisProvider,
CapabilityDescriptor,
ExecutionOptions,
ProviderIdentity,
} from "../AnalysisProvider.js";
import { closeAnalysisClient } from "./AnalysisClientCleanup.js";
@ -24,6 +27,88 @@ const compositeProviderIdentity = (
version: null,
});
const closeCompositeClients =
(
identity: ProviderIdentity,
clients: ReadonlyMap<AnalysisProvider, AnalysisClient>,
): AnalysisClient["close"] =>
async (options) => {
const outcomes = await Promise.all(
[...clients.entries()].map(([provider, client]) =>
closeAnalysisClient(client, provider.identity().id, options).then(
(result) => ({ providerId: provider.identity().id, result }),
),
),
);
const failures = outcomes.flatMap(({ providerId, result }) =>
result.ok ? [] : [{ providerId, error: result.error }],
);
if (failures.length === 0) return ok(null);
if (failures.length === 1) {
const failure = failures[0];
if (failure !== undefined) return err(failure.error);
}
const resources = [
...new Set(failures.flatMap(({ error }) => error.cleanupResources)),
];
return err(
new ProviderCleanupError(
identity.id,
resources,
{
reason:
"Multiple composed provider clients could not be confirmed closed.",
failures: failures.map(({ providerId, error }) => ({
provider_id: providerId,
message: error.message,
resources: [...error.cleanupResources],
...(error.diagnostics === undefined
? {}
: { diagnostics: error.diagnostics }),
})),
},
{
cause: new AggregateError(
failures.map(({ error }) => error),
"Composed provider cleanup failed.",
),
},
),
);
};
const executeCompositeHealth = async (
identity: ProviderIdentity,
clients: ReadonlyMap<AnalysisProvider, AnalysisClient>,
options: ExecutionOptions | undefined,
): ReturnType<AnalysisClient["execute"]> => {
for (const [provider, client] of clients) {
const health = client.operationHealthSnapshot?.();
if (
health !== undefined &&
health.state !== "idle" &&
health.state !== "busy"
)
return err(
new ProviderAdapterError(provider.identity().id, "health", {
diagnostics: {
reason: `instantiated provider client is ${health.state}`,
},
}),
);
const checked = await client.execute("health", {}, options);
if (!checked.ok) return checked;
}
return ok({
result: null,
rawResult: null,
provider: identity,
limitations: [],
locations: [],
subject: null,
});
};
/** Deterministically route disjoint operations without eagerly starting children. */
export class CompositeProvider implements AnalysisProvider {
readonly #identity: ProviderIdentity;
@ -85,14 +170,7 @@ export class CompositeProvider implements AnalysisProvider {
clients.set(provider, created);
return created;
};
const close: AnalysisClient["close"] = async (options) => {
const outcomes = await Promise.all(
[...clients.entries()].map(([provider, client]) =>
closeAnalysisClient(client, provider.identity().id, options),
),
);
return outcomes.find((outcome) => !outcome.ok) ?? ok(null);
};
const close = closeCompositeClients(this.#identity, clients);
return {
executeIfReady: (operation, parameters, options) => {
const provider = this.#providerByOperation.get(operation);
@ -103,18 +181,9 @@ export class CompositeProvider implements AnalysisProvider {
Promise.resolve(undefined)
);
},
execute: (operation, parameters, options) => {
execute: async (operation, parameters, options) => {
if (operation === "health")
return Promise.resolve(
ok({
result: null,
rawResult: null,
provider: this.#identity,
limitations: [],
locations: [],
subject: null,
}),
);
return executeCompositeHealth(this.#identity, clients, options);
const provider = this.#providerByOperation.get(operation);
return provider === undefined
? Promise.resolve(
@ -141,16 +210,16 @@ export class CompositeProvider implements AnalysisProvider {
compareUnicodeCodePoints(left.provider.id, right.provider.id),
),
operationHealthSnapshot: () => {
for (const client of clients.values()) {
const snapshot = client.operationHealthSnapshot?.();
if (snapshot !== undefined && snapshot.state !== "idle")
return snapshot;
}
for (const client of clients.values()) {
const snapshot = client.operationHealthSnapshot?.();
if (snapshot !== undefined) return snapshot;
}
const snapshots = [...clients.values()].map((client) =>
client.operationHealthSnapshot?.(),
);
if (snapshots.some((snapshot) => snapshot === undefined))
return undefined;
return (
snapshots.find(
(snapshot) => snapshot !== undefined && snapshot.state !== "idle",
) ?? snapshots[0]
);
},
close,
};