oikos/plans/done/2026-07-20-session-review-ten-sessions.md

# 2026-07-20 — Session review: past 10 sessions

**Status:** Implemented — all P0/P1/P2 items landed in v0.7.13.
**Scope:** Ten most-recently-active `agent:nomos` sessions by
`last_active_at`, pulled from `http://localhost:8092/sessions` on
2026-07-20. Method per `.agents/skills/session-review/SKILL.md`. Three
(`1e9c7691`, `55927f0a`, `2926de4e`) overlap with the 2026-07-18 review
and are summarized; the other seven are new.

---

## Sessions reviewed

| # | sid | goal (short) | outcome | msgs | toolcalls | top tools |
|---|---|---|---|---|---|---|
| 1 | `a51e2086` | reset rclone-backup & re-run | **partial** | 12 | 20 | run:7, set_goal:2, propose_plan:2, get_execution_status:2 |
| 2 | `fefa4fa3` | fix rclone OOM | success | 9 | 84 | run:28, update_plan_step:15, get_entity:8, list_entities:5 |
| 3 | `95fdd322` | quick fleet health check | success | 3 | 6 | get_health_summary/state_snapshot/list_lxcs/signal_history |
| 4 | `8c76bb3a` | greeting + title-sync test | success | 2 | 9 | update_plan_step:4, propose_plan, whoami, get_state_snapshot |
| 5 | `438ec8bd` | (no goal set) greeting | success | 2 | 2 | whoami, get_health_summary |
| 6 | `8acea2e3` | inspect rclone timer (live) | **partial** | 4 | 19 | run:6, update_plan_step:5, propose_plan, get_entity_knowledge |
| 7 | `1e9c7691` | debug chown hang on strong | success | 13 | 97 | run:60, update_plan_step:7, get_execution_status:7 |
| 8 | `55927f0a` | add NFS ludo-lvm → ZimaOS | success | 25 | 104 | run:49, update_plan_step:13, get_entity:10 |
| 9 | `2926de4e` | apt upgrade host:netbird-vps | success | 9 | 27 | update_plan_step:7, run:6, search_knowledge:2 |
| 10 | `cb8c8a4a` | inspect rclone timer (live) | success | 2 | 14 | update_plan_step:4, run:4, get_entity_knowledge |

**Score: 8 success / 2 partial / 0 blocked. No message exceeded 2.8 KB.**

---

## What worked

- **Read-only DB Q&A is now clean.** `95fdd322` and `438ec8bd` did exactly
  what the 2026-07-18 review asked: pure-DB question →
  `get_health_summary` + `get_state_snapshot` + `list_lxcs`, no `run`.
  The agent even narrates "This is a pure-DB Q&A — no `run` calls needed."
- **Knowledge writeback hygiene continues.** Every long-running session
  did `upsert_knowledge` + `update_entity_attributes` + `create_relationship`
  when applicable. The graph is current.
- **Plan lifecycle is followed everywhere** — `set_goal` → `propose_plan`
  → `update_plan_step` → `complete_task`. Even trivial sessions (greeting)
  follow it.
- **Poll-after-timeout pattern** is now the default — `fefa4fa3` after
  the rclone LXC reboot, `2926de4e` after the apt upgrade. No more blind
  retry storms like the 2026-07-18 chown case.
- **The rclone saga ended well** (`fefa4fa3`): root cause (2 GiB LXC OOM)
  was diagnosed via DB + live check; fix (pct set 2→4 GiB) was applied;
  test backup verified 245 transfers / 4 min / no OOM.

## What didn't

### 1. The rclone objective took three sessions to close (blocker)
Same operator goal — "rclone backup is broken" — spawned `a51e2086`
(partial), `8acea2e3` (partial), `cb8c8a4a` (success), and finally
`fefa4fa3` (success). The first three were the agent trying to inspect
the live systemd state and bouncing off the classifier:

- `8acea2e3`: `pct exec 132 systemctl status rclone-backup.timer`
  flagged `config_mutation` — sat in approval limbo until the user moved
  on.
- `a51e2086`: `curl http://192.168.8.214:5572/rc/...` (read-only RC API)
  flagged `config_mutation`. The agent kept reframing; user said "lets
  just close this session."
- `cb8c8a4a`: same goal, eventually succeeded — but only after the agent
  found a different path.
- `fefa4fa3`: only when the user escalated to "fix it so the backup
  works" did the agent pivot to the actual root cause (memory).

This is the single biggest friction point in the batch.

