Replace config update --print with --output - #4021
Conversation
worktrunk-bot
left a comment
There was a problem hiding this comment.
--output <path> writes a zero-byte file when there is nothing to migrate, which can empty a file the user cares about — including their own config. Inline suggestion below; the rest are notes, not blockers.
Reproduced against this branch's build (clean user config: worktree-path = "../{{ repo }}.{{ branch }}" plus [list] json-schema = 1):
$ printf 'IMPORTANT USER DATA\n' > dest.toml
$ wt config update --output dest.toml && echo "exit=$?"
exit=0
$ wc -c dest.toml
0 dest.toml
$ wt config update --output "$WORKTRUNK_CONFIG_PATH" # 69 bytes before
$ wc -c "$WORKTRUNK_CONFIG_PATH"
0 ...user-config.tomlhandle_config_update now branches on output before the candidates.is_empty() check, so a clean config reaches write_migrated_output with an empty artifact and write_atomically replaces the destination with it — silently, exit 0, nothing on stdout or stderr. The --print code this replaces had the guard: if print { if candidates.is_empty() { return Ok(()); } … }. The stdout arm still degrades safely by accident (an empty string prints as nothing, and the comment on that arm says so); only the file arm lost it. wt config update --output config.toml.new && mv config.toml.new config.toml is the shape that bites, and it's the workflow the flag's own help text invites. This is what CLAUDE.md's Data Safety section calls out — "Prefer failure over silent loss" and "No implicit destructive side effects" — so I'm holding the approval on it rather than treating it as a nit.
Two smaller things, both judgment calls rather than defects:
Output mode drops approved-commands without preserving them. The apply path copies them to approvals.toml first, with a comment saying rewriting config first "would silently lose the legacy approvals". Output mode skips that copy, and I confirmed the artifact has the approved-commands array removed with no approvals.toml written. That was already true of --print, but --print read as "show me the migration" while --output <path> reads as "produce the file I'll put in place" — so the write-then-move user now loses their approvals. A line in the long help, or a stderr note on the file destination, would cover it.
--print is removed with no bridge. CLAUDE.md lists CLI flags as a protected interface and says to "prefer deprecation warnings over silent breaks"; a script hitting this gets clap's unexpected argument '--print' found, which doesn't name --output=-. Keeping --print as a hidden arg that maps to --output=- and warns would carry scripts across one release. Your call — the CHANGELOG does flag the break, and -x in the same Unreleased section sets the precedent for taking one.
Also, #4015 (bot-authored) is the linked-worktree half of this change on its own; this PR subsumes it, so it can be closed.
Test-wise, the file destination is covered only with candidates present (test_config_update_output_destinations_emit_same_artifact) and the clean-config case only on stdout — the clean-config-plus-file branch, where the bug lives, has no test.
worktrunk-bot
left a comment
There was a problem hiding this comment.
--output <path> produces an artifact that isn't a config file when user and project config both migrate: the two documents are concatenated, so TOML table scope leaks from the first into the second and the project config's top-level keys land inside the user config's last table.
Reproduced against this branch's build — user config with a [list] section plus a deprecated template var, project config with a deprecated pre-start:
$ wt config update --output=-
# User config (/tmp/reprowt/user-config.toml)
worktree-path = "../{{ repo }}.{{ branch }}"
[list]
json-schema = 1
# Project config (/tmp/reprowt/myrepo/.config/wt.toml)
pre-start = "ln -sf {{ repo }}/node_modules ."
$ wt config update --output migrated.toml
$ cp migrated.toml user2.toml
$ WORKTRUNK_CONFIG_PATH=user2.toml wt config show
▲ User config has unknown field list.pre-start (will be ignored)format_migrated_output pushes each candidate's migrated string end to end with a # <label> (<path>) line between them, and a comment doesn't close [list] — so pre-start reparses as list.pre-start and is dropped. Exit 0, nothing on stderr.
Same shape as the approved-commands caveat you just documented, one step worse: that one drops a key the user can see is missing, this one silently reparents it. --print concatenated too, but it read as "show me the migration"; --output <path> "instead of applying it in place" is what invites the write-then-move workflow. And this PR newly makes the two-candidate case reachable from a linked worktree — read_only now passes actionable = true, where check_project_config previously returned None there — so the artifact that can't be applied anywhere is exactly the one a linked worktree now produces.
Cheapest fix is a sentence beside the approved-commands note; inline suggestion below. A stderr note on the file destination when candidates.len() > 1 would also reach the user who never opens --help — your call which is proportionate. Applying the suggestion changes generated help, so cargo run -- hook pre-merge --yes regenerates the doc mirrors and the --help snapshot.
worktrunk-bot
left a comment
There was a problem hiding this comment.
Two notes on the new rejection, neither a blocker.
File output is unreachable whenever the project config also has deprecations. Verified against this branch's build — user config with a deprecated template var, repo .config/wt.toml with a deprecated pre-start:
$ wt config update --output migrated.toml
✗ Cannot write multiple migrated configs to one file; use --output=- to inspect them or run wt config update to apply them in place
$ echo $?
1Both remedies the message offers miss what was asked for: --output=- hands back the combined inspection artifact, and wt config update applies in place, which is the thing --output <path> exists to avoid. So a user whose own config has a deprecation can't get it to a file at all from inside a repo whose committed project config also migrates — and that project config is usually not theirs to fix. The only escape I found is wt -C <dir outside any repo> config update --output <abs path>; it works, but nobody will find it.
Not arguing for a scoping flag — that's a product call. But the message naming the two configs would at least tell the user that the repo's .config/wt.toml is what blocked them, and candidates.iter().map(|c| c.info.label()) already has the labels the stdout artifact prints.
The message's commands aren't bolded. Inline suggestion below. .claude/skills/writing-user-outputs/SKILL.md says "Use <bold> for commands in warnings/errors (only hints use <underline>)", and the nearby bails follow it — src/commands/config/state.rs has bail!(cformat!("No trace records in {source}; run <bold>-vv</> to capture a trace")). The suggested text leaves the wording alone, so the contains(...) assertion in test_config_update_output_file_rejects_multiple_configs still matches.
wt config updatenow uses a destination-based output interface:--output <path>atomically writes the migration artifact, while--output=-emits the same bytes to stdout. This replaces--print, keeps output mode read-only, and includes project-config migrations when invoked from a linked worktree.Relative destinations honor the global
-Cdirectory. When there is nothing to migrate, stdout stays silent and an existing file destination remains untouched. If both user and project configs need migration, stdout emits a labeled inspection artifact while file output fails before writing. Help text also notes that output artifacts omit legacyapproved-commands; the in-place mode remains responsible for moving those entries toapprovals.toml.Integration coverage exercises file replacement, preservation and write failures, multi-config rejection, stdout piping and broken consumers, clean configs, linked worktrees, source-file preservation, help text, and rejection of the removed flag. The full pre-merge gate passed all 4,770 tests, targeted instrumentation covers both file-error branches, and the production docs build passed all 16 built-site and browser tests.