diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0d5924c9c2..45ee2cd50b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -868,6 +868,7 @@ jobs: tests/boundary/providers/native/artifactProvider.extractionCompleteness.test.ts tests/boundary/filesystem/setupUnavailableProvider.test.ts tests/boundary/filesystem/windowsNpxRegistration.test.ts + tests/boundary/filesystem/claudeDesktopSkillUninstallWindows.test.ts tests/boundary/filesystem/clientConfigurationEnvironment.test.ts tests/boundary/filesystem/clientConfigurationLayeredEnvironment.test.ts tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts diff --git a/scripts/ci/scopes.mjs b/scripts/ci/scopes.mjs index b004649203..60eb5d5889 100644 --- a/scripts/ci/scopes.mjs +++ b/scripts/ci/scopes.mjs @@ -178,6 +178,7 @@ export const SCOPES = { "tests/boundary/providers/native/artifactProvider.extractionCompleteness.test.ts", "tests/boundary/filesystem/setupUnavailableProvider.test.ts", "tests/boundary/filesystem/windowsNpxRegistration.test.ts", + "tests/boundary/filesystem/claudeDesktopSkillUninstallWindows.test.ts", "tests/boundary/filesystem/clientConfigurationEnvironment.test.ts", "tests/boundary/filesystem/clientConfigurationLayeredEnvironment.test.ts", "tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts", diff --git a/src/cli/omittableDefaults.ts b/src/cli/omittableDefaults.ts index 603a7265d8..6cf381dfe6 100644 --- a/src/cli/omittableDefaults.ts +++ b/src/cli/omittableDefaults.ts @@ -40,9 +40,10 @@ const withOmittableDefaults = (schema: z.ZodObject): z.ZodObject => { ); }; -/** Zod accepts an omitted value (`optin`) but always produces one (`optout`). */ +/** Let the object parser decide omission; Incur advertises required outputs. */ const fillsOmission = (field: z.ZodType): boolean => - field._zod.optin === "optional" && field._zod.optout === undefined; + field._zod.optout === undefined && + z.object({ value: field }).safeParse({}).success; const optionalWithMetadata = (field: z.ZodType): z.ZodType => { const metadata = field.meta(); diff --git a/tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts b/tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts index 494565b587..1bfaafffd8 100644 --- a/tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts +++ b/tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts @@ -533,39 +533,6 @@ describe("MSIX Claude Desktop uninstall", () => { expect(await readJson(appDataConfig)).toEqual(unrelated); }); - it("blocks managed skill removal through redirected AppData before any removal", async () => { - const { home, env, appDataConfig, privateConfig } = await windowsProfile(); - await writeJson(join(dirname(privateConfig), "Preferences"), {}); - await mkdir(dirname(dirname(appDataConfig)), { recursive: true }); - await symlink(dirname(privateConfig), dirname(appDataConfig), "dir"); - const agents = join(env.APPDATA, "dsh", "agents"); - const manifest = join( - agents, - "skills", - "reverse-engineer-anything", - "SKILL.md", - ); - await mkdir(dirname(manifest), { recursive: true }); - await writeFile( - manifest, - "---\nname: reverse-engineer-anything\n---\nmanaged fixture\n", - ); - const environment = { ...env, DSH_AGENTS_HOME: agents }; - const result = await runUninstall( - false, - systemUninstallHost(home, undefined, environment, "win32"), - ); - expect(result.status).toBe("failed"); - expect(result.items).toContainEqual( - expect.objectContaining({ - name: "skill", - status: "failed", - detail: expect.stringContaining("redirects its writes"), - }), - ); - expect(await readFile(manifest, "utf8")).toContain("managed fixture"); - }); - it("removes REA from both the private copy and AppData on uninstall", async () => { const { home, env, appDataConfig, privateConfig } = await windowsProfile(); await writeJson(privateConfig, { diff --git a/tests/boundary/filesystem/claudeDesktopSkillUninstallWindows.test.ts b/tests/boundary/filesystem/claudeDesktopSkillUninstallWindows.test.ts new file mode 100644 index 0000000000..d64dc857a5 --- /dev/null +++ b/tests/boundary/filesystem/claudeDesktopSkillUninstallWindows.test.ts @@ -0,0 +1,76 @@ +import { access, mkdir, readFile, symlink, writeFile } from "node:fs/promises"; +import { dirname, join } from "node:path"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { CLAUDE_DESKTOP_PACKAGE_FAMILY } from "../../../src/application/ClaudeDesktopConfigLocation.js"; +import { + runUninstall, + systemUninstallHost, +} from "../../../src/application/Uninstall.js"; +import { clearClientLocationEnvironment } from "../../fixtures/clientEnvironment.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +beforeEach(clearClientLocationEnvironment); +afterEach(() => vi.unstubAllEnvs()); + +// Explicit win32 skill destinations must be exercised on the native filesystem. +// A junction models the package's redirected AppData without symlink privilege. +it.runIf(process.platform === "win32").each([true, false])( + "preflights native managed skill removal with redirected AppData: %s", + async (redirected) => { + const home = await createTestTempDirectory("rea-claude-skill-uninstall-"); + const env = { + APPDATA: join(home, "AppData", "Roaming"), + LOCALAPPDATA: join(home, "AppData", "Local"), + }; + const privateClaude = join( + env.LOCALAPPDATA, + "Packages", + CLAUDE_DESKTOP_PACKAGE_FAMILY, + "LocalCache", + "Roaming", + "Claude", + ); + await mkdir(privateClaude, { recursive: true }); + await writeFile(join(privateClaude, "Preferences"), "{}"); + await mkdir(env.APPDATA, { recursive: true }); + if (redirected) + await symlink(privateClaude, join(env.APPDATA, "Claude"), "junction"); + const agents = join(env.APPDATA, "dsh", "agents"); + const manifests = [ + join(agents, "skills", "reverse-engineer-anything", "SKILL.md"), + join(home, ".claude", "skills", "reverse-engineer-anything", "SKILL.md"), + ]; + const contents = + "---\nname: reverse-engineer-anything\n---\nmanaged fixture\n"; + for (const manifest of manifests) { + await mkdir(dirname(manifest), { recursive: true }); + await writeFile(manifest, contents); + } + const result = await runUninstall( + false, + systemUninstallHost( + home, + undefined, + { ...env, DSH_AGENTS_HOME: agents }, + "win32", + ), + ); + expect(result.status).toBe(redirected ? "failed" : "complete"); + expect(result.items).toContainEqual( + expect.objectContaining({ + name: "skill", + status: redirected ? "failed" : "removed", + ...(redirected + ? { detail: expect.stringContaining("redirects its writes") } + : {}), + }), + ); + for (const manifest of manifests) { + if (redirected) expect(await readFile(manifest, "utf8")).toBe(contents); + else + await expect(access(manifest)).rejects.toMatchObject({ + code: "ENOENT", + }); + } + }, +); diff --git a/tests/boundary/filesystem/traeClientConfiguration.test.ts b/tests/boundary/filesystem/traeClientConfiguration.test.ts index 202220e525..528c81e7be 100644 --- a/tests/boundary/filesystem/traeClientConfiguration.test.ts +++ b/tests/boundary/filesystem/traeClientConfiguration.test.ts @@ -1,5 +1,5 @@ import { mkdir, readFile, writeFile } from "node:fs/promises"; -import { join } from "node:path"; +import { join, win32 } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -72,7 +72,12 @@ describe("Trae configuration paths", () => { ({ path }) => path, ), ).toEqual([ - join("C:\\Users\\a", ".trae", "skills", "reverse-engineer-anything"), + win32.join( + "C:\\Users\\a", + ".trae", + "skills", + "reverse-engineer-anything", + ), ]); }); }); diff --git a/tests/boundary/filesystem/workbuddyClientConfiguration.test.ts b/tests/boundary/filesystem/workbuddyClientConfiguration.test.ts index b6aac6577b..fc9b746d68 100644 --- a/tests/boundary/filesystem/workbuddyClientConfiguration.test.ts +++ b/tests/boundary/filesystem/workbuddyClientConfiguration.test.ts @@ -1,5 +1,5 @@ import { mkdir, readFile, writeFile } from "node:fs/promises"; -import { join } from "node:path"; +import { join, win32 } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -58,7 +58,12 @@ describe("WorkBuddy configuration paths", () => { ({ path }) => path, ), ).toEqual([ - join("C:\\Users\\a", ".workbuddy", "skills", "reverse-engineer-anything"), + win32.join( + "C:\\Users\\a", + ".workbuddy", + "skills", + "reverse-engineer-anything", + ), ]); }); }); diff --git a/tests/boundary/sqlite/sqliteDatabaseDescriptorCleanup.test.ts b/tests/boundary/sqlite/sqliteDatabaseDescriptorCleanup.test.ts index a6a0c4c536..954a1e0bba 100644 --- a/tests/boundary/sqlite/sqliteDatabaseDescriptorCleanup.test.ts +++ b/tests/boundary/sqlite/sqliteDatabaseDescriptorCleanup.test.ts @@ -19,12 +19,17 @@ import { silentLogger } from "../../../src/logger.js"; import { SqliteDatabaseProvider } from "../../../src/sqlite/SqliteDatabaseProvider.js"; import { PrivateRuntimeRoot } from "../../../src/process/PrivateRuntimeRoot.js"; import { spawnOwnedProviderProcess } from "../../../src/process/ProviderProcess.js"; -import { createSqliteDatabaseFixture } from "../../fixtures/sqlite/database.js"; +import { + createSqliteDatabaseFixture, + SQLITE_NATIVE_LIMITS_AVAILABLE, +} from "../../fixtures/sqlite/database.js"; import { createTestWorkspace, removeTestWorkspace, } from "../../support/workspace/workspaceFixture.js"; +const nativeInspectionTest = it.runIf(SQLITE_NATIVE_LIMITS_AVAILABLE); + const descriptorCleanup = vi.hoisted(() => ({ path: "", denied: false, @@ -351,7 +356,7 @@ it("retains reply-read cancellation and retries the failed reply descriptor clos await expect(access(root.path)).rejects.toMatchObject({ code: "ENOENT" }); }); -it.each([ +nativeInspectionTest.each([ { cancel: false, denied: false }, { cancel: true, denied: false }, { cancel: false, denied: true }, @@ -480,64 +485,70 @@ it.each([ }, ); -it("keeps a completed reply close failure primary when abort follows lease retention", async () => { - const selected = await fixture("reply"); - const controller = new AbortController(); - const reason = { requested: "abort after reply close failure capture" }; - let retainedReplyLease: string | undefined; - const retain = ArtifactResourceScope.prototype.retain; - const retainSpy = vi - .spyOn(ArtifactResourceScope.prototype, "retain") - .mockImplementation(function (this: ArtifactResourceScope, owner) { - retain.call(this, owner); - if (owner.kind === "lease" && owner.resource === descriptorCleanup.path) { - retainedReplyLease = owner.resource; - controller.abort(reason); - } - }); +nativeInspectionTest( + "keeps a completed reply close failure primary when abort follows lease retention", + async () => { + const selected = await fixture("reply"); + const controller = new AbortController(); + const reason = { requested: "abort after reply close failure capture" }; + let retainedReplyLease: string | undefined; + const retain = ArtifactResourceScope.prototype.retain; + const retainSpy = vi + .spyOn(ArtifactResourceScope.prototype, "retain") + .mockImplementation(function (this: ArtifactResourceScope, owner) { + retain.call(this, owner); + if ( + owner.kind === "lease" && + owner.resource === descriptorCleanup.path + ) { + retainedReplyLease = owner.resource; + controller.abort(reason); + } + }); - try { - const result = await selected.provider.inspect( - { path: selected.database.path }, - { signal: controller.signal }, - ); - expect(controller.signal.aborted).toBe(true); - expect(controller.signal.reason).toBe(reason); - expect(retainedReplyLease).toBe(descriptorCleanup.path); - expect(result.ok).toBe(false); - if (result.ok) return; + try { + const result = await selected.provider.inspect( + { path: selected.database.path }, + { signal: controller.signal }, + ); + expect(controller.signal.aborted).toBe(true); + expect(controller.signal.reason).toBe(reason); + expect(retainedReplyLease).toBe(descriptorCleanup.path); + expect(result.ok).toBe(false); + if (result.ok) return; - const primary = primaryAnalysisError(result.error); - expect(primary).toBeInstanceOf(ProviderCleanupError); - if (!(primary instanceof ProviderCleanupError)) return; - expect(primary).not.toMatchObject({ _tag: "AnalysisCancelledError" }); - expect(primary.cause).toBeInstanceOf(ArtifactFileCleanupFailure); - expect(primary.cause).toMatchObject({ - fileCleanupFailure: { - path: descriptorCleanup.path, - outcome: { kind: "completed" }, - completionCancellation: undefined, - }, - }); - expect(primary.cleanupIncomplete).toBe(true); - expect(primary.cleanupResources).toContain(descriptorCleanup.path); + const primary = primaryAnalysisError(result.error); + expect(primary).toBeInstanceOf(ProviderCleanupError); + if (!(primary instanceof ProviderCleanupError)) return; + expect(primary).not.toMatchObject({ _tag: "AnalysisCancelledError" }); + expect(primary.cause).toBeInstanceOf(ArtifactFileCleanupFailure); + expect(primary.cause).toMatchObject({ + fileCleanupFailure: { + path: descriptorCleanup.path, + outcome: { kind: "completed" }, + completionCancellation: undefined, + }, + }); + expect(primary.cleanupIncomplete).toBe(true); + expect(primary.cleanupResources).toContain(descriptorCleanup.path); - const handle = descriptorCleanup.handles[0]; - if (handle === undefined) - throw new Error("Expected owned reply descriptor"); - expect(handle.fd).toBeGreaterThanOrEqual(0); - expect(descriptorCleanup.attempts).toBe(1); + const handle = descriptorCleanup.handles[0]; + if (handle === undefined) + throw new Error("Expected owned reply descriptor"); + expect(handle.fd).toBeGreaterThanOrEqual(0); + expect(descriptorCleanup.attempts).toBe(1); - descriptorCleanup.denied = false; - await selected.provider.close(); - expect(descriptorCleanup.attempts).toBe(2); - expect(handle.fd).toBe(-1); - expect(primary.cleanupIncomplete).toBe(false); - expect(primary).toBeInstanceOf(ProviderCleanupError); - } finally { - retainSpy.mockRestore(); - } -}); + descriptorCleanup.denied = false; + await selected.provider.close(); + expect(descriptorCleanup.attempts).toBe(2); + expect(handle.fd).toBe(-1); + expect(primary.cleanupIncomplete).toBe(false); + expect(primary).toBeInstanceOf(ProviderCleanupError); + } finally { + retainSpy.mockRestore(); + } + }, +); it("retries source cleanup before admitting another snapshot", async () => { const selected = await fixture("database");