Skip to content

fix: preserve callback bridge runtime PATH#9764

Open
jodylarsen wants to merge 1 commit into
paperclipai:masterfrom
jodylarsen:fix/callback-bridge-runtime-path
Open

fix: preserve callback bridge runtime PATH#9764
jodylarsen wants to merge 1 commit into
paperclipai:masterfrom
jodylarsen:fix/callback-bridge-runtime-path

Conversation

@jodylarsen

Copy link
Copy Markdown

Thinking Path

  • Paperclip manages AI agents across local and remote execution environments.
  • Remote adapters use a sandbox callback bridge so agents can reach the Paperclip control plane.
  • Claude's sandbox preparation can install Node in a version-managed directory and expose it through the adapter runtime PATH.
  • Bridge startup discarded that prepared path and invoked nohup node against the sandbox's ambient environment.
  • This caused otherwise prepared Claude heartbeats to fail before the bridge became ready.
  • This pull request carries the prepared runtime PATH through bridge startup and verifies that the versioned Node shim is selected.
  • The benefit is deterministic callback-bridge startup without changing persisted data or non-Claude adapter behavior.

Linked Issues or Issue Description

  • Bug: a remote claude_local sandbox can successfully install and resolve a versioned Node runtime for the adapter process, while callback-bridge startup still fails with nohup: failed to run command node because bridge startup does not receive the adapter runtime PATH.
  • Expected behavior: the callback bridge starts with the same explicit runtime path prepared for the Claude invocation.
  • Actual behavior: bridge startup falls back to the sandbox runner's ambient path.

What Changed

  • Added optional runtime-path plumbing to callback-bridge startup.
  • Passed Claude's prepared PATH into the remote callback bridge.
  • Added a regression proving a versioned Node shim is selected from the explicit path.

Verification

  • pnpm exec vitest run packages/adapter-utils/src/sandbox-callback-bridge.test.ts packages/adapter-utils/src/execution-target-sandbox.test.ts — 40/40 passed.
  • pnpm --filter @paperclipai/adapter-utils typecheck — passed.
  • pnpm --filter @paperclipai/adapter-claude-local typecheck — passed.

Risks

  • Low risk: the new field is optional and only changes bridge process startup when Claude supplies an explicit path.
  • A malformed configured PATH could still prevent Node resolution; rollback is reverting commit 38874536 and redeploying the previous artifact.
  • No schema or persisted-data changes.

For core feature work, check ROADMAP.md first and discuss it in #dev before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See CONTRIBUTING.md.

Model Used

  • OpenAI GPT-5.6 Sol (gpt-5.6-sol), reasoning and tool-use mode with repository editing, shell execution, and test verification.

