Skip to content

Commit c36233d

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

7 files changed

Lines changed: 157 additions & 24 deletions

File tree

examples/native/agent-claude.kdl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ 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-
json-upsert ".claude/settings.local.json" #"""
21+
json-upsert ".claude/settings.local.json" arrays="union" #"""
2222
{
2323
"$schema": "https://json.schemastore.org/claude-code-settings.json",
2424
"hooks": {

src/catalog_transaction.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -922,6 +922,7 @@ fn normalize_agent(spec: &agent_spec::AgentSpec) -> Result<BTreeMap<String, Sema
922922
RenderOp::JsonUpsert {
923923
destination,
924924
content,
925+
arrays,
925926
} => {
926927
insert_value(
927928
&mut fields,
@@ -945,6 +946,14 @@ fn normalize_agent(spec: &agent_spec::AgentSpec) -> Result<BTreeMap<String, Sema
945946
SemanticType::String,
946947
&normalized,
947948
);
949+
if *arrays == crate::materialize::ArrayMerge::Union {
950+
insert_value(
951+
&mut fields,
952+
&format!("{root}/arrays"),
953+
SemanticType::String,
954+
"union",
955+
);
956+
}
948957
}
949958
RenderOp::EnsureLine { destination, line } => {
950959
insert_value(

src/claude_session.rs

Lines changed: 63 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use std::path::Path;
1515

1616
use anyhow::{Context as _, Result};
1717

18-
use crate::harness_state::{Activity, BlockedOn, InputBuffer, Observation};
18+
use crate::harness_state::{Activity, Ask, BlockedOn, InputBuffer, Observation};
1919
use crate::provider_session::{
2020
PROVIDER_POLL, STOP, SessionObserver, install_signal_handler, run_provider,
2121
};
@@ -75,8 +75,13 @@ pub fn run_observe(
7575
return Ok(());
7676
};
7777
let pty_session = runtime_id.unwrap_or(identity).to_string();
78-
harness_state::Writer::new(&agent_dir, identity, "claude", Some(pty_session))
79-
.observe(observation)
78+
let mut writer = harness_state::Writer::new(&agent_dir, identity, "claude", Some(pty_session));
79+
if event == "SessionStart" {
80+
// The one event that names a session boundary: even if the new session's first state
81+
// matches a fresh predecessor record, continuity must not be claimed across the restart.
82+
writer.interrupt();
83+
}
84+
writer.observe(observation)
8085
}
8186

8287
/// 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<Ob
117122
BlockedOn::None,
118123
InputBuffer::Unknown,
119124
)),
120-
"PermissionRequest" => Some(
121-
Observation::new(Activity::Active, BlockedOn::Human, InputBuffer::Unknown)
122-
.with_reason("permissionRequest"),
123-
),
125+
"PermissionRequest" => {
126+
// Driver-side classification (#162): the payload's tool_name distinguishes Claude's
127+
// question form from an ordinary permission prompt — the DQ-H1 captures show
128+
// AskUserQuestion arriving as a PermissionRequest like any other tool.
129+
let ask = if payload.get("tool_name").and_then(serde_json::Value::as_str)
130+
== Some("AskUserQuestion")
131+
{
132+
Ask::Question
133+
} else {
134+
Ask::Permission
135+
};
136+
Some(
137+
Observation::new(Activity::Active, BlockedOn::Human, InputBuffer::Unknown)
138+
.with_ask(ask)
139+
.with_reason("permissionRequest"),
140+
)
141+
}
124142
_ => None,
125143
}
126144
}
@@ -269,9 +287,14 @@ mod tests {
269287
let observer = SessionObserver::new(tmp.path(), "hetz.worker", "claude", "hetz.worker");
270288

271289
// A hook process wrote a blocked observation between wrapper ticks.
272-
harness_state::Writer::new(tmp.path(), "hetz.worker", "claude", None)
273-
.observe(observe_hook_event("PermissionRequest", &serde_json::Value::Null).unwrap())
274-
.unwrap();
290+
harness_state::Writer::new(
291+
tmp.path(),
292+
"hetz.worker",
293+
"claude",
294+
Some("hetz.worker".to_string()),
295+
)
296+
.observe(observe_hook_event("PermissionRequest", &serde_json::Value::Null).unwrap())
297+
.unwrap();
275298
let before = fs::read(&record).unwrap();
276299

277300
std::thread::sleep(Duration::from_millis(2));
@@ -293,9 +316,14 @@ mod tests {
293316
let stop = AtomicBool::new(false);
294317

295318
// A turn is in flight when the provider dies by signal.
296-
harness_state::Writer::new(tmp.path(), "hetz.worker", "claude", None)
297-
.observe(observe_hook_event("UserPromptSubmit", &serde_json::Value::Null).unwrap())
298-
.unwrap();
319+
harness_state::Writer::new(
320+
tmp.path(),
321+
"hetz.worker",
322+
"claude",
323+
Some("hetz.worker".to_string()),
324+
)
325+
.observe(observe_hook_event("UserPromptSubmit", &serde_json::Value::Null).unwrap())
326+
.unwrap();
299327

300328
let result = run_provider(
301329
"Claude",
@@ -337,4 +365,26 @@ mod tests {
337365
assert_eq!(observed.state, Activity::Ended);
338366
assert_eq!(observed.exit.as_deref(), Some("exit 0"));
339367
}
368+
369+
#[test]
370+
fn permission_requests_classify_their_ask_kind_from_the_tool_name() {
371+
use crate::harness_state::Ask;
372+
let permission = observe_hook_event(
373+
"PermissionRequest",
374+
&serde_json::json!({ "tool_name": "Bash", "tool_input": {} }),
375+
)
376+
.unwrap();
377+
assert_eq!(permission.ask, Ask::Permission);
378+
379+
let question = observe_hook_event(
380+
"PermissionRequest",
381+
&serde_json::json!({ "tool_name": "AskUserQuestion", "tool_input": {} }),
382+
)
383+
.unwrap();
384+
assert_eq!(question.ask, Ask::Question);
385+
386+
// Non-blocking events carry no ask.
387+
let idle = observe_hook_event("Stop", &serde_json::json!({})).unwrap();
388+
assert_eq!(idle.ask, Ask::None);
389+
}
340390
}

src/driver.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,18 @@ fn expand_claude(driver: &ClaudeDriver, bus_id: &str) -> Result<KdlDocument> {
125125
let mut render = KdlNode::new("render");
126126
render.set_children(document([
127127
node("json-upsert", vec![".mcp.json".to_string(), mcp]),
128-
node(
129-
"json-upsert",
130-
vec![".claude/settings.local.json".to_string(), settings],
131-
),
128+
{
129+
// Hook arrays join whatever the workspace already declares: replacement would clobber
130+
// user-registered hooks on every materialization, and union is idempotent.
131+
let mut upsert = node(
132+
"json-upsert",
133+
vec![".claude/settings.local.json".to_string(), settings],
134+
);
135+
upsert
136+
.entries_mut()
137+
.push(KdlEntry::new_prop("arrays", "union"));
138+
upsert
139+
},
132140
]));
133141

134142
let mut provider = vec!["claude".to_string()];

src/hooks.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,9 @@ mod tests {
582582
let mut entries = node
583583
.entries()
584584
.iter()
585+
// Positional arguments only: properties (e.g. `arrays="union"`) are merge
586+
// strategy, not payload.
587+
.filter(|entry| entry.name().is_none())
585588
.filter_map(|entry| match entry.value() {
586589
kdl::KdlValue::String(value) => Some(value.as_str()),
587590
_ => None,

src/materialize.rs

Lines changed: 68 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ pub enum RenderOp {
2626
JsonUpsert {
2727
destination: String,
2828
content: String,
29+
arrays: ArrayMerge,
2930
},
3031
EnsureLine {
3132
destination: String,
@@ -36,6 +37,17 @@ pub enum RenderOp {
3637
},
3738
}
3839

40+
/// How a json-upsert treats an array both sides declare. `Replace` is the default and the
41+
/// original contract; `union` appends patch elements the target lacks (exact-equality dedupe), so
42+
/// registrations can join arrays other owners also write — user-declared entries survive and
43+
/// re-materialization is idempotent.
44+
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
45+
pub enum ArrayMerge {
46+
#[default]
47+
Replace,
48+
Union,
49+
}
50+
3951
#[derive(Debug, Clone, PartialEq, Eq, Default)]
4052
pub struct RenderPlan {
4153
pub ops: Vec<RenderOp>,
@@ -68,6 +80,7 @@ impl RenderOp {
6880
| Self::JsonUpsert {
6981
destination,
7082
content,
83+
..
7184
} => {
7285
references_variable(destination, variable) || references_variable(content, variable)
7386
}
@@ -162,9 +175,22 @@ fn parse_render_node(node: &KdlNode, agent: &str) -> Result<RenderPlan> {
162175
serde_json::from_str::<serde_json::Value>(&content).with_context(|| {
163176
format!("agent '{agent}': json-upsert content is not valid JSON")
164177
})?;
178+
let arrays = match directive
179+
.entries()
180+
.iter()
181+
.find(|entry| entry.name().is_some_and(|name| name.value() == "arrays"))
182+
.and_then(|entry| entry.value().as_string())
183+
{
184+
None | Some("replace") => ArrayMerge::Replace,
185+
Some("union") => ArrayMerge::Union,
186+
Some(other) => anyhow::bail!(
187+
"agent '{agent}': json-upsert arrays=\"{other}\" (expected replace|union)"
188+
),
189+
};
165190
plan.ops.push(RenderOp::JsonUpsert {
166191
destination: destination.clone(),
167192
content,
193+
arrays,
168194
});
169195
}
170196
"ensure-line" => {
@@ -267,6 +293,7 @@ fn resolve_driver_render_executable(plan: &mut RenderPlan, agent: &str) -> Resul
267293
let RenderOp::JsonUpsert {
268294
destination,
269295
content,
296+
..
270297
} = operation
271298
else {
272299
continue;
@@ -312,6 +339,7 @@ fn effective_plan(root: &Path, spec: &AgentSpec, this_host: &str) -> Result<Rend
312339
plan.ops.push(RenderOp::JsonUpsert {
313340
destination: ".mcp.json".into(),
314341
content,
342+
arrays: ArrayMerge::Replace,
315343
});
316344
}
317345
Ok(plan)
@@ -466,18 +494,27 @@ fn ensure_line(path: &Path, line: &str) -> Result<bool> {
466494
Ok(true)
467495
}
468496

469-
fn deep_merge(target: &mut serde_json::Value, patch: serde_json::Value) {
497+
fn deep_merge(target: &mut serde_json::Value, patch: serde_json::Value, arrays: ArrayMerge) {
470498
match (target, patch) {
471499
(serde_json::Value::Object(target), serde_json::Value::Object(patch)) => {
472500
for (key, value) in patch {
473501
match target.get_mut(&key) {
474-
Some(existing) => deep_merge(existing, value),
502+
Some(existing) => deep_merge(existing, value, arrays),
475503
None => {
476504
target.insert(key, value);
477505
}
478506
}
479507
}
480508
}
509+
(serde_json::Value::Array(target), serde_json::Value::Array(patch))
510+
if arrays == ArrayMerge::Union =>
511+
{
512+
for element in patch {
513+
if !target.contains(&element) {
514+
target.push(element);
515+
}
516+
}
517+
}
481518
(target, patch) => *target = patch,
482519
}
483520
}
@@ -563,7 +600,7 @@ enum PreparedOp {
563600
#[derive(Debug, Clone, PartialEq)]
564601
enum RenderClaim {
565602
Replace(Vec<u8>),
566-
JsonUpsert(serde_json::Value),
603+
JsonUpsert(serde_json::Value, ArrayMerge),
567604
EnsureLine(String),
568605
}
569606

@@ -627,6 +664,7 @@ fn claims_for_agent(
627664
RenderOp::JsonUpsert {
628665
destination: raw_destination,
629666
content,
667+
arrays,
630668
} => {
631669
let patch = serde_json::from_str(&expand(&content, &env)).with_context(|| {
632670
format!(
@@ -636,7 +674,7 @@ fn claims_for_agent(
636674
})?;
637675
(
638676
destination(&workspace, &raw_destination, &env)?,
639-
RenderClaim::JsonUpsert(patch),
677+
RenderClaim::JsonUpsert(patch, arrays),
640678
)
641679
}
642680
RenderOp::EnsureLine {
@@ -784,6 +822,7 @@ pub fn materialize_agent(root: &Path, spec: &AgentSpec, this_host: &str) -> Resu
784822
RenderOp::JsonUpsert {
785823
destination: raw_destination,
786824
content,
825+
arrays,
787826
} => {
788827
let destination = destination(&workspace, &raw_destination, &env)?;
789828
let current = virtual_files
@@ -804,7 +843,7 @@ pub fn materialize_agent(root: &Path, spec: &AgentSpec, this_host: &str) -> Resu
804843
spec.identity
805844
)
806845
})?;
807-
deep_merge(&mut target, patch);
846+
deep_merge(&mut target, patch, arrays);
808847
let mut bytes = serde_json::to_vec_pretty(&target)?;
809848
bytes.push(b'\n');
810849
let note = format!("{}: upserted {}", spec.identity, raw_destination);
@@ -1056,6 +1095,7 @@ mod tests {
10561095
"nested": {"right": 2, "replace": "new"},
10571096
"array": [2]
10581097
}),
1098+
ArrayMerge::Replace,
10591099
);
10601100
assert_eq!(
10611101
target,
@@ -1066,4 +1106,27 @@ mod tests {
10661106
})
10671107
);
10681108
}
1109+
1110+
/// Union mode joins arrays idempotently: foreign entries survive and repeating the same
1111+
/// patch adds nothing — the contract the generated hook registration relies on.
1112+
#[test]
1113+
fn deep_merge_union_preserves_foreign_array_entries_and_is_idempotent() {
1114+
let mut target = serde_json::json!({
1115+
"hooks": {"Stop": [{"hooks": [{"type": "command", "command": "user-audit.sh"}]}]}
1116+
});
1117+
let ours = serde_json::json!({
1118+
"hooks": {"Stop": [{"hooks": [{"type": "command", "command": "$ST_HOOKS/claude-observe.sh Stop"}]}]}
1119+
});
1120+
deep_merge(&mut target, ours.clone(), ArrayMerge::Union);
1121+
deep_merge(&mut target, ours, ArrayMerge::Union);
1122+
assert_eq!(
1123+
target,
1124+
serde_json::json!({
1125+
"hooks": {"Stop": [
1126+
{"hooks": [{"type": "command", "command": "user-audit.sh"}]},
1127+
{"hooks": [{"type": "command", "command": "$ST_HOOKS/claude-observe.sh Stop"}]}
1128+
]}
1129+
})
1130+
);
1131+
}
10691132
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
render {
22
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}"
3-
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}"
3+
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
44
}
55
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."

0 commit comments

Comments
 (0)