feat(daemon): ping/sentinel socket health checks (replaces drain-progress watchdog)

[Siddhant-K-code]

Summary

Reworks the daemon socket health check from a trace-ingest drain watchdog (which self-restarted the daemon when ingest appeared stalled) into a liveness ping/sentinel mechanism that alerts instead of restarting. The watchdog's aggressive restarts risked memory spikes and restart loops; a heartbeat that surfaces the problem is simpler and less edge-case-prone.

Each health-check tick (every DAEMON_SOCKET_HEALTH_CHECK_SECS, default 30s) the loop now sends a liveness ping over both sockets and tracks when each was last acknowledged:

• Control socket: send ControlRequest::Ping and observe the synchronous round-trip result.
• Trace socket: write a one-way sentinel line {"event":"git_ai_health_ping"}. The trace reader recognizes it in process_trace_connection_line, stamps last_trace_ping_received_ns, and closes the connection without registering a root or enqueuing it for ingest. The loop confirms the watermark advanced.

A small SocketPingHealth detector decides when to alert:

// per socket, once per stall episode; re-arms after recovery
fn observe(&mut self, acknowledged: bool, now: Instant) -> Option<Duration> {
    if threshold.is_zero() || acknowledged { reset(); return None; }
    let since = *self.unacked_since.get_or_insert(now);
    (now - since >= threshold && !self.alerted).then(|| { self.alerted = true; now - since })
}

If a socket goes unacknowledged for DAEMON_PING_STALL_SECS (default 120s, override GIT_AI_DAEMON_PING_STALL_SECS, 0 disables) the loop emits a single tracing::warn! for that episode. It does not restart.

The pre-existing connect-probe restart safety net (dead/stale listener → spawn_self_restart) is retained unchanged — the pings are strictly additive and catch a wedge on an already-established connection that a bare accept() probe cannot.

Because the ping is on the health-check loop, nothing is added to the latency-sensitive trace ingestion path.

Scope note

Socket receive-buffer tuning (raising SO_RCVBUF to absorb trace2 bursts) is split into its own small PR (#1762) so it can land independently of this change.

Tests (TDD)

• socket_ping_health_alerts_once_per_stall_then_rearms — threshold, single alert per episode, re-arm after recovery.
• socket_ping_health_zero_threshold_never_alerts — disable switch.
• trace_health_ping_sentinel_is_recorded_and_not_ingested — sentinel stamps the watermark, closes the connection, registers no root, and never enters the ingest queue (real ActorDaemonCoordinator).
• daemon_trace_health_ping_sentinel_is_accepted_without_disrupting_ingest — end-to-end: after a sentinel, the real daemon still ingests a trace command and answers a control Ping.

Link to Devin session: https://app.devin.ai/sessions/a9931f84ae914fcc82663a2e52106bd5
Requested by: @Siddhant-K-code

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[Siddhant-K-code]

CIs will get fixed after merge & rebase of #1734

[devin-ai-integration]

Sounds good — I'll rebase onto main once #1734 merges and re-target this PR there.

One note: the only red check here is tls_native_certs::test_https_request_uses_system_certs, which failed with a network EINTR (Interrupted system call (os error 4)) on a live HTTPS call to example.com — an infra flake unrelated to both this change and #1734. It should clear on the post-rebase re-run.

[svarlamov]

• feat(daemon): ping/sentinel socket health checks (replaces drain-progress watchdog) #1735 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[Siddhant-K-code]

Pushed 593f3f4

Why

The linked repro is Git trace2 back-pressure: Git writes trace2 frames synchronously to the daemon's Unix socket. With small socket buffers, a large trace burst can fill the buffer and block raw git in write() if the daemon is stopped or slow to drain.

This matches the ~500-file-change case Sasha flagged: macOS-like ~8 KiB buffers make the issue easy to trigger, while Linux's larger defaults can mask it for moderate bursts.

What changed

• Set trace socket receive buffer target to 1 MiB.
• Apply it on the trace listener, so accepted sockets inherit the larger buffer as early as possible.
• Apply it again on each accepted trace connection as a backstop.
• Leave control sockets unchanged.
• Add a Unix-only unit test for the socket buffer helper.

Important

This is a mitigation, not a replacement for the watchdog. A larger buffer absorbs bursty trace output, but any finite buffer can still fill if the daemon stops draining. The watchdog remains the recovery path for real daemon-side stalls.

Validated

• task fmt
• task test TEST_FILTER=trace_socket_recv_buffer_helper
• task test TEST_FILTER=trace_ingest_stall

[Siddhant-K-code]

Pushed 9cb873b93: Windows lint failed because the 1 MiB trace socket buffer constant was compiled on Windows even though it is only used by Unix socket code, so I gated the constant with #[cfg(not(windows))].

This is safe for Windows: the trace daemon uses Windows named pipes there, not Unix sockets, and the setsockopt(SO_RCVBUF) helpers are already Unix-only. The change only aligns the constant with that existing platform boundary. Latest Lint (windows-latest) is green; Windows tests are still running for the broader validation.

[devin-ai-integration]

Pushed 552f34eaf: fixes the other red check — Test on ubuntu-latest core.

The trace_socket_recv_buffer_helper test asserted the socket buffer reaches the full 1 MiB target, but Linux clamps SO_RCVBUF to net.core.rmem_max (and reports back ~2× the stored value). On the CI runners rmem_max is the stock ~208 KiB, so getsockopt returns ~416 KiB — below 1 MiB — and the test failed on every Linux runner (it passed locally on hosts with a larger rmem_max / on macOS, which doesn't clamp or double).

The helper itself is correct; only the assertion was too strict. It now passes if the buffer either reaches the 1 MiB target or grew beyond the socket's default baseline, which holds on both Linux (clamped) and macOS. Fast lane (ubuntu + macOS core, all lint/fmt) is green again.

[Siddhant-K-code]

Current remaining failure is Test on windows-latest integration 5/6, not lint. The log shows broad Windows process startup failures (0xc0000142 / -1073741502) across many unrelated tests, including git-ai --version and daemon startup, while Windows lint, Windows core tests, and Windows integration 2/6 are green on this SHA.

This does not look related to the Unix-only trace socket buffer change. It may be covered by #1739; worth rerunning this PR after that lands.

[devin-ai-integration]

Devin Review found 1 new potential issue.

[devin-ai-integration]

📝 Info: Ping stall detection is alert-only by design, does not trigger restart

The SocketPingHealth detector only logs a warning when a socket stops acknowledging pings (src/daemon.rs:7206-7217). It deliberately does not trigger a restart — only the connect-probe failures at src/daemon.rs:7224 do that. The PR description and code comments explain this is intentional to avoid memory spikes and restart loops. This means a wedged read path that still accepts new connections will only produce log warnings, not self-heal. This is a conscious trade-off documented in the function's doc comment at src/daemon.rs:7131-7138.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

This is intentional, as the PR description calls out. The ping-stall detectors are deliberately alert-only to avoid the restart loops / memory spikes that aggressive drain-based restarts risked.

Hard back-pressure is still self-healing: the connect-probe path just below (src/daemon.rs:7208-7231) continues to spawn_self_restart() + request_shutdown() when a socket stops accepting new connections — which is the failure mode that actually wedges git commands. The ping stalls cover a softer "reader/handler quiet but socket still accepts" case where a warn is the safer signal. Agreed it's worth revisiting if we see stall warnings in production without recovery, but keeping it log-only for now is the deliberate trade-off.

[svarlamov]

havent dug into the details yet, but do these end up as daemon logs so that we can read them on our end?

[devin-ai-integration]

Yes, on both fronts:

• Local daemon log: these are tracing::warn! events, and the daemon's subscriber writes to stderr (src/daemon.rs:7335-7346), which is dup2-redirected into the per-daemon log file (maybe_setup_daemon_log_file). That file lives at ~/.git-ai/internal/daemon/logs/<pid>.log (see daemon_log_file_path, src/daemon.rs:2115-2127), so they're readable on-box. The default EnvFilter is info-level, so WARN is always captured (no GIT_AI_DEBUG=1 needed).
• Server-side: they also flow through DaemonLogUploadLayer (src/daemon/daemon_log_layer.rs), which captures WARN events (Level::WARN -> DaemonLogLevel::Warn) and best-effort uploads them for diagnostics whenever the daemon_log_upload flag is on (default true). So a sustained stall warning shows up on our end without needing the user to pull logs manually.