fix(satisfactory): game and messaging ports are not settings #10

Closed
greg wants to merge 1 commits from fix/port-satisfactory-vanilla-stable into main
pull from: fix/port-satisfactory-vanilla-stable
Owner

Nodes always publish container ports 7777/tcp, 7777/udp and 8888/tcp and only move the host side when servers share a node, so an owner who changed SERVERGAMEPORT or SERVERMESSAGINGPORT got a server nobody could reach.

  • Both are removed from defaultConfig and configSchema; wolveix/satisfactory-server already defaults them to 7777 and 8888.

Found by the portSettings draft check.

🤖 Generated with Claude Code

Nodes always publish container ports `7777/tcp`, `7777/udp` and `8888/tcp` and only move the host side when servers share a node, so an owner who changed `SERVERGAMEPORT` or `SERVERMESSAGINGPORT` got a server nobody could reach. - Both are removed from `defaultConfig` and `configSchema`; wolveix/satisfactory-server already defaults them to 7777 and 8888. Found by the `portSettings` draft check. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
greg added 1 commit 2026-10-25 19:50:37 +00:00
fix(satisfactory): game and messaging ports are not settings
Validate game templates / validate (push) Successful in 29s
Validate game templates / boot-test (push) Failing after 46s
Validate game templates / publish (push) Skipped
5f45dff24f
Nodes always publish container ports 7777 and 8888, so owner-set SERVERGAMEPORT or SERVERMESSAGINGPORT made the server unreachable. The image already defaults to 7777 and 8888.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
git_ai_bot left a comment
Member

🤖 AI Code Review

Review

1. Blocking issues

None. The change is logically sound: defaultPorts correctly pins the container-side ports (7777/tcp, 7777/udp, 8888/tcp) that the wolveix/satisfactory-server image binds to, and exposing SERVERGAMEPORT/SERVERMESSAGINGPORT as user-editable settings was misleading because changing them only altered the in-container bind address without updating the Docker port mapping — breaking connectivity. Removing them from defaultConfig and configSchema is the right fix.

2. Non-blocking suggestions

  • Stale values in existing config files. For users who previously saved a non-default SERVERGAMEPORT or SERVERMESSAGINGPORT, those key=value lines may persist in their generated .ini file after the schema no longer lists the keys. Depending on how the config-write path works (full regeneration vs. merge), a stale SERVERGAMEPORT=9999 line could still be emitted into the container's environment or config file, leaving the server broken in exactly the way the PR aims to fix. Worth verifying that the write path drops keys absent from the schema, or consider a one-time migration/scrub that strips these two keys from existing config blobs.

  • defaultPorts comment (optional). Since the entire point of this fix is that these ports are fixed by the image and not user-tunable, a brief YAML comment on defaultPorts (e.g. # Fixed by the image; not user-configurable) could help future maintainers avoid re-adding them as settings.

3. Tests

  • The PR description mentions the portSettings draft check that surfaced this. Confirm that check now passes for satisfactory-vanilla-stable and that it would have caught the pre-fix state (i.e., the test is in the repo or CI). The test.yaml file isn't modified in this diff; verify it doesn't assert on the presence of SERVERGAMEPORT/SERVERMESSAGINGPORT in configSchema.properties.

4. Overall assessment

Correct, minimal fix. The two ports belong to the container's fixed interface, not to user-tunable game settings. No logic, security, or API concerns. The only thing I'd want to confirm before merge is the stale-config-file question above — if the write path regenerates the file from the current schema each time, you're fine; if it merges, a scrub step is needed.


Automated review by AI Git Bot

## 🤖 AI Code Review ## Review ### 1. Blocking issues None. The change is logically sound: `defaultPorts` correctly pins the container-side ports (`7777/tcp`, `7777/udp`, `8888/tcp`) that the `wolveix/satisfactory-server` image binds to, and exposing `SERVERGAMEPORT`/`SERVERMESSAGINGPORT` as user-editable settings was misleading because changing them only altered the in-container bind address without updating the Docker port mapping — breaking connectivity. Removing them from `defaultConfig` and `configSchema` is the right fix. ### 2. Non-blocking suggestions - **Stale values in existing config files.** For users who previously saved a non-default `SERVERGAMEPORT` or `SERVERMESSAGINGPORT`, those key=value lines may persist in their generated `.ini` file after the schema no longer lists the keys. Depending on how the config-write path works (full regeneration vs. merge), a stale `SERVERGAMEPORT=9999` line could still be emitted into the container's environment or config file, leaving the server broken in exactly the way the PR aims to fix. Worth verifying that the write path drops keys absent from the schema, or consider a one-time migration/scrub that strips these two keys from existing config blobs. - **`defaultPorts` comment (optional).** Since the entire point of this fix is that these ports are fixed by the image and not user-tunable, a brief YAML comment on `defaultPorts` (e.g. `# Fixed by the image; not user-configurable`) could help future maintainers avoid re-adding them as settings. ### 3. Tests - The PR description mentions the `portSettings` draft check that surfaced this. Confirm that check now passes for `satisfactory-vanilla-stable` and that it would have caught the pre-fix state (i.e., the test is in the repo or CI). The `test.yaml` file isn't modified in this diff; verify it doesn't assert on the presence of `SERVERGAMEPORT`/`SERVERMESSAGINGPORT` in `configSchema.properties`. ### 4. Overall assessment Correct, minimal fix. The two ports belong to the container's fixed interface, not to user-tunable game settings. No logic, security, or API concerns. The only thing I'd want to confirm before merge is the stale-config-file question above — if the write path regenerates the file from the current schema each time, you're fine; if it merges, a scrub step is needed. --- *Automated review by AI Git Bot*
Author
Owner

Boot-test failed twice (first run and a re-run) with SteamCMD's ERROR! Failed to install app '1690800' (Missing configuration) before any game files download.

This is not caused by this change: running wolveix/satisfactory-server:latest locally fails the same way with the old SERVERGAMEPORT=7777/SERVERMESSAGINGPORT=8888 env, and also when run as root the way the image's own docs show. SteamCMD can't install app 1690800 anonymously right now, so main's template is broken too. Re-run once that clears, or fix the install separately.

🤖 Generated with Claude Code

Boot-test failed twice (first run and a re-run) with SteamCMD's `ERROR! Failed to install app '1690800' (Missing configuration)` before any game files download. This is not caused by this change: running `wolveix/satisfactory-server:latest` locally fails the same way with the old `SERVERGAMEPORT=7777`/`SERVERMESSAGINGPORT=8888` env, and also when run as root the way the image's own docs show. SteamCMD can't install app 1690800 anonymously right now, so main's template is broken too. Re-run once that clears, or fix the install separately. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
greg closed this pull request 2026-10-26 00:18:16 +00:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
No Reviewers
No labels
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: gbreed-public/gbr-games#10