Skip to content

Commit ec6d9ed

Browse files
committed
refactor: replace hand-rolled helpers with the library calls that already ship
zod issue paths render through z.core.toDotPath at the four sites that each spelled their own join, so a key with a non-word character reads as ["my-key"] instead of .my-key. The secret scanning pattern comparison uses isDeepStrictEqual from node:util instead of JSON.stringify equality. The two set-equality helpers use Set.prototype.isSubsetOf, and the tsconfig lib moves from ES2022 to ES2025 for its types while the target stays ES2022. The REST query string is built by URLSearchParams, which writes a space as + where encodeURIComponent wrote %20; GitHub decodes both. Central-mode file names go through extname and parse from node:path instead of a YAML extension regex. valueAt has one home in the shared list-section module, the two environments modules share one unreconcilable message builder, and the generator scripts call RegExp.escape instead of a local escapeRe.
1 parent dcb742f commit ec6d9ed

22 files changed

Lines changed: 85 additions & 119 deletions

File tree

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
import { SECTIONS } from "../../src/sections/registry.js";
2121
import { agree } from "../../src/text.js";
2222
import { countWord } from "./lib/count-word.js";
23-
import { escapeRe, type GeneratedRegion, regenerateRegions } from "./lib/generated-regions.js";
23+
import { type GeneratedRegion, regenerateRegions } from "./lib/generated-regions.js";
2424

2525
const ROOT = join(import.meta.dir, "..", "..");
2626

@@ -338,7 +338,7 @@ function blockShape(lines: string): RegExp {
338338
}
339339

340340
function tableShape(header: string, cells: string): RegExp {
341-
return blockShape(String.raw`${escapeRe(header)}\n(?:\| ${cells} \|\n)*`);
341+
return blockShape(String.raw`${RegExp.escape(header)}\n(?:\| ${cells} \|\n)*`);
342342
}
343343

344344
/** A JSON string literal as JSON.stringify() emits it: its own escapes only, bare quotes never. */
@@ -398,7 +398,7 @@ export const GENERATED_REGIONS: Readonly<Record<string, readonly GeneratedRegion
398398
{
399399
name: "policy-count-sentence",
400400
placement: { kind: "under-heading", heading: "# The undeclared policy" },
401-
body: blockShape(String.raw`[A-Z][a-z-]*${escapeRe(COUNT_SENTENCE_LEAD)}[^\n]+\.\n`),
401+
body: blockShape(String.raw`[A-Z][a-z-]*${RegExp.escape(COUNT_SENTENCE_LEAD)}[^\n]+\.\n`),
402402
render: block(() => renderPolicyCountSentence(knobbedSections())),
403403
},
404404
{
@@ -415,7 +415,7 @@ export const GENERATED_REGIONS: Readonly<Record<string, readonly GeneratedRegion
415415
{
416416
name: "permissions-grant-sentence",
417417
placement: { kind: "under-heading", heading: "## What to grant" },
418-
body: blockShape(String.raw`${escapeRe(GRANT_SENTENCE_LEAD)}[^\n]+\.\n`),
418+
body: blockShape(String.raw`${RegExp.escape(GRANT_SENTENCE_LEAD)}[^\n]+\.\n`),
419419
render: block(() => renderGrantSentence(SECTIONS)),
420420
},
421421
{
@@ -433,7 +433,7 @@ export const GENERATED_REGIONS: Readonly<Record<string, readonly GeneratedRegion
433433
heading: "## Checking settings changes on pull requests",
434434
},
435435
body: blockShape(
436-
String.raw`${escapeRe(NO_GATED_READS)}\n|${escapeRe(GATED_READS_LEAD_IN)}\n\n(?:${GATED_READ_BULLET})+`,
436+
String.raw`${RegExp.escape(NO_GATED_READS)}\n|${RegExp.escape(GATED_READS_LEAD_IN)}\n\n(?:${GATED_READ_BULLET})+`,
437437
),
438438
render: block(() => renderCheckModeGatedReads(SECTIONS)),
439439
},

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

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,7 @@ import type { UndeclaredPolicy } from "../../src/types.js";
1515
import { readArchitecture, renderArchitectureMermaid } from "./arch-lint.js";
1616
import { COVERAGE_DATA, type CoverageData } from "./coverage-data.js";
1717
import { ENDPOINT_ANCHORS, type EndpointAnchors } from "./endpoint-docs.js";
18-
import {
19-
escapeRe,
20-
type GeneratedRegion,
21-
regenerateRegions,
22-
regionBounds,
23-
} from "./lib/generated-regions.js";
18+
import { type GeneratedRegion, regenerateRegions, regionBounds } from "./lib/generated-regions.js";
2419

