diff --git a/src/review/content-lane/orchestrator.ts b/src/review/content-lane/orchestrator.ts index 8f69392873..c1c82efeef 100644 --- a/src/review/content-lane/orchestrator.ts +++ b/src/review/content-lane/orchestrator.ts @@ -85,11 +85,9 @@ function appendCountCloseSummary(maxAppendedEntries: number): string { } /** 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.`; + * the registry). Keep it fully generic: duplicate detection runs before content safety validation. */ +function duplicateEntryCloseSummary(): string { + return "A surface submission must not duplicate an entry already in this PR or already in the registry — resubmit without the duplicate."; } // A spec with no domain-specific validator configured yet (RegistryLaneSpec.assessAppendedEntry / @@ -171,7 +169,7 @@ export async function runSurfaceReview(spec: RegistryLaneSpec, input: SurfaceRev const existingEntries = surfacesOf(safeParseJson(baseRaw), spec.collectionField) ?? []; const duplicate = findDuplicateAppendedEntry(spec, appendedEntries, existingEntries); if (duplicate !== null) { - return { verdict: "close", summary: duplicateEntryCloseSummary(duplicate[0]) }; + return { verdict: "close", summary: duplicateEntryCloseSummary() }; } const assessEntry = spec.assessAppendedEntry; if (!assessEntry) { diff --git a/test/unit/content-lane-orchestrator.test.ts b/test/unit/content-lane-orchestrator.test.ts index 99ff401244..974ea4d8ba 100644 --- a/test/unit/content-lane-orchestrator.test.ts +++ b/test/unit/content-lane-orchestrator.test.ts @@ -277,15 +277,34 @@ describe("runSurfaceReview (deterministic + decisive: merge/close, rarely manual 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); + 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.", + }); }); 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); + 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.", + }); + }); + + it("does not echo unvalidated duplicate URLs into public close summaries", async () => { + const unsafeUrl = "not-a-safe-url \n### injected markdown"; + const duplicate = { ...newEntry, id: "unsafe-duplicate", url: unsafeUrl }; + const copy = { ...duplicate, id: "unsafe-duplicate-copy" }; + const r = await review([SUBNET], { [`head:${SUBNET}`]: doc([existing, duplicate, copy]), [`base:${SUBNET}`]: doc([existing]) }); + + 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.", + }); + expect(r?.summary).not.toContain(unsafeUrl); + expect(r?.summary).not.toContain("a-fake-token-that-should-never-leak"); + expect(r?.summary).not.toContain("### injected markdown"); }); it("a same-PR duplicate is still detected across trivial URL formatting differences (trailing slash/tracking params)", async () => {