fix(builder): boot drafts with the settings a new server really gets #18

2026-10-26T01:14:22Z

greg commented

2026-10-26 01:14:22 +00:00

The builder's boot test filled every blank password-like setting with a throwaway value. A real deploy only fills x-generate fields, so a draft whose server shuts down with an empty join password passed the builder and CI, and would have failed on its first real deploy. That's what the current draft in #17 does: booted with its own defaults, the server exits after 88 s.

New boottest-payload -as-deployed: only x-generate fields are filled; everything else is exactly the template's defaults.
build boots drafts that way. A server that won't run with its own defaults fails the build, and the model is told empty settings stay empty, so the value must come from a default or x-generate.
CI's boot test is unchanged (existing templates).

🤖 Generated with Claude Code

The builder's boot test filled every blank password-like setting with a throwaway value. A real deploy only fills `x-generate` fields, so a draft whose server shuts down with an empty join password passed the builder and CI, and would have failed on its first real deploy. That's what the current draft in #17 does: booted with its own defaults, the server exits after 88 s. - New `boottest-payload -as-deployed`: only `x-generate` fields are filled; everything else is exactly the template's defaults. - `build` boots drafts that way. A server that won't run with its own defaults fails the build, and the model is told empty settings stay empty, so the value must come from a default or `x-generate`. - CI's boot test is unchanged (existing templates). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

👀 1

greg added 1 commit 2026-10-26 01:14:22 +00:00

fix(builder): boot drafts with the settings a new server really gets

Validate game templates / validate (push) Successful in 31s

Details

Validate game templates / boot-test (push) Skipped

Details

Validate game templates / publish (push) Skipped

Details

72a4280d44

The builder's boot test filled every blank password-like setting with a
throwaway value. A real deploy only fills x-generate fields, so a draft whose
server refuses to run with an empty join password passed the builder and CI,
and then shut down on its first real deploy.

The builder now boots drafts with boottest-payload -as-deployed: only
x-generate fields are filled, everything else exactly as the template's
defaults. A server that won't run with its own defaults fails the build, and
the model is told that empty settings stay empty, so the value must come from
a default or x-generate. CI's boot test is unchanged.

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

git_ai_bot reviewed

2026-10-26 01:15:27 +00:00

git_ai_bot left a comment

🤖 AI Code Review

1. Blocking issues

None. The core logic change is correct and well-scoped.

Tracing the new condition:

if prop["x-generate"] == nil && (*asDeployed || !passwordKey.MatchString(key)) {
    continue
}

x-generate present → always falls through (gets filled) — correct.
x-generate nil, non-password key → always skips — correct (unchanged).
x-generate nil, password key, not -as-deployed → fills (unchanged CI behaviour).
x-generate nil, password key, -as-deployed → skips (new, intended).

The -as-deployed flag defaults to false, so existing CI callers in scripts/boot-test.sh are unaffected.

2. Non-blocking suggestions

Misleading hint in the missing-secrets path (build.go ~line 256).

return []string{"the boot test can't run: " + strings.TrimSpace(stderr.String()) + ". Remove 'secrets' from " +
    "test.yaml; boot tests fill password settings with a throwaway value"}, nil, nil

With -as-deployed active (which is always the case when this is reached from bootCheck), the statement "boot tests fill password settings with a throwaway value" is no longer accurate — only x-generate fields are filled. The actionable advice (remove secrets, use a default or x-generate instead) is still correct, but the explanation could mislead the model into thinking a plain ServerPassword will be auto-filled. Consider rewording to something like:

"test.yaml; a new deploy only auto-fills x-generate fields, so the value must come from a default or x-generate"

This aligns the hint with the bootFailure message that was already updated in the same PR.

AdminPassword preservation not asserted in the -as-deployed block.

