Only verified findings block; reviewer rules only in the agent prompt

docs/architecture.mdx

@@ -78,7 +78,7 @@ The reviewer sees one chunk. A finding can be wrong because of code in another p
    shall parses files lazily (`git grep` first). It marks hops that it resolved only by name with "(by name)". A keyword search runs at the same time. The search uses the YAML key path, `depends_on`, code spans of the statement and identifiers of the quote (code before docs). The search also covers files that have no grammar. For a diff, it also covers the file of the finding, except the lines that the verifier sees as the chunk: a diff shows only the changed hunks, and the setting that decides a finding (a variable, a client configuration) is frequently in another part of the same file.
 3. **Jev filters the excerpts** when there are more than 6 (one Noul per excerpt).
 4. **The `shall-verifier` agent** (no tools) answers confirmed / refuted / unknown. It must cite excerpt ids.
-5. **Code applies the verdict. Severity only goes down.** Refuted → code drops the finding only if the cited evidence exists. Unknown or confirmed-without-evidence → advisory. Confirmed → unchanged.
+5. **Code applies the verdict. Severity only goes down.** Refuted → code drops the finding only if the cited evidence exists. Unknown or confirmed-without-evidence → advisory. Confirmed → unchanged. A finding without a verdict (`--no-verify`, `review.verify: false`, or before `shall verify`) is advisory: only a confirmed finding blocks.
 
 Limits: shall resolves by imports and names, not by types. A language server or a SCIP index would resolve `this.partner.notify` by type. The graph cannot see dependency injection, reflection, and clients configured in YAML or environment variables.
 

docs/configuration.mdx

@@ -51,7 +51,7 @@ The organisation sets the settings in `shall.yaml`. A repository can change them
 | `selection.include_status` | `[approved, enforced]` | The statuses that reviews use. `--include-draft` adds drafts. |
 | `selection.facts` | `true` | Add known effects to the input of Jev and to the review |
 | `review.model` | `claude-sonnet-5` | The model of the reviewer and the verifier |
-| `review.verify` | `true` | Verify blocking findings against the repository before they block |
+| `review.verify` | `true` | Verify blocking findings against the repository. Only a verified finding blocks: with `false` or `--no-verify`, all findings are advisory. |
 | `review.cache` | `true` | Use the earlier answer of Claude again when its input is the same. `--no-cache` forces new calls. |
 | `review.max_comments` | `25` | The maximum number of comments for each review |
 | `conflicts.fail_threshold` | `0.75` | `conflicts --local` fails at this value or higher. |

plugins/shall/.claude-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "shall",
-  "version": "0.4.1",
+  "version": "0.4.2",
   "description": "shall engineering standards: author and maintain RFC-2119 standards, build the deterministic index, find which statements apply to a diff/spec/doc/task with Jev, review against them, lint whole files (scan), and check an agent's tool calls against them while it works (hooks).",
   "author": { "name": "gpavanello" },
   "keywords": ["standards", "rfc", "review", "governance", "jev", "typesafe"]

plugins/shall/skills/shall-review/references/in-session.md

@@ -9,7 +9,7 @@ Same pipeline as `shall review`, with the reviewer as an in-session agent.
    `.shall-out/pr/findings.agent.json` as `{"findings": [{"chunk": "c0", ...}]}`.
 4. `shall validate .shall-out/pr/findings.agent.json /tmp/pr.diff --selection .shall-out/pr/selection.json --out-dir .shall-out/pr/validated`
    (drops findings whose statement was not selected for the chunk or whose line is not changed,
-   dedupes, recomputes blocking, caps comments)
+   dedupes, caps comments; nothing blocks before step 5)
 5. `shall verify .shall-out/pr/validated/findings.json /tmp/pr.diff --selection .shall-out/pr/selection.json --out-dir .shall-out/pr`
    from the repository root (gathers evidence; refuted findings drop only with cited evidence,
    unknown ones stay advisory).

plugins/shall/tooling/src/shall/cli.py

@@ -355,8 +355,9 @@ def cmd_validate(args, cfg) -> ExitCode:
     raw = json.loads(Path(args.findings).read_text(encoding="utf-8"))
     raw = raw.get("findings", raw) if isinstance(raw, dict) else raw
     findings, dropped = validate_findings(corpus, artifact, selection, raw, int(cfg.get("review.max_comments")))
