feat(hub): poll remote API for beta/dev servers #96

smell-of-curry · 2026-07-22T23:57:38Z

smell-of-curry commented

2026-07-22 23:57:38 +00:00

(Migrated from github.com)

Dynamic dev-* registry + gated Beta Navigator item for Supporters/Mod+, shared CanAccessBeta helper.

Summary by CodeRabbit

New Features
- Added a Beta Navigator for eligible players to browse and select development servers, including live status and capacity details.
- Added automatic synchronization of available development servers.
- Added configurable development-server management settings and polling.
- Beta access is now available to supporters and higher-ranked players.
- Lobby menus now separate development servers from standard servers.
- Lobby kits are restored after player ranks load, except when parkour items are equipped.
Tests
- Added coverage for server synchronization and beta-access rules.

Dynamic dev-* registry + gated Beta Navigator item for Supporters/Mod+, shared CanAccessBeta helper.  ## Summary by CodeRabbit * **New Features** * Added a Beta Navigator for eligible players to browse and select development servers, including live status and capacity details. * Added automatic synchronization of available development servers. * Added configurable development-server management settings and polling. * Beta access is now available to supporters and higher-ranked players. * Lobby menus now separate development servers from standard servers. * Lobby kits are restored after player ranks load, except when parkour items are equipped. * **Tests** * Added coverage for server synchronization and beta-access rules.

coderabbitai[bot] commented

2026-07-22 23:57:53 +00:00

(Migrated from github.com)

📝 Walkthrough

Walkthrough

Adds configurable polling for remote dev servers, synchronizes them with the local registry, exposes beta servers through a gated navigator, excludes them from the standard navigator, and refreshes lobby kits after rank synchronization.

Changes

Dev server and beta access flow

Layer / File(s)	Summary
Polling and registry synchronization `config.example.toml`, `pokebedrock/config.go`, `pokebedrock/devserver/*`	Defines dev-server settings and API models, polls the manager endpoint, and registers, updates, or unregisters dynamic servers based on state differences.
Application service lifecycle wiring `pokebedrock/pokebedrock.go`	Starts the dev-server service from configuration and stops it during application shutdown.
Beta access and server navigation `pokebedrock/rank/`, `pokebedrock/handler/player.go`, `pokebedrock/kit/lobby.go`, `pokebedrock/form/`	Centralizes beta eligibility, conditionally adds the beta navigator, renders dynamic servers, and removes them from the standard server navigator.
Rank synchronization and lobby kit refresh `pokebedrock/session/ranks.go`	Reapplies the lobby kit after rank synchronization unless parkour kit items are present.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Player
  participant PlayerHandler
  participant BetaNavigator
  participant Registry
  Player->>PlayerHandler: Select beta-navigator
  PlayerHandler->>BetaNavigator: Open NewBetaNavigator()
  BetaNavigator->>Registry: Enumerate dev-identified servers
  Registry-->>BetaNavigator: Server status and capacity
  BetaNavigator-->>Player: Show beta server buttons
  Player->>BetaNavigator: Select server
  BetaNavigator->>Registry: Resolve selected dev server
  Registry-->>BetaNavigator: Server configuration
  BetaNavigator-->>Player: Show server confirmation

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: polling a remote API for beta/dev servers in the hub.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch dev-servers

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

Error: can't load config: can't unmarshal config by viper (flags, file): 1 error(s) decoding:

'output.formats' expected a map, got 'slice'
The command is terminated due to an error: can't load config: can't unmarshal config by viper (flags, file): 1 error(s) decoding:

'output.formats' expected a map, got 'slice'

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

