Skip to content

fix(dashboard): honour the live WebSocket port the handshake reports (#11331) - #11388

Merged
diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.50from
ntdatt812:fix/live-ws-honour-handshake-port
Aug 24, 2026
Merged

diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.50from
ntdatt812:fix/live-ws-honour-handshake-port

Conversation

@ntdatt812

@ntdatt812 ntdatt812 commented Aug 24, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ base-red inherited: #9985

Fixes #11331 (the part that is a bug).

The handshake already reports the port; the client throws it away

/api/v1/ws?handshake=1 returns the live server's real listening port:

live: {
  port: parseInt(process.env.LIVE_WS_PORT || "20132", 10),
  publicUrl: getLivePublicUrl(),
  path: getLiveWsPath(),
  ...
}

useLiveDashboard reads publicUrl and path out of that response — and nothing else:

const publicUrl = sanitizeWsPublicUrl(body?.live?.publicUrl);
if (publicUrl) setHandshakeUrl(publicUrl);
if (typeof body?.live?.path === "string" && body.live.path.startsWith("/")) setHandshakePath(body.live.path);

so the URL keeps the port compiled into the bundle:

return `${protocol}//${hostname}:20132${BUILD_TIME_WS_PATH}`;

NEXT_PUBLIC_* is inlined at build time — which is the whole reason this handshake exists, as the comment right above it says. A prebuilt Docker or npm image has no other way to learn a runtime value. So setting LIVE_WS_PORT moves the server and leaves the dashboard dialling 20132, with no way to correct it short of rebuilding the image. That is the Live disabled — WebSocket disconnected banner in the report.

The change

Read live.port from the handshake too, and resolve the URL in one place (shared/utils/wsPath.ts):

  1. an explicit wsUrl from the caller
  2. a complete live.publicUrl from the handshake
  3. the default URL with whatever port and path the handshake reported

A port outside 1..65535 is ignored rather than producing an unconnectable URL.

Also collapsed the if/else in getDefaultWsUrl whose two branches returned the same expression — it read as a distinction that no longer exists.

Scope — what this does not do

The report also asks the frontend to infer the proxy topology and connect over the origin port with a path prefix. That is already supported through LIVE_WS_PUBLIC_URL, which the handshake surfaces as live.publicUrl and which this PR keeps as the highest-priority source. Guessing a same-origin upgrade instead would break the documented setup, since src/proxy.ts has no upgrade handler on the HTTP port — that is the reason for the "Bug #1 fix" comment this PR preserved. So the fix here is the unambiguous half: a configured port must not be ignored.

Tests

tests/unit/live-ws-url-11331.test.ts — 11 cases:

  • a reported port replaces the compiled-in one, host and path untouched
  • no handshake data → default unchanged
  • 0, 70000, 1.5, "", "abc", null, NaN, {} → ignored
  • port and path applied together
  • a path that is not a path is ignored
  • publicUrl wins over the reported port; an explicit wsUrl wins over everything
  • an unparseable default is returned as-is instead of throwing
  • deriveLiveWsPath behaviour unchanged

Mutation-checked — dropping the url.port assignment turns 2 red:

✖ uses the port the handshake reports instead of the compiled-in one
✖ applies the port and the path together
ℹ pass 9  ℹ fail 2

tsc --noEmit reports nothing in the touched files; prettier/eslint pre-commit hooks clean.

/api/v1/ws?handshake=1 returns live.port - the port the live server is
actually listening on, straight from LIVE_WS_PORT. useLiveDashboard read
live.publicUrl and live.path out of that response and ignored live.port,
so it kept the port compiled into the bundle.

NEXT_PUBLIC_* is inlined at build time, which is exactly why the handshake
exists: a prebuilt Docker or npm image has no other way to learn the
runtime value. Setting LIVE_WS_PORT therefore moved the server and left
the dashboard dialling 20132, with no way to correct it short of
rebuilding the image - the Combo Studio banner in cheaper-inference#11331.

Read the port from the handshake as well, and resolve the URL through one
function in shared/utils/wsPath.ts: an explicit wsUrl wins, then a
complete publicUrl, then the default with whatever port and path the
handshake reported. A port outside 1..65535 is ignored rather than
producing an unconnectable URL.

Also collapsed the if/else in getDefaultWsUrl whose two branches returned
the same expression.
@Selaygermany

Copy link
Copy Markdown

Looked into the 3 failing checks on this PR (as of commit 913a51e):

  • Fast Quality Gates fails with 1 gate(s) failed: file-size. This matches the base-red drift already tracked in 🔴 Release branch not green: release/v3.8.50 #9985 on release/v3.8.50 — this diff only touches 4 small files (+174/-23), so the gate failure doesn't look introduced by this change.
  • Build (advisory) — non-blocking by design (name says so).
  • dast-smoke — cancelled after ~18min on the "Build CLI bundle" step ("The operation was canceled"), which looks like CI infra flakiness rather than a code issue.

Everything else is green: all 4 unit test shards, Vitest fast-path, "No new ESLint warnings", Docs Gates, semgrep x2, merge integrity, change classification.

The fix itself (sanitizeLiveWsPort + resolveLiveWsUrl in src/shared/utils/wsPath.ts) is well-scoped, validates the handshake-reported port (1-65535, type-checked) instead of trusting it blindly, and ships with 11 passing unit tests covering the sanitize/resolve/edge-case paths.

Per the base-red convention in AGENTS.md, might be worth noting ⚠️ base-red inherited: #9985 in the PR description so reviewers don't chase these 3 as regressions caused by this change.

@ntdatt812

Copy link
Copy Markdown
Contributor Author

Thanks — added ⚠️ base-red inherited: #9985 to the body per the AGENTS.md §Base-green convention.

I ran the gate locally to pin down which file trips it, because the mechanism is slightly different from inherited drift. file-size is already base-relative on PR events (#8522, --base-ref $PR_BASE_SHA), so ordinary inherited growth cannot red an innocent PR. What fails here is the test-file sub-check, which is absolute for files that are new relative to the frozen baseline:

$ npm run check:file-size -- --base-ref 7715825cb8cd170f5371c4c4bd5f940b5608f480
[file-size] OK — 161 arquivos congelados, cap 1000 para novos (4380 arquivos verificados)
[test-file-size] 1 test file violation(s) (testCap 1000):
  ✗ tests/unit/guardrails/videoBridgeResultCache.test.ts: 1036 > cap 1000 (arquivo novo acima do limite)

That file is not in this diff — it arrives with the base tip itself:

$ git log --oneline -1 -- tests/unit/guardrails/videoBridgeResultCache.test.ts
7715825cb fix(video): harden visual frame deduplication (#11382)

$ git show origin/release/v3.8.50:tests/unit/guardrails/videoBridgeResultCache.test.ts | wc -l
1039

7715825cb is the merge-base of this branch, so every PR cut from the current release/v3.8.50 tip will carry this failure until that test file is split or given a testFrozen entry. Worth flagging on #9985 / to @diegosouzapw, since the base-relative exemption does not cover it.

The other two are cancellations rather than failures — confirmed from the job API:

dast-smoke        conclusion=failure  →  step "Build CLI bundle" = cancelled
Build (advisory)  conclusion=failure  →  step "Run npm run build" = cancelled

Everything this PR actually changes is green.

@ntdatt812

Copy link
Copy Markdown
Contributor Author

The three red lanes here are not this diff.

Fast Quality Gates → file-size is the only real failure, and it fails on the base branch by itself:

$ git checkout origin/release/v3.8.50   # clean, zero changes from this PR
$ node scripts/check/check-file-size.mjs --base-ref c83116e634a054b6ad50d54eb49ac7449615fb2e
[file-size] 1 violação(ões):
  ✗ src/lib/guardrails/videoBridgeRuntime.ts: 1009 > cap 1000 (arquivo novo acima do limite)
[test-file-size] 1 test file violation(s) (testCap 1000):
  ✗ tests/unit/guardrails/videoBridgeResultCache.test.ts: 1040 > cap 1000
EXIT=1

tests/unit/guardrails/videoBridgeResultCache.test.ts is 1035 lines at this PR's base sha c83116e634a0 and 1039 on the current release/v3.8.50 tip — it was over the cap before this branch existed, and neither it nor videoBridgeRuntime.ts is in file-size-baseline.json, so both are measured against the 1000 cap for un-frozen files. It arrived with #11362 / #11382 (fix(video): harden result cache identity and coalescing, harden visual frame deduplication).

This PR touches four files — src/hooks/useLiveDashboard.ts, src/shared/utils/wsPath.ts, tests/unit/live-ws-url-11331.test.ts, changelog.d/fixes/11388-live-ws-handshake-port.md — none of them near the cap and none of them the flagged files.

Build (advisory) and dast-smoke did not fail on their own merits — both logs end at ##[error]The operation was canceled., cancelled after the gates job went red.

Happy to rebase once the two guardrails files are split or baselined, if you'd rather see the lane green before merging.

diegosouzapw pushed a commit that referenced this pull request Aug 24, 2026
)

Validado em lote combinado (batch-0824g, junto de #11388/#11397/#11415/#11418) contra o tip de release/v3.8.50: typecheck:core limpo, file-size/changelog/complexity/cognitive-complexity OK, 62/62 testes focados passando.

Diagnóstico correto e bem documentado: a falha do nightly Node 26 era um teste que sorteia um número e depende do resultado, não uma quebra de compatibilidade. Comportamento de produção inalterado (a janela de jitter continua aleatória; só o teste ganhou controle sobre ela). Obrigado pela investigação detalhada!
@diegosouzapw
diegosouzapw merged commit f93fecd into cheaper-inference:release/v3.8.50 Aug 24, 2026
13 of 16 checks passed
MumuTW added a commit to MumuTW/OmniRoute that referenced this pull request Aug 25, 2026
…rom wsPath.ts; add glm-5.3-max to the GLM test table

Two more release/v3.8.51 base-reds (cheaper-inference#11449), both mechanical once traced:

- src/shared/utils/wsPath.ts: cheaper-inference#11388 (merged 2026-08-24) fixed cheaper-inference#11331 by adding
  sanitizeLiveWsPort() + resolveLiveWsUrl() and wiring them into
  useLiveDashboard.ts with tests/unit/live-ws-url-11331.test.ts. The next day's
  cheaper-inference#11434 merge carried a competing cheaper-inference#11331 fix (resolveLiveWsPublicUrl) and its
  wsPath.ts version overwrote cheaper-inference#11388's hunk, while the hook and the test kept
  importing the two functions. Result on the tip: TS2305 in the hook, the unit
  test red, and the Turbopack build failing on "Export resolveLiveWsUrl doesn't
  exist". This re-adds cheaper-inference#11388's two functions verbatim next to the surviving
  resolveLiveWsPublicUrl; the names do not collide and the precedence they
  implement (explicit wsUrl → handshake publicUrl → default URL with handshake
  port/path) is unchanged.

- open-sse/mcp-server/__tests__/glmCodingProviderConfig.test.ts: the registry
  ships glm-5.3-max with a "max" effort tier. Z.AI's GLM-5.3 reference lists
  reasoning_effort ∈ {low, high, max} and recommends max for coding, so the
  registry is right and the inventory/tier tables in the test were stale (they
  already declared ["low","high","max"] on the base model).

Verification: tests/unit/live-ws-url-11331.test.ts 11/11; glmCodingProviderConfig
10/10; tsc reports no errors in useLiveDashboard.ts / wsPath.ts.
diegosouzapw pushed a commit that referenced this pull request Aug 26, 2026
#11484)

Validated in a combined sub-batch worktree off release/v3.8.51 tip. Its second commit reimplemented wsPath.ts's sanitizeLiveWsPort/resolveLiveWsUrl to fix a build break it hit — that exact break is already fixed on the current tip via #11502 (the original #11388 implementation restored verbatim). Conflicted against that; resolved by keeping the tip's established implementation and dropping the redundant reimplementation (both are functionally equivalent; the tip's carries the original #11331 doc comments and precedence contract). Pushed the same resolution to this branch.
- typecheck:core, file-size, changelog-integrity, complexity, cognitive-complexity — all OK
- Full-repo lint: 228 pre-existing dashboard react-hooks/* findings, unrelated to this diff
- Focused test: cli-settings-jcode.test.ts (integration) — part of sub-batch's 165/165 run

Thanks for the real end-to-end verification (production server, real ~/.jcode/config.toml, jcode --provider-profile omniroute serving a completion, jcode provider-doctor passing) — the managed-block approach (marker-delimited, preserves user-authored config, 409s instead of corrupting on a hand-written conflicting table) is exactly the right shape for this integration.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
cheaper-inference#11484)

Validated in a combined sub-batch worktree off release/v3.8.51 tip. Its second commit reimplemented wsPath.ts's sanitizeLiveWsPort/resolveLiveWsUrl to fix a build break it hit — that exact break is already fixed on the current tip via cheaper-inference#11502 (the original cheaper-inference#11388 implementation restored verbatim). Conflicted against that; resolved by keeping the tip's established implementation and dropping the redundant reimplementation (both are functionally equivalent; the tip's carries the original cheaper-inference#11331 doc comments and precedence contract). Pushed the same resolution to this branch.
- typecheck:core, file-size, changelog-integrity, complexity, cognitive-complexity — all OK
- Full-repo lint: 228 pre-existing dashboard react-hooks/* findings, unrelated to this diff
- Focused test: cli-settings-jcode.test.ts (integration) — part of sub-batch's 165/165 run

Thanks for the real end-to-end verification (production server, real ~/.jcode/config.toml, jcode --provider-profile omniroute serving a completion, jcode provider-doctor passing) — the managed-block approach (marker-delimited, preserves user-authored config, 409s instead of corrupting on a hand-written conflicting table) is exactly the right shape for this integration.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…aper-inference#11380)

Validado em lote combinado (batch-0824g, junto de cheaper-inference#11388/cheaper-inference#11397/cheaper-inference#11415/cheaper-inference#11418) contra o tip de release/v3.8.50: typecheck:core limpo, file-size/changelog/complexity/cognitive-complexity OK, 62/62 testes focados passando.

Diagnóstico correto e bem documentado: a falha do nightly Node 26 era um teste que sorteia um número e depende do resultado, não uma quebra de compatibilidade. Comportamento de produção inalterado (a janela de jitter continua aleatória; só o teste ganhou controle sobre ela). Obrigado pela investigação detalhada!
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…heaper-inference#11331) (cheaper-inference#11388)

Validado em lote combinado (batch-0824g) contra o tip de release/v3.8.50: typecheck:core limpo, gates estáticos OK, 62/62 testes focados passando (incluindo tests/unit/live-ws-url-11331.test.ts, 11 casos + mutation-check).

Resolve o incidente real do cheaper-inference#11331: o handshake já reportava a porta live real, mas o cliente descartava esse campo e ficava preso na porta compilada no bundle. Precedência clara (wsUrl explícito > publicUrl completo > porta/path do handshake aplicados ao default). Obrigado pela contribuição!
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.

[BUG] Combo Studio frontend ignores proxy variables and forces hardcoded WebSocket port 20132 (/live-ws)

3 participants