From 56bcad9fc6a7e06665a72ff575e7977a6a5b39ab Mon Sep 17 00:00:00 2001 From: sneak Date: Sun, 9 Aug 2026 04:59:00 +0000 Subject: [PATCH] docs: correct stale claims in MEMORY.md, TODO.md, and README.md (closes #3) Four documented claims had gone false and were actively misdirecting agents working this repo; the independent reviewer on PR #9 repeated one of them verbatim. Each claim was re-verified against the tree before rewriting. MEMORY.md "Error handling" described C's exit() calls being unwound by a gameEnd panic recovered in Run. Refactor step 8 removed that: gameEnd appears nowhere in the sources, myExit (game/rip.go) restores the terminal via Terminal.Fini and calls os.Exit(0), and Run() has no return values and never returns. The section now states that model and its testing consequence -- a death exits the test binary, which is why tests drive command() directly and crash sweeps pin the hero with fortify() in game/run_test.go. MEMORY.md "Linting" said approved exceptions are recorded in a "Repo-specific exceptions" block in .golangci.yml. There is no such block: the config is byte-identical to canonical (sha256 021cc83f...46bcb) and the approvals live in in-code //nolint directives carrying their dates. Following the old text would have meant editing the canonical config. The same paragraph listed paralleltest as an approved disable when it was fixed instead -- no paralleltest token exists in the tree and all 32 tests call t.Parallel(). MEMORY.md "Debugging" and README.md both told the reader to run go test directly. Since PR #9 the test target carries -timeout 30s -race -cover, so a raw invocation silently drops the race detector while appearing to verify the change. Both now point at make test / make check. TODO.md asserted the host golangci-lint is "currently v2.12.2". It is v2.10.1 and the repo pins nothing, so the claim documented an accident of one machine. Only the false claim is removed; the pin question is tracked separately. Documentation only: no code, Makefile, or config change. Next Step is deliberately not rotated, per the precedent for out-of-band issue work. --- MEMORY.md | 42 +++++++++++++++++++++++++++--------------- README.md | 12 +++++++----- TODO.md | 25 ++++++++++++++++++++++++- 3 files changed, 58 insertions(+), 21 deletions(-) diff --git a/MEMORY.md b/MEMORY.md index f1b02b4..0ce580b 100644 --- a/MEMORY.md +++ b/MEMORY.md @@ -7,25 +7,34 @@ the step queue and workflow) before starting work. Panicking on bad/unexpected errors is allowed and preferred over threading unlikely error returns through game code — e.g. write-side Close/encode failures -where continuing would mean corrupt state. The game already unwinds C's exit() -calls via a gameEnd panic recovered in Run. Return errors where a caller +where continuing would mean corrupt state. Return errors where a caller genuinely handles them (save-file prompts, restore validation). Reserve deliberate `_ =` discards for true best-effort paths (scorefile writes, signal-time autosave), always with a comment saying why. +C's exit() calls are not unwound: one game run is one process, so myExit +(game/rip.go) restores the terminal via Terminal.Fini and calls os.Exit(0), and +Run() never returns. There is nothing to recover — do not write code that +expects to regain control after game-over. The testing consequence is that any +death (combat, starvation, level drain, freezing) exits the _test binary_, so +tests drive command() directly rather than Run(), and crash-sweep drives pin the +hero each turn with the fortify() helper in game/run_test.go. + ## Linting -The .golangci.yml is the house standard and may only be modified with sneak's -explicit permission. To disable a linter, ask, explaining what the linter does; -he approves specific exceptions, which are recorded in the config's -"Repo-specific exceptions" block with the approval date. Approved so far: -paralleltest (2026-07-06); testpackage, exhaustive, and mnd (2026-07-07). The -complexity linters (cyclop, gocognit, nestif) are enabled and clean as of -refactor step 7 (2026-07-07): the whole golangci-lint run is 0 issues, so keep -it that way — decompose new hot spots rather than reaching for a nolint. -Line-level //nolint with a reason is used sparingly for C-faithfulness (e.g. the -authentic "missle" message spellings) and provably-safe gosec conversions; each -needs a justifying comment. +The .golangci.yml is byte-identical to the canonical shared config and must not +be edited — not even to add an exception. To disable a linter, ask sneak, +explaining what the linter does; approved exceptions are recorded as in-code +//nolint directives (file-level where a whole file is affected) carrying the +approval date, which is what keeps the config canonical. Approved so far: +testpackage, exhaustive, and mnd (2026-07-07). paralleltest was approved on +2026-07-06 but the exception is no longer in force — it was fixed instead, with +t.Parallel() in all 32 tests. The complexity linters (cyclop, gocognit, nestif) +are enabled and clean as of refactor step 7 (2026-07-07): the whole +golangci-lint run is 0 issues, so keep it that way — decompose new hot spots +rather than reaching for a nolint. Line-level //nolint with a reason is used +sparingly for C-faithfulness (e.g. the authentic "missle" message spellings) and +provably-safe gosec conversions; each needs a justifying comment. ## Faithfulness @@ -37,5 +46,8 @@ func_name)" breadcrumbs. ## Debugging -Write real, committed test files with t.Logf output and run plain `go test -v`; -no throwaway scratch scripts. Successful debug probes become regression tests. +Write real, committed test files with t.Logf output and run them with the make +targets — `make test` (or `make check` for the full gate); never raw `go test`. +The target carries `-timeout 30s -race -cover` and reruns verbosely on failure, +so a raw invocation silently drops the race detector. No throwaway scratch +scripts. Successful debug probes become regression tests. diff --git a/README.md b/README.md index 7dd6652..94cff6a 100644 --- a/README.md +++ b/README.md @@ -72,13 +72,15 @@ term/ tcell-backed terminal, replacing curses cmd/rogue/ the executable ``` -The engine package is fully headless-testable: `go test ./game/` runs scripted -command sequences, dungeon-generation golden checks, and an RNG compatibility -test against the original C generator. +The engine package is fully headless-testable: `make test` runs scripted command +sequences, dungeon-generation golden checks, and an RNG compatibility test +against the original C generator. For development, the `Makefile` wraps the toolchain: `make fmt` (gofmt + -prettier), `make lint` (golangci-lint), `make test`, and `make check` (all -three). +prettier), `make lint` (golangci-lint), `make test` (the suite, under the race +detector with coverage and a timeout), and `make check` (all three). Use the +targets rather than invoking `go test` directly — they carry the flags the +project relies on. ## License diff --git a/TODO.md b/TODO.md index c0833bc..2904373 100644 --- a/TODO.md +++ b/TODO.md @@ -34,6 +34,29 @@ wizard commands). # Completed Steps +- 2026-08-09 Stale-docs correction (`docs-staleness`, closes #3): four claims in + `MEMORY.md`/`TODO.md`/`README.md` had gone false and were misdirecting agents + — the reviewer on PR #9 repeated one of them verbatim. Each was re-verified + against the tree before rewriting. (1) `MEMORY.md` described C's `exit()` + being unwound by a `gameEnd` panic recovered in `Run`; refactor step 8 deleted + that, `gameEnd` appears nowhere in the sources, and `myExit` (`game/rip.go`) + now calls `Terminal.Fini` then `os.Exit(0)` while `Run()` never returns — so + the section states the exit model and its testing consequence (a death exits + the test binary; hence `fortify()` in `game/run_test.go`). (2) `MEMORY.md` + said approved lint exceptions live in a "Repo-specific exceptions" block in + `.golangci.yml`; no such block exists and the config is byte-identical to + canonical (sha256 `021cc83f…46bcb`), the approvals having moved to in-code + `//nolint` directives carrying their dates — and `paralleltest` was listed as + an approved disable when it was in fact fixed (no `paralleltest` token in the + tree; 32 `t.Parallel()` calls against 32 tests). (3) `MEMORY.md` "Debugging" + and (4) `README.md` both told the reader to run `go test` directly, which + since PR #9 silently drops `-timeout 30s -race -cover`; both now point at + `make test`/`make check`. Also dropped the false "currently v2.12.2" host + linter claim from the 2026-08-07 entry (the host is v2.10.1 and nothing is + pinned; the pin question is tracked separately). Documentation only — no code, + `Makefile`, or config change; `Next Step` deliberately not rotated, since this + was out-of-band issue work. + - 2026-08-09 Policy-shaped `make test` (`make-test-policy-pattern`): the `test:` target was a bare `go test $(GO_PKGS)` and now runs `-timeout 30s -race -cover` with the mandated conditional verbose rerun (on @@ -58,7 +81,7 @@ wizard commands). 24 long lines wrapped or their comments tightened, control bytes in `term/tcell.go` as character literals, and two `wsl_v5` defer cuddles. The repo has no golangci-lint version pin to bump (no Dockerfile or CI; - `make lint` runs the host `golangci-lint`, currently v2.12.2). + `make lint` runs whatever `golangci-lint` is on the host). - 2026-07-24 Seed compatibility — item tables (seed-compat): instrumented the C reference on modern-rogue with a DUMP mode (testdata/c_seedcompat.patch) that -- 2.49.1