fix(agent): stop heartbeat watchdog from firing every heartbeat on slow links (#2386)

[ToddHebebrand]

Closes #2386

Problem

The heartbeat watchdog armed a 15s timer around the whole runHeartbeat() — metrics collection, the primary POST (one 30s-capped context around the retry loop), and after consecutive failures a second full backup-probe POST (another 30s cap). Legitimate worst case is ~60-65s, so on any slow/flapping uplink the watchdog fired on every heartbeat (observed live: every 60s for 2+ days on a macOS device). Each fire did a stop-the-world runtime.Stack(buf, true), allocated 1MiB, and logged a 100KB WARN entry the shipper uploads — which the API rejects (fields > 32,000 stringified chars), 400ing the entire batch and discarding every legitimate log entry shipped with it.

Fix

1. Timeout 15s → 90s. The watchdog was added in Agent: desktop helper reconnect loop storms logs on headless Windows Server (Session 0 only) #387 to catch broker-mutex starvation — an indefinite block that exceeds any finite bound. 90s clears the ~60-65s legitimate worst case (30s primary POST + 30s backup probe + collection overhead) while fully preserving the starvation diagnostic. I chose raising the bound over re-scoping what's timed: re-scoping would require threading the watchdog through sendHeartbeat's internals and would stop covering the metrics-collection hang class the whole-call watchdog currently catches for free.
2. Rate-limit dumps: at most one per 10 minutes, across invocations. Suppressed fires log a cheap one-line WARN with a running suppressed_dumps counter; the next emitted dump reports suppressed_dumps_since_last. Concurrent/overlapping fires race for the slot via CAS.
3. Cap the dump at 8KB raw, cut at a goroutine boundary, with goroutine_count as a separate field. JSON escaping roughly doubles a stack dump, so 8KB leaves comfortable headroom under the API's 32,000-char fields ceiling (worst-case-escaping test included).
4. Shipper hardening: Enqueue now pre-caps any entry's fields at 31,000 JSON bytes (mirroring the API limit, byte-count is conservative vs. JS string length), replacing oversized fields with a small fields_dropped marker — so no single bloated entry can burn a whole batch again, whatever produces it. No per-entry retry protocol; the guard is purely local and pre-buffer.

Tests

• Existing watchdog tests (fires-when-blocked, no-fire-fast-path, cancels-on-panic) still pass; harness now resets the cross-invocation rate-limit state.
• New: rate-limit within interval (1 dump + suppressed-counter WARN), dump-again-after-interval (reports suppressed count), truncateGoroutineDump boundary/marker behavior, worst-case-escaped dump fits the 32,000-char API limit, capFields pass-through/replacement, and Enqueue capping an oversized entry while preserving the message.
• cd agent && go test -race ./internal/heartbeat/... ./internal/logging/... — green. gofmt clean on touched files, go vet clean.

🤖 Generated with Claude Code

[cloudflare-workers-and-pages]

Deploying breeze with    Cloudflare Pages

Latest commit:	0ebe700
Status:	✅  Deploy successful!
Preview URL:	https://d9450258.breeze-9te.pages.dev
Branch Preview URL:	https://fix-2386-heartbeat-watchdog.breeze-9te.pages.dev

View logs

[ToddHebebrand]

Review run: /pr-review-toolkit:review-pr (code-reviewer, pr-test-analyzer, silent-failure-hunter, comment-analyzer), plus a focused code-reviewer re-pass on the follow-up commit.

Findings: 10 raised across the four agents → all addressed in 0ebe700; 0 outstanding.

• HIGH — capFields marshal-error path passed unmarshalable fields through, guaranteeing a whole-batch drop in shipBatch (the marshal-failure flavor of the [Agent] Heartbeat watchdog fires on every heartbeat over a slow link, shipping 100KB goroutine dumps the API rejects #2386 bug) → now dropped by name via the same marker path.
• MEDIUM — oversized-fields marker discarded all co-resident fields → small correlating scalars (ids, durations) are now salvaged under a per-field 1KB cap and a half-limit running budget; dropped keys are named with sizes.
• MEDIUM — 8KB dump cap trimmed the local copy too → raised to 12KB (still <32,000 chars at worst-case 2x escaping, with the capFields backstop for pathological <>&-heavy dumps).
• Test quality — sleep-racy blocked-heartbeat helper replaced with observe-the-fire-then-release; added deterministic CAS-concurrency, interval-boundary, and production-defaults tests plus a NaN capFields test.
• Comment precision — backup-probe threshold, JSON-inflation phrasing, byte-vs-char claim scoping, rate-limit-slot note.

Not addressed (pre-existing, out of scope): shipper batches up to 500 entries while the API caps logs at 200 per request — the same burn-the-batch class, but present before this PR; worth its own issue.

Tests: cd agent && go test -race -count=2 ./internal/heartbeat/... ./internal/logging/... green (includes the three pre-existing watchdog tests, updated); go vet clean; touched files gofmt-clean. No TS changes, so no tsc run.

Status: review-clean, awaiting maintainer merge.

[ToddHebebrand]

Independent re-review of follow-up commit 0ebe700: clean — go vet clean, internal/heartbeat + internal/logging pass go test -race -count=3. Budget arithmetic in capFields verified sound against escaping-heavy values (value-side expansion is counted post-marshal). One theoretical gap noted, not reachable in this codebase: used counts raw key length, not escaped — dynamic keys containing <>& could re-breach the cap, but slog field keys here are always short ASCII literals. Optional belt-and-suspenders if ever desired: a final json.Marshal re-check after the salvage loop. Not blocking.