feat(missions): keep the tool's arguments, not just its name
The container tier's first measured mission recorded `Bash × 6` and not one of them said what it ran. Every behavioural question about the phase — did it run the tests, did it commit, did it call an API a skill forbids — was unanswerable from a record that looked complete. `vm_tool_tap::parse` already read `tool_input` to pull the path out of it, then dropped the rest on the floor. It now keeps it, bounded: file bodies (`content`, `new_string`, `old_string`, `edits`) become a byte count, and any other over-long string is truncated with a marker saying so. Bounded rather than whitelisted, because a whitelist silently loses the one argument that matters the first time a tool grows a field. `file.touch` keeps the absolute path in `detail.abs` alongside the repo-relative `target`. Normalising is what the map needs and exactly what destroys "did this write land outside the checkout". `tool.call` also gains `detail.path`, which the World's SSE has been reading and getting a null from on every container-tier call. `mission_events::tool_evidence_for_mission` is the reader — the counterpart to `narrative_for_mission`, and the reason it exists: the narrative is what an agent SAID it did. Host-side only. No image rebuild: the arguments were always in the tap file, the first parse threw them away. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_018i9Ten1LU4jUr5d7TAWda9
This commit is contained in:
co-authored by
Claude Opus 5
parent
0b4d91889a
commit
8cb38d1320
@@ -183,6 +183,68 @@ pub async fn narrative_for_mission(
|
|||||||
.collect())
|
.collect())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// One action an agent took, as a reader gets it back.
|
||||||
|
#[derive(Debug, Clone, PartialEq)]
|
||||||
|
pub struct ToolEvidence {
|
||||||
|
/// The tool's name, e.g. `Bash`, `Write`.
|
||||||
|
pub tool: String,
|
||||||
|
/// The absolute path inside the sandbox, when the tool named one.
|
||||||
|
///
|
||||||
|
/// Absolute, unlike the sibling `file.touch` row's `target`. See the note
|
||||||
|
/// in `phase_runner::record_vm_tools`: normalising is what destroys the
|
||||||
|
/// only question a path can settle.
|
||||||
|
pub path: Option<String>,
|
||||||
|
/// The tool's arguments, bounded by `vm_tool_tap::bounded_input`.
|
||||||
|
pub input: Value,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl ToolEvidence {
|
||||||
|
/// The shell command, for the tools that run one.
|
||||||
|
pub fn command(&self) -> Option<&str> {
|
||||||
|
self.input.get("command").and_then(Value::as_str)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Every tool call recorded for a mission, in order.
|
||||||
|
///
|
||||||
|
/// The counterpart to [`narrative_for_mission`], and the reason it exists: the
|
||||||
|
/// narrative is what an agent *said* it did. These rows are what it did. A
|
||||||
|
/// measurement built on the narrative alone scores prose, and prose is written
|
||||||
|
/// by the thing being measured.
|
||||||
|
///
|
||||||
|
/// **Bounded by [`PER_PHASE_CAP`].** A phase that ran more tools than the cap
|
||||||
|
/// returns the first `PER_PHASE_CAP` and no marker saying so, so a check that
|
||||||
|
/// concludes "this never happened" from an empty result is only sound for
|
||||||
|
/// phases under the cap. Every check in `skill_use` is one-sided in the safe
|
||||||
|
/// direction for that reason: it reports a violation it can see, never
|
||||||
|
/// compliance it inferred from silence.
|
||||||
|
pub async fn tool_evidence_for_mission(
|
||||||
|
pool: &PgPool,
|
||||||
|
mission_id: Uuid,
|
||||||
|
) -> Result<Vec<ToolEvidence>, sqlx::Error> {
|
||||||
|
let rows: Vec<(Option<String>, Value)> = sqlx::query_as(
|
||||||
|
"SELECT target, detail
|
||||||
|
FROM mission_events
|
||||||
|
WHERE mission_id = $1 AND kind = $2
|
||||||
|
ORDER BY id",
|
||||||
|
)
|
||||||
|
.bind(mission_id)
|
||||||
|
.bind(TOOL_CALL)
|
||||||
|
.fetch_all(pool)
|
||||||
|
.await?;
|
||||||
|
Ok(rows
|
||||||
|
.into_iter()
|
||||||
|
.map(|(target, detail)| ToolEvidence {
|
||||||
|
tool: target.unwrap_or_default(),
|
||||||
|
path: detail
|
||||||
|
.get("path")
|
||||||
|
.and_then(Value::as_str)
|
||||||
|
.map(str::to_string),
|
||||||
|
input: detail.get("input").cloned().unwrap_or(Value::Null),
|
||||||
|
})
|
||||||
|
.collect())
|
||||||
|
}
|
||||||
|
|
||||||
pub async fn record_all(pool: &PgPool, events: Vec<MissionEvent>) {
|
pub async fn record_all(pool: &PgPool, events: Vec<MissionEvent>) {
|
||||||
for e in events {
|
for e in events {
|
||||||
record(pool, e).await;
|
record(pool, e).await;
|
||||||
|
|||||||
@@ -2077,7 +2077,11 @@ pub(crate) async fn record_vm_tools(
|
|||||||
agent_id: None,
|
agent_id: None,
|
||||||
kind: crate::mission_events::TOOL_CALL.to_string(),
|
kind: crate::mission_events::TOOL_CALL.to_string(),
|
||||||
target: Some(t.tool.clone()),
|
target: Some(t.tool.clone()),
|
||||||
detail: serde_json::Value::Null,
|
// `path` because the World's SSE reads `detail.path` for this kind
|
||||||
|
// and was handed a null on every container-tier call; `input`
|
||||||
|
// because the tool name alone cannot answer a single behavioural
|
||||||
|
// question about the phase.
|
||||||
|
detail: serde_json::json!({ "path": t.path, "input": t.input }),
|
||||||
});
|
});
|
||||||
if let Some(path) = &t.path {
|
if let Some(path) = &t.path {
|
||||||
events.push(crate::mission_events::MissionEvent {
|
events.push(crate::mission_events::MissionEvent {
|
||||||
@@ -2090,7 +2094,13 @@ pub(crate) async fn record_vm_tools(
|
|||||||
path,
|
path,
|
||||||
&["/mission/repo", "/workspace"],
|
&["/mission/repo", "/workspace"],
|
||||||
)),
|
)),
|
||||||
detail: serde_json::json!({ "tool": t.tool }),
|
// The ABSOLUTE path as well as the repo-relative one. `target`
|
||||||
|
// is normalised for the map, where a `mission` → `repo` pair of
|
||||||
|
// directory orbs means nothing to a reader — but normalising is
|
||||||
|
// exactly what destroys the question "did this write land
|
||||||
|
// outside the checkout", which is the one boundary a skill can
|
||||||
|
// be scored on.
|
||||||
|
detail: serde_json::json!({ "tool": t.tool, "abs": path }),
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1114,9 +1114,13 @@ pub async fn world_live(
|
|||||||
// the durable source: `reasoning` rows carry the agent's own step
|
// the durable source: `reasoning` rows carry the agent's own step
|
||||||
// output, `tool.call` rows its actions.
|
// output, `tool.call` rows its actions.
|
||||||
//
|
//
|
||||||
// NOTE: on the container tier `tool.call` legitimately stays empty —
|
// This used to say the container tier "legitimately stays empty —
|
||||||
// those agents are tool-free behind the §15 door. Reasoning flows on
|
// those agents are tool-free". That was wrong, and it was wrong in
|
||||||
// every tier; tool lines appear where agents actually hold tools.
|
// the most expensive way: it explained the silence, so nobody
|
||||||
|
// looked. Those agents call `Bash` and `Write` constantly; the
|
||||||
|
// calls happen inside claude's own subprocess and so never reached
|
||||||
|
// ZeroClaw's executor. `container_tool_hooks` records them now, and
|
||||||
|
// `tool.call` is populated on both tiers.
|
||||||
if agent_ev_cursor < 0 {
|
if agent_ev_cursor < 0 {
|
||||||
agent_ev_cursor = sqlx::query_scalar(
|
agent_ev_cursor = sqlx::query_scalar(
|
||||||
"SELECT coalesce(max(id), 0) FROM mission_events",
|
"SELECT coalesce(max(id), 0) FROM mission_events",
|
||||||
|
|||||||
@@ -71,6 +71,70 @@ pub struct Observed {
|
|||||||
pub tool: String,
|
pub tool: String,
|
||||||
/// The path the tool's **input** named, if any. From JSON, never prose.
|
/// The path the tool's **input** named, if any. From JSON, never prose.
|
||||||
pub path: Option<String>,
|
pub path: Option<String>,
|
||||||
|
/// The tool's arguments, bounded by [`bounded_input`].
|
||||||
|
///
|
||||||
|
/// Kept because the tool NAME alone answers almost nothing. A phase that
|
||||||
|
/// recorded `Bash × 6` is indistinguishable from one that ran the test
|
||||||
|
/// suite six times, one that pushed to a branch it was told not to, and
|
||||||
|
/// one that queried an API a skill forbids. The argument is where the
|
||||||
|
/// behaviour is, and until now this parser read it, took the path out of
|
||||||
|
/// it, and dropped the rest on the floor.
|
||||||
|
pub input: Value,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// How much of one argument string is worth keeping.
|
||||||
|
///
|
||||||
|
/// A shell command longer than this is a heredoc or a generated payload; its
|
||||||
|
/// first half still carries the verb, which is what any check reads.
|
||||||
|
const MAX_ARG_LEN: usize = 512;
|
||||||
|
|
||||||
|
/// Argument keys whose value is a file BODY rather than a description of an
|
||||||
|
/// action.
|
||||||
|
///
|
||||||
|
/// Dropped to a byte count rather than truncated. These carry whole source
|
||||||
|
/// files — `mission_events` is already the largest write path on a coding
|
||||||
|
/// phase, and storing every `Write` twice (once in the event, once in the
|
||||||
|
/// delivered diff) buys nothing: no check reads the body, and the diff is the
|
||||||
|
/// authority on what was written anyway.
|
||||||
|
const BODY_KEYS: [&str; 4] = ["content", "new_string", "old_string", "edits"];
|
||||||
|
|
||||||
|
/// Shrink a tool's arguments to something safe to store on every call.
|
||||||
|
///
|
||||||
|
/// Bounded rather than whitelisted on purpose. A whitelist of "interesting"
|
||||||
|
/// keys silently drops the one argument that matters the first time a tool
|
||||||
|
/// grows a new field, and the loss is invisible — the event still looks
|
||||||
|
/// complete. Bounding keeps every key and says, in the record itself, where it
|
||||||
|
/// stopped.
|
||||||
|
pub fn bounded_input(input: &Value) -> Value {
|
||||||
|
let Some(obj) = input.as_object() else {
|
||||||
|
return Value::Null;
|
||||||
|
};
|
||||||
|
let mut out = serde_json::Map::new();
|
||||||
|
for (k, v) in obj {
|
||||||
|
if BODY_KEYS.contains(&k.as_str()) {
|
||||||
|
let bytes = match v {
|
||||||
|
Value::String(s) => s.len(),
|
||||||
|
other => other.to_string().len(),
|
||||||
|
};
|
||||||
|
out.insert(k.clone(), json!({ "omitted_bytes": bytes }));
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
match v {
|
||||||
|
Value::String(s) if s.len() > MAX_ARG_LEN => {
|
||||||
|
let cut = s
|
||||||
|
.char_indices()
|
||||||
|
.map(|(i, _)| i)
|
||||||
|
.take_while(|i| *i <= MAX_ARG_LEN)
|
||||||
|
.last()
|
||||||
|
.unwrap_or(0);
|
||||||
|
out.insert(k.clone(), Value::String(format!("{}…[truncated]", &s[..cut])));
|
||||||
|
}
|
||||||
|
other => {
|
||||||
|
out.insert(k.clone(), other.clone());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Value::Object(out)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The hook script. Copies stdin verbatim to the tap file and gets out of the
|
/// The hook script. Copies stdin verbatim to the tap file and gets out of the
|
||||||
@@ -180,6 +244,7 @@ pub fn parse(raw: &str) -> Vec<Observed> {
|
|||||||
.unwrap_or(Value::Null);
|
.unwrap_or(Value::Null);
|
||||||
Some(Observed {
|
Some(Observed {
|
||||||
path: crate::mission_events::tool_path(&input),
|
path: crate::mission_events::tool_path(&input),
|
||||||
|
input: bounded_input(&input),
|
||||||
tool,
|
tool,
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
@@ -268,12 +333,74 @@ mod tests {
|
|||||||
assert_eq!(
|
assert_eq!(
|
||||||
parse(raw),
|
parse(raw),
|
||||||
vec![
|
vec![
|
||||||
Observed { tool: "Edit".into(), path: Some("/mission/repo/src/a.rs".into()) },
|
Observed {
|
||||||
Observed { tool: "Bash".into(), path: None },
|
tool: "Edit".into(),
|
||||||
|
path: Some("/mission/repo/src/a.rs".into()),
|
||||||
|
input: json!({"file_path": "/mission/repo/src/a.rs"}),
|
||||||
|
},
|
||||||
|
Observed {
|
||||||
|
tool: "Bash".into(),
|
||||||
|
path: None,
|
||||||
|
input: json!({"command": "ls"}),
|
||||||
|
},
|
||||||
]
|
]
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The command survives the parse.
|
||||||
|
///
|
||||||
|
/// The regression this guards is the one that made the first container-tier
|
||||||
|
/// measurement unusable: six `Bash` calls were recorded and not one of them
|
||||||
|
/// said what it ran, so every behavioural question — did it run the tests,
|
||||||
|
/// did it commit, did it call the API a skill forbids — was unanswerable
|
||||||
|
/// from a record that looked complete.
|
||||||
|
#[test]
|
||||||
|
fn the_argument_is_what_carries_the_behaviour() {
|
||||||
|
let raw = concat!(
|
||||||
|
r#"{"tool_name":"Bash","tool_input":{"command":"cargo nextest run -p cm-api"}}"#,
|
||||||
|
"\n",
|
||||||
|
);
|
||||||
|
let got = parse(raw);
|
||||||
|
assert_eq!(got[0].input["command"], json!("cargo nextest run -p cm-api"));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A file body is counted, not stored; everything else survives bounded.
|
||||||
|
#[test]
|
||||||
|
fn bodies_are_dropped_and_long_arguments_are_marked() {
|
||||||
|
let long = "x".repeat(MAX_ARG_LEN + 50);
|
||||||
|
let got = bounded_input(&json!({
|
||||||
|
"file_path": "/mission/repo/src/a.rs",
|
||||||
|
"content": "fn main() {}",
|
||||||
|
"command": long,
|
||||||
|
}));
|
||||||
|
assert_eq!(got["file_path"], json!("/mission/repo/src/a.rs"));
|
||||||
|
assert_eq!(
|
||||||
|
got["content"],
|
||||||
|
json!({"omitted_bytes": 12}),
|
||||||
|
"a file body is stored in the delivered diff already; the event only \
|
||||||
|
needs to say how big it was"
|
||||||
|
);
|
||||||
|
let cmd = got["command"].as_str().expect("command kept");
|
||||||
|
assert!(cmd.ends_with("…[truncated]"), "{cmd}");
|
||||||
|
assert!(
|
||||||
|
cmd.len() < MAX_ARG_LEN + 40,
|
||||||
|
"a bounded argument must actually be bounded: {}",
|
||||||
|
cmd.len()
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Truncation must not split a multi-byte character.
|
||||||
|
///
|
||||||
|
/// `&s[..cut]` on a byte index inside a UTF-8 sequence panics, and the
|
||||||
|
/// panic would land in the drain — losing a whole phase's tap to a command
|
||||||
|
/// that happened to contain an emoji or an em dash.
|
||||||
|
#[test]
|
||||||
|
fn truncation_respects_character_boundaries() {
|
||||||
|
let long = "é".repeat(MAX_ARG_LEN);
|
||||||
|
let got = bounded_input(&json!({ "command": long }));
|
||||||
|
assert!(got["command"].as_str().unwrap().ends_with("…[truncated]"));
|
||||||
|
}
|
||||||
|
|
||||||
/// Exactly one place in the tree writes the guest settings document.
|
/// Exactly one place in the tree writes the guest settings document.
|
||||||
///
|
///
|
||||||
/// The unit test above proves `guest_settings` composes correctly; it says
|
/// The unit test above proves `guest_settings` composes correctly; it says
|
||||||
|
|||||||
Reference in New Issue
Block a user