Skip to content

fix(oauth): route zed-hosted native-app callback back to the dashboard port - #10517

Merged
diegosouzapw merged 9 commits into
cheaper-inference:release/v3.8.50from
phatchau036:fix/zed-hosted-oauth-callback-port
Aug 18, 2026
Merged

diegosouzapw merged 9 commits into
cheaper-inference:release/v3.8.50from
phatchau036:fix/zed-hosted-oauth-callback-port

Conversation

@phatchau036

Copy link
Copy Markdown
Contributor

Summary

  • The zed-hosted provider's native-app sign-in always redirects the browser to http://127.0.0.1:<native_app_port>/?user_id=...&access_token=.... That port was the hardcoded default 58443, so after signing in at zed.dev the browser lands on a dead port ("site can't be reached") and the login looks broken — even though the access token is right there in the URL.
  • The manual paste fallback could never complete either: handleManualSubmit requires a ?code= param that Zed's callback never carries, so pasting the URL always failed with "No authorization code found".
  • This PR derives native_app_port from the dashboard's own loopback port so the redirect lands back on OmniRoute, forwards the root / query to the /callback relay, teaches the relay to recognize the Zed payload, and fixes the manual paste path for zed-hosted.

Flow after the fix (true localhost): Connect → popup at zed.dev → Zed redirects to 127.0.0.1:<dashboard-port>/?user_id=...&access_token=... → root handler forwards to /callback → relay postMessages the full URL → modal exchanges it (RSA-decrypts with the authorize-time private key) and saves the connection. LAN/remote deployments keep the dead-port redirect and the (now working) manual paste flow.

Related Issues

  • No existing issue found for this failure mode (searched "zed oauth" / "zed-hosted"); happy to link one if it exists.

Validation

  • Change type: provider / UI
  • Focused tests and category gates from the golden path — not run locally (authoring machine could not install node_modules); CI on this PR runs them
  • npm run lint — not run locally (same reason)
  • Reconciled with the current active release base — branched from main @ ca23eed; all six touched files verified byte-identical to that base before editing
  • Production-code changes include a new or updated automated test in this PR

Tests Added Or Updated

Coverage Notes

  • The /callback relay origin change is covered by the updated postMessage scope test. The buildAuthUrl port derivation and the modal paste-path dispatch are exercised by the existing zed/oauth-modal suites in CI; no dedicated new unit test asserts the port derivation yet (noted below).

Reviewer Notes

  • Root-cause trace: createZedNativeAuthData always sends native_app_port (default 58443) and Zed ignores any redirect_uri/path, redirecting to the loopback root. Separately, the root page (/) 307-redirects to /dashboard and drops the query string, so even landing on the dashboard port would have lost the payload — hence the forward-to-/callback handler.
  • resolveDashboardLoopbackPort only accepts http: loopback redirect URIs; https/remote/LAN origins keep the old default port and the paste flow, so no remote behavior changes.
  • The loopback same-port additions to trustedTargetOrigins are both spellings of the operator's own machine on the same port (localhost vs 127.0.0.1) — the same rationale as the existing 1455 entries; a hostile local app would be on a different port and still never receives the code.
  • All six files transpile cleanly (esbuild parse check). End-to-end against live zed.dev was not possible from the authoring machine — a maintainer smoke-test of a real zed-hosted connect on localhost would be the last mile.

diegosouzapw and others added 6 commits August 8, 2026 00:08
…r-inference#189, cheaper-inference#190)

Bumps: nanoid ^3.3.17 (was transitive, now overridden), dompurify ^3.4.13
(with monaco-editor scoped override). Closes Dependabot cheaper-inference#189, cheaper-inference#190.

Remaining cheaper-inference#182-cheaper-inference#188 (js-yaml + mermaid) already closed by cheaper-inference#9651 merge —
awaiting Dependabot re-scan.

npm audit → 0 vulnerabilities.
_tasks is a SEPARATE nested git repo (gitignored). The pattern _tasks/ (trailing
slash) ignores only a directory, not a SYMLINK named _tasks. A self-referential
_tasks symlink can slip in via git add -A and, once pulled, checkout materializes
it over the real _tasks repo (destroying plans/specs/hands-off). Anchored /_tasks
ignores the symlink too, preventing re-capture.
…ference#10026)

