From 9fdf5a74873063176cfecb689708a6d24fe4eb75 Mon Sep 17 00:00:00 2001 From: Daedalus Date: Mon, 22 Jun 2026 14:55:37 +0200 Subject: [PATCH 1/2] feat(dashboard): typed stall pill + Send continue button (#4802 Cycle-1.5/1.7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Zod StallEventPayloadSchema (api/schemas.ts): mirrors server src/stall-events.ts bounded ErrorClass enum + 7 typed fields. Path 2 defensive: all fields .optional() with safe defaults. Added 'status.stall.typed' to SSEEventTypes enum for the new typed stall event. - StallBadge (components/session/StallBadge.tsx): renders pill from typed payload. Color: amber for transient_5xx, red for others. Sub-label: 'X/Y (auto-recovering...)' or 'X/Y — intervention required'. Kill-switch overlay icon when recoveryDisabled. - SendContinueButton (components/session/SendContinueButton.tsx): AC3b button gated on (1) typed payload present, (2) recovery exhausted (attempt >= max), (3) kill-switch NOT engaged. Calls POST /v1/sessions/:id/resume. - stallClassLabels utility: formatStallClassLabel, isRecoveryExhausted, isRecoveryDisabled, formatStallSubLabel, formatStallTooltip. - useStore: added stallMap + setStallMap + clearStallEntry. Path 2 default (empty map) until F-9 wires the typed payload to the SSE bus. - SessionHeader: added StallBadge next to SessionStateBadge. - SessionDetailPage: added SendContinueButton after PauseControlBar. Tests: 44 passing (16 stallClassLabels + 8 StallBadge + 20 schemas). Closes AC2 (visibility pill) and AC3b (Send continue button, gated on recoveryExhausted). AC3a (auto-recovery) is Hep's #4803. AC4 (telemetry) is Hep's #4803. F-9 follow-up wires the typed payload to emit sites; until then, the renderer falls back to generic 'Stalled' pill (Path 2 default). Close-format (per issuecomment-4767577518 PM canonical re-anchoring): Resolved by PR #4803 + PR # + PR Audit-trail anchors cited in PR body: 4767501614 (3-PR correction, my self-correction) 4767516114 (precision follow-up) 4767577518 (PM canonical re-anchoring, single source of truth) 4767621205 (PM matrix correction, Option B cell) --- dashboard/src/__tests__/StallBadge.test.tsx | 90 +++++++++++++ .../src/__tests__/stallClassLabels.test.ts | 127 ++++++++++++++++++ dashboard/src/api/schemas.ts | 61 ++++++++- .../components/session/SendContinueButton.tsx | 70 ++++++++++ .../src/components/session/SessionHeader.tsx | 6 + .../src/components/session/StallBadge.tsx | 108 +++++++++++++++ dashboard/src/pages/SessionDetailPage.tsx | 5 + dashboard/src/store/useStore.ts | 17 +++ dashboard/src/utils/stallClassLabels.ts | 115 ++++++++++++++++ 9 files changed, 597 insertions(+), 2 deletions(-) create mode 100644 dashboard/src/__tests__/StallBadge.test.tsx create mode 100644 dashboard/src/__tests__/stallClassLabels.test.ts create mode 100644 dashboard/src/components/session/SendContinueButton.tsx create mode 100644 dashboard/src/components/session/StallBadge.tsx create mode 100644 dashboard/src/utils/stallClassLabels.ts diff --git a/dashboard/src/__tests__/StallBadge.test.tsx b/dashboard/src/__tests__/StallBadge.test.tsx new file mode 100644 index 000000000..f4da09ec8 --- /dev/null +++ b/dashboard/src/__tests__/StallBadge.test.tsx @@ -0,0 +1,90 @@ +/** + * __tests__/StallBadge.test.tsx — Issue #4802: typed stall pill. + * + * Path 2 defensive: tests cover both typed payload (post-F-9) and missing + * fields (pre-F-9) scenarios. The badge must render gracefully when the + * typed payload isn't yet wired to the SSE bus. + */ + +import { describe, it, expect } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import { StallBadge } from '../components/session/StallBadge'; +import type { StallEventPayload } from '../api/schemas'; + +describe('Issue #4802: StallBadge', () => { + it('renders generic "Stalled" label when payload is empty (Path 2 default)', () => { + render(); + expect(screen.getByText('Stalled')).toBeDefined(); + }); + + it('renders typed errorClass label when present', () => { + const payload: Partial = { + errorClass: 'transient_5xx', + }; + render(); + expect(screen.getByText('Transient 5xx')).toBeDefined(); + }); + + it('renders JSONL Stall label for jsonl_stall class', () => { + const payload: Partial = { + errorClass: 'jsonl_stall', + }; + render(); + expect(screen.getByText('JSONL Stall')).toBeDefined(); + }); + + it('renders sub-label "X/Y (auto-recovering…)" when not exhausted', () => { + const payload: Partial = { + errorClass: 'transient_5xx', + recoveryAttemptCount: 3, + recoveryMaxAttempts: 5, + }; + render(); + expect(screen.getByText('Transient 5xx')).toBeDefined(); + expect(screen.getByText('3/5 (auto-recovering…)')).toBeDefined(); + }); + + it('renders sub-label "X/Y — intervention required" when exhausted', () => { + const payload: Partial = { + errorClass: 'transient_5xx', + recoveryAttemptCount: 5, + recoveryMaxAttempts: 5, + }; + render(); + expect(screen.getByText('5/5 — intervention required')).toBeDefined(); + }); + + it('hides sub-label when max is 0 (Path 2 default)', () => { + const payload: Partial = { + errorClass: 'transient_5xx', + recoveryAttemptCount: 0, + recoveryMaxAttempts: 0, + }; + const { container } = render(); + expect(screen.getByText('Transient 5xx')).toBeDefined(); + expect(container.textContent).not.toContain('0/0'); + }); + + it('renders kill-switch overlay when recoveryDisabled', () => { + const payload: Partial = { + errorClass: 'transient_5xx', + recoveryAttemptCount: 2, + recoveryMaxAttempts: 5, + recoveryDisabled: true, + }; + render(); + // Kill-switch icon has aria-label "Auto-recovery paused (operator kill-switch)" + expect(screen.getByLabelText('Auto-recovery paused (operator kill-switch)')).toBeDefined(); + }); + + it('marks data-stall-exhausted when cap is reached', () => { + const payload: Partial = { + errorClass: 'transient_5xx', + recoveryAttemptCount: 5, + recoveryMaxAttempts: 5, + }; + const { container } = render(); + const badge = container.querySelector('[data-stall-exhausted]'); + expect(badge).not.toBeNull(); + }); +}); diff --git a/dashboard/src/__tests__/stallClassLabels.test.ts b/dashboard/src/__tests__/stallClassLabels.test.ts new file mode 100644 index 000000000..316a8ade7 --- /dev/null +++ b/dashboard/src/__tests__/stallClassLabels.test.ts @@ -0,0 +1,127 @@ +/** + * __tests__/stallClassLabels.test.ts — Issue #4802: stall label + state helpers. + * + * Path 2 defensive: tests cover both typed payload (post-F-9) and missing + * fields (pre-F-9) scenarios. The renderer must degrade gracefully when the + * typed payload isn't yet wired to the SSE bus. + */ + +import { describe, it, expect } from 'vitest'; +import { + STALL_CLASS_LABELS, + STALL_GENERIC_LABEL, + formatStallClassLabel, + formatStallSubLabel, + isRecoveryExhausted, + isRecoveryDisabled, + formatStallTooltip, +} from '../utils/stallClassLabels'; + +describe('Issue #4802: stall label utilities', () => { + describe('formatStallClassLabel', () => { + it('maps each ErrorClass to a display label', () => { + for (const [key, expected] of Object.entries(STALL_CLASS_LABELS)) { + expect(formatStallClassLabel(key as never)).toBe(expected); + } + }); + + it('returns generic label for missing errorClass (Path 2 default)', () => { + expect(formatStallClassLabel(undefined)).toBe(STALL_GENERIC_LABEL); + expect(formatStallClassLabel(null)).toBe(STALL_GENERIC_LABEL); + }); + + it('returns generic label for unknown string', () => { + // Runtime guard at server side rejects these; renderer is defensive. + expect(formatStallClassLabel('5xx_529' as never)).toBe(STALL_GENERIC_LABEL); + }); + }); + + describe('isRecoveryExhausted', () => { + it('returns true when attempt >= max (both > 0)', () => { + expect(isRecoveryExhausted({ recoveryAttemptCount: 5, recoveryMaxAttempts: 5 })).toBe(true); + expect(isRecoveryExhausted({ recoveryAttemptCount: 6, recoveryMaxAttempts: 5 })).toBe(true); + }); + + it('returns false when attempt < max', () => { + expect(isRecoveryExhausted({ recoveryAttemptCount: 3, recoveryMaxAttempts: 5 })).toBe(false); + }); + + it('returns false when attempt < max', () => { + expect(isRecoveryExhausted({ recoveryAttemptCount: 1, recoveryMaxAttempts: 5 })).toBe(false); + }); + + it('returns false when max is 0 (Path 2 default — unknown)', () => { + expect(isRecoveryExhausted({ recoveryAttemptCount: 0, recoveryMaxAttempts: 0 })).toBe(false); + expect(isRecoveryExhausted({})).toBe(false); + }); + + it('returns false when only max is set, attempt is missing', () => { + expect(isRecoveryExhausted({ recoveryMaxAttempts: 5 })).toBe(false); + }); + }); + + describe('formatStallSubLabel', () => { + it('returns "X/Y (auto-recovering…)" when not exhausted', () => { + expect(formatStallSubLabel({ recoveryAttemptCount: 3, recoveryMaxAttempts: 5 })).toBe( + '3/5 (auto-recovering…)', + ); + }); + + it('returns "X/Y — intervention required" when exhausted', () => { + expect(formatStallSubLabel({ recoveryAttemptCount: 5, recoveryMaxAttempts: 5 })).toBe( + '5/5 — intervention required', + ); + }); + + it('returns null when max is 0 (Path 2 default — sub-label hidden)', () => { + expect(formatStallSubLabel({ recoveryAttemptCount: 0, recoveryMaxAttempts: 0 })).toBeNull(); + expect(formatStallSubLabel({})).toBeNull(); + }); + }); + + describe('isRecoveryDisabled', () => { + it('returns true when recoveryDisabled === true', () => { + expect(isRecoveryDisabled({ recoveryDisabled: true })).toBe(true); + }); + + it('returns false when recoveryDisabled is false or missing (Path 2 default)', () => { + expect(isRecoveryDisabled({ recoveryDisabled: false })).toBe(false); + expect(isRecoveryDisabled({})).toBe(false); + }); + }); + + describe('formatStallTooltip', () => { + it('composes metadata-only tooltip (no transcript text)', () => { + const tooltip = formatStallTooltip({ + errorClass: 'transient_5xx', + statusCode: 529, + lastErrorAt: '2026-06-22T12:00:00.000Z', + stallDurationMs: 600000, // 10 minutes + recoveryAttemptCount: 3, + recoveryMaxAttempts: 5, + }); + expect(tooltip).toContain('Transient 5xx'); + expect(tooltip).toContain('529'); + expect(tooltip).toContain('2026-06-22T12:00:00.000Z'); + expect(tooltip).toContain('10m'); + expect(tooltip).toContain('3/5'); + expect(tooltip).not.toContain('detail'); // F-6 redaction discipline + }); + + it('omits statusCode for non-transient_5xx classes', () => { + const tooltip = formatStallTooltip({ + errorClass: 'jsonl_stall', + statusCode: 529, // would be invalid in real payload, but defensive + recoveryAttemptCount: 1, + recoveryMaxAttempts: 5, + }); + expect(tooltip).toContain('JSONL Stall'); + expect(tooltip).not.toContain('529'); + }); + + it('handles missing fields gracefully (Path 2 default)', () => { + const tooltip = formatStallTooltip({}); + expect(tooltip).toBe('Stalled'); // just the generic label + }); + }); +}); diff --git a/dashboard/src/api/schemas.ts b/dashboard/src/api/schemas.ts index ded4c65cf..24dbf991e 100644 --- a/dashboard/src/api/schemas.ts +++ b/dashboard/src/api/schemas.ts @@ -368,9 +368,10 @@ const SSEEventTypes = z.enum([ 'subagent_stop', 'verification', 'permission_denied', + 'status.stall.typed', ]); -export const SessionSSEEventDataSchema: z.ZodType = z.object({ +export const SessionSSEEventDataSchema = z.object({ event: SSEEventTypes, sessionId: z.string(), timestamp: z.string(), @@ -380,7 +381,7 @@ export const SessionSSEEventDataSchema: z.ZodType = z.object({ }).transform((event) => ({ ...event, data: event.data ?? {}, -})); +})) as unknown as z.ZodType; // ── Global SSE Event (Issue #410) ────────────────────────────── @@ -440,3 +441,59 @@ export const WsInboundMessageSchema = z.discriminatedUnion('type', [ WsStreamMessageSchema, WsErrorMessageSchema, ]); + +// ── Stall Event Payload (Issue #4802) ──────────────────────────── + +/** + * Bounded ErrorClass enum mirroring server `src/stall-events.ts`. + * Renderer maps these to the dashboard pill label. Adding a new value is a + * schema PR that gets reviewed — schema drift cannot grow unchecked. + * + * Server-side isErrorClass() rejects unknown values, defending against + * prompt-injection inputs that try to inject new errorClass values. + */ +export const ErrorClassSchema = z.enum([ + 'transient_5xx', + 'permission_timeout', + 'jsonl_stall', + 'thinking_stall', + 'unknown_stall', + 'extended_working', +]); + +/** TypeScript type for the ErrorClass bounded enum (derived from ErrorClassSchema). */ +export type ErrorClass = z.infer; + +/** + * Zod schema for StallEventPayload, mirroring server `src/stall-events.ts`. + * + * Path 2 defensive defaults: all fields are `.optional()` with safe fallbacks. + * - Pre-F-9 (typed payload not yet wired to SSE bus): fields are missing, the + * renderer falls back to a generic "Stalled" pill with no sub-label or + * AC3b button. Safe default. + * - Post-F-9 (typed payload wired to SSE bus): fields populate, full pill + * (errorClass label + sub-label + AC3b button) renders. + * + * `recoveryExhausted` is NOT in the server's StallEventPayload yet — the + * renderer computes "exhausted" locally from + * `recoveryAttemptCount >= recoveryMaxAttempts` (when both > 0). + */ +export const StallEventPayloadSchema = z.object({ + errorClass: ErrorClassSchema.optional(), + statusCode: z.number().int().min(100).max(599).optional(), + lastErrorAt: z.string().optional(), + stallDurationMs: z.number().nonnegative().optional(), + recoveryAttemptCount: z.number().int().nonnegative().optional(), + recoveryMaxAttempts: z.number().int().nonnegative().optional(), + recoveryDisabled: z.boolean().optional(), +}).transform((p) => ({ + errorClass: p.errorClass, + statusCode: p.statusCode, + lastErrorAt: p.lastErrorAt, + stallDurationMs: p.stallDurationMs ?? 0, + recoveryAttemptCount: p.recoveryAttemptCount ?? 0, + recoveryMaxAttempts: p.recoveryMaxAttempts ?? 0, + recoveryDisabled: p.recoveryDisabled ?? false, +})); + +export type StallEventPayload = z.infer; diff --git a/dashboard/src/components/session/SendContinueButton.tsx b/dashboard/src/components/session/SendContinueButton.tsx new file mode 100644 index 000000000..347cb223d --- /dev/null +++ b/dashboard/src/components/session/SendContinueButton.tsx @@ -0,0 +1,70 @@ +/** + * components/session/SendContinueButton.tsx — Issue #4802 AC3b: manual recovery button. + * + * Renders a "Send continue" button that calls POST /v1/sessions/:id/resume + * (via useSessionIntervention's `resume` action) when auto-recovery has + * given up. + * + * Visibility (per close-path canonical issuecomment-4767577518): + * - Visible ONLY when stall payload is present AND `recoveryAttemptCount >= recoveryMaxAttempts` + * (i.e., auto-recovery has hit the cap and given up). + * - Hidden when stall payload is missing (Path 2 default — pre-F-9 typed payload not yet wired). + * - Hidden when `recoveryDisabled === true` (operator paused auto-recovery; the AC3b button + * is redundant in that case since auto-recovery is already paused). + * + * Renderer-only gating, no renderer-side computation that bypasses server config. + */ + +import { useStore } from '../../store/useStore'; +import { useSessionIntervention } from '../../hooks/useSessionIntervention'; +import { isRecoveryExhausted, isRecoveryDisabled } from '../../utils/stallClassLabels'; + +export interface SendContinueButtonProps { + sessionId: string; + className?: string; +} + +/** + * "Send continue" button. Renders null when AC3b gating conditions are not met. + */ +export function SendContinueButton({ sessionId, className }: SendContinueButtonProps) { + const stallPayload = useStore((s: { stallMap: Record }) => s.stallMap[sessionId]); + const { resume, isLoading, error, clearError } = useSessionIntervention(sessionId); + + // AC3b gating: only visible when (1) typed payload present, (2) recovery exhausted, + // (3) kill-switch NOT engaged. + if (!stallPayload) return null; + if (!isRecoveryExhausted(stallPayload)) return null; + if (isRecoveryDisabled(stallPayload)) return null; + + const handleClick = () => { + clearError(); + void resume(); + }; + + return ( +
+ + {error && ( + + {error} + + )} +
+ ); +} + +export default SendContinueButton; diff --git a/dashboard/src/components/session/SessionHeader.tsx b/dashboard/src/components/session/SessionHeader.tsx index aff6c2c9c..02c826492 100644 --- a/dashboard/src/components/session/SessionHeader.tsx +++ b/dashboard/src/components/session/SessionHeader.tsx @@ -3,6 +3,8 @@ import { useState, useRef, useEffect } from 'react'; import { GitFork, MoreHorizontal } from 'lucide-react'; import type { SessionHealth, SessionInfo } from '../../types'; import { SessionStateBadge, uiStateToSessionBadgeStatus } from './SessionStateBadge'; +import { StallBadge } from './StallBadge'; +import { useStore } from '../../store/useStore'; import { HoldButton } from '../shared/HoldButton'; import { CopyButton } from '../shared/CopyButton'; import { TimelineScrubber, type TimelineEvent } from './TimelineScrubber'; @@ -144,6 +146,7 @@ export function SessionHeader({ onFork, }: SessionHeaderProps) { const t = useT(); + const stallPayload = useStore((s: { stallMap: Record }) => s.stallMap[session.id]); const needsApproval = health.status === 'permission_prompt' || health.status === 'bash_approval'; const badgeStatus = uiStateToSessionBadgeStatus(health.status, health.alive); @@ -168,6 +171,9 @@ export function SessionHeader({ {formatSessionName(session.displayName)} + + {/* Issue #4802: typed stall pill — renders generic fallback when payload missing */} +
{truncateMiddle(session.workDir, 48)} diff --git a/dashboard/src/components/session/StallBadge.tsx b/dashboard/src/components/session/StallBadge.tsx new file mode 100644 index 000000000..c4daf8d6f --- /dev/null +++ b/dashboard/src/components/session/StallBadge.tsx @@ -0,0 +1,108 @@ +/** + * components/session/StallBadge.tsx — Issue #4802: typed stall pill. + * + * Renders a pill from a typed `StallEventPayload` (mirror of server + * `src/stall-events.ts`). Path 2 defensive: works on free-form emits + * (legacy `status.stall` event with `detail: string`) by falling back to + * a generic "Stalled" label. + * + * Pill text: ErrorClass label (e.g. "Transient 5xx", "Permission Timeout") + * Pill sub-label: "X/Y (auto-recovering…)" or "X/Y — intervention required" + * Kill-switch overlay icon: when `recoveryDisabled === true` + * Tooltip: composed metadata (errorClass + statusCode + timestamps + sub-label) + * + * Color: + * - `transient_5xx` → amber (retry-eligible, expected behavior) + * - others → red (more severe, requires attention) + */ + +import type { StallEventPayload } from '../../api/schemas'; +import { + formatStallClassLabel, + formatStallSubLabel, + isRecoveryExhausted, + isRecoveryDisabled, + formatStallTooltip, +} from '../../utils/stallClassLabels'; + +export interface StallBadgeProps { + payload: Partial; + className?: string; +} + +const STALL_COLOR_CLASSES: Record<'amber' | 'red', string> = { + amber: 'border-amber-500/40 bg-amber-500/10 text-amber-200', + red: 'border-red-500/40 bg-red-500/10 text-red-200', +}; + +/** + * Compact "kill-switch" indicator icon (operator paused auto-recovery for + * this session). Rendered as an inline overlay on the stall pill. + */ +function KillSwitchIcon({ className }: { className?: string }) { + return ( + + + + + ); +} + +/** + * Stall pill with errorClass label + sub-label + kill-switch overlay. + * + * Renders nothing useful (returns null) when no payload is provided — caller + * should guard with a presence check on the upstream event. + */ +export function StallBadge({ payload, className }: StallBadgeProps) { + const label = formatStallClassLabel(payload.errorClass); + const subLabel = formatStallSubLabel(payload); + const exhausted = isRecoveryExhausted(payload); + const disabled = isRecoveryDisabled(payload); + const tooltip = formatStallTooltip(payload); + + // Color: amber for transient_5xx (retry-eligible), red for others (more severe). + const colorKey: 'amber' | 'red' = + payload.errorClass === 'transient_5xx' ? 'amber' : 'red'; + const colorClasses = STALL_COLOR_CLASSES[colorKey]; + + // Subtle visual cue when cap is reached: slight ring outline. + const ringClass = exhausted ? 'ring-1 ring-amber-400/40' : ''; + + return ( + + {label} + {subLabel && ( + {subLabel} + )} + {disabled && ( + + )} + + ); +} + +export default StallBadge; diff --git a/dashboard/src/pages/SessionDetailPage.tsx b/dashboard/src/pages/SessionDetailPage.tsx index c8724f65f..b7f185af4 100644 --- a/dashboard/src/pages/SessionDetailPage.tsx +++ b/dashboard/src/pages/SessionDetailPage.tsx @@ -23,6 +23,7 @@ import { useSessionApproval } from '../hooks/useSessionApproval'; import { SessionHeader } from '../components/session/SessionHeader'; import { CliShortcutsPanel } from '../components/session/CliShortcutsPanel'; import { PauseControlBar } from '../components/session/PauseControlBar'; +import { SendContinueButton } from '../components/session/SendContinueButton'; import { DriverControlBar } from '../components/session/DriverControlBar'; import { useSessionParticipants } from '../hooks/useSessionParticipants'; import { useSessionTimeline } from '../hooks/useSessionTimeline'; @@ -322,6 +323,10 @@ export default function SessionDetailPage() { onResume={() => resume()} /> + {id && ( + + )} + ) => void; setHealth: (healthMap: Record) => void; + // Issue #4802: Per-session typed stall payloads (mirror of src/stall-events.ts). + // Keyed by session ID. The most recent typed stall event for a session. + stallMap: Record; + setStallMap: (stallMap: Record) => void; + clearStallEntry: (sessionId: string) => void; + // Global metrics metrics: GlobalMetrics | null; setMetrics: (metrics: GlobalMetrics) => void; @@ -143,6 +150,16 @@ export const useStore = create((set) => ({ areHealthMapsEqual(state.healthMap, healthMap) ? state : { healthMap } )), + // Issue #4802: typed stall map + stallMap: {}, + setStallMap: (stallMap: Record) => set({ stallMap }), + clearStallEntry: (sessionId) => set((state) => { + if (!(sessionId in state.stallMap)) return state; + const next = { ...state.stallMap }; + delete next[sessionId]; + return { stallMap: next }; + }), + // Metrics metrics: null, setMetrics: (metrics) => set((state) => (areMetricsEqual(state.metrics, metrics) ? state : { metrics })), diff --git a/dashboard/src/utils/stallClassLabels.ts b/dashboard/src/utils/stallClassLabels.ts new file mode 100644 index 000000000..154f42e65 --- /dev/null +++ b/dashboard/src/utils/stallClassLabels.ts @@ -0,0 +1,115 @@ +/** + * utils/stallClassLabels.ts — Issue #4802: stall pill label + state helpers. + * + * Server emits ErrorClass as a bounded enum (src/stall-events.ts). Renderer + * maps each value to a display label. The bounded enum is the single source + * of truth — no free-form strings, no concatenation, no prompt-injection + * surface for the pill. + * + * Path 2 defensive: if the typed payload is missing fields (pre-F-9), the + * renderer falls back to a generic "Stalled" label and hides the sub-label + * and AC3b button. Safe default. + */ + +import type { ErrorClass, StallEventPayload } from '../api/schemas'; + +/** + * Display labels for each ErrorClass. The bounded enum is enforced by + * `ErrorClassSchema` in api/schemas.ts — adding a new label requires adding + * a new ErrorClass value (schema PR that gets reviewed). + */ +export const STALL_CLASS_LABELS: Record = { + transient_5xx: 'Transient 5xx', + permission_timeout: 'Permission Timeout', + jsonl_stall: 'JSONL Stall', + thinking_stall: 'Thinking Stall', + unknown_stall: 'Unknown Stall', + extended_working: 'Extended Working', +}; + +/** Generic fallback when errorClass is not present in the wire payload (pre-F-9). */ +export const STALL_GENERIC_LABEL = 'Stalled'; + +/** Format ErrorClass to display label. Falls back to STALL_GENERIC_LABEL if missing. */ +export function formatStallClassLabel(errorClass: ErrorClass | undefined | null): string { + if (!errorClass) return STALL_GENERIC_LABEL; + return STALL_CLASS_LABELS[errorClass] ?? STALL_GENERIC_LABEL; +} + +/** + * Compute "exhausted" state — server doesn't yet emit recoveryExhausted, so + * the renderer derives it from the existing recoveryAttemptCount / + * recoveryMaxAttempts fields. When both are 0 (Path 2 default), exhaustion + * is unknown (not enough info to tell), so the AC3b button stays hidden. + * + * Post-F-9 (or if server adds recoveryExhausted), this can be replaced with + * `payload.recoveryExhausted === true`. + */ +export function isRecoveryExhausted(payload: Partial): boolean { + const attempt = payload.recoveryAttemptCount ?? 0; + const max = payload.recoveryMaxAttempts ?? 0; + if (max <= 0) return false; // unknown — keep button hidden + return attempt >= max; +} + +/** + * Compute sub-label for the stall pill. + * - When max > 0: "X/Y (auto-recovering…)" (State A) or "X/Y — intervention required" (State B) + * - When max === 0: return null (sub-label hidden, Path 2 default) + */ +export function formatStallSubLabel(payload: Partial): string | null { + const attempt = payload.recoveryAttemptCount ?? 0; + const max = payload.recoveryMaxAttempts ?? 0; + if (max <= 0) return null; // Path 2 default — no sub-label + const exhausted = isRecoveryExhausted(payload); + return exhausted + ? `${attempt}/${max} — intervention required` + : `${attempt}/${max} (auto-recovering…)`; +} + +/** + * Compute the kill-switch overlay state. When recoveryDisabled is true, + * the pill renders an overlay icon indicating the operator has paused + * auto-recovery for this session. + */ +export function isRecoveryDisabled(payload: Partial): boolean { + return payload.recoveryDisabled === true; +} + +/** + * Format a tooltip line for the stall pill. Composed of: + * - errorClass label + * - statusCode (if present and only for transient_5xx) + * - lastErrorAt (ISO timestamp) + * - stallDurationMs (formatted as "stalled Xm") + * - sub-label (X/Y recovery counter) + * + * Tooltip is metadata-only — never includes the raw `detail` field from + * the legacy free-form `status.stall` event, per F-6 redaction discipline. + */ +export function formatStallTooltip(payload: Partial): string { + const parts: string[] = []; + + const errorClassLabel = formatStallClassLabel(payload.errorClass); + parts.push(errorClassLabel); + + if (payload.statusCode !== undefined && payload.errorClass === 'transient_5xx') { + parts.push(`(${payload.statusCode})`); + } + + if (payload.lastErrorAt) { + parts.push(`since ${payload.lastErrorAt}`); + } + + if (payload.stallDurationMs !== undefined && payload.stallDurationMs > 0) { + const minutes = Math.round(payload.stallDurationMs / 60000); + parts.push(`stalled ${minutes}m`); + } + + const subLabel = formatStallSubLabel(payload); + if (subLabel) { + parts.push(subLabel); + } + + return parts.join(' — '); +} From d0295263ae6ebcfd932777f338263c69f7c16c32 Mon Sep 17 00:00:00 2001 From: Daedalus Date: Mon, 22 Jun 2026 16:53:59 +0200 Subject: [PATCH 2/2] fix(dashboard): StallBadge always-conditional guard (Argus review feedback) - StallBadge: return null when payload has no useful stall data (no errorClass AND no recoveryDisabled AND no recovery counter). Mirrors SendContinueButton L36 pattern. Prevents misleading 'Stalled' pill from rendering for healthy sessions on every page load. - SessionHeader: gate at callsite with {stallPayload && } in addition to the component guard (defense in depth). - schemas.ts: inline TODO marking removal point for the 'as unknown as z.ZodType' cast once F-9 lands and backend's SessionSSEEvent type includes 'status.stall.typed'. - Tests: added StallBadge.guard.test.tsx (6 new tests) for the always-conditional guard; updated obsolete test in StallBadge.test.tsx that expected the old 'render generic Stalled' behavior. Total: 50 tests passing (was 44). --- .../src/__tests__/StallBadge.guard.test.tsx | 56 +++++++++++++++++++ dashboard/src/__tests__/StallBadge.test.tsx | 9 ++- dashboard/src/api/schemas.ts | 8 +++ .../src/components/session/SessionHeader.tsx | 2 +- .../src/components/session/StallBadge.tsx | 19 ++++++- 5 files changed, 88 insertions(+), 6 deletions(-) create mode 100644 dashboard/src/__tests__/StallBadge.guard.test.tsx diff --git a/dashboard/src/__tests__/StallBadge.guard.test.tsx b/dashboard/src/__tests__/StallBadge.guard.test.tsx new file mode 100644 index 000000000..e1e577ccc --- /dev/null +++ b/dashboard/src/__tests__/StallBadge.guard.test.tsx @@ -0,0 +1,56 @@ +/** + * __tests__/StallBadge.guard.test.tsx — Issue #4802 (Argus review feedback): + * always-conditional component integration guard. + * + * StallBadge must return null when the payload has no useful stall data, + * to prevent a misleading "Stalled" pill from rendering for healthy sessions. + * Mirrors the SendContinueButton L36 pattern. + */ + +import { describe, it, expect } from 'vitest'; +import { render } from '@testing-library/react'; +import { StallBadge } from '../components/session/StallBadge'; + +describe('Issue #4802: StallBadge always-conditional guard (Argus review)', () => { + it('returns null when payload is empty (no stall data)', () => { + const { container } = render(); + expect(container.firstChild).toBeNull(); + }); + + it('returns null when payload has only undefined fields', () => { + const { container } = render( + , + ); + expect(container.firstChild).toBeNull(); + }); + + it('returns null when all counters are 0 (Path 2 default state)', () => { + const { container } = render( + , + ); + expect(container.firstChild).toBeNull(); + }); + + it('renders when errorClass is present (Path 2 with typed payload)', () => { + const { container } = render( + , + ); + expect(container.firstChild).not.toBeNull(); + }); + + it('renders when recoveryDisabled is true (kill-switch overlay)', () => { + const { container } = render( + , + ); + expect(container.firstChild).not.toBeNull(); + }); + + it('renders when recovery counter is non-zero', () => { + const { container } = render( + , + ); + expect(container.firstChild).not.toBeNull(); + }); +}); diff --git a/dashboard/src/__tests__/StallBadge.test.tsx b/dashboard/src/__tests__/StallBadge.test.tsx index f4da09ec8..cd0872399 100644 --- a/dashboard/src/__tests__/StallBadge.test.tsx +++ b/dashboard/src/__tests__/StallBadge.test.tsx @@ -12,9 +12,12 @@ import { StallBadge } from '../components/session/StallBadge'; import type { StallEventPayload } from '../api/schemas'; describe('Issue #4802: StallBadge', () => { - it('renders generic "Stalled" label when payload is empty (Path 2 default)', () => { - render(); - expect(screen.getByText('Stalled')).toBeDefined(); + it('returns null when payload is empty (always-conditional guard, Argus review feedback)', () => { + // Empty payload has no useful stall data: no errorClass, no kill-switch, + // no recovery counter. StallBadge must NOT render a misleading + // 'Stalled' pill for healthy sessions. Mirrors SendContinueButton L36. + const { container } = render(); + expect(container.firstChild).toBeNull(); }); it('renders typed errorClass label when present', () => { diff --git a/dashboard/src/api/schemas.ts b/dashboard/src/api/schemas.ts index 24dbf991e..d1da84bcd 100644 --- a/dashboard/src/api/schemas.ts +++ b/dashboard/src/api/schemas.ts @@ -382,6 +382,14 @@ export const SessionSSEEventDataSchema = z.object({ ...event, data: event.data ?? {}, })) as unknown as z.ZodType; +// TODO(#4802 F-9 follow-up): Remove the `as unknown as z.ZodType` +// cast once the backend's `SessionSSEEvent` type (src/api-contracts.ts) includes +// `'status.stall.typed'` in its `event` field union. At that point, F-9 has +// wired `buildStallEventPayload()` into the 12 emit sites, the typed stall +// payload flows in the wire, and the Zod enum in this file matches the +// backend's TypeScript union. The cast was needed because the new +// `'status.stall.typed'` event name was added to the local Zod enum +// ahead of the backend type update (forward-compatibility for the renderer). // ── Global SSE Event (Issue #410) ────────────────────────────── diff --git a/dashboard/src/components/session/SessionHeader.tsx b/dashboard/src/components/session/SessionHeader.tsx index 02c826492..3f204ef8a 100644 --- a/dashboard/src/components/session/SessionHeader.tsx +++ b/dashboard/src/components/session/SessionHeader.tsx @@ -173,7 +173,7 @@ export function SessionHeader({ {/* Issue #4802: typed stall pill — renders generic fallback when payload missing */} - + {stallPayload && }
{truncateMiddle(session.workDir, 48)} diff --git a/dashboard/src/components/session/StallBadge.tsx b/dashboard/src/components/session/StallBadge.tsx index c4daf8d6f..475b26402 100644 --- a/dashboard/src/components/session/StallBadge.tsx +++ b/dashboard/src/components/session/StallBadge.tsx @@ -61,10 +61,25 @@ function KillSwitchIcon({ className }: { className?: string }) { /** * Stall pill with errorClass label + sub-label + kill-switch overlay. * - * Renders nothing useful (returns null) when no payload is provided — caller - * should guard with a presence check on the upstream event. + * Returns null when the payload has no useful stall data to display + * (no errorClass AND no recoveryDisabled AND no recovery counter). This + * mirrors the SendContinueButton L36 pattern: always-conditional component + * integration — never render a "Stalled" pill for healthy sessions. + * + * Caller should still guard with a presence check on the upstream event + * (e.g. `{stallPayload && }` in SessionHeader), but the + * component itself is defensive against empty payloads. */ export function StallBadge({ payload, className }: StallBadgeProps) { + // No useful stall data: empty payload, no errorClass, no kill-switch, + // and no recovery counter → do not render a misleading "Stalled" pill. + const hasErrorClass = payload.errorClass !== undefined && payload.errorClass !== null; + const hasRecoveryCounter = + (payload.recoveryAttemptCount ?? 0) > 0 || (payload.recoveryMaxAttempts ?? 0) > 0; + const hasMeaningfulData = + hasErrorClass || payload.recoveryDisabled === true || hasRecoveryCounter; + if (!hasMeaningfulData) return null; + const label = formatStallClassLabel(payload.errorClass); const subLabel = formatStallSubLabel(payload); const exhausted = isRecoveryExhausted(payload);