diff --git a/AUTHORING.md b/AUTHORING.md index e21e185..9f7bea0 100644 --- a/AUTHORING.md +++ b/AUTHORING.md @@ -54,6 +54,8 @@ Know this before writing anything: - Read settings under the names the agent gives them (see above). `${RCON_PORT}` when the setting is `RCONPort` silently ignores the owner's value; it arrives as `RCONPORT`. - Use `${VAR:-default}` for anything that may be unset. +- Only use launch options the game's documentation lists, spelled as it spells them. Servers ignore options + they don't know, so an invented one (`-maxplayers` for Valheim) boots fine and its setting does nothing. - Quote text settings where they're used: `-name "${SERVER_NAME}"`. `ARGS="-name ${SERVER_NAME}"; ./server $ARGS` splits "GBRHosting - My Server" into words; build optional arguments in a bash array instead (`args=(-name "$SERVER_NAME"); [ -n "$PASSWORD" ] && args+=(-password "$PASSWORD"); ./server "${args[@]}"`). diff --git a/cmd/gbrgames/build.go b/cmd/gbrgames/build.go index 2bfcf4e..1d4a96b 100644 --- a/cmd/gbrgames/build.go +++ b/cmd/gbrgames/build.go @@ -138,7 +138,8 @@ func build(o buildOptions) (*buildResult, error) { steamApp = o.variant.SteamAppID } d, err := newDrafter(draftJob{GamesDir: o.gamesDir, SchemaDir: o.schemaDir, Name: o.name, ID: o.id, Image: res.Image, - SteamApp: steamApp, Notes: variantNotes(o.variant, o.notes), Report: report, Provider: o.provider, Model: o.model, NoPull: true}) + SteamApp: steamApp, Notes: variantNotes(o.variant, o.notes), Report: report, Provider: o.provider, Model: o.model, NoPull: true, + Docs: sourceText(o.variant.Sources)}) if err != nil { return nil, err } @@ -370,3 +371,27 @@ func buildSummary(o buildOptions, res *buildResult) string { } return b.String() } + +// sourceText reads the pages research based the variant on, in full, so the drafter can check that every +// launch option the model uses is documented there. Empty (check skipped) without a reader. +func sourceText(sources []string) string { + reader := os.Getenv("READER_URL") + if reader == "" || len(sources) == 0 { + fmt.Println("[build] no research sources to check launch options against (READER_URL or sources missing)") + return "" + } + var b strings.Builder + read := 0 + for _, s := range sources { + text, err := readPageLimit(reader, s, 200_000) + if err != nil { + fmt.Printf("[build] could not read %s: %v\n", s, err) + continue + } + read++ + b.WriteString(text) + b.WriteString("\n") + } + fmt.Printf("[build] read %d of %d research sources to check launch options against\n", read, len(sources)) + return b.String() +} diff --git a/cmd/gbrgames/draft.go b/cmd/gbrgames/draft.go index dc62d25..0708e0a 100644 --- a/cmd/gbrgames/draft.go +++ b/cmd/gbrgames/draft.go @@ -82,6 +82,9 @@ type draftJob struct { Report *bootReport Provider, Model string NoPull bool + // Text of the pages research based the variant on; when set, every launch option in the startup + // command must appear in it (or in the image's scripts) + Docs string } // drafter holds one conversation with the model about one game, so later problems (a failed boot) can @@ -200,6 +203,7 @@ func (d *drafter) check(answer string) []string { problems = append(problems, savesOutsideVolumes(g, d.job.Report)...) problems = append(problems, splitSettings(g)...) problems = append(problems, portSettings(g)...) + problems = append(problems, undocumentedOptions(g, d.job.Docs, d.facts)...) } } return problems diff --git a/cmd/gbrgames/draftchecks.go b/cmd/gbrgames/draftchecks.go index 5db39a0..61378f8 100644 --- a/cmd/gbrgames/draftchecks.go +++ b/cmd/gbrgames/draftchecks.go @@ -3,6 +3,7 @@ package main import ( "encoding/json" "fmt" + "path" "regexp" "sort" "strconv" @@ -248,3 +249,77 @@ func portNumber(v any) (int, bool) { p, err := strconv.Atoi(strings.TrimSpace(s)) return p, err == nil && p > 0 && p < 65536 } + +var ( + optionToken = regexp.MustCompile(`(?:^|[\s("'])(-{1,2}[A-Za-z][A-Za-z0-9_.]*(?:-[A-Za-z0-9_.]+)*)`) + segmentSep = regexp.MustCompile(`;|&&|\|\||\||\n`) + testOps = map[string]bool{"eq": true, "ne": true, "lt": true, "gt": true, "le": true, "ge": true, "nt": true, "ot": true, "ef": true} + // Options of these are the shell's and its tools', not the game's + shellTools = map[string]bool{"mkdir": true, "cp": true, "mv": true, "ln": true, "rm": true, "chmod": true, "chown": true, + "wget": true, "curl": true, "tar": true, "unzip": true, "test": true, "[": true, "[[": true, "echo": true, "printf": true, + "sleep": true, "find": true, "sed": true, "grep": true, "cd": true, "export": true, "touch": true, "bash": true, "sh": true, + "cat": true, "head": true, "tail": true, "ls": true, "set": true, "ulimit": true, "env": true, "id": true} +) + +// undocumentedOptions finds launch options in the startup command that none of the researched pages (nor +// the image's own scripts) mention. Servers silently ignore options they don't know, so a made-up one +// (Valheim has no -maxplayers) boots fine and passes every boot test while its setting does nothing. +func undocumentedOptions(game map[string]any, docs string, facts *imageFacts) []string { + if strings.TrimSpace(docs) == "" { + return nil + } + startup, _ := game["defaultStartupCommand"].(string) + corpus := docs + if facts != nil { + for _, s := range facts.Scripts { + corpus += "\n" + s + } + } + seen := map[string]bool{} + var missing []string + for _, opt := range launchOptions(unwrapShell(startup)) { + name := strings.TrimLeft(opt, "-") + key := strings.ToLower(name) + if seen[key] { + continue + } + seen[key] = true + documented := regexp.MustCompile(`(?i)(^|[^A-Za-z0-9_])-{1,2}` + regexp.QuoteMeta(name) + `($|[^A-Za-z0-9_])`) + if !documented.MatchString(corpus) { + missing = append(missing, opt) + } + } + if len(missing) == 0 { + return nil + } + return []string{fmt.Sprintf("the startup command passes %s, which none of the pages researched for this game mention. "+ + "Servers silently ignore options they don't know, so it boots fine but does nothing: remove it (and any setting that "+ + "only feeds it), or spell it exactly as the documentation does", strings.Join(missing, ", "))} +} + +// launchOptions lists the -options passed to the game, skipping the shell's own (mkdir -p, [ -n ... ]). +func launchOptions(cmd string) []string { + var out []string + for _, seg := range segmentSep.Split(cmd, -1) { + fields := strings.Fields(strings.TrimLeft(strings.TrimSpace(seg), "({")) + i := 0 + for i < len(fields) && (fields[i] == "exec" || fields[i] == "then" || fields[i] == "do" || fields[i] == "else" || + (assignWord.MatchString(fields[i]) && !strings.Contains(fields[i], "=("))) { + i++ + } + if i == len(fields) { + continue + } + if word := path.Base(fields[i]); shellTools[word] { + continue + } + for _, m := range optionToken.FindAllStringSubmatch(strings.Join(fields[i:], " "), -1) { + name := strings.TrimLeft(m[1], "-") + if len(name) < 2 || testOps[name] { + continue + } + out = append(out, m[1]) + } + } + return out +} diff --git a/cmd/gbrgames/draftchecks_test.go b/cmd/gbrgames/draftchecks_test.go index 148ea9f..83aedca 100644 --- a/cmd/gbrgames/draftchecks_test.go +++ b/cmd/gbrgames/draftchecks_test.go @@ -105,3 +105,47 @@ func TestPortSettings(t *testing.T) { t.Errorf("got %v", p) } } + +func TestUndocumentedOptionsCatchesMaxPlayers(t *testing.T) { + // PR #13's startup: every option is in Valheim's docs except -maxplayers + game := map[string]any{"defaultStartupCommand": `args=(-name "${SERVER_NAME:-GBRHosting - Valheim}" -port 2456 -world "${WORLD_NAME:-Dedicated}" -savedir /server); ` + + `[ -n "${SERVER_PASSWORD}" ] && args+=(-password "${SERVER_PASSWORD}"); [ "${CROSSPLAY}" = "true" ] && args+=(-crossplay); ` + + `[ -n "${MAX_PLAYERS}" ] && args+=(-maxplayers "${MAX_PLAYERS}"); ./valheim_server.x86_64 "${args[@]}"`} + docs := "Launch with `-name`, `-port`, `-world`, `-password`, `-savedir ` and `-crossplay` to allow consoles." + got := undocumentedOptions(game, docs, nil) + if len(got) != 1 || !strings.Contains(got[0], "passes -maxplayers,") { + t.Fatalf("want only -maxplayers flagged, got %v", got) + } + // Once removed, nothing is left to flag + game["defaultStartupCommand"] = strings.Replace(game["defaultStartupCommand"].(string), `[ -n "${MAX_PLAYERS}" ] && args+=(-maxplayers "${MAX_PLAYERS}"); `, "", 1) + if got := undocumentedOptions(game, docs, nil); len(got) != 0 { + t.Errorf("got %v", got) + } +} + +func TestUndocumentedOptionsIgnoresShellAndUsesImageScripts(t *testing.T) { + game := map[string]any{"defaultStartupCommand": `mkdir -p /server/.config; cp -f /a /b; [ -n "$X" ] && [ "$Y" -eq 1 ] && ` + + `exec ./server --listen-port=7777 -ServerPlatform=ALL -logs`} + facts := &imageFacts{Scripts: map[string]string{"/start.sh": `exec "$@" -logs`}} + got := strings.Join(undocumentedOptions(game, "Use --listen-port and -ServerPlatform=ALL for crossplay.", facts), "\n") + if got != "" { + t.Errorf("shell options, documented ones and ones the image's script uses must pass, got %q", got) + } + // Without sources the check is off (draft run by hand, or no reader configured) + game["defaultStartupCommand"] = "./server -madeup" + if p := undocumentedOptions(game, "", nil); p != nil { + t.Errorf("got %v", p) + } + if p := undocumentedOptions(game, "unrelated page", nil); len(p) != 1 { + t.Errorf("want -madeup flagged, got %v", p) + } +} + +func TestUndocumentedOptionsWholeWords(t *testing.T) { + // "-port" in the docs doesn't document "-portal", and "-port" isn't documented by "-portal" + game := map[string]any{"defaultStartupCommand": "./server -port 1 -portal x"} + got := strings.Join(undocumentedOptions(game, "options: -port", nil), "") + if !strings.Contains(got, "passes -portal,") || strings.Contains(got, "-port,") { + t.Errorf("got %q", got) + } +} diff --git a/cmd/gbrgames/research.go b/cmd/gbrgames/research.go index 167fa7a..1ddfa1a 100644 --- a/cmd/gbrgames/research.go +++ b/cmd/gbrgames/research.go @@ -250,13 +250,18 @@ func dedupeHits(hits []searchHit) []searchHit { } func readPage(reader, pageURL string) (string, error) { + return readPageLimit(reader, pageURL, 9000) +} + +// readPageLimit reads a page as text through the reader, cut to limit bytes. +func readPageLimit(reader, pageURL string, limit int) (string, error) { raw, err := httpGet(strings.TrimRight(reader, "/")+"/"+pageURL, 60*time.Second) if err != nil { return "", err } text := string(raw) - if len(text) > 9000 { - text = text[:9000] + "\n...(truncated)" + if len(text) > limit { + text = text[:limit] + "\n...(truncated)" } return text, nil }