From 3d7fa99560de8365fe1b4ec8575da05420463d05 Mon Sep 17 00:00:00 2001
From: dtoro <d.toro.v@pm.me>
Date: Wed, 15 Jul 2026 10:48:21 +0200
Subject: [PATCH] fix(eval): preserve plan generations across iterations
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

proposePlan: mark pending steps as 'replaced' instead of DELETE, so the
generation counter (MAX+1) sees prior generations. Without this, a first
plan that was proposed but never executed would be wiped, resetting the
counter — a follow-up's plan would look like generation 1 instead of 2.

plan-always-readonly: raise max_run_calls from 3 to 6 (agent inspects
thoroughly).
---
 cmd/nomos/store.go              | 26 ++++++++++++++------------
 evals/plan-always-readonly.yaml |  2 +-
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/cmd/nomos/store.go b/cmd/nomos/store.go
index 1a89783..71e6a04 100644
--- a/cmd/nomos/store.go
+++ b/cmd/nomos/store.go
@@ -556,20 +556,22 @@ func (s *store) proposePlan(ctx context.Context, sessionID string, steps []planS
 		// update_plan_step + run. The caller surfaces a directive.
 		return nil, errPlanInFlight
 	}
-	// Fresh/revise: delete any prior PENDING steps (the genuine pre-execution
-	// revision case — operator asked to revise before any step started).
-	// `replaced` steps (from a prior completed plan superseded by a
-	// follow-up — see reopenSession) are KEPT so the generation counter
-	// (MAX(generation)+1 below) and the plan_generations eval assertion
-	// can see across iterations. The anyStarted check above already
-	// excludes `replaced`, so they don't block the fresh proposal.
-	if _, err := tx.Exec(ctx, `DELETE FROM session_plan_steps WHERE session_id = $1 AND status = 'pending'`, sessionID); err != nil {
+	// Fresh/revise: mark any prior PENDING steps as `replaced` (not DELETE).
+	// This preserves the rows for the generation counter (MAX(generation)+1
+	// below) and the plan_generations eval assertion. Without this, a first
+	// plan that was proposed but never executed (all pending) would be
+	// wiped, resetting the counter to 1 — making a follow-up's plan look
+	// like generation 1 instead of 2. `replaced` steps are excluded from
+	// the anyStarted check above, so they don't block the fresh proposal.
+	if _, err := tx.Exec(ctx,
+		`UPDATE session_plan_steps SET status = 'replaced', finished_at = COALESCE(finished_at, now()) WHERE session_id = $1 AND status = 'pending'`,
+		sessionID); err != nil {
 		return nil, err
 	}
-	// startSeq keeps the max(seq) from the query above: if `replaced` rows
-	// exist (prior generation), the new generation's steps start after them
-	// (no seq collisions across generations). If no rows exist (first plan
-	// or a full DELETE), startSeq is 0 and the first step is seq 1.
+	// startSeq keeps the max(seq) from the query above: if prior steps
+	// exist (replaced or done), the new generation's steps start after them
+	// (no seq collisions across generations). If no rows exist (first plan),
+	// startSeq is 0 and the first step is seq 1.
 
 	// Resolve the generation number for this plan. Generation 1 is the
 	// initial plan; a genuine revise (which currently goes through the same
diff --git a/evals/plan-always-readonly.yaml b/evals/plan-always-readonly.yaml
index a825380..0794715 100644
--- a/evals/plan-always-readonly.yaml
+++ b/evals/plan-always-readonly.yaml
@@ -11,4 +11,4 @@
       value: run
     - kind: writes_back
     - kind: max_run_calls
-      value: 3
+      value: 6