fix(hooks): false 'No hook installed' warning when GitHub Copilot hook is the active integration by marcelsafin · Pull Request #3642 · rtk-ai/rtk

marcelsafin · 2026-08-22T23:29:05Z

Summary

Fix false [warn] No hook installed — run 'rtk init -g' from rtk gain / rtk gain --history when the user-global GitHub Copilot hook ($COPILOT_HOME/hooks/rtk-rewrite.json, default ~/.copilot) is the active integration. Root cause: hook_check::status() only inspected the Claude Code setup, so an existing-but-unconfigured ~/.claude reported Missing despite a working Copilot rewrite hook
Add shared read-only detection: hook_check::copilot_hook_registered parses the hook JSON (not mere file existence) and requires a PreToolUse command entry invoking rtk hook copilot — bare or absolute rtk path, via the same shell-split matching as the Claude hook. Missing file, malformed JSON, empty PreToolUse, or a foreign command never count as installed
Aggregate status() downgrades Claude Missing to Ok only when a valid Copilot hook exists; no-integration and outdated-Claude warnings remain. rtk init --show now reports GitHub Copilot hook status ([ok] registered / [warn] invalid or outdated / [--] not found) and lists the --copilot init flags

Test plan

cargo fmt --all && cargo clippy --all-targets && cargo test (2727 passed, 8 ignored, 10 suites)
New unit tests in hook_check.rs: stock config (incl. the exact COPILOT_HOOK_JSON rtk installs), absolute rtk path, malformed JSON, empty PreToolUse, wrong command, aggregate status matrix (Copilot-only OK with/without .claude, invalid Copilot never suppresses, outdated Claude not masked by Copilot) + matcher tests in hooks/mod.rs
New e2e regression suite tests/hook_status_copilot_test.rs (8 real-binary tests, sandboxed HOME/CLAUDE_CONFIG_DIR/COPILOT_HOME): rtk gain warns/doesn't warn correctly per state; rtk init --show reports each Copilot state
Manual testing: machine with valid ~/.copilot hook + unconfigured ~/.claude — released rtk 0.45.0 prints the false warning; this branch does not, and rtk init --show prints [ok] GitHub Copilot hook: registered (~/.copilot/hooks/rtk-rewrite.json)

rtk gain printed a false '[warn] No hook installed' and rtk init --show omitted Copilot entirely when the only automatic integration was the user-global GitHub Copilot hook. Root cause: hook_check::status() only inspected the Claude Code setup (~/.claude settings.json / legacy script). A machine with an existing but RTK-unconfigured ~/.claude and a valid Copilot hook under $COPILOT_HOME/hooks/rtk-rewrite.json (default ~/.copilot) was reported as Missing. Fix: - add is_copilot_hook_command (shared shell-split matcher with the Claude variant; accepts bare and absolute rtk paths) - add hook_check::copilot_hook_registered: parses the hook JSON and requires a PreToolUse command entry invoking 'rtk hook copilot' — missing files, malformed JSON, empty PreToolUse, or foreign commands do not count as installed - aggregate in status(): a Claude Missing result is downgraded to Ok only when a valid Copilot hook exists; Outdated Claude hooks still warn - rtk init --show now reports GitHub Copilot hook status ([ok]/[warn]/ [--]) and documents the --copilot init flags Detection stays read-only. Covered by new unit tests in hook_check.rs and e2e regression tests in tests/hook_status_copilot_test.rs using sandboxed HOME/CLAUDE_CONFIG_DIR/COPILOT_HOME. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CLAassistant · 2026-08-22T23:29:12Z

All committers have signed the CLA.

Copilot

Pull request overview

Fixes a false “No hook installed” warning by extending hook-status detection to recognize a valid GitHub Copilot user-global hook ($COPILOT_HOME / ~/.copilot/hooks/rtk-rewrite.json) and surfacing that status in rtk init --show.

Changes:

Add shared Copilot hook detection (JSON-parsed + command-validated) and aggregate it into hook_check::status() so valid Copilot setups suppress the missing-Claude-hook warning.
Extend rtk init --show to report GitHub Copilot hook status and document the aggregated behavior.
Add unit + end-to-end regression tests covering valid/invalid/missing Copilot hook scenarios and aggregate-status behavior.

Reviewed changes

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

Show a summary per file

File	Description
tests/hook_status_copilot_test.rs	New e2e regression tests validating `rtk gain` warning behavior and `rtk init --show` Copilot status output in a sandboxed environment.
src/hooks/README.md	Documentation update describing aggregated Claude + Copilot status behavior and warning semantics.
src/hooks/mod.rs	Generalize hook-command detection and add `is_copilot_hook_command` with matcher tests.
src/hooks/init.rs	Add Copilot hook status line to `rtk init --show` output and include `--copilot` usage hints.
src/hooks/hook_check.rs	Implement Copilot hook JSON validation and aggregate it into overall hook `status()` logic; add unit tests for Copilot + aggregate scenarios.

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