-    blocking = _write_outputs(cfg, Path(args.out_dir), artifact, selection, findings, dropped, args.format)
-    return ExitCode.BLOCKING if (blocking and args.fail_on_blocking) else ExitCode.OK
+    # nothing blocks before `shall verify` (JTOOL-0002.6): no --fail-on-blocking here
+    _write_outputs(cfg, Path(args.out_dir), artifact, selection, findings, dropped, args.format)
+    return ExitCode.OK
 
 
 def cmd_review(args, cfg) -> ExitCode:
@@ -368,6 +369,9 @@ def cmd_review(args, cfg) -> ExitCode:
     corpus = load_corpus(cfg)
     out_dir = Path(args.out_dir)
     verify = cfg.get("review.verify") and not args.no_verify
+    if args.fail_on_blocking and not verify:
+        print("shall review: verification is off, so no finding blocks and --fail-on-blocking "
+              "cannot fail (JTOOL-0002.6)", file=sys.stderr)
     # incremental re-review: unchanged chunks and unchanged evidence reuse earlier answers
     cache = answer_cache(cfg, enabled=not args.no_cache)
 
@@ -805,7 +809,8 @@ def artifact_args(s):
     s.add_argument("--format", choices=["markdown", "github"], default="markdown")
     s.add_argument("--fail-on-blocking", action="store_true", help=f"exit {ExitCode.BLOCKING} on blocking findings")
     s.add_argument("--dry-run", action="store_true", help="select only and write bundle.md, no agent call")
-    s.add_argument("--no-verify", action="store_true", help="skip verification of blocking findings")
+    s.add_argument("--no-verify", action="store_true",
+                   help="skip verification: no finding blocks, blocking findings stay advisory")
     s.set_defaults(fn=cmd_review)
 
     s = sub.add_parser("scan", help="lint whole files: secret scanner + Jev per file (warnings, no agent)")
@@ -842,7 +847,6 @@ def artifact_args(s):
     s.add_argument("--selection", required=True)
     s.add_argument("--out-dir", default=".shall-out")
     s.add_argument("--format", choices=["markdown", "github"], default="markdown")
-    s.add_argument("--fail-on-blocking", action="store_true")
     s.set_defaults(fn=cmd_validate)
 
     s = sub.add_parser("publish", help="post a review to a GitHub PR idempotently (via gh)")

plugins/shall/tooling/src/shall/findings.py

@@ -6,7 +6,7 @@
       -> validate_findings                             statement selected, line or quote found (JTOOL-0002.2)
     Finding           severity from the corpus         (corpus_blocking)
       -> Finding.with_verification(Verification)       (verify.py)
-    Finding.blocking  computed, never stored: verification can only lower it (JTOOL-0002.5, .6)
+    Finding.blocking  computed, never stored: only a `confirmed` verdict keeps it (JTOOL-0002.5, .6)
 
 Only this module makes a Finding from data (tests/test_boundaries.py checks it). `from_json` reads
 back a findings.json that shall itself wrote; agent output always goes through `validate_findings`.
@@ -103,12 +103,13 @@ class Finding:
 
     @property
     def blocking(self) -> bool:
-        """The corpus decides; a verification that is not `confirmed` can only lower it."""
-        return self.corpus_blocking and (self.verification is None or self.verification.verdict == "confirmed")
+        """The corpus decides, and only a `confirmed` verdict keeps it: a finding that was not
+        verified (`--no-verify`, `review.verify: false`, `shall validate`) is advisory (JTOOL-0002.6)."""
+        return self.corpus_blocking and self.verification is not None and self.verification.verdict == "confirmed"
 
     @property
     def downgraded(self) -> bool:
-        """Blocking by the corpus, advisory because its evidence was not seen."""
+        """Blocking by the corpus, advisory because no verifier confirmed it."""
         return self.corpus_blocking and not self.blocking
 
     def with_verification(self, verification: Verification) -> Finding:
@@ -125,7 +126,7 @@ def to_json(self) -> dict:
         if self.verification is not None:
             out["verification"] = self.verification.to_json()
         if self.downgraded:
-            out["downgraded"] = True  # evidence not seen: a comment, never a failed check
+            out["downgraded"] = True  # not confirmed: a comment, never a failed check
         return out
 
     @classmethod
