Sign in or sign up before continuing. Don't have an account yet? Register now to get started.
Register now
Parallelized fleet cleanup of `let_it_be` declarations in preparation for `freeze: true` default
<!--IssueSummary start--> <details> <summary> Everyone can contribute. [Help move this issue forward](https://handbook.gitlab.com/handbook/marketing/developer-relations/contributor-success/community-contributors-workflows/#contributor-links) while earning points, leveling up and collecting rewards. </summary> - [Label this issue](https://contributors.gitlab.com/manage-issue?action=label&projectId=278964&issueIid=600267) </details> <!--IssueSummary end--> ## Problem We would like to flip `let_it_be` (test-prof) to default to `freeze: true` so that any spec that mutates shared state surfaces a `FrozenError` at the declaration site rather than producing flaky or order-dependent failures downstream. The 2021 attempt at this change ([!71373](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/71373)) stalled because the volume of opt-outs is too large for a single MR — see [note_696716822](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/71373#note_696716822). A current scan of `master` (FY27-Q2-M1) finds **~11,630 spec files** containing at least one `let_it_be(` call without an explicit `freeze:` / `reload:` / `refind:` modifier. The companion cop MR ([!234596](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/234596), `RSpec/LetItBeImmutable`) lists ~11.5k offending files in `.rubocop_todo/rspec/let_it_be_immutable.yml`, confirming the scope. We want to: 1. Identify, for each affected spec file, which specific `let_it_be(:NAME)` declarations would raise `FrozenError` if frozen-by-default were enabled. 2. Add `freeze: false` (the behavior-preserving opt-out) only to those declarations. 3. Land each fix as a focused MR against `master`. Each MR is a no-op against today's `master`; collectively they prepare the codebase for the eventual default flip. ## Approach We are building an automation harness that uses a small fleet of GCP VMs (initially 1 for a pilot, scaling to ~10) to parallelize the mechanical work. The harness reuses the VM provisioning, bootstrap, and result-collection infrastructure from the existing [`ai-distilled-principles-evaluation-harness`](https://gitlab.com/pedropombeiro/ai-distilled-principles-evaluation-harness). ### Per-spec-file loop (deterministic Ruby, no LLM) 1. Reset gitlab repo to a pinned `master` SHA. 2. Apply a **working-tree-only** patch that enables `let_it_be` freeze by default. This patch is never committed to any branch we push. 3. Run RSpec on the file with `--format json` to capture all `FrozenError` failures. 4. Use a Prism-based locator to map each `FrozenError` backtrace frame back to a `let_it_be(:NAME)` declaration in the file (including dependency chains: e.g. when `let_it_be(:award_emoji) { create(:award_emoji, awardable: note) }` fails, both `award_emoji` and `note` are candidates). 5. Use a Prism-based patcher to insert `freeze: false` at each offending declaration site, preserving existing positional/keyword args. 6. Re-run only the previously-failing examples (`rspec --example-matches`). Iterate up to 3 times if new `FrozenError`s surface. 7. If green: discard the freeze-default patch from the working tree, restore only the spec edits, branch from upstream `master`, commit a single change, push, and open one MR per spec file against `master`. ### MR shape Each MR: - Targets `master`. - Contains a single commit changing one spec file. - Adds `freeze: false` to specific `let_it_be` declarations identified by the FrozenError traces. - Includes a `<details>` block with the pre-fix FrozenError traces as reviewer-verifiable evidence. - References both !71373 and !234596 for rollout context. - Is **a no-op against current master** — the change is preparation for the eventual default flip. ### Why `freeze: false` and not `let_it_be_with_refind` The harness adds `freeze: false` (a behavior-preserving opt-out), not `let_it_be_with_refind` or `reload: true`. The latter would change runtime semantics (refind issues a `find` each example) and would require human judgment per case — defeating the deterministic fleet model. The cop ([!234596](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/234596)) accepts any of the explicit modifiers; migrating to the behaviorally-stronger options can happen incrementally afterward (e.g. via Arturo's existing [`rspec-order-and-db-cleanup` skill](https://gitlab.com/arturoherrero/opencode/-/blob/master/skills/rspec-order-and-db-cleanup/SKILL.md)). ## Rollout phases | Phase | Scope | Approach | Outcome | |---|---|---|---| | Pilot | 50 spec/models files, 1 VM | per-file MRs | Validated pipeline (8/50 MRs); per-file shape rejected as un-scalable | | CE pass 1 | ~4,777 files, 20 VMs | rollup MRs at depth=3 | 21 MRs; 19 superseded by pass 2 | | CE pass 2 | 1,407 re-run files, 20 VMs | rollup at depth=2 + consolidation | 17 consolidated rollup MRs (!236382–!236398) | | CE LLM triage | 671 `needs_human` files | Opus-4.7 fallback via opencode | 604 patched, 16 supplementary MRs (!236400–!236415) | | EE smoke | 50 files, 1 VM | rollup at depth=2 | 3 patches, 1 MR (!236422) | | EE pass 1 | 5,468 files, 20 VMs | rollup at depth=3 | 598 patches, 16 MRs (!236456–!236471) | | EE LLM triage | 594 `needs_human` files | Opus-4.7 fallback | 573 patches, 14 MRs (!236485–!236499) | Detailed numbers and methodology live in [the harness repo](https://gitlab.com/pedropombeiro/let-it-be-frozen-cleanup-harness). ## Implementation status The harness is published at [`pedropombeiro/let-it-be-frozen-cleanup-harness`](https://gitlab.com/pedropombeiro/let-it-be-frozen-cleanup-harness). Components: - `lib/freeze_error_patcher.rb` — Prism-based, idempotent spec rewriter - `lib/frozen_error_locator.rb` — RSpec-JSON → `let_it_be` name resolver - `lib/rollup_aggregator.rb` — per-subtree rollup builder with split-on-cap - `lib/cell_runner.rb` — deterministic per-file loop (up to 3 iterations) - `bin/run_cleanup_remote` — 20-VM GCP orchestrator (uses IAP, GDK golden image) - `bin/aggregate_rollup` — assembles rollup branches from per-VM staged payloads - `bin/push_existing_rollup_mrs` — pushes branches + creates MRs without re-aggregating - `bin/llm_triage` — Opus-4.7 fallback for `needs_human` cases - `bin/assign_mr_reviewers` — reads danger-bot roulette comments, assigns + sets milestone - `bin/mr_status_report` — dashboard across all open rollup MRs - `skills/triage-needs-human/` — Agent Skill v1 spec for the LLM step - `skills/let-it-be-frozen-cleanup/SKILL.md` — top-level reference All 80 unit specs pass. End-to-end validation: 4,777 CE + 5,468 EE files processed; ~1,775 patches across ~66 open MRs. ## References - !71373 — original freeze-default attempt (2021) - !234596 — `RSpec/LetItBeImmutable` cop (current rollout) - Arturo's [`rspec-order-and-db-cleanup` skill](https://gitlab.com/arturoherrero/opencode/-/blob/master/skills/rspec-order-and-db-cleanup/SKILL.md) and [`rspec-database-queries` skill](https://gitlab.com/arturoherrero/opencode/-/blob/master/skills/rspec-database-queries/SKILL.md) — prior art for per-file, sequential, MR-batched spec cleanup workflows. - [test-prof `LetItBe` state-leakage detection](https://github.com/test-prof/test-prof/blob/master/docs/recipes/let_it_be.md#state-leakage-detection) - [Testing guide: Let's talk about `let`](https://docs.gitlab.com/development/testing_guide/best_practices/#lets-talk-about-let)
issue