Skip to content

Commit c86a7f7

Browse files
schicklingclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 4f278fc commit c86a7f7

5 files changed

Lines changed: 257 additions & 16 deletions

File tree

examples/native/agent-claude.kdl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ agent "<identity>" {
1818
copy "assets/bus.st2.md" ".st2/bus.md"
1919
ensure-line ".claude/rules/st2.md" "@../../.st2/PERSONA.md"
2020
ensure-line ".claude/rules/st2.md" "@../../.st2/bus.md"
21+
// Hooks-only observability: this seat launches claude directly (no session wrapper), so the
22+
// registrations below give it transitions and blocked-on-you — but no heartbeat owner and no
23+
// terminal record. A live-but-idle seat ages to `unknown` after the staleness horizon and an
24+
// exit leaves the last state to age out; both read indeterminate, never wrong. The full
25+
// producer (heartbeats, terminal exits) comes with the session wrapper, i.e. a typed
26+
// `claude {}` driver seat or `deliver "mcp"`.
2127
json-upsert ".claude/settings.local.json" arrays="union" #"""
2228
{
2329
"$schema": "https://json.schemastore.org/claude-code-settings.json",

src/claude_session.rs

Lines changed: 85 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,12 @@ pub fn run(
3939
let observer = SessionObserver::new(&agent_dir, &identity, "claude", &runtime_id);
4040
// The runtime ID reaches hook subprocesses through the provider environment, so their
4141
// transitions carry the same pty session the wrapper's records do.
42-
let env = [(RUNTIME_ID_ENV.to_string(), runtime_id.clone())];
42+
let env = [
43+
(RUNTIME_ID_ENV.to_string(), runtime_id.clone()),
44+
// Hook subprocesses adopt the wrapper's incarnation token, so their transitions are this
45+
// session's records: the wrapper can re-stamp them, and its terminal record fences them.
46+
(SESSION_ENV.to_string(), observer.session().to_string()),
47+
];
4348
run_provider(
4449
"Claude",
4550
&status::status_path(&agent_dir),
@@ -59,6 +64,8 @@ pub fn run(
5964
/// short-lived writer; the transition counter continues from disk.
6065
/// The env var carrying the wrapper's runtime/task ID into Claude's hook subprocesses.
6166
pub const RUNTIME_ID_ENV: &str = "ST2_CLAUDE_RUNTIME_ID";
67+
/// The env var carrying the wrapper's session incarnation token into Claude's hook subprocesses.
68+
pub const SESSION_ENV: &str = "ST2_CLAUDE_SESSION";
6269

6370
pub fn run_observe(
6471
catalog_root: &Path,
@@ -76,12 +83,32 @@ pub fn run_observe(
7683
};
7784
let pty_session = runtime_id.unwrap_or(identity).to_string();
7885
let mut writer = harness_state::Writer::new(&agent_dir, identity, "claude", Some(pty_session));
86+
// The wrapper's exported token makes hook writes this session's records. A wrapperless seat
87+
// (hooks registered on a plain hand-authored launch) falls back to Claude's own session_id —
88+
// stable across one Claude session's hooks, fresh on restart — so restatements still
89+
// coalesce and a restart still opens a new transition; what such a seat lacks is a
90+
// heartbeat/terminal owner, which is a documented hooks-only limitation.
91+
let session = std::env::var(SESSION_ENV)
92+
.ok()
93+
.filter(|token| !token.is_empty())
94+
.or_else(|| {
95+
payload
96+
.get("session_id")
97+
.and_then(serde_json::Value::as_str)
98+
.map(|id| format!("claude-session-{id}"))
99+
});
100+
if let Some(session) = session {
101+
writer = writer.with_session(session);
102+
}
79103
if event == "SessionStart" {
80104
// The one event that names a session boundary: even if the new session's first state
81105
// matches a fresh predecessor record, continuity must not be claimed across the restart.
82106
writer.interrupt();
83107
}
84-
writer.observe(observation)
108+
// A late hook finishing after the wrapper reaped Claude must not replace the terminal record
109+
// with a live state: the wrapper's `ended` carries this same token and is the session's last
110+
// word. (`false` = suppressed; the hook has nothing else to do with it.)
111+
writer.observe_unless_ended(observation).map(|_wrote| ())
85112
}
86113

87114
/// Map one Claude hook event to an observation, or `None` when the event says nothing about
@@ -286,13 +313,15 @@ mod tests {
286313
let record = harness_state_path(tmp.path());
287314
let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker");
288315

289-
// A hook process wrote a blocked observation between wrapper ticks.
316+
// A hook process wrote a blocked observation between wrapper ticks — carrying the
317+
// wrapper's exported token, exactly as the env plumbing arranges in a real seat.
290318
harness_state::Writer::new(
291319
tmp.path(),
292320
"hetz.worker",
293321
"claude",
294322
Some("hetz.worker".to_string()),
295323
)
324+
.with_session(observer.session())
296325
.observe(observe_hook_event("PermissionRequest", &serde_json::Value::Null).unwrap())
297326
.unwrap();
298327
let before = fs::read(&record).unwrap();
@@ -387,4 +416,57 @@ mod tests {
387416
let idle = observe_hook_event("Stop", &serde_json::json!({})).unwrap();
388417
assert_eq!(idle.ask, Ask::None);
389418
}
419+
420+
/// T2: a hook that finishes after the wrapper reaped Claude must not replace the terminal
421+
/// record — the wrapper's `ended` carries the shared token and is the session's last word —
422+
/// while a NEW session's boundary event still supersedes an old terminal record.
423+
#[test]
424+
fn a_late_hook_never_overwrites_this_sessions_terminal_record() {
425+
use crate::harness_state::{self, Activity};
426+
let tmp = tempfile::tempdir().unwrap();
427+
let record = harness_state_path(tmp.path());
428+
let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker");
429+
observer.ended("exit 0");
430+
431+
// The straggler hook shares the session token (env plumbing) and is suppressed.
432+
let mut late = harness_state::Writer::new(
433+
tmp.path(),
434+
"hetz.worker",
435+
"claude",
436+
Some("hetz.worker".to_string()),
437+
)
438+
.with_session(observer.session());
439+
assert!(
440+
!late
441+
.observe_unless_ended(
442+
observe_hook_event("PostToolUse", &serde_json::Value::Null).unwrap()
443+
)
444+
.unwrap()
445+
);
446+
assert_eq!(
447+
harness_state::read(&record, None).unwrap().state,
448+
Activity::Ended
449+
);
450+
451+
// A new Claude session is a new incarnation: its SessionStart replaces the old terminal.
452+
let mut fresh = harness_state::Writer::new(
453+
tmp.path(),
454+
"hetz.worker",
455+
"claude",
456+
Some("hetz.worker".to_string()),
457+
)
458+
.with_session("claude-session-fresh");
459+
fresh.interrupt();
460+
assert!(
461+
fresh
462+
.observe_unless_ended(
463+
observe_hook_event("SessionStart", &serde_json::Value::Null).unwrap()
464+
)
465+
.unwrap()
466+
);
467+
assert_eq!(
468+
harness_state::read(&record, None).unwrap().state,
469+
Activity::Idle
470+
);
471+
}
390472
}

src/materialize.rs

Lines changed: 80 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -494,12 +494,17 @@ fn ensure_line(path: &Path, line: &str) -> Result<bool> {
494494
Ok(true)
495495
}
496496

497-
fn deep_merge(target: &mut serde_json::Value, patch: serde_json::Value, arrays: ArrayMerge) {
497+
fn deep_merge(
498+
target: &mut serde_json::Value,
499+
patch: serde_json::Value,
500+
arrays: ArrayMerge,
501+
owned_prefixes: &[String],
502+
) {
498503
match (target, patch) {
499504
(serde_json::Value::Object(target), serde_json::Value::Object(patch)) => {
500505
for (key, value) in patch {
501506
match target.get_mut(&key) {
502-
Some(existing) => deep_merge(existing, value, arrays),
507+
Some(existing) => deep_merge(existing, value, arrays, owned_prefixes),
503508
None => {
504509
target.insert(key, value);
505510
}
@@ -509,6 +514,14 @@ fn deep_merge(target: &mut serde_json::Value, patch: serde_json::Value, arrays:
509514
(serde_json::Value::Array(target), serde_json::Value::Array(patch))
510515
if arrays == ArrayMerge::Union =>
511516
{
517+
// Exact-equality union alone would accumulate st2's own entries across hook-set
518+
// upgrades: `$ST_HOOKS` expands content-addressed, so every upgrade renders each
519+
// entry with a new path and the old one would be retained beside it. An element
520+
// recognizably st2's — one referencing the hook root — that the patch no longer
521+
// states is therefore superseded and dropped; foreign entries are never touched.
522+
target.retain(|element| {
523+
!contains_owned_string(element, owned_prefixes) || patch.contains(element)
524+
});
512525
for element in patch {
513526
if !target.contains(&element) {
514527
target.push(element);
@@ -519,6 +532,35 @@ fn deep_merge(target: &mut serde_json::Value, patch: serde_json::Value, arrays:
519532
}
520533
}
521534

535+
/// Whether any string inside `value` marks it as an st2-rendered element: a reference to the
536+
/// installed hook root (any set version) or the unexpanded `$ST_HOOKS` variable.
537+
fn contains_owned_string(value: &serde_json::Value, owned_prefixes: &[String]) -> bool {
538+
match value {
539+
serde_json::Value::String(text) => owned_prefixes
540+
.iter()
541+
.any(|prefix| text.starts_with(prefix.as_str())),
542+
serde_json::Value::Array(items) => items
543+
.iter()
544+
.any(|item| contains_owned_string(item, owned_prefixes)),
545+
serde_json::Value::Object(map) => map
546+
.values()
547+
.any(|item| contains_owned_string(item, owned_prefixes)),
548+
_ => false,
549+
}
550+
}
551+
552+
/// The string prefixes that mark a JSON element as st2-rendered for union supersession: the hook
553+
/// root that contains every installed set version, and the unexpanded variable spellings.
554+
fn owned_union_prefixes(env: &BTreeMap<String, String>) -> Vec<String> {
555+
let mut prefixes = vec!["$ST_HOOKS".to_string(), "${ST_HOOKS}".to_string()];
556+
if let Some(hooks) = env.get("ST_HOOKS")
557+
&& let Some(root) = Path::new(hooks).parent()
558+
{
559+
prefixes.push(format!("{}/", root.display()));
560+
}
561+
prefixes
562+
}
563+
522564
fn git_exclude(workspace: &Path, line: &str) -> Result<bool> {
523565
let output = Command::new("git")
524566
.args(["-C"])
@@ -843,7 +885,7 @@ pub fn materialize_agent(root: &Path, spec: &AgentSpec, this_host: &str) -> Resu
843885
spec.identity
844886
)
845887
})?;
846-
deep_merge(&mut target, patch, arrays);
888+
deep_merge(&mut target, patch, arrays, &owned_union_prefixes(&env));
847889
let mut bytes = serde_json::to_vec_pretty(&target)?;
848890
bytes.push(b'\n');
849891
let note = format!("{}: upserted {}", spec.identity, raw_destination);
@@ -1096,6 +1138,7 @@ mod tests {
10961138
"array": [2]
10971139
}),
10981140
ArrayMerge::Replace,
1141+
&[],
10991142
);
11001143
assert_eq!(
11011144
target,
@@ -1117,8 +1160,8 @@ mod tests {
11171160
let ours = serde_json::json!({
11181161
"hooks": {"Stop": [{"hooks": [{"type": "command", "command": "$ST_HOOKS/claude-observe.sh Stop"}]}]}
11191162
});
1120-
deep_merge(&mut target, ours.clone(), ArrayMerge::Union);
1121-
deep_merge(&mut target, ours, ArrayMerge::Union);
1163+
deep_merge(&mut target, ours.clone(), ArrayMerge::Union, &[]);
1164+
deep_merge(&mut target, ours, ArrayMerge::Union, &[]);
11221165
assert_eq!(
11231166
target,
11241167
serde_json::json!({
@@ -1129,4 +1172,36 @@ mod tests {
11291172
})
11301173
);
11311174
}
1175+
1176+
/// A hook-set upgrade renders every entry under a new content-addressed path. Union must
1177+
/// supersede st2's prior entries — recognizable by the hook root — rather than accumulate
1178+
/// them, while a user's entry under any other path survives every merge.
1179+
#[test]
1180+
fn union_supersedes_prior_hook_set_entries_but_never_foreign_ones() {
1181+
let owned = vec![
1182+
"$ST_HOOKS".to_string(),
1183+
"${ST_HOOKS}".to_string(),
1184+
"/state/st2/hooks/".to_string(),
1185+
];
1186+
let mut target = serde_json::json!({
1187+
"hooks": {"Stop": [
1188+
{"hooks": [{"type": "command", "command": "user-audit.sh"}]},
1189+
{"hooks": [{"type": "command", "command": "/state/st2/hooks/set-v1/claude-observe.sh Stop"}]}
1190+
]}
1191+
});
1192+
let upgraded = serde_json::json!({
1193+
"hooks": {"Stop": [{"hooks": [{"type": "command", "command": "/state/st2/hooks/set-v2/claude-observe.sh Stop"}]}]}
1194+
});
1195+
deep_merge(&mut target, upgraded.clone(), ArrayMerge::Union, &owned);
1196+
deep_merge(&mut target, upgraded, ArrayMerge::Union, &owned);
1197+
assert_eq!(
1198+
target,
1199+
serde_json::json!({
1200+
"hooks": {"Stop": [
1201+
{"hooks": [{"type": "command", "command": "user-audit.sh"}]},
1202+
{"hooks": [{"type": "command", "command": "/state/st2/hooks/set-v2/claude-observe.sh Stop"}]}
1203+
]}
1204+
})
1205+
);
1206+
}
11321207
}

src/pi_session.rs

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,17 @@ pub fn run(
7070
})?;
7171
let pi_argv = with_channel_extension(pi_argv, &set)?;
7272
install_signal_handler();
73+
// Terminal-only: the channel owns the live record and its heartbeat, but only this wrapper
74+
// survives long enough to see the stop path — its pre-escalation `ended` write is the one
75+
// that makes `Stopped(None)` observable at all. Same token as the channel, so the terminal
76+
// record fences exactly this session's live records.
77+
let observer = crate::provider_session::SessionObserver::terminal_only(
78+
&agent_dir,
79+
&identity,
80+
"pi",
81+
&runtime_id,
82+
&session,
83+
);
7384
let outcome = run_provider_observed(
7485
"pi",
7586
&status::status_path(&agent_dir),
@@ -78,7 +89,7 @@ pub fn run(
7889
status::STATUS_REFRESH,
7990
PROVIDER_POLL,
8091
&STOP,
81-
None,
92+
Some(&observer),
8293
)
8394
.with_context(|| format!("running pi driver '{runtime_id}'"))?;
8495
record_session_end(&agent_dir, &identity, &runtime_id, &session, &outcome);
@@ -352,4 +363,40 @@ mod tests {
352363
]
353364
);
354365
}
366+
367+
/// W6: the terminal-only observer records how the session ended but never re-stamps live
368+
/// state — the channel owns the heartbeat — and its token makes the write this session's.
369+
#[test]
370+
fn the_terminal_only_observer_ends_but_never_heartbeats() {
371+
use crate::harness_state::{self, Activity};
372+
let tmp = tempfile::tempdir().unwrap();
373+
let record = harness_state::harness_state_path(tmp.path());
374+
let session = harness_state::session_token();
375+
let mut channel =
376+
harness_state::Writer::new(tmp.path(), "h.worker", "pi", Some("h.worker".to_string()))
377+
.with_session(session.clone());
378+
channel
379+
.observe(harness_state::Observation::new(
380+
Activity::Active,
381+
harness_state::BlockedOn::None,
382+
harness_state::InputBuffer::Unknown,
383+
))
384+
.unwrap();
385+
let live = std::fs::read(&record).unwrap();
386+
387+
let observer = crate::provider_session::SessionObserver::terminal_only(
388+
tmp.path(),
389+
"h.worker",
390+
"pi",
391+
"h.worker",
392+
&session,
393+
);
394+
observer.heartbeat();
395+
assert_eq!(std::fs::read(&record).unwrap(), live, "no heartbeat");
396+
397+
observer.ended("signal 9");
398+
let observed = harness_state::read(&record, None).unwrap();
399+
assert_eq!(observed.state, Activity::Ended);
400+
assert_eq!(observed.exit.as_deref(), Some("signal 9"));
401+
}
355402
}

0 commit comments

Comments
 (0)