Recover hook install when a running agent CLI holds the plugin directory

[Yuandi (DDKinger)]

Summary

With an agent CLI already running elsewhere — most commonly VS Code, which keeps several copilot processes alive for the whole editor session — every hook install, update, and uninstall fails:

> copilot plugin install wt-agent-hooks@wt-local
Failed to install plugin: Error: Failed to install plugin: Access is denied. (os error 5)

This hits both entry points: the auto-upgrade at wta-master startup and the Settings UI / FRE "Install hooks" button.

Root cause

A live CLI process holds an open handle on the plugin directory it loaded at startup, so the CLI's "replace the installed plugin folder" step can neither rename nor remove that directory.

The handle only blocks operations on the directory itself. Probing ~/.copilot/installed-plugins/wt-local/wt-agent-hooks/ while 12 copilot processes were live:

Operation	Result
Rename / remove the directory	❌ Access is denied
Create a file or subdirectory inside	✅
Overwrite an existing file	✅
Delete a file or subdirectory inside	✅

Fix

run_plugin_cli_unlocking uses that asymmetry. When a plugin CLI fails with the locked-directory signature, it copies the directory contents aside, empties the directory with file-level deletes, and runs the same command again.

• Retry succeeds → the stash is discarded.
• Retry fails again → the contents are restored, and the original CLI error is reported. A permission problem we cannot recover from leaves the user's existing hooks exactly as they were.

The directory itself is never removed. That also keeps the working directory valid for live CLI sessions, which spawn hooks with their cwd set to the plugin root — removing it out from under them makes their hook spawns fail with os error 267.

Wired into install / update / uninstall for Copilot, Claude, Codex, and Gemini. OpenCode opts out: its hooks are a wta-owned file copy with no CLI-owned directory-replace step.

Behavior is unchanged unless all of these hold: the CLI ran but did not reach its goal state, its output matches the locked-directory signature, and the plugin directory exists and is non-empty.

Drive-by: plugin_cli_reached_goal and plugin_cli_exit_error are extracted out of run_plugin_cli_with_env so both paths report failures identically. The error now carries the CLI's own message instead of just an exit code, so wta-install-hooks.log says why a step failed.

Validation

Verified against Copilot CLI 1.0.81 with twelve live copilot processes.

Manual, before the fix — establishing that clearing is sufficient:

• Install into the populated directory → fails, 4/4 attempts.
• Install into a brand-new directory → succeeds.
• Empty the directory, then install into it → succeeds, 2/2 attempts.

End-to-end with the built binary, against a held directory handle:

• Retry succeeds → retry succeeded after clearing the locked plugin directory, plugin contents present, stash discarded.
• Retry still fails → contents restored byte-for-byte, original error surfaced, no data loss.
• Uninstall under the same lock → plugin=ok marketplace=ok staging=ok.

Automated:

cargo test --target x86_64-pc-windows-msvc --manifest-path tools/wta/Cargo.toml
test result: ok. 1619 passed; 0 failed

11 new tests cover the failure-signature matching (including the os error 32 sharing-violation spelling and the negative cases that must not trigger the destructive path), the per-CLI directory layouts, clear_dir_contents leaving the directory in place, the stash/restore round-trip, and the error-message plumbing.

Notes for reviewers

• The recovery is deliberately narrow. is_locked_plugin_dir_failure gates it so an unrelated failure — a missing marketplace, a stale source path — never causes us to clear a working install.
• cargo fmt was not run crate-wide: the committed tree is not clean under the locally installed rustfmt and formatting everything would rewrite ~46k lines. agent_hooks_installer.rs was verified to be rustfmt-clean on its own.

[Copilot]

Pull request overview

This PR hardens WTA’s agent hook install/update/uninstall flows against a common Windows failure mode where a long-running agent CLI (often VS Code’s copilot processes) holds an open handle on the installed plugin directory, preventing directory-level replace/remove operations. It introduces a targeted “unlock by clearing contents then retry” recovery path and expands unit test coverage around signature matching and rollback behavior.

Changes:

• Route Copilot/Claude/Codex/Gemini hook install/update/uninstall through run_plugin_cli_unlocking, which detects the locked-directory signature, stashes contents, clears the directory contents (without removing the directory), and retries the CLI command.
• Refactor plugin CLI result handling into plugin_cli_reached_goal + plugin_cli_exit_error so failures consistently surface the CLI’s own message (not just an exit code).
• Add new unit tests covering locked-directory signature detection, per-CLI installed-dir layout, clear/stash/restore behavior, and error plumbing.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
tools/wta/src/agent_hooks_installer.rs	Adds locked plugin-directory recovery logic, wires it into install/update/uninstall flows, and refactors plugin CLI goal/error handling.
tools/wta/src/agent_hooks_installer_tests.rs	Adds unit tests for failure-signature matching, directory layout resolution, clear/stash/restore round-trip, and improved error messages.

Suppressed comments (1)

tools/wta/src/agent_hooks_installer.rs:3121

