PT-4214/PT-4210 cleanup: declare the sync-block seam (getAutoSyncBlocking, onSyncWriteLockChanged) + public breakSyncLock stub

[rolfheij-sil]

Post-merge-window cleanup from the PT-4158 triage (2026-07-27). Two stacked commits:

1. Declare the held-back sync-block seam (PT-4214)
getAutoSyncBlocking (command), onSyncWriteLockChanged (event) and their SyncWriteLockSnapshot payload type are now declared in src/@types/paratext-bible-send-receive/index.d.ts. They shipped in #2574 served from C#, but were deliberately left undeclared to avoid colliding with #2570's seam re-home; #2570 has merged, so the reason is gone (flagged in the 2026-07-24 PT-4214 code-state audit). The renderer's auto-sync-blocking-service drops its local type duplicate and its untyped requestNoRetry workaround in favor of the typed sendCommand / non-deprecated getNetworkEvent overload; its fail-safe unknown-payload validation is unchanged. All new declarations carry @experimental.

2. Public breakSyncLock stub (PT-4210 Task 3)
paratextBibleSendReceive.breakSyncLock existed only in the Paratext 10 Studio patch, deviating from the public/private seam convention cancelSync/syncProjects follow. This adds the public C# stub (PlatformUnimplementedException, sibling-identical phrasing) and the @experimental d.ts declaration, wire-signature-matched to the real implementation ((projectIds: string[]) => Promise<{ [projectId: string]: boolean }>). Two deliberate deviations from the patch: the stub is non-async (an async throw-only body would emit CS1998; the private body-fill re-adds async), and the indefinite-timeout registration arg stays patch-side (it is a patch-only constant, as for syncProjects).

Pairing: paranext/paratext-10-studio#168 converts the patch from command-registration to stub body-fill and must merge only after this PR. No behavior change in plain Platform.Bible — the stub throws exactly like its siblings.

Verification: typecheck 0 errors (core + all workspaces); vitest auto-sync-blocking suites 35/35; dotnet build 0 errors, no new warnings; dotnet test --filter SendReceive 69/69; papi.d.ts regen run → zero diff (the ambient seam is not part of generation); eslint/prettier/csharpier/cspell clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_019MhYcp1XVTQ7JHiA4n2qaV

---

This change is 

---

Review fix round (2026-07-27, tip d12946c999b): self-review (/review-paratext, 4 passes) found 0 critical findings. Applied: wire-side experimental docs (ExperimentalMethodDocumentation.Create) on the breakSyncLock and getAutoSyncBlocking registrations; breakSyncLock docs now state it targets the server repository lock (not the local write gate) and that an empty array is a no-op (verified against the private implementation); the stale PT-4214 tracking comment in auto-sync-blocking-service.ts now points at PT-4265. Re-verified: build 0 errors, 69/69 filtered C# tests, typecheck clean, 14/14 renderer tests.

---

Follow-up (2026-07-27, tip 4fcaa9f7e96): adds a LocalParatextProjects constructor parameter (patch-bound per the documented CS9113 convention in this class) so the Paratext 10 Studio patch can notify project-list consumers after a sync completes — in-place Settings.xml rewrites and mid-clone states are invisible to the non-recursive directory watcher (per its own doc comment), leaving the app-lifetime toolbar picker and open New Tab views stale. No behavior change in plain Platform.Bible (the stub never uses it). The paired patch-side call lands in paranext/paratext-10-studio#168. Build 0 errors, 69/69 filtered C# tests.

---

