Test adapters in the environment a run would actually use (#10698)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Agents execute in environments — local, SSH, or sandboxes — resolved
at run time as agent environment → instance default → local
> - The Configuration page has a Test button that probes the adapter
(working directory, command, a model call) in the environment it will
run in
> - But the Test sent only the agent's own environment id, with no
instance-default fallback, so agents relying on the instance default
were probed on the Paperclip host instead
> - A sandbox image carrying an extra CLI then fails the Test with
"command not found" even though every real run would resolve to the
sandbox and succeed — the Test lies about a working setup
> - This pull request mirrors the run-time resolution in the Test call
via a small shared helper with tests
> - The benefit is that the Test button reports the truth about where
the agent actually runs
## Linked Issues or Issue Description
No existing public issue — inline description following the bug report
template:
**What happened?**
With the instance default environment set to a sandbox (whose image
includes the adapter CLI) and an agent that leaves its environment unset
("use instance default"), the Configuration page's Test fails with
`command not found` for that CLI.
**Expected behavior**
The Test probes the environment a real run would use — here the
instance-default sandbox, where the CLI exists — and passes.
**Steps to reproduce**
1. Set the instance default environment to a sandbox whose image carries
an adapter CLI not installed on the Paperclip host (e.g. `grok`).
2. Create a `grok_local` agent without selecting an environment.
3. Press Test on the agent's Configuration page → `command not found`,
while a real heartbeat run resolves to the sandbox and works.
**Paperclip version or commit**
Reproduced on `sha-53bcf38-cloud`-era master; root-caused in
`ui/src/components/AgentConfigForm.tsx` (`environmentId =
currentDefaultEnvironmentId || null`) versus the server's
`resolveExecutionWorkspaceEnvironmentId` (agent → instance default →
local).
## What Changed
- New `ui/src/lib/adapter-test-environment.ts`:
`resolveAdapterTestEnvironmentId` — agent environment first, else
instance default, else null (host probe) — documented as the mirror of
the server's run-time resolution.
- `AgentConfigForm` uses it in the Test mutation. The raw agent
environment id is now sent even when it points at the local environment:
the server already resolves the driver and probes the host for local, so
explicit-local behavior is unchanged, and the test-environment route's
remote paths (SSH/sandbox lease + custom-image template) engage exactly
as they do for the fallback environment.
- Tests pin the fallback (agent wins; instance default when agent unset;
null when neither).
Deliberately untouched: the onboarding wizard's adapter test still sends
no environment — during onboarding an instance default frequently
doesn't exist yet, and changing that flow deserves its own look.
## Verification
- `vitest run` on the new helper suite plus both `AgentConfigForm`
suites — 19 tests pass; `tsc` clean in `ui/`.
- Root cause verified against a live deployment: an agent with
`default_environment_id = NULL`, instance default = sandbox environment;
the Test posted `environmentId: null` and probed the host (no `Probing
inside environment: …` check in the result), which lacks the CLI that
the sandbox image carries.
## Risks
- Low. The change only widens which environment the Test probes,
matching run-time reality. Sandbox-backed tests boot a throwaway sandbox
(existing route behavior — lease, custom-image template,
archive-on-release), so Tests for instance-default-sandbox agents now
take sandbox-boot time instead of failing fast and wrongly.
## Model Used
Claude Fable 5 (`claude-fable-5`, extended thinking, via Claude Code
with tool use and code execution); diagnosis included live inspection of
a deployed instance's agent/environment configuration.
## 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
(helper doc-comment carries the rationale; no user-facing doc covers the
Test button)
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
This commit is contained in:
parent
717684ad8f
commit
2ffebd4836
|
|
@ -27,6 +27,7 @@ import {
|
|||
import { Button } from "@/components/ui/button";
|
||||
import { FolderOpen, Heart, ChevronDown, X } from "lucide-react";
|
||||
import { asBoolean, asFiniteNumber, asObject, cn } from "../lib/utils";
|
||||
import { resolveAdapterTestEnvironmentId } from "../lib/adapter-test-environment";
|
||||
import { extractModelName, extractProviderId } from "../lib/model-utils";
|
||||
import { queryKeys } from "../lib/queryKeys";
|
||||
import { useCompany } from "../context/CompanyContext";
|
||||
|
|
@ -662,7 +663,37 @@ export function AgentConfigForm(props: AgentConfigFormProps) {
|
|||
const adapterConfigPatch = flushedEnv ? { env: flushedEnv } : undefined;
|
||||
const primaryModel = currentModelId.trim() || null;
|
||||
const cheapTestCase = getCheapModelTestCase(adapterConfigPatch);
|
||||
const environmentId = currentDefaultEnvironmentId || null;
|
||||
// Probe where a real run would actually execute: the agent's own
|
||||
// environment, else the instance default. Testing the host for an
|
||||
// agent that runs in the instance-default sandbox reports failures
|
||||
// (e.g. a CLI that only exists in the sandbox image) a real run would
|
||||
// never hit. The raw id is sent even for a local environment — the
|
||||
// server resolves the driver and probes the host in that case.
|
||||
//
|
||||
// Test can be clicked before the settings query settles (or after it
|
||||
// failed with retry:false), so when the agent relies on the instance
|
||||
// default, resolve the settings here rather than trusting the
|
||||
// render-time cache. A fetch that still fails FAILS the test with an
|
||||
// honest diagnostic — silently probing the host instead would report
|
||||
// the exact false command-not-found failure this resolution exists to
|
||||
// fix. Agents with their own environment never need the settings.
|
||||
let settings = instanceSettings;
|
||||
if (!rawCurrentDefaultEnvironmentId && settings === undefined) {
|
||||
try {
|
||||
settings = await queryClient.ensureQueryData({
|
||||
queryKey: queryKeys.instance.settings,
|
||||
queryFn: () => instanceSettingsApi.get(),
|
||||
});
|
||||
} catch {
|
||||
throw new Error(
|
||||
"Could not load instance settings to determine which environment to test in. Retry the test.",
|
||||
);
|
||||
}
|
||||
}
|
||||
const environmentId = resolveAdapterTestEnvironmentId({
|
||||
agentDefaultEnvironmentId: rawCurrentDefaultEnvironmentId || null,
|
||||
instanceDefaultEnvironmentId: settings?.defaultEnvironmentId ?? null,
|
||||
});
|
||||
const testResults: Array<{ label: string; model: string | null; result: AdapterEnvironmentTestResult }> = [
|
||||
{
|
||||
label: "Primary model",
|
||||
|
|
|
|||
|
|
@ -0,0 +1,48 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { resolveAdapterTestEnvironmentId } from "./adapter-test-environment";
|
||||
|
||||
describe("resolveAdapterTestEnvironmentId", () => {
|
||||
it("prefers the agent's own environment", () => {
|
||||
expect(
|
||||
resolveAdapterTestEnvironmentId({
|
||||
agentDefaultEnvironmentId: "agent-env",
|
||||
instanceDefaultEnvironmentId: "instance-env",
|
||||
}),
|
||||
).toBe("agent-env");
|
||||
});
|
||||
|
||||
it("falls back to the instance default when the agent has none", () => {
|
||||
// The regression this pins: an agent relying on the instance default
|
||||
// (e.g. a managed sandbox with extra CLIs baked into its image) must be
|
||||
// tested inside that environment, not on the Paperclip host where the
|
||||
// CLI does not exist.
|
||||
expect(
|
||||
resolveAdapterTestEnvironmentId({
|
||||
agentDefaultEnvironmentId: null,
|
||||
instanceDefaultEnvironmentId: "instance-env",
|
||||
}),
|
||||
).toBe("instance-env");
|
||||
expect(
|
||||
resolveAdapterTestEnvironmentId({
|
||||
agentDefaultEnvironmentId: "",
|
||||
instanceDefaultEnvironmentId: "instance-env",
|
||||
}),
|
||||
).toBe("instance-env");
|
||||
});
|
||||
|
||||
it("returns null (host probe) when neither is set", () => {
|
||||
expect(
|
||||
resolveAdapterTestEnvironmentId({
|
||||
agentDefaultEnvironmentId: undefined,
|
||||
instanceDefaultEnvironmentId: undefined,
|
||||
}),
|
||||
).toBeNull();
|
||||
expect(
|
||||
resolveAdapterTestEnvironmentId({
|
||||
agentDefaultEnvironmentId: "",
|
||||
instanceDefaultEnvironmentId: null,
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,18 @@
|
|||
/**
|
||||
* Which environment should an adapter "Test" probe?
|
||||
*
|
||||
* Mirrors the server's run-time resolution
|
||||
* (`resolveExecutionWorkspaceEnvironmentId`): the agent's own environment
|
||||
* wins, otherwise the instance default, otherwise none (the server probes
|
||||
* the Paperclip host). Without the instance-default fallback, the Test
|
||||
* button probes the host for agents that rely on the instance default and
|
||||
* fails on commands that only exist inside the default environment — for
|
||||
* example a sandbox image with an extra CLI installed — even though a real
|
||||
* run would have resolved to that environment and succeeded.
|
||||
*/
|
||||
export function resolveAdapterTestEnvironmentId(input: {
|
||||
agentDefaultEnvironmentId: string | null | undefined;
|
||||
instanceDefaultEnvironmentId: string | null | undefined;
|
||||
}): string | null {
|
||||
return input.agentDefaultEnvironmentId || input.instanceDefaultEnvironmentId || null;
|
||||
}
|
||||
Loading…
Reference in New Issue