fix: exit through the normal shutdown path on Ctrl+C [patch]

[matt-edmondson]

Fixes #110

The problem

UIApplication.RunAsync() hides the cursor on startup and only restores it in its finally block. Ctrl+C never reached that block: .NET treats it as a process interrupt and terminates the process first, so the user was left with an invisible cursor in their terminal after the process exited — terminal state damage that outlives the program.

As the issue notes, SpectreConsoleProvider.ReadInputAsync() already had a Ctrl+C branch, but it was unreachable: Console.ReadKey never sees Ctrl+C unless TreatControlCAsInput is set. The intent was there; the enabling half was not.

The fix

Two halves, both needed for Ctrl+C to actually exit cleanly:

1. Take the signal. A new internal ConsoleInterruptSource hooks Console.CancelKeyPress and a PosixSignalRegistration for SIGTERM, cancels the runtime's default "terminate now" behaviour, and calls Shutdown(). The existing teardown then restores the cursor. RunAsync registers it for the duration of a run and releases it as the run ends, so a second Ctrl+C arriving during teardown still gets the default behaviour rather than a second shutdown.

Following the triage note, this uses CancelKeyPress rather than TreatControlCAsInput: the event is observed wherever the application is, not only while it happens to be inside a blocking read. SIGTERM is taken for the same reason. Registration is guarded against PlatformNotSupportedException, so a platform without SIGTERM still gets Ctrl+C.

Both delivery mechanisms funnel through one OnSignal(cancelDefaultTermination, onInterrupt). Cancelling comes first and the ordering is asserted: notifying runs application shutdown code, and until the default response is cancelled the runtime is still entitled to kill the process partway through it — which is the failure this PR exists to stop.

2. Let cancellation reach a blocked read. ProcessInputAsync awaited ConsoleProvider.ReadInputAsync() directly. A provider parked in Console.ReadKey does not observe the token, so cancelling only took effect once the user pressed an unrelated key — meaning Shutdown() alone would not have ended the run. The read is now abandoned when the token fires. This fixes the same latency for the CancellationToken callers pass to RunAsync.

The previously-dead branch in ReadInputAsync is kept and commented: it is now correct for a host that has set TreatControlCAsInput itself.

Why an internal seam, not a public contract

IInterruptSource is internal (TUI.Core already has InternalsVisibleTo ktsu.TUI.Test). The real signals cannot be raised from a test without signalling — and very likely terminating — the test host, so the seam exists to make the handling testable. No public API changes, hence [patch].

Tests

TUI.Test/UIApplicationLifecycleTests.cs covers the run lifecycle, using a BlockingConsoleProvider whose read does not complete until the test releases it — standing in for a real provider parked in Console.ReadKey, which is the situation the bug depended on.

Every wait in the class is bounded by a 10s timeout. That is deliberate: the behaviour under test is an application that fails to notice a shutdown request, so an unbounded wait would hang CI on a regression instead of reporting one. (An earlier draft did exactly that.)

Verified the tests fail without the fix. With the two production changes reverted (the new types kept so it still compiles), 4 of the then-7 new tests failed at the timeout and 3 passed:

failed ShutdownEndsARunBlockedOnInput (10s)
failed InterruptSignalEndsTheRunAndRestoresTheCursor (10s)
failed TheInterruptRegistrationIsReleasedWhenTheRunEnds (10s)
failed CancellingTheRunTokenEndsARunBlockedOnInput (10s)
  total: 7   failed: 4   succeeded: 3

The 3 that still passed are the ordinary exit-input path (unchanged by this PR) and the two ConsoleInterruptSource unit tests — correct, since neither depends on the reverted wiring.

Full suite: 129/129 pass. dotnet build TUI.sln is clean across net10.0;net9.0;net8.0.

Real signals, end to end. Unit tests use a fake source, so the actual signal plumbing was checked separately with a throwaway harness built against the real ConsoleInterruptSource. Both SIGINT and SIGTERM reach the callback and the process stays alive rather than being terminated by the runtime:

READY
INTERRUPT CALLBACK FIRED (1)
STILL ALIVE AFTER SIGNAL - process was not terminated by the runtime

This was re-run after the OnSignal refactor, since that commit changed exactly the code the unit tests cannot reach.

Coverage

