fix(agents): preserve requested controls and imported native settings

Apply Legacy ACP alias precedence before native Codex compatibility checks. Project actual import edits onto the saved native configuration, preserving hidden runtime and credential settings while honoring explicit removals and harness changes.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip committed 2026-10-12 16:56:31 -05:00
1 parent 9c84f19365
commit b63b779fe1
5 files changed
+209 -5

No files matched your search

@@ -9,6 +9,7 @@ vi.mock("./environment-probe.js", () => ({ readSandboxPlatformQualification: sta
vi.mock("./environment-config.js", () => ({ resolveEnvironmentDriverConfigForRuntime: state.resolveEnvironment }));
vi.mock("@paperclipai/adapter-utils/ssh", () => ({ runSshCommand: state.ssh }));
import { AGENT_RUNNER_SSH_PLATFORM_COMMAND, agentRunnerAvailability, resolveAgentRunnerTargetForCompany, resolveNewAgentRunner, resolveNewAgentRunnerForCompany } from "./agent-runner-selection.js";
import { buildCodexAcpConfig } from "@paperclipai/adapter-codex-local/server";
import { localAgentRunnerTarget } from "../lib/agent-runner-platform.js";
const target = { driver: "local", platform: "linux", architecture: "x64" };
const qualifiedTargets = [target, { driver: "local", platform: "linux", architecture: "arm64" }, { driver: "local", platform: "darwin", architecture: "arm64" }, { driver: "local", platform: "darwin", architecture: "x64" }];
@@ -117,6 +118,71 @@ describe("server-owned Codex runner selection", () => {
try { resolveNewAgentRunner({ adapterType: "codex_local", target, adapterConfig: { [field]: true } }); }
catch (error) { expect(error).toMatchObject({ status: 422, details: { fields: [field] } }); }
});
it.each([
["agentCommand", "acpAgentCommand", "custom-codex-acp"],
["stateDir", "acpStateDir", "/custom/acp-state"],
["mode", "acpMode", "oneshot"],
["permissionMode", "acpPermissionMode", "deny-all"],
["nonInteractivePermissions", "acpNonInteractivePermissions", "fail"],
["warmHandleIdleMs", "acpWarmHandleIdleMs", 30_000],
])("rejects effective Legacy ACP alias %s/%s before automatic native translation", (canonical, alias, value) => {
const adapterConfig = { engine: "acp", [alias]: value };
expect(buildCodexAcpConfig(adapterConfig)).toHaveProperty(canonical, value);
for (const runner of ["auto", "paperclip"] as const) {
expect(() => resolveNewAgentRunner({ adapterType: "codex_local", adapterConfig, target, runner }))
.toThrow(expect.objectContaining({ status: 422, details: { code: "agent_runner_config_incompatible", fields: [alias] } }));
}
expect(resolveNewAgentRunner({ adapterType: "codex_local", adapterConfig, target, runner: "legacy" }))
.toEqual({ adapterType: "codex_local", adapterConfig });
});
it.each([
["agentCommand", "acpAgentCommand", "canonical-command", "alias-command"],
["stateDir", "acpStateDir", "/canonical-state", "/alias-state"],
["permissionMode", "acpPermissionMode", "approve-all", "deny-all"],
])("checks the effective canonical ACP %s ahead of its alias", (canonical, alias, value, aliasedValue) => {
const adapterConfig = { engine: "acp", [canonical]: value, [alias]: aliasedValue };
expect(buildCodexAcpConfig(adapterConfig)).toHaveProperty(canonical, value);
expect(() => resolveNewAgentRunner({ adapterType: "codex_local", adapterConfig, target }))
.toThrow(expect.objectContaining({ status: 422, details: { code: "agent_runner_config_incompatible", fields: [canonical] } }));
});
it("uses Legacy ACP canonical precedence and removes compatible or shadowed aliases idempotently", () => {
const adapterConfig = { engine: "acp", mode: "persistent", acpMode: "oneshot",
nonInteractivePermissions: "deny", acpNonInteractivePermissions: "fail",
warmHandleIdleMs: 0, acpWarmHandleIdleMs: 30_000 };
expect(buildCodexAcpConfig(adapterConfig)).toMatchObject({ mode: "persistent", nonInteractivePermissions: "deny", warmHandleIdleMs: 0 });
const resolved = resolveNewAgentRunner({ adapterType: "codex_local", adapterConfig, target });
expect(resolved).toMatchObject({ adapterType: "paperclip_runner", adapterConfig: { codexPermissionMode: "never" } });
for (const field of Object.keys(adapterConfig)) expect(resolved.adapterConfig).not.toHaveProperty(field);
expect(resolveNewAgentRunner(resolved)).toEqual(resolved);
expect(resolveNewAgentRunner({ adapterType: "codex_local", adapterConfig, target, runner: "legacy" }))
.toEqual({ adapterType: "codex_local", adapterConfig });
});
it("accepts equivalent ACP default aliases without persisting legacy-only controls", () => {
const adapterConfig = { engine: "acp", acpMode: " persistent ", acpNonInteractivePermissions: " deny ", acpWarmHandleIdleMs: 0 };
expect(buildCodexAcpConfig(adapterConfig)).toMatchObject({ mode: "persistent", nonInteractivePermissions: "deny", warmHandleIdleMs: 0 });
const resolved = resolveNewAgentRunner({ adapterType: "codex_local", adapterConfig, target });
expect(resolved.adapterType).toBe("paperclip_runner");
for (const field of Object.keys(adapterConfig)) expect(resolved.adapterConfig).not.toHaveProperty(field);
expect(resolveNewAgentRunner(resolved)).toEqual(resolved);
});
it("uses nullish ACP idle precedence instead of treating a canonical zero as absent", () => {
const adapterConfig = { engine: "acp", warmHandleIdleMs: null, acpWarmHandleIdleMs: 30_000 };
expect(buildCodexAcpConfig(adapterConfig)).toHaveProperty("warmHandleIdleMs", 30_000);
expect(() => resolveNewAgentRunner({ adapterType: "codex_local", adapterConfig, target }))
.toThrow(expect.objectContaining({ status: 422, details: { code: "agent_runner_config_incompatible", fields: ["acpWarmHandleIdleMs"] } }));
});
it("uses a nonempty ACP alias when its canonical string is blank", () => {
const adapterConfig = { engine: "acp", permissionMode: " ", acpPermissionMode: " deny-all " };
expect(buildCodexAcpConfig(adapterConfig)).toHaveProperty("permissionMode", "deny-all");
expect(() => resolveNewAgentRunner({ adapterType: "codex_local", adapterConfig, target }))
.toThrow(expect.objectContaining({ status: 422, details: { code: "agent_runner_config_incompatible", fields: ["acpPermissionMode"] } }));
});
it("preserves explicit historical native profiles, including managed configurations", () => {
for (const config of [{ provider: "acpx", acpxAgent: "claude", model: "claude-sonnet-5" }, { provider: "claude_managed", managedProfileId: "profile", managedAgentsRetentionAcknowledged: true }, { provider: "aws_agentcore", agentCoreProfileId: "profile" }]) expect(resolveNewAgentRunner({ adapterType: "paperclip_runner", adapterConfig: config })).toEqual({ adapterType: "paperclip_runner", adapterConfig: config });
});
+27 -1
View File
@@ -61,6 +61,32 @@ export function resolveNewAgentRunner(input: AgentRunnerSelection): { adapterTyp
? "Paperclip Runner is unavailable for this sandbox. Choose an immutable OCI image digest and run its environment connection probe to qualify the OS, architecture and libc, or select Legacy runner in Advanced."
: "Paperclip Runner is unavailable for this harness and environment. Select Legacy runner in Advanced.", { code: "agent_runner_unavailable" });
if (native) return { adapterType, adapterConfig: normalizeLegacyRunnerProvider(config) };
const aliasSources = new Map<string, string>();
// Legacy ACP uses the first nonempty canonical/alias string, and nullish
// precedence for its idle timeout. Validate that effective choice, not an
// unchecked prefixed field, while retaining its name in actionable errors.
for (const [canonical, alias] of [
["agentCommand", "acpAgentCommand"], ["stateDir", "acpStateDir"],
["mode", "acpMode"], ["permissionMode", "acpPermissionMode"],
["nonInteractivePermissions", "acpNonInteractivePermissions"],
]) {
if (config[alias] === undefined) continue;
const canonicalValue = typeof config[canonical] === "string" ? config[canonical].trim() : "";
const aliasValue = typeof config[alias] === "string" ? config[alias].trim() : "";
if (canonicalValue || aliasValue) {
config[canonical] = canonicalValue || aliasValue;
if (!canonicalValue) aliasSources.set(canonical, alias);
} else if (config[alias] !== null && typeof config[alias] !== "string") incompatible([alias]);
else if (typeof config[canonical] === "string") delete config[canonical];
delete config[alias];
}
if (config.acpWarmHandleIdleMs !== undefined) {
if (config.warmHandleIdleMs == null && config.acpWarmHandleIdleMs != null) {
config.warmHandleIdleMs = config.acpWarmHandleIdleMs;
aliasSources.set("warmHandleIdleMs", "acpWarmHandleIdleMs");
}
delete config.acpWarmHandleIdleMs;
}
const unsupported = ["agentCommand", "stateDir", "warmHandleIdleMs", "extraArgs", "args", "search", "fastMode", "variant", "filesystemScope", "filesystemExtraPaths", "filesystemSandboxCommand", "networkScope", "networkAllowlist", "permissionMode", "maxTurns", "disableWebSearch", "outputInactivityTimeoutMs", "terminalResultCleanupGraceMs"]
.filter(key => config[key] !== undefined && config[key] !== false && config[key] !== "" && config[key] !== 0 && !(Array.isArray(config[key]) && config[key].length === 0));
if (config.command && config.command !== "codex") unsupported.push("command");
@@ -69,7 +95,7 @@ export function resolveNewAgentRunner(input: AgentRunnerSelection): { adapterTyp
if (config.nonInteractivePermissions && config.nonInteractivePermissions !== "deny") unsupported.push("nonInteractivePermissions");
if (config.dangerouslyBypassApprovalsAndSandbox === false) unsupported.push("dangerouslyBypassApprovalsAndSandbox");
if (config.dangerouslyBypassSandbox === false) unsupported.push("dangerouslyBypassSandbox");
if (unsupported.length) incompatible(unsupported);
if (unsupported.length) incompatible(unsupported.map(field => aliasSources.get(field) ?? field));
const effort = config.modelReasoningEffort ?? config.reasoningEffort ?? config.effort;
for (const key of ["engine", "command", "extraArgs", "args", "agentCommand", "mode", "nonInteractivePermissions", "stateDir", "warmHandleIdleMs", "dangerouslyBypassApprovalsAndSandbox", "dangerouslyBypassSandbox", "search", "fastMode", "reasoningEffort", "effort"]) delete config[key];
if (config.model !== undefined && typeof config.model !== "string") incompatible(["model"]);
@@ -12,6 +12,7 @@ import { CodexLocalConfigFields } from "./config-fields";
import { buildCodexRunnerConfig } from "./config-fields";
import { defaultCreateValues } from "@/components/agent-config-defaults";
import { resolveCodexLocalCliPolicy } from "@paperclipai/adapter-codex-local";
import { buildPaperclipRunnerConfig } from "@paperclipai/adapter-codex-local/ui";
beforeAll(() => { Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); });
@@ -54,6 +55,11 @@ async function renderRunner(config: Record<string, unknown>, expand?: string, in
}
describe("Paperclip Runner Codex configuration", () => {
it.each(["", "max"])("preserves or overrides saved native Codex reasoning effort (%s)", thinkingEffort => {
const values = { ...defaultCreateValues, 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(["", "--sandbox,read-only"])("shows actual automatic Legacy policy and preserves the explicit toggle (%s)", async (extraArgs) => {
const container = document.createElement("div");
document.body.appendChild(container);
+70
View File
@@ -1301,6 +1301,76 @@ 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 => {
mockOptionalCompany.mockReturnValue({ selectedCompanyId: "company-1" });
mockAdaptersApi.list.mockResolvedValue([
{ type: "claude_local", disabled: false },
{ 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",
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,
};
preview.manifest.agents[0]!.adapterType = "paperclip_runner";
preview.manifest.agents[0]!.runner = "paperclip";
preview.manifest.agents[0]!.adapterConfig = sourceConfig;
mockCompaniesApi.importPreview.mockResolvedValue(preview);
await renderPage();
await enterGithubUrl();
await clickButton(text => text === "Preview import");
if (edit === "changed-harness") {
const harness = findAdapterSelects()[0]!;
await act(async () => {
Object.getOwnPropertyDescriptor(HTMLSelectElement.prototype, "value")!.set!.call(harness, "claude_local");
harness.dispatchEvent(new Event("change", { bubbles: true }));
});
}
await clickButton(text => text.includes("configure adapter"));
await clickButton(text => text.includes("Advanced"));
const expected: Record<string, unknown> = structuredClone(sourceConfig);
if (edit === "legacy-model") {
await act(async () => container.querySelector<HTMLButtonElement>('[aria-label="Runner"]')!.click());
await act(async () => document.querySelector<HTMLButtonElement>('[role="option"][data-value="legacy"]')!.click());
}
if (edit === "model-clear" || edit === "legacy-model" || edit === "changed-harness") {
await act(async () => container.querySelector<HTMLButtonElement>('[aria-label="Model"]')!.click());
await act(async () => [...document.querySelectorAll<HTMLButtonElement>('[role="option"]')].find(option => option.textContent?.trim().startsWith("Default"))!.click());
delete expected.model;
} else if (edit === "effort-clear") {
expect(container.querySelector('[aria-label="Thinking effort"]')?.textContent).toContain("High");
await act(async () => container.querySelector<HTMLButtonElement>('[aria-label="Thinking effort"]')!.click());
await act(async () => document.querySelector<HTMLButtonElement>('[role="option"][data-value=""]')!.click());
delete expected.modelReasoningEffort;
delete expected.reasoningEffort;
} else {
const remove = container.querySelector<HTMLButtonElement>('[aria-label="Remove CODEX_HOME"]');
expect(remove).toBeTruthy();
await act(async () => remove!.click());
await clickButton(text => text === "Save");
expected.env = { OPENAI_API_KEY: sourceConfig.env.OPENAI_API_KEY };
}
await clickButton(text => text.startsWith("Import 3 file"));
await settle();
if (edit === "changed-harness") {
expect(lastImportMeta().adapterOverrides?.coder?.adapterType).toBe("claude_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);
return;
}
expect(lastImportMeta().adapterOverrides?.coder).toEqual({
adapterType: "codex_local", runner: edit === "legacy-model" ? "legacy" : "paperclip", adapterConfig: expected,
});
expect(preview.manifest.agents[0]!.adapterConfig).toEqual(sourceConfig);
});
it("does not reinterpret an unknown native provider as Codex when its configuration is edited", async () => {
mockAdaptersApi.list.mockResolvedValue([{ type: "paperclip_runner", disabled: false }]);
const preview = buildPreviewResult();
+40 -4
View File
@@ -26,8 +26,8 @@ import { getAgentOrderStorageKey, writeAgentOrder } from "../lib/agent-order";
import { MarkdownBody } from "../components/MarkdownBody";
import { Button } from "@/components/ui/button";
import { EmptyState } from "../components/EmptyState";
import { AgentConfigForm } from "../components/AgentConfigForm";
import { cn } from "../lib/utils";
import { AgentConfigForm, overlayValuesEqual } from "../components/AgentConfigForm";
import { asObject, cn } from "../lib/utils";
import {
ArrowRight,
Check,
@@ -584,7 +584,7 @@ function importAdapterValues(
...(runner !== undefined ? { runner } : nativeCodex ? { runner: agentRunner("paperclip_runner") } : {}),
...(sourceConfig && choice === "codex_local" ? {
model: typeof sourceConfig.model === "string" ? sourceConfig.model : "",
thinkingEffort: String(sourceConfig.reasoningEffort ?? sourceConfig.modelReasoningEffort ?? ""),
thinkingEffort: String(sourceConfig.modelReasoningEffort ?? sourceConfig.reasoningEffort ?? ""),
...(typeof savedBypass === "boolean" ? { codexLegacySandboxBypass: savedBypass } : {}),
// Runner-only imports preserve server-translated policy; display that policy too.
dangerouslyBypassSandbox: typeof savedBypass === "boolean" ? savedBypass
@@ -592,6 +592,18 @@ function importAdapterValues(
? (sourceConfig.codexPermissionMode ?? PAPERCLIP_RUNNER_PERMISSION_CAPABILITIES.codex.defaultMode) === "never"
: resolveCodexLocalCliPolicy(sourceConfig).bypass,
} : {}),
...(nativeCodex && sourceConfig ? {
cwd: typeof sourceConfig.cwd === "string" ? sourceConfig.cwd : "",
instructionsFilePath: typeof sourceConfig.instructionsFilePath === "string" ? sourceConfig.instructionsFilePath : "",
envBindings: { ...asObject(sourceConfig.env) },
workspaceStrategyType: String(asObject(sourceConfig.workspaceStrategy).type ?? "project_primary"),
workspaceBaseRef: String(asObject(sourceConfig.workspaceStrategy).baseRef ?? ""),
workspaceBranchTemplate: String(asObject(sourceConfig.workspaceStrategy).branchTemplate ?? ""),
worktreeParentDir: String(asObject(sourceConfig.workspaceStrategy).worktreeParentDir ?? ""),
runtimeServicesJson: sourceConfig.workspaceRuntime ? JSON.stringify(sourceConfig.workspaceRuntime) : "",
...(sourceConfig.lifecycleMode === "warm" || sourceConfig.lifecycleMode === "per_turn" ? { paperclipRunnerLifecycleMode: sourceConfig.lifecycleMode } : {}),
...(typeof sourceConfig.idleTimeoutMs === "number" ? { paperclipRunnerIdleTimeoutMs: sourceConfig.idleTimeoutMs } : {}),
} : {}),
...(sourceAdapterType === "paperclip_runner" && choice !== "openai_dot"
? { adapterSchemaValues: { ...sourceConfig } } : {}),
...(choice === "openai_dot" ? {
@@ -1733,7 +1745,31 @@ 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.");
}
override.adapterConfig = getUIAdapter(configVals.adapterType).buildAdapterConfig(configVals);
const build = getUIAdapter(configVals.adapterType).buildAdapterConfig;
const edited = build(configVals);
if (agent.adapterType === "paperclip_runner" && selectedType === agent.adapterChoice
&& selectedType === "codex_local" && configVals.adapterType === "codex_local"
&& agentHarnessType("paperclip_runner", configVals.adapterSchemaValues) === "codex_local") {
// Project only real form changes onto the saved native configuration.
// Form defaults must not replace bindings or runtime fields; explicit
// clears must not be filled back in from the source either.
const original = importAdapterValues(selectedType, agent.adapterConfig, agent.adapterType, configVals.runner);
const baseline = build(original);
const retained = { ...agent.adapterConfig };
const applyChanges = (before: Record<string, unknown>, after: Record<string, unknown>) => {
for (const key of new Set([...Object.keys(before), ...Object.keys(after)])) {
if (overlayValuesEqual(before[key], after[key])) continue;
if (after[key] === undefined) delete retained[key];
else retained[key] = after[key];
}
};
applyChanges(original.adapterSchemaValues ?? {}, configVals.adapterSchemaValues ?? {});
applyChanges(baseline, edited);
if (!overlayValuesEqual(baseline.modelReasoningEffort, edited.modelReasoningEffort)) delete retained.reasoningEffort;
override.adapterConfig = retained;
} else {
override.adapterConfig = edited;
}
}
overrides[agent.slug] = override;
}