fix(skills): a pinned skill contradicted the platform inside the same prompt
Extending the Skill-Use mechanical checks, per the baseline's own next step, found something bigger than a missing check. THE DEFECT `workspace-repo-commit-protocol` told agents that `/workspace/repo` was "the ONLY path where source-modifying edits belong". The platform mounts and advertises `/mission/repo` — 26 references in the code; `/workspace/repo` appears in none of them. The skill is bound on 29 role bindings and was delivered TWICE in the run already measured, so an agent received the real path in its tool preamble and a skill contradicting it a few hundred tokens later, in one prompt. An agent that obeyed the skill wrote source into a directory nothing collects — the phase then delivers nothing, and looks like an agent that did no work. The same skill instructed `file_read` / `file_write` / `shell`: ZeroClaw's names, the exact ones `phase_task_text` was fixed to stop advertising after five agents on a single mission spent 7.4k tokens describing the mismatch instead of working. The prompt was corrected and the skill kept saying it. Rewritten against what the code actually does, including the repo-less case (`/mission/repo` exists, is collected as artifacts, has nothing to push). THE CLASS, AND THE GUARD The skills were never checked against the platform they describe. Nothing compared them, so a skill could contradict the prompt it ships inside and stay that way indefinitely — the same shape as PLAN_COMPLETE being documented and never implemented. Two tests in `skills_loader::contradiction_tests` now hold it: no skill may name a repo path the platform does not mount, and none may instruct a tool the agent's subprocess does not expose. The second matches backticked instructions and skips corrective lines, so a skill may still WARN against the wrong names — as this one now does. Both negative-controlled by restoring the old wording. AND THE CHECK THAT STARTED IT `workspace-repo-commit-protocol` now has a Boundary check: writing outside `/mission/repo` fails, and the message names the consequence — a phase that delivers nothing — rather than just the wrong path. docs/SKILL-USE-BASELINE.md records this as the fourth defect the measurement found, and corrects the "next unit of work" note now that this one is done. Full workspace suite green: 106 binaries, zero build errors. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
113de610ec
commit
771092b165
@@ -170,3 +170,107 @@ mod tests {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod contradiction_tests {
|
||||
use std::path::PathBuf;
|
||||
|
||||
fn skills_root() -> PathBuf {
|
||||
PathBuf::from(env!("CARGO_MANIFEST_DIR"))
|
||||
.join("../../skills")
|
||||
.canonicalize()
|
||||
.expect("skills dir")
|
||||
}
|
||||
|
||||
fn all_skills() -> Vec<(String, String)> {
|
||||
fn walk(dir: &std::path::Path, out: &mut Vec<(String, String)>) {
|
||||
for e in std::fs::read_dir(dir).expect("read skills dir") {
|
||||
let p = e.expect("entry").path();
|
||||
if p.is_dir() {
|
||||
walk(&p, out);
|
||||
} else if p.extension().and_then(|x| x.to_str()) == Some("md") {
|
||||
out.push((
|
||||
p.file_name().unwrap().to_string_lossy().to_string(),
|
||||
std::fs::read_to_string(&p).expect("read skill"),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
let mut out = Vec::new();
|
||||
walk(&skills_root(), &mut out);
|
||||
out
|
||||
}
|
||||
|
||||
/// No skill may teach a workspace path the platform does not use.
|
||||
///
|
||||
/// `workspace-repo-commit-protocol` told agents that `/workspace/repo` was
|
||||
/// "the ONLY path where source-modifying edits belong". The platform mounts
|
||||
/// and advertises `/mission/repo` — in 26 places — and `/workspace/repo`
|
||||
/// appears nowhere in the code. The skill is pinned on 29 role bindings and
|
||||
/// was delivered twice in a single measured run, so agents received the
|
||||
/// platform's real path and a skill contradicting it in the SAME prompt.
|
||||
#[test]
|
||||
fn no_skill_teaches_a_repo_path_the_platform_does_not_mount() {
|
||||
let mut offenders = Vec::new();
|
||||
for (name, body) in all_skills() {
|
||||
if body.contains("/workspace/repo") {
|
||||
offenders.push(name);
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
offenders.is_empty(),
|
||||
"{} skill(s) name /workspace/repo; the mission checkout is \
|
||||
/mission/repo, so an agent following them writes somewhere that is \
|
||||
never delivered: {}",
|
||||
offenders.len(),
|
||||
offenders.join(", ")
|
||||
);
|
||||
}
|
||||
|
||||
/// No skill may instruct an agent to call a tool it does not have.
|
||||
///
|
||||
/// Every mission turn ends in `claude -p`, so the tools are Claude Code's
|
||||
/// (`Read`/`Edit`/`Write`/`Bash`/`Glob`/`Grep`). `phase_task_text` used to
|
||||
/// advertise ZeroClaw's names and was fixed after five agents spent 7.4k
|
||||
/// tokens on one mission describing the mismatch instead of working — and
|
||||
/// the same wrong names survived inside a pinned skill.
|
||||
///
|
||||
/// Matched as a backticked instruction, not as bare words: a skill may
|
||||
/// legitimately DISCUSS these names, as this one now does when warning
|
||||
/// against them.
|
||||
#[test]
|
||||
fn no_skill_instructs_an_agent_to_call_a_zeroclaw_tool() {
|
||||
const ZEROCLAW_TOOLS: &[&str] = &[
|
||||
"`file_read`",
|
||||
"`file_write`",
|
||||
"`file_edit`",
|
||||
"`content_search`",
|
||||
"`glob_search`",
|
||||
];
|
||||
let mut offenders = Vec::new();
|
||||
for (name, body) in all_skills() {
|
||||
// The line has to READ as an instruction. "Do not reach for
|
||||
// `file_read`" is the correction, not the defect.
|
||||
for line in body.lines() {
|
||||
let l = line.to_ascii_lowercase();
|
||||
if l.contains("do not")
|
||||
|| l.contains("never")
|
||||
|| l.contains("instead of")
|
||||
|| l.contains("not what")
|
||||
{
|
||||
continue;
|
||||
}
|
||||
if ZEROCLAW_TOOLS.iter().any(|t| line.contains(t)) {
|
||||
offenders.push(format!("{name}: {}", line.trim()));
|
||||
}
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
offenders.is_empty(),
|
||||
"{} skill line(s) tell an agent to use a tool its subprocess does \
|
||||
not expose:\n {}",
|
||||
offenders.len(),
|
||||
offenders.join("\n ")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user