Skip to content

Commit 2d9abc7

Browse files
VivswanVivswan
committed
fix(cli): word CLI problems through failRun, pin the whole merged document, assert the full refusal line
failRun takes a describe parameter defaulting to describeProblem, so the command line hands it describeCliProblem instead of re-implementing the failure tail; the artifact channel's refusal is an input-unsupported-value problem whose allowed list is derived from the channels. The merge test compares the whole merged document the fold writes, and the single-value refusal test asserts the exact stderr line.
1 parent 017e2c0 commit 2d9abc7

5 files changed

Lines changed: 67 additions & 40 deletions

File tree

‎src/cli/commands.ts‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ import {
1111
describeProblem,
1212
failRun,
1313
type GithubClient,
14+
INPUT_DECLS,
1415
type Io,
16+
PRIVATE_REPORT_CHANNELS,
1517
type Problem,
1618
type RunConfig,
1719
readSettingsFile,
@@ -133,16 +135,15 @@ export function describeCliProblem(problem: Problem): string {
133135
}
134136
}
135137

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");
147-
return 1;
148-
}
138+
/**
139+
* The one report channel a terminal cannot serve: the artifact upload needs
140+
* the Actions runner. Worded as the unsupported value it is from here.
141+
*/
142+
export const ARTIFACT_REFUSED: Problem = {
143+
code: "input-unsupported-value",
144+
input: "private-report",
145+
value: "artifact",
146+
noun: "private-report channel from the command line (the artifact upload needs the Actions runner)",
147+
allowed: PRIVATE_REPORT_CHANNELS.filter((channel) => channel !== "artifact"),
148+
fallback: INPUT_DECLS["private-report"].default,
149+
};

