Follow-ups from the local review panel on PR #90 (issue #82). None block that PR; each was deliberately deferred with a reason.
1. Converge the two spawn-and-signal implementations
ReplProcessProbe (public, Repl.Testing) and ShellCompletionTestHostRunner + Given_ProcessSignals's private helpers (internal, Repl.IntegrationTests) now implement the same primitives side by side: spawn with redirection, capture output, send kill -N pid, wait with a timeout, kill the tree on teardown. PR #90 deliberately did not re-point the existing suite at the new type, so the new code's failures stayed attributable during bake-in.
That reason expires. Trigger: once ReplProcessProbe has shipped a release with the scenarios Given_ProcessSignals covers exercised against it, delete those private helpers in favour of the public type.
One capability gap blocks a like-for-like swap today: Given_ProcessSignals.When_AutomaticRunReceivesSigQuit_Then_OperatingSystemTerminatesProcess sends SIGQUIT, which ReplProcessSignal cannot express — deliberately, because SIGQUIT is #83's business. So this follow-up is gated on #83, not merely on time.
Smaller instance of the same thing: ReplProcessProbe.SendAsync and Given_ProcessSignals.SendSignalAsync both build the same kill invocation.
2. The harness cannot exercise the IHost overloads
ReplProcessSignalHarness runs the application through ReplApp.RunAsync(string[], ReplRunOptions?, CancellationToken) — the only overload that installs the standalone signal bridge. Every overload taking an external IServiceProvider, IHost or IReplHost drops ProcessSignalHandling with a diagnostic.
That means #83's IHostApplicationLifetime/ConsoleLifetime integration, and its "avoid competing with a host that already installed lifetime handlers" question, are not testable through the harness as built. It extends cleanly — a second entry point, or a branch keyed on which overload a test wants — but it does not extend for free. Worth knowing before #83 starts rather than halfway through it.
Related: the harness always passes ProcessSignalHandling.Automatic, so it does not honour the application's own profile. That is right for a signal test, and is why the "run finished without registering a scope" guard is defensive only, but it means the harness cannot be used to assert that an application is correctly configured as caller-owned.
3. Collapse the internal registration observables into #84's public surface
ProcessSignalCoordinator.SigTermRegistrationDeclaredForTesting and SigTermRegistrationInstalledForTesting are, in effect, a private preview of the "declared vs installed" status #84 wants to expose. Good sign for the shape; but when #84 lands a public status type, these two should fold into it rather than live in parallel.
4. Two smaller items
- The real-host platform predicate is expressed twice.
ReplPlatformProfile.Current and ProcessSignalCoordinator's three fallback accessors each call OperatingSystem.Is*() independently. A future platform axis has to be added to both, with nothing forcing it. One internal accessor returning "the real host as a policy" would remove that.
IsSignalBridgeSupportedForTesting(bool, bool, bool, bool) takes four positional booleans. Given_ProcessSignalCancellationScope already mitigates the transposition risk with one DataRow per flag, but any new axis has to touch this signature, SignalRegistrationPolicy and ReplPlatformProfile in lockstep.
Follow-ups from the local review panel on PR #90 (issue #82). None block that PR; each was deliberately deferred with a reason.
1. Converge the two spawn-and-signal implementations
ReplProcessProbe(public,Repl.Testing) andShellCompletionTestHostRunner+Given_ProcessSignals's private helpers (internal,Repl.IntegrationTests) now implement the same primitives side by side: spawn with redirection, capture output, sendkill -N pid, wait with a timeout, kill the tree on teardown. PR #90 deliberately did not re-point the existing suite at the new type, so the new code's failures stayed attributable during bake-in.That reason expires. Trigger: once
ReplProcessProbehas shipped a release with the scenariosGiven_ProcessSignalscovers exercised against it, delete those private helpers in favour of the public type.One capability gap blocks a like-for-like swap today:
Given_ProcessSignals.When_AutomaticRunReceivesSigQuit_Then_OperatingSystemTerminatesProcesssends SIGQUIT, whichReplProcessSignalcannot express — deliberately, because SIGQUIT is #83's business. So this follow-up is gated on #83, not merely on time.Smaller instance of the same thing:
ReplProcessProbe.SendAsyncandGiven_ProcessSignals.SendSignalAsyncboth build the samekillinvocation.2. The harness cannot exercise the
IHostoverloadsReplProcessSignalHarnessruns the application throughReplApp.RunAsync(string[], ReplRunOptions?, CancellationToken)— the only overload that installs the standalone signal bridge. Every overload taking an externalIServiceProvider,IHostorIReplHostdropsProcessSignalHandlingwith a diagnostic.That means #83's
IHostApplicationLifetime/ConsoleLifetimeintegration, and its "avoid competing with a host that already installed lifetime handlers" question, are not testable through the harness as built. It extends cleanly — a second entry point, or a branch keyed on which overload a test wants — but it does not extend for free. Worth knowing before #83 starts rather than halfway through it.Related: the harness always passes
ProcessSignalHandling.Automatic, so it does not honour the application's own profile. That is right for a signal test, and is why the "run finished without registering a scope" guard is defensive only, but it means the harness cannot be used to assert that an application is correctly configured as caller-owned.3. Collapse the internal registration observables into #84's public surface
ProcessSignalCoordinator.SigTermRegistrationDeclaredForTestingandSigTermRegistrationInstalledForTestingare, in effect, a private preview of the "declared vs installed" status #84 wants to expose. Good sign for the shape; but when #84 lands a public status type, these two should fold into it rather than live in parallel.4. Two smaller items
ReplPlatformProfile.CurrentandProcessSignalCoordinator's three fallback accessors each callOperatingSystem.Is*()independently. A future platform axis has to be added to both, with nothing forcing it. One internal accessor returning "the real host as a policy" would remove that.IsSignalBridgeSupportedForTesting(bool, bool, bool, bool)takes four positional booleans.Given_ProcessSignalCancellationScopealready mitigates the transposition risk with oneDataRowper flag, but any new axis has to touch this signature,SignalRegistrationPolicyandReplPlatformProfilein lockstep.