From 55dcd518f8bd61abe44811f3ea2dab13dd737a0a Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Fri, 18 Sep 2026 01:31:09 +0000 Subject: [PATCH 1/4] ci: fail a PR that edits sync-owned Comfy Router files The Comfy API v2 spec sync owns openapi-v2.yaml, the hand-written Comfy Router reference/quickstart/limitations pages, the router-schemas mirror and the generated model pages, and rewrites them from upstream sources on every run. Nothing in docs CI said so, and three merged PRs in three days edited those pages directly: each shipped live and is reverted by the rolling sync PR's diff. Add a path-filtered pull_request workflow that refuses such an edit while its author can still move it upstream, naming every offending file and where the edit belongs instead. The sync's own PR is exempt by author or by its fixed head branch. The two generated page kinds are guarded conditionally: code-pages:check already fails a stale page, so committing a regenerated code.mdx is mandatory whenever a generator input changes. A PR that also changes an input is regenerating rather than hand-editing and passes. --- .../sync-owned/check-sync-owned.test.ts | 273 ++++++++++++++++++ .../scripts/sync-owned/check-sync-owned.ts | 253 ++++++++++++++++ .github/workflows/sync-owned-check.yml | 74 +++++ README.md | 18 ++ 4 files changed, 618 insertions(+) create mode 100644 .github/scripts/sync-owned/check-sync-owned.test.ts create mode 100644 .github/scripts/sync-owned/check-sync-owned.ts create mode 100644 .github/workflows/sync-owned-check.yml diff --git a/.github/scripts/sync-owned/check-sync-owned.test.ts b/.github/scripts/sync-owned/check-sync-owned.test.ts new file mode 100644 index 0000000000..13b3151f74 --- /dev/null +++ b/.github/scripts/sync-owned/check-sync-owned.test.ts @@ -0,0 +1,273 @@ +import { describe, expect, test } from "bun:test"; +import { existsSync } from "node:fs"; +import { join } from "node:path"; +import { + SYNC_PR_AUTHOR, + SYNC_PR_BRANCH, + changedGeneratorInput, + check, + classify, + exemptionReason, + formatFailure, + parsePaths, +} from "./check-sync-owned.ts"; + +const ROOT = join(import.meta.dir, "../../.."); + +/** Guarded whatever else the pull request changes. */ +const ALWAYS_GUARDED = [ + "openapi-v2.yaml", + "development/comfy-router/reference.mdx", + "development/comfy-router/quickstart.mdx", + "development/comfy-router/limitations.mdx", + "router-schemas/openai/gpt-image-1.json", +]; + +/** Guarded only when the pull request changes no generator input. */ +const GENERATED_GUARDED = [ + "development/comfy-router/models.mdx", + "development/comfy-router/models/openai/gpt-image-1/code.mdx", +]; + +/** Every path the acceptance criteria name, minus the one that is also an input. */ +const GUARDED_TOGETHER = [ + ...ALWAYS_GUARDED.filter((p) => !p.startsWith("router-schemas/")), + ...GENERATED_GUARDED, +]; + +/** Paths the sync touches partially or not at all, which must stay editable. */ +const EDITABLE = [ + "docs.json", + "development/comfy-router/api.mdx", + "development/comfy-router/queue.mdx", + "development/comfy-router/headers.mdx", + "zh/development/comfy-router/limitations.mdx", + "ja/development/comfy-router/quickstart.mdx", + "ko/development/comfy-router/reference.mdx", + "README.md", + "openapi-cloud.yaml", +]; + +describe("classify: the guarded set", () => { + for (const path of [...ALWAYS_GUARDED, ...GENERATED_GUARDED]) { + test(`flags ${path}`, () => { + const offences = classify([path]); + expect(offences.map((o) => o.path)).toEqual([path]); + expect(offences[0].guidance.length).toBeGreaterThan(0); + }); + } + + test("flags every guarded file in one pull request, not just the first", () => { + expect(classify(GUARDED_TOGETHER).map((o) => o.path)).toEqual(GUARDED_TOGETHER); + }); + + test("flags the router-schemas directory itself, not only files under it", () => { + expect(classify(["router-schemas"])).toHaveLength(1); + }); + + test("tolerates the ./-prefixed and whitespace-padded forms git can hand it", () => { + expect(classify(["./openapi-v2.yaml", " development/comfy-router/limitations.mdx "]).map((o) => o.path)).toEqual([ + "openapi-v2.yaml", + "development/comfy-router/limitations.mdx", + ]); + }); + + test("reports a path once even when the diff lists it twice", () => { + expect(classify(["openapi-v2.yaml", "openapi-v2.yaml"])).toHaveLength(1); + }); + + test("flags a guarded file that the pull request DELETES, which git reports as a plain path", () => { + expect(classify(["development/comfy-router/quickstart.mdx"])).toHaveLength(1); + }); +}); + +describe("classify: what stays editable", () => { + for (const path of EDITABLE) { + test(`passes ${path}`, () => { + expect(classify([path])).toEqual([]); + }); + } + + test("passes a pull request that changes nothing guarded", () => { + expect(classify(EDITABLE)).toEqual([]); + }); + + // The generated tree holds hand-curated `code.yaml` specs next to the generated + // pages, so a prefix match on the models directory would guard the wrong file. + test("does not guard a code.yaml merely because it sits under the generated models tree", () => { + expect(classify(["development/comfy-router/models/bfl/flux-3/code.yaml"])).toEqual([]); + }); + + // `router-schemas` is guarded by prefix; a sibling whose name merely starts with + // the same characters is not part of it. + test("does not guard a sibling directory that shares the router-schemas prefix", () => { + expect(classify(["router-schemas-notes/readme.md"])).toEqual([]); + }); +}); + +/** + * `code-pages:check` FAILS a pull request whose generated pages are stale, so any + * change to a generator input must ship the regenerated pages in the same diff. + * Guarding those pages unconditionally would make the two checks contradict each + * other and a `code.yaml` edit unshippable. + */ +describe("generated pages are a regeneration when an input changed in the same diff", () => { + const CODE_MDX = "development/comfy-router/models/openai/gpt-image-1/code.mdx"; + + test("a code.yaml edit plus its regenerated page passes", () => { + expect(classify(["development/comfy-router/models/openai/gpt-image-1/code.yaml", CODE_MDX])).toEqual([]); + }); + + test("a snippets/comfy-router fragment edit plus regenerated pages passes", () => { + expect(classify(["snippets/comfy-router/model-code-footer.mdx", CODE_MDX, "development/comfy-router/models.mdx"])).toEqual([]); + }); + + test("a generator change plus regenerated pages passes", () => { + expect(classify([".github/scripts/snippets/gen-code-pages.ts", CODE_MDX])).toEqual([]); + }); + + test("the excusal is reported, so the log says why the pages were allowed", () => { + const report = check(["development/comfy-router/models/openai/gpt-image-1/code.yaml", CODE_MDX]); + expect(report.excused).toEqual([CODE_MDX]); + expect(report.regeneratedBecause).toContain("code.yaml"); + }); + + test("a router-schemas edit excuses the regenerated pages but is itself still flagged", () => { + expect(classify(["router-schemas/openai/gpt-image-1.json", CODE_MDX]).map((o) => o.path)).toEqual([ + "router-schemas/openai/gpt-image-1.json", + ]); + }); + + test("the excusal does not spill onto the hand-written pages", () => { + expect( + classify([ + "development/comfy-router/models/openai/gpt-image-1/code.yaml", + CODE_MDX, + "development/comfy-router/quickstart.mdx", + ]).map((o) => o.path), + ).toEqual(["development/comfy-router/quickstart.mdx"]); + }); + + test("a hand-edited generated page with no input change is still refused", () => { + const report = check([CODE_MDX, "development/comfy-router/api.mdx"]); + expect(report.excused).toEqual([]); + expect(report.regeneratedBecause).toBeNull(); + expect(report.offences.map((o) => o.path)).toEqual([CODE_MDX]); + }); +}); + +describe("exemptionReason", () => { + test("exempts the sync bot", () => { + expect(exemptionReason(SYNC_PR_AUTHOR, "matt/some-branch")).toContain(SYNC_PR_AUTHOR); + }); + + test("exempts the fixed sync branch", () => { + expect(exemptionReason("someone-else", SYNC_PR_BRANCH)).toContain(SYNC_PR_BRANCH); + }); + + test("does not exempt an ordinary contributor", () => { + expect(exemptionReason("someone-else", "someone/fix-typo")).toBeNull(); + }); + + test("does not exempt when the event supplied neither signal", () => { + expect(exemptionReason(undefined, undefined)).toBeNull(); + }); + + test("does not exempt a branch that merely resembles the sync branch", () => { + expect(exemptionReason("someone-else", "chore/sync-comfy-api-v2-spec-2")).toBeNull(); + }); +}); + +describe("formatFailure", () => { + const message = formatFailure(classify(GUARDED_TOGETHER)); + + test("names every offending file", () => { + for (const path of GUARDED_TOGETHER) expect(message).toContain(path); + }); + + test("says per file where the edit belongs instead", () => { + expect(message).toContain("Edit the contract upstream"); + expect(message).toContain("Edit the upstream quickstart.mdx"); + expect(message).toContain("Edit the upstream limitations.mdx"); + expect(message).toContain("Edit the sibling code.yaml"); + expect(message).toContain("code-pages:gen"); + }); + + test("covers the router-schemas mirror too", () => { + expect(formatFailure(classify(["router-schemas/openai/gpt-image-1.json"]))).toContain("Edit the upstream contract"); + }); + + // This repository is public, so the failure a contributor reads must not carry + // an issue-tracker identifier or a link into a private repository. + test("leaks no tracker identifier and no private-repository link", () => { + expect(message).not.toMatch(/\b[A-Z]{2,4}-\d{3,}\b/); + expect(message).not.toMatch(/linear\.app/i); + expect(message).not.toMatch(/github\.com/i); + }); +}); + +describe("parsePaths", () => { + test("splits the NUL-separated form git diff -z produces", () => { + expect(parsePaths("a.mdx\0b.mdx\0")).toEqual(["a.mdx", "b.mdx"]); + }); + + test("splits the newline-separated form too", () => { + expect(parsePaths("a.mdx\nb.mdx\n")).toEqual(["a.mdx", "b.mdx"]); + }); + + test("returns nothing for an empty diff", () => { + expect(parsePaths("")).toEqual([]); + expect(parsePaths("\0")).toEqual([]); + }); +}); + +// A guard whose paths have drifted out of the repository silently guards nothing, +// which is the one failure mode that looks exactly like a clean run. +describe("the guarded paths exist in this repository", () => { + const present = [ + "openapi-v2.yaml", + "router-schemas", + "development/comfy-router/reference.mdx", + "development/comfy-router/quickstart.mdx", + "development/comfy-router/limitations.mdx", + "development/comfy-router/models", + "development/comfy-router/models.mdx", + "snippets/comfy-router", + ".github/scripts/snippets", + ]; + for (const path of present) { + test(`${path} is present`, () => { + expect(existsSync(join(ROOT, path))).toBe(true); + }); + } + + test("docs.json and at least one hand-curated code.yaml are present and unguarded", () => { + expect(existsSync(join(ROOT, "docs.json"))).toBe(true); + const first = new Bun.Glob("development/comfy-router/models/**/code.yaml").scanSync(ROOT).next().value; + expect(first).toBeString(); + expect(classify([first!.replaceAll("\\", "/")])).toEqual([]); + }); + + test("at least one generated code.mdx is present and guarded", () => { + const first = new Bun.Glob("development/comfy-router/models/**/code.mdx").scanSync(ROOT).next().value; + expect(first).toBeString(); + expect(classify([first!.replaceAll("\\", "/")])).toHaveLength(1); + }); +}); + +describe("changedGeneratorInput", () => { + test("names the most direct input, not whichever the diff listed first", () => { + const report = check([ + ".github/scripts/snippets/README.md", + "snippets/comfy-router/queue-preview-notice.mdx", + "development/comfy-router/models/openai/gpt-image-1/code.mdx", + ]); + expect(report.regeneratedBecause).toBe("a snippets/comfy-router fragment"); + expect(report.offences).toEqual([]); + }); + + test("prose beside the generator is not an input on its own", () => { + expect(changedGeneratorInput([".github/scripts/snippets/README.md"])).toBeNull(); + expect(changedGeneratorInput([".github/scripts/snippets/gen-code-pages.ts"])).toBe("the code-page generator"); + }); +}); diff --git a/.github/scripts/sync-owned/check-sync-owned.ts b/.github/scripts/sync-owned/check-sync-owned.ts new file mode 100644 index 0000000000..be1e0f18a2 --- /dev/null +++ b/.github/scripts/sync-owned/check-sync-owned.ts @@ -0,0 +1,253 @@ +#!/usr/bin/env bun +/** + * Fail a pull request that edits a file the Comfy API spec sync OWNS. + * + * git diff --name-only --no-renames -z ... \ + * | bun .github/scripts/sync-owned/check-sync-owned.ts + * + * A set of files in this repository is written by the upstream Comfy API v2 spec + * sync and rewritten from upstream sources on every sync run. An edit made here + * renders on docs.comfy.org until the next sync, and is then silently reverted by + * the sync's diff. This check refuses such an edit at pull-request time, while its + * author can still move it upstream. + * + * The guarded list mirrors the sync's own final status check, which runs + * `git status --porcelain --untracked-files=all` over: `openapi-v2.yaml`, + * `development/comfy-router/reference.mdx`, `development/comfy-router/quickstart.mdx`, + * `development/comfy-router/limitations.mdx`, `router-schemas`, + * `development/comfy-router/models`, `development/comfy-router/models.mdx` and + * `docs.json`. Two of those are deliberately NOT guarded here, because the sync + * only owns part of them: + * + * - `docs.json` is co-owned. The sync rewrites the `Models` nav group and the + * model-page redirects; everything else in it is this repository's to edit. + * - `development/comfy-router/models/**\/code.yaml` are HAND-CURATED generator + * inputs that live inside the generated tree. Only the sibling `code.mdx` + * pages are generated, so `code.yaml` stays editable and is in fact where an + * edit to a generated model page belongs. + * + * The two GENERATED page kinds (`development/comfy-router/models.mdx` and the + * per-model `code.mdx`) are guarded CONDITIONALLY, because committing them is + * mandatory whenever a generator input changes: `code-pages:check` fails a pull + * request whose pages are stale, so guarding them unconditionally would put the + * two checks in direct contradiction and make a `code.yaml` edit unshippable. A + * pull request that also changes one of the generator's inputs is therefore + * regenerating rather than hand-editing, and its generated pages pass; the + * freshness check is what proves the regeneration is honest. With no input + * changed, an edit to a generated page is a hand-edit and is refused. + * + * Exempt: the sync's own pull request, identified by its author or its fixed head + * branch. Both are accepted because the sync is allowed to rewrite what it owns. + * + * Reads NUL-separated (or newline-separated) changed paths on stdin. Exits 1 and + * lists every offending file, with per-file guidance, when any is guarded. + */ + +/** The bot that authors the rolling sync pull request. */ +export const SYNC_PR_AUTHOR = "comfy-pr-bot"; +/** The fixed branch the rolling sync pull request lives on. */ +export const SYNC_PR_BRANCH = "chore/sync-comfy-api-v2-spec"; + +const MODELS_DIR = "development/comfy-router/models"; + +export type SyncOwnedRule = { + /** Stable id, used by the tests and by nothing else. */ + id: string; + test: (path: string) => boolean; + /** Where the edit belongs instead. One sentence, rendered under the file. */ + guidance: string; + /** + * True for the pages `bun run code-pages:gen` writes. They are excused when the + * same pull request changes a generator input, since the freshness check then + * requires them to be committed. + */ + generated?: true; +}; + +/** + * The inputs `bun run code-pages:gen` reads. Mirrors the path filter of + * `code-pages-check.yml`, minus its two outputs (`docs.json`, and the generated + * pages themselves) and minus `package.json`, which only names the script. + */ +export const GENERATOR_INPUTS: { label: string; test: (path: string) => boolean }[] = [ + { + label: "a model's code.yaml spec", + test: (p) => p.startsWith(`${MODELS_DIR}/`) && p.endsWith("/code.yaml"), + }, + { + label: "the router-schemas mirror", + test: (p) => p === "router-schemas" || p.startsWith("router-schemas/"), + }, + { + label: "a snippets/comfy-router fragment", + test: (p) => p.startsWith("snippets/comfy-router/"), + }, + { + // `.ts` only: the prose beside the generator cannot change what it emits. + label: "the code-page generator", + test: (p) => p.startsWith(".github/scripts/snippets/") && p.endsWith(".ts"), + }, +]; + +/** + * The generator input this diff changes, if any. Iterates the inputs rather than + * the paths, so the label names the most direct input the pull request touches + * instead of whichever one the diff happened to list first. + */ +export function changedGeneratorInput(paths: readonly string[]): string | null { + const input = GENERATOR_INPUTS.find((i) => paths.some((p) => i.test(p))); + return input ? input.label : null; +} + +/** + * First match wins, so the `code.mdx` rule is written to exclude `code.yaml` + * rather than relying on ordering to protect it. + */ +export const SYNC_OWNED_RULES: SyncOwnedRule[] = [ + { + id: "openapi-v2", + test: (p) => p === "openapi-v2.yaml", + guidance: + "Vendored projection of the public Comfy API v2 specification. Edit the API contract upstream; the sync reprojects this file on every run.", + }, + { + id: "reference", + test: (p) => p === "development/comfy-router/reference.mdx", + guidance: + "GENERATED from the upstream Comfy API contract. Edit the contract upstream, not this copy.", + }, + { + id: "quickstart", + test: (p) => p === "development/comfy-router/quickstart.mdx", + guidance: + "Hand-written upstream and published here verbatim. Edit the upstream quickstart.mdx.", + }, + { + id: "limitations", + test: (p) => p === "development/comfy-router/limitations.mdx", + guidance: + "Hand-written upstream and published here verbatim. Edit the upstream limitations.mdx.", + }, + { + id: "router-schemas", + test: (p) => p === "router-schemas" || p.startsWith("router-schemas/"), + guidance: + "Mirror of the upstream GET /v2/models/{id}/openapi.json documents. The whole directory is re-mirrored on every sync. Edit the upstream contract.", + }, + { + id: "models-index", + test: (p) => p === `${MODELS_DIR}.mdx`, + guidance: + "GENERATED provider index, hand-edited: this pull request changes none of the generator's inputs. Change a model's code.yaml (or the upstream contract) and re-run `bun run code-pages:gen`.", + generated: true, + }, + { + id: "model-code-page", + test: (p) => p.startsWith(`${MODELS_DIR}/`) && p.endsWith("/code.mdx"), + guidance: + "GENERATED model page, hand-edited: this pull request changes none of the generator's inputs. Edit the sibling code.yaml (or the upstream contract) and re-run `bun run code-pages:gen`.", + generated: true, + }, +]; + +export type Offence = { path: string; guidance: string }; + +/** Normalize a path as git prints it: repo-relative, POSIX, no leading `./`. */ +const normalize = (path: string) => path.trim().replace(/^\.\//, ""); + +export type Report = { + offences: Offence[]; + /** Generated pages let through because the same diff changes a generator input. */ + excused: string[]; + /** Set when generated-page rules were suppressed, naming the input that did it. */ + regeneratedBecause: string | null; + /** How many paths were considered, after normalizing and de-duplicating. */ + checked: number; +}; + +export function check(rawPaths: readonly string[]): Report { + const paths: string[] = []; + const seen = new Set(); + for (const raw of rawPaths) { + const path = normalize(raw); + if (!path || seen.has(path)) continue; + seen.add(path); + paths.push(path); + } + + const regeneratedBecause = changedGeneratorInput(paths); + const offences: Offence[] = []; + const excused: string[] = []; + for (const path of paths) { + const rule = SYNC_OWNED_RULES.find((r) => r.test(path)); + if (!rule) continue; + if (rule.generated && regeneratedBecause) { + excused.push(path); + continue; + } + offences.push({ path, guidance: rule.guidance }); + } + return { offences, excused, regeneratedBecause, checked: paths.length }; +} + +/** Convenience wrapper for callers that only want the offending files. */ +export const classify = (paths: readonly string[]): Offence[] => check(paths).offences; + +/** + * The exemption reason, or `null` when the pull request is not the sync's own. + * Author OR branch: a re-pushed sync branch and a bot-authored run each have to + * pass on their own, since neither signal is guaranteed present in every event. + */ +export function exemptionReason(author: string | undefined, headRef: string | undefined): string | null { + if (author === SYNC_PR_AUTHOR) return `pull request is authored by ${SYNC_PR_AUTHOR}`; + if (headRef === SYNC_PR_BRANCH) return `pull request head branch is ${SYNC_PR_BRANCH}`; + return null; +} + +export function formatFailure(offences: readonly Offence[]): string { + const lines = [ + `❌ This pull request edits ${offences.length} file(s) owned by the Comfy API spec sync:`, + "", + ]; + for (const { path, guidance } of offences) { + lines.push(` ${path}`); + lines.push(` ${guidance}`); + lines.push(""); + } + lines.push( + "These files are rewritten from upstream sources on every sync run, so an edit made here is published now and reverted by the next sync. Move the change upstream (or into the generator input named above) and drop it from this pull request.", + ); + lines.push( + "The open sync pull request on the chore/sync-comfy-api-v2-spec branch describes each file's source; see also \"Sync-owned files\" in README.md.", + ); + return lines.join("\n"); +} + +/** Split on NUL or newline, so the script works with or without `git diff -z`. */ +export function parsePaths(input: string): string[] { + return input.split(/[\0\n]/).map(normalize).filter(Boolean); +} + +async function main() { + const reason = exemptionReason(process.env.PR_AUTHOR, process.env.PR_HEAD_REF); + if (reason) { + console.log(`✅ Sync-owned file check skipped: ${reason}.`); + return; + } + + const report = check(parsePaths(await Bun.stdin.text())); + if (report.excused.length > 0) { + console.log( + `ℹ️ ${report.excused.length} generated page(s) allowed: this pull request also changes ${report.regeneratedBecause}, so they are a regeneration. code-pages:check verifies they are fresh.`, + ); + } + if (report.offences.length === 0) { + console.log(`✅ No sync-owned files edited (${report.checked} changed file(s) checked).`); + return; + } + + console.error(formatFailure(report.offences)); + process.exitCode = 1; +} + +if (import.meta.main) await main(); diff --git a/.github/workflows/sync-owned-check.yml b/.github/workflows/sync-owned-check.yml new file mode 100644 index 0000000000..fd06105b70 --- /dev/null +++ b/.github/workflows/sync-owned-check.yml @@ -0,0 +1,74 @@ +name: Sync-Owned Files Check + +# Refuse a pull request that edits a file the Comfy API spec sync owns and +# rewrites on every run. Such an edit publishes now and is reverted by the next +# sync; this fails it while its author can still move the change upstream. +# +# The path filter IS the guarded set, so a pull request that touches none of +# these files never starts this workflow. `docs.json` (co-owned) and the +# hand-curated `development/comfy-router/models/**/code.yaml` generator inputs +# are deliberately absent from it. See the script's header comment for why. + +on: + pull_request: + paths: + - 'openapi-v2.yaml' + - 'development/comfy-router/reference.mdx' + - 'development/comfy-router/quickstart.mdx' + - 'development/comfy-router/limitations.mdx' + - 'development/comfy-router/models.mdx' + - 'development/comfy-router/models/**/code.mdx' + - 'router-schemas/**' + - '.github/scripts/sync-owned/**' + - '.github/workflows/sync-owned-check.yml' + +permissions: + contents: read + +jobs: + sync-owned: + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Setup Bun + uses: oven-sh/setup-bun@v2 + with: + bun-version: latest + + - name: Check for edits to sync-owned files + env: + # Read through the environment rather than interpolated into the shell: + # a head branch name is contributor-controlled text. + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + PR_HEAD_REF: ${{ github.event.pull_request.head.ref }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + # --no-renames so a guarded file renamed away is reported as a deletion + # of the guarded path rather than only as its new name. + git diff --name-only --no-renames -z "$BASE_SHA...$HEAD_SHA" \ + | bun .github/scripts/sync-owned/check-sync-owned.ts + + sync-owned-script-tests: + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + persist-credentials: false + + - name: Setup Bun + uses: oven-sh/setup-bun@v2 + with: + bun-version: latest + + - name: Run the sync-owned check unit tests + run: bun test ./.github/scripts/sync-owned/ diff --git a/README.md b/README.md index 1077406b64..0b02cbd1a3 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,24 @@ Or talk to us on our [discord](https://discord.com/invite/comfyorg) The documentation is built with Mintlify, please refer to [Mintlify documentation](https://mintlify.com/docs) to learn how to use it. +### Sync-owned files + +Some files in this repository are written by the Comfy API v2 specification sync and rewritten from upstream sources every time it runs. Editing them here publishes the change until the next sync, which then silently reverts it. Do not edit these files in a PR: + +| File | Where the edit belongs | +|------|------------------------| +| `openapi-v2.yaml` | The API contract upstream. This file is a vendored projection of it. | +| `development/comfy-router/reference.mdx` | The API contract upstream. This page is generated from it. | +| `development/comfy-router/quickstart.mdx` | The upstream `quickstart.mdx`, which is published here verbatim. | +| `development/comfy-router/limitations.mdx` | The upstream `limitations.mdx`, which is published here verbatim. | +| `router-schemas/**` | The API contract upstream. The whole directory is re-mirrored on every sync. | +| `development/comfy-router/models.mdx` | A model's `code.yaml`, or the upstream contract. Regenerate with `bun run code-pages:gen`. | +| `development/comfy-router/models/**/code.mdx` | The sibling `code.yaml`, or the upstream contract. Regenerate with `bun run code-pages:gen`. | + +Two things inside that tree stay editable: `docs.json` (the sync rewrites only the `Models` nav group and the model-page redirects) and the hand-curated `development/comfy-router/models/**/code.yaml` generator inputs, which are where a change to a generated model page belongs. + +The `Sync-Owned Files Check` workflow fails a PR that touches any of the guarded paths and prints, per file, where the edit belongs. The sync's own PR is exempt. Localized copies under `zh/`, `ja/` and `ko/` are maintained by the i18n sync and are not covered by this check. + ### i18n Contributions English MDX at the repo root is the **source of truth**. Translations mirror the same relative paths under language directories (for example `zh/get_started/introduction.mdx`, `ja/get_started/introduction.mdx`, `ko/get_started/introduction.mdx`). Reusable fragments live in `snippets/` with per-language copies under `snippets/zh/`, `snippets/ja/`, `snippets/ko/`, and so on. From 86a6ba05a25846baef5e89b7d5c9be9003404935 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Fri, 18 Sep 2026 01:34:49 +0000 Subject: [PATCH 2/4] ci(sync-owned): judge generated pages on freshness, not on the shape of the diff A sweep of the last 80 merged pull requests found a regeneration-only PR (#1610, 'regenerate Router code pages to restore freshness') that changes no generator input at all, so the input-change heuristic failed it. What actually separates a regeneration from a hand-edit is whether the committed page matches the generator's output, which is the assertion code-pages:check already makes. The workflow runs it and passes the verdict in. code-pages-check.yml's path filter omits development/comfy-router/models.mdx, so a PR that hand-edits only the generated provider index never starts it. Re-asserting freshness here closes that gap. --- .../sync-owned/check-sync-owned.test.ts | 86 +++++++---------- .../scripts/sync-owned/check-sync-owned.ts | 94 +++++++------------ .github/workflows/sync-owned-check.yml | 11 +++ README.md | 2 +- 4 files changed, 79 insertions(+), 114 deletions(-) diff --git a/.github/scripts/sync-owned/check-sync-owned.test.ts b/.github/scripts/sync-owned/check-sync-owned.test.ts index 13b3151f74..fff5657352 100644 --- a/.github/scripts/sync-owned/check-sync-owned.test.ts +++ b/.github/scripts/sync-owned/check-sync-owned.test.ts @@ -4,7 +4,6 @@ import { join } from "node:path"; import { SYNC_PR_AUTHOR, SYNC_PR_BRANCH, - changedGeneratorInput, check, classify, exemptionReason, @@ -23,17 +22,14 @@ const ALWAYS_GUARDED = [ "router-schemas/openai/gpt-image-1.json", ]; -/** Guarded only when the pull request changes no generator input. */ +/** Guarded only when the committed pages are stale, i.e. hand-edited. */ const GENERATED_GUARDED = [ "development/comfy-router/models.mdx", "development/comfy-router/models/openai/gpt-image-1/code.mdx", ]; -/** Every path the acceptance criteria name, minus the one that is also an input. */ -const GUARDED_TOGETHER = [ - ...ALWAYS_GUARDED.filter((p) => !p.startsWith("router-schemas/")), - ...GENERATED_GUARDED, -]; +/** Every path the guard covers, as a single stale-page pull request would present them. */ +const GUARDED_TOGETHER = [...ALWAYS_GUARDED, ...GENERATED_GUARDED]; /** Paths the sync touches partially or not at all, which must stay editable. */ const EDITABLE = [ @@ -41,6 +37,9 @@ const EDITABLE = [ "development/comfy-router/api.mdx", "development/comfy-router/queue.mdx", "development/comfy-router/headers.mdx", + "development/comfy-router/models/openai/gpt-image-1/code.yaml", + "snippets/comfy-router/model-code-footer.mdx", + ".github/scripts/snippets/gen-code-pages.ts", "zh/development/comfy-router/limitations.mdx", "ja/development/comfy-router/quickstart.mdx", "ko/development/comfy-router/reference.mdx", @@ -106,53 +105,51 @@ describe("classify: what stays editable", () => { }); /** - * `code-pages:check` FAILS a pull request whose generated pages are stale, so any - * change to a generator input must ship the regenerated pages in the same diff. - * Guarding those pages unconditionally would make the two checks contradict each - * other and a `code.yaml` edit unshippable. + * Committing a regenerated page is mandatory whenever a generator input changes, + * and an honest "regenerate to restore freshness" pull request changes no input + * at all. Freshness, not the shape of the diff, is what separates a regeneration + * from a hand-edit: a page that matches the generator's output IS its output. */ -describe("generated pages are a regeneration when an input changed in the same diff", () => { +describe("generated pages are guarded on freshness", () => { const CODE_MDX = "development/comfy-router/models/openai/gpt-image-1/code.mdx"; + const MODELS_MDX = "development/comfy-router/models.mdx"; + const FRESH = { generatedPagesFresh: true }; test("a code.yaml edit plus its regenerated page passes", () => { - expect(classify(["development/comfy-router/models/openai/gpt-image-1/code.yaml", CODE_MDX])).toEqual([]); - }); - - test("a snippets/comfy-router fragment edit plus regenerated pages passes", () => { - expect(classify(["snippets/comfy-router/model-code-footer.mdx", CODE_MDX, "development/comfy-router/models.mdx"])).toEqual([]); + expect(classify(["development/comfy-router/models/openai/gpt-image-1/code.yaml", CODE_MDX], FRESH)).toEqual([]); }); - test("a generator change plus regenerated pages passes", () => { - expect(classify([".github/scripts/snippets/gen-code-pages.ts", CODE_MDX])).toEqual([]); + test("a regeneration that changes no input at all passes", () => { + expect(classify([CODE_MDX, MODELS_MDX, "docs.json"], FRESH)).toEqual([]); }); test("the excusal is reported, so the log says why the pages were allowed", () => { - const report = check(["development/comfy-router/models/openai/gpt-image-1/code.yaml", CODE_MDX]); - expect(report.excused).toEqual([CODE_MDX]); - expect(report.regeneratedBecause).toContain("code.yaml"); + const report = check([CODE_MDX, MODELS_MDX], FRESH); + expect(report.excused).toEqual([CODE_MDX, MODELS_MDX]); + expect(report.offences).toEqual([]); }); - test("a router-schemas edit excuses the regenerated pages but is itself still flagged", () => { - expect(classify(["router-schemas/openai/gpt-image-1.json", CODE_MDX]).map((o) => o.path)).toEqual([ + test("a router-schemas edit beside fresh pages is still flagged on its own", () => { + expect(classify(["router-schemas/openai/gpt-image-1.json", CODE_MDX], FRESH).map((o) => o.path)).toEqual([ "router-schemas/openai/gpt-image-1.json", ]); }); test("the excusal does not spill onto the hand-written pages", () => { - expect( - classify([ - "development/comfy-router/models/openai/gpt-image-1/code.yaml", - CODE_MDX, - "development/comfy-router/quickstart.mdx", - ]).map((o) => o.path), - ).toEqual(["development/comfy-router/quickstart.mdx"]); + expect(classify([CODE_MDX, "development/comfy-router/quickstart.mdx"], FRESH).map((o) => o.path)).toEqual([ + "development/comfy-router/quickstart.mdx", + ]); }); - test("a hand-edited generated page with no input change is still refused", () => { - const report = check([CODE_MDX, "development/comfy-router/api.mdx"]); + test("a stale generated page is a hand-edit and is refused", () => { + const report = check([CODE_MDX, MODELS_MDX], { generatedPagesFresh: false }); expect(report.excused).toEqual([]); - expect(report.regeneratedBecause).toBeNull(); - expect(report.offences.map((o) => o.path)).toEqual([CODE_MDX]); + expect(report.offences.map((o) => o.path)).toEqual([CODE_MDX, MODELS_MDX]); + for (const offence of report.offences) expect(offence.guidance).toContain("hand-edit"); + }); + + test("an unknown freshness verdict guards rather than waving through", () => { + expect(classify([CODE_MDX])).toHaveLength(1); }); }); @@ -232,8 +229,6 @@ describe("the guarded paths exist in this repository", () => { "development/comfy-router/limitations.mdx", "development/comfy-router/models", "development/comfy-router/models.mdx", - "snippets/comfy-router", - ".github/scripts/snippets", ]; for (const path of present) { test(`${path} is present`, () => { @@ -254,20 +249,3 @@ describe("the guarded paths exist in this repository", () => { expect(classify([first!.replaceAll("\\", "/")])).toHaveLength(1); }); }); - -describe("changedGeneratorInput", () => { - test("names the most direct input, not whichever the diff listed first", () => { - const report = check([ - ".github/scripts/snippets/README.md", - "snippets/comfy-router/queue-preview-notice.mdx", - "development/comfy-router/models/openai/gpt-image-1/code.mdx", - ]); - expect(report.regeneratedBecause).toBe("a snippets/comfy-router fragment"); - expect(report.offences).toEqual([]); - }); - - test("prose beside the generator is not an input on its own", () => { - expect(changedGeneratorInput([".github/scripts/snippets/README.md"])).toBeNull(); - expect(changedGeneratorInput([".github/scripts/snippets/gen-code-pages.ts"])).toBe("the code-page generator"); - }); -}); diff --git a/.github/scripts/sync-owned/check-sync-owned.ts b/.github/scripts/sync-owned/check-sync-owned.ts index be1e0f18a2..4857236527 100644 --- a/.github/scripts/sync-owned/check-sync-owned.ts +++ b/.github/scripts/sync-owned/check-sync-owned.ts @@ -27,14 +27,19 @@ * edit to a generated model page belongs. * * The two GENERATED page kinds (`development/comfy-router/models.mdx` and the - * per-model `code.mdx`) are guarded CONDITIONALLY, because committing them is - * mandatory whenever a generator input changes: `code-pages:check` fails a pull - * request whose pages are stale, so guarding them unconditionally would put the - * two checks in direct contradiction and make a `code.yaml` edit unshippable. A - * pull request that also changes one of the generator's inputs is therefore - * regenerating rather than hand-editing, and its generated pages pass; the - * freshness check is what proves the regeneration is honest. With no input - * changed, an edit to a generated page is a hand-edit and is refused. + * per-model `code.mdx`) are guarded CONDITIONALLY, on FRESHNESS. Committing them + * is mandatory whenever a generator input changes, so guarding them + * unconditionally would make a `code.yaml` edit unshippable and would fail an + * honest "regenerate to restore freshness" pull request as well. A page that + * matches `bun run code-pages:gen`'s output is that generator's output by + * definition, however it got there; a page that does NOT match is a hand-edit. + * The caller passes the verdict in as `generatedPagesFresh`, since running the + * generator is the workflow's job and not this script's. + * + * `code-pages-check.yml` makes the same freshness assertion, but its path filter + * does not list `development/comfy-router/models.mdx`, so a pull request that + * hand-edits only the generated provider index never starts it. That gap is why + * this check re-asserts freshness rather than deferring to it. * * Exempt: the sync's own pull request, identified by its author or its fixed head * branch. Both are accepted because the sync is allowed to rewrite what it owns. @@ -51,54 +56,18 @@ export const SYNC_PR_BRANCH = "chore/sync-comfy-api-v2-spec"; const MODELS_DIR = "development/comfy-router/models"; export type SyncOwnedRule = { - /** Stable id, used by the tests and by nothing else. */ + /** Stable label for this rule, for readers and diagnostics. */ id: string; test: (path: string) => boolean; /** Where the edit belongs instead. One sentence, rendered under the file. */ guidance: string; /** - * True for the pages `bun run code-pages:gen` writes. They are excused when the - * same pull request changes a generator input, since the freshness check then - * requires them to be committed. + * True for the pages `bun run code-pages:gen` writes. Excused when those pages + * are fresh, which makes them the generator's output rather than a hand-edit. */ generated?: true; }; -/** - * The inputs `bun run code-pages:gen` reads. Mirrors the path filter of - * `code-pages-check.yml`, minus its two outputs (`docs.json`, and the generated - * pages themselves) and minus `package.json`, which only names the script. - */ -export const GENERATOR_INPUTS: { label: string; test: (path: string) => boolean }[] = [ - { - label: "a model's code.yaml spec", - test: (p) => p.startsWith(`${MODELS_DIR}/`) && p.endsWith("/code.yaml"), - }, - { - label: "the router-schemas mirror", - test: (p) => p === "router-schemas" || p.startsWith("router-schemas/"), - }, - { - label: "a snippets/comfy-router fragment", - test: (p) => p.startsWith("snippets/comfy-router/"), - }, - { - // `.ts` only: the prose beside the generator cannot change what it emits. - label: "the code-page generator", - test: (p) => p.startsWith(".github/scripts/snippets/") && p.endsWith(".ts"), - }, -]; - -/** - * The generator input this diff changes, if any. Iterates the inputs rather than - * the paths, so the label names the most direct input the pull request touches - * instead of whichever one the diff happened to list first. - */ -export function changedGeneratorInput(paths: readonly string[]): string | null { - const input = GENERATOR_INPUTS.find((i) => paths.some((p) => i.test(p))); - return input ? input.label : null; -} - /** * First match wins, so the `code.mdx` rule is written to exclude `code.yaml` * rather than relying on ordering to protect it. @@ -138,14 +107,14 @@ export const SYNC_OWNED_RULES: SyncOwnedRule[] = [ id: "models-index", test: (p) => p === `${MODELS_DIR}.mdx`, guidance: - "GENERATED provider index, hand-edited: this pull request changes none of the generator's inputs. Change a model's code.yaml (or the upstream contract) and re-run `bun run code-pages:gen`.", + "GENERATED provider index, and it does not match the generator's output, so this is a hand-edit. Change a model's code.yaml (or the upstream contract) and re-run `bun run code-pages:gen`.", generated: true, }, { id: "model-code-page", test: (p) => p.startsWith(`${MODELS_DIR}/`) && p.endsWith("/code.mdx"), guidance: - "GENERATED model page, hand-edited: this pull request changes none of the generator's inputs. Edit the sibling code.yaml (or the upstream contract) and re-run `bun run code-pages:gen`.", + "GENERATED model page, and it does not match the generator's output, so this is a hand-edit. Edit the sibling code.yaml (or the upstream contract) and re-run `bun run code-pages:gen`.", generated: true, }, ]; @@ -157,15 +126,21 @@ const normalize = (path: string) => path.trim().replace(/^\.\//, ""); export type Report = { offences: Offence[]; - /** Generated pages let through because the same diff changes a generator input. */ + /** Generated pages let through because they match the generator's output. */ excused: string[]; - /** Set when generated-page rules were suppressed, naming the input that did it. */ - regeneratedBecause: string | null; /** How many paths were considered, after normalizing and de-duplicating. */ checked: number; }; -export function check(rawPaths: readonly string[]): Report { +export type CheckOptions = { + /** + * Whether the checkout's generated pages match `bun run code-pages:gen`. + * Defaults to `false`, so an unknown verdict guards rather than waves through. + */ + generatedPagesFresh?: boolean; +}; + +export function check(rawPaths: readonly string[], options: CheckOptions = {}): Report { const paths: string[] = []; const seen = new Set(); for (const raw of rawPaths) { @@ -175,23 +150,23 @@ export function check(rawPaths: readonly string[]): Report { paths.push(path); } - const regeneratedBecause = changedGeneratorInput(paths); const offences: Offence[] = []; const excused: string[] = []; for (const path of paths) { const rule = SYNC_OWNED_RULES.find((r) => r.test(path)); if (!rule) continue; - if (rule.generated && regeneratedBecause) { + if (rule.generated && options.generatedPagesFresh) { excused.push(path); continue; } offences.push({ path, guidance: rule.guidance }); } - return { offences, excused, regeneratedBecause, checked: paths.length }; + return { offences, excused, checked: paths.length }; } /** Convenience wrapper for callers that only want the offending files. */ -export const classify = (paths: readonly string[]): Offence[] => check(paths).offences; +export const classify = (paths: readonly string[], options?: CheckOptions): Offence[] => + check(paths, options).offences; /** * The exemption reason, or `null` when the pull request is not the sync's own. @@ -235,10 +210,11 @@ async function main() { return; } - const report = check(parsePaths(await Bun.stdin.text())); + const generatedPagesFresh = process.env.GENERATED_PAGES_FRESH === "true"; + const report = check(parsePaths(await Bun.stdin.text()), { generatedPagesFresh }); if (report.excused.length > 0) { console.log( - `ℹ️ ${report.excused.length} generated page(s) allowed: this pull request also changes ${report.regeneratedBecause}, so they are a regeneration. code-pages:check verifies they are fresh.`, + `ℹ️ ${report.excused.length} generated page(s) allowed: they match what \`bun run code-pages:gen\` emits, so they are a regeneration and not a hand-edit.`, ); } if (report.offences.length === 0) { diff --git a/.github/workflows/sync-owned-check.yml b/.github/workflows/sync-owned-check.yml index fd06105b70..a43fd4ecb6 100644 --- a/.github/workflows/sync-owned-check.yml +++ b/.github/workflows/sync-owned-check.yml @@ -41,6 +41,16 @@ jobs: with: bun-version: latest + # A generated page that matches the generator's output is that output, + # however it got there, so it is a regeneration rather than a hand-edit and + # the guard lets it through. `code-pages-check.yml` asserts the same thing, + # but its path filter omits `development/comfy-router/models.mdx`, so a pull + # request that hand-edits only the provider index never starts it. + - name: Are the generated code pages fresh? + id: fresh + continue-on-error: true + run: bun run code-pages:check + - name: Check for edits to sync-owned files env: # Read through the environment rather than interpolated into the shell: @@ -49,6 +59,7 @@ jobs: PR_HEAD_REF: ${{ github.event.pull_request.head.ref }} BASE_SHA: ${{ github.event.pull_request.base.sha }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} + GENERATED_PAGES_FRESH: ${{ steps.fresh.outcome == 'success' }} run: | set -euo pipefail # --no-renames so a guarded file renamed away is reported as a deletion diff --git a/README.md b/README.md index 0b02cbd1a3..401ca8535a 100644 --- a/README.md +++ b/README.md @@ -89,7 +89,7 @@ Some files in this repository are written by the Comfy API v2 specification sync Two things inside that tree stay editable: `docs.json` (the sync rewrites only the `Models` nav group and the model-page redirects) and the hand-curated `development/comfy-router/models/**/code.yaml` generator inputs, which are where a change to a generated model page belongs. -The `Sync-Owned Files Check` workflow fails a PR that touches any of the guarded paths and prints, per file, where the edit belongs. The sync's own PR is exempt. Localized copies under `zh/`, `ja/` and `ko/` are maintained by the i18n sync and are not covered by this check. +The `Sync-Owned Files Check` workflow fails a PR that touches any of the guarded paths and prints, per file, where the edit belongs. The last two rows are the generator's output, so they are judged on freshness instead: committing pages that match `bun run code-pages:gen` is a regeneration and passes, while a page that does not match is a hand-edit and fails. The sync's own PR is exempt. Localized copies under `zh/`, `ja/` and `ko/` are maintained by the i18n sync and are not covered by this check. ### i18n Contributions From 971fb01215b68f4d500904668deaa4dbcf4ef4ed Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Fri, 18 Sep 2026 01:35:46 +0000 Subject: [PATCH 3/4] ci(sync-owned): point the failure footer at each file's own entry --- .github/scripts/sync-owned/check-sync-owned.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/scripts/sync-owned/check-sync-owned.ts b/.github/scripts/sync-owned/check-sync-owned.ts index 4857236527..dd4417ebd0 100644 --- a/.github/scripts/sync-owned/check-sync-owned.ts +++ b/.github/scripts/sync-owned/check-sync-owned.ts @@ -190,7 +190,7 @@ export function formatFailure(offences: readonly Offence[]): string { lines.push(""); } lines.push( - "These files are rewritten from upstream sources on every sync run, so an edit made here is published now and reverted by the next sync. Move the change upstream (or into the generator input named above) and drop it from this pull request.", + "These files are rewritten from upstream sources on every sync run, so an edit made here is published now and reverted by the next sync. Move each change to where its entry above says it belongs, then drop it from this pull request.", ); lines.push( "The open sync pull request on the chore/sync-comfy-api-v2-spec branch describes each file's source; see also \"Sync-owned files\" in README.md.", From 470fd8ad0fa989a62359e780bb2da2adc7f6727b Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Fri, 18 Sep 2026 01:36:19 +0000 Subject: [PATCH 4/4] docs(sync-owned): correct the path-filter note in the workflow header --- .github/workflows/sync-owned-check.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/sync-owned-check.yml b/.github/workflows/sync-owned-check.yml index a43fd4ecb6..92d984f326 100644 --- a/.github/workflows/sync-owned-check.yml +++ b/.github/workflows/sync-owned-check.yml @@ -4,10 +4,10 @@ name: Sync-Owned Files Check # rewrites on every run. Such an edit publishes now and is reverted by the next # sync; this fails it while its author can still move the change upstream. # -# The path filter IS the guarded set, so a pull request that touches none of -# these files never starts this workflow. `docs.json` (co-owned) and the -# hand-curated `development/comfy-router/models/**/code.yaml` generator inputs -# are deliberately absent from it. See the script's header comment for why. +# The path filter is the guarded set plus this check's own files, so a pull +# request that touches none of them never starts this workflow. `docs.json` +# (co-owned) and the hand-curated `development/comfy-router/models/**/code.yaml` +# generator inputs are deliberately absent. See the script's header comment. on: pull_request: