feat(config): experimental worktrunk.config.* git-config project-config source - #3609
feat(config): experimental worktrunk.config.* git-config project-config source#3609indexzero wants to merge 6 commits into
Conversation
b2be351 to
c2a50d0
Compare
worktrunk-bot
left a comment
worktrunk-bot
left a comment
There was a problem hiding this comment.
Reviewing as a draft — flagging anything that looks worth a quick fix. Mark ready for a full review.
Nicely structured — the mechanical key mapping, the all-or-nothing selection, and the consistent "name the source, keep the approval gate" treatment across every consumer (config show, hook show, diagnostic, approvals, create, alias) all hang together. The diagnostic path deliberately omitting values (only naming keys + the superseded file) is a good call given diagnose output gets pasted into public bug reports.
One functional gap in the wt step prune skip-hint, noted inline: the git_source_active → invoking_project_bytes = None change doesn't actually suppress the (different hooks on branch) annotation the way its comment claims — it inverts it for the supersession case. Details on the line.
worktrunk-bot
left a comment
worktrunk-bot
left a comment
There was a problem hiding this comment.
Just the new commit since the last review — mostly test coverage, plus one behavior change in the diagnose path worth a second look (inline).
f785a1d to
e73bf35
Compare
…ig source Any key under the worktrunk.config. prefix in git config now supplies project configuration (max-sixty#3454). Strip the prefix; the remainder is the exact .config/wt.toml key path. Selection is all-or-nothing: when any key exists, the merged effective git config is the complete project config, the file is ignored, and a warning names the superseded file. Values are strings only; schema violations fail loudly with no fallback to the file. Commands from this source pass through the same approval gate as file-based project config. Git owns precedence: keys come from the cached `git config --list -z` map, so system/global/local scopes and includes resolve before worktrunk sees them, at zero additional subprocess cost. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Findings from adversarial reviews of the two competing implementations, applied per the reviewed merge-fix plan: - Alias discovery routes through ProjectConfig::load, so git-config aliases appear in listings exactly when dispatch would run them. - A WORKTRUNK_PROJECT_CONFIG_PATH override — any value, including the empty kill switch — disables the git source, enforced in the sole accessor so every consumer inherits the deferral by construction. - wt config show propagates a failed git-config read instead of rendering the file as active while execution errors. - wt --diagnose names the git-config source, its key names, and the superseded file, but never the values: diagnose output is routinely pasted into public bug reports and this source exists for private configuration. - The supersession-warning latch is peeked before label resolution and set only on emit, so a no-file call cannot suppress a later born-superseded warning from wt config create --project. - Provenance docstring softened: include/includeIf can carry remotely-sourced content, which is why the approval gate applies unchanged. - Docs now state what the git source skips (file migration and its deprecation messaging — deprecated spellings may still deserialize), that per-worktree git config is unsupported, and describe the diagnostic command as listing matching keys rather than consumed values. - wt config create --project warns when the new file is born superseded; wt step prune suppresses the branch-hooks annotation under the git source (git config is branch-independent); project config resolves from a bare root via the git source, with wt config approvals framing its answers from it. - New tests: behavioral approval decline (hook must not run), includeIf resolution, both override-precedence forms, alias-listing consistency, born-superseded create, bare-root approvals, and diagnose value redaction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- wt config show snapshots for the two error renderings: a git-source value that violates the schema (Invalid config + gutter) and colliding key paths (conflict error), matching the file branch's treatment. - wt step prune --dry-run under the git source exercises the suppressed branch-hooks baseline. - A bare root without keys exercises the no-project-config guard and its path-free error. - A repeat-call unit test exercises the supersession-warning latch peek. - The diagnose Err arm collapses into the file fallback (best-effort surface; the file section already reports read failures) and the infallible-in-practice TOML render error map shrinks to a passthrough, removing two unreachable specialized handlers instead of pretending to test them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…r arm Two review findings on the follow-up commits: - wt step prune suppressed the "(different hooks on branch)" annotation by nulling the comparison baseline, which inverts it: a candidate with a committed .config/wt.toml compares Some(_) != None and flags the exact case where git-config hooks are identical across branches. The suppression now lives on the differs computation itself, pinned by a test with a committed file and an unapproved git-config pre-remove. - The diagnose Err arm returns: a failed bulk-config read is reported as such instead of falling through and printing .config/wt.toml as the active source — the file section's "(read failed)" covers a different read, so the fall-through misattributed, in the report meant to diagnose exactly that failure. Its uncovered line is an honest, effectively untestable gap, and is documented as one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
test_git_config_scope_precedence_local_over_global wrote its "global" worktrunk.config.* keys with `git config --global`, which lands in the process-shared test gitconfig (test_gitconfig_path) that every parallel test points GIT_CONFIG_GLOBAL at. Those keys then bled into other tests' merged `git config --list`, non-deterministically shifting, e.g., the TOML line number in test_git_config_source_invalid_value_fails_loudly (line 2 → line 8) whenever the two tests overlapped. Give the global tier its own GIT_CONFIG_GLOBAL file so the global scope stays private to this test. No other test writes --global. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address the codecov/patch gaps on the worktrunk.config.* changes: - git_source: the supersession-warning latch drops its untestable race-loser guard for a race-tolerant `let _ = WARNED.set(())`; the top peek still suppresses the common re-entry. - config show: the superseded-file warning and the diagnostic-command hint move from writeln!(…)? to push_str — writing into a String is infallible, so `?` left an uncoverable error region. - diagnostic: three in-process unit tests exercise config_show_output's git-source branch (names source + keys, omits values), the file fallback, and the failed-read arm; the superseded-file note builds via map/unwrap_or_default so its region records under coverage. - config.rs: project_config reverts to upstream's form. Upstream now has current_worktree().root() fall back to the discovery path instead of erroring, so load() is always reached and handles the git-config source itself — the earlier in_worktree/git-keys guard (and its dead else) is unnecessary. - config create: an error-path test (`.config` is a regular file, so create_dir_all fails) covers the create call's `?`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
f2f7d65 to
f022b69
Compare
|
@max-sixty after reviewing this a few times, I have started using this as my default for Will run this from now on to shake loose any other bugs, but it feels solid Curious to hear your thoughts! 🙏 |
worktrunk-bot
left a comment
worktrunk-bot
left a comment
There was a problem hiding this comment.
Full pass now that it's out of draft. The core is in good shape — mechanical key mapping, all-or-nothing selection, and the "name the source, keep the approval gate" treatment across every consumer are consistent, and both draft-review findings are properly fixed (the differs guard in prune is now on the computation, and the diagnostic keeps its explicit Err arm). Test coverage is unusually thorough for a config source: scope precedence, include.path, includeIf, the override kill switch, declined-approval non-execution, and alias discovery matching dispatch.
Not approving, on repo policy rather than on the code: the diff edits src/commands/step/prune.rs, which is wt step prune's removal driver (branch deletion via delete_branch_if_safe, worktree removal). The project's review rules hold any change reaching that surface for a human. Requesting @max-sixty.
Three things worth a look, plus inline suggestions.
--global × all-or-nothing is the sharp edge, and neither doc paragraph says it. The docs note that --global "puts a key in every repository" and, separately, that selection is all-or-nothing — but not the product: a single git config --global worktrunk.config.list.url … set for convenience silently switches every repository on the machine off its committed .config/wt.toml, dropping all project hooks. The supersession warning does fire, which is the right backstop, but a user reading only the docs won't see it coming. Suggestion inline.
config.worktree is not uniformly ignored. Both the module spec and the docs say a config.worktree value is "never consumed" because the bulk read runs from the common git dir. That holds for a linked worktree, but the main worktree's config.worktree lives at $GIT_COMMON_DIR/config.worktree, so the common-dir read does pick it up — this repo's own prewarm_git_config_from_common_dir docstring relies on exactly that ("the common dir … sees the full merged set", vs. a linked-worktree read "missing values set on the main worktree's config.worktree"). Under extensions.worktreeConfig, then, a key placed in the main worktree's worktree-scoped file supplies project config repository-wide — the opposite of what the reader is told to expect. Two inline suggestions.
The diagnostic's value omission is defeated inside the same artifact. config_show_output deliberately prints key names without values because "diagnose output is routinely pasted into public bug reports" — but the report's own trace section embeds the raw git config --list -z output, values included, as the integration test concedes in its scoping comment. That matters more than it would for an ordinary git key, because this source's stated purpose is private configuration and the triage path (running-tend) tells reporters to run wt -vv and gh gist create the bundle. Calling it pre-existing is fair for git config in general, but the values only get there because this feature put them there. Worth deciding between the two consistent endpoints: redact worktrunk.config.* values in the trace layer too, or drop the value omission and let the report say plainly that git config values are included (matching the module's Privacy section, which already lists "config files"). The current middle reads as a guarantee it doesn't deliver.
Smaller notes:
- Git lowercases the final key component, so user-chosen names get silently lowercased:
worktrunk.config.aliases.Deploylands asdeploy(verified against git). Fine as behavior, but it isn't covered by "keys must be written in lowercase — exactly how the schema spells them", since alias names aren't schema-spelled. Folded into the docs suggestion. warn_superseded_project_file's "SET it only on emit" comment justifies itself withwt config create --project's born-superseded warning, but that warning is an independenteprintln!increate.rsand never consults this latch. The behavior is right; the reason given for it isn't. Inline.
|
|
||
| `.git/config` is local to the repository and never committed, so these keys stay on one machine — and every linked worktree sees them, because the local scope lives in the shared git dir. `--global` puts a key in every repository. Git's normal precedence applies: local overrides global, and conditional includes work. | ||
|
|
||
| Selection is all-or-nothing. When any `worktrunk.config.*` key exists, those keys are the complete project config and `.config/wt.toml` is ignored — a warning names the superseded file. There is no key-level merging between the two sources. To return to the file, remove the keys. |
There was a problem hiding this comment.
The two facts are stated separately but never multiplied: --global reaches every repository, and selection is all-or-nothing. Together they mean one global key set for convenience in one repo silently disables the committed project config — and every project hook — in all of them. The load-time warning catches it, but only after the fact.
| Selection is all-or-nothing. When any `worktrunk.config.*` key exists, those keys are the complete project config and `.config/wt.toml` is ignored — a warning names the superseded file. There is no key-level merging between the two sources. To return to the file, remove the keys. | |
| Selection is all-or-nothing. When any `worktrunk.config.*` key exists, those keys are the complete project config and `.config/wt.toml` is ignored — a warning names the superseded file. There is no key-level merging between the two sources. To return to the file, remove the keys. Combined with `--global` this reaches every repository at once: a single global key supersedes the committed project config of every repository on the machine, so keep keys repository-local (or scope them with `includeIf`) unless that is the intent. |
The mirrors under docs/content/ and skills/worktrunk/reference/ regenerate from this.
|
|
||
| Values are strings, one per key. Settings that need other TOML types (such as the `step.copy-ignored.exclude` array) cannot be expressed here. Hooks and aliases defined this way go through the same approval prompt as file-based project config. | ||
|
|
||
| Use the canonical key spellings from the sections above. Deprecated spellings may still deserialize, but git-sourced configuration does not run file migration or emit deprecation guidance — `wt config update` has nothing to rewrite here. Per-worktree git config (`extensions.worktreeConfig`) is not supported: keys are read from the shared git dir, so a `config.worktree` value is never consumed. Setting `WORKTRUNK_PROJECT_CONFIG_PATH` — even to an empty value — disables this source entirely; the override names the project config source outright. |
There was a problem hiding this comment.
Two corrections here.
a config.worktree value is never consumed holds only for a linked worktree. The main worktree's worktree-scoped file is $GIT_COMMON_DIR/config.worktree, so the common-dir read that all_config() performs does include it — prewarm_git_config_from_common_dir's docstring depends on that exact property. A key placed there under extensions.worktreeConfig therefore supplies project config repository-wide.
Second, git lowercases the final key component, so a user-chosen name is silently renamed: git config worktrunk.config.aliases.Deploy 'make deploy' lists as worktrunk.config.aliases.deploy and is invoked as wt deploy. "Use the canonical key spellings from the sections above" doesn't cover it, since alias names aren't schema-spelled.
| Use the canonical key spellings from the sections above. Deprecated spellings may still deserialize, but git-sourced configuration does not run file migration or emit deprecation guidance — `wt config update` has nothing to rewrite here. Per-worktree git config (`extensions.worktreeConfig`) is not supported: keys are read from the shared git dir, so a `config.worktree` value is never consumed. Setting `WORKTRUNK_PROJECT_CONFIG_PATH` — even to an empty value — disables this source entirely; the override names the project config source outright. | |
| Use the canonical key spellings from the sections above. Git lowercases the final component of a key, so a user-chosen name is lowercased with it — an alias set as `worktrunk.config.aliases.Deploy` is invoked as `wt deploy`. Deprecated spellings may still deserialize, but git-sourced configuration does not run file migration or emit deprecation guidance — `wt config update` has nothing to rewrite here. Per-worktree git config (`extensions.worktreeConfig`) is only partly reachable: keys are read from the shared git dir, so a linked worktree's `config.worktree` is never consumed, while the main worktree's is — and then for the whole repository. Setting `WORKTRUNK_PROJECT_CONFIG_PATH` — even to an empty value — disables this source entirely; the override names the project config source outright. |
| //! - **No worktree scope.** The bulk config read runs from the common git | ||
| //! dir, so `config.worktree` values (`extensions.worktreeConfig`) are | ||
| //! never consumed. The diagnostic command, run inside a linked worktree, | ||
| //! can therefore list matching keys this source ignores. |
There was a problem hiding this comment.
The bullet's claim is true of a linked worktree but not of the main one: config.worktree for the main worktree lives at $GIT_COMMON_DIR/config.worktree, and all_config() runs git config --list -z with the common dir as cwd, so git reads it. prewarm_git_config_from_common_dir's docstring states the same thing from the other direction — the common-dir read "sees the full merged set", where a linked-worktree read is "missing values set on the main worktree's config.worktree".
So under extensions.worktreeConfig, a worktrunk.config.* key in the main worktree's config.worktree is consumed, and it supplies project config for every worktree — which is the surprising half and the one worth spelling out.
| //! - **No worktree scope.** The bulk config read runs from the common git | |
| //! dir, so `config.worktree` values (`extensions.worktreeConfig`) are | |
| //! never consumed. The diagnostic command, run inside a linked worktree, | |
| //! can therefore list matching keys this source ignores. | |
| //! - **No linked-worktree scope.** The bulk config read runs from the common | |
| //! git dir, so a *linked* worktree's `config.worktree` | |
| //! (`extensions.worktreeConfig`) is never consumed. The main worktree's | |
| //! `config.worktree` lives in the common dir and therefore is — supplying | |
| //! project config repo-wide, not just there. The diagnostic command, run | |
| //! inside a linked worktree, can list matching keys this source ignores. |
| // would consume it on the no-file path, and a later | ||
| // `wt config create --project` in the same invocation would then have | ||
| // its born-superseded warning silently suppressed. |
There was a problem hiding this comment.
The stated reason doesn't hold: wt config create --project's born-superseded warning is its own eprintln! pair in create.rs (guarded only by !repo.worktrunk_config_git_pairs()?.is_empty()), and never reads this latch — so setting WARNED up front couldn't suppress it. Emit-only latching is still the right behavior; it just needs a reason it actually has.
| // would consume it on the no-file path, and a later | |
| // `wt config create --project` in the same invocation would then have | |
| // its born-superseded warning silently suppressed. | |
| // would consume it on the no-file path, leaving a later load in the same | |
| // invocation — one that does have a superseded file to report — silently | |
| // latched out of the only warning that names it. |
2 participants
What this adds
An experimental second source for project configuration:
worktrunk.config.*keys in git config.One mechanical rule governs the mapping. Strip
worktrunk.config.; the remainder is the exact key path from.config/wt.toml.worktrunk.config.post-startis the top-levelpost-starthook.worktrunk.config.list.urlis[list] url. No renamed keys, no parallel schema.Why
.git/configis never committed, never pushed, never cloned. Configuration written there is private to one machine by construction — and shared across every linked worktree, because the local scope lives in the common git dir. That combination is what issue 3454 asks for, and what several earlier issues (650, 1077, 2818) kept circling: private, repo-local config without a new file format and without repo identities leaking into public dotfiles.How selection works
All-or-nothing. When any
worktrunk.config.*key exists, those keys are the complete project config;.config/wt.tomlis ignored. There is no key-level merging between sources.Two consequences follow, both deliberate:
git config --show-scope --show-origin --get-regexp '^worktrunk\.config\.'.wt config showandwt hook showname the active source.What stays the same
include/includeIf(a cloned dotfiles repo, for instance), so source alone is not a trust signal.git config --list -zmap — zero new subprocesses. Local overrides global; conditional includes work; worktrunk never sees scopes..config/wt.toml,[projects."…"]overrides, andWORKTRUNK_PROJECT_CONFIG_PATHare untouched.Limits in this first version
Values are strings, one per key. Fields needing other TOML types (
step.copy-ignored.exclude, an array) are not expressible and error clearly when attempted. Keys must be lowercase — git's key model makes middle segments case-sensitive, and the schema's own spelling is lowercase. No write commands:git configalready is the management interface.Testing
Unit tests cover the key mapping: nesting, value/table conflicts, empty segments, loud failure on non-string fields. Integration tests cover source selection, the supersession warning firing exactly when a file is superseded, local-over-global precedence,
include.pathresolution, approval gating, andwt config showin human and JSON forms. The config docs page gained a "Private project config in git config" section, and all generated doc mirrors and help snapshots are refreshed.