The first push failed SonarCloud's gate at 69.7% coverage on new code (required ≥ 80%). Measuring locally — git diff origin/main intersected with a cobertura report, the same thing the gate computes — showed the gap was concentrated in the two near-identical signal-handler bodies, which is what motivated consolidating them into OnSignal. Now 90.3% coverage on new code, 0% duplication, 0 new issues, 0 security hotspots.

Four lines remain uncovered: the SIGTERM callback body and the catch (PlatformNotSupportedException) guard. Covering them needs either a real signal delivered to the test host — which risks killing the test run — or a platform that rejects SIGTERM. Making the registration injectable would fake both, but that is a second seam existing only to move a metric, and the guard is genuine defensive code, so I left it.

One thing that is not verified here: driving TUI.App under a pseudo-terminal was inconclusive, because SetCursorVisibility issues a cursor-position query (ESC[6n) that nothing in a headless container answers, so the app never gets past hiding the cursor. That is an environment limitation, unrelated to this change — the acceptance criterion is covered by the lifecycle tests plus the signal harness above.

Merge with main

#113 (for #109) and #118 (for #114) landed while this was open, so main was merged in (a8c808e). UIApplication.cs merged cleanly — #113 rewrote Render's dirty-tracking model, which this branch does not touch.

UIApplicationTests.cs conflicted because both branches filled what had been an empty file, with two unrelated sets of tests. Neither side is redundant, so main's render-loop tests stay in UIApplicationTests.cs unchanged and this branch's run-lifecycle tests moved to UIApplicationLifecycleTests.cs — matching how the suite already splits BorderElementTests from BorderElementRenderTests.

🤖 Generated with Claude Code

https://claude.ai/code/session_01671tcpA4zkfbgPcJm8cTsB

[matt-edmondson]

The SonarCloud quality gate failed this PR on 69.7% coverage on new code (required ≥ 80%). That's this PR's to fix, so: addressed in 380f527.

I measured locally rather than guessing, by intersecting git diff origin/main with a cobertura report — the same thing the gate computes. The gap was concentrated in one place: ConsoleInterruptSource had two near-identical signal-handler bodies that no test could reach, because neither ConsoleCancelEventArgs nor PosixSignalContext can be constructed by a test.

What changed. Both handlers now funnel through a single OnSignal(cancelDefaultTermination, onInterrupt), so the response to a signal is stated once and is assertable separately from the delivery of one. The new test asserts the ordering, not just that both happen — notifying runs application shutdown code, and until the default response is cancelled the runtime is still entitled to kill the process partway through it, so cancelling has to come first. That's the actual invariant this PR rests on.

Also added an assertion that taking an interrupt is logged, through a recording ILogger: an operator reading logs should be able to tell why the app exited.

Result — new-code line coverage measured per file:

File	New executable lines covered
TUI.Core/Services/UIApplication.cs	16/16
TUI.Core/Services/ConsoleInterruptSource.cs	27/31
TUI.Core/Services/SpectreConsoleProvider.cs	0/3
Total	43/50 = 86.0%

Overall project coverage also rose from 48.0% to 54.8%. Suite is 122/122 passing, dotnet build TUI.sln clean across net10.0;net9.0;net8.0.

What is still uncovered, and why I left it. Four lines in ConsoleInterruptSource: the SIGTERM callback body and the catch (PlatformNotSupportedException) guard. Covering them needs either a real signal delivered to the test host — which risks killing the test run, so I'm not doing that in CI — or a platform that rejects SIGTERM. I could make the registration itself injectable to fake both, but that's a second seam existing only to move a metric, and the guard is genuine defensive code I don't think should be deleted to satisfy one. The other 3 are comment lines inside ReadInputAsync, which the coverage tool attributes to the lambda's range; that method calls Console.ReadKey and can't run with stdin redirected.

Separately, the real signal path is the part unit tests can't reach, and this commit refactored exactly that code — so I re-ran the throwaway harness against the real ConsoleInterruptSource afterwards. Both signals still behave:

=== SIGINT ===   INTERRUPT CALLBACK FIRED (1) / STILL ALIVE AFTER SIGNAL
=== SIGTERM ===  INTERRUPT CALLBACK FIRED (1) / STILL ALIVE AFTER SIGNAL

---

Generated by Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
90.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud