diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index 86ab450..1d30bdb 100644 --- a/plugin/.claude-plugin/plugin.json +++ b/plugin/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "scribe", "description": "Scribe for Claude Code: connects the scribe MCP server, adds the hooks that deliver live project state and relevant records at the right moment, ships the shared client-neutral Scribe skills (using-scribe, writing-plans, reporting-back, systematic-debugging, verification, brainstorming, reusing-code, shape-accounting), and syncs your saved Scribe Processes as skills (/scribe:sync).", - "version": "2026.09.24.1042", + "version": "2026.09.24.1358", "author": { "name": "Bryan Van Deusen" }, diff --git a/plugin/skills/using-scribe/SKILL.md b/plugin/skills/using-scribe/SKILL.md index da75f16..c469ab9 100644 --- a/plugin/skills/using-scribe/SKILL.md +++ b/plugin/skills/using-scribe/SKILL.md @@ -17,19 +17,8 @@ If the working directory maps to a Scribe project, call `enter_project(id)` — it returns the project's goal, the milestones and open tasks worked on most recently, its Systems and the titles of its own rules in one shot. -**A directory does not have to be a git repo to have a project.** A repo is -bound by its remote (`list_repo_bindings` shows the bindings). Anything else — -a notes folder, a server's config directory, a scratch directory — is bound by -a `.scribe` file naming the project: - - {"instance": "https://scribe.example.com", "project_id": 2, "project": "Homelab"} - -`instance` is what makes the id trustworthy. An id means nothing on its own — -it is a different project on every Scribe — so a marker that has travelled to -another instance is ignored rather than followed to the wrong project. A bare -`2` also works when writing the file by hand. When work plainly belongs to a -project and the directory names none, offer to write the marker; -`list_projects` has the id. +Binding a directory that is not a git repo, or starting a new project, has +its own steps: read [projects.md](projects.md). Then **ask before you act**: before anything hard to reverse or outward-facing, search the rules for what you are about to do. Reflex 2 below is why asking, @@ -120,17 +109,9 @@ Two constraints on *how* that's achieved: learned is that something MUST be done a certain way, that is a rule to propose, not a preference to harden in place. - **A lesson grows each time it proves itself.** When one arrives and the - situation it names is the one you are actually in, you are the single - reader placed to tell whether its trigger is keyed right and whether its - claim covers what you are seeing. `update_lesson` takes what you now know: - another incident added to what taught it, the claim stated more exactly, - or — the edit worth most — a trigger re-keyed to the situation that really - fired. A lesson nobody reaches is seldom wrong; far more often it is - waiting in a situation nobody is in. One claim that has met the same - failure four times is worth more than four claims that each met it once, - so when a near-duplicate create hands back an existing id, that is the - record to grow. + **A lesson grows each time it proves itself.** When one arrives and names the + situation you are actually in, `update_lesson` it with what you now know; + [writing-records.md](writing-records.md) says what is worth adding. **Preferences shape how work is done, never what gets recorded.** They govern your conduct — how you report, how carefully you pace, which form @@ -246,47 +227,10 @@ Two constraints on *how* that's achieved: re-measurement, a reversed decision), pass the old id in `supersedes` so the stale record is demoted and labelled rather than left competing. -10. **A few notes assert a FACT, and those can carry their own check.** - Supersession only fires once somebody has read a note and disagreed — which - is the case where it was already believed. A note asserting something about - *someone else's* software — what a service does on a duplicate upload, how a - forge numbers its CI runs, what an updater compares — can instead carry - `verify_with` (how to check it) and `expires_when` (the STATE that ends it: - "when the forge numbers runs per workflow", never "in six months"). - `notes_due_for_verification` lists them least-recently-confirmed first, with - never-checked at the top; `mark_note_verified` records what you found, and - `still_true=False` deliberately writes nothing — a note whose check failed - is wrong rather than in a state worth recording, so it keeps its place. - - **The test is one question: could this note become false without anyone - editing it?** If no, leave both fields empty. That is the normal case, and - an empty `verify_with` is the positive marker for "this is a decision, there - is nothing to go and check" — not an unfinished record. The sweep is only - worth reading while almost nothing is on it, so a check added out of - tidiness costs the whole surface, not just that note. - - **The sharper form of the same test: is the thing this note describes yours - to change?** If yes it is a decision — editing your own software is how it - changes, and you will know you did it. Measured against a real corpus, every - note that earned a check was about somebody ELSE's software: a signing - service, a forge, a hub, an SDK, a model, a dependency set. - - **Three that look like candidates and are not:** - - **Resume pointers and "current state" notes.** They go stale fastest of - anything, which is exactly why they tempt — but the cure is to update or - delete them, not to schedule a check. A sweep full of pointers is a sweep - nobody reads. - - **Measurements of your own system.** They go false because you changed - something, and you knew. A measurement earns a check only when what it - measures is outside your control. - - **A decision that RESTS on somebody else's behaviour.** The decision is - still a decision. Put the check on the note asserting the fact, and link - the decision to it. - - Not for tasks — a task's decay is its status, and a done issue records what - happened rather than asserting something that can go false. Not for snippets - either: `verify_snippet` compares the recorded location and code against the - repo, which is richer and already wired to drift detection. +10. **A note that asserts a fact about someone else's software can carry its + own check** (`verify_with`, `expires_when`), swept by + `notes_due_for_verification`. Most notes should not: read + [writing-records.md](writing-records.md) before filling either field. 11. **Report back in a shape the operator can read.** They were not there while you worked, so organise the reply around **where the work stands**, not the @@ -311,80 +255,18 @@ bound — confine the session to it: - If something clearly belongs to a *different* project, say so and **ask before switching** — never silently operate cross-project. -## Starting a project: decide what it inherits +## Read these when the moment comes -A project's inheritance is a **decision, not a default**. Before -`create_project`, ask the operator the two inception questions and pass the -answers — never create a project bare by default: +Each is part of this skill, kept in its own file because it matters at one +moment rather than on every turn: -- which **design system** its UI is built from (`list_design_systems`; or - none) → `design_system_id=` -- whether to **seed the standard starter Systems** so records can be tagged - from day one → `seed_systems=true|false` - -Rules are not an inception question: a global rule already applies to every -project, and a project's own rules are written on it as they come up. - -If `enter_project` returns an `inception` key, the project was never decided -(it inherits its defaults silently): raise that ask once, with the defaults it -carries, then `decide_project_inception(project_id, …)`. Existing projects -were stamped "legacy" and do not ask; any project can be re-decided. The -design-system and Systems tools still work one at a time — inception is the -moment they are decided together, and the record of why. - -## Where a new rule goes - -A rule has one of two homes, and the home IS its reach: - -- **Global** — in a rulebook (`create_rule` into a topic). It applies in every - project, and reaches a session wherever the work matches it. A rulebook is a - *themed* grouping of general rules (e.g. a review checklist), not a list of - projects it binds — there is no subscribing a project to one. -- **Project** (`create_project_rule`) — anything specific to one project (its - files, paths, quirks). It reaches only that project's sessions. - -Names one project's specifics → project rule; a standard that holds wherever -the kind of work it describes happens → global. Never put project-specific -detail in a rulebook — it would reach every other project. A project that -departs from a global rule writes its own and links it with -`relate_rules(kind="overrides")`, which says why. A rule that turns out to be -in the wrong home — a project rule that holds everywhere, a global one only a -single project needs — moves with `move_rule`, which keeps its id, history, -areas and edges. Propose the move and make it on a yes. - -**Whichever home it gets, a rule needs `when_to_apply`.** It is the only thing -that decides whether the rule is ever seen: nothing is preloaded, so a rule -with no trigger is not a quiet rule, it is an unreachable one. Write the moment -in the words a session actually produces — the command, the error, the -half-formed ask — not the category it belongs to. - -**Before writing one, ask what already covers that moment.** -`what_might_apply("the moment you are about to write a record for")` — fifty -candidates and no bar, so an existing record cannot hide under a threshold the -way it can from `search`. When something already covers the moment, the reach -is that record: improve its trigger or its statement rather than standing a -second one beside it. Two records describing the same moment compete in one -ranked list against one budget, and the slot they take from each other is the -third candidate that would have said something different. Two records may -legitimately share a moment and say *different* things — a rule for what must -happen, a preference for how to report it. What this catches is the same thing -said twice at two strengths, which is worse than either alone: a session that -retrieves the softer copy has been told that binding guidance is optional. - -**First ask whether it's a rule at all.** A rule is prose you have to remember -and apply; Scribe's other entities are structure a tool can resolve and check. -Visual standards belong in a **design system**, not a rulebook — a token can be -inherited, resolved per mode, rendered to a stylesheet and diffed against code, -and none of that survives being written as a rule. A repeatable procedure is a -**process**; reusable code is a **snippet**. Reach for a rule when the thing -really is a standing instruction about how to work. - -Then ask what force it carries, by the question above. A standing instruction -that merely costs consistency is a **preference** (`create_preference`), and a -transferable insight that costs time is a **lesson** (`create_lesson`), keyed -to the situation it applies to so a later session meets it there. Both are -first-class outcomes of noticing something, not what's left when a rule -proposal fails. +- [projects.md](projects.md) — a directory with no project; starting a project + (`decide_project_inception` when `enter_project` returns `inception`). +- [writing-records.md](writing-records.md) — before writing a rule, preference + or lesson (where it goes, its `when_to_apply`, what already covers the + moment), and before giving a note a check. +- [missed-retrieval.md](missed-retrieval.md) — a rule that missed the moment it + governed, or keeps arriving where it doesn't apply. ## You are the judge of what the record says @@ -415,49 +297,6 @@ stored, so when you fix how judgments are made, go and look at the ones already there. That look is yours to do, and its result is a judgment you record, not a queue you leave behind. -## When a record doesn't reach the moment it should - -Retrieval misjudging is ordinary, and it is fixable — but only by whoever -notices. **Either direction counts:** a rule that should have governed a moment -and never arrived, and a rule that arrives on every turn and never applies. So -does **either noticer**: the operator saying *"that should have fired"*, and you -noticing it yourself — you reached for a rule nobody offered you, or you were -handed the same rule five times and set it aside five times. - -**Take it to the record first and the dial second.** A rule's `when_to_apply` -IS the text its similarity score is computed against, so when a rule misses a -moment it governs, the overwhelmingly likely cause is that its trigger does not -describe that moment in the words a session actually produces. Rewording one -trigger changes one rule's reach. Moving a floor changes what every record on -that surface does, and a floor cannot tell a badly-worded trigger from a -genuinely distant record — so one lowered to rescue a single rule admits -everything else that was sitting in the same band. - -1. **Read the refused records.** `retrieval_telemetry(days=N, - near_miss_samples=5)` names by id what each surface refused and by how much. - Open them with `get_rule` / `get_note`. This is the step that carries the - answer: the statistic says a record was close, and only the record says - whether it was *right*. -2. **Fix the trigger.** `update_rule(when_to_apply=...)`, written as the - symptom — what the session was doing or saying at the moment it needed this - rule — not the situation the rule belongs to. Then check that it worked: - `what_might_apply("the moment, in the operator's own words")` and read where - the rule now ranks. The change is measurable, so measure it, and say the - before and after when you report it. -3. **Then consider the dial.** `retrieval_surfaces` shows what is in force per - arm and whether the number is still calibrated; `tune_retrieval` moves it. - `reason` is required and has to say what you read, because it is what lets - the operator disagree with a number they did not choose. - -Reaching for `tune_retrieval` before opening a single record is the wrong move, -and it is the one that feels efficient. Worked example, measured on this -install: a rule granting a routine push scored 0.6515 and ranked 5th for the -moment it governed, behind three rules that *restrained* the same act. Every -percentile said "lower the floor" — and lowering it would have delivered those -three restraints and still not the rule. Rewriting the trigger to lead with the -symptom moved the same rule to 1st at 0.7130, ahead of all three. Only then was -the floor worth touching. - ## Building UI: the project's design system binds `enter_project` names the project's `design_system` when it has one. Before diff --git a/plugin/skills/using-scribe/missed-retrieval.md b/plugin/skills/using-scribe/missed-retrieval.md new file mode 100644 index 0000000..d65750e --- /dev/null +++ b/plugin/skills/using-scribe/missed-retrieval.md @@ -0,0 +1,46 @@ +# When a record doesn't reach the moment it should + +Part of the using-scribe skill. Read it when a rule should have governed a +moment and never arrived, when one arrives on every turn and never applies, +or before touching a retrieval floor. + +Retrieval misjudging is ordinary, and it is fixable — but only by whoever +notices. **Either direction counts:** a rule that should have governed a moment +and never arrived, and a rule that arrives on every turn and never applies. So +does **either noticer**: the operator saying *"that should have fired"*, and you +noticing it yourself — you reached for a rule nobody offered you, or you were +handed the same rule five times and set it aside five times. + +**Take it to the record first and the dial second.** A rule's `when_to_apply` +IS the text its similarity score is computed against, so when a rule misses a +moment it governs, the overwhelmingly likely cause is that its trigger does not +describe that moment in the words a session actually produces. Rewording one +trigger changes one rule's reach. Moving a floor changes what every record on +that surface does, and a floor cannot tell a badly-worded trigger from a +genuinely distant record — so one lowered to rescue a single rule admits +everything else that was sitting in the same band. + +1. **Read the refused records.** `retrieval_telemetry(days=N, + near_miss_samples=5)` names by id what each surface refused and by how much. + Open them with `get_rule` / `get_note`. This is the step that carries the + answer: the statistic says a record was close, and only the record says + whether it was *right*. +2. **Fix the trigger.** `update_rule(when_to_apply=...)`, written as the + symptom — what the session was doing or saying at the moment it needed this + rule — not the situation the rule belongs to. Then check that it worked: + `what_might_apply("the moment, in the operator's own words")` and read where + the rule now ranks. The change is measurable, so measure it, and say the + before and after when you report it. +3. **Then consider the dial.** `retrieval_surfaces` shows what is in force per + arm and whether the number is still calibrated; `tune_retrieval` moves it. + `reason` is required and has to say what you read, because it is what lets + the operator disagree with a number they did not choose. + +Reaching for `tune_retrieval` before opening a single record is the wrong move, +and it is the one that feels efficient. Worked example, measured on this +install: a rule granting a routine push scored 0.6515 and ranked 5th for the +moment it governed, behind three rules that *restrained* the same act. Every +percentile said "lower the floor" — and lowering it would have delivered those +three restraints and still not the rule. Rewriting the trigger to lead with the +symptom moved the same rule to 1st at 0.7130, ahead of all three. Only then was +the floor worth touching. diff --git a/plugin/skills/using-scribe/projects.md b/plugin/skills/using-scribe/projects.md new file mode 100644 index 0000000..4088f1d --- /dev/null +++ b/plugin/skills/using-scribe/projects.md @@ -0,0 +1,41 @@ +# Projects: binding a directory, and starting one + +Part of the using-scribe skill. Read it when a working directory maps to no +project, or when a new project is about to be created. + +## Binding a directory to a project + +**A directory does not have to be a git repo to have a project.** A repo is +bound by its remote (`list_repo_bindings` shows the bindings). Anything else — +a notes folder, a server's config directory, a scratch directory — is bound by +a `.scribe` file naming the project: + + {"instance": "https://scribe.example.com", "project_id": 2, "project": "Homelab"} + +`instance` is what makes the id trustworthy. An id means nothing on its own — +it is a different project on every Scribe — so a marker that has travelled to +another instance is ignored rather than followed to the wrong project. A bare +`2` also works when writing the file by hand. When work plainly belongs to a +project and the directory names none, offer to write the marker; +`list_projects` has the id. + +## Starting a project: decide what it inherits + +A project's inheritance is a **decision, not a default**. Before +`create_project`, ask the operator the two inception questions and pass the +answers — never create a project bare by default: + +- which **design system** its UI is built from (`list_design_systems`; or + none) → `design_system_id=` +- whether to **seed the standard starter Systems** so records can be tagged + from day one → `seed_systems=true|false` + +Rules are not an inception question: a global rule already applies to every +project, and a project's own rules are written on it as they come up. + +If `enter_project` returns an `inception` key, the project was never decided +(it inherits its defaults silently): raise that ask once, with the defaults it +carries, then `decide_project_inception(project_id, …)`. Existing projects +were stamped "legacy" and do not ask; any project can be re-decided. The +design-system and Systems tools still work one at a time — inception is the +moment they are decided together, and the record of why. diff --git a/plugin/skills/using-scribe/writing-records.md b/plugin/skills/using-scribe/writing-records.md new file mode 100644 index 0000000..9876571 --- /dev/null +++ b/plugin/skills/using-scribe/writing-records.md @@ -0,0 +1,121 @@ +# Writing a rule, a lesson, or a note that asserts a fact + +Part of the using-scribe skill. Read it before `create_rule`, +`create_project_rule`, `create_preference` or `create_lesson`; when a lesson +arrives that names the situation you are actually in; and before filling +`verify_with` or `expires_when` on a note. + +## Contents +- Where a new rule goes — its home, its trigger, what already covers the moment +- A lesson grows each time it proves itself +- A note that asserts a fact can carry its own check + +## Where a new rule goes + +A rule has one of two homes, and the home IS its reach: + +- **Global** — in a rulebook (`create_rule` into a topic). It applies in every + project, and reaches a session wherever the work matches it. A rulebook is a + *themed* grouping of general rules (e.g. a review checklist), not a list of + projects it binds — there is no subscribing a project to one. +- **Project** (`create_project_rule`) — anything specific to one project (its + files, paths, quirks). It reaches only that project's sessions. + +Names one project's specifics → project rule; a standard that holds wherever +the kind of work it describes happens → global. Never put project-specific +detail in a rulebook — it would reach every other project. A project that +departs from a global rule writes its own and links it with +`relate_rules(kind="overrides")`, which says why. A rule that turns out to be +in the wrong home — a project rule that holds everywhere, a global one only a +single project needs — moves with `move_rule`, which keeps its id, history, +areas and edges. Propose the move and make it on a yes. + +**Whichever home it gets, a rule needs `when_to_apply`.** It is the only thing +that decides whether the rule is ever seen: nothing is preloaded, so a rule +with no trigger is not a quiet rule, it is an unreachable one. Write the moment +in the words a session actually produces — the command, the error, the +half-formed ask — not the category it belongs to. + +**Before writing one, ask what already covers that moment.** +`what_might_apply("the moment you are about to write a record for")` — fifty +candidates and no bar, so an existing record cannot hide under a threshold the +way it can from `search`. When something already covers the moment, the reach +is that record: improve its trigger or its statement rather than standing a +second one beside it. Two records describing the same moment compete in one +ranked list against one budget, and the slot they take from each other is the +third candidate that would have said something different. Two records may +legitimately share a moment and say *different* things — a rule for what must +happen, a preference for how to report it. What this catches is the same thing +said twice at two strengths, which is worse than either alone: a session that +retrieves the softer copy has been told that binding guidance is optional. + +**First ask whether it's a rule at all.** A rule is prose you have to remember +and apply; Scribe's other entities are structure a tool can resolve and check. +Visual standards belong in a **design system**, not a rulebook — a token can be +inherited, resolved per mode, rendered to a stylesheet and diffed against code, +and none of that survives being written as a rule. A repeatable procedure is a +**process**; reusable code is a **snippet**. Reach for a rule when the thing +really is a standing instruction about how to work. + +Then ask what force it carries, by the question SKILL.md's reflex 2 asks: what +happens if someone doesn't do this? A standing instruction that merely costs +consistency is a **preference** (`create_preference`), and a transferable +insight that costs time is a **lesson** (`create_lesson`), keyed to the +situation it applies to so a later session meets it there. Both are first-class +outcomes of noticing something, not what's left when a rule proposal fails. + +## A lesson grows each time it proves itself + +When one arrives and the situation it names is the one you are actually in, you +are the single reader placed to tell whether its trigger is keyed right and +whether its claim covers what you are seeing. `update_lesson` takes what you +now know: another incident added to what taught it, the claim stated more +exactly, or — the edit worth most — a trigger re-keyed to the situation that +really fired. A lesson nobody reaches is seldom wrong; far more often it is +waiting in a situation nobody is in. One claim that has met the same failure +four times is worth more than four claims that each met it once, so when a +near-duplicate create hands back an existing id, that is the record to grow. + +## A note that asserts a fact can carry its own check + +**A few notes assert a FACT, and those can carry their own check.** +Supersession only fires once somebody has read a note and disagreed — which +is the case where it was already believed. A note asserting something about +*someone else's* software — what a service does on a duplicate upload, how a +forge numbers its CI runs, what an updater compares — can instead carry +`verify_with` (how to check it) and `expires_when` (the STATE that ends it: +"when the forge numbers runs per workflow", never "in six months"). +`notes_due_for_verification` lists them least-recently-confirmed first, with +never-checked at the top; `mark_note_verified` records what you found, and +`still_true=False` deliberately writes nothing — a note whose check failed +is wrong rather than in a state worth recording, so it keeps its place. + +**The test is one question: could this note become false without anyone +editing it?** If no, leave both fields empty. That is the normal case, and +an empty `verify_with` is the positive marker for "this is a decision, there +is nothing to go and check" — not an unfinished record. The sweep is only +worth reading while almost nothing is on it, so a check added out of +tidiness costs the whole surface, not just that note. + +**The sharper form of the same test: is the thing this note describes yours +to change?** If yes it is a decision — editing your own software is how it +changes, and you will know you did it. Measured against a real corpus, every +note that earned a check was about somebody ELSE's software: a signing +service, a forge, a hub, an SDK, a model, a dependency set. + +**Three that look like candidates and are not:** +- **Resume pointers and "current state" notes.** They go stale fastest of + anything, which is exactly why they tempt — but the cure is to update or + delete them, not to schedule a check. A sweep full of pointers is a sweep + nobody reads. +- **Measurements of your own system.** They go false because you changed + something, and you knew. A measurement earns a check only when what it + measures is outside your control. +- **A decision that RESTS on somebody else's behaviour.** The decision is + still a decision. Put the check on the note asserting the fact, and link + the decision to it. + +Not for tasks — a task's decay is its status, and a done issue records what +happened rather than asserting something that can go false. Not for snippets +either: `verify_snippet` compares the recorded location and code against the +repo, which is richer and already wired to drift detection. diff --git a/tests/helpers.py b/tests/helpers.py index f844fe7..ccbebc0 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -475,3 +475,19 @@ async def rule_row(rule_id: int): async with async_session() as s: return await s.get(Rule, rule_id) + + +def skill_text(name: str) -> str: + """Everything a bundled skill states: its SKILL.md, then each reference file. + + A skill keeps what matters on most turns in SKILL.md and moves what matters + at one moment into sibling files it links (Agent Skills progressive + disclosure, #4398). Both are the skill's own statement, so a test asking + "does this skill still say X" reads them together rather than pinning X to + whichever file it happens to sit in today. + """ + import pathlib + + folder = pathlib.Path(__file__).resolve().parents[1] / "plugin" / "skills" / name + refs = sorted(p for p in folder.glob("*.md") if p.name != "SKILL.md") + return "\n\n".join(p.read_text() for p in [folder / "SKILL.md", *refs]) diff --git a/tests/test_guidance_ownership.py b/tests/test_guidance_ownership.py index f1fc2fd..513b81b 100644 --- a/tests/test_guidance_ownership.py +++ b/tests/test_guidance_ownership.py @@ -48,6 +48,8 @@ import pathlib import re from typing import NamedTuple +from tests.helpers import skill_text + ROOT = pathlib.Path(__file__).resolve().parents[1] @@ -75,7 +77,7 @@ def delivered_surfaces() -> dict[str, str]: The one definition of "delivered" for this module: - `instructions` — the MCP server's `_INSTRUCTIONS` (every MCP client) - `docstrings` — the MCP tool modules (tool descriptions, every client) - - `skill:` — each bundled Agent Skill + - `skill:` — each bundled Agent Skill, reference files included - `static` — the Claude Code adapter's static session context - `commands` — the Claude Code adapter's slash commands - `live` — the live session context the server builds @@ -90,8 +92,10 @@ def delivered_surfaces() -> dict[str, str]: "commands": "".join(p.read_text() for p in sorted((ROOT / "plugin/commands").glob("*.md"))), "live": _live_session_context_source(), } + # A skill is SKILL.md plus the reference files it links (#4398): one + # owner, however many files it is split across. for skill in sorted((ROOT / "plugin/skills").glob("*/SKILL.md")): - surfaces[f"skill:{skill.parent.name}"] = skill.read_text() + surfaces[f"skill:{skill.parent.name}"] = skill_text(skill.parent.name) return {label: _norm(text) for label, text in surfaces.items()} diff --git a/tests/test_instruction_surfaces_agree.py b/tests/test_instruction_surfaces_agree.py index 41c9654..47ab15f 100644 --- a/tests/test_instruction_surfaces_agree.py +++ b/tests/test_instruction_surfaces_agree.py @@ -45,11 +45,12 @@ def _all_surfaces() -> list[tuple[str, str]]: the push channel to the operator installing the plugin, and telling a human what the hook does is not the same act as telling an agent it need not pull. The boundary is "does a session read this", which is skills (loaded by - description match), the hook-injected static context, and the MCP server's + description match, and each reference file a skill links is read on its + own, so it is its own surface — #4398), the hook-injected static context, and the MCP server's own instructions. """ found = [(str(p.relative_to(ROOT)), p.read_text()) - for p in (ROOT / "plugin" / "skills").rglob("SKILL.md")] + for p in (ROOT / "plugin" / "skills").rglob("*.md")] found += [(str(p.relative_to(ROOT)), p.read_text()) for p in (ROOT / "plugin" / "hooks").glob("*.md")] server = ROOT / "src" / "scribe" / "mcp" / "server.py" diff --git a/tests/test_retrieval_miss_route.py b/tests/test_retrieval_miss_route.py index 24acc22..77a924e 100644 --- a/tests/test_retrieval_miss_route.py +++ b/tests/test_retrieval_miss_route.py @@ -41,7 +41,6 @@ import pathlib import re ROOT = pathlib.Path(__file__).resolve().parents[1] -SKILL = ROOT / "plugin/skills/using-scribe/SKILL.md" # The three tools that shipped with no route to them. Named together because # the gap was all three at once, and a partial fix would leave the loop broken @@ -51,7 +50,10 @@ TUNING_TOOLS = ("retrieval_telemetry", "update_rule", "retrieval_surfaces", def _skill() -> str: - return SKILL.read_text() + # The route lives in missed-retrieval.md, a reference file of using-scribe + # (#4398); the skill is SKILL.md and its references read together. + from tests.helpers import skill_text + return skill_text("using-scribe") def _instructions() -> str: diff --git a/tests/test_skill_structure.py b/tests/test_skill_structure.py new file mode 100644 index 0000000..5d83377 --- /dev/null +++ b/tests/test_skill_structure.py @@ -0,0 +1,109 @@ +"""Every bundled skill keeps the shape Agent Skills can load well (#4398). + +WHY THIS EXISTS + +Anthropic's skill authoring guidance +(platform.claude.com/docs/en/agents-and-tools/agent-skills/best-practices): +keep the SKILL.md body under 500 lines, split into reference files as it +nears that, and link each reference file ONE level deep from SKILL.md — +Claude may only preview a file reached through another reference file. + +using-scribe reached 478 lines because every new practice lands there, the +same squeeze `_INSTRUCTIONS` was in before #4389. It was split: what matters +on most turns stays in SKILL.md, what matters at one moment (placing a rule, +a missed retrieval, starting a project) moved to files SKILL.md names with +the moment to open them. These tests keep that shape from quietly undoing +itself. + +WHAT IT PINS + +1. A BUDGET BELOW THE GUIDELINE. 500 is where loading degrades; the budget + sits under it so the next addition is a choice about what moves out, not + a squeeze past the line. +2. EVERY REFERENCE FILE IS LINKED FROM SKILL.md. A file nothing links is + never read — it states guidance that reaches nobody. +3. NO REFERENCE FILE LINKS ANOTHER. One level deep, per the guidance. +4. A LONG REFERENCE FILE OPENS WITH ITS CONTENTS, so a partial read still + shows what the file covers. +""" +from __future__ import annotations + +import pathlib +import re + +SKILLS = pathlib.Path(__file__).resolve().parents[1] / "plugin" / "skills" + +SKILL_LINE_BUDGET = 350 +TOC_AFTER_LINES = 100 +_MD_LINK = re.compile(r"\]\(([^)#\s]+\.md)\)") + + +def _skill_dirs() -> list[pathlib.Path]: + return sorted(p.parent for p in SKILLS.glob("*/SKILL.md")) + + +def _refs(folder: pathlib.Path) -> list[pathlib.Path]: + return sorted(p for p in folder.glob("*.md") if p.name != "SKILL.md") + + +def over_budget(texts: dict[str, str], budget: int) -> list[str]: + return [f"{k}: {len(t.splitlines())} lines" for k, t in texts.items() + if len(t.splitlines()) > budget] + + +def unlinked(skill_md: str, ref_names: list[str]) -> list[str]: + linked = set(_MD_LINK.findall(skill_md)) + return [n for n in ref_names if n not in linked] + + +def nested(refs: dict[str, str]) -> list[str]: + return [f"{name} → {target}" for name, text in refs.items() + for target in _MD_LINK.findall(text)] + + +def missing_contents(refs: dict[str, str], after: int) -> list[str]: + return [name for name, text in refs.items() + if len(text.splitlines()) > after and "## Contents" not in text] + + +def test_every_skill_md_fits_its_budget(): + found = over_budget({d.name: (d / "SKILL.md").read_text() for d in _skill_dirs()}, + SKILL_LINE_BUDGET) + assert not found, ( + f"SKILL.md over {SKILL_LINE_BUDGET} lines: {found}. Move what matters at " + f"one moment into a reference file SKILL.md links with that moment " + f"(#4398), rather than raising the budget toward 500." + ) + + +def test_every_reference_file_is_linked_from_skill_md(): + found = [f"{d.name}/{n}" for d in _skill_dirs() + for n in unlinked((d / "SKILL.md").read_text(), [p.name for p in _refs(d)])] + assert not found, ( + f"reference files no SKILL.md links: {found}. Nothing reads a file " + f"nothing names — link it from SKILL.md with the moment to open it." + ) + + +def test_reference_files_are_one_level_deep(): + found = [f"{d.name}/{x}" for d in _skill_dirs() + for x in nested({p.name: p.read_text() for p in _refs(d)})] + assert not found, ( + f"reference files linking other files: {found}. Link each from SKILL.md " + f"directly; a file reached through another may only be previewed." + ) + + +def test_long_reference_files_open_with_contents(): + found = [f"{d.name}/{n}" for d in _skill_dirs() + for n in missing_contents({p.name: p.read_text() for p in _refs(d)}, + TOC_AFTER_LINES)] + assert not found, f"reference files over {TOC_AFTER_LINES} lines with no '## Contents': {found}" + + +def test_the_guards_can_fail(): + """Rule 167: each guard bites on the failure it exists for.""" + assert over_budget({"s": "x\n" * 400}, SKILL_LINE_BUDGET) == ["s: 400 lines"] + assert unlinked("see [a](a.md)", ["a.md", "b.md"]) == ["b.md"] + assert nested({"a.md": "then [b](b.md)"}) == ["a.md → b.md"] + assert missing_contents({"a.md": "x\n" * 150}, TOC_AFTER_LINES) == ["a.md"] diff --git a/tests/test_verification_guidance_survives.py b/tests/test_verification_guidance_survives.py index aa7fc45..139440b 100644 --- a/tests/test_verification_guidance_survives.py +++ b/tests/test_verification_guidance_survives.py @@ -91,12 +91,11 @@ def test_the_skill_carries_the_test_a_writer_can_actually_apply(): read while deciding what to write. The one-question form has to be in the second place too, or the guidance only reaches callers who already opened the tool.""" - import pathlib + from tests.helpers import skill_text - skill = pathlib.Path(__file__).resolve().parents[1] / ( - "plugin/skills/using-scribe/SKILL.md" - ) - text = " ".join(skill.read_text().split()) + # Reference files included: the full statement sits in writing-records.md + # and SKILL.md keeps the pointer (#4398). + text = " ".join(skill_text("using-scribe").split()) assert "could this note become false without anyone editing it" in text.lower(), ( "the using-scribe skill no longer carries the one-question test. That " "question is what makes the distinction applicable rather than merely "