diff --git a/.github/scripts/arch-lint.ts b/.github/scripts/arch-lint.ts index 1d3cd2f5c..230b86ccc 100644 --- a/.github/scripts/arch-lint.ts +++ b/.github/scripts/arch-lint.ts @@ -1,14 +1,10 @@ /** - * The architecture lint (bun run lint:arch): the imports between the layers - * of src/ are exactly the edges architecture.yml declares - an undeclared - * edge and a stale allowance both fail. gen-docs.ts renders the same - * declaration as the module map. dependency-cruiser was the intended tool but - * cannot cruise this repository: it needs the TypeScript compiler API, which - * the pinned typescript 7 no longer ships, so it resolves nothing. - * - * An edge is any way one file names another: the runtime loads the - * changed-sections scanner reads, plus type-only imports, re-exports, and - * `import("./x.js").T` and `import X = require("./x.js")` in type positions. + * The architecture lint (bun run lint:arch): src/ imports between layers must be exactly the edges architecture.yml + * declares; an undeclared edge and a stale allowance both fail. dependency-cruiser was the intended tool, but it + * needs the TypeScript compiler API, which the pinned typescript 7 no longer ships, so it resolves nothing here. + * runtime loads (what the changed-sections scanner reads) -> edges + * type-only imports, re-exports -> edges too + * `import("./x.js").T`, `import X = require("./x.js")` in type positions -> edges too */ import { readdirSync, readFileSync } from "node:fs"; @@ -31,7 +27,6 @@ export function readArchitecture(root: string): Architecture { return parseYaml(readFileSync(join(root, ARCHITECTURE_PATH), "utf8")) as Architecture; } -/** The layer owning a repo-relative path, or undefined. */ function layerOf(arch: Architecture, path: string): string | undefined { return Object.entries(arch.layers).find(([, paths]) => paths.some((owned) => (owned.endsWith("/") ? path.startsWith(owned) : path === owned)), @@ -53,7 +48,6 @@ function* nodesOf(value: unknown): Generator { } } -/** Every relative specifier `text` names, runtime and type-level alike. */ export function importSpecifiers(text: string, file: string): string[] { const { program, module } = parseSync(file, text); const typeLevel = [...nodesOf(program)].flatMap((node) => { @@ -76,11 +70,6 @@ export function importSpecifiers(text: string, file: string): string[] { return [...all].filter((specifier) => /^\.\.?\//.test(specifier)); } -/** - * The lint verdict for `root`: an import between layers the declaration lacks - * (naming both files), a declared edge no file draws, or a cruised file outside - * every layer. Empty means the declaration is exactly the tree. - */ export function lintArchitecture(root: string, arch = readArchitecture(root)): string[] { const excluded = arch.exclude.map((pattern) => new Bun.Glob(pattern)); const drawn = new Map(); @@ -129,7 +118,7 @@ export function lintArchitecture(root: string, arch = readArchitecture(root)): s return problems; } -/** The module map over the DECLARED edges; a hyphen in a layer name is edge syntax to mermaid. */ +/** A hyphen in a layer name is edge syntax to mermaid, so ids swap it for an underscore. */ export function renderArchitectureMermaid(arch: Architecture): string { const id = (layer: string): string => layer.replace(/-/g, "_"); return [ diff --git a/.github/scripts/changed-sections.ts b/.github/scripts/changed-sections.ts index 44e31858c..1692f8c71 100644 --- a/.github/scripts/changed-sections.ts +++ b/.github/scripts/changed-sections.ts @@ -1,24 +1,14 @@ /** - * Diff-aware section selector for the PR e2e smoke job. Given the files a PR - * changed, decide which settings sections the smoke job must exercise, so a PR - * touching one section runs that section's scenarios and fuzz rather than the - * whole corpus, and a docs-only PR skips the smoke job entirely. + * The diff-aware section selector for the PR e2e smoke job: a PR touching one section runs that section's scenarios + * and fuzz rather than the whole corpus, and a PR touching nothing settings-related skips the smoke steps. A + * src/sections/ path no rule recognizes throws, so a new file cannot silently skip them. * - * The rules follow the per-section layout: a src/sections//... directory - * (the key spelled verbatim) selects that key for every file under it, - * src/sections/shared/ code fans out to the sections that transitively import - * it (derived from the src/ import graph, see deriveSharedFanOut), and the - * cross-cutting files (the contract modules, the registry, the engine, the - * schema, the e2e harness) select every section. A path under src/sections/ - * that none of the rules recognize throws, so a new file cannot silently skip - * the smoke job - src/sections/ holds only the flat files named below, the - * per-section directories, contract/, and shared/, and a file anywhere else - * must gain a rule before it can land. + * src/sections//... -> , whatever the file + * src/sections/shared/.ts -> the sections that transitively import it (deriveSharedFanOut) + * contract/, registry.ts, the engine, the schema, the e2e harness -> every section * - * Usage (CI): `bun .github/scripts/changed-sections.ts [base-ref]` prints one - * of: a comma-separated section list, the literal `all`, or the literal - * `none`. The base ref defaults to `origin/main`. The smoke job runs when the - * output is not `none`. + * CLI: `bun .github/scripts/changed-sections.ts [base-ref]` (default origin/main) prints a comma-separated section + * list, `all`, or `none`; the smoke steps run unless the output is `none`. */ import { execFileSync } from "node:child_process"; @@ -32,36 +22,20 @@ export const ALL = "all"; /** The sentinel printed when nothing settings-related changed. */ export const NONE = "none"; -/** This script lives at .github/scripts/, two levels below the repository root. */ const REPO_ROOT = join(import.meta.dir, "..", ".."); -/** - * Flat src/sections/ files and what they select. registry.ts wires every - * handler together, so it selects EVERY section. A flat docs-only aggregator - * that never reaches the bundle selects NONE: docs drift is gated by - * build:check, the same reasoning that keeps lib/ out of - * ALL_SELECTING_PREFIXES. The contract's modules live under - * src/sections/contract/, an ALL_SELECTING_PREFIXES entry. - */ +/** registry.ts wires every handler, so it selects EVERY section. docs-registry.ts never reaches the bundle and its + * drift is gated by build:check, so it selects NONE (the same reasoning keeps lib/ out of ALL_SELECTING_PREFIXES). */ const ALL_SELECTING_SECTION_FILES = new Set(["registry.ts"]); const NONE_SELECTING_SECTION_FILES = new Set(["docs-registry.ts"]); -/** The section keys, as a Set of plain strings for path-segment lookups. */ const SECTION_KEY_SET: ReadonlySet = new Set(SECTION_KEYS); -/** - * Path prefixes/files that select every section: the shared engine, transport, - * action layer, discovery, reporting, the io seam, the entrypoint and schema, - * and the e2e harness itself (a harness change can change every scenario). - * `lib/` is deliberately NOT here: its only committed file is the generated - * settings.schema.json, which mirrors a `src/schema.ts` change and is gated - * by the schema-check job on its own. A unit test checks every top-level - * `src/` entry other than `sections/` is listed, so a new module cannot be - * silently skipped. - */ +/** `lib/` is deliberately NOT here: its only committed file is the generated settings.schema.json, which mirrors a + * `src/schema.ts` change and is gated by the schema-check job on its own. test/scripts/changed-sections.test.ts + * checks that every top-level `src/` entry other than `sections/` is listed, so a new module cannot be silently skipped. */ export const ALL_SELECTING_PREFIXES = [ - // The contract's layered modules: every section is written against them, - // so a change there selects everything. + // Every section is written against the contract's layered modules, so a change there selects everything. "src/sections/contract/", "src/engine/", "src/flows/", @@ -81,21 +55,18 @@ export const ALL_SELECTING_PREFIXES = [ "src/schema.ts", "src/types.ts", "test/e2e/", - // The selection machinery itself (this selector, sibling CI scripts, the smoke - // job's workflow, the local composite actions it runs): a PR touching only these - // must not select "none" and skip the very job they configure. + // The selection machinery itself: a PR touching only these must not select "none" and skip the very job they configure. ".github/scripts/", ".github/workflows/checks.yml", ".github/actions/", ]; -/** The decision for one changed-file set: every section, some, or none. */ export type Selection = | { kind: "all" } | { kind: "some"; sections: SectionKey[] } | { kind: "none" }; -/** One path from the diff. A deleted file has no code left to smoke. */ +/** A deleted file has no code left to smoke. */ export interface ChangedFile { path: string; deleted: boolean; @@ -104,7 +75,6 @@ export interface ChangedFile { /** bun's own TypeScript parser, so comments, strings, and templates never read as imports. */ const TRANSPILER = new Bun.Transpiler({ loader: "ts" }); -/** A specifier the graph can read: a string literal, or a template with nothing to substitute. */ function isLiteralSpecifier(node: Node | undefined): boolean { return ( (node?.type === "Literal" && typeof node.value === "string") || @@ -112,7 +82,6 @@ function isLiteralSpecifier(node: Node | undefined): boolean { ); } -/** Whether `node` is an `import(x)` or `require(x)` whose x is not a literal. */ function isComputedModuleLoad(node: Node): boolean { if (node.type === "ImportExpression") { return !isLiteralSpecifier(node.source); @@ -125,7 +94,6 @@ function isComputedModuleLoad(node: Node): boolean { ); } -/** Every AST node under `value`, in source order. */ function* nodesOf(value: unknown): Generator { if (Array.isArray(value)) { for (const item of value) { @@ -144,14 +112,10 @@ function* nodesOf(value: unknown): Generator { } } -/** - * Throw if `text` loads a module through a specifier the graph cannot read. - * A missing edge means a shared change could under-select - the one failure - * this selector exists to prevent - so the file must be rewritten, not skipped. - */ +/** A computed specifier is an edge the graph cannot read, and a missing edge can under-select, the one failure this + * selector exists to prevent: the file is rewritten, not skipped. */ function assertNoComputedImports(text: string, file: string): void { - // A second parser beside Bun.Transpiler: Bun's import list silently omits a - // computed import()/require(), so oxc walks the AST only to find those. + // Bun's import list silently omits a computed import()/require(), so oxc walks the AST only to find those. const { program, errors } = parseSync(file, text); const lineOf = (offset: number): number => text.slice(0, offset).split("\n").length; const [error] = errors; @@ -169,13 +133,8 @@ function assertNoComputedImports(text: string, file: string): void { } } -/** - * The relative specifiers `file` loads at runtime. Type-only imports are not - * edges (erased from the bundle) and neither are bare specifiers; a - * computed specifier throws rather than dropping an edge. Shared with - * arch-lint.ts, as is resolveImport, so their errors name the file and not - * this tool. - */ +/** Type-only imports are erased from the bundle, so they are not edges here (arch-lint.ts adds them itself). The + * errors name the file, not this tool, because arch-lint.ts shares this and resolveImport. */ export function scanImports(text: string, file: string): string[] { assertNoComputedImports(text, file); return TRANSPILER.scanImports(text) @@ -183,11 +142,8 @@ export function scanImports(text: string, file: string): string[] { .filter((specifier) => specifier.startsWith("./") || specifier.startsWith("../")); } -/** - * Resolve a relative specifier against its importer to the source on disk: `.js` maps to - * `.ts` then `/index.ts` (source spells the emitted extension); `.json` is data - * and resolves to itself. Nothing found throws: a dangling edge would silently drop an edge. - */ +/** Source spells the emitted `.js`, hence the `.ts` and `/index.ts` candidates. Nothing found throws: a dangling + * specifier would silently drop an edge. */ export function resolveImport(importer: string, specifier: string): string { const target = resolve(dirname(importer), specifier); const candidates = specifier.endsWith(".json") @@ -203,19 +159,14 @@ export function resolveImport(importer: string, specifier: string): string { ); } -/** - * Every non-test .ts file under `root`, as absolute paths. Unit tests are not - * smoke inputs (`bun test` runs them all on every PR), and the section tests - * that import the engine would otherwise pull the registry - and through it - * every section - into every shared file's fan-out. - */ +/** Unit tests are not smoke inputs, and the section tests that import the engine would pull the registry, and + * through it every section, into every shared file's fan-out. */ export function sourceFilesUnder(root: string): string[] { return readdirSync(root, { recursive: true, encoding: "utf8" }) .filter((entry) => entry.endsWith(".ts") && !entry.endsWith(".test.ts")) .map((entry) => join(root, entry)); } -/** The reverse import graph over `files`: each file -> the files that import it. */ export function reverseImportGraph(files: readonly string[]): Map> { const importedBy = new Map>(); for (const file of files) { @@ -229,7 +180,6 @@ export function reverseImportGraph(files: readonly string[]): Map>, file: string, @@ -247,16 +197,9 @@ export function transitiveDependents( return seen; } -/** - * src/sections/shared/ -> the section keys it fans out to, derived from - * the import graph over src/**: a shared file selects every section whose - * directory holds a transitive dependent. Dependents outside a section - * directory (src/schema.ts, the contract, the flat registry files) add no - * key - when they change they are in the diff as ALL_SELECTING_PREFIXES - * entries themselves. A shared file with no section dependent throws: it is - * either dead code to delete or a scan gap to fix, and neither may quietly - * select nothing. - */ +/** A dependent outside a section directory adds no key: an all-selecting path selects everything on its own, and a + * shared/ dependent has a fan-out of its own. A shared file with no section dependent is dead code or a scan gap, and + * neither may quietly select nothing. */ export function deriveSharedFanOut(repoRoot: string): Record { const sectionsDir = join(repoRoot, "src", "sections"); const sharedDir = join(sectionsDir, "shared"); @@ -281,37 +224,24 @@ export function deriveSharedFanOut(repoRoot: string): Record -> the section keys it fans out to. */ export type SharedFanOut = Record; let realFanOut: SharedFanOut | undefined; -/** The real tree's fan-out, derived once per process on the first shared/ path. */ function realSharedFanOut(): SharedFanOut { realFanOut ??= deriveSharedFanOut(REPO_ROOT); return realFanOut; } -/** - * The other file the resolver accepts for the same specifier: `foo.ts` and - * `foo/index.ts` are interchangeable to every importer of `./foo.js`. - */ +/** `foo.ts` and `foo/index.ts` are interchangeable to every importer of `./foo.js`. */ function siblingResolution(sharedPath: string): string { return sharedPath.endsWith("/index.ts") ? `${sharedPath.slice(0, -"/index.ts".length)}.ts` : `${sharedPath.slice(0, -".ts".length)}/index.ts`; } -/** - * Resolve one src/sections/ path (below the ALL_SELECTING_PREFIXES check, so - * src/sections/contract/ never reaches here) to the sections it selects: - * - src/sections//... (the key spelled verbatim) selects , whatever - * the file under it is - module, mock, schema, test, or scenario; - * - src/sections/shared/ fans out through the derived import graph; - * - the flat files select all (registry.ts) or none (docs-registry.ts). - * Anything else throws: a silently ignored section path would let a PR skip - * the very scenarios its change needs. - */ +/** Below the ALL_SELECTING_PREFIXES check, so src/sections/contract/ never reaches here. Anything unrecognized + * throws: a silently ignored section path would let a PR skip the very scenarios its change needs. */ function sectionsForSectionsPath( { path, deleted }: ChangedFile, sharedFanOut: () => SharedFanOut, @@ -338,13 +268,11 @@ function sectionsForSectionsPath( if (dir === "shared") { const sharedPath = rest.slice(slash + 1); if (sharedPath.endsWith(".docs.yml")) { - // The factories' schema prose: never in the bundle, gated by build:check - // like the docs registry, so it selects nothing on its own. + // The factories' schema prose never reaches the bundle; build:check gates it, as with the docs registry. return []; } if (deleted && sharedPath.endsWith(".ts")) { - // Nothing of its own left to smoke. Its importers either changed in the - // same diff (typecheck fails otherwise) and select their sections, or + // Its importers either changed in the same diff (typecheck fails otherwise) and select their sections, or // now resolve to the sibling spelling, whose fan-out is then theirs. return sharedFanOut()[siblingResolution(sharedPath)] ?? []; } @@ -361,25 +289,13 @@ function sectionsForSectionsPath( ); } -/** - * Map the diff's changed files (repo-relative, forward slashes) to the - * sections the smoke job must run. Any cross-cutting path forces "all", but - * every src/sections/ path is still resolved through sectionsForSectionsPath, - * which throws on an unrecognized one - a stale flat path cannot ride along - * unnoticed behind a cross-cutting change. Files that touch nothing - * settings-related (docs, config, `lib/`) are ignored, so such a PR yields - * "none". `sharedFanOut` is the derived map to consult, the real tree's - * unless a test hands in a synthetic one. - */ +/** Every src/sections/ path is resolved even when a cross-cutting path already forces "all", so a stale flat path + * cannot ride along unnoticed. */ export function sectionsForFiles( files: readonly ChangedFile[], sharedFanOut: () => SharedFanOut = realSharedFanOut, ): Selection { const selected = new Set(); - // No early return on an all-selecting path: every src/sections/ path is - // still resolved (and can throw), so an unrecognized or stale flat path - // fails loudly even when a cross-cutting file in the same diff already - // forces "all". let all = false; for (const file of files) { if (ALL_SELECTING_PREFIXES.some((prefix) => file.path.startsWith(prefix))) { @@ -387,8 +303,6 @@ export function sectionsForFiles( continue; } if (!file.path.startsWith("src/sections/")) { - // Everything else (README, COVERAGE, lib/, workflows, package.json, - // tests outside e2e) contributes no section. continue; } if (sectionsForSectionsPath(file, sharedFanOut) === "all") { @@ -412,11 +326,10 @@ export function sectionsForFiles( if (selected.size === 0) { return { kind: "none" }; } - // Emit in SECTION_KEYS order for a stable, readable list. + // SECTION_KEYS order, so the printed list is stable. return { kind: "some", sections: SECTION_KEYS.filter((key) => selected.has(key)) }; } -/** Render a Selection as the single token the CLI prints and the job branches on. */ export function renderSelection(selection: Selection): string { if (selection.kind === "all") { return ALL; @@ -427,24 +340,12 @@ export function renderSelection(selection: Selection): string { return selection.sections.join(","); } -/** - * Exactly the statuses `git diff --name-status --no-renames` can emit: - * added, deleted, modified, type-changed, unmerged, unknown, broken pairing. - * Anything else (a scored R/C, or a letter git does not use) is a shape we - * do not understand. - */ +/** Exactly the statuses `git diff --name-status --no-renames` can emit; a scored R/C or a letter git does not use is + * a shape this parser refuses. */ const GIT_STATUS = /^[ADMTUXB]$/; -/** - * Parse `git diff --name-status --no-renames -z` output: alternating - * NUL-terminated `` and `` fields. `-z` keeps paths raw (git - * would otherwise C-quote unicode, tabs, and newlines, hiding a - * src/sections/ prefix), and `--no-renames` keeps every record single-path - * (a rename is a D plus an A). Output that is not empty and not - * NUL-terminated, a field where a status should be that is not one, or a - * dangling status with no path, means a shape we do not understand and - * throws rather than being skipped. - */ +/** `-z` keeps paths raw (git would otherwise C-quote unicode, tabs, and newlines, hiding a src/sections/ prefix), and + * `--no-renames` keeps every record single-path (a rename is a D plus an A). Any other shape throws, never skipped. */ export function parseNameStatus(out: string): ChangedFile[] { if (out !== "" && !out.endsWith("\0")) { throw new Error( @@ -467,7 +368,6 @@ export function parseNameStatus(out: string): ChangedFile[] { return files; } -/** The files changed between `baseRef` and HEAD, with whether each was deleted. */ export function changedFiles(baseRef: string): ChangedFile[] { return parseNameStatus( execFileSync("git", ["diff", "--name-status", "--no-renames", "-z", `${baseRef}...HEAD`], { @@ -476,9 +376,6 @@ export function changedFiles(baseRef: string): ChangedFile[] { ); } -// CLI: print the selection token for the given base ref (default origin/main). -// Kept side-effect-free on import (the unit test imports the pure functions -// above) by gating on import.meta.main. if (import.meta.main) { const baseRef = process.argv[2] ?? "origin/main"; const selection = sectionsForFiles(changedFiles(baseRef)); diff --git a/.github/scripts/check-endpoint-coverage.ts b/.github/scripts/check-endpoint-coverage.ts index ad902f496..bc2b1404d 100644 --- a/.github/scripts/check-endpoint-coverage.ts +++ b/.github/scripts/check-endpoint-coverage.ts @@ -1,19 +1,9 @@ /** - * Endpoint-coverage tripwire for the nightly e2e run. Every "section.role" key - * in allEndpoints() AND allGraphqlOps() must be exercised at least once by the - * curated corpus; a cold route means the harness has a blind spot, an endpoint - * the action can call that no scenario reaches, so a regression there would - * ship unnoticed. + * Endpoint-coverage tripwire: every "section.role" key in allEndpoints() AND allGraphqlOps() must be reached by the + * curated corpus at least once. A cold route is a blind spot, an endpoint the action can call that no scenario + * reaches, so a regression there would ship unnoticed. * - * This runs the full corpus through runScenario and reads each report's request - * log (ScenarioReport.requests, the snapshot the runner takes before teardown), - * attributing every logged request to a registered route via matchesTemplate - * (the same matcher the mock routes with) - or, for GraphQL, via the - * operationName the mock dispatched. It then fails naming any route no - * request reached. - * - * Usage: `bun .github/scripts/check-endpoint-coverage.ts`. Exit 0 when every - * route was hit; exit 1 naming the cold routes otherwise. + * `bun .github/scripts/check-endpoint-coverage.ts` fails on any cold route, naming it. */ import { @@ -26,19 +16,10 @@ import type { LoggedRequest } from "../../test/e2e/mock/contract.js"; import { runScenario } from "../../test/e2e/runner.js"; import { loadScenarios, scenarioRoots } from "../../test/e2e/schema.js"; -/** - * One registered route, discriminated by how a hit is attributed: a REST - * route matches by method + path template, a GraphQL operation by the - * operationName the mock logged for the request. - */ type Route = | { kind: "rest"; key: string; method: string; path: string } | { kind: "graphql"; key: string; opName: string }; -/** - * The registered routes - every REST endpoint plus every GraphQL operation - - * so a cold GraphQL op fails the sweep exactly like a cold REST route. - */ export function registeredRoutes(): Route[] { return [ ...Object.entries(allEndpoints()).map( @@ -55,7 +36,6 @@ export function registeredRoutes(): Route[] { ]; } -/** Record every route each request hit into `hit` (mutated in place). */ export function recordHits( requests: readonly LoggedRequest[], routes: Route[], @@ -77,7 +57,6 @@ export function recordHits( } } -/** The registered route keys no request reached, sorted. */ export function coldRoutes(hit: ReadonlySet, routes: Route[]): string[] { return routes .filter((route) => !hit.has(route.key)) @@ -96,13 +75,9 @@ async function main(): Promise { return 1; } for (const scenario of scenarios) { - // A scenario's own pass/fail is the corpus job's concern; here we only - // aggregate which routes it reached, so a throw does not stop the sweep. A - // fault-injection scenario can drop the connection mid-run and make - // runScenario throw; runScenario snapshots its request log into the report - // and only returns on success, so a throw yields no report and its routes - // go unattributed. That could turn a real hit into a false cold route, so - // NAME the scenario loudly rather than swallow the throw. + // A scenario's own pass/fail is the corpus job's concern; here only its reached routes matter. runScenario + // returns a report (with the request log) even for a failing scenario, so a THROW is the one path that loses the + // routes it did reach and can turn a real hit into a cold route: name the scenario loudly, never swallow it. try { const report = await runScenario(scenario); recordHits(report.requests, routes, hit); diff --git a/.github/scripts/coverage-data.ts b/.github/scripts/coverage-data.ts index 4892740c3..4b4be7953 100644 --- a/.github/scripts/coverage-data.ts +++ b/.github/scripts/coverage-data.ts @@ -1,8 +1,7 @@ /** - * The authored half of COVERAGE.md (rendered by gen-docs.ts): the prose no declaration can - * derive, because it enumerates what does NOT exist - the repo-scoped gaps, the settings with no - * public API, and the account-scoped surface. The data is coverage-data.yml beside this file; - * this module is its shape and loader. The Supported table renders from each section's .docs.yml. + * The authored half of COVERAGE.md (rendered by gen-docs.ts): the prose no declaration can derive, because it + * enumerates what does NOT exist. The data is coverage-data.yml beside this file; the Supported table renders from + * each section's .docs.yml. */ import { join } from "node:path"; @@ -41,7 +40,6 @@ const CoverageGaps = z.union([ .readonly(), ]); -/** A non-empty list of authored bullets. */ const Items = z.tuple([z.string().min(1)], z.string().min(1)).readonly(); export const CoverageData = z diff --git a/.github/scripts/fetch-graphql-schema.ts b/.github/scripts/fetch-graphql-schema.ts index 25fc708f8..6612416d2 100644 --- a/.github/scripts/fetch-graphql-schema.ts +++ b/.github/scripts/fetch-graphql-schema.ts @@ -1,20 +1,12 @@ /** - * Fetch GitHub's public GraphQL schema and write it to disk, the GraphQL - * sibling of trim-openapi.ts. The query-validation unit test - * (test/sections/graphql-queries.test.ts) loads the schema from disk - never - * the network - so test runs stay hermetic and fast; this script is the ONLY - * thing that touches the network. The output is a FETCHED, gitignored - * artifact (a multi-MB generated blob kept out of history): local devs run - * this once, and CI restores it from actions/cache or re-fetches on a miss. + * Fetches GitHub's public GraphQL schema to disk, the GraphQL sibling of trim-openapi.ts. This script is the ONLY + * thing that touches the network; the output is a fetched, gitignored artifact. + * test/sections/graphql-queries.test.ts -> loads it from disk + * local dev -> runs this once + * CI -> restores it from cache, re-fetches on a miss + * UPSTREAM_REF -> PINNED to a github/docs commit, so two runs months apart fetch byte-identical text * - * Run: `bun .github/scripts/fetch-graphql-schema.ts` (writes the schema in - * place). Re-run to adopt a newer upstream ref. - * - * The upstream ref is PINNED to a github/docs commit SHA (the docs pipeline - * publishes the schema at src/graphql/data/fpt/schema.docs.graphql and - * updates it continuously on main), so two runs months apart fetch - * byte-identical schema text - the same reproducibility contract as - * trim-openapi's UPSTREAM_REF. + * Run: `bun .github/scripts/fetch-graphql-schema.ts`; re-run after bumping UPSTREAM_REF. */ import { mkdirSync, renameSync, writeFileSync } from "node:fs"; @@ -22,29 +14,19 @@ import { dirname, join } from "node:path"; import { buildSchema } from "graphql"; import { fetchTextWithRetry } from "./lib/fetch-retry.js"; -/** - * The github/docs commit the schema is fetched at. Bump this (and re-run) to - * adopt upstream schema changes; pinning to a SHA keeps the output - * reproducible. - */ const UPSTREAM_REF = "01f2174e1ab5d15d4946cfe96ef7dfb5c9a8b889"; -/** The free-tier (github.com) schema, the flavor the action targets. */ +/** fpt is the free-tier (github.com) flavor, the one the action targets. */ const SCHEMA_URL = `https://raw.githubusercontent.com/github/docs/${UPSTREAM_REF}` + "/src/graphql/data/fpt/schema.docs.graphql"; const OUT_PATH = join(import.meta.dir, "..", "..", "test", "e2e", "graphql", "schema.docs.graphql"); -/** Abandon a fetch attempt if the (large) schema has not arrived in this long. */ const FETCH_TIMEOUT_MS = 60_000; async function main(): Promise { console.log(`fetching ${SCHEMA_URL}`); - // Per-attempt timeout plus bounded retry (lib/fetch-retry.ts): a hung - // connection or a transient blip - even mid-download - fails loudly with - // advice instead of the script stalling forever, or one blip failing the - // whole CI gate. const fetched = await fetchTextWithRetry("GraphQL schema", SCHEMA_URL, FETCH_TIMEOUT_MS); if (!fetched.ok) { throw new Error( @@ -52,9 +34,7 @@ async function main(): Promise { ); } const text = fetched.text; - // Integrity at generation, the assertRefFree analog: a truncated download - // or a moved upstream file must fail HERE, not as an opaque parse error in - // the disk-only consumer. + // A truncated download or a moved upstream file must fail HERE, not as an opaque parse error in the disk-only consumer. try { buildSchema(text); } catch (error) { @@ -62,9 +42,8 @@ async function main(): Promise { `the fetched GraphQL schema from ${SCHEMA_URL} failed to parse: ${error instanceof Error ? error.message : String(error)}. The download may be truncated (re-run), or the upstream file changed shape (check UPSTREAM_REF)`, ); } - // Atomic write: serialize to a temp file, then rename over the target, so - // an aborted run leaves the previously written schema intact. The directory - // holds no tracked file, so a fresh checkout must create it first. + // Temp file then rename, so an aborted run leaves the previously written schema intact. The directory holds no + // tracked file, so a fresh checkout must create it first. mkdirSync(dirname(OUT_PATH), { recursive: true }); const tmpPath = `${OUT_PATH}.tmp`; writeFileSync(tmpPath, text); diff --git a/.github/scripts/file-fuzz-issue.ts b/.github/scripts/file-fuzz-issue.ts index 2ab44f80a..72c186104 100644 --- a/.github/scripts/file-fuzz-issue.ts +++ b/.github/scripts/file-fuzz-issue.ts @@ -1,15 +1,8 @@ /** - * File (or update) a GitHub issue when the nightly e2e run fails. The nightly - * workflow runs this only on failure, with gh authenticated through GH_TOKEN - * (issues: write). It walks every failing-scenario directory dumped under - * test/e2e/.artifacts/ and builds one issue body summarizing them: the replay - * command with each scenario's seed, its scenario.yml, the head of its report, - * and the run link. A recurring failure comments on the existing open e2e-fuzz - * issue instead of opening a duplicate. + * Files (or comments on) the GitHub issue for a failed nightly e2e run from the failing-scenario directories dumped + * under test/e2e/.artifacts/. A recurring failure comments on the open e2e-fuzz issue instead of opening a duplicate. * - * Usage: `bun .github/scripts/file-fuzz-issue.ts`. Context from the - * environment: GH_TOKEN (gh auth) and GITHUB_SERVER_URL / GITHUB_REPOSITORY / - * GITHUB_RUN_ID (the run link). + * Env: GH_TOKEN (gh auth), GITHUB_SERVER_URL / GITHUB_REPOSITORY / GITHUB_RUN_ID (the run link). */ import { appendFileSync, existsSync, readdirSync, readFileSync, statSync } from "node:fs"; @@ -19,28 +12,18 @@ const ROOT = join(import.meta.dir, "..", ".."); const ARTIFACTS = join(ROOT, "test", "e2e", ".artifacts"); const LABEL = "e2e-fuzz"; const ISSUE_TITLE = "e2e fuzz failures (nightly)"; -/** Report head shown per failing scenario; a body is a summary, not a log. */ +/** A body is a summary, not a log. */ const REPORT_LINES = 60; -/** Scenario YAML head shown per artifact; the full file is in the attachment. */ +/** The full file is in the uploaded artifacts. */ const SCENARIO_LINES = 80; -/** - * GitHub caps an issue or comment body at 65,536 characters. Stay comfortably - * under it and let the truncation notice and the uploaded artifacts carry the - * rest. - */ +/** GitHub caps an issue or comment body at 65,536 characters. */ const MAX_BODY = 60_000; -/** - * Hard per-block character cap after line truncation, so one very long single - * line (which line truncation cannot shorten) cannot dominate the body. Small - * enough that the fixed header/footer/notice plus at least one full block - * always fit inside MAX_BODY. - */ +/** Per-block cap after line truncation: one very long line cannot dominate the body, and the header, footer, notice, + * and at least one full block always fit inside MAX_BODY. */ const MAX_BLOCK_CHARS = 8_000; -/** Runs a `gh` subcommand and returns stdout; throws on a non-zero exit. */ export type GhRunner = (args: string[]) => Promise; -/** Run gh and return stdout; throws with gh's stderr on a non-zero exit. */ const gh: GhRunner = async (args) => { const proc = Bun.spawn(["gh", ...args], { stdout: "pipe", stderr: "pipe" }); const [stdout, stderr, code] = await Promise.all([ @@ -54,7 +37,6 @@ const gh: GhRunner = async (args) => { return stdout; }; -/** The failing-scenario directories dumped under .artifacts, oldest first. */ export function failureDirs(root: string): string[] { if (!existsSync(root)) { return []; @@ -70,16 +52,12 @@ export function failureDirs(root: string): string[] { .map((entry) => entry.path); } -/** - * The first `limit` lines of `text`, with a marker naming how many were cut. A - * single trailing newline is not counted as a line, so text of exactly `limit` - * lines plus a trailing newline is returned whole rather than reporting one - * phantom extra line. - */ +/** A single trailing newline is not a line, so text of exactly `limit` lines plus a trailing newline comes back whole + * rather than reporting one phantom extra line. */ export function head(text: string, limit: number): string { const lines = text.split("\n"); if (lines.length > 0 && lines[lines.length - 1] === "") { - lines.pop(); // a trailing newline splits to an empty final element; drop it + lines.pop(); } if (lines.length <= limit) { return text.trimEnd(); @@ -87,11 +65,6 @@ export function head(text: string, limit: number): string { return `${lines.slice(0, limit).join("\n")}\n... (${lines.length - limit} more lines)`; } -/** - * Truncate `text` to at most `max` characters, appending a marker when cut, so - * a single very long line (which line truncation cannot shorten) can never blow - * the body budget. The marker itself is counted, so the return is always <= max. - */ export function capChars(text: string, max: number): string { if (text.length <= max) { return text; @@ -101,29 +74,19 @@ export function capChars(text: string, max: number): string { return text.slice(0, keep) + marker; } -/** - * The fuzz seed for a failing artifact, or undefined for a corpus failure. - * Fuzz scenarios are named `fuzz-` or `fuzz-multi-` (test/e2e/ - * generators.ts), so the seed comes ONLY from the artifact name. A corpus - * scenario name has no such prefix, which tells the caller to emit a - * `run.ts --scenario` replay; report text is never consulted, so a corpus - * report that happens to contain the word "seed" cannot mislabel the replay. - */ +/** Only `fuzz-` and `fuzz-multi-` are recognized; the other fuzz families (fuzz-discovery, fuzz-merge in + * test/e2e/generators.ts, and fuzz.ts's own witness/input/chaos/fault names) fall through to a run.ts replay that + * cannot find them. The seed comes ONLY from the name, so a corpus report containing the word "seed" cannot mislabel the replay. */ export function seedFrom(name: string): string | undefined { return name.match(/^fuzz(?:-multi)?-(\d+)$/)?.[1]; } -/** - * The replay command for a failing artifact: a fuzz seed replays the exact - * iteration; a corpus scenario (no seed) replays by name through run.ts. - */ export function replayCommand(name: string, seed: string | undefined): string { return seed ? `bun test/e2e/fuzz.ts --iterations 1 --seed ${seed}` : `bun test/e2e/run.ts --scenario ${name}`; } -/** The run link built from the standard Actions environment variables. */ export function runUrl(env: NodeJS.ProcessEnv): string { const server = env.GITHUB_SERVER_URL; const repo = env.GITHUB_REPOSITORY; @@ -134,13 +97,11 @@ export function runUrl(env: NodeJS.ProcessEnv): string { return `${server}/${repo}/actions/runs/${runId}`; } -/** Read a named file from a dir, or "" when it is absent. */ function readIfPresent(dir: string, name: string): string { const path = join(dir, name); return existsSync(path) ? readFileSync(path, "utf8") : ""; } -/** Build the issue/comment body from every failing-scenario directory. */ export function buildBody(dirs: string[], env: NodeJS.ProcessEnv): string { const date = new Date().toISOString().slice(0, 10); if (dirs.length === 0) { @@ -158,23 +119,19 @@ export function buildBody(dirs: string[], env: NodeJS.ProcessEnv): string { } const url = runUrl(env); - // Count the artifact directories under .artifacts. const header = `Nightly e2e run on ${date} produced ${dirs.length} failure artifact(s).\n`; const footer = url ? `\nRun: ${url}` : ""; const artifactsNote = "\nThe full artifacts (scenario.yml, request trace, report) are attached to " + "the run as `e2e-artifacts`."; - // The omission notice is only present when some blocks are dropped, but its - // length is reserved up front so the running total stays a real character - // budget whether or not it ends up shown. Padded for the count digits. + // The notice's length is reserved up front, whether or not it ends up shown, so the running total stays a real + // character budget; sized with the full count so the digits fit. const omissionNotice = (count: number) => `\n${count} more failure artifact(s) omitted to stay under the GitHub body limit; see the attached artifacts.`; const noticeReserve = omissionNotice(dirs.length).length; - // Append per-scenario blocks while each fits the remaining budget; then stop - // and say how many were omitted. Every block (including the first) is both - // character-capped and budget-checked, so no single artifact can push the - // body past GitHub's limit and break the filing itself. + // Every block, the first included, is both character-capped and budget-checked, so no single artifact can push + // the body past GitHub's limit and break the filing itself. const budget = MAX_BODY - header.length - footer.length - artifactsNote.length - noticeReserve; const blocks: string[] = []; let used = 0; @@ -200,7 +157,7 @@ export function buildBody(dirs: string[], env: NodeJS.ProcessEnv): string { ].join("\n"), MAX_BLOCK_CHARS, ); - // +1 for the "\n" join between blocks. Stop before overflowing the budget. + // +1 for the "\n" join between blocks. if (used + block.length + 1 > budget) { break; } @@ -214,7 +171,6 @@ export function buildBody(dirs: string[], env: NodeJS.ProcessEnv): string { return `${header}\n${blocks.join("\n")}${truncation}${artifactsNote}${footer}`; } -/** The number of the open issue carrying the label, or undefined when none. */ async function openIssueNumber(run: GhRunner): Promise { const json = await run([ "issue", @@ -232,21 +188,15 @@ async function openIssueNumber(run: GhRunner): Promise { return issues[0]?.number; } -/** The trailing issue number from a `gh issue create` URL, or undefined. */ export function issueNumberFromUrl(url: string): number | undefined { const match = url.trim().match(/\/(\d+)\s*$/); return match ? Number(match[1]) : undefined; } -/** - * File the failure: comment on the open labeled issue if one exists, else create - * a new one. Assignment is left to the auto-assign workflow. Returns the issue - * number (existing or newly created) so the caller can dispatch auto-assign at - * it; undefined only if gh's create URL could not be parsed. `run` is injected - * so this is testable. - */ +/** Assignment is left to the auto-assign workflow, which the caller dispatches at the returned number. `run` is + * injected for tests. */ export async function fileIssue(run: GhRunner, body: string): Promise { - // Idempotent: --force turns an existing-label create into a no-op. + // --force updates an existing label's color and description instead of failing on it. await run([ "label", "create", diff --git a/.github/scripts/gen-action-docs.ts b/.github/scripts/gen-action-docs.ts index 92173cbcd..f178b055d 100644 --- a/.github/scripts/gen-action-docs.ts +++ b/.github/scripts/gen-action-docs.ts @@ -1,9 +1,3 @@ -/** - * Regenerate the declaration-derived regions of action.yml, the inputs, policy, - * and permissions references, and the check-mode guide between their BEGIN/END - * GENERATED markers: pure renderers plus a CLI (`bun run build:action-docs`). - */ - import { readFileSync, writeFileSync } from "node:fs"; import { dirname, join, posix } from "node:path"; import { OUTPUT_DECLS } from "../../src/action/io.js"; @@ -32,7 +26,6 @@ const ROOT = join(import.meta.dir, "..", ".."); /** Column budget for a folded description line, indent included. */ const YAML_WIDTH = 78; -/** Greedy word wrap of a single-line string to `width` columns. */ function wrap(text: string, width: number): string[] { const lines: string[] = []; let current = ""; @@ -47,11 +40,8 @@ function wrap(text: string, width: number): string[] { return current === "" ? lines : [...lines, current]; } -/** - * A `description: >-` folded block scalar at `indent` spaces. Folding turns - * each line break back into one space, so only single-spaced prose without - * edge spaces parses back to the declaration verbatim; anything else is rejected. - */ +/** Folding turns each line break back into one space, so only single-spaced prose without edge spaces parses back to + * the declaration verbatim; anything else is rejected. */ function foldedDescription(text: string, indent: number): string { if (text === "" || /^ | $|[^ \S]| {2}/.test(text)) { throw new Error(`a description must be single-spaced prose to fold losslessly: ${text}`); @@ -66,12 +56,10 @@ function foldedDescription(text: string, indent: number): string { /** Words a YAML 1.1 parser would re-type if left as a plain key. */ const YAML_WORDS = new Set(["null", "true", "false", "yes", "no", "on", "off", "y", "n"]); -/** A mapping key: plain when it round-trips as itself, double-quoted otherwise. */ function yamlKey(name: string): string { return /^[a-z][a-z0-9-]*$/.test(name) && !YAML_WORDS.has(name) ? name : JSON.stringify(name); } -/** The action.yml `inputs` entries, one per declaration, in declaration order. */ export function renderActionInputs( decls: Readonly>>, ): string { @@ -81,15 +69,13 @@ export function renderActionInputs( ` ${yamlKey(name)}:`, foldedDescription(decl.description, 4), " required: false", - // Always double-quoted: a bare default could otherwise re-type itself - // (2022-11-28 is a YAML timestamp, "" needs its quotes to exist). + // Always double-quoted: a bare default could re-type itself (2022-11-28 is a YAML timestamp, "" needs its quotes to exist). ` default: ${JSON.stringify(decl.default)}`, ].join("\n"), ) .join("\n"); } -/** The action.yml `outputs` entries, one per declaration, in declaration order. */ export function renderActionOutputs( decls: Readonly>, ): string { @@ -111,12 +97,10 @@ function cell(text: string): string { ); } -/** One markdown table row over already-rendered cell texts. */ function row(cells: readonly string[]): string { return `| ${cells.map(cell).join(" | ")} |`; } -/** The Inputs table's Default cell: the shown default, else the raw one (empty reads "(empty)"). */ function shownDefault(decl: Pick): string { if (decl.shownDefault !== undefined) { return decl.shownDefault; @@ -124,23 +108,20 @@ function shownDefault(decl: Pick): string return decl.default === "" ? "(empty)" : `\`${decl.default}\``; } -/** The Inputs table's header and rule lines, as rendered and as the region shape expects them. */ const INPUTS_TABLE_HEADER = "| Input | Default | Meaning |\n|---|---|---|"; /** A link target that is not a repository path: a URI scheme (any case), protocol-relative, or site-absolute. */ const ABSOLUTE_TARGET = /^(?:[a-z][a-z0-9+.-]*:|\/)/i; -/** - * `text` with every relative markdown link target, written root-relative in the declarations, - * rebased onto `pageDir`, so one summary reads right from every page the table renders on. - */ +/** Link targets are written root-relative in the declarations and rebased onto `pageDir`, so one summary reads right + * from every page the table renders on. */ function rebaseLinks(text: string, pageDir: string): string { return text.replace(/\]\(([^)#]+)(#[^)]*)?\)/g, (match, target: string, fragment: string = "") => ABSOLUTE_TARGET.test(target) ? match : `](${posix.relative(pageDir, target)}${fragment})`, ); } -/** The Inputs table, header included, for the page in `pageDir` (the repository root is "."). */ +/** `pageDir` is "." for the repository root. */ export function renderInputsTable( decls: Readonly>>, pageDir: string, @@ -153,7 +134,6 @@ export function renderInputsTable( ].join("\n"); } -/** "a, b, and c" (Oxford comma), "a and b", or "a". */ function proseList(items: readonly string[]): string { if (items.length <= 2) { return items.join(" and "); @@ -161,13 +141,11 @@ function proseList(items: readonly string[]): string { return `${items.slice(0, -1).join(", ")}, and ${items[items.length - 1]}`; } -/** A knobbed section as the policy page renders it. */ export interface KnobbedSection { readonly key: string; readonly undeclaredDefault: "delete" | "keep"; } -/** Delete-by-default sections first, each group in the given order. */ function deleteFirst(sections: readonly KnobbedSection[]): KnobbedSection[] { return [ ...sections.filter((section) => section.undeclaredDefault === "delete"), @@ -175,10 +153,8 @@ function deleteFirst(sections: readonly KnobbedSection[]): KnobbedSection[] { ]; } -/** The count sentence between its count word and its section list, as rendered and as the region shape expects it. */ const COUNT_SENTENCE_LEAD = " sections list the live resources sitting next to the declared ones: "; -/** The policy page's opening sentence: the count and every knobbed section. */ export function renderPolicyCountSentence(sections: readonly KnobbedSection[]): string { const word = countWord(sections.length); const keys = deleteFirst(sections).map((section) => `\`${section.key}\``); @@ -191,10 +167,8 @@ export interface PolicyRowProse { readonly override: string; } -/** The Defaults table's header and rule lines, as rendered and as the region shape expects them. */ const DEFAULTS_TABLE_HEADER = "| Section | Default | The override buys you |\n|---|---|---|"; -/** The `## Defaults per section` table, header included. */ export function renderPolicyDefaultsTable( sections: readonly KnobbedSection[], prose: Readonly>, @@ -215,10 +189,6 @@ export function renderPolicyDefaultsTable( return [DEFAULTS_TABLE_HEADER, ...rows].join("\n"); } -/** - * The per-section prose of the Defaults table. Total over the knobbed - * sections by type, so a new knob fails to compile until its row is written. - */ export const POLICY_ROW_PROSE: Record = { labels: { caveat: "Probot parity", @@ -269,24 +239,16 @@ function primaryLabel(permission: SectionPermission): string { return RESOURCE_LABEL[permission.repo[0]]; } -/** Insertion-ordered dedupe. */ function unique(items: readonly string[]): string[] { return [...new Set(items)]; } -/** The token-UI label of a permission's organization grant, if it has one. */ function orgLabel(permission: SectionPermission): string[] { return permission.org === undefined ? [] : [RESOURCE_LABEL_ORG[permission.org]]; } -/** The grant sentence's opening words, as rendered and as the region shape expects them. */ const GRANT_SENTENCE_LEAD = "To manage everything in one PAT, grant "; -/** - * The manage-everything sentence: each section's primary resource at write, - * each endpoint override at its advised level (write joins the write list), - * and every organization grant (section or override) at read. - */ export function renderGrantSentence(sections: readonly SectionMeta[]): string { const writes: string[] = []; const reads: string[] = []; @@ -321,10 +283,6 @@ export function renderGrantSentence(sections: readonly SectionMeta[]): string { return `${GRANT_SENTENCE_LEAD}${proseList(writeList)} at write${plus}.`; } -/** - * One bullet per section with a write-gated read, naming the gated reads' own - * permissions; a partly gated section names the gated routes. Empty when none. - */ export function renderGatedReads(sections: readonly SectionMeta[]): string { return sections .flatMap((section) => { @@ -334,7 +292,6 @@ export function renderGatedReads(sections: readonly SectionMeta[]): string { } const labels = unique(gated.map((read) => primaryLabel(read.permission))); if (readGating(section) === "write-gated") { - // "its" grant only for the section's own permission; an override is named. const grant = gated.every((read) => samePermission(read.permission, section.permission)) ? "its write grant" : `the ${proseList(labels)} write grant`; @@ -350,12 +307,10 @@ export function renderGatedReads(sections: readonly SectionMeta[]): string { .join("\n"); } -/** The check-mode caveat's two fixed texts, as rendered and as the region shape expects them. */ const NO_GATED_READS = "A read-only PAT covers every section in check mode."; const GATED_READS_LEAD_IN = "The read-only rule has exceptions, each a section to drop from the preview or grant at write:"; -/** The check-mode guide's read-only-PAT caveat: the gated reads under their lead-in, or the plain sentence when none is write-gated. */ export function renderCheckModeGatedReads(sections: readonly SectionMeta[]): string { const bullets = renderGatedReads(sections); if (bullets === "") { @@ -364,7 +319,6 @@ export function renderCheckModeGatedReads(sections: readonly SectionMeta[]): str return `${GATED_READS_LEAD_IN}\n\n${bullets}`; } -/** The knobbed sections with their declared defaults, in UNDECLARED_POLICY_SECTIONS order. */ function knobbedSections(): KnobbedSection[] { const byKey = new Map(SECTIONS.map((section) => [section.key, section])); return UNDECLARED_POLICY_SECTIONS.map((key) => { @@ -376,15 +330,11 @@ function knobbedSections(): KnobbedSection[] { }); } -/** - * A block body's shape: its opening newline, then `lines` (a source over newline-terminated - * lines), an empty rendering's second newline, or nothing (a freshly placed region). - */ +/** The empty alternative admits a freshly placed region and an empty rendering alike. */ function blockShape(lines: string): RegExp { return new RegExp(String.raw`^\n(?:${lines}|\n)?$`); } -/** The shape of a markdown table body under `header`: the header, then rows of `cells` (a source over the cells between the outer pipes). */ function tableShape(header: string, cells: string): RegExp { return blockShape(String.raw`${escapeRe(header)}\n(?:\| ${cells} \|\n)*`); } @@ -400,25 +350,17 @@ const QUOTED_KEY = `(?:"(?:${YAML_WORD})"|(?!"[a-z][a-z0-9-]*")${JSON_STRING})`; /** An action.yml mapping key line at two spaces, as yamlKey() renders it. */ const YAML_ENTRY_KEY = String.raw` (?:${PLAIN_KEY}|${QUOTED_KEY}):\n`; -/** A folded `description: >-` block at four spaces with its six-space lines, each starting on a word. */ const YAML_DESCRIPTION = String.raw` description: >-\n(?: \S[^\n]*\n)+`; -/** - * One gated-reads bullet in any form renderGatedReads() writes: a wholly gated section ("even - * the ... reads", its own or a named grant) or a partly gated one (routes named, "to verify what - * they return"). - */ const GATED_READ_BULLET = String.raw`- GitHub gates (?:even )?the [^\n]+ reads at write, so \x60[a-z_]+\x60 needs ` + String.raw`(?:its|the)(?: [^\n]+)? write grant in check mode (?:too|to verify what they ` + String.raw`return)\.\n`; -/** A block region's renderer: the rendered lines on lines of their own between the markers. */ function block(render: () => string): () => string { return () => `\n${render()}\n`; } -/** The Inputs table as region `name` under `heading` of the page at `path`, its links rebased onto that page. */ function inputsTableRegion(name: string, heading: string, path: string): GeneratedRegion { return { name, @@ -430,11 +372,8 @@ function inputsTableRegion(name: string, heading: string, path: string): Generat const INPUTS_PAGE_PATH = "docs/reference/inputs.md"; -/** - * Every generated region, keyed by file, with where it sits and the shape of every body this - * generator could have written for it, so a marker moved elsewhere fails instead of regenerating - * in the wrong place or erasing authored text. - */ +/** Each region's `body` matches every body this generator could have written for it, so a marker moved elsewhere + * fails instead of regenerating in the wrong place or erasing authored text. */ export const GENERATED_REGIONS: Readonly> = { "action.yml": [ { @@ -499,7 +438,6 @@ export const GENERATED_REGIONS: Readonly -->` markers (build:docs): the Sections -// table, the `result` list, the token-form link definition, COVERAGE's whole body, and the module -// map rendered from architecture.yml. Authored prose from the docs registry + coverage-data. import { readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { REPO_RESULTS, type RepoResult } from "../../src/engine/orchestrate.js"; @@ -33,14 +28,12 @@ const COVERAGE_PATH = "COVERAGE.md"; /** The repository these pages document; the token form's name and description derive from it. */ const REPO_SLUG = "Vivswan/github-settings-as-code"; -/** The Undeclared default column's display form of each undeclaredDefault. */ const UNDECLARED_DEFAULT_DISPLAY: Record = { delete: "deleted (settable)", keep: "kept (settable)", untouched: "untouched", }; -/** The section declarations the Sections table reads. */ export type SectionsTableRow = Pick< SectionMeta, "key" | "permission" | "grantCaveat" | "undeclaredDefault" @@ -53,14 +46,12 @@ const GRANT_CLAUSE = /** A grant token inside a caveat: `"Label" (read and write)` or `"Label" (read)`. */ const CAVEAT_GRANT_TOKEN = /"([^"]+)" \((read and write|read)\)/g; -/** The table's short access level: "write" for read-and-write, else "read". */ function shortLevel(level: string): "read" | "write" { return level === "read and write" ? "write" : "read"; } -// The PAT cell paraphrased from sectionGrant(): `Label: level` per clause (org clauses gain -// "(org permission)"), then a caveat only if it names extra grants. Every quoted token must be -// consumed, so a reworded grant or caveat throws instead of dropping out of the cell. +// The PAT cell paraphrases sectionGrant(). Every quoted token must be consumed, so a reworded grant or caveat throws +// instead of dropping out of the cell. export function renderPatCell(grant: string): string { const semicolon = grant.indexOf("; "); const advice = semicolon === -1 ? grant : grant.slice(0, semicolon); @@ -109,11 +100,9 @@ function cell(text: string, where: string): string { return text; } -/** The Sections table's header and rule lines, as rendered and as the region shape expects them. */ const TABLE_HEADER = "| Section | Endpoints | PAT permission | Undeclared default | Notes |\n|---|---|---|---|---|"; -/** The Sections table, one row per section in the given order; a section without docs throws. */ export function renderSectionsTable( sections: readonly SectionsTableRow[], docs: Readonly>>, @@ -160,19 +149,14 @@ function bullet(text: string, where: string): string { return `- ${text}`; } -/** The COVERAGE page's section headings, as rendered and as the region shape expects them. */ const SUPPORTED_HEADING = "## Supported"; const GAPS_HEADING = "## Repo-scoped gaps (not built yet)"; const NO_API_HEADING = "## No public API (cannot be built)"; const OUT_OF_SCOPE_HEADING = "## Out of scope (user or org account surface)"; -/** The COVERAGE tables' header and rule lines. */ const SUPPORTED_HEADER = "| Area | Section | Notes |\n|---|---|---|"; const GAPS_HEADER = "| Area | Endpoints | Why it matters |\n|---|---|---|"; -// The body of COVERAGE.md below its title: the Supported table (each section's rows, sections in -// the data's display order; the Section cell is the key plus the row's `keys` in parentheses), -// then the authored gaps table or its empty-state note, no-public-API list, and out-of-scope list. export function renderCoverage( sections: ReadonlyArray>, docs: Readonly>>, @@ -244,7 +228,6 @@ export function renderCoverage( ].join("\n"); } -/** Where each `result` value can appear, in display order; total over RepoResult. */ const RESULT_DISPLAY: Record = { applied: "any mode", partial: "any mode", @@ -254,13 +237,11 @@ const RESULT_DISPLAY: Record = { skipped: "multi-repo only", }; -/** The outputs enumeration's fixed phrases, as rendered and as the region shape expects them. */ const WORST_OF = "; worst-of across targets in multi-repo mode"; const CAN_ALSO_APPEAR = " can also appear"; /** The merge-mode result, a value outside RepoResult, closes the enumeration. */ const MERGE_ONLY = `; \`${MERGE_RESULT}\` in mode: merge`; -/** The `result` output's value enumeration: the any-mode values, the multi-repo-only ones, then the merge result. */ export function renderOutputsList(results: readonly RepoResult[]): string { const ordered = (Object.keys(RESULT_DISPLAY) as RepoResult[]).filter((value) => results.includes(value), @@ -276,14 +257,10 @@ export function renderOutputsList(results: readonly RepoResult[]): string { return `${withMulti}${MERGE_ONLY}`; } -/** A section operation tagged with its section, as patFormParameters reads it. */ export type TaggedOperation = Pick & { readonly section: string; }; -// The token form's permission parameters in `slugs` order: write if any operation naming the -// resource is write-gated, else read. A null slug is skipped only while every operation naming it -// also names a resource the form grants; otherwise this throws. export function patFormParameters( operations: readonly TaggedOperation[], slugs: Readonly>, @@ -314,7 +291,6 @@ export function patFormParameters( return parameters; } -/** The pre-filled fine-grained-token form link for the given name, description, and permission parameters. */ export function renderPatFormUrl( form: { readonly name: string; readonly description: string }, parameters: ReadonlyArray, @@ -330,7 +306,6 @@ export function renderPatFormUrl( /** The reference label a page's token-form link resolves through; the generated definition carries it. */ const PAT_FORM_LABEL = "pat-form"; -/** The pre-filled token-form link for this repository over every section operation's permission. */ function patFormUrl(): string { const operations = SECTIONS.flatMap((section) => sectionOperations(section).map((operation) => ({ ...operation, section: section.key })), @@ -342,9 +317,8 @@ function patFormUrl(): string { ); } -// The Sections table as region `name` under `heading`, and the shape of this generator's own -// output for it (or an empty body), built from the renderer constants, so a marker moved over -// authored prose or another table fails instead of erasing it. +// The body shape is this generator's own output (or an empty body), built from the renderer constants, so a marker +// moved over authored prose or another table fails instead of erasing it. function sectionsTableRegion(name: string, heading: string): GeneratedRegion { return { name, @@ -356,7 +330,6 @@ function sectionsTableRegion(name: string, heading: string): GeneratedRegion { }; } -/** The `result` value enumeration as inline region `name` under `heading`. */ function outputsListRegion(name: string, heading: string): GeneratedRegion { return { name, @@ -370,7 +343,7 @@ function outputsListRegion(name: string, heading: string): GeneratedRegion { }; } -/** The token-form link definition as tail region `name`: the page's `[...][pat-form]` reference resolves through it. */ +/** The page's `[...][pat-form]` reference resolves through this tail definition. */ function patUrlRegion(name: string): GeneratedRegion { return { name, @@ -380,8 +353,7 @@ function patUrlRegion(name: string): GeneratedRegion { }; } -// The module map: a mermaid fence rendered from architecture.yml's layers and edges (the lint -// keeps that declaration equal to the tree), as region `name` under `heading`. +// Rendered from architecture.yml, which the lint keeps equal to the tree. function architectureMapRegion(name: string, heading: string): GeneratedRegion { return { name, @@ -392,8 +364,6 @@ function architectureMapRegion(name: string, heading: string): GeneratedRegion { }; } -// Every page this generator writes besides COVERAGE.md, keyed by path: the reference tables in -// their homes, and the token-form link definition closing the README and the getting-started guide. export const PAGE_REGIONS: Readonly> = { "README.md": [patUrlRegion("readme-pat-url")], "docs/start/getting-started.md": [patUrlRegion("pat-url")], @@ -404,19 +374,17 @@ export const PAGE_REGIONS: Readonly> ], }; -// The page at `path` with its regions checked for placement, then regenerated. The result must -// reference the token-form label exactly as often as it defines it (full, collapsed, or shortcut -// form), and at most once: a renamed reference, a second one, or a stale definition ahead of the -// generated one (it wins) would leave the page wrong while regeneration stays a no-op. +// The result must reference the token-form label exactly as often as it defines it, at most once: a renamed +// reference, a second one, or a stale definition ahead of the generated one (it wins) would leave the page wrong +// while regeneration stays a no-op. export function renderPage(path: string, text: string): string { const regions = PAGE_REGIONS[path]; if (regions === undefined) { throw new Error(`gen-docs: no generated regions are registered for ${path}`); } const out = regenerateRegions(text, regions, path); - // CommonMark trims and case-folds labels and lets the first definition win, so every spelling - // counts: a mention opening a line and ending in ":" is a definition, any other bracketed - // mention is a reference. + // CommonMark trims and case-folds labels and lets the first definition win, so every spelling counts: a mention + // opening a line and ending in ":" is a definition, any other bracketed mention is a reference. const mentions = [...out.matchAll(/^ {0,3}\[([^\]]+)\]:|\[([^\]]+)\]/gm)].filter( (match) => (match[1] ?? match[2] ?? "").trim().toLowerCase() === PAT_FORM_LABEL, ); @@ -430,9 +398,7 @@ export function renderPage(path: string, text: string): string { return out; } -// The region shape's pieces, each exactly what its validator lets through: paragraph()/bullet() text -// (non-blank, one line), cell() text (also no pipe), codeSpan() text (also no backtick). nonBlank is -// unambiguous on purpose: overlapping parts would backtrack exponentially over a 38-row table. +// nonBlank is unambiguous on purpose: overlapping parts would backtrack exponentially over a 38-row table. const nonBlank = (excluded: string): string => String.raw`[ \t]*[^${excluded}\s][^${excluded}\r\n]*`; const PROSE_LINE = `${nonBlank("")}\n`; @@ -442,9 +408,8 @@ const GAP_ROWS = String.raw`(?:\| ${CELL} \| ${CELL} \| ${CELL} \|\n)+`; const GAPS_BODY = String.raw`(?:${PROSE_LINE}\n${escapeRe(GAPS_HEADER)}\n|${escapeRe(GAPS_HEADER)}\n${GAP_ROWS})`; const BULLETS = `(?:- ${PROSE_LINE})+`; -// The COVERAGE page's one region closes the file and holds everything below the title: the intro, -// then the four sections in order (or an empty body between fresh markers), so a marker moved over -// authored prose fails instead of erasing it. +// The one region closes the file and holds everything below the title (or an empty body between fresh markers), so +// a marker moved over authored prose fails instead of erasing it. const COVERAGE_REGIONS: readonly GeneratedRegion[] = [ { name: "coverage", @@ -458,11 +423,10 @@ const COVERAGE_REGIONS: readonly GeneratedRegion[] = [ }, ]; -/** The title line the COVERAGE region must directly follow. */ const COVERAGE_TITLE = "# Coverage\n\n"; -// COVERAGE.md with its region regenerated. Beyond the shared placement checks, the page is exactly -// the title, the region, and one final newline, or prose left outside could drift from the generator's. +// Beyond the shared placement checks, the page must be exactly the title, the region, and one final newline, or +// prose left outside could drift from the generator's. export function renderCoverageFile(coverage: string): string { const { begin, end } = regionBounds(coverage, "coverage", "html"); if (coverage.slice(0, begin[0]) !== COVERAGE_TITLE || coverage.slice(end[1]) !== "\n") { diff --git a/.github/scripts/gen-gaps-index.ts b/.github/scripts/gen-gaps-index.ts index 091472ec5..b07973305 100644 --- a/.github/scripts/gen-gaps-index.ts +++ b/.github/scripts/gen-gaps-index.ts @@ -1,12 +1,9 @@ /** - * Regenerate src/upstream-gaps/index.ts WHOLESALE from the directory listing. - * The index is a generated file (biome-exempt, header-marked): each gap file - * contributes one import and one GAPS array element, in sorted file order, - * and the type/value derivations split the two gap kinds by their `kind` - * field, so the generator needs nothing but the file names. The graduate - * script calls regenerateIndex() after deleting or transforming gap files; - * run `bun .github/scripts/gen-gaps-index.ts` by hand after adding one. A - * unit test pins the committed index to a fresh regeneration. + * Regenerates src/upstream-gaps/index.ts WHOLESALE from the directory listing: the derivations split the two gap + * kinds by their `kind` field, so nothing but the file names is needed. + * a gap file added by hand -> `bun .github/scripts/gen-gaps-index.ts` + * graduate-upstream-gaps.ts -> calls regenerateIndex() itself + * test/scripts/graduate-upstream-gaps.test.ts -> pins the committed index to a fresh regeneration */ import { readdirSync, writeFileSync } from "node:fs"; @@ -16,18 +13,9 @@ const ROOT = join(import.meta.dir, "..", ".."); const GAPS_DIR = "src/upstream-gaps"; const INDEX_PATH = `${GAPS_DIR}/index.ts`; -/** - * The directory's infrastructure files: the generated index and the shared - * gap machinery. - */ export const NON_GAP_FILE_NAMES: ReadonlySet = new Set(["index.ts", "gap.ts"]); -/** - * True for a bare file name the toolchain treats as a gap file: a .ts source - * directly in the directory that is neither infrastructure nor a - * declaration/test stray (a scratch notes.d.ts must not regenerate into a - * phantom import). - */ +/** A stray notes.d.ts or a test must not regenerate into a phantom import. */ export function isGapFileName(name: string): boolean { return ( name.endsWith(".ts") && @@ -42,7 +30,6 @@ export function camelCaseGapName(base: string): string { return base.replace(/-([a-z0-9])/g, (_, ch: string) => ch.toUpperCase()); } -/** The sorted gap-file base names in a directory listing. */ export function gapFileBases(listing: readonly string[]): string[] { return listing .filter(isGapFileName) @@ -50,11 +37,8 @@ export function gapFileBases(listing: readonly string[]): string[] { .sort(); } -/** - * Render the full index.ts source for the given gap-file base names. Pure - * and deterministic (the bases are re-sorted), so tests can compare the - * committed index against a fresh render without touching the disk. - */ +/** Pure and deterministic (the bases are re-sorted), so tests compare the committed index against a fresh render + * without touching the disk. */ export function generateIndex(bases: readonly string[]): string { const sorted = [...bases].sort(); const imports = [ @@ -113,7 +97,6 @@ export const UNDOCUMENTED_ROUTES: readonly (SupplementalRoute | SpecOnlyRoute)[] `; } -/** Re-list the real directory and rewrite the index in place. */ export function regenerateIndex(): number { const bases = gapFileBases(readdirSync(join(ROOT, GAPS_DIR))); writeFileSync(join(ROOT, INDEX_PATH), generateIndex(bases)); diff --git a/.github/scripts/gen-settings-schema.ts b/.github/scripts/gen-settings-schema.ts index 230fbca1d..f0853bca6 100644 --- a/.github/scripts/gen-settings-schema.ts +++ b/.github/scripts/gen-settings-schema.ts @@ -1,20 +1,12 @@ /** - * Emits lib/settings.schema.json from the zod single source in src/schema.ts - * (run by build:schema). z.toJSONSchema does the heavy lifting - .meta({id}) - * names the definitions, strict objects close with additionalProperties: - * false - the descriptions come from the docs files (each section's - * .docs.yml, the shared factories', the document root's; see - * lib/schema-descriptions.ts), and this script supplies the publication - * posture around it: - * - plain (strip) objects are OPENED by deleting the additionalProperties: - * false zod emits for them (the passthrough-first forward-compatibility - * tenet: GitHub-bound bodies must accept future fields; only strictObject - * declarations stay closed, exactly like the runtime); - * - the root layout is zod's own (zod >= 4.5 emits an id'd root as a - * top-level $ref plus its definitions.SettingsFile), passed through - * verbatim; - * - the stable $id is stamped (see SCHEMA_ID below); - * - definitions are sorted so the committed file diffs deterministically. + * Emits lib/settings.schema.json from the zod single source in src/schema.ts (build:schema). z.toJSONSchema does the + * heavy lifting and the docs files supply the descriptions (lib/schema-descriptions.ts); this script adds the + * publication posture: + * + * plain (strip) objects -> OPENED: the additionalProperties: false zod emits is deleted, since GitHub-bound bodies + * must accept future fields; only strictObject declarations stay closed, like the runtime + * root layout -> zod's own, passed through verbatim + * $id -> stamped (SCHEMA_ID); definitions sorted so the committed file diffs deterministically */ import { writeFileSync } from "node:fs"; @@ -26,11 +18,8 @@ import { attachDescriptions, type JsonSchemaNode } from "./lib/schema-descriptio const ROOT = join(import.meta.dir, "..", ".."); -/** - * The schema's identity: the raw copy at HEAD, naming no release. Editors pin - * a version through the same URL at a release ref (see the README); a versioned - * $id would need the schema regenerated on every major bump's release PR. - */ +/** The raw copy at HEAD, naming no release: editors pin a version through the same URL at a release ref (see the + * README), and a versioned $id would need the schema regenerated on every major bump's release PR. */ const SCHEMA_ID = "https://raw.githubusercontent.com/Vivswan/github-settings-as-code/HEAD/lib/settings.schema.json"; @@ -44,21 +33,17 @@ const generated = z.toJSONSchema(SettingsFile, { override(ctx) { const def = (ctx.zodSchema as unknown as { _zod: { def: ZodDefView } })._zod.def; const json = ctx.jsonSchema as Record; - // Open every plain (strip) object: zod emits additionalProperties: false - // for them, but the runtime passes unknown keys through to GitHub, and - // the published schema must not reject what the runtime accepts. Strict - // objects carry a catchall (z.never) and keep their false. + // The runtime passes unknown keys of a plain object through to GitHub, and the published schema must not reject + // what the runtime accepts. Strict objects carry a catchall (z.never) and keep their false. if (def.type === "object" && def.catchall === undefined) { delete json.additionalProperties; } - // z.record's propertyNames: {type: "string"} is a no-op in JSON (keys - // are always strings); dropped for a quieter document. + // z.record's propertyNames: {type: "string"} is a no-op in JSON (keys are always strings). if (def.type === "record" && JSON.stringify(json.propertyNames) === '{"type":"string"}') { delete json.propertyNames; } - // z.int()'s implicit safe-integer bounds are a JS implementation detail, - // not part of the documented file format; a deliberate .min()/.max() - // carries different values and stays. + // z.int()'s implicit safe-integer bounds are a JS implementation detail, not part of the documented file + // format; a deliberate .min()/.max() carries different values and stays. if (json.type === "integer") { if (json.minimum === Number.MIN_SAFE_INTEGER) { delete json.minimum; @@ -70,14 +55,10 @@ const generated = z.toJSONSchema(SettingsFile, { }, }) as Record & { definitions?: Record }; -// Every definition and property gets exactly one authored description, or the build fails -// naming the site (see lib/schema-descriptions.ts for the key spelling). attachDescriptions(generated.definitions ?? {}, SCHEMA_DESCRIPTIONS); -// The wrapper definition names carry "<" and ">"; percent-encode them inside -// $ref pointers so the refs stay valid URI references for strict consumers -// (ajv resolves both spellings; the previous generator emitted the encoded -// form). +// The wrapper definition names carry "<" and ">"; percent-encoded inside $ref pointers, the refs stay valid URI +// references for strict consumers (ajv resolves both spellings). function encodeRefs(node: unknown): void { if (Array.isArray(node)) { for (const item of node) { @@ -98,12 +79,8 @@ function encodeRefs(node: unknown): void { } encodeRefs(generated); -// Pass zod's emitted layout through verbatim (zod >= 4.5 emits an id'd root -// as a top-level $ref plus its own definitions.SettingsFile); only stamp the -// stable $id and sort the definitions so the committed file diffs -// deterministically. No layout assumption to guard: a future zod's shape -// change surfaces as schema-check drift, and a structurally broken emission -// fails the published-schema tests (ajv compile plus fixture round-trips). +// No layout assumption is guarded here: a future zod's shape change surfaces as schema-check drift, and a broken +// emission fails the published-schema tests (ajv compile plus fixture round-trips). const { definitions, ...rest } = generated; const sortedDefinitions = Object.fromEntries( Object.entries(definitions ?? {}).sort(([a], [b]) => (a < b ? -1 : 1)), diff --git a/.github/scripts/graduate-upstream-gaps.ts b/.github/scripts/graduate-upstream-gaps.ts index 4dcf432a3..926e87db1 100644 --- a/.github/scripts/graduate-upstream-gaps.ts +++ b/.github/scripts/graduate-upstream-gaps.ts @@ -1,24 +1,14 @@ /** - * Graduate upstream-gap files whose routes @octokit/types now ships. Each - * defineGap file under src/upstream-gaps/ carries a tripwire type that makes - * `bun x tsc -p . --noEmit` fail with TS2344 inside that file the moment - * upstream types one of its routes. This script turns that red build back - * green, per the file's lifecycle: a documentedInSpec: true gap is deleted - * outright, and a documentedInSpec: false gap (the pinned OpenAPI descriptor - * also lags its routes) is rewritten to the spec-only lifecycle - * (defineSpecOnlyGap: no tripwire, UNDOCUMENTED_ROUTES exemption kept). The - * gaps index is then regenerated wholesale via gen-gaps-index.ts. + * Graduates upstream-gap files whose routes @octokit/types now ships: each defineGap file's tripwire type turns + * `tsc` red with TS2344 inside that file, and this script turns the build green again. TypeScript 7 (tsgo) has no + * in-process compiler API, so the CLI plus diagnostic-line parsing IS the design, not a stopgap. * - * Run: `bun .github/scripts/graduate-upstream-gaps.ts` (from anywhere; it - * compiles the repo root). A clean compile means nothing to graduate. Any - * diagnostic that is not a TS2344 inside a gap file aborts the run untouched: - * something else is broken, and a half-fix would bury it. A re-compile after - * the deletions and rewrites must be green, or the run aborts for a human (a - * gap file whose routes shipped only partially must be split by hand; - * `git checkout -- src/upstream-gaps` restores the tree). + * documentedInSpec: true -> the file is deleted + * documentedInSpec: false -> rewritten to defineSpecOnlyGap (no tripwire; the UNDOCUMENTED_ROUTES exemption stays) + * a diagnostic that is not a TS2344 inside a gap file -> abort untouched; a half-fix would bury whatever else broke + * the re-compile still red -> abort for a human; a partially shipped file must be split by hand * - * TypeScript 7 (tsgo) has no in-process compiler API, so the CLI plus - * diagnostic-line parsing IS the design, not a stopgap. + * Run: `bun .github/scripts/graduate-upstream-gaps.ts`; `git checkout -- src/upstream-gaps` restores the tree. */ import { readFileSync, unlinkSync, writeFileSync } from "node:fs"; @@ -37,7 +27,6 @@ export interface Diagnostic { message: string; } -/** What the diagnostics demand: the gap files to graduate, and everything else. */ export interface GraduationPlan { /** Gap-file paths (repo-relative, deduplicated, sorted) with a TS2344. */ gapFiles: string[]; @@ -45,14 +34,8 @@ export interface GraduationPlan { foreign: Diagnostic[]; } -/** - * Parse `--pretty false` compiler output. Chained diagnostics continue on - * indented lines ("The types of 'a.b' are incompatible..."), which belong to - * the diagnostic above them. Non-empty lines that are neither diagnostics nor - * continuations (a crash trace, a config error without a location) come back - * in `unparsed` so the caller can refuse to act on output it does not - * understand. - */ +/** Chained diagnostics continue on indented lines, which belong to the diagnostic above them. Lines that are neither + * (a crash trace, a config error without a location) come back in `unparsed`, so the caller can refuse to act. */ export function parseDiagnostics(output: string): { diagnostics: Diagnostic[]; unparsed: string[]; @@ -87,11 +70,6 @@ export function parseDiagnostics(output: string): { return { diagnostics, unparsed }; } -/** - * True for a path this script may graduate: a gap file directly under - * src/upstream-gaps/ (per the generator's shared name predicate). A TS2344 - * anywhere else is foreign. - */ export function isGapFile(file: string): boolean { if (!file.startsWith(`${GAPS_DIR}/`)) { return false; @@ -100,7 +78,6 @@ export function isGapFile(file: string): boolean { return !rest.includes("/") && isGapFileName(rest); } -/** Split diagnostics into graduatable gap files and foreign noise. */ export function planGraduation(diagnostics: readonly Diagnostic[]): GraduationPlan { const gapFiles = new Set(); const foreign: Diagnostic[] = []; @@ -114,18 +91,13 @@ export function planGraduation(diagnostics: readonly Diagnostic[]): GraduationPl return { gapFiles: [...gapFiles].sort(), foreign }; } -/** - * True for a gap whose routes the published OpenAPI description does not - * document yet (documentedInSpec: false). Its tripwire firing means octokit - * caught up but the descriptor did not: the file is rewritten to the - * spec-only lifecycle instead of deleted, so its UNDOCUMENTED_ROUTES - * exemption survives until a bumped UPSTREAM_REF documents the paths. - */ -/** True for a spec-only gap source: octokit ships its routes already. */ +/** A spec-only gap source: octokit ships its routes already, so it carries no tripwire. */ export function isSpecOnly(gapSource: string): boolean { return gapSource.includes("defineSpecOnlyGap("); } +/** A documentedInSpec: false gap whose tripwire fired means octokit caught up but the pinned descriptor did not: it + * is rewritten rather than deleted, so its UNDOCUMENTED_ROUTES exemption survives until a bumped UPSTREAM_REF documents the paths. */ export function isSpecPinned(gapSource: string): boolean { return /documentedInSpec:\s*false/.test(gapSource); } @@ -139,13 +111,8 @@ function renderRoutes(routes: readonly string[]): string { return [" routes: [", ...routes.map((route) => ` "${route}",`), " ],"].join("\n"); } -/** - * The rewritten GAP doc: the original comment's feature clause (its text up - * to the first ";", which is where the octokit-lags prose starts), plus a - * generated clause describing the spec-only state. Preserving the whole - * original would keep a now-false "octokit does not carry these routes yet" - * sentence in a bot-committed file. - */ +/** Only the feature clause survives (the text up to the first ";", where the octokit-lags prose starts): preserving + * the whole original would keep a now-false "octokit does not carry these routes yet" sentence in a bot-committed file. */ function specOnlyDoc(doc: string): string { const text = doc .replace(/^\/\*\*/, "") @@ -158,18 +125,9 @@ function specOnlyDoc(doc: string): string { return `/** ${head}; @octokit/types ships these routes, but the pinned OpenAPI descriptor does not document them yet. */`; } -/** - * Rewrite a tripped documentedInSpec: false gap file to the spec-only - * lifecycle: keep its GAP doc's feature clause and its routes, emit the - * defineSpecOnlyGap template around them (no tripwire, no flag). - * Deterministic generation from those two extracted parts - never line - * surgery on the old source. Throws on a source that does not match the - * defineGap shape. - */ export function toSpecOnlyGapSource(source: string, gapFile: string): string { - // (?:[^*]|\*(?!\/))* spans exactly one block comment (no */ inside), so a - // module-head comment earlier in the file can never be mistaken for the - // GAP doc: only the comment directly above the export matches. + // (?:[^*]|\*(?!\/))* spans exactly one block comment (no */ inside), so a module-head comment earlier in the file + // can never be mistaken for the GAP doc: only the comment directly above the export matches. const match = /(?\/\*\*(?:[^*]|\*(?!\/))*\*\/)\s*\nexport const GAP = defineGap\(\{(?[\s\S]*?)\}\);/.exec( source, @@ -189,9 +147,8 @@ export function toSpecOnlyGapSource(source: string, gapFile: string): string { `${gapFile} declares no parsable routes; rewrite it to defineSpecOnlyGap by hand`, ); } - // Anything in the array besides plain string literals (an identifier, a - // comment, a template literal) would be dropped by the rewrite: refuse - // instead of silently losing it. + // Anything in the array besides plain string literals (an identifier, a comment, a template literal) would be + // dropped by the rewrite: refuse instead of silently losing it. const leftover = routesBody.replace(/"[^"]+"/g, "").replace(/[,\s]/g, ""); if (leftover !== "") { throw new Error( @@ -261,16 +218,14 @@ function main(): number { first.stderr, ); } - // Classify and render everything BEFORE touching the disk, so an - // unparsable spec-pinned file aborts with the tree untouched. + // Classify and render everything BEFORE touching the disk, so an unparsable spec-pinned file aborts with the tree untouched. const deletions: string[] = []; const rewrites: { gapFile: string; next: string }[] = []; for (const gapFile of plan.gapFiles) { const source = readFileSync(join(ROOT, gapFile), "utf8"); if (isSpecOnly(source)) { - // A spec-only gap carries no octokit tripwire, so a diagnostic inside - // one is not a graduation; deleting it would silently drop its - // UNDOCUMENTED_ROUTES exemption. Refuse instead of guessing. + // A spec-only gap carries no octokit tripwire, so a diagnostic inside one is not a graduation; deleting it + // would silently drop its UNDOCUMENTED_ROUTES exemption. abort( `${gapFile} is a spec-only gap but the compiler flagged it; that is not a graduation - fix the file by hand`, first.stdout, diff --git a/.github/scripts/lib/count-word.ts b/.github/scripts/lib/count-word.ts index b6d91cf0e..2238de350 100644 --- a/.github/scripts/lib/count-word.ts +++ b/.github/scripts/lib/count-word.ts @@ -28,7 +28,6 @@ const COUNT_WORDS = [ "twenty", ] as const; -/** The written-out form of `n`; throws past the listed range. */ export function countWord(n: number): string { const word = COUNT_WORDS[n]; if (word === undefined) { diff --git a/.github/scripts/lib/fetch-retry.ts b/.github/scripts/lib/fetch-retry.ts index a43615494..b8de18826 100644 --- a/.github/scripts/lib/fetch-retry.ts +++ b/.github/scripts/lib/fetch-retry.ts @@ -1,12 +1,7 @@ /** - * Bounded fetch retry with backoff for the artifact-fetching scripts - * (trim-openapi.ts, fetch-graphql-schema.ts). Both run on a cache miss - * inside the CI gate, so a single network blip must not fail all-green; - * a deterministic failure (a 4xx for a wrong ref or file name) still - * surfaces immediately for the caller to report with its own advice. - * - * Lives under lib/ so knip treats it as project code, not an entry point: - * if every caller stops importing it, knip flags it as unused. + * Bounded fetch retry with backoff for trim-openapi.ts and fetch-graphql-schema.ts, which run on a cache miss inside + * the CI gate, so a single network blip must not fail all-green; a deterministic 4xx still surfaces immediately. + * Under lib/ so knip treats it as project code: if every caller stops importing it, knip flags it as unused. */ const FETCH_ATTEMPTS = 3; @@ -15,12 +10,11 @@ const BACKOFF_BASE_MS = 2_000; /** Statuses below 500 that are still transient, not deterministic. */ const TRANSIENT_STATUSES = new Set([408, 429]); -/** The outcome of a fetch: the status line plus the fully read body. */ export interface FetchedText { ok: boolean; status: number; statusText: string; - /** The response body; empty on a non-ok response (callers report status). */ + /** Empty on a non-ok response; callers report the status. */ text: string; } @@ -32,14 +26,8 @@ export interface FetchRetryDeps { warn?: (line: string) => void; } -/** - * Fetch `url` and read its whole body, retrying transient failures (network - * errors, timeouts - including mid-body, so a dropped connection while a - * multi-MB artifact downloads retries too - 5xx, 408, 429) up to - * FETCH_ATTEMPTS times with exponential backoff. A deterministic non-ok - * response (a plain 4xx) is returned as-is with an empty body. `label` - * names the artifact in retry warnings and the exhaustion error. - */ +/** Timeouts include mid-body, so a dropped connection while a multi-MB artifact downloads retries too. A deterministic + * non-ok response (a plain 4xx) comes back as-is with an empty body rather than retrying. */ export async function fetchTextWithRetry( label: string, url: string, @@ -64,8 +52,7 @@ export async function fetchTextWithRetry( }; } if (response.ok) { - // The body read shares the attempt's AbortSignal, so a stall here - // also times out and lands in the catch below to be retried. + // The body read shares the attempt's AbortSignal, so a stall here also times out and is retried. const text = await response.text(); return { ok: true, status: response.status, statusText: response.statusText, text }; } diff --git a/.github/scripts/lib/schema-descriptions.ts b/.github/scripts/lib/schema-descriptions.ts index 0c02326ac..094d1dd03 100644 --- a/.github/scripts/lib/schema-descriptions.ts +++ b/.github/scripts/lib/schema-descriptions.ts @@ -1,15 +1,13 @@ /** - * Attaches the authored field descriptions to a generated JSON Schema. A description's key - * names its site: the definition (`LabelConfig`), then `.field` for a property, `|N` for an - * anyOf/oneOf arm, `[]` for array items, and `.*` for the additionalProperties schema - * (`LabelConfig.color`, `EnvironmentConfig.deployment_branch_policy|0.protected_branches`, - * `RepositoryConfig.*`). Two key patterns cover shapes minted for many definitions at once: - * `<*>` in the definition name matches every generic instance (`UndeclaredPolicyList<*>.entries`), - * and `{A,B}` lists alternatives (`{ActionsSecretConfig,AgentsSecretConfig}.name`). Every - * definition and property must end up with exactly one description and every key must describe - * something, so a renamed field, a forgotten sentence, and a double entry all fail the build. An - * additionalProperties schema (`.*`) may carry a description but is not required to: it stands - * for "any other key", which the parent's own description covers. + * Every definition and property must end up with exactly one description and every key must describe something, so + * a renamed field, a forgotten sentence, and a double entry all fail the build. The one exception is an + * additionalProperties schema (`.*`): it stands for "any other key", which the parent's own description covers. + * + * LabelConfig.color -> a property + * EnvironmentConfig.deployment_branch_policy|0.protected_branches -> arm 0 of an anyOf/oneOf + * RepositoryConfig.* -> the additionalProperties schema; `[]` is array items + * UndeclaredPolicyList<*>.entries -> every generic instance + * {ActionsSecretConfig,AgentsSecretConfig}.name -> each listed alternative */ export interface JsonSchemaNode { @@ -22,24 +20,18 @@ export interface JsonSchemaNode { additionalProperties?: JsonSchemaNode | boolean; } -/** One authored description and the file it came from (for the failure report). */ +/** `source` is the file it came from, for the failure report. */ export interface SchemaDescription { readonly key: string; readonly text: string; readonly source: string; } -/** A place a description attaches to; `required` is false for an additionalProperties schema. */ export interface Site { readonly node: JsonSchemaNode; readonly required: boolean; } -/** - * Every site a description may attach to, keyed as the keys above spell it: each definition, - * each property below it (through anyOf/oneOf arms, array items, and additionalProperties - * schemas), and each additionalProperties schema itself. - */ export function describableSites( definitions: Readonly>, ): Map { @@ -67,7 +59,6 @@ export function describableSites( return sites; } -/** The `{A,B}` alternatives of a key, expanded; a key without braces is itself. */ function expandBraces(key: string): string[] { const match = /^\{([^}]*)\}(.*)$/.exec(key); if (match === null) { @@ -77,7 +68,6 @@ function expandBraces(key: string): string[] { return list.split(",").map((name) => `${name.trim()}${rest}`); } -/** Whether `pattern` (after brace expansion, possibly carrying `<*>`) names `site`. */ function keyMatches(pattern: string, site: string): boolean { const generic = pattern.indexOf("<*>"); if (generic === -1) { @@ -92,10 +82,7 @@ function keyMatches(pattern: string, site: string): boolean { return /^<[^<>]+>$/.test(instance); } -/** - * Attach every description to its site, in place. Throws one error listing every problem: - * a key that names no site, a site that two descriptions claim, and a site nobody describes. - */ +/** In place. Throws one error listing every problem, so a docs sweep fixes them in one pass. */ export function attachDescriptions( definitions: Record, descriptions: readonly SchemaDescription[], @@ -135,9 +122,8 @@ export function attachDescriptions( const sources = claimed.map((d) => `${d.source} ("${d.key}")`).join(", "); problems.push(`${site} is described more than once: ${sources}`); } else { - // Description first, whatever order the generator emitted: one convention, stable diffs. - // A bare $ref takes no siblings in draft-7, so it moves under allOf, as zod itself emits - // a described reference. + // Description first, whatever order the generator emitted: one convention, stable diffs. A bare $ref takes no + // siblings in draft-7, so it moves under allOf, as zod itself emits a described reference. const rest = Object.fromEntries(Object.entries(node).filter(([k]) => k !== "description")); for (const key of Object.keys(node)) { delete node[key]; diff --git a/.github/scripts/release-pipeline.ts b/.github/scripts/release-pipeline.ts index fdbc5a3a5..958fc5e1b 100644 --- a/.github/scripts/release-pipeline.ts +++ b/.github/scripts/release-pipeline.ts @@ -1,42 +1,24 @@ /** - * The release pipeline's git topology, called step by step from the - * repo-owned workflows (update-release.yml for the release chain, checks.yml - * and update-release-pr.yml for the bookkeeping, post-green.yml for the - * build branch) and unit-tested against fixture repositories - * (test/scripts/release-pipeline.test.ts), so "the next release tags the - * right commits" is proven on every push. + * The release pipeline's git topology. main stays source-only, no version tag ever lands on it, and every ref a + * consumer names points at a packaged commit on the `build` branch (the tags cut before that branch existed point + * at main commits from when main still committed the bundle). * - * The scheme: main is source-only, and every ref a consumer can name from - * now on points at a packaged commit - a source commit's tree without its - * workflows, plus the built lib/index.js, and nothing else - on the `build` - * branch (the tags cut before that branch existed point at main commits - * that still carried the committed bundle). post-green appends a green main - * commit's packaged commit to refs/heads/build when it can publish, the - * release hook appends the release's when post-green did not (parent: the - * previous tip; the first chain commit is a ROOT, with no parent, so the - * chain never links into main's history), each naming its source in - * a Source trailer. The `latest` tag names the chain commit of the newest - * main source; the vX.Y.Z release tags and the moving major vX sit on the - * chain commit whose source is the release's merge commit. release-please cuts the DRAFT release itself (no tag: the - * config sets `draft` without `force-tag-creation`); these subcommands then - * run, one per workflow step: - * package packageRelease: tag the release's chain commit ONCE - * (appending it when post-green has not) and point latest - * at the newest main source's commit, or byte-verify an - * existing tag. No path moves a version tag. - * retag-major retagMajor: force-move the major tag, never backward. - * verify verifyPublishedRefs: origin's actual refs and tree. - * anchor anchorReleasePr: advance last-release-sha on the - * release PR branch (version tags live off main, so - * release-please's boundary is recorded config). - * boundary-check boundaryCheck: main's recorded boundary is fresh. - * anchor-check anchorCheck: the release PR carries the anchor. - * advance-build advanceBuild: append a green commit's packaged commit to - * build and point latest at the chain commit of the - * newest main source. - * Env: TAG and GITHUB_SHA (package/retag-major/anchor), GITHUB_SHA - * (advance-build), RUN_URL (optional provenance, package/advance-build). - * Node builtins only: `bun` runs it pre-install. + * packaged commit = source tree - .github/workflows + the built lib/index.js, `Source: ` trailer + * refs/heads/build root (no parent) -> chain commit -> chain commit ... only ever advances + * refs/tags/latest -> the newest source among build's tip, the publishing run's own chain commit, and where latest already points + * refs/tags/vX.Y.Z -> the chain commit whose source is the release's merge commit; never moved + * refs/tags/vX -> the same commit, force-moved on each release; retagMajor refuses a step backward (see its one blind race) + * + * release-please cuts the DRAFT release without a tag (`draft` on, `force-tag-creation` off); one subcommand runs + * per workflow step: + * + * advance-build post-green.yml GITHUB_SHA, RUN_URL (optional) + * package, retag-major update-release.yml TAG, GITHUB_SHA, RUN_URL (optional, package only) + * verify update-release.yml TAG, GITHUB_SHA + * anchor update-release-pr.yml GITHUB_SHA + * boundary-check, anchor-check checks.yml (the checkout alone) + * + * Node builtins only: bun runs this before `bun install`. Fixture-repository tests: test/scripts/release-pipeline.test.ts. */ import { execFileSync } from "node:child_process"; @@ -46,40 +28,26 @@ import { join } from "node:path"; const MANIFEST_FILE = ".release-please-manifest.json"; const CONFIG_FILE = "release-please-config.json"; const BUNDLE_FILE = "lib/index.js"; -/** Left out of every chain commit: GitHub refuses a push whose commits - * create, update, or delete a workflow file to any token without the - * workflows grant (judged on the diff each pushed commit introduces against - * its parent), and consumers run the action, not this repository's - * workflows. With none on the chain and the first chain commit a root (a - * child of its source would record the workflow files' deletion), a - * GITHUB_TOKEN with contents: write can push every chain commit, and no - * workflow can trigger on the build branch. */ +/** No chain commit carries a workflow file: GitHub judges each pushed commit's diff against its parent and refuses a + * workflow-file change to a token without the workflows grant. Consumers run the action, not the workflows. + * a chain commit against its parent -> no workflow change, so the default GITHUB_TOKEN can push it + * the first chain commit as its source's CHILD -> would record the workflows' deletion; hence a ROOT instead */ const WORKFLOWS_DIR = ".github/workflows"; const BUILD_REF = "refs/heads/build"; -/** Where readBuildTip lands origin's build: the whole chain, blobless. */ const BUILD_REMOTE = "refs/remotes/origin/build"; const LATEST_REF = "refs/tags/latest"; -/** How a tag that may name a chain commit is fetched: blobless and never - * depth-limited. A --depth fetch marks the commit it lands on shallow, which - * cuts its parent links in this clone, and the chain-membership check walks - * those links: a release behind a shallow-marked latest would read as off - * build. The one blob a check needs (the bundle, for its bytes or size) is - * fetched on demand where the server honors the filter, and came with the - * commit where it did not. */ +/** Never --depth: a depth-limited fetch marks the commit it lands on shallow, cutting its parent links in this + * clone, and assertOnChain walks those links, so a release behind a shallow-marked latest would read as off build. + * The bundle blob a check needs arrives on demand, or with the commit on a server without filter support. */ const TAG_FETCH = ["--filter=blob:none"]; -/** What a squash-merged release-please PR's subject looks like on main. - * Matched EXACTLY wherever release merges are recognized: a prefix match - * would let "chore(main): release pipeline documentation" impersonate a - * release merge and park the boundary check. */ +/** A squash-merged release-please PR's subject on main. Anchored at both ends wherever release merges are recognized: + * a prefix match would let "chore(main): release pipeline documentation" impersonate one and park the boundary check. */ const RELEASE_SUBJECT = /^chore\(main\): release (\d+\.\d+\.\d+)(?: \(#\d+\))?$/; -/** Run git in cwd, returning trimmed stdout; rethrows with the command and - * its stderr so a CI failure names the git call that produced it. */ function git(cwd: string, ...args: string[]): string { return gitWithEnv(cwd, {}, ...args); } -/** git() with extra environment (GIT_INDEX_FILE for the private index). */ function gitWithEnv(cwd: string, env: Record, ...args: string[]): string { try { return execFileSync("git", args, { @@ -98,18 +66,16 @@ function gitFailure(args: string[], error: unknown): Error { return new Error(`git ${args.join(" ")} failed${detail}`); } -/** The server's compare-and-set diagnostic: the ref is at another value than - * advertised, or was created since. */ +/** The server's compare-and-set diagnostic: the ref moved, or was created, after it was advertised. */ const LOCK_LOST = "cannot lock ref '[^']+': (?:is at [0-9a-f]+ but expected [0-9a-f]+|reference already exists)"; -/** git's words for the one push failure a retry can win: the remote ref moved - * after this run observed it. Either the client saw it (the `[rejected]` status - * line: a plain push no longer a fast-forward, a lease whose expected value went - * stale) or the server did between advertising the ref and updating it (GitHub - * puts its diagnostic in the status line, stock git on a `remote: error:` line - * above the rejection). Each form is anchored to its own line shape, so a hook - * message quoting the same words on any other line cannot match, and the - * `[remote rejected]` line a hook or ruleset produces matches none. */ +/** The one push failure a retry can win: the remote ref moved after this run observed it. Each form is anchored to + * its own line shape, so a hook message quoting the same words on another line cannot match, and a + * `[remote rejected]` line with any other reason (a hook or ruleset decline) matches none. + * + * ! [rejected] ... (fetch first|non-fast-forward|stale info) the client saw the move + * remote: error: cannot lock ref ... stock git's server-side race, on its own line + * ! [remote rejected] ... (cannot lock ref ...) GitHub's, in the status line */ const OVERTAKEN_PUSH = new RegExp( [ String.raw`^\s*!\s+\[rejected\]\s.*\((?:fetch first|non-fast-forward|stale info)\)\s*$`, @@ -119,14 +85,8 @@ const OVERTAKEN_PUSH = new RegExp( "m", ); -/** - * Push, telling the one failure worth retrying apart from every other, and - * deciding that HERE from git's stderr so no caller matches strings itself. - * Overtaken (another writer moved the ref after this run observed it) is - * returned with the stderr; a token without write access, a ruleset or - * protected-ref decline, a transport error, and anything else are permanent - * and thrown unchanged, git's stderr in the message. - */ +/** The one place git's stderr is read for "overtaken", so no caller matches strings itself; every other push + * failure (no write access, a ruleset decline, a transport error) is permanent and thrown. */ function pushUnlessOvertaken( cwd: string, ...pushArgs: string[] @@ -144,7 +104,6 @@ function pushUnlessOvertaken( } } -/** A yes/no git question: exit 0 is yes, exit 1 is no, anything else is a failure and throws. */ function gitYesNo(cwd: string, ...args: string[]): boolean { try { execFileSync("git", args, { cwd, encoding: "utf8", stdio: ["ignore", "ignore", "pipe"] }); @@ -157,12 +116,11 @@ function gitYesNo(cwd: string, ...args: string[]): boolean { } } -/** git() for byte-exact output (the bundle comparison must not decode). */ +/** No encoding: the bundle comparison must be byte-exact. */ function gitBytes(cwd: string, ...args: string[]): Buffer { return execFileSync("git", args, { cwd }); } -/** git() returning null instead of throwing, for existence probes. */ function tryGit(cwd: string, ...args: string[]): string | null { try { return execFileSync("git", args, { @@ -175,15 +133,13 @@ function tryGit(cwd: string, ...args: string[]): string | null { } } -/** The identity the pipeline's own commits (chain commits, anchor) carry. */ function configureIdentity(cwd: string): void { git(cwd, "config", "user.name", "settings-as-code-release"); git(cwd, "config", "user.email", "settings-as-code-release@users.noreply.github.com"); } -/** The one tag shape the scheme mints. Every ref derivation goes through - * this parse, so a malformed tag stops the pipeline instead of minting a - * wrong major (e.g. "v2" from "v2.1-rc.0"). */ +/** Every ref derivation passes through this parse, so a malformed tag stops the pipeline instead of minting "v2" + * from "v2.1-rc.0". */ function releaseMajor(tag: string): string { const match = tag.match(/^v(\d+)\.\d+\.\d+$/); if (!match) { @@ -194,18 +150,11 @@ function releaseMajor(tag: string): string { return `v${match[1]}`; } -/** - * The invariant every consumable ref must satisfy before consumers can be - * pointed at it: the tree carries the bundle as a non-empty REGULAR file (a - * symlink or a gitlink at that path has a size too, and no bundle). Asserted - * on every path that mints or blesses a ref - fresh package (before the - * push), rerun verification, the major move, and the build advance. - * Returns the entry so a caller can rebuild the tree it belongs in. - */ +/** A consumable ref must carry the bundle as a non-empty REGULAR file: a symlink or a gitlink at that path has a + * size too, and no bundle. Asserted on every path that mints or blesses a ref. */ function assertCarriesBundle(cwd: string, treeish: string): { mode: string; blob: string } { - // ls-tree answers a missing path with empty output and exit 0; a failing - // call (an object this checkout lacks, a transport error) must propagate, - // never read as "no bundle". + // ls-tree answers a missing path with empty output and exit 0; a failing call (a missing object, a transport + // error) must propagate, never read as "no bundle". const entry = git(cwd, "ls-tree", "-l", treeish, "--", BUNDLE_FILE); const [mode = "", , blob = "", size] = entry.split(/\s+/); const regularFile = mode === "100644" || mode === "100755"; @@ -218,12 +167,8 @@ function assertCarriesBundle(cwd: string, treeish: string): { mode: string; blob return { mode, blob }; } -/** - * sourceSha's tree minus the workflows directory plus whatever `addBundle` - * stages at BUNDLE_FILE, and nothing else: assembled in a private index read - * from sourceSha's tree, so no other path can enter it and the checkout's - * own index stays untouched. - */ +/** Assembled in a private index read from sourceSha's tree, so nothing beyond that tree but the bundle can enter and + * the checkout's own index stays untouched. */ function treePlusBundle( cwd: string, sourceSha: string, @@ -233,8 +178,7 @@ function treePlusBundle( const env = { GIT_INDEX_FILE: indexFile }; try { gitWithEnv(cwd, env, "read-tree", sourceSha); - // -f: the private index carries no stat data, so without it git would - // hold the entries against the worktree and refuse + // -f: the private index carries no stat data, so without it git would hold the entries against the worktree and refuse. gitWithEnv( cwd, env, @@ -254,22 +198,14 @@ function treePlusBundle( } } -/** Paths under the workflows directory in a tree; a packaged tree must have none. */ function workflowPaths(cwd: string, treeish: string): string[] { return git(cwd, "ls-tree", "-r", "--name-only", treeish, "--", WORKFLOWS_DIR) .split("\n") .filter((path) => path !== ""); } -/** - * The one definition of "packaged is source's package", asserted wherever a - * ref that already exists is blessed (a tagged rerun, the major move, every - * build tip): packaged's tree IS the tree rebuilt from source minus - * workflows plus packaged's own bundle entry. Tree identity, not a path - * diff: a diff lists paths, so an extra empty subtree or a rename hides - * from it, while no tree object hides from its own id. `remedy` tells the - * operator what to do. - */ +/** The one definition of "packaged is source's package", by tree identity rather than a path diff: a diff lists + * paths, so an extra empty subtree hides from it, while no tree object hides from its own id. */ function assertPackages( cwd: string, packaged: string, @@ -283,10 +219,7 @@ function assertPackages( ); const actual = git(cwd, "rev-parse", `${packaged}^{tree}`); if (actual !== expected) { - // A chain commit's diff against its source lists the workflow removals - // it is supposed to carry; what it must not carry is a workflow KEPT, - // which an identical file never shows in a diff, so those are listed - // from its tree instead. + // An unchanged workflow file never shows in the diff against the source, so every workflow path still in the tree is listed as kept. const changed = git(cwd, "diff", "--no-renames", "--name-only", source, packaged) .split("\n") .filter( @@ -303,13 +236,8 @@ function assertPackages( } } -/** - * The tree every chain commit carries: sourceSha's tree without its - * workflows plus the bundle built in the checkout. The checkout must BE - * sourceSha with a clean worktree: that is what makes the bundle a build of - * that source rather than of a by-hand edit (the bundle itself is gitignored - * and never shows as pending). - */ +/** The checkout must BE sourceSha with a clean worktree: that is what makes the bundle a build of that source rather + * than of a by-hand edit. The bundle itself is gitignored, so it never shows as pending. */ function packagedTree(cwd: string, sourceSha: string): string { const head = git(cwd, "rev-parse", "HEAD"); if (head !== sourceSha) { @@ -338,14 +266,10 @@ function packagedTree(cwd: string, sourceSha: string): string { return tree; } -/** The Source trailer as git parses it (empty when the commit has none). */ function sourceTrailer(cwd: string, sha: string): string { return git(cwd, "log", "-1", "--format=%(trailers:key=Source,valueonly)", sha); } -/** The one shape of chain commit: `tree` under the pipeline's identity, the - * source named in a Source trailer (the provenance trailer beside it), - * parented on `parent`, or a root commit when the chain starts here (null). */ function commitChain( cwd: string, tree: string, @@ -363,27 +287,17 @@ function commitChain( return git(cwd, "commit-tree", ...parents, "-m", subject, "-m", trailers.join("\n"), tree); } -/** Origin's build tip and main head, read together. */ interface BuildTip { - /** build's tip, or null while the branch does not exist. */ tip: string | null; - /** origin's main, read AFTER the tip: a commit landing between the two - * reads then postdates the tip, not the refresh. */ + /** Read AFTER the tip, so a commit landing between the two reads postdates the tip rather than the refresh. */ mainHead: string; } -/** - * Fetch the whole chain, blobless (the chain is rooted, so only chain - * commits and their trees arrive; on a server without filter support the - * fetch is plain, bounded by the chain's length), and refresh main. The - * tip's source may be a main commit newer than this checkout knows (a stale - * rerun, or a push race lost to a newer run), and the Source trailer is no - * ancestry edge git could follow, so main's head is kept: "on main" is - * granted only to a source that is reachable from it. - */ +/** The chain is rooted, so a blobless fetch of build brings only chain commits and their trees. Main's head rides + * along because the tip's source can be newer than this checkout knows (a stale rerun, a lost push race) and a + * Source trailer is no ancestry edge: "on main" is granted only to a source reachable from that head. */ function readBuildTip(cwd: string): BuildTip { - // A standalone git() propagates a failing ls-remote, so a transport - // error cannot read as "build does not exist yet". + // A standalone git() propagates a failing ls-remote, so a transport error cannot read as "build does not exist yet". let tip: string | null = null; if (git(cwd, "ls-remote", "origin", BUILD_REF) !== "") { git(cwd, "fetch", "--quiet", "--filter=blob:none", "origin", `+${BUILD_REF}:${BUILD_REMOTE}`); @@ -392,23 +306,14 @@ function readBuildTip(cwd: string): BuildTip { return { tip, mainHead: fetchMainHead(cwd) }; } -/** Origin's main head, freshly fetched. On a full clone its history lands - * with it; a shallow clone keeps its shallow boundary (a plain fetch does - * not deepen), so an ancestry verdict against it holds only on a full - * checkout - the callers that judge one run on fetch-depth 0 (advance-build - * refuses a shallow checkout outright), and the verify job's depth-1 clone - * reads main's head without judging ancestry against it. */ +/** A plain fetch does not deepen a shallow clone, so an ancestry verdict against this head holds only on a full + * checkout: the package and advance-build jobs check out with fetch-depth 0 (advanceBuild refuses a shallow one + * outright), and verifyPublishedRefs' depth-1 clone reads the head without judging ancestry against it. */ function fetchMainHead(cwd: string): string { git(cwd, "fetch", "--quiet", "origin", "refs/heads/main"); return git(cwd, "rev-parse", "FETCH_HEAD"); } -/** - * The chain commit whose Source is sourceSha, or null when none is. One - * log over the fetched chain, read from the tip down to its root, so the - * chain's length bounds it and nothing else does; a commit without a Source - * trailer (a hand push the chain has since buried) packages nothing. - */ function findPackaged(cwd: string, sourceSha: string): string | null { const log = git(cwd, "log", "--format=%H%x09%(trailers:key=Source,valueonly)", BUILD_REMOTE); for (const line of log.split("\n")) { @@ -423,12 +328,7 @@ function findPackaged(cwd: string, sourceSha: string): string | null { const BY_HAND = "refusing to build on a build branch this pipeline did not mint - inspect it by hand."; -/** - * No tip is trusted on its trailer alone: before one is built on, left, or - * pointed at by latest, it must name a source on main's history and its - * tree must be that source's package in the chain shape (minus workflows, - * plus the bundle) and nothing else. Returns the tip's source. - */ +/** A tip is never trusted on its Source trailer alone: a hand push can carry any trailer. */ function validateTip(cwd: string, tip: string, mainHead: string): string { const tipSource = sourceTrailer(cwd, tip); if (tipSource === "") { @@ -451,11 +351,6 @@ function validateTip(cwd: string, tip: string, mainHead: string): string { return tipSource; } -/** - * A chain commit that names this source must carry the very tree this - * checkout's build packages: the Source trailer cannot tell a tip that was - * not built from this source from a build that is not reproducible. - */ function assertSameBuild(cwd: string, packaged: string, sourceSha: string, tree: string): void { assertPackages( cwd, @@ -478,16 +373,8 @@ function assertSameBuild(cwd: string, packaged: string, sourceSha: string, tree: } } -/** - * Append `tree`, sourceSha's package, to build after the validated tip (or - * as the chain's root commit while build does not exist: a child of the - * source would record the workflow files' deletion, which GitHub judges as - * a workflow change the default token may not push), with a plain - * fast-forward push: git's compare-and-set is the concurrency control, so - * an overtaken push (another run appended first) is reported for the - * caller to re-read the chain and decide again, and every other failure is - * thrown as git worded it. - */ +/** A plain fast-forward push is the concurrency control: git's compare-and-set rejects an overtaken append, and the + * caller answers by re-reading the chain and deciding again. Why the first commit is a root: see WORKFLOWS_DIR. */ function appendChain( cwd: string, build: BuildTip, @@ -508,7 +395,7 @@ function appendChain( export interface PackageOptions { cwd: string; tag: string; - /** The release-please merge commit this run tested; the tag's recorded source. */ + /** The release's merge commit, resolved from the draft (not necessarily this run's own push); the tag's recorded source. */ sourceSha: string; /** Provenance trailer for a chain commit this run has to append (the workflow run URL). */ runUrl?: string; @@ -522,34 +409,20 @@ export interface PackagedRelease { } /** - * Create the release's version tag on its chain commit, exactly once. The - * worktree must be the merge commit with the bundle freshly built. The - * chain commit is normally build's tip (post-green appended it in the same - * workflow run, before this job) and is found by its Source trailer; when - * none packages this source (post-green skipped for want of its token, or - * its append never landed) it is appended here through the same path - * post-green uses, and an overtaken append re-walks: the rival's commit is - * tagged when it packages this source, built on otherwise. Once the tag is - * on, refs/tags/latest is moved the way post-green moves it (to the chain - * commit packaging the newest source, never back over a newer one), so - * @latest exists from the first release on even where post-green cannot - * push. A rerun finds the tag on origin and byte-verifies it instead - * (recorded source, whole tree, bundle bytes), so no rerun can move or - * replace a version tag - a mismatch is a loud stop - and then reconciles - * latest the same way, so a run that died between the tag push and the - * latest move is healed by its rerun. Before any of it pushes, the source - * must lie on origin's main: a draft whose target commit never reached main - * (a manifest-bearing commit on a side branch) would otherwise be appended - * and tagged, and only the latest move would notice - with the frozen tag - * and the build tip already poisoned. + * Mint the release's version tag on its chain commit exactly once; the checkout must be the merge commit with the + * bundle freshly built. A rerun finds the tag on origin and byte-verifies it instead, so no rerun can move or + * replace a version tag, then reconciles latest the same way, healing a run that died between the two pushes. + * + * source off origin's main -> stop before any push: the frozen tag and the build tip would be poisoned + * a chain commit packages it -> tagged (normally build's tip, appended by post-green earlier in this run) + * none does -> appended here through post-green's path; an overtaken append re-walks + * tag on -> latest moved as post-green moves it, so @latest exists even where post-green cannot push */ export function packageRelease(options: PackageOptions): PackagedRelease { const { cwd, tag, sourceSha, runUrl } = options; - releaseMajor(tag); // shape gate: nothing downstream may mint a non-vX.Y.Z ref + releaseMajor(tag); const tree = packagedTree(cwd, sourceSha); - // The tag must be the version THIS source released: a hand recovery with - // the wrong TAG, or a draft whose metadata points at the wrong commit, - // must stop here rather than mint an immutable tag from the wrong source. + // A hand recovery with the wrong TAG, or a draft pointing at the wrong commit, must stop before an immutable tag is minted. const manifest = JSON.parse(git(cwd, "show", `HEAD:${MANIFEST_FILE}`)) as Record; if (tag !== `v${manifest["."]}`) { throw new Error( @@ -563,8 +436,7 @@ export function packageRelease(options: PackageOptions): PackagedRelease { ); } const ref = `refs/tags/${tag}`; - // A standalone git() propagates a failing ls-remote, so a transport error - // cannot read as "the tag does not exist". + // A standalone git() propagates a failing ls-remote, so a transport error cannot read as "the tag does not exist". const existing = git(cwd, "ls-remote", "origin", ref); if (existing !== "") { git(cwd, "fetch", "--quiet", ...TAG_FETCH, "origin", `+${ref}:${ref}`); @@ -588,7 +460,6 @@ export function packageRelease(options: PackageOptions): PackagedRelease { packagedSha = appended.sha; break; } - // Another run appended in between; re-read the chain, it may now hold this source. console.error(`build push attempt ${attempt}/${attempts} overtaken: ${appended.overtaken}`); } if (packagedSha === null) { @@ -603,13 +474,8 @@ export function packageRelease(options: PackageOptions): PackagedRelease { return { created: true, packagedSha, latestSha: latest.sha }; } -/** - * Prove an existing version tag is THIS source's package: it records the - * merge commit as its source, its tree is that source's minus workflows - * plus the bundle (a planted commit that keeps the expected bundle but - * edits action.yml fails), it carries exactly the bytes the fresh build in - * the worktree produced, and it sits on build. - */ +/** An existing tag is trusted only as THIS source's package. A planted commit that keeps the expected bundle but + * edits action.yml fails the tree check; one whose bundle differs from the fresh build fails the byte check. */ function verifyPackagedTag(cwd: string, tag: string, sourceSha: string): string { const frozen = "the release-tags ruleset freezes version tags, so no rerun can replace it - inspect it by hand."; @@ -637,24 +503,15 @@ export interface RetagMajorOptions { sourceSha: string; } -/** - * Force-move the moving major tag (v2 for a v2.x.y release) to the version - * tag's packaged commit, re-verified against origin from scratch (the local - * ref is never trusted): major-pinned consumers must never receive an - * unpackaged commit, a package of the wrong source, or - on a rerun of an - * old release's job - a step backward to an older release than the line - * already shipped. The push is a compare-and-swap (--force-with-lease on - * the observed major), so a concurrent newer release cannot be clobbered - * between the newest-check and the push; an overtaken lease re-evaluates, - * any other failure is thrown as git worded it. - */ +/** The version tag is re-verified against origin from scratch, never trusted from the local ref, and newestInLine + * refuses a step backward: a rerun of an old release's job must not regress major-pinned consumers. + * the major moves after this run observed it -> the lease fails; re-read and retry + * a newer release lands between newestInLine and that read -> not caught: the observed value is the newer one and the lease passes */ export function retagMajor(options: RetagMajorOptions): { major: string; packagedSha: string } { const { cwd, tag, sourceSha } = options; const ref = `refs/tags/${tag}`; git(cwd, "fetch", "--quiet", ...TAG_FETCH, "origin", `+${ref}:${ref}`); - // The full packaged-tag verification (recorded source, whole tree, bundle - // bytes), not a weaker probe: the major must never bless a commit a fresh - // package run would refuse, however this command was reached. + // The full verification, not a weaker probe: the major must never bless a commit a fresh package run would refuse. const packagedSha = verifyPackagedTag(cwd, tag, sourceSha); const major = releaseMajor(tag); configureIdentity(cwd); @@ -684,8 +541,6 @@ export function retagMajor(options: RetagMajorOptions): { major: string; package ); } -/** The highest vX.Y.Z tag origin has in a major line, or null when the line - * is untagged (this release is its first). */ function newestInLine(cwd: string, major: string): string | null { const listed = git(cwd, "ls-remote", "origin", `refs/tags/${major}.*`); let newest: number[] | null = null; @@ -720,14 +575,7 @@ export interface VerifyOptions { sourceSha: string; } -/** - * The end-of-pipeline confirmation, against origin's ACTUAL state rather - * than anything this run holds locally: both refs a consumer resolves - the - * version tag and its major - must point at a packaged commit recording - * this release's merge commit as its source, that commit's tree must be the - * merge commit's minus workflows plus the bundle and nothing else, and it - * must sit on build. - */ +/** Reads origin's refs afresh into refs/verify/: nothing this run holds locally is trusted. */ export function verifyPublishedRefs(options: VerifyOptions): { major: string; packagedSha: string; @@ -750,8 +598,7 @@ export function verifyPublishedRefs(options: VerifyOptions): { `origin's refs/tags/${tag} points at ${packagedSha}, which records ${source === "" ? "no source" : `${source} as its source`}, not this release's merge commit ${sourceSha}.`, ); } - // The verify job's checkout is main's head at depth 1; the merge commit's - // tree comes from origin by sha so the whole-tree check can run here too. + // The verify job's checkout is main's head at depth 1; the merge commit's tree comes from origin by sha. git(cwd, "fetch", "--quiet", "--depth=1", "origin", sourceSha); const frozen = "the release-tags ruleset freezes version tags, so no rerun can replace it - inspect it by hand."; @@ -766,7 +613,7 @@ export function verifyPublishedRefs(options: VerifyOptions): { return { major, packagedSha }; } -/** The namespace release-please's PR branch lives under; the workflows' head_ref conditions spell it by hand (pinned by test). */ +/** checks.yml's head_ref conditions spell this by hand; test/docs/checks-workflow.test.ts pins them to it. */ export const RELEASE_PR_BRANCH_PREFIX = "release-please--"; const RELEASE_PR_BRANCH = `${RELEASE_PR_BRANCH_PREFIX}branches--main`; @@ -782,20 +629,13 @@ export interface AnchorResult { reason: string; } -/** - * Carry the boundary INSIDE the release PR: append a commit to the - * release-please branch setting last-release-sha to main's current head, - * the future merge commit's PARENT (known now, unlike the merge SHA). The - * squash merge lands it on main as part of the release: no push to main, - * no admin credential. The release-freshness gate keeps it correct (a PR - * must contain main's tip to merge, so the parent cannot be stale), and - * the parent is as good a boundary as the merge: the walk then includes - * the merge itself, a chore commit the changelog hides. - */ +/** The boundary rides INSIDE the release PR as last-release-sha = main's current head, the future merge commit's + * PARENT (known now, unlike the merge sha), so the squash merge lands it on main with no push to main and no admin credential. + * the parent, not the merge -> the changelog walk then also includes the merge itself, a chore commit it hides anyway + * a stale parent -> ruled out by the managed release-freshness gate: a PR must contain main's tip to merge */ export function anchorReleasePr(options: AnchorOptions): AnchorResult { const { cwd, sourceSha, attempts = 3 } = options; - // Anchor only the branch built on this head: if main moved, the newer - // push's run refreshes the branch and anchors the newer head. + // If main moved, the newer push's run refreshes the branch and anchors the newer head. const head = git(cwd, "ls-remote", "origin", "refs/heads/main").split("\t")[0]; if (head !== sourceSha) { return { changed: false, reason: `main moved to ${head ?? "?"}; the newer run anchors` }; @@ -805,16 +645,11 @@ export function anchorReleasePr(options: AnchorOptions): AnchorResult { if (git(cwd, "ls-remote", "origin", branchRef) === "") { return { changed: false, reason: "no release PR branch to anchor" }; } - // Detached on FETCH_HEAD, never a local branch: a retry after an - // overtaken push must re-fetch, and git refuses to fetch into a - // checked-out ref. + // Detached, never a local branch: a retry after an overtaken push re-fetches, and git refuses to fetch into a checked-out ref. git(cwd, "fetch", "--quiet", "--force", "origin", branchRef); git(cwd, "checkout", "--quiet", "--force", "--detach", "FETCH_HEAD"); - // Anchor only a branch release-please built on this head: this - // workflow also runs for failed CI runs (whose refresh may never have - // happened), and their anchor must not stamp this head onto a branch - // still built from an older main. A shallow history that cannot prove - // ancestry no-ops the same way - the refresh that follows anchors. + // A branch still built from an older main must not be stamped with this head. A shallow history that cannot + // prove ancestry no-ops the same way; the refresh that follows anchors. if (!isAncestor(cwd, sourceSha, "FETCH_HEAD")) { return { changed: false, @@ -839,10 +674,8 @@ export function anchorReleasePr(options: AnchorOptions): AnchorResult { "-m", "last-release-sha records the merge parent so the squash merge itself lands the next cycle's boundary on main: version tags live on packaged commits that are not on main, so release-please cannot find the boundary by tag.", ); - // Recheck main immediately before pushing: the fetch above can lose a - // race where a newer push's run already refreshed and anchored the - // branch - this run's commit would descend from that anchor and push - // cleanly, regressing the boundary to the older head. + // A newer push's run may already have refreshed and anchored the branch before the fetch above: this run's commit + // then descends from that anchor, pushes cleanly, and regresses the boundary to the older head. const headNow = git(cwd, "ls-remote", "origin", "refs/heads/main").split("\t")[0]; if (headNow !== sourceSha) { return { changed: false, reason: `main moved to ${headNow ?? "?"}; the newer run anchors` }; @@ -859,15 +692,9 @@ export function anchorReleasePr(options: AnchorOptions): AnchorResult { ); } -/** - * The tripwire behind the anchor: on the checked-out history (main), - * last-release-sha must be the newest release merge or its parent - - * anything else means an anchor was lost (or a release merge slipped past - * the pipeline) and every release PR refresh would be computed from a - * stale boundary. Loud and gating beats silently wrong changelogs. Only an - * unrecorded boundary may lack a release merge; one newer than every - * recognized merge belongs to a merge this check missed, so it is never rolled back. - */ +/** On main, last-release-sha must be the newest release merge or its parent; anything else means an anchor was lost + * or a release merge slipped past the pipeline, and every release PR refresh would compute from a stale boundary. + * A boundary NEWER than every recognized merge is a missed merge or a hand edit; either way it is never rolled back. */ export function boundaryCheck(cwd: string): { boundary: string } { if (git(cwd, "rev-parse", "--is-shallow-repository") === "true") { throw new Error( @@ -877,10 +704,8 @@ export function boundaryCheck(cwd: string): { boundary: string } { const config = JSON.parse(readFileSync(join(cwd, CONFIG_FILE), "utf8")) as { "last-release-sha"?: unknown; }; - // The grep narrows candidates cheaply (it matches any message line with - // the prefix); the strict subject regex then decides, so an ordinary - // commit like "chore(main): release pipeline documentation" can neither - // become the boundary nor hide the real newest release merge. + // The grep only narrows (it matches any message line with the prefix); RELEASE_SUBJECT decides, so + // "chore(main): release pipeline documentation" can neither become the boundary nor hide the real newest merge. const listed = git(cwd, "log", "--grep", "^chore(main): release ", "--format=%H%x09%s"); let latest = ""; for (const line of listed.split("\n")) { @@ -930,15 +755,9 @@ function isAncestor(cwd: string, ancestor: string, descendant: string): boolean ); } -/** - * The PR-side half of the boundary tripwire, run on the release PR's own - * checkout: the merge must land the anchor, and the managed - * release-freshness gate only proves the PR contains main's tip, not that - * the anchor commit survived (a release-please force-push wipes it until - * the anchor workflow re-applies it). Requiring last-release-sha to equal - * origin's CURRENT main tip makes an unanchored release PR unmergeable - * instead of parking the pipeline after its merge. - */ +/** Run on the release PR's own checkout. The managed release-freshness gate proves the PR contains main's tip, not + * that the anchor commit survived a release-please force-push, so requiring last-release-sha to equal origin's + * CURRENT main tip makes an unanchored release PR unmergeable instead of parking the pipeline after its merge. */ export function anchorCheck(cwd: string): { boundary: string } { const config = JSON.parse(readFileSync(join(cwd, CONFIG_FILE), "utf8")) as { "last-release-sha"?: unknown; @@ -968,32 +787,22 @@ export interface AdvanceBuildOptions { } export interface AdvanceBuildResult { - /** Whether this run appended to build. */ changed: boolean; /** The chain commit packaging this source, or the newer tip build was left at. */ buildSha: string; - /** Where refs/tags/latest points when this run ends: the chain commit - * packaging the newest source (normally build's tip). */ + /** Where refs/tags/latest points when this run ends: the chain commit packaging the newest source. */ latestSha: string; reason: string; } /** - * Append a green main commit's packaged commit to refs/heads/build and - * point the `latest` tag at the chain commit of the newest main source. - * Every commit on build is its source's tree minus workflows plus the bundle - * and names that source in a Source trailer; the next commit is parented on - * the current tip and pushed WITHOUT force, so build can only advance. The - * sources mostly walk forward along main, but a release-hook backfill can - * append an older source behind newer ones, which is why latest follows the - * newest source rather than the tip. A rerun on a source the chain - * already packages appends nothing (after checking that commit carries the - * very tree this checkout's build packages) and only reconciles latest; a - * rerun of an older commit's run finds the tip already past it and leaves - * it; a tip whose source is off main's history (a hand push) stops the run - * instead of being built on. - * Needs the full history: whether a recorded source lies behind sourceSha - * cannot be judged on a shallow checkout. + * build only ever advances: the next chain commit is parented on the tip and pushed without force. latest follows + * the newest source publishLatest can see rather than the tip alone, because a release-hook backfill can append an + * older source behind newer ones. + * + * the chain already packages sourceSha -> nothing appended (its tree must match this build); latest reconciled + * the tip is already past sourceSha -> left alone: a rerun of an older commit's run + * the tip's source is off main -> stop; a hand push is never built on */ export function advanceBuild(options: AdvanceBuildOptions): AdvanceBuildResult { const { cwd, sourceSha, runUrl, attempts = 3 } = options; @@ -1012,7 +821,6 @@ export function advanceBuild(options: AdvanceBuildOptions): AdvanceBuildResult { }; } -/** The build half of advanceBuild: the chain commit for this source, appended or found. */ function advanceChain( cwd: string, sourceSha: string, @@ -1054,7 +862,6 @@ function advanceChain( reason: `${BUILD_REF}: advanced to ${appended.sha}`, }; } - // Another run appended in between; re-evaluate on the new tip. console.error(`build push attempt ${attempt}/${attempts} overtaken: ${appended.overtaken}`); } throw new Error( @@ -1063,18 +870,12 @@ function advanceChain( } /** - * Point refs/tags/latest at the chain commit packaging the NEWEST source: - * build's tip, unless the tip is a release the hook backfilled behind this - * run's own commit (post-green skipped that release, newer commits landed, - * then the release job appended it), in which case this run's commit is - * newer. The push is a compare-and-swap on the tag value observed BEFORE - * the tip is read: build only ever advances and latest only ever names a - * value build has held, so a tip read after the observation is that value - * or a newer one, and a lease that goes stale means another run moved - * latest - re-read both and retry. A latest that already names a newer - * source than the target, on a chain commit that checks out, stays where it - * is (a stale rerun must not lease it back) until the next green push - * appends past. + * latest names the newest source among build's tip, this run's own chain commit, and where latest already points (a + * stale rerun must not lease it back). The tip is older than the own commit only when the hook backfilled a release + * behind commits post-green had appended. + * observe latest -> read the tip -> choose the target -> push with a lease on the observed value + * Observing BEFORE the tip read is what makes the lease sound: build only advances and latest only names values build + * has held, so the tip read is that value or newer, and a stale lease means another run moved latest. */ function publishLatest( cwd: string, @@ -1122,16 +923,8 @@ function publishLatest( ); } -/** - * The commit `observed` (what refs/tags/latest names, peeled) and its - * source, when latest must be left there: a commit ON build whose source is - * on main and strictly newer than the target's, and whose tree is that - * source's chain package. Null otherwise - the tag moved between the two - * reads, or a hand push planted something with a newer Source trailer but a - * tampered tree or off the chain - and the target takes over; the lease - * settles the race. The raw observed id stays the lease's expected value: an - * annotated tag's id is not its commit's. - */ +/** A hand push with a newer Source trailer but a tampered tree, or off the chain, yields null and the target takes + * over. The raw observed id stays the lease's expected value: an annotated tag's id is not its commit's. */ function newerChainCommit( cwd: string, observed: string, @@ -1139,9 +932,7 @@ function newerChainCommit( mainHead: string, ): { commit: string; source: string } | null { git(cwd, "fetch", "--quiet", ...TAG_FETCH, "origin", LATEST_REF); - // The tag can move between the ls-remote and this fetch: then the value - // judged here is not the value origin holds, so nothing is deferred to and - // the lease on the observed value settles it (overtaken, re-read, retry). + // The tag can move between the ls-remote and this fetch; then the lease on the observed value settles it. if (git(cwd, "rev-parse", "FETCH_HEAD") !== observed) { return null; } @@ -1161,13 +952,8 @@ function newerChainCommit( return isAncestor(cwd, commit, BUILD_REMOTE) ? { commit, source } : null; } -/** - * A packaged commit is trusted only ON build: the ruleset-protected chain - * is the trust boundary, and a detached commit with the right tree, bytes, - * and Source trailer must not be blessed as a release or become what latest - * names. Ancestry of the fetched chain, so the chain's length is the only - * bound. - */ +/** The ruleset-protected chain is the trust boundary: a detached commit with the right tree, bytes, and Source + * trailer must not be blessed as a release or become what latest names. */ function assertOnChain(cwd: string, packaged: string, ref: string, remedy: string): void { const build = readBuildTip(cwd); if (build.tip === null) { @@ -1180,7 +966,6 @@ function assertOnChain(cwd: string, packaged: string, ref: string, remedy: strin } } -/** assertPackages as a question, for a commit this pipeline may leave alone rather than stop on. */ function packages(cwd: string, packaged: string, source: string): boolean { try { assertPackages(cwd, packaged, source, packaged, ""); diff --git a/.github/scripts/trim-openapi.ts b/.github/scripts/trim-openapi.ts index 15030bd6c..89542ac14 100644 --- a/.github/scripts/trim-openapi.ts +++ b/.github/scripts/trim-openapi.ts @@ -1,18 +1,12 @@ /** - * Trim the published GitHub OpenAPI description down to exactly the paths the - * action can reach, and write it to disk. The e2e validator (test/e2e/openapi/ - * validate.ts) loads the trimmed spec from disk - never the network - so runs - * stay hermetic and fast; this script is the ONLY thing that touches the - * network. The output is a FETCHED, gitignored artifact (a ~2MB generated blob - * kept out of history): local devs run this once, and CI restores it from - * actions/cache or re-fetches on a miss. + * Trims the published GitHub OpenAPI description to exactly the paths the action can reach and writes it to disk. + * This script is the ONLY thing that touches the network; the output is a fetched, gitignored artifact (~2MB). + * test/e2e/openapi/validate.ts -> loads it from disk + * local dev -> runs this once + * CI -> restores it from cache, re-fetches on a miss + * UPSTREAM_REF -> PINNED to a commit SHA, so two runs months apart produce byte-identical output from the same USED_PATHS * - * Run: `bun .github/scripts/trim-openapi.ts` (writes the trimmed JSON in place). - * Re-run whenever USED_PATHS changes (a new section endpoint) or to adopt a - * newer upstream ref. - * - * The upstream ref is PINNED to a commit SHA, not a moving branch, so two runs - * months apart produce byte-identical output from the same USED_PATHS. + * Run: `bun .github/scripts/trim-openapi.ts`; re-run when USED_PATHS changes or after bumping UPSTREAM_REF. */ import { renameSync, writeFileSync } from "node:fs"; @@ -21,19 +15,10 @@ import { DEFAULT_API_VERSION } from "../../src/github/api.js"; import { UNDOCUMENTED_PATHS, USED_PATHS } from "../../test/e2e/openapi/paths.js"; import { fetchTextWithRetry } from "./lib/fetch-retry.js"; -/** - * The github/rest-api-description commit the trimmed spec is cut from. Bump - * this (and re-run) to adopt upstream changes; pinning to a SHA keeps the - * output reproducible. - */ const UPSTREAM_REF = "16bc535ad66fac59d585b1516d1d52f58f787962"; -/** - * The ref actually fetched: the TRIM_UPSTREAM_REF environment variable - * overrides the pin for one run (the nightly upstream probe points it at the - * latest descriptor); unset or blank means the pinned SHA, so default runs - * stay byte-identical. - */ +/** TRIM_UPSTREAM_REF overrides the pin for one run (the nightly upstream probe points it at the latest descriptor); + * unset or blank means the pinned SHA, so default runs stay byte-identical. */ const REF = resolveRef(process.env.TRIM_UPSTREAM_REF); function resolveRef(override: string | undefined): string { @@ -41,8 +26,7 @@ function resolveRef(override: string | undefined): string { if (!ref) { return UPSTREAM_REF; } - // The ref lands in a URL path: refuse anything that could reshape the URL - // (query, fragment, traversal) instead of fetching something surprising. + // The ref lands in a URL path: refuse anything that could reshape the URL (query, fragment, traversal). if (!/^[A-Za-z0-9._/-]+$/.test(ref) || ref.includes("..")) { throw new Error( `TRIM_UPSTREAM_REF "${ref}" is not a plain git ref (letters, digits, ".", "_", "/", "-"; no "..")`, @@ -51,11 +35,8 @@ function resolveRef(override: string | undefined): string { return ref; } -/** - * The dereferenced (no $ref) descriptor for our pinned API version. Dereferenced - * so the trimmed slice is self-contained: keeping a path drags its inlined - * schemas along, with no components/schemas graph to also carry. - */ +/** The dereferenced (no $ref) descriptor, so the trimmed slice is self-contained: keeping a path drags its inlined + * schemas along, with no components/schemas graph to also carry. */ const SPEC_URL = `https://raw.githubusercontent.com/github/rest-api-description/${REF}` + `/descriptions/api.github.com/dereferenced/api.github.com.${DEFAULT_API_VERSION}.deref.json`; @@ -70,7 +51,6 @@ const OUT_PATH = join( "github-openapi.trimmed.json", ); -/** The minimal OpenAPI shape we read: a paths map plus top-level metadata. */ interface OpenApiDoc { openapi: string; info: unknown; @@ -78,14 +58,9 @@ interface OpenApiDoc { [key: string]: unknown; } -/** Abandon a fetch attempt if the (large) descriptor has not arrived in this long. */ const FETCH_TIMEOUT_MS = 60_000; async function fetchSpec(url: string): Promise { - // Per-attempt timeout plus bounded retry (lib/fetch-retry.ts): a hung - // connection or a transient blip - even mid-download - fails loudly with - // advice instead of the script stalling forever, or one blip failing the - // whole CI gate. const fetched = await fetchTextWithRetry("OpenAPI descriptor", url, FETCH_TIMEOUT_MS); if (!fetched.ok) { throw new Error( @@ -108,17 +83,11 @@ async function fetchSpec(url: string): Promise { return doc; } -/** - * Assert the trimmed slice carries no $ref. The dereferenced descriptor should - * be fully inlined, but a partial deref upstream (or a wrong file name) could - * leave dangling $refs the disk-only validator cannot resolve - which would - * make ajv either throw at compile time or silently skip a subschema. Catching - * it here, at generation, keeps the trimmed spec self-contained by contract. - */ +/** A partial deref upstream or a wrong file name would leave dangling $refs the disk-only validator cannot resolve: + * ajv would throw at compile time or silently skip a subschema. Caught here, at generation. */ function assertRefFree(trimmed: OpenApiDoc): void { const serialized = JSON.stringify(trimmed); if (serialized.includes('"$ref"')) { - // Surface a couple of offending paths to make the upstream problem concrete. const matches = [...serialized.matchAll(/"\$ref":\s*"([^"]+)"/g)].slice(0, 5); const sample = matches.map((m) => m[1]).join(", "); throw new Error( @@ -127,13 +96,9 @@ function assertRefFree(trimmed: OpenApiDoc): void { } } -/** - * Keep only the USED_PATHS entries. USED_PATHS spells templates the same way - * OpenAPI keys them ("/repos/{owner}/{repo}/labels"), so the match is exact - * string equality - no normalization guessing. A USED_PATHS entry absent from - * the upstream spec is a hard error: it means the action calls a path GitHub - * does not document at this version, which the validator could never check. - */ +/** USED_PATHS spells templates as OpenAPI keys them ("/repos/{owner}/{repo}/labels"), so the match is exact string + * equality. An entry absent upstream is a hard error: the action calls a path GitHub does not document at this + * version, which the validator could never check. */ function trimPaths(doc: OpenApiDoc): { trimmed: OpenApiDoc; kept: string[]; missing: string[] } { const kept: string[] = []; const missing: string[] = []; @@ -159,14 +124,12 @@ function trimPaths(doc: OpenApiDoc): { trimmed: OpenApiDoc; kept: string[]; miss async function main(): Promise { console.log(`fetching ${SPEC_URL}`); const doc = await fetchSpec(SPEC_URL); - // The mirror of the missing-path check below: an UNDOCUMENTED_PATHS entry - // exists precisely BECAUSE the descriptor lacks it, so the moment upstream - // documents one, the carve-out must go (and validation switch on). + // An UNDOCUMENTED_PATHS entry exists precisely BECAUSE the descriptor lacks it, so the moment upstream documents + // one, the carve-out must go (and validation switch on). const nowDocumented = UNDOCUMENTED_PATHS.filter((path) => doc.paths[path] !== undefined); if (nowDocumented.length > 0) { - // On a probe run (overridden ref) the pinned descriptor may still lack - // the paths, so retiring the gap right away would break pinned runs: - // the pin must move first. + // On a probe run (overridden ref) the pinned descriptor may still lack the paths, so retiring the gap right away + // would break pinned runs: the pin must move first. const remedy = REF === UPSTREAM_REF ? "Retire the owning gap in src/upstream-gaps/ (delete the spec-only file, " + @@ -185,15 +148,11 @@ async function main(): Promise { `these USED_PATHS are not in the upstream descriptor at ${REF} for ${DEFAULT_API_VERSION}:\n ${missing.join("\n ")}\nEither the path is wrong in test/e2e/openapi/paths.ts, or UPSTREAM_REF/api-version needs updating`, ); } - // The dereferenced slice must be fully inlined; a stray $ref means the - // trimmed slice would not be self-contained for the disk-only validator. assertRefFree(trimmed); - // Stable key order and a trailing newline so re-runs are byte-identical and - // the file plays nicely with the repo's formatting. + // Stable key order and a trailing newline, so re-runs are byte-identical. const json = `${JSON.stringify(trimmed, null, 2)}\n`; - // Atomic write: serialize to a temp file, then rename over the target. A - // crash or an aborted run then leaves the previously written spec intact - // rather than a half-written file the validator would fail to parse. + // Temp file then rename, so an aborted run leaves the previously written spec intact rather than a half-written + // file the validator would fail to parse. const tmpPath = `${OUT_PATH}.tmp`; writeFileSync(tmpPath, json); renameSync(tmpPath, OUT_PATH); diff --git a/.github/workflows/auto-fix.yml b/.github/workflows/auto-fix.yml index 03eea4906..0760e4ffb 100644 --- a/.github/workflows/auto-fix.yml +++ b/.github/workflows/auto-fix.yml @@ -1,39 +1,21 @@ -# Repo-owned auto-fix: the commit-back fixes a PR can need. Main carries no -# bundle (lib/index.js is built where it runs), so no bundle rebuilds happen -# here; three fixes remain. lib/settings.schema.json is a committed build -# artifact, and CI fails when it drifts from src/ or the generator toolchain -# (a Dependabot generator bump changes the schema bytes on a branch nobody -# builds). README.md, COVERAGE.md, action.yml, and the docs pages under docs/ -# carry generated regions that build:docs and build:action-docs render from the -# declarations and coverage-data.ts, and CI fails when they drift. src/upstream-gaps/ holds tripwire files that turn the typecheck -# red with TS2344 the moment @octokit/types ships a route a gap file declares -# as missing; graduate-upstream-gaps.ts retires the shipped files (deleting -# spec-documented ones, rewriting spec-lagging ones to spec-only gaps) and -# regenerates the gaps index. This workflow runs the graduation and the schema -# and docs regeneration on every same-repo PR that touches a fix input and pushes the -# result back to the PR branch; an already-clean tree gets no commit. A PR -# with unrelated type errors fails the build job here by design (the -# graduation script refuses to half-fix a red build), just as it fails CI's -# typecheck. +# The commit-back fixes a same-repo PR can need, pushed to its branch; an already-clean tree gets no commit. A PR +# with unrelated type errors fails the build job by design: the graduation script refuses to half-fix a red build. +# lib/settings.schema.json -> build:schema (a Dependabot generator bump changes its bytes on a branch nobody builds) +# README, COVERAGE, action.yml, docs/ regions -> build:docs, build:action-docs +# src/upstream-gaps/ -> graduate-upstream-gaps.ts retires the gap files @octokit/types caught up with +# lib/index.js -> never: main carries no bundle # -# Unlike dependabot-bun-lockfile.yml, regenerating the schema necessarily -# executes PR-controlled code, so the fix is computed in an unprivileged job -# and handed to the push job as a git patch staged from -# lib/settings.schema.json, the generated docs, and src/upstream-gaps/ only: no code from the PR -# runs on the runner that holds the write token, applying a patch executes -# nothing, and the push job re-checks the applied paths before committing. -# --ignore-scripts keeps lefthook (the prepare script) and dependency lifecycle -# scripts out of the build job, and the push job never installs anything, so -# no git hooks exist on either runner. +# Regenerating the schema executes PR-controlled code, so the fix is computed in an unprivileged job and crosses to +# the push job as a git patch over the fix-owned paths only; applying a patch executes nothing, and the push job +# re-checks the applied paths before committing. +# --ignore-scripts on the build job's install -> lefthook and dependency lifecycle scripts never run +# no install at all on the push job -> no git hooks exist where the write token is # -# Dependabot-triggered runs read the Dependabot secrets store, not the -# Actions one. Register REPO_PLATFORM_TOKEN in both stores: a push made -# with github.token starts no workflows, so without the token the fix -# commit does not re-run CI (the job then posts a PR comment and a -# warning annotation; close/reopen the PR to re-run), and on Dependabot -# bun PRs the token is a hard prerequisite - the stale lockfile fails -# the frozen install here until dependabot-bun-lockfile.yml's dedupe -# push retriggers this workflow, which only a real token does. +# Dependabot-triggered runs read the Dependabot secrets store, not the Actions one, so REPO_PLATFORM_TOKEN goes in +# both. A push made with github.token starts no workflows. +# no token, install succeeds -> the fix lands but CI does not re-run; the job posts a PR comment and a warning (close/reopen the PR) +# no token, Dependabot bun PR -> the stale lockfile fails the frozen install here until dependabot-bun-lockfile.yml's +# dedupe push retriggers this workflow, which only a real token does name: Auto Fix @@ -66,16 +48,15 @@ on: permissions: contents: read -# Only the newest run per PR proceeds; a superseded run's push would race -# the new head (its push job skips on the head-moved guard anyway). +# Only the newest run per PR proceeds; a superseded run's push would race the new head (its push job skips on the +# head-moved guard anyway). concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number }} cancel-in-progress: true jobs: build: - # Same-repo PRs only: the push job's token cannot push to a fork's - # branch. + # The push job's token cannot push to a fork's branch. if: github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest timeout-minutes: 15 @@ -90,16 +71,11 @@ jobs: - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 with: bun-version-file: .bun-version - # On a Dependabot PR whose lockfile still has the stale nested - # entries dependabot-bun-lockfile.yml exists to fix, the frozen - # install fails here; that workflow's dedupe push retriggers this - # one on the fixed head. - name: Install dependencies run: bun install --frozen-lockfile --ignore-scripts - name: Graduate upstream gaps octokit now ships shell: bash - # Old branches cut before the graduate script landed lack it; the - # schema regeneration below still runs for them. + # Old branches cut before the graduate script landed lack it; the schema regeneration below still runs for them. run: | if [ -f .github/scripts/graduate-upstream-gaps.ts ]; then bun .github/scripts/graduate-upstream-gaps.ts @@ -111,23 +87,20 @@ jobs: shell: bash run: | bun run build:schema - # Old branches cut before the docs generator landed lack the - # script; the schema regeneration above still runs for them. + # Old branches cut before the docs generators landed lack them. if [ -f .github/scripts/gen-docs.ts ]; then bun run build:docs fi if [ -f .github/scripts/gen-action-docs.ts ]; then bun run build:action-docs fi - # Anything the earlier steps left staged is not this workflow's - # fix: start from an empty index so the patch holds exactly the - # allowed paths. + # Anything the earlier steps left staged is not this workflow's fix: start from an empty index so the + # patch holds exactly the allowed paths. git reset -q git add -A -- lib/settings.schema.json README.md COVERAGE.md action.yml \ docs/reference/undeclared-policy.md docs/reference/permissions.md docs/operate/check-mode.md \ docs/reference/sections.md docs/reference/inputs.md docs/start/getting-started.md - # Old branches cut before the gaps directory landed lack it, and - # an unmatched pathspec would fail the add. + # Old branches lack the gaps directory, and an unmatched pathspec would fail the add. if [ -d src/upstream-gaps ]; then git add -A -- src/upstream-gaps/ fi @@ -139,10 +112,8 @@ jobs: else git diff --cached --stat echo "changed=true" >> "$GITHUB_OUTPUT" - # A patch touching src/ means gap files graduated (a schema-only - # patch is plain generator drift); the push job picks the commit - # subject from this. --quiet exits 1 on a staged src/ change, - # and unlike piping --name-only into grep -q it cannot trip + # A patch touching src/ means gap files graduated; the push job picks the commit subject from this. + # --quiet exits 1 on a staged src/ change and, unlike piping --name-only into grep -q, cannot trip # pipefail on an early-exiting reader. if git diff --cached --quiet -- src/; then echo "pruned=false" >> "$GITHUB_OUTPUT" @@ -170,8 +141,7 @@ jobs: with: ref: ${{ github.event.pull_request.head.ref }} persist-credentials: false - # The patch lands OUTSIDE the checkout so nothing in the work tree - # can shadow or stage it. + # The patch lands OUTSIDE the checkout so nothing in the work tree can shadow or stage it. - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: name: autofix-patch @@ -186,11 +156,9 @@ jobs: PRUNED: ${{ needs.build.outputs.pruned }} shell: bash run: | - # The branch can move between the build and this push (a - # Dependabot rebase, or dependabot-bun-lockfile.yml's dedupe - # commit, which also invalidates the fix we built): the patch - # belongs to HEAD_SHA only, and the newer head's own run owns - # the rebuild. + # The branch can move between the build and this push (a Dependabot rebase, or dependabot-bun-lockfile.yml's + # dedupe commit, which also invalidates the fix): the patch belongs to HEAD_SHA only, and the newer head's + # own run owns the rebuild. superseded() { git fetch --quiet origin "$HEAD_REF" [ "$(git rev-parse "origin/$HEAD_REF")" != "$HEAD_SHA" ] @@ -200,11 +168,9 @@ jobs: exit 0 fi git apply --index --binary "$RUNNER_TEMP/autofix/autofix.patch" - # The patch was cut on a runner PR code had run on, so treat it - # as untrusted: refuse to commit anything it staged outside the - # fix-owned paths. --no-renames keeps a staged rename listed - # as its delete and add halves, so a rename OUT of a protected - # path cannot hide behind an allowed destination. + # The patch was cut on a runner PR code ran on, so it is untrusted: nothing staged outside the fix-owned + # paths may be committed. --no-renames lists a staged rename as its delete and add halves, so a rename OUT + # of a protected path cannot hide behind an allowed destination. while IFS= read -r -d '' path; do case "$path" in lib/settings.schema.json | README.md | COVERAGE.md | action.yml | \ @@ -227,20 +193,17 @@ jobs: git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" subject="build: regenerate settings schema" - # A patch that stages only generated docs is docs drift alone. if git diff --cached --quiet -- lib/settings.schema.json src/upstream-gaps/; then subject="docs: regenerate generated docs" fi if [ "$PRUNED" = "true" ]; then subject="fix: graduate upstream gaps octokit now ships" fi - # The body's [dependabot skip] keeps Dependabot rebasing and - # updating the PR over this commit; without it Dependabot stops - # touching a PR once someone else pushes to it. + # [dependabot skip] keeps Dependabot rebasing and updating the PR over this commit; without it Dependabot + # stops touching a PR once someone else pushes to it. git commit --no-verify -m "$subject" -m "[dependabot skip]" - # The lease pins the remote to the exact commit the fix was - # built from, so a concurrent push (even a force-reset to an - # ancestor) rejects this one instead of racing it. + # The lease pins the remote to the exact commit the fix was built from, so a concurrent push (even a + # force-reset to an ancestor) rejects this one instead of racing it. if ! git push --force-with-lease="refs/heads/${HEAD_REF}:${HEAD_SHA}" \ "https://x-access-token:${TOKEN}@github.com/${GITHUB_REPOSITORY}.git" \ HEAD:"refs/heads/${HEAD_REF}"; then @@ -253,15 +216,13 @@ jobs: if [ "$CAN_RETRIGGER" = "true" ]; then exit 0 fi - # The fix landed via github.token, which starts no workflows: - # this PR's checks will not re-run on the new head. Surface that - # as a run annotation and, in the step below, on the PR; the push - # itself succeeded. + # The fix landed via github.token, which starts no workflows, so the PR's checks will not re-run on the new + # head: surface that here and, in the step below, on the PR. echo "notify=true" >> "$GITHUB_OUTPUT" echo "::warning::auto-fix pushed without REPO_PLATFORM_TOKEN - checks will not re-run on the new head;" \ "close/reopen the PR or register the token" - # One sticky comment per PR (the header is its identity), updated in - # place on later pushes; a failed post fails this step, not the push. + # One sticky comment per PR (the header is its identity), updated in place on later pushes; a failed post fails + # this step, not the push. - name: Tell the PR its checks will not re-run if: steps.push.outputs.notify == 'true' uses: marocchino/sticky-pull-request-comment@5770ad5eb8f42dd2c4f34da00c94c5381e49af88 # v3.0.5 diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 1a0933614..5682d7a18 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -1,10 +1,8 @@ -# Repo-owned CI checks, called by the managed ci.yml inside the all-green -# gate: every job here must pass before all-green goes green. Never -# overwritten by template sync, so edit freely. -# -# Two limits from running inside the `checks` caller: jobs here get no -# secrets and are capped at the caller's permissions (contents: read), and -# all-green only sees the aggregate result. +# Repo-owned, never overwritten by sync; the managed ci.yml calls it inside the all-green gate, so every job here +# must pass before all-green goes green. Three limits come with running inside the `checks` caller: +# secrets -> none reach these jobs +# permissions -> capped at the caller's contents: read +# all-green -> sees only the aggregate result name: Checks @@ -35,16 +33,11 @@ jobs: - name: Test run: bun run test - # The freshness tripwire behind the single-tag scheme's boundary (the - # anchor half lives in update-release-pr.yml): last-release-sha in - # release-please-config.json must be the newest release merge on main or - # its parent - version tags live off main, so a lost anchor would have - # every release PR refresh computed from a stale boundary. Gating here - # parks the release job (it needs all-green) before release-please can - # build a garbage release PR. The skip lives on the STEPS, per side: - # all-green treats a skipped needed job as failure, and the main-side - # check must judge only main (the release PR deliberately carries the - # anchored NEXT boundary, which anchor-check below validates instead). + # The freshness tripwire behind the boundary anchor (update-release-pr.yml): version tags live off main, so a lost + # anchor would have every release PR refresh computed from a stale boundary, and gating here parks the release job + # before release-please can build a garbage release PR. + # on main -> boundary-check: last-release-sha is the newest release merge or its parent + # on a release PR -> anchor-check: the PR carries the anchored NEXT boundary, which the main-side rule would reject boundary-check: runs-on: ubuntu-latest timeout-minutes: 5 @@ -61,11 +54,8 @@ jobs: - name: Confirm last-release-sha matches the newest release merge if: github.event_name != 'pull_request' && github.ref == 'refs/heads/main' run: bun .github/scripts/release-pipeline.ts boundary-check - # The PR-side half, on release PRs only: the merge must land the - # anchor, and the managed release-freshness gate proves only that the - # PR contains main's tip - not that the anchor commit survived the - # last release-please force-push. An unanchored release PR must be - # unmergeable here, not park the pipeline after its merge. + # The PR-side half, on release PRs only: an unanchored release PR must be unmergeable here, not park the + # pipeline after its merge. - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 if: github.event_name == 'pull_request' && startsWith(github.head_ref, 'release-please--') - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 @@ -76,8 +66,6 @@ jobs: if: github.event_name == 'pull_request' && startsWith(github.head_ref, 'release-please--') run: bun .github/scripts/release-pipeline.ts anchor-check - # lib/settings.schema.json is the committed settings.yml JSON Schema, - # generated from the SettingsFile types in src/; fail if it drifts. schema-check: runs-on: ubuntu-latest steps: @@ -90,11 +78,8 @@ jobs: - name: Regenerate the schema and compare run: bun run build:check - # The npm library gate: build lib/pkg/ (tsdown), judge the package shape - # (publint, attw), then install the packed tarball into a fresh consumer - # project and import it under Node and under tsc with skipLibCheck off, so - # a declaration leaking a devDependency type fails here, not on a - # consumer's machine. The consumer runs under node, so node is installed. + # The packed tarball is imported from a fresh consumer project under Node and under tsc with skipLibCheck off, so a + # declaration leaking a devDependency type fails here, not on a consumer's machine. package-smoke: runs-on: ubuntu-latest steps: @@ -110,11 +95,8 @@ jobs: - name: Build, pack, install, import, and typecheck the library run: bun .github/scripts/package-smoke.ts - # Self-test: run the action from this checkout in check mode against this - # repository. The bundle is not committed, so build it first. Drift or - # permission skips are fine (the default token cannot read admin - # surfaces); a CRASH is not - assert the action produced a result output, - # which an exception never does. + # Drift and permission skips are fine here (the default token cannot read admin surfaces); a CRASH is not, and an + # exception never sets the result output. self-check: runs-on: ubuntu-latest steps: @@ -142,11 +124,8 @@ jobs: *) echo "::error::action crashed (no result output)"; exit 1 ;; esac - # Diff-aware e2e smoke: on a PR, run only the scenarios and fuzz for the - # sections the PR touched (docs-only PRs skip; core or cross-cutting changes - # run every section). The action bundle runs under node24, so the smoke job - # installs node too. Non-PR events (push, dispatch) have no PR base to diff - # against, so they run the full corpus. + # The action bundle runs under node24, so node is installed beside bun. A non-PR event (push, dispatch) has no base + # to diff against, so it runs the full corpus. e2e-smoke: runs-on: ubuntu-latest steps: @@ -173,8 +152,8 @@ jobs: fi echo "sections=$SECTIONS" >> "$GITHUB_OUTPUT" echo "selected sections: $SECTIONS" - # The harness validates mock responses against the fetched, gitignored spec; - # the composite restores or fetches it, skipped with the run on a docs-only PR. + # The harness validates mock responses against the fetched, gitignored spec; the composite restores or fetches + # it, skipped with the run when the selector picks nothing. - uses: ./.github/actions/fetch-test-artifacts if: steps.select.outputs.sections != 'none' - name: Run scenarios and fuzz @@ -190,14 +169,9 @@ jobs: bun test/e2e/fuzz.ts --iterations 25 --sections "$SECTIONS" fi - # Endpoint-coverage tripwire: every "section.role" route in allEndpoints() - # and allGraphqlOps() must be hit at least once by the curated corpus. The - # nightly also runs this, but a PR that adds an endpoint without a scenario - # reaching it must fail on that PR, not the next night. The checker runs - # the full corpus itself (the diff-aware smoke job may run a subset) - the - # ~10 minutes are skipped only when the section selector proves the PR - # touched no route-bearing code (docs-only). The action bundle runs under - # node24, so node is installed here too. + # A PR that adds an endpoint without a scenario reaching it must fail on that PR, not the next night. Coldness is a + # whole-corpus property (deleting one scenario can chill any route), so the full corpus runs (~10 minutes) unless + # the selector finds nothing settings-related in the diff. endpoint-coverage: runs-on: ubuntu-latest steps: @@ -212,11 +186,8 @@ jobs: node-version: 24 - name: Install dependencies run: bun install --frozen-lockfile - # The same selector e2e-smoke uses, but only its "none" answer matters - # here: coldness is a whole-corpus property (deleting one scenario can - # chill any route), so any code change runs the full sweep, and only a - # provably docs-only PR skips it. Non-PR events run it always. It runs - # BEFORE the spec cache/fetch so a docs-only PR skips the download too. + # Only the selector's "none" answer matters here. It runs BEFORE the spec cache/fetch so a PR that selects + # nothing skips the download too. - name: Check whether the diff can affect route coverage id: select env: @@ -229,7 +200,6 @@ jobs: fi echo "sections=$SECTIONS" >> "$GITHUB_OUTPUT" echo "selected sections: $SECTIONS" - # The same fetched artifacts as e2e-smoke above, skipped with the sweep. - uses: ./.github/actions/fetch-test-artifacts if: steps.select.outputs.sections != 'none' - name: Endpoint-coverage tripwire diff --git a/.github/workflows/post-green.yml b/.github/workflows/post-green.yml index dbc4d448b..b7cffe5a7 100644 --- a/.github/workflows/post-green.yml +++ b/.github/workflows/post-green.yml @@ -1,7 +1,5 @@ -# Repo-owned: generated once by the fleet sync, never overwritten by a -# later sync. ci.yml calls it after the all-green gate, before the release, with -# the judged sha. Pending runs coalesce (ci.yml's concurrency group), so -# jobs here reconcile current state and stay idempotent. +# Repo-owned, never overwritten by sync; ci.yml calls it after the all-green gate, before the release, with the +# judged sha. Pending runs coalesce (ci.yml's concurrency group), so jobs here reconcile current state and stay idempotent. name: Post Green @@ -14,13 +12,9 @@ on: type: string jobs: - # Appends the green commit's packaged commit to the `build` branch and moves - # the `latest` tag (release-pipeline.ts advance-build). - # No permissions key: the job inherits the caller's grant and pushes with - # GITHUB_TOKEN when that is contents: write, which suffices because chain - # commits carry no .github/workflows/ (the same fact update-release.yml's - # append relies on); with a read ceiling and no PAT it warns and skips, and - # the release hook publishes the release's chain commit and latest itself. + # No permissions key: the job inherits the caller's grant, which is enough because chain commits carry no .github/workflows/. + # GITHUB_TOKEN at contents: write, or a PAT -> pushes build and latest + # a read ceiling and no PAT -> warns and skips; the release hook publishes the release's chain commit and latest itself build: runs-on: ubuntu-latest timeout-minutes: 10 @@ -28,23 +22,18 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: ref: ${{ inputs.sha }} - # The script judges whether build's recorded sources lie on this - # commit's history; a shallow checkout cannot answer that. + # The script judges whether build's recorded sources lie on this commit's history; a shallow checkout cannot. fetch-depth: 0 token: ${{ secrets.REPO_PLATFORM_TOKEN || github.token }} - # A dry-run push asks origin for its receive-pack advertisement over - # the very channel the real push uses, which a token without write - # access is refused; nothing is created. The target is a ref that never - # exists, not build itself: HEAD is no descendant of build's tip, so a - # dry run against build would be rejected locally as a non-fast-forward - # whatever the token. git's refusal is printed as plain log lines inside - # a stop-commands fence keyed by a token minted for this run, never - # inside a workflow command: the runner decodes % and line breaks in a - # command's message and acts on any line whose first non-blank text is - # "::", and only the fence's own token resumes command processing, so - # remote-supplied text can neither forge a command nor swallow the - # static error that follows (awk, not sed: it terminates the last line - # even when git did not, so the closing fence is a line of its own). + # A dry-run push asks origin for its receive-pack advertisement over the channel the real push uses, which a + # token without write access is refused; nothing is created. The target is a ref that never exists: HEAD is no + # descendant of build's tip, so a dry run against build itself would be rejected locally whatever the token. + # + # git's refusal is remote-supplied text, so it is printed inside a stop-commands fence keyed by a per-run token + # and never inside a workflow command. + # the runner acts on any line whose first non-blank text is "::" -> the fence disarms every such line + # only the fence's own token resumes command processing -> the text can neither forge a command nor swallow the error after it + # awk, not sed -> ends the last line even when git did not, so the fence stays alone - name: Check the token can push to build id: token env: diff --git a/.github/workflows/update-release-pr.yml b/.github/workflows/update-release-pr.yml index 152923941..b7c5caeb8 100644 --- a/.github/workflows/update-release-pr.yml +++ b/.github/workflows/update-release-pr.yml @@ -1,19 +1,10 @@ -# The release-PR hook of the release pipeline: repo-owned (a generated-once -# starter upstream, so template sync never overwrites it), called by the -# managed release.yml whenever release-please creates or refreshes the -# release PR, independently of any release cut. +# The release-PR hook of the release pipeline: repo-owned (a generated-once starter, never overwritten by sync), +# called by the managed ci.yml whenever release-please creates or refreshes the release PR, independently of any +# release cut. Its one job is the boundary anchor (anchorReleasePr in .github/scripts/release-pipeline.ts): called +# in the run that refreshed the PR, GITHUB_SHA is exactly the main head the refresh was built on. # -# This repository's job here is the single-tag scheme's boundary anchor -# (see .github/scripts/release-pipeline.ts): called in the same run that -# refreshed the release PR, GITHUB_SHA is exactly the main head the refresh -# was built on, and the anchor commits it into the PR branch as -# last-release-sha. The script re-verifies against origin - main still at -# this head, the branch built on it - before committing, and rechecks main -# before pushing; a push a release-please refresh overtakes is reapplied on -# the refreshed branch, any other push failure stops the job. -# Token caveat: pushes to the PR branch made with the default github.token -# do NOT re-trigger the PR's CI checks (the close/reopen note in -# release.yml); with REPO_PLATFORM_TOKEN they do. +# The anchor is pushed with the default github.token, which starts no workflows, so the PR's checks do not re-run +# on the anchored head; anchor-check's failure message says to close and reopen the PR. name: Update Release PR diff --git a/.github/workflows/update-release.yml b/.github/workflows/update-release.yml index 20768e82e..01a789b21 100644 --- a/.github/workflows/update-release.yml +++ b/.github/workflows/update-release.yml @@ -1,50 +1,27 @@ -# The update stage of the release pipeline: repo-owned, called by the -# managed release.yml between release-please cutting the DRAFT release and -# the attested publish. This repository's stage is the single-tag scheme: -# main is source-only (lib/index.js is never committed there), yet every -# ref a consumer can name from now on - the vX.Y.Z tags, the moving major -# vX, and the latest tag - must point at a packaged commit (the source tree -# without its workflows, plus the built bundle), all of them on the `build` -# branch post-green.yml appends to (the tags cut before that branch existed -# stay on main commits from when main committed the bundle). So this hook, not release-please, mints the -# tag (release-please-config.json sets `draft` and pins the fleet's -# `force-tag-creation` OFF): -# 1. resolve the merge commit from the draft's target commitish - NOT -# this run's GITHUB_SHA: the managed pipeline lets a newer push's run -# recover an older merge's pending release - and rebuild the bundle -# from it, verifying the committed schema byte-for-byte; -# 2. find the build commit whose recorded source is the merge commit -# (post-green appended it earlier in this run), appending it when -# none exists (post-green skipped for want of its token), create -# vX.Y.Z once, on that commit - born runnable, never moved (the -# release-tags ruleset freezes it, as SECURITY.md states) - and -# reconcile the latest tag to the chain commit of the newest main -# source; -# 3. force-move the major tag there and upload bundle and schema to the -# draft (published releases freeze their assets); -# 4. confirm origin's refs and the draft's assets; the managed publish -# stage then attests every asset and flips the release live, binding -# it to the existing vX.Y.Z tag. -# Only release.yml reaches this workflow, with the tag release-please just -# cut from its own merged PR, so a hand-edited manifest never mints a tag. -# A rerun converges while this release is its line's newest (package -# byte-verifies an existing tag; assets re-upload with --clobber); once a -# NEWER release shipped, the major move stops by design. +# The update stage of the release pipeline: repo-owned, called by the managed ci.yml between release-please +# cutting the DRAFT release and the attested publish. This hook, not release-please, mints the tag: main is +# source-only, so every consumable ref must point at a packaged commit on the `build` branch (the topology is in +# .github/scripts/release-pipeline.ts; release-please-config.json sets `draft` and pins `force-tag-creation` off). +# +# resolve the merge commit from the draft's target commitish -> rebuild the bundle, verify the committed schema byte-for-byte +# package -> find or append the build commit whose source is the merge commit, mint vX.Y.Z there ONCE, reconcile latest +# retag-major -> force-move vX there, then upload bundle and schema to the draft (a published release freezes its assets) +# verify -> confirm origin's refs and the draft's assets; the managed publish stage then attests and flips the release live +# +# Only ci.yml reaches this workflow, with the tag release-please just cut from its own merged PR, so a hand-edited +# manifest never mints a tag. A rerun converges while this release is its line's newest; once a NEWER release +# shipped, the major move stops by design. -# Finishing a dead run by hand (prefer re-running its FAILED jobs: a hand -# run has no attestation.json, and a chain commit it appends no run trailer). is the -# draft's target commitish. NEVER publish before `package` has minted the -# tag: a tagless draft makes GitHub create vX.Y.Z lazily at the merge -# commit on main, which has no bundle (the managed release.yml's generic -# recovery notes assume force-tag-creation and do not apply here). -# git checkout +# Finishing a dead run by hand (prefer re-running its FAILED jobs: a hand run has no attestation.json, and a chain +# commit it appends carries no run trailer). NEVER publish before `package` has minted the tag: a tagless draft makes +# GitHub create vX.Y.Z lazily at the bundle-less merge commit on main. +# git checkout (the draft's target commitish) # bun install --frozen-lockfile --ignore-scripts && bun run build # TAG=vX.Y.Z GITHUB_SHA= bun .github/scripts/release-pipeline.ts package # TAG=vX.Y.Z GITHUB_SHA= bun .github/scripts/release-pipeline.ts retag-major # TAG=vX.Y.Z GITHUB_SHA= bun .github/scripts/release-pipeline.ts verify # gh release upload vX.Y.Z lib/settings.schema.json lib/index.js --clobber # gh release edit vX.Y.Z --draft=false -# comment-cap: ignore a hand-recovery runbook whose seven commands mirror the steps below and must stay beside them name: Update Release @@ -61,19 +38,14 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 15 permissions: - # Tag pushes and the append to build when post-green did not run it; - # chain commits carry no .github/workflows/, so the default token - # needs no workflows grant (post-green.yml relies on the same fact). - # Also what makes the draft visible to gh. + # Tag pushes, and the append to build when post-green did not run it: chain commits carry no + # .github/workflows/, so the default token needs no workflows grant. Also what makes the draft visible to gh. contents: write outputs: source-sha: ${{ steps.source.outputs.sha }} steps: - # The release's identity is the draft's target commitish (release-please - # records the release PR's merge commit there), never this run's - # GITHUB_SHA: the managed head-check skips a run main outran and lets - # the newer push's run recover the pending release, so the run - # packaging a release need not be the run of its merge push. + # The release's identity is the draft's target commitish, never this run's GITHUB_SHA: the managed head-check + # lets a newer push's run recover an older merge's pending release. - name: Resolve the release's merge commit from the draft id: source env: @@ -90,9 +62,7 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: ref: ${{ steps.source.outputs.sha }} - # The script rebuilds build's tip from its recorded source and - # judges whether that source lies on main; a shallow checkout - # cannot answer either. + # The script judges whether the build tip's source lies on main; a shallow checkout cannot answer that. fetch-depth: 0 - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 with: @@ -101,28 +71,17 @@ jobs: run: | bun install --frozen-lockfile --ignore-scripts bun run build - # The uploaded schema asset and the raw URLs at the version and - # major tags must serve the bytes committed at the merge commit; - # a divergent regeneration (generator drift, or a compromised - # generator - SECURITY.md leans on this) stops the release here. + # The schema asset and the raw URLs at the tags must serve the bytes committed at the merge commit; a + # divergent regeneration (generator drift, or a compromised generator) stops the release here. git diff --exit-code lib/settings.schema.json - # Create the version tag on the merge commit's build commit exactly - # once, appending that commit when build lacks it, and reconcile - # latest to the chain commit of the newest main source; a rerun - # byte-verifies the existing tag instead (recorded source, whole tree, - # bundle bytes) and never moves it. Both paths end with the worktree - # bundle equal to the tagged bundle. + # Both paths (mint, or byte-verify on a rerun) leave the worktree bundle equal to the tagged one, which the + # upload below relies on. - name: Create or verify the version tag on the build commit env: TAG: ${{ inputs.tag }} SOURCE_SHA: ${{ steps.source.outputs.sha }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} run: GITHUB_SHA="$SOURCE_SHA" bun .github/scripts/release-pipeline.ts package - # Move the moving major tag (v2 for a v2.x.y release) to the packaged - # commit, re-verified against origin from scratch: major-pinned - # consumers must never receive an unpackaged commit, a package of the - # wrong source, or (on a rerun of an old release's job) a step - # backward to an older release. - name: Move the major tag to the packaged commit env: TAG: ${{ inputs.tag }} @@ -134,11 +93,6 @@ jobs: TAG: ${{ inputs.tag }} run: gh release upload "$TAG" lib/settings.schema.json lib/index.js --clobber - # The gate between packaging and publishing, against origin's ACTUAL - # state rather than anything the packaging job held locally: the version - # tag and the major must both point at a packaged commit recording this - # merge commit as its source, and that commit's tree must carry a - # non-empty lib/index.js. verify-release: needs: [package-release] runs-on: ubuntu-latest @@ -155,13 +109,10 @@ jobs: TAG: ${{ inputs.tag }} SOURCE_SHA: ${{ needs.package-release.outputs.source-sha }} run: GITHUB_SHA="$SOURCE_SHA" bun .github/scripts/release-pipeline.ts verify - # Publishing freezes the asset list, so an incomplete one must stop - # the pipeline here, while the release is still a draft (v2.0.0 - # shipped assetless exactly this way). attestation.json is the managed - # publish stage's asset, attached after this job and replaced on its - # reruns, so the check ignores it. A legacy attestation.jsonl (the - # name before the template renamed it) is stale provenance that stage - # no longer cleans up, so it is deleted here before the check. + # Publishing freezes the asset list, so an incomplete one must stop here while the release is still a draft + # (v2.0.0 shipped assetless exactly this way). + # attestation.json -> the managed publish stage's asset, attached after this job; ignored by the check + # attestation.jsonl -> stale provenance under the old name, which that stage no longer cleans up; deleted first - name: Confirm the release carries both packaged assets env: GH_TOKEN: ${{ github.token }} diff --git a/architecture.yml b/architecture.yml index ec49419f3..b5c2f13e6 100644 --- a/architecture.yml +++ b/architecture.yml @@ -1,10 +1,7 @@ -# The import layering of src/: one declaration, two readers. `bun run lint:arch` -# (.github/scripts/arch-lint.ts) fails on an import between two layers that is -# not listed under `edges`, and on a listed edge no file draws; `bun run -# build:docs` renders the same list as the module map in -# docs/reference/architecture.md. An edge counts whether the import is runtime -# or type-only. Imports inside one layer are not edges. The e2e fragments -# beside each section (mock.ts, generators.ts) are test code and excluded. +# The import layering of src/: one declaration, two readers. An edge counts whether the import is runtime or +# type-only; imports inside one layer are not edges. +# bun run lint:arch (.github/scripts/arch-lint.ts) -> fails on an import between layers not under `edges`, and on a listed edge no file draws +# bun run build:docs -> renders the same list as the module map in docs/reference/architecture.md layers: main: [src/main.ts] diff --git a/lefthook.yml b/lefthook.yml index 3b6c8e640..a5a9dd71e 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -1,7 +1,5 @@ -# Git hooks, installed by `bun install` (the prepare script). The hook is -# deliberately cheap: staged lint plus a whole-project typecheck (~0.5s); -# builds, tests, and schema freshness are CI's job behind the all-green -# gate. +# Git hooks, installed by `bun install` (the prepare script). Deliberately cheap: staged lint plus a whole-project +# typecheck; builds, tests, and schema freshness are CI's job behind the all-green gate. pre-commit: parallel: true jobs: diff --git a/src/action/artifact.ts b/src/action/artifact.ts index ec256c275..42946a5be 100644 --- a/src/action/artifact.ts +++ b/src/action/artifact.ts @@ -12,10 +12,8 @@ import type { ArtifactUploader } from "../index.js"; /** @actions/artifact uploads files from disk, so the ciphertext round-trips through a private temp directory. */ export const actionsArtifactUploader: ArtifactUploader = { async upload(name, file) { - // DefaultArtifactClient emits its own core.warning before it throws on a - // missing runtime token, so invoking it without one would double-warn. - // Fail before constructing it; deliverArtifactReport turns this into the - // single warning callers see. + // DefaultArtifactClient emits its own core.warning before it throws on a missing runtime token; failing before + // constructing it keeps the single warning deliverArtifactReport turns this into. if (!process.env.ACTIONS_RUNTIME_TOKEN) { throw new Error( "the artifact service is unavailable: no ACTIONS_RUNTIME_TOKEN in the environment. Artifact upload needs a GitHub-hosted or self-hosted Actions runner (it is not available on GitHub Enterprise Server or outside Actions)", diff --git a/src/action/inputs.ts b/src/action/inputs.ts index e888d3c5c..aa0f4d986 100644 --- a/src/action/inputs.ts +++ b/src/action/inputs.ts @@ -1,15 +1,8 @@ -/** - * The action's inputs adapter: parseConfig over the runner's inputs and the - * GITHUB_* context, so run() never touches raw inputs. - */ - import * as core from "@actions/core"; import type { Result } from "neverthrow"; import { type Problem, parseConfig, type RunConfig } from "../index.js"; -/** Read and validate every input the step set; the first problem wins. */ export function parseActionConfig(): Result { - // @actions/core reads INPUT_ (uppercased, spaces to underscores - - // dashes survive, e.g. `settings-file` -> INPUT_SETTINGS-FILE) and trims. + // @actions/core reads INPUT_ (uppercased, spaces to underscores, dashes kept: `settings-file` -> INPUT_SETTINGS-FILE) and trims. return parseConfig((name) => core.getInput(name), process.env); } diff --git a/src/action/io.ts b/src/action/io.ts index be7c66cfd..7cc1886fe 100644 --- a/src/action/io.ts +++ b/src/action/io.ts @@ -1,17 +1,8 @@ -/** - * The Io implementation over @actions/core - the only module outside the - * runner that may import it, so every channel has one production path. - */ - import { appendFileSync } from "node:fs"; import * as core from "@actions/core"; import { type Io, maskRegistry, type OutputName } from "../index.js"; -/** - * The description of every action output, generated into the action.yml - * `outputs` block (bun run build:action-docs). The satisfies clause locks the - * keys to OutputName: a missing or phantom output fails to compile here. - */ +/** Generated into the action.yml `outputs` block (bun run build:action-docs); the satisfies clause locks the keys to OutputName. */ export const OUTPUT_DECLS = { result: { description: @@ -27,10 +18,8 @@ export const OUTPUT_DECLS = { }, } as const satisfies Record; -// @actions/core owns workflow-command escaping (%, CR, LF); the static map -// keeps the namespace access tree-shakeable (biome noDynamicNamespaceImportAccess). -// The references are captured at module load, so a test spying on core.warning -// after import would not be observed here - none does today. +// @actions/core owns workflow-command escaping (%, CR, LF). The static map keeps the namespace access tree-shakeable +// (biome noDynamicNamespaceImportAccess); the references are captured at module load, so a spy installed after import is not seen. const annotators = { notice: core.notice, warning: core.warning, error: core.error } as const; export function annotate(level: keyof typeof annotators, message: string): void { @@ -44,7 +33,6 @@ function setOutput(name: OutputName, value: string): void { } } -/** Append one summary block; skipped when GITHUB_STEP_SUMMARY is unset (local/test runs). */ function appendSummary(markdown: string): void { const file = process.env.GITHUB_STEP_SUMMARY; if (!file) { @@ -53,7 +41,6 @@ function appendSummary(markdown: string): void { appendFileSync(file, `${markdown}\n`); } -/** The production Io sink: annotations and the trace via the runner, logs to stdout. */ export const actionsIo: Io = { annotate, log: (line) => console.log(line), diff --git a/src/action/run.ts b/src/action/run.ts index b0871311d..bdf4cf5e4 100644 --- a/src/action/run.ts +++ b/src/action/run.ts @@ -1,25 +1,3 @@ -/** - * github-settings-as-code: apply a declarative .github/settings.yml to the repo. - * - * Policy model: - * - mode: apply (default) mutates; check reports drift and exits 1 on any. - * - on-missing-permission: fail (default) | warn. Under warn, a section the - * token cannot touch is skipped with a warning and the run stays green - * (partial success) - unless the section is listed in required-sections. - * - Non-permission errors always fail, loudly, with the API message. - * - * Multi-repo mode (repos / repos-dir / defaults-file inputs): one run in an - * admin repo applies settings to many repositories - from per-repo files - * checked into the admin repo (central), or from each target's own - * .github/settings.yml (remote), or the defaults-file document for a - * remote target that has no file. Targets run independently; the run fails - * at the end if any target failed. - * - * mode: merge folds an ordered list of settings files into one document - * written to merged-file, for a later apply or check step to run; it never - * touches GitHub (src/flows/merge.ts). - */ - import { type ArtifactUploader, concludeMerge, @@ -36,10 +14,7 @@ import { actionsArtifactUploader } from "./artifact.js"; import { parseActionConfig } from "./inputs.js"; import { actionsIo } from "./io.js"; -/** - * Execute the action; returns the process exit code. `overrides` exists for - * tests (a stub client, a capturing Io, a capturing uploader); production uses the defaults. - */ +/** `overrides` exists for tests (a stub client, a capturing Io, a capturing uploader). */ export async function run(overrides?: { api?: GithubClient; io?: Io; @@ -53,7 +28,7 @@ export async function run(overrides?: { return failRun(io, parsed.error); } const cfg = parsed.value; - // A merge folds local files only: no client is built, so no token is read. + if (cfg.kind === "merge") { return runMerge(cfg, io).match( (merged) => concludeMerge(io, merged), diff --git a/src/discovery/central.ts b/src/discovery/central.ts index ed7db6fd4..96d050a10 100644 --- a/src/discovery/central.ts +++ b/src/discovery/central.ts @@ -11,10 +11,6 @@ import { type CentralTarget, SLUG_RE } from "./targets.js"; const YAML_EXT = /\.ya?ml$/; -/** - * Read the repos-dir layout: `.yml` (owner = the admin repo's owner) - * at the top level, `/.yml` one directory deep. - */ export function resolveCentralTargets( reposDir: string, adminOwner: string, @@ -27,11 +23,10 @@ export function resolveCentralTargets( } const targets: CentralTarget[] = []; const warnings: string[] = []; - // Invalid filenames and duplicate slugs are collected across the WHOLE - // walk and reported once: each fix is a file rename or deletion, so N bad - // files must cost one run to discover, not N. + // Invalid filenames and duplicate slugs are collected across the WHOLE walk: each fix is a rename or deletion, so N + // bad files must cost one run to discover, not N. const errors: CentralFileProblem[] = []; - const seen = new Map(); // lowercased slug -> origin + const seen = new Map(); const addTarget = (slug: string, filePath: string): void => { if (!SLUG_RE.test(slug)) { errors.push({ kind: "not-a-slug", filePath, slug }); @@ -67,8 +62,7 @@ export function resolveCentralTargets( }; try { - // Top-level files needing an owner share ONE root cause when it is - // unknown; they are collected and reported as one error below. + // Top-level files needing an unknown owner share ONE root cause and are reported as one error below. const ownerlessFiles: string[] = []; for (const entry of readdirSync(reposDir).sort()) { const entryPath = join(reposDir, entry); diff --git a/src/discovery/discover.ts b/src/discovery/discover.ts index 4cc9c085e..45c017acf 100644 --- a/src/discovery/discover.ts +++ b/src/discovery/discover.ts @@ -17,7 +17,6 @@ type DiscoveredRepo = Pick< "full_name" | "archived" | "fork" | "topics" | "visibility" | "private" >; -/** One discovered repository the run will address: its slug plus the visibility the listing reported. */ export interface DiscoveredRepoRef { slug: string; visibility: "public" | "private" | "internal"; @@ -38,18 +37,11 @@ function sealFiltered(ref: DiscoveredRepoRef): FilteredRepoRef { } /** - * Narrow the listing's visibility for the REDACTION decision, failing closed. - * `visibility` is a plain string in the API schema and optional on GHES; the - * always-present `private` flag is the authority. A repo is treated as private - * whenever `private === true` (even if a stale/forged `visibility` says - * "public"), and when BOTH fields are missing (an unknown repo is hidden, never - * exposed). Only `private === false` (or a trustworthy non-public visibility) - * yields a non-private classification. `internal` survives when `visibility` - * names it and `private` does not contradict it. + * Fails closed for the REDACTION decision. `visibility` is a plain string in the API schema and optional on GHES, so + * the always-present `private` flag is the authority: private === true wins over any `visibility` (even a stale + * "public"), and with BOTH fields missing the repo is hidden, never exposed. */ function normalizeVisibility(repo: DiscoveredRepo): DiscoveredRepoRef["visibility"] { - // private === true always wins: it is the field the API guarantees, and a - // repo the token can see as private must never be classed public. if (repo.private === true) { return "internal" === repo.visibility ? "internal" : "private"; } @@ -57,7 +49,6 @@ function normalizeVisibility(repo: DiscoveredRepo): DiscoveredRepoRef["visibilit if (visibility === "public" || visibility === "private" || visibility === "internal") { return visibility; } - // visibility absent: trust an explicit private === false, else fail closed. return repo.private === false ? "public" : "private"; } @@ -68,10 +59,8 @@ export const FORKS_FILTERS = ["include", "exclude", "only"] as const; export const AFFILIATIONS = ["owner", "collaborator", "organization_member"] as const; /** - * The filter reason tag for a repo skipped because it is archived. Shared by - * the discovery rule that emits it and formatSkipNotice, which special-cases - * it for the unarchive-to-manage prose - a literal in one place and not the - * other would silently drop that guidance. + * Shared by the rule that emits it and formatSkipNotice, which special-cases it for the unarchive-to-manage prose; a + * literal in one place and not the other would silently drop that guidance. */ const ARCHIVED_REASON = "archived"; @@ -94,17 +83,11 @@ export const DEFAULT_DISCOVERY_FILTERS: DiscoveryFilters = { exclude: [], }; -/** Compile a "*"-only wildcard into an anchored, case-insensitive RegExp. */ function compileExcludePattern(pattern: string): RegExp { const escaped = pattern.replace(/[.+^${}()|[\]\\?]/g, "\\$&").replace(/\*/g, ".*"); return new RegExp(`^${escaped}$`, "i"); } -/** - * True when the pattern excludes this owner/name slug. A pattern containing - * "/" matches the full slug; otherwise it matches the name alone (same - * split as the repos-dir .yml vs /.yml layout). - */ export function excludeMatches(pattern: string, slug: string): boolean { const candidate = pattern.includes("/") ? slug : (slug.split("/")[1] ?? slug); return compileExcludePattern(pattern).test(candidate); @@ -112,16 +95,13 @@ export function excludeMatches(pattern: string, slug: string): boolean { export interface DiscoveryResult { repos: DiscoveredRepoRef[]; - /** Repositories dropped by a filter, grouped by the first reason that hit. */ filtered: Array<{ reason: string; repos: FilteredRepoRef[] }>; } -/** What discovery can fail with: the request, the transport, or a response that is not a list. */ export type DiscoveryProblem = ProblemOf< "discovery-request-failed" | "discovery-transport-failed" | "discovery-response-not-a-list" >; -/** Discover the repositories the token's user can see, applying the filters. */ export function discoverRepos( api: GithubClient, filters: DiscoveryFilters, @@ -142,10 +122,8 @@ export function discoverRepos( }), ).andThen((page) => { if ("error" in page) { - // The PAT advice fits genuine denials only. A rate-limit 403 is NOT a - // permission problem (isPermissionError excludes it), so it must not - // tell the operator to swap tokens and abandon "*" discovery for - // nothing. 401 is an invalid/expired token, which the PAT advice covers. + // A rate-limit 403 is NOT a permission problem (isPermissionError excludes it), so it never reads as denied and + // never tells the operator to swap tokens; 401 (an invalid or expired token) does. return err({ code: "discovery-request-failed", path, @@ -164,13 +142,10 @@ export function discoverRepos( }); } -/** Keep the repositories the filters admit; group the rest by the first reason that hit. */ function applyFilters( repos: readonly DiscoveredRepo[], filters: DiscoveryFilters, ): DiscoveryResult { - // One rule per filter, in reporting-attribution order; the first reason - // returned is the one a skipped repo is grouped under. const rules: Array<(repo: DiscoveredRepo) => string | null> = [ (repo) => { const isInternal = repo.visibility === "internal"; @@ -246,11 +221,8 @@ function applyFilters( } /** - * One aggregate notice per filter reason: with "*" fleets, per-repo notices - * would flood the annotations UI (GitHub caps annotations per step). - * `redactPrivate` keeps private and internal repository names out of the - * notice: only public slugs are listed, hidden ones become a count, and a - * group with no public repos renders as a count with no names at all. Under + * One aggregate notice per filter reason: per-repo notices for a "*" fleet would flood the annotations UI (GitHub caps + * annotations per step). Under `redactPrivate` only public slugs are listed and hidden ones become a count; under * `show` the operator opted into naming them, so the seal opens here. */ export function formatSkipNotice( diff --git a/src/discovery/repos-input.ts b/src/discovery/repos-input.ts index 4bd4f3504..31110cdbc 100644 --- a/src/discovery/repos-input.ts +++ b/src/discovery/repos-input.ts @@ -1,10 +1,7 @@ -/** Parsing for the "repos" input: explicit slugs or "*" discovery. */ - import { err, ok, type Result } from "neverthrow"; import type { ProblemOf } from "../problem.js"; import { SLUG_RE } from "./targets.js"; -/** Parse the repos input: comma/newline-separated slugs, or exactly "*". */ export function parseReposInput( raw: string, ): Result< @@ -21,9 +18,8 @@ export function parseReposInput( } return ok({ slugs: [], discover: true }); } - // Malformed and repeated entries are collected across the whole list and - // reported once, so N bad entries cost one run to discover, not N. Both - // pools are Sets: a bad entry pasted twice is one offender, not two. + // Malformed and repeated entries are collected across the whole list, so N bad entries cost one run to discover, not + // N; the pools are Sets, so a bad entry pasted twice is one offender. const seen = new Set(); const invalid = new Set(); const duplicated = new Set(); diff --git a/src/discovery/targets.ts b/src/discovery/targets.ts index 44e1fa9f3..1fcaf274a 100644 --- a/src/discovery/targets.ts +++ b/src/discovery/targets.ts @@ -1,8 +1,6 @@ /** - * The multi-repo target model shared by central resolution, repos-input - * parsing, and discovery. Central files WIN over repos-input entries for - * the same repository: the checked-in file is a curated, code-reviewed - * artifact; the remote file is self-service. + * The multi-repo target model. Central files WIN over repos-input entries for the same repository: the checked-in + * file is a curated, code-reviewed artifact; the remote file is self-service. */ import { err, ok, type Result } from "neverthrow"; @@ -26,12 +24,7 @@ export type Target = CentralTarget | RemoteTarget; export const SLUG_RE = /^[\w.-]+\/[\w.-]+$/; -/** - * A repository reference PARSED ONCE at a validating boundary: the owner and - * name halves plus the original slug, so downstream code never re-splits a - * string (and an owner that disagrees with its slug is unconstructible in - * practice - the only constructor derives all three from one value). - */ +/** PARSED ONCE at a validating boundary, so downstream code never re-splits a string; the only constructor derives all three from one value. */ export interface RepoRef { readonly owner: string; readonly name: string; @@ -39,11 +32,8 @@ export interface RepoRef { } /** - * Parse an owner/name slug into a RepoRef, or the problem when it is not one. - * The smart constructor lives beside SLUG_RE so every boundary (the - * repository input, the repos list, discovery's full_name) validates and - * splits through the same definition; internal code then carries the parsed - * proof instead of a bare string. + * The smart constructor lives beside SLUG_RE so every boundary (the repository input, the repos list, discovery's + * full_name) validates and splits through the same definition. */ export function parseRepoSlug(raw: string): Result> { if (!SLUG_RE.test(raw)) { @@ -54,14 +44,9 @@ export function parseRepoSlug(raw: string): Result no delta + * desired "", live null or absent -> no delta */ -/** - * One step of a delta's path below the compared root: an object key, the - * index of a list item paired by shape, or the key of a list item paired by - * a declared key ("rules[deletion]"). - */ type PathStep = string | number | { readonly key: string }; -/** How a list's items were paired: by a declared item key, by whole shape, or by value. */ type ListMatch = "key" | "shape" | "value"; -/** - * One divergence between a desired value and its live counterpart: mismatch (both present, - * unequal), phantom (a non-empty declared field the live object has no key for), missing (a - * declared list item no live item pairs with), undeclared (a live list item nothing pairs with). - */ +/** A phantom is a non-empty declared field the live object has no key for: a typo, or a write-only field. */ export type Delta = | { readonly kind: "mismatch"; @@ -46,14 +34,15 @@ export type Delta = export interface DeltaOptions { /** - * Per nested list (by dotted object path below the root), the item key to pair by; a missing or - * repeated key is a declaration bug, and an unnamed list pairs object items by shape, others by - * value. Omitted entirely, lists fall back to the legacy `type` sniffing subsetDiff callers rely on. + * Per nested list (by dotted object path below the root), the item key to pair by; a missing or repeated key is a + * declaration bug. + * + * a list not named here -> object items pair by shape, others by value + * matchBy omitted -> lists fall back to the legacy `type` sniffing subsetDiff callers rely on */ readonly matchBy?: Readonly>; } -/** A value with no JSON structure beneath it. */ function isScalar(value: unknown): boolean { return typeof value !== "object" || value === null; } @@ -107,8 +96,7 @@ function walk( } const liveRecord = liveValue as Record; for (const [key, value] of Object.entries(desired as Record)) { - // hasOwn, not indexing: a key named like a prototype member (toString) - // must read as absent, not as the inherited function. + // hasOwn, not indexing: a key named like a prototype member (toString) must read as absent, not as the inherited function. const child = Object.hasOwn(liveRecord, key) ? liveRecord[key] : ABSENT; walk(value, child, [...path, key], keyPath === "" ? key : `${keyPath}.${key}`, opts, out); } @@ -126,14 +114,12 @@ function walk( } } -/** The `type` key of an object list item, or null when the item has none (legacy sniffing). */ function typeOf(item: unknown): string | null { return typeof item === "object" && item !== null && "type" in (item as object) ? String((item as { type: unknown }).type) : null; } -/** The declared pairing key of an item in a matchBy list; a missing one is a declaration bug. */ function itemKey(item: unknown, key: string, keyPath: string, side: "desired" | "live"): string { if (typeof item !== "object" || item === null || !Object.hasOwn(item, key)) { throw new Error( @@ -167,9 +153,8 @@ function walkList( return; } if (opts.matchBy === undefined) { - // Legacy sniffing: pair by `type` only when types are unique on both - // sides (ruleset rules); environment reviewers repeat types and fall - // through to shape pairing below. + // Legacy sniffing pairs by `type` only when types are unique on both sides (ruleset rules); environment reviewers + // repeat types and fall through to shape pairing below. const desiredTypes = desired.map(typeOf); const liveTypes = live.map(typeOf); const typed = @@ -191,9 +176,7 @@ function walkList( desired.length > 0 && desired.every((item) => typeof item === "object" && item !== null && !Array.isArray(item)); if (objectList) { - // Order-insensitive: each desired item must match SOME live item, and - // live items matched by nothing are undeclared (a full-payload write - // would remove them). + // Order-insensitive; a live item nothing pairs with is undeclared because a full-payload write would remove it. const liveItems = [...live]; for (const [index, item] of desired.entries()) { const matchIndex = liveItems.findIndex( @@ -210,7 +193,6 @@ function walkList( } return; } - // Scalar lists (ref includes, topics) compare as sets. const desiredSet = new Set(desired.map((v) => JSON.stringify(v))); const liveSet = new Set(live.map((v) => JSON.stringify(v))); for (const [index, value] of desired.entries()) { @@ -227,7 +209,6 @@ function walkList( } } -/** Pair a list by an item key: declared-keys-only per item, and a live key absent from desired is undeclared. */ function walkKeyed( desired: unknown[], liveByKey: ReadonlyMap, @@ -261,7 +242,6 @@ function walkKeyed( } } -/** A delta's location as drift prose spells it: `root.field[0].nested[type]`. */ function renderPath(root: string, path: readonly PathStep[]): string { const steps = path.map((step) => typeof step === "string" @@ -273,7 +253,6 @@ function renderPath(root: string, path: readonly PathStep[]): string { return `${root}${steps.join("")}`; } -/** The prose for a value that is present on both sides but not the same, or declared and absent live. */ function mismatchLine(at: string, desired: unknown, live: unknown): string { if (desired === null || desired === undefined) { return `${at}: expected empty, live has ${JSON.stringify(live)}`; @@ -290,7 +269,6 @@ function mismatchLine(at: string, desired: unknown, live: unknown): string { return `${at}: ${JSON.stringify(desired)} != ${JSON.stringify(live)}`; } -/** The one rendering of a delta as a drift line, under the caller's root label. */ export function renderDelta(root: string, delta: Delta): string { const at = renderPath(root, delta.path); switch (delta.kind) { @@ -313,20 +291,11 @@ export function renderDelta(root: string, delta: Delta): string { } } -/** - * The rendered drift lines of a declared-keys-only comparison under `path`, - * pairing lists by the legacy sniffing (see DeltaOptions.matchBy). The - * string edge the sections not yet reading deltas() consume. - */ export function subsetDiff(desired: unknown, live: unknown, path: string): string[] { return deltas(desired, live).map((delta) => renderDelta(path, delta)); } -/** - * The declared top-level keys the live object has no key for (the top-level phantom deltas by - * name; declared null/"" read as equal to absent). Sections whose write is gated by a - * comparison note these so the gating keys do not silently rewrite on every run. - */ +/** Sections whose write is gated by a comparison note these, so the gating keys do not silently rewrite on every run. */ export function phantomKeys(desired: Record, live: unknown): string[] { return deltas(desired, live).flatMap((delta) => delta.kind === "phantom" && delta.path.length === 1 && typeof delta.path[0] === "string" @@ -335,11 +304,6 @@ export function phantomKeys(desired: Record, live: unknown): st ); } -/** - * The note for phantom keys, rendered in both modes beside the update they - * gate. `noun` names the live resource ("label"); `rewrite` says what apply - * will keep doing ("this update will re-run"). - */ export function phantomNote(prefix: string, keys: string[], noun: string, rewrite: string): string { const list = keys.map((k) => `"${k}"`).join(", "); return `${prefix}: declared key(s) ${list} do not exist on the live ${noun}, so if GitHub ignores them ${rewrite} on every apply without converging. Fix the key name, or remove it from the settings file`; diff --git a/src/engine/execute.ts b/src/engine/execute.ts index 3a86370e4..3e8ec7150 100644 --- a/src/engine/execute.ts +++ b/src/engine/execute.ts @@ -1,7 +1,6 @@ /** - * The plan executor: the ONE place a section's operations touch the API. Operations run in - * declaration order through the request helpers, so error classification (PermissionDenied vs hard - * error, hints, denial hints) matches the reads'; each success renders the operation's change line. + * The ONE place a section's operations touch the API. Operations go through the request helpers so error classification + * (PermissionDenied vs hard error, the hints) matches the reads'. */ import type { RepoRef } from "../discovery/targets.js"; @@ -16,11 +15,7 @@ import { tryCallDeclared, } from "../sections/contract/requests.js"; -/** - * How a plan's execution ended; a failure mid-plan leaves the earlier - * operations applied. `landed` counts the requests GitHub accepted, which - * the change lines cannot: a change or capture hook can fail after landing. - */ +/** `landed` counts the requests GitHub accepted, which the change lines cannot: a change or capture hook can fail after landing. */ interface PlanExecutionBase { readonly changes: readonly string[]; readonly notes: readonly string[]; @@ -32,9 +27,8 @@ type PlanExecution = | (PlanExecutionBase & { readonly status: "failed"; readonly error: unknown }); /** - * A declaration under `role` in `dict`, by OWN property only: an erased plan - * carries a bare string role, and an inherited name ("constructor", a - * polluted prototype key) must read as undeclared, never as a value to call. + * OWN property only: an erased plan carries a bare string role, and an inherited name ("constructor") must read as + * undeclared, never as a value to call. */ function declared(dict: Readonly> | undefined, role: string): T | undefined { return dict !== undefined && Object.hasOwn(dict, role) ? dict[role] : undefined; @@ -43,15 +37,13 @@ function declared(dict: Readonly> | undefined, role: string function noop(): void {} /** - * The change thunk and capture hook are synchronous by contract: a promise - * here would let the line record before the hook settled and drop its - * rejection, so a thenable is a bug caught before the line records. + * The change thunk and capture hook are synchronous by contract: a promise would let the line record before the hook + * settled and drop its rejection, so a thenable is a bug caught before the line records. */ function rejectThenable(section: SectionMeta, role: string, hook: string, value: unknown): void { const then = (value as { then?: unknown } | null)?.then; if (typeof then === "function") { - // The BUG below is the report; the discarded promise's own rejection - // must not surface a second time, and a then() that throws changes nothing. + // The BUG below is the report; the discarded promise's own rejection must not surface a second time. try { (then as (onFulfilled: () => void, onRejected: () => void) => unknown).call( value, @@ -65,10 +57,6 @@ function rejectThenable(section: SectionMeta, role: string, hook: string, value: } } -/** - * Execute every operation of `plan` against `api` under ONE failure rule: - * the change line records only once request, render, and capture succeeded. - */ export async function executePlan( plan: SectionPlan, section: SectionMeta, @@ -76,8 +64,6 @@ export async function executePlan( repo: RepoRef, tools: ExecTools, ): Promise { - // Thunks see a frozen projection holding the resolver and nothing else: whatever - // object the caller passed as tools never reaches section code. const exec: ExecTools = Object.freeze({ resolveSecret: (reference: string): string => tools.resolveSecret(reference), }); @@ -89,8 +75,7 @@ export async function executePlan( try { let response: unknown; if (typeof op.role !== "string") { - // Only a string can name a declared role: a number would coerce onto - // a matching key and a symbol would enter the property-key path. + // A number would coerce onto a matching key and a symbol would enter the property-key path. throw new Error( `BUG: ${section.key} planned an operation whose role is a ${typeof op.role}, not the name of a declared write`, ); @@ -98,8 +83,7 @@ export async function executePlan( const endpoint = declared(section.endpoints, op.role); if (endpoint !== undefined) { if (endpointMethod(endpoint.route) === "GET") { - // Only the erased view can name a read; executing it would render - // a change line for a request that changed nothing. + // Only the erased view can name a read; executing it would render a change line for a request that changed nothing. throw new Error( `BUG: ${section.key} planned an operation under role "${op.role}", which is a read endpoint (${endpoint.route}); only write roles are plannable`, ); @@ -147,7 +131,6 @@ export async function executePlan( const lines = typeof op.change === "function" ? op.change(response) : op.change; rejectThenable(section, op.role, "change thunk", lines); if (lines.length === 0) { - // The request landed, so apply must have something to report. throw new Error( `BUG: ${section.key}: operation "${op.role}" rendered no change line for a request that landed`, ); diff --git a/src/engine/layers.ts b/src/engine/layers.ts index 89cf95324..4253eac59 100644 --- a/src/engine/layers.ts +++ b/src/engine/layers.ts @@ -1,16 +1,12 @@ /** - * The layered merge: an ORDERED list of settings documents folded low to - * high into one document. Plain objects merge key by key; a higher scalar, - * array, or YAML-tagged value replaces; a higher `null` deletes what a lower - * layer declared (reported as a notice) and stays as written when nothing - * below declares the key, so `pages: null` keeps its engine meaning. The - * knobbed list sections (UNDECLARED_POLICY_SECTIONS) are the one place lists - * combine: under "merge", a section whose module declares a `layering` unions - * its entries by identity (two entries whose key sets intersect are one); - * every other list is replaced by the higher layer's. - * A layer that refers back into itself (a YAML anchor aliased inside its own - * node) is refused at the boundary: layers are trees. - * Pure: no Io, no GitHub. The helpers merge.ts shares live here too. + * The layered merge, folded low to high; pure (no Io, no GitHub), and layers are trees: a document aliasing a node + * inside itself is refused. Lists never combine except a knobbed section's entries under "merge", unioned by the + * module's key (and the nested lists that key declares). + * + * higher plain mapping -> merged key by key + * higher scalar, list, tagged -> replaces + * higher null over a lower declaration -> deletes it (an opt-out notice) + * higher null over nothing, or a null -> stays as written, so `pages: null` keeps its engine meaning */ import { err, ok, type Result } from "neverthrow"; @@ -27,7 +23,6 @@ export interface Layer { readonly doc: unknown; } -/** How a knobbed section combines with the layers below it. */ export type Layering = "merge" | "replace"; /** A lower declaration a higher layer deleted with `null`. */ @@ -36,7 +31,6 @@ export interface OptOutNotice { readonly path: string; } -/** The directive key, on a knobbed wrapper or at a document's top level. */ const LAYERING_KEY = "_layering"; const LAYERINGS: readonly Layering[] = ["merge", "replace"]; @@ -46,20 +40,10 @@ function isLayering(value: unknown): value is Layering { } /** - * Rewrite each UNDECLARED_POLICY_SECTIONS value from the plain array form to - * the wrapped one, PRESERVING OMISSION of the policy key - a plain array - * becomes `{entries}` with NO `_undeclared`. That omission is what lets a - * merge inherit a lower layer's policy: had the plain form been resolved to - * its default here, the higher layer's resolved default would overwrite the - * lower's explicit policy. Values in neither form (null opt-outs, malformed - * declarations) pass through untouched so the null semantics and post-merge - * validation see them as written; a wrapper keeps every key it carries, - * `_layering` included. Returns a shallow copy; the input is never mutated. + * A plain array becomes `{entries}` with NO `_undeclared`: that omission is what lets a merge inherit a lower layer's + * policy. Resolved to the section default here, a higher layer's default would overwrite the lower's explicit policy. */ function normalizeKnobbedSections(settings: unknown): unknown { - // A document that is not a mapping (a raw list, a scalar) has no sections - // to normalize; hand it on untouched so the top-level validator still sees - // exactly what was written. if (!isPlainObject(settings)) { return settings; } @@ -73,16 +57,11 @@ function normalizeKnobbedSections(settings: unknown): unknown { return out; } -/** The section's own default policy, from its undeclaredDefault declaration. */ function sectionDefaultPolicy(key: UndeclaredPolicySection): UndeclaredPolicy { return defaultUndeclaredPolicy(sectionModule(key)); } -/** - * After a merge: a wrapped section that still carries no explicit policy - * resolves to the section's default, so the merged document is - * self-describing. Runs on the merged document the caller owns. - */ +/** After the fold, a wrapper still without `_undeclared` takes the section default, so the merged document is self-describing. */ function resolveUndeclaredPolicies(merged: Record): void { for (const key of UNDECLARED_POLICY_SECTIONS) { const value = merged[key]; @@ -93,31 +72,22 @@ function resolveUndeclaredPolicies(merged: Record): void { } /** - * The clone in progress of every node on the current descent. A node met - * again while its clone is being built encloses itself and gets that clone, - * so a cyclic document terminates; a node aliased twice without enclosing - * itself is cloned per occurrence, in the position each occurrence sits in - * (a wrapper shared between a private key and `rulesets` is data under one - * and a keyed list under the other). + * The clone in progress of every node on the current descent: a node met again inside itself gets that clone, so a + * cyclic document terminates. A node aliased twice WITHOUT enclosing itself is cloned per occurrence, in the position + * each sits in (a wrapper shared between a private key and `rulesets` is data under one and a keyed list under the other). */ type Descent = WeakMap; -/** The keyed layering of a knobbed section's entries; undefined for a section that always replaces. */ function sectionLayering(key: string): KeyedListLayering | undefined { const section = UNDECLARED_POLICY_SECTIONS.find((candidate) => candidate === key); return section === undefined ? undefined : sectionModule(section).layering; } -/** A value the merge does not merge into: a mapping walks on, anything else is copied as written. */ function stripValue(value: unknown, descent: Descent): unknown { return isPlainObject(value) ? stripMapping(value, undefined, descent) : structuredClone(value); } -/** - * The merge's walk over one mapping (mergeMappings): every null-valued key - * is a marker and drops; `keyed` names the fields whose lists the merge - * combines by key, exactly as it names them there. - */ +/** Mirrors mergeMappings: a null-valued key is a marker and drops, and `keyed` names the same fields it names there. */ function stripMapping( map: Readonly>, keyed: Readonly> | undefined, @@ -144,12 +114,7 @@ function stripMapping( return out; } -/** - * A keyed list as the merge combines it (unionKeyed): only a "merge"-combined - * list is entered, its mapping entries walked with their own nested keyed - * lists; a "replace"-combined list, or a value that is not a list, is data - * the merge copies as written, nulls included. - */ +/** Mirrors unionKeyed: the same lists are entered. */ function stripKeyedList(list: unknown, keyed: KeyedListLayering, descent: Descent): unknown { if (!Array.isArray(list) || keyed.combine === "replace") { return stripValue(list, descent); @@ -170,15 +135,12 @@ function stripKeyedList(list: unknown, keyed: KeyedListLayering, descent: Descen } /** - * Drop every null the merge would read as a marker, so a layer validated on - * its own is seen as the merge could leave it: null-valued mapping keys at - * any depth the merge walks, including inside the entries of a keyed list it - * combines by key (`rulesets[main].bypass_actors: null`). Every other list is - * data the merge copies as written, so a null inside one stays for the - * validator to judge (`branches[].protection: null`, a null list element). A - * knobbed section is read in either of its forms, the plain list or the - * wrapper. Returns a fresh document; a cyclic input yields a cyclic clone, - * finite like the input (the merge is what refuses those). + * A layer validated on its own is seen as the merge could leave it: every null the merge would read as a marker drops, + * every other null stays for the validator to judge. A cyclic input yields a cyclic clone; the merge is what refuses those. + * + * `rulesets[main].bypass_actors: null` -> dropped (a mapping key inside a keyed list the merge combines) + * `branches[].protection: null` -> kept (inside a list the merge copies as written) + * a null list element -> kept */ export function stripNulls(doc: unknown): unknown { if (!isPlainObject(doc)) { @@ -209,59 +171,44 @@ export function stripNulls(doc: unknown): unknown { type Refusal = DistributiveOmit; /** - * A refusal at `site` of `layer`. INVARIANT, pinned by the marker test in - * test/engine/layers.test.ts: `actual` is the only document value a refusal - * carries (describeProblem describes it by shape); `keyField` is the module's - * declared key field; `site` is built from section keys, entry indices, - * module-declared field names, LAYERING_KEY, and the fixed phrase "the - * document". A document key or value never enters the prose. + * No document key or value ever enters a refusal's prose; the marker test in test/engine/layers.test.ts pins it. + * + * `actual` -> the only document value carried, and describeProblem describes it by shape + * `keyField` -> the module's declared key field + * `site` -> section keys, entry indices, module-declared field names, LAYERING_KEY, "the document" */ function refuse(layer: string, site: string, refusal: Refusal): Result { return err({ layer, site, ...refusal }); } -/** - * An opt-out notice's prose; value-free under the same invariant as the - * refusals (mode: merge has no redaction context, so no document value may - * reach a log through the merge). - */ +/** Value-free under the refusals' invariant: mode: merge has no redaction context, so no document value may reach a log through the merge. */ export function describeOptOut(notice: OptOutNotice): string { return `${notice.layer}: null removed ${notice.path} declared by a lower layer`; } -/** The fold state one layer's step reads and reports into. */ interface Step { readonly layer: string; readonly notices: OptOutNotice[]; } -/** A knobbed section of one admitted layer, ready to combine. */ interface AdmittedSection { - /** The wrapper's keys besides `entries` and `_layering` (`_undeclared`, or a typo for validation). */ + /** The wrapper's keys besides `entries` and `_layering`: `_undeclared`, or a typo kept for validation to name. */ readonly knobs: Readonly>; readonly entries: readonly Readonly>[]; - /** How this section combines with the layers below it in this step. */ readonly layering: Layering; - /** The section module's keyed layering, absent for a section that always replaces. */ readonly keyed: KeyedListLayering | undefined; } -/** One layer past the boundary: every knobbed section checked and typed, the rest as written. */ interface AdmittedLayer { readonly name: string; readonly doc: Readonly>; readonly sections: ReadonlyMap; } -/** The entries as mappings, or null when one is not. */ function asMappings(list: readonly unknown[]): readonly Readonly>[] | null { return list.every(isPlainObject) ? list : null; } -/** - * The entries as mappings, or a refusal naming the first that is not, by its - * index under `path` and its shape. - */ function admitEntries( layer: string, path: string, @@ -279,14 +226,7 @@ function admitEntries( }); } -/** - * Refuse a keyed list a layer declares with an entry that carries no key or - * two entries claiming one key (a label renaming into a sibling's name), at - * any nesting the declaration names; each entry's nested keyed lists are - * checked the same way. Past this check, every entry of the list has keys - * and no two claim one, so a layer's entries never collide among themselves - * in the union. - */ +/** Two entries of one layer claiming a key (a label renaming into a sibling's name) are refused here, so unionKeyed never meets them. */ function checkKeyed( layer: string, entries: readonly Readonly>[], @@ -328,10 +268,6 @@ function checkKeyed( return ok(); } -/** - * The top-level `_layering` of a document, validated; undefined when absent. - * Consumed here, so it never reaches the merged document. - */ function fileLayering( layer: string, doc: Readonly>, @@ -346,7 +282,6 @@ function fileLayering( return ok(value); } -/** One knobbed section past the boundary, or a refusal naming the layer and the section. */ function admitSection( layer: string, key: UndeclaredPolicySection, @@ -382,11 +317,8 @@ function admitSection( } /** - * Whether a node refers back into itself through lists and plain mappings (a - * YAML anchor aliased inside its own node). A node aliased twice without - * enclosing itself is a tree to the merge, which clones it per site, so only - * a node on the current descent counts; a node fully walked without one is - * never entered again. + * Only a node on the current descent counts: a node aliased twice without enclosing itself is a tree to the merge, which + * clones it per site. `walked` keeps a fully walked node from being entered again. */ function hasCycle(value: unknown, descent: WeakSet, walked: WeakSet): boolean { if (!Array.isArray(value) && !isPlainObject(value)) { @@ -407,10 +339,8 @@ function hasCycle(value: unknown, descent: WeakSet, walked: WeakSet { if (hasCycle(layer.doc, new WeakSet(), new WeakSet())) { @@ -456,11 +386,6 @@ function put(record: Record, key: string, value: unknown): void }); } -/** - * A higher `null`: deletes a lower declaration and says so, or stays as - * written when nothing below declares the key. A lower null is not a - * declaration, so nulling it again is not an opt-out and earns no notice. - */ function applyNull(out: Record, key: string, path: string, step: Step): void { const lower = own(out, key); if (lower !== undefined && lower !== null) { @@ -471,10 +396,6 @@ function applyNull(out: Record, key: string, path: string, step put(out, key, null); } -/** - * A higher value over a lower one: mappings merge key by key (with `keyed` - * naming the nested keyed lists, see mergeMappings), anything else replaces. - */ function mergeValue( below: unknown, above: unknown, @@ -488,11 +409,6 @@ function mergeValue( return structuredClone(above); } -/** - * Key-by-key merge of two mappings; `keyed` names the fields whose arrays on - * both sides are keyed lists (a merged ruleset's `rules`). Lower key order is - * preserved and higher-only keys follow. - */ function mergeMappings( below: Readonly>, above: Readonly>, @@ -521,11 +437,7 @@ function mergeMappings( return out; } -/** - * A higher entry's place in the union: at the slot of the first lower entry it - * matches, or appended. `index` is its position in the higher list, which is - * how the layer's notices name it. - */ +/** `index` is the entry's position in the higher list, which is how the layer's notices name it. */ type Placement = | { readonly item: Readonly>; @@ -536,17 +448,9 @@ type Placement = | { readonly item: unknown; readonly slot: undefined }; /** - * The keyed union. A higher entry supersedes every lower entry it matches - * (their key sets intersect) and stands where the first of them stood: under - * "replace" as written, under "merge" merged key by key into that first one, - * with its own nested keyed lists. Unmatched lower entries keep their order; - * higher-only entries append in theirs. Matching reads the lower list as it - * stood before this layer, so which entries result does not depend on the - * higher entries' order (only their order within one slot does): a lower - * entry two higher entries claim between them is superseded by both, and - * since the boundary keeps a layer's entries key-disjoint, the result is one - * the section's planner accepts. Total over whatever the lists hold: an item - * without keys pairs with nothing. + * Matching reads the lower list as it stood before this layer, so which entries result does not depend on the higher + * entries' order; a lower entry two higher entries claim is superseded by both, and checkKeyed's key-disjointness keeps + * the result one the section's planner accepts. */ function unionKeyed( lower: readonly unknown[], @@ -590,12 +494,6 @@ function unionKeyed( return out; } -/** - * One knobbed section over the accumulated document: the knobs merge key by - * key (an omitted `_undeclared` inherits the lower one), the entries combine - * per the section's layering in this step. A lower value that is not a - * wrapper (absent, or a null that stayed as written) declares nothing. - */ function mergeSection( key: string, lower: unknown, @@ -613,10 +511,8 @@ function mergeSection( return out; } -/** One fold step: the admitted layer over the accumulated document. */ function mergeStep(acc: unknown, layer: AdmittedLayer, notices: OptOutNotice[]): unknown { const step: Step = { layer: layer.name, notices }; - // A non-mapping below cannot be merged into; the mapping replaces it. const below = isPlainObject(acc) ? acc : {}; const out: Record = { ...below }; for (const [key, value] of Object.entries(layer.doc)) { @@ -640,15 +536,6 @@ function mergeStep(acc: unknown, layer: AdmittedLayer, notices: OptOutNotice[]): return out; } -/** - * Fold the layers low to high into one settings document (see the module - * header for the dialect). `layering` is the run's default for the knobbed - * sections; a layer overrides it per section with the wrapper's `_layering`, - * or for the whole document with a top-level one, both consumed here. The - * result carries every knobbed section resolved to an explicit policy and - * never a `_layering` key. Inputs are never mutated; a layer the boundary - * refuses comes back as the problem naming it. - */ export function mergeLayers( layers: readonly Layer[], options: { readonly layering: Layering }, diff --git a/src/engine/orchestrate.ts b/src/engine/orchestrate.ts index 5daadbe32..0475ad13e 100644 --- a/src/engine/orchestrate.ts +++ b/src/engine/orchestrate.ts @@ -1,7 +1,6 @@ /** - * Per-repository orchestration: the section pipeline (active-section filter, preflight barrier, - * section loop) that drives one repository, so the single- and multi-repo flows share one engine. - * All output goes through the Io sink; callers decide how (or whether) to tag lines per repository. + * The per-repository pipeline (active-section filter, preflight barrier, section loop) the single- and multi-repo flows + * share. All output goes through the Io sink; callers decide how to tag lines per repository. */ import { err, type Result } from "neverthrow"; @@ -26,12 +25,8 @@ import type { SectionSelection } from "./section-selection.js"; import { validateSectionShapes } from "./validate.js"; /** - * One section's end state, discriminated on `status` so an HTTP code can - * only exist where it means something: `httpStatus` is the safe code of the - * PermissionDenied behind a failed/skipped section (the redacted view - * surfaces it as `HTTP 403` in place of the hidden detail), and the - * `?: never` pin on the healthy arm makes a code on an applied/clean row - * unrepresentable instead of merely filtered out. + * `httpStatus` is the safe code of the PermissionDenied behind a failed or skipped section (the redacted view shows it + * as `HTTP 403` in place of the hidden detail); the `?: never` pin makes a code on a healthy row unrepresentable. */ export type SectionOutcome = | { @@ -49,46 +44,28 @@ export type SectionOutcome = }; /** - * A settings document that passed validateSettingsDoc, and nothing else: the - * brand has exactly one construction site (the success return below), so a - * RepoRunOptions built from an unvalidated document is a compile error - the - * "validate before run" ordering is carried by the type, not by call-site - * discipline. The value is the PARSED document validateSectionShapes built - * from zod's output, never the caller's object. + * The brand has exactly one construction site (validateSettingsDoc's success return), so a RepoRunOptions built from an + * unvalidated document is a compile error. The value is the PARSED document zod built, never the caller's object. */ declare const validatedSettings: unique symbol; export type ValidatedSettings = SettingsFile & { readonly [validatedSettings]: true }; export interface RepoRunOptions { - /** The target repository, parsed at the caller's validated boundary. */ repo: RepoRef; settings: ValidatedSettings; mode: "apply" | "check"; onMissingPermission: "fail" | "warn"; - /** Which sections run and which must fully apply, validated together at construction. */ sections: SectionSelection; - /** - * Who authored the settings document. Omitted, "operator" (the single-repo - * settings file, central files, and the defaults document are all - * operator-authored). The multi-repo flow passes "target" for a target - * repository's own settings.yml, so its secret references are refused. - */ + /** Omitted, "operator". The multi-repo flow passes "target" for a target's own settings.yml, so its secret references are refused. */ secretSource?: SettingsSource; - /** - * The environment secret references resolve from in apply mode. Tests - * inject a record; production omits it and process.env applies. - */ secretEnv?: Record; } export type RepoResult = "applied" | "partial" | "clean" | "drift" | "failed" | "skipped"; /** - * Every RepoResult value, in the worst-first ranking worstOf() applies. - * The single source for the aggregate ranking and for the action.yml - * `result` output docs (the contract test imports this). The satisfies - * clause and the exhaustiveness check below keep it locked to RepoResult: - * a new result value that is not listed here fails to compile. + * Worst-first, the ranking worstOf() applies; the single source for the action.yml `result` output docs too (the + * contract test imports it). The lockstep below keeps it locked to RepoResult. */ export const REPO_RESULTS = [ "failed", @@ -99,7 +76,6 @@ export const REPO_RESULTS = [ "clean", ] as const satisfies readonly RepoResult[]; -/** Compile-time lockstep: a RepoResult value missing from REPO_RESULTS fails here. */ type _UnlistedResult = MustBeNever>; export interface RepoRunResult { @@ -110,7 +86,6 @@ export interface RepoRunResult { preflightDenied: string[]; } -/** The keys of the skipped outcomes, derived at the read site. */ export function skippedSectionKeys( outcomes: ReadonlyArray>, ): SectionKey[] { @@ -118,12 +93,8 @@ export function skippedSectionKeys( } /** - * Top-level shape validation for one settings document (the unknown-key - * policy from run()): the ONE boundary that turns a raw parsed document into - * a ValidatedSettings the engine will accept. Returns the branded document, - * or the problem (caller fails the run or the repo); the sections-allowlist - * case downgrades to a warning. `sourceLabel` names the file the problem - * points at. + * The ONE boundary that turns a raw parsed document into the ValidatedSettings the engine accepts. Unknown top-level + * keys are errors, except outside a non-empty `sections` allowlist, where they downgrade to a warning. */ export function validateSettingsDoc( settings: unknown, @@ -138,23 +109,16 @@ export function validateSettingsDoc( shape: nonMappingShape(settings), }); } - // Only a PLAIN mapping may pass: an explicit YAML tag (!!timestamp, !!set, - // !!binary) parses to a Date/Set/Uint8Array, which is an object with no - // meaningful keys - branding it valid would turn the document into a - // silent green no-op. The same prototype rule requirePlainMapping applies - // to section values. + // A YAML tag (!!timestamp, !!set, !!binary) parses to a Date, Set, or Uint8Array: an object with no meaningful keys, + // which branded valid would turn the document into a silent green no-op. const proto = Object.getPrototypeOf(settings); if (proto !== Object.prototype && proto !== null) { return err({ code: "settings-not-plain-mapping", source: sourceLabel }); } const knownSections = new Set(SECTION_KEYS); - // The allowlist holds SectionKeys, but the DOCUMENT's unknown keys are - // arbitrary strings; the widened view keeps the lookup honest without a - // cast per key. const allowed: ReadonlySet = onlySections; - // A misspelled section silently doing nothing would violate the loud- - // failure promise; unknown top-level keys are hard errors (prefix custom - // keys with underscore to keep private notes in the file). + // A misspelled section silently doing nothing would break the loud-failure promise; underscore-prefixed keys are the + // sanctioned place for private notes. const unknownKeys = Object.keys(settings).filter( (key) => !knownSections.has(key) && !key.startsWith("_"), ); @@ -167,17 +131,12 @@ export function validateSettingsDoc( known: SECTION_KEYS, }); } - // A `sections` allowlist lets an older action version coexist with a - // config written for a newer one: unknown keys OUTSIDE the allowlist - // are warnings, not errors. + // A `sections` allowlist lets an older action version coexist with a config written for a newer one. io.annotate( "warning", `ignoring unknown top-level section(s) outside the "sections" allowlist: ${unknownKeys.join(", ")}. Upgrade the action to a version that knows them, or remove them from ${sourceLabel}`, ); } - // The one place the brand is minted: everything above proved the document - // is a mapping of known (or allowlist-tolerated/underscored) sections, and - // the parsed document holds exactly the known ones as their shapes' output. return validateSectionShapes(settings as Record, sourceLabel).map( (parsed) => parsed as ValidatedSettings, ); @@ -192,11 +151,7 @@ function nonMappingShape(value: unknown): TopLevelShape { return kind === "object" ? "null" : kind; } -/** - * Probe every active section by planning it and collect the permission denials as "key: detail" - * lines; empty means every section is accessible. A plan section has no write capability, so - * planning IS the read-only probe and the plan is discarded. `active` is injectable for tests. - */ +/** A plan section has no write capability, so planning IS the read-only probe; `active` is injectable for tests. */ export async function preflightProbe( api: GithubClient, repo: RepoRef, @@ -207,9 +162,7 @@ export async function preflightProbe( for (const section of active) { const declared = settings[section.key]; if (declared === undefined) { - // `active` is filtered to declared sections, so this can only fire on - // a caller bug (the parameter is injectable for tests); probing - // nothing silently would make such a test pass vacuously. + // `active` is filtered to declared sections; probing nothing silently would let an injected test list pass vacuously. throw new Error( `BUG: preflightProbe was given section "${section.key}" but the settings document does not declare it; the active list must be filtered to declared sections`, ); @@ -220,14 +173,12 @@ export async function preflightProbe( if (error instanceof PermissionDenied) { denied.push(`${section.key}: ${error.detail}`); } - // Other preflight errors are ignored here; the section loop surfaces - // them with full context. + // Other preflight errors are left for the section loop, which surfaces them with full context. } } return denied; } -/** Run the full section pipeline against one repository. */ export async function runForRepo( api: GithubClient, opts: RepoRunOptions, @@ -237,13 +188,10 @@ export async function runForRepo( const repo = opts.repo; const settings = opts.settings; - // The ONE statement of what runs: a section is absent (not declared), - // excluded (declared but outside a non-empty `sections` allowlist), or - // active. The preflight filter and the section loop below both read this, - // so the two can never disagree about which sections are live. + // The ONE statement of what runs: the preflight filter and the section loop both read it, so they can never disagree. const disposition = (key: SectionKey): "absent" | "excluded" | "active" => { if (settings[key] === undefined) { - return "absent"; // declared-keys-only: absent section = untouched + return "absent"; } if (opts.sections.only.size > 0 && !opts.sections.only.has(key)) { return "excluded"; @@ -252,11 +200,8 @@ export async function runForRepo( }; const active = SECTIONS.filter((section) => disposition(section.key) === "active"); - // Secret references, phase (a): collect every declared secret-field value - // from the ACTIVE sections (a section excluded by `sections` never runs, so - // its references must not fail the run) and validate syntax and provenance - // in BOTH modes, before the preflight barrier. No environment is read here: - // check mode and preflight see syntax only. + // Secret references are collected from the ACTIVE sections only (an excluded section's references must not fail the + // run) and their syntax and provenance checked in BOTH modes, before the preflight barrier and with no environment read. const secretValues = collectSecretValues(settings, active, opts.secretSource ?? "operator"); const secretFailure = (errorsBySection: Map): RepoRunResult => { const outcomes: SectionOutcome[] = []; @@ -289,12 +234,9 @@ export async function runForRepo( return secretFailure(syntaxErrors); } - // Preflight barrier: the API has no transactions, so a mid-apply - // permission failure would leave settings half-applied. Under the strict - // policy, probe every declared section read-only FIRST and refuse to - // write anything when any of them is inaccessible. (A token with read - // but not write access can still fail mid-apply; the engine is - // idempotent, so re-running after fixing the token converges.) + // The API has no transactions, so a mid-apply permission failure would leave settings half-applied: under the strict + // policy every active section is probed read-only FIRST. A token with read but not write access can still fail + // mid-apply; the engine is idempotent, so re-running after fixing the token converges. if (!check && opts.onMissingPermission === "fail") { const denied = await preflightProbe(api, repo, active, settings); if (denied.length > 0) { @@ -310,9 +252,8 @@ export async function runForRepo( } } - // Apply resolves EVERY secret reference up front, after the read-only preflight and before the - // first mutation, and masks each plaintext before the resolver exists; check mode executes - // nothing and builds no tools. An empty map in apply proves no legitimate lookup exists. + // Apply resolves EVERY secret reference after the read-only preflight and before the first mutation, and masks each + // plaintext before the resolver exists; check mode builds no tools. An empty map in apply proves no legitimate lookup exists. let tools: ExecTools | null = null; if (!check) { const resolved: Record = {}; @@ -374,9 +315,8 @@ export async function runForRepo( } const desired = settings[section.key]; if (desired === undefined) { - // disposition() already classified this section "active", which - // requires a declared value; reaching here is an engine bug, and - // planning on undefined would violate plan()'s SectionInput contract. + // disposition() classified this section active, which requires a declared value; planning on undefined would + // break plan()'s SectionInput contract. throw new Error( `BUG: section "${section.key}" was classified active but the settings document does not declare it`, ); @@ -384,28 +324,22 @@ export async function runForRepo( let result: | { check: true; drift: string[]; notes: string[] } | { check: false; changes: string[]; notes: string[] }; - // What a plan section produced before an operation failed, reported with - // the failure instead of vanishing. `landed` counts accepted requests: a - // change thunk can fail after its request landed, so lines undercount. + // What the section produced before an operation failed, reported with the failure instead of vanishing. `landed` + // counts accepted requests: a change thunk can fail after its request landed, so the lines cannot stand in for it. let produced: { notes: readonly string[]; changes: readonly string[]; landed: number } = { notes: [], changes: [], landed: 0, }; try { - // plan() runs in both modes over the read port; the mode decides - // what the plan becomes (drift lines plus the cannot-verify notes, or - // executed changes), and op-less drift surfaces as apply notes. const plan = await section.plan(planContext(section, api, repo), desired); if (tools === null) { result = { check: true, drift: planDrift(plan), notes: planCheckNotes(plan) }; } else { const execution = await executePlan(plan, section, api, repo, tools); - // The execution's notes are the tolerated operations' outcomes. const notes = [...plan.notes, ...plan.drift, ...execution.notes]; produced = { notes, changes: execution.changes, landed: execution.landed }; if (execution.status === "failed") { - // Already classified by the request helpers; the catch below reports it. throw execution.error; } result = { check: false, changes: [...execution.changes], notes }; @@ -420,8 +354,7 @@ export async function runForRepo( const before = [...produced.notes, ...produced.changes]; if (error instanceof PermissionDenied) { const required = opts.sections.required.has(section.key); - // A denial after some operations landed is a partial mutation, never - // a skip: the warn policy applies only when nothing was written. + // A denial after some operations landed is a partial mutation, never a skip: the warn policy applies only when nothing was written. const landed = produced.landed; if (opts.onMissingPermission === "warn" && !required && landed === 0) { io.annotate("warning", `${section.key}: skipped - ${error.detail}`); @@ -453,8 +386,7 @@ export async function runForRepo( failed = true; continue; } - // throwFor()-raised errors already carry section, cause, and fix; - // prefix anything else so the failing section is still named. A landed + // throwFor()-raised errors already carry section, cause, and fix; anything else gets the section prefixed. A landed // request is a real mutation with or without its line, so say so. const message = error instanceof Error ? error.message : String(error); const prefixed = message.startsWith(`${section.key}:`) @@ -472,8 +404,6 @@ export async function runForRepo( for (const note of result.notes) { io.annotate("notice", `${section.key}: ${note}`); } - // Narrowing on the result's own discriminant lets each branch read only - // the list its mode can carry: a check-mode result has no changes to misreport. if (result.check) { if (result.drift.length > 0) { drifted = true; @@ -491,9 +421,8 @@ export async function runForRepo( outcomes.push({ key: section.key, status: "applied", - // A section that changed nothing but left notes (a tolerated 409, a - // personal-account skip) is NOT "already in the desired state"; show - // the notes instead of claiming no changes were needed. + // A section that changed nothing but left notes (a tolerated 409, a personal-account skip) is NOT "already in + // the desired state"; the notes are shown instead of claiming no changes were needed. detail: result.changes.length > 0 ? result.changes @@ -524,7 +453,6 @@ export async function runForRepo( }; } -/** Aggregate result across targets: the worst outcome wins. */ export function worstOf(results: Array<{ result: RepoResult }>, check: boolean): RepoResult { for (const rank of REPO_RESULTS) { if (results.some((r) => r.result === rank)) { diff --git a/src/engine/secret-refs.ts b/src/engine/secret-refs.ts index e664e4822..98c859baa 100644 --- a/src/engine/secret-refs.ts +++ b/src/engine/secret-refs.ts @@ -1,25 +1,15 @@ /** - * $NAME secret references: how secret values stay out of committed settings - * files. settings.yml is plaintext in git and GitHub does not interpolate - * ${{ secrets }} inside repository files, so a designated secret field - * carries a whole-value `$NAME` reference resolved from the action step's - * environment at run time. Every edge fails closed: a literal is rejected - * (committed plaintext is exactly what the mechanism prevents), a - * near-reference like "prefix-$TOKEN" is rejected (shipping it as a literal - * secret is worse than failing), reserved runner variables are refused, and - * a reference in a target-fetched settings file is refused (a target - * repository must not route the operator's environment into itself). + * settings.yml is committed plaintext and GitHub does not interpolate ${{ secrets }} inside repository files, so a + * designated secret field carries a whole-value `$NAME` reference resolved from the step's environment at run time. + * Every edge fails closed; nothing here logs a value, and the module knows no field names. * - * The module knows no field names; future consumer sections call these - * functions per designated secret field. Nothing here logs a value. + * a literal value -> rejected: committed plaintext is what the mechanism prevents + * "prefix-$TOKEN" -> rejected: shipping it as a literal secret is worse than failing + * $INPUT_*, $GITHUB_*, ... -> refused: reserved runner variables + * a reference in a target's file -> refused: a target must not route the operator's environment into itself */ -/** - * Who authored the settings source a value came from. References are honored - * only in operator-owned sources; in a `target` source (a settings file - * fetched from the target repository itself) a reference is a hard error. - * Integration decides which files are which. - */ +/** Who authored the source: a `target` document (fetched from the target repository) has its references refused; flows/multi.ts decides. */ export type SettingsSource = "operator" | "target"; /** A syntactically valid reference: the env var name, without the `$`. */ @@ -29,29 +19,20 @@ interface SecretRef { export type SecretRefCheck = { ok: true; ref: SecretRef } | { ok: false; error: string }; -/** Whole-value reference shape: `$NAME` and nothing else. */ const REFERENCE_RE = /^\$[A-Z_][A-Z0-9_]*$/; -/** A reference-looking fragment anywhere in a value that is NOT a whole-value reference. */ const EMBEDDED_REFERENCE_RE = /\$[A-Z_][A-Z0-9_]*/; /** - * Variable prefixes a reference may never name. INPUT_* are the action's own - * inputs (INPUT_TOKEN is the admin token); GITHUB_*, ACTIONS_*, RUNNER_* and - * NODE_* are runner and workflow context. Routing any of them into a settings - * value would turn a settings file into an exfiltration channel. + * INPUT_* holds the action's own inputs (INPUT_TOKEN is the admin token); the rest are runner and workflow context. + * Routing any of them into a settings value would make the settings file an exfiltration channel. */ export const RESERVED_REF_PREFIXES = ["INPUT_", "GITHUB_", "ACTIONS_", "RUNNER_", "NODE_"] as const; /** - * Phase (a): syntax and policy validation for one designated secret field's - * value. Takes no environment and reads no values, so check mode and - * preflight can run it without touching secrets. `label` names the OWNING - * ENTRY (a secret name, a webhook url - configuration, never a secret), so - * the error points at the offending entry among many. Error strings name - * the rule, the label, and the reference, and never echo a non-reference - * value: a rejected literal or the text around an embedded `$NAME` may - * already be a secret. + * Reads no environment, so check mode and preflight run it without touching secrets; `label` names the owning entry + * (a secret name, a webhook url), never a value. Errors never echo a non-reference value either: a rejected literal, + * or the text around an embedded `$NAME`, may already be a secret. */ export function validateSecretRef( value: string, @@ -99,33 +80,22 @@ export type SecretRefsResolution = | { ok: false; errors: string[] }; /** - * One designated secret field's value, tagged with the provenance of the - * DOCUMENT that declared it. Provenance is decided once, where the document - * is chosen (flows/multi.ts readTargetSettings): a target-fetched document - * is "target", so its references are refused, and every other document is - * "operator", so its references resolve. + * One secret field's value with the provenance of the DOCUMENT that declared it, decided once where the document is + * chosen (flows/multi.ts readTargetSettings). */ export interface SourcedSecretValue { readonly value: string; - /** - * Names the declared entry the value belongs to, for error messages: a - * secret name, an environment-plus-secret pair, a webhook url - always - * configuration the settings file already spells, never a value. - */ + /** The owning entry as the settings file spells it, never a value. */ readonly label: string; readonly source: SettingsSource; } /** - * Phase (b): resolve every reference up front from the given environment, - * before any of them is used. Each value re-runs the full phase (a) - * validation with ITS OWN source, so a mixed batch cannot launder a - * target-declared reference behind operator-declared ones. All problems are - * collected, not just the - * first: a run with three broken references should say so once. An UNSET - * variable and a SET-BUT-EMPTY variable both fail - an empty vault lookup - * must not write an empty secret. Callers pass `process.env` at the edge and - * an injected record in tests. + * Every value re-runs validateSecretRef with ITS OWN source, so a mixed batch cannot launder a target-declared reference + * behind operator-declared ones. All problems are collected: a run with three broken references says so once. + * + * unset variable -> fails + * set but empty variable -> fails too: an empty vault lookup must not write an empty secret */ export function resolveSecretRefs( values: readonly SourcedSecretValue[], diff --git a/src/engine/secrets.ts b/src/engine/secrets.ts index 155ba4607..6d3d18d68 100644 --- a/src/engine/secrets.ts +++ b/src/engine/secrets.ts @@ -1,34 +1,21 @@ /** - * Engine-side collection of declared secret-field values. Sections declare - * WHICH of their fields are secrets (SectionModule.secretValues); this module - * walks the active sections of one settings document and pairs each raw - * value with the document's provenance, so orchestrate.ts can validate and - * resolve every reference in one place - before any section runs. + * Pairs each declared secret-field value (SectionModule.secretValues) with the document's provenance, so orchestrate.ts + * validates every reference before any section runs and, in apply, resolves them all before the first write. + * Provenance is one value per DOCUMENT: flows/multi.ts decides it where the document is chosen, and the single-repo + * flow takes the "operator" default. * - * Provenance is one value per DOCUMENT: operator- and target-authored values - * never share a document. A target repository's own settings.yml is - * target-authored; everything else - the single-repo settings file (and the - * operator layers merge mode folds into it), a central per-repo file, and the - * defaults document applied whole to a target that has no file of its own - - * is operator-authored. The multi-repo flow decides the value where it - * chooses the document (flows/multi.ts); the single-repo flow takes the - * engine's "operator" default. + * a target repository's own settings.yml -> target + * the single-repo file, a central file, the defaults document -> operator */ import type { SectionKey, SettingsFile } from "../schema.js"; import type { SectionModule } from "../sections/contract/module.js"; import type { SettingsSource, SourcedSecretValue } from "./secret-refs.js"; -/** One declared secret value, tagged with the section that declared it. */ export interface SectionSecretValue extends SourcedSecretValue { section: SectionKey; } -/** - * Every declared secret-field value across the given sections, each tagged - * with its owning section and the document's provenance. Sections without a - * secretValues declaration contribute nothing. - */ export function collectSecretValues( settings: SettingsFile, sections: readonly SectionModule[], diff --git a/src/engine/validate.ts b/src/engine/validate.ts index 6a2d92d17..0737f4ff6 100644 --- a/src/engine/validate.ts +++ b/src/engine/validate.ts @@ -1,8 +1,4 @@ -/** - * Shape validation for one settings document against each section's loose - * zod shape (sections//index.ts); the parsed output is what the engine - * applies. Closed sections and strict nested shapes reject unknown keys here. - */ +/** Shape validation against each section's loose zod shape; the parsed output, not the input, is what the engine applies. */ import { err, ok, type Result } from "neverthrow"; import { nonPlainKind } from "../plain-data.js"; @@ -11,17 +7,10 @@ import { SECTION_KEYS, type SettingsFile } from "../schema.js"; import { sectionModule, sectionShape } from "../sections/registry.js"; /** - * The first non-plain object anywhere in a declared value, as "path (kind)" - * prose - or null when the value is plain JSON data throughout. YAML's - * explicit tags (!!timestamp, !!set, !!binary) parse to Date/Set/Uint8Array, - * which zod's object schemas accept as empty mappings, so a tagged value - * nested ANYWHERE (actions.cache, a pages mapping, a future section) would - * otherwise validate and then silently configure nothing - or die later at - * the request boundary with less context. One walk here covers every - * section, present and future, instead of a per-shape guard that each new - * mapping must remember (requirePlainMapping remains the shape-level belt - * for the sections that wear it). `seen` breaks YAML anchor cycles: a - * cyclic document is not endorsed, but the validator must not hang on one. + * zod's object schemas accept a Date, Set, or Uint8Array (YAML !!timestamp, !!set, !!binary) as an empty mapping, so a + * tagged value where a mapping is expected (actions.cache, a pages mapping) would validate and then silently configure + * nothing. One walk here covers every section instead of a guard each new mapping must remember; `seen` keeps a YAML + * anchor cycle from hanging it. */ function findNonPlain(value: unknown, path: string, seen: WeakSet): string | null { if (value === null || typeof value !== "object") { @@ -53,11 +42,7 @@ function findNonPlain(value: unknown, path: string, seen: WeakSet): stri return null; } -/** - * Validate the declared sections' shapes. Returns the parsed document (the - * declared sections, each as zod's output: fresh plain objects at every node - * the shape describes) or the problem naming the source file and what to fix. - */ +/** The result is zod's output (fresh plain objects at every node the shape describes), never the caller's document. */ export function validateSectionShapes( settings: Record, sourceLabel: string, @@ -69,9 +54,7 @@ export function validateSectionShapes( if (declared === undefined) { continue; } - // Plain-data gate first: zod object schemas accept a Date or Set as an - // empty mapping, so the tagged-value rejection must not depend on any - // shape. + // Before the shape parse: zod would accept the tagged value as an empty mapping and never report it. const nonPlain = findNonPlain(declared, key, new WeakSet()); if (nonPlain !== null) { problems.push(nonPlain); @@ -87,9 +70,7 @@ export function validateSectionShapes( problems.push(`${key}${path}: ${issue.message}`); } if (issues.length > 5) { - // The cap keeps the message readable, but a silently truncated list - // would cost one fix-and-rerun cycle per hidden offender - say how - // many more there are. + // A silently truncated list costs one fix-and-rerun cycle per hidden offender. problems.push(`${key}: ...and ${issues.length - 5} more issue(s) in this section`); } continue; @@ -103,19 +84,9 @@ export function validateSectionShapes( return err({ code: "settings-malformed-sections", source: sourceLabel, issues: problems }); } -/** - * Unrecognized entry keys in a closed section, capped at 5 like the shape - * issues above. Runs only after the shape parse succeeded; entries that are - * not objects are skipped (for the current closed shapes the parse already - * excludes them, so the guard is only defensive). A knobbed section's - * wrapped `{_undeclared, entries}` form is unwrapped first, so a closed - * section that also takes the policy knob (collaborators) keeps its entry - * checks in both forms - the wrapper's own keys are validated by the - * strictObject in the section shape, never here. - */ +/** Only the entries are checked here, in either form; the wrapper's own keys are the section shape's strictObject to judge. */ function closedSurfaceProblems(key: (typeof SECTION_KEYS)[number], declared: unknown): string[] { - // The registry's generic view erases the per-section entry typing (the same - // erasure sectionShape accepts), so the declaration is re-widened here. + // The registry's generic view erases the per-section entry typing, so the declaration is re-widened here. const closed = sectionModule(key).closedSurface as | { known: Readonly>; @@ -136,7 +107,6 @@ function closedSurfaceProblems(key: (typeof SECTION_KEYS)[number], declared: unk if (entries === null) { return []; } - // The declaration's key order is the order the error prose lists. const knownKeys = Object.keys(closed.known); const known = new Set(knownKeys); const problems: string[] = []; @@ -154,8 +124,6 @@ function closedSurfaceProblems(key: (typeof SECTION_KEYS)[number], declared: unk } } if (problems.length > 5) { - // Same cap-with-a-count posture as the shape issues above: readable, but - // never silently incomplete. return [ ...problems.slice(0, 5), `${key}: ...and ${problems.length - 5} more entr${problems.length - 5 === 1 ? "y" : "ies"} with unrecognized keys in this section`, diff --git a/src/flows/deliver.ts b/src/flows/deliver.ts index d440d2fa4..b525e74c8 100644 --- a/src/flows/deliver.ts +++ b/src/flows/deliver.ts @@ -1,7 +1,6 @@ /** - * Where every run ends, single- or multi-repo. Each target closes its channel - * and delivers here, and the run's summary, outputs, and exit code are - * decided here, so the two flows cannot drift in what they report. + * Where every run ends. Each target closes its channel and delivers here, and the summary, outputs, and exit code are + * decided here, so the single- and multi-repo flows cannot drift in what they report. */ import { err, ok, type Result } from "neverthrow"; @@ -39,7 +38,6 @@ import { } from "./redact.js"; import { writeMergeSummary, writeMultiSummary, writeSummary } from "./summary.js"; -/** One target's end state before its channel closes: the engine result and the rich detail. */ export interface TargetResult { result: RepoResult; outcomes: SectionOutcome[]; @@ -47,25 +45,17 @@ export interface TargetResult { note?: string; } -/** A target that produced no outcomes because `message` stopped it. */ export function failedTarget(message: string): TargetResult { return { result: "failed", outcomes: [], note: message }; } -/** - * Record a failure that happened before the engine ran; the rich message goes - * to the target's channel (public in the clear, captured when redacted). - */ +/** A failure before the engine ran; the rich message goes to the target's channel (public in the clear, captured when redacted). */ export function targetFailure(channelIo: Io, richMessage: string): TargetResult { channelIo.annotate("error", richMessage); return failedTarget(richMessage); } -/** - * The engine's result as the target's end state. A preflight denial refused - * to write anything; its one line goes through the channel and its note heads - * the target's (otherwise empty) summary. - */ +/** A preflight denial refused to write anything; its one line goes through the channel and its note heads the target's otherwise empty summary. */ export function engineOutcome(run: RepoRunResult, channelIo: Io): TargetResult { const denied = run.preflightDenied.length; if (denied === 0) { @@ -83,9 +73,8 @@ export function engineOutcome(run: RepoRunResult, channelIo: Io): TargetResult { } /** - * The one outcome predicate: the `result` output is the worst target result, - * and the run exits 1 exactly when that result is failed or a check-mode - * drift. A single target's report opens under the same rule. + * The one outcome predicate: the run exits 1 exactly when the worst target result is failed or a check-mode drift. A + * single target's report opens under the same rule. */ export function runOutcome( results: ReadonlyArray<{ result: RepoResult }>, @@ -96,7 +85,6 @@ export function runOutcome( return { result, exitCode } as RunConclusion; } -/** The run inputs the delivery reads; both run flows' configs carry them. */ export interface DeliveryConfig { mode: "apply" | "check"; privateReport: PrivateReportChannel; @@ -108,7 +96,6 @@ export interface DeliveryConfig { runUrl: string; } -/** The inputs the single- and multi-repo flows share: the engine options, the redaction policy, and the delivery inputs. */ export interface RunFlowConfig extends DeliveryConfig { onMissingPermission: "fail" | "warn"; sections: SectionSelection; @@ -116,10 +103,7 @@ export interface RunFlowConfig extends DeliveryConfig { privateRepos: PrivateReposPolicy; } -/** - * The `artifact` channel cannot open without an upload port. Both flows check - * this before any API work; openReportChannel asserts the same rule. - */ +/** Both flows check this before any API work; openReportChannel asserts the same rule as its backstop. */ export function requireUploader( cfg: Pick, uploader: ArtifactUploader | undefined, @@ -129,10 +113,6 @@ export function requireUploader( : ok(); } -/** - * How a target appears in the public view: in the clear, or hidden behind - * its placeholder together with the visibility the probe resolved for it. - */ export type Exposure = { kind: "shown" } | { kind: "redacted"; visibility: RepoVisibility }; /** One target as it enters delivery; `repo` is null when the slug did not parse (the issue channel has nowhere to post). */ @@ -144,9 +124,8 @@ export interface OpenedTarget { export interface Delivery { /** - * Run `work` (told whether to inject the issue channel's marker label), then - * close the target: a redacted target proven private or internal gets its - * report, an unproven one the withheld notice, and its public line is closed-value only. + * `work` is told whether to inject the issue channel's marker label. On close, a redacted target proven private or + * internal gets its report, an unproven one the withheld notice, and its public line is closed-value only. */ target( opened: OpenedTarget, @@ -154,17 +133,13 @@ export interface Delivery { ): Promise>; } -/** - * Run `body` with the run's delivery open, then flush it even when `body` throws: - * the artifact channel uploads every accumulated report as ONE document here. - */ +/** The delivery is flushed even when `body` throws: the artifact channel uploads every accumulated report as ONE document there. */ export async function withDelivery( run: { api: GithubClient; cfg: DeliveryConfig; io: Io; uploader?: ArtifactUploader }, body: (delivery: Delivery) => Promise, ): Promise { const { api, cfg, io, uploader } = run; const check = cfg.mode === "check"; - // One timestamp for the whole run, so every target's report shares it. const meta = { adminRepo: cfg.selfSlug, runUrl: cfg.runUrl, @@ -179,9 +154,8 @@ export async function withDelivery( io, uploader, ); - // Redaction fails closed (hidden unless proven public), but delivery fails - // closed the other way: a report reaches a target only when it is proven - // private or internal, never one that might be public. + // Redaction fails closed (hidden unless proven public), but delivery fails closed the other way: a report reaches a + // target only when it is proven private or internal, never one that might be public. const deliverable = (exposure: Exposure): boolean => reports !== null && exposure.kind === "redacted" && isPrivateVisibility(exposure.visibility); const delivery: Delivery = { @@ -207,21 +181,17 @@ export async function withDelivery( try { return await body(delivery); } finally { - // The artifact's single upload deliberately follows every target's public - // line (each is redaction-safe on its own); a crash still flushes what accumulated. + // The artifact's single upload deliberately follows every target's public line (each is redaction-safe on its own); + // a crash still flushes what accumulated. await reports?.flush(); } } -/** The finished run as the two flows hand it over: one target, or every multi-repo target in order. */ export type FinishedRun = | { kind: "single"; mode: DeliveryConfig["mode"]; target: Omit } | { kind: "multi"; mode: DeliveryConfig["mode"]; targets: TargetOutcome[] }; -/** - * Every run that reached its targets ends here: summary, outputs, result line, - * exit code. The public view is projected first, so nothing below carries a redacted slug. - */ +/** The public view is projected first, so nothing below carries a redacted slug. */ export function concludeRun(io: Io, run: FinishedRun): number { if (run.kind === "single") { const view = publicDetail(run.target.detail); @@ -244,12 +214,7 @@ export function concludeRun(io: Io, run: FinishedRun): number { return conclude(io, views, run.mode === "check"); } -/** - * A run that failed before any target ran (bad inputs, an unreadable settings - * file, a fatal multi-repo setup error): the problem's line, then the - * conclusion a failed target gets. There is no summary, since nothing ran. - * The one place the action's fatal problems become text. - */ +/** A run that failed before any target ran gets a failed target's conclusion and no summary; the one place a fatal problem becomes text. */ export function failRun(io: Io, problem: Problem): number { const message = describeProblem(problem); io.annotate("error", message); @@ -257,20 +222,14 @@ export function failRun(io: Io, problem: Problem): number { return conclude(io, [failedTarget(message)], false); } -/** - * The `result` output of a mode: merge run. Not a RepoResult: a merge has no - * target, so it never enters worstOf and never appears beside the per-repo - * values. - */ +/** Not a RepoResult: a merge has no target, so it never enters worstOf and never appears beside the per-repo values. */ export const MERGE_RESULT = "merged"; -/** A finished mode: merge run as runMerge hands it over: what was folded and where it went. */ export interface FinishedMerge { layers: readonly string[]; mergedFile: string; } -/** A finished mode: merge run: the summary, the outputs, and the result line a merge earns, exit 0. */ export function concludeMerge(io: Io, run: FinishedMerge): number { writeMergeSummary(io, run.layers, run.mergedFile); io.log(`merged ${run.layers.length} layer(s) into ${run.mergedFile}`); diff --git a/src/flows/inputs.ts b/src/flows/inputs.ts index 96a7f1f62..b458747a8 100644 --- a/src/flows/inputs.ts +++ b/src/flows/inputs.ts @@ -1,10 +1,7 @@ /** - * The run's inputs: their declarations (the single source action.yml and the - * inputs reference page are generated from) and parseConfig(), which reads - * every input in a stable order through a caller-supplied port, validates it - * (each problem names the input and the fix), and returns the typed RunConfig - * the run executes - so no execution code touches raw inputs, and a CLI reads - * the same declarations the action does. + * parseConfig() validates every input read through a caller-supplied port (each problem names the input and the fix) + * into the RunConfig the run executes, so no execution code touches a raw input and a CLI reads the same declarations + * the action does. */ import { err, ok, type Result, safeTry } from "neverthrow"; @@ -38,10 +35,8 @@ export const DEFAULT_PRIVATE_REPOS = "redact" satisfies PrivateReposPolicy; const DEFAULT_PRIVATE_REPORT = "none" satisfies PrivateReportChannel; /** - * One action input: its action.yml entry (description, default) and its - * row in the generated Inputs table (summary, shown default) on the inputs - * reference page, docs/reference/inputs.md. The runner applies the - * defaults; parseConfig() falls back to them outside the runner. + * One input's action.yml entry and its row in the generated Inputs table on docs/reference/inputs.md. The runner + * applies the defaults; parseConfig() falls back to them outside the runner. */ export interface InputDecl { /** The action.yml description; the generator folds it to width. */ @@ -51,18 +46,15 @@ export interface InputDecl { /** The Inputs table's Meaning cell: the one-line gist. */ readonly summary: string; /** - * The Inputs table's Default cell when the raw default is not what a reader should - * see: an expression, a prose fallback, or the effective value the code - * supplies for an empty raw default (the discovery filters). + * The Inputs table's Default cell when the raw default is not what a reader should see: an expression, a prose + * fallback, or the effective value for an empty raw default. */ readonly shownDefault?: string; } /** - * Every input parseConfig() reads, in the order the inputs reference page - * and action.yml list them: the single source both are generated from - * (bun run build:action-docs), so adding an input here is the whole declaration. - * A new mode's inputs go beside their mode's, so declarations stay adjacent. + * The single source the inputs reference page and action.yml are generated from (bun run build:action-docs), in their + * listing order; adding an input here is the whole declaration. A new mode's inputs go beside their mode's. */ export const INPUT_DECLS = { token: { @@ -285,21 +277,19 @@ export const INPUT_DECLS = { export type InputName = keyof typeof INPUT_DECLS; -/** The port parseConfig reads inputs through: the raw value, empty when unset; parseConfig trims it. */ +/** Empty when unset; parseConfig trims, so a port need not. */ export type InputReader = (name: InputName) => string; /** - * The environment parseConfig reads beside the inputs, in process.env's shape. - * Four keys are read: GITHUB_TOKEN (the token fallback), GITHUB_REPOSITORY - * (the workflow's own repository), GITHUB_SERVER_URL and GITHUB_RUN_ID (the - * run URL). A caller outside Actions passes what it has, or nothing. + * process.env's shape; a caller outside Actions passes what it has, or nothing. + * GITHUB_TOKEN -> the token fallback + * GITHUB_REPOSITORY -> the workflow's own repository + * GITHUB_SERVER_URL, GITHUB_RUN_ID -> the run URL */ export type ConfigEnv = Readonly>; -/** The reader plus the declared-default fallback, so every helper reads one way. */ interface Inputs { readonly value: InputReader; - /** The input, or its declared default when the step (or a local run) left it unset. */ readonly orDefault: (name: InputName) => string; } @@ -309,14 +299,6 @@ function inputs(read: InputReader): Inputs { return { value, orDefault: (name) => value(name) || INPUT_DECLS[name].default }; } -/** - * Every discovery-filter input name, the single source both the FilterInput - * type and the discoveryFiltersSet scan derive from. `satisfies readonly - * (keyof DiscoveryFilters)[]` pins each entry to a real filter field, and the - * MustBeNever check below fails compilation if a DiscoveryFilters field is - * ever added without a matching input name here - the same exhaustiveness - * idiom SECTION_KEYS uses in schema.ts. - */ export const FILTER_INPUTS = [ "visibility", "archived", @@ -326,16 +308,10 @@ export const FILTER_INPUTS = [ "affiliation", ] as const satisfies readonly (keyof DiscoveryFilters)[]; -/** A discovery filter input name, a subset of the declared inputs. */ type FilterInput = (typeof FILTER_INPUTS)[number]; -/** Compile-time lockstep: a DiscoveryFilters field missing from FILTER_INPUTS fails here. */ type _UnlistedFilter = MustBeNever>; -/** - * Read an enum-valued input against the allowed list its type derives - * from, so the type, the check, and the problem's list cannot drift apart. - */ function readEnum( input: Inputs, name: InputName, @@ -354,7 +330,6 @@ function readEnum( /** What separates the entries of a list input; a single path can never contain one. */ const LIST_SEPARATOR = /[\n,]/; -/** A comma- or newline-separated list input, trimmed, empty entries dropped. */ function splitList(value: string): string[] { return value .split(LIST_SEPARATOR) @@ -362,27 +337,19 @@ function splitList(value: string): string[] { .filter(Boolean); } -/** The run kinds the `mode` input selects; apply and check run the engine, merge only folds files. */ export const MODES = ["apply", "check", "merge"] as const; export type Mode = (typeof MODES)[number]; -/** The `layering` input's values, locked to the engine's Layering type. */ const LAYERINGS = ["merge", "replace"] as const satisfies readonly Layering[]; type _UnlistedLayering = MustBeNever>; /** - * The inputs only mode: merge reads. Their declared defaults are empty so - * "explicitly set" is detectable, as with the discovery filters; apply and - * check reject a set one instead of silently ignoring it. + * Their declared defaults are empty so "explicitly set" is detectable, as with the discovery filters; apply and check + * reject a set one instead of silently ignoring it. */ const MERGE_ONLY_INPUTS = ["merged-file", "layering"] as const satisfies readonly InputName[]; -/** - * The `required-sections` and `sections` inputs as one validated selection: - * every unknown name in each is reported at once, against its own input name; - * SectionSelection.of then refuses a required section the allowlist excludes. - */ function readSectionSelection(input: Inputs): Result { const sectionInputs = ["required-sections", "sections"] as const; const names = sectionInputs.map((name) => ({ @@ -399,21 +366,14 @@ function readSectionSelection(input: Inputs): Result if (unknown.length > 0) { return err({ code: "input-unknown-sections", unknown, known: SECTION_KEYS }); } - // Past the rejection above every name is a known key, so the narrowed sets - // are honest - the guard is the proof, not a cast. const isSectionKey = (name: string): name is SectionKey => knownSections.has(name); const [required = [], only = []] = names.map((entry) => entry.names.filter(isSectionKey)); return SectionSelection.of({ only, required }); } /** - * Resolve and validate the `report-public-key` input against the chosen - * channel. The key is the age recipient the `artifact` channel encrypts to, so - * it is required exactly when the channel is `artifact` and rejected otherwise - * (a key set for `none`/`issue` would silently do nothing). A supplied key is - * validated through the age library at parse time, so a malformed recipient - * fails the run before any API work rather than at upload. Returns the trimmed - * key (empty for the non-artifact channels) or the problem. + * The key is required exactly when the channel is `artifact` and rejected otherwise (set for another channel it would + * silently do nothing); it is parsed through the age library here so a malformed recipient fails before any API work. */ function resolveReportPublicKey( input: Inputs, @@ -431,30 +391,20 @@ function resolveReportPublicKey( .mapErr((invalid) => ({ code: "input-report-key-invalid", reason: invalid.reason })); } -/** - * The inputs shared by the two engine modes (apply and check): the flow - * config plus what only the action reads. selfSlug (GITHUB_REPOSITORY) and - * runUrl (GITHUB_SERVER_URL/GITHUB_REPOSITORY/GITHUB_RUN_ID) are read from - * the environment once here, so the run flows stay env-free. - */ +/** selfSlug (GITHUB_REPOSITORY) and runUrl are read from the environment once here, so the run flows stay env-free. */ interface CommonConfig extends RunFlowConfig { token: string; apiVersion: string; } -/** Everything run() needs, already validated; `kind` picks the mode. */ export type RunConfig = | (CommonConfig & (({ kind: "single" } & SingleConfig) | ({ kind: "multi" } & MultiConfig))) | ({ kind: "merge" } & MergeConfig); /** - * The inputs a mode: merge run reads, plus `token`, which it tolerates unread - * (a workflow commonly sets it on every step). Every OTHER declared input is - * an apply/check-time control - a target, the API, a report, the section - * allowlist - so the merge rejects it unless it holds its declared default, - * which the runner supplies whether or not the workflow set the input. - * Derived from the declarations, so a future input is rejected here until it - * is listed as one the merge reads. + * `token` is tolerated unread (a workflow commonly sets it on every step). Every declared input NOT listed here is an + * apply/check control, so the merge rejects it unless it holds its declared default, which the runner supplies whether + * or not the workflow set the input. */ export const MERGE_INPUTS = [ "mode", @@ -465,15 +415,13 @@ export const MERGE_INPUTS = [ ] as const satisfies readonly InputName[]; /** - * The inputs mode: merge rejects when set to a non-default value: every - * declared input MERGE_INPUTS does not list. Exported so the layering guide's - * table is pinned to the whole set. + * Derived from the declarations, so a future input is rejected by the merge until listed in MERGE_INPUTS; exported so + * the layering guide's table is pinned to the whole set. */ export const MERGE_REJECTED_INPUTS: readonly InputName[] = ( Object.keys(INPUT_DECLS) as InputName[] ).filter((name) => !(MERGE_INPUTS as readonly string[]).includes(name)); -/** Read and validate the mode: merge inputs; the first problem wins. */ function parseMergeConfig(input: Inputs): Result, Problem> { return safeTry(function* () { const rejected = MERGE_REJECTED_INPUTS.filter((name) => { @@ -499,12 +447,10 @@ function parseMergeConfig(input: Inputs): Result { const input = inputs(read); return safeTry(function* () { - // The mode decides which inputs exist at all, so it is read before any of - // them: a merge never needs the token the engine modes require first. + // The mode decides which inputs exist at all, so it is read first: a merge never needs the token. const mode = yield* readEnum(input, "mode", MODES, INPUT_DECLS.mode.default, "mode"); if (mode === "merge") { return parseMergeConfig(input); @@ -517,8 +463,6 @@ export function parseConfig(read: InputReader, env: ConfigEnv): Result 0) { return err({ code: "discovery-filters-without-wildcard", @@ -657,8 +597,7 @@ export function parseConfig(read: InputReader, env: ConfigEnv): Result> { @@ -35,12 +30,11 @@ export function readLayerFiles( const KNOWN_SECTIONS: ReadonlySet = new Set(SECTION_KEYS); /** - * The layer as the standalone validation sees it. A null on a known section - * is an opt-out marker, not a setting to judge, and a knobbed wrapper's - * `_layering` is a directive the fold validates and consumes with a message - * naming the layer and the site; neither may reach the section shapes. A null - * on an unknown key opts out of nothing, and only this per-layer pass can name - * the file that misspelled it (a private `_` key's null is ignored either way). + * The layer as the standalone validation sees it; neither marker below may reach the section shapes. + * + * null on a known section -> dropped: an opt-out marker, not a setting to judge + * a wrapper's `_layering` -> dropped: a directive the fold validates itself + * null on an unknown key -> kept: it opts out of nothing, and only this per-layer pass can name the file that misspelled it */ function standaloneView(doc: unknown): unknown { const stripped = stripNulls(doc); @@ -62,18 +56,12 @@ function standaloneView(doc: unknown): unknown { } /** - * A merge has no `sections` allowlist: the merged document is applied later by - * a step whose allowlist this run cannot know, so every unknown top-level key - * is an error naming the layer, as it would be for an apply. + * A merge has no `sections` allowlist: the merged document is applied later by a step whose allowlist this run cannot + * know, so an unknown top-level section is an error naming the layer, as in an apply. */ const NO_ALLOWLIST: ReadonlySet = new Set(); -/** - * Validate every layer on its own terms, fold them low to high, and validate - * the result: a layer must be a valid settings document before it may - * contribute, so the merge can never complete a broken declaration into a - * valid one. `sourceLabel` names the merged document in its own errors. - */ +/** A layer must be a valid document before it may contribute, so the merge can never complete a broken declaration into a valid one. */ export function foldLayers( layers: readonly Layer[], sourceLabel: string, diff --git a/src/flows/library.ts b/src/flows/library.ts index bc955ef18..054d30bbe 100644 --- a/src/flows/library.ts +++ b/src/flows/library.ts @@ -1,10 +1,3 @@ -/** - * The library-shaped entry points over the engine. Each fixes what the action - * decides from its inputs (the mode, the Io sink, the source label), so a - * caller holding a client and a document gets a result back with the lines - * the run printed. - */ - import type { Result } from "neverthrow"; import { stringify as stringifyYaml } from "yaml"; import { @@ -19,12 +12,10 @@ import { type CollectedLine, collectingIo, type Io } from "../io.js"; import type { SettingsProblem } from "../problem.js"; import type { SectionKey } from "../schema.js"; -/** How a document with no caller-given source is named in its own errors. */ const UNNAMED_SOURCE = "the settings document"; const NO_ALLOWLIST: ReadonlySet = new Set(); -/** `sections` is the allowlist; unknown keys outside it come back as `warnings` instead of failing. */ export function validateSettings( doc: unknown, options: { source?: string; sections?: ReadonlySet } = {}, @@ -38,7 +29,6 @@ export function validateSettings( ).map((settings) => ({ settings, warnings: collected.lines.map((entry) => entry.line) })); } -/** The engine's result plus every line it printed when the caller brought no Io of their own. */ export type RepoRunReport = RepoRunResult & { log: CollectedLine[] }; async function runMode( @@ -52,7 +42,6 @@ async function runMode( return { ...result, log: io === undefined ? collected.lines : [] }; } -/** Plan and diff every active section against the live repository; nothing is written. */ export function checkRepository( client: GithubClient, opts: Omit, @@ -61,7 +50,6 @@ export function checkRepository( return runMode(client, opts, "check", io); } -/** Plan every active section and execute the plan's writes. */ export function applyRepository( client: GithubClient, opts: Omit, diff --git a/src/flows/merge.ts b/src/flows/merge.ts index 8d24ff705..28abc8512 100644 --- a/src/flows/merge.ts +++ b/src/flows/merge.ts @@ -1,8 +1,6 @@ /** - * The mode: merge run flow: fold the settings-file layers into one validated - * document and write it to merged-file. The written document is exactly what - * a later apply or check step runs from that path; nothing here touches - * GitHub, so the flow takes no client and needs no token. + * The mode: merge run flow. The written document is exactly what a later apply or check runs from that path; nothing + * here touches GitHub, so the flow takes no client and needs no token. */ import { mkdirSync, writeFileSync } from "node:fs"; @@ -15,25 +13,17 @@ import type { FinishedMerge } from "./deliver.js"; import { foldLayers, readLayerFiles } from "./layers.js"; import { renderMergedYaml } from "./library.js"; -/** - * A mode: merge run: the layers to fold, low to high, and where the result - * goes. No token, no API version, no target, no section allowlist: merge mode - * never reaches GitHub and writes every section the layers declare, and the - * type carries that (no field to read a token or an allowlist from). - */ export interface MergeConfig { settingsFiles: string[]; mergedFile: string; layering: Layering; } -/** How the merged document is named in its own validation errors. */ const MERGED_LABEL = "the merged settings document"; /** - * Refuse a merged-file that names one of the layers. Paths are compared - * resolved, so "./a.yml" and "a.yml" collide. Guarded here, beside the write: - * the next run would fold the merged document as if it were a layer. + * Paths are compared resolved, so "./a.yml" and "a.yml" collide. Guarded beside the write: the next run would fold the + * merged document as if it were a layer. */ function mergedFileCollision(cfg: MergeConfig): Result> { const mergedPath = resolve(cfg.mergedFile); @@ -44,13 +34,6 @@ function mergedFileCollision(cfg: MergeConfig): Result { return mergedFileCollision(cfg) .andThen(() => readLayerFiles(cfg.settingsFiles)) diff --git a/src/flows/multi.ts b/src/flows/multi.ts index ad466f3b4..cdf40d231 100644 --- a/src/flows/multi.ts +++ b/src/flows/multi.ts @@ -1,20 +1,13 @@ /** - * Multi-repo orchestration: resolve targets (central files, explicit - * repos, "*" discovery), read each target's settings, and run every - * target independently through the engine. A target's own document is - * applied exactly as written. The `defaults-file` document is a FALLBACK: - * it is applied whole to a target that has no settings file of its own, and - * never merged into a target that has one. + * Multi-repo orchestration; each target runs independently, and the `defaults-file` document is applied WHOLE to a + * target without a file, never merged into one that has. Under `private-repos: redact` a hidden target closes SEALED: + * the summary, outputs, and report reach it only through projections. * - * When `private-repos: redact` (the default), private and internal targets - * are hidden from this run's public view: their slug is masked and replaced - * with a "private repository #N" placeholder, and their engine output is - * captured rather than emitted. The redaction plan is built - and every - * masked slug registered with the runner and the trace hardening - BEFORE - * any annotation, log line, or output is produced, so nothing leaks in the - * window before masking takes effect. Each target then reports through the - * channel the plan opens for it, and a redacted target's end state closes - * SEALED: the summary, outputs, and report reach it only through projections. + * resolve targets -> resolve visibility -> plan redaction, mask every hidden slug + * -> flush buffered central warnings -> run each target through its channel + * + * Before the mask step nothing is emitted that could name a target, except a fatal exit: its message and the flushed + * central warnings may name slugs the operator wrote into the workflow or the admin repo. */ import { readFileSync } from "node:fs"; @@ -60,11 +53,7 @@ import { } from "./redact.js"; import { parseSettingsDoc, readSettingsFile } from "./settings-read.js"; -/** - * The settings file a remote target is read from, and the action's default - * `settings-file`: the single source for the action.yml default, the - * multi-repo override guard in src/flows/inputs.ts, and the prose below. - */ +/** The single source for the action.yml `settings-file` default, the multi-repo override guard in src/flows/inputs.ts, and the prose below. */ export const DEFAULT_SETTINGS_FILE = ".github/settings.yml"; export interface MultiConfig extends RunFlowConfig { @@ -78,23 +67,12 @@ export interface MultiConfig extends RunFlowConfig { discoveryFiltersSet: string[]; } -/** - * Process one target end to end and return its end state; each failure (read, - * parse, validation, preflight) returns early. A target without a settings - * file runs the defaults document when one was given and is skipped - * otherwise. The channel is the only sink in scope, so a redacted target's - * text lands only in its report. - */ +/** The channel is the only sink in scope, so a redacted target's text lands only in its report. */ async function processTarget(ctx: { api: GithubClient; target: Target; - /** The target as an owner/name pair, parsed once at the caller's boundary. */ repo: RepoRef; - /** - * The defaults document, validated once before any target ran; null when - * no `defaults-file` was given. Applied whole to a target that has no - * settings file, never to one that has. - */ + /** Validated once before any target ran; null when no `defaults-file` was given. */ defaults: ValidatedSettings | null; cfg: MultiConfig; injectMarker: boolean; @@ -103,10 +81,8 @@ async function processTarget(ctx: { const { api, target, defaults, cfg, injectMarker, channel } = ctx; const fail = (richMessage: string): TargetResult => targetFailure(channel.io, richMessage); - // Run one admitted document under the provenance decided where it was - // chosen. Marker injection is validity-preserving (it appends the constant - // marker label config, or strips a rename), so it happens after validation - // and keeps the brand. + // Marker injection is validity-preserving (it appends the constant marker label config, or strips a rename), so it + // happens after validation and keeps the brand. const run = async ( settings: ValidatedSettings, secretSource: SettingsSource, @@ -146,9 +122,7 @@ async function processTarget(ctx: { note: `no ${DEFAULT_SETTINGS_FILE} on the default branch`, }; } - // The defaults document is operator-authored, so it runs with operator - // provenance: a $NAME reference in it resolves from the operator's - // environment, exactly as in a central file. + // The defaults document is operator-authored, so a $NAME reference in it resolves from the operator's environment. channel.io.annotate( "notice", `applying the defaults file: the repository has no ${DEFAULT_SETTINGS_FILE} on its default branch`, @@ -163,9 +137,7 @@ async function processTarget(ctx: { ); } - // validateSettingsDoc names sourceLabel (the slug for remote targets) in - // its own warnings, so they go through the unprefixed sink. Its branded - // return is the engine's admission ticket. + // validateSettingsDoc names sourceLabel (the slug for remote targets) in its own warnings, so they go through the unprefixed sink. const validated = validateSettingsDoc( parsed.value, read.sourceLabel, @@ -179,9 +151,8 @@ async function processTarget(ctx: { } /** - * Channel and exposure come from ONE redaction decision, so a redacted channel - * never travels with a shown exposure. Discovery's full_name is API data, so - * parseRepoSlug is the boundary proving every delivered target is an owner/name pair. + * Channel and exposure come from ONE redaction decision, so a redacted channel never travels with a shown exposure. + * Discovery's full_name is API data, so parseRepoSlug is checked here; a slug that fails it becomes the target's failure. */ function openTarget( plan: RedactionPlan, @@ -199,14 +170,9 @@ function openTarget( } /** - * Read a target's raw settings: from the checked-in central file, or from the - * target repo's own default-branch settings.yml. The document's provenance is - * decided here, with the document: a central file is operator-authored, a - * target's own file is target-authored (its $NAME references are refused - a - * target must not route the operator's environment into itself). Returns - * `{raw, sourceLabel, source}`, `{missing: true}` when a remote target is - * proven to have no file, or `{error}` when the read failed or the absence - * could not be proven. + * The document's provenance is decided here, with the document: a central file is operator-authored, a target's own + * file is target-authored, so its $NAME references are refused (a target must not route the operator's environment + * into itself). `missing` means a remote target is PROVEN to have no file; an absence that could not be proven is `error`. */ async function readTargetSettings( api: GithubClient, @@ -246,25 +212,15 @@ async function readTargetSettings( return { raw: file.content, sourceLabel, source: "target" }; } -/** - * Multi-repo orchestration. Config-level problems (bad defaults file, no - * targets, duplicate definitions, discovery failure) come back as the error - * before any target executes; per-target problems mark that target failed or - * skipped and never stop the others. - */ export function runMulti( api: GithubClient, cfg: MultiConfig, io: Io, uploader?: ArtifactUploader, ): ResultAsync { - // Central-resolution warnings are buffered so nothing emits before the - // redaction mask is registered. Every exit path - fatal or not - flushes - // them through this one helper (the fatal ones via orTee), so a fatal config - // error later in setup can never silently swallow a warning about a - // repos-dir file. Central warnings name repos-dir paths and slugs, which are - // self-disclosed (checked into the public admin repo), so flushing them - // before masking leaks nothing. + // Central-resolution warnings are buffered so nothing emits before the redaction mask is registered; every exit path, + // fatal or not, flushes them through this one helper (the fatal ones via orTee). They name repos-dir paths and slugs, + // which are self-disclosed (checked into the public admin repo), so flushing them before masking on a fatal path leaks nothing. const bufferedWarnings: string[] = []; let warningsFlushed = false; const flushWarnings = (): void => { @@ -276,8 +232,7 @@ export function runMulti( io.annotate("warning", warning); } }; - // Typed so a literal's `code` stays a literal inside the generator, where no - // return type narrows it. + // Typed so a literal's `code` stays a literal inside the generator, where no return type narrows it. const fail = (problem: Problem): Err => err(problem); return safeTry(async function* () { @@ -302,10 +257,9 @@ export function runMulti( reason: string; repos: Parameters[0]["repos"]; }> = []; - // Visibility learned from discovery (authoritative for those repos), so the - // per-target probe is skipped for them. + // Visibility learned from discovery is authoritative for those repos, so their per-target probe is skipped. const knownVisibility = new Map(); - // Private slugs that discovery filtered out (sealed): masked, never placeholdered. + // Private slugs discovery filtered out are masked but never placeholdered. const filteredPrivateSlugs: Private[] = []; if (cfg.reposInput) { const parsed = yield* parseReposInput(cfg.reposInput); @@ -346,13 +300,13 @@ export function runMulti( const redact = cfg.privateRepos === "redact"; const self = cfg.selfSlug.toLowerCase(); - // Resolve visibility for every distinct target slug before the plan: use - // the discovery-supplied value when present, else one probe. Skipped - // entirely under `show` and for the self slug. The resolved visibility (not - // just a boolean) drives TWO decisions: redaction fails closed (redact unless - // proven public), but report DELIVERY fails closed the other way (deliver only - // when proven private or internal) - an unknown must never post a private - // report to a repo that might be public. + // Visibility is resolved for every distinct target slug before the plan, and the resolved value (not a boolean) drives + // two decisions that fail closed in opposite directions, so an unknown never posts a private report to a repo that + // might be public. + // discovery knew it -> that value, no probe + // `show`, or the self slug -> no probe + // redaction -> hide unless proven public + // report delivery -> deliver only when proven private or internal const resolveVisibility = createVisibilityResolver(api); const orderedSlugs = [...central, ...remote].map((t) => t.slug); const visibilityBySlug = new Map(); @@ -370,10 +324,8 @@ export function runMulti( visibilityBySlug.set(key, known ?? (await resolveVisibility(slug))); } } - // Under `redact` the map holds every target slug, so the fallback only - // fires under `show` (where visibility is never consulted) - but it still - // fails CLOSED: an unresolved slug is "unknown", which redaction treats as - // private and delivery treats as unproven, never "public". + // Under `redact` the map holds every target slug, so the fallback only fires under `show`, where visibility is never + // consulted; it still fails CLOSED, since "unknown" redacts as private and delivers as unproven. const visibilityOf = (slug: string): RepoVisibility => visibilityBySlug.get(slug.toLowerCase()) ?? "unknown"; @@ -385,15 +337,11 @@ export function runMulti( cfg.selfSlug, ); - // Mask every hidden slug BEFORE the first annotate/log/output; the API - // trace reads the same registry. + // Every hidden slug is masked before the first line that could name a target; the API trace reads the same registry. for (const slug of plan.maskedSlugs) { io.mask(slug); } - // Now safe to emit: buffered central warnings (flushed exactly once here on - // the happy path; orTee below flushes them on every fatal path), then the - // (redacting) skip notices. flushWarnings(); for (const group of skipGroups) { io.annotate("notice", formatSkipNotice(group, redact)); @@ -413,13 +361,10 @@ export function runMulti( const results = await withDelivery({ api, cfg, io, uploader }, async (delivery) => { const delivered: TargetOutcome[] = []; for (const target of targets) { - // The channel is opened BEFORE any processing so a read/parse/validation - // failure lands in a redacted target's transcript too; it is the only sink - // processing sees. + // The channel is opened BEFORE any processing so a read/parse/validation failure lands in a redacted target's transcript too. const opened = openTarget(plan, io, target.slug, visibilityOf); const { channel, repo } = opened; - // A crash mid-processing never stops the rest of the fleet; it becomes - // this target's failure and still closes through the same delivery. + // A crash mid-processing never stops the rest of the fleet; it becomes this target's failure and still closes through the same delivery. const closed = await delivery.target(opened, async (injectMarker) => repo === null ? targetFailure( diff --git a/src/flows/redact.ts b/src/flows/redact.ts index fe7790824..d47298534 100644 --- a/src/flows/redact.ts +++ b/src/flows/redact.ts @@ -1,14 +1,8 @@ /** - * Private-repo redaction: the plan that decides which targets are hidden, - * the placeholder names that replace their slugs everywhere the run is - * publicly readable, the per-target channel that routes a redacted target's - * lines into a transcript and seals its end state, and the projections that - * open the seal into the public view. - * - * GitHub Actions has no log-level access control: run logs, summaries, and - * outputs inherit the admin repository's visibility. When that repository is - * public, a target's slug and its live settings would leak. Redaction is the - * choke point that keeps private and internal targets out of the public view. + * GitHub Actions has no log-level access control: run logs, summaries, and outputs inherit the admin repository's + * visibility, so a public admin repo would leak a private target's slug and live settings. Redaction is the choke + * point: the plan decides which targets hide, the channel routes their lines into a transcript and seals the end state, + * and the projections open the seal into the public view. */ import type { Target } from "../discovery/targets.js"; @@ -20,27 +14,18 @@ import { revealPrivate } from "../private-open.js"; import type { RedactedDetail, TargetDetail } from "../report/delivery.js"; import type { SectionKey } from "../schema.js"; -/** The `private-repos` input values; the single source its type derives from. */ export const PRIVATE_REPOS_POLICIES = ["redact", "show"] as const; export type PrivateReposPolicy = (typeof PRIVATE_REPOS_POLICIES)[number]; -/** - * The note appended to every redacted line: it names the two escape hatches - * (opt out, or run from a context where the target's own logs are private). - */ export const REDACTED_NOTE = "details hidden: the repository is private or internal. Set private-repos: show to reveal them, or run the action inside that repository"; -/** The placeholder that replaces every hidden detail value in the public view. */ export const REDACTED_DETAIL = "hidden (private repository)"; /** - * The notice for a redacted target whose visibility could not be PROVEN - * private or internal, so the private report was withheld (delivery fails - * closed the opposite way from redaction). Shared verbatim by the single- - * and multi-repo run flows - the cause and the fix are slug-free, so one - * wording serves both without leaking anything. + * For a redacted target whose visibility could not be PROVEN private or internal, so the report was withheld (delivery + * fails closed the opposite way from redaction). Shared verbatim by both run flows: the cause and the fix are slug-free. */ export const WITHHELD_REPORT_NOTICE = "visibility could not be verified (the repository-metadata probe failed or was inconclusive " + @@ -65,15 +50,11 @@ type RedactedOutcome = { }; /** - * Strip a redacted target's section outcomes to safe values: the key and - * status (closed enums, provably leak-free) survive, and every detail value is - * replaced with the placeholder - plus the HTTP code on failed/skipped - * sections, the one piece of error context that is a safe closed value. + * The key and status (closed enums, provably leak-free) survive; every detail value becomes the placeholder plus, on + * failed/skipped rows, the HTTP code. */ function redactOutcomes(outcomes: RepoRunResult["outcomes"]): RedactedOutcome[] { return outcomes.map((o) => { - // The SectionOutcome union proves a code exists only on failed/skipped - // rows, so presence alone decides. const withCode = o.httpStatus !== undefined ? `${REDACTED_DETAIL}, HTTP ${o.httpStatus}` : REDACTED_DETAIL; return { key: o.key, status: o.status, detail: [withCode] }; @@ -86,11 +67,6 @@ export interface PublicDetail { note?: string; } -/** - * The leak-free projection of a target's detail: sealed detail reduces to - * statuses (plus HTTP codes) and the generic note, open detail passes through - * byte-identical. The one place a seal opens for a public surface. - */ export function publicDetail(detail: TargetOutcome["detail"]): PublicDetail { if (isPrivate(detail)) { return { outcomes: redactOutcomes(revealPrivate(detail).outcomes), note: REDACTED_NOTE }; @@ -101,7 +77,6 @@ export function publicDetail(detail: TargetOutcome["detail"]): PublicDetail { }; } -/** The public view of one multi-repo target (summary, outputs, annotations). */ export interface PublicTargetView extends PublicDetail { display: string; source: Target["source"]; @@ -117,15 +92,13 @@ export function toPublicView(target: TargetOutcome): PublicTargetView { }; } -/** True when a resolved visibility PROVES the repo private or internal. */ export function isPrivateVisibility(visibility: RepoVisibility): boolean { return visibility === "private" || visibility === "internal"; } /** - * The single generic annotation a redacted target gets: a failure names the - * failed section keys and HTTP codes, a drift its drifted keys (closed values - * only), a skip is noticed, a healthy run says nothing. + * The single generic annotation a redacted target gets, closed values only: failed and drifted section keys, HTTP + * codes; a healthy run says nothing. */ export function emitRedactedResult( io: Io, @@ -153,34 +126,19 @@ export function emitRedactedResult( } } -/** - * The redaction decision for one run: which slugs are hidden, the - * placeholder each redacted slug renders as, and the full masked set the - * caller registers with `io.mask` and the trace hardening. All slug lookups - * are case-insensitive; a central and a remote entry for the same repository - * share one placeholder. - */ export interface RedactionPlan { - /** True when this slug must be hidden from the public view. */ isRedacted(slug: string): boolean; - /** The placeholder for a redacted slug, or the slug itself when not redacted. */ display(slug: string): string; /** Every slug that must be masked: redacted targets plus discovery-filtered privates. */ maskedSlugs: string[]; } -/** The plan under `private-repos: show`: nothing is hidden, nothing is masked. */ const SHOW_EVERYTHING: RedactionPlan = { isRedacted: () => false, display: (slug) => slug, maskedSlugs: [], }; -/** - * Build the redaction plan (the identity plan under `show`): private targets - * get `private repository #N` in target order, discovery-filtered privates are - * only masked (not targets), and the self slug is never redacted. - */ export function planRedaction( policy: PrivateReposPolicy, orderedTargetSlugs: string[], @@ -211,7 +169,6 @@ export function planRedaction( if (key === self || masked.has(key)) { continue; } - // Discovery-filtered privates are masked but never placeholdered. masked.set(key, slug); } @@ -223,9 +180,8 @@ export function planRedaction( } /** - * An Io that lets nothing textual out: annotate/log are recorded for the - * private report, debug/summary/output are dropped (those surfaces are written - * from the public view), only the mask registry passes through. + * Lets nothing textual out: annotate/log are recorded for the private report, debug/summary/output are dropped (those + * surfaces are written from the public view), only the mask registry passes through. */ export function capturingIo(io: Io): { io: Io; drain(): CollectedLine[] } { const captured: CollectedLine[] = []; @@ -244,9 +200,8 @@ export function capturingIo(io: Io): { io: Io; drain(): CollectedLine[] } { } /** - * The channel one target reports through, opened ONCE from the redaction - * decision: in the clear it emits publicly and closes open, redacted it - * captures every line and closes sealed. Processing code holds only this. + * Opened ONCE from the redaction decision: in the clear it emits publicly and closes open, redacted it captures every + * annotation and log line and closes sealed. Processing code holds only this. */ export interface TargetChannel { /** The public label: the slug, or its placeholder. */ @@ -255,11 +210,9 @@ export interface TargetChannel { io: Io; /** Sink for lines that already name their source (validation warnings): unprefixed, or the same capture. */ unprefixed: Io; - /** Close the channel with the target's section outcomes and skip/failure note. */ close(outcomes: RepoRunResult["outcomes"], note?: string): TargetOutcome["detail"]; } -/** A target in the clear; `attributed` prefixes its lines with the slug (multi-repo mode). */ export function publicChannel(io: Io, slug: string, attributed: boolean): TargetChannel { return { display: slug, @@ -269,7 +222,6 @@ export function publicChannel(io: Io, slug: string, attributed: boolean): Target }; } -/** A redacted target: every line is captured, and the detail closes sealed with the transcript. */ export function redactedChannel(io: Io, slug: string, display: string): TargetChannel { const capture = capturingIo(io); return { @@ -281,8 +233,7 @@ export function redactedChannel(io: Io, slug: string, display: string): TargetCh } /** - * Run `work` through the channel: a crash (a preflight write attempt naming its - * path, an engine bug) is the target's failure, spoken only through the + * A crash (a preflight write attempt naming its path, an engine bug) is the target's failure, spoken only through the * channel's sink, so a redacted repository's text never reaches a top-level handler. */ export async function attempt( @@ -299,7 +250,6 @@ export async function attempt( } } -/** Open a multi-repo target's channel from the plan; lines in the clear carry the slug prefix. */ export function openTargetChannel(plan: RedactionPlan, io: Io, slug: string): TargetChannel { return plan.isRedacted(slug) ? redactedChannel(io, slug, plan.display(slug)) diff --git a/src/flows/settings-read.ts b/src/flows/settings-read.ts index 7e4de1744..6c7061f52 100644 --- a/src/flows/settings-read.ts +++ b/src/flows/settings-read.ts @@ -1,11 +1,6 @@ /** - * The one place YAML settings documents are read and parsed. The read failure - * carries the role the caller names the file by, because the right fix - * differs per source (defaults file, layer, single settings file); a remote - * file's text is parsed on its own through parseSettingsDoc. The parsed - * document comes back UNKNOWN on purpose: nothing has validated it yet, so - * only validateSettingsDoc (which returns the branded ValidatedSettings) can - * turn it into something the engine accepts. + * The one place settings YAML is parsed. The read problem carries the role the caller names the file by, because the + * right advice differs per source, and the document comes back `unknown`: only validateSettingsDoc mints ValidatedSettings. */ import { readFileSync } from "node:fs"; @@ -14,15 +9,12 @@ import { parse as parseYaml } from "yaml"; import type { ProblemOf, SettingsFileRole } from "../problem.js"; /** - * Parse one YAML settings document; empty/null documents become {}. + * `logLevel: "error"` is load-bearing: at its default the parser reports a warning (an unresolved tag, an anchor ending + * in ":") on a SUCCESSFUL parse through process.emitWarning, quoting the offending source line with its values straight + * to stderr, which nothing here redacts. Empty and null documents become {}. * - * At its default log level the parser reports a warning (an unresolved tag, - * an unknown directive, an anchor ending in ":", a collection used as a key) - * on a SUCCESSFUL parse through process.emitWarning, quoting the offending - * source line, values included, straight to stderr: a channel no message of - * ours words or redacts. "error" keeps every warning silent while a syntax - * error still throws into the error path below; "silent" would also swallow - * the errors and return a partial document. + * "error" -> warnings silent; a syntax error still throws into the error path + * "silent" -> would also swallow the syntax errors */ export function parseSettingsDoc(raw: string): Result> { try { @@ -32,7 +24,6 @@ export function parseSettingsDoc(raw: string): Result