Skip to content

Commit aafafa5

Browse files
committed
fix(showcase): validate verify-matrix boundaries (SSOT + summary shape), fail loud, test CLI contract
A 7-agent review of the verify-matrix resolver and its surrounding workflow plumbing found three boundary surfaces that could silently produce a GREEN deploy on a broken release, plus an untested CLI contract that CI compares against the literal strings 'true' / 'false'. FIX 1 — Validate the SSOT shape in loadSsotServices(). The prior `JSON.parse(...) as {services: SsotService[]}` was an unchecked cast: a truncated/drifted SSOT (emitter crashed mid-write, or schema renamed) parses fine but silently shrinks/empties the probe-eligible set → some redeployed services go unverified, or verify is skipped on a real redeploy. Extract a pure exported parseSsotServices(raw, path) that requires the shape we depend on (non-empty services array; each entry has a non-empty string name, an optional string|null dispatchName, and a probe object with a boolean staging). Throw `::error::SSOT <path> malformed: <detail>` on any violation. Also re-check existsSync(SSOT_JSON) after the regenerate-if-missing execFileSync — a regen that exits 0 without writing must not proceed to a useless JSON.parse crash. Drop the defensive `probe?.staging` once shape is guaranteed. FIX 2 — Validate summary.json shape in the redeploy-gate bash. The bullseye false-green surface: if redeploy-env.ts's schema ever drifts (e.g. `status` → `state`, `ok` → `success`), every `jq select(.status==...)` yields empty → redeploy_red=false AND ok_services="" → resolver skips verify → GREEN CI on a real unverified redeploy. Add a TOTAL vs WITH_STATUS shape guard right after loading the summary: if TOTAL > 0 && WITH_STATUS == 0, emit ::error::summary.json has $TOTAL entries but none with status ok|error (schema drift?) and exit 1. The legitimate empty-array path (TOTAL=0) is preserved. FIX 3 — Fail loud on unknown eventName in resolveVerifyMatrix. The prior code fell through to the workflow_run intersection branch for ANY unrecognized eventName (typo, unexpected trigger), silently emitting has_services=false → indistinguishable from a legit "summary absent" skip. Add an explicit guard so only workflow_run / workflow_dispatch are accepted; anything else throws ::error::resolve-verify-matrix: unexpected eventName '<value>'. Tighten the eventName parameter type to the literal union. FIX 4 — Trim ok tokens + warn on dropped tokens in okCsvToCanonicalNames. Split, then .map(t => t.trim()).filter(Boolean) so "a, b" (spaces) matches. Collect tokens that match NO SSOT service (by name or dispatchName) and have the CLI wrapper emit ::warning::ok_services tokens dropped (no SSOT match): <list> on stderr when non-empty — surfaces SSOT/build drift. The pure function stays IO-free; logging lives in the wrapper. FIX 5 — CLI wrapper integration test. New resolve-verify-matrix.cli.test.ts spawns `npx tsx showcase/scripts/resolve-verify-matrix.ts` with a temp $GITHUB_OUTPUT file across four scenarios and asserts the temp file contents EXACTLY (the workflow YAML compares has_services against the literal strings 'true'/'false', so the byte-for-byte format is part of the contract). Uses the real railway-envs.generated.json so the loader exercise is real. FIX 6 — Cleanup. Remove the dead `env: DISPATCH_SERVICE: ...` block on the redeploy-gate step (the next step redeclares it — leftover from the extraction). Soften the §3 decision-table all-errors bullet to match resolve-verify-matrix.ts's careful wording, and append that when the success-set is empty (or the intersection collapses to empty), verify is skipped and the gate reds independently. Append to showcase_build.yml's "Upload redeploy summary" path-(A) comment that `if-no-files-found: error` still reds path (A) even if a future change adds `if: always()`. Tests: red→green for FIX 1/3/4/5 verified locally. Resolve-verify-matrix vitest count: 12 → 28. Full requested suite (resolve-verify-matrix + cli + aggregate-build-results + lint-rule-no-public-env): 72 passed. showcase/bin ruby specs: 87 runs / 0 failures / 0 errors / 0 skips. actionlint baseline preserved (8 findings, identical to integration tip).
1 parent c579ad7 commit aafafa5

