mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-07 06:16:06 -10:00
Reject relative local Runner paths before native admission (#19776)
## Thinking Path > - Paperclip manages AI agents and their work. > - Heartbeat selects the runtime that will execute a task. > - A new local Native run uses the controller's Runner binary setting. > - A relative binary path is invalid. The resolver already rejects it, but that check occurs after Native admission. > - This pull request checks the same setting before it creates Native execution authority. > - Remote providers and admitted Native recovery keep their existing behavior. ## Linked Issues or Issue Description **What happened?** A relative `PAPERCLIP_RUNNER_BINARY` setting can reach local Native admission before the binary resolver rejects it. The run has transferred preparation ownership even though this setting cannot start the local Runner. **Expected behavior** Reject this invalid setting before creating a completion contract or Native coordinator. Keep the existing preparation owner and the run row unchanged. **Steps to reproduce** Set `PAPERCLIP_RUNNER_BINARY=relative-runner`. Select a new local `paperclip_runner` Native run. The selection must report the existing absolute-path error before Native admission. Refs: #19466 and #19340. Those merged changes repair Retry authority and saved-workspace recovery. This change only moves local path validation before new admission. ## What Changed - Add a pure absolute-path check beside the existing binary resolver. Both use the actual controller environment setting. - Run that check before a new local Runner admission. Skip remote execution, API-only providers, and an already admitted Native run. - Test the rejection, unchanged preparation ownership, controller setting selection, remote behavior, and immutable recovery. Keep the existing resolver checks. ## Verification - The full binary test file passed: 32 tests. - The full runtime-selection file passed against a dedicated PostgreSQL database: 62 tests. It exercises real admission transactions, contracts, coordinator rows, and instruction-copy controls. It replaces context materialization and remote workspace synchronization at their existing test boundaries. - After normal integration with current master, the normal Shared/SDK, database, eval-kernel and Runner builds passed, together with all seven normal application TypeScript programs. The original recovery-preparation case passed again on this integrated revision against a fresh dedicated database (1 passed; 61 filtered). The earlier full 32-test binary and 62-test selection results carry only where their source and executed boundaries are unchanged. - The first private selection wrapper failed during module resolution, before any case or database use. A TypeScript wrapper with the same body recovered that setup failure. This was not a product regression. - Reproduce with `pnpm --filter @paperclipai/server exec vitest run src/services/native-runtime/native-codex-runner.test.ts --maxWorkers=1 --no-file-parallelism`. - Set `PAPERCLIP_RUNTIME_SELECTION_TEST_DATABASE_URL` to a dedicated disposable test database before running `pnpm --filter @paperclipai/server exec vitest run src/services/heartbeat/runtime-selection.test.ts --maxWorkers=1 --no-file-parallelism`. ## Risks - This check validates path syntax only. An absolute path can still fail the existing file-access or artifact checks later. - A wrong branch condition could reject remote or existing Native work. The tests cover these exclusions. - This does not prove that a real provider can start or that a deployed service is healthy. It changes no database schema or retry authority. ## Model Used OpenAI `gpt-6.1-sol` through Codex, with source inspection and code-execution tools. The host does not expose the context-window size. ## 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 - [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 <[email protected]>
This commit is contained in:
1 parent
ee1e1a3994
commit
ae2d2956b3
4 files changed
+79
-3
No files matched your search
@@ -95,6 +95,53 @@ describe.skipIf(!support.supported)("heartbeat runtime selection boundary", () =
|
||||
const coordinators = (id: string) => db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, id));
|
||||
const sandbox = { kind: "remote" as const, transport: "sandbox" as const, remoteCwd: "/workspace", reusableLeaseConfigured: false };
|
||||
|
||||
it("rejects invalid local Runner configuration before native authority and retains the preparation owner", async () => {
|
||||
const f = await fixture();
|
||||
await claimPreparation(f);
|
||||
vi.stubEnv("PAPERCLIP_RUNNER_BINARY", "relative-invalid-dot-retry-controller");
|
||||
f.input.runtimeEnv = { PAPERCLIP_RUNNER_BINARY: join(tmpdir(), "ignored-injected-runnerd") };
|
||||
const before = await read(f.run.id);
|
||||
await expect(selectHeartbeatRuntime(db, f.input)).rejects.toThrow("PAPERCLIP_RUNNER_BINARY must be an absolute path");
|
||||
expect(await read(f.run.id)).toEqual(before);
|
||||
expect(await coordinators(f.run.id)).toEqual([]);
|
||||
expect(await db.select().from(completionContracts)).toHaveLength(0);
|
||||
expect(f.input.stopControllerLease).not.toHaveBeenCalled();
|
||||
expect(buildNativeRuntimeContext).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("ignores an injected runtime Runner value that the local backend does not consume", async () => {
|
||||
const f = await fixture();
|
||||
vi.stubEnv("PAPERCLIP_RUNNER_BINARY", join(tmpdir(), "configuration-only-controller-runnerd"));
|
||||
f.input.runtimeEnv = { PAPERCLIP_RUNNER_BINARY: "relative-ignored-injected-runnerd" };
|
||||
expect((await selectHeartbeatRuntime(db, f.input)).selected).toBe(true);
|
||||
expect((await read(f.run.id)).runtimeMode).toBe("native");
|
||||
expect(await coordinators(f.run.id)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("does not apply local Runner configuration to remote native selection", async () => {
|
||||
const f = await fixture();
|
||||
vi.stubEnv("PAPERCLIP_RUNNER_BINARY", "relative-unused-remote-controller");
|
||||
f.input.workspace.executionTarget = sandbox;
|
||||
expect((await selectHeartbeatRuntime(db, f.input)).selected).toBe(true);
|
||||
expect((await read(f.run.id)).runtimeMode).toBe("native");
|
||||
expect(await coordinators(f.run.id)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("preserves immutable native recovery when current local Runner configuration is invalid", async () => {
|
||||
const f = await fixture();
|
||||
const selected = await selectHeartbeatRuntime(db, f.input);
|
||||
if (!selected.selected || !selected.nativeExecution) throw new Error("Expected native selection");
|
||||
const saved = await read(f.run.id);
|
||||
f.input.run = saved;
|
||||
f.input.session.persistedRunnerProfile = saved.runnerProfileJson ?? {};
|
||||
f.input.session.persistedNativeExecutionInput = selected.nativeExecution;
|
||||
vi.stubEnv("PAPERCLIP_RUNNER_BINARY", "relative-unused-recovery-controller");
|
||||
const resumed = await selectHeartbeatRuntime(db, f.input);
|
||||
expect(resumed).toMatchObject({ selected: true, nativeExecution: selected.nativeExecution, nativeRunnerInstanceId: selected.nativeRunnerInstanceId });
|
||||
expect((await read(f.run.id)).runnerProfileJson).toEqual(saved.runnerProfileJson);
|
||||
expect(await coordinators(f.run.id)).toHaveLength(1);
|
||||
});
|
||||
|
||||
async function connectorFixture(durable = true) {
|
||||
const f = await fixture(), { input } = f;
|
||||
const home = await mkdtemp(join(tmpdir(), "selection-connector-assets-"));
|
||||
@@ -334,6 +381,7 @@ describe.skipIf(!support.supported)("heartbeat runtime selection boundary", () =
|
||||
it.each([false, true])("persists the Cursor provider API placement and completion authority without a Computer lease (ephemeral: %s)", async ephemeral => {
|
||||
vi.stubEnv("PAPERCLIP_EPHEMERAL_SERVER", String(ephemeral));
|
||||
const { input, run, issue } = await fixture();
|
||||
vi.stubEnv("PAPERCLIP_RUNNER_BINARY", "relative-unused-api-controller");
|
||||
input.agent = { ...input.agent, adapterConfig: { provider: "cursor_cloud", model: "caller-model", repoUrl: "https://github.com/example/qualification" } };
|
||||
await db.update(agents).set({ adapterConfig: input.agent.adapterConfig }).where(eq(agents.id, input.agent.id));
|
||||
input.workspace.activeEnvironmentLease = null;
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { assertPaperclipRunnerBinaryConfiguration } from "../native-runtime/native-codex-runner.js";
|
||||
import { readNativeWorkspaceMigration, migratedNativeExecutionCwd } from "../native-runtime/native-workspace-migration.js";
|
||||
import { resolveProviderApiPlacement } from "../native-runtime/provider-api-placement.js";
|
||||
import { readClaimedNativeStopRecovery } from "../native-runtime/native-cancellation-request.js";
|
||||
@@ -309,6 +310,13 @@ export async function selectHeartbeatRuntime(db: Db, input: HeartbeatRuntimeSele
|
||||
if (!issueRef) {
|
||||
throw new Error("native_runtime_ineligible: issue is required");
|
||||
}
|
||||
if (run.runtimeMode !== "native" && agent.adapterType === "paperclip_runner"
|
||||
&& executionTarget?.kind !== "remote"
|
||||
&& nativeRuntimeResolution.profile.backend !== "cursor_cloud_api"
|
||||
&& nativeRuntimeResolution.profile.backend !== "claude_managed_agents_api"
|
||||
&& nativeRuntimeResolution.profile.backend !== "aws_agentcore_harness_api") {
|
||||
assertPaperclipRunnerBinaryConfiguration();
|
||||
}
|
||||
const nativeExecutionWorkspaceId =
|
||||
persistedNativeExecutionInput?.provider.kind === "cursor_cloud"
|
||||
// Older Cursor runs keep their admitted binding identity. Their task
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
buildNativeRunnerArguments,
|
||||
buildNativeRunnerPreparePayload,
|
||||
resolvePaperclipRunnerBinary,
|
||||
assertPaperclipRunnerBinaryConfiguration,
|
||||
resolveRemotePaperclipRunnerBinary,
|
||||
} from "./native-codex-runner.js";
|
||||
|
||||
@@ -225,3 +226,12 @@ describe("buildNativeRunnerPreparePayload", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it("validates only local Runner configuration without requiring or probing an artifact", () => {
|
||||
expect(() => assertPaperclipRunnerBinaryConfiguration("relative-runnerd")).toThrow("PAPERCLIP_RUNNER_BINARY must be an absolute path");
|
||||
vi.stubEnv("PAPERCLIP_RUNNER_BINARY", "relative-controller-runnerd");
|
||||
expect(() => assertPaperclipRunnerBinaryConfiguration()).toThrow("PAPERCLIP_RUNNER_BINARY must be an absolute path");
|
||||
expect(() => resolvePaperclipRunnerBinary()).toThrow("PAPERCLIP_RUNNER_BINARY must be an absolute path");
|
||||
expect(() => assertPaperclipRunnerBinaryConfiguration("")).not.toThrow();
|
||||
expect(() => assertPaperclipRunnerBinaryConfiguration(join(tmpdir(), "not-present-configuration-only-runnerd"))).not.toThrow();
|
||||
});
|
||||
@@ -171,12 +171,22 @@ function executableName(): string {
|
||||
return process.platform === "win32" ? "paperclip-runnerd.exe" : "paperclip-runnerd";
|
||||
}
|
||||
|
||||
export function resolvePaperclipRunnerBinary(
|
||||
configuredPath = process.env.PAPERCLIP_RUNNER_BINARY,
|
||||
): string {
|
||||
function controllerPaperclipRunnerBinaryConfiguration(): string | undefined {
|
||||
return process.env.PAPERCLIP_RUNNER_BINARY;
|
||||
}
|
||||
|
||||
export function assertPaperclipRunnerBinaryConfiguration(
|
||||
configuredPath = controllerPaperclipRunnerBinaryConfiguration(),
|
||||
): void {
|
||||
if (configuredPath && !isAbsolute(configuredPath)) {
|
||||
throw new Error("PAPERCLIP_RUNNER_BINARY must be an absolute path");
|
||||
}
|
||||
}
|
||||
|
||||
export function resolvePaperclipRunnerBinary(
|
||||
configuredPath = controllerPaperclipRunnerBinaryConfiguration(),
|
||||
): string {
|
||||
assertPaperclipRunnerBinaryConfiguration(configuredPath);
|
||||
if (configuredPath) {
|
||||
try {
|
||||
accessSync(configuredPath, constants.R_OK | (process.platform === "win32" ? 0 : constants.X_OK));
|
||||
|
||||
Reference in new issue
Block a user