[Andy11-cpu]

Fixes #1857

Supersedes #753.

What

• Keeps install hooks enabled by default.
• Adds persistent install --no-hooks and install --hooks.
• Applies the policy across current hook-capable installer surfaces.
• Preserves MCP, skills, instructions, profiles, and foreign hook state.
• Keeps plan/dry-run non-mutating.

Why

#753 established the performance cost of per-call hook process spawning. This implements the maintainer-approved direction from that discussion: retain graph-first defaults while giving users a persistent opt-out.

Verification

• scripts/test.sh
• scripts/lint.sh --ci CLANG_FORMAT=clang-format-20
• scripts/ci/lint-mem.sh clang-tidy-22
• make -f Makefile.cbm security
• agent-config smoke
• focused hook-policy tests
• git diff --check

All pass locally.

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

[DeusData]

Thank you for implementing the previously discussed persistent hook opt-out and for preserving foreign hook state, dry-run behavior, and the remaining installer surfaces. The contribution queue is currently quite full, so the full review may take a little time. We have the PR routed and will come back with grounded feedback as soon as possible.

[DeusData]

Reviewed. The direction is settled — this implements the approved outcome from #753 — so this is about one build break, and it is a small one.

The Windows failure is real, and it is a missing platform guard

Both test-windows shards, pr-smoke (windows-latest) and test-windows-guards fail for the same reason: the tree does not compile on Windows.

src/cli/cli.c:8846: error: call to undeclared function 'cbm_remove_gemini_coverage_hook'
                    did you mean 'cbm_remove_augment_coverage_hook'?

The compiler's suggestion is a red herring. cbm_remove_gemini_coverage_hook does exist — at src/cli/cli.c:5769 — but its definition sits inside #ifndef _WIN32, which opens a few lines above it. On Windows it is compiled out entirely.

The existing call site already knows this. At cli.c:11104:

#ifndef _WIN32
        if (cbm_remove_gemini_coverage_hook(cp, installed_binary) != CLI_OK) {
            ...
        }
#endif

Your new call is not wrapped. Adding the same #ifndef _WIN32 / #endif pair around it should be the whole fix.

Why only Windows caught it: clang treats an implicit function declaration as an error, and the CLANG64 legs are the ones building with clang. On POSIX the function is present, so every other leg compiles cleanly and says nothing. Worth knowing, because a green Linux and macOS run tells you nothing about this class of mistake.

One knock-on to check while you are there: cbm_upsert_gemini_coverage_hook lives under the same guard at :5753. If the persistent-policy work added an upsert call anywhere, it needs the same treatment — the compiler only reports the first error.

The rest

The shape is right: hooks stay enabled by default, --no-hooks / --hooks persist the choice, plan and dry-run stay non-mutating, and foreign hook state along with MCP, skills, instructions and profiles is preserved. Keeping the graph-first default while giving a persistent opt-out is what #753 concluded, and doing it as a persisted policy rather than a per-invocation flag is what actually solves the reported problem.

Clearance is REVIEW(1) on scripts/smoke-test.sh — the established shape here. It needs a maintainer marker, not a change from you.

Also please rebase once the guard is in: main moved three times yesterday (broken by a duplicate-symbol merge, repaired by #1993, then #1703 landed).

[DeusData]

Thanks again for the persistent hook opt-out. The recorded review asks remain the Windows guard around the Gemini helper call and a branch refresh, preserving the non-mutating plan/dry-run behavior. The smoke clearance mentioned above is a maintainer step, not something you need to change in the patch. Sorry for the delay on our smoke clearance; I’ll follow up on that soon.

[DeusData]

Thank you, @Andy11-cpu, for carrying this all the way from #753. You did the hard part well. The opt-out is exhaustive: every hook-capable installer family is gated, not just Claude Code. The choice persists in the existing config store, so bare installs and update respect it. Foreign hooks and user-owned scripts are left alone, and plan and dry-run stay read-only. Your tests pin all of that through the same dispatch the real install uses, and the Windows guard you pushed within hours of the review was exactly the right fix.

main moved a long way while this waited, and that delay is on us. To spare you a painful rebase, we merged current main into your branch ourselves, as two merge commits plus two small commits on top of your three. None of your commits were rewritten. What changed and why:

1. Claude Code hooks are now registered shell-free. Since this PR was opened, main writes Claude's hooks as <binary> hook-augment entries instead of shim-script commands (Windows: SessionStart hook injects a cmd.exe banner and the echoed payload instead of additionalContext #1733), and the session-hook remover now takes the binary path. So the --no-hooks path for Claude calls the same binary-aware removers that uninstall uses on main; without that, the new entries would not be recognized as ours.
2. Test and smoke markers follow that change. Your settings.json checks looked for cbm-code-discovery-gate, which main no longer writes there. They now look for hook-augment, the owned marker main's own tests use, so the presence checks still bind. The smoke 8e-i checks use $GATE_SCRIPT, so the shim check is also real on Windows, where the file ends in .cmd.
3. Three small conflicts, all resolved by keeping both sides. main added the doc_candidates and watch_non_git config keys next to your install_hooks, and a pretool_written flag in the Codex installer at the spot where your --no-hooks branch starts.
4. Codex gained a PreToolUse hook while this waited (feat(cli): give Codex shell searches graph context through a PreToolUse hook #2719), and the opt-out now covers it. In config.toml the hook sits inside the managed block your opt-out already removes. In hooks.json it is a separate entry next to the SessionStart/SubagentStart pair, so --no-hooks now also removes it, using the same exact-command remover uninstall uses. A user's own PreToolUse hook stays. A new test, cli_no_hooks_removes_codex_pretool_hook, follows your tests' pattern and drives the real install through both representations: a default install, --no-hooks, a bare install with the stored preference, and --hooks.
5. One fixture follows a detection fix on main. Since The install script failed to detect Hermes(Desktop) and OpenCode(Desktop) on Windows #1180, a bare ~/.claude directory no longer counts as a Claude Code install, because other tools create it too. Your plan test now also writes .claude/settings.json, so claude-code is still detected and the test still covers every family.

Because the branch now has new commits from us, please pull before you push again.

Thanks again for building an opt-out that keeps the graph-first default for everyone else. It is a real improvement for the people who need it.