Skip to content

Commit bae19f0

Browse files
authored
fix(ci): harden PR-size exception governance (#657)
* fix(ci): harden PR-size exception governance * fix(ci): close PR-size governance control gaps
1 parent 9effdf0 commit bae19f0

5 files changed

Lines changed: 324 additions & 87 deletions

File tree

β€ŽREADME.mdβ€Ž

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
<img src="https://img.shields.io/badge/Storage-IndexedDB_v8-F59E0B" alt="IndexedDB v8">
1414
<img src="https://img.shields.io/badge/PWA-v3.0-5BB974?logo=pwa" alt="PWA v3.0">
1515
<img src="https://img.shields.io/badge/i18n-19_locales-2942_keys-0EA5E9" alt="i18n 19 locales β€” 2942 keys">
16-
<img src="https://img.shields.io/badge/Tests-7512%2B_%2F_602_files-22C55E" alt="7512+ tests / 602 files">
16+
<img src="https://img.shields.io/badge/Tests-7516%2B_%2F_602_files-22C55E" alt="7516+ tests / 602 files">
1717
<img src="https://img.shields.io/codecov/c/github/qnbs/WorldScript-Studio?logo=codecov&label=Coverage" alt="Codecov Coverage">
1818
<img src="https://img.shields.io/badge/License-MIT-22C55E" alt="License MIT">
1919
<img src="https://img.shields.io/github/actions/workflow/status/qnbs/WorldScript-Studio/.github/workflows/ci.yml?branch=main&logo=github" alt="CI Status">
@@ -511,7 +511,7 @@ The Settings β†’ AI panel shows a live GPU status badge with adapter details and
511511
| **Document Export** | docx + jszip | Word-compatible `.docx` generation (lazy-loaded) |
512512
| **PWA** | Service Worker + Web App Manifest v3 | Offline support, installability, Workbox chunking |
513513
| **i18n** | Custom React Context (`I18nContext.tsx`) | 2942 keys Γ— 19 locales (de/en/es/fr/it + ar/he/fa RTL Beta + ja/zh/pt/el/fi/sv/hu/is/eu/ru/ko Beta); EN fallback; `localStorage` persistence |
514-
| **Testing** | Vitest 4.x (7512+ tests / 602 files) + Playwright E2E | Unit/integration + cross-browser E2E; Stryker mutation (manual workflow) |
514+
| **Testing** | Vitest 4.x (7516+ tests / 602 files) + Playwright E2E | Unit/integration + cross-browser E2E; Stryker mutation (manual workflow) |
515515
| **Code Quality** | Biome (lint + format) + TypeScript 7 (tsgo) strict | `--error-on-warnings` in CI; zero `any` policy |
516516
| **Visualization** | Force-directed graph | Interactive character relationship network |
517517
| **Desktop** | Tauri v2 | Cross-platform installer; auto-updater via `latest.json` |
@@ -549,7 +549,7 @@ WorldScript-Studio/
549549
β”‚ β”œβ”€β”€ sw.js # PWA Service Worker
550550
β”‚ └── manifest.json # PWA Web App Manifest v3
551551
β”œβ”€β”€ tests/
552-
β”‚ β”œβ”€β”€ unit/ # Vitest unit tests (7512+ tests, 602 files) β€” count spans tests/, components/, packages/*/tests/, not just this folder
552+
β”‚ β”œβ”€β”€ unit/ # Vitest unit tests (7516+ tests, 602 files) β€” count spans tests/, components/, packages/*/tests/, not just this folder
553553
β”‚ β”‚ β”œβ”€β”€ ai/ # aiSmallModules, aiCoreFallbackPaths
554554
β”‚ β”‚ └── settings/ # WebLlmPanel, AiSections
555555
β”‚ └── e2e/ # Playwright specs + helpers.ts
@@ -714,7 +714,7 @@ The main pipeline is [`.github/workflows/ci.yml`](.github/workflows/ci.yml). Opt
714714
Raw bundle-budget ceilings (KB per uncompressed asset): entry **2500 KB**, vendor **6200 KB**, other JavaScript **2500 KB**, and WASM **30000 KB**.
715715

716716
**Current test metrics (2026-09-06, source-synchronized; CI remains authoritative for pass/fail):**
717-
- **7512+ unit tests** across **602 test files** β€” CI is authoritative for pass/fail
717+
- **7516+ unit tests** across **602 test files** β€” CI is authoritative for pass/fail
718718
- Coverage thresholds: lines β‰₯ 80 Β· branches β‰₯ 66 Β· functions β‰₯ 72 Β· statements β‰₯ 78 β€” enforced in CI (see Codecov badge for live metrics)
719719
- i18n: **2942 keys Γ— 19 locales** (en/de/fr/es/it + ar/he/fa RTL Beta + ja/zh/pt/el/fi/sv/hu/is/eu/ru/ko Beta)
720720

β€Žconfig/pr-size-exceptions.jsonβ€Ž

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@
7575
},
7676
{
7777
"id": "pr-596-532-e2e-startup-determinism",
78+
"status": "historical",
7879
"repository": "qnbs/WorldScript-Studio",
7980
"prNumber": 596,
8081
"baseRef": "main",
@@ -174,7 +175,7 @@
174175
"tests/unit/storage/idbCore.test.ts",
175176
"tests/unit/storage/idbResetGate.test.ts"
176177
],
177-
"reason": "PR #583 (issue #532 E2E startup/navigation nondeterminism) grew through iterative review convergence into a full async, generation/epoch-based IDB reset-quiescence contract covering every long-lived connection in the app (9 service modules plus the shared gate itself). The gate fails closed β€” beginIdbReset() rejects if any registered closer fails, after every closer has still had its chance to run, so wipeAllAppData() aborts before any database deletion on an unproven teardown β€” and a closer registered while a reset is draining joins that same awaited barrier instead of racing ahead of it as fire-and-forget. A centralized beginIdbOpenAdmission()/isIdbOpenStillValid() pair closes a further gap: an open that STARTS after a reset already bumped the generation would otherwise still match at completion, so every opener now also refuses to start a fresh open while a reset is in progress. Every reset closer also invalidates its own module's pending open flight (not just the cached database), so the first post-reset caller starts a genuinely fresh flight instead of reusing one already doomed to a generation-mismatch rejection. deleteAllIndexedDBDatabases() uses Promise.allSettled so a fast-rejecting deletion cannot release the gate while another deletion is still outstanding, and only targets database names it can prove it owns (exact KNOWN_DB_NAMES plus the worldscript-localfirst- prefix), never assuming ownership just because native enumeration returned a name. Also closes several stale-open-completion single-flight races via an identity token (proForgeHistoryStore, loraAdapterService, sceneRevisionService, logSinks, crossProjectIndexService, proForgeMemoryBank, DeadLetterQueue), a synchronous indexedDB.open() throw that could permanently memoize a rejected open promise (DeadLetterQueue), and a test-reset helper that swapped the fake IndexedDB factory without first releasing its own cached connection (loraAdapterService). persistProjectDoc() returns a distinct-identity transient NOOP when a reset is in progress (never the shared NOOP_PERSISTENCE singleton), so a local-first handle cached during an active reset is not mistaken for an intentional NOOP and reused indefinitely after the reset ends. This entry was fully recomputed (not carried forward) after #583 was actually rebased onto current main, which by then already carried PR #592's independent factory-reset persistence-admission/drain-coordinator fix (issues #591/#593) β€” the two PRs modify overlapping files (app/listenerMiddleware.ts, services/factoryResetService.ts, services/crossProjectIndexService.ts, tests/unit/factoryResetService.test.ts, tests/unit/listenerMiddleware.test.ts, tests/e2e/onboarding-entry-precondition.spec.ts) and were reconciled by layering both mechanisms: #592's isFactoryResetInProgress()/coordinator-draining gate (blocks new Redux-listener-level writes, drains in-flight ones) runs first inside wipeAllAppData(), then this PR's beginIdbReset() force-closes every other long-lived IDB connection the coordinators do not track. The rebase also revealed that PR #590 (merged earlier, unrelated) had already independently shipped the same locale-independent Settings/mobile-\"More\"-button navigation fix this PR originally introduced via components/SettingsView.tsx, components/settings/SettingsModals.tsx, components/settings/DataSection.tsx, components/Sidebar.tsx, and tests/e2e/helpers.ts β€” parallel convergent evolution left this PR's own changes to those five files fully superseded (zero net diff against current main), so they are correctly absent from allowedPaths below; every remaining path was independently re-verified as a genuine, currently non-empty diff. Final measured diff against current main: 65 governed files (84 incl. generated locale bundles), 1611 meaningful lines, 14 commits β€” exact measured ceilings freshly computed via check-pr-size.mjs itself, no speculative headroom. This recompute also found and fixed a latent bug in check-pr-size.mjs itself, exposed for the first time by an exception whose own ceiling (65/1611/14) legitimately exceeds TIERS.absolute (30/3000/15): when exceptionWithinLimits was true, evaluatePrSize() fell through to selectSeverity() against the fixed absolute tier instead of treating the exception's own ceiling as authoritative, so a fully-satisfied exception with a wide-enough scope would still report blocking:true β€” neither #539 (maxFiles:30, at the absolute tier's own boundary) nor #564 (maxFiles:3, well under it) had ever exercised this path. Fixed to short-circuit on exception.entry directly; a regression test covers a wide exception ceiling exceeding the fixed tier. Review-thread state was not re-verified as part of this recompute and must be checked live against the current PR before merge, independent of this file-scope/line-ceiling exception. allowedPaths lists all 84 changed paths, not just the 65 governed ones: check-pr-size.mjs's path-scope match (pathScopeMatch) requires every path in the raw, unfiltered diff to be present in allowedPaths for the exception to apply at all, even though the separate governed-file-count and meaningful-line-count checks (maxFiles, maxNonExemptMeaningfulLines) already exclude generated locale bundles via isGovernanceExcluded() β€” omitting the 19 public/locales/*/bundle.json paths here would make the exception silently fail to apply. The 17 sidebar.json files show only a trailing-newline diff: they lacked a final newline on main already, and Biome's format-on-commit hook re-adds one the moment any of them is staged for any reason in this PR β€” content is otherwise byte-identical to main, and this could not be avoided without skipping the pre-commit hook. No file outside this exact list is admitted; the ceiling is a one-time allowance for this PR's actual, fully-reviewed, freshly-rebased diff, not a new target. Identity note: PR #583 could not be reopened after its branch (fix/532-e2e-startup-determinism) was force-pushed during this same recompute -- GitHub permanently blocks reopening a closed PR once its head branch has been force-pushed or recreated. PR #596 was opened from the identical branch/commit as #583's successor with no further content change, so prNumber was updated from 583 to 596 to match (landed via #597). #596's own review then surfaced three further genuine, narrowly-scoped gaps in the already-touched files (app/listenerMiddleware.ts, services/ai/aiInferenceCacheService.ts, services/storage/idbResetGate.ts, plus their test files and a README metrics sync) -- a symmetric encryption-disable NOOP-discard case, an openPromise identity-clear matching the sibling services' established pattern, and a concurrent-beginIdbReset() barrier-join fix -- fixed in place by amending #596's own last commit (preserving the exact 14-commit count) rather than adding a 15th. This raised maxNonExemptMeaningfulLines from 1611 to 1741 (measured exactly via check-pr-size.mjs against the rebased branch); maxFiles (65) and maxCommits (14) are unchanged, and no path outside the existing allowedPaths list was touched, so that list and the recompute methodology below still apply verbatim. A second review wave (CodeAnt, CodeRabbit, cubic independently) then surfaced further genuine, narrowly-scoped gaps across docPersistence.ts (closer unregistered before its own teardown settled; a rejected destroy() was swallowed before the reset gate ever saw it), idbResetGate.ts (a closer registering after the drain loop already emptied was never invoked at all; a closer that itself registers another mid-drain could be double-invoked via the live Set iteration), idbCore.ts (concurrent initDB() callers before the first open resolved could each orphan a connection -- fixed with the same single-flight pattern the sibling services already use), factoryResetService.ts (deleteDatabase() rejected immediately on onblocked even though the same request can still reach a real onsuccess, settling before the true outcome was known -- now bounded by a timeout instead), sceneRevisionService.ts (missing the identity check loraAdapterService already had), plus a locale grammar fix, a dead data-testid removal, and several test-only wording/mock-leak fixes -- fixed as a new 15th commit (not amended, since this is a materially separate review round from the prior wave) plus a README metrics sync folded into the same commit. components/settings/FactoryResetDangerZone.tsx's data-testid removal reverted that file to byte-identical with main, so it drops out of the diff entirely; tests/unit/storage/idbCore.test.ts is a new regression-test file for the idbCore.ts fix. maxFiles stays 65 (one dropped, one added); maxCommits rises 14 -> 15; maxNonExemptMeaningfulLines rises 1741 -> 1900 (a same-review-cycle coderabbit follow-up on the idbCore.ts fix -- a live-handle guard missing alongside the single-flight one -- was folded into the same 15th commit rather than adding a 16th), all measured exactly via check-pr-size.mjs against the rebased branch. A subsequent codecov/patch failure (67.20% vs a 74.99% target) was root-caused by downloading and directly inspecting the actual CI-generated lcov.info artifact rather than trusting the Codecov dashboard alone: the gap was real (Codecov correctly counts partial-branch-coverage lines as not-fully-covered), not stale data, and traced to reset-in-progress-rejection/generation-mismatch/onversionchange/onerror branches left untested across most of the reset-gate's service modules by this PR's own earlier commits. Closed with test-only additions (no production code changed) as a new 16th commit: services/storage/idbCore.ts, services/crossProjectIndexService.ts, services/diagnostics/logSinks.ts, services/proForge/proForgeHistoryStore.ts, services/proForge/proForgeMemoryBank.ts, and packages/worker-bus/src/deadLetterQueue.ts. maxFiles rises 65 -> 70 (5 new test files), maxCommits rises 15 -> 16, maxNonExemptMeaningfulLines rises 1900 -> 2199 (a same-wave coderabbit follow-up replaced a fixed-microtask-count synchronization in two logSinks.test.ts reset-race tests with an observable indexedDB.open()/beginIdbOpenAdmission() spy, per the coding-guideline against tests coupled to unmocked timing internals -- net +6 lines), all measured exactly via check-pr-size.mjs against the rebased branch."
178+
"reason": "One-time PR #596 exception for reviewed issue #532 startup-determinism convergence; see PR #596 and issue #532."
178179
}
179180
]
180181
}

β€Žscripts/check-pr-size.d.mtsβ€Ž

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export interface SupplementalLineAllowance {
4040

4141
export interface PrSizeException {
4242
id: string;
43+
status?: 'active' | 'historical';
4344
repository: string;
4445
prNumber: number;
4546
baseRef: string;
@@ -72,7 +73,7 @@ export interface SizeTierLimits {
7273
commits: number;
7374
}
7475

75-
export type SizeTier = 'ok' | 'target' | 'hard' | 'docsGovernance' | 'absolute';
76+
export type SizeTier = 'ok' | 'target' | 'hard' | 'docsGovernance' | 'absolute' | 'exception';
7677

7778
export interface SizeSeverity {
7879
tier: SizeTier;

0 commit comments

Comments
Β (0)