diff --git a/CHANGELOG.md b/CHANGELOG.md index bfb4168a3..3456352b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ installable release; see the roadmap in [README.md](README.md). ### Changed +- **`search-tool` and `search-tool-bash` hooks default-on** ([#738](https://github.com/robotrocketscience/aelfrice/issues/738)). `aelf setup` (no flags) now wires both `PreToolUse:Grep|Glob` and `PreToolUse:Bash` (grep/rg/find/fd/ack) memory-first retrieval hooks. The README's "four parallel retrieval lanes" claim was previously aspirational โ€” the agent's own search calls skipped the retrieval pipeline unless the operator explicitly opted in. The flags now follow the `--X / --no-X` BooleanOptionalAction convention used by every other default-on hook; the legacy `--search-tool` opt-in flag and the asymmetric `--no-search-tool-bash` setup-time uninstall are removed. Use `aelf setup --no-search-tool` / `aelf setup --no-search-tool-bash` to opt out โ€” the opt-out persists across upgrades via `~/.aelfrice/opt-out-hooks.json`. Bare `aelf unsetup` removes both hooks for symmetry. Auto-install reconcile (`maybe_install_manifest`) picks up the two new manifest rows on the first `aelf ` after `uv tool upgrade aelfrice`. Worst-case latency impact at v3.0-typical 10k-belief corpus: ~22ms / turn for 5 Grep fires + ~12ms / turn for 3 Bash-search fires (bash hook capped at 3 firings per turn via `BASH_FIRE_CAP_PER_TURN`); no new retrieval code paths are enabled by the flip. The `bfs_enabled` (L2 graph walk) default-flip is tracked separately under #739 and is bench-gated on a fresh v3.0 latency band run. + - **Install / upgrade surface collapsed to `uv tool` only** ([#730](https://github.com/robotrocketscience/aelfrice/issues/730)). `aelf upgrade-cmd` (and the `/aelf:upgrade` slash) now emit a single in-place upgrade form (`uv tool upgrade aelfrice`) for uv-managed installs and a migration chain (`pipx uninstall aelfrice && uv tool install aelfrice`, or the pip equivalent) for any other installer. `UpgradeAdvice.context` collapses from four values (`uv_tool` / `pipx` / `venv` / `system`) to two (`uv_tool` / `non_uv`). Per-installer context notes are gone; `_UPGRADE_CONTEXT_NOTE` retains a single uv entry plus one explicit migration note. The pipx/venv/system detection helpers in `lifecycle.py` are still used internally to pick the right uninstall verb, but they no longer surface as distinct supported channels. README and `docs/INSTALL.md` rewritten around `uv tool install`. **Migration for existing pipx / pip users:** run `pipx uninstall aelfrice && uv tool install aelfrice` (or `pip uninstall -y aelfrice && uv tool install aelfrice`) once. The next `aelf upgrade-cmd` against a non-uv install emits the same line. ## [3.0.0] - 2026-05-13 diff --git a/docs/INSTALL.md b/docs/INSTALL.md index 5f2d97587..6a0c7bf39 100644 --- a/docs/INSTALL.md +++ b/docs/INSTALL.md @@ -142,22 +142,26 @@ Bare `aelf setup` wires the v1.2.0 auto-capture pipeline alongside the read-side | transcript-ingest | `UserPromptSubmit` + `Stop` + `PreCompact` + `PostCompact` | **on** | logs every turn to a per-project JSONL; PreCompact rotates the file and ingests it into beliefs/edges | | commit-ingest | `PostToolUse:Bash` | **on** | each successful `git commit` runs the triple extractor on the message | | session-start | `SessionStart` | **on** | new sessions open with L0 locked beliefs already injected | +| stop-lock-prompt | `Stop` | **on** | prompt to lock correction-class beliefs from this session (#582) | +| search-tool | `PreToolUse:Grep` / `Glob` | **on** (v3.0.1+) | belief-store check before the agent's own Grep/Glob fires | +| search-tool-bash | `PreToolUse:Bash` | **on** (v3.0.1+) | belief-store check before shell grep/rg/find/fd/ack fires | | rebuilder | `PreCompact` | off | retrieval-curated context rebuilder (augment-mode, v1.4 alpha) | -| search-tool | `PreToolUse:Grep` / `Glob` | off | belief-store check before the agent's own search tool fires | -Opt out per-hook: +Opt out per-hook (persists across upgrades via `~/.aelfrice/opt-out-hooks.json`): ```bash aelf setup --no-transcript-ingest # skip the four transcript-logger hooks aelf setup --no-commit-ingest # skip the commit-message ingest hook aelf setup --no-session-start # skip the SessionStart locked-belief injection +aelf setup --no-stop-hook # skip the Stop lock-prompt hook +aelf setup --no-search-tool # skip the PreToolUse:Grep|Glob hook +aelf setup --no-search-tool-bash # skip the PreToolUse:Bash hook ``` Opt in to the off-by-default hooks: ```bash aelf setup --rebuilder # PreCompact context rebuilder (alpha) -aelf setup --search-tool # PreToolUse:Grep|Glob memory-first search ``` `aelf unsetup` mirrors: bare invocation removes every default-on hook. `--no-*` flags suppress per-hook removal. diff --git a/docs/search_tool_hook.md b/docs/search_tool_hook.md index 933b74997..c9d0a1793 100644 --- a/docs/search_tool_hook.md +++ b/docs/search_tool_hook.md @@ -1,9 +1,13 @@ # Search-tool hook -**Status:** spec. -**Target milestone:** v1.2.x patch (pulled forward from v1.3.0 โ€” it ships -independently of the v1.3 retrieval wave and validates the `PreToolUse` -retrieval surface ahead of the bigger work). Default-on candidate at v1.3.0. +**Status:** shipped; **default-on as of v3.0.1** (#738). Both the +`PreToolUse:Grep|Glob` surface and the `PreToolUse:Bash` (grep/rg/find/ +fd/ack) extension wire automatically via `aelf setup`. Opt out per-hook +with `aelf setup --no-search-tool` / `--no-search-tool-bash` (opt-out +persists via `~/.aelfrice/opt-out-hooks.json`). Historical design +discussion below pre-dates the flip; treat the "default-OFF" / "gated +on telemetry" notes as the original v1.5.0 framing rather than current +state. **Dependencies:** stdlib only. Consumes the v1.0 retrieval pipeline ([`aelfrice.retrieval.retrieve`](../src/aelfrice/retrieval.py)) and the v1.1.0 per-project DB resolution @@ -76,8 +80,9 @@ configuration matches on tool calls and emits a JSON object with `hookSpecificOutput.additionalContext` to inject results. Configuration lives under the user's `~/.claude/settings.json`. The -opt-in surface is `aelf setup --search-tool`, mirroring `aelf setup ---commit-ingest` from v1.2.0. +hook is wired by default at v3.0.1+ (`aelf setup` with no flags +installs both this hook and the Bash extension). Pass +`aelf setup --no-search-tool` to skip and persist the opt-out. ### Hook contract (PreToolUse) diff --git a/src/aelfrice/auto_install.py b/src/aelfrice/auto_install.py index 2c6fd4ba4..71cf7b8fc 100644 --- a/src/aelfrice/auto_install.py +++ b/src/aelfrice/auto_install.py @@ -52,12 +52,16 @@ SettingsScope, USER_SETTINGS_PATH, install_commit_ingest_hook, + install_search_tool_bash_hook, + install_search_tool_hook, install_session_start_hook, install_stop_hook, install_transcript_ingest_hooks, install_user_prompt_submit_hook, resolve_commit_ingest_command, resolve_hook_command, + resolve_search_tool_bash_command, + resolve_search_tool_command, resolve_session_start_hook_command, resolve_stop_hook_command, resolve_transcript_logger_command, @@ -311,6 +315,14 @@ def _atomic_write_json(path: Path, data: dict[str, object]) -> None: resolve_stop_hook_command, install_stop_hook, ), + "search_tool": ( + resolve_search_tool_command, + install_search_tool_hook, + ), + "search_tool_bash": ( + resolve_search_tool_bash_command, + install_search_tool_bash_hook, + ), } diff --git a/src/aelfrice/cli.py b/src/aelfrice/cli.py index 1a42071fe..57fe67e51 100644 --- a/src/aelfrice/cli.py +++ b/src/aelfrice/cli.py @@ -2472,7 +2472,7 @@ def _cmd_setup(args: argparse.Namespace, out: object) -> int: f"(command={ci_command!r})", file=out, # type: ignore[arg-type] ) - if getattr(args, "search_tool", False): + if getattr(args, "search_tool", True): st_command = resolve_search_tool_command(scope) st_result = install_search_tool_hook( path, command=st_command, timeout=args.timeout, @@ -2489,7 +2489,7 @@ def _cmd_setup(args: argparse.Namespace, out: object) -> int: f"(command={st_command!r})", file=out, # type: ignore[arg-type] ) - if getattr(args, "search_tool_bash", False): + if getattr(args, "search_tool_bash", True): stb_command = resolve_search_tool_bash_command(scope) stb_result = install_search_tool_bash_hook( path, command=stb_command, timeout=args.timeout, @@ -2506,21 +2506,6 @@ def _cmd_setup(args: argparse.Namespace, out: object) -> int: f"{stb_result.path} (command={stb_command!r})", file=out, # type: ignore[arg-type] ) - if getattr(args, "no_search_tool_bash", False): - stb_rm = uninstall_search_tool_bash_hook( - path, command_basename=SEARCH_TOOL_BASH_SCRIPT_NAME, - ) - if stb_rm.removed == 0: - print( - f"no search-tool-bash hook in {stb_rm.path}", - file=out, # type: ignore[arg-type] - ) - else: - print( - f"removed {stb_rm.removed} search-tool-bash entr" - f"{'y' if stb_rm.removed == 1 else 'ies'} from {stb_rm.path}", - file=out, # type: ignore[arg-type] - ) slash_dest = getattr(args, "slash_commands_dir", None) slash_dest_path = Path(slash_dest) if slash_dest else None sc_result = install_slash_commands(slash_dest_path) @@ -2555,6 +2540,8 @@ def _cmd_setup(args: argparse.Namespace, out: object) -> int: "commit_ingest": "commit_ingest", "session_start": "session_start", "stop_hook": "stop_lock_prompt", + "search_tool": "search_tool", + "search_tool_bash": "search_tool_bash", } @@ -2761,7 +2748,7 @@ def _cmd_unsetup(args: argparse.Namespace, out: object) -> int: f"{'y' if ci_result.removed == 1 else 'ies'} from {ci_result.path}", file=out, # type: ignore[arg-type] ) - if getattr(args, "search_tool", False): + if getattr(args, "search_tool", True): st_result = uninstall_search_tool_hook( path, command_basename=SEARCH_TOOL_SCRIPT_NAME, ) @@ -2776,7 +2763,7 @@ def _cmd_unsetup(args: argparse.Namespace, out: object) -> int: f"{'y' if st_result.removed == 1 else 'ies'} from {st_result.path}", file=out, # type: ignore[arg-type] ) - if getattr(args, "search_tool_bash", False): + if getattr(args, "search_tool_bash", True): stb_result = uninstall_search_tool_bash_hook( path, command_basename=SEARCH_TOOL_BASH_SCRIPT_NAME, ) @@ -5550,38 +5537,29 @@ def build_parser(*, show_advanced: bool = False) -> argparse.ArgumentParser: ), ) p_setup.add_argument( - "--search-tool", dest="search_tool", action="store_true", + "--search-tool", dest="search_tool", + action=argparse.BooleanOptionalAction, default=True, help=( - "additionally wire the PreToolUse:Grep|Glob hook so the agent's " - "own search queries first run against the per-project belief " - "store and the results are injected as additionalContext. " - "If memory has the answer the agent can skip / refine the tool " - "call; if not, the tool result fills the gap. See " - "docs/search_tool_hook.md." + "wire the PreToolUse:Grep|Glob hook so the agent's own search " + "queries first run against the per-project belief store and the " + "results are injected as additionalContext. If memory has the " + "answer the agent can skip / refine the tool call; if not, the " + "tool result fills the gap. Default: ON. Pass --no-search-tool to " + "skip. See docs/search_tool_hook.md." ), ) - _stb_group = p_setup.add_mutually_exclusive_group() - _stb_group.add_argument( - "--search-tool-bash", dest="search_tool_bash", action="store_true", - default=False, + p_setup.add_argument( + "--search-tool-bash", dest="search_tool_bash", + action=argparse.BooleanOptionalAction, default=True, help=( - "additionally wire the PreToolUse:Bash hook so shell search " - "commands (grep, rg, find, fd, ack) run against the per-project " - "belief store before firing. Independent of --search-tool; " - "either, both, or neither may be installed. Default-OFF at " - "v1.5.0; default-on flip is gated on telemetry. See " + "wire the PreToolUse:Bash hook so shell search commands " + "(grep, rg, find, fd, ack) run against the per-project belief " + "store before firing. Independent of --search-tool; either, " + "both, or neither may be installed. Default: ON. Pass " + "--no-search-tool-bash to skip. See " "docs/search_tool_hook.md ยง Bash extension." ), ) - _stb_group.add_argument( - "--no-search-tool-bash", dest="no_search_tool_bash", action="store_true", - default=False, - help=( - "remove the PreToolUse:Bash search-tool-bash hook if present. " - "Idempotent (no-op when the hook is not installed). " - "Independent of --search-tool / --no-search-tool." - ), - ) p_setup.set_defaults(func=_cmd_setup) # Hidden: install lifecycle, surfaced by docs not by --help. @@ -5719,12 +5697,20 @@ def build_parser(*, show_advanced: bool = False) -> argparse.ArgumentParser: ), ) p_unsetup.add_argument( - "--search-tool", dest="search_tool", action="store_true", - help="also remove the PreToolUse:Grep|Glob search-tool hook entry.", + "--search-tool", dest="search_tool", + action=argparse.BooleanOptionalAction, default=True, + help=( + "remove the PreToolUse:Grep|Glob search-tool hook entry. " + "Default: ON. Pass --no-search-tool to leave it in place." + ), ) p_unsetup.add_argument( - "--search-tool-bash", dest="search_tool_bash", action="store_true", - help="also remove the PreToolUse:Bash search-tool-bash hook entry.", + "--search-tool-bash", dest="search_tool_bash", + action=argparse.BooleanOptionalAction, default=True, + help=( + "remove the PreToolUse:Bash search-tool-bash hook entry. " + "Default: ON. Pass --no-search-tool-bash to leave it in place." + ), ) p_unsetup.set_defaults(func=_cmd_unsetup) diff --git a/src/aelfrice/data/hook_manifest.json b/src/aelfrice/data/hook_manifest.json index 33e059139..bb2704fe0 100644 --- a/src/aelfrice/data/hook_manifest.json +++ b/src/aelfrice/data/hook_manifest.json @@ -40,6 +40,22 @@ "default_on": true, "since": "2.1.0", "description": "Prompt to lock session corrections at turn end (#582)." + }, + { + "name": "search_tool", + "basename": "aelf-search-tool-hook", + "installer": "search_tool", + "default_on": true, + "since": "3.0.1", + "description": "PreToolUse:Grep|Glob memory-first search before agent's own tool fires (#738)." + }, + { + "name": "search_tool_bash", + "basename": "aelf-search-tool-hook", + "installer": "search_tool_bash", + "default_on": true, + "since": "3.0.1", + "description": "PreToolUse:Bash memory-first check on grep/rg/find/fd/ack shell commands (#738)." } ] } diff --git a/tests/test_aelf_setup_search_tool_bash.py b/tests/test_aelf_setup_search_tool_bash.py index 34cd6a4a6..9a761a763 100644 --- a/tests/test_aelf_setup_search_tool_bash.py +++ b/tests/test_aelf_setup_search_tool_bash.py @@ -1,16 +1,20 @@ -"""Tests for `aelf setup --search-tool-bash` and -`aelf setup --no-search-tool-bash` (#155 AC7). - -Coverage: -- Install writes a PreToolUse:Bash hook entry. Idempotent (second call - is a no-op that prints "already installed"). -- Uninstall (--no-search-tool-bash) removes it. Idempotent (no-op when - absent). -- install + uninstall + install round-trip. -- Independent of --search-tool: can install both, either, or neither. -- aelf unsetup --search-tool-bash also removes the entry. -- install_search_tool_bash_hook / uninstall_search_tool_bash_hook - low-level unit tests matching the commit-ingest + search-tool pattern. +"""Tests for `aelf setup` search-tool / search-tool-bash flag behavior. + +History: #155 AC7 introduced these as opt-in flags. #738 (v3.0.1) flipped +them to default-on under the `--X / --no-X` BooleanOptionalAction +convention used by every other default-on hook. + +Current contract (post #738): +- Bare `aelf setup` wires both PreToolUse hooks. +- `--no-search-tool` / `--no-search-tool-bash` skip install AND persist + the opt-out at `~/.aelfrice/opt-out-hooks.json` so the next + auto-install reconcile does not re-add the hook. +- Setup-time --no-X does NOT actively uninstall an existing entry; for + that, use `aelf unsetup` (default-on for both flags). +- Both can be opted out together to reproduce the pre-3.0.1 no-PreToolUse + default. +- Low-level install_search_tool_bash_hook / uninstall_search_tool_bash_hook + unit tests match the commit-ingest + search-tool pattern. """ from __future__ import annotations @@ -182,21 +186,25 @@ def test_cli_setup_search_tool_bash_idempotent(tmp_path: Path) -> None: assert len(_bash_entries(_settings(p))) == 1 -def test_cli_setup_no_search_tool_bash_removes(tmp_path: Path) -> None: +def test_cli_setup_no_search_tool_bash_does_not_remove_existing(tmp_path: Path) -> None: + """Per #738, setup-time --no-X skips install but does not uninstall. + + Use `aelf unsetup` for explicit removal. + """ p = tmp_path / "settings.json" _run_setup(p, "--search-tool-bash") assert len(_bash_entries(_settings(p))) == 1 - rc, out = _run_setup(p, "--no-search-tool-bash") + rc, _ = _run_setup(p, "--no-search-tool-bash") assert rc == 0 - assert "removed" in out - assert _bash_entries(_settings(p)) == [] + assert len(_bash_entries(_settings(p))) == 1 -def test_cli_setup_no_search_tool_bash_idempotent_when_absent(tmp_path: Path) -> None: +def test_cli_setup_no_search_tool_bash_skips_install(tmp_path: Path) -> None: + """`aelf setup --no-search-tool-bash` on a fresh settings.json leaves bash unset.""" p = tmp_path / "settings.json" - rc, out = _run_setup(p, "--no-search-tool-bash") + rc, _ = _run_setup(p, "--no-search-tool-bash") assert rc == 0 - assert "no search-tool-bash" in out + assert _bash_entries(_settings(p)) == [] def test_cli_setup_search_tool_bash_independent_of_search_tool( @@ -210,12 +218,11 @@ def test_cli_setup_search_tool_bash_independent_of_search_tool( assert len(_bash_entries(data)) == 1 -def test_cli_setup_no_search_tool_bash_leaves_grep_glob_intact( +def test_cli_setup_no_search_tool_bash_installs_grep_glob_only( tmp_path: Path, ) -> None: + """Bare setup + --no-search-tool-bash wires Grep|Glob but not Bash.""" p = tmp_path / "settings.json" - _run_setup(p, "--search-tool") - _run_setup(p, "--search-tool-bash") _run_setup(p, "--no-search-tool-bash") data = _settings(p) assert len(_grep_glob_entries(data)) == 1 @@ -241,3 +248,63 @@ def test_cli_unsetup_search_tool_bash_idempotent(tmp_path: Path) -> None: rc, out = _run_unsetup(p, "--search-tool-bash") assert rc == 0 assert "no search-tool-bash" in out + + +# --------------------------------------------------------------------------- +# CLI: default-on flip (#738) +# --------------------------------------------------------------------------- + + +def test_cli_setup_bare_installs_both_search_tool_hooks(tmp_path: Path) -> None: + """Per #738: bare `aelf setup` (no flags) wires both PreToolUse hooks.""" + p = tmp_path / "settings.json" + rc, _ = _run_setup(p) + assert rc == 0 + data = _settings(p) + assert len(_grep_glob_entries(data)) == 1 + assert len(_bash_entries(data)) == 1 + + +def test_cli_setup_both_no_flags_reproduces_pre_3_0_1_default(tmp_path: Path) -> None: + """`aelf setup --no-search-tool --no-search-tool-bash` wires neither hook.""" + p = tmp_path / "settings.json" + rc, _ = _run_setup(p, "--no-search-tool", "--no-search-tool-bash") + assert rc == 0 + data = _settings(p) + assert _grep_glob_entries(data) == [] + assert _bash_entries(data) == [] + + +def test_cli_setup_no_search_tool_installs_bash_only(tmp_path: Path) -> None: + """`aelf setup --no-search-tool` wires search-tool-bash only.""" + p = tmp_path / "settings.json" + rc, _ = _run_setup(p, "--no-search-tool") + assert rc == 0 + data = _settings(p) + assert _grep_glob_entries(data) == [] + assert len(_bash_entries(data)) == 1 + + +def test_cli_unsetup_bare_removes_both_search_tool_hooks(tmp_path: Path) -> None: + """Per #738: bare `aelf unsetup` (no flags) removes both PreToolUse hooks.""" + p = tmp_path / "settings.json" + _run_setup(p) # bare install -> both present + data = _settings(p) + assert len(_grep_glob_entries(data)) == 1 + assert len(_bash_entries(data)) == 1 + rc, _ = _run_unsetup(p) + assert rc == 0 + data = _settings(p) + assert _grep_glob_entries(data) == [] + assert _bash_entries(data) == [] + + +def test_cli_unsetup_no_search_tool_leaves_grep_glob(tmp_path: Path) -> None: + """`aelf unsetup --no-search-tool` leaves the Grep|Glob hook in place.""" + p = tmp_path / "settings.json" + _run_setup(p) + rc, _ = _run_unsetup(p, "--no-search-tool") + assert rc == 0 + data = _settings(p) + assert len(_grep_glob_entries(data)) == 1 + assert _bash_entries(data) == [] diff --git a/tests/test_cli_setup_opt_out_sync.py b/tests/test_cli_setup_opt_out_sync.py index 83614af0d..67fcb10a0 100644 --- a/tests/test_cli_setup_opt_out_sync.py +++ b/tests/test_cli_setup_opt_out_sync.py @@ -87,3 +87,52 @@ def test_setup_no_stop_hook_uses_correct_manifest_name( opt_outs = auto_install.read_opt_outs(opt_out) assert "stop_lock_prompt" in opt_outs assert "stop_hook" not in opt_outs + + +def test_setup_no_search_tool_adds_opt_out( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, +) -> None: + """`aelf setup --no-search-tool` persists the opt-out (#738).""" + _, opt_out = _patch_paths(monkeypatch, tmp_path) + settings = tmp_path / "settings.json" + monkeypatch.setenv("AELF_NO_UPDATE_CHECK", "1") + monkeypatch.setenv("HOME", str(tmp_path)) + cli.main([ + "setup", "--settings", str(settings), "--scope", "project", + "--no-statusline", "--no-search-tool", + ]) + assert "search_tool" in auto_install.read_opt_outs(opt_out) + + +def test_setup_no_search_tool_bash_adds_opt_out( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, +) -> None: + """`aelf setup --no-search-tool-bash` persists the opt-out (#738).""" + _, opt_out = _patch_paths(monkeypatch, tmp_path) + settings = tmp_path / "settings.json" + monkeypatch.setenv("AELF_NO_UPDATE_CHECK", "1") + monkeypatch.setenv("HOME", str(tmp_path)) + cli.main([ + "setup", "--settings", str(settings), "--scope", "project", + "--no-statusline", "--no-search-tool-bash", + ]) + assert "search_tool_bash" in auto_install.read_opt_outs(opt_out) + + +def test_setup_bare_rescinds_search_tool_opt_outs( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, +) -> None: + """Bare `aelf setup` after a prior --no-search-tool drops the opt-out (#738).""" + _, opt_out = _patch_paths(monkeypatch, tmp_path) + settings = tmp_path / "settings.json" + monkeypatch.setenv("AELF_NO_UPDATE_CHECK", "1") + monkeypatch.setenv("HOME", str(tmp_path)) + auto_install.add_opt_out("search_tool", opt_out) + auto_install.add_opt_out("search_tool_bash", opt_out) + cli.main([ + "setup", "--settings", str(settings), "--scope", "project", + "--no-statusline", + ]) + opt_outs = auto_install.read_opt_outs(opt_out) + assert "search_tool" not in opt_outs + assert "search_tool_bash" not in opt_outs