2520
const ROOT = join(import.meta.dir, "..", "..");
2621
export const COVERAGE_PATH = "docs/reference/coverage.md";
@@ -428,7 +423,7 @@ function sectionsTableRegion(name: string, heading: string): GeneratedRegion {
428423
name,
429424
placement: { kind: "under-heading", heading },
430425
body: new RegExp(
431-
String.raw`^\n(?:${escapeRe(TABLE_HEADER)}\n(?:\| \x60[a-z_]+\x60 \| [^\n]* \|\n)*)?$`,
426+
String.raw`^\n(?:${RegExp.escape(TABLE_HEADER)}\n(?:\| \x60[a-z_]+\x60 \| [^\n]* \|\n)*)?$`,
432427
),
433428
render: () => `\n${renderSectionsTable(SECTIONS, DOCS)}\n`,
434429
};
@@ -439,7 +434,7 @@ function outputsListRegion(name: string, heading: string): GeneratedRegion {
439434
name,
440435
placement: { kind: "under-heading", heading },
441436
body: new RegExp(
442-
String.raw`^(?:\x60[a-z]+\x60(?: / \x60[a-z]+\x60)*${escapeRe(RESULT_TAIL)})?$`,
437+
String.raw`^(?:\x60[a-z]+\x60(?: / \x60[a-z]+\x60)*${RegExp.escape(RESULT_TAIL)})?$`,
443438
),
444439
render: () => renderOutputsList(RUN_RESULTS),
445440
};
@@ -450,7 +445,7 @@ function patUrlRegion(name: string): GeneratedRegion {
450445
return {
451446
name,
452447
placement: { kind: "tail" },
453-
body: new RegExp(String.raw`^\n(?:\[${escapeRe(PAT_FORM_LABEL)}\]: \S+\n)?$`),
448+
body: new RegExp(String.raw`^\n(?:\[${RegExp.escape(PAT_FORM_LABEL)}\]: \S+\n)?$`),
454449
render: () => `\n[${PAT_FORM_LABEL}]: ${patFormUrl()}\n`,
455450
};
456451
}
@@ -505,10 +500,10 @@ const nonBlank = (excluded: string): string =>
505500
String.raw`[ \t]*[^${excluded}\s][^${excluded}\r\n]*`;
506501
const PROSE_LINE = `${nonBlank("")}\n`;
507502
const CELL = nonBlank("|");
508-
const KEY_CELL = String.raw`\[\x60[a-z_]+\x60\]\(${escapeRe(SECTIONS_PAGE)}\)(?: \(\x60${nonBlank("|\x60")}\x60\))?`;
503+
const KEY_CELL = String.raw`\[\x60[a-z_]+\x60\]\(${RegExp.escape(SECTIONS_PAGE)}\)(?: \(\x60${nonBlank("|\x60")}\x60\))?`;
509504
const SUPPORTED_ROWS = String.raw`(?:\| ${CELL} \| ${KEY_CELL} \| ${CELL} \|\n)+`;
510505
const GAP_ROWS = String.raw`(?:\| ${CELL} \| ${CELL} \| ${CELL} \|\n)+`;
511-
const GAPS_BODY = String.raw`(?:${PROSE_LINE}\n${escapeRe(GAPS_HEADER)}\n|${escapeRe(GAPS_HEADER)}\n${GAP_ROWS})`;
506+
const GAPS_BODY = String.raw`(?:${PROSE_LINE}\n${RegExp.escape(GAPS_HEADER)}\n|${RegExp.escape(GAPS_HEADER)}\n${GAP_ROWS})`;
512507
const BULLETS = `(?:- ${PROSE_LINE})+`;
513508
const NOTE_GROUPS = String.raw`(?:\*\*${nonBlank("*")}\*\* \(\x60[a-z_]+\x60\)\n\n${BULLETS}\n)+`;
514509

