feat(skills): a skill that must be read cannot be left to be noticed
The `index` arm hands an agent a list of uris and trusts it to fetch what applies. Measured on the first A/B pair, that is mostly what happens — each agent fetched the skill bound to its own role and no other, which is the result that made Trigger observable at all. `workspace-repo-commit-protocol` is the case it fails on. It scored Trigger=FAIL beside a PASSING boundary check: the rule was live and unread. A procedure that applies to everyone who writes reads as nobody's in particular, so no agent recognises it as theirs and no agent fetches it. Upstream ZeroClaw arrived at the same place from the other direction and gave its compact injection mode an `always: true` frontmatter escape hatch (#9520). This is that hatch as a column: `skills.always_inject`, default FALSE, so nothing changes for an existing skill and the inline arm is untouched either way. Two halves, because delivering it and scoring it are different mistakes: - Delivery: under `Index`, an `always_inject` skill renders its BODY. - Scoring: the arm belongs to the PROMPT and `always_inject` belongs to the SKILL, so the scorer now asks per skill which one it got. A skill whose body is in the prompt was handed over, and a Trigger miss cannot be charged against an agent that was never asked to fetch anything. `skill_was_indexed` reads that off the rendered prompt via `READ_IT`, a constant now shared with `index_entry` — two spellings of one marker is how a detector quietly stops detecting. Suite: 108 binaries, 840 tests, green. 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
563b074116
commit
2f1a870949
@@ -185,7 +185,7 @@ pub fn index_entry(description: &str, when_to_use: Option<&str>, uri: &str) -> S
|
|||||||
.filter(|w| !w.is_empty())
|
.filter(|w| !w.is_empty())
|
||||||
.unwrap_or("not stated — judge from the description");
|
.unwrap_or("not stated — judge from the description");
|
||||||
format!(
|
format!(
|
||||||
"{}\nWhen to use: {}\nRead it: ReadMcpResourceTool(server=\"{}\", uri=\"{}\")",
|
"{}\nWhen to use: {}\n{READ_IT}server=\"{}\", uri=\"{}\")",
|
||||||
description.trim(),
|
description.trim(),
|
||||||
when,
|
when,
|
||||||
MCP_SERVER,
|
MCP_SERVER,
|
||||||
@@ -193,6 +193,45 @@ pub fn index_entry(description: &str, when_to_use: Option<&str>, uri: &str) -> S
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The line that makes an index entry recognisable as one.
|
||||||
|
///
|
||||||
|
/// Shared by the renderer and [`skill_was_indexed`] so the scorer cannot drift
|
||||||
|
/// from the delivery — two spellings of one marker is how a detector quietly
|
||||||
|
/// stops detecting.
|
||||||
|
pub const READ_IT: &str = "Read it: ReadMcpResourceTool(";
|
||||||
|
|
||||||
|
/// How was THIS skill delivered, regardless of the arm the prompt announces?
|
||||||
|
///
|
||||||
|
/// `Some(true)` — an index entry: named, described, and left to be fetched.
|
||||||
|
/// `Some(false)` — the body itself, which under `Index` means the skill is
|
||||||
|
/// marked `always_inject`.
|
||||||
|
/// `None` — not in the prompt at all (it was retrieved, or never delivered).
|
||||||
|
///
|
||||||
|
/// The arm is a property of the PROMPT; `always_inject` is a property of the
|
||||||
|
/// SKILL. Scoring the arm alone would report a Trigger failure against a skill
|
||||||
|
/// the agent was handed and was never asked to fetch.
|
||||||
|
pub fn skill_was_indexed(prompt: &str, skill: &str) -> Option<bool> {
|
||||||
|
let marker = crate::topology_exec::SKILL_MARKER;
|
||||||
|
let mut lines = prompt.lines();
|
||||||
|
// Find this skill's section...
|
||||||
|
lines.find(|l| {
|
||||||
|
l.trim()
|
||||||
|
.strip_prefix(marker)
|
||||||
|
.map(|rest| rest.trim_end_matches(" ---").trim() == skill)
|
||||||
|
.unwrap_or(false)
|
||||||
|
})?;
|
||||||
|
// ...and read to the next one.
|
||||||
|
for l in lines {
|
||||||
|
if l.trim().starts_with(marker) {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
if l.contains(READ_IT) {
|
||||||
|
return Some(true);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Some(false)
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|||||||
@@ -263,10 +263,19 @@ pub fn score(
|
|||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|skill| {
|
.map(|skill| {
|
||||||
let (compliance, boundary) = check(&skill, evidence);
|
let (compliance, boundary) = check(&skill, evidence);
|
||||||
|
// The arm belongs to the prompt; `always_inject` belongs to the
|
||||||
|
// skill. A skill whose BODY is in the prompt was handed over, so
|
||||||
|
// there is no reaching-for to observe even under `Index` — scoring
|
||||||
|
// it as a Trigger miss would report a failure against an agent that
|
||||||
|
// was never asked to fetch anything.
|
||||||
|
let delivered = match crate::skill_delivery::skill_was_indexed(prompt, &skill) {
|
||||||
|
Some(false) => crate::skill_delivery::Mode::Inline,
|
||||||
|
_ => mode,
|
||||||
|
};
|
||||||
SkillUse {
|
SkillUse {
|
||||||
source_kind: source_kinds(&skill),
|
source_kind: source_kinds(&skill),
|
||||||
trigger: trigger_verdict(
|
trigger: trigger_verdict(
|
||||||
mode,
|
delivered,
|
||||||
retrieved.contains(&skill),
|
retrieved.contains(&skill),
|
||||||
&compliance,
|
&compliance,
|
||||||
&boundary,
|
&boundary,
|
||||||
@@ -1007,6 +1016,62 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// The control arm has to be unchanged, or the A/B measures this edit too.
|
/// The control arm has to be unchanged, or the A/B measures this edit too.
|
||||||
|
/// The case this whole flag exists for.
|
||||||
|
///
|
||||||
|
/// `workspace-repo-commit-protocol` applies to every agent that writes,
|
||||||
|
/// which is exactly why no agent reads it as *theirs* — it scored
|
||||||
|
/// Trigger=FAIL beside a passing boundary check on the first A/B pair.
|
||||||
|
/// Marked `always_inject`, its body is in the prompt under the index arm,
|
||||||
|
/// and a skill the agent was handed cannot be a reaching-for failure.
|
||||||
|
#[test]
|
||||||
|
fn a_skill_delivered_in_full_under_the_index_arm_is_not_a_trigger_miss() {
|
||||||
|
let prompt = format!(
|
||||||
|
"{}\n{}{}",
|
||||||
|
crate::skill_delivery::INDEX_PREAMBLE,
|
||||||
|
crate::topology_exec::render_pinned_skill(
|
||||||
|
"workspace-repo-commit-protocol",
|
||||||
|
"Commit only inside /workspace/repo. Never write outside it.",
|
||||||
|
),
|
||||||
|
rendered_index(&[("arxiv-daily", "when sweeping arxiv")]),
|
||||||
|
);
|
||||||
|
let tools = [];
|
||||||
|
let ev = Evidence { text: "", tools: &tools };
|
||||||
|
let got = score(&prompt, &ev, &|_| "builtin".to_string());
|
||||||
|
let it = got
|
||||||
|
.iter()
|
||||||
|
.find(|u| u.skill == "workspace-repo-commit-protocol")
|
||||||
|
.expect("the always-injected skill must still be scored");
|
||||||
|
assert!(
|
||||||
|
matches!(it.trigger, Verdict::NotObservable(_)),
|
||||||
|
"handed over, not offered — there is no retrieval to miss: {:?}",
|
||||||
|
it.trigger
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The flag must not leak: a skill still delivered as an index entry keeps
|
||||||
|
/// being scored on whether it was fetched.
|
||||||
|
#[test]
|
||||||
|
fn an_indexed_skill_in_the_same_prompt_is_still_judged_on_retrieval() {
|
||||||
|
let prompt = format!(
|
||||||
|
"{}\n{}{}",
|
||||||
|
crate::skill_delivery::INDEX_PREAMBLE,
|
||||||
|
crate::topology_exec::render_pinned_skill(
|
||||||
|
"workspace-repo-commit-protocol",
|
||||||
|
"Commit only inside /workspace/repo.",
|
||||||
|
),
|
||||||
|
rendered_index(&[("arxiv-daily", "when sweeping arxiv")]),
|
||||||
|
);
|
||||||
|
for u in score(&prompt, &Evidence { text: "", tools: &[] }, &|_| "builtin".into()) {
|
||||||
|
if u.skill != "workspace-repo-commit-protocol" {
|
||||||
|
assert!(
|
||||||
|
!matches!(u.trigger, Verdict::NotObservable(_)),
|
||||||
|
"{} was offered by uri, so retrieval is observable for it",
|
||||||
|
u.skill
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn the_inline_arm_still_reports_trigger_as_unobservable() {
|
fn the_inline_arm_still_reports_trigger_as_unobservable() {
|
||||||
let prompt = rendered(&[("workspace-repo-commit-protocol", "body")]);
|
let prompt = rendered(&[("workspace-repo-commit-protocol", "body")]);
|
||||||
|
|||||||
@@ -399,8 +399,17 @@ impl ZeroClawDriveExecutor {
|
|||||||
let mut out = String::new();
|
let mut out = String::new();
|
||||||
let mut n = 0usize;
|
let mut n = 0usize;
|
||||||
for b in bindings.iter().filter(|b| b.pin_in_context) {
|
for b in bindings.iter().filter(|b| b.pin_in_context) {
|
||||||
|
// `always_inject` overrides the arm. Progressive disclosure asks
|
||||||
|
// the agent to recognise that a procedure applies before fetching
|
||||||
|
// it, and a CROSS-CUTTING procedure is the case that breaks: the
|
||||||
|
// first A/B pair had `workspace-repo-commit-protocol` scored
|
||||||
|
// Trigger=FAIL beside a passing boundary check, because a rule that
|
||||||
|
// applies to everyone who writes reads as nobody's in particular.
|
||||||
let text = match mode {
|
let text = match mode {
|
||||||
crate::skill_delivery::Mode::Inline => b.skill.body.clone(),
|
crate::skill_delivery::Mode::Inline => b.skill.body.clone(),
|
||||||
|
crate::skill_delivery::Mode::Index if b.skill.always_inject => {
|
||||||
|
b.skill.body.clone()
|
||||||
|
}
|
||||||
// An entry is a few hundred bytes whatever the body weighs, so
|
// An entry is a few hundred bytes whatever the body weighs, so
|
||||||
// the index arm cannot hit the cap that follows. That is the
|
// the index arm cannot hit the cap that follows. That is the
|
||||||
// point of it, and the reason the cap is checked against the
|
// point of it, and the reason the cap is checked against the
|
||||||
|
|||||||
@@ -29,6 +29,14 @@ pub struct Skill {
|
|||||||
pub workspace_id: Option<Uuid>,
|
pub workspace_id: Option<Uuid>,
|
||||||
pub current_version: i32,
|
pub current_version: i32,
|
||||||
pub body: String,
|
pub body: String,
|
||||||
|
/// Deliver the full body even in the `index` arm.
|
||||||
|
///
|
||||||
|
/// Progressive disclosure asks the agent to recognise that a procedure
|
||||||
|
/// applies before it fetches it. That works for role-shaped skills and
|
||||||
|
/// fails for cross-cutting ones — a commit protocol applies to every agent
|
||||||
|
/// that writes, which is precisely why no agent reads it as *theirs*.
|
||||||
|
#[serde(default)]
|
||||||
|
pub always_inject: bool,
|
||||||
#[serde(with = "time::serde::rfc3339")]
|
#[serde(with = "time::serde::rfc3339")]
|
||||||
pub created_at: OffsetDateTime,
|
pub created_at: OffsetDateTime,
|
||||||
#[serde(with = "time::serde::rfc3339")]
|
#[serde(with = "time::serde::rfc3339")]
|
||||||
@@ -155,7 +163,7 @@ pub async fn upsert_builtin(pool: &PgPool, b: UpsertBuiltinSkill<'_>) -> Result<
|
|||||||
/// All builtin + workspace-scoped skills the caller can see.
|
/// All builtin + workspace-scoped skills the caller can see.
|
||||||
pub async fn list_visible(pool: &PgPool, workspace_id: Uuid) -> Result<Vec<Skill>, DbError> {
|
pub async fn list_visible(pool: &PgPool, workspace_id: Uuid) -> Result<Vec<Skill>, DbError> {
|
||||||
let rows = sqlx::query(
|
let rows = sqlx::query(
|
||||||
"SELECT id, name, description, when_to_use, tags, source_kind,
|
"SELECT id, name, description, when_to_use, tags, source_kind, always_inject,
|
||||||
workspace_id, current_version, body, created_at, updated_at
|
workspace_id, current_version, body, created_at, updated_at
|
||||||
FROM skills
|
FROM skills
|
||||||
WHERE workspace_id IS NULL OR workspace_id = $1
|
WHERE workspace_id IS NULL OR workspace_id = $1
|
||||||
@@ -169,7 +177,7 @@ pub async fn list_visible(pool: &PgPool, workspace_id: Uuid) -> Result<Vec<Skill
|
|||||||
|
|
||||||
pub async fn get(pool: &PgPool, id: Uuid) -> Result<Option<Skill>, DbError> {
|
pub async fn get(pool: &PgPool, id: Uuid) -> Result<Option<Skill>, DbError> {
|
||||||
let row = sqlx::query(
|
let row = sqlx::query(
|
||||||
"SELECT id, name, description, when_to_use, tags, source_kind,
|
"SELECT id, name, description, when_to_use, tags, source_kind, always_inject,
|
||||||
workspace_id, current_version, body, created_at, updated_at
|
workspace_id, current_version, body, created_at, updated_at
|
||||||
FROM skills WHERE id = $1",
|
FROM skills WHERE id = $1",
|
||||||
)
|
)
|
||||||
@@ -188,7 +196,7 @@ pub async fn get_by_name(
|
|||||||
// in the migration.
|
// in the migration.
|
||||||
let sentinel: Uuid = "00000000-0000-0000-0000-000000000000".parse().unwrap();
|
let sentinel: Uuid = "00000000-0000-0000-0000-000000000000".parse().unwrap();
|
||||||
let row = sqlx::query(
|
let row = sqlx::query(
|
||||||
"SELECT id, name, description, when_to_use, tags, source_kind,
|
"SELECT id, name, description, when_to_use, tags, source_kind, always_inject,
|
||||||
workspace_id, current_version, body, created_at, updated_at
|
workspace_id, current_version, body, created_at, updated_at
|
||||||
FROM skills
|
FROM skills
|
||||||
WHERE COALESCE(workspace_id, $1::uuid) = COALESCE($2::uuid, $1::uuid)
|
WHERE COALESCE(workspace_id, $1::uuid) = COALESCE($2::uuid, $1::uuid)
|
||||||
@@ -365,7 +373,7 @@ pub async fn effective_for_agent(
|
|||||||
// Batch-fetch the skill bodies in one query.
|
// Batch-fetch the skill bodies in one query.
|
||||||
let ids: Vec<Uuid> = ordered.iter().map(|(id, _, _)| *id).collect();
|
let ids: Vec<Uuid> = ordered.iter().map(|(id, _, _)| *id).collect();
|
||||||
let skill_rows = sqlx::query(
|
let skill_rows = sqlx::query(
|
||||||
"SELECT id, name, description, when_to_use, tags, source_kind,
|
"SELECT id, name, description, when_to_use, tags, source_kind, always_inject,
|
||||||
workspace_id, current_version, body, created_at, updated_at
|
workspace_id, current_version, body, created_at, updated_at
|
||||||
FROM skills WHERE id = ANY($1)",
|
FROM skills WHERE id = ANY($1)",
|
||||||
)
|
)
|
||||||
@@ -405,6 +413,7 @@ fn row_to_skill(r: sqlx::postgres::PgRow) -> Skill {
|
|||||||
workspace_id: r.get("workspace_id"),
|
workspace_id: r.get("workspace_id"),
|
||||||
current_version: r.get("current_version"),
|
current_version: r.get("current_version"),
|
||||||
body: r.get("body"),
|
body: r.get("body"),
|
||||||
|
always_inject: r.get("always_inject"),
|
||||||
created_at: r.get("created_at"),
|
created_at: r.get("created_at"),
|
||||||
updated_at: r.get("updated_at"),
|
updated_at: r.get("updated_at"),
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,20 @@
|
|||||||
|
-- A skill that must reach the agent in FULL, whatever the delivery arm is.
|
||||||
|
--
|
||||||
|
-- The `index` arm hands an agent a list of uris and trusts it to fetch what
|
||||||
|
-- applies. Measured on the first A/B pair, that is mostly what happens — each
|
||||||
|
-- agent fetched the skill bound to its own role and no other. But a procedure
|
||||||
|
-- the agent does not RECOGNISE as applying is a procedure it never fetches, and
|
||||||
|
-- `workspace-repo-commit-protocol` scored Trigger=FAIL for exactly that reason
|
||||||
|
-- while its boundary check passed: the rule was live and unread.
|
||||||
|
--
|
||||||
|
-- Upstream ZeroClaw reached the same place from the other direction and added
|
||||||
|
-- an `always: true` frontmatter escape hatch to its compact injection mode
|
||||||
|
-- (#9520). This is that hatch, as a column.
|
||||||
|
--
|
||||||
|
-- Default FALSE, so nothing changes for any existing skill and the inline arm
|
||||||
|
-- is unaffected either way.
|
||||||
|
ALTER TABLE skills
|
||||||
|
ADD COLUMN IF NOT EXISTS always_inject BOOLEAN NOT NULL DEFAULT FALSE;
|
||||||
|
|
||||||
|
COMMENT ON COLUMN skills.always_inject IS
|
||||||
|
'Deliver this skill''s full body even in the index (progressive-disclosure) arm.';
|
||||||
Reference in New Issue
Block a user