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

# 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."