Skip to content

Drop config keys a migration's destination cannot hold - #3994

Merged
max-sixty merged 3 commits into
mainfrom
land/config-update-drop-unsupported
Sep 3, 2026
Merged

max-sixty merged 3 commits into
mainfrom
land/config-update-drop-unsupported

Conversation

@max-sixty

@max-sixty max-sixty commented Sep 2, 2026 •

Copy link
Copy Markdown
Owner

wt config update migrates [select] to [switch.picker] and [commit-generation] to [commit.generation] by moving the whole table. Any key the destination struct has no field for moved along with it, and then warned as an unknown field on every later command — at a path the user never typed, and one that wt config update, the command the warning points at, would rewrite rather than clear. The warning outlived every fix available to the user.

Given this config:

[select]
pager = "delta"
height = "50%"

[switch.picker] has a pager field and no height, so the update used to write height into [switch.picker] and leave ▲ unknown field height on every command from then on. It now drops the key and reports it:

$ wt config update --yes
▲ User config: [select] height is no longer supported and will be removed

The rewritten config loads clean. drop_unsupported_keys does the removal, and a key removal that empties a [projects."<id>"]-scoped section removes the section too, for the same reason.

Two boundaries the implementation keeps:

  • A key that is merely misplaced — valid in the other config file, such as a user-only commit.generation.command written into project config — stays, and keeps its redirect warning. That is why drop_unsupported_keys is given the union of the destination's schemas across config kinds.
  • This does not conflict with "never silently drop an old config key": the removal is reported, and the key had no home in either config file.

schema_property_names is factored out of schema_top_level_keys so section types can be queried the same way. It cannot see #[serde(alias)], so the destination schemas must not carry one; the docstring says so.

Both wholesale-move rules go through one migrate_section_doc, which holds the contract they share — record the rename only for a scope that got a replacement written, count a drop-only outcome as a change, emit renames before drops — so a third such rule is a table row and a migrator rather than a third copy. wt config update's help now says it removes keys as well as updating and adopting them; over an open-ended class of keys, that is the thing to know before putting --yes in a dotfiles script.

test_warning_fires_iff_update_changes now pins both halves of the invariant: what the update writes raises no deprecation warning and no unknown-field warning.

UX survey items: #8.

Reviewable files: src/config/deprecation.rs, src/config/mod.rs, tests/integration_tests/config_show.rs, and the Config Deprecation paragraph in CLAUDE.md. Generated mirrors and snapshots are regenerated.

This was written by Claude Code on behalf of max-sixty

🤖 Generated with Claude Code

https://claude.ai/code/session_01XAUYWFN9d9oh6jyoQiouHb

max-sixty and others added 2 commits September 1, 2026 16:54
A section rename that moved its table wholesale carried every key over,
including keys the destination struct has no field for. `[select] height =
"50%"` became `[switch.picker] height = "50%"`, so every command printed

    ▲ User config has unknown field switch.picker.height (will be ignored)

naming a path the user never typed, and `wt config update` — the command the
deprecation hint points at — wrote that path into the file instead of clearing
it. The warning outlived every fix available to the user.

The two wholesale-move rules now remove what the destination cannot hold and
report each key:

    ▲ User config: [select] height is no longer supported and will be removed

`drop_unsupported_keys` takes the destination's schemars property names, so the
set follows the struct. For `[commit.generation]` it is the union of the user
and project schemas, so a merely misplaced key keeps its "belongs in user
config" redirect and only a key with no home in either file goes. A section
left empty by the removals writes no replacement at all.

`[ci]` → `[forge]` is unchanged. It moves `platform` alone and leaves other
keys where the user wrote them, so those still warn at their own path.

`remove_section_key_in` had the same failure by another route. A default
section serializes away and `seed_schema_skeleton` only puts top-level keys
back, so the `[projects."<id>".list]` that `list.task-timeout-ms` removal left
behind came back from the unknown-key round-trip as `unknown field
projects.<id>.list`. It now removes a project-scoped section its key removal
empties. A top-level section stays, keeping its position and the comments above
its header.

`DeprecationKind::Select` gains the per-scope payload `CommitGeneration`
already had, so a `[projects."<id>".select]` reports its own path rather than a
top-level `[select]` the user doesn't have. Both share `ScopedSections`.

