feat(agent): runtime memory gauges on heartbeat + on-demand pprof capture (#2389) - #2394
Merged
Merged
Conversation
…ture command (#2389) The agent had zero runtime memory instrumentation — no ReadMemStats, no pprof — so field memory issues (e.g. the 2.5 GB macOS agent in #2387) could only be diagnosed by inference from outside the process. Two additions, per the shape agreed on the issue: - Always-on gauges: collect runtime.ReadMemStats (HeapAlloc, HeapInuse, HeapReleased, Sys, NumGC) + NumGoroutine on every heartbeat into a new agentRuntime payload field (~µs cost). The API heartbeatSchema gains a matching tolerant optional object (.catch(undefined), uint64-safe) and persists it into the existing device_metrics.custom_metrics jsonb column — no migration. - On-demand profiles: new capture_pprof privileged command (clones the set_log_level pattern) captures heap and/or goroutine profiles in-process via runtime/pprof and returns them base64 in the command result (1 MiB raw cap per profile, well inside the 5 MB stdout cap). Heap capture forces a GC first so the profile reflects live objects. Registered in CommandTypes server-side. Security: no pprof HTTP listener at all — the agent is a root daemon and exposes nothing reachable off-box; the signed/queued command path is the only trigger, and nothing is enabled by default. Closes #2389 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deploying breeze with
|
| Latest commit: |
5596942
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://08b102cf.breeze-9te.pages.dev |
| Branch Preview URL: | https://feat-2389-agent-memory-instr.breeze-9te.pages.dev |
- Add CAPTURE_PPROF to SHORT_TIMEOUT_TYPES (5-min reap window, matching set_log_level; also silences the unknown-type default-timeout warn). - Warn loudly (with the gauge values) when agentRuntime arrives on a heartbeat without metrics — the device_metrics OS columns are NOT NULL so there is no row to attach the gauges to, and that state (metrics collector failing) is exactly where a memory-sick agent likely is; the drop must be observable. + route test. - Reject non-string `profile` payload values in handleCapturePprof instead of silently defaulting to "all". + test. - Pin the outer "agentRuntime" heartbeat wire key (present when set, omitted when nil) with a Go test — a tag rename would silently darken the gauges fleet-wide with all other tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Review run: /pr-review-toolkit:review-pr (code-reviewer, pr-test-analyzer, silent-failure-hunter) Findings: 4 raised → all addressed in 5596942; 0 outstanding.
Conscious decision left for the maintainer: Tests: Status: review-clean, awaiting maintainer merge. |
This was referenced Jul 12, 2026
Closed
This was referenced Jul 13, 2026
ToddHebebrand
added a commit
that referenced
this pull request
Jul 13, 2026
…rt ephemeral watchdog tier (#2400) (#2408) Closes #2400 Two follow-ups from the #2387 worker-pool wedge investigation, unblocked by PR #2394 (agentRuntime heartbeat gauges) and PR #2395 (log-only in-flight watchdog). ## 1. Wedged/in-flight command count on the heartbeat - `executeCommandViaPool` now registers every pool-dispatched command in an in-flight tracker (start time + its watchdog tier, keyed by a per-dispatch sequence number so duplicate command IDs can't clobber each other) and deregisters when the dispatch loop exits. - The heartbeat reports two new gauges on the `agentRuntime` payload object from #2394: `commandsInFlight` (commands currently executing on the pool) and `commandsOverdue` (commands running longer than their watchdog tier — wedged-worker suspects). They land in `device_metrics.custom_metrics` through the existing #2394 persistence path, so no migration and no `heartbeat.ts` change. - API `heartbeatSchema`: the new fields are per-field `.optional().catch(undefined)` inside the existing tolerant `agentRuntime` object, so pre-#2400 agents (which omit them) keep their whole `agentRuntime` object and a bad value drops only that field. ## 2. Short watchdog tier for ephemeral commands - `isEphemeralCommand` types (terminal_data / tunnel_data / desktop input, which should complete in milliseconds) now get a **60s log-only** watchdog tier; everything else keeps the 2h default from #2395. - Still strictly log-only — the watchdog never fails, kills, or abandons a command. The warn line now also includes the tier (`warnAfter`) so 60s-tier and 2h-tier warnings are distinguishable in shipped logs. ## Tests - `agent/internal/heartbeat/heartbeat_inflight_stats_test.go`: injected-`now` overdue computation crossing each tier, live-pool gauge registration/deregistration around a blocked handler, tier selection (incl. override isolation between tiers), and log-only behavior of the short tier against the real `terminal_data` registry entry. - `runtime_stats_test.go`: JSON wire-shape contract extended with the two new keys. - `schemas.heartbeatTolerance.test.ts`: gauges parse, pre-#2400 agentRuntime still parses whole, bad gauge drops only itself. ## Verification - `cd agent && go test -race ./internal/heartbeat/... ./internal/collectors/... ./internal/workerpool/...` — all ok - `GOOS=windows/darwin/linux go build ./...` — all ok - `vitest run schemas.heartbeatTolerance.test.ts` (34 passed) + `heartbeat.test.ts` (78 passed); `tsc --noEmit` clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Todd Hebebrand <todd@lanternops.io> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ToddHebebrand
added a commit
that referenced
this pull request
Jul 13, 2026
… (#2409) ## Summary PR #2394 added the `capture_pprof` agent command (on-demand heap/goroutine profiles, base64 in the command result, 1 MiB raw cap per profile) but deliberately deferred AI exposure. This registers **`capture_agent_pprof`** as an approval-gated MCP/AI tool following the `set_agent_log_level` pattern — at **every** registration site in one PR, since partial registration across the gate's multiple maps is a known silent-drift trap. ### Registration sites (all mirrored from `set_agent_log_level`) | Site | Change | |---|---| | `services/aiToolsAgentLogs.ts` | Tool implementation — Tier 2, `deviceArgs: ['deviceId']`, org check + site-scope check, dispatches `capture_pprof` via `executeCommand` (30 s wait) | | `services/aiAgentSdkTools.ts` | `TOOL_TIERS: 2` + SDK `tool()` registration (`deviceId` uuid, `profile: heap\|goroutine\|all` optional) | | `services/aiGuardrails.ts` | `TOOL_PERMISSIONS` → `devices.execute`; `RATE_LIMITS` → 3 / 10 min; `buildApprovalDescription` case | | `services/aiToolSchemas.ts` | Zod input schema | | `services/aiAgentSystemPrompt.ts` | Logs tool list | | `web ai-risk/tierConfig.ts` (×3) | Tier-2 tool list, rate-limit table, permission map | Tier 2 means the chat path requires human approval (`aiAgentSdk` gates all tier ≥ 2 calls through the approval flow) and the MCP path requires write-capable tokens. The `aiToolsRegistryParity` test enforces schema + permission presence with **no** new legacy-gap entries. ### Artifact delivery — no megabytes of base64 in the transcript The agent returns profiles base64-encoded in the command-result stdout (up to ~2.7 MB for `all`). No existing AI tool convention fits (`take_screenshot` inlines base64 because the chat UI renders images; there is no file/artifact download convention in the AI tool layer), so per the issue the tool returns a **truncation-safe summary**: per-profile byte sizes, capture timestamp, and the runtime gauges snapshot (heap bytes, GC count, **goroutine count**), plus a `retrieval` pointer to `GET /devices/:id/commands/:commandId`. ### Supporting changes — making the artifact actually retrievable (incl. PR-review findings) The retrieval pointer would have been a lie without three fixes, two of which were caught by PR review: 1. **History redaction (planned):** `sanitizeCommandResultForHistory` unconditionally redacts stdout from command history — which also meant #2394's profiles were unretrievable by humans at all. Added a narrow allowlist (`RAW_STDOUT_COMMAND_TYPES = {'capture_pprof'}`) honored **only** by the single-command GET; list endpoints stay redacted so history pages never balloon. pprof profiles are allocation sites / goroutine stacks of our own agent binary — no tenant or user-generated content. Non-allowlisted types remain redacted even with the opt-in flag (pinned by tests in both directions, including a >4096-char payload proving the truncation/secret-pattern bypass). 2. **Ingest redaction (review finding, critical):** both result-ingest legs (agent WS + heartbeat REST) ran `redactSecretsFromOutput` on stdout before persisting. The case-insensitive `AKIA[0-9A-Z]{16}` pattern statistically fires inside megabytes of random base64 (~1/MB), silently corrupting the gzip-protobuf profiles while every layer reports success. Artifact-bearing stdout is now stored byte-for-byte on both legs (`isRawStdoutArtifactCommand`); stderr/error redaction is unchanged. Pinned by ingest tests on both legs using redaction-triggering substrings. 3. **Body limit (review finding, high):** the heartbeat/REST result route sat under the global 1 MB body limit, so a schema-valid multi-MB result (pprof `all` ≈ 2.8 MB; `commandResultSchema` allows 5 MB stdout) was 413-rejected on the WS-fallback leg and surfaced as a misleading generic timeout. Added a 12 MB carve-out for `/api/v1/agents/:id/commands/:commandId/result`. Also from review: a completed capture whose stdout carries no profile fields now returns an error instead of a success payload with empty `profiles`; parse-failure/command-failure paths include `commandId` (and log device/command context); `executeCommand` attaches `commandId` to its returned `CommandResult` on all post-insert paths; `capture_pprof` added to `AUDITED_COMMANDS` (behaviorally pinned). ## Testing - `pnpm exec vitest run` on the 10 affected suites (`aiToolsAgentLogs` + siteScope, `commandAudit`, `aiToolsRegistryParity`, `commandQueue`, `devices/commands`, `aiGuardrails`, `agents/commands`, `agentWs`, `bodyLimit`) — **256 passed**. - Adjacent registries: `aiAgentSdk.test.ts`, `clientAiTools.registry.test.ts`, `helperToolFilter.test.ts` — 88 passed. - `npx tsc --noEmit` clean in `apps/api` **and** `apps/web`. Closes #2401 Refs #2389, #2394. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Todd Hebebrand <todd@lanternops.io> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 13, 2026
ToddHebebrand
added a commit
that referenced
this pull request
Jul 13, 2026
…hrottle (#2422) (#2453) ## Summary Fixes both gaps in the `capture_pprof` diagnostic command (#2394/#2408 seam): 1. **Wedge gauges were always 0/0.** `handleCapturePprof` discarded its `*Heartbeat` receiver and embedded `collectors.CollectRuntimeStats()` directly, which never populates `commandsInFlight`/`commandsOverdue`. The snapshot now goes through `h.collectAgentRuntime(now)` — the wrapper that wires in `inFlightCommandStats` — so a capture taken while chasing an overdue-commands heartbeat trend reflects the real pool state. 2. **No throttle on GC-forcing captures.** Heap/all captures unconditionally ran `runtime.GC()`; as a server-queued command (10 concurrent / 100 queued), a burst could force back-to-back stop-the-world GCs. Added a 30s minimum interval between admitted captures using the same atomic CAS slot pattern as the heartbeat watchdog dump throttle (#2392). Throttled captures fail with an explicit `rate-limited` error. Payload validation runs before the throttle so malformed requests don't consume the slot. All changes are confined to `handlers_diag.go` / `handlers_diag_test.go` — `heartbeat.go` is untouched. ## Tests - New: `TestHandleCapturePprofIncludesWedgeGauges` (tracked overdue command shows up as 1/1 in the snapshot), `TestHandleCapturePprofThrottled` (second capture rejected, slot frees after interval), `TestHandleCapturePprofValidationDoesNotConsumeSlot`. - Existing diag tests updated to use a zero-value `*Heartbeat` + per-test throttle reset. - `cd agent && go test -race ./internal/heartbeat/...` — ok (41s); `go build ./...` clean; `go vet` clean. Closes #2422 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Todd Hebebrand <todd@lanternops.io> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The agent had zero runtime memory instrumentation — no
runtime.ReadMemStats, no pprof anywhere underagent/— so field memory issues like the 2.5 GB macOS agent in #2387 could only be diagnosed by inference from outside the process. This adds the two capabilities agreed on the issue.1. Always-on runtime gauges on the heartbeat
collectors.RuntimeStats(agent/internal/collectors/runtime_stats.go):HeapAlloc,HeapInuse,HeapReleased,Sys,NumGC+runtime.NumGoroutine(), collected on every heartbeat (ReadMemStatsis microseconds).agentRuntimefield onHeartbeatPayload; server-sideheartbeatSchemagains a matching tolerant optional object (.catch(undefined), uint64-magnitude-safe counters) so a bad value drops silently instead of 400-ing the heartbeat.device_metrics.custom_metricsjsonb column at the existing metrics insert site — no migration. Old agents that omit the field writecustom_metrics = NULL.The goroutine gauge alone would have made #2387's leading suspect (unbounded
go processCommand) visible from the dashboard.2. On-demand heap/goroutine profiles via privileged command (no listener)
capture_pprofcommand cloning theset_log_levelpattern: constant intools/types.go, dispatch entry inhandlers.go, handler in newhandlers_diag.go.runtime/pprofheap and/or goroutine profiles (profile:heap|goroutine|all, defaultall) in-process and returns them base64 in the command result — debug=0 gzip protobuf, directly consumable bygo tool pprof. Heap capture forces a GC first so the profile reflects live objects (same asnet/http/pprof?gc=1).CAPTURE_PPROFinCommandTypesso it flows through the existing signed/queued command infrastructure.Security (per the issue's note)
Not included (possible follow-up)
An MCP tool (
capture_agent_pprofalongsideset_agent_log_level) was considered but touches 7+ registration sites (SDK tool map, guardrails permission/rate maps, approval switch, tool schemas, web tierConfig ×3) — left out to keep this PR bounded.Testing
cd agent && go test -race ./internal/heartbeat/... ./internal/collectors/...— green (includes new tests: gauges non-zero + JSON wire-shape contract; pprof handler returns valid gzip profile bytes, heap/goroutine-only selection, unknown-profile rejection, size-cap failure;TestHandlerRegistryCompletenesscovers the new command).GOOS=windows|linux|darwin go build ./...— all green.pnpm exec vitest runonheartbeat.test.ts,schemas.heartbeatTolerance.test.ts,schemas.test.ts,commandQueue.test.ts— 153 passed (new: custom_metrics persistence + null for old agents; schema tolerance incl. >2^53 gauges, negative/missing-field drops).npx tsc --noEmit— clean.Closes #2389
🤖 Generated with Claude Code