feat(windows): add HyperHQ launcher and bridge plugin#806
Open
wizzomafizzo wants to merge 21 commits into
Open
feat(windows): add HyperHQ launcher and bridge plugin#806wizzomafizzo wants to merge 21 commits into
wizzomafizzo wants to merge 21 commits into
Conversation
Adds a HyperHQ launcher to the Windows platform. Zaparoo Core hosts a named pipe at \\.\pipe\zaparoo-hyperhq-ipc and exchanges JSON-line messages with a bridge plugin (separate Go module under scripts/windows/hyperhq-plugin/) that HyperHQ launches as a Socket.IO plugin per the API at https://docs.hyperai.io/docs/plugins/. Two wire formats: pipe (Core <-> bridge) is internal PascalCase JSON; HyperHQ Socket.IO (bridge <-> HyperHQ) is camelCase per the API. The bridge handles challenge-response auth, plugin:register, subscribeEvents, requestData/dataResponse routing by requestId, and the hyperHqEvent envelope (gameLaunched / gameClosed). Bridge module is exempt from forbidigo and depguard so it can use stdlib sync and log without pulling Zaparoo Core internals into its dep tree. Linted via task cross-lint:windows since winio is Windows-only.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces a Windows HyperHQ gaming platform integration with a named-pipe bridge, launcher implementation, Custom system fallback, enhanced LaunchBox mapping logic, plugin packaging, and supporting test updates. The work includes ~4,000 lines of new code and is organized into a pipe server implementation, plugin bridge executable, platform wiring, build tasks, and related refactoring. ChangesHyperHQ Windows Integration
Sequence Diagram(s)sequenceDiagram
participant HyperHQ as HyperHQ Platform
participant Bridge as Plugin Bridge
participant ZCore as Zaparoo Core
HyperHQ->>Bridge: Socket.IO: initialize request
Bridge->>HyperHQ: Socket.IO: response + challenge
HyperHQ->>Bridge: Socket.IO: auth with sessionToken
Bridge->>Bridge: auth succeed, start pipe loop
Bridge->>ZCore: Named Pipe: connect & auth
ZCore->>Bridge: Named Pipe: GetSystems command
Bridge->>HyperHQ: Socket.IO: getSystems request
HyperHQ->>Bridge: Socket.IO: systems data response
Bridge->>ZCore: Named Pipe: Systems event
ZCore->>Bridge: Named Pipe: GetGamesForSystem
Bridge->>HyperHQ: Socket.IO: getGamesForSystem request
HyperHQ->>Bridge: Socket.IO: games data response
Bridge->>ZCore: Named Pipe: Games event
HyperHQ->>Bridge: Socket.IO: hyperHqEvent (gameLaunched)
Bridge->>ZCore: Named Pipe: MediaStarted event
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/windows/hyperhq-plugin/main.go (1)
224-857: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftPlease add tests for the new bridge module.
I don't see companion coverage in this diff for the auth handshake, pipe request routing, or reconnect/error paths, and this file carries most of the new protocol risk.
As per coding guidelines, "Write tests for all new code — follow TESTING.md and pkg/testing/README.md patterns".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/windows/hyperhq-plugin/main.go` around lines 224 - 857, Add unit/integration tests for the new bridge behavior: create tests exercising the authentication handshake and reconnect path (exercise connectSocket by driving "connect" and "authenticated" events and assert sessionToken set and authDone behavior), request/response routing (test requestData/requestDataCtx together with handleDataResponse to ensure pendingData mapping, cleanup, and error paths), lifecycle request handling (call handleLifecycleRequest with various hqLifecycleRequest payloads including missing ID and hqMethodShutdown to verify plugin:response emit and cancel), pipe interaction (exercise writePipeEvent, servePipeOnce/runPipeLoop by faking a pipe connection or using an in-memory net.Pipe to verify pushed Systems/Games and command routing via handlePipeCommand), and edge cases (decodeFirst, unmarshalIfPresent, newRequestID uniqueness/fallback). Follow TESTING.md and pkg/testing/README.md patterns: add a _test.go beside main.go, use test doubles/mocked socketio server and a fake pipe connection, control contexts/timeouts to exercise reconnects and error returns, and assert logs/returned errors where appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/windows/hyperhq-plugin/go.mod`:
- Line 3: The linter version in .golangci.yml is pinned to go: "1.25" which
conflicts with the root module's go 1.26.3 and the plugin's go 1.26.2; update
the go setting in .golangci.yml from "1.25" to "1.26.3" so the linter target
matches the root go.mod, and optionally also bump the plugin's go directive in
scripts/windows/hyperhq-plugin/go.mod from 1.26.2 to 1.26.3 to keep all go
versions consistent across the repo.
In `@scripts/windows/hyperhq-plugin/main.go`:
- Around line 584-586: The goroutines started by b.pushSystems and pushGames are
using the shared b.pipeWriter and can outlive servePipeOnce, causing late writes
to hit a new session; fix by making the work session-scoped: in servePipeOnce
create a per-connection context or generation ID and a session-scoped writer
(e.g. capture the current b.pipeWriter into a local variable or pass a
sessionWriter interface into pushSystems/pushGames), pass that session writer
and context/generation into the goroutines, have the goroutines check the
context/generation (or return early if writer is nil/closed) before writing, and
cancel or bump the generation on reconnect so late completions are dropped
instead of writing to the new pipe.
- Around line 384-386: The disconnect handler registered via onSocket should
clear the bridge's authentication state so requestDataCtx doesn't think the
bridge is still authenticated; inside the "disconnect" callback clear
b.sessionToken (and optionally b.authenticated or any flag used to indicate an
authenticated state) so subsequent transient reconnects don't use a stale token
until a new "authenticated" event repopulates it.
---
Outside diff comments:
In `@scripts/windows/hyperhq-plugin/main.go`:
- Around line 224-857: Add unit/integration tests for the new bridge behavior:
create tests exercising the authentication handshake and reconnect path
(exercise connectSocket by driving "connect" and "authenticated" events and
assert sessionToken set and authDone behavior), request/response routing (test
requestData/requestDataCtx together with handleDataResponse to ensure
pendingData mapping, cleanup, and error paths), lifecycle request handling (call
handleLifecycleRequest with various hqLifecycleRequest payloads including
missing ID and hqMethodShutdown to verify plugin:response emit and cancel), pipe
interaction (exercise writePipeEvent, servePipeOnce/runPipeLoop by faking a pipe
connection or using an in-memory net.Pipe to verify pushed Systems/Games and
command routing via handlePipeCommand), and edge cases (decodeFirst,
unmarshalIfPresent, newRequestID uniqueness/fallback). Follow TESTING.md and
pkg/testing/README.md patterns: add a _test.go beside main.go, use test
doubles/mocked socketio server and a fake pipe connection, control
contexts/timeouts to exercise reconnects and error returns, and assert
logs/returned errors where appropriate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 207c0fcd-bec9-4c97-98a6-7d6d7c3f1593
⛔ Files ignored due to path filters (1)
scripts/windows/hyperhq-plugin/go.sumis excluded by!**/*.sum
📒 Files selected for processing (13)
.golangci.ymlTaskfile.dist.ymlpkg/platforms/shared/schemes.gopkg/platforms/windows/hyperhq.gopkg/platforms/windows/hyperhq_test.gopkg/platforms/windows/launchbox.gopkg/platforms/windows/platform.goscripts/tasks/cross-lint.ymlscripts/tasks/windows.ymlscripts/windows/hyperhq-plugin/Dockerfilescripts/windows/hyperhq-plugin/go.modscripts/windows/hyperhq-plugin/main.goscripts/windows/hyperhq-plugin/plugin.json
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/platforms/windows/hyperhq.go (1)
1-986:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd missing unit coverage for HyperHQ request/response correlation
pkg/platforms/windows/hyperhq_test.goalready covers the HyperHQ wire structs (JSON serialization/deserialization), the mapping helpers (buildHqMappings,shouldIgnoreEmptyHqSystemsRefresh,buildHqSystemLookupviaTestHyperHqPlatformMapping), and scanner buffer sizing, plus “not connected” error paths forIsConnected/LaunchGame/RequestSystems.Add unit tests for
hqSystemQueryKeyand for the"Games"event handling logic that matchespendingGamesReq.queryKeyand sends onpendingGamesReq.response(there are currently no tests exercisingRequestGamesForSystemSyncor this pending-response correlation path).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/platforms/windows/hyperhq.go` around lines 1 - 986, Add unit tests to cover hqSystemQueryKey and the pending-games response correlation: write a test that verifies hqSystemQueryKey produces the expected null-separated key for a hqSystemQueryTarget, and write a test that simulates receiving a "Games" event string by calling handleEvent (or by invoking the scanner path) with a JSON payload whose SystemId/SystemName/SystemReferenceId matches a pendingGamesReq.queryKey; set up the HyperHqPipeServer with a pendingGamesReq.response channel (protected by pendingGamesReqMu) and assert the response channel receives the expected hqGamesResponse (including Error and Games fields), and also a negative case where queryKey mismatches so no send occurs. Use the server methods RequestGamesForSystemSync and/or directly manipulate pendingGamesReq/pendingGamesReqMu to emulate an in-flight request and ensure proper locking/unlocking around pendingGamesReq in the test.
🧹 Nitpick comments (1)
pkg/platforms/windows/hyperhq.go (1)
277-318: 💤 Low valueConsider using afero for filesystem detection to improve testability.
findHyperHqDirusesos.Statdirectly, making it difficult to unit test without real filesystem access. Injecting anafero.Fsparameter would allow mocking the filesystem in tests.As per coding guidelines: "Use afero for filesystem operations in testable code".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/platforms/windows/hyperhq.go` around lines 277 - 318, The findHyperHqDir function currently calls os.Stat directly which prevents unit tests from mocking the filesystem; change its signature to accept an afero.Fs (e.g. func findHyperHqDir(fs afero.Fs, cfg *config.Instance) (string, error)), keep using os.UserHomeDir() for home resolution, replace any os.Stat(dir) checks with fs.Stat(dir) (or afero.Exists equivalents) and update all callers to pass an appropriate Fs (real afero.NewOsFs() in production, afero.NewMemMapFs() in tests); update tests to inject a mock in-memory FS and assert directory discovery without touching the real filesystem.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@pkg/platforms/windows/hyperhq.go`:
- Around line 1-986: Add unit tests to cover hqSystemQueryKey and the
pending-games response correlation: write a test that verifies hqSystemQueryKey
produces the expected null-separated key for a hqSystemQueryTarget, and write a
test that simulates receiving a "Games" event string by calling handleEvent (or
by invoking the scanner path) with a JSON payload whose
SystemId/SystemName/SystemReferenceId matches a pendingGamesReq.queryKey; set up
the HyperHqPipeServer with a pendingGamesReq.response channel (protected by
pendingGamesReqMu) and assert the response channel receives the expected
hqGamesResponse (including Error and Games fields), and also a negative case
where queryKey mismatches so no send occurs. Use the server methods
RequestGamesForSystemSync and/or directly manipulate
pendingGamesReq/pendingGamesReqMu to emulate an in-flight request and ensure
proper locking/unlocking around pendingGamesReq in the test.
---
Nitpick comments:
In `@pkg/platforms/windows/hyperhq.go`:
- Around line 277-318: The findHyperHqDir function currently calls os.Stat
directly which prevents unit tests from mocking the filesystem; change its
signature to accept an afero.Fs (e.g. func findHyperHqDir(fs afero.Fs, cfg
*config.Instance) (string, error)), keep using os.UserHomeDir() for home
resolution, replace any os.Stat(dir) checks with fs.Stat(dir) (or afero.Exists
equivalents) and update all callers to pass an appropriate Fs (real
afero.NewOsFs() in production, afero.NewMemMapFs() in tests); update tests to
inject a mock in-memory FS and assert directory discovery without touching the
real filesystem.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac8b2721-0204-436f-a7f5-d90ee18e46d3
⛔ Files ignored due to path filters (1)
scripts/windows/hyperhq-plugin/go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
pkg/assets/systems/Custom.jsonpkg/database/systemdefs/systemdefs.gopkg/platforms/windows/hyperhq.gopkg/platforms/windows/hyperhq_test.gopkg/platforms/windows/launchbox.gopkg/platforms/windows/launchbox_test.gopkg/platforms/windows/platform.gopkg/service/clock_monitor.gopkg/service/clock_monitor_test.goscripts/tasks/windows.ymlscripts/windows/hyperhq-plugin/go.modscripts/windows/hyperhq-plugin/main.goscripts/windows/hyperhq-plugin/main_test.goscripts/windows/hyperhq-plugin/pipe_windows.goscripts/windows/hyperhq-plugin/plugin.json
💤 Files with no reviewable changes (1)
- scripts/tasks/windows.yml
✅ Files skipped from review due to trivial changes (1)
- pkg/assets/systems/Custom.json
🚧 Files skipped from review as they are similar to previous changes (4)
- scripts/windows/hyperhq-plugin/pipe_windows.go
- pkg/platforms/windows/platform.go
- pkg/platforms/windows/hyperhq_test.go
- scripts/windows/hyperhq-plugin/main.go
* fix(windows): align HyperHQ plugin socket auth * fix(windows): use IPv4 loopback for HyperHQ plugin socket * fix(windows): avoid HyperHQ plugin auth timeout * Revert "fix(windows): avoid HyperHQ plugin auth timeout" This reverts commit 7cee7d0. * fix(windows): keep HyperHQ socket path on IPv4 loopback * fix(windows): update HyperHQ plugin test and Go patch * test(windows): fix HyperHQ plugin test lint --------- Co-authored-by: Callan Barrett <callan@zoocar.org>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/api/ws_dispatcher_test.go (2)
301-310: ⚡ Quick winMove
countingRequestTrackertype definition near the top of the file.Place this type (and related consts/types) before function declarations to match project Go file structure rules.
As per coding guidelines, "Define Go types and consts near the top of the file, before functions and methods".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/api/ws_dispatcher_test.go` around lines 301 - 310, Move the countingRequestTracker type and its methods (countingRequestTracker, RequestStarted, RequestEnded, inFlight) up near the top of the Go file alongside other types and consts so it is declared before any function definitions; relocate the entire block (type countingRequestTracker struct { count int } plus the three receiver methods) to the file header area where other type/const declarations live to comply with the project's Go file structure rules.
125-150: ⚡ Quick winAvoid hardcoded RPC IDs/read count in this ordering test.
Using fixed IDs (
3,4) and a fixed response read count (4) makes this test brittle against dispatcher concurrency constant changes and can create ambiguous assertions if IDs overlap with the initial burst.Suggested refactor
+ targetImageID := wsLowConcurrency + 1 + targetRunID := wsLowConcurrency + 2 + require.NoError(t, conn.WriteMessage(websocket.TextMessage, - []byte(`{"jsonrpc":"2.0","method":"media.image","id":3}`))) + []byte(fmt.Sprintf(`{"jsonrpc":"2.0","method":"media.image","id":%d}`, targetImageID)))) require.NoError(t, conn.WriteMessage(websocket.TextMessage, - []byte(`{"jsonrpc":"2.0","method":"run","id":4}`))) + []byte(fmt.Sprintf(`{"jsonrpc":"2.0","method":"run","id":%d}`, targetRunID)))) @@ - seen := make([]models.RPCID, 0, 4) - for range 4 { + seen := make([]models.RPCID, 0, wsLowConcurrency+2) + for range wsLowConcurrency + 2 { @@ - highIndex := indexRPCID(seen, models.NewNumberID(4)) - thirdImageIndex := indexRPCID(seen, models.NewNumberID(3)) + highIndex := indexRPCID(seen, models.NewNumberID(targetRunID)) + thirdImageIndex := indexRPCID(seen, models.NewNumberID(targetImageID))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/api/ws_dispatcher_test.go` around lines 125 - 150, The test currently uses hardcoded RPC IDs (3 and 4) and a fixed read loop of 4 which is brittle; instead generate unique IDs into variables (e.g., imageID := models.NewNumberID(n) and highID := models.NewNumberID(n+1)) and use those when sending the two requests, then replace the fixed read-for-range loop with a read-until loop that collects responses into seen (using conn.ReadMessage with a deadline) and exits when both expected IDs are observed or a timeout occurs; use indexRPCID(seen, highID) and indexRPCID(seen, imageID) for the final ordering assertion, and keep existing synchronization points (highStarted, close(releaseImages)) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/api/ws_dispatcher_test.go`:
- Line 56: The test currently discards the error from state.NewState; change the
call to capture the error (e.g., st, err := state.NewState(nil, "test-boot"))
and fail the test when err != nil (use t.Fatalf or require.NoError) so setup
failures are reported immediately; update the surrounding test in
ws_dispatcher_test.go to handle the error before using st.
---
Nitpick comments:
In `@pkg/api/ws_dispatcher_test.go`:
- Around line 301-310: Move the countingRequestTracker type and its methods
(countingRequestTracker, RequestStarted, RequestEnded, inFlight) up near the top
of the Go file alongside other types and consts so it is declared before any
function definitions; relocate the entire block (type countingRequestTracker
struct { count int } plus the three receiver methods) to the file header area
where other type/const declarations live to comply with the project's Go file
structure rules.
- Around line 125-150: The test currently uses hardcoded RPC IDs (3 and 4) and a
fixed read loop of 4 which is brittle; instead generate unique IDs into
variables (e.g., imageID := models.NewNumberID(n) and highID :=
models.NewNumberID(n+1)) and use those when sending the two requests, then
replace the fixed read-for-range loop with a read-until loop that collects
responses into seen (using conn.ReadMessage with a deadline) and exits when both
expected IDs are observed or a timeout occurs; use indexRPCID(seen, highID) and
indexRPCID(seen, imageID) for the final ordering assertion, and keep existing
synchronization points (highStarted, close(releaseImages)) intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b0cb92c0-12b6-4b6c-b893-818ce2835d3a
⛔ Files ignored due to path filters (1)
scripts/windows/hyperhq-plugin/go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
Taskfile.dist.ymlpkg/api/ws_dispatcher_test.gopkg/platforms/windows/platform.gopkg/service/clock_monitor.goscripts/windows/hyperhq-plugin/go.mod
💤 Files with no reviewable changes (1)
- scripts/windows/hyperhq-plugin/go.mod
✅ Files skipped from review due to trivial changes (2)
- pkg/service/clock_monitor.go
- Taskfile.dist.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/platforms/windows/platform.go
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/api/ws_dispatcher_test.go (2)
301-310: ⚡ Quick winMove
countingRequestTrackertype definition near the top of the file.Place this type (and related consts/types) before function declarations to match project Go file structure rules.
As per coding guidelines, "Define Go types and consts near the top of the file, before functions and methods".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/api/ws_dispatcher_test.go` around lines 301 - 310, Move the countingRequestTracker type and its methods (countingRequestTracker, RequestStarted, RequestEnded, inFlight) up near the top of the Go file alongside other types and consts so it is declared before any function definitions; relocate the entire block (type countingRequestTracker struct { count int } plus the three receiver methods) to the file header area where other type/const declarations live to comply with the project's Go file structure rules.
125-150: ⚡ Quick winAvoid hardcoded RPC IDs/read count in this ordering test.
Using fixed IDs (
3,4) and a fixed response read count (4) makes this test brittle against dispatcher concurrency constant changes and can create ambiguous assertions if IDs overlap with the initial burst.Suggested refactor
+ targetImageID := wsLowConcurrency + 1 + targetRunID := wsLowConcurrency + 2 + require.NoError(t, conn.WriteMessage(websocket.TextMessage, - []byte(`{"jsonrpc":"2.0","method":"media.image","id":3}`))) + []byte(fmt.Sprintf(`{"jsonrpc":"2.0","method":"media.image","id":%d}`, targetImageID)))) require.NoError(t, conn.WriteMessage(websocket.TextMessage, - []byte(`{"jsonrpc":"2.0","method":"run","id":4}`))) + []byte(fmt.Sprintf(`{"jsonrpc":"2.0","method":"run","id":%d}`, targetRunID)))) @@ - seen := make([]models.RPCID, 0, 4) - for range 4 { + seen := make([]models.RPCID, 0, wsLowConcurrency+2) + for range wsLowConcurrency + 2 { @@ - highIndex := indexRPCID(seen, models.NewNumberID(4)) - thirdImageIndex := indexRPCID(seen, models.NewNumberID(3)) + highIndex := indexRPCID(seen, models.NewNumberID(targetRunID)) + thirdImageIndex := indexRPCID(seen, models.NewNumberID(targetImageID))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/api/ws_dispatcher_test.go` around lines 125 - 150, The test currently uses hardcoded RPC IDs (3 and 4) and a fixed read loop of 4 which is brittle; instead generate unique IDs into variables (e.g., imageID := models.NewNumberID(n) and highID := models.NewNumberID(n+1)) and use those when sending the two requests, then replace the fixed read-for-range loop with a read-until loop that collects responses into seen (using conn.ReadMessage with a deadline) and exits when both expected IDs are observed or a timeout occurs; use indexRPCID(seen, highID) and indexRPCID(seen, imageID) for the final ordering assertion, and keep existing synchronization points (highStarted, close(releaseImages)) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/api/ws_dispatcher_test.go`:
- Line 56: The test currently discards the error from state.NewState; change the
call to capture the error (e.g., st, err := state.NewState(nil, "test-boot"))
and fail the test when err != nil (use t.Fatalf or require.NoError) so setup
failures are reported immediately; update the surrounding test in
ws_dispatcher_test.go to handle the error before using st.
---
Nitpick comments:
In `@pkg/api/ws_dispatcher_test.go`:
- Around line 301-310: Move the countingRequestTracker type and its methods
(countingRequestTracker, RequestStarted, RequestEnded, inFlight) up near the top
of the Go file alongside other types and consts so it is declared before any
function definitions; relocate the entire block (type countingRequestTracker
struct { count int } plus the three receiver methods) to the file header area
where other type/const declarations live to comply with the project's Go file
structure rules.
- Around line 125-150: The test currently uses hardcoded RPC IDs (3 and 4) and a
fixed read loop of 4 which is brittle; instead generate unique IDs into
variables (e.g., imageID := models.NewNumberID(n) and highID :=
models.NewNumberID(n+1)) and use those when sending the two requests, then
replace the fixed read-for-range loop with a read-until loop that collects
responses into seen (using conn.ReadMessage with a deadline) and exits when both
expected IDs are observed or a timeout occurs; use indexRPCID(seen, highID) and
indexRPCID(seen, imageID) for the final ordering assertion, and keep existing
synchronization points (highStarted, close(releaseImages)) intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b0cb92c0-12b6-4b6c-b893-818ce2835d3a
⛔ Files ignored due to path filters (1)
scripts/windows/hyperhq-plugin/go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
Taskfile.dist.ymlpkg/api/ws_dispatcher_test.gopkg/platforms/windows/platform.gopkg/service/clock_monitor.goscripts/windows/hyperhq-plugin/go.mod
💤 Files with no reviewable changes (1)
- scripts/windows/hyperhq-plugin/go.mod
✅ Files skipped from review due to trivial changes (2)
- pkg/service/clock_monitor.go
- Taskfile.dist.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/platforms/windows/platform.go
🛑 Comments failed to post (1)
pkg/api/ws_dispatcher_test.go (1)
56-56:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle
state.NewStateerror instead of discarding it.Ignoring the error here can hide setup failures and make downstream assertions misleading.
Proposed fix
- st, _ := state.NewState(nil, "test-boot") + st, err := state.NewState(nil, "test-boot") + require.NoError(t, err)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.st, err := state.NewState(nil, "test-boot") require.NoError(t, err)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/api/ws_dispatcher_test.go` at line 56, The test currently discards the error from state.NewState; change the call to capture the error (e.g., st, err := state.NewState(nil, "test-boot")) and fail the test when err != nil (use t.Fatalf or require.NoError) so setup failures are reported immediately; update the surrounding test in ws_dispatcher_test.go to handle the error before using st.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
\\.\pipe\zaparoo-hyperhq-ipcand exchanges JSON-line messages with a separate bridge plugin underscripts/windows/hyperhq-plugin/that HyperHQ runs as a Socket.IO plugin per the HyperAI plugin API.plugin:register,subscribeEvents,requestData/dataResponserouting byrequestId, and thehyperHqEventenvelope (gameLaunched/gameClosed).syncandlogwithout pulling Zaparoo Core internals into its dep tree. Linted viatask cross-lint:windowssince winio is Windows-only.Summary by CodeRabbit
Release Notes
New Features
Improvements