[siebe41]

Decisions for Andrew

• Which flaky test. The flake Pre-V1 audit: reliability under load (gateway latency, long runs, SQLite concurrency, flaky test) #225 came from (DurableStateHostTests.Purged_activity_cannot_be_recovered_from_the_database_files, Flaky test: Purged_activity_cannot_be_recovered_from_the_database_files fails when another test class clears every SQLite pool #234) was already fixed by Reliability under load: trackers, early-ended requests, stall timeout, retries, SQLite test isolation #239, with 200 clean runs then. CI's failure history since then shows a different one, failing only on Windows: ServiceTests.The_router_records_itself_while_it_runs_and_reports_an_address_in_use timed out in runs 39414399709, 39498310147 and 39554713148 (all on sprint branches that don't touch the host), and in the last of these together with LifecycleTests.A_restart_onto_a_taken_address_keeps_the_old_one. This PR fixes that one. Alternative: close H08 as "already fixed" and leave this flake for another task.
• Fix in the product, not by serialising the tests. The cause is in how a Windows router is stopped (below). Putting every test class that starts a router in-process into one non-parallel xUnit collection would also have worked, but any new test that starts a router would bring the flake back. Changing the product means service stop can only ever stop the router its record names.
• No CHANGELOG line. Outside the tests, a process runs one router at a time, so a user can't notice the difference. CONTRIBUTING.md asks for no line for changes like this.

What changed

• A Windows router now waits on an event of its own, Local\semantic-router-stop-<pid>-<guid>, and names it in semantic-router.instance.json (stopEvent). RouterInstance.RequestStop (used by service stop and the tests) sets that event.
• For compatibility it also still waits on the per-process event Local\semantic-router-stop-<pid>. An earlier release's service stop sets that one, and a record without stopEvent (written by an earlier release's router) makes the new service stop set it too.
• The router starts listening before it writes its record, so whoever reads the record can stop it. Before, there was a short window where the record existed but RequestStop found no event and service stop fell back to killing the process.
• New test ServiceTests.A_stop_request_reaches_only_the_router_it_names_and_an_earlier_release_can_still_send_one, and The_router_records_itself_… checks that a Windows record carries its stop event.
• docs/host.md: the instance file names the stop event on Windows.

Why

Every router in a process waited on the same auto-reset event, Local\semantic-router-stop-<pid>. Setting an auto-reset event wakes exactly one waiter. The test host runs ServiceTests, LifecycleTests and HostTests in parallel, and each starts routers in-process. So when ServiceTests asked its router to stop, Windows sometimes woke another test's router instead:

• ServiceTests waited 30 s for a router that never got the signal: TimeoutException at ServiceTests.cs:371, as in all three CI failures.
• LifecycleTests' router shut down mid-test: "No connection could be made because the target machine actively refused it", as in run 39554713148.

Linux and macOS stop the router with SIGTERM, which is sent to the process, so they never had this problem. That's why the flake appeared only on Windows.

How it was verified

All on this PC (Windows 11, 24 CPUs, shared with the other sprint lanes).

• Red first. The new test, written against main's API (two listeners, a stop request), failed on main: the router that listened first took the signal, and the named one timed out after 10 s.
• Baseline on main. dotnet test -c Release --no-build --filter "FullyQualifiedName~SemanticRouter.Tests.Host.ServiceTests|FullyQualifiedName~SemanticRouter.Tests.Host.LifecycleTests|FullyQualifiedName~SemanticRouter.Tests.Host.HostTests", looped 30 times: 8 of 30 runs failed, every time on exactly the CI pair (ServiceTests.The_router_records_itself_… 30 s timeout, LifecycleTests.A_restart_onto_a_taken_address_keeps_the_old_one connection refused).
• With the fix. The same filter (73 tests, including the new one), looped 200 times: 200 of 200 runs passed.
• dotnet build -c Release: 0 warnings, 0 errors.
• Full suite, dotnet test -c Release: 1463 of 1463 passed in 54 s (with OPENAI_API_KEY unset for this desktop, see H09).
• dotnet format --verify-no-changes: clean.
• No dotnet pack: SemanticRouter.Hosting is not packable, and no public API changed (RouterInstance is internal).

Other OSes. The change only affects Windows: on Linux and macOS, stopEvent stays null and no event is created, as before. The new test's compatibility check (an earlier release's record reads back with no stop event) runs on all three OSes. Its event checks run only on Windows, like the code they test. The second part uses a made-up process id, so setting the per-process event can't wake a router from another test.

Not done

• UpgradeFromReleasedVersionsTests.The_current_build_takes_over_a_home_made_by(release: "0.3.0") failed once on Windows CI (run 39533851908, a 500 from the router). That is a different failure, not looked into here.
• The 3 phone screenshot tests in dashboard-e2e fail on Windows on main too. This PR doesn't touch the dashboard.

Refs #225

[siebe41]

Looks right: the diagnosis is correct and the fix is the right size. Each Windows router now waits on an event of its own, and stopping still works in both directions between versions. I reproduced the flake on main and could not reproduce it on this branch. The findings below are small, and none of them blocks the merge.

A security finding was written up for the maintainer outside this thread.

Findings

1. Low: if starting fails after Kestrel listens, the router keeps the port and its stop events. src/SemanticRouter.Hosting/RouterHost.cs:339-340, src/SemanticRouter.Hosting/RouterInstance.cs:112-125

RouterHost.StartAsync has already started the app by the time it creates the stop events and writes the record. If instance.Write throws (the configuration directory is read-only, the disk is full, or another program holds the file), nobody disposes the app or, now, the stop listeners. The desktop app catches that IOException / UnauthorizedAccessException (DesktopRouter.cs:66) and shows "Could not start". Meanwhile the router it started keeps listening, so the next Start reports the address as in use. The app leak exists on main too. This PR adds to it: the listeners are now created before Write, and ListenForStop creates two events where it used to create one. If the second creation throws, the first event's registration and handle are never released.

Fix: in ListenForStop, dispose the listeners already created if a later one throws. In StartAsync, wrap ListenForStop and Write in a try whose catch disposes the listener and the app (await app.DisposeAsync()) and then rethrows.

2. Nit: RequestStop uses the record's stopEvent exactly as written. src/SemanticRouter.Hosting/RouterInstance.cs:131

A record edited by hand or corrupted can contain "stopEvent": "". Then EventWaitHandle.TryOpenExisting("") throws ArgumentException (I checked this on Windows), and service stop fails with an unhandled exception. It would be better if it fell back to the per-process event or to ending the process. The record is owner-only, so this is about robustness, not access. Fix: accept StopEvent only when it starts with StopEventName(ProcessId) + "-", which NewStopEventName always produces, and otherwise use StopEventName(ProcessId). That also ties the event name to the process id in the same record.

3. Nit: a desktop comment says the opposite of what the code does now. src/SemanticRouter.Desktop/DesktopRouter.cs:94

"on Windows, semantic-router service stop signals every router on the directory": that was already inaccurate, since it set a per-process event. After this PR, service stop stops exactly the router the record names. Suggest "Stopped from outside (on Windows, service stop sets the stop event this router's record names)".

4. Follow-up, not for this PR: a stale record plus a reused process id makes service stop end an unrelated process. src/SemanticRouter.Hosting/ServicePlatforms.cs:582-596

This happens on main too. A router that crashes, or a PC that loses power, leaves semantic-router.instance.json behind. ReadRunning only checks that some process with that id is alive. When the id has been reused, RequestStop finds no event and StopAsync calls process.Kill(entireProcessTree: true) on whatever process now has that id. This also runs on service restart, service uninstall and the desktop app's Turn off. One fix would be to have IsAlive also require process.StartTime <= StartedAt. That is worth its own issue.

Checked and fine

• How the name is made, and who creates and opens it. The name is per router instance: Local\semantic-router-stop-<pid>-<guid> (NewStopEventName). The router creates it in its own process (RouterHost.cs:338-339) and writes it to its record. service stop reads the record and opens that name (ServicePlatforms.cs:582 → RequestStop). The desktop app's Turn off and Restart use the same platform.StopAsync. Nothing else in the product calls RequestStop.
• Stopping across versions, both directions. The v0.5.0 RouterInstance is the same as main's before this PR: it uses the per-process name only, and its JSON options are JsonSerializerDefaults.Web with no unmapped-member setting.
  • New CLI, router from 0.5.0: the record has no stopEvent, so RequestStop sets Local\semantic-router-stop-<pid>, which that router waits on.
  • CLI from 0.5.0, new router: the old reader skips the unknown stopEvent property and sets the per-process event. The new router still listens on that event.
    The new test covers both the old record shape and the per-process listener.
• Namespace and session. It stays Local\, as before. The Windows background service is an HKCU Run entry started in the user's own session, not a session-0 Windows service, so service stop and the router see the same namespace, as they did before. The GUID name can't be guessed, and it is published only in the owner-only instance file.
• The start-up race. The listener is now created before the record is written (RouterHost.cs:339-340), so anyone who can read the record can stop the router. On main, a service stop in that window found no event and fell back to Kill.
• Auto-reset semantics. The per-router event has a single waiter, the registration is executeOnlyOnce, and a second Set from a concurrent service stop is harmless. The shared per-process event still wakes only one waiter. That is fine: outside tests a process runs one router at a time (the RunAsync loop disposes the old router before it starts the next one, and DesktopRouter holds one), and no test sets the real per-process event any more.
• Disposal. RunningRouter.DisposeAsync releases both registrations and handles, through StopListeners, before it stops the app.
• Linux and macOS. No event is created, and SIGTERM handling is unchanged. The only difference is that the record now contains "stopEvent": null, because the Web defaults write nulls. That is harmless, and the docs only promise the field on Windows.
• The regression test fails without the fix. I built the branch in a scratch worktree with two mutations:
  • RequestStop opens the per-process name, as main does: the new test failed 5 of 5 runs with a 10 s TimeoutException, because the router that listened first took the signal.
  • ListenForStop without the per-process listener: the test failed 3 of 3 on RequestStop().Should().BeTrue().
    The Windows-only part returns early on other OSes. That is how the other OS-specific tests in this repo do it (xUnit 2.9.3 has no runtime skip). The compatibility check before the early return runs on every OS. Part two uses a made-up process id, so it can't wake another test's router.
• The flake itself. I ran the ServiceTests|LifecycleTests|HostTests filter in a loop on this Windows 11 desktop. On main, 7 of 20 runs failed, each time on exactly the CI pair (The_router_records_itself_… 30 s timeout, plus A_restart_onto_a_taken_address_keeps_the_old_one). On this branch, all 20 of 20 passed (73 tests per run).
• CHANGELOG. I agree that no line is needed. CONTRIBUTING asks for one for changes a user could notice, and a single router per process behaves the same as before.
• Docs and PR body. The two docs/host.md sentences match the code. What the PR description says about the cause, the compatibility path, the start-up ordering and the other OSes matches the diff.