diff --git a/crates/stella-tools/src/registry/tests.rs b/crates/stella-tools/src/registry/tests.rs index d7a9adcdf..3ee547738 100644 --- a/crates/stella-tools/src/registry/tests.rs +++ b/crates/stella-tools/src/registry/tests.rs @@ -177,6 +177,66 @@ async fn state_tools_round_trip_through_registry_dispatch() { assert!(out.is_error(), "a deleted key must not read back"); } +/// Witness for #3297: two `save_state` calls writing *different same-length* +/// content under one key must render distinct success outputs. The engine's +/// stagnation detector (`stella-core`'s `loop_detect`, rung 3) kills +/// consecutive same-tool calls whose outputs are byte-identical — whatever +/// their arguments — so the old constant-shape `saved {key} (N bytes)` +/// string let a legitimate checkpoint loop (a fixed-width counter updated +/// under one key) be killed as stagnant mid-solve. The `sha256/8` suffix is +/// the content-derived identity that makes the outputs distinct. +#[tokio::test] +async fn save_state_outputs_differ_for_different_same_length_content() { + let (_root, reg) = bare_registry(); + let mut outputs = Vec::new(); + for content in ["counter=1", "counter=2"] { + let ToolOutput::Ok { content } = reg + .execute( + "save_state", + &serde_json::json!({"key": "checkpoint", "content": content}), + ) + .await + else { + panic!("save_state must succeed"); + }; + outputs.push(content); + } + assert!( + outputs[0].starts_with("saved checkpoint (9 bytes"), + "the `saved {{key}} ({{N}} bytes` prefix is stable — the identity is \ + appended, never restructured: {:?}", + outputs[0] + ); + assert_ne!( + outputs[0], outputs[1], + "different same-length content under one key must render distinct \ + outputs, or the stagnation detector kills a legitimate checkpoint \ + loop as stagnant (#3297)" + ); +} + +/// The complement that guards the detector's contract: re-saving *identical* +/// bytes must still render byte-identical output. A loop genuinely stuck +/// re-saving the same state is exactly what the detector exists to catch, +/// and only a content-derived suffix — never a timing, never randomness +/// (#2706) — keeps that catch intact. +#[tokio::test] +async fn save_state_output_is_identical_for_identical_content() { + let (_root, reg) = bare_registry(); + let save = serde_json::json!({"key": "checkpoint", "content": "counter=1"}); + let ToolOutput::Ok { content: first } = reg.execute("save_state", &save).await else { + panic!("save_state must succeed"); + }; + let ToolOutput::Ok { content: second } = reg.execute("save_state", &save).await else { + panic!("save_state must succeed"); + }; + assert_eq!( + first, second, + "re-saving identical bytes must render byte-identical output — the \ + stagnation detector's catch of a genuinely stuck loop depends on it" + ); +} + /// `get_environment` dispatches through the registry and reports the scratch /// directory the registry created — the one fact the CLI's prompt block /// cannot carry (#3102). diff --git a/crates/stella-tools/src/scratch.rs b/crates/stella-tools/src/scratch.rs index 1808d592c..9286b8d20 100644 --- a/crates/stella-tools/src/scratch.rs +++ b/crates/stella-tools/src/scratch.rs @@ -19,6 +19,14 @@ //! named error, which is what makes traversal impossible rather than //! filtered. //! - `save_state` refuses content over `MAX_SAVE_BYTES` with a named error. +//! - `save_state`'s success output embeds the saved content's `sha256/8`, so +//! distinct saves render distinct outputs. The engine's stagnation +//! detector (`stella-core`'s `loop_detect`) kills consecutive same-tool +//! calls whose outputs are byte-identical, and a constant-shape +//! `saved {key} (N bytes)` string made a legitimate checkpoint loop — +//! different same-length content under one key — indistinguishable from a +//! stuck one (#3297). The suffix is content-derived only: a timing or any +//! randomness would blind the detector to genuinely stuck loops (#2706). //! - `get_state` middle-truncates nothing: it pages (`offset`/`limit` //! in bytes, clamped to a `PAGE_CAP` slice) and names the remainder, so //! a partial read is always visibly partial. @@ -133,9 +141,19 @@ impl Tool for SaveState { } }; match std::fs::write(&path, content) { - Ok(()) => ToolOutput::Ok { - content: format!("saved {key} ({} bytes)", content.len()), - }, + Ok(()) => { + // The digest is the output's content-derived identity: without + // it, different same-length saves under one key render + // byte-identical outputs and the stagnation detector kills the + // loop as stuck (#3297). Deterministic by contract — never a + // timing, never randomness (#2706). `sha256/8` = the first 8 + // hex chars of the sha256; `digest` always returns 64 ASCII + // hex chars, so the slice cannot panic. + let identity = &crate::foundry_gate::digest(content.as_bytes())[..8]; + ToolOutput::Ok { + content: format!("saved {key} ({} bytes, sha256/8 {identity})", content.len()), + } + } Err(e) => ToolOutput::error(format!("failed to save {key}: {e}")), } }