refactor(memory-safety): close ratchet holes found in adversarial review

- fanout rule now matches the wrapped form the formatter actually produces
  (Promise.all/allSettled/any across two lines); the single-line regex missed
  the majority of real fan-outs.
- baseline keys on file:rule COUNT, so an already-baselined file can no longer
  add a second unbounded read of the same kind silently.
- bounded-by: now requires a non-empty rationale inside a comment; an escape
  that costs nothing gets pattern-matched into place without thought.
- corrected two hint strings that named exports which do not exist, and added a
  test asserting every hint names a real export so they cannot drift again.
- skip .generated. files so the baseline no longer depends on local build state.
- README: document the ratchet contract and the copy-per-update anti-pattern
  that caused the original crash but no lint rule can catch.
- BoundedMap.isSameKey uses Object.is; drop two set-after-get calls that are
  redundant now that get() marks MRU.

Co-authored-by: Orca <help@stably.ai>

config/scripts/check-bounded-ingress.mjs

@@ -19,8 +19,9 @@ const SKIP_PATH_PARTS = new Set([
 ])
 const BASELINE_PATH = 'config/bounded-ingress-baseline.json'
 
-// Each pattern names an unbounded way to pull data into memory.
-const PATTERNS = [
+// Each pattern names an unbounded way to pull data into memory. Hints must name real exports —
+// they are the text an agent copies when the ratchet blocks it (asserted in the test).
+export const PATTERNS = [
   {
     id: 'read-file',
     re: /\breadFile(?:Sync)?\s*\(/,
@@ -34,21 +35,26 @@ const PATTERNS = [
   {
     id: 'json-parse',
     re: /\bJSON\.parse\s*\(/,
-    hint: 'guard with assertJsonTextWithinStructureLimits (memory-safety/json-text-structure-limit)'
+    hint: 'guard with assertJsonTextStructureWithinLimits (memory-safety/json-text-structure-limit)'
   },
   {
     id: 'json-stringify',
     re: /\bJSON\.stringify\s*\(/,
-    hint: 'use stringifyJsonWithinLimit (memory-safety/node-bounded-json-stringify) when the value can be large'
+    hint: 'use stringifyJsonWithinByteLimit (memory-safety/node-bounded-json-stringify) when the value can be large'
   },
   {
     id: 'unbounded-fanout',
-    re: /Promise\.all\s*\(\s*[\w.]+\s*\.map\s*\(/,
+    // Why two lines joined: the formatter wraps `Promise.all(\n  items.map(` far more often than it
+    // keeps it on one, so a single-line regex misses the majority of real fan-outs.
+    re: /Promise\.(?:all|allSettled|any)\s*\(\s*[\w.[\]]+\s*\.map\s*\(/,
+    joinsNextLine: true,
     hint: 'use mapWithConcurrency so fan-out cannot grow with input size'
   }
 ]
 
-const ALLOW_RE = /bounded-by:/i
+// Why a non-empty rationale: an escape that costs nothing gets pattern-matched into place. Anchor to
+// a comment so a string literal containing the token cannot silence a real finding.
+const ALLOW_RE = /(?:\/\/|\/\*|\*)[^\n]*bounded-by:\s*\S/i
 
 export function findViolations(relativePath, source) {
   const lines = source.split('\n')
@@ -58,8 +64,19 @@ export function findViolations(relativePath, source) {
     if (ALLOW_RE.test(line) || (i > 0 && ALLOW_RE.test(lines[i - 1]))) {
       continue
     }
+    const joined = i + 1 < lines.length ? `${line} ${lines[i + 1].trim()}` : line
     for (const pattern of PATTERNS) {
-      if (pattern.re.test(line)) {
+      if (!pattern.joinsNextLine) {
+        if (pattern.re.test(line)) {
+          found.push({ file: relativePath, line: i + 1, id: pattern.id, hint: pattern.hint })
+        }
+        continue
+      }
+      // Report the wrapped form once, at the line that opens it.
+      if (
+        pattern.re.test(joined) &&
+        !(i > 0 && pattern.re.test(`${lines[i - 1]} ${line.trim()}`))
+      ) {
         found.push({ file: relativePath, line: i + 1, id: pattern.id, hint: pattern.hint })
       }
     }
@@ -75,6 +92,11 @@ function isSkipped(relative) {
   if (relative.includes('.test.') || relative.includes('.spec.') || relative.includes('.bench.')) {
     return true
   }
+  // Why: generated output is gitignored, so scanning it makes the baseline depend on whether the
+  // person regenerating it happens to have build artifacts present.
+  if (relative.includes('.generated.')) {
+    return true
+  }
   return relative.split('/').some((part) => SKIP_PATH_PARTS.has(part))
 }
 
@@ -114,21 +136,43 @@ export async function run(root, { updateBaseline = false } = {}) {
     violations.push(...findViolations(file.relative, source))
   }
 
-  const baselineFile = path.join(root, BASELINE_PATH)
-  if (updateBaseline) {
-    const keys = [...new Set(violations.map(violationKey))].sort()
-    await fs.writeFile(baselineFile, `${JSON.stringify({ allowed: keys }, null, 2)}\n`)
-    return { added: [], baselineSize: keys.length, updated: true }
+  // Why counts, not just keys: a file:rule key alone lets an already-baselined file add a SECOND
+  // unbounded read of the same kind silently — and follow-on work mostly touches baselined files.
+  const counts = {}
+  for (const v of violations) {
+    const key = violationKey(v)
+    counts[key] = (counts[key] ?? 0) + 1
   }
 
-  let allowed = new Set()
+  const baselineFile = path.join(root, BASELINE_PATH)
+  if (updateBaseline) {
+    const sorted = Object.fromEntries(Object.entries(counts).sort(([a], [b]) => (a < b ? -1 : 1)))
+    await fs.writeFile(baselineFile, `${JSON.stringify({ allowed: sorted }, null, 2)}\n`)
+    return { added: [], baselineSize: Object.keys(sorted).length, updated: true }
+  }
+
+  let allowed = {}
   try {
-    allowed = new Set(JSON.parse(await fs.readFile(baselineFile, 'utf8')).allowed)
+    const parsed = JSON.parse(await fs.readFile(baselineFile, 'utf8')).allowed
+    // Tolerate the original array form so the baseline can be regenerated, not hand-migrated.
+    allowed = Array.isArray(parsed)
+      ? Object.fromEntries(parsed.map((k) => [k, Number.POSITIVE_INFINITY]))
+      : (parsed ?? {})
   } catch {
     // No baseline yet: every occurrence is reported so the first run can seed one.
   }
-  const added = violations.filter((v) => !allowed.has(violationKey(v)))
-  return { added, baselineSize: allowed.size, updated: false }
+
+  const overBudget = new Set(Object.keys(counts).filter((key) => counts[key] > (allowed[key] ?? 0)))
+  const seen = new Set()
+  const added = violations.filter((v) => {
+    const key = violationKey(v)
+    if (!overBudget.has(key) || seen.has(key)) {
+      return false
+    }
+    seen.add(key)
+    return true
+  })
+  return { added, baselineSize: Object.keys(allowed).length, updated: false }
 }
 
 const invokedDirectly = process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href

config/scripts/check-bounded-ingress.test.mjs

@@ -1,5 +1,7 @@
+import { readFileSync } from 'node:fs'
+import path from 'node:path'
 import { describe, expect, it } from 'vitest'
-import { findViolations, violationKey } from './check-bounded-ingress.mjs'
+import { findViolations, violationKey, PATTERNS } from './check-bounded-ingress.mjs'
 
 describe('check-bounded-ingress', () => {
   it('flags an unbounded file read and JSON parse', () => {
@@ -42,4 +44,55 @@ describe('check-bounded-ingress', () => {
   it('ignores lines with no ingress pattern', () => {
     expect(findViolations('src/main/thing.ts', 'const total = a + b')).toEqual([])
   })
+
+  it('catches fan-out that the formatter wrapped onto the next line', () => {
+    const wrapped = findViolations(
+      'src/main/thing.ts',
+      ['await Promise.all(', '  items.map((i) => load(i))', ')'].join('\n')
+    )
+    expect(wrapped.map((v) => v.id)).toContain('unbounded-fanout')
+    for (const variant of ['Promise.allSettled', 'Promise.any']) {
+      expect(
+        findViolations(
+          'src/main/thing.ts',
+          `await ${variant}(\n  items.map((i) => load(i))\n)`
+        ).map((v) => v.id)
+      ).toContain('unbounded-fanout')
+    }
+  })
+
+  it('rejects an empty or non-comment bounded-by escape', () => {
+    // An escape that costs nothing gets pattern-matched into place without thought.
+    expect(
+      findViolations('src/main/thing.ts', 'const buf = await readFile(p) // bounded-by:')
+    ).toHaveLength(1)
+    expect(
+      findViolations('src/main/thing.ts', 'const label = "bounded-by: lol"; readFile(p)')
+    ).toHaveLength(1)
+    expect(
+      findViolations(
+        'src/main/thing.ts',
+        'const buf = await readFile(p) // bounded-by: stat-checked'
+      )
+    ).toEqual([])
+  })
+
+  it('every hint names a real export of the module it points at', () => {
+    // The hint is the text an agent copies when blocked; a stale name is an immediate import error.
+    const root = path.resolve(import.meta.dirname, '../..')
+    for (const pattern of PATTERNS) {
+      const match = /\b([a-zA-Z][\w]*)\s*\(memory-safety\/([\w-]+)\)/.exec(pattern.hint)
+      if (!match) {
+        continue
+      }
+      const [, fn, moduleName] = match
+      const source = readFileSync(
+        path.join(root, 'src/shared/memory-safety', `${moduleName}.ts`),
+        'utf8'
+      )
+      expect(source, `${pattern.id} hint names ${fn}`).toMatch(
+        new RegExp(`export (?:async )?function ${fn}\\b`)
+      )
+    }
+  })
 })

src/main/runtime/orca-runtime.ts

@@ -19535,7 +19535,6 @@ export class OrcaRuntimeService {
     const cacheKey = `${runtimeKey}::${repoPath}::${remote}`
     const cached = this.canonicalFetchKeyCache.get(cacheKey)
     if (cached !== undefined) {
-      this.canonicalFetchKeyCache.set(cacheKey, cached)
       return cached
     }
     let resolved = cacheKey
@@ -19577,7 +19576,6 @@ export class OrcaRuntimeService {
       return null
     }
     if (Date.now() - lastAt < FETCH_FRESHNESS_MS) {
-      this.fetchLastCompletedAt.set(key, lastAt)
       return lastAt
     }
     this.fetchLastCompletedAt.delete(key)

src/shared/memory-safety/README.md

@@ -23,16 +23,23 @@ can be wrong, and every ceiling is a new user-visible failure mode when it is hi
 
 If you cannot name the untrusted input and the bound it crosses, you do not need a limit.
 
+### Never copy an accumulator on every update
+
+The crash that started this (#9872) was not only an unbounded `Map` — it also rebuilt itself on every
+status ping (`state = { ...state, [key]: value }`). Copy-per-update turns a large collection into
+repeated large allocations, so it reaches the heap ceiling far sooner than its size suggests. Mutate
+in place, or use `BoundedMap`. No lint rule catches this; it is on you to notice.
+
 ## What's here
 
-| Module | Use it for |
-| --- | --- |
-| `bounded-map.ts` | A cache/map that must not grow forever. Count and/or byte ceiling with LRU eviction. |
-| `node-bounded-file-reader.ts` | Reading a file whose size you do not control. Checks before allocating and while growing. |
-| `node-bounded-json-stringify.ts` | Serializing to JSON when the result could be huge. |
-| `bounded-secure-json-file.ts` | Reading/writing a secret-bearing JSON file within a byte ceiling. |
-| `json-text-structure-limit.ts` | Guarding `JSON.parse` against depth/token amplification before it runs. |
-| `utf8-byte-limits.ts` | Measuring/truncating UTF-8 without materializing the whole string. |
+| Module                           | Use it for                                                                                |
+| -------------------------------- | ----------------------------------------------------------------------------------------- |
+| `bounded-map.ts`                 | A cache/map that must not grow forever. Count and/or byte ceiling with LRU eviction.      |
+| `node-bounded-file-reader.ts`    | Reading a file whose size you do not control. Checks before allocating and while growing. |
+| `node-bounded-json-stringify.ts` | Serializing to JSON when the result could be huge.                                        |
+| `bounded-secure-json-file.ts`    | Writing a secret-bearing JSON file within a byte ceiling.                                 |
+| `json-text-structure-limit.ts`   | Guarding `JSON.parse` against depth/token amplification before it runs.                   |
+| `utf8-byte-limits.ts`            | Measuring/truncating UTF-8 without materializing the whole string.                        |
 
 Domain-specific ceilings (e.g. `git-status-limit.ts`, `terminal-scrollback-limits.ts`) live next to
 the code they bound, not here. This folder is only for primitives reused across domains.
@@ -50,6 +57,28 @@ the code they bound, not here. This folder is only for primitives reused across
 5. **Test the exact boundary and boundary+1**, through every entry point — including hydration,
    restore, and replay paths, not just the create path.
 
+## The ratchet (`pnpm run check:bounded-ingress`)
+
+Runs in `pnpm lint`. It scans `src/` and `mobile/src/` for five unbounded-ingress patterns —
+`readFile`/`readFileSync`, `response.text()/.json()/.arrayBuffer()/.blob()`, `JSON.parse`,
+`JSON.stringify`, and `Promise.all|allSettled|any(x.map(...))` — and fails when a **file's count for
+a rule exceeds its baseline count** in `config/bounded-ingress-baseline.json`.
+
+Two ways to pass when it flags you:
+
+1. **Use a helper** from this folder. This is the default; the failure message names the exact export.
+2. **Justify it** with a `bounded-by:` comment on the line or the line above, naming what actually
+   bounds it — `// bounded-by: stat-checked < 1 MiB above`. An empty `bounded-by:` is rejected, and
+   the token only counts inside a comment.
+
+**Do not grow the baseline.** `--update-baseline` exists to _shrink_ it as unbounded reads are
+replaced, and to re-seed after a large refactor — not to admit new ones.
+
+What it does **not** enforce: `createReadStream`, `readdir`, recursion depth, copy-per-update
+amplification, and long-lived accumulator growth. Those are doctrine, not lint; the rules above still
+apply to them. `JSON.stringify` is in the list not as ingress but because serializing a large object
+transiently doubles it.
+
 ## Before you add one
 
 Prefer fixing the actual growth. The real crash was fixed by capping one map that was already known

src/shared/memory-safety/bounded-map.ts

@@ -39,7 +39,7 @@ function assertCeiling(name: string, value: number | undefined): void {
 // Why SameValueZero: Map key equality treats NaN as equal to NaN, but === does not; a === guard
 // would fail to protect a just-admitted NaN key and evict it after reporting success.
 function isSameKey<K>(a: K, b: K): boolean {
-  return a === b || (Number.isNaN(a as unknown as number) && Number.isNaN(b as unknown as number))
+  return a === b || Object.is(a, b)
 }
 
 export class BoundedMap<K, V> {