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

The SIGHUP/SIGTERM handler gob-encoded the live game tree from the signal
goroutine while the game goroutine was mid-turn mutating it, and AutoSave
removed the save file before encoding — so the failure mode was not a
stale save but a deleted one followed by a possibly torn replacement,
with a window in which the player had neither. The suite has run under
-race since 2026-08-09 and was green because nothing had ever driven the
turn loop concurrently with a signal: evidence of untested, not of safe.

The handler no longer writes anything. AutoSaveOnSignal posts a request,
wakes the input read, and waits up to signalSaveTimeout for the game
goroutine to take it; the encode runs on the goroutine that owns the
state, at the three points where that goroutine can sit: between turns
(command), on waking from a blocked readchar, and while parked in the `!`
shell escape (runShellEscape, which now runs the shell on a helper
goroutine so a hangup during it still rescues the game).

Blocked on input is the case that matters — a dropped connection lands
while the player is thinking, so a flag checked only between turns would
never be looked at. Terminal.ReadChar therefore returns (byte, bool),
with ok false meaning "woken by Interrupt, no key", and term.Tcell posts
a tcell.EventInterrupt onto tcell's own event queue to unpark PollEvent.
readchar services the request and reads again, so no caller sees it.

Running the shell on a helper goroutine would also have moved
term.Tcell.ShellEscape's panic on a failed Screen.Resume onto it, and a
panic at the top of any goroutine terminates the process without running
the deferred calls of the others — including cmd/rogue/main.go's
`defer t.Fini()`. The tty would have been left raw on precisely the path
where the terminal is already broken, which is issue #12's failure on a
path this change created. runShellEscape therefore recovers the helper's
panic and re-raises it on the game goroutine, whose stack has the restore
in it, so "every path restores the terminal via Terminal.Fini before
exiting" stays true.

saveFile writes a temporary file in the save's own directory, fsyncs it
and renames it over the target instead of truncating in place, so a save
that fails — or never happens because the deadline ran out — leaves the
player's previous save whole.

What the handoff guarantees is stated exactly rather than flatteringly:
the encode runs on the state-owning goroutine, so the snapshot is
internally consistent and restorable, but it is not necessarily taken
between commands. Only the check at the top of command is; the other two
service points both sit inside a command call already under way. readchar
is reached from mid-command prompts (--More--, askOverwrite, getStr, the
direction and pack prompts) with the command's mutations already applied,
and runShellEscape is reached from shell, an ordinary '!' command handler
dispatched inside command, with that turn's DoDaemons(Before) and
DoFuses(Before) already fired and its AFTER pass not yet. Restoring
re-enters playit at the top of command, so either way the rest of that
command is lost and a fresh BEFORE pass runs on top of the one in the
snapshot.

The SIGINT/SIGQUIT no-save decision and the single-signal-read ordering
guarantee are untouched. pendingSaver reads the game out from under its
mutex rather than delegating with it held, because the delegated call now
blocks until the save is taken.

ARCHITECTURE.md

@@ -1484,9 +1484,11 @@ needed function pointers for — so it is also what the save format stores.
 // Terminal is the physical device, behind an interface so tests run headless.
 // The real game uses term.Tcell; tests use a scripted testTerm.
 type Terminal interface {
-    Render(w *Window)  // blit a window to the device
+    Render(w *Window)         // blit a window to the device
-    ReadChar() byte    // event loop → C char codes (arrows→hjkl, ^C→quit)
+    ReadChar() (byte, bool)   // event loop → C char codes (arrows→hjkl, ^C→quit);
-    Fini()             // restore the device (curses endwin) on exit
+                              // ok is false when Interrupt woke the read
+    Interrupt()               // wake a blocked ReadChar (signal goroutine only)
+    Fini()                    // restore the device (curses endwin) on exit
 }
 
 // Screen exposes the curses vocabulary the ported code speaks, over a Terminal.
@@ -1518,12 +1520,65 @@ SIGWINCH — SIGTSTP is not among the signals it takes, and is dropped (§9).
 
 Signals are handled in `cmd/rogue/main.go`: one `os/signal` channel read by one
 goroutine that reads exactly one signal, so a second signal can never call
-`os.Exit` out from under an in-flight save. SIGHUP and SIGTERM `AutoSave` on the
+`os.Exit` out from under an in-flight save. SIGHUP and SIGTERM autosave on the
 way out (save.c `auto_save`); SIGINT and SIGQUIT restore the terminal and exit
 without saving, matching C — where `auto_save` is reserved for HUP/TERM and
 neither `leave()` nor `quit()` nor `endit()` writes a save file — and keeping a
 deliberate interrupt from becoming a free checkpoint.
 
+The signal goroutine does not write the save itself. It calls
+`RogueGame.AutoSaveOnSignal`, which posts a request, wakes the input read
+through `Terminal.Interrupt`, and waits for the game goroutine to take it or for
+a deadline to run out; the encode happens on the game goroutine, the only one
+that touches game state. It answers between turns (`command`), on waking from a
+blocked `readchar`, and while parked in the `!` shell escape (`runShellEscape`)
+— the three places it can sit for any length of time. Before issue #24 the
+handler gob-encoded the live game tree from its own goroutine after removing the
+save file, which raced every mutation the game was making and left a window with
+no save file at all; `saveFile` now writes a temporary file in the save's own
+directory and renames it over the target, so the previous save survives any
+failure, including the deadline expiring with nothing written.
+
+What that guarantees precisely, and what it does not: the encode runs on the one
+goroutine that owns the state, so the snapshot is internally consistent and
+always restorable. It is not guaranteed to be a between-commands snapshot. Only
+one of the three service points gives that: the check at the top of `command`,
+which runs after the previous command returned and before this turn's
+`DoDaemons(Before)`/`DoFuses(Before)`. The other two are both reached from
+inside a `command` call already under way. `readchar` is reached from prompts
+raised part-way through a command (`--More--`, `askOverwrite`, `getStr`, the
+direction and pack prompts), and the command has already mutated state by then —
+`fight` sets `Count`/`Quiet` and runs `runTo` before any message, `revealXeroc`
+writes `Disguise` before emitting one; the ordinary top-of-turn key read is
+inside `command` too, after that turn's BEFORE daemons and `turnUpkeep`.
+`runShellEscape` is no safer: `shell` is an ordinary command handler (`'!'` in
+the dispatch table), reached through `executeCommand`, so a goroutine parked in
+the shell escape has already run this turn's `DoDaemons(Before)`,
+`DoFuses(Before)`, `turnUpkeep` and the last-command bookkeeping, and has not
+yet run `DoDaemons(After)`, `DoFuses(After)` or `ringTurnEffects`. Restoring
+re-enters `playit` at the top of `command` in either case, so the rest of that
+command never runs — its AFTER daemons and fuses and its ring effects are lost —
+and the restored game opens with a fresh BEFORE pass on top of the one already
+in the snapshot: `rollwand`, a live BEFORE daemon once `swander` has fired,
+ticks again, and any BEFORE fuse is decremented again. Not every consequence of
+that pass is shared by both service points, though. `visuals` returns
+immediately unless `g.After`, and `After` is part of the snapshot, so `DVisuals`
+never re-ticks after a shell-escape save — `shell` sets `g.After = false` as its
+first statement, before it parks — whereas after a `readchar` save it usually
+does, because `turnUpkeep` sets `g.After = true` just before the top-of-turn
+read; the exception is a handler that clears `After` before prompting, as
+`identifyTrapCommand` does ahead of `promptDirection`. The result is a coherent
+state one turn's worth of effects off, which is the price of being able to save
+at all for a player whose line dropped mid-prompt or who is away in a shell.
+
+The shell escape runs the shell on a helper goroutine so that the game goroutine
+stays free to answer, but a panic out of `Terminal.ShellEscape` (which is how a
+failed `Screen.Resume` is reported) is recovered there and re-raised on the game
+goroutine. A panic reaching the top of a helper goroutine would kill the process
+without running the main goroutine's `defer t.Fini()`, leaving the tty raw — the
+failure issue #12 removed, on the one path where the terminal is already broken.
+Re-raising keeps the invariant below true.
+
 The handlers are installed immediately after `term.New()`, which is the call
 that puts the tty in raw mode, and before the game exists — the saver is handed
 over afterwards through `pendingSaver`. That ordering is what makes "every path
@@ -1611,26 +1666,26 @@ Tombstone/victory screens port verbatim from rip.c.
 
 ## 6. C construct → Go construct map
 
