mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-07 07:55:47 +01:00
## Thinking Path > - Paperclip manages agent work and its execution state. > - Remote Runner processes use health reads to check their saved process identity. > - A failed health request cannot prove that the process stopped. > - Three fast gateway errors can exhaust the old retry limit in half a second. > - The caller already gives these reads a shared 30-second recovery budget. > - This pull request uses that budget with backoff for read-only failures. > - A short gateway outage can then clear without stopping the existing turn. ## Linked Issues or Issue Description **What happened?** A transient HTTP 502 during sandbox process supervision caused an active native Codex turn to fail. The probe stopped after three attempts, even when most of its 30-second budget remained. The run reported `notification_transport_failed` and required workspace recovery. **Expected behavior** Retry read-only transport failures within the existing deadline. Keep process identity conflicts authoritative. Do not repeat a signal after a lost acknowledgement. **Steps to reproduce** 1. Start a read-only process verification with a 30-second deadline. 2. Return HTTP 502 from its first three transport reads, then allow the next read to succeed. 3. The old helper rejects after three reads. The fixed helper returns the successful observation. Related repairs: #16451 introduced bounded remote probes. #16835 and #17799 require proof of process exit before replacement. This change retains those guards. ## What Changed - Use the existing absolute deadline instead of a three-attempt limit for read-only probe failures. - Back off from 250 milliseconds to two seconds. Clip each request to ten seconds or the remaining budget. - Preserve the original error and diagnostic when the deadline is exhausted. Reject non-finite deadlines before an RPC. - Add a red/green regression and a repeated-gateway-failure case around a real listening process. - Update execution semantics and both recovery maps. ## Verification - Before the fix: the new probe suite failed two behavioral assertions. The gateway case returned its HTTP 502 error instead of the later successful observation. - After the fix: 75 focused probe, diagnostic, process-supervision and adoption tests pass. The diagnostic failure tests use a controlled clock for their full 30-second budget. Their error and redaction assertions stay unchanged. - Map alignment and module-boundary checks pass. - Full workspace typecheck and build pass at final commit `ddd42f66378ebc36ec1d7e0ca1f45943473561b3`. - The broader affected suite against disposable PostgreSQL passed 994 of 997 tests; its three startup/database timeouts passed unchanged in isolation. The initial full default local run encountered embedded PostgreSQL startup failures and did not complete. The four diagnostic tests that assumed the old short retry timing now use a controlled clock without weakening their assertions. - Incident evidence came from Linux AMD64, Node 24.22.2, Core commit `0c70cedce9a803bdc201324528abb35e423e7cd0`, native Codex `gpt-6.1-sol`, and a shared ChatGPT subscription. Two separate runs recorded the same supervision HTTP 502 within one second. - A disposable live Daytona sandbox check passed on Linux x86_64 and Node 24.22.1. Three injected gateway failures recovered in 3.31 seconds on production source from `e54a355fa179022caaa6854302ff56bd85de0229` (unchanged by the final test-only commit). The real Node listening process had one launch, retained its exact identity and saved file across a second read, and produced a confirmed Stop receipt. Sandbox deletion succeeded. This is process-supervision integration evidence, not a Codex task or model execution. - Fixed-revision hosted acceptance and recovery of the original conversation require rollout. This PR does not claim either. The duration of the original provider outage remains unknown. - Final commit `ddd42f66378ebc36ec1d7e0ca1f45943473561b3`: required `ci / verify` and `ci / e2e` checks pass; Greptile is 5/5 with no inline findings. The separate Paperclip Review service has not completed an assessment and remains queued. It is not a required branch check. ## Risks - Persistent fast transport failures cause more read-only requests within the same deadline. Capped backoff limits their rate. - Missing or changed identity still returns immediately. Failed health reads still cannot authorize replacement, process signals, or provider replay. - Signal requests still have one attempt. The timeout budget and execution permissions do not change. - No schema or package changes. ## Model Used - OpenAI GPT-6, through Codex, with reasoning, repository inspection, and shell tool use. The exact deployment model ID and context-window size are not exposed by 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 - [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>
91 lines
4.4 KiB
TypeScript
91 lines
4.4 KiB
TypeScript
import { afterEach, expect, it, vi } from "vitest";
|
|
import type { CommandManagedRuntimeRunner } from "@paperclipai/adapter-utils/command-managed-runtime";
|
|
import { readRemoteRunnerRecoveryError } from "./remote-runner-recovery-diagnostics.js";
|
|
import { readRemoteRunnerRecoveryProbe } from "./remote-runner-recovery-probe.js";
|
|
|
|
afterEach(() => vi.useRealTimers());
|
|
|
|
const timeout = { exitCode: null, signal: null, timedOut: true, stdout: "", stderr: "", pid: null, startedAt: "2026-10-15T00:00:00Z" };
|
|
|
|
it.each([NaN, Infinity, -Infinity])("rejects a non-finite deadline (%s) before a probe", async deadline => {
|
|
const execute = vi.fn().mockResolvedValue({ ...timeout, timedOut: false, exitCode: 0 });
|
|
const result = await readRemoteRunnerRecoveryProbe({
|
|
runner: { execute }, request: { command: "sh" }, stage: "process_verification",
|
|
readOnly: true, deadline,
|
|
}).catch(readRemoteRunnerRecoveryError);
|
|
expect(result).toEqual({ stage: "process_verification", reason: "probe_timeout" });
|
|
expect(execute).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("uses the read deadline when a transport outage outlasts three fast probes", async () => {
|
|
vi.useFakeTimers();
|
|
const unavailable = Object.assign(new Error("Request failed with status code 502"), { response: { status: 502 } });
|
|
const healthy = { ...timeout, timedOut: false, exitCode: 0 };
|
|
const execute = vi.fn()
|
|
.mockRejectedValueOnce(unavailable)
|
|
.mockRejectedValueOnce(unavailable)
|
|
.mockRejectedValueOnce(unavailable)
|
|
.mockResolvedValue(healthy);
|
|
const result = readRemoteRunnerRecoveryProbe({
|
|
runner: { execute }, request: { command: "sh" }, stage: "process_verification",
|
|
readOnly: true, deadline: Date.now() + 30_000,
|
|
}).catch(error => error);
|
|
await vi.runAllTimersAsync();
|
|
expect(await result).toBe(healthy);
|
|
expect(execute).toHaveBeenCalledTimes(4);
|
|
});
|
|
|
|
it("bounds fast transport failures by the existing deadline and retains the cause", async () => {
|
|
vi.useFakeTimers();
|
|
const unavailable = new Error("sandbox unavailable");
|
|
const execute = vi.fn().mockRejectedValue(unavailable);
|
|
const startedAt = Date.now();
|
|
const result = readRemoteRunnerRecoveryProbe({
|
|
runner: { execute }, request: { command: "sh" }, stage: "process_verification",
|
|
readOnly: true, deadline: startedAt + 2_000,
|
|
}).catch(error => error);
|
|
await vi.runAllTimersAsync();
|
|
expect(await result).toBe(unavailable);
|
|
expect(readRemoteRunnerRecoveryError(unavailable)).toEqual({ stage: "process_verification", reason: "probe_transport_failed" });
|
|
expect(execute.mock.calls.length).toBeGreaterThan(3);
|
|
expect(execute.mock.calls.length).toBeLessThan(9);
|
|
expect(Date.now() - startedAt).toBeLessThanOrEqual(2_000);
|
|
expect(execute.mock.calls.at(-1)?.[0].timeoutMs).toBeLessThanOrEqual(250);
|
|
});
|
|
|
|
it("shares the remaining deadline across timed-out reads", async () => {
|
|
vi.useFakeTimers();
|
|
const execute = vi.fn(async (request) => {
|
|
await new Promise(resolve => setTimeout(resolve, request.timeoutMs));
|
|
return timeout;
|
|
});
|
|
const result = readRemoteRunnerRecoveryProbe({
|
|
runner: { execute } as CommandManagedRuntimeRunner,
|
|
request: { command: "sh" }, stage: "identity_marker",
|
|
readOnly: true, deadline: Date.now() + 15_000,
|
|
}).catch(readRemoteRunnerRecoveryError);
|
|
await vi.runAllTimersAsync();
|
|
expect(await result).toEqual({ stage: "identity_marker", reason: "probe_timeout" });
|
|
expect(execute.mock.calls.map(([request]) => request.timeoutMs)).toEqual([10_000, 4_750]);
|
|
});
|
|
|
|
it("never retries a signal when its acknowledgement is lost", async () => {
|
|
const execute = vi.fn().mockRejectedValue(new Error("connection lost"));
|
|
const result = await readRemoteRunnerRecoveryProbe({
|
|
runner: { execute }, request: { command: "sh" }, stage: "process_verification",
|
|
readOnly: false, deadline: Date.now() + 30_000,
|
|
}).catch(readRemoteRunnerRecoveryError);
|
|
expect(result).toEqual({ stage: "process_verification", reason: "probe_transport_failed" });
|
|
expect(execute).toHaveBeenCalledTimes(1);
|
|
});
|
|
|
|
it("returns a definitive ownership conflict without retrying it", async () => {
|
|
const conflict = { ...timeout, timedOut: false, exitCode: 47 };
|
|
const execute = vi.fn().mockResolvedValue(conflict);
|
|
expect(await readRemoteRunnerRecoveryProbe({
|
|
runner: { execute }, request: { command: "sh" }, stage: "process_verification",
|
|
readOnly: true, deadline: Date.now() + 30_000,
|
|
})).toBe(conflict);
|
|
expect(execute).toHaveBeenCalledTimes(1);
|
|
});
|