Repository navigation
tests: enforce the skill spine + routing selection (#109, #110) - #111
Conversation
Reference files load lazily, so a guardrail living only in a shared file is one the model may never read — an unloaded guardrail is no guardrail. The spine is therefore duplicated into every skill body on purpose; the cost is drift (#105 shipped because two copies of one governance claim diverged). Add an invariant over every plugin/skills/*/SKILL.md (the #108 glob) asserting each body carries its load-bearing markers: the untrusted-data principle, evidence-over- assertion, and a write-gating rule for its tier (union set across investigate/sweep/ propose tiers, so skills not yet written must still carry one). Distinctive-phrase match, case-insensitive, strict on absence — proves the lock exists, doesn't freeze prose. Coherence by construction. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VcpWhxi8ZFKgUTSHZY5ZhF Signed-off-by: Matt W <matt.willems@exabeam.com>
With multiple skills, routing moved to the description layer — Claude Code picks a skill by matching the ask against frontmatter descriptions — and nothing tested it (the #103 Finding A collision: bare "triage" owned by both soc-investigate and triage-cases). - evals/routing-corpus.json: canonical asks -> expected skill, incl. the decided answer for the once-ambiguous "triage this alert/case". - tests/test_skill_routing.py: deterministic (no model, no tenant) — no advertised trigger phrase may be a substring of another skill's (the collision guard that catches Finding A), and every corpus ask must route to exactly its expected skill via those phrases. A proxy for the model's selection, chosen so it grades in CI and fails the moment two descriptions overlap. New skill => new cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VcpWhxi8ZFKgUTSHZY5ZhF Signed-off-by: Matt W <matt.willems@exabeam.com>
virtualsteve-exa
left a comment
There was a problem hiding this comment.
Reviewed by running it and then trying to break it — for a test PR the review is mutation testing, and your "verified against a violation" claims both check out on my machine:
- Full suite on the branch: 285 passed, matching your count (13 corpus cases + advertise + collision + spine = 16 new).
- Mutation 1 — re-added bare
"triage"to soc-investigate's description:test_no_trigger_phrase_collides_across_skillsfails, and the corpus test independently fails onmorning triagenow routing to both skills. Two nets catching the same fish from different sides is exactly what you want from a routing guard. - Mutation 2 — swapped
untrusted data, never instructionsforuntrusted contentin rule-tuning: spine test fails, correctly named marker in the message.
Approving. Three findings for the record — one real, two nits, none blocking:
1. The spine's write-gating marker is union-across-tiers, and that's a documented softness worth a line in the test comment. I ran a third mutation you didn't claim: stripping soc-investigate's explicit-ask language. It happened to still pass because "ask the analyst" occurs twice in the body — but the design property underneath is that the union set asserts some recognized write-gating phrase exists, not this skill's tier's gate exists. A future investigate-tier skill could satisfy the marker with propose-only prose while lacking any human-ask gate. Your comment already flags the union as deliberate; I'd add the one-sentence honest limit, and when a skill ever carries tier metadata (a frontmatter tier: key, say), bind marker-subset to tier then. Not this PR's job.
2. _description only parses the >- folded form. A future skill writing description: one line yields zero phrases and fails test_skills_advertise_trigger_phrases with a message that will read as "add trigger phrases" when the actual problem is the parser. Cheap fix whenever: fall back to the single-line form (the #108 reference branch has the two-form regex to lift).
3. The corpus asks are the advertised phrases nearly verbatim, which makes the corpus test partially tautological today — its real value arrives with the next skill, when new phrases must coexist with these. Your docstring is honest that this is a proxy for model selection, and I think the proxy is the right call for CI; just flagging that a model-graded paraphrase corpus ("what's hot this morning?") is the eventual second half if routing ever misbehaves live despite green tests.
On #108: the body says "yours" — per Steve's allocation it's assigned to you with the rest of the hardening stream; the reference branch tests/all-skills-frontmatter is there to cherry-pick (its frontmatter test also has the two-form description regex from finding 2). Your _skill_dirs() glob landing first makes that diff smaller, which is a nice side effect.
Good speed on this — #109 and #110 closed within a day of being assigned, and closed with tests that bite.
Closes #109, closes #110. Sequenced behind #103 (now merged); implements the two enforcement mechanisms from Steve's architecture review — coherence by construction, not by discipline.
#109 — enforce the shared skill spine
An invariant over every
plugin/skills/*/SKILL.mdasserting each body carries its load-bearing safety markers: the untrusted-data principle, evidence-over-assertion, and a write-gating rule for its tier (union set across investigate / sweep / propose, so a skill nobody has written yet still has to carry one). Distinctive-phrase match, case-insensitive, strict on absence — proves the lock exists without freezing prose. Duplicating safety prose into every body is the correct doctrine (reference files load lazily — an unloaded guardrail is no guardrail); this makes the duplication safe by catching drift, the failure mode behind #105.#110 — routing corpus + selection tests
evals/routing-corpus.json— canonical asks → expected skill, including the decided tie-break for the once-ambiguous"triage this alert/case".tests/test_skill_routing.py— deterministic (no model, no tenant): no advertised trigger phrase may be a substring of another skill's (the collision guard that would have caught skills: fleet-triage suite — triage-cases + rule-tuning (drafts for feedback) #103 Finding A — bare "triage" nested in "triage the queue"), and every corpus ask routes to exactly its expected skill via those phrases. It's a proxy for the model's selection, chosen so it grades in CI and fails the moment two descriptions overlap.Notes
_skill_dirs()introduces theplugin/skills/*glob. I kept the existing soc-investigate-hardcoded tests untouched so Skill invariant tests validate only soc-investigate — every added skill escapes the suite #108's generalization doesn't conflict — that glob is now available for it.triage-alertslands straight into both harnesses (new skill ⇒ spine markers enforced + new routing cases).Verification
pytest285 passed (16 new: the spine invariant + 15 routing checks). No changes to any skill, connector, or governance config — tests only.