Skip to content

fix: collapse worktrees to main repo, ignore $HOME-as-git-root#212

Merged
vakovalskii merged 2 commits into
mainfrom
fix/git-root-worktree-and-home
May 13, 2026
Merged

fix: collapse worktrees to main repo, ignore $HOME-as-git-root#212
vakovalskii merged 2 commits into
mainfrom
fix/git-root-worktree-and-home

Conversation

@NovakPAai
Copy link
Copy Markdown
Collaborator

Summary

Sessions whose cwd lives inside a linked git worktree, or sessions launched from \$HOME when home itself happens to be a git repo, were being mismapped to the wrong project / wrong GitHub remote in the dashboard.

This PR fixes resolveGitRoot() in src/data.js so the project identity (and downstream remote inference) is stable for those cases.

Changes

  • Linked worktrees collapse to the main repo. git worktree list --porcelain is parsed for its first record (the main worktree) before falling back to rev-parse --show-toplevel. Previously every linked worktree appeared as its own "project".
  • Bare repos are skipped. Paths ending in .git (what --porcelain reports for bare repos) are dropped — downstream code assumes a working tree.
  • Symlinked paths are normalized via fs.realpathSync so macOS /var vs /private/var doesn't duplicate the same physical project.
  • \$HOME-as-git-root is rejected. When the home directory itself is a git repo (e.g. a dotfiles repo), sessions launched from home no longer inherit that remote.
  • Disk cache filename bumped to -v2.json to invalidate stale entries from before the fix.

Tests

