fix(ark-crossplay): game port is not a setting #7

Merged

greg merged 2 commits from fix/port-ark-survival-ascended-crossplay into main 2026-10-26 00:26:18 +00:00

Conversation 2 Commits 2 Files Changed 1

+1 -8

greg commented 2026-10-25 19:50:35 +00:00

Owner

Copy link

Copy source

Nodes always publish container port 7777/udp and only move the host side when servers share a node, so an owner who changed Port got a server nobody could reach.

• Port is removed from defaultConfig and configSchema.
• The startup command passes ?Port=7777 directly instead of ${PORT:-7777}.

Found by the portSettings draft check. Servers that already stored Port keep it in their config; it is no longer read.

🤖 Generated with Claude Code

Nodes always publish container port `7777/udp` and only move the host side when servers share a node, so an owner who changed `Port` got a server nobody could reach. - `Port` is removed from `defaultConfig` and `configSchema`. - The startup command passes `?Port=7777` directly instead of `${PORT:-7777}`. Found by the `portSettings` draft check. Servers that already stored `Port` keep it in their config; it is no longer read. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

👀 1

greg added 1 commit 2026-10-25 19:50:35 +00:00

fix(ark-crossplay): game port is not a setting ... 1951b7e7e9

Nodes always publish container port 7777, so an owner-set Port made the server unreachable. The startup command now passes Port=7777 directly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

git_ai_bot reviewed 2026-10-25 19:51:43 +00:00

git_ai_bot left a comment

Member

Copy link

Copy source

🤖 AI Code Review

1. Blocking issues

None identified. The change is logically sound and self-consistent.

2. Non-blocking suggestions

Verify platform-side handling of the stale Port config key.
The PR description notes that existing servers will retain Port in their saved config and it will simply go unread. This is fine for the startup command (which no longer references $PORT), but it would be worth confirming that:

• The platform's config-to-env-var mapping layer (the thing that previously translated Port → PORT env var) doesn't error or log a warning when the key is absent from the schema. If it iterates over configSchema.properties to build env vars, the removal is clean. If it iterates over the stored config and tries to look up each key in the schema, a missing entry could produce a log warning or, in stricter implementations, a validation failure.
• Any UI that renders the config form will simply no longer show the field (expected), but confirm there's no hard-coded reference to the Port key elsewhere in the platform or dashboard code.

This is unlikely to be a real problem, but a 5-minute grep for Port / portSettings in the platform codebase would close the loop.

Minor: RCONPort remains env-var-driven while game port is now fixed.
This asymmetry is intentional and correct (RCON port genuinely needs to differ per-instance in shared-node setups), but a short comment in the schema description or a code comment near the startup command explaining why Port is hardcoded while RCONPort is not could save a future maintainer from "fixing" it back to a variable.

3. Tests

The PR description mentions the change was "found by the portSettings draft check," which is good. A couple of items to confirm:

• Ensure the portSettings draft check (or equivalent CI validation) actually asserts that any key in defaultPorts is not present in configSchema.properties. That would prevent regression if someone re-adds Port as a setting.
• A basic smoke test that the container starts and the game process listens on 7777/udp internally would be ideal, though if the existing deployment pipeline already covers "server comes up and is reachable," that's sufficient for this scope.

4. Overall assessment

Clean, minimal fix that correctly models the container networking: the game process always binds 7777 internally, and host-side port selection is a platform/infra concern, not a game-setting concern. Removing the misleading user-facing Port knob eliminates a class of "server unreachable after I changed the port" support tickets. The diff is tight, the reasoning is sound, and backward compatibility for existing stored configs is handled gracefully. LGTM.

---

Automated review by AI Git Bot

## 🤖 AI Code Review ## 1. Blocking issues None identified. The change is logically sound and self-consistent. ## 2. Non-blocking suggestions **Verify platform-side handling of the stale `Port` config key.** The PR description notes that existing servers will retain `Port` in their saved config and it will simply go unread. This is fine for the startup command (which no longer references `$PORT`), but it would be worth confirming that: - The platform's config-to-env-var mapping layer (the thing that previously translated `Port` → `PORT` env var) doesn't error or log a warning when the key is absent from the schema. If it iterates over `configSchema.properties` to build env vars, the removal is clean. If it iterates over the *stored* config and tries to look up each key in the schema, a missing entry could produce a log warning or, in stricter implementations, a validation failure. - Any UI that renders the config form will simply no longer show the field (expected), but confirm there's no hard-coded reference to the `Port` key elsewhere in the platform or dashboard code. This is unlikely to be a real problem, but a 5-minute grep for `Port` / `portSettings` in the platform codebase would close the loop. **Minor: `RCONPort` remains env-var-driven while game port is now fixed.** This asymmetry is intentional and correct (RCON port genuinely needs to differ per-instance in shared-node setups), but a short comment in the schema description or a code comment near the startup command explaining *why* `Port` is hardcoded while `RCONPort` is not could save a future maintainer from "fixing" it back to a variable. ## 3. Tests The PR description mentions the change was "found by the `portSettings` draft check," which is good. A couple of items to confirm: - Ensure the `portSettings` draft check (or equivalent CI validation) actually asserts that any key in `defaultPorts` is **not** present in `configSchema.properties`. That would prevent regression if someone re-adds `Port` as a setting. - A basic smoke test that the container starts and the game process listens on `7777/udp` internally would be ideal, though if the existing deployment pipeline already covers "server comes up and is reachable," that's sufficient for this scope. ## 4. Overall assessment Clean, minimal fix that correctly models the container networking: the game process always binds 7777 internally, and host-side port selection is a platform/infra concern, not a game-setting concern. Removing the misleading user-facing `Port` knob eliminates a class of "server unreachable after I changed the port" support tickets. The diff is tight, the reasoning is sound, and backward compatibility for existing stored configs is handled gracefully. **LGTM.** --- *Automated review by AI Git Bot*

