feat(toolkit): verify-branch-state skill + /verify + warn-mode hook (DOJ-4872) — v1.31.0#44
Open
lapc506 wants to merge 6 commits into
Open
Conversation
…e hook (DOJ-4872) — v1.31.0
Ships the plugin-layer (Level 1) extension of the working-tree-vs-ref
discipline rule. DOJ-4868 shipped the repo-layer (Level 2) enforcement in
dojo-os; this release makes the same rule travel with the toolkit to every
repo it is installed in.
New artifacts:
- skills/verify-branch-state/SKILL.md — auto-loading skill, tuned description
triggers on "verify against develop", "does X exist on <branch>", "what's
on origin/...", "did this land on main", "re-validate the subagent". Body
covers the rule, canonical ref-explicit commands (git fetch + git show /
ls-tree / grep / diff against origin/<ref>), anti-patterns (ls, grep -rn
src/, find, git status), pre-flight checklist, multi-worktree context, and
the 2026-06-03 DOJ-4851/4863/4864 origin incident. Cites memory
feedback_working_tree_is_not_truth.
- commands/verify.md — slash command /verify <ref> <path-or-pattern>. Runs
git fetch origin <ref> --quiet then git ls-tree origin/<ref> -- <path>
for paths or git grep <pattern> origin/<ref> -- src/ for patterns. Emits
a single verdict line citing the ref's short SHA in the shape
"origin/<ref> @ <sha7> — <path-or-pattern> <PRESENT|NOT PRESENT|N MATCHES>"
so the claim can be re-checked. Handles ref-not-found, shell
metacharacters in patterns, and squash-merge cache staleness (always
re-fetches).
- hooks/pre-tool-use-claim-verification.sh — warn-mode PreToolUse hook,
repo-agnostic, no per-repo manifest. Detects bare-FS verification ops
combined with intent keywords; prints stderr warning with local HEAD ref
+ SHA, a suggested ref-explicit replacement, and pointers to the skill +
slash command. Always exit 0. Defensive set +e. Uses cd -P + pwd -P from
day 1 (DOJ-4868 PR #2604 lesson). Trailing-newline-safe while read.
Honors CLAUDE_DISABLE_PLUGIN_HOOKS=1. Shellcheck clean. Wired into
hooks/hooks.json under the Bash PreToolUse matcher with a 3s timeout.
Version bump 1.30.0 -> 1.31.0 across .claude-plugin/marketplace.json
(top-level + plugin version) + .claude-plugin/plugin.json + package.json +
README.md Version header. CHANGELOG entry under [1.31.0] - 2026-06-03 with
Added / Changed / Why sections + reference link at bottom. README "Start
here" grows a peer entry point ("Also start here: verify-branch-state +
/verify") between the domain-driven-advisor section and "What's Inside".
Commands table 29 -> 30 (adds /verify); Skills table 10 -> 11 (adds
verify-branch-state). Marketplace plugin description pair-mentions the new
skill.
Smoke tests pass:
- bash hooks/pre-tool-use-claim-verification.sh "ls src/foo.tsx" -> warn
- bash hooks/pre-tool-use-claim-verification.sh "echo hello" -> silent
- shellcheck hooks/pre-tool-use-claim-verification.sh -> 0 issues
- jq -r .version on all four manifests -> 1.31.0 (agree)
AC9 note (cohesive-PR exception): 9 files instead of the soft ≤8 target
because the hooks.json registration is required to make the new hook
non-dead. The new skill + command + hook are a single cohesive surface
(working-tree-vs-ref discipline) and the version+CHANGELOG+README updates
are the standard release-cut shape, mirroring the v1.30.0 commit shape.
Related: DOJ-4868 (dojo-os Level-2 sibling), v1.30.0 release (today),
memory feedback_working_tree_is_not_truth.
Created by Claude Code on behalf of @lapc506.
Collaborator
Author
|
@dojo-code-reviewer review |
![dojo-code-reviewer[bot]](https://avatars.githubusercontent.com/in/3092887?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Approved
Approved — 0 blockers, 2 P2, 2 P3. Confidence: 3/5.
🟡 P2 — Major
-
commands/verify.md:42— 🟡 P2 (major) — The slash command currently assumes the user always passes a branch name that needs to be prefixed with 'origin/'. However, if a user passes a remote-tracking branch like 'origin/develop', a tag like 'v1.31.0', or a commit SHA, this leads to failures like 'origin/origin/develop' or 'origin/v1.31.0' during fetch, rev-parse, and ls-tree/grep. Additionally, the hardcoded '-C "$REPO"' directory switches can fail if the environment hasn't defined the '$REPO' variable. Normalize the reference explicitly first to handle all formats seamlessly, and execute directly within the current workspace. -
hooks/pre-tool-use-claim-verification.sh:116— 🟡 P2 (major) — The regex matches bare-filesystem operations followed by a space, which means it will fail to detect verification commands that appear at the end of a line or command string without a trailing space (for example, 'ls' alone or the end of a chain like 'cd src && ls'). Use '([[:space:]]|$)' to ensure the hook reliably triggers in these scenarios.
if printf '%s' "$cmd" | grep -qE '(^|[;&|][[:space:]]*)(ls|find|cat|head)([[:space:]]|$)'; then
🔵 P3 — Minor
hooks/pre-tool-use-claim-verification.sh:162— 🔵 P3 (minor) — To ensure that the suggested replacement helper is chosen correctly when 'ls' is used at the end of a command string without trailing space, replace 'ls[[:space:]]' with 'ls([[:space:]]|$)'.
if printf '%s' "$COMMAND" | grep -qE '(^|[;&|][[:space:]]*)ls([[:space:]]|$)'; then
hooks/pre-tool-use-claim-verification.sh:181— 🔵 P3 (minor) — To ensure that the suggested replacement helper is chosen correctly when 'find' is used at the end of a command string without trailing space, replace 'find[[:space:]]' with 'find([[:space:]]|$)'.
elif printf '%s' "$COMMAND" | grep -qE '(^|[;&|][[:space:]]*)find([[:space:]]|$)'; then
Total findings: 2 compliance, 2 business context (4 total)
ℹ️ Inline anchoring failed for the comments below; they are listed here instead.
commands/verify.md:42— 🟡 P2 (major) — The slash command currently assumes the user always passes a branch name that needs to be prefixed with 'origin/'. However, if a user passes a remote-tracking branch like 'origin/develop', a tag like 'v1.31.0', or a commit SHA, this leads to failures like 'origin/origin/develop' or 'origin/v1.31.0' during fetch, rev-parse, and ls-tree/grep. Additionally, the hardcoded '-C "$REPO"' directory switches can fail if the environment hasn't defined the '$REPO' variable. Normalize the reference explicitly first to handle all formats seamlessly, and execute directly within the current workspace.hooks/pre-tool-use-claim-verification.sh:116— 🟡 P2 (major) — The regex matches bare-filesystem operations followed by a space, which means it will fail to detect verification commands that appear at the end of a line or command string without a trailing space (for example, 'ls' alone or the end of a chain like 'cd src && ls'). Use '([[:space:]]|$)' to ensure the hook reliably triggers in these scenarios.
if printf '%s' "$cmd" | grep -qE '(^|[;&|][[:space:]]*)(ls|find|cat|head)([[:space:]]|$)'; then
hooks/pre-tool-use-claim-verification.sh:162— 🔵 P3 (minor) — To ensure that the suggested replacement helper is chosen correctly when 'ls' is used at the end of a command string without trailing space, replace 'ls[[:space:]]' with 'ls([[:space:]]|$)'.
if printf '%s' "$COMMAND" | grep -qE '(^|[;&|][[:space:]]*)ls([[:space:]]|$)'; then
hooks/pre-tool-use-claim-verification.sh:181— 🔵 P3 (minor) — To ensure that the suggested replacement helper is chosen correctly when 'find' is used at the end of a command string without trailing space, replace 'find[[:space:]]' with 'find([[:space:]]|$)'.
elif printf '%s' "$COMMAND" | grep -qE '(^|[;&|][[:space:]]*)find([[:space:]]|$)'; then
Bot review at 3/5 confidence flagged 2 P2 + 2 P3. All four addressed:
P2 — commands/verify.md ref normalization
- The behavior section assumed every <ref> arg was a bare branch name
and unconditionally prefixed `origin/`, breaking `origin/develop`
(→ `origin/origin/develop`), tags (`v1.31.0` → `origin/v1.31.0`),
and SHAs. Added a normalization step at the top of the pipeline:
* `refs/tags/<tag>` for tag inputs (fetched via `git fetch origin
tag <tag> --quiet`)
* bare SHA for already-local commits (skips fetch entirely;
commits are immutable)
* `origin/<branch>` left as-is when user passes `origin/...`
* `origin/<branch>` only synthesized for bare branch input
- Dropped the `-C "$REPO"` indirection — the slash command's cwd is
the repo root, no need for the hardcoded variable that would fail
when unset.
- Updated the verdict-line shape to `<resolved-ref> @ <sha7>` so the
citation reflects whatever normalization produced (tag refs, SHAs,
branch refs all render correctly). Added tag + SHA examples.
- Expanded the Edge cases section: `origin/<branch>` input, tag input,
commit SHA input, ref not found.
P2 — hooks/pre-tool-use-claim-verification.sh line 116
- The bare-FS detector regex `(ls|find|cat|head)[[:space:]]` failed
to match the command-as-last-token cases like a bare `ls` invocation
or `cd src && ls` (no trailing whitespace). Switched the trailing
class to `([[:space:]]|$)` so end-of-string also matches.
P3 — hooks/pre-tool-use-claim-verification.sh line 162
- Same fix in the suggester branch for `ls`: `ls[[:space:]]` →
`ls([[:space:]]|$)` so the ls-specific replacement text is emitted
even when the command ends in `ls`.
P3 — hooks/pre-tool-use-claim-verification.sh line 181
- Same fix in the suggester branch for `find`: `find[[:space:]]` →
`find([[:space:]]|$)`.
Verification:
- shellcheck hooks/pre-tool-use-claim-verification.sh → 0 issues
- bash hooks/pre-tool-use-claim-verification.sh "ls" → warn (new)
- bash hooks/pre-tool-use-claim-verification.sh "cd src && ls" → warn (new)
- bash hooks/pre-tool-use-claim-verification.sh "ls src/foo" → warn (regression)
- bash hooks/pre-tool-use-claim-verification.sh "echo hello" → silent (regression)
- bash hooks/pre-tool-use-claim-verification.sh "find" → warn (new)
Created by Claude Code on behalf of @lapc506.
Collaborator
Author
|
@dojo-code-reviewer review Addressed all 4 findings from prior review (3/5 confidence):
Smoke tests + shellcheck still green. Fix commit: 31f10c9. |
There was a problem hiding this comment.
✅ Approved
Approved — 0 blockers, 2 P3. Confidence: 4/5.
🔵 P3 — Minor
hooks/pre-tool-use-claim-verification.sh:145— 🔵 P3 (minor) — Simplify and make the path argument extraction forlscommands more robust. The currentsedregular expression assumes the flags always have a trailing space, which fails to strip flags if they are at the end of the command (e.g.,ls -la), resulting inpath_argbeing extracted as-la. Usingawkto loop through the tokens and extract the first non-option token afterlsis cleaner, shorter, and correctly handles trailing options or extra whitespace.
path_arg="$(printf '%s' "$COMMAND" \
| awk '{for(i=1;i<=NF;i++) if ($i == "ls") {for(j=i+1;j<=NF;j++) if ($j !~ /^-/) {print $j; exit}}}')"
hooks/pre-tool-use-claim-verification.sh:158— 🔵 P3 (minor) — Fix thegreppattern extraction when multiple option flags are specified separately. In the current implementation, if a user runsgrep -i -r "Symbol" src/, theawkblock matches-iand immediately extracts the next token$(i+1)(which is-r) as the pattern. By utilizing a nested loop inawkthat skips any subsequent option tokens starting with-, you can correctly isolate the actual search pattern.
pattern_arg="$(printf '%s' "$COMMAND" \
| awk '{for(i=1;i<=NF;i++) if ($i ~ /^-[rRnliN]+$/) {for(j=i+1;j<=NF;j++) if ($j !~ /^-/) {print $j; exit}}}' \
| tr -d '\"')"
Total findings: 2 business context (2 total)
ℹ️ Inline anchoring failed for the comments below; they are listed here instead.
hooks/pre-tool-use-claim-verification.sh:145— 🔵 P3 (minor) — Simplify and make the path argument extraction forlscommands more robust. The currentsedregular expression assumes the flags always have a trailing space, which fails to strip flags if they are at the end of the command (e.g.,ls -la), resulting inpath_argbeing extracted as-la. Usingawkto loop through the tokens and extract the first non-option token afterlsis cleaner, shorter, and correctly handles trailing options or extra whitespace.
path_arg="$(printf '%s' "$COMMAND" \
| awk '{for(i=1;i<=NF;i++) if ($i == "ls") {for(j=i+1;j<=NF;j++) if ($j !~ /^-/) {print $j; exit}}}')"
hooks/pre-tool-use-claim-verification.sh:158— 🔵 P3 (minor) — Fix thegreppattern extraction when multiple option flags are specified separately. In the current implementation, if a user runsgrep -i -r "Symbol" src/, theawkblock matches-iand immediately extracts the next token$(i+1)(which is-r) as the pattern. By utilizing a nested loop inawkthat skips any subsequent option tokens starting with-, you can correctly isolate the actual search pattern.
pattern_arg="$(printf '%s' "$COMMAND" \
| awk '{for(i=1;i<=NF;i++) if ($i ~ /^-[rRnliN]+$/) {for(j=i+1;j<=NF;j++) if ($j !~ /^-/) {print $j; exit}}}' \
| tr -d '\"')"
…n hook (DOJ-4872)
Round-2 bot review at 4/5 confidence flagged 2 P3 polish items on the
suggester's argument extraction. Both addressed; suggestions applied
verbatim with explanatory comments.
P3 — hooks/pre-tool-use-claim-verification.sh:145 (ls path extraction)
- Old: sed prefix-strip + awk first-token, which fails on commands
like `ls -la` (no path arg, trailing flags) — sed regex `-flags`
requires a trailing space, so the flags leak through and `-la` ends
up as the "path arg" in the suggested replacement.
- New: awk-only nested loop that locates the literal `ls` token,
then walks forward skipping any token starting with `-`, and
emits the first non-flag token. Empty when only flags are present
(correct), so the suggester falls through to the `<path>` placeholder.
P3 — hooks/pre-tool-use-claim-verification.sh:158 (grep pattern extraction)
- Old: awk fallback matched the first `-FLAGS` token and emitted the
immediately following token, which on `grep -i -r "Symbol" src/`
is `-r` — wrong.
- New: nested awk loop that, after finding the flag token, walks
forward skipping additional flag tokens before emitting the first
non-flag one. Correctly isolates `Symbol` for `grep -i -r "Symbol"`,
still correct for the single-flag-block form `grep -rn "Symbol"`.
Verification:
- shellcheck → 0 issues
- "ls -la" → suggester emits <path> placeholder (was: `-la`)
- "ls src/foo.tsx" → suggester emits `src/foo.tsx` (regression)
- "grep -i -r \"Symbol\" src/" → emits "Symbol" (was: `-r`)
- "grep -rn \"Symbol\" src/" → emits "Symbol" (regression)
- "echo hello" → silent (regression)
Created by Claude Code on behalf of @lapc506.
Collaborator
Author
|
@dojo-code-reviewer review Round-2 feedback (4/5) addressed — both P3 items applied verbatim with explanatory comments. Fix commit: $(git rev-parse --short HEAD).
Smoke tests + shellcheck green. |
There was a problem hiding this comment.
✅ Approved
Approved — 0 blockers, 1 P3. Confidence: 4/5.
🔵 P3 — Minor
hooks/pre-tool-use-claim-verification.sh:185— 🔵 P3 (minor) — Strip single quotes as well as double quotes in the fallback pattern extractor to handle single-quoted search patterns (e.g.grep -rn 'Symbol').
| tr -d "\"'")"
Total findings: 1 business context (1 total)
ℹ️ Inline anchoring failed for the comments below; they are listed here instead.
hooks/pre-tool-use-claim-verification.sh:185— 🔵 P3 (minor) — Strip single quotes as well as double quotes in the fallback pattern extractor to handle single-quoted search patterns (e.g.grep -rn 'Symbol').
| tr -d "\"'")"
…n hook (DOJ-4872) Round-3 bot review at 4/5 confidence flagged 1 P3: the pattern extractor only handled double-quoted forms, so `grep -rn 'Symbol' src/` would fall through to the awk fallback and (until the awk fallback also stripped single quotes) emit `'Symbol'` with the quotes attached. Applied: - Added a second extractor branch that mirrors the double-quote form but for single-quoted patterns. Tries double-quote first (cheapest grep), then single-quote, then the multi-flag awk fallback. - Extended `tr -d` in the awk fallback to strip both `"` and `'`, so the unquoted-token output is clean regardless of which quote style the original command used. Verification: - shellcheck → 0 issues - "grep -rn 'Symbol' src/" → "Symbol" (was: 'Symbol') - "grep -rn \"Symbol\" src/" → "Symbol" (regression) - "grep -i -r 'Symbol' src/" → "Symbol" (new, multi-flag + single quote) - "grep -i -r \"Symbol\" src/" → "Symbol" (regression) - "ls src/foo.tsx" → src/foo.tsx (regression) - "echo hello" → silent (regression) Created by Claude Code on behalf of @lapc506.
Collaborator
Author
|
@dojo-code-reviewer review Round-3 feedback (4/5, 1 P3) addressed:
Smoke tests + shellcheck green. Fix commit: $(git rev-parse --short HEAD). |
There was a problem hiding this comment.
✅ Approved
Approved — 0 blockers, 1 P2, 3 P3. Confidence: 3/5.
🟡 P2 — Major
hooks/pre-tool-use-claim-verification.sh:81— 🟡 P2 (major) — The fallback regex"[^\"]*"stops at the first backslash (which JSON uses to escape internal double quotes), resulting in truncated command extraction when the executed bash command contains quotes (e.g.grep -rn \"Symbol\" src/). This causes subsequent command matching and warning suggestions to fail.
Using an escape-aware JSON string regex resolves this.
| grep -oE '"command"[[:space:]]*:[[:space:]]*"([^"\\]|\\.)*"' \
🔵 P3 — Minor
hooks/pre-tool-use-claim-verification.sh:164— 🔵 P3 (minor) — Redundant backslashes inside a single-quoted string are treated as literal backslashes by some shells and grep implementations, which can cause pattern extraction to fail or behave inconsistently depending on the platform's grep engine (e.g. macOS BSD grep vs GNU grep). Removing the backslashes before the double quotes inside single quotes improves cross-platform reliability.
Please also apply the same cleanup to the double quote backslashes on line 166.
| grep -oE 'grep[[:space:]]+-[rRnliN]+[[:space:]]+"[^"]+"' \
-
commands/verify.md:112— 🔵 P3 (minor) — Hardcoding-- src/restricts pattern verification to thesrc/directory. If the toolkit is installed in a repository that uses a different directory structure (e.g.,lib/,app/,packages/, or root-level files),git grepwill fail with a fatal pathspec error. Removing the-- src/suffix or falling back to the repo root ifsrc/is absent makes the command truly generic and repo-agnostic. -
commands/verify.md:45— 🔵 P3 (minor) — Remote-only tags will fail to resolve. If a tag (e.g.,v1.31.0) exists on the remote but has not been fetched locally yet,git rev-parsewill fail, pushing the ref to theelseblock which resolves it to the nonexistent branchorigin/v1.31.0. Pre-fetching tags or queryinggit ls-remotefirst would prevent this failure mode.
Total findings: 1 compliance, 3 business context (4 total)
ℹ️ Inline anchoring failed for the comments below; they are listed here instead.
hooks/pre-tool-use-claim-verification.sh:81— 🟡 P2 (major) — The fallback regex"[^\"]*"stops at the first backslash (which JSON uses to escape internal double quotes), resulting in truncated command extraction when the executed bash command contains quotes (e.g.grep -rn \"Symbol\" src/). This causes subsequent command matching and warning suggestions to fail.
Using an escape-aware JSON string regex resolves this.
| grep -oE '"command"[[:space:]]*:[[:space:]]*"([^"\\]|\\.)*"' \
hooks/pre-tool-use-claim-verification.sh:164— 🔵 P3 (minor) — Redundant backslashes inside a single-quoted string are treated as literal backslashes by some shells and grep implementations, which can cause pattern extraction to fail or behave inconsistently depending on the platform's grep engine (e.g. macOS BSD grep vs GNU grep). Removing the backslashes before the double quotes inside single quotes improves cross-platform reliability.
Please also apply the same cleanup to the double quote backslashes on line 166.
| grep -oE 'grep[[:space:]]+-[rRnliN]+[[:space:]]+"[^"]+"' \
commands/verify.md:112— 🔵 P3 (minor) — Hardcoding-- src/restricts pattern verification to thesrc/directory. If the toolkit is installed in a repository that uses a different directory structure (e.g.,lib/,app/,packages/, or root-level files),git grepwill fail with a fatal pathspec error. Removing the-- src/suffix or falling back to the repo root ifsrc/is absent makes the command truly generic and repo-agnostic.commands/verify.md:45— 🔵 P3 (minor) — Remote-only tags will fail to resolve. If a tag (e.g.,v1.31.0) exists on the remote but has not been fetched locally yet,git rev-parsewill fail, pushing the ref to theelseblock which resolves it to the nonexistent branchorigin/v1.31.0. Pre-fetching tags or queryinggit ls-remotefirst would prevent this failure mode.
…y (DOJ-4872)
Round-4 bot review at 3/5 confidence flagged 1 P2 + 3 P3. All four addressed.
P2 — hooks/pre-tool-use-claim-verification.sh JSON command extractor
- The jq-less fallback regex `"[^"]*"` stops at the first inner quote.
For a bash command like `grep -rn \"Symbol\" src/`, the JSON envelope
embeds it as `"command": "grep -rn \\\"Symbol\\\" src/"`, and the
naive regex truncates at the first `\"`. Subsequent detector / suggester
logic then sees a partial command and emits the wrong replacement.
- Switched to the escape-aware JSON-string body
`([^"\\]|\\.)*` which accepts any non-quote-non-backslash byte OR a
backslash followed by any byte. Verified with an envelope containing
`grep -rn \"Symbol\" src/` — the full command extracts cleanly and
the suggester emits the right line.
P3 — hooks/pre-tool-use-claim-verification.sh cross-platform quote handling
- The bot flagged that the explicit single-quoted-pattern grep extractor
(added in round 3) used embedded-quote shell escaping that can drift
between GNU and BSD grep. Simplified by removing the explicit single-
quoted branch entirely: the awk fallback already strips both `"` and
`'` from the extracted token, so it covers both quote styles without
embedded escapes. Net code reduction; same behavior across the four
smoke-tested variants (single/double quoted, single/multi flag).
P3 — commands/verify.md repo-agnostic grep path
- The behavior section hardcoded `-- src/` as the path filter, which
would fail with a `fatal: pathspec` error in toolkit consumers that
have a different layout (`lib/`, `app/`, `packages/`, flat). Added a
detect-or-fallback step: if `src/` exists on the resolved ref, use
it as the path filter; otherwise pass no path filter and grep the
whole tree.
P3 — commands/verify.md remote-only tag resolution
- The normalization step probed `git rev-parse --verify refs/tags/<tag>`
which only sees LOCAL tags. A remote-only tag (e.g. `v1.31.0` just
pushed by a teammate but not yet fetched locally) would fall through
to the bare-branch branch and resolve to a nonexistent
`origin/v1.31.0`. Added a `git ls-remote --tags --exit-code origin
refs/tags/<tag>` probe between the local-tag and SHA branches; on hit,
runs `git fetch origin tag <tag> --quiet` before resolving. Comment
documents the failure mode.
Verification:
- shellcheck → 0 issues
- JSON envelope with escaped quotes:
`{"tool_input":{"command":"grep -rn \\\"Symbol\\\" src/"}}` →
Command: `grep -rn "Symbol" src/` (was: `grep -rn `)
Suggested: `git grep "Symbol" origin/<branch> -- src/` (was: garbled)
- "grep -rn 'Symbol' src/" → Suggested: "Symbol" (regression)
- "grep -rn \"Symbol\" src/" → Suggested: "Symbol" (regression)
- "grep -i -r \"Symbol\" src/" → Suggested: "Symbol" (regression)
- "ls src/foo.tsx" → Suggested: src/foo.tsx (regression)
- "echo hello" → silent (regression)
Created by Claude Code on behalf of @lapc506.
Collaborator
Author
|
@dojo-code-reviewer review Round-4 feedback (3/5, 1 P2 + 3 P3) addressed:
Smoke tests + shellcheck green. Fix commit: $(git rev-parse --short HEAD). |
There was a problem hiding this comment.
✅ Approved
Approved — 0 blockers, 4 P2. Confidence: 2/5.
🟡 P2 — Major
commands/verify.md:44— 🟡 P2 (major) — TheARGvariable is used in later steps (e.g.,"$ARG") but is never defined or assigned. This will cause$ARGto resolve to an empty string, breaking file path and pattern verification when Claude follows these instructions. AssigningARG="$2"next toRAW_REFensures proper parameter mapping.
RAW_REF="$1"
ARG="$2"
commands/verify.md:54— 🟡 P2 (major) — Runninggit fetch origin "tag $RAW_REF"(with quotes grouping thetagkeyword and the tag name together) passes them as a single argument to Git. This will cause the fetch operation to fail with a "does not appear to be a git repository" error. Separating them as distinct arguments ensures the remote-only tag is fetched successfully.
git fetch origin tag "$RAW_REF" --quiet
hooks/pre-tool-use-claim-verification.sh:99— 🟡 P2 (major) — The unanchored patterngrep[[:space:]]+-[rRnliN]+matches any occurrence of grep followed by flags, which includes legitimate Git commands likegit grep -n. This causes the hook to generate false-positive warnings even when users are running the exact ref-explicitgit grepcommands recommended by this very skill. Anchoringgrepto the start of the command or a separator prevents these false positives.
if printf '%s' "$cmd" | grep -qE '(^|[;&|][[:space:]]*)grep[[:space:]]+-[rRnliN]+'; then
hooks/pre-tool-use-claim-verification.sh:145— 🟡 P2 (major) — The unanchored patterngrep[[:space:]]+-[rRnliN]+matchesgit grephere as well, causing false positives in the suggestion logic. Anchoringgrepto the start of the command or a separator keeps the matching logic consistent with the detector.
elif printf '%s' "$COMMAND" | grep -qE '(^|[;&|][[:space:]]*)grep[[:space:]]+-[rRnliN]+'; then
Total findings: 4 compliance (4 total)
ℹ️ Inline anchoring failed for the comments below; they are listed here instead.
commands/verify.md:44— 🟡 P2 (major) — TheARGvariable is used in later steps (e.g.,"$ARG") but is never defined or assigned. This will cause$ARGto resolve to an empty string, breaking file path and pattern verification when Claude follows these instructions. AssigningARG="$2"next toRAW_REFensures proper parameter mapping.
RAW_REF="$1"
ARG="$2"
commands/verify.md:54— 🟡 P2 (major) — Runninggit fetch origin "tag $RAW_REF"(with quotes grouping thetagkeyword and the tag name together) passes them as a single argument to Git. This will cause the fetch operation to fail with a "does not appear to be a git repository" error. Separating them as distinct arguments ensures the remote-only tag is fetched successfully.
git fetch origin tag "$RAW_REF" --quiet
hooks/pre-tool-use-claim-verification.sh:99— 🟡 P2 (major) — The unanchored patterngrep[[:space:]]+-[rRnliN]+matches any occurrence of grep followed by flags, which includes legitimate Git commands likegit grep -n. This causes the hook to generate false-positive warnings even when users are running the exact ref-explicitgit grepcommands recommended by this very skill. Anchoringgrepto the start of the command or a separator prevents these false positives.
if printf '%s' "$cmd" | grep -qE '(^|[;&|][[:space:]]*)grep[[:space:]]+-[rRnliN]+'; then
hooks/pre-tool-use-claim-verification.sh:145— 🟡 P2 (major) — The unanchored patterngrep[[:space:]]+-[rRnliN]+matchesgit grephere as well, causing false positives in the suggestion logic. Anchoringgrepto the start of the command or a separator keeps the matching logic consistent with the detector.
elif printf '%s' "$COMMAND" | grep -qE '(^|[;&|][[:space:]]*)grep[[:space:]]+-[rRnliN]+'; then
…OJ-4872)
Round-5 bot review at 2/5 confidence flagged 4 P2. All four are real
correctness bugs (no nits this round); applied verbatim.
P2 — hooks/pre-tool-use-claim-verification.sh detector grep false-positive
- The unanchored pattern `grep[[:space:]]+-[rRnliN]+` matched any
grep-followed-by-flags substring, including the EXACT ref-explicit
replacement this hook recommends: `git grep -n "Symbol" origin/develop
-- src/`. Result: the hook warned against its own suggested fix,
creating a self-reinforcing false-positive loop. Anchored with
`(^|[;&|][[:space:]]*)` so `grep` must be at the start of the command
or after a separator, never inside `git grep`.
P2 — hooks/pre-tool-use-claim-verification.sh suggester grep false-positive
- Same unanchored pattern in the suggester branch caused the
`git grep` suggester to fire on legitimate `git grep` invocations
that fell through to the suggester (they shouldn't get there at all
if the detector is right, but defense-in-depth). Applied the same
`(^|[;&|][[:space:]]*)` anchor.
P2 — commands/verify.md ARG undefined
- The behavior section referenced `$ARG` in step 5 but never assigned
it. Following the example verbatim would leave `$ARG` empty and break
every verification. Added `ARG="$2"` next to `RAW_REF="$1"` at the
top of step 1.
P2 — commands/verify.md tag-fetch arg gluing
- `git fetch origin "tag $RAW_REF" --quiet` quotes the `tag` keyword
together with the tag name as a single arg, which git parses as a
refspec name → "does not appear to be a git repository" or "fatal:
couldn't find remote ref" depending on git version. Switched to
separate `tag` + `"$RAW_REF"` args, and refactored step 3 to use a
case-statement dispatcher over `$FETCH_KIND` ("tag" | "branch" | "")
so the tag-vs-branch fetch shapes stay distinct.
Verification:
- shellcheck → 0 issues
- "grep -rn \"Symbol\" src/" → warn (regression)
- "git grep -n \"Symbol\" origin/develop -- src/" → silent (was: WARN — the hook
was attacking its
own recommended fix)
- "cd src && grep -rn \"Symbol\" ." → warn (chain-after-separator works)
- "echo hello" → silent (regression)
Created by Claude Code on behalf of @lapc506.
Collaborator
Author
|
@dojo-code-reviewer review Round-5 feedback (2/5, 4 P2) addressed — all four were real correctness bugs:
Smoke tests + shellcheck green. Fix commit: $(git rev-parse --short HEAD). |
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
1 participant
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
Plugin-layer (Level 1) extension of the working-tree-vs-ref discipline rule. This release operationalizes memory
feedback_working_tree_is_not_truthat the toolkit layer so the rule travels with the plugin to every repo it's installed in.Incident origin (2026-06-03). A re-audit subagent correctly flagged
usePathwayViewModel.tsas deleted ondevelopduring DOJ-4849 triage. The orchestrator "corrected" the subagent by runningls src/hooks/path/usePathwayViewModel.tsagainst a local main checkout that another session had switched to a different branch. The bare-FS read returned a stale projection; authoritative "FRESH — proceed as briefed" comments went to DOJ-4851/4863/4864 and had to be retracted publicly. Root cause: working tree was treated as truth instead of one projection.Defense in depth. Earlier today, DOJ-4868 shipped the repo-layer (Level 2) enforcement via
.claude/hooks/+ rootCLAUDE.mdindojo-os(PR #2604). DOJ-4872 ships the plugin-layer (Level 1) here: skill + slash command + warn-mode PreToolUse hook. Without it, agents usingmake-no-mistakes-toolkitin any other repo have zero guidance about the rule.Linear: DOJ-4872
Sibling: DOJ-4868 (dojo-os, Level 2)
Memory:
feedback_working_tree_is_not_truthRelease: v1.30.0 → v1.31.0
What ships
4 new artifacts:
skills/verify-branch-state/SKILL.md— auto-loading skill, tuned description triggers on "verify against develop", "does X exist on<branch>", "what's on origin/…", "did this land on main", "re-validate the subagent", "before I claim", "post-merge state". Body covers the rule statement, canonical ref-explicit commands (git fetch + git show / ls-tree / grep / diffagainstorigin/<ref>), anti-patterns (ls,grep -rn src/,find,git status), pre-flight checklist, multi-worktree context, and the 2026-06-03 origin incident.commands/verify.md— slash command/verify <ref> <path-or-pattern>. Runsgit fetch origin <ref> --quietthengit ls-tree origin/<ref> -- <path>for paths orgit grep <pattern> origin/<ref> -- src/for patterns. Emits a single verdict line citing the ref's short SHA in the shapeorigin/<ref> @ <sha7> — <path-or-pattern> <PRESENT|NOT PRESENT|N MATCHES>so the claim can be re-checked later. Handles ref-not-found, shell metacharacters in patterns, and squash-merge cache staleness (always re-fetches).hooks/pre-tool-use-claim-verification.sh— warn-mode PreToolUse hook, repo-agnostic, no per-repo manifest required. Detects bare-FS verification ops (ls,grep -rn src/,find src/,cat src/,head src/) and, when combined with intent keywords in the recent context envelope (orCLAUDE_HOOK_CONTEXTfor smoke tests), prints a stderr warning with the local checkout's HEAD ref + short SHA, a suggested ref-explicit replacement specific to the detected op, and pointers to the skill + slash command. Alwaysexit 0. Defensiveset +e. Usescd -P+pwd -Pfrom day 1 (DOJ-4868 PR #2604 lesson). Trailing-newline-safewhile read. HonorsCLAUDE_DISABLE_PLUGIN_HOOKS=1. Shellcheck clean.CHANGELOG.md[1.31.0] - 2026-06-03— Added / Changed / Why sections + reference link added at bottom.4 edits:
hooks/hooks.json— registers the new hook under theBashPreToolUse matcher with a 3-second timeout, alongside the existingpre-bash.shdispatcher..claude-plugin/marketplace.json— top-level + pluginversion1.30.0 → 1.31.0; plugin description pair-mentions the new skill; component counts updated to 30 commands / 11 skills / 2 agents..claude-plugin/plugin.json—version1.30.0 → 1.31.0.package.json—version1.30.0 → 1.31.0.README.md—**Version:**header bumped; new "Also start here:verify-branch-state+/verify" peer subsection sits between thedomain-driven-advisor"Start here" block and "What's Inside"; Commands table 29 → 30 (adds/verify); Skills table 10 → 11 (addsverify-branch-state).Cohesive-PR exception (9 files)
9 files instead of the soft ≤8 target. The 4 new artifacts + 4 release-cut edits sum to 8; the 9th is
hooks/hooks.json, which is required to make the new hook non-dead code. The whole set is a single cohesive surface (working-tree-vs-ref discipline) plus the standard release-cut shape mirroring v1.30.0's commit shape. Documented per CLAUDE.md rule 7 cohesive-PR exception.Per-AC ship status
/verify <ref> <path-or-pattern>cites ref SHAset +e,cd -P+pwd -P,exit 0always, shellcheck cleanjq[1.31.0] - 2026-06-03+ reference linkfeedback_working_tree_is_not_truthv1.31.0Smoke test outputs (AC7)
Post-merge actions (deferred to maintainer)
These are explicitly out of this PR's scope and will be performed after merge by @lapc506:
git tag -a v1.31.0 -m "v1.31.0"+git push origin v1.31.0(AC10)#doj-repo-health(Slack thread)Test plan
jq -r '.version' …× 4)ls, silent onecho, warn ongrep -rn, warn onfindshellcheck hooks/pre-tool-use-claim-verification.sh→ 0 issuesCLAUDE_DISABLE_PLUGIN_HOOKS=1(earlyexit 0)cd -P+pwd -Pused for hook self-location (day 1, per DOJ-4868 PR #2604)descriptionfield tuned for auto-trigger on the canonical intent phrases[1.31.0]: …/releases/tag/v1.31.0)domain-driven-advisorstyleCreated by Claude Code on behalf of @lapc506.