diff --git a/c-sharp-tests/Projects/LocalParatextProjectsNotifyTests.cs b/c-sharp-tests/Projects/LocalParatextProjectsNotifyTests.cs index b9bc34f1365..5adb230bb2e 100644 --- a/c-sharp-tests/Projects/LocalParatextProjectsNotifyTests.cs +++ b/c-sharp-tests/Projects/LocalParatextProjectsNotifyTests.cs @@ -4,10 +4,15 @@ namespace TestParanextDataProvider.Projects { /// - /// Verifies debounces: a burst of calls + /// Verifies the notify plumbing of : + /// debounces — a burst of calls /// (e.g. an inline setting-write notify plus the watcher catching that same on-disk write, or a /// run of display-setting writes) must collapse into a single emitted - /// platform.onDidChangeProjects event, since each event drives a full metadata refetch. + /// platform.onDidChangeProjects event, since each event drives a full metadata refetch — + /// and the watcher path funnels through the shared + /// , which itself no-ops + /// before (an early writer must not broadcast + /// a bogus project-list change off an uninitialised ScrTextCollection). /// [ExcludeFromCodeCoverage] internal class LocalParatextProjectsNotifyTests @@ -16,6 +21,43 @@ internal class LocalParatextProjectsNotifyTests private const int SettleTimeoutMs = 5000; private const int PollIntervalMs = 50; + // Window to prove an event does NOT arrive: comfortably past the 500ms notify debounce + // (3x), so a wrongly-scheduled emit would land inside it. + private static readonly TimeSpan NoEventSettleWindow = TimeSpan.FromMilliseconds(1500); + + /// + /// Substitutes with a + /// counter and exposes the protected watcher callback, so the funnel test below can pin the + /// delegation without touching the global ScrTextCollection (via RefreshScrTexts) or + /// the real PAPI event. + /// + private sealed class FunnelObservingProjects(DummyPapiClient papiClient) + : LocalParatextProjects(papiClient) + { + public int RefreshAndNotifyCallCount { get; private set; } + + public override void RefreshAndNotifyProjectsChanged() => RefreshAndNotifyCallCount++; + + public void FireOnProjectDirectoriesChanged() => OnProjectDirectoriesChanged(); + } + + [Test] + public void OnProjectDirectoriesChanged_DelegatesToRefreshAndNotifyProjectsChanged() + { + // The watcher path must funnel through the shared refresh-then-notify method so its + // best-effort contract (a refresh throw must not suppress the notify) and any test + // substitution of the refresh apply to the inline and watcher paths alike. + using var projects = new FunnelObservingProjects(new DummyPapiClient()); + + projects.FireOnProjectDirectoriesChanged(); + + Assert.That( + projects.RefreshAndNotifyCallCount, + Is.EqualTo(1), + "the watcher callback must delegate to RefreshAndNotifyProjectsChanged" + ); + } + [Test] public void NotifyProjectsChanged_CoalescesRapidCallsIntoOneEvent() { @@ -36,5 +78,34 @@ public void NotifyProjectsChanged_CoalescesRapidCallsIntoOneEvent() Is.EqualTo(LocalParatextProjects.PROJECTS_CHANGED_EVENT_TYPE) ); } + + [Test] + public void RefreshAndNotifyProjectsChanged_BeforeInitialize_DoesNotNotify() + { + // A writer can land before startup initialization completes (e.g. a sync finishing + // before Initialize() has set up the ScrTextCollection). Refreshing then would scan + // an uninitialised collection and broadcast a bogus project-list change, so the + // funnel must no-op until Initialize completes. + var client = new DummyPapiClient(); + using var projects = new LocalParatextProjects(client); + + projects.RefreshAndNotifyProjectsChanged(); + + Assert.That( + SpinWait.SpinUntil(() => client.SentEventCount > 0, NoEventSettleWindow), + Is.False, + "RefreshAndNotifyProjectsChanged before Initialize must not emit a project-list change" + ); + + // Control: the direct notify path has no initialization guard (it never touches the + // ScrTextCollection), so an emit still arrives — proving the zero above pinned the + // guard rather than a client that records nothing. + projects.NotifyProjectsChanged(); + Assert.That( + () => client.SentEventCount, + Is.EqualTo(1).After(SettleTimeoutMs, PollIntervalMs), + "NotifyProjectsChanged must still emit pre-Initialize" + ); + } } } diff --git a/c-sharp-tests/Projects/SendReceive/ParatextProjectSendReceiveServiceStubDocsTests.cs b/c-sharp-tests/Projects/SendReceive/ParatextProjectSendReceiveServiceStubDocsTests.cs new file mode 100644 index 00000000000..7a6520eb265 --- /dev/null +++ b/c-sharp-tests/Projects/SendReceive/ParatextProjectSendReceiveServiceStubDocsTests.cs @@ -0,0 +1,50 @@ +using System.Diagnostics.CodeAnalysis; +using Paranext.DataProvider; +using Paranext.DataProvider.Projects; +using Paranext.DataProvider.Projects.SendReceive; + +namespace TestParanextDataProvider.Projects.SendReceive +{ + /// + /// Registration-documentation tests for the open-source + /// stub: breakSyncLock is @experimental (see + /// the TS declaration), so its registration must carry experimental OpenRPC documentation + /// (x-experimental: true) surfaced by rpc.discover — the same doc-assertion + /// pattern as VersificationConversionServiceTests. Deliberately NOT named + /// ParatextProjectSendReceiveServiceTests (or any of the per-feature + /// ParatextProjectSendReceiveService*Tests names the Paratext 10 Studio overlay adds), + /// so the stub's tests and the overlay's tests can coexist. + /// + [TestFixture] + [ExcludeFromCodeCoverage] + internal class ParatextProjectSendReceiveServiceStubDocsTests : PapiTestBase + { + [Test] + public async Task InitializeAsync_RegistersBreakSyncLockWithExperimentalDocs() + { + const string wireName = "command:paratextBibleSendReceive.breakSyncLock"; + var service = new ParatextProjectSendReceiveService( + Client, + new ParatextProjectDataProviderFactory(Client, ParatextProjects), + new AppInfo("test", "1.0.0", "test"), + ParatextProjects + ); + + await service.InitializeAsync(); + + Assert.That( + Client.IsHandlerRegistered(wireName), + Is.True, + "breakSyncLock handler registered under its exact wire name" + ); + var docs = Client.GetDocumentationFor(wireName); + Assert.That(docs, Is.Not.Null, "breakSyncLock registered with OpenRPC documentation"); + Assert.That(docs!.Method.Experimental, Is.True, "breakSyncLock marked experimental"); + Assert.That( + docs.Method.Params.Select(p => p.Name), + Is.EqualTo(new[] { "projectIds" }), + "documents the single projectIds parameter" + ); + } + } +} diff --git a/c-sharp-tests/Projects/SendReceive/SendReceiveBlockNotifierServiceTests.cs b/c-sharp-tests/Projects/SendReceive/SendReceiveBlockNotifierServiceTests.cs index b18ef6db6c9..8ab1c63bd68 100644 --- a/c-sharp-tests/Projects/SendReceive/SendReceiveBlockNotifierServiceTests.cs +++ b/c-sharp-tests/Projects/SendReceive/SendReceiveBlockNotifierServiceTests.cs @@ -1,6 +1,7 @@ using System.Diagnostics.CodeAnalysis; using System.Text.Json; using Paranext.DataProvider.JsonUtils; +using Paranext.DataProvider.NetworkObjects.Documentation; using Paranext.DataProvider.Projects.SendReceive; namespace TestParanextDataProvider.Projects.SendReceive @@ -32,6 +33,10 @@ public override async Task TestSetupAsync() SendReceiveWriteLock.ResetForTests(); _service = new SendReceiveBlockNotifierService(Client); await _service.InitializeAsync(); + // InitializeAsync emits the current gate snapshot once (the restart re-baseline emit — + // see the dedicated tests below); drain it so each test's event assertions see only the + // transitions that test drives. + _ = Client.NextSentEvent; } [TearDown] @@ -61,10 +66,39 @@ public void InitializeAsync_RegistersTheEventWithTheCentralRegistry() Assert.That(requestType, Is.EqualTo("network:registerEvent")); Assert.That( requestContents, - Is.EqualTo(new object?[] { BlockStateChangedEvent }), + Has.Count.EqualTo(2), + "the registration must carry the event name and its documentation" + ); + Assert.That( + requestContents![0], + Is.EqualTo(BlockStateChangedEvent), "the registration must name the onSyncWriteLockChanged event" ); + Assert.That( + requestContents[1], + Is.InstanceOf(), + "the second argument must be the notification documentation" + ); }); + // The event is @experimental (TS declaration), so its registration must carry the + // x-experimental wire marker for rpc.discover. + var documentation = (OpenRpcSingleNotificationDocumentation)requestContents![1]!; + Assert.That( + documentation.Notification.Experimental, + Is.True, + "the event documentation must carry the x-experimental wire marker" + ); + } + + [Test] + public void InitializeAsync_RegistersGetAutoSyncBlockingWithExperimentalDocs() + { + // The command is @experimental (TS declaration), so its registration must carry + // experimental OpenRPC documentation for rpc.discover (the same doc-assertion pattern as + // VersificationConversionServiceTests). + var docs = Client.GetDocumentationFor(GetAutoSyncBlockingCommand); + Assert.That(docs, Is.Not.Null, "command registered with OpenRPC documentation"); + Assert.That(docs!.Method.Experimental, Is.True, "command marked experimental"); } [Test] @@ -90,6 +124,9 @@ public async Task InitializeAsync_RegistryRejectsEventRegistration_LogsAndStillE { Console.SetError(originalError); } + // Drain the init-time current-snapshot emit so the assertions below see only the + // transition push. + _ = client.NextSentEvent; Assert.Multiple(() => { @@ -142,6 +179,9 @@ public async Task InitializeAsync_EventRegistrationThrows_IsCaughtAndStillEmits( { Console.SetError(originalError); } + // Drain the init-time current-snapshot emit so the assertions below see only the + // transition push. + _ = client.NextSentEvent; Assert.Multiple(() => { @@ -174,10 +214,70 @@ public async Task InitializeAsync_EventRegistrationThrows_IsCaughtAndStillEmits( Assert.That(eventType, Is.EqualTo(BlockStateChangedEvent)); } + [Test] + public async Task InitializeAsync_EmitsTheCurrentSnapshotOnce() + { + // Restart re-baseline emit: after initialization the service pushes the gate's CURRENT + // snapshot once, so a subscriber that was already listening across a backend restart + // converges on the fresh process's state instead of keeping its stale last-seen state. + // Fresh gate + client so only the service under test here is subscribed (the base SetUp + // already initialized and drained its own init emit). + SendReceiveWriteLock.ResetForTests(); + using var client = new DummyPapiClient(); + var service = new SendReceiveBlockNotifierService(client); + + await service.InitializeAsync(); + + Assert.That( + client.SentEventCount, + Is.EqualTo(1), + "InitializeAsync must emit the current snapshot exactly once" + ); + var (eventType, payload) = client.NextSentEvent; + Assert.That(eventType, Is.EqualTo(BlockStateChangedEvent)); + var state = (SendReceiveBlockState)payload!; + Assert.Multiple(() => + { + Assert.That( + state.IsBlocking, + Is.False, + "an idle gate re-baselines as not blocking" + ); + Assert.That(state.ProjectIds, Is.Empty); + }); + } + + [Test] + public async Task InitializeAsync_GateAlreadyArmed_EmitsTheArmedSnapshot() + { + // The init emit is the gate's CURRENT snapshot, not a hard-coded not-blocking one: a + // service initializing while the gate is already armed must report the armed state. + SendReceiveWriteLock.ResetForTests(); + SendReceiveWriteLock.SetSyncing(["projectA"]); + using var client = new DummyPapiClient(); + var service = new SendReceiveBlockNotifierService(client); + + await service.InitializeAsync(); + + Assert.That(client.SentEventCount, Is.EqualTo(1)); + var (eventType, payload) = client.NextSentEvent; + Assert.That(eventType, Is.EqualTo(BlockStateChangedEvent)); + var state = (SendReceiveBlockState)payload!; + Assert.Multiple(() => + { + Assert.That(state.IsBlocking, Is.True); + Assert.That(state.ProjectIds, Is.EquivalentTo(new[] { "projectA" })); + }); + } + [Test] public void GateArm_PushesOnSyncWriteLockChangedEventWithBlockingSnapshot() { - Assert.That(Client.SentEventCount, Is.Zero, "no events before any transition"); + Assert.That( + Client.SentEventCount, + Is.Zero, + "no events before any transition (SetUp drained the init emit)" + ); SendReceiveWriteLock.SetSyncing(["projectA", "projectB"]); diff --git a/c-sharp/NetworkObjects/Documentation/OpenRpcNotificationDocumentation.cs b/c-sharp/NetworkObjects/Documentation/OpenRpcNotificationDocumentation.cs new file mode 100644 index 00000000000..5d183dc023a --- /dev/null +++ b/c-sharp/NetworkObjects/Documentation/OpenRpcNotificationDocumentation.cs @@ -0,0 +1,33 @@ +using System.Text.Json.Serialization; + +namespace Paranext.DataProvider.NetworkObjects.Documentation; + +/// +/// OpenRPC documentation for a single network event (the inner notification object of +/// ). Same shape as +/// minus the result — per the OpenRPC convention, a +/// notification has no result. Set to mark the event experimental; it +/// surfaces as x-experimental: true in the live OpenRPC document returned by +/// rpc.discover. Informational only — it does not change runtime behavior. +/// +public record OpenRpcNotificationDocumentation +{ + /// + /// Marks the notification experimental in the OpenRPC document. Mirrors the TypeScript + /// notification['x-experimental'] field. Informational only. + /// + [JsonPropertyName("x-experimental")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public bool? Experimental { get; set; } + + /// A short summary of what the notification carries and when it fires. + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public string? Summary { get; set; } + + /// A verbose explanation of the notification behavior. + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public string? Description { get; set; } + + /// The notification's parameters (its payload), in positional order. + public IReadOnlyList Params { get; set; } = []; +} diff --git a/c-sharp/NetworkObjects/Documentation/OpenRpcSingleNotificationDocumentation.cs b/c-sharp/NetworkObjects/Documentation/OpenRpcSingleNotificationDocumentation.cs new file mode 100644 index 00000000000..fa6b797e857 --- /dev/null +++ b/c-sharp/NetworkObjects/Documentation/OpenRpcSingleNotificationDocumentation.cs @@ -0,0 +1,15 @@ +namespace Paranext.DataProvider.NetworkObjects.Documentation; + +/// +/// Wire shape sent as the optional second argument to network:registerEvent. Mirrors the +/// TypeScript SingleNotificationDocumentation type ({ notification }) — the +/// notification counterpart of . The main process +/// stores it and emits it into the OpenRPC document returned by rpc.discover, so +/// C#-registered network events can carry the same documentation (including +/// x-experimental) as TypeScript-registered ones. +/// +public record OpenRpcSingleNotificationDocumentation +{ + /// The notification documentation. + public OpenRpcNotificationDocumentation Notification { get; set; } = new(); +} diff --git a/c-sharp/Program.cs b/c-sharp/Program.cs index 1dd2503f897..694e7957cd7 100644 --- a/c-sharp/Program.cs +++ b/c-sharp/Program.cs @@ -90,7 +90,8 @@ public static async Task Main() var paratextSendReceiveService = new ParatextProjectSendReceiveService( papi, paratextFactory, - appInfo + appInfo, + paratextProjects ); var inventoryDataProvider = new InventoryDataProvider(papi, paratextProjects); var checkRunner = new CheckRunner(papi, inventoryDataProvider); diff --git a/c-sharp/Projects/LocalParatextProjects.cs b/c-sharp/Projects/LocalParatextProjects.cs index 51c7bab30a5..5c01a71ae70 100644 --- a/c-sharp/Projects/LocalParatextProjects.cs +++ b/c-sharp/Projects/LocalParatextProjects.cs @@ -55,7 +55,11 @@ internal class LocalParatextProjects : IDisposable // Set first in Dispose (before the watcher/timer teardown it guards), so an in-flight // AttachContainerWatcher/event-handler thread racing the Dispose sees it and backs off instead - // of registering a watcher into (or touching a timer of) an already-disposed instance. + // of registering a watcher into (or touching a timer of) an already-disposed instance. The + // debounce-timer schedulers read it INSIDE their debounce lock — the same lock Dispose tears + // the timer down under — so check-then-schedule is atomic with the teardown; a bare read + // before the lock would leave a window to Change a disposed timer, or to construct a new + // timer after Dispose that nothing ever disposes. private volatile bool _disposed; private Timer? _projectChangeDebounceTimer; @@ -198,6 +202,48 @@ public virtual void Initialize() } } + /// + /// Refresh ParatextData's in-memory project list from disk + /// () and then ask project-list consumers to + /// refetch via . For writers whose on-disk changes are + /// invisible to the project-directory watcher — the watcher is non-recursive, so an in-place + /// Settings.xml rewrite or a mid-clone state landing during a Send/Receive never + /// reaches it (see ) — and that therefore must both + /// re-read the project set and notify inline themselves. Also the funnel for the watcher path + /// ( delegates here). Best-effort: a refresh failure + /// must not suppress the notify (a stale collection is better than a permanently stale list). + /// No-op until completes — refreshing an uninitialised + /// would broadcast a bogus project-list change. + /// Virtual so tests can substitute the ParatextData refresh. + /// + public virtual void RefreshAndNotifyProjectsChanged() + { + // Post-Dispose there is no consumer left to serve, so skip the refresh. Best-effort + // early-out only: the notify debounce timer itself is guarded atomically inside + // NotifyProjectsChanged (checked under _notifyLock), so a call racing Dispose past this + // check is still safe. + if (_disposed) + return; + // Pre-Initialize there is nothing to refresh yet: the ScrTextCollection is only set up + // once Initialize() runs (ParatextGlobals.Initialize), so a caller landing early (e.g. a + // sync completing before startup initialization finishes) would refresh an uninitialised + // collection and broadcast a bogus project-list change. Initialize's own scan supersedes + // any call skipped here. Lock-free read matches Initialize's own fast-path check. + if (!_isInitialized) + return; + try + { + ScrTextCollection.RefreshScrTexts(); + } + catch (Exception ex) + { + Console.Error.WriteLine( + $"RefreshScrTexts failed during a project-list refresh; notifying consumers anyway: {ex}" + ); + } + NotifyProjectsChanged(); + } + /// /// Ask project-list consumers to refetch their cheap metadata by emitting /// . Call after a project is added/removed or after one @@ -211,12 +257,16 @@ public void NotifyProjectsChanged() { if (_papiClient == null) return; - // Symmetric with ScheduleProjectDirectoriesChanged: a racing post-Dispose call must not - // touch a debounce timer that may already be disposed. - if (_disposed) - return; lock (_notifyLock) { + // Read under the lock — Dispose tears the timer down under this same lock — so + // check-then-schedule is atomic with the teardown. Checked before the lock, a call + // racing Dispose could Change an already-disposed timer (ObjectDisposedException) + // or, if no notify had ever been scheduled, construct a brand-new timer AFTER + // Dispose that nothing ever disposes and that later emits against a disposing + // PapiClient. + if (_disposed) + return; _notifyDebounceTimer ??= new Timer(_ => EmitProjectsChanged()); _notifyDebounceTimer.Change(s_notifyDebounce, Timeout.InfiniteTimeSpan); } @@ -224,6 +274,11 @@ public void NotifyProjectsChanged() private void EmitProjectsChanged() { + // Second line of defence for a callback already in flight when Dispose ran — + // Timer.Dispose() does not wait for in-flight callbacks — so a dying timer does not + // emit against a disposing PapiClient. + if (_disposed) + return; if (_papiClient == null) return; ThreadingUtils.ObserveTaskLoggingErrorsToStderr( @@ -439,40 +494,34 @@ private void OnProjectDirectoryWatcherError(object sender, ErrorEventArgs e) private void ScheduleProjectDirectoriesChanged() { - // A watcher event/error can race Dispose (see Dispose's comment); back off rather than - // touching a debounce timer that may already be disposed. - if (_disposed) - return; // Debounce: a clone/install fires a burst of events; collapse them into one refresh+notify. lock (_projectChangeLock) { + // A watcher event/error can race Dispose; read under the lock — Dispose tears this + // timer down under this same lock — so check-then-schedule is atomic with the + // teardown. Same shape and rationale as NotifyProjectsChanged. + if (_disposed) + return; _projectChangeDebounceTimer ??= new Timer(_ => OnProjectDirectoriesChanged()); _projectChangeDebounceTimer.Change(s_projectChangeDebounce, Timeout.InfiniteTimeSpan); } } /// - /// Run (debounced, on a timer thread) when a project was added/removed on disk. Refreshes - /// ParatextData's collection so the change is reflected, then notifies consumers. Best-effort: a - /// refresh failure must not suppress the notify (a stale collection is better than a permanently - /// stale list). Virtual so tests can observe firings without mutating the global - /// ScrTextCollection. + /// Run (debounced, on a timer thread) when a project was added/removed on disk. Delegates to + /// — the shared refresh-then-notify funnel, + /// including its best-effort contract (a refresh failure must not suppress the notify). Virtual + /// so tests can observe firings without mutating the global ScrTextCollection. /// - protected virtual void OnProjectDirectoriesChanged() - { - try - { - ScrTextCollection.RefreshScrTexts(); - } - catch (Exception ex) - { - Console.Error.WriteLine( - $"RefreshScrTexts failed after a project directory change: {ex}" - ); - } - NotifyProjectsChanged(); - } + protected virtual void OnProjectDirectoriesChanged() => RefreshAndNotifyProjectsChanged(); + /// + /// Tear down the watchers and debounce timers. Safe to race with watcher events and notify + /// calls: is set first, and each debounce timer is disposed under the + /// same lock its scheduler checks under, so a racing scheduler either + /// backs off or completes its schedule entirely before the teardown — it can never Change a + /// disposed timer nor construct a new one after disposal. + /// public virtual void Dispose() { // Set first (before any teardown below) so a racing AttachContainerWatcher/Register or a @@ -487,8 +536,15 @@ public virtual void Dispose() _watchers.Clear(); _watchedContainerPaths.Clear(); } - _projectChangeDebounceTimer?.Dispose(); - _notifyDebounceTimer?.Dispose(); + // Each timer's teardown happens under the same lock its scheduler checks _disposed under + // (see the summary above). Deadlock-free: no timer callback holds either lock while + // waiting on Dispose (EmitProjectsChanged takes no locks; OnProjectDirectoriesChanged only + // briefly re-takes _notifyLock inside NotifyProjectsChanged), and Timer.Dispose() does not + // block on in-flight callbacks. + lock (_projectChangeLock) + _projectChangeDebounceTimer?.Dispose(); + lock (_notifyLock) + _notifyDebounceTimer?.Dispose(); GC.SuppressFinalize(this); } diff --git a/c-sharp/Projects/SendReceive/ParatextProjectSendReceiveService.cs b/c-sharp/Projects/SendReceive/ParatextProjectSendReceiveService.cs index 8e5d9fe6669..9227047e0a2 100644 --- a/c-sharp/Projects/SendReceive/ParatextProjectSendReceiveService.cs +++ b/c-sharp/Projects/SendReceive/ParatextProjectSendReceiveService.cs @@ -1,21 +1,36 @@ using Paranext.DataProvider.Services; +using static Paranext.DataProvider.NetworkObjects.Documentation.ExperimentalMethodDocumentation; namespace Paranext.DataProvider.Projects.SendReceive; /// /// Commands on the papi that handle Send/Receive-related operations /// -// pdpFactory and appInfo are unread in this open-source implementation, but the closed-source -// Paratext 10 Studio overlay patch binds them into properties, so the constructor signature must -// keep them. Suppress the unread-parameter warning rather than removing the parameters. -#pragma warning disable CS9113 internal class ParatextProjectSendReceiveService( PapiClient papiClient, ParatextProjectDataProviderFactory pdpFactory, - AppInfo appInfo + AppInfo appInfo, + LocalParatextProjects paratextProjects ) -#pragma warning restore CS9113 { + #region Constructors, consts, and fields + + /// + /// Request timeout for the long-running Send/Receive commands, carried on both the syncProjects + /// and breakSyncLock registrations: a whole-project sync is a multi-minute server operation + /// (clone/pull/push round-trips per project), and a lock break makes one potentially ~100s + /// server call per project (N projects in sequence), so either can outlive the default 30s papi + /// request timeout — with that default the caller would give up while the operation is still + /// running (and succeeding) on the backend. 0 = deliberately unbounded: no finite budget fits + /// every project count/size, and the only request a timeout would rescue is a lost response on + /// an otherwise-live socket — a risk every S/R command shares. Inert in plain Platform.Bible + /// since the stub bodies below throw immediately, but the registrations already carry it so the + /// Paratext 10 Studio patch (which fills in the real implementations) inherits the correct + /// timeout. + /// + internal static readonly TimeSpan s_sendReceiveTimeout = TimeSpan.FromSeconds(0); // 0 = no timeout + #endregion + #region Public properties and methods public async Task InitializeAsync() @@ -32,11 +47,36 @@ await Task.WhenAll( ), PapiClient.RegisterRequestHandlerAsync( "command:paratextBibleSendReceive.syncProjects", - SyncProjects + SyncProjects, + s_sendReceiveTimeout ), PapiClient.RegisterRequestHandlerAsync( "command:paratextBibleSendReceive.cancelSync", CancelSync + ), + PapiClient.RegisterRequestHandlerAsync( + "command:paratextBibleSendReceive.breakSyncLock", + BreakSyncLock, + s_sendReceiveTimeout, + documentation: Create( + "Breaks (releases) the Send/Receive server-side repository lock for each given " + + "project and reports per-project success. Unrelated to the local " + + "in-process sync write gate reported by onSyncWriteLockChanged / " + + "getAutoSyncBlocking. Only implemented in Paratext 10 Studio; throws " + + "PlatformUnimplementedException elsewhere.", + [ + Param( + "projectIds", + "Ids of the projects whose server lock to break. An empty array is a " + + "no-op (empty result, server not contacted).", + "array" + ), + ], + ResultOf( + "object", + "Map of (upper-cased) project id → whether that project's lock was broken" + ) + ) ) ); } @@ -44,8 +84,25 @@ await Task.WhenAll( #endregion #region Protected properties and methods + protected PapiClient PapiClient { get; } = papiClient; + // The three properties below are read only by the closed-source Paratext 10 Studio patch, + // which replaces this class's stub bodies with real implementations. Do not remove them — + // removing them breaks the patch. + protected ParatextProjectDataProviderFactory PdpFactory { get; } = pdpFactory; + + protected AppInfo AppInfo { get; } = appInfo; + + // ParatextProjects in particular exists for the patch's shared sync wrapper (the single helper + // both the manual and the scheduled sync command paths funnel through): core's + // project-directory watcher is non-recursive and cannot see an in-place Settings.xml metadata + // rewrite (name/language/editable) landing during a receive, so after a sync that can change + // the project set or its display metadata the patch must call + // ParatextProjects.RefreshAndNotifyProjectsChanged() for Home / New Tab / the project picker + // to refresh (see that method's doc). + protected LocalParatextProjects ParatextProjects { get; } = paratextProjects; + #endregion #region Protected properties and methods @@ -91,15 +148,10 @@ protected void CommitDaily(String projectId) /// protected void SyncProjects(String[]? projectIds) { - // PT10 integration note: paranext-core's project-directory watcher is non-recursive and - // does NOT catch an in-place Settings.xml metadata rewrite (name/language/editable) landing - // during a Send/Receive receive. PT10's patched implementation of this method must call - // LocalParatextProjects.NotifyProjectsChanged() after a sync that can change the project set - // or display metadata, so Home / New Tab / the project picker refresh. #if DEBUG // Dev-only placeholder: paranext-core has no S/R impl. PT10 patches the whole method. NotificationService.Send( - papiClient, + PapiClient, new("Syncing projects… (dev placeholder)", NotificationSeverity.Info) { Duration = 3000, @@ -130,5 +182,30 @@ protected void CancelSync(NotificationId? notificationId = null) ); } + /// + /// Breaks (releases) the Send/Receive server lock for each given project and reports + /// per-project success. Recovery for a project whose lock is held by the current user + /// THEMSELVES (this same person on another computer, or a previous interrupted sync). The + /// server only permits breaking a lock you own, so this can never break another user's lock. + /// This is the server-side repository lock held on the S/R server — unrelated to the + /// local in-process write gate () reported by the + /// neighboring onSyncWriteLockChanged event / getAutoSyncBlocking command. + /// Exception is thrown if this function is not implemented in the current application. + /// + /// Ids of the projects whose server lock to break. An empty list is + /// a no-op: the result is an empty map and the server is never contacted. + /// Map of (upper-cased) project id → whether that project's lock was broken. + protected Task> BreakSyncLock(List projectIds) + { + // Deliver the fault through the returned task rather than throwing synchronously, so the + // stub's fault mode matches the async Paratext 10 Studio implementation that replaces this + // body (and the method stays async-free, avoiding CS1998). + return Task.FromException>( + new PlatformUnimplementedException( + $"Command '{nameof(BreakSyncLock)}' is not implemented in Platform.Bible. Must be running Paratext 10 Studio to use this command." + ) + ); + } + #endregion } diff --git a/c-sharp/Projects/SendReceive/SendReceiveBlockNotifierService.cs b/c-sharp/Projects/SendReceive/SendReceiveBlockNotifierService.cs index d99e66041af..44c00970012 100644 --- a/c-sharp/Projects/SendReceive/SendReceiveBlockNotifierService.cs +++ b/c-sharp/Projects/SendReceive/SendReceiveBlockNotifierService.cs @@ -1,3 +1,6 @@ +using Paranext.DataProvider.NetworkObjects.Documentation; +using static Paranext.DataProvider.NetworkObjects.Documentation.ExperimentalMethodDocumentation; + namespace Paranext.DataProvider.Projects.SendReceive; /// @@ -21,9 +24,12 @@ namespace Paranext.DataProvider.Projects.SendReceive; /// /// /// -/// Inert in open-source Platform.Bible. Nothing arms in -/// public core, so never fires and the command -/// always returns a not-blocking snapshot. The service is truthful either way — it just has nothing +/// The gate never arms in open-source Platform.Bible. Nothing arms +/// in public core, so +/// never fires there and the command always +/// returns a not-blocking snapshot. Every build — plain Platform.Bible included — still emits the +/// event exactly once per backend (re)start: the not-blocking baseline snapshot at the end of +/// . The service is truthful either way — it just has nothing further /// to report until the Paratext 10 Studio patch brackets a sync with the gate (PT-4210). /// /// @@ -65,11 +71,54 @@ internal class SendReceiveBlockNotifierService(PapiClient papiClient) /// private const string RegisterEventMethod = "network:registerEvent"; + /// + /// OpenRPC documentation sent along with the registration + /// (the C# counterpart of the TypeScript SingleNotificationDocumentation second argument + /// to network:registerEvent). The event is @experimental (see the TS declaration), so + /// this carries the x-experimental wire marker. + /// + private static readonly OpenRpcSingleNotificationDocumentation s_blockStateChangedEventDocumentation = + new() + { + Notification = new() + { + Experimental = true, + Summary = + "Emitted whenever the S/R write gate arms or disarms, carrying the gate's " + + "full current block-state snapshot ({ isBlocking, projectIds }).", + Params = + [ + new() + { + Name = "state", + Summary = "The write gate's current block-state snapshot", + Required = true, + Schema = new() { Type = "object" }, + }, + ], + }, + }; + private PapiClient PapiClient { get; } = papiClient; + /// + /// Registers both wire surfaces and then emits the gate's baseline snapshot, awaited, so the + /// emit attempt completes before this method — and therefore before the startup barrier + /// (Program.cs's critical Task.WhenAll) — returns. That is deliberately NOT a + /// baseline-before-any-arm guarantee: the S/R command registrations are members of the SAME + /// barrier, and Task.WhenAll does not order its members, so a command dispatched + /// mid-barrier can arm the gate and emit before the baseline goes out. That ordering is safe + /// without being prevented, because the baseline is a LIVE read — + /// evaluated at emit time — so a baseline that + /// loses the race carries the armed state rather than overwriting it with a stale not-blocking + /// snapshot. What the await does buy: any arm AFTER the barrier emits strictly behind the + /// already-transmitted baseline (one connection, FIFO delivery) for every subscriber connected + /// throughout. A subscriber that connects later gets no replay either way — it seeds via + /// getAutoSyncBlocking. + /// public async Task InitializeAsync() { - // Subscribe to the gate FIRST, before the registration round-trip below, so no transition + // Subscribe to the gate FIRST, before the registration round-trips below, so no transition // can slip through unsubscribed while the network:registerEvent request is in flight. A // transition that beats registration just announces unregistered (main logs its once-per-name // deprecation warning), which is the right trade for the single block-state signal — a missed @@ -81,34 +130,75 @@ public async Task InitializeAsync() // Register the event with main's central event registry (best-effort — on rejection or // failure, log and continue; emitting unregistered still works, and startup must never break // over this). We accept that a transition arriving during this await may announce before the - // registration completes; see the subscribe-first rationale above. - try + // registration completes; see the subscribe-first rationale above. A local function so the + // try/catch travels with the request into the Task.WhenAll below. + async Task RegisterEventBestEffortAsync() { - bool accepted = await PapiClient.SendRequestAsync( - RegisterEventMethod, - [BlockStateChangedEvent] - ); - if (!accepted) + try + { + bool accepted = await PapiClient.SendRequestAsync( + RegisterEventMethod, + [BlockStateChangedEvent, s_blockStateChangedEventDocumentation] + ); + if (!accepted) + Console.Error.WriteLine( + $"Central registry rejected network event '{BlockStateChangedEvent}' (already " + + "registered by another process?); announcements will warn as unregistered" + ); + } + catch (Exception ex) + { Console.Error.WriteLine( - $"Central registry rejected network event '{BlockStateChangedEvent}' (already " - + "registered by another process?); announcements will warn as unregistered" + $"Failed to register network event '{BlockStateChangedEvent}' with the central " + + $"registry; announcements will warn as unregistered. {ex}" ); + } + } + + // The command registration serves the pull surface: a renderer reads the current snapshot on + // demand instead of waiting for the next transition (GetBlockState returns the snapshot the + // handler serializes straight back to the caller). Run both registrations in parallel — each + // is an independent round-trip to main, and awaiting them serially would let one stalled + // response stack a second full request timeout onto the startup barrier. + await Task.WhenAll( + RegisterEventBestEffortAsync(), + PapiClient.RegisterRequestHandlerAsync( + GetAutoSyncBlockingCommand, + SendReceiveWriteLock.GetBlockState, + null, + Create( + "Returns the S/R write gate's current block-state snapshot ({ isBlocking, " + + "projectIds }) so a renderer can seed its blocking state on demand instead of " + + "waiting for the next onSyncWriteLockChanged transition.", + result: ResultOf("object", "The write gate's current block-state snapshot") + ) + ) + ); + + // Emit the current gate snapshot once now that both surfaces are up, so a subscriber that + // was already listening across a backend restart converges on the fresh process's state + // instead of keeping whatever stale state it last saw. AWAITED — unlike the event-driven + // forwards through OnBlockStateChanged, which must never delay the gate's raise — so the + // emit attempt completes before InitializeAsync (and so the startup barrier) returns, and + // any gate arm AFTER the barrier emits strictly behind it (one connection, FIFO). An arm + // DURING the barrier can still emit first — see this method's doc for why that ordering is + // safe (the GetBlockState() below is a live read at emit time, so a baseline that loses + // that race reports the armed state, not a stale not-blocking one). Still best-effort like + // the registration above: a failed emit is logged, not thrown, because the next gate + // transition re-converges the subscriber and startup must never break over a notify. + try + { + await PapiClient.SendEventAsync( + BlockStateChangedEvent, + SendReceiveWriteLock.GetBlockState() + ); } catch (Exception ex) { Console.Error.WriteLine( - $"Failed to register network event '{BlockStateChangedEvent}' with the central " - + $"registry; announcements will warn as unregistered. {ex}" + $"Failed to emit the baseline {BlockStateChangedEvent} snapshot: {ex}" ); } - - // Register the pull command so a renderer can read the current snapshot on demand instead of - // waiting for the next transition. GetBlockState returns the snapshot the handler serializes - // straight back to the caller. - await PapiClient.RegisterRequestHandlerAsync( - GetAutoSyncBlockingCommand, - SendReceiveWriteLock.GetBlockState - ); } /// diff --git a/src/@types/paratext-bible-send-receive/index.d.ts b/src/@types/paratext-bible-send-receive/index.d.ts index e549d1ebafb..2b5582dc2c5 100644 --- a/src/@types/paratext-bible-send-receive/index.d.ts +++ b/src/@types/paratext-bible-send-receive/index.d.ts @@ -9,6 +9,9 @@ // When the Send/Receive contract changes, re-sync the parts declared here from that file. // NOTE: Preserve any types that exist here but not in the upstream file (structural refinements // added in core before the upstream adopts them). Do not replace the module block wholesale. +// Likewise for declarations that exist in BOTH copies: a re-sync must not downgrade a richer +// declaration here (extra doc detail or type refinements) to a poorer upstream one — merge the +// two, and upstream the improvement instead. // // Why this lives in `src/@types` and not under an extension's `src/types`: // @@ -267,6 +270,21 @@ declare module 'paratext-bible-send-receive' { /** 0–1 fraction; null/undefined ⇒ indeterminate. */ progressValue?: number | null; }; + + /** + * Backend-authoritative snapshot of which projects an automatic Send/Receive is blocking edits on + * (the wire shape of the C# `SendReceiveBlockState`). Carried identically by both the + * `paratextBibleSendReceive.onSyncWriteLockChanged` network event and the + * `paratextBibleSendReceive.getAutoSyncBlocking` command return. + * + * @experimental This type is unstable and may change shape or disappear without notice + */ + export type SyncWriteLockSnapshot = { + /** Whether the S/R write gate is currently blocking edits on any project */ + isBlocking: boolean; + /** Ids of the blocked projects. `isBlocking` false always pairs with an empty array. */ + projectIds: string[]; + }; } declare module 'papi-shared-types' { @@ -274,6 +292,7 @@ declare module 'papi-shared-types' { ResultsData, SyncProgressDetail, SyncProgressEvent, + SyncWriteLockSnapshot, } from 'paratext-bible-send-receive'; import type { SharedProjectsInfo } from 'platform-scripture'; @@ -406,6 +425,54 @@ declare module 'papi-shared-types' { * this command (e.g., Paratext 10 Studio) */ 'paratextBibleSendReceive.cancelSync': (notificationId?: string | number) => Promise; + + /** + * Breaks (releases) the Send/Receive server lock for each given project and reports per-project + * success. Recovery for a project whose lock is held by the current user THEMSELVES (this same + * person on another computer, or a previous interrupted sync). The server only permits breaking + * a lock you own, so this can never break another user's lock. + * + * This is the **server-side repository lock** held on the S/R server — unrelated to the local + * in-process sync write gate reported by the neighboring + * `paratextBibleSendReceive.onSyncWriteLockChanged` event / + * `paratextBibleSendReceive.getAutoSyncBlocking` command. + * + * Note: this command is served from the dotnet process. + * + * @param projectIds Ids of the projects whose server lock to break. An empty array is a no-op: + * it resolves to an empty map without contacting the server. A null array is out of contract + * (the type is `string[]`); implementations treat it like the empty array. Null/blank ids are + * skipped and omitted from the result map. Ids are trimmed, and case-variant duplicates + * collapse to a single upper-cased key (first occurrence wins) + * @returns Map of project id → whether that project's lock was broken. Keys are upper-cased — + * index the map with upper-cased ids. `false` means "not broken", not "attempt failed": an + * attempt may not have been made at all (e.g. the request was refused while a sync was in + * progress), and a later retry can succeed + * @throws `PlatformUnimplementedException` if not running in an application that implements + * this command (e.g., Paratext 10 Studio) + * @experimental This command is unstable and may change or disappear without notice + */ + 'paratextBibleSendReceive.breakSyncLock': ( + projectIds: string[], + ) => Promise<{ [projectId: string]: boolean }>; + + /** + * Returns the S/R write gate's current {@link SyncWriteLockSnapshot} so subscribers can seed + * their blocking state on init instead of assuming unblocked (e.g. after a renderer reload + * during an in-flight sync). + * + * Note: this command is served from the dotnet process. Unlike most Send/Receive commands it is + * registered by core's own `SendReceiveBlockNotifierService` rather than the extension, so it + * is answered on plain Platform.Bible too (always not-blocking there — only Paratext 10 Studio + * arms the gate). The realistic failure mode is a cold-start race: if the dotnet process has + * not registered the command within main's retry budget (~9s), the request rejects — callers + * should keep their fail-safe not-blocking default, and there is no re-query until PT-4265 + * lands. + * + * @returns The write gate's current snapshot + * @experimental This command is unstable and may change or disappear without notice + */ + 'paratextBibleSendReceive.getAutoSyncBlocking': () => Promise; } export interface NetworkEvents { @@ -413,5 +480,15 @@ declare module 'papi-shared-types' { 'paratextBibleSendReceive.onSyncStateChanged': SyncProgressEvent; /** Emitted repeatedly during a sync with the current project name or reconnect status */ 'paratextBibleSendReceive.onSyncProgress': SyncProgressDetail; + /** + * Emitted by the dotnet process whenever the S/R write gate arms or disarms, carrying the + * gate's full current {@link SyncWriteLockSnapshot}. Fires for ALL sync types (manual + + * scheduled + session). The gate only ever arms in Paratext 10 Studio builds — never in plain + * Platform.Bible, where the only emission is a single not-blocking baseline snapshot each time + * the backend (re)starts (every build emits that baseline). + * + * @experimental This event is unstable and may change or disappear without notice + */ + 'paratextBibleSendReceive.onSyncWriteLockChanged': SyncWriteLockSnapshot; } } diff --git a/src/renderer/services/auto-sync-blocking-service.test.ts b/src/renderer/services/auto-sync-blocking-service.test.ts index a1cadd66df9..4bd88ac705a 100644 --- a/src/renderer/services/auto-sync-blocking-service.test.ts +++ b/src/renderer/services/auto-sync-blocking-service.test.ts @@ -1,12 +1,17 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { getNetworkEvent, requestNoRetry } from '@shared/services/network.service'; +import type { SyncWriteLockSnapshot } from 'paratext-bible-send-receive'; +import { sendCommand } from '@shared/services/command.service'; +import { getNetworkEvent } from '@shared/services/network.service'; import { setBlockedProjects } from '@renderer/services/auto-sync-blocking-store'; import { logger } from '@shared/services/logger.service'; import { initAutoSyncBlockingService } from './auto-sync-blocking-service'; +vi.mock('@shared/services/command.service', () => ({ + sendCommand: vi.fn(), +})); + vi.mock('@shared/services/network.service', () => ({ getNetworkEvent: vi.fn(), - requestNoRetry: vi.fn(), })); vi.mock('@renderer/services/auto-sync-blocking-store', () => ({ @@ -19,6 +24,7 @@ vi.mock('@shared/services/logger.service', () => ({ const SYNC_WRITE_LOCK_CHANGED_EVENT = 'paratextBibleSendReceive.onSyncWriteLockChanged'; const LEGACY_BLOCKING_CHANGED_EVENT = 'paratextBibleSendReceive.onAutoSyncBlockingChanged'; +const GET_AUTO_SYNC_BLOCKING_COMMAND = 'paratextBibleSendReceive.getAutoSyncBlocking'; /** Flushes the service's fire-and-forget seeding chain (await + then-callbacks). */ async function flushSeeding(): Promise { @@ -57,7 +63,7 @@ describe('initAutoSyncBlockingService', () => { ); // Default: no core serves the initial-state command (an older core / absent extension). - vi.mocked(requestNoRetry).mockRejectedValue(new Error('command not registered')); + vi.mocked(sendCommand).mockRejectedValue(new Error('command not registered')); }); describe('event source', () => { @@ -101,8 +107,30 @@ describe('initAutoSyncBlockingService', () => { expect(vi.mocked(setBlockedProjects)).toHaveBeenCalledTimes(2); expect(vi.mocked(setBlockedProjects)).toHaveBeenNthCalledWith(1, []); expect(vi.mocked(setBlockedProjects)).toHaveBeenNthCalledWith(2, []); - // Warn once per service lifetime, not once per malformed event. + // Warn once per source per service lifetime, not once per malformed event — and the message + // must name the source that produced the malformed payload. expect(vi.mocked(logger.warn)).toHaveBeenCalledTimes(1); + expect(vi.mocked(logger.warn)).toHaveBeenCalledWith( + expect.stringContaining(`the ${SYNC_WRITE_LOCK_CHANGED_EVENT} event`), + ); + }); + + it('latches the malformed warning per source — init query and event each warn once', async () => { + vi.mocked(sendCommand).mockResolvedValue('malformed'); + initAutoSyncBlockingService(); + await flushSeeding(); // the malformed init-query snapshot warns, latching the query source + if (!capturedHandler) throw new Error('capturedHandler not set'); + capturedHandler(undefined); // a malformed event still warns on its own latch + capturedHandler(undefined); // …but only once + expect(vi.mocked(logger.warn)).toHaveBeenCalledTimes(2); + expect(vi.mocked(logger.warn)).toHaveBeenNthCalledWith( + 1, + expect.stringContaining(`the ${GET_AUTO_SYNC_BLOCKING_COMMAND} init query`), + ); + expect(vi.mocked(logger.warn)).toHaveBeenNthCalledWith( + 2, + expect.stringContaining(`the ${SYNC_WRITE_LOCK_CHANGED_EVENT} event`), + ); }); it('fails safe to block-none when projectIds contains a non-string', () => { @@ -119,13 +147,18 @@ describe('initAutoSyncBlockingService', () => { it('queries the current blocking state on init', async () => { initAutoSyncBlockingService(); await flushSeeding(); - expect(vi.mocked(requestNoRetry)).toHaveBeenCalledWith( - 'command:paratextBibleSendReceive.getAutoSyncBlocking', + expect(vi.mocked(sendCommand)).toHaveBeenCalledWith( + 'paratextBibleSendReceive.getAutoSyncBlocking', ); }); it('seeds the blocked projects when the query reports an in-flight sync (reload mid-sync)', async () => { - vi.mocked(requestNoRetry).mockResolvedValue({ isBlocking: true, projectIds: ['p1'] }); + // `satisfies` pins the well-formed mocks to the seam's wire shape, so a contract change + // breaks these tests at compile time (deliberately-malformed mocks stay untyped). + vi.mocked(sendCommand).mockResolvedValue({ + isBlocking: true, + projectIds: ['p1'], + } satisfies SyncWriteLockSnapshot); initAutoSyncBlockingService(); await flushSeeding(); expect(vi.mocked(setBlockedProjects)).toHaveBeenCalledTimes(1); @@ -133,29 +166,42 @@ describe('initAutoSyncBlockingService', () => { }); it('does not seed when the query reports no sync in flight', async () => { - vi.mocked(requestNoRetry).mockResolvedValue({ isBlocking: false, projectIds: [] }); + vi.mocked(sendCommand).mockResolvedValue({ + isBlocking: false, + projectIds: [], + } satisfies SyncWriteLockSnapshot); initAutoSyncBlockingService(); await flushSeeding(); expect(vi.mocked(setBlockedProjects)).not.toHaveBeenCalled(); }); - it('does not seed when the query result is malformed', async () => { - vi.mocked(requestNoRetry).mockResolvedValue('yes'); + it('does not seed when the query result is malformed, and warns naming the query', async () => { + vi.mocked(sendCommand).mockResolvedValue('yes'); initAutoSyncBlockingService(); await flushSeeding(); expect(vi.mocked(setBlockedProjects)).not.toHaveBeenCalled(); + expect(vi.mocked(logger.warn)).toHaveBeenCalledTimes(1); + expect(vi.mocked(logger.warn)).toHaveBeenCalledWith( + expect.stringContaining(`the ${GET_AUTO_SYNC_BLOCKING_COMMAND} init query`), + ); }); - it('swallows a failed query and keeps the assume-unblocked default', async () => { - vi.mocked(requestNoRetry).mockRejectedValue(new Error('extension absent')); + it('swallows a failed query, keeps the assume-unblocked default, and warns', async () => { + vi.mocked(sendCommand).mockRejectedValue(new Error('backend not up yet')); initAutoSyncBlockingService(); await flushSeeding(); expect(vi.mocked(setBlockedProjects)).not.toHaveBeenCalled(); + // Warn (not debug): the command is registered by core's own backend, so a rejection is + // anomalous — realistically the cold-start race — and there is no re-query until PT-4265. + expect(vi.mocked(logger.warn)).toHaveBeenCalledTimes(1); + expect(vi.mocked(logger.warn)).toHaveBeenCalledWith( + expect.stringContaining('could not read the initial blocking state'), + ); }); it('lets a live event win over the snapshot — no seeding after an event arrived', async () => { let resolveQuery: ((snapshot: unknown) => void) | undefined; - vi.mocked(requestNoRetry).mockImplementation( + vi.mocked(sendCommand).mockImplementation( async () => new Promise((resolve) => { resolveQuery = resolve; @@ -175,7 +221,7 @@ describe('initAutoSyncBlockingService', () => { it('does not seed after cleanup', async () => { let resolveQuery: ((snapshot: unknown) => void) | undefined; - vi.mocked(requestNoRetry).mockImplementation( + vi.mocked(sendCommand).mockImplementation( async () => new Promise((resolve) => { resolveQuery = resolve; diff --git a/src/renderer/services/auto-sync-blocking-service.ts b/src/renderer/services/auto-sync-blocking-service.ts index 03b743662e8..775f97e6bb2 100644 --- a/src/renderer/services/auto-sync-blocking-service.ts +++ b/src/renderer/services/auto-sync-blocking-service.ts @@ -1,30 +1,28 @@ -import { CATEGORY_COMMAND } from '@shared/data/rpc.model'; +import { sendCommand } from '@shared/services/command.service'; import { logger } from '@shared/services/logger.service'; -import { getNetworkEvent, requestNoRetry } from '@shared/services/network.service'; -import { serializeRequestType } from '@shared/utils/util'; +import { getNetworkEvent } from '@shared/services/network.service'; import { getErrorMessage, isString } from 'platform-bible-utils'; import { setBlockedProjects } from './auto-sync-blocking-store'; -/** - * Backend-authoritative snapshot of which projects an automatic Send/Receive is blocking edits on. - * Carried identically by both the change event and the init-consult command (see the C# - * `SendReceiveBlockState`). `isBlocking` false always pairs with an empty `projectIds`. - */ -type SyncWriteLockSnapshot = { isBlocking: boolean; projectIds: string[] }; +// Payload types (`SyncWriteLockSnapshot`) come from the `paratext-bible-send-receive` seam +// declarations in `src/@types/paratext-bible-send-receive/index.d.ts`. -// Backend-authoritative network event: the C# write gate emits a full snapshot on every arm/disarm, -// for ALL sync types (manual + scheduled + session). This is the single signal source for blocking -// — the store never combines it with any other opinion. Only fires in Paratext 10 Studio builds -// where the patch arms the gate; plain Platform.Bible never emits it. +// Backend-authoritative network event (declared in the seam's `NetworkEvents`): the C# write gate +// emits a full snapshot on every arm/disarm, for ALL sync types (manual + scheduled + session). +// This is the single signal source for blocking — the store never combines it with any other +// opinion. The gate only ever arms in Paratext 10 Studio builds (the patch arms it); in plain +// Platform.Bible the only emission is a single not-blocking baseline snapshot each time the +// backend (re)starts. const SYNC_WRITE_LOCK_CHANGED_EVENT = 'paratextBibleSendReceive.onSyncWriteLockChanged'; /** * Command served by the C# backend (the emitter of {@link SYNC_WRITE_LOCK_CHANGED_EVENT}) returning - * the current {@link SyncWriteLockSnapshot}, so this service can seed the store on init instead of - * assuming unblocked. Requested untyped (the same pattern as `paratextBibleSendReceive.cancelSync` - * in shutdown-tasks) because the copied `paratext-bible-send-receive.d.ts` command contract does - * not declare it. Served on Paratext 10 Studio (returns not-blocking on plain Platform.Bible); - * older cores lack it entirely and the request rejects, leaving the assume-unblocked default. + * the current snapshot, so this service can seed the store on init instead of assuming unblocked. + * Declared in the seam's `CommandHandlers`, so it is sent through the typed `sendCommand`. Core's + * own backend registers it, so it is answered on plain Platform.Bible too (always not-blocking + * there). The realistic failure is a cold-start race: the backend has not registered the command + * within main's retry budget (~9s), the request rejects, and the assume-unblocked default stands — + * with no re-query until PT-4265 lands. */ const GET_AUTO_SYNC_BLOCKING_COMMAND = 'paratextBibleSendReceive.getAutoSyncBlocking'; @@ -42,15 +40,15 @@ const GET_AUTO_SYNC_BLOCKING_COMMAND = 'paratextBibleSendReceive.getAutoSyncBloc export function initAutoSyncBlockingService(): () => void { let hasReceivedEvent = false; let isDisposed = false; - let hasWarnedMalformed = false; + const warnedMalformedSources = new Set(); /** * Extracts the project ids to block from an untrusted snapshot payload. A well-formed * not-blocking snapshot yields `[]`; a malformed/missing-field payload also yields `[]` - * (fail-safe assume-unblocked) and warns once per service lifetime, consistent with the - * assume-unblocked init philosophy — a broken signal must never leave the workspace blocked. + * (fail-safe assume-unblocked) and warns once per `source` per service lifetime, consistent with + * the assume-unblocked init philosophy — a broken signal must never leave the workspace blocked. */ - const readBlockedProjectIds = (payload: unknown): string[] => { + const readBlockedProjectIds = (payload: unknown, source: string): string[] => { if ( typeof payload === 'object' && payload && @@ -58,25 +56,27 @@ export function initAutoSyncBlockingService(): () => void { 'projectIds' in payload ) { const { isBlocking, projectIds } = payload; - if (typeof isBlocking === 'boolean' && Array.isArray(projectIds)) { - const stringIds = projectIds.filter(isString); - if (stringIds.length === projectIds.length) return isBlocking ? stringIds : []; - } + if ( + typeof isBlocking === 'boolean' && + Array.isArray(projectIds) && + projectIds.every(isString) + ) + return isBlocking ? projectIds : []; } - if (!hasWarnedMalformed) { - hasWarnedMalformed = true; + if (!warnedMalformedSources.has(source)) { + warnedMalformedSources.add(source); logger.warn( - `auto-sync blocking service received a malformed ${SYNC_WRITE_LOCK_CHANGED_EVENT} snapshot; assuming not blocking`, + `auto-sync blocking service received a malformed snapshot from ${source}; assuming not blocking`, ); } return []; }; - const unsubscribe = getNetworkEvent(SYNC_WRITE_LOCK_CHANGED_EVENT)(( - event, - ) => { + const unsubscribe = getNetworkEvent(SYNC_WRITE_LOCK_CHANGED_EVENT)((event) => { hasReceivedEvent = true; - setBlockedProjects(readBlockedProjectIds(event)); + // `event` is typed by the seam declaration, but it is untrusted wire data — keep the runtime + // validation. + setBlockedProjects(readBlockedProjectIds(event, `the ${SYNC_WRITE_LOCK_CHANGED_EVENT} event`)); }); // LIMITATION: this init consult is the only backend re-seed — there is no re-query on websocket @@ -84,22 +84,31 @@ export function initAutoSyncBlockingService(): () => void { // fire-and-forget SendEventAsync, an off-contract raise reorder, or a provider that restarts // disarmed without re-emitting), the visible blocked set stays stale until the next real // transition or a full renderer reload. Closing that gap needs an editor-mount re-query, tracked - // on PT-4214; until that lands, this one-shot seed is the only recovery. + // on PT-4265; until that lands, this one-shot seed is the only recovery. (async () => { try { - const snapshot = await requestNoRetry<[], unknown>( - serializeRequestType(CATEGORY_COMMAND, GET_AUTO_SYNC_BLOCKING_COMMAND), - ); + // Plain `sendCommand` — with main's retry-on-timeout — is deliberate now that the command is + // declared in the seam; the `requestNoRetry` this call once used was only a workaround for + // the missing declaration. + const snapshot = await sendCommand(GET_AUTO_SYNC_BLOCKING_COMMAND); // Only seed if nothing live has spoken: an event that arrived while the request was in flight // (either direction) supersedes this snapshot and must win, so we never clobber it here. if (!hasReceivedEvent && !isDisposed) { - const blockedProjectIds = readBlockedProjectIds(snapshot); + // `snapshot` is typed by the seam declaration, but it is untrusted wire data — keep the + // runtime validation. + const blockedProjectIds = readBlockedProjectIds( + snapshot, + `the ${GET_AUTO_SYNC_BLOCKING_COMMAND} init query`, + ); if (blockedProjectIds.length > 0) setBlockedProjects(blockedProjectIds); } } catch (e) { - // Command not served (older core) or the extension is absent — keep the assume-unblocked - // default. Debug, not warn: this is the expected path on plain Platform.Bible and older cores. - logger.debug( + // The seam is in-repo-only (core's own backend registers the command and ships with this + // service), so any rejection here is anomalous — realistically the cold-start race: the + // backend had not registered the command within main's retry budget. Keep the + // assume-unblocked default, but warn: with no re-query until PT-4265, a lost seed is worth + // noticing. + logger.warn( `auto-sync blocking service could not read the initial blocking state: ${getErrorMessage(e)}`, ); }