diff --git a/src/review/content-lane/source-evidence.ts b/src/review/content-lane/source-evidence.ts index d24d1b2e74..1b65971a17 100644 --- a/src/review/content-lane/source-evidence.ts +++ b/src/review/content-lane/source-evidence.ts @@ -228,6 +228,33 @@ function listSourceUrlValues(source: string, spec: ContentRepoSpec): SubmittedSo return values; } +// A SCALAR source field authored as a YAML block sequence (`field:` with an empty inline value, then `- item` +// lines) must yield one URL per item — exactly as listSourceUrlValues already does for a list field. Otherwise +// parseSimpleFrontmatter collapses the items into one comma-joined string that `new URL()` cannot parse, so two +// live sources hard-fail a valid submission (#9668). Returns null for every NON-sequence form (an inline +// scalar, a flow `[a, b]` list, or a block scalar `|`/`>`), which the scalar reader already handles correctly; +// only a bare block sequence needs the per-item split. Mirrors listSourceUrlValues' `- item` grammar. +function scalarSequenceItems(source: string, field: string): string[] | null { + // State-machine over the raw block, matching listSourceUrlValues' `for..of` + `as string` style (the + // capture groups always participate when the regex matches, so no `?? ""` index fallbacks are needed). + const items: string[] = []; + let inSequence = false; + for (const line of frontmatterBlock(source).split(/\r?\n/)) { + if (inSequence) { + if (line.trim() === "") break; // a blank line ends the sequence + const item = /^\s*-\s*(.*?)\s*$/.exec(line); + if (!item) break; // the next top-level key (or any non-`-` line) ends the sequence + items.push(item[1] as string); + continue; + } + const head = /^([A-Za-z][A-Za-z0-9_]*):\s*(.*?)\s*$/.exec(line); + if (!head || head[1] !== field) continue; + if ((head[2] as string) !== "") return null; // an inline value present ⇒ scalar / flow / block-scalar header, not a bare sequence + inSequence = true; + } + return items.length > 0 ? items : null; +} + function isAbsoluteHttpUrl(url: string): boolean { try { const protocol = new URL(url).protocol; @@ -244,8 +271,18 @@ export function extractSubmittedSourceUrls( const fields = parseSimpleFrontmatter(source); const urls: SubmittedSourceUrl[] = []; for (const field of spec.sourceUrlFields) { - for (const url of scalarSourceUrlValues(fields[field] || "")) { - urls.push({ field, url }); + const sequenceItems = scalarSequenceItems(source, field); + if (sequenceItems) { + // One URL per block-sequence item (mirrors listSourceUrlValues), so a multi-item scalar sequence is not + // fused into one unparseable string; a single-item sequence still yields exactly one URL as before. + for (const item of sequenceItems) { + const url = unquoteYamlValue(item); + if (url) urls.push({ field, url }); + } + } else { + for (const url of scalarSourceUrlValues(fields[field] || "")) { + urls.push({ field, url }); + } } } urls.push(...listSourceUrlValues(source, spec)); diff --git a/test/unit/content-lane-source-evidence.test.ts b/test/unit/content-lane-source-evidence.test.ts index 111fedf12c..acd762ed8a 100644 --- a/test/unit/content-lane-source-evidence.test.ts +++ b/test/unit/content-lane-source-evidence.test.ts @@ -100,6 +100,24 @@ describe("checkSubmittedSourceEvidence", () => { expect(report.status).toBe("passed"); }); + it("REGRESSION (#9668): fetch-verifies BOTH URLs of a two-item scalar sequence instead of hard-failing the joined string", async () => { + // Before the fix the two items were joined into "https://a.example/1, https://b.example/2", which new URL() + // rejects → outcome invalid_url → the whole report "failed" on a submission whose sources are both live. + const a = "https://a.example/1"; + const b = "https://b.example/2"; + const src = ["---", "documentationUrl:", ` - ${a}`, ` - ${b}`, "---", "", "body"].join("\n"); + const report = await checkSubmittedSourceEvidence(src, fakeFetch({ [a]: 200, [b]: 200 })); + expect(report.urls.map((u) => u.url).sort()).toEqual([a, b]); + expect(report.status).toBe("passed"); + }); + + it("still hard-fails a genuinely malformed scalar source value — the false-positive removal must not weaken the real check", async () => { + const report = await checkSubmittedSourceEvidence(mdx({ documentationUrl: "notaurl" }), fakeFetch({})); + const item = report.urls.find((u) => u.field === "documentationUrl") ?? report.warnings.find((u) => u.field === "documentationUrl"); + expect(item?.outcome).toBe("invalid_url"); + expect(item?.status).toBe("hard_failure"); + }); + it("is retryable (not hard) on a 403/429/5xx canonical source", async () => { const src = mdx({ githubUrl: "https://github.com/acme/x" }); const report = await checkSubmittedSourceEvidence(src, fakeFetch({ "https://github.com/acme/x": 403 })); @@ -359,6 +377,47 @@ describe("extractSubmittedSourceUrls — frontmatter parsing edge cases", () => expect(urls.map((u) => `${u.field}:${u.url}`)).toContain("documentationUrl:https://docs.acme.example/guide"); }); + it("regression (#9668): a scalar source field authored as a MULTI-item block sequence yields one URL per item", () => { + const a = "https://a.example/1"; + const b = "https://b.example/2"; + const src = ["---", "documentationUrl:", ` - ${a}`, ` - ${b}`, "---", "", "body"].join("\n"); + const urls = extractSubmittedSourceUrls(src).filter((u) => u.field === "documentationUrl"); + expect(urls.map((u) => u.url)).toEqual([a, b]); // two distinct URLs, not one comma-joined string + }); + + it("regression (#9668): a single-item scalar sequence is byte-identical — exactly one URL, its raw value", () => { + const url = "https://docs.acme.example/only"; + const src = ["---", "documentationUrl:", ` - ${url}`, "---", "", "body"].join("\n"); + const urls = extractSubmittedSourceUrls(src).filter((u) => u.field === "documentationUrl"); + expect(urls.map((u) => u.url)).toEqual([url]); + }); + + it("regression (#9668): a scalar sequence terminates at the next top-level key or a blank line, and skips an empty item", () => { + const src = [ + "---", + "documentationUrl:", + " - https://a.example/1", + " - ", // an empty item is skipped, not surfaced as an empty URL + " - https://b.example/2", + "githubUrl: https://github.com/acme/x", // a non-`-` top-level key ends the documentationUrl sequence + "sourceUrl:", + " - https://c.example/3", + "", // a blank line ends the sourceUrl sequence + "---", + "", + "body", + ].join("\n"); + const byField = (f: string): string[] => extractSubmittedSourceUrls(src).filter((u) => u.field === f).map((u) => u.url); + expect(byField("documentationUrl")).toEqual(["https://a.example/1", "https://b.example/2"]); + expect(byField("sourceUrl")).toEqual(["https://c.example/3"]); + expect(byField("githubUrl")).toEqual(["https://github.com/acme/x"]); // an inline scalar field is unchanged + }); + + it("regression (#9668): a scalar field with an empty value and no items yields no URL, not a bogus one", () => { + const src = ["---", "documentationUrl:", "---", "", "body"].join("\n"); + expect(extractSubmittedSourceUrls(src).filter((u) => u.field === "documentationUrl")).toEqual([]); + }); + it("does not surface any block-scalar header on a list field as a bogus URL (both indicator orders)", () => { // `retrievalSources` is a list field; a block-scalar header is not a URL. The old guard only skipped the bare // `|`/`>`, so `|-` leaked as the literal url "|-". YAML allows chomping and the indentation digit in EITHER