Skip to content

fix(core): accept address websocket subscriptions - #1278

Open
realfishsam wants to merge 1 commit into
mainfrom
fix/ws-address-subscriptions-1177
Open

realfishsam wants to merge 1 commit into
mainfrom
fix/ws-address-subscriptions-1177

Conversation

@realfishsam

Copy link
Copy Markdown
Contributor

Summary

  • Add watchAddress to the sidecar WebSocket streaming allowlist.
  • Map unwatchAddress back to watchAddress for subscription-key lookup during unsubscribe.
  • Add a regression test that checks all exchange methods declared as verb: "ws" in method-verbs.json are registered by the WebSocket handler.

Fixes #1177

Test Plan

  • npm test --workspace=pmxt-core -- --runTestsByPath test/pipeline/ws-handler-streaming-methods.test.ts --runInBand
  • npm test --workspace=pmxt-core -- --runTestsByPath test/pipeline/watch-order-book-api.test.ts test/pipeline/get-post-parity.test.ts --runInBand
  • npm run build --workspace=pmxt-core
  • git diff --check

Add watchAddress/unwatchAddress to the sidecar WebSocket method registry so methods declared as ws in method-verbs.json are not rejected before dispatch.

Fixes #1177
@realfishsam

Copy link
Copy Markdown
Contributor Author

Verification update from autonomous fix launcher:

Targeted local checks passed for the WebSocket registry fix:

  • npm test --workspace=pmxt-core -- --runTestsByPath test/pipeline/ws-handler-streaming-methods.test.ts --runInBand
  • npm test --workspace=pmxt-core -- --runTestsByPath test/pipeline/watch-order-book-api.test.ts test/pipeline/get-post-parity.test.ts --runInBand
  • npm run build --workspace=pmxt-core
  • git diff --check

GitHub generated-sync checks are currently red due unrelated repo-wide generated drift, not this focused WebSocket change. The failed logs show broad changes to core/COMPLIANCE.md, SDK API references, and generated client methods (including hosted-mode/client-method drift) unrelated to core/src/server/ws-handler.ts or the new registry regression test. I kept this PR scoped and did not fold that broad generated drift into it.

@realfishsam

Copy link
Copy Markdown
Contributor Author

PR Review: PASS (NOT VERIFIED)

What This Does

Registers watchAddress and unwatchAddress in the sidecar WebSocket handler’s streaming method allow-lists. This matters because methods already declared as verb: "ws" in method-verbs.json were being rejected before dispatch.

Blast Radius

Core sidecar WebSocket dispatch only: core/src/server/ws-handler.ts, plus a new registry-consistency Jest test. Exchange implementations and SDK method signatures are unchanged.

Consumer Verification

Before (base branch):
Static consumer-path check against origin/main showed the sidecar method registry declared address streaming methods as WebSocket methods, but the WebSocket handler did not register them:

base_missing ['watchAddress', 'unwatchAddress']

A WebSocket subscribe/unsubscribe for those methods would be rejected by the handler registry before reaching exchange dispatch.

After (PR branch):
The same registry check on the PR branch shows no missing WebSocket methods:

pr_missing []

The new Jest test also passed as part of the core test suite.

Test Results

  • Build: PASS (npm run build --workspace=pmxt-core)
  • Unit tests: PASS for core (npm run test --workspace=pmxt-core: 700 passed, 3 skipped); root npm test later failed only because the review environment lacks Python pytest.
  • Server starts: PASS (/health returned {"status":"ok",...} on the PR branch)
  • E2E smoke: PASS for focused registry/handler coverage; NOT VERIFIED against a live address stream venue because no exchange credentials/live WebSocket fixture were used.

Findings

No blocking findings.

PMXT Pipeline Check

  • Field propagation (3-layer): N/A
  • OpenAPI sync: N/A
  • Financial precision: N/A
  • Type safety: OK — small registry additions plus test coverage.
  • Auth safety: OK — no credential handling changes.

Semver Impact

patch -- fixes a WebSocket dispatch allow-list regression for existing methods.

Risk

The handler now accepts/dispatches address subscriptions, but I did not verify a live venue address stream end-to-end with real credentials; this review verifies the pre-dispatch blocker is removed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant