fix(ci): repair boundary regressions and evidence assertions

Detach cancelled CDP sessions within the cleanup budget and retain event-finalization failures across retries. Honor Git filesystem encodings and exclude the promise-API probe from source-read failures.

Align existing integration regressions with retained process resources, filesystem-order-independent truncation, owned skill files, explicit native access facts and streamed JSON errors.

Validation: 132 acceptance tests passed (2 skipped), 83 affected boundary tests passed, Git/JSON/CLI boundary regressions passed, check:ci, docs:check and test discovery passed. Local verification used Node 24.18.0 on macOS; cross-platform CI remains pending.
This commit is contained in:
morluto 2026-10-11 01:00:30 +08:00
parent 603d04ee3a
commit 11e3fae697
14 changed files with 184 additions and 71 deletions

View File

@ -53,9 +53,24 @@ const resolveSourceHead = async (
...fs.promises,
// isomorphic-git treats every read failure as absence. Preserve real
// failures so a readable packed ref cannot hide an unreadable loose ref.
readFile: async (path: string) => {
readFile: async (
path: string | undefined,
options:
| BufferEncoding
| { readonly encoding?: BufferEncoding | null }
| null = null,
) => {
// The library probes readFile() without a path to detect promise APIs.
// Reject that invocation without attributing it to a source read.
if (path === undefined)
throw new TypeError("Git filesystem read requires a path");
try {
return await readRegularFile(path, { signal });
const bytes = await readRegularFile(path, { signal });
const encoding =
typeof options === "string" ? options : options?.encoding;
return encoding === undefined || encoding === null
? bytes
: bytes.toString(encoding);
} catch (cause: unknown) {
if (errorCode(cause) !== "ENOENT") failedRead ??= { cause };
throw cause;

View File

@ -31,7 +31,16 @@ const graphIdentities = async (source: string) => {
...(identity.strategy === "artifact-local-key"
? {
namespace: identity.namespace,
key: identity.key.replace(/^[0-9a-f]{64}:/u, ""),
// Module-export identity is a JSON tuple whose first entry
// binds the whole container; only that digest changes here.
key:
identity.namespace === "module-export"
? JSON.stringify(
(JSON.parse(identity.key) as unknown[]).slice(1),
)
: identity.namespace === "source-module"
? identity.key.replace(/^[0-9a-f]{64}:/u, "")
: identity.key,
}
: {}),
}))

View File

@ -68,18 +68,20 @@ export const closeCdpTargetSession = async (
// Best-effort domain cleanup; transport close is the definitive boundary.
}
}
if (!cleanup.signal.aborted && sessionId !== undefined)
try {
await connection.send(
"Target.detachFromTarget",
{ sessionId },
undefined,
cleanup.signal,
);
} catch {
// Best-effort detach; transport close is the definitive boundary.
}
}
// Cancellation skips optional domain shutdown but still releases the
// attached session using its independent, bounded cleanup signal.
if (!cleanup.signal.aborted && sessionId !== undefined)
try {
await connection.send(
"Target.detachFromTarget",
{ sessionId },
undefined,
cleanup.signal,
);
} catch {
// Best-effort detach; transport close is the definitive boundary.
}
} finally {
clearTimeout(timeout);
cleanup.abort();

View File

@ -42,7 +42,6 @@ export class PlaywrightScenarioBrowserCleanupOwner {
#profileRemoved: boolean;
#closePromise: Promise<void> | undefined;
#eventFinalizationPromise: Promise<void> | undefined;
#eventFinalizationFailureReported = false;
/** Keep the narrow provider actions needed to release one opened browser. */
constructor(private readonly resources: PlaywrightScenarioCleanupResources) {
@ -98,10 +97,10 @@ export class PlaywrightScenarioBrowserCleanupOwner {
try {
await eventFinalization;
} catch (cause: unknown) {
if (!this.#eventFinalizationFailureReported) {
failures.push(cause);
this.#eventFinalizationFailureReported = true;
}
// A failed finalization stays failed: the memoized attempt rejects on
// every retry so cleanup can never report success for unfinalized
// events.
failures.push(cause);
}
if (!this.#browserClosed) {
try {

View File

@ -23,7 +23,12 @@ it("preserves a 12,000-member native import through both analyzers", () => {
const ir = analyzeParsedJavaScriptSemantics(parsed);
expect(staticAnalysis.parse_status).toBe("complete");
expect(staticAnalysis.electron.native_addon_bindings).toEqual([
expect.objectContaining({ specifier: "./addon.node", members: ["last"] }),
expect.objectContaining({
specifier: "./addon.node",
members: ["next"],
namespace_access: false,
dynamic_member_access: false,
}),
]);
expect(ir.coverage).toEqual({ status: "complete", omittedCount: 0 });
const path = origin(onlyBinding(ir, "value"));

View File

@ -16,29 +16,41 @@ import { classifyFilesystemEffects } from "./ProcessFilesystemEffects.js";
it.each(["after", "before"] as const)(
"keeps absence unknown when the %s snapshot omits an existing file",
async (partialSide) => {
// Enumeration order is filesystem-dependent. Choose a genuinely omitted
// entry by comparing a complete snapshot with a capacity-limited one.
const root = await createTestTempDirectory("rea-fs-partial-absence-");
await writeFile(join(root, "z.txt"), "unchanged");
await writeFile(join(root, "a1.txt"), "first");
await writeFile(join(root, "a2.txt"), "second");
await writeFile(join(root, "z.txt"), "target");
const scenario = parseProcessScenario({
executable: process.execPath,
working_directory: root,
filesystem_observation_paths: [root],
limits: { files: 2 },
});
if (partialSide === "before") await writeFile(join(root, "a.txt"), "other");
const before = await snapshotRoots(scenario);
if (partialSide === "after") await writeFile(join(root, "a.txt"), "other");
else await rm(join(root, "a.txt"));
const after = await snapshotRoots(scenario);
expect(await readFile(join(root, "z.txt"), "utf8")).toBe("unchanged");
const complete = await snapshotRoots({
...scenario,
limits: { ...scenario.limits, files: 4 },
});
const partial = await snapshotRoots(scenario);
expect(complete.completeRoots).toEqual(["root_0"]);
expect(partial.truncated).toBe(true);
const omitted = complete.files.find(
(file) => !partial.files.some(({ path }) => path === file.path),
);
if (omitted === undefined) throw new Error("Expected an omitted entry");
const before = partialSide === "before" ? partial : complete;
const after = partialSide === "after" ? partial : complete;
const effect = classifyFilesystemEffects(before, after).find(
({ path }) => path === "root_0:z.txt",
({ path }) => path === omitted.path,
);
expect(effect?.status).toBe("unknown");
expect(effect).toMatchObject(
partialSide === "after"
? { before: { path: "root_0:z.txt" }, after: null }
: { before: null, after: { path: "root_0:z.txt" } },
);
expect(effect).toMatchObject({
...(partialSide === "after"
? { before: omitted, after: null }
: { before: null, after: omitted }),
reason: expect.stringContaining("did not exhaust path enumeration"),
});
},
);

View File

@ -18,6 +18,7 @@ import {
} from "./ProcessCaptureError.js";
import {
releaseProcessResources,
ProcessCaptureResourceScope,
createProcessCaptureObservationBuffer,
prepareProcessCapture,
resolveProcessResult,
@ -291,26 +292,31 @@ it("keeps empty cleanup exception messages actionable in the report", async () =
});
it.each(["termination", "verification"] as const)(
"retains observations and releases the temporary root when process %s throws",
"retains observations and retries the temporary root when process %s throws",
async (stage) => {
const cleanupFailure = new Error(`${stage} inspection failed`);
const removeTemporaryRoot = vi.fn(async () => undefined);
let failing = true;
const scope = new ProcessCaptureResourceScope();
const host: ProcessCaptureCleanupHost = {
platform: "linux",
cleanupProcessGroup: async () => {
if (stage === "termination") throw cleanupFailure;
if (failing && stage === "termination") throw cleanupFailure;
return {
cleaned: true,
signaled: true,
unverified: [{ pid: 900, diagnostic: "environment unavailable" }],
unverified: failing
? [{ pid: 900, diagnostic: "environment unavailable" }]
: [],
};
},
verifyTokenOwnedProcesses: async () => {
throw cleanupFailure;
if (failing) throw cleanupFailure;
return { cleaned: true, signaled: false };
},
removeTemporaryRoot,
};
const cleanup = await releaseProcessResources({
const cleanup = await scope.release({
timers: new Set(),
terminal: { pid: 321 },
renderer: undefined,
@ -319,8 +325,11 @@ it.each(["termination", "verification"] as const)(
host,
});
expect(removeTemporaryRoot).toHaveBeenCalledWith("/fixture/root");
expect(cleanup.temporary_root).toEqual({ state: "cleaned", reason: null });
expect(removeTemporaryRoot).not.toHaveBeenCalled();
expect(cleanup.temporary_root).toEqual({
state: "unverified",
reason: `temporary root retained because owned process cleanup is failed: ${cleanupFailure.message}`,
});
expect(cleanup.owned_process_group).toMatchObject({
state: "failed",
reason: cleanupFailure.message,
@ -357,6 +366,13 @@ it.each(["termination", "verification"] as const)(
},
});
}
failing = false;
await scope.retryCleanup();
expect(removeTemporaryRoot).toHaveBeenCalledExactlyOnceWith(
"/fixture/root",
);
await scope.close();
expect(removeTemporaryRoot).toHaveBeenCalledTimes(1);
},
);

View File

@ -8,23 +8,18 @@ import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
import { cliTest } from "../../support/cli/cliFixture.js";
cliTest(
"rejects malformed oversized CLI input and preserves Evidence decoder resource failures",
"rejects malformed oversized JSON through both CLI and Evidence readers",
async ({ cli, processes }) => {
const root = await createTestTempDirectory("rea-json-string-limit-");
const input = join(root, "oversized.json");
const file = await open(input, "wx");
try {
// Sparse zero bytes are valid UTF-8. The Evidence reader reaches the
// decoded-length limit; the streamed CLI rejects their invalid JSON syntax.
// Sparse zero bytes are valid UTF-8 but invalid JSON. Both readers
// stream the input and reject the syntax before assembling a value.
await file.truncate(constants.MAX_STRING_LENGTH + 1);
} finally {
await file.close();
}
const limits = {
input_path: input,
input_bytes: constants.MAX_STRING_LENGTH + 1,
max_string_code_units: constants.MAX_STRING_LENGTH,
};
// Run readers sequentially in owned child processes, releasing each large
// read buffer when its process exits instead of retaining it in Vitest.
const result = await cli.run({
@ -48,11 +43,11 @@ cliTest(
input,
]);
expect(JSON.parse(evidence.stdout)).toMatchObject({
code: "resource_constraint",
code: "evidence_integrity_mismatch",
details: {
operation: "read_evidence_file",
resource: "memory",
reported_limits: limits,
operation: "read",
reason: "invalid-json",
path: input,
},
});
expect(evidence.stderr).toBe("");

View File

@ -17,6 +17,7 @@ import { parse as parseJsonc, type ParseError } from "jsonc-parser";
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
import { clearClientLocationEnvironment } from "../../fixtures/clientEnvironment.js";
import { supportedClients } from "../../../src/application/SupportedClients.js";
import { PRODUCT_IDENTITY } from "../../../src/identity.js";
import { readClientRegistrationStatuses } from "../../../src/application/ClientRegistrationStatus.js";
import { configureClientConfiguration } from "../../../src/application/SetupClientConfiguration.js";
@ -31,6 +32,7 @@ import {
} from "../../../src/application/Uninstall.js";
const roots: string[] = [];
const ownedSkillManifest = `---\nname: ${PRODUCT_IDENTITY.skillName}\n---\n# REA\n`;
beforeEach(clearClientLocationEnvironment);
afterEach(async () => {
vi.unstubAllEnvs();
@ -1353,7 +1355,7 @@ describe("client configuration filesystem removal", () => {
},
}),
);
await writeFile(skill, "managed");
await writeFile(skill, ownedSkillManifest);
await mkdir(join(home, ".rea"), { recursive: true });
await import("node:fs/promises").then(({ symlink }) =>
symlink(home, join(home, ".rea/cache")),
@ -1422,7 +1424,9 @@ describe("client configuration filesystem removal", () => {
readFile(`${configPath}.rea.backup`, "utf8"),
).rejects.toThrow();
});
});
describe("uninstall configuration preflight", () => {
it("fails closed on malformed client configuration", async () => {
const home = await createTestTempDirectory("rea-uninstall-bad-");
roots.push(home);
@ -1447,8 +1451,12 @@ describe("client configuration filesystem removal", () => {
roots.push(home);
const codex = join(home, ".codex/config.toml");
const cursor = join(home, ".cursor/mcp.json");
const skillMarker = join(
home,
".agents/skills/reverse-engineer-anything/marker.txt",
);
const managed = [
join(home, ".agents/skills/reverse-engineer-anything/marker.txt"),
skillMarker,
join(home, ".rea/cache/marker.txt"),
join(home, ".rea/state/marker.txt"),
];
@ -1463,6 +1471,8 @@ describe("client configuration filesystem removal", () => {
await writeFile(codex, "[mcp_servers.rea]\ncommand = [broken\n");
await writeFile(cursor, cursorConfig);
for (const path of managed) await writeFile(path, "marker");
const manifest = join(dirname(skillMarker), "SKILL.md");
await writeFile(manifest, ownedSkillManifest);
const stopped = await runUninstall(true, systemUninstallHost(home));
@ -1478,6 +1488,7 @@ describe("client configuration filesystem removal", () => {
expect(await readFile(cursor, "utf8")).toBe(cursorConfig);
for (const path of managed)
expect(await readFile(path, "utf8")).toBe("marker");
expect(await readFile(manifest, "utf8")).toBe(ownedSkillManifest);
await writeFile(codex, "");
const repaired = await runUninstall(true, systemUninstallHost(home));
@ -1485,7 +1496,12 @@ describe("client configuration filesystem removal", () => {
expect(JSON.parse(await readFile(cursor, "utf8"))).toEqual({
mcpServers: { other: { command: "echo", args: ["hello"] } },
});
for (const path of managed)
// Uninstall removes only the managed bundle, never arbitrary skill files.
expect(await readFile(skillMarker, "utf8")).toBe("marker");
await expect(readFile(manifest, "utf8")).rejects.toMatchObject({
code: "ENOENT",
});
for (const path of managed.slice(1))
await expect(readFile(path, "utf8")).rejects.toThrow();
});
});
@ -1625,6 +1641,7 @@ describe("client configuration filesystem failure reporting", () => {
roots.push(home);
const skillRoot = join(home, ".agents/skills/reverse-engineer-anything");
await mkdir(skillRoot, { recursive: true });
await writeFile(join(skillRoot, "SKILL.md"), ownedSkillManifest);
const result = await runUninstall(
false,
systemUninstallHost(home, {
@ -1637,11 +1654,14 @@ describe("client configuration filesystem failure reporting", () => {
name: "skill",
status: "failed",
detail: expect.stringContaining(
"This item could not be removed. Check file permissions, then rerun uninstall.",
`Managed file could not be removed: ${join(skillRoot, "SKILL.md")}`,
),
}),
);
expect(JSON.stringify(result)).not.toContain("SECRET");
expect(await readFile(join(skillRoot, "SKILL.md"), "utf8")).toBe(
ownedSkillManifest,
);
});
it("uninstalls all managed skill roots, including a custom Claude Code directory", async () => {
@ -1653,7 +1673,11 @@ describe("client configuration filesystem failure reporting", () => {
join(customClaudeDirectory, "skills/reverse-engineer-anything"),
];
await Promise.all(
rootsToRemove.map((path) => mkdir(path, { recursive: true })),
rootsToRemove.map(async (path) => {
await mkdir(path, { recursive: true });
await writeFile(join(path, "SKILL.md"), ownedSkillManifest);
await writeFile(join(path, "user-notes.txt"), "user-authored");
}),
);
const result = await systemUninstallHost(home, testFileSystem, {
@ -1663,7 +1687,12 @@ describe("client configuration filesystem failure reporting", () => {
expect(result).toMatchObject({ status: "removed" });
await Promise.all(
rootsToRemove.map(async (path) => {
await expect(lstat(path)).rejects.toMatchObject({ code: "ENOENT" });
await expect(
readFile(join(path, "SKILL.md"), "utf8"),
).rejects.toMatchObject({ code: "ENOENT" });
expect(await readFile(join(path, "user-notes.txt"), "utf8")).toBe(
"user-authored",
);
}),
);
});

View File

@ -84,6 +84,14 @@ it("reads a linked worktree's own branch and detached HEAD through shared Git re
head: mainHead,
dirty: null,
});
// A non-regular loose ref must remain unknown even when packed-refs has
// a readable value. The library's promise-API probe is not a source failure.
await mkdir(join(repository, ".git", "refs", "heads", "main"));
expect(await readReferenceSourceVcs(repository)).toEqual({
kind: "unknown",
head: null,
dirty: null,
});
const controller = new AbortController();
controller.abort();
expect(await readReferenceSourceVcs(linked, controller.signal)).toEqual({

View File

@ -59,7 +59,10 @@ const expectUnverifiedHostCleanup = (error: ProcessCaptureError): void => {
/^(?:[1-9][0-9]*=(?:environment_unavailable|environment_errno_(?:EACCES|EPERM)))(?:, [1-9][0-9]*=(?:environment_unavailable|environment_errno_(?:EACCES|EPERM)))*$/u,
);
expect(report?.terminal_renderer.state).toBe("cleaned");
expect(report?.temporary_root.state).toBe("cleaned");
expect(report?.temporary_root).toEqual({
state: "unverified",
reason: `temporary root retained because owned process cleanup is unverified: ${report?.owned_process_group.reason}`,
});
};
const captureObservations = (
@ -275,7 +278,10 @@ itWithCaptureCapability(
const projected = projectAnalysisError(result.error);
expect(projected).toMatchObject({
code: "cleanup_incomplete",
details: { cleanup: "incomplete", resources: ["owned_process_group"] },
details: {
cleanup: "incomplete",
resources: ["owned_process_group", "temporary_root"],
},
});
expect(projected).not.toHaveProperty("stack");
expect(projected).not.toHaveProperty("cause");

View File

@ -169,7 +169,13 @@ itWithCaptureCapability.each(["cli", "mcp"] as const)(
await writeFile(join(root, "z.txt"), "unchanged");
const scenario = {
executable: process.execPath,
arguments: ["-e", "require('node:fs').writeFileSync('a.txt','added')"],
// Delete the known file and overflow the remaining directory capacity.
// Incomplete enumeration still cannot prove its absence, regardless of
// which new entry the filesystem returns first.
arguments: [
"-e",
"const fs=require('node:fs');fs.unlinkSync('z.txt');fs.writeFileSync('a.txt','added');fs.writeFileSync('b.txt','added');",
],
working_directory: root,
filesystem_observation_paths: [root],
limits: { files: 2 },
@ -179,7 +185,9 @@ itWithCaptureCapability.each(["cli", "mcp"] as const)(
);
const capture = parseProcessCapture(evidence.normalized_result);
expect(capture.truncated).toBe(true);
expect(await readFile(join(root, "z.txt"), "utf8")).toBe("unchanged");
await expect(readFile(join(root, "z.txt"), "utf8")).rejects.toMatchObject({
code: "ENOENT",
});
expect(
capture.filesystem_effects.find(({ path }) => path === "root_0:z.txt")
?.status,

View File

@ -194,9 +194,14 @@ itWithCaptureCapability.each(["cli", "mcp"] as const)(
expect(details.filesystem_after.enumeration_reasons).toContain(
"files_limit",
);
expect(
result.files_after.find(({ path }) => path === "root_0:b.txt")?.sha256,
).toBeNull();
// The truncated subset follows filesystem enumeration order. The large
// file may be retained without a digest, or not enumerated at all.
const largeFile = result.files_after.find(
({ path }) => path === "root_0:b.txt",
);
if (largeFile !== undefined) expect(largeFile.sha256).toBeNull();
expect(details.filesystem_after.enumeration_truncated).toBe(true);
expect(result.files_after.length).toBeLessThanOrEqual(3);
expect(processSourceTruncated(result, "terminal_rendered")).toBe(true);
expect(compareProcessCaptures(result, result)).toMatchObject({
terminal: "unknown",

View File

@ -72,12 +72,16 @@ describe("static Electron application analysis", () => {
const result = await reconstructJavaScriptArtifact({ input_path: root });
const graph = parseJavaScriptApplicationGraph(result.graph);
const requestedMembers = graph.nodes.flatMap((node) =>
// Computed access keeps no known members; the unknown access is an
// explicit dynamic flag rather than a "*" sentinel inside members.
const accesses = graph.nodes.flatMap((node) =>
node.kind === "native-export"
? node.observations.flatMap(({ properties }) => {
const members = properties.requested_members;
return Array.isArray(members) ? members : [];
})
? node.observations.map(({ properties }) => ({
members: Array.isArray(properties.requested_members)
? properties.requested_members
: [],
dynamicMemberAccess: properties.dynamic_member_access ?? false,
}))
: [],
);
@ -86,7 +90,7 @@ describe("static Electron application analysis", () => {
native_addon_bindings: 1,
resolved_native_addon_bindings: 1,
});
expect(requestedMembers).toEqual(["*"]);
expect(accesses).toEqual([{ members: [], dynamicMemberAccess: true }]);
});
it("maps Electron boundaries through bundler-renamed bindings", async () => {