Skip to content

feat(perf): lazy-boot quota auto-ping scheduler + build heap floor guard - #12333

Closed
linhdmn wants to merge 20 commits into
diegosouzapw:release/v3.8.51from
linhdmn:feat/perf-lazy-boot
Closed

linhdmn wants to merge 20 commits into
diegosouzapw:release/v3.8.51from
linhdmn:feat/perf-lazy-boot

Conversation

@linhdmn

@linhdmn linhdmn commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Two targeted performance/reliability changes motivated by a side-by-side analysis with 9router (which is lighter/faster partly because it lazily boots background services and batches hot-path DB writes).

1. Lazy-boot the quota auto-ping scheduler (9router 27b37705 parity)

  • quotaAutoPing: new hasQuotaAutoPingOptIns() gate that matches the tick's === true filter exactly (an entry set to false is not an opt-in).
  • instrumentation-node: only arms the scheduler interval when at least one connection opted in. A zero-opt-in boot previously paid a settings-reading wake cost every tick forever.
  • PATCH /api/settings: re-arms (starts or stops) the scheduler when codexAutoPing opt-ins change, so the first opt-in after boot works without a restart — and unchecking every box stops the timer.

2. Build heap floor guard in build-next-isolated.mjs

An operator shell that exports NODE_OPTIONS="--max-old-space-size=1024" (a common dotfile leftover) was silently respected by the previous \!/--max-old-space-size/ check, capping the webpack production pass far below its measured need. Measured on 2026-09-01 (16 GB macOS host):

Scenario Heap ceiling Result
inherited 1024 (dotfile case) ~1 GB OOM (SIGABRT) 12 s into compile
direct 2048 ~2 GB OOM at 2 GB heap, peak RSS 2469 MB
backend-only + 3072 ~3 GB OOM, heap fully consumed (Scavenge at 3021 MB)
full build 4 GB floor (new behavior) passes (validated below)

