diff --git a/.github/scripts/changed-sections.ts b/.github/scripts/changed-sections.ts index 614eee3e..44e31858 100644 --- a/.github/scripts/changed-sections.ts +++ b/.github/scripts/changed-sections.ts @@ -77,6 +77,7 @@ export const ALL_SELECTING_PREFIXES = [ "src/plain-data.ts", "src/private-open.ts", "src/private.ts", + "src/problem.ts", "src/schema.ts", "src/types.ts", "test/e2e/", diff --git a/.github/scripts/gen-action-docs.ts b/.github/scripts/gen-action-docs.ts index 637a9997..92173cbc 100644 --- a/.github/scripts/gen-action-docs.ts +++ b/.github/scripts/gen-action-docs.ts @@ -6,9 +6,9 @@ import { readFileSync, writeFileSync } from "node:fs"; import { dirname, join, posix } from "node:path"; -import type { InputDecl } from "../../src/action/inputs.js"; -import { INPUT_DECLS } from "../../src/action/inputs.js"; import { OUTPUT_DECLS } from "../../src/action/io.js"; +import type { InputDecl } from "../../src/flows/inputs.js"; +import { INPUT_DECLS } from "../../src/flows/inputs.js"; import { UNDECLARED_POLICY_SECTIONS, type UndeclaredPolicySection } from "../../src/schema.js"; import { overrideAdviceLevel } from "../../src/sections/contract/errors.js"; import type { SectionMeta } from "../../src/sections/contract/module.js"; diff --git a/action.yml b/action.yml index 1f303d87..be8da34b 100644 --- a/action.yml +++ b/action.yml @@ -6,7 +6,7 @@ branding: color: blue inputs: - # BEGIN GENERATED: action-inputs (bun run build:action-docs; edit src/action/inputs.ts) + # BEGIN GENERATED: action-inputs (bun run build:action-docs; edit src/flows/inputs.ts) token: description: >- Token used for the API calls. Most sections need a fine-grained PAT with diff --git a/architecture.yml b/architecture.yml index 4afdd494..ec49419f 100644 --- a/architecture.yml +++ b/architecture.yml @@ -18,6 +18,7 @@ layers: report: [src/report/] schema: [src/schema.ts] io: [src/io.ts] + problem: [src/problem.ts] types: [src/types.ts] plain-data: [src/plain-data.ts] private: [src/private.ts, src/private-open.ts] @@ -34,12 +35,13 @@ exclude: edges: main: [action] action: [library] - library: [discovery, engine, flows, github, io, report, schema, sections, types] - flows: [discovery, engine, github, io, private, report, schema] - engine: [discovery, github, io, plain-data, schema, sections, types] + library: [discovery, engine, flows, github, io, problem, report, schema, sections, types] + flows: [discovery, engine, github, io, plain-data, private, problem, report, schema, types] + engine: [discovery, github, io, plain-data, problem, schema, sections, types] sections: [discovery, engine, github, schema, types, upstream-gaps] github: [io, plain-data] - discovery: [github, private] - report: [discovery, engine, github, io, private, schema, sections, types] + discovery: [github, private, problem] + report: [discovery, engine, github, io, private, problem, schema, sections, types] + problem: [plain-data, schema] schema: [sections, types] upstream-gaps: [types] diff --git a/bun.lock b/bun.lock index be4aa865..9b00a129 100644 --- a/bun.lock +++ b/bun.lock @@ -16,6 +16,7 @@ "@octokit/plugin-throttling": "11.0.5", "age-encryption": "0.3.1", "bottleneck": "2.19.5", + "neverthrow": "8.2.0", "yaml": "2.9.0", "zod": "4.5.4", }, @@ -232,6 +233,8 @@ "@protobuf-ts/runtime-rpc": ["@protobuf-ts/runtime-rpc@2.11.1", "", { "dependencies": { "@protobuf-ts/runtime": "^2.11.1" } }, "sha512-4CqqUmNA+/uMz00+d3CYKgElXO9VrEbucjnBFEjqI4GuDrEQ32MaI3q+9qPBvIGOlL4PmHXrzM32vBPWRhQKWQ=="], + "@rollup/rollup-linux-x64-gnu": ["@rollup/rollup-linux-x64-gnu@4.63.1", "", { "os": "linux", "cpu": "x64" }, "sha512-gfI5T24WLLuFfSKw7Go/zDXjAAV0fny0swTaDv+WjK7vqcw4cRhFfdsyKL1n+ukI+ooBxn3bVQnyrn06WpI50w=="], + "@scure/base": ["@scure/base@2.4.0", "", {}, "sha512-thZ1TuJwFwBblOhgsjDKvvGirBxNp+wSvY/DR6tJBJOTDhdAAcHJ8Vbr2eFnqaxeca4+t0i9KBf+uHYGWwZORg=="], "@tybys/wasm-util": ["@tybys/wasm-util@0.10.3", "", { "dependencies": { "tslib": "^2.4.0" } }, "sha512-F3fo1MYrRJYL3zER0OUOmkutjr1Vp23m7OsSgp7nq4SP6OqX6C/56XFIPAl5bt3zaBRjmW7SGz3u/6LwFpYcOg=="], @@ -468,6 +471,8 @@ "ms": ["ms@2.1.3", "", {}, "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA=="], + "neverthrow": ["neverthrow@8.2.0", "", { "optionalDependencies": { "@rollup/rollup-linux-x64-gnu": "^4.24.0" } }, "sha512-kOCT/1MCPAxY5iUV3wytNFUMUolzuwd/VF/1KCx7kf6CutrOsTie+84zTGTpgQycjvfLdBBdvBvFLqFD2c0wkQ=="], + "normalize-path": ["normalize-path@3.0.0", "", {}, "sha512-6eZs5Ls3WtCisHWp9S2GUy8dqkpGi4BVSz3GaqiE6ezub0512ESztXUwUB6C6IKbQkY2Pnb/mD4WYojCRwcwLA=="], "oxc-parser": ["oxc-parser@0.147.0", "", { "dependencies": { "@oxc-project/types": "^0.147.0" }, "optionalDependencies": { "@oxc-parser/binding-android-arm-eabi": "0.147.0", "@oxc-parser/binding-android-arm64": "0.147.0", "@oxc-parser/binding-darwin-arm64": "0.147.0", "@oxc-parser/binding-darwin-x64": "0.147.0", "@oxc-parser/binding-freebsd-x64": "0.147.0", "@oxc-parser/binding-linux-arm-gnueabihf": "0.147.0", "@oxc-parser/binding-linux-arm-musleabihf": "0.147.0", "@oxc-parser/binding-linux-arm64-gnu": "0.147.0", "@oxc-parser/binding-linux-arm64-musl": "0.147.0", "@oxc-parser/binding-linux-ppc64-gnu": "0.147.0", "@oxc-parser/binding-linux-riscv64-gnu": "0.147.0", "@oxc-parser/binding-linux-riscv64-musl": "0.147.0", "@oxc-parser/binding-linux-s390x-gnu": "0.147.0", "@oxc-parser/binding-linux-x64-gnu": "0.147.0", "@oxc-parser/binding-linux-x64-musl": "0.147.0", "@oxc-parser/binding-openharmony-arm64": "0.147.0", "@oxc-parser/binding-win32-arm64-msvc": "0.147.0", "@oxc-parser/binding-win32-ia32-msvc": "0.147.0", "@oxc-parser/binding-win32-x64-msvc": "0.147.0" } }, "sha512-5xaug6t7GfV3BO5Iv+xHW1rmQkDEQ3BEu3L8g3InsvWO5i8CYGc4tCZ2X985QcwWNycFJam+aOns6Nr2XAThTA=="], diff --git a/docs/reference/architecture.md b/docs/reference/architecture.md index e3f7fd13..f0e43b0b 100644 --- a/docs/reference/architecture.md +++ b/docs/reference/architecture.md @@ -239,6 +239,7 @@ graph TD report["src/report/"] schema["src/schema.ts"] io["src/io.ts"] + problem["src/problem.ts"] types["src/types.ts"] plain_data["src/plain-data.ts"] private["src/private.ts
src/private-open.ts"] @@ -250,6 +251,7 @@ graph TD library --> flows library --> github library --> io + library --> problem library --> report library --> schema library --> sections @@ -258,13 +260,17 @@ graph TD flows --> engine flows --> github flows --> io + flows --> plain_data flows --> private + flows --> problem flows --> report flows --> schema + flows --> types engine --> discovery engine --> github engine --> io engine --> plain_data + engine --> problem engine --> schema engine --> sections engine --> types @@ -278,14 +284,18 @@ graph TD github --> plain_data discovery --> github discovery --> private + discovery --> problem report --> discovery report --> engine report --> github report --> io report --> private + report --> problem report --> schema report --> sections report --> types + problem --> plain_data + problem --> schema schema --> sections schema --> types upstream_gaps --> types diff --git a/docs/reference/inputs.md b/docs/reference/inputs.md index 2cd3205d..540d8b78 100644 --- a/docs/reference/inputs.md +++ b/docs/reference/inputs.md @@ -8,7 +8,7 @@ Every `with:` input the action accepts, and the outputs it sets for the steps af ## Inputs - + | Input | Default | Meaning | |---|---|---| | `token` | `github.token` | Token for the API calls (see [Token permissions](permissions.md)) | diff --git a/package.json b/package.json index b7c8810d..73866f61 100644 --- a/package.json +++ b/package.json @@ -35,6 +35,7 @@ "@octokit/plugin-throttling": "11.0.5", "age-encryption": "0.3.1", "bottleneck": "2.19.5", + "neverthrow": "8.2.0", "yaml": "2.9.0", "zod": "4.5.4" }, diff --git a/src/action/inputs.ts b/src/action/inputs.ts index 1d7be212..e888d3c5 100644 --- a/src/action/inputs.ts +++ b/src/action/inputs.ts @@ -1,749 +1,15 @@ /** - * GitHub Actions input reading and validation. parseConfig() reads every - * input in a stable order, validates it (each error names the input and - * the fix), and returns the typed RunConfig run() executes - so the - * execution code never touches raw inputs. + * 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 { - AFFILIATIONS, - ARCHIVED_FILTERS, - DEFAULT_API_VERSION, - DEFAULT_DISCOVERY_FILTERS, - DEFAULT_SETTINGS_FILE, - type DiscoveryFilters, - FORKS_FILTERS, - type Layering, - type MergeConfig, - type MultiConfig, - type MustBeNever, - PRIVATE_REPORT_CHANNELS, - PRIVATE_REPOS_POLICIES, - type PrivateReportChannel, - type PrivateReposPolicy, - parseRecipient, - parseRepoSlug, - quoteList, - type RunFlowConfig, - SECTION_KEYS, - type SectionKey, - type SingleConfig, - VISIBILITY_FILTERS, -} from "../index.js"; +import type { Result } from "neverthrow"; +import { type Problem, parseConfig, type RunConfig } from "../index.js"; -/** Default `private-repos`, pinned against action.yml by the contract test. */ -export const DEFAULT_PRIVATE_REPOS = "redact" satisfies PrivateReposPolicy; - -/** Default `private-report`, pinned against action.yml by the contract test. */ -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. - */ -export interface InputDecl { - /** The action.yml description; the generator folds it to width. */ - readonly description: string; - /** The action.yml default, verbatim (an empty string means "unset"). */ - readonly default: string; - /** 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). - */ - 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. - */ -export const INPUT_DECLS = { - token: { - description: - "Token used for the API calls. Most sections need a fine-grained PAT with Administration read/write on the repository - the default GITHUB_TOKEN can never hold that permission.", - // biome-ignore lint/suspicious/noTemplateCurlyInString: a workflow expression the runner resolves, not a JS template - default: "${{ github.token }}", - summary: "Token for the API calls (see [Token permissions](docs/reference/permissions.md))", - shownDefault: "`github.token`", - }, - repository: { - description: - "Target repository (owner/name). Defaults to the current repository. Single-repo mode only; cannot be combined with repos or repos-dir.", - default: "", - summary: "Target `owner/name` (single-repo mode only)", - shownDefault: "current repo", - }, - "settings-file": { - description: - "Path to the settings YAML file: exactly one in apply and check. In mode: merge, the " + - "ordered list of settings files to fold instead, newline- or comma-separated, lowest " + - "layer first. Newlines and commas are list separators in every mode, so a settings-file " + - "path can never contain a comma. Single-repo and merge modes only; multi-repo targets " + - "read repos-dir files or each repository's own .github/settings.yml, so overriding it " + - "alongside repos or repos-dir fails the run.", - default: DEFAULT_SETTINGS_FILE, - summary: - "Settings file path (single-repo mode); in `mode: merge`, the ordered list of layers to fold, low to high", - }, - mode: { - description: - "apply (mutate), check (report drift, exit 1 on any), or merge (fold the settings-file " + - "layers into one document written to merged-file, with no token and no GitHub API call; " + - "merge reads only settings-file, merged-file, and layering, ignores token, and rejects " + - "every other input set to a non-default value, since each controls an apply or check " + - "run). check makes no settings changes, though a private report may still be delivered.", - default: "apply", - summary: - "`apply` mutates; `check` reports drift and exits 1 on any, making no settings changes (a private report may still be delivered); `merge` folds the settings-file layers into merged-file without touching GitHub", - }, - "merged-file": { - description: - "mode: merge only, and required there: the path the merged settings document is written " + - "to (parent directories are created). The file holds exactly what apply would run: every " + - "section validated, each section that takes an undeclared policy in its policy-wrapper " + - "form with the policy made explicit, the other sections in their own shape, and private " + - "underscore keys and the _layering directives dropped. Feed it to a later apply or check " + - "step as its settings-file. Must not name one of the settings-file layers (the merge " + - "would overwrite it). Fails when set in apply or check.", - default: "", - summary: - "`mode: merge` only (required there): where the merged document is written, exactly what `apply` would run", - }, - "on-missing-permission": { - description: - "fail (default) or warn. Under warn, sections the token cannot access are skipped with a warning and the run stays green (partial success).", - default: "fail", - summary: "`warn` skips sections the token cannot access (partial success)", - }, - "required-sections": { - description: - "Comma-separated section names that must fully apply even under on-missing-permission: " + - 'warn (minimum requirements). Every name must also be allowed by the "sections" input ' + - "when that allowlist is set; a required section the allowlist excludes is rejected up " + - "front, because the run could never attempt it.", - default: "", - summary: "Sections that must fully apply even under `warn`", - }, - sections: { - description: - "Optional comma-separated allowlist of sections to process. apply and check only: mode: merge writes every section its layers declare, so the allowlist belongs on the step that runs the merged document and fails the merge when set.", - default: "", - summary: - "Comma-separated allowlist of sections to process (apply and check only; rejected in `mode: merge`)", - shownDefault: "(all declared)", - }, - "api-version": { - description: - "X-GitHub-Api-Version header value. Override to opt into a newer REST API version before this action defaults to it.", - default: DEFAULT_API_VERSION, - summary: "`X-GitHub-Api-Version` header; override to opt into a newer REST API version", - }, - repos: { - description: - "Multi-repo remote mode: comma- or newline-separated owner/name targets, each applied " + - 'from its own .github/settings.yml (default branch), or "*" alone to discover every ' + - "repository the token's user owns, filterable via the visibility, archived, forks, " + - "exclude, topics, and affiliation inputs. Combinable with repos-dir; a repos-dir file " + - "for the same repository wins.", - default: "", - summary: - "Multi-repo remote mode: `owner/name` list (comma/newline), or `*` to discover owned repos", - }, - "repos-dir": { - description: - "Multi-repo central mode: a directory in the checked-out admin repository holding per-repo settings files - .yml (same owner as this repository) or /.yml. Requires actions/checkout.", - default: "", - summary: "Multi-repo central mode: directory of per-repo settings files in this repo", - }, - "defaults-file": { - description: - "YAML settings document applied to every multi-repo target that has no settings file of " + - "its own (a repos target without .github/settings.yml, which is otherwise skipped). A " + - "target with its own file is applied as written; the defaults are never merged into it. " + - 'With repos: "*" every discovered repository without a settings file receives the ' + - "defaults; run mode: check first. Multi-repo mode only; fails when set without repos or " + - "repos-dir.", - default: "", - summary: - "YAML applied to every multi-repo target without a settings file (multi-repo mode only)", - }, - layering: { - description: - "mode: merge only: merge (default) or replace, the run-wide default for how the keyed " + - "list sections (labels, rulesets) combine with the layers below them; a layer's own " + - "_layering directive, at its top level or on a section's {entries} wrapper, overrides it " + - "per file or per section. Every other list is replaced by the higher layer's. Fails when " + - "set in apply or check.", - default: "", - summary: - "`mode: merge` only: `merge` unions the keyed list sections (labels, rulesets) by key across layers, `replace` lets the higher layer's list win; a layer's `_layering` overrides it", - shownDefault: "`merge`", - }, - "private-repos": { - description: - "redact (default) or show. Under redact, private and internal targets are hidden from " + - "this run's public logs, summary, and outputs: their slug becomes a \"private repository " + - '#N" placeholder, live values and error bodies are replaced with "hidden (private ' + - "repository)\", and each slug is registered with the runner's secret masker. A target " + - "equal to GITHUB_REPOSITORY is never redacted. show reveals everything (today's " + - "behavior); only use it when the run's logs are not publicly readable.", - default: DEFAULT_PRIVATE_REPOS, - summary: - "`redact` hides private and internal targets from public logs, summary, and outputs; `show` reveals them", - }, - "private-report": { - description: - "none (default), issue, issue-on-failure, or artifact. Delivers the full unredacted " + - "report only for redacted targets the visibility probe proves private or internal (an " + - "unknown visibility is redacted but excluded from delivery). Under issue, each such " + - "target's report is delivered as a reused, marker-labelled issue on that target " + - "repository itself (the one GitHub-private channel a public run has): the body is " + - "replaced every run, and the issue is opened when the target fails or drifts and closed " + - "when it is healthy. issue-on-failure is the quiet variant: a failing or drifting target " + - "gets the same issue, but a healthy run only closes a still-open issue from a previous " + - "failure and otherwise writes nothing - no issue ever appears on a repository that never " + - "needed attention (though a declared labels section still creates the marker label, and " + - "a manually-removed marker label defers the close: the next failing run reattaches it, " + - "and the first healthy run after that closes the issue). Under artifact, those reports " + - "are concatenated, age-encrypted to report-public-key, and uploaded as one workflow " + - "artifact (settings-as-code-private-report) for readers who hold the key but no GitHub " + - "access to the targets; the artifact channel needs the Actions artifact service, so on " + - "GitHub Enterprise Server it warns and uploads nothing. Applies only to redacted " + - "targets, so it is rejected alongside private-repos: show. Report delivery writes even " + - "in mode: check, and its failure never changes the run's result.", - default: DEFAULT_PRIVATE_REPORT, - summary: - "`issue` delivers each redacted target's full report to a reused issue on that target " + - "repository; `issue-on-failure` writes that issue only when the target fails or drifts, " + - "closing it once healthy; `artifact` uploads all reports as one age-encrypted workflow " + - "artifact; rejected with `private-repos: show`", - }, - "report-public-key": { - description: - 'The age recipient (an "age1..." public key) the artifact channel encrypts every report ' + - 'to; safe to commit in the workflow. Generate a keypair with "age-keygen -o key.txt", ' + - 'keep key.txt secret, and decrypt a downloaded artifact with "age -d -i key.txt ' + - 'private-report.md.age". Required when private-report is artifact and rejected otherwise.', - default: "", - summary: - "The `age1...` recipient the `artifact` channel encrypts reports to; required with `private-report: artifact`, rejected otherwise", - }, - visibility: { - description: - 'Keeps only repositories of this visibility in repos: "*" discovery. One of all (default), public, private, or internal; internal is matched client-side (Enterprise only). Fails if set without repos: "*".', - default: "", - summary: "Discovery-only: keep `public`, `private`, or `internal` repositories", - shownDefault: `\`${DEFAULT_DISCOVERY_FILTERS.visibility}\``, - }, - archived: { - description: - 'Archived-repository policy for repos: "*" discovery. One of skip (default; settings writes fail on archived repositories), include, or only (mostly useful with mode: check). Fails if set without repos: "*".', - default: "", - summary: "Discovery-only: `skip`, `include`, or `only` archived repositories", - shownDefault: `\`${DEFAULT_DISCOVERY_FILTERS.archived}\``, - }, - forks: { - description: - 'Fork policy for repos: "*" discovery. One of include (default), exclude, or only. Fails if set without repos: "*".', - default: "", - summary: "Discovery-only: `include`, `exclude`, or `only` forks", - shownDefault: `\`${DEFAULT_DISCOVERY_FILTERS.forks}\``, - }, - exclude: { - description: - 'Comma- or newline-separated wildcard patterns removing repositories from repos: "*" ' + - 'discovery. "*" matches any characters; a pattern containing "/" matches the full ' + - 'owner/name, any other the name alone. Case-insensitive. Fails if set without repos: "*".', - default: "", - summary: - "Discovery-only: `*` wildcard patterns (name, or `owner/name` if the pattern has a `/`) to drop", - }, - topics: { - description: - 'Comma- or newline-separated topics; repos: "*" discovery keeps only repositories carrying at least one of them. Unrelated to the topics settings section. Fails if set without repos: "*".', - default: "", - summary: "Discovery-only: keep repositories carrying at least one listed topic", - }, - affiliation: { - description: - 'Comma-separated affiliations for repos: "*" discovery, passed to the GitHub /user/repos ' + - "listing. Any of owner, collaborator, organization_member; the list replaces the default " + - "(owner), so use owner,collaborator to widen rather than move discovery. Fails if set " + - 'without repos: "*".', - default: "", - summary: "Discovery-only: `owner`, `collaborator`, `organization_member` (comma list)", - shownDefault: `\`${DEFAULT_DISCOVERY_FILTERS.affiliation.join(",")}\``, - }, -} as const satisfies Record; - -type InputName = keyof typeof INPUT_DECLS; - -function input(name: InputName): string { +/** 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. - return core.getInput(name); -} - -/** The input, or its declared default when the step (or a local run) left it unset. */ -function inputOrDefault(name: InputName): string { - return input(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", - "forks", - "exclude", - "topics", - "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 error message cannot drift apart. - */ -function readEnum( - name: InputName, - allowed: readonly T[], - fallback: T, - noun: string, -): T | { error: string } { - const value = input(name) || fallback; - if (!(allowed as readonly string[]).includes(value)) { - const values = allowed.map((v) => (v === fallback ? `"${v}" (default)` : `"${v}"`)); - return { - error: `the "${name}" input is "${value}", which is not a supported ${noun}. Set it to ${values.join(", ")}`, - }; - } - return value as T; -} - -/** 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) - .map((s) => s.trim()) - .filter(Boolean); -} - -/** The run kinds the `mode` input selects; apply and check run the engine, merge only folds files. */ -const MODES = ["apply", "check", "merge"] as const; - -/** 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. - */ -const MERGE_ONLY_INPUTS = ["merged-file", "layering"] as const satisfies readonly InputName[]; - -/** - * The `required-sections` and `sections` inputs as validated key sets: every - * unknown name in each is reported at once, against its own input name, and - * a required section the allowlist excludes is rejected. - */ -function readSectionSets(): - | { requiredSections: Set; onlySections: Set } - | { error: string } { - const readSectionNames = (name: "required-sections" | "sections"): Set => - new Set(splitList(input(name))); - const requiredNames = readSectionNames("required-sections"); - const onlyNames = readSectionNames("sections"); - const knownSections = new Set(SECTION_KEYS); - const unknownIn = (names: Set, inputName: string): string | null => { - const unknown = [...names].filter((name) => !knownSections.has(name)); - if (unknown.length === 0) { - return null; - } - const quoted = unknown.map((name) => `"${name}"`).join(", "); - return unknown.length === 1 - ? `unknown section ${quoted} in the "${inputName}" input; it matches none of: ${SECTION_KEYS.join(", ")}. Fix the name in the workflow's input list` - : `unknown sections ${quoted} in the "${inputName}" input; each matches none of: ${SECTION_KEYS.join(", ")}. Fix the names in the workflow's input list`; - }; - const unknownSections = [ - unknownIn(requiredNames, "required-sections"), - unknownIn(onlyNames, "sections"), - ].filter((problem): problem is string => problem !== null); - if (unknownSections.length > 0) { - return { error: unknownSections.join("; ") }; - } - // 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 requiredSections = new Set([...requiredNames].filter(isSectionKey)); - const onlySections = new Set([...onlyNames].filter(isSectionKey)); - // required-sections is a proof obligation ("this section must not be - // skipped"), but a `sections` allowlist EXCLUDES sections from running at - // all - the engine reports them "excluded" without attempting them - so a - // required section outside a non-empty allowlist would let the run pass - // green having proven nothing about it. Reject the contradiction up front. - if (onlySections.size > 0) { - const excluded = [...requiredSections].filter((name) => !onlySections.has(name)); - if (excluded.length > 0) { - const quoted = quoteList(excluded); - const [noun, pronoun] = - excluded.length === 1 ? (["entry", "it"] as const) : (["entries", "them"] as const); - return { - error: - `the "required-sections" ${noun} ${quoted} ${excluded.length === 1 ? "is" : "are"} ` + - `excluded by the "sections" allowlist, so the run would pass without ever attempting ` + - `${pronoun}. Add ${pronoun} to the "sections" input, or remove ${pronoun} from ` + - `"required-sections"`, - }; - } - } - return { requiredSections, onlySections }; -} - -/** - * 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 a loud error. - */ -function resolveReportPublicKey(channel: PrivateReportChannel): string | { error: string } { - const key = input("report-public-key"); - if (channel !== "artifact") { - if (key) { - return { - error: `the "report-public-key" input only applies to private-report: artifact, but the channel is "${channel}", so the key would never be used. Remove report-public-key, or set private-report: artifact`, - }; - } - return ""; - } - if (!key) { - return { - error: - 'private-report: artifact needs a "report-public-key" input: the age recipient every ' + - 'report is encrypted to. Generate a keypair with "age-keygen -o key.txt", keep key.txt ' + - 'secret, and set report-public-key to the printed "age1..." recipient (safe to commit)', - }; - } - const parsed = parseRecipient(key); - if (!parsed.ok) { - return { - error: `the "report-public-key" input is not a valid age recipient: ${parsed.error}. It must be an "age1..." public key from "age-keygen" (the recipient line, not the AGE-SECRET-KEY identity)`, - }; - } - return key; -} - -/** - * 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. - */ -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. - */ -const MERGE_INPUTS = [ - "mode", - "settings-file", - "merged-file", - "layering", - "token", -] 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. - */ -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(): { config: Extract } | { error: string } { - const rejected = MERGE_REJECTED_INPUTS.filter((name) => { - const value = input(name); - return value !== "" && value !== INPUT_DECLS[name].default; - }); - if (rejected.length > 0) { - return { - error: - `the ${quoteList(rejected)} input(s) do not apply to mode: merge, which only folds the ` + - `settings-file layers into merged-file: it never targets a repository, calls the GitHub ` + - `API, delivers a report, or narrows the sections it writes. Remove the input(s), or move ` + - `them to the apply or check step that runs the merged document`, - }; - } - const mergedFile = input("merged-file"); - if (!mergedFile) { - return { - error: - 'mode: merge needs a "merged-file" input: the path the merged settings document is written to. Set it (for example .github/settings.merged.yml) and feed that path to a later apply or check step as its settings-file', - }; - } - const layering = readEnum("layering", LAYERINGS, "merge", "layering"); - if (typeof layering !== "string") { - return { error: layering.error }; - } - const settingsFiles = splitList(inputOrDefault("settings-file")); - if (settingsFiles.length === 0) { - return { - error: `the "settings-file" input is "${inputOrDefault("settings-file")}", which lists no file. In mode: merge it is the ordered list of layers to fold, newline- or comma-separated, lowest first; name at least one settings file`, - }; - } - return { config: { kind: "merge", settingsFiles, mergedFile, layering } }; -} - -/** Read and validate every input; the first problem wins. */ -export function parseConfig(): { config: RunConfig } | { error: string } { - // 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. - const mode = readEnum("mode", MODES, INPUT_DECLS.mode.default, "mode"); - if (typeof mode !== "string") { - return { error: mode.error }; - } - if (mode === "merge") { - return parseMergeConfig(); - } - const mergeOnly = MERGE_ONLY_INPUTS.filter((name) => input(name) !== ""); - if (mergeOnly.length > 0) { - return { - error: `the ${quoteList(mergeOnly)} input(s) only apply to mode: merge, but this run is in ${mode} mode, so ${mergeOnly.length === 1 ? "it" : "they"} would never be used. Remove the input(s), or set mode: merge to fold settings files`, - }; - } - const token = input("token") || process.env.GITHUB_TOKEN || ""; - if (!token) { - return { - error: - 'cannot call the GitHub API: no token was provided. Set the "token" input on the action step (or export GITHUB_TOKEN)', - }; - } - // The workflow's own repository, read once and reused for the self slug, the - // run URL, the central-mode admin owner, and the single-repo fallback target. - const githubRepository = process.env.GITHUB_REPOSITORY ?? ""; - const onMissingPermission = readEnum( - "on-missing-permission", - ["fail", "warn"] as const, - INPUT_DECLS["on-missing-permission"].default, - "policy", - ); - if (typeof onMissingPermission !== "string") { - return { error: onMissingPermission.error }; - } - const sets = readSectionSets(); - if ("error" in sets) { - return { error: sets.error }; - } - const { requiredSections, onlySections } = sets; - const apiVersion = inputOrDefault("api-version"); - const privateRepos = readEnum( - "private-repos", - PRIVATE_REPOS_POLICIES, - INPUT_DECLS["private-repos"].default, - "private-repository policy", - ); - if (typeof privateRepos !== "string") { - return { error: privateRepos.error }; - } - const privateReport = readEnum( - "private-report", - PRIVATE_REPORT_CHANNELS, - INPUT_DECLS["private-report"].default, - "private-report channel", - ); - if (typeof privateReport !== "string") { - return { error: privateReport.error }; - } - // A report channel only ever runs for a REDACTED target, so combining it with - // private-repos: show (which redacts nothing) would silently deliver no - // report - a silent no-op violates the loud-failure promise, so reject it. - if (privateReport !== "none" && privateRepos === "show") { - return { - error: - 'the "private-report" input delivers reports only for redacted targets, but "private-repos" is "show", so nothing is redacted and no report would ever be sent. Set private-repos: redact, or set private-report: none', - }; - } - const reportPublicKey = resolveReportPublicKey(privateReport); - if (typeof reportPublicKey !== "string") { - return { error: reportPublicKey.error }; - } - const serverUrl = process.env.GITHUB_SERVER_URL ?? ""; - const runId = process.env.GITHUB_RUN_ID ?? ""; - const runUrl = - serverUrl && githubRepository && runId - ? `${serverUrl}/${githubRepository}/actions/runs/${runId}` - : ""; - const common: CommonConfig = { - token, - mode, - onMissingPermission, - requiredSections, - onlySections, - apiVersion, - privateRepos, - privateReport, - reportPublicKey, - selfSlug: githubRepository, - runUrl, - }; - - const discoveryFiltersSet = FILTER_INPUTS.filter((name) => input(name) !== ""); - const list = (name: FilterInput): string[] => splitList(input(name)); - const visibility = readEnum( - "visibility", - VISIBILITY_FILTERS, - DEFAULT_DISCOVERY_FILTERS.visibility, - "discovery filter", - ); - if (typeof visibility !== "string") { - return { error: visibility.error }; - } - const archived = readEnum( - "archived", - ARCHIVED_FILTERS, - DEFAULT_DISCOVERY_FILTERS.archived, - "archived-repository policy", - ); - if (typeof archived !== "string") { - return { error: archived.error }; - } - const forks = readEnum("forks", FORKS_FILTERS, DEFAULT_DISCOVERY_FILTERS.forks, "fork policy"); - if (typeof forks !== "string") { - return { error: forks.error }; - } - const affiliation = [...new Set(list("affiliation"))]; - for (const entry of affiliation) { - if (!(AFFILIATIONS as readonly string[]).includes(entry)) { - return { - error: `the "affiliation" input entry "${entry}" is not a supported affiliation, so discovery cannot build the /user/repos query. Use a comma-separated list of ${AFFILIATIONS.map((a) => `"${a}"`).join(", ")}`, - }; - } - } - const exclude = list("exclude"); - for (const pattern of exclude) { - const parts = pattern.split("/"); - if (parts.length > 2 || (parts.length === 2 && (!parts[0] || !parts[1]))) { - return { - error: - `the "exclude" input pattern "${pattern}" can never match an owner/name repository: a ` + - `pattern takes at most one "/", with a non-empty glob on each side of it. Use ` + - `"" or "/", where "*" matches any characters`, - }; - } - } - const discoveryFilters: DiscoveryFilters = { - visibility, - archived, - forks, - affiliation: affiliation.length > 0 ? affiliation : DEFAULT_DISCOVERY_FILTERS.affiliation, - topics: list("topics").map((topic) => topic.toLowerCase()), - exclude, - }; - - const reposInput = input("repos"); - const reposDir = input("repos-dir"); - const defaultsFile = input("defaults-file"); - const settingsFile = inputOrDefault("settings-file"); - - if (reposInput || reposDir) { - // Multi-repo mode: the single-repo inputs make no sense here. - if (input("repository")) { - return { - error: - 'the "repository" input cannot be combined with "repos" or "repos-dir"; multi-repo targets come from those inputs. Remove "repository", or remove the multi-repo inputs to stay in single-repo mode', - }; - } - if (settingsFile !== DEFAULT_SETTINGS_FILE) { - return { - error: - 'the "settings-file" input cannot be combined with "repos" or "repos-dir": central targets are read from repos-dir files and remote targets from each repository\'s own .github/settings.yml. Remove the settings-file override', - }; - } - const adminOwner = githubRepository.split("/")[0] ?? ""; - return { - config: { - ...common, - kind: "multi", - reposDir, - reposInput, - defaultsFile, - adminOwner, - discoveryFilters, - discoveryFiltersSet, - }, - }; - } - - // Single-repo mode (unchanged legacy behavior). - if (discoveryFiltersSet.length > 0) { - return { - error: `the discovery filter input(s) ${quoteList(discoveryFiltersSet)} only apply to repos: "*" discovery, but this run is in single-repo mode. Set repos: "*" to discover repositories, or remove the filter input(s)`, - }; - } - if (defaultsFile) { - return { - error: - 'the "defaults-file" input only applies to multi-repo mode, but this run is in single-repo mode, so the defaults would never apply. Remove the input, or add "repos" or "repos-dir" to switch to multi-repo mode', - }; - } - // Only a merge folds a list; the engine modes read exactly one file, so - // even a stray separator ("only.yml,") is rejected rather than repaired. - if (LIST_SEPARATOR.test(settingsFile)) { - return { - error: - `the "settings-file" input is "${settingsFile}", which contains a list separator: ` + - `${mode} mode reads exactly one settings file, and only mode: merge takes a newline- or ` + - `comma-separated list. Name one file, or set mode: merge to fold the list into one ` + - `document`, - }; - } - const rawRepo = input("repository") || githubRepository; - const repo = parseRepoSlug(rawRepo); - if (repo === null) { - return { - error: `cannot target a repository: "${rawRepo}" is not an owner/name slug. Set the "repository" input (or GITHUB_REPOSITORY) to a value like "octocat/hello-world"`, - }; - } - return { config: { ...common, kind: "single", repo, settingsFile } }; + return parseConfig((name) => core.getInput(name), process.env); } diff --git a/src/action/run.ts b/src/action/run.ts index 503c900a..b0871311 100644 --- a/src/action/run.ts +++ b/src/action/run.ts @@ -22,6 +22,7 @@ import { type ArtifactUploader, + concludeMerge, concludeRun, failRun, GithubApi, @@ -32,7 +33,7 @@ import { runSingle, } from "../index.js"; import { actionsArtifactUploader } from "./artifact.js"; -import { parseConfig } from "./inputs.js"; +import { parseActionConfig } from "./inputs.js"; import { actionsIo } from "./io.js"; /** @@ -47,28 +48,29 @@ export async function run(overrides?: { const io = overrides?.io ?? actionsIo; const uploader = overrides?.uploader ?? actionsArtifactUploader; - const parsed = parseConfig(); - if ("error" in parsed) { + const parsed = parseActionConfig(); + if (parsed.isErr()) { return failRun(io, parsed.error); } - const cfg = parsed.config; + 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); + return runMerge(cfg, io).match( + (merged) => concludeMerge(io, merged), + (problem) => failRun(io, problem), + ); } const api = overrides?.api ?? new GithubApi({ token: cfg.token, io, apiVersion: cfg.apiVersion }); if (cfg.kind === "multi") { - const { fatal, targets } = await runMulti(api, cfg, io, uploader); - if (fatal) { - return failRun(io, fatal); - } - return concludeRun(io, { kind: "multi", mode: cfg.mode, targets }); + return runMulti(api, cfg, io, uploader).match( + (targets) => concludeRun(io, { kind: "multi", mode: cfg.mode, targets }), + (problem) => failRun(io, problem), + ); } - const single = await runSingle(api, cfg, io, uploader); - if ("fatal" in single) { - return failRun(io, single.fatal); - } - return concludeRun(io, { kind: "single", mode: cfg.mode, target: single.target }); + return runSingle(api, cfg, io, uploader).match( + (target) => concludeRun(io, { kind: "single", mode: cfg.mode, target }), + (problem) => failRun(io, problem), + ); } diff --git a/src/discovery/central.ts b/src/discovery/central.ts index bc3d72b5..ed7db6fd 100644 --- a/src/discovery/central.ts +++ b/src/discovery/central.ts @@ -5,6 +5,8 @@ import { existsSync, readdirSync, statSync } from "node:fs"; import { join } from "node:path"; +import { err, ok, type Result } from "neverthrow"; +import type { CentralFileProblem, ProblemOf } from "../problem.js"; import { type CentralTarget, SLUG_RE } from "./targets.js"; const YAML_EXT = /\.ya?ml$/; @@ -16,32 +18,29 @@ const YAML_EXT = /\.ya?ml$/; export function resolveCentralTargets( reposDir: string, adminOwner: string, -): { targets: CentralTarget[]; warnings: string[] } | { error: string } { +): Result< + { targets: CentralTarget[]; warnings: string[] }, + ProblemOf<"repos-dir-missing" | "repos-dir-unreadable" | "repos-dir-invalid-files"> +> { if (!existsSync(reposDir)) { - return { - error: `repos-dir "${reposDir}" does not exist in the workspace, so there are no central settings files to read. Add an actions/checkout step before this action, or fix the repos-dir path`, - }; + return err({ code: "repos-dir-missing", reposDir }); } 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. - const errors: string[] = []; + const errors: CentralFileProblem[] = []; const seen = new Map(); // lowercased slug -> origin const addTarget = (slug: string, filePath: string): void => { if (!SLUG_RE.test(slug)) { - errors.push( - `${filePath} resolves to the target "${slug}", which is not a valid owner/name slug. Rename the file so and contain only letters, digits, dots, underscores, and dashes`, - ); + errors.push({ kind: "not-a-slug", filePath, slug }); return; } const key = slug.toLowerCase(); const existing = seen.get(key); if (existing) { - errors.push( - `duplicate target ${slug}: defined by both ${existing} and ${filePath}. Keep exactly one settings file per repository`, - ); + errors.push({ kind: "duplicate", slug, first: existing, second: filePath }); return; } seen.set(key, filePath); @@ -90,19 +89,13 @@ export function resolveCentralTargets( addTarget(`${adminOwner}/${entry.replace(YAML_EXT, "")}`, entryPath); } if (ownerlessFiles.length > 0) { - errors.push( - `cannot resolve ${ownerlessFiles.join(", ")}: top-level repos-dir files use the current repository's owner, which is unknown outside GitHub Actions. Use the /.yml layout instead`, - ); + errors.push({ kind: "ownerless", files: ownerlessFiles }); } } catch (error) { - return { - error: `cannot read repos-dir "${reposDir}": ${String(error)}. Check that it is a readable directory of settings files`, - }; + return err({ code: "repos-dir-unreadable", reposDir, reason: String(error) }); } if (errors.length > 0) { - return { - error: `repos-dir "${reposDir}" has ${errors.length} invalid settings file(s):\n- ${errors.join("\n- ")}`, - }; + return err({ code: "repos-dir-invalid-files", reposDir, files: errors }); } - return { targets, warnings }; + return ok({ targets, warnings }); } diff --git a/src/discovery/discover.ts b/src/discovery/discover.ts index 5daa6942..4cc9c085 100644 --- a/src/discovery/discover.ts +++ b/src/discovery/discover.ts @@ -5,10 +5,12 @@ */ import type { components } from "@octokit/openapi-types"; -import { type GithubClient, isPermissionError, RERUN_ADVICE } from "../github/api.js"; +import { err, ok, ResultAsync } from "neverthrow"; +import { type GithubClient, isPermissionError } from "../github/api.js"; import { paginate } from "../github/paginate.js"; import { isPrivate, markPrivate, type Private } from "../private.js"; import { revealPrivate } from "../private-open.js"; +import type { ProblemOf } from "../problem.js"; type DiscoveredRepo = Pick< components["schemas"]["repository"], @@ -114,11 +116,16 @@ export interface DiscoveryResult { 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 async function discoverRepos( +export function discoverRepos( api: GithubClient, filters: DiscoveryFilters, -): Promise { +): ResultAsync { const params = [`affiliation=${filters.affiliation.join(",")}`]; if (filters.visibility === "public" || filters.visibility === "private") { // The API's visibility param has no "internal" value; that case (and the @@ -126,38 +133,42 @@ export async function discoverRepos( params.push(`visibility=${filters.visibility}`); } const path = `/user/repos?${params.join("&")}`; - const wrap = (message: string, advice: string): { error: string } => ({ - error: `cannot discover repositories for repos: "*": ${message}. ${advice}`, - }); - let page: Awaited>; - try { - page = await paginate(api, path); - } catch (error) { - // Network-level failure: tryRequest throws once the retries are spent. - return wrap(error instanceof Error ? error.message : String(error), RERUN_ADVICE); - } - if ("error" in page) { - const cause = `GET ${path} failed: ${page.error.status} ${page.error.message}`; - // The PAT advice fits genuine denials only. A rate-limit 403 is NOT a - // permission problem (isPermissionError excludes it), so it must fall - // through to RERUN_ADVICE instead of telling the operator to swap tokens - // and abandon "*" discovery for nothing. 401 is an invalid/expired token, - // which the same PAT advice covers. - if (isPermissionError(page.error) || page.error.status === 401) { - return wrap( - cause, - `Discovery needs a user PAT; the workflow GITHUB_TOKEN and GitHub App installation tokens cannot enumerate a user's repositories. List the target repositories explicitly in the "repos" input`, - ); + // Network-level failure: tryRequest throws once the retries are spent. + return ResultAsync.fromPromise( + paginate(api, path), + (error): DiscoveryProblem => ({ + code: "discovery-transport-failed", + reason: error instanceof Error ? error.message : String(error), + }), + ).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. + return err({ + code: "discovery-request-failed", + path, + status: page.error.status, + message: page.error.message, + denied: isPermissionError(page.error) || page.error.status === 401, + }); } - return wrap(cause, RERUN_ADVICE); - } - if ("malformed" in page) { - return wrap( - `GET ${path} returned a JSON value that is not a list, so the response cannot be paginated`, - RERUN_ADVICE, - ); - } - const repos = page.items as DiscoveredRepo[]; + if ("malformed" in page) { + return err({ + code: "discovery-response-not-a-list", + path, + }); + } + return ok(applyFilters(page.items as DiscoveredRepo[], filters)); + }); +} + +/** 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> = [ diff --git a/src/discovery/repos-input.ts b/src/discovery/repos-input.ts index 3d6b63d5..4bd4f350 100644 --- a/src/discovery/repos-input.ts +++ b/src/discovery/repos-input.ts @@ -1,22 +1,25 @@ /** 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, -): { slugs: string[]; discover: boolean } | { error: string } { +): Result< + { slugs: string[]; discover: boolean }, + ProblemOf<"repos-input-wildcard-mixed" | "repos-input-invalid-entries"> +> { const items = raw .split(/[\n,]/) .map((s) => s.trim()) .filter(Boolean); if (items.includes("*")) { if (items.length > 1) { - return { - error: `the "repos" input mixes "*" with explicit repositories. Use "*" alone to discover every repository the token owns, or list the repositories without it`, - }; + return err({ code: "repos-input-wildcard-mixed" }); } - return { slugs: [], discover: true }; + 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 @@ -36,24 +39,11 @@ export function parseReposInput( seen.add(key); } if (invalid.size > 0 || duplicated.size > 0) { - const parts: string[] = []; - if (invalid.size > 0) { - parts.push( - `${[...invalid].map((item) => `"${item}"`).join(", ")} ` + - `${invalid.size === 1 ? "is not an owner/name slug" : "are not owner/name slugs"} ` + - '(use values like "octocat/hello-world", comma- or newline-separated)', - ); - } - if (duplicated.size > 0) { - parts.push( - `${[...duplicated].map((item) => `"${item}"`).join(", ")} ` + - `${duplicated.size === 1 ? "is" : "are"} listed more than once ` + - "(keep exactly one entry per repository)", - ); - } - return { - error: `the "repos" input has ${invalid.size + duplicated.size} invalid entr${invalid.size + duplicated.size === 1 ? "y" : "ies"}: ${parts.join("; ")}. Or use "*" alone to discover repositories`, - }; + return err({ + code: "repos-input-invalid-entries", + invalid: [...invalid], + duplicated: [...duplicated], + }); } - return { slugs: items, discover: false }; + return ok({ slugs: items, discover: false }); } diff --git a/src/discovery/targets.ts b/src/discovery/targets.ts index 28b298dd..44e1fa9f 100644 --- a/src/discovery/targets.ts +++ b/src/discovery/targets.ts @@ -5,6 +5,9 @@ * artifact; the remote file is self-service. */ +import { err, ok, type Result } from "neverthrow"; +import type { ProblemOf } from "../problem.js"; + interface TargetBase { slug: string; // owner/name, original casing /** Where this target came from, for messages: a file path or the input name. */ @@ -36,18 +39,18 @@ export interface RepoRef { } /** - * Parse an owner/name slug into a RepoRef, or null 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. + * 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. */ -export function parseRepoSlug(raw: string): RepoRef | null { +export function parseRepoSlug(raw: string): Result> { if (!SLUG_RE.test(raw)) { - return null; + return err({ code: "repo-slug-invalid", value: raw }); } const separator = raw.indexOf("/"); - return { owner: raw.slice(0, separator), name: raw.slice(separator + 1), slug: raw }; + return ok({ owner: raw.slice(0, separator), name: raw.slice(separator + 1), slug: raw }); } /** diff --git a/src/engine/layers.ts b/src/engine/layers.ts index 1d5f253c..89cf9532 100644 --- a/src/engine/layers.ts +++ b/src/engine/layers.ts @@ -13,10 +13,13 @@ * Pure: no Io, no GitHub. The helpers merge.ts shares live here too. */ +import { err, ok, type Result } from "neverthrow"; +import { isPlainObject } from "../plain-data.js"; +import type { LayerProblem } from "../problem.js"; import { UNDECLARED_POLICY_SECTIONS, type UndeclaredPolicySection } from "../schema.js"; import { defaultUndeclaredPolicy, type KeyedListLayering } from "../sections/contract/module.js"; import { sectionModule } from "../sections/registry.js"; -import type { UndeclaredPolicy } from "../types.js"; +import type { DistributiveOmit, UndeclaredPolicy } from "../types.js"; /** One settings document in the stack, named for notices and refusals. */ export interface Layer { @@ -42,22 +45,6 @@ function isLayering(value: unknown): value is Layering { return LAYERINGS.some((layering) => layering === value); } -/** - * A PLAIN mapping only: the prototype must be Object.prototype or null. A - * YAML explicit tag (!!timestamp, !!set) parses to a Date or Set, which is - * an object too - treating one as a mapping would spread it into `{}` and - * quietly hand the merge (or a knobbed-section normalization) a document - * nobody wrote. Non-plain objects REPLACE like scalars, surviving the merge - * as written for post-merge validation to reject. - */ -export function isPlainObject(value: unknown): value is Record { - if (typeof value !== "object" || value === null || Array.isArray(value)) { - return false; - } - const proto = Object.getPrototypeOf(value); - return proto === Object.prototype || proto === null; -} - /** * Rewrite each UNDECLARED_POLICY_SECTIONS value from the plain array form to * the wrapped one, PRESERVING OMISSION of the policy key - a plain array @@ -218,73 +205,26 @@ export function stripNulls(doc: unknown): unknown { return out; } -/** - * The kinds of problem the layer boundary refuses. Each carries only what - * its prose needs: the prose fragments are literal unions, the positions are - * numbers, and a document value enters only as `actual`, which the prose - * describes by SHAPE. The one free string, `keyField`, is passed from the - * module's declared key field, never read from a document. - */ -type Problem = - | { readonly kind: "cycle" } - | { - readonly kind: "wrong-shape"; - readonly expected: "a mapping" | "a list of mappings or an {_undeclared, entries} wrapper"; - readonly actual: unknown; - readonly detail?: " without an entries list"; - } - | { readonly kind: "bad-directive"; readonly actual: unknown } - | { readonly kind: "no-layering-key" } - | { readonly kind: "no-key"; readonly keyField: string } - | { - readonly kind: "duplicate-key"; - readonly keyField: string; - readonly first: number; - readonly second: number; - }; - -/** A layer the merge refuses at its boundary; worded by describeRefusal alone. */ -class LayerRefusal extends Error {} +/** A layer refusal minus its position: the boundary adds the layer and site where it fires. */ +type Refusal = DistributiveOmit; /** - * The ONE place a refusal is worded, and with describeOptOut the only prose - * the merge produces. INVARIANT: a message names the layer as the layer list - * names it, the site's key path, and the kind of problem - never a value from - * the document. mode: merge has no private-repos redaction context, so a - * value echoed here (a label name, a rule type, a mis-shaped section body) - * could land a private repository's settings in a public log. - * What the types enforce: a Problem's prose fragments are literal unions and - * its positions are numbers, so `actual` and `keyField` are the only fields - * that could carry a document string. What stays the code's invariant, - * pinned by the marker test in test/engine/layers.test.ts: `actual` reaches - * the prose only through describeShape; `keyField` is the module's declared - * key field; `site` is built from section keys, entry indices, + * 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". + * document". A document key or value never enters the prose. */ -function describeRefusal(layer: string, site: string, problem: Problem): string { - const at = `layer ${quote(layer)}: ${site}`; - switch (problem.kind) { - case "cycle": - return `${at} contains a reference cycle (a YAML anchor that includes itself); layers must be trees`; - case "wrong-shape": - return `${at} must be ${problem.expected}; got ${describeShape(problem.actual)}${problem.detail ?? ""}`; - case "bad-directive": - return `${at} must be "merge" or "replace"; got ${describeShape(problem.actual)}${typeof problem.actual === "string" ? " that is neither" : ""}`; - case "no-layering-key": - return `${at} has no layering key, so it cannot be layered by "merge"; declare ${LAYERING_KEY}: replace or drop the directive`; - case "no-key": - return `${at} carries no string ${quote(problem.keyField)}, which every entry needs to layer by`; - case "duplicate-key": - return `${at}[${problem.first}] and ${site}[${problem.second}] both claim one ${problem.keyField}; each ${problem.keyField} belongs to one entry within a layer`; - } +function refuse(layer: string, site: string, refusal: Refusal): Result { + return err({ layer, site, ...refusal }); } -function refuse(layer: string, site: string, problem: Problem): never { - throw new LayerRefusal(describeRefusal(layer, site, problem)); -} - -/** An opt-out notice's prose; value-free under the same invariant as describeRefusal. */ +/** + * 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). + */ export function describeOptOut(notice: OptOutNotice): string { return `${notice.layer}: null removed ${notice.path} declared by a lower layer`; } @@ -313,27 +253,6 @@ interface AdmittedLayer { readonly sections: ReadonlyMap; } -function quote(value: unknown): string { - return JSON.stringify(String(value)); -} - -/** A value's kind for refusal prose: what it is, not what it contains. */ -function describeShape(value: unknown): string { - if (value === null) { - return "null"; - } - if (Array.isArray(value)) { - return "a list"; - } - if (isPlainObject(value)) { - return "a mapping"; - } - if (typeof value === "object") { - return `a ${value.constructor?.name ?? "tagged"} value`; - } - return `a ${typeof value}`; -} - /** The entries as mappings, or null when one is not. */ function asMappings(list: readonly unknown[]): readonly Readonly>[] | null { return list.every(isPlainObject) ? list : null; @@ -347,14 +266,14 @@ function admitEntries( layer: string, path: string, list: readonly unknown[], -): readonly Readonly>[] { +): Result>[], LayerProblem> { const mappings = asMappings(list); if (mappings !== null) { - return mappings; + return ok(mappings); } const index = list.findIndex((entry) => !isPlainObject(entry)); return refuse(layer, `${path}[${index}]`, { - kind: "wrong-shape", + code: "layer-wrong-shape", expected: "a mapping", actual: list[index], }); @@ -373,18 +292,18 @@ function checkKeyed( entries: readonly Readonly>[], keyed: KeyedListLayering, path: string, -): void { +): Result { const seen = new Map(); - entries.forEach((entry, index) => { + for (const [index, entry] of entries.entries()) { const keys = keyed.keys(entry); if (keys === null) { - refuse(layer, `${path}[${index}]`, { kind: "no-key", keyField: keyed.keyField }); + return refuse(layer, `${path}[${index}]`, { code: "layer-no-key", keyField: keyed.keyField }); } for (const key of keys) { const first = seen.get(key); if (first !== undefined) { - refuse(layer, path, { - kind: "duplicate-key", + return refuse(layer, path, { + code: "layer-duplicate-key", keyField: keyed.keyField, first, second: index, @@ -398,24 +317,33 @@ function checkKeyed( continue; } const nestedPath = `${path}[${index}].${field}`; - checkKeyed(layer, admitEntries(layer, nestedPath, value), nested, nestedPath); + const checked = admitEntries(layer, nestedPath, value).andThen((mappings) => + checkKeyed(layer, mappings, nested, nestedPath), + ); + if (checked.isErr()) { + return checked; + } } - }); + } + 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>): Layering | undefined { +function fileLayering( + layer: string, + doc: Readonly>, +): Result { const value = doc[LAYERING_KEY]; if (value === undefined) { - return undefined; + return ok(undefined); } if (!isLayering(value)) { - refuse(layer, LAYERING_KEY, { kind: "bad-directive", actual: value }); + return refuse(layer, LAYERING_KEY, { code: "layer-bad-directive", actual: value }); } - return value; + return ok(value); } /** One knobbed section past the boundary, or a refusal naming the layer and the section. */ @@ -424,29 +352,33 @@ function admitSection( key: UndeclaredPolicySection, value: unknown, fallback: { readonly file: Layering | undefined; readonly run: Layering }, -): AdmittedSection { +): Result { if (!isPlainObject(value) || !Array.isArray(value.entries)) { - refuse(layer, key, { - kind: "wrong-shape", + return refuse(layer, key, { + code: "layer-wrong-shape", expected: "a list of mappings or an {_undeclared, entries} wrapper", actual: value, detail: isPlainObject(value) ? " without an entries list" : undefined, }); } - const entries = admitEntries(layer, key, value.entries); - const { entries: _entries, [LAYERING_KEY]: directive, ...knobs } = value; - if (directive !== undefined && !isLayering(directive)) { - refuse(layer, `${key}.${LAYERING_KEY}`, { kind: "bad-directive", actual: directive }); - } - const explicit = directive ?? fallback.file; - const keyed = sectionModule(key).layering; - if (keyed === undefined && explicit === "merge") { - refuse(layer, key, { kind: "no-layering-key" }); - } - if (keyed !== undefined) { - checkKeyed(layer, entries, keyed, key); - } - return { knobs, entries, layering: explicit ?? fallback.run, keyed }; + return admitEntries(layer, key, value.entries).andThen((entries) => { + const { entries: _entries, [LAYERING_KEY]: directive, ...knobs } = value; + if (directive !== undefined && !isLayering(directive)) { + return refuse(layer, `${key}.${LAYERING_KEY}`, { + code: "layer-bad-directive", + actual: directive, + }); + } + const explicit = directive ?? fallback.file; + const keyed = sectionModule(key).layering; + if (keyed === undefined && explicit === "merge") { + return refuse(layer, key, { code: "layer-no-layering-key" }); + } + const section: AdmittedSection = { knobs, entries, layering: explicit ?? fallback.run, keyed }; + return keyed === undefined + ? ok(section) + : checkKeyed(layer, entries, keyed, key).map(() => section); + }); } /** @@ -480,23 +412,29 @@ function hasCycle(value: unknown, descent: WeakSet, walked: WeakSet { if (hasCycle(layer.doc, new WeakSet(), new WeakSet())) { - refuse(layer.name, "the document", { kind: "cycle" }); + return refuse(layer.name, "the document", { code: "layer-cycle" }); } const doc = normalizeKnobbedSections(layer.doc); if (!isPlainObject(doc)) { - return null; + return ok(null); } - const file = fileLayering(layer.name, doc); - const sections = new Map(); - for (const key of UNDECLARED_POLICY_SECTIONS) { - const value = doc[key]; - if (value !== undefined && value !== null) { - sections.set(key, admitSection(layer.name, key, value, { file, run })); + return fileLayering(layer.name, doc).andThen((file) => { + const sections = new Map(); + for (const key of UNDECLARED_POLICY_SECTIONS) { + const value = doc[key]; + if (value === undefined || value === null) { + continue; + } + const admitted = admitSection(layer.name, key, value, { file, run }); + if (admitted.isErr()) { + return err(admitted.error); + } + sections.set(key, admitted.value); } - } - return { name: layer.name, doc, sections }; + return ok({ name: layer.name, doc, sections }); + }); } function childPath(path: string, key: string): string { @@ -709,27 +647,26 @@ function mergeStep(acc: unknown, layer: AdmittedLayer, notices: OptOutNotice[]): * 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 an error naming it. + * refuses comes back as the problem naming it. */ export function mergeLayers( layers: readonly Layer[], options: { readonly layering: Layering }, -): { settings: unknown; notices: OptOutNotice[] } | { error: string } { +): Result<{ settings: unknown; notices: OptOutNotice[] }, LayerProblem> { const notices: OptOutNotice[] = []; let acc: unknown = {}; - try { - for (const layer of layers) { - const admitted = admit(layer, options.layering); - acc = admitted === null ? structuredClone(layer.doc) : mergeStep(acc, admitted, notices); - } - } catch (error) { - if (error instanceof LayerRefusal) { - return { error: error.message }; + for (const layer of layers) { + const admitted = admit(layer, options.layering); + if (admitted.isErr()) { + return err(admitted.error); } - throw error; + acc = + admitted.value === null + ? structuredClone(layer.doc) + : mergeStep(acc, admitted.value, notices); } if (isPlainObject(acc)) { resolveUndeclaredPolicies(acc); } - return { settings: acc, notices }; + return ok({ settings: acc, notices }); } diff --git a/src/engine/orchestrate.ts b/src/engine/orchestrate.ts index def8a2d4..5daadbe3 100644 --- a/src/engine/orchestrate.ts +++ b/src/engine/orchestrate.ts @@ -4,9 +4,11 @@ * All output goes through the Io sink; callers decide how (or whether) to tag lines per repository. */ +import { err, type Result } from "neverthrow"; import type { RepoRef } from "../discovery/targets.js"; import type { GithubClient } from "../github/api.js"; import type { Io } from "../io.js"; +import type { SettingsProblem, TopLevelShape } from "../problem.js"; import { SECTION_KEYS, type SectionKey, type SettingsFile } from "../schema.js"; import { PermissionDenied } from "../sections/contract/errors.js"; import { @@ -20,6 +22,7 @@ import type { MustBeNever } from "../types.js"; import { executePlan } from "./execute.js"; import { resolveSecretRefs, type SettingsSource, validateSecretRef } from "./secret-refs.js"; import { collectSecretValues, type SectionSecretValue } from "./secrets.js"; +import type { SectionSelection } from "./section-selection.js"; import { validateSectionShapes } from "./validate.js"; /** @@ -62,8 +65,8 @@ export interface RepoRunOptions { settings: ValidatedSettings; mode: "apply" | "check"; onMissingPermission: "fail" | "warn"; - requiredSections: ReadonlySet; - onlySections: ReadonlySet; + /** 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 @@ -118,20 +121,22 @@ 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 an error message (caller fails the run or the repo); the - * sections-allowlist case downgrades to a warning. `sourceLabel` names the - * file the message points at. + * 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. */ export function validateSettingsDoc( settings: unknown, sourceLabel: string, onlySections: ReadonlySet, io: Io, -): { settings: ValidatedSettings } | { error: string } { +): Result { if (typeof settings !== "object" || settings === null || Array.isArray(settings)) { - return { - error: `${sourceLabel} must be a YAML mapping of section names to settings, but its top level parsed as ${Array.isArray(settings) ? "a list" : `a ${settings === null ? "null" : typeof settings}`}. Rewrite the top level as "section: ..." keys`, - }; + return err({ + code: "settings-not-mapping", + source: sourceLabel, + 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 @@ -140,9 +145,7 @@ export function validateSettingsDoc( // to section values. const proto = Object.getPrototypeOf(settings); if (proto !== Object.prototype && proto !== null) { - return { - error: `${sourceLabel} must be a plain YAML mapping of section names to settings, but its top level parsed as another type (a YAML-tagged value like !!timestamp parses to a Date). Rewrite the top level as "section: ..." keys`, - }; + 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 @@ -157,9 +160,12 @@ export function validateSettingsDoc( ); if (unknownKeys.length > 0) { if (onlySections.size === 0 || unknownKeys.some((key) => allowed.has(key))) { - return { - error: `unknown top-level section(s) in ${sourceLabel}: ${unknownKeys.join(", ")} (known: ${SECTION_KEYS.join(", ")}). Fix the typo, or prefix private keys with "_", or set the "sections" input to limit processing`, - }; + return err({ + code: "settings-unknown-sections", + source: sourceLabel, + unknown: unknownKeys, + 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 @@ -169,14 +175,21 @@ export function validateSettingsDoc( `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}`, ); } - const parsed = validateSectionShapes(settings as Record, sourceLabel); - if ("error" in parsed) { - return { error: parsed.error }; - } // 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 { settings: parsed.settings as ValidatedSettings }; + return validateSectionShapes(settings as Record, sourceLabel).map( + (parsed) => parsed as ValidatedSettings, + ); +} + +/** A non-mapping document's top level in typeof terms; the only object left by the caller's guard is null. */ +function nonMappingShape(value: unknown): TopLevelShape { + if (Array.isArray(value)) { + return "list"; + } + const kind = typeof value; + return kind === "object" ? "null" : kind; } /** @@ -232,7 +245,7 @@ export async function runForRepo( if (settings[key] === undefined) { return "absent"; // declared-keys-only: absent section = untouched } - if (opts.onlySections.size > 0 && !opts.onlySections.has(key)) { + if (opts.sections.only.size > 0 && !opts.sections.only.has(key)) { return "excluded"; } return "active"; @@ -406,7 +419,7 @@ export async function runForRepo( } const before = [...produced.notes, ...produced.changes]; if (error instanceof PermissionDenied) { - const required = opts.requiredSections.has(section.key); + 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. const landed = produced.landed; diff --git a/src/engine/section-selection.ts b/src/engine/section-selection.ts new file mode 100644 index 00000000..334b66a9 --- /dev/null +++ b/src/engine/section-selection.ts @@ -0,0 +1,50 @@ +/** + * Which sections a run processes and which of them must fully apply. The + * pair is validated ONCE here: a required section outside a non-empty + * allowlist would let the run pass green having proven nothing about it, + * because the engine reports excluded sections without attempting them. The + * constructor is private and the fields are too, so neither a literal nor a + * spread of an existing selection typechecks as one (a spread drops private + * members): every selection the engine sees passed `of`, whether it came from + * the action's inputs or a library caller. + */ + +import { err, ok, type Result } from "neverthrow"; +import type { ProblemOf } from "../problem.js"; +import type { SectionKey } from "../schema.js"; + +export class SectionSelection { + /** The selection that processes every declared section and requires none. */ + static readonly ALL = new SectionSelection(new Set(), new Set()); + + private constructor( + private readonly onlyKeys: ReadonlySet, + private readonly requiredKeys: ReadonlySet, + ) {} + + /** The only public way in; a refusal names the required sections the allowlist excludes. */ + static of(input: { + only?: Iterable; + required?: Iterable; + }): Result> { + const only = new Set(input.only ?? []); + const required = new Set(input.required ?? []); + if (only.size > 0) { + const excluded = [...required].filter((key) => !only.has(key)); + if (excluded.length > 0) { + return err({ code: "required-sections-excluded", excluded }); + } + } + return ok(new SectionSelection(only, required)); + } + + /** The allowlist; empty means every declared section is processed. */ + get only(): ReadonlySet { + return this.onlyKeys; + } + + /** The sections that must fully apply even under on-missing-permission: warn. */ + get required(): ReadonlySet { + return this.requiredKeys; + } +} diff --git a/src/engine/validate.ts b/src/engine/validate.ts index 8c07a916..6a2d92d1 100644 --- a/src/engine/validate.ts +++ b/src/engine/validate.ts @@ -4,7 +4,9 @@ * applies. Closed sections and strict nested shapes reject unknown keys here. */ +import { err, ok, type Result } from "neverthrow"; import { nonPlainKind } from "../plain-data.js"; +import type { ProblemOf } from "../problem.js"; import { SECTION_KEYS, type SettingsFile } from "../schema.js"; import { sectionModule, sectionShape } from "../sections/registry.js"; @@ -54,12 +56,12 @@ function findNonPlain(value: unknown, path: string, seen: WeakSet): stri /** * 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 an error naming the source file and what to fix. + * the shape describes) or the problem naming the source file and what to fix. */ export function validateSectionShapes( settings: Record, sourceLabel: string, -): { settings: SettingsFile } | { error: string } { +): Result> { const problems: string[] = []; const parsedSections: Record = {}; for (const key of SECTION_KEYS) { @@ -96,15 +98,9 @@ export function validateSectionShapes( parsedSections[key] = parsed.data; } if (problems.length === 0) { - return { settings: parsedSections as SettingsFile }; + return ok(parsedSections as SettingsFile); } - return { - error: - `${sourceLabel} has malformed section entries: ${problems.join("; ")}. Fix these values in ` + - `the settings file (only the named keys are validated; extra fields pass through, except ` + - `in closed sections and strict nested objects like actions.cache, which reject ` + - `unrecognized keys)`, - }; + return err({ code: "settings-malformed-sections", source: sourceLabel, issues: problems }); } /** diff --git a/src/flows/deliver.ts b/src/flows/deliver.ts index 141a2aed..d440d2fa 100644 --- a/src/flows/deliver.ts +++ b/src/flows/deliver.ts @@ -4,6 +4,7 @@ * decided here, so the two flows cannot drift in what they report. */ +import { err, ok, type Result } from "neverthrow"; import type { RepoRef } from "../discovery/targets.js"; import { type RepoResult, @@ -12,19 +13,19 @@ import { skippedSectionKeys, worstOf, } from "../engine/orchestrate.js"; +import type { SectionSelection } from "../engine/section-selection.js"; import type { GithubClient } from "../github/api.js"; import type { RepoVisibility } from "../github/repo-visibility.js"; import type { Io } from "../io.js"; import { isPrivate } from "../private.js"; +import { describeProblem, type Problem, type ProblemOf } from "../problem.js"; import type { ArtifactUploader } from "../report/artifact-report.js"; import { - ARTIFACT_NEEDS_UPLOADER, isIssueChannel, openReportChannel, type PrivateReportChannel, type RunConclusion, } from "../report/delivery.js"; -import type { SectionKey } from "../schema.js"; import { emitRedactedResult, isPrivateVisibility, @@ -36,7 +37,7 @@ import { toPublicView, WITHHELD_REPORT_NOTICE, } from "./redact.js"; -import { writeMultiSummary, writeSummary } from "./summary.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 { @@ -110,23 +111,22 @@ export interface DeliveryConfig { /** 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"; - requiredSections: Set; - onlySections: Set; + sections: SectionSelection; /** Whether to hide private/internal targets from the public view. */ privateRepos: PrivateReposPolicy; } /** - * The fatal message when the `artifact` channel has no upload port, null otherwise. - * Both flows check it before any API work; openReportChannel asserts the same rule. + * The `artifact` channel cannot open without an upload port. Both flows check + * this before any API work; openReportChannel asserts the same rule. */ -export function missingUploaderProblem( +export function requireUploader( cfg: Pick, uploader: ArtifactUploader | undefined, -): string | null { +): Result> { return cfg.privateReport === "artifact" && uploader === undefined - ? ARTIFACT_NEEDS_UPLOADER - : null; + ? err({ code: "artifact-uploader-missing" }) + : ok(); } /** @@ -246,10 +246,12 @@ export function concludeRun(io: Io, run: FinishedRun): number { /** * A run that failed before any target ran (bad inputs, an unreadable settings - * file, a fatal multi-repo setup error): the error line, then the conclusion a - * failed target gets. There is no summary, since nothing ran. + * 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. */ -export function failRun(io: Io, message: string): number { +export function failRun(io: Io, problem: Problem): number { + const message = describeProblem(problem); io.annotate("error", message); // The mode may be unknown here (a config error); a failure exits 1 under either. return conclude(io, [failedTarget(message)], false); @@ -262,8 +264,15 @@ export function failRun(io: Io, message: string): number { */ export const MERGE_RESULT = "merged"; -/** A finished mode: merge run: the outputs and result line a merge earns, exit 0. */ -export function concludeMerge(io: Io, run: { layers: string[]; mergedFile: string }): number { +/** 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}`); io.output("skipped-sections", ""); io.output("result", MERGE_RESULT); diff --git a/src/flows/inputs.ts b/src/flows/inputs.ts new file mode 100644 index 00000000..96a7f1f6 --- /dev/null +++ b/src/flows/inputs.ts @@ -0,0 +1,671 @@ +/** + * 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. + */ + +import { err, ok, type Result, safeTry } from "neverthrow"; +import { + AFFILIATIONS, + ARCHIVED_FILTERS, + DEFAULT_DISCOVERY_FILTERS, + type DiscoveryFilters, + FORKS_FILTERS, + VISIBILITY_FILTERS, +} from "../discovery/discover.js"; +import { parseRepoSlug } from "../discovery/targets.js"; +import type { Layering } from "../engine/layers.js"; +import { SectionSelection } from "../engine/section-selection.js"; +import { DEFAULT_API_VERSION } from "../github/api.js"; +import type { Problem } from "../problem.js"; +import { parseRecipient } from "../report/artifact-report.js"; +import { PRIVATE_REPORT_CHANNELS, type PrivateReportChannel } from "../report/delivery.js"; +import { SECTION_KEYS, type SectionKey } from "../schema.js"; +import type { MustBeNever } from "../types.js"; +import type { RunFlowConfig } from "./deliver.js"; +import type { MergeConfig } from "./merge.js"; +import { DEFAULT_SETTINGS_FILE, type MultiConfig } from "./multi.js"; +import { PRIVATE_REPOS_POLICIES, type PrivateReposPolicy } from "./redact.js"; +import type { SingleConfig } from "./single.js"; + +/** Default `private-repos`, pinned against action.yml by the contract test. */ +export const DEFAULT_PRIVATE_REPOS = "redact" satisfies PrivateReposPolicy; + +/** Default `private-report`, pinned against action.yml by the contract test. */ +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. + */ +export interface InputDecl { + /** The action.yml description; the generator folds it to width. */ + readonly description: string; + /** The action.yml default, verbatim (an empty string means "unset"). */ + readonly default: string; + /** 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). + */ + 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. + */ +export const INPUT_DECLS = { + token: { + description: + "Token used for the API calls. Most sections need a fine-grained PAT with Administration read/write on the repository - the default GITHUB_TOKEN can never hold that permission.", + // biome-ignore lint/suspicious/noTemplateCurlyInString: a workflow expression the runner resolves, not a JS template + default: "${{ github.token }}", + summary: "Token for the API calls (see [Token permissions](docs/reference/permissions.md))", + shownDefault: "`github.token`", + }, + repository: { + description: + "Target repository (owner/name). Defaults to the current repository. Single-repo mode only; cannot be combined with repos or repos-dir.", + default: "", + summary: "Target `owner/name` (single-repo mode only)", + shownDefault: "current repo", + }, + "settings-file": { + description: + "Path to the settings YAML file: exactly one in apply and check. In mode: merge, the ordered " + + "list of settings files to fold instead, newline- or comma-separated, lowest layer first. " + + "Newlines and commas are list separators in every mode, so a settings-file path can never " + + "contain a comma. Single-repo and merge modes only; multi-repo targets read repos-dir files " + + "or each repository's own .github/settings.yml, so overriding it alongside repos or " + + "repos-dir fails the run.", + default: DEFAULT_SETTINGS_FILE, + summary: + "Settings file path (single-repo mode); in `mode: merge`, the ordered list of layers to fold, low to high", + }, + mode: { + description: + "apply (mutate), check (report drift, exit 1 on any), or merge (fold the settings-file " + + "layers into one document written to merged-file, with no token and no GitHub API call; " + + "merge reads only settings-file, merged-file, and layering, ignores token, and rejects every " + + "other input set to a non-default value, since each controls an apply or check run). check " + + "makes no settings changes, though a private report may still be delivered.", + default: "apply", + summary: + "`apply` mutates; `check` reports drift and exits 1 on any, making no settings changes (a private report may still be delivered); `merge` folds the settings-file layers into merged-file without touching GitHub", + }, + "merged-file": { + description: + "mode: merge only, and required there: the path the merged settings document is written to " + + "(parent directories are created). The file holds exactly what apply would run: every " + + "section validated, each section that takes an undeclared policy in its policy-wrapper form " + + "with the policy made explicit, the other sections in their own shape, and private " + + "underscore keys and the _layering directives dropped. Feed it to a later apply or check " + + "step as its settings-file. Must not name one of the settings-file layers (the merge would " + + "overwrite it). Fails when set in apply or check.", + default: "", + summary: + "`mode: merge` only (required there): where the merged document is written, exactly what `apply` would run", + }, + "on-missing-permission": { + description: + "fail (default) or warn. Under warn, sections the token cannot access are skipped with a warning and the run stays green (partial success).", + default: "fail", + summary: "`warn` skips sections the token cannot access (partial success)", + }, + "required-sections": { + description: + "Comma-separated section names that must fully apply even under on-missing-permission: " + + 'warn (minimum requirements). Every name must also be allowed by the "sections" input ' + + "when that allowlist is set; a required section the allowlist excludes is rejected up " + + "front, because the run could never attempt it.", + default: "", + summary: "Sections that must fully apply even under `warn`", + }, + sections: { + description: + "Optional comma-separated allowlist of sections to process. apply and check only: mode: merge writes every section its layers declare, so the allowlist belongs on the step that runs the merged document and fails the merge when set.", + default: "", + summary: + "Comma-separated allowlist of sections to process (apply and check only; rejected in `mode: merge`)", + shownDefault: "(all declared)", + }, + "api-version": { + description: + "X-GitHub-Api-Version header value. Override to opt into a newer REST API version before this action defaults to it.", + default: DEFAULT_API_VERSION, + summary: "`X-GitHub-Api-Version` header; override to opt into a newer REST API version", + }, + repos: { + description: + "Multi-repo remote mode: comma- or newline-separated owner/name targets, each applied " + + 'from its own .github/settings.yml (default branch), or "*" alone to discover every ' + + "repository the token's user owns, filterable via the visibility, archived, forks, " + + "exclude, topics, and affiliation inputs. Combinable with repos-dir; a repos-dir file " + + "for the same repository wins.", + default: "", + summary: + "Multi-repo remote mode: `owner/name` list (comma/newline), or `*` to discover owned repos", + }, + "repos-dir": { + description: + "Multi-repo central mode: a directory in the checked-out admin repository holding per-repo settings files - .yml (same owner as this repository) or /.yml. Requires actions/checkout.", + default: "", + summary: "Multi-repo central mode: directory of per-repo settings files in this repo", + }, + "defaults-file": { + description: + "YAML settings document applied to every multi-repo target that has no settings file of " + + "its own (a repos target without .github/settings.yml, which is otherwise skipped). A " + + "target with its own file is applied as written; the defaults are never merged into it. " + + 'With repos: "*" every discovered repository without a settings file receives the ' + + "defaults; run mode: check first. Multi-repo mode only; fails when set without repos or " + + "repos-dir.", + default: "", + summary: + "YAML applied to every multi-repo target without a settings file (multi-repo mode only)", + }, + layering: { + description: + "mode: merge only: merge (default) or replace, the run-wide default for how the keyed list " + + "sections (labels, rulesets) combine with the layers below them; a layer's own _layering " + + "directive, at its top level or on a section's {entries} wrapper, overrides it per file or " + + "per section. Every other list is replaced by the higher layer's. Fails when set in apply " + + "or check.", + default: "", + summary: + "`mode: merge` only: `merge` unions the keyed list sections (labels, rulesets) by key across layers, `replace` lets the higher layer's list win; a layer's `_layering` overrides it", + shownDefault: "`merge`", + }, + "private-repos": { + description: + "redact (default) or show. Under redact, private and internal targets are hidden from " + + "this run's public logs, summary, and outputs: their slug becomes a \"private repository " + + '#N" placeholder, live values and error bodies are replaced with "hidden (private ' + + "repository)\", and each slug is registered with the runner's secret masker. A target " + + "equal to GITHUB_REPOSITORY is never redacted. show reveals everything (today's " + + "behavior); only use it when the run's logs are not publicly readable.", + default: DEFAULT_PRIVATE_REPOS, + summary: + "`redact` hides private and internal targets from public logs, summary, and outputs; `show` reveals them", + }, + "private-report": { + description: + "none (default), issue, issue-on-failure, or artifact. Delivers the full unredacted " + + "report only for redacted targets the visibility probe proves private or internal (an " + + "unknown visibility is redacted but excluded from delivery). Under issue, each such " + + "target's report is delivered as a reused, marker-labelled issue on that target " + + "repository itself (the one GitHub-private channel a public run has): the body is " + + "replaced every run, and the issue is opened when the target fails or drifts and closed " + + "when it is healthy. issue-on-failure is the quiet variant: a failing or drifting target " + + "gets the same issue, but a healthy run only closes a still-open issue from a previous " + + "failure and otherwise writes nothing - no issue ever appears on a repository that never " + + "needed attention (though a declared labels section still creates the marker label, and " + + "a manually-removed marker label defers the close: the next failing run reattaches it, " + + "and the first healthy run after that closes the issue). Under artifact, those reports " + + "are concatenated, age-encrypted to report-public-key, and uploaded as one workflow " + + "artifact (settings-as-code-private-report) for readers who hold the key but no GitHub " + + "access to the targets; the artifact channel needs the Actions artifact service, so on " + + "GitHub Enterprise Server it warns and uploads nothing. Applies only to redacted " + + "targets, so it is rejected alongside private-repos: show. Report delivery writes even " + + "in mode: check, and its failure never changes the run's result.", + default: DEFAULT_PRIVATE_REPORT, + summary: + "`issue` delivers each redacted target's full report to a reused issue on that target " + + "repository; `issue-on-failure` writes that issue only when the target fails or drifts, " + + "closing it once healthy; `artifact` uploads all reports as one age-encrypted workflow " + + "artifact; rejected with `private-repos: show`", + }, + "report-public-key": { + description: + 'The age recipient (an "age1..." public key) the artifact channel encrypts every report ' + + 'to; safe to commit in the workflow. Generate a keypair with "age-keygen -o key.txt", ' + + 'keep key.txt secret, and decrypt a downloaded artifact with "age -d -i key.txt ' + + 'private-report.md.age". Required when private-report is artifact and rejected otherwise.', + default: "", + summary: + "The `age1...` recipient the `artifact` channel encrypts reports to; required with `private-report: artifact`, rejected otherwise", + }, + visibility: { + description: + 'Keeps only repositories of this visibility in repos: "*" discovery. One of all (default), public, private, or internal; internal is matched client-side (Enterprise only). Fails if set without repos: "*".', + default: "", + summary: "Discovery-only: keep `public`, `private`, or `internal` repositories", + shownDefault: `\`${DEFAULT_DISCOVERY_FILTERS.visibility}\``, + }, + archived: { + description: + 'Archived-repository policy for repos: "*" discovery. One of skip (default; settings writes fail on archived repositories), include, or only (mostly useful with mode: check). Fails if set without repos: "*".', + default: "", + summary: "Discovery-only: `skip`, `include`, or `only` archived repositories", + shownDefault: `\`${DEFAULT_DISCOVERY_FILTERS.archived}\``, + }, + forks: { + description: + 'Fork policy for repos: "*" discovery. One of include (default), exclude, or only. Fails if set without repos: "*".', + default: "", + summary: "Discovery-only: `include`, `exclude`, or `only` forks", + shownDefault: `\`${DEFAULT_DISCOVERY_FILTERS.forks}\``, + }, + exclude: { + description: + 'Comma- or newline-separated wildcard patterns removing repositories from repos: "*" ' + + 'discovery. "*" matches any characters; a pattern containing "/" matches the full ' + + 'owner/name, any other the name alone. Case-insensitive. Fails if set without repos: "*".', + default: "", + summary: + "Discovery-only: `*` wildcard patterns (name, or `owner/name` if the pattern has a `/`) to drop", + }, + topics: { + description: + 'Comma- or newline-separated topics; repos: "*" discovery keeps only repositories carrying at least one of them. Unrelated to the topics settings section. Fails if set without repos: "*".', + default: "", + summary: "Discovery-only: keep repositories carrying at least one listed topic", + }, + affiliation: { + description: + 'Comma-separated affiliations for repos: "*" discovery, passed to the GitHub /user/repos ' + + "listing. Any of owner, collaborator, organization_member; the list replaces the default " + + "(owner), so use owner,collaborator to widen rather than move discovery. Fails if set " + + 'without repos: "*".', + default: "", + summary: "Discovery-only: `owner`, `collaborator`, `organization_member` (comma list)", + shownDefault: `\`${DEFAULT_DISCOVERY_FILTERS.affiliation.join(",")}\``, + }, +} as const satisfies Record; + +export type InputName = keyof typeof INPUT_DECLS; + +/** The port parseConfig reads inputs through: the raw value, empty when unset; parseConfig trims it. */ +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. + */ +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; +} + +function inputs(read: InputReader): Inputs { + // The runner's getInput trims; a CLI's port may not. Trimming here gives every port one rule. + const value: InputReader = (name) => read(name).trim(); + 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", + "forks", + "exclude", + "topics", + "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, + allowed: readonly T[], + fallback: T, + noun: string, +): Result { + const value = input.value(name) || fallback; + const match = allowed.find((candidate) => candidate === value); + if (match === undefined) { + return err({ code: "input-unsupported-value", input: name, value, noun, allowed, fallback }); + } + return ok(match); +} + +/** 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) + .map((s) => s.trim()) + .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. + */ +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) => ({ + input: name, + names: splitList(input.value(name)), + })); + const knownSections = new Set(SECTION_KEYS); + const unknown = names + .map(({ input: name, names: listed }) => ({ + input: name, + names: [...new Set(listed)].filter((entry) => !knownSections.has(entry)), + })) + .filter((entry) => entry.names.length > 0); + 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. + */ +function resolveReportPublicKey( + input: Inputs, + channel: PrivateReportChannel, +): Result { + const key = input.value("report-public-key"); + if (channel !== "artifact") { + return key ? err({ code: "input-report-key-unused", channel }) : ok(""); + } + if (!key) { + return err({ code: "input-report-key-missing" }); + } + return parseRecipient(key) + .map(() => key) + .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. + */ +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. + */ +export const MERGE_INPUTS = [ + "mode", + "settings-file", + "merged-file", + "layering", + "token", +] 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. + */ +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) => { + const value = input.value(name); + return value !== "" && value !== INPUT_DECLS[name].default; + }); + if (rejected.length > 0) { + return err({ code: "input-rejected-in-merge", inputs: rejected }); + } + const mergedFile = input.value("merged-file"); + if (!mergedFile) { + return err({ code: "input-merged-file-missing" }); + } + const layering = yield* readEnum(input, "layering", LAYERINGS, "merge", "layering"); + const settingsFiles = splitList(input.orDefault("settings-file")); + if (settingsFiles.length === 0) { + return err({ + code: "input-settings-file-empty", + value: input.orDefault("settings-file"), + }); + } + return ok({ kind: "merge", settingsFiles, mergedFile, layering }); + }); +} + +/** Read and validate every input through `read`; the first problem wins. */ +export function parseConfig(read: InputReader, env: ConfigEnv): 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. + const mode = yield* readEnum(input, "mode", MODES, INPUT_DECLS.mode.default, "mode"); + if (mode === "merge") { + return parseMergeConfig(input); + } + const mergeOnly = MERGE_ONLY_INPUTS.filter((name) => input.value(name) !== ""); + if (mergeOnly.length > 0) { + return err({ code: "input-merge-only", inputs: mergeOnly, mode }); + } + const token = input.value("token") || env.GITHUB_TOKEN || ""; + if (!token) { + return err({ code: "input-token-missing" }); + } + // The workflow's own repository, read once and reused for the self slug, the + // run URL, the central-mode admin owner, and the single-repo fallback target. + const githubRepository = env.GITHUB_REPOSITORY ?? ""; + const onMissingPermission = yield* readEnum( + input, + "on-missing-permission", + ["fail", "warn"] as const, + INPUT_DECLS["on-missing-permission"].default, + "policy", + ); + const sections = yield* readSectionSelection(input); + const apiVersion = input.orDefault("api-version"); + const privateRepos = yield* readEnum( + input, + "private-repos", + PRIVATE_REPOS_POLICIES, + INPUT_DECLS["private-repos"].default, + "private-repository policy", + ); + const privateReport = yield* readEnum( + input, + "private-report", + PRIVATE_REPORT_CHANNELS, + INPUT_DECLS["private-report"].default, + "private-report channel", + ); + // A report channel only ever runs for a REDACTED target, so combining it with + // private-repos: show (which redacts nothing) would silently deliver no + // report - a silent no-op violates the loud-failure promise, so reject it. + if (privateReport !== "none" && privateRepos === "show") { + return err({ code: "input-report-without-redaction" }); + } + const reportPublicKey = yield* resolveReportPublicKey(input, privateReport); + const serverUrl = env.GITHUB_SERVER_URL ?? ""; + const runId = env.GITHUB_RUN_ID ?? ""; + const runUrl = + serverUrl && githubRepository && runId + ? `${serverUrl}/${githubRepository}/actions/runs/${runId}` + : ""; + const common: CommonConfig = { + token, + mode, + onMissingPermission, + sections, + apiVersion, + privateRepos, + privateReport, + reportPublicKey, + selfSlug: githubRepository, + runUrl, + }; + + const discoveryFiltersSet = FILTER_INPUTS.filter((name) => input.value(name) !== ""); + const list = (name: FilterInput): string[] => splitList(input.value(name)); + const visibility = yield* readEnum( + input, + "visibility", + VISIBILITY_FILTERS, + DEFAULT_DISCOVERY_FILTERS.visibility, + "discovery filter", + ); + const archived = yield* readEnum( + input, + "archived", + ARCHIVED_FILTERS, + DEFAULT_DISCOVERY_FILTERS.archived, + "archived-repository policy", + ); + const forks = yield* readEnum( + input, + "forks", + FORKS_FILTERS, + DEFAULT_DISCOVERY_FILTERS.forks, + "fork policy", + ); + const affiliation = [...new Set(list("affiliation"))]; + const unsupported = affiliation.find( + (entry) => !(AFFILIATIONS as readonly string[]).includes(entry), + ); + if (unsupported !== undefined) { + return err({ + code: "input-affiliation-unsupported", + entry: unsupported, + allowed: AFFILIATIONS, + }); + } + const exclude = list("exclude"); + const unmatchable = exclude.find((pattern) => { + const parts = pattern.split("/"); + return parts.length > 2 || (parts.length === 2 && (!parts[0] || !parts[1])); + }); + if (unmatchable !== undefined) { + return err({ code: "input-exclude-pattern-invalid", pattern: unmatchable }); + } + const discoveryFilters: DiscoveryFilters = { + visibility, + archived, + forks, + affiliation: affiliation.length > 0 ? affiliation : DEFAULT_DISCOVERY_FILTERS.affiliation, + topics: list("topics").map((topic) => topic.toLowerCase()), + exclude, + }; + + const reposInput = input.value("repos"); + const reposDir = input.value("repos-dir"); + const defaultsFile = input.value("defaults-file"); + const settingsFile = input.orDefault("settings-file"); + + if (reposInput || reposDir) { + // Multi-repo mode: the single-repo inputs make no sense here. + if (input.value("repository")) { + return err({ code: "input-repository-with-multi" }); + } + if (settingsFile !== DEFAULT_SETTINGS_FILE) { + return err({ code: "input-settings-file-with-multi" }); + } + const adminOwner = githubRepository.split("/")[0] ?? ""; + return ok({ + ...common, + kind: "multi", + reposDir, + reposInput, + defaultsFile, + adminOwner, + discoveryFilters, + discoveryFiltersSet, + }); + } + + // Single-repo mode (unchanged legacy behavior). + if (discoveryFiltersSet.length > 0) { + return err({ + code: "discovery-filters-without-wildcard", + filters: discoveryFiltersSet, + targets: "single-repo", + }); + } + if (defaultsFile) { + return err({ code: "input-defaults-file-without-multi" }); + } + // Only a merge folds a list; the engine modes read exactly one file, so + // even a stray separator ("only.yml,") is rejected rather than repaired. + if (LIST_SEPARATOR.test(settingsFile)) { + return err({ code: "input-settings-file-is-list", value: settingsFile, mode }); + } + const rawRepo = input.value("repository") || githubRepository; + const repo = yield* parseRepoSlug(rawRepo).mapErr( + (): Problem => ({ code: "input-repository-not-slug", value: rawRepo }), + ); + return ok({ ...common, kind: "single", repo, settingsFile }); + }); +} diff --git a/src/flows/layers.ts b/src/flows/layers.ts index 81916346..481b5c9c 100644 --- a/src/flows/layers.ts +++ b/src/flows/layers.ts @@ -4,8 +4,8 @@ * (engine/layers.ts), and validate the result. Nothing here reaches GitHub. */ +import { ok, Result } from "neverthrow"; import { - isPlainObject, type Layer, type Layering, mergeLayers, @@ -14,22 +14,22 @@ import { } from "../engine/layers.js"; import { type ValidatedSettings, validateSettingsDoc } from "../engine/orchestrate.js"; import type { Io } from "../io.js"; +import { isPlainObject } from "../plain-data.js"; +import type { LayerProblem, ProblemOf, SettingsProblem } from "../problem.js"; import { SECTION_KEYS, type SectionKey, UNDECLARED_POLICY_SECTIONS } from "../schema.js"; import { readSettingsFile } from "./settings-read.js"; -/** Read and parse every layer, in order; the first unreadable path fails, named. */ -export function readLayerFiles(paths: readonly string[]): { layers: Layer[] } | { error: string } { - const layers: Layer[] = []; - for (const path of paths) { - const read = readSettingsFile(path); - if ("error" in read) { - return { - error: `cannot read the settings layer ${path}: ${read.error}. Check that every path in the "settings-file" input exists and is valid YAML`, - }; - } - layers.push({ name: path, doc: read.doc }); - } - return { layers }; +/** Read and parse every layer, in order; the first unreadable path fails, named, and the rest are not read. */ +export function readLayerFiles( + paths: readonly string[], +): Result> { + return paths.reduce>>( + (layers, path) => + layers.andThen((read) => + readSettingsFile(path, "layer").map((doc) => [...read, { name: path, doc }]), + ), + ok([]), + ); } const KNOWN_SECTIONS: ReadonlySet = new Set(SECTION_KEYS); @@ -79,20 +79,20 @@ export function foldLayers( sourceLabel: string, layering: Layering, io: Io, -): { settings: ValidatedSettings; notices: OptOutNotice[] } | { error: string } { - for (const layer of layers) { - const standalone = validateSettingsDoc(standaloneView(layer.doc), layer.name, NO_ALLOWLIST, io); - if ("error" in standalone) { - return { error: standalone.error }; - } - } - const merged = mergeLayers(layers, { layering }); - if ("error" in merged) { - return { error: merged.error }; - } - const validated = validateSettingsDoc(merged.settings, sourceLabel, NO_ALLOWLIST, io); - if ("error" in validated) { - return { error: validated.error }; - } - return { settings: validated.settings, notices: merged.notices }; +): Result< + { settings: ValidatedSettings; notices: OptOutNotice[] }, + SettingsProblem | LayerProblem +> { + return Result.combine( + layers.map((layer) => + validateSettingsDoc(standaloneView(layer.doc), layer.name, NO_ALLOWLIST, io), + ), + ) + .andThen(() => mergeLayers(layers, { layering })) + .andThen((merged) => + validateSettingsDoc(merged.settings, sourceLabel, NO_ALLOWLIST, io).map((settings) => ({ + settings, + notices: merged.notices, + })), + ); } diff --git a/src/flows/library.ts b/src/flows/library.ts index 0c3d1d51..bc955ef1 100644 --- a/src/flows/library.ts +++ b/src/flows/library.ts @@ -1,5 +1,11 @@ -/** The engine for a caller with no action inputs: the mode and the Io sink are fixed here. */ +/** + * 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 { type RepoRunOptions, @@ -10,6 +16,7 @@ import { } from "../engine/orchestrate.js"; import type { GithubClient } from "../github/api.js"; 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. */ @@ -21,22 +28,14 @@ const NO_ALLOWLIST: ReadonlySet = new Set(); export function validateSettings( doc: unknown, options: { source?: string; sections?: ReadonlySet } = {}, -): { ok: true; settings: ValidatedSettings; warnings: string[] } | { ok: false; error: string } { +): Result<{ settings: ValidatedSettings; warnings: string[] }, SettingsProblem> { const collected = collectingIo(); - const validated = validateSettingsDoc( + return validateSettingsDoc( doc, options.source ?? UNNAMED_SOURCE, options.sections ?? NO_ALLOWLIST, collected.io, - ); - if ("error" in validated) { - return { ok: false, error: validated.error }; - } - return { - ok: true, - settings: validated.settings, - warnings: collected.lines.map((entry) => entry.line), - }; + ).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. */ diff --git a/src/flows/merge.ts b/src/flows/merge.ts index 927b3a3c..8d24ff70 100644 --- a/src/flows/merge.ts +++ b/src/flows/merge.ts @@ -7,12 +7,13 @@ import { mkdirSync, writeFileSync } from "node:fs"; import { dirname, resolve } from "node:path"; +import { err, ok, type Result } from "neverthrow"; import { describeOptOut, type Layering } from "../engine/layers.js"; import type { Io } from "../io.js"; -import { concludeMerge, failRun } from "./deliver.js"; +import type { Problem, ProblemOf } from "../problem.js"; +import type { FinishedMerge } from "./deliver.js"; import { foldLayers, readLayerFiles } from "./layers.js"; import { renderMergedYaml } from "./library.js"; -import { writeMergeSummary } from "./summary.js"; /** * A mode: merge run: the layers to fold, low to high, and where the result @@ -30,50 +31,44 @@ export interface MergeConfig { const MERGED_LABEL = "the merged settings document"; /** - * The layer merged-file would overwrite, as a message, or null. 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. + * 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. */ -function mergedFileCollision(cfg: MergeConfig): string | null { +function mergedFileCollision(cfg: MergeConfig): Result> { const mergedPath = resolve(cfg.mergedFile); - const collision = cfg.settingsFiles.findIndex((layer) => resolve(layer) === mergedPath); - if (collision === -1) { - return null; - } - return ( - `the "merged-file" input "${cfg.mergedFile}" is layer ${collision + 1} of the ` + - `"settings-file" list ("${cfg.settingsFiles[collision]}"): the merge would overwrite that ` + - `layer with the folded document, and the next run would fold the merged document as a ` + - `layer. Write the merged document to a path outside the layer list` - ); + const index = cfg.settingsFiles.findIndex((layer) => resolve(layer) === mergedPath); + const layer = cfg.settingsFiles[index]; + return layer === undefined + ? ok() + : err({ code: "merged-file-is-layer", mergedFile: cfg.mergedFile, index, layer }); } -/** Execute a mode: merge run; returns the process exit code. */ -export function runMerge(cfg: MergeConfig, io: Io): number { - const collision = mergedFileCollision(cfg); - if (collision !== null) { - return failRun(io, collision); - } - const read = readLayerFiles(cfg.settingsFiles); - if ("error" in read) { - return failRun(io, read.error); - } - const folded = foldLayers(read.layers, MERGED_LABEL, cfg.layering, io); - if ("error" in folded) { - return failRun(io, folded.error); - } - for (const notice of folded.notices) { - io.annotate("notice", describeOptOut(notice)); - } - try { - mkdirSync(dirname(cfg.mergedFile), { recursive: true }); - writeFileSync(cfg.mergedFile, renderMergedYaml(folded.settings)); - } catch (error) { - return failRun( - io, - `cannot write the merged document to ${cfg.mergedFile}: ${String(error)}. Check that the "merged-file" input names a writable path`, - ); - } - writeMergeSummary(io, cfg.settingsFiles, cfg.mergedFile); - return concludeMerge(io, { layers: cfg.settingsFiles, mergedFile: cfg.mergedFile }); +/** + * Execute a mode: merge run: read, fold, and write. A problem before the + * document is written (a merged-file that is a layer, an unreadable layer, a + * refused fold, an unwritable path) comes back for the caller to render; the + * finished merge is what concludeMerge turns into the summary, the outputs, + * and the exit code. + */ +export function runMerge(cfg: MergeConfig, io: Io): Result { + return mergedFileCollision(cfg) + .andThen(() => readLayerFiles(cfg.settingsFiles)) + .andThen((layers) => foldLayers(layers, MERGED_LABEL, cfg.layering, io)) + .andThen((folded): Result => { + for (const notice of folded.notices) { + io.annotate("notice", describeOptOut(notice)); + } + try { + mkdirSync(dirname(cfg.mergedFile), { recursive: true }); + writeFileSync(cfg.mergedFile, renderMergedYaml(folded.settings)); + } catch (error) { + return err({ + code: "merged-file-unwritable", + path: cfg.mergedFile, + reason: String(error), + }); + } + return ok({ layers: cfg.settingsFiles, mergedFile: cfg.mergedFile }); + }); } diff --git a/src/flows/multi.ts b/src/flows/multi.ts index 3f8b14e3..ad466f3b 100644 --- a/src/flows/multi.ts +++ b/src/flows/multi.ts @@ -18,6 +18,7 @@ */ import { readFileSync } from "node:fs"; +import { type Err, err, ok, type ResultAsync, safeTry } from "neverthrow"; import { resolveCentralTargets } from "../discovery/central.js"; import { type DiscoveryFilters, discoverRepos, formatSkipNotice } from "../discovery/discover.js"; import { parseReposInput } from "../discovery/repos-input.js"; @@ -31,19 +32,20 @@ import { } from "../discovery/targets.js"; import { runForRepo, type ValidatedSettings, validateSettingsDoc } from "../engine/orchestrate.js"; import type { SettingsSource } from "../engine/secret-refs.js"; -import { type GithubClient, isPermissionError, RERUN_ADVICE } from "../github/api.js"; +import { type GithubClient, isPermissionError } from "../github/api.js"; import { getRepoFile } from "../github/repo-file.js"; import { createVisibilityResolver, type RepoVisibility } from "../github/repo-visibility.js"; import type { Io } from "../io.js"; import type { Private } from "../private.js"; +import { describeProblem, type Problem, RERUN_ADVICE } from "../problem.js"; import type { ArtifactUploader } from "../report/artifact-report.js"; import { applyMarkerInjection } from "../report/delivery.js"; import { engineOutcome, failedTarget, - missingUploaderProblem, type OpenedTarget, type RunFlowConfig, + requireUploader, type TargetResult, targetFailure, withDelivery, @@ -61,15 +63,10 @@ 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/action/inputs.ts, and the prose below. + * multi-repo override guard in src/flows/inputs.ts, and the prose below. */ export const DEFAULT_SETTINGS_FILE = ".github/settings.yml"; -/** Names as an error message lists them: each quoted, comma-separated. */ -export function quoteList(names: string[]): string { - return names.map((name) => `"${name}"`).join(", "); -} - export interface MultiConfig extends RunFlowConfig { reposDir: string; reposInput: string; @@ -125,8 +122,7 @@ async function processTarget(ctx: { settings: injected.settings, mode: cfg.mode, onMissingPermission: cfg.onMissingPermission, - requiredSections: cfg.requiredSections, - onlySections: cfg.onlySections, + sections: cfg.sections, secretSource, }, channel.io, @@ -161,23 +157,25 @@ async function processTarget(ctx: { } const parsed = parseSettingsDoc(read.raw); - if ("error" in parsed) { - return fail(`cannot parse ${read.sourceLabel}: ${parsed.error}. Fix the YAML in that file`); + if (parsed.isErr()) { + return fail( + `cannot parse ${read.sourceLabel}: ${parsed.error.reason}. Fix the YAML in that file`, + ); } // 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. const validated = validateSettingsDoc( - parsed.doc, + parsed.value, read.sourceLabel, - cfg.onlySections, + cfg.sections.only, channel.unprefixed, ); - if ("error" in validated) { - return fail(validated.error); + if (validated.isErr()) { + return fail(describeProblem(validated.error)); } - return run(validated.settings, read.source); + return run(validated.value, read.source); } /** @@ -192,7 +190,7 @@ function openTarget( visibilityOf: (slug: string) => RepoVisibility, ): OpenedTarget { return { - repo: parseRepoSlug(slug), + repo: parseRepoSlug(slug).unwrapOr(null), channel: openTargetChannel(plan, io, slug), exposure: plan.isRedacted(slug) ? { kind: "redacted", visibility: visibilityOf(slug) } @@ -250,22 +248,23 @@ async function readTargetSettings( /** * Multi-repo orchestration. Config-level problems (bad defaults file, no - * targets, duplicate definitions, discovery failure) return `fatal` before - * any target executes; per-target problems mark that target failed or + * 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 async function runMulti( +export function runMulti( api: GithubClient, cfg: MultiConfig, io: Io, uploader?: ArtifactUploader, -): Promise<{ fatal: string | null; targets: TargetOutcome[] }> { +): 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, 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. + // 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. const bufferedWarnings: string[] = []; let warningsFlushed = false; const flushWarnings = (): void => { @@ -277,193 +276,167 @@ export async function runMulti( io.annotate("warning", warning); } }; - const fail = (message: string): { fatal: string; targets: TargetOutcome[] } => { - flushWarnings(); - return { fatal: message, targets: [] }; - }; + // 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); - const noUploader = missingUploaderProblem(cfg, uploader); - if (noUploader !== null) { - return fail(noUploader); - } + return safeTry(async function* () { + yield* requireUploader(cfg, uploader); - let defaults: ValidatedSettings | null = null; - if (cfg.defaultsFile) { - const read = readSettingsFile(cfg.defaultsFile); - if ("error" in read) { - return fail( - `cannot read the defaults file ${cfg.defaultsFile}: ${read.error}. Check the "defaults-file" path and that the file is valid YAML`, - ); + let defaults: ValidatedSettings | null = null; + if (cfg.defaultsFile) { + const doc = yield* readSettingsFile(cfg.defaultsFile, "defaults-file"); + defaults = yield* validateSettingsDoc(doc, cfg.defaultsFile, cfg.sections.only, io); } - const validated = validateSettingsDoc(read.doc, cfg.defaultsFile, cfg.onlySections, io); - if ("error" in validated) { - return fail(validated.error); - } - defaults = validated.settings; - } - let central: CentralTarget[] = []; - if (cfg.reposDir) { - const resolved = resolveCentralTargets(cfg.reposDir, cfg.adminOwner); - if ("error" in resolved) { - return fail(resolved.error); + let central: CentralTarget[] = []; + if (cfg.reposDir) { + const resolved = yield* resolveCentralTargets(cfg.reposDir, cfg.adminOwner); + bufferedWarnings.push(...resolved.warnings); + central = resolved.targets; } - bufferedWarnings.push(...resolved.warnings); - central = resolved.targets; - } - let remote: RemoteTarget[] = []; - let filteredOutCount = 0; - const skipGroups: Array<{ - reason: string; - repos: Parameters[0]["repos"]; - }> = []; - // Visibility learned from discovery (authoritative for those repos), so the - // per-target probe is skipped for them. - const knownVisibility = new Map(); - // Private slugs that discovery filtered out (sealed): masked, never placeholdered. - const filteredPrivateSlugs: Private[] = []; - if (cfg.reposInput) { - const parsed = parseReposInput(cfg.reposInput); - if ("error" in parsed) { - return fail(parsed.error); - } - let slugs = parsed.slugs; - let origin = 'the "repos" input'; - if (parsed.discover) { - const discovered = await discoverRepos(api, cfg.discoveryFilters); - if ("error" in discovered) { - return fail(discovered.error); - } - for (const group of discovered.filtered) { - skipGroups.push(group); - filteredOutCount += group.repos.length; - for (const repo of group.repos) { - if (repo.visibility !== "public") { - filteredPrivateSlugs.push(repo.slug); + let remote: RemoteTarget[] = []; + let filteredOutCount = 0; + const skipGroups: Array<{ + reason: string; + repos: Parameters[0]["repos"]; + }> = []; + // Visibility learned from discovery (authoritative for those repos), so the + // per-target probe is skipped for them. + const knownVisibility = new Map(); + // Private slugs that discovery filtered out (sealed): masked, never placeholdered. + const filteredPrivateSlugs: Private[] = []; + if (cfg.reposInput) { + const parsed = yield* parseReposInput(cfg.reposInput); + let slugs = parsed.slugs; + let origin = 'the "repos" input'; + if (parsed.discover) { + const discovered = yield* discoverRepos(api, cfg.discoveryFilters); + for (const group of discovered.filtered) { + skipGroups.push(group); + filteredOutCount += group.repos.length; + for (const repo of group.repos) { + if (repo.visibility !== "public") { + filteredPrivateSlugs.push(repo.slug); + } } } + for (const repo of discovered.repos) { + knownVisibility.set(repo.slug.toLowerCase(), repo.visibility); + } + slugs = discovered.repos.map((repo) => repo.slug); + origin = 'repos: "*" discovery'; + } else if (cfg.discoveryFiltersSet.length > 0) { + return fail({ + code: "discovery-filters-without-wildcard", + filters: cfg.discoveryFiltersSet, + targets: "explicit-repos", + }); } - for (const repo of discovered.repos) { - knownVisibility.set(repo.slug.toLowerCase(), repo.visibility); - } - slugs = discovered.repos.map((repo) => repo.slug); - origin = 'repos: "*" discovery'; + remote = slugs.map((slug) => ({ slug, source: "remote" as const, origin })); } else if (cfg.discoveryFiltersSet.length > 0) { - return fail( - `the discovery filter input(s) ${quoteList(cfg.discoveryFiltersSet)} only apply when repos is "*", but the "repos" input lists explicit repositories. Set repos: "*", or remove the filter input(s)`, - ); + return fail({ + code: "discovery-filters-without-wildcard", + filters: cfg.discoveryFiltersSet, + targets: "repos-dir", + }); } - remote = slugs.map((slug) => ({ slug, source: "remote" as const, origin })); - } else if (cfg.discoveryFiltersSet.length > 0) { - return fail( - `the discovery filter input(s) ${quoteList(cfg.discoveryFiltersSet)} only apply to repos: "*" discovery, but targets come only from repos-dir files. Set repos: "*", or remove the filter input(s)`, - ); - } - const redact = cfg.privateRepos === "redact"; - const self = cfg.selfSlug.toLowerCase(); + 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. - const resolveVisibility = createVisibilityResolver(api); - const orderedSlugs = [...central, ...remote].map((t) => t.slug); - const visibilityBySlug = new Map(); - if (redact) { - for (const slug of orderedSlugs) { - const key = slug.toLowerCase(); - if (visibilityBySlug.has(key)) { - continue; - } - if (key === self) { - visibilityBySlug.set(key, "public"); - continue; + // 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. + const resolveVisibility = createVisibilityResolver(api); + const orderedSlugs = [...central, ...remote].map((t) => t.slug); + const visibilityBySlug = new Map(); + if (redact) { + for (const slug of orderedSlugs) { + const key = slug.toLowerCase(); + if (visibilityBySlug.has(key)) { + continue; + } + if (key === self) { + visibilityBySlug.set(key, "public"); + continue; + } + const known = knownVisibility.get(key); + visibilityBySlug.set(key, known ?? (await resolveVisibility(slug))); } - const known = knownVisibility.get(key); - 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". - const visibilityOf = (slug: string): RepoVisibility => - visibilityBySlug.get(slug.toLowerCase()) ?? "unknown"; - - const plan = planRedaction( - cfg.privateRepos, - orderedSlugs, - filteredPrivateSlugs, - (slug) => visibilityOf(slug) !== "public", - cfg.selfSlug, - ); + // 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". + const visibilityOf = (slug: string): RepoVisibility => + visibilityBySlug.get(slug.toLowerCase()) ?? "unknown"; - // Mask every hidden slug BEFORE the first annotate/log/output; the API - // trace reads the same registry. - for (const slug of plan.maskedSlugs) { - io.mask(slug); - } + const plan = planRedaction( + cfg.privateRepos, + orderedSlugs, + filteredPrivateSlugs, + (slug) => visibilityOf(slug) !== "public", + cfg.selfSlug, + ); - // Now safe to emit: buffered central warnings (flushed exactly once here on - // the happy path; the fail() helper flushes them on every fatal path), then - // the (redacting) skip notices. - flushWarnings(); - for (const group of skipGroups) { - io.annotate("notice", formatSkipNotice(group, redact)); - } + // Mask every hidden slug BEFORE the first annotate/log/output; the API + // trace reads the same registry. + for (const slug of plan.maskedSlugs) { + io.mask(slug); + } - const targets = dedupeTargets( - central, - remote, - (message) => io.annotate("notice", message), - (slug) => plan.display(slug), - (slug) => plan.isRedacted(slug), - ); - if (targets.length === 0) { - if (filteredOutCount > 0) { - return fail( - `multi-repo mode found no targets: repos: "*" discovery found ${filteredOutCount} ` + - `${filteredOutCount === 1 ? "repository" : "repositories"}, but the discovery filters ` + - `removed all of them (see the notices above). Relax the filter inputs, or add per-repo ` + - `files to the repos-dir`, - ); + // 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)); } - return fail( - `multi-repo mode found no targets: repos-dir yielded no settings files and the "repos" input resolved to no repositories. Add per-repo files to the repos-dir, or list repositories in the "repos" input`, - ); - } - 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. - 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. - const closed = await delivery.target(opened, async (injectMarker) => - repo === null - ? targetFailure( - channel.io, - `the repository name "${target.slug}" from ${target.origin} is not an owner/name slug, so it cannot be targeted`, - ) - : attempt( - channel, - () => processTarget({ api, target, repo, defaults, cfg, injectMarker, channel }), - failedTarget, - ), - ); - delivered.push({ source: target.source, ...closed }); + const targets = dedupeTargets( + central, + remote, + (message) => io.annotate("notice", message), + (slug) => plan.display(slug), + (slug) => plan.isRedacted(slug), + ); + if (targets.length === 0) { + return fail({ code: "no-targets", filteredOut: filteredOutCount }); } - return delivered; - }); - return { fatal: null, targets: results }; + 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. + 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. + const closed = await delivery.target(opened, async (injectMarker) => + repo === null + ? targetFailure( + channel.io, + `the repository name "${target.slug}" from ${target.origin} is not an owner/name slug, so it cannot be targeted`, + ) + : attempt( + channel, + () => processTarget({ api, target, repo, defaults, cfg, injectMarker, channel }), + failedTarget, + ), + ); + delivered.push({ source: target.source, ...closed }); + } + return delivered; + }); + + return ok(results); + }).orTee(flushWarnings); } diff --git a/src/flows/settings-read.ts b/src/flows/settings-read.ts index cc0d84a2..7e4de174 100644 --- a/src/flows/settings-read.ts +++ b/src/flows/settings-read.ts @@ -1,15 +1,17 @@ /** - * The one place YAML settings documents are read and parsed. Callers - * compose their own advice around the returned raw error string, because - * the right fix differs per source (defaults file, central file, single - * settings file, remote file). 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 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. */ import { readFileSync } from "node:fs"; +import { err, ok, type Result } from "neverthrow"; import { parse as parseYaml } from "yaml"; +import type { ProblemOf, SettingsFileRole } from "../problem.js"; /** * Parse one YAML settings document; empty/null documents become {}. @@ -22,21 +24,30 @@ import { parse as parseYaml } from "yaml"; * error still throws into the error path below; "silent" would also swallow * the errors and return a partial document. */ -export function parseSettingsDoc(raw: string): { doc: unknown } | { error: string } { +export function parseSettingsDoc(raw: string): Result> { try { - return { doc: parseYaml(raw, { logLevel: "error" }) ?? {} }; + return ok(parseYaml(raw, { logLevel: "error" }) ?? {}); } catch (error) { - return { error: String(error) }; + return err({ code: "yaml-invalid", reason: String(error) }); } } -/** Read and parse one settings file; the error covers both steps. */ -export function readSettingsFile(path: string): { doc: unknown } | { error: string } { +/** Read and parse one settings file; the problem covers both steps under the file's role. */ +export function readSettingsFile( + path: string, + role: SettingsFileRole, +): Result> { + const unreadable = (reason: string): ProblemOf<"settings-file-unreadable"> => ({ + code: "settings-file-unreadable", + role, + path, + reason, + }); let raw: string; try { raw = readFileSync(path, "utf8"); } catch (error) { - return { error: String(error) }; + return err(unreadable(String(error))); } - return parseSettingsDoc(raw); + return parseSettingsDoc(raw).mapErr((parse) => unreadable(parse.reason)); } diff --git a/src/flows/single.ts b/src/flows/single.ts index 9122a645..e961b530 100644 --- a/src/flows/single.ts +++ b/src/flows/single.ts @@ -7,19 +7,21 @@ * them when the target is a different, non-public repository. */ +import { ResultAsync } from "neverthrow"; import type { RepoRef } from "../discovery/targets.js"; -import { runForRepo, validateSettingsDoc } from "../engine/orchestrate.js"; +import { runForRepo, type ValidatedSettings, validateSettingsDoc } from "../engine/orchestrate.js"; import type { GithubClient } from "../github/api.js"; import { createVisibilityResolver } from "../github/repo-visibility.js"; import type { Io } from "../io.js"; +import type { Problem } from "../problem.js"; import type { ArtifactUploader } from "../report/artifact-report.js"; import { applyMarkerInjection } from "../report/delivery.js"; import { type Exposure, engineOutcome, failedTarget, - missingUploaderProblem, type RunFlowConfig, + requireUploader, withDelivery, } from "./deliver.js"; import { @@ -68,38 +70,42 @@ async function openSingleRepoChannel( }; } +/** The one target's outcome; the source is implied (the local settings file). */ +export type SingleOutcome = Omit; + /** * Run one repository from its settings file. A problem before the target runs * (no uploader for the artifact channel, an unreadable or invalid settings - * file) is `fatal`; otherwise the target's outcome, which concludeRun turns - * into the summary, the outputs, and the exit code. + * file) comes back as the error; otherwise the target's outcome, which + * concludeRun turns into the summary, the outputs, and the exit code. */ -export async function runSingle( +export function runSingle( api: GithubClient, cfg: SingleConfig, io: Io, uploader?: ArtifactUploader, -): Promise<{ fatal: string } | { target: Omit }> { - const noUploader = missingUploaderProblem(cfg, uploader); - if (noUploader !== null) { - return { fatal: noUploader }; - } - const read = readSettingsFile(cfg.settingsFile); - if ("error" in read) { - return { - fatal: `cannot read settings from ${cfg.settingsFile}: ${read.error}. Check that the file exists at that path (set the "settings-file" input if it lives elsewhere) and is valid YAML`, - }; - } - const validated = validateSettingsDoc(read.doc, cfg.settingsFile, cfg.onlySections, io); - if ("error" in validated) { - return { fatal: validated.error }; - } +): ResultAsync { + return requireUploader(cfg, uploader) + .andThen(() => readSettingsFile(cfg.settingsFile, "settings-file")) + .andThen((doc) => validateSettingsDoc(doc, cfg.settingsFile, cfg.sections.only, io)) + .asyncAndThen((settings) => + ResultAsync.fromSafePromise(runTarget(api, cfg, io, settings, uploader)), + ); +} +/** The target's run, past every check that could refuse it: open its channel, run, deliver. */ +async function runTarget( + api: GithubClient, + cfg: SingleConfig, + io: Io, + settings: ValidatedSettings, + uploader: ArtifactUploader | undefined, +): Promise { const opened = await openSingleRepoChannel(api, cfg, io); const { channel } = opened; - const target = await withDelivery({ api, cfg, io, uploader }, (delivery) => + return withDelivery({ api, cfg, io, uploader }, (delivery) => delivery.target({ repo: cfg.repo, ...opened }, (injectsMarker) => { - const injected = applyMarkerInjection(validated.settings, injectsMarker); + const injected = applyMarkerInjection(settings, injectsMarker); if (injected.notice) { channel.io.annotate("notice", injected.notice); } @@ -114,8 +120,7 @@ export async function runSingle( settings: injected.settings, mode: cfg.mode, onMissingPermission: cfg.onMissingPermission, - requiredSections: cfg.requiredSections, - onlySections: cfg.onlySections, + sections: cfg.sections, }, channel.io, ), @@ -125,5 +130,4 @@ export async function runSingle( ); }), ); - return { target }; } diff --git a/src/github/api.ts b/src/github/api.ts index d12753d7..66ceb332 100644 --- a/src/github/api.ts +++ b/src/github/api.ts @@ -49,16 +49,6 @@ export interface ApiError { graphqlTypes?: readonly string[]; } -/** - * The advice appended to a transient (non-permission) API failure: a network - * blip or a 5xx that survived the retries. The single source shared by the - * three transient-error builders (discovery's paginate failure and its - * non-permission status branch, plus multi.ts's remote-file read failure), so - * the "not a permission problem" wording cannot drift between them. - */ -export const RERUN_ADVICE = - "This is not a permission problem; re-run the workflow, and retry later if it persists"; - /** * Pinned X-GitHub-Api-Version. The single source for the header default * here, the action.yml `api-version` default, and the inputs fallback; the diff --git a/src/index.ts b/src/index.ts index 535a6d55..75bc85c9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -12,6 +12,7 @@ export { ARCHIVED_FILTERS, DEFAULT_DISCOVERY_FILTERS, type DiscoveryFilters, + type DiscoveryProblem, discoverRepos, FORKS_FILTERS, VISIBILITY_FILTERS, @@ -48,8 +49,31 @@ export { validateSettingsDoc, worstOf, } from "./engine/orchestrate.js"; +export { SectionSelection } from "./engine/section-selection.js"; // The run flows -export { concludeRun, failRun, MERGE_RESULT, type RunFlowConfig } from "./flows/deliver.js"; +export { + concludeMerge, + concludeRun, + type FinishedMerge, + failRun, + MERGE_RESULT, + type RunFlowConfig, +} from "./flows/deliver.js"; +export { + type ConfigEnv, + DEFAULT_PRIVATE_REPOS, + FILTER_INPUTS, + INPUT_DECLS, + type InputDecl, + type InputName, + type InputReader, + MERGE_INPUTS, + MERGE_REJECTED_INPUTS, + MODES, + type Mode, + parseConfig, + type RunConfig, +} from "./flows/inputs.js"; export { foldLayers, readLayerFiles } from "./flows/layers.js"; export { applyRepository, @@ -59,7 +83,7 @@ export { validateSettings, } from "./flows/library.js"; export { type MergeConfig, runMerge } from "./flows/merge.js"; -export { DEFAULT_SETTINGS_FILE, type MultiConfig, quoteList, runMulti } from "./flows/multi.js"; +export { DEFAULT_SETTINGS_FILE, type MultiConfig, runMulti } from "./flows/multi.js"; export { capturingIo, PRIVATE_REPOS_POLICIES, @@ -71,7 +95,7 @@ export { toPublicView, } from "./flows/redact.js"; export { parseSettingsDoc, readSettingsFile } from "./flows/settings-read.js"; -export { runSingle, type SingleConfig } from "./flows/single.js"; +export { runSingle, type SingleConfig, type SingleOutcome } from "./flows/single.js"; // The client export { type ApiError, @@ -81,7 +105,6 @@ export { type GraphqlOp, isPermissionError, isRateLimitError, - RERUN_ADVICE, type TraceIo, } from "./github/api.js"; export { getRepoFile } from "./github/repo-file.js"; @@ -98,6 +121,19 @@ export { prefixedIo, silentIo, } from "./io.js"; +// Problems +export { + type CentralFileProblem, + describeProblem, + type LayerProblem, + type Problem, + type ProblemOf, + quoteList, + RERUN_ADVICE, + type SettingsFileRole, + type SettingsProblem, + type TopLevelShape, +} from "./problem.js"; // The private report export { type ArtifactUploader, diff --git a/src/plain-data.ts b/src/plain-data.ts index abefa364..4a679970 100644 --- a/src/plain-data.ts +++ b/src/plain-data.ts @@ -1,11 +1,27 @@ /** - * The prose vocabulary for values that are not plain JSON/YAML data, shared - * by the two boundaries that reject them: upfront document validation - * (engine/validate.ts) and the secret-payload normalizer - * (github/secret-scan.ts). One prototype ladder, so the two boundaries can - * never describe the same tagged value differently. + * The vocabulary for values that are not plain JSON/YAML data: the one + * plain-mapping test, and the rejection prose shared by the boundaries that + * refuse tagged values (engine/validate.ts, github/secret-scan.ts). One + * prototype ladder, so no two boundaries describe the same tagged value + * differently. */ +/** + * A PLAIN mapping only: the prototype must be Object.prototype or null. A + * YAML explicit tag (!!timestamp, !!set) parses to a Date or Set, which is + * an object too - treating one as a mapping would spread it into `{}` and + * quietly hand the merge (or a knobbed-section normalization) a document + * nobody wrote. Non-plain objects REPLACE like scalars, surviving the merge + * as written for post-merge validation to reject. + */ +export function isPlainObject(value: unknown): value is Record { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return false; + } + const proto = Object.getPrototypeOf(value); + return proto === Object.prototype || proto === null; +} + /** * The value class of a non-plain value, for rejection prose. Prototype * comparison only - the same reflective read the callers already perform; diff --git a/src/problem.ts b/src/problem.ts new file mode 100644 index 00000000..912b1e9d --- /dev/null +++ b/src/problem.ts @@ -0,0 +1,479 @@ +/** + * The failures the run returns instead of throwing: config parsing, settings + * reading and validation, the layer fold, target resolution, and the flows' + * fatal paths. One union, discriminated on `code`, and the one renderer that + * words each member. A library caller branches on the code; the action + * renders at its edge, so its lines stay exactly what they were. + * Outside this union: the sections' API failures, which stay exceptions so + * the permission policy can classify them. + */ + +import { isPlainObject } from "./plain-data.js"; +import type { SectionKey } from "./schema.js"; + +/** + * The advice appended to a transient (non-permission) API failure: a network + * blip or a 5xx that survived the retries. One source for the discovery + * problems rendered here and multi.ts's remote-file read failure, so the "not + * a permission problem" wording cannot drift between them. + */ +export const RERUN_ADVICE = + "This is not a permission problem; re-run the workflow, and retry later if it persists"; + +/** Names as an error message lists them: each quoted, comma-separated. */ +export function quoteList(names: readonly string[]): string { + return names.map((name) => `"${name}"`).join(", "); +} + +/** The run modes that read exactly one settings file. */ +type EngineMode = "apply" | "check"; + +/** How a non-mapping settings document's top level reads, in typeof terms. */ +export type TopLevelShape = + | "list" + | "null" + | "undefined" + | "boolean" + | "number" + | "bigint" + | "string" + | "symbol" + | "function"; + +/** Which input named an unreadable settings file; each role's advice names its fix. */ +export type SettingsFileRole = "settings-file" | "defaults-file" | "layer"; + +/** One repos-dir file the central resolution cannot turn into a target. */ +export type CentralFileProblem = + | { readonly kind: "not-a-slug"; readonly filePath: string; readonly slug: string } + | { + readonly kind: "duplicate"; + readonly slug: string; + readonly first: string; + readonly second: string; + } + | { readonly kind: "ownerless"; readonly files: readonly string[] }; + +/** + * Every failure the run reports as data. The layer members carry only what + * their prose needs: the prose fragments are literal unions, the positions are + * numbers, and a document value enters only as `actual`, which the renderer + * describes by SHAPE. `keyField` is the module's declared key field, never + * read from a document. + */ +export type Problem = + // Action inputs + | { + readonly code: "input-unsupported-value"; + readonly input: string; + readonly value: string; + readonly noun: string; + readonly allowed: readonly string[]; + readonly fallback: string; + } + | { + readonly code: "input-unknown-sections"; + readonly unknown: ReadonlyArray<{ + readonly input: "required-sections" | "sections"; + readonly names: readonly string[]; + }>; + readonly known: readonly string[]; + } + | { readonly code: "input-report-key-unused"; readonly channel: string } + | { readonly code: "input-report-key-missing" } + | { readonly code: "input-report-key-invalid"; readonly reason: string } + | { readonly code: "input-rejected-in-merge"; readonly inputs: readonly string[] } + | { readonly code: "input-merged-file-missing" } + | { readonly code: "input-settings-file-empty"; readonly value: string } + | { + readonly code: "input-merge-only"; + readonly inputs: readonly string[]; + readonly mode: EngineMode; + } + | { readonly code: "input-token-missing" } + | { readonly code: "input-report-without-redaction" } + | { + readonly code: "input-affiliation-unsupported"; + readonly entry: string; + readonly allowed: readonly string[]; + } + | { readonly code: "input-exclude-pattern-invalid"; readonly pattern: string } + | { readonly code: "input-repository-with-multi" } + | { readonly code: "input-settings-file-with-multi" } + | { + readonly code: "discovery-filters-without-wildcard"; + readonly filters: readonly string[]; + /** Where the run's targets come from instead of `repos: "*"`. */ + readonly targets: "single-repo" | "explicit-repos" | "repos-dir"; + } + | { readonly code: "input-defaults-file-without-multi" } + | { + readonly code: "input-settings-file-is-list"; + readonly value: string; + readonly mode: EngineMode; + } + | { readonly code: "input-repository-not-slug"; readonly value: string } + // The settings document + | { + readonly code: "settings-not-mapping"; + readonly source: string; + readonly shape: TopLevelShape; + } + | { readonly code: "settings-not-plain-mapping"; readonly source: string } + | { + readonly code: "settings-unknown-sections"; + readonly source: string; + readonly unknown: readonly string[]; + readonly known: readonly string[]; + } + | { + readonly code: "settings-malformed-sections"; + readonly source: string; + readonly issues: readonly string[]; + } + // Settings sources + | { readonly code: "yaml-invalid"; readonly reason: string } + | { + readonly code: "settings-file-unreadable"; + readonly role: SettingsFileRole; + readonly path: string; + readonly reason: string; + } + // The layer boundary + | { readonly code: "layer-cycle"; readonly layer: string; readonly site: string } + | { + readonly code: "layer-wrong-shape"; + readonly layer: string; + readonly site: string; + readonly expected: "a mapping" | "a list of mappings or an {_undeclared, entries} wrapper"; + readonly actual: unknown; + readonly detail?: " without an entries list"; + } + | { + readonly code: "layer-bad-directive"; + readonly layer: string; + readonly site: string; + readonly actual: unknown; + } + | { readonly code: "layer-no-layering-key"; readonly layer: string; readonly site: string } + | { + readonly code: "layer-no-key"; + readonly layer: string; + readonly site: string; + readonly keyField: string; + } + | { + readonly code: "layer-duplicate-key"; + readonly layer: string; + readonly site: string; + readonly keyField: string; + readonly first: number; + readonly second: number; + } + // The section selection + | { readonly code: "required-sections-excluded"; readonly excluded: readonly SectionKey[] } + // The run flows + | { readonly code: "artifact-uploader-missing" } + | { + readonly code: "merged-file-is-layer"; + readonly mergedFile: string; + /** The colliding layer's position in the settings-file list, from 0. */ + readonly index: number; + readonly layer: string; + } + | { readonly code: "merged-file-unwritable"; readonly path: string; readonly reason: string } + | { readonly code: "no-targets"; readonly filteredOut: number } + // Target resolution + | { readonly code: "repo-slug-invalid"; readonly value: string } + | { readonly code: "repos-input-wildcard-mixed" } + | { + readonly code: "repos-input-invalid-entries"; + readonly invalid: readonly string[]; + readonly duplicated: readonly string[]; + } + | { readonly code: "repos-dir-missing"; readonly reposDir: string } + | { readonly code: "repos-dir-unreadable"; readonly reposDir: string; readonly reason: string } + | { + readonly code: "repos-dir-invalid-files"; + readonly reposDir: string; + readonly files: readonly CentralFileProblem[]; + } + | { + readonly code: "discovery-request-failed"; + readonly path: string; + readonly status: number; + readonly message: string; + /** True for a denial or an invalid token, where a user PAT is the fix. */ + readonly denied: boolean; + } + | { readonly code: "discovery-transport-failed"; readonly reason: string } + | { readonly code: "discovery-response-not-a-list"; readonly path: string } + // The private report + | { readonly code: "age-recipient-invalid"; readonly reason: string }; + +/** The members with these codes, for a function's own error type. */ +export type ProblemOf = Extract; + +/** The layer boundary's members: what mergeLayers refuses. */ +export type LayerProblem = Extract; + +/** The settings document's members: what validateSettingsDoc refuses (a file's read failure is not one). */ +export type SettingsProblem = ProblemOf< + | "settings-not-mapping" + | "settings-not-plain-mapping" + | "settings-unknown-sections" + | "settings-malformed-sections" +>; + +const PAT_ADVICE = + "Discovery needs a user PAT; the workflow GITHUB_TOKEN and GitHub App installation tokens " + + 'cannot enumerate a user\'s repositories. List the target repositories explicitly in the "repos" input'; + +const PASSTHROUGH_ADVICE = + "Fix these values in the settings file (only the named keys are validated; extra fields pass " + + "through, except in closed sections and strict nested objects like actions.cache, which reject " + + "unrecognized keys)"; + +function quote(value: unknown): string { + return JSON.stringify(String(value)); +} + +/** A value's kind for refusal prose: what it is, not what it contains. */ +function describeShape(value: unknown): string { + if (value === null) { + return "null"; + } + if (Array.isArray(value)) { + return "a list"; + } + if (isPlainObject(value)) { + return "a mapping"; + } + if (typeof value === "object") { + return `a ${value.constructor?.name ?? "tagged"} value`; + } + return `a ${typeof value}`; +} + +function describeCentralFile(file: CentralFileProblem): string { + switch (file.kind) { + case "not-a-slug": + return `${file.filePath} resolves to the target "${file.slug}", which is not a valid owner/name slug. Rename the file so and contain only letters, digits, dots, underscores, and dashes`; + case "duplicate": + return `duplicate target ${file.slug}: defined by both ${file.first} and ${file.second}. Keep exactly one settings file per repository`; + case "ownerless": + return `cannot resolve ${file.files.join(", ")}: top-level repos-dir files use the current repository's owner, which is unknown outside GitHub Actions. Use the /.yml layout instead`; + } +} + +function describeUnreadable(problem: ProblemOf<"settings-file-unreadable">): string { + switch (problem.role) { + case "settings-file": + return `cannot read settings from ${problem.path}: ${problem.reason}. Check that the file exists at that path (set the "settings-file" input if it lives elsewhere) and is valid YAML`; + case "defaults-file": + return `cannot read the defaults file ${problem.path}: ${problem.reason}. Check the "defaults-file" path and that the file is valid YAML`; + case "layer": + return `cannot read the settings layer ${problem.path}: ${problem.reason}. Check that every path in the "settings-file" input exists and is valid YAML`; + } +} + +function describeFiltersWithoutWildcard( + problem: ProblemOf<"discovery-filters-without-wildcard">, +): string { + const named = quoteList(problem.filters); + switch (problem.targets) { + case "single-repo": + return `the discovery filter input(s) ${named} only apply to repos: "*" discovery, but this run is in single-repo mode. Set repos: "*" to discover repositories, or remove the filter input(s)`; + case "explicit-repos": + return `the discovery filter input(s) ${named} only apply when repos is "*", but the "repos" input lists explicit repositories. Set repos: "*", or remove the filter input(s)`; + case "repos-dir": + return `the discovery filter input(s) ${named} only apply to repos: "*" discovery, but targets come only from repos-dir files. Set repos: "*", or remove the filter input(s)`; + } +} + +function describeUnknownSectionInput( + unknown: ProblemOf<"input-unknown-sections">["unknown"][number], + known: readonly string[], +): string { + const quoted = quoteList(unknown.names); + return unknown.names.length === 1 + ? `unknown section ${quoted} in the "${unknown.input}" input; it matches none of: ${known.join(", ")}. Fix the name in the workflow's input list` + : `unknown sections ${quoted} in the "${unknown.input}" input; each matches none of: ${known.join(", ")}. Fix the names in the workflow's input list`; +} + +function describeInvalidReposEntries(problem: ProblemOf<"repos-input-invalid-entries">): string { + const parts: string[] = []; + if (problem.invalid.length > 0) { + parts.push( + `${quoteList(problem.invalid)} ` + + `${problem.invalid.length === 1 ? "is not an owner/name slug" : "are not owner/name slugs"} ` + + '(use values like "octocat/hello-world", comma- or newline-separated)', + ); + } + if (problem.duplicated.length > 0) { + parts.push( + `${quoteList(problem.duplicated)} ` + + `${problem.duplicated.length === 1 ? "is" : "are"} listed more than once ` + + "(keep exactly one entry per repository)", + ); + } + const count = problem.invalid.length + problem.duplicated.length; + return `the "repos" input has ${count} invalid entr${count === 1 ? "y" : "ies"}: ${parts.join("; ")}. Or use "*" alone to discover repositories`; +} + +/** + * The ONE place a problem is worded. INVARIANT for the layer members: a + * message names the layer as the layer list names it, the site's key path, and + * the kind of problem - never a value from the document. mode: merge has no + * private-repos redaction context, so a value echoed there (a label name, a + * rule type, a mis-shaped section body) could land a private repository's + * settings in a public log. `actual` reaches the prose only through + * describeShape; the marker test in test/engine/layers.test.ts pins this. + */ +export function describeProblem(problem: Problem): string { + switch (problem.code) { + case "input-unsupported-value": { + const values = problem.allowed.map((v) => + v === problem.fallback ? `"${v}" (default)` : `"${v}"`, + ); + return `the "${problem.input}" input is "${problem.value}", which is not a supported ${problem.noun}. Set it to ${values.join(", ")}`; + } + case "input-unknown-sections": + return problem.unknown + .map((unknown) => describeUnknownSectionInput(unknown, problem.known)) + .join("; "); + case "required-sections-excluded": { + const one = problem.excluded.length === 1; + const [noun, pronoun] = one ? ["entry", "it"] : ["entries", "them"]; + return ( + `the "required-sections" ${noun} ${quoteList(problem.excluded)} ${one ? "is" : "are"} ` + + `excluded by the "sections" allowlist, so the run would pass without ever attempting ` + + `${pronoun}. Add ${pronoun} to the "sections" input, or remove ${pronoun} from ` + + `"required-sections"` + ); + } + case "input-report-key-unused": + return `the "report-public-key" input only applies to private-report: artifact, but the channel is "${problem.channel}", so the key would never be used. Remove report-public-key, or set private-report: artifact`; + case "input-report-key-missing": + return ( + 'private-report: artifact needs a "report-public-key" input: the age recipient every ' + + 'report is encrypted to. Generate a keypair with "age-keygen -o key.txt", keep key.txt ' + + 'secret, and set report-public-key to the printed "age1..." recipient (safe to commit)' + ); + case "input-report-key-invalid": + return `the "report-public-key" input is not a valid age recipient: ${problem.reason}. It must be an "age1..." public key from "age-keygen" (the recipient line, not the AGE-SECRET-KEY identity)`; + case "input-rejected-in-merge": + return ( + `the ${quoteList(problem.inputs)} input(s) do not apply to mode: merge, which only folds ` + + "the settings-file layers into merged-file: it never targets a repository, calls the GitHub " + + "API, delivers a report, or narrows the sections it writes. Remove the input(s), or move " + + "them to the apply or check step that runs the merged document" + ); + case "input-merged-file-missing": + return 'mode: merge needs a "merged-file" input: the path the merged settings document is written to. Set it (for example .github/settings.merged.yml) and feed that path to a later apply or check step as its settings-file'; + case "input-settings-file-empty": + return `the "settings-file" input is "${problem.value}", which lists no file. In mode: merge it is the ordered list of layers to fold, newline- or comma-separated, lowest first; name at least one settings file`; + case "input-merge-only": + return ( + `the ${quoteList(problem.inputs)} input(s) only apply to mode: merge, but this run is in ` + + `${problem.mode} mode, so ${problem.inputs.length === 1 ? "it" : "they"} would never be ` + + "used. Remove the input(s), or set mode: merge to fold settings files" + ); + case "input-token-missing": + return 'cannot call the GitHub API: no token was provided. Set the "token" input on the action step (or export GITHUB_TOKEN)'; + case "input-report-without-redaction": + return 'the "private-report" input delivers reports only for redacted targets, but "private-repos" is "show", so nothing is redacted and no report would ever be sent. Set private-repos: redact, or set private-report: none'; + case "input-affiliation-unsupported": + return `the "affiliation" input entry "${problem.entry}" is not a supported affiliation, so discovery cannot build the /user/repos query. Use a comma-separated list of ${quoteList(problem.allowed)}`; + case "input-exclude-pattern-invalid": + return ( + `the "exclude" input pattern "${problem.pattern}" can never match an owner/name repository: a ` + + `pattern takes at most one "/", with a non-empty glob on each side of it. Use ` + + `"" or "/", where "*" matches any characters` + ); + case "input-repository-with-multi": + return 'the "repository" input cannot be combined with "repos" or "repos-dir"; multi-repo targets come from those inputs. Remove "repository", or remove the multi-repo inputs to stay in single-repo mode'; + case "input-settings-file-with-multi": + return 'the "settings-file" input cannot be combined with "repos" or "repos-dir": central targets are read from repos-dir files and remote targets from each repository\'s own .github/settings.yml. Remove the settings-file override'; + case "discovery-filters-without-wildcard": + return describeFiltersWithoutWildcard(problem); + case "input-defaults-file-without-multi": + return 'the "defaults-file" input only applies to multi-repo mode, but this run is in single-repo mode, so the defaults would never apply. Remove the input, or add "repos" or "repos-dir" to switch to multi-repo mode'; + case "input-settings-file-is-list": + return ( + `the "settings-file" input is "${problem.value}", which contains a list separator: ` + + `${problem.mode} mode reads exactly one settings file, and only mode: merge takes a ` + + "newline- or comma-separated list. Name one file, or set mode: merge to fold the list into " + + "one document" + ); + case "input-repository-not-slug": + return `cannot target a repository: "${problem.value}" is not an owner/name slug. Set the "repository" input (or GITHUB_REPOSITORY) to a value like "octocat/hello-world"`; + case "settings-not-mapping": + return `${problem.source} must be a YAML mapping of section names to settings, but its top level parsed as a ${problem.shape}. Rewrite the top level as "section: ..." keys`; + case "settings-not-plain-mapping": + return `${problem.source} must be a plain YAML mapping of section names to settings, but its top level parsed as another type (a YAML-tagged value like !!timestamp parses to a Date). Rewrite the top level as "section: ..." keys`; + case "settings-unknown-sections": + return `unknown top-level section(s) in ${problem.source}: ${problem.unknown.join(", ")} (known: ${problem.known.join(", ")}). Fix the typo, or prefix private keys with "_", or set the "sections" input to limit processing`; + case "settings-malformed-sections": + return `${problem.source} has malformed section entries: ${problem.issues.join("; ")}. ${PASSTHROUGH_ADVICE}`; + case "yaml-invalid": + return problem.reason; + case "settings-file-unreadable": + return describeUnreadable(problem); + case "layer-cycle": + return `${layerSite(problem)} contains a reference cycle (a YAML anchor that includes itself); layers must be trees`; + case "layer-wrong-shape": + return `${layerSite(problem)} must be ${problem.expected}; got ${describeShape(problem.actual)}${problem.detail ?? ""}`; + case "layer-bad-directive": + return `${layerSite(problem)} must be "merge" or "replace"; got ${describeShape(problem.actual)}${typeof problem.actual === "string" ? " that is neither" : ""}`; + case "layer-no-layering-key": + return `${layerSite(problem)} has no layering key, so it cannot be layered by "merge"; declare _layering: replace or drop the directive`; + case "layer-no-key": + return `${layerSite(problem)} carries no string ${quote(problem.keyField)}, which every entry needs to layer by`; + case "layer-duplicate-key": + return `${layerSite(problem)}[${problem.first}] and ${problem.site}[${problem.second}] both claim one ${problem.keyField}; each ${problem.keyField} belongs to one entry within a layer`; + case "artifact-uploader-missing": + return "private-report: artifact needs an artifact uploader, and none was supplied: the action supplies its own; a library caller passes one as the uploader argument, or picks another private-report channel"; + case "merged-file-is-layer": + return ( + `the "merged-file" input "${problem.mergedFile}" is layer ${problem.index + 1} of the ` + + `"settings-file" list ("${problem.layer}"): the merge would overwrite that layer with the ` + + "folded document, and the next run would fold the merged document as a layer. Write the " + + "merged document to a path outside the layer list" + ); + case "merged-file-unwritable": + return `cannot write the merged document to ${problem.path}: ${problem.reason}. Check that the "merged-file" input names a writable path`; + case "no-targets": + return problem.filteredOut > 0 + ? `multi-repo mode found no targets: repos: "*" discovery found ${problem.filteredOut} ` + + `${problem.filteredOut === 1 ? "repository" : "repositories"}, but the discovery filters ` + + `removed all of them (see the notices above). Relax the filter inputs, or add per-repo ` + + `files to the repos-dir` + : `multi-repo mode found no targets: repos-dir yielded no settings files and the "repos" input resolved to no repositories. Add per-repo files to the repos-dir, or list repositories in the "repos" input`; + case "repo-slug-invalid": + return `"${problem.value}" is not an owner/name repository slug (use a value like "octocat/hello-world")`; + case "repos-input-wildcard-mixed": + return 'the "repos" input mixes "*" with explicit repositories. Use "*" alone to discover every repository the token owns, or list the repositories without it'; + case "repos-input-invalid-entries": + return describeInvalidReposEntries(problem); + case "repos-dir-missing": + return `repos-dir "${problem.reposDir}" does not exist in the workspace, so there are no central settings files to read. Add an actions/checkout step before this action, or fix the repos-dir path`; + case "repos-dir-unreadable": + return `cannot read repos-dir "${problem.reposDir}": ${problem.reason}. Check that it is a readable directory of settings files`; + case "repos-dir-invalid-files": + return `repos-dir "${problem.reposDir}" has ${problem.files.length} invalid settings file(s):\n- ${problem.files.map(describeCentralFile).join("\n- ")}`; + case "discovery-request-failed": + return `cannot discover repositories for repos: "*": GET ${problem.path} failed: ${problem.status} ${problem.message}. ${problem.denied ? PAT_ADVICE : RERUN_ADVICE}`; + case "discovery-transport-failed": + return `cannot discover repositories for repos: "*": ${problem.reason}. ${RERUN_ADVICE}`; + case "discovery-response-not-a-list": + return `cannot discover repositories for repos: "*": GET ${problem.path} returned a JSON value that is not a list, so the response cannot be paginated. ${RERUN_ADVICE}`; + case "age-recipient-invalid": + return `not a valid age recipient: ${problem.reason}`; + } +} + +/** The `layer "": ` prefix every layer refusal opens with. */ +function layerSite(problem: LayerProblem): string { + return `layer ${quote(problem.layer)}: ${problem.site}`; +} diff --git a/src/report/artifact-report.ts b/src/report/artifact-report.ts index 3d4d8a49..e699659e 100644 --- a/src/report/artifact-report.ts +++ b/src/report/artifact-report.ts @@ -10,6 +10,8 @@ */ import { Encrypter } from "age-encryption"; +import { err, ok, type Result } from "neverthrow"; +import type { ProblemOf } from "../problem.js"; export const ARTIFACT_NAME = "settings-as-code-private-report"; export const ARTIFACT_FILE = "private-report.md.age"; @@ -19,12 +21,17 @@ export const ARTIFACT_FILE = "private-report.md.age"; * at config parse: a malformed `report-public-key` must be rejected before * any API work. Accepts exactly what the age library accepts (`age1...`). */ -export function parseRecipient(recipient: string): { ok: true } | { ok: false; error: string } { +export function parseRecipient( + recipient: string, +): Result> { try { new Encrypter().addRecipient(recipient); - return { ok: true }; + return ok(); } catch (error) { - return { ok: false, error: error instanceof Error ? error.message : String(error) }; + return err({ + code: "age-recipient-invalid", + reason: error instanceof Error ? error.message : String(error), + }); } } diff --git a/src/report/delivery.ts b/src/report/delivery.ts index 6b9dba2f..1e255ccc 100644 --- a/src/report/delivery.ts +++ b/src/report/delivery.ts @@ -10,6 +10,7 @@ import type { GithubClient } from "../github/api.js"; import type { CollectedLine, Io } from "../io.js"; import type { Private } from "../private.js"; import { revealPrivate } from "../private-open.js"; +import { describeProblem } from "../problem.js"; import { type ArtifactUploader, deliverArtifactReport } from "./artifact-report.js"; import { composeReport } from "./composer.js"; import { @@ -33,10 +34,6 @@ export const PRIVATE_REPORT_CHANNELS = ["none", "issue", "issue-on-failure", "ar export type PrivateReportChannel = (typeof PRIVATE_REPORT_CHANNELS)[number]; -/** The `artifact` channel cannot open without an upload port; the message names the fix. */ -export const ARTIFACT_NEEDS_UPLOADER = - "private-report: artifact needs an artifact uploader, and none was supplied: the action supplies its own; a library caller passes one as the uploader argument, or picks another private-report channel"; - /** The channels that deliver through the target repo's report issue. */ export type IssueChannel = Extract; @@ -151,7 +148,8 @@ export interface ReportChannel { * The run's report channel from the `private-report` input, null for `none`. * Issue channels post per target as it closes; the artifact channel uploads ONE * encrypted document on flush through `uploader`, which the flows check for - * before any API work (missingUploaderProblem); the throw here is the backstop. + * before any API work (requireUploader); the throw here is the backstop for a + * caller that skipped that check, an invariant violation and not a run outcome. */ export function openReportChannel( api: GithubClient, @@ -170,7 +168,7 @@ export function openReportChannel( return issueChannel(api, meta, "on-failure", io); case "artifact": if (uploader === undefined) { - throw new Error(ARTIFACT_NEEDS_UPLOADER); + throw new Error(describeProblem({ code: "artifact-uploader-missing" })); } return artifactChannel(meta, reportPublicKey, io, uploader); } diff --git a/src/sections/actions/actions.test.ts b/src/sections/actions/actions.test.ts index f2428346..a97c31c3 100644 --- a/src/sections/actions/actions.test.ts +++ b/src/sections/actions/actions.test.ts @@ -5,6 +5,7 @@ import { driftOf, type PlannedOp, planContext } from "../../../src/sections/cont import { MockApi } from "../../../test/mock-api.js"; import { provePlanIdempotent } from "../../../test/sections/plan-idempotence.js"; import { REPO } from "../../../test/sections/section-run.js"; +import { describeProblem } from "../../problem.js"; import { PermissionDenied } from "../contract/errors.js"; import { grantFor } from "../contract/permissions.js"; import { actionsSection, endpointRouted } from "./index.js"; @@ -15,8 +16,7 @@ import type { ActionsConfig } from "./schema.js"; /** The verdict's error prose, or null when the document validated. */ function shapeError(doc: Record, sourceLabel: string): string | null { - const verdict = validateSectionShapes(doc, sourceLabel); - return "error" in verdict ? verdict.error : null; + return validateSectionShapes(doc, sourceLabel).match(() => null, describeProblem); } const BASE = "/repos/o/r/actions/permissions"; diff --git a/src/sections/actions_secrets/actions_secrets.test.ts b/src/sections/actions_secrets/actions_secrets.test.ts index 90887b02..b76de723 100644 --- a/src/sections/actions_secrets/actions_secrets.test.ts +++ b/src/sections/actions_secrets/actions_secrets.test.ts @@ -17,6 +17,8 @@ import { import { MockApi } from "../../../test/mock-api.js"; import { provePlanIdempotent } from "../../../test/sections/plan-idempotence.js"; import { REPO } from "../../../test/sections/section-run.js"; +import { SectionSelection } from "../../engine/section-selection.js"; +import { describeProblem } from "../../problem.js"; import { driftOf, type ExecTools, type PlannedOp, planContext } from "../contract/plan.js"; import { actionsSecretsSection } from "./index.js"; @@ -409,18 +411,17 @@ describe("actions_secrets execution", () => { new Set(), io, ); - if ("error" in validated) { - throw new Error(validated.error); + if (validated.isErr()) { + throw new Error(describeProblem(validated.error)); } const result = await runForRepo( api, { repo: REPO, - settings: validated.settings, + settings: validated.value, mode: "apply", onMissingPermission: "fail", - requiredSections: new Set(), - onlySections: new Set(), + sections: SectionSelection.ALL, secretEnv: { DEPLOY_TOKEN: "s3cret-plaintext" }, }, io, diff --git a/src/sections/repository/repository.test.ts b/src/sections/repository/repository.test.ts index dc273519..9c0c601e 100644 --- a/src/sections/repository/repository.test.ts +++ b/src/sections/repository/repository.test.ts @@ -6,13 +6,13 @@ import { MockApi } from "../../../test/mock-api.js"; import { provePlanIdempotent } from "../../../test/sections/plan-idempotence.js"; import { REPO } from "../../../test/sections/section-run.js"; import { validateSectionShapes } from "../../engine/validate.js"; +import { describeProblem } from "../../problem.js"; import { PermissionDenied } from "../contract/errors.js"; import { FEATURE_TOGGLES, normalizeTopics, repositorySection } from "./index.js"; /** The verdict's error prose, or null when the document validated. */ function shapeError(doc: Record, sourceLabel: string): string | null { - const verdict = validateSectionShapes(doc, sourceLabel); - return "error" in verdict ? verdict.error : null; + return validateSectionShapes(doc, sourceLabel).match(() => null, describeProblem); } const GET = "GET /repos/o/r"; diff --git a/src/sections/secret_scanning_custom_patterns/secret_scanning_custom_patterns.test.ts b/src/sections/secret_scanning_custom_patterns/secret_scanning_custom_patterns.test.ts index f9dbe350..bf4637e1 100644 --- a/src/sections/secret_scanning_custom_patterns/secret_scanning_custom_patterns.test.ts +++ b/src/sections/secret_scanning_custom_patterns/secret_scanning_custom_patterns.test.ts @@ -5,6 +5,7 @@ import { provePlanIdempotent } from "../../../test/sections/plan-idempotence.js" import { REPO } from "../../../test/sections/section-run.js"; import { validateSettingsDoc } from "../../engine/orchestrate.js"; import { silentIo } from "../../io.js"; +import { describeProblem } from "../../problem.js"; import { type PlainData, planContext, type SectionPlan } from "../contract/plan.js"; import { secretScanningPatternsSection } from "./index.js"; import { secretScanningCustomPatternsMockHandlers } from "./mock.js"; @@ -273,7 +274,7 @@ describe("secret_scanning_custom_patterns", () => { for (const key of ["start_delimiter", "end_delimiter"] as const) { const doc = { secret_scanning_custom_patterns: [{ ...INTERNAL, [key]: "" }] }; const invalid = validateSettingsDoc(doc, "test doc", new Set(), silentIo()); - expect("error" in invalid ? invalid.error : "").toContain( + expect(invalid.match(() => "", describeProblem)).toContain( "cannot be cleared with an empty string", ); } @@ -347,8 +348,8 @@ describe("secret_scanning_custom_patterns closed surface", () => { new Set(), silentIo(), ); - expect("error" in error, `a declared "${key}" must be rejected`).toBe(true); - const message = "error" in error ? error.error : ""; + expect(error.isErr(), `a declared "${key}" must be rejected`).toBe(true); + const message = error.match(() => "", describeProblem); expect(message).toContain(`"${key}"`); expect(message).toContain("read-only"); } diff --git a/src/sections/webhooks/webhooks.test.ts b/src/sections/webhooks/webhooks.test.ts index a98efe19..a5acee13 100644 --- a/src/sections/webhooks/webhooks.test.ts +++ b/src/sections/webhooks/webhooks.test.ts @@ -4,6 +4,7 @@ import { MockApi } from "../../../test/mock-api.js"; import { fragmentFake } from "../../../test/sections/fragment-fake.js"; import { provePlanIdempotent } from "../../../test/sections/plan-idempotence.js"; import { REPO } from "../../../test/sections/section-run.js"; +import { describeProblem } from "../../problem.js"; import { driftOf, type ExecTools, @@ -18,8 +19,7 @@ import type { WebhookConfig } from "./schema.js"; /** The verdict's error prose, or null when the document validated. */ function shapeError(doc: Record, sourceLabel: string): string | null { - const verdict = validateSectionShapes(doc, sourceLabel); - return "error" in verdict ? verdict.error : null; + return validateSectionShapes(doc, sourceLabel).match(() => null, describeProblem); } const LIST = "GET /repos/o/r/hooks?per_page=100&page=1"; diff --git a/src/types.ts b/src/types.ts index 9f8bcc8b..3f919938 100644 --- a/src/types.ts +++ b/src/types.ts @@ -30,3 +30,9 @@ export interface UndeclaredPolicyList { * orchestrate.ts, inputs.ts), so the idiom cannot drift between them. */ export type MustBeNever = T; + +/** + * Omit distributed over a union: `Omit` collapses to the common + * keys, losing each member's own fields, while this keeps one member per arm. + */ +export type DistributiveOmit = T extends unknown ? Omit : never; diff --git a/test/action/action-yml.test.ts b/test/action/action-yml.test.ts index 6c9b1aca..4785b0b8 100644 --- a/test/action/action-yml.test.ts +++ b/test/action/action-yml.test.ts @@ -4,20 +4,15 @@ * themselves are covered by test/scripts/gen-action-docs.test.ts. */ -import { afterEach, describe, expect, test } from "bun:test"; +import { describe, expect, test } from "bun:test"; import { readFileSync } from "node:fs"; import { join } from "node:path"; import { parse as parseYaml } from "yaml"; -import { - FILTER_INPUTS, - INPUT_DECLS, - type InputDecl, - parseConfig, -} from "../../src/action/inputs.js"; import { OUTPUT_DECLS } from "../../src/action/io.js"; import { DEFAULT_DISCOVERY_FILTERS } from "../../src/discovery/discover.js"; import { REPO_RESULTS } from "../../src/engine/orchestrate.js"; import { MERGE_RESULT } from "../../src/flows/deliver.js"; +import { FILTER_INPUTS, INPUT_DECLS, type InputDecl } from "../../src/flows/inputs.js"; const ROOT = join(import.meta.dir, "..", ".."); @@ -96,53 +91,3 @@ describe("output declarations", () => { ).toEqual([]); }); }); - -describe("parseConfig <-> input declarations", () => { - // Every env key the parse reads: the INPUT_* the runner would set from - // action.yml (@actions/core keeps the dashes) and the GITHUB_* context. - const touched = [ - ...Object.keys(INPUT_DECLS).map((name) => `INPUT_${name.toUpperCase()}`), - "GITHUB_TOKEN", - "GITHUB_REPOSITORY", - "GITHUB_SERVER_URL", - "GITHUB_RUN_ID", - ]; - const saved = new Map(touched.map((key) => [key, process.env[key]])); - afterEach(() => { - for (const [key, value] of saved) { - if (value === undefined) { - delete process.env[key]; - } else { - process.env[key] = value; - } - } - }); - - test("every unset input resolves to its declared default", () => { - for (const key of touched) { - delete process.env[key]; - } - process.env.INPUT_TOKEN = "t"; - process.env.GITHUB_REPOSITORY = "o/r"; - const parsed = parseConfig(); - if ("error" in parsed) { - throw new Error(`expected a config, got: ${parsed.error}`); - } - expect(parsed.config).toEqual({ - kind: "single", - token: "t", - mode: INPUT_DECLS.mode.default, - onMissingPermission: INPUT_DECLS["on-missing-permission"].default, - requiredSections: new Set(), - onlySections: new Set(), - apiVersion: INPUT_DECLS["api-version"].default, - privateRepos: INPUT_DECLS["private-repos"].default, - privateReport: INPUT_DECLS["private-report"].default, - reportPublicKey: "", - selfSlug: "o/r", - runUrl: "", - repo: { owner: "o", name: "r", slug: "o/r" }, - settingsFile: INPUT_DECLS["settings-file"].default, - }); - }); -}); diff --git a/test/action/inputs.test.ts b/test/action/inputs.test.ts index 69ff5b68..db871e62 100644 --- a/test/action/inputs.test.ts +++ b/test/action/inputs.test.ts @@ -1,16 +1,18 @@ import { afterEach, describe, expect, test } from "bun:test"; -import { INPUT_DECLS, parseConfig, type RunConfig } from "../../src/action/inputs.js"; -import { SECTION_KEYS } from "../../src/schema.js"; +import { ok } from "neverthrow"; +import { parseActionConfig } from "../../src/action/inputs.js"; +import { INPUT_DECLS, SectionSelection } from "../../src/index.js"; /** - * parseConfig() reads inputs from the INPUT_* environment (via - * @actions/core), so each test clears every declared input, sets exactly - * the env it needs, and the afterEach restores every touched key. + * The adapter reads INPUT_* through @actions/core and the GITHUB_* context + * from the environment, so the test sets exactly that env and restores it. */ const ENV_KEYS = [ ...Object.keys(INPUT_DECLS).map((name) => `INPUT_${name.toUpperCase()}`), "GITHUB_TOKEN", "GITHUB_REPOSITORY", + "GITHUB_SERVER_URL", + "GITHUB_RUN_ID", ]; const saved = new Map(ENV_KEYS.map((key) => [key, process.env[key]])); @@ -24,256 +26,32 @@ afterEach(() => { } }); -/** Clears every declared input, then sets `base` and `inputs` (INPUT_ names, lowercase) in that order. */ -function setInputs(base: Record, inputs: Record): void { - for (const key of ENV_KEYS) { - delete process.env[key]; - } - for (const [key, value] of Object.entries({ ...base, ...inputs })) { - process.env[`INPUT_${key.toUpperCase()}`] = value; - } -} - -/** A single-repo apply run's env, with `inputs` on top. */ -function setEnv(inputs: Record): void { - setInputs({ token: "t", repository: "o/r" }, inputs); -} - -/** A mode: merge run's env: no token anywhere, the two layers, the output path, plus `inputs`. */ -function setMergeEnv(inputs: Record): void { - setInputs( - { mode: "merge", "settings-file": "fleet.yml\nrepo.yml", "merged-file": "out/merged.yml" }, - inputs, - ); -} - -function rejection(): string { - const parsed = parseConfig(); - if (!("error" in parsed)) { - throw new Error(`expected a rejection, got: ${JSON.stringify(parsed.config)}`); - } - return parsed.error; -} - -describe("required-sections x sections cross-validation", () => { - test("rejects a required section excluded by the sections allowlist", () => { - setEnv({ "required-sections": "labels", sections: "repository" }); - expect(rejection()).toBe( - 'the "required-sections" entry "labels" is excluded by the "sections" allowlist, so the run would pass without ever attempting it. Add it to the "sections" input, or remove it from "required-sections"', - ); - }); - - test("names every excluded required section at once", () => { - setEnv({ "required-sections": "labels,milestones,repository", sections: "repository" }); - const error = rejection(); - expect(error).toContain('entries "labels", "milestones" are excluded'); - expect(error).not.toContain('"repository"'); - }); - - test("accepts required sections inside the allowlist", () => { - setEnv({ "required-sections": "labels", sections: "labels,repository" }); - const parsed = parseConfig(); - if ("error" in parsed || parsed.config.kind === "merge") { - throw new Error(`expected a single-repo config, got: ${JSON.stringify(parsed)}`); - } - // Accepted AND carried into the config: a parse that silently dropped - // either set would otherwise pass. - expect([parsed.config.requiredSections, parsed.config.onlySections]).toEqual([ - new Set(["labels"]), - new Set(["labels", "repository"]), - ]); - }); - - test("an empty sections input restricts nothing, so any required section passes", () => { - setEnv({ "required-sections": "labels" }); - const parsed = parseConfig(); - if ("error" in parsed || parsed.config.kind === "merge") { - throw new Error(`expected a single-repo config, got: ${JSON.stringify(parsed)}`); +describe("parseActionConfig", () => { + test("reads the step's inputs and the GITHUB_* context; every unset input takes its declared default", () => { + for (const key of ENV_KEYS) { + delete process.env[key]; } - expect([parsed.config.requiredSections, parsed.config.onlySections]).toEqual([ - new Set(["labels"]), - new Set(), - ]); - }); - - test("unknown-name validation still wins over the cross-check", () => { - setEnv({ "required-sections": "nope", sections: "repository" }); - expect(rejection()).toBe( - `unknown section "nope" in the "required-sections" input; it matches none of: ${SECTION_KEYS.join(", ")}. Fix the name in the workflow's input list`, - ); - }); -}); - -describe("the mode input", () => { - test("an unsupported mode is rejected naming every supported one", () => { - setEnv({ mode: "dry-run" }); - expect(rejection()).toBe( - 'the "mode" input is "dry-run", which is not a supported mode. Set it to "apply" (default), "check", "merge"', - ); - }); - - test.each([ - ["layering", { layering: "replace" }, 'the "layering" input(s) only apply to mode: merge'], - [ - "merged-file", - { "merged-file": "out.yml" }, - 'the "merged-file" input(s) only apply to mode: merge', - ], - [ - "both merge-only inputs", - { layering: "merge", "merged-file": "out.yml" }, - 'the "merged-file", "layering" input(s) only apply to mode: merge, but this run is in check mode, so they would never be used', - ], - ] as const)("%s outside merge mode is rejected", (_case, inputs, fragment) => { - setEnv({ mode: "check", ...inputs }); - expect(rejection()).toContain(fragment); - }); - - const SEPARATOR_CASES = [ - ["a comma list", "a.yml,b.yml"], - ["a newline list", "a.yml\nb.yml"], - ["an empty list", ","], - ["a trailing separator", "only.yml,"], - ["a leading separator", ",only.yml"], - ] as const; - - test.each( - (["apply", "check"] as const).flatMap((mode) => - SEPARATOR_CASES.map(([label, value]) => [mode, label, value] as const), - ), - )("%s mode rejects a settings-file with %s rather than repairing it", (mode, _case, value) => { - setEnv({ mode, "settings-file": value }); - expect(rejection()).toBe( - `the "settings-file" input is "${value}", which contains a list separator: ${mode} mode ` + - `reads exactly one settings file, and only mode: merge takes a newline- or ` + - `comma-separated list. Name one file, or set mode: merge to fold the list into one ` + - `document`, - ); - }); - - test("a plain settings-file path is carried verbatim into the single-repo config", () => { - setEnv({ "settings-file": "conf/only.yml" }); - expect(parseConfig()).toEqual({ - config: { + process.env["INPUT_SETTINGS-FILE"] = "conf/only.yml"; + process.env.GITHUB_TOKEN = "t"; + process.env.GITHUB_REPOSITORY = "o/r"; + process.env.GITHUB_SERVER_URL = "https://github.com"; + process.env.GITHUB_RUN_ID = "42"; + expect(parseActionConfig()).toEqual( + ok({ + kind: "single", token: "t", - mode: "apply", - onMissingPermission: "fail", - requiredSections: new Set(), - onlySections: new Set(), - apiVersion: "2022-11-28", - privateRepos: "redact", - privateReport: "none", + mode: INPUT_DECLS.mode.default, + onMissingPermission: INPUT_DECLS["on-missing-permission"].default, + sections: SectionSelection.ALL, + apiVersion: INPUT_DECLS["api-version"].default, + privateRepos: INPUT_DECLS["private-repos"].default, + privateReport: INPUT_DECLS["private-report"].default, reportPublicKey: "", - selfSlug: "", - runUrl: "", - kind: "single", + selfSlug: "o/r", + runUrl: "https://github.com/o/r/actions/runs/42", repo: { owner: "o", name: "r", slug: "o/r" }, settingsFile: "conf/only.yml", - }, - }); - }); -}); - -describe("mode: merge", () => { - /** What the two-layer merge env parses to; the tests below vary one input around it. */ - const MERGE_CONFIG: Extract = { - kind: "merge", - settingsFiles: ["fleet.yml", "repo.yml"], - mergedFile: "out/merged.yml", - layering: "merge", - }; - - test("parses without any token, carrying the ordered layers, the output path, and the layering", () => { - setMergeEnv({ layering: "replace" }); - expect(parseConfig()).toEqual({ config: { ...MERGE_CONFIG, layering: "replace" } }); - }); - - test("the layering input defaults to merge and a comma list of layers works too", () => { - setMergeEnv({ "settings-file": " fleet.yml , team.yml ,repo.yml" }); - expect(parseConfig()).toEqual({ - config: { ...MERGE_CONFIG, settingsFiles: ["fleet.yml", "team.yml", "repo.yml"] }, - }); - }); - - test("an empty layer list is rejected", () => { - setMergeEnv({ "settings-file": "," }); - expect(rejection()).toBe( - 'the "settings-file" input is ",", which lists no file. In mode: merge it is the ordered list of layers to fold, newline- or comma-separated, lowest first; name at least one settings file', + }), ); }); - - test("a missing merged-file is rejected", () => { - setMergeEnv({ "merged-file": "" }); - expect(rejection()).toBe( - 'mode: merge needs a "merged-file" input: the path the merged settings document is written to. Set it (for example .github/settings.merged.yml) and feed that path to a later apply or check step as its settings-file', - ); - }); - - test("a merged-file beside the layers but not among them is accepted", () => { - setMergeEnv({ "merged-file": "./merged.yml" }); - expect(parseConfig()).toEqual({ config: { ...MERGE_CONFIG, mergedFile: "./merged.yml" } }); - }); - - test("an unsupported layering is rejected", () => { - setMergeEnv({ layering: "union" }); - expect(rejection()).toBe( - 'the "layering" input is "union", which is not a supported layering. Set it to "merge" (default), "replace"', - ); - }); - - test.each([ - ["repository", { repository: "o/r" }, '"repository"'], - ["repos", { repos: "o/a" }, '"repos"'], - ["defaults-file", { "defaults-file": "defaults.yml" }, '"defaults-file"'], - ["required-sections", { "required-sections": "labels" }, '"required-sections"'], - ["a discovery filter", { forks: "exclude" }, '"forks"'], - ["private-report", { "private-report": "issue" }, '"private-report"'], - ["report-public-key", { "report-public-key": "age1x" }, '"report-public-key"'], - ["sections: the allowlist belongs on the apply step", { sections: "labels" }, '"sections"'], - ["on-missing-permission: warn", { "on-missing-permission": "warn" }, '"on-missing-permission"'], - ["a custom api-version", { "api-version": "2099-01-01" }, '"api-version"'], - ["private-repos: show", { "private-repos": "show" }, '"private-repos"'], - [ - "several at once, every one named in declaration order", - { repos: "o/a", "repos-dir": "repos", "private-report": "artifact" }, - '"repos", "repos-dir", "private-report"', - ], - [ - "a previously tolerated control beside an always-rejected one, both named", - { repos: "o/a", sections: "labels", "api-version": "2099-01-01" }, - '"sections", "api-version", "repos"', - ], - ] as const)( - "%s is rejected: a merge has no repository, API, report, or allowlist", - (_case, inputs, named) => { - setMergeEnv(inputs); - expect(rejection()).toBe( - `the ${named} input(s) do not apply to mode: merge, which only folds the settings-file ` + - `layers into merged-file: it never targets a repository, calls the GitHub API, ` + - `delivers a report, or narrows the sections it writes. Remove the input(s), or move ` + - `them to the apply or check step that runs the merged document`, - ); - }, - ); - - test("every declared default passes: the runner supplies them whether or not the workflow set the input", () => { - // Every input outside the merge env itself, at the exact value action.yml - // declares (token's is the unresolved workflow expression; the runner - // resolves it, which is why token is tolerated at any value). - const defaults = Object.fromEntries( - Object.entries(INPUT_DECLS) - .filter(([name]) => !["mode", "settings-file", "merged-file"].includes(name)) - .map(([name, decl]) => [name, decl.default]), - ); - setMergeEnv(defaults); - expect(parseConfig()).toEqual({ config: MERGE_CONFIG }); - }); - - test("token is tolerated and never carried: the config has no field to read it from", () => { - setMergeEnv({ token: "ghp_stepwide" }); - process.env.GITHUB_TOKEN = "ghp_envwide"; - const parsed = parseConfig(); - expect(parsed).toEqual({ config: MERGE_CONFIG }); - expect(JSON.stringify(parsed)).not.toContain("ghp_"); - }); }); diff --git a/test/discovery/central.test.ts b/test/discovery/central.test.ts index f18d49ad..8afe2486 100644 --- a/test/discovery/central.test.ts +++ b/test/discovery/central.test.ts @@ -1,57 +1,65 @@ import { describe, expect, test } from "bun:test"; +import { err, ok } from "neverthrow"; import { resolveCentralTargets } from "../../src/discovery/central.js"; describe("resolveCentralTargets", () => { test("reads owner-shorthand and owner/name files, warns on strays", () => { // The walk sorts directory entries, so the target and warning order is // fixed: top-level files first, then each owner directory. - expect(resolveCentralTargets("test/fixtures/repos", "viv")).toEqual({ - targets: [ - { - slug: "viv/api", - source: "central", - origin: "test/fixtures/repos/api.yml", - filePath: "test/fixtures/repos/api.yml", - }, - { - slug: "octo/web", - source: "central", - origin: "test/fixtures/repos/octo/web.yml", - filePath: "test/fixtures/repos/octo/web.yml", - }, - ], - warnings: [ - "ignoring test/fixtures/repos/README.md: not a .yml/.yaml file, so it defines no target repository", - "ignoring test/fixtures/repos/octo/deep: repos-dir supports only .yml and /.yml, nothing deeper. Move the files up or remove the directory", - ], - }); + expect(resolveCentralTargets("test/fixtures/repos", "viv")).toEqual( + ok({ + targets: [ + { + slug: "viv/api", + source: "central", + origin: "test/fixtures/repos/api.yml", + filePath: "test/fixtures/repos/api.yml", + }, + { + slug: "octo/web", + source: "central", + origin: "test/fixtures/repos/octo/web.yml", + filePath: "test/fixtures/repos/octo/web.yml", + }, + ], + warnings: [ + "ignoring test/fixtures/repos/README.md: not a .yml/.yaml file, so it defines no target repository", + "ignoring test/fixtures/repos/octo/deep: repos-dir supports only .yml and /.yml, nothing deeper. Move the files up or remove the directory", + ], + }), + ); }); - test("shorthand without a known admin owner is an error", () => { - expect(resolveCentralTargets("test/fixtures/repos", "")).toEqual({ - error: - 'repos-dir "test/fixtures/repos" has 1 invalid settings file(s):\n' + - "- cannot resolve test/fixtures/repos/api.yml: top-level repos-dir files use the current " + - "repository's owner, which is unknown outside GitHub Actions. Use the /.yml " + - "layout instead", - }); + test("shorthand without a known admin owner is refused, naming the ownerless files", () => { + expect(resolveCentralTargets("test/fixtures/repos", "")).toEqual( + err({ + code: "repos-dir-invalid-files", + reposDir: "test/fixtures/repos", + files: [{ kind: "ownerless", files: ["test/fixtures/repos/api.yml"] }], + }), + ); }); - test("the same repo defined twice is an error", () => { - expect(resolveCentralTargets("test/fixtures/repos-dup", "viv")).toEqual({ - error: - 'repos-dir "test/fixtures/repos-dup" has 1 invalid settings file(s):\n' + - "- duplicate target viv/x: defined by both test/fixtures/repos-dup/viv/x.yml and " + - "test/fixtures/repos-dup/x.yml. Keep exactly one settings file per repository", - }); + test("the same repo defined twice is refused, naming both files", () => { + expect(resolveCentralTargets("test/fixtures/repos-dup", "viv")).toEqual( + err({ + code: "repos-dir-invalid-files", + reposDir: "test/fixtures/repos-dup", + files: [ + { + kind: "duplicate", + slug: "viv/x", + first: "test/fixtures/repos-dup/viv/x.yml", + second: "test/fixtures/repos-dup/x.yml", + }, + ], + }), + ); }); - test("missing dir errors with a checkout hint", () => { - expect(resolveCentralTargets("test/fixtures/nope", "viv")).toEqual({ - error: - 'repos-dir "test/fixtures/nope" does not exist in the workspace, so there are no central ' + - "settings files to read. Add an actions/checkout step before this action, or fix the " + - "repos-dir path", - }); + test("a missing dir is its own problem", () => { + expect(resolveCentralTargets("test/fixtures/nope", "viv")).toEqual( + err({ code: "repos-dir-missing", reposDir: "test/fixtures/nope" }), + ); }); }); diff --git a/test/discovery/discover.test.ts b/test/discovery/discover.test.ts index 474e6604..ca59a1e8 100644 --- a/test/discovery/discover.test.ts +++ b/test/discovery/discover.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { err } from "neverthrow"; import { DEFAULT_DISCOVERY_FILTERS, type DiscoveredRepoRef, @@ -9,6 +10,7 @@ import { formatSkipNotice, } from "../../src/discovery/discover.js"; import { markPrivate } from "../../src/private.js"; +import { describeProblem } from "../../src/problem.js"; import { MockApi } from "../mock-api.js"; describe("excludeMatches", () => { @@ -44,11 +46,12 @@ describe("discoverRepos", () => { routes: ConstructorParameters[0], overrides: Partial = {}, ) => { - const discovered = await discoverRepos(new MockApi(routes), filters(overrides)); - if ("error" in discovered) { - throw new Error(discovered.error); - } - return discovered; + return (await discoverRepos(new MockApi(routes), filters(overrides))).match( + (discovered) => discovered, + (problem) => { + throw new Error(describeProblem(problem)); + }, + ); }; const slugs = (repos: DiscoveredRepoRef[]) => repos.map((repo) => repo.slug); /** Filtered refs by reason; a non-public one compares against its sealed slug (`hidden`). */ @@ -227,15 +230,15 @@ describe("discoverRepos", () => { error: { status: 403, message: "Resource not accessible", body: "" }, }, }); - const discovered = await discoverRepos(api, DEFAULT_DISCOVERY_FILTERS); - expect(discovered).toEqual({ - error: - 'cannot discover repositories for repos: "*": GET /user/repos?affiliation=owner failed: ' + - "403 Resource not accessible. " + - "Discovery needs a user PAT; the workflow GITHUB_TOKEN and GitHub App installation tokens " + - "cannot enumerate a user's repositories. List the target repositories explicitly in the " + - '"repos" input', - }); + expect(await discoverRepos(api, DEFAULT_DISCOVERY_FILTERS)).toEqual( + err({ + code: "discovery-request-failed", + path: "/user/repos?affiliation=owner", + status: 403, + message: "Resource not accessible", + denied: true, + }), + ); }); test("a rate-limit 403 gets re-run advice, not PAT advice", async () => { @@ -247,13 +250,15 @@ describe("discoverRepos", () => { error: { status: 403, message: "API rate limit exceeded for user", body: "" }, }, }); - const discovered = await discoverRepos(api, DEFAULT_DISCOVERY_FILTERS); - expect(discovered).toEqual({ - error: - 'cannot discover repositories for repos: "*": GET /user/repos?affiliation=owner failed: ' + - "403 API rate limit exceeded for user. " + - "This is not a permission problem; re-run the workflow, and retry later if it persists", - }); + expect(await discoverRepos(api, DEFAULT_DISCOVERY_FILTERS)).toEqual( + err({ + code: "discovery-request-failed", + path: "/user/repos?affiliation=owner", + status: 403, + message: "API rate limit exceeded for user", + denied: false, + }), + ); }); test("an expired-token 401 explains the PAT requirement", async () => { @@ -262,15 +267,15 @@ describe("discoverRepos", () => { error: { status: 401, message: "Bad credentials", body: "" }, }, }); - const discovered = await discoverRepos(api, DEFAULT_DISCOVERY_FILTERS); - expect(discovered).toEqual({ - error: - 'cannot discover repositories for repos: "*": GET /user/repos?affiliation=owner failed: ' + - "401 Bad credentials. " + - "Discovery needs a user PAT; the workflow GITHUB_TOKEN and GitHub App installation tokens " + - "cannot enumerate a user's repositories. List the target repositories explicitly in the " + - '"repos" input', - }); + expect(await discoverRepos(api, DEFAULT_DISCOVERY_FILTERS)).toEqual( + err({ + code: "discovery-request-failed", + path: "/user/repos?affiliation=owner", + status: 401, + message: "Bad credentials", + denied: true, + }), + ); }); test("a server error gets re-run advice, not PAT advice", async () => { @@ -279,13 +284,15 @@ describe("discoverRepos", () => { error: { status: 500, message: "boom", body: "" }, }, }); - const discovered = await discoverRepos(api, DEFAULT_DISCOVERY_FILTERS); - expect(discovered).toEqual({ - error: - 'cannot discover repositories for repos: "*": GET /user/repos?affiliation=owner failed: ' + - "500 boom. " + - "This is not a permission problem; re-run the workflow, and retry later if it persists", - }); + expect(await discoverRepos(api, DEFAULT_DISCOVERY_FILTERS)).toEqual( + err({ + code: "discovery-request-failed", + path: "/user/repos?affiliation=owner", + status: 500, + message: "boom", + denied: false, + }), + ); }); }); diff --git a/test/discovery/repos-input.test.ts b/test/discovery/repos-input.test.ts index 2c6637bc..a9c04a0e 100644 --- a/test/discovery/repos-input.test.ts +++ b/test/discovery/repos-input.test.ts @@ -1,37 +1,28 @@ import { describe, expect, test } from "bun:test"; +import { err, ok } from "neverthrow"; import { parseReposInput } from "../../src/discovery/repos-input.js"; describe("parseReposInput", () => { test("splits on commas and newlines", () => { - expect(parseReposInput("o/a, o/b\no/c")).toEqual({ - slugs: ["o/a", "o/b", "o/c"], - discover: false, - }); + expect(parseReposInput("o/a, o/b\no/c")).toEqual( + ok({ slugs: ["o/a", "o/b", "o/c"], discover: false }), + ); }); test("* alone switches to discovery", () => { - expect(parseReposInput("*")).toEqual({ slugs: [], discover: true }); + expect(parseReposInput("*")).toEqual(ok({ slugs: [], discover: true })); }); - test("* mixed with slugs is an error", () => { - expect(parseReposInput("*, o/a")).toEqual({ - error: - 'the "repos" input mixes "*" with explicit repositories. Use "*" alone to discover every ' + - "repository the token owns, or list the repositories without it", - }); + test("* mixed with slugs is refused", () => { + expect(parseReposInput("*, o/a")).toEqual(err({ code: "repos-input-wildcard-mixed" })); }); - test("bad slugs and duplicates are reported once, together, with counts", () => { - expect(parseReposInput("not-a-slug")).toEqual({ - error: - 'the "repos" input has 1 invalid entry: "not-a-slug" is not an owner/name slug (use values like "octocat/hello-world", comma- or newline-separated). Or use "*" alone to discover repositories', - }); - expect(parseReposInput("o/a, O/A, bad, bad, worse")).toEqual({ - error: - 'the "repos" input has 3 invalid entries: "bad", "worse" are not owner/name slugs (use ' + - 'values like "octocat/hello-world", comma- or newline-separated); "O/A" is listed more ' + - 'than once (keep exactly one entry per repository). Or use "*" alone to discover ' + - "repositories", - }); + test("bad slugs and duplicates are reported once, together, each pasted twice counting once", () => { + expect(parseReposInput("not-a-slug")).toEqual( + err({ code: "repos-input-invalid-entries", invalid: ["not-a-slug"], duplicated: [] }), + ); + expect(parseReposInput("o/a, O/A, bad, bad, worse")).toEqual( + err({ code: "repos-input-invalid-entries", invalid: ["bad", "worse"], duplicated: ["O/A"] }), + ); }); }); diff --git a/test/docs/guides.test.ts b/test/docs/guides.test.ts index f1556e00..f4054c0c 100644 --- a/test/docs/guides.test.ts +++ b/test/docs/guides.test.ts @@ -13,12 +13,14 @@ import { describe, expect, test } from "bun:test"; import { existsSync, readdirSync, readFileSync } from "node:fs"; import { join, posix } from "node:path"; import { DecodingMode, decodeHTML } from "entities"; +import { ok } from "neverthrow"; import { parse as parseYaml } from "yaml"; -import { MERGE_REJECTED_INPUTS } from "../../src/action/inputs.js"; import { type Layer, mergeLayers, stripNulls } from "../../src/engine/layers.js"; import { validateSettingsDoc } from "../../src/engine/orchestrate.js"; +import { MERGE_REJECTED_INPUTS } from "../../src/flows/inputs.js"; import { foldLayers } from "../../src/flows/layers.js"; import { silentIo } from "../../src/io.js"; +import { describeProblem } from "../../src/problem.js"; import { SECTION_KEYS } from "../../src/schema.js"; import { NESTED_KEYS } from "../../src/sections/environments/nested.js"; import { SECTIONS } from "../../src/sections/registry.js"; @@ -718,10 +720,9 @@ describe("docs/ guide pages", () => { expect(layers).toHaveLength(count); const results = fencedBlocks(section, "yaml settings").map((block) => parseYaml(block)); expect(results).toHaveLength(1); - expect(mergeLayers(layers, { layering: "merge" })).toEqual({ - settings: results[0], - notices, - }); + expect(mergeLayers(layers, { layering: "merge" })).toEqual( + ok({ settings: results[0], notices }), + ); }, ); @@ -774,13 +775,17 @@ describe("docs/ guide pages", () => { ? malformedSectionEntries(layerName, row.quoted) : `layer "${layerName}": ${row.quoted}`; const folded = foldLayers([{ name: layerName, doc }], "merged", "merge", silentIo()); - expect("error" in folded ? folded.error : null, `layer ${input}`).toBe(expected); + expect( + folded.match(() => null, describeProblem), + `layer ${input}`, + ).toBe(expected); if (row.gate === "validation") { // "with the same messages a standalone file gets" const standalone = validateSettingsDoc(doc, layerName, new Set(), silentIo()); - expect("error" in standalone ? standalone.error : null, `standalone ${input}`).toBe( - expected, - ); + expect( + standalone.match(() => null, describeProblem), + `standalone ${input}`, + ).toBe(expected); } } }); @@ -796,11 +801,12 @@ describe("docs/ guide pages", () => { new Set(), silentIo(), ); - if (!("error" in result)) { + if (result.isOk()) { throw new Error("the validator accepted the v2 wrapper key"); } - expect(result.error).toContain('"undeclared" was renamed to "_undeclared"'); - expect(fencedBlocks(guide, "text").map((block) => block.trim())).toContain(result.error); + const message = describeProblem(result.error); + expect(message).toContain('"undeclared" was renamed to "_undeclared"'); + expect(fencedBlocks(guide, "text").map((block) => block.trim())).toContain(message); }); test("the troubleshooting guide quotes the stale-version hint verbatim", () => { diff --git a/test/docs/readme.test.ts b/test/docs/readme.test.ts index 5310fdac..015d8bff 100644 --- a/test/docs/readme.test.ts +++ b/test/docs/readme.test.ts @@ -12,8 +12,8 @@ import { existsSync, readdirSync, readFileSync } from "node:fs"; import { join } from "node:path"; import { parse as parseYaml } from "yaml"; import { countWord } from "../../.github/scripts/lib/count-word.js"; -import { DEFAULT_PRIVATE_REPOS } from "../../src/action/inputs.js"; import { REPO_RESULTS } from "../../src/engine/orchestrate.js"; +import { DEFAULT_PRIVATE_REPOS } from "../../src/flows/inputs.js"; import { REDACTED_DETAIL } from "../../src/flows/redact.js"; import { ARTIFACT_FILE, ARTIFACT_NAME } from "../../src/report/artifact-report.js"; import { PRIVATE_REPORT_CHANNELS } from "../../src/report/delivery.js"; diff --git a/test/e2e/generators.test.ts b/test/e2e/generators.test.ts index 39f4ede9..2fb50970 100644 --- a/test/e2e/generators.test.ts +++ b/test/e2e/generators.test.ts @@ -5,6 +5,7 @@ import { parse as parseYaml, stringify as stringifyYaml } from "yaml"; import { describeOptOut } from "../../src/engine/layers.js"; import { validateSettingsDoc } from "../../src/engine/orchestrate.js"; import { silentIo } from "../../src/io.js"; +import { describeProblem } from "../../src/problem.js"; import { SECTION_KEYS, type SectionKey } from "../../src/schema.js"; import { allEndpoints, sectionShape } from "../../src/sections/registry.js"; import { type LiveWitnessKind, UNDECLARED_KEY } from "./gen-support.js"; @@ -473,12 +474,13 @@ describe("genInvalidSettings", () => { for (let i = 0; i < 25; i++) { const { doc, offendingToken } = build(new Rng(i * 13 + 1)); const verdict = validateSettingsDoc(doc, "settings.yml", new Set(), silentIo()); - if (!("error" in verdict)) { + if (verdict.isOk()) { throw new Error(`case "${name}" produced a doc the validator accepts`); } - if (!verdict.error.includes(offendingToken)) { + const rendered = describeProblem(verdict.error); + if (!rendered.includes(offendingToken)) { throw new Error( - `case "${name}" token "${offendingToken}" missing from error: ${verdict.error}`, + `case "${name}" token "${offendingToken}" missing from error: ${rendered}`, ); } } diff --git a/test/e2e/oracle.test.ts b/test/e2e/oracle.test.ts index 45565a39..86ff9712 100644 --- a/test/e2e/oracle.test.ts +++ b/test/e2e/oracle.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; import { describeOptOut, mergeLayers, type OptOutNotice } from "../../src/engine/layers.js"; +import { describeProblem } from "../../src/problem.js"; import { UNDECLARED_POLICY_SECTIONS } from "../../src/schema.js"; import { sectionModule } from "../../src/sections/registry.js"; import { ADMIN_SLUG } from "./constants.js"; @@ -1095,11 +1096,12 @@ function stack(...docs: Record[]): MergeLayer[] { /** The notices the ENGINE's fold reports for a stack, to pin the oracle's paths against. */ function engineNotices(layers: readonly MergeLayer[]): OptOutNotice[] { - const result = mergeLayers(layers, { layering: "merge" }); - if ("error" in result) { - throw new Error(`the engine refused a stack the oracle folded: ${result.error}`); - } - return result.notices; + return mergeLayers(layers, { layering: "merge" }).match( + (folded) => folded.notices, + (problem) => { + throw new Error(`the engine refused a stack the oracle folded: ${describeProblem(problem)}`); + }, + ); } describe("foldMergeLayers (the oracle's own dialect)", () => { diff --git a/test/e2e/oracle.ts b/test/e2e/oracle.ts index c09befbb..332384d0 100644 --- a/test/e2e/oracle.ts +++ b/test/e2e/oracle.ts @@ -10,6 +10,7 @@ import type { OptOutNotice } from "../../src/engine/layers.js"; import { validateSettingsDoc } from "../../src/engine/orchestrate.js"; import { silentIo } from "../../src/io.js"; +import { describeProblem } from "../../src/problem.js"; import { SECTION_KEYS, type SectionKey, @@ -1248,8 +1249,8 @@ export function predictMerge(meta: MergeScenarioMeta): MergePrediction { // document it just folded (cross-field rules the published schema cannot // spell, so the generator cannot avoid them by construction). const validated = validateSettingsDoc(folded.merged, "merged", new Set(), silentIo()); - if ("error" in validated) { - return { kind: "invalid", error: validated.error }; + if (validated.isErr()) { + return { kind: "invalid", error: describeProblem(validated.error) }; } return { kind: "merged", ...folded }; } diff --git a/test/e2e/runner.test.ts b/test/e2e/runner.test.ts index fd94d3dc..1e9ea07c 100644 --- a/test/e2e/runner.test.ts +++ b/test/e2e/runner.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from "bun:test"; import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { ok } from "neverthrow"; import { parseRecipient } from "../../src/report/artifact-report.js"; import { ARTIFACT_TEST_RECIPIENT } from "./generators.js"; import { @@ -54,7 +55,7 @@ describe("ARTIFACT_TEST_RECIPIENT", () => { // ever stops parsing, every artifact-delivery scenario would silently fall // into the config-rejection path instead. Pin it against the same validator // the action uses at config parse. - expect(parseRecipient(ARTIFACT_TEST_RECIPIENT)).toEqual({ ok: true }); + expect(parseRecipient(ARTIFACT_TEST_RECIPIENT)).toEqual(ok()); }); }); diff --git a/test/e2e/schema.ts b/test/e2e/schema.ts index 1f24a26c..cad75ebe 100644 --- a/test/e2e/schema.ts +++ b/test/e2e/schema.ts @@ -11,8 +11,8 @@ import { type Dirent, readdirSync, readFileSync } from "node:fs"; import { join } from "node:path"; import { parse as parseYaml, stringify as stringifyYaml } from "yaml"; import { z } from "zod"; -import { FILTER_INPUTS } from "../../src/action/inputs.js"; import { RESERVED_REF_PREFIXES } from "../../src/engine/secret-refs.js"; +import { FILTER_INPUTS } from "../../src/flows/inputs.js"; import { MARKER_LABEL, MARKER_LABEL_CONFIG } from "../../src/report/issue-report.js"; import { SECTION_KEYS } from "../../src/schema.js"; import type { PatResource } from "../../src/sections/contract/permissions.js"; diff --git a/test/engine/check-purity.test.ts b/test/engine/check-purity.test.ts index 60b79e2b..b9276fd2 100644 --- a/test/engine/check-purity.test.ts +++ b/test/engine/check-purity.test.ts @@ -6,7 +6,9 @@ import { describe, expect, test } from "bun:test"; import { runForRepo, validateSettingsDoc } from "../../src/engine/orchestrate.js"; +import { SectionSelection } from "../../src/engine/section-selection.js"; import { silentIo } from "../../src/io.js"; +import { describeProblem } from "../../src/problem.js"; import type { SectionKey } from "../../src/schema.js"; import { SECTION_KEYS } from "../../src/schema.js"; import { MOCK_SECRETS_KEY_ID, MOCK_SECRETS_PUBLIC_KEY } from "../e2e/mock/secrets.js"; @@ -226,18 +228,17 @@ describe("check-mode purity", () => { // Brand the fixture document through the REAL boundary: an invalid // fixture fails loudly here instead of riding a cast into runForRepo. const verdict = validateSettingsDoc(FIXTURES, "purity fixtures", new Set(), silentIo()); - if ("error" in verdict) { - throw new Error(`purity fixtures failed validation: ${verdict.error}`); + if (verdict.isErr()) { + throw new Error(`purity fixtures failed validation: ${describeProblem(verdict.error)}`); } const result = await runForRepo( api, { repo: { owner: "o", name: "r", slug: "o/r" }, - settings: verdict.settings, + settings: verdict.value, mode: "check", onMissingPermission: "fail", - requiredSections: new Set(), - onlySections: new Set(), + sections: SectionSelection.ALL, }, silentIo(), ); diff --git a/test/engine/layers.test.ts b/test/engine/layers.test.ts index be5bef37..29345cfe 100644 --- a/test/engine/layers.test.ts +++ b/test/engine/layers.test.ts @@ -1,7 +1,9 @@ import { describe, expect, test } from "bun:test"; +import { err, ok } from "neverthrow"; import { type Layer, type Layering, mergeLayers, stripNulls } from "../../src/engine/layers.js"; import { validateSettingsDoc } from "../../src/engine/orchestrate.js"; import { silentIo } from "../../src/io.js"; +import { describeProblem, type LayerProblem } from "../../src/problem.js"; import { planContext } from "../../src/sections/contract/plan.js"; import { labelsSection } from "../../src/sections/labels/index.js"; import { MockApi } from "../mock-api.js"; @@ -26,8 +28,12 @@ function layer(name: string, doc: unknown): Layer { return { name, doc: deepFreeze(doc) }; } +/** The fold's verdict as the tests pin it: the merged document, or the refusal's code and its prose. */ function merge(layers: Layer[], layering: Layering = "merge") { - return mergeLayers(layers, { layering }); + return mergeLayers(layers, { layering }).match( + (folded) => folded, + (problem) => ({ code: problem.code, error: describeProblem(problem) }), + ); } const MAIN_RULESET = { @@ -644,95 +650,112 @@ describe("mergeLayers: the _layering directive", () => { describe("mergeLayers: layer-boundary refusals", () => { const fleet = layer("fleet", { labels: [{ name: "fleet" }], milestones: [{ title: "v0" }] }); - test.each([ + test.each<[string, unknown, LayerProblem["code"], string]>([ [ "a rule without a type", { rulesets: [{ name: "main", rules: [{ parameters: {} }] }] }, + "layer-no-key", 'layer "repo": rulesets[0].rules[0] carries no string "type", which every entry needs to layer by', ], [ "a duplicate rule type in one ruleset", { rulesets: [{ name: "main", rules: [{ type: "deletion" }, { type: "deletion" }] }] }, + "layer-duplicate-key", 'layer "repo": rulesets[0].rules[0] and rulesets[0].rules[1] both claim one type; each type belongs to one entry within a layer', ], [ "a non-mapping rule", { rulesets: [{ name: "main", rules: ["deletion"] }] }, + "layer-wrong-shape", 'layer "repo": rulesets[0].rules[0] must be a mapping; got a string', ], [ "a non-mapping rule in the second ruleset, after a mapping rule", { rulesets: [{ name: "tags" }, { name: "main", rules: [{ type: "deletion" }, 7] }] }, + "layer-wrong-shape", 'layer "repo": rulesets[1].rules[1] must be a mapping; got a number', ], [ "duplicate label names, case-folded", { labels: [{ name: "Bug" }, { name: "bug" }] }, + "layer-duplicate-key", 'layer "repo": labels[0] and labels[1] both claim one name; each name belongs to one entry within a layer', ], [ "a label renaming into a sibling's name in one layer", { labels: [{ name: "bug", new_name: "Defect" }, { name: "docs" }, { name: "defect" }] }, + "layer-duplicate-key", 'layer "repo": labels[0] and labels[2] both claim one name; each name belongs to one entry within a layer', ], [ "duplicate ruleset names", { rulesets: [{ name: "main" }, { name: "main" }] }, + "layer-duplicate-key", 'layer "repo": rulesets[0] and rulesets[1] both claim one name; each name belongs to one entry within a layer', ], [ "a nameless label", { labels: [{ name: "ok" }, { color: "ffffff" }] }, + "layer-no-key", 'layer "repo": labels[1] carries no string "name", which every entry needs to layer by', ], [ "a scalar where a list belongs", { labels: "oops" }, + "layer-wrong-shape", 'layer "repo": labels must be a list of mappings or an {_undeclared, entries} wrapper; got a string', ], [ "a wrapper without entries", { labels: { _undeclared: "keep" } }, + "layer-wrong-shape", 'layer "repo": labels must be a list of mappings or an {_undeclared, entries} wrapper; got a mapping without an entries list', ], [ "a YAML-tagged value where a list belongs", { milestones: new Date(0) }, + "layer-wrong-shape", 'layer "repo": milestones must be a list of mappings or an {_undeclared, entries} wrapper; got a Date value', ], [ "a non-mapping entry in a section without a layering key", { milestones: [{ title: "v1" }, "v2"] }, + "layer-wrong-shape", 'layer "repo": milestones[1] must be a mapping; got a string', ], [ "an invalid top-level directive", { _layering: "union", labels: [{ name: "mine" }] }, + "layer-bad-directive", 'layer "repo": _layering must be "merge" or "replace"; got a string that is neither', ], [ "an invalid wrapper directive", { labels: { _layering: "union", entries: [{ name: "mine" }] } }, + "layer-bad-directive", 'layer "repo": labels._layering must be "merge" or "replace"; got a string that is neither', ], [ "a non-string wrapper directive", { labels: { _layering: true, entries: [{ name: "mine" }] } }, + "layer-bad-directive", 'layer "repo": labels._layering must be "merge" or "replace"; got a boolean', ], [ "a wrapper merge directive on a section without a layering key", { milestones: { _layering: "merge", entries: [{ title: "v1" }] } }, + "layer-no-layering-key", 'layer "repo": milestones has no layering key, so it cannot be layered by "merge"; declare _layering: replace or drop the directive', ], [ "a file-level merge directive reaching a section without a layering key", { _layering: "merge", milestones: [{ title: "v1" }] }, + "layer-no-layering-key", 'layer "repo": milestones has no layering key, so it cannot be layered by "merge"; declare _layering: replace or drop the directive', ], - ])("%s is refused naming the layer", (_case, doc, error) => { - expect(merge([fleet, layer("repo", doc)])).toEqual({ error }); - expect(merge([fleet, layer("repo", doc)], "replace")).toEqual({ error }); + ])("%s is refused naming the layer", (_case, doc, code, error) => { + expect(merge([fleet, layer("repo", doc)])).toEqual({ code, error }); + expect(merge([fleet, layer("repo", doc)], "replace")).toEqual({ code, error }); }); test("a section without a layering key merges under the run default without complaint", () => { @@ -753,25 +776,29 @@ describe("mergeLayers: layer-boundary refusals", () => { const M = "ZZ_MARKER"; const cyclic: Record = { [M]: M }; cyclic[`${M}_self`] = cyclic; - const shaped: [string, unknown, string][] = [ + const shaped: [string, unknown, LayerProblem["code"], string][] = [ [ "a scalar section", { labels: M }, + "layer-wrong-shape", 'layer "repo": labels must be a list of mappings or an {_undeclared, entries} wrapper; got a string', ], [ "a wrapper without entries", { labels: { _undeclared: M, [M]: M } }, + "layer-wrong-shape", 'layer "repo": labels must be a list of mappings or an {_undeclared, entries} wrapper; got a mapping without an entries list', ], [ "a scalar entry", { rulesets: [{ name: M, rules: [M] }] }, + "layer-wrong-shape", 'layer "repo": rulesets[0].rules[0] must be a mapping; got a string', ], [ "a directive that is a marker", { labels: { _layering: M, entries: [{ name: M }] } }, + "layer-bad-directive", 'layer "repo": labels._layering must be "merge" or "replace"; got a string that is neither', ], ]; @@ -785,8 +812,8 @@ describe("mergeLayers: layer-boundary refusals", () => { ["two entries claiming one key", { labels: [{ name: M }, { name: M.toLowerCase() }] }], ["a cycle", { repository: cyclic }], ]; - for (const [, doc, error] of shaped) { - expect(merge([fleet, layer("repo", doc)])).toEqual({ error }); + for (const [, doc, code, error] of shaped) { + expect(merge([fleet, layer("repo", doc)])).toEqual({ code, error }); } for (const [name, doc] of [...shaped, ...kinded]) { const result = merge([fleet, layer("repo", doc)]); @@ -822,10 +849,13 @@ describe("mergeLayers: cyclic documents", () => { ])( "a layer that includes itself through %s is refused by name, before any merge", (_kind, make) => { - const error = - 'layer "repo": the document contains a reference cycle (a YAML anchor that includes itself); layers must be trees'; - expect(merge([fleet, layer("repo", make())])).toEqual({ error }); - expect(merge([layer("repo", make()), fleet])).toEqual({ error }); + const refused = { + code: "layer-cycle" as const, + error: + 'layer "repo": the document contains a reference cycle (a YAML anchor that includes itself); layers must be trees', + }; + expect(merge([fleet, layer("repo", make())])).toEqual(refused); + expect(merge([layer("repo", make()), fleet])).toEqual(refused); }, ); @@ -973,10 +1003,14 @@ describe("stripNulls", () => { // Widened so the whole verdict can be pinned by value; the brand is opaque to toEqual. const validate = (doc: unknown): unknown => validateSettingsDoc(doc, "repo", new Set(), silentIo()); - expect(validate(upper)).toEqual({ - error: expect.stringContaining("rulesets"), - }); - expect(validate(stripNulls(upper))).toEqual({ settings: { rulesets: [{ name: "main" }] } }); + expect(validate(upper)).toEqual( + err({ + code: "settings-malformed-sections", + source: "repo", + issues: [expect.stringContaining("rulesets")], + }), + ); + expect(validate(stripNulls(upper))).toEqual(ok({ rulesets: [{ name: "main" }] })); const merged = merge([layer("fleet", lower), layer("repo", upper)]); expect(merged).toEqual({ settings: { rulesets: { _undeclared: "keep", entries: [MAIN_RULESET] } }, @@ -985,9 +1019,9 @@ describe("stripNulls", () => { if ("error" in merged) { throw new Error(merged.error); } - expect(validate(merged.settings)).toEqual({ - settings: { rulesets: { _undeclared: "keep", entries: [MAIN_RULESET] } }, - }); + expect(validate(merged.settings)).toEqual( + ok({ rulesets: { _undeclared: "keep", entries: [MAIN_RULESET] } }), + ); }); test("a key named __proto__ survives as an own property", () => { diff --git a/test/engine/orchestrate.test.ts b/test/engine/orchestrate.test.ts index cc19fb46..18693abf 100644 --- a/test/engine/orchestrate.test.ts +++ b/test/engine/orchestrate.test.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, spyOn, test } from "bun:test"; +import { err } from "neverthrow"; import { preflightProbe, @@ -9,9 +10,11 @@ import { validateSettingsDoc, worstOf, } from "../../src/engine/orchestrate.js"; +import { SectionSelection } from "../../src/engine/section-selection.js"; import type { Io } from "../../src/io.js"; import { maskRegistry, prefixedIo } from "../../src/io.js"; -import type { SettingsFile } from "../../src/schema.js"; +import { describeProblem, type TopLevelShape } from "../../src/problem.js"; +import { SECTION_KEYS, type SettingsFile } from "../../src/schema.js"; import type { EndpointDecl } from "../../src/sections/contract/endpoints.js"; import type { SectionPlan } from "../../src/sections/contract/plan.js"; import { pagesSection } from "../../src/sections/pages/index.js"; @@ -51,11 +54,12 @@ function validated(doc: SettingsFile): ValidatedSettings { output: () => {}, ...maskRegistry(() => {}), }; - const verdict = validateSettingsDoc(doc, "test fixture", new Set(), silent); - if ("error" in verdict) { - throw new Error(`test fixture failed validation: ${verdict.error}`); - } - return verdict.settings; + return validateSettingsDoc(doc, "test fixture", new Set(), silent).match( + (settings) => settings, + (problem) => { + throw new Error(`test fixture failed validation: ${describeProblem(problem)}`); + }, + ); } function opts(overrides: Partial = {}): RepoRunOptions { @@ -64,8 +68,7 @@ function opts(overrides: Partial = {}): RepoRunOptions { settings: validated({ repository: { has_wiki: false } }), mode: "apply" as const, onMissingPermission: "fail" as const, - requiredSections: new Set(), - onlySections: new Set(), + sections: SectionSelection.ALL, ...overrides, }; } @@ -193,9 +196,12 @@ describe("runForRepo", () => { const polluted = JSON.parse( '{"rulesets":{"entries":[{"name":"r"}],"__proto__":{"planted":2}}}', ); - const verdict = validateSettingsDoc(polluted, "s.yml", new Set(), captureIo().io); - expect("error" in verdict ? verdict.error : "").toContain( - 'rulesets: Unrecognized key: "__proto__"', + expect(validateSettingsDoc(polluted, "s.yml", new Set(), captureIo().io)).toEqual( + err({ + code: "settings-malformed-sections", + source: "s.yml", + issues: [expect.stringContaining('rulesets: Unrecognized key: "__proto__"')], + }), ); }); @@ -319,7 +325,7 @@ describe("runForRepo secret references", () => { ...webhookSettings("a-literal-that-would-fail"), repository: { has_wiki: false }, }), - onlySections: new Set(["repository"]), + sections: SectionSelection.of({ only: ["repository"] })._unsafeUnwrap(), }), io, ); @@ -328,19 +334,28 @@ describe("runForRepo secret references", () => { }); describe("validateSettingsDoc", () => { - const errorOf = (verdict: ReturnType): string => - "error" in verdict ? verdict.error : ""; - - test("unknown top-level keys are errors naming the source", () => { + test("unknown top-level keys are a problem naming the source and the known sections", () => { const { io } = captureIo(); - const err = errorOf(validateSettingsDoc({ labls: [] }, "repos/x.yml", new Set(), io)); - expect(err).toContain("repos/x.yml"); - expect(err).toContain("labls"); + expect(validateSettingsDoc({ labls: [] }, "repos/x.yml", new Set(), io)).toEqual( + err({ + code: "settings-unknown-sections", + source: "repos/x.yml", + unknown: ["labls"], + known: SECTION_KEYS, + }), + ); }); - test("non-mapping documents are rejected", () => { + test.each<[what: string, doc: unknown, shape: TopLevelShape]>([ + ["a list", [], "list"], + ["null", null, "null"], + ["a string", "labels", "string"], + ["a number", 7, "number"], + ])("a non-mapping document (%s) is rejected with its shape", (_what, doc, shape) => { const { io } = captureIo(); - expect(errorOf(validateSettingsDoc([], "f.yml", new Set(), io))).toContain("a list"); + expect(validateSettingsDoc(doc, "f.yml", new Set(), io)).toEqual( + err({ code: "settings-not-mapping", source: "f.yml", shape }), + ); }); test("a YAML-tagged top-level value (a Date) is rejected, never branded", () => { @@ -348,23 +363,16 @@ describe("validateSettingsDoc", () => { // branding it valid would turn the whole document into a silent green // no-op. Only a plain-prototype mapping may pass the boundary. const { io } = captureIo(); - const err = errorOf(validateSettingsDoc(new Date(0), "f.yml", new Set(), io)); - expect(err).toContain("plain YAML mapping"); - expect(err).toContain("!!timestamp"); - expect(errorOf(validateSettingsDoc(new Set(["a"]), "f.yml", new Set(), io))).toContain( - "plain YAML mapping", - ); + const tagged = err({ code: "settings-not-plain-mapping" as const, source: "f.yml" }); + expect(validateSettingsDoc(new Date(0), "f.yml", new Set(), io)).toEqual(tagged); + expect(validateSettingsDoc(new Set(["a"]), "f.yml", new Set(), io)).toEqual(tagged); }); test("a valid document comes back branded, ready for runForRepo", () => { const { io } = captureIo(); const doc = { repository: { has_wiki: false } }; - const verdict = validateSettingsDoc(doc, "s.yml", new Set(), io); - if ("error" in verdict) { - throw new Error(`expected the document to validate: ${verdict.error}`); - } // The brand is compile-time only; the value is zod's parsed copy. - const branded: unknown = verdict.settings; + const branded: unknown = validateSettingsDoc(doc, "s.yml", new Set(), io)._unsafeUnwrap(); expect(branded).toEqual(doc); expect(branded).not.toBe(doc); }); diff --git a/test/engine/secrets.test.ts b/test/engine/secrets.test.ts index aff98cef..c20ea294 100644 --- a/test/engine/secrets.test.ts +++ b/test/engine/secrets.test.ts @@ -6,7 +6,9 @@ import { } from "../../src/engine/orchestrate.js"; import type { SettingsSource } from "../../src/engine/secret-refs.js"; import { collectSecretValues } from "../../src/engine/secrets.js"; +import { SectionSelection } from "../../src/engine/section-selection.js"; import { type Io, maskRegistry } from "../../src/io.js"; +import { describeProblem } from "../../src/problem.js"; import type { SectionKey, SettingsFile } from "../../src/schema.js"; import { SECTIONS } from "../../src/sections/registry.js"; import { MockApi } from "../mock-api.js"; @@ -162,17 +164,16 @@ describe("runForRepo provenance", () => { ...maskRegistry(() => {}), }; const verdict = validateSettingsDoc(doc, "fixture", new Set(), silent); - if ("error" in verdict) { - throw new Error(`fixture failed validation: ${verdict.error}`); + if (verdict.isErr()) { + throw new Error(`fixture failed validation: ${describeProblem(verdict.error)}`); } - return verdict.settings; + return verdict.value; }; const baseOpts = (settings: unknown) => ({ repo: { owner: "o", name: "r", slug: "o/r" }, settings: validated(settings), onMissingPermission: "fail" as const, - requiredSections: new Set(), - onlySections: new Set(), + sections: SectionSelection.ALL, }); /** A remote target's own document, run the way multi.ts runs it. */ const targetOpts = (targetDoc: SettingsFile) => ({ @@ -245,7 +246,7 @@ describe("runForRepo provenance", () => { { ...targetOpts(targetDoc), mode: "check" as const, - onlySections: new Set(["labels"]), + sections: SectionSelection.of({ only: ["labels"] })._unsafeUnwrap(), }, io, ); diff --git a/test/engine/section-selection.test.ts b/test/engine/section-selection.test.ts new file mode 100644 index 00000000..6c7b51ae --- /dev/null +++ b/test/engine/section-selection.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, test } from "bun:test"; +import { err } from "neverthrow"; +import { SectionSelection } from "../../src/engine/section-selection.js"; +import type { SectionKey } from "../../src/schema.js"; + +describe("SectionSelection.of", () => { + test.each< + [what: string, only: SectionKey[], required: SectionKey[], excluded: SectionKey[] | null] + >([ + ["nothing restricted, nothing required", [], [], null], + ["a required section inside the allowlist", ["labels", "repository"], ["labels"], null], + ["an empty allowlist restricts nothing, so any required section passes", [], ["labels"], null], + ["a required section outside the allowlist", ["repository"], ["labels"], ["labels"]], + [ + "every excluded required section, in the given order, and only those", + ["repository"], + ["labels", "milestones", "repository"], + ["labels", "milestones"], + ], + ])("%s", (_what, only, required, excluded) => { + const selection = SectionSelection.of({ only, required }); + if (excluded === null) { + expect(selection.map((value) => [value.only, value.required])._unsafeUnwrap()).toEqual([ + new Set(only), + new Set(required), + ]); + } else { + expect(selection).toEqual(err({ code: "required-sections-excluded" as const, excluded })); + } + }); + + test("ALL is the empty selection", () => { + expect(SectionSelection.ALL).toEqual(SectionSelection.of({})._unsafeUnwrap()); + }); + + test("a spread or a literal is not a selection: the fields are private, so only `of` mints one", () => { + // Public fields would make the spread typecheck; its directive then reports unused. + // @ts-expect-error a spread keeps no private members, so it is not a SectionSelection + const spread: SectionSelection = { + ...SectionSelection.ALL, + only: new Set(["repository"]), + required: new Set(["labels"]), + }; + // @ts-expect-error an object literal cannot supply private members + const literal: SectionSelection = { + only: new Set(), + required: new Set(), + }; + expect([spread.only, literal.only]).toEqual([new Set(["repository"]), new Set()]); + }); +}); diff --git a/test/engine/validate.test.ts b/test/engine/validate.test.ts index 4905791a..c7c89ed1 100644 --- a/test/engine/validate.test.ts +++ b/test/engine/validate.test.ts @@ -1,28 +1,33 @@ import { describe, expect, test } from "bun:test"; +import { err, ok } from "neverthrow"; import { validateSectionShapes } from "../../src/engine/validate.js"; -/** The verdict's error prose, or null when the document validated. */ -function errorOf(doc: Record, sourceLabel = "f.yml"): string | null { - const verdict = validateSectionShapes(doc, sourceLabel); - return "error" in verdict ? verdict.error : null; +/** The verdict's issue list, or null when the document validated. */ +function issuesOf(doc: Record, sourceLabel = "f.yml"): readonly string[] | null { + return validateSectionShapes(doc, sourceLabel).match( + () => null, + (problem) => problem.issues, + ); } -/** - * Every shape verdict is one line: the source label, the problems joined by - * "; ", and the same passthrough clause. The tables below carry the problem - * list and the assertion pins the whole line. - */ -const PASSTHROUGH = - "Fix these values in the settings file (only the named keys are validated; extra fields pass " + - "through, except in closed sections and strict nested objects like actions.cache, which reject " + - "unrecognized keys)"; - describe("section shape validation", () => { test("pages: null passes", () => { - expect(validateSectionShapes({ pages: null }, "f.yml")).toEqual({ settings: { pages: null } }); + expect(validateSectionShapes({ pages: null }, "f.yml")).toEqual(ok({ pages: null })); }); - test.each<[what: string, doc: Record, problem: string]>([ + test("a shape failure is one problem naming the source, with every issue listed", () => { + expect( + validateSectionShapes({ workflows: [{ path: "ci.yml", state: "paused" }] }, "settings.yml"), + ).toEqual( + err({ + code: "settings-malformed-sections", + source: "settings.yml", + issues: ['workflows[0].state: Invalid option: expected one of "active"|"disabled"'], + }), + ); + }); + + test.each<[what: string, doc: Record, issue: string]>([ [ "a bad workflows state fails naming the path", { workflows: [{ path: "ci.yml", state: "paused" }] }, @@ -46,8 +51,8 @@ describe("section shape validation", () => { { pages: { source: null } }, "pages.source: Invalid input: expected object, received null", ], - ])("the fields handlers dereference are shape-checked: %s", (_what, doc, problem) => { - expect(errorOf(doc)).toBe(`f.yml has malformed section entries: ${problem}. ${PASSTHROUGH}`); + ])("the fields handlers dereference are shape-checked: %s", (_what, doc, issue) => { + expect(issuesOf(doc)).toEqual([issue]); }); test("the happy shapes pass, and the parsed document carries the unknown keys through untouched", () => { @@ -56,7 +61,7 @@ describe("section shape validation", () => { labels: [{ name: "v2", new_name: "2.0" }], pages: { source: { branch: "main" }, extra_field: true }, }; - expect(validateSectionShapes(happy, "f.yml")).toEqual({ settings: happy }); + expect(validateSectionShapes(happy, "f.yml")).toEqual(ok(happy)); }); test("only the declared known sections make up the parsed document", () => { @@ -64,7 +69,7 @@ describe("section shape validation", () => { { _notes: "private", pages: { source: { branch: "main" } } }, "f.yml", ); - expect(verdict).toEqual({ settings: { pages: { source: { branch: "main" } } } }); + expect(verdict).toEqual(ok({ pages: { source: { branch: "main" } } })); }); }); @@ -77,29 +82,26 @@ describe("YAML-tagged values are rejected anywhere in a section", () => { ])( "a tagged section VALUE is rejected for the mapping section %s, which has no required key", (site, doc) => { - expect(errorOf(doc, "settings.yml")).toBe( - `settings.yml has malformed section entries: ${site} is not plain YAML data (a Date, ` + - `e.g. from a YAML !!timestamp tag); replace it with a plain value. ${PASSTHROUGH}`, - ); + expect(issuesOf(doc, "settings.yml")).toEqual([ + `${site} is not plain YAML data (a Date, e.g. from a YAML !!timestamp tag); replace it with a plain value`, + ]); }, ); test("a tagged NESTED value is rejected with its key path", () => { - expect(errorOf({ actions: { cache: new Date(0) } })).toBe( - "f.yml has malformed section entries: actions.cache is not plain YAML data (a Date, e.g. " + - `from a YAML !!timestamp tag); replace it with a plain value. ${PASSTHROUGH}`, - ); - expect(errorOf({ labels: [{ name: "bug", color: new Set(["d73a4a"]) }] })).toBe( - "f.yml has malformed section entries: labels[0].color is not plain YAML data (a set, e.g. " + - `from a YAML !!set tag); replace it with a plain value. ${PASSTHROUGH}`, - ); + expect(issuesOf({ actions: { cache: new Date(0) } })).toEqual([ + "actions.cache is not plain YAML data (a Date, e.g. from a YAML !!timestamp tag); replace it with a plain value", + ]); + expect(issuesOf({ labels: [{ name: "bug", color: new Set(["d73a4a"]) }] })).toEqual([ + "labels[0].color is not plain YAML data (a set, e.g. from a YAML !!set tag); replace it with a plain value", + ]); }); test("a cyclic document (YAML anchors) does not hang the validator", () => { const cyclic: Record = { description: "x" }; cyclic.self = cyclic; // Not endorsed, but the walk must terminate; the shape parse still rules. - expect(() => errorOf({ repository: cyclic } as Record)).not.toThrow(); + expect(() => issuesOf({ repository: cyclic } as Record)).not.toThrow(); }); }); @@ -109,24 +111,22 @@ describe("closed-surface sections reject unrecognized entry keys upfront", () => "intended one"; test("a misspelled collaborator permission fails validation, before any write", () => { - expect(errorOf({ collaborators: [{ username: "alice", permision: "admin" }] })).toBe( - 'f.yml has malformed section entries: collaborators[alice]: declares "permision", which ' + - `this section does not recognize (known keys: username, permission) - ${ROLE_CONSEQUENCE}. ` + - `Fix the key name, or remove it. ${PASSTHROUGH}`, - ); + expect(issuesOf({ collaborators: [{ username: "alice", permision: "admin" }] })).toEqual([ + 'collaborators[alice]: declares "permision", which this section does not recognize ' + + `(known keys: username, permission) - ${ROLE_CONSEQUENCE}. Fix the key name, or remove it`, + ]); }); test("teams and workflows are closed too", () => { - expect(errorOf({ teams: [{ name: "t", permissions: "admin" }] })).toBe( - 'f.yml has malformed section entries: teams[t]: declares "permissions", which this ' + - `section does not recognize (known keys: name, permission) - ${ROLE_CONSEQUENCE}. ` + - `Fix the key name, or remove it. ${PASSTHROUGH}`, - ); - expect(errorOf({ workflows: [{ path: "ci.yml", state: "active", enabled: true }] })).toBe( - 'f.yml has malformed section entries: workflows[ci.yml]: declares "enabled", which this ' + - "section does not recognize (known keys: path, state) - the enable/disable calls send no " + - `payload, so the key would silently do nothing. Fix the key name, or remove it. ${PASSTHROUGH}`, - ); + expect(issuesOf({ teams: [{ name: "t", permissions: "admin" }] })).toEqual([ + 'teams[t]: declares "permissions", which this section does not recognize ' + + `(known keys: name, permission) - ${ROLE_CONSEQUENCE}. Fix the key name, or remove it`, + ]); + expect(issuesOf({ workflows: [{ path: "ci.yml", state: "active", enabled: true }] })).toEqual([ + 'workflows[ci.yml]: declares "enabled", which this section does not recognize (known keys: ' + + "path, state) - the enable/disable calls send no payload, so the key would silently do " + + "nothing. Fix the key name, or remove it", + ]); }); test("open sections still pass extra keys through", () => { @@ -135,19 +135,18 @@ describe("closed-surface sections reject unrecognized entry keys upfront", () => milestones: [{ title: "v1", due_on: "2027-01-01T00:00:00Z" }], labels: [{ name: "bug", extra_field: true }], }; - expect(validateSectionShapes(doc, "f.yml")).toEqual({ settings: doc }); + expect(validateSectionShapes(doc, "f.yml")).toEqual(ok(doc)); }); test("closed-surface entry checks see through the wrapper (collaborators)", () => { expect( - errorOf({ + issuesOf({ collaborators: { _undeclared: "keep", entries: [{ username: "alice", permision: "x" }] }, }), - ).toBe( - 'f.yml has malformed section entries: collaborators[alice]: declares "permision", which ' + - `this section does not recognize (known keys: username, permission) - ${ROLE_CONSEQUENCE}. ` + - `Fix the key name, or remove it. ${PASSTHROUGH}`, - ); + ).toEqual([ + 'collaborators[alice]: declares "permision", which this section does not recognize ' + + `(known keys: username, permission) - ${ROLE_CONSEQUENCE}. Fix the key name, or remove it`, + ]); }); }); @@ -160,32 +159,30 @@ describe("the wrapped undeclared-policy form", () => { rulesets: { _undeclared: "delete", entries: [{ name: "r" }] }, milestones: { _undeclared: "delete", entries: [{ title: "v1" }] }, }; - expect(validateSectionShapes(doc, "f.yml")).toEqual({ settings: doc }); + expect(validateSectionShapes(doc, "f.yml")).toEqual(ok(doc)); }); test("wrapper typos fail upfront: an unknown wrapper key and a bad policy value", () => { // The wrapper is this action's own strict vocabulary; unlike entry // passthrough fields, its extra keys have nowhere to go. A misspelled // "entries" reads as both a missing list and an unrecognized key. - expect(errorOf({ labels: { entires: [{ name: "bug" }] } })).toBe( - "f.yml has malformed section entries: labels.entries: Invalid input: expected array, " + - `received undefined; labels: Unrecognized key: "entires". ${PASSTHROUGH}`, - ); - expect(errorOf({ milestones: { _undeclared: "detele", entries: [] } })).toBe( - "f.yml has malformed section entries: milestones._undeclared: Invalid option: expected " + - `one of "keep"|"delete". ${PASSTHROUGH}`, - ); + expect(issuesOf({ labels: { entires: [{ name: "bug" }] } })).toEqual([ + "labels.entries: Invalid input: expected array, received undefined", + 'labels: Unrecognized key: "entires"', + ]); + expect(issuesOf({ milestones: { _undeclared: "detele", entries: [] } })).toEqual([ + 'milestones._undeclared: Invalid option: expected one of "keep"|"delete"', + ]); }); test("entry paths keep their precision inside the wrapper", () => { expect( - errorOf({ + issuesOf({ rulesets: { entries: [{ name: "r", conditions: { ref_name: { include: "main" } } }] }, }), - ).toBe( - "f.yml has malformed section entries: rulesets.entries[0].conditions.ref_name.include: " + - `Invalid input: expected array, received string. ${PASSTHROUGH}`, - ); + ).toEqual([ + "rulesets.entries[0].conditions.ref_name.include: Invalid input: expected array, received string", + ]); }); test.each([ @@ -204,12 +201,10 @@ describe("the wrapped undeclared-policy form", () => { site: "environments[0].variables", }, ])("the pre-v3 policy key fails naming the rename on $name", ({ doc, site }) => { - expect(errorOf(doc)).toBe( - `f.yml has malformed section entries: ${site}: Unrecognized key: "undeclared"; the wrapper's ` + - `policy key "undeclared" was renamed to "_undeclared" in v3 (a directive, like _layering) - ` + - `write _undeclared: keep or _undeclared: delete. Fix these values in the settings file (only ` + - `the named keys are validated; extra fields pass through, except in closed sections and strict ` + - `nested objects like actions.cache, which reject unrecognized keys)`, - ); + expect(issuesOf(doc)).toEqual([ + `${site}: Unrecognized key: "undeclared"; the wrapper's policy key "undeclared" was renamed ` + + 'to "_undeclared" in v3 (a directive, like _layering) - write _undeclared: keep or ' + + "_undeclared: delete", + ]); }); }); diff --git a/test/flows/deliver.test.ts b/test/flows/deliver.test.ts index cec1bf22..3b5e4977 100644 --- a/test/flows/deliver.test.ts +++ b/test/flows/deliver.test.ts @@ -65,11 +65,7 @@ class TracingApi extends MockApi { } function repo(slug: string): RepoRef { - const parsed = parseRepoSlug(slug); - if (parsed === null) { - throw new Error(`test slug ${slug} must parse`); - } - return parsed; + return parseRepoSlug(slug)._unsafeUnwrap(); } const FAILED_LABELS: SectionOutcome[] = [ @@ -404,12 +400,12 @@ describe("concludeRun", () => { expect(events.join("\n")).not.toContain("o/priv"); }); - test("failRun: the error line, then the conclusion a failed target gets, with no summary", () => { + test("failRun: the problem's line, then the conclusion a failed target gets, with no summary", () => { const { io, events, outputs } = eventIo(); - expect(failRun(io, "the token input is required")).toBe(1); + expect(failRun(io, { code: "input-token-missing" })).toBe(1); expect(outputs).toEqual({ "skipped-sections": "", result: "failed" }); expect(events).toEqual([ - "error: the token input is required", + 'error: cannot call the GitHub API: no token was provided. Set the "token" input on the action step (or export GITHUB_TOKEN)', "output skipped-sections=", "output result=failed", "log: result: failed", diff --git a/test/flows/inputs.test.ts b/test/flows/inputs.test.ts new file mode 100644 index 00000000..e2e10050 --- /dev/null +++ b/test/flows/inputs.test.ts @@ -0,0 +1,331 @@ +import { describe, expect, test } from "bun:test"; +import { ok } from "neverthrow"; +import { + type ConfigEnv, + INPUT_DECLS, + type InputName, + type Problem, + parseConfig, + type RunConfig, + SECTION_KEYS, + SectionSelection, +} from "../../src/index.js"; + +/** Parse a step's inputs (unset ones read as empty, as the runner reports them) under `env`. */ +function parse(inputs: Partial>, env: ConfigEnv = {}) { + return parseConfig((name) => inputs[name] ?? "", env); +} + +/** A single-repo apply run's inputs, with `inputs` on top. */ +function single(inputs: Partial>, env: ConfigEnv = {}) { + return parse({ token: "t", repository: "o/r", ...inputs }, env); +} + +/** A mode: merge run's inputs: no token anywhere, the two layers, the output path, plus `inputs`. */ +function merge(inputs: Partial>) { + return parse({ + mode: "merge", + "settings-file": "fleet.yml\nrepo.yml", + "merged-file": "out/merged.yml", + ...inputs, + }); +} + +function rejection(parsed: ReturnType): Problem { + return parsed.match( + (config) => { + throw new Error(`expected a rejection, got: ${JSON.stringify(config)}`); + }, + (problem) => problem, + ); +} + +/** The parsed config of an engine mode; a merge config or a rejection fails the test. */ +function engineConfig( + parsed: ReturnType, +): Extract { + const config = parsed._unsafeUnwrap(); + if (config.kind === "merge") { + throw new Error(`expected an engine config, got: ${JSON.stringify(config)}`); + } + return config; +} + +describe("the declared defaults", () => { + test("every unset input resolves to its declared default; the GITHUB_* context fills the rest", () => { + expect( + parse( + {}, + { + GITHUB_TOKEN: "t", + GITHUB_REPOSITORY: "o/r", + GITHUB_SERVER_URL: "https://ghe.test", + GITHUB_RUN_ID: "7", + }, + ), + ).toEqual( + ok({ + kind: "single", + token: "t", + mode: INPUT_DECLS.mode.default, + onMissingPermission: INPUT_DECLS["on-missing-permission"].default, + sections: SectionSelection.ALL, + apiVersion: INPUT_DECLS["api-version"].default, + privateRepos: INPUT_DECLS["private-repos"].default, + privateReport: INPUT_DECLS["private-report"].default, + reportPublicKey: "", + selfSlug: "o/r", + runUrl: "https://ghe.test/o/r/actions/runs/7", + repo: { owner: "o", name: "r", slug: "o/r" }, + settingsFile: INPUT_DECLS["settings-file"].default, + }), + ); + }); + + test("the token input wins over GITHUB_TOKEN, and no token anywhere is the problem", () => { + expect(engineConfig(single({}, { GITHUB_TOKEN: "env" })).token).toBe("t"); + expect(rejection(parse({ repository: "o/r" }))).toEqual({ code: "input-token-missing" }); + }); + + test("every input is trimmed, so a whitespace-only token is unset and the env token applies", () => { + const config = engineConfig( + parse( + { token: " ", repository: " o/r ", "settings-file": " conf/only.yml " }, + { + GITHUB_TOKEN: "env", + }, + ), + ); + expect([ + config.token, + config.kind === "single" ? config.repo.slug : "", + config.kind === "single" ? config.settingsFile : "", + ]).toEqual(["env", "o/r", "conf/only.yml"]); + }); +}); + +describe("required-sections x sections cross-validation", () => { + test("rejects a required section excluded by the sections allowlist", () => { + expect(rejection(single({ "required-sections": "labels", sections: "repository" }))).toEqual({ + code: "required-sections-excluded", + excluded: ["labels"], + }); + }); + + test("names every excluded required section at once, and only those", () => { + expect( + rejection( + single({ "required-sections": "labels,milestones,repository", sections: "repository" }), + ), + ).toEqual({ code: "required-sections-excluded", excluded: ["labels", "milestones"] }); + }); + + test("accepts required sections inside the allowlist, carried as the validated selection", () => { + // Accepted AND carried into the config: a parse that silently dropped + // either set would otherwise pass. + expect( + engineConfig(single({ "required-sections": "labels", sections: "labels,repository" })) + .sections, + ).toEqual( + SectionSelection.of({ only: ["labels", "repository"], required: ["labels"] })._unsafeUnwrap(), + ); + }); + + test("an empty sections input restricts nothing, so any required section passes", () => { + expect(engineConfig(single({ "required-sections": "labels" })).sections).toEqual( + SectionSelection.of({ required: ["labels"] })._unsafeUnwrap(), + ); + }); + + test("unknown-name validation still wins over the cross-check, per input", () => { + expect(rejection(single({ "required-sections": "nope", sections: "repository,typo" }))).toEqual( + { + code: "input-unknown-sections", + unknown: [ + { input: "required-sections", names: ["nope"] }, + { input: "sections", names: ["typo"] }, + ], + known: SECTION_KEYS, + }, + ); + }); +}); + +describe("the mode input", () => { + test("an unsupported mode is rejected carrying every supported one and the default", () => { + expect(rejection(single({ mode: "dry-run" }))).toEqual({ + code: "input-unsupported-value", + input: "mode", + value: "dry-run", + noun: "mode", + allowed: ["apply", "check", "merge"], + fallback: "apply", + }); + }); + + test.each([ + ["layering", { layering: "replace" }, ["layering"]], + ["merged-file", { "merged-file": "out.yml" }, ["merged-file"]], + [ + "both merge-only inputs", + { layering: "merge", "merged-file": "out.yml" }, + ["merged-file", "layering"], + ], + ] as const)("%s outside merge mode is rejected", (_case, inputs, named) => { + expect(rejection(single({ mode: "check", ...inputs }))).toEqual({ + code: "input-merge-only", + inputs: named, + mode: "check", + }); + }); + + const SEPARATOR_CASES = [ + ["a comma list", "a.yml,b.yml"], + ["a newline list", "a.yml\nb.yml"], + ["an empty list", ","], + ["a trailing separator", "only.yml,"], + ["a leading separator", ",only.yml"], + ] as const; + + test.each( + (["apply", "check"] as const).flatMap((mode) => + SEPARATOR_CASES.map(([label, value]) => [mode, label, value] as const), + ), + )("%s mode rejects a settings-file with %s rather than repairing it", (mode, _case, value) => { + expect(rejection(single({ mode, "settings-file": value }))).toEqual({ + code: "input-settings-file-is-list", + value, + mode, + }); + }); + + test("a plain settings-file path is carried verbatim into the single-repo config", () => { + expect(single({ "settings-file": "conf/only.yml" })).toEqual( + ok({ + token: "t", + mode: "apply", + onMissingPermission: "fail", + sections: SectionSelection.ALL, + apiVersion: "2022-11-28", + privateRepos: "redact", + privateReport: "none", + reportPublicKey: "", + selfSlug: "", + runUrl: "", + kind: "single", + repo: { owner: "o", name: "r", slug: "o/r" }, + settingsFile: "conf/only.yml", + }), + ); + }); + + test("a repository that is not an owner/name slug is rejected with the value", () => { + expect(rejection(single({ repository: "not-a-slug" }))).toEqual({ + code: "input-repository-not-slug", + value: "not-a-slug", + }); + }); +}); + +describe("mode: merge", () => { + /** What the two-layer merge inputs parse to; the tests below vary one input around it. */ + const MERGE_CONFIG: Extract = { + kind: "merge", + settingsFiles: ["fleet.yml", "repo.yml"], + mergedFile: "out/merged.yml", + layering: "merge", + }; + + test("parses without any token, carrying the ordered layers, the output path, and the layering", () => { + expect(merge({ layering: "replace" })).toEqual(ok({ ...MERGE_CONFIG, layering: "replace" })); + }); + + test("the layering input defaults to merge and a comma list of layers works too", () => { + expect(merge({ "settings-file": " fleet.yml , team.yml ,repo.yml" })).toEqual( + ok({ ...MERGE_CONFIG, settingsFiles: ["fleet.yml", "team.yml", "repo.yml"] }), + ); + }); + + test("an empty layer list is rejected", () => { + expect(rejection(merge({ "settings-file": "," }))).toEqual({ + code: "input-settings-file-empty", + value: ",", + }); + }); + + test("a missing merged-file is rejected", () => { + expect(rejection(merge({ "merged-file": "" }))).toEqual({ code: "input-merged-file-missing" }); + }); + + test("a merged-file beside the layers is accepted; the collision with a layer is the fold's to refuse", () => { + expect(merge({ "merged-file": "./merged.yml" })).toEqual( + ok({ ...MERGE_CONFIG, mergedFile: "./merged.yml" }), + ); + }); + + test("an unsupported layering is rejected", () => { + expect(rejection(merge({ layering: "union" }))).toEqual({ + code: "input-unsupported-value", + input: "layering", + value: "union", + noun: "layering", + allowed: ["merge", "replace"], + fallback: "merge", + }); + }); + + test.each([ + ["repository", { repository: "o/r" }, ["repository"]], + ["repos", { repos: "o/a" }, ["repos"]], + ["defaults-file", { "defaults-file": "defaults.yml" }, ["defaults-file"]], + ["required-sections", { "required-sections": "labels" }, ["required-sections"]], + ["a discovery filter", { forks: "exclude" }, ["forks"]], + ["private-report", { "private-report": "issue" }, ["private-report"]], + ["report-public-key", { "report-public-key": "age1x" }, ["report-public-key"]], + ["sections: the allowlist belongs on the apply step", { sections: "labels" }, ["sections"]], + ["on-missing-permission: warn", { "on-missing-permission": "warn" }, ["on-missing-permission"]], + ["a custom api-version", { "api-version": "2099-01-01" }, ["api-version"]], + ["private-repos: show", { "private-repos": "show" }, ["private-repos"]], + [ + "several at once, every one named in declaration order", + { repos: "o/a", "repos-dir": "repos", "private-report": "artifact" }, + ["repos", "repos-dir", "private-report"], + ], + [ + "a previously tolerated control beside an always-rejected one, both named", + { repos: "o/a", sections: "labels", "api-version": "2099-01-01" }, + ["sections", "api-version", "repos"], + ], + ] as const)( + "%s is rejected: a merge has no repository, API, report, or allowlist", + (_case, inputs, named) => { + expect(rejection(merge(inputs))).toEqual({ code: "input-rejected-in-merge", inputs: named }); + }, + ); + + test("every declared default passes: the runner supplies them whether or not the workflow set the input", () => { + // Every input outside the merge inputs themselves, at the exact value + // action.yml declares (token's is the unresolved workflow expression; the + // runner resolves it, which is why token is tolerated at any value). + const defaults = Object.fromEntries( + Object.entries(INPUT_DECLS) + .filter(([name]) => !["mode", "settings-file", "merged-file"].includes(name)) + .map(([name, decl]) => [name, decl.default]), + ); + expect(merge(defaults)).toEqual(ok(MERGE_CONFIG)); + }); + + test("token is tolerated and never carried: the config has no field to read it from", () => { + const parsed = parse( + { + mode: "merge", + "settings-file": "fleet.yml\nrepo.yml", + "merged-file": "out/merged.yml", + token: "ghp_stepwide", + }, + { GITHUB_TOKEN: "ghp_envwide" }, + ); + expect(parsed).toEqual(ok(MERGE_CONFIG)); + expect(JSON.stringify(parsed)).not.toContain("ghp_"); + }); +}); diff --git a/test/flows/layers.test.ts b/test/flows/layers.test.ts new file mode 100644 index 00000000..6e2f3973 --- /dev/null +++ b/test/flows/layers.test.ts @@ -0,0 +1,50 @@ +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { err, ok } from "neverthrow"; +import { readLayerFiles } from "../../src/flows/layers.js"; +import * as settingsRead from "../../src/flows/settings-read.js"; + +describe("readLayerFiles", () => { + let dir = ""; + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "read-layers-")); + writeFileSync(join(dir, "fleet.yml"), "repository:\n has_wiki: false\n"); + writeFileSync(join(dir, "repo.yml"), "repository:\n has_issues: true\n"); + writeFileSync(join(dir, "broken.yml"), "labels: [oops, unclosed\n"); + }); + afterEach(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + test("reads every layer in path order, each named by its path", () => { + const fleet = join(dir, "fleet.yml"); + const repo = join(dir, "repo.yml"); + expect(readLayerFiles([repo, fleet])).toEqual( + ok([ + { name: repo, doc: { repository: { has_issues: true } } }, + { name: fleet, doc: { repository: { has_wiki: false } } }, + ]), + ); + }); + + test("the first unreadable layer is the problem, and the layers after it are never read", () => { + const read = spyOn(settingsRead, "readSettingsFile"); + try { + const missing = join(dir, "missing.yml"); + expect(readLayerFiles([missing, join(dir, "broken.yml")])).toEqual( + err({ + code: "settings-file-unreadable" as const, + role: "layer" as const, + path: missing, + reason: expect.stringContaining("ENOENT"), + }), + ); + // The broken layer's own syntax error never becomes a candidate: one read, the failed one. + expect(read.mock.calls).toEqual([[missing, "layer"]]); + } finally { + read.mockRestore(); + } + }); +}); diff --git a/test/flows/library.test.ts b/test/flows/library.test.ts index fba3eab4..a94f4eef 100644 --- a/test/flows/library.test.ts +++ b/test/flows/library.test.ts @@ -1,35 +1,37 @@ import { describe, expect, test } from "bun:test"; +import { err, ok } from "neverthrow"; import { stringify as stringifyYaml } from "yaml"; import { applyRepository, checkRepository, collectingIo, + describeProblem, parseRepoSlug, renderMergedYaml, + SectionSelection, type ValidatedSettings, validateSettings, } from "../../src/index.js"; import { MockApi } from "../mock-api.js"; -const repo = parseRepoSlug("o/r") as NonNullable>; +const repo = parseRepoSlug("o/r")._unsafeUnwrap(); /** The brand is minted only by validation, so the tests' documents pass through it. */ function branded(doc: unknown): ValidatedSettings { - const validated = validateSettings(doc); - if (!validated.ok) { - throw new Error(validated.error); - } - return validated.settings; + return validateSettings(doc).match( + (validated) => validated.settings, + (problem) => { + throw new Error(describeProblem(problem)); + }, + ); } const settings = branded({ repository: { has_wiki: false } }); describe("validateSettings", () => { test("a valid document comes back branded with no warnings", () => { - expect(validateSettings({ repository: { has_wiki: false } })).toEqual({ - ok: true, - settings, - warnings: [], - }); + expect(validateSettings({ repository: { has_wiki: false } })).toEqual( + ok({ settings, warnings: [] }), + ); }); test("an unknown key outside the sections allowlist is a returned warning, not a printed one", () => { @@ -38,21 +40,20 @@ describe("validateSettings", () => { { repository: { has_wiki: false }, typo: 1 }, { source: "fleet.yml", sections: new Set(["repository"]) }, ), - ).toEqual({ - ok: true, - settings, - warnings: [ - 'ignoring unknown top-level section(s) outside the "sections" allowlist: typo. Upgrade the action to a version that knows them, or remove them from fleet.yml', - ], - }); + ).toEqual( + ok({ + settings, + warnings: [ + 'ignoring unknown top-level section(s) outside the "sections" allowlist: typo. Upgrade the action to a version that knows them, or remove them from fleet.yml', + ], + }), + ); }); - test("an invalid document names the default source in its error", () => { - expect(validateSettings([1])).toEqual({ - ok: false, - error: - 'the settings document must be a YAML mapping of section names to settings, but its top level parsed as a list. Rewrite the top level as "section: ..." keys', - }); + test("an invalid document comes back as its problem, naming the default source", () => { + expect(validateSettings([1])).toEqual( + err({ code: "settings-not-mapping", source: "the settings document", shape: "list" }), + ); }); }); @@ -63,8 +64,7 @@ describe("checkRepository and applyRepository", () => { repo, settings, onMissingPermission: "fail", - requiredSections: new Set(), - onlySections: new Set(), + sections: SectionSelection.ALL, }); expect(api.mutations()).toEqual([]); expect(result).toEqual({ @@ -89,8 +89,7 @@ describe("checkRepository and applyRepository", () => { repo, settings, onMissingPermission: "fail", - requiredSections: new Set(), - onlySections: new Set(), + sections: SectionSelection.ALL, }, collected.io, ); @@ -110,6 +109,24 @@ describe("checkRepository and applyRepository", () => { }); }); +describe("the section selection a library call runs under", () => { + test("a required section outside the allowlist is the problem, and no check runs", async () => { + // The engine reports an excluded section without attempting it, so this + // pair would pass green having proven nothing; the selection refuses it + // before checkRepository can be given one. + const api = new MockApi({ "GET /repos/o/r": { data: { has_wiki: true } } }); + const outcome = await SectionSelection.of({ + only: ["repository"], + required: ["labels"], + }).match( + (sections) => checkRepository(api, { repo, settings, onMissingPermission: "fail", sections }), + (problem) => problem, + ); + expect(outcome).toEqual({ code: "required-sections-excluded", excluded: ["labels"] }); + expect(api.calls).toEqual([]); + }); +}); + describe("renderMergedYaml", () => { test("is the document's YAML serialization, byte for byte", () => { expect(renderMergedYaml(settings)).toBe(stringifyYaml(settings)); diff --git a/test/flows/merge.test.ts b/test/flows/merge.test.ts index 18cfc346..09e91260 100644 --- a/test/flows/merge.test.ts +++ b/test/flows/merge.test.ts @@ -2,7 +2,8 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { collectingIo, type MergeConfig, runMerge } from "../../src/index.js"; +import { err, ok } from "neverthrow"; +import { collectingIo, concludeMerge, type MergeConfig, runMerge } from "../../src/index.js"; const FLEET = "repository:\n has_wiki: false\n"; const REPO = "repository:\n has_issues: true\n"; @@ -24,35 +25,35 @@ describe("runMerge", () => { layering: "merge", }); - test("folds the layers into merged-file and concludes merged", () => { + test("folds the layers into merged-file, and the finished merge concludes merged", () => { const collected = collectingIo(); - expect(runMerge(cfg(join(dir, "out", "merged.yml")), collected.io)).toBe(0); - expect(readFileSync(join(dir, "out", "merged.yml"), "utf8")).toBe( + const mergedFile = join(dir, "out", "merged.yml"); + const merged = runMerge(cfg(mergedFile), collected.io); + expect(merged).toEqual(ok({ layers: cfg(mergedFile).settingsFiles, mergedFile })); + expect(readFileSync(mergedFile, "utf8")).toBe( "repository:\n has_wiki: false\n has_issues: true\n", ); + expect(concludeMerge(collected.io, merged._unsafeUnwrap())).toBe(0); expect(collected.outputs).toEqual({ "skipped-sections": "", result: "merged" }); }); test.each<[string, (dir: string) => string, number, string]>([ - ["a layer's own path", (d) => join(d, "repo.yml"), 2, "repo.yml"], - ["a ./ spelling of a layer, compared resolved", (d) => `${d}/./fleet.yml`, 1, "fleet.yml"], + ["a layer's own path", (d) => join(d, "repo.yml"), 1, "repo.yml"], + ["a ./ spelling of a layer, compared resolved", (d) => `${d}/./fleet.yml`, 0, "fleet.yml"], ])( "a merged-file naming %s fails before any write, naming the layer's position", - (_case, mergedFile, position, layer) => { + (_case, mergedFile, index, layer) => { const collected = collectingIo(); const target = mergedFile(dir); - expect(runMerge(cfg(target), collected.io)).toBe(1); - expect(collected.lines).toEqual([ - { - level: "error", - line: - `the "merged-file" input "${target}" is layer ${position} of the ` + - `"settings-file" list ("${join(dir, layer)}"): the merge would overwrite that ` + - `layer with the folded document, and the next run would fold the merged document as a ` + - `layer. Write the merged document to a path outside the layer list`, - }, - { line: "result: failed" }, - ]); + expect(runMerge(cfg(target), collected.io)).toEqual( + err({ + code: "merged-file-is-layer" as const, + mergedFile: target, + index, + layer: join(dir, layer), + }), + ); + expect(collected.lines).toEqual([]); expect(readFileSync(join(dir, "fleet.yml"), "utf8")).toBe(FLEET); expect(readFileSync(join(dir, "repo.yml"), "utf8")).toBe(REPO); }, diff --git a/test/flows/multi.test.ts b/test/flows/multi.test.ts index c6e7f7ab..2080ef01 100644 --- a/test/flows/multi.test.ts +++ b/test/flows/multi.test.ts @@ -4,16 +4,17 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { Decrypter, generateX25519Identity, identityToRecipient } from "age-encryption"; import { DEFAULT_DISCOVERY_FILTERS } from "../../src/discovery/discover.js"; +import { SectionSelection } from "../../src/engine/section-selection.js"; import { runMulti } from "../../src/flows/multi.js"; import type { TargetOutcome } from "../../src/flows/redact.js"; import { type Io, maskRegistry } from "../../src/io.js"; import { isPrivate } from "../../src/private.js"; +import { describeProblem, type Problem } from "../../src/problem.js"; import { ARTIFACT_FILE, ARTIFACT_NAME, type ArtifactUploader, } from "../../src/report/artifact-report.js"; -import type { SectionKey } from "../../src/schema.js"; import { MockApi } from "../mock-api.js"; function captureIo(): { @@ -56,6 +57,26 @@ function captureIo(): { const redacted = (target: TargetOutcome | undefined): boolean => target !== undefined && isPrivate(target.detail); +/** The run's targets; a fatal problem fails the test with its rendered line. */ +async function runTargets(...args: Parameters): Promise { + return (await runMulti(...args)).match( + (targets) => targets, + (problem) => { + throw new Error(describeProblem(problem)); + }, + ); +} + +/** The fatal problem a run ends in before any target executes. */ +async function runFatal(...args: Parameters): Promise { + return (await runMulti(...args)).match( + (targets) => { + throw new Error(`expected a fatal problem, got ${targets.length} target(s)`); + }, + (problem) => problem, + ); +} + function cfg(overrides: Partial[1]> = {}) { return { reposDir: "", @@ -64,8 +85,7 @@ function cfg(overrides: Partial[1]> = {}) { adminOwner: "o", mode: "apply" as const, onMissingPermission: "fail" as const, - requiredSections: new Set(), - onlySections: new Set(), + sections: SectionSelection.ALL, discoveryFilters: DEFAULT_DISCOVERY_FILTERS, discoveryFiltersSet: [], // Existing scenarios predate redaction and assert on raw slugs; default @@ -111,8 +131,7 @@ describe("runMulti", () => { "GET /repos/o/a/contents/.github/settings.yml": { data: HOOK_WITH_REF }, }); const { io, annotations } = captureIo(); - const { fatal, targets } = await runMulti(api, cfg({ reposInput: "o/a", mode: "check" }), io); - expect(fatal).toBeNull(); + const targets = await runTargets(api, cfg({ reposInput: "o/a", mode: "check" }), io); expect(targets.map((t) => [t.display, t.result])).toEqual([["o/a", "failed"]]); expect(api.calls.filter((c) => c.path.includes("/hooks"))).toEqual([]); expect(annotations.some((a) => a.includes("target-fetched settings file"))).toBe(true); @@ -126,12 +145,11 @@ describe("runMulti", () => { "GET /repos/o/c/hooks?per_page=100&page=1": { data: [] }, }); const { io, annotations } = captureIo(); - const { fatal, targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/c", defaultsFile: join(dir, "defaults.yml"), mode: "check" }), io, ); - expect(fatal).toBeNull(); expect(targets.map((t) => [t.display, t.result])).toEqual([["o/c", "drift"]]); expect(annotations.filter((a) => a.includes("target-fetched"))).toEqual([]); }); @@ -144,12 +162,11 @@ describe("runMulti", () => { "GET /repos/o/api/hooks?per_page=100&page=1": { data: [] }, }); const { io, annotations } = captureIo(); - const { fatal, targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposDir: join(dir, "repos"), adminOwner: "o", mode: "check" }), io, ); - expect(fatal).toBeNull(); expect(targets.map((t) => [t.display, t.result])).toEqual([["o/api", "drift"]]); expect(annotations.filter((a) => a.includes("target-fetched"))).toEqual([]); }); @@ -175,12 +192,11 @@ describe("runMulti", () => { "GET /repos/o/c/git/ref/heads/main": { data: { ref: "refs/heads/main" } }, }).allowMutations("PATCH /repos/o/a"); const { io, annotations } = captureIo(); - const { fatal, targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/a, o/b, o/c", onMissingPermission: "warn" }), io, ); - expect(fatal).toBeNull(); const bySlug = Object.fromEntries(targets.map((t) => [t.display, t.result])); expect(bySlug).toEqual({ "o/a": "applied", "o/b": "failed", "o/c": "skipped" }); expect(annotations.some((a) => a.includes("o/c: skipped - the repository has no"))).toBe(true); @@ -195,9 +211,12 @@ describe("runMulti", () => { "PATCH /repos/o/a": { error: { status: 500, message: "boom", body: "" } }, }); const { io, annotations } = captureIo(); - const { targets } = await runMulti( + const targets = await runTargets( api, - cfg({ reposInput: "o/a", onlySections: new Set(["repository"]) }), + cfg({ + reposInput: "o/a", + sections: SectionSelection.of({ only: ["repository"] })._unsafeUnwrap(), + }), io, ); expect(targets[0]?.result).toBe("failed"); @@ -218,12 +237,11 @@ describe("runMulti", () => { "GET /repos/o/c/git/ref/heads/main": { data: { ref: "refs/heads/main" } }, }).allowMutations("PATCH /repos/o/c"); const { io, annotations } = captureIo(); - const { fatal, targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/c", defaultsFile: "test/fixtures/defaults.yml" }), io, ); - expect(fatal).toBeNull(); expect(targets.map((t) => [t.display, t.result])).toEqual([["o/c", "applied"]]); expect(api.mutations().map((m) => [m.method, m.path, m.payload])).toEqual([ ["PATCH", "/repos/o/c", { has_projects: false }], @@ -243,12 +261,11 @@ describe("runMulti", () => { }, }).allowMutations("PATCH /repos/o/a"); const { io, annotations } = captureIo(); - const { fatal, targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/a", defaultsFile: "test/fixtures/defaults.yml" }), io, ); - expect(fatal).toBeNull(); expect(targets.map((t) => [t.display, t.result])).toEqual([["o/a", "applied"]]); expect(api.mutations().map((m) => [m.method, m.path, m.payload])).toEqual([ ["PATCH", "/repos/o/a", { has_wiki: false }], @@ -262,8 +279,7 @@ describe("runMulti", () => { "GET /repos/o/c/git/ref/heads/main": { data: { ref: "refs/heads/main" } }, }); const { io, annotations } = captureIo(); - const { fatal, targets } = await runMulti(api, cfg({ reposInput: "o/c" }), io); - expect(fatal).toBeNull(); + const targets = await runTargets(api, cfg({ reposInput: "o/c" }), io); expect(targets.map((t) => [t.display, t.result])).toEqual([["o/c", "skipped"]]); expect(api.mutations()).toEqual([]); expect(annotations).toEqual([ @@ -281,7 +297,7 @@ describe("runMulti", () => { "GET /repos/octo/web": { data: { has_wiki: true, has_projects: true } }, }).allowMutations("PATCH /repos/viv/api", "PATCH /repos/octo/web"); const { io } = captureIo(); - const { fatal, targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposDir: "test/fixtures/repos", @@ -291,7 +307,6 @@ describe("runMulti", () => { }), io, ); - expect(fatal).toBeNull(); expect(targets.map((t) => [t.display, t.result]).sort()).toEqual([ ["octo/web", "applied"], ["viv/api", "applied"], @@ -309,8 +324,10 @@ describe("runMulti", () => { test("no targets at all is a fatal config error", async () => { const api = new MockApi({}); const { io } = captureIo(); - const { fatal } = await runMulti(api, cfg({ reposInput: " , " }), io); - expect(fatal).toContain("no targets"); + expect(await runFatal(api, cfg({ reposInput: " , " }), io)).toEqual({ + code: "no-targets", + filteredOut: 0, + }); }); test("a token-invisible repo fails loudly instead of skipping", async () => { @@ -318,8 +335,7 @@ describe("runMulti", () => { // which is how a fine-grained token reports lost access. const api = new MockApi({}); const { io, annotations } = captureIo(); - const { fatal, targets } = await runMulti(api, cfg({ reposInput: "o/x" }), io); - expect(fatal).toBeNull(); + const targets = await runTargets(api, cfg({ reposInput: "o/x" }), io); expect(targets[0]?.result).toBe("failed"); expect(annotations.some((a) => a.includes("the token was denied"))).toBe(true); }); @@ -333,12 +349,11 @@ describe("runMulti", () => { "GET /repos/o/x": { data: { default_branch: "main", has_projects: true } }, }); const { io, annotations } = captureIo(); - const { fatal, targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/x", defaultsFile: "test/fixtures/defaults.yml" }), io, ); - expect(fatal).toBeNull(); expect(targets.map((t) => [t.display, t.result])).toEqual([["o/x", "failed"]]); expect(api.mutations()).toEqual([]); expect(annotations).toEqual([ @@ -352,13 +367,16 @@ describe("runMulti", () => { test("discovery filters with repos-dir-only targets are fatal", async () => { const api = new MockApi({}); const { io, annotations } = captureIo(); - const { fatal } = await runMulti( + const fatal = await runFatal( api, cfg({ reposDir: "test/fixtures/repos", adminOwner: "viv", discoveryFiltersSet: ["forks"] }), io, ); - expect(fatal).toContain("repos-dir"); - expect(fatal).toContain('"forks"'); + expect(fatal).toEqual({ + code: "discovery-filters-without-wildcard", + filters: ["forks"], + targets: "repos-dir", + }); // Finding D: central-resolution warnings buffered before this fatal return // must still be emitted, not silently swallowed. The fixture's README.md // (non-yaml) and octo/deep/ (too deep) each produce an "ignoring" warning. @@ -375,7 +393,7 @@ describe("runMulti", () => { }, }); const { io } = captureIo(); - const { fatal } = await runMulti( + const fatal = await runFatal( api, cfg({ reposInput: "*", @@ -384,8 +402,7 @@ describe("runMulti", () => { }), io, ); - expect(fatal).toContain("discovery filters removed all of them"); - expect(fatal).toContain("2 repositories"); + expect(fatal).toEqual({ code: "no-targets", filteredOut: 2 }); }); test("skip notices list at most 20 slugs, then a count of the rest", async () => { @@ -403,7 +420,7 @@ describe("runMulti", () => { }, }); const { io, annotations } = captureIo(); - const { fatal } = await runMulti( + await runTargets( api, cfg({ reposInput: "*", @@ -412,7 +429,6 @@ describe("runMulti", () => { }), io, ); - expect(fatal).toBeNull(); const notice = annotations.find((a) => a.includes("forks=exclude")); expect(notice).toContain("skipped 21 repositories"); expect(notice).toContain("o/fork-19"); @@ -439,8 +455,7 @@ describe("runMulti under on-missing-permission: fail", () => { }, }).allowMutations("PATCH /repos/o/a"); const { io, annotations } = captureIo(); - const { fatal, targets } = await runMulti(api, cfg({ reposInput: "o/a, o/d" }), io); - expect(fatal).toBeNull(); + const targets = await runTargets(api, cfg({ reposInput: "o/a, o/d" }), io); const bySlug = Object.fromEntries(targets.map((t) => [t.display, t.result])); expect(bySlug).toEqual({ "o/a": "applied", "o/d": "failed" }); expect(api.mutations().every((m) => m.path.startsWith("/repos/o/a"))).toBe(true); @@ -468,7 +483,7 @@ describe("runMulti redaction (private-repos: redact)", () => { test("a private target is masked before the first emission, and never leaks its slug", async () => { const api = mixApi(); const { io, annotations, logs, masks, events } = captureIo(); - const { targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/pub, o/priv", mode: "check", privateRepos: "redact" }), io, @@ -527,7 +542,7 @@ describe("runMulti redaction (private-repos: redact)", () => { }, }); const { io, annotations, masks } = captureIo(); - const { targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/self", mode: "check", privateRepos: "redact", selfSlug: "o/self" }), io, @@ -559,7 +574,7 @@ describe("runMulti redaction (private-repos: redact)", () => { }, }); const { io, annotations } = captureIo(); - const { targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "*", mode: "check", privateRepos: "redact" }), io, @@ -581,7 +596,7 @@ describe("runMulti redaction (private-repos: redact)", () => { // must be generic, never naming the slug. const api = new MockApi({}); const { io, annotations } = captureIo(); - const { targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/mystery", privateRepos: "redact" }), io, @@ -605,7 +620,7 @@ describe("runMulti redaction (private-repos: redact)", () => { "GET /repos/o/p2/contents/.github/settings.yml": { data: "repository:\n has_wiki: false\n" }, }); const { io } = captureIo(); - const { targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/p1, o/pub, o/p2", mode: "check", privateRepos: "redact" }), io, @@ -653,7 +668,7 @@ describe("runMulti private-report: issue wiring", () => { test("delivers the full report to the target issue; body has detail+transcript, public surfaces do not", async () => { const api = reportApi(); const { io, annotations, logs } = captureIo(); - const { targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/priv", @@ -719,7 +734,7 @@ describe("runMulti private-report: issue wiring", () => { }, }); const { io, annotations } = captureIo(); - const { targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/priv", @@ -784,7 +799,7 @@ describe("runMulti private-report: issue wiring", () => { }, }); const { io, annotations } = captureIo(); - const { targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "*", mode: "check", privateRepos: "redact", privateReport: "issue" }), io, @@ -814,7 +829,7 @@ describe("runMulti private-report: issue wiring", () => { "POST /repos/o/maybe/labels": { error: { status: 422, message: "exists", body: "" } }, }); const { io, annotations } = captureIo(); - const { targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/maybe", mode: "check", privateRepos: "redact", privateReport: "issue" }), io, @@ -855,7 +870,7 @@ describe("runMulti private-report: issue wiring", () => { "GET /repos/o/priv/contents/.github/settings.yml": { data: "repository: [oops\n" }, }); const { io, annotations } = captureIo(); - const { targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/priv", mode: "apply", privateRepos: "redact", privateReport: "issue" }), io, @@ -895,7 +910,7 @@ describe("runMulti private-report: issue-on-failure wiring", () => { "PATCH /repos/o/priv/issues/7": { data: { number: 7 } }, }); const { io, annotations, logs } = captureIo(); - const { targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/priv", @@ -927,7 +942,7 @@ describe("runMulti private-report: issue-on-failure wiring", () => { [openListPath("o/priv")]: { data: [] }, }); const { io, annotations } = captureIo(); - const { targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/priv", @@ -1051,7 +1066,7 @@ describe("runMulti private-report: artifact wiring", () => { }, }); const { io, annotations, logs } = captureIo(); - const { targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/a, o/b", @@ -1099,7 +1114,7 @@ describe("runMulti private-report: artifact wiring", () => { }, }); const { io, annotations } = captureIo(); - const { targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/known, o/maybe", @@ -1164,7 +1179,7 @@ describe("runMulti private-report: artifact wiring", () => { }, }); const { io, annotations } = captureIo(); - const { targets } = await runMulti( + const targets = await runTargets( api, cfg({ reposInput: "o/priv", diff --git a/test/flows/settings-read.test.ts b/test/flows/settings-read.test.ts index d6491f00..221edd84 100644 --- a/test/flows/settings-read.test.ts +++ b/test/flows/settings-read.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { err, ok } from "neverthrow"; import { parseSettingsDoc } from "../../src/flows/settings-read.js"; /** @@ -75,7 +76,7 @@ describe("parseSettingsDoc", () => { "a document with $name parses to the same object and prints nothing", async ({ raw, doc }) => { expect(await captureOutput(() => parseSettingsDoc(raw))).toEqual({ - result: { doc }, + result: ok(doc), warnings: [], stderr: "", }); @@ -86,12 +87,12 @@ describe("parseSettingsDoc", () => { expect( await captureOutput(() => parseSettingsDoc("repository:\n name: x\nlabels:\n - name: a\n")), ).toEqual({ - result: { doc: { repository: { name: "x" }, labels: [{ name: "a" }] } }, + result: ok({ repository: { name: "x" }, labels: [{ name: "a" }] }), warnings: [], stderr: "", }); expect(await captureOutput(() => parseSettingsDoc(""))).toEqual({ - result: { doc: {} }, + result: ok({}), warnings: [], stderr: "", }); @@ -100,9 +101,10 @@ describe("parseSettingsDoc", () => { test("a syntax error still fails through the error path, not a partial parse", async () => { const captured = await captureOutput(() => parseSettingsDoc("labels: [oops, unclosed\n")); expect(captured).toEqual({ - result: { - error: expect.stringMatching(/^YAMLParseError: Flow sequence in block collection/), - }, + result: err({ + code: "yaml-invalid", + reason: expect.stringMatching(/^YAMLParseError: Flow sequence in block collection/), + }), warnings: [], stderr: "", }); diff --git a/test/flows/single.test.ts b/test/flows/single.test.ts index 7b4620b9..479542a2 100644 --- a/test/flows/single.test.ts +++ b/test/flows/single.test.ts @@ -1,23 +1,23 @@ import { describe, expect, test } from "bun:test"; +import { err, ok } from "neverthrow"; import { collectingIo, parseRepoSlug, runMulti, runSingle, + SectionSelection, type SingleConfig, } from "../../src/index.js"; -import { ARTIFACT_NEEDS_UPLOADER } from "../../src/report/delivery.js"; import { MockApi } from "../mock-api.js"; -const repo = parseRepoSlug("o/r") as NonNullable>; +const repo = parseRepoSlug("o/r")._unsafeUnwrap(); const cfg = (overrides: Partial = {}): SingleConfig => ({ repo, settingsFile: "test/fixtures/single.yml", mode: "check", onMissingPermission: "fail", - requiredSections: new Set(), - onlySections: new Set(), + sections: SectionSelection.ALL, privateRepos: "show", privateReport: "none", reportPublicKey: "", @@ -30,8 +30,8 @@ describe("runSingle", () => { test("a clean check returns the one target's outcome and prints nothing", async () => { const api = new MockApi({ "GET /repos/o/r": { data: { has_wiki: false } } }); const collected = collectingIo(); - expect(await runSingle(api, cfg(), collected.io)).toEqual({ - target: { + expect(await runSingle(api, cfg(), collected.io)).toEqual( + ok({ result: "clean", display: "o/r", detail: { @@ -39,14 +39,14 @@ describe("runSingle", () => { outcomes: [{ key: "repository", status: "clean", detail: [] }], note: undefined, }, - }, - }); + }), + ); expect(collected.lines).toEqual([]); }); test("the artifact channel without an uploader is fatal before any API call", async () => { const api = new MockApi({}); - const fatal = { fatal: ARTIFACT_NEEDS_UPLOADER }; + const fatal = err({ code: "artifact-uploader-missing" as const }); expect( await runSingle( api, @@ -75,18 +75,21 @@ describe("runSingle", () => { }, collectingIo().io, ), - ).toEqual({ ...fatal, targets: [] }); + ).toEqual(fatal); expect(api.calls).toEqual([]); }); - test("an unreadable settings file is fatal, naming the path and the input", async () => { + test("an unreadable settings file is fatal, carrying the path under the settings-file role", async () => { const api = new MockApi({}); const result = await runSingle(api, cfg({ settingsFile: "missing.yml" }), collectingIo().io); - expect(result).toEqual({ - fatal: expect.stringMatching( - /^cannot read settings from missing\.yml: .*ENOENT.*\. Check that the file exists at that path \(set the "settings-file" input if it lives elsewhere\) and is valid YAML$/, - ), - }); + expect(result).toEqual( + err({ + code: "settings-file-unreadable", + role: "settings-file", + path: "missing.yml", + reason: expect.stringContaining("ENOENT"), + }), + ); expect(api.calls).toEqual([]); }); }); diff --git a/test/library/public-surface.test.ts b/test/library/public-surface.test.ts index cbab525d..838046b1 100644 --- a/test/library/public-surface.test.ts +++ b/test/library/public-surface.test.ts @@ -32,29 +32,44 @@ describe("the public entry", () => { "AnnotationLevel", "ApiError", "ArtifactUploader", + "CentralFileProblem", "CentralTarget", "CollectedLine", + "ConfigEnv", "DEFAULT_API_VERSION", "DEFAULT_DISCOVERY_FILTERS", + "DEFAULT_PRIVATE_REPOS", "DEFAULT_SETTINGS_FILE", "DOCUMENT_DIRECTIVE_KEYS", "DiscoveryFilters", + "DiscoveryProblem", "EndpointDecl", + "FILTER_INPUTS", "FORKS_FILTERS", + "FinishedMerge", "GithubApi", "GithubClient", "GraphqlOp", "GraphqlOpDecl", + "INPUT_DECLS", "ISSUE_TITLE", + "InputDecl", + "InputName", + "InputReader", "Io", "KeyedListLayering", "Layer", + "LayerProblem", "Layering", "MARKER_LABEL", "MARKER_LABEL_CONFIG", + "MERGE_INPUTS", + "MERGE_REJECTED_INPUTS", "MERGE_RESULT", + "MODES", "MaskPair", "MergeConfig", + "Mode", "MultiConfig", "MustBeNever", "OptOutNotice", @@ -65,6 +80,8 @@ describe("the public entry", () => { "PatResource", "PrivateReportChannel", "PrivateReposPolicy", + "Problem", + "ProblemOf", "PublicTargetView", "REPO_RESULTS", "RERUN_ADVICE", @@ -77,6 +94,7 @@ describe("the public entry", () => { "RepoVisibility", "ReportInput", "Route", + "RunConfig", "RunFlowConfig", "SECTIONS", "SECTION_KEYS", @@ -85,11 +103,16 @@ describe("the public entry", () => { "SectionModule", "SectionOutcome", "SectionPermission", + "SectionSelection", "SettingsFile", + "SettingsFileRole", + "SettingsProblem", "SingleConfig", + "SingleOutcome", "TaggedEndpoint", "Target", "TargetOutcome", + "TopLevelShape", "TraceIo", "UNDECLARED_POLICY_SECTIONS", "UndeclaredPolicy", @@ -105,12 +128,14 @@ describe("the public entry", () => { "checkRepository", "collectingIo", "composeReport", + "concludeMerge", "concludeRun", "createVisibilityResolver", "dedupeTargets", "deliverArtifactReport", "denialPosture", "describeOptOut", + "describeProblem", "discoverRepos", "encryptReport", "endpointMethod", @@ -124,6 +149,7 @@ describe("the public entry", () => { "maskRegistry", "mergeLayers", "openReportChannel", + "parseConfig", "parseRecipient", "parseRepoSlug", "parseReposInput", diff --git a/test/private.test.ts b/test/private.test.ts index 04c2d167..9a4a5eec 100644 --- a/test/private.test.ts +++ b/test/private.test.ts @@ -39,7 +39,6 @@ const OPENER_IMPORTERS: Record = { ], "src/private.ts": ["isPrivate", "markPrivate"], "src/report/delivery.ts": [ - "ARTIFACT_NEEDS_UPLOADER", "PRIVATE_REPORT_CHANNELS", "applyMarkerInjection", "isIssueChannel", diff --git a/test/problem.test.ts b/test/problem.test.ts new file mode 100644 index 00000000..8de5dd81 --- /dev/null +++ b/test/problem.test.ts @@ -0,0 +1,396 @@ +/** + * The problem union's renderer: one specimen per member, each pinned to the + * exact line the action prints for it. The specimen table is typed over every + * code, so a new member fails compilation here until it has a specimen, and + * describeProblem's switch fails until it has a case. + */ + +import { describe, expect, test } from "bun:test"; +import { + describeProblem, + type Problem, + type ProblemOf, + quoteList, + type SettingsProblem, +} from "../src/problem.js"; +import { SECTION_KEYS } from "../src/schema.js"; + +const KNOWN = SECTION_KEYS.join(", "); + +const PASSTHROUGH = + "Fix these values in the settings file (only the named keys are validated; extra fields pass " + + "through, except in closed sections and strict nested objects like actions.cache, which reject " + + "unrecognized keys)"; + +const RERUN = + "This is not a permission problem; re-run the workflow, and retry later if it persists"; + +const PAT = + "Discovery needs a user PAT; the workflow GITHUB_TOKEN and GitHub App installation tokens " + + 'cannot enumerate a user\'s repositories. List the target repositories explicitly in the "repos" input'; + +/** Every member once: its specimen and the line it renders to. */ +const SPECIMENS = { + "input-unsupported-value": [ + { + code: "input-unsupported-value", + input: "mode", + value: "dry-run", + noun: "mode", + allowed: ["apply", "check", "merge"], + fallback: "apply", + }, + 'the "mode" input is "dry-run", which is not a supported mode. Set it to "apply" (default), "check", "merge"', + ], + "input-unknown-sections": [ + { + code: "input-unknown-sections", + unknown: [ + { input: "required-sections", names: ["nope"] }, + { input: "sections", names: ["typo", "nope"] }, + ], + known: SECTION_KEYS, + }, + `unknown section "nope" in the "required-sections" input; it matches none of: ${KNOWN}. Fix the name in the workflow's input list; ` + + `unknown sections "typo", "nope" in the "sections" input; each matches none of: ${KNOWN}. Fix the names in the workflow's input list`, + ], + "required-sections-excluded": [ + { code: "required-sections-excluded", excluded: ["labels", "milestones"] }, + 'the "required-sections" entries "labels", "milestones" are excluded by the "sections" allowlist, so the run would pass without ever attempting them. Add them to the "sections" input, or remove them from "required-sections"', + ], + "input-report-key-unused": [ + { code: "input-report-key-unused", channel: "issue" }, + 'the "report-public-key" input only applies to private-report: artifact, but the channel is "issue", so the key would never be used. Remove report-public-key, or set private-report: artifact', + ], + "input-report-key-missing": [ + { code: "input-report-key-missing" }, + 'private-report: artifact needs a "report-public-key" input: the age recipient every report ' + + 'is encrypted to. Generate a keypair with "age-keygen -o key.txt", keep key.txt secret, and ' + + 'set report-public-key to the printed "age1..." recipient (safe to commit)', + ], + "input-report-key-invalid": [ + { code: "input-report-key-invalid", reason: "invalid recipient" }, + 'the "report-public-key" input is not a valid age recipient: invalid recipient. It must be an "age1..." public key from "age-keygen" (the recipient line, not the AGE-SECRET-KEY identity)', + ], + "input-rejected-in-merge": [ + { code: "input-rejected-in-merge", inputs: ["repos", "repos-dir"] }, + 'the "repos", "repos-dir" input(s) do not apply to mode: merge, which only folds the ' + + "settings-file layers into merged-file: it never targets a repository, calls the GitHub API, " + + "delivers a report, or narrows the sections it writes. Remove the input(s), or move them to " + + "the apply or check step that runs the merged document", + ], + "input-merged-file-missing": [ + { code: "input-merged-file-missing" }, + 'mode: merge needs a "merged-file" input: the path the merged settings document is written to. Set it (for example .github/settings.merged.yml) and feed that path to a later apply or check step as its settings-file', + ], + "input-settings-file-empty": [ + { code: "input-settings-file-empty", value: "," }, + 'the "settings-file" input is ",", which lists no file. In mode: merge it is the ordered list of layers to fold, newline- or comma-separated, lowest first; name at least one settings file', + ], + "input-merge-only": [ + { code: "input-merge-only", inputs: ["merged-file", "layering"], mode: "check" }, + 'the "merged-file", "layering" input(s) only apply to mode: merge, but this run is in check mode, so they would never be used. Remove the input(s), or set mode: merge to fold settings files', + ], + "input-token-missing": [ + { code: "input-token-missing" }, + 'cannot call the GitHub API: no token was provided. Set the "token" input on the action step (or export GITHUB_TOKEN)', + ], + "input-report-without-redaction": [ + { code: "input-report-without-redaction" }, + 'the "private-report" input delivers reports only for redacted targets, but "private-repos" is "show", so nothing is redacted and no report would ever be sent. Set private-repos: redact, or set private-report: none', + ], + "input-affiliation-unsupported": [ + { + code: "input-affiliation-unsupported", + entry: "member", + allowed: ["owner", "collaborator", "organization_member"], + }, + 'the "affiliation" input entry "member" is not a supported affiliation, so discovery cannot build the /user/repos query. Use a comma-separated list of "owner", "collaborator", "organization_member"', + ], + "input-exclude-pattern-invalid": [ + { code: "input-exclude-pattern-invalid", pattern: "a/b/c" }, + 'the "exclude" input pattern "a/b/c" can never match an owner/name repository: a pattern takes at most one "/", with a non-empty glob on each side of it. Use "" or "/", where "*" matches any characters', + ], + "input-repository-with-multi": [ + { code: "input-repository-with-multi" }, + 'the "repository" input cannot be combined with "repos" or "repos-dir"; multi-repo targets come from those inputs. Remove "repository", or remove the multi-repo inputs to stay in single-repo mode', + ], + "input-settings-file-with-multi": [ + { code: "input-settings-file-with-multi" }, + 'the "settings-file" input cannot be combined with "repos" or "repos-dir": central targets are read from repos-dir files and remote targets from each repository\'s own .github/settings.yml. Remove the settings-file override', + ], + "discovery-filters-without-wildcard": [ + { code: "discovery-filters-without-wildcard", filters: ["forks"], targets: "single-repo" }, + 'the discovery filter input(s) "forks" only apply to repos: "*" discovery, but this run is in single-repo mode. Set repos: "*" to discover repositories, or remove the filter input(s)', + ], + "input-defaults-file-without-multi": [ + { code: "input-defaults-file-without-multi" }, + 'the "defaults-file" input only applies to multi-repo mode, but this run is in single-repo mode, so the defaults would never apply. Remove the input, or add "repos" or "repos-dir" to switch to multi-repo mode', + ], + "input-settings-file-is-list": [ + { code: "input-settings-file-is-list", value: "a.yml,b.yml", mode: "apply" }, + 'the "settings-file" input is "a.yml,b.yml", which contains a list separator: apply mode ' + + "reads exactly one settings file, and only mode: merge takes a newline- or comma-separated " + + "list. Name one file, or set mode: merge to fold the list into one document", + ], + "input-repository-not-slug": [ + { code: "input-repository-not-slug", value: "nope" }, + 'cannot target a repository: "nope" is not an owner/name slug. Set the "repository" input (or GITHUB_REPOSITORY) to a value like "octocat/hello-world"', + ], + "settings-not-mapping": [ + { code: "settings-not-mapping", source: "f.yml", shape: "list" }, + 'f.yml must be a YAML mapping of section names to settings, but its top level parsed as a list. Rewrite the top level as "section: ..." keys', + ], + "settings-not-plain-mapping": [ + { code: "settings-not-plain-mapping", source: "f.yml" }, + 'f.yml must be a plain YAML mapping of section names to settings, but its top level parsed as another type (a YAML-tagged value like !!timestamp parses to a Date). Rewrite the top level as "section: ..." keys', + ], + "settings-unknown-sections": [ + { code: "settings-unknown-sections", source: "f.yml", unknown: ["labls"], known: SECTION_KEYS }, + `unknown top-level section(s) in f.yml: labls (known: ${KNOWN}). Fix the typo, or prefix private keys with "_", or set the "sections" input to limit processing`, + ], + "settings-malformed-sections": [ + { + code: "settings-malformed-sections", + source: "f.yml", + issues: ["labels[0].new_name: Invalid input: expected string, received number", "pages: x"], + }, + `f.yml has malformed section entries: labels[0].new_name: Invalid input: expected string, received number; pages: x. ${PASSTHROUGH}`, + ], + "yaml-invalid": [ + { code: "yaml-invalid", reason: "YAMLParseError: Flow sequence in block collection" }, + "YAMLParseError: Flow sequence in block collection", + ], + "settings-file-unreadable": [ + { + code: "settings-file-unreadable", + role: "settings-file", + path: "missing.yml", + reason: "ENOENT", + }, + 'cannot read settings from missing.yml: ENOENT. Check that the file exists at that path (set the "settings-file" input if it lives elsewhere) and is valid YAML', + ], + "layer-cycle": [ + { code: "layer-cycle", layer: "repo", site: "the document" }, + 'layer "repo": the document contains a reference cycle (a YAML anchor that includes itself); layers must be trees', + ], + "layer-wrong-shape": [ + { + code: "layer-wrong-shape", + layer: "repo", + site: "labels", + expected: "a list of mappings or an {_undeclared, entries} wrapper", + actual: { _undeclared: "keep" }, + detail: " without an entries list", + }, + 'layer "repo": labels must be a list of mappings or an {_undeclared, entries} wrapper; got a mapping without an entries list', + ], + "layer-bad-directive": [ + { code: "layer-bad-directive", layer: "repo", site: "labels._layering", actual: "union" }, + 'layer "repo": labels._layering must be "merge" or "replace"; got a string that is neither', + ], + "layer-no-layering-key": [ + { code: "layer-no-layering-key", layer: "repo", site: "milestones" }, + 'layer "repo": milestones has no layering key, so it cannot be layered by "merge"; declare _layering: replace or drop the directive', + ], + "layer-no-key": [ + { code: "layer-no-key", layer: "repo", site: "labels[1]", keyField: "name" }, + 'layer "repo": labels[1] carries no string "name", which every entry needs to layer by', + ], + "layer-duplicate-key": [ + { + code: "layer-duplicate-key", + layer: "repo", + site: "labels", + keyField: "name", + first: 0, + second: 2, + }, + 'layer "repo": labels[0] and labels[2] both claim one name; each name belongs to one entry within a layer', + ], + "artifact-uploader-missing": [ + { code: "artifact-uploader-missing" }, + "private-report: artifact needs an artifact uploader, and none was supplied: the action supplies its own; a library caller passes one as the uploader argument, or picks another private-report channel", + ], + "merged-file-is-layer": [ + { code: "merged-file-is-layer", mergedFile: "./repo.yml", index: 1, layer: "repo.yml" }, + 'the "merged-file" input "./repo.yml" is layer 2 of the "settings-file" list ("repo.yml"): ' + + "the merge would overwrite that layer with the folded document, and the next run would fold " + + "the merged document as a layer. Write the merged document to a path outside the layer list", + ], + "merged-file-unwritable": [ + { code: "merged-file-unwritable", path: "out/merged.yml", reason: "EACCES" }, + 'cannot write the merged document to out/merged.yml: EACCES. Check that the "merged-file" input names a writable path', + ], + "no-targets": [ + { code: "no-targets", filteredOut: 2 }, + 'multi-repo mode found no targets: repos: "*" discovery found 2 repositories, but the discovery filters removed all of them (see the notices above). Relax the filter inputs, or add per-repo files to the repos-dir', + ], + "repo-slug-invalid": [ + { code: "repo-slug-invalid", value: "nope" }, + '"nope" is not an owner/name repository slug (use a value like "octocat/hello-world")', + ], + "repos-input-wildcard-mixed": [ + { code: "repos-input-wildcard-mixed" }, + 'the "repos" input mixes "*" with explicit repositories. Use "*" alone to discover every repository the token owns, or list the repositories without it', + ], + "repos-input-invalid-entries": [ + { code: "repos-input-invalid-entries", invalid: ["bad", "worse"], duplicated: ["O/A"] }, + 'the "repos" input has 3 invalid entries: "bad", "worse" are not owner/name slugs (use values ' + + 'like "octocat/hello-world", comma- or newline-separated); "O/A" is listed more than once ' + + '(keep exactly one entry per repository). Or use "*" alone to discover repositories', + ], + "repos-dir-missing": [ + { code: "repos-dir-missing", reposDir: "repos" }, + 'repos-dir "repos" does not exist in the workspace, so there are no central settings files to read. Add an actions/checkout step before this action, or fix the repos-dir path', + ], + "repos-dir-unreadable": [ + { code: "repos-dir-unreadable", reposDir: "repos", reason: "EACCES" }, + 'cannot read repos-dir "repos": EACCES. Check that it is a readable directory of settings files', + ], + "repos-dir-invalid-files": [ + { + code: "repos-dir-invalid-files", + reposDir: "repos", + files: [ + { kind: "not-a-slug", filePath: "repos/o/a b.yml", slug: "o/a b" }, + { kind: "duplicate", slug: "o/x", first: "repos/o/x.yml", second: "repos/x.yml" }, + { kind: "ownerless", files: ["repos/api.yml", "repos/web.yml"] }, + ], + }, + 'repos-dir "repos" has 3 invalid settings file(s):\n' + + '- repos/o/a b.yml resolves to the target "o/a b", which is not a valid owner/name slug. Rename the file so and contain only letters, digits, dots, underscores, and dashes\n' + + "- duplicate target o/x: defined by both repos/o/x.yml and repos/x.yml. Keep exactly one settings file per repository\n" + + "- cannot resolve repos/api.yml, repos/web.yml: top-level repos-dir files use the current repository's owner, which is unknown outside GitHub Actions. Use the /.yml layout instead", + ], + "discovery-request-failed": [ + { + code: "discovery-request-failed", + path: "/user/repos?affiliation=owner", + status: 403, + message: "Resource not accessible", + denied: true, + }, + `cannot discover repositories for repos: "*": GET /user/repos?affiliation=owner failed: 403 Resource not accessible. ${PAT}`, + ], + "discovery-transport-failed": [ + { code: "discovery-transport-failed", reason: "fetch failed" }, + `cannot discover repositories for repos: "*": fetch failed. ${RERUN}`, + ], + "discovery-response-not-a-list": [ + { code: "discovery-response-not-a-list", path: "/user/repos?affiliation=owner" }, + `cannot discover repositories for repos: "*": GET /user/repos?affiliation=owner returned a JSON value that is not a list, so the response cannot be paginated. ${RERUN}`, + ], + "age-recipient-invalid": [ + { code: "age-recipient-invalid", reason: "invalid recipient" }, + "not a valid age recipient: invalid recipient", + ], +} satisfies { [C in Problem["code"]]: [ProblemOf, string] }; + +describe("describeProblem", () => { + test.each(Object.entries(SPECIMENS))("renders %s", (_code, [problem, line]) => { + expect(describeProblem(problem)).toBe(line); + }); + + test.each<[what: string, problem: Problem, line: string]>([ + [ + "one excluded required section reads in the singular", + { code: "required-sections-excluded", excluded: ["labels"] }, + 'the "required-sections" entry "labels" is excluded by the "sections" allowlist, so the run would pass without ever attempting it. Add it to the "sections" input, or remove it from "required-sections"', + ], + [ + "one merge-only input reads in the singular", + { code: "input-merge-only", inputs: ["layering"], mode: "apply" }, + 'the "layering" input(s) only apply to mode: merge, but this run is in apply mode, so it would never be used. Remove the input(s), or set mode: merge to fold settings files', + ], + [ + "filters beside an explicit repos list", + { + code: "discovery-filters-without-wildcard", + filters: ["forks", "topics"], + targets: "explicit-repos", + }, + 'the discovery filter input(s) "forks", "topics" only apply when repos is "*", but the "repos" input lists explicit repositories. Set repos: "*", or remove the filter input(s)', + ], + [ + "filters beside repos-dir targets only", + { code: "discovery-filters-without-wildcard", filters: ["forks"], targets: "repos-dir" }, + 'the discovery filter input(s) "forks" only apply to repos: "*" discovery, but targets come only from repos-dir files. Set repos: "*", or remove the filter input(s)', + ], + [ + "a defaults file that cannot be read", + { code: "settings-file-unreadable", role: "defaults-file", path: "d.yml", reason: "ENOENT" }, + 'cannot read the defaults file d.yml: ENOENT. Check the "defaults-file" path and that the file is valid YAML', + ], + [ + "a layer that cannot be read", + { code: "settings-file-unreadable", role: "layer", path: "fleet.yml", reason: "ENOENT" }, + 'cannot read the settings layer fleet.yml: ENOENT. Check that every path in the "settings-file" input exists and is valid YAML', + ], + [ + "no targets with nothing filtered", + { code: "no-targets", filteredOut: 0 }, + 'multi-repo mode found no targets: repos-dir yielded no settings files and the "repos" input resolved to no repositories. Add per-repo files to the repos-dir, or list repositories in the "repos" input', + ], + [ + "one filtered repository reads in the singular", + { code: "no-targets", filteredOut: 1 }, + 'multi-repo mode found no targets: repos: "*" discovery found 1 repository, but the discovery filters removed all of them (see the notices above). Relax the filter inputs, or add per-repo files to the repos-dir', + ], + [ + "one invalid repos entry reads in the singular", + { code: "repos-input-invalid-entries", invalid: ["not-a-slug"], duplicated: [] }, + 'the "repos" input has 1 invalid entry: "not-a-slug" is not an owner/name slug (use values like "octocat/hello-world", comma- or newline-separated). Or use "*" alone to discover repositories', + ], + [ + "a request failure that is not a denial gets re-run advice", + { + code: "discovery-request-failed", + path: "/user/repos?affiliation=owner", + status: 403, + message: "API rate limit exceeded for user", + denied: false, + }, + `cannot discover repositories for repos: "*": GET /user/repos?affiliation=owner failed: 403 API rate limit exceeded for user. ${RERUN}`, + ], + [ + "a non-string directive is described by shape alone", + { code: "layer-bad-directive", layer: "repo", site: "_layering", actual: true }, + 'layer "repo": _layering must be "merge" or "replace"; got a boolean', + ], + [ + "a tagged value where a list belongs is described by its class", + { + code: "layer-wrong-shape", + layer: "repo", + site: "milestones", + expected: "a list of mappings or an {_undeclared, entries} wrapper", + actual: new Date(0), + }, + 'layer "repo": milestones must be a list of mappings or an {_undeclared, entries} wrapper; got a Date value', + ], + ])("renders the variant: %s", (_what, problem, line) => { + expect(describeProblem(problem)).toBe(line); + }); +}); + +describe("SettingsProblem", () => { + test("names the four validation refusals and not a file's read failure", () => { + const codes: SettingsProblem["code"][] = [ + "settings-not-mapping", + "settings-not-plain-mapping", + "settings-unknown-sections", + "settings-malformed-sections", + ]; + // @ts-expect-error a read failure shares the prefix but is not a validation problem + const unreadable: SettingsProblem["code"] = "settings-file-unreadable"; + expect(codes).not.toContain(unreadable); + }); +}); + +describe("quoteList", () => { + test("quotes each name and joins with commas", () => { + expect(quoteList(["a", "b c"])).toBe('"a", "b c"'); + expect(quoteList([])).toBe(""); + }); +}); diff --git a/test/published-schema.test.ts b/test/published-schema.test.ts index b560a02a..5377f5b3 100644 --- a/test/published-schema.test.ts +++ b/test/published-schema.test.ts @@ -12,6 +12,7 @@ import { describe, expect, test } from "bun:test"; import { readFileSync } from "node:fs"; import { join } from "node:path"; import { Ajv, type ValidateFunction } from "ajv"; +import { ok } from "neverthrow"; import { validateSectionShapes } from "../src/engine/validate.js"; import { SettingsFile, UNDECLARED_POLICY_SECTIONS } from "../src/schema.js"; import { FLAG_PAIRING_FIXTURES } from "./fixtures/environment-flag-pairing.js"; @@ -325,6 +326,6 @@ describe("the document-level _layering directive", () => { test("the apply-path shape validation copies only sections, so the directive never reaches the engine", () => { expect( validateSectionShapes({ _layering: "replace", labels: [{ name: "bug" }] }, "settings.yml"), - ).toEqual({ settings: { labels: [{ name: "bug" }] } }); + ).toEqual(ok({ labels: [{ name: "bug" }] })); }); }); diff --git a/test/report/artifact-report.test.ts b/test/report/artifact-report.test.ts index 6bf9baa5..c3a0b53f 100644 --- a/test/report/artifact-report.test.ts +++ b/test/report/artifact-report.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; import { Decrypter, generateX25519Identity, identityToRecipient } from "age-encryption"; +import { err, ok } from "neverthrow"; import { ARTIFACT_FILE, ARTIFACT_NAME, @@ -45,7 +46,7 @@ describe("encryptReport", () => { describe("parseRecipient", () => { test("accepts a generated age recipient", async () => { const { recipient } = await testKeypair(); - expect(parseRecipient(recipient)).toEqual({ ok: true }); + expect(parseRecipient(recipient)).toEqual(ok()); }); test.each([ @@ -54,8 +55,9 @@ describe("parseRecipient", () => { "age1shortandinvalid", // gitleaks:allow "AGE-SECRET-KEY-1NOTPUBLIC", ])("rejects a malformed recipient: %j", (recipient) => { - const result = parseRecipient(recipient); - expect(result.ok).toBe(false); + expect(parseRecipient(recipient)).toEqual( + err({ code: "age-recipient-invalid", reason: expect.any(String) }), + ); }); }); diff --git a/test/report/delivery.test.ts b/test/report/delivery.test.ts index cc6dc820..8e200f8d 100644 --- a/test/report/delivery.test.ts +++ b/test/report/delivery.test.ts @@ -10,6 +10,7 @@ import { runOutcome } from "../../src/flows/deliver.js"; import { redactedChannel } from "../../src/flows/redact.js"; import { type Io, maskRegistry, silentIo } from "../../src/io.js"; import { isPrivate, type Private } from "../../src/private.js"; +import { describeProblem } from "../../src/problem.js"; import type { ArtifactUploader } from "../../src/report/artifact-report.js"; import { applyMarkerInjection, @@ -58,11 +59,7 @@ function sealed(slug: string, outcomes: SectionOutcome[]): Private { // here instead of riding a cast into the injection. const validated = (doc: SettingsFile): ValidatedSettings => { const verdict = validateSettingsDoc(doc, "test fixture", new Set(), silentIo()); - if ("error" in verdict) { - throw new Error(`test fixture failed validation: ${verdict.error}`); + if (verdict.isErr()) { + throw new Error(`test fixture failed validation: ${describeProblem(verdict.error)}`); } - return verdict.settings; + return verdict.value; }; const bug = { name: "bug", color: "d73a4a" }; const marker = { name: MARKER, color: "0e2a47" }; diff --git a/test/report/issue-report.test.ts b/test/report/issue-report.test.ts index 2d7a1a60..6f254ef9 100644 --- a/test/report/issue-report.test.ts +++ b/test/report/issue-report.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test"; import { validateSettingsDoc } from "../../src/engine/orchestrate.js"; import { silentIo } from "../../src/io.js"; +import { describeProblem } from "../../src/problem.js"; import { deliverIssueReport, ISSUE_TITLE, @@ -500,7 +501,7 @@ describe("injectMarkerLabel", () => { expect(result.outcome).toBe(expected as typeof result.outcome); const verdict = validateSettingsDoc(result.settings, "injected doc", new Set(), silentIo()); expect( - "error" in verdict ? verdict.error : null, + verdict.match(() => null, describeProblem), `outcome "${expected}" produced a document validation rejects`, ).toBeNull(); } diff --git a/test/scripts/gen-action-docs.test.ts b/test/scripts/gen-action-docs.test.ts index 1ac3a4d6..791b65ce 100644 --- a/test/scripts/gen-action-docs.test.ts +++ b/test/scripts/gen-action-docs.test.ts @@ -22,8 +22,8 @@ import { type RegionSpec, regionBounds, } from "../../.github/scripts/lib/generated-regions.js"; -import { INPUT_DECLS } from "../../src/action/inputs.js"; import { OUTPUT_DECLS } from "../../src/action/io.js"; +import { INPUT_DECLS } from "../../src/flows/inputs.js"; import type { SectionMeta } from "../../src/sections/contract/module.js"; import { sectionModule } from "../../src/sections/registry.js"; import { relocatedRegion } from "./relocated-region.js";