Skip to content

Fuse jit_decode_u00() into jit_unescape() and surface a failed block manifest - #229

Merged
fdaviddpt merged 3 commits into
mainfrom
fix/226
Aug 26, 2026
Merged

Fuse jit_decode_u00() into jit_unescape() and surface a failed block manifest#229
fdaviddpt merged 3 commits into
mainfrom
fix/226

Conversation

@fdaviddpt

@fdaviddpt fdaviddpt commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

jit_decode_u00() ran as a second awk pass after jit_unescape() had already collapsed
an entry own escaped backslash back to one literal backslash. That meant ordinary
prose in an entry body, shaped like a JSON control-byte escape, became byte-identical
to a genuine encoder-emitted escape once the first pass had run -- jit_decode_u00()
then collapsed both cases the same way, shrinking the block by five bytes and desyncing
it from the hook own byte-length manifest. The manifest check failed and the whole
thing fell back to the pre-#219/#223 forgeable splitter, through ordinary prose alone,
no adversarial payload needed. Closes #226 by fusing both passes into one left-to-right
walk (jit_unescape_blocks()) so the ambiguity never arises.

Separately, jit_blk_manifest_ok (the flag jit_split_ctx_blocks() already computed) was
read by nobody, so a failed manifest verification degraded silently in both consumers.
That silence is why #226 shipped green in the first place. Closes #227 by making both
consumers read the flag: jit-match.sh (must never fail hard) names the degrade as a
notice; jit-dry-run.sh (must fail loudly) prints a NOTE and moves off exit 0.

A third fact turned up during the #227 review, out of scope for this PR: pre-tool-hook.sh
and pre-path-hook.sh never build a manifest at all, only pre-prompt-hook.sh does. Naively
gating the #227 fix on the bare manifest-ok flag reported every genuine tool/path match as
could-not-evaluate -- caught by a large existing-test regression before it shipped. Worked
around with a second flag, jit_blk_manifest_seen (1 iff a manifest header was ever present),
so this PR does not regress the existing suite. The deeper gap -- extending #219 own
manifest-producer mechanism to those two hooks -- is filed separately as its own issue
rather than folded in here, since it is a materially bigger, separately reviewable change.

Self-review (Explore + oss:auditor, spawned concurrently) found two things, both fixed
in this PR: a stale comment in jit-match.sh still naming the deleted jit_decode_u00(),
and no automated test for the #227 notification path in either consumer (proved by
hardcoding both gating conditions false and getting a fully green suite) -- fixed with
tests/test-manifest-desync-227.sh, confirmed red against the pre-#227 commit.

Test plan

Verified by the maintainer

The fix, checked against my own fixture rather than the branch's tests. I reproduced #226 independently before filing it; re-running that same fixture against this branch:

  • jit-match.sh1 entry matched, exit 0, and the match is tricky.md — the genuine entry. The forged # Vocabulary: billing.md (matched: billing) stays inside tricky.md's body as prose instead of being promoted to a second match, and the six-character JSON control escape survives literally as prose.
  • jit-dry-run.sh --prompt "tricky situation"tricky.md(WHOLE BODY). Before this branch it printed billing.md(WHOLE BODY) at exit 0 for a keyword that was not in the prompt.

That is the class closed, on the exact input that opened it.

Red re-run against 408990d, each changed suite dropped in alone:

Suite On main
test-manifest-desync-227.sh exit 1, 3 failures — #227's degrade is genuinely unobservable there
test-block-framing.sh exit 1, 4 failures — the new #226 cells
test-pre-prompt-hook.sh exit 1, dies at the new #226 section; 126/126 and 199 lines on this branch against 108 lines on main
test-jit-dry-run-report-forgery.sh exit 0 — correct, its diff is a comment and one assertion message, no behaviour

One thing to know about that third row rather than discover later: on the pre-fix code that section terminates the suite instead of printing a FAIL. run-all.sh still catches it — the exit code is non-zero and honest — but the log shows a section header and then nothing, which reads like a truncated run rather than a failure. Not blocking, and not worth a separate issue, but it is the one place in this diff where a red is quieter than the convention here asks for.

Blast radius against 408990d: 11 files — the three scripts, four suites, README.md, and three changelog fragments including the correction to changelog.d/223.fixed.md's now-false claim. Nothing outside the hooks lane.