marcelsafin · 2026-08-22T23:40:50Z

 pub fn status() -> HookStatus {
+    let claude_dir = resolve_claude_dir().ok();
+    let copilot_dir = copilot_user_dir().ok();
+    status_at(claude_dir.as_deref(), copilot_dir.as_deref())
+}


Good catch — fixed in 4af75d6. check_and_warn() now checks the rate-limit marker before computing status, so a fresh marker skips all detection I/O and JSON parsing on the hot path. Output behavior is unchanged since a fresh marker suppressed the warning anyway.

check_and_warn() runs on most CLI invocations via maybe_warn(). Check the once-per-day rate-limit marker first so a fresh marker skips all hook detection disk I/O and JSON parsing. Output behavior unchanged: a fresh marker suppressed the warning anyway. Addresses Copilot review feedback on rtk-ai#3642. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Suppressed comments (1)

src/hooks/hook_check.rs:135

This accepts the command field even when the entry declares another hook type (for example "type": "http"). Such an entry does not execute rtk hook copilot, but it would suppress the missing-hook warning. Allow an omitted type (which defaults to command) or require it to be exactly command.

    entries
        .iter()
        .filter_map(|entry| entry.get("command")?.as_str())
        .any(is_copilot_hook_command)

marcelsafin · 2026-08-22T23:47:40Z

+    let root: serde_json::Value = match serde_json::from_str(&content) {
+        Ok(v) => v,
+        Err(_) => return false,
+    };


Fixed in d1d5c9a — copilot_hook_registered now requires version == 1 before inspecting entries; missing or different versions no longer count as installed. Covered by test_copilot_hook_wrong_or_missing_version_not_registered.

marcelsafin · 2026-08-22T23:47:42Z

+    /// Seed the tracking database so `rtk gain` reaches its summary view
+    /// (and thus its hook-status warning) instead of "No tracking data yet".
+    fn seed_tracking_data(&self) {
+        let out = self.rtk(&["proxy", "echo", "ok"]);


Fixed in d1d5c9a — seeding now proxies the rtk binary itself (CARGO_BIN_EXE_rtk --version) instead of echo, so it is portable to Windows.

marcelsafin · 2026-08-22T23:47:43Z

    let binary_name = binary.rsplit(['/', '\\']).next().unwrap_or(binary);

-    binary_name == "rtk" && hook == "hook" && claude == "claude"
+    binary_name == "rtk" && hook == "hook" && target == agent


Fixed in d1d5c9a — the matcher now parses the command string without shell unescaping (backslash paths survive), accepts a quoted binary path with spaces, and strips a trailing .exe case-insensitively. Added Windows-path tests incl. lookalike rejections (not-rtk.exe, rtk.exe.bak).

marcelsafin · 2026-08-22T23:47:44Z

+            .env("HOME", &self.home)
+            .env("USERPROFILE", &self.home)
+            .env("XDG_DATA_HOME", self.home.join(".local/share"))
+            .env("XDG_CONFIG_HOME", self.home.join(".config"))
+            .env("CLAUDE_CONFIG_DIR", &self.claude_dir)
+            .env("COPILOT_HOME", &self.copilot_home)


Fixed in d1d5c9a — the sandbox now pins RTK_DB_PATH inside the temp dir, so tests never touch the real tracking database and cannot race each other.

- require version == 1 in rtk-rewrite.json: Copilot rejects hook files with a missing or different version, so they never run and must not count as installed - make the rtk-hook-command matcher Windows-aware: parse the command string without shell unescaping so backslash paths survive, accept a quoted binary path with spaces, and strip a trailing .exe - isolate integration tests: pin RTK_DB_PATH inside the sandbox (it outranks XDG/home paths and is honored on Windows) and seed tracking by proxying the rtk binary itself instead of echo, which is a shell built-in on Windows Addresses Copilot review feedback on rtk-ai#3642. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

Suppressed comments (1)

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

src/hooks/hook_check.rs:139

The detector ignores the required hook-entry type. A file containing { "type": "prompt", "command": "rtk hook copilot" } is therefore reported as registered even though Copilot will not execute it as a command, which can reintroduce the false suppression this change is intended to fix. Filter for type == "command" before matching the command string.

    entries
        .iter()
        .filter_map(|entry| entry.get("command")?.as_str())
        .any(is_copilot_hook_command)

marcelsafin · 2026-08-23T00:05:37Z

+            let inner = &trimmed[1..];
+            let Some(end) = inner.find(quote) else {
+                return false;
+            };
+            (&inner[..end], &inner[end + 1..])
+        }


Fixed in 8f7e960 — the closing quote must now be followed by whitespace (or end the string); "rtk"hook copilot and similar glued forms are rejected. Covered by hook_command_rejects_quote_glued_to_next_token.

"rtk"hook copilot is not a registration of the rtk binary — the closing quote must end the token. Addresses Copilot review feedback on rtk-ai#3642. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

Suppressed comments (3)

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

src/hooks/hook_check.rs:139