An `args` under `[commit-generation]` that cannot merge into `command` is now
removed and reported rather than preserved. `[commit.generation]` has no `args`
field, and `args` was only ever a way of spelling part of `command`.

test_warning_fires_iff_update_changes gains the new cases and a second
assertion, that what an update writes raises no unknown-field warning either.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAUYWFN9d9oh6jyoQiouHb

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Two notes, neither blocking — one inline on the duplicated rule bodies, one here for a file the diff doesn't touch.

wt config update's after_long_help in src/cli/config.rs (the block opening Updates deprecated settings in user and project config files / to their current equivalents) describes updating and adopting, never removing. Removal isn't new — approved-commands and list.task-timeout-ms already went that way — but it's this PR's headline behavior and now covers an open-ended class of keys rather than two named ones, and that block is the primary source for the docs page someone reads before putting wt config update --yes in a dotfiles script. One clause after "to their current equivalents" would close it, e.g. "and removes deprecated keys that have no equivalent, reporting each". Happy to push that with the regenerated mirrors and help snapshots if you want it.

On the rest: the union-across-config-kinds boundary, --yes still printing each removal plus the full diff before the write, and the args shapes that now drop (all already non-functional — command absent fails is_configured, non-string command is a serde type error, a non-string element never joined) all check out. The one way this could take a supported key — SWITCH_PICKER_KEYS or COMMIT_GENERATION_KEYS collapsing to empty — is pinned by test_select_timeout_ms_removed_alongside_rename and test_project_config_keeps_misplaced_user_only_keys.

Comment thread src/config/deprecation.rs
`migrate_select_doc` and `migrate_commit_generation_doc` were the same eighteen
lines twice, differing only in the per-table migrator and the kind
constructor. The duplicated part is the wholesale-move contract this branch
introduces — record the rename only when something moved, count a drop-only
outcome as a change, emit renames before drops — so two copies means a third
rule re-derives it. `migrate_section_doc` holds it once and both rules become a
call.

`wt config update`'s help described updating and adopting, never removing.
Removal is now this command's headline behavior over an open-ended class of
keys, and that block is the primary source for the page someone reads before
putting `wt config update --yes` in a dotfiles script.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAUYWFN9d9oh6jyoQiouHb
@max-sixty

Copy link
Copy Markdown
Owner Author

Both folded in.

wt config update's after_long_help now reads "removes deprecated keys that have no equivalent and reports each one" — your wording, near enough. Nothing mirrored needed regenerating: the update subdoc marker that puts this block on the config page arrives in a later PR in this series, and there is no help_config_update snapshot, so test_docs_are_in_sync and test_help both come back clean.

The duplicated rule bodies are collapsed into migrate_section_doc — replied inline.

This was written by Claude Code on behalf of max-sixty

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One observation on scope, not a defect in the change. The two things I raised last round are both closed — migrate_section_doc holds the contract once, and the after_long_help clause is there. I confirmed the "nothing mirrored needed regenerating" claim independently: to their current equivalents appears nowhere under docs/, skills/, plugins/, or tests/snapshots/, so test_docs_are_in_sync and test_help are untouched by that edit.

The loop is still open for the configs the old migration wrote. [select] timeout-ms now drops and reports. But a user who ran wt config update on any earlier release already has [switch.picker] timeout-ms in their file — put there by the wholesale move this PR is fixing — and test_config_update_leaves_retired_keys_for_unknown_field_warnings pins that shape as permanent: has unknown field switch.picker.timeout-ms on every command, No deprecated settings found from update, file untouched. commit.generation.template-file and squash-template-file reached their paths the same way and are pinned alongside it. So the sentence the new CLAUDE.md paragraph opens with — the warning outliving every fix available to the user — is still true, for exactly the configs worktrunk wrote on the user's behalf.

I read #3949's call as deliberate: a retired key sitting at its canonical path isn't the deprecation layer's business, and that's right for a key the user typed there. What this PR makes visible is that the same key now gets opposite treatment depending on which release the user happened to run update on. Whether the relocated ones earn a row of their own is your call — it'd be one UpdateOnly rule over the three known paths, and that test would need splitting, since template-file is reachable both ways. Not blocking, and not something I'd fold into this PR; flagging it because the invariant this PR adds is what makes the divergence legible.