@@ -151,7 +152,9 @@ def read_findings(path: Path) -> list[Finding]:
 
 
 def sort_key(f: Finding) -> tuple:
-    return (not f.blocking, f.path, f.line or 0, f.statement_id)
+    # corpus_blocking second: before verification nothing blocks, and the comment cap
+    # (validate_findings) must still keep the findings that can block
+    return (not f.blocking, not f.corpus_blocking, f.path, f.line or 0, f.statement_id)
 
 
 # ---------------------------------------------------------------- prompts and bundle
@@ -168,20 +171,14 @@ def render_statements(corpus: Corpus, ids: list[str]) -> str:
 
 
 def chunk_prompt(corpus: Corpus, chunk: Chunk, ids: list[str], facts: list[str] | None = None) -> str:
-    anchor = (
-        "Only lines marked '+' (added) can carry a finding; use their line number."
-        if chunk.kind == "diff" else
-        "Use the line number shown before '|'. Use line null for a finding about something missing from the whole document."
-    )
+    """The data of one review call. The instructions are only in agents/shall-reviewer.md (JTOOL-0004.2)."""
     return (
         f"<chunk kind=\"{chunk.kind}\" file=\"{chunk.path}\" language=\"{chunk.language}\">\n"
         f"{chunk.rendered}\n</chunk>\n\n"
         + (f"<known_effects>\nFacts found by code (effects registry and code graph) about what the "
            f"code in the chunk calls:\n" + "\n".join(f"- {x}" for x in facts) + "\n</known_effects>\n\n"
            if facts else "")
-        + f"<standards>\n{render_statements(corpus, ids)}\n</standards>\n\n"
-        f"{anchor}\nReturn only violations of the standards listed above. "
-        "Return an empty findings list when the chunk complies."
+        + f"<standards>\n{render_statements(corpus, ids)}\n</standards>"
     )
 
 
@@ -191,10 +188,9 @@ def write_bundle(corpus: Corpus, artifact: Artifact, selection: dict, out: Path)
     parts = [
         f"# shall review bundle — {artifact.source}",
         "",
-        "For each chunk, check only the listed standards. Write all findings to one JSON file",
-        "with the shape `{\"findings\": [{\"chunk\": \"c0\", \"statement_id\": ..., \"line\": ..., "
-        "\"quote\": ..., \"message\": ..., \"suggestion\": ...}]}`, then run",
-        f"`shall validate <findings.json> --selection <selection.json> {artifact.source}`.",
+        "Give each chunk section below to the `shall-reviewer` agent. Collect its findings in one JSON",
+        "file as `{\"findings\": [{\"chunk\": \"c0\", ...}]}`, then run `shall validate` and `shall verify`",
+        "(skill shall-review, references/in-session.md). Only a verified finding blocks.",
         "",
     ]
     for c in selection["chunks"]:
@@ -283,7 +279,7 @@ def validate_findings(corpus: Corpus, artifact: Artifact, selection: dict, raw:
 def verification_line(f: Finding) -> str:
     v = f.verification
     if v is None:
-        return ""
+        return "_Not verified — not blocking._" if f.corpus_blocking else ""
     where = f" ({', '.join(v.evidence)})" if v.evidence else ""
     if v.verdict == "confirmed":
         return f"_Verified{where}:_ {v.reason}"
@@ -317,8 +313,8 @@ def render_markdown(artifact: Artifact, selection: dict, findings: list[Finding]
         out.append(f"  {f.message}")
         if f.suggestion:
             out.append(f"  _Suggestion:_ {f.suggestion}")
-        if f.verification:
-            out.append(f"  {verification_line(f)}")
+        if note := verification_line(f):
+            out.append(f"  {note}")
     return "\n".join(out).rstrip() + "\n"
 
 
@@ -340,8 +336,8 @@ def comment_body(f: Finding, key: str | None = None, lhash: str | None = None) -
     body = f"{tag} · **{f.statement_id}** {f.title} ({f.level})\n\n{f.message}"
     if f.suggestion:
         body += f"\n\n**Suggestion:** {f.suggestion}"
-    if f.verification:
-        body += f"\n\n{verification_line(f)}"
+    if note := verification_line(f):
+        body += f"\n\n{note}"
     body += f"\n\n> {f.statement_text}\n\n<sub>Standard: `{f.source}`</sub>"
     if key:
         body += f"\n\n<!-- shall:finding key={key} line={lhash} -->"

plugins/shall/tooling/src/shall/verify.py

@@ -2,7 +2,8 @@
 
 The reviewer sees one chunk. A finding such as "claude runs with tools" can be wrong because
 another file adds `--tools ""`. Which findings are verified is decided by code, not by the
-reviewer: every blocking finding, plus advisory findings that declare `depends_on`.
+reviewer: every finding the corpus marks blocking (it blocks only when confirmed), plus advisory
+findings that declare `depends_on`.
 
     terms (depends_on + identifiers of the quote)          code
       -> excerpts from the repository (git grep)           code
@@ -88,7 +89,7 @@ class Claim:
 
 
 def needs_verification(finding: Finding) -> bool:
-    return finding.blocking or bool(finding.depends_on)
+    return finding.corpus_blocking or bool(finding.depends_on)
 
 
 CODE_SPAN_RE = re.compile(r"`([^`]+)`")

plugins/shall/tooling/tests/test_boundaries.py

@@ -82,6 +82,22 @@ def test_findings_schema_matches_raw_finding():
                                        "agent returns, RawFinding is how code reads it.")
 
 
+def test_review_instructions_live_only_in_the_agent_prompt():
+    """JTOOL-0004.2: code sends the reviewer data only; its rules are in agents/shall-reviewer.md,
+    which the in-session agent and CI both use. A second copy in code drifts from the first."""
+    from shall.artifact import build_artifact
+    from shall.corpus import load_corpus
+    from shall.findings import chunk_prompt
+    corpus = load_corpus(load_config(str(REPO / "shall.yaml")))
+    diff = "diff --git a/a.py b/a.py\n--- a/a.py\n+++ b/a.py\n@@ -0,0 +1 @@\n+print(1)\n"
+    for name, raw, kind in (("a.diff", diff, "diff"), ("a.md", "# Title\n\nText.\n", "doc")):
+        chunk = build_artifact(name, raw, kind, 4000).chunks[0]
+        prompt = chunk_prompt(corpus, chunk, ["JRFC-0003.1"], ["a fact"])
+        rest = re.sub(r"<(chunk|known_effects|standards)\b.*?</\1>", "", prompt, flags=re.S).strip()
+        assert not rest, (f"chunk_prompt ({kind}) has text outside its data tags: {rest!r}. Put review "
+                          "rules in plugins/shall/agents/shall-reviewer.md (JTOOL-0004.2)")
+
+
 def test_every_claude_call_is_tool_less():
     cfg = type("Cfg", (), {"get": lambda self, key: ["claude", "-p"]})()
     for schema in (None, {"type": "object"}):

plugins/shall/tooling/tests/test_verify.py

@@ -105,6 +105,25 @@ def test_confirmed_without_existing_evidence_does_not_block():
     assert kept.verification is not None and kept.verification.verdict == "unknown"
 
 
+def test_a_finding_that_was_not_verified_never_blocks():
+    """JTOOL-0002.6: `--no-verify`, `review.verify: false` and `shall validate` give no verdict,
+    so the exit code, the GitHub review and the Stop hook see an advisory finding."""
+    from shall.findings import Finding, render_github, render_markdown, sort_key
+    f = finding()
+    assert f.blocking is False and f.downgraded is True
+    assert needs_verification(f)  # the corpus still decides what verification looks at
+    data = f.to_json()
+    assert data["blocking"] is False and data["corpus_blocking"] is True
+    assert Finding.from_json(data).blocking is False  # findings.json read back (Stop hook) stays advisory
+    artifact = build_artifact("shall.yaml", "jev:\n  command: [claude, -p]\n", "code", 4000)
+    selection = {"applicable": [], "model": "m", "strategy": "flat"}
+    assert render_github(artifact, selection, [f], [])["event"] == "COMMENT"
+    assert "Not verified — not blocking" in render_markdown(artifact, selection, [f], [])
+    # before verification nothing blocks: the comment cap must still keep what can block
+    advisory = finding(corpus_blocking=False, path="a.yaml")
+    assert sorted([advisory, f], key=sort_key) == [f, advisory]
+
+
 def test_missing_verdict_counts_as_unknown():
     kept, _ = apply_verdict(finding(), None, [])
     assert kept is not None and kept.blocking is False