fix(workspaces): keep cleanup Git scans fresh and dedupe page reads (#16302)

## Thinking Path

> - Paperclip manages execution workspaces for agent tasks.
> - It must preserve work when it closes a workspace.
> - The terminal-workspace worker checks Git before it decides whether a
workspace can close.
> - A request for fresh Git status could join a scan that started before
a file changed.
> - Several records for one directory can also repeat the same Git work
in a bounded page.
> - The worker already reads the task tree before Git, then reads it
again for delivery assessment.
> - This pull request fixes the fresh-read boundary and removes those
repeated reads.

## Linked Issues or Issue Description

- Refs #14759 and #14423 for the background Git scan load.
- Refs #16282 for the merged event-driven terminal-workspace worker.
- Refs #13619 for the original community investigation and tests. Thanks
to @fayonation for identifying the shared-directory and in-flight scan
cases. This implementation starts from current master and adapts those
ideas to the new worker.

## What Changed

- Give correctness-sensitive Git status requests their own scheduler
scan. They skip completed cache entries and older scans in progress.
- Share compatible Git readiness results across workspace records in one
bounded worker page. The key includes company, path, branch, base ref,
and runtime ownership. Each record keeps its own task and delivery
assessment.
- Reuse the task tree already read by the worker during delivery
assessment.
- Add regression tests for a file created during an older scan,
compatible scan sharing, and company and branch separation. Update
developer and workspace map documentation.

## Verification

- `pnpm exec vitest run
server/src/services/workspace-git-operation-scheduler.test.ts
server/src/__tests__/execution-workspaces-service.test.ts` — 92 tests
passed.
- `pnpm -r typecheck` — passed with the repository's pnpm 9.15.4.
- `node scripts/check-maps.mjs` — passed.
- `pnpm build` — passed.
- The full local `pnpm test:run` was interrupted when its original
worktree disappeared. All PR test shards passed in GitHub CI.
- Latest commit `e7622127c7d978a9a90857112675834725965d72`: 53
successful PR checks, 2 expected skips, and no failures. Greptile
reviewed this commit at 5/5 with no open findings.

## Risks

- A shared page inspection can become stale if the directory changes
before the next record is assessed. The final cleanup still starts a
separate Git scan and checks current task, ownership, and lifecycle
state under the existing guards.
- The new scan key changes request coalescing for correctness-sensitive
callers. They still use the scheduler's concurrency and queue limits.
There is no database migration or API change.

> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.

## Model Used

- OpenAI GPT-6 through Codex, with reasoning, code editing, and shell
tools. The exact serving model ID and context window are not exposed in
this session.

## 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>
This commit is contained in:
DottaandPaperclip authored and GitHub committed 2026-10-14 10:36:41 -05:00
1 parent bcfc03bc94
commit 74ba530b9e
6 files changed
+99 -15

No files matched your search

+9 -5
View File
@@ -2273,9 +2273,9 @@ The developer-server health poll also stops in hidden tabs.
Workspace detail responses share concurrent Git inspections and reuse their
results for up to thirty seconds after completion. Workspace identity and metadata
changes invalidate the cached result. The cache holds at most 256
entries. Close-readiness checks, the terminal-workspace reaper, and the final
cleanup validation still inspect Git afresh. A display result never authorizes
worktree removal.
entries. Close-readiness checks and final cleanup validation request separate
Git scans that cannot join an older in-flight scan. A display result never
authorizes worktree removal.
The terminal-workspace reaper uses the database work coordinator. Startup scans
recover existing workspace rows in bounded pages. Issue status and tree changes,
@@ -2285,8 +2285,12 @@ transaction settles and holds idle admission while writes or cleanup are active.
Empty and nonterminal-only lanes have no recurring timer or query. Terminal trees
schedule their actual cooldown deadline; unresolved delivery or cleanup eligibility
is checked again after five minutes. Nonterminal trees, cooldowns, and active runs
are checked before Git. Unchanged terminal candidates also back off for five
minutes during unrelated commit wakes. Scoped task/workspace hints invalidate
are checked before Git. The worker shares Git readiness across compatible rows
that point at one directory within a bounded page. Company, branch, base ref,
and runtime ownership must match. Each row keeps its own task and delivery
assessment, which reuses the task tree read by the early gate. Unchanged terminal
candidates also back off for five minutes during unrelated commit wakes. Scoped
task/workspace hints invalidate
affected candidates; changed tree or workspace revisions invalidate them too.
Deadline selection reuses task trees read by those pages and performs no separate
full-inventory query. Commits during a paged rotation request another bounded
+3 -1
View File
@@ -109,7 +109,9 @@ Manual: Provision a disposable workspace, wait for readiness, start an available
- Workspace IDs, runtime leases, and repository paths are not interchangeable.
- Terminal workspace cleanup wakes after committed task/workspace/PR changes and
restores cooldown deadlines at startup. Empty lanes stop scanning. Display Git
results can lag by thirty seconds; destructive cleanup still reads fresh Git.
results can lag by thirty seconds. Compatible records share one advisory Git
inspection per bounded page, while each record keeps its own delivery check.
Destructive cleanup starts a separate Git scan even if an older scan is running.
The [worker tests](../../server/src/__tests__/terminal-workspace-work.test.ts)
cover quiet lanes and deadlines. The
[service tests](../../server/src/__tests__/execution-workspaces-service.test.ts)
@@ -573,6 +573,36 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => {
expect(await fs.readFile(path.join(seed.worktreePath, "unsaved.txt"), "utf8")).toBe("keep this work\n");
});
it("shares compatible Git inspections within a batch while keeping branch and company decisions separate", async () => {
const first = await seedTerminalWorkspace();
const otherCompany = await seedTerminalWorkspace();
const [original] = await db.select().from(executionWorkspaces)
.where(eq(executionWorkspaces.id, first.executionWorkspaceId));
await db.insert(executionWorkspaces).values([
{ ...original!, id: randomUUID(), name: "Same directory", updatedAt: new Date("2020-01-01") },
{ ...original!, id: randomUUID(), name: "Different base", baseRef: "other-base", updatedAt: new Date("2020-01-01") },
]);
await db.update(executionWorkspaces).set({
cwd: first.worktreePath,
providerRef: first.worktreePath,
branchName: original!.branchName,
baseRef: original!.baseRef,
}).where(eq(executionWorkspaces.id, otherCompany.executionWorkspaceId));
const service = executionWorkspaceService(db, { workspaceReaperCooldownDays: 0 });
const scan = vi.spyOn(workspaceGitOperationScheduler, "run");
const select = vi.spyOn(db, "select");
try {
expect(await service.sweepTerminalWorkspaces()).toMatchObject({ checked: 4, archived: 0 });
expect(scan.mock.calls.filter(([input]) => input.operation === "execution_workspaces.close_readiness_status"))
.toHaveLength(3);
expect(select.mock.calls.filter(([fields]) => fields && "completedAt" in fields && "cancelledAt" in fields))
.toHaveLength(4);
} finally {
scan.mockRestore();
select.mockRestore();
}
});
it("backs off unchanged terminal candidates while unrelated commits wake the worker", async () => {
await seedTerminalWorkspace();
let clock = Date.now();
+18 -4
View File
@@ -467,6 +467,7 @@ async function runExpensiveGitStatus(input: {
operation: input.operation,
fairnessKeys: input.fairnessKeys,
cacheTtlMs: 0,
bypassSingleFlight: true,
// Nested task worktrees can exceed the scheduler's 1 MiB default.
// Keep exact file counts for readiness and reconciliation checks.
maxStdoutBytes: 32 * 1024 * 1024,
@@ -1697,8 +1698,9 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
async function assessDelivery(
workspace: ExecutionWorkspaceRow,
git: ExecutionWorkspaceCloseGitReadiness | null,
precomputedIssueTree?: Awaited<ReturnType<typeof listWorkspaceIssueTree>>,
) {
const issueTree = await listWorkspaceIssueTree(workspace);
const issueTree = precomputedIssueTree ?? await listWorkspaceIssueTree(workspace);
const sourceIssue = issueTree.find((issue) => issue.id === workspace.sourceIssueId) ?? null;
const sourceIssueTerminal = Boolean(sourceIssue && TERMINAL_ISSUE_STATUSES.has(sourceIssue.status));
const subtreeTerminal = Boolean(sourceIssue && issueTree.every((issue) => TERMINAL_ISSUE_STATUSES.has(issue.status)));
@@ -2953,6 +2955,7 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
skippedBackoff: 0,
clearedStaleReopenPending: 0,
};
const readinessByWorkingTree = new Map<string, ReturnType<typeof inspectGitCloseReadiness>>();
for (const workspace of candidates) {
signal?.throwIfAborted();
@@ -3001,13 +3004,24 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
nextAt: now().getTime() + opts.terminalWorkspaceInspectionBackoffMs,
});
}
const executionWorkspace = toExecutionWorkspace(workspace);
const { git, statusInspectionSucceeded } = await inspectGitCloseReadiness(executionWorkspace);
const workspacePath = readNullableString(workspace.providerRef) ?? readNullableString(workspace.cwd);
const createdByRuntime = workspace.providerType === "git_worktree"
? isRuntimeOwnedGitBranch(workspace.metadata)
: workspace.metadata?.createdByRuntime === true;
const readinessKey = workspacePath
? JSON.stringify([workspace.companyId, workspacePath, workspace.baseRef, workspace.branchName, createdByRuntime])
: `workspace:${workspace.id}`;
let inspection = readinessByWorkingTree.get(readinessKey);
if (!inspection) {
inspection = inspectGitCloseReadiness(toExecutionWorkspace(workspace));
readinessByWorkingTree.set(readinessKey, inspection);
}
const { git, statusInspectionSucceeded } = await inspection;
if (!statusInspectionSucceeded) {
result.skippedUndelivered += 1;
continue;
}
const assessment = await assessDelivery(workspace, git);
const assessment = await assessDelivery(workspace, git, issueTree);
if (!assessment.sourceIssueTerminal || !assessment.subtreeTerminal) {
if (reopenPending) {
// The source issue left the terminal state, so the reopen transition
@@ -241,6 +241,37 @@ describe("WorkspaceGitOperationScheduler", () => {
expect(scheduler.snapshot().inFlightCount).toBe(0);
});
it("starts a fresh scan after a file changes while an older scan is still running", async () => {
const workspace = await makeWorkspace();
const firstCaptured = deferred();
const releaseFirst = deferred();
let calls = 0;
const runner: WorkspaceGitRunner = async () => {
calls += 1;
const files = await fs.readdir(workspace);
if (calls === 1) {
firstCaptured.resolve();
await releaseFirst.promise;
}
return { stdout: files.join("\n"), stderr: "" };
};
const scheduler = createWorkspaceGitOperationScheduler({ runner, concurrency: 2 });
const input = scanInput(workspace, "freshness");
const earlier = scheduler.run(input);
await firstCaptured.promise;
await fs.writeFile(path.join(workspace, "unsaved.txt"), "keep this work\n");
const fresh = await scheduler.run({ ...input, bypassSingleFlight: true });
releaseFirst.resolve();
expect((await earlier).stdout).toBe("");
expect(fresh).toMatchObject({ stdout: "unsaved.txt", cacheHit: false, singleFlightJoined: false });
expect(scheduler.snapshot()).toMatchObject({
cacheEntryCount: 0,
totals: { started: 2, singleFlightJoins: 0 },
});
});
it("serves cached results until TTL expiry and evicts least-recently-used entries", async () => {
const firstWorkspace = await makeWorkspace("same-name");
const secondWorkspace = await makeWorkspace("same-name");
@@ -65,8 +65,8 @@ export interface WorkspaceGitScanInput {
*/
fairnessKeys?: readonly string[];
signal?: AbortSignal;
/** Successful-result cache duration. Use zero for correctness-sensitive guards. */
cacheTtlMs?: number;
bypassSingleFlight?: boolean;
/** Per-operation wall-clock deadline. Defaults to the process-wide setting. */
timeoutMs?: number;
env?: NodeJS.ProcessEnv;
@@ -364,7 +364,10 @@ export class WorkspaceGitOperationScheduler {
maxStdoutBytes,
maxStderrBytes,
});
const cacheTtlMs = input.onStdout ? 0 : clampInteger(input.cacheTtlMs, this.defaultCacheTtlMs, 0, 60_000);
const inFlightKey = input.bypassSingleFlight ? `${key}#fresh:${randomUUID()}` : key;
const cacheTtlMs = input.onStdout || input.bypassSingleFlight
? 0
: clampInteger(input.cacheTtlMs, this.defaultCacheTtlMs, 0, 60_000);
// A correctness-sensitive caller that explicitly disables caching must not
// consume a result populated earlier by the file browser.
const cached = cacheTtlMs > 0 ? this.readCache(key) : null;
@@ -390,7 +393,7 @@ export class WorkspaceGitOperationScheduler {
};
}
const existing = this.inFlight.get(key);
const existing = this.inFlight.get(inFlightKey);
if (existing) {
existing.joinCount += 1;
this.totals.singleFlightJoins += 1;
@@ -425,7 +428,7 @@ export class WorkspaceGitOperationScheduler {
...(input.fairnessKeys ?? []).filter(Boolean),
])).sort();
const scan: PendingScan = {
key,
key: inFlightKey,
operation: input.operation,
canonicalWorkspacePath,
workspaceHash,
@@ -443,7 +446,7 @@ export class WorkspaceGitOperationScheduler {
waiters: new Map(),
joinCount: 0,
};
this.inFlight.set(key, scan);
this.inFlight.set(inFlightKey, scan);
this.queue.push(scan);
const promise = this.addWaiter(scan, input.signal, false);
this.drain();