docs,save: correct a stale TODO claim and rename encodeSnapshot (closes #27)

Four cleanups recorded as advisories during the PR #26 review and
deliberately kept out of it. No behaviour change.

The 2026-08-09 sig-leave (closes #12) TODO entry still argued in the
present tense that declining to save on SIGINT/SIGQUIT was also the safe
choice, "because AutoSave gob-encodes live state that the main goroutine
is still mutating, after removing the old file". Both halves stopped
being true with #24: the encode runs on the game goroutine and saveFile
is CreateTemp/Sync/Chmod/Rename with no Remove. The paragraph is now in
the past tense and marked superseded, pointing at the fix/autosave-race
entry, and says the split stands on C and on semantics alone — which is
what the current savesOnSignal comment says. The false claim was in the
#12 entry, not the #24 one; the latter's account of the old
remove-then-write is correctly historical and is untouched, as is the
err113 mention in the 2026-07-06 entry.

encodeSnapshot becomes writeSnapshotFile: it encodes, fsyncs, chmods
0400 and closes, and the old name claimed only the first of those. Its
doc comment now names all four and why the fsync is there.

TestAutoSaveOnSignalWhileInShellEscape used t.Error for a precondition,
so a save that was never taken fell through into assertRestorable, which
can then only report a second, derived failure. t.Fatal, matching the
identical assertion in the blocked-on-input test.

serviceAutoSaveRequest's doc comment carried a 24-column stub line
("The result is still a") left by an earlier edit. gofmt does not rewrap
comments, so fmt-check was legitimately green and nothing would have
caught it; the paragraph is rewrapped to the block's width.

Next Step deliberately not rotated: out-of-band issue work.

TODO.md

@@ -34,6 +34,32 @@ wizard commands).
 
 # Completed Steps
 
+- 2026-08-09 Cleanups deferred from the PR #26 review (`cleanup/pr26-followups`,
+  closes #27): four items, no behaviour change. (1) The `sig-leave` entry below
+  still argued, in the present tense, that declining to save on SIGINT/SIGQUIT
+  was the safe choice because `AutoSave` encodes live state after removing the
+  file — both halves untrue since #24, and the entry read as a claim about how
+  the code works now rather than a record of what was weighed then. It is in the
+  past tense and marked superseded, pointing at the `fix/autosave-race` entry.
+  Nothing else in the file was touched — in particular the `err113` linter name
+  in the 2026-07-06 entry, which a `grep` for `113` still matches, and the "over
+  a hundred reports" wording the #26 rework had already corrected. Note for
+  anyone chasing this class of bug: the false claim was in the #12 entry, not
+  the #24 one, whose account of the old remove-then-write is correctly past
+  tense — find these by content, since `make fmt` reflows the file and cited
+  line numbers rot. (2) `encodeSnapshot` is `writeSnapshotFile`: it encodes,
+  fsyncs, chmods 0400 and closes, and the old name claimed only the first of
+  those. One call site (`saveFile`), and the doc comment now lists what it does
+  and why the fsync is there. (3) `TestAutoSaveOnSignalWhileInShellEscape` used
+  `t.Error` for its precondition, so a save that was never taken fell through
+  into `assertRestorable`, which can then only report a second, derived failure;
+  it is `t.Fatal`, matching the identical assertion in the blocked-on-input
+  test. (4) `serviceAutoSaveRequest`'s doc comment had a 24-column stub line
+  ("The result is still a") left by an earlier edit — `gofmt` does not rewrap
+  comments, so `fmt-check` was legitimately green and nothing would ever have
+  caught it. Rewrapped to the block's width. `Next Step` deliberately not
+  rotated: out-of-band issue work.
+
 - 2026-08-09 Signal-time autosave moved onto the game goroutine
   (`fix/autosave-race`, closes #24): the SIGHUP/SIGTERM handler gob-encoded the
   live game tree from the signal goroutine while the game goroutine was mid-turn
@@ -115,45 +141,51 @@ wizard commands).
   HUP/TERM — and the semantics agree: HUP/TERM are involuntary teardown worth
   rescuing a game from, while INT/QUIT are a deliberate "stop now" that must not
   become a one-keystroke checkpoint against a save discipline built to be
-  anti-save-scum. It is also the safe choice: `AutoSave` gob-encodes live state
+  anti-save-scum. A third ground was weighed at the time and has since been
-  that the main goroutine is still mutating, after removing the old file, so on
+  superseded: back then `AutoSave` gob-encoded live state that the main
-  the signals with nothing to rescue the port takes the option with no
+  goroutine was still mutating, after removing the old file, so declining to
-  corruption window. The single-reader design closes the window the issue warned
+  save on the signals with nothing to rescue was also the option with no
-  about: a second signal arriving mid-save stays unread in the buffer instead of
+  corruption window. That window is gone as of the `fix/autosave-race` entry
-  exiting out from under the writer (`TestLeaveOnSignalIgnoresLaterSignals`
+  above (#24) — the encode now runs on the game goroutine and `saveFile` renames
-  reproduces exactly that interleaving). New `cmd/rogue/main_test.go` pins the
+  a temporary file into place — so nothing here should be read as a statement
-  membership of `handledSignals()` itself (`TestHandledSignalsSet` — without it
+  about how saving works now; the split stands on C and on semantics alone, as
-  the rest of the file, which iterates that set, would pass against a set that
+  the current `savesOnSignal` comment says. The single-reader design closes the
-  had silently lost SIGINT and SIGQUIT again), and covers the ordering for each
+  window the issue warned about: a second signal arriving mid-save stays unread
-  signal, the save/no-save split against `savesOnSignal`, the
+  in the buffer instead of exiting out from under the writer
-  mid-save-second-signal case, the pre-game `pendingSaver` window, and real
+  (`TestLeaveOnSignalIgnoresLaterSignals` reproduces exactly that interleaving).
-  SIGINT/SIGQUIT/SIGHUP/SIGTERM delivered to the test process through the same
+  New `cmd/rogue/main_test.go` pins the membership of `handledSignals()` itself
-  `notifySignals` wiring the game uses; the tty leaving raw mode is the one step
+  (`TestHandledSignalsSet` — without it the rest of the file, which iterates
-  not checkable headlessly (it needs a controlling terminal), and
+  that set, would pass against a set that had silently lost SIGINT and SIGQUIT
-  `term.Tcell.Fini` is a direct pass-through to tcell's `Screen.Fini` that
+  again), and covers the ordering for each signal, the save/no-save split
-  `myExit` already depends on. Two premises in the issue turned out to be wrong
+  against `savesOnSignal`, the mid-save-second-signal case, the pre-game
-  and are recorded in ARCHITECTURE.md: `leave()` is not installed on
+  `pendingSaver` window, and real SIGINT/SIGQUIT/SIGHUP/SIGTERM delivered to the
-  SIGINT/SIGQUIT during play (the wiring is in `mdport.c`, the shipped build
+  test process through the same `notifySignals` wiring the game uses; the tty
-  calls `md_onsignal_default()` and installs nothing, and `leave()` appears only
+  leaving raw mode is the one step not checkable headlessly (it needs a
-  in the endgame paths of `rip.c`/`main.c`), and Ctrl-C never generated SIGINT
+  controlling terminal), and `term.Tcell.Fini` is a direct pass-through to
-  here anyway, since tcell's raw mode clears `ISIG` and the key arrives as byte
+  tcell's `Screen.Fini` that `myExit` already depends on. Two premises in the
-  `0x03` — as it did in C, whose `setup()` calls curses `raw()`. The real
+  issue turned out to be wrong and are recorded in ARCHITECTURE.md: `leave()` is
-  exposure is `kill -INT`/`kill -QUIT`, a SIGINT to the process group while the
+  not installed on SIGINT/SIGQUIT during play (the wiring is in `mdport.c`, the
-  `!` shell escape has the screen suspended, and the window **after**
+  shipped build calls `md_onsignal_default()` and installs nothing, and
-  `term.New()`: nothing is raw before it, and the handlers used to be installed
+  `leave()` appears only in the endgame paths of `rip.c`/`main.c`), and Ctrl-C
-  only once the game existed, leaving the restore path and `-d`'s `DeathDemo()`
+  never generated SIGINT here anyway, since tcell's raw mode clears `ISIG` and
-  — which never returns, blocking in `waitFor` inside `death()` — running raw
+  the key arrives as byte `0x03` — as it did in C, whose `setup()` calls curses
-  with no handler at all. The handlers are therefore installed immediately after
+  `raw()`. The real exposure is `kill -INT`/`kill -QUIT`, a SIGINT to the
-  `term.New()`, with the game handed to them afterwards via `pendingSaver`; a
+  process group while the `!` shell escape has the screen suspended, and the
-  signal before the game exists restores the terminal and exits with nothing to
+  window **after** `term.New()`: nothing is raw before it, and the handlers used
-  save, and the SIGHUP/SIGTERM autosave behavior on the play path is unchanged.
+  to be installed only once the game existed, leaving the restore path and
-  ARCHITECTURE.md §9 gained rows for SIGTSTP/`tstp()` (dropped: raw mode means
+  `-d`'s `DeathDemo()` — which never returns, blocking in `waitFor` inside
-  Ctrl-Z cannot reach us, a suspend from the signal goroutine would race the
+  `death()` — running raw with no handler at all. The handlers are therefore
-  drawing goroutine, and C armed `tstp` only after a `restore()`; the `!` shell
+  installed immediately after `term.New()`, with the game handed to them
-  escape covers the need), for SIGINT not routing to the interactive `quit()`
+  afterwards via `pendingSaver`; a signal before the game exists restores the
-  prompt, and for `auto_save` on the fault signals; §5.3's claim that tcell
+  terminal and exits with nothing to save, and the SIGHUP/SIGTERM autosave
-  handles SIGTSTP was false — tcell registers only SIGWINCH — and is corrected.
+  behavior on the play path is unchanged. ARCHITECTURE.md §9 gained rows for
-  `Next Step` deliberately not rotated: out-of-band issue work.
+  SIGTSTP/`tstp()` (dropped: raw mode means Ctrl-Z cannot reach us, a suspend
+  from the signal goroutine would race the drawing goroutine, and C armed `tstp`
+  only after a `restore()`; the `!` shell escape covers the need), for SIGINT
+  not routing to the interactive `quit()` prompt, and for `auto_save` on the
+  fault signals; §5.3's claim that tcell handles SIGTSTP was false — tcell
+  registers only SIGWINCH — and is corrected. `Next Step` deliberately not
+  rotated: out-of-band issue work.
 
 - 2026-08-09 Wizard-create bounds fix (`fix/wizard-which-bounds`, closes #10):
   `createObj` stored the raw `0-f` nibble as `Object.Which` with no bounds

game/autosave_test.go

@@ -152,7 +152,7 @@ func TestAutoSaveOnSignalWhileInShellEscape(t *testing.T) {
 	<-st.entered
 
 	if !g.AutoSaveOnSignal(autoSaveWait) {
-		t.Error("the save was not taken while the game was in the shell escape")
+		t.Fatal("the save was not taken while the game was in the shell escape")
 	}
 
 	assertRestorable(t, g.FileName)

game/save.go

@@ -670,7 +670,7 @@ func (g *RogueGame) saveFile(path string) error {
 
 	tmp := f.Name()
 
-	writeErr := encodeSnapshot(f, g.snapshot())
+	writeErr := writeSnapshotFile(f, g.snapshot())
 	if writeErr != nil {
 		_ = os.Remove(tmp) // never leave a half-written file behind
 
@@ -687,11 +687,13 @@ func (g *RogueGame) saveFile(path string) error {
 	return nil
 }
 
-// encodeSnapshot encodes the snapshot into an open temporary file and
+// writeSnapshotFile writes the snapshot into an open temporary file: it
-// closes it, leaving it read-only as the C game's saves were (save.c
+// encodes, fsyncs so the bytes reach the disk before the caller renames
-// save_file). It never removes the file: its caller owns the cleanup, so
+// the file into place, chmods it read-only as the C game's saves were
-// that one place decides what happens to a failed write.
+// (save.c save_file), and closes it. It never removes the file: its
-func encodeSnapshot(f *os.File, st *SaveState) error {
+// caller owns the cleanup, so that one place decides what happens to a
+// failed write.
+func writeSnapshotFile(f *os.File, st *SaveState) error {
 	encErr := gob.NewEncoder(f).Encode(st)
 	if encErr == nil {
 		encErr = f.Sync()
@@ -812,11 +814,10 @@ func (g *RogueGame) AutoSaveOnSignal(timeout time.Duration) bool {
 // handler that clears After before prompting, as identifyTrapCommand
 // does ahead of promptDirection.
 //
-// The result is still a
+// The result is still a coherent game state, one turn's worth of effects
-// coherent game state, one turn's worth of effects off — strictly better
+// off — strictly better than the torn encode this replaced, and the cost
-// than the torn encode this replaced, and the cost of being able to save
+// of being able to save a player whose line dropped mid-prompt, or who
-// a player whose line dropped mid-prompt, or who is away in a shell, at
+// is away in a shell, at all.
-// all.
 func (g *RogueGame) serviceAutoSaveRequest() {
 	select {
 	case req := <-g.sigSave: