fix(server): schedule automatic completion review retirement (#17680)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Native finalization preserves accepted work and resolves task
status.
> - A historical cleanup removes automatic completion-review cards that
current policy no longer creates.
> - The periodic finalization pass repeats that lookup even after all
matching cards are gone.
> - This PR gives this cleanup its own committed hints and pending-card
retries.
> - The existing owner retains withdrawal receipts and protects genuine
reviews.
> - Idle history stops causing recurring cleanup reads.

## Linked Issues or Issue Description

Refs #17679. This is a separate stacked slice for automatic
completion-review retirement. Obsolete version-change reviews belong to
the parent PR.

**What needs to be improved?**

`reconcileNativeFinalizations` repeatedly discovers old automatic
completion reviews. The discovery helper also converts SQL errors to an
empty result, which would let a new event-driven worker forget failed
reconstruction.

**Why is this improvement important?**

Idle tenants should not query for absent cleanup work. Failed discovery
must preserve the retry, and removing a historical card must preserve
the durable trigger for later evidence reassessment.

**Proposed solution**

Use the existing card, decision, effect, assessment and contract rows.
Reconstruct candidates at startup, signal the existing decision commit,
and retry exact pending cards. Keep the direct cleanup call inside
active finalization. ROADMAP.md and the related native review work were
inspected; this change owns the scheduling migration only.

## What Changed

- Add automatic completion-review registration to the shared scoped
deadline worker. Reconstruct 100-row keyset pages and dispatch fair
batches of twenty.
- Signal company/issue IDs before the native decision transaction
writes. Keep outer-transaction settlement with the existing work-signal
boundary.
- Split raw discovery from the public helper's compatibility error
handling. Worker lookup failures propagate so the shared scheduler
retains dirty scope and retry.
- Reuse the complete historical predicate and existing withdrawal owner.
Recheck the selected card identity, payload and system author fields in
the conditional update.
- Preserve the withdrawal reason, chat publication, activity record,
direct finalizer call, startup/manual recovery, and subsequent evidence
reassessment.
- Skip only this duty in periodic reaping. Retain admission,
drain/standby, retry and joined shutdown.
- Add producer/lifecycle documentation. Add no table, schema migration,
provider branch or independent timer.

## Verification

- Current source `3f4d35ac0154aedc76cc39486d3758c73b1e6ff4`: all 462
cases across seven complete files pass with no skips in 49.31 seconds.
This covers both review workers through the real status-authority
corpus, startup/timing, recovery, finalization, external chat and
governed milestones. Nested corpus fixtures are not double-counted.
- Five separate fresh PostgreSQL fixtures migrated through parent 0409
before setup. Database build, server typecheck/build, module boundaries
and target migration order pass. The whole database source tree matches
the parent; no migration or table is added.
- Normal integration preserves substantive owned edits and unrelated
upstream blobs. Actual landed-parent ancestry and fresh current-head
Greptile 5/5 and clean CI remain required. This is worker and
integration acceptance, not deployed sleep qualification.

- The inherited P2 fix changes only two fixture prefix values. All
production code, other fixtures, assertions and timing limits are
unchanged. The full 72-case corpus also passes again on the same
database in 12.95 seconds. Retained PSC/OUT rows and an unrelated
sentinel remain unchanged after both runs. The repeat is not
double-counted among the 462 distinct cases. The parent audit retains
the original collision and rejected cleanup attempt.

- CI correction: the 40f head fails the passive native-presentation
initial idle observation in chat shard 1. An unchanged local pair
passes. A controlled committed hint plus a 600 ms held run attempt makes
the original one-second observation fail; the identical probe passes
with the established five-second observation bound. This demonstrates
the coalescing boundary, not the unrecorded exact CI token timeline. No
unchanged-head CI retry was used.
- The probe is removed. The only permanent code change is that initial
poll bound. It still requires zero active work and no deadline, followed
by two minutes with no SQL, transaction or provider writes. Production
code, worker/business deadlines, rollback and committed-settlement
assertions are unchanged. All 274 cases in the named GitHub agent-review
workflow pass in 232.06 seconds. Other cases in the large file are
outside that named selection. The earlier 462-case selection retains
identical source. Both owning audits preserve the failure and scope
limits. Fresh updated-head CI and reviews remain required.

- Master integration: independent master
`49fe8b87a49aa5a8985a08dd3d5ce57bdd24756c` adds one company-skill
cleanup assertion. A normal merge preserves both complete binary
patches. All production code and the accepted GitHub integration file
are unchanged. The full six-case cleanup-removal suite passes in 8.16
seconds on fresh PostgreSQL. Database history is unchanged.

## Risks

- A normal native decision can cause one scoped lookup even when it
creates no historical card. It retains no periodic scan when settled.
- Startup reconstruction cost scales with historical data and joins. No
constant-cost or fleet-savings claim is made.
- Process-local signals cover instrumented writers. Independent SQL
writers and concurrent older controllers need explicit integration.
- Retirement and status reassessment are distinct obligations. The
existing durable withdrawal receipt and remaining finalization owner are
preserved.
- Failed withdrawals retain retries while exact pending cards remain. A
database lookup failure must not be interpreted as completion.
- Other finalization and recovery duties remain. Cloud wake coverage and
host-sleep eligibility are unchanged.

## Model Used

OpenAI Codex, GPT-6 family, with reasoning, shell tools, and PostgreSQL
tests. The runtime does not expose a more precise model ID or
context-window size for this turn.

## 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

Remote acceptance: head `be668fb975f119efeb4830405d4bb1ab65791f87` has
53 successful gates, verified with `gh pr checks` (no non-passing
checks). Greptile reviewed that head at 5/5. There are no unresolved
review threads. The PR remains draft and unmerged.

Current published head `181bf800a79449961e09c995f2785dd7f8060bb3` has
passing CI, fresh Greptile 5/5 and Ponytail 5/5 with no actionable
findings. Earlier failures remain recorded in the audits.

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip authored and GitHub committed 2026-10-28 23:40:45 -05:00
1 parent 49fe8b87a4
commit 73d24aa5ac
15 files changed
+452 -39

No files matched your search

@@ -2623,3 +2623,9 @@ Slice 43 final local acceptance: 120 owner/startup cases plus 209 affected consu
Cancelled workspace preservation is draft [#17677](https://github.com/paperclipai/paperclip/pull/17677) at `33567f91551d60053722c013f0d9b402c1db29e6`. All 53 checks passed, Greptile reviewed that head at 5/5, and there are no unresolved review threads. It remains unmerged.
The native finalization audit found separate historical unsafe-export recovery, obsolete policy-review cleanup, automatic completion-review cleanup, effect delivery, committed response repair, evidence reassessment and workspace/result finalization. Slice 44 owns obsolete policy-review cleanup only. See [its audit](2026-10-25-obsolete-policy-review-sleep-audit.md). The owner/startup selection passes 116 tests after repairing a reproduced notification serialization bug. Consumer and final workspace verification remain pending. Older stack gates and final master reconciliation remain outstanding.
## Slice 44 accepted; slice 45 automatic completion reviews
Obsolete policy-review cleanup is draft [#17679](https://github.com/paperclipai/paperclip/pull/17679) at `e671c66bba6e9a42aa570826e6f3420dba199399`. All 53 checks pass, Greptile reviewed that head at 5/5, and no unresolved threads remain. It is mergeable and unmerged.
Slice 45 owns automatic completion-review retirement only. It preserves direct finalizer ordering and the durable withdrawal trigger for reassessment. The worker's raw lookup propagates failures so failed discovery cannot look empty. See [the audit](2026-10-25-automatic-completion-review-sleep-audit.md). All 370 local owner/startup/consumer tests pass, as do workspace types/build. Exact-head remote gates remain required. Older stack CI/review gaps, master reconciliation and the remaining original duties are still outstanding.
@@ -0,0 +1,242 @@
# Automatic completion-review retirement
Date: 2026-10-25; current integration: 2026-10-29. Parent: obsolete policy
reviews #17679 at actual squash `dfd5b1ac6205b6ec8f56752dfc5d3f9c02c26b6d`.
The original draft parent was `e671c66bba6e9a42aa570826e6f3420dba199399`.
## One reason and its owner
The native finalization pass calls `dismissAutomaticCompletionReviews` globally,
or per issue when run-scoped. `finalizeNativeRun` also calls it for the active
issue before resolving status. This slice preserves that direct call and its
ordering and migrates only periodic discovery to a dedicated deadline worker.
The service in `native-runtime/automatic-completion-reviews.ts` retains the
historical predicate, withdrawal, chat publication, and audit ownership.
This duty is separate from obsolete policy reviews and subsequent reassessment.
A retired card's existing cancelled result reason is the durable trigger that
`decisionHasRetiredAutomaticReview` checks. Retirement is not finalization
completion. Reassessment and workspace barriers remain with their owners.
## Exact obligation and producers
The existing candidate query identifies:
- A pending system confirmation with no user or agent creator.
- An applied in-review decision with completion_claim_incomplete,
completion_claim_conflict, or external_verification_required.
- Matching effect target, company, issue, source run and assessment.
- A low-risk agent_claim_policy contract with no assessment attention requests.
- The exact native-review idempotency key, native_completion_review target,
decision revision, and historical prompt first line.
Current arbitration no longer creates completion-claim reviews by default.
Explicit attention uses actionable_attention_pending and keyed requests, which
this query excludes. The typed committer still supports historical input; it
registers the new company/issue notification before its decision/effect writes
inside its existing transaction. The notification serializes only those IDs.
Outer commit, rollback, and uncertain writes use the existing shared protocol.
The production inserter of decisions and status effects is
`commitNativeStatusDecision`. No production updates of completion contracts or
work assessments were found; these records are immutable inputs. Existing
decision updates adjust lineage or applicationState within the committer.
Interaction response, dismissal, expiry, summary, and connection-intent updates
do not create this historical decision/effect lineage or reopen answered native
reviews. A generic authored card also fails the system-card predicate.
Startup covers historical rows. Independent SQL writers or concurrent older
controllers require explicit integration; process-local signals do not cover them.
## Worker and invariants
The existing scoped deadline helper reconstructs 100-row keyset pages, stores
interaction/company/issue IDs and deadlines, and processes fair batches of
20. Normal commit hints inspect only their company/issue. Settled history leaves
no recurring SQL or deadline. No table, migration, separate timer, or provider
branch is added.
Discovery now has a raw internal path that propagates SQL errors. The public
manual lookup retains its logging/empty-list compatibility. The worker must not
mistake a failed lookup for successful reconstruction and discard a dirty scope.
The shared helper retains failed discovery with backoff.
Each attempt rereads the exact candidate. Withdrawal retains the existing
coordinator/issue locks and additionally matches the selected kind, source run,
idempotency key, system author fields and payload in its conditional update.
A changed or answered card is not withdrawn from stale discovery. The existing
durable withdrawal reason, terminal chat publication and activity remain in the
same transaction. A failed withdrawal leaves a matching card pending and retains
its 30-second retry; another candidate can still settle. A removed or completed
obligation disarms. No age or retry-count cutoff discards a card.
The worker shares admission, drain and standby gates. Startup while paused and
writes during pause retain a request without passive database reads. Shutdown
joins admitted transactions and removes registration. Only periodic orphan
reaping passes the new skip flag through the finalization options. Startup,
manual reconciliation and the direct active finalizer call remain compatible.
## Evidence and limits
The previous slice's owner/consumer corpus is the pre-change baseline. The
first server typecheck and the typecheck with lifecycle additions passed.
The complete owner/startup/timing selection passed all 135 tests without skips:
72 status-authority corpus cases, 60 startup cases and three timing cases.
The corpus includes its existing 53-fixture native/legacy matrix test.
Both review owners run the same lifecycle contract against real PostgreSQL:
genuine/settled quiet, real committer activation, outer commit and rollback,
restart, drain, standby, paused startup, isolated cleanup failure, retry after
pause, concurrent card edits, concurrent answers, shutdown join, and 103-card
paging. Automatic cleanup additionally uses the raw failure path. The injected
discovery failure leaves a pending deadline and repairs after resume. Existing
manual tests preserve explicit attention, stronger authority, later task edits,
newer contracts, workspace failure and unread child-completion obligations.
A rolled-back fixture added 10,000 completed interactions on the same issue and
restored one pending automatic review. The scoped lookup used existing indexes,
returned one candidate, and took 0.080 ms locally. SQL and plan are retained in
`/tmp/sleep45-query-plan.sql` and `.log`. This supports the access path, not a
production latency promise. Global startup reconstruction still scales with
historical data and joins.
Affected consumer suites, final workspace checks and exact-head remote gates
remain required until their results are recorded. The host uses fresh disposable
PostgreSQL 17 test databases because its embedded harness is unavailable.
Other finalization duties, earlier stack CI/review findings and final master
reconciliation remain in scope. This slice makes no claim about Cloud alarm
coverage, host/database suspension, or measured fleet savings.
Final affected-consumer verification passed all 235 tests without skips; together
with the owner/startup/timing selection, 370 local tests passed. Workspace
typecheck and build passed. The two initial typechecks and final checks all
passed; no failed selection was discarded. Remote gates are pending.
## Earlier parent and acceptance (2026-10-29)
A normal merge adopts the verified obsolete-policy cleanup parent and current
recovery source. All substantive owned edits and unrelated upstream blobs are
preserved. The complete database source tree equals the parent: no migration,
table or executable history change is added, and the parent 0407 history with
five newest snapshots remains. No new code comments are added.
Source `0d9087775b54dc7272bcae51345ff690e3407235` passes **462 cases across seven complete
files, no skips**, in 49.57 seconds. The status-authority corpus runs both review
workers against the lifecycle contract. Fixtures nested inside its matrix case
are not counted again. Exact file counts:
| Complete file | Passing cases |
| --- | ---: |
| src/services/native-runtime/external-chat-wait.integration.test.ts | 234 |
| src/__tests__/native-status-arbiter-corpus.test.ts | 72 |
| src/services/heartbeat/recovery.test.ts | 45 |
| src/__tests__/chat-run-milestone-governed-work.test.ts | 16 |
| src/services/native-runtime/native-run-finalizer-telemetry.test.ts | 29 |
| src/__tests__/server-startup-feedback-export.test.ts | 63 |
| src/services/native-runtime/native-finalization-reconciler-timing.test.ts | 3 |
Five separate fresh PostgreSQL 17 databases are migrated before corpus, recovery,
finalizer, external-chat and governed-milestone fixture setup. All selected suites
use their supported external URL path; none is skipped because this host cannot
allocate another embedded cluster. Database build, server typecheck/build, module
boundaries, target migration order and all five fresh parent-0407 migrations pass.
The previous slice's circular-scope-payload failure remains recorded in its
owning audit and is not erased by these passes.
These checks establish current cleanup, authority and affected-consumer behavior,
not deployed host/database suspension or external alarms. Actual landed parent
ancestry and fresh published-head review and CI remain required.
## Current migration409 parent and complete acceptance
Normal integration `27378007cd64c308f41940097a999d51201b8ba8` preserves the
complete original runtime patch and all unrelated upstream blobs. The entire
database source tree equals the current parent, with 0409 history and five
newest snapshots unchanged. No migration or table is added.
All **462 cases across seven complete files pass without skips**, in 49.75
seconds. The selection and per-file counts match the preceding table. Five
separate fresh PostgreSQL 17 databases migrate through 0409 before setup.
Database build, server types/build, module boundaries and exact-head migration
order pass. All suites use their supported external-database fixture path; no
source adaptation or skipped suite is required. Earlier failures remain recorded.
This proves current worker and affected consumer behavior, not deployed
host/database suspension or Cloud alarms. Publication stays held until the
actual prerequisite merge and squash ancestry reconciliation. Fresh current-head
CI and reviews remain required before merge.
## Published obsolete-review parent and actual preservation ancestry
Normal merge `c0f3ada4c018943982731635c26d37c7bb4b9ad9` adopts published parent
`45cb0d9ccc9b0c37068f56a709f16572bd7b7ddc`. It carries the actual cancelled-preservation
squash ancestry and parent audit only. Complete server, package, UI and test
trees remain identical to accepted source `27378007cd64c308f41940097a999d51201b8ba8`,
including the whole 0409 database tree. The 462-case acceptance retains its
original scope. Publication remains held until the actual obsolete-review merge
and ancestry reconciliation. Fresh current-head CI and review remain required.
## Inherited corpus repeatability correction
Published obsolete-review parent `9aa67199da64d700fd6230829d38f2ef522a07fe`
carries the verified two-value fixture prefix correction. Normal integration
`3f4d35ac0154aedc76cc39486d3758c73b1e6ff4` changes only those two corpus
fixture values and parent documentation/ancestry. All production source, other
test files and the complete 0409 database tree remain unchanged. The parent's
audit retains the collision reproduction and rejected foreign-key cleanup
attempt. No assertion, timing limit, teardown or worker behavior changes.
All **462 distinct cases in seven complete files pass without skips**, in
49.31 seconds. The full 72-case corpus then passes again on the same database
in 12.95 seconds. Three preexisting PSC/OUT/sentinel rows remain unchanged in
the inspected ID/name/prefix fields after both runs. Each invocation appends
only its two fixture companies with distinct prefixes. The repeat is not added again
to the distinct case count. Five fresh PostgreSQL 17 fixtures migrate through
0409; database build, server types/build, boundaries and migration order pass.
The actual prerequisite must still merge before publication. Fresh published-head
CI and reviews remain required. No deployed sleep or external-alarm claim is made.
## Actual obsolete-review squash ancestry
Parent #17679 landed as `dfd5b1ac6205b6ec8f56752dfc5d3f9c02c26b6d`. Its whole tree
equals reviewed parent `9aa67199da64d700fd6230829d38f2ef522a07fe`, already present
in this child history. Normal merge `8c5c5da563e32b9a5520a4f8ddefc38c29b4755e`
resolves inspected duplicate squash-ancestry conflicts and preserves the entire
accepted child tree. Complete server, package, UI and test trees remain
identical to the 462-case source and same-database 72-case repeat. The complete
0409 database source remains exactly parent-identical. Fresh published-head
CI and reviews remain required.
## Current-head chat CI correction
The published 40f head fails the initial passive native-presentation idle
observation in chat shard 1. The owner audit records the original failure,
unchanged local baseline, controlled red/green comparison and its limits:
[Native presentation startup observation](2026-10-25-github-terminal-response-sleep-audit.md#ci-native-presentation-startup-observation).
The permanent correction changes one observation from the default one-second
window to the existing five-second settlement bound. Production code and
business/worker deadlines do not change. All quietness, rollback and committed
settlement assertions remain. With the probe removed, all 274 selected GitHub
agent-review cases pass in 232.06 seconds. The earlier 462-case selection and
72-case repeat retain identical production and selected test sources. Fresh
CI and review are required on the updated published head.
## Independent master cleanup assertion preserved
Master advanced from the actual obsolete-review squash to
`49fe8b87a49aa5a8985a08dd3d5ce57bdd24756c`. The sole change is an assertion
that company cleanup removes company skills. Normal merge
`4dbec260d36cae02bd73281bbdaac406ee989010` preserves both complete binary
patches: this PR's full fix and the independent upstream assertion. All production
source and the accepted GitHub integration file remain unchanged. The complete
cleanup-removal service suite passes all six cases in 8.16 seconds on a fresh
0409 PostgreSQL 17 fixture. The whole database source is unchanged. Current-base
migration order and fresh published-head CI/reviews remain required.
@@ -377,3 +377,46 @@ Original producer, quietness, failure-isolation, skipped-suite and presentation
results above retain their original scopes. Fresh exact-head CI and Greptile 5/5
with no actionable findings remain required. This does not establish a full
local repository result, live GitHub delivery or physical host/database sleep.
## CI: native presentation startup observation
PR #17680 head `40f84082526d6ba405f206475c919c3b4b67ec4c` fails chat shard
1 in run 40814818307, job 123253388098. The passive native-presentation case
expires its initial default one-second idle poll with no deadline and two active
tracked operations. The aggregate verify check fails because of that shard.
This is retained as a real CI failure; no unchanged-head retry was issued.
The scoped worker's minimum pass interval is also one second. The initial poll
was added after the earlier coalescing correction documented above and still
used the default, while neighboring settlement observations use five seconds.
The unchanged local focused pair passes, so there is no claim of an unmodified
local reproduction of CI scheduling.
A temporary controlled fixture commits a real milestone hint during the initial
registered run and holds the second registered run attempt for 600 ms after
delegating to the existing implementation. That attempt may defer at the
unchanged one-second gate, retaining a coalesced follow-up. Both old observations
fail at the original one-second bound. With the identical probe, both pass when
the initial observation uses the existing five-second bound. The controlled
failures have a pending deadline and one active token; CI saw no deadline and
two active tokens. This proves that the observation window cannot reliably fit
allowed startup follow-up, not the exact unrecorded CI token-owner timeline.
The probe is removed byte for byte before final acceptance. The permanent
change is one argument on the initial poll. It still requires a null deadline
and zero active work, then proves two virtual minutes with zero SQL, transaction
or provider writes. Rollback, committed settlement and final drain assertions
are unchanged. Production code, scheduler cadence, retry/business deadlines and
the overall test budget are unchanged. Only this eventual-startup observation
uses the established five-second bound.
The no-probe focused pair passes. The complete named GitHub agent-review
workflow then passes all **274 selected cases** in 232.06 seconds on fresh
PostgreSQL 17. The other 1507 cases in the large integration file are outside
that named selection, not counted as acceptance. Five dedicated fresh 0409
fixtures isolate baseline, controlled red, controlled green, focused and workflow
runs. No embedded CI-environment equivalence is claimed. The source file hash
for permanent acceptance is
`d0dbef76741f03862dfef0a42338f97d5ea7a9f36d4b465c7538a5eaacd65b2c`.
Fresh published-head CI and review remain required.