PaperClipAI

Jannik/PaperClipAI

0 Fork 0

mirror of https://github.com/paperclipai/paperclip.git synced 2026-11-07 07:55:47 +01:00

Commit Graph

Author	SHA1	Message	Date
DottaandPaperclip	73d24aa5ac	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>	2026-10-28 23:40:45 -05:00
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
Devin FoleyandPaperclip	cda77edc60	perf(server): stop readiness waiting on startup housekeeping (#16329 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - On restart, `/api/health` reports `starting` until startup recovery finishes, and deploy tooling waits for `ok` > - After heartbeat recovery, readiness also waits for housekeeping that every scheduler tick repeats and that serving requests does not need > - #16330 added OpenTelemetry spans for the main startup phases; native recovery and orphan reaping are the slow ones, and both run the native finalization reconciliation pass, which has no inner breakdown > - This pull request stops readiness waiting on that housekeeping, and records the phases of slow reconciliation passes as spans > - The benefit is faster readiness on every restart, and traces that show where a slow reconciliation pass spends its time ## Linked Issues or Issue Description Builds on #16330, which records `controller.startup.<phase>` spans. Related to #16328. What existing behavior does this improve? Server readiness time after a restart, and diagnosis of slow native recovery. Current behavior - After heartbeat recovery, readiness waits for the custom-image setup cleanup, tool connection health (remote MCP checks), decision expiry, the adapter and setup-token login reapers, environment lease cleanup (provider calls) and the retention sweep. Each also runs on every scheduler tick. On one instance they added 11–28 s to every measured restart. A throw in any of them fails startup. - On the same instance, native recovery took from 33 s to 11.5 min and orphan reaping from 30 s to 4 min across restarts in one hour. Both run the native finalization reconciliation pass. The #16330 spans time each step as a whole, but not the phases inside that pass. Proposed behavior - The housekeeping steps run after readiness. The scheduler interval still starts after them. - With an OTLP endpoint, a reconciliation pass of at least one second records a span with one child span per phase. Reason and benefit Readiness depends only on what serving requests and run recovery need. Slow reconciliation passes become attributable to a phase in the existing traces, without adding trace volume for the periodic pass. Breaking changes None. A failed housekeeping step no longer fails startup; it is logged and the next step runs. ## What Changed - `server/src/index.ts`: moves the housekeeping steps into `runStartupHousekeeping`, which runs after readiness under `trackHeartbeatSchedulerWork` (idle sleep and shutdown wait for it). Each step catches and logs its own failure (`startup housekeeping step failed`). The heartbeat scheduler interval starts when housekeeping finishes, unless the scheduler was stopped, so the reapers still finish before the first timer tick. - New `server/src/services/native-runtime/native-reconciliation-timing.ts`: records the phase boundaries of one pass. When the pass took at least one second and an OTLP endpoint is configured, it emits a `controller.native_reconciliation` root span (`paperclip.native-reconciliation`) with child spans `legacy_workspace_exports`, `obsolete_policy_reviews`, `automatic_completion_reviews`, `candidate_scan` and `candidates`, using the recorded start and end times. It records `scoped` and the candidate count only. A failed pass marks the pass span and the running phase. Tracing failures are swallowed. - `native-finalization-reconciler.ts`: the exported function wraps the pass in that timing and finishes it in `finally`, so failed passes are recorded; the original error is rethrown. - `doc/execution-semantics.md`: what readiness waits for, and the housekeeping order. `doc/observability.md`: the reconciliation spans. ## Verification - `pnpm exec vitest run src/__tests__/server-startup-feedback-export.test.ts src/services/native-runtime/native-reconciliation-timing.test.ts src/services/native-runtime/native-finalization-reconciler-timing.test.ts src/startup-recovery-timing.test.ts` (in `server/`): 52 passed. New tests: - Readiness is `ready` while a housekeeping step hangs, and the heartbeat interval starts only after the step settles. - A failed housekeeping step is logged, and the scheduler still starts after the remaining steps. - Span timing: nothing for a fast pass or without an endpoint; a slow pass emits the pass span and one child per phase with the recorded times; a failed pass marks the running phase and the pass; a failing tracer does not throw. - The reconciler records its phases and a failed outcome when a pass throws, and rethrows. - Three existing startup tests settle housekeeping before they read the scheduler interval or the setup cleanup. - `pnpm exec vitest run src/services/native-runtime`: 4,399 passed. - `pnpm typecheck` in `server/`: passed. ## Risks - Housekeeping now runs concurrently with the first requests and with `waitForExternalAdapters`. None of these steps is needed to serve requests, and each one also runs on every scheduler tick. - Readiness is reported earlier; the heartbeat timer starts at the same point relative to housekeeping as before. - The new spans exist only with an OTLP endpoint and only for passes of at least one second. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), 1M context, through Claude Code with tool use (shell, file edits, test runs). ## 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 - [ ] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>	2026-10-14 15:55:13 -07:00

Author

SHA1

Message

Date

DottaandPaperclip

73d24aa5ac

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>

2026-10-28 23:40:45 -05:00

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

Devin FoleyandPaperclip

cda77edc60

perf(server): stop readiness waiting on startup housekeeping (#16329 )

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - On restart, `/api/health` reports `starting` until startup recovery
finishes, and deploy tooling waits for `ok`
> - After heartbeat recovery, readiness also waits for housekeeping that
every scheduler tick repeats and that serving requests does not need
> - #16330 added OpenTelemetry spans for the main startup phases; native
recovery and orphan reaping are the slow ones, and both run the native
finalization reconciliation pass, which has no inner breakdown
> - This pull request stops readiness waiting on that housekeeping, and
records the phases of slow reconciliation passes as spans
> - The benefit is faster readiness on every restart, and traces that
show where a slow reconciliation pass spends its time

## Linked Issues or Issue Description

Builds on #16330, which records `controller.startup.<phase>` spans.
Related to #16328.

**What existing behavior does this improve?**
Server readiness time after a restart, and diagnosis of slow native
recovery.

**Current behavior**
- After heartbeat recovery, readiness waits for the custom-image setup
cleanup, tool connection health (remote MCP checks), decision expiry,
the adapter and setup-token login reapers, environment lease cleanup
(provider calls) and the retention sweep. Each also runs on every
scheduler tick. On one instance they added 11–28 s to every measured
restart. A throw in any of them fails startup.
- On the same instance, native recovery took from 33 s to 11.5 min and
orphan reaping from 30 s to 4 min across restarts in one hour. Both run
the native finalization reconciliation pass. The #16330 spans time each
step as a whole, but not the phases inside that pass.

**Proposed behavior**
- The housekeeping steps run after readiness. The scheduler interval
still starts after them.
- With an OTLP endpoint, a reconciliation pass of at least one second
records a span with one child span per phase.

**Reason and benefit**
Readiness depends only on what serving requests and run recovery need.
Slow reconciliation passes become attributable to a phase in the
existing traces, without adding trace volume for the periodic pass.

**Breaking changes**
None. A failed housekeeping step no longer fails startup; it is logged
and the next step runs.

## What Changed

- `server/src/index.ts`: moves the housekeeping steps into
`runStartupHousekeeping`, which runs after readiness under
`trackHeartbeatSchedulerWork` (idle sleep and shutdown wait for it).
Each step catches and logs its own failure (`startup housekeeping step
failed`). The heartbeat scheduler interval starts when housekeeping
finishes, unless the scheduler was stopped, so the reapers still finish
before the first timer tick.
- New
`server/src/services/native-runtime/native-reconciliation-timing.ts`:
records the phase boundaries of one pass. When the pass took at least
one second and an OTLP endpoint is configured, it emits a
`controller.native_reconciliation` root span
(`paperclip.native-reconciliation`) with child spans
`legacy_workspace_exports`, `obsolete_policy_reviews`,
`automatic_completion_reviews`, `candidate_scan` and `candidates`, using
the recorded start and end times. It records `scoped` and the candidate
count only. A failed pass marks the pass span and the running phase.
Tracing failures are swallowed.
- `native-finalization-reconciler.ts`: the exported function wraps the
pass in that timing and finishes it in `finally`, so failed passes are
recorded; the original error is rethrown.
- `doc/execution-semantics.md`: what readiness waits for, and the
housekeeping order. `doc/observability.md`: the reconciliation spans.

## Verification

- `pnpm exec vitest run
src/__tests__/server-startup-feedback-export.test.ts
src/services/native-runtime/native-reconciliation-timing.test.ts
src/services/native-runtime/native-finalization-reconciler-timing.test.ts
src/startup-recovery-timing.test.ts` (in `server/`): 52 passed. New
tests:
- Readiness is `ready` while a housekeeping step hangs, and the
heartbeat interval starts only after the step settles.
- A failed housekeeping step is logged, and the scheduler still starts
after the remaining steps.
- Span timing: nothing for a fast pass or without an endpoint; a slow
pass emits the pass span and one child per phase with the recorded
times; a failed pass marks the running phase and the pass; a failing
tracer does not throw.
- The reconciler records its phases and a failed outcome when a pass
throws, and rethrows.
- Three existing startup tests settle housekeeping before they read the
scheduler interval or the setup cleanup.
- `pnpm exec vitest run src/services/native-runtime`: 4,399 passed.
- `pnpm typecheck` in `server/`: passed.

## Risks

- Housekeeping now runs concurrently with the first requests and with
`waitForExternalAdapters`. None of these steps is needed to serve
requests, and each one also runs on every scheduler tick.
- Readiness is reported earlier; the heartbeat timer starts at the same
point relative to housekeeping as before.
- The new spans exist only with an OTLP endpoint and only for passes of
at least one second.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`), 1M context, through Claude Code
with tool use (shell, file edits, test runs).

## 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
- [ ] I will address all Greptile and reviewer comments before
requesting merge

Co-authored-by: Paperclip <noreply@paperclip.ing>

2026-10-14 15:55:13 -07:00