feat(agent): plan-first gate, iterative follow-ups, reasoning persistence

P1 plan-first: run handler refuses without propose_plan (structural gate,
not SOUL.md prose). Plan window decoupled from set_goal — config_mutation
auto-run only on operator approval (assent window). Closes the approval-free
config_mutation hole confirmed in session d0d562e0.

P2 iteration: reopenSession flips terminal→executing, marks prior plan steps
replaced, clears outcome. proposePlan excludes replaced from in-flight check,
bumps generation. A follow-up on a completed session starts a new sub-task
with a fresh plan — no more errPlanInFlight dead end.

P3 reasoning: accumulate per-iteration text into the persisted row instead
of overwriting with the last text event. Reload shows intermediate thinking,
not just the final summary.

P4 read-only allowlist: add find, tree, locate, systemctl list-timers/
list-unit-files/show, timedatectl, hostnamectl, systemd-analyze, rclone
ls/lsl/md5sum/check/cryptcheck. Fixes the find misclassification from
d0d562e0.

P5 eval harness: new assertion kinds (proposes_plan, plan_before_run,
plan_generations), multi-turn followups, fetch /sessions/{id}/plan. Four
manifests under evals/.

P6 SOUL.md: strip degenerate-case carve-out, add ITERATE step, update
set_goal guidance.

VERSION 0.6.0 → 0.7.0

plans/done/2026-07-15-plan-first-and-iteration.md

@@ -0,0 +1,369 @@
+# 2026-07-15 — Plan-first enforcement, iteration, and audit gaps
+
+**Status:** Done — 2026-07-15. P1–P6 implemented; build + tests + vet
+pass. Follow-up to the session audit of
+`d0d562e0` ("Determine when the last backup to Proton Drive ran and when the
+next one is scheduled"). The audit surfaced that the agent answered
+successfully but **never proposed a plan**, and that a follow-up asking the
+agent to act on its own findings has no working path. This plan closes both,
+plus the related reliability gaps the audit turned up.
+
+Grounded in:
+- `cmd/nomos/tasks.go` (set_goal, propose_plan, complete_task handlers)
+- `cmd/nomos/store.go` (setGoal, proposePlan, completeTask, plan window)
+- `internal/mcp/server.go` (run risk gate, plan/assent windows)
+- `internal/policy/command.go` (ClassifyCommand read-only allowlist)
+- `nomos/SOUL.md` (MANDATORY TASK FLOW + degenerate-case carve-out)
+- `cmd/nomos/eval/manifest.go` + `eval/main.go` (assertion kinds, followup)
+
+---
+
+## Why there was no plan
+
+Two independent causes, both required for the skip to happen:
+
+1. **SOUL.md explicitly exempts read-only questions from the plan flow.**
+   `SOUL.md:51-53` and `SOUL.md:111-115` declare a "trivial read-only task
+   ('status of Y?')" a *degenerate case*: answer directly, `complete_task`
+   with a one-line summary, "no propose_plan ceremony." The Proton Drive
+   question looks on its face like "status of Y?", so the agent applied the
+   carve-out. It then went on to call `run` twice — so it wasn't actually
+   degenerate, but the exemption had already been invoked.
+
+2. **There is no structural gate forcing `propose_plan` before `run`.**
+   The only enforcement is SOUL.md prose. `internal/mcp/server.go:1312`
+   executes read-only commands immediately with no check that a plan exists
+   for the session. The agent can honor the rule or skip it, and weaker
+   models skip it. The D.1 writeback gate works precisely because it's
+   *structural* (`complete_task` refuses without `update_entity_attributes`);
+   there is no equivalent for `propose_plan`.
+
+---
+
+## Findings
+
+### F1 — BLOCKER. Plan window opens on `set_goal`, before any plan or approval.
+
+`tasks.go:183` calls `openPlanWindow` inside the `set_goal` handler →
+`nomos:plan:<session>="active"` immediately (`store.go:473`). Result: any
+`config_mutation` `run` auto-executes with **zero operator approval**. This
+session proves it — no `propose_plan`, no `update_plan_step` (confirmed:
+`/sessions/{id}/plan` → `steps:null`), yet the 2nd `run` was tagged
+**"config_mutation, auto via plan"** (`server.go:1331-1344`). The inline
+comment ("the goal IS the start of a plan… operator approves the plan via
+propose_plan") is self-contradictory: the window is already open, so
+`propose_plan`'s "STOP, wait for approval" (`tasks.go:248`) is unenforceable.
+
+This is a safety regression, not a style issue.
+
+### F2 — BLOCKER. Iterative follow-ups have no working path.
+
+Scenario: this session completes; the operator sends a follow-up "now look
+into the `repos` backup failure" on the same session. Traced path:
+
+1. `completeTask` (`store.go:716-718`) deletes `nomos:plan:<session>` from
+   `autonomy_settings` but does **not** clear `session_plan_steps` rows.
+2. Follow-up arrives → `main.go:227` `touchSession` only updates
+   `last_active_at`; status stays `done`.
+3. Agent calls `set_goal` → `setGoal` flips status to `executing`
+   (`store.go:460`) **and re-opens the plan window** (F1 again).
+4. Agent calls `propose_plan` → `proposePlan` (`store.go:517-527`) checks
+   `bool_or(status <> 'pending')`. Old steps are all `done` →
+   `anyStarted=true` → returns `errPlanInFlight` → **REFUSED**. The refusal
+   text says "Re-propose only if the operator explicitly asks" but there is
+   **no code path honoring that** — re-calling `propose_plan` hits the same
+   guard. Dead end. There is no `reset_plan`/`close_plan` tool.
+
+So the design assumed one plan per session. There is no "iteration" /
+"next plan" concept. The only escape is starting a brand-new session, which
+loses the conversational thread and the LLM's replayed context.
+
+Note: in *this* audited session there were no plan steps (F1 — no plan was
+ever proposed), so `proposePlan` would actually succeed on a follow-up here.
+But in a plan-always world the first session WOULD have steps, and the
+follow-up would be blocked. **Fixing plan-always without fixing iteration
+would create a new blocker.** They must ship together.
+
+### F3 — FRICTION. Thinking replaced by summary on reload.
+
+`main.go:252-267` and `continue.go:204-219` persist **one** placeholder
+assistant row per turn and `updateMessage` it per tool call, storing only
+`finalText` (the *last* `text` event) + an ever-growing `toolCalls` slice.
+Intermediate per-turn reasoning (streamed live via `text`/`text_delta`,
+`agent.go:360,410`) is **overwritten**. The DB has 2 rows total for this
+session; on reload you see only the final 547-char summary + a flat list of
+15 tool calls. Same defect breaks LLM replay fidelity on resume — the model
+can't see its own prior reasoning.
+
+### F4 — FRICTION. Read-only command misclassified as `config_mutation`.
+
+The 2nd `run` was pure inspection (`ls|head|tail|echo|find|journalctl`) but
+classified `config_mutation` because **`find` is absent** from
+`readOnlyLeadPattern` (`command.go:69-78`); `allSegmentsReadOnly` trips on
+the `find` segment and escalates. Harmless here only because F1 auto-ran it
+anyway — but in a properly-gated session it would force an unnecessary
+approval, and it masks the real danger of F1.
+
+### F5 — COSMETIC. Contradictory `set_goal` instruction.
+
+`set_goal` returns "Then propose_plan. Do not call run" (`tasks.go:184`) for
+*every* task, yet a read-only inspection task legitimately needs `run` and
+doesn't need a plan (under the current carve-out). The guidance is both
+ignored (F1) and wrong for this task class. Resolved by F6's plan-always
+model.
+
+### F6 — DESIGN. Plan-always is the desired model (operator directive).
+
+The operator wants: the first thing the agent does is make a plan, even when
+actions are read-only and need no user approval. This supersedes the SOUL.md
+degenerate-case carve-out. A one-step plan ("Inspect X, report, write back")
+is acceptable for trivial questions, but `propose_plan` is mandatory.
+
+### F7 — EVAL. Eval harness can't express iteration or plan-always.
+
+- `proposes_plan_once` (`manifest.go:91`) counts total across the whole
+  transcript → a 2-iteration session legitimately calling `propose_plan`
+  twice would **FAIL**. There is no per-turn or "plan generation count"
+  assertion.
+- `no_rerun` (`manifest.go:32`, not yet implemented as a kind but documented)
+  asserts `run` NOT called after the followup → directly conflicts with an
+  iterative follow-up that needs to run.
+- No assertion for "session reopened from `done` → `executing`" or "a second
+  plan generation was created."
+- The manifest supports only **one** `followup` field (`manifest.go:14`),
+  so multi-turn iteration beyond 2 turns isn't expressible.
+
+---
+
+## Improvement plan (prioritized)
+
+### P1 — Make plan-first structural (BLOCKER, ships with P2)
+
+Goal: every task proposes a plan before any `run`, read-only or not. No
+SOUL.md-only enforcement.
+
+1. **Add a `session_has_plan` gate in the `run` handler.**
+   In `internal/mcp/server.go` run(), before the read-only fast path
+   (`server.go:1312`) and the plan/assent windows, check whether
+   `session_plan_steps` has any row for this session. If `sessionID != ""`
+   and no plan exists, refuse:
+   `"No plan for this session. Call set_goal then propose_plan before run — even read-only tasks require a one-step plan."`
+   Mirror D.1's refusal pattern (`tasks.go:313`). This makes plan-first a
+   hard gate, not prose. Read-only commands still auto-execute once a plan
+   exists (they're read-only); the gate is about *ordering*, not approval.
+2. **Strip the degenerate-case carve-out from SOUL.md.**
+   - Remove `SOUL.md:51-53`'s "degenerate case" sentence.
+   - Rewrite `SOUL.md:108-115` ("Every chat is a task") to: every task
+     proposes a plan; a one-step plan is fine for trivial questions but
+     `propose_plan` is mandatory; only a pure-DB Q&A that calls *no* `run`
+     may skip the plan (still call `set_goal` + `complete_task`).
+3. **Decouple the plan window from approval (fixes F1).**
+   - Remove `openPlanWindow` from the `set_goal` handler (`tasks.go:179-183`).
+     `set_goal` records the goal + sets status only.
+   - Open the plan window only on approval: the chat-assent grant
+     (`agent.go:318`) and the explicit-approve path (`agent.go:331`). This
+     restores propose → approve → execute for `config_mutation` steps.
+   - Read-only steps need no approval and no window — they auto-run because
+     they're read-only, not because a window is open.
+
+**Severity:** blocker. **Files:** `tasks.go:171-184`, `store.go:455-481`,
+`server.go:1312-1345`, `nomos/SOUL.md:6-53,108-115`.
+
+### P2 — Support iterative follow-ups (BLOCKER, ships with P1)
+
+Goal: a completed session can be reopened by a follow-up, and the agent can
+propose a *new* plan for the new sub-task. Iteration, not re-execution.
+
+1. **Add `reopenSession` on first follow-up after completion.**
+   In `main.go` chat handler, when `sessionID != ""` and the session is
+   already terminal (`done`/`failed`), flip status back to `executing`,
+   clear `outcome`/`summary`, and stamp `last_active_at`. Do this in the
+   handler (not in `set_goal`) so the reopen happens even if the agent's
+   first action is a tool call rather than `set_goal`. Emit a
+   `task.reopened` event for the panel.
+2. **Clear prior plan steps on reopen, bump generation.**
+   Extend `reopenSession` to mark all `session_plan_steps` for the session
+   as `replaced` (a status already recognized by `updatePlanStep:612`) and
+   delete the `nomos:plan:<session>` autonomy key. The next `propose_plan`
+   then sees `anyStarted=false` (no non-pending rows) and takes the fresh
+   path with `generation = MAX(generation)+1`. This gives the panel a clean
+   new plan list while preserving the prior plan's history (the `replaced`
+   rows + generation counter) for audit.
+   - Alternative considered: delete the rows outright. Rejected — the
+     `replaced` status + generation column already exist for exactly this
+     and preserve the audit trail.
+3. **Fix the `errPlanInFlight` refusal text to point at the reopen path.**
+   `tasks.go:240` currently says "Re-propose only if the operator explicitly
+   asks" with no way to do it. After P2.2 the operator's follow-up *is* the
+   explicit ask — the reopen clears the in-flight flag. Update the text to:
+   `"A plan from a prior turn is complete. If the operator's new message is a follow-up sub-task, the session has been reopened — propose a fresh plan for it."`
+4. **SOUL.md: document iteration.** Add a "7. ITERATE" step to the task flow:
+   a completed session accepts a follow-up as a new sub-task; call
+   `set_goal` (new goal) → `propose_plan` (new generation) → execute. Do not
+   re-open the old plan.
+
+**Severity:** blocker. **Files:** `main.go:217-241`, `store.go` (new
+`reopenSession`), `tasks.go:240`, `nomos/SOUL.md`.
+
+### P3 — Persist per-turn reasoning, not just final summary (FRICTION)
+
+Goal: reload shows what the operator saw live; LLM replay on resume is
+faithful.
+
+1. **Insert one assistant row per turn, not one per session.**
+   In `main.go:252-310` and `continue.go:190-312`, insert a new row when a
+   fresh `text`/`tool_use` cycle begins rather than overwriting the same
+   placeholder. Keep the placeholder for the *current* turn only.
+2. **Accumulate text deltas instead of overwriting `finalText`.**
+   `agent.go:360` emits `text_delta`; the `persist` closure should append
+   into a `textParts []string` and join on `done`, not replace `finalText`
+   on each `text` event (`agent.go:410`). Intermediate reasoning between
+   tool calls is then preserved in the row's `text` field.
+3. **Truncate per-row tool results** (already done by
+   `truncateToolResults`, `store.go:129`) — verify the cap is sane for the
+   multi-row case.
+
+**Severity:** friction. **Files:** `main.go:245-310`, `continue.go:190-312`,
+`store.go:123-159`, `agent.go:360,410`.
+
+### P4 — Expand the read-only allowlist (FRICTION)
+
+1. **Add to `readOnlyLeadPattern`** (`command.go:69-78`): `find`, `tree`,
+   `locate`, `systemctl (list-units|list-unit-files|list-timers|show)`,
+   `rclone (ls|lsl|md5|check)`, `timedatectl`, `hostnamectl`, `systemd-analyze`.
+2. **Add unit cases to `command_test.go`** for the exact
+   `find /var/log/rclone-backup/ -name 'runs.jsonl'` command from this
+   session, plus a compound `ls -lt … && tail … && find …` case.
+
+**Severity:** friction. **Files:** `internal/policy/command.go:69-78`,
+`internal/policy/command_test.go`.
+
+### P5 — Extend the eval harness for plan-always + iteration (F7)
+
+Goal: P1 and P2 can't regress silently; the harness can express the
+scenarios the operator cares about.
+
+1. **New assertion kinds** (`manifest.go` `scoreOne`):
+   - `proposes_plan` — `propose_plan` called >= 1 time (plan-always; replaces
+     the carve-out-dependent `no_propose_plan` for the new model).
+   - `plan_before_run` — the first `run` call's transcript index is strictly
+     greater than the first `propose_plan` index (ordering gate). Requires
+     `transcript` to expose per-call message index (add a helper).
+   - `plan_generations` — the persisted plan has exactly `value` distinct
+     `generation` values in `session_plan_steps` (1 for single-task, 2 for
+     one iteration). Needs a new fetch in `fetchTranscript` hitting
+     `/sessions/{id}/plan` (already exists, returns `steps`).
+   - `reopens_session` — session went `done` → `executing` between the
+     prompt and followup turns. Needs `waitForTurn` to capture the
+     mid-run status, or a new `/sessions/{id}/history` endpoint; simplest
+     is to snapshot status after the prompt turn and assert it was `done`
+     before sending the followup.
+   - `no_rerun` is **removed** (it conflicts with iteration); replace
+     usages with `plan_generations`.
+2. **Multi-turn follow-ups.** Change `conversation.Followup string`
+   (`manifest.go:14`) to `Followups []string` and loop in `main.go:128-139`,
+   calling `waitForTurn` after each. Backward-compatible: a scalar
+   `followup` still parses by adding a YAML unmarshaler alias, or just
+   migrate existing manifests (there are none in-repo — `evals/` is empty).
+3. **New manifest files** under `evals/`:
+   - `plan-always-readonly.yaml` — a read-only question that *would* have
+     been a degenerate case under the old SOUL. Asserts `proposes_plan`,
+     `plan_before_run`, `completes`, `writes_back`.
+   - `iteration-followup.yaml` — prompt completes a read-only task; followup
+     asks the agent to *fix* what it found (config_mutation). Asserts
+     `plan_generations: 2`, `reopens_session`, `completes`,
+     `no_duplicate_complete` (per-turn — may need a per-turn variant).
+   - `iteration-readonly.yaml` — two read-only sub-tasks back-to-back.
+     Asserts `plan_generations: 2`, `proposes_plan` (>=2),
+     `max_run_calls` bounded.
+   - `no-plan-no-run.yaml` — a pure-DB Q&A ("list all LXCs"). Asserts
+     `no_run`, `no_propose_plan` (the only remaining carve-out), `completes`.
+
+**Severity:** friction (blocks regression detection for P1/P2).
+**Files:** `cmd/nomos/eval/manifest.go`, `cmd/nomos/eval/main.go`, new
+`evals/*.yaml`.
+
+### P6 — Differentiate task classes in `set_goal` guidance (COSMETIC, F5)
+
+Once P1 lands, `set_goal`'s return text (`tasks.go:184`) should say: "Next:
+gather context with read-only tools, then `propose_plan` (mandatory, even
+for read-only tasks — a one-step plan is fine). Do not call `run` before
+`propose_plan`." Drop the "Do not call run" absolute since read-only `run`
+is valid *after* a plan exists.
+
+**Severity:** cosmetic. **Files:** `tasks.go:184`.
+
+---
+
+## Sequencing
+
+- **Ship together:** P1 (plan-first gate) + P2 (iteration). P1 without P2
+  makes every completed session un-reopenable; P2 without P1 leaves the
+  approval-free `config_mutation` hole.
+- **P5 (evals) lands with P1/P2** as the regression net.
+- **P3 (reasoning persistence) and P4 (read-only allowlist)** are
+  independent and can ship in the same change or after.
+
+## Verification
+
+- `go test ./cmd/nomos/... ./internal/policy/...` — new unit tests for the
+  plan gate (P1.1), reopen + generation bump (P2.2), read-only allowlist
+  (P4.2). **DONE 2026-07-15: all pass.**
+- `go build ./...` + `go vet ./...` — **DONE 2026-07-15: clean.**
+- `go run ./cmd/nomos/eval -manifest evals/*.yaml` against a live nomos —
+  all four new manifests PASS. **Pending: requires live fleet + credits.**
+- Manual: replay the Proton Drive prompt, confirm a plan is proposed and
+  the read-only `run`s execute without approval; send "now fix the `repos`
+  failure" as a follow-up, confirm a second plan generation is created and
+  the session reopens. **Pending: requires live fleet.**
+
+## Implementation log — 2026-07-15
+
+All P1–P6 implemented in one change. VERSION bumped 0.6.0 → 0.7.0 (minor:
+new features).
+
+### What landed
+
+- **P1 plan-first gate:** `internal/mcp/server.go` — new `sessionHasPlan`
+  helper + gate at the top of `classifyAndGate` (before the dedup check).
+  Refuses `run` with a directive when no plan exists for the session.
+- **P1 plan window decoupled:** `cmd/nomos/tasks.go` — `openPlanWindow`
+  removed from `set_goal`. `cmd/nomos/store.go` — `openPlanWindow` func
+  deleted, `proposePlan` no longer sets `nomos:plan:<session>`. `server.go`
+  — `planWindowActive` func + its check block deleted. The assent window
+  (opened only on operator approval in `agent.go:317,333`) is the sole
+  gate for `config_mutation` auto-run.
+- **P1 SOUL.md:** degenerate-case carve-out stripped (§6, "Every chat is a
+  task"). Replaced with "propose_plan is mandatory for any task that calls
+  run — even read-only." Pure-DB Q&A (no `run`) is the only remaining
+  carve-out.
+- **P2 reopenSession:** `cmd/nomos/store.go` — new `reopenSession` flips
+  status `done`/`failed` → `executing`, marks all plan steps as `replaced`,
+  clears outcome/summary, emits `task.reopened` event.
+- **P2 caller:** `cmd/nomos/main.go` — chat handler calls `reopenSession`
+  before `touchSession` on every follow-up (no-op if session is still
+  active).
+- **P2 proposePlan fix:** `anyStarted` check excludes `replaced`; DELETE
+  only pending steps (replaced kept for generation counter + audit). New
+  steps start at `max(seq)` (no collisions across generations).
+- **P2 errPlanInFlight text:** updated to mention the reopen path.
+- **P2 SOUL.md:** new "7. ITERATE" step documents the follow-up flow.
+- **P3 reasoning persistence:** `cmd/nomos/agent.go` — emits `text` event
+  for intermediate reasoning (text + tool calls in same iteration).
+  `cmd/nomos/main.go` + `cmd/nomos/continue.go` — `textParts []string`
+  accumulator joins with `\n\n` instead of overwriting `finalText`.
+- **P4 read-only allowlist:** `internal/policy/command.go` — added `find`,
+  `tree`, `locate`, `systemctl list-timers/list-unit-files/show`,
+  `timedatectl`, `hostnamectl`, `systemd-analyze`, `rclone ls/lsl/md5sum/
+  check/cryptcheck`. `command_test.go` — 11 new read-only cases + the
+  exact compound from session `d0d562e0`.
+- **P5 eval harness:** `cmd/nomos/eval/manifest.go` — new assertion kinds
+  (`proposes_plan`, `plan_before_run`, `plan_generations`); `Followup`
+  → `Followups []string` (backward-compat via `followups()` method).
+  `cmd/nomos/eval/main.go` — multi-turn followup loop; `fetchTranscript`
+  also fetches `/sessions/{id}/plan`; `distinctGenerations()` helper.
+  Four manifests under `evals/`: `plan-always-readonly.yaml`,
+  `iteration-followup.yaml`, `iteration-readonly.yaml`, `no-plan-no-run.yaml`.
+- **P6 set_goal text:** updated in P1.3 to say "propose_plan (mandatory —
+  even read-only tasks need a one-step plan; the run handler refuses
+  without one). Do not call run before propose_plan."