mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
fix(workspaces): allow larger status output for readiness checks (#14414)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The server checks workspace contents before it allows cleanup or branch reconciliation. > - These checks need the full Git status output to count untracked files. > - Nested task worktrees can make this output exceed the scheduler's default 1 MiB limit. > - A scan failure then blocks an otherwise inspectable workspace. > - This pull request raises the limit for these status checks to 32 MiB. > - The checks retain exact counts and still protect uncommitted work from cleanup. ## Linked Issues or Issue Description Refs #14194 and #14253. Those changes address snapshot enumeration. This PR addresses buffered execution-workspace status checks. Refs #13619 for separate work on caching these checks. The related journal identity failure is covered by #14312. **What happened?** Close-readiness checks failed when Git status output exceeded 1 MiB. A workspace with thousands of long untracked paths could not report its file count or complete readiness inspection. **Expected behavior** Allow up to 32 MiB of status output for execution-workspace readiness and branch reconciliation. Preserve exact counts. Continue to block cleanup when the workspace has uncommitted data or the scan exceeds its bound. **Steps to reproduce** 1. Create an execution workspace with a merged delivery. 2. Add 5,000 long untracked filenames under a nested task directory. The status output exceeds 1 MiB. 3. Request close readiness. Before this fix the status scan fails. After this fix it reports all 5,000 files. 4. Run the terminal-workspace sweep. Confirm that it preserves the workspace and files. **Paperclip version or commit** Reproduced on master at `14795136f5` before this fix. **Deployment mode** Server with managed Git workspaces. ## What Changed - Set a 32 MiB stdout bound for execution-workspace status scans. - Add a real Git regression with 5,000 long untracked paths and a cleanup-preservation assertion. Assert that the measured status output exceeds 1 MiB. - Document the bound and failure behavior. ## Verification - The new regression failed on master before the service change: the status result had no untracked files or count after the scan exceeded its bound. - `pnpm exec vitest run server/src/__tests__/execution-workspaces-service.test.ts server/src/services/workspace-git-operation-scheduler.test.ts` passed: 82 tests, including the new regression. The regression and server typecheck also passed after the explicit byte-count assertion. - `pnpm build` and `pnpm -r typecheck` passed. The full local `pnpm test:run` was attempted and stopped after the failures listed below. Greptile is 5/5 with zero unresolved review threads on the latest head. All checks for head `45f93ad5ad` passed (53 successful, two intentional skips). - Full local validation did not pass. The attempt reproduced 13 company/runtime skill-cache permission failures, the terminal-workspace cleanup assertion, and a heartbeat feedback timeout. It was stopped during the general-server stage after these failures. Remaining general-server tests, other workspace groups, and serialized-server stages did not complete locally. Earlier clean-master checks reproduced the cache failures and isolated cleanup retries passed. The latest-head GitHub suite passed all of these groups. - One GitHub browser shard initially failed because its GitHub-connection test checked the resume-action array before the mocked request completed. The single failed-job retry passed without a source change. All latest-head checks are green. - No browser suites ran locally. This change does not affect browser behavior. ## Risks - Each active status scan can buffer up to 32 MiB before parsing. The existing scheduler bounds scan concurrency and queue size. - Output above the bound still fails the scan and blocks destructive cleanup. The change does not truncate output or change cleanup rules. - There are no API, database, or snapshot-streaming changes. ## Model Used OpenAI Codex, GPT-6, with repository inspection, code editing, and test execution. The exact deployment 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 (focused suites; full local-run failures and incomplete stages are documented above) - [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:
1 parent
6f40e23536
commit
e9debd3eac
3 files changed
+32
No files matched your search
@@ -715,6 +715,8 @@ These bounds apply to application filename storage, not total process memory. Re
|
||||
|
||||
Workspace preparation resolves an existing root symlink before reading the snapshot and uses that resolved directory for the rest of the operation. Overlay staging checks the captured root identity and each selected path's ancestors before and after copying. A replaced root or a symlink in an ancestor directory stops staging before upload. A missing source file can be skipped; other source inspection errors stop staging. Selected symlink entries remain symlinks.
|
||||
|
||||
Execution-workspace close-readiness and branch-reconciliation status checks allow up to 32 MiB of buffered output. This accommodates nested task worktrees while preserving exact untracked-file counts. Larger output fails the scan and blocks destructive cleanup. Other buffered scan bounds stay unchanged.
|
||||
|
||||
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`.
|
||||
|
||||
Sandbox Git sync treats only the selected repository root as a clone source. A selected subfolder uses directory sync within that folder, applies the enclosing repository's ignore rules, and does not transfer parent files or Git history.
|
||||
|
||||
@@ -1059,6 +1059,33 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => {
|
||||
expect(workspace?.status).toBe("active");
|
||||
});
|
||||
|
||||
it("counts large untracked worktrees without allowing destructive cleanup", async () => {
|
||||
const seeded = await seedTerminalWorkspace({ mergedPr: true });
|
||||
const directory = path.join(seeded.worktreePath, ".worktrees", "task-retry");
|
||||
await fs.mkdir(directory, { recursive: true });
|
||||
for (let offset = 0; offset < 5_000; offset += 100) {
|
||||
await Promise.all(Array.from({ length: 100 }, (_, index) =>
|
||||
fs.writeFile(path.join(directory, `${offset + index}-${"source".repeat(32)}.ts`), "uncommitted\n"),
|
||||
));
|
||||
}
|
||||
|
||||
const status = await execFileAsync(
|
||||
"git",
|
||||
["-C", seeded.worktreePath, "status", "--porcelain", "--untracked-files=all"],
|
||||
{ maxBuffer: 2 * 1024 * 1024 },
|
||||
);
|
||||
expect(Buffer.byteLength(status.stdout, "utf8")).toBeGreaterThan(1024 * 1024);
|
||||
|
||||
const readiness = await svc.getCloseReadiness(seeded.executionWorkspaceId);
|
||||
expect(readiness?.git).toMatchObject({ hasUntrackedFiles: true, untrackedEntryCount: 5_000 });
|
||||
expect(readiness?.warnings).toContain("The workspace has 5000 untracked files.");
|
||||
expect(readiness?.blockingReasons).not.toContain(
|
||||
"Paperclip could not verify the workspace git status. Retry before destructive cleanup.",
|
||||
);
|
||||
expect(await svc.sweepTerminalWorkspaces()).toMatchObject({ archived: 0, skippedUndelivered: 1 });
|
||||
await expect(fs.access(seeded.worktreePath)).resolves.toBeUndefined();
|
||||
}, 20_000);
|
||||
|
||||
it("refuses cleanup when the worktree changes after delivery assessment", async () => {
|
||||
const seeded = await seedTerminalWorkspace({ mergedPr: true });
|
||||
await db.update(executionWorkspaces).set({
|
||||
|
||||
@@ -415,6 +415,9 @@ async function runExpensiveGitStatus(input: {
|
||||
operation: input.operation,
|
||||
fairnessKeys: input.fairnessKeys,
|
||||
cacheTtlMs: 0,
|
||||
// Nested task worktrees can exceed the scheduler's 1 MiB default.
|
||||
// Keep exact file counts for readiness and reconciliation checks.
|
||||
maxStdoutBytes: 32 * 1024 * 1024,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user