mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 04:12:10 +03:00
494b1c961a
* fix(ci): stop the reconciliation range and the fragment sweep from hiding work Two release-tooling defects found during the v3.8.49 run (gaps 4 and 7 of the process dossier). Both fail by hiding work rather than announcing themselves, which is why each one had already cost a real mistake. ## The reconciliation range was 62× too wide `list-uncovered-commits.mjs` bounded its scan with `git describe --tags`. Releases reach `main` by SQUASH, so no commit on a release branch is ever an ancestor of the tag, and `vPREV..HEAD` re-lists the un-squashed history of every earlier cycle. Measured on release/v3.8.50 @7eca04fd12: v3.8.49..HEAD ....... 1361 commits cycle open..HEAD ..... 22 commits The report drowns in noise, and that is how a previous reconciliation let ~200 PRs through with no CHANGELOG bullet. The base is now resolved by CONTENT — the oldest commit that introduced this version string into package.json — deliberately NOT by commit subject, because the subject has already changed format once: chore(release): bump v3.8.49 (development cycle version) older chore(release): open v3.8.50 development cycle current A message-matching resolver would have silently reverted to the broken tag base the first time someone reworded the bump. The fallback now writes a WARNING to stderr explaining that a tag range re-lists previous cycles, so a shallow clone degrades loudly instead of quietly reproducing the bug. One of the five tests asserts exactly that the warning says "squash" and "noise". ## The back-merge resurrects fragments that already shipped The release lands on `main` as one squash commit, so `main` still carries every `changelog.d/` fragment the reconciliation folded in and deleted. Back-merging `main` restores all of them — 191 in the v3.8.49 run. Nothing breaks at that instant; the next aggregation folds them in a SECOND time and the section grows duplicates that have to be hand-unpicked. New `scripts/release/sweep-stale-fragments.mjs` (`npm run sweep:stale-fragments`) reports them, and `--apply` removes them. Report mode exits 1 so the back-merge step can gate on it. The identity rule took two attempts, and the second one exists because running the script against the live repo refuted the first. Matching on any `#N` in the bullet flagged `changelog.d/features/8980-deprecate-gemini-cli-provider.md` as stale, because that bullet cites issue **#7034** for context and #7034 shipped in an earlier cycle — it would have deleted an unreleased fragment and dropped its credit. A bullet routinely cites issues it merely references; only the `<PR-number>-<slug>.md` filename says which PR the fragment *is*. That case is now a regression test. Every ambiguous case resolves toward KEEPING: no number in the filename falls back to normalized text, text shorter than 12 chars is never matched, and anything matching neither is kept. A surviving duplicate is a nuisance someone notices; a deleted fragment silently costs a contributor their credit. node --import tsx/esm --test tests/unit/release-cycle-base-resolver.test.ts # 5 pass node --import tsx/esm --test tests/unit/sweep-stale-fragments.test.ts # 11 pass node scripts/release/list-uncovered-commits.mjs --json → baseed2db6cb19, baseSource "cycle-open", total 22 (was 1361) node scripts/release/sweep-stale-fragments.mjs → 4 fragments, 0 stale, exit 0 * docs(changelog): fragment for #8985 * fix(ci): four quality gates that punished the wrong thing Gaps 6, 9, 10 and 23 of the v3.8.49 process dossier. Each one either blocked work it should have waved through, or reported a number that was never the code's. ## 6 — test-masking is unusable at release scale My own dossier entry for this was WRONG and the measurement says so: tracked test files ............ 3977 (I had written 1277) absolute tautology scan ....... ~1 s (I had written >30 min) the diff uses base...HEAD three dots — already merge-base diff vs release branch ........ 0 files, 0 s diff vs main (today) .......... 3 files, 0 s The base choice was never the problem, and it cannot be reproduced today at all: `main` has since received the v3.8.49 squash, so the merge-base is recent. The pathology only exists DURING a release, in the window before `main` gets the squash — then the merge-base is the PREVIOUS cycle's fork point and the diff legitimately spans the whole cycle (~1277 changed test files, each costing a `git show` process plus a full regex pass). That is the same squash-merge topology as gap 4, and it is why the check ran twice without finishing. Fix: above 300 changed test files the per-file diff subchecks are skipped, since every one of those files was already gated by this check on its own PR. The absolute tautology scan still runs unconditionally over all 3977 files, so the floor is untouched. The skip is deliberately loud — a silent skip is gap 12, which cost two production bugs this cycle. `shouldSkipDiffSubchecks` never skips on unparseable input, so a broken count cannot disable the gate. ## 9 — a capital letter invalidated 41 translations `"Reset Defaults"` → `"Reset defaults"` marked the key stale in 41 locales. Every translation was still correct, and in locales with no letter case the "fix" is not expressible. Worse, the escape hatch (`__MISSING__:`) is BANNED in `vi` by tests/unit/i18n-vi-completeness.test.ts, so `vi` had no legitimate way out. `isCosmeticRewrite` folds case, whitespace runs and trailing punctuation — and nothing else. Most of the nine tests exist to pin what is NOT cosmetic: a changed word, an added word, and any edit inside an interpolation like `{count}` all still flag. Two end-to-end tests hold both directions: a cosmetic edit leaves every locale alone, a real rewrite still flags all of them. ## 10 — the ratchet compared numbers from two different auditors `pipx install zizmor` was unpinned, so the runner installed whatever PyPI served that day and measured 1 finding MORE than the devbox on the identical commit (190 vs 189) — a second rebaseline push per release, chasing a number that was never the code's. Pinned to 1.25.2 (what the devbox runs), and check-workflows.mjs now prints `zizmorVersion=` next to the count so any future rebaseline is traceable to the tool that produced it. ## 23 — a PR pointed at its own branch #8912 has head == base == release/v3.8.50: no diff, can never merge, and it sits in the queue with a full check board on every push to that branch. It survived because nothing looks wrong — the checks pass, since there is nothing to check. New guard in the `changes` job (one field comparison, before anything is spent). The distinction that makes it safe to block on: an equal head/base BRANCH is conclusive, an equal head/base SHA is NOT — a branch cut moments ago has an identical tip and is legitimate, so that case warns instead of failing. Half a signal never fails either. node --import tsx/esm --test tests/unit/test-masking-release-scale.test.ts # 6 pass node --import tsx/esm --test tests/unit/ui-value-drift-cosmetic.test.ts # 9 pass node --import tsx/esm --test tests/unit/pr-self-target-guard.test.ts # 7 pass check:workflows --ratchet → 178 findings, zizmorVersion=zizmor 1.25.2, baseline 190 the i18n suite is unaffected (5 files re-run, all green) * fix(ci): allowlist the four CI-only env vars the new gates read The env-doc-sync gate failed three unit shards plus Docs Gates on this PR, and it was right to: it requires every `process.env.X` read in code to be documented in `.env.example`, and this PR introduced four new reads. They do not belong in `.env.example`. That file is OmniRoute's runtime configuration; these are CI signals with no meaning in a user's `.env`: HEAD_REF / HEAD_SHA / BASE_SHA the `changes` job passes github.head_ref, github.base_ref and the PR head/base SHAs to the self-targeting-PR guard TEST_MASKING_MAX_CHANGED_TESTS the escape hatch that raises the test-masking gate's release-scale skip threshold So they go in IGNORE_FROM_CODE, which exists for exactly this and already carries the precedent one line above: `BASE_REF`, allowlisted because CI passes it to the OpenAPI breaking-change gate. `BASE_REF` being already listed is also why only four of my five reads failed. Each entry carries its justification and the script that reads it, per the allowlist policy. node --import tsx/esm --test tests/unit/issue-7793-env-doc-sync-repro.test.ts # 1 pass npm run check:env-doc-sync → all three directions in sync * fix(i18n): narrow the cosmetic-rewrite exemption to the scope actually reported The gap-9 fix folded whitespace in addition to case, and that collided with a pre-existing test which pins the opposite — tests/unit/i18n-ui-value-drift.test.ts, "a value that only changes whitespace still counts as an edit". Its comment states the reasoning: Conservative on purpose: trailing-space churn is rare, and treating it as a no-op would let a real reword slip through behind an innocuous-looking diff. That is a documented decision by whoever wrote it. The problem actually reported was CASE — `"Reset Defaults"` → `"Reset defaults"` invalidating 41 correct translations — and whitespace was scope I added on my own. Reversing someone else's reasoned call, silently, to fix something nobody reported is not this change's job, so the exemption is narrowed to case + trailing terminal punctuation. No test pins either of those. The reported case is still fixed, verified end to end: that rewrite invalidates 0 locales. And whitespace is now asserted NON-cosmetic in my own test file too, so a later tidy-up cannot quietly fold it back in. node --import tsx/esm --test tests/unit/i18n-ui-value-drift.test.ts # 11 pass (pre-existing) node --import tsx/esm --test tests/unit/ui-value-drift-cosmetic.test.ts # 10 pass --------- Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>