Comment thread src/config/deprecation.rs
///
/// This is the contract a wholesale-move rule owes, in one place: record the
/// rename only for a scope that actually got a replacement section written,
/// count a drop-only outcome as a change so the document is rewritten, and

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The rewrite isn't caused by this clause. migrate_section_doc calls for_each_config_table_mut as a statement and drops its bool, so the changed the closure returns goes nowhere — what actually decides the document was rewritten is modified |= !new_kinds.is_empty() in apply_rules, and a drop-only outcome reaches it by having pushed an UnsupportedKey. No bug: moved implies a recorded scope and a pushed rename kind, and a non-empty dropped implies pushed drop kinds, so the bool and the kinds can't disagree. But this is the contract docstring a third wholesale-move rule gets written against, and the clause credits a channel that isn't wired.

Suggested change
/// count a drop-only outcome as a change so the document is rewritten, and
/// report a drop-only outcome as a change to the scope walk, and

@max-sixty

Copy link
Copy Markdown
Owner Author

Agreed on both counts, and noted as a follow-up rather than folded in.

The divergence you describe is real: a key the user typed at a retired path is left alone by design, but the same key relocated there by an earlier wt config update is worktrunk's own doing, and it reads identically in the file. So which treatment a user gets turns on which release they happened to run update on. Carrying that item out of this PR — one UpdateOnly rule over the three known relocated paths, with test_config_update_leaves_retired_keys_for_unknown_field_warnings split because template-file is reachable both ways.

Merging this as it stands; the invariant it adds is what makes that case legible in the first place.

This was written by Claude Code on behalf of max-sixty

@max-sixty
max-sixty merged commit 469d73e into main Sep 3, 2026
43 checks passed
@max-sixty
max-sixty deleted the land/config-update-drop-unsupported branch September 3, 2026 01:12
max-sixty added a commit that referenced this pull request Sep 3, 2026
Adding the `update` subdoc marker is what puts `wt config update`'s
`after_long_help` on the config page, so #3994's removal clause reaches the
mirrors here rather than there.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAUYWFN9d9oh6jyoQiouHb
max-sixty added a commit that referenced this pull request Sep 9, 2026
`merge_args_into_command` declines to merge `args` into `command` unless
every element is a string, and an inline comment justified that by
saying the alternative would drop user data "while `args` was removed".
Since #3994, `drop_unsupported_keys` removes every key the destination
struct has no field for, and `[commit.generation]` has no `args` field —
so an unmerged `args` is removed a few rules later regardless, and the
comment described a preservation the code no longer performs.

Declining still changes the outcome, just not the one the comment named.
With the precondition, `[commit-generation] command = "claude", args =
[1, "--ok"]` migrates to `command = "claude"`, and the removal is
reported both as `▲ User config: [commit-generation] args is no longer
supported and will be removed` and as `-args = [1, "--ok"]` in the `wt
config update` diff preview. Without it (checked by building the
variant), it migrates to `command = "claude --ok"`: the `1` disappears,
`command` gains a value the user never wrote, and nothing says so. So
the precondition stays and the docstring now states the live reason.

The body collects the strings with a `let`-else, which makes the
precondition the extraction itself. That removes the separate invariant
a comment had to carry — and the four `unwrap()`s it guaranteed.

Behavior is unchanged; existing tests pin every branch. A new test
covers `args = []`, which had none.

> _This was written by Claude Code on behalf of max-sixty_

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
social4hyq pushed a commit to social4hyq/homebrew-core that referenced this pull request Sep 20, 2026
worktrunk 0.77.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

