From da9f211ef2cfc3d47cd3c4327bb152198b026dd1 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Sat, 8 Aug 2026 23:09:21 +0800 Subject: [PATCH] fix(design-system): let the visual baseline binding accept a refresh, not only the first adoption `validateCandidateSourceBinding` required `candidateSourceHead` to name a commit whose `AWAITING_BASELINE` still declared the canonical six, and the current tree to declare none. That models the FIRST adoption and nothing else. After #1729 landed, no commit on main declares the six again, so no future capture head could ever satisfy it -- the binding was satisfiable exactly once, and that once has passed. The consequence is only latent today, and this commit does not pretend otherwise: `validateLinuxVisualBaselineSet` short-circuits on `declaredPaths.length === 0` and no surface yet declares `baseline.status: "committed"`, so the whole contract is dormant and refreshing a golden is currently ungated. The defect bites the moment those surfaces are declared (#242) -- at which point the first intentional design change would leave the goldens red with no supported way to re-adopt them, which is precisely backwards for a gate meant to make design changes provable. So the binding now accepts either shape: FIRST ADOPTION candidate head declares the six, current declares none. REFRESH both ends empty, suite byte-identical. A refresh is the ordinary case once baselines exist: a surface is deliberately re-shot and its goldens replaced while the suite itself does not change at all. Normalisation already blanks the AWAITING values on both sides, so the "suite changed beyond the AWAITING declaration" check stays exact for both. Nothing is loosened beyond that. A candidate head with five ids, seven ids, a dynamic set, a spread or a duplicate still fails, as do a non-empty current list, a non-ancestor head, a hash mismatch, a wrong platform and an unreviewed set. The one previously-untested shape -- a refresh -- now has its own case, because the gap was invisible precisely because nothing exercised it. Verified: tests/design-system-adoption.test.ts 51 passed (51), including the new refresh case; check:design-system-adoption and check:design-system-contract exit 0; format:check clean. Refs #242, #118 --- scripts/generate-design-system-adoption.mjs | 27 ++++++++++++++-- tests/design-system-adoption.test.ts | 34 ++++++++++++++++++--- 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/scripts/generate-design-system-adoption.mjs b/scripts/generate-design-system-adoption.mjs index 4b47310836..188d78eb76 100644 --- a/scripts/generate-design-system-adoption.mjs +++ b/scripts/generate-design-system-adoption.mjs @@ -418,15 +418,33 @@ function validateCandidateSourceBinding(candidateSourceHead, { root, policy }) { const candidateAwaiting = candidateSuite === null ? null : visualBaselineAwaitingIds(candidateSuite, visualSuiteFile); const currentAwaiting = currentSuite === null ? null : visualBaselineAwaitingIds(currentSuite, visualSuiteFile); + // Two legitimate shapes, and the second is what makes updating the design possible. + // + // FIRST ADOPTION — the capture head still declares the canonical six as awaiting, + // and the adopting commit empties that list. + // REFRESH — the list is already empty at both ends. Deliberately re-shooting a + // surface produces exactly this, and it is the ORDINARY case once baselines + // exist: the pixels move, the goldens are replaced, and the suite itself does + // not change at all. + // + // Requiring the six-to-empty transition alone made this binding satisfiable + // exactly once. After the first adoption no commit declares the six again, so no + // capture head could ever qualify and every later refresh was unprovable — the + // goldens would go red on the first intentional design change with no supported + // way to re-adopt them. const candidateAwaitingCanonical = candidateAwaiting?.valid && JSON.stringify(candidateAwaiting.ids) === JSON.stringify(canonicalIds); + const candidateAwaitingEmpty = candidateAwaiting?.valid && candidateAwaiting.ids.length === 0; + const candidateAwaitingAdoptable = candidateAwaitingCanonical || candidateAwaitingEmpty; const currentAwaitingEmpty = currentAwaiting?.valid && currentAwaiting.ids.length === 0; if (!candidateAwaiting?.valid) { failures.push( `candidateSourceHead AWAITING_BASELINE must be a static literal Set: ${candidateAwaiting?.failure ?? "missing suite"}`, ); - } else if (!candidateAwaitingCanonical) { - failures.push("candidateSourceHead AWAITING_BASELINE must contain exactly the canonical six ids"); + } else if (!candidateAwaitingAdoptable) { + failures.push( + "candidateSourceHead AWAITING_BASELINE must be either the canonical six ids (first adoption) or empty (refresh)", + ); } if (!currentAwaiting?.valid) { failures.push( @@ -444,8 +462,11 @@ function validateCandidateSourceBinding(candidateSourceHead, { root, policy }) { currentSuite === null || currentAwaiting === null ? null : normalizeVisualSuiteAwaitingValues(currentSuite, currentAwaiting); + // Normalisation blanks the AWAITING_BASELINE values on both sides, so this stays + // exact for a refresh (where the suite is byte-identical) as well as for the + // first adoption (where only those values moved). const visualSuiteOnlyChangedAwaiting = - candidateAwaitingCanonical && + candidateAwaitingAdoptable && currentAwaitingEmpty && normalizedCandidateSuite !== null && normalizedCandidateSuite === normalizedCurrentSuite; diff --git a/tests/design-system-adoption.test.ts b/tests/design-system-adoption.test.ts index 3e7b58d392..3f9926eb7a 100644 --- a/tests/design-system-adoption.test.ts +++ b/tests/design-system-adoption.test.ts @@ -831,15 +831,25 @@ describe("design-system adoption manifest", () => { } }); - it("requires the exact static AWAITING_BASELINE transition from canonical six to empty", { timeout: 90_000 }, () => { + it("accepts a first adoption or a refresh, and nothing else", { timeout: 90_000 }, () => { const exactRoot = fs.mkdtempSync(path.join(os.tmpdir(), "design-system-awaiting-exact-")); + const refreshRoot = fs.mkdtempSync(path.join(os.tmpdir(), "design-system-awaiting-refresh-")); const retainedRoot = fs.mkdtempSync(path.join(os.tmpdir(), "design-system-awaiting-retained-")); const missingRoot = fs.mkdtempSync(path.join(os.tmpdir(), "design-system-awaiting-missing-")); const extraRoot = fs.mkdtempSync(path.join(os.tmpdir(), "design-system-awaiting-extra-")); const dynamicRoot = fs.mkdtempSync(path.join(os.tmpdir(), "design-system-awaiting-dynamic-")); const spreadRoot = fs.mkdtempSync(path.join(os.tmpdir(), "design-system-awaiting-spread-")); const duplicateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "design-system-awaiting-duplicate-")); - const fixtureRoots = [exactRoot, retainedRoot, missingRoot, extraRoot, dynamicRoot, spreadRoot, duplicateRoot]; + const fixtureRoots = [ + exactRoot, + refreshRoot, + retainedRoot, + missingRoot, + extraRoot, + dynamicRoot, + spreadRoot, + duplicateRoot, + ]; try { initialiseCandidateRepository(exactRoot, canonicalAwaitingValues); setCurrentAwaitingValues(exactRoot, ""); @@ -848,6 +858,18 @@ describe("design-system adoption manifest", () => { validateLinuxVisualBaselineSet(exact.paths, { root: exactRoot, trackedFiles: exact.trackedFiles }), ).toEqual([]); + // A REFRESH binds too: empty at both ends, suite byte-identical. This is the + // ordinary case once baselines exist — a surface is deliberately re-shot and + // its goldens replaced. Without it the six-to-empty transition was satisfiable + // exactly once, so the first intentional design change would have left the + // goldens red with no supported way to re-adopt them. + initialiseCandidateRepository(refreshRoot, ""); + setCurrentAwaitingValues(refreshRoot, ""); + const refresh = writeBaselineSet(refreshRoot); + expect( + validateLinuxVisualBaselineSet(refresh.paths, { root: refreshRoot, trackedFiles: refresh.trackedFiles }), + ).toEqual([]); + initialiseCandidateRepository(retainedRoot, canonicalAwaitingValues); setCurrentAwaitingValues(retainedRoot, JSON.stringify("dashboard-shell")); const retained = writeBaselineSet(retainedRoot); @@ -872,14 +894,18 @@ describe("design-system adoption manifest", () => { root: missingRoot, trackedFiles: missing.trackedFiles, }), - ).toContain("candidateSourceHead AWAITING_BASELINE must contain exactly the canonical six ids"); + ).toContain( + "candidateSourceHead AWAITING_BASELINE must be either the canonical six ids (first adoption) or empty (refresh)", + ); initialiseCandidateRepository(extraRoot, `${canonicalAwaitingValues}, "extra-target"`); setCurrentAwaitingValues(extraRoot, ""); const extra = writeBaselineSet(extraRoot); expect( validateLinuxVisualBaselineSet(extra.paths, { root: extraRoot, trackedFiles: extra.trackedFiles }), - ).toContain("candidateSourceHead AWAITING_BASELINE must contain exactly the canonical six ids"); + ).toContain( + "candidateSourceHead AWAITING_BASELINE must be either the canonical six ids (first adoption) or empty (refresh)", + ); initialiseCandidateRepository(dynamicRoot, "BASELINE_IDS"); setCurrentAwaitingValues(dynamicRoot, "");