mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(runner): record a runner exit mid-turn as runner loss, and stop the lineage sampler panicking (#20259)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The native runner (`runnerd`) runs each provider turn, and the server records why a run stopped and what the person can do next > - When `runnerd` exits during a provider turn, the server saved the run as `native_provider_terminal_failed`: "The provider session is permanently unusable", with automatic retries stopped > - The real cause was the runner process, and the transport already names it (`native_runner_process_exited`) at the start of the closed-stream message > - Separately, the process-lineage sampler added in #19727 panicked on Linux debug builds when a provider tool started its own session, and that panic text became the run's visible failure > - This pull request records a runner exit mid-turn as runner loss, and reads session IDs without the assertion that panicked > - The benefit is that a person sees the true cause of a stopped run, and recovery treats a lost runner like every other runner exit ## Linked Issues or Issue Description Refs #19727 **What happened** A chat eval killed only the owned `runnerd` process (SIGKILL) while a provider tool was running. The run failed with `errorCode: native_provider_terminal_failed` and the next action "The provider session is permanently unusable". The error text was `Notification transport failed closed: native_runner_process_exited: runnerd exited unexpectedly: paperclip-runnerd panic: panicked at rustix-1.1.4/src/pid.rs:61:9: assertion failed: raw > 0`. The panic came from the lineage sampler thread before the kill. It called `rustix::process::getsid`, which asserts a nonzero session. A kernel thread, or a process whose session leader is outside the PID namespace, reports session 0. **Expected behavior** A runner process exit is recorded as `native_runner_process_exited`, the same cause the server already uses for a runner exit outside a turn. Recovery then applies its normal runner-loss rules (checkpoint evidence, bounded attempts, cleanup quarantine). The lineage sampler does not panic, and its diagnostics do not replace the real failure text. **Steps to reproduce** 1. Start a native OpenCode run on Linux with a debug `runnerd`. 2. Let the agent run a shell tool that starts its own session (for example a long `node -e` command). 3. Send SIGKILL to the `runnerd` PID only. 4. Read `GET /api/heartbeat-runs/<runId>`: `errorCode` is `native_provider_terminal_failed` and the error names the rustix panic. ## What Changed - `nativeSessionFailureSourceCode` returns `native_runner_process_exited` for a `NativeProviderTerminalFailure` with code `notification_transport_failed` when its message starts with `Notification transport failed closed: native_runner_process_exited: `. Only the runner transport writes that prefix. Other closed-stream causes stay `native_provider_terminal_failed`. - `paperclip-process-table` has a new `session(pid)` function. Linux reads the session field of `/proc/<pid>/stat`. macOS calls `getsid`. A session of 0 or an error gives `None`. - `process_lineage` reads sessions through `paperclip_process_table::session` instead of rustix `getsid`. - Regression tests: the runner-loss classification (attempts 1 and 3, and three negative shapes), the stat session parser with a kernel-thread line, and a live read of this process's session. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-session-executor.test.ts -t "runner process exit that closed|preserves stable provider and runner|model rejection from a provider terminal|operator-owned through terminal"` (8 passed) - `cd packages/paperclip-runner/runner && cargo test -p paperclip-process-table` (macOS: 3 passed; the Linux-only parser tests run in CI) - `cd packages/paperclip-runner/runner && cargo test -p paperclip-runner-core --lib process_lineage` (6 passed) - End to end: the `recovery.*` chat eval cells that kill `runnerd` are run against this change on a separate eval branch. ## Risks - Behavior shift: a runner exit during a turn now follows runner-loss recovery instead of stopping as a permanent provider failure. Recovery still decides from durable evidence. Ambiguous state stays terminal, and an unverified cleanup stays quarantined. - Previous-release story: no persisted shape changes. Runs saved earlier with `native_provider_terminal_failed` keep that code. Only new failures are classified with the new rule. - The session read on Linux now uses `/proc` instead of a syscall. It gives the same value, and it reports 0 (now `None`) in the same cases. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use (shell, file edits). ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
b3bf12a5db
commit
3374ca54f5
4 files changed
+83
-4
No files matched your search
@@ -68,6 +68,16 @@ pub fn read(pid: i32) -> Read {
|
||||
platform::read(pid)
|
||||
}
|
||||
|
||||
/// The session a process belongs to. `None` when the process cannot be read,
|
||||
/// or when its session is not visible here: a kernel thread, or a process
|
||||
/// whose session leader is outside this PID namespace, reports session 0.
|
||||
pub fn session(pid: i32) -> Option<i32> {
|
||||
if pid <= 0 {
|
||||
return None;
|
||||
}
|
||||
platform::session(pid).filter(|sid| *sid > 0)
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
mod platform {
|
||||
use super::{ProcessEntry, Read};
|
||||
@@ -137,6 +147,12 @@ mod platform {
|
||||
zombie: info.pbi_status == SZOMB,
|
||||
})
|
||||
}
|
||||
|
||||
pub(super) fn session(pid: i32) -> Option<i32> {
|
||||
// SAFETY: getsid reads no caller memory; it returns -1 on error.
|
||||
let sid = unsafe { libc::getsid(pid) };
|
||||
(sid > 0).then_some(sid)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
@@ -181,6 +197,17 @@ mod platform {
|
||||
zombie: state == "Z" || state == "X",
|
||||
})
|
||||
}
|
||||
|
||||
pub(super) fn session(pid: i32) -> Option<i32> {
|
||||
parse_session(&std::fs::read_to_string(format!("/proc/{pid}/stat")).ok()?)
|
||||
}
|
||||
|
||||
/// The session field of `/proc/<pid>/stat`. The kernel writes 0 when the
|
||||
/// session is not visible in the reader's PID namespace.
|
||||
pub(super) fn parse_session(stat: &str) -> Option<i32> {
|
||||
let rest = &stat[stat.rfind(')')? + 1..];
|
||||
rest.split_whitespace().nth(3)?.parse().ok()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(any(target_os = "linux", target_os = "macos")))]
|
||||
@@ -194,6 +221,10 @@ mod platform {
|
||||
pub(super) fn read(_pid: i32) -> Read {
|
||||
Read::Unreadable
|
||||
}
|
||||
|
||||
pub(super) fn session(_pid: i32) -> Option<i32> {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -239,5 +270,31 @@ mod tests {
|
||||
(parsed.ppid, parsed.pgid, parsed.started, parsed.zombie),
|
||||
(1, 4200, 987654, false)
|
||||
);
|
||||
assert_eq!(platform::parse_session(stat), Some(4200));
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
#[test]
|
||||
fn reports_no_session_for_a_kernel_thread() {
|
||||
// A kernel thread, or a process whose session leader is outside this
|
||||
// PID namespace, reports session 0. rustix's getsid asserts a nonzero
|
||||
// session in debug builds, which panicked runnerd's lineage sampler.
|
||||
let stat = "2 (kthreadd) S 0 0 0 0 -1 2129984 0 0 0 0 0 0 0 0 20 0 1 0 9 0 0";
|
||||
assert_eq!(platform::parse_session(stat), Some(0));
|
||||
let kthreadd = std::fs::read_to_string("/proc/2/stat")
|
||||
.ok()
|
||||
.filter(|stat| stat.contains("(kthreadd)"));
|
||||
if kthreadd.is_some() {
|
||||
assert_eq!(session(2), None);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(any(target_os = "linux", target_os = "macos"))]
|
||||
#[test]
|
||||
fn reads_this_process_session() {
|
||||
let me = i32::try_from(std::process::id()).unwrap();
|
||||
assert!(session(me).is_some_and(|sid| sid > 0));
|
||||
assert_eq!(session(0), None);
|
||||
assert_eq!(session(-1), None);
|
||||
}
|
||||
}
|
||||
@@ -59,10 +59,10 @@ impl ProcessTable for LiveProcessTable {
|
||||
}
|
||||
|
||||
fn session_of(&self, pid: i32) -> Option<i32> {
|
||||
let pid = rustix::process::Pid::from_raw(pid)?;
|
||||
rustix::process::getsid(Some(pid))
|
||||
.ok()
|
||||
.map(|sid| sid.as_raw_nonzero().get())
|
||||
// Not rustix's getsid: it asserts a nonzero session, and a kernel
|
||||
// thread or a process whose session leader is outside this PID
|
||||
// namespace reports 0. That assertion killed the sampler thread.
|
||||
paperclip_process_table::session(pid)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -20548,6 +20548,22 @@ it("saves a runner-marked model rejection from a provider terminal as a model ch
|
||||
expect(nativeSessionFailureSourceCode(new NativeProviderTerminalFailure("provider_turn_failed", false,
|
||||
"openrouter/unavailable-model is not a valid model ID"))).toBe("native_provider_terminal_failed");
|
||||
});
|
||||
it("records a runner process exit that closed the provider event stream as runner loss", () => {
|
||||
// Shape the Codex/OpenCode drivers write when runnerd exits mid-turn.
|
||||
const lost = new NativeProviderTerminalFailure("notification_transport_failed", false,
|
||||
"Notification transport failed closed: native_runner_process_exited: runnerd exited unexpectedly with code 137");
|
||||
const code = nativeSessionFailureSourceCode(lost);
|
||||
expect(code).toBe("native_runner_process_exited");
|
||||
expect(nativeSessionFailureDisposition(1, new Date(), code)).toMatchObject({ phase: "retryable_failure", failureCode: "native_session_interrupted" });
|
||||
expect(nativeSessionFailureDisposition(3, new Date(), code)).toEqual({ phase: "terminal_failure", failureCode: "native_session_retry_exhausted", nextAttemptAt: null });
|
||||
// Other closed-stream causes, and the runner code only inside provider text, stay provider failures.
|
||||
expect(nativeSessionFailureSourceCode(new NativeProviderTerminalFailure("notification_transport_failed", false,
|
||||
"Notification transport failed closed: provider stream ended"))).toBe("native_provider_terminal_failed");
|
||||
expect(nativeSessionFailureSourceCode(new NativeProviderTerminalFailure("provider_turn_failed", false,
|
||||
"Notification transport failed closed: native_runner_process_exited: runnerd exited unexpectedly"))).toBe("native_provider_terminal_failed");
|
||||
expect(nativeSessionFailureSourceCode(new NativeProviderTerminalFailure("notification_transport_failed", false,
|
||||
"Notification transport failed closed: tool said native_runner_process_exited: oops"))).toBe("native_provider_terminal_failed");
|
||||
});
|
||||
it("keeps an ACPX missing runtime package failure actionable and stops automatic retries", () => {
|
||||
const failure = new Error('PRP command session.open failed: {"result":{"code":"command_execution_failed","message":"failed to start ACPX provider: ACPX sidecar command session.open was rejected (retryable=false, classification=runtime_package_missing)"},"status":"failed"}');
|
||||
const code = nativeSessionFailureSourceCode(failure);
|
||||
|
||||
@@ -8078,6 +8078,12 @@ export function nativeSessionFailureSourceCode(
|
||||
// terminal message; it needs a model choice, not a generic provider failure.
|
||||
if (!error.recoverable && /^native_provider_model_rejected:/.test(error.message))
|
||||
return "native_provider_model_rejected";
|
||||
// The event stream closed because the runner process itself exited. The
|
||||
// runner transport names that cause first in the closed-stream message; it
|
||||
// is runner loss, not a provider verdict on its session.
|
||||
if (error.providerCode === "notification_transport_failed"
|
||||
&& /^Notification transport failed closed: native_runner_process_exited: /.test(error.message))
|
||||
return "native_runner_process_exited";
|
||||
return "native_provider_terminal_failed";
|
||||
}
|
||||
if (error instanceof NativeSessionProtocolIntegrityError)
|
||||
|
||||
Reference in new issue
Block a user