- **`wt list` sizes and aligns its columns to the terminal width**: one long branch name no longer sizes the Branch column for every row — it caps at 32 characters and elides, so a narrow terminal keeps its other columns instead of degenerating into a branch list. `Remote⇅` stops holding blank space open in a repo with no remote, and alignment now follows the value type. `--format=json` still carries the whole name. ([#3998](max-sixty/worktrunk#3998))

- **`wt list` names what it hid and marks detached worktrees**: the summary reads `hidden: Path, Commit` instead of counting columns, a detached worktree shows `⊘` rather than borrowing `⚑`, and a prunable row leaves its cells blank instead of showing loading dots that never resolve. ([#3998](max-sixty/worktrunk#3998))

- **`wt list --format=json` gains a `marker` field and stops reporting `detached` twice**: the branch marker set by `wt config state marker` is now readable without parsing it out of `symbols`, and `state` no longer reports a detached worktree as `branch_worktree_mismatch` — the sibling `detached` boolean carries it. ([#3998](max-sixty/worktrunk#3998))

- **`wt list --format=json` now defaults to schema 2**: callers get the envelope with repository metadata and orthogonal per-item facts without configuring `[list] json-schema`. `wt list statusline --format=json` follows the same key. Set `json-schema = 1` to retain the original bare-array format. (Breaking.) ([#4038](max-sixty/worktrunk#4038))

- **`wt config show` fails on a broken config**: it exits 1 on an unreadable or invalid config, an invalid `[list] columns`, or an invalid `approvals.toml`. Unknown keys and deprecations stay warnings and still exit 0. A new `APPROVALS` section counts project commands awaiting approval. (Breaking: it always exited 0 before.) ([#3999](max-sixty/worktrunk#3999))

- **Config migration output names its destination**: `wt config update --output <path>` writes the migration artifact to that file instead of applying it in place, and `--output=-` writes it to stdout. Output mode includes project config when run from a linked worktree. (Breaking: `--print` was removed.) ([#4021](max-sixty/worktrunk#4021))

- **Pi joins the agent integrations**: `wt config plugins pi install` writes an activity hook to `~/.omp/agent/hooks/pre/worktrunk.ts`, so Pi sessions show 🤖/💬 markers in `wt list` like Claude Code, Codex, OpenCode, and Gemini. `$PI_CONFIG_DIR`, `$PI_CODING_AGENT_DIR`, and named `$OMP_PROFILE`/`$PI_PROFILE` profiles are honored. [Docs](https://worktrunk.dev/claude-code/) ([#3594](max-sixty/worktrunk#3594), thanks @adity982, and @ortonomy for the request)

- **`wt config plugins codex install` installs the plugin too**: it previously registered the marketplace and left you to run `/plugins` in Codex yourself. Uninstall follows for both Codex and Claude Code, removing the plugin and then the marketplace, so the two commands are inverses. (Breaking: Codex uninstall previously left an installed plugin alone; Claude uninstall now also removes the marketplace.) ([#4019](max-sixty/worktrunk#4019))

- **`wt step copy-ignored` says whether it reflinked or copied**: the summary now reads `(reflinked, no extra disk)`, `(full copy)`, or a partial count, so the same `29.5 GB` line distinguishes a free copy from one that actually wrote the bytes. `--format=json` gains `reflinked` and `written`; a `--dry-run` plan no longer reports `files`/`bytes`. ([#4025](max-sixty/worktrunk#4025))

- **`wt config update` removes keys its destination cannot hold**: migrating `[select]` or `[commit-generation]` drops keys the new section has no field for, reporting each one. Otherwise the key landed at a path the user never typed and warned on every command, with no way to clear it. ([#3994](max-sixty/worktrunk#3994))

### Fixed

- **A failed `wt switch pr:<n>` against a fork no longer deletes a branch it didn't create**: the rollback force-deleted the branch on any setup error, including the one case where the branch wasn't its own — a `pre-switch` hook or a concurrent session claiming the name in the window after the forge answered. Branch and worktree are now created in one `git worktree add -b`, which writes nothing when the name is taken. ([#3984](max-sixty/worktrunk#3984))

- **Codex on Windows: the activity hooks no longer fail on every event**: each hook led with a bare `bash`, which `cmd.exe` resolves to the WSL launcher rather than Git Bash, so every event raised a `Hook failed` banner. Hooks now run `wt.sh` through a shim that finds Git Bash by path. ([#4008](max-sixty/worktrunk#4008), fixes [#4007](max-sixty/worktrunk#4007), thanks @McNultyyy for reporting and diagnosing)

- **Templates leave `branch` unset in a detached worktree**: aliases and hooks rendered `{{ branch }}` as the literal `HEAD`, which git resolves, so an unguarded template ran against the wrong ref — the reported case ended in `git push origin --delete HEAD`, which git refused. `base` and `target` follow. (Breaking: an unguarded `{{ branch }}` now errors.) ([#4010](max-sixty/worktrunk#4010), fixes [#4009](max-sixty/worktrunk#4009))

- **The OpenCode plugin loads under OpenCode 2**: OpenCode 2's loader dropped the old bare-function export silently, so activity markers stopped appearing. The plugin now exports one object carrying both the v2 `setup` and v1 `server` entry points, and spawns `wt` through `node:child_process` rather than the Bun shell. Re-run `wt config plugins opencode install` to pick it up; the one file covers OpenCode 2 and OpenCode 1.16 or later. ([#4018](max-sixty/worktrunk#4018), thanks @pragmaticivan for the request)

- **`wt step commit --branch` builds its prompt from the worktree being committed**: prompt-building git plumbing ran in the invoking worktree, so with nothing staged there the generated message described none of the changes it committed. `wt step relocate --commit` had the same bug, and `--dry-run`/`--show-prompt` now preview the worktree `--branch` names. ([#3996](max-sixty/worktrunk#3996))

- **LLM commit messages keep diffs for quoted paths**: a file whose name git quotes in the `diff --git` header — any non-ASCII name under the default `core.quotePath` — had its whole diff section dropped from the prompt once the diff exceeded the 100 KB budget. ([#4041](max-sixty/worktrunk#4041))

- **An unparsable user config is reported, not a panic**: `wt config show --format=json` and `--full`, plus `wt step prune`, `relocate`, `eval`, and `for-each`, panicked on a debug build and printed a bare `✗ Command failed` on a release one. They now report `✗ Failed to load config` with the parser's caret diagram. ([#4002](max-sixty/worktrunk#4002))

- **`wt switch --execute` names the directory the program actually starts in**: the `Executing (--execute) @ …` header rendered the worktree the background hooks run in, not the program's own directory — which differs under `--no-cd` or a switch from a subdirectory. The path is omitted when the shell already stands there. ([#4043](max-sixty/worktrunk#4043), fixes [#4042](max-sixty/worktrunk#4042), thanks @yajo for reporting)

- **`wt config plugins claude|codex uninstall` is safe to re-run**: removing an already-absent marketplace reported a failure, and a Claude uninstall that removed the plugin and then failed left the marketplace with no way to clear it. Both now finish a half-done uninstall and succeed when there is nothing left to remove. ([#4033](max-sixty/worktrunk#4033), [#4034](max-sixty/worktrunk#4034))

- **`wt config state marker` set and clear no-op outside a git repository**: they exited 1 with a git error, so an agent plugin running outside a repository printed one every turn (the hook's `|| true` kept the session going). ([#3981](max-sixty/worktrunk#3981), thanks @mahirhir)

- **`wt list` produces output in a bare repo with no worktrees**: it printed nothing at all and exited 0. It now reports `○ No worktrees` with a hint, `--branches` lists the branches, and `--format json` emits the envelope with an empty `items`. ([#3992](max-sixty/worktrunk#3992))

- **`wt list` warns when it can't parse the project config** rather than ignoring it silently; the listing still succeeds. ([#3993](max-sixty/worktrunk#3993))

- **Relayed command output no longer prints ahead of the delayed-stream progress line**, and delayed-stream output routes through anstream so color is stripped consistently when redirected. ([#4037](max-sixty/worktrunk#4037), [#3995](max-sixty/worktrunk#3995))

- **A hook filter that matches nothing names the unconfigured source**, rather than reporting no match without saying which source was empty. ([#3997](max-sixty/worktrunk#3997))

### Documentation

- **`wt hook` says that `post-*` hooks from the two sources run concurrently**, so dependent commands belong in one source; `wt merge` records that `post-commit` can't run when the merge removes the worktree it anchors on. ([#4020](max-sixty/worktrunk#4020), [#4026](max-sixty/worktrunk#4026))

- **`wt step` help pages describe what the commands actually do**: `wt step prune` says it removes br

See merge request: Harmonybrew/homebrew-core!18763
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.

2 participants