diff --git a/packages/adapter-utils/src/acpx-engine/execute.test.ts b/packages/adapter-utils/src/acpx-engine/execute.test.ts index 5981cc79cf..1e3b8abd00 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.test.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.test.ts @@ -1548,6 +1548,30 @@ describe("shared ACPX engine runtime behavior", () => { expect(env.PAPERCLIP_CLOUD_PROVIDER_TOKEN).toBe("cloud-token"); }); + it.each(["OPENAI_API_KEY", "CODEX_API_KEY"] as const)( + "selects Codex ACP API-key authentication when %s is configured", + async (apiKeyName) => { + const root = await makeTempRoot(); + const codexHome = path.join(root, "codex-home"); + await fs.mkdir(codexHome, { recursive: true }); + + const { sessionInputs } = await runExecutor({ + agent: "codex", + stateDir: path.join(root, "state"), + env: { + CODEX_HOME: codexHome, + [apiKeyName]: "sk-acp-test-key", + }, + paperclipRuntimeSkills: [], + paperclipSkillSync: { desiredSkills: [] }, + }); + + const env = (sessionInputs[0]!.sessionOptions as { env: Record }).env; + expect(env[apiKeyName]).toBe("sk-acp-test-key"); + expect(env.DEFAULT_AUTH_REQUEST).toBe(JSON.stringify({ methodId: "api-key" })); + }, + ); + it("busts the session fingerprint when resolved adapter env changes but not across wakes", async () => { const root = await makeTempRoot(); const stateDir = path.join(root, "state"); @@ -4854,13 +4878,13 @@ describe("ACPX engine sandbox-start spans (opt-in root + child parenting)", () = const { traceContext, spans } = createRecordingStartupTrace(); // A codex bring-up runs the codex-home.seed step, which nests skills.reconcile. - const { events } = await runExecutor( + const { events, sessionInputs } = await runExecutor( { agent: "codex", agentCommand: "node ./fake-acp.js", stateDir, cwd: localCwd, - env: { CODEX_HOME: codexHome }, + env: { CODEX_HOME: codexHome, OPENAI_API_KEY: "sk-acp-test-key" }, }, { authToken: "real-run-jwt", executionTarget, startupTraceContext: traceContext }, ); @@ -5426,13 +5450,13 @@ describe("ACPX engine per-step startup timing (run.startup.step events)", () => runner: createLocalSandboxRunner(), }; - const { events } = await runExecutor( + const { events, sessionInputs } = await runExecutor( { agent: "codex", agentCommand: "node ./fake-acp.js", stateDir, cwd: localCwd, - env: { CODEX_HOME: codexHome }, + env: { CODEX_HOME: codexHome, OPENAI_API_KEY: "sk-acp-test-key" }, }, { authToken: "real-run-jwt", executionTarget }, ); @@ -5454,6 +5478,9 @@ describe("ACPX engine per-step startup timing (run.startup.step events)", () => expect(typeof event!.payload?.durationMs).toBe("number"); expect(event!.payload?.durationMs as number).toBeGreaterThanOrEqual(0); } + const sessionEnv = (sessionInputs[0]?.sessionOptions as { env: Record }).env; + expect(sessionEnv.OPENAI_API_KEY).toBe("sk-acp-test-key"); + expect(sessionEnv.DEFAULT_AUTH_REQUEST).toBe(JSON.stringify({ methodId: "api-key" })); }); it("emits the 5 non-codex boundaries for a custom-agent sandbox bring-up (no codex steps)", async () => { diff --git a/packages/adapter-utils/src/acpx-engine/execute.ts b/packages/adapter-utils/src/acpx-engine/execute.ts index ae10093fcb..b4971bd718 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.ts @@ -1967,6 +1967,17 @@ async function buildRuntime(input: { // are absent from tempKeysApplied and keep their compatibility protection. if (!scratchKeys.has(key) || value !== scratch.dir) resolvedAdapterEnv[key] = value; } + // codex-acp supports both key names, but ACP clients must select its + // api-key authentication method during session creation. Without this + // request, the server advertises authentication and rejects session/new even + // though the credential is present in the launched process environment. + if ( + acpxAgent === "codex" && + (env.OPENAI_API_KEY || env.CODEX_API_KEY) && + !env.DEFAULT_AUTH_REQUEST + ) { + env.DEFAULT_AUTH_REQUEST = JSON.stringify({ methodId: "api-key" }); + } if (authToken) env.PAPERCLIP_API_KEY = authToken; // For the claude agent, set model via ANTHROPIC_MODEL at startup rather than // via session/set_config_option — the ACP server's set_config_option handler diff --git a/packages/shared/src/validators/agent.ts b/packages/shared/src/validators/agent.ts index c1fdf16c72..fbff34fa72 100644 --- a/packages/shared/src/validators/agent.ts +++ b/packages/shared/src/validators/agent.ts @@ -242,6 +242,8 @@ export const resetAgentSessionSchema = z.object({ export type ResetAgentSession = z.infer; export const testAdapterEnvironmentSchema = z.object({ + /** Saved agent whose redacted environment entries are restored for this probe. */ + agentId: z.string().guid().optional(), /** One-shot provider keys for a probe. Never persist these in agent config. */ testCredentials: z.object({ ANTHROPIC_API_KEY: z.string().max(16384), diff --git a/server/src/__tests__/agent-adapter-validation-routes.test.ts b/server/src/__tests__/agent-adapter-validation-routes.test.ts index 918560aaa1..8f1c2e11d3 100644 --- a/server/src/__tests__/agent-adapter-validation-routes.test.ts +++ b/server/src/__tests__/agent-adapter-validation-routes.test.ts @@ -536,6 +536,52 @@ describe("agent routes adapter validation", () => { expect(String(env.CODEX_HOME)).toContain(`/companies/company-1/agents/${agentId}/codex-home`); }); + it("restores a saved agent's redacted CODEX_HOME before testing its adapter", async () => { + const agentId = "11111111-1111-4111-8111-111111111111"; + const storedHome = "/paperclip/companies/company-1/agents/agent-1/codex-home"; + mockAgentService.getById.mockResolvedValue({ + ...(await mockAgentService.getById()), + id: agentId, + adapterType: "external_test", + adapterConfig: { env: { CODEX_HOME: storedHome } }, + }); + const { registerServerAdapter } = await import("../adapters/index.js"); + registerServerAdapter(externalAdapter); + const app = await createApp(); + const res = await requestApp(app, (baseUrl) => + request(baseUrl) + .post("/api/companies/company-1/adapters/external_test/test-environment") + .send({ + agentId, + adapterConfig: { env: { CODEX_HOME: { type: "plain", value: "***REDACTED***" } } }, + }), + ); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockSecretService.normalizeAdapterConfigForPersistence).toHaveBeenCalledWith( + "company-1", + { env: { CODEX_HOME: storedHome } }, + expect.objectContaining({ adapterType: "external_test" }), + ); + }); + + it("rejects redacted-value restoration from an incompatible saved agent", async () => { + const { registerServerAdapter } = await import("../adapters/index.js"); + registerServerAdapter(externalAdapter); + const app = await createApp(); + const res = await requestApp(app, (baseUrl) => + request(baseUrl) + .post("/api/companies/company-1/adapters/external_test/test-environment") + .send({ + agentId: "11111111-1111-4111-8111-111111111111", + adapterConfig: { env: { CODEX_HOME: { type: "plain", value: "***REDACTED***" } } }, + }), + ); + + expect(res.status).toBe(422); + expect(mockSecretService.normalizeAdapterConfigForPersistence).not.toHaveBeenCalled(); + }); + it("rejects unknown adapter types even when schema accepts arbitrary strings", async () => { const app = await createApp(); const res = await requestApp(app, (baseUrl) => diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 6efa0888c9..9585da7cce 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -3139,9 +3139,32 @@ export function agentRoutes( if (requestedEnvironmentId) { await assertAdapterTestEnvironmentForCompany(companyId, requestedEnvironmentId); } + // Agent reads redact every plain environment value. When this is a saved + // agent test, restore those display-only placeholders from the + // server-side config before validating or resolving secrets; otherwise + // the probe treats "***REDACTED***" as a value to persist. + const savedAgentId = typeof req.body.agentId === "string" ? req.body.agentId : null; + let adapterConfigForTest = inputAdapterConfig; + if (savedAgentId) { + const savedAgent = await getAccessibleResource(req, res, svc.getById(savedAgentId), "Agent not found"); + if (!savedAgent) return; + if (savedAgent.companyId !== companyId) throw notFound("Agent not found"); + const providerAdapter = savedAgent.adapterType === "paperclip_runner" + ? inputAdapterConfig.provider === "codex" + ? "codex_local" + : inputAdapterConfig.provider === "acpx" && inputAdapterConfig.acpxAgent === "claude" + ? "claude_local" + : null + : null; + if (savedAgent.adapterType !== type && providerAdapter !== type) { + throw unprocessable("Saved agent is not compatible with the adapter being tested"); + } + await assertCanUpdateAgent(req, savedAgent); + adapterConfigForTest = restoreRedactedAgentEnv(inputAdapterConfig, savedAgent.adapterConfig); + } const normalizedAdapterConfig = await secretsSvc.normalizeAdapterConfigForPersistence( companyId, - inputAdapterConfig, + adapterConfigForTest, { strictMode: strictSecretsMode, adapterType: type }, ); // Prospective, non-persisted config: resolve the acting user's own user diff --git a/ui/src/api/agents.ts b/ui/src/api/agents.ts index 43ec821358..c55fe7c625 100644 --- a/ui/src/api/agents.ts +++ b/ui/src/api/agents.ts @@ -223,6 +223,7 @@ export const agentsApi = { type: string, data: { adapterConfig: Record; + agentId?: string; testCredentials?: Record; environmentId?: string | null; }, diff --git a/ui/src/components/AgentConfigForm.tsx b/ui/src/components/AgentConfigForm.tsx index 1e22442fcc..91534ffb16 100644 --- a/ui/src/components/AgentConfigForm.tsx +++ b/ui/src/components/AgentConfigForm.tsx @@ -1054,15 +1054,16 @@ export function AgentConfigForm(props: AgentConfigFormProps) { visibleEnvironmentIds: environmentList.map((environment) => environment.id), }); const adapterConfig = buildAdapterConfigForTest(adapterConfigPatch); + const agentId = isCreate ? undefined : props.agent.id; if (props.compactTestFeedback) { const providerAdapter = adapterType === "paperclip_runner" ? adapterConfig.provider === "codex" ? "codex_local" : adapterConfig.provider === "acpx" && adapterConfig.acpxAgent === "claude" ? "claude_local" : adapterType : adapterType; - return testAgentSetup({ companyId: selectedCompanyId, adapterType, providerAdapter, adapterConfig, environmentId }); + return testAgentSetup({ companyId: selectedCompanyId, adapterType, providerAdapter, adapterConfig, agentId, environmentId }); } - return agentsApi.testEnvironment(selectedCompanyId, adapterType, { adapterConfig, environmentId }); + return agentsApi.testEnvironment(selectedCompanyId, adapterType, { adapterConfig, agentId, environmentId }); }, }); const [testActionPending, setTestActionPending] = useState(false); diff --git a/ui/src/lib/test-agent-setup.test.ts b/ui/src/lib/test-agent-setup.test.ts index 015d7ab27c..34e926fa4a 100644 --- a/ui/src/lib/test-agent-setup.test.ts +++ b/ui/src/lib/test-agent-setup.test.ts @@ -4,6 +4,7 @@ const testEnvironment = vi.hoisted(() => vi.fn()); vi.mock("../api/agents", () => ({ agentsApi: { testEnvironment } })); const input = { companyId: "company-1", + agentId: "agent-1", adapterType: "paperclip_runner", providerAdapter: "claude_local", environmentId: "sandbox-1", @@ -48,6 +49,7 @@ it("does not report a connection when runtime readiness passes but provider auth "company-1", "claude_local", { + agentId: "agent-1", environmentId: "sandbox-1", adapterConfig: { ...input.adapterConfig, engine: "cli" }, }, diff --git a/ui/src/lib/test-agent-setup.ts b/ui/src/lib/test-agent-setup.ts index 81b9f1aa80..8d0f2dd2e2 100644 --- a/ui/src/lib/test-agent-setup.ts +++ b/ui/src/lib/test-agent-setup.ts @@ -9,11 +9,13 @@ export async function testAgentSetup(input: { adapterType: string; providerAdapter: string; adapterConfig: Record; + agentId?: string; testCredentials?: Record; environmentId: string | null; }): Promise { const payload = { adapterConfig: input.adapterConfig, + ...(input.agentId ? { agentId: input.agentId } : {}), ...(input.testCredentials ? { testCredentials: input.testCredentials } : {}), environmentId: input.environmentId, };