The existing test verifies AdminPassword is preserved in the non-as-deployed path (it's kept-by-owner). The -as-deployed block only checks RCON_PASSWORD (filled) and ServerPassword (empty). A one-line assertion that AdminPassword is still "kept-by-owner" in the as-deployed output would guard against a future regression where non-empty values are accidentally overwritten.

3. Tests

The added assertions cover the two key scenarios:

x-generate field is still filled → RCON_PASSWORD gets a boot-* value. ✓
Non-x-generate password field stays empty → ServerPassword is "". ✓

Suggested additions (low priority):

Assert AdminPassword remains "kept-by-owner" in the -as-deployed output (see above).
A case where a password-like key has no entry in configSchema.properties at all — verify it's not filled in either mode. (This guards against the schemaProperties map accidentally including keys that shouldn't be touched.)

4. Overall assessment

Clean, minimal, well-motivated fix. The diff is small, the logic is correct, the failure message to the model is updated to explain the new contract, and the test pins the expected behaviour. The one small wording inconsistency in the missing-secrets hint is the only thing I'd tighten before merge, but it's not a correctness risk. Good to go.

Automated review by AI Git Bot

## 🤖 AI Code Review ## 1. Blocking issues None. The core logic change is correct and well-scoped. Tracing the new condition: ```go if prop["x-generate"] == nil && (*asDeployed || !passwordKey.MatchString(key)) { continue } ``` - `x-generate` present → always falls through (gets filled) — correct. - `x-generate` nil, non-password key → always skips — correct (unchanged). - `x-generate` nil, password key, **not** `-as-deployed` → fills (unchanged CI behaviour). - `x-generate` nil, password key, **`-as-deployed`** → skips (new, intended). The `-as-deployed` flag defaults to `false`, so existing CI callers in `scripts/boot-test.sh` are unaffected. ## 2. Non-blocking suggestions **Misleading hint in the missing-secrets path (`build.go` ~line 256).** ```go return []string{"the boot test can't run: " + strings.TrimSpace(stderr.String()) + ". Remove 'secrets' from " + "test.yaml; boot tests fill password settings with a throwaway value"}, nil, nil ``` With `-as-deployed` active (which is always the case when this is reached from `bootCheck`), the statement "boot tests fill password settings with a throwaway value" is no longer accurate — only `x-generate` fields are filled. The actionable advice (remove `secrets`, use a default or `x-generate` instead) is still correct, but the explanation could mislead the model into thinking a plain `ServerPassword` will be auto-filled. Consider rewording to something like: ```go "test.yaml; a new deploy only auto-fills x-generate fields, so the value must come from a default or x-generate" ``` This aligns the hint with the `bootFailure` message that was already updated in the same PR. **`AdminPassword` preservation not asserted in the `-as-deployed` block.** The existing test verifies `AdminPassword` is preserved in the non-as-deployed path (it's `kept-by-owner`). The `-as-deployed` block only checks `RCON_PASSWORD` (filled) and `ServerPassword` (empty). A one-line assertion that `AdminPassword` is still `"kept-by-owner"` in the as-deployed output would guard against a future regression where non-empty values are accidentally overwritten. ## 3. Tests The added assertions cover the two key scenarios: - `x-generate` field is still filled → `RCON_PASSWORD` gets a `boot-*` value. ✓ - Non-`x-generate` password field stays empty → `ServerPassword` is `""`. ✓ Suggested additions (low priority): - Assert `AdminPassword` remains `"kept-by-owner"` in the `-as-deployed` output (see above). - A case where a password-like key has **no** entry in `configSchema.properties` at all — verify it's not filled in either mode. (This guards against the `schemaProperties` map accidentally including keys that shouldn't be touched.) ## 4. Overall assessment Clean, minimal, well-motivated fix. The diff is small, the logic is correct, the failure message to the model is updated to explain the new contract, and the test pins the expected behaviour. The one small wording inconsistency in the missing-secrets hint is the only thing I'd tighten before merge, but it's not a correctness risk. Good to go. --- *Automated review by AI Git Bot*

greg merged commit cc4a8e357d into main

2026-10-26 01:31:14 +00:00

greg deleted branch fix/builder-boot-as-deployed

2026-10-26 01:31:14 +00:00

greg referenced this pull request

2026-10-26 01:57:22 +00:00

fix(boot-test): exactly what a new server gets, and pinpoint settings it can't start without #20

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: gbreed-public/gbr-games#18