From 3c509681a767e6cf359206fb63490a459ce36e53 Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 12 Oct 2026 17:04:24 -0500 Subject: [PATCH] fix(claude): preserve permission selections and explicit import removals Co-Authored-By: Paperclip --- .../codex-local/src/ui/build-config.test.ts | 11 +++++--- .../codex-local/src/ui/build-config.ts | 2 +- ui/src/pages/CompanyImport.test.tsx | 28 +++++++++++-------- ui/src/pages/CompanyImport.tsx | 17 ++++++++++- 4 files changed, 41 insertions(+), 17 deletions(-) diff --git a/packages/adapters/codex-local/src/ui/build-config.test.ts b/packages/adapters/codex-local/src/ui/build-config.test.ts index ed6156cddd7..a160fc6ef27 100644 --- a/packages/adapters/codex-local/src/ui/build-config.test.ts +++ b/packages/adapters/codex-local/src/ui/build-config.test.ts @@ -118,10 +118,13 @@ describe("buildCodexLocalConfig", () => { }); describe("buildPaperclipRunnerConfig", () => { - it.each(["", "max"])("preserves or overrides saved native Codex reasoning effort (%s)", thinkingEffort => { - const values = makeValues({ thinkingEffort, adapterSchemaValues: { provider: "codex", modelReasoningEffort: "high" } }); - expect(buildPaperclipRunnerConfig(values).modelReasoningEffort).toBe(thinkingEffort || "high"); - expect(buildPaperclipRunnerConfig({ ...values, adapterSchemaValues: { provider: "acpx", acpxAgent: "claude", modelReasoningEffort: "high" } })).not.toHaveProperty("modelReasoningEffort"); + it.each(["codex", "claude"])("preserves, overrides and explicitly clears saved %s reasoning effort", harness => { + const profile = harness === "codex" ? { provider: "codex" } : { provider: "acpx", acpxAgent: "claude" }; + const values = makeValues({ adapterSchemaValues: { ...profile, modelReasoningEffort: "high", reasoningEffort: "low", effort: "low" } }); + expect(buildPaperclipRunnerConfig({ ...values, thinkingEffort: undefined }).modelReasoningEffort).toBe("high"); + expect(buildPaperclipRunnerConfig({ ...values, thinkingEffort: "max" }).modelReasoningEffort).toBe("max"); + const cleared = buildPaperclipRunnerConfig({ ...values, thinkingEffort: "" }); + for (const key of ["modelReasoningEffort", "reasoningEffort", "effort"]) expect(cleared).not.toHaveProperty(key); }); it.each([undefined, false, true])("preserves Dot attachment consent in create/import forms: %s", value => { const config = buildPaperclipRunnerConfig(makeValues({ adapterType: "paperclip_runner", adapterSchemaValues: { provider: "openai_dot", dotAttachmentAccess: value } })); diff --git a/packages/adapters/codex-local/src/ui/build-config.ts b/packages/adapters/codex-local/src/ui/build-config.ts index 6a79f7a4a85..6a60e33cf42 100644 --- a/packages/adapters/codex-local/src/ui/build-config.ts +++ b/packages/adapters/codex-local/src/ui/build-config.ts @@ -230,7 +230,7 @@ export function buildPaperclipRunnerConfig(v: CreateConfigValues): Record { expect(preview.manifest.agents[0]!.adapterConfig).toEqual(savedSourceConfig); }); - it.each(["deny-all", "approve-reads"])("preserves native Claude settings and saves an edited permission from %s", async permission => { + it.each(["deny-all", "approve-reads"].flatMap(permission => ["paperclip_runner", "claude_local"].map(sourceType => ({ permission, sourceType }))))("saves the selected Claude permission from $sourceType ($permission)", async ({ permission, sourceType }) => { mockOptionalCompany.mockReturnValue({ selectedCompanyId: "company-1" }); mockAdaptersApi.list.mockResolvedValue([ { type: "claude_local", disabled: false, supportedRunners: ["paperclip", "legacy"], defaultRunner: "paperclip" }, @@ -1277,16 +1277,22 @@ describe("CompanyImport", () => { ]); const preview = buildPreviewResult(); const source = { provider: "acpx", acpxAgent: "claude", model: "claude-opus-5-5", modelReasoningEffort: "high", acpxPermissionMode: permission, timeoutSec: 180, lifecycleMode: "per_turn" }; - preview.manifest.agents[0]!.adapterType = "paperclip_runner"; - preview.manifest.agents[0]!.runner = "paperclip"; + preview.manifest.agents[0]!.adapterType = sourceType; + preview.manifest.agents[0]!.runner = sourceType === "paperclip_runner" ? "paperclip" : "legacy"; preview.manifest.agents[0]!.adapterConfig = source; mockCompaniesApi.importPreview.mockResolvedValue(preview); await renderPage(); await enterGithubUrl(); await clickButton(text => text === "Preview import"); await clickButton(text => text.includes("configure adapter")); + if (sourceType === "claude_local") { + await clickButton(text => text.includes("Advanced")); + await act(async () => container.querySelector('[aria-label="Runner"]')!.click()); + await act(async () => document.querySelector('[role="option"][data-value="paperclip"]')!.click()); + await flushReact(); + } const select = container.querySelector('[aria-label="Permission mode"]'); - expect(select?.textContent).toContain(permission === "deny-all" ? "Deny all" : "Allow Paperclip reads"); + if (sourceType === "paperclip_runner") expect(select?.textContent).toContain(permission === "deny-all" ? "Deny all" : "Allow Paperclip reads"); await act(async () => select!.click()); const nextPermission = permission === "deny-all" ? "approve-reads" : "deny-all"; const option = [...document.querySelectorAll('[role="option"]')].find(element => element.textContent === (nextPermission === "deny-all" ? "Deny all" : "Allow Paperclip reads")); @@ -1297,7 +1303,7 @@ describe("CompanyImport", () => { await settle(); expect(lastImportMeta().adapterOverrides?.coder).toMatchObject({ runner: "paperclip", - adapterConfig: { ...source, acpxPermissionMode: nextPermission }, + adapterConfig: sourceType === "paperclip_runner" ? { ...source, acpxPermissionMode: nextPermission } : { provider: "acpx", acpxAgent: "claude", acpxPermissionMode: nextPermission }, }); expect(preview.manifest.agents[0]!.adapterConfig).toEqual(source); }); @@ -1360,22 +1366,22 @@ describe("CompanyImport", () => { expect(lastImportMeta().adapterOverrides?.coder?.adapterConfig).not.toHaveProperty("codexLegacySandboxBypass"); }); - it.each(["model-clear", "effort-clear", "env-remove", "legacy-model", "changed-harness"] as const)("preserves native Codex source configuration during an ordinary import edit (%s)", async edit => { + it.each(["codex_local", "claude_local"].flatMap(harness => ["model-clear", "effort-clear", "env-remove", "legacy-model", "changed-harness"].map(edit => ({ harness, edit }))))("preserves native source configuration during an ordinary import edit ($harness, $edit)", async ({ harness, edit }) => { mockOptionalCompany.mockReturnValue({ selectedCompanyId: "company-1" }); mockAdaptersApi.list.mockResolvedValue([ - { type: "claude_local", disabled: false }, + { type: "claude_local", disabled: false, supportedRunners: ["paperclip", "legacy"], defaultRunner: "paperclip" }, { type: "codex_local", disabled: false, supportedRunners: ["paperclip", "legacy"], defaultRunner: "paperclip" }, { type: "paperclip_runner", disabled: false }, ]); const preview = buildPreviewResult(); const sourceConfig = { - provider: "codex", model: "gpt-5.6-sol", modelReasoningEffort: "high", reasoningEffort: "low", + ...(harness === "codex_local" ? { provider: "codex", model: "gpt-5.6-sol" } : { provider: "acpx", acpxAgent: "claude", model: "claude-opus-5-5", acpxPermissionMode: "approve-all" }), modelReasoningEffort: "high", reasoningEffort: "low", env: { CODEX_HOME: { type: "secret_ref", secretId: "saved-account", version: "latest" }, OPENAI_API_KEY: { type: "user_secret_ref", key: "OPENAI_API_KEY", version: "latest", required: true } }, timeoutSec: 120, graceSec: 30, cwd: "/saved/workspace", instructionsFilePath: "/saved/AGENTS.md", workspaceStrategy: { type: "git_worktree", baseRef: "main", branchTemplate: "qa/{runId}", worktreeParentDir: "/saved/worktrees" }, workspaceRuntime: { services: [{ name: "app", command: "pnpm dev" }] }, - lifecycleMode: "warm", idleTimeoutMs: 45000, + lifecycleMode: "per_turn", idleTimeoutMs: 45000, }; preview.manifest.agents[0]!.adapterType = "paperclip_runner"; preview.manifest.agents[0]!.runner = "paperclip"; @@ -1387,7 +1393,7 @@ describe("CompanyImport", () => { if (edit === "changed-harness") { const harness = findAdapterSelects()[0]!; await act(async () => { - Object.getOwnPropertyDescriptor(HTMLSelectElement.prototype, "value")!.set!.call(harness, "claude_local"); + Object.getOwnPropertyDescriptor(HTMLSelectElement.prototype, "value")!.set!.call(harness, sourceConfig.provider === "codex" ? "claude_local" : "codex_local"); harness.dispatchEvent(new Event("change", { bubbles: true })); }); } @@ -1418,7 +1424,7 @@ describe("CompanyImport", () => { await clickButton(text => text.startsWith("Import 3 file")); await settle(); if (edit === "changed-harness") { - expect(lastImportMeta().adapterOverrides?.coder?.adapterType).toBe("claude_local"); + expect(lastImportMeta().adapterOverrides?.coder?.adapterType).toBe(harness === "codex_local" ? "claude_local" : "codex_local"); const config = lastImportMeta().adapterOverrides?.coder?.adapterConfig; for (const field of ["provider", "env", "workspaceStrategy", "workspaceRuntime", "lifecycleMode", "idleTimeoutMs", "modelReasoningEffort"]) expect(config).not.toHaveProperty(field); expect(config?.timeoutSec).toBe(0); diff --git a/ui/src/pages/CompanyImport.tsx b/ui/src/pages/CompanyImport.tsx index e982e0db5f0..77b925a93d6 100644 --- a/ui/src/pages/CompanyImport.tsx +++ b/ui/src/pages/CompanyImport.tsx @@ -1750,7 +1750,10 @@ export function CompanyImport() { if (selectedType === "openai_dot" && configVals.adapterSchemaValues?.allowUnmeteredProvider !== true) { throw new Error("OpenAI Dot requires acknowledgement of external billing. Configure the Dot adapter and enable Allow externally billed provider."); } - const build = getUIAdapter(retainsNativeHarness && selectedType === "claude_local" ? "paperclip_runner" : configVals.adapterType).buildAdapterConfig; + const nativeClaudeForm = selectedType === "claude_local" && (retainsNativeHarness + || (configVals.runner !== "legacy" && configVals.adapterSchemaValues?.provider === "acpx" + && configVals.adapterSchemaValues.acpxAgent === "claude")); + const build = getUIAdapter(nativeClaudeForm ? "paperclip_runner" : configVals.adapterType).buildAdapterConfig; const edited = build(configVals); if (agent.adapterType === "paperclip_runner" && selectedType === agent.adapterChoice && ["codex_local", "claude_local"].includes(selectedType) && configVals.adapterType === selectedType @@ -1770,6 +1773,18 @@ export function CompanyImport() { }; applyChanges(original.adapterSchemaValues ?? {}, configVals.adapterSchemaValues ?? {}); applyChanges(baseline, edited); + if (selectedType === "claude_local") { + // The native builder preserves source schema fields. Project common + // form edits separately so a removed binding cannot be restored there. + const commonBuild = getUIAdapter("claude_local").buildAdapterConfig; + const commonBefore = commonBuild(original); + const commonAfter = commonBuild(configVals); + for (const key of ["cwd", "instructionsFilePath", "env", "workspaceStrategy", "workspaceRuntime"]) { + if (overlayValuesEqual(commonBefore[key], commonAfter[key])) continue; + if (commonAfter[key] === undefined) delete retained[key]; + else retained[key] = commonAfter[key]; + } + } if (original.model !== configVals.model && !configVals.model) delete retained.model; if (original.thinkingEffort !== configVals.thinkingEffort) { delete retained.reasoningEffort;