Files
PaperClipAI/server/src/services
DottaandPaperclip b891e69bd2 fix(native): correct unread wake comments before completion (#18937)
## Thinking Path

> - Paperclip manages AI agents and their work.
> - Native agents report completion through a server-owned tool
boundary.
> - A truncated external-chat request requires a complete run-bound
comment read.
> - The terminal guard checks that receipt after the provider ends its
turn.
> - Early completion feedback omitted the same check and could accept an
unread report.
> - The provider then lost its chance to correct the report in the same
turn.
> - This pull request returns the existing read requirement through the
completion tool before accepting the report.
> - The final guard still protects the exact source and snapshot after
provider settlement.

## Linked Issues or Issue Description

Related: #16995 keeps run-scoped readers available to retained provider
threads. #18913 preserves queued comment input. This change fixes a
separate completion-feedback boundary.

**What happened?**

A native external-chat run could submit an accepted completion without
reading all server-bound comments. The terminal guard then failed the
run with `native_current_wake_comments_unread`. Early feedback also
accepted partial, changed and foreign read receipts. The agent did not
receive the required correction while it could still act in the same
turn.

**Expected behavior**

The completion tool must reject the report with a
`read_current_wake_comments` instruction and the cursor requirements. A
complete current read must permit the corrected report in the same turn.
The final snapshot check must remain effective.

**Steps to reproduce**

1. Create a running native task with a truncated external-chat wake and
exact accepted comment IDs.
2. Call completion feedback before reading the comments, or after only a
partial or stale read.
3. Observe that the old code returns an accepted report instead of the
read correction.

## What Changed

- Check the existing run, company, task and agent binding before every
completion-feedback acceptance return.
- Reuse the authoritative current-wake reader and terminal receipt
validator.
- Share the two unchanged read-error codes from their owner across the
validator, feedback and executor classification.
- Convert only the two owned unread or changed-snapshot errors into a
bounded tool correction. Other errors still propagate.
- Keep the final validator, receipt parser, result persistence and
admission policy unchanged.
- Add real PostgreSQL regressions and an actual OpenCode MCP tool
sequence with a controlled local provider process.
- Document the early correction and final snapshot fence.

## Verification

The source checkout uses Node 24.22.2 on macOS arm64 with a fresh owned
embedded PostgreSQL database.

Final published tip: `37bfb269432f86f98cab1a58c09b5519ed9477fd`. Its
production code is commit `8c7297318c3a841fc1c72c846acfaabf58adabe9`;
the last commit adapts three existing module mocks. The expanded
affected group passes **1,284 cases** with **35 existing opt-in skips**.
The skips require captured private-runner transition fixtures and a
configured fresh database, which were not supplied. All nine real
PostgreSQL/MCP cases execute and pass. Recursive typecheck and the full
build pass for production commit `8c7297318c`; the final changed test
layer also passes.

- Untouched master `1a39dea9aa2bb9bea416439b0a574c788e88b9a0`: four
causal failures. Unread, partial, changed and foreign complete receipts
incorrectly receive accepted completion feedback. The ordinary
inline-complete control passes.
- Current source: all nine new cases pass. The actual authorized reader
returns the full accepted comment. The same running task and run remain
intact, with no replacement or accepted-result replay.
- The OpenCode native backend sends a real MCP finish rejection to the
controlled provider process. The process follows the returned reader
instruction, reads every page, and resubmits finish. One provider turn
produces one semantic result proposal. The terminal snapshot fence still
rejects a later comment edit.
- All 98 affected OpenCode driver and MCP bridge cases pass with the
fixture-mode addition.
- The first tool-sequence attempt failed on a fixture-only relative path
before starting the provider. The test now uses its module URL. This
setup failure is separate from the four causal failures.
- The first recursive typecheck found missing local workspace links in
the clone's excluded sandbox-provider directories. Existing dependency
links were copied into the isolated clone. No lockfile or source change
was made for this setup repair.
- At `c57e532a87`, all 1255 cases in the six-file affected server
family, recursive Node 24 typecheck and the full build pass. At
production commit `1ae9f54107e1762bc8222572b365a013d4572d1a`, all 1,257
affected cases, recursive Node 24 typecheck and the full build pass.
- Greptile found that the initial correction could restart a partial
read after page two. On `c57e532a87`, following that correction fails
with the actual reader cursor-order error. The follow-up continues the
caller's most recent cursor and retains the changed-snapshot restart
path. All nine new cases pass, including this regression, a
stale-partial control, the foreign receipt refusal and the actual
same-turn MCP sequence.
- [Linux server3/12
job](https://github.com/paperclipai/paperclip/actions/runs/41788671370/job/126437378334)
found five failures in the older format-feedback SQL fixture. Its flat
heartbeat row did not match the real joined projection. The same five
failures reproduced locally. The test-only follow-up returns a coherent
run/agent/task projection and keeps the real resolver and formatter
assertions. All 14 format and real-PG/MCP cases pass, and the generated
server test layer typechecks. Production code and the earlier
1,257-case/typecheck/build proof are unchanged.
- Qualification of the shared-code change found 49 failures because
whole-module mocks omitted the new exports. The three mocks now pass
through actual module exports and keep their controlled reader methods.
Literal error assertions remain independent. The final 1,284-case group
passes with the exact 35 opt-in skips described above. No production
fallback or substitute code was added.
- [One Linux Runner package
attempt](https://github.com/paperclipai/paperclip/actions/runs/41792315273/job/126448500037)
passed 4,913 cases and skipped 10, but its synthetic post-rename
credential durability case failed with ENOENT. The complete Runner
package tree and lockfile are identical to the [earlier Linux
control](https://github.com/paperclipai/paperclip/actions/runs/41790199252/job/126441739681),
where all 42 credential cases passed on Node 24.22.2. The exact case
also passes once on both base and head on macOS with Node 24.22.2. The
original failure cause remains unverified; OS/load and shared-test
timing uncertainty remain. Credential code and assertions are unchanged.
The single affected-job rerun passes, including all 42 credential tests.
The original failure remains unexplained.
- An ordinary full local test attempt was stopped before the review
repair. It has no completion summary and is INCOMPLETE. Current-head
full CI completed; no local full-suite pass is claimed. Greptile
reviewed the final published tip at 5/5 with zero open threads. All
current-tip checks pass: 54 successful check runs, one successful legacy
commit status, 3 skipped and 2 neutral, with no pending or failing
checks. Paperclip also reviewed final tip `37bfb26943` at 5/5 with no
remaining findings. Its prior 4/5 code-ownership recommendation is fixed
and resolved. No review nudge or hosted task retry was issued.

The controlled OpenCode process uses the supported v1 factory envelope
to exercise the common native feedback and MCP boundary. It does not
call a live model or qualify a provider pack. PostgreSQL, authorization,
pagination and receipt checks are real. No hosted task, model,
permission or saved result was changed. Recovery of an already accepted
historical run remains unverified.

## Risks

- Each finish call adds an exact binding query. A mandatory read also
validates the current accepted-comment snapshot through the existing
transaction. Cost grows with those exact comment IDs, bodies and
attachment metadata; it does not scan unrelated task history or add idle
polling.
- A valid read can become stale before the provider settles. The
unchanged terminal guard still rejects that race.
- Missing ownership or unexpected validation errors remain refusals.
This change does not grant replay authority or turn an accepted
historical result into fresh work.

## Model Used

OpenAI Codex, GPT-6 family, with repository inspection, code execution
and GitHub tools. The runtime does not expose the exact serving model ID
or context window. The coordinator performed independent read-only
source review.

## 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 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>
2026-11-04 07:19:12 -06:00
..
…
…
…
…
…