chooseSeed silently ignores an unparseable SEED instead of failing loudly #25

New Issue

2026-08-09T08:10:34+02:00

clawbot commented

2026-08-09 08:10:34 +02:00

Problem

chooseSeed in cmd/rogue/main.go falls back to the time+pid default when
SEED is set but cannot be parsed. The user asked for a specific dungeon and
silently got a random one.

Set-but-invalid configuration should fail loudly. Silently substituting a
different value is the worst of the three options (honour it, reject it, or
quietly ignore it) because nothing distinguishes "I typo'd SEED" from "the
seed worked".

This matters more here than in a typical program: SEED exists precisely so a
dungeon can be reproduced — for bug reports, for the seed-compat work, and
for wizard-mode debugging. A silently-ignored seed produces an unreproducible
session while the user believes it is pinned, which can send someone chasing a
"non-deterministic" bug that is nothing of the kind.

Found by the reworker of PR #23 (recorded there as M5) and correctly left
alone as out of scope.

Definition of done

SEED set and unparseable → the program exits non-zero with a message that
names SEED and shows the offending value. It must not start a game.
SEED unset → unchanged behaviour (time+pid default).
SEED set and valid → unchanged behaviour.
Decide and document, in a code comment, what counts as valid: is an empty
SEED= an error or the same as unset? Is a negative value accepted? State
the choice rather than letting it fall out of strconv.
Tests cover all three cases in item 1-3 plus whatever item 4 settles.
The error goes to stderr, and the terminal is left usable — note the
check should happen before raw mode is raised, or after Fini; do not
introduce a path that prints an error into a raw terminal.
make check fully green.
TODO.md updated in the same commit — Completed Steps entry, and do not
rotate "Next Step".
Commit title ends with (closes #N).

Implementation requirements

Check what C does with a bad seed first (git show origin/c-master:main.c,
and the ROGUEOPTS/seed handling). If C was permissive, this is a
deliberate divergence and must be commented as such — but "fail loudly on
invalid config" is the right call regardless, since C had no SEED
environment variable to be faithful to in the first place. Confirm that.
Do NOT change the signal-handling or pendingSaver code settled in PR #23.
Do NOT regenerate goldens under game/testdata/.
Leave c-master and modern-rogue alone.
Lint environment warning: golangci-lint on this host shares one cache and
lock across ~18 concurrent sessions. Run make lint/make check in a retry
loop and only accept a run that neither reports
parallel golangci-lint is running nor mentions paths outside your own
worktree. Otherwise you may report green on a red branch.
make targets only. Do NOT modify .golangci.yml. No Dockerfile/CI/script/.
Never mention Claude or Anthropic anywhere.

Depends on

PR #23 (#12) — it touches the same file; land that first to avoid a conflict.

Priority

Low-medium. Small, self-contained, and removes a genuine debugging trap.

## Problem `chooseSeed` in `cmd/rogue/main.go` falls back to the time+pid default when `SEED` is set but cannot be parsed. The user asked for a specific dungeon and silently got a random one. Set-but-invalid configuration should fail loudly. Silently substituting a different value is the worst of the three options (honour it, reject it, or quietly ignore it) because nothing distinguishes "I typo'd `SEED`" from "the seed worked". This matters more here than in a typical program: `SEED` exists precisely so a dungeon can be **reproduced** — for bug reports, for the seed-compat work, and for wizard-mode debugging. A silently-ignored seed produces an unreproducible session while the user believes it is pinned, which can send someone chasing a "non-deterministic" bug that is nothing of the kind. Found by the reworker of PR #23 (recorded there as M5) and correctly left alone as out of scope. ## Definition of done 1. `SEED` set and unparseable → the program exits non-zero with a message that names `SEED` and shows the offending value. It must not start a game. 2. `SEED` unset → unchanged behaviour (time+pid default). 3. `SEED` set and valid → unchanged behaviour. 4. Decide and document, in a code comment, what counts as valid: is an empty `SEED=` an error or the same as unset? Is a negative value accepted? State the choice rather than letting it fall out of `strconv`. 5. Tests cover all three cases in item 1-3 plus whatever item 4 settles. 6. The error goes to **stderr**, and the terminal is left usable — note the check should happen before raw mode is raised, or after `Fini`; do not introduce a path that prints an error into a raw terminal. 7. `make check` fully green. 8. `TODO.md` updated in the same commit — Completed Steps entry, and do **not** rotate "Next Step". 9. Commit title ends with ` (closes #N)`. ## Implementation requirements - Check what C does with a bad seed first (`git show origin/c-master:main.c`, and the `ROGUEOPTS`/seed handling). If C was permissive, this is a deliberate divergence and must be commented as such — but "fail loudly on invalid config" is the right call regardless, since C had no `SEED` environment variable to be faithful to in the first place. Confirm that. - Do NOT change the signal-handling or `pendingSaver` code settled in PR #23. - Do NOT regenerate goldens under `game/testdata/`. - Leave `c-master` and `modern-rogue` alone. - **Lint environment warning:** golangci-lint on this host shares one cache and lock across ~18 concurrent sessions. Run `make lint`/`make check` in a retry loop and only accept a run that neither reports `parallel golangci-lint is running` nor mentions paths outside your own worktree. Otherwise you may report green on a red branch. - `make` targets only. Do NOT modify `.golangci.yml`. No Dockerfile/CI/`script/`. - Never mention Claude or Anthropic anywhere. ## Depends on PR #23 (#12) — it touches the same file; land that first to avoid a conflict. ## Priority Low-medium. Small, self-contained, and removes a genuine debugging trap.

clawbot referenced this issue

2026-08-09 08:27:33 +02:00

SIGHUP/SIGTERM autosave is a data race: gob-encodes live state from a second goroutine #24

clawbot referenced this issue

2026-08-09 08:49:33 +02:00

fix: take the signal-time autosave on the game goroutine (closes #24) #26

clawbot referenced this issue

2026-08-09 09:01:10 +02:00

fix: take the signal-time autosave on the game goroutine (closes #24) #26

clawbot referenced this issue

2026-08-09 09:09:47 +02:00

fix: take the signal-time autosave on the game goroutine (closes #24) #26