From ec9d8ef920f410153fbf45a5c9e0293ec89bbed3 Mon Sep 17 00:00:00 2001 From: Johannes Schickling Date: Sun, 23 Aug 2026 19:01:59 +0200 Subject: [PATCH 01/14] =?UTF-8?q?feat(harness-state):=20Claude=20producer?= =?UTF-8?q?=20=E2=80=94=20hook=20transitions,=20wrapper=20heartbeat,=20ter?= =?UTF-8?q?minal=20writes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hook-driven turn transitions land through a new fail-open claude-observe.sh (shipped in the immutable hook set) and 'st2 driver claude-observe': UserPromptSubmit/PreToolUse/PostToolUse -> active, Stop -> idle, PermissionRequest -> active blocked-on-human, SessionStart -> idle. Subagent events (non-empty agent_id) never move top-level state. The shared provider wrapper gains a SessionObserver that re-stamps the record on the presence cadence without clobbering hook-written state, writes the terminal record on every reaped child, and writes it BEFORE SIGKILL-escalating its own process group so st2's own teardown no longer leaves a dead agent reading active. Co-Authored-By: Claude Fable 5 --- examples/native/agent-claude.kdl | 54 +++++++++ hooks/claude-observe.sh | 16 +++ src/claude_session.rs | 202 ++++++++++++++++++++++++++++++- src/hooks.rs | 4 +- src/main.rs | 13 ++ src/pi_session.rs | 2 + src/provider_session.rs | 91 ++++++++++++-- 7 files changed, 371 insertions(+), 11 deletions(-) create mode 100755 hooks/claude-observe.sh diff --git a/examples/native/agent-claude.kdl b/examples/native/agent-claude.kdl index 48f95d19..df0675bc 100644 --- a/examples/native/agent-claude.kdl +++ b/examples/native/agent-claude.kdl @@ -30,6 +30,10 @@ agent "" { "async": true, "asyncRewake": true, "command": "$ST_HOOKS/claude-session-start.sh" + }, + { + "type": "command", + "command": "$ST_HOOKS/claude-observe.sh SessionStart" } ] } @@ -53,6 +57,56 @@ agent "" { } ] } + ], + "UserPromptSubmit": [ + { + "hooks": [ + { + "type": "command", + "command": "$ST_HOOKS/claude-observe.sh UserPromptSubmit" + } + ] + } + ], + "Stop": [ + { + "hooks": [ + { + "type": "command", + "command": "$ST_HOOKS/claude-observe.sh Stop" + } + ] + } + ], + "PermissionRequest": [ + { + "hooks": [ + { + "type": "command", + "command": "$ST_HOOKS/claude-observe.sh PermissionRequest" + } + ] + } + ], + "PreToolUse": [ + { + "hooks": [ + { + "type": "command", + "command": "$ST_HOOKS/claude-observe.sh PreToolUse" + } + ] + } + ], + "PostToolUse": [ + { + "hooks": [ + { + "type": "command", + "command": "$ST_HOOKS/claude-observe.sh PostToolUse" + } + ] + } ] } } diff --git a/hooks/claude-observe.sh b/hooks/claude-observe.sh new file mode 100755 index 00000000..620b49da --- /dev/null +++ b/hooks/claude-observe.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +# st2 Claude observe hook: forward one hook event (name in $1, payload on stdin) to the agent's +# observed-harness-state record. Fail-open; observation must never wedge or slow the harness. + +set -u + +event="${1:-}" +identity="${ST_AGENT:-}" +root="${ST_ROOT:-${CATALOG:-}}" +if [[ -z "$event" || -z "$identity" || -z "$root" ]] || ! command -v st2 >/dev/null 2>&1; then + exit 0 +fi + +st2 --catalog "$root" driver claude-observe --identity "$identity" --event "$event" \ + >/dev/null 2>&1 || true +exit 0 diff --git a/src/claude_session.rs b/src/claude_session.rs index 753e3827..d8f052af 100644 --- a/src/claude_session.rs +++ b/src/claude_session.rs @@ -4,13 +4,22 @@ //! provider still lives. This wrapper launches the provider and refreshes presence while that exact //! child remains alive. It uses the provider's existing terminal process group. The launch body //! itself lives in [`crate::provider_session`], which every interactive harness wrapper shares. +//! +//! Observed harness state for Claude has two producers with one owner each: hook invocations +//! (`st2 driver claude-observe`, [`run_observe`]) write turn transitions, and the wrapper's poll +//! loop re-stamps and terminates the record through [`SessionObserver`] without ever overwriting a +//! state a hook wrote in between. +use std::io::Read as _; use std::path::Path; use anyhow::{Context as _, Result}; -use crate::provider_session::{PROVIDER_POLL, STOP, install_signal_handler, run_provider}; -use crate::{message, status}; +use crate::harness_state::{Activity, BlockedOn, InputBuffer, Observation}; +use crate::provider_session::{ + PROVIDER_POLL, STOP, SessionObserver, install_signal_handler, run_provider, +}; +use crate::{harness_state, message, status}; /// Run one interactive Claude provider and maintain its presence until it exits. pub fn run( @@ -27,6 +36,7 @@ pub fn run( "Claude driver '{runtime_id}' has no provider argv" ); install_signal_handler(); + let observer = SessionObserver::new(&agent_dir, &identity, "claude"); run_provider( "Claude", &status::status_path(&agent_dir), @@ -35,10 +45,71 @@ pub fn run( status::STATUS_REFRESH, PROVIDER_POLL, &STOP, + Some(&observer), ) .with_context(|| format!("running Claude driver '{runtime_id}'")) } +/// Apply one Claude hook event (payload on stdin) to the agent's observed-harness-state record. +/// +/// Invoked per event by the fail-open `claude-observe.sh` hook, so each invocation is its own +/// short-lived writer; the transition counter continues from disk. +pub fn run_observe(catalog_root: &Path, identity: &str, event: &str) -> Result<()> { + let agent_dir = message::resolve_agent_dir(catalog_root, identity, &crate::run::detect_host())? + .with_context(|| format!("Claude driver agent '{identity}' is not declared"))?; + let mut raw = String::new(); + let _ = std::io::stdin().read_to_string(&mut raw); + let payload = serde_json::from_str(&raw).unwrap_or(serde_json::Value::Null); + let Some(observation) = observe_hook_event(event, &payload) else { + return Ok(()); + }; + harness_state::Writer::new(&agent_dir, identity, "claude", Some(identity.to_string())) + .observe(observation) +} + +/// Map one Claude hook event to an observation, or `None` when the event says nothing about +/// top-level harness state. +/// +/// Claude gives no call identity on the event that enters `blocked` (`PermissionRequest` carries +/// no `tool_use_id`), so the exit edge is the next `PreToolUse`/`PostToolUse`/`Stop`. Under +/// batched tool calls that can clear `blocked` while a later call in the batch still holds a +/// prompt; the limit is accepted and fenced in the VRS rather than papered over with a temporal +/// heuristic that fails for exactly the parallel case it would need to handle. +pub fn observe_hook_event(event: &str, payload: &serde_json::Value) -> Option { + // Any event carrying an agent identity is a subagent's and must never move top-level state: + // a phantom `SubagentStop` trails every completed turn, 1.5-2.9s after `Stop`. That phantom + // populating `agent_id` is an undocumented emergent property — if a future Claude build omits + // it, subagent completions read as top-level activity again and nothing here would catch it. + if payload + .get("agent_id") + .and_then(serde_json::Value::as_str) + .is_some_and(|id| !id.is_empty()) + { + return None; + } + match event { + "SessionStart" => Some( + Observation::new(Activity::Idle, BlockedOn::None, InputBuffer::Unknown) + .with_reason("sessionStart"), + ), + "UserPromptSubmit" | "PreToolUse" | "PostToolUse" => Some(Observation::new( + Activity::Active, + BlockedOn::None, + InputBuffer::Unknown, + )), + "Stop" => Some(Observation::new( + Activity::Idle, + BlockedOn::None, + InputBuffer::Unknown, + )), + "PermissionRequest" => Some( + Observation::new(Activity::Active, BlockedOn::Human, InputBuffer::Unknown) + .with_reason("permissionRequest"), + ), + _ => None, + } +} + #[cfg(test)] mod tests { use std::fs; @@ -46,6 +117,7 @@ mod tests { use std::time::Duration; use super::*; + use crate::harness_state::harness_state_path; #[test] fn idle_provider_refreshes_presence_without_mcp_input() { @@ -63,6 +135,7 @@ mod tests { Duration::from_millis(25), Duration::from_millis(5), &stop, + None, ) .unwrap(); @@ -70,4 +143,129 @@ mod tests { assert_ne!(after, before); assert_eq!(status::read_state(&presence), status::State::Available); } + + #[test] + fn hook_events_map_to_observations_with_the_blocked_edges() { + let none = serde_json::Value::Null; + + let blocked = observe_hook_event("PermissionRequest", &none).unwrap(); + assert_eq!(blocked.state, Activity::Active); + assert_eq!(blocked.blocked_on, BlockedOn::Human); + assert_eq!(blocked.reason.as_deref(), Some("permissionRequest")); + + // The exit edges: tool progress or a turn boundary clears the human hold. + for event in ["PreToolUse", "PostToolUse"] { + let cleared = observe_hook_event(event, &none).unwrap(); + assert_eq!(cleared.state, Activity::Active); + assert_eq!(cleared.blocked_on, BlockedOn::None); + } + let stop = observe_hook_event("Stop", &none).unwrap(); + assert_eq!(stop.state, Activity::Idle); + assert_eq!(stop.blocked_on, BlockedOn::None); + + assert_eq!( + observe_hook_event("UserPromptSubmit", &none).unwrap().state, + Activity::Active + ); + assert_eq!( + observe_hook_event("SessionStart", &none).unwrap().state, + Activity::Idle + ); + + // Unmapped events say nothing rather than guessing. + assert_eq!(observe_hook_event("Notification", &none), None); + assert_eq!(observe_hook_event("SubagentStop", &none), None); + } + + #[test] + fn subagent_events_never_move_top_level_state() { + let subagent = serde_json::json!({"agent_id": "sub-1", "agent_type": ""}); + for event in [ + "Stop", + "UserPromptSubmit", + "PermissionRequest", + "PostToolUse", + ] { + assert_eq!(observe_hook_event(event, &subagent), None, "{event}"); + } + // An empty agent_id is the top-level shape. + let top = serde_json::json!({"agent_id": ""}); + assert!(observe_hook_event("Stop", &top).is_some()); + } + + #[test] + fn wrapper_heartbeat_re_stamps_without_clobbering_hook_written_state() { + let tmp = tempfile::tempdir().unwrap(); + let record = harness_state_path(tmp.path()); + let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude"); + + // A hook process wrote a blocked observation between wrapper ticks. + harness_state::Writer::new(tmp.path(), "hetz.worker", "claude", None) + .observe(observe_hook_event("PermissionRequest", &serde_json::Value::Null).unwrap()) + .unwrap(); + let before = fs::read(&record).unwrap(); + + std::thread::sleep(Duration::from_millis(2)); + observer.heartbeat(); + let after = fs::read(&record).unwrap(); + assert_ne!(before, after, "heartbeat must re-stamp bytes"); + let observed = harness_state::read(&record, None).unwrap(); + assert_eq!(observed.state, Activity::Active); + assert_eq!(observed.blocked_on, BlockedOn::Human); + assert_eq!(observed.reason.as_deref(), Some("permissionRequest")); + } + + #[test] + fn a_provider_killed_mid_turn_reads_ended_rather_than_active() { + let tmp = tempfile::tempdir().unwrap(); + let presence = status::status_path(tmp.path()); + let record = harness_state_path(tmp.path()); + let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude"); + let stop = AtomicBool::new(false); + + // A turn is in flight when the provider dies by signal. + harness_state::Writer::new(tmp.path(), "hetz.worker", "claude", None) + .observe(observe_hook_event("UserPromptSubmit", &serde_json::Value::Null).unwrap()) + .unwrap(); + + let result = run_provider( + "Claude", + &presence, + &["sh".into(), "-c".into(), "kill -9 $$".into()], + &[], + Duration::from_millis(25), + Duration::from_millis(5), + &stop, + Some(&observer), + ); + assert!(result.is_err(), "a signalled provider is a failed run"); + + let observed = harness_state::read(&record, None).unwrap(); + assert_eq!(observed.state, Activity::Ended); + assert_eq!(observed.exit.as_deref(), Some("signal 9")); + } + + #[test] + fn a_clean_provider_exit_writes_the_terminal_record() { + let tmp = tempfile::tempdir().unwrap(); + let presence = status::status_path(tmp.path()); + let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude"); + let stop = AtomicBool::new(false); + + run_provider( + "Claude", + &presence, + &["true".into()], + &[], + Duration::from_millis(25), + Duration::from_millis(5), + &stop, + Some(&observer), + ) + .unwrap(); + + let observed = harness_state::read(&harness_state_path(tmp.path()), None).unwrap(); + assert_eq!(observed.state, Activity::Ended); + assert_eq!(observed.exit.as_deref(), Some("exit 0")); + } } diff --git a/src/hooks.rs b/src/hooks.rs index e741488e..c85ca939 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -20,6 +20,7 @@ const CODEX_STOP: &[u8] = include_bytes!("../hooks/codex-stop.sh"); const CLAUDE_SESSION_START: &[u8] = include_bytes!("../hooks/claude-session-start.sh"); const CLAUDE_PRE_COMPACT: &[u8] = include_bytes!("../hooks/claude-pre-compact.sh"); const CLAUDE_STOP_FAILURE: &[u8] = include_bytes!("../hooks/claude-stop-failure.sh"); +const CLAUDE_OBSERVE: &[u8] = include_bytes!("../hooks/claude-observe.sh"); // pi has no lifecycle-hook mechanism of its own; an extension is where a pi session exposes the // same surface, so it is published and verified as part of the same immutable set. const PI_CHANNEL: &[u8] = include_bytes!("../hooks/pi-channel.ts"); @@ -28,13 +29,14 @@ const SCHEMA: u32 = 1; const RECEIPT_FILE: &str = "current.json"; const SET_MANIFEST_FILE: &str = "manifest.json"; const SETS_DIR: &str = "sets"; -const HOOKS: [(&str, &[u8]); 7] = [ +const HOOKS: [(&str, &[u8]); 8] = [ ("codex-session-start.sh", CODEX_SESSION_START), ("codex-pre-compact.sh", CODEX_PRE_COMPACT), ("codex-stop.sh", CODEX_STOP), ("claude-session-start.sh", CLAUDE_SESSION_START), ("claude-pre-compact.sh", CLAUDE_PRE_COMPACT), ("claude-stop-failure.sh", CLAUDE_STOP_FAILURE), + ("claude-observe.sh", CLAUDE_OBSERVE), ("pi-channel.ts", PI_CHANNEL), ]; static TEMP_COUNTER: AtomicU64 = AtomicU64::new(0); diff --git a/src/main.rs b/src/main.rs index 11586395..74f0875d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -351,6 +351,14 @@ enum DriverCmd { #[arg(required = true, trailing_var_arg = true, allow_hyphen_values = true)] argv: Vec, }, + /// Apply one Claude hook event (payload on stdin) to observed harness state. + ClaudeObserve { + #[arg(long)] + identity: String, + /// The Claude hook event name, e.g. `Stop` or `PermissionRequest`. + #[arg(long)] + event: String, + }, /// Run pi under the session-owned presence wrapper. PiSession { #[arg(long)] @@ -1053,6 +1061,11 @@ fn main() -> Result<()> { let catalog = catalog.canonicalize().unwrap_or(catalog); st2::claude_session::run(&catalog, identity, runtime_id, argv) } + Command::Driver(DriverCmd::ClaudeObserve { identity, event }) => { + let catalog = catalog_arg(None)?; + let catalog = catalog.canonicalize().unwrap_or(catalog); + st2::claude_session::run_observe(&catalog, &identity, &event) + } Command::Driver(DriverCmd::Expand { spec, agent, host }) => { let catalog = catalog_arg(None)?; driver_expand_cmd(&catalog, &spec, agent.as_deref(), host.as_deref()) diff --git a/src/pi_session.rs b/src/pi_session.rs index 551d253c..683012e0 100644 --- a/src/pi_session.rs +++ b/src/pi_session.rs @@ -118,6 +118,7 @@ pub fn run( status::STATUS_REFRESH, PROVIDER_POLL, &STOP, + None, ) .with_context(|| format!("running pi driver '{runtime_id}'"))?; record_session_end(&agent_dir, &identity, &runtime_id, &session, seq, &outcome); @@ -242,6 +243,7 @@ mod tests { Duration::from_millis(10), Duration::from_millis(5), &stop, + None, ) .unwrap(); diff --git a/src/provider_session.rs b/src/provider_session.rs index 44933278..a0acd3c8 100644 --- a/src/provider_session.rs +++ b/src/provider_session.rs @@ -7,8 +7,8 @@ //! exact child remains alive. Each harness module keeps only what is genuinely harness-specific: //! how its provider argv is assembled and what environment the harness needs to reach st2 back. -use std::os::unix::process::CommandExt as _; -use std::path::Path; +use std::os::unix::process::{CommandExt as _, ExitStatusExt as _}; +use std::path::{Path, PathBuf}; use std::process::{Child, Command, ExitStatus, Stdio}; use std::sync::atomic::{AtomicBool, Ordering}; use std::thread; @@ -16,7 +16,7 @@ use std::time::{Duration, Instant}; use anyhow::{Context as _, Result}; -use crate::status; +use crate::{harness_state, status}; pub(crate) const PROVIDER_POLL: Duration = Duration::from_millis(250); const STOP_GRACE: Duration = Duration::from_secs(5); @@ -52,9 +52,58 @@ pub(crate) enum ProviderOutcome { Stopped(Option), } +/// The observed-harness-state handle a wrapper threads through its poll loop. Every operation +/// constructs a fresh writer over the on-disk record, so the wrapper re-stamps or terminates +/// whatever state a hook process wrote in between and never clobbers a fresher observation. +pub(crate) struct SessionObserver { + agent_dir: PathBuf, + identity: String, + harness: &'static str, +} + +impl SessionObserver { + pub(crate) fn new(agent_dir: &Path, identity: &str, harness: &'static str) -> Self { + Self { + agent_dir: agent_dir.to_path_buf(), + identity: identity.to_string(), + harness, + } + } + + fn writer(&self) -> harness_state::Writer { + harness_state::Writer::new( + &self.agent_dir, + &self.identity, + self.harness, + Some(self.identity.clone()), + ) + } + + /// Re-stamp whatever live state is on disk. The wrapper's evidence is the provider child it is + /// polling, so this is called only while that child is alive. + pub(crate) fn heartbeat(&self) { + let _ = self.writer().heartbeat(); + } + + /// Best-effort terminal record; observation must never turn a clean teardown into an error. + pub(crate) fn ended(&self, exit: &str) { + let _ = self.writer().ended(exit); + } +} + +fn describe_exit(exit: ExitStatus) -> String { + match (exit.code(), exit.signal()) { + (Some(code), _) => format!("exit {code}"), + (None, Some(signal)) => format!("signal {signal}"), + (None, None) => "exit unknown".to_string(), + } +} + /// Run one interactive provider in this wrapper's terminal process group, refreshing presence on /// `refresh_interval` for exactly as long as the spawned child lives. Fails on a nonzero exit; -/// wrappers that need the exit itself use [`run_provider_observed`]. +/// wrappers that need the exit itself use [`run_provider_observed`]. With an observer, the +/// terminal record lands on every exit path this process survives. +#[allow(clippy::too_many_arguments)] pub(crate) fn run_provider( provider: &str, status_path: &Path, @@ -63,6 +112,7 @@ pub(crate) fn run_provider( refresh_interval: Duration, poll: Duration, stop: &AtomicBool, + observed: Option<&SessionObserver>, ) -> Result<()> { match run_provider_observed( provider, @@ -72,14 +122,23 @@ pub(crate) fn run_provider( refresh_interval, poll, stop, + observed, )? { - ProviderOutcome::Exited(exit) => completed_provider(provider, exit), + ProviderOutcome::Exited(exit) => { + if let Some(observed) = observed { + observed.ended(&describe_exit(exit)); + } + completed_provider(provider, exit) + } ProviderOutcome::Stopped(_) => Ok(()), } } /// [`run_provider`], but reporting how the session ended instead of judging it, so a wrapper can -/// record a terminal observation before deciding what the exit means. +/// record its own terminal observation before deciding what the exit means. The stop path still +/// writes the observer's terminal record in-line, because after SIGKILL escalation no caller code +/// is guaranteed to run. +#[allow(clippy::too_many_arguments)] pub(crate) fn run_provider_observed( provider: &str, status_path: &Path, @@ -88,6 +147,7 @@ pub(crate) fn run_provider_observed( refresh_interval: Duration, poll: Duration, stop: &AtomicBool, + observed: Option<&SessionObserver>, ) -> Result { let (program, args) = argv .split_first() @@ -114,7 +174,7 @@ pub(crate) fn run_provider_observed( let mut next_refresh = Instant::now(); loop { if stop.load(Ordering::SeqCst) { - return stop_provider_group(&mut child).map(ProviderOutcome::Stopped); + return stop_provider_group(&mut child, observed).map(ProviderOutcome::Stopped); } if let Some(exit) = child .try_wait() @@ -125,6 +185,9 @@ pub(crate) fn run_provider_observed( let now = Instant::now(); if now >= next_refresh { let _ = status::refresh(status_path); + if let Some(observed) = observed { + observed.heartbeat(); + } next_refresh = now + refresh_interval; } thread::sleep(poll.min(next_refresh.saturating_duration_since(Instant::now()))); @@ -136,7 +199,10 @@ fn completed_provider(provider: &str, exit: ExitStatus) -> Result<()> { Ok(()) } -fn stop_provider_group(child: &mut Child) -> Result> { +fn stop_provider_group( + child: &mut Child, + observed: Option<&SessionObserver>, +) -> Result> { let process_group = unsafe { libc::getpgrp() }; anyhow::ensure!( process_group > 1, @@ -148,10 +214,19 @@ fn stop_provider_group(child: &mut Child) -> Result> { let deadline = Instant::now() + STOP_GRACE; while Instant::now() < deadline { if let Some(exit) = child.try_wait()? { + if let Some(observed) = observed { + observed.ended(&describe_exit(exit)); + } return Ok(Some(exit)); } thread::sleep(Duration::from_millis(25)); } + // The escalation SIGKILLs this wrapper's own process group, so the wrapper dies with the + // provider and nothing after the kill is guaranteed to run. The terminal record must land + // first: a liveness record that stops being written is still being read. + if let Some(observed) = observed { + observed.ended("signal 9"); + } unsafe { libc::kill(-process_group, libc::SIGKILL); } From 80c0790f61716056449a5ba7ffaf866029739fff Mon Sep 17 00:00:00 2001 From: Johannes Schickling Date: Sun, 23 Aug 2026 21:09:25 +0200 Subject: [PATCH 02/14] fix(claude): pinned session-start observer, runtime-id sessions, and catalog-first hook resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review-pass fixes: SessionObserver pins one session-start timestamp so its per-tick fresh writers never re-stamp a predecessor session's record; ptySession carries the wrapper's runtime ID end to end (wrapper env ST2_CLAUDE_RUNTIME_ID -> hook script -> claude-observe --runtime-id), aliasing the identity only on driver-expanded seats; and the hook script resolves --catalog CATALOG-first — its sibling hooks' ST_ROOT-first order is for bus writes, and under a custom bus root declaration resolution would silently drop every transition. Co-Authored-By: Claude Fable 5 --- hooks/claude-observe.sh | 11 ++++++++--- src/claude_session.rs | 26 +++++++++++++++++++------- src/main.rs | 11 +++++++++-- src/pi_session.rs | 1 + src/provider_session.rs | 18 ++++++++++++++++-- 5 files changed, 53 insertions(+), 14 deletions(-) diff --git a/hooks/claude-observe.sh b/hooks/claude-observe.sh index 620b49da..d3bc2677 100755 --- a/hooks/claude-observe.sh +++ b/hooks/claude-observe.sh @@ -6,11 +6,16 @@ set -u event="${1:-}" identity="${ST_AGENT:-}" -root="${ST_ROOT:-${CATALOG:-}}" +# CATALOG-first, deliberately diverging from the sibling hooks' ST_ROOT-first order: their +# ST_ROOT is a bus root for message writes, while --catalog here resolves the agent DECLARATION — +# with a custom bus root (ST_ROOT != CATALOG) declaration resolution under ST_ROOT finds nothing +# and every transition would silently drop. +root="${CATALOG:-${ST_ROOT:-}}" +runtime_id="${ST2_CLAUDE_RUNTIME_ID:-$identity}" if [[ -z "$event" || -z "$identity" || -z "$root" ]] || ! command -v st2 >/dev/null 2>&1; then exit 0 fi -st2 --catalog "$root" driver claude-observe --identity "$identity" --event "$event" \ - >/dev/null 2>&1 || true +st2 --catalog "$root" driver claude-observe --identity "$identity" --runtime-id "$runtime_id" \ + --event "$event" >/dev/null 2>&1 || true exit 0 diff --git a/src/claude_session.rs b/src/claude_session.rs index d8f052af..b56bb233 100644 --- a/src/claude_session.rs +++ b/src/claude_session.rs @@ -36,12 +36,15 @@ pub fn run( "Claude driver '{runtime_id}' has no provider argv" ); install_signal_handler(); - let observer = SessionObserver::new(&agent_dir, &identity, "claude"); + let observer = SessionObserver::new(&agent_dir, &identity, "claude", &runtime_id); + // The runtime ID reaches hook subprocesses through the provider environment, so their + // transitions carry the same pty session the wrapper's records do. + let env = [(RUNTIME_ID_ENV.to_string(), runtime_id.clone())]; run_provider( "Claude", &status::status_path(&agent_dir), &claude_argv, - &[], + &env, status::STATUS_REFRESH, PROVIDER_POLL, &STOP, @@ -54,7 +57,15 @@ pub fn run( /// /// Invoked per event by the fail-open `claude-observe.sh` hook, so each invocation is its own /// short-lived writer; the transition counter continues from disk. -pub fn run_observe(catalog_root: &Path, identity: &str, event: &str) -> Result<()> { +/// The env var carrying the wrapper's runtime/task ID into Claude's hook subprocesses. +pub const RUNTIME_ID_ENV: &str = "ST2_CLAUDE_RUNTIME_ID"; + +pub fn run_observe( + catalog_root: &Path, + identity: &str, + runtime_id: Option<&str>, + event: &str, +) -> Result<()> { let agent_dir = message::resolve_agent_dir(catalog_root, identity, &crate::run::detect_host())? .with_context(|| format!("Claude driver agent '{identity}' is not declared"))?; let mut raw = String::new(); @@ -63,7 +74,8 @@ pub fn run_observe(catalog_root: &Path, identity: &str, event: &str) -> Result<( let Some(observation) = observe_hook_event(event, &payload) else { return Ok(()); }; - harness_state::Writer::new(&agent_dir, identity, "claude", Some(identity.to_string())) + let pty_session = runtime_id.unwrap_or(identity).to_string(); + harness_state::Writer::new(&agent_dir, identity, "claude", Some(pty_session)) .observe(observation) } @@ -197,7 +209,7 @@ mod tests { fn wrapper_heartbeat_re_stamps_without_clobbering_hook_written_state() { let tmp = tempfile::tempdir().unwrap(); let record = harness_state_path(tmp.path()); - let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude"); + let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker"); // A hook process wrote a blocked observation between wrapper ticks. harness_state::Writer::new(tmp.path(), "hetz.worker", "claude", None) @@ -220,7 +232,7 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); let presence = status::status_path(tmp.path()); let record = harness_state_path(tmp.path()); - let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude"); + let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker"); let stop = AtomicBool::new(false); // A turn is in flight when the provider dies by signal. @@ -249,7 +261,7 @@ mod tests { fn a_clean_provider_exit_writes_the_terminal_record() { let tmp = tempfile::tempdir().unwrap(); let presence = status::status_path(tmp.path()); - let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude"); + let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker"); let stop = AtomicBool::new(false); run_provider( diff --git a/src/main.rs b/src/main.rs index 74f0875d..0b03825d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -355,6 +355,9 @@ enum DriverCmd { ClaudeObserve { #[arg(long)] identity: String, + /// The wrapper's runtime/task ID; the record's pty session. Defaults to the identity. + #[arg(long)] + runtime_id: Option, /// The Claude hook event name, e.g. `Stop` or `PermissionRequest`. #[arg(long)] event: String, @@ -1061,10 +1064,14 @@ fn main() -> Result<()> { let catalog = catalog.canonicalize().unwrap_or(catalog); st2::claude_session::run(&catalog, identity, runtime_id, argv) } - Command::Driver(DriverCmd::ClaudeObserve { identity, event }) => { + Command::Driver(DriverCmd::ClaudeObserve { + identity, + runtime_id, + event, + }) => { let catalog = catalog_arg(None)?; let catalog = catalog.canonicalize().unwrap_or(catalog); - st2::claude_session::run_observe(&catalog, &identity, &event) + st2::claude_session::run_observe(&catalog, &identity, runtime_id.as_deref(), &event) } Command::Driver(DriverCmd::Expand { spec, agent, host }) => { let catalog = catalog_arg(None)?; diff --git a/src/pi_session.rs b/src/pi_session.rs index 683012e0..80da46ff 100644 --- a/src/pi_session.rs +++ b/src/pi_session.rs @@ -317,6 +317,7 @@ mod tests { Duration::from_secs(60), Duration::from_millis(5), &stop, + None, ) .unwrap(); match outcome { diff --git a/src/provider_session.rs b/src/provider_session.rs index a0acd3c8..8d7da2e8 100644 --- a/src/provider_session.rs +++ b/src/provider_session.rs @@ -59,14 +59,27 @@ pub(crate) struct SessionObserver { agent_dir: PathBuf, identity: String, harness: &'static str, + pty_session: String, + session_start_ms: u64, } impl SessionObserver { - pub(crate) fn new(agent_dir: &Path, identity: &str, harness: &'static str) -> Self { + /// `pty_session` is the wrapper's runtime/task ID — the registry entry whose liveness vouches + /// for the record. The session start is pinned once here so every per-operation fresh writer + /// agrees where this session began: a predecessor session's record stays heartbeat-ineligible + /// until something of this session is observed. + pub(crate) fn new( + agent_dir: &Path, + identity: &str, + harness: &'static str, + pty_session: &str, + ) -> Self { Self { agent_dir: agent_dir.to_path_buf(), identity: identity.to_string(), harness, + pty_session: pty_session.to_string(), + session_start_ms: crate::message::now_ms(), } } @@ -75,8 +88,9 @@ impl SessionObserver { &self.agent_dir, &self.identity, self.harness, - Some(self.identity.clone()), + Some(self.pty_session.clone()), ) + .session_started_at(self.session_start_ms) } /// Re-stamp whatever live state is on disk. The wrapper's evidence is the provider child it is From 163922d47e2f9598150818b4857e86406b43765c Mon Sep 17 00:00:00 2001 From: Johannes Schickling Date: Sun, 23 Aug 2026 20:31:44 +0200 Subject: [PATCH 03/14] docs(vrs): resolve DQ-H1 with the measured batched-permission capture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude Code 2.1.237 serializes tool execution around an open permission prompt — no hook event fires while a prompt is up, in either batch ordering — so the shipped blocked-exit rule is correct, not merely conservative. The residual limit is the eventless deny path. The measured grant sequence is replayed verbatim as a mapping fixture, and the phantom SubagentStop's emergent shape is pinned from this build. Co-Authored-By: Claude Fable 5 --- .../2026-08-23-claude-batched-permission.md | 83 +++++++++++++++++++ docs/vrs/05-harness-state/open-questions.md | 50 ++++++----- src/claude_session.rs | 65 ++++++++++++++- 3 files changed, 175 insertions(+), 23 deletions(-) create mode 100644 docs/vrs/05-harness-state/.experiments/2026-08-23-claude-batched-permission.md diff --git a/docs/vrs/05-harness-state/.experiments/2026-08-23-claude-batched-permission.md b/docs/vrs/05-harness-state/.experiments/2026-08-23-claude-batched-permission.md new file mode 100644 index 00000000..33830528 --- /dev/null +++ b/docs/vrs/05-harness-state/.experiments/2026-08-23-claude-batched-permission.md @@ -0,0 +1,83 @@ +# Claude batched tool calls under an open permission prompt + +Date: 2026-08-23. Binary: Claude Code 2.1.237 (Fable 5 bundle), Linux, driven under `pty`. +Purpose: take the capture DQ-H1 (and #268 §C) said was missing — a batch where one call needs +permission — and settle whether the shipped blocked-exit rule +(`src/claude_session.rs::observe_hook_event`: clear on next `PreToolUse`/`PostToolUse`/`Stop`) +false-clears while a prompt is open. + +## Method + +A scratch project (isolated CWD; hooks are project-scoped) with `.claude/settings.local.json` +allowlisting `Bash(echo:*)` and registering logging hooks that append every payload as one JSON +line and always exit 0, for: `PreToolUse`, `PostToolUse`, `PermissionRequest`, `Stop`, +`SubagentStop` — later runs added `PermissionDenied`, `UserPromptSubmit`, `SessionEnd`. + +- Runs 1–2: `claude -p` (non-interactive), 120–180 s timeouts. +- Runs 3–4: interactive `claude --permission-mode default` in a detached ephemeral `pty` session + (`pty run -d -e --id --cwd --env LOGFILE=… -- claude --permission-mode default`), + prompts sent with `pty send --seq … --seq key:return`, screen sampled with `pty peek --plain`, + the log dumped **while the prompt was open** and again after answering. + +Four short turns total, ≈$0.61. + +## Measured sequences + +**Run 2, `-p`, batch allowlisted-first** (`echo a` + `touch scratch-file.txt`): + +``` +PreToolUse Bash "echo a" tool_use_id=toolu_014yt… +PostToolUse Bash "echo a" (before the second call starts) +PreToolUse Bash "touch scratch-file.txt" tool_use_id=toolu_01RrZ… +PreToolUse Write … (fallback attempt, then waits) +Stop +``` + +No `PermissionRequest` fires in `-p` mode; the non-interactive path blocks/denies without one. +Execution is strictly serial: the first call's `Post` precedes the second call's `Pre`. + +**Run 3, interactive, batch permission-first** (`touch scratch2.txt` + allowlisted `echo b`): + +``` +481.34 PreToolUse Bash "touch scratch2.txt" tool_use_id=toolu_01HK5… prompt_id=0ea832de… +481.67 PermissionRequest Bash "touch scratch2.txt" NO tool_use_id prompt_id=0ea832de… + — prompt open 33 s; log dumped during: NO further event of any kind, + although the TUI already rendered the batched `echo b` — +514.10 PostToolUse Bash "touch scratch2.txt" tool_use_id=toolu_01HK5… (the grant) +514.30 PreToolUse Bash "echo b" tool_use_id=toolu_01NhR… +514.59 PostToolUse Bash "echo b" +516.17 Stop +519.44 SubagentStop agent_id="a5c61ec4ef268c3cc" agent_type="" (phantom, +3.3 s) +``` + +**Runs 3b/4, deny path** ("3. No" on the prompt, run 4 with `PermissionDenied` + `SessionEnd` +registered): after `PreToolUse` + `PermissionRequest`, denial produced **zero further events** — +no `PostToolUse`, no `Stop`, no `PermissionDenied`. The turn ends silently. + +## Findings + +1. **Execution serializes around an open permission prompt.** In both orderings no hook event + fires while a prompt is up — a parallel-batched allowlisted call waits out the grant. The + false-clear #268 §C predicted (first call's `Post` clearing `blocked` during the second call's + prompt) is unobservable in this build: the next `Pre`/`Post`/`Stop` after `PermissionRequest` + is always the blocked call's own resolution. The shipped exit rule is correct, not merely + conservative. +2. **`PermissionRequest` still carries no call identity.** `tool_use_id` is present on + `PreToolUse`/`PostToolUse` but absent on `PermissionRequest`; its `prompt_id` is turn-scoped + (identical across every event of the turn) and cannot correlate a call. +3. **Denial is eventless.** Even with `PermissionDenied` registered, "No" ends the turn with no + event, so `blockedOn: human` stands until the next `UserPromptSubmit`/`SessionStart`. Half-true + semantically (a person's direction is still what the session waits on), but the state axis + reads `active` while the model is stopped. Pinned as DQ-H1's residual limit. +4. **The phantom `SubagentStop` reproduces**: 3.3 s after `Stop`, `agent_id` non-empty, + `agent_type` the empty string, no subagent ran — the guard's emergent-property basis holds in + this build. +5. `-p` mode never fires `PermissionRequest`; permission evidence exists only interactively. + +## VRS Impact + +- DQ-H1 re-resolved: the batched-capture gate is met; the exit rule stands on measured ground and + the open question narrows to the eventless deny window. +- The measured grant sequence is replayed verbatim by + `src/claude_session.rs::measured_batched_grant_sequence_holds_blocked_until_the_granted_calls_own_post`. +- No requirement text changes: OHS-R05's producer rule is confirmed, not amended. diff --git a/docs/vrs/05-harness-state/open-questions.md b/docs/vrs/05-harness-state/open-questions.md index 9e9d32ec..2caf89e4 100644 --- a/docs/vrs/05-harness-state/open-questions.md +++ b/docs/vrs/05-harness-state/open-questions.md @@ -4,16 +4,27 @@ Each entry links a spec `DQ-H*`. Questions leave this file when resolved — into [spec.md](./spec.md) as decisions or `.experiments/` as tested hypotheses. -- **DQ-H1 Claude blocked-exit edge.** `PermissionRequest` carries no - `tool_use_id`, so leaving `blockedOn: human` can only match on tool *name* — - and Claude batches tool calls (`PostToolBatch` carries plural `tool_calls`), - so the first call's `PostToolUse` would clear `blocked` while the human still - faces the second call's prompt. The corpus enters `blocked` in 2 of 9 - captures and exits in 1, with one tool and no batching: a rule validated on - a single exit path is not validated. Until then the producer holds - `blockedOn: human` until turn end (`Stop`) rather than encoding an exit rule - that cannot hold. Resolves by: a capture with batched tool calls where the - second call needs permission, then specifying the exit edge against it. +- **DQ-H1 Claude blocked-exit edge.** The batched capture #268 §C asked for + was taken on 2026-08-23 against Claude Code 2.1.237 + (`.experiments/2026-08-23-claude-batched-permission.md`), and it resolves + the predicted false-clear in the rule's favor: **tool execution is + serialized around an open permission prompt.** In both batch orderings + (allowlisted-first and permission-first), no hook event of any kind fires + while the prompt is open — a parallel-batched allowlisted call renders on + screen but its `PreToolUse` waits 33 s for the grant — so the next + `PreToolUse`/`PostToolUse`/`Stop` after `PermissionRequest` is precisely the + blocked call's own resolution, and the shipped rule + (`src/claude_session.rs::observe_hook_event`) is correct, not merely + conservative. `PermissionRequest` still carries no `tool_use_id` (its + `prompt_id` is turn-scoped, shared by every event in the turn — not a call + correlator). The residual limit is the **deny path**: selecting "No" ends + the turn with *zero* further events — no `PostToolUse`, no `Stop`, and no + `PermissionDenied` even when that hook is registered — so `blockedOn: human` + stands until the next `UserPromptSubmit` or `SessionStart`. That reading is + semantically half-true (a human's direction is still what the session + waits on) and it under-reports nothing, but the state axis says `active` + while the model is not running. Resolves by: a Claude build whose denial + emits any hook event; until then the deny window is the pinned limit. - **DQ-H2 Transport cost of per-transition writes.** Presence refreshes every five minutes; turn boundaries are far more frequent, and burst coalescing measured 4 transitions per turn 0.1–0.4 ms apart. No measurement establishes @@ -48,12 +59,13 @@ hypotheses. `unknown` is no fresh observation, never proof of ill health, and never gates local work. Resolves by: specifying remote-reader semantics with a proof, or explicitly scoping the record same-host advisory. -- **DQ-H6 OpenCode state source.** The producer needs a verified source. - Candidate: OpenCode's server/SDK event surface (session state, message - lifecycle); fallback: the shipped composer adapter's positive markers - (`ctrl+p commands` footer) with documented limits. Nothing is measured yet — - the DING adapter (#313) proves only composer classification. Resolves by: - an `.experiments/` capture of OpenCode's event surface on a pinned version, - then choosing the source and its skew policy — the repo's standing rule - (pin where version skew fails silently, as `checks.pi-extension-types` and - `SUPPORTED_CODEX_CLI_VERSIONS` do) applies. +- **DQ-H6 OpenCode blocked-entry capture.** The state source itself is + resolved: the server's SSE event surface, measured on 1.18.19 + (`.experiments/2026-08-23-opencode-surface.md`) and gated by + `SUPPORTED_OPENCODE_VERSIONS` plus the live `/doc` subset check. What + remains open is the blocked-on-human pair: `permission.asked` / + `permission.replied` are schema-backed with explicit `^per` ids — the exit + edge is clean by construction, unlike Claude's — but no live capture of a + real permission prompt exists (headless runs with `{"bash":"ask"}` never + asked). Resolves by: one capture from a TUI seat with a real permission + prompt, confirming the events fire and carry the id the producer matches. diff --git a/src/claude_session.rs b/src/claude_session.rs index b56bb233..d3a9fa17 100644 --- a/src/claude_session.rs +++ b/src/claude_session.rs @@ -83,10 +83,13 @@ pub fn run_observe( /// top-level harness state. /// /// Claude gives no call identity on the event that enters `blocked` (`PermissionRequest` carries -/// no `tool_use_id`), so the exit edge is the next `PreToolUse`/`PostToolUse`/`Stop`. Under -/// batched tool calls that can clear `blocked` while a later call in the batch still holds a -/// prompt; the limit is accepted and fenced in the VRS rather than papered over with a temporal -/// heuristic that fails for exactly the parallel case it would need to handle. +/// no `tool_use_id`; its `prompt_id` is turn-scoped), so the exit edge is the next +/// `PreToolUse`/`PostToolUse`/`Stop`. Measured 2026-08-23 (Claude Code 2.1.237, DQ-H1): tool +/// execution serializes around an open permission prompt — no hook event fires while a prompt is +/// up, even for a parallel-batched allowlisted call — so that next event is the blocked call's own +/// resolution and the batched false-clear #268 §C predicted cannot occur. The residual limit is +/// denial: "No" ends the turn with zero further events (no Stop, and no PermissionDenied even when +/// registered), so `blocked` stands until the next `UserPromptSubmit`/`SessionStart`. pub fn observe_hook_event(event: &str, payload: &serde_json::Value) -> Option { // Any event carrying an agent identity is a subagent's and must never move top-level state: // a phantom `SubagentStop` trails every completed turn, 1.5-2.9s after `Stop`. That phantom @@ -205,6 +208,60 @@ mod tests { assert!(observe_hook_event("Stop", &top).is_some()); } + /// The measured grant-path sequence from the DQ-H1 capture (2026-08-23, Claude Code 2.1.237, + /// `docs/vrs/05-harness-state/.experiments/2026-08-23-claude-batched-permission.md`): in a + /// two-call batch where the first call needs permission, execution serializes around the open + /// prompt, so the event after `PermissionRequest` is the granted call's own `PostToolUse` and + /// the exit rule clears `blocked` at exactly the right moment. Replayed verbatim so a future + /// mapping change that breaks the measured sequence fails here, not in the field. + #[test] + fn measured_batched_grant_sequence_holds_blocked_until_the_granted_calls_own_post() { + let pre_touch = serde_json::json!({ + "hook_event_name": "PreToolUse", "tool_name": "Bash", + "tool_input": {"command": "touch scratch2.txt"}, + "tool_use_id": "toolu_01HK5aLKavjdbrCk48cfd58k", + "prompt_id": "0ea832de-ece1-4575-900f-4dab5e2f6849", + }); + let permission_request = serde_json::json!({ + "hook_event_name": "PermissionRequest", "tool_name": "Bash", + "tool_input": {"command": "touch scratch2.txt"}, + "permission_suggestions": [], + "prompt_id": "0ea832de-ece1-4575-900f-4dab5e2f6849", + }); + let post_touch = serde_json::json!({ + "hook_event_name": "PostToolUse", "tool_name": "Bash", + "tool_input": {"command": "touch scratch2.txt"}, + "tool_use_id": "toolu_01HK5aLkavjdbrCk48cfd58k", + "prompt_id": "0ea832de-ece1-4575-900f-4dab5e2f6849", + }); + // The phantom SubagentStop trailing the turn: non-empty agent_id, EMPTY agent_type, no + // subagent ran — the exact emergent shape the guard keys on, reproduced in this build. + let phantom_subagent_stop = serde_json::json!({ + "hook_event_name": "SubagentStop", + "agent_id": "a5c61ec4ef268c3cc", "agent_type": "", + "prompt_id": "0ea832de-ece1-4575-900f-4dab5e2f6849", + }); + + let entered = observe_hook_event("PermissionRequest", &permission_request).unwrap(); + assert_eq!( + (entered.state, entered.blocked_on), + (Activity::Active, BlockedOn::Human) + ); + // 33 s of open prompt produced no intervening event in the capture; the very next event + // is the granted call's own PostToolUse, which correctly releases the block. + let released = observe_hook_event("PostToolUse", &post_touch).unwrap(); + assert_eq!( + (released.state, released.blocked_on), + (Activity::Active, BlockedOn::None) + ); + let _ = observe_hook_event("PreToolUse", &pre_touch); + assert_eq!( + observe_hook_event("SubagentStop", &phantom_subagent_stop), + None, + "the phantom SubagentStop must not resurrect activity after Stop" + ); + } + #[test] fn wrapper_heartbeat_re_stamps_without_clobbering_hook_written_state() { let tmp = tempfile::tempdir().unwrap(); From e00170c77da6c5eb248ed682cad1afab9f7a366a Mon Sep 17 00:00:00 2001 From: Johannes Schickling Date: Sun, 23 Aug 2026 20:36:26 +0200 Subject: [PATCH 04/14] feat(claude): render the canonical hook registration from driver expansion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Driver-declared Claude seats now receive the same .claude/settings.local.json hook registration a hand-authored seat carries, closing the gap where they had no observed-state producer and no lifecycle hooks at all. The registration has one canonical shape (hooks::claude_settings_registration) shared by expansion and pinned against the maintained example declaration by a consistency test, so the two surfaces cannot drift. The materializer admits the new destination verbatim — its $ST_HOOKS references are render variables, not executable paths — and driver seats now meet the same verified-hook-set gate hand-authored $ST_HOOKS renders always met. Render output is not part of the launch fingerprint, so live seats converge without disruption on their next materialization and session start. Co-Authored-By: Claude Fable 5 --- docs/vrs/05-harness-state/spec.md | 25 ++++++--- src/driver.rs | 29 ++++++++--- src/eval_run.rs | 6 +++ src/hooks.rs | 78 ++++++++++++++++++++++++++++ src/materialize.rs | 6 +++ tests/driver_expansion.rs | 5 ++ tests/fixtures/driver/claude.out.kdl | 1 + 7 files changed, 137 insertions(+), 13 deletions(-) diff --git a/docs/vrs/05-harness-state/spec.md b/docs/vrs/05-harness-state/spec.md index 9ad8dfeb..ef399467 100644 --- a/docs/vrs/05-harness-state/spec.md +++ b/docs/vrs/05-harness-state/spec.md @@ -167,13 +167,24 @@ classified from the payload's `tool_name` (`AskUserQuestion` → `question`, anything else → `permission`) — (its meaning is specifically "a human is about to be asked" — it fires only under permission modes that ask). Events carrying `agent_id` are subagent-nested and never move -top-level state. The blocked *exit* edge under batched tool calls is -unspecified until a capture proves a rule (`DQ-H1`); until then the spec -states the limit rather than a rule that cannot hold. The wrapper side owns -liveness: it re-stamps the heartbeat on its existing presence cadence while -the child is alive, and writes the terminal record from its `try_wait` reap -and its SIGTERM path — before any SIGKILL escalation into its own process -group, which no in-process write survives (OHS-T04). +top-level state. The blocked *exit* edge is the next `PreToolUse`, +`PostToolUse`, or `Stop` — measured-correct, not merely conservative: the +2026-08-23 batched-permission capture (`DQ-H1`) shows tool execution +serializes around an open permission prompt, so no event can clear the +blocked state early. The residual limit is the eventless deny path pinned in +`DQ-H1`. The wrapper side owns liveness: it re-stamps the heartbeat on its +existing presence cadence while the child is alive — through a fresh writer +each time, so it never clobbers a state a hook process wrote in between — +and writes the terminal record from its `try_wait` reap and its SIGTERM +path, before any SIGKILL escalation into its own process group, which no +in-process write survives (OHS-T04). Hook registration has one canonical +shape (`hooks::claude_settings_registration`): the maintained example +declaration (`examples/native/agent-claude.kdl`) carries it by hand, and +`expand_claude` renders the same `.claude/settings.local.json` upsert for +driver-declared seats, so both surfaces register identical hooks and a test +fails if they drift. A live seat converges without disruption: render output +is not part of the launch fingerprint, the merged settings land on the next +materialization pass, and the hooks take effect at the next session start. ## pi producer (OHS-R05, OHS-R08) diff --git a/src/driver.rs b/src/driver.rs index fe09648b..fac5671a 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -119,11 +119,17 @@ fn expand_claude(driver: &ClaudeDriver, bus_id: &str) -> Result { } }); let mcp = serde_json::to_string_pretty(&mcp)?; + // The same registration a hand-authored seat carries: without it a driver-declared + // seat has no observed-state producer and no lifecycle hooks at all. + let settings = serde_json::to_string_pretty(&crate::hooks::claude_settings_registration())?; let mut render = KdlNode::new("render"); - render.set_children(document([node( - "json-upsert", - vec![".mcp.json".to_string(), mcp], - )])); + render.set_children(document([ + node("json-upsert", vec![".mcp.json".to_string(), mcp]), + node( + "json-upsert", + vec![".claude/settings.local.json".to_string(), settings], + ), + ])); let mut provider = vec!["claude".to_string()]; if let Some(model) = &driver.model { @@ -263,8 +269,15 @@ mod tests { assert_eq!(output.nodes().len(), 2); let render = output.get("render").unwrap(); - let upsert = render.children().unwrap().get("json-upsert").unwrap(); - let upsert = strings(upsert); + let upserts: Vec<&KdlNode> = render + .children() + .unwrap() + .nodes() + .iter() + .filter(|node| node.name().value() == "json-upsert") + .collect(); + assert_eq!(upserts.len(), 2); + let upsert = strings(upserts[0]); assert_eq!(upsert[0], ".mcp.json"); let mcp: serde_json::Value = serde_json::from_str(upsert[1]).unwrap(); assert_eq!(mcp["mcpServers"]["st2"]["type"], "stdio"); @@ -280,6 +293,10 @@ mod tests { "host.worker" ]) ); + let settings = strings(upserts[1]); + assert_eq!(settings[0], ".claude/settings.local.json"); + let settings: serde_json::Value = serde_json::from_str(settings[1]).unwrap(); + assert_eq!(settings, crate::hooks::claude_settings_registration()); assert_eq!( strings(output.get("argv").unwrap()), [ diff --git a/src/eval_run.rs b/src/eval_run.rs index 53907a4d..bf1e4503 100644 --- a/src/eval_run.rs +++ b/src/eval_run.rs @@ -1709,6 +1709,12 @@ mod tests { fn canonical_eval_runtime_inventory_compiles_driver_launches() { let catalog = tempfile::tempdir().unwrap(); std::fs::create_dir_all(catalog.path().join("worker")).unwrap(); + // The Claude driver expansion registers `$ST_HOOKS` hooks, and materialization refuses an + // unverified hook set — the same contract an eval host satisfies with `st2 hooks install`. + // The root outlives the test (env stays set for the process), so it is kept, not dropped. + let hooks = tempfile::tempdir().unwrap().keep(); + crate::hooks::install_at(&hooks, false).unwrap(); + unsafe { std::env::set_var("ST_HOOKS", &hooks) }; write_eval_agent( catalog.path(), "agents/evalhost/worker/agent.kdl", diff --git a/src/hooks.rs b/src/hooks.rs index c85ca939..e7ea4760 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -103,6 +103,50 @@ pub fn hookset_id() -> String { expected_manifest().hookset } +/// The canonical Claude workspace hook registration: the exact +/// `.claude/settings.local.json` payload a maintained Claude seat carries. The +/// hand-authored example declaration and `expand_claude` both resolve to this one +/// shape, so a driver-declared seat and a hand-authored seat register identical +/// hooks and there is a single place a hook event can be added. +pub fn claude_settings_registration() -> serde_json::Value { + fn observe(event: &str) -> serde_json::Value { + serde_json::json!([{ "hooks": [{ + "type": "command", + "command": format!("$ST_HOOKS/claude-observe.sh {event}"), + }] }]) + } + serde_json::json!({ + "$schema": "https://json.schemastore.org/claude-code-settings.json", + "hooks": { + "SessionStart": [{ "hooks": [ + { + "type": "command", + "async": true, + "asyncRewake": true, + "command": "$ST_HOOKS/claude-session-start.sh", + }, + { + "type": "command", + "command": "$ST_HOOKS/claude-observe.sh SessionStart", + }, + ] }], + "PreCompact": [{ "hooks": [{ + "type": "command", + "command": "$ST_HOOKS/claude-pre-compact.sh", + }] }], + "StopFailure": [{ "hooks": [{ + "type": "command", + "command": "$ST_HOOKS/claude-stop-failure.sh", + }] }], + "UserPromptSubmit": observe("UserPromptSubmit"), + "Stop": observe("Stop"), + "PermissionRequest": observe("PermissionRequest"), + "PreToolUse": observe("PreToolUse"), + "PostToolUse": observe("PostToolUse"), + } + }) +} + /// Install-owned hook root. `$ST_HOOKS` can pin a scratch or custom state layout; otherwise use /// `$XDG_STATE_HOME/st2/hooks` or `~/.local/state/st2/hooks`. pub fn hooks_root() -> Result { @@ -515,6 +559,40 @@ pub fn install_at(root: &Path, replace: bool) -> Result { mod tests { use super::*; + /// The example declaration is the hand-authored half of the one canonical Claude hook + /// registration; expansion emits the other half from [`claude_settings_registration`]. If the + /// two drift, a driver-declared and a hand-authored seat stop registering the same hooks. + #[test] + fn example_claude_declaration_registers_the_canonical_hook_settings() { + let path = concat!( + env!("CARGO_MANIFEST_DIR"), + "/examples/native/agent-claude.kdl" + ); + let raw = std::fs::read_to_string(path).unwrap(); + let document: kdl::KdlDocument = raw.parse().unwrap(); + let agent = document.get("agent").unwrap(); + let render = agent.children().unwrap().get("render").unwrap(); + let content = render + .children() + .unwrap() + .nodes() + .iter() + .filter(|node| node.name().value() == "json-upsert") + .find_map(|node| { + let mut entries = node + .entries() + .iter() + .filter_map(|entry| match entry.value() { + kdl::KdlValue::String(value) => Some(value.as_str()), + _ => None, + }); + (entries.next() == Some(".claude/settings.local.json")).then(|| entries.next())? + }) + .expect("example declares the settings.local.json upsert"); + let registered: serde_json::Value = serde_json::from_str(content).unwrap(); + assert_eq!(registered, claude_settings_registration()); + } + /// The gate that holds a pi launch until the set is verified keys on this predicate, so a /// fencepost here silently ungates every pi agent. The `driver pi-session` shape is the one /// expansion actually emits. diff --git a/src/materialize.rs b/src/materialize.rs index b0cd6a97..aa51a037 100644 --- a/src/materialize.rs +++ b/src/materialize.rs @@ -271,6 +271,12 @@ fn resolve_driver_render_executable(plan: &mut RenderPlan, agent: &str) -> Resul else { continue; }; + // The hook registration ships verbatim: its `$ST_HOOKS` references are render + // variables the materializer resolves against the verified set, not an executable + // path this binary should rewrite. + if destination == ".claude/settings.local.json" { + continue; + } anyhow::ensure!( destination == ".mcp.json", "agent '{agent}' driver expansion produced an unexpected JSON destination" diff --git a/tests/driver_expansion.rs b/tests/driver_expansion.rs index ef3a830f..ee24fd80 100644 --- a/tests/driver_expansion.rs +++ b/tests/driver_expansion.rs @@ -250,6 +250,11 @@ fn claude_driver_matches_deliver_after_normalizing_the_legacy_command_namespace( ); assert_eq!(driver_task, legacy_task); + // The driver expansion registers `$ST_HOOKS` hooks, and materialization refuses an unverified + // hook set — install one in a scratch root, as `st2 hooks install` does on a real host. + let hooks = tempfile::tempdir().unwrap().keep(); + st2::hooks::install_at(&hooks, false).unwrap(); + unsafe { std::env::set_var("ST_HOOKS", &hooks) }; materialize_agent(&catalog, &legacy, "h").unwrap(); materialize_agent(&catalog, &driver, "h").unwrap(); let legacy_mcp: serde_json::Value = diff --git a/tests/fixtures/driver/claude.out.kdl b/tests/fixtures/driver/claude.out.kdl index 7cb3605c..48045bb4 100644 --- a/tests/fixtures/driver/claude.out.kdl +++ b/tests/fixtures/driver/claude.out.kdl @@ -1,4 +1,5 @@ render { json-upsert .mcp.json "{\n \"mcpServers\": {\n \"st2\": {\n \"args\": [\n \"--catalog\",\n \"$CATALOG\",\n \"driver\",\n \"claude-mcp\",\n \"--identity\",\n \"Silber.fabric\"\n ],\n \"command\": \"st2\",\n \"type\": \"stdio\"\n }\n }\n}" + json-upsert ".claude/settings.local.json" "{\n \"$schema\": \"https://json.schemastore.org/claude-code-settings.json\",\n \"hooks\": {\n \"PermissionRequest\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh PermissionRequest\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"PostToolUse\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh PostToolUse\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"PreCompact\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-pre-compact.sh\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"PreToolUse\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh PreToolUse\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"SessionStart\": [\n {\n \"hooks\": [\n {\n \"async\": true,\n \"asyncRewake\": true,\n \"command\": \"$ST_HOOKS/claude-session-start.sh\",\n \"type\": \"command\"\n },\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh SessionStart\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"Stop\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh Stop\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"StopFailure\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-stop-failure.sh\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"UserPromptSubmit\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh UserPromptSubmit\",\n \"type\": \"command\"\n }\n ]\n }\n ]\n }\n}" } argv st2 --catalog $CATALOG driver claude-session --identity Silber.fabric --runtime-id Silber.fabric -- claude --model opus --effort xhigh "--dangerously-load-development-channels=server:st2" --permission-mode bypassPermissions --model override "Start the assigned work." From 1240d1645f88af3f911c604a41675cfa7a2e1944 Mon Sep 17 00:00:00 2001 From: Johannes Schickling Date: Sun, 23 Aug 2026 22:10:41 +0200 Subject: [PATCH 05/14] fix(claude): union-merged hook registration, session-boundary starts, and ask classification json-upsert gains arrays="union" (exact-equality array union, default stays replace) so the generated hook registration joins user-declared hooks instead of clobbering them each materialization; SessionStart marks the writer's session boundary; PermissionRequest classifies its ask kind from tool_name (AskUserQuestion -> question, else permission); the catalog resolves CATALOG-first in claude-observe.sh. Co-Authored-By: Claude Fable 5 --- examples/native/agent-claude.kdl | 2 +- src/catalog_transaction.rs | 9 ++++ src/claude_session.rs | 76 +++++++++++++++++++++++----- src/driver.rs | 16 ++++-- src/hooks.rs | 3 ++ src/materialize.rs | 73 ++++++++++++++++++++++++-- tests/fixtures/driver/claude.out.kdl | 2 +- 7 files changed, 157 insertions(+), 24 deletions(-) diff --git a/examples/native/agent-claude.kdl b/examples/native/agent-claude.kdl index df0675bc..f97fac2c 100644 --- a/examples/native/agent-claude.kdl +++ b/examples/native/agent-claude.kdl @@ -18,7 +18,7 @@ agent "" { copy "assets/bus.st2.md" ".st2/bus.md" ensure-line ".claude/rules/st2.md" "@../../.st2/PERSONA.md" ensure-line ".claude/rules/st2.md" "@../../.st2/bus.md" - json-upsert ".claude/settings.local.json" #""" + json-upsert ".claude/settings.local.json" arrays="union" #""" { "$schema": "https://json.schemastore.org/claude-code-settings.json", "hooks": { diff --git a/src/catalog_transaction.rs b/src/catalog_transaction.rs index 243bde09..091e0f39 100644 --- a/src/catalog_transaction.rs +++ b/src/catalog_transaction.rs @@ -922,6 +922,7 @@ fn normalize_agent(spec: &agent_spec::AgentSpec) -> Result { insert_value( &mut fields, @@ -945,6 +946,14 @@ fn normalize_agent(spec: &agent_spec::AgentSpec) -> Result { insert_value( diff --git a/src/claude_session.rs b/src/claude_session.rs index d3a9fa17..af85220c 100644 --- a/src/claude_session.rs +++ b/src/claude_session.rs @@ -15,7 +15,7 @@ use std::path::Path; use anyhow::{Context as _, Result}; -use crate::harness_state::{Activity, BlockedOn, InputBuffer, Observation}; +use crate::harness_state::{Activity, Ask, BlockedOn, InputBuffer, Observation}; use crate::provider_session::{ PROVIDER_POLL, STOP, SessionObserver, install_signal_handler, run_provider, }; @@ -75,8 +75,13 @@ pub fn run_observe( return Ok(()); }; let pty_session = runtime_id.unwrap_or(identity).to_string(); - harness_state::Writer::new(&agent_dir, identity, "claude", Some(pty_session)) - .observe(observation) + let mut writer = harness_state::Writer::new(&agent_dir, identity, "claude", Some(pty_session)); + if event == "SessionStart" { + // The one event that names a session boundary: even if the new session's first state + // matches a fresh predecessor record, continuity must not be claimed across the restart. + writer.interrupt(); + } + writer.observe(observation) } /// Map one Claude hook event to an observation, or `None` when the event says nothing about @@ -117,10 +122,23 @@ pub fn observe_hook_event(event: &str, payload: &serde_json::Value) -> Option Some( - Observation::new(Activity::Active, BlockedOn::Human, InputBuffer::Unknown) - .with_reason("permissionRequest"), - ), + "PermissionRequest" => { + // Driver-side classification (#162): the payload's tool_name distinguishes Claude's + // question form from an ordinary permission prompt — the DQ-H1 captures show + // AskUserQuestion arriving as a PermissionRequest like any other tool. + let ask = if payload.get("tool_name").and_then(serde_json::Value::as_str) + == Some("AskUserQuestion") + { + Ask::Question + } else { + Ask::Permission + }; + Some( + Observation::new(Activity::Active, BlockedOn::Human, InputBuffer::Unknown) + .with_ask(ask) + .with_reason("permissionRequest"), + ) + } _ => None, } } @@ -269,9 +287,14 @@ mod tests { let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker"); // A hook process wrote a blocked observation between wrapper ticks. - harness_state::Writer::new(tmp.path(), "hetz.worker", "claude", None) - .observe(observe_hook_event("PermissionRequest", &serde_json::Value::Null).unwrap()) - .unwrap(); + harness_state::Writer::new( + tmp.path(), + "hetz.worker", + "claude", + Some("hetz.worker".to_string()), + ) + .observe(observe_hook_event("PermissionRequest", &serde_json::Value::Null).unwrap()) + .unwrap(); let before = fs::read(&record).unwrap(); std::thread::sleep(Duration::from_millis(2)); @@ -293,9 +316,14 @@ mod tests { let stop = AtomicBool::new(false); // A turn is in flight when the provider dies by signal. - harness_state::Writer::new(tmp.path(), "hetz.worker", "claude", None) - .observe(observe_hook_event("UserPromptSubmit", &serde_json::Value::Null).unwrap()) - .unwrap(); + harness_state::Writer::new( + tmp.path(), + "hetz.worker", + "claude", + Some("hetz.worker".to_string()), + ) + .observe(observe_hook_event("UserPromptSubmit", &serde_json::Value::Null).unwrap()) + .unwrap(); let result = run_provider( "Claude", @@ -337,4 +365,26 @@ mod tests { assert_eq!(observed.state, Activity::Ended); assert_eq!(observed.exit.as_deref(), Some("exit 0")); } + + #[test] + fn permission_requests_classify_their_ask_kind_from_the_tool_name() { + use crate::harness_state::Ask; + let permission = observe_hook_event( + "PermissionRequest", + &serde_json::json!({ "tool_name": "Bash", "tool_input": {} }), + ) + .unwrap(); + assert_eq!(permission.ask, Ask::Permission); + + let question = observe_hook_event( + "PermissionRequest", + &serde_json::json!({ "tool_name": "AskUserQuestion", "tool_input": {} }), + ) + .unwrap(); + assert_eq!(question.ask, Ask::Question); + + // Non-blocking events carry no ask. + let idle = observe_hook_event("Stop", &serde_json::json!({})).unwrap(); + assert_eq!(idle.ask, Ask::None); + } } diff --git a/src/driver.rs b/src/driver.rs index fac5671a..4c0f8dce 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -125,10 +125,18 @@ fn expand_claude(driver: &ClaudeDriver, bus_id: &str) -> Result { let mut render = KdlNode::new("render"); render.set_children(document([ node("json-upsert", vec![".mcp.json".to_string(), mcp]), - node( - "json-upsert", - vec![".claude/settings.local.json".to_string(), settings], - ), + { + // Hook arrays join whatever the workspace already declares: replacement would clobber + // user-registered hooks on every materialization, and union is idempotent. + let mut upsert = node( + "json-upsert", + vec![".claude/settings.local.json".to_string(), settings], + ); + upsert + .entries_mut() + .push(KdlEntry::new_prop("arrays", "union")); + upsert + }, ])); let mut provider = vec!["claude".to_string()]; diff --git a/src/hooks.rs b/src/hooks.rs index e7ea4760..974453c8 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -582,6 +582,9 @@ mod tests { let mut entries = node .entries() .iter() + // Positional arguments only: properties (e.g. `arrays="union"`) are merge + // strategy, not payload. + .filter(|entry| entry.name().is_none()) .filter_map(|entry| match entry.value() { kdl::KdlValue::String(value) => Some(value.as_str()), _ => None, diff --git a/src/materialize.rs b/src/materialize.rs index aa51a037..c9b8a0f1 100644 --- a/src/materialize.rs +++ b/src/materialize.rs @@ -26,6 +26,7 @@ pub enum RenderOp { JsonUpsert { destination: String, content: String, + arrays: ArrayMerge, }, EnsureLine { destination: String, @@ -36,6 +37,17 @@ pub enum RenderOp { }, } +/// How a json-upsert treats an array both sides declare. `Replace` is the default and the +/// original contract; `union` appends patch elements the target lacks (exact-equality dedupe), so +/// registrations can join arrays other owners also write — user-declared entries survive and +/// re-materialization is idempotent. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum ArrayMerge { + #[default] + Replace, + Union, +} + #[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct RenderPlan { pub ops: Vec, @@ -68,6 +80,7 @@ impl RenderOp { | Self::JsonUpsert { destination, content, + .. } => { references_variable(destination, variable) || references_variable(content, variable) } @@ -162,9 +175,22 @@ fn parse_render_node(node: &KdlNode, agent: &str) -> Result { serde_json::from_str::(&content).with_context(|| { format!("agent '{agent}': json-upsert content is not valid JSON") })?; + let arrays = match directive + .entries() + .iter() + .find(|entry| entry.name().is_some_and(|name| name.value() == "arrays")) + .and_then(|entry| entry.value().as_string()) + { + None | Some("replace") => ArrayMerge::Replace, + Some("union") => ArrayMerge::Union, + Some(other) => anyhow::bail!( + "agent '{agent}': json-upsert arrays=\"{other}\" (expected replace|union)" + ), + }; plan.ops.push(RenderOp::JsonUpsert { destination: destination.clone(), content, + arrays, }); } "ensure-line" => { @@ -267,6 +293,7 @@ fn resolve_driver_render_executable(plan: &mut RenderPlan, agent: &str) -> Resul let RenderOp::JsonUpsert { destination, content, + .. } = operation else { continue; @@ -312,6 +339,7 @@ fn effective_plan(root: &Path, spec: &AgentSpec, this_host: &str) -> Result Result { Ok(true) } -fn deep_merge(target: &mut serde_json::Value, patch: serde_json::Value) { +fn deep_merge(target: &mut serde_json::Value, patch: serde_json::Value, arrays: ArrayMerge) { match (target, patch) { (serde_json::Value::Object(target), serde_json::Value::Object(patch)) => { for (key, value) in patch { match target.get_mut(&key) { - Some(existing) => deep_merge(existing, value), + Some(existing) => deep_merge(existing, value, arrays), None => { target.insert(key, value); } } } } + (serde_json::Value::Array(target), serde_json::Value::Array(patch)) + if arrays == ArrayMerge::Union => + { + for element in patch { + if !target.contains(&element) { + target.push(element); + } + } + } (target, patch) => *target = patch, } } @@ -563,7 +600,7 @@ enum PreparedOp { #[derive(Debug, Clone, PartialEq)] enum RenderClaim { Replace(Vec), - JsonUpsert(serde_json::Value), + JsonUpsert(serde_json::Value, ArrayMerge), EnsureLine(String), } @@ -627,6 +664,7 @@ fn claims_for_agent( RenderOp::JsonUpsert { destination: raw_destination, content, + arrays, } => { let patch = serde_json::from_str(&expand(&content, &env)).with_context(|| { format!( @@ -636,7 +674,7 @@ fn claims_for_agent( })?; ( destination(&workspace, &raw_destination, &env)?, - RenderClaim::JsonUpsert(patch), + RenderClaim::JsonUpsert(patch, arrays), ) } RenderOp::EnsureLine { @@ -784,6 +822,7 @@ pub fn materialize_agent(root: &Path, spec: &AgentSpec, this_host: &str) -> Resu RenderOp::JsonUpsert { destination: raw_destination, content, + arrays, } => { let destination = destination(&workspace, &raw_destination, &env)?; let current = virtual_files @@ -804,7 +843,7 @@ pub fn materialize_agent(root: &Path, spec: &AgentSpec, this_host: &str) -> Resu spec.identity ) })?; - deep_merge(&mut target, patch); + deep_merge(&mut target, patch, arrays); let mut bytes = serde_json::to_vec_pretty(&target)?; bytes.push(b'\n'); let note = format!("{}: upserted {}", spec.identity, raw_destination); @@ -1056,6 +1095,7 @@ mod tests { "nested": {"right": 2, "replace": "new"}, "array": [2] }), + ArrayMerge::Replace, ); assert_eq!( target, @@ -1066,4 +1106,27 @@ mod tests { }) ); } + + /// Union mode joins arrays idempotently: foreign entries survive and repeating the same + /// patch adds nothing — the contract the generated hook registration relies on. + #[test] + fn deep_merge_union_preserves_foreign_array_entries_and_is_idempotent() { + let mut target = serde_json::json!({ + "hooks": {"Stop": [{"hooks": [{"type": "command", "command": "user-audit.sh"}]}]} + }); + let ours = serde_json::json!({ + "hooks": {"Stop": [{"hooks": [{"type": "command", "command": "$ST_HOOKS/claude-observe.sh Stop"}]}]} + }); + deep_merge(&mut target, ours.clone(), ArrayMerge::Union); + deep_merge(&mut target, ours, ArrayMerge::Union); + assert_eq!( + target, + serde_json::json!({ + "hooks": {"Stop": [ + {"hooks": [{"type": "command", "command": "user-audit.sh"}]}, + {"hooks": [{"type": "command", "command": "$ST_HOOKS/claude-observe.sh Stop"}]} + ]} + }) + ); + } } diff --git a/tests/fixtures/driver/claude.out.kdl b/tests/fixtures/driver/claude.out.kdl index 48045bb4..60548aa0 100644 --- a/tests/fixtures/driver/claude.out.kdl +++ b/tests/fixtures/driver/claude.out.kdl @@ -1,5 +1,5 @@ render { json-upsert .mcp.json "{\n \"mcpServers\": {\n \"st2\": {\n \"args\": [\n \"--catalog\",\n \"$CATALOG\",\n \"driver\",\n \"claude-mcp\",\n \"--identity\",\n \"Silber.fabric\"\n ],\n \"command\": \"st2\",\n \"type\": \"stdio\"\n }\n }\n}" - json-upsert ".claude/settings.local.json" "{\n \"$schema\": \"https://json.schemastore.org/claude-code-settings.json\",\n \"hooks\": {\n \"PermissionRequest\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh PermissionRequest\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"PostToolUse\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh PostToolUse\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"PreCompact\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-pre-compact.sh\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"PreToolUse\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh PreToolUse\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"SessionStart\": [\n {\n \"hooks\": [\n {\n \"async\": true,\n \"asyncRewake\": true,\n \"command\": \"$ST_HOOKS/claude-session-start.sh\",\n \"type\": \"command\"\n },\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh SessionStart\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"Stop\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh Stop\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"StopFailure\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-stop-failure.sh\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"UserPromptSubmit\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh UserPromptSubmit\",\n \"type\": \"command\"\n }\n ]\n }\n ]\n }\n}" + json-upsert ".claude/settings.local.json" "{\n \"$schema\": \"https://json.schemastore.org/claude-code-settings.json\",\n \"hooks\": {\n \"PermissionRequest\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh PermissionRequest\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"PostToolUse\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh PostToolUse\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"PreCompact\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-pre-compact.sh\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"PreToolUse\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh PreToolUse\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"SessionStart\": [\n {\n \"hooks\": [\n {\n \"async\": true,\n \"asyncRewake\": true,\n \"command\": \"$ST_HOOKS/claude-session-start.sh\",\n \"type\": \"command\"\n },\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh SessionStart\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"Stop\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh Stop\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"StopFailure\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-stop-failure.sh\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"UserPromptSubmit\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh UserPromptSubmit\",\n \"type\": \"command\"\n }\n ]\n }\n ]\n }\n}" arrays=union } argv st2 --catalog $CATALOG driver claude-session --identity Silber.fabric --runtime-id Silber.fabric -- claude --model opus --effort xhigh "--dangerously-load-development-channels=server:st2" --permission-mode bypassPermissions --model override "Start the assigned work." From 12e79a0ab7d783f821ae6e559d1f188a4617b173 Mon Sep 17 00:00:00 2001 From: Johannes Schickling Date: Mon, 24 Aug 2026 00:21:32 +0200 Subject: [PATCH 06/14] fix(claude,pi,materialize): shared session tokens, terminal fencing, and hook-set supersession The wrapper mints and exports its incarnation token (ST2_CLAUDE_SESSION; wrapperless hooks fall back to Claude's own session_id), late hooks can no longer overwrite the session's terminal record, pi gains a terminal-only observer so the pre-escalation ended write is real for its stop path, union merges supersede st2's own prior hook-set entries while never touching foreign ones, and the maintained example states its hooks-only limitation. Co-Authored-By: Claude Fable 5 --- examples/native/agent-claude.kdl | 6 +++ src/claude_session.rs | 88 ++++++++++++++++++++++++++++++-- src/materialize.rs | 85 ++++++++++++++++++++++++++++-- src/pi_session.rs | 49 +++++++++++++++++- src/provider_session.rs | 45 +++++++++++++--- 5 files changed, 257 insertions(+), 16 deletions(-) diff --git a/examples/native/agent-claude.kdl b/examples/native/agent-claude.kdl index f97fac2c..8937a221 100644 --- a/examples/native/agent-claude.kdl +++ b/examples/native/agent-claude.kdl @@ -18,6 +18,12 @@ agent "" { copy "assets/bus.st2.md" ".st2/bus.md" ensure-line ".claude/rules/st2.md" "@../../.st2/PERSONA.md" ensure-line ".claude/rules/st2.md" "@../../.st2/bus.md" + // Hooks-only observability: this seat launches claude directly (no session wrapper), so the + // registrations below give it transitions and blocked-on-you — but no heartbeat owner and no + // terminal record. A live-but-idle seat ages to `unknown` after the staleness horizon and an + // exit leaves the last state to age out; both read indeterminate, never wrong. The full + // producer (heartbeats, terminal exits) comes with the session wrapper, i.e. a typed + // `claude {}` driver seat or `deliver "mcp"`. json-upsert ".claude/settings.local.json" arrays="union" #""" { "$schema": "https://json.schemastore.org/claude-code-settings.json", diff --git a/src/claude_session.rs b/src/claude_session.rs index af85220c..8ccc6fee 100644 --- a/src/claude_session.rs +++ b/src/claude_session.rs @@ -39,7 +39,12 @@ pub fn run( let observer = SessionObserver::new(&agent_dir, &identity, "claude", &runtime_id); // The runtime ID reaches hook subprocesses through the provider environment, so their // transitions carry the same pty session the wrapper's records do. - let env = [(RUNTIME_ID_ENV.to_string(), runtime_id.clone())]; + let env = [ + (RUNTIME_ID_ENV.to_string(), runtime_id.clone()), + // Hook subprocesses adopt the wrapper's incarnation token, so their transitions are this + // session's records: the wrapper can re-stamp them, and its terminal record fences them. + (SESSION_ENV.to_string(), observer.session().to_string()), + ]; run_provider( "Claude", &status::status_path(&agent_dir), @@ -59,6 +64,8 @@ pub fn run( /// short-lived writer; the transition counter continues from disk. /// The env var carrying the wrapper's runtime/task ID into Claude's hook subprocesses. pub const RUNTIME_ID_ENV: &str = "ST2_CLAUDE_RUNTIME_ID"; +/// The env var carrying the wrapper's session incarnation token into Claude's hook subprocesses. +pub const SESSION_ENV: &str = "ST2_CLAUDE_SESSION"; pub fn run_observe( catalog_root: &Path, @@ -76,12 +83,32 @@ pub fn run_observe( }; let pty_session = runtime_id.unwrap_or(identity).to_string(); let mut writer = harness_state::Writer::new(&agent_dir, identity, "claude", Some(pty_session)); + // The wrapper's exported token makes hook writes this session's records. A wrapperless seat + // (hooks registered on a plain hand-authored launch) falls back to Claude's own session_id — + // stable across one Claude session's hooks, fresh on restart — so restatements still + // coalesce and a restart still opens a new transition; what such a seat lacks is a + // heartbeat/terminal owner, which is a documented hooks-only limitation. + let session = std::env::var(SESSION_ENV) + .ok() + .filter(|token| !token.is_empty()) + .or_else(|| { + payload + .get("session_id") + .and_then(serde_json::Value::as_str) + .map(|id| format!("claude-session-{id}")) + }); + if let Some(session) = session { + writer = writer.with_session(session); + } if event == "SessionStart" { // The one event that names a session boundary: even if the new session's first state // matches a fresh predecessor record, continuity must not be claimed across the restart. writer.interrupt(); } - writer.observe(observation) + // A late hook finishing after the wrapper reaped Claude must not replace the terminal record + // with a live state: the wrapper's `ended` carries this same token and is the session's last + // word. (`false` = suppressed; the hook has nothing else to do with it.) + writer.observe_unless_ended(observation).map(|_wrote| ()) } /// Map one Claude hook event to an observation, or `None` when the event says nothing about @@ -286,13 +313,15 @@ mod tests { let record = harness_state_path(tmp.path()); let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker"); - // A hook process wrote a blocked observation between wrapper ticks. + // A hook process wrote a blocked observation between wrapper ticks — carrying the + // wrapper's exported token, exactly as the env plumbing arranges in a real seat. harness_state::Writer::new( tmp.path(), "hetz.worker", "claude", Some("hetz.worker".to_string()), ) + .with_session(observer.session()) .observe(observe_hook_event("PermissionRequest", &serde_json::Value::Null).unwrap()) .unwrap(); let before = fs::read(&record).unwrap(); @@ -387,4 +416,57 @@ mod tests { let idle = observe_hook_event("Stop", &serde_json::json!({})).unwrap(); assert_eq!(idle.ask, Ask::None); } + + /// T2: a hook that finishes after the wrapper reaped Claude must not replace the terminal + /// record — the wrapper's `ended` carries the shared token and is the session's last word — + /// while a NEW session's boundary event still supersedes an old terminal record. + #[test] + fn a_late_hook_never_overwrites_this_sessions_terminal_record() { + use crate::harness_state::{self, Activity}; + let tmp = tempfile::tempdir().unwrap(); + let record = harness_state_path(tmp.path()); + let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker"); + observer.ended("exit 0"); + + // The straggler hook shares the session token (env plumbing) and is suppressed. + let mut late = harness_state::Writer::new( + tmp.path(), + "hetz.worker", + "claude", + Some("hetz.worker".to_string()), + ) + .with_session(observer.session()); + assert!( + !late + .observe_unless_ended( + observe_hook_event("PostToolUse", &serde_json::Value::Null).unwrap() + ) + .unwrap() + ); + assert_eq!( + harness_state::read(&record, None).unwrap().state, + Activity::Ended + ); + + // A new Claude session is a new incarnation: its SessionStart replaces the old terminal. + let mut fresh = harness_state::Writer::new( + tmp.path(), + "hetz.worker", + "claude", + Some("hetz.worker".to_string()), + ) + .with_session("claude-session-fresh"); + fresh.interrupt(); + assert!( + fresh + .observe_unless_ended( + observe_hook_event("SessionStart", &serde_json::Value::Null).unwrap() + ) + .unwrap() + ); + assert_eq!( + harness_state::read(&record, None).unwrap().state, + Activity::Idle + ); + } } diff --git a/src/materialize.rs b/src/materialize.rs index c9b8a0f1..f7e82f30 100644 --- a/src/materialize.rs +++ b/src/materialize.rs @@ -494,12 +494,17 @@ fn ensure_line(path: &Path, line: &str) -> Result { Ok(true) } -fn deep_merge(target: &mut serde_json::Value, patch: serde_json::Value, arrays: ArrayMerge) { +fn deep_merge( + target: &mut serde_json::Value, + patch: serde_json::Value, + arrays: ArrayMerge, + owned_prefixes: &[String], +) { match (target, patch) { (serde_json::Value::Object(target), serde_json::Value::Object(patch)) => { for (key, value) in patch { match target.get_mut(&key) { - Some(existing) => deep_merge(existing, value, arrays), + Some(existing) => deep_merge(existing, value, arrays, owned_prefixes), None => { target.insert(key, value); } @@ -509,6 +514,14 @@ fn deep_merge(target: &mut serde_json::Value, patch: serde_json::Value, arrays: (serde_json::Value::Array(target), serde_json::Value::Array(patch)) if arrays == ArrayMerge::Union => { + // Exact-equality union alone would accumulate st2's own entries across hook-set + // upgrades: `$ST_HOOKS` expands content-addressed, so every upgrade renders each + // entry with a new path and the old one would be retained beside it. An element + // recognizably st2's — one referencing the hook root — that the patch no longer + // states is therefore superseded and dropped; foreign entries are never touched. + target.retain(|element| { + !contains_owned_string(element, owned_prefixes) || patch.contains(element) + }); for element in patch { if !target.contains(&element) { target.push(element); @@ -519,6 +532,35 @@ fn deep_merge(target: &mut serde_json::Value, patch: serde_json::Value, arrays: } } +/// Whether any string inside `value` marks it as an st2-rendered element: a reference to the +/// installed hook root (any set version) or the unexpanded `$ST_HOOKS` variable. +fn contains_owned_string(value: &serde_json::Value, owned_prefixes: &[String]) -> bool { + match value { + serde_json::Value::String(text) => owned_prefixes + .iter() + .any(|prefix| text.starts_with(prefix.as_str())), + serde_json::Value::Array(items) => items + .iter() + .any(|item| contains_owned_string(item, owned_prefixes)), + serde_json::Value::Object(map) => map + .values() + .any(|item| contains_owned_string(item, owned_prefixes)), + _ => false, + } +} + +/// The string prefixes that mark a JSON element as st2-rendered for union supersession: the hook +/// root that contains every installed set version, and the unexpanded variable spellings. +fn owned_union_prefixes(env: &BTreeMap) -> Vec { + let mut prefixes = vec!["$ST_HOOKS".to_string(), "${ST_HOOKS}".to_string()]; + if let Some(hooks) = env.get("ST_HOOKS") + && let Some(root) = Path::new(hooks).parent() + { + prefixes.push(format!("{}/", root.display())); + } + prefixes +} + fn git_exclude(workspace: &Path, line: &str) -> Result { let output = Command::new("git") .args(["-C"]) @@ -843,7 +885,7 @@ pub fn materialize_agent(root: &Path, spec: &AgentSpec, this_host: &str) -> Resu spec.identity ) })?; - deep_merge(&mut target, patch, arrays); + deep_merge(&mut target, patch, arrays, &owned_union_prefixes(&env)); let mut bytes = serde_json::to_vec_pretty(&target)?; bytes.push(b'\n'); let note = format!("{}: upserted {}", spec.identity, raw_destination); @@ -1096,6 +1138,7 @@ mod tests { "array": [2] }), ArrayMerge::Replace, + &[], ); assert_eq!( target, @@ -1117,8 +1160,8 @@ mod tests { let ours = serde_json::json!({ "hooks": {"Stop": [{"hooks": [{"type": "command", "command": "$ST_HOOKS/claude-observe.sh Stop"}]}]} }); - deep_merge(&mut target, ours.clone(), ArrayMerge::Union); - deep_merge(&mut target, ours, ArrayMerge::Union); + deep_merge(&mut target, ours.clone(), ArrayMerge::Union, &[]); + deep_merge(&mut target, ours, ArrayMerge::Union, &[]); assert_eq!( target, serde_json::json!({ @@ -1129,4 +1172,36 @@ mod tests { }) ); } + + /// A hook-set upgrade renders every entry under a new content-addressed path. Union must + /// supersede st2's prior entries — recognizable by the hook root — rather than accumulate + /// them, while a user's entry under any other path survives every merge. + #[test] + fn union_supersedes_prior_hook_set_entries_but_never_foreign_ones() { + let owned = vec![ + "$ST_HOOKS".to_string(), + "${ST_HOOKS}".to_string(), + "/state/st2/hooks/".to_string(), + ]; + let mut target = serde_json::json!({ + "hooks": {"Stop": [ + {"hooks": [{"type": "command", "command": "user-audit.sh"}]}, + {"hooks": [{"type": "command", "command": "/state/st2/hooks/set-v1/claude-observe.sh Stop"}]} + ]} + }); + let upgraded = serde_json::json!({ + "hooks": {"Stop": [{"hooks": [{"type": "command", "command": "/state/st2/hooks/set-v2/claude-observe.sh Stop"}]}]} + }); + deep_merge(&mut target, upgraded.clone(), ArrayMerge::Union, &owned); + deep_merge(&mut target, upgraded, ArrayMerge::Union, &owned); + assert_eq!( + target, + serde_json::json!({ + "hooks": {"Stop": [ + {"hooks": [{"type": "command", "command": "user-audit.sh"}]}, + {"hooks": [{"type": "command", "command": "/state/st2/hooks/set-v2/claude-observe.sh Stop"}]} + ]} + }) + ); + } } diff --git a/src/pi_session.rs b/src/pi_session.rs index 80da46ff..82f50c5d 100644 --- a/src/pi_session.rs +++ b/src/pi_session.rs @@ -110,6 +110,17 @@ pub fn run( } }; install_signal_handler(); + // Terminal-only: the channel owns the live record and its heartbeat, but only this wrapper + // survives long enough to see the stop path — its pre-escalation `ended` write is the one + // that makes `Stopped(None)` observable at all. Same token as the channel, so the terminal + // record fences exactly this session's live records. + let observer = crate::provider_session::SessionObserver::terminal_only( + &agent_dir, + &identity, + "pi", + &runtime_id, + &session, + ); let outcome = run_provider_observed( "pi", &status::status_path(&agent_dir), @@ -118,7 +129,7 @@ pub fn run( status::STATUS_REFRESH, PROVIDER_POLL, &STOP, - None, + Some(&observer), ) .with_context(|| format!("running pi driver '{runtime_id}'"))?; record_session_end(&agent_dir, &identity, &runtime_id, &session, seq, &outcome); @@ -399,4 +410,40 @@ mod tests { ] ); } + + /// W6: the terminal-only observer records how the session ended but never re-stamps live + /// state — the channel owns the heartbeat — and its token makes the write this session's. + #[test] + fn the_terminal_only_observer_ends_but_never_heartbeats() { + use crate::harness_state::{self, Activity}; + let tmp = tempfile::tempdir().unwrap(); + let record = harness_state::harness_state_path(tmp.path()); + let session = harness_state::session_token(); + let mut channel = + harness_state::Writer::new(tmp.path(), "h.worker", "pi", Some("h.worker".to_string())) + .with_session(session.clone()); + channel + .observe(harness_state::Observation::new( + Activity::Active, + harness_state::BlockedOn::None, + harness_state::InputBuffer::Unknown, + )) + .unwrap(); + let live = std::fs::read(&record).unwrap(); + + let observer = crate::provider_session::SessionObserver::terminal_only( + tmp.path(), + "h.worker", + "pi", + "h.worker", + &session, + ); + observer.heartbeat(); + assert_eq!(std::fs::read(&record).unwrap(), live, "no heartbeat"); + + observer.ended("signal 9"); + let observed = harness_state::read(&record, None).unwrap(); + assert_eq!(observed.state, Activity::Ended); + assert_eq!(observed.exit.as_deref(), Some("signal 9")); + } } diff --git a/src/provider_session.rs b/src/provider_session.rs index 8d7da2e8..39482a5d 100644 --- a/src/provider_session.rs +++ b/src/provider_session.rs @@ -60,14 +60,17 @@ pub(crate) struct SessionObserver { identity: String, harness: &'static str, pty_session: String, - session_start_ms: u64, + session: String, + /// A terminal-only observer records how the session ended but never re-stamps live state — + /// for wrappers whose heartbeat belongs to another sibling process (pi's channel). + heartbeats: bool, } impl SessionObserver { /// `pty_session` is the wrapper's runtime/task ID — the registry entry whose liveness vouches - /// for the record. The session start is pinned once here so every per-operation fresh writer - /// agrees where this session began: a predecessor session's record stays heartbeat-ineligible - /// until something of this session is observed. + /// for the record. The observer mints the session incarnation token; the wrapper exports it + /// to its sibling writer processes (hooks, a channel) so ownership — coalescing, heartbeat + /// eligibility, terminal fencing — is decided by token equality across all of them. pub(crate) fn new( agent_dir: &Path, identity: &str, @@ -79,10 +82,36 @@ impl SessionObserver { identity: identity.to_string(), harness, pty_session: pty_session.to_string(), - session_start_ms: crate::message::now_ms(), + session: harness_state::session_token(), + heartbeats: true, } } + /// An observer that records only how the session ended: `heartbeat` is a no-op because a + /// sibling process owns the live record and its freshness. Adopts that sibling's token so + /// the terminal record fences exactly this session's records. + pub(crate) fn terminal_only( + agent_dir: &Path, + identity: &str, + harness: &'static str, + pty_session: &str, + session: &str, + ) -> Self { + Self { + agent_dir: agent_dir.to_path_buf(), + identity: identity.to_string(), + harness, + pty_session: pty_session.to_string(), + session: session.to_string(), + heartbeats: false, + } + } + + /// The session incarnation token sibling writer processes must adopt. + pub(crate) fn session(&self) -> &str { + &self.session + } + fn writer(&self) -> harness_state::Writer { harness_state::Writer::new( &self.agent_dir, @@ -90,13 +119,15 @@ impl SessionObserver { self.harness, Some(self.pty_session.clone()), ) - .session_started_at(self.session_start_ms) + .with_session(self.session.clone()) } /// Re-stamp whatever live state is on disk. The wrapper's evidence is the provider child it is /// polling, so this is called only while that child is alive. pub(crate) fn heartbeat(&self) { - let _ = self.writer().heartbeat(); + if self.heartbeats { + let _ = self.writer().heartbeat(); + } } /// Best-effort terminal record; observation must never turn a clean teardown into an error. From 201bd83c9bd99009bff5aca5fa5d284528d411d2 Mon Sep 17 00:00:00 2001 From: Johannes Schickling Date: Mon, 24 Aug 2026 00:51:04 +0200 Subject: [PATCH 07/14] fix(claude,materialize): exported ownership claims and structural hook recognition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The observer claims the ownership sequence at construction and exports it beside its token (ST2_CLAUDE_SESSION_SEQ); hooks adopt the pair, so a hook straggling from a superseded session is refused. Union supersession recognizes st2's entries structurally — a managed basename under any set-shaped path (sets/sha256-*), or $ST_HOOKS at a token boundary — so a relocated hook root cannot duplicate registrations and $ST_HOOKS_SUFFIX-style foreign variables are never misclassified. Co-Authored-By: Claude Fable 5 --- src/claude_session.rs | 30 ++++++++++------ src/hooks.rs | 35 ++++++++++++++++++ src/materialize.rs | 78 +++++++++++++++-------------------------- src/pi_session.rs | 2 ++ src/provider_session.rs | 11 +++++- 5 files changed, 95 insertions(+), 61 deletions(-) diff --git a/src/claude_session.rs b/src/claude_session.rs index 8ccc6fee..e39cf7ac 100644 --- a/src/claude_session.rs +++ b/src/claude_session.rs @@ -44,6 +44,7 @@ pub fn run( // Hook subprocesses adopt the wrapper's incarnation token, so their transitions are this // session's records: the wrapper can re-stamp them, and its terminal record fences them. (SESSION_ENV.to_string(), observer.session().to_string()), + (SESSION_SEQ_ENV.to_string(), observer.seq().to_string()), ]; run_provider( "Claude", @@ -66,6 +67,8 @@ pub fn run( pub const RUNTIME_ID_ENV: &str = "ST2_CLAUDE_RUNTIME_ID"; /// The env var carrying the wrapper's session incarnation token into Claude's hook subprocesses. pub const SESSION_ENV: &str = "ST2_CLAUDE_SESSION"; +/// The env var carrying the wrapper's claimed ownership sequence beside the token. +pub const SESSION_SEQ_ENV: &str = "ST2_CLAUDE_SESSION_SEQ"; pub fn run_observe( catalog_root: &Path, @@ -88,17 +91,24 @@ pub fn run_observe( // stable across one Claude session's hooks, fresh on restart — so restatements still // coalesce and a restart still opens a new transition; what such a seat lacks is a // heartbeat/terminal owner, which is a documented hooks-only limitation. - let session = std::env::var(SESSION_ENV) + let exported = std::env::var(SESSION_ENV) .ok() - .filter(|token| !token.is_empty()) - .or_else(|| { - payload - .get("session_id") - .and_then(serde_json::Value::as_str) - .map(|id| format!("claude-session-{id}")) - }); - if let Some(session) = session { - writer = writer.with_session(session); + .filter(|token| !token.is_empty()); + if let Some(session) = exported { + // Full adopted ownership when the wrapper exported it: the claimed sequence makes the + // token directional, so a hook straggling from a superseded session is refused. + writer = match std::env::var(SESSION_SEQ_ENV) + .ok() + .and_then(|seq| seq.parse::().ok()) + { + Some(seq) => writer.with_ownership(session, seq), + None => writer.with_session(session), + }; + } else if let Some(id) = payload + .get("session_id") + .and_then(serde_json::Value::as_str) + { + writer = writer.with_session(format!("claude-session-{id}")); } if event == "SessionStart" { // The one event that names a session boundary: even if the new session's first state diff --git a/src/hooks.rs b/src/hooks.rs index 974453c8..9057ba83 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -147,6 +147,41 @@ pub fn claude_settings_registration() -> serde_json::Value { }) } +/// Whether one rendered string refers to a file of ANY st2 hook set, structurally — used by the +/// union merge to supersede st2's own prior registrations without ever touching a foreign entry. +/// Two spellings are owned: the `$ST_HOOKS` variable at a token boundary (`$ST_HOOKS/...`, +/// `${ST_HOOKS}/...` — `$ST_HOOKS_SUFFIX` is somebody else's variable), and an expanded path +/// whose basename is a managed hook file sitting under a set-shaped directory +/// (`.../sets/sha256-.../`), which recognizes every set version under every past +/// or relocated root without consulting the current environment. +pub(crate) fn is_managed_hook_reference(text: &str) -> bool { + let command = text.split_whitespace().next().unwrap_or(""); + if let Some(rest) = command.strip_prefix("$ST_HOOKS") { + return rest.is_empty() || rest.starts_with('/'); + } + if let Some(rest) = command.strip_prefix("${ST_HOOKS}") { + return rest.is_empty() || rest.starts_with('/'); + } + let path = std::path::Path::new(command); + let managed_basename = path + .file_name() + .and_then(|name| name.to_str()) + .is_some_and(|name| HOOKS.iter().any(|(managed, _)| *managed == name)); + if !managed_basename { + return false; + } + let mut components = path.components().rev().skip(1); + let set_shaped = components + .next() + .and_then(|segment| segment.as_os_str().to_str()) + .is_some_and(|segment| segment.starts_with("sha256-")); + let sets_dir = components + .next() + .and_then(|segment| segment.as_os_str().to_str()) + .is_some_and(|segment| segment == SETS_DIR); + set_shaped && sets_dir +} + /// Install-owned hook root. `$ST_HOOKS` can pin a scratch or custom state layout; otherwise use /// `$XDG_STATE_HOME/st2/hooks` or `~/.local/state/st2/hooks`. pub fn hooks_root() -> Result { diff --git a/src/materialize.rs b/src/materialize.rs index f7e82f30..a2947478 100644 --- a/src/materialize.rs +++ b/src/materialize.rs @@ -494,17 +494,12 @@ fn ensure_line(path: &Path, line: &str) -> Result { Ok(true) } -fn deep_merge( - target: &mut serde_json::Value, - patch: serde_json::Value, - arrays: ArrayMerge, - owned_prefixes: &[String], -) { +fn deep_merge(target: &mut serde_json::Value, patch: serde_json::Value, arrays: ArrayMerge) { match (target, patch) { (serde_json::Value::Object(target), serde_json::Value::Object(patch)) => { for (key, value) in patch { match target.get_mut(&key) { - Some(existing) => deep_merge(existing, value, arrays, owned_prefixes), + Some(existing) => deep_merge(existing, value, arrays), None => { target.insert(key, value); } @@ -517,11 +512,10 @@ fn deep_merge( // Exact-equality union alone would accumulate st2's own entries across hook-set // upgrades: `$ST_HOOKS` expands content-addressed, so every upgrade renders each // entry with a new path and the old one would be retained beside it. An element - // recognizably st2's — one referencing the hook root — that the patch no longer - // states is therefore superseded and dropped; foreign entries are never touched. - target.retain(|element| { - !contains_owned_string(element, owned_prefixes) || patch.contains(element) - }); + // recognizably st2's — structurally, a managed hook file under any set-shaped path + // or the `$ST_HOOKS` variable at a token boundary — that the patch no longer states + // is therefore superseded and dropped; foreign entries are never touched. + target.retain(|element| !contains_owned_string(element) || patch.contains(element)); for element in patch { if !target.contains(&element) { target.push(element); @@ -532,35 +526,17 @@ fn deep_merge( } } -/// Whether any string inside `value` marks it as an st2-rendered element: a reference to the -/// installed hook root (any set version) or the unexpanded `$ST_HOOKS` variable. -fn contains_owned_string(value: &serde_json::Value, owned_prefixes: &[String]) -> bool { +/// Whether any string inside `value` marks it as an st2-rendered element, structurally (see +/// [`crate::hooks::is_managed_hook_reference`]). +fn contains_owned_string(value: &serde_json::Value) -> bool { match value { - serde_json::Value::String(text) => owned_prefixes - .iter() - .any(|prefix| text.starts_with(prefix.as_str())), - serde_json::Value::Array(items) => items - .iter() - .any(|item| contains_owned_string(item, owned_prefixes)), - serde_json::Value::Object(map) => map - .values() - .any(|item| contains_owned_string(item, owned_prefixes)), + serde_json::Value::String(text) => crate::hooks::is_managed_hook_reference(text), + serde_json::Value::Array(items) => items.iter().any(contains_owned_string), + serde_json::Value::Object(map) => map.values().any(contains_owned_string), _ => false, } } -/// The string prefixes that mark a JSON element as st2-rendered for union supersession: the hook -/// root that contains every installed set version, and the unexpanded variable spellings. -fn owned_union_prefixes(env: &BTreeMap) -> Vec { - let mut prefixes = vec!["$ST_HOOKS".to_string(), "${ST_HOOKS}".to_string()]; - if let Some(hooks) = env.get("ST_HOOKS") - && let Some(root) = Path::new(hooks).parent() - { - prefixes.push(format!("{}/", root.display())); - } - prefixes -} - fn git_exclude(workspace: &Path, line: &str) -> Result { let output = Command::new("git") .args(["-C"]) @@ -885,7 +861,7 @@ pub fn materialize_agent(root: &Path, spec: &AgentSpec, this_host: &str) -> Resu spec.identity ) })?; - deep_merge(&mut target, patch, arrays, &owned_union_prefixes(&env)); + deep_merge(&mut target, patch, arrays); let mut bytes = serde_json::to_vec_pretty(&target)?; bytes.push(b'\n'); let note = format!("{}: upserted {}", spec.identity, raw_destination); @@ -1138,7 +1114,6 @@ mod tests { "array": [2] }), ArrayMerge::Replace, - &[], ); assert_eq!( target, @@ -1160,8 +1135,8 @@ mod tests { let ours = serde_json::json!({ "hooks": {"Stop": [{"hooks": [{"type": "command", "command": "$ST_HOOKS/claude-observe.sh Stop"}]}]} }); - deep_merge(&mut target, ours.clone(), ArrayMerge::Union, &[]); - deep_merge(&mut target, ours, ArrayMerge::Union, &[]); + deep_merge(&mut target, ours.clone(), ArrayMerge::Union); + deep_merge(&mut target, ours, ArrayMerge::Union); assert_eq!( target, serde_json::json!({ @@ -1178,28 +1153,31 @@ mod tests { /// them, while a user's entry under any other path survives every merge. #[test] fn union_supersedes_prior_hook_set_entries_but_never_foreign_ones() { - let owned = vec![ - "$ST_HOOKS".to_string(), - "${ST_HOOKS}".to_string(), - "/state/st2/hooks/".to_string(), - ]; + // The prior set lives under a RELOCATED root — recognition is structural (set-shaped + // path + managed basename), not derived from the current environment. let mut target = serde_json::json!({ "hooks": {"Stop": [ {"hooks": [{"type": "command", "command": "user-audit.sh"}]}, - {"hooks": [{"type": "command", "command": "/state/st2/hooks/set-v1/claude-observe.sh Stop"}]} + {"hooks": [{"type": "command", "command": "/old/root/sets/sha256-aaa/claude-observe.sh Stop"}]}, + {"hooks": [{"type": "command", "command": "/home/x/claude-observe.sh Stop"}]}, + {"hooks": [{"type": "command", "command": "$ST_HOOKS_SUFFIX/tool.sh"}]} ]} }); let upgraded = serde_json::json!({ - "hooks": {"Stop": [{"hooks": [{"type": "command", "command": "/state/st2/hooks/set-v2/claude-observe.sh Stop"}]}]} + "hooks": {"Stop": [{"hooks": [{"type": "command", "command": "/new/root/sets/sha256-bbb/claude-observe.sh Stop"}]}]} }); - deep_merge(&mut target, upgraded.clone(), ArrayMerge::Union, &owned); - deep_merge(&mut target, upgraded, ArrayMerge::Union, &owned); + deep_merge(&mut target, upgraded.clone(), ArrayMerge::Union); + deep_merge(&mut target, upgraded, ArrayMerge::Union); assert_eq!( target, serde_json::json!({ "hooks": {"Stop": [ {"hooks": [{"type": "command", "command": "user-audit.sh"}]}, - {"hooks": [{"type": "command", "command": "/state/st2/hooks/set-v2/claude-observe.sh Stop"}]} + // A managed basename OUTSIDE a set-shaped path is a user's wrapper: foreign. + {"hooks": [{"type": "command", "command": "/home/x/claude-observe.sh Stop"}]}, + // `$ST_HOOKS_SUFFIX` is somebody else's variable, not ours at a boundary. + {"hooks": [{"type": "command", "command": "$ST_HOOKS_SUFFIX/tool.sh"}]}, + {"hooks": [{"type": "command", "command": "/new/root/sets/sha256-bbb/claude-observe.sh Stop"}]} ]} }) ); diff --git a/src/pi_session.rs b/src/pi_session.rs index 82f50c5d..5a5ab7da 100644 --- a/src/pi_session.rs +++ b/src/pi_session.rs @@ -120,6 +120,7 @@ pub fn run( "pi", &runtime_id, &session, + seq, ); let outcome = run_provider_observed( "pi", @@ -437,6 +438,7 @@ mod tests { "pi", "h.worker", &session, + harness_state::claim_seq(tmp.path()), ); observer.heartbeat(); assert_eq!(std::fs::read(&record).unwrap(), live, "no heartbeat"); diff --git a/src/provider_session.rs b/src/provider_session.rs index 39482a5d..9b40d40a 100644 --- a/src/provider_session.rs +++ b/src/provider_session.rs @@ -61,6 +61,7 @@ pub(crate) struct SessionObserver { harness: &'static str, pty_session: String, session: String, + seq: u64, /// A terminal-only observer records how the session ended but never re-stamps live state — /// for wrappers whose heartbeat belongs to another sibling process (pi's channel). heartbeats: bool, @@ -78,6 +79,7 @@ impl SessionObserver { pty_session: &str, ) -> Self { Self { + seq: harness_state::claim_seq(agent_dir), agent_dir: agent_dir.to_path_buf(), identity: identity.to_string(), harness, @@ -96,6 +98,7 @@ impl SessionObserver { harness: &'static str, pty_session: &str, session: &str, + seq: u64, ) -> Self { Self { agent_dir: agent_dir.to_path_buf(), @@ -103,6 +106,7 @@ impl SessionObserver { harness, pty_session: pty_session.to_string(), session: session.to_string(), + seq, heartbeats: false, } } @@ -112,6 +116,11 @@ impl SessionObserver { &self.session } + /// The ownership sequence this session claimed — exported beside the token. + pub(crate) fn seq(&self) -> u64 { + self.seq + } + fn writer(&self) -> harness_state::Writer { harness_state::Writer::new( &self.agent_dir, @@ -119,7 +128,7 @@ impl SessionObserver { self.harness, Some(self.pty_session.clone()), ) - .with_session(self.session.clone()) + .with_ownership(self.session.clone(), self.seq) } /// Re-stamp whatever live state is on disk. The wrapper's evidence is the provider child it is From 02c6ed1baf33265bc55438c9618cb640a43bc6ec Mon Sep 17 00:00:00 2001 From: Johannes Schickling Date: Mon, 24 Aug 2026 01:28:51 +0200 Subject: [PATCH 08/14] fix(claude,materialize): the observer's written claim and descending supersession MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SessionObserver performs the written ownership claim at construction (fatal if unwritable — an unclaimed writer would be refused by every record); union supersession descends into matcher groups, removing only the superseded managed entries so a group holding user and st2 hooks keeps the user's, and dropping only genuine husks. Co-Authored-By: Claude Fable 5 --- src/claude_session.rs | 43 +++++++++++++++---- src/materialize.rs | 95 ++++++++++++++++++++++++++++++++++++++--- src/pi_session.rs | 6 ++- src/provider_session.rs | 21 +++++---- 4 files changed, 141 insertions(+), 24 deletions(-) diff --git a/src/claude_session.rs b/src/claude_session.rs index e39cf7ac..fd4bcf71 100644 --- a/src/claude_session.rs +++ b/src/claude_session.rs @@ -36,7 +36,7 @@ pub fn run( "Claude driver '{runtime_id}' has no provider argv" ); install_signal_handler(); - let observer = SessionObserver::new(&agent_dir, &identity, "claude", &runtime_id); + let observer = SessionObserver::new(&agent_dir, &identity, "claude", &runtime_id)?; // The runtime ID reaches hook subprocesses through the provider environment, so their // transitions carry the same pty session the wrapper's records do. let env = [ @@ -321,7 +321,8 @@ mod tests { fn wrapper_heartbeat_re_stamps_without_clobbering_hook_written_state() { let tmp = tempfile::tempdir().unwrap(); let record = harness_state_path(tmp.path()); - let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker"); + let observer = + SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker").unwrap(); // A hook process wrote a blocked observation between wrapper ticks — carrying the // wrapper's exported token, exactly as the env plumbing arranges in a real seat. @@ -351,7 +352,8 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); let presence = status::status_path(tmp.path()); let record = harness_state_path(tmp.path()); - let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker"); + let observer = + SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker").unwrap(); let stop = AtomicBool::new(false); // A turn is in flight when the provider dies by signal. @@ -385,7 +387,8 @@ mod tests { fn a_clean_provider_exit_writes_the_terminal_record() { let tmp = tempfile::tempdir().unwrap(); let presence = status::status_path(tmp.path()); - let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker"); + let observer = + SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker").unwrap(); let stop = AtomicBool::new(false); run_provider( @@ -435,7 +438,8 @@ mod tests { use crate::harness_state::{self, Activity}; let tmp = tempfile::tempdir().unwrap(); let record = harness_state_path(tmp.path()); - let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker"); + let observer = + SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker").unwrap(); observer.ended("exit 0"); // The straggler hook shares the session token (env plumbing) and is suppressed. @@ -458,15 +462,38 @@ mod tests { Activity::Ended ); - // A new Claude session is a new incarnation: its SessionStart replaces the old terminal. - let mut fresh = harness_state::Writer::new( + // A wrapperless fresh session (token-only, the session_id fallback) cannot take over a + // claimed record: only a written claim supersedes. + let mut fallback = harness_state::Writer::new( tmp.path(), "hetz.worker", "claude", Some("hetz.worker".to_string()), ) .with_session("claude-session-fresh"); - fresh.interrupt(); + fallback.interrupt(); + assert!( + !fallback + .observe_unless_ended( + observe_hook_event("SessionStart", &serde_json::Value::Null).unwrap() + ) + .unwrap() + ); + assert_eq!( + harness_state::read(&record, None).unwrap().state, + Activity::Ended + ); + + // A claimed new session — the wrapper path — supersedes the old terminal record. + let next = + SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker").unwrap(); + let mut fresh = harness_state::Writer::new( + tmp.path(), + "hetz.worker", + "claude", + Some("hetz.worker".to_string()), + ) + .with_ownership(next.session().to_string(), next.seq()); assert!( fresh .observe_unless_ended( diff --git a/src/materialize.rs b/src/materialize.rs index a2947478..49bba9ca 100644 --- a/src/materialize.rs +++ b/src/materialize.rs @@ -511,11 +511,21 @@ fn deep_merge(target: &mut serde_json::Value, patch: serde_json::Value, arrays: { // Exact-equality union alone would accumulate st2's own entries across hook-set // upgrades: `$ST_HOOKS` expands content-addressed, so every upgrade renders each - // entry with a new path and the old one would be retained beside it. An element - // recognizably st2's — structurally, a managed hook file under any set-shaped path - // or the `$ST_HOOKS` variable at a token boundary — that the patch no longer states - // is therefore superseded and dropped; foreign entries are never touched. - target.retain(|element| !contains_owned_string(element) || patch.contains(element)); + // entry with a new path and the old one would be retained beside it. Supersession + // DESCENDS: only the managed nested entries (structurally st2's — a managed hook + // file under any set-shaped path, or `$ST_HOOKS` at a token boundary) the patch no + // longer states are removed, so a matcher group holding a user hook beside an st2 + // one keeps the user's; containers left with nothing but empty arrays are dropped. + let mut kept_leaves = Vec::new(); + for element in patch.iter() { + collect_owned_leaves(element, &mut kept_leaves); + } + target.retain_mut(|element| { + if patch.contains(element) || !contains_owned_string(element) { + return true; + } + keep_after_supersession(element, &kept_leaves) + }); for element in patch { if !target.contains(&element) { target.push(element); @@ -526,6 +536,70 @@ fn deep_merge(target: &mut serde_json::Value, patch: serde_json::Value, arrays: } } +/// Prune superseded managed entries INSIDE `element`, returning whether the element itself is +/// still worth keeping. An owned leaf (no nested arrays) survives only if the patch still states +/// it somewhere; containers are pruned recursively and dropped once every nested array is empty +/// — a husk that only ever held superseded registrations. +fn keep_after_supersession(element: &mut serde_json::Value, kept: &[serde_json::Value]) -> bool { + if !has_nested_array(element) { + return !contains_owned_string(element) || kept.contains(element); + } + let mut any_array_content = false; + match element { + serde_json::Value::Array(items) => { + items.retain_mut(|item| keep_after_supersession(item, kept)); + any_array_content = !items.is_empty(); + } + serde_json::Value::Object(map) => { + for value in map.values_mut() { + if let serde_json::Value::Array(items) = value { + items.retain_mut(|item| keep_after_supersession(item, kept)); + if !items.is_empty() { + any_array_content = true; + } + } else if has_nested_array(value) { + if keep_after_supersession(value, kept) { + any_array_content = true; + } + } + } + } + _ => {} + } + any_array_content +} + +/// Owned leaves — managed entries with no nested arrays — anywhere inside `value`. +fn collect_owned_leaves(value: &serde_json::Value, out: &mut Vec) { + if !has_nested_array(value) { + if contains_owned_string(value) { + out.push(value.clone()); + } + return; + } + match value { + serde_json::Value::Array(items) => { + for item in items { + collect_owned_leaves(item, out); + } + } + serde_json::Value::Object(map) => { + for item in map.values() { + collect_owned_leaves(item, out); + } + } + _ => {} + } +} + +fn has_nested_array(value: &serde_json::Value) -> bool { + match value { + serde_json::Value::Array(_) => true, + serde_json::Value::Object(map) => map.values().any(has_nested_array), + _ => false, + } +} + /// Whether any string inside `value` marks it as an st2-rendered element, structurally (see /// [`crate::hooks::is_managed_hook_reference`]). fn contains_owned_string(value: &serde_json::Value) -> bool { @@ -1154,9 +1228,14 @@ mod tests { #[test] fn union_supersedes_prior_hook_set_entries_but_never_foreign_ones() { // The prior set lives under a RELOCATED root — recognition is structural (set-shaped - // path + managed basename), not derived from the current environment. + // path + managed basename), not derived from the current environment — and a MIXED + // matcher group keeps its user hook while losing only the superseded st2 entry. let mut target = serde_json::json!({ "hooks": {"Stop": [ + {"matcher": "Bash", "hooks": [ + {"type": "command", "command": "user-guard.sh"}, + {"type": "command", "command": "/old/root/sets/sha256-aaa/claude-observe.sh Stop"} + ]}, {"hooks": [{"type": "command", "command": "user-audit.sh"}]}, {"hooks": [{"type": "command", "command": "/old/root/sets/sha256-aaa/claude-observe.sh Stop"}]}, {"hooks": [{"type": "command", "command": "/home/x/claude-observe.sh Stop"}]}, @@ -1172,6 +1251,10 @@ mod tests { target, serde_json::json!({ "hooks": {"Stop": [ + // The mixed group survives with only its user hook. + {"matcher": "Bash", "hooks": [ + {"type": "command", "command": "user-guard.sh"} + ]}, {"hooks": [{"type": "command", "command": "user-audit.sh"}]}, // A managed basename OUTSIDE a set-shaped path is a user's wrapper: foreign. {"hooks": [{"type": "command", "command": "/home/x/claude-observe.sh Stop"}]}, diff --git a/src/pi_session.rs b/src/pi_session.rs index 5a5ab7da..e7200c34 100644 --- a/src/pi_session.rs +++ b/src/pi_session.rs @@ -420,9 +420,11 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); let record = harness_state::harness_state_path(tmp.path()); let session = harness_state::session_token(); + // Real wiring order: the wrapper's written claim first, then the channel adopts it. + let seq = harness_state::claim(tmp.path(), "h.worker", "pi", &session).unwrap(); let mut channel = harness_state::Writer::new(tmp.path(), "h.worker", "pi", Some("h.worker".to_string())) - .with_session(session.clone()); + .with_ownership(session.clone(), seq); channel .observe(harness_state::Observation::new( Activity::Active, @@ -438,7 +440,7 @@ mod tests { "pi", "h.worker", &session, - harness_state::claim_seq(tmp.path()), + seq, ); observer.heartbeat(); assert_eq!(std::fs::read(&record).unwrap(), live, "no heartbeat"); diff --git a/src/provider_session.rs b/src/provider_session.rs index 9b40d40a..a632bdf2 100644 --- a/src/provider_session.rs +++ b/src/provider_session.rs @@ -69,24 +69,29 @@ pub(crate) struct SessionObserver { impl SessionObserver { /// `pty_session` is the wrapper's runtime/task ID — the registry entry whose liveness vouches - /// for the record. The observer mints the session incarnation token; the wrapper exports it - /// to its sibling writer processes (hooks, a channel) so ownership — coalescing, heartbeat - /// eligibility, terminal fencing — is decided by token equality across all of them. + /// for the record. The observer mints the session incarnation token and performs the WRITTEN + /// ownership claim (superseding whatever a predecessor left, fresh live records included); + /// the wrapper exports both to its sibling writer processes (hooks, a channel) so ownership + /// — coalescing, heartbeat eligibility, terminal fencing — is decided by the claim across + /// all of them. A claim that cannot be written is fatal at construction: acting without + /// ownership would silently produce a writer every record refuses. pub(crate) fn new( agent_dir: &Path, identity: &str, harness: &'static str, pty_session: &str, - ) -> Self { - Self { - seq: harness_state::claim_seq(agent_dir), + ) -> anyhow::Result { + let session = harness_state::session_token(); + let seq = harness_state::claim(agent_dir, identity, harness, &session)?; + Ok(Self { + seq, agent_dir: agent_dir.to_path_buf(), identity: identity.to_string(), harness, pty_session: pty_session.to_string(), - session: harness_state::session_token(), + session, heartbeats: true, - } + }) } /// An observer that records only how the session ended: `heartbeat` is a no-op because a From 17b08854c878c12038d84b5e0d9b4b47806db8a6 Mon Sep 17 00:00:00 2001 From: Johannes Schickling Date: Mon, 24 Aug 2026 01:55:18 +0200 Subject: [PATCH 09/14] fix(claude,materialize): wrapperless succession, legacy-seat registration, and single-home managed leaves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A wrapperless seat's SessionStart performs the written claim (its only session boundary — token-only writers never claim, so session 2 would otherwise be refused forever), legacy deliver-mcp seats render the same canonical hook registration their claude-session wrapper assumes, and a managed entry survives only inside its canonical group — nested in a user's group it is superseded rather than registered twice. Co-Authored-By: Claude Fable 5 --- src/claude_session.rs | 118 +++++++++++++++++++++++++++++++++--------- src/materialize.rs | 117 ++++++++++++++++++++++++++++------------- 2 files changed, 174 insertions(+), 61 deletions(-) diff --git a/src/claude_session.rs b/src/claude_session.rs index fd4bcf71..1285ba25 100644 --- a/src/claude_session.rs +++ b/src/claude_session.rs @@ -84,32 +84,17 @@ pub fn run_observe( let Some(observation) = observe_hook_event(event, &payload) else { return Ok(()); }; - let pty_session = runtime_id.unwrap_or(identity).to_string(); - let mut writer = harness_state::Writer::new(&agent_dir, identity, "claude", Some(pty_session)); - // The wrapper's exported token makes hook writes this session's records. A wrapperless seat - // (hooks registered on a plain hand-authored launch) falls back to Claude's own session_id — - // stable across one Claude session's hooks, fresh on restart — so restatements still - // coalesce and a restart still opens a new transition; what such a seat lacks is a - // heartbeat/terminal owner, which is a documented hooks-only limitation. - let exported = std::env::var(SESSION_ENV) - .ok() - .filter(|token| !token.is_empty()); - if let Some(session) = exported { - // Full adopted ownership when the wrapper exported it: the claimed sequence makes the - // token directional, so a hook straggling from a superseded session is refused. - writer = match std::env::var(SESSION_SEQ_ENV) + let mut writer = observe_writer( + &agent_dir, + identity, + runtime_id, + event, + &payload, + std::env::var(SESSION_ENV).ok().filter(|t| !t.is_empty()), + std::env::var(SESSION_SEQ_ENV) .ok() - .and_then(|seq| seq.parse::().ok()) - { - Some(seq) => writer.with_ownership(session, seq), - None => writer.with_session(session), - }; - } else if let Some(id) = payload - .get("session_id") - .and_then(serde_json::Value::as_str) - { - writer = writer.with_session(format!("claude-session-{id}")); - } + .and_then(|seq| seq.parse::().ok()), + ); if event == "SessionStart" { // The one event that names a session boundary: even if the new session's first state // matches a fresh predecessor record, continuity must not be claimed across the restart. @@ -121,6 +106,52 @@ pub fn run_observe( writer.observe_unless_ended(observation).map(|_wrote| ()) } +/// Select the ownership a hook write acts under. The wrapper's exported token makes hook writes +/// this session's records (adopted ownership when the claimed sequence travels beside it). A +/// wrapperless seat falls back to Claude's own session_id — and because token-only writers never +/// claim, the SessionStart arm IS that path's session boundary and performs the WRITTEN claim +/// (degrading to token-only with a warning if the claim cannot be written); later hooks of the +/// same session adopt its records by token. What such a seat still lacks is a heartbeat and +/// terminal owner — the documented hooks-only limitation. +#[allow(clippy::too_many_arguments)] +fn observe_writer( + agent_dir: &Path, + identity: &str, + runtime_id: Option<&str>, + event: &str, + payload: &serde_json::Value, + exported_session: Option, + exported_seq: Option, +) -> harness_state::Writer { + let pty_session = runtime_id.unwrap_or(identity).to_string(); + let writer = harness_state::Writer::new(agent_dir, identity, "claude", Some(pty_session)); + if let Some(session) = exported_session { + return match exported_seq { + Some(seq) => writer.with_ownership(session, seq), + None => writer.with_session(session), + }; + } + if let Some(id) = payload + .get("session_id") + .and_then(serde_json::Value::as_str) + { + let token = format!("claude-session-{id}"); + if event == "SessionStart" { + return match harness_state::claim(agent_dir, identity, "claude", &token) { + Ok(seq) => writer.with_ownership(token, seq), + Err(error) => { + eprintln!( + "st2 claude-observe: observed-state claim failed; degrading to token-only: {error:#}" + ); + writer.with_session(token) + } + }; + } + return writer.with_session(token); + } + writer +} + /// Map one Claude hook event to an observation, or `None` when the event says nothing about /// top-level harness state. /// @@ -506,4 +537,41 @@ mod tests { Activity::Idle ); } + + /// W8-12: two sessions of a WRAPPERLESS seat. Session A's hooks write; session B's + /// SessionStart performs the written claim and takes over; A's straggler is refused and B's + /// later hooks adopt B's records. + #[test] + fn a_wrapperless_seat_survives_its_own_session_succession() { + use crate::harness_state::{self, Activity}; + let tmp = tempfile::tempdir().unwrap(); + let record = harness_state_path(tmp.path()); + let payload_a = serde_json::json!({ "session_id": "aaa" }); + let payload_b = serde_json::json!({ "session_id": "bbb" }); + let drive = |event: &str, payload: &serde_json::Value| { + let mut writer = + observe_writer(tmp.path(), "hetz.worker", None, event, payload, None, None); + writer + .observe_unless_ended(observe_hook_event(event, payload).unwrap()) + .unwrap() + }; + + assert!(drive("SessionStart", &payload_a), "A claims"); + assert!(drive("UserPromptSubmit", &payload_a), "A's hooks adopt"); + assert_eq!( + harness_state::read(&record, None).unwrap().state, + Activity::Active + ); + + assert!(drive("SessionStart", &payload_b), "B claims over A"); + assert!( + !drive("PostToolUse", &payload_a), + "A's straggler is refused" + ); + assert!(drive("UserPromptSubmit", &payload_b), "B's hooks adopt"); + assert_eq!( + harness_state::read(&record, None).unwrap().state, + Activity::Active + ); + } } diff --git a/src/materialize.rs b/src/materialize.rs index 49bba9ca..32ea8d00 100644 --- a/src/materialize.rs +++ b/src/materialize.rs @@ -341,6 +341,15 @@ fn effective_plan(root: &Path, spec: &AgentSpec, this_host: &str) -> Result bool { +/// still worth keeping. A managed leaf (no nested arrays) never survives here: it belongs only +/// inside its canonical group, which the caller already retained by whole-element equality — a +/// managed entry nested in a USER's group would otherwise register twice. Containers are pruned +/// recursively and dropped once every nested array is empty — husks that only ever held +/// superseded registrations. +fn keep_after_supersession(element: &mut serde_json::Value) -> bool { if !has_nested_array(element) { - return !contains_owned_string(element) || kept.contains(element); + return !contains_owned_string(element); } let mut any_array_content = false; match element { serde_json::Value::Array(items) => { - items.retain_mut(|item| keep_after_supersession(item, kept)); + items.retain_mut(|item| keep_after_supersession(item)); any_array_content = !items.is_empty(); } serde_json::Value::Object(map) => { for value in map.values_mut() { if let serde_json::Value::Array(items) = value { - items.retain_mut(|item| keep_after_supersession(item, kept)); + items.retain_mut(|item| keep_after_supersession(item)); if !items.is_empty() { any_array_content = true; } } else if has_nested_array(value) { - if keep_after_supersession(value, kept) { + if keep_after_supersession(value) { any_array_content = true; } } @@ -569,29 +576,6 @@ fn keep_after_supersession(element: &mut serde_json::Value, kept: &[serde_json:: any_array_content } -/// Owned leaves — managed entries with no nested arrays — anywhere inside `value`. -fn collect_owned_leaves(value: &serde_json::Value, out: &mut Vec) { - if !has_nested_array(value) { - if contains_owned_string(value) { - out.push(value.clone()); - } - return; - } - match value { - serde_json::Value::Array(items) => { - for item in items { - collect_owned_leaves(item, out); - } - } - serde_json::Value::Object(map) => { - for item in map.values() { - collect_owned_leaves(item, out); - } - } - _ => {} - } -} - fn has_nested_array(value: &serde_json::Value) -> bool { match value { serde_json::Value::Array(_) => true, @@ -1265,4 +1249,65 @@ mod tests { }) ); } + + /// W8-14: a CURRENT managed entry nested inside a USER's group is still superseded — the + /// canonical registration lives only in its own group, or Claude runs it twice. + #[test] + fn a_managed_leaf_survives_only_inside_its_canonical_group() { + let current = "/root/sets/sha256-bbb/claude-observe.sh Stop"; + let mut target = serde_json::json!({ + "hooks": {"Stop": [ + {"matcher": "Bash", "hooks": [ + {"type": "command", "command": "user-guard.sh"}, + {"type": "command", "command": current} + ]} + ]} + }); + let patch = serde_json::json!({ + "hooks": {"Stop": [{"hooks": [{"type": "command", "command": current}]}]} + }); + deep_merge(&mut target, patch.clone(), ArrayMerge::Union); + deep_merge(&mut target, patch, ArrayMerge::Union); + assert_eq!( + target, + serde_json::json!({ + "hooks": {"Stop": [ + {"matcher": "Bash", "hooks": [{"type": "command", "command": "user-guard.sh"}]}, + {"hooks": [{"type": "command", "command": current}]} + ]} + }) + ); + } + + /// W8-13: legacy `deliver "mcp"` seats render the same canonical hook registration the typed + /// driver does — they run under claude-session too. + #[test] + fn legacy_mcp_seats_render_the_canonical_hook_registration() { + let tmp = tempfile::tempdir().unwrap(); + let declaration = tmp.path().join("agents/h/worker/agent.kdl"); + std::fs::create_dir_all(declaration.parent().unwrap()).unwrap(); + std::fs::write( + &declaration, + r#"agent "worker" { host "h"; command "claude"; deliver "mcp"; workspace "$CATALOG" }"#, + ) + .unwrap(); + let found = crate::discover(tmp.path()); + assert!(found.errors.is_empty(), "{:?}", found.errors); + let plan = effective_plan(tmp.path(), &found.specs[0], "h").unwrap(); + let settings = plan + .ops + .iter() + .find_map(|op| match op { + RenderOp::JsonUpsert { + destination, + content, + arrays, + } if destination == ".claude/settings.local.json" => Some((content, arrays)), + _ => None, + }) + .expect("legacy mcp seats register hooks"); + assert_eq!(*settings.1, ArrayMerge::Union); + let rendered: serde_json::Value = serde_json::from_str(settings.0).unwrap(); + assert_eq!(rendered, crate::hooks::claude_settings_registration()); + } } From 028057af9e53258771c6f98702f9e7e3498cef26 Mon Sep 17 00:00:00 2001 From: Johannes Schickling Date: Mon, 24 Aug 2026 02:01:21 +0200 Subject: [PATCH 10/14] fix(claude): the wrapperless session boundary claims only where a live wrapper is not Co-Authored-By: Claude Fable 5 --- src/claude_session.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/claude_session.rs b/src/claude_session.rs index 1285ba25..36a8da2c 100644 --- a/src/claude_session.rs +++ b/src/claude_session.rs @@ -136,7 +136,7 @@ fn observe_writer( .and_then(serde_json::Value::as_str) { let token = format!("claude-session-{id}"); - if event == "SessionStart" { + if event == "SessionStart" && harness_state::wrapperless_claim_allowed(agent_dir) { return match harness_state::claim(agent_dir, identity, "claude", &token) { Ok(seq) => writer.with_ownership(token, seq), Err(error) => { From c786b0e4029e3487b43794a45b4b0dc04332df82 Mon Sep 17 00:00:00 2001 From: Johannes Schickling Date: Mon, 24 Aug 2026 02:23:44 +0200 Subject: [PATCH 11/14] fix(claude): the wrapperless session boundary uses the atomic claim Eligibility and the written takeover are one act under the record lock; ineligible and unwritable both degrade to token-only. Co-Authored-By: Claude Fable 5 --- src/claude_session.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/claude_session.rs b/src/claude_session.rs index 36a8da2c..cc7db370 100644 --- a/src/claude_session.rs +++ b/src/claude_session.rs @@ -135,10 +135,16 @@ fn observe_writer( .get("session_id") .and_then(serde_json::Value::as_str) { - let token = format!("claude-session-{id}"); - if event == "SessionStart" && harness_state::wrapperless_claim_allowed(agent_dir) { - return match harness_state::claim(agent_dir, identity, "claude", &token) { - Ok(seq) => writer.with_ownership(token, seq), + let token = format!("{}{id}", harness_state::WRAPPERLESS_PREFIX); + if event == "SessionStart" { + // Eligibility and the written takeover are ONE act under the record lock: a + // hooks-only SessionStart racing a wrapper's startup can no longer steal the + // sequence between the wrapper's read and its write. Ineligible (a live wrapper or + // its fresh claim placeholder owns the record) or unwritable both degrade to + // token-only. + return match harness_state::claim_wrapperless(agent_dir, identity, "claude", &token) { + Ok(Some(seq)) => writer.with_ownership(token, seq), + Ok(None) => writer.with_session(token), Err(error) => { eprintln!( "st2 claude-observe: observed-state claim failed; degrading to token-only: {error:#}" From 93ffb781dbedb76360efb262727ecc493b6d75ae Mon Sep 17 00:00:00 2001 From: Johannes Schickling Date: Mon, 24 Aug 2026 02:24:59 +0200 Subject: [PATCH 12/14] fix(provider): launch-error arms write a real terminal record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spawn and liveness-check error arms end the record honestly (ended, exit unknown, launch-error) when an observer is present, so the claim placeholder never stands as the visible state of a launch that never ran — while the ordinary nonzero-exit path keeps its real exit. Co-Authored-By: Claude Fable 5 --- src/provider_session.rs | 74 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 66 insertions(+), 8 deletions(-) diff --git a/src/provider_session.rs b/src/provider_session.rs index a632bdf2..09648ff3 100644 --- a/src/provider_session.rs +++ b/src/provider_session.rs @@ -148,6 +148,20 @@ impl SessionObserver { pub(crate) fn ended(&self, exit: &str) { let _ = self.writer().ended(exit); } + + /// The terminal record for a session whose provider never ran (or could no longer be + /// checked): a real ended record, so the claim placeholder is not the last word. + pub(crate) fn launch_error(&self) { + let _ = self.writer().observe( + harness_state::Observation::new( + harness_state::Activity::Ended, + harness_state::BlockedOn::None, + harness_state::InputBuffer::Unknown, + ) + .with_reason("launch-error") + .with_exit("exit unknown"), + ); + } } fn describe_exit(exit: ExitStatus) -> String { @@ -227,19 +241,32 @@ pub(crate) fn run_provider_observed( Ok(()) }); } - let mut child = command - .spawn() - .with_context(|| format!("starting {provider} provider {program}"))?; + // The error arms are terminal outcomes too: the claim placeholder must not stand as the + // visible state after a launch that never ran — while the ordinary nonzero-exit path keeps + // its real exit and is deliberately not covered here. + let mut child = match command.spawn() { + Ok(child) => child, + Err(error) => { + if let Some(observed) = observed { + observed.launch_error(); + } + return Err(error).with_context(|| format!("starting {provider} provider {program}")); + } + }; let mut next_refresh = Instant::now(); loop { if stop.load(Ordering::SeqCst) { return stop_provider_group(&mut child, observed).map(ProviderOutcome::Stopped); } - if let Some(exit) = child - .try_wait() - .with_context(|| format!("checking {provider} provider"))? - { - return Ok(ProviderOutcome::Exited(exit)); + match child.try_wait() { + Ok(Some(exit)) => return Ok(ProviderOutcome::Exited(exit)), + Ok(None) => {} + Err(error) => { + if let Some(observed) = observed { + observed.launch_error(); + } + return Err(error).with_context(|| format!("checking {provider} provider")); + } } let now = Instant::now(); if now >= next_refresh { @@ -291,3 +318,34 @@ fn stop_provider_group( } Ok(child.wait().ok()) } + +#[cfg(test)] +mod tests { + use super::*; + + /// n6: an unspawnable provider is a terminal outcome — the claim placeholder must not stand. + #[test] + fn an_unspawnable_provider_writes_a_real_terminal_record() { + use crate::harness_state::{self, Activity}; + let tmp = tempfile::tempdir().unwrap(); + let observer = + SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker").unwrap(); + let stop = AtomicBool::new(false); + let result = run_provider_observed( + "test", + &crate::status::status_path(tmp.path()), + &["/nonexistent/provider-binary".to_string()], + &[], + Duration::from_secs(60), + Duration::from_millis(5), + &stop, + Some(&observer), + ); + assert!(result.is_err()); + let record = + harness_state::read(&harness_state::harness_state_path(tmp.path()), None).unwrap(); + assert_eq!(record.state, Activity::Ended); + assert_eq!(record.exit.as_deref(), Some("exit unknown")); + assert_eq!(record.reason.as_deref(), Some("launch-error")); + } +} From a7ea487dea28ad0de32000974c34664b7b9346b6 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:06:33 +0200 Subject: [PATCH 13/14] fix(claude): quote generated hook paths, and scope hook pruning to the canonical registration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The $ST_HOOKS expansion in every generated hook command is now double-quoted — a whitespace-containing custom or XDG root made Claude treat only the first path segment as the executable, silently dropping transitions — and the managed-reference recognizer reads the quoted executable token. st2-hook supersession was a property of EVERY arrays="union" merge, so an unrelated union merge could delete user array elements that merely match the managed-path heuristic. It is now the canonical Claude settings registration's maintenance rule only; other destinations union by exact equality alone. Co-Authored-By: Claude Fable 5 agent-identity: unknown agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.3 agent-runtime: OMP 18.0.3 tooling-profile: dotfiles@f33cd9c-dirty --- examples/native/agent-claude.kdl | 18 +++++----- src/hooks.rs | 23 ++++++++---- src/materialize.rs | 61 ++++++++++++++++++++++++++------ 3 files changed, 76 insertions(+), 26 deletions(-) diff --git a/examples/native/agent-claude.kdl b/examples/native/agent-claude.kdl index 8937a221..0822e36d 100644 --- a/examples/native/agent-claude.kdl +++ b/examples/native/agent-claude.kdl @@ -35,11 +35,11 @@ agent "" { "type": "command", "async": true, "asyncRewake": true, - "command": "$ST_HOOKS/claude-session-start.sh" + "command": "\"$ST_HOOKS/claude-session-start.sh\"" }, { "type": "command", - "command": "$ST_HOOKS/claude-observe.sh SessionStart" + "command": "\"$ST_HOOKS/claude-observe.sh\" SessionStart" } ] } @@ -49,7 +49,7 @@ agent "" { "hooks": [ { "type": "command", - "command": "$ST_HOOKS/claude-pre-compact.sh" + "command": "\"$ST_HOOKS/claude-pre-compact.sh\"" } ] } @@ -59,7 +59,7 @@ agent "" { "hooks": [ { "type": "command", - "command": "$ST_HOOKS/claude-stop-failure.sh" + "command": "\"$ST_HOOKS/claude-stop-failure.sh\"" } ] } @@ -69,7 +69,7 @@ agent "" { "hooks": [ { "type": "command", - "command": "$ST_HOOKS/claude-observe.sh UserPromptSubmit" + "command": "\"$ST_HOOKS/claude-observe.sh\" UserPromptSubmit" } ] } @@ -79,7 +79,7 @@ agent "" { "hooks": [ { "type": "command", - "command": "$ST_HOOKS/claude-observe.sh Stop" + "command": "\"$ST_HOOKS/claude-observe.sh\" Stop" } ] } @@ -89,7 +89,7 @@ agent "" { "hooks": [ { "type": "command", - "command": "$ST_HOOKS/claude-observe.sh PermissionRequest" + "command": "\"$ST_HOOKS/claude-observe.sh\" PermissionRequest" } ] } @@ -99,7 +99,7 @@ agent "" { "hooks": [ { "type": "command", - "command": "$ST_HOOKS/claude-observe.sh PreToolUse" + "command": "\"$ST_HOOKS/claude-observe.sh\" PreToolUse" } ] } @@ -109,7 +109,7 @@ agent "" { "hooks": [ { "type": "command", - "command": "$ST_HOOKS/claude-observe.sh PostToolUse" + "command": "\"$ST_HOOKS/claude-observe.sh\" PostToolUse" } ] } diff --git a/src/hooks.rs b/src/hooks.rs index 9057ba83..9feda222 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -112,7 +112,9 @@ pub fn claude_settings_registration() -> serde_json::Value { fn observe(event: &str) -> serde_json::Value { serde_json::json!([{ "hooks": [{ "type": "command", - "command": format!("$ST_HOOKS/claude-observe.sh {event}"), + // Quoted so a whitespace-containing `$ST_HOOKS` root — a custom or XDG state + // layout — still resolves to one executable when Claude runs the command. + "command": format!("\"$ST_HOOKS/claude-observe.sh\" {event}"), }] }]) } serde_json::json!({ @@ -123,20 +125,20 @@ pub fn claude_settings_registration() -> serde_json::Value { "type": "command", "async": true, "asyncRewake": true, - "command": "$ST_HOOKS/claude-session-start.sh", + "command": "\"$ST_HOOKS/claude-session-start.sh\"", }, { "type": "command", - "command": "$ST_HOOKS/claude-observe.sh SessionStart", + "command": "\"$ST_HOOKS/claude-observe.sh\" SessionStart", }, ] }], "PreCompact": [{ "hooks": [{ "type": "command", - "command": "$ST_HOOKS/claude-pre-compact.sh", + "command": "\"$ST_HOOKS/claude-pre-compact.sh\"", }] }], "StopFailure": [{ "hooks": [{ "type": "command", - "command": "$ST_HOOKS/claude-stop-failure.sh", + "command": "\"$ST_HOOKS/claude-stop-failure.sh\"", }] }], "UserPromptSubmit": observe("UserPromptSubmit"), "Stop": observe("Stop"), @@ -153,9 +155,16 @@ pub fn claude_settings_registration() -> serde_json::Value { /// `${ST_HOOKS}/...` — `$ST_HOOKS_SUFFIX` is somebody else's variable), and an expanded path /// whose basename is a managed hook file sitting under a set-shaped directory /// (`.../sets/sha256-.../`), which recognizes every set version under every past -/// or relocated root without consulting the current environment. +/// or relocated root without consulting the current environment. Each spelling may be +/// double-quoted — the generated commands quote the executable so whitespace-containing roots +/// survive the shell — and the quoted executable token (not the first whitespace token) is +/// what is inspected. pub(crate) fn is_managed_hook_reference(text: &str) -> bool { - let command = text.split_whitespace().next().unwrap_or(""); + let command = if let Some(quoted) = text.strip_prefix('"') { + quoted.split('"').next().unwrap_or("") + } else { + text.split_whitespace().next().unwrap_or("") + }; if let Some(rest) = command.strip_prefix("$ST_HOOKS") { return rest.is_empty() || rest.starts_with('/'); } diff --git a/src/materialize.rs b/src/materialize.rs index 32ea8d00..6940af5d 100644 --- a/src/materialize.rs +++ b/src/materialize.rs @@ -503,12 +503,17 @@ fn ensure_line(path: &Path, line: &str) -> Result { Ok(true) } -fn deep_merge(target: &mut serde_json::Value, patch: serde_json::Value, arrays: ArrayMerge) { +fn deep_merge( + target: &mut serde_json::Value, + patch: serde_json::Value, + arrays: ArrayMerge, + supersede_managed_hooks: bool, +) { match (target, patch) { (serde_json::Value::Object(target), serde_json::Value::Object(patch)) => { for (key, value) in patch { match target.get_mut(&key) { - Some(existing) => deep_merge(existing, value, arrays), + Some(existing) => deep_merge(existing, value, arrays, supersede_managed_hooks), None => { target.insert(key, value); } @@ -526,7 +531,9 @@ fn deep_merge(target: &mut serde_json::Value, patch: serde_json::Value, arrays: // longer states are removed, so a matcher group holding a user hook beside an st2 // one keeps the user's; containers left with nothing but empty arrays are dropped. target.retain_mut(|element| { - if patch.contains(element) || !contains_owned_string(element) { + if patch.contains(element) || !supersede_managed_hooks + || !contains_owned_string(element) + { return true; } keep_after_supersession(element) @@ -919,7 +926,11 @@ pub fn materialize_agent(root: &Path, spec: &AgentSpec, this_host: &str) -> Resu spec.identity ) })?; - deep_merge(&mut target, patch, arrays); + // st2-hook supersession is the CANONICAL Claude registration's maintenance + // rule, not a property of union merges: an unrelated union merge must never + // delete user array elements that merely match the managed-path heuristic. + let supersede_managed_hooks = raw_destination == ".claude/settings.local.json"; + deep_merge(&mut target, patch, arrays, supersede_managed_hooks); let mut bytes = serde_json::to_vec_pretty(&target)?; bytes.push(b'\n'); let note = format!("{}: upserted {}", spec.identity, raw_destination); @@ -1172,6 +1183,7 @@ mod tests { "array": [2] }), ArrayMerge::Replace, + false, ); assert_eq!( target, @@ -1193,8 +1205,8 @@ mod tests { let ours = serde_json::json!({ "hooks": {"Stop": [{"hooks": [{"type": "command", "command": "$ST_HOOKS/claude-observe.sh Stop"}]}]} }); - deep_merge(&mut target, ours.clone(), ArrayMerge::Union); - deep_merge(&mut target, ours, ArrayMerge::Union); + deep_merge(&mut target, ours.clone(), ArrayMerge::Union, false); + deep_merge(&mut target, ours, ArrayMerge::Union, false); assert_eq!( target, serde_json::json!({ @@ -1229,8 +1241,8 @@ mod tests { let upgraded = serde_json::json!({ "hooks": {"Stop": [{"hooks": [{"type": "command", "command": "/new/root/sets/sha256-bbb/claude-observe.sh Stop"}]}]} }); - deep_merge(&mut target, upgraded.clone(), ArrayMerge::Union); - deep_merge(&mut target, upgraded, ArrayMerge::Union); + deep_merge(&mut target, upgraded.clone(), ArrayMerge::Union, true); + deep_merge(&mut target, upgraded, ArrayMerge::Union, true); assert_eq!( target, serde_json::json!({ @@ -1266,8 +1278,8 @@ mod tests { let patch = serde_json::json!({ "hooks": {"Stop": [{"hooks": [{"type": "command", "command": current}]}]} }); - deep_merge(&mut target, patch.clone(), ArrayMerge::Union); - deep_merge(&mut target, patch, ArrayMerge::Union); + deep_merge(&mut target, patch.clone(), ArrayMerge::Union, true); + deep_merge(&mut target, patch, ArrayMerge::Union, true); assert_eq!( target, serde_json::json!({ @@ -1279,6 +1291,35 @@ mod tests { ); } + /// Supersession is the CANONICAL Claude registration's maintenance rule, not a property + /// of union merges: the same merge against any other destination must never delete user + /// array elements that merely match the managed-path heuristic. + #[test] + fn union_outside_the_canonical_settings_never_prunes_managed_shaped_entries() { + let mut target = serde_json::json!({ + "tools": {"audit": [ + {"command": "user-audit.sh"}, + {"command": "/old/root/sets/sha256-aaa/claude-observe.sh Stop"} + ]} + }); + let patch = serde_json::json!({ + "tools": {"audit": [{"command": "vendor-check.sh"}]} + }); + deep_merge(&mut target, patch, ArrayMerge::Union, false); + assert_eq!( + target, + serde_json::json!({ + "tools": {"audit": [ + {"command": "user-audit.sh"}, + // A managed-SHAPED path is only superseded where st2 owns the file; + // here it is somebody's data. + {"command": "/old/root/sets/sha256-aaa/claude-observe.sh Stop"}, + {"command": "vendor-check.sh"} + ]} + }) + ); + } + /// W8-13: legacy `deliver "mcp"` seats render the same canonical hook registration the typed /// driver does — they run under claude-session too. #[test] From 94af370f6a9974a18c797c7405487e021609ab94 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:43:41 +0200 Subject: [PATCH 14/14] test(claude): the expansion snapshot carries the quoted hook commands Co-Authored-By: Claude Fable 5 agent-identity: unknown agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.3 agent-runtime: OMP 18.0.3 tooling-profile: dotfiles@f33cd9c-dirty --- tests/fixtures/driver/claude.out.kdl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fixtures/driver/claude.out.kdl b/tests/fixtures/driver/claude.out.kdl index 60548aa0..4ffc439f 100644 --- a/tests/fixtures/driver/claude.out.kdl +++ b/tests/fixtures/driver/claude.out.kdl @@ -1,5 +1,5 @@ render { json-upsert .mcp.json "{\n \"mcpServers\": {\n \"st2\": {\n \"args\": [\n \"--catalog\",\n \"$CATALOG\",\n \"driver\",\n \"claude-mcp\",\n \"--identity\",\n \"Silber.fabric\"\n ],\n \"command\": \"st2\",\n \"type\": \"stdio\"\n }\n }\n}" - json-upsert ".claude/settings.local.json" "{\n \"$schema\": \"https://json.schemastore.org/claude-code-settings.json\",\n \"hooks\": {\n \"PermissionRequest\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh PermissionRequest\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"PostToolUse\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh PostToolUse\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"PreCompact\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-pre-compact.sh\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"PreToolUse\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh PreToolUse\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"SessionStart\": [\n {\n \"hooks\": [\n {\n \"async\": true,\n \"asyncRewake\": true,\n \"command\": \"$ST_HOOKS/claude-session-start.sh\",\n \"type\": \"command\"\n },\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh SessionStart\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"Stop\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh Stop\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"StopFailure\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-stop-failure.sh\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"UserPromptSubmit\": [\n {\n \"hooks\": [\n {\n \"command\": \"$ST_HOOKS/claude-observe.sh UserPromptSubmit\",\n \"type\": \"command\"\n }\n ]\n }\n ]\n }\n}" arrays=union + json-upsert ".claude/settings.local.json" "{\n \"$schema\": \"https://json.schemastore.org/claude-code-settings.json\",\n \"hooks\": {\n \"PermissionRequest\": [\n {\n \"hooks\": [\n {\n \"command\": \"\\\"$ST_HOOKS/claude-observe.sh\\\" PermissionRequest\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"PostToolUse\": [\n {\n \"hooks\": [\n {\n \"command\": \"\\\"$ST_HOOKS/claude-observe.sh\\\" PostToolUse\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"PreCompact\": [\n {\n \"hooks\": [\n {\n \"command\": \"\\\"$ST_HOOKS/claude-pre-compact.sh\\\"\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"PreToolUse\": [\n {\n \"hooks\": [\n {\n \"command\": \"\\\"$ST_HOOKS/claude-observe.sh\\\" PreToolUse\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"SessionStart\": [\n {\n \"hooks\": [\n {\n \"async\": true,\n \"asyncRewake\": true,\n \"command\": \"\\\"$ST_HOOKS/claude-session-start.sh\\\"\",\n \"type\": \"command\"\n },\n {\n \"command\": \"\\\"$ST_HOOKS/claude-observe.sh\\\" SessionStart\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"Stop\": [\n {\n \"hooks\": [\n {\n \"command\": \"\\\"$ST_HOOKS/claude-observe.sh\\\" Stop\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"StopFailure\": [\n {\n \"hooks\": [\n {\n \"command\": \"\\\"$ST_HOOKS/claude-stop-failure.sh\\\"\",\n \"type\": \"command\"\n }\n ]\n }\n ],\n \"UserPromptSubmit\": [\n {\n \"hooks\": [\n {\n \"command\": \"\\\"$ST_HOOKS/claude-observe.sh\\\" UserPromptSubmit\",\n \"type\": \"command\"\n }\n ]\n }\n ]\n }\n}" arrays=union } argv st2 --catalog $CATALOG driver claude-session --identity Silber.fabric --runtime-id Silber.fabric -- claude --model opus --effort xhigh "--dangerously-load-development-channels=server:st2" --permission-mode bypassPermissions --model override "Start the assigned work."