Merged
Conversation
… needle is compared, not parsed as an option (#214) grep -qF "$expected" <<<"$output" with no -- separator lets grep parse a needle beginning with -- as an option instead of a pattern, exits non-zero on a usage error, and the helper -- which branches only on exit status -- cannot tell that apart from a genuine no-match. assert_not_contains is the dangerous direction: it reported PASS having compared nothing. Fixed the 16 suites that still lacked the -- separator (9 already had it). Kept the mechanical sweep rather than centralizing the helper: the meta-suite in test-assertion-helpers.sh extracts each suite's real function body by reading the file, so a shared/sourced helper would have nothing left to extract and would collapse DROVE to near zero -- the sweep is the route that does not fight that architecture. test-assertion-helpers.sh now also drives a --prefixed needle in both directions for every capture/file/path-arg contains and not_contains helper, so a reintroduced missing -- is caught structurally rather than waiting to be tripped over again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNPrdageDpVsLFP8eL9VSj
fdaviddpt
added a commit
that referenced
this pull request
Aug 25, 2026
… needle is compared, not parsed as an option (#214) (#224) grep -qF "$expected" <<<"$output" with no -- separator lets grep parse a needle beginning with -- as an option instead of a pattern, exits non-zero on a usage error, and the helper -- which branches only on exit status -- cannot tell that apart from a genuine no-match. assert_not_contains is the dangerous direction: it reported PASS having compared nothing. Fixed the 16 suites that still lacked the -- separator (9 already had it). Kept the mechanical sweep rather than centralizing the helper: the meta-suite in test-assertion-helpers.sh extracts each suite's real function body by reading the file, so a shared/sourced helper would have nothing left to extract and would collapse DROVE to near zero -- the sweep is the route that does not fight that architecture. test-assertion-helpers.sh now also drives a --prefixed needle in both directions for every capture/file/path-arg contains and not_contains helper, so a reintroduced missing -- is caught structurally rather than waiting to be tripped over again. Claude-Session: https://claude.ai/code/session_01MNPrdageDpVsLFP8eL9VSj Co-authored-by: Florian DAVID <fdavid@digital-village.fr> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
fdaviddpt
pushed a commit
that referenced
this pull request
Aug 25, 2026
#214) fix/223 branched from db0c07f, before #224 (6899dd6) taught assert_contains/assert_not_contains a -- separator so a --prefixed needle is compared rather than parsed as a grep option, and taught test-assertion-helpers.sh to drive every declared helper with one structurally. My new suite's helpers predate that fix and were caught by it: the meta-suite drove a --prefixed needle against grep -qF "$expected" with no separator and found the composition defect between two individually-correct changes. Rebased fix/223 onto origin/main (6899dd6) -- no conflicts, since #224 never touched a file this branch created or modified -- and added the same -- separator #224 already applied to the other 16 suites, matching its convention exactly rather than inventing a variant. bash tests/run-all.sh prints "All suites passed." across all 45 tests/test-*.sh files, including test-assertion-helpers.sh (315/315) and this suite's own 8 driven checks in both directions.
fdaviddpt
pushed a commit
that referenced
this pull request
Aug 26, 2026
#214) fix/223 branched from db0c07f, before #224 (6899dd6) taught assert_contains/assert_not_contains a -- separator so a --prefixed needle is compared rather than parsed as a grep option, and taught test-assertion-helpers.sh to drive every declared helper with one structurally. My new suite's helpers predate that fix and were caught by it: the meta-suite drove a --prefixed needle against grep -qF "$expected" with no separator and found the composition defect between two individually-correct changes. Rebased fix/223 onto origin/main (6899dd6) -- no conflicts, since #224 never touched a file this branch created or modified -- and added the same -- separator #224 already applied to the other 16 suites, matching its convention exactly rather than inventing a variant. bash tests/run-all.sh prints "All suites passed." across all 45 tests/test-*.sh files, including test-assertion-helpers.sh (315/315) and this suite's own 8 driven checks in both directions.
fdaviddpt
added a commit
that referenced
this pull request
Aug 26, 2026
… (#225) * Teach jit-dry-run.sh's report_hook() to trust the block manifest (#223) report_hook() greped raw hook stdout for "# Vocabulary: X.md" / "# JIT Context: X.md" header text with no manifest awareness, so an entry whose own body quoted that header text verbatim was reported as a fabricated second match at exit 0. #219 had already closed the equivalent class in jit-match.sh by trusting pre-prompt-hook.sh's byte-length manifest instead of searching for the "\n---\n" separator an entry body can forge. Moves the manifest-plus-fallback block split (jit_split_ctx_blocks()) and jit_decode_u00() into common.sh (JIT_AWK_BLOCKS) so jit-match.sh and report_hook() share one reader instead of carrying a second copy that can drift out of step again -- which is what left this one unfixed the first time. report_hook() additionally decodes a block decision's "reason" field, whose one genuine header is always its first line and needs no manifest to extract safely. jit_index_verified()'s structural cross-check (#216) is deliberately not ported: it needs a loaded vocabulary index, and report_hook() spans three dimensions across however many layers a tree defines, while the manifest split alone already closes the forgery class whenever it verifies. * Fix two review findings: the path-matched vocabulary regex and jit_decode_u00() (#223) Explore review found that report_hook()'s new manifest-aware regex was anchored on "(matched:" and never matched pre-path-hook.sh's own vocabulary- by-path header, "(matched path:" -- a genuine second spelling that silently dropped every such match as "no rule fired". Both spellings are now accepted, and the sub() that strips the parenthetical off the extracted name is widened the same way. oss:auditor found a second, more severe issue in jit_decode_u00() itself (inherited unmodified from #219, now shared via common.sh): it decoded any "\u00XX" sequence regardless of value, but the hooks' own encoder only ever produces that shape for a control byte 0..31 (excluding \t/\n/\r). Ordinary entry prose that happens to spell out the same 6-byte ASCII shape -- e.g. describing JSON escaping -- was decoded exactly like a real escaped control byte, shrinking it from 6 bytes to 1 and desyncing the decoded length from the hook's byte-length manifest, which still counts the original 6. That failed the manifest check and fell back to the pre-#219/#223 "\n---\n"- search splitter via ordinary prose, reopening the exact forgery class this fix closes -- no adversarial payload required. jit_decode_u00() now only decodes a value the encoder can actually produce; the fix is shared, so it also corrects the same latent defect in jit-match.sh's already-shipped #219. Both are pinned in tests/test-jit-dry-run-report-forgery.sh, driven red against the prior commit before being fixed. * Add -- to grep -qF in the new suite's helpers, rebased onto #224 (#223, #214) fix/223 branched from db0c07f, before #224 (6899dd6) taught assert_contains/assert_not_contains a -- separator so a --prefixed needle is compared rather than parsed as a grep option, and taught test-assertion-helpers.sh to drive every declared helper with one structurally. My new suite's helpers predate that fix and were caught by it: the meta-suite drove a --prefixed needle against grep -qF "$expected" with no separator and found the composition defect between two individually-correct changes. Rebased fix/223 onto origin/main (6899dd6) -- no conflicts, since #224 never touched a file this branch created or modified -- and added the same -- separator #224 already applied to the other 16 suites, matching its convention exactly rather than inventing a variant. bash tests/run-all.sh prints "All suites passed." across all 45 tests/test-*.sh files, including test-assertion-helpers.sh (315/315) and this suite's own 8 driven checks in both directions. --------- Co-authored-by: Florian DAVID <fdavid@digital-village.fr>
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
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.
Closes #214
What was broken
assert_contains()/assert_not_contains()are duplicated inline acrosstests/*.sh, and the shape wasgrep -qF \"$expected\" <<<\"$output\"with no--separator. A needle beginning with--(exactly the--flag needs a targetshape this repo's CLI-heavy suites reach for constantly) is parsed by grep as an option, grep exits non-zero on a usage error, and the helper -- which branches only on exit status -- cannot tell that apart from a genuine no-match.assert_not_containsis the dangerous direction: it reported PASS having compared nothing.The decision this issue left open
The issue named two routes -- add
--everywhere, or centralise the helper -- and declined to pick. I took the mechanical sweep, not centralisation, for a structural reason rather than a preference:tests/test-assertion-helpers.sh(the meta-suite that already drives every suite's real helpers) extracts each suite's function body by reading the file and sourcing the extracted text. A shared/sourced helper would leave nothing for that extractor to find in each suite file, collapsing itsDROVEcount toward zero and breaking theDROVE < 20floor it already enforces. The sweep is the route that does not fight that architecture.What changed
assert_contains/assert_not_containslacked the--separator (9 already had it, one --test-marker-degradation.sh-- uses acaseglob match and was never affected). All 16 now pass--before the needle in everygrep -qF/grep -qcall inside those two helpers.tests/test-assertion-helpers.shgained a---prefixed-needle payload (DASHNEEDLE/DASHBIG/DASHBIGFILE), driven through the existingcapture/file:*/path-arg×contains/not_containsbranches indrive_declared(), in both directions. A reintroduced missing--is now caught structurally rather than waiting to be tripped over again.changelog.d/214.fixed.md.Test evidence
Red (before the 16-file fix, with only the new DASHNEEDLE fixture added to the meta-suite):
Results: 269 passed, 30 failed, all 30 failures the expectedassert_contains should report PASS/assert_not_contains should report FAILpairs across exactly the 16 broken files.Green (after):
Results: 299 passed, 0 failed.bash tests/run-all.sh(the repo'stest_command):All suites passed.Scope note
assert_confined()intests/test-pre-path-hook.shhas the same unguardedgrep -q \"$forbidden\"shape and was not touched -- every call site passes a fixed literal rule name (\"i18n rules\",\"leaf rule\"), never a---prefixed value, so it is not reachable with today's test data and is out of #214's stated scope (assert_contains/assert_not_containsspecifically). Flagging it here as a below-bar finding rather than silently leaving it: a real latent shape, no reachable caller today.Verified by the maintainer
Red re-run, independent of the agent's. Checked out
db0c07f(currentmain, which contains #222) into a detached worktree, dropped in this branch'stests/test-assertion-helpers.shalone, and ran it with the 16-file sweep absent:Results: 277 passed, 30 failed. Same 30 failures the agent reported; my pass count differs (277 vs 269) only because my base carries #222's three extra suites. The fixture is not vacuous.The sweep is complete after the rebase, which was not obvious at dispatch. This branch was cut from
3b13092, before #222 merged, so it never sawtest-block-framing.sh,test-jit-match.shortest-sample-call-log.sh. I briefed the lane not to touch them and flagged that the sweep might therefore land incomplete. Checked directly: all three already carrygrep -qF --, so there is no gap to close after rebasing ontomain.Blast radius against the true merge-base (
3b13092..fix/214, not againstmain, which mixes in #222's reverse diff): 17 files — the 16 swept suites, the meta-suite, andchangelog.d/214.fixed.md. Nothing outsidetests/. None of #222's three files touched, so the lane held.On the centralisation route being declined: I accept the argument as stated.
test-assertion-helpers.shextracts each suite's helper body by reading the file and sourcing the extracted text, and enforces aDROVE < 20floor — a sourced shared helper leaves that extractor nothing to find per suite. That is a structural reason, not a preference, and it is the one I would have wanted named.On the
assert_confined()below-bar item: accepted as below the bar. Every call site passes a fixed literal rule name, so it is unreachable with today's test data. Recorded here rather than filed, per the receipt rule.