FabledScribe/tests/test_instruction_surfaces_agree.py

"""What the instruction surfaces must say that one owner per topic cannot enforce.

WHY THIS FILE IS SMALLER THAN IT WAS

Until milestone 410 this file pinned REDUNDANCY: every session-start surface —
the MCP `_INSTRUCTIONS`, the plugin's static context, the `using-scribe` skill —
had to restate how a rule reaches a session, what an empty session means, the
Systems reflex and the snippet-recording triggers. That was the design #2494
chose after #2497 (two surfaces disagreeing about who loads the rules) and
#2198 (every hook silently inert): say it everywhere, so no single failure
loses it.

Decision #4027 replaced that with ownership, and tests/test_guidance_ownership.py
now enforces it: every topic stated in full by exactly one owner, with a
one-line pointer in the server's index. The tests that required every surface
to repeat itself were retired there, because they enforced the very duplication
that drifted (#4022). What they protected is still protected:
  - "state how to ask for a rule" / "say an empty session is not an empty
    rulebook" → the rules topic: owned by using-scribe, indexed in
    `_INSTRUCTIONS` with both phrases;
  - the Systems reflex and the snippet-recording triggers → their topics'
    markers, required on their owners;
  - "never name the SessionStart push without stating the ask" (#2497's exact
    shape) → retired outright. Since milestone 394 the push carries no rules,
    so naming it can no longer imply rules were handled, and requiring every
    surface that mentions it to restate the ask would force a copy.

WHAT STAYS HERE

Properties of a CLAIM rather than of who owns a topic: the index fits the
fold; a surface that says rules bind also names what does not; using-scribe
still sends a session to the reporting-back skill.
"""
from __future__ import annotations

import pathlib

ROOT = pathlib.Path(__file__).resolve().parents[1]


