Repository navigation
fix(server): resolve git common dirs through symlinked subdirectories - #18152
Conversation
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a focused server bug fix that resolves Git’s relative common-directory paths against the physical working directory, correcting checkpoint and ref discovery for symlinked subdirectories while preserving ordinary behavior. Regression tests cover both affected flows, with no schema, configuration-default, or static-analysis changes. You can add or adjust custom eligibility rules. Learn more. |
|
Warning Review limit reachedOnly developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing. Next included review available in 4 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
Dismissing prior approval to re-evaluate f725ef5
c34f027 to
6572c84
Compare
…ctories Git's relative --git-common-dir is computed from the physical working directory. Joining it with path.resolve walked out of a symlink into a repository subdirectory, so checkpoint capture wrote its index into a missing directory or a different repository. Ask for an absolute common dir in checkpoint capture and repository path lookup. The regression tests create the decoy repository before the only lookup, so a cached pre-decoy result cannot hide the wrong path.
6572c84 to
7389f15
Compare
|
Taking over this one as maintainer for batch worktree-git-08. I’ll preserve the symlink-resolution fix, reduce regression coverage to the necessary behaviors, merge current main, and push follow-up commits here. Thanks @macodev00 for the reproduction and fix. |
…eckpoint-capture-redo2-9e22-v2 # Conflicts: # apps/server/src/vcs/GitVcsDriver.test.ts
Dismissing prior approval to re-evaluate 487a72b
# Conflicts: # apps/server/src/vcs/GitVcsDriver.test.ts # apps/server/src/vcs/GitVcsDriver.ts
Dismissing prior approval to re-evaluate a476a75
…pingdotgg#18152) Co-authored-by: macodev00 <macodev00@users.noreply.github.com> Co-authored-by: Julius Marminge <julius0216@outlook.com> (cherry picked from commit 6e1bf41)
Merges `pingdotgg/t3code` `main` at `e64766c56a` (367 commits from `143ab1606d`) into the fork. It branches from `main`; no other merge PR is open. ## Numbers - Landed: 2630 files from `git diff --stat HEAD^1 HEAD`, against 2629 in the upstream range. The gap: - Kept deleted: `pair.ts`, `pair.test.ts` and `useDiscoveredLocalServers.test.ts`. - Kept as fork files: `branding.test.ts` and `addBrowserSurface.test.ts`. - Extra on the merge side: four ported fork browser tests, `fork/ThreadParentLine.tsx` and `inventory.json`. - Fork delta: 812 files, down from 817. ## Conflicts (40, each by its preflight verdict) - **Upstream's test prune (pingdotgg#22557):** upstream's deletions win, fork-only tests stay, and fork adaptations of deleted upstream tests are dropped. - Upstream's deletion accepted for the `ThreadDetailsPrRows`, `PreviewEmptyState`, `previewEmptyStateLogic`, `projectScriptEditor.permissions` and `terminalSessionAvailability` tests. - Four fork browser tests are ported to the new `require-disposable-render` pattern: ThreadForkDialog, MoatlessArtifactChip, PreviewView.frame and MobileSidebarDrawer. - **`threadRoutes.ts`, `ThreadRouteView.tsx`, `ChatView.tsx`:** take upstream's `unlistedThreadDetail` (pingdotgg#19724), plus three fork changes: - An unlisted thread is "missing" once its subscription has failed. - A closed thread is not redirected by `archivedAt`, because it is absent from a Moatless listing and still opens by URL. - `onOpenRelatedThread` shows its toast only for a deleted shell. The `unlisted-thread-adoption` inventory entry now owns `threadRoutes.test.ts` and `ChatView.tsx`. - **`AGENTS.md` (`decide`):** kept the fork bullet and appended upstream's test rules and browser-test section. - **Upstream's change with the fork delta re-stated:** `BranchToolbar` (`showWorkspaceControls`), `GitActionsControl`, `ChatOverlayHosts`, `FilePreviewPanel`, `projectFilesQueryState`, `usePreviewSession` (on upstream's `inactiveAtom`) and `useThreadActions`. - **`getSidebarForkParentThreadId`:** upstream removed it, so it moves into `fork/ThreadParentLine.tsx`. - **New inventory entries:** the `branding-test` path policy and the `preview-session-use-atom-value` duplicate-add exception. - **Sweep:** 19 owned-concern hits, all upstream-owned and taken as upstream's. No new workflow. ## Feature classification ### Usable as-is - Mermaid rendering through pele (pingdotgg#21472) and project icon previews (pingdotgg#20372). - Dialog and composer polish, PR panel fixes and centred tables. - The Waiting thread state (pingdotgg#21500), which shows once the backend sends `blockedOnThreadWait`. - The even-pace tick on Limits (pingdotgg#22408). ### Unsupported in Moatless / needs implementation - `device.easAccount` and EAS Simulator device hosts (pingdotgg#21841–pingdotgg#21846). Added to the `UnsupportedMethodError` union, and the Integrations page that holds them is already flag-gated. - `projects.listIgnoredFiles` and the `ignoredFileListing` capability, which "Add project to another machine" uses (pingdotgg#22180). Added to the union. The dialog falls back to tracked files only while the capability is absent. - `pullRequests.changeBreakdown` (pingdotgg#22189, pingdotgg#22255). Its union already carries `UnsupportedMethodError`, and the tooltip drops the split when the call fails. - T3 Connect reachability (pingdotgg#22172). - Scheduled-task webhook filters and dedupe (pingdotgg#22194, pingdotgg#22234). - `lastEventLoopStall` (pingdotgg#22213) and the `previewClose` scope (pingdotgg#22156). - `compactIdleThreads` and `autoCompactThreshold` (pingdotgg#21682). - MCP diff tools (pingdotgg#15464), the install id (pingdotgg#21506) and the writing-style setting (pingdotgg#22203). ### Backend behavior to consider reproducing in Moatless - **Settlement and waits:** `blockedOnThreadWait`, orphaned follow-ups and delegated waits (pingdotgg#21499, pingdotgg#22581, pingdotgg#20037, pingdotgg#20025). - **Stop and wake:** `latestRunStopRequested` stops a stopped usage-limit thread from auto-resuming (pingdotgg#21114). A wake run settles when the provider never opens the wake turn (pingdotgg#19196). Snooze survives a run's completion (pingdotgg#22202, pingdotgg#17118). - **Runs and shells:** run failures are persisted (pingdotgg#19646), and shell updates coalesce for a slow client (pingdotgg#14859). - **VCS:** bare repositories (pingdotgg#17340, pingdotgg#16688, pingdotgg#18152, pingdotgg#20368), stack layers (pingdotgg#20415, pingdotgg#20388), VCS safety fixes (pingdotgg#15036, pingdotgg#19802) and PR provider robustness (pingdotgg#18780). - **Other:** update and restart safety (pingdotgg#22253), terminal and checkpoint performance (pingdotgg#21242, pingdotgg#21291), resource metrics (pingdotgg#22122), provider polling (pingdotgg#18851) and tool-token expiry (pingdotgg#18552, pingdotgg#19491). `docs/fork/gaps.md` is updated under *Methods the backend does not dispatch* and *Runtime fixes upstream made to its own server*. ## Verification Full `verify.mjs` run: every check except `test` passes (duplicate-adds, tripwires, resolution-check, unsupported-methods with ADD 2 / DROP 0 / KEEP 2, lockfile, fmt, lint, typecheck and build). Test failures and what was done about each: - **Machine failures seen in earlier merges:** `shared` `AgentScope`, `t3` `OrchestratorReplayFixtures` and `provider-opencode` `OpenCodeServerLedger`. The `GitVcsDriver` and `RepositoryIdentityResolver` partial-clone tests fail because of the sandbox's `insteadOf` rewrite. - **New and environmental:** `t3` `ProviderInstanceRegistry` reads the sandbox's own `ANTHROPIC_*` and `CLAUDE_CONFIG_DIR`. With those unset, all 9 of its tests pass. - **`web`:** - The fork's `AppRoot.test.tsx` broke on upstream's new `useEffect` (pingdotgg#20886). It now stubs `useEffect` and passes. - Four browser files failed to import in the parallel run and pass alone. - **Playwright:** 1.60 wants chromium build 1223, which this sandbox lacks. Browser tests ran against 1243 through a symlinked `PLAYWRIGHT_BROWSERS_PATH`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- Moatless task: https://moatless.soaplabstest.com/tasks/c3df6407-7ad0-483c-a46c-e8b603326150
Projects opened through a symlink into a repository subdirectory could resolve Git’s relative common directory into a missing directory or another repository. Checkpoint capture then failed or placed its scratch index in the wrong repository; ref discovery could list the wrong repository’s branches.
Resolve relative common-directory paths against the physical working directory at both call sites. This keeps older Git working without requiring
rev-parse --path-format. Existing path normalization remains in place.Fixes #13187. Maintainer takeover of this PR, with credit to @macodev00 for the original fix and decoy-repository reproduction. Contributor history is preserved, with current main merged into the branch.
Verification of 487a72baf1 on headless Linux (Node 24.21.0, Git 2.43.0):
previousPathreview-diff failure, reproduced with main’s production files. The final rerun has the same result: 226 passed and that one failure.Proof uses real temporary Git repositories through the server driver. No client UI changed, so screenshots are not applicable. No browser or running application was used. macOS, Windows, iOS, Android, and Electron desktop were not verified; no pre-2.31 Git binary was available. CI on this head remains a separate readiness gate. No merge was performed.
Implementation: Codex GPT-6.1-Sol (high), tests: Codex GPT-6-Astra (high), reviews: Claude Opus 5.5 and Codex GPT-6-Sol, all through T3 Code’s delegated agents; parent: GPT-6.1-Sol in the Codex harness.