feat(#489): fire the MCP-search advisory on Read/Glob/Grep for workspace paths#490
Merged
atlas-apex merged 1 commit intoJun 3, 2026
Conversation
…ace paths Extends suggest-mcp-search.sh to also fire on Read, Glob, and Grep PreToolUse events when the target path resolves inside a managed-project workspace clone (workspace/<project>/). Previously an agent could sidestep the nudge by using native read tools instead of Bash; this closes that gap. - Hook tool gate: case-branch on Bash (original path) vs Read|Glob|Grep (new path); any other tool exits 0 immediately. - Path extraction for Read/Glob/Grep: .tool_input.file_path // .tool_input.path covers all three tools in one jq query. - Workspace guard: new branch exits 0 when the path is NOT under workspace/. - Install-gate fully preserved: the apexyard-search .mcp.json check applies to ALL tools — free adopters without the premium MCP see nothing. - settings.json: new Read|Glob|Grep PreToolUse matcher block wired to the same hook wrapper as Bash. - Tests: 23 cases (up from 9) — Read/Glob/Grep fire on workspace paths (gate open), stay silent outside workspace/, stay silent with gate closed (the load-bearing free-adopter case), Write/Edit not triggered. Closes #489 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
atlas-apex
commented
Jun 3, 2026
atlas-apex
left a comment
Collaborator
Author
There was a problem hiding this comment.
Code Review: PR #490
Commit: 8255f5bf72ef63dac1c7ea6ab22e1adfa8488714
Note: submitted as a comment because GitHub blocks self-approval on this PR. The Rex verdict is APPROVED and the
490-rex.approvedmarker has been written with the matching HEAD SHA.
Summary
Extends the suggest-mcp-search advisory hook from Bash-only to also fire on Read/Glob/Grep when the target path is inside a managed-project workspace clone (workspace/<project>/), closing the bypass where an agent could sidestep the MCP-first nudge by using native read tools. Bash behaviour is unchanged; the install-gate (only nudge when apexyard-search is in .mcp.json) is preserved for all tools.
Checklist Results
- ✅ Architecture & Design: Pass — clean
case/ifbranch dispatch, shared install-gate at the convergence point - ✅ Code Quality: Pass — shellcheck -S warning clean, well-commented field-layout rationale
- ✅ Testing: Pass — 23/23 cases, incl. 3 load-bearing free-adopter no-banner cases
- ✅ Security: Pass — advisory-only, no secrets, no injection surface
- ✅ Performance: Pass — hot-path is a single jq + case match
- ✅ PR Description & Glossary: Pass — narrative Summary calls out the free-adopter gate; Glossary present; Closes #489
- ⚠ Summary Bullet Narrative: Pass — bullets answer what + why
- ✅ Technical Decisions (AgDR):N/A — extends an existing hook, no new dependency/pattern
- ✅ Adopter Handbooks: N/A — no handbooks triggered by this diff
Issues Found
None.
Verification performed (load-bearing checks)
- Install-gate preserved (the explicit CEO requirement). Read the hook end-to-end: the Read/Glob/Grep branch (lines 104-125) only does path filtering, then falls through to the shared install-gate
$mcp_has_search || exit 0at line 145 — it is NOT bypassed. Confirmed adversarially:- Read on
workspace/...withapexyard-searchABSENT from.mcp.json→ SILENT (free adopter sees nothing). ✅ - Read on
workspace/...withapexyard-searchPRESENT → banner fires. ✅
- Read on
- Path extraction.
.tool_input.file_path // .tool_input.path // emptycorrectly covers Read (file_path) and Glob/Grep (path); absent path → exit 0 (verified, no banner). Workspace check (*workspace/*) mirrors the Bash branch intent. - Bash branch byte-identical in effect —
grep -rnon a workspace path still fires; original framework/workspace pattern scanner untouched. - Advisory/non-blocking — exit 0 in every path; banner emitted as
hookSpecificOutput.additionalContextJSON on STDOUT, matching the #469 channel. - settings.json — valid JSON; new
Read|Glob|Grepmatcher uses the identical ops-root-resolvingbash -cwrapper; existing Bash/Edit-Write matchers undisturbed. - Tests + lint —
test_suggest_mcp_search.sh23 passed / 0 failed;test_site_counts.shgreen (hooks count unchanged at 38 — edited existing file);shellcheck -S warningexit 0.
Suggestions
None blocking. Optional future thought: if Claude Code ever adds a tool that carries a path under a third field name, the file_path // path pattern would need extending — the inline comment already documents the schema, so this is covered for now.
Verdict
APPROVED
🤖 Reviewed by Rex (Code Reviewer Agent)
📌 Reviewed commit: 8255f5bf72ef63dac1c7ea6ab22e1adfa8488714
me2resh
added a commit
that referenced
this pull request
Jun 5, 2026
…ace paths (#490) Extends suggest-mcp-search.sh to also fire on Read, Glob, and Grep PreToolUse events when the target path resolves inside a managed-project workspace clone (workspace/<project>/). Previously an agent could sidestep the nudge by using native read tools instead of Bash; this closes that gap. - Hook tool gate: case-branch on Bash (original path) vs Read|Glob|Grep (new path); any other tool exits 0 immediately. - Path extraction for Read/Glob/Grep: .tool_input.file_path // .tool_input.path covers all three tools in one jq query. - Workspace guard: new branch exits 0 when the path is NOT under workspace/. - Install-gate fully preserved: the apexyard-search .mcp.json check applies to ALL tools — free adopters without the premium MCP see nothing. - settings.json: new Read|Glob|Grep PreToolUse matcher block wired to the same hook wrapper as Bash. - Tests: 23 cases (up from 9) — Read/Glob/Grep fire on workspace paths (gate open), stay silent outside workspace/, stay silent with gate closed (the load-bearing free-adopter case), Write/Edit not triggered. Closes #489 Co-authored-by: me2resh <ahmed.abdelaliem@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
suggest-mcp-searchadvisory previously only fired onBashtool calls. An agent could sidestep the nudge entirely by reaching forRead,Glob, orGrepagainst a workspace-clone path, which never triggered the banner. This PR extends the hook to cover all three tools when the target path is insideworkspace/<project>/.workspace/. Framework file reads (e.g.Read roles/engineering/tech-lead.md) are untouched and stay silent, preserving the original behaviour's intent that the nudge only applies to managed-project code lookups.apexyard-searchcheck in.mcp.jsonapplies equally to the new Read/Glob/Grep branch. Free adopters who don't have the premiumapexyard-searchMCP configured will see nothing from this hook regardless of which tool they use. This was an explicit requirement and is verified by three dedicated test cases (one per tool in the no-MCP fixture).Read|Glob|GrepPreToolUse matcher block invokes the same ops-root-resolving hook wrapper already used by the Bash entry.Testing
Test coverage added (#489 cases):
workspace/<project>/...path WITH apexyard-search configured → advisory firesworkspace/<project>/...path WITHOUT apexyard-search (free adopter) → silentworkspace/(framework files,src/, etc.) → silentGlossary
PreToolUsehook that emits guidance ashookSpecificOutput.additionalContextJSON on stdout and always exits 0 — never blocks the tool call.mcp.jsoncheck that ensures the hook stays silent for free adopters who don't have theapexyard-searchpremium MCP configuredworkspace/<name>/in the ops repoapexyard-searchsemantic search overgrep/Readfor managed-project code lookups (~3–5× fewer tokens)Closes #489