def _all_surfaces() -> list[tuple[str, str]]:
    """(label, text) for every file a SESSION loads as instructions.

    Deliberately not every markdown file under plugin/: `README.md` describes
    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
    own instructions.
    """
    found = [(str(p.relative_to(ROOT)), p.read_text())
             for p in (ROOT / "plugin" / "skills").rglob("SKILL.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"
    found.append((str(server.relative_to(ROOT)), server.read_text()))
    return found


def _instructions_text() -> str:
    """The _INSTRUCTIONS literal from server.py, as the client would see it."""
    import re
    src = (ROOT / "src" / "scribe" / "mcp" / "server.py").read_text()
    match = re.search(r'_INSTRUCTIONS = """(.*?)"""', src, re.S)
    assert match, "server.py no longer defines _INSTRUCTIONS as a triple-quoted literal"
    return match.group(1).strip()


# Claude Code injects only the first ~2,048 characters of an MCP server's
# instructions and silently cuts the rest mid-word (#2562: observed live —
# the previous 20k-char version delivered ~10% of itself, and none of the
# Systems tagging guidance ever reached a session). 2,000 leaves margin.
INSTRUCTIONS_BUDGET = 2000


def test_instructions_fit_the_fold():
    text = _instructions_text()
    assert len(text) <= INSTRUCTIONS_BUDGET, (
        f"_INSTRUCTIONS is {len(text)} chars; Claude Code injects only ~2,048 "
        f"and silently cuts the rest (#2562). This block is an INDEX — state "
        f"the topic in full on its owner (a skill or the tool's docstring, "
        f"decision #4027) and give it at most a line here; see the comment "
        f"above _INSTRUCTIONS."
    )


# ── force: a surface that says rules bind must say what does not ────────
#
# Added with the preference kind (milestone 399). Before it, "rules bind" was
# the whole truth and every surface said so flatly. It is now half of one, and
# the half that is missing is the dangerous half to omit: a session reading
# only "rules bind" and then receiving a preference has been told, by the most
# authoritative surface it has, to treat it as binding.
#
# That failure is silent in both directions. Treating a preference as a rule
# produces a session that refuses to proceed over something the operator only
# preferred; and it removes the reason preferences exist, which is that they
# can be brought up to date rather than obeyed.
#
# Same bargain as every test in this file: STRUCTURE, not wording. A surface
# passes by mentioning the other kind at all, so the prose stays free.
#
# PINNED ON THE CLAIM, NOT THE WORD "bind". A bare substring also matches
# `bind_repo`, `list_repo_bindings` and the DNS-rebinding comment in
# server.py — so it would one day fail a skill that mentions repo binding and
# has nothing to do with force, which is rule 167's named failure: a guard
# raising a false alarm about the very thing it protects. These phrases are
# the ones that actually assert bindingness to a reader.
BINDING_CLAIMS = (
    "rules bind",
    "binding rules",
    "rules are binding",
    "treat every one as binding",
)
OTHER_KIND = "preference"


def test_a_surface_claiming_rules_bind_also_names_what_does_not():
    offenders = [
        label for label, text in _all_surfaces()
        if any(c in text.lower() for c in BINDING_CLAIMS)
        and OTHER_KIND not in text.lower()
    ]
    assert not offenders, (
        f"these surfaces tell a session that rules bind and never mention "
        f"preferences: {offenders}. A preference arrives through the same arms "
        f"and renders in the same line shape, so a surface that describes only "
        f"the binding kind is read as covering both — and the session treats "
        f"'how the operator likes this done' as something it may not proceed "
        f"past. Name the other kind, however briefly."
    )


# ── the index names every strength, and who keeps the middle one current ──
#
# Milestones 385 and 399 both end here, and for the same reason. The index is
# the one guidance surface EVERY MCP client receives — a client that reads no
# Agent Skills still gets it — so a strength the index omits does not exist for
# that session. Before the lesson kind, "rules bind; preferences guide" was the
# whole corpus; a session that meets a third line shape with no name for it
# falls back on the nearest one it has, which is the binding one.
#
# The currency half is the claim #3896 was written around: a preference is the
# one record a session updates ITSELF, without asking. Stated only in the
# skill, it reaches a session that loaded the skill — which is exactly the
# session least likely to need telling. In the index it reaches everyone.
#
# Words, not wording: each entry is a family, and a rewrite satisfies it by
# saying the thing in any of these ways.
INDEX_STRENGTHS = {
    "that rules bind": ("rules bind",),
    "that preferences guide": ("preferences guide",),
    "that lessons inform": ("lessons inform", "lessons are", "a lesson informs"),
    "that a preference is kept current": ("current",),
}


def test_the_index_names_all_three_strengths():
    text = " ".join(_instructions_text().split()).lower()
    missing = [claim for claim, family in INDEX_STRENGTHS.items()
               if not any(t in text for t in family)]
    assert not missing, (
        f"_INSTRUCTIONS no longer says {missing}. It is the only guidance every "
        f"client receives, and a record kind it does not name is one the session "
        f"has to guess the force of — which it does by reaching for the kind it "
        f"does know. Keep it to a clause; the full statement lives on "
        f"using-scribe (decision #4027)."
    )


def test_the_strength_guard_can_fail():
    """Rule 167. The guard has to be able to bite, and it bites on omission —
    the failure that actually happens, when a kind is added to the product and
    the index is left describing the corpus that came before it."""
    text = "rules bind; preferences guide and you keep them current."
    missing = [claim for claim, family in INDEX_STRENGTHS.items()
               if not any(t in text for t in family)]
    assert missing == ["that lessons inform"]


# ── Reporting back (milestone 409 step 3) ──────────────────────────────
#
# The reporting-back skill carries the shapes, but a skill only helps if it
# fires. The reflex that sends a session to it is stated in using-scribe; the
# server's index carries a REPORT line pointing at `placement`, and the in-band
# cue on update_task is the half that fires on its own in every client. The
# static context was a second plugin-side copy until milestone 410.

REPORT_REFLEX = "report back in a shape the operator can read"
REPORT_SURFACES = (
    ROOT / "plugin" / "skills" / "using-scribe" / "SKILL.md",
)


def test_the_reporting_reflex_is_stated_in_using_scribe():
    missing = []
    for path in REPORT_SURFACES:
        text = " ".join(path.read_text().split()).lower()
        if REPORT_REFLEX not in text or "reporting-back" not in text:
            missing.append(str(path.relative_to(ROOT)))
    assert not missing, (
        f"these surfaces no longer send a session to the reporting-back skill "
        f"({REPORT_REFLEX!r} plus the skill's name): {missing}. Without the "
        f"reflex the skill loads only when its description happens to match."
    )


def test_the_reporting_reflex_takes_placement_from_the_record():
    """The failure milestone 409 began from: a placement reconstructed from
    memory reads exactly like a real one when it is wrong."""
    for path in REPORT_SURFACES:
        text = " ".join(path.read_text().split())
        assert "`placement`" in text, f"{path.relative_to(ROOT)} no longer points at the placement block"