FabledScribe/tests/test_instruction_surfaces_agree.py

"""The instruction surfaces must agree that the agent pulls the rules itself.

WHY THIS EXISTS

Rule #119 makes the instruction surfaces the SPECIFICATION for product
behaviour — there is no other place the "load the operator's rules" obligation
is written down, and no code path enforces it. So a surface that states it
differently isn't a documentation slip; it is the product behaving differently.

That happened (#2497). `_INSTRUCTIONS` said the SessionStart hook "is the
bridge" for getting rules into a session, while the `using-scribe` skill said to
pull them yourself and treat any push as a bonus. An agent weighting the first
would reasonably skip the pull.

#2198 is the case where that is wrong: every plugin hook was silently inert for
an extended period, and nothing announced it. An agent trusting the push would
have run with no binding rules and no signal — while those rules govern branch,
commit, push and other hard-to-reverse actions.

The asymmetry is the whole argument, and it is what these tests pin: pulling
when a push also arrived costs one redundant call; not pulling when the push
never came costs the operator's rules entirely.

WHAT THIS DOES NOT DO

It cannot tell whether two surfaces contradict each other in prose generally —
that needs a reader. It pins the one instruction whose absence is known to be
load-bearing, and the specific shape the #2497 defect took: naming the push
without also stating the pull.
"""
from __future__ import annotations

import pathlib

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

# The pull instruction, however a surface phrases the surrounding prose.
PULL = "list_always_on_rules"

# Surfaces a session loads before substantive work. Hand-written because
# "is this a session-start surface?" is an editorial fact, not a derivable one —
# but each entry is asserted to EXIST, so a move or rename fails loudly here
# instead of quietly dropping that surface from the check.
SESSION_START_SURFACES = (
    ROOT / "src" / "scribe" / "mcp" / "server.py",
    ROOT / "plugin" / "hooks" / "scribe_static_context.md",
    ROOT / "plugin" / "skills" / "using-scribe" / "SKILL.md",
)


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 test_every_session_start_surface_states_the_pull():
    missing = []
    for path in SESSION_START_SURFACES:
        assert path.exists(), (
            f"{path.relative_to(ROOT)} is gone — it was one of the surfaces "
            f"carrying the load-the-rules instruction. If it moved, update "
            f"SESSION_START_SURFACES; if it was retired, check the instruction "
            f"still lives somewhere a fresh session reads."
        )
        if PULL not in path.read_text():
            missing.append(str(path.relative_to(ROOT)))
    assert not missing, (
        f"these surfaces no longer tell the agent to call {PULL}(): {missing}. "
        f"The rules are pull-only and the push is best-effort, so a surface "
        f"that omits this leaves a session bound by nothing (#2198, #2497)."
    )


def test_no_surface_names_the_push_without_stating_the_pull():
    """The exact shape #2497 took.

    Mentioning the SessionStart hook is fine and often useful. Mentioning it
    *instead of* the pull is the defect: it reads as "this is handled", and the
    surface that says so is the one an agent has least reason to doubt.
    """
    offenders = [
        label for label, text in _all_surfaces()
        if "SessionStart" in text and PULL not in text
    ]
    assert not offenders, (
        f"these surfaces describe the SessionStart push but never state the "
        f"explicit pull: {offenders}. The push is a delivery optimisation, not "
        f"the bridge — it can be absent without saying so. Name it if it helps, "
        f"but say to call {PULL}() regardless."
    )