fix: reduce false container-failed notifications, add error details to UI

Backend (health_monitor.py):
- Skip health checks for instances with no container_id
- Treat 'not_found' as error only when container was previously running
- Skip duplicate error notifications when already in error state
- Skip 'not_found' notifications for containers that never ran

Frontend (notification-item.tsx):
- Display notification.message (detailed error text)
- Add expandable Details section showing metadata (exit_code, previous_status, etc.)
- New CSS styles for message and metadata display

Quality gates: py_compile, tsc --noEmit, 80/80 tests pass
This commit is contained in:
Developer
2026-06-06 09:01:01 +00:00
parent 2169b24875
commit 1ef9d66eed
6 changed files with 697 additions and 475 deletions
@@ -90,8 +90,16 @@ class HealthMonitor:
instance: ToolInstance, instance: ToolInstance,
) -> None: ) -> None:
"""Check a single instance and handle state transitions.""" """Check a single instance and handle state transitions."""
# Skip instances that have never been assigned a container.
if not instance.container_id:
logger.debug(
"Skipping health check for instance %s: no container_id",
instance.id,
)
return
try: try:
container_info = get_container_status(instance.container_id or "") container_info = get_container_status(instance.container_id)
except Exception: except Exception:
logger.exception( logger.exception(
"Health check failed for instance %s", "Health check failed for instance %s",
@@ -135,7 +143,11 @@ class HealthMonitor:
previous = self._last_known_state.get(instance.id) previous = self._last_known_state.get(instance.id)
# Determine new status # Determine new status
new_status = self._derive_status(snapshot) new_status = self._derive_status(
snapshot,
previous,
instance.status,
)
# If first check or state changed # If first check or state changed
if previous is None or not self._snapshots_equal(previous, snapshot): if previous is None or not self._snapshots_equal(previous, snapshot):
@@ -144,10 +156,34 @@ class HealthMonitor:
) )
self._last_known_state[instance.id] = snapshot self._last_known_state[instance.id] = snapshot
def _derive_status(self, snapshot: HealthSnapshot) -> str: def _derive_status(
"""Derive instance status from health snapshot.""" self,
snapshot: HealthSnapshot,
previous: HealthSnapshot | None,
current_status: str | None,
) -> str:
"""Derive instance status from health snapshot.
Treats missing containers as an error only when the container was
previously known to be running. This avoids false "container failed"
alerts for instances that are still starting or have no container yet.
"""
if snapshot.container_status == "not_found":
# If the container was never seen running, assume it's still
# starting or was deleted intentionally; don't flag as error.
if previous is None and current_status == "starting":
return "starting"
if previous is not None and previous.container_status == "running":
return "error"
# Fall back to current status to avoid spurious errors.
return current_status or "error"
if snapshot.container_status == "exited":
return "error"
if snapshot.container_status != "running": if snapshot.container_status != "running":
return "error" return "error"
if snapshot.tunnel_healthy is False: if snapshot.tunnel_healthy is False:
return "unhealthy" return "unhealthy"
return "running" return "running"
@@ -223,6 +259,16 @@ class HealthMonitor:
# Create notification for instance owner (fire-and-forget) # Create notification for instance owner (fire-and-forget)
# Only send warnings and errors; skip "recovered" info notifications. # Only send warnings and errors; skip "recovered" info notifications.
if new_status == "error": if new_status == "error":
# Skip duplicate error notifications if already in error state.
if previous_status == "error":
return
# Skip "not_found" errors for containers that were never running
# (e.g. still starting, or intentionally stopped/deleted).
if (
snapshot.container_status == "not_found"
and (previous is None or previous.container_status != "running")
):
return
category = "instance" category = "instance"
severity = "error" severity = "error"
title = "Container failed" title = "Container failed"
@@ -1,3 +1,4 @@
import { useState } from "react";
import { Icon } from "../../icon"; import { Icon } from "../../icon";
import { formatRelativeTime } from "../../../utils/time"; import { formatRelativeTime } from "../../../utils/time";
import type { NotificationItem as NotificationItemType } from "../../../api/notifications"; import type { NotificationItem as NotificationItemType } from "../../../api/notifications";
@@ -17,6 +18,18 @@ const severityIconMap: Record<string, IconName> = {
success: "success", success: "success",
}; };
function formatMetadataValue(value: unknown): string {
if (value === null || value === undefined) return "—";
if (typeof value === "string") return value;
if (typeof value === "number") return String(value);
if (typeof value === "boolean") return value ? "Yes" : "No";
try {
return JSON.stringify(value);
} catch {
return String(value);
}
}
export function NotificationItem({ export function NotificationItem({
notification, notification,
onMarkRead, onMarkRead,
@@ -24,6 +37,11 @@ export function NotificationItem({
}: NotificationItemProps) { }: NotificationItemProps) {
const isUnread = notification.read_at === null; const isUnread = notification.read_at === null;
const iconName = severityIconMap[notification.severity] ?? "info"; const iconName = severityIconMap[notification.severity] ?? "info";
const [expanded, setExpanded] = useState(false);
const hasDetails =
!!notification.message ||
Object.keys(notification.metadata || {}).length > 0;
return ( return (
<li <li
@@ -35,11 +53,36 @@ export function NotificationItem({
</div> </div>
<div className="notification-item-content"> <div className="notification-item-content">
<div className="notification-item-title">{notification.title}</div> <div className="notification-item-title">{notification.title}</div>
{notification.message && (
<div className="notification-item-message">
{notification.message}
</div>
)}
<div className="notification-item-time"> <div className="notification-item-time">
{formatRelativeTime(notification.created_at)} {formatRelativeTime(notification.created_at)}
</div> </div>
{expanded && notification.metadata && (
<dl className="notification-item-metadata">
{Object.entries(notification.metadata).map(([key, value]) => (
<div key={key} className="notification-metadata-row">
<dt>{key.replace(/_/g, " ")}</dt>
<dd>{formatMetadataValue(value)}</dd>
</div>
))}
</dl>
)}
</div> </div>
<div className="notification-item-actions"> <div className="notification-item-actions">
{hasDetails && (
<button
type="button"
className="notification-item-action"
onClick={() => setExpanded(!expanded)}
aria-label={expanded ? "Hide details" : "Show details"}
>
{expanded ? "Less" : "Details"}
</button>
)}
{isUnread && ( {isUnread && (
<button <button
type="button" type="button"
@@ -14,7 +14,11 @@ interface Props {
onConfirmDelete: () => void; onConfirmDelete: () => void;
onCancelDelete: () => void; onCancelDelete: () => void;
onCreateWorkspace: (repoId: string) => void; onCreateWorkspace: (repoId: string) => void;
onWorkspaceAction: (repoId: string, workspace: WorkspaceSummary, action: "sync" | "delete") => void; onWorkspaceAction: (
repoId: string,
workspace: WorkspaceSummary,
action: "sync" | "delete",
) => void;
onCancelCreate: () => void; onCancelCreate: () => void;
onCreated: () => void; onCreated: () => void;
} }
@@ -38,15 +42,23 @@ export const ProjectCard = ({
return ( return (
<article className="card project-card"> <article className="card project-card">
<div className="project-info-row"> <div className="project-info-row">
<button className="project-toggle" onClick={onToggle} type="button" aria-expanded={expanded}> <button
className="project-toggle"
onClick={onToggle}
type="button"
aria-expanded={expanded}
>
<Icon name={expanded ? "chevron-down" : "chevron-right"} size="sm" /> <Icon name={expanded ? "chevron-down" : "chevron-right"} size="sm" />
<div> <div>
<h3>{project.name}</h3> <h3>{project.name}</h3>
{project.description && <p className="muted project-description">{project.description}</p>} {project.description && (
<p className="muted project-description">{project.description}</p>
)}
</div> </div>
{project.repositories?.length > 0 && ( {project.repositories?.length > 0 && (
<span className="repo-count"> <span className="repo-count">
{project.repositories.length} repo{project.repositories.length > 1 ? "s" : ""} {project.repositories.length} repo
{project.repositories.length > 1 ? "s" : ""}
</span> </span>
)} )}
</button> </button>
@@ -57,15 +69,27 @@ export const ProjectCard = ({
{deleteConfirm ? ( {deleteConfirm ? (
<div className="delete-confirm"> <div className="delete-confirm">
<span>Are you sure?</span> <span>Are you sure?</span>
<button className="danger-button" onClick={onConfirmDelete} type="button"> <button
className="danger-button"
onClick={onConfirmDelete}
type="button"
>
<Icon name="delete" size="sm" /> Delete <Icon name="delete" size="sm" /> Delete
</button> </button>
<button className="ghost-button" onClick={onCancelDelete} type="button"> <button
className="ghost-button"
onClick={onCancelDelete}
type="button"
>
<Icon name="cancel" size="sm" /> Cancel <Icon name="cancel" size="sm" /> Cancel
</button> </button>
</div> </div>
) : ( ) : (
<button className="ghost-button danger-text" onClick={onDelete} type="button"> <button
className="ghost-button danger-text"
onClick={onDelete}
type="button"
>
<Icon name="delete" size="sm" /> Delete <Icon name="delete" size="sm" /> Delete
</button> </button>
)} )}
@@ -82,29 +106,59 @@ export const ProjectCard = ({
<div key={repo.id} className="repo-block"> <div key={repo.id} className="repo-block">
<div className="repo-header"> <div className="repo-header">
<h4>{repo.name}</h4> <h4>{repo.name}</h4>
<button className="btn btn-sm btn-primary" onClick={() => onCreateWorkspace(repo.id)} type="button"> <button
className="btn btn-sm btn-primary"
onClick={() => onCreateWorkspace(repo.id)}
type="button"
>
<Icon name="add" size="sm" /> New Workspace <Icon name="add" size="sm" /> New Workspace
</button> </button>
</div> </div>
{showCreateForm === repo.id && ( {showCreateForm === repo.id && (
<WorkspaceCreateForm defaultProjectId={project.id} defaultRepoId={repo.id} onSubmit={onCreated} onCancel={onCancelCreate} /> <WorkspaceCreateForm
defaultProjectId={project.id}
defaultRepoId={repo.id}
onSubmit={onCreated}
onCancel={onCancelCreate}
/>
)} )}
{repo.workspaces.length === 0 ? ( {repo.workspaces.length === 0 ? (
<p className="muted">No workspaces.</p> <p className="muted">No workspaces.</p>
) : ( ) : (
<div className="workspace-grid"> <div className="workspace-grid">
{repo.workspaces.map((ws) => ( {repo.workspaces.map((ws) => (
<div key={ws.id} className={`workspace-chip ${ws.status}`}> <div
key={ws.id}
className={`workspace-chip ${ws.status}`}
>
<a href={`/workspaces/${ws.id}`}>{ws.name}</a> <a href={`/workspaces/${ws.id}`}>{ws.name}</a>
<span className="ws-branch"><Icon name="branch" size="sm" /> {ws.branch}</span> <span className="ws-branch">
<Icon name="branch" size="sm" /> {ws.branch}
</span>
{ws.instance_count > 0 && ( {ws.instance_count > 0 && (
<span className="ws-instances">{ws.instance_count} tool{ws.instance_count > 1 ? "s" : ""}</span> <span className="ws-instances">
{ws.instance_count} tool
{ws.instance_count > 1 ? "s" : ""}
</span>
)} )}
<div className="ws-actions"> <div className="ws-actions">
<button type="button" disabled={workspaceLoading === ws.id} onClick={() => onWorkspaceAction(repo.id, ws, "sync")}> <button
type="button"
disabled={workspaceLoading === ws.id}
onClick={() =>
onWorkspaceAction(repo.id, ws, "sync")
}
>
<Icon name="refresh" size="sm" /> <Icon name="refresh" size="sm" />
</button> </button>
<button type="button" className="danger-text" disabled={workspaceLoading === ws.id} onClick={() => onWorkspaceAction(repo.id, ws, "delete")}> <button
type="button"
className="danger-text"
disabled={workspaceLoading === ws.id}
onClick={() =>
onWorkspaceAction(repo.id, ws, "delete")
}
>
<Icon name="delete" size="sm" /> <Icon name="delete" size="sm" />
</button> </button>
</div> </div>
@@ -1,11 +1,21 @@
import { useEffect, useRef, useState } from "react"; import { useEffect, useRef, useState } from "react";
import { createRepository, parseGitUrl, type GitRepositoryCreate, type URLParseResult } from "../../../api/git-repositories"; import {
createRepository,
parseGitUrl,
type GitRepositoryCreate,
type URLParseResult,
} from "../../../api/git-repositories";
import { listSSHKeys, type SSHKey } from "../../../api/ssh-keys"; import { listSSHKeys, type SSHKey } from "../../../api/ssh-keys";
import { Icon } from "../../icon"; import { Icon } from "../../icon";
type CreateMode = "clone" | "blank"; type CreateMode = "clone" | "blank";
type UrlValidationStatus = "idle" | "validating" | "valid" | "needs-parsing" | "invalid"; type UrlValidationStatus =
| "idle"
| "validating"
| "valid"
| "needs-parsing"
| "invalid";
interface RepositoryCreateDialogProps { interface RepositoryCreateDialogProps {
projectId: string; projectId: string;
@@ -15,7 +25,13 @@ interface RepositoryCreateDialogProps {
onCreated: () => Promise<void> | void; onCreated: () => Promise<void> | void;
} }
export const RepositoryCreateDialog = ({ projectId, open, title, onClose, onCreated }: RepositoryCreateDialogProps) => { export const RepositoryCreateDialog = ({
projectId,
open,
title,
onClose,
onCreated,
}: RepositoryCreateDialogProps) => {
const [createMode, setCreateMode] = useState<CreateMode>("clone"); const [createMode, setCreateMode] = useState<CreateMode>("clone");
const [formName, setFormName] = useState(""); const [formName, setFormName] = useState("");
const [owner, setOwner] = useState(""); const [owner, setOwner] = useState("");
@@ -148,7 +164,9 @@ export const RepositoryCreateDialog = ({ projectId, open, title, onClose, onCrea
} catch (error: unknown) { } catch (error: unknown) {
const response = error as { response?: { data?: { detail?: string } } }; const response = error as { response?: { data?: { detail?: string } } };
const detail = response.response?.data?.detail; const detail = response.response?.data?.detail;
setFormError(typeof detail === "string" ? detail : "Failed to create repository"); setFormError(
typeof detail === "string" ? detail : "Failed to create repository",
);
} }
}; };
@@ -180,7 +198,8 @@ export const RepositoryCreateDialog = ({ projectId, open, title, onClose, onCrea
<div className="dialog"> <div className="dialog">
<h3>{title}</h3> <h3>{title}</h3>
<p className="muted"> <p className="muted">
Clone an existing repository from git.commumedia.org, or create a blank bare repo here. Clone an existing repository from git.commumedia.org, or create a
blank bare repo here.
</p> </p>
<form onSubmit={handleSubmit} className="stack"> <form onSubmit={handleSubmit} className="stack">
<div className="form-field"> <div className="form-field">
@@ -246,7 +265,10 @@ export const RepositoryCreateDialog = ({ projectId, open, title, onClose, onCrea
))} ))}
</select> </select>
</label> </label>
<p className="muted">SSH target: git@git.commumedia.org:{owner || "owner"}/{repoName || "repo"}.git</p> <p className="muted">
SSH target: git@git.commumedia.org:{owner || "owner"}/
{repoName || "repo"}.git
</p>
<button <button
type="button" type="button"
className="secondary-button small" className="secondary-button small"
@@ -268,20 +290,26 @@ export const RepositoryCreateDialog = ({ projectId, open, title, onClose, onCrea
className={getUrlInputClass()} className={getUrlInputClass()}
/> />
{urlValidation.status === "validating" && ( {urlValidation.status === "validating" && (
<span className="validation-status validating">Validating...</span> <span className="validation-status validating">
Validating...
</span>
)} )}
{urlValidation.status === "valid" && ( {urlValidation.status === "valid" && (
<span className="validation-status valid"> <span className="validation-status valid">
<Icon name="success" size="sm" /> Valid git URL <Icon name="success" size="sm" /> Valid git URL
</span> </span>
)} )}
{urlValidation.status === "needs-parsing" && urlValidation.result && ( {urlValidation.status === "needs-parsing" &&
urlValidation.result && (
<div className="url-suggestion"> <div className="url-suggestion">
<span className="validation-status warning"> <span className="validation-status warning">
<Icon name="warning" size="sm" /> This looks like a browser URL <Icon name="warning" size="sm" /> This looks like a
browser URL
</span> </span>
<div className="suggestion-actions"> <div className="suggestion-actions">
<span className="suggested-url">Suggested: {urlValidation.result.base_url}</span> <span className="suggested-url">
Suggested: {urlValidation.result.base_url}
</span>
<button <button
type="button" type="button"
className="secondary-button small" className="secondary-button small"
@@ -327,13 +355,19 @@ export const RepositoryCreateDialog = ({ projectId, open, title, onClose, onCrea
</div> </div>
)} )}
<div className="dialog-actions"> <div className="dialog-actions">
<button className="secondary-button" onClick={handleClose} type="button"> <button
className="secondary-button"
onClick={handleClose}
type="button"
>
<Icon name="cancel" size="sm" /> <Icon name="cancel" size="sm" />
Cancel Cancel
</button> </button>
<button className="primary-button" type="submit"> <button className="primary-button" type="submit">
<Icon name="add" size="sm" /> <Icon name="add" size="sm" />
{createMode === "clone" ? "Clone Repository" : "Create Blank Repository"} {createMode === "clone"
? "Clone Repository"
: "Create Blank Repository"}
</button> </button>
</div> </div>
</form> </form>
+16 -9
View File
@@ -10,7 +10,7 @@ const mockProjects = vi.fn();
const mockRepos = vi.fn(); const mockRepos = vi.fn();
vi.mock("../api/dashboard", () => ({ vi.mock("../api/dashboard", () => ({
getDashboardSummary: (...args: unknown[]) => mockDashboard(...args) getDashboardSummary: (...args: unknown[]) => mockDashboard(...args),
})); }));
vi.mock("../api/sessions", () => ({ vi.mock("../api/sessions", () => ({
@@ -19,19 +19,19 @@ vi.mock("../api/sessions", () => ({
startInstance: vi.fn(), startInstance: vi.fn(),
stopInstance: vi.fn(), stopInstance: vi.fn(),
deleteInstance: vi.fn(), deleteInstance: vi.fn(),
recreateInstanceTunnel: vi.fn() recreateInstanceTunnel: vi.fn(),
})); }));
vi.mock("../api/projects", () => ({ vi.mock("../api/projects", () => ({
listProjects: (...args: unknown[]) => mockProjects(...args) listProjects: (...args: unknown[]) => mockProjects(...args),
})); }));
vi.mock("../api/git-repositories", () => ({ vi.mock("../api/git-repositories", () => ({
listRepositories: (...args: unknown[]) => mockRepos(...args) listRepositories: (...args: unknown[]) => mockRepos(...args),
})); }));
vi.mock("../api/tool-types", () => ({ vi.mock("../api/tool-types", () => ({
listToolTypes: vi.fn().mockResolvedValue([]) listToolTypes: vi.fn().mockResolvedValue([]),
})); }));
describe("HomePage", () => { describe("HomePage", () => {
@@ -43,7 +43,12 @@ describe("HomePage", () => {
}); });
it("shows overview sections", async () => { it("shows overview sections", async () => {
mockDashboard.mockResolvedValue({ projects: 1, repositories: 2, sshKeys: 3, recentActivity: [] }); mockDashboard.mockResolvedValue({
projects: 1,
repositories: 2,
sshKeys: 3,
recentActivity: [],
});
mockSessions.mockResolvedValue([]); mockSessions.mockResolvedValue([]);
mockProjects.mockResolvedValue([]); mockProjects.mockResolvedValue([]);
mockRepos.mockResolvedValue([]); mockRepos.mockResolvedValue([]);
@@ -51,7 +56,7 @@ describe("HomePage", () => {
render( render(
<MemoryRouter> <MemoryRouter>
<HomePage /> <HomePage />
</MemoryRouter> </MemoryRouter>,
); );
expect(screen.getByText("Loading overview...")).toBeInTheDocument(); expect(screen.getByText("Loading overview...")).toBeInTheDocument();
@@ -69,11 +74,13 @@ describe("HomePage", () => {
render( render(
<MemoryRouter> <MemoryRouter>
<HomePage /> <HomePage />
</MemoryRouter> </MemoryRouter>,
); );
await waitFor(() => { await waitFor(() => {
expect(screen.getByText("Unable to load your workspace overview.")).toBeInTheDocument(); expect(
screen.getByText("Unable to load your workspace overview."),
).toBeInTheDocument();
}); });
fireEvent.click(screen.getAllByRole("button", { name: "Retry" })[0]); fireEvent.click(screen.getAllByRole("button", { name: "Retry" })[0]);
+38
View File
@@ -2474,6 +2474,44 @@ a:active,
color: var(--muted); color: var(--muted);
} }
.notification-item-message {
font-size: 0.85rem;
color: var(--muted);
line-height: 1.35;
margin-top: 0.15rem;
word-break: break-word;
}
.notification-item-metadata {
margin-top: 0.5rem;
padding: 0.5rem 0.75rem;
background: var(--bg);
border-radius: 6px;
border: 1px solid var(--border);
font-size: 0.8rem;
}
.notification-metadata-row {
display: flex;
gap: 0.5rem;
padding: 0.15rem 0;
}
.notification-metadata-row dt {
font-weight: 500;
color: var(--ink);
min-width: 6rem;
text-transform: capitalize;
flex-shrink: 0;
}
.notification-metadata-row dd {
margin: 0;
color: var(--muted);
font-family: monospace;
word-break: break-word;
}
.notification-item-actions { .notification-item-actions {
display: flex; display: flex;
gap: 0.35rem; gap: 0.35rem;