diff --git a/plans/2026-07-11-task-completion-safety-net.md b/plans/done/2026-07-11-task-completion-safety-net.md similarity index 72% rename from plans/2026-07-11-task-completion-safety-net.md rename to plans/done/2026-07-11-task-completion-safety-net.md index b53fc91..e491579 100644 --- a/plans/2026-07-11-task-completion-safety-net.md +++ b/plans/done/2026-07-11-task-completion-safety-net.md @@ -1,22 +1,25 @@ # Task completion safety net: every live task is stuck "Running" -Status: In Progress — 2026-07-11. Fixes 1-3 implemented, built, tested -(`go build ./...`, `go test ./cmd/nomos/...`), and committed -(`3b9c75f`). Not yet deployed or verified live. Fix 4 (backfill of the 50 -already-stuck live sessions) intentionally not started — per the -implementation order below, it needs 1-3 deployed and verified first. +Status: Done — 2026-07-12. Fixes 1-3 implemented, built, tested +(`go build ./...`, `go test ./cmd/nomos/...`), committed (`3b9c75f`), +deployed, and verified live (see Verification below — fresh trivial Q&A +sessions now reach `done` immediately; a goal-bearing session that went +idle was correctly nudged and auto-resolved by the existing resume-failure +path). Fix 4 (backfill) was replaced with deletion — see "Fix 4, revised" +below; the original backfill-with-a-fabricated-outcome approach was never +run. ## Scope Fix the root cause of a production-wide defect found while UI-testing -[`2026-07-11-ui-review-ia-usability.md`](done/2026-07-11-ui-review-ia-usability.md): +[`2026-07-11-ui-review-ia-usability.md`](2026-07-11-ui-review-ia-usability.md): every session on the live task board shows as "Running" forever. Traced through `cmd/nomos/` and confirmed against the running database — this is not a frontend bug (the board correctly reflects real `agent_sessions.status` values). It's an agent-behavior gap: the model almost never calls the lifecycle tools (`set_goal` / `propose_plan` / `complete_task`) that the task-board feature (shipped today, -[`done/2026-07-11-goal-oriented-chat-control-panel.md`](done/2026-07-11-goal-oriented-chat-control-panel.md)) +[`done/2026-07-11-goal-oriented-chat-control-panel.md`](2026-07-11-goal-oriented-chat-control-panel.md)) depends on to know a task is finished. ## Evidence @@ -33,7 +36,7 @@ per-session transcripts) against the running mac-mini stack: lxc:caddy?"), the model runs one read tool (`run hostname`), answers in plain text, and the turn ends — no lifecycle tool call at all. This is exactly the case - [`nomos/SOUL.md:105-109`](../nomos/SOUL.md#L105) calls out by name + [`nomos/SOUL.md:105-109`](../../nomos/SOUL.md#L105) calls out by name ("a trivial read-only task... is a degenerate case... answer it and `complete_task` with a one-line summary") — the instruction exists and is explicit, and the model skips it anyway, consistently. @@ -74,7 +77,7 @@ gap. Neither exit currently checks whether the session ever reached a terminal state — the turn just ends, and `agent_sessions.status` is left wherever it was (usually `active`, its creation-time default, -[`store.go:71,84`](../cmd/nomos/store.go#L71)). +[`store.go:71,84`](../../cmd/nomos/store.go#L71)). ## Design @@ -92,7 +95,7 @@ answered. Call `store.completeTask` server-side right there, before the `return`, with `outcome="success"` and a summary derived from the response text (first ~120 chars, same truncation pattern `buildContinuationNote` already uses at -[`continue.go:203-205`](../cmd/nomos/continue.go#L203)). No LLM call needed +[`continue.go:203-205`](../../cmd/nomos/continue.go#L203)). No LLM call needed — this is a mechanical default, not a judgment call, matching the "trivial task" case SOUL.md already describes. @@ -105,7 +108,7 @@ covers them. **2. Structured (goal/plan set) sessions that stall — idle sweep, not inline.** Extend the existing `runContinuationWorker` ticker -([`continue.go:41-57`](../cmd/nomos/continue.go#L41), already polling every +([`continue.go:41-57`](../../cmd/nomos/continue.go#L41), already polling every 4s for a different purpose) with a second, coarser sweep — e.g. every 5 minutes — that finds sessions where: - `status` is `active`, `planning`, or `executing` (not already terminal or @@ -121,7 +124,7 @@ session ("[System: this task has been idle for N minutes with no you're genuinely still working, ignore this.]") the same way `buildContinuationNote` already injects notes into resumed sessions — reuse `resumeSession`'s live-persist pattern -([`continue.go:106-196`](../cmd/nomos/continue.go#L106)) so the nudge and +([`continue.go:106-196`](../../cmd/nomos/continue.go#L106)) so the nudge and the model's response show up in the transcript, not silently. If a second idle sweep finds the same session still not completed (i.e. @@ -130,7 +133,7 @@ the nudge didn't take), auto-complete it directly with unanswered nudge — same reasoning as `resumeSession`'s existing "give the task a real, operator-visible terminal state instead of leaving it silently stuck forever" logic at -[`continue.go:179-193`](../cmd/nomos/continue.go#L179), which already does +[`continue.go:179-193`](../../cmd/nomos/continue.go#L179), which already does exactly this for a different failure mode (a resume that produces no response). This is the same architectural pattern, applied to a session that produces responses but never a terminal tool call. @@ -165,6 +168,51 @@ through with no lifecycle signal at all. of new correctly-terminating ones. This should be a script, not a manual UPDATE — the classification logic will already exist in Go. +## Fix 4, revised: deletion instead of backfill + +The plan as written proposed backfilling the 50 already-stuck sessions with +a mechanically-assigned outcome (`success` for case 1, `partial` for case +2). When it came time to execute that, the operator raised a better +question: these were overwhelmingly one-off test/smoke-test sessions +("hi", "what's the hostname of lxc:caddy?") with no lasting value — +assigning them a fabricated `success` outcome would make the task board +lie in the opposite direction (claiming verified success on things nobody +verified). The operator's call: delete them instead of backfilling a +guessed outcome, with one condition — don't lose any recorded knowledge. + +Before deleting anything, verified directly against the database (not +assumed from reading the code): +- Zero `documents` relationship edges exist linking any of the candidate + sessions to any `knowledge_entities` row. +- Zero `upsert_knowledge` calls appear anywhere in the candidate sessions' + transcripts. +- Zero `knowledge_entities` rows exist system-wide mentioning the one + topic (`typetype`) the operator specifically asked to preserve. + +`deleteSession` (`store.go:293`, already the live code path behind the +UI's "Delete task" button — reused as-is, not reimplemented) removes the +session, its messages, its own task entity, and that entity's relationship +edges — it never touches `knowledge_entities` rows or entities the task +merely referenced (e.g. `lxc:typetype` itself), only the provenance edges +back to the now-deleted task. Given the verification above, this was safe: +there was nothing to preserve because nothing had ever been recorded. + +Executed in two batches, both via the same `DELETE /sessions/:id` route: +- **47 sessions** — the original candidate set from `curl + localhost:8092/sessions`, all non-`done`/`failed` at the time. +- **6 more sessions** — found *after* the first batch, when they surfaced + on the task board: `listSessions` (`store.go:193`) hardcodes + `ORDER BY last_active_at DESC LIMIT 50` with no pagination, so the + original audit's "50 sessions total" was actually "the 50 most + recently active" — it silently excluded 6 older stuck sessions from + 2026-07-08 (predating the task-board feature entirely, same trivial + "hi"/smoke-test pattern). Worth knowing about `listSessions`'s cap for + any future audit of this table — a `count(*)` query directly against + the database is the only way to get a true total. + +Final state: `agent_sessions` holds exactly 3 rows — the two `done` and +one `failed` sessions produced during live verification of fixes 1-3. + ## Implementation order 1. Fix 1 (inline safety net) first — it's the highest-leverage, lowest-risk diff --git a/plans/done/2026-07-11-ui-review-ia-usability.md b/plans/done/2026-07-11-ui-review-ia-usability.md index d7374f2..f6da665 100644 --- a/plans/done/2026-07-11-ui-review-ia-usability.md +++ b/plans/done/2026-07-11-ui-review-ia-usability.md @@ -25,13 +25,13 @@ structure — implement on a later "proceed." ### A. Information architecture **A1. Entity detail has two competing UI patterns for the same content.** -[`Entities.svelte:16-19,155`](../web/src/pages/Entities.svelte) opens entity +[`Entities.svelte:16-19,155`](../../web/src/pages/Entities.svelte) opens entity detail as an in-page `EntitySheet` slide-over (no URL change, no sidebar -state change). [`Knowledge.svelte:57-59`](../web/src/pages/Knowledge.svelte) -and [`Graph.svelte:464`](../web/src/pages/Graph.svelte) instead navigate via +state change). [`Knowledge.svelte:57-59`](../../web/src/pages/Knowledge.svelte) +and [`Graph.svelte:464`](../../web/src/pages/Graph.svelte) instead navigate via `location.hash = '#/entity/' + slug`, which `App.svelte`'s router resolves to a full-page `EntityDetail` route — but `'entity'` isn't in `navItems` -([`App.svelte:68-79`](../web/src/App.svelte)), so landing there leaves the +([`App.svelte:68-79`](../../web/src/App.svelte)), so landing there leaves the sidebar with nothing highlighted and the header showing the raw slug instead of a section name. Same underlying view (`EntityDetailContent.svelte`), three different entry points, two @@ -42,8 +42,8 @@ A user who reaches an entity via Knowledge or Graph has no way back to **A2. Two chat entry points with no visual link between them.** The sidebar's "Tasks" section (board → `Chat.svelte` detail, `isActive={page === 'tasks' || page === 'chat'}`, -[`App.svelte:127`](../web/src/App.svelte)) and the footer's "Chat drawer" -button ([`App.svelte:160-163`](../web/src/App.svelte), opens a `Sheet` +[`App.svelte:127`](../../web/src/App.svelte)) and the footer's "Chat drawer" +button ([`App.svelte:160-163`](../../web/src/App.svelte), opens a `Sheet` wrapping the same `Chat` component) are both valid, intentional ways to reach chat — but nothing in the UI explains they're different modes (drawer = overlay on current page, keeps your place; Tasks = full navigation). A @@ -51,10 +51,10 @@ first-time user has no way to know which one preserves their current page. Low-severity, but worth a tooltip/label distinction. **A3. Overview's KPI cards don't drill down.** -[`Overview.svelte`](../web/src/pages/Overview.svelte) shows "Pending +[`Overview.svelte`](../../web/src/pages/Overview.svelte) shows "Pending approvals," "Open signals," and fleet-health counts as static cards. The header badges for the same data (`approvalsPending`, `openSignals`, -[`App.svelte:185-194`](../web/src/App.svelte)) ARE clickable and navigate to +[`App.svelte:185-194`](../../web/src/App.svelte)) ARE clickable and navigate to Ops/Signals — so the pattern exists in the app, just not on the page whose entire purpose is summarizing this data. A dashboard card showing a count that doesn't lead anywhere is a standard drill-down gap. @@ -63,17 +63,17 @@ that doesn't lead anywhere is a standard drill-down gap. **B1. Table-row click targets lack keyboard/screen-reader support in one place but not others.** -[`Entities.svelte:117-120`](../web/src/pages/Entities.svelte) makes an +[`Entities.svelte:117-120`](../../web/src/pages/Entities.svelte) makes an entire `Table.Row` clickable via a bare `onclick`, with no `role`, `tabindex`, or `onkeydown` — unreachable and inoperable via keyboard, and screen readers get no indication the row is interactive. This is a regression against the codebase's own established pattern: `Tasks.svelte` wraps its cards in real `