From 6ca23764440766e307df8936220c53eb8ed837f0 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Fri, 7 Aug 2026 16:04:03 -0700 Subject: [PATCH 1/2] refactor(drift-sync): extract commit-message construction into buildSyncCommitMessage --- scripts/drift-sync.ts | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/scripts/drift-sync.ts b/scripts/drift-sync.ts index 7ad82f33..2a8ee113 100644 --- a/scripts/drift-sync.ts +++ b/scripts/drift-sync.ts @@ -1289,12 +1289,10 @@ export function computeChangesetKey(outcome: SyncCoreOutcome): string { return createHash("sha256").update(entries.join("\n")).digest("hex").slice(0, 16); } -/** Stage + commit exactly the sync core's own touched files (never a catch-all `git add`). */ -function commitSyncChanges(outcome: SyncCoreOutcome): boolean { - const changed = getChangedFiles().filter( - (f) => f === MODEL_REGISTRY_REL_PATH || f.startsWith(`${DRIFT_PROPOSALS_DIR}/`), - ); - if (changed.length === 0) return false; +export function buildSyncCommitMessage(outcome: SyncCoreOutcome): { + subject: string; + body: string; +} { const applied = outcome.outcomes.filter( (o) => o.action === "added" || o.action === "deprecation-recorded", ); @@ -1302,12 +1300,18 @@ function commitSyncChanges(outcome: SyncCoreOutcome): boolean { applied.length > 0 ? applied.map((o) => `${o.action} ${o.provider}/${o.family}`).join(", ") : "needs-human note file(s)"; + return { subject: `fix(drift-sync): mechanical model-family sync (${summary})`, body: "" }; +} + +/** Stage + commit exactly the sync core's own touched files (never a catch-all `git add`). */ +function commitSyncChanges(outcome: SyncCoreOutcome): boolean { + const changed = getChangedFiles().filter( + (f) => f === MODEL_REGISTRY_REL_PATH || f.startsWith(`${DRIFT_PROPOSALS_DIR}/`), + ); + if (changed.length === 0) return false; + const { subject, body } = buildSyncCommitMessage(outcome); execFileSafe("git", ["add", ...changed]); - execFileSafe("git", [ - "commit", - "-m", - `fix(drift-sync): mechanical model-family sync (${summary})`, - ]); + execFileSafe("git", ["commit", ...(body ? ["-m", subject, "-m", body] : ["-m", subject])]); return true; } From 2aa92572d2ac04322eb9376a18c04149e8d9b9b2 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Fri, 7 Aug 2026 16:05:34 -0700 Subject: [PATCH 2/2] fix(drift-sync): bound the sync commit subject so the bot's own PR can merge --- scripts/drift-sync.ts | 80 ++++++++++++++++++++++-- src/__tests__/drift-sync-core.test.ts | 89 +++++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 5 deletions(-) diff --git a/scripts/drift-sync.ts b/scripts/drift-sync.ts index 2a8ee113..8384a9c6 100644 --- a/scripts/drift-sync.ts +++ b/scripts/drift-sync.ts @@ -1289,6 +1289,60 @@ export function computeChangesetKey(outcome: SyncCoreOutcome): string { return createHash("sha256").update(entries.join("\n")).digest("hex").slice(0, 16); } +/** + * commitlint's `header-max-length` (and its `body-max-line-length` companion), + * both 100 in `@commitlint/config-conventional`, which this repo extends. + * Mirrored here as a constant because the sync bot's commit is authored by CI + * with no human in the loop: a header over the limit fails the `commitlint` + * check on the bot's own PR, which then can NEVER merge without a human + * rewriting the message — defeating the automation entirely. + */ +export const COMMIT_LINE_MAX_LENGTH = 100; + +const SYNC_COMMIT_SUBJECT_PREFIX = "fix(drift-sync): mechanical model-family sync"; + +/** + * Hard-wrap `text` to `max` columns, breaking at a space where one exists in + * range and hard-breaking an over-long unbreakable token otherwise. Lossless — + * every character survives, only whitespace is re-flowed — so the per-family + * detail stays greppable in the log while every emitted line satisfies + * `body-max-line-length`. + */ +function wrapToWidth(text: string, max: number): string[] { + const out: string[] = []; + for (const paragraphLine of text.split("\n")) { + let rest = paragraphLine; + if (rest.length === 0) { + out.push(""); + continue; + } + while (rest.length > max) { + const breakAt = rest.lastIndexOf(" ", max); + const cut = breakAt > 0 ? breakAt : max; + out.push(rest.slice(0, cut)); + rest = rest.slice(breakAt > 0 ? cut + 1 : cut); + } + if (rest.length > 0) out.push(rest); + } + return out; +} + +/** + * Build the sync bot's commit message: a subject BOUNDED at + * `COMMIT_LINE_MAX_LENGTH` for ANY number of changes, plus a body carrying the + * full per-family detail (the body has no length limit beyond per-line width, + * so nothing is lost). + * + * WHY the subject summarizes rather than enumerates: it used to interpolate + * every ` /` pair, so it grew without bound with the + * changeset — a real ten-deprecation run (CI run 31225520102, PR #366) produced + * a 525-character header and a `commitlint` failure the bot cannot clear on its + * own. It degrades by COUNT, not by characters: a chopped-off list would sever + * its own trailing `)` mid-token and read as garbage, whereas "N families: + * " stays true and legible at every size. The provider list itself + * collapses to a count if naming every provider would ever breach the bound, so + * the guarantee holds structurally, not by slicing. + */ export function buildSyncCommitMessage(outcome: SyncCoreOutcome): { subject: string; body: string; @@ -1296,11 +1350,27 @@ export function buildSyncCommitMessage(outcome: SyncCoreOutcome): { const applied = outcome.outcomes.filter( (o) => o.action === "added" || o.action === "deprecation-recorded", ); - const summary = - applied.length > 0 - ? applied.map((o) => `${o.action} ${o.provider}/${o.family}`).join(", ") - : "needs-human note file(s)"; - return { subject: `fix(drift-sync): mechanical model-family sync (${summary})`, body: "" }; + if (applied.length === 0) { + return { subject: `${SYNC_COMMIT_SUBJECT_PREFIX} (needs-human note file(s))`, body: "" }; + } + + const count = `${applied.length} ${applied.length === 1 ? "family" : "families"}`; + const providers = [...new Set(applied.map((o) => o.provider))].sort(); + // Widest-first, each fallback strictly shorter than the last. The final form + // is ` ()` — 48 characters plus a decimal array length, which + // cannot approach 100 — so SOME candidate always satisfies the bound. + const candidates = [ + `${SYNC_COMMIT_SUBJECT_PREFIX} (${count}: ${providers.join(", ")})`, + `${SYNC_COMMIT_SUBJECT_PREFIX} (${count} across ${providers.length} providers)`, + `${SYNC_COMMIT_SUBJECT_PREFIX} (${count})`, + ]; + const subject = + candidates.find((c) => c.length <= COMMIT_LINE_MAX_LENGTH) ?? candidates[candidates.length - 1]; + + const body = applied + .flatMap((o) => wrapToWidth(`- ${o.action} ${o.provider}/${o.family}`, COMMIT_LINE_MAX_LENGTH)) + .join("\n"); + return { subject, body }; } /** Stage + commit exactly the sync core's own touched files (never a catch-all `git add`). */ diff --git a/src/__tests__/drift-sync-core.test.ts b/src/__tests__/drift-sync-core.test.ts index d7ba329e..f6d9bc88 100644 --- a/src/__tests__/drift-sync-core.test.ts +++ b/src/__tests__/drift-sync-core.test.ts @@ -35,12 +35,16 @@ import { runDriftSyncCore, computeChangesetKey, revertSyncFiles, + buildSyncCommitMessage, + COMMIT_LINE_MAX_LENGTH, SyncCoreReason, MODEL_REGISTRY_REL_PATH, DRIFT_PROPOSALS_DIR, type SyncCoreDeps, type ProviderChurnInput, type SyncCheckResultLike, + type SyncCoreOutcome, + type Provider, } from "../../scripts/drift-sync.js"; // --------------------------------------------------------------------------- @@ -1127,3 +1131,88 @@ describe("computeChangesetKey (G#3: stable, date-independent PR-dedup key)", () expect(key).toMatch(/^[0-9a-f]{16}$/); }); }); + +// --------------------------------------------------------------------------- +// Commit-message bound (the bot's PR must be able to merge unattended). +// +// The sync bot's commit is authored by CI with no human in the loop. Its +// subject used to enumerate every changed family, so it grew without bound: +// production run 31225520102 (PR #366) emitted a 525-character header, which +// `commitlint`'s `header-max-length` (100, via @commitlint/config-conventional) +// rejects — permanently blocking the bot's own PR until a human rewrites the +// message. These pin the bound at the sizes that matter. +// --------------------------------------------------------------------------- + +function appliedOutcome(n: number, provider: Provider = "anthropic"): SyncCoreOutcome { + return { + ok: true, + reason: SyncCoreReason.OK_APPLIED, + detail: "applied", + outcomes: Array.from({ length: n }, (_, i) => ({ + provider, + family: `claude-family-${i}`, + action: "deprecation-recorded" as const, + detail: "recorded", + })), + skipped: [], + }; +} + +describe("buildSyncCommitMessage", () => { + it.each([1, 2, 10, 50, 500])("keeps the subject within the commitlint bound (n=%i)", (n) => { + const { subject } = buildSyncCommitMessage(appliedOutcome(n)); + expect(subject.length).toBeLessThanOrEqual(COMMIT_LINE_MAX_LENGTH); + }); + + it("keeps the subject bounded when every provider changed at once", () => { + const outcome = appliedOutcome(0); + const providers: Provider[] = ["anthropic", "gemini", "openai"]; + outcome.outcomes = providers.flatMap((provider) => + Array.from({ length: 30 }, (_, i) => ({ + provider, + family: `family-${i}`, + action: "deprecation-recorded" as const, + detail: "recorded", + })), + ); + const { subject } = buildSyncCommitMessage(outcome); + expect(subject.length).toBeLessThanOrEqual(COMMIT_LINE_MAX_LENGTH); + expect(subject).toContain("90 families"); + }); + + it("stays bounded even when a single family name is pathologically long", () => { + const outcome = appliedOutcome(1); + outcome.outcomes[0].family = "x".repeat(400); + const { subject, body } = buildSyncCommitMessage(outcome); + expect(subject.length).toBeLessThanOrEqual(COMMIT_LINE_MAX_LENGTH); + for (const line of body.split("\n")) { + expect(line.length).toBeLessThanOrEqual(COMMIT_LINE_MAX_LENGTH); + } + }); + + it("reads naturally for a single change (never '1 changes')", () => { + const { subject } = buildSyncCommitMessage(appliedOutcome(1)); + expect(subject).toContain("1 family:"); + expect(subject).not.toContain("1 families"); + }); + + it("moves the full per-family detail into the body, one bounded line each", () => { + const { body } = buildSyncCommitMessage(appliedOutcome(10)); + const lines = body.split("\n"); + expect(lines).toHaveLength(10); + for (const line of lines) { + expect(line.length).toBeLessThanOrEqual(COMMIT_LINE_MAX_LENGTH); + } + expect(body).toContain("- deprecation-recorded anthropic/claude-family-0"); + expect(body).toContain("- deprecation-recorded anthropic/claude-family-9"); + }); + + it("keeps the needs-human-only subject unchanged and bodyless", () => { + const { subject, body } = buildSyncCommitMessage(appliedOutcome(0)); + expect(subject).toBe( + "fix(drift-sync): mechanical model-family sync (needs-human note file(s))", + ); + expect(subject.length).toBeLessThanOrEqual(COMMIT_LINE_MAX_LENGTH); + expect(body).toBe(""); + }); +});