mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 13:24:55 +01:00
fix: bound sandbox bridge waits and flag silent runs sooner (#14979)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Sandbox agents exchange input and output through bridge control commands. > - A provider can stop responding to a command even when it receives a timeout. > - These small commands can inherit a four-hour agent lifetime and block input or teardown. > - The board also calls a silent run healthy for the first hour. > - This pull request bounds bridge control waits and surfaces silence sooner. ## Linked Issues or Issue Description **What happened?** A sandbox run can remain active when a bridge control command never returns. The shared helper passes a timeout to the provider but does not enforce it on the host. It also accepts the agent's hours-long timeout. Output silence remains `ok` for an hour and becomes `critical` only after four hours. **Expected behavior** Bound short bridge operations even if the provider never settles. Report failed input delivery through the existing shutdown path. Warn after five silent minutes and escalate after fifteen. Keep normal agent command limits and require verified termination before releasing execution ownership. **Steps to reproduce** 1. Use a sandbox runner whose bridge read or input-upload promise never settles. 2. Set its configured timeout to four hours. 3. Observe that the old queue client never returns or rejects. 4. Inspect a running task with 35 minutes of output silence. The old summary still reports `ok`. **Paperclip version or commit** Base commit `d6d88b9de2`. **Deployment mode** Self-hosted server with sandbox execution. **Agent adapter(s) involved** Shared command-managed sandbox bridge, including Codex ACP sessions. The informational silence thresholds apply to active runs across adapters. Related: #14889 recovers stalled Daytona output streams; #14485 retries explicit gateway failures during input delivery. This change bounds short control operations whose provider promises never settle. It does not add tool replay or automatic cancellation for output silence. #6297 proposes configurable per-agent silence thresholds; this patch only changes the existing defaults. ## What Changed - Enforce at most 30 seconds per bridge control shell command on the host and provider, including callback startup and shutdown, process-session launch, and payload setup. Preserve shorter configured deadlines and launch environments. - Keep the long-lived agent command outside this deadline. Use a fixed timeout diagnostic without command payloads. - Surface suspicious output silence after five minutes and critical silence after fifteen minutes. - Decouple the shared-workspace holder cutoff from warning thresholds and preserve its existing one-hour value. - Add regressions for hung reads, a late upload response, failed input delivery, exact warning boundaries, and fresh output clearing warnings. - Update the adapter guide and execution contract. ## Verification - The three new queue-client regressions fail on the unchanged base and pass with this patch. - Final callback bridge and sandbox session suites: 214 passed. These cover hung reads, writes, startup, shutdown, process-session launch, payload setup, and the separate long-running agent limit. - Stdin ordering and shutdown suite: 56 passed after the lifecycle change. - Daytona and watchdog coverage passed in the earlier focused runs. Across the focused suites, 602 distinct tests pass. - `pnpm -r typecheck` and `pnpm build`: passed. Server and adapter typecheck/build also passed after their respective follow-up changes. - `pnpm test:run`: attempted and stopped after known local failures. Four chat/email cases used an external ancestor skill path, three skill-cache cases failed on macOS, and one wakeup case timed out. The wakeup case passes alone (1 passed, 27 skipped). This run spanned the workspace-cutoff follow-up and also failed its new holder case; a fresh final-head workspace suite passes all 19 tests. The interrupted run is not a full local-suite pass or final-head verification. - A filesystem queue-drain test failed once during the lifecycle rerun and passed on the complete two-suite rerun. It uses the filesystem client, outside the changed command-runner path. - Complete CI on `ff2212c235`: 53 successful checks and two expected skips, including the full test suite and canary packaging dry run. No failed or pending checks. - Greptile reviewed `ff2212c235` at 5/5. All review findings are addressed, no threads remain unresolved, and the branch has no merge conflicts with `master`. - `git diff --check` and a scan of added text for secrets and private identifiers passed. ## Risks - A bridge control operation that needs more than 30 seconds now fails, even if the caller selected a longer run lifetime. Agent commands retain their own limits. - Timing out a provider promise does not cancel the remote operation or prove it stopped. Existing execution settlement still owns termination verification. No uncertain tool action is replayed. - Quiet healthy runs display warnings sooner. Existing snooze, continue, and false-positive dismissal controls still apply. Silence alone does not cancel a run, create review work, or change assignments. - No schema or API shape change. ## Model Used OpenAI GPT-6 through Codex, with tool use and code execution. The exact serving model ID and context window 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 change-specific 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:
1 parent
d7bdfc422c
commit
22cea6b2e6
12 files changed
+225
-44
No files matched your search
@@ -230,6 +230,27 @@ describeEmbeddedPostgres("active-run output watchdog", () => {
|
||||
expect(manager?.status).toBe("idle");
|
||||
}
|
||||
|
||||
it("warns after five silent minutes and escalates after fifteen without changing active work", async () => {
|
||||
const now = new Date("2026-04-22T20:00:00.000Z");
|
||||
const seeded = await seedRunningRun({ now, ageMs: 0 });
|
||||
const summaryAt = (elapsedMs: number) => buildSummary(seeded.runId, new Date(now.getTime() + elapsedMs));
|
||||
await expect(summaryAt(5 * 60_000 - 1)).resolves.toMatchObject({ level: "ok" });
|
||||
await expect(summaryAt(5 * 60_000)).resolves.toMatchObject({ level: "suspicious" });
|
||||
await expect(summaryAt(15 * 60_000 - 1)).resolves.toMatchObject({ level: "suspicious" });
|
||||
await expect(summaryAt(15 * 60_000)).resolves.toMatchObject({ level: "critical" });
|
||||
const { recovery, enqueueWakeup } = createRecovery();
|
||||
await recovery.scanSilentActiveRuns({ now: new Date(now.getTime() + 35 * 60_000), companyId: seeded.companyId });
|
||||
const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, seeded.runId));
|
||||
expect(run?.status).toBe("running");
|
||||
expect(enqueueWakeup).not.toHaveBeenCalled();
|
||||
await expectNoReviewArtifacts(seeded);
|
||||
|
||||
// Fresh output clears the warning even on a long-running task.
|
||||
await db.update(heartbeatRuns).set({ lastOutputAt: new Date(now.getTime() + 35 * 60_000) })
|
||||
.where(eq(heartbeatRuns.id, seeded.runId));
|
||||
await expect(summaryAt(36 * 60_000)).resolves.toMatchObject({ level: "ok" });
|
||||
});
|
||||
|
||||
it.each(["stale_active_run_evaluation", "issue_productivity_review"])("keeps blocked and %s sources artifact-free", async (originKind) => {
|
||||
const now = new Date("2026-04-22T20:00:00.000Z");
|
||||
const blocked = await seedRunningRun({
|
||||
|
||||
@@ -523,8 +523,8 @@ describeEmbeddedPostgres("shared-workspace run serialization", () => {
|
||||
expect(retryRuns).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("defers a run whose issue targets a busy shared workspace and schedules a bounded retry", async () => {
|
||||
const fixture = await seedWorkspaceFixture();
|
||||
it.each([0, 35 * 60_000])("defers a run in a busy shared workspace after %i ms of holder silence", async (silenceMs) => {
|
||||
const fixture = await seedWorkspaceFixture({ holderActivityAt: new Date(Date.now() - silenceMs) });
|
||||
|
||||
const run = await heartbeat.invoke(
|
||||
fixture.agentId,
|
||||
|
||||
@@ -527,10 +527,7 @@ import {
|
||||
type StrandedRecoveryNoticeSeed,
|
||||
} from "./recovery/stranded-notice.js";
|
||||
import { withRecoveryContext } from "./recovery/status-only-context.js";
|
||||
import {
|
||||
ACTIVE_RUN_OUTPUT_SUSPICION_THRESHOLD_MS as RECOVERY_ACTIVE_RUN_OUTPUT_SUSPICION_THRESHOLD_MS,
|
||||
recoveryService,
|
||||
} from "./recovery/service.js";
|
||||
import { recoveryService } from "./recovery/service.js";
|
||||
import {
|
||||
createRunDispatch,
|
||||
type PostCommitEffect,
|
||||
@@ -914,16 +911,10 @@ export const WORKSPACE_BUSY_RETRY_WAKE_REASON = "workspace_busy_retry";
|
||||
export const WORKSPACE_BUSY_ERROR_CODE = "workspace_busy";
|
||||
export const WORKSPACE_BUSY_RETRY_BASE_DELAY_MS = 60 * 1000;
|
||||
export const WORKSPACE_BUSY_RETRY_JITTER_MS = 60 * 1000;
|
||||
// A running run stops counting as a shared-workspace holder once it has been
|
||||
// silent this long. This is recovery's own "suspicious silence" bar for active
|
||||
// runs (scanSilentActiveRuns escalates such runs), so a zombie holder cannot
|
||||
// park other work on the workspace forever: it stops blocking here at the same
|
||||
// moment the recovery machinery starts treating it as stuck. A LIVE holder, in
|
||||
// contrast, never gets overtaken — a deferred run keeps rescheduling until the
|
||||
// workspace frees, because dispatching alongside a live holder is exactly the
|
||||
// concurrent-mutation failure this gate exists to prevent.
|
||||
export const WORKSPACE_BUSY_HOLDER_STALE_AFTER_MS =
|
||||
RECOVERY_ACTIVE_RUN_OUTPUT_SUSPICION_THRESHOLD_MS;
|
||||
// Preserve the one-hour shared-workspace holder cutoff independently of the
|
||||
// informational output-silence warnings. Warning sooner must not let another
|
||||
// run overtake a quiet holder and mutate its shared workspace.
|
||||
export const WORKSPACE_BUSY_HOLDER_STALE_AFTER_MS = 60 * 60 * 1000;
|
||||
// Issue-level executionWorkspaceSettings.mode values that unambiguously opt an
|
||||
// issue's runs out of the shared project workspace, and therefore out of
|
||||
// shared-workspace serialization ("isolated" is the legacy alias
|
||||
@@ -16277,8 +16268,8 @@ export function heartbeatService(
|
||||
// issue shares the same project workspace, i.e. the run that currently
|
||||
// "holds" the shared working tree. Runs that have been silent past
|
||||
// WORKSPACE_BUSY_HOLDER_STALE_AFTER_MS do not count — a zombie holder must
|
||||
// not park other work forever, and recovery's silent-run escalation is
|
||||
// already reaping it. When isolated workspaces are enabled, holders whose
|
||||
// not park other work forever. This cutoff is independent of informational
|
||||
// silence warnings. When isolated workspaces are enabled, holders whose
|
||||
// issue explicitly opted into an isolated workspace never touch the shared
|
||||
// tree, so they are excluded; a NULL/agent_default mode may resolve to the
|
||||
// shared tree and counts as a holder (over-serializing is the safe
|
||||
|
||||
@@ -163,8 +163,8 @@ const UNSUCCESSFUL_HEARTBEAT_RUN_TERMINAL_STATUSES = [
|
||||
"cancelled",
|
||||
"timed_out",
|
||||
] as const;
|
||||
export const ACTIVE_RUN_OUTPUT_SUSPICION_THRESHOLD_MS = 60 * 60 * 1000;
|
||||
export const ACTIVE_RUN_OUTPUT_CRITICAL_THRESHOLD_MS = 4 * 60 * 60 * 1000;
|
||||
export const ACTIVE_RUN_OUTPUT_SUSPICION_THRESHOLD_MS = 5 * 60 * 1000;
|
||||
export const ACTIVE_RUN_OUTPUT_CRITICAL_THRESHOLD_MS = 15 * 60 * 1000;
|
||||
export const ACTIVE_RUN_OUTPUT_CONTINUE_REARM_MS = 30 * 60 * 1000;
|
||||
const STRANDED_ISSUE_RECOVERY_ORIGIN_KIND =
|
||||
RECOVERY_ORIGIN_KINDS.strandedIssueRecovery;
|
||||
|
||||
Reference in new issue
Block a user