From 8087661bb891d37aa511a9572fceee5a768b7348 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 17 Aug 2026 23:11:30 -0400 Subject: [PATCH] fix: bound workspace Git scans (#11572) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Workspaces let users and agents inspect files that belong to an issue > - Changed-file views use full-tree Git status scans > - Many issue views could start those scans at the same time and make the server unresponsive > - Route-level limits did not protect the process or coalesce work for one repository > - This pull request adds one bounded scheduler for every expensive workspace Git scan > - It also starts browser scans only when the file panel is open and visible > - The benefit is bounded child-process use and responsive health checks during request storms ## Linked Issues or Issue Description **What happened?** Many changed-file requests could start full `git status --porcelain=v1 -z --untracked-files=all` scans at the same time. One production incident produced about 270 direct Git child processes. The Node process stayed alive but stopped answering health requests in time. **Expected behavior** Paperclip must bound expensive Git work across all companies, actors, issues, repositories, and browser tabs. Duplicate requests for one worktree must share work. Excess requests must fail fast with a retryable response. Hidden or closed file panels must not start scans. **Steps to reproduce** 1. Open changed-file views for many issue and actor keys. 2. Send requests for two large workspace roots at the same time. 3. Observe that route-level limiter keys allow many full Git scans to run together. 4. Observe delayed health responses and accumulated Git children. **Paperclip version or commit** Reproduced on master before commit `43ab441f0f`. **Deployment mode** Self-hosted server with local workspace repositories. ## What Changed - Add a process-wide scheduler with configurable concurrency, queue capacity, timeout, and cache TTL. - Add fair admission, a bounded queue, canonical worktree keys, single-flight joins, and bounded result caching. - Add subprocess timeouts, TERM-to-KILL escalation, bounded output, waiter cancellation, and slot cleanup. - Route full-tree status work from file resources, workspace runtime, execution workspaces, and adapter overlay sync through the scheduler. - Return stable retryable `503` and `504` error codes for saturation and timeout. - Add structured logs with safe workspace hashes, durations, queue state, cache use, joins, and terminal outcomes. - Gate UI queries on panel and document visibility. Cancel queries on close, hide, unmount, and workspace change. - Disable focus and reconnect bursts. Keep one explicit refresh action and a retryable unavailable state. - Document the 10-second default freshness tradeoff and all configuration variables. - Add unit, route, UI, adapter, and deterministic 500-request load coverage. ## Verification - `pnpm -r typecheck` - `pnpm build` - `pnpm check:token-gates` - `pnpm --filter @paperclipai/server exec vitest run src/services/workspace-git-operation-scheduler.test.ts src/__tests__/file-resources-git-scan-load.test.ts --reporter=dot` — 16 tests passed. - `pnpm --filter @paperclipai/ui exec vitest run src/components/WorkspaceFileBrowser.test.tsx src/lib/page-visibility.test.ts --reporter=dot` — 38 tests passed. - `pnpm --filter @paperclipai/adapter-utils exec vitest run src/git-workspace-sync.test.ts --reporter=dot` — 16 tests passed. - Existing file-resource, workspace-runtime, and execution-workspace regression selections passed. - Two cleanup safety regressions prove failed scans preserve the worktree before archive and at the final deletion fence. - Before: the incident produced about 270 Git children and health requests timed out. - After: 500 concurrent requests across 500 issue keys, 73 actors, and two roots started two underlying scans. Peak scan concurrency was 2. All 500 requests succeeded. Health p99 was 4.94 ms. The harness found zero unreaped children. - The full local Vitest run passed 4,267 tests. Ten existing fixed-port HTTPS exposure tests could not run because this host already owns Tailnet listeners on ports 42000 and 52000. Clean GitHub CI is the final full-suite result. - Latest-head GitHub CI passed all required test, typecheck, build, canary, e2e, policy, and security gates. - Greptile completed at 5/5 with zero unresolved comments, recommendations, or follow-ups. ## Risks - Changed-file results can be up to 10 seconds old by default. Explicit refresh remains available. - A full queue returns a retryable `503` instead of waiting without a bound. - A scan that exceeds the default 8-second deadline returns a retryable `504` and terminates its process group. - Operators can tune all limits with documented environment variables. Safe defaults protect local and shared servers. > 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 Codex, GPT-5 family. The runtime does not expose the exact deployment ID or context-window size. High reasoning, tool use, and code execution were enabled. ## 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 --- .env.example | 6 + doc/DEVELOPING.md | 15 + packages/adapter-utils/CHANGELOG.md | 6 + .../src/git-workspace-sync.test.ts | 27 + .../adapter-utils/src/git-workspace-sync.ts | 49 +- server/CHANGELOG.md | 6 + .../execution-workspaces-service.test.ts | 49 + .../file-resources-git-scan-load.test.ts | 252 +++++ server/src/routes/file-resources.ts | 53 +- server/src/services/execution-workspaces.ts | 64 +- server/src/services/index.ts | 6 + .../src/services/workspace-file-resources.ts | 49 +- .../workspace-git-operation-scheduler.test.ts | 424 +++++++++ .../workspace-git-operation-scheduler.ts | 891 ++++++++++++++++++ server/src/services/workspace-runtime.ts | 52 +- ui/src/api/file-resources.ts | 9 +- ui/src/components/FileViewerSheet.tsx | 2 + .../components/WorkspaceFileBrowser.test.tsx | 109 +++ ui/src/components/WorkspaceFileBrowser.tsx | 153 ++- ui/src/lib/page-visibility.test.ts | 9 + ui/src/lib/page-visibility.ts | 5 + 21 files changed, 2158 insertions(+), 78 deletions(-) create mode 100644 server/src/__tests__/file-resources-git-scan-load.test.ts create mode 100644 server/src/services/workspace-git-operation-scheduler.test.ts create mode 100644 server/src/services/workspace-git-operation-scheduler.ts diff --git a/.env.example b/.env.example index e747df0914..ddd69484ba 100644 --- a/.env.example +++ b/.env.example @@ -4,5 +4,11 @@ SERVE_UI=false BETTER_AUTH_SECRET=paperclip-dev-secret PAPERCLIP_TOOL_ACTION_SIGNING_SECRET=paperclip-dev-tool-action-signing-secret-change-me +# Process-wide protection for expensive full-tree workspace Git scans. +# PAPERCLIP_WORKSPACE_GIT_SCAN_CONCURRENCY=2 +# PAPERCLIP_WORKSPACE_GIT_SCAN_QUEUE_CAPACITY=32 +# PAPERCLIP_WORKSPACE_GIT_SCAN_TIMEOUT_MS=8000 +# PAPERCLIP_WORKSPACE_GIT_SCAN_CACHE_TTL_MS=10000 + # Discord webhook for daily merge digest (scripts/discord-daily-digest.sh) # DISCORD_WEBHOOK_URL=https://discord.com/api/webhooks/... diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 5887970f06..dd287b1ee5 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -425,6 +425,21 @@ Agent, project, environment, secret, skill, and workspace config edits are sampl When effective run config changes, Paperclip may intentionally skip a saved adapter session, refresh persisted workspace runtime config, replace a reused execution workspace, or avoid reusing a sandbox/environment lease. Fresh execution can lose adapter-specific session, workspace, or sandbox state; correctness of the next run's config takes priority over continuity. Plain environment values affect freshness through value hashes; run result JSON and workspace operation logs expose only the non-sensitive freshness decision categories, without storing secret values, full env maps, provider credentials, or private path details. +## Workspace Git Scan Protection + +Paperclip applies one process-wide scheduler to expensive host-side workspace Git enumeration, including changed-file browsing, runtime/finalization cleanliness guards, and adapter sandbox-sync snapshots. The scheduler defaults to two active scans and a bounded queue of 32. Identical scans of the same canonical worktree share one subprocess, while successful changed-file listings are cached for 10 seconds. Correctness-sensitive runtime guards bypass the result cache. + +The cache intentionally trades up to a few seconds of changed-file freshness for stable server latency. The file browser retains an explicit refresh action, does not start its query while the panel or browser tab is hidden, and presents overloads as retryable failures rather than an empty workspace. A full queue returns `503` with code `workspace_git_scan_saturated`; a scan exceeding its wall-clock limit returns `504` with code `workspace_git_scan_timeout`. Both responses include `Retry-After: 1`. + +Environment overrides: + +- `PAPERCLIP_WORKSPACE_GIT_SCAN_CONCURRENCY` (default `2`, range `1`–`16`) +- `PAPERCLIP_WORKSPACE_GIT_SCAN_QUEUE_CAPACITY` (default `32`, range `0`–`1024`) +- `PAPERCLIP_WORKSPACE_GIT_SCAN_TIMEOUT_MS` (default `8000`, range `100`–`120000`) +- `PAPERCLIP_WORKSPACE_GIT_SCAN_CACHE_TTL_MS` (default `10000`, range `0`–`60000`) + +Structured `workspace_git_scan` logs expose the operation name, a non-reversible workspace-path hash, queue and execution durations, active/queued counts, cache and single-flight use, and terminal outcome. Saturation and timeout warnings are rate-limited so an overload does not create a second logging storm. + ## Worktree-local Instances When developing from multiple git worktrees, do not point two Paperclip servers at the same embedded PostgreSQL data directory. diff --git a/packages/adapter-utils/CHANGELOG.md b/packages/adapter-utils/CHANGELOG.md index 76cabbd73f..a59d037528 100644 --- a/packages/adapter-utils/CHANGELOG.md +++ b/packages/adapter-utils/CHANGELOG.md @@ -1,5 +1,11 @@ # @paperclipai/adapter-utils +## Unreleased + +### Patch Changes + +- Allow the Paperclip host to route adapter sandbox-sync full-tree Git enumeration through its process-wide bounded scheduler. + ## 0.3.1 ### Patch Changes diff --git a/packages/adapter-utils/src/git-workspace-sync.test.ts b/packages/adapter-utils/src/git-workspace-sync.test.ts index 3f5b70ca8c..1a233e4c5a 100644 --- a/packages/adapter-utils/src/git-workspace-sync.test.ts +++ b/packages/adapter-utils/src/git-workspace-sync.test.ts @@ -15,6 +15,7 @@ import { readGitWorkspaceSnapshot, runLocalGit, sanitizeGitRemoteUrl, + setExpensiveWorkspaceGitExecutor, withShallowGitWorkspaceClone, } from "./git-workspace-sync.js"; @@ -28,6 +29,7 @@ describe("git workspace sync", () => { const cleanupDirs: string[] = []; afterEach(async () => { + setExpensiveWorkspaceGitExecutor(null); while (cleanupDirs.length > 0) { const dir = cleanupDirs.pop(); if (!dir) continue; @@ -35,6 +37,31 @@ describe("git workspace sync", () => { } }); + it("delegates every host-side full-tree enumeration to the registered scheduler", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-scheduler-hook-")); + cleanupDirs.push(rootDir); + const repo = await createRepo(rootDir); + await writeFile(path.join(repo, "untracked.txt"), "untracked\n", "utf8"); + const operations: string[] = []; + setExpensiveWorkspaceGitExecutor(async (input) => { + operations.push(input.operation); + return await runLocalGit(input.localDir, [...input.args], { + timeout: input.timeout, + maxBuffer: input.maxBuffer, + }); + }); + + const snapshot = await readGitWorkspaceSnapshot(repo); + + expect(snapshot?.overlayPaths).toContain("untracked.txt"); + expect(operations.sort()).toEqual([ + "adapter_sync.deleted_files", + "adapter_sync.ignored_files", + "adapter_sync.overlay_diff", + "adapter_sync.untracked_files", + ]); + }); + async function createRepo(rootDir: string): Promise { const repo = path.join(rootDir, "repo"); await mkdir(repo, { recursive: true }); diff --git a/packages/adapter-utils/src/git-workspace-sync.ts b/packages/adapter-utils/src/git-workspace-sync.ts index dd0517ec41..fd3525ecc0 100644 --- a/packages/adapter-utils/src/git-workspace-sync.ts +++ b/packages/adapter-utils/src/git-workspace-sync.ts @@ -17,6 +17,29 @@ export interface GitWorkspaceSnapshot { ignoredPaths: string[]; } +export interface ExpensiveWorkspaceGitInput { + localDir: string; + args: readonly string[]; + operation: string; + timeout: number; + maxBuffer: number; +} + +export type ExpensiveWorkspaceGitExecutor = ( + input: ExpensiveWorkspaceGitInput, +) => Promise; + +let expensiveWorkspaceGitExecutor: ExpensiveWorkspaceGitExecutor | null = null; + +/** + * Lets a host process apply its process-wide admission policy to the adapter + * package's full-tree Git walks. Standalone adapter-utils consumers retain the + * existing timeout/buffer-bounded fallback. + */ +export function setExpensiveWorkspaceGitExecutor(executor: ExpensiveWorkspaceGitExecutor | null): void { + expensiveWorkspaceGitExecutor = executor; +} + export const GIT_ARCHIVE_EXCLUDES = [".git", ".git/*"] as const; function shellQuote(value: string) { @@ -53,6 +76,24 @@ export async function runLocalGit( }); } +async function runExpensiveWorkspaceGit( + localDir: string, + args: string[], + operation: string, + options: { timeout: number; maxBuffer: number }, +): Promise { + if (expensiveWorkspaceGitExecutor) { + return await expensiveWorkspaceGitExecutor({ + localDir, + args, + operation, + timeout: options.timeout, + maxBuffer: options.maxBuffer, + }); + } + return await runLocalGit(localDir, args, options); +} + export async function readGitWorkspaceSnapshot(localDir: string): Promise { try { const insideWorkTree = await runLocalGit(localDir, ["rev-parse", "--is-inside-work-tree"], { @@ -72,19 +113,19 @@ export async function readGitWorkspaceSnapshot(localDir: string): Promise