Skip to content
Merged
25 changes: 21 additions & 4 deletions .github/workflows/enforce-pr-target.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@ on:
- edited
- ready_for_review
- synchronize
# A maintainer issue comment ("not touching gui") waives the GUI-screenshot
# gate. `pull_request_target` types do not include issue comments, so a
# separate `issue_comment` trigger re-runs the gate the moment the waiver is
# posted. The gate is idempotent — it re-reads the live PR and updates the
# single consolidated comment — so a comment cannot race or double-mutate.
issue_comment:
types:
- created
- edited
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
Wibias marked this conversation as resolved.
Comment thread
Wibias marked this conversation as resolved.

# pull-requests:write covers title/comment/label updates.
# contents:write is required for convertPullRequestToDraft /
Expand All @@ -19,7 +28,8 @@ permissions:
pull-requests: write

concurrency:
group: enforce-pr-target-${{ github.event.pull_request.number }}
# `issue_comment` events carry the issue number, not the PR number.
group: enforce-pr-target-${{ github.event.pull_request.number || github.event.issue.number }}

jobs:
enforce-target:
Expand All @@ -33,8 +43,10 @@ jobs:
# runs this workflow from the base revision, and the scripts must come
# from the same revision or a merged gate would run against the
# pre-promotion scripts on `main`. The immutable SHA pins the checkout
# to the exact base commit the event was built against.
ref: ${{ github.event.pull_request.base.sha }}
# to the exact base commit the event was built against. On an
# `issue_comment` event there is no PR payload, so the checkout falls
# back to the repository default branch — the trusted gate source.
ref: ${{ github.event.pull_request.base.sha || github.event.repository.default_branch }}
Comment thread
Wibias marked this conversation as resolved.
Outdated
persist-credentials: false
sparse-checkout: |
.github/scripts
Expand Down Expand Up @@ -115,7 +127,12 @@ jobs:
const MAINTAINERS_FILE = "MAINTAINERS.md";

const { owner, repo } = context.repo;
const pull_number = context.payload.pull_request.number;
// `issue_comment` events carry the PR's issue object, not a
// `pull_request` object. The issue number is the PR number either
// way, so resolve it from whichever payload the event delivered.
const pull_number =
context.payload.pull_request?.number ??
context.payload.issue?.number;
Comment thread
Wibias marked this conversation as resolved.