@@ -519,10 +514,10 @@ const COVERAGE_REGIONS: readonly GeneratedRegion[] = [
519514
name: "coverage",
520515
placement: { kind: "tail" },
521516
body: new RegExp(
522-
String.raw`^\n(?:(?:${PROSE_LINE}\n)+${escapeRe(SUPPORTED_HEADING)}\n\n${escapeRe(SUPPORTED_HEADER)}\n` +
523-
String.raw`${SUPPORTED_ROWS}\n${escapeRe(NOTES_HEADING)}\n\n${NOTE_GROUPS}` +
524-
String.raw`${escapeRe(GAPS_HEADING)}\n\n${GAPS_BODY}\n${escapeRe(NO_API_HEADING)}\n\n` +
525-
String.raw`${PROSE_LINE}\n${BULLETS}\n${escapeRe(OUT_OF_SCOPE_HEADING)}\n\n${BULLETS})?$`,
517+
String.raw`^\n(?:(?:${PROSE_LINE}\n)+${RegExp.escape(SUPPORTED_HEADING)}\n\n${RegExp.escape(SUPPORTED_HEADER)}\n` +
518+
String.raw`${SUPPORTED_ROWS}\n${RegExp.escape(NOTES_HEADING)}\n\n${NOTE_GROUPS}` +
519+
String.raw`${RegExp.escape(GAPS_HEADING)}\n\n${GAPS_BODY}\n${RegExp.escape(NO_API_HEADING)}\n\n` +
520+
String.raw`${PROSE_LINE}\n${BULLETS}\n${RegExp.escape(OUT_OF_SCOPE_HEADING)}\n\n${BULLETS})?$`,
526521
),
527522
render: () => `\n${renderCoverage(SECTIONS, DOCS, COVERAGE_DATA, ENDPOINT_ANCHORS)}\n`,
528523
},

‎.github/scripts/lib/generated-regions.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,6 @@ import { Parser } from "yaml";
88
/** Which comment syntax a file's markers use: a complete `<!-- -->` comment, or a whole-line YAML `#` comment. */
99
export type MarkerSyntax = "html" | "yaml";
1010

11-
/** `text` as a regex source matching itself literally; body shapes splice renderer constants through it. */
12-
export function escapeRe(text: string): string {
13-
return text.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
14-
}
15-
1611
/** Character offsets of one marker: `[start, end)`. */
1712
export type MarkerSpan = readonly [start: number, end: number];
1813

‎src/discovery/central.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@
44
*/
55

66
import { existsSync, readdirSync, statSync } from "node:fs";
7-
import { join } from "node:path";
7+
import { extname, join, parse } from "node:path";
88
import { err, ok, type Result } from "neverthrow";
99
import { type SlugKey, slugKey } from "../github/slug.js";
1010
import type { CentralFileProblem, ProblemOf } from "../problem.js";
1111
import { type CentralTarget, SLUG_RE } from "./targets.js";
1212

13-
const YAML_EXT = /\.ya?ml$/;
13+
const YAML_EXTENSIONS = new Set([".yml", ".yaml"]);
1414

1515
export function resolveCentralTargets(
1616
reposDir: string,
@@ -52,13 +52,13 @@ export function resolveCentralTargets(
5252
);
5353
continue;
5454
}
55-
if (!YAML_EXT.test(inner)) {
55+
if (!YAML_EXTENSIONS.has(extname(inner))) {
5656
warnings.push(
5757
`ignoring ${innerPath}: not a .yml/.yaml file, so it defines no target repository`,
5858
);
5959
continue;
6060
}
61-
addTarget(`${owner}/${inner.replace(YAML_EXT, "")}`, innerPath);
61+
addTarget(`${owner}/${parse(inner).name}`, innerPath);
6262
}
6363
};
6464

@@ -71,7 +71,7 @@ export function resolveCentralTargets(
7171
scanOwnerDir(entryPath, entry);
7272
continue;
7373
}
74-
if (!YAML_EXT.test(entry)) {
74+
if (!YAML_EXTENSIONS.has(extname(entry))) {
7575
warnings.push(
7676
`ignoring ${entryPath}: not a .yml/.yaml file, so it defines no target repository`,
7777
);
@@ -81,7 +81,7 @@ export function resolveCentralTargets(
8181
ownerlessFiles.push(entryPath);
8282
continue;
8383
}
84-
addTarget(`${adminOwner}/${entry.replace(YAML_EXT, "")}`, entryPath);
84+
addTarget(`${adminOwner}/${parse(entry).name}`, entryPath);
8585
}
8686
if (ownerlessFiles.length > 0) {
8787
errors.push({ kind: "ownerless", files: ownerlessFiles });

‎src/engine/validate.ts‎

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/** Shape validation against each section's loose zod shape; the parsed output, not the input, is what the engine applies. */
22

33
import { err, ok, type Result } from "neverthrow";
4+
import { z } from "zod";
45
import { nonPlainKind } from "../plain-data.js";
56
import type { ProblemOf } from "../problem.js";
67
import { LIST_SECTIONS, type ListSection, SECTION_KEYS, type SettingsFile } from "../schema.js";
@@ -10,6 +11,7 @@ import {
1011
type DeclaredSecretValue,
1112
} from "../sections/contract/module.js";
1213
import { listLayering, sectionModule, sectionShape } from "../sections/registry.js";
14+
import { valueAt } from "../sections/shared/list-section.js";
1315
import { agree, countNoun } from "../text.js";
1416
import { type SettingsSource, validateSecretRef } from "./secret-refs.js";
1517

@@ -19,18 +21,6 @@ function isListSection(key: string): key is ListSection {
1921
return LIST_KEYS.has(key);
2022
}
2123

22-
/** The value at an issue's path, so a null the author wrote can be told from a type the author got wrong. */
23-
function valueAt(root: unknown, path: readonly PropertyKey[]): unknown {
24-
let node: unknown = root;
25-
for (const step of path) {
26-
if (typeof node !== "object" || node === null) {
27-
return undefined;
28-
}
29-
node = (node as Record<PropertyKey, unknown>)[step];
30-
}
31-
return node;
32-
}
33-
3424
/** zod's issue fields this module reads; `legal` is this action's own, set where a shape refuses null itself. */
3525
interface NullIssue {
3626
code: string;
@@ -230,18 +220,14 @@ export function validateSectionShapes(
230220
if (!parsed.success) {
231221
const issues = parsed.error.issues;
232222
for (const issue of issues.slice(0, 5)) {
233-
const path = issue.path
234-
.map((p) => (typeof p === "number" ? `[${p}]` : `.${String(p)}`))
235-
.join("");
223+
const path = z.core.toDotPath([key, ...issue.path]);
236224
// A null the shape refused is the author saying "empty" where GitHub has no empty state: name the values that
237225
// exist. A shape's own diagnostic (a custom issue) already names the fix, unless it supplies the legal values itself.
238226
if (valueAt(declared, issue.path) === null && rewritesForNull(issue as NullIssue)) {
239-
problems.push(
240-
`${key}${path} has no empty state; write ${legalValues(issue as NullIssue)}`,
241-
);
227+
problems.push(`${path} has no empty state; write ${legalValues(issue as NullIssue)}`);
242228
continue;
243229
}
244-
problems.push(`${key}${path}: ${issue.message}`);
230+
problems.push(`${path}: ${issue.message}`);
245231
}
246232
if (issues.length > 5) {
247233
// A silently truncated list costs one fix-and-rerun cycle per hidden offender.

‎src/sections/contract/endpoints.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -243,10 +243,7 @@ export function expand(
243243
);
244244
}
245245
if (query && Object.keys(query).length > 0) {
246-
const qs = Object.entries(query)
247-
.map(([key, value]) => `${encodeURIComponent(key)}=${encodeURIComponent(value)}`)
248-
.join("&");
249-
return `${path}?${qs}`;
246+
return `${path}?${new URLSearchParams(query)}`;
250247
}
251248
return path;
252249
}

‎src/sections/contract/live.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*/
66

77
import { err, ok, type Result } from "neverthrow";
8-
import type { z } from "zod";
8+
import { z } from "zod";
99
import { countNoun } from "../../text.js";
1010
import { endpointMethod, endpointPath } from "./endpoints.js";
1111
import type { SectionFailure } from "./errors.js";
@@ -85,10 +85,7 @@ export function parseLive<T>(
8585
}
8686
const issues = parsed.error.issues;
8787
const shown = issues.slice(0, 3).map((issue) => {
88-
const path = issue.path
89-
.map((part) => (typeof part === "number" ? `[${part}]` : `.${String(part)}`))
90-
.join("");
91-
return `${path.replace(/^\./, "") || "(body)"}: ${issue.message}`;
88+
return `${z.core.toDotPath(issue.path) || "(body)"}: ${issue.message}`;
9289
});
9390
const more =
9491
issues.length > 3 ? `; and ${countNoun(issues.length - 3, "more issue", "more issues")}` : "";

‎src/sections/contract/permissions.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,7 @@ export function samePermission(
3535
}
3636
const resources = new Set(a.repo);
3737
const others = new Set(b.repo);
38-
return (
39-
a.org === b.org &&
40-
resources.size === others.size &&
41-
[...resources].every((resource) => others.has(resource))
42-
);
38+
return a.org === b.org && resources.size === others.size && resources.isSubsetOf(others);
4339
}
4440

4541
/** Human-facing label for each PAT resource, as shown in the token UI. */

‎src/sections/custom_properties/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ function sameValue(a: WireValue, b: WireValue): boolean {
5454
if (Array.isArray(a) && Array.isArray(b)) {
5555
const setA = new Set(a);
5656
const setB = new Set(b);
57-
return setA.size === setB.size && [...setA].every((element) => setB.has(element));
57+
return setA.size === setB.size && setA.isSubsetOf(setB);
5858
}
5959
return a === b;
6060
}

‎src/sections/environments/branch-policies.ts‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { err, ok, Result, safeTry } from "neverthrow";
77
import { z } from "zod";
88
import { subsetDiff } from "../../engine/diff.js";
99
import type { UndeclaredPolicy } from "../../types.js";
10-
import { type SectionFailure, sectionFailure } from "../contract/errors.js";
10+
import type { SectionFailure } from "../contract/errors.js";
1111
import { liveByIdentity, liveIdentity } from "../contract/live.js";
1212
import {
1313
type DeclaredIssue,
@@ -18,7 +18,11 @@ import {
1818
undeclaredNote,
1919
} from "../contract/module.js";
2020
import { hasDrift, plainData, type Read } from "../contract/plan.js";
21-
import type { EnvironmentRestOp, EnvironmentsRestContext } from "./endpoints.js";
21+
import {
22+
type EnvironmentRestOp,
23+
type EnvironmentsRestContext,
24+
unreconcilable,
25+
} from "./endpoints.js";
2226
import type { LiveEnvironmentBody } from "./index.js";
2327
import type { NestedPlan } from "./nested.js";
2428
import type { DeploymentBranchPolicyConfig } from "./schema.js";
@@ -46,23 +50,18 @@ function livePolicyType(policy: LiveBranchPolicy): string {
4650
return typeof policy.type === "string" ? policy.type : "branch";
4751
}
4852

49-
function unreconcilable(envName: string, what: string): SectionFailure {
50-
return sectionFailure(
51-
"live-shape",
52-
`environments: the deployment branch-policy list for environment "${envName}" returned a policy without ${what}, so it cannot be reconciled. Check the "api-version" input against the GitHub REST docs for this endpoint`,
53-
);
54-
}
53+
const POLICY = { list: "deployment branch-policy", entry: "policy" };
5554

5655
function livePolicyId(policy: LiveBranchPolicy, envName: string): Result<string, SectionFailure> {
5756
if (policy.id === undefined) {
58-
return err(unreconcilable(envName, "an id"));
57+
return err(unreconcilable(POLICY, envName, "an id"));
5958
}
6059
return ok(String(policy.id));
6160
}
6261

6362
function livePolicyName(policy: LiveBranchPolicy, envName: string): Result<string, SectionFailure> {
6463
if (typeof policy.name !== "string") {
65-
return err(unreconcilable(envName, "a name"));
64+
return err(unreconcilable(POLICY, envName, "a name"));
6665
}
6766
return ok(policy.name);
6867
}

0 commit comments

Comments
 (0)