### 2. Classifier overreach on read-only `pct exec` / `curl` (blocker)
The preflight classifier in `internal/policy` matches command substrings
(`pct exec`, `curl`, `dd`, etc.) without parsing the actual command. A
read-only `systemctl status` becomes `config_mutation`. The agent has
no tool to ask "classify this command before I send it" — it just keeps
retrying with cosmetic changes until the user bails.

### 3. `update_plan_step` is the second-largest tool bucket (cosmetic → friction)
Across 10 sessions: `run` ~199, `update_plan_step` ~57. That's ~22% of
all tool calls spent on bookkeeping. For a 2-message greeting session
(`8c76bb3a`) the agent still called `update_plan_step` ×4 plus
`propose_plan`. The scaffolding is louder than the work.

### 4. `pending_approvals` doesn't match reality (cosmetic, but misleading)
`a51e2086` summary literally says *"Both commands are queued"* — yet
`pending_approvals=0`. The field is `hasPendingApprovals`
(`store.go:962`) which only counts executions currently in
`pending_approval` state; once they're cancelled/expired it drops to 0
even though the session was *blocked* by approvals. As an audit signal
it lies. A session can be `outcome=partial` because of approval
friction without `pending_approvals` ever being non-zero at review time.

### 5. Title is still the first sentence of the first assistant message (cosmetic)
`"Assent window is open — executing the plan\n\nMemory bumped:
4294967296..."` is not a useful label. Same complaint applies to
`8c76bb3a` ("Hey! 👋 Nomos here, running on mac-mini:8092...") and
`95fdd322` ("This is a pure-DB Q&A — no `run` calls needed..."). The
list view ends up being unreadable without opening each row.

### 6. Goal field empty on one session (`438ec8bd`) (cosmetic)
`set_goal` was never called for the bare greeting. Minor, but it means
the session is unsearchable by goal text.

---

## Ease of getting session details

I had to write Python+curl to audit 10 sessions. The pain points:

1. **Two endpoints must be merged by hand.** `/sessions` returns
   metadata (`title`, `goal`, `outcome`, `summary`, `status`,
   `pending_approvals`, timestamps) but **no message/tool counts**.
   `/sessions/{id}` returns **only** `session_id` + `messages` — no
   metadata at all. `cmd/nomos/eval/main.go:302-303` already carries a
   comment complaining about this ("only session_id + messages"). Any
   consumer has to do the same join I did.
2. **No aggregates on the list endpoint.** `message_count`,
   `tool_call_count`, `top_tools`, `duration` — all require fetching
   every session's full transcript and walking the message tree. For
   10 sessions that's 10 extra HTTP round trips and ~600 KB of JSON
   parsed client-side. For a fleet audit at scale it's quadratic.
3. **No filtering or pagination on `/sessions`.** It returns every
   session in one shot. The skill's own script does `.sessions[:5]` and
   `.sessions[:10]` client-side.
4. **Tool calls are nested two levels deep**
   (`messages[].content.tool_calls[].name`) with `content` stored as
   `json.RawMessage`. The jq path requires `?.` everywhere. A flat
   `/sessions/{id}/tool_calls` view would be far easier to analyze.
5. **No `/sessions?outcome=partial` or `?entity_id=...` filter.**
   Finding "show me every session that touched `lxc:rclone` and didn't
   succeed" requires the full scan.
6. **`title` is the raw first assistant text.** Useless for skimming a
   list — you have to open each row to know what it was.
7. **No `closed_at` / `outcome_set_at`.** `last_active_at` is the
   closest proxy but it conflates "agent is still working" with
   "operator just opened the transcript." Duration can only be
   computed as `last_active - created`, which is wrong for reopened
   sessions (`a51e2086` shows "5647 min" = 4 days because the user
   re-opened it on 2026-07-19 to close it).
8. **No "blocker reason" field.** When `outcome=partial`, the *why* is
   buried in the last assistant text. A structured
   `blocker: "approval_timeout"` / `blocker: "classifier_overreach"` /
   `blocker: "user_abandoned"` would make trend analysis trivial.

---

## Improvement plan

### P0 — Blockers ✅

1. ✅ **Stop the classifier from flagging read-only `pct exec` / `curl` as
   `config_mutation`.** In `internal/policy`, parse the command (not
   just substring-match) before assigning risk class. Concretely:
   `pct exec <id> -- <cmd>` should be classified by *the inner command*,
   not the wrapper. `curl <url>` without `-X POST` / `-d` /
   `--upload-file` is read-only. This single change would have
   collapsed sessions #1, #3, #6, #10 into a handful of tool calls each
   and avoided three duplicate rclone sessions.
   - Done: `internal/policy/command.go` now unwraps `pct exec`, `qm
     guest exec`, `bash -c`, `sh -c`, `sudo`, and env-var assignments
     before classification. Curl GET (the default) without POST/data/
     upload/output flags is now read-only. Output redirection (`>`/
     `>>`) disqualifies the read-only path. Tests in
     `internal/policy/command_test.go` cover the new behaviors.