const { data: pr } = await github.rest.pulls.get({
owner,
Expand Down
8 changes: 5 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,18 @@ A ready-for-review PR is the author's claim that the change is complete, underst
## Pre-push hook

After cloning, run once to install a local pre-push hook that runs the typecheck,
GUI eslint, unit-test, privacy-scan, and (when `gui/` changed) React Doctor
unit-test, privacy-scan, and (when `gui/` changed) GUI eslint and React Doctor
portions of the CI gate:

```sh
bun run setup:hooks
```

This installs a `pre-push` hook (into the hooks dir git reports, so worktrees and
`core.hooksPath` work) that runs `bun run prepush` — `typecheck`, `lint:gui`,
`test`, `privacy:scan`, and `doctor:gui:if-changed` — before every `git push`.
`core.hooksPath` work) that runs `bun run prepush` — `typecheck`,
`lint:gui:if-changed`, `test`, `privacy:scan`, and `doctor:gui:if-changed` —
before every `git push`. Both `lint:gui:if-changed` and `doctor:gui:if-changed`
run their check only when the push touches `gui/`.
The same checks run on ubuntu-latest, macos-latest, and windows-latest in CI (CI
additionally builds the GUI and smoke-tests the CLI). Skip in an emergency with
`git push --no-verify`.
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@
"prepublishOnly": "bun run audit:high && bun run typecheck && bun run build:gui",
"release": "bun scripts/release.ts",
"release:watch": "bun scripts/release.ts watch",
"prepush": "bun run typecheck && bun run lint:gui && bun run test && bun run privacy:scan && bun run doctor:gui:if-changed",
"prepush": "bun run typecheck && bun run lint:gui:if-changed && bun run test && bun run privacy:scan && bun run doctor:gui:if-changed",
"lint:gui": "cd gui && bun run lint",
"lint:gui:if-changed": "bun scripts/lint-gui-if-changed.ts",
"doctor:gui": "cd gui && bun run doctor",
"doctor:gui:full": "cd gui && bun run doctor:full",
"doctor:gui:if-changed": "bun scripts/doctor-gui-if-changed.ts",
Expand Down
3 changes: 3 additions & 0 deletions scripts/fixtures/lint-findings-exit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Simulate eslint finding violations: non-zero exit with finding text.
process.stdout.write("2 problems (2 errors, 0 warnings)\n");
process.exit(1);
99 changes: 99 additions & 0 deletions scripts/lint-gui-if-changed.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/**
* Run GUI eslint when this push includes gui/ changes.
* Used by `bun run prepush`. Skip with: git push --no-verify
*
* Mirrors `scripts/doctor-gui-if-changed.ts` so the local pre-push gate and
* the CI `gates` job agree: GUI lint runs only when the push actually touches
* `gui/`. Unlike doctor there is no engine to fetch, so lint findings always
* fail the push — there is no infra-degradation path to soft-skip on.
*
* Test hooks: LINT_DRY_RUN=1 prints the run/skip decision without spawning;
* LINT_FILES (newline-separated) overrides git-derived changed files;
* LINT_CMD overrides the spawned command.
*/
import { spawnSync } from "node:child_process";
import { join, resolve } from "node:path";

/** True when any changed path is the gui directory or inside it (slash-guarded). */
function guiPathsChanged(files: string[]): boolean {
return files.some(f => f === "gui" || f.startsWith("gui/"));
}

if (import.meta.main) {
const repoRoot = resolve(import.meta.dirname, "..");
const guiDir = join(repoRoot, "gui");

const hasRef = (ref: string): boolean => {
try {
const probe = spawnSync("git", ["rev-parse", "--verify", ref], {
cwd: repoRoot,
stdio: "ignore",
});
return probe.status === 0;
} catch {
return false;
}
};

const diffNames = (range: string): string[] => {
try {
const diff = spawnSync("git", ["diff", "--name-only", range], {
cwd: repoRoot,
encoding: "utf8",
});
if (diff.status !== 0) return [];
return (diff.stdout ?? "")
.split(/\r?\n/)
.map(line => line.trim())
.filter(Boolean);
} catch {
return [];
}
};

let files: string[];
let hadBase = true;
if (process.env.LINT_FILES !== undefined) {
files = process.env.LINT_FILES.split(/\r?\n/).map(f => f.trim()).filter(Boolean);
} else {
let range: string | null = null;
if (hasRef("@{u}")) range = "@{u}...HEAD";
else if (hasRef("origin/main")) range = "origin/main...HEAD";
else if (hasRef("main")) range = "main...HEAD";
hadBase = range !== null;
files = range ? diffNames(range) : [];
}

// No usable base — run lint so GUI pushes still get a check.
const shouldRun = hadBase ? guiPathsChanged(files) : true;

if (process.env.LINT_DRY_RUN === "1") {
console.log(shouldRun ? "lint:run" : "lint:skip");
process.exit(0);
}

if (!shouldRun) {
console.log("lint:gui: skip (no gui/ changes in push range)");
process.exit(0);
}

console.log("lint:gui: gui/ changed — running eslint (scope=changed)");
const [cmd, ...args] = process.env.LINT_CMD
? process.env.LINT_CMD.split(" ")
: ["bun", "run", "lint"];

const result = spawnSync(cmd!, args, {
cwd: guiDir,
encoding: "utf8",
stdio: "inherit",
});

// Lint is local and deterministic: findings fail the push, and a failed
// spawn is a real error, not an infrastructure soft-skip.
if (result.error) {
console.error(`lint:gui: could not run eslint: ${result.error.message}`);
process.exit(1);
}

process.exit(result.status === null ? 1 : result.status);
}
7 changes: 4 additions & 3 deletions scripts/setup-hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
* Sets up the git pre-push hook for local development.
* Run once after cloning: bun run setup:hooks
*
* The hook runs `bun run prepush` (typecheck + gui eslint + tests + privacy scan +
* React Doctor when `gui/` changed) before every push — the local portion of the CI gate.
* The hook runs `bun run prepush` (typecheck + tests + privacy scan + GUI
* eslint and React Doctor when `gui/` changed) before every push — the local
* portion of the CI gate.
*
* To skip in an emergency: git push --no-verify
*/
Expand Down Expand Up @@ -58,5 +59,5 @@ try {
// Windows: Git for Windows calls sh.exe directly, executable bit not required.
}

console.log(`pre-push hook installed at ${dest}. Runs typecheck + gui eslint + tests + privacy scan (+ React Doctor when gui/ changed) before every push.`);
console.log(`pre-push hook installed at ${dest}. Runs typecheck + tests + privacy scan (+ GUI eslint and React Doctor when gui/ changed) before every push.`);
console.log("Skip in an emergency with: git push --no-verify");
Loading
Loading