fix(server): restore contract methods used by perf latency test#4
Conversation
The serverLatency.perf.test.ts cherry-pick referenced WS methods that weren't present in this fork, causing typecheck failures: - Adds subscribeOrchestrationDomainEvents streaming RPC so the perf harness can observe orchestration domain events end-to-end - Switches the git RPC latency probes to gitRefreshStatus, the existing equivalent of upstream's git.status unary RPC
github-actions
Bot
added
size:S
vouch:trusted
There was a problem hiding this comment.
Pull request overview
Restores missing websocket RPC contract surface needed by the server latency perf integration test, aligning this fork with the perf harness expectations.
Changes:
- Added a new websocket streaming RPC
subscribeOrchestrationDomainEventsthat streamsOrchestrationEventvalues (contract + group wiring). - Implemented the server-side websocket handler to expose
orchestrationEngine.streamDomainEventsviaobserveRpcStream. - Updated perf latency probes to use the fork’s existing
gitRefreshStatusRPC instead of upstreamgitStatus.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/contracts/src/rpc.ts | Adds the new WS method constant and WsSubscribeOrchestrationDomainEventsRpc, and includes it in WsRpcGroup. |
| apps/server/src/ws.ts | Adds the websocket RPC handler that streams orchestration domain events. |
| apps/server/integration/perf/serverLatency.perf.test.ts | Switches git latency probes from gitStatus to gitRefreshStatus. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| harness!.rpc.request((client) => | ||
| client[WS_METHODS.gitStatus]({ | ||
| client[WS_METHODS.gitRefreshStatus]({ | ||
| cwd: harness!.seededState.workspaceRoot, |
There was a problem hiding this comment.
The latency series is still labeled name: "git.status", but the RPC being executed is now WS_METHODS.gitRefreshStatus. This will make the perf artifact/metrics misleading; consider renaming the series (e.g. to git.refreshStatus) or otherwise keeping the label consistent with the actual RPC method.
| harness!.rpc.request((client) => | ||
| client[WS_METHODS.gitStatus]({ | ||
| client[WS_METHODS.gitRefreshStatus]({ | ||
| cwd: harness!.seededState.workspaceRoot, |
There was a problem hiding this comment.
The latency series is still labeled name: "git.status", but the RPC being executed is now WS_METHODS.gitRefreshStatus. To avoid confusing perf reports, update the series name (or otherwise ensure the label matches the method being measured).
Keeps the perf artifact/metric labels consistent with the RPC method actually being measured, addressing review feedback on #4.
Summary
Fixes the
@berkayorhan/bcode#typecheckfailure coming fromapps/server/integration/perf/serverLatency.perf.test.ts. The cherry-picked perf test referenced WS contract methods that hadn't been cherry-picked into this fork.subscribeOrchestrationDomainEventsstreaming RPC (contract + server handler) so the perf harness can observe raw orchestration domain events over the existing websocket RPC group.gitRefreshStatusunary RPC (upstreamgit.statusequivalent).Changes
packages/contracts/src/rpc.ts— addsWS_METHODS.subscribeOrchestrationDomainEvents, theWsSubscribeOrchestrationDomainEventsRpcdefinition (streamsOrchestrationEvent), and wires it intoWsRpcGroup.apps/server/src/ws.ts— adds the handler that returnsorchestrationEngine.streamDomainEventsthroughobserveRpcStream.apps/server/integration/perf/serverLatency.perf.test.ts— replacesWS_METHODS.gitStatuswithWS_METHODS.gitRefreshStatus.Test plan
bun turbo run typecheck— all 8 packages succeedbun run lint— no new errors