Files
PaperClipAI/server/src/services
DottaandPaperclip c38844eafa test: observe complete workspace retention cleanup (#18507)
## Thinking Path

> - Paperclip manages AI agents and preserves their work.
> - Remote workspace retention removes saved objects after their last
pin clears.
> - The retention worker deletes each object before it commits the final
snapshot state.
> - The parent-revisit test treats the first object deletion as
completion of all work.
> - That valid intermediate state can fail the next-deadline assertion.
> - This pull request observes complete storage, database, and worker
settlement within the existing bound.
> - The test still fails if the parent is not revisited after the child
releases its pin.

## Linked Issues or Issue Description

Refs: #18351
Refs: #18506

**What happened?**

[The server6 CI
job](https://github.com/paperclipai/paperclip/actions/runs/41263476916/job/124721633674)
failed at `remote-workspace-snapshot-retention.test.ts:248`. The test
expected a null retention deadline after the first parent object
disappeared. The parent could still have its second object and a
`delete_requested` row. The worker was not finished.

**Expected behavior**

The test must wait for both parent objects, both snapshot rows, and the
retention worker to finish. It must retain the four-second bound and
40ms observation interval.

**Steps to reproduce**

1. Use the parent/unfinished-child fixture in the retention test.
2. Hold the second parent-object deletion after the first object is
removed.
3. Read the old assertion while the parent row remains
`delete_requested`.
4. The old assertion receives `1793361600000` instead of null. Release
the hold and observe the completed state.

**Paperclip version or commit**

The failed CI checkout was `ab849702a78f6bb619b1527596cd52db402d9189`.
This test-only repair starts from actual mainline
`b110271c9b2fd6dd5a1beb119e7f0663385df8b9`. The relevant test, retention
service, coordinator, storage, test-helper, and configuration blobs
match the failed checkout and mainline
`756599e4ea069c9338c320fe8846523d3dfb92a0`.

**Deployment mode**

Built-from-source server tests. The local controls used one disposable
PostgreSQL17 instance and local disk storage. They used no deployed
company or model.

## What Changed

- Observe both parent objects and the exact parent and child database
rows in one bounded assertion.
- Require deleted rows with cleared deadlines, zero active tracked work,
and no coordinator wake before the test ends.
- Keep the original four-second timeout and 40ms interval. Production
retention and scheduling code stay unchanged.

## Verification

- All17 tests in `remote-workspace-snapshot-retention.test.ts` passed on
Node24 with current-source aliases, current migrations, and a disposable
PostgreSQL17 database.
- At immutable head `1e7eeb422be260bfaf752792596f8cdd126fff8d`, the
second-deletion hold control also passed all17 file tests. Removing only
the parent-revisit signal still failed the affected case within four
seconds. The owned database container was stopped and its automatic
removal was verified.
- Normal checkout command: `pnpm --filter @paperclipai/server exec
vitest run src/services/remote-workspace-snapshot-retention.test.ts
--reporter=verbose`.
- Four bounded controls used the exact failed CI source: unchanged
baseline PASS; held second deletion with the old assertion RED; the same
held state with complete-settlement observation GREEN; removed
parent-revisit signal RED under the original four-second limit.
- The held state proved: first parent object absent, second present,
child row deleted, parent row `delete_requested`, non-null deadline, and
active worker. This proves a reachable observer bug. The exact
interleaving in the historical CI job was not recorded and remains
unknown.
- The same case passed at mainline756 in [the canonical readiness
control](https://github.com/paperclipai/paperclip/actions/runs/41260612021/job/124712930107).
Source identity and a passing control alone do not establish cause; the
held-state control supplies that evidence.
- `git diff --check` passed. The net source change is one test file,
with no added comments or runtime changes.
- Final exact head `1e7eeb422be260bfaf752792596f8cdd126fff8d`:
[canonical
CI41264965700](https://github.com/paperclipai/paperclip/actions/runs/41264965700)
completed successfully. All57 checks are terminal and clean, with no
pending or failed checks. This includes the aggregate verify and Snyk
gates. The PR is mergeable and clean. No old failed job was retried.
- [Greptile reviewed this exact full head
at5/5](https://github.com/paperclipai/paperclip/pull/18507#issuecomment-6552310742),
with no actionable findings and zero open review threads. Complete
thread pagination was checked.
- [The current Paperclip assessment
is4/5](https://github.com/paperclipai/paperclip/pull/18507#issuecomment-6552292204),
with a final full-head footer. Its check conclusion is neutral. It calls
the fix correct and identifies one minor, non-blocking concern about a
process-wide active-work counter. This score remains4/5; it is not
represented as5/5.

## Risks

The assertion is stricter because it checks complete deletion and worker
settlement. A real missing revisit, retained object, unresolved row, or
continuing worker must still fail. The bound is unchanged. No schema,
API, production storage, scheduler, or deployed configuration changes
are included. No user-facing command or behavior changed, so no product
documentation update is needed.

The Paperclip review notes that `idleWorkSnapshot().active` is
process-wide. The current test configuration uses fork isolation, one
worker, and serial tests, so the counter is scoped to this fixture in
the present test run. We retain it because deleted rows and an empty
wake queue can be observed before the coordinator finishes its active
work. If test isolation or fixture concurrency changes, this assertion
will need a worker-specific settlement check. This is the review's
accepted non-blocking limitation; it is not waived or scored as
resolved5/5.

## Model Used

OpenAI Codex, GPT-6 family, with reasoning, source inspection, code
execution, and tool use. The session does not expose a more specific
backend model ID or context-window size; neither is inferred.

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

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-10-31 21:54:42 -05:00
..
…
…
…