Repository navigation
🐛 dashboard: derive sandboxed agent health from the last run, not the missing session - #14845
Conversation
…issing session Sandboxed agents (Podman or Job runtime) sit in the idle state between kicks and have no tmux session, so the SPA painted them red with "Stopped — see the agent's log" / "session down" and the token collector never saw their usage. Persist a SandboxRun record per kick (outcome, summary, usage) under <work_dir>/sandbox-runs/, restore it on restart, expose it as sandboxRun on /api/status with an ok/running/failed/overdue verdict derived from the run and the cadence, make the SPA use that instead of the session check, and ingest the run history in the token collector. Fixes #14844 Signed-off-by: Andrew Anderson <andy@clubanderson.com>
kubestellar-prow
Bot
added
the
dco-signoff: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Changelog: this PR changes code but carries no changelog entry If it is user-visible — a feature, a fix an operator would notice, a This is a reminder, not a gate; it never blocks a merge. |
kubestellar-prow
Bot
added
the
size/XXL
|
@clubanderson is auto-promoted for contributor-lane merging on this PR: the last 5 merged PRs by this author in this hive's repositories have no revert or sentinel-alert evidence. CI, reviewer verdict, diff fences, soak, holds and branch protection still apply. |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of
|
github-actions
Bot
added
the
conflicts-with-open-pr
hivecommons-hive
Bot
added
author/human
|
@clubanderson this PR is held at Failing checks:
Next: the hive does not push to your branch. Push a new head (or reply here); CI and the review swarm re-run automatically and the PR returns to the normal path. |
Signed-off-by: Andrew Anderson <andy@clubanderson.com>
![hivecommons-hive[bot]](https://avatars.githubusercontent.com/in/3568013?s=60&v=4){kind=small}
There was a problem hiding this comment.
Docs currency (read at head 6ff633a) — sandbox-isolation.md and the changelog fragment match the code (verdict table ↔ sandbox_health.go:91-149, blocker labels ↔ index.html agentSandboxDownLabel/blocker branch, session id ↔ sandbox_scanner.go sandboxSessionIDPrefix + agent + "-" + startMs, "scrubbed tail" ↔ sandbox_executor.go:169). Two operator docs that enumerate things this PR extends were not updated:
src/docs/token-tracking.md"Collection sources" (lines 7-17) lists every source the collector merges. This PR adds one —collector.goSetSandboxRunHistoryPath/scan()→ScanSandboxRuns, wired incmd/hive/main.go:3023— but no bullet. Same for the diagnostics list (line ~111):last_claude_scan_error, last_copilot_scan_error, last_bob_scan_erroris now missing the newDiagnostics.LastSandboxScanError(collector.go:192).
--- a/src/docs/token-tracking.md
+++ b/src/docs/token-tracking.md
@@ Collection sources @@
- Bob CLI chat recordings under `data.bob_sessions_dir` ...
+- Sandbox run history at `<work_dir>/sandbox-runs/runs.jsonl` (`HIVE_WORK_DIR`, default `/data/agents`), written by `crux/agent` after every Podman/Job kick (#14844). Each run that reported usage becomes one `sandbox` backend session (`sandbox-<agent>-<startMs>`); plain-text transcripts contribute no tokens. See [sandbox-isolation.md](sandbox-isolation.md#health-and-metrics-for-sandboxed-agents).
- In the optional Claude Managed Agents hosted path, ...
@@ Metering diagnostics @@
-- `last_claude_scan_error`, `last_copilot_scan_error`, `last_bob_scan_error` — a per-source scan failed.
+- `last_claude_scan_error`, `last_copilot_scan_error`, `last_bob_scan_error`, `last_sandbox_scan_error` — a per-source scan failed.src/docs/api-reference.md/api/statusrow (line 20) documents the per-agent extras (kicksUndeliverable…,agents[].run) but not the newagents[].sandboxRunadded inserver.go:832-836/status_builder.go:1123-1132. Suggest appending to that cell:; a sandboxed agent carriesagents[].sandboxRun—{health (ok|running|failed|overdue), runtime, lastRunAt, lastRunEndedAt, durationS, succeeded, timedOut, error, commitCount, branch, prURL, summary, tokens, model, dueAt}(#14844).
Advisory (not blocking): sandbox-isolation.md:129 still says the manager "returns it to idle or failed" and "Dashboard status uses the existing agent status structures"; the new section (line 177+) supersedes that — consider pointing it there.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
@clubanderson this PR is held at Failing checks:
Next: the hive does not push to your branch. Push a new head (or reply here); CI and the review swarm re-run automatically and the PR returns to the normal path. |
|
Hive filed review findings that do not block this PR as backlog items:
Cap: 10 backlog item(s) per PR per day. |
There was a problem hiding this comment.
Security perspective (read at head 857df8a; the dispatched SHA 6ff633a has been superseded) — no blocking findings.
Trust-boundary check: the transcript tail and error strings that now reach /api/status and disk are scrubbed at the executor boundary (src/crux/agent/sandbox_executor.go:169-170), re-redacted on the API payload (src/pkg/dashboard/sandbox_health.go:115,120), and HTML-escaped in the SPA (index.html:17899 title="${esc(sessionTip)}", :10904 escBlock(a.detailSummary)).
Low (advisory): the new activity-summary fallback bypasses the control-character sanitizer that the existing path applies — status_builder.go:1112/1114 wrap pane text in paneclass.SanitizePaneText(...), but :1129-1130 assigns sr.Summary directly, so ANSI/CSI sequences from the transcript tail reach the client raw (escaped, so no XSS; just unsanitized). Suggested:
if detailSummary == "" && sr.Summary != "" {
detailSummary = paneclass.SanitizePaneText(sr.Summary, maxDetailLines)
}
— hive: agent=adjudicator-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Correctness review (read at 6ff633a; go build of crux/pkg/dashboard/pkg/tokens/cmd/hive passes).
1. Operator pause/stop mid-kick is recorded as a failed run and renders red after resume — src/crux/agent/manager_sandbox.go:217
The comment says the record is taken "on every outcome but an operator cancel", but the guard only excludes the resume-after-cancel case. Pause (manager_pause.go:79-82) calls agent.cancel() and sets sandboxResumeAfterCancel = false; Stop (manager.go:1271) does the same. In both, exec.Run returns context.Canceled, so buildSandboxRun stores Succeeded:false, Error:"context canceled" and persists it. sandboxRunHealth checks !Succeeded before the paused/stopped checks (pkg/dashboard/sandbox_health.go:119-122), so once the agent is resumed (state → idle) the SPA shows a red dot, last run failed, and tooltip "The last sandboxed run failed: context canceled" until the next kick completes — and a Stopped agent reads as a failure instead of stopped by operator. The executor's timeout uses a child context (sandbox_executor.go:148), so the parent ctx.Err() cleanly distinguishes operator cancel from a real timeout/failure:
// The run record is what a sandboxed agent's health and activity derive
// from: there is no session to probe between kicks. Recorded on every
// outcome but an operator cancel (pause/stop/remove), and persisted so a
// pod roll does not turn yesterday's successful run into "never ran".
if err == nil || (ctx.Err() == nil && !agent.sandboxResumeAfterCancel) {
2. Operator-stopped sandboxed agent reads as healthy — src/pkg/dashboard/sandbox_health.go:115-125
After (1), a sandboxed agent stopped while running sits in StateStopped with its previous successful run, so health is ok; agentSessionDown (index.html ~17835) returns false and the card/nav label becomes idle/Job ok rather than stopped by operator, and the overdue timer will later flip it red for the wrong reason. Suggest a stopped verdict so the SPA keeps the existing label:
if proc.State == crux.StateRunning {
out.Health = SandboxHealthRunning
return out
}
+ if proc.State == crux.StateStopped {
+ out.Health = SandboxHealthStopped // new const "stopped"
+ return out
+ } function agentSessionDown(a) {
const sr = agentSandboxRun(a);
- if (sr) return sr.health === 'failed' || sr.health === 'overdue';
+ if (sr) return sr.health === 'failed' || sr.health === 'overdue' || sr.health === 'stopped';(and leave agentSandboxDownLabel returning '' for stopped so the existing operatorStopped branch supplies the label.)
Tests
sandbox_health_test.go / sandbox_run_test.go cover the pure helpers only; nothing exercises runSandboxKick recording. A manager-level test that pauses during a sandbox kick and asserts SandboxLastRun is unchanged would have caught (1).
Out of scope (advisory)
persistSandboxRundoes file I/O (two writes + rename + append) while holdingm.mu(manager_sandbox.go:224); other paths in this manager deliberately defer disk I/O past the unlock (manager_pause.go:88-90).runs.jsonlis append-only with no rotation andScanSandboxRunsre-reads it in full every collector scan (pkg/tokens/sandbox_scanner.go:49-80).
— hive: agent=adjudicator backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Style / maintainability review — read at head 857df8a (the PR moved past 6ff633a).
1. src/crux/agent/manager_sandbox.go:213-217 — the comment promises "recorded on every outcome but an operator cancel"; the guard only skips the resume-while-draining case. Manager.Stop (manager.go:1271-1272) cancels the kick context, and sandboxResumeAfterCancel is set only in manager_pause.go:178. So an operator stop reaches buildSandboxRun with err = context canceled, is persisted to <agent>.json, and sandboxRunHealth (pkg/dashboard/sandbox_health.go:126-129) checks !Succeeded before the paused/stopped guards — the card reads "Last run failed — see the agent's log · context canceled" until the next kick completes, which is exactly the false-red #14844 is fixing. Make the code match the comment:
if err == nil || res.TimedOut || ctx.Err() == nil {
(ctx is the per-kick runCtx; only an operator cancel sets ctx.Err(). If the executor's timeout is layered on that same context rather than a child, keep the res.TimedOut escape so timeouts still record as failed.)
2. manager_sandbox.go:220-223 — agent.KickOutcome is assigned directly, bypassing the settle path. Every other producer goes through settleKickOutcomeLocked (kick_outcome.go:177-211), which logs and calls m.notifyKickOutcome so the governor observer stops counting non-outcomes as completed kicks. Setting the field silently means sandbox kicks are the one source the observer never hears about. Minimal fix after the KickAt adjustment:
m.notifyKickOutcome(name, agent.KickOutcome)
3. src/pkg/tokens/sandbox_scanner.go:23-40 (advisory) — sandboxRunRecord hand-mirrors the crux.SandboxRun/SandboxRunHistoryEntry JSON tags; the comment calls the keys "the contract", but nothing pins it, so a tag rename in crux/process_types.go would silently zero every sandbox session. A _test.go in pkg/tokens can import crux/agent, json.Marshal a SandboxRunHistoryEntry and unmarshal into sandboxRunRecord to lock the shape.
— hive: agent=ci-maintainer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
merged by Hive trusted-author auto-merge: author @clubanderson holds role owner and GitHub permission admin on repo; CI green at 857df8a. |
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
hivecommons-hive
Bot
removed
role/trusted
Fixes #14844
Problem
sandbox.runtime: job(and Podman) agents are parked inStateIdlebetween kicks (crux/agent/manager.goStart,manager_sandbox.gorunSandboxKick) — they have no tmux session or in-pod process by design. The SPA equatesa.state !== 'running'with a dead session (index.htmlisStoppedin card/nav/detail/agentNowStateOf,agentBlockerLineHtml→ "session down"), so a healthy sandboxed agent renders red with "Stopped — see the agent's log". The Job's CLI home never reaches the hive pod andsandbox_executor.gopersisted no usage, so TOKENS/SESSIONS/cost were "—" and the activity summary vanished on pod roll.Fix
crux.SandboxRun(crux/process_types.go): outcome, timing, PR URL, summary (agent-report summary else transcript tail), usage. Built at the end of every sandboxed kick (crux/agent/sandbox_run.go,manager_sandbox.go), kept on the process assandboxLastRun, persisted to<work_dir>/sandbox-runs/<agent>.json+runs.jsonl, restored onStart().--output-format json/stream-jsonresultline and Copilot "Breakdown by AI model" footer. Plain-text runs report 0 (no estimates).pkg/dashboard/sandbox_health.go):sandboxRun.health=ok/running/failed/overdue(missed cadence occurrence older than grace = period/2 clamped 15m–2h; on-demand, paused,pause/continuousnever overdue). Exposed onFrontendAgent.SandboxRunin/api/status;detailSummaryfalls back to the run summary.agentSessionDown(a)replaces the foura.state !== 'running'sites; blocker line showsJob ok / Job running / last run failed / run overdue; "Now:" line shows last run outcome, duration, PR link, summary.pkg/tokens/sandbox_scanner.go,collector.go):runs.jsonlbecomes onesandboxsession per run with usage, merged into the aggregate; wired incmd/hive/main.go.docs/sandbox-isolation.md"Health and metrics for sandboxed agents"; changelog.d entry; api-reference citations refreshed.Tests
sandbox_health_test.go(verdict table, grace clamp, buildAgents wiring),crux/agent/sandbox_run_test.go(usage parsing, summary precedence/truncation, persist/load round-trip),pkg/tokens/sandbox_scanner_test.go(scan, missing file, collector merge). Notime.Sleep. Existing snippet tests updated for theagentSessionDownhelper.Not verified live on a1: the cluster API was unreachable from the dev host during this work.