Skip to content

ci(lint): nx cache + affected, plus the escape-hatch ratchet#2661

Open
jordan-simonovski wants to merge 13 commits into
mainfrom
jordansimonovski/lint-nx-cache-affected
Open

ci(lint): nx cache + affected, plus the escape-hatch ratchet#2661
jordan-simonovski wants to merge 13 commits into
mainfrom
jordansimonovski/lint-nx-cache-affected

Conversation

@jordan-simonovski

@jordan-simonovski jordan-simonovski commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Two related lint-pipeline changes, combined into one PR so the CI lint job stays coherent (they'd otherwise conflict at merge and the ratchet could be silently dropped).

What changed

  • Faster lint via nx cache + affected. ci:lint/ci:build are now cacheable in nx.json; the lint job persists .nx/cache, runs nx affected -t ci:lint on PRs (only changed projects) and the full nx run-many on push. Locally, a warm ci:lint drops from ~41s to ~0.4s.
  • Escape-hatch ratchet. scripts/ci/ratchet.mjs counts as any / @ts-ignore / eslint-disable per package against a committed baseline; above-baseline fails CI, below-baseline warns (non-fatal). Exposed as yarn ratchet / yarn ratchet:update.
  • The ratchet runs as its own explicit lint-job step (node scripts/ci/ratchet.mjs), unconditionally — because the lint job now calls nx directly rather than make ci-lint, and the ratchet is a whole-repo check that must not be gated by nx affected.

Key decisions

  • No false cache hits: ci:lint's inputs include ^production, so editing packages/common-utils invalidates packages/app's cached lint (app's tsc typechecks against it). Verified. The root .prettierrc/.prettierignore are in sharedGlobals so a root Prettier change busts every project's cached lint too.
  • Below-baseline is non-fatal (warns, exits 0): an improvement must never red main or block other PRs when two count-lowering PRs merge close together. Above-baseline still hard-fails — that's the gate that matters.
  • Combined, not stacked: folding the ratchet into this PR means the explicit ratchet step and the nx-direct lint job land together, so the gate can't be lost in a merge-order gap.

Impact

  • PRs lint/build only affected projects and replay from cache; the ratchet gate runs every time.
  • affected on PRs, full run-many on push (nothing skipped on trunk). make ci-lint / make ci-build unchanged for local use.
  • CI only — no changeset.
Implementation detail
  • ci:lint dependsOn: ["^ci:build"] so upstream builds run first; nx hashes inputs internally, so a restored cache only replays on an input-hash match. Cache key nx-${{ runner.os }}-${{ github.sha }} + prefix restore.
  • Ratchet: grep exit-2 on a missing/renamed package dir is guarded with existsSync (returns 0, no crash). Verified: above-baseline exits 1, below-baseline warns + exits 0, at-baseline "ok".

The lint CI job rebuilt and re-linted every package from scratch on each
run with no cross-run caching, taking 3-4 minutes.

Make the ci:build and ci:lint targets nx-cacheable and wire ci:lint's
dependsOn to ^ci:build in nx.json, so a downstream project's tsc/lint is
correctly invalidated when an upstream package such as common-utils
changes (no false cache hits). Track root tsconfig.base.json via
sharedGlobals, and declare the common-utils build output so it can be
replayed from cache.

In the workflow, persist .nx/cache via actions/cache, checkout with full
history, run nx affected -t ci:lint against the PR base on pull_request,
and keep a full nx run-many on push. make ci-lint / make ci-build are
unchanged and still work locally.
@vercel

vercel Bot commented Jul 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment Jul 23, 2026 12:08am
hyperdx-storybook Ready Ready Preview, Comment Jul 23, 2026 12:08am

Request Review

@changeset-bot

changeset-bot Bot commented Jul 16, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 9ade8ea

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions Bot added the review/tier-4 Critical — deep review + domain expert sign-off label Jul 16, 2026
@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

🔴 Tier 4 — Critical

Touches auth, data models, config, tasks, OTel pipeline, ClickHouse, or CI/CD.

Why this tier:

  • Critical-path files (1):
    • .github/workflows/main.yml

Review process: Deep review from a domain expert. Synchronous walkthrough may be required.
SLA: Schedule synchronous review within 2 business days.

Stats
  • Production files changed: 5
  • Production lines changed: 133
  • Branch: jordansimonovski/lint-nx-cache-affected
  • Author: jordan-simonovski

To override this classification, remove the review/tier-4 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Placeholder

@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes two complementary CI improvements: nx computation caching (with affected on PRs and run-many on push) for faster lint/build, and a new ratchet script that hard-fails when as any / @ts-ignore / eslint-disable counts exceed the committed baseline.

  • nx cachingci:build and ci:lint are now cacheable targets in nx.json, keyed on inputs: [\"production\"/\"default\", \"^production\"] so upstream changes correctly bust downstream caches; .nx/cache is persisted in GitHub Actions with a SHA-exact key and an OS-prefix restore key.
  • Escape-hatch ratchetscripts/ci/ratchet.mjs counts occurrences per package via grep -rEo against a committed ratchet-baseline.json; above-baseline exits 1 (hard failure), below-baseline warns and exits 0 to avoid turning main red when two improvement PRs race; yarn ratchet:update re-baselines locally.
  • Ratchet runs unconditionally as an explicit step after the nx lint step, ensuring it can't be skipped by nx affected on small-diff PRs.

Confidence Score: 5/5

Safe to merge — changes are confined to CI infrastructure with no impact on application code or data.

All changes are CI tooling: nx cache configuration, a new escape-hatch ratchet script, and workflow restructuring. The nx input/output wiring is correct (named inputs are properly defined, ^production propagates upstream changes), the ratchet's above-baseline hard-fail and below-baseline warn logic is sound, and the affected/run-many split is standard. The two findings are cosmetic output quality issues that don't affect correctness or gate reliability.

No files require special attention. The only items flagged are a now-unneeded bc install and a mildly misleading final log line in ratchet.mjs.

Important Files Changed

Filename Overview
.github/workflows/main.yml Lint job restructured: fetch-depth:0 added for nx affected, nx cache step added, lint split into affected (PR) and full (push) variants, explicit ratchet step added. The bc package in Install core libs is now unused but still installed.
scripts/ci/ratchet.mjs New ratchet script; hardcoded PACKAGES list, grep-based counting with existsSync guard for missing dirs. Final "ratchet ok" log fires even when below-baseline warnings were emitted.
nx.json ci:build and ci:lint targets made cacheable with proper inputs/outputs; sharedGlobals extended to bust cache on root config changes. All named inputs are properly defined.
scripts/ci/ratchet-baseline.json Initial committed baseline for escape-hatch counts across 5 packages. Values are plausible given codebase size.
Makefile ci-lint target extended to also run ratchet.mjs, keeping local make ci-lint aligned with what CI now runs.
package.json Added yarn ratchet and yarn ratchet:update convenience scripts exposing the new ratchet tool.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Push / PR event] --> B{event type?}
    B -- pull_request --> C["nx affected -t ci:lint\n--base=origin/BASE_REF"]
    B -- push to main/v1 --> D["nx run-many -t ci:lint"]
    C --> E[Escape-hatch ratchet\nnode scripts/ci/ratchet.mjs]
    D --> E
    E --> F{any count > baseline?}
    F -- yes --> G[exit 1 — CI fails]
    F -- no --> H{any count < baseline?}
    H -- yes --> I[warn + exit 0\nrun yarn ratchet:update]
    H -- no --> J[ratchet ok — exit 0]

    subgraph nx cache
        C
        D
        K[ci:build dependsOn upstream\nserved from .nx/cache on hit]
    end
    C -. dependsOn .-> K
    D -. dependsOn .-> K
Loading

Reviews (11): Last reviewed commit: "Merge branch 'main' into jordansimonovsk..." | Re-trigger Greptile

Comment thread nx.json
@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 240 passed • 1 skipped • 1049s

Status Count
✅ Passed 240
❌ Failed 0
⚠️ Flaky 3
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

Adds scripts/ci/ratchet.mjs, which counts tracked escape hatches (as any,
@ts-ignore, eslint-disable) per package and compares them against a committed
baseline. Counts above baseline fail CI (a new occurrence was added); counts
below baseline also fail, prompting the contributor to run yarn ratchet:update
so the improvement is locked in. This two-way-strict behaviour keeps the
baseline honest and stops the escape-hatch count from silently creeping up.

The check is wired into the ci-lint Makefile target so the existing lint CI job
enforces it, and yarn ratchet / yarn ratchet:update scripts are exposed for
local use.
@jordan-simonovski jordan-simonovski changed the title ci(lint): cache and scope the lint job with nx affected ci(lint): nx cache + affected, plus the escape-hatch ratchet Jul 16, 2026
@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Deep Review

Scope: CI lint pipeline — nx.json cache/target config, new scripts/ci/ratchet.mjs + ratchet-baseline.json, package.json ratchet scripts, and the reworked lint job in .github/workflows/main.yml. Base fcdcd78.

Environment caveat: git and all Bash commands failed here (sandbox init failure on every invocation), so this review was reconstructed from the head-state of the changed files and the Makefile rather than a line-level diff. Findings below are verified against file contents.

✅ No critical issues found. make ci-lint (Makefile:113) is exactly nx run-many -t ci:lint + the ratchet, which the workflow mirrors, so no check is dropped by calling nx directly. ci:build's declared outputs: ["{projectRoot}/dist"] matches the only two packages that define that target (common-utils, cli, both via tsup); the packages without a ci:build target are unaffected. The dependsOn: ["^ci:build"] + ^production input wiring correctly invalidates a dependent's cached ci:lint when a dependency's source changes, and BASE_REF is passed to the nx affected step via env (injection-safe).

🟡 P2 — recommended

  • scripts/ci/ratchet.mjs — this new CI gate carries non-trivial logic (occurrence counting via grep -rEo, --update baseline rewrite, above/below-baseline exit-code branching, and the missing-directory guard) but ships with no test coverage.
    • Fix: Add a test exercising the above-baseline (exit 1), below-baseline (warn, exit 0), at-baseline (exit 0), and missing-directory paths.
    • testing, correctness
🔵 P3 nitpicks (2)
  • scripts/ci/ratchet.mjs:20PACKAGES is a hardcoded list, so a newly added workspace package is silently excluded from the ratchet and its escape hatches are never gated.
    • Fix: Derive the package list from the workspace layout, or fail loudly when a packages/* source dir is absent from PACKAGES.
  • .github/workflows/main.yml (Escape-hatch ratchet step) — the ratchet step has no if: always(), so it is skipped whenever the preceding lint step fails; it is not literally unconditional as the step comment states.
    • Fix: Add if: always() if the ratchet result should be reported independently of the lint step's outcome.

Reviewers (6): correctness, reliability, security, maintainability, testing, project-standards.

Testing gaps: ratchet.mjs has no automated tests; the environment prevented git/diff access, so cache-invalidation and output wiring were verified against file head-state rather than a line-level diff.

Merging main raised the tracked escape-hatch counts above the frozen
baseline (notably common-utils as-any 69->116), all from code that
landed on main independently of this PR. Regenerate the baseline with
`yarn ratchet:update` to lock in current counts, which also captures
the reductions in app and cli.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-4 Critical — deep review + domain expert sign-off

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant