Repository navigation
✨ feat(agent,dashboard): repeat-failure row note, check states, no-data safeguard and coverage table - #14623
Conversation
… safeguard and coverage table Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[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 |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of
|
|
hive/awaiting-ci: CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
…and keep the blockers fast path intact The crux boundary ratchet holds pkg/dashboard at 34 agent.<Symbol> references; the new RepeatFailureStatus types pushed it to 39. They are observation types, so they live in pkg/crux/types like the other row-status values, re-exported by pkg/crux and aliased by pkg/agent. The detail panel's poll fast path keeps its guarded statement and appends the repeat-failure line separately, and the settings parity allowlist drops repeat_failure.no_data_after_s now that the UI references it. Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
hive/awaiting-ci: pushed b567c27 — moved RepeatFailureStatus types to pkg/crux/types (crux boundary ratchet), kept the detail-panel blockers fast-path statement intact, dropped the now-referenced settings-parity allowlist entry. CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
…to the crux module v6 relocated pkg/agent and pkg/crux into the crux module (#14399), so the new repeat-failure files move to crux/agent and the status types to package crux. Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
hive/awaiting-ci: merged v6 (post-#14399 crux-module layout) into this branch at 5b7a0a5 — repeat-failure files relocated to crux/agent, status types into package crux. CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
The v6 merge moved crux.Process into crux/process.go and the field the agent snapshot and dashboard builder populate was lost in the resolution, so every Go package failed to typecheck. Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
hive/awaiting-ci: pushed 7f10303 restoring the 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
…Runtime and Host interfaces v6 moved cmd/hive onto crux.Runtime/crux.Host after this branch was written, so StartRepeatFailureMonitor and SetRepeatFailureConfigResolver were only on *agent.Manager and the build broke. Add them to the Background and Host families and to every implementer (api client, cruxtest recorder, sidecar host, serve loop list and HostSetters table). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
…for every background loop v6 added omnigent to config.CLIBackends, so the coverage table and its docs mirror need a row; the serve observer test counted four background loops but the repeat-failure monitor made it five. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
hive/awaiting-ci: pushed the omnigent coverage row and the background-loop count fix; CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
There was a problem hiding this comment.
Read the new monitor against the tree at bcad0c9, not just the diff. Looks correct to me.
What I verified:
- No double counting across ticks.
CollectRepeatFailuresbuilds a fresh collector each tick, butcollectFilerenames the record file to a.collectingsidecar anddrainCollectingremoves it (src/crux/agent/repeat_failure_record.go:398,:433), so each record reachesrecordRepeatFailureonce. - "Logged once" holds.
RepeatFailureDetector.Recordreturns a note only when the threshold is first crossed andnilon every later hit for the same command/result key (src/crux/agent/repeat_failure_detect.go:117-131), so theWarninrecordRepeatFailurefires once per raise. - Row note ordering.
Notessorts newest-first (repeat_failure_detect.go:148-153), matchingFormatRepeatFailureNote's "most recent plus K others". - Lock order.
refreshRepeatFailureStatustakesm.muthenmon.mu;ObserveRepeatFailureToolreleasesm.mubefore takingmon.mu;recordRepeatFailuretakes onlymon.mu. Consistent with the order documented onrepeatFailureMonitor; no inversion. - Off means off. The disabled branch of
CollectRepeatFailuresclears monitor state and everyAgentProcess.RepeatFailurewithout touching the record paths, andbuildAgentsWithHiddenemits all three JSON fields asomitempty, so the row renders nothing (repeatFailureLineHtmlreturns''on empty check and note). ObserveRepeatFailureToolhas no caller in this PR, as the body says;repeatFailureCheckStatetherefore returnson/not_coveredonly until #14559 lands a producer.
One note for the maintainer, not a defect: repeat_failure.no_data_after_s was dropped from settings_parity_allowlist.txt on the strength of a tooltip string in index.html naming the key; there is still no dashboard control for it. Fine if that is the intended bar for parity.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read the new monitor against the tree at merge-base e4aa972 (status/coverage files, Manager wiring, Process snapshot, status_builder.go, families.go, the settings-parity test). The code looks correct to me; two small documentation gaps below, neither blocking.
Verified
- Lock order holds everywhere:
repeatFailureRecordPathstakes onlym.mu.RLock,recordRepeatFailureonlymon.mu, andObserveRepeatFailureTool/refreshRepeatFailureStatustakem.mubeforemon.mu(src/crux/agent/repeat_failure_status.go:218-331). The collector callback runs with noManagerlock held. - Off-path clears state and touches no file (
repeat_failure_status.go:154-165);Process.RepeatFailureis copied insnapshot()and surfaced viastatus_builder.go:1259-1263withredactTokenson the note. ObserveRepeatFailureToolhas no production caller and every coverage row isCovered:false, sono_datais unreachable today and every row reads "not available for this backend". Both are stated in the PR body, so judged as declared scope, not a gap.
Docs-currency (low)
- The coverage file moved into the crux module in
5b7a0a59d, but three places still point at the old pathpkg/agent/repeat_failure_coverage.go:src/docs/backend-support-tiers.md:568, the failure message insrc/crux/agent/repeat_failure_coverage_test.go:113, and the PR body. Actual path:src/crux/agent/repeat_failure_coverage.go. src/docs/dashboard-sections.md:628and the tooltip atsrc/pkg/dashboard/static/index.html:17586explain "not available for this backend" as "Hive cannot record this backend's shell commands yet, so repeated failures are not noticed." Per the PR body's last paragraph, that is not what happens for the BASH_ENV backends (claude, codex, goose, pi, gemini, bob, opencode, kilo): records do arrive and aRepeating failure: …note renders on the same row (refreshRepeatFailureStatuscomputesNoteindependently ofCheck,repeat_failure_status.go:328-335). An operator will see a repeating-failure note directly above text saying repeated failures are not noticed. Suggest wording the not-covered text as "not verified for this backend (no committed real-run evidence); notes may still appear from the neutral route".
Suggestions only; I have not approved or requested changes.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
Closes #14558
Part of #14550 (tracker; #14559 backend-specific routes remain)
Targets v6.
What / why
This connects the repeat-failure pieces merged earlier to what operators see on the dashboard: the record collector (#14565), the normalizer (#14566), the detector (#14562) and the neutral BASH_ENV route (#14574). It is monitor-only: nothing here pauses, kicks or interrupts an agent.
AgentProcess.RepeatFailure(not a watchdog condition, notBusyCondition). It is refreshed at each 30 s collection byStartRepeatFailureMonitor, which starts with the other agent loops. Collected records are scrubbed and normalized, then passed to the detector. A raised note is logged once.<command>failed N times in M min (exit S)", or(failed)when there is no exit number. When several commands repeat, the row shows the most recent plus "and K others".on/ "repeat-failure check not available for this backend" / "repeat-failure check not receiving data". They appear on the agent card and in both detail-panel paths (repeatFailureLineHtml).ObserveRepeatFailureTooltakes shell / read / edit tool events for the current turn. A covered agent is "not receiving data" only when a shell call finished at leastno_data_after_sago (default 900), no record has arrived since, and no shell call is still running. File reads and edits alone don't count, and a command that is still running doesn't start the timer. This PR adds no caller: each covered backend's signal producer comes with [scanner] repeat-failure: backend-specific routes (copilot, omp, openhands, muse, agy, aider) and real-run evidence #14559.repeat_failure.enabledis false, each tick only clears the field and touches no file. The row shows no state and no note.pkg/agent/repeat_failure_coverage.gohas one row for everyconfig.CLIBackendsandconfig.InferenceBackendsentry. Tests fail if a backend is missing, a row lacks a reason or follow-up issue, a covered row lacks committed evidence (incl.--version) or a shell-activity signal, or the docs table differs from the rendered code table.backend-support-tiers.md(advisor-lane style); the note and the states indashboard-sections.md(Agents); a cross-reference inoperator-reference.md.api-reference.mdcitations were refreshed with--fix.Coverage decisions
No backend is marked covered. No committed real-run harness evidence exists in the tree yet, and C4 forbids claiming coverage without it.
As a result, with the check on today, every row shows "not available for this backend". Notes still appear when records from the neutral route arrive.
— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 requested_by=@Danathar