mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 16:02:36 +01:00
Dotta
65aee998a3
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Environment workers retain cleanup duties and retry them safely. > - A metadata change can notify the worker without adding cleanup debt. > - The pending-cleanup worker kept a work hold during its thirty-second spacing window even when its owner reported no debt. > - Historical cleanup already checks that condition without advancing a provider sweep. > - This PR applies the same owner check to pending cleanup and preserves later notifications. > - An empty notification can settle while real cleanup and uncertain work still block sleep. ## Linked Issues or Issue Description Refs #18646. That change covers empty historical-cleanup notifications. This follow-up covers the pending-cleanup queue. Related runtime work: #18988. A search of open and closed PRs and issues found no duplicate pending-queue fix. This repairs an existing worker under the roadmap's Cloud deployment work. **What happened?** A normal metadata update on a released lease left one tracked work intent and a scheduled pass. The actual cleanup deadline was null before and after the update. The worker kept the hold until its spacing window ended. **Expected behavior** The worker can clear an empty notification through its existing owner predicate after a normal sweep and deadline read complete. It must retain new debt, unresolved transactions, failed sweeps, failed reads and later notifications. Provider sweep spacing and cleanup retry deadlines must stay unchanged. **Steps to reproduce** 1. Create and release a fixture cleanup lease through the normal services. 2. Start the registered worker and confirm that its cleanup deadline is null. 3. Change the released lease's metadata before the thirty-second spacing window ends. 4. Run the new test. The original worker keeps an active intent even though eligibility remains empty. **Paperclip version or commit** Candidate: `69aaee95cd73322a37e0270354d4a06bf5512ba7`. It normally merges actual master `cd3832d2357eab481a861331ba1bc944a0b951f4`. The focused retry repair follows the original two-file fix. Every other tree blob still matches that master. The diff has two files. **Deployment mode** Source checkout with external PostgreSQL 17 and controlled coordinator time. No deployed host or database suspension result is claimed. ## What Changed - Apply the existing cheap deadline check to both registered cleanup queues during provider spacing. - Clear only the captured notification generation when the owner returns no deadline and the latest normal sweep completed successfully. - Invalidate that coverage before every full sweep. Startup and later sweep failures keep their existing retry even if a metadata notification has no database deadline. - Add real PostgreSQL controls for empty metadata changes, concurrent debt, unresolved transactions, failed reads through drain/resume, and unread-spool recovery after startup and later sweep failures. - Retain the original provider sweep, retry, deferred-buffer, creation, startup, historical and shutdown controls. ## Verification **Real PostgreSQL red/green qualification passed.** The original empty-notification counterfactual failed at active work one, expected zero, after actual null eligibility reads (16.706s). Review then found a missing failed-sweep combination. Two additional cases using the real malformed spool and ordinary metadata notifications reproduced lost retries on the earlier published head, both at the intended pending predicate (42.144s). One case fails at startup; the other first completes a successful sweep, then fails a later sweep. Failed fixtures and original logs remain retained. At measured source `1ccd227c51e03ab4306d7cb85cf81efb0e3d3678`, all **29 owner cases passed in 113.061s**, without filters or skips. The tests repair the actual spool, recover the ordinary cleanup lease, perform provider cleanup once, preserve the later-sweep backoff, and return to quiet. Normal migrations used a fresh owned external database; each pool was capped at four with one worker. The first test-fixture attempt accidentally replaced the real spool driver; that setup failure is preserved separately and was corrected before the counterfactual. No final cleanup result or receipt was seeded. Run the full owner suite against a fresh, owned external test database with normal migrations applied: ```sh pnpm --filter @paperclipai/server exec vitest run src/__tests__/environment-cleanup-work.test.ts --maxWorkers=1 --no-file-parallelism ``` Set `PAPERCLIP_ENVIRONMENT_CLEANUP_TEST_DATABASE_URL` to that fixture. Local qualification uses one worker and a pool of four. Current native TypeScript checks passed: normal production declaration layers (44.127s) and the affected test entry using the repository's actual CI options and references (3.922s). Own dependency builds passed. A frozen install first found stale upstream ACPX patch metadata; normally carrying #19294 corrected the hash references for the same package version. No lock policy was relaxed. Full local workspace typecheck, tests and build were not run in this bounded external-PG lease. Fresh checks on the current head `69aaee95cd73322a37e0270354d4a06bf5512ba7` completed with **53 successes, one neutral Pony review and two skips**, with no pending or failed gates. Its [PR workflow](https://github.com/paperclipai/paperclip/actions/runs/42104787089), aggregate verification and commitperclip review succeeded. The complete latest edited Greptile and Pony summaries both gave **5/5**. Both review threads are resolved, and all root and nested thread pages were inspected. The current head only removes four changed code-comment lines to follow CONTRIBUTING.md. Its emitted JavaScript is byte-identical to the measured parent `1ccd227c51e03ab4306d7cb85cf81efb0e3d3678`; tests, configuration and dependencies are unchanged. The local 29-case evidence therefore carries to this head without a duplicate local run. The remote results above are fresh for the current head. On the earlier head, Runner CI failed an omitted-context checkpoint-settlement case. The fresh Runner job on this head passed without a rerun. The original failure logs are preserved and its cause remains unproven; current success is recovery evidence, not a causal explanation. The recorded test and migration command PIDs were reaped; an independent audit verifies zero owned database clients before normal cleanup. Failed fixtures remain retained. The process audit covers recorded command PIDs, not an exhaustive history of child processes. The long quiet assertion uses controlled coordinator time and actual SQL counters. It does not prove a real twenty-minute quiet window, provider behavior or deployed suspension. ## Risks The empty read must not erase a notification that arrives while it runs. The captured generation and transaction controls preserve that boundary. An unknown or failed read keeps work pending. Empty deadline rows cannot cover a failed sweep or an unread local spool; only a successful normal sweep and deadline read establish that coverage. No cleanup row, resource claim, provider action, authorization rule, sweep spacing or retry deadline is removed. ## Model Used OpenAI `gpt-6.1-sol` produced the delegated regression tests and source review. OpenAI Codex, based on GPT-6, implemented the small worker change and managed integration and verification. Both used repository tools and code execution. Exact served backend revisions and context limits were not exposed. ## 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 linked an existing public issue or described the issue using the issue template - [x] I have not referenced internal or instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal issue ID - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have checked that no documentation change is needed for this internal worker fix - [x] I have considered and documented risks - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open findings - [x] I will address all reviewer comments before requesting merge