watcher: detach from controlling terminal to survive terminal close

[alexeyzimarev]

Summary

• Watcher inherits the coding agent's controlling terminal and process group. When the user closes the terminal, the kernel delivers SIGHUP to every process in the foreground group — codex/Claude and the watcher — and the watcher dies instantly (no SIGHUP handler, default action = terminate). The 5-second parent-PID poll added in [AI-647] Close orphaned session when watcher's parent coding agent exits #75 never gets to run, so PostSessionEndOnParentExitAsync never fires and the session stays "active" in the UI forever. Reproduced with codex session 019e5e36ce9c7053a8ef8adfb67d7857 — log ends mid-stream at 11:08:08 with no shutdown markers, both watcher (22271) and parent (21667) gone.
• Fix: call setsid() at watcher startup so the process moves to a new session with no controlling terminal, and SIGHUP from terminal close no longer reaches it. The captured parentPid (coding agent's PGID, recorded before the watcher spawned) is unchanged and still resolves to the agent's PID, so the existing 5s poll fires when the agent dies and the cleanup path runs as designed.
• Defense-in-depth: also register a SIGHUP handler that mirrors the parent-exit cleanup (set parentExited=1, cancel the CTS, ctx.Cancel = true to suppress the default terminate). Covers the edge case where a shell explicitly forwards SIGHUP to its process-group children before setsid lands, or where setsid fails.
• Vendor-agnostic: same WatcherManager.SpawnWatcher path runs for Claude and Codex. Claude sessions are usually rescued by Claude's own SessionEnd hook firing independently of the watcher, but the watcher is now a reliable backstop for crashes, IDE detach, or SessionEnd failures.

Test plan

• dotnet build src/Kapacitor.Cli/Kapacitor.Cli.csproj — clean
• dotnet publish src/Kapacitor.Cli/Kapacitor.Cli.csproj -c Release — no IL3050/IL2026 AOT warnings
• ProcessHelpersTests (7/7), WatchCommandTests (1/1), WatcherManagerSpawnArgsTests (4/4) pass
• Manual: start a fresh codex session under a new terminal, close the terminal, verify watcher log shows either Parent pid X exited; shutting down watcher (within ~5s) or Received SIGHUP; treating as parent-exit (immediate), followed by Parent-exit session-end POST succeeded, and that the session no longer shows "active" in the UI

🤖 Generated with Claude Code

[qodo-code-review]

Review Summary by Qodo

Detach watcher from controlling terminal to survive terminal close

🐞 Bug fix ✨ Enhancement

Walkthroughs

Description

• Detach watcher from controlling terminal via setsid() to survive terminal close
• Add SIGHUP signal handler as defense-in-depth for edge cases
• Prevent sessions from staying "active" when terminal closes unexpectedly
• Enable reliable parent-exit detection and session-end cleanup

Diagram

flowchart LR
  A["Terminal Close"] -->|SIGHUP to process group| B["Watcher Process"]
  C["setsid() Call"] -->|Detach from terminal| B
  B -->|No controlling terminal| D["SIGHUP Blocked"]
  E["Parent PID Poll"] -->|Detects agent death| F["Session-End POST"]
  G["SIGHUP Handler"] -->|Defense-in-depth| F
  D --> E

Loading

File Changes

1. src/Kapacitor.Cli/Commands/WatchCommand.cs 🐞 Bug fix +31/-10

Add terminal detachment and SIGHUP signal handling

• Call ProcessHelpers.DetachFromControllingTerminal() at startup to detach from controlling
 terminal
• Add SIGHUP signal handler that treats SIGHUP as parent-exit event and triggers cleanup
• Move parentExited variable declaration before signal handlers for clarity
• Add detailed comments explaining terminal detachment and SIGHUP defense strategy

src/Kapacitor.Cli/Commands/WatchCommand.cs

---

2. src/Kapacitor.Cli/ProcessHelpers.cs ✨ Enhancement +47/-0

Add setsid() P/Invoke and terminal detachment method

• Add P/Invoke binding for setsid() libc function
• Implement DetachFromControllingTerminal() public method that calls setsid() on Unix
• Return true on success or Windows (no-op), false if setsid() fails
• Add comprehensive XML documentation explaining terminal detachment mechanism and edge cases

src/Kapacitor.Cli/ProcessHelpers.cs

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Signal registrations not retained ✓ Resolved 🐞 Bug ☼ Reliability

Description

RunWatch drops the IDisposable returned by PosixSignalRegistration.Create for SIGTERM and SIGHUP,
leaving the registrations eligible for GC/finalization and silent unregistration. If that happens,
the watcher can miss SIGTERM/SIGHUP and terminate without running the drain + parent-exit
session-end POST path.

Code

src/Kapacitor.Cli/Commands/WatchCommand.cs[R60-72]

Evidence

Both signal handlers are registered via PosixSignalRegistration.Create(...) but the returned
registration objects aren’t assigned to anything, so nothing roots them for the lifetime of the
watcher. This directly undermines the PR’s new SIGHUP backstop and the existing SIGTERM handling.

src/Kapacitor.Cli/Commands/WatchCommand.cs[55-72]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`PosixSignalRegistration.Create(...)` returns an `IDisposable` registration object. In `RunWatch`, both SIGTERM and SIGHUP registrations are created and immediately discarded, which can allow the registrations to be garbage-collected/finalized and therefore unregistered.

## Issue Context
This PR relies on SIGHUP handling as defense-in-depth when `setsid()` fails or when a shell forwards SIGHUP. If the registration is dropped, that defense can disappear at runtime.

## Fix Focus Areas
- src/Kapacitor.Cli/Commands/WatchCommand.cs[55-72]

Suggested implementation direction:
- Assign each registration to a local `using var` (or store them in fields) for the lifetime of `RunWatch`, e.g. `using var sigtermReg = ...; using var sighupReg = ...;`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. SIGTERM default not canceled ✓ Resolved 🐞 Bug ☼ Reliability

Description

The SIGTERM handler cancels the CTS but does not set PosixSignalContext.Cancel=true, so the default
SIGTERM action may still terminate the process before shutdown/drain completes. This can bypass the
cleanup path that posts session-end on parent exit and defeats the watcher’s graceful-shutdown
intent.

Code

src/Kapacitor.Cli/Commands/WatchCommand.cs[60]

Evidence

SIGTERM registration currently ignores the context entirely, while the newly added SIGHUP handler
explicitly sets ctx.Cancel = true, showing the intended suppression of default termination for
graceful cleanup.

src/Kapacitor.Cli/Commands/WatchCommand.cs[60-72]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The SIGTERM handler ignores the provided `PosixSignalContext`, so the default signal behavior may still run and terminate the process immediately.

## Issue Context
The SIGHUP handler in the same method explicitly sets `ctx.Cancel = true`, indicating the intended pattern to suppress default termination while the watcher runs its shutdown/drain logic.

## Fix Focus Areas
- src/Kapacitor.Cli/Commands/WatchCommand.cs[55-61]

Suggested implementation direction:
- Change SIGTERM registration to accept `ctx` and set `ctx.Cancel = true` after calling `cts.Cancel()` (and keep the returned registration rooted as part of the other fix).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---