Mirror the request-time exclusion rule (provider_specific_data.excludedModels)
in the unified catalog builder: a model is hidden when its provider has
connections but none of them is eligible for it. Applied across the
PROVIDER_MODELS, synced, custom, alias-backed, and managed-fallback loops
so ghost models no longer appear as available.

Co-authored-by: ritheshcn25 <ritheshcn25@users.noreply.github.com>
…per-inference#10055)

* fix(models): memoize getModelsDevPricing for /v1/models catalog

resolveCatalogPricing called getModelsDevPricing once per model while
building GET /v1/models. Each call re-scanned models_dev_pricing and
JSON.parsed every row (~10k SQL scans + multi-GB parse work), pegging
the event loop so even /healthz timed out (cheaper-inference#9685, cheaper-inference#10052).

Memoize the parsed map until saveModelsDevPricing / clearModelsDevPricing
and add a unit test for invalidation.

Signed-off-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com>

* fix(db): invalidate modelsDevPricing cache on DB reset (cheaper-inference#10055)

Copilot review fixes:
1. Register invalidateModelsDevPricingCache() with DB state reset system
   so resetDbInstance() clears the process-local memo, preventing stale
   pricing data from surviving across DB reset/restore operations.
2. Add test assertion verifying DB reset bypasses the memo (Copilot cheaper-inference#10055).

The process-local memo at modelsDevSync.ts:204 caches getModelsDevPricing()
results until saveModelsDevPricing()/clearModelsDevPricing() to avoid
re-scanning all pricing rows on every /v1/models request. Without this hook,
backup restore and test DB resets would serve stale cached data from the
previous connection.

Tests: npm run test:unit:serial -- tests/unit/modelsDevSync-extended.test.ts

---------

Signed-off-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com>
Co-authored-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Zed's native-app sign-in always redirects the browser to the loopback port
sent as native_app_port (hardcoded default 58443), where nothing listens:
the browser shows "site can't be reached" and the login looks broken even
though the token is in the URL. The manual paste fallback was broken too -
handleManualSubmit requires a ?code= param that Zed's callback
(user_id + access_token) never carries, so the flow could never complete.

- zed-hosted: derive native_app_port from the dashboard's own loopback
  port so the redirect lands back on OmniRoute; remote/LAN origins keep
  the old default port and the paste flow
- app root: forward ?user_id=...&access_token=... to the /callback relay
  instead of dropping the query string on the /dashboard redirect
- /callback relay: recognize the Zed payload (no code param) and relay the
  full URL as the exchange payload; allow postMessage to both loopback
  spellings (localhost/127.0.0.1) of the same port
- OAuthModal: zed-hosted popup auto-completes on true localhost; the
  manual paste path passes the full URL through to the exchange instead
  of erroring with "No authorization code found"
- manual input panel: zed-hosted-specific placeholder and hint
- tests: extend the postMessage scope guard with the loopback same-port
  trusted origins
@diegosouzapw
diegosouzapw changed the base branch from main to release/v3.8.50 August 17, 2026 05:34
@diegosouzapw

Copy link
Copy Markdown
Collaborator

The callback redesign is focused and the existing Zed/provider tests are green, but I would not merge yet. On an HTTPS localhost dashboard, the new branch treats zed-hosted as automatic because isTrueLocalhost is true, while the redirect strategy still constructs http://localhost:443/callback; zed-hosted then advertises an HTTP 127.0.0.1:443 callback. A TLS listener normally cannot serve that plaintext request, so the popup can fail instead of falling back to the manual URL path. Please guard HTTPS localhost (or implement a verified compatible handoff). Also, the only added test covers postMessage origin scope; it does not exercise port derivation, root forwarding, or full-URL exchange, and the PR validation says no focused/live test was run. Hard Rule #18 therefore still needs a failing-then-passing regression test or documented VPS smoke test. The focused probes I ran passed, but they ran in the review worktree rather than a checkout of this PR head.

@diegosouzapw

Copy link
Copy Markdown
Collaborator

The PR is now correctly targeted at release/v3.8.50; its head is unchanged. The callback redesign still needs the HTTPS-localhost behavior and the browser/provider regression covered on the contributor branch, followed by fresh release gates. It remains outside the train until that fix-in-place work is complete.

phatchau036 and others added 2 commits August 18, 2026 00:44
…r scheme/port

resolveDashboardLoopbackPort() previously re-derived the dashboard's loopback
port from the browser-supplied redirectUri (window.location.port ||
protocol === "https:" ? "443" : "80"), which produced http://127.0.0.1:443/
native-app redirects when the dashboard was reached over HTTPS on its
implicit default port (e.g. behind a local TLS-terminating reverse proxy) -
a scheme/port mismatch, since Zed's own redirect is always plain http and
nothing serves plain HTTP on 443 in that scenario.

This code runs server-side (in the OAuth authorize API route), so once the
redirect URI's hostname is confirmed loopback it now uses the OmniRoute
process's own authoritative listening port via getRuntimePorts()
(OMNIROUTE_PORT/PORT/DASHBOARD_PORT) instead of re-deriving it from the
browser-observed scheme/port. Non-loopback (remote/LAN) redirect URIs still
return null and fall back to the manual paste flow.

Adds tests/unit/zed-hosted-loopback-port-derivation.test.ts (8 cases)
covering the port-derivation logic directly, including the HTTPS-default-port
mismatch scenario that motivated this fix, env-var precedence, IPv6 loopback,
non-loopback/remote fallback, and buildAuthUrl's native_app_port wiring.

Also rebaselines config/quality/file-size-baseline.json for OAuthModal.tsx's
own growth from this PR's earlier commit (1134->1149 gate units) - legitimate
zed-hosted callback wiring at the existing provider-switch chokepoint, not
extractable without a broader modal decomposition (tracked in cheaper-inference#3501).

The live Zed OAuth handshake itself (root -> /callback -> OAuthModal exchange
against the real zed.dev endpoint) still needs a documented VPS smoke test
per Hard Rule cheaper-inference#18; this fix covers the TDD-able port-derivation logic that
motivated the change.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…sted-oauth-callback-port

# Conflicts:
#	.gitignore
#	config/quality/file-size-baseline.json
#	package-lock.json
#	package.json
#	src/lib/modelsDevSync.ts
#	tests/unit/modelsDevSync-extended.test.ts
@diegosouzapw
diegosouzapw merged commit 72eff76 into cheaper-inference:release/v3.8.50 Aug 18, 2026
1 of 3 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…d port (cheaper-inference#10517)

* fix(deps): bump nanoid, dompurify for 2 new Dependabot alerts (cheaper-inference#189, cheaper-inference#190)

Bumps: nanoid ^3.3.17 (was transitive, now overridden), dompurify ^3.4.13
(with monaco-editor scoped override). Closes Dependabot cheaper-inference#189, cheaper-inference#190.

Remaining cheaper-inference#182-cheaper-inference#188 (js-yaml + mermaid) already closed by cheaper-inference#9651 merge —
awaiting Dependabot re-scan.

npm audit → 0 vulnerabilities.

* fix(repo): harden .gitignore to also ignore a _tasks symlink (/_tasks)

_tasks is a SEPARATE nested git repo (gitignored). The pattern _tasks/ (trailing
slash) ignores only a directory, not a SYMLINK named _tasks. A self-referential
_tasks symlink can slip in via git add -A and, once pulled, checkout materializes
it over the real _tasks repo (destroying plans/specs/hands-off). Anchored /_tasks
ignores the symlink too, preventing re-capture.

* Hide health-check excluded models from /v1/models catalog (cheaper-inference#10026)

Mirror the request-time exclusion rule (provider_specific_data.excludedModels)
in the unified catalog builder: a model is hidden when its provider has
connections but none of them is eligible for it. Applied across the
PROVIDER_MODELS, synced, custom, alias-backed, and managed-fallback loops
so ghost models no longer appear as available.

Co-authored-by: ritheshcn25 <ritheshcn25@users.noreply.github.com>

* fix(models): memoize getModelsDevPricing (event loop / healthz) (cheaper-inference#10055)

* fix(models): memoize getModelsDevPricing for /v1/models catalog

resolveCatalogPricing called getModelsDevPricing once per model while
building GET /v1/models. Each call re-scanned models_dev_pricing and
JSON.parsed every row (~10k SQL scans + multi-GB parse work), pegging
the event loop so even /healthz timed out (cheaper-inference#9685, cheaper-inference#10052).

Memoize the parsed map until saveModelsDevPricing / clearModelsDevPricing
and add a unit test for invalidation.

Signed-off-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com>

* fix(db): invalidate modelsDevPricing cache on DB reset (cheaper-inference#10055)

Copilot review fixes:
1. Register invalidateModelsDevPricingCache() with DB state reset system
   so resetDbInstance() clears the process-local memo, preventing stale
   pricing data from surviving across DB reset/restore operations.
2. Add test assertion verifying DB reset bypasses the memo (Copilot cheaper-inference#10055).

The process-local memo at modelsDevSync.ts:204 caches getModelsDevPricing()
results until saveModelsDevPricing()/clearModelsDevPricing() to avoid
re-scanning all pricing rows on every /v1/models request. Without this hook,
backup restore and test DB resets would serve stale cached data from the
previous connection.

Tests: npm run test:unit:serial -- tests/unit/modelsDevSync-extended.test.ts

---------

Signed-off-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com>
Co-authored-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>

* fix(oauth): route Zed hosted sign-in callback back to the dashboard port

Zed's native-app sign-in always redirects the browser to the loopback port
sent as native_app_port (hardcoded default 58443), where nothing listens:
the browser shows "site can't be reached" and the login looks broken even
though the token is in the URL. The manual paste fallback was broken too -
handleManualSubmit requires a ?code= param that Zed's callback
(user_id + access_token) never carries, so the flow could never complete.

- zed-hosted: derive native_app_port from the dashboard's own loopback
  port so the redirect lands back on OmniRoute; remote/LAN origins keep
  the old default port and the paste flow
- app root: forward ?user_id=...&access_token=... to the /callback relay
  instead of dropping the query string on the /dashboard redirect
- /callback relay: recognize the Zed payload (no code param) and relay the
  full URL as the exchange payload; allow postMessage to both loopback
  spellings (localhost/127.0.0.1) of the same port
- OAuthModal: zed-hosted popup auto-completes on true localhost; the
  manual paste path passes the full URL through to the exchange instead
  of erroring with "No authorization code found"
- manual input panel: zed-hosted-specific placeholder and hint
- tests: extend the postMessage scope guard with the loopback same-port
  trusted origins

* changelog: fragment for cheaper-inference#10517

* fix(oauth): derive Zed native_app_port from server config, not browser scheme/port

resolveDashboardLoopbackPort() previously re-derived the dashboard's loopback
port from the browser-supplied redirectUri (window.location.port ||
protocol === "https:" ? "443" : "80"), which produced http://127.0.0.1:443/
native-app redirects when the dashboard was reached over HTTPS on its
implicit default port (e.g. behind a local TLS-terminating reverse proxy) -
a scheme/port mismatch, since Zed's own redirect is always plain http and
nothing serves plain HTTP on 443 in that scenario.

This code runs server-side (in the OAuth authorize API route), so once the
redirect URI's hostname is confirmed loopback it now uses the OmniRoute
process's own authoritative listening port via getRuntimePorts()
(OMNIROUTE_PORT/PORT/DASHBOARD_PORT) instead of re-deriving it from the
browser-observed scheme/port. Non-loopback (remote/LAN) redirect URIs still
return null and fall back to the manual paste flow.

Adds tests/unit/zed-hosted-loopback-port-derivation.test.ts (8 cases)
covering the port-derivation logic directly, including the HTTPS-default-port
mismatch scenario that motivated this fix, env-var precedence, IPv6 loopback,
non-loopback/remote fallback, and buildAuthUrl's native_app_port wiring.

Also rebaselines config/quality/file-size-baseline.json for OAuthModal.tsx's
own growth from this PR's earlier commit (1134->1149 gate units) - legitimate
zed-hosted callback wiring at the existing provider-switch chokepoint, not
extractable without a broader modal decomposition (tracked in cheaper-inference#3501).

The live Zed OAuth handshake itself (root -> /callback -> OAuthModal exchange
against the real zed.dev endpoint) still needs a documented VPS smoke test
per Hard Rule cheaper-inference#18; this fix covers the TDD-able port-derivation logic that
motivated the change.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

---------

Signed-off-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com>
Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>
Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouza.pw@outlook.com>
Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
Co-authored-by: ritheshcn25 <rithesh.chandran@snb.ca>
Co-authored-by: ritheshcn25 <ritheshcn25@users.noreply.github.com>
Co-authored-by: Ravi Tharuma <25951435+RaviTharuma@users.noreply.github.com>
Co-authored-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
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.

4 participants