Commit Graph
2 Commits
Author SHA1 Message Date
Devin FoleyandPaperclip aec80cbc8c fix(server): keep workspace compaction out of the startup sweep (#16328)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - When the server starts, `/api/health` answers "starting" until
startup recovery finishes, and deploy tooling waits for "ok"
> - Startup recovery waits for the first sweep of the terminal-workspace
worker
> - #16304 added workspace compaction to that sweep. Compaction walks
worktrees, runs Git, and deletes dependency trees, and it re-checks
every eligible workspace after a restart
> - Compaction is never needed to serve requests, so it should not delay
readiness
> - This pull request skips compaction in the startup sweep only; later
sweeps compact as before
> - The benefit is that a compaction backlog cannot make a restart
slower

## Linked Issues or Issue Description

Follow-up to #16304.

**What existing behavior does this improve?**
Server startup time after #16304.

**Current behavior**
`await terminalWorkspaceWorker.ready` blocks the readiness flip until
the first terminal-workspace sweep ends. That sweep compacts every
eligible workspace in its page. The per-cycle compaction memo is in
memory, so after each restart every eligible workspace is walked and
checked with Git again, and any directories found are deleted before the
sweep ends.

**Proposed behavior**
The first sweep skips compaction. Pending compactions keep their
deadlines, so the worker wakes and a later sweep compacts them.

**Reason and benefit**
Readiness depends only on work that is needed to serve requests. On one
measured instance the first sweep took 5–8 s with compaction; with a
larger backlog, compaction could add seconds per workspace.

**Breaking changes**
None. Compaction happens one sweep later after a restart.

## What Changed

- `sweepTerminalWorkspaces(limit, signal, { compact })`: `compact:
false` skips compaction for that sweep and keeps each pending compaction
deadline.
- `registerTerminalWorkspaceWork` passes `compact: false` to its first
sweep (the one readiness waits for) and `compact: true` after that.
- `server/src/index.ts` forwards the option through the logging wrapper.
- `doc/DEVELOPING.md`: the compaction section says the startup sweep
never compacts.

## Verification

- `pnpm exec vitest run src/__tests__/terminal-workspace-work.test.ts
src/__tests__/execution-workspace-compaction.test.ts
src/__tests__/workspace-compaction.test.ts` (in `server/`): 35 passed.
- New: the worker passes `compact: false` only to the startup sweep and
`compact: true` to the next one.
- New: a sweep with `compact: false` leaves the files in place, the
compaction deadline stays due, and the next sweep compacts.
- `pnpm typecheck` in `server/`: passed.

## Risks

- Low risk. After a restart, compaction runs a few seconds later than
before. Archive behavior does not change.

## 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:27:28 -07:00
DottaandPaperclip 7c016c049b perf(workspaces): wake terminal cleanup from committed changes (#16282)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Agents use execution workspaces to retain their files and Git
branches.
> - A reaper closes eligible workspaces after their task tree is
terminal.
> - The reaper scanned every heartbeat tick and read Git before cheap
eligibility checks.
> - Permanent scans also query idle databases when no cleanup work
exists.
> - This pull request wakes cleanup after transaction settlement and
restores bounded work at startup.
> - The benefit is fewer scans while fresh cleanup guards continue to
protect retained work.

## Linked Issues or Issue Description

Related work: Refs #13619, Refs #11753. Those PRs bound scans or back
off unchanged candidates. This PR uses the transaction work coordinator
to remove the permanent reaper tick. It overlaps their early
terminal-state filtering but does not change their runtime restart
behavior or introduce a second Git scheduler cache.

**What existing behavior does this improve?**

Terminal workspace cleanup and Git inspection for workspace detail
reads.

**Subsystem affected**

Server orchestration, execution workspaces, and idle sleep admission.

**Current behavior**

The heartbeat tick scans workspace candidates every thirty seconds. It
runs Git before rejecting nonterminal trees, cooldowns, and active runs.
Display inspections expire after five seconds.

**Proposed behavior**

Recover workspace rows in bounded startup pages. Wake after relevant
issue, workspace, and pull-request work-product transactions settle.
Schedule the terminal tree cooldown or a five-minute eligibility retry.
Back off unchanged candidates across unrelated wakes, and invalidate
their backoff after relevant committed changes. Park empty and
nonterminal-only lanes. Reuse display inspections for thirty seconds.
Read fresh Git before destructive cleanup.

**Reason and benefit**

Avoid recurring database and Git work when no cleanup is possible.
Retain transaction admission holds, restart recovery, and fresh deletion
checks. Retention eligibility can wait for the next host wake; active
and failed recovery work still blocks sleep.

**Breaking changes**

No schema or API shape changes. Display Git state can lag by thirty
seconds. Cleanup eligibility retries can take five minutes.

## What Changed

- Add a terminal-workspace worker to the existing transaction work
coordinator and scheduler.
- Signal relevant writes before mutation on the owning transaction.
Outer transaction settlement wakes cleanup.
- Derive cooldown deadlines from the full company-scoped terminal task
tree. Continue bounded recovery pages without a permanent sweep.
- Bound unchanged candidate inspections to five minutes, retain at most
256 entries, and invalidate affected entries with scoped transaction
hints. Repeated wakes preserve the earliest inspection deadline.
- Collect deadlines from task trees already read by bounded pages.
Restore the cursor after a failed page and rescan commits received
during a rotation.
- Check terminal state, cooldown, and active runs before Git. Preserve
lifecycle locks, generation fences, and fresh final checks.
- Keep failed or incomplete recovery visible to idle admission. Preserve
the passive retention exception.
- Extend the bounded display cache to thirty seconds and document the
scheduling and wake limits.

## Verification

- Focused workspace, coordinator, cache, and local sleep suites: 111
tests passed with real local PostgreSQL and Git worktrees where
applicable.
- Final worker, display cache, and local sleep suites: 28 tests passed.
- The regressions prove three ineligible workspaces invoke zero Git
scans, cached clean display state cannot remove new untracked files, and
real producer notifications wait for outer transaction settlement.
- `pnpm -r typecheck` and `pnpm build` passed locally. Map and
module-boundary checks passed.
- Work-product and sleep-safety suites: 221 tests passed. Reopen and
issue-tree control suites: 46 tests passed. Final backoff, workspace,
worker, and work-product suites: 87 tests passed.
- Final workspace and worker suites: 81 tests passed. Deadline selection
performs zero database reads. A failed full page retries its unread
terminal workspace before recovery can park.
- Full CI coverage passed on head `2c397970f7`: general tests,
serialized server suites, runner verification, E2E shards, typecheck,
build, and the final verification gate. The duplicate serial local `pnpm
test:run` was stopped after full CI coverage passed; it is not reported
as a completed local pass.
- Greptile: 5/5 with both findings resolved. The security scan also
passed. No failing or pending PR checks remain.

## Risks

- Display state can be stale for thirty seconds. Destructive guards
bypass the display cache.
- Undelivered or otherwise ineligible terminal work retries after five
minutes instead of each heartbeat tick.
- Transaction signals cover the owning `createDb` scope. Direct SQL and
independent server processes require startup recovery.
- Local cooldown timers are maintenance hints. They do not establish an
external wake alarm or whole-instance sleep safety. Existing blockers
remain in force; failed bounded recovery keeps the host awake.

## Model Used

OpenAI GPT-6, model ID `gpt-6`, through Codex. Reasoning, repository
inspection, shell execution, and browser tools assisted the change. The
runtime does not expose the context-window size.

## 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-14 08:28:45 -05:00