diff --git a/tests/test_guidance_ownership.py b/tests/test_guidance_ownership.py new file mode 100644 index 0000000..e92d43a --- /dev/null +++ b/tests/test_guidance_ownership.py @@ -0,0 +1,198 @@ +"""Every piece of agent guidance has one owner — and none of it is lost on the way there. + +WHY THIS EXISTS (milestone 410, decision #4027) + +Scribe's guidance to agents was written up to five times over: the MCP +`_INSTRUCTIONS`, the plugin's static session context, the live session context +the server builds, the `using-scribe` skill, and tool docstrings. An earlier +design (#2494) made that deliberate — insurance against any one surface failing +silently — and the copies drifted apart instead (#2497, #4022). + +Decision #4027 replaced the redundancy with ownership: the server orients, the +skills hold the depth, and each client adapter holds only its own timing and +conventions. This module is the registry that decision is enforced from. + +WHAT IT PINS NOW, AND WHAT COMES LATER + +Step 1 — the LOSS GUARD. Every topic below must still be stated on at least one +surface a session actually receives. Milestone 410's later steps delete copies; +this is what stops the last copy of a topic going with them. + +Step 6 will add the OWNERSHIP guard: a topic's full statement on its `owner` +and nowhere else. The `owner` column is recorded here from the start so the +registry is written once, but nothing asserts it yet — during the moves a +topic is legitimately in several places at once. + +MARKERS ARE PHRASES, NOT WORDS + +A topic is "stated" when ALL of its markers appear on one surface — so a +topic's markers must travel together, and a lone common word never counts +(BINDING_CLAIMS in test_instruction_surfaces_agree explains the false alarm a +bare word raises). Tool names are the preferred marker: they change only when +the tool does. Reword a topic deliberately and update its markers in the same +commit; the failure message names which marker went missing where. +""" +from __future__ import annotations + +import pathlib +import re +from typing import NamedTuple + +ROOT = pathlib.Path(__file__).resolve().parents[1] + + +def _norm(text: str) -> str: + # Whitespace-flattened and lowercased: prose is hard-wrapped, so a marker + # can straddle a line break without the guidance having changed. + return " ".join(text.split()).lower() + + +def _live_session_context_source() -> str: + """The source of build_session_context — the prose it emits lives there. + + Read as text rather than called: the function needs a database, and what + this module checks is what the product SAYS, which is the literal strings. + """ + src = (ROOT / "src/scribe/services/plugin_context.py").read_text() + start = src.index("async def build_session_context") + nxt = re.search(r"\n(?:async def|def) ", src[start + 1:]) + return src[start:start + 1 + nxt.start()] if nxt else src[start:] + + +def delivered_surfaces() -> dict[str, str]: + """Every surface a session receives as guidance, by label. + + The one definition of "delivered" for this module and its successors: + - `instructions` — the MCP server's `_INSTRUCTIONS` (every MCP client) + - `docstrings` — the MCP tool modules (tool descriptions, every client) + - `skill:` — each bundled Agent Skill + - `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 + """ + server = (ROOT / "src/scribe/mcp/server.py").read_text() + match = re.search(r'_INSTRUCTIONS = """(.*?)"""', server, re.S) + assert match, "server.py no longer defines _INSTRUCTIONS as a triple-quoted literal" + surfaces = { + "instructions": match.group(1), + "docstrings": "".join(p.read_text() for p in sorted((ROOT / "src/scribe/mcp/tools").glob("*.py"))), + "static": (ROOT / "plugin/hooks/scribe_static_context.md").read_text(), + "commands": "".join(p.read_text() for p in sorted((ROOT / "plugin/commands").glob("*.md"))), + "live": _live_session_context_source(), + } + for skill in sorted((ROOT / "plugin/skills").glob("*/SKILL.md")): + surfaces[f"skill:{skill.parent.name}"] = skill.read_text() + return {label: _norm(text) for label, text in surfaces.items()} + + +class Topic(NamedTuple): + key: str + owner: str # a label from delivered_surfaces(); asserted from step 6 + markers: tuple[str, ...] + + +# The ownership map from milestone 410's body, one row per topic, plus the +# topics once guarded as "displaced from _INSTRUCTIONS" (#2562), folded in so +# there is one list. Retired topics (the surface-precedence tiebreaker) are +# absent on purpose: nothing has to keep saying them. +TOPICS: tuple[Topic, ...] = ( + # ── the working reflexes — owned by the using-scribe skill ── + Topic("scribe is the system of record; keep one copy", "skill:using-scribe", ("one copy",)), + Topic("orient: enter the project, check repo bindings", "skill:using-scribe", + ("enter_project", "list_repo_bindings")), + Topic("rules are retrieved; ask before a consequential act", "skill:using-scribe", + ('content_type="rule"', "nothing matched")), + Topic("rules bind, preferences guide and are kept current", "skill:using-scribe", + ("preference", "update_preference")), + Topic("recall before acting", "skill:using-scribe", ("recall before acting",)), + Topic("stay inside the active project's scope", "skill:using-scribe", + ("stay inside the active project", "cross-project")), + Topic("record as you go; honest status; fixes are issues", "skill:using-scribe", + ("add_task_log", "in_progress", 'kind="issue"')), + Topic("an id exists only once a create returns it", "skill:using-scribe", + ("exists only once a create", "{{ref:")), + Topic("tag records to systems as you write", "skill:using-scribe", ("system_ids", "create_system")), + Topic("the project's design system binds ui", "skill:using-scribe", ("resolve_design_system",)), + Topic("name the record, never just its number", "skill:using-scribe", ("name the record",)), + Topic("project inception is a decision", "skill:using-scribe", ("decide_project_inception",)), + Topic("where a new rule goes, and its trigger", "skill:using-scribe", + ("create_project_rule", "when_to_apply")), + Topic("a rule vs the other entities", "skill:using-scribe", ("standing instruction",)), + Topic("reference notes update in place; dev-logs don't", "skill:using-scribe", ("reference note",)), + # ── process arcs — owned by their skills ── + Topic("plan in a milestone, steps created together", "skill:writing-plans", ("start_planning", "{{ref:")), + Topic("reuse recorded shapes; record at first build", "skill:reusing-code", + ("create_snippet", "when_to_use", "first build", "second copy")), + Topic("report back where the work stands", "skill:reporting-back", ("reporting-back", "placement")), + # ── per-tool contracts and in-band behaviour — owned by the server ── + Topic("closing a task cues the report", "docstrings", ("report_back",)), + Topic("a note that asserts a fact carries its check", "docstrings", ("verify_with", "expires_when")), + Topic("supersession demotes, never hides", "docstrings", ("supersedes",)), + Topic("deletes are recoverable from the trash", "docstrings", ("deleted_batch_id",)), + Topic("creates are duplicate-gated", "docstrings", ("near-duplicate",)), + Topic("shared records are another user's suggestion", "docstrings", ("shared: true",)), + Topic("stored processes are followed verbatim", "docstrings", ("stored processes", "verbatim")), + Topic("a project is never guessed", "docstrings", ("never guessing a project",)), + Topic("an unbound repo gets a bind hint", "live", ("bind_repo",)), + # ── the Claude Code adapter's own conventions ── + Topic("compact at clean seams", "static", ("/compact",)), + Topic("stored processes sync into local skills", "commands", ("scribe-proc-",)), + Topic("say so when scribe's tools are unavailable", "static", ("tools are unavailable",)), +) + + +def missing_topics(topics, surfaces: dict[str, str]) -> list[str]: + """Topics no single surface states in full — with the nearest miss named. + + Pure, so the guard's ability to fail is itself testable. + """ + missing = [] + for topic in topics: + markers = [m.lower() for m in topic.markers] + if any(all(m in text for m in markers) for text in surfaces.values()): + continue + partial = { + label: [m for m in markers if m not in text] + for label, text in surfaces.items() + if any(m in text for m in markers) + } + missing.append(f"{topic.key!r} — markers {topic.markers}; nearest: {partial or 'nowhere'}") + return missing + + +def test_no_guidance_topic_has_fallen_off_every_surface(): + missing = missing_topics(TOPICS, delivered_surfaces()) + assert not missing, ( + "these guidance topics are no longer stated in full on ANY delivered " + "surface:\n " + "\n ".join(missing) + "\nMilestone 410 moves guidance " + "to one owner per topic (decision #4027); a move that deletes a copy " + "must leave the topic stated on its owner. If the topic was reworded on " + "purpose, update its markers here in the same commit." + ) + + +def test_every_owner_is_a_surface_that_exists(): + labels = set(delivered_surfaces()) + unknown = [(t.key, t.owner) for t in TOPICS if t.owner not in labels] + assert not unknown, f"owners that name no delivered surface: {unknown}" + + +def test_topic_keys_are_unique(): + keys = [t.key for t in TOPICS] + assert len(keys) == len(set(keys)) + + +def test_the_loss_guard_can_fail(): + """Rule 167: a guard that cannot fail protects nothing. + + A topic whose markers are split across two surfaces is NOT stated — the + phrases have to travel together — and one whose marker is nowhere is + reported with 'nowhere'. + """ + surfaces = {"a": "enter_project here", "b": "list_repo_bindings there"} + split = Topic("split", "a", ("enter_project", "list_repo_bindings")) + absent = Topic("absent", "a", ("no such phrase",)) + whole = Topic("whole", "a", ("enter_project",)) + reported = missing_topics((split, absent, whole), surfaces) + assert len(reported) == 2 + assert reported[0].startswith("'split'") and "nowhere" in reported[1] diff --git a/tests/test_instruction_surfaces_agree.py b/tests/test_instruction_surfaces_agree.py index ce02051..c3c29ef 100644 --- a/tests/test_instruction_surfaces_agree.py +++ b/tests/test_instruction_surfaces_agree.py @@ -211,50 +211,10 @@ def test_floor_names_the_snippet_recording_triggers(): ) -# Topics displaced from _INSTRUCTIONS when it was cut to fit the fold. Each -# must remain stated on at least one DELIVERED surface: a tool docstring -# (arrives with the tool schema), the plugin static context (always arrives), -# or a bundled skill (arrives on trigger match). Keyed by a phrase distinctive -# enough that its disappearance means the guidance is gone, not reworded — -# update the phrase alongside a deliberate rewording. -DISPLACED_TOPICS = { - "supersedes": "supersedes", - "trash is recoverable": "deleted_batch_id", - "duplicate gate": "duplicate", - "systems tag-as-you-write": "system_ids", - "reference note vs dev-log": "reference note", - "work-logs over body rewrites": "add_task_log", - "rule homes / altitude": "create_project_rule", - "rules vs other entities": "standing instruction", - "shared records are suggestions": "shared", - "processes run verbatim": "verbatim", - "snippet reuse reflex": "when_to_use", - "compaction at seams": "compact", - "plans are milestones": "start_planning", - "scope to the entered project": "cross-project", - "project bootstrap needs confirmation": "never guessing a project", -} - - -def test_displaced_topics_live_on_a_delivered_surface(): - corpus = "" - for p in (ROOT / "src" / "scribe" / "mcp" / "tools").glob("*.py"): - corpus += p.read_text() - corpus += (ROOT / "plugin" / "hooks" / "scribe_static_context.md").read_text() - for p in (ROOT / "plugin" / "skills").rglob("SKILL.md"): - corpus += p.read_text() - corpus = corpus.lower() - missing = [ - f"{topic} (phrase: {phrase!r})" - for topic, phrase in DISPLACED_TOPICS.items() - if phrase.lower() not in corpus - ] - assert not missing, ( - f"guidance displaced from _INSTRUCTIONS has fallen off every delivered " - f"surface (tool docstrings / static context / skills): {missing}. It " - f"was cut from _INSTRUCTIONS deliberately (#2562) on the premise it " - f"lives elsewhere — restore it somewhere that delivers." - ) +# The "displaced from _INSTRUCTIONS" topics (#2562) used to be listed here with +# their own delivered-surface check. They are folded into the one topic registry +# in tests/test_guidance_ownership.py (milestone 410), which covers every topic +# of the ownership map and defines "delivered surface" once. def test_no_surface_names_the_push_without_stating_the_ask():