[![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/smell-of-curry/pokebedrock-hub/pull/96?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)   <details> <summary>📝 Walkthrough</summary> ## Walkthrough Adds configurable polling for remote dev servers, synchronizes them with the local registry, exposes beta servers through a gated navigator, excludes them from the standard navigator, and refreshes lobby kits after rank synchronization. ### Changes **Dev server and beta access flow** |Layer / File(s)|Summary| |---|---| |**Polling and registry synchronization** `config.example.toml`, `pokebedrock/config.go`, `pokebedrock/devserver/*`|Defines dev-server settings and API models, polls the manager endpoint, and registers, updates, or unregisters dynamic servers based on state differences.| |**Application service lifecycle wiring** `pokebedrock/pokebedrock.go`|Starts the dev-server service from configuration and stops it during application shutdown.| |**Beta access and server navigation** `pokebedrock/rank/*`, `pokebedrock/handler/player.go`, `pokebedrock/kit/lobby.go`, `pokebedrock/form/*`|Centralizes beta eligibility, conditionally adds the beta navigator, renders dynamic servers, and removes them from the standard server navigator.| |**Rank synchronization and lobby kit refresh** `pokebedrock/session/ranks.go`|Reapplies the lobby kit after rank synchronization unless parkour kit items are present.| **Estimated code review effort:** 4 (Complex) | ~45 minutes ### Sequence Diagram(s) ```mermaid sequenceDiagram participant Player participant PlayerHandler participant BetaNavigator participant Registry Player->>PlayerHandler: Select beta-navigator PlayerHandler->>BetaNavigator: Open NewBetaNavigator() BetaNavigator->>Registry: Enumerate dev-identified servers Registry-->>BetaNavigator: Server status and capacity BetaNavigator-->>Player: Show beta server buttons Player->>BetaNavigator: Select server BetaNavigator->>Registry: Resolve selected dev server Registry-->>BetaNavigator: Server configuration BetaNavigator-->>Player: Show server confirmation ``` </details>   <details> <summary>🚥 Pre-merge checks | ✅ 4 | ❌ 1</summary> ### ❌ Failed checks (1 warning) | Check name | Status | Explanation | Resolution | | :----------------: | :--------- | :------------------------------------------------------------------------------------ | :--------------------------------------------------------------------------------- | | Docstring Coverage | ⚠️ Warning | Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | <details> <summary>✅ Passed checks (4 passed)</summary> | Check name | Status | Explanation | | :------------------------: | :------- | :-------------------------------------------------------------------------------------------------- | | Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. | | Title check | ✅ Passed | The title clearly summarizes the main change: polling a remote API for beta/dev servers in the hub. | | Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. | | Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. | </details> </details>   <details> <summary>✨ Finishing Touches</summary> <details> <summary>📝 Generate docstrings</summary> - [ ]  Create stacked PR - [ ]  Commit on current branch </details> <details> <summary>🧪 Generate unit tests (beta)</summary> - [ ]  Create PR with unit tests - [ ]  Commit unit tests in branch `dev-servers` </details> </details>   > [!WARNING] > There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. > > <details> > <summary>🔧 golangci-lint (2.12.2)</summary> > > Error: can't load config: can't unmarshal config by viper (flags, file): 1 error(s) decoding: > > * 'output.formats' expected a map, got 'slice' > The command is terminated due to an error: can't load config: can't unmarshal config by viper (flags, file): 1 error(s) decoding: > > * 'output.formats' expected a map, got 'slice' > > > > > </details>   --- Thanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=smell-of-curry/pokebedrock-hub&utm_content=96)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. <details> <summary>❤️ Share</summary> - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code) </details> Comment `@coderabbitai help` to get the list of available commands.

coderabbitai[bot] (Migrated from github.com) reviewed

2026-07-23 00:07:52 +00:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 3

🤖 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 `@pokebedrock/devserver/service.go`:
- Around line 88-98: Update the Service lifecycle and poller context so
in-flight fetch requests use a stored service-level cancel context rather than a
standalone context.Background cancellation scope. Initialize and retain that
cancel function when starting the poller, invoke it in Stop before waiting, and
ensure the poller’s done signal is emitted only after the cancelled request and
any server registration cleanup finish.

In `@pokebedrock/devserver/types.go`:
- Around line 72-86: The DesiredConfigs function must validate each running
server’s Port before constructing and registering its srv.Config. Reject ports
outside 1..65535 by returning an error, ensuring pollOnce retains the current
registry rather than applying invalid updates; leave valid server processing
unchanged.

In `@pokebedrock/form/beta_navigator.go`:
- Around line 49-59: Enforce current beta eligibility at every access path: in
pokebedrock/form/beta_navigator.go lines 49-59, recheck the submitter’s ranks
before calling NewServerConfirm; in pokebedrock/handler/player.go lines 99-100,
reject "beta-navigator" actions when CanAccessBeta() is false; and in
pokebedrock/session/ranks.go lines 143-149, refresh the guarded lobby kit after
resetting ranks to UnLinked on fetch failure so stale gated items are removed.

🪄 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: c35154a3-aa48-4576-b63b-31f44987b24f

📥 Commits

Reviewing files that changed from the base of the PR and between fddd8bfcc2 and 193c36337b.

📒 Files selected for processing (14)

config.example.toml
pokebedrock/config.go
pokebedrock/devserver/diff_test.go
pokebedrock/devserver/service.go
pokebedrock/devserver/types.go
pokebedrock/form/beta_navigator.go
pokebedrock/form/server_confirm.go
pokebedrock/form/server_navigator.go
pokebedrock/handler/player.go
pokebedrock/kit/lobby.go
pokebedrock/pokebedrock.go
pokebedrock/rank/access.go
pokebedrock/rank/access_test.go
pokebedrock/session/ranks.go

**Actionable comments posted: 3** <details> <summary>🤖 Prompt for all review comments with AI agents</summary> ``` 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 `@pokebedrock/devserver/service.go`: - Around line 88-98: Update the Service lifecycle and poller context so in-flight fetch requests use a stored service-level cancel context rather than a standalone context.Background cancellation scope. Initialize and retain that cancel function when starting the poller, invoke it in Stop before waiting, and ensure the poller’s done signal is emitted only after the cancelled request and any server registration cleanup finish. In `@pokebedrock/devserver/types.go`: - Around line 72-86: The DesiredConfigs function must validate each running server’s Port before constructing and registering its srv.Config. Reject ports outside 1..65535 by returning an error, ensuring pollOnce retains the current registry rather than applying invalid updates; leave valid server processing unchanged. In `@pokebedrock/form/beta_navigator.go`: - Around line 49-59: Enforce current beta eligibility at every access path: in pokebedrock/form/beta_navigator.go lines 49-59, recheck the submitter’s ranks before calling NewServerConfirm; in pokebedrock/handler/player.go lines 99-100, reject "beta-navigator" actions when CanAccessBeta() is false; and in pokebedrock/session/ranks.go lines 143-149, refresh the guarded lobby kit after resetting ranks to UnLinked on fetch failure so stale gated items are removed. ``` </details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ]  Push a commit to this branch (recommended) - [ ]  Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `c35154a3-aa48-4576-b63b-31f44987b24f` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between fddd8bfcc2aef23f9b367cc3167855f3f1156610 and 193c36337ba7fdcca235b0b886126e04bf9540be. </details> <details> <summary>📒 Files selected for processing (14)</summary> * `config.example.toml` * `pokebedrock/config.go` * `pokebedrock/devserver/diff_test.go` * `pokebedrock/devserver/service.go` * `pokebedrock/devserver/types.go` * `pokebedrock/form/beta_navigator.go` * `pokebedrock/form/server_confirm.go` * `pokebedrock/form/server_navigator.go` * `pokebedrock/handler/player.go` * `pokebedrock/kit/lobby.go` * `pokebedrock/pokebedrock.go` * `pokebedrock/rank/access.go` * `pokebedrock/rank/access_test.go` * `pokebedrock/session/ranks.go` </details> </details>

