v0.7.9.5 — two answers to one question, and the copy nobody read

Both faults are in what 0.7.9.4 had just built, and both are the same
shape: a second copy of an answer that agreed with the first until it
didn't.

#96 — the §3.3 vote has no actor, and the screen named one anyway. The
vote is PARALLEL: every un-voted seat may vote at any moment, in any
order, one refusal ends it, and `apply.ts` says where it accepts one that
there is no actor to be. The turn chart named the last seat to move
before the timetable ran out — no more claim on the vote than anybody
else — directly above a tally correctly showing three seats outstanding.

The cause is worth more than the symptom. `currentActor(game)`
(`web/game.ts`) guarded on `status !== 'active'`; `currentActorOfState`
(`sim/view.ts`), added the same day in #95 and the one the frame calls,
did not, so it handed back whatever `clock.currentActor` was left
holding. The view now carries the guard and `currentActor` delegates to
it. That matters more than the tidiness: `currentActor` is what REFUSES
an intent, so a screen answering differently tells the table to wait on a
player the server would turn away.

The fourth of this class after Gitea#21, #22 and #94 — but the first
found by asking a view helper its question in a state the game is not
`active` in, which is the generalisation and is cheaper than finding the
fifth the same way.

#97 — narration reaches a seat once, by one path. `Frame.lines` carried
the whole log on every push to every seat, and nothing read it:
`RemoteSession` accumulates from `push.lines` alone and its `lines()`
returns that accumulator, so the log was serialised into every frame,
grew all game, and was discarded on arrival while `linesSince` sent the
same text correctly beside it.

The duplicate was masking a bug rather than merely wasting bandwidth.
`connect()` cleared `lastFrame` but not `sentLines`, so a reconnecting
seat was told "nothing new since your last push" while the browser it
answered had just reloaded from an EMPTY accumulator — the history panel
came back blank, mid-game, with the server holding the whole log. So the
two halves are one change, and the plan's instruction taken alone ("stop
passing the full game log into `frameFor()`") would have deleted a real
behaviour rather than a duplicate.

Every remaining reader of `Frame.lines` was checked before the field was
emptied: all of them are the solitaire and replay path, which builds
Frames through `snapshot()` directly and never goes near a session.

One test was wrong before the code was. The first draft of the reconnect
test connected inside its own fixture, so both sides of the comparison
were the empty array and it passed against the broken server. Each test
now asserts its premise is non-empty before comparing.

Also: `docs/plans/jitsi-common-board.md` is committed. It was never added
— not ignored, just missed — while TODO.md cites it twice as the plan for
all of v0.8.0 and the last two releases were built from it, so a clone
got a TODO pointing at a file that did not exist.