2. ✅ **Add a command-scoped `preflight` MCP tool.** The existing `preflight`
   in AGENTS.md §3 is entity/service-scoped, not command-scoped. The
   agent today has to keep reframing and re-submitting to discover what
   the classifier will accept. A command preflight returns
   `{risk_class, reason}` synchronously so the agent can decide whether
   to submit, rephrase, or surface to the operator.
   - Done: new `classify_command` MCP tool in `internal/mcp/tools.go`
     that takes `command` + optional `declared_risk` and returns the
     exact risk class that `run` would assign. Documented in
     `nomos/SOUL.md` with explicit guidance to pre-classify before
     `run` when the classification is uncertain — "Do NOT submit a `run`,
     get it queued for approval, and then retry with cosmetic variations."

### P1 — Friction ✅

3. ✅ **De-dupe sessions for the same entity + problem.** When a session
   is `outcome=partial` against an entity and a new session is created
   within 24h with a similar goal, surface the prior session to the
   agent at `set_goal` time. Three rclone sessions exist because each
   new session started from scratch.
   - Done: `cmd/nomos/store.go` gained `recentPartialSessions(ctx,
     excludeSessionID, since)`; the `set_goal` handler in
     `cmd/nomos/tasks.go` calls it and includes up to 5 prior partial/
     failed sessions (with goal + summary) in the response. The agent
     is told to search_knowledge or read the prior transcript before
     re-planning.

4. ✅ **Quiet the `update_plan_step` scaffolding.** Either (a) make the
   agent not call it for single-step sessions (greeting/health-check),
   or (b) stop persisting it as a message — keep it only in a
   `plan_steps` table that the UI hydrates from `/sessions/{id}/plan`
   (which already exists). It currently inflates transcript size and
   tool-call counts.
   - Done: `completeTask` in `cmd/nomos/store.go` now auto-closes any
     in-flight plan steps (pending/running → done on success, →
     skipped on partial/failure). SOUL.md §6 documents the new pattern:
     "for one-step plans ... propose_plan → answer → complete_task,
     skipping the per-step running→done dance entirely."

5. ✅ **Add `blocker` and `closed_at` to the `session` struct.** Set
   `blocker` automatically when `outcome=partial`/`failed`: scan the
   last assistant message for signatures ("queued for approval",
   "cancel", "close this session"). Surface in `/sessions` list so
   trends are queryable.
   - Done: migration `021_session_blocker_and_closed_at.up.sql` adds the
     two columns + backfills `closed_at` for existing terminal sessions
     + adds a partial-index on `closed_at DESC WHERE status IN
     ('done','failed')`. `cmd/nomos/store.go` `completeTask` sets
     `closed_at = now()` and derives `blocker` from the last assistant
     message via `deriveBlocker`. The blocker patterns table covers
     approval_timeout, user_abandoned, classifier_overreach,
     model_refusal, model_empty_response, missing_knowledge,
     missing_capability, tool_error.

### P2 — Cosmetic / API ergonomics ✅

6. ✅ **Add aggregates to `/sessions` list.** `message_count`,
   `tool_call_count`, `duration_seconds`. Computed server-side at list
   time (single SQL pass with LEFT JOINs to `agent_messages` and
   `agent_activity`). Eliminates the N+1 transcript fetch I had to do.
   - Done: `session` struct in `cmd/nomos/store.go` carries the three
     new fields; `listSessionsFiltered`, `getSession`, and
     `recentPartialSessions` all populate them.

7. ✅ **Single endpoint that returns both metadata and messages.** Either
   enrich `/sessions/{id}` with the full `session` struct, or add
   `?include=messages` on the list endpoint. The split-persistence is a
   leaky abstraction called out in `eval/main.go:302-303`.
   - Done: `GET /sessions/{id}` in `cmd/nomos/main.go` now returns
     `{session_id, session, messages}` — the `session` field carries
     the full metadata (title, goal, outcome, summary, blocker,
     pending_approvals, message_count, tool_call_count, etc.). The
     `messages` field is unchanged. Clients that only read `messages`
     keep working.

