From f0b06d2de95603977d7edf727797a9c449c64b13 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Tue, 4 Aug 2026 10:31:14 -0700 Subject: [PATCH] feat(claude): environment-aware test-environment probe and claude-local CI coverage (#10833) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The claude_local adapter runs Claude Code on sandbox execution targets, and operators verify an agent's configuration with the test-environment probe before running it > - Real runs merge the selected environment's env vars (secret refs included) under the agent's adapter config env, but the probe built its config from the adapter config alone — so environment-level auth worked in runs while the Test button reported missing auth, and a dropped secret binding passed silently > - The claude env-test hints also did not recognize `CLAUDE_CODE_OAUTH_TOKEN` even though the CLI accepts it, and a hello probe that hit the subscription usage limit reported a hard failure although authentication worked > - Separately, the claude-local package test suites were absent from the CI project list, so two suites drifted broken without notice > - This pull request makes the probe resolve the same layered env as a real run, adds the missing auth hint, classifies usage-limit probe results as a warning, repairs the drifted suites, and turns the claude-local project on in CI > - The benefit is a Test button that tells the truth about environment-level configuration, and a test suite that actually gates the claude-local adapter ## Linked Issues or Issue Description No public issue exists; related open PRs: Refs #9488 (recognizes CLAUDE_CODE_OAUTH_TOKEN in environment checks — overlaps with the auth-hint portion of this PR via a differently named check; it does not cover the environment-envVars probe merge, the usage-limit classification, or the CI coverage), Refs #9933 (live credential validation in environment checks — complementary, no file-level conflict with the route change). The underlying problem, following the enhancement template: **Current behavior** The test-environment route builds the probe config from the agent's adapterConfig only. Real runs merge the selected environment's envVars under the agent env, so environment-level env vars (including auth such as `ANTHROPIC_API_KEY` or `CLAUDE_CODE_OAUTH_TOKEN` bound as environment secrets) work in runs while "Test environment" cannot see them, and a missing secret binding passes silently. The claude env-test hints do not recognize `CLAUDE_CODE_OAUTH_TOKEN`. A hello probe that hits the subscription usage limit reports a hard `claude_hello_probe_failed`. The claude-local package test suites do not run in CI, and two of them are stale. **Proposed behavior** The probe resolves the selected environment's envVars (environment-consumer secret bindings included) and merges them under the agent config env with the run-path precedence; missing bindings surface as an explicit error check that fails the test. The env-test emits a `claude_oauth_token_configured` info check when that variable is set. Usage-limit probe results classify as a `claude_hello_probe_usage_limited` warning because auth works and only the usage window is spent. The claude-local suites run in CI. Docs state the resulting facts. **Reason and benefit** The Test button should tell the truth: it previously contradicted run behavior for environment-level configuration and hid broken secret bindings. Enabling the package suites in CI prevents further silent drift — two suites were already broken on master without anyone noticing. **Breaking changes** None. Runs are unchanged. The probe route only adds env layers and checks; setups without environment envVars behave exactly as before. ## What Changed - `server/src/routes/agents.ts`: the test-environment route resolves the selected environment's envVars (forbidden keys stripped, environment-consumer secret context) and merges them under the agent adapterConfig env, mirroring `resolveExecutionRunAdapterConfig` precedence. Missing secret bindings are skipped, reported as an `environment_env_binding_missing` error check, and fail the test — matching the `ConfigurationIncompleteFailure` a real dispatch would raise. - `packages/adapters/claude-local/src/server/test.ts`: new `claude_oauth_token_configured` info hint between the API-key warning and the subscription fallback; hello-probe classification gains a `claude_hello_probe_usage_limited` warning for provider-quota results (previously a hard `claude_hello_probe_failed`). - `scripts/run-vitest-stable.mjs`: add `@paperclipai/adapter-claude-local` to `nonServerProjects` so CI runs the package suites. - `packages/adapters/claude-local/src/server/execute.remote.test.ts`: assert both runtime asset syncs (skills and mcp-config); the suite predated the mcp-config asset. - `packages/adapters/claude-local/src/server/test.probe.test.ts`: usage-limit fixture now expects the usage-limited warning; new fixture covers the genuine transient path (529 overloaded); new tests cover the token hint and API-key precedence. - `server/src/__tests__/agent-test-environment-routes.test.ts`: new tests for the env merge (agent wins on conflict, forbidden key filtered), missing-binding reporting, and the no-execution-target fallback path. - `docs/adapters/claude-local.md`, `docs/adapters/overview.md`: state the auth-input facts (API key or oauth token wins over stored logins; snapshot-owns-auth applies when neither is configured) and describe the environment-aware Test behavior. ## Verification - `npx vitest run --project @paperclipai/adapter-claude-local` — 131 tests pass (both drifted suites repaired; they fail on master today). - `npx vitest run server/src/__tests__/agent-test-environment-routes.test.ts` — 7 tests pass. - `node --test ./scripts/__tests__/run-vitest-stable-shard.test.mjs` — passes with the added project. - `pnpm typecheck` in `server/` and `packages/adapters/claude-local/` — clean. ## Risks - Low risk. The run path is untouched; the probe route change is additive and inert when the environment has no envVars. - The probe now performs environment-consumer secret resolution at test time; access is authorized per binding exactly as at run time, and the audit consumer is the environment (as before for adapter-config resolution). - Enabling the claude-local project in CI adds about 2 seconds of vitest wall time to the general workspaces group and could surface future regressions in that package — which is the point. ## Model Used Claude Fable 5 (`claude-fable-5`), extended thinking enabled, agentic tool use via Claude Code (CLI). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- docs/adapters/claude-local.md | 15 +- docs/adapters/overview.md | 2 +- .../src/server/execute.remote.test.ts | 7 +- .../src/server/test.probe.test.ts | 81 ++++++++- .../adapters/claude-local/src/server/test.ts | 54 ++++-- scripts/run-vitest-stable.mjs | 1 + .../agent-test-environment-routes.test.ts | 159 ++++++++++++++++++ server/src/routes/agents.ts | 80 ++++++++- 8 files changed, 376 insertions(+), 23 deletions(-) diff --git a/docs/adapters/claude-local.md b/docs/adapters/claude-local.md index 76cb4b337b..1a13d0459c 100644 --- a/docs/adapters/claude-local.md +++ b/docs/adapters/claude-local.md @@ -8,8 +8,9 @@ The `claude_local` adapter runs Anthropic's Claude Code CLI locally. It supports ## Prerequisites - Claude Code CLI installed (`claude` command available) -- Either `ANTHROPIC_API_KEY` in adapter env/host env, or a Claude Code - subscription login available to the execution target +- Either `ANTHROPIC_API_KEY` or `CLAUDE_CODE_OAUTH_TOKEN` in adapter or + environment env (or host env), or a Claude Code subscription login + available to the execution target ## Configuration Fields @@ -72,6 +73,7 @@ The adapter creates a temporary directory with symlinks to Paperclip skills and ## Remote credential ownership +When no API key or `CLAUDE_CODE_OAUTH_TOKEN` is configured, `claude_local` uses a snapshot-owns-auth topology for managed sandbox execution targets. When the run uses a sandbox execution target and no explicit `CLAUDE_CONFIG_DIR` is configured, Paperclip creates a remote @@ -111,5 +113,12 @@ Use the "Test Environment" button in the UI to validate the adapter config. It c - Claude CLI is installed and accessible - Working directory is absolute and available (auto-created if missing and permitted) -- API key/auth mode hints (`ANTHROPIC_API_KEY` vs subscription login) +- API key/auth mode hints (`ANTHROPIC_API_KEY` vs `CLAUDE_CODE_OAUTH_TOKEN` vs subscription login) - A live hello probe (`claude --print - --output-format stream-json --verbose` with prompt `Respond with hello.`) to verify CLI readiness + +The probe sees the same layered env as a real run: when an environment is +selected, its environment variables (secret refs included) are resolved and +merged under the adapter config's `env`, so environment-level auth is +reflected in the test result. A secret binding that is missing surfaces as +an `environment_env_binding_missing` failure instead of a silently passing +probe. diff --git a/docs/adapters/overview.md b/docs/adapters/overview.md index 83c14e10ef..6fdf1723f3 100644 --- a/docs/adapters/overview.md +++ b/docs/adapters/overview.md @@ -39,7 +39,7 @@ before the CLI starts: | Adapter | Credential topology | Which credential file wins on managed sandbox targets | |---------|---------------------|-------------------------------------------------------| | [`codex_local`](/adapters/codex-local) | Host-owns-auth for Paperclip-managed `CODEX_HOME` | A host-owned `auth.json` is symlinked into the managed `CODEX_HOME` and uploaded to the sandbox. If a per-agent `OPENAI_API_KEY` is configured, Paperclip writes an API-key `auth.json` instead and that file wins. A login baked into the sandbox image is shadowed because Codex runs with Paperclip's uploaded `CODEX_HOME`. | -| [`claude_local`](/adapters/claude-local) | Snapshot-owns-auth for managed remote Claude config | Paperclip uploads only sanitized settings and skill/runtime assets. When the remote managed config has no Claude credential files, it copies `.credentials.json` or `credentials.json` from the sandbox image's own `$HOME/.claude`, so the image's login wins. | +| [`claude_local`](/adapters/claude-local) | Snapshot-owns-auth for managed remote Claude config | A configured `ANTHROPIC_API_KEY` or `CLAUDE_CODE_OAUTH_TOKEN` (agent or environment env) wins over any stored login. Otherwise Paperclip uploads only sanitized settings and skill/runtime assets, and when the remote managed config has no Claude credential files it copies `.credentials.json` or `credentials.json` from the sandbox image's own `$HOME/.claude`, so the image's login wins. | Worked examples: diff --git a/packages/adapters/claude-local/src/server/execute.remote.test.ts b/packages/adapters/claude-local/src/server/execute.remote.test.ts index fad8dc2bb4..ab27ab03f6 100644 --- a/packages/adapters/claude-local/src/server/execute.remote.test.ts +++ b/packages/adapters/claude-local/src/server/execute.remote.test.ts @@ -169,11 +169,16 @@ describe("claude remote execution", () => { localDir: workspaceDir, remoteDir: managedRemoteWorkspace, })); - expect(syncDirectoryToSsh).toHaveBeenCalledTimes(1); + // One sync per registered runtime asset: skills and mcp-config. + expect(syncDirectoryToSsh).toHaveBeenCalledTimes(2); expect(syncDirectoryToSsh).toHaveBeenCalledWith(expect.objectContaining({ remoteDir: `${managedRemoteWorkspace}/.paperclip-runtime/claude/skills`, followSymlinks: true, })); + expect(syncDirectoryToSsh).toHaveBeenCalledWith(expect.objectContaining({ + remoteDir: `${managedRemoteWorkspace}/.paperclip-runtime/claude/mcp-config`, + followSymlinks: true, + })); expect(runChildProcess).toHaveBeenCalledTimes(1); const call = runChildProcess.mock.calls[0] as unknown as | [string, string, string[], { env: Record; remoteExecution?: { remoteCwd: string } | null }] diff --git a/packages/adapters/claude-local/src/server/test.probe.test.ts b/packages/adapters/claude-local/src/server/test.probe.test.ts index 9ff7b5f0e8..58088fa47d 100644 --- a/packages/adapters/claude-local/src/server/test.probe.test.ts +++ b/packages/adapters/claude-local/src/server/test.probe.test.ts @@ -101,7 +101,7 @@ describe("claude sandbox hello probe diagnostics", () => { expect(failed?.detail).not.toContain('"subtype":"init"'); }); - it("classifies rate-limit/overload failures as a transient warning, not a hard fail", async () => { + it("classifies subscription usage-limit failures as a usage-limited warning, not a hard fail", async () => { probeResult.value = { exitCode: 1, stdout: [ @@ -119,7 +119,31 @@ describe("claude sandbox hello probe diagnostics", () => { environmentName: "Daytona", }); + expect(result.checks.some((check) => check.code === "claude_hello_probe_usage_limited")).toBe(true); + expect(result.checks.some((check) => check.code === "claude_hello_probe_transient_upstream")).toBe(false); + expect(result.checks.some((check) => check.code === "claude_hello_probe_failed")).toBe(false); + }); + + it("classifies overload failures as a transient warning, not a hard fail", async () => { + probeResult.value = { + exitCode: 1, + stdout: [ + initLine, + '{"type":"result","subtype":"error_during_execution","is_error":true,"result":"API Error: 529 overloaded_error","session_id":"abc"}', + ].join("\n"), + stderr: "", + }; + + const result = await testEnvironment({ + companyId: "company-1", + adapterType: "claude_local", + config: { engine: "cli", command: "claude" }, + executionTarget: sandboxTarget, + environmentName: "Daytona", + }); + expect(result.checks.some((check) => check.code === "claude_hello_probe_transient_upstream")).toBe(true); + expect(result.checks.some((check) => check.code === "claude_hello_probe_usage_limited")).toBe(false); expect(result.checks.some((check) => check.code === "claude_hello_probe_failed")).toBe(false); }); @@ -161,3 +185,58 @@ describe("claude sandbox hello probe diagnostics", () => { expect(failed?.detail).toBeUndefined(); }); }); + +describe("claude auth mode hints", () => { + const successStdout = [ + initLine, + '{"type":"result","subtype":"success","is_error":false,"result":"hello","session_id":"abc"}', + ].join("\n"); + + it("reports the configured subscription token for remote targets", async () => { + probeResult.value = { exitCode: 0, stdout: successStdout, stderr: "" }; + + const result = await testEnvironment({ + companyId: "company-1", + adapterType: "claude_local", + config: { + engine: "cli", + command: "claude", + env: { CLAUDE_CODE_OAUTH_TOKEN: "oauth-test-token" }, + }, + executionTarget: sandboxTarget, + environmentName: "Daytona", + }); + + const hint = result.checks.find((check) => check.code === "claude_oauth_token_configured"); + expect(hint).toBeTruthy(); + expect(hint?.level).toBe("info"); + expect(hint?.detail).toContain("configured environment variables"); + expect( + result.checks.some((check) => check.code === "claude_anthropic_api_key_overrides_subscription"), + ).toBe(false); + }); + + it("keeps the API-key warning authoritative when both ANTHROPIC_API_KEY and the token are set", async () => { + probeResult.value = { exitCode: 0, stdout: successStdout, stderr: "" }; + + const result = await testEnvironment({ + companyId: "company-1", + adapterType: "claude_local", + config: { + engine: "cli", + command: "claude", + env: { + ANTHROPIC_API_KEY: "api-test-key", + CLAUDE_CODE_OAUTH_TOKEN: "oauth-test-token", + }, + }, + executionTarget: sandboxTarget, + environmentName: "Daytona", + }); + + expect( + result.checks.some((check) => check.code === "claude_anthropic_api_key_overrides_subscription"), + ).toBe(true); + expect(result.checks.some((check) => check.code === "claude_oauth_token_configured")).toBe(false); + }); +}); diff --git a/packages/adapters/claude-local/src/server/test.ts b/packages/adapters/claude-local/src/server/test.ts index caedc9d732..1c50a7ae47 100644 --- a/packages/adapters/claude-local/src/server/test.ts +++ b/packages/adapters/claude-local/src/server/test.ts @@ -28,6 +28,7 @@ import { import { describeClaudeFailure, detectClaudeLoginRequired, + isClaudeProviderQuotaError, isClaudeTransientUpstreamError, parseClaudeStreamJson, } from "./parse.js"; @@ -278,6 +279,20 @@ export async function testEnvironment( detail: `Detected in ${source}.`, hint: "Unset ANTHROPIC_API_KEY if you want subscription-based Claude login behavior.", }); + } else if ( + isNonEmpty(env.CLAUDE_CODE_OAUTH_TOKEN) || + (considerHostEnv && isNonEmpty(process.env.CLAUDE_CODE_OAUTH_TOKEN)) + ) { + const source = isNonEmpty(env.CLAUDE_CODE_OAUTH_TOKEN) + ? "configured environment variables" + : "server environment"; + checks.push({ + code: "claude_oauth_token_configured", + level: "info", + message: + "CLAUDE_CODE_OAUTH_TOKEN is set. Claude will authenticate with the configured subscription token; no stored login is needed on the execution target.", + detail: `Detected in ${source}.`, + }); } else if (!targetIsRemote) { checks.push({ code: "claude_subscription_mode_possible", @@ -428,27 +443,44 @@ export async function testEnvironment( (stdoutFallback ? truncateDetail(stdoutFallback) : "") || detail || ""; + // Provider-quota exhaustion (usage/session limit) is classified + // separately from generic transient upstream errors: auth works, the + // subscription's usage window is just spent. Surface it as its own + // warning instead of a hard probe failure. + const usageLimited = isClaudeProviderQuotaError({ + parsed, + stdout: probe.stdout, + stderr: probe.stderr, + }); const transient = isClaudeTransientUpstreamError({ parsed, stdout: probe.stdout, stderr: probe.stderr, }); checks.push( - transient + usageLimited ? { - code: "claude_hello_probe_transient_upstream", + code: "claude_hello_probe_usage_limited", level: "warn", - message: "Claude hello probe hit a transient upstream error (rate limit or overload).", + message: "Claude hello probe hit the subscription usage limit.", ...(failureDetail ? { detail: failureDetail } : {}), - hint: "This is usually temporary. Wait a moment and re-run Test.", + hint: "Authentication works; the account's usage window is exhausted. Wait for the limit to reset and re-run Test.", } - : { - code: "claude_hello_probe_failed", - level: "error", - message: "Claude hello probe failed.", - ...(failureDetail ? { detail: failureDetail } : {}), - hint: `Exit code ${probe.exitCode ?? "unknown"}. Run \`claude --print - --output-format stream-json --verbose\` manually in this directory and prompt \`Respond with hello\` to debug.`, - }, + : transient + ? { + code: "claude_hello_probe_transient_upstream", + level: "warn", + message: "Claude hello probe hit a transient upstream error (rate limit or overload).", + ...(failureDetail ? { detail: failureDetail } : {}), + hint: "This is usually temporary. Wait a moment and re-run Test.", + } + : { + code: "claude_hello_probe_failed", + level: "error", + message: "Claude hello probe failed.", + ...(failureDetail ? { detail: failureDetail } : {}), + hint: `Exit code ${probe.exitCode ?? "unknown"}. Run \`claude --print - --output-format stream-json --verbose\` manually in this directory and prompt \`Respond with hello\` to debug.`, + }, ); } } diff --git a/scripts/run-vitest-stable.mjs b/scripts/run-vitest-stable.mjs index d37ee4d405..4712c86fec 100644 --- a/scripts/run-vitest-stable.mjs +++ b/scripts/run-vitest-stable.mjs @@ -19,6 +19,7 @@ const nonServerProjects = [ "@paperclipai/skills-catalog", "@paperclipai/db", "@paperclipai/adapter-utils", + "@paperclipai/adapter-claude-local", "@paperclipai/adapter-codex-local", "@paperclipai/adapter-opencode-local", "@paperclipai/plugin-sdk", diff --git a/server/src/__tests__/agent-test-environment-routes.test.ts b/server/src/__tests__/agent-test-environment-routes.test.ts index de490e11ba..fef44e1e1a 100644 --- a/server/src/__tests__/agent-test-environment-routes.test.ts +++ b/server/src/__tests__/agent-test-environment-routes.test.ts @@ -19,6 +19,12 @@ const mockAccessService = vi.hoisted(() => ({ const mockSecretService = vi.hoisted(() => ({ normalizeAdapterConfigForPersistence: vi.fn(async (_companyId: string, config: Record) => config), resolveAdapterConfigForRuntime: vi.fn(async (_companyId: string, config: Record) => ({ config })), + collectMissingRuntimeBindings: vi.fn(async () => [] as Array>), + resolveEnvBindings: vi.fn(async () => ({ + env: {} as Record, + secretKeys: new Set(), + manifest: [], + })), })); const mockEnvironmentService = vi.hoisted(() => ({ @@ -348,4 +354,157 @@ describe("agent test-environment route", () => { status: "failed", }); }); + + describe("environment envVars merge", () => { + const environmentId = "11111111-1111-4111-8111-111111111111"; + const sandboxExecutionTarget = { + kind: "remote", + transport: "sandbox", + remoteCwd: "/home/user/paperclip-workspace", + providerKey: "fake-plugin", + runner: { execute: vi.fn() }, + }; + + it("merges resolved environment envVars under the agent adapterConfig env", async () => { + mockEnvironmentService.getById.mockResolvedValue({ + id: environmentId, + companyId: "company-1", + name: "Sandbox QA", + driver: "sandbox", + config: { provider: "fake-plugin" }, + envVars: { + CLAUDE_CODE_OAUTH_TOKEN: { type: "secret_ref", secretId: "secret-1" }, + FOO: { type: "plain", value: "env-foo" }, + PAPERCLIP_API_KEY: { type: "plain", value: "must-not-flow" }, + }, + }); + mockResolveEnvironmentExecutionTarget.mockResolvedValueOnce(sandboxExecutionTarget); + mockSecretService.resolveEnvBindings.mockResolvedValueOnce({ + env: { CLAUDE_CODE_OAUTH_TOKEN: "resolved-token", FOO: "env-foo" }, + secretKeys: new Set(["CLAUDE_CODE_OAUTH_TOKEN"]), + manifest: [], + }); + const app = await createApp(); + + const res = await request(app) + .post("/api/companies/company-1/adapters/external_test/test-environment") + .send({ + adapterConfig: { env: { FOO: "agent-foo" } }, + environmentId, + }); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockSecretService.resolveEnvBindings).toHaveBeenCalledWith( + "company-1", + { + CLAUDE_CODE_OAUTH_TOKEN: { type: "secret_ref", secretId: "secret-1" }, + FOO: { type: "plain", value: "env-foo" }, + }, + expect.objectContaining({ + consumerType: "environment", + consumerId: environmentId, + }), + ); + expect(testEnvironmentSpy).toHaveBeenCalledTimes(1); + // Environment env is the base layer; the agent's own env wins on conflicts. + expect(testEnvironmentSpy.mock.calls[0]?.[0]?.config?.env).toEqual({ + CLAUDE_CODE_OAUTH_TOKEN: "resolved-token", + FOO: "agent-foo", + }); + expect(res.body.status).toBe("pass"); + }); + + it("skips env vars with missing secret bindings and fails the test", async () => { + mockEnvironmentService.getById.mockResolvedValue({ + id: environmentId, + companyId: "company-1", + name: "Sandbox QA", + driver: "sandbox", + config: { provider: "fake-plugin" }, + envVars: { + MISSING_TOKEN: { type: "secret_ref", secretId: "secret-gone" }, + GOOD: { type: "plain", value: "ok" }, + }, + }); + mockResolveEnvironmentExecutionTarget.mockResolvedValueOnce(sandboxExecutionTarget); + mockSecretService.collectMissingRuntimeBindings.mockResolvedValueOnce([ + { + consumerType: "environment", + consumerId: environmentId, + configPath: "env.MISSING_TOKEN", + envKey: "MISSING_TOKEN", + secretId: "secret-gone", + secretName: "Gone", + }, + ]); + mockSecretService.resolveEnvBindings.mockResolvedValueOnce({ + env: { GOOD: "ok" }, + secretKeys: new Set(), + manifest: [], + }); + const app = await createApp(); + + const res = await request(app) + .post("/api/companies/company-1/adapters/external_test/test-environment") + .send({ adapterConfig: {}, environmentId }); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + // The unresolved key is excluded from resolution; the rest still flows. + expect(mockSecretService.resolveEnvBindings).toHaveBeenCalledWith( + "company-1", + { GOOD: { type: "plain", value: "ok" } }, + expect.objectContaining({ consumerType: "environment" }), + ); + expect(testEnvironmentSpy).toHaveBeenCalledTimes(1); + expect(testEnvironmentSpy.mock.calls[0]?.[0]?.config?.env).toEqual({ GOOD: "ok" }); + // A missing binding blocks real dispatch, so the test reports fail even + // though the adapter probe itself passed. + expect(res.body.status).toBe("fail"); + expect(res.body.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: "environment_env_binding_missing", + level: "error", + message: expect.stringContaining("MISSING_TOKEN"), + }), + ]), + ); + }); + + it("includes environment env checks when the environment cannot produce an execution target", async () => { + mockEnvironmentService.getById.mockResolvedValue({ + id: environmentId, + companyId: "company-1", + name: "Sandbox QA", + driver: "sandbox", + config: { provider: "fake-plugin" }, + envVars: { + MISSING_TOKEN: { type: "secret_ref", secretId: "secret-gone" }, + }, + }); + mockSecretService.collectMissingRuntimeBindings.mockResolvedValueOnce([ + { + consumerType: "environment", + consumerId: environmentId, + configPath: "env.MISSING_TOKEN", + envKey: "MISSING_TOKEN", + secretId: "secret-gone", + secretName: "Gone", + }, + ]); + const app = await createApp(); + + const res = await request(app) + .post("/api/companies/company-1/adapters/external_test/test-environment") + .send({ adapterConfig: {}, environmentId }); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(testEnvironmentSpy).not.toHaveBeenCalled(); + expect(res.body.status).toBe("fail"); + expect(res.body.checks).toEqual([ + expect.objectContaining({ code: "environment_target_unsupported", level: "warn" }), + expect.objectContaining({ code: "environment_env_binding_missing", level: "error" }), + ]); + }); + }); }); diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index d3659591b2..70a5b3400c 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -30,6 +30,8 @@ import { LOW_TRUST_REVIEW_PRESET, } from "@paperclipai/shared"; import { + isForbiddenConfigEnvKey, + parseObject, resolvePaperclipInstanceRootForAdapter, readPaperclipSkillSyncPreference, writePaperclipSkillSyncPreference, @@ -1836,20 +1838,77 @@ export function agentRoutes( let releaseStatus: "released" | "failed" = "released"; try { + // Mirror the run path (resolveExecutionRunAdapterConfig): the selected + // environment's envVars are the base env layer and the agent's + // adapterConfig.env wins on key conflicts. Without this merge the probe + // cannot see environment-level auth (e.g. CLAUDE_CODE_OAUTH_TOKEN) that + // real runs receive. + const environmentEnvChecks: AdapterEnvironmentCheck[] = []; + let effectiveAdapterConfig = runtimeAdapterConfig; + if (requestedEnvironmentId) { + const selectedEnvironment = await environmentsSvc.getById(requestedEnvironmentId); + const environmentEnv = Object.fromEntries( + Object.entries(parseObject(selectedEnvironment?.envVars)).filter( + ([key]) => !isForbiddenConfigEnvKey(key), + ), + ); + if (Object.keys(environmentEnv).length > 0) { + const environmentSecretContext = buildActorSecretContext(req, { + consumerType: "environment", + consumerId: requestedEnvironmentId, + }); + const missingBindings = + typeof secretsSvc.collectMissingRuntimeBindings === "function" + ? await secretsSvc.collectMissingRuntimeBindings( + companyId, + environmentEnv, + environmentSecretContext, + ) + : []; + const missingKeys = new Set(missingBindings.map((binding) => binding.envKey)); + if (missingKeys.size > 0) { + environmentEnvChecks.push({ + code: "environment_env_binding_missing", + level: "error", + message: `Environment variables with missing secret bindings were skipped: ${[...missingKeys].join(", ")}.`, + hint: "Re-save the environment's variables to restore the secret binding, then test again.", + }); + } + const resolvableEnvironmentEnv = Object.fromEntries( + Object.entries(environmentEnv).filter(([key]) => !missingKeys.has(key)), + ); + const environmentEnvResolution = await secretsSvc.resolveEnvBindings( + companyId, + resolvableEnvironmentEnv, + environmentSecretContext, + ); + if (Object.keys(environmentEnvResolution.env).length > 0) { + effectiveAdapterConfig = { + ...runtimeAdapterConfig, + env: { + ...environmentEnvResolution.env, + ...parseObject(runtimeAdapterConfig.env), + }, + }; + } + } + } + // If the caller explicitly selected an environment, never fall back to // probing the host when we couldn't resolve that environment's // execution target. Surface the diagnostic checks instead. if (requestedEnvironmentId && !executionTarget && fallbackChecks.length > 0) { - const status: AdapterEnvironmentTestResult["status"] = fallbackChecks.some((c) => c.level === "error") + const combinedChecks = [...fallbackChecks, ...environmentEnvChecks]; + const status: AdapterEnvironmentTestResult["status"] = combinedChecks.some((c) => c.level === "error") ? "fail" - : fallbackChecks.some((c) => c.level === "warn") + : combinedChecks.some((c) => c.level === "warn") ? "warn" : "pass"; if (status === "fail") releaseStatus = "failed"; const synthesized: AdapterEnvironmentTestResult = { adapterType: type, status, - checks: fallbackChecks, + checks: combinedChecks, testedAt: new Date().toISOString(), }; res.json(synthesized); @@ -1859,15 +1918,24 @@ export function agentRoutes( const result = await adapter.testEnvironment({ companyId, adapterType: type, - config: runtimeAdapterConfig, + config: effectiveAdapterConfig, executionTarget, environmentName, }); - if (result.status === "fail") releaseStatus = "failed"; + const prefixChecks = [ + ...(sandboxIdentityCheck ? [sandboxIdentityCheck] : []), + ...environmentEnvChecks, + ]; + // A missing environment secret binding blocks real dispatch + // (ConfigurationIncompleteFailure in the heartbeat), so the test + // reports fail even when the adapter probe itself passed. + const status = environmentEnvChecks.some((c) => c.level === "error") ? "fail" : result.status; + if (status === "fail") releaseStatus = "failed"; res.json({ ...result, - checks: sandboxIdentityCheck ? [sandboxIdentityCheck, ...result.checks] : result.checks, + status, + checks: prefixChecks.length > 0 ? [...prefixChecks, ...result.checks] : result.checks, }); } catch (err) { releaseStatus = "failed";