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 000000000..fff565735 --- /dev/null +++ b/.github/scripts/sync-owned/check-sync-owned.test.ts @@ -0,0 +1,251 @@ +import { describe, expect, test } from "bun:test"; +import { existsSync } from "node:fs"; +import { join } from "node:path"; +import { + SYNC_PR_AUTHOR, + SYNC_PR_BRANCH, + 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 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 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 = [ + "docs.json", + "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", + "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([]); + }); +}); + +/** + * 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 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], FRESH)).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([CODE_MDX, MODELS_MDX], FRESH); + expect(report.excused).toEqual([CODE_MDX, MODELS_MDX]); + expect(report.offences).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([CODE_MDX, "development/comfy-router/quickstart.mdx"], FRESH).map((o) => o.path)).toEqual([ + "development/comfy-router/quickstart.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.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); + }); +}); + +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", + ]; + 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); + }); +}); 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 000000000..dd4417ebd --- /dev/null +++ b/.github/scripts/sync-owned/check-sync-owned.ts @@ -0,0 +1,229 @@ +#!/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, 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. + * + * 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 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. Excused when those pages + * are fresh, which makes them the generator's output rather than a hand-edit. + */ + generated?: true; +}; + +/** + * 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, 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, 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, + }, +]; + +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 they match the generator's output. */ + excused: string[]; + /** How many paths were considered, after normalizing and de-duplicating. */ + checked: number; +}; + +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) { + const path = normalize(raw); + if (!path || seen.has(path)) continue; + seen.add(path); + paths.push(path); + } + + 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 && options.generatedPagesFresh) { + excused.push(path); + continue; + } + offences.push({ path, guidance: rule.guidance }); + } + return { offences, excused, checked: paths.length }; +} + +/** Convenience wrapper for callers that only want the offending files. */ +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. + * 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 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.", + ); + 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 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: they match what \`bun run code-pages:gen\` emits, so they are a regeneration and not a hand-edit.`, + ); + } + 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 000000000..92d984f32 --- /dev/null +++ b/.github/workflows/sync-owned-check.yml @@ -0,0 +1,85 @@ +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 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: + 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 + + # 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: + # 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 }} + GENERATED_PAGES_FRESH: ${{ steps.fresh.outcome == 'success' }} + 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 1077406b6..401ca8535 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 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 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.