Skip to content

fix(sse): honor comment opt-out for final metadata (#9305) - #9378

Merged
diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.50from
xiaoyaner0201:fix/9305-sse-comments-optout
Aug 13, 2026
Merged

diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.50from
xiaoyaner0201:fix/9305-sse-comments-optout

Conversation

@xiaoyaner0201

Copy link
Copy Markdown
Contributor

Summary

  • Make OMNIROUTE_SSE_COMMENTS consistently control both comment-shaped heartbeats and final : x-omniroute-* SSE metadata.
  • Accept conventional disabled spellings (off, false, 0, no) with surrounding whitespace and mixed case while preserving default-on behavior.
  • Preserve data-shaped heartbeats, ordinary chunks, synthetic finish output, [DONE], usage/accounting, callbacks, cleanup, logging order, and non-SSE metadata headers.

Related Issues

Validation

  • Change type: other (SSE stream behavior)
  • Focused tests and category gates from the golden path
  • npm run lint
  • Reconciled with the current active release base; focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR
  • SonarQube PR analysis is green or any remaining issues are explicitly documented below (pending CI)

Exact published candidate: fde6637d03537733415431122cab52a3342f616e (tree 47f4f1bd008d6171d28990dfa1efecafb9152ec1) on live release/v3.8.50 base 7163081f5ed2a2104e85c31bfd1588033d43c580.

Commands run on the exact clean tree:

  • node --import tsx/esm --test tests/unit/sseHeartbeat.test.ts tests/unit/sse-comments-optout-9305.test.ts — 10/10 pass.
  • Related heartbeat/metadata/stream/accounting/error suites — 46/46 pass.
  • Focused chatCore metadata/header test — 1/1 pass.
  • Process-unavailable edge probe — pass.
  • npm run test:security — 23/23 pass.
  • Changed-file Prettier and ESLint — pass.
  • npm run lint — pass.
  • npm run typecheck:core — pass.
  • npm run check:cycles — pass.
  • npm run check:build-scope — pass.
  • git diff --check — pass.
  • npm run check:secrets — exited 0 but skipped because gitleaks is not installed.

The shell uses Node 26 while the hard-linked better-sqlite3 binary targets an older ABI; focused DB-backed tests reported that mismatch, then successfully used the configured SQLite fallback with isolated temporary DATA_DIR paths. Full unit shards, coverage, Vitest, and production build are left to CI per the contribution golden path.

Tests Added Or Updated

  • tests/unit/sse-comments-optout-9305.test.ts
  • tests/unit/sseHeartbeat.test.ts

Coverage Notes

The focused tests cover both upstream [DONE] and natural-EOF finalization, disabled/enabled parsing, comment-only suppression, stream ordering, usage/accounting callbacks, cleanup, and data-heartbeat preservation.

Reviewer Notes

  • Diff: 4 files, +261/-42; production delta is one shared boolean-policy expansion and one final-comment emission guard.
  • No dependency, lockfile, generated-file, schema, migration, database, API-route, provider, UI, release, or package-version changes.
  • Architecture risk: low and localized to shared SSE comment emission. Security risk: low; no auth, secret, input-validation, or external-request surface changed.
  • This PR intentionally remains Draft. CI, merge-train eligibility, and the maintainer decision are pending.

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Review: PR #9378 — fix(sse): honor comment opt-out for final metadata

Verdict: merge-ready (5/5 stars)

Summary

The PR fixes issue #9305: OMNIROUTE_SSE_COMMENTS=off was supposed to suppress all SSE comment lines, but it only suppressed heartbeat keepalives (: keepalive). The final : x-omniroute-* metadata at stream end was still emitted. Two small changes fix this:

  1. open-sse/utils/sseHeartbeat.ts (+1/-1): Expand sseCommentsEnabled() to accept "off", "false", "0", "no" (case-insensitive, whitespace-trimmed)
  2. open-sse/utils/stream.ts (+3/-1): Guard emitFinalSseMetadata() to skip final metadata comment when comments are disabled

Quality

  • Test coverage: 10 focused cases (all 4 disabled spellings × [DONE] and natural-EOF) + 46 related heartbeat/metadata/stream tests, all self-reported passing
  • Edge cases: Draft state close, non-comment-shaped heartbeat : ping preserved, no duplication, x-omniroute-usage and x-omniroute-accounting reporting preserved
  • No base-red regression: none of the 4 inherited failures are touched

Next steps

  • Disengage Draft status
  • Let CI run
  • Merge into release/v3.8.50

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw
diegosouzapw marked this pull request as ready for review August 13, 2026 10:54
@diegosouzapw
diegosouzapw self-requested a review as a code owner August 13, 2026 10:54
@diegosouzapw
diegosouzapw merged commit 9a4cca4 into cheaper-inference:release/v3.8.50 Aug 13, 2026
12 of 13 checks passed
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Merged into release/v3.8.50 — thank you @xiaoyaner0201 for the contribution! It was validated on a combined merge-train (static gates + affected tests + vitest) together with 30 sibling PRs before landing.

diegosouzapw pushed a commit that referenced this pull request Aug 13, 2026
Drains the HARD failures reported by Release-Green run 31693210948 on issue #9985
(ESLint errors: 2) plus the merge-integrity red every open PR is inheriting.

- ESLint error 1: @omniroute/opencode-plugin/src/index.ts had a stray extra
  '});' (introduced by #9316) that broke parsing with 'unexpected file in NFT list'
  on the build path.
- ESLint error 2: cli-env-inline-comment-10100.test.ts used new Function to extract
  parseEnvValue from the bin entrypoint (no-new-func, Hard Rule #3). Extracted the
  helper to bin/cli/utils/parseEnvValue.mjs and import it from both the entrypoint
  and the test (same behavior, no eval).
- open-sse-typecheck (Fast Quality Gates): open-sse/utils/stream.ts imported
  sseCommentsEnabled twice (#9378) causing TS2300 Duplicate identifier; removed the
  duplicate import.
- Merge integrity (changelog + generated skills): skills/omni-settings/SKILL.md was
  edited manually by #10169 without updating the generator source, so
  check:agent-skills-sync failed on every PR (Generated: 1). Moved the curated
  thinking-budget content into a <!-- skill:custom-start --> block (the documented
  preservation mechanism), which the generator now keeps in sync.

Refs #9985
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Aug 13, 2026
open-sse/utils/stream.ts imported sseCommentsEnabled from ./sseHeartbeat.ts
twice (line 31 and line 77, from cheaper-inference#9378), causing a Turbopack/TS2300
duplicate-identifier build error on every request through the Middleware
bundle. Pre-existing on the current upstream/release/v3.8.50 tip (already
tracked/fixed upstream in cheaper-inference#10252 and cheaper-inference#10260, neither merged yet); backported
here since the last rebake pulled this tip directly into dev-combined,
breaking the live omniroute-dev instance.
diegosouzapw added a commit that referenced this pull request Aug 14, 2026
* fix(ci): clear base-reds on release/v3.8.50 (round 4)

Drains the HARD failures reported by Release-Green run 31693210948 on issue #9985
(ESLint errors: 2) plus the merge-integrity red every open PR is inheriting.

- ESLint error 1: @omniroute/opencode-plugin/src/index.ts had a stray extra
  '});' (introduced by #9316) that broke parsing with 'unexpected file in NFT list'
  on the build path.
- ESLint error 2: cli-env-inline-comment-10100.test.ts used new Function to extract
  parseEnvValue from the bin entrypoint (no-new-func, Hard Rule #3). Extracted the
  helper to bin/cli/utils/parseEnvValue.mjs and import it from both the entrypoint
  and the test (same behavior, no eval).
- open-sse-typecheck (Fast Quality Gates): open-sse/utils/stream.ts imported
  sseCommentsEnabled twice (#9378) causing TS2300 Duplicate identifier; removed the
  duplicate import.
- Merge integrity (changelog + generated skills): skills/omni-settings/SKILL.md was
  edited manually by #10169 without updating the generator source, so
  check:agent-skills-sync failed on every PR (Generated: 1). Moved the curated
  thinking-budget content into a <!-- skill:custom-start --> block (the documented
  preservation mechanism), which the generator now keeps in sync.

Refs #9985

* fix(tests): align wave1-a poolside test with #10216 probed catalog

#10216 published Poolside's two authenticated-probe models
(poolside/laguna-xs-2.1, poolside/laguna-s-2.1) as static seeds, but the
wave1-a free-tier test still asserted 'no invented static model ids'
(entry.models === []), failing every open PR. Separate poolside from the
empty-models assertion and pin its probed catalog explicitly so a future
catalog change is a deliberate update, not a silent drift.

* fix(pack): register parseEnvValue.mjs in PACK_ARTIFACT_REQUIRED_PATHS

The extract of parseEnvValue to bin/cli/utils/parseEnvValue.mjs added a new
direct import to bin/omniroute.mjs, which pack-artifact-entrypoint-closures
enforces against PACK_ARTIFACT_REQUIRED_PATHS. Register the module so a future
tarball omission fails loudly.

* fix(combo): restore default same-model retry semantics after #10217

#10217 wired config.failoverBeforeRetry into the same-model retry guard in
both the priority/auto and round-robin loops, but DEFAULT_COMBO_CONFIG
defaulted the flag to true — flipping same-model retry off for every combo
that never touched the setting, not just the opt-in case. Round-4 bisect
(06f41cd vs d2fd88d) reproduced this against
tests/unit/combo-499-abort.test.ts, tests/unit/combo-quota-exhaustion-only-fallback.test.ts
and tests/unit/combo-stream-readiness-fallback.test.ts. Flip the default to
false so the historical retry-before-failover behavior returns for combos
that never set the flag, while explicit opt-in (the two new tests #10217
added to combo-routing-engine.test.ts) still works.

* fix(quality): register visionBridge-responses-9597 in stryker tap.testFiles

check-mutation-test-coverage.mjs flagged tests/unit/guardrails/visionBridge-responses-9597.test.ts
as covering open-sse/services/combo/comboStructure.ts without being listed
in stryker.conf.json's tap.testFiles array. Add it so mutation coverage
attribution stays accurate.

* test(pack): expect parseEnvValue.mjs in the missing-artifact-paths fixture

The prior commit on this branch registered bin/cli/utils/parseEnvValue.mjs
in PACK_ARTIFACT_REQUIRED_PATHS but the "findMissingArtifactPaths flags
missing root runtime files in the tarball" test still hardcoded the old
expected list, so it never accounted for the new required path being
absent from the simulated tarball. Add it in its alphabetical slot.

* chore(lint): prune stale no-explicit-any suppression for call-log-file-rotation

--prune-suppressions found tests/unit/call-log-file-rotation.test.ts no
longer produces the 5 suppressed @typescript-eslint/no-explicit-any
warnings recorded in config/quality/eslint-suppressions.json. Remove the
dead entry so a regression would be caught again. Full-tree run with
--max-warnings 0 is clean: 0 errors, 0 warnings.

* fix(combo): decouple failoverBeforeRetry same-model guard from the skipUpstreamRetry default

Audit found that DEFAULT_COMBO_CONFIG.failoverBeforeRetry has defaulted to
true since before #10217 (predates #2417), and that value also feeds the
independent skipUpstreamRetry mechanism (src/sse/handlers/chat.ts:859,1126).
The previous commit on this branch flipped that default to false to fix the
#10217 same-model retry guard, which silently disabled skipUpstreamRetry's
own default-on behavior for every combo without an opt-in — a regression in
the opposite direction (executor-level retries before the loop's own
failover, changing latency/failure behavior).

Revert the default back to true and decouple the two mechanisms instead:
resolveComboConfig/resolveComboSetupConfig now also compute
failoverBeforeRetryExplicit, true only when a cascade layer (combo/provider/
global) literally sets failoverBeforeRetry to true — not merely inherited
from the default. The #10217 same-model retry guards in combo.ts (priority/
auto and round-robin loops) now read failoverBeforeRetryExplicit instead of
config.failoverBeforeRetry, restoring opt-in-only behavior for that guard
while the skipUpstreamRetry pass-through (config.failoverBeforeRetry at
combo.ts:1297,2865) is untouched and keeps its historical default-on.

* fix(combo,i18n): align getDefaultComboConfig with 10217 explicit flag; pt denoRelay entities

Two round-4 follow-ups exposed by the combinated base-red PR run:

1. comboConfig.ts: #10217 round-4 fix (104afed) added
   failoverBeforeRetryExplicit to resolveComboConfig/resolveComboSetupConfig
   but getDefaultComboConfig() returned only DEFAULT_COMBO_CONFIG, so the
   combo-config.test.ts deepEqual (resolveComboConfig(null) ===
   getDefaultComboConfig()) failed on the extra field. Mirror the opt-in flag
   as false in the default.

2. pt.json: denoRelayOrgDomainHint still carried raw <app-name>/<org-slug>
   (the UNCLOSED_TAG RSC regression) — encode as &lt;...&gt; like the other
   42 locales, greening i18n-deno-relay-unclosed-tag.test.ts.

* chore(lint): disable @next/next/no-location-assign-relative-destination pending per-case review (#10292)

The eslint-config-next bump in #10043 shipped this new rule, flagging 6
pre-existing window.location.href navigations — several are deliberate
full-page reloads (login/logout state reset). Off with tracking issue
rather than a blanket router.push rewrite.

* fix(i18n): fill 439 missing UI keys (thinkingMode ×39 locales + pt catch-up) to restore 100% coverage

The #10169 Thinking Budget keys existed only in en/pt-BR/vi and the pt (PT-PT)
catalog from #10250 lagged 88 recent keys, dropping i18nUiCoverage to 99.3%
vs the frozen 100% ratchet baseline. Translated via the i18n:sync-ui marker
pipeline; glossary + ICU placeholder post-pass clean.

* fix(i18n): zh-TW glossary — replace retired 默認 with canonical 預設 in new thinkingMode keys

* chore(quality): rebase dead-code baseline 248 -> 409 for knip 6.32 bump (#10043)

dependabot #10043 upgraded knip 6.27 -> 6.32, which detects 162 MORE
genuinely-unused exports (331 vs 169) that 6.27 missed; DEAD_FILES
unchanged (78). Reproduced identically on the clean release/v3.8.50 tip
266e39d with a fresh 6.32 node_modules, so every PR is born red until
the tool change is absorbed. Owner authorized rebaseline (2026-08-13 via
PR #10260). Structural cleanup of the newly-surfaced dead exports remains
separate debt.

---------

Co-authored-by: adevwithpurpose <adevwithpurpose@users.noreply.github.com>
Co-authored-by: backryun <bakryun0718@proton.me>
diegosouzapw pushed a commit that referenced this pull request Aug 14, 2026
…rfaces

Both defects these guards cover were fixed on the base while this branch
was open, so this PR is now purely the regression guards:

- stream-imports-no-duplicates.test.ts: fails on duplicate import bindings
  in open-sse/utils/stream.ts. The #9378 merge left sseCommentsEnabled
  imported twice; tsx/esbuild dedupe it silently, so only the webpack
  release build caught it.
- opencode-plugin-parses.test.ts: parses every @omniroute/opencode-plugin
  source with the TypeScript compiler. The plugin is a standalone package
  (outside typecheck:core and both runners), so the doubled '});' from the
  #9316 merge only surfaced at tsup DTS time.

Both broke the release build on the same day, in surfaces no gate reads
until publish time. Verified failing on the pre-fix trees and passing on
the current base.
diegosouzapw added a commit that referenced this pull request Aug 14, 2026
…rfaces (stream.ts + opencode-plugin) (#10318)

* fix(sse): remove duplicate sseCommentsEnabled import that breaks the production build

The #9378 merge auto-resolve left open-sse/utils/stream.ts importing
sseCommentsEnabled from sseHeartbeat.ts twice (lines 31 and 77). tsx/esbuild
(typecheck + both test runners) silently dedupe the binding, but webpack
fails the release build with "Identifier 'sseCommentsEnabled' has already
been declared" — this is the open-sse-typecheck / build base-red on
release/v3.8.50.

Adds a static regression guard (tests/unit/stream-imports-no-duplicates.test.ts,
RED on the duplicate, GREEN after) so the next merge auto-resolve of this
hot file fails in the unit suite instead of at release-build time. Also
includes the prettier canonicalization of the three style drifts the same
merge introduced (applied by lint-staged either way).

Validated: webpack release build passes on this tree (192.168.0.113 build box).

Refs #9985

* fix(opencode-plugin): remove doubled '});' that breaks the release build

The #9316 merge auto-resolve left a duplicated '});' at
@omniroute/opencode-plugin/src/index.ts:5481. The plugin is a standalone
package (outside typecheck:core and both test runners), so nothing parses
it until the release build — where tsup's DTS step fails with a cascade of
"Cannot find name" errors. The same syntax error also blocked prettier
from parsing the file, so this commit necessarily carries the prettier
pass lint-staged applies on staging (formatting was frozen since the bad
merge).

Adds tests/unit/opencode-plugin-parses.test.ts: parses every plugin source
file with the TypeScript compiler and fails on syntax diagnostics, so the
next merge-resolve accident in this uncovered package dies in the unit
suite instead of at release-build time.

Validated: release build:cli passes on the 192.168.0.113 build box with
this hotfix applied.

Refs #9985

* test(build): guard against merge auto-resolve damage in build-only surfaces

Both defects these guards cover were fixed on the base while this branch
was open, so this PR is now purely the regression guards:

- stream-imports-no-duplicates.test.ts: fails on duplicate import bindings
  in open-sse/utils/stream.ts. The #9378 merge left sseCommentsEnabled
  imported twice; tsx/esbuild dedupe it silently, so only the webpack
  release build caught it.
- opencode-plugin-parses.test.ts: parses every @omniroute/opencode-plugin
  source with the TypeScript compiler. The plugin is a standalone package
  (outside typecheck:core and both runners), so the doubled '});' from the
  #9316 merge only surfaced at tsup DTS time.

Both broke the release build on the same day, in surfaces no gate reads
until publish time. Verified failing on the pre-fix trees and passing on
the current base.

---------

Co-authored-by: Xiangzhe <bakryun0718@proton.me>
tkgo11 pushed a commit to tkgo11/OmniRoute that referenced this pull request Sep 23, 2026
…rfaces (stream.ts + opencode-plugin) (cheaper-inference#10318)

* fix(sse): remove duplicate sseCommentsEnabled import that breaks the production build

The cheaper-inference#9378 merge auto-resolve left open-sse/utils/stream.ts importing
sseCommentsEnabled from sseHeartbeat.ts twice (lines 31 and 77). tsx/esbuild
(typecheck + both test runners) silently dedupe the binding, but webpack
fails the release build with "Identifier 'sseCommentsEnabled' has already
been declared" — this is the open-sse-typecheck / build base-red on
release/v3.8.50.

Adds a static regression guard (tests/unit/stream-imports-no-duplicates.test.ts,
RED on the duplicate, GREEN after) so the next merge auto-resolve of this
hot file fails in the unit suite instead of at release-build time. Also
includes the prettier canonicalization of the three style drifts the same
merge introduced (applied by lint-staged either way).

Validated: webpack release build passes on this tree (192.168.0.113 build box).

Refs cheaper-inference#9985

* fix(opencode-plugin): remove doubled '});' that breaks the release build

The cheaper-inference#9316 merge auto-resolve left a duplicated '});' at
@omniroute/opencode-plugin/src/index.ts:5481. The plugin is a standalone
package (outside typecheck:core and both test runners), so nothing parses
it until the release build — where tsup's DTS step fails with a cascade of
"Cannot find name" errors. The same syntax error also blocked prettier
from parsing the file, so this commit necessarily carries the prettier
pass lint-staged applies on staging (formatting was frozen since the bad
merge).

Adds tests/unit/opencode-plugin-parses.test.ts: parses every plugin source
file with the TypeScript compiler and fails on syntax diagnostics, so the
next merge-resolve accident in this uncovered package dies in the unit
suite instead of at release-build time.

Validated: release build:cli passes on the 192.168.0.113 build box with
this hotfix applied.

Refs cheaper-inference#9985

* test(build): guard against merge auto-resolve damage in build-only surfaces

Both defects these guards cover were fixed on the base while this branch
was open, so this PR is now purely the regression guards:

- stream-imports-no-duplicates.test.ts: fails on duplicate import bindings
  in open-sse/utils/stream.ts. The cheaper-inference#9378 merge left sseCommentsEnabled
  imported twice; tsx/esbuild dedupe it silently, so only the webpack
  release build caught it.
- opencode-plugin-parses.test.ts: parses every @omniroute/opencode-plugin
  source with the TypeScript compiler. The plugin is a standalone package
  (outside typecheck:core and both runners), so the doubled '});' from the
  cheaper-inference#9316 merge only surfaced at tsup DTS time.

Both broke the release build on the same day, in surfaces no gate reads
until publish time. Verified failing on the pre-fix trees and passing on
the current base.

---------

Co-authored-by: Xiangzhe <bakryun0718@proton.me>
tkgo11 pushed a commit to tkgo11/OmniRoute that referenced this pull request Sep 23, 2026
Drains the HARD failures reported by Release-Green run 31693210948 on issue cheaper-inference#9985
(ESLint errors: 2) plus the merge-integrity red every open PR is inheriting.

- ESLint error 1: @omniroute/opencode-plugin/src/index.ts had a stray extra
  '});' (introduced by cheaper-inference#9316) that broke parsing with 'unexpected file in NFT list'
  on the build path.
- ESLint error 2: cli-env-inline-comment-10100.test.ts used new Function to extract
  parseEnvValue from the bin entrypoint (no-new-func, Hard Rule cheaper-inference#3). Extracted the
  helper to bin/cli/utils/parseEnvValue.mjs and import it from both the entrypoint
  and the test (same behavior, no eval).
- open-sse-typecheck (Fast Quality Gates): open-sse/utils/stream.ts imported
  sseCommentsEnabled twice (cheaper-inference#9378) causing TS2300 Duplicate identifier; removed the
  duplicate import.
- Merge integrity (changelog + generated skills): skills/omni-settings/SKILL.md was
  edited manually by cheaper-inference#10169 without updating the generator source, so
  check:agent-skills-sync failed on every PR (Generated: 1). Moved the curated
  thinking-budget content into a <!-- skill:custom-start --> block (the documented
  preservation mechanism), which the generator now keeps in sync.

Refs cheaper-inference#9985
tkgo11 pushed a commit to tkgo11/OmniRoute that referenced this pull request Sep 23, 2026
…production build

The cheaper-inference#9378 merge auto-resolve left open-sse/utils/stream.ts importing
sseCommentsEnabled from sseHeartbeat.ts twice (lines 31 and 77). tsx/esbuild
(typecheck + both test runners) silently dedupe the binding, but webpack
fails the release build with "Identifier 'sseCommentsEnabled' has already
been declared" — this is the open-sse-typecheck / build base-red on
release/v3.8.50.

Adds a static regression guard (tests/unit/stream-imports-no-duplicates.test.ts,
RED on the duplicate, GREEN after) so the next merge auto-resolve of this
hot file fails in the unit suite instead of at release-build time. Also
includes the prettier canonicalization of the three style drifts the same
merge introduced (applied by lint-staged either way).

Validated: webpack release build passes on this tree (192.168.0.113 build box).

Refs cheaper-inference#9985
tkgo11 pushed a commit to tkgo11/OmniRoute that referenced this pull request Sep 23, 2026
…rfaces

Both defects these guards cover were fixed on the base while this branch
was open, so this PR is now purely the regression guards:

- stream-imports-no-duplicates.test.ts: fails on duplicate import bindings
  in open-sse/utils/stream.ts. The cheaper-inference#9378 merge left sseCommentsEnabled
  imported twice; tsx/esbuild dedupe it silently, so only the webpack
  release build caught it.
- opencode-plugin-parses.test.ts: parses every @omniroute/opencode-plugin
  source with the TypeScript compiler. The plugin is a standalone package
  (outside typecheck:core and both runners), so the doubled '});' from the
  cheaper-inference#9316 merge only surfaced at tsup DTS time.

Both broke the release build on the same day, in surfaces no gate reads
until publish time. Verified failing on the pre-fix trees and passing on
the current base.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…#9305) (cheaper-inference#9378)

* test(sse): add RED coverage for comment opt-out

* fix(sse): honor comment opt-out for final metadata

---------

Co-authored-by: 千乘妍 (Xiaoyaner) <xiaoyaner0201@users.noreply.github.com>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…nce#10260)

* fix(ci): clear base-reds on release/v3.8.50 (round 4)

Drains the HARD failures reported by Release-Green run 31693210948 on issue cheaper-inference#9985
(ESLint errors: 2) plus the merge-integrity red every open PR is inheriting.

- ESLint error 1: @omniroute/opencode-plugin/src/index.ts had a stray extra
  '});' (introduced by cheaper-inference#9316) that broke parsing with 'unexpected file in NFT list'
  on the build path.
- ESLint error 2: cli-env-inline-comment-10100.test.ts used new Function to extract
  parseEnvValue from the bin entrypoint (no-new-func, Hard Rule cheaper-inference#3). Extracted the
  helper to bin/cli/utils/parseEnvValue.mjs and import it from both the entrypoint
  and the test (same behavior, no eval).
- open-sse-typecheck (Fast Quality Gates): open-sse/utils/stream.ts imported
  sseCommentsEnabled twice (cheaper-inference#9378) causing TS2300 Duplicate identifier; removed the
  duplicate import.
- Merge integrity (changelog + generated skills): skills/omni-settings/SKILL.md was
  edited manually by cheaper-inference#10169 without updating the generator source, so
  check:agent-skills-sync failed on every PR (Generated: 1). Moved the curated
  thinking-budget content into a <!-- skill:custom-start --> block (the documented
  preservation mechanism), which the generator now keeps in sync.

Refs cheaper-inference#9985

* fix(tests): align wave1-a poolside test with cheaper-inference#10216 probed catalog

cheaper-inference#10216 published Poolside's two authenticated-probe models
(poolside/laguna-xs-2.1, poolside/laguna-s-2.1) as static seeds, but the
wave1-a free-tier test still asserted 'no invented static model ids'
(entry.models === []), failing every open PR. Separate poolside from the
empty-models assertion and pin its probed catalog explicitly so a future
catalog change is a deliberate update, not a silent drift.

* fix(pack): register parseEnvValue.mjs in PACK_ARTIFACT_REQUIRED_PATHS

The extract of parseEnvValue to bin/cli/utils/parseEnvValue.mjs added a new
direct import to bin/omniroute.mjs, which pack-artifact-entrypoint-closures
enforces against PACK_ARTIFACT_REQUIRED_PATHS. Register the module so a future
tarball omission fails loudly.

* fix(combo): restore default same-model retry semantics after cheaper-inference#10217

cheaper-inference#10217 wired config.failoverBeforeRetry into the same-model retry guard in
both the priority/auto and round-robin loops, but DEFAULT_COMBO_CONFIG
defaulted the flag to true — flipping same-model retry off for every combo
that never touched the setting, not just the opt-in case. Round-4 bisect
(dec88b5 vs 3c3b434) reproduced this against
tests/unit/combo-499-abort.test.ts, tests/unit/combo-quota-exhaustion-only-fallback.test.ts
and tests/unit/combo-stream-readiness-fallback.test.ts. Flip the default to
false so the historical retry-before-failover behavior returns for combos
that never set the flag, while explicit opt-in (the two new tests cheaper-inference#10217
added to combo-routing-engine.test.ts) still works.

* fix(quality): register visionBridge-responses-9597 in stryker tap.testFiles

check-mutation-test-coverage.mjs flagged tests/unit/guardrails/visionBridge-responses-9597.test.ts
as covering open-sse/services/combo/comboStructure.ts without being listed
in stryker.conf.json's tap.testFiles array. Add it so mutation coverage
attribution stays accurate.

* test(pack): expect parseEnvValue.mjs in the missing-artifact-paths fixture

The prior commit on this branch registered bin/cli/utils/parseEnvValue.mjs
in PACK_ARTIFACT_REQUIRED_PATHS but the "findMissingArtifactPaths flags
missing root runtime files in the tarball" test still hardcoded the old
expected list, so it never accounted for the new required path being
absent from the simulated tarball. Add it in its alphabetical slot.

* chore(lint): prune stale no-explicit-any suppression for call-log-file-rotation

--prune-suppressions found tests/unit/call-log-file-rotation.test.ts no
longer produces the 5 suppressed @typescript-eslint/no-explicit-any
warnings recorded in config/quality/eslint-suppressions.json. Remove the
dead entry so a regression would be caught again. Full-tree run with
--max-warnings 0 is clean: 0 errors, 0 warnings.

* fix(combo): decouple failoverBeforeRetry same-model guard from the skipUpstreamRetry default

Audit found that DEFAULT_COMBO_CONFIG.failoverBeforeRetry has defaulted to
true since before cheaper-inference#10217 (predates cheaper-inference#2417), and that value also feeds the
independent skipUpstreamRetry mechanism (src/sse/handlers/chat.ts:859,1126).
The previous commit on this branch flipped that default to false to fix the
cheaper-inference#10217 same-model retry guard, which silently disabled skipUpstreamRetry's
own default-on behavior for every combo without an opt-in — a regression in
the opposite direction (executor-level retries before the loop's own
failover, changing latency/failure behavior).

Revert the default back to true and decouple the two mechanisms instead:
resolveComboConfig/resolveComboSetupConfig now also compute
failoverBeforeRetryExplicit, true only when a cascade layer (combo/provider/
global) literally sets failoverBeforeRetry to true — not merely inherited
from the default. The cheaper-inference#10217 same-model retry guards in combo.ts (priority/
auto and round-robin loops) now read failoverBeforeRetryExplicit instead of
config.failoverBeforeRetry, restoring opt-in-only behavior for that guard
while the skipUpstreamRetry pass-through (config.failoverBeforeRetry at
combo.ts:1297,2865) is untouched and keeps its historical default-on.

* fix(combo,i18n): align getDefaultComboConfig with 10217 explicit flag; pt denoRelay entities

Two round-4 follow-ups exposed by the combinated base-red PR run:

1. comboConfig.ts: cheaper-inference#10217 round-4 fix (104afed) added
   failoverBeforeRetryExplicit to resolveComboConfig/resolveComboSetupConfig
   but getDefaultComboConfig() returned only DEFAULT_COMBO_CONFIG, so the
   combo-config.test.ts deepEqual (resolveComboConfig(null) ===
   getDefaultComboConfig()) failed on the extra field. Mirror the opt-in flag
   as false in the default.

2. pt.json: denoRelayOrgDomainHint still carried raw <app-name>/<org-slug>
   (the UNCLOSED_TAG RSC regression) — encode as &lt;...&gt; like the other
   42 locales, greening i18n-deno-relay-unclosed-tag.test.ts.

* chore(lint): disable @next/next/no-location-assign-relative-destination pending per-case review (cheaper-inference#10292)

The eslint-config-next bump in cheaper-inference#10043 shipped this new rule, flagging 6
pre-existing window.location.href navigations — several are deliberate
full-page reloads (login/logout state reset). Off with tracking issue
rather than a blanket router.push rewrite.

* fix(i18n): fill 439 missing UI keys (thinkingMode ×39 locales + pt catch-up) to restore 100% coverage

The cheaper-inference#10169 Thinking Budget keys existed only in en/pt-BR/vi and the pt (PT-PT)
catalog from cheaper-inference#10250 lagged 88 recent keys, dropping i18nUiCoverage to 99.3%
vs the frozen 100% ratchet baseline. Translated via the i18n:sync-ui marker
pipeline; glossary + ICU placeholder post-pass clean.

* fix(i18n): zh-TW glossary — replace retired 默認 with canonical 預設 in new thinkingMode keys

* chore(quality): rebase dead-code baseline 248 -> 409 for knip 6.32 bump (cheaper-inference#10043)

dependabot cheaper-inference#10043 upgraded knip 6.27 -> 6.32, which detects 162 MORE
genuinely-unused exports (331 vs 169) that 6.27 missed; DEAD_FILES
unchanged (78). Reproduced identically on the clean release/v3.8.50 tip
6e1e80e with a fresh 6.32 node_modules, so every PR is born red until
the tool change is absorbed. Owner authorized rebaseline (2026-08-13 via
PR cheaper-inference#10260). Structural cleanup of the newly-surfaced dead exports remains
separate debt.

---------

Co-authored-by: adevwithpurpose <adevwithpurpose@users.noreply.github.com>
Co-authored-by: backryun <bakryun0718@proton.me>
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…rfaces (stream.ts + opencode-plugin) (cheaper-inference#10318)

* fix(sse): remove duplicate sseCommentsEnabled import that breaks the production build

The cheaper-inference#9378 merge auto-resolve left open-sse/utils/stream.ts importing
sseCommentsEnabled from sseHeartbeat.ts twice (lines 31 and 77). tsx/esbuild
(typecheck + both test runners) silently dedupe the binding, but webpack
fails the release build with "Identifier 'sseCommentsEnabled' has already
been declared" — this is the open-sse-typecheck / build base-red on
release/v3.8.50.

Adds a static regression guard (tests/unit/stream-imports-no-duplicates.test.ts,
RED on the duplicate, GREEN after) so the next merge auto-resolve of this
hot file fails in the unit suite instead of at release-build time. Also
includes the prettier canonicalization of the three style drifts the same
merge introduced (applied by lint-staged either way).

Validated: webpack release build passes on this tree (192.168.0.113 build box).

Refs cheaper-inference#9985

* fix(opencode-plugin): remove doubled '});' that breaks the release build

The cheaper-inference#9316 merge auto-resolve left a duplicated '});' at
@omniroute/opencode-plugin/src/index.ts:5481. The plugin is a standalone
package (outside typecheck:core and both test runners), so nothing parses
it until the release build — where tsup's DTS step fails with a cascade of
"Cannot find name" errors. The same syntax error also blocked prettier
from parsing the file, so this commit necessarily carries the prettier
pass lint-staged applies on staging (formatting was frozen since the bad
merge).

Adds tests/unit/opencode-plugin-parses.test.ts: parses every plugin source
file with the TypeScript compiler and fails on syntax diagnostics, so the
next merge-resolve accident in this uncovered package dies in the unit
suite instead of at release-build time.

Validated: release build:cli passes on the 192.168.0.113 build box with
this hotfix applied.

Refs cheaper-inference#9985

* test(build): guard against merge auto-resolve damage in build-only surfaces

Both defects these guards cover were fixed on the base while this branch
was open, so this PR is now purely the regression guards:

- stream-imports-no-duplicates.test.ts: fails on duplicate import bindings
  in open-sse/utils/stream.ts. The cheaper-inference#9378 merge left sseCommentsEnabled
  imported twice; tsx/esbuild dedupe it silently, so only the webpack
  release build caught it.
- opencode-plugin-parses.test.ts: parses every @omniroute/opencode-plugin
  source with the TypeScript compiler. The plugin is a standalone package
  (outside typecheck:core and both runners), so the doubled '});' from the
  cheaper-inference#9316 merge only surfaced at tsup DTS time.

Both broke the release build on the same day, in surfaces no gate reads
until publish time. Verified failing on the pre-fix trees and passing on
the current base.

---------

Co-authored-by: Xiangzhe <bakryun0718@proton.me>
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.

2 participants