Skip to content

Commit 4561d2c

Browse files
author
settings-as-code-release
committed
fix(cli): derive list flags from the input declarations, refuse the artifact channel, drop the snapshot placeholders
Which inputs are lists is now one declaration: InputDecl gains `list`, parseConfig reads such inputs only through its list() port (a name without the attribute cannot be split), and the CLI lets exactly those flags repeat. A complete Record over every input runs one repeated-argv case per name, so a new list input cannot be missed. The snapshot and init subcommands are gone until the snapshot mode exists; no build could run them. `--private-report artifact` is refused before parsing (the upload needs the Actions runner) and report-public-key is not a flag. The no-token and not-a-slug problems name --token, --repository, and the environment variables; the --repository help says the flag is required outside Actions. Tests assert whole outcomes: exact option sets and descriptions per subcommand, the fold's own shape in the merged file, the whole --json object with exit code and stderr, and where an unnamed summary goes. The package smoke gains a failing control (validate on an invalid file exits 1). The redacting stream honors the target's backpressure, with a guard test seen failing on the old code; the bin's empty declaration file is removed after the build; docs and AGENTS.md trimmed to what the code cannot show.
1 parent 8c06ebd commit 4561d2c

13 files changed

Lines changed: 438 additions & 190 deletions

File tree

‎.github/scripts/package-smoke.ts‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
* directory is removed on every path, failure included.
1313
*/
1414

15-
import { execFileSync } from "node:child_process";
15+
import { execFileSync, spawnSync } from "node:child_process";
1616
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
1717
import { tmpdir } from "node:os";
1818
import { join } from "node:path";
@@ -43,12 +43,26 @@ export const ok: boolean = result.isOk() && first === "repository";
4343
/** The settings file the installed CLI validates: one section, valid as written. */
4444
const SETTINGS_FILE = "labels:\n - name: bug\n color: d73a4a\n";
4545

46+
/** The control: a label without its name, which the validator refuses. */
47+
const INVALID_SETTINGS_FILE = "labels:\n - color: d73a4a\n";
48+
4649
/** Run a command to completion in `cwd`, streaming its output; a non-zero exit throws. */
4750
function run(command: string, args: string[], cwd: string): void {
4851
console.log(`$ ${[command, ...args].join(" ")}`);
4952
execFileSync(command, args, { cwd, stdio: "inherit" });
5053
}
5154