pokebedrock/devserver/service.go Outdated

					
				@ -0,0 +95,4 @@

					select {

					case <-s.done:

					case <-time.After(3 * time.Second):

					}

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C3 'func \(s \*Service\) Stop|context\.Background|WithTimeout|time\.After|pollOnce' \
  pokebedrock/devserver/service.go

Repository: smell-of-curry/pokebedrock-hub

Length of output: 1086

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## file outline"
ast-grep outline pokebedrock/devserver/service.go --view expanded || true

echo
echo "## service.go relevant sections"
cat -n pokebedrock/devserver/service.go | sed -n '1,230p'

echo
echo "## usages of Stop and PokeBedrock.Close"
rg -n -C3 '\bStop\(\)|PokeBedrock\.Close\(|\.Close\(' -g '*.go' .

Repository: smell-of-curry/pokebedrock-hub

Length of output: 23721

Cancel an in-flight poll before shutdown continues.

Stop gives the poller a 3s shutdown window, but a long fetch request can run for 5s because its cancel only wraps context.Background(). If that call is pending when the ticker fires after Stop, it returns after the 3s timeout and then registers/unregisters servers after PokeBedrock.Close() has already called the stop and proceeds to close the server. Store a service-level cancel context, cancel it from Stop, and wait for done from the cancelled request.

