Repository navigation
fix(config): migrate deprecated keys in an inline [projects] scope - #4122
Conversation
for_each_config_table_mut visited only Item::Table for the projects container and for each project entry, so a scope written inline was skipped by every rule on that walk. The deprecated key then reached serde at a path with no alias and was dropped from the typed config. Closes #4116
Covers both shapes the scope walk has to step over: a scalar entry and an array-of-tables entry.
worktrunk-bot
left a comment
There was a problem hiding this comment.
Self-review. The migration behavior itself checks out — I ran the inline shapes
through migrate_content and compute_migrated_content, and the inline project
entry, the fully-inline projects container, and a dotted merge.no-ff = true
inside an inline entry all migrate and reach the typed config. Three points:
The changelog entry lists [ci] among the renames this walk covers, but
migrate_ci_doc never calls for_each_config_table_mut — it reads forge and
ci off the document root and writes there. Both
[projects."a/b".ci]\nplatform = "gitea" and its inline equivalent come back
byte-identical with no deprecation reported, so the entry promises a migration
that doesn't exist for that key.
migrate_inline_scope writes the migrated table back only when f returns
true, so a rule that mutates its scope table and then reports no change keeps
that mutation on the standard-table path and loses it on the inline one. None of
the five rules on the walk does that today. But migrate_section_doc already
computes its return from two independent outcomes (moved, dropped), and the
module's design rests on rules whose predicate and rewrite can't drift — this is
a new way for them to drift, silently and only in one shape. Worth stating as a
requirement on f where the contract lives.
test_migrate_leaves_project_entries_that_are_not_tables_alone doesn't pin what
its comment says it pins. result.contains("\"a/b\"") is implied by the
disjunct next to it, and neither checks the entry's value survived. For the
array-of-tables case, migrating the no-ff = true inside [[projects."a/b"]]
would satisfy both assertions — ff = false comes from the top-level [merge].
`migrate_ci_doc` reads and writes the document root; it never routes through `for_each_config_table_mut`, so `[ci]` has no per-project scope in either shape and this walk doesn't reach it. Also record what the conditional inline write-back requires of `f`, pin the text each non-table `[projects]` entry has to keep verbatim, and borrow the scope key instead of allocating it per entry.
worktrunk-bot
left a comment
There was a problem hiding this comment.
affected tests (macos, advisory) is red on this head: integration_tests::switch_picker::test_switch_picker_prs_rows_survive_alt_x_removal panicked after 30s waiting for Loading open PRs to appear, with a blank picker frame on screen. Flake, not a regression — the required test (macos) job ran the same 4876 tests on this exact commit and reported 4876 passed, the advisory macOS job was green on the previous head, and the diff reaches only src/config/deprecation.rs and CHANGELOG.md, which that picker test does not exercise. Every required check and codecov/patch are green, so the red advisory job needs no rerun before merge.
# Conflicts: # CHANGELOG.md
Drop this PR's CHANGELOG.md entry. Changelog entries are written from the commits at release time (CLAUDE.md, Releases), so an open PR carrying an entry drops it rather than resolving a conflict on it.
# Conflicts: # src/config/deprecation.rs
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Version bump to 0.78.0 plus the changelog section for this release, a comment-preservation fix the release review turned up, a PTY test-filter fix, and merges of `main` (commits that landed during the review, including #4124, #4122, #4141, #4140 and #4145). **Bump level**: minor. `cargo semver-checks` reports 2 breaking library changes — a new `leftover_branch` field on the `GitError::WorktreeCreationFailed` struct variant, and `DEPRECATED_TEMPLATE_VARS` removed. Pre-1.0, so a breaking change takes a minor bump. ## Comment-preservation fix A config save — declining the commit-generation offer, say — dropped the trailing comment on every value it rewrote. That was already true in v0.77.0 for a line the save itself changed (`skip-commit-generation-prompt = false # note` → `true`, reproduced on the v0.77.0 binary). #4080 widened it to lines the command never touched: retired template variables now migrate on every load, so the next unrelated save rewrote any line naming one. #4080's own PR said this "needs a call before merge" and merged without one. - `replace_keeping_decor` keeps the value's decor (spacing after `=`, trailing comment) when `merge_tables` replaces a scalar. Comments *above* a line sit on the key and were never at risk. - `main`'s #4120 independently reworked the inline-table arm and moved a key's leading comment onto the rewritten header, but documented that a trailing comment after the closing brace "is still dropped". `replace_inline_with_table` now carries it after the header's `]`, so both of its callers keep it. - Tests: the existing comment-preservation test goes back to a retired name (the only fixture that exercises the replacement) and adds a comment on the mutated line; a new test snapshots and re-parses an inline `[projects]` entry. Each fails without its half of the fix. Not fixed, and identical on v0.77.0: a save deletes an explicitly written *default* value (e.g. `skip-commit-generation-prompt = false # note`) as a stale key, comment included. ## `--help` correction #4104 moved the OpenCode plugin path to etcetera's `Xdg` strategy, which uses `$XDG_CONFIG_HOME` only when absolute — a deliberate, documented choice — but `wt config plugins opencode --help` still said install mirrors OpenCode's precedence. OpenCode reads a relative value as-is, so the help now says an empty `$OPENCODE_CONFIG_DIR` and an empty or relative `$XDG_CONFIG_HOME` are ignored. ## Data-loss surface review Independent finders swept the full diff, including the five commits merged from `main`. Nothing holds the tag; candidates were adjudicated against v0.77.0 binaries: - **Fixed in this release, reproduced on v0.77.0**: #4104 stops `wt config shell install` (with Nushell) deleting a `nushell/vendor/autoload/wt.nu` under the current directory when `$XDG_CONFIG_HOME` is empty or relative; #4120 stops a save or migration from writing an inline section's comment inside `[commit]` brackets, which made the config unparseable, and from dropping unknown keys. - **`wt step prune --min-age` doesn't apply where no reflog exists** — pre-existing, proven by building the pre-#4077 binary. `git clone --bare` and `git fetch` into `refs/heads/*` write no reflog, so in the bare layout `tips-patterns.md` recommends every branch starts unprotected. Docstring corrected here; behavioral fix is follow-up. - **`wt config update --output <path>` replaces any named file whole**, no existence check or prompt — pre-existing, follow-up. - **`wt config shell install fish --yes` replaces a hand-written wrapper at the XDG path** — accepted ("ownership is the path", predates #3602), newly reachable only for users with `$XDG_CONFIG_HOME` set. The FAQ's delete inventory now cross-references it. - **`--var <retired-name>` binds a name nothing references** after #4080 — inert, no shipped usage; follow-up. - **#4135 migrates nothing**: an existing oh-my-pi hook stays put, `pi uninstall` points at `omp uninstall`. `PI_CODING_AGENT_DIR='~/x'` is used literally (Pi expands `~`) — creates a stray directory, deletes nothing; follow-up. ## Changelog 37 entries; each new or edited entry was re-verified against v0.77.0 until a pass came back clean (six passes in all). Passes caught, among others: the OpenCode `-x` recipe was wrong since 0.76.0, not 0.75.0; #4080's JSON reaches every hook, not just `pre-start`/`post-start`; a quoted git error string git never prints; a copy-button overlap that existed only inside this release; #4104's relative-`XDG_CONFIG_HOME` OpenCode change described as a fix when OpenCode reads that path as-is; and which commands a broken inline section stopped. **Fixed** leads with the data-loss entries. ## Open PRs checked against v0.77.0 - **#4124** fixed a bug new in this release, caused by #4080, and has landed and been merged in here. The hand-rolled template-variable scanner it extends is slated for removal; see its commit message. - **#4112** — no data-loss regression vs v0.77.0; two small non-safety regressions from #4111 (an uncapped auto-staging file list, a narrow `Nothing to commit` refusal under `status.showUntrackedFiles = no`). Not merged, by decision. - **#4122** (merged in) and **#4123** — pre-existing bugs, identical on v0.77.0. ## PTY filter fix `test (macos)` failed once on a shell's failed-`setpgid` diagnostic that landed straight after the capture's trailing `\x1b[0m`, with no newline between, so #4136's line-anchored filter missed it. The filter now also matches right after an SGR escape and keeps the escape; its test gains that shape and fails on the old pattern. ## Validation - `cargo run -- hook pre-merge --yes` — 4930 tests passed, 1 skipped; clippy, fmt, doc-sync, snapshot and lockfile checks green - Nightly on the original cut-from tip (https://github.com/max-sixty/worktrunk/actions/runs/35060617020): green across `full-tests` on linux/macos/windows, `feature-powerset`, all `release-target`s, `nix-flake`, `minimal-versions`, `link-check` > _This was written by Claude Code on behalf of max-sixty_ 🤖 Generated with [Claude Code](https://claude.com/claude-code)
worktrunk 0.78.0 Created-by: HarmonybrewBot Commit-by: HarmonybrewBot Merged-by: HarmonybrewBot Description: Created by `brew bump` --- Created with `brew bump-formula-pr`.<details> <summary>release notes</summary> <pre>## Release Notes ### Improved - **Hook scripts read renamed context keys**: the JSON piped to every hook drops `worktree`, `repo_root`, `main_worktree`, and `main_worktree_path` in favour of `worktree_path`, `repo_path`, `repo`, and `primary_worktree_path`. Config templates migrate on load, so hooks written with the old names keep working; scripts reading the JSON, `--execute` templates, and `--var` overrides need updating. (Breaking.) ([#4080](max-sixty/worktrunk#4080)) - **`wt config plugins pi` installs for Pi; oh-my-pi moves to `wt config plugins omp`**: `pi install` wrote an oh-my-pi hook, which Pi never loads; Pi users re-run it to get a Pi extension. Existing oh-my-pi hooks keep working, listed as outdated until `wt config plugins omp install`. (Breaking: scripts that run `wt config plugins pi` for oh-my-pi need `omp`.) [Docs](https://worktrunk.dev/claude-code/#oh-my-pi) ([#4135](max-sixty/worktrunk#4135), thanks @ashebanow) - **Interactive prompts no longer open with a blank line**: `wt config shell install`, `wt config plugins claude install`, and the commit-message setup offer in `wt merge` began with one. ([#4059](max-sixty/worktrunk#4059)) - **`wt config show` gives a single `wt config shell install` hint**: an outdated wrapper, a fish wrapper at the deprecated `conf.d` path, and missing fish completions each printed their own hint. The zsh `compinit` snippet is now syntax-highlighted. ([#4059](max-sixty/worktrunk#4059)) - **`wt config update --output <path>` confirms the write**: it now prints `✓ Wrote user config migration @ ~/migrated.toml` where it printed nothing. `--output=-` stays silent. ([#4053](max-sixty/worktrunk#4053)) ### Fixed - **Removal no longer discards untracked files hidden by `status.showUntrackedFiles = no`**: under that setting, a worktree holding only untracked files looked clean, so `wt remove`, `wt merge`, and `wt step prune` removed it without `--force`. Safety checks now always count untracked files, and auto-staging warnings list every file they stage. ([#4111](max-sixty/worktrunk#4111), thanks @Duang777) - **`wt remove` and `wt merge` check the right worktree under an inherited `GIT_DIR`**: run from a `!wt` git alias or a git hook, which pass `GIT_DIR` down, `wt remove` could delete a dirty worktree without `--force`, and `wt merge` could refuse a clean merge. Both now check the worktree they act on. ([#4082](max-sixty/worktrunk#4082), thanks @Duang777) - **`wt merge` no longer deletes a worktree locked with `git worktree lock`**: the merge completes and prints `Worktree preserved (locked)`. Every Worktrunk removal refuses a locked worktree, even with `--force`; run `git worktree unlock <path>` first. ([#4073](max-sixty/worktrunk#4073), thanks @Duang777) - **`wt config update` no longer overwrites a config edited while its prompt is open**: it wrote the pre-prompt snapshot over the newer file and reported success. It now fails with the path and leaves the newer file; re-running previews the current contents. ([#4127](max-sixty/worktrunk#4127), thanks @zach-hammad-vs for reporting) - **`wt step prune` no longer deletes a branch created minutes ago from an older commit**: `--min-age` aged a branch without a worktree by its commit's date, so a new branch off a day-old commit was pruned on the next run. Age now comes from the branch's oldest reflog entry. ([#4077](max-sixty/worktrunk#4077)) - **A comment or blank line above an inline config section no longer breaks the config**: a save or migration that rewrote `commit = { … }` as `[commit]` put that line inside the brackets. `wt switch`, `wt merge`, `wt remove`, and several `wt step` commands then failed with `Failed to load config`; `wt list` warned and ran without the config. Saves also dropped unrecognized keys from inline sections. ([#4120](max-sixty/worktrunk#4120), thanks @zach-hammad-vs for reporting) - **Config saves keep the trailing comment on a value they rewrite**: a save such as answering the commit-generation offer dropped the comment after each value it rewrote, including one after an inline section's closing brace. ([#4138](max-sixty/worktrunk#4138)) - **Ignored project-config keys warn from every worktree**: an unknown key in `.config/wt.toml`, or one that belongs in user config such as `[merge]`, warned only in the main worktree, so `wt merge` from a linked worktree ignored it silently. ([#4145](max-sixty/worktrunk#4145), thanks @ee-prog for reporting) - **`wt merge` skips `post-commit` when it removes that hook's worktree**: before, the hook ran in the primary worktree when the removed one sat inside the repository, and otherwise didn't run. The merge now prints `▲ Skipped post-commit`; move work that must finish there to `pre-remove`. ([#4049](max-sixty/worktrunk#4049)) - **`wt switch` recovers from a removed worktree in a bare repository**: `wt switch` and the picker surfaced git's raw `fatal: Unable to read current working directory` instead, and the removed-directory message lacked `wt switch ^`. ([#4067](max-sixty/worktrunk#4067)) - **`wt config shell install fish` puts the wrapper where fish reads it under a custom `$XDG_CONFIG_HOME`**: the wrapper always went to `~/.config/fish/functions/`, so install reported success while `wt switch` changed no directory. On Windows, the fish completion moves from `%APPDATA%\fish\completions` to the wrapper's directory. ([#4107](max-sixty/worktrunk#4107)) - **`wt config shell install zsh` honours `$ZDOTDIR` only when it is absolute**: an empty or relative value resolved against the current directory, so install appended to a `.zshrc` under the current directory and reported success while the file zsh reads went untouched. A non-absolute value now falls back to `$HOME`. ([#4085](max-sixty/worktrunk#4085)) - **`wt config shell install` for Nushell no longer deletes a `wt.nu` under the current directory**: an empty or relative `$XDG_CONFIG_HOME` resolved against it, so install removed a `nushell/vendor/autoload/wt.nu` beneath it unread, as a stale wrapper. It now falls back to `~/.config`, which also fixes unread `tea` logins that let `wt switch pr:<n>` treat a Gitea host as GitHub. ([#4104](max-sixty/worktrunk#4104)) - **`wt switch --create` names the branch a failed worktree add leaves behind**: `git worktree add -b` writes the branch ref before populating the worktree, so a failure in between left the branch with no worktree, and the next attempt reported a name collision. The error now says how to delete or reuse it. ([#4109](max-sixty/worktrunk#4109), thanks @technicalpickles for reporting) - **`wt config shell install` no longer tells an already-wrapped shell to restart**: reinstalling from a shell with the wrapper loaded, such as after a version bump, printed `↳ Restart shell to activate shell integration`. ([#4059](max-sixty/worktrunk#4059)) - **`wt config shell install` migrates a deprecated `conf.d` fish wrapper when `~/.config/fish/functions` is missing**: before, only `wt config shell install fish` migrated it. `wt config show` now reports that wrapper even when fish isn't on `PATH`. ([#4059](max-sixty/worktrunk#4059)) - **Plugin status and uninstall checks ask the agent CLIs**: `wt config show` could list an installed Claude Code plugin or Gemini CLI extension as not installed, and `wt config plugins claude|codex uninstall` could report success over a marketplace still configured. A repeat Claude uninstall now runs both removals instead of stopping at `Plugin not installed`. ([#4048](max-sixty/worktrunk#4048), [#4054](max-sixty/worktrunk#4054)) - **`wt config plugins opencode install` no longer writes the plugin into the current directory**: with `OPENCODE_CONFIG_DIR` set but empty, it wrote `plugins/worktrunk.ts` there, typically inside a repository, reporting success though OpenCode never saw it. An empty value now counts as unset. ([#4084](max-sixty/worktrunk#4084)) - **The OpenCode plugin clears its activity marker when OpenCode 1 disposes it**: only explicit session deletion cleared it, so a finished session kept its `💬` and `wt list` showed it as active until `wt config state marker clear` was run by hand. ([#4129](max-sixty/worktrunk#4129), thanks @drewdas for reporting) - **`wt config update` migrates `[ci] platform` into a `[forge]` section that only sets `hostname`**: that config, common with GitHub Enterprise and self-hosted GitLab, kept the deprecated key with no deprecation warning. ([#4061](max-sixty/worktrunk#4061)) - **Deprecated keys in an inline `[projects]` entry now migrate**: `"example.com/org/repo" = { merge = { no-ff = true } }` skipped the migration the section form got, so the per-project setting was ignored. ([#4122](max-sixty/worktrunk#4122), thanks @zach-hammad-vs for reporting) - **A deprecated config section the migration can't rewrite now warns**: `select = "not a table"`, or a `[select]` whose `[switch.picker]` destination is already set, was ignored without a message. It now reports `unknown field select (will be ignored)`. ([#4121](max-sixty/worktrunk#4121), thanks @zach-hammad-vs for reporting) - **`wt config update` no longer changes what a template renders**: it could rename `{{ repo_root }}` inside a quoted s See merge request: Harmonybrew/homebrew-core!20435
Problem
for_each_config_table_mut— the scope walk behind every per-project deprecation rule — matched onlyItem::Table, for theprojectscontainer and for each entry inside it. TOML's two equivalent shapes therefore migrated differently:The skipped form is not merely left unmigrated.
UserProjectOverrides'sMergeConfighasffand nono-ffalias, so the unmigrated key reaches serde at a path with no field, is dropped from the typed config, and the per-project non-fast-forward merge silently stops applying. An unknown-field warning may fire, but the setting is gone either way.The rule reached inside an inline
merge = { … }already —migrate_negated_bool_inlineexists for exactly that — so the gap was the outer scope, one level up.Solution
The walk now visits a
projectscontainer and a project entry in either shape. An inline entry is migrated through a table view and written back inline, and only when a rule reported a change, so an entry nothing touched keeps its formatting.This covers every rule on that walk, not just
merge.no-ff:switch.no-cd, the removedlist.task-timeout-ms, and the[select]/[commit-generation]section renames.[ci]→[forge]is not among them —migrate_ci_docreads and writes the document root directly, so it has no per-project scope in either shape.rename_hook_keyandremove_approved_commands_dockeep their own table-only traversals ofprojects— #4116 flags them as worth an owner-level look. They are left alone here: both move or delete whole items rather than rewriting a key inside a section, so the fix is not the same shape, and folding them into the shared walk is a larger change than this bug needs.Testing
test_migrate_no_ff_inline_project_scope— asserts both the migrated text and that the setting reaches the typed config (config.projects[…].merge.ff == Some(false)), which is the part that was silently lost.test_migrate_no_ff_fully_inline_projects_table— theprojectscontainer itself inline.test_migrate_leaves_unrelated_inline_project_keys_alone— an entry no rule touched keeps its formatting, and sibling keys in a migrated entry survive the table round-trip.test_warning_fires_iff_update_changesgains fourrewrittenentries covering the inline scope acrossmerge.no-ff,list.task-timeout-ms, the[select]rename, andswitch.no-cd, so the framework invariant is checked for the newly-reachable shapes too.All four fail before the change.
cargo test --libpasses apart fromcopy::tests::test_copy_dir_recursive_does_not_count_a_non_regular_file, which fails onmainin this sandbox too (socket(AF_UNIX, …)is blocked).cargo clippy --all-targetsandcargo fmt --checkare clean.Reported by @zach-hammad-vs from source inspection; the reproduction above confirms it.
Closes #4116 — automated triage