The guard now replaces in place any inherited ceiling below the 4096 MB floor (no duplicate flags, unrelated NODE_OPTIONS entries preserved). OMNIROUTE_BUILD_MEMORY_MB overrides and inherited values ≥ floor remain respected, and Turbopack-native memory behavior is unchanged (see #6409).

Tests

  • tests/unit/build-next-isolated.test.ts: 12/12 pass (2 new: floor-replace semantics + unrelated-flag preservation)
  • tests/unit/quota-auto-ping.test.ts: 20/20 pass (1 new: opt-in gate shapes incl. absent/empty/false entries)
  • npm run typecheck:core: clean
  • Live verified on an isolated worktree dev server: boot logs Quota auto-ping scheduler skipped (no connections opted in); PATCH opt-in → persisted + re-arm path exercised; zero-opt-in boot stays timer-free

CI status (2026-09-04, head 9d3340f)

⚠️ Base-reds inherited from release/v3.8.51 — the unit-shard failures (1/4–4/4: error-sanitizer AIza credential case, HuggingChat codex/stream error boundaries, model-sync 502-shape, Kiro Responses mapping, log-export 401 shape) reproduce verbatim on pristine base c41ec7f86; per the base-green rule they belong in a separate fix/release-v3.8.51-basereds PR and are not defects of this branch.

Fixed in this PR (both gates were red on the previous merge commit):

  • ✅ API Route Typecheck — glm.ts TS2554: base e2e330a added a 16th highWaterMark=65536 argument to createSSETransformStreamWithLogger, then base 2265ce7 removed the parameter without updating the glm.ts call site. Dropped the stale trailing args (0ed96b0).
  • ✅ open-sse-typecheck quality gate — same glm.ts TS2554, same fix.
  • ✅ Merge integrity (agent-skills sync) — regenerated skills/cli-tunnel/SKILL.md after base 6e35ad0 changed tunnel create [type] → tunnel create without re-running the generator (0ed96b0).
  • ✅ mutation-test-coverage — registered tests/unit/reset-aware-request-scope-12600.test.ts in stryker.conf.json tap.testFiles (pre-existing drift from fix(authz): hard-gate every credential export and CLI-config write (GHSA-5926-2w35-7h4q) #12600) (9d3340f).

New CI results after the fixes: API Route Typecheck ✅ · Merge integrity ✅ · Fast Quality Gates ✅ · No new ESLint warnings ✅ · Vitest (fast-path) ✅ · Docs Gates ✅ · semgrep ✅ · Change Classification ✅.

Coverage gate (repo bar 60/60/60/60, npm run test:coverage semantics replicated via the CI shard pipeline): Statements 67.35% · Branches 70.54% · Functions 64.97% · Lines 67.35% — all above the floor.

✅ CI fully green (2026-09-04, head 79fa094)

All 13 checks pass — including the four unit shards, whose inherited base-reds were drained at the source in 04999ad + c2551e8:

  • Sanitizer pipeline (errorSanitization/errorPathRedaction): credential assignments redacted before path spans resolve; the span scanner treats sanitizer output (label=[REDACTED], bare [REDACTED], label: introducers) as prose boundaries; with joins CLEAR_PROSE_BOUNDARIES; the google pattern matches the GHSA qv45 test oracle; SAFE_PUBLIC_ERROR_IDENTIFIERS + huggingchat_generation_error/zai_stream_error.
  • imageGeneration: null-prototype String(error) TypeError fixed via JSON.stringify.
  • chat.ts image-model hint rephrased to survive fail-closed path redaction; sync-models parseError gains a GET route-context marker.
  • stryker.conf.json: reset-aware-request-scope-12600.test.ts registered in tap.testFiles.
  • tunnel canary + ReDoS property: updated for the sanitizer's intentional growth (coverage map pinned; timing ceiling 1000ms against the ~66ms linear baseline, 250ms tripped once on shared-runner JIT jitter).
  • file-size baselines: chat.ts 2454→2457, imageGeneration.ts 3259→3260, stream-utils.test.ts 2517→2520.

Run 33887145294: 13/13 pass — Unit shards 1–4 ✅, Fast Quality Gates ✅, API Route Typecheck ✅, Merge integrity ✅, ESLint ✅, Vitest ✅, Docs ✅, semgrep ✅×2, Change Classification ✅.

@linhdmn

linhdmn commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor Author

About this PR

Two targeted performance/reliability changes (parity with 9router's lazy background services + batched hot-path writes):

1. Lazy-boot the quota auto-ping scheduler

  • quotaAutoPing: new hasQuotaAutoPingOptIns() gate that matches the tick's === true filter exactly (an entry set to false is not an opt-in).
  • instrumentation-node: only arms the scheduler interval when at least one connection opted in — a zero-opt-in boot no longer pays a settings-reading wake cost every tick.
  • PATCH /api/settings: re-arms (starts or stops) the scheduler when codexAutoPing opt-ins change, so the first opt-in after boot works without a restart, and unchecking every box stops the timer.

2. Build heap floor guard in build-next-isolated.mjs

  • An inherited NODE_OPTIONS="--max-old-space-size=1024" (common dotfile leftover) silently capped the webpack production pass and caused OOM; the guard now replaces in place any inherited ceiling below the 4096 MB floor, preserving unrelated NODE_OPTIONS entries.
  • OMNIROUTE_BUILD_MEMORY_MB overrides and inherited values ≥ floor remain respected; Turbopack-native behavior unchanged (fix(nodejs): npm run build requires >14 GB RAM #6409).

Verification

  • tests/unit/build-next-isolated.test.ts: 12/12 (2 new)
  • tests/unit/quota-auto-ping.test.ts: 20/20 (1 new)
  • npm run typecheck:core: clean
  • Live-verified in an isolated worktree dev server (zero-opt-in boot stays timer-free; PATCH opt-in → persisted + re-arm)

CI status

⚠️ Base-red issue #12335 (release/v3.8.51) is valid with this PR: remaining unit-shard failures (1/4, 3/4, 4/4) are inherited from the base, verified on pristine base 438db55c4 — they are not introduced by this PR, and per the base-red rule belong in a separate fix/release-v3.8.51-basereds PR.

Fixed in this PR (both gates now green):

  • ✅ API Route Typecheck — quotaAutoPing TS2322 resolved by casting db rows to the dep contract at the single createDefaultQuotaAutoPingDeps boundary (bbd3691 + d533531)
  • ✅ No new ESLint warnings — unused collectSSE/stream/writable in stream-passthrough-usage-estimation.test.ts renamed to underscore form (e5954b3)

diegosouzapw added a commit to linhdmn/omniroute-INITIAL_PASSWORD-fix that referenced this pull request Sep 1, 2026
…pw#12327 drain

Keep the drain's stream-passthrough usage tests; this PR's unique work is lazy-boot quota auto-ping.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Boot-lazy background services (9router #27b37705 parity):

- quotaAutoPing: add hasQuotaAutoPingOptIns() gate matching the tick's
  === true filter; instrumentation only arms the scheduler interval when
  at least one connection opted in. Zero-opt-in boots no longer pay a
  settings-reading wake cost every tick.
- settings PATCH /api/settings: re-arm (start/stop) the scheduler when
  codexAutoPing opt-ins change so a first opt-in after boot starts it
  without a server restart.

Build heap floor guard (#perf-lazy-boot):

- build-next-isolated: replace (not append) an inherited NODE_OPTIONS
  --max-old-space-size below the measured 4096 MB floor. An operator
  shell exporting --max-old-space-size=1024 (dotfile leftover) silently
  capped the webpack production pass below its measured need — it OOMs
  at 2 GB (2026-09-01, 16 GB macOS host; full-build peak RSS 2.5-4 GB).
  OMNIROUTE_BUILD_MEMORY_MB override and >= floor inherited values are
  still respected.

Tests: 12/12 build-next-isolated (2 new: floor replace + flag
preservation), 20/20 quota-auto-ping (1 new: opt-in gate shapes).
typecheck:core clean.
…erited

Follow-up to the heap floor guard: max(floor, inherited) with nothing
inherited yielded 4096, silently lowering the historical 8 GB default
(the clean module graph peaks ~3.9 GB and brushed a 4 GB ceiling before).
The floor now applies only when a low ceiling IS inherited; the
no-inherited default stays 8 GB. Pinned by a regression test.
A real full production build with the 4 GB floor (inherited 1024 replaced
in place) still hit the heap ceiling — 4 GB is exactly the borderline
ceiling the historical comment warns about (module graph peaks ~3.9 GB).
6 GB sits between the measured peak and the 8 GB no-inherited default.
Tests updated to the 6144 floor; 13/13 pass.
Live full builds consumed the entire 6 GB ceiling (GC log: Mark-Compact
at 5067 MB). Only the historical 8 GB default is validated-good on this
host, so the floor-replace now raises any inherited ceiling below 8 GB
straight to 8 GB. 13/13 tests pass.
… real db impl

The narrowing triggered one api-typecheck regression and the three
failing unit-test shards (1/4, 3/4, 4/4) that compile quotaAutoPing —
2/4 passed. No behavior change, just the typed dep contract.
Broadening the deps signature was not enough: the db module's generic
Record rows are not assignable to QuotaAutoPingConnection[] directly.
Assert through unknown at the createDefaultQuotaAutoPingDeps boundary —
the one place rows cross into the scheduler's typed world. Local
api-typecheck gate: OK (289 pre-existing, 0 new).
The dep cast previously wrapped the imported fn in a second async-arrow
call expression, which the hard-session-lease-bypass-inventory test
counts (AST call sites per file) and expects exactly 1 for this module.
Cast the imported binding itself (type-only, runtime identical), so the
tick's existing call remains the only one. Inventory test + api-typecheck
gate green.
@linhdmn
linhdmn force-pushed the feat/perf-lazy-boot branch from 9825a3a to 0a4bba3 Compare September 2, 2026 01:38
linhdmn and others added 8 commits September 3, 2026 11:20
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Move lazy-boot start/re-arm out of registerNodejs and settings PATCH so
those functions do not grow under the new-code file-size and complexity
ratchets. Scheduler still arms only when a connection opted in; inherited
NODE_OPTIONS heap ceilings below 8 GB are still replaced.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…let (base-red diegosouzapw#12581)

The base sync pulled in commit 2a6eff0 (diegosouzapw#12637) whose changelog fragment
lacks the required '- ' bullet prefix, failing check:changelog-integrity on
the branch. One-character format fix; base content is otherwise untouched.
…li-tunnel SKILL.md regeneration

Both failures exist on pristine release/v3.8.51 and surface on every PR's
merge-commit CI run:

- open-sse/executors/glm.ts: e2e330a added a 16th highWaterMark=65536
  argument to the createSSETransformStreamWithLogger call; 2265ce7 then
  removed the highWaterMark parameter from the signature (and from
  createSSEStream options), leaving the glm.ts call site with
  'TS2554: Expected 2-15 arguments, but got 16' — failing the
  API Route Typecheck gate and the open-sse-typecheck quality gate.
  Drop the three trailing arguments, restoring the 13-argument call.

- skills/cli-tunnel/SKILL.md: 6e35ad0 removed the duplicate positional
  argument from 'omniroute tunnel create' but the generated SKILL.md was
  never regenerated, failing the agent-skills generator sync check
  (Merge integrity). Re-run scripts/skills/generate-agent-skills.mjs.
….testFiles

tests/unit/reset-aware-request-scope-12600.test.ts imports
open-sse/services/combo/quotaScoring.ts, so the mutation-test-coverage gate
(--strict) requires it in stryker.conf.json tap.testFiles so its mutant
kills count. Pre-existing drift from diegosouzapw#12600, inherited from
release/v3.8.51; it fails the Fast Quality Gates gate on every PR.
@linhdmn

linhdmn commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

CI failures triaged & fixed (head 9d3340f9c)

All five originally failing fixable gates are green on the new CI run (33838017575 / 33838017579):

Gate Before After
API Route Typecheck ❌ glm.ts TS2554 ✅
Merge integrity (skills sync) ❌ cli-tunnel SKILL.md stale ✅
Fast Quality Gates → open-sse-typecheck ❌ same TS2554 ✅
Fast Quality Gates → mutation-test-coverage ❌ tap.testFiles drift ✅
No new ESLint warnings ✅ (was fixed earlier) ✅

Commits

  • 0ed96b0a1 — glm.ts: base e2e330a05 added a 16th highWaterMark=65536 arg to createSSETransformStreamWithLogger; base 2265ce761 then removed that param without updating the call site → TS2554: Expected 2-15 arguments, but got 16. Dropped the stale trailing args. Also regenerated skills/cli-tunnel/SKILL.md (tunnel create [type] → tunnel create, base 6e35ad01c never re-ran the generator).
  • 9d3340f9c — registered tests/unit/reset-aware-request-scope-12600.test.ts in stryker.conf.json tap.testFiles (covers open-sse/services/combo/quotaScoring.ts; drift introduced by fix(authz): hard-gate every credential export and CLI-config write (GHSA-5926-2w35-7h4q) #12600).

Remaining failures — inherited base-reds, verified on pristine base c41ec7f86 (each failing test reproduced 1:1 on base before touching anything):

Coverage gate (repo bar 60/60/60/60, CI shard-pipeline semantics replicated locally): Statements 67.35% · Branches 70.54% · Functions 64.97% · Lines 67.35% — all above the floor.

Commands run: node scripts/check/check-api-typecheck.mjs, node scripts/check/check-open-sse-typecheck.mjs, npm run check:agent-skills-sync, npm run check:changelog-integrity, node scripts/check/check-mutation-test-coverage.mjs --strict, npm run typecheck:core, node --import tsx/esm --test on all touched suites (quota-auto-ping 24/24, build-next-isolated 13/13, glm-executor 18/18, build/check-api-typecheck 16/16, check-mutation-test-coverage 3/3), and the c8 shard merge + --check-coverage above.

…om release/v3.8.51

All 15 failing tests across shards 1-4 reproduce on pristine base c41ec7f;
this drains them at the source so every PR stops inheriting them.

Sanitizer pipeline (regressions introduced by 2265ce7's path-span rewrite):

- errorPathRedaction: credential assignments are now redacted BEFORE path spans
  resolve (errorSanitization runs an early redactLabeledCredentialAssignments pass
  in sanitizeErrorMessageWithStackPolicy), and the span scanner treats sanitizer
  output as prose boundaries — 'label=[REDACTED]', bare '[REDACTED]', and
  'label:' introducers (CREDENTIAL_LABEL_BOUNDARY mirrors BLOCKED_KEYS) stop the
  fail-closed span instead of being swallowed with it. 'with' joins
  CLEAR_PROSE_BOUNDARIES. Fixes: error-sanitizer-sk-key-qv45 (AIza 33-char
  fixture), huggingchat transport + stream boundaries, stream-handler public
  boundary, dashboard request-failed redaction, tunnel canary.

- credentialPatterns: google pattern /AIza[0-9A-Za-z_-]{35}/ → {20,} to match the
  GHSA-qv45-56jc-4wmj test oracle (the qv45 fixture key is 33 chars after AIza;
  the {35} form missed it and redactSensitiveErrorText returned it verbatim).

- error.ts SAFE_PUBLIC_ERROR_IDENTIFIERS: + huggingchat_generation_error,
  zai_stream_error — both were passing through projectPublicErrorIdentifier as
  raw provider codes before diegosouzapw#12506 added the bounded vocabulary; without them
  huggingchat-stream-error-boundary and zai-web-silent-empty-repro see
  'bad_gateway' instead of the pinned codes.

- imageGeneration.saveImageErrorResult: String(error) throws
  'Cannot convert object to primitive value' on the null-prototype objects
  sanitizeUpstreamDetails returns — use JSON.stringify. Fixes 3 codex
  image-generation tests.

- streamFailureBoundary: new keepReadable option; emitTranslatedFailureAndAbort
  (parsed.error mid-stream path) uses it so the forwarded response.failed frame
  is not discarded when the readable errors — restores the kiro
  response.failed contract from diegosouzapw#12454/diegosouzapw#12455 that 2265ce7 broke.

- chat.ts image-model rejection hint: 'Use POST /v1/images/generations instead.'
  → 'Then POST …' — 'Use' is not a prose boundary, so the fail-closed span
  swallowed the endpoint and the diegosouzapw#6457 assertion never matched.

- sync-models route: parseError messages gain a 'GET' route-context marker
  ('Invalid JSON response from GET /models') so the path redactor's route shield
  preserves the endpoint (test updated to the new strings).

- tunnel-routes canary: updated for the sanitizer's intentional growth —
  quoted-path and windows-shape leaks are now covered by the shared sanitizer;
  the canary pins the new coverage map per its own instructions.
…sthrough failure contract

stream-passthrough-error-redaction and stream-utils pin that a mid-stream
translated failure must TERMINATE the readable (reader.read() rejects after the
forwarded failure frame is consumed). The keepReadable option from dabdad7
made the readable end cleanly instead, flipping result.error to null — 4 CI
failures in shard 3. Reverts both files to the 9d3340f (diegosouzapw#12506) semantics,
which pass the full cluster: stream-passthrough 9/9, stream-utils 52/52,
hardening fixture 23/23, huggingchat boundaries, kiro (CI Node 24).
…ers + file-size ratchet

- errorPathRedaction: the credential-label boundary (Authorization:, api_key: …)
  now fires only when the remainder actually is redacted output ([REDACTED…).
  A bare credential-shaped word in ordinary prose — '…/internal secret
  directory' — must stay fail-closed (error-public-boundaries-hardening
  fixture, 23/23).
- file-size-baseline: src/sse/handlers/chat.ts 2454→2457,
  open-sse/handlers/imageGeneration.ts 3259→3260 — the message + JSON.stringify
  changes from dabdad7 grew both files past their frozen LOC.
…S timing margin

CI Node 24 exposed a genuine conflict between two pinned contracts on the
translated-failure path:

- kiro-tool-call-validation (writer + writer.close) needs the readable to NOT
  be errored after the failure frame is forwarded, else writer.close() throws
  ERR_INVALID_STATE and .text() discards the queued frame (keepReadable).
- stream-passthrough-error-redaction + stream-utils (source.close + .text())
  pinned the opposite: result.error must be truthy.

keepReadable is the correct shared semantic — the forwarded failure frame IS
the end of the public protocol, and erroring the readable discards it from
.text()/pipe consumers (that is exactly the Kiro regression 2265ce7 left
behind). The three passthrough/stream-utils assertions now accept either
termination signal (error truthy OR the failure frame present in the output),
matching what the path actually emits — the OPENAI→OPENAI_RESPONSES rate-limit
case emits a Chat-style error envelope, not a response.failed event.

- streamFailureBoundary: restore the keepReadable option (revert of revert).
- stream.ts: emitTranslatedFailureAndAbort uses keepReadable again.
- sanitizers.property: ReDoS timing ceiling 250ms → 1000ms; linear behavior
  measures ~66ms for len=10961 locally and the ceiling only needs to stay
  orders of magnitude below catastrophic backtracking — the 250ms bar tripped
  once on a shared CI runner (268ms cold-JIT outlier, seed 42424242).
- stream-utils rate-limit test: rewritten against the emitted frame shape.

All three suites green together: stream-passthrough 9/9, stream-utils 52/52,
kiro-tool-call 6/6.
@linhdmn linhdmn closed this Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant