config: write the path the user named, and check every config write the same way

CHANGELOG.md

@@ -10,14 +10,14 @@
 
 - **Messages name the destination, and end with the command to run**: without shell integration, `wt merge` and `wt remove` printed `Cannot change directory — shell integration not installed` and no path; the warning now names the worktree, as `wt switch` already did. "Cannot determine default branch" had three wordings, and three recovery hints put their command first rather than last. ([#4163](https://github.com/max-sixty/worktrunk/pull/4163), [#4155](https://github.com/max-sixty/worktrunk/pull/4155), thanks @blrain3 for reporting)
 
+- **`wt config update --output <path>` writes the path it names**: a destination already holding a file is replaced, as `cp` and a shell redirect do, and so is the config being migrated, which used to be refused when the migration dropped `approved-commands`. The warning above the write still says those approvals reach approvals.toml only via `wt config update`. ([#4153](https://github.com/max-sixty/worktrunk/pull/4153), [#4204](https://github.com/max-sixty/worktrunk/pull/4204))
+
 - **`/wt-switch-create` no longer stalls at Claude Code's path-entry dialog**: Claude Code confirms every entry outside `.claude/worktrees/`, which is every Worktrunk worktree, so a background session waited for someone to attach. The plugin now approves entry into a worktree at its `worktree-path` location, and reads a bare name as the repo or the branch. ([#4158](https://github.com/max-sixty/worktrunk/pull/4158), [#4159](https://github.com/max-sixty/worktrunk/pull/4159))
 
 ### Fixed
 
 - **Diff display config no longer changes what Worktrunk reads**: `color.ui = always` and `diff.external` reached the commit, squash, and summary prompts. Two settings could also hide a branch's changes from the integration check, so `wt remove` deleted the branch without `-D`: `diff.relative` when run from a subdirectory the changes sat outside, and `submodule.<name>.ignore` on a submodule bump. Every diff Worktrunk parses now runs plumbing, which ignores that configuration, and upgrading discards the cached answers. ([#4146](https://github.com/max-sixty/worktrunk/pull/4146), [#4147](https://github.com/max-sixty/worktrunk/pull/4147), [#4181](https://github.com/max-sixty/worktrunk/pull/4181))
 
-- **`wt config update --output <path>` asks before overwriting**: it replaced whatever was there, so `--output ~/.config/worktrunk/config.toml` from a repo with a deprecated project config wiped the user config and reported success. It now prompts, takes `--yes`, and refuses the config being migrated. (Breaking: `--output` naming the config being migrated used to succeed unless the migration dropped `approved-commands`.) ([#4153](https://github.com/max-sixty/worktrunk/pull/4153))
-
 - **`wt merge` on a dirty worktree describes what it commits**: the generated message covered the branch's earlier commits and said nothing about the work the merge had just staged into the same commit. `wt step squash --dry-run` now previews what the run would commit, as `wt step commit --dry-run` does. ([#4178](https://github.com/max-sixty/worktrunk/pull/4178))
 
 - **A config save no longer rewrites the rest of your config**: declining a prompt, or setting the commit-generation command, rewrote the whole file, dropping values written at their default and re-spelling hook pipelines and dropping their comments. It also reloaded that file over the in-memory config, so `wt step commit --config-set 'commit.stage="none"'` staged everything once it had asked about commit generation. ([#4160](https://github.com/max-sixty/worktrunk/pull/4160), [#4156](https://github.com/max-sixty/worktrunk/pull/4156), [#4151](https://github.com/max-sixty/worktrunk/pull/4151))

CLAUDE.md

@@ -64,6 +64,7 @@ Never risk data loss without explicit user consent. A failed command that preser
 - **Favor the failing variant on races** — `git reset --keep` (fails if tracked files were modified) over `--hard`; `git checkout --merge` over `--force`. If no safer variant exists, document the risk inline.
 - **Time-of-check vs time-of-use** — be conservative when there's a gap between the safety check and the operation. `wt merge` verifies clean before rebasing, but files could appear before cleanup — don't force-remove during cleanup.
 - **Replace full files, never truncate them** — `fs::write` truncates before it writes, so a crash mid-write leaves the file empty. Every full-file rewrite worktrunk can't put back (shell wrappers, `config.toml`, `approvals.toml`, another tool's `settings.json`) goes through `utils::write_atomically`, which renames a sibling temp file over the target; the spec on that function covers symlinks, mode, and what a rename costs. When a user-owned file was observed missing, `utils::write_new_atomically` refuses to overwrite one that appears before persistence. Adding to an existing Bash, Zsh, or PowerShell rc file instead opens it in append mode, so a stale snapshot cannot replace concurrent edits; never truncate the file to recover from an append error. Don't grow the install path into a rebuild. Removal still rewrites those rc files whole (`uninstall_previewed_lines`), which is why the `write_atomically` spec still names them. Regenerable content (the cache, the `-vv` diagnostic report) keeps the plain write.
+- **A path the user named is written, not second-guessed** — `wt config update --output <path>` replaces whatever is there, the way `cp` and a shell redirect do, and pointed at the config being migrated it writes the migration there too. No-clobber belongs to the other case: a path worktrunk chose itself and observed absent (a shell rc file, `wt config create`), where a file appearing before the rename is one wt never read and a dangling symlink is a dotfile manager's link to keep. Deciding between the two at a call site is the smell — each write site is in one case or the other, so it calls one function unconditionally.
 - **Shell-config concurrency has a deliberate boundary** — the append lock coordinates Worktrunk installers, and no-clobber creation turns the missing-file race into a fail-and-rerun outcome. An editor save racing rc-file uninstall can still be overwritten; accept this final check-to-rename window because sidecar locks, backups, retries, and extra pre-rename checks do not close it. Revisit after an observed incident or a simpler write design. Worktrunk-owned wrappers and completions use last-writer-wins semantics, as does the merge into another tool's `settings.json`.
 
 These stop where git's own protections stop, and matching git is deliberate in each case. The named spec says why:

src/commands/config/create.rs

@@ -124,9 +124,11 @@ fn create_config_file(
         std::fs::create_dir_all(parent).context("Failed to create config directory")?;
     }
 
-    // Write the example config with all values commented out
+    // Write the example config with all values commented out. The path was
+    // observed absent above, so a file that appears in the window is one wt
+    // never read — including a dangling symlink a dotfile manager owns.
     let commented_config = comment_out_config(content);
-    worktrunk::utils::write_atomically(&path, &commented_config)
+    worktrunk::utils::write_new_atomically(&path, &commented_config)
         .context("Failed to write config file")?;
 
     // Success message

src/commands/config/update.rs

@@ -6,7 +6,6 @@
 //! migration to disk.
 
 use std::fmt::Write as _;
-use std::io::IsTerminal;
 use std::path::{Path, PathBuf};
 
 use anyhow::{Context, bail};
@@ -17,12 +16,12 @@ use worktrunk::config::{
     format_migration_diff_block,
 };
 use worktrunk::git::{Repository, resolve_input_path};
-use worktrunk::path::{format_path_for_display, paths_match};
+use worktrunk::path::format_path_for_display;
 use worktrunk::styling::{
     eprint, eprintln, format_bash_with_gutter, hint_message, info_message, print, success_message,
     suggest_command_in_dir, warning_message,
 };
-use worktrunk::utils::{write_atomically, write_new_atomically};
+use worktrunk::utils::write_atomically;
 
 use crate::output::prompt::{PromptResponse, prompt_yes_no_preview};
 
@@ -70,7 +69,7 @@ pub fn handle_config_update(yes: bool, output: Option<PathBuf>) -> anyhow::Resul
     }
 
     if let Some(output) = output {
-        write_migrated_output(&output, &candidates, yes)?;
+        write_migrated_output(&output, &candidates)?;
         return Ok(());
     }
 
@@ -157,23 +156,14 @@ pub fn handle_config_update(yes: bool, output: Option<PathBuf>) -> anyhow::Resul
 ///
 /// The two destinations differ in what they can carry, so they run as separate
 /// paths rather than one path testing `-` at each step. Stdout labels and
-/// concatenates every candidate and needs no confirmation — the artifact is
-/// right there. A file takes exactly one migration, so the checks below and
-/// the success line can name the config it came from.
+/// concatenates every candidate. A file takes exactly one migration, so the
+/// check below and the success line can name the config it came from.
 ///
-/// A file destination replaces nothing without consent. The config being
-/// migrated is refused outright: rewriting it is the in-place update's job,
-/// which previews the diff, re-reads the file after the prompt, and moves
-/// `approved-commands` to approvals.toml. Any other existing file is replaced
-/// only after a prompt, which `--yes` answers in advance; with no terminal to
-/// prompt on, the command fails instead. A destination that was absent is
-/// created without clobbering, so a file that appears before the write lands
-/// survives it.
-fn write_migrated_output(
-    output: &Path,
-    candidates: &[UpdateCandidate],
-    yes: bool,
-) -> anyhow::Result<()> {
+/// The named path is written, replacing whatever is there, the way `cp` and a
+/// shell redirect do. Pointed at the config being migrated it still writes only
+/// the migration: moving `approved-commands` to approvals.toml belongs to the
+/// in-place update, which is what the warning above the write says.
+fn write_migrated_output(output: &Path, candidates: &[UpdateCandidate]) -> anyhow::Result<()> {
     if output == Path::new("-") {
         for candidate in candidates {
             eprint!("{}", format_dropped_approvals_warning(candidate));
@@ -195,43 +185,12 @@ fn write_migrated_output(
 
     let output = resolve_input_path(output);
     let label = candidate.info.label().to_lowercase();
-    if paths_match(&output, &candidate.config_path) {
-        bail!(cformat!(
-            "Cannot overwrite <bold>{label}</> with <bold>--output</>; to apply the migration in place, run <bold>wt config update</>"
-        ));
-    }
     let display_path = format_path_for_display(&output);
 
-    let approvals_warning = format_dropped_approvals_warning(candidate);
-    eprint!("{approvals_warning}");
-
-    let replace = output.exists();
-    if replace && !yes {
-        if !std::io::stdin().is_terminal() {
-            bail!(cformat!(
-                "{display_path} already exists; to overwrite it with the {label} migration, add <bold>--yes</>"
-            ));
-        }
-        if !approvals_warning.is_empty() {
-            eprintln!();
-        }
-        let prompt = format!("Overwrite {display_path} with the {label} migration?");
-        match prompt_yes_no_preview(&prompt, || {})? {
-            PromptResponse::Accepted => {}
-            PromptResponse::Declined => {
-                eprintln!("{}", info_message("Update cancelled"));
-                return Ok(());
-            }
-        }
-    }
+    eprint!("{}", format_dropped_approvals_warning(candidate));
 
-    let artifact = format_migrated_output(candidates);
-    let written = if replace {
-        write_atomically(&output, &artifact)
-    } else {
-        write_new_atomically(&output, &artifact)
-    };
-    written.with_context(|| format!("Failed to write output @ {display_path}"))?;
+    write_atomically(&output, &format_migrated_output(candidates))
+        .with_context(|| format!("Failed to write output @ {display_path}"))?;
     eprintln!(
         "{}",
         success_message(format!("Wrote {label} migration @ {display_path}"))

src/config/mod.rs

@@ -185,10 +185,12 @@ pub fn is_user_project_override_key(key: &str) -> bool {
 ///
 /// wt can't load such a file: every later command skips user config with a
 /// warning, and the commands that need project config fail, until the user
-/// hand-edits it. Both writers of a config file the user owns check the
-/// content they are about to write: the `UserConfig` mutations and
-/// `wt config update`. Neither starts from invalid TOML, so this fires only when
-/// the edit itself broke the syntax, and the file on disk stays as it was.
+/// hand-edits it. `wt config update` runs this over the migration it computed,
+/// before either destination is written. A `UserConfig` mutation faces a
+/// stronger test of its own — that the file it is about to write loads back as
+/// the config the mutation asked for — so it needs no second, weaker one.
+/// Neither starts from invalid TOML, so this fires only when the migration
+/// itself broke the syntax, and the file on disk stays as it was.
 pub fn ensure_config_parses(content: &str) -> Result<(), ConfigError> {
     content.parse::<toml::Table>().map(|_| ()).map_err(|e| {
         ConfigError(format!(

src/config/user/mutation.rs

@@ -72,7 +72,6 @@ impl UserConfig {
             return Ok(());
         };
         let edited = file.edited(&edit, &changed)?;
-        crate::config::ensure_config_parses(edited.content())?;
         crate::utils::write_atomically(config_path, edited.content()).map_err(|e| {
             ConfigError(format!(
                 "Failed to write config file {}: {}",

src/config/user/persistence.rs

@@ -31,6 +31,16 @@ pub(super) struct ConfigEdit<'a> {
 }
 
 impl ConfigEdit<'_> {
+    /// The dotted key this edit writes, for naming it in an error.
+    fn key_path(&self) -> String {
+        self.tables
+            .iter()
+            .chain(std::iter::once(&self.key))
+            .copied()
+            .collect::<Vec<_>>()
+            .join(".")
+    }
+
     /// Set the value in `doc`, leaving everything else as it is.
     ///
     /// An existing value is replaced in place and keeps its decor — the spacing
@@ -118,35 +128,50 @@ impl ConfigFile {
     /// The edit goes into the file as written when that loads as `expected`.
     /// Otherwise a load-time migration touches the edit's path — a deprecated
     /// `[commit-generation]` migrates to `[commit.generation]` only while that
-    /// table is absent — and the edit goes into the migrated file, which loads
-    /// as `expected` by construction: the migrations are idempotent and the
-    /// edit lands after them. That file carries *every* load-path migration,
-    /// not just the one on the edit's path, so it can also move an unrelated
-    /// deprecated section and drop the keys its destination has no field for.
-    /// A mutation is otherwise not what materializes migrations —
-    /// `wt config update` is — so [`Edited::Migrated`] says so, and its caller
-    /// tells the user.
+    /// table is absent — and the edit goes into the migrated file, which carries
+    /// *every* load-path migration, not just the one on the edit's path, so it
+    /// can also move an unrelated deprecated section and drop the keys its
+    /// destination has no field for. A mutation is otherwise not what
+    /// materializes migrations — `wt config update` is — so [`Edited::Migrated`]
+    /// says so, and its caller tells the user.
+    ///
+    /// Both candidates face the same test, [`loads_as`], and no content leaves
+    /// here untested: the migrated file is expected to pass by construction (the
+    /// migrations are idempotent and the edit lands after them), and an error
+    /// rather than a silent write is what says so.
     pub(super) fn edited(
         &self,
         edit: &ConfigEdit,
         expected: &UserConfig,
     ) -> Result<Edited, ConfigError> {
         let mut doc = self.doc.clone();
         edit.apply(&mut doc)?;
-        if load(&doc).is_ok_and(|config| &config == expected) {
+        if loads_as(&doc, expected) {
             return Ok(Edited::AsWritten(doc.to_string()));
         }
 
         let mut doc = self.doc.clone();
         let changes = migrate_doc(&mut doc);
         edit.apply(&mut doc)?;
+        if !loads_as(&doc, expected) {
+            return Err(ConfigError(format!(
+                "Refusing to write a config file wt could not read back: {} would not load as written",
+                edit.key_path()
+            )));
+        }
         Ok(Edited::Migrated {
             content: doc.to_string(),
             changes,
         })
     }
 }
 
+/// Whether `doc` loads as `expected` — the one test a mutation's content has to
+/// pass before it is written.
+fn loads_as(doc: &DocumentMut, expected: &UserConfig) -> bool {
+    load(doc).is_ok_and(|config| &config == expected)
+}
+
 /// A config file with an edit applied, and whether writing it took the load-path
 /// migrations with it.
 pub(super) enum Edited {

src/config/user/tests.rs

@@ -3083,9 +3083,10 @@ fn test_with_locked_mutation_propagates_write_error() {
     let dir_for_closure = config_dir.clone();
     let mut config = UserConfig::default();
     let err = config
-        .with_locked_mutation(&config_path, move |_config| {
+        .with_locked_mutation(&config_path, move |config| {
             std::fs::set_permissions(&dir_for_closure, std::fs::Permissions::from_mode(0o555))
                 .unwrap();
+            config.skip_shell_integration_prompt = true;
             Some(super::persistence::ConfigEdit {
                 tables: vec![],
                 key: "skip-shell-integration-prompt",
@@ -3101,6 +3102,35 @@ fn test_with_locked_mutation_propagates_write_error() {
     assert_eq!(std::fs::read_to_string(&config_path).unwrap(), "# valid\n");
 }
 
+/// A mutator sets a value and returns the edit that writes it, so the file
+/// loads back as the config the mutation asked for. One whose edit says
+/// something else is refused rather than written — the same test both candidate
+/// documents face, whether or not writing them takes the migrations along.
+#[test]
+fn test_with_locked_mutation_refuses_an_edit_that_does_not_load_back() {
+    let dir = tempfile::tempdir().unwrap();
+    let config_path = dir.path().join("config.toml");
+    std::fs::write(&config_path, "# valid\n").unwrap();
+
+    let mut config = UserConfig::default();
+    let err = config
+        .with_locked_mutation(&config_path, |_config| {
+            // The edit, without the matching change to the config beside it.
+            Some(super::persistence::ConfigEdit {
+                tables: vec![],
+                key: "skip-shell-integration-prompt",
+                value: true.into(),
+            })
+        })
+        .unwrap_err();
+    let msg = err.to_string();
+    assert!(
+        msg.contains("skip-shell-integration-prompt would not load as written"),
+        "the refusal should name the key, got: {msg}"
+    );
+    assert_eq!(std::fs::read_to_string(&config_path).unwrap(), "# valid\n");
+}
+
 #[test]
 fn test_pattern_project_key_applies_to_every_repo_on_a_host() {
     let config = UserConfig::load_from_str(