fix(config): let wt config update --print read project config from a linked worktree - #4015
fix(config): let wt config update --print read project config from a linked worktree#4015worktrunk-bot wants to merge 2 commits into
Conversation
…linked worktree The main-worktree requirement in check_project_config exists to keep the rewrite off a linked checkout, but it also gated --print, which writes nothing. From a linked worktree -- where a worktree tool's users mostly stand -- wt config update --print emitted an empty stdout, silently dropping the project config from the pipe, plus a stderr hint naming 'wt -C <main> config update' (a writing command) in answer to a read. Gate the rewrite on !is_linked as before; gate --print on nothing.
worktrunk-bot
left a comment
There was a problem hiding this comment.
The production change is right: --print writes nothing, handle_config_update returns before the write loop, and check_and_migrate has no filesystem side effects, so widening actionable can't reach the rewrite. One test gap and two observations.
The regression test's write-freedom assertion checks the wrong file. From a linked worktree, project_config_path() resolves against the current worktree root, so the command reads and would write <feature-print>/.config/wt.toml — not the main worktree's copy the test snapshots into before. Verified against a scratch repo with the two copies deliberately diverged: --print from the linked worktree printed the linked copy. The final assert_eq! therefore passes no matter what the command does to the file at risk, which is the half of the --print contract this test exists to pin. Inline suggestion below adds an assertion on the file it actually reads; I applied it locally and the test passes (cargo fmt --check clean).
Two things I'm not asking for in this PR:
- The same read-vs-write conflation is still live in
wt config show.format_deprecation_detailsinsrc/config/deprecation.rsreturns early onif let Some(main_path) = &info.main_worktree_path, so from a linked worktree it prints the warnings and theTo apply:hint but skips the migration diff — a read-only render withheld for a reason that only applies to the rewrite, the same shape this PR fixes for--print. - Related, and pre-existing: the retained
!actionablebranch hintswt -C <main> config update, which acts on the main worktree's.config/wt.toml. Since.config/wt.tomlis tracked, that can be a different file with different (or no) deprecations than the linked worktree's copy the warning was derived from. Worth a separate look at whether the write path should refuse a linked worktree at all, rather than redirect to a file the user wasn't asking about.
Heads-up on overlap: #3999 edits the --print block's "stderr stays empty" contract, adding a dropped-approved-commands warning on stderr in print mode. No textual conflict with this diff, and the new test's fixture has no approved-commands so its stderr.is_empty() assertion still holds — but whichever lands second should re-read the other's contract wording.
…grated The write-freedom assertion checked the main worktree's .config/wt.toml, which the command never touches when run from a linked worktree: project_config_path() resolves against the current worktree root, so the file read — and the one a regression would rewrite — is <feature-print>/.config/wt.toml. Assert on that copy too.
|
Superseded by #4021, which landed on That PR replaced What is left of this branch after merging |
wt config update --printrenders the migrated config to stdout and writes nothing, butcheck_project_configgated it behind the same main-worktree requirement that guards the rewrite. From a linked worktree — where a worktree tool's users mostly stand — the command emitted an empty stdout, silently dropping the project config from the pipe, plus a stderr hint namingwt -C <main> config update, a writing command, in answer to a request that only reads. The rewrite still requires the main worktree;--printno longer does.Found by the nightly survey while reading
src/commands/config/update.rs, then reproduced against a scratch repo before the fix.Reproduction and verification
Against a repo whose committed
.config/wt.tomluses the deprecated{{ repo_root }}:The stderr half also contradicts the stated contract a few lines up in the same function's caller — "
--printis for piping, so stderr stays empty" — whichtest_config_update_print_emits_migrated_without_writingalready pins, but only for user config from the main worktree.Regression test:
test_config_update_print_emits_project_config_from_linked_worktreeasserts the migrated project config reaches stdout, stderr stays empty, and the file is untouched. Confirmed it fails onmain(got:— empty stdout) and passes with the fix.Verification:
cargo clippy --all-targetsandcargo fmt --checkclean; 137config_show/config_updateintegration tests pass, includingtest_config_update_project_config_from_linked_worktree_shows_hint, which pins that the writing path still declines from a linked worktree.