Skip to content

Harden migration and Nx cache validation contracts - #889

Merged
kentcdodds merged 5 commits into
mainfrom
cursor/review-fix-validation-tooling-e067
Jul 23, 2026
Merged

kentcdodds merged 5 commits into
mainfrom
cursor/review-fix-validation-tooling-e067

Conversation

@kentcdodds

@kentcdodds kentcdodds commented Jul 22, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • anchor historical migration filenames, contents, and ledger entries to a genuinely pre-change Git commit
  • reject HEAD as a trust base; CI supplies PR-base/push-before SHAs, local branches use their merge base, and main/detached checkouts fall back to the first parent
  • fail safely when post-bootstrap migrations cannot be checked against available history; CI fetches complete history without persisting checkout credentials
  • canonicalize migration hashes to LF and enforce LF migration checkouts with .gitattributes
  • include the root Wrangler wrapper and Cloudflare mock server in relevant Nx test cache inputs
  • cover PR, main-push, local branch, detached/shallow, bootstrap, runtime main co-edit, CRLF, and Nx hash contracts

Tests

  • focused regression suites: 2 files, 13 tests passed
  • npm run validate: passed (format, lint, typecheck, 1,091 unit/worker tests, 17 Playwright tests, 2 MCP E2E tests, primitives, migrations)
  • replacement CI at 280ce219: Validate, preview deployment, Cursor Bugbot, and CodeRabbit passed
  • pre-commit and pre-push hooks: passed
System recap — composes existing primitives (low risk)

Mode: recap · Base: main @ ac6b9736 · Head: 280ce219

Classification: composes — repository validation behavior changes without adding or changing a runtime system primitive.

Primitives touched

None. The diff is limited to repository tooling, CI/Nx metadata, regression tests, checkout attributes, and contributor documentation.

Before / after

Before After
Validation could accept HEAD itself as trusted history Trust resolution rejects HEAD and selects PR base, push-before SHA, merge base, or first parent
Editable ledger digests were the only trust anchor for post-bootstrap migrations Pre-change Git contents and ledger entries anchor every migration already present before the change
Missing Git history could silently trust future ledger entries Missing history fails safely for post-bootstrap migrations; CI checks out full history without retaining credentials
Raw-byte hashes varied under core.autocrlf=true SQL hashes canonicalize CRLF to LF and Git enforces LF migration checkouts
Worker test caches omitted external runtime dependencies Nx hashes Cloudflare mock sources for test and wrangler-env.ts for test-mcp

Invariants

Applied migration filenames and contents are immutable. Migration and ledger digest co-edits fail against pre-change history, including on main pushes. The grandfathered duplicate-prefix pairs, including both 0009 files, remain exact and mandatory.

Open in Web Open in Cursor 

Summary by CodeRabbit

  • New Features
    • Added a migration ledger with SHA-256 checksums and expanded validation to ensure migration integrity, immutability, correct ordering, and proper add-only behavior.
    • Added support for trusted-history verification and improved error reporting when history or ledgers are co-edited.
    • Normalized migration hashing to consistent LF line endings for reliable comparisons.
  • Documentation
    • Updated contributing guidance for migration authoring, ledger updates, hashing, naming rules, and trusted-history checks.
  • Tests
    • Expanded migration validation and Nx cache hashing coverage with additional scenario-based checks.
  • Chores
    • Updated repository settings and CI workflow to enforce LF handling and consistent validation baselines.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Migration validation now tracks SQL content in an append-only ledger, verifies trusted Git history, and documents CI/base-selection rules. Worker test cache inputs include additional dependencies with contract tests confirming cache invalidation.

Changes

Migration validation

Layer / File(s) Summary
Ledger and hashing contracts
tools/check-migrations.ts, tools/migration-ledger.json, tools/check-migrations.node.test.ts
Adds ledger types, baseline metadata, canonical LF SHA-256 hashing, ledger shape validation, and hashing tests.
Directory validation flow
tools/check-migrations.ts, tools/check-migrations.node.test.ts
Combines filename checks with ledger/file digest verification, validates migration ordering, rejects historical mutations, and updates checker output.
Trusted history and CI wiring
tools/check-migrations.ts, .github/workflows/validate.yml, .gitattributes, docs/contributing/setup.md, tools/check-migrations.node.test.ts
Loads trusted migration history from Git, configures the validation base and full checkout history, enforces LF endings, and documents ledger maintenance with integration coverage.

Worker cache contracts

Layer / File(s) Summary
Worker test cache inputs
nx.json, tools/nx-cache-contract.node.test.ts
Adds Cloudflare mock-server and wrangler-env.ts inputs to worker test targets and verifies their changes affect computed cache hashes.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CI
  participant checkMigrations
  participant Git
  participant MigrationLedger
  CI->>checkMigrations: set MIGRATION_VALIDATION_BASE and run checker
  checkMigrations->>Git: resolve base and read trusted migration history
  Git-->>checkMigrations: trusted SQL files and ledger
  checkMigrations->>MigrationLedger: compare checkout filenames and SHA-256 digests
  MigrationLedger-->>CI: validation result
Loading

Possibly related PRs

  • kentcdodds/kody#407: Updates migration authoring guidance for grandfathered duplicate prefixes.
  • kentcdodds/kody#839: Adds the filename-ordering checker that this migration ledger validation extends.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes to migration validation and Nx cache inputs.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/review-fix-validation-tooling-e067

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kody-bot
kody-bot marked this pull request as ready for review July 22, 2026 22:15
@github-actions

github-actions Bot commented Jul 22, 2026 •

Copy link
Copy Markdown
Contributor

🔎 Preview deployed: https://kody-pr-889.kody-a99.workers.dev

Worker: kody-pr-889
D1: kody-pr-889-db
KV: kody-pr-889-oauth-kv

Mocks:

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/validate.yml:
- Around line 37-40: Update the checkout action configuration containing
fetch-depth: 0 to set persist-credentials to false, ensuring the workflow does
not retain the Actions token in Git configuration while leaving the full-history
checkout behavior unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 50ef6e0f-4d8f-4151-8466-5b16959c22c2

📥 Commits

Reviewing files that changed from the base of the PR and between ac6b973 and 8ed977c.

📒 Files selected for processing (8)
  • .gitattributes
  • .github/workflows/validate.yml
  • docs/contributing/setup.md
  • nx.json
  • tools/check-migrations.node.test.ts
  • tools/check-migrations.ts
  • tools/migration-ledger.json
  • tools/nx-cache-contract.node.test.ts

Comment thread .github/workflows/validate.yml
@kentcdodds
kentcdodds merged commit 2e1b038 into main Jul 23, 2026
5 checks passed
@kentcdodds
kentcdodds deleted the cursor/review-fix-validation-tooling-e067 branch July 23, 2026 02:27
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.

3 participants