fix(server): skip host provisionCommand for sandbox-driver environments (#11626)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The run orchestrator prepares an execution environment before an agent starts > - A sandbox driver creates a remote folder before the adapter uploads repository content > - The orchestrator ran the host `provisionCommand` in that empty folder > - The command failed with exit 127 before the adapter could run its `stage.sync` step > - This pull request skips host provisioning for sandbox drivers and keeps the existing local and SSH behavior > - The benefit is that sandbox runs reach the adapter sync step without an empty-folder setup failure ## Linked Issues or Issue Description **What happened?** A sandbox environment ran the host `provisionCommand` before the adapter uploaded repository content. The command ran in an empty remote folder and failed with exit 127. **Expected behavior** The orchestrator should skip host provisioning for a sandbox driver. The adapter should upload the provisioned tree during its `stage.sync` step. **Steps to reproduce** 1. Configure an environment with the `sandbox` driver and a host `provisionCommand`. 2. Start a run that uses this environment. 3. Observe that the command runs in the empty sandbox folder and the run fails with `setup_failed`. **Paperclip version or commit** Reproduced on the current `master` commit before this change. **Deployment mode** Built from source with a sandbox environment. **Installation method** Built from source with pnpm. **Agent adapter(s) involved** Not adapter-specific (core bug). The sandbox adapter syncs the tree after environment setup. **Database mode** Not database-related. **Additional context** Related context: [#11091](https://github.com/paperclipai/paperclip/pull/11091) changes provision behavior for reused workspaces. This pull request covers the separate sandbox ordering failure. ## What Changed - Skip the orchestrator provision step when `environment.driver` is `sandbox`. - Keep the existing skip for `local` and the provision step for `ssh`. - Log one info message when a sandbox skip drops a present command. - Keep the existing `plugin` path because it has no `stage.sync` step and runs against the host filesystem. - Add tests for sandbox, local, SSH, plugin, logging, and provision failures. ## Verification - Run `./node_modules/.bin/vitest run server/src/__tests__/environment-run-orchestrator.test.ts`. - Confirm that the test run passes all 10 tests. - Confirm that CI checks pass on this pull request. ## Risks - Low risk. The change affects only the provision gate for sandbox drivers. - SSH and local behavior stays unchanged. - The plugin driver stays on its current path. - The new log line makes a sandbox skip visible to operators. ## Model Used OpenAI, GPT-5, exact runtime model `gpt-5`, with tool use and code review support. The implementation author used this model to inspect code, edit source and tests, and run the targeted test suite. ## 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 (no exact duplicate found; related PR #11091 reviewed) - [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 (no documentation change applies to this internal gate correction) - [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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Nicky Leach
GitHub
parent
6b8e42168e
commit
0e9b03832d
|
|
@ -10,6 +10,7 @@ const mockBuildWorkspaceRealizationRequest = vi.hoisted(() => vi.fn());
|
|||
const mockUpdateLeaseMetadata = vi.hoisted(() => vi.fn());
|
||||
const mockUpdateExecutionWorkspace = vi.hoisted(() => vi.fn());
|
||||
const mockLogActivity = vi.hoisted(() => vi.fn());
|
||||
const mockLoggerInfo = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("../services/environment-execution-target.js", () => ({
|
||||
resolveEnvironmentExecutionTarget: mockResolveEnvironmentExecutionTarget,
|
||||
|
|
@ -44,6 +45,15 @@ vi.mock("../services/activity-log.js", () => ({
|
|||
logActivity: mockLogActivity,
|
||||
}));
|
||||
|
||||
vi.mock("../middleware/logger.js", () => ({
|
||||
logger: {
|
||||
info: mockLoggerInfo,
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
debug: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Imports after mocks
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
@ -407,6 +417,84 @@ describe("environmentRunOrchestrator — realizeForRun", () => {
|
|||
});
|
||||
|
||||
it("runs a remote provision command after workspace realization when configured", async () => {
|
||||
mockBuildWorkspaceRealizationRequest.mockReturnValue({
|
||||
version: 1,
|
||||
adapterType: "claude_local",
|
||||
companyId: "company-1",
|
||||
environmentId: "env-1",
|
||||
executionWorkspaceId: null,
|
||||
issueId: null,
|
||||
heartbeatRunId: "run-1",
|
||||
requestedMode: null,
|
||||
source: {
|
||||
kind: "project_primary",
|
||||
localPath: "/workspace/project",
|
||||
projectId: null,
|
||||
projectWorkspaceId: null,
|
||||
repoUrl: null,
|
||||
repoRef: null,
|
||||
strategy: "project_primary",
|
||||
branchName: null,
|
||||
worktreePath: null,
|
||||
},
|
||||
runtimeOverlay: {
|
||||
provisionCommand: "npm install -g @anthropic-ai/claude-code",
|
||||
},
|
||||
});
|
||||
mockResolveEnvironmentExecutionTarget.mockResolvedValue({
|
||||
kind: "remote",
|
||||
transport: "ssh",
|
||||
remoteCwd: "/remote/workspace",
|
||||
environmentId: "env-1",
|
||||
leaseId: "lease-1",
|
||||
spec: {
|
||||
host: "ssh.example.test",
|
||||
port: 22,
|
||||
username: "ssh-user",
|
||||
remoteCwd: "/remote/workspace",
|
||||
remoteWorkspacePath: "/remote/workspace",
|
||||
privateKey: null,
|
||||
knownHosts: null,
|
||||
strictHostKeyChecking: true,
|
||||
},
|
||||
});
|
||||
|
||||
const runtime = makeMockRuntime({
|
||||
realizeWorkspace: vi.fn().mockResolvedValue({
|
||||
cwd: "/remote/workspace",
|
||||
metadata: {
|
||||
workspaceRealization: {
|
||||
version: 1,
|
||||
transport: "ssh",
|
||||
remote: { path: "/remote/workspace" },
|
||||
},
|
||||
},
|
||||
}),
|
||||
});
|
||||
const orchestrator = environmentRunOrchestrator(mockDb, { environmentRuntime: runtime });
|
||||
|
||||
await orchestrator.realizeForRun(makeRealizeInput({
|
||||
environment: makeEnvironment("ssh"),
|
||||
}));
|
||||
|
||||
// The `ssh` driver runs the command on the remote host that shares the
|
||||
// workspace path, so the configured provision command still runs there.
|
||||
expect(runtime.execute).toHaveBeenCalledOnce();
|
||||
expect(runtime.execute).toHaveBeenCalledWith(expect.objectContaining({
|
||||
environment: expect.objectContaining({ driver: "ssh" }),
|
||||
lease: expect.objectContaining({ id: "lease-1" }),
|
||||
command: "bash",
|
||||
args: ["-lc", "npm install -g @anthropic-ai/claude-code"],
|
||||
cwd: "/remote/workspace",
|
||||
env: {
|
||||
SHELL: "/bin/bash",
|
||||
},
|
||||
}));
|
||||
// The sandbox skip log is specific to the sandbox driver; ssh stays quiet.
|
||||
expect(mockLoggerInfo).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("skips the host provision command for a sandbox environment and logs the skip", async () => {
|
||||
mockBuildWorkspaceRealizationRequest.mockReturnValue({
|
||||
version: 1,
|
||||
adapterType: "claude_local",
|
||||
|
|
@ -458,17 +546,59 @@ describe("environmentRunOrchestrator — realizeForRun", () => {
|
|||
environment: makeEnvironment("sandbox"),
|
||||
}));
|
||||
|
||||
expect(runtime.execute).toHaveBeenCalledOnce();
|
||||
expect(runtime.execute).toHaveBeenCalledWith(expect.objectContaining({
|
||||
environment: expect.objectContaining({ driver: "sandbox" }),
|
||||
lease: expect.objectContaining({ id: "lease-1" }),
|
||||
command: "bash",
|
||||
args: ["-lc", "npm install -g @anthropic-ai/claude-code"],
|
||||
cwd: "/remote/workspace",
|
||||
env: {
|
||||
SHELL: "/bin/bash",
|
||||
// The sandbox receives the provisioned tree through the adapter stage.sync
|
||||
// step, so the orchestrator must not run the host command in the sandbox.
|
||||
expect(runtime.execute).not.toHaveBeenCalled();
|
||||
// The skip is observable: exactly one log line records it with the driver.
|
||||
expect(mockLoggerInfo).toHaveBeenCalledOnce();
|
||||
expect(mockLoggerInfo).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ driver: "sandbox", environmentId: "env-1" }),
|
||||
expect.stringContaining("Skip host provisionCommand"),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not rerun the provision command during local environment realization", async () => {
|
||||
mockBuildWorkspaceRealizationRequest.mockReturnValue({
|
||||
version: 1,
|
||||
adapterType: "claude_local",
|
||||
companyId: "company-1",
|
||||
environmentId: "env-1",
|
||||
executionWorkspaceId: null,
|
||||
issueId: null,
|
||||
heartbeatRunId: "run-1",
|
||||
requestedMode: null,
|
||||
source: {
|
||||
kind: "project_primary",
|
||||
localPath: "/workspace/project",
|
||||
projectId: null,
|
||||
projectWorkspaceId: null,
|
||||
repoUrl: null,
|
||||
repoRef: null,
|
||||
strategy: "project_primary",
|
||||
branchName: null,
|
||||
worktreePath: null,
|
||||
},
|
||||
runtimeOverlay: {
|
||||
provisionCommand: "npm install -g @anthropic-ai/claude-code",
|
||||
},
|
||||
});
|
||||
mockResolveEnvironmentExecutionTarget.mockResolvedValue({
|
||||
kind: "local",
|
||||
environmentId: "env-1",
|
||||
leaseId: "lease-1",
|
||||
});
|
||||
|
||||
const runtime = makeMockRuntime();
|
||||
const orchestrator = environmentRunOrchestrator(mockDb, { environmentRuntime: runtime });
|
||||
|
||||
await orchestrator.realizeForRun(makeRealizeInput({
|
||||
environment: makeEnvironment("local"),
|
||||
}));
|
||||
|
||||
// Local workspace provisioning already ran the command before realizeForRun.
|
||||
expect(runtime.execute).not.toHaveBeenCalled();
|
||||
// The sandbox skip log is specific to the sandbox driver; local stays quiet.
|
||||
expect(mockLoggerInfo).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("runs project-level provision commands for ssh environments", async () => {
|
||||
|
|
@ -588,7 +718,7 @@ describe("environmentRunOrchestrator — realizeForRun", () => {
|
|||
const orchestrator = environmentRunOrchestrator(mockDb, { environmentRuntime: runtime });
|
||||
|
||||
await expect(orchestrator.realizeForRun(makeRealizeInput({
|
||||
environment: makeEnvironment("sandbox"),
|
||||
environment: makeEnvironment("ssh"),
|
||||
}))).rejects.toSatisfy(
|
||||
(err: unknown) =>
|
||||
err instanceof EnvironmentRunError &&
|
||||
|
|
|
|||
|
|
@ -45,6 +45,7 @@ import {
|
|||
import { buildWorkspaceRealizationRequest } from "./workspace-realization.js";
|
||||
import { executionWorkspaceService } from "./execution-workspaces.js";
|
||||
import { logActivity } from "./activity-log.js";
|
||||
import { logger } from "../middleware/logger.js";
|
||||
import { parseObject } from "../adapters/utils.js";
|
||||
import type { RealizedExecutionWorkspace } from "./workspace-runtime.js";
|
||||
import type { PluginWorkerManager } from "./plugin-worker-manager.js";
|
||||
|
|
@ -420,7 +421,29 @@ export function environmentRunOrchestrator(
|
|||
(typeof lease.metadata?.remoteCwd === "string" && lease.metadata.remoteCwd.trim().length > 0
|
||||
? lease.metadata.remoteCwd.trim()
|
||||
: executionWorkspace.cwd);
|
||||
if (provisionCommand && environment.driver !== "local") {
|
||||
// The host `provisionCommand` runs on the host worktree during the
|
||||
// `workspace_provision` step, before the run reaches the environment.
|
||||
// A `sandbox`-driver environment does not receive the repo tree here. The
|
||||
// sandbox driver `realizeWorkspace` step only creates the remote folder.
|
||||
// The adapter uploads the provisioned tree later, in its `stage.sync` step.
|
||||
// So the host command must not run inside the still-empty sandbox; it fails
|
||||
// there (exit 127). Skip the step for `sandbox`, and keep the existing skip
|
||||
// for `local`. Keep the step for `ssh`, which runs the command on the
|
||||
// remote host that shares the workspace path.
|
||||
const driverSkipsHostProvision =
|
||||
environment.driver === "local" || environment.driver === "sandbox";
|
||||
if (provisionCommand && environment.driver === "sandbox") {
|
||||
logger.info(
|
||||
{
|
||||
environmentId: environment.id,
|
||||
driver: environment.driver,
|
||||
issueId,
|
||||
heartbeatRunId,
|
||||
},
|
||||
"Skip host provisionCommand for sandbox-driver environment; the adapter stage.sync step delivers the provisioned tree",
|
||||
);
|
||||
}
|
||||
if (provisionCommand && !driverSkipsHostProvision) {
|
||||
try {
|
||||
const provisionResult = await environmentRuntime.execute({
|
||||
environment,
|
||||
|
|
|
|||
Loading…
Reference in New Issue