• restore_plugin_dir_contents uses copy_dir_recursive(stash, dir) without clearing dir first. If the retry created any new files/subdirectories before failing, those extra entries will remain and the “restore” won’t actually return the plugin directory to its previous state. To make the rollback reliable, clear dir’s contents before copying back from stash (and keep stash if the clear/copy fails).

/// Put a stash taken by [`stash_plugin_dir_contents`] back and discard
/// it. Best-effort: a failure is logged at error because it leaves the
/// plugin directory empty, with the only surviving copy in the stash.
fn restore_plugin_dir_contents(stash: &Path, dir: &Path) {
    match copy_dir_recursive(stash, dir) {
        Ok(()) => discard_plugin_dir_stash(stash),
        Err(e) => tracing::error!(

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

tools/wta/src/agent_hooks_installer.rs:3146

• restore_plugin_dir_contents restores by calling copy_dir_recursive(stash, dir) without first removing entries that may have been created by the failed retry. Because copy_dir_recursive only creates/overwrites paths and never deletes preexisting destination entries, a retry that partially wrote new files can leave extra files behind, so the plugin dir may not be restored to its pre-recovery state (potentially mixing old + new hook contents). Consider clearing dir’s contents (leaving the directory itself) before copying from stash, and only discarding the stash after both the clear and copy succeed.

/// Put a stash taken by [`stash_plugin_dir_contents`] back and discard
/// it. Copies over whatever is still in `dir`, so it works for both a
/// fully emptied directory and one a failed clear left half-populated.
/// Best-effort: a failure is logged at error because it leaves the
/// plugin directory incomplete, with the only surviving copy in the
/// stash.
fn restore_plugin_dir_contents(stash: &Path, dir: &Path) {
    match copy_dir_recursive(stash, dir) {
        Ok(()) => discard_plugin_dir_stash(stash),
        Err(e) => tracing::error!(
            target: "agent_hooks",
            err = %e,
            stash = %stash.display(),
            dir = %dir.display(),
            "failed to restore plugin directory contents; the previous contents remain in `stash`",
        ),
    }
}

[Yuandi (DDKinger)]

Picked up the suppressed finding from the last review (restore_plugin_dir_contents, agent_hooks_installer.rs:3146) in c45d1e1 — it was correct and worth acting on even though the bot filed it as low-confidence.

copy_dir_recursive only creates and overwrites; it never deletes destination entries the source lacks. That is harmless on the rollback path in stash_plugin_dir_contents, where dir only ever holds a subset of the stash. It is not harmless on the retry path: the CLI runs against the cleared directory and can write part of the new plugin before failing, so entries with no counterpart in the stash survived the restore and the user got a mix of old and new hook files instead of the pre-recovery state.

restore_plugin_dir_contents now empties the directory before copying the stash back, and keeps the stash unless both the clear and the copy succeeded — it is the only pristine copy left if either step fails. Covered by restore_plugin_dir_contents_removes_what_a_failed_retry_left_behind, which stages a partial retry write and asserts both that shared paths roll back and that the leftover subtree is gone.

1621 tests pass.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

tools/wta/src/agent_hooks_installer.rs:3274

• spawn_step_unlocking formats failures as fail: <exe> <args> :: <error>, but plugin_cli_exit_error already embeds <exe> <args> in the error string. This produces redundant, noisy messages like fail: copilot ... :: copilot ... exited 1: ... in uninstall output. Consider changing the failure formatting to avoid duplicating the command (e.g. fail: {e} or fail: exited ...: ...).

        Err(e) => {
            messages.push(format!("fail: {} {} :: {}", exe, args.join(" "), e));
            false

tools/wta/src/agent_hooks_installer.rs:2924

• run_plugin_cli_with_env still takes a log_target parameter (now named _log_target) but never uses it; all tracing in this path uses the hard-coded target "agent_hooks". Keeping an unused parameter makes call sites misleading and suggests per-CLI log routing that no longer happens. Consider removing the parameter (and updating callers) or reintroducing a meaningful use (e.g. a structured field) so intent is clear.

    args: &[&str],
    env: &[(&str, &str)],
    _log_target: &str,
    idempotency_substrings: &[&str],

tools/wta/src/agent_hooks_installer.rs:3085

• is_locked_plugin_dir_failure currently treats any occurrence of generic phrases like "operation not permitted" / "resource busy" as a locked-plugin-dir signature. But this file already documents another EPERM case (EPERM: operation not permitted, scandir ... when installing from WindowsApps) that is unrelated to directory locks; matching it would unnecessarily clear an existing plugin dir and could temporarily break running hook spawns. Consider tightening the predicate to require a more specific combination (e.g. os error 5/32 / "access is denied" / "being used by another process", or for Node errors require EPERM/EBUSY and an operation like rename/rmdir).

fn is_locked_plugin_dir_failure(stdout: &str, stderr: &str) -> bool {
    matches_idempotency_substring(stdout, stderr, LOCKED_PLUGIN_DIR_NEEDLES)
}

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.