-| C construct                                          | Go translation                                                                                  |
+| C construct                                          | Go translation                                                                                                                                           |
-| ---------------------------------------------------- | ----------------------------------------------------------------------------------------------- |
+| ---------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| global variable                                      | `RogueGame` field (or Player/Level/subsystem field)                                             |
+| global variable                                      | `RogueGame` field (or Player/Level/subsystem field)                                                                                                      |
-| file-scope static                                    | unexported field on `RogueGame` or subsystem struct                                             |
+| file-scope static                                    | unexported field on `RogueGame` or subsystem struct                                                                                                      |
-| `THING` union                                        | `Creature` / `Object` structs                                                                   |
+| `THING` union                                        | `Creature` / `Object` structs                                                                                                                            |
-| linked lists (`l_next/l_prev`, attach/detach)        | slices + `attach(&s, x)`-style helpers (prepend, remove by identity)                            |
+| linked lists (`l_next/l_prev`, attach/detach)        | slices + `attach(&s, x)`-style helpers (prepend, remove by identity)                                                                                     |
-| `coord*` aliasing (t_dest)                           | `*Coord` pointers into live structs (kept)                                                      |
+| `coord*` aliasing (t_dest)                           | `*Coord` pointers into live structs (kept)                                                                                                               |
-| function pointers (daemons, options, nameit)         | enum IDs → handler tables on `gameData` (daemons + effect dispatch); closures (options, nameit) |
+| function pointers (daemons, options, nameit)         | enum IDs → handler tables on `gameData` (daemons + effect dispatch); closures (options, nameit)                                                          |
-| `when`/`otherwise`/`until` macros                    | `case`/`default`/`for !cond`                                                                    |
+| `when`/`otherwise`/`until` macros                    | `case`/`default`/`for !cond`                                                                                                                             |
-| `goto` (retry loops in pack.c, move.c, passages.c)   | labeled loops / restructured `for`                                                              |
+| `goto` (retry loops in pack.c, move.c, passages.c)   | labeled loops / restructured `for`                                                                                                                       |
-| char-indexed tables (`monsters[type-'A']`)           | same, `monsterTable[t.Type-'A']`                                                                |
+| char-indexed tables (`monsters[type-'A']`)           | same, `monsterTable[t.Type-'A']`                                                                                                                         |
-| damage strings `"3x4/1x2"`                           | parsed once into a `DiceSpec` (`[]DiceRoll`) by `ParseDice` at table time                       |
+| damage strings `"3x4/1x2"`                           | parsed once into a `DiceSpec` (`[]DiceRoll`) by `ParseDice` at table time                                                                                |
-| curses stdscr/hw windows                             | `Window` cell buffers over tcell                                                                |
+| curses stdscr/hw windows                             | `Window` cell buffers over tcell                                                                                                                         |
-| `mvinch` screen reads                                | `Window.Inch` from the buffer                                                                   |
+| `mvinch` screen reads                                | `Window.Inch` from the buffer                                                                                                                            |
-| signal handlers (SIGHUP autosave, SIGTSTP, SIGINT)   | `os/signal` goroutine → channel checked in ReadChar; tcell handles TSTP/resize                  |
+| signal handlers (SIGHUP autosave, SIGTSTP, SIGINT)   | `os/signal` goroutine; the autosave is handed to the game goroutine and taken between turns or on waking a blocked `ReadChar`; tcell handles resize only |
-| `setjmp`-free exits (`my_exit`, `exit()` everywhere) | `myExit` restores the terminal and calls `os.Exit(0)`; one game run is one process              |
+| `setjmp`-free exits (`my_exit`, `exit()` everywhere) | `myExit` restores the terminal and calls `os.Exit(0)`; one game run is one process                                                                       |
-| `vsprintf` message building                          | `fmt.Sprintf`                                                                                   |
+| `vsprintf` message building                          | `fmt.Sprintf`                                                                                                                                            |
-| XOR-encrypted binary saves (state.c)                 | gob snapshot (§5.6)                                                                             |
+| XOR-encrypted binary saves (state.c)                 | gob snapshot (§5.6)                                                                                                                                      |
-| DES wizard password                                  | `ROGUE_WIZARD=1` env check                                                                      |
+| DES wizard password                                  | `ROGUE_WIZARD=1` env check                                                                                                                               |
-| `md_*` platform shims                                | stdlib (`os`, `os/user`, `time`) or deleted                                                     |
+| `md_*` platform shims                                | stdlib (`os`, `os/user`, `time`) or deleted                                                                                                              |
 
 **Exit discipline** deserves a note: the C code calls `exit()` from deep inside
 call chains (death, victory, save). One game run is one process, so the port
@@ -1752,13 +1807,19 @@ the signal goroutine while the game goroutine may be inside `Render` or
 call from the signal goroutine is safe — it is a _logical_ race over the screen
 state: the game goroutine can redraw into a screen the handler has just
 suspended, or resume under a half-finished frame. Getting it right means
