Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pdettori merged 3 commits into from
Sep 2, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
08b4d2e
fix(pre-commit): run the hooks again -- 'typescript' is not an identi…
pdettori Sep 2, 2026
b6f85a4
style: apply prettier across the tree for the first time
pdettori Sep 2, 2026
0409617
fix(pre-commit): drop the prettier hook's `exclude`, keep one ignore …
pdettori Sep 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/dependabot.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ updates:
# bumped together — they fail CI if versions diverge across steps.
codeql-action:
patterns:
- "github/codeql-action*"
- 'github/codeql-action*'

- package-ecosystem: npm
directory: /
Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/build.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,26 +46,26 @@ jobs:
file: deploy/knative/echo-target/Dockerfile
steps:
- name: Checkout
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
submodules: recursive

- name: Set up QEMU
uses: docker/setup-qemu-action@96fe6ef7f33517b61c61be40b68a1882f3264fb8 # v4
uses: docker/setup-qemu-action@96fe6ef7f33517b61c61be40b68a1882f3264fb8 # v4

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@37fe631027851001ddb9b187196cc803df7f5f0e # v4
uses: docker/setup-buildx-action@37fe631027851001ddb9b187196cc803df7f5f0e # v4

- name: Log in to ghcr.io
uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4
uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4
with:
registry: ${{ env.REGISTRY }}
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Extract Docker metadata
id: meta
uses: docker/metadata-action@dc802804100637a589fabce1cb79ff13a1411302 # v6
uses: docker/metadata-action@dc802804100637a589fabce1cb79ff13a1411302 # v6
with:
images: ${{ matrix.image }}
tags: |
Expand All @@ -74,7 +74,7 @@ jobs:
type=semver,pattern={{version}}

- name: Build and push
uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a # v7
uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a # v7
with:
context: ${{ matrix.context }}
file: ${{ matrix.file }}
Expand Down
59 changes: 48 additions & 11 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,27 @@ jobs:
--health-timeout 5s
--health-retries 5
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
submodules: recursive

- uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v6
- uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v6
with:
node-version: "22"
node-version: '22'

- uses: pnpm/action-setup@0977fd99725f1db4007ccb2928dbb4e90d06cc86 # v6.0.10
- uses: pnpm/action-setup@0977fd99725f1db4007ccb2928dbb4e90d06cc86 # v6.0.10
with:
version: 9

- name: Cache pnpm store
uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
path: ~/.local/share/pnpm/store/v3
key: pnpm-${{ hashFiles('pnpm-lock.yaml') }}
restore-keys: pnpm-

- name: Cache pi-fork node_modules
uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
path: pi-fork/node_modules
key: pi-fork-${{ hashFiles('pi-fork/package-lock.json') }}
Expand All @@ -68,6 +68,43 @@ jobs:
- name: Test
run: pnpm -r test

# Runs the same `make lint` (pre-commit over all files) that contributors run locally, so
# a config that silently executes zero hooks can no longer look identical to a green
# build -- which is exactly how the broken `typescript` identify tag survived unnoticed.
# Needs node/pnpm because the prettier hook drives the repo's own pinned Prettier, and
# needs the pi-fork submodule because the workspace links into it.
lint:
runs-on: ubuntu-latest
timeout-minutes: 10
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
submodules: recursive

- uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v6
with:
node-version: '22'

- uses: pnpm/action-setup@0977fd99725f1db4007ccb2928dbb4e90d06cc86 # v6.0.10
with:
version: 9

- name: Cache pre-commit environments
uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
path: ~/.cache/pre-commit
key: pre-commit-${{ hashFiles('.pre-commit-config.yaml') }}
restore-keys: pre-commit-

- name: Install workspace
run: pnpm install --frozen-lockfile

- name: Install pre-commit
run: pipx install pre-commit==4.3.0

- name: Run hooks
run: make lint

# The deploy/ shell tests are cluster-free (kubectl/kind/docker are mocked on PATH), so
# they need no node, no pnpm and no cluster -- hence their own fast job rather than a step
# tacked onto `check`. Without this job nothing runs them: security-scans.yml only
Expand All @@ -76,7 +113,7 @@ jobs:
runs-on: ubuntu-latest
timeout-minutes: 5
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1

- name: Run deploy shell tests
run: make test-deploy
Expand All @@ -85,14 +122,14 @@ jobs:
runs-on: ubuntu-latest
timeout-minutes: 10
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1

- uses: bufbuild/buf-setup-action@a47c93e0b1648d5651a065437926377d060baa99 # v1
- uses: bufbuild/buf-setup-action@a47c93e0b1648d5651a065437926377d060baa99 # v1
with:
version: "1.71.0"
version: '1.71.0'
github_token: ${{ github.token }}

- uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
- uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
with:
go-version-file: gen/go/go.mod

Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/scorecard.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ on:
push:
branches: [main]
schedule:
- cron: "30 6 * * 1"
- cron: '30 6 * * 1'
workflow_dispatch:

permissions: read-all
Expand All @@ -16,22 +16,22 @@ jobs:
security-events: write
id-token: write
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false

- uses: ossf/scorecard-action@2d1146689b8cda280b9bc96326124645441f03bc # v2.4.4
- uses: ossf/scorecard-action@2d1146689b8cda280b9bc96326124645441f03bc # v2.4.4
with:
results_file: results.sarif
results_format: sarif
publish_results: true

- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
with:
name: scorecard-results
path: results.sarif
retention-days: 30

