fix(server): honor explicit plugin RPC timeouts (#10460)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Plugin workers connect Paperclip to external runtimes and sandbox
providers.
> - Some adapter heartbeats run a full sandbox session inside one
`environmentExecute` RPC.
> - The worker manager limited every RPC timeout to 15 minutes, even
when the caller gave a longer timeout.
> - This pull request keeps the normal default timeout behavior but
honors explicit caller timeouts.
> - The benefit is that long sandboxed agent sessions can continue past
15 minutes while other safety guards still bound hung work.

## Linked Issues or Issue Description

No public GitHub issue exists for this bug.

### Bug Report

Pre-submission checklist:
- Searched existing open and closed issues and did not find a duplicate.
- Confirmed the bug is reproducible on `master` from the current source
tree.
- Confirmed the error starts in Paperclip timeout handling, not in an
adapter provider or local configuration.

What happened?
- A sandbox-backed adapter heartbeat can run a full agent session inside
one `environmentExecute` plugin RPC.
- The plugin worker manager capped every RPC timeout at 15 minutes.
- The cap also applied when the caller passed a longer explicit timeout
for an execute-style call.
- A long sandbox command could fail before the adapter budget expired.

Expected behavior:
- Ordinary plugin RPC calls should keep the normal 30-second default
timeout.
- The default timeout path should still have a 15-minute maximum.
- A caller-supplied positive finite timeout should be honored, including
values above 15 minutes.

Steps to reproduce:
1. Use a plugin environment driver that calls `environmentExecute` with
an explicit timeout above 15 minutes.
2. Run a command that stays active longer than 15 minutes and remains
inside the adapter budget.
3. Observe that the worker manager times out the RPC at 15 minutes
before this fix.
4. Run the same path after this fix and observe that the explicit
timeout is used.

Paperclip version or commit:
- Reproduced from the current `master` line before this change.

Deployment mode:
- Local dev or self-hosted server with sandbox-backed execution.

Installation method:
- Built from source.

Agent adapter(s) involved:
- Codex.
- Custom or external plugin adapter.
- Core plugin worker timeout handling.

Database mode:
- Not database-related.

Access context:
- Agent execution context.

Node.js version:
- Not version-specific.

Operating system:
- Not OS-specific.

Relevant logs or output:
```shell
RPC call "environmentExecute" timed out after 900000ms
```

Relevant config:
- Not config-related.

Additional context:
- Execute-style sandbox calls already have adapter inactivity monitors,
platform silent-run checks, and provider command timeouts. This PR
removes the unintended worker-manager clamp only for explicit positive
finite caller timeouts.

Privacy checklist:
- Reviewed all pasted output for PII, user paths, API keys, tokens,
company names, and internal instance links.

Duplicate search:
- Searched open PRs and open issues in `paperclipai/paperclip` for
`environmentExecute timeout`, `MAX_RPC_TIMEOUT_MS`, and
`plugin-worker-manager timeout`.
- Searched the same terms in `HenkDz/paperclip`.
- Found no matching open PRs or issues.
- Compared this patch-id against my open PRs in `paperclipai/paperclip`;
no match was found.

## What Changed

- Added `resolveRpcCallTimeoutMs()` to keep explicit positive finite
timeouts intact.
- Kept the 15-minute maximum only for the default timeout path.
- Updated `callInternal()` to use the new resolver.
- Added unit tests for explicit long timeouts, default timeout clamping,
fractional values, and invalid explicit values.
- Clarified why notification invocation scopes still use the 15-minute
TTL.

## Verification

- `corepack pnpm install --frozen-lockfile`
- `corepack pnpm --filter @paperclipai/plugin-sdk ensure-build-deps`
- `corepack pnpm --filter @paperclipai/server exec vitest run
src/__tests__/plugin-worker-manager.test.ts`
- `corepack pnpm --filter @paperclipai/server exec tsc --noEmit`
- `git diff --check 9574cad3e8 HEAD`
- GitHub PR checks on `bf7bfd0d`: all passing; Storybook visual
regression skipped by workflow.
- Greptile on `bf7bfd0d`: 5/5, no blocking failure remains; prior P2
thread resolved.

## Risks

Low risk. The change affects RPC timeout resolution in the plugin worker
manager. Ordinary plugin calls still use the 30-second default and the
default path is still capped at 15 minutes. Callers that pass explicit
long timeouts now own that budget. Adapter inactivity monitors and
platform silent-run safety checks still bound hung runs.

## Model Used

OpenAI Codex, GPT-5-based coding agent, with shell and GitHub CLI tool
use.

## 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 <noreply@paperclip.ing>
This commit is contained in:
Devin Foley 2026-07-29 14:27:23 -07:00 committed by GitHub
parent 15ce70dc18
commit 5c5366d0c1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 94 additions and 2 deletions

View File

@ -13,6 +13,7 @@ import {
appendStderrExcerpt,
createPluginWorkerHandle,
formatWorkerFailureMessage,
resolveRpcCallTimeoutMs,
} from "../services/plugin-worker-manager.js";
const FIXTURES_DIR = path.join(path.dirname(fileURLToPath(import.meta.url)), "fixtures");
@ -35,6 +36,56 @@ const TEST_MANIFEST: PaperclipPluginManifestV1 = {
entrypoints: { worker: "dist/worker.js" },
};
describe("resolveRpcCallTimeoutMs", () => {
const MAX_RPC_TIMEOUT_MS = 15 * 60 * 1_000;
const MAX_NODE_TIMER_TIMEOUT_MS = 2_147_483_647;
const DEFAULT_RPC_TIMEOUT_MS = 30_000;
it("honors an explicit timeout above the 15-minute default ceiling", () => {
// The sandbox environment driver requests ~4h + 30s buffer for
// environmentExecute; this must not be clamped to 15 minutes.
const fourHoursPlusBuffer = 4 * 60 * 60 * 1_000 + 30_000;
expect(resolveRpcCallTimeoutMs(fourHoursPlusBuffer, DEFAULT_RPC_TIMEOUT_MS)).toBe(
fourHoursPlusBuffer,
);
});
it("honors an explicit timeout below the ceiling", () => {
expect(resolveRpcCallTimeoutMs(100, DEFAULT_RPC_TIMEOUT_MS)).toBe(100);
expect(resolveRpcCallTimeoutMs(MAX_RPC_TIMEOUT_MS - 1, DEFAULT_RPC_TIMEOUT_MS)).toBe(
MAX_RPC_TIMEOUT_MS - 1,
);
});
it("truncates fractional explicit timeouts", () => {
expect(resolveRpcCallTimeoutMs(1_000.9, DEFAULT_RPC_TIMEOUT_MS)).toBe(1_000);
});
it("normalizes explicit timeouts to Node's timer-safe range", () => {
expect(resolveRpcCallTimeoutMs(0.5, DEFAULT_RPC_TIMEOUT_MS)).toBe(1);
expect(resolveRpcCallTimeoutMs(MAX_NODE_TIMER_TIMEOUT_MS + 1, DEFAULT_RPC_TIMEOUT_MS)).toBe(
MAX_NODE_TIMER_TIMEOUT_MS,
);
});
it("uses the default timeout when no explicit timeout is provided", () => {
expect(resolveRpcCallTimeoutMs(undefined, DEFAULT_RPC_TIMEOUT_MS)).toBe(
DEFAULT_RPC_TIMEOUT_MS,
);
});
it("clamps only the default path to the 15-minute ceiling", () => {
expect(resolveRpcCallTimeoutMs(undefined, 24 * 60 * 60 * 1_000)).toBe(MAX_RPC_TIMEOUT_MS);
});
it("falls back to the clamped default for unusable explicit timeouts", () => {
for (const bad of [0, -1, Number.NaN, Number.POSITIVE_INFINITY]) {
expect(resolveRpcCallTimeoutMs(bad, DEFAULT_RPC_TIMEOUT_MS)).toBe(DEFAULT_RPC_TIMEOUT_MS);
}
expect(resolveRpcCallTimeoutMs(Number.NaN, 24 * 60 * 60 * 1_000)).toBe(MAX_RPC_TIMEOUT_MS);
});
});
describe("plugin-worker-manager stderr failure context", () => {
it("appends worker stderr context to failure messages", () => {
expect(

View File

@ -61,9 +61,23 @@ import { logger } from "../middleware/logger.js";
/** Default timeout for RPC calls in milliseconds. */
const DEFAULT_RPC_TIMEOUT_MS = 30_000;
/** Hard upper bound for any RPC timeout (15 minutes). Prevents unbounded waits. */
/**
* Upper bound for the *default* RPC timeout path (15 minutes). Explicit
* caller-supplied timeouts are not subject to this cap: execute-class RPCs such
* as `environmentExecute` run entire sandboxed agent sessions in one call and
* their callers deliberately request multi-hour budgets (see
* `resolvePluginExecuteRpcTimeoutMs` in plugin-environment-driver.ts).
* Clamping those explicit budgets here killed long sandboxed runs mid-work.
*/
const MAX_RPC_TIMEOUT_MS = 15 * 60 * 1_000;
/**
* Maximum delay accepted by Node timers before Node clamps the timeout to 1ms.
* Keep accepted explicit RPC budgets inside this range before calling
* setTimeout, otherwise a huge timeout can expire almost immediately.
*/
const MAX_NODE_TIMER_TIMEOUT_MS = 2_147_483_647;
/** Timeout for the initialize RPC call. */
const INITIALIZE_TIMEOUT_MS = 15_000;
@ -154,6 +168,30 @@ export function formatWorkerFailureMessage(message: string, stderrExcerpt: strin
return `${message}\n\nWorker stderr:\n${excerpt}`;
}
/**
* Resolve the effective timeout for an RPC call.
*
* An explicit, positive, finite caller-supplied timeout bypasses the 15-minute
* RPC cap after normalization to Node's timer-safe integer range. Callers that
* pass one (e.g. the environment driver for `environmentExecute`) own their
* budget, and independent inactivity/safety guards bound hung runs. Only the
* default path (no usable explicit timeout) is clamped to MAX_RPC_TIMEOUT_MS so
* ordinary plugin calls stay bounded.
*/
export function resolveRpcCallTimeoutMs(
explicitTimeoutMs: number | undefined,
defaultTimeoutMs: number,
): number {
if (
explicitTimeoutMs !== undefined &&
Number.isFinite(explicitTimeoutMs) &&
explicitTimeoutMs > 0
) {
return Math.min(Math.max(Math.trunc(explicitTimeoutMs), 1), MAX_NODE_TIMER_TIMEOUT_MS);
}
return Math.min(defaultTimeoutMs, MAX_RPC_TIMEOUT_MS);
}
/**
* Options for starting a worker process.
*/
@ -1232,7 +1270,7 @@ export function createPluginWorkerHandle(
}
const id = nextRequestId++;
const timeout = Math.min(timeoutMs ?? rpcTimeoutMs, MAX_RPC_TIMEOUT_MS);
const timeout = resolveRpcCallTimeoutMs(timeoutMs, rpcTimeoutMs);
const invocationScope = deriveInvocationScope(method, params);
const invocation = invocationScope ? registerInvocation(invocationScope) : null;
@ -1357,6 +1395,9 @@ export function createPluginWorkerHandle(
notify(method: string, params: unknown) {
if (status !== "running") return;
const invocationScope = deriveInvocationScope(method, params);
// Notifications have no response to settle on, so the invocation scope
// is GC'd by TTL. Call-path invocations are registered without a TTL and
// cleared on settlement, so they survive arbitrarily long call timeouts.
const invocation = invocationScope ? registerInvocation(invocationScope, MAX_RPC_TIMEOUT_MS) : null;
try {
sendMessage({