Make install hooks opt-in via --hooks - #753
Conversation
PreToolUse and SessionStart augmenter hooks are no longer installed by default. Pass --hooks to install to opt in. Install plan receipts omit hook paths unless --hooks is set. Signed-off-by: Andy11-cpu <canada11@duck.com>
|
Thanks for splitting out the hook opt-in behavior. Triage: installer/editor-integration UX change, currently conflicting. Please rebase on current |
|
Thank you for this, and for splitting it out of #625 as asked — that made it reviewable on its own terms. What this actually is: a product-direction question, not a code review. Making hooks opt-in changes what every user of the documented one-line install gets. The performance motivation is real — a process spawn per Grep and Glob is not nothing — but the hooks are also the mechanism that steers agents toward the graph rather than grepping, which is this project's whole findability-first premise. Making them opt-in means the majority who use the one-liner silently lose that steering layer. Both positions are defensible, and it is not mine to decide. I have put it to the maintainer directly: hooks opt-in (performance-polite) or opt-out (findability-first default)? Everything else here is fixable mechanics that follow from that answer. Your test is well-placed. One thing I would want changed regardless of which way the decision goes, because it is a genuine design gap rather than a preference:
Three smaller things, all consequences of it being a user-facing default change:
On staleness, so you can plan: every signature you touch has moved. Please hold off reworking until the direction answer arrives. Between the framework rebuild and the smoke changes this needs, it is a substantial piece of work, and it would be unfair to have you do it against a decision that might go the other way. I will come back to you either way. |
|
No worries. Havent had a chance to catch up with this , i have been using a heavily modified version of this as a daily driver but the set up is something pretty manual and not very agentic in my opinion ( the way i like it ) . if you need anything though let me know , would like to pitch in 100% |
|
The direction question has an answer — thank you for waiting on it, and sorry it took this long. Verdict: keep hooks on by default, add an opt-OUT flag, and persist the choice. The reasoning, so it's not just a verdict: those hooks are the mechanism that steers an agent toward the graph instead of grep. If they become opt-in, the overwhelming majority of installs silently never get graph-first behaviour, and the product quietly becomes worse for everyone who didn't read the flag list. But your complaint is real and you measured it — per-call process-spawn overhead is a genuine cost, not a preference — so the escape hatch should exist. It just needs to point the other way. Concretely, what we'd merge: 1. 2. The choice persists. This is the part that matters most and wasn't in the original PR. There's an existing per-user key-value store —
That last point is what makes it real: 3. Every hook site must be gated, not just Claude Code's. This is the part I'd most want you to know before starting, because I went looking and it's larger than it appears. Hook writes live in at least: Claude Code (gate + session + subagent), Gemini CLI (before-tool + after-tool + session), Codex, Copilot/VS Code, Qoder, GitLab Duo, Devin, Kimi, Hermes, Qwen, Factory, Augment — plus the agent-client registry path. They're spread across A partially-applied opt-out — hooks off for Claude, still written for Codex — is worse than no flag at all, so this needs to be exhaustive. 4. Each skipped client should say so. The original only printed a skip notice on the Claude Code path. Silence about a skipped group is precisely what made #1387 hard to see; one line per client ("hooks: skipped (install --hooks to enable)") is enough. 5. Smoke phases 8d/8e assert hooks exist after a plain install. Since the default is unchanged they should stay green as-is — but a 6. A I started sketching this myself and stopped: it's a wider change than it looks, and it's the install path, where a half-applied version is genuinely harmful. You offered to help and you're the one who cared enough to profile the overhead — so it's yours if you want it, and I'll review promptly this time rather than leaving you on a queue. If you'd rather not carry it, say so and I'll pick it up; no hard feelings either way. Either way, please rebase before starting — the branch conflicts with |
|
On it |
|
Superseded by #1862. I rebuilt the implementation from current \main\ around the maintainer-approved default-on, persistent opt-out design and expanded coverage across the current hook surfaces. |
Summary
--hookstoinstallto opt in--hooksis setMotivation
Split from #625 — avoids spawning hook augmenters on every Grep/Glob unless the user explicitly opts in.
Test plan
cli_install_plan_hooks_opt_in_defaultintest_cli.cmake -f Makefile.cbm test(CI)Signed-off-by: Andy11-cpu canada11@duck.com