Skip to content

fix: discover separate-git-dir checkouts on first run - #90

Merged
HemSoft merged 1 commit into
mainfrom
fix/issue-88-discover-separate-git-dir
Nov 6, 2026
Merged

HemSoft merged 1 commit into
mainfrom
fix/issue-88-discover-separate-git-dir

Conversation

@HemSoft

@HemSoft HemSoft commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Risk level: 2/10 (project-dashboard)
Worked on by: claude-opus-5-5 on Laptop, for initiating Home70828f7a

Summary

First-run discovery treated every .git file as a linked worktree, so it missed a checkout made with git clone --separate-git-dir, even though the dashboard reads that checkout's identity correctly. config::discover (crates/dashboard-core/src/config.rs) now also counts a .git file whose gitdir: folder holds HEAD and no commondir. Real git 2.55 matches this rule: a separate git folder has HEAD and no commondir, and a linked worktree's git folder has both.

  • A linked worktree is still skipped: its git folder has commondir.
  • The .git file is read only when it is a regular file of at most 64 KiB. A pipe, device or oversized file is never opened. A missing, empty, malformed or non-UTF-8 file, or a target without HEAD, is skipped. There is no panic, no git process and no new dependency.
  • Results stay sorted. Only direct children of the root are looked at, so a checkout two levels down is still skipped.