Checklist

  • I have included a thinking path that traces from project context to this change
  • I have specified the model used (with version and capability details)
  • I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
  • I have searched GitHub for duplicate or related PRs and linked them above
  • I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
  • I have not referenced internal/instance-local Paperclip issues or links (only public GitHub #NNN / github.com/paperclipai/paperclip URLs)
  • My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details
  • I have run tests locally and they pass
  • I have added or updated tests where applicable
  • I have updated relevant documentation to reflect my changes (no operator-facing documentation change required)
  • 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
  • I will address all Greptile and reviewer comments before requesting merge

Made with Cursor

Forward the prepared Claude runtime PATH when starting the sandbox callback bridge so version-managed Node installations remain resolvable.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
Co-authored-by: Cursor <cursoragent@cursor.com>
@commitperclip

commitperclip Bot commented Jul 17, 2026

Copy link
Copy Markdown

Hey @jodylarsen! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

@greptile-apps

greptile-apps Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes callback-bridge startup failures in remote claude_local sandboxes where a versioned Node runtime is installed but not visible to the nohup node invocation because bridge startup previously discarded the adapter's prepared PATH. The fix threads an optional runtimePath field through startAdapterExecutionTargetPaperclipBridgestartSandboxCallbackBridgeServer and sets PATH in the shell environment before the nohup command runs.

  • Adds runtimePath?: string | null to both startAdapterExecutionTargetPaperclipBridge and startSandboxCallbackBridgeServer; the field is optional so all other adapters are unaffected.
  • Passes env.PATH (the Claude invocation's prepared runtime path) at the call site in execute.ts, read before bridge env is merged, which is the correct ordering.
  • Adds an integration test that creates a versioned node shim, starts the bridge with an explicit runtimePath, and asserts the shim was invoked before the real node process took over.

Confidence Score: 4/5

Safe to merge; the change is additive and isolated to the Claude remote bridge startup path, with a solid regression test and no schema changes.

The change is additive and isolated to the Claude remote bridge startup path, with a solid regression test and no schema changes. One minor spread-ordering concern in sandbox-callback-bridge.ts where the custom PATH is placed before ...env rather than after — harmless today because the bridge-env builder never emits a PATH key, but fragile if that changes.

The env spread order in packages/adapter-utils/src/sandbox-callback-bridge.ts around line 931 is worth a second look.

Important Files Changed

Filename Overview
packages/adapter-utils/src/sandbox-callback-bridge.ts Adds optional runtimePath parameter; spread ordering puts the custom PATH before ...env, which works today but would be silently overridden if the bridge-env builder ever sets PATH.
packages/adapter-utils/src/execution-target.ts Threads the new optional runtimePath field through startAdapterExecutionTargetPaperclipBridge into startSandboxCallbackBridgeServer; straightforward plumbing with no logic changes.
packages/adapters/claude-local/src/server/execute.ts Passes env.PATH (the prepared runtime PATH) as runtimePath when starting the Paperclip bridge, read before bridge env is merged in — correct call-site ordering.
packages/adapter-utils/src/sandbox-callback-bridge.test.ts Adds a regression test that installs a versioned node shim and verifies it is selected when runtimePath is supplied; test structure correctly races on the bridge-ready signal rather than an arbitrary sleep.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
packages/adapter-utils/src/sandbox-callback-bridge.ts:931-935
The explicit `runtimePath` is spread before `...env`, so if `buildSandboxCallbackBridgeEnv` ever gains a `PATH` key the bridge env would silently shadow the versioned-node path — defeating the entire fix. Moving the `PATH` spread after `...env` ensures the caller-supplied path always wins, regardless of what the bridge-env builder returns.

```suggestion
    env: {
      [SANDBOX_EXEC_CHANNEL_ENV]: SANDBOX_EXEC_CHANNEL_BRIDGE,
      ...env,
      ...(input.runtimePath?.trim() ? { PATH: input.runtimePath } : {}),
    },
```

Reviews (1): Last reviewed commit: "fix: preserve callback bridge runtime PA..." | Re-trigger Greptile

Comment on lines 931 to 935
env: {
[SANDBOX_EXEC_CHANNEL_ENV]: SANDBOX_EXEC_CHANNEL_BRIDGE,
...(input.runtimePath?.trim() ? { PATH: input.runtimePath } : {}),
...env,
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The explicit runtimePath is spread before ...env, so if buildSandboxCallbackBridgeEnv ever gains a PATH key the bridge env would silently shadow the versioned-node path — defeating the entire fix. Moving the PATH spread after ...env ensures the caller-supplied path always wins, regardless of what the bridge-env builder returns.

Suggested change
env: {
[SANDBOX_EXEC_CHANNEL_ENV]: SANDBOX_EXEC_CHANNEL_BRIDGE,
...(input.runtimePath?.trim() ? { PATH: input.runtimePath } : {}),
...env,
},
env: {
[SANDBOX_EXEC_CHANNEL_ENV]: SANDBOX_EXEC_CHANNEL_BRIDGE,
...env,
...(input.runtimePath?.trim() ? { PATH: input.runtimePath } : {}),
},
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/adapter-utils/src/sandbox-callback-bridge.ts
Line: 931-935

Comment:
The explicit `runtimePath` is spread before `...env`, so if `buildSandboxCallbackBridgeEnv` ever gains a `PATH` key the bridge env would silently shadow the versioned-node path — defeating the entire fix. Moving the `PATH` spread after `...env` ensures the caller-supplied path always wins, regardless of what the bridge-env builder returns.

```suggestion
    env: {
      [SANDBOX_EXEC_CHANNEL_ENV]: SANDBOX_EXEC_CHANNEL_BRIDGE,
      ...env,
      ...(input.runtimePath?.trim() ? { PATH: input.runtimePath } : {}),
    },
```

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant