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

fix(core): accept address websocket subscriptions#1278
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.

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