diff --git a/crates/ghostlight-dungeon/src/eve.rs b/crates/ghostlight-dungeon/src/eve.rs index 62105af..2948403 100644 --- a/crates/ghostlight-dungeon/src/eve.rs +++ b/crates/ghostlight-dungeon/src/eve.rs @@ -491,11 +491,25 @@ pub(crate) fn authenticated_surface( // id), so the server resolves the open question // itself instead of trusting a client-supplied id at // all (`runtime.rs::execute_world`'s `world.play` - // arm). PA.f84's guarantee still holds: a stale - // `routeHint.sourceVersion` — one that predates the - // question the player is answering — is refused - // there, so an answer can never land on a question - // the player never saw. + // arm). PA.f170's guarantee now holds by an opaque + // per-question token rather than PA.f161's version + // comparison (see `play::PlayTurn::question_token`'s + // own doc comment for why a version alone cannot tell + // two questions in the same turn apart): the button's + // own `props.action` carries it DOM-invisibly, the + // way `props.action.command` already does — a real + // client spreads a button's own `action` fields + // straight into the payload beside `bindings` + // (`commandPayload`, + // `vendor/eve/packages/eve-browser-lowering/src/index.ts:2101`), + // never through a renderer that could show or edit + // it. `None` while no question is open: the button's + // own action then carries no token at all, matching a + // plain continue/opening, which needs none. + let play_action = play + .and_then(|view| view.question.as_ref()) + .map(|question| json!({"answerToken": question.token})) + .unwrap_or_else(|| json!({})); children.extend([ json!({ "id":"world.play.text", @@ -508,7 +522,7 @@ pub(crate) fn authenticated_surface( "world.play", "Play", "world.play", - json!({}), + play_action, &["text"], ), ]); diff --git a/crates/ghostlight-dungeon/src/play.rs b/crates/ghostlight-dungeon/src/play.rs index 3eb39ba..f229a71 100644 --- a/crates/ghostlight-dungeon/src/play.rs +++ b/crates/ghostlight-dungeon/src/play.rs @@ -43,20 +43,32 @@ use sha2::{Digest, Sha256}; use tokio::sync::{Mutex, Semaphore}; const STORE_TYPE: &str = "ghostlight.play_turn_store.v1"; -/// PA.f172: bumped from `ghostlight.play_turn_store.v1` when -/// `question_surface_version` was added to `PlayTurn` without -/// `#[serde(default)]` — the row's own shape moved, so its schema id moves -/// with it. `question_surface_version` now carries `#[serde(default)]`, so -/// this v2 reader is a compatible read of a genuine v1 row too: `open` still -/// gates strictly on the row's own declared `schema_id` (a fresh legacy -/// branch recognizes `LEGACY_STORE_SCHEMA_V1` by name, never silently), but -/// once recognized, decodes it with exactly the same struct and upgrades it -/// in place rather than discarding the turn it names as unreadable. -const STORE_SCHEMA: &str = "ghostlight.play_turn_store.v2"; +/// PA.f172 bumped this from v1 to v2 when `question_surface_version` was +/// added to `PlayTurn` without `#[serde(default)]`. PA.f170 (Cut 13) bumps it +/// again, v2 to v3: `question_surface_version` is deleted outright (the +/// version-only staleness comparison it served is gone, replaced by the +/// opaque per-question `question_token`) and `question_token` carries +/// `#[serde(default)]` in its place, for exactly the same reason — a v1 or v2 +/// row's own bytes never had either field, so the same current struct decodes +/// either as `None`. `open` still gates strictly on the row's own declared +/// `schema_id` (a legacy branch recognizes `LEGACY_STORE_SCHEMA_V1` and +/// `LEGACY_STORE_SCHEMA_V2` by name, never silently), but once recognized, +/// upgrades the row to `STORE_SCHEMA` in place rather than discarding the +/// turn it names as unreadable. A row recovered this way whose question is +/// still open (`state == AwaitingPlayer`) mints a fresh token as part of the +/// upgrade — no earlier schema ever recorded one, and the card is re-rendered +/// with whatever the row holds, so a player returning to an upgraded session +/// can still answer. +const STORE_SCHEMA: &str = "ghostlight.play_turn_store.v3"; /// The schema id every row written before `question_surface_version` existed /// carries. Recognized by `PlayTurnStore::open` as legacy, not corrupt, and -/// upgraded to `STORE_SCHEMA` in place (PA.f172). +/// upgraded to `STORE_SCHEMA` in place (PA.f172, PA.f170). const LEGACY_STORE_SCHEMA_V1: &str = "ghostlight.play_turn_store.v1"; +/// The schema id every row written between PA.f172 and PA.f170 carries: it +/// has `question_surface_version` (now deleted) but not `question_token`. +/// Recognized by `PlayTurnStore::open` as legacy, not corrupt, and upgraded +/// to `STORE_SCHEMA` in place the same way a v1 row is (PA.f170). +const LEGACY_STORE_SCHEMA_V2: &str = "ghostlight.play_turn_store.v2"; const STORE_KEY: &str = "primary"; /// The one round budget: exhausting it closes the turn and narrates, rather @@ -252,23 +264,27 @@ struct PlayTurn { calls: Vec, persona_turns: Vec<(SubjectId, PersonaTurn)>, question: Option, - /// PA.f161: the world's own `eve::surface_version` at the instant this - /// question opened (`execute_round`'s `ASK_PLAYER_TOOL` arm), set and - /// cleared alongside `question` — never re-derived from whatever the - /// world's current revision happens to be when a later request arrives. - /// This is what lets an answer be refused as stale by "older than the - /// version the question first appeared on" rather than by "not exactly - /// today's version," the comparison PA.f161 replaces. `#[serde(default)]` - /// (PA.f172, reversing the earlier PA.f95 reasoning here): a row written - /// before this field existed — a real shape on a workstation that ran - /// Dungeon before Cut 11, not a hypothetical — decodes to `None`, exactly - /// what "no question ever recorded a surface version" means for a turn - /// this old. `STORE_SCHEMA` moved to v2 alongside this default so the - /// canonical re-encode check below still holds current-schema rows to - /// their exact bytes; only the legacy v1 read path in - /// `PlayTurnStore::open` relies on this default to recover an older row. + /// PA.f170 (Cut 13): an opaque random handle minted fresh whenever + /// `execute_round`'s `ASK_PLAYER_TOOL` arm opens a question, set and + /// cleared alongside `question`. It carries no turn id, round, slot, + /// subject, or any other world handle — it is an answer's handle and + /// nothing else (invariant 8: reading it in devtools teaches nothing + /// about the world). This replaces PA.f161's `question_surface_version` + /// and the version-only staleness comparison it served: two questions + /// opened in the same turn carry the same surface version (asking is + /// conversational and never moves it), so a version comparison alone + /// cannot tell them apart — the gap Soul proved for PA.f170. Runtime's + /// `world.play` arm refuses an answer whose own token does not name this + /// one. `#[serde(default)]`, matching the earlier `question_surface_version` + /// reasoning it replaces: a row written under `LEGACY_STORE_SCHEMA_V1` or + /// `LEGACY_STORE_SCHEMA_V2` never had this field, so the same current + /// struct decodes it to `None` for either — exactly what "no question + /// ever recorded a token" means for a turn that old. `PlayTurnStore::open` + /// mints a fresh one for a recovered row whose question is still open, so + /// an upgraded session stays answerable; the canonical re-encode check + /// below still holds current-schema (v3) rows to their exact bytes. #[serde(default)] - question_surface_version: Option, + question_token: Option, refusal: Option, narration: Option, /// Set when the turn closed on a fault rather than the player's own @@ -355,20 +371,19 @@ impl PlayTurn { } } -/// `turn.open_question_id()` paired with the question's own text and the -/// surface version it opened on (PA.f134/PA.f161): `None` whenever -/// `open_question_id` is, since `turn.question` and -/// `turn.question_surface_version` are only ever set alongside it -/// (`execute_round`'s `ASK_PLAYER_TOOL` arm sets all three in the same -/// round, and `answer_turn` and `close_with_fault` both clear -/// `question`/`question_surface_version` on their own way out of -/// `AwaitingPlayer`, PA.f173). +/// `turn.open_question_id()` paired with the question's own text and its +/// opaque answer token (PA.f134/PA.f170): `None` whenever `open_question_id` +/// is, since `turn.question` and `turn.question_token` are only ever set +/// alongside it (`execute_round`'s `ASK_PLAYER_TOOL` arm sets all three in +/// the same round, and `answer_turn` and `close_with_fault` both clear +/// `question`/`question_token` on their own way out of `AwaitingPlayer`, +/// PA.f173). fn open_question_of(turn: &PlayTurn) -> Option { let id = turn.open_question_id()?; Some(OpenQuestion { id, text: turn.question.clone().unwrap_or_default(), - surface_version: turn.question_surface_version.unwrap_or(0), + token: turn.question_token.clone().unwrap_or_default(), }) } @@ -483,36 +498,38 @@ impl PlayTurnStore { } (row.clone(), state) } - // PA.f172: a row written before `question_surface_version` - // existed — named legacy, not corrupt, because it is neither: it - // is a real, previously-canonical shape this process itself - // wrote, under the schema id that was current when it did. - // `question_surface_version` now carries `#[serde(default)]`, so - // the very same `PlayTurnStoreState` decodes it correctly (the - // field reads back `None`, matching what "no question ever - // recorded a surface version" means for a turn this old). This - // branch upgrades the row to `STORE_SCHEMA` (v2) in place and - // persists that upgrade immediately, so the legacy path is only - // ever taken once per row; the canonical re-encode check does - // not apply here — a v1 row's bytes were never canonical under - // v2 and were never meant to be — it resumes for every write - // this store makes from here on. + // PA.f172/PA.f170: a row written before `question_token` existed + // — named legacy, not corrupt, because it is neither: it is a + // real, previously-canonical shape this process itself wrote, + // under the schema id that was current when it did. + // `question_token` carries `#[serde(default)]`, so the very same + // `PlayTurnStoreState` decodes it correctly (the field reads back + // `None`, matching what "no question ever recorded a token" + // means for a turn this old). `upgrade_legacy_row` mints a fresh + // token when the recovered turn's own question is still open, + // upgrades the row to `STORE_SCHEMA` (v3) in place, and persists + // that upgrade immediately, so the legacy path is only ever taken + // once per row; the canonical re-encode check does not apply + // here — a legacy row's bytes were never canonical under v3 and + // were never meant to be — it resumes for every write this store + // makes from here on. [row] if row.r#type == STORE_TYPE && row.key == STORE_KEY && row.schema_id.as_deref() == Some(LEGACY_STORE_SCHEMA_V1) => { - let mut state: PlayTurnStoreState = rmp_serde::from_slice(&row.payload) - .context("play-turn legacy v1 state is corrupt")?; - if state.schema != LEGACY_STORE_SCHEMA_V1 { - bail!("play-turn legacy v1 state schema is invalid"); - } - state.schema = STORE_SCHEMA.into(); - let upgraded_row = envelope(&state)?; - if !store.compare_and_swap_batch(std::slice::from_ref(row), vec![upgraded_row.clone()])? { - bail!("play-turn store changed while upgrading a legacy v1 row"); - } - (upgraded_row, state) + upgrade_legacy_row(&store, row, LEGACY_STORE_SCHEMA_V1)? + } + // PA.f170: a row written between PA.f172 and PA.f170 — it has + // `question_surface_version` (deleted, silently ignored on + // decode since nothing here reads it) but no `question_token`. + // Upgraded exactly like a v1 row above. + [row] + if row.r#type == STORE_TYPE + && row.key == STORE_KEY + && row.schema_id.as_deref() == Some(LEGACY_STORE_SCHEMA_V2) => + { + upgrade_legacy_row(&store, row, LEGACY_STORE_SCHEMA_V2)? } _ => bail!("play-turn store contains foreign or otherwise-unreadable records; start with a fresh store"), }; @@ -631,6 +648,50 @@ fn envelope(state: &PlayTurnStoreState) -> anyhow::Result { }) } +/// Mints a fresh opaque per-question answer token (PA.f170): a random handle +/// with no world meaning of its own — not a `QuestionId`, not a subject, +/// turn, round, or slot, and forging one only answers the forger's own +/// already-open question (invariant 8). `execute_round`'s `ASK_PLAYER_TOOL` +/// arm mints one every time a question opens; `upgrade_legacy_row` mints one +/// for a recovered legacy row whose question is still open, since no earlier +/// schema ever recorded one. +fn mint_question_token() -> String { + uuid::Uuid::new_v4().to_string() +} + +/// Shared by `PlayTurnStore::open`'s `LEGACY_STORE_SCHEMA_V1` and +/// `LEGACY_STORE_SCHEMA_V2` branches (PA.f170): decodes `row` under the +/// current `PlayTurnStoreState`/`PlayTurn` structs — both legacy schemas +/// decode through exactly the same `#[serde(default)]` fields a current- +/// schema row would — mints a fresh `question_token` when the recovered +/// turn's own question is still open (no earlier schema ever recorded one), +/// then upgrades the row to `STORE_SCHEMA` in place and persists that +/// upgrade immediately, so the legacy path is only ever taken once per row. +fn upgrade_legacy_row( + store: &OwnedRedbMessagePackBackingStore, + row: &CultCacheEnvelope, + legacy_schema: &str, +) -> anyhow::Result<(CultCacheEnvelope, PlayTurnStoreState)> { + let mut state: PlayTurnStoreState = rmp_serde::from_slice(&row.payload) + .with_context(|| format!("play-turn legacy {legacy_schema} state is corrupt"))?; + if state.schema != legacy_schema { + bail!("play-turn legacy {legacy_schema} state schema is invalid"); + } + if let Some(turn) = state.turn.as_mut() + && turn.state == PlayTurnState::AwaitingPlayer + && turn.question.is_some() + && turn.question_token.is_none() + { + turn.question_token = Some(mint_question_token()); + } + state.schema = STORE_SCHEMA.into(); + let upgraded_row = envelope(&state)?; + if !store.compare_and_swap_batch(std::slice::from_ref(row), vec![upgraded_row.clone()])? { + bail!("play-turn store changed while upgrading a legacy {legacy_schema} row"); + } + Ok((upgraded_row, state)) +} + #[derive(Debug, thiserror::Error)] pub(crate) enum PlayError { #[error("the play table is poisoned by a prior kernel invariant fault; restart to clear it")] @@ -687,10 +748,13 @@ pub(crate) enum PlayError { pub(crate) struct OpenQuestion { pub(crate) id: QuestionId, pub(crate) text: String, - /// PA.f161: `eve::surface_version` at the moment this question opened — - /// the value `runtime.rs`'s `world.play` arm compares an answer's own - /// `routeHint.sourceVersion` against, refusing anything older. - pub(crate) surface_version: u64, + /// PA.f170: the opaque per-question answer token minted when this + /// question opened — the value `runtime.rs`'s `world.play` arm compares + /// an answer's own `answerToken` against, refusing anything else as + /// stale. Replaces PA.f161's `surface_version`, which only ever told + /// apart questions that opened on different world revisions, not two + /// questions opened in the same turn. + pub(crate) token: String, } /// One `run` call's own outcome (PA.f109): `Replayed` names the turn a @@ -994,7 +1058,7 @@ impl PlayTable { question: open_question.map(|id| OpenQuestion { id, text: existing.question.clone().unwrap_or_default(), - surface_version: existing.question_surface_version.unwrap_or(0), + token: existing.question_token.clone().unwrap_or_default(), }), }); } @@ -1158,7 +1222,7 @@ impl PlayTable { calls: Vec::new(), persona_turns: Vec::new(), question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, @@ -1178,7 +1242,7 @@ impl PlayTable { } } turn.question = None; - turn.question_surface_version = None; + turn.question_token = None; turn.state = PlayTurnState::Running; turn } @@ -1209,7 +1273,7 @@ impl PlayTable { // on `state == AwaitingPlayer`, which a fault always leaves behind by // setting `state = Closed` in the same write. turn.question = None; - turn.question_surface_version = None; + turn.question_token = None; self.persist(turn).await } @@ -1485,25 +1549,16 @@ impl PlayTable { None, ); turn.question = Some(question); - // PA.f161/PA.f175: the world's own surface version at the - // instant this question opens, so a later answer is judged - // against the version the question actually first appeared - // on, never whatever the world's version happens to be when - // the answer arrives. This must be a fresh snapshot taken - // right here, not the round-entry `snapshot` above: a round - // that authors (a `PLAY_TOOLS` run, committed earlier in this - // same `while` loop) and then asks would otherwise record the - // version as of before its own commits, not the version the - // question actually opened on. - let question_snapshot = match self.fetch_round_snapshot().await { - Ok(snapshot) => snapshot, - Err(detail) => { - self.close_with_fault(turn, detail).await?; - return Ok(RoundOutcome::Closed); - } - }; - turn.question_surface_version = - Some(crate::eve::surface_version(Some(&question_snapshot))); + // PA.f170: a fresh opaque token, minted the instant this + // question opens, binds an answer to *this* question rather + // than to a world revision — PA.f161's `surface_version` + // told apart questions that opened on different revisions, + // but two questions opened in the same turn share one + // revision (asking is conversational and never moves it), so + // that comparison alone could not tell them apart. Unlike + // the version it replaces, the token needs no snapshot: it + // carries no world handle at all. + turn.question_token = Some(mint_question_token()); turn.state = PlayTurnState::AwaitingPlayer; return Ok(RoundOutcome::AwaitingPlayer); } @@ -3292,52 +3347,41 @@ pub(crate) mod tests { ); } - /// PA.f175: `execute_round` takes one snapshot at round entry - /// (`round_snapshot`) and, before this fix, reused it for every arm in - /// that same round — including `ASK_PLAYER_TOOL` — so a round that - /// commits an authoring call (a `PLAY_TOOLS` run) and then asks in the - /// same round recorded `question_surface_version` from before its own - /// commit. A later "is this answer stale" comparison - /// (`runtime.rs::an_answer_naming_a_version_older_than_the_question_is_refused_as_stale`) - /// judges a version the question's own commit had already passed, so an - /// answer naming the version the commit actually landed at — the version - /// a real client would have just been served — could be wrongly refused - /// as older than the question. + /// PA.f170: `execute_round`'s `ASK_PLAYER_TOOL` arm mints a fresh + /// `question_token` every time a question opens, never reusing one + /// across two distinct questions in the same turn. The version it + /// replaces (PA.f161's `surface_version`) could not make this promise: + /// asking is conversational and never moves the world's own revision, so + /// two questions opened in the same turn — even in two different rounds, + /// with nothing committed between them — would have recorded the same + /// version. This is the token's own reason to exist: it, not a version, + /// is what tells the two questions apart at `runtime.rs`'s own boundary. /// - /// Mutation: revert `execute_round`'s `ASK_PLAYER_TOOL` arm to record - /// `crate::eve::surface_version(Some(&snapshot))` (the round-entry - /// snapshot) instead of fetching a fresh one at the moment the question - /// opens — this test's asserted version would then equal the version - /// from before the round's own `declare_fact` commit, one behind the - /// version this test expects. + /// Mutation: have `execute_round`'s `ASK_PLAYER_TOOL` arm reuse + /// `turn.question_token` when it is already `Some` instead of calling + /// `mint_question_token()` unconditionally — this test's two tokens + /// would then be equal. #[tokio::test] - async fn a_round_that_authors_and_then_asks_records_the_version_as_of_the_question_not_the_round() { - let fixture = play_world(None, "player-author-then-ask").await; + async fn two_questions_in_one_turn_mint_two_distinct_tokens() { + let fixture = play_world(None, "player-two-questions-distinct-tokens").await; let directory = tempfile::tempdir().unwrap(); let store_path = directory.path().join("play-turn-v1.cc"); - let world_before = fixture.world.snapshot().await.unwrap(); - let version_before = crate::eve::surface_version(Some(&world_before)); - - let round = output( + let ask_one = output( "r0", - vec![ - call_event( - "c0", - "declare_fact", - serde_json::json!({ - "handle": "ruling", - "label": "The Ruling", - "statement": "The gate creaks open on its own.", - "standing": {"standing": "ruled"}, - }), - ), - call_event( - "c1", - ASK_PLAYER_TOOL, - serde_json::json!({"question": "Which way?"}), - ), - ], + vec![call_event( + "c0", + ASK_PLAYER_TOOL, + serde_json::json!({"question": "Which way?"}), + )], + ); + let ask_two = output( + "r1", + vec![call_event( + "c1", + ASK_PLAYER_TOOL, + serde_json::json!({"question": "Are you sure?"}), + )], ); let personas = PersonaLane::new( ControllerPort::new(fixture.world.clone()), @@ -3349,7 +3393,7 @@ pub(crate) mod tests { let table = PlayTable::new( fixture.world.clone(), personas, - ScriptedPort::new(vec![round]), + ScriptedPort::new(vec![ask_one, ask_two]), "gpt-5.6-terra".into(), Arc::new(Semaphore::new(2)), &store_path, @@ -3360,40 +3404,43 @@ pub(crate) mod tests { .run(&fixture.principal, test_turn_id(91), "I stand at a crossroads.".into()) .await .unwrap(); + let first_view = table.current_turn_view().await.unwrap(); + let first_question = first_view.question.clone().expect("Q1 must be open"); + assert_eq!(first_question.text, "Which way?"); - let asked_view = table.current_turn_view().await.unwrap(); - let question = asked_view - .question - .as_ref() - .expect("the scripted ask_player call must open a question"); + table + .run( + &fixture.principal, + test_turn_id(92), + PlayRequest { + text: "I go left.".into(), + answers: Some(first_question.id.clone()), + }, + ) + .await + .unwrap(); + let second_view = table.current_turn_view().await.unwrap(); + let second_question = second_view.question.clone().expect("Q2 must be open"); + assert_eq!(second_question.text, "Are you sure?"); - let world_after = fixture.world.snapshot().await.unwrap(); - let version_after = crate::eve::surface_version(Some(&world_after)); - assert!( - version_after > version_before, - "declare_fact must have committed and moved the world's own revision \ - ({version_before} -> {version_after})" - ); - assert_eq!( - question.surface_version, version_after, - "the question's own recorded version must be the version as of when it opened \ - (after this round's own declare_fact commit, {version_after}), not the \ - round-entry version {version_before}" + assert_ne!( + first_question.token, second_question.token, + "two distinct questions in the same turn must never share an answer token" ); } /// PA.f173: `close_with_fault` used to leave `question` and - /// `question_surface_version` set on the turn it closed, though a - /// `Closed` turn should never still carry an open question. This was - /// unobservable through `current_turn_view`/`open_question_id` only - /// because both gate on `state == AwaitingPlayer`, which closing always - /// leaves behind in the same write — so this test reaches past that - /// gate, at the raw persisted row. + /// `question_token` set on the turn it closed, though a `Closed` turn + /// should never still carry an open question. This was unobservable + /// through `current_turn_view`/`open_question_id` only because both gate + /// on `state == AwaitingPlayer`, which closing always leaves behind in + /// the same write — so this test reaches past that gate, at the raw + /// persisted row. /// - /// Mutation: remove the `turn.question = None; turn.question_surface_version - /// = None;` lines from `close_with_fault` — the persisted row would then - /// still carry the question's own text and version after the turn - /// closed, and this test's assertions would fail. + /// Mutation: remove the `turn.question = None; turn.question_token = + /// None;` lines from `close_with_fault` — the persisted row would then + /// still carry the question's own text and token after the turn closed, + /// and this test's assertions would fail. #[tokio::test] async fn close_with_fault_clears_the_open_question_off_the_persisted_row() { let fixture = play_world(None, "player-fault-clears-question").await; @@ -3434,7 +3481,7 @@ pub(crate) mod tests { let mut turn = table.store.lock().await.current().unwrap().clone(); assert!(turn.question.is_some()); - assert!(turn.question_surface_version.is_some()); + assert!(turn.question_token.is_some()); table .close_with_fault(&mut turn, "manufactured fault for PA.f173".into()) .await @@ -3447,15 +3494,15 @@ pub(crate) mod tests { "a closed turn must not still carry the question it closed on top of" ); assert_eq!( - persisted.question_surface_version, None, - "a closed turn must not still carry the question's own surface version" + persisted.question_token, None, + "a closed turn must not still carry the question's own answer token" ); } /// A pre-Cut-11 process's own `PlayTurn`, replicated field-for-field - /// minus `question_surface_version` — the shape every row actually - /// written under `LEGACY_STORE_SCHEMA_V1` carries (PA.f172). Test-only: - /// no production writer ever mints this shape again. + /// minus `question_surface_version`/`question_token` — the shape every + /// row actually written under `LEGACY_STORE_SCHEMA_V1` carries (PA.f172). + /// Test-only: no production writer ever mints this shape again. #[derive(Clone, Serialize)] struct LegacyPlayTurnV1 { turn_id: String, @@ -3480,14 +3527,44 @@ pub(crate) mod tests { revision: u64, } - /// PA.f172: a row written before `question_surface_version` existed — - /// under `LEGACY_STORE_SCHEMA_V1`, the schema id every pre-Cut-11 process + /// A between-PA.f172-and-PA.f170 process's own `PlayTurn`: it has + /// `question_surface_version` (deleted by this cut) but not + /// `question_token` — the shape every row actually written under + /// `LEGACY_STORE_SCHEMA_V2` carries. Test-only: no production writer + /// ever mints this shape again. + #[derive(Clone, Serialize)] + struct LegacyPlayTurnV2 { + turn_id: String, + applied_keys: Vec, + opening_prompt: String, + player_prose: Vec, + rounds: Vec, + calls: Vec, + persona_turns: Vec<(SubjectId, PersonaTurn)>, + question: Option, + question_surface_version: Option, + refusal: Option, + narration: Option, + fault: Option, + state: PlayTurnState, + } + + #[derive(Clone, Serialize)] + struct LegacyPlayTurnStoreStateV2 { + schema: String, + turn: Option, + ledger: KeyLedger, + revision: u64, + } + + /// PA.f172: a row written before `question_token` existed — under + /// `LEGACY_STORE_SCHEMA_V1`, the schema id every pre-Cut-11 process /// actually wrote (it is exactly what `STORE_SCHEMA` used to be) — is /// recognized by `PlayTurnStore::open` as legacy, not corrupt, and its /// turn recovered rather than discarded: the missing field decodes to - /// `None`, exactly what "no question ever recorded a surface version" - /// means for a turn this old. The row is also upgraded to `STORE_SCHEMA` - /// in place, durably, so the legacy path is only ever taken once. + /// `None`, exactly what "no question ever recorded a token" means for a + /// turn this old. The row is also upgraded to `STORE_SCHEMA` in place, + /// durably, so the legacy path is only ever taken once. /// /// Mutation: remove the `schema_id.as_deref() == /// Some(LEGACY_STORE_SCHEMA_V1)` arm from `PlayTurnStore::open` — this @@ -3542,8 +3619,9 @@ pub(crate) mod tests { assert_eq!(recovered.turn_id, "11111111-1111-1111-1111-111111111111"); assert_eq!(recovered.opening_prompt, "You stand at the gate."); assert_eq!( - recovered.question_surface_version, None, - "a pre-Cut-11 turn never recorded a surface version; the missing field must decode to None" + recovered.question_token, None, + "a pre-Cut-11 turn never recorded a token, and its own question is not open (Running); \ + the missing field must decode to None and no token must be minted for it" ); assert_eq!( store.row.schema_id.as_deref(), @@ -3563,6 +3641,152 @@ pub(crate) mod tests { assert_eq!(reopened.current().unwrap().turn_id, recovered_turn_id); } + /// PA.f170: the same recognition and in-place upgrade `a_legacy_v1_row_*` + /// proves, for a row written between PA.f172 and PA.f170 instead — it + /// carries `question_surface_version` (now deleted, silently ignored on + /// decode) but not `question_token`. + /// + /// Mutation: remove the `schema_id.as_deref() == + /// Some(LEGACY_STORE_SCHEMA_V2)` arm from `PlayTurnStore::open` — this + /// row would then fall through to the catch-all and `open` would return + /// `Err`, silently disabling play on any workstation that ran Dungeon + /// between Cut 11 and Cut 13. + #[tokio::test] + async fn a_legacy_v2_row_is_recognized_and_upgraded_not_refused_as_corrupt() { + let directory = tempfile::tempdir().unwrap(); + let store_path = directory.path().join("play-turn-v1.cc"); + + let legacy_turn = LegacyPlayTurnV2 { + turn_id: "22222222-2222-2222-2222-222222222222".into(), + applied_keys: vec!["key-b".into()], + opening_prompt: "You stand at the gate.".into(), + player_prose: vec!["I knock.".into()], + rounds: Vec::new(), + calls: Vec::new(), + persona_turns: Vec::new(), + question: None, + question_surface_version: None, + refusal: None, + narration: None, + fault: None, + state: PlayTurnState::Running, + }; + let legacy_state = LegacyPlayTurnStoreStateV2 { + schema: LEGACY_STORE_SCHEMA_V2.into(), + turn: Some(legacy_turn), + ledger: KeyLedger::default(), + revision: 4, + }; + let legacy_row = CultCacheEnvelope { + key: STORE_KEY.into(), + r#type: STORE_TYPE.into(), + payload: rmp_serde::to_vec_named(&legacy_state).unwrap(), + stored_at: Utc::now().to_rfc3339(), + schema_id: Some(LEGACY_STORE_SCHEMA_V2.into()), + }; + { + let raw_store = OwnedRedbMessagePackBackingStore::new(&store_path).unwrap(); + assert!( + raw_store + .compare_and_swap_batch(&[], vec![legacy_row]) + .unwrap(), + "seeding the legacy row must succeed against an empty store" + ); + } + + let store = PlayTurnStore::open(&store_path) + .expect("a genuine v2 row must open, not be refused as corrupt"); + let recovered = store.current().expect("the legacy row's own turn must be recovered"); + assert_eq!(recovered.turn_id, "22222222-2222-2222-2222-222222222222"); + assert_eq!( + recovered.question_token, None, + "a v2 turn never recorded a token, and its own question is not open (Running); \ + the missing field must decode to None and no token must be minted for it" + ); + assert_eq!( + store.row.schema_id.as_deref(), + Some(STORE_SCHEMA), + "the legacy row must be upgraded to the current schema in place, not left at v2" + ); + } + + /// PA.f170: an upgraded row whose question is still open gets a freshly + /// minted token as part of the upgrade — no earlier schema ever recorded + /// one, and the card is re-rendered with whatever the row holds, so a + /// player returning to an upgraded session (Raven's workstation, named in + /// the cut's own spec) must still be able to answer. + /// + /// Mutation: remove the mint-if-open branch from `upgrade_legacy_row` — + /// the recovered turn's own `question_token` would then stay `None` while + /// `AwaitingPlayer`, and the answer this test goes on to submit would be + /// refused as stale by `runtime.rs`'s own boundary (no token to match) + /// even though the question is genuinely still open. + #[tokio::test] + async fn an_upgraded_legacy_row_whose_question_is_still_open_mints_a_fresh_token() { + let directory = tempfile::tempdir().unwrap(); + let store_path = directory.path().join("play-turn-v1.cc"); + + let legacy_turn = LegacyPlayTurnV1 { + turn_id: "33333333-3333-3333-3333-333333333333".into(), + applied_keys: vec!["key-c".into()], + opening_prompt: "You stand at the gate.".into(), + player_prose: vec!["I knock.".into()], + rounds: vec![output( + "r0", + vec![call_event( + "c0", + ASK_PLAYER_TOOL, + serde_json::json!({"question": "Which way?"}), + )], + )], + calls: vec![CallRecord { + call_id: "c0".into(), + round: 0, + slot: 0, + body: Some(RecordedCall::AskPlayer("Which way?".into())), + result: None, + }], + persona_turns: Vec::new(), + question: Some("Which way?".into()), + refusal: None, + narration: None, + fault: None, + state: PlayTurnState::AwaitingPlayer, + }; + let legacy_state = LegacyPlayTurnStoreStateV1 { + schema: LEGACY_STORE_SCHEMA_V1.into(), + turn: Some(legacy_turn), + ledger: KeyLedger::default(), + revision: 5, + }; + let legacy_row = CultCacheEnvelope { + key: STORE_KEY.into(), + r#type: STORE_TYPE.into(), + payload: rmp_serde::to_vec_named(&legacy_state).unwrap(), + stored_at: Utc::now().to_rfc3339(), + schema_id: Some(LEGACY_STORE_SCHEMA_V1.into()), + }; + { + let raw_store = OwnedRedbMessagePackBackingStore::new(&store_path).unwrap(); + assert!( + raw_store + .compare_and_swap_batch(&[], vec![legacy_row]) + .unwrap(), + "seeding the legacy row must succeed against an empty store" + ); + } + + let store = PlayTurnStore::open(&store_path) + .expect("a genuine pre-Cut-11 row must open, not be refused as corrupt"); + let recovered = store.current().expect("the legacy row's own turn must be recovered"); + assert_eq!(recovered.state, PlayTurnState::AwaitingPlayer); + let token = recovered + .question_token + .clone() + .expect("an upgraded row whose question is still open must mint a fresh token"); + assert!(!token.is_empty(), "a minted token must not be the empty-string fallback"); + } + /// PA.f172's own negative: a row that matches the current schema id but /// whose bytes do not decode at all is still refused as corrupt — the /// legacy path recognizes only the one schema id every pre-Cut-11 @@ -3631,7 +3855,7 @@ pub(crate) mod tests { calls: Vec::new(), persona_turns: Vec::new(), question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, @@ -5085,7 +5309,7 @@ pub(crate) mod tests { }], persona_turns: Vec::new(), question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, @@ -5211,7 +5435,7 @@ pub(crate) mod tests { }], persona_turns: Vec::new(), question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, @@ -5290,7 +5514,7 @@ pub(crate) mod tests { }], persona_turns: Vec::new(), question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, @@ -6069,7 +6293,7 @@ pub(crate) mod tests { }], persona_turns: Vec::new(), question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, @@ -6760,7 +6984,7 @@ pub(crate) mod tests { // need a `persona_turns` entry here. persona_turns: vec![(a, fake_persona_turn("a acted")), (b, fake_persona_turn("b acted"))], question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, @@ -6794,7 +7018,7 @@ pub(crate) mod tests { }], persona_turns: Vec::new(), question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, @@ -6969,7 +7193,7 @@ pub(crate) mod tests { // speak call below quotes from. persona_turns: vec![(mara_subject, fake_persona_turn(QUOTE))], question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, @@ -7309,7 +7533,7 @@ pub(crate) mod tests { calls: Vec::new(), persona_turns: vec![(mara, fake_persona_turn(PROSE))], question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, @@ -7645,7 +7869,7 @@ pub(crate) mod tests { calls: Vec::new(), persona_turns: Vec::new(), question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, @@ -8099,7 +8323,7 @@ pub(crate) mod tests { calls: Vec::new(), persona_turns: Vec::new(), question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, @@ -8200,7 +8424,7 @@ pub(crate) mod tests { calls: Vec::new(), persona_turns: Vec::new(), question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, @@ -8250,7 +8474,7 @@ pub(crate) mod tests { calls: Vec::new(), persona_turns: Vec::new(), question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, @@ -8318,7 +8542,7 @@ pub(crate) mod tests { calls: Vec::new(), persona_turns: Vec::new(), question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: Some("The end.".into()), fault: None, @@ -8736,7 +8960,7 @@ pub(crate) mod tests { calls: Vec::new(), persona_turns: Vec::new(), question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, @@ -9364,7 +9588,7 @@ pub(crate) mod tests { }], persona_turns: vec![(mara, fake_persona_turn(PROSE))], question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, @@ -9480,7 +9704,7 @@ pub(crate) mod tests { ], persona_turns: vec![(mara, fake_persona_turn(PROSE))], question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, @@ -9770,7 +9994,7 @@ pub(crate) mod tests { calls: Vec::new(), persona_turns: Vec::new(), question: None, - question_surface_version: None, + question_token: None, refusal: None, narration: None, fault: None, diff --git a/crates/ghostlight-dungeon/src/runtime.rs b/crates/ghostlight-dungeon/src/runtime.rs index abe28b1..2d2db0e 100644 --- a/crates/ghostlight-dungeon/src/runtime.rs +++ b/crates/ghostlight-dungeon/src/runtime.rs @@ -149,29 +149,43 @@ struct CreatePayload { /// `world.activate`, whose actions capture nothing) passes through /// unchanged — `commandPayload` on the lowering's own side never sets /// `bindings` when `captureBindings` is empty. -/// Refuses a payload that carries both the envelope `bindings` object and -/// flat sibling fields (PA.f168): a real client's payload is `{"bindings": -/// {...}}` alone (`commandPayload` on the lowering's own side never adds a -/// sibling), so a sibling next to `bindings` names a shape nothing produces -/// on purpose — silently keeping only `bindings` would discard it instead of -/// surfacing the mismatch. -fn unwrap_bindings(payload: &Value) -> Result<&Value, String> { +/// PA.f168's original rule refused any sibling next to the envelope +/// `bindings` object outright. PA.f170 changes what a real client's payload +/// carries: a button's own `props.action` fields (Cut 13's per-question +/// answer token, for `world.play`) ride the *same* top-level payload object +/// `commandPayload` builds — `{...action, bindings: {...captured}}` — so a +/// sibling next to `bindings` is now the shape the real lowering produces on +/// purpose, not a mismatch to surface. This merges the two into one flat +/// object instead: `bindings`' own fields plus every sibling field, so a +/// payload struct's ordinary `#[serde(deny_unknown_fields)]` still catches a +/// genuinely unknown field exactly as it did before. A field named by both +/// `bindings` and a sibling is refused outright — that shape nothing on the +/// real client's own side produces, and silently preferring one over the +/// other would hide the collision instead of surfacing it. +fn unwrap_bindings(payload: &Value) -> Result { let Some(object) = payload.as_object() else { - return Ok(payload); + return Ok(payload.clone()); }; - match object.get("bindings") { - Some(bindings) if bindings.is_object() => { - if object.len() != 1 { - return Err( - "payload carries an envelope 'bindings' object alongside flat sibling \ - fields; send one or the other, never both" - .into(), - ); - } - Ok(bindings) + let Some(bindings_object) = object.get("bindings").and_then(Value::as_object) else { + return Ok(payload.clone()); + }; + let mut merged = serde_json::Map::with_capacity(object.len() + bindings_object.len()); + for (key, value) in object { + if key == "bindings" { + continue; } - _ => Ok(payload), + if bindings_object.contains_key(key) { + return Err(format!( + "payload's envelope 'bindings' object and its own sibling action field both \ + name '{key}'; send it in exactly one place" + )); + } + merged.insert(key.clone(), value.clone()); } + for (key, value) in bindings_object { + merged.insert(key.clone(), value.clone()); + } + Ok(Value::Object(merged)) } /// Cut 10 (PA.f149): every one of these three fields is edited in a plain @@ -229,19 +243,29 @@ fn is_handle_shape(value: &str) -> bool { && chars.all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '_' || c == '-') } -/// `world.play`'s own payload (Cut 8b): the player's prose alone. Cut 10 -/// (PA.f151) deleted the client-supplied `answers` field: the vendored Eve -/// browser lowering has no authored, non-editable binding value (`hidden` is -/// not a prop any renderer reads), so the id-carrying control it used to -/// name would have rendered as a plain, visible, editable text box holding -/// the raw `QuestionId` — a leak, and once editable, a forgeable one. The -/// server resolves the currently open question itself now -/// (`execute_world`'s `world.play` arm), refusing a stale -/// `routeHint.sourceVersion` instead of trusting a client-named id. +/// `world.play`'s own payload. Cut 10 (PA.f151) deleted the client-supplied +/// `answers` field: the vendored Eve browser lowering has no authored, +/// non-editable binding value (`hidden` is not a prop any renderer reads), so +/// the id-carrying control it used to name would have rendered as a plain, +/// visible, editable text box holding the raw `QuestionId` — a leak, and once +/// editable, a forgeable one. The server still resolves the currently open +/// question itself (`execute_world`'s `world.play` arm), but PA.f170 replaces +/// the version-only staleness check that once stood in for identity: +/// `answer_token` is the opaque per-question token `play::ASK_PLAYER_TOOL` +/// mints when the question opens, carried DOM-invisibly in the "Play" +/// button's own `props.action` (`eve::authenticated_surface`) rather than +/// through a `captureBindings` control — the real lowering spreads a button's +/// `action` fields straight into the payload alongside `bindings` +/// (`commandPayload`, `vendor/eve/packages/eve-browser-lowering/src/index.ts:2101`). +/// Absent or not naming the currently open question's own token, the answer +/// is refused as stale, never applied to whatever question happens to be +/// open when the request is processed. #[derive(Debug, Deserialize)] -#[serde(deny_unknown_fields)] +#[serde(deny_unknown_fields, rename_all = "camelCase")] struct PlayPayload { text: String, + #[serde(default)] + answer_token: Option, } #[derive(Debug, Deserialize)] @@ -1199,7 +1223,7 @@ async fn execute_world( .unwrap_or(""), )?; if invocation.operation.operation_id == "world.create" { - let payload: CreatePayload = serde_json::from_value(unwrap_bindings(&invocation.payload).map_err(RuntimeCommandError::Payload)?.clone()) + let payload: CreatePayload = serde_json::from_value(unwrap_bindings(&invocation.payload).map_err(RuntimeCommandError::Payload)?) .map_err(|error| RuntimeCommandError::Payload(error.to_string()))?; if let Some(root) = payload .jurisdictions @@ -1259,24 +1283,25 @@ async fn execute_world( "only the world's owner may play".into(), )); } - let payload: PlayPayload = serde_json::from_value(unwrap_bindings(&invocation.payload).map_err(RuntimeCommandError::Payload)?.clone()) + let payload: PlayPayload = serde_json::from_value(unwrap_bindings(&invocation.payload).map_err(RuntimeCommandError::Payload)?) .map_err(|error| RuntimeCommandError::Payload(error.to_string()))?; let table = state .play .clone() .ok_or_else(|| RuntimeCommandError::Payload("the play table is unavailable".into()))?; // The server resolves the open question itself — the client no - // longer supplies one at all. No question open: this is a plain + // longer names it by `QuestionId`. No question open: this is a plain // continue/opening, `answers: None`. A question open: the caller - // must be answering it. PA.f84's guarantee (an answer cannot land on - // a question the player never saw) holds by refusing a - // `routeHint.sourceVersion` older than `question.surface_version` — - // the world's own `eve::surface_version` at the instant this - // question opened (`play.rs`'s `execute_round`, PA.f161) — the only - // comparison this makes. A real client always echoes the most recent - // `version` it was actually served, so this can only refuse a - // genuinely stale answer, never one served after the question - // appeared, regardless of how many world commits landed in between. + // must be answering it, and PA.f170's guarantee (an answer cannot + // land on a question the player never saw) is judged by the opaque + // per-question token the question's own render carried in the "Play" + // button's `props.action` (`eve::authenticated_surface`), not by + // `routeHint.sourceVersion`: two questions opened in the same turn + // carry the same surface version (asking is conversational and never + // moves it), so a version-only comparison cannot tell them apart — + // exactly the gap Soul proved (PA.f170). A token that is absent, or + // does not name the currently open question's own token, is refused + // as stale. let view = table.current_turn_view().await; // PA.f155: `PlayTurnView::state` is the actual rule this branches // on — the turn is currently waiting on the player — rather than @@ -1289,11 +1314,10 @@ async fn execute_world( let answers = match view.as_ref().and_then(|view| view.question.as_ref()).filter(|_| is_awaiting_player) { None => None, Some(question) => { - let source_version = invocation.operation.route_hint.source_version.unwrap_or(0); - if source_version < question.surface_version { + if payload.answer_token.as_deref() != Some(question.token.as_str()) { return Err(RuntimeCommandError::Payload(format!( - "the answer is stale: it names a surface older than the one the question \ - \"{}\" first appeared on", + "the answer is stale: its token does not name the question \"{}\" \ + currently open", question.text ))); } @@ -1365,17 +1389,17 @@ async fn execute_world( })?; let body = match invocation.operation.operation_id.as_str() { "world.approve" => { - serde_json::from_value::(unwrap_bindings(&invocation.payload).map_err(RuntimeCommandError::Payload)?.clone()) + serde_json::from_value::(unwrap_bindings(&invocation.payload).map_err(RuntimeCommandError::Payload)?) .map_err(|error| RuntimeCommandError::Payload(error.to_string()))?; CommandBody::ApproveDraft } "world.activate" => { - serde_json::from_value::(unwrap_bindings(&invocation.payload).map_err(RuntimeCommandError::Payload)?.clone()) + serde_json::from_value::(unwrap_bindings(&invocation.payload).map_err(RuntimeCommandError::Payload)?) .map_err(|error| RuntimeCommandError::Payload(error.to_string()))?; CommandBody::ActivateWorld } "world.advance_time" => { - let payload: AdvanceTimePayload = serde_json::from_value(unwrap_bindings(&invocation.payload).map_err(RuntimeCommandError::Payload)?.clone()) + let payload: AdvanceTimePayload = serde_json::from_value(unwrap_bindings(&invocation.payload).map_err(RuntimeCommandError::Payload)?) .map_err(|error| RuntimeCommandError::Payload(error.to_string()))?; CommandBody::AdvanceTime { minutes: TickMinutes::new(payload.minutes).ok_or_else(|| { @@ -1387,7 +1411,7 @@ async fn execute_world( // through its own port, as the owner, and the receipt reports what one // session did rather than what one command committed. "world.seed" => { - let payload: SeedPayload = serde_json::from_value(unwrap_bindings(&invocation.payload).map_err(RuntimeCommandError::Payload)?.clone()) + let payload: SeedPayload = serde_json::from_value(unwrap_bindings(&invocation.payload).map_err(RuntimeCommandError::Payload)?) .map_err(|error| RuntimeCommandError::Payload(error.to_string()))?; let outcome = seed_once(state, verified_principal, &snapshot, payload).await?; let after = current_world(state) @@ -2882,6 +2906,107 @@ mod tests { } } + /// PA.f170's own round trip through the real client: with a question + /// open, the "Play" button's own `props.action` (the answer token) and + /// the bound `text` field must arrive together in one payload — + /// `{"answerToken":..., "bindings":{"text":...}}` — exactly the shape + /// `commandPayload` produces by spreading a button's own action fields + /// into the payload beside `bindings` + /// (`vendor/eve/packages/eve-browser-lowering/src/index.ts:2101`). This + /// is not in the committed fixture above (that walkthrough's own + /// `world.play` click never has an open question, so its action carries + /// no token to prove); this test builds its own scripted question so the + /// merge actually has something in `props.action` to carry. Skips, with + /// a message, exactly when the fixture-matching test above does. + #[tokio::test] + async fn the_real_client_carries_the_answer_token_and_the_bound_text_together() { + if !eve_client_bridge_is_available() { + eprintln!( + "SKIPPED the_real_client_carries_the_answer_token_and_the_bound_text_together: \ + `node` or the vendored lowering's built `dist/index.js` is not available in this \ + environment, so this test did not drive the real Eve client bridge." + ); + return; + } + let (fixture, served_version) = play_world_with_a_scripted_question().await; + let table = fixture.state.play.clone().unwrap(); + + let played = post( + &fixture.state, + &fixture.cookie, + invocation( + "world.play", + "ghostlight.world_play.v0", + served_version, + json!({"text":"I stand at a crossroads."}), + &uuid::Uuid::new_v4().to_string(), + ), + ) + .await; + assert_eq!(played["state"], "accepted"); + + tokio::time::timeout(Duration::from_secs(5), async { + loop { + if let Some(view) = table.current_turn_view().await + && view.state == PlayTurnState::AwaitingPlayer + { + return; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("the scripted ask_player call must open a question"); + + let provider = get(&fixture.state, &fixture.cookie, "/api/eve/provider").await; + let surface = get(&fixture.state, &fixture.cookie, "/api/eve/surfaces/ghostlight.play").await; + let play_intent = client_intents( + &surface, + &provider, + json!([ + {"set": {"world.play.text": "I go left."}}, + {"click": "world.play"} + ]), + ) + .into_iter() + .next() + .unwrap(); + + let payload = play_intent["payload"].clone(); + assert!( + payload + .get("answerToken") + .and_then(Value::as_str) + .is_some_and(|token| !token.is_empty()), + "the real client's own button action must carry a non-empty answerToken: {payload}" + ); + assert_eq!( + payload["bindings"]["text"], "I go left.", + "the real client's own captured binding must still carry the bound text beside the \ + action's own answerToken: {payload}" + ); + + let answered = post(&fixture.state, &fixture.cookie, play_intent).await; + assert_eq!( + answered["state"], "accepted", + "the real client's own combined payload must be accepted: {answered}" + ); + + let closed = tokio::time::timeout(Duration::from_secs(5), async { + loop { + if let Some(view) = table.current_turn_view().await { + if view.state == PlayTurnState::Closed { + return view; + } + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("the scripted end_turn call must close the turn"); + assert_eq!(closed.narration.as_deref(), Some("The hall falls quiet.")); + } + /// PA.f164: `targets`, `jurisdictions`, and `brief` are all documented /// "required, may be empty" (`CreatePayload`) — a legitimate deliberate /// choice, not a value nobody supplied. Before this fix, only `lens_weights` @@ -2939,17 +3064,21 @@ mod tests { ); } - /// PA.f168: `unwrap_bindings` refuses a payload carrying both the - /// envelope `bindings` object and a flat sibling field, rather than - /// silently keeping only `bindings` and discarding the sibling. No real - /// client sends this shape (`commandPayload` on the lowering's own side - /// never adds one), so this exercises the refusal directly. + /// PA.f168 originally refused any sibling next to the envelope + /// `bindings` object outright. PA.f170 changes what a real client's + /// payload carries — a button's own `props.action` fields (the answer + /// token, for `world.play`) ride beside `bindings` on purpose — so + /// `unwrap_bindings` now merges the two instead. An unrecognized sibling + /// is still refused, just one door downstream: the merged object still + /// has to satisfy the operation's own payload struct, and + /// `world.approve`'s `EmptyPayload` (`#[serde(deny_unknown_fields)]`) + /// declares none at all, so `"rogue"` is still an unknown field there. /// - /// Mutation: revert `unwrap_bindings` to its old - /// `.filter(|bindings| bindings.is_object()).unwrap_or(payload)` shape — - /// this command would then be accepted, silently discarding `"rogue"`. + /// Mutation: have `unwrap_bindings` silently drop an unrecognized sibling + /// instead of merging it in — this command would then be accepted, with + /// `"rogue"` discarded rather than refused. #[tokio::test] - async fn a_payload_with_a_sibling_field_beside_bindings_is_refused() { + async fn a_payload_with_an_unrecognized_sibling_field_beside_bindings_is_refused() { let fixture = fixture().await; let created = post( &fixture.state, @@ -2986,11 +3115,77 @@ mod tests { .await; assert_eq!(denied["state"], "denied"); assert!( - denied["message"] - .as_str() - .unwrap_or_default() - .contains("bindings"), - "a sibling field beside the envelope must be refused, not silently discarded: {denied}" + denied["message"].as_str().unwrap_or_default().contains("rogue"), + "an unrecognized sibling field beside the envelope must still be refused, not silently \ + discarded: {denied}" + ); + } + + /// PA.f170's own negative on the merge itself: a payload naming the same + /// field both inside `bindings` and as a sibling of it — a shape nothing + /// on the real lowering's own side produces (`commandPayload` builds + /// `bindings` and the action's own fields from disjoint sources) — is + /// refused as an outright collision, never resolved by silently + /// preferring one side over the other. + /// + /// Mutation: have `unwrap_bindings` prefer the sibling (or the `bindings` + /// entry) on a name collision instead of refusing it — this command + /// would then be accepted with one of the two conflicting `"minutes"` + /// values silently discarded. + #[tokio::test] + async fn a_payload_naming_the_same_field_in_bindings_and_as_a_sibling_is_refused() { + let fixture = fixture().await; + let created = post( + &fixture.state, + &fixture.cookie, + invocation( + "world.create", + "ghostlight.world_create.v4", + 0, + json!({ + "title":"Colliding Field World", + "brief":"", + "subject_label":"Operator", + "targets":{}, + "jurisdictions":[], + "lens_weights":{"patina":1,"charter":1,"ledger":1,"hearth":1,"tangle":1,"veil":1,"ember":1,"numen":1} + }), + &uuid::Uuid::new_v4().to_string(), + ), + ) + .await; + assert_eq!(created["state"], "accepted"); + let approved = post( + &fixture.state, + &fixture.cookie, + invocation("world.approve", "ghostlight.world_approve.v0", 1, json!({}), &uuid::Uuid::new_v4().to_string()), + ) + .await; + assert_eq!(approved["state"], "accepted"); + let activated = post( + &fixture.state, + &fixture.cookie, + invocation("world.activate", "ghostlight.world_activate.v0", 2, json!({}), &uuid::Uuid::new_v4().to_string()), + ) + .await; + assert_eq!(activated["state"], "accepted"); + + let denied = post( + &fixture.state, + &fixture.cookie, + invocation( + "world.advance_time", + "ghostlight.world_advance_time.v0", + 3, + json!({"bindings":{"minutes":5}, "minutes":9}), + &uuid::Uuid::new_v4().to_string(), + ), + ) + .await; + assert_eq!(denied["state"], "denied"); + assert!( + denied["message"].as_str().unwrap_or_default().contains("minutes"), + "a field named by both bindings and a sibling must be refused as a collision: {denied}" ); } @@ -3254,17 +3449,12 @@ mod tests { ); } - /// PA.f161's own harness: builds a play world through the real HTTP - /// route, then swaps `state.play` for a table over `crate::play::tests`' - /// own `ScriptedPort`, scripted to call `ask_player` and then `end_turn` - /// — a deterministic question this test can actually answer through the - /// route, which the fixture's own unreachable test inference organ - /// cannot produce. Returns the fixture and the world's `sourceVersion` - /// after activation (the version `world.play`'s own staleness check - /// compares an answer against, since asking a question is conversational - /// and moves nothing on the world). - async fn play_world_with_a_scripted_question() -> (Fixture, u64) { - use crate::play::tests::{ScriptedPort, call_event, output, text_event}; + /// The create/approve/activate boilerplate every scripted-question + /// harness below needs, factored out so `play_world_with_a_scripted_question` + /// and `play_world_with_two_scripted_questions` (Cut 13) do not each + /// repeat it. Returns the fixture and the world's `sourceVersion` after + /// activation. + async fn activated_fixture(title: &str) -> (Fixture, u64) { let fixture = fixture().await; let created = post( &fixture.state, @@ -3274,7 +3464,7 @@ mod tests { "ghostlight.world_create.v4", 0, json!({ - "title":"Scripted Question World", + "title":title, "brief":"", "subject_label":"Operator", "targets":{}, @@ -3301,6 +3491,21 @@ mod tests { .await; assert_eq!(activated["state"], "accepted"); let served_version = activated["sourceVersion"].as_u64().unwrap(); + (fixture, served_version) + } + + /// PA.f161's own harness: builds a play world through the real HTTP + /// route, then swaps `state.play` for a table over `crate::play::tests`' + /// own `ScriptedPort`, scripted to call `ask_player` and then `end_turn` + /// — a deterministic question this test can actually answer through the + /// route, which the fixture's own unreachable test inference organ + /// cannot produce. Returns the fixture and the world's `sourceVersion` + /// after activation (still a required `routeHint.sourceVersion` field on + /// every command envelope, even though `world.play` no longer reads it + /// for staleness — PA.f170 judges an answer by its own token instead). + async fn play_world_with_a_scripted_question() -> (Fixture, u64) { + use crate::play::tests::{ScriptedPort, call_event, output, text_event}; + let (fixture, served_version) = activated_fixture("Scripted Question World").await; let ask = output( "r0", @@ -3334,19 +3539,83 @@ mod tests { (fixture, served_version) } - /// PA.f161's own end-to-end proof: an answer whose `routeHint.sourceVersion` - /// names the version a real client actually served the question on — the - /// world's own `surface_version` at the instant it opened, unmoved since - /// (asking a question is conversational) — is accepted, and the turn - /// closes with the scripted narration. - /// - /// Mutation: change `source_version < question.surface_version` to - /// `source_version <= question.surface_version` at `runtime.rs`'s - /// `world.play` arm — this test's own answer, echoing exactly - /// `question.surface_version`, would then be refused as stale, failing - /// the `accepted` assertion below. - #[tokio::test] - async fn an_answer_naming_the_version_the_question_actually_opened_on_is_accepted() { + /// PA.f170's own harness: two scripted `ask_player` calls, one per round, + /// with no `end_turn` — so answering Q1 leaves the turn `AwaitingPlayer` + /// on Q2 rather than closed, the exact shape Soul's own probe needed to + /// show an answer composed for Q1 landing on Q2. + async fn play_world_with_two_scripted_questions() -> (Fixture, u64) { + use crate::play::tests::{ScriptedPort, call_event, output}; + let (fixture, served_version) = activated_fixture("Two Scripted Questions World").await; + + let ask_one = output( + "r0", + vec![call_event("c0", "ask_player", serde_json::json!({"question": "Which way?"}))], + ); + let ask_two = output( + "r1", + vec![call_event("c1", "ask_player", serde_json::json!({"question": "Are you sure?"}))], + ); + let personas = PersonaLane::new( + ControllerPort::new(fixture.state.world.clone()), + ScriptedPort::new(vec![]), + "gpt-5.6-sol".into(), + "gpt-5.6-sol".into(), + ) + .unwrap(); + let directory = tempfile::tempdir().unwrap(); + let table = PlayTable::new( + fixture.state.world.clone(), + personas, + ScriptedPort::new(vec![ask_one, ask_two]), + "gpt-5.6-terra".into(), + Arc::new(Semaphore::new(TEST_CONTROLLER_CONCURRENCY)), + directory.path().join("play-turn-v1.cc"), + ) + .unwrap(); + let mut fixture = fixture; + fixture.state.play = Some(Arc::new(table)); + fixture._play_directory = Some(directory); + (fixture, served_version) + } + + /// Finds the surface node named `id`, depth-first, the same way + /// `crate::play::tests::find_surface_node` does for `play.rs`'s own + /// tests — duplicated locally rather than exposed `pub(crate)` across a + /// module boundary for one small helper. + fn find_surface_node<'a>(node: &'a Value, id: &str) -> Option<&'a Value> { + if node.get("id").and_then(Value::as_str) == Some(id) { + return Some(node); + } + for child in node.get("children").and_then(Value::as_array).into_iter().flatten() { + if let Some(found) = find_surface_node(child, id) { + return Some(found); + } + } + None + } + + /// Reads the open question's own answer token the same way a real + /// client does (PA.f170): off the "Play" button's own served + /// `props.action.answerToken`, from the actually served surface document + /// — never from `PlayTurnView`/`OpenQuestion` directly, which a test + /// could read but a browser never does. + async fn play_button_token(state: &AppState, cookie: &str) -> Option { + let surface = get(state, cookie, "/api/eve/surfaces/ghostlight.play").await; + let root = surface.get("surface")?.get("root")?; + let button = find_surface_node(root, "world.play")?; + button + .get("props")? + .get("action")? + .get("answerToken")? + .as_str() + .map(str::to_owned) + } + + /// Drives `play_world_with_a_scripted_question` through `world.play` and + /// waits for the scripted `ask_player` call to open the turn's own + /// question, returning the fixture, table, and served version so each + /// caller only writes its own answer's own assertions. + async fn played_and_awaiting_question() -> (Fixture, Arc, u64) { let (fixture, served_version) = play_world_with_a_scripted_question().await; let table = fixture.state.play.clone().unwrap(); @@ -3364,23 +3633,38 @@ mod tests { .await; assert_eq!(played["state"], "accepted"); - let asked = tokio::time::timeout(Duration::from_secs(5), async { + tokio::time::timeout(Duration::from_secs(5), async { loop { - if let Some(view) = table.current_turn_view().await { - if view.state == PlayTurnState::AwaitingPlayer { - return view; - } + if let Some(view) = table.current_turn_view().await + && view.state == PlayTurnState::AwaitingPlayer + { + return; } tokio::time::sleep(Duration::from_millis(10)).await; } }) .await .expect("the scripted ask_player call must open a question"); - let question_version = asked.question.as_ref().unwrap().surface_version; - assert_eq!( - question_version, served_version, - "asking a question is conversational and must not move surface_version" - ); + (fixture, table, served_version) + } + + /// PA.f170's own end-to-end proof: a question asked, the card served, + /// its own answer token read out of the served surface document — never + /// out of `PlayTurnView` directly, which a real client never sees — the + /// answer accepted, and the turn closes with the scripted narration. + /// + /// Mutation: change `payload.answer_token.as_deref() != Some(question.token.as_str())` + /// to always pass (or always fail) at `runtime.rs`'s `world.play` arm — + /// this test's own answer, echoing the exact token the surface served, + /// would then be wrongly refused (or a broken-but-lenient comparison + /// would let a later mismatch test wrongly pass). + #[tokio::test] + async fn an_answer_naming_the_open_questions_own_token_read_from_the_served_surface_is_accepted() { + let (fixture, table, served_version) = played_and_awaiting_question().await; + + let token = play_button_token(&fixture.state, &fixture.cookie) + .await + .expect("the served surface's own \"Play\" button must carry the open question's answer token"); let answered = post( &fixture.state, @@ -3388,15 +3672,15 @@ mod tests { invocation( "world.play", "ghostlight.world_play.v0", - question_version, - json!({"text":"I go left."}), + served_version, + json!({"text":"I go left.","answerToken":token}), &uuid::Uuid::new_v4().to_string(), ), ) .await; assert_eq!( answered["state"], "accepted", - "an answer naming the exact version the question opened on must be accepted: {answered}" + "an answer naming the open question's own token, read off the served surface, must be accepted: {answered}" ); let closed = tokio::time::timeout(Duration::from_secs(5), async { @@ -3414,19 +3698,113 @@ mod tests { assert_eq!(closed.narration.as_deref(), Some("The hall falls quiet.")); } - /// PA.f161's own refusal proof: an answer naming a version older than the - /// one the question opened on — here, the version served just before - /// `world.play` was even called to open it — is refused as stale, and the - /// turn stays open rather than being resumed by an answer the player - /// could not have actually seen the question at. + /// PA.f170's own refusal proof: an answer that carries no token at all + /// while a question is open is refused as stale, and the turn stays open + /// rather than being resumed by an answer that could not have named the + /// question the player was actually shown. /// - /// Mutation: change `source_version < question.surface_version` to - /// `source_version < question.surface_version.saturating_sub(1)` (or any - /// weakening of the comparison) — this test's stale answer, one version - /// behind, would then be wrongly accepted. + /// Mutation: change `payload.answer_token.as_deref() != Some(question.token.as_str())` + /// to treat a missing token (`None`) as a match — this test's answer, + /// carrying no `answerToken` field at all, would then be wrongly + /// accepted. #[tokio::test] - async fn an_answer_naming_a_version_older_than_the_question_is_refused_as_stale() { - let (fixture, served_version) = play_world_with_a_scripted_question().await; + async fn an_answer_missing_the_open_questions_own_token_is_refused_as_stale() { + let (fixture, table, served_version) = played_and_awaiting_question().await; + + let stale_answered = post( + &fixture.state, + &fixture.cookie, + invocation( + "world.play", + "ghostlight.world_play.v0", + served_version, + json!({"text":"I go left."}), + &uuid::Uuid::new_v4().to_string(), + ), + ) + .await; + assert_eq!(stale_answered["state"], "denied"); + assert!( + stale_answered["message"] + .as_str() + .unwrap_or_default() + .contains("stale"), + "an answer with no token at all must be refused as stale: {stale_answered}" + ); + + let still_open = table.current_turn_view().await.unwrap(); + assert_eq!( + still_open.state, + PlayTurnState::AwaitingPlayer, + "a stale answer must leave the question open, not resume the turn" + ); + } + + /// PA.f170: an answer naming a token that does not match the currently + /// open question's own — forged, mistyped, or simply wrong — is refused + /// as stale exactly like a missing one, never applied to the open + /// question. + /// + /// Mutation: compare only a prefix or a hash of the token instead of the + /// exact string — this test's near-miss token (the real one with its + /// last character flipped) would then be wrongly accepted for at least + /// some mutations of that weakened comparison. + #[tokio::test] + async fn an_answer_naming_a_mismatched_token_is_refused_as_stale() { + let (fixture, table, served_version) = played_and_awaiting_question().await; + + let real_token = play_button_token(&fixture.state, &fixture.cookie) + .await + .expect("the served surface must carry the open question's own answer token"); + let mut forged = real_token.clone(); + forged.push('x'); + + let stale_answered = post( + &fixture.state, + &fixture.cookie, + invocation( + "world.play", + "ghostlight.world_play.v0", + served_version, + json!({"text":"I go left.","answerToken":forged}), + &uuid::Uuid::new_v4().to_string(), + ), + ) + .await; + assert_eq!(stale_answered["state"], "denied"); + assert!( + stale_answered["message"] + .as_str() + .unwrap_or_default() + .contains("stale"), + "a mismatched token must be refused as stale: {stale_answered}" + ); + + let still_open = table.current_turn_view().await.unwrap(); + assert_eq!( + still_open.state, + PlayTurnState::AwaitingPlayer, + "a stale answer must leave the question open, not resume the turn" + ); + } + + /// PA.f170's own core proof — the defect Soul demonstrated (an answer + /// composed for Q1 lands on Q2, the player never saw): two questions open + /// in the same turn, an answer carries Q1's own token, and by the time it + /// is submitted the turn has moved on to Q2. `question_surface_version` + /// could not refuse this: asking is conversational and never moves the + /// world's own revision, so Q1 and Q2 would have carried the same + /// version, and a version-only comparison could not tell them apart. The + /// per-question token can, and must. + /// + /// Mutation: have `runtime.rs`'s `world.play` arm resolve `answers` from + /// `payload.text` alone, ignoring `answer_token` entirely (reverting to + /// "any request while a question is open answers whatever is open now") + /// — this test's Q1-token answer would then be wrongly accepted against + /// Q2. + #[tokio::test] + async fn soul_an_answer_composed_for_q1_lands_on_q2_the_player_never_saw() { + let (fixture, served_version) = play_world_with_two_scripted_questions().await; let table = fixture.state.play.clone().unwrap(); let played = post( @@ -3443,47 +3821,175 @@ mod tests { .await; assert_eq!(played["state"], "accepted"); - let asked = tokio::time::timeout(Duration::from_secs(5), async { + tokio::time::timeout(Duration::from_secs(5), async { loop { - if let Some(view) = table.current_turn_view().await { - if view.state == PlayTurnState::AwaitingPlayer { - return view; - } + if let Some(view) = table.current_turn_view().await + && view.question.as_ref().is_some_and(|question| question.text == "Which way?") + { + return; } tokio::time::sleep(Duration::from_millis(10)).await; } }) .await - .expect("the scripted ask_player call must open a question"); - let question_version = asked.question.as_ref().unwrap().surface_version; - assert!(question_version >= 1, "the version arithmetic below needs headroom"); + .expect("the first scripted ask_player call must open Q1"); - let stale_answered = post( + // The player reads Q1's own token off the served surface — exactly + // what a real client's "Play" button would carry — but does not + // answer it before the table moves on to Q2. + let q1_token = play_button_token(&fixture.state, &fixture.cookie) + .await + .expect("the served surface must carry Q1's own answer token"); + + let answered_q1 = post( &fixture.state, &fixture.cookie, invocation( "world.play", "ghostlight.world_play.v0", - question_version - 1, - json!({"text":"I go left."}), + served_version, + json!({"text":"left","answerToken":q1_token}), &uuid::Uuid::new_v4().to_string(), ), ) .await; - assert_eq!(stale_answered["state"], "denied"); - assert!( - stale_answered["message"] - .as_str() - .unwrap_or_default() - .contains("stale"), - "a version older than the question's own must be refused as stale: {stale_answered}" + assert_eq!( + answered_q1["state"], "accepted", + "Q1's own token must still answer Q1: {answered_q1}" ); - let still_open = table.current_turn_view().await.unwrap(); + tokio::time::timeout(Duration::from_secs(5), async { + loop { + if let Some(view) = table.current_turn_view().await + && view.question.as_ref().is_some_and(|question| question.text == "Are you sure?") + { + return; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("answering Q1 must open Q2"); + + // A late request still carrying Q1's own token — the exact shape a + // slow retry or a second tab would produce — must never be applied + // to Q2, which the player has not been shown yet. + let stale_on_q2 = post( + &fixture.state, + &fixture.cookie, + invocation( + "world.play", + "ghostlight.world_play.v0", + served_version, + json!({"text":"yes","answerToken":q1_token}), + &uuid::Uuid::new_v4().to_string(), + ), + ) + .await; assert_eq!( - still_open.state, - PlayTurnState::AwaitingPlayer, - "a stale answer must leave the question open, not resume the turn" + stale_on_q2["state"], "denied", + "an answer composed for Q1 must never land on Q2: {stale_on_q2}" + ); + assert!( + stale_on_q2["message"].as_str().unwrap_or_default().contains("stale"), + "the refusal must name it stale: {stale_on_q2}" + ); + + let still_on_q2 = table.current_turn_view().await.unwrap(); + assert_eq!( + still_on_q2.question.as_ref().map(|question| question.text.as_str()), + Some("Are you sure?"), + "the wrongly-refused answer must leave Q2 open, not resolve it" + ); + } + + /// PA.f170: a token is single-use across distinct questions — reusing + /// the token a *closed* turn's own answered question carried, replayed + /// against a fresh question with a fresh token, is refused exactly like + /// any other mismatch (the two-questions probe above already proves + /// reuse within one still-open turn is refused; this proves reuse is + /// never grandfathered in just because the token was once genuine). + #[tokio::test] + async fn a_token_from_an_already_answered_question_is_refused_against_the_next_one() { + let (fixture, served_version) = play_world_with_two_scripted_questions().await; + let table = fixture.state.play.clone().unwrap(); + + let played = post( + &fixture.state, + &fixture.cookie, + invocation( + "world.play", + "ghostlight.world_play.v0", + served_version, + json!({"text":"I stand at a crossroads."}), + &uuid::Uuid::new_v4().to_string(), + ), + ) + .await; + assert_eq!(played["state"], "accepted"); + + tokio::time::timeout(Duration::from_secs(5), async { + loop { + if let Some(view) = table.current_turn_view().await + && view.question.as_ref().is_some_and(|question| question.text == "Which way?") + { + return; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("the first scripted ask_player call must open Q1"); + let q1_token = play_button_token(&fixture.state, &fixture.cookie) + .await + .expect("the served surface must carry Q1's own answer token"); + + let answered_q1 = post( + &fixture.state, + &fixture.cookie, + invocation( + "world.play", + "ghostlight.world_play.v0", + served_version, + json!({"text":"left","answerToken":q1_token.clone()}), + &uuid::Uuid::new_v4().to_string(), + ), + ) + .await; + assert_eq!(answered_q1["state"], "accepted"); + + tokio::time::timeout(Duration::from_secs(5), async { + loop { + if let Some(view) = table.current_turn_view().await + && view.question.as_ref().is_some_and(|question| question.text == "Are you sure?") + { + return; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("answering Q1 must open Q2"); + let q2_token = play_button_token(&fixture.state, &fixture.cookie) + .await + .expect("the served surface must carry Q2's own answer token"); + assert_ne!(q1_token, q2_token, "Q1's own spent token must never be reissued for Q2"); + + let replayed_q1_token = post( + &fixture.state, + &fixture.cookie, + invocation( + "world.play", + "ghostlight.world_play.v0", + served_version, + json!({"text":"yes","answerToken":q1_token}), + &uuid::Uuid::new_v4().to_string(), + ), + ) + .await; + assert_eq!( + replayed_q1_token["state"], "denied", + "Q1's own already-spent token must never answer Q2: {replayed_q1_token}" ); }