Review round 2 (2026-07-28, tip 1159cb1de06): all 26 inline findings + review notes processed (verified-then-acted). Highlights: stub-side indefinite timeout (kills the patch's fragile registration hunk); public virtual LocalParatextProjects.RefreshAndNotifyProjectsChanged() + retargeted stub instruction; init-time snapshot emit; wire x-experimental on the event via new notification-doc records; Task.FromException stub; per-source payload-validation latch; cold-start-race doc rewrite (+warn); doc-assertion tests (new stub-docs test file); contract docs (upper-cased keys, skipped ids, false = not-broken); seam-header re-sync guard; CS9124 eliminated; and D2(b) — core-declared patch-bound properties replacing the CS9113 pragma. Build 0 errors, 73/73 filtered C#, 57/57 renderer tests.

[timothy-mccormack]

Review: 26 findings (max-effort pass)

Ran a nine-angle parallel review over this PR plus the two companion PRs (paratext-10-studio#168, paratext-bible-internal-extensions#192), the patched core in temp-build, and the Auto-Sync PRD.

No merge blocker. I built and ran both sides rather than reasoning about them: dotnet build 0 errors, 69/69 C# tests, 14/14 vitest on auto-sync-blocking-service.test.ts, tsc --noEmit clean in all six changed files (3 pre-existing errors elsewhere from unbuilt generated artifacts), eslint clean, and PT10 patch #168 still applies. Everything below is contract accuracy, test coverage, and cleanup.

The 26 inline comments are numbered by priority: 1-11 are worth fixing before merge, 12-18 (numbered 15-18 here) are worth fixing, 19-30 (numbered 20-30 here) are cleanup. Four further findings belong to studio#168 and one to ext#192; they're posted on those PRs rather than here.

The three I'd act on first

1. onSyncWriteLockChanged gets the TSDoc @experimental but not the x-experimental wire marker — six of nine review angles raised this independently. Marking two of the three new surfaces is worse than marking none: a consumer reading rpc.discover concludes the event is stable.
2. The breakSyncLock wire docs describe Paratext 10 Studio behavior in a build that always throws — including a security-flavored claim ("the server only permits breaking a lock you own") that public core has no code path to produce or verify.
3. The getAutoSyncBlocking doc names two unreachable failure modes and omits the reachable one — with the downstream cost that the real failure is deliberately logged at debug.

Verified and explicitly not findings

Recording these so they don't get re-litigated:

• The requestNoRetry → sendCommand switch is behaviorally inert, not a retry-cost regression: RpcClient.request takes only two params and drops skipRetry, and main re-dispatches through requestWithRetry either way. The old call already paid the full retry loop; the wire message is byte-identical. (The direction is still right — if RpcClient is ever fixed to forward the flag, the old call would drop the seed on attempt 1. Worth one sentence noting the retry is now deliberate, so a future cleanup doesn't revert it.)
• No TS2717 collision with the extension's vendored copy — extension repos' typeRoots deliberately omit core's src/@types, and the two breakSyncLock signatures are byte-identical anyway.
• No duplicate breakSyncLock registration — the extension only calls it; the command it registers is the differently-named breakSyncLocksAndRetry.
• The sync-throwing Task<T> stub does not break JSON-RPC — verified with a StreamJsonRpc 2.22.11 harness (see comment 25).
• The 5th registration costs ~0 wall-clock (all five round-trips overlap) and the docs payload is 1001 bytes once per process.
• The paratextProjects ctor param is not redundant — _paratextProjects is protected on the factory base, so a non-subclass can't reach it.

Out of scope but shouldn't get lost

tsconfig.json:40's "!**/*.test.{ts,tsx}" entry is inert — TypeScript include doesn't support ! negation, so all 101 test files are in the typecheck program. Harmless today (it's what makes comment 15 actionable), but the entry states an intent the build doesn't honor.

---

🤖 Generated with Claude Code

[timothy-mccormack]

Round-2 verification: 21 of 26 addressed, 5 consciously dropped

Re-reviewed 4fcaa9f7e → 1159cb1de (4 commits, +559/−93 across 11 files). Detailed responses are in the individual threads; this is the status roll-up plus the one cross-repo consequence.

Verified on 1159cb1de, not taken on trust:

Check	Result
dotnet build c-sharp/ParanextDataProvider.csproj	0 warnings, 0 errors (CS9113 and CS9124 gone)
dotnet test --filter ~SendReceive	73 passed (was 69)
vitest auto-sync-blocking-service.test.ts	15 passed (was 14)
tsc -p tsconfig.json --noEmit	clean in all changed files (3 pre-existing env errors elsewhere)

Fixed (21)

Wire marker for onSyncWriteLockChanged (1) · PT10-only caveat on the breakSyncLock wire doc (2) · cold-start-race doc + debug→warn (3) · per-source malformed latch (4) · doc-assertion tests (5) · false ≠ attempt-failed (6) · upper-cased key guidance (7) · skipped-ids documented (8) · seam re-sync no-downgrade rule (10) · s_sendReceiveTimeout (11) · satisfies on the mocks (15) · untrusted-wire comments (16) · parallel registrations (17) · init snapshot emit (18) · seam pointer comment (21) · redundant marker comments (23) · .every(isString) (24) · Task.FromException (25) · papiClient→PapiClient (27) · ctor params bound into properties (28) · documentation: named arg (29).

Three went past what was asked, and better: the notification docs got factored into a reusable OpenRpcNotificationDocumentation rather than a one-off; the init emit sends the gate's current snapshot with a test for the already-armed case; and LocalParatextProjects.RefreshAndNotifyProjectsChanged() closes a finding I'd raised on #168 from the core side, which is the right place for it.

Two I re-verified specifically because they could plausibly have been subtly wrong:

• TimeSpan.FromSeconds(0) really does mean "no timeout" — a stored 0 survives TryGetValue<int> instead of falling back to 30 s, and both SendRequestAsync overloads gate on timeoutMs > 0.
• The new C# notification-doc records match TS SingleNotificationDocumentation exactly, field for field including the x-experimental casing and the omitted name.

Dropped (5) — all previously flagged low or optional

(9) the C# <param> still asserts the empty-list no-op the patch's doc drops — partially addressed, d.ts side is now the richer copy · (20) registration bools still discarded · (22) Create(...) prose still a third copy — it grew, correctly, to carry the unimplemented caveat · (26) List<string> vs String[]? · (30) SyncWriteLockSnapshot members not individually tagged, which was the judgement call I asked for.

No objection to any of these. Details in the threads.

⚠️ One thing to sequence: paratext-10-studio#168 must be re-saved

Not a defect here — the expected cost of moving the ctor-param bindings into core. I applied #168's patch to 1159cb1de to confirm rather than guess:

$ git apply --3way pt168-core.patch ; echo $?
Applied patch to 'c-sharp/Projects/SendReceive/ParatextProjectSendReceiveService.cs' with conflicts.
1

5 conflict blocks, all in that one file, each traceable to a round-2 fix (s_sendReceiveTimeout ×2, the three properties, the SyncProjects DEBUG branch, and the BreakSyncLock stub body). All resolutions are mechanical — full table in the thread on the properties change, mirrored onto #168.

Also worth doing while #168 is open: its patch still calls ParatextProjects.NotifyProjectsChanged() with a separate RefreshScrTexts above it, which the new RefreshAndNotifyProjectsChanged() now replaces in one call.

Ordering is unchanged: get #168 re-saved and approved but unmerged → merge this → merge #168 within minutes. productInfo.json has coreRepo.branch: "main" with no SHA pin, so the studio build is red during whatever gap you leave.

---

From my side this PR is ready; leaving the formal approval to a human reviewer rather than casting it myself.

🤖 Generated with Claude Code

[rolfheij-sil]

🤖 Claude: Round processed — all 26 inline items plus the review-body notes; every claim was verified against the tip before acting. Shipped in four commits (a10aa370447, 384c89a3852, 02395a802ca, 1159cb1de06, tip 1159cb1de06): the D1 cluster (stub-side indefinite timeout; LocalParatextProjects.RefreshAndNotifyProjectsChanged() — public, virtual — with the stub instruction retargeted at it; the notify rationale moved to the ctor comment; init-time snapshot emit), the fix set (wire docs on the event, WhenAll'd registrations, Task.FromException stub, per-source malformed-payload latch, cold-start-race rewording + warn, doc-assertion tests incl. a new stub-docs test file, mock satisfies, .every(isString), CS9124 eliminated), the doc set (build-agnostic wire sentence, upper-cased-keys/skipped-ids/false-means-not-broken contract notes, seam-header re-sync guard), and D2(b) (core-declared patch-bound properties replacing the pragma). Review-summary items: the deliberate-retry sentence is in (02395a802ca); the tsconfig include negation being inert is out of this PR's surface — noted for an upstream cleanup if anyone wants it. Verification: build 0 errors with CS9124 gone and no CS9113 return, 73/73 filtered C# tests (69 + 4 new), 57/57 renderer suite tests, typecheck/lint clean. Merge ordering per your note: ext #192 can go anytime; this PR merges first of the pair, studio #168 undrafts and merges immediately after (its patch now expects this tip, 1159cb1de06).

Round 2: your verification table matches ours on every point — items 1/5/11/15/18 are confirmed on both sides, and 20/22/26/30 close with the same rationale we'd drafted (the patch side of 20 did land: df3449a captures the registration bools and logs failures). One correction to the re-save warning: studio #168 was already re-saved at df3449a (pushed 12:13Z, before your round-2 pass) — none of the five conflict sites exist in that patch, and plain git apply --check against this PR's tip exits 0. The merge sequence you proposed stands as written.

[timothy-mccormack]

Review from a multi-angle pass over the diff (ten independent finder angles plus a verification pass), cross-traced against PapiClient, network.service.ts, async-variable.ts, rpc.model.ts, SendReceiveWriteLock, and the paired PRs in paratext-10-studio and paratext-bible-internal-extensions.

The PR's own verification claims hold up. dotnet build 0 errors and no new warnings; C# suite 1556/1556; vitest 15/15 on the changed test file; tsc -p tsconfig.json reports no error in any changed file. One concern that came up in review — that swapping requestNoRetry for the generic sendCommand breaks typechecking in auto-sync-blocking-service.test.ts — I could not reproduce: the test file is in the tsc program (the "!**/*.test.ts" include entry is a no-op, as tsc only honours ! in exclude), and a full run emits nothing for it.

Six inline comments below are the ones I'd hold the PR on. Three are one-line fixes, two are decisions.

---

Blocker with no line to anchor to: merge ordering

The committed patch on paratext-10-studio main carries the deleted #pragma warning restore CS9113 as hunk context, and separately adds its own "command:paratextBibleSendReceive.breakSyncLock" registration and a BreakSyncLock body — both of which this PR now adds to core. Merging this PR first makes git apply --3way produce conflict markers and then CS0111 duplicate members, breaking every Studio build until paratext-10-studio#168 lands. #168 is still a draft. These two have to go in lockstep, and nothing in this PR signals the dependency to a core maintainer reviewing it on its own merits.

---

Non-blocking (24) — follow-up material, not merge blockers

Correctness-adjacent

1. ThreadingUtils.RunTask — the continuation binds ContinueWith(Action<Task,object?>, object? state, TaskScheduler), so TaskContinuationOptions.OnlyOnFaulted lands in state and a continuation is scheduled on success too. Pre-existing, and harmless because the body re-checks t.Exception != null — but the new comment at SendReceiveBlockNotifierService.cs:178 sells this as a fault-only continuation, which isn't true. ObserveTaskLoggingErrorsToStderr uses the correct overload.
2. auto-sync-blocking-service.ts:61 — filter(...).length === length to every(isString) silently accepts sparse arrays (every skips holes; filter counted them). Not reachable from JSON wire data, but this is the function whose stated job is validating untrusted payloads.
3. SendReceiveBlockNotifierService.cs:145 — Task.WhenAll(Task, Task<bool>) binds params Task[], discarding the result. RegisterRequestHandlerAsync signals failure by returning false, never throwing, so a failed command registration ships silently — while the event arm two lines up logs its rejection.
4. ParatextProjectSendReceiveService.cs:192 — BreakSyncLock is the only handler returning Task<T>. PapiClient.SendRequestAsync<T> short-circuits local handlers with (T)handler.DynamicInvoke(...) and no await, so an in-process caller gets InvalidCastException; the void overload discards the faulted task entirely. The CS1998 justification is a red herring — a non-async Task<T> method may simply throw.
5. SendReceiveBlockNotifierService.cs:110 — InitializeAsync is non-idempotent: BlockStateChanged += … on a process-wide static with no guard and no unsubscribe. A second init doubles every transition and adds another baseline emit.
6. index.d.ts:442 — the d.ts documents an empty-array no-op ("resolves to an empty map without contacting the server") that the stub cannot exhibit; it faults for every input including []. The @param and @throws clauses contradict each other.
7. ParatextProjectSendReceiveService.cs:192 — List<string> is non-nullable under <Nullable>enable</Nullable> while both docs promise "null/blank ids are skipped". Sibling SyncProjects uses String[]?.
8. index.d.ts:445 — the new upper-cased-keys contract isn't honoured by the only consumer (same-user-lock-prompt.ts indexes with raw pending keys). Latent only because upstream ids happen to be upper-case.

Test integrity
9. DummyPapiClient.RegisterRequestHandlerAsync discards the timeout argument with no accessor, so deleting s_sendReceiveTimeout, from the registration passes the whole suite — the riskiest new value in the PR is untestable.
10. RefreshAndNotifyProjectsChanged has no test and no override, despite being virtual "so tests can substitute the ParatextData refresh".
11. ParatextProjectSendReceiveServiceStubDocsTests constructs and InitializeAsynces the class the Studio overlay replaces. In the patched build that body does registry/Hg init and sets a static failure string that then poisons later patched S/R tests — order-dependent failures caused by a file in the public repo.
12. SendReceiveBlockNotifierServiceTests.cs:39 — _ = Client.NextSentEvent; throws when empty and now sits in SetUp; if the init emit changes, all twelve tests fail there with "No sent events to dequeue." Two other fixtures already keep private DrainEventQueue() copies — the helper belongs on DummyPapiClient.
13. auto-sync-blocking-service.test.ts:102 — two synchronous tests assert exact logger.warn counts while a rejected sendCommand is guaranteed pending. They pass only because no microtask checkpoint intervenes; adding any await breaks them silently.
14. DummyPapiClient._sentRequests is a plain Queue<> while this PR introduces real concurrency — the class already upgraded _sentEvents and _documentationByRequestType to concurrent types for exactly this reason.
15. s_blockStateChangedEventDocumentation is a static instance whose every property is { get; set; } rather than init — process-lifetime mutable state handed to the serializer, where ExperimentalMethodDocumentation.Create allocates fresh per registration.

Conventions
16. ParatextProjectSendReceiveServiceStubDocsTests.cs:6 — block-style namespace. .editorconfig sets csharp_style_namespace_declarations = file_scoped:warning; 288/290 files under c-sharp/ are file-scoped. Silently unenforced off ai/* branches, and the whole body is over-indented as a result.
17. ParatextProjectSendReceiveService.cs:26 — internal with no consumer outside the class; the only non-private s_ field in the tree (the other 39 are private, and wider-visibility statics use PascalCase); 103 chars against the 100-char limit, which csharpier won't reflow because the overflow is a trailing comment.
18. Two #region Protected properties and methods with the identical name (:80, :102). Pre-existing, but this PR grows the first from one member to four, entrenching it. Three-line deletion.
19. Public-core comments describe the closed-source patch's internals — :91 documents "the patch's shared sync wrapper (the single helper both the manual and the scheduled sync command paths funnel through)", and the new test file enumerates the overlay's test-file naming scheme. These can't be verified from core and rot silently.

Reuse / simplification
20. RefreshAndNotifyProjectsChanged duplicates OnProjectDirectoriesChanged — one should delegate to the other. That single change also closes both inline blockers below.
21. OpenRpcNotificationDocumentation re-declares all four members of OpenRpcMethodDocumentation verbatim, attributes included, where the TS source models it structurally as Omit<Method, 'result'>. Description on the new record is never assigned anywhere.
22. The event's param is hand-rolled where Param(...) — already using static-imported in that file and used 60 lines below — returns exactly the same descriptor. There's also no ExperimentalNotificationDocumentation factory, so the event path hand-writes what the method path has a helper for, and no PapiClient.RegisterEventAsync wrapper, so network:registerEvent is assembled inline.
23. Positional null for requestTimeout at SendReceiveBlockNotifierService.cs:150 vs named documentation: in the sibling file — the same call written two ways in one PR, and the two spellings of "nothing special here" carry opposite meanings.
24. Prose duplication: the breakSyncLock contract exists in 3-4 near-verbatim copies and has already drifted in this commit (only the TS copy mentions null/blank ids and that false means "not broken", not "attempt failed"); the PT-4265 caveat is restated five times across three files; the non-recursive-watcher rationale three times, while being deleted from SyncProjects — the one place an implementer of that method would read it.

Separately worth a decision: the OpenRPC schemas published here are weaker than the TS contract they mirror (array with no items, object with no additionalProperties), so rpc.discover can't actually validate the seam it exists to describe. And getAutoSyncBlocking / onSyncWriteLockChanged / SyncWriteLockSnapshot are core-owned but declared inside the extension's module namespace, so the file's own "re-sync from upstream" rule has no clause protecting them.

[timothy-mccormack]

Re-reviewed at 3178580. Detail is in the six threads above; short version:

Satisfied (4/6): the refresh/notify funnel + _disposed guard, the syncProjects timeout and its rewritten rationale, and the "never emits" correction across all three doc sites. The funnel in particular is the better of the two fixes I suggested — one refresh-then-notify path with one failure contract, and the new delegation test covers it.

Needs one more pass (1): the awaited baseline emit. Awaiting it is a genuine improvement, but the new InitializeAsync doc claims the startup barrier guarantees the baseline precedes any command-driven arm, and Task.WhenAll doesn't order its members — paratextSendReceiveService.InitializeAsync() and sendReceiveBlockNotifierService.InitializeAsync() are both members of the same barrier at Program.cs:144. Two small ways to make the guarantee real are in that thread.

Correction from me (1): in my _disposed comment I claimed both other RefreshScrTexts() call sites run under _initializationLock. Only the one inside Initialize() does; the watcher path never held it. The locking concern is pre-existing rather than something this PR departs from, and I overstated it — details in that thread. The _isInitialized point there is still open but minor.

Nothing else in the 24 non-blocking items was expected to move in this round, and the paratext-10-studio#168 lockstep dependency is unchanged (still a draft).

[timothy-mccormack]

The round-4 refresh/notify funnel looks right — RefreshAndNotifyProjectsChanged guarding _disposed, catching a RefreshScrTexts throw and notifying anyway, and OnProjectDirectoriesChanged delegating to it are all good changes, and having the best-effort contract live in one place rather than at each call site is the right home for it.

One related race is still open in the same file, and it gets easier to hit now that the funnel is meant to be called from arbitrary worker threads rather than just papi-request threads and the watcher timer.

NotifyProjectsChanged's _disposed check is outside _notifyLock.

if (_disposed)          // :248 — read here
    return;
lock (_notifyLock)      // :250 — lock taken here
{
    _notifyDebounceTimer ??= new Timer(_ => EmitProjectsChanged());
    _notifyDebounceTimer.Change(s_notifyDebounce, Timeout.InfiniteTimeSpan);
}

Dispose() sets _disposed = true, then calls _notifyDebounceTimer?.Dispose() — taking _watchersLock, but never _notifyLock. So a thread that has already passed the _disposed check can interleave two ways:

• _notifyDebounceTimer was non-null → .Change(...) runs against an already-disposed Timer → ObjectDisposedException.
• _notifyDebounceTimer was still null (no notify had ever been scheduled) → the ??= constructs a brand-new Timer after Dispose. Nothing will ever dispose that one, and s_notifyDebounce later it fires EmitProjectsChanged() → SendEventAsync on a disposing PapiClient. EmitProjectsChanged checks _papiClient == null but not _disposed, so there's no second line of defence.

The second one is the nastier variant since it survives past Dispose entirely.

Moving the _disposed read inside lock (_notifyLock), and having Dispose() take that same lock around the _notifyDebounceTimer teardown, would make the check-then-act atomic. ScheduleProjectDirectoriesChanged / _projectChangeDebounceTimer look like they have the same shape and would want the same treatment.

Happy to open this as a separate issue if you'd rather keep this PR focused.

[rolfheij-sil]

🤖 Claude: Answering the re-review roll-up and the NotifyProjectsChanged follow-up together.
Thanks for the fast second pass. The four threads you closed (funnel, timeout, d.ts, renderer
doc) need nothing further from us — noted here so the ledger is complete.

The two open halves are answered in-thread:

• Locking correction — appreciated, and your corrected reading matches the file; the
  _isInitialized note stays accepted-risk as discussed there.
• Baseline-emit ordering — you're right that the doc claimed an ordering Task.WhenAll
  doesn't provide; corrected in e14e830a55b (doc-only — the in-thread reply explains why the
  code itself doesn't need re-sequencing: the baseline is a live read at emit time, so an arm
  that beats it is reflected in it rather than overwritten by it).

Four post-review commits are on the branch. Three are doc-only: 6061118 (the breakSyncLock
@param now states id trimming + case-variant duplicate collapse to a single upper-cased key,
first occurrence wins — closing the last undocumented wire-visible behavior for valid input),
e14e830a55b (the ordering correction above), and d3bec7bfd23 (the same @param now also
covers the null-array case: out of contract — the type is string[] — and treated like the
empty array). The fourth is the race fix below. Studio #168 is re-aligned to the tip.

On the NotifyProjectsChanged/Dispose race (your 15:50Z comment): verified, real, both
variants — Timer.Change on a disposed timer, and the nastier post-Dispose ??= resurrect
that EmitProjectsChanged won't catch (it checks _papiClient, not _disposed). Your point
that this PR's funnel widens the caller set to arbitrary worker threads is what tipped it for
us: rather than split it off, we took the atomicity fix on this PR, in f2c01e04330 — the
_disposed check moved inside the debounce lock at both scheduler sites (NotifyProjectsChanged
under _notifyLock, ScheduleProjectDirectoriesChanged under _projectChangeLock), and
Dispose now tears each timer down under the same lock its scheduler checks under, making
check-then-schedule atomic with the teardown (deadlock-free: no timer callback holds these locks
while waiting on Dispose, and Timer.Dispose() doesn't block on in-flight callbacks).
EmitProjectsChanged also gained a _disposed back-off as a second line of defence for a
callback already in flight at disposal. The check-to-lock window isn't deterministically
reachable from the existing test seams, so the invariant is documented on Dispose rather than
pinned by a test.

[timothy-mccormack]

Re-reviewed at e14e830. All six of my blockers are now resolved or downgraded — nothing from my review is holding this PR.

e14e830 replaces the overstated ordering guarantee with an accurate account of what the barrier does and doesn't buy; detail and one optional nit are in that thread, but it is no longer blocking.

6061118 documents id trimming and case-variant collapse in the breakSyncLock @param. That's a non-blocking item from my review body, and a useful one — the documented contract now matches the implementation's folding behaviour rather than half of it. Worth noting it also sharpens the follow-up I filed on paratext-bible-internal-extensions#192: core now documents the collapse explicitly, while same-user-lock-prompt.ts still indexes the result map with raw, un-upper-cased pending keys.

Two things outstanding, neither a code objection to this PR:

1. Merge ordering. paratext-10-studio#168 is still a draft. Landing this first breaks every Studio build and the patch-apply CI on main — the patch carries the deleted CS9113 pragma as hunk context and adds its own breakSyncLock registration and body, both of which this PR now adds to core. These need to merge in lockstep.
2. One minor open item in the _disposed thread: no _isInitialized / Initialize() check, so a sync arriving before paratextProjects.Initialize() completes would refresh against an uninitialised ScrTextCollection and broadcast a project-list change. Cheap to close, fine as follow-up.

The other 23 non-blocking items from my review body stand as follow-up material, not merge conditions.

[timothy-mccormack]

Approving at b5d544a. All six blockers from my review round are resolved, and the two commits since my last pass closed the remaining open item and a non-blocking one on top of it.

Re-reviewed since e14e830 — three commits I hadn't seen when I said this looked ready, so I went through them rather than approving the older head:

• b5d544a closes the last open item from the _disposed thread: RefreshAndNotifyProjectsChanged now no-ops until Initialize() completes, so an early writer can't refresh an uninitialised ScrTextCollection and broadcast a bogus project-list change. The accompanying test is well built — it proves the absence of an event over a 3x-debounce window and carries a control (NotifyProjectsChanged still emits pre-Initialize) so the zero-count assertion can't pass because of a client that records nothing. That's the failure mode most "assert nothing happened" tests miss.
• f2c01e0 goes beyond anything I raised: moving the _disposed reads inside _notifyLock / _projectChangeLock, and disposing each timer under the same lock its scheduler checks under, makes check-then-schedule genuinely atomic with teardown. The previous shape really could Change a disposed timer, or construct a fresh one after Dispose that nothing would ever dispose. I worked the deadlock argument through independently and it holds: the locks are never nested, no timer callback holds either lock while waiting on Dispose, and parameterless Timer.Dispose() doesn't block on in-flight callbacks. The added _disposed check in EmitProjectsChanged is the right second line of defence.
• d3bec7b resolves the null-array contradiction I raised in the review body — the @param doc no longer promises tolerance the string[] type doesn't admit, and instead states null is out of contract with lenient implementations.

CI is green across all six checks on this head.

---

Two notes that do not affect the approval.

Merging still has a hard external gate. paratext-10-studio#168 is still a draft. repo-patches/paranext-core.patch on that repo's main carries the deleted CS9113 pragma as hunk context and adds its own breakSyncLock registration and body, both of which this PR now adds to core — so landing this first produces git apply --3way conflicts and then CS0111 duplicate members, breaking every Studio build and the patch-apply CI until #168 lands. This approval is of the code, not a signal to merge ahead of that.

One footnote, deliberately not a request. The new if (!_isInitialized) return; is a lock-free read of a non-volatile field. That exactly matches Initialize()'s own double-checked fast path, which is why I'm not asking for a change — it's the established pattern here, the CLR's implemented memory model gives stores release semantics on the platforms .NET targets, and the benign direction (reading stale false) merely skips a refresh that Initialize's own scan supersedes. Worth knowing it rests on that rather than on volatile, if this area is ever revisited.

The remaining items from my review body stand as follow-up, not merge conditions. If any get scheduled, my order would be the discarded RegisterRequestHandlerAsync result (a failed registration currently ships silently), the RunTask fault-only continuation being mis-bound repo-wide, and the breakSyncLock upper-case key contract having no consumer-side enforcement — that last one now filed on paratext-bible-internal-extensions#192.

Nice work on the review rounds — the funnel and the timer-disposal fix both came back better than what I asked for.