review: add the per-lens consumer payload seam

.changeset/review-lens-payload-seam.md

@@ -0,0 +1,28 @@
+---
+"review": minor
+---
+
+Add the per-lens consumer payload seam. A consuming repo may now define
+`.github/aw/review/lenses/<lens>.md` for any of the eleven specialist lenses, plus
+`lenses/correctness.md` for the always-on `correctness-reviewer`; each file is
+runtime-imported (optional form) into a new "Repo-specific rules and hunts" section
+of the matching reviewer prompt, carrying that repo's surface-specific rules and
+extra tri-state hunts. Lens names stay generic and shared; only payloads vary per
+repo. Behavior-neutral for every current consumer: no consumer carries a payload
+file yet, and a missing optional import inlines nothing at runtime.
+`correctness-checks.md` remains imported as a deprecated alias for
+`lenses/correctness.md` (frontend carries one today); repos should migrate the file
+and carry at most one of the two, and the alias is removed in the next major
+release. Payloads are additive by contract: the lens prompts state that payload
+rules never relax or override the shared rules, which win on any conflict. The
+router now warns (through `routingConfig.warnings`, surfaced in the review body's
+note lines) when a payload would be silently inert: a filename matching no imported
+payload, a specialist payload no ROUTING rule routes, or the correctness alias
+carried alongside its replacement. The eval's import resolution now matches
+production for the optional form (missing resolves to empty, not the "(not
+configured for this eval case)" note), so corpus case trees can carry payloads; the
+required-form fallback note is unchanged. README documents the new surface and the
+three-way contribution rule (shared skeleton vs lens payload vs skills) and fixes
+two stale claims in the consumer-config section (the undocumented
+`correctness-checks.md`, and the assertion that the optional import form was
+dropped).

workflows/review/README.md

@@ -111,14 +111,18 @@ locally at compile/run time, not from this repo). Create them under
 | `ci-tooling.md` | **Required** | The lint/format/type/test issues your CI already catches. Imported into `correctness-reviewer` so it doesn't flag them, and into `claim-validator` so it drops any correctness claim that flags a CI-caught issue. |
 | `skills.md` | **Required** | The catalog of best-practice skill files (and when each applies). Imported into `skill-auditor` to evaluate the diff against, and into `claim-validator` so it can verify a flagged skill violation against the skill's actual rule. |
 | `ROUTING` | Optional | The machine-readable path map the deterministic router reads (see below). Without it the router spawns no specialist lenses and floors the run budget, and the review notes the missing config on the PR. |
+| `lenses/<lens>.md` | Optional | Per-lens payloads: your repo's surface-specific review rules and extra hunts, imported into the matching reviewer (see [Per-lens payloads](#per-lens-payloads-lenseslensmd)). Absent files import nothing. |
+| `correctness-checks.md` | Deprecated | Alias for `lenses/correctness.md`; still imported for compatibility, and removed in the next major release. Carry one or the other, not both. |
 
-All four are **required**, but validated at different times. `config.md` is a
+The first four are **required**, but validated at different times. `config.md` is a
 frontmatter import, embedded and checked at **compile time** — `gh aw compile` fails if
 it's missing. The other three are `{{#runtime-import}}` body imports inside the
 sub-agent prompts; they resolve when the workflow **runs**, so a missing one surfaces as
 a `Runtime import file not found` failure on the next PR — not at compile time. The
-optional `{{#runtime-import? … }}` form was dropped either way, so a missing config
-fails loudly rather than silently degrading the review.
+required configs deliberately avoid the optional `{{#runtime-import? … }}` form, so a
+missing one fails loudly rather than silently degrading the review. The lens payloads
+(and the deprecated `correctness-checks.md`) use the optional form: a repo that
+defines none is valid, and a missing payload file imports nothing.
 
 These imported snippets are plain Markdown — they must not contain
 `${{ }}` expressions (gh-aw rejects those inside imports). `add-reviewer` lives
@@ -129,6 +133,51 @@ Repo-specific frontmatter that imports can't merge (e.g. an `if:` condition to s
 deploy/automation branches or forks) goes directly in your installed `review.md` as
 a local edit; `gh aw update` preserves it.
 
+### Per-lens payloads (`lenses/<lens>.md`)
+
+A consuming repo may define surface-specific review rules and extra hunts for any
+reviewer lens by adding `.github/aw/review/lenses/<lens>.md`. Each file is imported
+at runtime into the matching reviewer prompt, in a "Repo-specific rules and hunts"
+section the reviewer treats exactly like its built-in rules and tri-state hunts. All
+imports are optional: a repo with no `lenses/` directory gets exactly the shared
+behavior, and lens names never vary per repo; only their payloads do.
+
+Valid names are the eleven specialist lenses (`security-auth`,
+`ai-safety-moderation`, `mass-comms-coppa`, `caching-resource`, `data-migrations`,
+`concurrency-async`, `api-federation-compat`, `cross-deploy-serialization`,
+`deploy-infra-config`, `money-payments`, `content-i18n`) plus `correctness`, which
+feeds the always-on `correctness-reviewer`. `lenses/correctness.md` supersedes the
+older `correctness-checks.md` (still imported as a deprecated alias until the next
+major release; carry at most one of the two). A payload only reaches a specialist
+lens on PRs where the router actually spawns that lens, so a payload without
+matching `ROUTING` `lens=` rules is inert. The router warns in the review body's
+note lines when a payload would be silently inert: a filename that matches no
+imported payload, a specialist payload no `ROUTING` rule routes, or the correctness
+alias carried alongside its replacement.
+
+Payloads are **additive**: they extend the lens's shared rules and hunts but never
+relax or override them, and the shared rules win on any conflict (the lens prompts
+state this next to the import). A payload cannot whitelist a defect or lower the
+evidence bar; it can only add repo-specific things to check.
+
+**Where a rule belongs** (the three-way contribution rule):
+
+- **Shared skeleton** (this repo's `review.md`): rules that hold for every consumer
+  on every stack. If a rule needs stack-specific phrasing ("Datastore query",
+  "DOM sink"), it does not belong here; keep only its surface-neutral core.
+- **Lens payload** (your repo's `lenses/<lens>.md`): rules and hunts specific to
+  your repo's surface. Server repos carry server-surface checks (query bounds,
+  datastore idioms); client repos carry client-surface checks (DOM XSS sinks, token
+  storage, postMessage origins).
+- **Skills** (your repo's `skills.md` catalog): house conventions and sanctioned
+  fixes, audited by `skill-auditor` rather than baked into a lens.
+
+Payloads are model-facing prose owned by the consuming repo's engineers, like
+`risk-classification.md` and the skills catalog before them; the repo's normal code
+review is the bar for editing them. Note that a payload changes reviewer behavior
+without touching Khan/actions: when in doubt about a large payload change, ask the
+workflow maintainers for an eval run over payload-carrying corpus cases.
+
 ### Per-directory `REVIEW.md` contracts (optional)
 
 Separately from `.github/aw/review/`, a consuming repo may carry `REVIEW.md` files in

workflows/review/eval/live-producer.test.ts

@@ -360,21 +360,38 @@ describe("produceLive", () => {
 });
 
 describe("resolveRuntimeImports", () => {
-    it("inlines imports present in the checkout and notes absent ones", () => {
+    it("inlines imports present in the checkout and notes absent required ones", () => {
         const vol = Volume.fromJSON({
             "/checkout/.github/aw/review/skills.md": "## skills index",
         });
         const fs = volFs(vol);
         const prompt = [
             "Skills:\n{{#runtime-import .github/aw/review/skills.md}}",
-            "CI:\n{{#runtime-import? .github/aw/review/ci-tooling.md}}",
+            "CI:\n{{#runtime-import .github/aw/review/ci-tooling.md}}",
         ].join("\n");
         const resolved = resolveRuntimeImports(prompt, "/checkout", fs);
         expect(resolved).toContain("## skills index");
         expect(resolved).toContain("(not configured for this eval case)");
         expect(resolved).not.toMatch(/runtime-import/);
     });
 
+    it("resolves a missing optional import to nothing, like production", () => {
+        const vol = Volume.fromJSON({
+            "/checkout/.github/aw/review/lenses/security-auth.md":
+                "## repo security payload",
+        });
+        const fs = volFs(vol);
+        const prompt = [
+            "Payload:\n{{#runtime-import? .github/aw/review/lenses/security-auth.md}}",
+            "Absent:\n{{#runtime-import? .github/aw/review/lenses/money-payments.md}}",
+        ].join("\n");
+        const resolved = resolveRuntimeImports(prompt, "/checkout", fs);
+        expect(resolved).toContain("## repo security payload");
+        expect(resolved).toContain("Absent:\n");
+        expect(resolved).not.toContain("(not configured for this eval case)");
+        expect(resolved).not.toMatch(/runtime-import/);
+    });
+
     it("reaches the dispatched prompts through produceLive", async () => {
         const {runner, requests} = scriptedRunner({
             "correctness-reviewer": [JSON.stringify({findings: []})],

workflows/review/eval/live-producer.ts

@@ -18,8 +18,11 @@
  *    router's `lensesToSpawn`.
  *  - `{{#runtime-import <path>}}` directives are compile-time inlines of
  *    consumer-repo files. Here they resolve against the case's checkout tree
- *    when the file exists there, else to a fixed "not configured" note, so a
- *    case can opt into a skills index by carrying the file in its tree.
+ *    when the file exists there, so a case can opt into a skills index or a
+ *    lens payload by carrying the file in its tree. A missing optional
+ *    import (`{{#runtime-import? …}}`) resolves to empty exactly as in
+ *    production; a missing required one resolves to a fixed "not
+ *    configured" note where production would fail the run.
  *  - The investigation-cap CLI the prompts invoke is not staged; sub-agents
  *    run with read-only tools and treat the unavailable cap as a denied
  *    budget (the prompt's own fallback: stop investigating, report what you
@@ -201,26 +204,35 @@ const parseReconciliation = (output: string): LiveReconciliation => {
 /* Prompt resolution                                                          */
 /* -------------------------------------------------------------------------- */
 
-const RUNTIME_IMPORT = /\{\{#runtime-import\??\s+([^}\s]+)\s*\}\}/g;
+const RUNTIME_IMPORT = /\{\{#runtime-import(\?)?\s+([^}\s]+)\s*\}\}/g;
 
 const IMPORT_FALLBACK = "(not configured for this eval case)";
 
 /**
  * Inline `{{#runtime-import <path>}}` directives from the case's checkout
- * tree, falling back to a fixed note when the tree does not carry the file.
- * Exported for the A/B runner's reporting (which imports resolved per case).
+ * tree. A missing OPTIONAL import (`{{#runtime-import? …}}`) resolves to the
+ * empty string, matching production (gh-aw's runtime_import.cjs warns and
+ * inlines nothing), so an absent lens payload is behavior-identical to
+ * production. A missing REQUIRED import falls back to a fixed note; this is
+ * the one deliberate deviation (production fails the run), so cases need not
+ * carry every consumer config file. Exported for the A/B runner's reporting
+ * (which imports resolved per case).
  */
 export const resolveRuntimeImports = (
     prompt: string,
     checkoutDir: string,
     fs: Pick<StageFs, "existsSync" | "readFileSync">,
 ): string =>
-    prompt.replace(RUNTIME_IMPORT, (_match, importPath: string) => {
-        const full = `${checkoutDir}/${importPath}`;
-        return fs.existsSync(full)
-            ? fs.readFileSync(full, "utf8")
-            : IMPORT_FALLBACK;
-    });
+    prompt.replace(
+        RUNTIME_IMPORT,
+        (_match, optional: string | undefined, importPath: string) => {
+            const full = `${checkoutDir}/${importPath}`;
+            if (fs.existsSync(full)) {
+                return fs.readFileSync(full, "utf8");
+            }
+            return optional ? "" : IMPORT_FALLBACK;
+        },
+    );
 
 /* -------------------------------------------------------------------------- */
 /* Output parsing: the three sub-agent contracts -> RecordedFinding           */

workflows/review/lib/disciplines.test.ts

@@ -163,6 +163,11 @@ describe("each specialist lens definition", () => {
             expect(section).toContain(
                 "{{#runtime-import .github/aw/review/skills.md}}",
             );
+            // The per-lens consumer payload seam: optional, resolves to
+            // nothing when the host repo carries no payload file.
+            expect(section).toContain(
+                `{{#runtime-import? .github/aw/review/lenses/${lens}.md}}`,
+            );
             expect(section).toContain(`\`lens\` is exactly \`${lens}\``);
             expect(section).toContain(
                 "Domain notes for §Bounded investigation",
@@ -171,6 +176,20 @@ describe("each specialist lens definition", () => {
     }
 });
 
+describe("the correctness payload seam", () => {
+    it("correctness-reviewer imports lenses/correctness.md and its alias", () => {
+        const section = lensSection("correctness-reviewer");
+        expect(section).toContain(
+            "{{#runtime-import? .github/aw/review/lenses/correctness.md}}",
+        );
+        // The deprecated alias stays imported until consumers migrate
+        // (frontend carries one); dropping either import is a regression.
+        expect(section).toContain(
+            "{{#runtime-import? .github/aw/review/correctness-checks.md}}",
+        );
+    });
+});
+
 describe("the label-shape reviewers still carry their own disciplines", () => {
     // Their variants differ materially (CLI cap invocation with per-agent id
     // semantics, `discussion` instead of `evidence_trace`), so they were

workflows/review/lib/lens-payloads.test.ts

@@ -0,0 +1,88 @@
+import {describe, it, expect} from "vitest";
+
+import {
+    CORRECTNESS_ALIAS_PATH,
+    LENS_PAYLOAD_DIR,
+    lensPayloadWarnings,
+} from "./lens-payloads.ts";
+import {SPECIALIST_LENSES} from "./router.ts";
+import type {Lens} from "./finding-schema.ts";
+import type {LensRule} from "./routing-config.ts";
+
+/**
+ * Lens-payload validation tests. The failure mode under test is silence: a
+ * payload file that nothing imports (typo, unknown name) or that an imported
+ * lens never reads (no ROUTING rule spawns it) is invisible at runtime, so
+ * the only surface where the author learns about it is these warnings. The
+ * CLI wiring (readdir + append to routingConfig.warnings) is pinned in
+ * router.test.ts.
+ */
+
+const routed = (lens: Lens): LensRule[] => [
+    {pattern: "src/**", lenses: [lens]},
+];
+
+const warningsFor = (
+    payloadFiles: readonly string[],
+    lensRules: readonly LensRule[] = [],
+    aliasPresent = false,
+): string[] =>
+    lensPayloadWarnings(
+        payloadFiles,
+        lensRules,
+        aliasPresent,
+        SPECIALIST_LENSES,
+    );
+
+describe("lensPayloadWarnings", () => {
+    it("is silent for a routed specialist payload and correctness.md", () => {
+        expect(
+            warningsFor(
+                ["security-auth.md", "correctness.md"],
+                routed("security-auth"),
+            ),
+        ).toEqual([]);
+    });
+
+    it("warns on a filename matching no imported payload", () => {
+        const warnings = warningsFor(
+            ["security_auth.md", "README.md", "notes.txt"],
+            routed("security-auth"),
+        );
+        expect(warnings).toHaveLength(3);
+        for (const warning of warnings) {
+            expect(warning).toContain("matches no imported payload");
+        }
+        expect(warnings[0]).toContain(`${LENS_PAYLOAD_DIR}/security_auth.md`);
+    });
+
+    it("warns on a specialist payload no ROUTING rule routes", () => {
+        const warnings = warningsFor(
+            ["money-payments.md"],
+            routed("security-auth"),
+        );
+        expect(warnings).toHaveLength(1);
+        expect(warnings[0]).toContain("lens=money-payments");
+        expect(warnings[0]).toContain("inert");
+    });
+
+    it("never flags correctness.md as unrouted (always-on reviewer)", () => {
+        expect(warningsFor(["correctness.md"])).toEqual([]);
+    });
+
+    it("warns when correctness.md and its deprecated alias coexist", () => {
+        const warnings = warningsFor(["correctness.md"], [], true);
+        expect(warnings).toHaveLength(1);
+        expect(warnings[0]).toContain(CORRECTNESS_ALIAS_PATH);
+    });
+
+    it("stays silent on the alias alone (supported during migration)", () => {
+        expect(warningsFor([], [], true)).toEqual([]);
+    });
+
+    it("accepts every specialist lens name as a payload filename", () => {
+        for (const lens of SPECIALIST_LENSES) {
+            expect(warningsFor([`${lens}.md`], routed(lens))).toEqual([]);
+        }
+    });
+});