8. ✅ **Filtering & pagination on `/sessions`.** `?outcome=partial&entity_id=...&since=...&limit=20&cursor=...`.
   Removes the "fetch everything, filter client-side" pattern in the
   skill's own script.
   - Done: `cmd/nomos/main.go` `handleSessionsList` parses
     `outcome`/`status`/`entity_id`/`blocker`/`since`/`cursor`/`limit`
     query params. `listFilter` + `listSessionsFiltered` in
     `cmd/nomos/store.go` build a dynamic WHERE + LIMIT. `since`
     accepts both RFC3339 timestamps and Go durations ("24h", "7d" →
     parsed as hours). The response includes `next_cursor` for paging.

9. ✅ **Auto-title from `goal` (when set), not from the first assistant
   text.** Fall back to the assistant text only if no goal. The greeting
   session `438ec8bd` has `goal=""` and a useless title; `fefa4fa3` has
   goal "Fix the rclone backup so it completes successfully instead of
   OOM-killing" — that's the right title.
   - Done: `setGoal` in `cmd/nomos/store.go` now sets
     `title = goal` on the same UPDATE that sets the goal. The
     title-from-first-assistant-text path in `cmd/nomos/main.go`
     preserves the goal title when one exists (falls back to
     `truncate(finalText, 80)` only when no goal is set). Truncates the
     goal title to 120 chars.

10. ✅ **Add `/sessions/{id}/tool_calls` flat view.** Returns
    `[{id, name, args, result, error, type, message_id, role, seq,
    created_at}]` without the message-shell nesting. Makes jq one-liners
    and trend scripts trivial.
    - Done: new route in `cmd/nomos/main.go` `handleSessionDetail`;
    `getSessionToolCalls` in `cmd/nomos/store.go` walks messages and
    flattens `tool_calls[]` into a chronological flat list. Each
    tool_use/tool_result pair is emitted as two rows sharing an id
    (preserving the persisted shape) — clients that want the merged
    shape can group by ID.

---

## Suggested order

If only two land: **P0.1** (parse the inner command for `pct exec` /
`curl` classification) and **P2.6** (aggregates on `/sessions`). The
first eliminates the most visible user-facing friction in this batch
(three duplicate rclone sessions); the second makes future audits like
this one a single `curl | jq` instead of a Python script.

---

## Verification commands

```bash
# Re-pull any session for follow-up
curl -s http://localhost:8092/sessions | jq '.sessions[:10]'

curl -s http://localhost:8092/sessions/a51e2086-a816-4206-a556-dbca362cdda6 | jq .
curl -s http://localhost:8092/sessions/8acea2e3-fc4d-4953-b9df-8e58e59a549a | jq .
curl -s http://localhost:8092/sessions/cb8c8a4a-14a5-4dff-8393-6ed1e7ea7c30 | jq .
curl -s http://localhost:8092/sessions/fefa4fa3-5414-4633-8e5a-51aa4a76609c | jq .

# After P0.1 lands: confirm read-only commands classify as reversible_low
# (whatever the preflight surface becomes — TBC when the tool is added)
```

---

## Related files

- `cmd/nomos/main.go` — `/sessions` and `/sessions/{id}` handlers
  (`handleSessionsList` line 363, `handleSessionDetail` line 383)
- `cmd/nomos/store.go` — `session` struct (line 89), `message` struct
  (line 103), `listSessions` (line 317), `getMessages` (line 377),
  `hasPendingApprovals` (line 962)
- `cmd/nomos/agent.go` — agent loop, retry behavior, goal state
- `cmd/nomos/eval/main.go:302` — comment calling out the
  `/sessions/{id}` "only session_id + messages" gap
- `internal/policy/*` — risk-class classifier (target of P0.1)
- `internal/mcp/server.go` — `run` tool, `preflight` (entity-scoped), all
  MCP tool implementations
- `nomos/SOUL.md` — agent persona, tool-selection rules
- `.agents/skills/session-review/SKILL.md` — the audit protocol
- `plans/2026-07-18-session-review-three-sessions.md` — prior review;
  three sessions overlap with this one

---

## Relationship to the 2026-07-18 review

That review's P0.1 (retry cap), P0.2 (investigate-before-retry SOUL
guidance), P1.3 (runbook capture), P1.5 (bulk inspection tool),
P1.6 (`vm:` target support), P1.8 (ask-before-migrate) all landed or
are tracked separately. This review does **not** re-open them. The
remaining open items from that review are P1.7 (approval window
auto-extends on execution timeout) and P2.9 (long-running command
PENDING detection), both deferred there with rationale; this review
found no new evidence that would change that deferral.