From 8aa98215f6a00bc27a310e931383a6de9e2d8252 Mon Sep 17 00:00:00 2001 From: willbot Date: Tue, 21 Jul 2026 13:46:33 +0200 Subject: [PATCH 1/3] refactor(service-rpc): the idempotency LRU tracks entries, not a joined string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The store flattened (method, key) into a delimited string to use it as an LRU Map key, and picked NUL as the delimiter because neither part can contain one. That literal NUL is what made git treat serve.ts as binary (fixed in 808a8ce by escaping it) — but the delimiter was the real smell. Remove the flattening entirely: a completed entry carries its own method and key, and the LRU is a Set of entries in insertion order. Eviction reads the oldest entry's own location. No delimiter, no lruKey helper, no composite Map key. Same behavior — dedup, single-flight, replay, and LRU eviction tests all pass unchanged. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .../2-authoring/service-rpc/src/serve.ts | 58 ++++++++++--------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/packages/0-framework/2-authoring/service-rpc/src/serve.ts b/packages/0-framework/2-authoring/service-rpc/src/serve.ts index d1deab4f..8aee5c96 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/serve.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/serve.ts @@ -112,9 +112,16 @@ export const REPLAY_CACHE_MAX_ENTRIES = 1000; /** How long a completed 2xx/4xx answer stays replayable for a repeated key. */ const REPLAY_TTL_MS = 60_000; -type CacheEntry = - | { readonly kind: 'pending'; readonly promise: Promise } - | { readonly kind: 'completed'; readonly outcome: Outcome; readonly completedAt: number }; +interface CompletedEntry { + readonly kind: 'completed'; + readonly outcome: Outcome; + readonly completedAt: number; + // The entry knows where it lives, so eviction can delete it from byMethod + // without reconstructing its location. + readonly method: string; + readonly key: string; +} +type CacheEntry = { readonly kind: 'pending'; readonly promise: Promise } | CompletedEntry; /** * Per-method, per-key deduplication. A duplicate arriving mid-execution @@ -124,8 +131,9 @@ type CacheEntry = */ class IdempotencyStore { private readonly byMethod = new Map>(); - // LRU order over completed entries; Map insertion order is the list, so re-inserting moves a key to the end. - private readonly lruOrder = new Map(); + // Completed entries in least-recently-used order. A Set keeps insertion + // order, so deleting then re-adding an entry moves it to the newest end. + private readonly lru = new Set(); async dispatch(method: string, key: string, run: () => Promise): Promise { const bucket = this.bucketFor(method); @@ -136,11 +144,12 @@ class IdempotencyStore { } if (existing?.kind === 'completed') { if (Date.now() - existing.completedAt < REPLAY_TTL_MS) { - this.touch(method, key); + this.lru.delete(existing); + this.lru.add(existing); return existing.outcome; } bucket.delete(key); - this.lruOrder.delete(this.lruKey(method, key)); + this.lru.delete(existing); } const promise = run(); @@ -157,8 +166,16 @@ class IdempotencyStore { if (result.status >= 500) { bucket.delete(key); // retryable outcome — a retry must re-execute } else { - bucket.set(key, { kind: 'completed', outcome: result, completedAt: Date.now() }); - this.touch(method, key); + const entry: CompletedEntry = { + kind: 'completed', + outcome: result, + completedAt: Date.now(), + method, + key, + }; + bucket.set(key, entry); + this.lru.add(entry); + this.evictOverflow(); } return result; } @@ -172,23 +189,12 @@ class IdempotencyStore { return bucket; } - private lruKey(method: string, key: string): string { - return `${method}\0${key}`; - } - - /** Marks (method, key) most-recently-used, evicting the oldest completed entry once over the bound. */ - private touch(method: string, key: string): void { - const lruKey = this.lruKey(method, key); - this.lruOrder.delete(lruKey); - this.lruOrder.set(lruKey, { method, key }); - - if (this.lruOrder.size > REPLAY_CACHE_MAX_ENTRIES) { - const oldestKey = this.lruOrder.keys().next().value; - const oldest = oldestKey === undefined ? undefined : this.lruOrder.get(oldestKey); - if (oldestKey !== undefined && oldest !== undefined) { - this.lruOrder.delete(oldestKey); - this.byMethod.get(oldest.method)?.delete(oldest.key); - } + private evictOverflow(): void { + if (this.lru.size <= REPLAY_CACHE_MAX_ENTRIES) return; + const oldest = this.lru.values().next().value; + if (oldest !== undefined) { + this.lru.delete(oldest); + this.byMethod.get(oldest.method)?.delete(oldest.key); } } } From f474f29cb3981b87c77b5b5dbf24e0e443b11533 Mon Sep 17 00:00:00 2001 From: willbot Date: Tue, 21 Jul 2026 13:50:09 +0200 Subject: [PATCH 2/3] docs(rules): forbid control bytes in source, never as separators A NUL used as an LRU-key separator made a file read as binary and hid its entire diff on review. Record the rule so no agent reaches for a control byte again: compose keys with a data structure, not a delimited string; if a delimiter is unavoidable, use a printable character. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .agents/rules/no-control-bytes-in-source.mdc | 55 ++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 .agents/rules/no-control-bytes-in-source.mdc diff --git a/.agents/rules/no-control-bytes-in-source.mdc b/.agents/rules/no-control-bytes-in-source.mdc new file mode 100644 index 00000000..2945c4f8 --- /dev/null +++ b/.agents/rules/no-control-bytes-in-source.mdc @@ -0,0 +1,55 @@ +--- +description: Never put control bytes (NUL/0x00 and other non-printable C0 characters) in source, and never use one as a separator or delimiter. A NUL makes git treat the file as binary, so GitHub renders "Binary file not shown" and the entire diff is hidden — the file becomes unreviewable. Compose keys with a data structure, not a delimited string; if a delimiter is truly unavoidable, use an ordinary printable character. +alwaysApply: false +globs: ["**/*"] +--- + +# No control bytes in source — especially not as separators + +**Never** embed a NUL byte (`0x00`, `\0`) or any other non-printable control +character (C0 range `0x00`–`0x1F` except tab/newline/carriage-return, plus +`0x7F`) in a source file. This is not a style preference; it breaks tooling in +ways that are expensive to diagnose. + +The worst offender is using a control byte — most often NUL — as a **separator** +because "the parts can't contain it, so it can't collide." Do not do this. + +## Why a control byte is never worth it + +- **It makes the file unreviewable.** Git classifies any file containing a NUL + as binary. GitHub then shows `Binary file not shown` and hides the *entire* + diff — not just the offending line. A reviewer cannot see what changed, and an + approval means nothing. (This happened: a NUL separator in an LRU key hid a + whole file's diff on a PR.) +- **It breaks text tooling.** `grep`, editors, `less`, terminals, and log + pipelines all mishandle embedded control bytes — silently truncating, + corrupting, or misrendering. +- **A literal control byte in source is invisible.** It looks like a space or + nothing at all, so it survives review and lands. + +## What to do instead + +**Prefer a data structure over a delimited string.** Reaching for +`` `${a}${b}` `` to key a `Map` by a tuple is the smell that invites a +"clever" separator. Use a structure that needs no delimiter: + +- Nested maps: `Map>`. +- A `Set`/`Map` of entry objects where each entry carries its own fields + (`{ a, b, ... }`), so nothing has to be re-parsed out of a joined key. +- An object/tuple wrapper keyed by identity when reference semantics fit. + +**If a delimiter is genuinely unavoidable**, use an ordinary printable ASCII +character that cannot appear in either part, and state in a comment *why* it +cannot collide. A newline or a printable punctuation character is fine; a +control byte never is. Even the escaped form (`\0`) is banned in source — it +still produces a control byte at runtime and signals the same bad instinct. + +## Detecting it + +A file that should be text but reads as binary is the tell: + +```bash +git diff --numstat ...HEAD # a text file showing "- -" is binary to git +file path/to/suspect.ts # "data" instead of "… text" means a bad byte +LC_ALL=C grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]' path/to/file # locate control bytes +``` From 4308867fcd2f31f373556695d95097c6144f5eec Mon Sep 17 00:00:00 2001 From: willbot Date: Tue, 21 Jul 2026 13:53:05 +0200 Subject: [PATCH 3/3] docs(rules): NUL is a reserved sentinel, not a collision-free delimiter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrite the rule to lead with the actual reason, not the one symptom I hit: a NUL is the string terminator — a reserved control signal across the whole stack — so "it can't collide" is exactly backwards. It is collision-free only because every correct system refuses to put one in data, and it silently truncates or corrupts in every downstream layer it reaches (C strings, DB drivers, serializers, logs, transports); the hidden git diff is one instance, not the reason. Co-Authored-By: Claude Fable 5 Signed-off-by: willbot Signed-off-by: Will Madden --- .agents/rules/no-control-bytes-in-source.mdc | 73 +++++++++++--------- 1 file changed, 42 insertions(+), 31 deletions(-) diff --git a/.agents/rules/no-control-bytes-in-source.mdc b/.agents/rules/no-control-bytes-in-source.mdc index 2945c4f8..1e722e62 100644 --- a/.agents/rules/no-control-bytes-in-source.mdc +++ b/.agents/rules/no-control-bytes-in-source.mdc @@ -1,53 +1,64 @@ --- -description: Never put control bytes (NUL/0x00 and other non-printable C0 characters) in source, and never use one as a separator or delimiter. A NUL makes git treat the file as binary, so GitHub renders "Binary file not shown" and the entire diff is hidden — the file becomes unreviewable. Compose keys with a data structure, not a delimited string; if a delimiter is truly unavoidable, use an ordinary printable character. +description: A NUL byte (0x00) and other control characters are reserved sentinels — control signals with defined meanings across the entire computing stack (NUL is THE string terminator), not data. Never put one in source, and never, ever use one as a separator or delimiter. It silently truncates or corrupts in every downstream system it flows through — C strings, DB drivers, serializers, logs, terminals, transports, git. Compose keys with a data structure, not a delimited string. alwaysApply: false globs: ["**/*"] --- -# No control bytes in source — especially not as separators +# Control bytes are signals, not data — never use one as a separator -**Never** embed a NUL byte (`0x00`, `\0`) or any other non-printable control -character (C0 range `0x00`–`0x1F` except tab/newline/carriage-return, plus -`0x7F`) in a source file. This is not a style preference; it breaks tooling in -ways that are expensive to diagnose. +A NUL byte (`0x00`, `\0`) is not an unused character you can borrow. It is **the +string terminator** — a reserved sentinel with a defined meaning across the +whole computing stack: C strings, POSIX syscalls, filenames, most database +drivers, wire protocols, terminals, and git all treat NUL as "end of string" or +"end of record." The other C0 control characters (`0x00`–`0x1F` except +tab/newline/carriage-return, and `0x7F`) are signals too, not text. -The worst offender is using a control byte — most often NUL — as a **separator** -because "the parts can't contain it, so it can't collide." Do not do this. +Using one as data — most often as a **separator** — is the specific mistake this +rule exists to stop. -## Why a control byte is never worth it +## Why the "it can't collide" reasoning is exactly backwards -- **It makes the file unreviewable.** Git classifies any file containing a NUL - as binary. GitHub then shows `Binary file not shown` and hides the *entire* - diff — not just the offending line. A reviewer cannot see what changed, and an - approval means nothing. (This happened: a NUL separator in an LRU key hid a - whole file's diff on a PR.) -- **It breaks text tooling.** `grep`, editors, `less`, terminals, and log - pipelines all mishandle embedded control bytes — silently truncating, - corrupting, or misrendering. -- **A literal control byte in source is invisible.** It looks like a space or - nothing at all, so it survives review and lands. +The tempting logic is: "a method name / a UUID / a path can't contain a NUL, so +it's a collision-proof delimiter." That property is real, and it is the whole +problem. **A NUL is collision-free in your data precisely because every +correctly-built system refuses to put one there.** You are not finding a clever +unused byte; you are being the one component that violates an invariant the +entire rest of the stack relies on. + +And the byte does not stay in your code. It flows downstream, where its reserved +meaning takes over and silently corrupts: + +- **C-string boundaries** (and anything backed by them) truncate at the NUL — + your `"a\0b"` becomes `"a"`, losing data with no error. +- **Database drivers, serializers, and wire protocols** reject it, truncate it, + or mangle it — often only in production, only for some inputs. +- **Logs, terminals, and text tooling** (`grep`, editors, `less`) misrender or + drop it, so the corruption is invisible while you debug. +- **git** classifies any file containing a NUL as binary, so the diff renders as + `Binary file not shown` and the file becomes unreviewable — one concrete + instance of the general breakage, not the reason. + +None of these are worth a delimiter. There is no gain that offsets shipping a +reserved sentinel into data. ## What to do instead -**Prefer a data structure over a delimited string.** Reaching for +**Compose with a data structure, not a delimited string.** Reaching for `` `${a}${b}` `` to key a `Map` by a tuple is the smell that invites a -"clever" separator. Use a structure that needs no delimiter: +"clever" separator. Use a shape that needs no delimiter at all: - Nested maps: `Map>`. -- A `Set`/`Map` of entry objects where each entry carries its own fields - (`{ a, b, ... }`), so nothing has to be re-parsed out of a joined key. -- An object/tuple wrapper keyed by identity when reference semantics fit. +- A `Set`/`Map` of entry objects that each carry their own fields + (`{ a, b, ... }`), so nothing is re-parsed out of a joined key. +- Identity keys when reference semantics fit. -**If a delimiter is genuinely unavoidable**, use an ordinary printable ASCII -character that cannot appear in either part, and state in a comment *why* it -cannot collide. A newline or a printable punctuation character is fine; a -control byte never is. Even the escaped form (`\0`) is banned in source — it -still produces a control byte at runtime and signals the same bad instinct. +**If a delimiter is genuinely unavoidable**, use an ordinary printable character +that cannot appear in either part, and say in a comment why it cannot collide. A +control byte is never the answer — not raw, and not as the `\0` escape, which +produces the same byte at runtime and signals the same broken instinct. ## Detecting it -A file that should be text but reads as binary is the tell: - ```bash git diff --numstat ...HEAD # a text file showing "- -" is binary to git file path/to/suspect.ts # "data" instead of "… text" means a bad byte