55+
/** Run a command expected to fail; any other exit code, zero included, throws. */
56+
function expectExit(code: number, command: string, args: string[], cwd: string): void {
57+
console.log(`$ ${[command, ...args].join(" ")} # expecting exit ${code}`);
58+
const result = spawnSync(command, args, { cwd, stdio: "inherit" });
59+
if (result.status !== code) {
60+
throw new Error(
61+
`${[command, ...args].join(" ")} exited ${result.status ?? "by signal"}, expected ${code}`,
62+
);
63+
}
64+
}
65+
5266
/** Run a command and return its stdout. */
5367
function capture(command: string, args: string[], cwd: string): string {
5468
console.log(`$ ${[command, ...args].join(" ")}`);
@@ -130,6 +144,9 @@ async function main(): Promise<void> {
130144
}
131145
writeFileSync(join(consumerDir, "settings.yml"), SETTINGS_FILE);
132146
run("node", [bin, "validate", "settings.yml"], consumerDir);
147+
// The failing control: a bin that swallowed its exit code would pass the line above.
148+
writeFileSync(join(consumerDir, "invalid.yml"), INVALID_SETTINGS_FILE);
149+
expectExit(1, "node", [bin, "validate", "invalid.yml"], consumerDir);
133150
writeFileSync(join(consumerDir, "main.ts"), TS_CONSUMER);
134151
run(
135152
join(REPO_ROOT, "node_modules", ".bin", "tsc"),

‎AGENTS.md‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,7 @@ GitHub Settings as Code: GitHub Action applying declarative repository settings:
3636

3737
- `lib/settings.schema.json` is the COMMITTED JSON Schema for settings.yml, generated from the zod slices in `src/sections/<key>/schema.ts` as composed into the settings document by `src/schema.ts`. Each slice is ONE declaration that produces its config type (z.infer), the section's tolerant runtime shape (the section module derives `loosen(<slice>)` itself; the factory-minted families - secrets, variables, the two setup sections, and the list sections on `listSection` - get it from their shared factory in `src/sections/shared/`, which does that derivation once), and its part of the published schema (`.meta({id})` names its definitions; the descriptions come from the `schema:` map in the section's `<key>.docs.yml`, keyed `Definition.field`, which the generator attaches and fails on when a site or a key is unmatched); `src/schema.ts` adds only the document-level wrappers and pins each property to its slice with a lockstep type. Regenerate the schema with `bun run build:schema` after a schema-affecting `src/` change - CI's schema-check job fails when it drifts. `lib/index.js`, the bundled entrypoint the action runs (node24), is NOT committed on main: every workflow that executes the action builds it first (`bun run build:bundle`), the e2e runner builds it to a temp path, and a release builds and commits it on a packaged child of the release commit, which is what every `vX.Y.Z` tag and the moving major point at (main itself stays source-only). `lib/` is exempt from the typography check (third-party unicode in the bundle; the schema descriptions are checked at source) and excluded from [biome](https://biomejs.dev).
3838
- `lib/pkg/` is the npm library build: `bun run build:lib` (tsdown, configured in `tsdown.config.ts`) bundles `src/index.ts` into one ESM `index.js` plus one `index.d.ts` with the runtime dependencies external; like `lib/index.js` it is built, never committed. `package.json` publishes it as `@vivswan/github-settings-as-code` through its `exports` map (`.` to the build, `./settings.schema.json` to the committed schema, `./package.json` to itself; `files` ships only those plus LICENSE.md and README.md), and `test/package-json.test.ts` pins the manifest's field set and holds its `version` equal to `.release-please-manifest.json` (release-please's json extra-file rewrites it). The package-smoke job in `checks.yml` (`bun .github/scripts/package-smoke.ts`) builds the library, runs publint and attw, installs the packed tarball into a fresh consumer, and imports it under Node and under tsc with skipLibCheck off, so a declaration that leaks a devDependency type or a type the emitter cannot name fails there; a type a section module exposes must therefore be exported from its home module. The library reference is docs/reference/library.md.
39-
- `src/cli.ts` and `src/cli/` are the command line, a second CONSUMER of the public entry with the action's one edge, `../index.js`:
40-
a capability the CLI needs is exported from `src/index.ts` first, and `test/cli/inputs.test.ts` pins its argv-to-config equal to the action's env-to-config. Guide: docs/start/cli.md.
39+
- `src/cli/` is the command line, a consumer of the public entry like the action: a capability it needs is exported from `src/index.ts` first, never imported across. Guide: docs/start/cli.md.
4140
- The import layering of `src/` is declared in `architecture.yml` and enforced by `bun run lint:arch`, so a new cross-layer import is a deliberate edit to that file; the module map in `docs/reference/architecture.md` is generated from it.
4241
- The apply/check engine layout: `src/main.ts` is the thin bundled entrypoint; `src/index.ts` is the public entry (declaration-only re-exports, pinned by test/library/public-surface.test.ts); `src/action/` is the GitHub Actions layer and a CONSUMER of that entry: its only import outside itself is `../index.js`, the one edge `architecture.yml` grants it; `src/flows/` holds the run flows the action and a library caller share (single, multi, merge, delivery, redaction); `src/io.ts` is the import-free Io output port; `src/types.ts` holds the leaf type vocabulary; `src/engine/` is the per-repo pipeline (orchestrate, validate, merge, diff, secret references); `src/github/` is the REST client, pagination, and repo-file fetch; `src/discovery/` resolves multi-repo targets; `src/report/` composes and delivers the private report. Under `src/sections/`, each settings section is one self-contained DIRECTORY `src/sections/<key>/`: `index.ts` (the `SectionModule` - key, PAT grant advice, the loose shape derived from the slice, handler), `schema.ts` (the zod slice), `mock.ts` (the e2e mock fragment), `generators.ts` (the fuzz fragment; the secret and variable factory families share theirs in `test/e2e/generators.ts` instead), `<key>.test.ts` (absent where a shared factory's suite under `test/sections/` covers the section), and `scenarios/`. Beside the section directories sit `contract/` (the layered section contract: permissions/endpoints/graphql/module/errors/requests/live), `shared/` (the cross-section engines and factories, plus the schema helpers), and `registry.ts` (the single registration point). All GitHub API list calls must go through `listAll()` (bare-array endpoints) or `listAllEnveloped()` (endpoints that wrap the list in a `{total_count, <key>: []}` envelope), both backed by the single page loop in `src/github/paginate.ts`; errors through `call()`/`throwFor()` so the permission policy (`on-missing-permission`, `required-sections`) works.
4342
- Adding a section: create `src/sections/<key>/` with the six pieces above plus its `<key>.docs.yml` (the Sections table cells for docs/reference/sections.md, the COVERAGE.md prose, and the schema descriptions the generators render; the docs loader fails without it), compose the `SettingsFile` property from the slice and add the key to `SECTION_KEYS` in `src/schema.ts`, and add one line in `src/sections/registry.ts` - the compiler flags each forgotten step (the registry's mapped type, the mock fragment registration, the slice composition pin, and the generator aggregation are all keyed by SectionKey).

‎docs/start/cli.md‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ order: 40
55
# Command line
66

77
The engine behind the action is also a command in the npm package `@vivswan/github-settings-as-code`: `github-settings-as-code`, or `gsac` for short.
8-
It runs the same checks, applies, and merges the action runs, from a terminal or any CI, plus three file-only commands the action has no step for:
9-
validate a settings file, print the PAT grant it needs, and (in a build that carries the snapshot mode) write a repository's live settings as a file.
8+
It runs the same checks, applies, and merges the action runs, from a terminal or any CI, plus two file-only commands the action has no step for:
9+
validate a settings file, and print the PAT grant it needs.
1010

1111
## Install
1212

@@ -35,10 +35,6 @@ The package needs Node 22.14 or newer; `bunx` fetches it the same way and honors
3535
| `merge` | Fold an ordered list of settings files into one document | no |
3636
| `validate <file>` | Validate a settings file against the schema | no |
3737
| `permissions <file>` | Print the PAT grant each section the file declares needs | no |
38-
| `snapshot` | Write the live repository settings as a settings file | yes |
39-
| `init` | Snapshot into `.github/settings.yml` and print the PAT grant that file needs | yes |
40-
41-
`snapshot` and `init` need a build of the package whose library carries `mode: snapshot`; a build without it exits 1 naming the missing mode.
4238

4339
### check
4440

@@ -58,7 +54,8 @@ The defaults are the action's, redaction included: a private repository's lines
5854
gsac apply --repository octo-org/api --settings-file .github/settings.yml --on-missing-permission warn
5955
```
6056

61-
Every flag of the action's `apply` and `check` inputs is a flag here, spelled the same way; `--repos`, `--repos-dir`, and `--defaults-file` switch to [multi-repo mode](../operate/multi-repo.md), and the discovery filters follow.
57+
Every flag of the action's `apply` and `check` inputs is a flag here, spelled the same way, with two exceptions below. `--repos` or `--repos-dir` switches to [multi-repo mode](../operate/multi-repo.md); `--defaults-file` and the discovery filters apply there, as in the action.
58+
Outside GitHub Actions there is no `GITHUB_REPOSITORY`, so single-repo runs need `--repository`.
6259

6360
### merge
6461

@@ -90,6 +87,8 @@ The subcommand is the action's `mode` input. Every other input of that mode is a
9087
the [inputs reference](../reference/inputs.md) lists each one with its default and meaning, and `gsac <command> --help` prints the same descriptions.
9188
A list input (`--settings-file` under merge, `--repos`, `--exclude`, `--topics`, `--affiliation`, `--sections`, `--required-sections`) takes a comma-separated value or the flag repeated; repeating any other value flag, `--token` and `--summary` included, is an error naming it.
9289

90+
Two inputs have no command-line form: `--private-report artifact` is refused (the artifact upload needs the Actions runner; `issue`, `issue-on-failure`, and `none` work), and `report-public-key`, which only that channel reads, is not a flag.
91+
9392
Four flags are the command line's own:
9493

9594
| Flag | Does |
@@ -117,5 +116,5 @@ With `--json` the outputs are one object instead, and stdout carries nothing els
117116

118117
The exit codes are the action's: `check` exits 1 on drift or failure, `apply` and `merge` exit 1 on failure only, `validate` exits 1 on an invalid file, `permissions` exits 0 for a valid file.
119118
A flag the mode does not read is unknown to that subcommand and exits 1 with the parser's message, where the action rejects the same input by name;
120-
a value the mode refuses (a filter without `--repos "*"`, a `--repository` that is not a slug) fails with the action's own message.
119+
a value the mode refuses (a filter without `--repos "*"`, a `--repository` that is not a slug) fails with the action's own message, reworded where the action's remedy names the workflow step.
121120
The token passed on the command line is masked as `***` wherever a line would echo it.

‎src/cli/commands.ts‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
failRun,
1313
type GithubClient,
1414
type Io,
15+
type Problem,
1516
type RunConfig,
1617
readSettingsFile,
1718
runMerge,
@@ -117,12 +118,31 @@ export function permissionsFor(file: string, io: Io, bold: (text: string) => str
117118
);
118119
}
119120

120-
/** A command whose mode this build's library does not carry: one error line, exit 1. */
121-
export function unavailable(command: string, io: Io): number {
122-
io.annotate(
123-
"error",
124-
`${command} needs mode: snapshot, which this build of github-settings-as-code does not include. ` +
125-
"Upgrade to a release whose inputs reference lists snapshot among the modes",
126-
);
121+
/**
122+
* The action's wording for a problem, except where the remedy names the
123+
* workflow step: from a terminal the fix is a flag or an environment variable.
124+
*/
125+
export function describeCliProblem(problem: Problem): string {
126+
switch (problem.code) {
127+
case "input-token-missing":
128+
return "cannot call the GitHub API: no token was provided. Pass --token, or export GITHUB_TOKEN";
129+
case "input-repository-not-slug":
130+
return `cannot target a repository: "${problem.value}" is not an owner/name slug. Pass --repository owner/name (inside GitHub Actions, GITHUB_REPOSITORY supplies it)`;
131+
default:
132+
return describeProblem(problem);
133+
}
134+
}
135+
136+
/** The one report channel a terminal cannot serve: the artifact upload needs the Actions runner. */
137+
export const ARTIFACT_UNSUPPORTED =
138+
'"--private-report artifact" uploads through the Actions artifact service, which the command line has no access to. ' +
139+
'Use "--private-report issue" or "issue-on-failure" (a report on each private target repository), or "none"';
140+
141+
/** End a run on a problem the library has no code for, exactly as failRun ends one. */
142+
export function failCli(io: Io, message: string): number {
143+
io.annotate("error", message);
144+
io.output("skipped-sections", "");
145+
io.output("result", "failed");
146+
io.log("result: failed");
127147
return 1;
128148
}

‎src/cli/inputs.ts‎

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import { InvalidArgumentError, Option } from "commander";
1111
import {
1212
INPUT_DECLS,
13+
type InputDecl,
1314
type InputName,
1415
type InputReader,
1516
MERGE_INPUTS,
@@ -23,16 +24,23 @@ const INPUT_NAMES = Object.keys(INPUT_DECLS) as InputName[];
2324
/** The two inputs that are not subcommand flags: the mode is the subcommand, the token is global. */
2425
const PROGRAM_INPUTS = ["mode", "token"] as const satisfies readonly InputName[];
2526

27+
/**
28+
* Inputs no subcommand exposes: the artifact report channel needs the Actions
29+
* artifact service, which a terminal has no upload for, so its key has no use.
30+
*/
31+
export const CLI_UNSUPPORTED_INPUTS = ["report-public-key"] as const satisfies readonly InputName[];
32+
2633
/**
2734
* The flags a mode's subcommand takes: the inputs its mode reads. `settings-file`
2835
* is the one input both the merge and the engine modes read.
2936
*/
3037
export function inputsForMode(mode: Mode): InputName[] {
3138
const mergeReads = (name: InputName): boolean =>
3239
(MERGE_INPUTS as readonly InputName[]).includes(name);
40+
const hidden: readonly InputName[] = [...PROGRAM_INPUTS, ...CLI_UNSUPPORTED_INPUTS];
3341
return INPUT_NAMES.filter(
3442
(name) =>
35-
!(PROGRAM_INPUTS as readonly InputName[]).includes(name) &&
43+
!hidden.includes(name) &&
3644
(mode === "merge" ? mergeReads(name) : !mergeReads(name) || name === "settings-file"),
3745
);
3846
}
@@ -43,20 +51,29 @@ export function exposedInputs(): InputName[] {
4351
return INPUT_NAMES.filter((name) => flags.has(name));
4452
}
4553

46-
/**
47-
* The inputs parseConfig splits on newlines and commas (its splitList calls),
48-
* so a repeated flag may accumulate into one list. Pinned to parseConfig in
49-
* test/cli/inputs.test.ts: every name here accepts a newline-joined pair.
50-
*/
51-
export const LIST_INPUTS = [
52-
"settings-file",
53-
"required-sections",
54-
"sections",
55-
"repos",
56-
"exclude",
57-
"topics",
58-
"affiliation",
59-
] as const satisfies readonly InputName[];
54+
/** The sentence the action's `repository` description spends on a default a terminal never has. */
55+
const ACTIONS_DEFAULT_SENTENCE = "Defaults to the current repository.";
56+
const CLI_REPOSITORY_SENTENCE =
57+
"Required unless repos or repos-dir is set (inside GitHub Actions, GITHUB_REPOSITORY supplies it).";
58+
if (!INPUT_DECLS.repository.description.includes(ACTIONS_DEFAULT_SENTENCE)) {
59+
throw new Error(
60+
`BUG: the repository input's description no longer says "${ACTIONS_DEFAULT_SENTENCE}"; reword the CLI's replacement with it`,
61+
);
62+
}
63+
64+
/** The flag's help text: the declaration's, reworded where it assumes the Actions runner. */
65+
export function inputDescription(name: InputName): string {
66+
const description = INPUT_DECLS[name].description;
67+
return name === "repository"
68+
? description.replace(ACTIONS_DEFAULT_SENTENCE, CLI_REPOSITORY_SENTENCE)
69+
: description;
70+
}
71+
72+
/** Whether the declaration is a list; read through InputDecl since only the list members carry the field. */
73+
export function isList(name: InputName): boolean {
74+
const decl: InputDecl = INPUT_DECLS[name];
75+
return decl.list === true;
76+
}
6077

6178
/** A repeated list flag accumulates as a newline-separated list, the form parseConfig splits. */
6279
function accumulate(value: string, previous?: string): string {
@@ -73,10 +90,10 @@ export function once(flag: string): (value: string, previous?: string) => string
7390
};
7491
}
7592

76-
/** The commander option for one input: `--<name> <value>` with the action's description. */
93+
/** The commander option for one input: `--<name> <value>`, repeatable when the declaration is a list. */
7794
export function inputOption(name: InputName): Option {
78-
const parse = (LIST_INPUTS as readonly InputName[]).includes(name) ? accumulate : once(name);
79-
return new Option(`--${name} <value>`, INPUT_DECLS[name].description).argParser(parse);
95+
const parse = isList(name) ? accumulate : once(name);
96+
return new Option(`--${name} <value>`, inputDescription(name)).argParser(parse);
8097
}
8198

8299
/** Commander's attribute for each flag (camelCase of the name), read from commander itself. */

‎src/cli/io.ts‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,6 @@
11
/**
2-
* The CLI's output boundary and its Io. maskedStreams() wraps stdout and
3-
* stderr so every write, the parser's messages included, passes one mask
4-
* registry (no runner masks for a terminal); cliIo() is the Io the flows
5-
* write through over those streams: annotations and the debug trace on
6-
* stderr as `level: message`, log lines on stdout, the summary appended to a
7-
* file when one is named, and the outputs collected for flush() to print as
8-
* `name=value` lines or one JSON object.
2+
* The CLI's output boundary and its Io. No runner masks for a terminal, so
3+
* every writer, the parser included, goes through maskedStreams().
94
*/
105

116
import { appendFileSync } from "node:fs";
@@ -47,8 +42,12 @@ class RedactingStream extends Writable {
4742
}
4843

4944
override _write(chunk: unknown, _encoding: BufferEncoding, callback: () => void): void {
50-
this.target.write(this.redact(String(chunk)));
51-
callback();
45+
// A full target holds the next chunk until it drains, so backpressure reaches the writer.
46+
if (this.target.write(this.redact(String(chunk)))) {
47+
callback();
48+
} else {
49+
this.target.once("drain", callback);
50+
}
5251
}
5352
}
5453

0 commit comments

Comments
 (0)