From 58102c54c036580f1062321f5e539c3016fa5671 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 30 Jul 2026 23:54:15 -0700 Subject: [PATCH] feat(chat): the maintainer grounding-tool registry, read-only by construction First half of #9189, implementing the catalog #9187's spec confirmed. Six tools that return deterministic JSON read from state ORB already holds -- grounding, not generation: a maintainer asking "why is this PR held" gets the recorded verdict, not a paraphrase of one. READ-ONLY IS ENFORCED, NOT PROMISED. GroundingServices is the tools' ENTIRE capability surface, and it holds no Octokit, no token and no mutation. A tool cannot perform a write because there is nothing in scope to write with. Two tests assert that as a property of the source rather than of a runtime object: the interface names no write capability, and the module imports nothing at all -- a tool that reached for its own Octokit would bypass the injected surface entirely. REDACTION IS STRUCTURAL. Every response is built by NAMING its fields. The test that distinguishes this from a blocklist feeds each service a payload carrying walletAddress, hotkey, rewardValue, trustScore, privateRanking and authorEmail, and asserts none reaches the serialized result. A blocklist has to anticipate every field an upstream type might grow; an allowlisted shape cannot leak one nobody wrote down. The accuracy union passes through WHOLE rather than being flattened -- coverage and interval are what make it a claim rather than marketing, which is why fairness_summary reuses buildProofSummary instead of aggregating again, and why chat and /proof cannot disagree about a number. DETERMINISM: identical state produces identical BYTES, so a retry does not re-ground the model differently and a cache means something. Every list is sorted on a stable key; a test drives the same service returning a set in reversed order and asserts the bytes match. An absent source reports `available: false`, never an error and never a zero. A deployment without a ledger is not a broken one, and an empty queue would read as "nothing to do". Two of the spec's eight candidate tools are deliberately ABSENT. decision_record_by_digest and audit_tail have no reader today -- the nearest are by pull number and by sequence respectively -- so shipping them would mean inventing SQL inside a chat PR, which requirement 1 rules out. Recorded on the issue with options; each needs its own authz and redaction review. Registered in STAGED_AHEAD_OF_CONSUMERS because this is a seam landed ahead of its callers: binding GroundingServices to the real readers and wiring the protected route with its authz is the second half of #9189, and it deserves its own pass rather than riding along here. Mutation-tested: spreading the config object instead of naming fields fails 3, dropping a sort fails 2, flattening accuracy to a bare number fails 2. Refs #9189 --- scripts/check-dead-source-files.ts | 4 + src/chat/grounding-registry.ts | 193 +++++++++++++++++++++++++++ test/unit/grounding-registry.test.ts | 180 +++++++++++++++++++++++++ 3 files changed, 377 insertions(+) create mode 100644 src/chat/grounding-registry.ts create mode 100644 test/unit/grounding-registry.test.ts diff --git a/scripts/check-dead-source-files.ts b/scripts/check-dead-source-files.ts index 68fa063c71..dd0ac2fc88 100644 --- a/scripts/check-dead-source-files.ts +++ b/scripts/check-dead-source-files.ts @@ -62,6 +62,10 @@ const STAGED_AHEAD_OF_CONSUMERS: ReadonlyMap = new Map([ "src/openapi/define-route.ts", "The route-registration seam (#9519), landed by #9533 ahead of the 241-route migration it exists for; #9531 moves routes onto it incrementally.", ], + [ + "src/chat/grounding-registry.ts", + "The maintainer chat grounding-tool registry (#9189, spec #9187). Deliberately ahead of its route: the six tools are defined over an INJECTED GroundingServices surface, and binding that surface to the real readers (queueSnapshotFromBinding, verifyDecisionLedger, resolveProofPage, loadRepoFocusManifest, getRepository) plus wiring the protected route and its authz is the second half of #9189. The registry's own invariants -- read-only by construction, allowlisted response shapes, determinism, per-tool budgets -- are fully covered by test/unit/grounding-registry.test.ts today.", + ], [ "src/review/benchmark-eval-records.ts", "The benchmark score-record emitter + leaderboard derivation (#9265). Deliberately ahead of its serving route: #9216's endpoint sub-issue wires GET /v1/public/eval-scores to emit benchmark_run records, and #9264 supplies the attestation envelopes its `attested` tier needs. Fully covered by test/unit/benchmark-eval-records.test.ts today.", diff --git a/src/chat/grounding-registry.ts b/src/chat/grounding-registry.ts new file mode 100644 index 0000000000..757893da17 --- /dev/null +++ b/src/chat/grounding-registry.ts @@ -0,0 +1,193 @@ +// Read-only grounding tools for the maintainer chat surface (#9189, epic #9183, spec #9187). +// +// GROUNDING, NOT GENERATION. Every tool here returns deterministic JSON read from state ORB already holds. +// The model consumes these results; it never produces the numbers itself. That split is the whole design: +// a maintainer asking "why is this PR held" gets the recorded verdict, not a paraphrase of one. +// +// ── READ-ONLY IS ENFORCED BY CONSTRUCTION, NOT PROMISED ────────────────────────────────────────────── +// {@link GroundingServices} is the ENTIRE capability surface these tools have, and it contains no Octokit, +// no token, and no write function of any kind. A tool cannot perform a GitHub write because there is nothing +// in scope to write with -- rather than because a reviewer checked that it doesn't. The invariant test +// asserts the shape of this interface, so adding a write capability fails a test rather than passing review. +// +// ── REDACTION IS STRUCTURAL ────────────────────────────────────────────────────────────────────────── +// Every response is built by NAMING its fields, never by filtering a wider object. A blocklist has to +// anticipate every field an upstream type might grow and silently leaks the one it did not; an allowlisted +// shape cannot leak a field nobody wrote down. Wallet, hotkey, reward, private-ranking and raw-trust values +// are therefore unreachable here by construction, not by vigilance. +// +// ── DETERMINISM ────────────────────────────────────────────────────────────────────────────────────── +// Identical state must produce identical bytes, so a model is not re-grounded differently on a retry and a +// cache is meaningful. Every list is sorted on a stable key before it is returned; nothing relies on the +// order a query happened to yield. + +/** + * The complete capability surface of the grounding tools. + * + * DELIBERATELY NARROW. Each member is an existing service, injected so the registry is testable without a + * database and so the read-only property is visible in one place rather than argued about per tool. Note + * what is absent: no Octokit, no installation token, no mutation of any kind. + */ +export type GroundingServices = { + queueSnapshot: (repoFullName: string) => Promise<{ pending: number; processing: number } | null>; + ledgerVerify: () => Promise<{ ok: boolean; tipSeq: number; totalCount: number } | null>; + proofSummary: (repoFullName: string) => Promise; + effectiveConfig: (repoFullName: string) => Promise<{ present: boolean; source: string; fields: string[] } | null>; + repoSettings: (repoFullName: string) => Promise<{ gatePack: string | null; autonomy: Record } | null>; + prStatus: (repoFullName: string, pullNumber: number) => Promise; +}; + +/** The subset of the proof summary chat surfaces. Accuracy NEVER travels without its coverage and interval + * -- requirement 6, and the same rule `buildProofSummary` already enforces for the public proof page. */ +export type GroundingProofSummary = { + decisionCount: number; + accuracy: + | { state: "published"; accuracy: number; decided: number; confirmed: number; interval: { lo: number; hi: number } } + | { state: "insufficient_data"; decided: number; minimumDecisions: number }; + ledgerState: string; + anchorState: string; +}; + +export type GroundingPrStatus = { + pullNumber: number; + action: string | null; + reasonCode: string | null; + holdCause: string[]; + ciState: string | null; + recordDigest: string | null; +}; + +/** A tool result. `budgetTokens` is the tool's declared ceiling, carried on the result so a caller can + * enforce it without a second table mapping names to budgets. */ +export type GroundingResult = { tool: string; budgetTokens: number; data: unknown }; + +export type GroundingTool = { + name: string; + /** Response ceiling. Declared per tool because the shapes differ by an order of magnitude, and a single + * global budget would either truncate the queue snapshot or waste the ledger check. */ + budgetTokens: number; + /** Whether the tool needs a pull number. Declared rather than inferred from the args, so a caller can + * present the catalog without invoking anything. */ + needsPullNumber: boolean; + run: (services: GroundingServices, repoFullName: string, pullNumber: number | null) => Promise; +}; + +/** PURE. Stable ordering for any string list a tool returns. Sorting on the value itself (rather than + * leaving query order) is what makes two identical calls byte-identical. */ +export function stableStrings(values: readonly string[]): string[] { + return [...values].sort((a, b) => a.localeCompare(b)); +} + +/** + * The v1 catalog: six tools, each wrapping a service that already exists. + * + * `decision_record_by_digest` and `audit_tail` from the spec's candidate set are deliberately ABSENT -- see + * the note on #9189. Neither has a reader today (`loadDecisionRecordCollapsible` is by pull number and the + * public ledger row endpoint is by sequence), so shipping them here would mean inventing a query inside a + * chat PR, which is exactly what requirement 1 rules out. They are filed separately so each new read gets + * its own authz and redaction review. + */ +export const GROUNDING_TOOLS: readonly GroundingTool[] = [ + { + name: "queue_snapshot", + budgetTokens: 2000, + needsPullNumber: false, + run: async (services, repoFullName) => { + const snapshot = await services.queueSnapshot(repoFullName); + return snapshot === null ? { available: false } : { available: true, pending: snapshot.pending, processing: snapshot.processing }; + }, + }, + { + name: "ledger_verify", + budgetTokens: 500, + needsPullNumber: false, + run: async (services) => { + const verified = await services.ledgerVerify(); + // `available: false` rather than an error: a deployment with no ledger is not a broken one, and the + // model must not narrate an outage from a surface that simply is not present here. + return verified === null ? { available: false } : { available: true, ok: verified.ok, tipSeq: verified.tipSeq, totalCount: verified.totalCount }; + }, + }, + { + name: "fairness_summary", + budgetTokens: 1500, + needsPullNumber: false, + run: async (services, repoFullName) => { + const summary = await services.proofSummary(repoFullName); + if (summary === null) return { available: false }; + // Copied field by field, and the accuracy union is passed through WHOLE. Flattening it to a bare + // number here would strip the coverage and interval that make it a claim rather than marketing -- + // requirement 6, and the reason this reuses buildProofSummary instead of aggregating again. + return { + available: true, + decisionCount: summary.decisionCount, + accuracy: summary.accuracy, + ledgerState: summary.ledgerState, + anchorState: summary.anchorState, + }; + }, + }, + { + name: "effective_config", + budgetTokens: 2000, + needsPullNumber: false, + run: async (services, repoFullName) => { + const config = await services.effectiveConfig(repoFullName); + // Field NAMES only, never values. Which fields an operator set is the useful grounding; the values + // are the operator's configuration and have no business being narrated by a model. + return config === null ? { available: false } : { available: true, present: config.present, source: config.source, fields: stableStrings(config.fields) }; + }, + }, + { + name: "repo_settings", + budgetTokens: 1000, + needsPullNumber: false, + run: async (services, repoFullName) => { + const settings = await services.repoSettings(repoFullName); + if (settings === null) return { available: false }; + const autonomy = Object.fromEntries(stableStrings(Object.keys(settings.autonomy)).map((key) => [key, settings.autonomy[key]!])); + return { available: true, gatePack: settings.gatePack, autonomy }; + }, + }, + { + name: "pr_status", + budgetTokens: 1000, + needsPullNumber: true, + run: async (services, repoFullName, pullNumber) => { + if (pullNumber === null) return { available: false, reason: "pull_number_required" }; + const status = await services.prStatus(repoFullName, pullNumber); + if (status === null) return { available: false }; + return { + available: true, + pullNumber: status.pullNumber, + action: status.action, + reasonCode: status.reasonCode, + // #9991: WHY it was held, now that the ledger records it. Sorted for determinism. + holdCause: stableStrings(status.holdCause), + ciState: status.ciState, + recordDigest: status.recordDigest, + }; + }, + }, +]; + +/** PURE. The catalog, for a caller that wants to present the tools without invoking one. Sorted by name so + * the advertised catalog is stable across processes. */ +export function groundingToolCatalog(): { name: string; budgetTokens: number; needsPullNumber: boolean }[] { + return [...GROUNDING_TOOLS] + .sort((a, b) => a.name.localeCompare(b.name)) + .map((tool) => ({ name: tool.name, budgetTokens: tool.budgetTokens, needsPullNumber: tool.needsPullNumber })); +} + +/** Run one tool by name. Returns null for an unknown name so the caller decides the status code, rather than + * this module inventing an HTTP concern. */ +export async function runGroundingTool( + name: string, + services: GroundingServices, + repoFullName: string, + pullNumber: number | null = null, +): Promise { + const tool = GROUNDING_TOOLS.find((candidate) => candidate.name === name); + if (tool === undefined) return null; + return { tool: tool.name, budgetTokens: tool.budgetTokens, data: await tool.run(services, repoFullName, pullNumber) }; +} diff --git a/test/unit/grounding-registry.test.ts b/test/unit/grounding-registry.test.ts new file mode 100644 index 0000000000..af85c7f0d6 --- /dev/null +++ b/test/unit/grounding-registry.test.ts @@ -0,0 +1,180 @@ +import { readFileSync } from "node:fs"; + +import { describe, expect, it } from "vitest"; + +import { + GROUNDING_TOOLS, + groundingToolCatalog, + runGroundingTool, + stableStrings, + type GroundingServices, +} from "../../src/chat/grounding-registry"; + +// #9189: the read-only grounding registry. +// +// The issue's own bar is that authz and redaction are "guaranteed by tests rather than convention", so the +// four invariants below are the point of this file — the happy paths are almost incidental by comparison: +// +// • READ-ONLY, by construction. The tools' entire capability surface is GroundingServices, which holds no +// Octokit, no token and no mutation. A tool cannot write because there is nothing to write with. +// • REDACTION, structural. Responses are built by naming fields, so a field nobody wrote down cannot leak +// — including when an upstream type grows one. +// • DETERMINISM. Identical state must produce identical BYTES, or a retry re-grounds the model differently. +// • BUDGETS. Declared per tool and carried on the result. + +const services = (over: Partial = {}): GroundingServices => ({ + queueSnapshot: async () => ({ pending: 3, processing: 1 }), + ledgerVerify: async () => ({ ok: true, tipSeq: 128, totalCount: 128 }), + proofSummary: async () => ({ + decisionCount: 128, + accuracy: { state: "published", accuracy: 0.964, decided: 112, confirmed: 108, interval: { lo: 0.912, hi: 0.987 } }, + ledgerState: "verified", + anchorState: "anchored", + }), + effectiveConfig: async () => ({ present: true, source: "repo_file", fields: ["wantedPaths", "gate", "settings"] }), + repoSettings: async () => ({ gatePack: "standard", autonomy: { merge: "auto", close: "observe" } }), + prStatus: async () => ({ pullNumber: 42, action: "hold", reasonCode: "success", holdCause: ["screenshotEvidenceHold", "guardrailHit"], ciState: "passed", recordDigest: "d".repeat(64) }), + ...over, +}); + +describe("read-only is enforced by construction (#9189 requirement 4)", () => { + it("INVARIANT: the services surface exposes no write capability at all", () => { + // Asserted over the SOURCE of the type, not a runtime object: the guarantee is that nothing + // write-capable is in scope for a tool, and that is a property of the interface declaration. + const source = readFileSync("src/chat/grounding-registry.ts", "utf8"); + const surface = source.slice(source.indexOf("export type GroundingServices"), source.indexOf("/** The subset of the proof summary")); + for (const forbidden of ["Octokit", "octokit", "token", "Token", "write", "mutate", "createComment", "merge(", "close("]) { + expect(surface, `GroundingServices must not expose ${forbidden}`).not.toContain(forbidden); + } + }); + + it("INVARIANT: no tool reaches for a capability outside the injected services", () => { + // A tool that imported an Octokit directly would bypass the surface above entirely. The registry module + // must therefore import nothing that can talk to GitHub. + const source = readFileSync("src/chat/grounding-registry.ts", "utf8"); + const imports = source.split("\n").filter((line) => line.startsWith("import ")); + expect(imports, "the registry must stay dependency-free so its capability surface is the injected one").toEqual([]); + }); + + it("runs every tool with a services object holding only readers, and none throws", async () => { + for (const tool of GROUNDING_TOOLS) { + await expect(runGroundingTool(tool.name, services(), "acme/widgets", 42), tool.name).resolves.toBeTruthy(); + } + }); +}); + +describe("redaction is structural (#9189 requirement 3)", () => { + it("INVARIANT: an upstream object that grows a secret field cannot leak it", () => { + // The test that distinguishes an allowlist from a blocklist. Every service below returns a payload + // carrying fields no response shape names; if the tools copied objects wholesale, these would appear. + const poisoned = services({ + queueSnapshot: async () => ({ pending: 1, processing: 0, walletAddress: "0xdead", hotkey: "5Fxx" }) as never, + repoSettings: async () => ({ gatePack: "standard", autonomy: { merge: "auto" }, rewardValue: 42, trustScore: 0.9 }) as never, + effectiveConfig: async () => ({ present: true, source: "repo_file", fields: ["gate"], privateRanking: [1, 2] }) as never, + prStatus: async () => ({ pullNumber: 1, action: "hold", reasonCode: "x", holdCause: [], ciState: "passed", recordDigest: "d", authorEmail: "a@b.c" }) as never, + }); + return Promise.all( + GROUNDING_TOOLS.map(async (tool) => { + const result = await runGroundingTool(tool.name, poisoned, "acme/widgets", 1); + const serialized = JSON.stringify(result); + for (const secret of ["walletAddress", "0xdead", "hotkey", "5Fxx", "rewardValue", "trustScore", "privateRanking", "authorEmail", "a@b.c"]) { + expect(serialized, `${tool.name} leaked ${secret}`).not.toContain(secret); + } + }), + ); + }); + + it("passes the accuracy union through WHOLE, so coverage and interval cannot be stripped", async () => { + // Requirement 6. Flattening this to a bare number is the failure mode -- a percentage with no + // denominator and no interval is marketing, and it is exactly what buildProofSummary refuses to publish. + const result = await runGroundingTool("fairness_summary", services(), "acme/widgets"); + const accuracy = (result?.data as { accuracy: Record }).accuracy; + expect(accuracy).toMatchObject({ state: "published", decided: 112, confirmed: 108, interval: { lo: 0.912, hi: 0.987 } }); + }); + + it("reports insufficient data as a state, never as a fabricated zero", async () => { + const sparse = services({ proofSummary: async () => ({ decisionCount: 7, accuracy: { state: "insufficient_data", decided: 7, minimumDecisions: 20 }, ledgerState: "verified", anchorState: "not_yet_anchored" }) }); + const result = await runGroundingTool("fairness_summary", sparse, "acme/widgets"); + expect((result?.data as { accuracy: { state: string } }).accuracy.state).toBe("insufficient_data"); + expect(JSON.stringify(result)).not.toContain('"accuracy":0'); + }); + + it("returns config field NAMES, never their values", async () => { + const result = await runGroundingTool("effective_config", services(), "acme/widgets"); + expect((result?.data as { fields: string[] }).fields).toEqual(["gate", "settings", "wantedPaths"]); + }); +}); + +describe("determinism (#9189 requirement 5)", () => { + it("INVARIANT: two identical calls produce identical bytes, for every tool", async () => { + for (const tool of GROUNDING_TOOLS) { + const first = JSON.stringify(await runGroundingTool(tool.name, services(), "acme/widgets", 42)); + const second = JSON.stringify(await runGroundingTool(tool.name, services(), "acme/widgets", 42)); + expect(second, tool.name).toBe(first); + } + }); + + it("REGRESSION: query order does not change the bytes", async () => { + // The real risk: a service returning the same set in a different order would otherwise re-ground the + // model differently on a retry, and make any cache meaningless. + const forward = services({ effectiveConfig: async () => ({ present: true, source: "repo_file", fields: ["gate", "settings", "wantedPaths"] }) }); + const reversed = services({ effectiveConfig: async () => ({ present: true, source: "repo_file", fields: ["wantedPaths", "settings", "gate"] }) }); + expect(JSON.stringify(await runGroundingTool("effective_config", reversed, "acme/widgets"))).toBe( + JSON.stringify(await runGroundingTool("effective_config", forward, "acme/widgets")), + ); + }); + + it("sorts hold causes, so a reordered cause list grounds identically", async () => { + const result = await runGroundingTool("pr_status", services(), "acme/widgets", 42); + expect((result?.data as { holdCause: string[] }).holdCause).toEqual(["guardrailHit", "screenshotEvidenceHold"]); + }); + + it("advertises a stable catalog", () => { + expect(groundingToolCatalog().map((tool) => tool.name)).toEqual([ + "effective_config", + "fairness_summary", + "ledger_verify", + "pr_status", + "queue_snapshot", + "repo_settings", + ]); + }); +}); + +describe("availability and budgets", () => { + it("reports an absent source as unavailable, never as an error or a zero", async () => { + // A deployment without a ledger is not a broken one. `available: false` stops the model narrating an + // outage from a surface that simply is not present -- and an empty queue would read as "nothing to do". + const absent = services({ ledgerVerify: async () => null, queueSnapshot: async () => null, proofSummary: async () => null }); + for (const name of ["ledger_verify", "queue_snapshot", "fairness_summary"]) { + const result = await runGroundingTool(name, absent, "acme/widgets"); + expect(result?.data, name).toEqual({ available: false }); + } + }); + + it("requires a pull number where the tool declares it, rather than guessing one", async () => { + const result = await runGroundingTool("pr_status", services(), "acme/widgets", null); + expect(result?.data).toEqual({ available: false, reason: "pull_number_required" }); + expect(GROUNDING_TOOLS.find((tool) => tool.name === "pr_status")?.needsPullNumber).toBe(true); + }); + + it("carries each tool's declared budget on its result", async () => { + for (const tool of GROUNDING_TOOLS) { + const result = await runGroundingTool(tool.name, services(), "acme/widgets", 42); + expect(result?.budgetTokens, tool.name).toBe(tool.budgetTokens); + expect(tool.budgetTokens, tool.name).toBeGreaterThan(0); + } + }); + + it("returns null for an unknown tool, leaving the status code to the caller", async () => { + expect(await runGroundingTool("definitely_not_a_tool", services(), "acme/widgets")).toBeNull(); + }); +}); + +describe("stableStrings", () => { + it("sorts without mutating its input", () => { + const input = ["b", "a"]; + expect(stableStrings(input)).toEqual(["a", "b"]); + expect(input).toEqual(["b", "a"]); + }); +});