ReviewOS

also looking at this

stacks/bun-query-builder

fix(client): write the cache-key delimiter as \0, not a raw NUL byte

#1102
Merged glennmichael123 wants to merge fix/nul-byte-cache-key-delimiter into main
2 files +83 -1
10 days ago

src/client.ts carries a literal NUL byte (0x00) as the delimiter in a query cache key, written as the raw control character instead of the \0 escape. From d7c3a16 (June).

const cacheKey = useCache
  ? `${String(finalQuery)}<0x00>${JSON.stringify(whereParams)}`
  : ''

Why it matters

The runtime string is correct, so nothing ever failed. What it breaks is every tool that reads the file. A single NUL makes grep classify the whole file as binary and print binary file matches instead of matching lines — ripgrep, VS Code search and agent search tools inherit the same heuristic.

The repo's largest and most-edited source file, 8000+ lines, silently returns nothing for ordinary searches:

$ grep -c "async paginate" src/client.ts
0                          # exit 1 — while `grep -a` finds it at line 5596

A search that returns nothing is indistinguishable from a symbol that does not exist. This is not hypothetical: it produced a wrong conclusion about missing code during this week's triage (I briefly concluded whereJsonContains had been deleted from the source), and three queued backlog items require searching this file.

Why it is safe

The escape and the raw byte are equivalent, verified rather than assumed:

  • both sources transpile to byte-identical output (180277 bytes)
  • the runtime cache-key string is unchanged
  • it still contains exactly one NUL character, so the delimiter keeps the property d7c3a16 chose it for — a SQL tail and a parameter head cannot run together into the same key

dist/ never contained the raw byte, and the key is never persisted across a boundary.

Guard

Adds test/source-hygiene.test.ts, which scans src/ for C0 control characters other than tab, newline and carriage return. Confirmed it fails against the pre-fix file:

guard against pre-fix main: WOULD FAIL -> line 5907 U+0000

It also pins the delimiter itself, because swapping it for a printable character would silently reintroduce the collision d7c3a16 fixed — client.caching.test.ts:227 covers that behaviour, but nothing covered the delimiter's identity.

Checks

  • bun test — 1957 pass, 4 skip, 1 fail; typecheck and pickier clean

The one failure is CLI > version prints package version, which is unrelated and local-only: bin/qbx is a gitignored binary compiled at build time, and a stale local copy reports an old version. CI rebuilds it.

🤖 Generated with Claude Code

2 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.