diff --git a/CHANGELOG.md b/CHANGELOG.md index ee42145..7d629f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- `ack.sh` left the previous state's `transitions` in `state.json`, so a second ack before the hook fired again failed with `invalid next state ''`. The agent had to retry the gated command after every step. `ack.sh` now sets `transitions` and `next_state` for the new state, and a session started by an older steplock gets the fixed `ack.sh` on its next block + ## [0.2.0] - 2026-09-29 ### Changed diff --git a/core/scripts/ack.sh b/core/scripts/ack.sh index 8553090..4ac944d 100644 --- a/core/scripts/ack.sh +++ b/core/scripts/ack.sh @@ -24,10 +24,13 @@ if [ -z "$MATCHED" ]; then exit 1 fi +# Set the new state's transitions from the flow, so the next ack works +# without the hook firing in between. jq --arg cur "$CURRENT" --arg next "$NEXT" ' .visited += [$cur] | .current_state = $next | - .next_state = null + .transitions = (.flow_transitions[$next] // []) | + .next_state = (if (.transitions | length) == 1 then .transitions[0] else null end) ' "$STATE" > "$TMP" && mv "$TMP" "$STATE" # Append ack event to audit.log — failures silently ignored (audit must never block). diff --git a/core/src/ack.sh b/core/src/ack.sh deleted file mode 100644 index 8553090..0000000 --- a/core/src/ack.sh +++ /dev/null @@ -1,41 +0,0 @@ -#!/bin/sh -DIR="$(cd "$(dirname "$0")" && pwd)" -STATE="$DIR/state.json" -TMP="$STATE.tmp.$$" - -CURRENT=$(jq -r '.current_state' "$STATE") - -if [ "$CURRENT" = "[*]" ]; then - CHECKLIST=$(jq -r '.checklist' "$STATE") - echo "steplock: nothing to acknowledge for '$CHECKLIST' (session already complete)" - exit 0 -fi - -NEXT=$(jq -r '.next_state // empty' "$STATE") -NEXT="${1:-$NEXT}" - -VALID=$(jq -r '.transitions[]' "$STATE") -MATCHED=$(printf '%s\n' $VALID | grep -Fx "$NEXT") - -if [ -z "$MATCHED" ]; then - echo "steplock: invalid next state '$NEXT'" >&2 - echo "Valid transitions from '$CURRENT':" >&2 - printf '%s\n' $VALID | sed 's/^/ /' >&2 - exit 1 -fi - -jq --arg cur "$CURRENT" --arg next "$NEXT" ' - .visited += [$cur] | - .current_state = $next | - .next_state = null -' "$STATE" > "$TMP" && mv "$TMP" "$STATE" - -# Append ack event to audit.log — failures silently ignored (audit must never block). -STEPLOCK_DIR="$(cd "$DIR/../../.." && pwd)" -SESSION="$(basename "$(dirname "$DIR")")" -CHECKLIST="$(jq -r '.checklist' "$STATE")" -TS="$(date -u +%Y-%m-%dT%H:%M:%SZ)" -jq -cn --arg e "ack" --arg c "$CHECKLIST" --arg s "$CURRENT" \ - --arg sess "$SESSION" --arg ts "$TS" \ - '{event:$e,checklist:$c,state:$s,session:$sess,ts:$ts}' \ - >> "$STEPLOCK_DIR/audit.log" 2>/dev/null || true diff --git a/core/src/gate.rs b/core/src/gate.rs index 93f5a73..3d79de5 100644 --- a/core/src/gate.rs +++ b/core/src/gate.rs @@ -1,4 +1,5 @@ //! Checklist gate: decides whether one checklist blocks a hook event. +use std::collections::HashMap; use std::fmt::Write as _; use std::fs; use std::path::Path; @@ -90,6 +91,7 @@ fn block_reset_always( current_state: initial_state.to_owned(), next_state, transitions, + flow_transitions: HashMap::new(), visited: vec![], }; audit::append( @@ -152,6 +154,7 @@ fn block_reset_session( .cloned() .filter(|_| raw_transitions.len() == 1); state.transitions = raw_transitions; + state.flow_transitions.clone_from(&flow.transitions); save_state(&state_path, &state)?; scripts::ensure_ack_sh(&session_dir)?; diff --git a/core/src/run_tests.rs b/core/src/run_tests.rs index 6286d7b..dd6cf5d 100644 --- a/core/src/run_tests.rs +++ b/core/src/run_tests.rs @@ -76,6 +76,7 @@ fn approves_and_resets_state_when_complete() { current_state: "[*]".to_owned(), next_state: None, transitions: vec![], + flow_transitions: HashMap::new(), visited: vec!["clean_code".to_owned()], }; save_state(&session_dir.join("state.json"), &state).unwrap(); @@ -635,6 +636,7 @@ fn unknown_current_state_in_flow_skips_checklist() { current_state: "nonexistent_state".to_owned(), next_state: None, transitions: vec![], + flow_transitions: HashMap::new(), visited: vec![], }; save_state(&session_dir.join("state.json"), &state).unwrap(); @@ -660,6 +662,7 @@ fn complete_event_written_to_audit_log() { current_state: "[*]".to_owned(), next_state: None, transitions: vec![], + flow_transitions: HashMap::new(), visited: vec!["clean_code".to_owned()], }; save_state(&session_dir.join("state.json"), &state).unwrap(); diff --git a/core/src/scripts.rs b/core/src/scripts.rs index 126245d..6b51279 100644 --- a/core/src/scripts.rs +++ b/core/src/scripts.rs @@ -7,14 +7,15 @@ use crate::flow::FlowGraph; static ACK_SH: &str = include_str!("../scripts/ack.sh"); -/// Write ack.sh to `dir` only if it does not already exist. +/// Write ack.sh to `dir` unless it already holds the current script. +/// A session started by an older steplock gets the current script on its next block. /// /// # Errors /// /// Returns `Err` if writing the file or setting its permissions fails. pub fn ensure_ack_sh(dir: &Path) -> Result<()> { let path = dir.join("ack.sh"); - if path.exists() { + if fs::read_to_string(&path).is_ok_and(|s| s == ACK_SH) { return Ok(()); } write_executable(&path, ACK_SH) diff --git a/core/src/scripts_tests.rs b/core/src/scripts_tests.rs index 0992c61..458c6b3 100644 --- a/core/src/scripts_tests.rs +++ b/core/src/scripts_tests.rs @@ -41,13 +41,12 @@ fn ack_sh_appends_audit_event() { } #[test] -fn ensure_ack_sh_is_idempotent() { +fn ensure_ack_sh_replaces_stale_script() { let tmp = TempDir::new().unwrap(); let path = tmp.path().join("ack.sh"); - fs::write(&path, "custom content").unwrap(); + fs::write(&path, "old script").unwrap(); ensure_ack_sh(tmp.path()).unwrap(); - // Should not overwrite existing file - assert_eq!(fs::read_to_string(&path).unwrap(), "custom content"); + assert_eq!(fs::read_to_string(&path).unwrap(), ACK_SH); } #[test] diff --git a/core/src/state.rs b/core/src/state.rs index e749363..57915a1 100644 --- a/core/src/state.rs +++ b/core/src/state.rs @@ -22,6 +22,10 @@ pub struct SessionState { pub next_state: Option, /// All valid next state names from `current_state` (including `"[*]"`). pub transitions: Vec, + /// Outgoing transitions for every state in the flow, so `ack.sh` can set + /// `transitions` and `next_state` for the state it advances to. + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + pub flow_transitions: HashMap>, /// States that have been acknowledged in this session so far. pub visited: Vec, } @@ -66,6 +70,7 @@ pub fn init_state(checklist: &str, initial_state: &str) -> SessionState { current_state: initial_state.to_owned(), next_state: None, transitions: Vec::new(), + flow_transitions: HashMap::new(), visited: Vec::new(), } } diff --git a/core/src/state_tests.rs b/core/src/state_tests.rs index 0a3e0fe..f371bbc 100644 --- a/core/src/state_tests.rs +++ b/core/src/state_tests.rs @@ -1,5 +1,6 @@ //! Unit tests for `state`. use super::*; +use std::collections::HashMap; use tempfile::TempDir; #[test] @@ -25,6 +26,7 @@ fn is_complete_true_at_end() { current_state: "[*]".to_owned(), next_state: None, transitions: vec![], + flow_transitions: HashMap::new(), visited: vec!["step_one".to_owned()], }; assert!(s.is_complete()); @@ -39,6 +41,7 @@ fn save_and_load_roundtrip() { current_state: "check_one".to_owned(), next_state: Some("check_two".to_owned()), transitions: vec!["check_two".to_owned()], + flow_transitions: HashMap::new(), visited: vec!["prev".to_owned()], }; save_state(&path, &s).unwrap(); diff --git a/core/tests/cli_tests.rs b/core/tests/cli_tests.rs index 55cf6f0..dbc03e1 100644 --- a/core/tests/cli_tests.rs +++ b/core/tests/cli_tests.rs @@ -416,3 +416,50 @@ fn hermes_hook_uses_real_home_global_checklist_when_home_is_sandboxed() { "global checklist from the real home must run: {json}" ); } + +#[test] +fn ack_sh_advances_twice_without_a_hook_call_between() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().join(".steplock/checklists/gate"); + fs::create_dir_all(&dir).unwrap(); + fs::write( + dir.join("config.toml"), + "on_event = \"tool:before\"\non_tool = \"bash\"\nreset = \"session\"\n", + ) + .unwrap(); + fs::write( + dir.join("flow.mmd"), + "stateDiagram-v2\n [*] --> one\n one --> two\n two --> left\n two --> right\n left --> three\n right --> three\n three --> [*]\n one: Step one\n two: Step two\n left: Go left\n right: Go right\n three: Step three\n", + ) + .unwrap(); + + let stdin = hook_event("bash", "git push", "sess-ack"); + let (first_code, _, _) = run_steplock(tmp.path(), &stdin); + assert_eq!(first_code, 0, "first call blocks at step one"); + + let ack = tmp.path().join(".steplock/sessions/sess-ack/gate/ack.sh"); + let run_ack = |arg: Option<&str>| { + let mut cmd = Command::new("sh"); + cmd.arg(&ack); + if let Some(a) = arg { + cmd.arg(a); + } + cmd.output().unwrap() + }; + // one -> two (linear), two -> right (branch), right -> three (linear): no hook call between. + for arg in [None, Some("right"), None] { + let out = run_ack(arg); + assert!( + out.status.success(), + "ack {arg:?} failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + } + + let (code, stdout, _) = run_steplock(tmp.path(), &stdin); + assert_eq!(code, 0); + assert!( + stdout.contains("Step three"), + "expected block at step three, got: {stdout}" + ); +} diff --git a/schemas/session-state.schema.json b/schemas/session-state.schema.json index a098d50..5853604 100644 --- a/schemas/session-state.schema.json +++ b/schemas/session-state.schema.json @@ -24,6 +24,11 @@ "description": "Valid next state names from current_state. ack.sh validates $1 against this list.", "items": { "type": "string" } }, + "flow_transitions": { + "type": "object", + "description": "Outgoing transitions for every state in the flow. ack.sh reads it to set transitions and next_state for the state it advances to.", + "additionalProperties": { "type": "array", "items": { "type": "string" } } + }, "visited": { "type": "array", "description": "State node names already acknowledged this scope. Written by ack.sh.",