5 files changed

Lines changed: 550 additions & 27 deletions

File tree

.github/workflows/showcase_build.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,13 @@ jobs:
602602
# redeployed", skip the gate, and ship a false-green.
603603
# The legitimate "services == '' → nothing redeployed → no upload"
604604
# path is preserved by the services != '' guard.
605+
#
606+
# If a future change ever switches this step to `if: always()`,
607+
# `if-no-files-found: error` STILL reds path (A): the redeploy
608+
# step's non-zero exit on a HARD crash is independent of upload
609+
# gating, and `if-no-files-found: error` on `always()` then trips
610+
# because summary.json was never written. So neither relaxation
611+
# alone opens a false-green window.
605612
if: steps.changed.outputs.services != ''
606613
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
607614
with:

.github/workflows/showcase_deploy.yml

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,6 @@ jobs:
139139

140140
- name: Gate on staging redeploy errors
141141
id: redeploy-gate
142-
env:
143-
DISPATCH_SERVICE: ${{ github.event.inputs.service }}
144142
run: |
145143
set -euo pipefail
146144
SUMMARY=".redeploy/summary.json"
@@ -150,6 +148,21 @@ jobs:
150148
echo "ok_services=" >> "$GITHUB_OUTPUT"
151149
exit 0
152150
fi
151+
# Shape guard: redeploy-env.ts writes per-entry `status` of
152+
# exactly "ok" or "error". If the schema ever drifts (e.g.
153+
# `status`→`state`, `ok`→`success`) every `select(.status==...)`
154+
# silently yields empty → redeploy_red=false AND ok_services=""
155+
# → resolve-verify-matrix skips verify on a real unverified
156+
# redeploy = green CI on a broken release. Refuse the ambiguity:
157+
# if the file has entries but none carry an ok|error status,
158+
# fail loud here so enforce-redeploy-gate reds the workflow
159+
# (resolve-matrix.result == 'failure' fans into the gate).
160+
TOTAL=$(jq 'length' "$SUMMARY")
161+
WITH_STATUS=$(jq '[.[] | select(.status == "ok" or .status == "error")] | length' "$SUMMARY")
162+
if [ "$TOTAL" -gt 0 ] && [ "$WITH_STATUS" -eq 0 ]; then
163+
echo "::error::summary.json has $TOTAL entries but none with status ok|error (schema drift?)"
164+
exit 1
165+
fi
153166
# Per spec §3: the workflow MUST turn red on any staging
154167
# status:"error", while verify still runs against the success-set.
155168
ERRORS=$(jq -c '[.[] | select(.status == "error")]' "$SUMMARY")
@@ -180,14 +193,19 @@ jobs:
180193
# - workflow_run + summary_present=false → has_services=false
181194
# (build redeployed nothing).
182195
# - workflow_run + summary_present=true + ok empty
183-
# → has_services=false. Every service errored on redeploy;
184-
# the success-set is empty so there is nothing to verify.
185-
# `enforce-redeploy-gate` independently reds the workflow
186-
# on redeploy_red=true, so this case is already loud.
196+
# → has_services=false (success-set empty; verify is skipped).
197+
# In practice redeploy-env.ts only emits status ok|error,
198+
# so this branch implies redeploy_red=true and
199+
# enforce-redeploy-gate reds the workflow independently —
200+
# skipping verify here is correct (no ok services left to
201+
# probe; the gate has already turned the workflow red).
187202
# - workflow_run + summary_present=true + ok non-empty
188203
# → intersect ok_services (SSOT key OR dispatchName aliases)
189204
# with probe.staging-eligible SSOT services. has_services
190-
# reflects CSV emptiness.
205+
# reflects CSV emptiness. When the intersection collapses
206+
# to empty (e.g. every ok service is non-probe-eligible),
207+
# verify is skipped and `enforce-redeploy-gate` reds the
208+
# workflow independently on redeploy_red=true.
191209
run: npx tsx showcase/scripts/resolve-verify-matrix.ts
192210

