Skip to content

chore: enforce formatting in CI and fix existing drift - #436

Merged
V3RON merged 1 commit into
mainfrom
chore/enforce-formatting-in-ci
Aug 20, 2026
Merged

chore: enforce formatting in CI and fix existing drift#436
V3RON merged 1 commit into
mainfrom
chore/enforce-formatting-in-ci

Conversation

@V3RON

@V3RON V3RON commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Description

pnpm format:all fails on main today. That matters more since #435, because CONTRIBUTING.md and the pull request guide now point contributors at pnpm checks:affected, and that script ends in format:all.

  • Reformats three files that predate the move to oxfmt.
  • Adds a Check formatting step to CI so it cannot drift again.
  • Ignores Claude Code's local state, which was breaking format:all independently.

Related Issue

None — repository tooling, follow-up to #435.

Context

Why the three files were unformatted. The diffs unwrap lines rather than wrapping them — a six-line union type collapses to one line, a markdown table's column padding widens. That is the signature of files formatted at Prettier's narrower print width and never reformatted when the repo moved to oxfmt. They are stale leftovers, not new drift.

The only token-level changes are trailing commas appearing/disappearing as arguments collapse onto one line, and one leading union | — all semantically inert. Verified by checksumming both revisions with whitespace, commas and that pipe stripped, and by a full typecheck/lint/test pass on both affected packages. No behavioral change, so no version plan.

Why nothing caught it. No workflow has ever run pnpm format:all; CI runs turbo run typecheck build lint test --affected only. The new step runs before the turbo step so it fails fast — oxfmt checks all 1246 files in under 400ms.

One gap this does not close: the workflow has paths-ignore: website/**, so a website-only PR runs no CI at all and could still land unformatted .mdx. Whether website PRs should gate on CI is a separate decision.

Why only two .claude/ paths are ignored. .claude/settings.local.json and .claude/worktrees/ were excluded only through per-machine files — the global git excludes file that Claude Code writes to, and .git/info/exclude. Git therefore ignored them but oxfmt did not, since oxfmt reads .gitignore/.prettierignore from the repo only. The result was that pnpm format:all failed for anyone using Claude Code, regardless of the three files above.

Ignoring .claude/ wholesale would be wrong: per the Claude Code settings documentation, .claude/settings.json, agents/, skills/ and commands/ are all intended to be committed and shared with the team. Only the two machine-local paths are ignored here.

Testing

  • pnpm format:all — passes (1246 files, 383ms)
  • npx turbo run typecheck lint test118 tasks, all successful
  • npx turbo run typecheck lint test --filter=@rozenite/network-activity-plugin --filter=@rozenite/web — 38 tasks, all successful
  • Confirmed the reformatting is semantically inert by checksum, as described above
  • CI workflow YAML re-parsed after editing; step order verified

`pnpm format:all` failed on `main`, which matters now that CONTRIBUTING.md and
the pull request guide point contributors at `pnpm checks:affected` — that
script ends in `format:all`. No workflow has ever run it, which is why the
drift went unnoticed.

- Reformat three files that predated the move to oxfmt. The diffs unwrap lines
  that were wrapped at Prettier's narrower print width; the only token changes
  are trailing commas and one leading union `|`, all semantically inert.
  Verified by checksumming both revisions with whitespace, commas and that
  pipe stripped, and by a full typecheck/lint/test pass on both affected
  packages.
- Add a `Check formatting` step to the Validate job so this cannot drift again.
  oxfmt checks all 1246 files in under 400ms, so it runs before the turbo step
  to fail fast.
- Ignore Claude Code's local state. `.claude/settings.local.json` and
  `.claude/worktrees/` were only excluded through per-machine files (the global
  git excludes file and `.git/info/exclude`), so oxfmt still picked the settings
  file up and `pnpm format:all` failed for anyone using Claude Code. Only these
  two paths are ignored: shared `.claude/` config such as `settings.json`,
  `agents/`, `skills/` and `commands/` is meant to be committed.
@V3RON
V3RON merged commit 1bf08b5 into main Aug 20, 2026
4 checks passed
@V3RON
V3RON deleted the chore/enforce-formatting-in-ci branch August 20, 2026 09:04
V3RON added a commit that referenced this pull request Aug 20, 2026
## Description

Caches Turborepo artifacts in CI, restoring on every run but writing
only from pushes to `main`.

The Validate job spends **319s of its 6m19s** inside `turbo run`.
Nothing persists between runs today: `.turbo` is gitignored and only the
pnpm store is cached, so every run rebuilds affected packages and their
entire dependency chain from scratch.

> Stacked on #436 — the base will retarget to `main` once that merges.
Only the two workflow files are this PR's own change.

## Related Issue

None — repository tooling, follow-up to #435.

## Context

**Why saves are restricted to `main`.** GitHub scopes a pull request's
cache to its merge ref, and that cache "can only be restored by re-runs
of the pull request" — so a fork PR cannot reach `main`'s scope or
another PR's regardless. Restricting saves closes the remaining case: a
run writing a cache that a later re-run *of the same PR* would restore.

That distinction matters more for Turborepo than for the pnpm store. A
poisoned store still leaves every task to actually run. A poisoned
Turborepo artifact makes tasks report a cache hit **without executing**
and replays their stored logs — and #435's `outputLogs` now suppresses
cache-hit logs, so the replay is quieter than it used to be. A fork PR
could use that to forge a green check on a diff that would otherwise
fail. Nothing detects this for us: pnpm documents `verifyStoreIntegrity`
as catching accidental corruption, explicitly not deliberate tampering,
and Turborepo's signature verification applies only to *remote* caches,
not the filesystem cache `actions/cache` persists. Scope is the control
that matters.

Saving only from `main` means every cached artifact was produced by
already-merged, reviewed code. PRs still restore `main`'s cache, which
covers every dependency build they did not change; they only re-execute
tasks for the packages they actually touch, which `--affected` already
bounds.

**Why the key rotates weekly with no fallback.** Turborepo never prunes
`.turbo/cache`. The accumulated local cache in this repository is
already **1.8GB across 10050 entries**. Because each `main` run restores
before it saves, the saved artifact is the union of everything cached so
far — so an unrotated key grows without limit until it hits the 10GB
per-repository cache limit and evicts everything else, including the
pnpm store cache.

There is deliberately no `restore-keys` fallback to the previous period:
falling back would carry the accumulation forward and defeat the reset.
The cost is one cold run at the start of each week, which is no worse
than today's behaviour, where every run is cold.

**Why `release.yml` stays uncached.** It holds `contents: write` and
`id-token: write` and publishes to npm. A restored artifact would be
published with provenance attesting to something that was never built
from this source. The job now carries a comment saying so, since this is
exactly the kind of thing a later "optimization" would undo.

## Testing

- Both workflow files re-parsed as YAML after editing; step order and
`if:` conditions verified
- `actions/cache` pinned to `55cc8345863c7cc4c66a329aec7e433d2d1c52a9`,
confirmed via the API to be v6.1.0 in the first-party `actions/` org,
matching this repository's pin-everything convention
- Confirmed the `restore` and `save` sub-actions exist at that exact SHA
- `npx turbo run typecheck lint test` — **118 tasks, all successful**
- `pnpm format:all` — passes

Cache behaviour itself can only be confirmed on a real run. Two things
to watch:

1. **The path.** Turborepo resolves `.turbo/cache` at the repository
root in a plain clone; I could only verify the resolution logic locally
inside a git worktree, where it deliberately redirects to the main
checkout. If the path were wrong the symptom is a cache that never hits,
not a wrong result.
2. **Effect on runtime.** The baseline to beat is the 6m19s Validate run
on #435. The first push to `main` after this merges is still cold — it
populates the cache. Subsequent runs are the ones to measure.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant