fix: resolve project config from bare root when primary worktree is off default branch - #3462
Conversation
…t branch In a bare-repo layout, running a command from the bare root resolves project config via primary_worktree(), which finds the worktree whose branch equals the default branch. When the primary worktree is briefly checked out to a non-default branch (common in agent-driven workflows), no worktree holds the default branch, so project_config_path() returned None and every project hook was silently dropped with no error. Fall back to the first non-bare worktree that ships a .config/wt.toml when the default-branch worktree isn't checked out anywhere. Project config is a repo-level artifact present in every worktree, so this restores project hooks without depending on the default branch's checkout state. Scoped to project-config resolution; primary_worktree() semantics are unchanged. Closes #3461
worktrunk-bot
left a comment
There was a problem hiding this comment.
Self-review (COMMENT — can't self-approve). The fix is well-scoped and the regression test genuinely fails on main: with the primary parked off the default branch, primary_worktree() is None, the hook plan is empty, and the marker never gets written. The is_file() gate is right for the fallback (it has to find which worktree carries the config), and it correctly avoids the bare root because list_worktrees() already filters bare entries (worktrees.rs:31, .filter(|wt| !wt.bare)) — so test_bare_repo_ignores_config_in_bare_root stays green.
One design point worth a maintainer's eye (inline). Also: the fallback's safety hinges on that !wt.bare filter, but no test pins it in the off-branch state — test_bare_repo_ignores_config_in_bare_root only covers the primary-present case where this loop never runs. A companion negative test (bare-root config present and primary parked off-branch, asserting the hook does not fire) would lock the invariant down so a future change to the filter can't silently start reading bare_root/.config/wt.toml.
The new test in 07aa23c wrote the `config show` builder call on one line; rustfmt wraps `.current_dir(...)` onto its own line. Fixes the failing `lint` (cargo fmt) check.
|
I'm skeptical of "find the first", seems arbitrary and likely to lead to confusion how about loading it in with git show? I realize it'll be another 50ms, but seems reasonable? |
Replace the worktree-scan fallback with an object-store read of the default branch's committed .config/wt.toml (git show <default>:.config/wt.toml) when the primary worktree is parked off the default branch. This reads the canonical default-branch config rather than whichever branch a worktree happens to hold, addressing review feedback that 'find the first' worktree was arbitrary.
|
Agreed — and reading it via Pushed in
One scoping note: The regression test now also pins the semantics: after parking the primary off-branch it overwrites the working-tree config with a divergent hook and asserts that hook does not run — proving resolution reads the committed default-branch tree, not the checked-out files. |
worktrunk-bot
left a comment
There was a problem hiding this comment.
Self-review (COMMENT — can't self-approve). The git show rework reads cleanly and the two new tests pin the important semantics: the default branch's committed config wins over the parked worktree's on-disk files, and an absent config resolves to None rather than a phantom. One merge gate and one narrow correctness question below.
codecov/patch is failing (merge gate). Six added lines are uncovered, both defensive branches the two new tests never exercise:
src/config/project.rslines 276–280 — themap_errwrapper fordefault_branch_project_config_content()returningErr. Only agit showspawn failure hits this; a non-zero exit is already folded intoOk(None)inside the helper, so the happy-path tests never take it.src/git/repository/config.rsline 724 — thelet Some(branch) = self.default_branch() else { return Ok(None) }guard. In the bare + primary-off-branch state the default-branch ref still exists (it's just checked out nowhere), sodefault_branch()returnsSomeand this arm is never reached; both new tests parkmainoff-branch withmainstill a live ref.
Both are genuinely hard to drive deterministically. Per the codecov merge-gate rule in CLAUDE.md (§Coverage), this needs either a targeted test or explicit maintainer sign-off before merge — flagging rather than rubber-stamping over the red signal.
Override-contract question (narrow). default_branch_project_config_content() isn't gated on WORKTRUNK_PROJECT_CONFIG_PATH. project_config_path() documents that override as authoritative — "A missing file at that path still resolves to Ok(None)" — and the test at switch.rs (test_switch_create_honors_project_config_path_override) asserts it "overrides the path for every config read." But in the new fallback path, a set-but-missing override in the bare + primary-off-branch state falls through the _ arm in load() and reads git show <default>:.config/wt.toml instead of resolving to None — so the override is no longer authoritative there. Contrived (non-existent override target + bare + off-branch), but it does make that doc/contract false in exactly the state this PR adds. Worth a one-line guard (if std::env::var_os("WORKTRUNK_PROJECT_CONFIG_PATH").is_some() { return Ok(None); }) at the top of the helper if you want the override to stay the single source; happy to leave it if you consider the combination out of scope.
Address codecov/patch: restructure default_branch_project_config_content to read HEAD:.config/wt.toml (bare HEAD is a symref to the default branch, always resolvable, no branch lookup) and match on the git show result so the no-config arm is exercised by the negative test. Returns Option, dropping the error closure in ProjectConfig::load. All new lines are now covered.
|
Small refinement in CI is green — |
|
Correction to my note above on Opened #3466 to resolve the default branch by name ( Verified end-to-endBare repo;
|
Semantic resolution beyond the textual conflict: main's #3462 committed default-branch config fallback (default_branch_project_config_content) now returns None when WORKTRUNK_PROJECT_CONFIG_PATH is set — an explicit override names the config source outright, so the committed fallback must not supersede it (it exists for test isolation, where reading the repo's own committed config is the leak being prevented). Pinned by test_bare_repo_committed_config_does_not_supersede_override. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Follow-up to #3462 (merged). That PR's final commit changed the object-store fallback to read `git show HEAD:.config/wt.toml`, and this reintroduces #3461 in the common agent case. ## The bug `git show` resolves `HEAD` against the **invocation cwd's per-worktree HEAD**, and the read runs with `discovery_path` (the cwd `wt` was invoked from) as its working directory. When `wt` runs from *inside a linked worktree parked on another branch* — exactly the agent-driven workflow the #3461 fix targets — `HEAD` is that worktree's branch, not the default branch. So the fallback either reads the wrong branch's config or, when the invoking worktree's branch carries no committed config, drops the default branch's config and every project hook. The merged PR's two regression tests only invoke `wt` from the **bare root**, where `HEAD` does point at the default branch, so they can't catch this. ## Verified end-to-end Bare repo; default branch `main` has `.config/wt.toml` with a `post-start` hook; `main` checked out in no worktree; a second worktree `other` whose branch dropped the config on disk: | Invoked from | `git show HEAD:.config/wt.toml` | Hook fires? | |---|---|---| | bare root | reads `main`'s config | ✅ (what the merged tests cover) | | `other` worktree | reads `other`'s HEAD → no config | ❌ **default-branch config dropped** | The absolute ref `git show main:.config/wt.toml` reads the config correctly from any cwd — which is what the pre-regression commit (and @max-sixty's original "load it in with git show" request on #3462) did. ## The fix Resolve the default branch **by name** (`<default-branch>:.config/wt.toml`), which is cwd-independent. This is a one-line change back to an absolute ref, plus a doc-comment correction. The added regression test invokes `wt switch --create` from inside the `other` worktree and asserts the default branch's hook fires. It fails against the merged `HEAD:` behavior (marker never written) and passes with the fix; the two existing #3461 tests stay green. ## Coverage note `self.default_branch()?` restores an early-`None` return when the default branch can't be determined (bare repo with no resolvable default). That branch is a defensive guard and isn't exercised by the tests — the `HEAD` form avoided it, which is likely why it was introduced, but correctness outranks covering a one-line unreachable-in-practice guard. Flagging in case `codecov/patch` trips on it. Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>
…ot (#3460) `WORKTRUNK_PROJECT_CONFIG_PATH` was returned verbatim, so a relative value resolved against the process cwd while the default `.config/wt.toml` is anchored to the worktree root. Running `wt` from a subdirectory made a relative override silently no-op (or read an unintended file). This is the config-path footgun from #3454 (the RFC there is a separate discussion). A relative override now resolves against the same worktree root as the default (current worktree, or the primary worktree at a bare root), so it behaves identically from any subdirectory. Values that can't be anchored fail loudly instead of guessing: - relative with no worktree root to anchor to (bare repo with no linked worktrees) errors - Windows forms that are neither fully absolute nor relative (drive-relative `C:cfg`, rooted-but-driveless `\cfg` or `/tmp/x`) error rather than resolving against the process drive or cwd Absolute values are unchanged (returned verbatim, no git calls), which keeps the test-isolation use of the var working. An empty value still means no project config, as before. `wt config show` (human format) and `wt config update` previously folded a `project_config_path()` error into "not in a git repository" / a silent skip; they now propagate it, matching the JSON path. Merging main brought in #3462's committed default-branch config fallback for bare repos whose primary worktree is parked off the default branch. That fallback now stands down when `WORKTRUNK_PROJECT_CONFIG_PATH` is set: an explicit override names the config source outright (a missing file there means no project config), and the override exists for test isolation, where reading the repo's own committed config is exactly the leak being prevented. The human format of `wt config show` now reports "No project config" when no location exists (bare repo with no worktrees, empty override) instead of the inaccurate "Not in a git repository", which stays reserved for actually being outside a repo. Compatibility: invoked from the worktree root, cwd and root coincide, so old and new resolution agree. From a subdirectory the old resolution found nothing, which is the bug being fixed, so no deprecation shim is included. Thanks @indexzero for reporting in #3454. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Release v0.68.0. Highlights: on-demand picker preview tabs (#3439), bare-repo project config read from the object store when the default branch is checked out nowhere (#3462), shell detection via the process tree (#3455), and `wt config state` flagging a stale default-branch cache (#3478). Full details in CHANGELOG.md. > _This was written by Claude Code on behalf of max_
Problem
In a bare-repo layout, running
wt switch --create <branch>from the bare root silently skips all project hooks (pre-start,post-start, …) when the primary worktree happens to be checked out to a non-default branch. No error, no "declined" message — the operation reports success and only user hooks fire.Root cause, as diagnosed in #3461: from the bare root,
project_config_path()locates the project config viaprimary_worktree(), which looks up the worktree whose branch equals the default branch. When an agent-driven workflow briefly parks a PR/feature branch in the primary worktree, no worktree holds the default branch, soprimary_worktree()returnsNone,project_config_path()returnsNone, andHookPlanBuilderdrops the project source silently. This is the gap left by #1692'sprimary_worktree()fallback for the bare-root path.Solution
Scope the fix to project-config resolution. When the default-branch worktree isn't checked out anywhere (
primary_worktree()isNone), fall back to the first non-bare worktree that actually ships a.config/wt.tomlrather than dropping all project hooks. Project config is a repo-level artifact present in every worktree, so this restores hooks without depending on the default branch's checkout state.primary_worktree()semantics are deliberately unchanged — see the design note below.Testing
Added
test_bare_repo_project_config_found_when_primary_on_non_default_branchintests/integration_tests/bare_repository.rs, a sibling to the existing #1691 tests. It commits apost-starthook to the primary worktree's config, checks out a non-default branch there, runswt switch --createfrom the bare root, and asserts the hook fires. It fails onmain(marker never written) and passes with this change. The fulltest_bare_repo*suite (24 tests, includingtest_bare_repo_ignores_config_in_bare_root) still passes — the fallback scans worktree paths, never the bare root, so a config placed in the bare root is still correctly ignored.Design note for reviewers
The issue proposed fixing
primary_worktree()itself to identify the primary "by role, e.g. the first non-linked worktree." I did not take that path, for two reasons:git worktree list --porcelainorders them alphabetically, not by role or creation. In amain+featlayout with the primary off-branch, "the first worktree" isfeat, notmain— so that fallback would pick the wrong worktree.primary_worktree()broadly would also move{{ primary_worktree_path }},copy-ignored's source, andis_primary_worktree(), and contradicts the documented terminology (primary worktree = default-branch worktree for bare repos).copy-ignoredalready fails with a clear error in this state rather than silently, so the silent loss is specific to project config.If a maintainer would prefer a role-based
primary_worktree()redefinition (which would also set{{ primary_worktree_path }}in this state), that's a larger, separate change with a real semantic decision — happy to follow up.Closes #3461 — automated triage