Files
PaperClipAI/server/src/__tests__/terminal-workspace-work.test.ts
T
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

142 lines
6.6 KiB
TypeScript

import { afterEach, describe, expect, it, vi } from "vitest";
import { installDatabaseWorkSignals } from "../../../packages/db/src/work-signals.js";
import { createDeliveryWorkCoordinator } from "../services/delivery-work-coordinator.js";
import { notifyTerminalWorkspaceWork, registerTerminalWorkspaceWork, TERMINAL_WORKSPACE_RETRY_MS } from "../services/terminal-workspace-work.js";
import { idleWorkSnapshot } from "../services/task-admission.js";
const coordinators: ReturnType<typeof createDeliveryWorkCoordinator>[] = [];
afterEach(async () => {
for (const coordinator of coordinators.splice(0)) await coordinator.stop();
vi.useRealTimers();
});
function setup() {
vi.useFakeTimers();
const execute = vi.fn(async () => [{ xid: "42", status: "committed" }]);
const owner = installDatabaseWorkSignals({ execute, async transaction(callback: (tx: any) => Promise<any>) {
return callback({ execute });
} });
const enabled = vi.fn(() => true), primary = vi.fn(() => true), onError = vi.fn();
const coordinator = createDeliveryWorkCoordinator({ owner,
canRun: () => enabled() && primary(), canReconcile: primary, onError,
});
coordinators.push(coordinator);
const service = {
sweepTerminalWorkspaces: vi.fn(async (_limit?: number, _signal?: AbortSignal, _options?: { compact?: boolean }) => ({ checked: 0 })),
hasTerminalWorkspaceSweepContinuation: vi.fn(() => false),
nextTerminalWorkspaceSweepAt: vi.fn(async (): Promise<number | null> => null),
};
return { owner, coordinator, service, enabled, primary, onError,
start: () => registerTerminalWorkspaceWork(coordinator, service, enabled) };
}
describe("terminal workspace work", () => {
it("recovers once at startup and makes no scans or SQL for an empty lane", async () => {
const s = setup();
const worker = s.start();
await worker.ready;
await vi.advanceTimersByTimeAsync(3_600_000);
expect(s.service.sweepTerminalWorkspaces).toHaveBeenCalledTimes(1);
expect(s.service.nextTerminalWorkspaceSweepAt).toHaveBeenCalledTimes(1);
expect(s.coordinator.nextWakeAt()).toBeNull();
expect(vi.getTimerCount()).toBe(0);
expect(idleWorkSnapshot().active).toBe(0);
expect(worker.hasUnrecoveredWork()).toBe(false);
});
it("skips compaction only in the startup sweep that readiness waits for", async () => {
const s = setup();
s.service.hasTerminalWorkspaceSweepContinuation.mockReturnValueOnce(true);
await s.start().ready;
expect(s.service.sweepTerminalWorkspaces).toHaveBeenCalledTimes(1);
expect(s.service.sweepTerminalWorkspaces.mock.calls[0]?.[2]).toEqual({ compact: false });
await vi.advanceTimersByTimeAsync(1_000);
expect(s.service.sweepTerminalWorkspaces).toHaveBeenCalledTimes(2);
expect(s.service.sweepTerminalWorkspaces.mock.calls[1]?.[2]).toEqual({ compact: true });
});
it("restores the cooldown deadline, lets commits preempt it, and parks after cleanup", async () => {
const s = setup(), due = Date.now() + 86_400_000;
s.service.nextTerminalWorkspaceSweepAt.mockResolvedValue(due);
const worker = s.start();
await worker.ready;
expect(worker.hasUnrecoveredWork()).toBe(false);
await vi.advanceTimersByTimeAsync(3_600_000);
expect(s.service.sweepTerminalWorkspaces).toHaveBeenCalledTimes(1);
await s.owner.transaction(tx => notifyTerminalWorkspaceWork(tx));
await s.owner.transaction(tx => notifyTerminalWorkspaceWork(tx));
await vi.advanceTimersByTimeAsync(1);
expect(s.service.sweepTerminalWorkspaces).toHaveBeenCalledTimes(2);
expect(s.coordinator.nextWakeAt()).toBe(due);
s.service.nextTerminalWorkspaceSweepAt.mockResolvedValue(null);
await vi.advanceTimersByTimeAsync(due - Date.now());
expect(s.service.sweepTerminalWorkspaces).toHaveBeenCalledTimes(3);
expect(vi.getTimerCount()).toBe(0);
});
it("finishes bounded recovery pages then retries only existing overdue work", async () => {
const s = setup();
s.service.hasTerminalWorkspaceSweepContinuation.mockReturnValueOnce(true);
s.service.nextTerminalWorkspaceSweepAt.mockResolvedValue(Date.now() - 1);
await s.start().ready;
expect(s.coordinator.nextWakeAt()).toBe(Date.now() + 1000);
await vi.advanceTimersByTimeAsync(1000);
expect(s.coordinator.nextWakeAt()).toBe(Date.now() + TERMINAL_WORKSPACE_RETRY_MS);
s.service.nextTerminalWorkspaceSweepAt.mockResolvedValue(null);
await vi.advanceTimersByTimeAsync(TERMINAL_WORKSPACE_RETRY_MS);
expect(s.service.sweepTerminalWorkspaces).toHaveBeenCalledTimes(3);
expect(vi.getTimerCount()).toBe(0);
});
it("keeps failed recovery visible to sleep admission until a successful empty pass", async () => {
const s = setup();
s.service.sweepTerminalWorkspaces.mockRejectedValueOnce(new Error("database unavailable"));
const worker = s.start();
await worker.ready;
expect(s.onError).toHaveBeenCalledTimes(1);
expect(worker.hasUnrecoveredWork()).toBe(true);
await vi.advanceTimersByTimeAsync(TERMINAL_WORKSPACE_RETRY_MS);
expect(worker.hasUnrecoveredWork()).toBe(false);
expect(vi.getTimerCount()).toBe(0);
});
it("defers scans and deadline SQL during standby and drain", async () => {
const s = setup();
s.primary.mockReturnValue(false);
await s.start().ready;
s.primary.mockReturnValue(true);
s.enabled.mockReturnValue(false);
await s.owner.transaction(tx => notifyTerminalWorkspaceWork(tx));
await vi.advanceTimersByTimeAsync(1);
expect(s.service.sweepTerminalWorkspaces).not.toHaveBeenCalled();
expect(s.service.nextTerminalWorkspaceSweepAt).not.toHaveBeenCalled();
s.enabled.mockReturnValue(true);
await vi.advanceTimersByTimeAsync(TERMINAL_WORKSPACE_RETRY_MS);
expect(s.service.sweepTerminalWorkspaces).toHaveBeenCalledTimes(1);
expect(vi.getTimerCount()).toBe(0);
});
it("holds admission until accepted cleanup finishes and waits for it during stop", async () => {
const s = setup();
let release!: () => void;
s.service.sweepTerminalWorkspaces.mockImplementationOnce(async (_limit, signal) => {
await new Promise<void>(resolve => { release = resolve; });
expect(signal?.aborted).toBe(true);
return { checked: 1 };
});
const worker = s.start();
await vi.advanceTimersByTimeAsync(1);
expect(idleWorkSnapshot().active).toBe(1);
let stopped = false;
const stopping = s.coordinator.stop().then(() => { stopped = true; });
await vi.advanceTimersByTimeAsync(1);
expect(stopped).toBe(false);
release();
await stopping;
await worker.ready;
expect(idleWorkSnapshot().active).toBe(0);
expect(s.service.nextTerminalWorkspaceSweepAt).not.toHaveBeenCalled();
expect(vi.getTimerCount()).toBe(0);
});
});