fix(status): diagnose bare-worktree repo missing hop.json#26
Conversation
When git hop status is invoked inside a bare-worktree-shaped repo that
lacks hop.json (e.g. a bare repo created outside git hop or whose hub
config was lost), the command produced two misleading outcomes:
- At the bare-repo root, FindHub failed and IsInsideWorkTree returned
false (bare), so status printed "Not in a hub or worktree." even
though the directory is clearly a worktree-shaped repo.
- Inside hops/<branch>/, FindHub still failed but IsInsideWorkTree
returned true, so status fell into showWorktreeStatus and printed
only the 2-line git portion ("On branch X / nothing to commit"),
output indistinguishable from plain git status with no hint that
the hub is unregistered.
Detect the case via a pure helper that walks up from cwd looking for an
ancestor that (1) is a git bare repo, (2) has a non-empty hops/
subdirectory, and (3) lacks hop.json. When matched, print a "hint:"
line pointing at git hop init in git-porcelain style, and return
without falling through to the misleading branches above.
There was a problem hiding this comment.
Pull request overview
This PR improves git hop status diagnostics for bare-worktree-shaped repositories that are missing hop.json, so users get an actionable recovery hint instead of misleading status output.
Changes:
- Adds a pre-worktree status check for unregistered bare-worktree-shaped repositories.
- Introduces helper functions for detecting the repo shape and formatting the diagnostic.
- Adds unit tests for detection scenarios and the hint message.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
cmd/status.go |
Invokes the new diagnostic before falling through to worktree/plain status behavior. |
cmd/status_diagnose.go |
Adds detection and hint formatting helpers. |
cmd/status_diagnose_test.go |
Adds table-driven tests for repository-shape detection. |
cmd/status_diagnose_message_test.go |
Verifies the diagnostic mentions the path, missing file, and recovery command. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // writeBareConfig writes a minimal git bare-repo config file at <root>/config. | ||
| func writeBareConfig(t *testing.T, fs afero.Fs, root string) { | ||
| t.Helper() |
There was a problem hiding this comment.
The premise is empirically false. afero.NewMemMapFs's WriteFile opens with O_CREATE, which transparently creates any missing parent directories on MemMapFs — verified directly:
fs := afero.NewMemMapFs()
afero.WriteFile(fs, "/a/b/c/d/file.txt", []byte("hi"), 0644) // succeeds; /a/b/c/d created implicitlyThe 9 detection cases run as written — including the ones that depend on writeBareConfig succeeding to assert wantOK: true. If the helper actually failed silently those cases would return false and the test would fail.
That said, the underlying suggestion (assert setup didn't fail) is fair insurance against a future afero change. Tightened in 9d2c7bb — mkdir and writeFile helpers fail the test on any setup error.
| _ = afero.WriteFile(fs, "/repo/config", []byte(content), 0644) | ||
| _ = fs.MkdirAll("/repo/hops/main", 0755) |
There was a problem hiding this comment.
Same premise as the helper comment — the writes succeed on afero.NewMemMapFs because WriteFile creates missing parents, so this case really does exercise the non-bare-config path (the detection returns false because the config doesn't contain bare = true, not because the file is absent).
The error-check suggestion is the right call regardless. Both MkdirAll and WriteFile now go through helpers that t.Fatalf on any error (9d2c7bb), so a setup failure can't masquerade as a passing negative case.
Wrap MkdirAll and afero.WriteFile in t-aware helpers that fail the test via t.Fatalf on any setup error. Previous code used the project- local "_ = afero.WriteFile(...)" pattern (matches existing tests in internal/hooks/install_test.go), which works in practice because afero.MemMapFs.WriteFile creates missing parent dirs implicitly (via OpenFile with O_CREATE) — verified empirically. The explicit checks just remove the dependency on that behavior so a future afero change can't make a negative-case test pass for the wrong reason. No behavior change in production code.
Summary
git hop statuswas misleading in two ways when run inside a bare-worktree-shaped repo that lackshop.json: at the bare-repo root it printedNot in a hub or worktree.; insidehops/<branch>/it fell through toshowWorktreeStatusand printed a 2-line summary indistinguishable from plaingit status.hops/+ nohop.json) and print a git-porcelain-stylehint:line pointing the user atgit hop init, before either misleading branch can fire.detectUnregisteredBareWorktreeRepo,unregisteredBareWorktreeHint) added incmd/status_diagnose.goso the detection and the message are both unit-testable without shelling out.Test plan
go build ./...clean.go test ./internal/... ./cmd/...— 857 pass, 0 fail (includes 9 new cases incmd/status_diagnose_test.goandcmd/status_diagnose_message_test.go, all written test-first).go test ./...against unmodified main HEAD shows the same 12 pre-existingtest/e2e/failures (golden-fixture drift + env-dependent deps tests). No new regressions from this change.hop.jsonfrom a real bare-worktree repo, rangit hop statusfrom the bare root and from insidehops/main/— both now print the new diagnostic and exit 0. Restoringhop.jsonbrings back the full hub table.go vet ./cmd/clean.Notes
CLAUDE.md: lowercasehint:prefix to stderr; result/header to stdout.git hop initdetect this same condition inhandleAlreadyInitializedand back-fillhop.jsonautomatically, instead of just printing "Repository already initialized" and exiting.