Skip already-current CLIs when Settings installs agent hooks - #650
Skip already-current CLIs when Settings installs agent hooks#650Yuandi (DDKinger) wants to merge 6 commits into
Conversation
The Settings "Install hooks" button ran `wta hooks install`, which re-ran every supported CLI's install commands regardless of what was already on disk. On a machine that was already fully installed that is two Node spawns per CLI to rewrite plugin directories byte-for-byte — and Windows denies that rewrite while a running agent CLI holds one of those directories open, so the button reported a hard failure for work that never needed doing. Add `wta hooks install --only-missing` and have the button pass it. A CLI is skipped only when `is_up_to_date` can prove re-installing would change nothing: on PATH, marketplace registered and path-valid, plugin installed and enabled, no filesystem-fallback detection, and an installed version at least as new as the bundled one. Every looser state — partial, disabled, stale path, unknown version, older build — is still repaired, and a bare `wta hooks install` remains the full reinstall escape hatch. When the flag skips every CLI, the pre-pass status report is reused as the post-install verification instead of paying a second round of per-CLI queries. Verified on a fully-installed machine: the old path took 15.9s and reported failures for copilot and codex; `--only-missing` takes 7.4s and reports success. After removing opencode's hooks it reinstalls opencode alone and leaves the other four untouched. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8e932b4c-7859-42f8-a576-90d5febb343a
There was a problem hiding this comment.
Pull request overview
This PR improves the “Settings > AI Agents > Install hooks” flow by adding an opt-in --only-missing mode to wta hooks install, allowing already-current agent CLIs to be skipped. This reduces redundant Node-driven reinstalls that can fail on Windows due to locked plugin directories, while preserving the existing “full reinstall” behavior when the flag is not used.
Changes:
- Added
wta hooks install --only-missingand threaded the flag through CLI parsing and dispatch. - Implemented per-CLI selection logic based on a strict “up-to-date” predicate, plus caching of the pre-pass status report when no installs are needed.
- Updated TerminalSettingsEditor to invoke
wta hooks install --only-missingfrom the Settings UI, and added unit tests covering the new behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tools/wta/src/cli/mod.rs | Wires the new only_missing flag into the hooks install command dispatch. |
| tools/wta/src/cli/hooks.rs | Implements --only-missing selection + pre-status reuse; adds tests for selection behavior. |
| tools/wta/src/cli/args.rs | Adds the --only-missing flag to clap args and updates help text. |
| tools/wta/src/cli_tests.rs | Adds parsing tests ensuring --only-missing remains opt-in. |
| tools/wta/src/agent_hooks_installer.rs | Adds ensure_installed_for(&[CliKind]) and introduces is_up_to_date predicate used by selection. |
| tools/wta/src/agent_hooks_installer_tests.rs | Adds unit tests for is_up_to_date strictness and edge cases. |
| src/cascadia/TerminalSettingsEditor/AIAgentsViewModel.idl | Updates the Settings UI documentation comment to reflect the new install behavior. |
| src/cascadia/TerminalSettingsEditor/AIAgentsViewModel.cpp | Updates the Settings “Install hooks” button to run wta hooks install --only-missing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| let missing: Vec<&str> = report | ||
| .clis | ||
| .iter() |
The previous commit skipped CLIs that were already complete and current,
but still routed a complete-but-out-of-date bridge to the install flow.
That does not upgrade anything: every supported CLI answers a second
`plugin install` with "already installed" and no-ops, so the button
burned two Node spawns and then reported a success that never happened.
Replace the boolean skip check with a three-way `InstallAction`:
* incomplete in any way (not on PATH, marketplace missing or pointing
at a pruned path, plugin missing or disabled, or an fs-heuristic
verdict) -> Install
* complete but behind the bundle -> Upgrade, which runs the per-CLI
upgrade flow (`plugin update` / `extensions update` / a Codex
reinstall) -- the only commands that actually move the version
* complete and not provably behind -> Skip
An unreadable version on either side lands in Skip: it isn't proof of
staleness, install would no-op against a complete bridge anyway, and
`upgrade_installed_hooks` re-checks it at master startup with a richer
probe than CliStatus carries.
`upgrade_one_cli` now returns Result<(), String> so a failed upgrade
reaches the per-CLI JSON report instead of being swallowed;
`upgrade_installed_hooks` keeps using just the pass/fail bit.
Verified live. Baseline (all five complete and current): every CLI plans
Skip, exit 0. With the copilot bundle bumped to 0.1.7 the plan becomes
Upgrade and wta runs `copilot plugin update wt-agent-hooks@wt-local` --
not `plugin install` -- and surfaces its real failure. With the opencode
bundle bumped, the plan is Upgrade -> OpenCodeCopy and the installed
version moves 0.1.6 -> 0.1.7, exit 0.
cargo test: 1620 passed, 0 failed. TerminalSettingsEditor: 0 errors.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8e932b4c-7859-42f8-a576-90d5febb343a
This comment has been minimized.
This comment has been minimized.
`App::request_install_hooks` had no callers and was already carrying an `#[allow(dead_code)]`. It existed for an older design in which the Settings "Install hooks" button signalled the helper over a channel and the helper ran the installer in-process; the button now shells out to `wta.exe hooks install --only-missing` and never touches the helper. Removing the channel leaves `ensure_installed`, `ensure_installed_scoped` and `ensure_installed_for` with no callers either -- every remaining entry point goes through `apply_install_plan`, which is the only one that can also route an out-of-date bridge to the upgrade flow. Drop all three rather than leave a second, weaker install path around to be picked up by mistake. Two docs referenced the removed symbol. The wt-agent-hooks README also claimed wta auto-installs hooks on startup, which has not been true since installation became explicit; both are corrected here. No behavior change: `wta hooks install` still resolves to the same per-CLI install commands, and 1620 tests pass unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8e932b4c-7859-42f8-a576-90d5febb343a
This comment has been minimized.
This comment has been minimized.
`agent_check::ensure_installed` (find_exe -> winget install -> refresh_path -> find_exe) has no callers. The two live paths call the pieces directly: `app_events.rs` gates on `AgentStatus::can_auto_install` and then calls `agent_check::install`, and `install_copilot` already calls `refresh_path` itself on success, so the composite adds nothing. `find_exe`, `install` and `refresh_path` all keep other callers and stay. The module doc comment listed the removed composite; it is updated too. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8e932b4c-7859-42f8-a576-90d5febb343a
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
tools/wta/wt-agent-hooks/README.md:49
agent_hooks_installer::apply_install_plannow takes a plan argument; documenting it asapply_install_plan()looks like a zero-arg call and is misleading for readers trying to follow the code path.
Installation is always explicit — the Settings "Install hooks" button, the
first-run setup flow, or `wta hooks install`. Nothing installs hooks on an
ordinary `wta` startup. Each entry point ends up in
`agent_hooks_installer::apply_install_plan()`, which dispatches per CLI:
doc/security-model.md:319
- This diagram names
agent_hooks_installer::apply_install_plan()as a zero-argument call, but the function now takes a plan parameter. Usingapply_install_plan(...)keeps the diagram accurate without over-specifying call-site details.
-> agent_hooks_installer::apply_install_plan()
`ensure_installed_in` is only reachable from `install_dispatches_codex`, so the non-test build reported it as dead code. Gate it on `cfg(test)` and fix the two doc comments that still pointed at the removed `ensure_installed` entry point. Warning count drops 50 -> 49; tests unchanged at 1620 passed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8e932b4c-7859-42f8-a576-90d5febb343a
This comment has been minimized.
This comment has been minimized.
check-spelling's line_forbidden.patterns flags `out of date` followed by a word, so reword the comment to use the attributive hyphenated form the rest of this PR already uses. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8e932b4c-7859-42f8-a576-90d5febb343a
Problem
The Settings > AI Agents Install hooks button ran
wta hooks install, which re-ran every supported CLI's install commands regardless of what was already on disk. Two separate problems fell out of that:1. Re-installing a complete bridge is wasted work that can fail. It is two Node spawns per CLI, and
<cli> plugin installrewrites the plugin directory — which Windows denies while a running agent CLI holds it open. Measured on a machine where all five CLIs were already installed and current:2. Re-installing an out-of-date bridge does not upgrade it. Every supported CLI answers a second install with "already installed" and changes nothing, and wta's idempotency whitelist converts that into success — so the button reported an upgrade that never happened:
Change
Add
wta hooks install --only-missingand have the button pass it. Instead of a flat list, the pass builds a per-CLI plan from one status pre-pass:InstallUpgradeplugin update/extensions update/ a Codex reinstall;installwould no-opSkipAn unreadable version on either side lands in
Skip: it is not proof of staleness,installwould no-op against a complete bridge anyway, andupgrade_installed_hooksre-checks it at master startup with a richer probe thanCliStatuscarries.Supporting changes:
decide_install_actionis a pure function over oneCliStatusrow, so the whole decision is testable without spawning an agent CLI.plan_installapplies it across the scope and dropsSkipentries, so an empty plan means "nothing to do".upgrade_one_clinow returnsResult<(), String>so a failed upgrade reaches the per-CLI JSON report the Settings UI reads, instead of being swallowed into a barefalse.upgrade_installed_hooksstill only needs the pass/fail bit.wta hooks installis unchanged: every in-scope CLI getsInstall, neverUpgrade. It stays the full-reinstall escape hatch for a break thatstatuscannot see.Verification
Rust:
cargo test --target x86_64-pc-windows-msvc --manifest-path tools/wta/Cargo.toml— 1620 passed, 0 failed.C++: incremental
bxofTerminalSettingsEditor— 0 errors.Live, all five CLIs complete and current:
hooks installhooks install --only-missingSkipRepair path, after
wta hooks uninstall --cli opencode— only opencode is reinstalled, the other four are untouched.Upgrade path, with the copilot bundle manifest bumped to 0.1.7 so the installed 0.1.6 is behind:
plugin update, notplugin install— and the failure (this machine holds the plugin dir open) is reported rather than masked as success.Same with the opencode bundle bumped, which has no process to lock anything, so the upgrade lands:
Upgrade path, all five CLIs
Each was driven by bumping that CLI's bundle manifest above the installed version, then running
wta hooks install --cli <x> --only-missing, and reading the version back fromhooks status.Upgradecopilot plugin update wt-agent-hooks@wt-localupdated successfully (v0.1.6 → v0.1.7), 0.1.6 → 0.1.7Upgradeclaude plugin updateUpgradeextensions uninstall+extensions install(GeminiReinstall)Upgradeplugin remove+marketplace remove+marketplace add+plugin addUpgradeOpenCodeCopy(file copy)All five exited 0. Copilot needed an isolated
USERPROFILEto test: on the dev machine eight livecopilotprocesses hold~/.copilot/installed-plugins/wt-local/open, soplugin updatereturnsAccess is denied. (os error 5)there. That is the same lock that made the old unconditional re-install report failures, and it is now surfaced as a real per-CLI failure rather than masked as success.Codex is worth calling out: its upgrade removes the marketplace before re-adding it, so it also clears the
marketplace 'wt-local' is already added from a different sourcestate that the plain install path could only fail on.Not exercised: gemini's
GeminiUpdateInPlacebranch (gemini extensions update). Gemini's recorded install source did not point under the current bundle, sodecide_upgradecorrectly routed toGeminiReinstallinstead. That branch is covered by the existingupgrade_decision_*unit tests.Bundle manifests were reverted and all five CLIs restored to the bundled 0.1.6 afterwards.