Repository navigation
Tidy shell-integration output: prompt spacing, restart hint, one install hint - #4059
Conversation
…all hint Four things the shell-config surface got wrong. `prompt_yes_no_preview` printed a blank line before every prompt, but most prompts are the first thing their command prints — `wt config shell install`, `wt config plugins claude install`, the commit-generation offer at the top of `wt merge` — so the blank was the leading blank /writing-user-outputs forbids. The two PTY snapshots covering the install prompt had a `trim_start_matches` compensating for it. The separator now belongs to the narration it separates from: the four callers with output above them emit it themselves. `wt config shell install` told an already-wrapped shell to restart. The hint says integration needs activating, and reinstalling from inside a wrapped shell — a version bump, or the fish conf.d → functions relocation — is exactly when it doesn't. Gated on `is_shell_integration_active()`. `wt config show` printed a per-shell remediation hint for an outdated wrapper, a deprecated fish location, and a missing fish completions file, so three problems printed three commands where the bare `wt config shell install` fixes all of them. Each row now counts toward the section's single trailing hint, and the fish deprecated-location row names its destination inline, the way the install output's cleanup line does. The zsh compinit snippet used `format_with_gutter`, so it rendered plain where the same snippet in the install output was dim and highlighted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BX6oZUpZKn4hT5xABGqUAC
Dropping the prompt's leading blank left the value single-line inside a multi-line inline snapshot, which insta reports as a legacy format on every run of the suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BX6oZUpZKn4hT5xABGqUAC
worktrunk-bot
left a comment
There was a problem hiding this comment.
Two things worth deciding before this merges, plus one finding inline on the hint consolidation.
codecov/patch is red. 19/23 patch lines, all four misses in src/commands/config/show.rs. Three aren't real: 1171 and 1292 are the closing )?; of writeln! calls that config_show_fish_without_completions and config_show_fish_outdated_wrapper do exercise — they only became patch lines because the reformatting from writeln!(out, "{warning}\n{hint}") split them onto their own line, and the identical unchanged )?; at 1123, 1142 and 1162 are misses on the base commit too. 1151 is the completion_path()-fails arm, which no test can trigger. The fourth, 1465, is a real gap, and it's where the inline finding lives. Per CLAUDE.md → Coverage that leaves the merge decision with you; the arithmetic is above.
Data-safety surface. src/commands/configure_shell.rs carries the fs::remove_file calls behind uninstall and the legacy-wrapper cleanups, and this diff moves the blank line out of prompt_yes_no() — the confirmation that gates them. I traced it: prompt_yes_no has one caller, prompt_for_uninstall_confirmation re-emits the blank ahead of it, and the inline snapshot in test_uninstall_preserves_rc_replaced_after_preview shows preview and consent unchanged. Noting it because the repo's review guidance holds this surface for a human regardless of how the change reads.
Consolidating config show's per-shell hints onto the bare `wt config shell install` assumed that command covers every state the section reports. It didn't: with a wrapper at `~/.config/fish/conf.d/wt.fish` and no `~/.config/fish/functions` directory, fish has no config location, so the scan skips it and only `wt config shell install fish` — which sets `allow_create` through its shell filter — migrated the wrapper. The hint named a command that did nothing for the row it sat under. A worktrunk wrapper at the deprecated path is a config location: fish is configured, just at the old path. The scan now says so, which also collapses `should_configure` into `allow_create || has_config_location`, and config show's skipped-shell branch for legacy fish becomes unreachable — the row comes from the configured loop. That branch was also how a legacy wrapper reached the report at all when fish wasn't on PATH; it didn't, because the skipped list is gated on `is_installed`. Such a user now gets the row. Adds the two tests the consolidation left unpinned: the bare install migrating fish from conf.d, and the restart hint staying silent under an active wrapper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BX6oZUpZKn4hT5xABGqUAC
worktrunk-bot
left a comment
There was a problem hiding this comment.
The last review's two findings are addressed: scan_shell_configs now treats the deprecated wrapper as a config location, and test_configure_shell_no_restart_hint_when_integration_active pins the restart-hint suppression. The patch line that review called a real coverage gap — the skipped-shell legacy branch in render_shell_status — is deleted here.
Withholding approval on the data-loss hold rather than on a defect. This commit widens what a bare wt config shell install deletes: cleanup_legacy_fish_conf_d previously ran only when fish was already part of the install, and pulling fish into configured on the existence of ~/.config/fish/conf.d/wt.fish means that file is now removed on any bare install — including for a user with no ~/.config/fish/functions who never named fish. Consent is intact on the interactive path, where legacy_preview names the removal before the prompt; --yes removes it unpreviewed, as it does for the other cleanups. Existence rather than content reads as the right predicate — it is the ownership-by-path rule cleanup_legacy_fish_conf_d's own docstring settles, and a content check would leave the original bug standing for a wrapper the detector doesn't recognize. So this is .claude/skills/running-tend/references/review-pr.md → "Data-Loss Surface: Hold for Human Review" putting the merge call on you, not a change request.
# Conflicts: # CHANGELOG.md
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
Five things the shell-config surface got wrong, all found by running
wt config shell installandwt config showon a machine with five shells configured.The prompt opened with a blank line.
prompt_yes_no_previewprinted one before every prompt, but most prompts are the first thing their command prints —wt config shell install,wt config plugins claude install, the commit-generation offer at the top ofwt merge— so the blank was the leading blank/writing-user-outputsforbids. Two PTY snapshots covering the install prompt had atrim_start_matches('\n')compensating for it, now removed. The separator belongs to the narration it separates from, so the four callers that print first emit it themselves.wt config shell installtold an already-wrapped shell to restart. The hint says integration needs activating, and reinstalling from inside a wrapped shell is exactly when it doesn't: a version bump, or the fishconf.dtofunctionsrelocation, which writes a new file for a wrapper the running shell already has. Gated onis_shell_integration_active().wt config showprinted one remediation command per problem. An outdated wrapper, fish integration at the deprecatedconf.dpath, and a missing fish completions file each carried their ownwt config shell install <shell>, so three problems printed three commands where the barewt config shell installfixes all of them. Each row now counts toward the section's single trailing hint, and the fish deprecated-location row names its destination inline the way the install output's cleanup line does.To verify wrapper loaded: type wtstays separate, since installing doesn't fix it.A deprecated fish wrapper wasn't a config location. Consolidating onto the bare command assumed it covers every state the section reports, and it didn't: with a wrapper at
~/.config/fish/conf.d/wt.fishand no~/.config/fish/functionsdirectory, fish has no config location, so the scan skipped it and onlywt config shell install fish— which setsallow_createthrough its shell filter — migrated the wrapper. A worktrunk wrapper at the deprecated path now counts as fish being configured, just at the old path. That collapsesshould_configureintoallow_create || has_config_location, makes config show's skipped-shell branch for legacy fish unreachable, and gets the wrapper into the report for a user who doesn't have fish onPATH— where the skipped list, gated onis_installed, showed nothing for fish at all.The zsh
compinitsnippet rendered plain. It went throughformat_with_gutterwhere the same snippet in the install output goes throughformat_bash_with_gutter, so one was dim and highlighted and the other wasn't.