From cc42e46140e73e62292f72b027b9fb7b0a31a57b Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Tue, 22 Sep 2026 20:08:40 -0700 Subject: [PATCH 01/14] Verify the live docs site after every Pages deployment A release PR always edits docs/guide/** (the {{reactorVersion}} substitutions), so the push that merges it matches the docs workflow's path filter, and the release tag then points at that same merge commit. Both runs hand actions/deploy-pages the same pages_build_version -- it is github.sha, with no input to override it -- so the two deployments collide under one identity and one is silently stranded. Cutting 0.1.0-preview.16 that is what happened: every job green, the deployment reporting success, gh-pages byte-correct, and the live site still serving preview.15 for 2h25m. Nothing in the pipeline looked at the live site, so no amount of green CI could have caught it. Two changes: A verify job now polls the deployed site and fails unless it is serving the artifact this run produced. Its oracle is deploy-stamp.json, written into the Pages artifact by publish and carrying the run id. Comparing versions.json alone would be vacuous for a main push, since mike deploy main republishes an existing version and leaves the version set unchanged. Every probe uses a unique ?nc= key so a pass cannot come from cache, and an already-published version fetched with the same request shape is the positive control that separates a stranded deploy (exit 1) from a broken probe (exit 2). A main push whose commit already carries a v* tag now stands down from deploying and leaves it to the tag run. publish still runs, so gh-pages is unaffected. A tag pushed after that check is invisible to it, so this narrows the race rather than closing it, which is why verify carries correctness. The decision logic is a pure function with 19 node cases; every comparison was mutation-checked, as were the five workflow seams asserted by DocsDeployWiringTests. Fixes: #1268 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff518ae2-0ea3-466c-a9c0-4ea2265937c5 --- .github/scripts/verify-docs-deployment.mjs | 327 ++++++++++++++++++ .../scripts/verify-docs-deployment.test.mjs | 316 +++++++++++++++++ .github/workflows/docs.yml | 109 ++++++ CHANGELOG.md | 14 + docs/contributing/release-runbook.md | 57 ++- .../DocsDeployWiringTests.cs | 155 +++++++++ .../DocsDeploymentVerifierTests.cs | 90 +++++ .../Reactor.DocPipeline.Tests.csproj | 4 + 8 files changed, 1071 insertions(+), 1 deletion(-) create mode 100644 .github/scripts/verify-docs-deployment.mjs create mode 100644 .github/scripts/verify-docs-deployment.test.mjs create mode 100644 tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs create mode 100644 tests/Reactor.DocPipeline.Tests/DocsDeploymentVerifierTests.cs diff --git a/.github/scripts/verify-docs-deployment.mjs b/.github/scripts/verify-docs-deployment.mjs new file mode 100644 index 000000000..dd3675ee0 --- /dev/null +++ b/.github/scripts/verify-docs-deployment.mjs @@ -0,0 +1,327 @@ +// Asserts that the GitHub Pages deployment *this* workflow run produced is the +// one the live docs site is actually serving. +// +// Why this exists (issue #1268). A release PR always edits docs/guide/**, which +// matches the `push: branches: [main]` path filter in docs.yml, and the release +// tag then points at that same merge commit. Both runs hand +// `actions/deploy-pages` the same `pages_build_version` — it is `github.sha`, +// and the action exposes no input to override it — so the two deployments +// collide under one identity and one of them is silently stranded. Every job +// goes green, the deployment reports success, and `gh-pages` is byte-correct; +// the only symptom is that the live site keeps showing the previous release. +// +// Nothing else in the pipeline ever looks at the live site, so no amount of +// green CI could catch that. This gate is cause-agnostic: it fails whenever the +// bytes on the live site did not come from this run, whatever stranded them. +// +// Run it locally against a deployed site with: +// DOCS_BASE_URL=https://microsoft.github.io/microsoft-ui-reactor/ \ +// DOCS_EXPECTED_RUN_ID=123 DOCS_EXPECTED_VERSIONS='[{"version":"main","aliases":[]}]' \ +// node .github/scripts/verify-docs-deployment.mjs +// +// The decision logic is a pure function so it can be tested without a network: +// see .github/scripts/verify-docs-deployment.test.mjs. + +import { randomUUID } from "node:crypto"; +import { pathToFileURL } from "node:url"; + +export const STAMP_PATH = "deploy-stamp.json"; +export const VERSIONS_PATH = "versions.json"; +export const LATEST_ALIAS = "latest"; + +/** + * @typedef {{ ok: boolean, status: number|null, body: string|null, error: string|null }} Probe + * @typedef {{ version: string, title?: string, aliases?: string[] }} VersionEntry + */ + +/** Human-readable shorthand for what a probe actually returned. */ +export function describeProbe(probe) { + if (!probe) return "not attempted"; + if (probe.error) return `request failed: ${probe.error}`; + return `HTTP ${probe.status}`; +} + +function parseJson(text) { + try { + return { value: JSON.parse(text ?? ""), error: null }; + } catch (err) { + return { value: null, error: err instanceof Error ? err.message : String(err) }; + } +} + +function aliasHolder(entries, alias) { + const match = entries.find((entry) => (entry?.aliases ?? []).includes(alias)); + return match ? match.version : null; +} + +/** + * Picks the two version directories worth fetching. + * + * `target` is the version this run most plausibly published — the one holding + * `latest`, else `main`, else whatever came first. `control` is any *other* + * published version, fetched with an identical request shape: it is the + * positive control that separates "the probe cannot see the site at all" from + * "the site is serving someone else's deployment". A no-match is not a + * measurement until the same probe is shown able to match. + */ +export function selectProbeTargets(expectedVersions) { + const entries = Array.isArray(expectedVersions) ? expectedVersions : []; + const target = + aliasHolder(entries, LATEST_ALIAS) ?? + entries.find((entry) => entry?.version === "main")?.version ?? + entries[0]?.version ?? + null; + const control = entries.find((entry) => entry?.version && entry.version !== target)?.version ?? null; + return { target, control }; +} + +/** + * Classifies one round of observations. Pure — no clock, no network. + * + * @returns {{ status: "pass"|"stranded"|"probe-broken", failures: {kind: string, message: string}[], evidence: string[], liveRunId: string|null }} + */ +export function evaluate({ expectedRunId, expectedVersions, observations }) { + const { stamp, versions, target, control } = observations; + const failures = []; + const evidence = []; + let liveRunId = null; + + // --- The per-run oracle. --------------------------------------------- + // Comparing the live version list alone is vacuous for a `main` push: + // `mike deploy main` republishes an existing version, so the version *set* + // is unchanged and the comparison passes whether or not this run's + // deployment ever landed. The stamp is what makes the check mean something + // on every trigger. + if (!stamp?.ok) { + failures.push({ + kind: "stamp-unreachable", + message: `${STAMP_PATH} did not respond 200 (${describeProbe(stamp)}).`, + }); + } else { + const parsed = parseJson(stamp.body); + if (parsed.error !== null) { + failures.push({ + kind: "stamp-unparseable", + message: `${STAMP_PATH} responded 200 but is not JSON (${parsed.error}).`, + }); + } else { + evidence.push(`${STAMP_PATH} responded 200 with parseable JSON`); + liveRunId = parsed.value?.run_id == null ? null : String(parsed.value.run_id); + if (liveRunId !== String(expectedRunId)) { + failures.push({ + kind: "stamp-stale", + message: + `The live site is serving run ${liveRunId ?? "(no run_id)"} ` + + `(sha ${parsed.value?.sha ?? "unknown"}), not this run ${expectedRunId}. ` + + "Two deployments collided under one pages_build_version and this one lost — see issue #1268.", + }); + } + } + } + + // --- Defence in depth: the version list and one rendered page. ------- + if (!versions?.ok) { + failures.push({ + kind: "versions-unreachable", + message: `${VERSIONS_PATH} did not respond 200 (${describeProbe(versions)}).`, + }); + } else { + const parsed = parseJson(versions.body); + if (parsed.error !== null || !Array.isArray(parsed.value)) { + failures.push({ + kind: "versions-unparseable", + message: + `${VERSIONS_PATH} responded 200 but is not a JSON array ` + + `(${parsed.error ?? "parsed to a non-array"}).`, + }); + } else { + evidence.push(`${VERSIONS_PATH} responded 200 with a parseable array`); + const live = parsed.value; + const liveNames = new Set(live.map((entry) => entry?.version)); + const missing = (expectedVersions ?? []) + .map((entry) => entry?.version) + .filter((name) => name && !liveNames.has(name)); + if (missing.length > 0) { + failures.push({ + kind: "versions-missing", + message: + `The live ${VERSIONS_PATH} is missing ${missing.join(", ")}. ` + + `It lists: ${[...liveNames].join(", ") || "(nothing)"}.`, + }); + } + + const expectedLatest = aliasHolder(expectedVersions ?? [], LATEST_ALIAS); + const liveLatest = aliasHolder(live, LATEST_ALIAS); + if (expectedLatest !== null && liveLatest !== expectedLatest) { + failures.push({ + kind: "latest-mismatch", + message: + `The live '${LATEST_ALIAS}' alias points at ${liveLatest ?? "nothing"}, ` + + `but this run published it on ${expectedLatest}.`, + }); + } + } + } + + if (target && !target.probe?.ok) { + failures.push({ + kind: "target-unreachable", + message: `${target.version}/ did not respond 200 (${describeProbe(target.probe)}).`, + }); + } else if (target) { + evidence.push(`${target.version}/ responded 200`); + } + + if (control?.probe?.ok) { + evidence.push(`the positive control ${control.version}/ responded 200`); + } + + if (failures.length === 0) { + return { status: "pass", failures, evidence, liveRunId }; + } + + // A probe that cannot see *any* known-good content proves nothing about the + // content it failed to find, so say so rather than blaming the deployment. + if (evidence.length === 0) { + return { status: "probe-broken", failures, evidence, liveRunId }; + } + return { status: "stranded", failures, evidence, liveRunId }; +} + +/** + * Fetches one URL with a unique cache-busting key, so a pass can never come + * from a CDN entry that predates this deployment. + * + * The base is normalised to end in `/` first: relative resolution drops the + * last path segment otherwise, so a `page_url` of + * `https://owner.github.io/repo` would send every probe to + * `https://owner.github.io/versions.json` and report the whole site missing. + */ +export async function probe(baseUrl, path, { fetchImpl = fetch, uuid = randomUUID } = {}) { + const base = baseUrl.endsWith("/") ? baseUrl : `${baseUrl}/`; + const url = new URL(path, base); + url.searchParams.set("nc", uuid()); + try { + const response = await fetchImpl(url.toString(), { cache: "no-store", redirect: "follow" }); + const body = await response.text(); + return { ok: response.status === 200, status: response.status, body, error: null }; + } catch (err) { + return { ok: false, status: null, body: null, error: err instanceof Error ? err.message : String(err) }; + } +} + +async function observe(baseUrl, expectedVersions, deps) { + const { target, control } = selectProbeTargets(expectedVersions); + const [stamp, versions, targetProbe, controlProbe] = await Promise.all([ + probe(baseUrl, STAMP_PATH, deps), + probe(baseUrl, VERSIONS_PATH, deps), + target ? probe(baseUrl, `${target}/`, deps) : Promise.resolve(null), + control ? probe(baseUrl, `${control}/`, deps) : Promise.resolve(null), + ]); + return { + stamp, + versions, + target: target ? { version: target, probe: targetProbe } : null, + control: control ? { version: control, probe: controlProbe } : null, + }; +} + +/** + * Polls the live site until it serves this run's artifact, or the window closes. + * + * Pages propagation is not instantaneous, so a single shot would turn ordinary + * latency into a red release. Polling on *any* non-pass verdict is deliberate: + * a stale stamp is exactly what a deployment still propagating looks like. + */ +export async function verifyDeployment({ + baseUrl, + expectedRunId, + expectedVersions, + timeoutMs = 600_000, + intervalMs = 15_000, + fetchImpl = fetch, + uuid = randomUUID, + now = () => Date.now(), + sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)), + log = console.log, +}) { + const deadline = now() + timeoutMs; + let attempt = 0; + let verdict; + + for (;;) { + attempt += 1; + const observations = await observe(baseUrl, expectedVersions, { fetchImpl, uuid }); + verdict = { ...evaluate({ expectedRunId, expectedVersions, observations }), attempt }; + + if (verdict.status === "pass") return verdict; + + if (now() >= deadline) return verdict; + + log( + `Attempt ${attempt}: ${verdict.status} — ${verdict.failures[0]?.message ?? "no detail"} ` + + `Retrying in ${Math.round(intervalMs / 1000)}s.`, + ); + await sleep(intervalMs); + } +} + +/** Renders a verdict as GitHub Actions annotations. Returns the process exit code. */ +export function report(verdict, { baseUrl, expectedRunId, log = console.log, err = console.error } = {}) { + if (verdict.status === "pass") { + log(`The live site at ${baseUrl} is serving run ${expectedRunId}.`); + for (const line of verdict.evidence) log(` ✓ ${line}`); + return 0; + } + + const headline = + verdict.status === "probe-broken" + ? `Could not read ${baseUrl} at all, so the deployment is unverified. ` + + "Not one probe returned known-good content, so this says nothing about the deployment itself — treat it as a broken check, not a stranded deploy." + : `${baseUrl} is not serving the artifact this run published (issue #1268). ` + + "Re-serve it by dispatching the Publish docs workflow on main — the artifact is the whole gh-pages branch, so any allowed ref republishes every version."; + + // Everything on one stream, so the annotation and the observations it rests + // on cannot interleave out of order in the run log. + err(`::error::${headline}`); + for (const failure of verdict.failures) err(`::error:: ${failure.kind}: ${failure.message}`); + for (const line of verdict.evidence) err(` ✓ ${line} — so the probe itself works`); + err(`Gave up after ${verdict.attempt} attempt(s).`); + + return verdict.status === "probe-broken" ? 2 : 1; +} + +function requireEnv(name) { + const value = process.env[name]; + if (!value) { + console.error(`::error::${name} is required.`); + process.exit(3); + } + return value; +} + +// `import.meta.main` is Node 24+; compare URLs so this also runs on Node 20/22. +// pathToFileURL rather than string concatenation: a Windows drive letter would +// otherwise parse as the URL host and never match. +const invokedDirectly = Boolean(process.argv[1]) && import.meta.url === pathToFileURL(process.argv[1]).href; +if (invokedDirectly) { + const baseUrl = requireEnv("DOCS_BASE_URL"); + const expectedRunId = requireEnv("DOCS_EXPECTED_RUN_ID"); + const rawVersions = requireEnv("DOCS_EXPECTED_VERSIONS"); + + const parsed = parseJson(rawVersions); + if (parsed.error !== null || !Array.isArray(parsed.value)) { + console.error(`::error::DOCS_EXPECTED_VERSIONS is not a JSON array: ${parsed.error ?? "parsed to a non-array"}`); + process.exit(3); + } + + const verdict = await verifyDeployment({ + baseUrl, + expectedRunId, + expectedVersions: parsed.value, + timeoutMs: Number(process.env.DOCS_VERIFY_TIMEOUT_SECONDS ?? 600) * 1000, + intervalMs: Number(process.env.DOCS_VERIFY_INTERVAL_SECONDS ?? 15) * 1000, + }); + + process.exit(report(verdict, { baseUrl, expectedRunId })); +} diff --git a/.github/scripts/verify-docs-deployment.test.mjs b/.github/scripts/verify-docs-deployment.test.mjs new file mode 100644 index 000000000..c638cca93 --- /dev/null +++ b/.github/scripts/verify-docs-deployment.test.mjs @@ -0,0 +1,316 @@ +// Regression tests for verify-docs-deployment.mjs — the gate that decides +// whether the live docs site is serving the artifact this workflow run +// published (issue #1268). +// +// Run directly with `node .github/scripts/verify-docs-deployment.test.mjs`, or +// let CI run it through DocsDeploymentVerifierTests in +// tests/Reactor.DocPipeline.Tests. +// +// Every case asserts a verdict the product code must *compute*: deleting any +// one of the four comparisons in evaluate() reddens at least one case here. + +import test from "node:test"; +import assert from "node:assert/strict"; + +import { + evaluate, + probe, + report, + selectProbeTargets, + verifyDeployment, +} from "./verify-docs-deployment.mjs"; + +const THIS_RUN = "35774072302"; +const OTHER_RUN = "35773982188"; + +const EXPECTED_VERSIONS = [ + { version: "main", title: "main (development)", aliases: [] }, + { version: "0.1.0-preview.16", title: "0.1.0-preview.16", aliases: ["latest"] }, + { version: "0.1.0-preview.15", title: "0.1.0-preview.15", aliases: [] }, +]; + +function ok(body) { + return { ok: true, status: 200, body, error: null }; +} + +function notFound() { + return { ok: false, status: 404, body: "", error: null }; +} + +function unreachable() { + return { ok: false, status: null, body: null, error: "getaddrinfo ENOTFOUND" }; +} + +/** A healthy live site: this run's stamp, the full version list, both pages. */ +function healthy(overrides = {}) { + const { target, control } = selectProbeTargets(EXPECTED_VERSIONS); + return { + stamp: ok(JSON.stringify({ run_id: THIS_RUN, sha: "459f7234" })), + versions: ok(JSON.stringify(EXPECTED_VERSIONS)), + target: { version: target, probe: ok("") }, + control: { version: control, probe: ok("") }, + ...overrides, + }; +} + +function verdictFor(observations, expectedVersions = EXPECTED_VERSIONS) { + return evaluate({ expectedRunId: THIS_RUN, expectedVersions, observations }); +} + +test("probe targets pick the latest holder and a distinct control", () => { + assert.deepEqual(selectProbeTargets(EXPECTED_VERSIONS), { + target: "0.1.0-preview.16", + control: "main", + }); +}); + +test("probe targets fall back to main when nothing holds latest", () => { + const unreleased = [{ version: "main", aliases: [] }]; + assert.deepEqual(selectProbeTargets(unreleased), { target: "main", control: null }); +}); + +test("a live site serving this run passes", () => { + const verdict = verdictFor(healthy()); + assert.equal(verdict.status, "pass"); + assert.deepEqual(verdict.failures, []); + assert.equal(verdict.liveRunId, THIS_RUN); +}); + +// The reported bug, exactly: gh-pages was byte-correct, the deployment reported +// success, and the live site was serving the *other* run's artifact. +test("a stamp from another run is stranded and names both runs", () => { + const verdict = verdictFor( + healthy({ stamp: ok(JSON.stringify({ run_id: OTHER_RUN, sha: "459f7234" })) }), + ); + assert.equal(verdict.status, "stranded"); + assert.deepEqual( + verdict.failures.map((f) => f.kind), + ["stamp-stale"], + ); + assert.match(verdict.failures[0].message, new RegExp(OTHER_RUN)); + assert.match(verdict.failures[0].message, new RegExp(THIS_RUN)); +}); + +test("a numeric run_id still matches the string this run is identified by", () => { + const verdict = verdictFor(healthy({ stamp: ok(JSON.stringify({ run_id: Number(THIS_RUN) })) })); + assert.equal(verdict.status, "pass"); +}); + +test("a site that has never served a stamp is stranded, not passing", () => { + const verdict = verdictFor(healthy({ stamp: notFound() })); + assert.equal(verdict.status, "stranded"); + assert.deepEqual( + verdict.failures.map((f) => f.kind), + ["stamp-unreachable"], + ); + assert.match(verdict.failures[0].message, /HTTP 404/); +}); + +test("a stamp that is not JSON is stranded", () => { + const verdict = verdictFor(healthy({ stamp: ok("404") })); + assert.equal(verdict.status, "stranded"); + assert.deepEqual( + verdict.failures.map((f) => f.kind), + ["stamp-unparseable"], + ); +}); + +test("a version this run published but the live site lacks is stranded", () => { + const stale = EXPECTED_VERSIONS.filter((v) => v.version !== "0.1.0-preview.16"); + const verdict = verdictFor(healthy({ versions: ok(JSON.stringify(stale)) })); + assert.equal(verdict.status, "stranded"); + assert.ok(verdict.failures.some((f) => f.kind === "versions-missing")); + assert.match( + verdict.failures.find((f) => f.kind === "versions-missing").message, + /0\.1\.0-preview\.16/, + ); +}); + +test("latest sitting on the wrong version is stranded", () => { + const dragged = EXPECTED_VERSIONS.map((v) => ({ + ...v, + aliases: v.version === "0.1.0-preview.15" ? ["latest"] : [], + })); + const verdict = verdictFor(healthy({ versions: ok(JSON.stringify(dragged)) })); + assert.equal(verdict.status, "stranded"); + assert.ok(verdict.failures.some((f) => f.kind === "latest-mismatch")); +}); + +test("a live site carrying extra newer versions still passes", () => { + const newer = [{ version: "0.1.0-preview.17", aliases: [] }, ...EXPECTED_VERSIONS]; + const verdict = verdictFor(healthy({ versions: ok(JSON.stringify(newer)) })); + assert.equal(verdict.status, "pass"); +}); + +test("a published version whose directory 404s is stranded", () => { + const { target } = selectProbeTargets(EXPECTED_VERSIONS); + const verdict = verdictFor(healthy({ target: { version: target, probe: notFound() } })); + assert.equal(verdict.status, "stranded"); + assert.deepEqual( + verdict.failures.map((f) => f.kind), + ["target-unreachable"], + ); +}); + +// The positive control. Without it, a probe that cannot reach the site at all +// is indistinguishable from a site that is serving the wrong deployment, and +// the gate would blame the release for a broken network. +test("a probe that sees no known-good content anywhere reports probe-broken", () => { + const { target, control } = selectProbeTargets(EXPECTED_VERSIONS); + const verdict = verdictFor({ + stamp: unreachable(), + versions: unreachable(), + target: { version: target, probe: unreachable() }, + control: { version: control, probe: unreachable() }, + }); + assert.equal(verdict.status, "probe-broken"); + assert.deepEqual(verdict.evidence, []); +}); + +test("a reachable versions.json keeps a stamp failure classified as stranded", () => { + const { target, control } = selectProbeTargets(EXPECTED_VERSIONS); + const verdict = verdictFor({ + stamp: notFound(), + versions: ok(JSON.stringify(EXPECTED_VERSIONS)), + target: { version: target, probe: notFound() }, + control: { version: control, probe: notFound() }, + }); + assert.equal(verdict.status, "stranded"); + assert.ok(verdict.evidence.length > 0); +}); + +test("the poll loop returns as soon as the deployment propagates", async () => { + let clock = 0; + let calls = 0; + const fetchImpl = async (url) => { + // The first round still serves the other run; the second serves this one. + const servedRun = calls < 4 ? OTHER_RUN : THIS_RUN; + calls += 1; + if (url.includes("deploy-stamp.json")) { + return { status: 200, text: async () => JSON.stringify({ run_id: servedRun }) }; + } + if (url.includes("versions.json")) { + return { status: 200, text: async () => JSON.stringify(EXPECTED_VERSIONS) }; + } + return { status: 200, text: async () => "" }; + }; + + const verdict = await verifyDeployment({ + baseUrl: "https://example.test/docs/", + expectedRunId: THIS_RUN, + expectedVersions: EXPECTED_VERSIONS, + timeoutMs: 60_000, + intervalMs: 1_000, + fetchImpl, + uuid: () => "fixed", + now: () => clock, + sleep: async (ms) => { + clock += ms; + }, + log: () => {}, + }); + + assert.equal(verdict.status, "pass"); + assert.equal(verdict.attempt, 2); +}); + +test("the poll loop gives up once the window closes", async () => { + let clock = 0; + const fetchImpl = async (url) => { + if (url.includes("deploy-stamp.json")) { + return { status: 200, text: async () => JSON.stringify({ run_id: OTHER_RUN }) }; + } + if (url.includes("versions.json")) { + return { status: 200, text: async () => JSON.stringify(EXPECTED_VERSIONS) }; + } + return { status: 200, text: async () => "" }; + }; + + const verdict = await verifyDeployment({ + baseUrl: "https://example.test/docs/", + expectedRunId: THIS_RUN, + expectedVersions: EXPECTED_VERSIONS, + timeoutMs: 3_000, + intervalMs: 1_000, + fetchImpl, + uuid: () => "fixed", + now: () => clock, + sleep: async (ms) => { + clock += ms; + }, + log: () => {}, + }); + + assert.equal(verdict.status, "stranded"); + assert.equal(verdict.attempt, 4); +}); + +// A cached response predating the deployment would make every check pass +// against content this run never produced, so the key must be both present and +// different on every request. +test("every probe carries a unique cache-busting key", async () => { + const seen = []; + let counter = 0; + const fetchImpl = async (url) => { + seen.push(url); + return { status: 200, text: async () => "{}" }; + }; + + await probe("https://example.test/docs/", "versions.json", { + fetchImpl, + uuid: () => `k${(counter += 1)}`, + }); + await probe("https://example.test/docs/", "versions.json", { + fetchImpl, + uuid: () => `k${(counter += 1)}`, + }); + + assert.deepEqual(seen, [ + "https://example.test/docs/versions.json?nc=k1", + "https://example.test/docs/versions.json?nc=k2", + ]); +}); + +// actions/deploy-pages happens to emit a trailing slash today, but relative URL +// resolution drops the last segment without one — every probe would silently +// move to the domain root and report the whole site missing. +test("a base URL without a trailing slash still resolves inside the site", async () => { + const seen = []; + const fetchImpl = async (url) => { + seen.push(url); + return { status: 200, text: async () => "{}" }; + }; + + await probe("https://owner.github.io/repo", "versions.json", { fetchImpl, uuid: () => "k" }); + await probe("https://owner.github.io/repo/", "0.1.0-preview.16/", { fetchImpl, uuid: () => "k" }); + + assert.deepEqual(seen, [ + "https://owner.github.io/repo/versions.json?nc=k", + "https://owner.github.io/repo/0.1.0-preview.16/?nc=k", + ]); +}); + +test("a transport failure becomes a probe result rather than a throw", async () => { const result = await probe("https://example.test/docs/", "versions.json", { + fetchImpl: async () => { + throw new Error("socket hang up"); + }, + uuid: () => "fixed", + }); + assert.deepEqual(result, { ok: false, status: null, body: null, error: "socket hang up" }); +}); + +test("exit codes separate a stranded deploy from a broken probe", () => { + const silence = () => {}; + assert.equal(report({ ...verdictFor(healthy()), attempt: 1 }, { log: silence, err: silence }), 0); + assert.equal( + report({ ...verdictFor(healthy({ stamp: notFound() })), attempt: 1 }, { log: silence, err: silence }), + 1, + ); + const broken = verdictFor({ + stamp: unreachable(), + versions: unreachable(), + target: null, + control: null, + }); + assert.equal(report({ ...broken, attempt: 1 }, { log: silence, err: silence }), 2); +}); diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index a9e6b5ed6..e72ab7346 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -46,6 +46,12 @@ env: jobs: publish: runs-on: ubuntu-latest + outputs: + # Whether this run owns the Pages deployment, and the exact version list + # the verify job must find on the live site. See the two steps that set + # them for why each exists. + deploy: ${{ steps.deploy-gate.outputs.deploy }} + versions: ${{ steps.artifact.outputs.versions }} permissions: contents: write # mike commits the rendered site to gh-pages steps: @@ -183,7 +189,43 @@ jobs: mike set-default --branch "$MIKE_BRANCH" --push "${highest_backfilled#v}" fi + - name: Decide whether this run owns the deployment + id: deploy-gate + run: | + # A release PR always edits docs/guide/** (the {{reactorVersion}} + # substitutions), so the push that merges it matches this workflow's + # path filter — and the release tag then points at that same merge + # commit. Both runs hand actions/deploy-pages the same + # `pages_build_version`: it is github.sha, and the action exposes no + # input to override it. So the two deployments collide under one + # identity and one of them is silently stranded (issue #1268). + # + # The tag run is the one that publishes the release and moves the + # `latest` alias, so it keeps the deployment and the branch push + # stands down. `publish` still runs either way — it is what writes the + # `main` version to gh-pages, and the tag run's artifact is the whole + # branch, so nothing is lost by not serving it from here. + # + # This narrows the window rather than closing it: a tag pushed after + # this step has already looked is invisible to it, and both runs + # deploy again. That residual race is what the verify job below is + # for — it turns a stranded deployment from silent into red. + if [ "$GITHUB_EVENT_NAME" = "push" ] && [ "$GITHUB_REF_TYPE" = "branch" ]; then + # Checked as late as possible, against the freshest tag list, so the + # window between "main's run looked" and "the tag was pushed" is as + # small as the job length allows. + git fetch --tags --force --quiet origin || echo "Could not refresh tags; falling back to the fetched set." + tags="$(git tag --points-at "$GITHUB_SHA" --list 'v*' | tr '\n' ' ')" + if [ -n "$tags" ]; then + echo "::notice::$GITHUB_SHA is already tagged ($tags) — leaving the deployment to the tag run so the two cannot collide (issue #1268)." + echo "deploy=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + fi + echo "deploy=true" >> "$GITHUB_OUTPUT" + - name: Assemble the Pages artifact from every published version + id: artifact # `git archive` rather than a checkout: it materialises the branch # without leaving git metadata in the directory that gets uploaded. run: | @@ -209,12 +251,45 @@ jobs: echo "Publishing these versions:" cat site/versions.json + # Stamps the artifact with the run that produced it, so the verify job + # can tell *this* deployment from any other one serving the same site. + # + # Comparing versions.json alone would be vacuous for a main push: + # `mike deploy main` republishes an existing version, so the version + # set is unchanged and the comparison passes whether or not this run's + # deployment ever landed. The run id is what makes the check mean + # something on every trigger. + # + # Written here rather than committed to gh-pages for the same reason + # 404.html is: it is a property of a deployment, not of a published + # version. Named without a leading `.` or `_` deliberately — Pages has + # historically excluded those, and an unservable stamp would fail + # every run. + jq -n \ + --arg run_id "$GITHUB_RUN_ID" \ + --arg run_attempt "$GITHUB_RUN_ATTEMPT" \ + --arg run_url "$GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" \ + --arg sha "$GITHUB_SHA" \ + --arg ref "$GITHUB_REF" \ + --arg published_at "$(date -u +%Y-%m-%dT%H:%M:%SZ)" \ + '{run_id: $run_id, run_attempt: $run_attempt, run_url: $run_url, sha: $sha, ref: $ref, published_at: $published_at}' \ + > site/deploy-stamp.json + cat site/deploy-stamp.json + + # Compact, because a job output is a single line. + echo "versions=$(jq -c . site/versions.json)" >> "$GITHUB_OUTPUT" + - uses: actions/upload-pages-artifact@fc324d3547104276b827a68afc52ff2a11cc49c9 # v5.0.0 with: path: site deploy: needs: publish + # Skipped when the pushed commit already carries a release tag: the tag run + # owns the deployment, so the two cannot collide under one + # pages_build_version. See the "Decide whether this run owns the + # deployment" step above (issue #1268). + if: needs.publish.outputs.deploy == 'true' runs-on: ubuntu-latest permissions: pages: write @@ -222,6 +297,40 @@ jobs: environment: name: github-pages url: ${{ steps.deployment.outputs.page_url }} + outputs: + page_url: ${{ steps.deployment.outputs.page_url }} steps: - id: deployment uses: actions/deploy-pages@368f82528645a54fb793d4d04e342629a3f51346 # v5.0.1 + + verify: + name: Verify the live deployment + needs: [publish, deploy] + runs-on: ubuntu-latest + permissions: + # Only to check out the verifier script. The job then reads a public site + # over HTTPS and writes nothing. + contents: read + # Comfortably above the poll window below, so a hung request surfaces as a + # timeout on this job rather than an open-ended run. + timeout-minutes: 15 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: '22.13.0' + + # `deploy` reporting success is not evidence that the site serves what + # this run built: a deployment can collide with another under the same + # pages_build_version and be stranded while every job stays green + # (issue #1268). This is the only step in the pipeline that looks at the + # live site, so it is the only one that can catch that. + - name: Assert the live site is serving this run's artifact + env: + DOCS_BASE_URL: ${{ needs.deploy.outputs.page_url }} + DOCS_EXPECTED_RUN_ID: ${{ github.run_id }} + DOCS_EXPECTED_VERSIONS: ${{ needs.publish.outputs.versions }} + DOCS_VERIFY_TIMEOUT_SECONDS: '600' + DOCS_VERIFY_INTERVAL_SECONDS: '15' + run: node .github/scripts/verify-docs-deployment.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 42c660613..f74cc094d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,8 +28,22 @@ Conventions for contributors: ### Added +- `Publish docs` now verifies the live site after deploying. The `publish` job stamps the + Pages artifact with the run that built it, and a new `verify` job polls + until it is serving *that* run — + failing the workflow otherwise. Every probe carries a unique query key so a pass cannot + come from cached content, and an already-published version fetched with the same request + shape acts as a positive control, so a broken probe is reported as unverified rather than + blamed on the deployment. (issue #1268) + ### Changed +- A push to `main` no longer deploys the docs when the pushed commit already carries a `v*` + tag; the release tag's own run owns the deployment. Both runs previously handed + `actions/deploy-pages` the same `pages_build_version` (it is `github.sha`, with no input + to override it), so the two deployments collided under one identity and one was silently + stranded. `publish` still runs on both, so `gh-pages` is unaffected. (issue #1268) + ### Deprecated ### Removed diff --git a/docs/contributing/release-runbook.md b/docs/contributing/release-runbook.md index 8669f69fa..e2817551c 100644 --- a/docs/contributing/release-runbook.md +++ b/docs/contributing/release-runbook.md @@ -179,7 +179,9 @@ After pushing the tag: 1. Confirm the GitHub `Package` workflow runs for the tag and creates a GitHub Release. 2. Confirm the OneBranch official pipeline starts for the tag. -3. Confirm the `Publish docs` workflow runs for the tag and that the new version is selectable at (see [Versioned documentation site](#versioned-documentation-site)). Check **both** of its jobs: a green `publish` beside a red `deploy` means the environment refused the tag ref, and the site stays stale even though `gh-pages` is already correct — see [Which refs may deploy](#which-refs-may-deploy). +3. Confirm the `Publish docs` workflow runs for the tag and that the new version is selectable at (see [Versioned documentation site](#versioned-documentation-site)). Check **all three** of its jobs: + - a green `publish` beside a red `deploy` means the environment refused the tag ref, and the site stays stale even though `gh-pages` is already correct — see [Which refs may deploy](#which-refs-may-deploy); + - a green `deploy` beside a red `verify` means the deployment reported success but the live site is serving something else — see [Why a release commit deploys only once](#why-a-release-commit-deploys-only-once). 4. Approve the `Production_PublishNuGet` stage when ready to publish to NuGet.org. 5. Verify the packages appear on NuGet.org. 6. Install the released template package or a locally packed template and create a smoke app that restores against NuGet.org. @@ -201,6 +203,9 @@ Nothing about this is manual on the happy path. `.github/workflows/docs.yml` han | Push to `main` touching the docs | Republishes the `main (development)` version | | Push of a `v*` tag | Publishes ``, moves the `latest` alias to it, and repoints the site root | +Both write to `gh-pages`, but only one of them *serves* a release commit — see +[Why a release commit deploys only once](#why-a-release-commit-deploys-only-once). + The site root redirects to whichever version holds the `latest` alias, so readers landing on the bare URL always get the newest release rather than unreleased `main`. Every version that does *not* hold `latest` renders the outdated-version banner defined in @@ -249,6 +254,56 @@ Once the policy admits the ref, dispatch `Publish docs` on `main` to serve the s release: the artifact is the whole `gh-pages` branch, so a deploy from any allowed ref publishes every version already committed to it. +### Why a release commit deploys only once + +A release PR always edits `docs/guide/**` — the `{{reactorVersion}}` substitutions — so the +push that merges it matches the workflow's path filter, and the release tag then points at +that same merge commit. Left alone, that produces **two** `Publish docs` runs with an +identical `github.sha`. + +That is a problem because `actions/deploy-pages` sends `pages_build_version = github.sha` +and exposes no input to override it, so both runs create a Pages deployment under one +identity. One wins and the other is silently stranded: every job green, the deployment +reporting `success`, `gh-pages` byte-correct — and the live site still showing the previous +release. That is exactly what happened cutting 0.1.0-preview.16, and it went unnoticed for +2h25m (issue #1268). + +Two things now prevent it: + +- The `publish` job checks whether the pushed commit already carries a `v*` tag and, if so, + **stands down from deploying** — the tag run owns the deployment. `publish` itself still + runs, because it is what writes the `main` version to `gh-pages`; only the serving step is + skipped, and the tag run's artifact is the whole branch, so nothing is lost. +- The `verify` job then polls the live site and asserts that the artifact being served is + the one **this run** produced. That is the part that carries correctness: a tag pushed + after `publish` has already looked is invisible to the check above, so the race is + narrowed rather than closed. + +`verify` distinguishes two failures, and they call for different responses: + +| Annotation | Meaning | What to do | +| --- | --- | --- | +| `…is not serving the artifact this run published` | The deployment was stranded. It names the run id the live site *is* serving. | Dispatch `Publish docs` on `main` (below). | +| `Could not read … at all, so the deployment is unverified` | Not one probe reached known-good content, so the run says nothing about the deployment. | Treat it as a broken check: confirm the site is up, then re-run the job. | + +Every probe uses a unique `?nc=` query key, so a pass cannot come from a cached response, +and an already-published version is fetched with the identical request shape as a positive +control — a no-match is not a measurement until the same probe is shown able to match. + +**The remedy, verified.** Dispatch `Publish docs` on `main`. The `publish` job assembles the +artifact from the whole `gh-pages` branch, so any allowed ref re-serves every published +version. With no `backfill_tags` the release step is skipped, and `mike set-default` runs +only `if ! mike list latest` — `latest` already exists — so it **cannot drag `latest` +backwards**. This is what restored preview.16. + +The gate's own oracle is `deploy-stamp.json` at the site root: `publish` writes it into the +artifact with the run id that built it, and `verify` asserts the live copy matches. It is +refreshed from the workflow on every deploy rather than committed to `gh-pages`, for the +same reason `404.html` is — it describes a deployment, not a published version. Comparing +`versions.json` alone would not do: `mike deploy main` republishes an existing version, so +on a `main` push the version set is unchanged and the comparison would pass whether or not +the deployment ever landed. + ### Legacy unversioned links Before versioning, pages lived at unversioned paths such as diff --git a/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs b/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs new file mode 100644 index 000000000..9c138ec42 --- /dev/null +++ b/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs @@ -0,0 +1,155 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using Xunit; +using YamlDotNet.RepresentationModel; + +namespace Microsoft.UI.Reactor.Cli.Docs.Tests; + +/// +/// Structural assertions on .github/workflows/docs.yml, covering the +/// wiring that turns a stranded Pages deployment from silent into red +/// (issue #1268). +/// +/// The bug these guard against is not "the gate reports the wrong answer" — the +/// node suite in covers that — it is +/// "the gate is still present but no longer connected to anything". Each seam +/// below fails open if it is quietly removed: without the stamp the verifier +/// compares a version list that a main push never changes; without +/// needs: deploy the verifier reads the site before the deployment; and +/// without github.run_id as the expected value, the comparison it makes +/// is a tautology. A workflow file is not covered by any compiler, so these are +/// asserted here rather than assumed. +/// +public sealed class DocsDeployWiringTests +{ + private static readonly YamlMappingNode Workflow = LoadWorkflow(); + private static readonly YamlMappingNode Jobs = Map(Workflow, "jobs"); + + [Fact] + public void Publish_exposes_the_deploy_decision_and_the_published_version_list() + { + var outputs = Map(Map(Jobs, "publish"), "outputs"); + + Assert.Equal("${{ steps.deploy-gate.outputs.deploy }}", Scalar(outputs, "deploy")); + Assert.Equal("${{ steps.artifact.outputs.versions }}", Scalar(outputs, "versions")); + } + + [Fact] + public void Publish_decides_whether_this_run_owns_the_deployment() + { + var run = StepRun("publish", "deploy-gate"); + + // The whole point of the step: a merge commit that already carries a + // release tag must leave the deployment to the tag run, so the two + // cannot collide under one pages_build_version. + Assert.Contains("git tag --points-at", run, StringComparison.Ordinal); + Assert.Contains("deploy=false", run, StringComparison.Ordinal); + Assert.Contains("deploy=true", run, StringComparison.Ordinal); + } + + [Fact] + public void The_artifact_carries_a_stamp_identifying_the_run_that_built_it() + { + var run = StepRun("publish", "artifact"); + + // Without the stamp the gate falls back to comparing versions.json, + // which a `main` push never changes — so the check would pass whether + // or not this run's deployment ever landed. + // + // Matched as a redirect, not as a bare mention: the step also cats the + // file back out for the log, and a substring check alone stays green + // when the write itself is removed. + Assert.Matches(@">\s*site/deploy-stamp\.json", run); + Assert.Contains("$GITHUB_RUN_ID", run, StringComparison.Ordinal); + + // Named without a leading `.` or `_`: Pages has historically excluded + // those, and an unservable stamp would fail every run. + Assert.DoesNotContain("site/.deploy-stamp", run, StringComparison.Ordinal); + Assert.DoesNotContain("site/_deploy-stamp", run, StringComparison.Ordinal); + + Assert.Contains("versions=", run, StringComparison.Ordinal); + Assert.Contains("$GITHUB_OUTPUT", run, StringComparison.Ordinal); + } + + [Fact] + public void Deploy_stands_down_when_publish_says_the_tag_run_owns_it() + { + var deploy = Map(Jobs, "deploy"); + + Assert.Equal("needs.publish.outputs.deploy == 'true'", Scalar(deploy, "if")); + Assert.Equal("${{ steps.deployment.outputs.page_url }}", Scalar(Map(deploy, "outputs"), "page_url")); + } + + [Fact] + public void A_verify_job_runs_after_the_deployment_and_reads_the_live_site() + { + var verify = Map(Jobs, "verify"); + + var needs = ((YamlSequenceNode)verify.Children["needs"]).Children + .Select(n => ((YamlScalarNode)n).Value) + .ToArray(); + Assert.Contains("publish", needs); + Assert.Contains("deploy", needs); + + var step = Steps("verify").Single(s => Scalar(s, "run")?.Contains("verify-docs-deployment.mjs", StringComparison.Ordinal) == true); + var env = Map(step, "env"); + + Assert.Equal("${{ needs.deploy.outputs.page_url }}", Scalar(env, "DOCS_BASE_URL")); + Assert.Equal("${{ github.run_id }}", Scalar(env, "DOCS_EXPECTED_RUN_ID")); + Assert.Equal("${{ needs.publish.outputs.versions }}", Scalar(env, "DOCS_EXPECTED_VERSIONS")); + } + + [Fact] + public void The_verifier_and_its_regression_suite_are_present() + { + var repoRoot = FindRepoRoot(); + foreach (var name in new[] { "verify-docs-deployment.mjs", "verify-docs-deployment.test.mjs" }) + { + var path = Path.Join(repoRoot, ".github", "scripts", name); + Assert.True(File.Exists(path), $"Missing {path}, which docs.yml runs after every Pages deployment."); + } + } + + private static IEnumerable Steps(string job) => + ((YamlSequenceNode)Map(Jobs, job).Children["steps"]).Children.Cast(); + + private static string StepRun(string job, string stepId) + { + var step = Steps(job).SingleOrDefault(s => Scalar(s, "id") == stepId); + Assert.NotNull(step); + var run = Scalar(step!, "run"); + Assert.False(string.IsNullOrWhiteSpace(run), $"Step '{stepId}' in job '{job}' has no run block."); + return run!; + } + + private static YamlMappingNode Map(YamlMappingNode parent, string key) + { + Assert.True(parent.Children.ContainsKey(new YamlScalarNode(key)), $"Expected a '{key}' mapping."); + return (YamlMappingNode)parent.Children[new YamlScalarNode(key)]; + } + + private static string? Scalar(YamlMappingNode parent, string key) => + parent.Children.TryGetValue(new YamlScalarNode(key), out var value) ? ((YamlScalarNode)value).Value : null; + + private static YamlMappingNode LoadWorkflow() + { + var path = Path.Join(FindRepoRoot(), ".github", "workflows", "docs.yml"); + var stream = new YamlStream(); + stream.Load(new StringReader(File.ReadAllText(path))); + return (YamlMappingNode)stream.Documents[0].RootNode; + } + + private static string FindRepoRoot() + { + var dir = Directory.GetCurrentDirectory(); + while (dir is not null) + { + if (File.Exists(Path.Join(dir, "Reactor.slnx")) || Directory.Exists(Path.Join(dir, ".git"))) + return dir; + dir = Path.GetDirectoryName(dir); + } + throw new InvalidOperationException("Reactor repo root not found from test cwd."); + } +} diff --git a/tests/Reactor.DocPipeline.Tests/DocsDeploymentVerifierTests.cs b/tests/Reactor.DocPipeline.Tests/DocsDeploymentVerifierTests.cs new file mode 100644 index 000000000..3dcd6ec5b --- /dev/null +++ b/tests/Reactor.DocPipeline.Tests/DocsDeploymentVerifierTests.cs @@ -0,0 +1,90 @@ +using System; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.UI.Reactor.Cli.Docs.Tests; + +/// +/// Runs the regression suite for .github/scripts/verify-docs-deployment.mjs, +/// the gate that decides whether the live docs site is serving the artifact a +/// Publish docs run produced (issue #1268). +/// +/// That script is the only thing in the pipeline that looks at the live site. +/// Everything else — the strict build, the mike push, the Pages deployment — +/// stayed green while a release was stranded, so a mistake in this one file is +/// a mistake in the only signal that can catch the bug it exists for. +/// +/// The cases live in JavaScript next to the script so a release engineer can +/// run them with plain node; this test exists so CI runs them too. It is +/// wired into the docs-build job, which is armed whenever a non-Markdown +/// file changes — and both the script and its test script qualify. +/// +public sealed class DocsDeploymentVerifierTests +{ + [Fact] + public async Task Deployment_verifier_cases_pass() + { + var repoRoot = FindRepoRoot(); + var testScript = Path.Join(repoRoot, ".github", "scripts", "verify-docs-deployment.test.mjs"); + + Assert.True( + File.Exists(testScript), + $"Missing {testScript}. The docs-deployment gate is unverified without it."); + + using var process = Process.Start(new ProcessStartInfo + { + FileName = "node", + // TAP explicitly: node's default reporter varies with the node + // version and whether stdout is a TTY, and the completeness check + // below reads the summary counters it emits. + ArgumentList = { "--test-reporter=tap", testScript }, + WorkingDirectory = repoRoot, + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false, + CreateNoWindow = true, + })!; + + var cancellationToken = TestContext.Current.CancellationToken; + + // Start both reads before awaiting exit: a child that fills one pipe + // buffer blocks on the write while the parent waits for it to exit. + var stdoutTask = process.StandardOutput.ReadToEndAsync(cancellationToken); + var stderrTask = process.StandardError.ReadToEndAsync(cancellationToken); + await process.WaitForExitAsync(cancellationToken); + + var stdout = await stdoutTask; + var output = string.Join( + Environment.NewLine, + new[] { stdout, await stderrTask }.Where(s => !string.IsNullOrWhiteSpace(s))); + + // Fails rather than skips when node is absent, for the same reason + // SiteRootRedirectTests does: every GitHub-hosted runner ships node, so + // a missing interpreter is a broken environment — and a test that + // quietly opts out when its fixture fails is a test that cannot fail. + Assert.True( + process.ExitCode == 0, + $".github/scripts/verify-docs-deployment.test.mjs failed (exit {process.ExitCode}):{Environment.NewLine}{output}"); + + // An exit code of zero is also what `node` returns for a file that + // registered no tests at all, so assert the suite actually ran. Without + // this, renaming the cases out from under the runner would read green. + Assert.Contains("# pass ", stdout, StringComparison.Ordinal); + Assert.DoesNotContain("# pass 0", stdout, StringComparison.Ordinal); + } + + private static string FindRepoRoot() + { + var dir = Directory.GetCurrentDirectory(); + while (dir is not null) + { + if (File.Exists(Path.Join(dir, "Reactor.slnx")) || Directory.Exists(Path.Join(dir, ".git"))) + return dir; + dir = Path.GetDirectoryName(dir); + } + throw new InvalidOperationException("Reactor repo root not found from test cwd."); + } +} diff --git a/tests/Reactor.DocPipeline.Tests/Reactor.DocPipeline.Tests.csproj b/tests/Reactor.DocPipeline.Tests/Reactor.DocPipeline.Tests.csproj index c6fb85c4f..1574b5ba9 100644 --- a/tests/Reactor.DocPipeline.Tests/Reactor.DocPipeline.Tests.csproj +++ b/tests/Reactor.DocPipeline.Tests/Reactor.DocPipeline.Tests.csproj @@ -21,6 +21,10 @@ + + all runtime; build; native; contentfiles; analyzers From 1e001c4b43f5345af766f4789f793d971f28375a Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 23 Sep 2026 12:06:18 -0700 Subject: [PATCH 02/14] Address code-quality findings in DocsDeployWiringTests Three findings from github-code-quality on DocsDeployWiringTests.cs: - LoadWorkflow leaked a StringReader; it is now a using declaration. - Map did a ContainsKey lookup followed by an indexer lookup; it now uses a single TryGetValue. The missing-key path still fails with the same assertion message rather than returning null, confirmed by mutating the publish job's outputs key out of docs.yml. - The_verifier_and_its_regression_suite_are_present mapped its loop variable on the first line of the body; the projection moved to a Select. No assertion changed. 469/469 doc-pipeline tests pass and the five workflow-seam mutations still redden their intended cases. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff518ae2-0ea3-466c-a9c0-4ea2265937c5 --- .../DocsDeployWiringTests.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs b/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs index 9c138ec42..c913186b5 100644 --- a/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs +++ b/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs @@ -105,9 +105,11 @@ public void A_verify_job_runs_after_the_deployment_and_reads_the_live_site() public void The_verifier_and_its_regression_suite_are_present() { var repoRoot = FindRepoRoot(); - foreach (var name in new[] { "verify-docs-deployment.mjs", "verify-docs-deployment.test.mjs" }) + var paths = new[] { "verify-docs-deployment.mjs", "verify-docs-deployment.test.mjs" } + .Select(name => Path.Join(repoRoot, ".github", "scripts", name)); + + foreach (var path in paths) { - var path = Path.Join(repoRoot, ".github", "scripts", name); Assert.True(File.Exists(path), $"Missing {path}, which docs.yml runs after every Pages deployment."); } } @@ -126,8 +128,9 @@ private static string StepRun(string job, string stepId) private static YamlMappingNode Map(YamlMappingNode parent, string key) { - Assert.True(parent.Children.ContainsKey(new YamlScalarNode(key)), $"Expected a '{key}' mapping."); - return (YamlMappingNode)parent.Children[new YamlScalarNode(key)]; + var found = parent.Children.TryGetValue(new YamlScalarNode(key), out var value); + Assert.True(found, $"Expected a '{key}' mapping."); + return (YamlMappingNode)value!; } private static string? Scalar(YamlMappingNode parent, string key) => @@ -137,7 +140,8 @@ private static YamlMappingNode LoadWorkflow() { var path = Path.Join(FindRepoRoot(), ".github", "workflows", "docs.yml"); var stream = new YamlStream(); - stream.Load(new StringReader(File.ReadAllText(path))); + using var reader = new StringReader(File.ReadAllText(path)); + stream.Load(reader); return (YamlMappingNode)stream.Documents[0].RootNode; } From 656845745b3065e619dcb1c7f8de098c859d33ba Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 23 Sep 2026 12:17:21 -0700 Subject: [PATCH 03/14] Queue pending docs runs so a release tag run cannot be evicted Copilot review finding on #1269: the deploy stand-down could lose the only release deployment. The publish job stands main down when the pushed commit already carries a v* tag, on the assumption that the tag run will deploy. Under the Actions default concurrency behaviour that assumption does not hold. \queue: single\ keeps at most one pending run per group and cancels the previous one when a new run queues, so a docs push landing while the tag run waited behind main would evict the tag run before it ever started. The release version would then never reach gh-pages, and main had already deferred its own deployment to a run that no longer existed, so neither the deployment nor its verification happened. Setting queue: max makes runs wait in FIFO order (up to 100 pending) instead of evicting each other, which makes the stand-down sound rather than speculative. cancel-in-progress is dropped: false is the default, and combining queue: max with cancel-in-progress: true is a workflow validation error. Because the two are now coupled, DocsDeployWiringTests asserts the queue setting; removing it would silently re-arm the failure. Both mutations (reverting to the default queue, and adding the illegal cancel-in-progress) redden that test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff518ae2-0ea3-466c-a9c0-4ea2265937c5 --- .github/workflows/docs.yml | 21 ++++++++++++++++++- CHANGELOG.md | 5 +++++ docs/contributing/release-runbook.md | 10 +++++++++ .../DocsDeployWiringTests.cs | 17 +++++++++++++++ 4 files changed, 52 insertions(+), 1 deletion(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index e72ab7346..c9a84dba9 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -34,9 +34,22 @@ permissions: {} # Serialises Pages deployments *and* the gh-pages pushes below, so two runs can # never race for the same branch. +# +# `queue: max` is load-bearing, not a tidiness knob. The default (`single`) +# keeps at most one *pending* run per group and cancels the previous one when a +# new run queues. During a release that is reachable and costly: the merge's +# `main` run holds the group, the release tag's run waits behind it, and any +# further docs push to `main` would evict the tag run before it ever ran. The +# release version would then never be written to gh-pages at all, and the +# "Decide whether this run owns the deployment" step below would have already +# stood `main` down in favour of a run that no longer exists. Queuing makes the +# tag run wait its turn instead (FIFO, up to 100 pending). +# +# `cancel-in-progress` is deliberately absent: it defaults to false, and +# `queue: max` with `cancel-in-progress: true` is a workflow validation error. concurrency: group: pages - cancel-in-progress: false + queue: max env: MIKE_BRANCH: gh-pages @@ -206,6 +219,12 @@ jobs: # `main` version to gh-pages, and the tag run's artifact is the whole # branch, so nothing is lost by not serving it from here. # + # Standing down is only safe because the concurrency block above sets + # `queue: max`. Under the default `single`, a later docs push could + # evict the still-pending tag run, and this step would have deferred + # to a run that never happens — losing the release deployment and its + # verification together. Do not remove one without the other. + # # This narrows the window rather than closing it: a tag pushed after # this step has already looked is invisible to it, and both runs # deploy again. That residual race is what the verify job below is diff --git a/CHANGELOG.md b/CHANGELOG.md index f74cc094d..6415f1f76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,11 @@ Conventions for contributors: `actions/deploy-pages` the same `pages_build_version` (it is `github.sha`, with no input to override it), so the two deployments collided under one identity and one was silently stranded. `publish` still runs on both, so `gh-pages` is unaffected. (issue #1268) +- The `Publish docs` concurrency group now sets `queue: max`. The Actions default keeps at + most one *pending* run per group and cancels the previous one when a new run queues, so a + docs push landing while a release tag's run waited behind `main` would evict the tag run + before it started — leaving the release unpublished. Runs now wait in FIFO order. + (issue #1268) ### Deprecated diff --git a/docs/contributing/release-runbook.md b/docs/contributing/release-runbook.md index e2817551c..33dc63207 100644 --- a/docs/contributing/release-runbook.md +++ b/docs/contributing/release-runbook.md @@ -279,6 +279,16 @@ Two things now prevent it: after `publish` has already looked is invisible to the check above, so the race is narrowed rather than closed. +**The stand-down depends on the concurrency queue.** The workflow's concurrency block sets +`queue: max`. Under the Actions default (`queue: single`) only one run may be *pending* per +group, and a newly queued run cancels the previous pending one. During a release that is +reachable: the merge's `main` run holds the group, the tag run waits behind it, and any +further docs push to `main` evicts the tag run before it ever starts. The release version +would then never reach `gh-pages` at all, and `main` would already have deferred its own +deployment to a run that no longer exists — losing the release and its verification +together. `queue: max` makes runs wait in FIFO order instead. Removing it silently +re-arms that failure, which is why `DocsDeployWiringTests` asserts it. + `verify` distinguishes two failures, and they call for different responses: | Annotation | Meaning | What to do | diff --git a/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs b/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs index c913186b5..4bd25dd1b 100644 --- a/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs +++ b/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs @@ -36,6 +36,23 @@ public void Publish_exposes_the_deploy_decision_and_the_published_version_list() Assert.Equal("${{ steps.artifact.outputs.versions }}", Scalar(outputs, "versions")); } + [Fact] + public void Pending_runs_queue_instead_of_evicting_each_other() + { + var concurrency = Map(Workflow, "concurrency"); + + // Paired with the stand-down below, and unsafe without it. Under the + // default `queue: single` a later docs push evicts a still-pending tag + // run, so `publish` would defer the deployment to a run that never + // happens and the release would be lost with nothing going red. + Assert.Equal("max", Scalar(concurrency, "queue")); + + // `queue: max` plus `cancel-in-progress: true` is a workflow validation + // error, which would take the whole workflow offline rather than fail + // one job. + Assert.NotEqual("true", Scalar(concurrency, "cancel-in-progress")); + } + [Fact] public void Publish_decides_whether_this_run_owns_the_deployment() { From 8b7c3d6d500ccc9cbd59d5eb05b4a524771b6e59 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 23 Sep 2026 12:36:59 -0700 Subject: [PATCH 04/14] Probe the versions a docs run actually published, not just latest Copilot review finding on #1269: the verifier could miss a broken newly published version. selectProbeTargets picked the version holding the latest alias, so it never touched the directory a run had just written whenever those differ. Two reachable cases: publishing a backported or re-cut tag deliberately does not move latest (see the Publish the release version step), and a main push republishes main while latest sits on a release. In both, a 404 or otherwise broken new directory passed as long as versions.json listed it and the stamp matched. Every publishing step now appends the version it wrote to \/\, the artifact step turns that into a published job output, and the verify job passes it as DOCS_PUBLISHED_VERSIONS. The verifier probes each of those plus the latest alias holder, keeping a distinct control. DOCS_PUBLISHED_VERSIONS is required rather than defaulted, and the artifact step hard-fails on an empty list, so a wiring mistake is loud instead of quietly narrowing the gate back to the alias holder. Probes now request /index.html rather than the bare directory. That was found by the local end-to-end check reporting a pass it should not have: python http.server answers a directory with no index by generating a 200 listing, while Pages 404s it, so the harness masked the very break it was meant to show. Against real gh-pages bytes with 0.1.0-preview.14/index.html removed, the old selection exits 0 and the new one exits 1 naming the 404. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff518ae2-0ea3-466c-a9c0-4ea2265937c5 --- .github/scripts/verify-docs-deployment.mjs | 124 ++++++++++++------ .../scripts/verify-docs-deployment.test.mjs | 109 +++++++++++---- .github/workflows/docs.yml | 29 ++++ .../DocsDeployWiringTests.cs | 22 ++++ 4 files changed, 224 insertions(+), 60 deletions(-) diff --git a/.github/scripts/verify-docs-deployment.mjs b/.github/scripts/verify-docs-deployment.mjs index dd3675ee0..a6c9a70ea 100644 --- a/.github/scripts/verify-docs-deployment.mjs +++ b/.github/scripts/verify-docs-deployment.mjs @@ -17,6 +17,7 @@ // Run it locally against a deployed site with: // DOCS_BASE_URL=https://microsoft.github.io/microsoft-ui-reactor/ \ // DOCS_EXPECTED_RUN_ID=123 DOCS_EXPECTED_VERSIONS='[{"version":"main","aliases":[]}]' \ +// DOCS_PUBLISHED_VERSIONS='["main"]' \ // node .github/scripts/verify-docs-deployment.mjs // // The decision logic is a pure function so it can be tested without a network: @@ -29,6 +30,19 @@ export const STAMP_PATH = "deploy-stamp.json"; export const VERSIONS_PATH = "versions.json"; export const LATEST_ALIAS = "latest"; +/** + * The page a version probe fetches. + * + * Explicitly `index.html` rather than the bare directory: a bare directory is + * only equivalent on a server that has no directory listing. GitHub Pages 404s + * a directory whose index is missing, but a plain static server answers 200 + * with a generated listing — which quietly turns a broken deployment into a + * passing probe when this gate is exercised locally. + */ +function versionIndex(version) { + return `${version}/index.html`; +} + /** * @typedef {{ ok: boolean, status: number|null, body: string|null, error: string|null }} Probe * @typedef {{ version: string, title?: string, aliases?: string[] }} VersionEntry @@ -55,24 +69,46 @@ function aliasHolder(entries, alias) { } /** - * Picks the two version directories worth fetching. + * Picks the version directories worth fetching. + * + * `targets` are the versions this run actually published, because those are the + * ones whose bytes are new on the live site. Probing only the `latest` holder + * would miss them: publishing a backported tag deliberately does not move + * `latest` (see the "Publish the release version" step in docs.yml), and a + * `main` push republishes `main` while `latest` sits on a release. In both + * cases a broken new directory would pass as long as `versions.json` listed it. + * The `latest` holder is probed as well, since it is what the site root serves. * - * `target` is the version this run most plausibly published — the one holding - * `latest`, else `main`, else whatever came first. `control` is any *other* - * published version, fetched with an identical request shape: it is the - * positive control that separates "the probe cannot see the site at all" from - * "the site is serving someone else's deployment". A no-match is not a - * measurement until the same probe is shown able to match. + * `control` is any *other* published version, fetched with an identical request + * shape: it is the positive control that separates "the probe cannot see the + * site at all" from "the site is serving someone else's deployment". A no-match + * is not a measurement until the same probe is shown able to match. */ -export function selectProbeTargets(expectedVersions) { +export function selectProbeTargets(expectedVersions, publishedVersions = []) { const entries = Array.isArray(expectedVersions) ? expectedVersions : []; - const target = - aliasHolder(entries, LATEST_ALIAS) ?? - entries.find((entry) => entry?.version === "main")?.version ?? - entries[0]?.version ?? - null; - const control = entries.find((entry) => entry?.version && entry.version !== target)?.version ?? null; - return { target, control }; + const known = new Set(entries.map((entry) => entry?.version).filter(Boolean)); + const published = (Array.isArray(publishedVersions) ? publishedVersions : []).filter((v) => known.has(v)); + const latest = aliasHolder(entries, LATEST_ALIAS); + + const targets = []; + for (const version of published) { + if (!targets.includes(version)) targets.push(version); + } + + // Only reached when the caller could not say what it published; keeps the + // gate meaningful rather than probing nothing at all. + if (targets.length === 0) { + const fallback = + latest ?? entries.find((entry) => entry?.version === "main")?.version ?? entries[0]?.version ?? null; + if (fallback) targets.push(fallback); + } + + if (latest && !targets.includes(latest)) targets.push(latest); + + const control = + entries.map((entry) => entry?.version).find((version) => version && !targets.includes(version)) ?? null; + + return { targets, control }; } /** @@ -81,7 +117,7 @@ export function selectProbeTargets(expectedVersions) { * @returns {{ status: "pass"|"stranded"|"probe-broken", failures: {kind: string, message: string}[], evidence: string[], liveRunId: string|null }} */ export function evaluate({ expectedRunId, expectedVersions, observations }) { - const { stamp, versions, target, control } = observations; + const { stamp, versions, control } = observations; const failures = []; const evidence = []; let liveRunId = null; @@ -163,13 +199,15 @@ export function evaluate({ expectedRunId, expectedVersions, observations }) { } } - if (target && !target.probe?.ok) { - failures.push({ - kind: "target-unreachable", - message: `${target.version}/ did not respond 200 (${describeProbe(target.probe)}).`, - }); - } else if (target) { - evidence.push(`${target.version}/ responded 200`); + for (const target of observations.targets ?? []) { + if (!target.probe?.ok) { + failures.push({ + kind: "target-unreachable", + message: `${target.version}/ did not respond 200 (${describeProbe(target.probe)}).`, + }); + } else { + evidence.push(`${target.version}/ responded 200`); + } } if (control?.probe?.ok) { @@ -210,19 +248,19 @@ export async function probe(baseUrl, path, { fetchImpl = fetch, uuid = randomUUI } } -async function observe(baseUrl, expectedVersions, deps) { - const { target, control } = selectProbeTargets(expectedVersions); - const [stamp, versions, targetProbe, controlProbe] = await Promise.all([ +async function observe(baseUrl, expectedVersions, publishedVersions, deps) { + const { targets, control } = selectProbeTargets(expectedVersions, publishedVersions); + const [stamp, versions, ...rest] = await Promise.all([ probe(baseUrl, STAMP_PATH, deps), probe(baseUrl, VERSIONS_PATH, deps), - target ? probe(baseUrl, `${target}/`, deps) : Promise.resolve(null), - control ? probe(baseUrl, `${control}/`, deps) : Promise.resolve(null), + ...targets.map((version) => probe(baseUrl, versionIndex(version), deps)), + ...(control ? [probe(baseUrl, versionIndex(control), deps)] : []), ]); return { stamp, versions, - target: target ? { version: target, probe: targetProbe } : null, - control: control ? { version: control, probe: controlProbe } : null, + targets: targets.map((version, i) => ({ version, probe: rest[i] })), + control: control ? { version: control, probe: rest[targets.length] } : null, }; } @@ -237,6 +275,7 @@ export async function verifyDeployment({ baseUrl, expectedRunId, expectedVersions, + publishedVersions = [], timeoutMs = 600_000, intervalMs = 15_000, fetchImpl = fetch, @@ -251,7 +290,7 @@ export async function verifyDeployment({ for (;;) { attempt += 1; - const observations = await observe(baseUrl, expectedVersions, { fetchImpl, uuid }); + const observations = await observe(baseUrl, expectedVersions, publishedVersions, { fetchImpl, uuid }); verdict = { ...evaluate({ expectedRunId, expectedVersions, observations }), attempt }; if (verdict.status === "pass") return verdict; @@ -300,6 +339,15 @@ function requireEnv(name) { return value; } +function requireJsonArrayEnv(name) { + const parsed = parseJson(requireEnv(name)); + if (parsed.error !== null || !Array.isArray(parsed.value)) { + console.error(`::error::${name} is not a JSON array: ${parsed.error ?? "parsed to a non-array"}`); + process.exit(3); + } + return parsed.value; +} + // `import.meta.main` is Node 24+; compare URLs so this also runs on Node 20/22. // pathToFileURL rather than string concatenation: a Windows drive letter would // otherwise parse as the URL host and never match. @@ -307,18 +355,18 @@ const invokedDirectly = Boolean(process.argv[1]) && import.meta.url === pathToFi if (invokedDirectly) { const baseUrl = requireEnv("DOCS_BASE_URL"); const expectedRunId = requireEnv("DOCS_EXPECTED_RUN_ID"); - const rawVersions = requireEnv("DOCS_EXPECTED_VERSIONS"); + const expectedVersions = requireJsonArrayEnv("DOCS_EXPECTED_VERSIONS"); - const parsed = parseJson(rawVersions); - if (parsed.error !== null || !Array.isArray(parsed.value)) { - console.error(`::error::DOCS_EXPECTED_VERSIONS is not a JSON array: ${parsed.error ?? "parsed to a non-array"}`); - process.exit(3); - } + // Required rather than defaulted: without it the probe silently falls back to + // the `latest` holder, which is exactly the blind spot that lets a broken + // backported-tag or `main` directory pass. A wiring mistake should be loud. + const publishedVersions = requireJsonArrayEnv("DOCS_PUBLISHED_VERSIONS"); const verdict = await verifyDeployment({ baseUrl, expectedRunId, - expectedVersions: parsed.value, + expectedVersions, + publishedVersions, timeoutMs: Number(process.env.DOCS_VERIFY_TIMEOUT_SECONDS ?? 600) * 1000, intervalMs: Number(process.env.DOCS_VERIFY_INTERVAL_SECONDS ?? 15) * 1000, }); diff --git a/.github/scripts/verify-docs-deployment.test.mjs b/.github/scripts/verify-docs-deployment.test.mjs index c638cca93..249e686d1 100644 --- a/.github/scripts/verify-docs-deployment.test.mjs +++ b/.github/scripts/verify-docs-deployment.test.mjs @@ -29,6 +29,9 @@ const EXPECTED_VERSIONS = [ { version: "0.1.0-preview.15", title: "0.1.0-preview.15", aliases: [] }, ]; +// What a release-tag run publishes: the tag holds `latest` because it is newest. +const PUBLISHED_RELEASE = ["0.1.0-preview.16"]; + function ok(body) { return { ok: true, status: 200, body, error: null }; } @@ -41,14 +44,14 @@ function unreachable() { return { ok: false, status: null, body: null, error: "getaddrinfo ENOTFOUND" }; } -/** A healthy live site: this run's stamp, the full version list, both pages. */ -function healthy(overrides = {}) { - const { target, control } = selectProbeTargets(EXPECTED_VERSIONS); +/** A healthy live site: this run's stamp, the full version list, every page. */ +function healthy(overrides = {}, published = PUBLISHED_RELEASE) { + const { targets, control } = selectProbeTargets(EXPECTED_VERSIONS, published); return { stamp: ok(JSON.stringify({ run_id: THIS_RUN, sha: "459f7234" })), versions: ok(JSON.stringify(EXPECTED_VERSIONS)), - target: { version: target, probe: ok("") }, - control: { version: control, probe: ok("") }, + targets: targets.map((version) => ({ version, probe: ok("") })), + control: control ? { version: control, probe: ok("") } : null, ...overrides, }; } @@ -57,16 +60,45 @@ function verdictFor(observations, expectedVersions = EXPECTED_VERSIONS) { return evaluate({ expectedRunId: THIS_RUN, expectedVersions, observations }); } -test("probe targets pick the latest holder and a distinct control", () => { - assert.deepEqual(selectProbeTargets(EXPECTED_VERSIONS), { - target: "0.1.0-preview.16", +test("probe targets cover the published version and the latest holder", () => { + assert.deepEqual(selectProbeTargets(EXPECTED_VERSIONS, PUBLISHED_RELEASE), { + targets: ["0.1.0-preview.16"], control: "main", }); }); +// A backported tag publishes its version without moving `latest`, so probing +// only the alias holder would never touch the directory this run just wrote. +test("a backported tag is probed alongside the untouched latest holder", () => { + const { targets, control } = selectProbeTargets(EXPECTED_VERSIONS, ["0.1.0-preview.15"]); + assert.deepEqual(targets, ["0.1.0-preview.15", "0.1.0-preview.16"]); + assert.equal(control, "main"); +}); + +test("a main push probes main, which does not hold latest", () => { + const { targets } = selectProbeTargets(EXPECTED_VERSIONS, ["main"]); + assert.deepEqual(targets, ["main", "0.1.0-preview.16"]); +}); + +test("a backfill probes every version it published", () => { + const { targets } = selectProbeTargets(EXPECTED_VERSIONS, ["0.1.0-preview.15", "0.1.0-preview.16"]); + assert.deepEqual(targets, ["0.1.0-preview.15", "0.1.0-preview.16"]); +}); + +test("probe targets fall back to the latest holder when nothing was reported", () => { + assert.deepEqual(selectProbeTargets(EXPECTED_VERSIONS, []).targets, ["0.1.0-preview.16"]); +}); + test("probe targets fall back to main when nothing holds latest", () => { const unreleased = [{ version: "main", aliases: [] }]; - assert.deepEqual(selectProbeTargets(unreleased), { target: "main", control: null }); + assert.deepEqual(selectProbeTargets(unreleased, []), { targets: ["main"], control: null }); +}); + +test("a published version the live site never listed is not probed blindly", () => { + // Guards the set intersection: probing a name absent from versions.json would + // report a 404 that the versions-missing check already explains better. + const { targets } = selectProbeTargets(EXPECTED_VERSIONS, ["0.9.9-never-published"]); + assert.deepEqual(targets, ["0.1.0-preview.16"]); }); test("a live site serving this run passes", () => { @@ -143,24 +175,49 @@ test("a live site carrying extra newer versions still passes", () => { }); test("a published version whose directory 404s is stranded", () => { - const { target } = selectProbeTargets(EXPECTED_VERSIONS); - const verdict = verdictFor(healthy({ target: { version: target, probe: notFound() } })); + const { targets } = selectProbeTargets(EXPECTED_VERSIONS, PUBLISHED_RELEASE); + const verdict = verdictFor( + healthy({ targets: targets.map((version) => ({ version, probe: notFound() })) }), + ); + assert.equal(verdict.status, "stranded"); + assert.deepEqual( + verdict.failures.map((f) => f.kind), + ["target-unreachable"], + ); +}); + +// The reviewer's case: a backported tag does not move `latest`, so a gate that +// only probed the alias holder would pass while the directory this run just +// published was broken. +test("a broken backported directory is stranded even though latest is fine", () => { + const published = ["0.1.0-preview.15"]; + const { targets, control } = selectProbeTargets(EXPECTED_VERSIONS, published); + const verdict = verdictFor({ + stamp: ok(JSON.stringify({ run_id: THIS_RUN })), + versions: ok(JSON.stringify(EXPECTED_VERSIONS)), + targets: targets.map((version) => ({ + version, + probe: version === "0.1.0-preview.15" ? notFound() : ok(""), + })), + control: control ? { version: control, probe: ok("") } : null, + }); assert.equal(verdict.status, "stranded"); assert.deepEqual( verdict.failures.map((f) => f.kind), ["target-unreachable"], ); + assert.match(verdict.failures[0].message, /0\.1\.0-preview\.15/); }); // The positive control. Without it, a probe that cannot reach the site at all // is indistinguishable from a site that is serving the wrong deployment, and // the gate would blame the release for a broken network. test("a probe that sees no known-good content anywhere reports probe-broken", () => { - const { target, control } = selectProbeTargets(EXPECTED_VERSIONS); + const { targets, control } = selectProbeTargets(EXPECTED_VERSIONS, PUBLISHED_RELEASE); const verdict = verdictFor({ stamp: unreachable(), versions: unreachable(), - target: { version: target, probe: unreachable() }, + targets: targets.map((version) => ({ version, probe: unreachable() })), control: { version: control, probe: unreachable() }, }); assert.equal(verdict.status, "probe-broken"); @@ -168,11 +225,11 @@ test("a probe that sees no known-good content anywhere reports probe-broken", () }); test("a reachable versions.json keeps a stamp failure classified as stranded", () => { - const { target, control } = selectProbeTargets(EXPECTED_VERSIONS); + const { targets, control } = selectProbeTargets(EXPECTED_VERSIONS, PUBLISHED_RELEASE); const verdict = verdictFor({ stamp: notFound(), versions: ok(JSON.stringify(EXPECTED_VERSIONS)), - target: { version: target, probe: notFound() }, + targets: targets.map((version) => ({ version, probe: notFound() })), control: { version: control, probe: notFound() }, }); assert.equal(verdict.status, "stranded"); @@ -181,12 +238,14 @@ test("a reachable versions.json keeps a stamp failure classified as stranded", ( test("the poll loop returns as soon as the deployment propagates", async () => { let clock = 0; - let calls = 0; + let stampFetches = 0; + const requested = []; const fetchImpl = async (url) => { - // The first round still serves the other run; the second serves this one. - const servedRun = calls < 4 ? OTHER_RUN : THIS_RUN; - calls += 1; + requested.push(url); if (url.includes("deploy-stamp.json")) { + // The first round still serves the other run; the second serves this one. + stampFetches += 1; + const servedRun = stampFetches === 1 ? OTHER_RUN : THIS_RUN; return { status: 200, text: async () => JSON.stringify({ run_id: servedRun }) }; } if (url.includes("versions.json")) { @@ -199,6 +258,7 @@ test("the poll loop returns as soon as the deployment propagates", async () => { baseUrl: "https://example.test/docs/", expectedRunId: THIS_RUN, expectedVersions: EXPECTED_VERSIONS, + publishedVersions: PUBLISHED_RELEASE, timeoutMs: 60_000, intervalMs: 1_000, fetchImpl, @@ -212,6 +272,10 @@ test("the poll loop returns as soon as the deployment propagates", async () => { assert.equal(verdict.status, "pass"); assert.equal(verdict.attempt, 2); + + // The driver must actually request the version this run published, not just + // decide it should have. + assert.ok(requested.some((url) => url.includes("/0.1.0-preview.16/index.html?nc="))); }); test("the poll loop gives up once the window closes", async () => { @@ -230,6 +294,7 @@ test("the poll loop gives up once the window closes", async () => { baseUrl: "https://example.test/docs/", expectedRunId: THIS_RUN, expectedVersions: EXPECTED_VERSIONS, + publishedVersions: PUBLISHED_RELEASE, timeoutMs: 3_000, intervalMs: 1_000, fetchImpl, @@ -282,11 +347,11 @@ test("a base URL without a trailing slash still resolves inside the site", async }; await probe("https://owner.github.io/repo", "versions.json", { fetchImpl, uuid: () => "k" }); - await probe("https://owner.github.io/repo/", "0.1.0-preview.16/", { fetchImpl, uuid: () => "k" }); + await probe("https://owner.github.io/repo/", "0.1.0-preview.16/index.html", { fetchImpl, uuid: () => "k" }); assert.deepEqual(seen, [ "https://owner.github.io/repo/versions.json?nc=k", - "https://owner.github.io/repo/0.1.0-preview.16/?nc=k", + "https://owner.github.io/repo/0.1.0-preview.16/index.html?nc=k", ]); }); @@ -309,7 +374,7 @@ test("exit codes separate a stranded deploy from a broken probe", () => { const broken = verdictFor({ stamp: unreachable(), versions: unreachable(), - target: null, + targets: [], control: null, }); assert.equal(report({ ...broken, attempt: 1 }, { log: silence, err: silence }), 2); diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index c9a84dba9..781237d5c 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -55,6 +55,15 @@ env: MIKE_BRANCH: gh-pages LATEST_ALIAS: latest MAIN_VERSION: main + # Every publishing step appends the version it wrote to + # "$RUNNER_TEMP/$PUBLISHED_VERSIONS_FILE", and the artifact step turns that + # into a job output. The verify job probes exactly these directories: probing + # only the `latest` holder would skip a backported tag (which deliberately + # does not move `latest`) and skip `main` on a branch push, so a broken new + # directory could pass on the strength of its versions.json entry alone. + # Held as a bare filename because workflow-level env cannot expand + # $RUNNER_TEMP; every use joins the two. + PUBLISHED_VERSIONS_FILE: published-versions.txt jobs: publish: @@ -65,6 +74,7 @@ jobs: # them for why each exists. deploy: ${{ steps.deploy-gate.outputs.deploy }} versions: ${{ steps.artifact.outputs.versions }} + published: ${{ steps.artifact.outputs.published }} permissions: contents: write # mike commits the rendered site to gh-pages steps: @@ -104,6 +114,7 @@ jobs: run: | mike deploy --branch "$MIKE_BRANCH" --push --alias-type copy \ --title 'main (development)' "$MAIN_VERSION" + echo "$MAIN_VERSION" >> "$RUNNER_TEMP/$PUBLISHED_VERSIONS_FILE" # Until a release version exists there is nothing for the site root to # redirect to, which would leave it a 404. Point the root at main for @@ -120,6 +131,7 @@ jobs: run: | version="${GITHUB_REF_NAME#v}" newest="$(git tag --list 'v*' --sort=-v:refname | head -n 1)" + echo "$version" >> "$RUNNER_TEMP/$PUBLISHED_VERSIONS_FILE" if [ "$GITHUB_REF_NAME" = "$newest" ]; then mike deploy --branch "$MIKE_BRANCH" --push --alias-type copy \ @@ -180,6 +192,7 @@ jobs: else mike deploy --branch "$MIKE_BRANCH" --push --alias-type copy "${tag#v}" fi + echo "${tag#v}" >> "$RUNNER_TEMP/$PUBLISHED_VERSIONS_FILE" echo "::endgroup::" done @@ -298,6 +311,21 @@ jobs: # Compact, because a job output is a single line. echo "versions=$(jq -c . site/versions.json)" >> "$GITHUB_OUTPUT" + # The versions this run actually wrote, for the verify job to probe. + # Every trigger path publishes at least one (main push, release tag, + # or backfill), so an empty list means a publishing step stopped + # recording and the gate would quietly fall back to probing only the + # `latest` holder. Fail instead of verifying less than we think. + published_file="$RUNNER_TEMP/$PUBLISHED_VERSIONS_FILE" + if [ ! -s "$published_file" ]; then + echo "::error::No publishing step recorded a version in $published_file, so the verify job cannot know what to probe. Every trigger path must append the version it published." + exit 1 + fi + + published="$(jq -R -s -c 'split("\n") | map(select(length > 0)) | unique' < "$published_file")" + echo "This run published: $published" + echo "published=$published" >> "$GITHUB_OUTPUT" + - uses: actions/upload-pages-artifact@fc324d3547104276b827a68afc52ff2a11cc49c9 # v5.0.0 with: path: site @@ -350,6 +378,7 @@ jobs: DOCS_BASE_URL: ${{ needs.deploy.outputs.page_url }} DOCS_EXPECTED_RUN_ID: ${{ github.run_id }} DOCS_EXPECTED_VERSIONS: ${{ needs.publish.outputs.versions }} + DOCS_PUBLISHED_VERSIONS: ${{ needs.publish.outputs.published }} DOCS_VERIFY_TIMEOUT_SECONDS: '600' DOCS_VERIFY_INTERVAL_SECONDS: '15' run: node .github/scripts/verify-docs-deployment.mjs diff --git a/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs b/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs index 4bd25dd1b..f24b82bad 100644 --- a/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs +++ b/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs @@ -88,6 +88,27 @@ public void The_artifact_carries_a_stamp_identifying_the_run_that_built_it() Assert.Contains("versions=", run, StringComparison.Ordinal); Assert.Contains("$GITHUB_OUTPUT", run, StringComparison.Ordinal); + + // The versions this run published drive which directories verify + // probes. Without them the probe silently narrows to the `latest` + // holder, which never covers a backported tag or a `main` push. + Assert.Matches(@"published=", run); + Assert.Contains("PUBLISHED_VERSIONS_FILE", run, StringComparison.Ordinal); + } + + [Theory] + [InlineData("Publish the main version")] + [InlineData("Publish the release version")] + [InlineData("Backfill release versions")] + public void Every_publishing_step_records_what_it_published(string stepName) + { + var step = Steps("publish").Single(s => Scalar(s, "name") == stepName); + var run = Scalar(step, "run")!; + + // The artifact step hard-fails on an empty list, so a step that stops + // recording turns into a red run rather than a quieter gate — but only + // if every step records in the first place. + Assert.Contains("$RUNNER_TEMP/$PUBLISHED_VERSIONS_FILE", run, StringComparison.Ordinal); } [Fact] @@ -116,6 +137,7 @@ public void A_verify_job_runs_after_the_deployment_and_reads_the_live_site() Assert.Equal("${{ needs.deploy.outputs.page_url }}", Scalar(env, "DOCS_BASE_URL")); Assert.Equal("${{ github.run_id }}", Scalar(env, "DOCS_EXPECTED_RUN_ID")); Assert.Equal("${{ needs.publish.outputs.versions }}", Scalar(env, "DOCS_EXPECTED_VERSIONS")); + Assert.Equal("${{ needs.publish.outputs.published }}", Scalar(env, "DOCS_PUBLISHED_VERSIONS")); } [Fact] From e8a549684b7d9824a80a34d6a7b9f3fb4fb7cf1a Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 23 Sep 2026 12:57:31 -0700 Subject: [PATCH 05/14] Probe the site root and fail on a published/versions.json divergence Three Copilot review findings on #1269. 1. The published list and mike's versions.json are produced independently, so selectProbeTargets intersecting them could silently shrink the probe set on exactly the run that needed it. The artifact step now fails when a recorded version is absent from site/versions.json, the verifier no longer filters, and evaluate reports published-version-unlisted. The release step also recorded its version before mike deploy ran; it now records after. 2. Nothing probed the site root, which mike writes only via set-default and no version directory covers, so a stale or 404 root passed. The verifier now fetches the root index.html, asserts 200, and asserts it still forwards to the expected default (latest, else main, else unjudged rather than guessed). 3. The stamp-stale message asserted a cause the gate cannot observe. It now states the mismatch and leaves the collision explanation to the runbook. The jq for (1) was wrong on first write: inside select() the dot rebinds to the element, so map(.version) ran against a string and the filter errored, which made the guard fire on the passing case too. Caught by running the real step against origin/gh-pages rather than assuming; replaced with array difference and re-tested consistent, divergent, and empty inputs (exit 0, 1, 1). Root probing verified against real bytes as well: healthy passes, and rewriting the root stub to forward to main/ instead of latest/ exits 1 with root-mistargeted. All five new comparisons are mutation-checked, including one that initially survived because no test proved the driver actually requests the root URL. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff518ae2-0ea3-466c-a9c0-4ea2265937c5 --- .github/scripts/verify-docs-deployment.mjs | 74 ++++++++++++++++-- .../scripts/verify-docs-deployment.test.mjs | 76 ++++++++++++++++--- .github/workflows/docs.yml | 18 ++++- docs/contributing/release-runbook.md | 9 +++ .../DocsDeployWiringTests.cs | 6 ++ 5 files changed, 166 insertions(+), 17 deletions(-) diff --git a/.github/scripts/verify-docs-deployment.mjs b/.github/scripts/verify-docs-deployment.mjs index a6c9a70ea..e985ac132 100644 --- a/.github/scripts/verify-docs-deployment.mjs +++ b/.github/scripts/verify-docs-deployment.mjs @@ -28,6 +28,7 @@ import { pathToFileURL } from "node:url"; export const STAMP_PATH = "deploy-stamp.json"; export const VERSIONS_PATH = "versions.json"; +export const ROOT_INDEX_PATH = "index.html"; export const LATEST_ALIAS = "latest"; /** @@ -86,10 +87,13 @@ function aliasHolder(entries, alias) { */ export function selectProbeTargets(expectedVersions, publishedVersions = []) { const entries = Array.isArray(expectedVersions) ? expectedVersions : []; - const known = new Set(entries.map((entry) => entry?.version).filter(Boolean)); - const published = (Array.isArray(publishedVersions) ? publishedVersions : []).filter((v) => known.has(v)); + const published = Array.isArray(publishedVersions) ? publishedVersions.filter(Boolean) : []; const latest = aliasHolder(entries, LATEST_ALIAS); + // Deliberately not intersected with `expectedVersions`. A recorded version + // that the site does not list is a real inconsistency, and dropping it here + // would shrink the probe set on exactly the run that needs it most; evaluate() + // reports it instead. const targets = []; for (const version of published) { if (!targets.includes(version)) targets.push(version); @@ -111,13 +115,29 @@ export function selectProbeTargets(expectedVersions, publishedVersions = []) { return { targets, control }; } +/** + * The version or alias the site root should redirect to. + * + * mike's `set-default` writes a root index.html that forwards to whichever + * version or alias is default — `latest` once any release exists, and `main` + * during the window before the first one. Returns null when neither is + * published, because the workflow's own fallback is ambiguous there and a + * guessed expectation would be worse than none. + */ +export function expectedRootTarget(expectedVersions) { + const entries = Array.isArray(expectedVersions) ? expectedVersions : []; + if (aliasHolder(entries, LATEST_ALIAS)) return LATEST_ALIAS; + if (entries.some((entry) => entry?.version === "main")) return "main"; + return null; +} + /** * Classifies one round of observations. Pure — no clock, no network. * * @returns {{ status: "pass"|"stranded"|"probe-broken", failures: {kind: string, message: string}[], evidence: string[], liveRunId: string|null }} */ -export function evaluate({ expectedRunId, expectedVersions, observations }) { - const { stamp, versions, control } = observations; +export function evaluate({ expectedRunId, expectedVersions, publishedVersions = [], observations }) { + const { stamp, versions, control, root } = observations; const failures = []; const evidence = []; let liveRunId = null; @@ -149,7 +169,7 @@ export function evaluate({ expectedRunId, expectedVersions, observations }) { message: `The live site is serving run ${liveRunId ?? "(no run_id)"} ` + `(sha ${parsed.value?.sha ?? "unknown"}), not this run ${expectedRunId}. ` + - "Two deployments collided under one pages_build_version and this one lost — see issue #1268.", + "Whatever stranded it, the bytes this run published are not the bytes being served.", }); } } @@ -199,6 +219,44 @@ export function evaluate({ expectedRunId, expectedVersions, observations }) { } } + // The publishing steps and mike produce these two lists independently, so a + // name in one and not the other means the run does not know what it wrote. + // The workflow fails earlier on this, which makes reaching it here a sign + // that the earlier guard was bypassed rather than a routine outcome. + const declared = new Set((expectedVersions ?? []).map((entry) => entry?.version)); + const unlisted = (publishedVersions ?? []).filter((version) => version && !declared.has(version)); + if (unlisted.length > 0) { + failures.push({ + kind: "published-version-unlisted", + message: + `This run reported publishing ${unlisted.join(", ")}, but the artifact's version list does not contain ` + + `${unlisted.length === 1 ? "it" : "them"}. The two are produced independently, so they must agree.`, + }); + } + + // The URL readers actually land on. mike writes it only via `set-default`, + // and it is the one page no version directory covers, so a stale or broken + // root is invisible to every other check here. + const rootTarget = expectedRootTarget(expectedVersions); + if (root) { + if (!root.probe?.ok) { + failures.push({ + kind: "root-unreachable", + message: `The site root did not respond 200 (${describeProbe(root.probe)}).`, + }); + } else { + evidence.push("the site root responded 200"); + if (rootTarget && !(root.probe.body ?? "").includes(`${rootTarget}/`)) { + failures.push({ + kind: "root-mistargeted", + message: + `The site root does not redirect to '${rootTarget}/'. Readers landing on the bare URL ` + + "would be sent somewhere other than the version this run made default.", + }); + } + } + } + for (const target of observations.targets ?? []) { if (!target.probe?.ok) { failures.push({ @@ -250,15 +308,17 @@ export async function probe(baseUrl, path, { fetchImpl = fetch, uuid = randomUUI async function observe(baseUrl, expectedVersions, publishedVersions, deps) { const { targets, control } = selectProbeTargets(expectedVersions, publishedVersions); - const [stamp, versions, ...rest] = await Promise.all([ + const [stamp, versions, rootProbe, ...rest] = await Promise.all([ probe(baseUrl, STAMP_PATH, deps), probe(baseUrl, VERSIONS_PATH, deps), + probe(baseUrl, ROOT_INDEX_PATH, deps), ...targets.map((version) => probe(baseUrl, versionIndex(version), deps)), ...(control ? [probe(baseUrl, versionIndex(control), deps)] : []), ]); return { stamp, versions, + root: { probe: rootProbe }, targets: targets.map((version, i) => ({ version, probe: rest[i] })), control: control ? { version: control, probe: rest[targets.length] } : null, }; @@ -291,7 +351,7 @@ export async function verifyDeployment({ for (;;) { attempt += 1; const observations = await observe(baseUrl, expectedVersions, publishedVersions, { fetchImpl, uuid }); - verdict = { ...evaluate({ expectedRunId, expectedVersions, observations }), attempt }; + verdict = { ...evaluate({ expectedRunId, expectedVersions, publishedVersions, observations }), attempt }; if (verdict.status === "pass") return verdict; diff --git a/.github/scripts/verify-docs-deployment.test.mjs b/.github/scripts/verify-docs-deployment.test.mjs index 249e686d1..1adbbfe5c 100644 --- a/.github/scripts/verify-docs-deployment.test.mjs +++ b/.github/scripts/verify-docs-deployment.test.mjs @@ -14,6 +14,7 @@ import assert from "node:assert/strict"; import { evaluate, + expectedRootTarget, probe, report, selectProbeTargets, @@ -44,20 +45,24 @@ function unreachable() { return { ok: false, status: null, body: null, error: "getaddrinfo ENOTFOUND" }; } +// The real mike root stub, which forwards to whichever version is default. +const ROOT_STUB = ''; + /** A healthy live site: this run's stamp, the full version list, every page. */ function healthy(overrides = {}, published = PUBLISHED_RELEASE) { const { targets, control } = selectProbeTargets(EXPECTED_VERSIONS, published); return { stamp: ok(JSON.stringify({ run_id: THIS_RUN, sha: "459f7234" })), versions: ok(JSON.stringify(EXPECTED_VERSIONS)), + root: { probe: ok(ROOT_STUB) }, targets: targets.map((version) => ({ version, probe: ok("") })), control: control ? { version: control, probe: ok("") } : null, ...overrides, }; } -function verdictFor(observations, expectedVersions = EXPECTED_VERSIONS) { - return evaluate({ expectedRunId: THIS_RUN, expectedVersions, observations }); +function verdictFor(observations, expectedVersions = EXPECTED_VERSIONS, publishedVersions = PUBLISHED_RELEASE) { + return evaluate({ expectedRunId: THIS_RUN, expectedVersions, publishedVersions, observations }); } test("probe targets cover the published version and the latest holder", () => { @@ -94,11 +99,58 @@ test("probe targets fall back to main when nothing holds latest", () => { assert.deepEqual(selectProbeTargets(unreleased, []), { targets: ["main"], control: null }); }); -test("a published version the live site never listed is not probed blindly", () => { - // Guards the set intersection: probing a name absent from versions.json would - // report a 404 that the versions-missing check already explains better. - const { targets } = selectProbeTargets(EXPECTED_VERSIONS, ["0.9.9-never-published"]); - assert.deepEqual(targets, ["0.1.0-preview.16"]); +test("a published version the live site never listed is reported, not silently dropped", () => { + // The recorded list and versions.json are produced independently, so a name + // in one and not the other means the run does not know what it wrote. + const published = ["0.9.9-never-published"]; + const { targets } = selectProbeTargets(EXPECTED_VERSIONS, published); + assert.deepEqual(targets, ["0.9.9-never-published", "0.1.0-preview.16"]); + + const verdict = verdictFor(healthy({}, published), EXPECTED_VERSIONS, published); + assert.equal(verdict.status, "stranded"); + assert.ok(verdict.failures.some((f) => f.kind === "published-version-unlisted")); + assert.match( + verdict.failures.find((f) => f.kind === "published-version-unlisted").message, + /0\.9\.9-never-published/, + ); +}); + +test("the expected site-root target follows latest, then main", () => { + assert.equal(expectedRootTarget(EXPECTED_VERSIONS), "latest"); + assert.equal(expectedRootTarget([{ version: "main", aliases: [] }]), "main"); + assert.equal(expectedRootTarget([{ version: "0.1.0", aliases: [] }]), null); +}); + +// The URL readers land on. mike writes it only via `set-default`, so no version +// directory covers it and a stale root is invisible to every other check. +test("a site root that 404s is stranded", () => { + const verdict = verdictFor(healthy({ root: { probe: notFound() } })); + assert.equal(verdict.status, "stranded"); + assert.ok(verdict.failures.some((f) => f.kind === "root-unreachable")); +}); + +test("a site root redirecting to the wrong version is stranded", () => { + const stale = ''; + const verdict = verdictFor(healthy({ root: { probe: ok(stale) } })); + assert.equal(verdict.status, "stranded"); + assert.ok(verdict.failures.some((f) => f.kind === "root-mistargeted")); +}); + +test("the site root is not judged when no default can be determined", () => { + const unknown = [{ version: "0.1.0", aliases: [] }]; + const verdict = evaluate({ + expectedRunId: THIS_RUN, + expectedVersions: unknown, + publishedVersions: ["0.1.0"], + observations: { + stamp: ok(JSON.stringify({ run_id: THIS_RUN })), + versions: ok(JSON.stringify(unknown)), + root: { probe: ok("nothing recognisable") }, + targets: [{ version: "0.1.0", probe: ok("") }], + control: null, + }, + }); + assert.equal(verdict.status, "pass"); }); test("a live site serving this run passes", () => { @@ -251,6 +303,9 @@ test("the poll loop returns as soon as the deployment propagates", async () => { if (url.includes("versions.json")) { return { status: 200, text: async () => JSON.stringify(EXPECTED_VERSIONS) }; } + if (url.includes("/docs/index.html")) { + return { status: 200, text: async () => ROOT_STUB }; + } return { status: 200, text: async () => "" }; }; @@ -273,9 +328,9 @@ test("the poll loop returns as soon as the deployment propagates", async () => { assert.equal(verdict.status, "pass"); assert.equal(verdict.attempt, 2); - // The driver must actually request the version this run published, not just - // decide it should have. + // The driver must actually request these, not just decide it should. assert.ok(requested.some((url) => url.includes("/0.1.0-preview.16/index.html?nc="))); + assert.ok(requested.some((url) => url.includes("/docs/index.html?nc="))); }); test("the poll loop gives up once the window closes", async () => { @@ -287,6 +342,9 @@ test("the poll loop gives up once the window closes", async () => { if (url.includes("versions.json")) { return { status: 200, text: async () => JSON.stringify(EXPECTED_VERSIONS) }; } + if (url.includes("/docs/index.html")) { + return { status: 200, text: async () => ROOT_STUB }; + } return { status: 200, text: async () => "" }; }; diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 781237d5c..e2afcfaf8 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -131,7 +131,6 @@ jobs: run: | version="${GITHUB_REF_NAME#v}" newest="$(git tag --list 'v*' --sort=-v:refname | head -n 1)" - echo "$version" >> "$RUNNER_TEMP/$PUBLISHED_VERSIONS_FILE" if [ "$GITHUB_REF_NAME" = "$newest" ]; then mike deploy --branch "$MIKE_BRANCH" --push --alias-type copy \ @@ -144,6 +143,11 @@ jobs: mike deploy --branch "$MIKE_BRANCH" --push --alias-type copy "$version" fi + # Recorded only after mike has actually written the version, so the + # list the verify job probes never claims something that was not + # published. + echo "$version" >> "$RUNNER_TEMP/$PUBLISHED_VERSIONS_FILE" + - name: Backfill release versions if: inputs.backfill_tags env: @@ -323,6 +327,18 @@ jobs: fi published="$(jq -R -s -c 'split("\n") | map(select(length > 0)) | unique' < "$published_file")" + + # The two lists are produced independently — one by the publishing + # steps, one by mike — so a divergence means the run does not actually + # know what it published. Fail here rather than letting the verify job + # quietly probe fewer directories than it should. + missing="$(jq -c --argjson published "$published" \ + '(map(.version)) as $known | $published - $known' site/versions.json)" + if [ "$missing" != "[]" ]; then + echo "::error::These versions were recorded as published but are absent from site/versions.json: $missing. The publishing steps and mike disagree, so the verify job cannot be trusted to probe the right directories." + exit 1 + fi + echo "This run published: $published" echo "published=$published" >> "$GITHUB_OUTPUT" diff --git a/docs/contributing/release-runbook.md b/docs/contributing/release-runbook.md index 33dc63207..1a180bd11 100644 --- a/docs/contributing/release-runbook.md +++ b/docs/contributing/release-runbook.md @@ -300,6 +300,15 @@ Every probe uses a unique `?nc=` query key, so a pass cannot come from a cached and an already-published version is fetched with the identical request shape as a positive control — a no-match is not a measurement until the same probe is shown able to match. +What `verify` actually asserts, beyond the stamp: that the live `versions.json` contains +every version the artifact declared and puts `latest` where this run put it; that each +version **this run published** serves its own `index.html` (not just the `latest` holder, +which a backported tag deliberately does not move and a `main` push never touches); and +that the site root — the URL readers land on, written only by `mike set-default` — responds +and still forwards to the expected default. The list of versions a run published is +recorded by the publishing steps and cross-checked against mike's own `versions.json` +before the artifact is uploaded, so the two cannot drift apart unnoticed. + **The remedy, verified.** Dispatch `Publish docs` on `main`. The `publish` job assembles the artifact from the whole `gh-pages` branch, so any allowed ref re-serves every published version. With no `backfill_tags` the release step is skipped, and `mike set-default` runs diff --git a/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs b/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs index f24b82bad..3279b26d2 100644 --- a/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs +++ b/tests/Reactor.DocPipeline.Tests/DocsDeployWiringTests.cs @@ -94,6 +94,12 @@ public void The_artifact_carries_a_stamp_identifying_the_run_that_built_it() // holder, which never covers a backported tag or a `main` push. Assert.Matches(@"published=", run); Assert.Contains("PUBLISHED_VERSIONS_FILE", run, StringComparison.Ordinal); + + // The recorded list and mike's versions.json are produced + // independently. Without this cross-check a divergence would quietly + // shrink what verify probes instead of failing. + Assert.Contains("site/versions.json", run, StringComparison.Ordinal); + Assert.Matches(@"missing\b", run); } [Theory] From bb81a9d11a923e0fba302c28a3bec9ea3d7000ae Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 23 Sep 2026 13:11:12 -0700 Subject: [PATCH 06/14] Probe the latest alias path and floor the verifier suite size Two Copilot review findings on #1269. 1. mike deploy --alias-type copy publishes latest/ as its own path in the gh-pages tree, and the site root forwards there, so a missing or stale latest/index.html 404s every reader arriving at the bare URL. Probing the version that owns the alias does not cover it: confirmed on origin/gh-pages that latest/index.html and 0.1.0-preview.16/index.html are distinct tree entries (they happen to share a blob today only because git deduplicates identical content). The verifier now probes the alias path explicitly and reports alias-unreachable, without treating latest as a published version. 2. DocsDeploymentVerifierTests only asserted a positive TAP pass count, so a suite gutted to one passing case would still have kept the xUnit gate green. It now asserts a floor, with a message naming the constant to lower if cases are deliberately removed. Verified by replacing the suite with a single test: the gate fails with 'Only 1 cases ran, below the 30 this suite is expected to carry.' observe() also stopped using positional destructuring. The probe set is conditional, so an off-by-one there would have silently swapped two results rather than failing; it now builds a labelled job list. Both new comparisons are mutation-checked, including that the driver actually requests the alias URL rather than merely deciding it should. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff518ae2-0ea3-466c-a9c0-4ea2265937c5 --- .github/scripts/verify-docs-deployment.mjs | 62 ++++++++++++++----- .../scripts/verify-docs-deployment.test.mjs | 14 +++++ .../DocsDeploymentVerifierTests.cs | 26 ++++++-- 3 files changed, 83 insertions(+), 19 deletions(-) diff --git a/.github/scripts/verify-docs-deployment.mjs b/.github/scripts/verify-docs-deployment.mjs index e985ac132..339cdff1e 100644 --- a/.github/scripts/verify-docs-deployment.mjs +++ b/.github/scripts/verify-docs-deployment.mjs @@ -137,7 +137,7 @@ export function expectedRootTarget(expectedVersions) { * @returns {{ status: "pass"|"stranded"|"probe-broken", failures: {kind: string, message: string}[], evidence: string[], liveRunId: string|null }} */ export function evaluate({ expectedRunId, expectedVersions, publishedVersions = [], observations }) { - const { stamp, versions, control, root } = observations; + const { stamp, versions, control, root, alias } = observations; const failures = []; const evidence = []; let liveRunId = null; @@ -272,6 +272,22 @@ export function evaluate({ expectedRunId, expectedVersions, publishedVersions = evidence.push(`the positive control ${control.version}/ responded 200`); } + // The `latest` alias is its own copied tree, and the site root forwards to + // it, so a broken `latest/` 404s every reader arriving at the bare URL even + // when the version it aliases is perfectly healthy. + if (alias) { + if (!alias.probe?.ok) { + failures.push({ + kind: "alias-unreachable", + message: + `${alias.version}/ did not respond 200 (${describeProbe(alias.probe)}). ` + + "The site root forwards there, so readers landing on the bare URL would get nothing.", + }); + } else { + evidence.push(`${alias.version}/ responded 200`); + } + } + if (failures.length === 0) { return { status: "pass", failures, evidence, liveRunId }; } @@ -308,20 +324,36 @@ export async function probe(baseUrl, path, { fetchImpl = fetch, uuid = randomUUI async function observe(baseUrl, expectedVersions, publishedVersions, deps) { const { targets, control } = selectProbeTargets(expectedVersions, publishedVersions); - const [stamp, versions, rootProbe, ...rest] = await Promise.all([ - probe(baseUrl, STAMP_PATH, deps), - probe(baseUrl, VERSIONS_PATH, deps), - probe(baseUrl, ROOT_INDEX_PATH, deps), - ...targets.map((version) => probe(baseUrl, versionIndex(version), deps)), - ...(control ? [probe(baseUrl, versionIndex(control), deps)] : []), - ]); - return { - stamp, - versions, - root: { probe: rootProbe }, - targets: targets.map((version, i) => ({ version, probe: rest[i] })), - control: control ? { version: control, probe: rest[targets.length] } : null, - }; + const latest = aliasHolder(expectedVersions ?? [], LATEST_ALIAS); + + // Built as a labelled list rather than positional destructuring: the probe + // set is conditional, and an off-by-one there would silently swap two + // results instead of failing. + const jobs = [ + { key: "stamp", path: STAMP_PATH }, + { key: "versions", path: VERSIONS_PATH }, + { key: "root", path: ROOT_INDEX_PATH }, + // `mike deploy --alias-type copy` publishes `latest/` as its own tree, and + // the site root forwards there, so a missing `latest/index.html` 404s every + // reader arriving at the bare URL. Probing the version that *owns* the + // alias does not cover it: they are separate directories. + ...(latest ? [{ key: "alias", path: versionIndex(LATEST_ALIAS) }] : []), + ...targets.map((version) => ({ key: "target", version, path: versionIndex(version) })), + ...(control ? [{ key: "control", version: control, path: versionIndex(control) }] : []), + ]; + + const results = await Promise.all(jobs.map((job) => probe(baseUrl, job.path, deps))); + + const observations = { stamp: null, versions: null, root: null, alias: null, targets: [], control: null }; + jobs.forEach((job, i) => { + const result = results[i]; + if (job.key === "target") observations.targets.push({ version: job.version, probe: result }); + else if (job.key === "control") observations.control = { version: job.version, probe: result }; + else if (job.key === "alias") observations.alias = { version: LATEST_ALIAS, probe: result }; + else if (job.key === "root") observations.root = { probe: result }; + else observations[job.key] = result; + }); + return observations; } /** diff --git a/.github/scripts/verify-docs-deployment.test.mjs b/.github/scripts/verify-docs-deployment.test.mjs index 1adbbfe5c..315263271 100644 --- a/.github/scripts/verify-docs-deployment.test.mjs +++ b/.github/scripts/verify-docs-deployment.test.mjs @@ -55,6 +55,7 @@ function healthy(overrides = {}, published = PUBLISHED_RELEASE) { stamp: ok(JSON.stringify({ run_id: THIS_RUN, sha: "459f7234" })), versions: ok(JSON.stringify(EXPECTED_VERSIONS)), root: { probe: ok(ROOT_STUB) }, + alias: { version: "latest", probe: ok("") }, targets: targets.map((version) => ({ version, probe: ok("") })), control: control ? { version: control, probe: ok("") } : null, ...overrides, @@ -136,6 +137,18 @@ test("a site root redirecting to the wrong version is stranded", () => { assert.ok(verdict.failures.some((f) => f.kind === "root-mistargeted")); }); +// `--alias-type copy` makes `latest/` a separate tree from the version it +// aliases, and the site root forwards to it, so it needs its own probe. +test("a broken latest alias is stranded even when its version is healthy", () => { + const verdict = verdictFor(healthy({ alias: { version: "latest", probe: notFound() } })); + assert.equal(verdict.status, "stranded"); + assert.deepEqual( + verdict.failures.map((f) => f.kind), + ["alias-unreachable"], + ); + assert.match(verdict.failures[0].message, /site root forwards there/); +}); + test("the site root is not judged when no default can be determined", () => { const unknown = [{ version: "0.1.0", aliases: [] }]; const verdict = evaluate({ @@ -331,6 +344,7 @@ test("the poll loop returns as soon as the deployment propagates", async () => { // The driver must actually request these, not just decide it should. assert.ok(requested.some((url) => url.includes("/0.1.0-preview.16/index.html?nc="))); assert.ok(requested.some((url) => url.includes("/docs/index.html?nc="))); + assert.ok(requested.some((url) => url.includes("/latest/index.html?nc="))); }); test("the poll loop gives up once the window closes", async () => { diff --git a/tests/Reactor.DocPipeline.Tests/DocsDeploymentVerifierTests.cs b/tests/Reactor.DocPipeline.Tests/DocsDeploymentVerifierTests.cs index 3dcd6ec5b..19aa2992f 100644 --- a/tests/Reactor.DocPipeline.Tests/DocsDeploymentVerifierTests.cs +++ b/tests/Reactor.DocPipeline.Tests/DocsDeploymentVerifierTests.cs @@ -1,7 +1,9 @@ using System; using System.Diagnostics; +using System.Globalization; using System.IO; using System.Linq; +using System.Text.RegularExpressions; using System.Threading.Tasks; using Xunit; @@ -24,6 +26,13 @@ namespace Microsoft.UI.Reactor.Cli.Docs.Tests; /// public sealed class DocsDeploymentVerifierTests { + /// + /// Floor for the number of cases the node suite must run, so it cannot be + /// quietly reduced to one passing case while the xUnit gate stays green. + /// Raising it as cases are added is optional; lowering it is a decision. + /// + private const int MinimumCases = 30; + [Fact] public async Task Deployment_verifier_cases_pass() { @@ -70,10 +79,19 @@ public async Task Deployment_verifier_cases_pass() $".github/scripts/verify-docs-deployment.test.mjs failed (exit {process.ExitCode}):{Environment.NewLine}{output}"); // An exit code of zero is also what `node` returns for a file that - // registered no tests at all, so assert the suite actually ran. Without - // this, renaming the cases out from under the runner would read green. - Assert.Contains("# pass ", stdout, StringComparison.Ordinal); - Assert.DoesNotContain("# pass 0", stdout, StringComparison.Ordinal); + // registered no tests at all, and a positive pass count alone would + // still be satisfied by a suite gutted to a single case. Assert a floor + // instead: growth is fine, silent shrinkage is not. + var match = Regex.Match(stdout, @"^# pass (\d+)$", RegexOptions.Multiline); + Assert.True(match.Success, $"No TAP pass count in the runner output:{Environment.NewLine}{output}"); + + var passed = int.Parse(match.Groups[1].Value, CultureInfo.InvariantCulture); + Assert.True( + passed >= MinimumCases, + $"Only {passed} cases ran, below the {MinimumCases} this suite is expected to carry. " + + "The live-site verifier is the only check that can catch a stranded deployment, so its " + + "regression suite must not shrink. If cases were deliberately removed or merged, lower " + + $"{nameof(MinimumCases)} in this file with the reason in the commit message."); } private static string FindRepoRoot() From 87a2f0e4cf8297d35b662722347ffb1d6933e7c0 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 23 Sep 2026 13:39:27 -0700 Subject: [PATCH 07/14] Parse the root redirect, bound every probe, close the tag-refresh fail-open Three Copilot review findings on #1269. 1. The site-root check searched for the expected target anywhere in the HTML. That accepted a root redirecting to not-latest/, since it contains latest/, and accepted one whose only mention of the target was the human-visible while the real script redirect pointed elsewhere. The redirect target is now parsed from the location.replace call, falling back to the noscript meta refresh, and compared exactly; an unrecognisable root is reported as root-unparseable rather than passing. 2. The stand-down gate failed open on a tag-refresh failure. git fetch --tags was allowed to fail with only a log line, after which git tag --points-at would find nothing on a stale checkout and write deploy=true, restoring the very main/tag pages_build_version collision the gate prevents, silently. It now retries three times and fails the job, which costs a re-run rather than a publish: gh-pages is already written by that point. 3. Probes were unbounded. Neither fetch nor response.text() times out on its own and observe() awaits them together, so one stalled connection held the polling loop past its deadline and swallowed the exit-2 diagnostic, leaving only the outer job timeout. Each probe now carries an AbortSignal with a 20s ceiling, well under the 15s-interval polling window. Suite is 37 cases; the DocsDeploymentVerifierTests floor moved with it. All four new comparisons are mutation-checked and redden only their intended cases. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff518ae2-0ea3-466c-a9c0-4ea2265937c5 --- .github/scripts/verify-docs-deployment.mjs | 87 ++++++++++++++-- .../scripts/verify-docs-deployment.test.mjs | 99 ++++++++++++++++++- .github/workflows/docs.yml | 21 +++- docs/contributing/release-runbook.md | 6 +- .../DocsDeployWiringTests.cs | 6 ++ .../DocsDeploymentVerifierTests.cs | 2 +- 6 files changed, 205 insertions(+), 16 deletions(-) diff --git a/.github/scripts/verify-docs-deployment.mjs b/.github/scripts/verify-docs-deployment.mjs index 339cdff1e..468094d57 100644 --- a/.github/scripts/verify-docs-deployment.mjs +++ b/.github/scripts/verify-docs-deployment.mjs @@ -31,6 +31,16 @@ export const VERSIONS_PATH = "versions.json"; export const ROOT_INDEX_PATH = "index.html"; export const LATEST_ALIAS = "latest"; +/** + * Per-request ceiling. Deliberately well under the polling window so a stalled + * connection costs one round rather than the whole budget, and under the job's + * own timeout so the failure is this gate's diagnostic rather than a silent + * runner kill. + */ +export const DEFAULT_REQUEST_TIMEOUT_MS = 20_000; + +const defaultAbortSignal = (ms) => AbortSignal.timeout(ms); + /** * The page a version probe fetches. * @@ -115,6 +125,34 @@ export function selectProbeTargets(expectedVersions, publishedVersions = []) { return { targets, control }; } +/** + * The version or alias a mike-generated site root forwards to, or null when the + * document does not look like one. + * + * Parsed rather than substring-matched. `set-default` writes the target into a + * `location.replace(...)` call and a `