On the adjacent finding, which I have filed as #230 and am not blocking the tag on. Verified independently: grep -ln 'JIT-CTX-BLOCKS' scripts/*.sh returns common.sh, jit-dry-run.sh and pre-prompt-hook.sh only; the count in pre-tool-hook.sh and pre-path-hook.sh is zero. So the manifest has one producer and the other two hooks always take the forgeable fallback. forges is a blocking row and I am shipping 0.6.0 over it anyway, because the defect is pre-existingv0.5.0 has it, this release neither introduces nor widens it, and the tag is not the mechanism by which it takes effect. Blocking would leave users on a version with this gap plus the one #226 closes. That is a judgment against the letter of the ranking table and it is reversible: #230 carries the argument and the priority.

jit_blk_manifest_seen is a workaround and the PR says so, which is the right call. Gating on the bare flag would have reported every genuine tool/path match as could-not-evaluate — the cascade is what found #230. Removing the second flag belongs to #230's fix, not here.

Florian DAVID and others added 3 commits August 26, 2026 04:33
…manifest (#226, #227)

jit_decode_u00() ran as a second pass after jit_unescape() had already collapsed an
entry's own escaped backslash, so ordinary prose shaped like a JSON \u00XX escape became
byte-identical to a genuine control-byte escape and desynced the block manifest, falling
back to the pre-#219/#223 forgeable splitter -- reopening that class through prose alone.
jit_unescape_blocks() fuses both passes into one left-to-right walk so the ambiguity
never arises.

jit_blk_manifest_ok was computed by common.sh and read by nobody, which is why the #226
desync could ship green. Both consumers now read it: jit-match.sh names the degrade as a
notice (it must never fail hard), jit-dry-run.sh prints a NOTE and moves off exit 0 (it
must fail loudly). Gated on a new jit_blk_manifest_seen flag rather than the bare ok flag,
because pre-tool-hook.sh and pre-path-hook.sh never build a manifest at all -- a separate,
pre-existing gap filed on its own -- and moving the exit code on every fallback use would
have reported ordinary tool/path matches as "could not evaluate".

changelog.d/223.fixed.md's own claim that this class was closed was wrong; corrected here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MNPrdageDpVsLFP8eL9VSj
…stale comment

Both the reviewer and the auditor found the same gap: nothing in the suite drove
jit-dry-run.sh's BLOCKS_DESYNC/NOTE path or jit-match.sh's notice path -- every
assertion that would catch #227 regressing could pass unchanged if either consumer's
reading of jit_blk_manifest_ok/jit_blk_manifest_seen were reverted, mistyped, or
inverted. Confirmed by hand: hardcoding both conditions false left tests/run-all.sh
green.

tests/test-manifest-desync-227.sh forces a genuine desync structurally, in a scratch
copy of scripts/ with one line of common.sh patched so a manifest that would otherwise
verify is reported as failed, then runs the real, unmodified jit-match.sh and
jit-dry-run.sh against a real fixture. Confirmed red against the pre-#227 commit (3
failures) and green against this branch. A second section pins jit_blk_manifest_seen
directly: a path-matched vocabulary entry, which never had a manifest to begin with,
must not be reported as a desync by either consumer.

Also: scripts/jit-match.sh's comment above the block-decode section still named
jit_decode_u00(), which #226 deleted (fused into jit_unescape_blocks() in common.sh).
Updated to name the function that actually exists.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MNPrdageDpVsLFP8eL9VSj
tests/test-manifest-desync-227.sh named changelog.d/227.fixed.md by path in a comment --
a fragment the next release deletes, so the citation goes stale the moment it is tagged
(changelog.d/README.md's own rule, enforced by tests/test-changelog-fragment-refs.sh).
Point at CHANGELOG.md instead, where the prose lands and stays.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MNPrdageDpVsLFP8eL9VSj
@fdaviddpt
fdaviddpt merged commit 4f1ad01 into main Aug 26, 2026
5 checks passed
@fdaviddpt
fdaviddpt deleted the fix/226 branch August 26, 2026 04:25
fdaviddpt pushed a commit that referenced this pull request Aug 26, 2026
Bump the four version sites and fold 24 changelog fragments into `## [0.6.0]`.
Gate 3's blocking finding (#226) and its silencer (#227) landed in #229 before
this tag; #230 is filed and deliberately not blocking, being pre-existing in
0.5.0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MNPrdageDpVsLFP8eL9VSj
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant