Skip to content

feat(orchestrator): add CLOSE_WAIT socket-spin watchdog for #264 signature - #266

Merged
chrisleekr merged 3 commits into
mainfrom
feat/issue-265-socket-watchdog
Jul 18, 2026
Merged

feat(orchestrator): add CLOSE_WAIT socket-spin watchdog for #264 signature#266
chrisleekr merged 3 commits into
mainfrom
feat/issue-265-socket-watchdog

Conversation

@chrisleekr

@chrisleekr chrisleekr commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Adds a Linux-only watchdog that samples /proc/net/tcp{,6} on a timer, detects sockets stuck in CLOSE_WAIT (state 08) on the orchestrator's HTTP listener port keyed by inode, and structurally logs the #264 signature once it persists across N samples. If CPU is also pinned to a full core AND self-heal is explicitly enabled (default OFF), it exits 75 (EX_TEMPFAIL) so k8s restarts the pod with a distinguishable lastState.terminated.exitCode.

Closes #265
Relates to #264

What / Why

The #264 death left no trace: the container burned exactly 1.000 CPU core for 14 days and died reason=Error with nothing to read afterwards. This PR does not fix #264, it instruments the signature so the next occurrence self-documents, and optionally bounds the burn.

What this does NOT do

Design

  • Pure /proc parser (src/orchestrator/proc-net-tcp.ts): per-word IPv6 hex decode, no fs/timers, whitespace-tokenized never fixed-offset, fail-open on malformed rows, row-count capped.
  • Pure detector state machine + thin timer (src/orchestrator/socket-health.ts), wired into startup/shutdown in src/app.ts.
  • Fail-open throughout: no procfs (macOS dev) disables the watchdog; a read error warns once and continues; it never crashes the process it watches.
  • Persistent CLOSE_WAIT is the trigger; CPU alone is never sufficient (a legitimate 13.5s scheduler.scan burns a core without leaking sockets).
  • 5 new SOCKET_HEALTH_* env vars (Group 7 in src/config.ts), self-heal opt-in and default off.
  • New Zod-.strict() Pino event family (src/orchestrator/socket-health-log-fields.ts) pinning every emitted field against its co-located test.
  • docs/operate/configuration.md, docs/operate/observability.md, and env-contract.json updated to match.
flowchart LR
  TCP["/proc/net/tcp<br/>and tcp6"]:::src --> PARSE["proc-net-tcp.ts<br/>pure parser"]:::keep
  PARSE --> DETECT["socket-health.ts<br/>detector state machine"]:::keep
  DETECT -->|N samples leaked| LOG["Pino event<br/>close_wait.detected"]:::keep
  DETECT -->|leak plus CPU pinned<br/>and self-heal enabled| EXIT["process.exit 75<br/>EX_TEMPFAIL"]:::warn
  EXIT --> K8S["k8s restarts pod<br/>distinguishable exitCode"]:::keep
  classDef src fill:#2c3e50,color:#ffffff
  classDef keep fill:#2c3e50,color:#ffffff
  classDef warn fill:#ecf0f1,color:#2c3e50
Loading

Testing

100 new tests: parser against real captured /proc fixtures (IPv4-mapped ::ffff: word-order trap, port-not-swapped, inode keying), detector state machine with injected deps + fake timers, schema pinning, and config bounds. CI's per-file test isolation (scripts/test-isolated.sh) shows the same failing-file set as clean main (pre-existing infra-gated skips), so this is zero regressions.

Why exit 75 and not 1: a distinct code lets lastState.terminated.exitCode distinguish a deliberate self-heal from a real crash, exactly the ambiguity that made the original #264 death undiagnosable.

Summary by CodeRabbit

  • New Features

    • Added socket health monitoring to detect persistent CLOSE_WAIT socket leaks and CPU-related connection issues.
    • Added optional self-healing behavior that can restart the process when severe socket issues are detected.
    • Added configurable monitoring intervals, thresholds, CPU limits, and self-healing controls.
    • Added structured watchdog events for detection, failures, disabled monitoring, and self-healing exits.
  • Documentation

    • Documented socket health configuration, watchdog events, and recommended alerting guidance.
  • Tests

    • Added coverage for TCP parsing, socket detection, configuration validation, logging, and watchdog behavior.

…ature

Adds a Linux-only watchdog that samples /proc/net/tcp{,6} on a timer, detects
sockets stuck in CLOSE_WAIT (state 08) on the orchestrator HTTP listener port
keyed by inode, and structurally logs the #264 signature once it persists across
N samples. If CPU is also pinned to a full core AND self-heal is explicitly
enabled (default OFF), it exits 75 (EX_TEMPFAIL) so k8s restarts the pod with a
distinguishable lastState.terminated.exitCode.

This does NOT fix #264 (the underlying Bun defect, real but dormant, not
reproducible across 3 labs / ~28k closes). The server.setTimeout mitigation
proposed there is disproven and unsafe (fires on a 4s wheel regardless of the
configured value, and silently drops any response slower than that; scheduler.scan
takes 13.5s). This instruments the signature so the next occurrence self-documents
and optionally bounds the burn.

Persistent CLOSE_WAIT is the trigger; CPU is never sufficient alone. Fail-open
throughout: no procfs disables the watchdog, a read error warns once and continues,
never crashing the process it watches. 5 new SOCKET_HEALTH_* env vars, self-heal
opt-in. New Zod-strict Pino event family. Docs + env-contract.json updated.

Closes #265
Relates to #264

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@chrisleekr, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 30 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6f539617-521e-4633-aafa-d3c46afa4d0c

📥 Commits

Reviewing files that changed from the base of the PR and between dd281b4 and 5360baf.

📒 Files selected for processing (5)
  • src/app.ts
  • src/orchestrator/proc-net-tcp.ts
  • src/orchestrator/socket-health.ts
  • test/orchestrator/proc-net-tcp.test.ts
  • test/orchestrator/socket-health.test.ts
📝 Walkthrough

Walkthrough

Adds a configurable socket-health watchdog that parses Linux TCP procfs tables, detects persistent CLOSE_WAIT and CPU-spin signatures, emits structured events, optionally exits with code 75, integrates with startup and shutdown, and documents the new controls and observability events.

Changes

Socket health watchdog

Layer / File(s) Summary
Watchdog configuration
src/config.ts, env-contract.json, docs/operate/configuration.md, test/config.test.ts
Adds socket-health environment settings, schema defaults and validation, strict boolean parsing, configuration documentation, and coverage for defaults and boundaries.
Procfs socket parsing
src/orchestrator/proc-net-tcp.ts, test/orchestrator/fixtures/*, test/orchestrator/proc-net-tcp.test.ts
Parses IPv4 and IPv6 /proc/net/tcp data, formats endpoints, filters CLOSE_WAIT sockets by port, excludes inode 0, and deduplicates sockets by inode.
Detection and structured logging
src/orchestrator/socket-health.ts, src/orchestrator/socket-health-log-fields.ts, test/orchestrator/socket-health.test.ts, test/orchestrator/socket-health-log-fields.test.ts
Tracks socket persistence, calculates CPU usage, classifies leak and spin states, handles procfs failures, emits validated events, and optionally exits with code 75.
Runtime wiring and operations
src/app.ts, docs/operate/observability.md
Starts the watchdog during startup, stops it during shutdown, and documents emitted events, alerts, self-healing, and exit semantics.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant App
  participant SocketHealthWatchdog
  participant Procfs
  participant Logger
  participant Process
  App->>SocketHealthWatchdog: start watchdog during startup
  SocketHealthWatchdog->>Procfs: read tcp and tcp6 tables
  Procfs-->>SocketHealthWatchdog: CLOSE_WAIT socket records
  SocketHealthWatchdog->>Logger: emit detection or spin event
  SocketHealthWatchdog->>Process: exit with code 75 when enabled
  App->>SocketHealthWatchdog: stop watchdog during shutdown
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: a CLOSE_WAIT socket-spin watchdog for issue #264.
Linked Issues check ✅ Passed The changes add the watchdog, self-heal path, fail-open behavior, docs, env contract, and tests aligned with #264/#265.
Out of Scope Changes check ✅ Passed The modified docs, config, runtime, and tests all support the watchdog scope; no unrelated changes are evident.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/orchestrator/proc-net-tcp.ts`:
- Around line 153-155: The proc-net TCP parser currently limits split input and
can miss watched rows after the prefix; update the parsing loop around the
table-processing function to scan the complete input without materializing an
unbounded line array, while limiting only retained results to MAX_ROWS. In
test/orchestrator/proc-net-tcp.test.ts lines 110-133, add a regression case with
over 5,000 noise rows followed by a valid watched CLOSE_WAIT row, verifying the
later row is detected.

In `@src/orchestrator/socket-health.ts`:
- Around line 356-405: Update src/orchestrator/socket-health.ts lines 356-405 to
add watchdog lifecycle cancellation/generation checks, invalidate pending probes
during stopSocketHealthWatchdog, and serialize interval-triggered sampleOnce
executions so stale or overlapping samples cannot log or trigger self-healing
after shutdown; update src/app.ts line 792 to call stopSocketHealthWatchdog()
before server.close().
- Around line 230-235: Update the IPv6 read handling around the tcp6 readFile
call to catch the filesystem error, return an empty table only when its code is
ENOENT, and rethrow all other failures so the existing sample_failed handling
can report them. Preserve the current successful read and missing-file behavior.

In `@test/orchestrator/socket-health.test.ts`:
- Around line 376-388: Refactor captureStdout to use async/await instead of the
current then/finally promise chain. Preserve the existing stdout interception,
captured chunk joining, and guaranteed restoration of process.stdout.write by
keeping cleanup in a finally block.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f5ac96b8-040d-4c05-855a-746fd629f229

📥 Commits

Reviewing files that changed from the base of the PR and between 76e7810 and dd281b4.

📒 Files selected for processing (14)
  • docs/operate/configuration.md
  • docs/operate/observability.md
  • env-contract.json
  • src/app.ts
  • src/config.ts
  • src/orchestrator/proc-net-tcp.ts
  • src/orchestrator/socket-health-log-fields.ts
  • src/orchestrator/socket-health.ts
  • test/config.test.ts
  • test/orchestrator/fixtures/proc-net-tcp.txt
  • test/orchestrator/fixtures/proc-net-tcp6.txt
  • test/orchestrator/proc-net-tcp.test.ts
  • test/orchestrator/socket-health-log-fields.test.ts
  • test/orchestrator/socket-health.test.ts

Comment thread src/orchestrator/proc-net-tcp.ts Outdated
Comment thread src/orchestrator/socket-health.ts
Comment thread src/orchestrator/socket-health.ts
Comment thread test/orchestrator/socket-health.test.ts Outdated
chrisleekr and others added 2 commits July 18, 2026 02:05
- proc-net-tcp: stream lines via a generator and filter CLOSE_WAIT inline in
  collectCloseWaitSockets, so a large table neither drops watched rows past a
  positional cap nor materializes a full-table array (replaces the flawed
  split-with-limit cap).
- socket-health: the tcp6 read catch now only treats ENOENT as "IPv6 disabled";
  every other error (EMFILE/EACCES) rethrows so sample_failed fires, mirroring
  the tcp4 fix.
- app/socket-health: disarm the watchdog at the top of shutdown() before
  server.close(), since stuck CLOSE_WAIT can block close() forever; a generation
  counter plus a re-entrancy latch stops an in-flight sample from logging or
  exiting 75 after stop, so a graceful shutdown cannot trip the self-heal signal.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses CodeRabbit trivial finding. Behavior-preserving; no assertion changed.
Scoped require-atomic-updates disable on the single-threaded global restore.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@chrisleekr
chrisleekr merged commit 8990ee0 into main Jul 18, 2026
9 checks passed
@chrisleekr
chrisleekr deleted the feat/issue-265-socket-watchdog branch July 18, 2026 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

1 participant