917 tests pass, up from 909.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y5boPxP6JHRYMm8adXaF5R
This commit is contained in:
Jesse.Markowitz
2026-09-07 20:35:19 -04:00
co-authored by Claude Opus 5
parent ebd16983e2
commit d5445badcc
9 changed files with 1215 additions and 15 deletions
+15 -1
View File
@@ -174,8 +174,17 @@ function buildSession(
};
let open: OpenSpan | null = openSpanFor(Date.now());
/**
* A seat's full Frame, with `lines` deliberately EMPTY (#97).
*
* Narration reaches a client by ONE path — `Push.lines` — because `RemoteSession`
* (`web/session.ts`) accumulates from that field alone and its `lines()` returns the accumulator.
* Passing `game.log` here serialised the entire log into every frame for every seat, where it grew
* all game and was thrown away on arrival, while `linesSince` correctly sent the same text beside
* it. The duplicate was not merely waste: it masked the reconnect bug `connect` fixes below.
*/
function frameFor(seat: PlayerIndex): Frame {
return snapshot(game.state, game.log, null, null, null, false, seat);
return snapshot(game.state, [], null, null, null, false, seat);
}
function linesSince(seat: PlayerIndex): { text: string; tone: string }[] {
@@ -341,6 +350,11 @@ function buildSession(
// A (re)connect always starts from a clean slate — no cache to trust across a lost connection
// (or a server restart, Phase 3) — so the honest thing is a full Frame, not a delta.
lastFrame.delete(seat);
// ...and the NARRATION watermark with it (#97). The browser this answers has just reloaded
// from an empty accumulator, so a seat told "nothing new since your last push" came back to a
// blank history panel mid-game, with the server holding the whole log. `Push.lines` on a
// connect IS the history, which is what lets the Frame stop carrying a second copy.
sentLines.delete(seat);
return pushFor(seat, null);
},
+20 -8
View File
@@ -1407,17 +1407,29 @@ export function projectDivision(s: GameState): DivisionView[] {
/**
* WHO THE GAME IS WAITING ON — the one answer, asked one way (Gitea#20 step 1).
*
* `clock.currentActor` is not it. The engine sets that null while an interruption is standing —
* a §8.1 clearance goes to the Superintendent, a Yard Office offer to the district's owner — and a
* view reading the raw field therefore reports "nobody" during exactly the moments a player is being
* waited on. `actingPlayer` knows the rule and is the engine's own answer.
* TWO THINGS HAVE TO BE TRUE AT ONCE, and each was somewhere else before #96 put them together.
*
* Exported so the player views, the bot driver and the common board all ask the same question of the
* same function rather than three near-copies of it. Measured before adding it: across six seeds and
* 3,600 decision points the raw field and this never disagreed, because both are only consulted when
* somebody is genuinely acting — so this is a guard against the next reader, not a live fix.
* `clock.currentActor` alone is not it: the engine sets that null while an interruption is standing
* — a §8.1 clearance goes to the Superintendent, a Yard Office offer to the district's owner — so a
* view reading the raw field reports "nobody" during exactly the moments a player is being waited
* on. `actingPlayer` knows that rule and is the engine's own answer to it.
*
* `actingPlayer` alone is not it either, and THIS is what #96 was: it has no status guard, so when
* the game is not running it hands back whatever `clock.currentActor` was left holding — the last
* seat to move before the timetable ran out. The §3.3 vote is the state that exposed it. That vote
* is PARALLEL, open to every un-voted seat at once, and `apply.ts` says in as many words that there
* is no actor to be; the screen named the last mover anyway, beside a tally correctly showing three
* seats outstanding.
*
* So: nobody is acting unless the game is `active`, and when it is, `actingPlayer` decides who.
*
* `currentActor(game)` in `web/game.ts` is this function taking a `Game`, and delegates to it —
* ONE answer, not two that agree until they don't. That matters more than it looks: `currentActor`
* is what refuses an intent, so a screen answering differently tells the table to wait on a player
* the server would turn away.
*/
export function currentActorOfState(s: GameState): PlayerIndex | null {
if (s.status !== 'active') return null;
return actingPlayer(s);
}
+12 -5
View File
@@ -38,6 +38,7 @@ import { cuesFor, narrate } from '../sim/narrate.ts';
import {
cardDescription,
cardName,
currentActorOfState,
describeIntent,
geometryLabel,
snapshot,
@@ -362,12 +363,18 @@ export function drain(game: Game): void {
record(game, pump(game.state));
}
/** Whose turn it is, or null if the game is over or waiting on nothing. */
/**
* Whose turn it is, or null if the game is over or waiting on nothing.
*
* `currentActorOfState` (sim/view.ts) IS this, taking the state rather than the `Game` — so this is
* the adapter and not a second copy. It used to be the second copy: it carried the status guard and
* the view's version did not, which is #96 — the turn chart named the last seat to move all the way
* through the §3.3 vote, while this function correctly refused every intent that seat could send.
* Two functions that agree until they don't are worse than one, because the disagreement surfaces
* as a screen nobody can square with the server.
*/
export function currentActor(game: Game): PlayerIndex | null {
if (game.state.status !== 'active') return null;
// `actingPlayer` (state.ts) knows which player each kind of interruption goes to — the
// Superintendent for a §8.1 clearance, the district's owner for a Yard Office offer.
return actingPlayer(game.state);
return currentActorOfState(game.state);
}
/** Every legal action right now, grouped for display. Empty when there is nothing to decide. */