mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 01:57:31 +01:00
Nicky LeachandPaperclip
a6f786479e
## Thinking Path > - Paperclip coordinates AI agents and their execution. > - Hosted Runners can retain a provider process between turns. > - The host must prove that process is idle before allowing its VM to sleep. > - A completed turn alone does not rule out an active goal or trailing provider work. > - This PR adds a conservative park-readiness hook for the existing Codex process. > - The later host-control consumer can use this proof with its identity and sequence fences. ## Linked Issues or Issue Description **Current behavior** There is no provider eligibility hook for host parking. The earlier implementation in #19060 also missed an active goal that arrived during its tail drain. **Proposed behavior** Default every executor to ineligible. Permit the surviving native Codex process only when its completed-turn authority still matches and it has no live or unsettled work. **Reason and benefit** Keep active goals and unresolved provider obligations awake. Review the provider proof separately from socket handling and the native park/reclaim loop. ## What Changed - Add `host_park_ready` with a default false result. - Refresh the configured credential observer before checking pending delivery. Refuse parking during credential delivery, without selecting or restoring a provider. - Verify the current Codex process and completed-turn identity; reject startup, active turns/goals/tools, and unsettled event queues. - Require completion actually observed by that process. Accepted-finish reconciliation still preserves ordinary result authority without qualifying a restored process to park. - Refuse known helper lineage, whose diagnostics do not prove that helper work has finished. - Drain bounded trailing output through the existing accounting path. Retain goal updates and clears in durable state and normal event delivery, then recheck live goal state. - Add process tests for normal eligibility and the late-goal race. Assert that a restored process is ineligible until it completes an authoritative turn in its new generation. This PR is based on master, which contains the merged Codex (#19132) and ACPX (#19153) recovery foundations. The private socket codec in #19129 is merged; #19140 consumes this proof in the shared native loop. This PR does not advertise parking support. A separate ACPX readiness change must prove its live sidecar/session has no active or unsettled work. Both Codex App Server and qualified ACPX are required in the remaining park/reclaim, Core restart, eviction, and live sleep/wake acceptance phases. ## Verification - `cargo test --offline --locked --manifest-path packages/paperclip-runner/runner/Cargo.toml -p paperclip-runner-core --test codex_provider hosted_park` — 5 passed. - `ordinary_recovery_restores_a_completed_idle_session` passes and asserts that the genuinely restored provider cannot park before it completes an authoritative turn in its new generation. The fixture proves two provider starts and only one admitted turn. - The initial live-goal recheck mutation failed as expected. Review then identified that goal changes also needed durable retention. The new activation and clear cases verify saved goal state, retained events, and eligibility after delivery. - Existing goal, active-goal attachment, warm-tail, and late-accounting process regressions passed after sharing the goal event path. - The initial late-goal fixture timed out because it omitted `--durable-turn-ids`, which enables its existing first-turn gate. Added that fixture switch; both park tests then passed. - The credential-observation regression fails before the fix and passes afterward. Three focused credential cases prove durable replay and acknowledgement, fresh ACPX startup, and readiness without provider selection. The three provider parking checks also pass after the fix. - Full regression testing is left to GitHub CI. - Changed lines scanned for secrets, personal data, and private deployment references before push. Current head `28d41b083f58ee93db4305b1217719796bf3171d` adds one scoped review-fix commit to readiness `e8ba82374e0f72c82ac3195d4b1b3798c9072bab`, based on master `3d0d178ff7051aa44e07cd873fd4281357e0d815` without duplicating either recovery foundation. The PR remains eight parking-specific files. Review identified quiet helper work and reconstructed accepted-finish authority that could incorrectly qualify. The provider now records its actually observed terminal authority separately in memory, and parking refuses known helper lineage in both the saved state and the live inventory after draining. No durable field, lifecycle, public API, or ordinary recovery policy changed. Both process regressions fail when the parking guards are removed and pass when restored. Seventeen focused checks pass, including ordinary recovery, helper requests, receipt replay, credentials, and ACPX ineligibility. Rustfmt, diff and added-line scans passed. All exact-head CI is green: [PR run](https://github.com/paperclipai/paperclip/actions/runs/42087773928), [Runner Windows](https://github.com/paperclipai/paperclip/actions/runs/42087773720), and [Docker Runner](https://github.com/paperclipai/paperclip/actions/runs/42087773712) passed; Storybook was skipped. Standard review 15 scored 5/5 before the authorized follow-up [Apex review](https://github.com/paperclipai/paperclip/runs/127471966630). Apex review 16 also scored [5/5 on this exact head](https://github.com/paperclipai/paperclip/pull/19133#issuecomment-6645371152), with zero new comments and zero unresolved threads. On this current head, the first server shard 11/12 [job 127458190155](https://github.com/paperclipai/paperclip/actions/runs/42087773928/job/127458190155) failed the unchanged decision-model concurrency test: after releasing the database lock, its `waitFor` observed one provider call instead of two. The identical file passed all 37 tests on the prior reviewed head. One targeted unchanged-head retry [job 127466020677](https://github.com/paperclipai/paperclip/actions/runs/42087773928/job/127466020677) passed all 1,779 tests, including those 37 decision-model cases; 13 tests were skipped. No code, test timeout, or CI configuration changed between attempts. The original failure's cause remains unestablished. On prior reviewed head `e8ba82374e0f72c82ac3195d4b1b3798c9072bab`, the first `General tests (workspaces-c)` job [127389771745](https://github.com/paperclipai/paperclip/actions/runs/42068371976/job/127389771745) failed the unchanged database SIGTERM test at `packages/db/src/test-postgres-command.test.ts:44`: `JSON.parse` read empty readiness-file content immediately after an existence check. The fixture writes directly to that path, leaving a create-before-content race. One targeted unchanged-head retry [127398676711](https://github.com/paperclipai/paperclip/actions/runs/42068371976/job/127398676711) passed all **313 tests in 72 files**, including SIGTERM and SIGINT. No source or timeout changed between attempts. On the superseded `ca088abd552b` head, one Cloud sign-in recovery case returned to the expected task URL but missed its heading within the five-second assertion window; the other 40 cases passed. One targeted unchanged-head retry (job 127136265280) passed all **41 cases**. No test timeout or application code changed between attempts, and the first failure's cause remains unestablished. The superseded c7 parent failed eight retained-cleanup settlement cases because prepared restoration started a provider during a drain-only epoch. The corrected parent preserves ordinary prepared behavior; the new completed-source path excludes prepared providers. The parent reports all 19 settlement rows passing. This PR adds no prepared-session proof or restoration policy. ## Risks Known helpers remain conservatively ineligible even after their progress diagnostics: this slice has no authoritative helper-completion ledger. The eligibility check drains the existing bounded provider tail and can persist late accounting. That accounting must settle before parking succeeds. This hook alone does not prove VM suspension or vendor goal behavior across a process restart. Live sleep/wake qualification remains a later rollout check. ## Model Used OpenAI GPT-6.1 Sol (`gpt-6.1-sol`), High reasoning, through Codex with tool use and 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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing>