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.

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.

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

@@ -1539,27 +1539,6 @@ 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.
-`readchar` is also 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. A save serviced at such a prompt freezes that command half applied;
-restoring re-enters `playit` at the top of `command`, so the rest of that
-command never runs. The result is a coherent state one turn's worth of effects
-short, which is the price of being able to save at all for a player whose line
-dropped mid-prompt.
-
-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

MEMORY.md

@@ -22,24 +22,8 @@ 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. `readchar` is reached from
-mid-command prompts (`--More--`, `askOverwrite`, `getStr`, direction and pack
-prompts) and the command has already mutated state by then, so a save taken
-there freezes that command half applied and the player loses the rest of it on
-restore. That is acceptable and documented; the false stronger claim was caught
-in review of PR #26 and must not 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.
+whole; never reintroduce a `Remove` before the write, and never encode game
+state from any goroutine but the game's.
 
 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

@@ -80,21 +80,7 @@ wizard commands).
   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 first version of this
-  work claimed in four places that nothing is half-mutated at the `readchar`
-  service point; that is false, since `readchar` is reached from mid-command
-  prompts. 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`
+  row and §9's SIGTSTP paragraph are corrected to match. `Next Step`
   deliberately not rotated: out-of-band issue work.
 
 - 2026-08-09 Signal-time terminal restore (`sig-leave`, closes #12): the port

game/autosave_test.go

@@ -161,65 +161,6 @@ func TestAutoSaveOnSignalWhileInShellEscape(t *testing.T) {
 	<-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
@@ -491,21 +432,3 @@ 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

@@ -11,8 +11,8 @@ func (g *RogueGame) command() {
 	// 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.
+	// #24). The other service points are readchar (io.c) and shell,
+	// covering the two ways this goroutine can be parked.
 	g.serviceAutoSaveRequest()
 
 	ntimes := 1 // number of player moves
@@ -916,44 +916,18 @@ func (g *RogueGame) shell() {
 // 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/io.go

@@ -171,15 +171,11 @@ func stepOk(ch byte) bool {
 // 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 not
-// necessarily a between-commands snapshot: readchar is also reached from
-// prompts raised part-way through a command — --More--, askOverwrite,
-// getStr, the direction and pack prompts — and mutation has already
-// happened by then. See serviceAutoSaveRequest (save.go) for what that
-// costs the player.
+// save runs here, on the game goroutine, before reading again. Nothing is
+// half-mutated at this point — the pending command has not run yet — so
+// the snapshot is the state as of the start of the command, the same
+// state the player would have restored to had they never answered the
+// prompt.
 func (g *RogueGame) readchar() byte {
 	for {
 		ch, ok := g.scr.term.ReadChar()

game/save.go

@@ -732,8 +732,7 @@ type autoSaveRequest struct {
 // 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.
+// command; one parked in the `!` shell escape picks it up in shell.
 //
 // 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
@@ -768,24 +767,8 @@ func (g *RogueGame) AutoSaveOnSignal(timeout time.Duration) bool {
 // 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.
-// Two of the three service points are, but 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. A save serviced at such a
-// prompt therefore freezes that command half applied; restoring re-enters
-// playit at the top of command, so the rest of that command never runs
-// and the player loses its remaining effects. That is a coherent game
-// state, one turn's worth of effects short — strictly better than the
-// torn encode this replaced, and the cost of being able to save a player
-// whose line dropped mid-prompt at all.
+// the game state is not half-mutated: between turns, or while parked
+// waiting for input or for the shell escape.
 func (g *RogueGame) serviceAutoSaveRequest() {
 	select {
 	case req := <-g.sigSave:

game/term_test.go

@@ -15,7 +15,7 @@ func (t *testTerm) Render(*Window) {}
 func (t *testTerm) Fini() {}
 
 // Interrupt has nothing to wake: this terminal's ReadChar never blocks.
-// The blocking case has its own fake, blockingTerm in autosave_test.go.
+// The blocking case has its own fake, blockingTerm in save_test.go.
 func (t *testTerm) Interrupt() {}
 
 func (t *testTerm) ReadChar() (byte, bool) {