From 3898f6a45f9b6c22971f9b928c41306baa2810d0 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:15:55 -0500 Subject: [PATCH] fix(native): honor duplex transport for GitHub callback bridges (#16455) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native remote runners use a session-owned broker for managed GitHub operations. > - That broker uses the existing authenticated sandbox callback bridge. > - The native caller omitted the server-selected duplex flag. An enabled and capable target still used file polling. > - Warm sessions must observe a changed flag and channel loss before the next admission. > - This pull request forwards the existing flag and retires unavailable or changed bridges before retained instruction preparation. > - The benefit is less idle remote traffic without changing credential authority or the default transport. ## Linked Issues or Issue Description **What happened?** A native GitHub callback bridge selected `queue_v1` even when its sandbox target had duplex enabled and supported it. The file bridge continues remote queue checks while the warm session remains available. **Expected behavior** An enabled, capable target should use the existing HTTP/2 bridge. Disabled or unsupported targets and failed channel startup must retain the authenticated file fallback. A changed server flag must take effect at the next normal warm-session boundary. **Steps to reproduce** 1. Create native GitHub access on a sandbox target with the server-selected duplex flag enabled, verified duplex capability and a working channel. 2. Observe the selected callback transport. 3. Before this fix, it is `queue_v1`. The added real-wire regression expects `http2_v1` and fails on the old source. ## What Changed - Forward the existing server-selected duplex flag through the native GitHub broker. - Record that transport selection on the broker. Retire the warm provider and broker through the existing cleanup path when the flag changes. - Read live channel failure when checking broker readiness. Mark normal session cleanup as an orderly completion. - Include the selected flag in instruction preparation identity. Retire a failed broker before adopting retained instructions, while preserving existing ownership and cleanup fences. - Add real local HTTP/2 credential-rotation and file-fallback controls. Add native warm-session flag-change controls. - Document selection, fallback and warm-session behavior. - Scan CI secret expressions in one forward pass, including quoted braces and escaped apostrophes. Preserve direct, bracket, multiline and whole-context references while rejecting filename and literal-string false positives. ## Verification - Current head `41440b07ea45e9e882e36ed495a66e501c8dd4df` resolves the actual CI-policy conflict with master `307eb16c0c59abd9e0626fde9201939b262e7f5f`. It retains upstream's stricter whole-expression credential guard, including quoted secret text, and uses the forward scanner instead of a backtracking regex. All **16 CI policy tests** pass, including the bounded 28- and 10,000-apostrophe controls in both policy modes. The reported old regex still has its recorded two-second timeout (14 passed, one failed). On this final merged source, **809 affected native cases** pass with zero selected skips (65.20s). Full repository typecheck (31.44s) and build (22.13s) pass. Frozen source hashes stayed unchanged. Fresh exact-head full CI, configured security and completed Apex remain pending. - Before master integration, the frozen native repair passed repository `pnpm -r typecheck` passed (33.0s) and `pnpm build` passed (32.1s). - Before the latest master integration, all **800 affected cases passed with zero selected skips**: complete native executor (725), resume (50), GitHub broker (16), environment duplex-gate (4), and heartbeat instruction-copy (5) suites. The final run took 95.3s. Source hashes stayed unchanged through these checks. - The initial head passed 788 affected cases and full CI, but Apex scored it **3/5**. It found a dead HTTP/2 broker could remain reusable and a flag change could fail after retained instruction preparation. Both findings required the new repair. Three added before-fix regressions failed on channel availability, orderly close, and preparation identity. - The first repair run failed three new warm-instruction fixtures because fake provider close left its recorded state ready. The corrected fixture writes a suspended checkpoint without weakening the production ownership guard. A separate real-wire run then failed two availability assertions because its helper spread had frozen the readiness getter. The helper now preserves the getter. All failed attempts remain in the local evidence. - One disposable real Daytona sandbox exercised both transports on the initial transport patch through the production native broker and callback bridge. The provider shell wrapper used the then-current production declarations. During equal two-second idle windows, file mode issued **9 remote commands** and HTTP/2 issued **0**. Both variants rotated synthetic credentials between two run bindings and denied idle requests. Cleanup deleted the sandbox. - Daytona component setup took 3.64s for file mode and 4.74s for HTTP/2. This is an idle-traffic reduction, not a startup-speed claim. That component run predates the availability and preparation repair; it does not qualify the repaired paths on Daytona. The new failure controls use a real local HTTP/2 channel. The credential resolver was synthetic and no model was called. Real GitHub-account grants, staging replies, and the end-to-end latency target remain unverified by this component test. - Repaired-head CI found a policy-test false positive: a broad `secrets.` regex matched an upstream filename. The unchanged test reproduced the failure, and the corrected version passed all 15 controls. Master later supplied a stronger direct/bracket check, which we retained when resolving the merge conflict. Review then found that formatted expressions could stop that check at a quoted brace. The new reported-case baseline failed (one failed, 14 passed). The final check treats quoted strings as complete tokens, respects expression boundaries, and passes all 15 cases. The later master conflict also added `toJSON(secrets)` controls. The old branch pattern failed that whole-context case. Review subsequently found exponential backtracking with escaped apostrophes; the final forward scanner retains the quoted-boundary and whole-context controls and passes all 15 tests, including the bounded long-expression regression. No workflow or credential handling changed. All failed attempts remain recorded. - The full local repository test suite was not rerun. [Prior-head full CI](https://github.com/paperclipai/paperclip/actions/runs/38775801821), including `ci / verify` and `ci / e2e`, passed on previous head `e5f8bdb425081cf08110c0f7423f96609e405920`. All 52 check runs succeeded, two optional Storybook jobs skipped, and legacy Snyk passed. Completed Apex review is **5/5** on that exact head, with no unresolved review threads. ## Risks - Only an explicit server-selected opt-in can use HTTP/2. The existing provider-capability, channel-startup and authenticated file-fallback checks remain in the shared bridge. - Toggling the flag or losing the HTTP/2 channel replaces an idle retained provider at the next run boundary. Instruction preparation must make the same retirement decision. Normal ownership, settled history and cleanup checks remain in effect. - Lower idle traffic may reduce provider/controller pressure, but no staging latency improvement is claimed. This feature remains inactive when the existing flag is off. Other preparation and finalization costs remain. - No schema, API, UI, credential-selection, tracing endpoint or default-setting changes. ## Model Used - OpenAI Codex, based on GPT-6, with tool-assisted reasoning and code execution. The exact hosted snapshot ID and context-window size are not exposed in this session. ## 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 --------- Co-authored-by: Paperclip --- .github/scripts/tests/ci-profile.test.mjs | 76 ++++++++++++- doc/execution-github-identity.md | 11 ++ .../instruction-working-copy.test.ts | 17 ++- .../heartbeat/instruction-working-copy.ts | 2 + .../native-github-access.test.ts | 105 ++++++++++++++++-- .../native-runtime/native-github-access.ts | 7 +- .../native-session-executor.test.ts | 82 +++++++++++--- .../native-runtime/native-session-executor.ts | 5 +- 8 files changed, 273 insertions(+), 32 deletions(-) diff --git a/.github/scripts/tests/ci-profile.test.mjs b/.github/scripts/tests/ci-profile.test.mjs index 1401fd62899..67852ab023d 100644 --- a/.github/scripts/tests/ci-profile.test.mjs +++ b/.github/scripts/tests/ci-profile.test.mjs @@ -232,19 +232,56 @@ test("export paces batches and retries only rejected transient events", async () await assert.rejects(sendEvents([events[0]], "test-only", "paperclip-ci", async () => ({ ok: true, json: async () => [{ status: 400 }] })), /status 400/); }); -function assertNoPrWorkflowCredentials(source) { - assert.doesNotMatch(source, /HONEYCOMB.*KEY/); - for (const [expression] of source.matchAll(/\$\{\{(?:'(?:[^']|'')*'|(?!\}\})[\s\S])*\}\}/g)) { - assert.doesNotMatch(expression, /\bsecrets\b/); +function secretReference(text, includeQuoted = false) { + for (const line of text.split(/[\r\n\u2028\u2029]/)) { + const start = line.indexOf("HONEYCOMB"); + if (start >= 0 && line.indexOf("KEY", start + "HONEYCOMB".length) >= 0) return true; } + let offset = 0; + while (offset < text.length) { + const start = text.indexOf("${{", offset); + if (start < 0) return false; + offset = start + 3; + let quoted = false; + let expression = ""; + while (offset < text.length) { + const char = text[offset]; + if (char === "'") { + if (quoted && text[offset + 1] === "'") { + offset += 2; + continue; + } + quoted = !quoted; + expression += includeQuoted ? char : " "; + } else if (!quoted) { + if (char === "}" && text[offset + 1] === "}") { + offset += 2; + break; + } + expression += char; + } else if (includeQuoted) { + expression += char; + } + offset += 1; + } + if (/\bsecrets\b/.test(expression)) return true; + } + return false; +} + +function assertNoPrWorkflowCredentials(source) { + assert.equal(secretReference(source, true), false); } test("PR credential guard permits filenames but rejects secret expressions", () => { assertNoPrWorkflowCredentials("run: node --test scripts/docker-compose-secrets.test.mjs"); - for (const source of ["env: ${{ secrets.EXAMPLE }}", "env: ${{ secrets['EXAMPLE'] }}", "env: ${{ toJSON(secrets) }}", "env: ${{ format('}}', secrets.EXAMPLE) }}"]) { + for (const source of ["env: ${{ secrets.EXAMPLE }}", "env: ${{ secrets['EXAMPLE'] }}", "env: ${{ toJSON(secrets) }}", "env: ${{ format('}}', secrets.EXAMPLE) }}", + "# Expressions start with '${{'.\nTOKEN: ${{ secrets.PR_TOKEN }}", + "name: Example # Expressions start with '${{'.\nTOKEN: ${{ secrets.PR_TOKEN }}"]) { assert.throws(() => assertNoPrWorkflowCredentials(source)); } assert.throws(() => assertNoPrWorkflowCredentials("HONEYCOMB_API_KEY: example")); + assert.throws(() => assertNoPrWorkflowCredentials("${{ format('secrets.EXAMPLE', 'literal') }}")); }); test("workflow exporter runs trusted code and keeps keys out of the PR jobs", () => { @@ -254,6 +291,35 @@ test("workflow exporter runs trusted code and keeps keys out of the PR jobs", () assert.match(workflow, /environment: ci-profiling/); assert.doesNotMatch(workflow, /head_sha|secrets: inherit|contents: write|pull-requests: write|pnpm install/); const pr = readFileSync(new URL("../../workflows/pr-trusted.yml", import.meta.url), "utf8"); + assert.equal(secretReference("run: node --test scripts/docker-compose-secrets.test.mjs"), false); + for (const lookup of ["${{ secrets.PR_TOKEN }}", "${{ secrets['PR_TOKEN'] }}", "${{\n secrets.PR_TOKEN\n }}", "${{ toJSON(secrets) }}", "${{ secrets.HONEYCOMB_API_KEY }}"]) { + assert.equal(secretReference(lookup), true); + } + for (const lookup of [ + "${{ format('Bearer {0}', secrets.PR_TOKEN) }}", + "${{ format('{0}', toJSON(secrets)) }}", + "${{ format('{{Bearer}} {0}', secrets['PR_TOKEN']) }}", + "${{ format('it''s {0}',\n secrets.PR_TOKEN) }}", + ]) { + assert.equal(secretReference(lookup), true); + } + for (const nonSecret of [ + "${{ format('secrets.PR_TOKEN', 'literal') }}", + "${{ github.sha }}\nrun: node --test scripts/docker-compose-secrets.test.mjs", + ]) { + assert.equal(secretReference(nonSecret), false); + } + const escapedExpressions = [28, 10_000].flatMap(count => { + const expression = "${{ format('" + "it''s ".repeat(count) + "{0}', github.sha) }}"; + return [expression, expression.replace("github.sha", "secrets.PR_TOKEN"), expression.replace("github.sha", "toJSON(secrets)")]; + }); + const boundedScan = spawnSync(process.execPath, ["-e", `const fs=require('node:fs');const scan=${secretReference.toString()};process.stdout.write(JSON.stringify(JSON.parse(fs.readFileSync(0,'utf8')).map(value=>[scan(value),scan(value,true)])));`], { + input: JSON.stringify(escapedExpressions), + encoding: "utf8", timeout: 2_000, + }); + assert.equal(boundedScan.status, 0, boundedScan.error?.message); + assert.deepEqual(JSON.parse(boundedScan.stdout), [[false, false], [true, true], [true, true], [false, false], [true, true], [true, true]]); + assert.equal(secretReference(pr), false); assertNoPrWorkflowCredentials(pr); assert.match(pr, /if: always\(\) && env.PAPERCLIP_CI_PROFILE == 'true'/); assert.doesNotMatch(pr, /PAPERCLIP_CI_PROFILE:.*labels/); diff --git a/doc/execution-github-identity.md b/doc/execution-github-identity.md index 8c6ef1e2a05..c98452969ca 100644 --- a/doc/execution-github-identity.md +++ b/doc/execution-github-identity.md @@ -18,6 +18,17 @@ Native runners retain a session-owned broker and launcher path across warm turns The session broker listens only on controller loopback and accepts only its authenticated GitHub credential operation. Remote executions reach it through the existing authenticated callback bridge. Its transport and launchers are retired with the provider session. If remote bridge startup fails, anonymous launchers keep ordinary work available; the next run retries setup with a fresh session. Controller restart/cold recovery still uses the existing checkpoint and process-recovery rules; the broker's in-memory authority is not persisted for adoption. +The native GitHub callback bridge uses HTTP/2 only when the server-selected +`enableSandboxDuplexBridge` flag and the sandbox's verified duplex capability +permit it. Missing settings, unsupported providers and failed channel startup +keep the authenticated file bridge. Changing the server flag retires a retained +warm bridge at the next normal run boundary, before a retained instruction copy +is claimed. A lost HTTP/2 channel also makes the broker unavailable and retires +that warm session at its next admission. An unchanged flag and a healthy broker +permit reuse. Normal session cleanup closes the channel as an orderly completion. +This selection changes transport only; operation-time credential checks remain +the same. Local and SSH execution retain their existing transport. + Other adapters continue using the run-scoped signed capability and public broker endpoint. That endpoint rejects browser origins and session cookies, validates a distinct signed runtime scope, and rechecks the company, agent, and live run. GitHub credentials are returned only to the managed command process, never persisted in identity history or injected into the long-lived provider process. Low-trust executions cannot receive raw GitHub credentials, including dedicated diff --git a/server/src/services/heartbeat/instruction-working-copy.test.ts b/server/src/services/heartbeat/instruction-working-copy.test.ts index 3ed57790ea7..d3f25c28409 100644 --- a/server/src/services/heartbeat/instruction-working-copy.test.ts +++ b/server/src/services/heartbeat/instruction-working-copy.test.ts @@ -14,7 +14,7 @@ type Input = Parameters[0]; type Prepare = Parameters["prepare"]>[0]; type Copy = NonNullable>>; -function fixture() { +function fixture(executionTarget: Input["workspace"]["executionTarget"] = null) { let run = { id: "current-run", companyId: "company", agentId: "agent", runnerProfileJson: null, status: "queued" } as ReturnType; const copy = { runId: run.id, companyId: run.companyId, agentId: run.agentId, localRoot: "/workspace/copy", executionRoot: "/workspace/copy", entryFile: "AGENTS.md", @@ -35,7 +35,7 @@ function fixture() { agent: { id: "agent", companyId: "company", adapterType: "codex_local", adapterConfig: { lifecycleMode: "warm" }, runtimeConfig: {} } as unknown as Input["agent"], context, session: { taskSession: null, taskSessionForRun: { lastRunId: "previous-run" }, sessionConfigMetadata: {} as Input["session"]["sessionConfigMetadata"] }, - workspace: { executionTarget: null, executionWorkspace: { cwd: "/workspace" } as Input["workspace"]["executionWorkspace"], persistedExecutionWorkspace: null }, + workspace: { executionTarget, executionWorkspace: { cwd: "/workspace" } as Input["workspace"]["executionWorkspace"], persistedExecutionWorkspace: null }, appendRunEvent, }); const prepare: Prepare = { runtimeConfig: { retained: true }, hasInstructionFilesystem: true, @@ -49,6 +49,19 @@ function fixture() { beforeEach(() => { vi.clearAllMocks(); native.claim.mockResolvedValue(null); }); describe("instruction working-copy extraction boundary", () => { + it("changes instruction preparation identity before claiming a copy when duplex changes", async () => { + const keys: (string | undefined)[] = []; + for (const enabled of [false, false, true]) { + const f = fixture({ kind: "remote", transport: "sandbox", remoteCwd: "/workspace", enableSandboxDuplexBridge: enabled }); + await f.controller.prepare({ ...f.prepare, nativeRuntimeResolution: { + kind: "native", profile: { backend: "opencode_server" }, + } as Prepare["nativeRuntimeResolution"] }); + keys.push(f.controller.nativeInstructionWorkingCopy()?.preparationKey); + } + expect(keys[0]).toBeTypeOf("string"); + expect(keys[1]).toBe(keys[0]); + expect(keys[2]).not.toBe(keys[0]); + }); it("retains a warm reservation for outer cleanup when subsequent preparation throws", async () => { const f = fixture(); f.copies.prepare.mockRejectedValue(new Error("preparation interrupted")); diff --git a/server/src/services/heartbeat/instruction-working-copy.ts b/server/src/services/heartbeat/instruction-working-copy.ts index 87b443b92a7..1d96c850481 100644 --- a/server/src/services/heartbeat/instruction-working-copy.ts +++ b/server/src/services/heartbeat/instruction-working-copy.ts @@ -2,6 +2,7 @@ import { createHash } from "node:crypto"; import path from "node:path"; import { and, eq, sql } from "drizzle-orm"; import { heartbeatRuns, type Db } from "@paperclipai/db"; +import { adapterExecutionTargetEnablesSandboxDuplexBridge } from "@paperclipai/adapter-utils/execution-target"; import { parseObject } from "../../adapters/utils.js"; import { logger } from "../../middleware/logger.js"; import { AgentDirectoryReuseInvalidatedError, isAgentDirectoryCopy } from "../agent-directory-working-copies.js"; @@ -78,6 +79,7 @@ export function createHeartbeatInstructionWorkingCopy(input: InstructionWorkingC let instructionSave: Record | null = null; const instructionPreparationKey = createHash("sha256").update(JSON.stringify({ adapterType: agent.adapterType, adapterConfig: agent.adapterConfig, runtimeConfig: agent.runtimeConfig, sessionConfigMetadata, + duplexBridge: adapterExecutionTargetEnablesSandboxDuplexBridge(executionTarget), workspace: nativeSessionWorkspaceScope({ binding: { runId: readRun().id, executionWorkspaceId: persistedExecutionWorkspace?.id ?? readRun().id }, workspace: executionWorkspace, diff --git a/server/src/services/native-runtime/native-github-access.test.ts b/server/src/services/native-runtime/native-github-access.test.ts index e6c1aaea539..b472beaf34e 100644 --- a/server/src/services/native-runtime/native-github-access.test.ts +++ b/server/src/services/native-runtime/native-github-access.test.ts @@ -2,7 +2,8 @@ import { afterEach, expect, it, vi } from "vitest"; import { spawn, execFile } from "node:child_process"; import { mkdtemp, mkdir, readFile, rm, writeFile, access } from "node:fs/promises"; import { promisify } from "node:util"; -import type { CommandManagedRuntimeRunner } from "@paperclipai/adapter-utils/command-managed-runtime"; +import type { CommandManagedDuplexChannel, CommandManagedRuntimeRunner } from "@paperclipai/adapter-utils/command-managed-runtime"; +import { startAdapterExecutionTargetPaperclipBridge } from "@paperclipai/adapter-utils/execution-target"; import { tmpdir } from "node:os"; import path from "node:path"; import { createInterface } from "node:readline"; @@ -11,15 +12,38 @@ const scope = { companyId: "company-a", agentId: "agent-a", issueId: "task-a" }; const run = (runId: string) => ({ ...scope, runId }); const cleanups: (() => Promise)[] = []; afterEach(async () => { for (const cleanup of cleanups.splice(0).reverse()) await cleanup(); }); +function localDuplexChannel(command: readonly string[], cwd: string): CommandManagedDuplexChannel { + const child = spawn(command[0]!, command.slice(1), { cwd, stdio: ["pipe", "pipe", "pipe"] }); + const buffered: Buffer[] = []; + let dataListener: ((chunk: Uint8Array) => void) | undefined; + let exitListener: ((exit: { exitCode: number | null }) => void) | undefined; + let exit: { exitCode: number | null } | undefined; + child.stdin.on("error", () => undefined); + child.stderr.resume(); + child.stdout.on("data", (chunk: Buffer) => dataListener ? dataListener(chunk) : buffered.push(chunk)); + const closed = new Promise(resolve => child.once("close", code => { + exit = { exitCode: code }; + exitListener?.(exit); + resolve(); + })); + return { + write: data => { child.stdin.write(data); }, + onData: listener => { dataListener = listener; for (const chunk of buffered.splice(0)) listener(chunk); }, + onExit: listener => { exitListener = listener; if (exit) listener(exit); }, + stop: () => { child.kill(); }, + close: async () => { child.kill(); await closed; }, + }; +} async function broker(resolveCredentials = vi.fn(async (binding: ReturnType) => ({ status: "available", env: { GH_TOKEN: `fixture-${binding.runId}` }, -})), remote = false, bridgeUnavailable = false, onLog?: (stream: "stdout" | "stderr", chunk: string) => Promise) { +})), remote = false, bridgeUnavailable = false, onLog?: (stream: "stdout" | "stderr", chunk: string) => Promise, +duplex?: { enabled?: boolean; capability: boolean; unavailable?: boolean }) { const root = await mkdtemp(path.join(tmpdir(), "native-github-reuse-")); cleanups.push(() => rm(root, { recursive: true, force: true })); const bin = path.join(root, "real-bin"); await mkdir(bin); await writeFile(path.join(bin, "gh"), `#!${process.execPath}\nprocess.stdout.write(process.env.GH_TOKEN || 'anonymous');`, { mode: 0o700 }); - const execute: CommandManagedRuntimeRunner["execute"] = async (input) => { + const execute = vi.fn(async (input) => { const startedAt = new Date().toISOString(); const child = promisify(execFile)(input.command, input.args ?? [], { cwd: input.cwd ?? root, env: { ...process.env, ...input.env }, timeout: 15_000, maxBuffer: 4 * 1024 * 1024, @@ -28,20 +52,87 @@ async function broker(resolveCredentials = vi.fn(async (binding: ReturnType>(async input => { + if (duplex?.unavailable) throw new Error("fixture channel unavailable"); + return localDuplexChannel(input.command, root); + }); const target = remote ? { kind: "remote" as const, transport: "sandbox" as const, providerKey: "test", - remoteCwd: root, runner: { execute }, streamRunLogs: false } : null; + remoteCwd: root, runner: { execute, ...(duplex ? { openDuplexChannel } : {}) }, streamRunLogs: false, + ...(duplex ? { enableSandboxDuplexBridge: duplex.enabled, effectiveCapabilities: { + reusableLeases: true, nativeSyncIn: false, nativeSyncOut: false, persistentProcessSessions: false, + independentControlCommands: true, incrementalSessionOutput: false, concurrentSyncOperations: false, + duplexCommandStream: duplex.capability, runnerWebSocketIngress: false, + } } : {}) } : null; + const bridgeModes: (string | undefined)[] = []; const result = await createNativeGitHubAccess({ scope, target, cwd: root, env: { PATH: `${bin}:${path.dirname(process.execPath)}:/usr/bin:/bin`, PAPERCLIP_API_KEY: "old-run-api-key" }, resolveCredentials, onLog }, - bridgeUnavailable ? async () => { throw new Error("fixture bridge unavailable"); } : undefined); + bridgeUnavailable ? async () => { throw new Error("fixture bridge unavailable"); } : async input => { + const bridge = await startAdapterExecutionTargetPaperclipBridge(input); + bridgeModes.push(bridge?.env.PAPERCLIP_API_BRIDGE_MODE); + return bridge; + }); cleanups.push(result.stop); - return { ...result, root, resolveCredentials }; + return Object.assign(result, { root, resolveCredentials, execute, openDuplexChannel, bridgeModes }); } function request(broker: NativeGitHubAccess, extra: RequestInit = {}, endpoint = "/runtime-tools/github/credentials") { return fetch(broker.env.PAPERCLIP_GITHUB_BROKER_URL + endpoint, { method: "POST", body: "{}", headers: { authorization: `Bearer ${broker.env.PAPERCLIP_GITHUB_BRIDGE_TOKEN}`, "content-type": "application/json" }, ...extra, }); } +it("uses the selected duplex transport across credential rotation without file polling", async () => { + const b = await broker(undefined, true, false, undefined, { enabled: true, capability: true }); + expect(b.bridgeModes).toEqual(["http2_v1"]); + expect(b.openDuplexChannel).toHaveBeenCalledOnce(); + const commandsAfterStartup = b.execute.mock.calls.length; + expect((await request(b)).status).toBe(403); + const releaseA = b.activate(run("a")); + expect(await (await request(b)).json()).toMatchObject({ env: { GH_TOKEN: "fixture-a" } }); + releaseA(); + expect((await request(b)).status).toBe(403); + const releaseB = b.activate(run("b")); + releaseA(); + expect(await (await request(b)).json()).toMatchObject({ env: { GH_TOKEN: "fixture-b" } }); + expect(b.resolveCredentials.mock.calls.map(([binding]) => binding.runId)).toEqual(["a", "b"]); + expect(b.execute.mock.calls).toHaveLength(commandsAfterStartup); + releaseB(); + await b.stop(); + expect(() => b.activate(run("c"))).toThrow("closed"); +}); +it("marks a lost HTTP/2 bridge unavailable before the next warm admission", async () => { + const b = await broker(undefined, true, false, undefined, { enabled: true, capability: true }); + const release = b.activate(run("a")); + expect((await request(b)).status).toBe(200); + release(); + expect(b.ready).toBe(true); + const channel = await b.openDuplexChannel.mock.results[0]!.value; + channel.stop(); + await vi.waitFor(() => expect(b.ready).toBe(false), { timeout: 2000 }); +}); +it("closes a healthy HTTP/2 bridge without reporting a lost active run", async () => { + const onLog = vi.fn(async (_stream: "stdout" | "stderr", _chunk: string) => undefined); + const b = await broker(undefined, true, false, onLog, { enabled: true, capability: true }); + const release = b.activate(run("a")); + expect((await request(b)).status).toBe(200); + release(); + await b.stop(); + expect(b.ready).toBe(false); + expect(onLog.mock.calls.filter(([, chunk]) => chunk.includes("channel lost"))).toEqual([]); +}); +it.each([ + { enabled: false, capability: true, unavailable: false, opens: 0 }, + { enabled: undefined, capability: true, unavailable: false, opens: 0 }, + { enabled: true, capability: false, unavailable: false, opens: 0 }, + { enabled: true, capability: true, unavailable: true, opens: 1 }, +])("retains authorized file fallback with duplex settings %j", async options => { + const b = await broker(undefined, true, false, undefined, options); + expect(b.bridgeModes).toEqual(["queue_v1"]); + expect(b.openDuplexChannel).toHaveBeenCalledTimes(options.opens); + const release = b.activate(run("fallback")); + expect(await (await request(b)).json()).toMatchObject({ env: { GH_TOKEN: "fixture-fallback" } }); + release(); + expect((await request(b)).status).toBe(403); +}); it.each([false, true])("keeps one live parent and its original launcher environment across two authorized runs (callback bridge: %s)", async (remote) => { const b = await broker(undefined, remote); const parent = spawn(process.execPath, ["-e", ` diff --git a/server/src/services/native-runtime/native-github-access.ts b/server/src/services/native-runtime/native-github-access.ts index a9711329ff3..87233394538 100644 --- a/server/src/services/native-runtime/native-github-access.ts +++ b/server/src/services/native-runtime/native-github-access.ts @@ -2,6 +2,7 @@ import { randomBytes, randomUUID, timingSafeEqual } from "node:crypto"; import { createServer } from "node:http"; import { cleanupGitHubOperationLaunchers, + adapterExecutionTargetEnablesSandboxDuplexBridge, githubOperationLauncherDirectory, prepareGitHubOperationLaunchers, startAdapterExecutionTargetPaperclipBridge, @@ -24,6 +25,7 @@ export async function createNativeGitHubAccess(input: { resolveCredentials: (binding: Binding) => Promise; onLog?: (stream: "stdout" | "stderr", chunk: string) => Promise; }, startBridge = startAdapterExecutionTargetPaperclipBridge) { + const duplexRequested = adapterExecutionTargetEnablesSandboxDuplexBridge(input.target); const token = randomBytes(32).toString("hex"); const location = { runId: `native-session-${randomUUID()}`, target: input.target }; let active: Binding | null = null; @@ -76,6 +78,7 @@ export async function createNativeGitHubAccess(input: { const stop = () => stopping ??= (async () => { stopped = true; active = null; + bridge?.markOrderlyCompletion?.(); server.closeAllConnections(); const results = await Promise.allSettled([ bridge?.stop(), @@ -103,6 +106,7 @@ export async function createNativeGitHubAccess(input: { adapterKey: "native-github", hostApiToken: token, hostApiUrl: url, + enableSandboxDuplexBridge: duplexRequested, onLog: input.onLog, }); if (input.target?.kind === "remote" && !bridge) { @@ -128,7 +132,8 @@ export async function createNativeGitHubAccess(input: { }); return { env, - ready, + get ready() { return ready && !stopped && bridge?.readRunDisposition?.().failed !== true; }, + duplexRequested, activate(binding: Binding) { if (stopped || active) throw new Error("GitHub session is closed or busy"); if (binding.companyId !== input.scope.companyId || binding.agentId !== input.scope.agentId || diff --git a/server/src/services/native-runtime/native-session-executor.test.ts b/server/src/services/native-runtime/native-session-executor.test.ts index 362aa46db98..16ddc55e4f9 100644 --- a/server/src/services/native-runtime/native-session-executor.test.ts +++ b/server/src/services/native-runtime/native-session-executor.test.ts @@ -8306,6 +8306,19 @@ describe("native warm session supervision", () => { it.each( [ + ...([[false, true], [true, false], [true, true]] as const).map(([firstDuplex, secondDuplex]) => ({ + firstBroker: true, secondBroker: true, projectless: false, local: false, brokerReady: true, + firstMode: "managed", secondMode: "managed", managedGitHub: true, firstDuplex, secondDuplex, + })), + ...([[false, true], [true, false], [true, true]] as const).map(([firstDuplex, secondDuplex]) => ({ + firstBroker: true, secondBroker: true, projectless: false, local: false, brokerReady: true, + firstMode: "managed", secondMode: "managed", managedGitHub: true, firstDuplex, secondDuplex, instructionFiles: true, + })), + ...[false, true].map(instructionFiles => ({ + firstBroker: true, secondBroker: true, projectless: false, local: false, brokerReady: true, + firstMode: "managed", secondMode: "managed", managedGitHub: true, firstDuplex: true, secondDuplex: true, + instructionFiles, loseBridge: true, + })), ...[false, true].flatMap((local) => [true, false].map(brokerReady => ({ firstBroker: true, secondBroker: true, projectless: false, local, brokerReady, firstMode: "managed", secondMode: "managed", managedGitHub: true, @@ -8375,10 +8388,14 @@ describe("native warm session supervision", () => { checkpointContract: "valid", managedGitHub: false, brokerReady: true, + firstDuplex: false, + secondDuplex: false, + instructionFiles: false, + loseBridge: false, ...scenario, })), )( - "verifies a live warm owner before refreshing run authority (broker: $firstBroker -> $secondBroker, projectless: $projectless, local: $local, auth: $firstMode -> $secondMode, network: $firstNetwork -> $secondNetwork, checkpoint: $checkpointContract, managed access: $managedGitHub, broker ready: $brokerReady)", + "verifies a live warm owner before refreshing run authority (broker: $firstBroker -> $secondBroker, projectless: $projectless, local: $local, auth: $firstMode -> $secondMode, network: $firstNetwork -> $secondNetwork, checkpoint: $checkpointContract, managed access: $managedGitHub, broker ready: $brokerReady, duplex: $firstDuplex -> $secondDuplex, files: $instructionFiles, loss: $loseBridge)", async ({ firstBroker, secondBroker, @@ -8391,18 +8408,30 @@ describe("native warm session supervision", () => { checkpointContract, managedGitHub, brokerReady, + firstDuplex, + secondDuplex, + instructionFiles, + loseBridge, }) => { - githubAccess.create.mockReset().mockResolvedValue({ - env: { PAPERCLIP_GITHUB_BROKER_TOKEN: "stable-session-capability" }, - ready: brokerReady, - activate: githubAccess.activate.mockReset().mockImplementation(() => vi.fn()), - stop: githubAccess.stop.mockReset().mockResolvedValue(undefined), + let brokerLost = false; + const activate = githubAccess.activate.mockReset().mockImplementation(() => vi.fn()); + const stop = githubAccess.stop.mockReset().mockResolvedValue(undefined); + githubAccess.create.mockReset().mockImplementation(async () => { + const creation = githubAccess.create.mock.calls.length; + return { + env: { PAPERCLIP_GITHUB_BROKER_TOKEN: "stable-session-capability" }, + get ready() { return brokerReady && (creation !== 1 || !brokerLost); }, + duplexRequested: creation === 1 ? firstDuplex : secondDuplex, + activate, + stop, + }; }); const replacesProvider = !brokerReady || (!managedGitHub && (firstBroker || secondBroker)) || firstMode !== secondMode || - firstNetwork !== secondNetwork; + firstNetwork !== secondNetwork || + firstDuplex !== secondDuplex || loseBridge; const stateBase = await mkdtemp( join(tmpdir(), "paperclip-runnerd-warm-authority-"), ); @@ -8410,7 +8439,13 @@ describe("native warm session supervision", () => { const previousPaperclipHome = process.env.PAPERCLIP_HOME; process.env.PAPERCLIP_RUNNER_STATE_DIR = stateBase; process.env.PAPERCLIP_HOME = stateBase; - const firstClose = vi.fn(async () => undefined); + let checkpointFirstRunner: (() => Promise) | undefined; + const firstClose = vi.fn(async () => { + if (instructionFiles) { + if (!checkpointFirstRunner) throw new Error("fixture runner checkpoint unavailable"); + await checkpointFirstRunner(); + } + }); const firstSession = { close: firstClose }; const first = { ...execution, @@ -8444,7 +8479,7 @@ describe("native warm session supervision", () => { : first.binding.executionWorkspaceId, }, } as NativeExecutionInputV1; - const remoteTarget = ( + const remoteTarget = (duplex: boolean) => ( local ? { kind: "local" as const, @@ -8455,9 +8490,13 @@ describe("native warm session supervision", () => { transport: "sandbox" as const, environmentId: "environment-runnerd-warm-authority", remoteCwd: "/home/daytona/paperclip-workspace", + enableSandboxDuplexBridge: duplex, runner: { execute: vi.fn() }, } ) as never; + const firstCopy = { runId: first.binding.runId, root: first.workspace.cwd!, preparationKey: JSON.stringify({ duplex: firstDuplex }), + hasChanges: vi.fn(async () => false), collectStopped: vi.fn(async () => undefined) }; + const secondCopy = { ...firstCopy, runId: second.binding.runId, preparationKey: JSON.stringify({ duplex: secondDuplex }) }; const result = { result: { summary: "completed" }, terminal: { runTerminalState: "succeeded" }, @@ -8528,7 +8567,8 @@ describe("native warm session supervision", () => { runnerInstanceId: "runner-runnerd-warm", useRunnerd: true, managedGitHub, - runnerExecutionTarget: remoteTarget, + runnerExecutionTarget: remoteTarget(firstDuplex), + instructionWorkingCopy: instructionFiles ? firstCopy : undefined, }); if (projectless && replacesProvider) { // Also prove an upgrade can resume the old per-run workspace digest @@ -8594,6 +8634,8 @@ describe("native warm session supervision", () => { join(durableRoot, "runner", "runner-state.json"), JSON.stringify(durableRunnerState(durableIdentity, "ready")), ); + checkpointFirstRunner = () => writeFile(join(durableRoot, "runner", "runner-state.json"), + JSON.stringify(durableRunnerState(durableIdentity, "suspended"))); const continuationDb = { ...leaseDb(second), select: () => ({ @@ -8610,6 +8652,14 @@ describe("native warm session supervision", () => { }), }), } as unknown as Db; + brokerLost = loseBridge; + const releasePreparation = instructionFiles ? await claimWarmNativeInstructionCopy({ + priorExecution: first, companyId: first.binding.companyId, agentId: first.binding.agentId, + executionWorkspaceId: second.binding.executionWorkspaceId, workspace: second.workspace, + environmentId: "environment-runnerd-warm-authority", runId: second.binding.runId, + preparationKey: secondCopy.preparationKey, adopt: async () => secondCopy, + }) : null; + if (instructionFiles) expect(releasePreparation === null).toBe(replacesProvider); await executePaperclipNativeSession({ db: continuationDb, execution: second, @@ -8623,16 +8673,18 @@ describe("native warm session supervision", () => { runnerInstanceId: "runner-runnerd-warm", useRunnerd: true, managedGitHub, - runnerExecutionTarget: remoteTarget, + runnerExecutionTarget: remoteTarget(secondDuplex), + instructionWorkingCopy: instructionFiles ? secondCopy : undefined, }); + await releasePreparation?.(); if (managedGitHub) { - expect(state.execute.mock.calls[1]?.[0].existingSession).toBe(brokerReady ? firstSession : undefined); - expect(githubAccess.create).toHaveBeenCalledTimes(brokerReady ? 1 : 2); + expect(state.execute.mock.calls[1]?.[0].existingSession).toBe(replacesProvider ? undefined : firstSession); + expect(githubAccess.create).toHaveBeenCalledTimes(replacesProvider ? 2 : 1); expect(githubAccess.activate.mock.calls.map(([binding]) => binding.runId)) .toEqual([first.binding.runId, second.binding.runId]); // Replacement fixture publishes no new provider handle: both the old // owner and the unclaimed replacement broker must be cleaned up. - expect(githubAccess.stop).toHaveBeenCalledTimes(brokerReady ? 0 : 2); + expect(githubAccess.stop).toHaveBeenCalledTimes(replacesProvider ? 2 : 0); for (const activation of githubAccess.activate.mock.results) { expect(activation.value).toHaveBeenCalledOnce(); } @@ -8640,7 +8692,7 @@ describe("native warm session supervision", () => { if (replacesProvider) { expect(firstClose).toHaveBeenCalledOnce(); expect(firstClose).toHaveBeenCalledWith({ - reason: "warm native session configuration changed", + reason: instructionFiles ? "warm instruction materialization changed before preparation" : "warm native session configuration changed", }); } else { expect(firstClose).not.toHaveBeenCalled(); diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index a4431a8d711..d21249e442f 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -107,7 +107,7 @@ import { type RunnerProcessLaunchSpec, type NativeSessionGoalControl, } from "../../vendor/paperclip-runner/index.js"; -import type { AdapterExecutionTarget } from "@paperclipai/adapter-utils/execution-target"; +import { adapterExecutionTargetEnablesSandboxDuplexBridge, type AdapterExecutionTarget } from "@paperclipai/adapter-utils/execution-target"; import { createNativeSshCommandRunner } from "./native-ssh-command-runner.js"; import { discoverRemoteExecutable, parseRemoteExecutableCandidate } from "./codex-model-fallback.js"; export { parseRemoteExecutableCandidate } from "./codex-model-fallback.js"; @@ -520,7 +520,7 @@ export async function claimWarmNativeInstructionCopy(input: { } finally { entry.busy = false; } }; try { - const preparationChanged = input.forceRetirement === true || entry.instructionWorkingCopy.preparationKey !== input.preparationKey; + const preparationChanged = input.forceRetirement === true || entry.githubAccess?.ready === false || entry.instructionWorkingCopy.preparationKey !== input.preparationKey; // The prior capability may close over an expired remote lease. Rebind // collection authority first; adoption itself grants no reuse permission. const adopted = await input.adopt(entry.instructionWorkingCopy.runId); @@ -8515,6 +8515,7 @@ async function executePaperclipNativeSessionWithinScope( credentialRunChanged || Boolean(entry.githubAccess) !== Boolean(input.managedGitHub) || entry.githubAccess?.ready === false || + (entry.githubAccess !== undefined && entry.githubAccess.duplexRequested !== adapterExecutionTargetEnablesSandboxDuplexBridge(input.runnerExecutionTarget)) || entry.githubAuthenticationMode !== input.runnerEnvironment?.PAPERCLIP_GITHUB_AUTH_MODE || entry.networkAccess !==