Skip to content

Commit 5b1a54c

Browse files
author
RealDiligent
committed
fix(content-lane): split a scalar source field's multi-item block sequence per URL
parseSimpleFrontmatter collapses a block sequence to a comma-joined string, and for a scalar source field that value goes to scalarSourceUrlValues, which only splits the flow (`[a, b]`) form. So a documentationUrl authored as a two-item block sequence yielded the single string "https://a.example/1, https://b.example/2"; new URL() throws on it, producing outcome invalid_url / hard_failure and failing the whole source-evidence report on a submission whose sources are both live. #8016's tests covered only the single-item case (a one-item join is a valid URL), so it went undetected. Add scalarSequenceItems, which reads the raw frontmatter block and returns one value per `- item` for a bare block sequence (mirroring listSourceUrlValues), used by the scalar-field loop; a non-sequence form (inline, flow, or block scalar) returns null and keeps the existing scalar reader. A single-item sequence still yields exactly one URL, and a genuinely malformed value still hard-fails. Closes #9668
1 parent af98111 commit 5b1a54c

2 files changed

Lines changed: 70 additions & 2 deletions

File tree

src/review/content-lane/source-evidence.ts

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,31 @@ function listSourceUrlValues(source: string, spec: ContentRepoSpec): SubmittedSo
228228
return values;
229229
}
230230

