Files
PaperClipAI/doc/plans
DottaandPaperclip dfd5b1ac62 fix(server): wake obsolete policy review cleanup from commits (#17679)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Native finalization repairs saved results and related task state.
> - Its periodic pass also searches for obsolete policy-review cards.
> - Those cards have a precise historical signature and existing durable
rows.
> - This PR gives that cleanup its own committed signals and
pending-work retries.
> - The existing owner still protects real reviews and later task
changes.
> - Settled review history no longer causes repeated cleanup queries.

## Linked Issues or Issue Description

Refs #17677. This PR is stacked on cancelled workspace preservation. It
owns only obsolete policy-review cleanup within the native finalization
reconciler.

**What needs to be improved?**

Every periodic native finalization pass queries for old version-change
review cards, even after all such cards are gone. The parent reaper
combines this cleanup with unrelated finalization and recovery duties.

**Why is this improvement important?**

Empty queries prevent idle tenants from becoming quiet. Removing the
query must preserve historical repair, real review requests, task status
authority, and cleanup after a failed write.

**Proposed solution**

Reconstruct matching pending cards at startup. Signal the existing
native decision transaction before its writes. Schedule bounded cleanup
only while an exact card remains pending. Skip this duty in periodic
reaping while keeping startup and manual entry points. ROADMAP.md and
related PR search found no duplicate of this migration.

## What Changed

- Register obsolete-review cleanup with the existing scoped deadline
worker. Reconstruct in 100-row keyset pages and process fair batches of
twenty.
- Use company/issue hints from the existing status-decision commit
boundary. Serialize only the two IDs; preserve outer commit and
rollback.
- Reuse the exact historical signature and existing cleanup owner. Keep
status-version, decision-pointer, separate-review, activity and
chat-publication guards.
- Recheck selected payload and system-card identity in the conditional
withdrawal so a concurrent edit cannot be cancelled from a stale
snapshot.
- Retain a 30-second retry only while a matching pending card survives.
Preserve drain, standby, startup and joined shutdown.
- Skip only this duty in the periodic finalization pass. Automatic
completion reviews, unsafe exports, status effects, evidence
reassessment and workspace finalization remain separate migrations.
- Add the owner audit and validated external-test database support for
the existing status-authority corpus. Add no schema, table, independent
timer or provider branch.

## Verification

- Current source `4cc0d03ae96bd92a4fdad44168c2a8647d42466c`: all 443
cases across seven complete files pass with no skips in 46.46 seconds.
This covers the full status-authority corpus and cleanup worker,
startup/timing, heartbeat recovery, native finalization, external chat
and governed milestones. The audit gives per-file counts without
double-counting fixtures inside the corpus matrix.
- Five distinct 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. The earlier circular notification-payload failure
remains in the audit. Actual parent
`51c3f8924cb0bd5fe7cbda15ad9b8f06a6a11055` is reconciled through a
normal merge, with its full tree equal to the reviewed ancestor and the
entire accepted child tree unchanged. Fresh current-head Greptile 5/5
and clean CI are still required. No deployed sleep qualification is
claimed.

- Review correction: the old head reproduces a fixed-prefix collision on
a same-database corpus rerun. The initial deletion attempt passed
assertions but failed teardown on non-cascading audit foreign keys; it
was removed. The final two-value change uses each fixture company UUID
in its prefix and changes no production behavior, assertions, timing
limits or teardown.
- The complete 56-case corpus passes a second time on the same database
in 10.15 seconds. Retained PSC/OUT rows and an unrelated sentinel
company remain unchanged after both runs. The repeat is not
double-counted among the 443 distinct cases. The audit retains both
failures and their causes. Fresh CI and Greptile are required for the
updated head.

## Risks

- Startup reconstruction still scans historical candidates and joins;
its cost scales with history. Scoped commits use existing indexes. No
recurring query remains after this worker settles.
- The signal covers instrumented single-process writes. Independent SQL
writers and concurrent older controllers need explicit integration.
- A normal native decision can cause one scoped lookup, even when it
creates no obsolete review. It does not retain a periodic scan.
- Failed withdrawals retain retries while matching cards remain. Actual
cleanup still uses the existing locking, authority and activity owner.
- Other finalization duties still run. This PR does not establish
whole-instance quiet, external alarm coverage, provider suspension or
fleet savings.

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

Current published head `9aa67199da64d700fd6230829d38f2ef522a07fe` has
fresh Greptile 5/5, Ponytail 5/5 with an unconditional ship verdict, and
all current CI checks passing or skipped. The reproduced rerun P2 is
fixed and its thread is resolved.

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-10-28 22:53:19 -05:00
..