-plumbing the signal through the input loop and handling it synchronously, a
+plumbing the signal through the input loop and handling it synchronously. Half
-design change well beyond a signal-safety fix. C's own wiring here is vestigial:
+of that plumbing now exists — `Terminal.Interrupt` wakes a blocked `ReadChar` so
-`tstp` is armed only by `md_tstpresume()`, which runs after a successful
+the game goroutine can act on a signal (§5.3, issue #24) — but the suspend and
-`restore()`, so a freshly started C game never had a SIGTSTP handler either.
+the resume still have to be sequenced against the game's own drawing, which is
-`term.Tcell.ShellEscape` (the `!` command) already covers getting to a shell and
+the part that remains a design change rather than a signal-safety fix. C's own
-back, doing the same suspend/resume dance synchronously on the game goroutine
+wiring here is vestigial: `tstp` is armed only by `md_tstpresume()`, which runs
-where it is safe.
+after a successful `restore()`, so a freshly started C game never had a SIGTSTP
+handler either. `term.Tcell.ShellEscape` (the `!` command) already covers
+getting to a shell and back. Since issue #24 the shell itself runs on a helper
+goroutine (`runShellEscape`), so a hangup arriving while the player is away in
+the shell still rescues the game; the game goroutine waits there without
+drawing, so the suspend/resume dance is as undisturbed as it was when the call
+ran inline.
 
 **SIGINT → `quit()`.** Under `md_onsignal_autosave` C routed SIGINT into the
 interactive "really quit?" prompt. The port exits instead (after restoring the

MEMORY.md

@@ -10,7 +10,42 @@ unlikely error returns through game code — e.g. write-side Close/encode failur
 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.
+`Terminal.Interrupt`'s post to a full event queue), always with a comment saying
+why.
+
+Signal-time autosave used to be on that list and no longer is (issue #24). It is
+best effort in the sense that nothing can be reported to a player whose terminal
+is already going away, but the outcome is a value, not a discard: the signal
+goroutine calls `AutoSaveOnSignal`, which hands the save to the game goroutine —
+the only one allowed to touch game state — and returns whether it was taken
+before the deadline. The game answers between turns (`command`), while parked
+waiting for a key (`readchar`), and while parked in the `!` shell escape
+(`runShellEscape`). `saveFile` writes a temporary file and renames it over the
+target, so a save that fails or never happens leaves the player's previous save
+whole; never reintroduce a `Remove` before the write in `autoSave`, and never
+encode game state from any goroutine but the game's.
+
+Do not upgrade that into "the snapshot is always taken between commands" — it is
+not. What is true is that the encode runs on the state-owning goroutine, so the
+snapshot is internally consistent and restorable. Only the check at the top of
+`command` is a between-commands snapshot; the other two service points both sit
+inside a `command` call already under way. `readchar` is reached from
+mid-command prompts (`--More--`, `askOverwrite`, `getStr`, direction and pack
+prompts) with the command's mutations already applied, and `runShellEscape` is
+reached from `shell`, an ordinary `'!'` command handler, with that turn's
+`DoDaemons(Before)`/`DoFuses(Before)` already fired and its AFTER pass not yet.
+Restoring re-enters `playit` at the top of `command`, so either way the rest of
+that command is lost and a fresh BEFORE pass runs on top of the one already in
+the snapshot. That is acceptable and documented; two successive false claims —
+first that `readchar` was safe, then that two of the three service points were
+between-commands — were caught in review of PR #26, and neither may come back.
+
+Related, and easy to reintroduce: work moved onto a helper goroutine must not be
+allowed to panic there. A panic at the top of any goroutine kills the process
+without running the other goroutines' defers, including `cmd/rogue/main.go`'s
+`defer t.Fini()`, which is what leaves a raw tty (issue #12). `runShellEscape`
+recovers its helper's panic and re-raises it on the game goroutine for exactly
+that reason.
 
 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

TODO.md

@@ -34,6 +34,74 @@ wizard commands).
 
 # Completed Steps
 
+- 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
+  mutating it, and `AutoSave` **removed** the save file before encoding — so the
+  failure mode was not a stale save but a deleted one followed by a possibly
+  torn replacement, with a window in which the player had neither. `make test`
+  has run with `-race` since 2026-08-09 and was green, because no test had ever
+  driven the turn loop concurrently with a signal: evidence of untested, not of
+  safe. The handler now writes nothing itself. `AutoSaveOnSignal` posts a
+  request on a one-deep channel, wakes the input read, and waits up to
+  `signalSaveTimeout` (3s) for the game goroutine to take it; the encode happens
+  on the goroutine that owns the state. **The blocked-on-input case is the whole
+  point** — a dropped connection lands while the player is thinking, so a flag
+  checked only between turns would never be looked at — and it is handled by
+  making the read interruptible: `Terminal.ReadChar` returns `(byte, bool)` with
+  `ok == false` meaning "woken by `Interrupt`, no key", `term.Tcell.Interrupt`
+  posts a `tcell.EventInterrupt` onto tcell's own event queue to unpark
+  `PollEvent`, and `readchar` services the request and reads again, so no caller
+  sees the wake-up. The other unbounded park is the `!` shell escape, where a
+  hangup used to save and would otherwise have regressed to not saving: the
+  shell now runs on a helper goroutine and `runShellEscape` selects on {shell
+  finished, save request}, keeping the encode on the game goroutine while it
+  draws nothing. Between turns (`command`) covers a game that is busy rather
+  than parked. The wait is bounded so that a game goroutine wedged with no
+  service point can never stop a signal from getting the process out; giving up
+  costs nothing now that `saveFile` writes a temporary file in the save's own
+  directory, fsyncs it, and renames it over the target instead of truncating in
+  place — a failed or skipped save leaves the previous save whole. New
+  `game/autosave_test.go` drives the real turn loop while a second goroutine
+  asks for 25 saves (the interleaving that never existed before), plus the
+  parked-on-input case with a terminal fake that genuinely blocks, the shell
+  case, the deadline case (previous save byte-for-byte intact), the no-file-name
+  case, and the rename discipline — the last pinned by a handle opened before
+  the save, which still reads the old file whole after it. Each was
+  mutation-proved: reverting `AutoSaveOnSignal` to encode on the calling
+  goroutine (the pre-fix behavior) makes the turn-loop test fail under `-race`
+  with over a hundred reports, and removing each of the three service points
+  fails exactly the test for that park with its own message. `pendingSaver` now
+  reads the game out from under its mutex instead of delegating with it held,
+  because the delegated call blocks until the save is taken — the PR #23
+  review's N3 note, load-bearing rather than hypothetical, and pinned by a test.
+  The SIGINT/SIGQUIT no-save decision and the single-signal-read ordering
+  guarantee are untouched; `savesOnSignal`'s third ground ("safety") is
+  rewritten, since the corruption window it weighed no longer exists.
+  `MEMORY.md` stops listing signal-time autosave among the deliberate `_ =`
+  discards and states the new discipline; `ARCHITECTURE.md` §5.3, the `Terminal`
+  sketch, the C-to-Go mapping row and §9's SIGTSTP paragraph are corrected to
+  match. Two things review caught and this entry records so they are not undone:
+  moving the shell onto a helper goroutine also moved `term.Tcell.ShellEscape`'s
+  `panic` on a failed `Screen.Resume` there, and a panic at the top of any
+  goroutine kills the process without running the deferred calls of the others —
+  including `cmd/rogue/main.go`'s `defer t.Fini()`, so the tty would have been
+  left raw on exactly the path where the terminal is already broken (issue #12's
+  failure, reintroduced on a new path). `runShellEscape` recovers the helper's
+  panic and re-raises it on the game goroutine, pinned by
+  `TestShellEscapePanicUnwindsTheGameGoroutine`. And the doc comment took two
+  rounds to get right: the first version claimed in four places that nothing is
+  half-mutated at the `readchar` service point, and the revision that fixed that
+  claimed two of the three service points were between-commands. Both are false.
+  Only the check at the top of `command` is between commands — `readchar` is
+  reached from mid-command prompts, and `runShellEscape` is reached from
+  `shell`, an ordinary `'!'` command handler dispatched inside `command`, with
+  that turn's `DoDaemons(Before)`/`DoFuses(Before)` already fired and its AFTER
+  pass and ring effects not yet. What is actually guaranteed is that the encode
+  runs on the state-owning goroutine, so the snapshot is internally consistent
+  and restorable, though it may freeze a command half applied. `Next Step`
+  deliberately not rotated: out-of-band issue work.
+
 - 2026-08-09 Signal-time terminal restore (`sig-leave`, closes #12): the port
   handled only SIGHUP and SIGTERM, so SIGINT and SIGQUIT killed the process with
   tcell still holding the tty, leaving the user at a shell with no echo. All

cmd/rogue/main.go

@@ -115,11 +115,32 @@ func loadParams() game.Params {
 // saver is the autosave half of *game.RogueGame that the signal handler
 // needs; an interface so the handler is testable headlessly.
 type saver interface {
-	// AutoSave writes the game to its save file, best effort (save.c
+	// AutoSaveOnSignal asks the game goroutine to write the save file and
-	// auto_save).
+	// waits up to timeout for it, reporting whether the save ran (save.c
-	AutoSave()
+	// auto_save). The handler never encodes anything itself; see
+	// signalSaveTimeout.
+	AutoSaveOnSignal(timeout time.Duration) bool
 }
 
+// signalSaveTimeout bounds how long the signal handler waits for the game
+// goroutine to take its autosave.
+//
+// The handler cannot encode the game itself — that was issue #24's data
+// race — so it has to hand the work to the goroutine that owns the state
+// and wait. The game answers between turns, while parked waiting for a
+// key, and while parked in the shell escape, which covers everywhere it
+// can sit for any length of time; the deadline is the backstop for a game
+// goroutine wedged somewhere with no service point, so that a signal can
+// never fail to get the process out. It is generous next to the
+// milliseconds a gob encode of one game takes, and invisible to a player
+// whose connection has already dropped.
+//
+// Giving up costs nothing now that saveFile renames over the target
+// (game/save.go): a save that does not happen leaves the previous save
+// whole, where the old remove-then-encode could leave the player with
+// neither.
+const signalSaveTimeout = 3 * time.Second
+
 // finisher is the terminal-restoring half of game.Terminal that the
 // signal handler needs (curses endwin).
 type finisher interface {
@@ -130,26 +151,35 @@ type finisher interface {
 // pendingSaver is the saver the signal handler holds from the moment the
 // terminal goes raw. The handler has to be armed before there is a game
 // to save — restoring a save file and the death demo both run with the
-// tty already raw — so AutoSave does nothing until set hands over the
+// tty already raw — so AutoSaveOnSignal does nothing until set hands over
-// real game. The mutex is not decoration: set runs on the main goroutine
+// the real game. The mutex is not decoration: set runs on the main
-// and AutoSave on the signal goroutine.
+// goroutine and AutoSaveOnSignal on the signal goroutine.
 type pendingSaver struct {
 	mu   sync.Mutex
 	game saver
 }
 
-// AutoSave saves the game if there is one yet, and otherwise does
+// AutoSaveOnSignal saves the game if there is one yet, and otherwise does
 // nothing: a signal arriving before the game is built still restores the
 // terminal, which is the part that matters.
-func (p *pendingSaver) AutoSave() {
+//
+// The lock is held only long enough to read the game, not across the
+// delegated save. That changed with issue #24: the real
+// AutoSaveOnSignal now blocks until the game goroutine takes the save or
+// the deadline expires, and holding the mutex across a wait that long
+// would stall a concurrent set — the case the PR #23 review flagged as
+// safe only for as long as set is called exactly once. This shape does
+// not depend on that.
+func (p *pendingSaver) AutoSaveOnSignal(timeout time.Duration) bool {
 	p.mu.Lock()
-	defer p.mu.Unlock()
+	g := p.game
+	p.mu.Unlock()
 
-	if p.game == nil {
+	if g == nil {
-		return
+		return false
 	}
 
-	p.game.AutoSave()
+	return g.AutoSaveOnSignal(timeout)
 }
 
 // set hands the signal handler the game to autosave, once one exists.
@@ -196,12 +226,16 @@ func handledSignals() []os.Signal {
 // would turn it into a one-keystroke undo for a bad turn: a gameplay
 // change, not a robustness fix.
 //
-// Safety: this runs on a goroutine while the main goroutine is mid-turn
+// Safety: this used to be the third ground, back when the handler
-// mutating game state, and AutoSave removes the save file before
+// gob-encoded live game state from its own goroutine after removing the
-// gob-encoding that live state. On HUP/TERM that risk is accepted
+// save file — a data race with a window in which the player had no save
-// because the process is about to die regardless and a best-effort save
+// at all, accepted on HUP/TERM because the process was dying anyway and
-// beats none. On INT/QUIT there is nothing to rescue, so the right
+// avoided entirely on INT/QUIT. Issue #24 removed the window instead of
-// choice is the one with no corruption window at all.
+// living with it: the handler now hands the save to the game goroutine
+// and waits (AutoSaveOnSignal), and the write goes to a temporary file
+// renamed over the target. The split above stands on C and on semantics,
+// which is where it always belonged; INT and QUIT do not save because
+// the player asked to stop, not because saving is dangerous.
 func savesOnSignal(sig os.Signal) bool {
 	return sig == syscall.SIGHUP || sig == syscall.SIGTERM
 }
@@ -239,14 +273,19 @@ func notifySignals() chan os.Signal {
 // leaveOnSignal waits for one signal and takes the game out.
 //
 // Exactly one goroutine reads exactly one signal, which is what makes
-// the exit safe: a second signal (a SIGINT landing while a SIGHUP's
+// the exit safe: a second signal (a SIGINT landing while a SIGHUP's save
-// AutoSave is still writing, say) stays in the buffer unread and can
+// is still being written, say) stays in the buffer unread and can never
-// never call exit out from under an in-flight save. The order within is
+// call exit out from under an in-flight save. The order within is the
-// the same one myExit uses (game/rip.go): save if this signal saves,
+// same one myExit uses (game/rip.go): save if this signal saves, then
-// then restore the terminal, then exit.
+// restore the terminal, then exit.
+//
+// AutoSaveOnSignal returns once the game goroutine has finished writing,
+// or once signalSaveTimeout has run out, so the save is complete before
+// the terminal is torn down and the process leaves — and the process
+// leaves either way.
 func leaveOnSignal(sig <-chan os.Signal, g saver, t finisher, exit func(int)) {
 	if savesOnSignal(<-sig) {
-		g.AutoSave()
+		g.AutoSaveOnSignal(signalSaveTimeout)
 	}
 
 	t.Fini()

cmd/rogue/main_test.go

@@ -11,6 +11,7 @@ import (
 	"sync"
 	"syscall"
 	"testing"
+	"time"
 )
 
 // The steps the signal handler can take, in the order signalRecorder
@@ -61,9 +62,13 @@ func newSignalRecorder() *signalRecorder {
 	return &signalRecorder{done: make(chan struct{})}
 }
 
-// AutoSave records a save attempt (the saver half).
+// AutoSaveOnSignal records a save attempt (the saver half). The real one
-func (r *signalRecorder) AutoSave() {
+// hands the work to the game goroutine and waits; the recorder stands in
+// for a game that takes it immediately.
+func (r *signalRecorder) AutoSaveOnSignal(time.Duration) bool {
 	r.record(stepSave)
+
+	return true
 }
 
 // Fini records a terminal restore (the finisher half).
@@ -231,11 +236,12 @@ type blockingSaver struct {
 	extra os.Signal
 }
 
-// AutoSave delivers the extra signal mid-save, then records the save.
+// AutoSaveOnSignal delivers the extra signal mid-save, then records the
-func (b *blockingSaver) AutoSave() {
+// save.
+func (b *blockingSaver) AutoSaveOnSignal(timeout time.Duration) bool {
 	b.queue <- b.extra
 
-	b.rec.AutoSave()
+	return b.rec.AutoSaveOnSignal(timeout)
 }
 
 // TestLeaveOnRealSignal is the deepest headless check available: it
@@ -314,10 +320,63 @@ func TestPendingSaverArmsBeforeTheGameExists(t *testing.T) {
 	// Once the game is handed over, the same saver writes it.
 	started := newSignalRecorder()
 	pending.set(started)
-	pending.AutoSave()
+
+	if !pending.AutoSaveOnSignal(signalSaveTimeout) {
+		t.Error("after set: the save was not reported as taken")
+	}
 
 	saved, _ := started.taken()
 	if want := []string{stepSave}; !slices.Equal(saved, want) {
 		t.Errorf("after set: steps = %v, want %v", saved, want)
 	}
 }
+
+// TestPendingSaverDoesNotHoldItsLockAcrossTheSave pins the reason
+// pendingSaver reads the game out from under the mutex instead of
+// delegating with it held: since issue #24 the delegated save blocks
+// until the game goroutine takes it or the deadline expires, so a mutex
+// held across it would stall whoever calls set. Nothing calls set twice
+// today, which is why the PR #23 review recorded this as a future-proof
+// note rather than a bug — this test is what stops it becoming one.
+func TestPendingSaverDoesNotHoldItsLockAcrossTheSave(t *testing.T) {
+	t.Parallel()
+
+	pending := &pendingSaver{}
+	stuck := &stuckSaver{entered: make(chan struct{}), release: make(chan struct{})}
+	pending.set(stuck)
+
+	go pending.AutoSaveOnSignal(signalSaveTimeout)
+
+	<-stuck.entered // the delegated save is in flight
+
+	done := make(chan struct{})
+
+	go func() {
+		defer close(done)
+
+		pending.set(newSignalRecorder()) // must not block on the save
+	}()
+
+	select {
+	case <-done:
+	case <-time.After(time.Second):
+		t.Error("set blocked while a save was in flight: the lock is held across it")
+	}
+
+	close(stuck.release)
+}
+
+// stuckSaver blocks inside the delegated save until it is released,
+// standing in for a game goroutine that is slow to answer.
+type stuckSaver struct {
+	entered chan struct{}
+	release chan struct{}
+}
+
+// AutoSaveOnSignal blocks until the test releases it.
+func (s *stuckSaver) AutoSaveOnSignal(time.Duration) bool {
+	close(s.entered)
+	<-s.release
+
+	return true
+}

game/autosave_test.go

@@ -0,0 +1,511 @@
+//nolint:testpackage // white-box tests reach unexported state (approved 2026-07-07)
+package game
+
+// Tests for the signal-triggered autosave handoff (issue #24): the signal
+// goroutine must never encode game state itself, and the game goroutine
+// must answer wherever it is parked.
+
+import (
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+// autoSaveWait is the deadline the tests hand AutoSaveOnSignal when they
+// expect the save to be taken. It is long enough that a loaded machine
+// cannot turn a working handoff into a spurious failure, and it is never
+// actually waited out on a passing run.
+const autoSaveWait = 10 * time.Second
+
+// TestAutoSaveOnSignalRacesTurnLoop is the test issue #24 exists for: it
+// drives the real turn loop on one goroutine while another asks for a
+// signal-triggered autosave over and over, which is the interleaving no
+// test in the suite used to produce. `make test` runs with -race, so a
+// save that encodes the live game tree from the asking goroutine — what
+// the old AutoSave did straight from the signal handler — is reported as
+// a data race and fails this test.
+//
+// Non-vacuity: with AutoSaveOnSignal's body replaced by a direct
+// g.autoSave() call, i.e. exactly the pre-#24 behavior, this test fails
+// under -race with the encoder reading state that command() is writing.
+func TestAutoSaveOnSignalRacesTurnLoop(t *testing.T) {
+	t.Parallel()
+
+	// Same mix as TestTurnLoopCrashSweep: the spaces answer any --More--
+	// prompt, and the script is long enough that the drive never runs it
+	// out.
+	script := []byte(strings.Repeat("h j k l y u b n s . ", 400))
+
+	g := New(Params{Seed: 20260809, Term: &testTerm{input: script}})
+	g.FileName = filepath.Join(t.TempDir(), "rogue.save")
+	g.startLevel()
+	g.prePlay()
+
+	const wantSaves = 25
+
+	var taken int
+
+	done := make(chan struct{})
+
+	go func() {
+		defer close(done)
+
+		for range wantSaves {
+			if g.AutoSaveOnSignal(autoSaveWait) {
+				taken++
+			}
+		}
+	}()
+
+	driveUntilDone(t, g, done)
+
+	// The close of done orders that goroutine's writes before this read.
+	if taken != wantSaves {
+		t.Errorf("saves taken = %d, want %d", taken, wantSaves)
+	}
+
+	// Every request was answered by the turn loop, so the file is the
+	// work of the game goroutine and must be a whole save.
+	assertRestorable(t, g.FileName)
+}
+
+// driveUntilDone runs turns until the saving goroutine is finished,
+// fortifying the hero each turn so no death exits the test binary. The
+// turn cap keeps a broken handoff from hanging the suite instead of
+// failing it.
+func driveUntilDone(t *testing.T, g *RogueGame, done <-chan struct{}) {
+	t.Helper()
+
+	const maxTurns = 1000
+
+	for range maxTurns {
+		select {
+		case <-done:
+			return
+		default:
+		}
+
+		fortify(g)
+		g.command()
+	}
+
+	t.Fatal("the turn loop ran out of turns before the saves were taken")
+}
+
+// TestAutoSaveOnSignalWhileBlockedOnInput is the case the fix is really
+// for: the connection drops while the player is staring at the screen,
+// so the game goroutine is parked in ReadChar and will not reach the
+// between-turns check on its own. A flag checked only between turns would
+// never be looked at here.
+func TestAutoSaveOnSignalWhileBlockedOnInput(t *testing.T) {
+	t.Parallel()
+
+	bt := newBlockingTerm()
+	g := mkBlockedGame(t, bt)
+
+	read := make(chan byte)
+
+	go func() { read <- g.readchar() }()
+
+	// The wake is buffered, so this is correct whether or not the reader
+	// has reached ReadChar yet.
+	if !g.AutoSaveOnSignal(autoSaveWait) {
+		t.Fatal("the save was not taken while the game was blocked on input")
+	}
+
+	assertRestorable(t, g.FileName)
+
+	// The interrupt must not have been mistaken for a keystroke: the
+	// reader is still waiting, and still returns the real key.
+	bt.keys <- 'x'
+
+	if ch := <-read; ch != 'x' {
+		t.Errorf("readchar() = %q, want 'x'", ch)
+	}
+}
+
+// TestAutoSaveOnSignalWhileInShellEscape covers the other place the game
+// goroutine parks for an unbounded time: the `!` shell escape, where it
+// used to sit inside the shell call with no way to answer. A dropped line
+// while the player is off in a shell is as much a hangup as any other.
+func TestAutoSaveOnSignalWhileInShellEscape(t *testing.T) {
+	t.Parallel()
+
+	st := &shellTerm{
+		blockingTerm: newBlockingTerm(),
+		entered:      make(chan struct{}),
+		release:      make(chan struct{}),
+	}
+	g := mkBlockedGame(t, st)
+
+	left := make(chan struct{})
+
+	go func() {
+		defer close(left)
+
+		g.shell()
+	}()
+
+	<-st.entered
+
+	if !g.AutoSaveOnSignal(autoSaveWait) {
+		t.Error("the save was not taken while the game was in the shell escape")
+	}
+
+	assertRestorable(t, g.FileName)
+
+	close(st.release)
+	<-left
+}
+
+// TestShellEscapePanicUnwindsTheGameGoroutine pins the reason
+// runShellEscape recovers its helper's panic.
+//
+// term.Tcell.ShellEscape panics when Screen.Resume fails, and the shell
+// now runs on a helper goroutine. A panic reaching the top of that helper
+// would kill the process without running the deferred calls of any other
+// goroutine — including cmd/rogue/main.go's `defer t.Fini()`, which is
+// the only thing that takes the tty back out of raw mode. That is issue
+// #12's failure, and it would land on the one path where the terminal is
+// already broken.
+//
+// So the panic has to arrive on the goroutine that runs the game, with
+// that goroutine's deferred restore still on the stack. This test stands
+// in for main: a Fini deferred around the g.shell() call, and the panic
+// caught after it, asserting both that the restore ran and that the
+// original value came through. Against the unrecovered version there is
+// nothing to assert — the panic escapes a helper goroutine and takes the
+// whole test binary down, which is the failure being prevented.
+func TestShellEscapePanicUnwindsTheGameGoroutine(t *testing.T) {
+	t.Parallel()
+
+	pt := &panickingShellTerm{blockingTerm: newBlockingTerm()}
+	g := mkBlockedGame(t, pt)
+
+	caught := make(chan any, 1)
+
+	go func() {
+		// Registered first, so it runs last: it sees the terminal
+		// already restored, exactly as the runtime would have printed
+		// the trace after main's Fini.
+		defer func() { caught <- recover() }()
+
+		// Stands in for cmd/rogue/main.go's `defer t.Fini()`.
+		defer pt.Fini()
+
+		g.shell()
+	}()
+
+	got := <-caught
+
+	if got == nil {
+		t.Fatal("the resume failure did not reach the game goroutine")
+	}
+
+	if msg, ok := got.(string); !ok || msg != errShellResume {
+		t.Errorf("recovered %v, want %q", got, errShellResume)
+	}
+
+	if !pt.restored {
+		t.Error("the terminal was not restored on the way out")
+	}
+
+	// shell() must not have resumed into its InShell reset and refresh:
+	// there is no screen left to draw into.
+	if !g.InShell {
+		t.Error("shell() carried on drawing after the resume failed")
+	}
+}
+
+// TestAutoSaveOnSignalTimesOutLeavingTheOldSave pins the backstop: a game
+// goroutine that never reaches a service point must not hold the process
+// open, and giving up must cost the player nothing. The old save is still
+// there, byte for byte — which is the whole point of renaming over the
+// target instead of removing it first.
+func TestAutoSaveOnSignalTimesOutLeavingTheOldSave(t *testing.T) {
+	t.Parallel()
+
+	g := mkGame(t, 77)
+	g.FileName = filepath.Join(t.TempDir(), "rogue.save")
+
+	const old = "an older save nobody is allowed to destroy"
+
+	writeErr := os.WriteFile(g.FileName, []byte(old), 0o600)
+	if writeErr != nil {
+		t.Fatal(writeErr)
+	}
+
+	// Nothing drives the turn loop, so nothing will ever answer.
+	start := time.Now()
+
+	if g.AutoSaveOnSignal(100 * time.Millisecond) {
+		t.Error("AutoSaveOnSignal reported a save that nobody took")
+	}
+
+	if waited := time.Since(start); waited > time.Second {
+		t.Errorf("waited %v for an unanswered save, want the deadline to bound it",
+			waited)
+	}
+
+	got, readErr := os.ReadFile(g.FileName)
+	if readErr != nil {
+		t.Fatalf("the previous save was destroyed: %v", readErr)
+	}
+
+	if string(got) != old {
+		t.Error("the previous save was overwritten by a save that never ran")
+	}
+}
+
+// TestAutoSaveOnSignalWithoutASaveFile covers the death demo's terminal
+// case: a game with no file name has nothing to write, and must say so
+// rather than reporting a save that did not happen.
+func TestAutoSaveOnSignalWithoutASaveFile(t *testing.T) {
+	t.Parallel()
+
+	g := New(Params{Seed: 5, Term: &testTerm{
+		input: []byte(strings.Repeat("s . ", 200)),
+	}})
+	g.FileName = ""
+	g.startLevel()
+	g.prePlay()
+
+	var answered bool
+
+	done := make(chan struct{})
+
+	go func() {
+		defer close(done)
+
+		answered = g.AutoSaveOnSignal(autoSaveWait)
+	}()
+
+	driveUntilDone(t, g, done)
+
+	if answered {
+		t.Error("AutoSaveOnSignal = true with no save file name")
+	}
+}
+
+// TestSaveFileReplacesTargetAtomically pins the write discipline: the new
+// save arrives by rename, so the file the player already had is never
+// written into, and the temporary file it came from is not left lying in
+// the save directory.
+//
+// The load-bearing assertion is the handle opened before the save. A
+// rename leaves the old file whole and merely stops it being reachable by
+// name, so that handle still reads the old save; the truncate-in-place
+// write this replaced would empty it under the reader — the same
+// in-place write that, interrupted, left the player with a file that
+// could no longer be restored.
+func TestSaveFileReplacesTargetAtomically(t *testing.T) {
+	t.Parallel()
+
+	g := mkGame(t, 11)
+	dir := t.TempDir()
+	path := filepath.Join(dir, "rogue.save")
+
+	const old = "an older save"
+
+	writeErr := os.WriteFile(path, []byte(old), 0o600)
+	if writeErr != nil {
+		t.Fatal(writeErr)
+	}
+
+	held, openErr := os.Open(path) //nolint:gosec // G304: test temp path
+	if openErr != nil {
+		t.Fatal(openErr)
+	}
+
+	defer func() { _ = held.Close() }()
+
+	saveErr := g.saveFile(path)
+	if saveErr != nil {
+		t.Fatalf("saveFile: %v", saveErr)
+	}
+
+	kept, readErr := io.ReadAll(held)
+	if readErr != nil {
+		t.Fatalf("reading the file that was there before the save: %v", readErr)
+	}
+
+	if string(kept) != old {
+		t.Errorf("the previous save was written into rather than replaced: %q",
+			string(kept))
+	}
+
+	entries, readErr := os.ReadDir(dir)
+	if readErr != nil {
+		t.Fatal(readErr)
+	}
+
+	if len(entries) != 1 || entries[0].Name() != "rogue.save" {
+		t.Errorf("save directory = %v, want just the save file", names(entries))
+	}
+
+	info, statErr := os.Stat(path)
+	if statErr != nil {
+		t.Fatal(statErr)
+	}
+
+	if perm := info.Mode().Perm(); perm != 0o400 {
+		t.Errorf("save file mode = %v, want 0400", perm)
+	}
+
+	assertRestorable(t, path)
+}
+
+// TestSaveFileLeavesTargetWhenTheRenameFails is the other half of the
+// same discipline: a save that cannot be completed must leave what the
+// player already had. The target here is a non-empty directory, which no
+// rename can replace — the one write failure that can be forced without
+// depending on file permissions, and therefore on not being root.
+func TestSaveFileLeavesTargetWhenTheRenameFails(t *testing.T) {
+	t.Parallel()
+
+	g := mkGame(t, 12)
+	dir := t.TempDir()
+	path := filepath.Join(dir, "rogue.save")
+
+	mkErr := os.Mkdir(path, 0o700)
+	if mkErr != nil {
+		t.Fatal(mkErr)
+	}
+
+	keep := filepath.Join(path, "keep")
+
+	writeErr := os.WriteFile(keep, []byte("still here"), 0o600)
+	if writeErr != nil {
+		t.Fatal(writeErr)
+	}
+
+	saveErr := g.saveFile(path)
+	if saveErr == nil {
+		t.Error("saveFile over an unreplaceable target reported success")
+	}
+
+	_, statErr := os.Stat(keep)
+	if statErr != nil {
+		t.Errorf("the target was damaged by a failed save: %v", statErr)
+	}
+
+	entries, readErr := os.ReadDir(dir)
+	if readErr != nil {
+		t.Fatal(readErr)
+	}
+
+	if len(entries) != 1 {
+		t.Errorf("save directory = %v, want no temporary file left behind",
+			names(entries))
+	}
+}
+
+// names lists directory entry names for a failure message.
+func names(entries []os.DirEntry) []string {
+	out := make([]string, 0, len(entries))
+	for _, e := range entries {
+		out = append(out, e.Name())
+	}
+
+	return out
+}
+
+// assertRestorable checks that path holds a save this program can load,
+// which is what "the save was taken" has to mean: a file of the right
+// size proves nothing about a torn encode.
+func assertRestorable(t *testing.T, path string) {
+	t.Helper()
+
+	_, err := Restore(path, Params{Term: &testTerm{}})
+	if err != nil {
+		t.Errorf("the saved file does not restore: %v", err)
+	}
+}
+
+// mkBlockedGame builds a game with a save file name and a terminal whose
+// reads block, for the tests that park the game goroutine.
+func mkBlockedGame(t *testing.T, term Terminal) *RogueGame {
+	t.Helper()
+
+	g := New(Params{Seed: 4242, Term: term})
+	g.NewLevel()
+	g.FileName = filepath.Join(t.TempDir(), "rogue.save")
+
+	return g
+}
+
+// blockingTerm is a Terminal that genuinely blocks in ReadChar until a
+// key is pushed or Interrupt wakes it — which testTerm, whose reads never
+// block, cannot reproduce.
+type blockingTerm struct {
+	keys chan byte
+	wake chan struct{}
+}
+
+func newBlockingTerm() *blockingTerm {
+	return &blockingTerm{
+		keys: make(chan byte),
+		// Buffered by one and posted to without blocking, the same
+		// contract term.Tcell.Interrupt has with tcell's event queue: an
+		// interrupt that arrives before the read still wakes it.
+		wake: make(chan struct{}, 1),
+	}
+}
+
+func (t *blockingTerm) Render(*Window) {}
+
+func (t *blockingTerm) Fini() {}
+
+// Interrupt wakes a blocked ReadChar; called from the saving goroutine.
+func (t *blockingTerm) Interrupt() {
+	select {
+	case t.wake <- struct{}{}:
+	default:
+	}
+}
+
+// ReadChar blocks until a key arrives or Interrupt wakes it.
+func (t *blockingTerm) ReadChar() (byte, bool) {
+	select {
+	case ch := <-t.keys:
+		return ch, true
+	case <-t.wake:
+		return 0, false
+	}
+}
+
+// shellTerm is a blockingTerm that also offers a shell escape which stays
+// in the shell until the test lets it out.
+type shellTerm struct {
+	*blockingTerm
+
+	entered chan struct{}
+	release chan struct{}
+}
+
+// ShellEscape parks the caller in the "shell" until released.
+func (t *shellTerm) ShellEscape() {
+	close(t.entered)
+	<-t.release
+}
+
+// errShellResume is what panickingShellTerm panics with, standing in for
+// the value term.Tcell.ShellEscape raises when Screen.Resume fails.
+const errShellResume = "resume failed"
+
+// panickingShellTerm is a blockingTerm whose shell escape panics on the
+// way out, the way term.Tcell.ShellEscape does when the screen cannot be
+// resumed. It records whether Fini ran, which is the thing that must
+// still happen.
+type panickingShellTerm struct {
+	*blockingTerm
+
+	restored bool
+}
+
+func (t *panickingShellTerm) Fini() { t.restored = true }
+
+func (t *panickingShellTerm) ShellEscape() { panic(errShellResume) }

game/command.go

@@ -8,6 +8,13 @@ package game
 func (g *RogueGame) command() {
 	p := &g.Player
 
+	// Between turns is the one point in the loop where the game state is
+	// whole, so it is where a signal-triggered autosave is answered when
+	// the game goroutine is busy rather than waiting for a key (issue
+	// #24). The other service points are readchar (io.c) and
+	// runShellEscape, covering the two ways this goroutine can be parked.
+	g.serviceAutoSaveRequest()
+
 	ntimes := 1 // number of player moves
 	if p.On(Hasted) {
 		ntimes++
@@ -890,7 +897,7 @@ func (g *RogueGame) shell() {
 	if se, ok := g.scr.term.(interface{ ShellEscape() }); ok {
 		g.InShell = true
 
-		se.ShellEscape()
+		g.runShellEscape(se)
 
 		g.InShell = false
 		g.refresh()
@@ -898,3 +905,58 @@ func (g *RogueGame) shell() {
 		g.msg("shell escape is not available")
 	}
 }
+
+// runShellEscape runs the shell and returns when it exits, answering
+// signal-triggered autosave requests in the meantime (issue #24).
+//
+// The shell blocks for as long as the player is away — minutes, or until
+// they forget — and a line dropping while they are in it is exactly the
+// case SIGHUP autosave exists for, so this goroutine cannot simply sit
+// inside the call. The shell runs on a helper goroutine and the game
+// goroutine waits here, still the only one that ever encodes game state.
+// It draws nothing while it waits, so the suspend/resume dance is as
+// undisturbed as it was when it ran inline (ARCHITECTURE.md section 9).
+//
+// A panic out of ShellEscape must not be allowed to unwind on the helper
+// goroutine. term.Tcell.ShellEscape panics when Screen.Resume fails, and
+// a panic reaching the top of any goroutine kills the process without
+// running any *other* goroutine's deferred calls — which is where
+// cmd/rogue/main.go's `defer t.Fini()` lives. Running the shell off the
+// game goroutine would therefore have left the tty raw on exactly the
+// path where the terminal is already broken, reintroducing issue #12 on a
+// path this change created. So the helper recovers, and the value is
+// re-raised below on the game goroutine, whose stack does have Fini in
+// it. The recover deferral is registered after `defer close(done)` and so
+// runs before it, which is what publishes panicVal to the reader.
+func (g *RogueGame) runShellEscape(se interface{ ShellEscape() }) {
+	done := make(chan struct{})
+
+	var panicVal any
+
+	go func() {
+		defer close(done)
+
+		defer func() {
+			panicVal = recover()
+		}()
+
+		se.ShellEscape()
+	}()
+
+	for {
+		select {
+		case <-done:
+			if panicVal != nil {
+				// Re-raised here so the unwind passes through the game
+				// goroutine's deferred Fini. shell()'s InShell reset and
+				// refresh are skipped deliberately: there is no screen
+				// left to draw into.
+				panic(panicVal)
+			}
+
+			return
+		case req := <-g.sigSave:
+			g.runAutoSaveRequest(req)
+		}
+	}
+}

game/game.go

@@ -140,6 +140,13 @@ type RogueGame struct {
 	rogueOpts string // the ROGUEOPTS string, re-parsed by playit as in C
 	restored  bool   // game came from a save file; Run skips setup
 
+	// sigSave carries signal-triggered autosave requests from the signal
+	// goroutine to the game goroutine, which is the only one allowed to
+	// touch the state above (issue #24). Buffered by one: the handler
+	// reads exactly one signal, so there is never more than one request.
+	// See AutoSaveOnSignal and serviceAutoSaveRequest in save.go.
+	sigSave chan *autoSaveRequest
+
 	// data is the game's copy of the static tables (extern.c and friends).
 	data *gameData
 }
@@ -161,6 +168,7 @@ func New(params Params) *RogueGame {
 		Depth:     1,
 		ScorePath: params.ScorePath,
 		LastScore: -1,
+		sigSave:   make(chan *autoSaveRequest, 1),
 	}
 	g.Options = Options{
 		SeeFloor:  true,

game/io.go

@@ -166,15 +166,39 @@ func stepOk(ch byte) bool {
 
 // readchar reads and returns a character, checking for gross input errors
 // (io.c readchar).
+//
+// Waiting for a key is where the game spends nearly all of its wall
+// clock, so it is also where a signal-triggered autosave usually finds
+// it: a dropped connection lands while the player is thinking, not
+// mid-turn. Terminal.Interrupt wakes the read for exactly that, and the
+// save runs here, on the game goroutine, before reading again.
+//
+// What that buys is a snapshot taken by the goroutine that owns the
+// state, so it is internally consistent and restorable. It is never a
+// between-commands snapshot: readchar is reached from readCommand at the
+// top of a turn that has already run its BEFORE daemons and turnUpkeep,
+// and from prompts raised part-way through a command — --More--,
+// askOverwrite, getStr, the direction and pack prompts — by which point
+// the command has mutated state as well. See serviceAutoSaveRequest
+// (save.go) for the full statement of what the handoff guarantees and
+// what it costs the player.
 func (g *RogueGame) readchar() byte {
-	ch := g.scr.term.ReadChar()
+	for {
-	if ch == 3 { // ^C
+		ch, ok := g.scr.term.ReadChar()
-		g.quit(0)
+		if !ok {
+			g.serviceAutoSaveRequest()
 
-		return 27
+			continue
+		}
+
+		if ch == 3 { // ^C
+			g.quit(0)
+
+			return 27
+		}
+
+		return ch
 	}
-
-	return ch
 }
 
 // statusCache is the set of static shadow variables in io.c status() that

game/save.go

@@ -6,6 +6,8 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"path/filepath"
+	"time"
 )
 
 // save.c + state.c — game persistence. The hand-written, XOR-encrypted C
@@ -643,38 +645,207 @@ func (g *RogueGame) askOverwrite() saveAnswer {
 	}
 }
 
-// saveFile writes the saved game (save.c save_file). A failed write means
+// saveFile writes the saved game (save.c save_file).
-// a corrupt save, so the file is removed before reporting the error.
+//
+// The snapshot goes to a temporary file in the target's own directory and
+// is renamed over the target, so there is no instant at which the player
+// has no save file: until the rename the old file is whole, and after it
+// the new one is. C wrote straight over the target, and this port did the
+// same with a remove in front of it (AutoSave), so a write that failed —
+// or a signal-time save cut short by the process dying — could leave the
+// player with neither the old save nor a usable new one (issue #24).
+//
+// The temporary file is fsynced before the rename so its contents reach
+// the disk ahead of the directory entry that will point at it. The
+// directory itself is not fsynced: that would only matter for a machine
+// that loses power in the same instant, and the old save survives that
+// case anyway. A process killed mid-encode leaves its temporary file
+// behind, which is litter next to a destroyed save file, and the dot
+// prefix keeps it out of the way.
 func (g *RogueGame) saveFile(path string) error {
-	f, err := os.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o400) //nolint:gosec,lll // G304: user-chosen save path
+	f, err := os.CreateTemp(filepath.Dir(path), ".rogue-save-*")
 	if err != nil {
 		return err
 	}
 
-	encErr := gob.NewEncoder(f).Encode(g.snapshot())
+	tmp := f.Name()
-	closeErr := f.Close()
 
-	if encErr != nil || closeErr != nil {
+	writeErr := encodeSnapshot(f, g.snapshot())
-		_ = os.Remove(path) // don't leave a corrupt save behind
+	if writeErr != nil {
+		_ = os.Remove(tmp) // never leave a half-written file behind
 
-		if encErr != nil {
+		return writeErr
-			return encErr
-		}
-
-		return closeErr
 	}
 
-	return os.Chmod(path, 0o400)
+	renErr := os.Rename(tmp, path)
+	if renErr != nil {
+		_ = os.Remove(tmp)
+
+		return renErr
+	}
+
+	return nil
 }
 
-// AutoSave silently saves to the current file name; used on SIGHUP/SIGTERM
+// encodeSnapshot encodes the snapshot into an open temporary file and
-// (save.c auto_save). Best effort by design: it runs on the way out of a
+// closes it, leaving it read-only as the C game's saves were (save.c
-// dying process.
+// save_file). It never removes the file: its caller owns the cleanup, so
-func (g *RogueGame) AutoSave() {
+// that one place decides what happens to a failed write.
-	if g.FileName != "" {
+func encodeSnapshot(f *os.File, st *SaveState) error {
-		_ = os.Remove(g.FileName)
+	encErr := gob.NewEncoder(f).Encode(st)
-		_ = g.saveFile(g.FileName)
+	if encErr == nil {
+		encErr = f.Sync()
 	}
+
+	if encErr == nil {
+		encErr = f.Chmod(0o400)
+	}
+
+	closeErr := f.Close()
+
+	if encErr != nil {
+		return encErr
+	}
+
+	return closeErr
+}
+
+// autoSaveRequest is one signal-triggered autosave in flight: the signal
+// goroutine posts it and waits, the game goroutine performs the save and
+// closes done. ok is written before done is closed and read only after,
+// so the close is the happens-before edge that publishes it.
+type autoSaveRequest struct {
+	done chan struct{}
+	ok   bool
+}
+
+// AutoSaveOnSignal asks the game goroutine to autosave and waits up to
+// timeout for it to finish, reporting whether the save actually ran
+// (save.c auto_save, the SIGHUP/SIGTERM handler). It is the only entry
+// point the signal goroutine may use, and it deliberately touches no game
+// state: the gob encoder used to walk the live game tree from the signal
+// goroutine while the game goroutine was mid-turn mutating it (issue
+// #24).
+//
+// Blocked on input is the case that matters, since a dropped connection
+// is the whole reason the handler exists: the request is posted first and
+// the input read is then interrupted, so a game goroutine parked in
+// ReadChar wakes, saves in readchar, and reads again. A game goroutine
+// that is running turns instead picks the request up between turns, in
+// command; one parked in the `!` shell escape picks it up in
+// runShellEscape.
+//
+// The wait is bounded because the signal goroutine's job is to get the
+// process out. If the game goroutine is somewhere with no service point
+// at all, the deadline expires, this reports false, and the caller
+// restores the terminal and exits — leaving the player's previous save
+// file exactly as it was, which is the point of the rename in saveFile.
+func (g *RogueGame) AutoSaveOnSignal(timeout time.Duration) bool {
+	req := &autoSaveRequest{done: make(chan struct{})}
+
+	select {
+	case g.sigSave <- req:
+	default:
+		// A request is already queued and unserviced, or there is no
+		// game loop to service one; either way this one would not be
+		// answered either.
+		return false
+	}
+
+	g.scr.Interrupt()
+
+	timer := time.NewTimer(timeout)
+	defer timer.Stop()
+
+	select {
+	case <-req.done:
+		return req.ok
+	case <-timer.C:
+		return false
+	}
+}
+
+// serviceAutoSaveRequest performs a pending signal-triggered autosave, if
+// one is waiting, and otherwise returns at once. It runs on the game
+// goroutine — that is the whole design — so it must only be called where
+// that goroutine is not itself inside the encode: between turns, or while
+// parked waiting for input or for the shell escape.
+//
+// What is guaranteed, exactly: the encode runs on the one goroutine that
+// owns the state, so the snapshot is internally consistent and always
+// restorable. It is *not* guaranteed to be a between-commands snapshot.
+// Only one of the three service points gives that: the check at the top
+// of command, which runs after the previous command returned and before
+// this turn's DoDaemons(Before)/DoFuses(Before). The other two are both
+// reached from inside a command call already under way, and both cost
+// the same on restore.
+//
+// readchar is reached from prompts raised part-way through a command —
+// --More-- on the second message of a turn, askOverwrite, getStr, the
+// direction and pack prompts — and by then the command has already
+// mutated state: fight sets g.Count and g.Quiet and runs runTo before
+// any message, revealXeroc writes tp.Disguise before emitting one. The
+// ordinary top-of-turn key read in readCommand is inside command too,
+// after that turn's BEFORE daemons and turnUpkeep.
+//
+// runShellEscape is no safer. shell is an ordinary command handler ('!'
+// in the tables.go dispatch table), reached through executeCommand, so a
+// goroutine parked in the shell escape has already run this turn's
+// DoDaemons(Before), DoFuses(Before), turnUpkeep and the last-command
+// bookkeeping, and has not yet run DoDaemons(After), DoFuses(After) or
+// ringTurnEffects.
+//
+// The cost, at both: restoring re-enters playit at the top of command,
+// so the rest of that command never runs — its AFTER daemons and fuses
+// and its ring effects are lost — and the restored game opens with a
+// fresh BEFORE pass on top of the one already in the snapshot. That
+// second BEFORE pass is not free: rollwand, a live Before daemon once
+// swander has fired, ticks again and draws from the RNG every fourth
+// tick, and any Before fuse is decremented again.
+//
+// Not every consequence of that pass is shared by both, though. visuals
+// returns immediately unless g.After, and After is part of the snapshot,
+// so DVisuals never re-ticks after a shell-escape save: shell sets
+// g.After = false as its first statement, before it parks. After a
+// readchar save it usually does re-tick, because turnUpkeep sets
+// g.After = true just before the top-of-turn read; the exception is a
+// handler that clears After before prompting, as identifyTrapCommand
+// does ahead of promptDirection.
+//
+// The result is still a
+// coherent game state, one turn's worth of effects off — strictly better
+// than the torn encode this replaced, and the cost of being able to save
+// a player whose line dropped mid-prompt, or who is away in a shell, at
+// all.
+func (g *RogueGame) serviceAutoSaveRequest() {
+	select {
+	case req := <-g.sigSave:
+		g.runAutoSaveRequest(req)
+	default:
+	}
+}
+
+// runAutoSaveRequest answers one request: save, then release the waiter.
+func (g *RogueGame) runAutoSaveRequest(req *autoSaveRequest) {
+	req.ok = g.autoSave()
+
+	close(req.done)
+}
+
+// autoSave silently saves to the current file name (save.c auto_save),
+// reporting whether it wrote a save. Game-goroutine only — reach it
+// through AutoSaveOnSignal from anywhere else.
+//
+// The error is not surfaced: there is no player to tell, since the
+// terminal is on its way out, and nothing sensible to do about it. It is
+// reported to the waiting signal goroutine as a failed save rather than
+// discarded outright, which is what the old `_ =` here used to do.
+func (g *RogueGame) autoSave() bool {
+	if g.FileName == "" {
+		return false
+	}
+
+	return g.saveFile(g.FileName) == nil
 }
 
 // ErrSaveOutOfDate reports a save file from an incompatible version.
@@ -746,6 +917,7 @@ func Restore(path string, params Params) (*RogueGame, error) {
 		FileName:  path,
 		rogueOpts: params.RogueOpts,
 		restored:  true,
+		sigSave:   make(chan *autoSaveRequest, 1),
 	}
 	g.scr = NewScreen(params.Term)
 	g.Msgs.attach(g.scr, g.look, g.readchar)

game/screen.go

@@ -14,8 +14,15 @@ type Terminal interface {
 	// Render blits the window to the device.
 	Render(w *Window)
 	// ReadChar blocks for the next key, translated to Rogue's input bytes
-	// (arrows become hjkl, control keys their C0 codes).
+	// (arrows become hjkl, control keys their C0 codes). ok is false when
-	ReadChar() byte
+	// the read was woken by Interrupt instead of by a key, which is how a
+	// signal-triggered autosave reaches a game parked on input; the byte
+	// is meaningless then.
+	ReadChar() (ch byte, ok bool)
+	// Interrupt wakes a ReadChar that is blocked waiting for a key. It is
+	// the one Terminal method called from another goroutine, so an
+	// implementation must be safe to call concurrently with ReadChar.
+	Interrupt()
 	// Fini restores the device to its pre-game state (curses endwin). The
 	// game calls it on its way out, since one game run is one process.
 	Fini()
@@ -223,6 +230,15 @@ func (s *Screen) Fini() {
 	}
 }
 
+// Interrupt wakes a device read that is blocked waiting for a key, if
+// there is a device. Called from the signal goroutine; everything else on
+// Screen belongs to the game goroutine.
+func (s *Screen) Interrupt() {
+	if s.term != nil {
+		s.term.Interrupt()
+	}
+}
+
 // RefreshWin pushes an arbitrary window to the device (curses wrefresh).
 func (s *Screen) RefreshWin(w *Window) {
 	if s.term != nil {

game/term_test.go

@@ -14,18 +14,22 @@ func (t *testTerm) Render(*Window) {}
 
 func (t *testTerm) Fini() {}
 
-func (t *testTerm) ReadChar() byte {
+// Interrupt has nothing to wake: this terminal's ReadChar never blocks.
+// The blocking case has its own fake, blockingTerm in autosave_test.go.
+func (t *testTerm) Interrupt() {}
+
+func (t *testTerm) ReadChar() (byte, bool) {
 	if t.pos < len(t.input) {
 		c := t.input[t.pos]
 		t.pos++
 
-		return c
+		return c, true
 	}
 
 	t.tick++
 	if t.tick%2 == 0 {
-		return '\n'
+		return '\n', true
 	}
 
-	return ' '
+	return ' ', true
 }

term/tcell.go

@@ -79,8 +79,9 @@ func (t *Tcell) Render(w *game.Window) {
 }
 
 // ReadChar blocks for the next key, translated to the byte codes the C
-// game reads: arrows become hjkl, control keys their C0 codes.
+// game reads: arrows become hjkl, control keys their C0 codes. ok is
-func (t *Tcell) ReadChar() byte {
+// false when Interrupt woke the read instead of a key arriving.
+func (t *Tcell) ReadChar() (byte, bool) {
 	for {
 		ev := t.screen.PollEvent()
 		switch ev := ev.(type) {
@@ -88,14 +89,32 @@ func (t *Tcell) ReadChar() byte {
 			if t.last != nil {
 				t.Render(t.last)
 			}
+		case *tcell.EventInterrupt:
+			// Interrupt posted this from the signal goroutine: hand
+			// control back so the game goroutine can service a pending
+			// autosave, then it reads again.
+			return 0, false
 		case *tcell.EventKey:
 			if b, ok := translateKey(ev); ok {
-				return b
+				return b, true
 			}
 		}
 	}
 }
 
+// Interrupt wakes a ReadChar parked in PollEvent by posting an interrupt
+// event onto tcell's own event queue — the mechanism tcell provides for
+// exactly this, and the only Tcell method called from another goroutine
+// (Screen.PostEvent is a channel send, safe to call concurrently).
+//
+// Best effort by design: PostEvent fails only when the event queue is
+// full, which means the game goroutine is not parked waiting for a key,
+// and a game goroutine that is running turns reaches the between-turns
+// check on its own.
+func (t *Tcell) Interrupt() {
+	_ = t.screen.PostEvent(tcell.NewEventInterrupt(nil))
+}
+
 // translateKey converts a key event to a game input byte; ok is false
 // for keys the C game does not understand.
 func translateKey(ev *tcell.EventKey) (byte, bool) {