The detector accepts any entry containing this command, even when its type is not command. A structurally invalid entry such as {"type":"prompt","command":"rtk hook copilot"} can therefore suppress the missing-hook warning and be shown as registered although Copilot will not execute it as a command hook. Require type == "command" before matching command.

    entries
        .iter()
        .filter_map(|entry| entry.get("command")?.as_str())
        .any(is_copilot_hook_command)

tests/hook_status_copilot_test.rs:78

The database is isolated, but seeding still invokes rtk proxy, which runs maybe_warn() before the proxy command. In the no-hook/invalid-hook cases this reads and writes dirs::data_local_dir()/rtk/.hook_warn_last; on Windows that location is not redirected by HOME, USERPROFILE, or the XDG variables here, so tests can mutate the real user's warning marker and race each other. Run the seed command with a separate nonexistent Claude config (so status is Ok) or otherwise isolate the marker path.

            // RTK_DB_PATH outranks the XDG/home paths and is honored on
            // Windows too — pin the tracking DB inside the sandbox so tests
            // never touch the developer's real database.
            .env("RTK_DB_PATH", self.home.join("rtk-history.db"))
            .env("CLAUDE_CONFIG_DIR", &self.claude_dir)
            .env("COPILOT_HOME", &self.copilot_home)

src/hooks/mod.rs:53

Lowercasing the basename on every platform accepts case-variant lookalikes such as /tmp/RTK hook claude on case-sensitive Unix systems. That can falsely mark a foreign hook as valid and can cause Claude init/uninstall logic to treat it as RTK's own entry. Apply case-insensitive rtk/rtk.exe matching only on Windows; retain exact rtk matching elsewhere.

    let binary_name = binary.rsplit(['/', '\\']).next().unwrap_or(binary);
    let binary_name = binary_name.to_ascii_lowercase();
    let binary_name = binary_name.strip_suffix(".exe").unwrap_or(&binary_name);

    binary_name == "rtk" && hook == "hook" && target == agent

marcelsafin · 2026-08-23T01:21:42Z

+        Some(_) => {
+            let end = trimmed.find(char::is_whitespace).unwrap_or(trimmed.len());
+            trimmed.split_at(end)
+        }


Fixed in 46cbcc5 — the matcher now accepts either interpretation: POSIX shell form via shell_split (restoring /opt/RTK\ Tools/rtk hook claude and quoted paths with spaces) or the raw Windows form (preserving C:\rtk\rtk.exe hook copilot). Added regression tests covering escaped Unix paths alongside the existing Windows-path tests.

The raw parser rejected valid POSIX shell-escaped executable paths such as /opt/RTK\ Tools/rtk hook claude, which shell_split previously handled. Match against both interpretations: POSIX shell form (via shell_split) or raw Windows form, so escaped Unix paths and backslash Windows paths are both recognized. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI lite review requested due to automatic review settings August 22, 2026 23:29

Copilot started reviewing on behalf of marcelsafin August 22, 2026 23:29 View session

Copilot AI reviewed Aug 22, 2026

View reviewed changes

marcelsafin requested a balanced review from Copilot August 22, 2026 23:34

Copilot started reviewing on behalf of marcelsafin August 22, 2026 23:34 View session

Copilot AI reviewed Aug 22, 2026

View reviewed changes

marcelsafin requested a balanced review from Copilot August 22, 2026 23:50

Copilot started reviewing on behalf of marcelsafin August 22, 2026 23:50 View session

Copilot AI reviewed Aug 22, 2026

View reviewed changes

fix(hooks): require whitespace after quoted binary in hook matcher

8f7e960

"rtk"hook copilot is not a registration of the rtk binary — the closing quote must end the token. Addresses Copilot review feedback on rtk-ai#3642. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

marcelsafin requested a balanced review from Copilot August 23, 2026 00:21

Copilot started reviewing on behalf of marcelsafin August 23, 2026 00:21 View session

Copilot AI reviewed Aug 23, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix(hooks): false 'No hook installed' warning when GitHub Copilot hook is the active integration - #3642

fix(hooks): false 'No hook installed' warning when GitHub Copilot hook is the active integration#3642
marcelsafin wants to merge 5 commits into
rtk-ai:developfrom
marcelsafin:fix/copilot-hook-status-detection

marcelsafin commented Aug 22, 2026 •

edited

Loading

Uh oh!

CLAassistant commented Aug 22, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

marcelsafin Aug 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

marcelsafin Aug 22, 2026

Uh oh!

marcelsafin Aug 22, 2026

Uh oh!

marcelsafin Aug 22, 2026

Uh oh!

marcelsafin Aug 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

marcelsafin Aug 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

marcelsafin Aug 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

marcelsafin commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

CLAassistant commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

marcelsafin Aug 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

marcelsafin Aug 22, 2026

Choose a reason for hiding this comment

Uh oh!

marcelsafin Aug 22, 2026

Choose a reason for hiding this comment

Uh oh!

marcelsafin Aug 22, 2026

Choose a reason for hiding this comment

Uh oh!

marcelsafin Aug 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

marcelsafin Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

marcelsafin Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

marcelsafin commented Aug 22, 2026 •

edited

Loading

CLAassistant commented Aug 22, 2026 •

edited

Loading