diff --git a/src/review/content-lane-wire.ts b/src/review/content-lane-wire.ts index fc6ef08f97..2d60c8172a 100644 --- a/src/review/content-lane-wire.ts +++ b/src/review/content-lane-wire.ts @@ -137,7 +137,8 @@ export async function runMetagraphedSurfaceGate( // An unreadable HEAD — or a null BASE for a file GitHub reports as "modified" (whose base MUST exist, so a // null read is a transient fetch blip, NOT an absent base) — would make a valid submission read as empty/ // invalid → a spurious one-shot close. Defer to the generic gate instead. A null base for an ADDED file is - // the expected brand-new-entry case and is left to the orchestrator (one new entry merges; many close). + // the expected brand-new-entry case and is left to the orchestrator, whose spec-driven entry-count policy + // decides the verdict (METAGRAPHED_LANE_SPEC allows any number of clean entries — see maxAppendedEntries). if (ref === "head" && content === null) deferUnreadable = true; if (ref === "base" && content === null && statusByPath.get(path) === "modified") deferUnreadable = true; return content; diff --git a/src/review/content-lane/index.ts b/src/review/content-lane/index.ts index 1138ae8a0d..50d0e733b7 100644 --- a/src/review/content-lane/index.ts +++ b/src/review/content-lane/index.ts @@ -109,7 +109,7 @@ export { type ProviderLike, type Verdict, } from "./registry-logic"; -export { runSurfaceReview, diffAppendedSurfaceEntry, type SurfaceReviewInput, type SurfaceReviewResult } from "./orchestrator"; +export { runSurfaceReview, diffAppendedSurfaceEntries, type SurfaceReviewInput, type SurfaceReviewResult } from "./orchestrator"; export { checkNetuidExists, fetchSubnetRecord, diff --git a/src/review/content-lane/orchestrator.ts b/src/review/content-lane/orchestrator.ts index af690eff51..d591b059ab 100644 --- a/src/review/content-lane/orchestrator.ts +++ b/src/review/content-lane/orchestrator.ts @@ -2,17 +2,23 @@ // sole adjudicator). Given a lane spec, the PR's changed files, and an injected file-content loader, it: // 1. classifies the PR via classifyRegistryPrScope (entry / provider / not-a-direct-submission), // 2. loads the head (+ base, for entries) document content, -// 3. resolves the SINGLE appended surfaces[] entry by diffing head vs base, and -// 4. returns a normalized verdict from assessSubnetDocument / assessProviderDocument. +// 3. resolves the appended surfaces[] entries by diffing head vs base, capped at the spec's maxAppendedEntries +// (omitted ⇒ today's strict single-entry-only default), +// 4. rejects a duplicate appended entry when the spec opts into duplicateKeyFields (omitted ⇒ off), and +// 5. validates EACH remaining appended entry independently and returns one aggregate verdict from +// assessSubnetDocument / assessProviderDocument: close if any entry is invalid, manual if any (remaining) +// needs manual review, merge only when every entry is clean. // Pure + injectable: unit tests pass a loadFile stub, so no network. The live wiring (a per-repo, // flag-gated branch in the review body) is a separate follow-up. import { + type Assessment, type ProviderAssessment, type RegistryLaneSpec, type Verdict, assessProviderDocument, assessSubnetDocument, classifyRegistryPrScope, + findDuplicateAppendedEntry, toCoreVerdict, } from "./registry-logic"; @@ -44,18 +50,18 @@ function surfacesOf(doc: unknown, field: string): unknown[] | null { } /** - * The single surfaces[] entry present at head but absent at base — the deterministic "exactly one appended - * entry" rule. Returns null when head is unreadable / has no surfaces[] array, or when the count of added - * entries !== 1 (a reorder/reformat/edit of existing entries reads as multiple "added" and is rejected upstream). - * A missing base file (a brand-new entry file) means every head entry is new, so it passes only when there is one. + * ALL surfaces[] entries present at head but absent at base — a pure head-vs-base structural diff. Returns null + * when head is unreadable / has no surfaces[] array; returns an empty array when nothing was added (a + * reorder/reformat/edit of existing entries reads as zero "added"). A missing base file (a brand-new entry file) + * means every head entry is new. Makes no count judgement itself — the caller (runSurfaceReview) enforces the + * spec's maxAppendedEntries cap and the ≥1-entry requirement. */ -export function diffAppendedSurfaceEntry(headRaw: string | null, baseRaw: string | null, field: string): unknown { +export function diffAppendedSurfaceEntries(headRaw: string | null, baseRaw: string | null, field: string): unknown[] | null { const headEntries = surfacesOf(safeParseJson(headRaw), field); if (headEntries === null) return null; const baseEntries = surfacesOf(safeParseJson(baseRaw), field) ?? []; const baseKeys = new Set(baseEntries.map((entry) => JSON.stringify(entry))); - const added = headEntries.filter((entry) => !baseKeys.has(JSON.stringify(entry))); - return added.length === 1 ? added[0] : null; + return headEntries.filter((entry) => !baseKeys.has(JSON.stringify(entry))); } function fromProvider(assessment: ProviderAssessment): SurfaceReviewResult { @@ -65,13 +71,64 @@ function fromProvider(assessment: ProviderAssessment): SurfaceReviewResult { : { verdict: "close", summary: assessment.summary, reason: assessment.reason }; } +// Spec-less backward compat: a lane that doesn't opt into a higher/unlimited cap stays at today's strict +// single-entry-only behavior (see RegistryLaneSpec.maxAppendedEntries). +const DEFAULT_MAX_APPENDED_ENTRIES = 1; + +/** The close summary for an appended-entry count outside [1, maxAppendedEntries]. */ +function appendCountCloseSummary(maxAppendedEntries: number): string { + if (maxAppendedEntries === 1) { + return "A surface submission must append exactly one new surfaces[] entry — resubmit a clean single-entry append."; + } + return Number.isFinite(maxAppendedEntries) + ? `A surface submission must append between 1 and ${maxAppendedEntries} new surfaces[] entries in one PR — resubmit a clean append within that range.` + : "A surface submission must append at least one new surfaces[] entry — resubmit a clean append."; +} + +/** The close summary for a duplicate appended entry (a same-PR repeat, or a resubmission of an entry already in + * the registry) — names the colliding url when the duplicate entry has one, for a concrete resubmit target. */ +function duplicateEntryCloseSummary(duplicate: unknown): string { + const url = (duplicate as { url?: unknown } | null)?.url; + const detail = typeof url === "string" && url.trim() !== "" ? ` (${url.trim()})` : ""; + return `A surface submission must not duplicate an entry already in this PR or already in the registry${detail} — resubmit without the duplicate.`; +} + +/** + * Aggregate N independent per-entry assessments into ONE verdict: close if ANY entry is invalid, manual if ANY + * (of the remainder) needs manual review (e.g. auth_required), merge only if EVERY entry is clean — mirroring the + * single-entry decisiveness policy (merge/close dominate; manual is the rare exception) at whatever count the + * spec allows. When more than one entry was appended, the surfaced summary is prefixed with its position so a + * multi-entry PR's close/manual reason still points at the specific offending entry. + */ +function pickAggregateAssessment(assessments: Assessment[]): Assessment { + const count = assessments.length; + // label() is only ever called below on a "closed" or "manual-review" assessment, and every such assessment sets + // summary (fail() requires it; the explicit closed/manual-review returns in assessSurfaceEntry both set it) — + // so assessment.summary is never undefined here. + const label = (assessment: Assessment, idx: number): Assessment => + count <= 1 ? assessment : { ...assessment, summary: `Surface entry ${idx + 1} of ${count}: ${assessment.summary}` }; + let manual: [number, Assessment] | null = null; + let first: Assessment | null = null; + for (const [idx, assessment] of assessments.entries()) { + first ??= assessment; + if (assessment.verdict === "closed") return label(assessment, idx); + if (manual === null && assessment.verdict === "manual-review") manual = [idx, assessment]; + } + if (manual !== null) return label(manual[1], manual[0]); + // Every remaining assessment.verdict is "merged" (the only member of MetaVerdict left), and runSurfaceReview + // never calls this with an empty array (the appended-entry-count guard there returns early first). + return first as Assessment; +} + /** * Adjudication policy (deterministic, DECISIVE): the overwhelming majority of outcomes are merge or close — * manual review is the rare exception. A clean valid submission MERGES; anything invalid or non-standard - * (a malformed/violating entry, a non-clean append, a bundled "mixed-files" PR, an invalid provider) CLOSES with - * a resubmit message. A PR that is NOT a registry submission at all returns `null` — the surface lane does not - * apply, so the caller falls through to the generic gate. The only residual MANUAL comes from the per-entry - * validator (an authenticated interface needing a human to confirm the public auth scheme) — a "very few" case. + * (a malformed/violating entry, an out-of-range append count, a duplicate entry when the spec opts into + * duplicateKeyFields, a bundled "mixed-files" PR, an invalid provider) CLOSES with a resubmit message. A PR that + * is NOT a registry submission at all returns `null` — the surface lane does not apply, so the caller falls + * through to the generic gate. The only residual MANUAL comes from the per-entry validator (an authenticated + * interface needing a human to confirm the public auth scheme) — a "very few" case, and one bad entry among + * several still closes the whole PR (see pickAggregateAssessment). */ export async function runSurfaceReview(spec: RegistryLaneSpec, input: SurfaceReviewInput): Promise { const scope = classifyRegistryPrScope(spec, input.changedFiles); @@ -95,10 +152,19 @@ export async function runSurfaceReview(spec: RegistryLaneSpec, input: SurfaceRev return fromProvider(assessProviderDocument(safeParseJson(headRaw), input.opts)); } const baseRaw = await input.loadFile(directFile, "base"); - const appendedEntry = diffAppendedSurfaceEntry(headRaw, baseRaw, spec.collectionField); - if (appendedEntry === null) { - return { verdict: "close", summary: "A surface submission must append exactly one new surfaces[] entry — resubmit a clean single-entry append." }; + const appendedEntries = diffAppendedSurfaceEntries(headRaw, baseRaw, spec.collectionField); + const maxAppendedEntries = spec.maxAppendedEntries ?? DEFAULT_MAX_APPENDED_ENTRIES; + if (appendedEntries === null || appendedEntries.length === 0 || appendedEntries.length > maxAppendedEntries) { + return { verdict: "close", summary: appendCountCloseSummary(maxAppendedEntries) }; + } + const existingEntries = surfacesOf(safeParseJson(baseRaw), spec.collectionField) ?? []; + const duplicate = findDuplicateAppendedEntry(spec, appendedEntries, existingEntries); + if (duplicate !== null) { + return { verdict: "close", summary: duplicateEntryCloseSummary(duplicate[0]) }; } - const assessment = assessSubnetDocument(safeParseJson(headRaw), { ...input.opts, appendedEntry }); + const headDoc = safeParseJson(headRaw); + const assessment = pickAggregateAssessment( + appendedEntries.map((appendedEntry) => assessSubnetDocument(headDoc, { ...input.opts, appendedEntry })), + ); return { verdict: toCoreVerdict(assessment.verdict), summary: assessment.summary, reason: assessment.reason }; } diff --git a/src/review/content-lane/registry-logic.ts b/src/review/content-lane/registry-logic.ts index a74d779614..86b311c5ef 100644 --- a/src/review/content-lane/registry-logic.ts +++ b/src/review/content-lane/registry-logic.ts @@ -544,10 +544,12 @@ export function assessSubnetDocument( if (!Array.isArray(doc.surfaces)) { return fail("unsupported-shape", "Subnet document must carry a surfaces[] array."); } - // The orchestrator resolves the single appended entry by diffing head vs base surfaces[]; it passes null when - // the PR adds zero or more than one entry (or edits an existing one) — never a silent multi-entry merge. + // A per-entry call: the orchestrator resolves every appended entry by diffing head vs base surfaces[], enforces + // the spec's maxAppendedEntries cap (and the ≥1-entry requirement) BEFORE calling this per entry, then calls it + // once per appended entry. So appendedEntry is null/undefined here only when a specific array element itself is + // missing/malformed (a data-shape problem) — it is no longer a "wrong count" sentinel. if (appendedEntry === null || appendedEntry === undefined) { - return fail("unsupported-shape", "PR must append exactly one surface entry to surfaces[]."); + return fail("unsupported-shape", "Surface entry to assess is missing — the appended entry could not be resolved."); } // Whole-document secret scan catches material in the envelope (outside the entry); the entry is re-scanned below. if (secretsScan && containsSecretLikeText(JSON.stringify(doc))) { @@ -667,6 +669,20 @@ export interface RegistryLaneSpec { artifactPattern?: RegExp; /** The array field on an entry file a contribution appends to (the surface model: "surfaces"). */ collectionField: string; + /** Max surfaces[] entries a single PR may append in one run. Omitted ⇒ today's strict single-entry-only + * default — safe-by-default backward compat for every spec that doesn't explicitly opt in (including future, + * unknown per-repo registries). Set explicitly to raise the cap; `Infinity` removes it entirely (e.g. + * metagraphed's documented "several surfaces for one subnet in one diff is one merge" anti-farming policy). */ + maxAppendedEntries?: number; + /** Entry field names whose COMBINED values identify "the same entry" for duplicate detection (e.g. `["url"]`). + * Omitted ⇒ duplicate detection is OFF (safe-by-default backward compat — a spec that doesn't opt in gets no + * new close reason). When set, an appended entry whose identity matches an entry already present in the base + * document's `collectionField` array, OR an earlier entry appended in the SAME PR, closes the whole PR. A field + * whose value looks like a URL is compared via `normalizePublicUrl` (so trivial formatting differences don't + * count as different); every other field is compared as a trimmed, case-insensitive string (or a structural + * JSON comparison for a non-string value). Generic — the engine only duck-types the configured field names off + * each entry, it never assumes any domain-specific shape (kind/netuid/etc. are metagraphed's own vocabulary). */ + duplicateKeyFields?: readonly string[]; } export type RegistryPrScope = "entry-submission" | "provider-submission" | "mixed-files" | "not-direct-submission"; @@ -713,6 +729,58 @@ export function isRegistrySubmissionScope(scope: RegistryPrScope): boolean { return scope === "entry-submission" || scope === "provider-submission"; } +/** Normalizes a single field value for duplicate-identity comparison. A URL-shaped string is canonicalized via + * normalizePublicUrl (so http/https/case/trailing-slash/tracking-param differences don't count as different); + * any other string is trimmed + lowercased; a non-string value falls back to a structural JSON comparison. + * Returns null for a null/undefined value (an absent field contributes nothing to the identity). */ +function normalizeIdentityValue(value: unknown): string | null { + if (value === null || value === undefined) return null; + if (typeof value === "string") return normalizePublicUrl(value) ?? value.trim().toLowerCase(); + return JSON.stringify(value); +} + +/** The duplicate-identity key for `entry` under `fields` (e.g. `["url"]` or `["url", "kind"]`): the normalized + * values of those fields, combined via JSON.stringify. A plain joined string (e.g. space-separated) would be + * AMBIGUOUS — two entries whose field values straddle the join boundary differently could collide onto the same + * string (`["alpha beta", "docs"]` vs `["alpha", "beta docs"]` both joining to "alpha beta docs") and falsely + * read as duplicates; JSON.stringify's quoting/structure makes field boundaries unambiguous regardless of + * content. Returns null when EVERY configured field is absent on this entry — there's nothing to key on, so it + * can never match or be matched. */ +function duplicateIdentityKey(entry: unknown, fields: readonly string[]): string | null { + const record = entry as Record | null; + const parts = fields.map((field) => normalizeIdentityValue(record?.[field])); + return parts.every((part) => part === null) ? null : JSON.stringify(parts); +} + +/** + * The first entry in `appendedEntries` whose duplicate-identity key (under `spec.duplicateKeyFields`) collides + * with an EARLIER appended entry (a same-PR duplicate) or with any entry already in `existingEntries` (a + * resubmission of an entry already in the registry) — wrapped in a 1-tuple so a legitimate falsy/null entry value + * is never confused with "no duplicate found" (plain `null`). Returns null when the spec has no + * `duplicateKeyFields` (the default — duplicate detection is opt-in per spec) or no collision exists. Generic: + * works for ANY RegistryLaneSpec by duck-typing the configured field names, not just metagraphed's. + */ +export function findDuplicateAppendedEntry( + spec: RegistryLaneSpec, + appendedEntries: readonly unknown[], + existingEntries: readonly unknown[], +): [unknown] | null { + const fields = spec.duplicateKeyFields; + if (!fields || fields.length === 0) return null; + const seen = new Set(); + for (const entry of existingEntries) { + const key = duplicateIdentityKey(entry, fields); + if (key !== null) seen.add(key); + } + for (const entry of appendedEntries) { + const key = duplicateIdentityKey(entry, fields); + if (key === null) continue; + if (seen.has(key)) return [entry]; + seen.add(key); + } + return null; +} + // metagraphed's spec — the first RegistryLaneSpec. surfaces[] live in registry/subnets/.json; providers // are FLAT registry/providers/.json (the community/ subdir was retired). A PR touching the old // registry/candidates/community/* path matches none of these → mixed-files / not-direct (correctly not adopted @@ -724,4 +792,13 @@ export const METAGRAPHED_LANE_SPEC: RegistryLaneSpec = { providerFilePattern: FLAT_PROVIDER_PATTERN, artifactPattern: ARTIFACT_PATTERN, collectionField: "surfaces", + // metagraphed's contributor docs deliberately allow appending SEVERAL surfaces[] entries for one subnet in one + // PR (the 2026-06 anti-farming fix: splitting one subnet's surfaces into many near-identical PRs is what the + // single-entry cap used to force) — no cap here, per entry validated independently by the orchestrator. + maxAppendedEntries: Infinity, + // Removing the single-entry cap also removed its incidental side effect of rejecting a same-PR duplicate + // surfaces[] entry (added.length!==1 used to close it). Opt back into duplicate detection explicitly, keyed on + // `url` alone (a subnet's surfaces are distinct interfaces; the same url appearing twice — in one PR or against + // an entry already registered — is a resubmission, not a new surface). + duplicateKeyFields: ["url"], }; diff --git a/test/unit/content-lane-orchestrator.test.ts b/test/unit/content-lane-orchestrator.test.ts index 8915092aa4..9af6b47d8b 100644 --- a/test/unit/content-lane-orchestrator.test.ts +++ b/test/unit/content-lane-orchestrator.test.ts @@ -1,39 +1,54 @@ import { describe, expect, it } from "vitest"; -import { METAGRAPHED_LANE_SPEC } from "../../src/review/content-lane/registry-logic"; -import { diffAppendedSurfaceEntry, runSurfaceReview, type SurfaceReviewInput } from "../../src/review/content-lane/orchestrator"; +import { + ARTIFACT_PATTERN, + FLAT_PROVIDER_PATTERN, + METAGRAPHED_LANE_SPEC, + SUBNET_ENTRY_PATTERN, + type RegistryLaneSpec, +} from "../../src/review/content-lane/registry-logic"; +import { diffAppendedSurfaceEntries, runSurfaceReview, type SurfaceReviewInput } from "../../src/review/content-lane/orchestrator"; const existing = { kind: "website", url: "https://old.example.ai", source_url: "https://github.com/a/b", public_safe: true }; const newEntry = { kind: "subnet-api", url: "https://api.example.ai", source_url: "https://github.com/x/y", public_safe: true }; +const newEntry2 = { kind: "openapi", url: "https://api2.example.ai", source_url: "https://github.com/x/z", public_safe: true }; const SUBNET = "registry/subnets/foo.json"; const PROVIDER = "registry/providers/acme.json"; +// A spec-less-backward-compat stand-in: the default single-entry cap (no maxAppendedEntries override), otherwise +// identical to metagraphed's file layout so SUBNET still classifies as an entry-submission. +const STRICT_SPEC: RegistryLaneSpec = { + entryFilePattern: SUBNET_ENTRY_PATTERN, + providerFilePattern: FLAT_PROVIDER_PATTERN, + artifactPattern: ARTIFACT_PATTERN, + collectionField: "surfaces", +}; // Inject a file loader keyed by `${ref}:${path}` so the orchestrator never hits the network. function loader(files: Record): SurfaceReviewInput["loadFile"] { return (path, ref) => Promise.resolve(files[`${ref}:${path}`] ?? null); } -const review = (changedFiles: string[], files: Record) => - runSurfaceReview(METAGRAPHED_LANE_SPEC, { changedFiles, loadFile: loader(files) }); +const review = (changedFiles: string[], files: Record, spec: RegistryLaneSpec = METAGRAPHED_LANE_SPEC) => + runSurfaceReview(spec, { changedFiles, loadFile: loader(files) }); -describe("diffAppendedSurfaceEntry", () => { +describe("diffAppendedSurfaceEntries", () => { const doc = (surfaces: unknown[]) => JSON.stringify({ netuid: 14, surfaces }); - it("returns the single entry added at head", () => { - expect(diffAppendedSurfaceEntry(doc([existing, newEntry]), doc([existing]), "surfaces")).toEqual(newEntry); + it("returns every entry added at head", () => { + expect(diffAppendedSurfaceEntries(doc([existing, newEntry]), doc([existing]), "surfaces")).toEqual([newEntry]); + expect(diffAppendedSurfaceEntries(doc([existing, newEntry, newEntry2]), doc([existing]), "surfaces")).toEqual([newEntry, newEntry2]); }); - it("treats every entry as new when the base file is absent (passes only with exactly one)", () => { - expect(diffAppendedSurfaceEntry(doc([newEntry]), null, "surfaces")).toEqual(newEntry); - expect(diffAppendedSurfaceEntry(doc([existing, newEntry]), null, "surfaces")).toBeNull(); + it("treats every entry as new when the base file is absent", () => { + expect(diffAppendedSurfaceEntries(doc([newEntry]), null, "surfaces")).toEqual([newEntry]); + expect(diffAppendedSurfaceEntries(doc([existing, newEntry]), null, "surfaces")).toEqual([existing, newEntry]); }); - it("returns null for zero or multiple added entries", () => { - expect(diffAppendedSurfaceEntry(doc([existing]), doc([existing]), "surfaces")).toBeNull(); - expect(diffAppendedSurfaceEntry(doc([existing, newEntry, { kind: "other" }]), doc([existing]), "surfaces")).toBeNull(); + it("returns an empty array when nothing was added", () => { + expect(diffAppendedSurfaceEntries(doc([existing]), doc([existing]), "surfaces")).toEqual([]); }); it("returns null when head is unparseable or has no surfaces[] array", () => { - expect(diffAppendedSurfaceEntry("{not json", doc([existing]), "surfaces")).toBeNull(); - expect(diffAppendedSurfaceEntry(JSON.stringify({ netuid: 14 }), doc([existing]), "surfaces")).toBeNull(); + expect(diffAppendedSurfaceEntries("{not json", doc([existing]), "surfaces")).toBeNull(); + expect(diffAppendedSurfaceEntries(JSON.stringify({ netuid: 14 }), doc([existing]), "surfaces")).toBeNull(); }); }); @@ -102,8 +117,194 @@ describe("runSurfaceReview (deterministic + decisive: merge/close, rarely manual expect(r?.verdict).toBe("close"); }); - it("CLOSES a non-clean append (multiple new entries) — resubmit clean, not a manual punt", async () => { - const r = await review([SUBNET], { [`head:${SUBNET}`]: doc([existing, newEntry, { kind: "extra" }]), [`base:${SUBNET}`]: doc([existing]) }); + it("closes a zero-entry append (an edit-only PR that appends nothing new)", async () => { + const r = await review([SUBNET], { [`head:${SUBNET}`]: doc([existing]), [`base:${SUBNET}`]: doc([existing]) }); + expect(r).toEqual({ + verdict: "close", + summary: "A surface submission must append at least one new surfaces[] entry — resubmit a clean append.", + }); + }); + + it("closes when head is unreadable/malformed (diffAppendedSurfaceEntries returns null)", async () => { + const r = await review([SUBNET], { [`head:${SUBNET}`]: "{not json", [`base:${SUBNET}`]: doc([existing]) }); + expect(r?.verdict).toBe("close"); + }); + + // metagraphed's documented 2026-06 anti-farming policy: several surfaces[] entries for ONE subnet in ONE PR is + // one merge (PR #2619's exact shape — one registry/subnets/.json file, two clean new surfaces entries). + it("merges a clean multi-entry append against the metagraphed spec (PR #2619 shape)", async () => { + const r = await review([SUBNET], { + [`head:${SUBNET}`]: doc([existing, newEntry, newEntry2]), + [`base:${SUBNET}`]: doc([existing]), + }); + expect(r?.verdict).toBe("merge"); + }); + + // Regression for PR #2619 itself (JSONbored/metagraphed): a brand-new registry/subnets/affine.json (GitHub + // status "added", so base is absent) appending two clean surfaces[] entries in one file. Orb one-shot-closed + // this with "A surface submission must append exactly one new surfaces[] entry" under the old single-entry cap + // — it must now MERGE. + it("merges the real PR #2619 shape: a brand-new subnet file with two clean appended entries", async () => { + const AFFINE_SUBNET = "registry/subnets/affine.json"; + const affineDoc = { + categories: [], + curation: { level: "community-seeded", review_state: "unreviewed" }, + name: "Affine", + netuid: 120, + schema_version: 1, + slug: "sn-120", + status: "active", + surfaces: [ + { + auth_required: false, + authority: "community", + id: "sn-120-affine-openapi", + kind: "openapi", + name: "Affine API OpenAPI schema", + notes: "Machine-readable OpenAPI schema for the public Affine validator API.", + provider: "affine", + public_safe: true, + review: { state: "community-submitted", submitted_by: "dragunovx16" }, + schema_status: "machine-readable", + schema_url: "https://api.affine.io/openapi.json", + source_urls: [ + "https://raw.githubusercontent.com/AffineFoundation/affine-cortex/main/affine/api/server.py", + "https://raw.githubusercontent.com/AffineFoundation/affine-cortex/main/affine/utils/api_client.py", + ], + url: "https://api.affine.io/openapi.json", + }, + { + auth_required: false, + authority: "community", + id: "sn-120-affine-subnet-api", + kind: "subnet-api", + name: "Affine API health", + notes: "Safe read-only health endpoint for the public Affine validator API.", + provider: "affine", + public_safe: true, + review: { state: "community-submitted", submitted_by: "dragunovx16" }, + schema_url: "https://api.affine.io/openapi.json", + source_urls: [ + "https://raw.githubusercontent.com/AffineFoundation/affine-cortex/main/affine/utils/api_client.py", + "https://raw.githubusercontent.com/AffineFoundation/affine-cortex/main/affine/api/server.py", + ], + url: "https://api.affine.io/api/v1/health", + }, + ], + }; + const r = await review( + [AFFINE_SUBNET], + { [`head:${AFFINE_SUBNET}`]: JSON.stringify(affineDoc) }, // base absent: GitHub reports this file as "added" + ); + expect(r?.verdict).toBe("merge"); + }); + + it("CLOSES a multi-entry append against the metagraphed spec when ANY appended entry is invalid", async () => { + const bad = { ...newEntry2, public_safe: false }; + const r = await review([SUBNET], { [`head:${SUBNET}`]: doc([existing, newEntry, bad]), [`base:${SUBNET}`]: doc([existing]) }); expect(r?.verdict).toBe("close"); + expect(r?.summary).toContain("Surface entry 2 of 2"); + }); + + it("routes a multi-entry append to MANUAL when one entry needs manual review and none are invalid", async () => { + const authEntry = { ...newEntry2, auth_required: true }; + const r = await review([SUBNET], { [`head:${SUBNET}`]: doc([existing, newEntry, authEntry]), [`base:${SUBNET}`]: doc([existing]) }); + expect(r?.verdict).toBe("manual"); + expect(r?.summary).toContain("Surface entry 2 of 2"); + }); + + it("a close among several appended entries wins over an earlier manual one (manual precedes close in array order)", async () => { + const authEntry = { ...newEntry, auth_required: true }; + const bad = { ...newEntry2, public_safe: false }; + const r = await review([SUBNET], { [`head:${SUBNET}`]: doc([existing, authEntry, bad]), [`base:${SUBNET}`]: doc([existing]) }); + expect(r?.verdict).toBe("close"); + }); + + it("a close among several appended entries wins over a later manual one too (close precedes manual in array order)", async () => { + const bad = { ...newEntry, public_safe: false }; + const authEntry = { ...newEntry2, auth_required: true }; + const r = await review([SUBNET], { [`head:${SUBNET}`]: doc([existing, bad, authEntry]), [`base:${SUBNET}`]: doc([existing]) }); + expect(r?.verdict).toBe("close"); + }); + + it("CLOSES a non-clean append (multiple new entries) when the spec caps at the default of one — resubmit clean, not a manual punt", async () => { + const r = await review( + [SUBNET], + { [`head:${SUBNET}`]: doc([existing, newEntry, newEntry2]), [`base:${SUBNET}`]: doc([existing]) }, + STRICT_SPEC, + ); + expect(r).toEqual({ + verdict: "close", + summary: "A surface submission must append exactly one new surfaces[] entry — resubmit a clean single-entry append.", + }); + }); + + it("still merges a clean SINGLE append against the default (spec-less) single-entry cap", async () => { + const r = await review([SUBNET], { [`head:${SUBNET}`]: doc([existing, newEntry]), [`base:${SUBNET}`]: doc([existing]) }, STRICT_SPEC); + expect(r?.verdict).toBe("merge"); + }); + + it("closes a multi-entry append that exceeds an explicit finite cap with the 'between 1 and N' message", async () => { + const cappedSpec: RegistryLaneSpec = { ...STRICT_SPEC, maxAppendedEntries: 2 }; + const r = await review( + [SUBNET], + { [`head:${SUBNET}`]: doc([existing, newEntry, newEntry2, { ...newEntry, url: "https://api3.example.ai" }]), [`base:${SUBNET}`]: doc([existing]) }, + cappedSpec, + ); + expect(r).toEqual({ + verdict: "close", + summary: "A surface submission must append between 1 and 2 new surfaces[] entries in one PR — resubmit a clean append within that range.", + }); + }); + + it("merges a clean append landing EXACTLY on an explicit finite cap (the boundary is inclusive, not exclusive)", async () => { + const cappedSpec: RegistryLaneSpec = { ...STRICT_SPEC, maxAppendedEntries: 2 }; + const r = await review( + [SUBNET], + { [`head:${SUBNET}`]: doc([existing, newEntry, newEntry2]), [`base:${SUBNET}`]: doc([existing]) }, + cappedSpec, + ); + expect(r?.verdict).toBe("merge"); + }); + + // METAGRAPHED_LANE_SPEC opts into duplicateKeyFields: ["url"] specifically because removing the single-entry + // cap also removed its incidental side effect of rejecting a same-PR duplicate append. + it("closes a same-PR duplicate append against the metagraphed spec (removing the entry cap removed this incidental protection)", async () => { + const copy = { ...newEntry, id: "a-copy-of-newEntry" }; + const r = await review([SUBNET], { [`head:${SUBNET}`]: doc([existing, newEntry, copy]), [`base:${SUBNET}`]: doc([existing]) }); + expect(r?.verdict).toBe("close"); + expect(r?.summary).toContain(newEntry.url); + }); + + it("closes an appended entry that resubmits a url already present in the base document's surfaces[]", async () => { + const resubmission = { ...existing, id: "resubmitted-existing-url" }; + const r = await review([SUBNET], { [`head:${SUBNET}`]: doc([existing, resubmission]), [`base:${SUBNET}`]: doc([existing]) }); + expect(r?.verdict).toBe("close"); + expect(r?.summary).toContain(existing.url); + }); + + it("a same-PR duplicate is still detected across trivial URL formatting differences (trailing slash/tracking params)", async () => { + const messyDuplicate = { ...newEntry, url: `${newEntry.url}/?utm_source=test` }; + const r = await review([SUBNET], { [`head:${SUBNET}`]: doc([existing, newEntry, messyDuplicate]), [`base:${SUBNET}`]: doc([existing]) }); + expect(r?.verdict).toBe("close"); + }); + + it("a spec without duplicateKeyFields never flags a duplicate (opt-in default preserved for spec-less/backward-compat consumers)", async () => { + // STRICT_SPEC has no duplicateKeyFields; a SINGLE append (within its cap of 1) that resubmits an existing url + // must merge, not close, since duplicate detection was never opted into for this spec. + const resubmission = { ...existing, id: "resubmitted-existing-url" }; + const r = await review([SUBNET], { [`head:${SUBNET}`]: doc([existing, resubmission]), [`base:${SUBNET}`]: doc([existing]) }, STRICT_SPEC); + expect(r?.verdict).toBe("merge"); + }); + + it("the duplicate close summary omits the url detail when the colliding entries have no usable url (a non-'url' duplicateKeyFields spec)", async () => { + const kindOnlySpec: RegistryLaneSpec = { ...STRICT_SPEC, maxAppendedEntries: Infinity, duplicateKeyFields: ["kind"] }; + const first = { kind: "openapi", public_safe: true }; + const dup = { kind: "openapi", public_safe: true, name: "a differently-named duplicate" }; + const r = await review([SUBNET], { [`head:${SUBNET}`]: doc([existing, first, dup]), [`base:${SUBNET}`]: doc([existing]) }, kindOnlySpec); + expect(r).toEqual({ + verdict: "close", + summary: "A surface submission must not duplicate an entry already in this PR or already in the registry — resubmit without the duplicate.", + }); }); }); diff --git a/test/unit/content-lane-registry-logic.test.ts b/test/unit/content-lane-registry-logic.test.ts index 7e8505bc95..b5d2672030 100644 --- a/test/unit/content-lane-registry-logic.test.ts +++ b/test/unit/content-lane-registry-logic.test.ts @@ -5,6 +5,7 @@ import { assessFreshness, assessProviderDocument, classifyRegistryPrScope, + findDuplicateAppendedEntry, isRegistrySubmissionScope, METAGRAPHED_LANE_SPEC, type RegistryLaneSpec, @@ -186,7 +187,7 @@ describe("assessSubnetDocument (whole-file gate: root netuid + exactly-one appen expect(assessSubnetDocument({ netuid: 14 }, { appendedEntry: entry }).reason).toBe("unsupported-shape"); }); - it("rejects when the orchestrator found zero-or-many appended entries (sentinel null/undefined)", () => { + it("rejects a null/undefined appendedEntry (the orchestrator found no valid entry to assess for this call)", () => { expect(assessSubnetDocument(doc, { appendedEntry: null }).reason).toBe("unsupported-shape"); expect(assessSubnetDocument(doc, { appendedEntry: undefined }).reason).toBe("unsupported-shape"); }); @@ -264,6 +265,12 @@ describe("probeFunctionalSurface", () => { }); }); +describe("METAGRAPHED_LANE_SPEC", () => { + it("has no per-PR cap on appended surfaces[] entries (the 2026-06 anti-farming policy)", () => { + expect(METAGRAPHED_LANE_SPEC.maxAppendedEntries).toBe(Infinity); + }); +}); + describe("classifyRegistryPrScope (generic surface model, metagraphed spec)", () => { const spec = METAGRAPHED_LANE_SPEC; it("recognizes a subnet entry-submission with an allowed generated-artifact companion", () => { @@ -320,6 +327,119 @@ describe("classifyRegistryPrScope (generic surface model, metagraphed spec)", () }); }); +describe("findDuplicateAppendedEntry (generic, spec-driven duplicate detection — opt-in per RegistryLaneSpec)", () => { + const specNoDedup: RegistryLaneSpec = { entryFilePattern: /^x$/, collectionField: "surfaces" }; + const specUrl: RegistryLaneSpec = { entryFilePattern: /^x$/, collectionField: "surfaces", duplicateKeyFields: ["url"] }; + const specUrlKind: RegistryLaneSpec = { entryFilePattern: /^x$/, collectionField: "surfaces", duplicateKeyFields: ["url", "kind"] }; + const a = { kind: "openapi", url: "https://api.example.ai/openapi.json" }; + const b = { kind: "subnet-api", url: "https://api.example.ai/health" }; + + it("is off by default: a spec with no duplicateKeyFields never flags a duplicate", () => { + expect(findDuplicateAppendedEntry(specNoDedup, [a, a], [])).toBeNull(); + expect(findDuplicateAppendedEntry(specNoDedup, [a], [a])).toBeNull(); + }); + + it("is off when duplicateKeyFields is an explicit empty array (no fields to key on)", () => { + const specEmpty: RegistryLaneSpec = { entryFilePattern: /^x$/, collectionField: "surfaces", duplicateKeyFields: [] }; + expect(findDuplicateAppendedEntry(specEmpty, [a, a], [])).toBeNull(); + }); + + it("flags a same-PR duplicate: a later appended entry whose url matches an earlier one", () => { + const dup = { ...a, id: "copy" }; + const result = findDuplicateAppendedEntry(specUrl, [a, dup], []); + expect(result).toEqual([dup]); + }); + + it("flags an appended entry that resubmits a url already in the base document's existing entries", () => { + const existingA = { ...a, id: "existing-a" }; + const appendedDuplicate = { ...a, id: "new-submission-same-url" }; + expect(findDuplicateAppendedEntry(specUrl, [appendedDuplicate], [existingA])).toEqual([appendedDuplicate]); + }); + + it("detects a url that only differs by trivial formatting (trailing slash/case/tracking params) as the same entry", () => { + const canonical = { kind: "website", url: "https://Example.com/path/?utm_source=x" }; + const trivialVariant = { kind: "website", url: "https://example.com/path/" }; + expect(findDuplicateAppendedEntry(specUrl, [canonical, trivialVariant], [])).toEqual([trivialVariant]); + }); + + it("does not flag two appended entries with genuinely different urls", () => { + expect(findDuplicateAppendedEntry(specUrl, [a, b], [])).toBeNull(); + }); + + it("skips an entry whose configured field is absent — it can never match or be matched", () => { + const noUrl1 = { kind: "website" }; + const noUrl2 = { kind: "docs" }; + expect(findDuplicateAppendedEntry(specUrl, [noUrl1, noUrl2], [a])).toBeNull(); + }); + + it("skips an EXISTING entry whose configured field is absent — it is never added to the seen set", () => { + const existingWithoutUrl = { kind: "docs" }; + // `a` is appended fresh; the only existing entry lacks a url entirely, so it can't collide with anything. + expect(findDuplicateAppendedEntry(specUrl, [a], [existingWithoutUrl])).toBeNull(); + }); + + it("a compound key with only SOME fields present still keys correctly (mixed null/non-null parts)", () => { + const kindOnly = { kind: "openapi" }; // no url: the "url" part of the compound key is null + const urlOnly = { url: "https://mixed.example/x" }; // no kind: the "kind" part is null + // Neither collides with the other (their non-null parts land in different positions), and neither collides + // with itself appended twice under a DIFFERENT partial-field entry — this just exercises the mixed null/ + // non-null `part ?? ""` join path without asserting a match. + expect(findDuplicateAppendedEntry(specUrlKind, [kindOnly, urlOnly], [])).toBeNull(); + const kindOnlyRepeat = { kind: "openapi", extra: "still no url" }; + expect(findDuplicateAppendedEntry(specUrlKind, [kindOnly, kindOnlyRepeat], [])).toEqual([kindOnlyRepeat]); + }); + + it("a compound key (url + kind) treats the same url under a DIFFERENT kind as a distinct entry", () => { + const sameUrlDifferentKind = { kind: "website", url: a.url }; + expect(findDuplicateAppendedEntry(specUrlKind, [a, sameUrlDifferentKind], [])).toBeNull(); + }); + + it("a compound key (url + kind) still flags the same url under the SAME kind", () => { + const sameUrlSameKind = { ...a, name: "renamed copy" }; + expect(findDuplicateAppendedEntry(specUrlKind, [a, sameUrlSameKind], [])).toEqual([sameUrlSameKind]); + }); + + it("compares a non-string field value structurally (JSON.stringify fallback) rather than by url normalization", () => { + const specNumericField: RegistryLaneSpec = { entryFilePattern: /^x$/, collectionField: "surfaces", duplicateKeyFields: ["netuid"] }; + const first = { netuid: 14 }; + const dup = { netuid: 14, name: "different metadata" }; + const distinct = { netuid: 15 }; + expect(findDuplicateAppendedEntry(specNumericField, [first, dup], [])).toEqual([dup]); + expect(findDuplicateAppendedEntry(specNumericField, [first, distinct], [])).toBeNull(); + }); + + it("a non-URL string field is compared case-insensitively and trimmed", () => { + const specKindOnly: RegistryLaneSpec = { entryFilePattern: /^x$/, collectionField: "surfaces", duplicateKeyFields: ["kind"] }; + expect(findDuplicateAppendedEntry(specKindOnly, [{ kind: "Website" }, { kind: " website " }], [])).toEqual([{ kind: " website " }]); + }); + + it("regression: a compound key does NOT let two GENUINELY DIFFERENT entries collide by straddling the field-join boundary", () => { + // Two distinct entries whose field values, if naively joined with a plain separator, would produce the SAME + // string ("alpha beta" + "docs" vs "alpha" + "beta docs" both join to "alpha beta docs"). The identity key + // must be built so field boundaries stay unambiguous regardless of content — these must NOT be flagged. + const specTitleKind: RegistryLaneSpec = { entryFilePattern: /^x$/, collectionField: "surfaces", duplicateKeyFields: ["title", "kind"] }; + const entryA = { title: "alpha beta", kind: "docs" }; + const entryB = { title: "alpha", kind: "beta docs" }; + expect(findDuplicateAppendedEntry(specTitleKind, [entryA, entryB], [])).toBeNull(); + // The genuinely identical pair (same title AND same kind) must still be caught. + const entryC = { title: "alpha beta", kind: "docs", extra: "irrelevant" }; + expect(findDuplicateAppendedEntry(specTitleKind, [entryA, entryC], [])).toEqual([entryC]); + }); + + it("wraps the duplicate in a 1-tuple so a falsy entry value (null) is distinguishable from 'no duplicate found'", () => { + const specNullable: RegistryLaneSpec = { entryFilePattern: /^x$/, collectionField: "surfaces", duplicateKeyFields: ["url"] }; + // null/non-object entries have no indexable "url" field → normalizeIdentityValue sees undefined → key null → + // never added to `seen` and never matched. This asserts the NOT-a-duplicate outcome stays a plain `null`. + expect(findDuplicateAppendedEntry(specNullable, [null, undefined], [])).toBeNull(); + }); + + it("ignores entries already present at both base AND head (an unrelated pre-existing pair never trips a same-PR-only check)", () => { + // b is appended once; the base document ALSO already independently contains an unrelated entry `a` — the + // presence of an unrelated existing entry must not spuriously flag the single new append as a duplicate. + expect(findDuplicateAppendedEntry(specUrl, [b], [a])).toBeNull(); + }); +}); + describe("isBaseLayerKind", () => { it("recognizes the chain base-layer kinds", () => { expect(isBaseLayerKind("subtensor-wss")).toBe(true); diff --git a/test/unit/content-lane-wire.test.ts b/test/unit/content-lane-wire.test.ts index e765a5fa1c..9e05938050 100644 --- a/test/unit/content-lane-wire.test.ts +++ b/test/unit/content-lane-wire.test.ts @@ -180,6 +180,14 @@ describe("runMetagraphedSurfaceGate (injected loader — adapter logic)", () => const out = await run([{ path: SUBNET, status: "added" }], { [`head:${SUBNET}`]: doc([newEntry]) }); expect(out?.conclusion).toBe("success"); }); + + it("a same-PR duplicate append (METAGRAPHED_LANE_SPEC's opt-in duplicateKeyFields) → failure end-to-end through the live adapter wiring", async () => { + const advisory = { findings: [] as AdvisoryFinding[] }; + const copy = { ...newEntry, id: "a-copy" }; + const out = await run([{ path: SUBNET, status: "modified" }], { [`head:${SUBNET}`]: doc([existing, newEntry, copy]), [`base:${SUBNET}`]: doc([existing]) }, advisory); + expect(out?.conclusion).toBe("failure"); + expect(advisory.findings.map((f) => f.code)).toEqual(["surface_lane_reject"]); + }); }); describe("resolveSurfaceRefs", () => {