Only discover and its tests change. Link checkout (#87) is untouched.

Acceptance Criteria

  • A direct child whose .git file points at a git folder without commondir is discovered: first_run_discovers_separate_git_dir_checkouts_but_not_worktrees_or_broken_git_files finds separate (absolute gitdir:) and relative (gitdir: ../../gitdirs/rel.git with CRLF), through both config::discover and Config::load_or_create_in.
  • A linked worktree, a plain folder and a checkout two levels down are still skipped: the same test has plain-pr-1, whose git folder holds HEAD and commondir, so commondir is the only difference from separate. It also has group/deep. The existing first_run_discovers_checkouts_one_level_under_the_root_and_writes_them is unchanged and passes. It covers notes and alpha.worktrees/pr-1.
  • An unreadable .git file, one without a gitdir: line, or one pointing at a missing folder is skipped without panic: the same test has missing, no-head, malformed, empty, not-utf8, huge (a valid first line padded past 64 KiB) and, on Linux, pipe, whose .git is a FIFO that would block a read. discovery_of_a_missing_root_or_a_file_is_empty covers a missing root and a root that is a file.
  • No git subprocess, and the result stays sorted: no_production_source_starts_a_child_process passes. The test asserts the exact sorted list [plain, relative, separate].
  • The discover doc comment matches the new rule: updated, and is_primary_checkout documents the rule.

Definition of Done

  • Fix and fixture-folder tests in dashboard-core, with no git needed in tests: two new tests in crates/dashboard-core/tests/storage.rs. Negative control: against main's discover, the new test fails, finding only [plain] instead of [plain, relative, separate].
  • fmt, clippy, workspace tests and advisories at this head: all pass on Linux (see Validation).
  • Windows workspace gates: both required Windows jobs passed at 36dacbb in CI 42265803779. Local full workspace builds were omitted to preserve the shared build slot.
  • Clean current-head AI review: SFL PASS round 1 on 36dacbb, no findings and zero unresolved threads.

Validation

At head 36dacbbec47d1bebd93aee55669a8af006d782ac, on Mini (Linux), from a git archive of that commit:

  • cargo fmt --all --check: pass
  • cargo clippy --workspace --all-targets --locked -- -D warnings: pass
  • cargo test --workspace --locked: pass (dashboard-app 184 passed, 14 ignored; every dashboard-core suite passed, including storage 12/12 with the FIFO case, and no_child_process 1/1)
  • cargo deny --locked check advisories: advisories ok

On Laptop (Windows 11), at the same source: cargo fmt --all --check passed, and cargo test -p dashboard-core --locked --test storage --test no_child_process gave storage 12/12 and no_child_process 1/1. The FIFO case is Linux-only, so the Linux run above exercises it.

The fixtures model git's layout, checked with git 2.55: git init --separate-git-dir <dir>/sep.git separate writes separate/.git = gitdir: <dir>/sep.git, and that folder has HEAD and no commondir. git worktree add ../separate-wt writes a gitdir: pointing at sep.git/worktrees/separate-wt, which has HEAD and commondir.

No screenshots: this changes only which folders the first-run config.toml lists. No view, layout or text in the window changes.

Risks and deferred work

  • Windows gaps: I didn't start a full Windows workspace build here, to keep the Laptop's one heavy Windows build slot free. The Windows evidence for this head is the focused core run above plus CI's Windows jobs.
  • Behavior change on first run only: an existing config.toml is never rescanned.

Closes #88

🤖 Generated with Claude Code

All five required CI jobs passed at 36dacbb, including every executed step. The separate packaging run is in_progress; it adds no required status context.

First-run discovery kept only folders whose .git is a folder, taking every
.git file for a linked worktree. A checkout made with --separate-git-dir has a
.git file too, but its git folder is its own: it has no commondir.

Count a .git file whose gitdir: target holds HEAD and no commondir. The file
is read only when it is a regular file of at most 64 KiB, so a pipe or device
is never opened, and a missing, malformed or non-UTF-8 file is skipped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@HemSoft HemSoft added the model:claude-opus-5-5 PR authored by claude-opus-5-5 label Nov 6, 2026
@HemSoft

HemSoft commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

@sfl review

@hemsoft-sfl-reviewer hemsoft-sfl-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Verdict: PASS • Round 1

Confidence: 8/10

Review done by claude-opus-5-5 (same family: same-family: codex gated: codex Weekly 163% SUS 61 🔴 pause, defer)

The PR changes first-run discover so it also counts a checkout whose .git is a regular gitdir: file under 64 KiB, pointing at a git folder that has HEAD and no commondir. Linked worktrees, pipes, oversized, malformed, non-UTF-8 and missing-target files are still skipped, and new storage tests cover each case. I found no evidenced defect in the diff. The only limit on merging now is that CI is still running.

🔨 Full-Spectrum Review Summary

Effort: high
Review time: 1m 39s
Token count: Unavailable

Pass 🔴 Critical 🟠 High 🟡 Medium ⚪ Low
🔒 Security 0 0 0 0
✅ Accuracy 0 0 0 0
🧰 Quality & Maintainability 0 0 0 0
Total 0 0 0 0

Key Findings

No material defects found.

Coverage

  • Files reviewed:
    • crates/dashboard-core/src/config.rs
    • crates/dashboard-core/tests/storage.rs
  • Context read:
    • AGENTS.md
    • README.md
    • Cargo.toml
    • Cargo.lock
    • crates/dashboard-core/Cargo.toml
    • crates/dashboard-core/src/config_atomic_73.rs
    • crates/dashboard-core/tests/no_child_process.rs
    • issue #88
    • PR #90 metadata
  • Verified:
    • Compare base_commit.sha is af1436057f950b0d704a5941e7690c27fc8a210e, the last commit is 36dacbbec47d1bebd93aee55669a8af006d782ac, the merge base equals the base, ahead_by 1, behind_by 0
    • PR #90 head and base SHAs match the reserved SHAs
    • Only two files changed: config.rs (+32/-3) and storage.rs (+82)
    • is_primary_checkout calls metadata (which follows symlinks) and rejects non-regular files before open, so the FIFO case in the test is never read
    • The MAX_GIT_FILE size check plus take(MAX_GIT_FILE) bounds the read; a non-UTF-8 file returns an error from read_to_string and is skipped
    • The gitdir: prefix match and joining a relative target to the checkout folder match how git reads a gitfile
    • A target with commondir (linked worktree) is excluded; a target with no HEAD or a missing target is excluded
    • Test expectation [plain, relative, separate] is sorted and matches the rule; ../../gitdirs/rel.git resolves to gitdirs/rel.git in the scratch folder
    • The test's rustix use is allowed because the Linux target dependency rustix =1.1.5 with fs is present in the dashboard-core manifest and lock
    • No Command/Stdio use was added to src, so the no_child_process check is unaffected
    • The discover doc comment was updated to the new rule, as issue #88 requires
    • The existing first-run test's worktree case is still excluded under the new rule (its target is missing)
  • Not verified:
    • Compilation, clippy and fmt: CI check runs core and rust on both OSes were in_progress at review time
    • rustix mknodat and Mode From<u32> signatures in 1.1.5 were inferred from API knowledge, not read from source
    • Windows behavior of joining a forward-slash relative gitdir target
    • Whether pmd::checkout_identity reads separate-git-dir checkouts: the claim comes from issue #88, and that code was not read
  • Tests run: None. Reviewer execution is disabled.

HemSoft full-spectrum review • 3 evidence-based passes • Reviewed head 36dacbbec47d1bebd93aee55669a8af006d782ac • SFL v0

@HemSoft
HemSoft merged commit ba301ae into main Nov 6, 2026
6 checks passed
@HemSoft
HemSoft deleted the fix/issue-88-discover-separate-git-dir branch November 6, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

model:claude-opus-5-5 PR authored by claude-opus-5-5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Discover separate-git-dir checkouts on first run

1 participant