Skip to content

fix(config): migrate deprecated keys in env vars and inline tables - #3158

Merged
max-sixty merged 1 commit into
mainfrom
config-set-deprecation-migration
Jun 22, 2026
Merged

fix(config): migrate deprecated keys in env vars and inline tables#3158
max-sixty merged 1 commit into
mainfrom
config-set-deprecation-migration

Conversation

@max-sixty

Copy link
Copy Markdown
Owner

The gap

#3152 routed the --config-set layer through the deprecation-migration layer, but a deprecated config key could still slip through unmigrated in two other places. Both are now closed with the same mechanism (migrate_content).

Env-var layer

migrate_env_overlay() runs the assembled WORKTRUNK_* overlay through the same migrate_content() config files and --config-set use, before the deep-merge. So WORKTRUNK__MERGE__NO_FF=true resolves to the deprecated key merge.no-ff and now takes effect as merge.ff=false, instead of falling through as an unknown field and being silently ignored. Migrating before the merge (not after) keeps the env value winning over a lower layer's canonical key, matching the layer precedence.

One behavior edge, now documented on the function and pinned by a test: canonicalization surfaces a type mismatch the deprecated name hid. WORKTRUNK__COMMIT_GENERATION__COMMAND=42 migrates to commit.generation.command = 42, which fails to deserialize (the field is a String), so the whole env layer is dropped with an attributed LoadError::Env warning and file config is preserved — the same contract a type-mismatched value in a canonical env var already follows. Previously the unknown key was silently ignored and other env vars survived.

Inline-table form

migrate_negated_bool_doc resolved its section with as_table_mut(), which returns None for an inline merge = { no-ff = true }, so the inline form was never migrated (and surfaced as a double unknown-field warning instead of a deprecation warning). It now branches on standard vs inline table, fixing config files, --config-set, and env at once — the inline shape is preserved (merge = { ff = false }).

remove_switch_picker_timeout_in had the same as_table_mut()-on-switch limitation, so it's given matching inline-switch handling and now every switch sub-key rule is inline-aware. With that, the DEPRECATION_RULES ordering comment is corrected: the [select][switch.picker] rule's inline-to-standard switch conversion is no longer relied on by any rule; only its timeout-ms move remains load-bearing.

Tests

  • Unit: test_env_overlay_migrates_deprecated_key (full EnvVar → resolve → migrate path), test_migrate_negated_bool_inline_table.
  • Integration: test_switch_env_var_migrates_deprecated_no_cd (cd-directive bug-catcher — reverting the fix re-adds the "Cannot change directory" line), test_list_config_env_deprecated_type_mismatch_drops_layer (the documented whole-layer-drop edge).
  • Inline cases added to the test_warning_fires_iff_update_changes battery (negated-bool inline, inline-switch timeout-ms).

This was written by Claude Code on behalf of max

Extends the `--config-set` deprecation migration (#3152) to the two remaining
places a deprecated config key could slip through unmigrated.

Env-var layer: `migrate_env_overlay()` runs the assembled `WORKTRUNK_*` overlay
through the same `migrate_content()` config files and `--config-set` use, before
the deep-merge, so `WORKTRUNK__MERGE__NO_FF=true` takes effect as `merge.ff`
instead of falling through as an unknown field. Migrating before the merge keeps
env winning over a lower layer's canonical key. A side effect, now documented
and tested: a deprecated env var whose canonical target is a non-bool field, set
to a type-mismatched value (`WORKTRUNK__COMMIT_GENERATION__COMMAND=42`), joins
the existing whole-env-layer `LoadError::Env` drop instead of being silently
ignored — the same contract a bad value in a canonical env var already follows.

Inline-table form: `migrate_negated_bool_doc` resolved its section via
`as_table_mut()`, which is `None` for an inline `merge = { no-ff = true }`, so
the inline form was never migrated (and surfaced as a double unknown-field
warning). It now handles both standard and inline tables, fixing config files,
`--config-set`, and env at once. `remove_switch_picker_timeout_in` gets the same
inline-`switch` handling so every `switch` sub-key rule is consistent, and the
`DEPRECATION_RULES` ordering comment is corrected: the `[select]` rule's
inline-to-standard conversion is no longer relied on, only its `timeout-ms` move.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@worktrunk-bot

Copy link
Copy Markdown
Collaborator

Approved. One advisory check went red — affected tests (macos, advisory) — but it's an unrelated flake, so the approval stands.

The failure is a 180s timeout on integration_tests::shell_wrapper::unix_tests::test_source_flag_forwards_errors::case_1, a shell-wrapper PTY test. It's unrelated to this PR (pure TOML deprecation-migration logic, platform-independent): the same advisory job passed on linux and windows, the required test (macos) passed, and the cause is a non-deterministic timeout rather than a build/assertion error. A rerun of that leg should clear it.

The required test (windows) was still in progress when I stopped polling — unverified here, but required CI on linux/macos plus codecov/patch are green.

@max-sixty
max-sixty merged commit 333d630 into main Jun 22, 2026
36 of 37 checks passed
@max-sixty
max-sixty deleted the config-set-deprecation-migration branch June 22, 2026 04:52
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