‎src/cli/program.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import { Command, CommanderError, Option } from "commander";
99
import pc from "picocolors";
1010
import {
11+
failRun,
1112
GithubApi,
1213
INPUT_DECLS,
1314
type Io,
@@ -16,10 +17,9 @@ import {
1617
type RunConfig,
1718
} from "../index.js";
1819
import {
19-
ARTIFACT_UNSUPPORTED,
20+
ARTIFACT_REFUSED,
2021
type CliHost,
2122
describeCliProblem,
22-
failCli,
2323
permissionsFor,
2424
type Rendered,
2525
runConfig,
@@ -145,10 +145,10 @@ export function buildProgram(options: ProgramOptions): {
145145
// Refused before parseConfig, which would otherwise ask for the channel's age key first.
146146
exitCode =
147147
read("private-report") === "artifact"
148-
? failCli(io, ARTIFACT_UNSUPPORTED)
148+
? failRun(io, ARTIFACT_REFUSED, describeCliProblem)
149149
: await parseConfig(read, host.env).match(
150150
(cfg) => execute(cfg, io),
151-
async (problem) => failCli(io, describeCliProblem(problem)),
151+
async (problem) => failRun(io, problem, describeCliProblem),
152152
);
153153
flush();
154154
});

‎src/flows/deliver.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,16 @@ export function concludeRun(io: Io, run: FinishedRun): number {
214214
return conclude(io, views, run.mode === "check");
215215
}
216216

217-
/** A run that failed before any target ran gets a failed target's conclusion and no summary; the one place a fatal problem becomes text. */
218-
export function failRun(io: Io, problem: Problem): number {
219-
const message = describeProblem(problem);
217+
/**
218+
* A run that failed before any target ran gets a failed target's conclusion and no summary; the one place a fatal problem becomes text.
219+
* `describe` is the action's wording unless the caller's face (the command line) words a remedy differently.
220+
*/
221+
export function failRun(
222+
io: Io,
223+
problem: Problem,
224+
describe: (problem: Problem) => string = describeProblem,
225+
): number {
226+
const message = describe(problem);
220227
io.annotate("error", message);
221228
// The mode may be unknown here (a config error); a failure exits 1 under either.
222229
return conclude(io, [failedTarget(message)], false);

‎test/cli/inputs.test.ts‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
import { beforeAll, describe, expect, test } from "bun:test";
1212
import { generateX25519Identity, identityToRecipient } from "age-encryption";
13-
import { ARTIFACT_UNSUPPORTED, describeCliProblem } from "../../src/cli/commands.js";
13+
import { ARTIFACT_REFUSED, describeCliProblem } from "../../src/cli/commands.js";
1414
import {
1515
CLI_UNSUPPORTED_INPUTS,
1616
exposedInputs,
@@ -161,7 +161,7 @@ function cases(): Case[] {
161161
"private-report": "artifact",
162162
},
163163
env: { GITHUB_REPOSITORY: "o/admin" },
164-
cliRefuses: ARTIFACT_UNSUPPORTED,
164+
cliRefuses: describeCliProblem(ARTIFACT_REFUSED),
165165
},
166166
{
167167
name: "merge, layers as repeated flags",
@@ -458,20 +458,22 @@ describe("the per-mode flag split", () => {
458458
if (!exposed.has(name)) {
459459
continue; // the mode is the subcommand; an unsupported input has no flag
460460
}
461-
const flag =
462-
name === "token"
463-
? ["check", "--repository", "o/r"]
464-
: inputsForMode("check").includes(name)
465-
? check
466-
: ["merge"];
461+
// The base argv minus this flag, so the two repeats below are its only occurrences.
462+
const base = inputsForMode("check").includes(name) || name === "token" ? check : ["merge"];
463+
const flag = base.filter((argument, index) => {
464+
const value = index > 0 && base[index - 1] === `--${name}`;
465+
return argument !== `--${name}` && !value;
466+
});
467467
// Long values: a token value is masked, and a one-letter mask would eat the message.
468468
const actual = await throughArgv(
469469
[...flag, `--${name}`, "first-value", `--${name}`, "second-value"],
470470
{},
471471
);
472+
// The token's echoed value is masked (registered before the parse); the rest print as given.
473+
const echoed = name === "token" ? "***" : "second-value";
472474
expect(actual.code, name).toBe(1);
473-
expect(actual.stderr, name).toContain(
474-
`--${name} takes one value and was given more than once`,
475+
expect(actual.stderr, name).toBe(
476+
`error: option '--${name} <value>' argument '${echoed}' is invalid. --${name} takes one value and was given more than once\n`,
475477
);
476478
continue;
477479
}

‎test/cli/program.test.ts‎

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ describe("check and apply", () => {
142142
[
143143
"the artifact channel",
144144
["check", ...target, "--private-report", "artifact"],
145-
'"--private-report artifact" uploads through the Actions artifact service, which the command line has no access to. Use "--private-report issue" or "issue-on-failure" (a report on each private target repository), or "none"',
145+
'the "private-report" input is "artifact", which is not a supported private-report channel from the command line (the artifact upload needs the Actions runner). Set it to "none" (default), "issue", "issue-on-failure"',
146146
],
147147
])("%s fails with a remedy a terminal can follow", async (_case, args, message) => {
148148
const api = new MockApi({});
@@ -228,15 +228,32 @@ describe("merge", () => {
228228
]);
229229
expect(result.code).toBe(0);
230230
expect(result.stdout).toEndWith("result: merged\nskipped-sections=\nresult=merged\n");
231-
// Only the fold produces this shape: team.yml's label joins fleet.yml's two under
232-
// the explicit policy wrapper, and its `has_projects: null` removes fleet.yml's key.
233-
const merged = parseYaml(readFileSync(out, "utf8")) as {
234-
repository: Record<string, unknown>;
235-
labels: { _undeclared: string; entries: Array<{ name: string }> };
236-
};
237-
expect(merged.repository).toEqual({ has_wiki: false });
238-
expect(merged.labels._undeclared).toBe("delete");
239-
expect(merged.labels.entries.map((label) => label.name)).toEqual(["bug", "docs", "team"]);
231+
// The whole document only the fold produces: team.yml's label and rule join
232+
// fleet.yml's under explicit policy wrappers, its `has_projects: null` removes
233+
// fleet.yml's key, and pages passes through untouched.
234+
expect(parseYaml(readFileSync(out, "utf8"))).toEqual({
235+
repository: { has_wiki: false },
236+
labels: {
237+
_undeclared: "delete",
238+
entries: [
239+
{ name: "bug", color: "d73a4a" },
240+
{ name: "docs", color: "0075ca" },
241+
{ name: "team", color: "00ff00" },
242+
],
243+
},
244+
rulesets: {
245+
_undeclared: "keep",
246+
entries: [
247+
{
248+
name: "main",
249+
target: "branch",
250+
enforcement: "active",
251+
rules: [{ type: "deletion" }, { type: "non_fast_forward" }],
252+
},
253+
],
254+
},
255+
pages: { build_type: "workflow", source: { branch: "main", path: "/" } },
256+
});
240257
});
241258

242259
test("a merge without merged-file fails with the action's line", async () => {

0 commit comments

Comments
 (0)