Skip to content

feat(web): extract the viewer SDK from the cockpit - #3565

Open
paul-nechifor wants to merge 1 commit into
paul/feat/t6from
paul/feat/w1-web-sdk
Open

feat(web): extract the viewer SDK from the cockpit#3565
paul-nechifor wants to merge 1 commit into
paul/feat/t6from
paul/feat/w1-web-sdk

Conversation

@paul-nechifor

Copy link
Copy Markdown
Contributor

No description provided.

- New web/sdk package (@dimos/sdk): the read-only viewer library. Reconnecting
  WebTransport, session state, stores, decoders, refcounted subscriptions.
- Code moves from web/cockpit/src/session with the wire behavior unchanged.
  React hooks live on the @dimos/sdk/react subpath, the root entry is React-free.
- The cockpit is rebuilt on the SDK. The SDK subscribes to nothing by itself,
  so the panel subscription policy moves to cockpit/src/subscriptions.ts.
- sdk/src/testing/fakeRelay.ts gives SDK unit tests a scripted fake relay.
- sdk/fixture/ is a minimal non-cockpit consumer for manual demos.
- CI runs the SDK checks and tests. setup.py and MANIFEST.in ship the SDK
  sources in the sdist.
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
4183 1 4182 175
View the top 1 failed test(s) by shortest run time
dimos.codebase_checks.test_no_sections::test_no_section_markers
Stack Traces | 0.782s run time
def test_no_section_markers():
        """
        Fail if any file contains section-style comment markers.
    
        If a file is too complicated to be understood without sections, then the
        sections should be files. We don't need "subfiles".
        """
        violations = find_section_markers()
        if violations:
            report_lines = [
                f"Found {len(violations)} section marker(s). "
                "If a file is too complicated to be understood without sections, "
                'then the sections should be files. We don\'t need "subfiles".',
                "",
            ]
            for path, lineno, text in violations:
                report_lines.append(f"  {path}:{lineno}: {text.strip()}")
>           raise AssertionError("\n".join(report_lines))
E           AssertionError: Found 2 section marker(s). If a file is too complicated to be understood without sections, then the sections should be files. We don't need "subfiles".
E           
E             .../web/relay_bridge/test_relay_bridge_module.py:1174: # --- Teleop (the tele_cmd_vel tx channel) ---
E             .../web/relay_bridge/test_relay_bridge_e2e.py:403: # --- Teleop e2e: viewer datagrams -> relay lease gate -> bridge publishes ---

lineno     = 403
path       = '.../web/relay_bridge/test_relay_bridge_e2e.py'
report_lines = ['Found 2 section marker(s). If a file is too complicated to be understood without sections, then the sections should ...y_bridge/test_relay_bridge_e2e.py:403: # --- Teleop e2e: viewer datagrams -> relay lease gate -> bridge publishes ---']
text       = '# --- Teleop e2e: viewer datagrams -> relay lease gate -> bridge publishes ---'
violations = [('.../web/relay_bridge/test_relay_bridge_module.py', 1174, '# --- Teleop (the tele_cmd_vel tx channel) ---'), ('dim...idge/test_relay_bridge_e2e.py', 403, '# --- Teleop e2e: viewer datagrams -> relay lease gate -> bridge publishes ---')]

dimos/codebase_checks/test_no_sections.py:145: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@greptile-apps

greptile-apps Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extracts Cockpit viewer and session functionality into the SDK and updates Cockpit to consume it. A browser hosted on a different origin from its relay cannot complete relay discovery through connect({ url }): the relay’s /api/info response is blocked by the browser’s cross-origin policy, after which the connection continues retrying instead of creating a session.

Merge safety: address cross-origin discovery before merging for deployments that host the viewer and relay on separate origins.

Confidence Score: 4/5

The change is not safe for cross-origin viewer-to-relay deployments until relay discovery is made accessible to supported browser origins.

A real headless-browser request reproduced the failed discovery request, and the same request succeeded when the relay response included a matching CORS header.

Files Needing Attention: web/sdk/src/session.ts initiates the absolute discovery request; web/relay/server.ts needs an explicit /api/info cross-origin response policy; web/sdk/src/transport.ts exposes the failure as repeated reconnecting.

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced proof for a posted P1 finding and linked it to the associated review comment details.
  • T-Rex produced a second proof for another posted P1 finding.
  • T-Rex produced a general-contract-validation-proof that documents the cross-origin issue, including how session.ts constructs the /api/info request, how the relay responds, and why the browser cannot consume the response without ACAO, along with a browser-level regression test as a validation scenario.
  • Artifacts were collected to support the findings, including the cross-origin browser test source and related browser/network logs.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P1 Cross-origin relay URLs cannot fetch relay discovery information

    • Bug
      • connect({ url: "https://other-origin..." }) resolves and fetches that relay's absolute /api/info. The relay's /api/info response has no Access-Control-Allow-Origin, so browser Fetch rejects even though the relay returns JSON. ReconnectingTransport swallows this failure and repeatedly transitions to reconnecting, never creating WebTransport.
    • Cause
      • web/sdk/src/session.ts:214 permits a consumer-provided absolute origin for fetchRelayInfo, while web/relay/server.ts:180-185 returns Response.json without CORS headers. web/sdk/src/transport.ts:218-235 treats the rejected fetch as a retryable connection failure.
    • Fix
      • Add an explicit CORS policy to the relay /api/info response (at least for supported cockpit origins, with Vary: Origin as appropriate), or change the public cross-origin URL flow to use a same-origin proxy. Add an actual-browser regression test that calls a relay on a different origin and asserts discovery succeeds.

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "feat(web): extract the viewer SDK from t..." | Re-trigger Greptile

Comment thread web/sdk/src/session.ts
transportDeps,
{
...deps,
fetchInfo: deps.fetchInfo ?? ((signal) => fetchRelayInfo(infoUrl, signal)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Cross-origin relay discovery lacks CORS support

connect({ url }) resolves an absolute relay URL and fetches its /api/info endpoint directly. The relay returns this endpoint without Access-Control-Allow-Origin, so a browser consumer hosted on another origin cannot read the discovery response. The transport treats that rejected fetch as retryable and remains in its reconnect loop instead of establishing WebTransport. Add an explicit CORS policy for supported /api/info consumers, including Vary: Origin when reflecting allowed origins, or route discovery through a same-origin proxy.

Artifacts

Focused cross-origin browser test source

  • The executable Node harness serves separate application and relay origins and toggles the relay CORS header, takeaway: it reproduces the browser CORS boundary directly.

Captured focused browser test source

  • Captured command output records the exact source of the executed focused test, takeaway: the tested browser flow is fully traceable.

Browser fetch without relay CORS permission

  • Headless Chrome requested relay `/api/info` across origins, the relay logged an Origin and no ACAO header, and Fetch rejected with `TypeError: Failed to fetch`, takeaway: the claimed browser failure occurs.

Browser fetch with relay CORS permission

  • The identical headless Chrome request fulfilled with HTTP 200 after the relay supplied matching ACAO, takeaway: missing CORS permission is the causal difference.

SDK transport and relay source excerpts

  • Captured source excerpts show the absolute SDK fetch, the no-CORS relay response, and the reconnect-on-fetch-failure loop, takeaway: the observed failure maps directly to the reported code locations.

View artifacts

T-Rex Ran code and verified through T-Rex

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