Skip to content

Commit ce4b2d0

Browse files
committed
fix(snapshot): gate folds - rejection wording, exact result enumeration, generated outputs list, artifact note
The snapshot rejection's fix names the apply, check, or merge step an input belongs to (merged-file and layering are refused by apply and check too). The action.yml result-output test asserts the enumerated values as an exact set, so a value dropped from the enumeration while still named in prose fails. The outputs-list generated region on the inputs reference now carries the snapshot results through renderOutputsList, replacing the hand-kept list beside it. The snapshot guide's fleet example says the uploaded directory holds redacted targets' full documents in the clear and points at the redaction guide's artifact-visibility note.
1 parent 65da6a2 commit ce4b2d0

8 files changed

Lines changed: 30 additions & 19 deletions

File tree

‎.github/scripts/gen-docs.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { readFileSync, writeFileSync } from "node:fs";
22
import { join } from "node:path";
33
import { REPO_RESULTS, type RepoResult } from "../../src/engine/orchestrate.js";
44
import { MERGE_RESULT } from "../../src/flows/deliver.js";
5+
import { SNAPSHOT_RESULTS } from "../../src/flows/snapshot.js";
56
import type { SectionDocs } from "../../src/sections/contract/docs.js";
67
import {
78
type SectionMeta,
@@ -239,8 +240,12 @@ const RESULT_DISPLAY: Record<RepoResult, "any mode" | "multi-repo only"> = {
239240

240241
const WORST_OF = "; worst-of across targets in multi-repo mode";
241242
const CAN_ALSO_APPEAR = " can also appear";
242-
/** The merge-mode result, a value outside RepoResult, closes the enumeration. */
243+
/** The merge- and snapshot-mode results, values outside RepoResult, close the enumeration. */
243244
const MERGE_ONLY = `; \`${MERGE_RESULT}\` in mode: merge`;
245+
const SNAPSHOT_ONLY = `; ${[...SNAPSHOT_RESULTS]
246+
.reverse()
247+
.map((value) => `\`${value}\``)
248+
.join(" / ")} in mode: snapshot`;
244249

245250
export function renderOutputsList(results: readonly RepoResult[]): string {
246251
const ordered = (Object.keys(RESULT_DISPLAY) as RepoResult[]).filter((value) =>
@@ -254,7 +259,7 @@ export function renderOutputsList(results: readonly RepoResult[]): string {
254259
const lead = `${anyMode.join(" / ")}${WORST_OF}`;
255260
const withMulti =
256261
multiOnly.length === 0 ? lead : `${lead}, where ${multiOnly.join(" and ")}${CAN_ALSO_APPEAR}`;
257-
return `${withMulti}${MERGE_ONLY}`;
262+
return `${withMulti}${MERGE_ONLY}${SNAPSHOT_ONLY}`;
258263
}
259264

260265
export type TaggedOperation = Pick<SectionOperation, "role" | "grade" | "permission"> & {
@@ -337,7 +342,7 @@ function outputsListRegion(name: string, heading: string): GeneratedRegion {
337342
body: new RegExp(
338343
String.raw`^(?:\x60[a-z]+\x60(?: / \x60[a-z]+\x60)*${escapeRe(WORST_OF)}(?:, where ` +
339344
String.raw`\x60[a-z]+\x60(?: and ` +
340-
String.raw`\x60[a-z]+\x60)*${escapeRe(CAN_ALSO_APPEAR)})?(?:${escapeRe(MERGE_ONLY)})?)?$`,
345+
String.raw`\x60[a-z]+\x60)*${escapeRe(CAN_ALSO_APPEAR)})?(?:${escapeRe(MERGE_ONLY)})?(?:${escapeRe(SNAPSHOT_ONLY)})?)?$`,
341346
),
342347
render: () => renderOutputsList(REPO_RESULTS),
343348
};

‎docs/operate/snapshot.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ jobs:
156156

157157
Copy the directory in as your `repos-dir` to bring the fleet under management, one reviewable file per repository. The step summary lists every target with its result and file, then one section table per target, and the `repos-result` output carries the per-repository results as JSON.
158158

159-
Private and internal targets are redacted by default, exactly as in a multi-repo apply (see [private repositories](private-repositories.md)): their file is written like the others, but the public surfaces know them only as `private repository #N`, with the file name and every detail hidden. Their notes are in their file header; there is no report channel in snapshot mode.
159+
Private and internal targets are redacted by default, exactly as in a multi-repo apply (see [private repositories](private-repositories.md)): their file is written like the others, but the public surfaces know them only as `private repository #N`, with the file name and every detail hidden. Their notes are in their file header; there is no report channel in snapshot mode. The uploaded `snapshots` artifact therefore holds those targets' full documents in the clear, and an artifact inherits the admin repository's visibility, so on a public admin repository encrypt it or skip the upload (see [what redaction does and does not protect](private-repositories.md#what-redaction-does-and-does-not-protect)).
160160

161161
## Inputs in mode: snapshot
162162

‎docs/reference/inputs.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ The discovery-only inputs apply to `repos: "*"`; the [multi-repo guide](../opera
4141

4242
## Outputs
4343

44-
- `result`: <!-- BEGIN GENERATED: outputs-list (bun run build:docs; derived from REPO_RESULTS in src/engine/orchestrate.ts) -->`applied` / `partial` / `clean` / `drift` / `failed`; worst-of across targets in multi-repo mode, where `skipped` can also appear; `merged` in mode: merge<!-- END GENERATED: outputs-list -->. In `mode: snapshot` it reads `snapshot`, `partial`, or `failed`; the [snapshot guide](../operate/snapshot.md) has that table.
44+
- `result`: <!-- BEGIN GENERATED: outputs-list (bun run build:docs; derived from REPO_RESULTS in src/engine/orchestrate.ts) -->`applied` / `partial` / `clean` / `drift` / `failed`; worst-of across targets in multi-repo mode, where `skipped` can also appear; `merged` in mode: merge; `snapshot` / `partial` / `failed` in mode: snapshot<!-- END GENERATED: outputs-list -->. The [snapshot guide](../operate/snapshot.md) has the snapshot values' exit-code table.
4545
- `skipped-sections`: the sections skipped for missing permissions under `on-missing-permission: warn`, comma-separated (a deduped union across targets in multi-repo mode).
4646
- `repos-result`: multi-repo mode only, a JSON map of `owner/name` to `{result, source, skippedSections}`. A redacted private target is keyed by its `private repository #N` placeholder instead of its slug; see [Private repositories](../operate/private-repositories.md).
4747

‎src/problem.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ export function describeProblem(problem: Problem): string {
413413
`the ${quoteList(problem.inputs)} input(s) do not apply to mode: snapshot, which only reads the ` +
414414
"target repositories' live settings into snapshot-file or snapshot-dir: it applies no " +
415415
"document, folds no layers, and delivers no report. Remove the input(s), or move them to " +
416-
"the apply or check step that runs the snapshot"
416+
"the apply, check, or merge step they belong to"
417417
);
418418
case "input-snapshot-destination-missing":
419419
return 'mode: snapshot needs exactly one of the "snapshot-file" input (one repository\'s settings written to that file) or the "snapshot-dir" input (one <owner>/<name>.yml per repos or repos-dir target under that directory). Set one of them';

‎test/action/action-yml.test.ts‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,20 @@ describe("input declarations <-> discovery defaults", () => {
5656
});
5757

5858
describe("output declarations", () => {
59-
test("the result description mentions every RepoResult value, the merge result, and the snapshot results", () => {
60-
// MERGE_RESULT and SNAPSHOT_RESULTS are the values outside REPO_RESULTS (neither mode has a target).
61-
const missing = [...REPO_RESULTS, MERGE_RESULT, ...SNAPSHOT_RESULTS].filter(
62-
(value) => !OUTPUT_DECLS.result.description.includes(value),
59+
test("the result description enumerates exactly the RepoResult values, the merge result, and the snapshot results", () => {
60+
// The enumerated values are the `a | b | c` chains; a value named only in prose ("in mode: snapshot") does not count.
61+
const { description } = OUTPUT_DECLS.result;
62+
const enumerated = (description.match(/[a-z]+(?: \| [a-z]+)+/g) ?? []).flatMap((chain) =>
63+
chain.split(" | "),
64+
);
65+
expect(new Set(enumerated)).toEqual(
66+
new Set([...REPO_RESULTS.filter((value) => value !== "skipped"), ...SNAPSHOT_RESULTS]),
67+
);
68+
expect(description).toContain("where skipped can also appear");
69+
expect(description).toContain(`${MERGE_RESULT} in mode: merge`);
70+
// SNAPSHOT_RESULTS is worst-first; the description reads best-first like the RepoResult chain.
71+
expect(description).toContain(
72+
`${[...SNAPSHOT_RESULTS].reverse().join(" | ")} in mode: snapshot`,
6373
);
64-
expect(
65-
missing,
66-
`the "result" output description omits result value(s): ${missing.join(", ")}`,
67-
).toEqual([]);
6874
});
6975
});

‎test/action/run.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -871,7 +871,7 @@ describe("run in mode: snapshot", () => {
871871
'error: the "settings-file" input(s) do not apply to mode: snapshot, which only reads the ' +
872872
"target repositories' live settings into snapshot-file or snapshot-dir: it applies no " +
873873
"document, folds no layers, and delivers no report. Remove the input(s), or move them to " +
874-
"the apply or check step that runs the snapshot",
874+
"the apply, check, or merge step they belong to",
875875
"result: failed",
876876
]);
877877
});

‎test/problem.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ const SPECIMENS = {
100100
'the "settings-file", "layering" input(s) do not apply to mode: snapshot, which only reads the ' +
101101
"target repositories' live settings into snapshot-file or snapshot-dir: it applies no " +
102102
"document, folds no layers, and delivers no report. Remove the input(s), or move them to " +
103-
"the apply or check step that runs the snapshot",
103+
"the apply, check, or merge step they belong to",
104104
],
105105
"input-snapshot-destination-missing": [
106106
{ code: "input-snapshot-destination-missing" },

‎test/scripts/gen-docs.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -280,12 +280,12 @@ describe("renderPatCell", () => {
280280
});
281281

282282
describe("renderOutputsList", () => {
283-
test("lists the any-mode values in display order, then the multi-repo-only ones, then the merge result", () => {
283+
test("lists the any-mode values in display order, then the multi-repo-only ones, then the merge and snapshot results", () => {
284284
expect(renderOutputsList(REPO_RESULTS)).toBe(
285-
"`applied` / `partial` / `clean` / `drift` / `failed`; worst-of across targets in multi-repo mode, where `skipped` can also appear; `merged` in mode: merge",
285+
"`applied` / `partial` / `clean` / `drift` / `failed`; worst-of across targets in multi-repo mode, where `skipped` can also appear; `merged` in mode: merge; `snapshot` / `partial` / `failed` in mode: snapshot",
286286
);
287287
expect(renderOutputsList(["failed", "applied"])).toBe(
288-
"`applied` / `failed`; worst-of across targets in multi-repo mode; `merged` in mode: merge",
288+
"`applied` / `failed`; worst-of across targets in multi-repo mode; `merged` in mode: merge; `snapshot` / `partial` / `failed` in mode: snapshot",
289289
);
290290
});
291291
});

0 commit comments

Comments
 (0)