Repository navigation
Conversation
hasHead treated every failed rev-parse as an unborn branch, so a pruned worktree admin directory made both diff routes return no changes. Probe the git directory first and fail with DiffError when it is unreadable; status keeps its documented empty-list fallback.
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks sound. I reproduced the problem on the base and confirmed the fix with a live server: open a linked worktree, then remove its .git/worktrees/<name> admin directory. On the base, /api/vcs/diff (working and committed) returns 200 []. With this PR both return 503 Unable to read Git repository, and /api/vcs/status still returns [] as described. The new test fails on the base and passes here. The whole of test/vcs.test.ts passes and the packages/core typecheck is clean.
Public API: no endpoints, schemas or SDK types change. For an unreadable repository, /api/vcs/diff and /api/vcs/base now return their existing ServiceUnavailableError (503) where they used to return [] / null. Callers that already handle that error are unaffected.
Two things to be aware of:
base()also goes throughhasHead, so/api/vcs/basenow fails instead of returningnullfor an unreadable repository. The TUI diff viewer then shows an error instead of an empty view. That seems right, but the description doesn't mention it.- This only applies when the repository becomes unreadable after the location was resolved as Git. If you open a worktree that is already broken, project discovery finds no Git repository, so no VCS provider is chosen and the diff is still an empty
200. The workspace-delete check from #50934 also still treats the repository as clean, becausestatuskeeps its fallback (intended here, but #50934 isn't fully covered).
base() shares hasHead, so an unreadable repository now reports unavailable instead of resolving to null; pin that in the same repro.
|
Hi — #57010 and #52413 are green and mergeable: #57010 reports an unreadable repository instead of an empty diff (Closes #50934), and #52413 keeps an undecodable service config from being read as empty (Fixes #52412). Both are on |
Issue for this PR
Closes #50934
Type of change
What does this PR do?
hasHeadusedgit rev-parse --verify HEADand treated every non-zero exit as "unborn branch". An unreadable repository (e.g. a linked worktree whose.git/worktrees/<name>admin directory was pruned) also exits 128 there, so bothvcs.diff("working")andvcs.diff("committed")returned an empty diff — indistinguishable from a clean tree — before any listing ran.This is the V2 (
dev) port of #52544, which fixed the V1 tree and was closed when V1 PRs stopped being accepted; the V1 plugin path it changed no longer exists ondev.The probe now runs
git rev-parse --git-dirfirst. If that fails the repository cannot be read, so the diff routes fail withDiffError("Unable to read Git repository"). If it succeeds, the existing--verify HEADprobe still distinguishes an unborn branch (exit 128 after a readable git dir) from a commit, sodiff("working")keeps diffing the first commit's working tree anddiff("committed")stays empty.base()shares the probe, so it now reports unavailable instead of resolving tonullfor an unreadable repository.Vcs.statushas no error channel (Adapter.statusand/api/vcs/statusdeclare none), so it keeps its empty-list fallback for an unreadable repository; widening that contract is the separate decision discussed in the issue and is not bundled here.How did you verify your code works?
Repro: a repo with one commit, a detached linked worktree containing an untracked file, then
mv repo/.git/worktrees/wt elsewhere. In that worktreegit rev-parse --git-direxits 128 with empty stdout.On
ba06ff9e0with only the new test,bun test test/vcs.test.ts -t unreadablefails atexpect(Exit.isFailure(working)).toBeTrue()(receivedfalse). With the change it passes, and the same test also assertsdiff("committed")fails andstatus()is still[].AI-assisted development; the commands above were run locally against this branch.
Screenshots / recordings
Not a UI change.
Checklist