feat(skills): a files arm — progressive disclosure through Read, not a deferred tool
The `index` arm retrieves through `ReadMcpResourceTool`, which is DEFERRED:
absent from the agent's default tool list until `ToolSearch` loads it. Across
three matched production runs (same recipe, same task, same three offered
uris) it retrieved 1 skill in 9 chances:
01a07812 delegation forced no instruction 0/3
01a0842e no delegation no instruction 1/3
01a09877 no delegation told to load it 0/3
The third run is the decisive one. The preamble said in plain words to run
ToolSearch first; all three prompts carried it; zero ToolSearch calls, and the
three reasoning narratives never mention skills at all. The section was not
declined, it was never engaged with. Instruction is not the lever.
`Read` is a core tool. Never deferred, and every one of those agents used it.
So this arm keeps progressive disclosure exactly as `index` has it — a name, a
`when_to_use`, and a pointer the agent has to follow — and changes only what
the pointer is: a path under /mission/skills instead of an MCP uri. The bodies
are written into the container at launch (every visible skill, one tar upload;
bindings resolve per agent at turn time so a per-mission subset is not knowable
here) and a `Read` of that path is a tapped tool call, so Trigger is exactly as
observable as before.
A third arm and not a replacement, selected per mission like the others, so
the comparison runs against one binary. `resolve` falls back to `inline` when
the files were not written, for the reason `index` does: a pointer to nothing
reads as an agent ignoring its skills.
The writer and reader of a path are one pair of functions
(`skill_file_path` / `skill_from_file_path`), matched by the scorer through
the same seam `parse_uri` uses, and the end-to-end test fails when the matcher
is broken. `Mode::is_retrieval` exists so the next arm cannot silently inherit
`inline`'s "not observable" for what is a miss.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01WZb5A2kfVfjpdwSochkuHz
This commit is contained in:
co-authored by
Claude Opus 5
parent
8d6310f126
commit
00160739de
@@ -39,7 +39,7 @@ pub const SETTINGS_PATH: &str = "/root/toolhooks/settings.json";
|
||||
/// Where the `PostToolUse` tap appends, inside the container.
|
||||
pub const TAP_DIR: &str = "/root/toolhooks/tap";
|
||||
|
||||
const INSTALL_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
pub const INSTALL_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
|
||||
/// Install the pre-execution gate and the tool tap into a mission container.
|
||||
///
|
||||
|
||||
@@ -247,6 +247,51 @@ pub async fn put_file(
|
||||
.map_err(|e| format!("upload {path} to {container}: {e}"))
|
||||
}
|
||||
|
||||
/// Build a flat tar of several files. [`single_file_archive`] for many.
|
||||
fn files_archive(files: &[(String, Vec<u8>)]) -> Result<Vec<u8>, String> {
|
||||
let mut builder = tar::Builder::new(Vec::new());
|
||||
for (name, contents) in files {
|
||||
let mut header = tar::Header::new_gnu();
|
||||
header
|
||||
.set_path(name)
|
||||
.map_err(|e| format!("tar path {name}: {e}"))?;
|
||||
header.set_size(contents.len() as u64);
|
||||
// World-readable, unlike `single_file_archive`'s 0600: that one carries
|
||||
// a credential, this one carries procedures the agent is meant to read.
|
||||
header.set_mode(0o644);
|
||||
header.set_entry_type(tar::EntryType::Regular);
|
||||
header.set_cksum();
|
||||
builder
|
||||
.append(&header, contents.as_slice())
|
||||
.map_err(|e| format!("tar {name}: {e}"))?;
|
||||
}
|
||||
builder
|
||||
.into_inner()
|
||||
.map_err(|e| format!("finish archive of {} files: {e}", files.len()))
|
||||
}
|
||||
|
||||
/// Write several files into one directory of a container, in one upload.
|
||||
///
|
||||
/// `dir` must already exist — `upload_to_container` will not create it, the
|
||||
/// same constraint [`sync_in`] works around. Size-independent for the reason
|
||||
/// [`put_file`] gives; fifty skill bodies would be well past `ARG_MAX` as a
|
||||
/// printf.
|
||||
pub async fn put_files(
|
||||
docker: &Docker,
|
||||
container: &str,
|
||||
dir: &str,
|
||||
files: &[(String, Vec<u8>)],
|
||||
) -> Result<(), String> {
|
||||
let archive = files_archive(files)?;
|
||||
let opts = bollard::query_parameters::UploadToContainerOptionsBuilder::default()
|
||||
.path(dir)
|
||||
.build();
|
||||
docker
|
||||
.upload_to_container(container, Some(opts), bollard::body_full(archive.into()))
|
||||
.await
|
||||
.map_err(|e| format!("upload {} files to {container}:{dir}: {e}", files.len()))
|
||||
}
|
||||
|
||||
/// Copy a directory back out of a container onto the host.
|
||||
pub async fn copy_out(
|
||||
docker: &Docker,
|
||||
|
||||
@@ -349,17 +349,20 @@ pub async fn on_launch(
|
||||
// Only when this mission got its OWN container — the shared runtime is
|
||||
// not ours to reconfigure, and `mission_gateway` being Some is exactly
|
||||
// the signal that `ensure_container` ran.
|
||||
let door = if mission_gateway.is_some() {
|
||||
install_skills_door(
|
||||
pool,
|
||||
user_id,
|
||||
mission_id,
|
||||
&crate::mission_runtime::container_name(mission_id),
|
||||
p,
|
||||
)
|
||||
.await
|
||||
} else {
|
||||
false
|
||||
// What a retrieval arm retrieves FROM is installed here, per arm: the
|
||||
// MCP door for `index`, the skill files for `files`. `inline` installs
|
||||
// nothing and `installed` is irrelevant to it.
|
||||
let requested = crate::skill_delivery::requested_for(&mission.config);
|
||||
let container = crate::mission_runtime::container_name(mission_id);
|
||||
let installed = match requested {
|
||||
_ if mission_gateway.is_none() => false,
|
||||
crate::skill_delivery::Mode::Index => {
|
||||
install_skills_door(pool, user_id, mission_id, &container, p).await
|
||||
}
|
||||
crate::skill_delivery::Mode::Files => {
|
||||
install_skill_files(pool, workspace_id, mission_id, &container).await
|
||||
}
|
||||
crate::skill_delivery::Mode::Inline => false,
|
||||
};
|
||||
// Decided here and recorded, not re-derived per turn: this is the only
|
||||
// point that knows whether the door actually installed, and an arm that
|
||||
@@ -367,10 +370,7 @@ pub async fn on_launch(
|
||||
record_skill_delivery(
|
||||
pool,
|
||||
mission_id,
|
||||
crate::skill_delivery::resolve(
|
||||
crate::skill_delivery::requested_for(&mission.config),
|
||||
door,
|
||||
),
|
||||
crate::skill_delivery::resolve(requested, installed),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
@@ -991,6 +991,80 @@ async fn install_skills_door(
|
||||
true
|
||||
}
|
||||
|
||||
/// Write every skill the workspace can see into the mission container as a
|
||||
/// file, for the `files` arm.
|
||||
///
|
||||
/// Every visible skill and not only the bound ones, because bindings are
|
||||
/// resolved per AGENT at turn time (`effective_for_agent`) and this runs once
|
||||
/// per mission before any turn — the same reason the MCP door serves the whole
|
||||
/// catalogue rather than a per-mission subset. A few KB each; the whole
|
||||
/// catalogue is smaller than one phase's evidence.
|
||||
///
|
||||
/// Returns whether the files are in place. `false` means the mission falls
|
||||
/// back to `inline` (see `skill_delivery::resolve`) — an entry that points at
|
||||
/// a file which is not there reads exactly like an agent ignoring its skills,
|
||||
/// which is the failure this arm exists to stop misdiagnosing.
|
||||
async fn install_skill_files(
|
||||
pool: &PgPool,
|
||||
workspace_id: WorkspaceId,
|
||||
mission_id: Uuid,
|
||||
container: &str,
|
||||
) -> bool {
|
||||
let skills = match cm_db::repo::skills_catalog::list_visible(pool, workspace_id.as_uuid()).await
|
||||
{
|
||||
Ok(v) => v,
|
||||
Err(e) => {
|
||||
eprintln!(
|
||||
"mission_orchestrator: could not list skills for the files arm ({e}) — \
|
||||
mission {mission_id} delivers skills inline"
|
||||
);
|
||||
return false;
|
||||
}
|
||||
};
|
||||
let docker = match crate::container_exec::connect() {
|
||||
Ok(d) => d,
|
||||
Err(e) => {
|
||||
eprintln!("mission_orchestrator: cannot reach docker for the skill files: {e}");
|
||||
return false;
|
||||
}
|
||||
};
|
||||
let dir = crate::skill_delivery::SKILLS_DIR;
|
||||
// `upload_to_container` will not create the directory.
|
||||
let argv = vec!["sh".to_string(), "-lc".to_string(), format!("mkdir -p {dir}")];
|
||||
match crate::container_exec::exec_as_root(
|
||||
&docker,
|
||||
container,
|
||||
None,
|
||||
&argv,
|
||||
crate::container_tool_hooks::INSTALL_TIMEOUT,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(out) if out.exit_code == Some(0) => {}
|
||||
other => {
|
||||
eprintln!(
|
||||
"mission_orchestrator: could not create {dir} in {container} ({other:?}) — \
|
||||
mission {mission_id} delivers skills inline"
|
||||
);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
let files: Vec<(String, Vec<u8>)> = skills
|
||||
.iter()
|
||||
.map(|sk| (format!("{}.md", sk.name), sk.body.clone().into_bytes()))
|
||||
.collect();
|
||||
let n = files.len();
|
||||
if let Err(e) = crate::mission_fs::put_files(&docker, container, dir, &files).await {
|
||||
eprintln!(
|
||||
"mission_orchestrator: could not write the skill files ({e}) — mission \
|
||||
{mission_id} delivers skills inline"
|
||||
);
|
||||
return false;
|
||||
}
|
||||
eprintln!("mission_orchestrator: {n} skill file(s) installed for mission {mission_id} under {dir}");
|
||||
true
|
||||
}
|
||||
|
||||
/// Record which arm this mission runs, so every turn composes the same one and
|
||||
/// the score can be attributed to it afterwards.
|
||||
///
|
||||
|
||||
@@ -7,7 +7,11 @@
|
||||
//! prompt. What production has always done.
|
||||
//! - [`Mode::Index`] — the prompt carries each skill's name, description and
|
||||
//! `when_to_use` plus the URI that returns its body, and the agent fetches
|
||||
//! the ones it judges relevant.
|
||||
//! the ones it judges relevant through the MCP door.
|
||||
//! - [`Mode::Files`] — the same entry with a file path where the URI was; the
|
||||
//! bodies are written into the container and the agent `Read`s them. Added
|
||||
//! after `Index` measured 1 retrieval in 9 across three matched runs — see
|
||||
//! [`FILES_PREAMBLE`] for why.
|
||||
//!
|
||||
//! # Why this is an A/B and not a switch
|
||||
//!
|
||||
@@ -90,10 +94,62 @@ returns it. Where an entry applies to what you are about to do, read it FIRST an
|
||||
follow it. ReadMcpResourceTool may not be loaded in this session: if you do not already \
|
||||
have it, run ToolSearch with the query select:ReadMcpResourceTool before your first read.";
|
||||
|
||||
/// Where the `files` arm puts skill bodies inside the mission container.
|
||||
///
|
||||
/// Under `/mission` because that is the one directory every container-tier
|
||||
/// mission has ([`crate::mission_fs::CONTAINER_MISSION_DIR`]), and beside
|
||||
/// `repo/` rather than inside it so a skill never shows up in a diff or a
|
||||
/// delivery.
|
||||
pub const SKILLS_DIR: &str = "/mission/skills";
|
||||
|
||||
/// The file a skill's body is written to under the `files` arm, and the path
|
||||
/// the index entry tells the agent to `Read`. One function for both, so the
|
||||
/// writer and the reader cannot spell it differently.
|
||||
pub fn skill_file_path(name: &str) -> String {
|
||||
format!("{SKILLS_DIR}/{name}.md")
|
||||
}
|
||||
|
||||
/// The skill a `Read` of this path is a retrieval of, if it is one.
|
||||
///
|
||||
/// The scorer's half of [`skill_file_path`]. Anything outside [`SKILLS_DIR`]
|
||||
/// is an ordinary file read and returns `None`.
|
||||
pub fn skill_from_file_path(path: &str) -> Option<String> {
|
||||
let rest = path.strip_prefix(SKILLS_DIR)?.strip_prefix('/')?;
|
||||
let name = rest.strip_suffix(".md")?;
|
||||
if name.is_empty() || name.contains('/') {
|
||||
return None;
|
||||
}
|
||||
Some(name.to_string())
|
||||
}
|
||||
|
||||
/// The `# Your skills` preamble under [`Mode::Files`].
|
||||
///
|
||||
/// # Why a third arm
|
||||
///
|
||||
/// `Index` retrieves through `ReadMcpResourceTool`, which is a DEFERRED tool:
|
||||
/// absent from the agent's default list until `ToolSearch` loads it. Measured
|
||||
/// across three matched production runs (`01a07812`, `01a0842e`, `01a09877` —
|
||||
/// same recipe, same task, same three offered uris), that path retrieved
|
||||
/// **1 skill in 9 chances**, and telling the agent in the preamble to load
|
||||
/// the tool first changed nothing: the third run's three reasoning narratives
|
||||
/// never mention skills at all. The section was not declined; it was never
|
||||
/// engaged with.
|
||||
///
|
||||
/// `Read` is a core tool. It is never deferred, and every one of those agents
|
||||
/// used it. So this arm keeps progressive disclosure exactly as `Index` has it
|
||||
/// — name, `when_to_use`, and a pointer the agent has to follow — and changes
|
||||
/// only what the pointer is: a file path instead of an MCP uri. A `Read` of
|
||||
/// that path is a tapped tool call, so Trigger stays as observable as before.
|
||||
pub const FILES_PREAMBLE: &str = "These procedures are AVAILABLE to you; their bodies are \
|
||||
not included below. Each entry names one, says when it applies, and gives the path of the \
|
||||
file that holds it. Where an entry applies to what you are about to do, Read that file FIRST \
|
||||
and then follow it.";
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum Mode {
|
||||
Inline,
|
||||
Index,
|
||||
Files,
|
||||
}
|
||||
|
||||
impl Mode {
|
||||
@@ -101,8 +157,19 @@ impl Mode {
|
||||
match self {
|
||||
Mode::Inline => "inline",
|
||||
Mode::Index => "index",
|
||||
Mode::Files => "files",
|
||||
}
|
||||
}
|
||||
|
||||
/// Does this arm hand the agent a pointer rather than a body?
|
||||
///
|
||||
/// The two retrieval arms share every rule that follows from that — the
|
||||
/// scorer's Trigger axis, the `always_inject` override, the fallback when
|
||||
/// nothing was installed — and branching on this rather than on `Index`
|
||||
/// is what keeps a third arm from silently inheriting `Inline`'s answers.
|
||||
pub fn is_retrieval(self) -> bool {
|
||||
matches!(self, Mode::Index | Mode::Files)
|
||||
}
|
||||
}
|
||||
|
||||
/// Parse a recorded or configured arm. Unrecognised input is `None`, and every
|
||||
@@ -112,6 +179,7 @@ pub fn parse(s: &str) -> Option<Mode> {
|
||||
match s.trim().to_ascii_lowercase().as_str() {
|
||||
"inline" => Some(Mode::Inline),
|
||||
"index" | "progressive" => Some(Mode::Index),
|
||||
"files" | "file" => Some(Mode::Files),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
@@ -128,7 +196,7 @@ pub fn requested() -> Mode {
|
||||
Some(m) => m,
|
||||
None => {
|
||||
eprintln!(
|
||||
"skill_delivery: {ENV_VAR}={raw:?} is not `inline` or `index` — \
|
||||
"skill_delivery: {ENV_VAR}={raw:?} is not `inline`, `index` or `files` — \
|
||||
delivering skills inline"
|
||||
);
|
||||
Mode::Inline
|
||||
@@ -151,27 +219,29 @@ pub fn requested_for(config: &serde_json::Value) -> Mode {
|
||||
Some(m) => m,
|
||||
None => {
|
||||
eprintln!(
|
||||
"skill_delivery: config.skill_delivery={raw:?} is not `inline` or \
|
||||
`index` — falling back to the deployment default"
|
||||
"skill_delivery: config.skill_delivery={raw:?} is not `inline`, `index` \
|
||||
or `files` — falling back to the deployment default"
|
||||
);
|
||||
requested()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The arm a mission will actually run, given whether its skills door installed.
|
||||
pub fn resolve(requested: Mode, door_installed: bool) -> Mode {
|
||||
match (requested, door_installed) {
|
||||
(Mode::Index, true) => Mode::Index,
|
||||
(Mode::Index, false) => {
|
||||
/// The arm a mission will actually run, given whether what it retrieves from
|
||||
/// was installed — the MCP door for `index`, the skill files for `files`.
|
||||
pub fn resolve(requested: Mode, installed: bool) -> Mode {
|
||||
match (requested, installed) {
|
||||
(m, true) if m.is_retrieval() => m,
|
||||
(m, false) if m.is_retrieval() => {
|
||||
eprintln!(
|
||||
"skill_delivery: {ENV_VAR} asked for `index` but this mission has no \
|
||||
skills door — falling back to `inline`, because an index the agent \
|
||||
cannot fetch from is worse than no index"
|
||||
"skill_delivery: `{}` was asked for but this mission has nothing to \
|
||||
retrieve from — falling back to `inline`, because an index the agent \
|
||||
cannot fetch from is worse than no index",
|
||||
m.as_str()
|
||||
);
|
||||
Mode::Inline
|
||||
}
|
||||
(Mode::Inline, _) => Mode::Inline,
|
||||
_ => Mode::Inline,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -180,6 +250,7 @@ pub fn preamble(mode: Mode) -> &'static str {
|
||||
match mode {
|
||||
Mode::Inline => INLINE_PREAMBLE,
|
||||
Mode::Index => INDEX_PREAMBLE,
|
||||
Mode::Files => FILES_PREAMBLE,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -196,14 +267,16 @@ pub fn mode_in_prompt(prompt: &str) -> Mode {
|
||||
// Both spellings, because this reads prompts composed by older builds as
|
||||
// well as the current one. A stored measurement that changes arm when the
|
||||
// writer is edited is not a measurement.
|
||||
if prompt
|
||||
.lines()
|
||||
.any(|l| l.trim() == INDEX_PREAMBLE || l.trim() == INDEX_PREAMBLE_V1)
|
||||
{
|
||||
Mode::Index
|
||||
} else {
|
||||
Mode::Inline
|
||||
for l in prompt.lines() {
|
||||
let l = l.trim();
|
||||
if l == INDEX_PREAMBLE || l == INDEX_PREAMBLE_V1 {
|
||||
return Mode::Index;
|
||||
}
|
||||
if l == FILES_PREAMBLE {
|
||||
return Mode::Files;
|
||||
}
|
||||
}
|
||||
Mode::Inline
|
||||
}
|
||||
|
||||
/// One index entry's text — everything under the `--- SKILL: <name> ---`
|
||||
@@ -233,6 +306,24 @@ pub fn index_entry(description: &str, when_to_use: Option<&str>, uri: &str) -> S
|
||||
/// stops detecting.
|
||||
pub const READ_IT: &str = "Read it: ReadMcpResourceTool(";
|
||||
|
||||
/// [`READ_IT`]'s counterpart for the `files` arm. Same rule: one constant,
|
||||
/// written by [`file_entry`] and read by [`skill_was_indexed`].
|
||||
pub const READ_FILE_IT: &str = "Read it: Read(file_path=\"";
|
||||
|
||||
/// One `files`-arm entry — [`index_entry`] with a path where the uri was.
|
||||
pub fn file_entry(description: &str, when_to_use: Option<&str>, path: &str) -> String {
|
||||
let when = when_to_use
|
||||
.map(str::trim)
|
||||
.filter(|w| !w.is_empty())
|
||||
.unwrap_or("not stated — judge from the description");
|
||||
format!(
|
||||
"{}\nWhen to use: {}\n{READ_FILE_IT}{}\")",
|
||||
description.trim(),
|
||||
when,
|
||||
path,
|
||||
)
|
||||
}
|
||||
|
||||
/// How was THIS skill delivered, regardless of the arm the prompt announces?
|
||||
///
|
||||
/// `Some(true)` — an index entry: named, described, and left to be fetched.
|
||||
@@ -258,7 +349,7 @@ pub fn skill_was_indexed(prompt: &str, skill: &str) -> Option<bool> {
|
||||
if l.trim().starts_with(marker) {
|
||||
break;
|
||||
}
|
||||
if l.contains(READ_IT) {
|
||||
if l.contains(READ_IT) || l.contains(READ_FILE_IT) {
|
||||
return Some(true);
|
||||
}
|
||||
}
|
||||
@@ -298,6 +389,52 @@ mod tests {
|
||||
assert_eq!(resolve(Mode::Inline, true), Mode::Inline);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_files_arm_parses_resolves_and_reads_back() {
|
||||
assert_eq!(parse("files"), Some(Mode::Files));
|
||||
assert_eq!(resolve(Mode::Files, true), Mode::Files);
|
||||
assert_eq!(
|
||||
resolve(Mode::Files, false),
|
||||
Mode::Inline,
|
||||
"files that were never written must not be advertised"
|
||||
);
|
||||
let prompt = format!("Task: x\n\n# Your skills\n\n{FILES_PREAMBLE}\n\nentry");
|
||||
assert_eq!(mode_in_prompt(&prompt), Mode::Files);
|
||||
assert_eq!(preamble(Mode::Files), FILES_PREAMBLE);
|
||||
}
|
||||
|
||||
/// The writer and the reader of a skill path are one pair of functions.
|
||||
#[test]
|
||||
fn a_skill_path_round_trips_and_nothing_else_parses_as_one() {
|
||||
let p = skill_file_path("web-search-triage");
|
||||
assert_eq!(p, "/mission/skills/web-search-triage.md");
|
||||
assert_eq!(skill_from_file_path(&p).as_deref(), Some("web-search-triage"));
|
||||
for not_a_skill in [
|
||||
"/mission/repo/skills/x.md",
|
||||
"/mission/skills/x.txt",
|
||||
"/mission/skills/.md",
|
||||
"/mission/skills/a/b.md",
|
||||
"/mission/skills",
|
||||
"mission/skills/x.md",
|
||||
] {
|
||||
assert_eq!(skill_from_file_path(not_a_skill), None, "{not_a_skill}");
|
||||
}
|
||||
}
|
||||
|
||||
/// `skill_was_indexed` is how the scorer tells a pointer from a body. A
|
||||
/// file entry must read as a pointer, or `always_inject` logic would treat
|
||||
/// every `files`-arm skill as handed over.
|
||||
#[test]
|
||||
fn a_file_entry_reads_as_indexed_not_inlined() {
|
||||
let entry = file_entry("Summarise.", Some("when asked"), &skill_file_path("x"));
|
||||
assert!(entry.contains(READ_FILE_IT), "{entry}");
|
||||
let prompt = format!(
|
||||
"Task\n\n{}x ---\n{entry}\n",
|
||||
crate::topology_exec::SKILL_MARKER
|
||||
);
|
||||
assert_eq!(skill_was_indexed(&prompt, "x"), Some(true));
|
||||
}
|
||||
|
||||
/// A prompt composed before the tool-loading sentence existed must still
|
||||
/// score as `Index`. Stored prompts are held for 90 days and re-scored
|
||||
/// when the scorer changes; if this regressed, every one of them would
|
||||
|
||||
+111
-12
@@ -174,13 +174,26 @@ impl<'a> Evidence<'a> {
|
||||
pub fn retrieved_skills(ev: &Evidence<'_>) -> Vec<String> {
|
||||
let mut out = Vec::new();
|
||||
for t in ev.tools {
|
||||
if t.tool != "ReadMcpResourceTool" {
|
||||
continue;
|
||||
}
|
||||
let Some(uri) = t.input.get("uri").and_then(|v| v.as_str()) else {
|
||||
continue;
|
||||
let name = match t.tool.as_str() {
|
||||
"ReadMcpResourceTool" => t
|
||||
.input
|
||||
.get("uri")
|
||||
.and_then(|v| v.as_str())
|
||||
.and_then(crate::mcp_skills::parse_uri)
|
||||
.map(|(_, name)| name),
|
||||
// The `files` arm: the pointer is a path and the retrieval is a
|
||||
// plain `Read`. Matched through `skill_from_file_path`, the reader
|
||||
// half of the function that wrote the path, for the same reason
|
||||
// the uri goes through `parse_uri`. A `Read` anywhere else is an
|
||||
// ordinary file read and is not a retrieval of anything.
|
||||
"Read" => t
|
||||
.input
|
||||
.get("file_path")
|
||||
.and_then(|v| v.as_str())
|
||||
.and_then(crate::skill_delivery::skill_from_file_path),
|
||||
_ => None,
|
||||
};
|
||||
if let Some((_, name)) = crate::mcp_skills::parse_uri(uri) {
|
||||
if let Some(name) = name {
|
||||
if !out.contains(&name) {
|
||||
out.push(name);
|
||||
}
|
||||
@@ -216,12 +229,12 @@ fn trigger_verdict(
|
||||
through the door instead and this becomes a tool call"
|
||||
.into(),
|
||||
),
|
||||
crate::skill_delivery::Mode::Index => {
|
||||
// Offered by name and `when_to_use`, and never opened. Whether that
|
||||
// is a miss depends on whether the skill had anything to say about
|
||||
// this phase at all: a skill with no machine-checkable consequence
|
||||
// here is one an agent is right to pass over, and scoring that as a
|
||||
// failure would punish correct triage.
|
||||
// Both retrieval arms: offered by name and `when_to_use`, and never
|
||||
// opened. Whether that is a miss depends on whether the skill had
|
||||
// anything to say about this phase at all: a skill with no
|
||||
// machine-checkable consequence here is one an agent is right to pass
|
||||
// over, and scoring that as a failure would punish correct triage.
|
||||
crate::skill_delivery::Mode::Index | crate::skill_delivery::Mode::Files => {
|
||||
if matches!(compliance, Verdict::NotApplicable)
|
||||
&& matches!(boundary, Verdict::NotApplicable)
|
||||
{
|
||||
@@ -948,6 +961,92 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
fn rendered_files(skills: &[(&str, &str)]) -> String {
|
||||
let body: String = skills
|
||||
.iter()
|
||||
.map(|(name, when)| {
|
||||
crate::topology_exec::render_pinned_skill(
|
||||
name,
|
||||
&crate::skill_delivery::file_entry(
|
||||
"a procedure",
|
||||
Some(when),
|
||||
&crate::skill_delivery::skill_file_path(name),
|
||||
),
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
format!(
|
||||
"Task: x\n\n# Your skills\n\n{}\n{body}",
|
||||
crate::skill_delivery::FILES_PREAMBLE
|
||||
)
|
||||
}
|
||||
|
||||
fn read_file(path: &str) -> ToolEvidence {
|
||||
ToolEvidence {
|
||||
tool: "Read".into(),
|
||||
path: Some(path.into()),
|
||||
input: json!({ "file_path": path }),
|
||||
response: serde_json::Value::Null,
|
||||
}
|
||||
}
|
||||
|
||||
/// The `files` arm's loop, end to end, for the same reason as the uri
|
||||
/// test below: `skill_file_path` writes the path, `file_entry` puts it in
|
||||
/// the prompt, `skill_from_file_path` reads it back off a `Read`.
|
||||
#[test]
|
||||
fn the_path_the_files_arm_advertises_is_the_one_the_scorer_recovers() {
|
||||
let prompt = rendered_files(&[("workspace-repo-commit-protocol", "before committing")]);
|
||||
assert!(
|
||||
prompt.contains("Read(file_path=\"/mission/skills/workspace-repo-commit-protocol.md\")"),
|
||||
"the entry must name the file to read:\n{prompt}"
|
||||
);
|
||||
assert_eq!(
|
||||
crate::skill_delivery::mode_in_prompt(&prompt),
|
||||
crate::skill_delivery::Mode::Files
|
||||
);
|
||||
let tools = vec![read_file("/mission/skills/workspace-repo-commit-protocol.md")];
|
||||
let ev = Evidence::new("", &tools);
|
||||
assert_eq!(retrieved_skills(&ev), vec!["workspace-repo-commit-protocol"]);
|
||||
let scored = score(&prompt, &ev, &builtin);
|
||||
assert_eq!(scored.len(), 1);
|
||||
assert!(
|
||||
matches!(scored[0].trigger, Verdict::Pass),
|
||||
"a Read of the advertised path IS the Trigger axis: {:?}",
|
||||
scored[0].trigger
|
||||
);
|
||||
}
|
||||
|
||||
/// Agents read files constantly. Only a `Read` INSIDE the skills directory
|
||||
/// is a retrieval; anything else scoring as one would make Trigger a count
|
||||
/// of file reads.
|
||||
#[test]
|
||||
fn an_ordinary_read_is_not_a_retrieval() {
|
||||
let tools = vec![
|
||||
read_file("/mission/repo/research/REPORT.md"),
|
||||
read_file("/mission/skills"),
|
||||
read_file("/mission/skills/nested/x.md"),
|
||||
read_file("/etc/passwd"),
|
||||
];
|
||||
assert!(retrieved_skills(&Evidence::new("", &tools)).is_empty());
|
||||
}
|
||||
|
||||
/// Under `files`, never opened is a miss when the skill had a checkable
|
||||
/// consequence — the same rule as `index`, and NOT `inline`'s
|
||||
/// "not observable", which would report the arm's own defect as a blind spot.
|
||||
#[test]
|
||||
fn the_files_arm_scores_a_miss_like_the_index_arm() {
|
||||
let prompt = rendered_files(&[("int-xx-marker-protocol", "when writing markers")]);
|
||||
let tools: Vec<ToolEvidence> = vec![];
|
||||
let ev = Evidence::new("INT-01 something without the required shape", &tools);
|
||||
let scored = score(&prompt, &ev, &builtin);
|
||||
assert_eq!(scored.len(), 1);
|
||||
assert!(
|
||||
!matches!(scored[0].trigger, Verdict::NotObservable(_)),
|
||||
"files is a retrieval arm; a miss must not read as inline: {:?}",
|
||||
scored[0].trigger
|
||||
);
|
||||
}
|
||||
|
||||
/// The whole loop, end to end: the index writes a uri, the agent reads that
|
||||
/// exact uri back, and the scorer recovers the skill's name from it.
|
||||
///
|
||||
|
||||
@@ -407,18 +407,21 @@ impl ZeroClawDriveExecutor {
|
||||
// applies to everyone who writes reads as nobody's in particular.
|
||||
let text = match mode {
|
||||
crate::skill_delivery::Mode::Inline => b.skill.body.clone(),
|
||||
crate::skill_delivery::Mode::Index if b.skill.always_inject => {
|
||||
b.skill.body.clone()
|
||||
}
|
||||
m if m.is_retrieval() && b.skill.always_inject => b.skill.body.clone(),
|
||||
// An entry is a few hundred bytes whatever the body weighs, so
|
||||
// the index arm cannot hit the cap that follows. That is the
|
||||
// point of it, and the reason the cap is checked against the
|
||||
// rendered text rather than against the body.
|
||||
// the retrieval arms cannot hit the cap that follows. That is
|
||||
// the point of them, and the reason the cap is checked against
|
||||
// the rendered text rather than against the body.
|
||||
crate::skill_delivery::Mode::Index => crate::skill_delivery::index_entry(
|
||||
&b.skill.description,
|
||||
b.skill.when_to_use.as_deref(),
|
||||
&crate::mcp_skills::skill_uri(b.skill.workspace_id, &b.skill.name),
|
||||
),
|
||||
crate::skill_delivery::Mode::Files => crate::skill_delivery::file_entry(
|
||||
&b.skill.description,
|
||||
b.skill.when_to_use.as_deref(),
|
||||
&crate::skill_delivery::skill_file_path(&b.skill.name),
|
||||
),
|
||||
};
|
||||
// Bounded, and truncation is STATED. A silently clipped procedure
|
||||
// is worse than an absent one: the agent follows the half it can
|
||||
|
||||
Reference in New Issue
Block a user