Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/review/content-lane-wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/review/content-lane/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
100 changes: 83 additions & 17 deletions src/review/content-lane/orchestrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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 {
Expand All @@ -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<SurfaceReviewResult | null> {
const scope = classifyRegistryPrScope(spec, input.changedFiles);
Expand All @@ -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 };
}
83 changes: 80 additions & 3 deletions src/review/content-lane/registry-logic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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))) {
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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<string, unknown> | 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<string>();
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/<slug>.json; providers
// are FLAT registry/providers/<slug>.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
Expand All @@ -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"],
};
Loading