test(desktop): pin profile-owned remote WebSocket target - #1
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR strengthens regression coverage for profile-owned remote backends by mocking HermesGateway.connect in a shared place and asserting that ensureGatewayForProfile passes the exact profile-owned remote WebSocket URL to the gateway while not reusing the primary gateway. Sequence diagram for profile-owned remote WebSocket connection in testssequenceDiagram
actor Tester
participant setPrimaryGateway
participant installDesktop
participant ensureGatewayForProfile
participant getConnection
participant HermesGateway
participant $gateway
Tester->>setPrimaryGateway: setPrimaryGateway(primary, default)
Tester->>installDesktop: installDesktop({ getConnection })
Tester->>ensureGatewayForProfile: ensureGatewayForProfile(worker)
ensureGatewayForProfile->>getConnection: getConnection()
getConnection-->>ensureGatewayForProfile: descriptor(wsUrl)
ensureGatewayForProfile->>HermesGateway: connect(wsUrl)
ensureGatewayForProfile->>$gateway: $gateway.get()
$gateway-->>Tester: gatewayInstance
alt [primary not reused]
Tester->>Tester: expect(gatewayInstance).not.toBe(primary)
Tester->>Tester: expect(gatewayMocks.connect).toHaveBeenCalledWith(wsUrl)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Because
gatewayMocks.connectis hoisted and shared across tests, consider explicitly resetting its call history inbeforeEach/afterEachin this file to avoid cross-test coupling and make the expectations more robust. - The test hardcodes a specific
wsUrlstring; consider constructing the expected URL from the descriptor fields within the test so that the assertion remains stable if the URL-building logic changes (e.g., query params or path).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Because `gatewayMocks.connect` is hoisted and shared across tests, consider explicitly resetting its call history in `beforeEach`/`afterEach` in this file to avoid cross-test coupling and make the expectations more robust.
- The test hardcodes a specific `wsUrl` string; consider constructing the expected URL from the descriptor fields within the test so that the assertion remains stable if the URL-building logic changes (e.g., query params or path).Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR strengthens Desktop regression coverage around ensureGatewayForProfile when profiles use profile-owned remote backends, verifying that the exact dedicated wsUrl is passed through to HermesGateway.connect() rather than reusing the primary gateway connection.
Changes:
- Hoists and captures the
HermesGateway.connect()mock so tests can assert dial attempts and arguments. - Updates the “profile-owned remote” test case to provide a fully-specified remote descriptor (including
wsUrl) and asserts the exact URL is used. - Keeps the existing assertion that the primary gateway is not selected for profile-owned remote connections.
Suppressed comments (1)
apps/desktop/src/store/gateway-shared-remote.test.ts:72
- If connect is restored to “throw by default” for stronger shared-primary coverage, this test needs to explicitly allow the expected dial so it doesn’t fail for the intended path.
setPrimaryGateway(primary as never, 'default')
installDesktop({
getConnection: vi.fn(async () => ({
authMode: 'token',
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
71bff94
into
Tigrannnnnnn:fix/per-profile-remote-ws-routing
What does this PR do?
Strengthens the regression coverage in NousResearch#85750 for profile-owned remote backends.
The existing test proves that a profile-owned descriptor does not reuse the primary gateway. This follow-up additionally verifies that the profile's exact remote
wsUrlis passed toHermesGateway.connect().This is a test-only change. It does not modify the production fix.
Related work
Changes made
HermesGateway.connect().profilebut notsharedPrimary.wsUrlis dialed.The fixture uses the reserved
.invaliddomain and synthetic credentials; the gateway client is mocked and no network request is made.How to test
RED on affected
mainatc896c09c42:GREEN on the NousResearch#85750 head at
e74ea76700:Additional verification:
Type of change
Checklist
Summary by Sourcery
Strengthen desktop gateway regression coverage for profile-owned remote WebSocket connections.
Tests: