From c8b479d5653131e7f4ab7a57e81adbcf12d4e813 Mon Sep 17 00:00:00 2001 From: dtoro Date: Sun, 12 Jul 2026 00:14:15 +0200 Subject: [PATCH] docs: close out task-completion safety net plan; fix stale relative links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes 1-3 deployed and verified live: fresh trivial Q&A sessions now reach done immediately, and a goal-bearing session that stalled was correctly nudged by the idle sweep. Fix 4 (backfill) was replaced with deletion after the operator's call — verified against the DB first that zero knowledge notes were linked to or written by any of the 53 removed sessions, so nothing was lost. Documents the pagination gap in listSessions (hardcoded LIMIT 50, no total count) that hid 6 of those sessions from the original audit. Also fixes relative links in this plan and in the UI-review plan that broke when both moved from plans/ to plans/done/ (one directory level deeper). Co-Authored-By: Claude Sonnet 5 --- .../2026-07-11-task-completion-safety-net.md | 74 +++++++++++++++---- .../done/2026-07-11-ui-review-ia-usability.md | 44 +++++------ plans/index.md | 2 +- 3 files changed, 84 insertions(+), 36 deletions(-) rename plans/{ => done}/2026-07-11-task-completion-safety-net.md (72%) 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 `