2 Commits

Include renames

Author	SHA1	Message	Date
DottaandPaperclip	e801be7aa9	fix: fence nested Git repositories during workspace migration (#19805) ... ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Online workspace migration must preserve local files before it assigns remote authority. > - A task directory can contain a Git repository below its root. > - The source inspector treated that directory as ordinary, but the archive owner correctly required a shared Git source fence. > - This change includes each nested repository in the existing source ownership and locking checks. > - The archive still captures the full task directory, including sibling files and staged and unstaged Git work. ## Linked Issues or Issue Description Related: #19688 and #19409. Those changes cover physical source fences and nested Git archive reconstruction. This change repairs source classification for an ordinary wrapper that contains a repository. No duplicate fix was found. **What happened?** Online migration admitted a managed task directory whose root had no `.git` marker. A repository below `.review/repository` then reached the archive owner without a shared Git source fence. Capture failed with `remote_workspace_archive_online_source_git_requires_shared_fence`. Its operation and pending snapshot remained available for canonical resume. **Expected behavior** Migration must include the nested Git stores and all their source owners in the existing physical fence. It must preserve the wrapper directory and sibling files. It must refuse foreign aliases, unowned stores, busy shared sources, and source changes. **Steps to reproduce** 1. Create a managed ordinary task directory with a sibling file. 2. Create a real Git repository below `.review/repository` and leave staged and unstaged edits. 3. Run canonical online workspace migration. 4. Observe the shared-fence refusal, instead of a durable archive that can restore all the work. ## What Changed - Add a bounded metadata walk for nested Git roots. Reuse the existing strict Git pointer, common-directory, and alternate-store reader. - Add nested source identities to the existing alias closure, company checks, source locks, and metadata fingerprints. Keep the wrapper as the capture root. - Validate bounded Git-maintained reverse references in selected and borrowed stores. Require stable two-way pointers and containment by included registered workspaces. Bind the listings and pointer receipts into publication revalidation. - Add seventeen integration cases for fresh migration, pending-journal resume, shared linked and alternate stores, unrelated progress, foreign and unowned stores, and late metadata changes. ## Verification - Genuine baseline RED: both fresh and pending-journal cases failed at the existing archive shared-fence guard before the source fix. - The first treatment passed six of seven cases. The late-change case correctly failed at the earlier recapture guard; its expected-error assertion was corrected. The final test also verifies that the canonical Git fingerprint changed. - Head `8bb9f5a3ce`: real PostgreSQL, all **187 tests passed** in 59.22 seconds across `remote-workspace-migration.test.ts` and `workspace-git-source-roots.test.ts`. The canonical wrapper creates disposable databases and completed cleanup. Run from `server/`: ```sh node ../cli/node_modules/tsx/dist/cli.mjs ../packages/db/src/test-postgres-command.ts -- node ../node_modules/vitest/vitest.mjs run src/services/remote-workspace-migration.test.ts src/services/workspace-git-source-roots.test.ts --hookTimeout 90000 --maxWorkers 1 --no-file-parallelism --fsModuleCache ``` - Required alias RED: both hidden ordinary-wrapper cases failed before the reverse-reference fix. The shared case bypassed its held writer lock; the foreign case was incorrectly eligible. Both now pass. A further RED proved the same omission through a borrowed store; its hidden writer is now included too. - Review RED: a repository below `.review/.paperclip-runtime` reached the archive guard. Discovery now excludes the runtime name only at the capture root, matching the archive. - The positive cases download the authenticated immutable archive, remove the disposable source, and cold restore the sibling file, HEAD, index content, and working content. Resume retains the exact operation and pending snapshot IDs. - Resume fidelity: migration, snapshot, and layout owners are byte-identical to source revision `2d4f80504e`. Source kind comes from the workspace row, not online classification. The unchanged serializer writes the same empty source, source fingerprint, marker, fixed empty child scope, and pending generation-1/epoch-1 snapshot at the tested failure boundary. The test asserts that shape and retains the exact IDs on resume. The fixture injects the failure at that unchanged boundary; it does not execute the old binary. - Shared-store cases hold a real PostgreSQL source lock. The related import is refused before a journal exists, while an unrelated ordinary import completes. - Invalid-link controls reject foreign and unregistered roots, malformed pointers, and symlinked metadata before a journal exists. A new detached worktree after upload changes the listing fingerprint and blocks authority publication. - Final head `e525b6b1a5`: all 17 focused real PostgreSQL cases pass in 16.23 seconds after the enclosing-host-repository review fix. Its true RED first showed the valid managed wrapper as ineligible. The cold restore now excludes the ancestor Git directory and host-only file and leaves the host HEAD unchanged. Server TypeScript passes again. - Server TypeScript and module boundary checks pass. Canonical agent preflight passes. Environment: macOS arm64, Node 24.22.2, real PostgreSQL and Git. - A first test launch lacked the built plugin SDK, and a later sandbox launch could not open TSX IPC. Both were setup failures, not regression results. The existing dependency build and permitted isolated test runner resolved them. - Final exact-head CI passes: [run 42363940320](https://github.com/paperclipai/paperclip/actions/runs/42363940320). The final gate has 56 successful checks, four skipped checks, one neutral check, and no pending or failed checks. Greptile reviewed this head at 5/5, with zero open review threads. No hosted workspace was changed or imported by this PR work. ## Risks - Nested discovery and reverse-reference checks add bounded directory metadata I/O. Reverse listings allow at most 256 worktrees per store, and all links must resolve to registered physical workspace roots. Stale or unsupported links fail closed. - Nested discovery refuses more than 100,000 entries, 64 directory levels, or 64 repositories. A large or unsupported tree remains ineligible. This includes dependency or build trees that exceed those limits. No broad directory skip or metadata cache bypasses final source checks. - The archive guard, source lock owner, runtime writer checks, publication checks, and recovery predicates remain required. This change grants no permission to retire source work or ignore a held journal. - No schema, API, runtime setting, provider lifecycle, or background worker changes. - This PR documents the internal source-classification contract above. It adds no operator command or configuration to document separately. ## Model Used OpenAI `gpt-6.1-sol` through Codex, with reasoning, repository tools, code execution, and regression tests. The context window size is not exposed. ## 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>	2026-11-07 04:40:08 -06:00
DottaandPaperclip	d5c4f7dd02	fix: preserve shared Git sources during online workspace migration (#19688) ... ## Thinking Path > - Paperclip preserves the work produced by its agents. > - Remote workspace migration must preserve legacy Git history and local edits. > - Linked checkouts can share administration and objects with other tasks. > - A task lock alone cannot exclude writers of a shared or borrowed store. > - The online source owner must derive and retain the full physical ownership closure. > - This change reuses task, project, lifecycle and finalizer locks for that closure. > - Idle legacy Git work can then migrate while unrelated remote turns continue. ## Linked Issues or Issue Description Refs: #19642. **What happened?** Online migration keeps legacy Git sources blocked because several task checkouts can share a project base or a private Git cache. A read-only source observation found 160 pending Git records, 120 distinct paths and nine common stores. Its largest project cohort contained 126 records and 110 task owners. An ordinary-directory fence cannot preserve these sources by itself. **Expected behavior** Derive the physical source owners from stable Git metadata. Hold their existing source and lifecycle keys through publication. Retain all history and recovery obligations. Reject foreign aliases, unknown stores and changed sources. Let unrelated object-backed work continue. **Steps to reproduce** 1. Create a managed project checkout and linked task worktrees. 2. Keep a settled prepared recovery source on a sibling task. 3. Keep unrelated remote work running. 4. Request an online import of an idle leaf checkout. 5. The existing owner keeps the shared source blocked. **Paperclip version or commit** This branch builds on merged #19642 (`7ac99718a4`) and fresh master `fb3cf1a072`. It adds the Git closure. It does not add a writers-stopped shortcut. **Deployment mode** Hosted native Runner with remote workspace storage. Local qualification uses owned synthetic Git repositories, disposable PostgreSQL databases and the actual archive and storage owners. ## What Changed - Read bounded stable Git pointers, common-directory metadata and local alternates. Derive the complete physical source ownership cohort. Preserve all object stores. - Retain common and borrowed store owners in preparation, lifecycle, runtime, expiration and actual reopen paths. Keep company boundaries and the existing 128-key bound. - Hold exact sibling native recovery proofs under their existing physical finalizer keys. Recheck descriptors, allocations, recovery actions and source identities before capture and publication. Preserve original states and warnings. - Require leaf-first nested imports. Reuse the existing immutable child download, restore and source comparison before parent exclusion. - Bind the self-contained archive owner to the held Git namespaces. Keep its three walks, history normalization, fsck, bytes and digest format. Reject bare, detached, network, unknown or changed Git storage. - Check both foreign path fields. Keep foreign metadata private. Document the proof, bounds, source observation and remaining eligibility limits. ## Verification - Final revision `2934e41c923b93e16508b978c849b747c86be645`: all checks are terminal, with 56 successful and four skipped, zero failures and zero pending. The [required CI aggregate](https://github.com/paperclipai/paperclip/actions/runs/42326464408/job/128266279210) passed. Build, types, all server/Runner/serialized suites, end-to-end aggregate and Snyk passed. [Fresh exact-head Greptile review](https://github.com/paperclipai/paperclip/pull/19688#issuecomment-6699760104) is 5/5 with no open threads. GitHub reports MERGEABLE and CLEAN. - Review then found two correctness gaps: an ordinary managed directory inside an unrelated host repository was blocked, and a Git parent could miss a completed nested operation after its child snapshot became missing or deleted. Three actual-owner regressions reproduced both gaps in 9.70 seconds. The corrected owner passes 12 focused cases in 17.08 seconds. It imports and restores only the ordinary root bytes, preserves the outer repository, rejects both missing/deleted child receipts before a parent journal, and retains valid/changed child coverage and private-cache contention. Its wrapper exited 0 and cleanup completed. The first bootstrap attempt failed before tests ran because the host exhausted its 32 shared-memory segment limit; no shared services were stopped and no pass is claimed for that attempt. The final qualified source/test type graph, module boundaries and diff checks pass. - Receipt scans keep canonical path resolution. A lexical SQL prefix can omit children reached through symlinked ancestors. The observed full metadata closure completed in 1.17 seconds; no measured bottleneck justifies weakening that alias proof. Writer admission still rejects uncertain Git ownership before mutation. Expiration returns its typed skip, and reopen returns its structured failure. Runtime and lifecycle do not bypass an unknown source. No broad error-contract change was added. - Full CI at `720104ea975e` exposed five existing boundary cases across three server shards. Four exact cases reproduced locally as failures in 25.98 seconds. Reopen now preserves its structured failure when source inspection rejects the path, without reconstructing it. Runtime recognizes only the exact regular ignore-only `.git/info/exclude` fragment; migration still rejects that fragment. The original arbitrary-path and file-URL import/delete-host/resume fixtures keep both supported input forms and use the genuine offline setup path after their sole writer finishes. The production mode gate is unchanged. A new real-owner control proves the offline flag still rejects a running legacy database writer. - The final focused run passed all ten cases across four suites in 22.75 seconds, including all five CI failures, invalid-pointer admission, fragment rejection, offline writer exclusion and actual nested reopen lock ordering. Its wrapper exited 0 and cleanup completed. The known invalid-pointer case remains fail-closed; the separate unexpected-I/O diagnostic retains safe error metadata without command text or credentials. Existing Git stderr redaction coverage remains intact. The final metadata reader suite passed 20 cases in 2.68 seconds. The qualified server/test source type graph, module boundaries and diff checks pass. No production assertions or timeout defaults were weakened. Fresh CI and review are required for the repair revision. - A later review found that the ordinary early return still used only one foreign path field. The actual ordinary providerRef-only regression failed as incorrectly eligible. One source-path owner now enumerates both local locators for company/foreign ordinary and Git scans, layout checks, early admission, lifecycle/reopen/runtime and expiration/move/preparation retention. Capture and display keep their original preferred path. The actual owner canary passed 49 cases across two suites in 109.15 seconds. It covers cwd-only/ref-only/different/symlink locators, same-company mismatches, foreign owners, lifecycle ordering, Git/recovery imports and expiration/reopen. Three selected-locator fallback cases also failed before their guard; the final eight-case control passed in 31.05 seconds, including the configured alias and valid import/private-child controls. All wrappers exited 0 and completed cleanup. The metadata/locator suite passed 19 cases. The final source/test type graph, module boundaries and diff checks pass. - Review found two P1 gaps. Both configured-directory symlink cases failed before canonicalization. The actual parent import lost an already-imported child's unique private cache and failed its archive fence instead of retaining that owner. The corrected real-owner canary passed four cases in 20.78 seconds: configured symlink runtime provisioning, private-cache contention before the parent journal, successful parent publication after release, and unchanged/changed child coverage. The metadata suite passed 15 cases in 4.14 seconds. Both wrappers exited 0 and cleanup completed. - A fresh source read-only observation found 160 pending Git records and nine common stores. An additional exact receipt query found 1,243 completed migration receipts and 557 nested source paths under pending Git roots. All 126 records in the largest project were present in the captured Git metadata set. For all 110 present candidate workspace records, the derived closure fits the unchanged 128-key cap. The worst case needs 110 issue keys and one project key, with 17 keys of headroom. Its 173 imported children introduce no physical issue owner outside those 110 logical task owners. This checks the observed key bound only. It does not claim migration eligibility or a coherent snapshot. - The initial stacked CI did not run four Runner test lanes: trusted master passed new --package-lane/--native-server options to the old base lane script. The branch is now rebased onto actual merged master and retargeted before synchronization. It includes the canonical current lane parser without an unrelated Runner patch. Fresh new-head CI is required. - The actual importer/source owner run passed 36 cases across three suites in 187.75 seconds. It covers sibling prepared recovery, unrelated running remote work, common-cache contention, changed coordinator/allocation/path state during upload, exact nested coverage, dirty unpushed work, and existing expiration/reopen owners. Wrapper exit 0 and cleanup completed. - A separate previously hanging lifecycle-worker restore passed in 17.33 seconds with the unchanged 15-second test-body limit. An earlier run timed out in three expiration/reopen cases and blocked cleanup; its wrapper later exited 1. These failures remain recorded. They are not counted as passes. - Two boundary controls first failed: a foreign providerRef-only alias was incorrectly eligible, and actual reopen did not admit its distinct cache owner at the initial source boundary. The narrow guards now inspect both foreign locators and derive physical Git keys before the raw lifecycle transaction. The foreign alias and existing real nested reopen contention passed. The private-cache control first retained contradictory recorded source provenance and stayed fail-closed after release. The corrected present legacy-worktree fixture passed in 12.61 seconds. These failed attempts remain recorded; they are not passes. - The metadata reader and serialized/local archive controls use actual Git repositories. They compare archive bytes and digests, remove every original source, restore staged and unstaged work, and run fsck. The metadata reader passed 13 cases in 2.85 seconds. The local/serialized archive controls passed nine cases in 9.27 seconds. All processes exited 0. An additional actual importer control changed a relative commondir pointer to the absolute name of the same store during upload. Publication was rejected, the uploaded journal remained available, and original files stayed intact (one case, 5.86 seconds; wrapper exit 0 and cleanup complete). - The final rebased full local server/test type graph passes with db/shared/Runner source aliases because reused dependency declarations predate new master exports. The first rebased attempt reported those stale declaration fields/tables, and is not counted as a pass. Module boundaries and diff checks pass. Standard package builds, types and broad regression checks remain required in CI. - No hosted import, admission hold, cancellation, provider mutation, source filesystem write or shared-cache Git command was performed for this change. The source observation is read-only and is not a locked eligibility proof. ## Risks - A missing writer owner could make physical source exclusion incomplete. Existing mutation owners retain the same company/task/project keys, and uncertain metadata fails closed. - A project can contain a large cohort. Limits remain finite. A bound failure keeps the source blocked. - Bare, detached, unowned or network Git stores remain blocked online. Missing archived sources retain their existing authenticated reconstruction path. - Exact recovery proofs preserve authority and obligations. They do not infer provider termination or mark a prepared descriptor finalized. - This change does not claim all observed Git records are immediately eligible. Existing preparation, provider, recovery, runtime and source-change blockers remain. ## Model Used OpenAI GPT-6 through Codex, with reasoning, tool use and code execution. The host does not expose a more specific backend model ID or context-window size. ## 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>	2026-11-06 20:08:19 -06:00