From 69485e27f7bee406f79a6cf862c4431ea3bc9e64 Mon Sep 17 00:00:00 2001 From: N0zoM1z0 <1705004398@qq.com> Date: Sat, 10 Oct 2026 16:34:17 +0000 Subject: [PATCH] fix(ci): restore capture and maintenance regression checks --- src/apktool/ApktoolProvider.ts | 2 +- src/application/UpdateMaintenance.ts | 4 ++- ...aScriptApplicationTemplateLiterals.test.ts | 11 +++++- src/browser/CdpTargetSession.ts | 22 ++++++------ .../PlaywrightBrowserScenarioProvider.test.ts | 7 ++-- ...criptSemanticAnalysis.member_depth.test.ts | 3 +- .../capture/FilesystemCoverage.test.ts | 26 ++++++++------ src/process/capture/ProcessHarness.test.ts | 9 +++-- .../filesystem/clientLifecycle.test.ts | 35 +++++++++++++++---- .../process/processCaptureLifecycle.test.ts | 5 ++- .../processFilesystemCoverageAdapters.test.ts | 13 ++++--- .../process/processTruncationAdapters.test.ts | 6 +++- .../process-global/updateMaintenance.test.ts | 35 +++++++++++++++---- 13 files changed, 127 insertions(+), 51 deletions(-) diff --git a/src/apktool/ApktoolProvider.ts b/src/apktool/ApktoolProvider.ts index 3bcb98510..d898b785a 100644 --- a/src/apktool/ApktoolProvider.ts +++ b/src/apktool/ApktoolProvider.ts @@ -157,7 +157,7 @@ const runApktool = async ( timeoutMs: limits.timeoutMs, diagnosticBytes: limits.maxOutputBytes, }, - (signal === undefined ? {} : { signal }), + signal === undefined ? {} : { signal }, ); return ok({ stdout: output.stdout.text, stderr: output.stderr.text }); } catch (cause) { diff --git a/src/application/UpdateMaintenance.ts b/src/application/UpdateMaintenance.ts index 40d66bcbc..51ae5342f 100644 --- a/src/application/UpdateMaintenance.ts +++ b/src/application/UpdateMaintenance.ts @@ -130,7 +130,9 @@ export const planIntegrationMaintenance = async ( }; if (plan.data.plannedActions.length === 0) return { status: "current", plannedActions: [] }; - const allowedIds = new Set(scope.clients.map((client) => `configure_client:${client}`)); + const allowedIds = new Set( + scope.clients.map((client) => `configure_client:${client}`), + ); const skillTargets = new Set(skillDestinations.map(({ path }) => path)); if ( plan.data.plannedActions.some((action) => diff --git a/src/application/javascript/JavaScriptApplicationTemplateLiterals.test.ts b/src/application/javascript/JavaScriptApplicationTemplateLiterals.test.ts index 3988a1a7f..56e2c3a1e 100644 --- a/src/application/javascript/JavaScriptApplicationTemplateLiterals.test.ts +++ b/src/application/javascript/JavaScriptApplicationTemplateLiterals.test.ts @@ -2,6 +2,7 @@ import { writeFile } from "node:fs/promises"; import { join } from "node:path"; import { expect, it } from "vitest"; +import { z } from "zod"; import { createTestTempDirectory } from "../../../tests/fixtures/temporaryDirectory.js"; import { javascriptApplicationAnalysisResultSchema } from "../../domain/javascript/javascriptApplicationAnalysis.js"; @@ -31,7 +32,15 @@ const graphIdentities = async (source: string) => { ...(identity.strategy === "artifact-local-key" ? { namespace: identity.namespace, - key: identity.key.replace(/^[0-9a-f]{64}:/u, ""), + key: + identity.namespace === "module-export" + ? JSON.stringify( + z + .array(z.string()) + .parse(JSON.parse(identity.key)) + .slice(1), + ) + : identity.key.replace(/^[0-9a-f]{64}:/u, ""), } : {}), })) diff --git a/src/browser/CdpTargetSession.ts b/src/browser/CdpTargetSession.ts index a76e2a5a0..588f83e94 100644 --- a/src/browser/CdpTargetSession.ts +++ b/src/browser/CdpTargetSession.ts @@ -68,18 +68,18 @@ 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. - } } + 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(); diff --git a/src/browser/PlaywrightBrowserScenarioProvider.test.ts b/src/browser/PlaywrightBrowserScenarioProvider.test.ts index af94b7f90..eb4187292 100644 --- a/src/browser/PlaywrightBrowserScenarioProvider.test.ts +++ b/src/browser/PlaywrightBrowserScenarioProvider.test.ts @@ -553,7 +553,7 @@ it("shares concurrent close and retries browser cleanup after failure", async () expect(cleanupSettlements).toEqual([false, true]); }); -it("keeps failed event finalization failed across cleanup retries", async () => { +it("reports event finalization failure before allowing settled cleanup retries", async () => { const finishEvents = vi .fn<() => Promise>() .mockRejectedValue(new Error("event finalization failed")); @@ -567,10 +567,9 @@ it("keeps failed event finalization failed across cleanup retries", async () => await expect(cleanup.close(finishEvents)).rejects.toMatchObject({ reason: "cleanup_failed", + cleanup: { reason: "event finalization failed" }, }); - await expect(cleanup.close(finishEvents)).rejects.toMatchObject({ - reason: "cleanup_failed", - }); + await expect(cleanup.close(finishEvents)).resolves.toBeUndefined(); expect(finishEvents).toHaveBeenCalledTimes(1); expect(closeBrowser).toHaveBeenCalledTimes(1); }); diff --git a/src/domain/javascript/javascriptSemanticAnalysis.member_depth.test.ts b/src/domain/javascript/javascriptSemanticAnalysis.member_depth.test.ts index fa2eafa8e..4ee60cae7 100644 --- a/src/domain/javascript/javascriptSemanticAnalysis.member_depth.test.ts +++ b/src/domain/javascript/javascriptSemanticAnalysis.member_depth.test.ts @@ -22,8 +22,9 @@ it("preserves a 12,000-member native import through both analyzers", () => { const staticAnalysis = analyzeParsedJavaScriptStaticSource("", parsed); const ir = analyzeParsedJavaScriptSemantics(parsed); expect(staticAnalysis.parse_status).toBe("complete"); + // The addon exports `next`; `last` belongs to an object reached from it. expect(staticAnalysis.electron.native_addon_bindings).toEqual([ - expect.objectContaining({ specifier: "./addon.node", members: ["last"] }), + expect.objectContaining({ specifier: "./addon.node", members: ["next"] }), ]); expect(ir.coverage).toEqual({ status: "complete", omittedCount: 0 }); const path = origin(onlyBinding(ir, "value")); diff --git a/src/process/capture/FilesystemCoverage.test.ts b/src/process/capture/FilesystemCoverage.test.ts index 2eec44bd5..22cfa4450 100644 --- a/src/process/capture/FilesystemCoverage.test.ts +++ b/src/process/capture/FilesystemCoverage.test.ts @@ -17,27 +17,33 @@ it.each(["after", "before"] as const)( "keeps absence unknown when the %s snapshot omits an existing file", async (partialSide) => { const root = await createTestTempDirectory("rea-fs-partial-absence-"); - await writeFile(join(root, "z.txt"), "unchanged"); + const names = ["a.txt", "z.txt"]; + for (const name of names) await writeFile(join(root, name), "unchanged"); 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 partial = await snapshotRoots(scenario); + const omitted = names.find( + (name) => !partial.files.some(({ path }) => path === `root_0:${name}`), + ); + if (omitted === undefined) + throw new Error("Expected an omitted fixture file"); + for (const name of names) if (name !== omitted) await rm(join(root, name)); + const complete = await snapshotRoots(scenario); + const before = partialSide === "before" ? partial : complete; + const after = partialSide === "after" ? partial : complete; + expect(await readFile(join(root, omitted), "utf8")).toBe("unchanged"); const effect = classifyFilesystemEffects(before, after).find( - ({ path }) => path === "root_0:z.txt", + ({ path }) => path === `root_0:${omitted}`, ); 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" } }, + ? { before: { path: `root_0:${omitted}` }, after: null } + : { before: null, after: { path: `root_0:${omitted}` } }, ); }, ); diff --git a/src/process/capture/ProcessHarness.test.ts b/src/process/capture/ProcessHarness.test.ts index 65ca8d3d5..be74741c2 100644 --- a/src/process/capture/ProcessHarness.test.ts +++ b/src/process/capture/ProcessHarness.test.ts @@ -291,7 +291,7 @@ 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 the temporary root when process %s throws", async (stage) => { const cleanupFailure = new Error(`${stage} inspection failed`); const removeTemporaryRoot = vi.fn(async () => undefined); @@ -319,8 +319,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, diff --git a/tests/boundary/filesystem/clientLifecycle.test.ts b/tests/boundary/filesystem/clientLifecycle.test.ts index 62efeddd7..38f054d12 100644 --- a/tests/boundary/filesystem/clientLifecycle.test.ts +++ b/tests/boundary/filesystem/clientLifecycle.test.ts @@ -31,6 +31,8 @@ import { } from "../../../src/application/Uninstall.js"; const roots: string[] = []; +const managedSkillManifest = + "---\nname: reverse-engineer-anything\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, managedSkillManifest); await mkdir(join(home, ".rea"), { recursive: true }); await import("node:fs/promises").then(({ symlink }) => symlink(home, join(home, ".rea/cache")), @@ -1441,14 +1443,16 @@ describe("client configuration filesystem removal", () => { ); expect(await readFile(config, "utf8")).toBe("not-json"); }); +}); +describe("client configuration uninstall preflight", () => { it("stops before any removal when one client configuration is malformed", async () => { const home = await createTestTempDirectory("rea-uninstall-stop-"); roots.push(home); const codex = join(home, ".codex/config.toml"); const cursor = join(home, ".cursor/mcp.json"); const managed = [ - join(home, ".agents/skills/reverse-engineer-anything/marker.txt"), + join(home, ".agents/skills/reverse-engineer-anything/SKILL.md"), join(home, ".rea/cache/marker.txt"), join(home, ".rea/state/marker.txt"), ]; @@ -1462,7 +1466,11 @@ describe("client configuration filesystem removal", () => { await mkdir(dirname(path), { recursive: true }); await writeFile(codex, "[mcp_servers.rea]\ncommand = [broken\n"); await writeFile(cursor, cursorConfig); - for (const path of managed) await writeFile(path, "marker"); + for (const path of managed) + await writeFile( + path, + path === managed[0] ? managedSkillManifest : "marker", + ); const stopped = await runUninstall(true, systemUninstallHost(home)); @@ -1477,7 +1485,9 @@ 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(path, "utf8")).toBe( + path === managed[0] ? managedSkillManifest : "marker", + ); await writeFile(codex, ""); const repaired = await runUninstall(true, systemUninstallHost(home)); @@ -1625,6 +1635,8 @@ describe("client configuration filesystem failure reporting", () => { roots.push(home); const skillRoot = join(home, ".agents/skills/reverse-engineer-anything"); await mkdir(skillRoot, { recursive: true }); + const manifest = join(skillRoot, "SKILL.md"); + await writeFile(manifest, managedSkillManifest); const result = await runUninstall( false, systemUninstallHost(home, { @@ -1637,14 +1649,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: ${manifest}.`, ), }), ); expect(JSON.stringify(result)).not.toContain("SECRET"); }); - it("uninstalls all managed skill roots, including a custom Claude Code directory", async () => { + it("uninstalls owned skill files across roots while preserving other files", async () => { const home = await createTestTempDirectory("rea-uninstall-skills-"); roots.push(home); const customClaudeDirectory = join(home, "claude-config"); @@ -1655,6 +1667,10 @@ describe("client configuration filesystem failure reporting", () => { await Promise.all( rootsToRemove.map((path) => mkdir(path, { recursive: true })), ); + for (const path of rootsToRemove) { + await writeFile(join(path, "SKILL.md"), managedSkillManifest); + await writeFile(join(path, "user-notes.txt"), "user-authored notes"); + } const result = await systemUninstallHost(home, testFileSystem, { CLAUDE_CONFIG_DIR: customClaudeDirectory, @@ -1663,7 +1679,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"))).rejects.toMatchObject({ + code: "ENOENT", + }); + expect(await readFile(join(path, "user-notes.txt"), "utf8")).toBe( + "user-authored notes", + ); }), ); }); diff --git a/tests/boundary/process/processCaptureLifecycle.test.ts b/tests/boundary/process/processCaptureLifecycle.test.ts index b3bfaea2e..ce626ece7 100644 --- a/tests/boundary/process/processCaptureLifecycle.test.ts +++ b/tests/boundary/process/processCaptureLifecycle.test.ts @@ -275,7 +275,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"); diff --git a/tests/boundary/process/processFilesystemCoverageAdapters.test.ts b/tests/boundary/process/processFilesystemCoverageAdapters.test.ts index ff3b118a7..3ab27d2f5 100644 --- a/tests/boundary/process/processFilesystemCoverageAdapters.test.ts +++ b/tests/boundary/process/processFilesystemCoverageAdapters.test.ts @@ -169,9 +169,14 @@ 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')"], + arguments: [ + "-e", + "const fs=require('node:fs');fs.mkdirSync('added');fs.writeFileSync('added/a.txt','added')", + ], working_directory: root, - filesystem_observation_paths: [root], + // Before capture the first root is absent. After capture it consumes + // both entries, so the unchanged second root is certainly unexamined. + filesystem_observation_paths: [join(root, "added"), join(root, "z.txt")], limits: { files: 2 }, }; const evidence = await (adapter === "cli" ? captureViaCli : captureViaMcp)( @@ -181,13 +186,13 @@ itWithCaptureCapability.each(["cli", "mcp"] as const)( expect(capture.truncated).toBe(true); expect(await readFile(join(root, "z.txt"), "utf8")).toBe("unchanged"); expect( - capture.filesystem_effects.find(({ path }) => path === "root_0:z.txt") + capture.filesystem_effects.find(({ path }) => path === "root_1:.") ?.status, ).toBe("unknown"); expect( capture.filesystem_checkpoints .find(({ name }) => name === "after_settlement") - ?.effects.find(({ path }) => path === "root_0:z.txt")?.status, + ?.effects.find(({ path }) => path === "root_1:.")?.status, ).toBe("unknown"); }, 20_000, diff --git a/tests/boundary/process/processTruncationAdapters.test.ts b/tests/boundary/process/processTruncationAdapters.test.ts index 20d41e639..fe19fb471 100644 --- a/tests/boundary/process/processTruncationAdapters.test.ts +++ b/tests/boundary/process/processTruncationAdapters.test.ts @@ -195,8 +195,12 @@ itWithCaptureCapability.each(["cli", "mcp"] as const)( "files_limit", ); expect( - result.files_after.find(({ path }) => path === "root_0:b.txt")?.sha256, + result.files_before.find(({ path }) => path === "root_0:b.txt")?.sha256, ).toBeNull(); + const after = result.files_after.find( + ({ path }) => path === "root_0:b.txt", + ); + if (after !== undefined) expect(after.sha256).toBeNull(); expect(processSourceTruncated(result, "terminal_rendered")).toBe(true); expect(compareProcessCaptures(result, result)).toMatchObject({ terminal: "unknown", diff --git a/tests/process-global/updateMaintenance.test.ts b/tests/process-global/updateMaintenance.test.ts index 760bd96aa..ca5936858 100644 --- a/tests/process-global/updateMaintenance.test.ts +++ b/tests/process-global/updateMaintenance.test.ts @@ -148,6 +148,7 @@ describe("existing REA integration maintenance", () => { ).resolves.toEqual({ clients: ["codex", "vscode"], skill: false, + skillDestinations: [], }); }); @@ -157,6 +158,7 @@ describe("existing REA integration maintenance", () => { ).resolves.toEqual({ clients: [], skill: false, + skillDestinations: [], }); await writeSkill("---\nname: another-skill\n---\nUser-authored content\n"); await expect( @@ -164,6 +166,7 @@ describe("existing REA integration maintenance", () => { ).resolves.toEqual({ clients: [], skill: false, + skillDestinations: [], }); }); @@ -177,6 +180,7 @@ describe("existing REA integration maintenance", () => { ).resolves.toEqual({ clients: [], skill: true, + skillDestinations: [{ client: "shared", path: dirname(path) }], }); expect(await readFile(path, "utf8")).toBe(original); }); @@ -224,7 +228,7 @@ describe("existing REA integration maintenance", () => { ]); expect(result).toEqual({ status: "planned", - scope: { clients: ["codex"], skill: false }, + scope: { clients: ["codex"], skill: false, skillDestinations: [] }, command: recorded[0]?.slice(0, -2), plannedActions: [action], }); @@ -273,6 +277,8 @@ describe("updated executable maintenance planning", () => { it("runs real setup as a noninteractive dry run and creates no new integrations or backups", async () => { const configPath = await writeClient("codex", staleCodex); const untouched = await writeClient("claude_code", '{"mcpServers":{}}'); + const skillContent = `---\nname: ${PRODUCT_IDENTITY.skillName}\nmetadata:\n version: "1"\n---\n# REA\n`; + const skillPath = await writeSkill(skillContent); vi.stubEnv("npm_command", "exec"); const result = await planIntegrationMaintenance( home, @@ -282,18 +288,35 @@ describe("updated executable maintenance planning", () => { ); expect(result).toMatchObject({ status: "planned", - scope: { clients: ["codex"], skill: false }, - plannedActions: [ - { id: "configure_client:codex", kind: "configure_client" }, - ], + scope: { + clients: ["codex"], + skill: true, + skillDestinations: [{ client: "shared", path: dirname(skillPath) }], + }, }); + if (result.status !== "planned") throw new Error(JSON.stringify(result)); + expect(result.plannedActions).toEqual([ + expect.objectContaining({ + id: "configure_client:codex", + kind: "configure_client", + }), + expect.objectContaining({ + id: "install_skill", + kind: "install_skill", + target: dirname(skillPath), + }), + ]); expect(await readFile(configPath, "utf8")).toBe(staleCodex); expect(await readFile(untouched, "utf8")).toBe('{"mcpServers":{}}'); + expect(await readFile(skillPath, "utf8")).toBe(skillContent); await expect(access(`${configPath}.rea.backup`)).rejects.toMatchObject({ code: "ENOENT", }); + await expect(access(`${skillPath}.rea.backup`)).rejects.toMatchObject({ + code: "ENOENT", + }); await expect( - access(join(home, ".agents", "skills", PRODUCT_IDENTITY.skillName)), + access(join(home, ".claude", "skills", PRODUCT_IDENTITY.skillName)), ).rejects.toMatchObject({ code: "ENOENT" }); }); });