mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
DottaandClaude Fable 5.2
76da2fec4f
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The server runs each native agent session through a runner transport. The transport reads the events that the runner commits to the session's control plane and delivers them to the driver. > - The transport found those events with a timer. Every live transport ran a pass every 5 ms, for as long as the session was open, whether or not an event had arrived. > - A CPU profile of the staging server shows the cost. The main thread was at 96% of one core, and this pass used 5.6% of it with about a dozen live sessions. The cost grows with the number of sessions and never stops. > - The same timer also delayed every event. An event waited up to 5 ms for the next pass before the driver saw it. Agent boot time and time to first token are measured requirements. > - This pull request makes the control plane tell the transport when a reader can see a change, and the transport delivers in the next task. The fixed timer is gone. > - The benefit is that an open session with nothing to deliver costs nothing on the main thread, and an event reaches the driver sooner than before. ## Linked Issues or Issue Description No public issue exists. The problem is described here. **What happened?** The server's main thread on a hosted instance stayed at 96% of one core, so every request and every agent turn waited behind it. A 30 second CPU profile of the live server (Node 24, about a dozen live native agent sessions) attributes 5.6% of the main thread to `#pumpEventsSafely` and 4.7% self time to `#pumpEvents` in `packages/paperclip-runner/src/live/runnerd-codex-transport.ts`. Three places started `setInterval(() => this.#pumpEventsSafely(), 5)`. Each pass scans the session's committed-event window, so the cost is `sessions x 200 passes per second x window size`, with nothing to deliver. **Expected behavior** An open session that has no event to deliver does no periodic work. An event that arrives is delivered at once, not at the next timer tick. **Steps to reproduce** 1. `cd packages/paperclip-runner` 2. `PAPERCLIP_TRANSPORT_PUMP_BENCHMARK=1 pnpm exec vitest run src/live/runnerd-transport-pump.benchmark.test.ts` 3. Read the `TRANSPORT_PUMP_BENCHMARK` lines. `stateReadsPerSessionPerSecond` counts how often an open, idle transport reads the control plane. `deliveryWaitMs` is the time from the moment a reader can see a committed event to the moment the transport delivers it. **Paperclip version or commit** Present on master at `c8ae02e5ce`. The profile is from `556b249897`. **Deployment mode** Hosted server with native agent sessions. The same code runs in local development. ## What Changed - `DurablePrpControlPlane` has a new `onReadableStateChange(listener)`. It fires when the state that a reader of `store` can see may have changed. The control plane calls it after every stored-state replacement or save, after a saved checkpoint releases events that were waiting for it, and after an authenticated runner moves the recovered cursor. - The transport subscribes when it creates its control plane. A change wakes one delivery pass in the next task (`setImmediate`). The pass never runs inside the control plane's own commit, and several changes in one task share one pass. This keeps the existing order: the control plane finishes its commit before a reader reacts. - The three `setInterval(..., 5)` timers are gone. `#startPump` and `#stopPump` replace them and keep the same lifecycle: the pump runs from session start, and it stops on release, controller retirement, failure, and close. - The delivery pass now reports what it left behind. Events that the transport holds back itself (a pending turn start, a pending recovery binding, a trace write that must be retried) are retried every 5 ms, as before, and only while they are held. This is the only timer that remains. - A pass that stops after it exposes `turn/started` (so the driver can bind the turn first) now continues in the next task. Before, the events after it waited for the next 5 ms tick. - `attachRun` wakes the pump when it moves the transport to the new run identity. - New regression test `src/live/runnerd-codex-transport-pump.test.ts`, new opt-in benchmark `src/live/runnerd-transport-pump.benchmark.test.ts`, a paced provider fixture, and a test-only probe that records when each committed event becomes readable. How this meets `rules/sleep-aware-background-work.mdc`: | Item | Answer | | --- | --- | | Trigger | The control plane's readable-state change. No timer. | | Source of truth | The control plane's committed-event window and the transport's delivery cursor. A notification is a hint to look at them. | | Pending-work predicate | The pass returns `held` or `next_task`, or a trace write is waiting. | | Next deadline or retry | 5 ms, only while the predicate is true. `next_task` uses one `setImmediate`. | | Quiet condition | Nothing is held. Then the transport has no timer and no scheduled task. Release, retirement, failure, and close stop it completely. | ## Verification **Reproduction and before/after measurement** Run from `packages/paperclip-runner`: ```sh PAPERCLIP_TRANSPORT_PUMP_BENCHMARK=1 pnpm exec vitest run src/live/runnerd-transport-pump.benchmark.test.ts ``` The benchmark opens 12 real transports against `runnerd` and a credential-free provider fixture. It measures the open sessions while nothing arrives. Then all 12 sessions stream one turn at the same time (40 events each, one every 15 ms). "Before" is the unchanged source at `e2cf9a670f`, which was the base of this branch when the measurements were taken. "After" is this change on that base. Each cell shows two runs, and the runs alternate between before and after. The host was a shared macOS arm64 machine with other work running (load average 8 to 10), so absolute numbers carry that noise. External checkpoint persistence. This is the mode that the server uses. | Measure | Before | After | | --- | --- | --- | | Idle: control-plane state reads per session per second | 364 / 365 | 0 / 0 | | Idle: event-loop utilization, 12 new sessions | 1.5% / 1.5% | 0.0% / 0.0% | | Idle: event-loop utilization, 49 retained events per session | 2.1% / 2.2% | 0.0% / 0.0% | | Idle: process CPU, 49 retained events per session | 12.8 / 11.8 ms per second | 0.05 / 0.04 ms per second | | Delivery wait, mean (480 events) | 3.48 / 4.22 ms | 1.91 / 1.81 ms | | Delivery wait, p50 | 3.16 / 3.97 ms | 0.83 / 0.58 ms | | Delivery wait, p95 | 7.78 / 8.49 ms | 6.36 / 7.47 ms | | Delivery wait, max | 8.84 / 9.23 ms | 9.07 / 8.86 ms | | Delivery wait of the turn's terminal event, p50 | 2.37 / 2.58 ms | 0.08 / 0.05 ms | The idle cost grows with the retained event window, as the profile predicts. With `PAPERCLIP_TRANSPORT_PUMP_DELTAS=400` (409 retained events per session, one run each): idle event-loop utilization 3.4% before and 0.0% after, idle process CPU 21.5 ms per second before and 0.08 after. Delivery wait in that run: mean 5.93 ms before and 3.55 ms after, p95 16.27 ms before and 8.03 ms after, max 66.94 ms before and 14.04 ms after. Delivery wait is the time from the moment a reader can see a committed event to the moment the transport hands it to the driver. It is equal or better at every point. The p95 and the max did not move much. They belong to the first events of each turn, which wait for the turn start response. That wait is a separate 10 ms poll (`waitForRunnerCommand`), and this pull request does not change it. File persistence (the local journal, which writes and syncs on the main thread): | Measure | Before | After | | --- | --- | --- | | Idle: state reads per session per second | 363 / 364 | 0 / 0 | | Delivery wait, 12 sessions, mean | 10.33 / 13.76 ms | 11.88 / 12.73 ms | | Delivery wait, 12 sessions, p50 | 6.04 / 6.87 ms | 6.34 / 6.20 ms | | Delivery wait, 4 sessions and 400 events each, mean | 4.75 ms | 2.39 ms | | Delivery wait, 4 sessions and 400 events each, p50 / p95 | 0.07 / 22.99 ms | 0.06 / 11.80 ms | With 12 sessions the file journal saturates the main thread with its own synchronous writes, and the wait is the time behind those writes. Three more before-runs of that case gave means of 9.0, 14.0 and 20.2 ms, so the two columns are the same within the noise. With 4 sessions the journal does not saturate and the after column is better. **Regression test (red, then green)** ```sh pnpm exec vitest run src/live/runnerd-codex-transport-pump.test.ts ``` - `stops looking for events while an open session has none to deliver` (file and external persistence). On the unchanged source: `expected 708 to be less than 100` and `expected 732 to be less than 100` state reads in a 2 second idle window. With the fix: pass. - `delivers each committed event in the next task, without a timer tick` (file and external persistence). The test freezes every interval timer, runs a full turn, and checks that each event is delivered before a task that the test queues at the moment the event becomes readable. This is an order check and does not depend on the speed of the host. On the unchanged source: `the turn's events were delivered without a periodic poll: expected false to be true`. With the fix: pass. - Result: 4 of 4 failed on the unchanged source, 4 of 4 passed three times with the fix, and 4 of 4 passed again after the rebase. **Other local checks** - `node ./node_modules/typescript/bin/tsc -p tsconfig.json --noEmit` and `-p tsconfig.test.json --noEmit` in `packages/paperclip-runner`: pass on the rebased commit. - `pnpm exec vitest run src/live/runnerd-codex-transport` (15 files, 455 tests). These suites cover attachment, recovery, warm transition, suspension, and release and resume on real `runnerd` processes. Before the rebase, on a quieter host: all passed. On the rebased commit, at a host load average of 15 to 20: 454 passed and 1 failed. The failure is the terminal-replay row that is described below. - `pnpm exec vitest run src/live src/control-plane src/native-session-runtime.test.ts src/backends src/drivers/dot/runnerd-dot-driver.test.ts src/drivers/muse/runnerd-muse-driver.test.ts src/drivers/codex/codex-protocol-integrity.test.ts --maxWorkers=3` (81 files): 1566 passed, 32 failed, 6 skipped. The failures: - 30 failures in `runnerd-acpx-permission-ordering.test.ts` and `runnerd-acpx-permission-forwarding.test.ts`. Both replace the control plane with a fixture that appended events in place and relied on the 5 ms poll to notice them. The fixture now implements `onReadableStateChange` and notifies its readers when it emits. Both files pass (30 of 30). - 1 failure in `runnerd-codex-transport.test.ts`, `stops a lost runnerd's provider and escaped tool job from its lineage journal (journal=true)`: `runner did not durably suspend before checkpoint`. It passed 3 of 3 when run again alone. - 1 failure in `runnerd-codex-transport-settlement.test.ts`, the terminal-replay row of `settles only retained control authority without starting another provider turn`: `native_cleanup_maintenance_unproven`, with events still in the runner outbox. It failed again in 2 runs of that test group and in the rebased run above, each time while the host load average was 11 to 20. At a load average near 8 it passed 3 of 3 alone and 1 of 1 as a group with the fix. The unchanged master source (`c8ae02e5ce`) gives the same result: it passed 2 of 2 alone and 1 of 1 as a group at a load average near 8, and the same row failed with the same error (`native_cleanup_maintenance_unproven`, 27 events still in the runner outbox) in a group run at a load average of 11 to 15. So this row is sensitive to host load with or without this change. The failing step is the retained-session maintenance path. That path builds its own control plane, has no transport pump, has no listener on the new notification, and has a fixed 30 second deadline. - `pnpm verify` returns exit 2 (broader verification required, because of the new fixture file). CI supplies the broad coverage. - Not run locally: the server test suites, the repository-wide typecheck, and the build. CI ran them on `4bce83f50a`: 59 checks passed and 4 were skipped (Storybook deploy, Storybook visual regression, Sandbox provider tests, Select Vitest files). ## Risks - **A change that does not notify.** With no periodic pass, an event is delivered only when something wakes the pump. Every wake comes from one of a small, closed set: the five places where the store replaces or saves its state, the checkpoint publication, the recovered-cursor update, and the transport's own identity and start points. A new code path that changes readable state without one of these would leave an event undelivered until the next change. The regression test runs a full turn with every interval timer frozen, in both persistence modes, so a missing wake on the normal path fails the test. The existing transport suites (attachment, recovery, warm transition, suspension, release and resume) now run with no timer that could hide a missing wake. - **Delivery in the next task, not later.** An event now reaches the driver up to 5 ms earlier. The pass still runs in its own task after the commit, and the same gates hold a terminal event until the turn start response is released. The order of events does not change. - **No protocol, schema, Rust runner, or provider pack change.** The contract fingerprint inputs are untouched. Saved state from an older release resumes the same way. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Fable 5.2 (`claude-fable-5-2`), Anthropic, in Claude Code. Tool use and local code execution. ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>