mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
perf(runner): skip the turn-end provider restart in a per-turn Runner (#20312)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The Paperclip Runner runs each agent's provider (for Claude, an ACPX
sidecar) and keeps its session across runs.
> - #20190 restarts the ACPX provider when a turn settles, so a warm
follow-up does not pay the respawn on its critical path.
> - A per-turn Runner (`lifecycleMode: per_turn`, the default) exits
with its run. Nothing reuses the generation that restart prepares: the
next run starts a new Runner process and loads the session again.
> - In that mode the restart can only spawn a provider for teardown. On
a loaded host it can also fail, which was the cause of the closed
sessions that #20285 fixed.
> - This pull request skips the turn-end restart when the controller
launched the Runner as per-turn.
> - The benefit is that per-turn runs never spend a provider spawn they
cannot use, and never risk a restart failure at the end of their run.
## Linked Issues or Issue Description
Refs #20190, #20285
**What happened?**
A per-turn Runner restarted its ACPX provider after its turn settled,
although the Runner exits with its run. The next run launches a new
Runner process (`runner.process.launch mode=resume`, then
`provider.session.load`), so the restarted generation is torn down
unused. In an earlier A/B on a host with load average about 17–27, this
restart fired and failed, and the failure closed the session (fixed in
#20285).
**Steps to reproduce**
1. Run a Claude task with the default per-turn lifecycle.
2. Let the turn settle and the controller acknowledge its events before
it shuts the Runner down (more likely on a loaded host).
3. The Runner reaps the provider, spawns a new generation, and then
exits with the run, which reaps the new generation too.
**Expected behavior**
A per-turn Runner does not restart its provider after its turn. A warm
Runner keeps the #20190 restart. A later turn of the same run still
restarts the provider at its start.
## What Changed
- `DurableRunnerConfig` has a new `turn_scoped` field.
- `paperclip-runnerd` sets it from `--lifecycle-mode`, which the
controller already passes (`durable-prp-control-plane.ts` pushes
`--lifecycle-mode <mode>`), but which `runnerd` ignored until now. Only
`per_turn` sets it. `warm`, and launches without the flag (older
controllers), keep today's behavior.
- `NativeProviderCommandExecutor::prepare_next_turn` returns early when
`turn_scoped` is set. The lazy restart at the next turn start is
unchanged.
- Tests:
- New
`a_per_turn_runner_skips_the_turn_end_restart_and_restarts_a_later_turn_at_its_start`:
no new provider process and no sidecar command after the turn settles; a
later turn of the same run still restarts at its start
(`session.suspend`, `initialize`). It fails without the skip (2 process
starts instead of 1).
- New `only_a_per_turn_lifecycle_is_turn_scoped` for the flag parsing.
- The other `DurableRunnerConfig` literals set `turn_scoped: false`.
## Verification
- `cargo test -p paperclip-runner-core`:
- `--lib`: 602 passed.
- `--test acpx_park_preparation`: 22 passed.
- `acpx_park_readiness`: 8 passed.
- `acpx_warm_attachment`: 4 passed.
- `acpx_provider_suspend`: 5 passed.
- `acpx_provider_session`: 12 passed.
- `acpx_provider_turns`: 29 passed.
- `native_provider_backend`: 47 passed.
- `durable_recovery`: 3 passed.
- `--bin paperclip-runnerd turn_scoped`: 1 passed.
- `cargo build --workspace --all-targets` passes.
- Local live A/B with `ttft-baseline --adapters claude --samples 3
--restarts 0 --phases coldwarm`. Control is master after #20285
(7fe9345030); the other arm is this branch. Host load average was about
6–9.
| | Cold first activity p50 | Cold TTFT p50 | Warm first activity p50 |
Warm TTFT p50 | Result-accepted → exit |
|---|---|---|---|---|---|
| Control | 5.61 s | 8.86 s | 4.95 s | 5.00 s | 1.49–1.84 s |
| This branch | 6.72 s | 9.83 s | 5.52 s | 5.57 s | 1.45–1.63 s (one
cold run 3.64 s) |
- At this load the difference is within run-to-run noise. Both arms
passed 6/6 samples.
- A process sampler saw no short-lived provider process in either arm.
Every provider process lived 11–16 s, about one run. So at load ~8 the
per-turn Runner exited before the turn-end restart ran, and this change
had nothing to remove.
- The restart did run, and fail, in the earlier A/B at load ~17–27
(#20285). This change removes that path for per-turn Runners. I did not
reproduce that load level here, so this PR has no measured speed gain to
claim.
## Risks
- **Low risk.** The change only removes work in per-turn Runners. Warm
Runners and launches without `--lifecycle-mode` behave as before.
- If a per-turn run starts a second turn, that turn restarts its
provider at its start, as before #20190. That costs the respawn on its
critical path; per-turn runs normally have one turn.
- Remote launchers that build their own `runnerd` arguments and do not
pass `--lifecycle-mode` keep today's restart. That is the safe default.
## Model Used
Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool
use and code execution, via Claude Code.
## 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
🤖 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
189fefb991
commit
97188f276c
20 files changed
+87
-1
No files matched your search
@@ -273,6 +273,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
|
||||
value("--reconnect-grace-ms")?.parse()?,
|
||||
)),
|
||||
max_runtime: Duration::ZERO,
|
||||
turn_scoped: false,
|
||||
};
|
||||
run_durable_runner(
|
||||
config,
|
||||
|
||||
@@ -3341,6 +3341,7 @@ mod tests {
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::from_secs(60),
|
||||
turn_scoped: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -243,6 +243,13 @@ fn optional_value(args: &[String], name: &str) -> Result<Option<String>, LocalRu
|
||||
.ok_or_else(|| LocalRunnerError::invalid(format!("missing value for {name}")))
|
||||
}
|
||||
|
||||
/// Launches without `--lifecycle-mode` (older controllers) and warm launches
|
||||
/// keep the turn-end provider restart; only an explicit per-turn lifecycle,
|
||||
/// whose Runner exits with its run, skips it.
|
||||
fn turn_scoped(args: &[String]) -> Result<bool, LocalRunnerError> {
|
||||
Ok(optional_value(args, "--lifecycle-mode")?.as_deref() == Some("per_turn"))
|
||||
}
|
||||
|
||||
fn optional_u64(args: &[String], name: &str) -> Result<Option<u64>, LocalRunnerError> {
|
||||
let Some(index) = args.iter().position(|argument| argument == name) else {
|
||||
return Ok(None);
|
||||
@@ -497,6 +504,7 @@ fn run_durable(args: &[String]) -> Result<(), LocalRunnerError> {
|
||||
reconnect_delay: duration("--reconnect-delay-ms", 250)?,
|
||||
reconnect_grace: optional_u64(args, "--reconnect-grace-ms")?.map(Duration::from_millis),
|
||||
max_runtime: duration("--max-runtime-ms", 0)?,
|
||||
turn_scoped: turn_scoped(args)?,
|
||||
};
|
||||
if host_control.is_some() && !config.connect_url.starts_with("listen://") {
|
||||
return Err(LocalRunnerError::invalid(
|
||||
@@ -696,6 +704,26 @@ fn run_main(args: Vec<String>) -> ExitCode {
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn only_a_per_turn_lifecycle_is_turn_scoped() {
|
||||
let args = |values: &[&str]| {
|
||||
values
|
||||
.iter()
|
||||
.map(|value| value.to_string())
|
||||
.collect::<Vec<_>>()
|
||||
};
|
||||
assert!(turn_scoped(&args(&["--lifecycle-mode", "per_turn"])).unwrap());
|
||||
assert!(!turn_scoped(&args(&[
|
||||
"--lifecycle-mode",
|
||||
"warm",
|
||||
"--idle-timeout-ms",
|
||||
"1"
|
||||
]))
|
||||
.unwrap());
|
||||
assert!(!turn_scoped(&args(&[])).unwrap());
|
||||
assert!(turn_scoped(&args(&["--lifecycle-mode"])).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cursor_cloud_launch_flags_are_optional_as_a_whole_and_required_as_an_exact_pin_set() {
|
||||
assert!(cursor_cloud_launch_profile(&[]).unwrap().is_none());
|
||||
|
||||
@@ -249,6 +249,10 @@ pub struct DurableRunnerConfig {
|
||||
pub reconnect_grace: Option<Duration>,
|
||||
/// Zero disables the total process lifetime limit.
|
||||
pub max_runtime: Duration,
|
||||
/// The controller launched this Runner for one run (`--lifecycle-mode
|
||||
/// per_turn`): the process exits with its run, so nothing reuses a provider
|
||||
/// generation prepared after its turn settles.
|
||||
pub turn_scoped: bool,
|
||||
}
|
||||
|
||||
impl DurableRunnerConfig {
|
||||
@@ -429,6 +433,7 @@ mod tests {
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::from_secs(60),
|
||||
turn_scoped: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -3053,6 +3053,7 @@ mod tests {
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::from_secs(1),
|
||||
turn_scoped: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -2859,6 +2859,7 @@ mod tests {
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::from_secs(1),
|
||||
turn_scoped: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1783,6 +1783,7 @@ mod tests {
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::from_secs(1),
|
||||
turn_scoped: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1084,6 +1084,7 @@ mod tests {
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::from_secs(60),
|
||||
turn_scoped: false,
|
||||
}
|
||||
}
|
||||
fn command(kind: &str, payload: Value) -> Command {
|
||||
|
||||
@@ -2617,6 +2617,7 @@ mod tests {
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::from_secs(60),
|
||||
turn_scoped: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -345,6 +345,12 @@ impl CommandExecutor for NativeProviderCommandExecutor {
|
||||
}
|
||||
|
||||
fn prepare_next_turn(&mut self) -> Result<(), DurableRunnerError> {
|
||||
// A per-turn Runner exits with its run. Restarting the provider after
|
||||
// its turn would only spawn a generation for teardown; a later turn of
|
||||
// the same run still restarts lazily at its start.
|
||||
if self.config.turn_scoped {
|
||||
return Ok(());
|
||||
}
|
||||
match self.selected.as_mut() {
|
||||
Some(SelectedExecutor::Acpx(executor)) => executor.prepare_next_turn(),
|
||||
_ => Ok(()),
|
||||
|
||||
@@ -412,6 +412,7 @@ mod tests {
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::ZERO,
|
||||
turn_scoped: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -268,6 +268,7 @@ mod tests {
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::ZERO,
|
||||
turn_scoped: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -60,6 +60,10 @@ struct Harness {
|
||||
|
||||
impl Harness {
|
||||
fn new() -> Self {
|
||||
Self::configured(|_| {})
|
||||
}
|
||||
|
||||
fn configured(adjust: impl FnOnce(&mut DurableRunnerConfig)) -> Self {
|
||||
let catalog: Value =
|
||||
serde_json::from_str(include_str!("../../../../acpx-profiles.json")).unwrap();
|
||||
let profile = &catalog["profiles"]["claude"];
|
||||
@@ -84,7 +88,7 @@ impl Harness {
|
||||
.unwrap();
|
||||
fs::set_permissions(&sidecar, fs::Permissions::from_mode(0o700)).unwrap();
|
||||
let args = vec!["--profile-digest".to_owned(), digest.to_owned()];
|
||||
let config = DurableRunnerConfig {
|
||||
let mut config = DurableRunnerConfig {
|
||||
connect_url: "ws://127.0.0.1/runner".to_owned(),
|
||||
ca_bundle_path: None,
|
||||
state_dir: fixture.0.clone(),
|
||||
@@ -114,6 +118,7 @@ impl Harness {
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::from_secs(30),
|
||||
turn_scoped: false,
|
||||
};
|
||||
let descriptor = json!({
|
||||
"kind": "acpx", "provider": "acpx", "driver": "acpx_runtime",
|
||||
@@ -131,6 +136,7 @@ impl Harness {
|
||||
"mcp": {"assignmentSetId": "assignment-1", "digest": "e".repeat(64), "bindingId": "binding-run-1"},
|
||||
},
|
||||
});
|
||||
adjust(&mut config);
|
||||
let mut executor = NativeProviderCommandExecutor::with_runner_config(&fixture.0, &config);
|
||||
executor
|
||||
.execute(&command(1, "run.prepare", json!({"provider": descriptor})))
|
||||
|
||||
@@ -85,6 +85,7 @@ fn fixture(
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::from_secs(30),
|
||||
turn_scoped: false,
|
||||
};
|
||||
let descriptor = json!({
|
||||
"kind": "acpx", "provider": "acpx", "driver": "acpx_runtime",
|
||||
|
||||
@@ -79,6 +79,7 @@ fn checkpoints_the_sidecar_and_rebinds_consecutive_warm_runs_before_accepting_wo
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::from_secs(30),
|
||||
turn_scoped: false,
|
||||
};
|
||||
let mut descriptor = json!({
|
||||
"kind": "acpx", "provider": "acpx", "driver": "acpx_runtime",
|
||||
@@ -238,6 +239,7 @@ fn reopens_the_conversation_for_a_turn_after_a_suspension_stop() {
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::from_secs(30),
|
||||
turn_scoped: false,
|
||||
};
|
||||
let descriptor = json!({
|
||||
"kind": "acpx", "provider": "acpx", "driver": "acpx_runtime",
|
||||
@@ -355,6 +357,7 @@ fn restores_the_conversation_after_losing_an_active_turn_to_a_runner_restart() {
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::from_secs(30),
|
||||
turn_scoped: false,
|
||||
};
|
||||
let descriptor = json!({
|
||||
"kind": "acpx", "provider": "acpx", "driver": "acpx_runtime",
|
||||
@@ -497,6 +500,7 @@ fn restores_a_turn_lost_to_a_runner_upgrade_under_the_new_launch_profile() {
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::from_secs(30),
|
||||
turn_scoped: false,
|
||||
};
|
||||
let descriptor = json!({
|
||||
"kind": "acpx", "provider": "acpx", "driver": "acpx_runtime",
|
||||
|
||||
@@ -594,6 +594,7 @@ fn durable_config(directory: &Path) -> DurableRunnerConfig {
|
||||
reconnect_delay: std::time::Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: std::time::Duration::from_secs(5),
|
||||
turn_scoped: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -32,6 +32,7 @@ fn config(state_dir: PathBuf) -> DurableRunnerConfig {
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::from_secs(1),
|
||||
turn_scoped: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -152,6 +152,7 @@ fn config(state_dir: &Path) -> DurableRunnerConfig {
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::from_secs(60),
|
||||
turn_scoped: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -56,6 +56,7 @@ fn real_cli_uses_relative_state_from_another_directory_and_never_process_run_ide
|
||||
reconnect_delay: Duration::from_millis(1),
|
||||
reconnect_grace: None,
|
||||
max_runtime: Duration::ZERO,
|
||||
turn_scoped: false,
|
||||
};
|
||||
let store = DurableStateStore::new(&path).unwrap();
|
||||
let (mut state, _) = store.load_or_create(&config).unwrap();
|
||||
|
||||
+23
@@ -484,3 +484,26 @@ fn a_runner_restart_in_the_same_run_keeps_the_failed_turn_end_restart_closed() {
|
||||
harness.complete_turn("next-run");
|
||||
harness.executor.shutdown().unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_per_turn_runner_skips_the_turn_end_restart_and_restarts_a_later_turn_at_its_start() {
|
||||
let mut harness = Harness::configured(|config| config.turn_scoped = true);
|
||||
let old_pid = harness.opened["processId"].clone();
|
||||
harness.complete_turn("first");
|
||||
let before = harness.fixture.journal().len();
|
||||
// The Runner exits with its run: no generation is spawned for teardown.
|
||||
harness.executor.prepare_next_turn().unwrap();
|
||||
assert_eq!(harness.fixture.process_starts().len(), 1);
|
||||
assert_eq!(
|
||||
commands_after(&harness.fixture.journal(), before),
|
||||
Vec::<String>::new()
|
||||
);
|
||||
assert_eq!(live_providers(&harness), [old_pid.clone()]);
|
||||
// A later turn of the same run still gets a fresh generation, at its start.
|
||||
let (events, commands, _) = start_follow_up(&mut harness, "second");
|
||||
assert_eq!(commands[..2], ["session.suspend", "initialize"]);
|
||||
assert_eq!(reconciled(&events)["previousProcessId"], old_pid);
|
||||
assert_eq!(harness.fixture.process_starts().len(), 2);
|
||||
harness.executor.shutdown().unwrap();
|
||||
assert_eq!(live_providers(&harness), Vec::<Value>::new());
|
||||
}
|
||||
Reference in new issue
Block a user