- uses: github/codeql-action/upload-sarif@db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28 # v4
- uses: github/codeql-action/upload-sarif@db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28 # v4
with:
sarif_file: results.sarif
20 changes: 10 additions & 10 deletions .github/workflows/security-scans.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ jobs:
contents: read
pull-requests: write
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- uses: actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 # v4
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- uses: actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 # v4
with:
fail-on-severity: high
deny-licenses: GPL-3.0, AGPL-3.0
Expand All @@ -26,16 +26,16 @@ jobs:
contents: read
security-events: write
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0
with:
scan-type: fs
scan-ref: .
severity: CRITICAL,HIGH
exit-code: 1
format: sarif
output: trivy-results.sarif
- uses: github/codeql-action/upload-sarif@db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28 # v4
- uses: github/codeql-action/upload-sarif@db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28 # v4
if: always()
with:
sarif_file: trivy-results.sarif
Expand All @@ -49,19 +49,19 @@ jobs:
matrix:
language: [javascript-typescript]
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- uses: github/codeql-action/init@db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28 # v4
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- uses: github/codeql-action/init@db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28 # v4
with:
languages: ${{ matrix.language }}
queries: security-extended
- uses: github/codeql-action/analyze@db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28 # v4
- uses: github/codeql-action/analyze@db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28 # v4

shellcheck:
runs-on: ubuntu-latest
permissions:
contents: read
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- name: Run shellcheck
run: find deploy/ -name '*.sh' -exec shellcheck -x -S warning {} +

Expand All @@ -70,6 +70,6 @@ jobs:
permissions:
contents: read
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- name: Run hadolint
run: docker run --rm -i -v "$PWD/.hadolint.yaml:/.config/hadolint.yaml" hadolint/hadolint < Dockerfile
2 changes: 1 addition & 1 deletion .hadolint.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
ignored:
- DL3018 # Alpine package version pinning breaks on minor bumps
- DL3018 # Alpine package version pinning breaks on minor bumps
36 changes: 28 additions & 8 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,47 @@ repos:
args: [--maxkb=500]
- id: check-merge-conflict

- repo: https://github.com/pre-commit/mirrors-prettier
rev: v4.0.0-alpha.8
# Prettier runs from the repo's own pinned devDependency (package.json) instead of
# pre-commit/mirrors-prettier: that mirror is archived upstream and was pinned to a
# Prettier 4 pre-release, so it could format differently from `make fmt`. Driving the
# same binary from both makes them identical by construction. Needs `pnpm install`.
#
# `ts`/`tsx` are the identify tags for TypeScript -- there is no `typescript` tag, and
# naming one silently matched no .ts file and then failed config validation outright.
#
# Deliberately no `exclude:`. Prettier applies `.prettierignore` even to paths handed to
# it explicitly, which is exactly how pre-commit invokes it, so `.prettierignore` is the
# single source of truth for the ignore set -- one list to keep correct instead of two
# that must agree. Same reasoning as the single pinned binary above.
- repo: local
hooks:
- id: prettier
types_or: [javascript, typescript, json, yaml, markdown]
exclude: ^(pi-fork/|packages/k8s-sandbox/src/gen/|gen/)
name: prettier
entry: pnpm exec prettier --write --ignore-unknown
language: system
types_or: [javascript, jsx, ts, tsx, json, yaml, markdown]

@pdettori pdettori Sep 2, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 pdettori Sep 2, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


# `-S warning` matches the shellcheck gate in .github/workflows/security-scans.yml
# exactly, so the hook and that job cannot disagree about what fails. The scripts are
# clean at warning+; the remaining info/style findings include SC1091 false positives
# (shellcheck runs from the repo root and cannot resolve `source ./lib.sh`). Tightening
# the severity is a separate change from getting the hooks running at all.
- repo: https://github.com/shellcheck-py/shellcheck-py
rev: v0.10.0.1
hooks:
- id: shellcheck
args: [-x]
args: [-x, -S, warning]
files: \.sh$

# `entry` must stay `<image> hadolint`: this previously overrode it to a bare `hadolint`,
# which is the entry of upstream's *system* hook, so docker_image resolved an image
# literally named `hadolint` and every run failed before linting anything. The image is
# pinned to match `rev` -- upstream's own entry is untagged, i.e. :latest.
- repo: https://github.com/hadolint/hadolint
rev: v2.12.0
hooks:
- id: hadolint-docker
entry: hadolint
language: docker_image
types: [dockerfile]
entry: ghcr.io/hadolint/hadolint:v2.12.0 hadolint

- repo: https://github.com/gitleaks/gitleaks
rev: v8.21.2
Expand Down
6 changes: 6 additions & 0 deletions .prettierignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,9 @@ dist/
pnpm-lock.yaml
packages/k8s-sandbox/src/gen/
gen/

# Sibling checkouts of this same repo. `prettier --write .` (make fmt) walks the directory tree
# and reads only this file -- not .gitignore -- so without these it reformats files belonging to
# other worktrees, dirtying branches that have nothing to do with the current one.
.worktrees/
.claude/worktrees/
16 changes: 8 additions & 8 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ serverless-harness/

## Key Commands

| Task | Command |
|------|---------|
| Install deps | `pnpm install` |
| Task | Command |
| ------------------- | --------------------------------------- |
| Install deps | `pnpm install` |
| Build pi-fork types | `cd pi-fork && npm ci && npm run build` |
| Lint | `make lint` |
| Format | `make fmt` |
| Test (all) | `make test` |
| Typecheck | `make typecheck` |
| Pre-commit install | `pre-commit install` |
| Lint | `make lint` |
| Format | `make fmt` |
| Test (all) | `make test` |
| Typecheck | `make typecheck` |
| Pre-commit install | `pre-commit install` |

## Development Setup

Expand Down
Loading
Loading