Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions config/pr-size-exceptions.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,12 @@
"baseRef": "main",
"headRef": "fix/532-e2e-startup-determinism",
"maxFiles": 65,
"maxCommits": 14,
"maxNonExemptMeaningfulLines": 1741,
"maxCommits": 15,
"maxNonExemptMeaningfulLines": 1900,
"supplementalLineAllowances": [],
"allowedPaths": [
"README.md",
"app/listenerMiddleware.ts",
"components/settings/FactoryResetDangerZone.tsx",
"hooks/useFactoryReset.ts",
"hooks/useSettingsView.ts",
"locales/ar/settings.json",
Expand Down Expand Up @@ -167,9 +166,10 @@
"tests/unit/settings/EncryptionRecoveryModal.test.tsx",
"tests/unit/settings/IdbUnlockModal.test.tsx",
"tests/unit/settings/SettingsModals.test.tsx",
"tests/unit/storage/idbCore.test.ts",
"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. 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."
"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."
}
]
}
Loading