mirror of
https://github.com/Sea-Haven-Industries/shoc-frontend-new.git
synced 2026-09-30 04:33:11 +00:00
fix(ci): classify G13 per queued PR on merge_group
Run live isolation on the merge-queue event so ci-complete actually gates mixed change sets, without treating the group union as one PR.
This commit is contained in:
parent
ec9812ae8d
commit
cc7ecb9b5a
8 changed files with 207 additions and 56 deletions
3
.github/workflows/ci.yaml
vendored
3
.github/workflows/ci.yaml
vendored
|
|
@ -86,7 +86,8 @@ jobs:
|
|||
# Repo-owned gates: godfile ratchet, changed-file maintainability,
|
||||
# Terraform fmt/validate, import-plan guard, HCP run guard, CloudFront
|
||||
# verify, GitHub workflow shell, isolation classifier tests, and live G13
|
||||
# (pull_request and local only). Format, lint, build, and unit tests run
|
||||
# (pull_request, merge_group per queued PR, and local). Format, lint, build,
|
||||
# and unit tests run
|
||||
# in the parallel jobs above, not here.
|
||||
#
|
||||
# GOVERNANCE_BASE points the changed-file gate at the right diff:
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ isolation). A task is not done until this is green.
|
|||
## Gate matrix
|
||||
|
||||
| Gate | Command / rule source | Enforced by | Scope |
|
||||
| ----------------------------------- | ----------------------------------------------------------------------------------------------------------- | ---------------------- | ------------------------------------------------------------------ |
|
||||
| ----------------------------------- | ----------------------------------------------------------------------------------------------------------------------------- | ---------------------- | ------------------------------------------------------------------- |
|
||||
| Formatting | `npm run format:check` (Prettier) | `verify` + lint-staged | Whole repo |
|
||||
| Lint, zero warnings | `npm run lint` → `eslint . --max-warnings=0` | `verify` + CI | Governed TS/TSX (`eslint.config.js`) |
|
||||
| Type-check + production build | `npm run build` → `tsc -b && vite build` | `verify` + CI | Whole app |
|
||||
|
|
@ -31,7 +31,7 @@ isolation). A task is not done until this is green.
|
|||
| HCP run guard | `npm run test:hcp-run-guard` → `scripts/test-hcp-run-guard.py` | `governance` + CI | Workspace invariants + apply reconcile |
|
||||
| CloudFront release verify | `npm run test:cloudfront-release-verify` → `scripts/test-verify-cloudfront-release.sh` | `governance` + CI | Stubbed aws/curl |
|
||||
| GitHub workflow shell | `npm run test:github-workflows` → `scripts/check-github-workflows.sh` | `governance` + CI | `bash -n` + actionlint |
|
||||
| G13 App/Terraform isolation | `python3 scripts/check_app_terraform_isolation.py` vs merge-base of `GOVERNANCE_BASE` | `governance` + CI | Deployable app files vs `terraform/` (live classifier: PR + local) |
|
||||
| G13 App/Terraform isolation | `python3 scripts/check_app_terraform_isolation.py` vs merge-base of `GOVERNANCE_BASE` (merge_group: each first-parent commit) | `governance` + CI | Deployable app files vs `terraform/` (live: PR, merge_group, local) |
|
||||
|
||||
## No-false-pass guarantees
|
||||
|
||||
|
|
@ -66,9 +66,11 @@ isolation). A task is not done until this is green.
|
|||
(Playwright visual), and `governance` (`npm run governance`, with Terraform
|
||||
1.16.0) run in parallel. `ci-complete` fails unless all of those jobs
|
||||
succeeded and is the required merge-queue check. Live G13 runs on
|
||||
`pull_request` and locally; it skips `merge_group` and `push`. Terraform
|
||||
fmt/validate and the related unit tests run inside `governance` on every
|
||||
event.
|
||||
`pull_request` (merge-base range), `merge_group` (each queued PR as a
|
||||
first-parent commit), and locally. It skips `push`. A Terraform-only PR
|
||||
stacked with an app-only PR still passes; a mixed change set still fails.
|
||||
Terraform fmt/validate and the related unit tests run inside `governance` on
|
||||
every event.
|
||||
|
||||
## Toolchain pin
|
||||
|
||||
|
|
|
|||
|
|
@ -136,9 +136,10 @@ commitlint enforces conventional commit messages. Run `npx tsc --noEmit` (or
|
|||
approvals. PRs merge through the merge queue, so a branch does not need to
|
||||
be updated with `main` before it merges. Merged branches are deleted
|
||||
automatically.
|
||||
- PRs cannot mix `terraform/` with deployable application files (G13). Workflow,
|
||||
docs, and gate-script changes may travel with either side. `deploy-web.yaml`
|
||||
still ignores `terraform/**` so a Terraform-only merge does not sync the bucket.
|
||||
- A change set cannot mix `terraform/` with deployable application files (G13),
|
||||
including each queued PR on the merge-group check. Workflow, docs, and
|
||||
gate-script changes may travel with either side. `deploy-web.yaml` still
|
||||
ignores `terraform/**` so a Terraform-only merge does not sync the bucket.
|
||||
- Promotion flow: merge to `main` deploys `dev.seahaven.com`. A person cuts
|
||||
`vX.Y.Z-staging` for `staging.seahaven.com`. Core `vX.Y.Z` waits until a
|
||||
prod distribution exists.
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@
|
|||
"test:hcp-run-guard": "python3 scripts/test-hcp-run-guard.py",
|
||||
"test:cloudfront-release-verify": "bash scripts/test-verify-cloudfront-release.sh",
|
||||
"test:github-workflows": "bash scripts/check-github-workflows.sh",
|
||||
"test:app-terraform-isolation": "python3 scripts/test_check_app_terraform_isolation.py",
|
||||
"test:app-terraform-isolation": "python3 scripts/test_check_app_terraform_isolation.py && node --test scripts/test-g13-live-isolation.mjs",
|
||||
"lint": "eslint . --max-warnings=0",
|
||||
"lint:fix": "eslint . --fix --max-warnings=0",
|
||||
"format": "prettier --write .",
|
||||
|
|
|
|||
29
scripts/g13-live-isolation.mjs
Normal file
29
scripts/g13-live-isolation.mjs
Normal file
|
|
@ -0,0 +1,29 @@
|
|||
/**
|
||||
* Live G13 scheduling and how a GitHub event is split into change sets.
|
||||
*
|
||||
* Isolation is a per-change rule. pull_request and local runs classify the
|
||||
* merge-base...HEAD range (one PR). merge_group classifies each first-parent
|
||||
* commit vs its parent (one queued PR per squash or merge-commit). The group
|
||||
* union is not a change set: a Terraform-only PR stacked with an app-only PR
|
||||
* must still pass. push is not classified; landing already happened.
|
||||
*/
|
||||
|
||||
export function shouldRunLiveIsolation(eventName) {
|
||||
return !eventName || eventName === "pull_request" || eventName === "merge_group";
|
||||
}
|
||||
|
||||
export function usesPerCommitIsolation(eventName) {
|
||||
return eventName === "merge_group";
|
||||
}
|
||||
|
||||
/**
|
||||
* File lists to run through check_app_terraform_isolation.py.
|
||||
* `commitDiffs` is first-parent order (oldest first); ignored except on
|
||||
* merge_group.
|
||||
*/
|
||||
export function liveIsolationFileSets(eventName, rangeFiles, commitDiffs) {
|
||||
if (usesPerCommitIsolation(eventName)) {
|
||||
return commitDiffs.map((commit) => commit.files);
|
||||
}
|
||||
return [rangeFiles];
|
||||
}
|
||||
|
|
@ -3,6 +3,8 @@ import { existsSync, readFileSync } from "node:fs";
|
|||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
import { shouldRunLiveIsolation, usesPerCommitIsolation } from "./g13-live-isolation.mjs";
|
||||
|
||||
const SCRIPT_DIR = path.dirname(fileURLToPath(import.meta.url));
|
||||
const ROOT = path.resolve(SCRIPT_DIR, "..");
|
||||
const BASELINE_PATH = path.join(SCRIPT_DIR, "governance-baseline.json");
|
||||
|
|
@ -218,23 +220,77 @@ function runRepositoryGate(label, script) {
|
|||
return { label, status: result.status, error: result.error };
|
||||
}
|
||||
|
||||
function shouldRunLiveIsolation() {
|
||||
const eventName = process.env.GITHUB_EVENT_NAME;
|
||||
return !eventName || eventName === "pull_request";
|
||||
function classifyIsolationPaths(files) {
|
||||
return spawnSync("python3", ["scripts/check_app_terraform_isolation.py"], {
|
||||
cwd: ROOT,
|
||||
encoding: "utf8",
|
||||
input: files.length > 0 ? `${files.join("\n")}\n` : "",
|
||||
});
|
||||
}
|
||||
|
||||
function runIsolationGate(baseRef) {
|
||||
function firstParentCommitDiffs(baseRef) {
|
||||
const shas = gitLines(["rev-list", "--reverse", "--first-parent", `${baseRef}..HEAD`]);
|
||||
return shas.map((sha) => ({
|
||||
sha,
|
||||
files: gitLines(["diff", "--name-only", "--diff-filter=ACMR", `${sha}^`, sha]),
|
||||
}));
|
||||
}
|
||||
|
||||
function runIsolationGate(baseRef, eventName) {
|
||||
if (usesPerCommitIsolation(eventName)) {
|
||||
const commits = firstParentCommitDiffs(baseRef);
|
||||
return {
|
||||
mode: "per-commit",
|
||||
results: commits.map((commit) => ({
|
||||
...classifyIsolationPaths(commit.files),
|
||||
sha: commit.sha,
|
||||
})),
|
||||
};
|
||||
}
|
||||
// Diff from the merge base, not the moving base tip. A two-dot diff against
|
||||
// a branch that has advanced reports everything the base gained after the
|
||||
// branch point as if this change reverted it.
|
||||
const mergeBase = gitText(["merge-base", baseRef, "HEAD"]);
|
||||
const files = gitLines(["diff", "--name-only", "--diff-filter=ACMR", mergeBase, "HEAD"]);
|
||||
const result = spawnSync("python3", ["scripts/check_app_terraform_isolation.py"], {
|
||||
cwd: ROOT,
|
||||
encoding: "utf8",
|
||||
input: `${files.join("\n")}\n`,
|
||||
});
|
||||
return { ...result, mergeBase };
|
||||
return {
|
||||
mode: "range",
|
||||
mergeBase,
|
||||
results: [{ ...classifyIsolationPaths(files), sha: null }],
|
||||
};
|
||||
}
|
||||
|
||||
function recordIsolationFailures(isolation, failures) {
|
||||
const startError = isolation.results.find((result) => result.error);
|
||||
if (startError) {
|
||||
failures.push(`G13: could not start: ${startError.error.message}`);
|
||||
}
|
||||
if (isolation.results.some((result) => !result.error && result.status !== 0)) {
|
||||
failures.push("G13: do not mix deployable application files with terraform/");
|
||||
}
|
||||
}
|
||||
|
||||
function logIsolationGate(baseRef, isolation) {
|
||||
if (isolation.mode === "per-commit") {
|
||||
console.log(
|
||||
`G13: application and Terraform isolation (merge_group, ${plural(isolation.results.length, "queued PR")} vs ${baseRef.slice(0, 7)})`,
|
||||
);
|
||||
for (const result of isolation.results) {
|
||||
const output = `${result.stdout ?? ""}${result.stderr ?? ""}`.trim();
|
||||
const prefix = result.sha ? result.sha.slice(0, 7) : "commit";
|
||||
if (output) {
|
||||
console.log(` ${prefix}: ${output.replaceAll("\n", "\n ")}`);
|
||||
}
|
||||
}
|
||||
return;
|
||||
}
|
||||
const mergeBase = isolation.mergeBase ?? "unresolvable";
|
||||
console.log(
|
||||
`G13: application and Terraform isolation (${baseRef}...HEAD, merge base ${mergeBase.slice(0, 7)})`,
|
||||
);
|
||||
const result = isolation.results[0];
|
||||
if (!result) return;
|
||||
const output = `${result.stdout ?? ""}${result.stderr ?? ""}`.trim();
|
||||
if (output) console.log(` ${output.replaceAll("\n", "\n ")}`);
|
||||
}
|
||||
|
||||
function main() {
|
||||
|
|
@ -336,9 +392,10 @@ function main() {
|
|||
}
|
||||
|
||||
console.log("─".repeat(64));
|
||||
if (!shouldRunLiveIsolation()) {
|
||||
const eventName = process.env.GITHUB_EVENT_NAME;
|
||||
if (!shouldRunLiveIsolation(eventName)) {
|
||||
console.log(
|
||||
`G13: skipped — live isolation is a pull-request property (event: ${process.env.GITHUB_EVENT_NAME})`,
|
||||
`G13: skipped — live isolation runs on pull_request, merge_group, and local (event: ${eventName})`,
|
||||
);
|
||||
} else if (!baseRef) {
|
||||
console.log("G13: application and Terraform isolation (no base...HEAD)");
|
||||
|
|
@ -349,28 +406,16 @@ function main() {
|
|||
} else {
|
||||
let isolation;
|
||||
try {
|
||||
isolation = runIsolationGate(baseRef);
|
||||
isolation = runIsolationGate(baseRef, eventName);
|
||||
} catch (error) {
|
||||
console.log(`G13: application and Terraform isolation (${baseRef}...HEAD)`);
|
||||
console.log(" FAIL (could not resolve merge base)");
|
||||
console.log(" FAIL (could not resolve isolation diffs)");
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
failures.push(`G13: could not resolve merge base against HEAD: ${message}`);
|
||||
failures.push(`G13: could not resolve isolation diffs against HEAD: ${message}`);
|
||||
}
|
||||
if (isolation) {
|
||||
const mergeBase = isolation.mergeBase ?? "unresolvable";
|
||||
console.log(
|
||||
`G13: application and Terraform isolation (${baseRef}...HEAD, merge base ${mergeBase.slice(0, 7)})`,
|
||||
);
|
||||
if (isolation.error) {
|
||||
console.log(" FAIL (could not start)");
|
||||
failures.push(`G13: could not start: ${isolation.error.message}`);
|
||||
} else {
|
||||
const output = `${isolation.stdout ?? ""}${isolation.stderr ?? ""}`.trim();
|
||||
if (output) console.log(` ${output.replaceAll("\n", "\n ")}`);
|
||||
if (isolation.status !== 0) {
|
||||
failures.push("G13: do not mix deployable application files with terraform/");
|
||||
}
|
||||
}
|
||||
logIsolationGate(baseRef, isolation);
|
||||
recordIsolationFailures(isolation, failures);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
73
scripts/test-g13-live-isolation.mjs
Normal file
73
scripts/test-g13-live-isolation.mjs
Normal file
|
|
@ -0,0 +1,73 @@
|
|||
import assert from "node:assert/strict";
|
||||
import { spawnSync } from "node:child_process";
|
||||
import path from "node:path";
|
||||
import { describe, it } from "node:test";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
import {
|
||||
liveIsolationFileSets,
|
||||
shouldRunLiveIsolation,
|
||||
usesPerCommitIsolation,
|
||||
} from "./g13-live-isolation.mjs";
|
||||
|
||||
const SCRIPT_DIR = path.dirname(fileURLToPath(import.meta.url));
|
||||
const CLASSIFIER = path.join(SCRIPT_DIR, "check_app_terraform_isolation.py");
|
||||
const TERRAFORM_ONLY = ["terraform/live/dev/main.tf"];
|
||||
const APP_ONLY = ["src/app/routes.tsx"];
|
||||
|
||||
function classify(files) {
|
||||
const result = spawnSync("python3", [CLASSIFIER, ...files], { encoding: "utf8" });
|
||||
return result.status;
|
||||
}
|
||||
|
||||
function anySetFails(fileSets) {
|
||||
return fileSets.some((files) => classify(files) !== 0);
|
||||
}
|
||||
|
||||
describe("shouldRunLiveIsolation", () => {
|
||||
it("runs locally and on pull_request", () => {
|
||||
assert.equal(shouldRunLiveIsolation(undefined), true);
|
||||
assert.equal(shouldRunLiveIsolation(""), true);
|
||||
assert.equal(shouldRunLiveIsolation("pull_request"), true);
|
||||
});
|
||||
|
||||
it("runs on merge_group so the required check classifies the candidate", () => {
|
||||
assert.equal(shouldRunLiveIsolation("merge_group"), true);
|
||||
assert.equal(usesPerCommitIsolation("merge_group"), true);
|
||||
});
|
||||
|
||||
it("skips push and other CI events", () => {
|
||||
assert.equal(shouldRunLiveIsolation("push"), false);
|
||||
assert.equal(shouldRunLiveIsolation("workflow_dispatch"), false);
|
||||
assert.equal(usesPerCommitIsolation("pull_request"), false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("liveIsolationFileSets", () => {
|
||||
it("classifies the PR range as one change set", () => {
|
||||
const range = [...TERRAFORM_ONLY, ...APP_ONLY];
|
||||
const sets = liveIsolationFileSets("pull_request", range, [
|
||||
{ files: TERRAFORM_ONLY },
|
||||
{ files: APP_ONLY },
|
||||
]);
|
||||
assert.deepEqual(sets, [range]);
|
||||
assert.equal(anySetFails(sets), true);
|
||||
});
|
||||
|
||||
it("classifies each queued PR, not the merge-group union", () => {
|
||||
const union = [...TERRAFORM_ONLY, ...APP_ONLY];
|
||||
const sets = liveIsolationFileSets("merge_group", union, [
|
||||
{ files: TERRAFORM_ONLY },
|
||||
{ files: APP_ONLY },
|
||||
]);
|
||||
assert.deepEqual(sets, [TERRAFORM_ONLY, APP_ONLY]);
|
||||
assert.equal(anySetFails(sets), false);
|
||||
assert.equal(classify(union), 1);
|
||||
});
|
||||
|
||||
it("fails a single queued PR that mixes terraform and app files", () => {
|
||||
const mixed = [...TERRAFORM_ONLY, ...APP_ONLY];
|
||||
const sets = liveIsolationFileSets("merge_group", mixed, [{ files: mixed }]);
|
||||
assert.equal(anySetFails(sets), true);
|
||||
});
|
||||
});
|
||||
|
|
@ -57,7 +57,7 @@ bash scripts/test-verify-cloudfront-release.sh
|
|||
PRs cannot mix `terraform/` with deployable application files. Workflow, docs,
|
||||
and gate-script changes may travel with either side. G13 is
|
||||
`python3 scripts/check_app_terraform_isolation.py` against the merge base of
|
||||
the PR.
|
||||
the PR, and against each queued PR (first-parent commit) on `merge_group`.
|
||||
|
||||
`npm run test:terraform` and `npm run verify` wrap the same gates. They never
|
||||
create an HCP run or touch AWS.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue