mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-14 08:53:19 +01:00
fix(accounting): close a remote-Runner turn completed across a session restore as a lower bound (#20524)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runs settle their charge once the final usage receipt is complete. > - #20517 fixed one case for the local Runner. If a restart stops the provider before a turn's final usage report, and the restored Runner then reports that turn completed, the turn now closes as a lower bound instead of waiting 10 minutes. > - Remote Runners deliver usage through a separate producer-owned receiver. That receiver dropped `session.resumed`, so the same turn there still waited for the 10-minute closure timeout. > - This pull request applies the #20517 rule to that receiver. > - The benefit is that remote ACPX and OpenCode runs settle promptly with an honest lower-bound label after a mid-turn restore. ## Linked Issues or Issue Description Refs #20517. **What happened?** After #20517, `session.resumed` reached turn accounting only through the local Runner's fallback recorder. The remote producer receiver (`native-usage-delivery.ts`) and the session-authority filter (`isNativeUsageAccountingEvent`) both dropped it. A remote ACPX or OpenCode turn that was open across a session restore, and then reported terminal without a complete report, left the receipt incomplete until the closure timeout. **Expected behavior** Same rule as the local Runner. Such a turn closes as a lower bound right away. A complete report that arrives after the restore is still used, and a turn that starts after the restore stays pending until its own report is complete. **Steps to reproduce** `native-usage-delivery.integration.test.ts > a turn restored mid-turn on a remote Runner > closes the turn as a lower bound, also after the controller restarts between restore and terminal` fails on master: the receipt stays `accountingReceiptReady: false`. ## What Changed - **Receiver (`createNativeUsageReceiver`).** It now accepts `session.resumed` for turn-reporting providers (ACPX, OpenCode) while a turn is open. It journals the restore as a receipt like any other producer event, so turn accounting marks the open turn restored. A replacement controller rebuilds the same state from that journal. - **Restores never fail delivery.** A restore from another run or source, or one with no open turn, is ignored, not rejected. An unacknowledged lifecycle event would otherwise replay without end. - **Session-authority filter.** `acceptAccountingEvent` now uses a new `isNativeUsageDeliveryEvent`: the usage events plus `session.resumed`. A restore for another run is ignored there too. - **Recovery routing is unchanged.** `acceptRecoveryAccountingEvent` and `onRecoveryAccountingEvent` keep the usage-only `isNativeUsageAccountingEvent`, so a source run's restore never requires a proven recovery source. - **Turn accounting** gets a `hasOpenTurn()` helper. - **Docs:** `doc/SPEC-implementation.md` now covers both Runners. ## Verification - `native-usage-delivery.integration.test.ts`, describe `a turn restored mid-turn on a remote Runner` (real PostgreSQL): | Test | On master | Here | |---|---|---| | `closes the turn as a lower bound, also after the controller restarts between restore and terminal`: the restore is journaled by one receiver and the terminal arrives at a rebuilt one; replaying everything again is idempotent | fails | passes | | `keeps the turn's own final report when it arrives after the restore` (guard) | passes | passes | | `keeps a turn started after the restore pending, and journals nothing for a restore with no open turn` (guard) | passes | passes | - `native-session-executor.test.ts > binds retained warm transport usage to the successor run authority` covers routing through the session-authority filter: - a current-run restore reaches the successor's capture; - a draining-run restore reaches the drained capture; - an unrelated-run restore resolves with no capture; - recovery routing ignores a source restore. It fails when the filter is reverted to `isNativeUsageAccountingEvent`. - Related suites pass: `native-usage-delivery.integration` and `native-usage-accounting` (46 tests), and the usage, accounting, receipt and authority cases in `native-session-executor.test.ts` (224 tests). Server typecheck passes. - **The live remote path is unproven.** No Daytona wave ran because the Daytona Runner image has not been rebuilt yet. The local Runner counterpart (#20517) passed restart-chaos L1, seed 20386, 4/4 (`w-20261110T104332-s20386`). ## Risks - **Upgrade safety.** Usage state saved by current master has no journaled restore and rebuilds as before. A receiver that predates this change replays a journaled restore as an ignored event, which is the behaviour on master. A restore that an older controller already acknowledged is not replayed, so that run waits for the closure timeout as it does today. - **One extra receipt per mid-turn restore**, only for ACPX and OpenCode and only while a turn is open. Nothing is added before `turn.start`: a restore with no open turn returns immediately. - **Codex and other providers are unaffected.** They do not use turn accounting. - No schema or persisted-format change. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with shell, file-edit and code-execution tools. ## 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 Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
16c30ac32b
commit
c0593b69f1
6 files changed
+92
-6
No files matched your search
@@ -26,7 +26,7 @@ import { prepareRemoteOpenCodeArtifacts, type OpenCodeLaunchArtifacts } from "./
|
||||
import { rehydratePinnedSkillAssets } from "./runtime-context.js";
|
||||
import { createNativeUsageDelivery } from "./native-usage-delivery.js";
|
||||
import { createNativeRecoveryUsageDelivery } from "./native-recovery-usage-delivery.js";
|
||||
import { createNativeTurnAccounting, NATIVE_LOST_TURN_USAGE_EVIDENCE, nativeUsageCostUsd, nativeUsageBiller, normalizeNativeUsage, usesNativeUsageDelivery, isNativeUsageAccountingEvent } from "./native-usage-accounting.js";
|
||||
import { createNativeTurnAccounting, NATIVE_LOST_TURN_USAGE_EVIDENCE, nativeUsageCostUsd, nativeUsageBiller, normalizeNativeUsage, usesNativeUsageDelivery, isNativeUsageAccountingEvent, isNativeUsageDeliveryEvent } from "./native-usage-accounting.js";
|
||||
import { crashPoint } from "@paperclipai/shared/crash-points";
|
||||
export { nativeUsageCostUsd, nativeUsageBiller, normalizeNativeUsage } from "./native-usage-accounting.js";
|
||||
import { measureRunnerOperation, recordRunnerTimingReceipt } from "./runner-operation-timing.js";
|
||||
@@ -1968,7 +1968,9 @@ class SessionAuthorityEpoch {
|
||||
}
|
||||
|
||||
async acceptAccountingEvent(event: PrpEvent) {
|
||||
if (!isNativeUsageAccountingEvent(event)) return;
|
||||
if (!isNativeUsageDeliveryEvent(event)) return;
|
||||
// A restore carries no usage; it never fails delivery for another run.
|
||||
if (!isNativeUsageAccountingEvent(event) && event.runId !== this.runId && event.runId !== this.#drainingRun?.runId) return;
|
||||
this.#assertCurrent();
|
||||
if (event.runId !== this.runId && event.runId !== this.#drainingRun?.runId) throw new Error("native_usage_run_binding_mismatch");
|
||||
await (event.runId === this.runId ? this.onAccountingEvent : this.#drainingRun?.capture)?.(event);
|
||||
|
||||
Reference in new issue
Block a user