Files
PaperClipAI/server
DottaandPaperclip f211d44c00 fix(heartbeat): preserve queued coalesced comments at claim (#18913)
## Thinking Path

> - Paperclip manages agents and their work.
> - The wake queue records task comments before an agent starts.
> - A queued run can accept another comment through coalescing.
> - Claim rebuilds that run's input from its primary wake.
> - Coalescing updated the run and its receipt but left the primary
batch unchanged.
> - This change saves the accepted batch atomically in the owning
writer.
> - The benefit is complete input at claim without repeating old work.

## Linked Issues or Issue Description

Related: #18887 preserves offline queued turns. #13948 addresses
equivalent automatic wake deduplication. This change repairs the
accepted comment batch at the existing claim boundary.

**What happened?**

A queued run started with comment A. Admission accepted comment B, saved
`[A, B]` in the run context, and finished B's coalesced receipt. The
primary wake still held `[A]`. Actual claim rebuilt input from that wake
and dropped B.

**Expected behavior**

Claim must retain every accepted live comment in order. Stop, changed
source authority, deleted comments, and company boundaries must still
apply. Already claimed wakes must retain their existing recovery source.

**Steps to reproduce**

1. Queue a task run whose primary wake has an authoritative comment
batch `[A]`.
2. Admit comment B through `createHeartbeatQueue.enqueueWakeup` and the
real Postgres admission writer.
3. Confirm that admission accepts `[A, B]` and records B's coalesced
receipt.
4. Claim the original queued run. Before this fix, the claimed input
contains only A.

**Paperclip version or commit**

The untouched producer regression fails on
`d7283e875772ca8c684dc86e471e6b48d543b7b1` with both process and native
Runner/Codex adapter selection. Provider execution is disabled in the
fixture.

## What Changed

- Save the accepted comment batch in the unclaimed primary wake, run
context, and coalesced receipt in one transaction.
- Lock the primary wake before the run. Recheck the expected run status
and same-company, same-agent and run bindings; apply the existing actor
equality fence only to durable receipts before mutation.
- Preserve incoming input as normal deferred work when the locked source
changed. Preserve receipt identity and actor fields.
- Preserve the original primary actor and trigger comment while updating
its accepted batch. Each incoming receipt retains its actual actor. Keep
claimed source payloads and legacy envelopes on their existing recovery
paths.
- Safely defer a same-named different agent without rewriting the
original agent's run, wake or receipt. Missing and foreign-company
targets remain rejected. Do not rebuild input from historical receipts.
- Add actual process/native producer, real database rollback, Stop,
deletion, foreign-source, and fallback controls. Update three direct
writer fixtures for the required expected-status contract.

## Verification

- The untouched process and native producer red on
`d7283e875772ca8c684dc86e471e6b48d543b7b1` loses accepted comment B at
actual claim. The initial incomplete mock is retained separately and
excluded from behavioral proof.
- Current process/native matrix: 22 passed, including mixed authors,
system primary, same-named different agents, changed status, claimed
sources, Stop, deletion and foreign-source boundaries. Queue, Postgres
admission and use-case suites passed 234 cases before the
source-preserving serializer follow-up; its 22 actual producer cases
passed again afterward.
- CI on the previous head found a real ordinary mixed-author
workspace-copyback regression. The earlier 13-case selection omitted
this test; that coverage gap is preserved. The first correction exposed
a second real source-provenance/copyback failure. After preserving the
primary trigger and actor while updating only its batch, the complete
workspace recovery file passed all 188 cases, including ordinary
mixed-author input and native copyback.
- Real HTTP queued-comment consumption, discard/reorder and Stop
boundaries: 101 passed before these review corrections. Existing chat
and Slack consumers each passed their selected real writer case.
- Previous-head CI also timed out in the CLI child-server startup
fixture and a ZIP transfer case. The full untouched transfer file passed
17 cases locally. Startup remains unclassified; its intentionally
reserved port is part of the fixture and is not diagnosed as the cause.
No CI retry was issued.
- An earlier queue run passed its 231 assertions but timed out during
teardown; its preserved replay passed all 60 queue cases. This is
distinct from the behavioral reds.
- Exact `ab59d944895774d89f950248e356e93af00f4832` server type layers
and production server/Runner build passed. The unchanged-default
serializer file passed all 11 cases. Current-head CI and both
substantive reviews remain required; previous grades and checks do not
qualify this head.
- No hosted task was retried or changed. Local tests do not establish
any hosted deferred event's cause, provider execution, native
acknowledgement, physical cleanup, or serving revision.

## Risks

- Admission now performs bounded row reads and locks for each coalesced
wake. It adds no idle scan or remote request. The existing task
transaction owns the operation; lock order follows queued claim.
- Claimed-wake recovery uses source identity. Its payload remains
unchanged, and the existing actual native copyback tests cover that
boundary.
- A source that changes before the lock can defer the new input. This
preserves work instead of consuming it into a stale run. No migration or
historical backfill is required.

## Model Used

OpenAI `gpt-6.1-sol`, with reasoning, repository inspection, shell
execution, and test tools. The runtime does not report the
context-window size. A separate OpenAI coordinator reviewed the source.

## 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
- [ ] All Paperclip CI gates are green
- [ ] 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>
2026-11-03 22:09:58 -06:00
..