Lock test layout ownership before migration - #3984
Merged
Merged
Conversation
The first taxonomy slice needs a code-enforced inventory before later PRs move files or narrow runner tasks. This adds the canonical leaf-suite registry, a path classifier, and a temporary migration-count ratchet so every current executable test has exactly one owner without changing runner selection. Constraint: Runtime is represented as one runtime suite with node and bun variants, not separate competing owners. Constraint: This PR must inventory the existing tree without moving tests or changing runner commands. Rejected: Per-file migration inventory | too large for the first taxonomy slice and harder to review than prefix counts with ratcheted totals. Rejected: Grep-based validation | would not exercise the classifier behavior consumed by later PRs. Confidence: high Scope-risk: narrow Directive: Shrink or split TEST_LAYOUT_MIGRATION_ENTRIES as tests move into canonical leaves; do not grow counts without a deliberate taxonomy decision. Tested: npx --yes --package=deno@2.7.7 --call 'deno test --config=scripts/test.deno.json --no-check --allow-read scripts/test/suites.test.ts scripts/test/test-layout.test.ts scripts/test/run-test-file.test.ts' Tested: npx --yes --package=deno@2.7.7 --call 'deno task test:layout' Tested: npx --yes --package=deno@2.7.7 --call 'deno task fmt:check' Tested: npx --yes --package=deno@2.7.7 --call 'deno task lint' Tested: npx --yes --package=deno@2.7.7 --call 'deno task typecheck' Tested: npx --yes --package=deno@2.7.7 --call 'deno task verify:quick' Not-tested: Full verify/test suites beyond verify:quick; this slice is taxonomy-only.
The first taxonomy slice needs a fail-closed ownership registry that later movement PRs can shrink. This amends the layout gate to treat runtime as variant metadata, emits concrete inventory records, and replaces broad migration prefix counts with explicit temporary violation paths. Constraint: Task 1 must not move tests or alter runner selection Constraint: Controller ruling requires finite migration paths carrying owner and removal PR Rejected: Keep prefix/count migration ownership | it can bless new off-layout tests by increasing a committed count Rejected: Model tests/unit as canonical unit root | approved contract keeps unit tests colocated in source roots Confidence: high Scope-risk: narrow Directive: Delete migration entries as files move; do not add broad prefixes or per-file canonical manifests Tested: deno test --config=scripts/test.deno.json --allow-read --allow-write scripts/test/suites.test.ts scripts/test/test-layout.test.ts Tested: deno task test:layout Tested: deno check --config=scripts/test.deno.json scripts/test/suites.ts scripts/test/test-layout.ts scripts/test/test-layout-migration.ts scripts/test/suites.test.ts scripts/test/test-layout.test.ts Tested: deno task fmt:check Tested: deno task lint Tested: deno task verify:quick Not-tested: deno task test:scripts remains blocked by missing npm/esm/cli/commands/mcp/handler.js artifact outside this layout slice
The explicit migration list needs a historical ratchet, not only per-entry metadata. The layout gate now compares current migration paths to a Git-resolved base path set, fails on additions, allows shrinkage, and preserves the initial seed when the base lacks the migration file. Constraint: Initial Task 1 seed is valid because origin/main has no test-layout migration file Constraint: CI must pass event base evidence without changing unrelated checkout depth Rejected: Validate only current explicit entries | adding a matching entry would still bless new off-layout tests Rejected: Execute historical TypeScript from the base ref | validation should inspect evidence without running old code Confidence: high Scope-risk: narrow Directive: Keep migration paths as explicit executable string literals until the temporary inventory is removed Tested: deno test --config=scripts/test.deno.json --allow-read --allow-write scripts/test/test-layout.test.ts Tested: deno test --config=scripts/test.deno.json --allow-read --allow-write scripts/test/suites.test.ts scripts/test/test-layout.test.ts Tested: deno task test:layout Tested: deno check --config=scripts/test.deno.json scripts/test/suites.ts scripts/test/test-layout.ts scripts/test/test-layout-migration.ts scripts/test/suites.test.ts scripts/test/test-layout.test.ts Tested: deno task fmt:check Tested: deno task lint Tested: deno task test:scripts Tested: deno task verify:quick Not-tested: GitHub merge_group payload live execution
|
Warning Review limit reached
Next review available in: 30 minutes Limit details: You’ve used the included review currently available. 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?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
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 |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
![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: fea18cea0a
ℹ️ 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".
Late review exposed three boundary errors: E2E runner ownership conflated Deno journeys with Playwright, the migration ratchet could not represent its terminal empty state, and manual CI fetched a local tracking ref as though it were remote. The ratchet now reads only the exported syntax tree, so comments and templates cannot become baseline evidence. Constraint: Historical baselines must be inspected without executing repository code Rejected: Raw string-literal scanning | comments and templates can masquerade as migration paths Rejected: One Playwright owner for tests/e2e | Deno critical-flow tests share that tree Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep future migration inventory shapes syntax-readable by parseMigrationBaselineSource and pin them with parity tests Tested: Deno 2.7.7 verify:quick; 151 script tests / 484 steps; 12 live tool-search tests; focused adversarial parser tests Not-tested: Live workflow_dispatch and merge_group event execution
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Design boundary
This is PR 1 of a 12-stage, 25-PR migration. Every PR must be green and internally consistent when merged after its declared predecessors. Later slices rely on the ownership contract introduced here, but this PR has no dependency on another unmerged migration PR.
Verification
deno task test:layout: 2,338 executable tests; 2,287 canonical; 51 migration entriesdeno task verify:quickStaging
Not applicable: this PR changes repository taxonomy and CI validation only; it has no deployed runtime surface.
Follow-up
PR 2 will add
scripts/test/run-suite.ts, route existing selection and sharding through this registry, and prove exact selected-case parity without moving tests.Related to #3974.