diff --git a/config/pr-size-exceptions.json b/config/pr-size-exceptions.json index e92ade5b..cdedd6d4 100644 --- a/config/pr-size-exceptions.json +++ b/config/pr-size-exceptions.json @@ -74,9 +74,9 @@ "reason": "One-time S5 R-15 normative secure-storage contract whose size grew through iterative security, durability, migration, recovery, and data-integrity review convergence. The exception is restricted to the single contract document plus its two minimal ledger/classification integrations; no executable or production source changes are admitted." }, { - "id": "pr-583-532-e2e-startup-determinism", + "id": "pr-596-532-e2e-startup-determinism", "repository": "qnbs/WorldScript-Studio", - "prNumber": 583, + "prNumber": 596, "baseRef": "main", "headRef": "fix/532-e2e-startup-determinism", "maxFiles": 65, @@ -169,7 +169,7 @@ "tests/unit/settings/SettingsModals.test.tsx", "tests/unit/storage/idbResetGate.test.ts" ], - "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." + "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; every other figure in this entry (65/1611/14, the allowedPaths list, and the recompute methodology) is unchanged and still applies verbatim." } ] }