231+
// A SCALAR source field authored as a YAML block sequence (`field:` with an empty inline value, then `- item`
232+
// lines) must yield one URL per item — exactly as listSourceUrlValues already does for a list field. Otherwise
233+
// parseSimpleFrontmatter collapses the items into one comma-joined string that `new URL()` cannot parse, so two
234+
// live sources hard-fail a valid submission (#9668). Returns null for every NON-sequence form (an inline
235+
// scalar, a flow `[a, b]` list, or a block scalar `|`/`>`), which the scalar reader already handles correctly;
236+
// only a bare block sequence needs the per-item split. Mirrors listSourceUrlValues' `- item` grammar.
237+
function scalarSequenceItems(source: string, field: string): string[] | null {
238+
const lines = frontmatterBlock(source).split(/\r?\n/);
239+
for (let i = 0; i < lines.length; i += 1) {
240+
const head = /^([A-Za-z][A-Za-z0-9_]*):\s*(.*?)\s*$/.exec(lines[i] ?? "");
241+
if (!head || head[1] !== field) continue;
242+
if ((head[2] ?? "") !== "") return null; // an inline value present ⇒ scalar / flow / block-scalar header, not a bare sequence
243+
const items: string[] = [];
244+
for (let j = i + 1; j < lines.length; j += 1) {
245+
const line = lines[j] ?? "";
246+
if (line.trim() === "") break; // a blank line ends the sequence
247+
const item = /^\s*-\s*(.*?)\s*$/.exec(line);
248+
if (!item) break; // the next top-level key (or any non-`-` line) ends the sequence
249+
items.push(item[1] ?? "");
250+
}
251+
return items.length > 0 ? items : null;
252+
}
253+
return null;
254+
}
255+
231256
function isAbsoluteHttpUrl(url: string): boolean {
232257
try {
233258
const protocol = new URL(url).protocol;
@@ -244,8 +269,18 @@ export function extractSubmittedSourceUrls(
244269
const fields = parseSimpleFrontmatter(source);
245270
const urls: SubmittedSourceUrl[] = [];
246271
for (const field of spec.sourceUrlFields) {
247-
for (const url of scalarSourceUrlValues(fields[field] || "")) {
248-
urls.push({ field, url });
272+
const sequenceItems = scalarSequenceItems(source, field);
273+
if (sequenceItems) {
274+
// One URL per block-sequence item (mirrors listSourceUrlValues), so a multi-item scalar sequence is not
275+
// fused into one unparseable string; a single-item sequence still yields exactly one URL as before.
276+
for (const item of sequenceItems) {
277+
const url = unquoteYamlValue(item);
278+
if (url) urls.push({ field, url });
279+
}
280+
} else {
281+
for (const url of scalarSourceUrlValues(fields[field] || "")) {
282+
urls.push({ field, url });
283+
}
249284
}
250285
}
251286
urls.push(...listSourceUrlValues(source, spec));

test/unit/content-lane-source-evidence.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,24 @@ describe("checkSubmittedSourceEvidence", () => {
100100
expect(report.status).toBe("passed");
101101
});
102102

103+
it("REGRESSION (#9668): fetch-verifies BOTH URLs of a two-item scalar sequence instead of hard-failing the joined string", async () => {
104+
// Before the fix the two items were joined into "https://a.example/1, https://b.example/2", which new URL()
105+
// rejects → outcome invalid_url → the whole report "failed" on a submission whose sources are both live.
106+
const a = "https://a.example/1";
107+
const b = "https://b.example/2";
108+
const src = ["---", "documentationUrl:", ` - ${a}`, ` - ${b}`, "---", "", "body"].join("\n");
109+
const report = await checkSubmittedSourceEvidence(src, fakeFetch({ [a]: 200, [b]: 200 }));
110+
expect(report.urls.map((u) => u.url).sort()).toEqual([a, b]);
111+
expect(report.status).toBe("passed");
112+
});
113+
114+
it("still hard-fails a genuinely malformed scalar source value — the false-positive removal must not weaken the real check", async () => {
115+
const report = await checkSubmittedSourceEvidence(mdx({ documentationUrl: "notaurl" }), fakeFetch({}));
116+
const item = report.urls.find((u) => u.field === "documentationUrl") ?? report.warnings.find((u) => u.field === "documentationUrl");
117+
expect(item?.outcome).toBe("invalid_url");
118+
expect(item?.status).toBe("hard_failure");
119+
});
120+
103121
it("is retryable (not hard) on a 403/429/5xx canonical source", async () => {
104122
const src = mdx({ githubUrl: "https://github.com/acme/x" });
105123
const report = await checkSubmittedSourceEvidence(src, fakeFetch({ "https://github.com/acme/x": 403 }));
@@ -359,6 +377,21 @@ describe("extractSubmittedSourceUrls — frontmatter parsing edge cases", () =>
359377
expect(urls.map((u) => `${u.field}:${u.url}`)).toContain("documentationUrl:https://docs.acme.example/guide");
360378
});
361379

380+
it("regression (#9668): a scalar source field authored as a MULTI-item block sequence yields one URL per item", () => {
381+
const a = "https://a.example/1";
382+
const b = "https://b.example/2";
383+
const src = ["---", "documentationUrl:", ` - ${a}`, ` - ${b}`, "---", "", "body"].join("\n");
384+
const urls = extractSubmittedSourceUrls(src).filter((u) => u.field === "documentationUrl");
385+
expect(urls.map((u) => u.url)).toEqual([a, b]); // two distinct URLs, not one comma-joined string
386+
});
387+
388+
it("regression (#9668): a single-item scalar sequence is byte-identical — exactly one URL, its raw value", () => {
389+
const url = "https://docs.acme.example/only";
390+
const src = ["---", "documentationUrl:", ` - ${url}`, "---", "", "body"].join("\n");
391+
const urls = extractSubmittedSourceUrls(src).filter((u) => u.field === "documentationUrl");
392+
expect(urls.map((u) => u.url)).toEqual([url]);
393+
});
394+
362395
it("does not surface any block-scalar header on a list field as a bogus URL (both indicator orders)", () => {
363396
// `retrievalSources` is a list field; a block-scalar header is not a URL. The old guard only skipped the bare
364397
// `|`/`>`, so `|-` leaked as the literal url "|-". YAML allows chomping and the indentation digit in EITHER

0 commit comments

Comments
 (0)