fix: pre-commit runs zero hooks — restore the hooks, add a CI gate, apply Prettier

[pdettori]

Fixes #196.

What was broken

pre-commit run executed zero hooks — the config failed validation, so shellcheck, gitleaks, check-yaml and the whitespace fixers never ran either. Root cause exactly as reported: types_or named typescript, which is not an identify tag (the tags are ts/tsx), so .ts files were never matched even before newer pre-commit turned the unknown tag into a hard InvalidConfigError.

Getting the config to load then surfaced two more broken hooks, both invisible for as long as nothing ran them:

• hadolint never linted anything. entry was overridden to a bare hadolint — that is the entry of upstream's system hook — so language: docker_image resolved an image literally named hadolint and every run failed before reaching a Dockerfile. All five Dockerfiles are in fact clean.
• shellcheck found a real SC2034 in remote-worker/deploy-incluster.sh (unused loop var), outside the deploy/ tree that the security-scans.yml shellcheck job covers.

make fmt was separately broken: .prettierrc/.prettierignore were committed but no package.json declared Prettier, so pnpm exec prettier died with ERR_PNPM_RECURSIVE_EXEC_NO_PACKAGE.

Changes

Commit 1 — fix(pre-commit)

• Root package.json pinning prettier to 3.9.6 exactly → make fmt works from a clean pnpm install.
• Prettier hook moved to repo: local driving that same pinned binary, replacing pre-commit/mirrors-prettier (archived upstream, and pinned to a Prettier 4 pre-release — two Prettiers that could format differently). One binary for both means make fmt and the hook cannot disagree. This is the issue's "optional follow-up", taken up because two acceptance criteria jointly require it.
• ts/tsx (and jsx) added so TypeScript is actually covered.
• hadolint image reference restored, pinned to match rev (upstream's own entry is untagged, i.e. :latest).
• shellcheck set to -S warning, matching the severity security-scans.yml already enforces, so the hook and that job cannot disagree.
• CI lint job running make lint. It reuses only actions already pinned in this repo plus preinstalled pipx, so no new third-party action to vet.
• CONTRIBUTING.md documents pre-commit install, make lint and make fmt.

Commit 2 — style: the repo-wide Prettier reformat (267 files), kept separate so it does not bury the fix.

Judgment call worth reviewing

-S warning for shellcheck is a deliberate choice over fixing ~40 info/style findings in a PR about pre-commit config. The scripts are clean at warning+ once the SC2034 is fixed, the existing CI gate already uses that exact severity, and the info-level set includes SC1091 false positives (shellcheck runs from the repo root and cannot resolve source ./lib.sh). Tightening severity is a reasonable follow-up; happy to do it here instead if you prefer.

Verification

Acceptance criterion	Result
pre-commit validate-config exits 0	✅ exit=0
pre-commit run --all-files runs every hook and passes	✅ exit=0, all 9 hooks Passed
make fmt works from a clean pnpm install	✅ and idempotent (0 files changed on a formatted tree)
A CI job fails the build when hooks fail	✅ new lint job
CONTRIBUTING.md mentions pre-commit install	✅

The reformat was checked for semantics, not just eyeballed:

• All 31 changed YAML/JSON files parse to documents identical to their previous contents (compared as parsed structures, not text) — this covers the deploy/ manifests, where Prettier reflowed flow sequences and switched quote style.
• make typecheck clean.
• pnpm -r test: 827 passed, 15 skipped.
• make test-deploy passes.
• Commit-time path confirmed: a deliberately misformatted staged .ts file is now caught and fixed by the hook — the case the old config could never catch.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

[pdettori]

CI status: the new lint gate passes; the two red checks are pre-existing and not from this PR

Green, including the new gate this PR adds: lint ✅ (all nine hooks run and pass in CI), check ✅, proto ✅, deploy-scripts ✅, shellcheck ✅, hadolint ✅, DCO ✅, dependency-review ✅.

Two checks are red. I verified both against main rather than assuming, and neither is caused by this branch.

1. trivy-scan — a newly published CVE, fails on main too

The job's exit-code: 1 combined with format: sarif means the action scans at all severities (its log says Building SARIF report with all severities and runs plain trivy fs .), so any finding fails it.

Scanning two clean checkouts with the same Trivy version and DB — one at main, one at this branch's HEAD:

Checkout	Exit	Finding
main	1	CVE-2026-84304 — google.golang.org/grpc v1.82.1 → fixed in 1.83.1
this branch	1	identical, same two files

Present in both gen/go/go.mod and remote-worker/go.mod, and v1.82.1 is byte-identical on main. pnpm-lock.yaml reports 0 vulnerabilities, so the root package.json this PR adds is not implicated — Prettier is a devDependency and Trivy suppresses dev deps anyway.

Recent PRs (#195, #199, #201) passed because they ran before this CVE entered Trivy's DB; this PR is just the first run after it. Consistent with how #162 handled the previous grpc bump, this belongs in its own issue/PR — happy to open one bumping grpc to 1.83.1 in both modules.

2. CodeQL — a pre-existing alert re-attributed to the reformat

One high-severity alert, Polynomial regular expression used on uncontrolled data, at harness/src/run-leaf.ts:152. GitHub's own summary carries the caveat: "Alerts not introduced by this pull request might have been detected because the code changes were too large."

That is exactly what happened. The flagged regex is workspaceRef.replace(/\/+$/, ...), which exists unchanged on main (line 109 there, line 152 here). The only edit on that line is the replacement string's quote style, " → ', per .prettierrc:

-  const filePath = workspaceRef ? `${workspaceRef.replace(/\/+$/, "")}/${item.file}` : item.file;
+  const filePath = workspaceRef ? `${workspaceRef.replace(/\/+$/, '')}/${item.file}` : item.file;

The regex itself is untouched, so this is a line-movement artifact of the 267-file reformat, not a new alert. It can be dismissed, or hardened separately if you'd like the pattern changed on its merits — I left it alone deliberately, since silently editing a regex is not what an issue about pre-commit config should do.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

[pdettori]

Reviewed the fix commit closely and verified the reformat commit's safety claims independently rather than taking them from the description.

Author: pdettori (MEMBER — maintainer)
Areas reviewed: pre-commit config, CI workflows, shell, YAML/JSON reformat, docs, dependency manifest
Agent/IDE config (.claude/.vscode): none
Commits: 2, all signed off: yes
CI status: 2 red (trivy-scan, CodeQL) — both pre-existing, see below

Verified independently

Claim	Result
Reformat is semantics-preserving	✅ Parsed base vs head for all 34 modified YAML/JSON files: 31 identical as parsed documents. The only 3 semantic diffs are the intended ones (ci.yml, .pre-commit-config.yaml, pnpm-lock.yaml). This covers the deploy/ manifests where Prettier reflowed flow sequences and switched quote style.
Prettier pinned exactly	✅ 3.9.6 is npm's current latest; lockfile delta is +13/-0 — a root importer with prettier as its sole devDependency, no lockfileVersion change, no other packages.
All hooks actually run	✅ 9 hook ids declared; CI lint job green.
Least privilege / supply chain	✅ ci.yml has top-level permissions: contents: read, inherited by the new lint job. All actions SHA-pinned; no new third-party action. No .claude/ or .vscode/ changes in the diff.
language: system prerequisite	✅ CONTRIBUTING.md orders pnpm install before pre-commit install, so the hook's dependency on the workspace binary is documented in the right order.

The hadolint entry restoration and the -S warning alignment with security-scans.yml both check out: the hook rev (v2.12.0) and the pinned image tag match, and the shellcheck severity is now identical to the CI gate, so the two cannot disagree. The for i → for _ SC2034 fix is correct. Splitting the 267-file reformat into its own commit was the right call.

Both red checks are pre-existing — not introduced here

trivy-scan — I ran trivy against the base tree and the head tree separately. The findings are identical: google.golang.org/grpc v1.82.1 → CVE-2026-84304 (HIGH, fixed in 1.83.1) in both gen/go/go.mod and remote-worker/go.mod, plus zero pnpm findings and zero secrets on both sides. Neither go.mod is touched by this PR. The job uses format: sarif, so nothing prints to stdout and the failure looks opaque in the log.

Why recent PRs were green: the CVE published 2026-09-01T18:19Z and was last modified 2026-09-02T03:47Z, entering trivy's DB after #201's green run at 2026-09-01T19:39Z. This is the same class of breakage as #163 (grpc CVE red-lining the gate) and needs a separate 1.82.1 → 1.83.1 bump.

CodeQL — 1 HIGH "Polynomial regular expression used on uncontrolled data" at harness/src/run-leaf.ts:152. The regex /\/+$/ exists verbatim on main at line 109; Prettier's import expansion shifted it down and changed only the quote style, so CodeQL attributed a pre-existing alert to this PR — exactly the caveat its own summary raises ("Alerts not introduced by this pull request might have been detected because the code changes were too large"). Needs dismissal or its own fix to unblock merge.

Neither is a reason to change anything in this PR, but both block it mechanically.

Minor aside

CONTRIBUTING.md:17 and :96 still point at kagenti/* (clone URL and Code of Conduct) while the repo lives under rossoctl/. Pre-existing and possibly deliberate, so noting rather than flagging.

No must-fix findings. Submitted as a comment rather than an approval only because GitHub does not permit approving your own PR.

Assisted-By: Claude Code

[pdettori]

suggestion — exclude: on the next line duplicates a subset of .prettierignore: it carries pi-fork/, packages/k8s-sandbox/src/gen/ and gen/, but not node_modules/, dist/, *.log or pnpm-lock.yaml.

Behavior is already correct today, because Prettier applies .prettierignore even to paths passed explicitly on the command line — which this PR incidentally demonstrates: pnpm-lock.yaml is in .prettierignore and came through the repo-wide reformat with only the +13/-0 dependency addition, no reflow.

So this is not a bug, just a second list that has to stay in agreement with the first. Since the stated goal here is that make fmt and the hook "cannot disagree by construction", dropping exclude entirely and letting .prettierignore be the single source of truth would extend that same property to the ignore set. Non-blocking.

[pdettori]

Applied in e34b5e9 — exclude is gone, .prettierignore is now the only ignore list.

Agreed on the reasoning, and I checked the mechanism rather than trusting it. Appending deliberately misformatted code to packages/k8s-sandbox/src/gen/sandbox/v1/sandbox.ts and passing that path directly to pnpm exec prettier --write --ignore-unknown:

$ tail -1 packages/k8s-sandbox/src/gen/sandbox/v1/sandbox.ts
const   ugly={a:1,   b:"x"}
$ pnpm exec prettier --write --ignore-unknown packages/k8s-sandbox/src/gen/sandbox/v1/sandbox.ts; echo "exit=$?"
exit=0
$ tail -1 packages/k8s-sandbox/src/gen/sandbox/v1/sandbox.ts
const   ugly={a:1,   b:"x"}     # untouched

Two things that confirm it is safe beyond the happy path: the file is left alone even though it was named explicitly, and exit is 0 even though every path passed was ignored — so pre-commit batches consisting entirely of ignored files do not fail the hook.

Coverage-wise .prettierignore is a strict superset of what exclude carried, so nothing widens: pi-fork/, packages/k8s-sandbox/src/gen/ and gen/ are all in it, plus the four it was missing.

make lint still exits 0 with all nine hooks running, and no generated or submodule file is touched. I left a comment at the hook explaining why there is no exclude, so it does not get re-added later as an apparent oversight.

[pdettori]

Opened #203 to clear the trivy-scan half of the two red checks here: grpc v1.82.1 → v1.83.1 in both Go modules, which is the pre-existing CVE-2026-84304. Verified there that the same all-severity gate CI applies now exits 0 with zero findings, and that both modules still build, vet, test (-race) and produce no codegen drift.

That PR is independent of this one — it branches from main and touches only the four Go dependency files, no overlap with anything here.

Once #203 lands, trivy-scan should go green on this branch too on the next run. The remaining red check is then just the CodeQL alert, which is the pre-existing /\/+$/ regex discussed above — still needing either a dismissal or its own change, whichever you prefer.

[pdettori]

Rebased onto main (post #203, #204) — all 11 checks now green ✅

Both previously-red checks clear, at source rather than by dismissal:

Check	Before	Now	Why
trivy-scan	❌	✅	#203 bumped grpc to v1.83.1
CodeQL	❌	✅	#204 replaced the polynomial regex with a linear scan
lint	✅	✅	unchanged

Everything else (check, proto, deploy-scripts, shellcheck, hadolint, DCO, dependency-review, Trivy, codeql) is green too.

The reformat commit was regenerated, not replayed

#204 edited harness/src/run-leaf.ts and its test — the same files the reformat touches — so replaying the old 267-file diff would have conflicted for no benefit. Formatting is mechanical output, so I re-ran make fmt on the rebased tree instead. That also formats the code #204 added, which I had deliberately written in main's double-quote style.

Verified after rebasing: pre-commit run --all-files exits 0 with all nine hooks; make typecheck clean; pnpm -r test 851 passed, 15 skipped; make test-deploy passes; make fmt idempotent. I also confirmed #204's fix survived intact — stripTrailingSlashes still shared by both builders, no polynomial trim regex anywhere in harness/src, and the ReDoS guard test still present.

Commit order is unchanged (fix → reformat → review follow-up), so the messages still read correctly.

One addition worth re-reviewing: .prettierignore now excludes sibling worktrees

+.worktrees/
+.claude/worktrees/

Found by running this for real: make fmt is prettier --write ., which walks the directory tree and reads only .prettierignore — not .gitignore. On a checkout with git worktrees under .worktrees/, it therefore reformats files belonging to other branches' checkouts. It did exactly that here, dirtying 13 sibling worktrees with 200–260 modified files each, all of them unrelated to this PR.

Worth flagging that my original acceptance evidence for "make fmt works" missed this: I counted only the main checkout's tracked files, which showed a clean 0 and hid the side effect entirely. The two ignore entries stop it at the source, and prettier --check . now passes scoped to this checkout alone.

This is the third instance of the same theme in this PR — a formatting tool doing something invisible because nothing checked the blast radius.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com