ci(root): hash every module a package type-checks - #858
Conversation
check-types enumerated what to include, so a package keeping modules outside src/ had them hashed by nothing: turbo hashed 5 of e2e's 35, and a type error under tests/ replayed a green earned before it existed. Starts from $TURBO_DEFAULT$ and subtracts instead, and adds the shared tsconfig as a global dependency, which 22 packages extend and no hash covered. A test asserts the property from turbo's own resolved inputs.
|
@codex please review this PR |
|
Warning Review limit reached
Next review available in: 3 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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. Comment |
@nextlyhq/adapter-drizzle
@nextlyhq/adapter-mysql
@nextlyhq/adapter-postgres
@nextlyhq/adapter-sqlite
@nextlyhq/admin
@nextlyhq/admin-css
@nextlyhq/blocks-engine
@nextlyhq/blocks-react
@nextlyhq/builder
create-nextly-app
nextly
@nextlyhq/plugin-form-builder
@nextlyhq/plugin-page-builder
@nextlyhq/plugin-sdk
@nextlyhq/plugin-seo
@nextlyhq/storage-s3
@nextlyhq/storage-uploadthing
@nextlyhq/storage-vercel-blob
@nextlyhq/ui
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19f5621316
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex please review this PR |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
A tsconfig paths entry pointing at a sibling's source puts that sibling's files in this package's program, and inputs are package-scoped however they are written, so editing packages/nextly/src left admin's hash unmoved. The dry run also lists a task whose script is absent, marking it <NONEXISTENT>, so membership alone did not prove anything runs.
|
@codex please review this PR |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Slicing at the first brace anywhere starts the parse inside a warning that happens to contain one, which is what CI hit: the same "Expected property name or '}' at position 1" reproduces from a preceding line carrying a brace. A failed parse now names what it received.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bb960eb89
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex please review this PR |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
What was wrong
mainwas reporting a green@nextlyhq/e2e#check-typeswhile the package held two real type errors. Nothing was wrong with the check. It was never executed.check-typesdeclared its inputs as an INCLUDE list:e2ehas nosrc/directory. Its modules live intests/, which no glob matches. Asked directly, turbo hashed 5 files for a package holding 35:So a type error introduced under
tests/left the hash unmoved and turbo replayed the previous entry. Since turbo does not cache failures, the replayed result was a green computed before the break existed. This is not a stale answer; it is an answer to a different question.A second, larger instance of the same defect
check-typesdeclaresdependsOn: [], so no package has a graph edge to@nextlyhq/tsconfig— yet 22 packages extend itsbase.json, which decidesstrict,targetandlib. Nothing hashed it. Measured before the fix:Changing a compiler setting would have replayed stale greens across the entire monorepo at once.
The fix
Subtract from the default instead of enumerating what to include. An include list is unbounded — every directory, extension and config a package might adopt must be predicted — and each gap is silent and reports green. An exclude list is bounded, and its gaps cost only an unnecessary re-run. This is also what Turborepo's own docs prescribe:
inputsopts you out of all defaults, and$TURBO_DEFAULT$exists to re-enter them.The list being replaced had already been patched once along the EXTENSION axis and still failed along the DIRECTORY axis. Adding
tests/**would have fixed this case and left the next axis to be found the same way.Evidence
Same planted type error in
e2e/tests/canvas/poc-driver.ts, both configs, warm cache:cache hit, replaying logs e2f45f80054274ba→ Tasks: 1 successfulcache miss, executing 9a6450b5f310f129→ failed,error TS2322Hash movement, measured and reverted to baseline:
packages/tsconfig/base.jsone2e/tests/**tests/hashedMy first attempt at the negative control was worthless and I am recording that: the cache was cold, so the run executed and failed, which proves nothing about replay. A cold cache cannot demonstrate a replayed green. Redone with the cache warmed under the old config first, which is the table above.
The guard
scripts/turbo-inputs.test.mjsasserts the property from turbo's own resolved inputs rather than by reviewing globs — reading the patterns and judging them complete is the same act that produced the gap. Stub-verified: against the old config it fails naming all three defects (e2e 30 unhashed, plugin-page-builder 2, missing global tsconfig).It asserts the population by MEMBERSHIP before any verdict, because "no package has an unhashed module" is satisfied perfectly by a run that read nothing. Packages that legitimately ship no TypeScript (
tsconfig,eslint-config,prettier-config,admin-css) are handled there rather than by a per-package floor that would fail on all four.Scope and honesty
check-types --forceon a built tree: 22 of 22 pass. The failures visible on an unbuilt tree are the missing-diststate AGENTS.md describes, not this change.main, a false positive costs a re-run.plugin-page-builder/e2e/*.spec.tsare in no tsc program at all — a separate gap this does not fix. Now hashed, still unchecked. Worth its own task.restore-keys: turbo-v2-${{ runner.os }}-${{ github.job }}-) while the exact key carries the commit SHA, so the exact key never hits and every run restores from some earlier commit. That makes any future input-set gap silently cross-branch rather than confined to one lane. Reported by the verdict-gate lane.Prior art
AGENTS.mdalready documents the mechanism under "measure with--force". So this is not a discovery — it is an unenforced documented rule, whichAGENTS.mditself calls out: "A documented rule with nothing enforcing it is not a control, and filing a task is not installing one." This is the first time it cost something.No changeset: CI configuration and a test, nothing published changes.
A naturally occurring instance, found independently
The evidence above uses a planted error. The page-builder lane hit the same
mechanism by accident while merging
maininto #854, which is worth recordingbecause nobody constructed it:
That is the defect stated from the other direction: over the whole life of that
cache entry, the only thing that ever invalidated it was a file in the one
directory the globs happened to cover. Every change under
tests/— 30 modules,including the two that introduced the TS2339 this PR's sibling fixed — left it
untouched.