feat(procedures): gate how-do-i on mutation + field-anchored mistakes.jsonl recall - #39
Merged
Merged
Conversation
The deny message promises read-only inspection stays available; the allowlist did not deliver it. A Read was allowed only under references/procedures/, and a Bash command had to BOTH match a small shape set AND name the discovery surface — so pure reads like `tmux capture-pane | grep | tail`, `wc -l < f` and a Read of a task's own output file were denied. Replace the path co-requirement with a command classifier (hooks/lib/readonly-shape.sh) that judges the whole line: separators are split and every segment must be a known read-only invocation, while command substitution, process substitution and output redirection to a file are refused outright. Unrecognised shapes fail closed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 52 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds field-anchored recall search for ChangesSemantic Recall Search
Read-Only Gate Enforcement
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Scout
participant QueryRecords
participant RecallMatcher
participant MistakesStore
Scout->>QueryRecords: --recall query
QueryRecords->>RecallMatcher: normalized searchable tokens
RecallMatcher->>MistakesStore: scan semantic JSON fields
MistakesStore-->>RecallMatcher: matching records
RecallMatcher-->>QueryRecords: matched records
QueryRecords-->>Scout: capped recent results and total
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
drewdrewthis
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/procedures/agents/procedure-scout.md`:
- Around line 95-98: The procedure-scout instructions currently hard-code
record-store paths instead of discovering all readable stores. Update the
guidance around the `query-records.sh` usage to run `--list-stores` first, use
its output as the readable store boundary, and select scans from those stores
rather than enumerating `references/solutions/` or other paths.
- Around line 107-108: Remove the two non-operational recap lines from the
procedure guidance near the recall discussion; retain only prose that gives the
agent an actionable instruction, boundary, or failure mode.
In `@plugins/procedures/hooks/lib/gate-allowlist.sh`:
- Around line 48-62: Update the jq extraction in the Read|NotebookRead|Grep|Glob
branch to include Glob’s pattern and Grep’s glob fields in the traversal check,
preserving the existing refusal for paths containing "..". Add a Bats test under
hooks/tests/ covering a traversal-only Glob pattern and keep the existing
pattern-only allow behavior for non-traversing patterns.
- Line 37: Update the path resolution around the readonly-shape.sh source in
gate-allowlist.sh to handle BASH_SOURCE[0] values without a slash, matching the
existing guard in how-do-i-gate.sh. Ensure bare-name sourcing resolves the
current directory correctly before sourcing readonly-shape.sh, while preserving
path-based sourcing behavior.
In `@plugins/procedures/hooks/lib/readonly-shape.sh`:
- Around line 108-116: Restrict the unconditional allowlist in the readonly
command classifier so write-capable invocations cannot return 0: remove or gate
sort, uniq, xxd, yq, ip, and date, preserving only read-only forms such as sort
without output, yq without in-place editing, ip show/list, and date without
setting. Add Bats coverage under hooks/tests/ for each newly denied write form.
- Around line 50-82: Update ros__git_is_read_only to reject external-program and
helper-enabling options before allowing read-only subcommands: deny grep’s
-O/--open-files-in-pager forms, diff’s --ext-diff and --config-env* forms, and
attached -c<name>=<value> forms while preserving fail-closed handling for
separate -c arguments. Add Bats coverage for each rejected option and ensure
ordinary read-only commands remain allowed.
In `@plugins/procedures/scripts/query-records.sh`:
- Around line 181-235: Remove the local recall-mode implementation from
plugins/procedures/scripts/query-records.sh lines 181-235 and remove the
plugin-local matcher from plugins/procedures/scripts/lib/recall-match.awk lines
1-60 until the upstream vendor includes them. Update
plugins/procedures/agents/procedure-scout.md lines 95-113 to match the
upstream-vendored behavior, keeping all three files byte-close to upstream.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 52499c0d-2782-41d1-8268-bce3c17cb974
📒 Files selected for processing (9)
plugins/procedures/agents/procedure-scout.mdplugins/procedures/hooks/how-do-i-gate.shplugins/procedures/hooks/lib/gate-allowlist.shplugins/procedures/hooks/lib/readonly-shape.shplugins/procedures/hooks/tests/gate-failopen.batsplugins/procedures/hooks/tests/gate-libs.batsplugins/procedures/hooks/tests/recall.batsplugins/procedures/scripts/lib/recall-match.awkplugins/procedures/scripts/query-records.sh
…y-records.sh Closes #41. scripts/lib/recall-match.awk: whole-word, phrase-aware matching over semantic field values only (never paths/URLs/session ids), tolerant of both JSONL spacing styles, field-boundary-safe. --recall mode: loud count line, 20-most-recent default cap, --limit override, exit 2/3 fail-loud contract. procedure-scout step 4 swapped off raw grep. CodeRabbit: store boundary via --list-stores, recap prose dropped, gate-allowlist BASH_SOURCE bare-name guard. hooks/tests/recall.bats: 16 tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
drewdrewthis
force-pushed
the
fix/how-do-i-gate-look-vs-act
branch
from
August 11, 2026 11:41
86918d4 to
12fb5f3
Compare
- gate-allowlist: per-tool traversal fields (Grep path+glob, Glob path+pattern); Grep regex pattern exempt (.. is any-two-chars) - readonly-shape: reject git options that hand execution to an external program (-O/--open-files-in-pager, --ext-diff, --config-env) - readonly-shape: drop ip from the unconditional allowlist; gate the write modes of sort/-o, yq/-i, date/-s, uniq/xxd second operand - PLUGIN ADAPTATION markers on query-records.sh recall + recall-match.awk - bats coverage for every newly gated shape (203 green)
Owner
Author
|
@coderabbitai full review |
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
plugins/procedures/hooks/lib/readonly-shape.sh (1)
123-127: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winReject execution and mutation options in the remaining reader allowlist.
rg --pre /path/to/programexecutes the preprocessor.tree -o outputwrites a file.ss -K ...closes sockets. GNUsort --compress-program=/path/to/programinvokes a helper when it spills temporary files. Each command currently reachesreturn 0, so a Bash payload can act while the gate classifies it as read-only.
plugins/procedures/hooks/lib/readonly-shape.sh#L123-L127: Removerg,tree, andssfrom the unconditional branch, or reject--pre,--pre=*,-o*,-K, and--kill.plugins/procedures/hooks/lib/readonly-shape.sh#L139-L143: Reject--compress-programand--compress-program=*.- Add Bats cases for every denied form.
#!/bin/bash set -euo pipefail for tool in rg sort tree ss; do if command -v "$tool" >/dev/null 2>&1; then printf '\n--- %s help ---\n' "$tool" "$tool" --help 2>&1 | rg -n -- '--pre|compress-program|output|-o|--kill|-K' || true fi done printf '\n--- classifier branches ---\n' sed -n '119,158p' plugins/procedures/hooks/lib/readonly-shape.shAs per path instructions: “Add Bats coverage for every new behavior.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/procedures/hooks/lib/readonly-shape.sh` around lines 123 - 127, The readonly command classifier must reject execution and mutation options instead of allowing them unconditionally. In plugins/procedures/hooks/lib/readonly-shape.sh lines 123-127, update the rg, tree, and ss handling to deny --pre/--pre=*, -o*, -K, and --kill (or remove those tools from the unconditional branch); in lines 139-143, reject sort’s --compress-program and --compress-program=* options. Add Bats coverage for every denied form.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/procedures/agents/procedure-scout.md`:
- Around line 97-109: Update the recall instructions in the how-do-i skill text
to use query-records.sh with the --recall option and describe its semantic
whole-word matching and batch-read flow. Remove the stale claim that recall is
unsupported and that mistakes.jsonl should be grepped directly, while preserving
the explicit prohibition against raw grep fallback.
In `@plugins/procedures/hooks/tests/recall.bats`:
- Around line 15-20: Expand the recall fixture records in the heredoc used by
the recall tests so positive cases contain unique search terms in every
supported semantic field: pattern, correction, face, category, skill, summary,
what, and fix, in addition to description. Update or add corresponding
assertions to verify each term matches only through its intended field,
preserving the existing field-anchoring coverage.
- Around line 1-5: Add a literal “# PLUGIN ADAPTATION: <why>” comment near the
header of the Bats test file, explaining why this plugin-local recall behavior
diverges from upstream; keep the existing test content unchanged.
---
Duplicate comments:
In `@plugins/procedures/hooks/lib/readonly-shape.sh`:
- Around line 123-127: The readonly command classifier must reject execution and
mutation options instead of allowing them unconditionally. In
plugins/procedures/hooks/lib/readonly-shape.sh lines 123-127, update the rg,
tree, and ss handling to deny --pre/--pre=*, -o*, -K, and --kill (or remove
those tools from the unconditional branch); in lines 139-143, reject sort’s
--compress-program and --compress-program=* options. Add Bats coverage for every
denied form.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 17c45361-4241-4038-8225-d250085e3941
📒 Files selected for processing (7)
plugins/procedures/agents/procedure-scout.mdplugins/procedures/hooks/lib/gate-allowlist.shplugins/procedures/hooks/lib/readonly-shape.shplugins/procedures/hooks/tests/gate-libs.batsplugins/procedures/hooks/tests/recall.batsplugins/procedures/scripts/lib/recall-match.awkplugins/procedures/scripts/query-records.sh
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.
Combined PR, two changes that together close out the open gate/recall friction set (per Drew: drive these in one PR).
1. Gate how-do-i on mutation, not the discovery path
(unchanged from the original PR — see commit b83c5b1)
Fixes the gate blocking read-only verification and discovery:
2. Field-anchored recall over mistakes.jsonl (closes #41)
query-records.shgains a--recall '<term set>'mode backed byscripts/lib/recall-match.awk:pattern description correction face category skill summary what fix) — a term inside a path, URL, or session id cannot hit (the noise defect of the rawgrep -iprocedure-scout step 4 prescribed)pickup-loopmatches the phrase across any separator, never bareloop; common words no longer explode the match setrecall: N matched — showing the 20 most recent),--limitoverride, never a silent cap"k":"v"and"k": "v"record styles (the live file mixes them)Verified against the live 504KB/481-line store: exact hit-parity with the old grep on its own term set (5/5, zero lost hits), noise cases eliminated, 0.1–0.3s wall.
Tests:
bats hooks/tests— 193/193 green, including newhooks/tests/recall.bats(11 tests).Title is
feat(procedures):so release-please cuts a minor bump (covers the fix too).Related: #34 (query-records as sole retrieval surface — this extends that surface to recall).
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests