From 265755ba531246efce08dfd053565bab3a265c44 Mon Sep 17 00:00:00 2001 From: Ouroboros <311266734+ouroboros-agent@users.noreply.github.com> Date: Wed, 2 Sep 2026 16:24:28 +0000 Subject: [PATCH] scripts(v7next): a hook-resolution error says hook:, not release: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hook resolution runs for every `done` row in BOTH modes — it is a property of a shipped row, not of the --release invocation — but its five messages were prefixed `release:` and the docstring still claimed «Outside --release hooks stay free prose». A reader of a default-mode run was pointed at a switch that had nothing to do with the failure. Prefix is now `hook:`; the two genuine release-bar messages (pending-decision, status != done) keep `release:`. The docstring says what actually gates the resolution (`done`, not the mode), and the manifest's Notes now state the same rule, so the code comment that cites them is true. Red-first: the new parametrized pin drives four hook shapes (prose-only, missing file, `tests/../` escape, bogus `::nodeid`) through the DEFAULT mode and asserts no message claims the release bar. On the pre-fix shape 4 failed, 19 passed; after the rename 23 passed. The fifth message (unparseable hook file) is not driven — it needs a planted syntax-error file — and is renamed with the others. No behaviour change: the same rows are red in the same modes. --- ADOPTION_v7next.md | 4 ++++ scripts/v7next_adoption.py | 25 ++++++++++++++----------- tests/test_v7next_adoption.py | 18 ++++++++++++++++++ 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/ADOPTION_v7next.md b/ADOPTION_v7next.md index 01b51ccda..26ade95a3 100644 --- a/ADOPTION_v7next.md +++ b/ADOPTION_v7next.md @@ -46,6 +46,10 @@ Schema (fixed; one row per artifact-level delta family, never per commit): - `verification hook` — the suite/checker that proves the row when it lands (suites named from the frozen reference arrive with their domain transplant, plan §5.3 step 4; hooks marked "(new)" are named now, built in their phase). + Once the row is `done` the validator resolves EVERY token of its hook — path + and `::nodeid` half alike — in both modes, because a resolvable hook is a + property of a shipped row and not of the `--release` invocation. Until then + the hook may stay free prose naming the suite the work will land in. | id | kind | what | disposition | status | phase | verification hook | |---|---|---|---|---|---|---| diff --git a/scripts/v7next_adoption.py b/scripts/v7next_adoption.py index 80028d114..49477716a 100644 --- a/scripts/v7next_adoption.py +++ b/scripts/v7next_adoption.py @@ -260,9 +260,9 @@ def validate(rows: list[dict[str, str]], release: bool) -> list[str]: # a genuine disclosure on a shipped row stays sayable. for r in rows: errors.extend(_honesty_errors(r)) - # Hook resolution follows the rule the manifest already states in its - # Notes ("for a `done` row the validator resolves every token"): it is - # a property of a shipped row, not of the release invocation. + # Hook resolution is a property of a shipped row, not of the release + # invocation, so it runs in both modes and its messages say `hook:`. + # The manifest's own Notes state the same rule for its readers. if r["status"] == "done": errors.extend(_hook_resolution_errors(r)) # Phase pinning of the required inventory. @@ -346,29 +346,32 @@ def _hook_nodeid_errors(row: dict[str, str], hook: str) -> list[str]: try: defined = _defined_names(path) except SyntaxError as exc: # unparseable file: say so, do not pass it - errors.append(f"release: {row['id']} hook file {rel} does not parse ({exc})") + errors.append(f"hook: {row['id']} hook file {rel} does not parse ({exc})") continue for part in tail.split("::"): if part and part not in defined: - errors.append(f"release: {row['id']} hook names {rel}::{part}, " + errors.append(f"hook: {row['id']} hook names {rel}::{part}, " f"which {rel} does not define") return errors def _hook_resolution_errors(row: dict[str, str]) -> list[str]: - """Release-bar hook contract (F0 review rounds 1-4): a shipped row's + """Shipped-row hook contract (F0 review rounds 1-4): a shipped row's verification hook must RESOLVE — prose alone cannot pass. At least one repo-path reference must be present, EVERY referenced token must exist (any extension — a smuggled bogus reference next to a valid one is an error, not ignored), and the path must stay inside its top directory - (`tests/../x` traversal is rejected). Outside --release hooks stay free - prose (they name future suites while the work is pending).""" + (`tests/../x` traversal is rejected). This runs for every `done` row in + BOTH modes — it is a property of a shipped row, not of the --release + invocation — so the messages are prefixed `hook:`, not `release:`. A row + that is not yet `done` keeps a free-prose hook, naming the suite the work + will land in.""" hook = row["verification hook"] paths = _HOOK_PATH_RE.findall(hook.replace("\\|", "|")) errors: list[str] = [] if not paths: errors.append( - f"release: {row['id']} hook has no resolvable repo-path reference " + f"hook: {row['id']} hook has no resolvable repo-path reference " "(tests/, scripts/ or docs/ file) — prose-only hooks cannot ship") for p in paths: top = p.split("/", 1)[0] @@ -378,9 +381,9 @@ def _hook_resolution_errors(row: dict[str, str]) -> list[str]: # separators (round 5: the "/"-suffix check broke on Windows). inside = candidate == top_root or top_root in candidate.parents if ".." in p.split("/") or not inside: - errors.append(f"release: {row['id']} hook path escapes {top}/: {p}") + errors.append(f"hook: {row['id']} hook path escapes {top}/: {p}") elif not candidate.is_file(): - errors.append(f"release: {row['id']} hook references missing file {p}") + errors.append(f"hook: {row['id']} hook references missing file {p}") errors.extend(_hook_nodeid_errors(row, hook)) return errors diff --git a/tests/test_v7next_adoption.py b/tests/test_v7next_adoption.py index 54db2d05a..868811b28 100644 --- a/tests/test_v7next_adoption.py +++ b/tests/test_v7next_adoption.py @@ -96,6 +96,24 @@ def test_a_bogus_hook_nodeid_turns_the_bar_red(rows): assert any("test_no_such_pin_was_ever_written" in e for e in errors), errors +@pytest.mark.parametrize("hook", [ + "the suites this row moved bytes in", # prose only + "tests/test_no_such_suite_was_ever_written.py", # missing file + "tests/../scripts/v7next_adoption.py", # escapes tests/ + "tests/test_smoke.py::test_no_such_pin_was_ever_written", # bogus nodeid +]) +def test_a_hook_error_does_not_claim_the_release_bar(rows, hook): + """Hook resolution runs for every ``done`` row in BOTH modes — it is a + property of a shipped row, not of the ``--release`` invocation. A message + reported in the default mode must therefore not say ``release:``, or the + reader is told to look for a switch that has nothing to do with it.""" + victim = _first_done(rows) + errors = validate(_mutate(rows, victim["id"], **{"verification hook": hook}), + release=False) + assert errors, "the hook shape was accepted in the default mode" + assert not [e for e in errors if e.startswith("release:")], errors + + def test_a_hook_nodeid_that_names_a_real_test_stays_green(rows): """The AST read must accept what the manifest legitimately names, including a data carrier (``tests/_shared.py::SETTINGS_WRITERS``) — a hook may point