🤖 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 `@pokebedrock/devserver/service.go` around lines 88 - 98, Update the Service
lifecycle and poller context so in-flight fetch requests use a stored
service-level cancel context rather than a standalone context.Background
cancellation scope. Initialize and retain that cancel function when starting the
poller, invoke it in Stop before waiting, and ensure the poller’s done signal is
emitted only after the cancelled request and any server registration cleanup
finish.

_🩺 Stability & Availability_ | _🟠 Major_ | _⚡ Quick win_ <details> <summary>🧩 Analysis chain</summary> 🏁 Script executed: ```shell #!/bin/bash rg -n -C3 'func \(s \*Service\) Stop|context\.Background|WithTimeout|time\.After|pollOnce' \ pokebedrock/devserver/service.go ``` Repository: smell-of-curry/pokebedrock-hub Length of output: 1086 --- 🏁 Script executed: ```shell #!/bin/bash set -euo pipefail echo "## file outline" ast-grep outline pokebedrock/devserver/service.go --view expanded || true echo echo "## service.go relevant sections" cat -n pokebedrock/devserver/service.go | sed -n '1,230p' echo echo "## usages of Stop and PokeBedrock.Close" rg -n -C3 '\bStop\(\)|PokeBedrock\.Close\(|\.Close\(' -g '*.go' . ``` Repository: smell-of-curry/pokebedrock-hub Length of output: 23721 --- </details> **Cancel an in-flight poll before shutdown continues.** `Stop` gives the poller a 3s shutdown window, but a long `fetch` request can run for 5s because its cancel only wraps `context.Background()`. If that call is pending when the ticker fires after `Stop`, it returns after the 3s timeout and then registers/unregisters servers after `PokeBedrock.Close()` has already called the stop and proceeds to close the server. Store a service-level cancel context, cancel it from `Stop`, and wait for `done` from the cancelled request. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pokebedrock/devserver/service.go` around lines 88 - 98, Update the Service lifecycle and poller context so in-flight fetch requests use a stored service-level cancel context rather than a standalone context.Background cancellation scope. Initialize and retain that cancel function when starting the poller, invoke it in Stop before waiting, and ensure the poller’s done signal is emitted only after the cancelled request and any server registration cleanup finish. ``` </details>

smell-of-curry marked this conversation as resolved

pokebedrock/devserver/types.go Outdated

					
				@ -0,0 +83,4 @@

							Name:       DisplayName(s),

							Identifier: id,

							Address:    net.JoinHostPort(host, strconv.Itoa(s.Port)),

							BetaLock:   true,

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C2 'JoinHostPort|s\.Port|DesiredConfigs' pokebedrock/devserver

Repository: smell-of-curry/pokebedrock-hub

Length of output: 2670

🏁 Script executed:

#!/bin/bash
set -eu

printf '--- files ---\n'
git ls-files pokebedrock/devserver

printf '\n--- types.go relevant section ---\n'
cat -n pokebedrock/devserver/types.go | sed -n '1,130p'