greg added 1 commit 2026-10-26 00:19:41 +00:00

Merge branch 'main' into fix/port-ark-survival-ascended-crossplay edc755fdb8

git_ai_bot reviewed 2026-10-26 00:20:22 +00:00

git_ai_bot left a comment

Member

Copy link

Copy source

🤖 AI Code Review

1. Blocking issues

None. The diff is clean and the logic is correct.

2. Non-blocking suggestions

Same as my initial review — two minor points worth a quick check, nothing that blocks merge:

• Grep the platform codebase for PORT / Port references specific to this game ID. The startup command no longer reads $PORT, but if the platform's env-var injection layer unconditionally sets PORT=<user-configured-value> for every key in defaultConfig, the stale value will simply be an unused env var. Harmless, but worth confirming there's no downstream code path (e.g., a health-check script, a firewall rule generator, or a server-browser announcement) that reads PORT from the environment to decide which host port to advertise. If the platform derives the advertised port purely from defaultPorts + its own host-side allocation, you're fine.

• Asymmetry note. Port is now a fixed constant while RCONPort remains configurable. This is the right call (RCON host port genuinely varies in shared-node setups; game port never does inside the container), but a one-line comment in the RCONPort schema description — e.g. "Unlike the game port (always 7777 internally), the RCON port may be remapped by the platform in shared-node deployments" — would prevent a future contributor from "symmetrizing" it back to a fixed value or, conversely, trying to make Port configurable again.

3. Tests

• Confirm the portSettings draft check (or equivalent CI validator) asserts that every entry in defaultPorts is absent from configSchema.properties. This would have caught the original bug and prevents regression.
• A smoke test verifying the container exposes 7777/udp and that ASA_START_PARAMS contains Port=7777 (not Port=8888 or similar) after a deploy with a previously-custom Port value in the stored config would give end-to-end confidence. If the existing deploy pipeline already covers "server is reachable," this is optional.

4. Overall assessment

The change is minimal, correct, and well-motivated. Hardcoding the game port to 7777 in the container-internal startup command while removing the misleading user-facing setting is the right fix. The RCONPort asymmetry is justified. Existing stored configs that contain a Port key will simply be ignored by the startup command — no data migration needed. LGTM.

---

Automated review by AI Git Bot

## 🤖 AI Code Review ## 1. Blocking issues None. The diff is clean and the logic is correct. ## 2. Non-blocking suggestions Same as my initial review — two minor points worth a quick check, nothing that blocks merge: - **Grep the platform codebase for `PORT` / `Port` references specific to this game ID.** The startup command no longer reads `$PORT`, but if the platform's env-var injection layer unconditionally sets `PORT=<user-configured-value>` for every key in `defaultConfig`, the stale value will simply be an unused env var. Harmless, but worth confirming there's no downstream code path (e.g., a health-check script, a firewall rule generator, or a server-browser announcement) that reads `PORT` from the environment to decide which host port to advertise. If the platform derives the advertised port purely from `defaultPorts` + its own host-side allocation, you're fine. - **Asymmetry note.** `Port` is now a fixed constant while `RCONPort` remains configurable. This is the right call (RCON host port genuinely varies in shared-node setups; game port never does inside the container), but a one-line comment in the `RCONPort` schema description — e.g. *"Unlike the game port (always 7777 internally), the RCON port may be remapped by the platform in shared-node deployments"* — would prevent a future contributor from "symmetrizing" it back to a fixed value or, conversely, trying to make `Port` configurable again. ## 3. Tests - Confirm the `portSettings` draft check (or equivalent CI validator) asserts that every entry in `defaultPorts` is **absent** from `configSchema.properties`. This would have caught the original bug and prevents regression. - A smoke test verifying the container exposes `7777/udp` and that `ASA_START_PARAMS` contains `Port=7777` (not `Port=8888` or similar) after a deploy with a previously-custom `Port` value in the stored config would give end-to-end confidence. If the existing deploy pipeline already covers "server is reachable," this is optional. ## 4. Overall assessment The change is minimal, correct, and well-motivated. Hardcoding the game port to 7777 in the container-internal startup command while removing the misleading user-facing setting is the right fix. The `RCONPort` asymmetry is justified. Existing stored configs that contain a `Port` key will simply be ignored by the startup command — no data migration needed. **LGTM.** --- *Automated review by AI Git Bot*

greg merged commit 2d63e8eb6d into main 2026-10-26 00:26:18 +00:00

greg deleted branch fix/port-ark-survival-ascended-crossplay 2026-10-26 00:26:18 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

git_ai_bot

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

gbr-bot git_ai_bot git_pipeline greg (Gregory Reed) openhands

No Assignees

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: gbreed-public/gbr-games#7