mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-10 20:50:08 +02:00
DottaandPaperclip
0ca4b7c4c0
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The heartbeat service admits, executes, and settles agent runs. > - Earlier extractions separated workspace, preparation, state, retry, recovery, queue, and lifecycle handling. > - Cancellation and resource release still live inside the main service. > - These operations share execution ownership and saved-comment continuation rules. > - This pull request moves them into one run-control module and adds boundary tests. > - Reviewers can inspect cancellation and cleanup separately from adapter execution. ## Linked Issues or Issue Description **What existing behavior does this improve?** The structure and testability of heartbeat cancellation and cleanup. **Subsystem affected** server/ — orchestration services. **Current behavior** Stop, pause and budget cancellation, environment lease release, and saved-comment delivery live in `heartbeat.ts`. **Proposed behavior** Keep these operations in `server/src/services/heartbeat/run-control.ts`. Bind their database and existing callbacks with `createHeartbeatRunControl`. **Reason and benefit** This removes 1,115 lines from `heartbeat.ts`, leaving 10,443 lines. The new module has 1,360 lines. Related cancellation and cleanup rules stay together in one file. **Breaking changes** None. Function bodies, service methods, public helpers, status conditions, and side effects keep their behavior. **Additional context** Continues the merged lifecycle extraction in #15672. Related cancellation work: #15212. Searches of open and closed issues and PRs found no duplicate run-control extraction. This does not add a roadmap feature. ## What Changed - Move run cancellation, pause and budget cancellation, lease release, issue-lock release, and saved-comment resumption into `heartbeat/run-control.ts`. - Supply lifecycle, queue, recovery, and cleanup callbacks through an explicit dependency interface. - Keep executor and cancellation maps shared by all service instances. Preserve callback construction order with forwarding functions. - Keep the existing helpers available from `heartbeat.ts`. - Add 15 tests for inert construction, cancellation gates, shared Stop barriers, failed termination, warm-resource retention, pending wake batches, project scope, and stale budget enforcement. - Document the module boundary in `doc/DEVELOPING.md`. ## Verification - Before extraction: **298 tests passed across six existing cancellation and cleanup suites**. - After extraction: **313 tests passed across seven files**, including the 15 new module tests and real PostgreSQL coverage. ```sh pnpm exec vitest run server/src/services/heartbeat/run-control.test.ts server/src/__tests__/heartbeat-native-runner-cancellation.test.ts server/src/__tests__/heartbeat-run-terminalize-before-release.test.ts server/src/services/run-cancellation.test.ts server/src/services/adapter-execution-control.test.ts server/src/services/explicit-native-continuation.test.ts server/src/__tests__/native-cancellation-request.integration.test.ts ``` - Wider queue, recovery, reassignment, comment delivery, and native cancellation verification: **139 tests passed across nine files**, including the final module tests. The recovery suite first hit its 20-second database setup timeout while local builds were running. After removing verified orphaned PostgreSQL headers, its isolated run passed **380 of 381 cases**. One teardown assertion exceeded its one-second wait for a successor to settle; that exact case passed in a filtered rerun. Combined focused coverage is **818 distinct passing tests across 16 files**. The module suite appears in both runs. ```sh pnpm exec vitest run server/src/services/heartbeat/run-control.test.ts server/src/__tests__/heartbeat-comment-wake-batching.test.ts server/src/__tests__/heartbeat-process-recovery.test.ts server/src/__tests__/heartbeat-native-cleanup-admission.test.ts server/src/__tests__/heartbeat-lock-release-on-reassignment.test.ts server/src/services/heartbeat/queue.test.ts server/src/services/heartbeat/recovery.test.ts server/src/services/heartbeat/retries.test.ts server/src/services/heartbeat-stop-metadata.test.ts server/src/services/native-runtime/native-cancellation-request.test.ts ``` ```sh pnpm exec vitest run server/src/__tests__/heartbeat-process-recovery.test.ts pnpm exec vitest run server/src/__tests__/heartbeat-process-recovery.test.ts -t "does not adopt unrelated queued comments for a non-coalescing recipient after Stop" ``` - Full `pnpm -r typecheck` and `pnpm build` passed on head `b26efd5f9de390ef8a675f84980f49f7fb7799c2`. - Full local `pnpm test:run` was started on this head and stopped after the complete CI suite passed. It did not finish and is not counted as a full local pass. - All **54 final-head checks passed**; two optional Storybook checks were skipped. This includes the full general and serialized test matrix, browser tests, typechecks, build, Runner verification, and canary dry run. The complete recovery suite passed in CI. [CI run](https://github.com/paperclipai/paperclip/actions/runs/37945389261). - Greptile completed on head `b26efd5f9de390ef8a675f84980f49f7fb7799c2` with **5/5 and no actionable findings**. There are no review threads or merge conflicts. - Structural comparison confirms all 21 moved function bodies, the cancellation options type, remaining service logic, and all 140 existing public exports are preserved. ## Risks Moving closures can break callback construction order or shared Stop barriers. The factory starts no work and receives the same process-wide maps and existing callbacks. Tests cover concurrent Stop callers and failed termination. Native authority, status predicates, provider receipts, cleanup gates, and budget enforcement keep their original bodies. This PR has no schema or API changes. ## Model Used OpenAI GPT-6 via Codex. The exact serving model ID and context window were not exposed in this session. Used reasoning, repository inspection, shell tools, 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>