printf '\n--- service.go polling section ---\n'
cat -n pokebedrock/devserver/service.go | sed -n '100,140p'

printf '\n--- tests for desired configs ---\n'
cat -n pokebedrock/devserver/diff_test.go | sed -n '1,110p'

printf '\n--- behavioral probe: net.JoinHostPort formatting ---\n'
go run - <<'GO'
package main

import (
	"fmt"
	"net"
	"strconv"
)

func main() {
	ports := []string{"0", "-1", "65535", "65536", "1", "100000"}
	for _, p := range ports {
		fmt.Printf("%s -> %q\n", p, net.JoinHostPort("example.com", p))
	}
	fmt.Printf("valid 0: %v\n", net.JoinHostPort("example.com", strconv.Itoa(0)))
	fmt.Printf("valid -1: %v\n", net.JoinHostPort("example.com", strconv.Itoa(-1)))
	fmt.Printf("valid 65536: %v\n", net.JoinHostPort("example.com", strconv.Itoa(65536)))
}
GO

Repository: smell-of-curry/pokebedrock-hub

Length of output: 259

🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- files ---'
git ls-files pokebedrock/devserver

printf '%s\n' ''
printf '%s\n' '--- types.go relevant section ---'
cat -n pokebedrock/devserver/types.go | sed -n '1,130p'

printf '%s\n' ''
printf '%s\n' '--- service.go polling section ---'
cat -n pokebedrock/devserver/service.go | sed -n '100,140p'

printf '%s\n' ''
printf '%s\n' '--- tests for desired configs ---'
cat -n pokebedrock/devserver/diff_test.go | sed -n '1,110p'

printf '%s\n' ''
printf '%s\n' '--- behavioral probe: net.JoinHostPort formatting ---'
go run - <<'GO'
package main

import (
	"fmt"
	"net"
	"strconv"
)

func main() {
	ports := []string{"0", "-1", "65535", "65536", "1", "100000"}
	for _, p := range ports {
		fmt.Printf("%s -> %q\n", p, net.JoinHostPort("example.com", p))
	}
	fmt.Printf("valid 0: %v\n", net.JoinHostPort("example.com", strconv.Itoa(0)))
	fmt.Printf("valid -1: %v\n", net.JoinHostPort("example.com", strconv.Itoa(-1)))
	fmt.Printf("valid 65536: %v\n", net.JoinHostPort("example.com", strconv.Itoa(65536)))
}
GO

Repository: smell-of-curry/pokebedrock-hub

Length of output: 9759

🌐 Web query:

Go net JoinHostPort host port 0 65536 formatting behavior

💡 Result:

The Go net.JoinHostPort(host, port) function does not perform validation on the host or port arguments [1][2]. It is a string manipulation utility that combines a host and a port into the "host:port" format, or "[host]:port" if the host contains colons (typically identifying it as an IPv6 address) [1][2]. Regarding your query about the 0 to 65536 range: 1. No Validation: JoinHostPort does not check if the port is a valid numeric port, nor does it enforce the 0–65536 range [1][2]. It accepts any string for the port argument and simply appends it to the host separated by a colon [1][2]. 2. Formatting Behavior: - If the host contains a colon, the function wraps the host in square brackets: [host]:port [1][2]. - If the host does not contain a colon, it returns the host directly followed by a colon and the port: host:port [1][2]. - If the port argument is empty, the function still appends the colon, resulting in "host:" or "[host]:" [3][4]. Because the function treats the port as a raw string, passing "65536" or "0" will result in those literal strings being placed after the colon without error [1][2]. Any validation of the port range must be performed by the caller before calling JoinHostPort [1].

Citations:

Reject invalid remote server ports before registering them.

net.JoinHostPort formats ports as strings without range validation, so a malformed port from the /dev-servers response can be registered as unreachable. Validate Port as 1..65535 so pollOnce keeps the current registry instead of applying an invalid update.