test/git-root-resolve.test.js — 7 cases:

  • 3 unit tests for _parseMainWorktree (happy path, empty input, lines that look like records but aren't).
  • 4 integration tests using real git in os.tmpdir(): linked worktree → main, bare repo → empty, symlink → normalized to real path, \$HOME-as-git-root → empty (skipped if home isn't a repo on the machine).

All 7 pass locally. Pre-existing test/wsl-windows.test.js failure on macOS is unrelated to this change.

Deferred (out of scope)

  • Atomic disk-cache writes (writeFileSync → tmp + rename). Project-wide pattern in the existing cache helpers; addressing it here would balloon the diff.
  • Debug-log on the silent porcelain → rev-parse fallback. Zero-dependency project has no logger; left as a comment-level TODO.

Test plan

  • Run node --test test/git-root-resolve.test.js — all 7 pass.
  • In the dashboard, sessions whose cwd was a linked worktree now group under the main repo's project card.
  • Sessions whose cwd is \$HOME (when home is a git repo) no longer carry that repo's remote.

resolveGitRoot now:
- Uses `git worktree list --porcelain` first record to map linked
  worktrees to the main repo (was: separate "project" per worktree).
- Drops bare-repo paths (root ending in .git) so they don't pollute
  downstream code expecting a working tree.
- Normalizes the result via realpath so /var <-> /private/var on macOS
  doesn't duplicate the same physical project.
- Rejects roots that equal $HOME, fixing the bug where sessions
  launched from home inherited the dotfiles repo's remote and
  mismapped to the wrong GitHub project.

Disk cache bumped to -v2.json to invalidate stale entries.

Tests: 7 cases in test/git-root-resolve.test.js - unit tests for
the porcelain parser + integration tests using real `git init` /
`git worktree add` / bare init / symlink in tmpdir.

Deferred (tracked separately): atomic disk-cache writes (project-wide
pattern), debug-log on porcelain fallback.
@NovakPAai
Copy link
Copy Markdown
Collaborator Author

Merge sequence

Merge this PR first. It has no conflicts with main and is needed as the foundation for #213.

After this merges:

#213 is not safe to merge before this one — without the bare-repo skip + $HOME rejection + symlink normalization from this PR, the getKnownGitRoots() set used by /api/repo-refresh/trigger and initOnStartup would contain spurious entries.

Follow-up to d2420c1. Review surfaced edge cases in the new resolveGitRoot:

- H1: detectHomes now realpath's each home so the ALL_HOMES guard still
  fires when $HOME itself is a symlink (Docker, NFS, atypical macOS).
- M1: HOME-as-gitRoot comparison normalizes path separators and lowercases
  on win32 so C:\Users\foo == C:/users/FOO.
- M2: _parseMainWorktree detects the `bare` attribute inside the first
  worktree record so bare repos without a .git suffix in their name are
  no longer mistaken for working trees.
- L1: tests drop `git init -b main` for compatibility with git < 2.28.
- L2: misleading test name corrected; added a parse test for bare records.

Deferred (tech debt, recorded for follow-up, no user-visible impact):
- M3: doubled timeout worst-case (4s) when worktree-list times out and
  rev-parse fallback runs — timeouts are rare, no fix needed now.
- M4: tests do not reset module-scope _gitRootCache between cases —
  mkdtemp uniqueness makes collision near-impossible.
- L3: no test for detached HEAD main worktree (behavior covered by
  parser unit tests).
- L4: no test for WSL multi-home guard (would need mock infrastructure).
- L5: _saveGitRootDiskCache uses non-atomic writeFileSync — consistent
  with other cache writers in this file; loader try/catch handles a
  truncated JSON gracefully (falls back to empty cache).
Copy link
Copy Markdown
Owner

@vakovalskii vakovalskii left a comment

Choose a reason for hiding this comment

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

LGTM ✅ Focused bug fix, clean execution.

Verified:

  • node -c src/data.js passes
  • node --test test/git-root-resolve.test.js → 7 pass + 1 skipped (the $HOME-as-git-root integration test, intentionally skipped when home isn't a git repo — matches the PR description)
  • CI green 6/6

Spot-checked the logic:

  • _parseMainWorktree handles porcelain record boundaries correctly (bare line inside first record → return empty, blank line → return captured candidate)
  • Fallback chain worktree list --porcelainrev-parse --show-toplevel covers older git versions
  • fs.realpathSync on the resolved root fixes the /var ↔ /private/var macOS dupe
  • $HOME guard normalizes paths cross-platform (replace(/\\/g, '/').toLowerCase() for Windows)
  • Cache filename bump to -v2.json invalidates stale entries without forcing users to clear /tmp manually 👍

Deferred items (atomic disk-cache writes, debug-log on fallback) are correctly scoped out.

@vakovalskii vakovalskii merged commit 95d4843 into main May 13, 2026
6 checks passed
@vakovalskii vakovalskii deleted the fix/git-root-worktree-and-home branch May 13, 2026 19:36
NovakPAai pushed a commit that referenced this pull request May 14, 2026
Adds per-project "Auto-fetch on new chat" toggle, a manual refresh
button on each project card, and a global "Fetch all on codbash start"
toggle. Runs `git fetch --all --prune` in the background — never touches
the working tree, never blocks the HTTP server.

Goal: when a session starts, the LLM sees current origin/<branch> so
new branches start from a fresh base and continuations don't pile on
top of stale state.

Triggers:
- Manual: click the refresh button on a project card.
- New chat: when the per-project toggle is on, /trigger + /wait
  (timeoutMs: 2000) fire before /api/launch. Session opens after fetch
  finishes, or after 2s with stale refs (graceful degradation).
- Service start: bin/cli.js wires the known-roots gate then calls
  repoRefreshManager.initOnStartup() on process.nextTick.

Backend:
- src/repo-refresh.js — singleton via createRepoRefreshManager(opts);
  state machine idle <-> fetching <-> error, single-flight per gitRoot,
  semaphore max 4, 60s timeout with SIGTERM -> 2s grace -> SIGKILL.
- src/repo-refresh-routes.js — 4 routes under /api/repo-refresh/*
  (state, trigger, wait, settings). Body cap 1 MiB, /wait timeoutMs
  clamped to 10s, asyncHandler sends 500 on uncaught throws.
- src/atomic.js — atomicWriteJson(path, obj, {mode}) — tmp + fsync +
  rename with cleanup on rename failure. Settings written 0o600;
  existing disk caches in data.js retrofitted (closes deferred MEDIUM
  from PR #212 about non-atomic writes).
- Persistence: ~/.codedash/refresh-settings.json. Corrupt file ->
  defaults + warning, file left untouched. Debounced 500ms saves.
- Known-roots gate: getKnownGitRoots() = loadProjects() U session
  git_roots, 5s TTL cache. Wired into manager via
  setKnownGitRootsProvider(); initOnStartup refuses to fetch paths
  not in the known set. Defends against settings-file injection.
- Credential redaction: https://user:token@host stripped from any
  captured stderr before storing in lastError (which is exposed via
  /state to the browser).

Frontend:
- Per-project card: status badge (Fetching / Updated 2 min ago /
  Refresh failed: <msg>), refresh button (28x28 visual, 44x44 hit
  area via ::after), Auto-fetch toggle. Header carries global
  "Fetch all on codbash start" toggle.
- Polling 2s only while a visible repo is fetching; time-ticker 30s
  re-renders relative timestamps. Both skip the swap if focus is
  inside the slot (no focus theft).
- Optimistic toggle with rollback + 3s-deduplicated toasts. Input
  disabled during inflight POST (closes the toggle race risk).
- new-chat hook shows a spinner on the launch button + aria-busy.
  Re-entrancy guard via dataset.rrLaunchInflight.
- A11y: native checkbox (no role="switch"), role="group" wrapper,
  role="status" + aria-live="polite" on the outer badge slot so
  innerHTML swaps don't tear down the live region, aria-describedby
  -> visually-hidden span with full error text for SR/keyboard.
  prefers-reduced-motion disables the spinner animation.

Tests: 34 new tests across atomic.js, repo-refresh.js, the routes,
and explicit cases for credential redaction, known-roots gating,
and provider injection.

Deferred (documented LOW from review pass 2, separate follow-ups):
- Retrofit pre-existing readBody to enforce body size cap across
  16 callers (out of scope for this feature; chore PR).
- Periodic scheduler (5/10/15/30/60 min intervals).
- Page-refresh bulk trigger from the browser.
- "Behind by N commits" indicator.
- Connection-lost banner when poll fails.
- toast queue/stack (current dedupe is sufficient for v1).
vakovalskii pushed a commit that referenced this pull request May 15, 2026
Adds per-project "Auto-fetch on new chat" toggle, a manual refresh
button on each project card, and a global "Fetch all on codbash start"
toggle. Runs `git fetch --all --prune` in the background — never touches
the working tree, never blocks the HTTP server.

Goal: when a session starts, the LLM sees current origin/<branch> so
new branches start from a fresh base and continuations don't pile on
top of stale state.

Triggers:
- Manual: click the refresh button on a project card.
- New chat: when the per-project toggle is on, /trigger + /wait
  (timeoutMs: 2000) fire before /api/launch. Session opens after fetch
  finishes, or after 2s with stale refs (graceful degradation).
- Service start: bin/cli.js wires the known-roots gate then calls
  repoRefreshManager.initOnStartup() on process.nextTick.

Backend:
- src/repo-refresh.js — singleton via createRepoRefreshManager(opts);
  state machine idle <-> fetching <-> error, single-flight per gitRoot,
  semaphore max 4, 60s timeout with SIGTERM -> 2s grace -> SIGKILL.
- src/repo-refresh-routes.js — 4 routes under /api/repo-refresh/*
  (state, trigger, wait, settings). Body cap 1 MiB, /wait timeoutMs
  clamped to 10s, asyncHandler sends 500 on uncaught throws.
- src/atomic.js — atomicWriteJson(path, obj, {mode}) — tmp + fsync +
  rename with cleanup on rename failure. Settings written 0o600;
  existing disk caches in data.js retrofitted (closes deferred MEDIUM
  from PR #212 about non-atomic writes).
- Persistence: ~/.codedash/refresh-settings.json. Corrupt file ->
  defaults + warning, file left untouched. Debounced 500ms saves.
- Known-roots gate: getKnownGitRoots() = loadProjects() U session
  git_roots, 5s TTL cache. Wired into manager via
  setKnownGitRootsProvider(); initOnStartup refuses to fetch paths
  not in the known set. Defends against settings-file injection.
- Credential redaction: https://user:token@host stripped from any
  captured stderr before storing in lastError (which is exposed via
  /state to the browser).

Frontend:
- Per-project card: status badge (Fetching / Updated 2 min ago /
  Refresh failed: <msg>), refresh button (28x28 visual, 44x44 hit
  area via ::after), Auto-fetch toggle. Header carries global
  "Fetch all on codbash start" toggle.
- Polling 2s only while a visible repo is fetching; time-ticker 30s
  re-renders relative timestamps. Both skip the swap if focus is
  inside the slot (no focus theft).
- Optimistic toggle with rollback + 3s-deduplicated toasts. Input
  disabled during inflight POST (closes the toggle race risk).
- new-chat hook shows a spinner on the launch button + aria-busy.
  Re-entrancy guard via dataset.rrLaunchInflight.
- A11y: native checkbox (no role="switch"), role="group" wrapper,
  role="status" + aria-live="polite" on the outer badge slot so
  innerHTML swaps don't tear down the live region, aria-describedby
  -> visually-hidden span with full error text for SR/keyboard.
  prefers-reduced-motion disables the spinner animation.

Tests: 34 new tests across atomic.js, repo-refresh.js, the routes,
and explicit cases for credential redaction, known-roots gating,
and provider injection.

Deferred (documented LOW from review pass 2, separate follow-ups):
- Retrofit pre-existing readBody to enforce body size cap across
  16 callers (out of scope for this feature; chore PR).
- Periodic scheduler (5/10/15/30/60 min intervals).
- Page-refresh bulk trigger from the browser.
- "Behind by N commits" indicator.
- Connection-lost banner when poll fails.
- toast queue/stack (current dedupe is sufficient for v1).

Co-authored-by: jackrescuer-gif <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants