CB-WP-0031: the comment box outlives the game

The note channel closed at the moment it is worth most — a player who has
just seen the outcome is the one with something to say, and that reading
was collectable at every moment of the game except the one after it.

Two independent defects: doc::ending never rendered the box, and
serve_end had no POST /note arm, so even a hand-built post fell through to
404. Fixing either alone leaves the channel shut, so the test asserts both
and is mutation-proven to fail on each half separately.

A post-game note binds to the final position but is not an observation
made at the last decision point. RoundStep::End is the last step of a
ROUND, not the end of the game, so record_note now takes the step as an
argument and the post-game path passes "after the end" — otherwise an
after-the-fact reading is filed as an in-play one, which is the
wrong-subject family ADR-0018 was written for.

A note does not end the session: every other POST in that loop breaks it,
and a player must be able to write a second one and then still play again.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

tools/cb-play/src/hotseat.rs

@@ -126,7 +126,19 @@ impl Server {
     /// **The state hash is the binding.** Round and step orient a reader;
     /// the hash is what lets one *reach* the position, because it is the
     /// same value that makes a session comparable to its replay.
-    fn record_note(&self, state: &GroundState, note: &Note) -> Result<(), String> {
+    /// `step` is passed in rather than read off the state (CB-WP-0031),
+    /// because a note written after the game has ended is not a note taken
+    /// at the final decision point and must not be logged as one. The
+    /// position it binds to is genuinely the final state — `round` and
+    /// `state_hash` are that state's — but *when the player said it* is a
+    /// different fact, and conflating them would file an after-the-fact
+    /// reading as an in-play observation.
+    ///
+    /// `"after the end"` cannot collide with a real step: `RoundStep`'s
+    /// `Debug` values are bare identifiers (`Select`, `Reveal`, `Resolve`,
+    /// `End`), and **`End` is the last step of a round, not the end of the
+    /// game** — which is exactly the confusion this label avoids.
+    fn record_note(&self, state: &GroundState, step: &str, note: &Note) -> Result<(), String> {
         let Some(path) = self.trial.as_ref() else {
             // No trial log configured: refuse rather than drop. A note
             // that vanishes is worse than a note that was never offered,
@@ -139,7 +151,7 @@ impl Server {
         log.push(TrialNote {
             n,
             round: state.round,
-            step: format!("{:?}", state.step),
+            step: step.to_string(),
             state_hash: hash[..12].to_string(),
             text: note.text.clone(),
         });
@@ -285,21 +297,23 @@ impl Server {
                 // conversion between them. The game does not advance here
                 // and the loop keeps waiting for the seat's actual move.
                 ("POST", "/note") => match Note::parse(&req.body) {
-                    Ok(note) => match self.record_note(state, &note) {
-                        Ok(()) => {
-                            // 303 so the browser re-GETs the table rather
-                            // than leaving a form POST in history — a
-                            // reload would otherwise re-submit the note.
-                            //
-                            // WITH THE TOKEN. Redirecting to bare `/` sent
-                            // the browser to a request control 1 refuses,
-                            // so the note was saved and the player was
-                            // shown "no session token" — which reads as
-                            // the note having failed.
-                            respond_seeother(&mut stream, &self.guard.page_path());
+                    Ok(note) => {
+                        match self.record_note(state, &format!("{:?}", state.step), &note) {
+                            Ok(()) => {
+                                // 303 so the browser re-GETs the table rather
+                                // than leaving a form POST in history — a
+                                // reload would otherwise re-submit the note.
+                                //
+                                // WITH THE TOKEN. Redirecting to bare `/` sent
+                                // the browser to a request control 1 refuses,
+                                // so the note was saved and the player was
+                                // shown "no session token" — which reads as
+                                // the note having failed.
+                                respond_seeother(&mut stream, &self.guard.page_path());
+                            }
+                            Err(e) => respond(&mut stream, 500, "text/plain", &e),
                         }
-                        Err(e) => respond(&mut stream, 500, "text/plain", &e),
-                    },
+                    }
                     Err(why) => respond(&mut stream, 400, "text/plain", &why),
                 },
                 _ => respond(&mut stream, 404, "text/plain", "no such thing here"),
@@ -327,9 +341,18 @@ impl Server {
     /// seen — the terminal page carries a `done` control and posts it —
     /// with `linger` as a bound so an abandoned tab cannot hold the
     /// process open forever.
+    ///
+    /// **`final_state` is what makes the comment box work here**
+    /// (CB-WP-0031). This loop had no `/note` arm at all, so a note posted
+    /// from the ending page fell through to `404 the game is over` — and
+    /// the ending page did not render the box in the first place, so the
+    /// channel closed at the moment a player has just learned how it went.
+    /// `None` means the game stopped without a position, and then the box
+    /// is not offered rather than offered and refused.
     pub fn serve_end(
         &self,
         final_view: Option<&games_ground::view::GroundView>,
+        final_state: Option<&GroundState>,
         message: &str,
         linger: std::time::Duration,
         tally: &crate::table::MatchTally,
@@ -365,15 +388,44 @@ impl Server {
             }
             match (req.method.as_str(), req.path.as_str()) {
                 ("GET", "/") => {
+                    let note_to = final_state.map(|_| self.guard.note_endpoint());
                     let page = cb_render_html::doc::ending(
                         final_view,
                         message,
                         &self.guard.endpoint(),
+                        note_to.as_deref(),
                         &self.log_lines(),
                         &series_lines(tally),
                     );
                     respond(&mut stream, 200, "text/html; charset=utf-8", &page);
                 }
+                // CB-WP-0031. The same second channel as in `next_choice`,
+                // and it still cannot carry a move — here it could not
+                // anyway, because the game is finished.
+                //
+                // **It does not `break`.** Every other POST here ends the
+                // loop; a note must leave the session exactly as it found
+                // it, so the player can write a second one, or read the
+                // log again, or then press `play again`.
+                ("POST", "/note") => {
+                    let Some(state) = final_state else {
+                        respond(
+                            &mut stream,
+                            409,
+                            "text/plain",
+                            "the game stopped without a final position, so a note \
+                             has nothing to bind to",
+                        );
+                        continue;
+                    };
+                    match Note::parse(&req.body) {
+                        Ok(note) => match self.record_note(state, "after the end", &note) {
+                            Ok(()) => respond_seeother(&mut stream, &self.guard.page_path()),
+                            Err(e) => respond(&mut stream, 500, "text/plain", &e),
+                        },
+                        Err(why) => respond(&mut stream, 400, "text/plain", &why),
+                    }
+                }
                 ("POST", "/command") => {
                     // CB-WP-0020 T05: the browser could not start a second
                     // game without going back to a terminal, which made it
@@ -750,6 +802,7 @@ mod tests {
         server
             .serve_end(
                 Some(&view),
+                Some(&state()),
                 "30 commands, hash f6c890a65271",
                 std::time::Duration::from_secs(20),
                 &crate::table::MatchTally::default(),
@@ -793,6 +846,104 @@ mod tests {
         assert!(replies[1].contains("closed"), "{}", replies[1]);
     }
 
+    /// CB-WP-0031. The comment box must survive the end of the game.
+    ///
+    /// **Two things go wrong independently and this asserts both.** The
+    /// ending page did not render the form, and `serve_end` had no
+    /// `/note` arm — so even a hand-built POST fell through to
+    /// `404 the game is over`. Fixing either alone leaves the channel
+    /// shut, so a test that only checked the markup would have passed
+    /// against a server that still refused.
+    ///
+    /// **And the note must not end the session.** Every other POST in
+    /// this loop breaks it; a player writing what they thought must be
+    /// able to write a second one and then still press `play again`,
+    /// which is what the trailing command here proves.
+    #[test]
+    fn a_note_can_be_written_after_the_game_has_ended() {
+        let dir = std::env::temp_dir().join(format!("cb-note-end-{}", std::process::id()));
+        let log = dir.join("trial.md");
+        let server = Server::bind(0).expect("bind").with_trial(Some(log.clone()));
+        let port = server.listener.local_addr().unwrap().port();
+        let token = server.guard.token().to_string();
+        let client = converse(
+            port,
+            vec![
+                format!("GET /?t={token} HTTP/1.1\r\nHost: 127.0.0.1:{port}\r\n\r\n"),
+                // The body is its own binding and the length is computed
+                // from it. Writing `Content-Length: 21` by hand beside a
+                // 22-byte body is a test that fails for a reason having
+                // nothing to do with the feature.
+                post(&token, "/note", "note=so+that+is+why+it"),
+                post(&token, "/command", "down=done&up=done"),
+            ],
+        );
+        let view = state().project(Viewer::Spectator);
+        server
+            .serve_end(
+                Some(&view),
+                Some(&state()),
+                "30 commands",
+                std::time::Duration::from_secs(20),
+                &crate::table::MatchTally::default(),
+            )
+            .expect("serve_end");
+        let replies = client.join().expect("client");
+
+        assert!(
+            replies[0].contains("id=\"cb-note\""),
+            "the ending page offered no comment box"
+        );
+        // 303 back to the TOKEN-CARRYING path. Redirecting to bare `/`
+        // is the defect that made a SAVED note read as a failed one.
+        assert!(
+            replies[1].contains("303"),
+            "the note was refused: {}",
+            replies[1]
+        );
+        assert!(
+            replies[1].contains(&format!("t={token}")),
+            "the redirect dropped the token, which reads as a refusal: {}",
+            replies[1]
+        );
+        // The session survived the note.
+        assert!(
+            replies[2].contains("closed"),
+            "the note ended the session, so nothing could follow it: {}",
+            replies[2]
+        );
+
+        let written = std::fs::read_to_string(&log).expect("trial log");
+        assert!(written.contains("so that is why it"), "{written}");
+        // The WHEN is not the final step. `RoundStep::End` is the last
+        // step of a round; filing an after-the-fact reading under it
+        // would claim the player said this at a decision point.
+        assert!(
+            written.contains("after the end"),
+            "a post-game note was logged as an in-play one: {written}"
+        );
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
+    /// The other half of the same decision: no position, no box.
+    ///
+    /// A crashed game has nothing to bind a note to, and offering a box
+    /// that would answer 409 is worse than not offering one.
+    #[test]
+    fn a_game_that_stopped_offers_no_comment_box() {
+        let mut s = String::new();
+        let page = cb_render_html::doc::ending(None, "it broke", "/c", None, &[], &[]);
+        s.push_str(&page);
+        assert!(
+            !s.contains("id=\"cb-note\""),
+            "offered a box with nothing to bind to"
+        );
+        assert!(
+            s.contains("nothing to bind a note to"),
+            "did not say why: {s}"
+        );
+    }
+
     /// **CB-WP-0020 T05.** "play again" must deal a second game, not just
     /// answer politely and exit — the browser could not start a second
     /// game without going back to a terminal, which made it a strictly
@@ -857,8 +1008,14 @@ mod tests {
     /// game that never happened.
     #[test]
     fn a_game_that_ended_badly_says_so_and_shows_no_table() {
-        let page =
-            cb_render_html::doc::ending(None, "P1 ran out of input", "/command?t=x", &[], &[]);
+        let page = cb_render_html::doc::ending(
+            None,
+            "P1 ran out of input",
+            "/command?t=x",
+            None,
+            &[],
+            &[],
+        );
         assert!(
             page.contains("P1 ran out of input"),
             "the reason is missing"
@@ -1105,9 +1262,17 @@ mod tests {
         )
     }
 
-    fn post(token: &str, body: &str) -> String {
+    /// One form POST. **`Content-Length` is derived from the body**, which
+    /// is the whole reason to have this rather than hand-written request
+    /// literals: the first draft of the post-game note test wrote
+    /// `Content-Length: 21` beside a 22-byte body, the server read 21 of
+    /// them, and the note came back `400 unrecognised field` — a failure
+    /// that looked exactly like the feature being broken.
+    ///
+    /// `path` because the note channel is a second endpoint (CB-WP-0031).
+    fn post(token: &str, path: &str, body: &str) -> String {
         format!(
-            "POST /command?t={token} HTTP/1.1\r\nHost: 127.0.0.1\r\n\
+            "POST {path}?t={token} HTTP/1.1\r\nHost: 127.0.0.1\r\n\
              Sec-Fetch-Site: same-origin\r\nContent-Length: {}\r\n\r\n{body}",
             body.len()
         )
@@ -1142,7 +1307,10 @@ mod tests {
         let token = server.url().rsplit("t=").next().unwrap().to_string();
         let client = converse(
             port,
-            vec![get(&token), post(&token, "down=action-ground&up=table")],
+            vec![
+                get(&token),
+                post(&token, "/command", "down=action-ground&up=table"),
+            ],
         );
 
         let choice = server
@@ -1181,7 +1349,7 @@ mod tests {
             vec![
                 "GET / HTTP/1.1\r\nHost: 127.0.0.1\r\nSec-Fetch-Site: same-origin\r\n\r\n"
                     .to_string(),
-                post(&token, "down=action-ground&up=table"),
+                post(&token, "/command", "down=action-ground&up=table"),
             ],
         );
 
@@ -1213,8 +1381,8 @@ mod tests {
         let client = converse(
             port,
             vec![
-                post(&token, "down=seat-1&up=seat-2"),
-                post(&token, "down=action-ground&up=table"),
+                post(&token, "/command", "down=seat-1&up=seat-2"),
+                post(&token, "/command", "down=action-ground&up=table"),
             ],
         );
 
@@ -1277,7 +1445,7 @@ mod tests {
             // 3. send exactly what the JavaScript produced — not what this
             //    test thinks it should have produced
             let mut s = TcpStream::connect(("127.0.0.1", port)).expect("connect");
-            s.write_all(post(&token, &posts[0].body).as_bytes())
+            s.write_all(post(&token, "/command", &posts[0].body).as_bytes())
                 .expect("write");
             let mut reply = String::new();
             let _ = s.read_to_string(&mut reply);
@@ -1327,7 +1495,7 @@ mod tests {
                 pages.push(page);
 
                 let mut s = TcpStream::connect(("127.0.0.1", port)).expect("connect");
-                s.write_all(post(&tok, "down=action-ground&up=table").as_bytes())
+                s.write_all(post(&tok, "/command", "down=action-ground&up=table").as_bytes())
                     .expect("write");
                 let mut reply = String::new();
                 let _ = s.read_to_string(&mut reply);

tools/cb-play/src/table.rs

@@ -270,7 +270,7 @@ fn end_badly<T>(
         // nothing to the series and the panel stays absent. Passing an
         // empty tally rather than the live one is deliberate: a crashed
         // game must not be counted as a played one.
-        let _ = s.serve_end(None, &msg, END_LINGER, &MatchTally::default());
+        let _ = s.serve_end(None, None, &msg, END_LINGER, &MatchTally::default());
     }
     Err(msg)
 }
@@ -434,7 +434,7 @@ fn run_game<'a, R: BufRead + 'a, W: Write + 'a>(
             tally.record(o);
         }
         let msg = format!("{} commands, hash {}", game.commands, &end_hash[..12]);
-        end_choice = srv.serve_end(Some(&ended), &msg, END_LINGER, tally)?;
+        end_choice = srv.serve_end(Some(&ended), Some(&game.state), &msg, END_LINGER, tally)?;
     }
 
     let scenario = games_ground::record::to_scenario(