Proposed fix

 	for _, s := range servers {
 		if !strings.EqualFold(s.Status, "running") {
 			continue
 		}
+		if s.Port < 1 || s.Port > 65535 {
+			return nil, fmt.Errorf("server %q has invalid port %d", s.Name, s.Port)
+		}
 		id, err := IdentifierFor(s)

📝 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.

func DesiredConfigs(servers []APIServer, host string) (map[string]srv.Config, error) {
	desired := make(map[string]srv.Config, len(servers))
	for _, s := range servers {
		if !strings.EqualFold(s.Status, "running") {
			continue
		}
		if s.Port < 1 || s.Port > 65535 {
			return nil, fmt.Errorf("server %q has invalid port %d", s.Name, s.Port)
		}
		id, err := IdentifierFor(s)
		if err != nil {
			return nil, err
		}
		desired[id] = srv.Config{
			Name:       DisplayName(s),
			Identifier: id,
			Address:    net.JoinHostPort(host, strconv.Itoa(s.Port)),
			BetaLock:   true,

🤖 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 `@pokebedrock/devserver/types.go` around lines 72 - 86, The DesiredConfigs
function must validate each running server’s Port before constructing and
registering its srv.Config. Reject ports outside 1..65535 by returning an error,
ensuring pollOnce retains the current registry rather than applying invalid
updates; leave valid server processing unchanged.

_🩺 Stability & Availability_ | _🟠 Major_ | _⚡ Quick win_ <details> <summary>🧩 Analysis chain</summary> 🏁 Script executed: ```shell #!/bin/bash rg -n -C2 'JoinHostPort|s\.Port|DesiredConfigs' pokebedrock/devserver ``` Repository: smell-of-curry/pokebedrock-hub Length of output: 2670 --- 🏁 Script executed: ```shell #!/bin/bash set -eu printf '--- files ---\n' git ls-files pokebedrock/devserver printf '\n--- types.go relevant section ---\n' cat -n pokebedrock/devserver/types.go | sed -n '1,130p' printf '\n--- service.go polling section ---\n' cat -n pokebedrock/devserver/service.go | sed -n '100,140p' printf '\n--- tests for desired configs ---\n' cat -n pokebedrock/devserver/diff_test.go | sed -n '1,110p' printf '\n--- behavioral probe: net.JoinHostPort formatting ---\n' go run - <<'GO' package main import ( "fmt" "net" "strconv" ) func main() { ports := []string{"0", "-1", "65535", "65536", "1", "100000"} for _, p := range ports { fmt.Printf("%s -> %q\n", p, net.JoinHostPort("example.com", p)) } fmt.Printf("valid 0: %v\n", net.JoinHostPort("example.com", strconv.Itoa(0))) fmt.Printf("valid -1: %v\n", net.JoinHostPort("example.com", strconv.Itoa(-1))) fmt.Printf("valid 65536: %v\n", net.JoinHostPort("example.com", strconv.Itoa(65536))) } GO ``` Repository: smell-of-curry/pokebedrock-hub Length of output: 259 --- 🏁 Script executed: ```shell #!/bin/bash set -eu printf '%s\n' '--- files ---' git ls-files pokebedrock/devserver printf '%s\n' '' printf '%s\n' '--- types.go relevant section ---' cat -n pokebedrock/devserver/types.go | sed -n '1,130p' printf '%s\n' '' printf '%s\n' '--- service.go polling section ---' cat -n pokebedrock/devserver/service.go | sed -n '100,140p' printf '%s\n' '' printf '%s\n' '--- tests for desired configs ---' cat -n pokebedrock/devserver/diff_test.go | sed -n '1,110p' printf '%s\n' '' printf '%s\n' '--- behavioral probe: net.JoinHostPort formatting ---' go run - <<'GO' package main import ( "fmt" "net" "strconv" ) func main() { ports := []string{"0", "-1", "65535", "65536", "1", "100000"} for _, p := range ports { fmt.Printf("%s -> %q\n", p, net.JoinHostPort("example.com", p)) } fmt.Printf("valid 0: %v\n", net.JoinHostPort("example.com", strconv.Itoa(0))) fmt.Printf("valid -1: %v\n", net.JoinHostPort("example.com", strconv.Itoa(-1))) fmt.Printf("valid 65536: %v\n", net.JoinHostPort("example.com", strconv.Itoa(65536))) } GO ``` Repository: smell-of-curry/pokebedrock-hub Length of output: 9759 --- 🌐 Web query: `Go net JoinHostPort host port 0 65536 formatting behavior` 💡 Result: The Go net.JoinHostPort(host, port) function does not perform validation on the host or port arguments [1][2]. It is a string manipulation utility that combines a host and a port into the "host:port" format, or "[host]:port" if the host contains colons (typically identifying it as an IPv6 address) [1][2]. Regarding your query about the 0 to 65536 range: 1. No Validation: JoinHostPort does not check if the port is a valid numeric port, nor does it enforce the 0–65536 range [1][2]. It accepts any string for the port argument and simply appends it to the host separated by a colon [1][2]. 2. Formatting Behavior: - If the host contains a colon, the function wraps the host in square brackets: [host]:port [1][2]. - If the host does not contain a colon, it returns the host directly followed by a colon and the port: host:port [1][2]. - If the port argument is empty, the function still appends the colon, resulting in "host:" or "[host]:" [3][4]. Because the function treats the port as a raw string, passing "65536" or "0" will result in those literal strings being placed after the colon without error [1][2]. Any validation of the port range must be performed by the caller before calling JoinHostPort [1]. Citations: - 1: https://pkg.go.dev/net - 2: https://github.com/golang/go/blob/e8a02230f215efb075cccd4146b3d0d1ada4870e/src/lib/net/net.go - 3: https://github.com/golang/go/issues/61093 - 4: https://github.com/golang/go/issues/25874 --- </details> **Reject invalid remote server ports before registering them.** `net.JoinHostPort` formats ports as strings without range validation, so a malformed `port` from the `/dev-servers` response can be registered as unreachable. Validate `Port` as `1..65535` so `pollOnce` keeps the current registry instead of applying an invalid update. <details> <summary>Proposed fix</summary> ```diff for _, s := range servers { if !strings.EqualFold(s.Status, "running") { continue } + if s.Port < 1 || s.Port > 65535 { + return nil, fmt.Errorf("server %q has invalid port %d", s.Name, s.Port) + } id, err := IdentifierFor(s) ``` </details>  <details> <summary>📝 Committable suggestion</summary> > ‼️ **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. ```suggestion func DesiredConfigs(servers []APIServer, host string) (map[string]srv.Config, error) { desired := make(map[string]srv.Config, len(servers)) for _, s := range servers { if !strings.EqualFold(s.Status, "running") { continue } if s.Port < 1 || s.Port > 65535 { return nil, fmt.Errorf("server %q has invalid port %d", s.Name, s.Port) } id, err := IdentifierFor(s) if err != nil { return nil, err } desired[id] = srv.Config{ Name: DisplayName(s), Identifier: id, Address: net.JoinHostPort(host, strconv.Itoa(s.Port)), BetaLock: true, ``` </details>  <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pokebedrock/devserver/types.go` around lines 72 - 86, The DesiredConfigs function must validate each running server’s Port before constructing and registering its srv.Config. Reject ports outside 1..65535 by returning an error, ensuring pollOnce retains the current registry rather than applying invalid updates; leave valid server processing unchanged. ``` </details>

smell-of-curry marked this conversation as resolved

pokebedrock/form/beta_navigator.go Outdated

					
				@ -0,0 +56,4 @@

						return

					}

					p.SendForm(NewServerConfirm(server))

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Enforce beta eligibility at use time.

Eligibility is currently only checked when constructing the lobby kit. A previously issued form or stale compass can still reach confirmation; ServerConfirm only rejects servers with BetaLock, while this navigator lists all dev identifiers.

pokebedrock/form/beta_navigator.go#L49-L59: recheck the submitter’s current ranks before sending NewServerConfirm.
pokebedrock/handler/player.go#L99-L100: reject "beta-navigator" actions when !h.CanAccessBeta().
pokebedrock/session/ranks.go#L143-L149: also refresh the guarded lobby kit after resetting ranks to UnLinked on fetch failure, removing stale gated items.

📍 Affects 3 files

pokebedrock/form/beta_navigator.go#L49-L59 (this comment)
pokebedrock/handler/player.go#L99-L100
pokebedrock/session/ranks.go#L143-L149

🤖 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 `@pokebedrock/form/beta_navigator.go` around lines 49 - 59, Enforce current
beta eligibility at every access path: in pokebedrock/form/beta_navigator.go
lines 49-59, recheck the submitter’s ranks before calling NewServerConfirm; in
pokebedrock/handler/player.go lines 99-100, reject "beta-navigator" actions when
CanAccessBeta() is false; and in pokebedrock/session/ranks.go lines 143-149,
refresh the guarded lobby kit after resetting ranks to UnLinked on fetch failure
so stale gated items are removed.

_🔒 Security & Privacy_ | _🟠 Major_ | _⚡ Quick win_ **Enforce beta eligibility at use time.** Eligibility is currently only checked when constructing the lobby kit. A previously issued form or stale compass can still reach confirmation; `ServerConfirm` only rejects servers with `BetaLock`, while this navigator lists all dev identifiers. - `pokebedrock/form/beta_navigator.go#L49-L59`: recheck the submitter’s current ranks before sending `NewServerConfirm`. - `pokebedrock/handler/player.go#L99-L100`: reject `"beta-navigator"` actions when `!h.CanAccessBeta()`. - `pokebedrock/session/ranks.go#L143-L149`: also refresh the guarded lobby kit after resetting ranks to `UnLinked` on fetch failure, removing stale gated items. <details> <summary>📍 Affects 3 files</summary> - `pokebedrock/form/beta_navigator.go#L49-L59` (this comment) - `pokebedrock/handler/player.go#L99-L100` - `pokebedrock/session/ranks.go#L143-L149` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pokebedrock/form/beta_navigator.go` around lines 49 - 59, Enforce current beta eligibility at every access path: in pokebedrock/form/beta_navigator.go lines 49-59, recheck the submitter’s ranks before calling NewServerConfirm; in pokebedrock/handler/player.go lines 99-100, reject "beta-navigator" actions when CanAccessBeta() is false; and in pokebedrock/session/ranks.go lines 143-149, refresh the guarded lobby kit after resetting ranks to UnLinked on fetch failure so stale gated items are removed. ``` </details>

smell-of-curry marked this conversation as resolved

smell-of-curry force-pushed dev-servers from 193c36337b to cc4da48c03

2026-08-08 18:35:29 +00:00

Compare

smell-of-curry commented

2026-08-08 18:36:38 +00:00

Addressed the 3 CodeRabbit findings in cc4da48 (rebased onto main):

service.go - service-level context.WithCancel; Stop cancels in-flight fetch; skip registry mutate if cancelled
types.go - reject ports outside 1..65535 (test added)
beta access - recheck on navigator submit + item use; re-apply lobby kit after rank fetch ? UnLinked

Addressed the 3 CodeRabbit findings in `cc4da48` (rebased onto `main`): 1. **service.go** - service-level `context.WithCancel`; `Stop` cancels in-flight fetch; skip registry mutate if cancelled 2. **types.go** - reject ports outside `1..65535` (test added) 3. **beta access** - recheck on navigator submit + item use; re-apply lobby kit after rank fetch ? `UnLinked`

smell-of-curry merged commit dae366d983 into main

2026-08-08 18:44:42 +00:00

smell-of-curry referenced this pull request from a commit

2026-08-08 18:44:44 +00:00

feat(hub): poll remote API for beta/dev servers (#96)

Sign in to join this conversation.

No reviewers