mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-12 18:06:11 +02:00
DottaandPaperclip
c27ee928b0
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Heartbeat execution connects provider runs to task state, accounting, and recovery. > - Earlier refactors moved scheduling and lifecycle services into the heartbeat folder. > - The remaining executor still contains result persistence and two large failure handlers. > - These handlers must preserve the outcome chosen by Stop or native recovery. > - This pull request moves them into a typed run-completion module at the same call and catch boundaries. > - Reviewers can now inspect completion separately from provider dispatch and resource cleanup. ## Linked Issues or Issue Description Refs: #15717. Related behavior work: #13857 (agent status convergence). **What existing behavior does this improve?** Maintainability of heartbeat execution and terminal result handling. **Subsystem affected** server/ — orchestration services. **Current behavior** The main heartbeat service has 9,335 lines. Result persistence, accounting, presentation, session updates, and execution/setup failure handling are embedded in `executeRun`. **Proposed behavior** Move those steps into `server/src/services/heartbeat/run-completion.ts`. Keep the existing decisions and ordering. The main service becomes 8,125 lines, a reduction of 1,210 lines. **Reason and benefit** This creates a substantial boundary for the next execution refactor. It also gives terminal handling direct tests without starting a provider. **Breaking changes** None intended. Public heartbeat exports and method signatures remain compatible. ## What Changed - Add `createHeartbeatRunCompletion` with `completeRun`, `failRun`, and `failRunSetup` handlers. - Move terminal-result persistence, accounting, response presentation, retry decisions, and session updates with their failure helpers. - Keep Stop barriers and native ownership/recovery exceptions in the executor. Keep the outer cleanup in its original location. - Pass live output getters and an immediate trace-finalization callback so cleanup sees state even if a later completion step throws. - Add 19 boundary tests, update the existing publication-order source guard to read the extracted module, and document the module in the developer guide. ## Verification - Passed `pnpm -r typecheck` and `pnpm build`. - Structural AST comparison confirms all 74 top-level statements in the three extracted blocks, the moved helper bodies, and all remaining service statements are unchanged after normalizing the explicit boundary parameters. All 140 public exports are preserved. - Passed 304 existing tests across seven adjacent lifecycle, accounting, session, Stop, and presentation suites. After rebase, all 19 new boundary tests and all 12 publication tests pass together. Total focused coverage: 335 passing tests. - Full local `pnpm test:run` was stopped after more than 28 minutes without completing. Additional route-suite diagnosis hit local database startup timeouts. The complete CI run passed on the final head, including all test shards, build, typecheck, browser tests, and canary install checks. - Rebased onto the workspace-repair fix in #15726. Preserve its `defaultBranch` metadata in the moved helper and add a regression test. All 31 completion and publication tests and the server typecheck pass on the rebased head. - Greptile reviewed current head `65e3f8d8ac165dfec4508c467b1b2af8f2510f1a`: 5/5 with no actionable findings. - Final-head [CI run](https://github.com/paperclipai/paperclip/actions/runs/37991844034) passed. All 53 completed check runs succeeded; two optional Storybook checks were skipped. The legacy security status also passed. No unresolved review comments or merge conflicts remain. ## Risks Terminal handling is sensitive to Stop races, late native results, and failures during persistence. The tests exercise these paths with real database status writes, accounting, and task sessions. Native recovery exceptions and resource release stay at their existing boundaries. There are no schema or API changes. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, code editing, and command execution. The session does not expose the exact model variant or context window size. ## 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>