mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-30 13:20:54 +01:00
2d30ee8868fe653eda140a1fced8778f2a00dd69
2
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
d6d67b00d3 |
Prevent background workspace scans from refreshing the Git index (#14666)
## Thinking Path Paperclip runs background workspace scans alongside real Git writers. `git status` can refresh the index as an optional side effect, taking a lock that makes another operation fail. Disable optional locking in the shared scan subprocess so background observation does not compete with workspace updates. ## Linked Issues or Issue Description **What existing behavior does this improve?** Workspace Git scans used by changed-file browsing, cleanliness guards, and sandbox snapshots. **Current behavior** The scan process inherits Git's default optional-lock behavior. Even a clean `status` can rewrite stale stat-cache entries in the index and contend with a concurrent writer. **Proposed behavior** Always set `GIT_OPTIONAL_LOCKS=0` for the shared scan subprocess while preserving the selected environment and Git's required write locks. **Reason and benefit** Background reads stop creating avoidable index contention. Git documents this behavior and recommends disabling optional locks for background status: [background refresh](https://git-scm.com/docs/git-status#_background_refresh). **Breaking changes** None to scan results or required write locking. Later scans may repeat stat checks that would otherwise have been cached in the index. Related scan implementation: #11572, #14253. This avoids one known contention source; it does not identify every historical lock owner or repair abandoned locks. ## What Changed - Disable optional locking at the shared scan subprocess boundary, including explicit caller environments. - Test a clean status against a deliberately stale index and prove an ordinary status would rewrite it. - Test tracked/untracked results with an existing index lock, preservation of that lock and working files, and continued rejection of a mandatory-lock write. - Document the scan behavior and performance tradeoff. ## Verification - Focused stream, workspace-sync, and scheduler suites: 63 tests passed. - Full `pnpm -r typecheck` and `pnpm build` passed locally. Final head `effe6420c77bd18d36af6b093db3a564c04b8b38` passed all 53 CI checks, including complete test coverage, typecheck, build, and browser/runner gates; two unrelated checks intentionally skipped. - Full local test attempts initially had missing embedded-Postgres library symlinks; the dependency setup was repaired. Duplicate local full-suite runs were stopped after full CI completed. This PR does not claim a completed full local suite. - Review regression: real Git honors the supplied `GIT_CONFIG_*` setting and the input environment remains unchanged; all four direct subprocess cases passed. - Reviewed the diff for secrets, customer data, and internal references. ## Risks Low risk. Disabling optional index refresh can repeat filesystem stat work on later scans. Required locks remain enforced; no lock is removed, no failed reset is retried, and workspace mutation guards are unchanged. No schema changes. ## Model Used OpenAI GPT-6 via Codex, with repository inspection, code execution, and tests. Exact model build identifier is not exposed by this session. ## Checklist - [x] Thinking path and model are specified - [x] Checked ROADMAP.md; this is a maintenance correction, not planned feature work - [x] Searched for duplicate and related PRs - [x] Described the issue using the enhancement template - [x] No internal issue references, customer data, or private instance links - [x] Descriptive branch name - [x] Focused regression tests pass - [x] Added tests and updated documentation - [x] Risks documented - [x] Required validation and CI gates are green (full suite validated in CI; local scope documented above) - [x] Greptile is 5/5 with no unresolved findings - [x] I will address review comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
4b38db9622 |
fix(runtime): stream workspace Git snapshots through disk manifests (#14253)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Managed runs copy a selected workspace to an execution environment and restore its changes. > - Git snapshots select the files for that copy and for later recovery. > - A fixed output limit stops large generated trees before the run can start. > - Increasing the limit still keeps the complete filename lists in memory. > - This pull request stores those lists and merge baselines in disk manifests. > - Large snapshots can now complete with bounded filename buffers and explicit failure handling. ## Linked Issues or Issue Description Refs #14194. This is the streaming follow-up to the merged 32 MiB limit fix. Related: #13619 and #11621 cover workspace scan admission and demand. This change keeps the shared scheduler and changes the snapshot data path. ## What Changed - Stream changed, untracked, deleted, and ignored paths through the shared scheduler and the standalone adapter path. - Use SQLite manifests for file selection, duplicate removal, ignored-path lookup, baseline capture, and merge lookup. - Set a configurable 30-minute snapshot deadline. Keep the existing interactive scan deadlines. - Wait for each child process and pending sink write before removing temporary storage after failure or cancellation. - Use NUL archive lists and bounded deletion batches. Preserve unusual names, explicit selection, nested repositories, and source root checks. - Store manifest references in native recovery format v2. Check their location and digest before recovery reads. Keep v1 descriptors readable. - Remove temporary manifests at lifecycle completion. Use fixed-size temporary copy names for long basenames. - Admit each manifest with a SQLite page allowance based on current disk capacity. Keep a configurable free-space reserve and fail explicitly when either limit is reached. - Preserve a host file that replaces a directory deleted by the sandbox, and continue the rest of the restore. ## Verification - Current head `5ab622ec43cd16d35429d79dedee6a5d8e3d2df2` has 54 successful checks/statuses and two skipped Storybook jobs. No checks failed or remain pending. - [CI passed](https://github.com/paperclipai/paperclip/actions/runs/36318966368): typecheck, build, all test shards, E2E, Rust checks, and the aggregate verify job. - [Greptile is 5/5](https://github.com/paperclipai/paperclip/pull/14253#issuecomment-5855670338) on the current head. All four review threads are resolved. Security checks passed. - 229 focused tests passed across Git sync, runtime staging, merge, manifest integrity, native recovery, and the scheduler (214 adapter/runtime tests and 15 scheduler tests). - A real 40,000-file fixture produces 43,428,890 filename bytes. The original standalone and scheduled scans fail. The new test passes all four filename paths, complete staging, exclusion of late files, unusual names, and deletion replay. - Recovery tests reject changed bytes, symlinks, and paths outside the controller state directory. Adapter-utils typecheck passed. - A test executor returned buffered output and caused two retry integration failures. The fixture now uses the shared streaming scheduler. All 13 tests passed with `corepack pnpm exec vitest run server/src/__tests__/heartbeat-project-repositories.test.ts`. The same CI shard now passes. - Ran `pnpm -r typecheck`, `pnpm test:run`, and `pnpm build` locally. Each full local command hit SIGKILL/exit 137 in the 4 GiB container. These local commands did not pass. The current-head CI gates above provide the full verification. - A real-Git disk-capacity regression confirms a typed failure and removal of the incomplete manifest. Repeated writer attempts cannot exceed the permitted page count. - Follow-up real Daytona and separate staging qualification passed with the related archive validator (#14315) and exact-owner finalization fix (#14314). Three successive turns copied back all 60,000 files with 39,828,890 filename bytes and five unusual names. Independent host inventories verified every file and the pinned Git HEAD. Native, provider, session, and process identities stayed fixed; no retry remained. The task reached Done, and its browser-downloaded final proof matched exactly. The reusable regression is #14316, including an assertion of the effective environment idle policy. ## Risks - SQLite manifests use disk space. Each receives one quarter of the available capacity above the host reserve at creation. The reserve defaults to 256 MiB and has a 64 MiB configuration minimum. Disk capacity, filesystem quotas, per-path limits, Git resource use, and execution deadlines remain limits. - Each path and sink chunk has a 64 KiB limit. SQLite connections use a 1 MiB page cache. Invalid or incomplete records fail explicitly. - Restore transport keeps fixed and configured archive exclusions. A remotely created Git-ignored file can be transferred, but the host merge excludes it through the manifest. - Provider archive buffers, Git and tar memory, repository metadata, legacy v1 arrays, and the separate referenced-source resolver retain their own limits. Existing provider safety validators still buffer textual tar listings: Daytona allows 32 MiB and Kubernetes allows 64 MiB. These separate transport limits can stop a sufficiently large restore before merge. This change does not claim bounded total process memory or unlimited transport size. - New descriptors use v2. Existing v1 recovery remains supported; a downgrade cannot read v2 descriptors. ## Model Used OpenAI GPT-6 through Codex. The exact deployment ID and context limit are not exposed in this run. The agent used code editing, terminal execution, tests, and GitHub tools. ## 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> |