refactor(service-rpc): the idempotency LRU tracks entries, not a joined string

.agents/rules/no-control-bytes-in-source.mdc

@@ -0,0 +1,66 @@
+---
+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: ["**/*"]
+---
+
+# Control bytes are signals, not data — never use one as a separator
+
+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.
+
+Using one as data — most often as a **separator** — is the specific mistake this
+rule exists to stop.
+
+## Why the "it can't collide" reasoning is exactly backwards
+
+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
+
+**Compose with a data structure, not a delimited string.** Reaching for
+`` `${a}<sep>${b}` `` to key a `Map` by a tuple is the smell that invites a
+"clever" separator. Use a shape that needs no delimiter at all:
+
+- Nested maps: `Map<a, Map<b, V>>`.
+- 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 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
+
+```bash
+git diff --numstat <base>...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
+```

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<Outcome> }
-  | { 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<Outcome> } | CompletedEntry;
 
 /**
  * Per-method, per-key deduplication. A duplicate arriving mid-execution
@@ -124,8 +131,9 @@ type CacheEntry =
  */
 class IdempotencyStore {
   private readonly byMethod = new Map<string, Map<string, CacheEntry>>();
-  // 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<string, { readonly method: string; readonly key: string }>();
+  // 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<CompletedEntry>();
 
   async dispatch(method: string, key: string, run: () => Promise<Outcome>): Promise<Outcome> {
     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);
     }
   }
 }