PT-4159: auto-sync blocking overlay (edit block for scheduled S/R)#2555
Merged
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rolfheij-sil
requested review from
irahopkinson,
lyonsil and
tjcouch-sil
as code owners
July 14, 2026 12:56
While the auto-sync blocking overlay is visible, subscribe to the Send/Receive extension's paratextBibleSendReceive.onSyncProgress network event and display PT9-style live progress under the label: a status text plus a determinate progress bar (platform-bible-react Progress, 0-1 payload fraction scaled to its 0-100 range). Indeterminate progress (progressValue null/undefined) is shown as text only because the shadcn Progress component has no indeterminate visual. The subscription is gated on blocking being visible and the progress state is cleared when blocking ends so the next episode starts clean. The payload's null is normalized to undefined at the subscription boundary. The legacy project-switch overlay passes no progress props and its rendered output is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ndings) - handleBlur no longer re-contains focus when it moves into a layer that renders above the overlay (a [data-overlay-host] subtree or an [aria-modal="true"] element), so a modal opened mid-sync keeps focus instead of ping-ponging with our re-containment. Non-modal escapes (relatedTarget null / body / the covered editor) are still re-contained, and the microtask fallback applies the same yield check to document.activeElement (Radix moves dialog focus asynchronously). - In cancel mode the container now uses role="dialog" + aria-modal="true" + aria-label (from the label already rendered), replacing role="status", which stays for the legacy passive project-switch overlay. The legacy render is unchanged when the new props are absent. - Storybook: add WithCancel, WithDeterminateProgress, and WithIndeterminateProgress variants for the new presentational props. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d test isInHigherFocusLayer matched [data-overlay-host] as well as [aria-modal], but every overlay-host layer (popover, context menu, command palette, modal dialog) portals its focusable content to document.body, so nothing focusable is ever a DOM descendant of [data-overlay-host]. Worse, three of those four layers render at Z_INDEX 400, below this overlay's 499 — if a future layer ever stopped portaling, the clause would wrongly yield focus to something this overlay should be covering. Only the modal dialog genuinely renders above (Z_INDEX_MODAL, 500) and it already carries aria-modal="true". Narrow the selector to aria-modal only and rewrite the doc comment to state that real invariant. Add a test for the microtask-only yield branch: a blur with a null relatedTarget (so the synchronous check can't see the modal yet) where focus lands inside an aria-modal container before the queued microtask re-checks document.activeElement — the overlay must not re-contain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rework the auto-sync block from a full-workspace focus-trapping overlay
into an editing-only block. The store and network-event service are
unchanged (they still provide the 200 ms grace, ref-count, and safety
timeout); only what "blocking" does to the UI changes.
- Add auto-sync-edit-block-driver: subscribes to the surviving blocking
store and, on visibility change, sets isSyncBlocked on every open
Scripture editor's web view state (getAllOpenWebViewDefinitionsSync +
updateWebViewDefinitionSync). While blocking it also flags editors
opened mid-block via onDidOpenWebView. A missing flag normalizes to
false so init/unblock never write redundantly. Unit tested.
- app.component: drop the <AutoSyncBlockingOverlay/> mount, init the
driver instead; keep initAutoSyncBlockingService().
- Delete the auto-sync overlay component and its focus-trap/dialog tests.
- Revert overlay-workspace-updating.component + stories to their legacy
{ label }-only presentational shape (shared with the project-switch
overlay); drop the cancel/progress/focus-containment additions.
- Remove the now-unused %overlay_autoSyncBlocking% string (en + es).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Consume the driver's isSyncBlocked web-view state in the Scripture editor: fold it into isReadOnlyEffective so scripture text is frozen, and block comment/note creation too (that path is gated by canUserCreateComments, not read-only) — the context-menu item, the insert-comment hotkey, and the note-caller click path. - Add SyncBlockedBanner: a slim, non-covering strip across the top of the editor pane, shown only when isSyncBlocked && !isReadOnly (a genuine viewer shouldn't say "editing paused"). It shows localized text, live sync progress (paratextBibleSendReceive.onSyncProgress — the same public seam the old overlay used), and a single-shot Cancel that sends paratextBibleSendReceive.cancelSync (disabled after click, re-armed on rejection). Reuses %general_cancel%. - Rejection handling: alongside the existing permissions revert-and-notify path, recognize the backend write-gate's (SR_EDIT_BLOCKED) sentinel and revert with a distinct "editing paused during Send/Receive" warning. - main.ts factory: always rebuild saved state with isSyncBlocked: false so a crash/reload mid-sync can never persist a read-only editor. - New en + es strings for the banner and the rejection notification. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add SendReceiveWriteLock: a process-wide, thread-safe registry of the projects currently being synced by an automatic Send/Receive, so an editor write can never race the sync's on-disk file replacement. Inert in open-source Platform.Bible: nothing in public core calls SetSyncing, so IsBlocked always returns false and no write is ever rejected — public behavior is unchanged. The Paratext 10 Studio patch activates it by bracketing each automatic sync with SetSyncing/Clear (follow-up). Project IDs are case-normalized; the blocked set is an immutable set behind a volatile reference (lock-free reads). Wire ThrowIfSyncBlocked() into every project write choke point in ParatextProjectDataProvider (scripture USFM/USX setters, project settings, extension data, and the comment mutations create/add/update/ delete/resolve). When blocked, the write throws with a message ending in the exact sentinel " (SR_EDIT_BLOCKED)" that the editor recognizes. NUnit tests cover the registry (set/clear/case-insensitive/replace/ null-safe/throw-with-sentinel) plus one gate test proving SetChapterUsfm is rejected while syncing and succeeds after Clear. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adversarial review of the write-gate found two PAPI-registered services that write directly to ScrText/project files with no SendReceiveWriteLock check. Gate them at the wire-method entry (before any mutation or orchestrator call), consistent with the PDP gates: - ManageBooksService: deleteBooks, createBooks, importBooks (target project), copyBooks and copyCustomVersification (destination project — the only project written; the source is read-only). The review named the first four; copyCustomVersification is gated too because it mutates the destination's custom.vrs and versification table. - InventoryDataProvider: setInventoryOptionValues (writes project settings via SetSetting/RemoveSetting/Save) and setInventoryItemStatus (the review named only the former, but inventory.Save() persists valid/invalid items into the project's checking settings, so it is gated too). Extend SendReceiveWriteLockGateTests so every reachable gated method is called once while SetSyncing is active, asserting the sentinel-suffixed rejection: all 11 ParatextProjectDataProvider write methods and the 5 mutating ManageBooksService wire methods (16 gate tests total, incl. the original round-trip). The two InventoryDataProvider setters are private (reachable only via PAPI wire dispatch), so they get no direct wiring test; the gate line itself is covered by the registry tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TS review findings on the edit-block rework: - onCommentEditorSave was ungated: a comment popover opened before the block began is not closed by it, and Save flowed to the backend's CreateComment, whose (SR_EDIT_BLOCKED) rejection was only logged — no user feedback, popover stuck open, pending highlight lingering. Fix both layers: early-return with the localized sync-blocked warning when isSyncBlocked (popover stays open so the typed text isn't lost), and an SR_EDIT_BLOCKED branch in the catch (an exact-moment race past the guard) that shows the same warning and discards the pending state via onCommentEditorCancel — matching the scripture-path's revert-and-notify pattern. - Factory-guard comment reworded: isSyncBlocked:true does persist into the saved layout during a block; what the guard prevents is RESTORING a read-only editor from it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The driver import added to app.component.tsx transitively evaluates theme.service-host's module-scope window.matchMedia call, which jsdom does not implement, failing the suite at import time. Mock the driver at the module boundary, matching share-layout.dialog.test.tsx. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the counter-based write gate with a wrapper over ReaderWriterLockSlim (NoRecursion), inverting the usual roles: a project write takes the RWLS read lock (many can run at once, the "reader" side), and Send/Receive takes the write lock (exclusive, the "writer" side). This keeps EnterWrite's fail-fast contract exact while adding a real drain instead of a counter that could never distinguish "the last writer just left" from "a writer is still mid-mutation". - EnterWrite(projectId): TryEnterReadLock(0) — zero timeout, so a user's write is never queued behind a sync. On failure, throw the existing "(SR_EDIT_BLOCKED)" sentinel. On success, a belt-and- suspenders check re-consults the armed project-id set (covers the window where a sync proceeded unheld after a drain timeout) and rejects there too. Dispose releases the read lock. - SetSyncing: swaps the armed set first (it stays pure data for IsBlocked and the rejection message), then TryEnterWriteLock with a bounded 10s timeout — that bounded wait *is* the drain of in-flight writes, since a writer-preferring RWLS blocks new readers from the moment it queues. On timeout it warn-logs and proceeds unheld (a private held-flag records this so Clear only calls ExitWriteLock when actually held); a repeat SetSyncing without an intervening Clear skips re-acquiring an already-held write lock. Clear disarms the set and releases the write lock if held. - Documented at length: both directions of the gate, why the raw RWLS isn't exposed (thread affinity, block-by-default readers vs. our fail-fast requirement, and "read lock for a write" reading confusingly at call sites), the same-thread contract for SetSyncing/Clear (violations throw SynchronizationLockException — desirably loud), and how this in-process gate differs from the S/R server-side repository lock (lockrepo/unlockrepo). - Converted all 11 ParatextProjectDataProvider write methods, the 5 mutating ManageBooksService wire methods, and the 2 InventoryDataProvider setters to `using var _ = SendReceiveWriteLock.EnterWrite(projectId);` as their first statement, so each scope covers the whole mutation. SetBookUsx now calls a new un-gated SetBookUsfmInScope instead of the public SetBookUsfm, avoiding a same-thread double-EnterWrite (which NoRecursion would reject). - Reworked SendReceiveWriteLockTests.cs for the new call shape: armed → EnterWrite throws with the sentinel; an in-flight write held on another thread → SetSyncing blocks until it disposes; a drain timeout → SetSyncing proceeds unheld and the belt check still rejects; Clear() is a safe no-op when never armed; a same-thread SetSyncing/Clear round-trip works. The 16 existing gate-wiring tests now arm via a small FakeSync background-thread helper (arming and writing on the same thread would hit NoRecursion instead of the intended sentinel). Still inert in public core: nothing calls SetSyncing here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The RWLS-backed SendReceiveWriteLock (rebuilt earlier this branch) is only useful if every new project-mutating code path actually calls EnterWrite. Nothing previously said so, and nothing would catch a miss. - CLAUDE.md: add a "Send/Receive Write Gate" section stating the rule (gate any ScrText/Settings/FileManager/comment/extension-data mutation with `using var _ = SendReceiveWriteLock.EnterWrite(projectId);` at its entry point), both directions of the gate in one sentence, and that it is distinct from the S/R server-side repository lock. - c-sharp-tests/Projects/SendReceive/SendReceiveWriteLockCoverageTests.cs: a coarse file-level scan for direct write patterns (PutText, Settings.Save/SetSetting/RemoveSetting, FileManager.Delete) in c-sharp/ .cs files outside an explicit allowlist. Resolves the source directory by walking up from the test assembly location (path-separator-safe, no hardcoded relative-path depth), scans files in deterministic order, and matches against full file text (not per-line) so formatter-wrapped calls are still caught. Allowlist: the three already-gated services (ParatextProjectDataProvider, ManageBooksService, InventoryDataProvider) plus four ManageBooks orchestrator/helper files (ImportBooksOrchestrator, ScriptureTemplateService, CopyBooksOrchestrator, DeleteBooksOrchestrator) whose write patterns are today only reached through one of those services' gated wire methods but aren't gated at their own entry point — each marked `// TODO(PT-4210): assess` rather than gated here, per instructions not to gate speculatively. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Concurrency-review follow-ups on the Send/Receive write gate. - CheckRunner.DenyCheckResult/AllowCheckResult (papi "denyCheckResult"/ "allowCheckResult") persisted ErrorMessageDenials into the project folder (denials.Save()) with no gate, so a Checks-UI click could write mid-sync. Both are synchronous (no await/Task.Run) and call no other gated method, so they are safe to bracket: added `using var _ = EnterWrite(projectId);` as the first statement of each, plus wiring tests (armed => throw with the SR_EDIT_BLOCKED sentinel) invoked via reflection since the methods are private and CheckRunner is sealed. - SendReceiveWriteLockCoverageTests: broadened the scan to match what the CLAUDE.md rule promises — a general `.Save(` heuristic (subsumes Settings.Save; catches ScrText.Save, denials.Save, inventory.Save, XDocument.Save), comment persistence (SaveUser / CommentEditHelper. SaveEdits) and raw File.Delete, alongside the existing PutText / SetSetting / RemoveSetting / FileManager.Delete. Allowlist now carries a one-line reason per entry (gated / TODO(PT-4210) / not-project-data) and gains CheckRunner (gated) plus UserProjectSettings, MarblePackage Discoverer and RawDirectoryProjectStreamManager (not shared project data the S/R merge replaces). Verified detection: an injected ungated .Save() in a non-allowlisted file fails the scan; clean tree passes. - Documented the thread-affinity constraints the gate relies on: a gated method must hold its EnterWrite scope synchronously (no await/Task.Run across it — the RWLS read lock is thread-affine), and SetSyncing/Clear must run on one thread. Added to CLAUDE.md and the EnterWrite/SetSyncing/ Clear docstrings. - SetSyncing/Clear: record the arming thread and have Clear throw InvalidOperationException on a cross-thread call (defends the held-flag on the degraded path, where the RWLS holds nothing to reject). Added a test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…startup Both initAutoSyncBlockingService() and initAutoSyncEditBlockDriver() were defined and tested but never called anywhere, so the edit-block never engaged at runtime — live E2E showed typing going through during a scheduled Send/Receive. Call both once in src/renderer/index.tsx after the network service and the webview/notification service block are up (their dependencies), blocking service first, then the driver.
lyonsil
reviewed
Jul 15, 2026
lyonsil
left a comment
lyonsil
left a comment
Member
There was a problem hiding this comment.
Automated max-effort review of this branch (10 finder angles + independent verification), filed as inline comments.
The core RWLS write-gate design is sound — I verified no same-thread gated re-entrancy (NoRecursion won't throw in normal operation), consistent lock ordering, a race-free fail-fast/belt-check, and that the driver covers hidden tabs and doesn't clobber live editor state.
The comments below are wiring / lifecycle / edge-case / coverage issues. Most are activation-dependent (the gate and the blocking event are inert in open-source core until PT-4210 wires up SetSyncing); finding 1 (double-init) affects core today. Highest priority: 1 (double-init), 2 (rebuilt editors unblock mid-sync), 4/5 (the backend gate can wedge or leak the write lock with no recovery).
A few earlier candidates were dropped after verification: a Console.Error.WriteLine concern — that is the C# logging convention and stderr is captured into the common log; a ~200 ms comment-loss window — acceptable; and a cross-language sentinel-constant concern — no codegen exists to keep such constants in sync anyway.
(AI-assisted, with my guidance)
lyonsil
reviewed
Jul 15, 2026
lyonsil
reviewed
Jul 15, 2026
lyonsil
reviewed
Jul 15, 2026
4 tasks
* PT-4159: rebuild S/R write gate without thread affinity Replace the ReaderWriterLockSlim-based SendReceiveWriteLock with a single-atomic-word design: an armed flag plus an in-flight write count packed into one long, every transition an Interlocked RMW on that word. The exclusion invariant now rests on single-location total ordering (a write can only enter by atomically observing not-armed while incrementing; arming atomically sets the flag), with no cross-variable fences, no held-lock bookkeeping bool, and no thread-affine state. Motivation (concurrency review follow-ups): the RWLS bracket required SetSyncing/Clear on one thread with no await between them - a contract the closed-source PT-4210 activation could not be forced to honor - and every violation ended in a permanently held write lock (process-wide edit block until restart). The new gate is forgiving by design: - SetSyncing/Clear may run on any threads; an await across the bracket is safe; Clear is idempotent and doubles as crash recovery - write scopes may cross awaits and dispose on another thread - nested EnterWrite is benign (no NoRecursion crash trap) - double-dispose and over-release are guarded no-ops - null/empty ids in a batch are ignored; the set is built before any state is touched (no torn arm); armed-ness is an explicit flag, so the empty-batch degraded-path hole is gone Semantics preserved: fail-fast rejection with the (SR_EDIT_BLOCKED) sentinel, bounded 10s drain of in-flight writes, degraded proceed-on- timeout with new writes still rejected, global gate, per-project IsBlocked, activation API (SetSyncing/Clear) unchanged, inert in public core. Tests: TDD - four new-contract tests written first and observed failing against the RWLS implementation (LockRecursionException, cross-thread Clear throw, cross-thread dispose SynchronizationLockException, null-id NRE), then green after the rewrite. Two old-contract tests removed, FakeSync helper no longer needed, recovery-while-stuck test added. Full c-sharp suite: 1503 passed / 0 failed (baseline 1500; delta is the net new tests). Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * PT-4159: harden S/R write gate: drain cancel, Clear(token), doc fixes Address the confirmed/plausible findings from the multi-agent review of PR #2564: - Drain now uses SpinWait.SpinUntil and ends early when the arm is cancelled (Clear) or replaced (newer SetSyncing) mid-drain, instead of burning the full DrainTimeout on a broken premise; the timeout warning now reports the stuck in-flight count. - SetSyncing returns an arm token (8-bit generation packed into bits 33-40 of the same atomic word); new Clear(token) disarms only the bracket that owns the sync slot, so a late/stray Clear from an overlapping bracket is a logged no-op instead of silently disarming a live sync. Parameterless Clear() stays as the force/crash-recovery path. - New optional SetSyncing(ids, throwOnDrainTimeout): when true, a drain timeout rolls the arm back (nothing stays armed, writes flow again, no caller cleanup owed) and throws TimeoutException instead of proceeding degraded, so the sync scheduler can retry/defer/notify. Default false preserves the existing degraded-proceed semantics. - An all-invalid (null/empty) SetSyncing batch still arms fail-safe but now logs a warning naming the IsBlocked incoherence. - Corrected the false "nesting is benign / gate is re-entrant" claim at every doc site (class remarks, EnterWrite, CLAUDE.md, ParatextProjectDataProvider): nesting throws mid-mutation if a sync arms while the outer scope is open; one scope per mutation is required. Added a pinning test for the armed-nesting rejection. - Tightened the "can never wedge" remark to carve out leaked scopes; added internal ResetForTests (full state reset incl. count) used by test SetUp/TearDown to contain a leaked scope's blast radius. - Test falsifiability: dispose/double-dispose/cross-thread-dispose and null-id-batch tests now assert InFlightWriteCount/ArmedProjectIds (the old re-enter/DoesNotThrow forms could not fail); the straggler dispose after a degraded Clear is now asserted; a new test (observed red before the fix) pins the prompt return on Clear-mid-drain; the cross-thread Clear test now asserts the armed premise before Clearing. - Doc staleness: refreshed the stress-test summary to the atomic-word design (no more read/write-lock wording) and dropped the dangling "queued" from the CLAUDE.md gate section. Full c-sharp suite: 1510 passed / 0 failed / 6 skipped (+7 new tests). Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Session-URL: https://claude.ai/code/session_01K7mGuL6RtKen6Rm94KWRP6 * PT-4159: address external review of the write-gate hardening Fixes from the independent whole-PR review (pr-2564-review-findings.md): - Token 0 is never issued: the generation skips 0 on wrap, so callers can safely treat default(long) as a "no arm" sentinel (finding #1); the generation field is widened from 8 to 30 bits (bits 33-62), making a stale-token false-match need ~10^9 intervening arms (#7). New wrap test parks the generation at MaxGeneration via a ResetForTests overload and asserts the wrapped arm skips 0 and still round-trips (#15). - Drain no longer busy-burns a core: SpinWait.SpinUntil internally disables Sleep(1) escalation (measured ~40x the CPU of a SpinOnce loop over a 500 ms wait; worse on Windows) — replaced with an explicit SpinWait.SpinOnce() deadline loop that settles at ~1 ms polling, keeping the disarm/replaced re-check; comment now states the escalation accurately (#3). - Stress test de-vacuumed: the arm/replacement windows use Thread.Sleep instead of Thread.SpinWait (which starved writers of the core on small CI runners), a rejections-during-replacement counter asserts writers actually contended inside the windows that matter, and the Task.WaitAll result is asserted (#5, #18). - Degraded-path test now asserts a LOWER time bound too, so a regression that skips the drain wait entirely cannot pass (#6). - Doc precision: the pure-data set's disagreement window under out-of-contract overlap is stated as potentially bracket-long, not "transient" (#9); the Volatile.Read comment now states the real guarantee (BCL long overload is interlocked on 32-bit) instead of the ambiguous atomicity claim (#11); bare <see cref="Clear"/> qualified (#20a); stale "TearDown Clear" wording fixed (#20b). - Cleanup: EmptyProjectIds is a cached static readonly field (declared above its reader; the property allocated per access) (#12); GenerationOf/CountOf helpers replace inline bit-decodes and the drain-failure message fragment is shared between the throw and warn paths (#14). Declined with rationale (see PR discussion): #2/#10 documented contract, #4 runtime nesting guard (partial coverage; revisit at PT-4210 activation), #8 Monitor redesign (decided), #13 parameter design (decided), #16 covered via the stale-token test, #17 the upper bound guards drain-condition regressions and stays, #19 acknowledged weak by the review. Full c-sharp suite: 1511 passed / 0 failed / 6 skipped (+1 new test). Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Session-URL: https://claude.ai/code/session_01K7mGuL6RtKen6Rm94KWRP6 --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Finding 1 (double initialization): initAutoSyncBlockingService and initAutoSyncEditBlockDriver were wired up both in renderer/index.tsx and in app.component.tsx's Main useEffects, giving two event subscriptions and two driver instances. If Main unmounted after a raise, only its cleanup ran and editors stayed read-only until the 10-min safety timer. Initialize each in exactly one place (index.tsx, like every other renderer service) and remove the app.component.tsx wiring. The app.component test's edit-block driver mock (a jsdom matchMedia workaround for the now-removed import chain) is dead, so remove it; the smoke test still passes. Finding 2 (rebuilt editors silently unblock mid-sync): the driver re-applied isSyncBlocked via onDidOpenWebView but never onDidUpdateWebView, so an in-place rebuild during a sustained block (reloadWebView / interface-mode switch / loadLayout) came back editable with the banner gone. While blocking is active, also subscribe onDidUpdateWebView and re-flag Scripture-editor webviews whose isSyncBlocked came back falsy. Guarded against self-triggering: the re-flag's own update event shows isSyncBlocked true, so it is a no-op and cannot loop. Subscription mirrors the late-join lifecycle (subscribe while blocked, unsubscribe after). Finding 21 (duplicated constant): SAFETY_TIMEOUT_MS re-hardcoded SHUTDOWN_SYNC_TIME_OUT_MS's 10*60*1000 literal. Hoist to a single shared constant in shared/data/platform.data.ts, used by both shutdown-tasks.ts and the blocking store; keep the store's comment explaining why it tracks the shutdown-sync timeout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Finding 24 (EnterWrite placement consistency): InventoryDataProvider's SetInventoryItemStatus and SetInventoryOptionValues placed their ArgumentException.ThrowIfNullOrEmpty guards before the write scope, unlike every other gated site. Surveyed all ~22 gated sites: the overwhelming majority (ParatextProjectDataProvider x11, CheckRunner x2, ManageBooksService x5 — the last doing precondition validation inside the scope) open the scope as the method's first statement, matching the CLAUDE.md rule verbatim. Moved the two Inventory gates above the guards so code and rule agree; the guards only validate (no mutation) and EnterWrite already rejects a null project id. Finding 15 (coverage scan includes bin/obj): the source enumeration greps bin/ and obj/ build output (*.g.cs, AssemblyInfo.cs); a future source generator emitting a matched pattern there would fail the test with an un-allowlistable obj-relative path. Exclude any file under a bin/ or obj/ segment from the scan. Finding 14 (file-granularity allowlist hole): a whole-file exemption let a new ungated write added to an already-allowlisted file (e.g. a 12th method in ParatextProjectDataProvider) stay green. Replaced the blanket per-file skip with a per-write-site check: each hit must be (a) gated in its enclosing method (an EnterWrite/EnterSyncWriteScope call above it, found by an indentation-bounded upward walk — the gate is always the first statement), or (b) carry an inline `// SR-write-gate: exempt — <reason>` marker (added to the un-gated SetBookUsfmInScope core and the ManageBooks orchestrators / RawDirectoryProjectStreamManager, each citing its gated caller + TODO(PT-4210)), or (c) sit in a not-project-data file (the only remaining whole-file exemptions: UserProjectSettings, MarblePackageDiscoverer). Comment lines are skipped so doc references like `inventory.Save()` do not false-fire. Validated by temporarily injecting an ungated write into ParatextProjectDataProvider: the test failed on it, then passed once removed. The test docstring is honest about the heuristic's bounds. Updated the CLAUDE.md gate section to describe the per-site enforcement. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Finding 23 (duplicated sync-blocked notification): the
{severity:'warning', message:%…error_syncEditBlocked%} payload appeared three
times in platform-scripture-editor.web-view.tsx (usj-save ternary, comment-save
guard, comment catch). Extracted a single notifySyncEditBlocked() helper so the
message/severity cannot drift.
Finding 13 (fire-and-forget notifications.send): the comment-save guard (~1590)
and the sentinel catch (~1657) called send() without await/.catch, so a
rejection from the notification service became an unhandled promise rejection.
Folded both into notifySyncEditBlocked(), which awaits inside try/catch and logs
on failure.
Finding 10 (insert-comment hotkey silent no-op): the hotkey path early-returned
with no feedback while the context-menu item is visibly disabled during a block.
Split the isSyncBlocked case out of the combined guard and show the shared
"editing paused" notice via notifySyncEditBlocked() (reuses the existing
error_syncEditBlocked string — no new key).
Finding 25 (l10n key ordering): the %…_syncEditBlocked_banner% key sat between
error_syncEditBlocked and info; a `sync…` key sorts after switchScriptureView.
Moved it to the correct alphabetical position in both the en and es blocks.
Finding 9 (only the editor translated the sentinel): the write-gate also rejects
ManageBooks, Inventory, and Checks mutations, but those UIs surfaced a raw
technical error or swallowed the failure. Added a shared, reusable
sentinel-detection helper — sync-edit-blocked.util.ts (isSyncEditBlockedError,
mirroring the editor's SYNC_EDIT_BLOCKED_REGEX; kept local to platform-scripture
since all three consumers live there and platform-bible-utils ships committed
dist artifacts) — and wired the friendly "editing paused" notification into all
three real UI routes:
- ManageBooks: intercept in onMutationResult (the single chokepoint where the
dialog routes a thrown backend error as a MutationResult error entry) —
warning toast instead of the raw sentinel at error severity.
- Inventory: the approved/unapproved save catches only logged (invisible); now
they notify on the sentinel and still log other errors. Added `papi` import.
- Checks: handleDenyCheck/handleAllowCheck had no try/catch, so a rejection was
an unhandled promise rejection with zero UI (buttons are fire-and-forget);
added a catch that notifies on the sentinel and re-throws anything else.
Added the shared %webView_platformScripture_error_syncEditBlocked% string
(en + es, alphabetized) to platform-scripture's localizedStrings.json. The
editor's existing handling is untouched.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The shared constant SHUTDOWN_SYNC_TIME_OUT_MS was added to shared/data/platform.data but the generated papi.d.ts was not regenerated, so CI's 'Report file changes' step failed on all platforms. Regenerated via 'npm run build:types'. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n unblock When a scheduled sync finishes, syncState() called applyToAllEditors(false) before tearing down the onDidUpdateWebView re-flag subscription (added in e79699c for the rebuilt-editor case). The driver's own unflag write fires onDidUpdateWebView synchronously (buffered emitter and this subscription share the same PapiNetworkEventEmitter instance), so the still-live handler observed the just-written `false`, passed its "came back unblocked" guard, and re-flagged every open Scripture editor straight back to blocked — with no recovery short of a full window reload. Found live in E2E, 2026-07-16. Fix: unsubscribe both the open- and update-subscriptions before applying the unblock, so the handler is gone before the unflag write can be observed. Extends the driver test mocks so updateWebViewDefinitionSync synchronously dispatches to any registered onDidUpdateWebView handler (mirroring the real shared-emitter behavior), and adds a regression test that fails against the unfixed code (verified: [true, false, true]) and passes with the fix ([true, false]).
lyonsil
approved these changes
Jul 16, 2026
lyonsil
left a comment
lyonsil
left a comment
Member
There was a problem hiding this comment.
@lyonsil reviewed 34 files and all commit messages, made 1 comment, and resolved 24 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved.
rolfheij-sil
enabled auto-merge (squash)
July 16, 2026 16:53
This was referenced Jul 16, 2026
rolfheij-sil
added a commit
that referenced
this pull request
Jul 17, 2026
…h a self-catching helper Add notifySyncEditBlocked() to sync-edit-blocked.util.ts - the platform-scripture twin of the scripture editor's helper of the same name: it awaits papi.notifications.send inside try/catch and logs a failure, so fire-and-forget callers can never leak an unhandled promise rejection, and the shared "editing paused during Send/Receive" message and warning severity cannot drift per call site. Route all five bare send call sites through it: - checks-side-panel.web-view.tsx (deny/allow catch paths) - inventory.web-view.tsx (approved/unapproved items catch paths) - manage-books.web-view.tsx (mutation-result toast routing; this one was pre-existing - its send sat inside a try/catch without await, so the catch only ever saw synchronous throws. The non-blocked generic toast there now handles rejections with .catch for the same reason.) Also adopt the runtime promise `useProjectSetting`'s setters return in inventory's two try blocks (the hook's declared setter type erases it, so a bare await has no effect on the type and the catch looked dead). Unit tests cover the helper's happy path and that a rejected send is swallowed and logged. Addresses lyonsil's re-review note N1 on #2555. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rolfheij-sil
added a commit
that referenced
this pull request
Jul 17, 2026
…URATION_MS Since #2555 hoisted it to platform.data.ts, the constant bounds both the main process's shutdown-sync wait and the renderer's auto-sync edit-block safety leash - the "shutdown" name no longer fit half its uses, and its doc comment had to spend two lines reconciling that. Name it for what it bounds (the maximum duration of a single automatic Send/Receive) and simplify the doc comments at both consumers. papi.d.ts regenerated (npm run build:types) - the constant is exported there. Addresses lyonsil's re-review note N2 on #2555. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rolfheij-sil
added a commit
that referenced
this pull request
Jul 17, 2026
…or auto-sync blocking Two hardenings from #2555's review (PT-4214 Stage T): Finding 8: the auto-sync-blocking store's safety timer hard-reset blockCount to 0 when it fired, so under overlapping blocks one stale expiry wiped newer in-flight blockers (and a block released by the reset could later be released again by its own clear, stealing another blocker's count). Each raise now arms its own one-shot leash that releases only its own block, exactly once; a clear cancels the oldest outstanding leash so a cleared block's leash can never fire later. Same flaw, same fix in the near-twin workspace-updating-store (30 s leash). The two fixes are deliberately symmetrical without a shared abstraction - that extraction is deferred to PT-4214 Stage U (reviewer-agreed). Finding 7 (mitigation): the blocking service's protocol was event-only, so a renderer reload during an in-flight scheduled sync came up unblocked while the backend was still syncing - the documented limitation in auto-sync-blocking-service.ts. Implement the documented upgrade path: on init, query the emitter's current blocking state (paratextBibleSendReceive.getAutoSyncBlocking, requested untyped like cancelSync in shutdown-tasks) and seed the store from it. Best-effort: a live event always wins over the snapshot, a failed request keeps the assume-unblocked default, and the per-blocker leash bounds a seeded block whose clearing event never arrives. Registering the command on the Send/Receive extension side remains for PT-4214. Tests: overlapping blockers where a stale leash expires while a newer block is active (count stays correct, UI stays blocked until the newer one clears), clear-pairs-with-oldest-leash, and the seeding paths (seeds on true, skips false/non-boolean/failure, event wins over snapshot, no seeding after cleanup). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
timothy-mccormack
pushed a commit
that referenced
this pull request
Jul 21, 2026
…h a self-catching helper Add notifySyncEditBlocked() to sync-edit-blocked.util.ts - the platform-scripture twin of the scripture editor's helper of the same name: it awaits papi.notifications.send inside try/catch and logs a failure, so fire-and-forget callers can never leak an unhandled promise rejection, and the shared "editing paused during Send/Receive" message and warning severity cannot drift per call site. Route all five bare send call sites through it: - checks-side-panel.web-view.tsx (deny/allow catch paths) - inventory.web-view.tsx (approved/unapproved items catch paths) - manage-books.web-view.tsx (mutation-result toast routing; this one was pre-existing - its send sat inside a try/catch without await, so the catch only ever saw synchronous throws. The non-blocked generic toast there now handles rejections with .catch for the same reason.) Also adopt the runtime promise `useProjectSetting`'s setters return in inventory's two try blocks (the hook's declared setter type erases it, so a bare await has no effect on the type and the catch looked dead). Unit tests cover the helper's happy path and that a rejected send is swallowed and logged. Addresses lyonsil's re-review note N1 on #2555. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
timothy-mccormack
pushed a commit
that referenced
this pull request
Jul 21, 2026
…URATION_MS Since #2555 hoisted it to platform.data.ts, the constant bounds both the main process's shutdown-sync wait and the renderer's auto-sync edit-block safety leash - the "shutdown" name no longer fit half its uses, and its doc comment had to spend two lines reconciling that. Name it for what it bounds (the maximum duration of a single automatic Send/Receive) and simplify the doc comments at both consumers. papi.d.ts regenerated (npm run build:types) - the constant is exported there. Addresses lyonsil's re-review note N2 on #2555. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
timothy-mccormack
pushed a commit
that referenced
this pull request
Jul 21, 2026
…or auto-sync blocking Two hardenings from #2555's review (PT-4214 Stage T): Finding 8: the auto-sync-blocking store's safety timer hard-reset blockCount to 0 when it fired, so under overlapping blocks one stale expiry wiped newer in-flight blockers (and a block released by the reset could later be released again by its own clear, stealing another blocker's count). Each raise now arms its own one-shot leash that releases only its own block, exactly once; a clear cancels the oldest outstanding leash so a cleared block's leash can never fire later. Same flaw, same fix in the near-twin workspace-updating-store (30 s leash). The two fixes are deliberately symmetrical without a shared abstraction - that extraction is deferred to PT-4214 Stage U (reviewer-agreed). Finding 7 (mitigation): the blocking service's protocol was event-only, so a renderer reload during an in-flight scheduled sync came up unblocked while the backend was still syncing - the documented limitation in auto-sync-blocking-service.ts. Implement the documented upgrade path: on init, query the emitter's current blocking state (paratextBibleSendReceive.getAutoSyncBlocking, requested untyped like cancelSync in shutdown-tasks) and seed the store from it. Best-effort: a live event always wins over the snapshot, a failed request keeps the assume-unblocked default, and the per-blocker leash bounds a seeded block whose clearing event never arrives. Registering the command on the Send/Receive extension side remains for PT-4214. Tests: overlapping blockers where a stale leash expires while a newer block is active (count stays correct, UI stays blocked until the newer one clears), clear-pairs-with-oldest-leash, and the seeding paths (seeds on true, skips false/non-boolean/failure, event wins over snapshot, no seeding after cleanup). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
timothy-mccormack
pushed a commit
that referenced
this pull request
Jul 21, 2026
…Stage T) (#2571) Follow-ups to the #2555 edit-block round: - Release edit-block and workspace-switch leashes by identity (replaces oldest-first ref-count pairing) - Per-blocker safety leashes + initial-state seeding for auto-sync blocking - Declare useProjectSetting's real async setter/reset types - Route sync-edit-blocked notifications through a self-catching helper - Rename SHUTDOWN_SYNC_TIME_OUT_MS -> AUTO_SYNC_MAX_DURATION_MS and tag it @experimental - Regenerate papi.d.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PT-4159 — the "block edits during a scheduled sync" half of the automatic Send/Receive concurrency contract (epic PT-4158). Editing-only block: mostly renderer, plus a small inert C# write-gate (see below).
Current design — editing-only block (reworked 2026-07-15 per UX decision)
Per the 2026-07-15 UX decisions (epic PT-4158): scheduled (unattended) Send/Receive must block editing, but must not block the UI. This PR now ships that shape — menus, dialogs, navigation, and the rest of the app stay fully usable during a scheduled sync; only Scripture editing (and comment/note creation) freezes for its duration. (See "History" at the bottom for the original full-workspace-overlay design this replaced.)
auto-sync-blocking-store.ts, unchanged since the original design): nested raises don't clear early, 200 ms show-grace (a sync finishing inside it never shows anything), 10-minute safety auto-clear (matchesSHUTDOWN_SYNC_TIME_OUT_MS; the project-switch store's 30 s would unblock mid-sync).auto-sync-blocking-service.tssubscribesparatextBibleSendReceive.onAutoSyncBlockingChanged{ isBlocking: boolean }— the same event contract the interval engine (PT-4163, ext PR #179) brackets its fire path with viasetBlocking(true)/setBlocking(false).auto-sync-edit-block-driver) replaces the original full-workspace overlay: it mirrors the store's visible state onto each open Scripture editor by setting anisSyncBlockedweb-view state (and flags editors opened mid-block). No workspace overlay, no focus trap —AutoSyncBlockingOverlayand its focus/dialog tests are deleted, andoverlay-workspace-updating.component+ stories are reverted to their legacy{ label }-only shape (shared with the project-switch overlay).platform-scripture-editor):isSyncBlockedfolds intoisReadOnlyEffective(freezes scripture text) and separately gates comment/note creation (context menu, hotkey, note-caller). A slim, non-covering banner across the top of the editor pane shows localized "editing paused" text, live sync progress (paratextBibleSendReceive.onSyncProgress), and a single-shot Cancel (paratextBibleSendReceive.cancelSync, re-arms on request failure) — the bottom-right progress toast keeps its own Cancel too. The web-view factory forcesisSyncBlocked: falsewhen rebuilding saved state, so a crash mid-sync can't persist a read-only editor.c-sharp): a new thread-safeSendReceiveWriteLockregistry, consulted at every project write choke point inParatextProjectDataProvider(scripture USFM/USX, project settings, extension data, comment mutations),ManageBooksService(deleteBooks/createBooks/copyBooks/copyCustomVersification/importBooks, gating the written project), andInventoryDataProvider's two setters. When a project is syncing, the write throws with a message ending in the sentinel(SR_EDIT_BLOCKED), which the editor recognizes to revert the change and show an "editing paused during Send/Receive" notification. The comment-editor Save path is sync-block-aware on both layers (early-return warning +SR_EDIT_BLOCKEDcatch branch that closes the popover and clears the pending highlight).SendReceiveWriteLock.SetSyncing, soIsBlockedis alwaysfalseand no write is ever rejected in open-source Platform.Bible — public behavior is unchanged. Activation (bracketing each automatic sync withSetSyncing/Clear) is deferred to the Paratext 10 Studio patch — tracked as PT-4210, blocked on this PR merging.%overlay_autoSyncBlocking%-family strings carry over (en + es); Cancel reuses%general_cancel%.Known limitations
Verification
npm run typecheck(all workspaces) → 0; eslint + prettier + cspell on changed files → clean; csharpier → clean.dotnet testc-sharp suite → 1487+ passed / 0 failed, incl. the newSendReceiveWriteLockregistry + gate tests (PDP, ManageBooks, Inventory wiring).npm run test:core -- --run src/renderer→ 535 passed.Update (2026-07-15): write-gate rebuilt on ReaderWriterLockSlim, + enforcement
SendReceiveWriteLock(c-sharp/Projects/SendReceive/SendReceiveWriteLock.cs) was reworked from the counter-based registry described above into a wrapper overReaderWriterLockSlim, with reader/writer roles deliberately inverted: a project write takes the RWLS read lock (the concurrent "reader" side — many can run at once), and Send/Receive takes the write lock (the exclusive "writer" side). Both directions of the gate now rest on real synchronization instead of a counter:EnterWritefails fast (TryEnterReadLock(0), sentinel(SR_EDIT_BLOCKED)) the instant a sync is armed or holds the write lock, so a user's keystroke never queues behind one;SetSyncingarms the project set and then acquires the write lock with a bounded 10 s timeout — that acquisition is the drain of in-flight writes, since a writer-preferring RWLS blocks new readers from the moment it queues, so it waits for open write scopes to close before returning (on timeout it proceeds unheld, with a belt-and-suspenders armed-set check inEnterWritestill rejecting writes). RWLS is thread-affine, soSetSyncing/Clearmust run on the same thread — satisfied by PT-4210's planned activation, which brackets each automatic sync in its own worker thread'stry/finally. The activation API for PT-4210 is unchanged: it still only needs to callSetSyncing(projectIds)before a sync andClear()after.Also added in this update:
CLAUDE.md, "Send/Receive Write Gate"): any new C# code path that mutates project data must gate the mutation withusing var _ = SendReceiveWriteLock.EnterWrite(projectId);as the first statement of its entry point.c-sharp-tests/Projects/SendReceive/SendReceiveWriteLockCoverageTests.cs): scans thec-sharp/source tree for direct project-write call patterns (PutText,Settings.Save/SetSetting/RemoveSetting,FileManager.Delete) outside an explicit allowlist, so an ungated write added later fails the suite instead of silently slipping past the gate.Post-review hardening (2026-07-15)
Follow-ups from a concurrency review of the write-gate:
CheckRunner.DenyCheckResult/AllowCheckResult(papidenyCheckResult/allowCheckResult) persistedErrorMessageDenialsinto the project folder with no gate, so a Checks-UI click could write mid-sync. Both are synchronous and call no other gated method, so each now opensEnterWrite(projectId)as its first statement; added wiring tests (armed ⇒ throw with theSR_EDIT_BLOCKEDsentinel).CLAUDE.mdrule promises: a general.Save(heuristic (subsumesSettings.Save; catchesScrText.Save,denials.Save,inventory.Save,XDocument.Save), comment persistence (SaveUser/CommentEditHelper.SaveEdits), and rawFile.Delete, alongside the existingPutText/SetSetting/RemoveSetting/FileManager.Delete. The allowlist now carries a one-line reason per entry (gated /TODO(PT-4210)/ not-project-data) and addsCheckRunner(gated) plusUserProjectSettings,MarblePackageDiscovererandRawDirectoryProjectStreamManager(not the sharedScrText/Settingsdata the S/R merge replaces). Detection re-verified: an injected ungated.Save()in a non-allowlisted file fails the scan; the clean tree passes.EnterWritescope synchronously — noawait/Task.Runacross it (the RWLS read lock is thread-affine) — andSetSyncing/Clearmust run on one thread. Added toCLAUDE.mdand theEnterWrite/SetSyncing/Cleardocstrings, plus a defensive owner-thread check that makes a cross-threadClearthrow.Full
dotnet testc-sharp suite after these changes → 1500 passed / 6 skipped / 0 failed (prior baseline 1497; delta is only the added passing tests).Live E2E fix (2026-07-15)
Live end-to-end testing against a real scheduled sync found the block never engaged at runtime:
initAutoSyncBlockingService()andinitAutoSyncEditBlockDriver()were defined and unit-tested but nothing called either one at renderer startup, so a user could keep typing straight through a 6.5 s scheduled Send/Receive. Fixed by wiring both intosrc/renderer/index.tsx, called once after the network service and the webview/notification service block are up (their dependencies) — blocking service first, then the driver.Review fixes — round 2 (renderer, 2026-07-16)
Renderer-side findings from @lyonsil's review:
initAutoSyncBlockingService/initAutoSyncEditBlockDriverwere wired up in bothrenderer/index.tsxandapp.component.tsx'sMainuseEffects, so two event subscriptions + two driver instances resulted; aMainunmount after a raise (hot-reload / StrictMode / route change) ran only its cleanup, leaving editors read-only until the 10-min safety timer. Now initialized in exactly one place (index.tsx, like every other renderer service); removed theapp.component.tsxwiring and the now-dead edit-block-drivervi.mockinapp.component.test.tsx(a jsdom-matchMediaworkaround for the removed import chain — the smoke test still passes without it).isSyncBlockedviaonDidOpenWebViewbut neveronDidUpdateWebView, so an in-place rebuild during a sustained block (reloadWebView/ interface-mode switch /loadLayout, which forceisSyncBlocked: false) came back editable with the banner gone. While blocking is active the driver now also subscribesonDidUpdateWebViewand re-flags Scripture-editor webviews whoseisSyncBlockedcame back falsy — guarded against self-triggering (its own re-flag emits an update showing the flagtrue, a bounded no-op) and released when the block ends. Added tests: rebuild-during-block re-flagged, no self-loop (bounded update calls), subscription released on unblock.SAFETY_TIMEOUT_MSre-hardcodedSHUTDOWN_SYNC_TIME_OUT_MS's10*60*1000literal. Hoisted to one sharedSHUTDOWN_SYNC_TIME_OUT_MSinshared/data/platform.data.ts, imported by bothmain/shutdown-tasks.tsand the blocking store (which keeps its comment explaining why it tracks the shutdown-sync timeout).syncState()applied the unblock (applyToAllEditors(false)) before unsubscribing fromonDidUpdateWebView, so the driver's own synchronous unflag write was observed by the still-live handler and bounced every open Scripture editor straight back to blocked, with no recovery short of a full window reload. Fixed by unsubscribing both subscriptions before applying the unblock; added a regression test that fails against the pre-fix code ([true, false, true]) and passes with the fix ([true, false]).Renderer vitest (
src/renderer) → 520 passed; the three auto-sync service specs +app.componentspec → 33 passed; typecheck 0; eslint/prettier/cspell on changed files clean.Design + plan docs: PRD folder,
2026-07-14-pt-4159-edit-block-*.md.AI-assisted (Claude Code session, 2026-07-14/15).
🤖 Generated with Claude Code
This change is
History: original overlay design (superseded 2026-07-15)
The full-workspace-overlay design below was this PR's original scope. It was reworked (see the design above) after the 2026-07-15 UX decisions established that editing — not the whole UI — must be blocked during automatic syncs.
What (original)
A full-workspace blocking surface for scheduled (unattended) Send/Receive, copying PT9's automatic-sync surface semantics (
ProgressUtilsImpl.cs/ProgressPopupForm): no close chrome, Cancel as the only control, a 200 ms grace period (a sync finishing inside it never shows anything), silent on success — it just clears.auto-sync-blocking-store.ts— ref-counted visibility (nested raises don't clear early) with the 200 ms show-grace and a 10-minute safety auto-clear (matchesSHUTDOWN_SYNC_TIME_OUT_MS; the project-switch store's 30 s would unblock mid-sync).auto-sync-blocking-service.ts— subscribesparatextBibleSendReceive.onAutoSyncBlockingChanged{ isBlocking: boolean }by name (same idiom asworkspace-updating-service/ the toolbar'sonSyncStateChangedsubscription). The emitter lands with the scheduler work (PT-4162/PT-4163); until then this surface is inert.paratextBibleSendReceive.onSyncProgressevent; indeterminate progress is text-only — the platform Progress component has no indeterminate mode). Progress state clears between episodes.AutoSyncBlockingOverlay— reusesWorkspaceUpdatingOverlayPresentational(which gains optional cancel props; the project-switch overlay's output is unchanged and its tests pass unmodified). Cancel is single-shot, re-arms on request failure or when blocking clears, and firescommand:paratextBibleSendReceive.cancelSyncby serialized request type (theshutdown-tasks.tsidiom).%overlay_autoSyncBlocking%(en + es); Cancel reuses%general_cancel%.Tests (original, pre-rework)
+37 renderer tests (store timer interleavings incl. grace-boundary races, service subscription, overlay render/focus/trap/single-shot/re-arm).
npm run test:core -- --run src/renderer: 525 passed. Typecheck and lint clean.Design update note (2026-07-15 UX decisions), posted mid-review
onAutoSyncBlockingChangedevent contract, ref-count store, grace/safety timers, and Cancel wiring remain valid — the change is scoped to the blocking surface. Rework lands as follow-up commits on this branch; hold review of the overlay component until then.Review fixes — round 3 (C# + extensions, 2026-07-16)
InventoryDataProvidergates above theirThrowIfNullOrEmptyguards so code matches the CLAUDE.md "first statement" rule and the 20/22 majority gate-first pattern (guards only validate;EnterWritealready rejects null). Rule text unchanged.bin/objsegment.// SR-write-gate: exempt — <reason>marker (added toSetBookUsfmInScope+ the ManageBooks orchestrators +RawDirectoryProjectStreamManager), OR a not-project-data whole-file exemption (now onlyUserProjectSettings/MarblePackageDiscoverer). Comment lines skipped. Validated by injecting an ungated write into an already-gated file (test failed, then passed on removal). CLAUDE.md gate section updated.notifySyncEditBlocked()helper in the scripture editor.%…_syncEditBlocked_banner%to alphabetical order in both en and es.sync-edit-blocked.util.ts(isSyncEditBlockedError) in platform-scripture and wired the friendly "editing paused" notification into ManageBooks (onMutationResult), Inventory (save catches), and Checks (deny/allow — previously an unhandled rejection with no UI). Added%webView_platformScripture_error_syncEditBlocked%(en+es). Editor handling untouched.