[tayfuryldz]

Fixes #4297.

Hook subcommands now skip the daily telemetry ping and hook-status warning, keeping those background operations out of the latency-sensitive intercepted-command path. Other commands retain the existing behavior.

Validation: cargo test --locked test_hook_claude_parses --bin rtk; git diff --check.

[CLAassistant]

All committers have signed the CLA.

[rtk-wshm-sync-bot]

wshm · Automated triage by AI

📊 Automated PR Analysis

🐛 Type	bug-fix
🟢 Risk	low

Summary

Hook subcommands now skip the daily telemetry ping and hook-status update check, keeping those background operations off the latency-sensitive intercepted-command path used by hooks. Other commands retain their existing telemetry and hook-check behavior.

Review Checklist

• Tests present
• Breaking change
• Docs updated

Linked issues: #4297

---

Analyzed automatically by wshm · This is an automated analysis, not a human review.

[yeahjack]

Tested a87dcef0 against develop 45b0b4c3 with isolated configuration. This addresses the reported hook startup checks: all eight hook processors and hook check skip both maybe_ping and maybe_warn. Entry probes confirmed the calls are skipped; the unmodified binaries produced identical hook stdout and exit codes while the unrelated installation warning disappeared. The 11 existing Codex protocol and safety tests also passed. No real telemetry was sent.

Two points before calling the behavior unchanged for other commands:

1. Please add a regression test that exercises the CLI entry path and checks these side effects, with a normal-command control. test_hook_claude_parses only checks parsing and would pass even if both calls were restored for hooks.
2. Moving maybe_ping after successful parsing also skips it for --help, --version, parse errors, and unknown-command passthrough such as rtk uname -s. The passthrough command still runs normally, but its telemetry behavior changes. If intentional, please state this in the PR description; otherwise preserve the intended behavior on that path.

The tested parsed non-hook paths retained their startup-check behavior: rewrite calls both functions, while Init/Gain/Verify call telemetry only. Those three already skipped the hook warning, but did not previously skip telemetry.

[tayfuryldz]

Addressed the telemetry behavior point: hook commands still skip startup telemetry and hook-status I/O, while help/version, parse failures, and unknown-command passthrough now preserve the previous telemetry path. The existing CLI-level hook warning scope test suite passes (5/5), cargo fmt --check and git diff --check are clean. I am keeping the side-effect coverage scoped to observable CLI behavior rather than adding test-only instrumentation to telemetry.

[tayfuryldz]

Added a regression at the binary entry path rather than another parser-only test. It runs a real hook invocation in an isolated home, then a normal command and verifies the hook did not consume the daily startup warning. The normal command still exercises the background check. The focused integration test is 6/6, and the branch is rebased onto current develop.

[tayfuryldz]

Small correction to my previous note: the first rebase attempt was blocked by a transient working-tree change from the local test run, so I did not want to leave a false claim. I reran the rebase cleanly now, reran the focused integration test, and force-pushed the rebased branch.

[KuSh]

Round 1 — APPROVED, follow-ups filed

Claim: rtk hook … invocations skip the daily telemetry ping and the hook-status warning; every other invocation keeps both.
Scope: accept (round 1, frozen). Since 5d8102a, develop skips the warning for Commands::Hook already, so after the merge this PR contributes the telemetry skip and the first test that pins the warning skip.
Ran: four release binaries (merge-base ff9bd8e, this head 9a1dac0, develop 16a20a2, this head merged onto develop), each built with a telemetry URL pointing at a closed local port so the ping gate can be observed. 22 invocations each, every one in a fresh home with telemetry consent and an empty .claude, LC_ALL=C, stdin from a payload or /dev/null:

• the nine hook processors on a Claude PreToolUse payload
• hook claude </dev/null, hook opencode, hook check, a hook with a bad flag
• ls, git status, rewrite, gain, verify, --help, --version, a passthrough (uname -s)
• the agent sequence: hook, then the command it returns

Invariants: under the hook, split streams, every decision. Savings: not applicable, since stdout and exit codes are identical on every row.
Pins: 3 decisions, 3 mutated, 2 survived on the head as merged. The survivors are both ping decisions: no test build compiles a telemetry URL, so maybe_ping returns at its first line in every test. The warning skip is killed by hook_entry_leaves_startup_warning_for_a_normal_command. Develop's own suite does not catch that mutation, so the new test is the first one to pin it.
Matrix: executed this round; pin check: executed this round
CI @b925ff965: all green (15 checks, run 38984802644)

Blocks merge (0)

Filed as follow-ups

• #4615 rtk rewrite runs the daily hook reminder and telemetry ping on the hook path, so shell hooks spend the reminder into /dev/null. This is outside this PR's hunks: the shell hooks, Hermes and Pi reach rtk through rtk rewrite, not rtk hook.
• #4616 The telemetry marker is written before a detached send that process exit kills, so a fast command spends the day's ping without sending it. Pre-existing in src/core/telemetry.rs, outside this PR's hunks.

Checked and correct — no need to re-verify

• From merge-base to this head, the nine hook processors, hook check and hook claude </dev/null stop writing both markers. Stdout and exit codes are byte-identical on all 22 rows.
• From develop to the merge, the only change is that those hook rows stop writing the telemetry marker. hook claude --bogus, --help, --version and the passthrough still ping, as before.
• This is what the change does for an agent session, measured with strace (3 of 3 runs each). The agent runs rtk hook claude, then the rtk git status it returns.
  • On develop, the hook writes the daily marker and exits before its send thread connects. rtk git status then sees the marker and does not ping. Nothing is sent that day.
  • On the merge, the hook writes nothing, and rtk git status writes the marker and connects.
  • This measures a closed local port, so it shows the send starts, not that a real HTTPS send completes before exit. That part is the second follow-up.
• Latency of rtk hook claude once the markers exist: 7.7 ms on both develop and the merge, with consent and without (hyperfine, 300 runs). The gain is the once-a-day work and the marker no longer being spent inside the hook process, not a per-call cost.
• Full gate on this head, on the merge, and on the pushed fixup in its own checkout: fmt clean, clippy with 0 warnings, all tests pass (4958 across 49 suites on b925ff9).

Your questions

• "I am keeping the side-effect coverage scoped to observable CLI behavior rather than adding test-only instrumentation to telemetry" → kept. The fixup adds no instrumentation. The telemetry half has no observable CLI effect in a test build, so the decision moved into a function the unit tests call on the real parser.

Next

Fixup pushed as b925ff9 and code-reviewed. It changes two things:

• ffdde91 merges develop. run_cli now lives in src/lib.rs, and develop's warning skip already lists Commands::Hook.

• b925ff9 moves the ping decision into pings_telemetry(&parsed), so there is one call site. Two unit tests cover it on the real parser:

  • parsed hook commands (hook claude, hook codex) do not ping;
  • ls, gain, and the invocations clap rejects (help, version, a passthrough, a hook with a bad flag) still run the ping.

  Each of the three ways to get the decision wrong now fails one of the tests. The call site itself stays unpinned for the reason above. Behaviour is unchanged from your head: an independent run of the 22 rows gave identical results.

Approving.
Rounds: 1. Threads: 0 open, 0 resolved this round.