193211
verify:
Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
/**
2+
* resolve-verify-matrix.cli.test.ts — pins the byte-for-byte $GITHUB_OUTPUT
3+
* contract that showcase_deploy.yml depends on.
4+
*
5+
* The deploy workflow's `verify:` job has
6+
* if: needs.resolve-matrix.outputs.has_services == 'true'
7+
* which compares against the LITERAL strings 'true'/'false' (everything in
8+
* $GITHUB_OUTPUT is a string). A regression that writes `has_services=1`
9+
* or `has_services=True` would silently skip verify on every redeploy.
10+
* Pin that contract here so the byte-for-byte format is part of the test
11+
* surface, not just the pure-function unit tests.
12+
*
13+
* The pure resolver is covered by resolve-verify-matrix.test.ts. THIS
14+
* file spawns the CLI as a child process against the real
15+
* railway-envs.generated.json so the loader + parseSsotServices +
16+
* writeGithubOutput contract gets end-to-end coverage.
17+
*
18+
* Cases (mirrors decision-table boundaries the workflow YAML depends on):
19+
* 1. workflow_run + SUMMARY_PRESENT=true + OK_FROM_REDEPLOY=""
20+
* → services_csv=\nhas_services=false\n [Issue A pinned end-to-end]
21+
* 2. workflow_run + SUMMARY_PRESENT=true + OK_FROM_REDEPLOY="<two real names>"
22+
* → services_csv=<sorted CSV>\nhas_services=true\n
23+
* 3. workflow_dispatch + no summary → full probe-eligible set, has_services=true.
24+
* 4. workflow_dispatch + service=<unknown> → non-zero exit, stderr ::error::Unknown service.
25+
*/
26+
import { execFileSync } from "node:child_process";
27+
import { mkdtempSync, readFileSync, writeFileSync } from "node:fs";
28+
import { tmpdir } from "node:os";
29+
import { dirname, join, resolve } from "node:path";
30+
import { fileURLToPath } from "node:url";
31+
import { describe, expect, it } from "vitest";
32+
33+
const __filename = fileURLToPath(import.meta.url);
34+
const __dirname = dirname(__filename);
35+
36+
// The repo root is the cwd the workflow YAML uses for the script (paths
37+
// inside the script are written as `showcase/scripts/...`, relative to
38+
// repo root). Tests therefore must spawn from repo root.
39+
const REPO_ROOT = resolve(__dirname, "..", "..", "..");
40+
const SCRIPT = "showcase/scripts/resolve-verify-matrix.ts";
41+
const SSOT_PATH = join(REPO_ROOT, "showcase/scripts/railway-envs.generated.json");
42+
43+
interface SpawnResult {
44+
status: number;
45+
stderr: string;
46+
output: string;
47+
}
48+
49+
function runCli(env: Record<string, string>): SpawnResult {
50+
const dir = mkdtempSync(join(tmpdir(), "rvm-cli-"));
51+
const outputPath = join(dir, "gh_output");
52+
writeFileSync(outputPath, "");
53+
const fullEnv = {
54+
...process.env,
55+
GITHUB_OUTPUT: outputPath,
56+
...env,
57+
};
58+
try {
59+
execFileSync("npx", ["tsx", SCRIPT], {
60+
cwd: REPO_ROOT,
61+
env: fullEnv,
62+
stdio: ["ignore", "pipe", "pipe"],
63+
});
64+
return {
65+
status: 0,
66+
stderr: "",
67+
output: readFileSync(outputPath, "utf-8"),
68+
};
69+
} catch (e) {
70+
// execFileSync throws on non-zero exit; the thrown error carries
71+
// `.status` and `.stderr` (Buffer when stdio captures).
72+
const err = e as {
73+
status?: number;
74+
stderr?: Buffer | string;
75+
};
76+
return {
77+
status: typeof err.status === "number" ? err.status : 1,
78+
stderr:
79+
typeof err.stderr === "string"
80+
? err.stderr
81+
: err.stderr
82+
? err.stderr.toString("utf-8")
83+
: "",
84+
output: readFileSync(outputPath, "utf-8"),
85+
};
86+
}
87+
}
88+
89+
// Pull two real probe-eligible names off the actual SSOT so the test
90+
// stays in lock-step with the emitter (rather than hard-coding fixtures
91+
// that could drift).
92+
function realProbeEligibleNames(): string[] {
93+
const raw = JSON.parse(readFileSync(SSOT_PATH, "utf-8")) as {
94+
services: { name: string; probe: { staging: boolean } }[];
95+
};
96+
return raw.services
97+
.filter((s) => s.probe.staging === true)
98+
.map((s) => s.name)
99+
.sort();
100+
}
101+
102+
describe("resolve-verify-matrix CLI (end-to-end against real SSOT)", () => {
103+
// The CLI spawns `npx tsx`. Cold-start can take >1s; bump timeout.
104+
it(
105+
"workflow_run + summary_present + empty ok_services → has_services=false (Issue A end-to-end)",
106+
() => {
107+
const r = runCli({
108+
EVENT_NAME: "workflow_run",
109+
SUMMARY_PRESENT: "true",
110+
OK_FROM_REDEPLOY: "",
111+
DISPATCH_SERVICE: "",
112+
});
113+
expect(r.status).toBe(0);
114+
// Byte-for-byte: the workflow YAML compares against the literal
115+
// strings 'true' / 'false'. Match the EXACT bytes (including the
116+
// trailing newlines and key=value form).
117+
expect(r.output).toBe("services_csv=\nhas_services=false\n");
118+
},
119+
30_000,
120+
);
121+
122+
it(
123+
"workflow_run + summary_present + two real ok_services → sorted CSV + has_services=true",
124+
() => {
125+
const probe = realProbeEligibleNames();
126+
// Pick TWO probe-eligible names. Use a CSV order that is NOT
127+
// already sorted so the sort-on-intersection assertion is real.
128+
// probe[0] sorts before probe[1] (alphabetical); feed reversed.
129+
const picked = [probe[0], probe[1]];
130+
const reversedCsv = `${picked[1]},${picked[0]}`;
131+
const sortedCsv = picked.join(",");
132+
const r = runCli({
133+
EVENT_NAME: "workflow_run",
134+
SUMMARY_PRESENT: "true",
135+
OK_FROM_REDEPLOY: reversedCsv,
136+
DISPATCH_SERVICE: "",
137+
});
138+
expect(r.status).toBe(0);
139+
expect(r.output).toBe(
140+
`services_csv=${sortedCsv}\nhas_services=true\n`,
141+
);
142+
},
143+
30_000,
144+
);
145+
146+
it(
147+
"workflow_dispatch + no summary → full probe-eligible set + has_services=true",
148+
() => {
149+
const probe = realProbeEligibleNames();
150+
const r = runCli({
151+
EVENT_NAME: "workflow_dispatch",
152+
SUMMARY_PRESENT: "",
153+
OK_FROM_REDEPLOY: "",
154+
DISPATCH_SERVICE: "",
155+
});
156+
expect(r.status).toBe(0);
157+
expect(r.output).toBe(
158+
`services_csv=${probe.join(",")}\nhas_services=true\n`,
159+
);
160+
},
161+
30_000,
162+
);
163+
164+
it(
165+
"workflow_dispatch + unknown service → non-zero exit with ::error::Unknown service",
166+
() => {
167+
const r = runCli({
168+
EVENT_NAME: "workflow_dispatch",
169+
SUMMARY_PRESENT: "",
170+
OK_FROM_REDEPLOY: "",
171+
DISPATCH_SERVICE: "totally-not-a-real-service",
172+
});
173+
expect(r.status).not.toBe(0);
174+
expect(r.stderr).toMatch(
175+
/::error::Unknown service 'totally-not-a-real-service'/,
176+
);
177+
},
178+
30_000,
179+
);
180+
});

0 commit comments

Comments
 (0)