mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-31 14:16:39 +01:00
fix: persist native Runner usage before transport acknowledgement (#18354)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agent execution must preserve usage so Paperclip can price work and enforce budgets. > - Remote Runner execution can outlive its web receiver. > - The receiver previously acknowledged Runner events before its later usage recorder saved the accounting receipt. > - This pull request saves required native usage receipts before that transport acknowledgement. > - A replacement receiver can accept the same retained report and create one ledger charge. ## Linked Issues or Issue Description **What happened?** Native remote Runner usage could leave its durable outbox before the web receiver saved its accounting receipt. The later recorder required a local receipt directory. A replacement web receiver could lose the only remaining report when the database write failed. **Expected behavior** The Runner retains unaccepted structured usage. Postgres owns accepted receipts. Repeated delivery preserves one receipt for each source report and one charge for the run. Missing usage keeps accounting pending. **Steps to reproduce** 1. Start a supported built-in native remote Runner. 2. Make the receipt database insert fail after the provider reports usage. 3. Replace the web receiver and its local directory. 4. Reconnect the same Runner and inspect the saved receipt and ledger. **Paperclip version or commit** The initial investigation used `ebd05e9f4c88c66af7473db86bbaacfd1544aa0a`. **Deployment mode** A web server with a remote native Runner and shared Postgres. Related work: #18327 owns output logs and provider traces. This change has no dependency on its S3 export path. #18031 covers interrupted Codex receipt recovery. #18005 covers legacy spool reads. #17516 covers Codex stop-boundary retention. ## What Changed - Add native producer receipt delivery through the existing Runner callback before transport acknowledgement. - Reuse `run_usage_receipts`, stable source sequences, and deterministic report identities. Reject conflicting replay and retry a lost commit response with the same receipt. - Restore per-turn usage from structured receipt facts. Preserve provider turns that share one PRP envelope turn. - Bind retained warm transport callbacks to the current run authority epoch, allowing old attachment control acknowledgements while preserving accounting ownership. - Persist Codex session baselines and cumulative totals in the existing provider checkpoint. Mark qualified whole-run deltas, preserve them across resume/restart/attachment, and leave old unmarked reports incomplete and unpriced. - Normalize the bounded final usage tail before the terminal event. Retain later increases for explicit reconciliation, block further attachment, and fail an affected successor without changing its baseline. - Complete runs from the accepted immutable receipt and its exact saved rate-card amount; preserve existing live-budget enforcement. - Skip the local receipt recorder for supported built-in native remote Runner execution. Keep local, legacy, and external Dot/Muse behavior unchanged. - Keep pricing, reservations, ledger settlement, source fencing, and budget holds with their existing owners. - Put the actual Runner integration test in the existing Rust-cached native test lane. Build before opening Postgres and give cold compilation its own bounded budget. ## Verification - Fresh Greptile review is 5/5 on `e7376a50c8162770da1d581ecf25c8cffac64520`, with no unresolved findings. Parent and independent peer review approved the final changes. - Real PostgreSQL 18.6 and a real Rust Runner exercise failed receipt insertion, receiver-directory loss, replay from the same producer, and exactly one 25-cent ledger charge. The actual accounting spool path stays absent. Optional logs/traces are disabled. The provider is controlled; no paid request is made and the fixture never pre-saves the expected receipt. - Producer checks passed 32 usage/provider tests,37 terminal controls, corrected completion, all 3 previously failing exit cases, and both full transport goal-continuation cases. Rejected active-goal completion retains its 120-token tail before failure. Baseline 100→successor150→33 duplicate old snapshots→old120 retains explicit incomplete reconciliation evidence and positive budget exposure. - Heartbeat executor checks passed 982/982. PostgreSQL tests preserve exact 1.002 USD pricing, the 100.2-cent ledger amount, budget holds, source takeover, partial/final receipts, cost-only corrections, equal rate-card replay, and stale-completion interleaving. Newer incomplete usage remains pending rather than settling an older snapshot. - Runtime replay and strict usage-schema checks passed 2/2; generated contracts and Rust formatting pass. Test-partition contracts passed 32/32. - [Final combined-head evidence](https://github.com/paperclipai/paperclip/pull/18354#issuecomment-6531526158) records full workspace typecheck/build and 41/41 accounting tests against all five cloud PR heads, including this final accounting commit. It includes commands and evidence limits. - Exact-head CI is green: 53successful checks, 6intentional skips, and the legacy Snyk status passed. Typecheck, build, Runner, server, browser, and aggregate gates pass. Runner Vitest1/2 initially failed an unchanged mocked Antigravity login fixture; its focused case passed locally, then one unchanged job retry passed 131 files/1995 tests, including all 11 Antigravity tests. The initial failed attempt remains recorded in CI. Initial local attempts exposed the host SysV semaphore limit and corrected harness path, IPC readiness, and queue-counter defects. A temporary test loader selected an existing POSIX-semaphore PostgreSQL 18.6 build without changing repository dependencies. An optional expanded debug provider run was interrupted at its unrelated 8192-turn rollover stress case; no full-file local pass is claimed. Required failure cases were run directly, and the full Runner CI remains required. ## Risks - The guarantee covers built-in native remote Runner execution. Local native execution, legacy adapters, external Dot/Muse execution, and server-originated model calls retain their existing contracts. - This does not provide traffic switching or general web-server failover. The existing controller identity, checkpoint, and generation gates still apply. - The receipt JSON gains optional structured producer facts. No schema migration or accounting S3 spool is added. Text output and raw trace payloads are excluded. - Codex accounting authority requires a Runner artifact built from this provider baseline implementation and qualified by digest. Update receivers and strict usage-schema readers before releasing that Runner; PRP does not negotiate this payload field. Existing unmarked Codex reports remain pending/unpriced. No deployed Runner artifact is claimed fixed. - Usage arriving beyond the bounded terminal tail may require manual reconciliation or a fresh session. Its evidence remains in the existing producer checkpoint/outbox; no adjustment ledger or recurring scan is added. ## Model Used OpenAI `gpt-6.1-sol` assisted with the implementation, tests, and documentation through code execution and repository tools. The context window size is unavailable. ## 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>
This commit is contained in:
1 parent
dbff3747b8
commit
9f524ec1a7
29 files changed
+2391
-313
No files matched your search
@@ -320,6 +320,7 @@ test("the real serialized shard partition is complete, non-overlapping and durat
|
||||
const chatSuitePath = "server/src/__tests__/chat-channels.integration.test.ts";
|
||||
const nativeRunnerSuitePath =
|
||||
"server/src/services/native-runtime/native-codex-runner.integration.test.ts";
|
||||
const usageDeliverySuitePath = "server/src/services/native-runtime/native-usage-delivery.integration.test.ts";
|
||||
const dotRunnerSuitePath = "server/src/__tests__/dot-runner.test.ts";
|
||||
const restartRecoverySuitePath =
|
||||
"server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts";
|
||||
@@ -363,7 +364,8 @@ test("12 PR without-chat shards plus the dedicated chat and native-runner lanes
|
||||
assert.ok(!files.includes(nativeRunnerSuitePath));
|
||||
assert.ok(!files.includes(dotRunnerSuitePath));
|
||||
assert.ok(!files.includes(restartRecoverySuitePath));
|
||||
assert.deepEqual([...files, chatSuitePath, nativeRunnerSuitePath, dotRunnerSuitePath, restartRecoverySuitePath, "server/src/services/native-runtime/plugin-runner-probe.test.ts"].sort(), full.selectedGeneralServerSuites.sort());
|
||||
assert.ok(!files.includes(usageDeliverySuitePath));
|
||||
assert.deepEqual([...files, chatSuitePath, nativeRunnerSuitePath, dotRunnerSuitePath, restartRecoverySuitePath, usageDeliverySuitePath, "server/src/services/native-runtime/plugin-runner-probe.test.ts"].sort(), full.selectedGeneralServerSuites.sort());
|
||||
assert.equal(new Set(files).size, files.length);
|
||||
const defaultRun = dryRunJson([], prEnv);
|
||||
assert.ok(defaultRun.generalServerSuiteCount === full.generalServerSuiteCount);
|
||||
@@ -412,6 +414,7 @@ test("the native-runner lane runs exactly the cargo-dependent suites", () => {
|
||||
"server/src/services/native-runtime/native-codex-runner.integration.test.ts",
|
||||
dotRunnerSuitePath,
|
||||
restartRecoverySuitePath,
|
||||
usageDeliverySuitePath,
|
||||
]);
|
||||
});
|
||||
|
||||
|
||||
@@ -92,6 +92,7 @@ const nativeRunnerSuites = [
|
||||
"server/src/__tests__/dot-runner.test.ts",
|
||||
// These process cases otherwise skip in server shards without Runner binaries.
|
||||
"server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts",
|
||||
"server/src/services/native-runtime/native-usage-delivery.integration.test.ts",
|
||||
];
|
||||
// In the PR workflow (pr.yml, the caller of pr-trusted.yml — reusable
|
||||
// workflows inherit the caller's GITHUB_WORKFLOW), the last Verify Paperclip
|
||||
|
||||
Reference in new issue
Block a user