From 53844999032e3e2dd234dcd1a6956dae4652c998 Mon Sep 17 00:00:00 2001 From: dtoro Date: Sat, 11 Jul 2026 14:07:10 +0200 Subject: [PATCH] fix(tasks): plan panel showed only the latest step, not the full plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: proposePlan unconditionally deleted and replaced the whole session_plan_steps list on every call. The model isn't strictly held to "call propose_plan once with the full list" — nothing stopped it (and production evidence + live testing showed it happening) from calling propose_plan once per step as it worked. Each such call wiped every already-completed step, so the operator only ever saw the model's latest single step ("1/1") instead of the real, growing plan. Fix, two layers: - store.go: proposePlan now only does a destructive replace when no step has left 'pending' yet (a genuine pre-execution revision). Once any step has started, a new call APPENDS after the current max seq instead of wiping — so the panel accumulates the full history regardless of how the model chooses to call the tool. plan.proposed now carries `appended` so the frontend knows whether to replace or append. - workspace.ts: plan.proposed handler respects `appended` (update vs set). - tasks.go / SOUL.md: strengthened the propose_plan description and task- loop guidance to call it ONCE with the complete step list end-to-end, using update_plan_step (not re-calling propose_plan) to advance — fixing the root behavioral cause, with the store-side append as a safety net that holds even if the model still calls it incrementally. Verified: forced the exact incremental-call pattern (propose_plan with 1 step, mark it running, propose_plan again with 1 more step) — the second call appended at seq 2 instead of erasing seq 1, and its plan.proposed event carried appended=true. Co-Authored-By: Claude Opus 4.8 --- cmd/nomos/store.go | 42 ++++++++++++++++++++++++++------- cmd/nomos/tasks.go | 14 +++++++---- nomos/SOUL.md | 18 +++++++++----- web/src/lib/stores/workspace.ts | 15 +++++++----- 4 files changed, 63 insertions(+), 26 deletions(-) diff --git a/cmd/nomos/store.go b/cmd/nomos/store.go index b6c515b..7c1bbd6 100644 --- a/cmd/nomos/store.go +++ b/cmd/nomos/store.go @@ -298,11 +298,21 @@ type planStepInput struct { TargetSlug string } -// proposePlan replaces the task's plan with a fresh ordered step list and moves -// the task into executing. v1 does a clean replace (delete + insert): revising a -// plan mid-flight starts a new list rather than versioning the old one. Emits -// plan.proposed with the persisted steps (seq + id) so the panel can render and -// later address them by id. +// proposePlan sets the task's plan and moves it into executing. Emits +// plan.proposed with the persisted steps (seq + id) so the panel can render +// and later address them by id. +// +// Two modes, chosen by whether any existing step has left 'pending': +// - Fresh/revise (no step started yet): full replace (delete + insert). This +// covers the first call, and a genuine re-plan before any work began. +// - Mid-flight (some step is running/done/failed/…): APPEND the new steps +// after the current max seq instead of wiping. The model is instructed to +// propose the whole plan in one call, but nothing stops it from calling +// propose_plan again per-step as it goes — a destructive replace in that +// case would erase every already-completed step, leaving the operator +// seeing only the most recent single step ("1/1") instead of real +// progress. Appending makes the panel's step history correct regardless +// of how the model chooses to call the tool. func (s *store) proposePlan(ctx context.Context, sessionID string, steps []planStepInput) ([]map[string]any, error) { if s == nil || sessionID == "" || sessionID == "ephemeral" { return nil, nil @@ -313,24 +323,36 @@ func (s *store) proposePlan(ctx context.Context, sessionID string, steps []planS } defer tx.Rollback(ctx) - if _, err := tx.Exec(ctx, `DELETE FROM session_plan_steps WHERE session_id = $1`, sessionID); err != nil { + var startSeq int + var anyStarted bool + if err := tx.QueryRow(ctx, ` + SELECT COALESCE(max(seq), 0), COALESCE(bool_or(status <> 'pending'), false) + FROM session_plan_steps WHERE session_id = $1`, sessionID).Scan(&startSeq, &anyStarted); err != nil { return nil, err } + if !anyStarted { + if _, err := tx.Exec(ctx, `DELETE FROM session_plan_steps WHERE session_id = $1`, sessionID); err != nil { + return nil, err + } + startSeq = 0 + } + out := make([]map[string]any, 0, len(steps)) for i, st := range steps { var targetSlug *string if st.TargetSlug != "" { targetSlug = &st.TargetSlug } + seq := startSeq + i + 1 var id uuid.UUID if err := tx.QueryRow(ctx, ` INSERT INTO session_plan_steps (session_id, seq, title, detail, target_slug) VALUES ($1, $2, $3, $4, $5) RETURNING id`, - sessionID, i+1, st.Title, st.Detail, targetSlug).Scan(&id); err != nil { + sessionID, seq, st.Title, st.Detail, targetSlug).Scan(&id); err != nil { return nil, err } out = append(out, map[string]any{ - "id": id.String(), "seq": i + 1, "title": st.Title, + "id": id.String(), "seq": seq, "title": st.Title, "detail": st.Detail, "target_slug": st.TargetSlug, }) } @@ -342,8 +364,10 @@ func (s *store) proposePlan(ctx context.Context, sessionID string, steps []planS return nil, err } // Event after commit so subscribers only ever see a persisted plan. + // appended=true tells the panel to add these steps to its existing list + // rather than replace it (mirrors the mid-flight append above). _ = observability.Event(ctx, sqlcgen.New(s.pool), "plan.proposed", s.taskEntityPtr(ctx, sessionID), - "info", "nomos", sessionID, map[string]any{"steps": out}) + "info", "nomos", sessionID, map[string]any{"steps": out, "appended": anyStarted}) return out, nil } diff --git a/cmd/nomos/tasks.go b/cmd/nomos/tasks.go index 926d87e..30c07cf 100644 --- a/cmd/nomos/tasks.go +++ b/cmd/nomos/tasks.go @@ -33,11 +33,15 @@ func taskToolDefs() []toolDef { }, { Name: "propose_plan", - Description: "Lay out the ordered steps you'll take to reach the goal. The " + - "operator sees these in the context panel and watches them progress. " + - "Call this before you start executing (after gathering what you need); " + - "re-call it to revise the plan. As you work, call update_plan_step to " + - "advance each one.", + Description: "Lay out ALL the ordered steps you'll take to reach the goal, in ONE " + + "call, listing every step end-to-end — not just the next one. The operator " + + "sees the full list in the context panel and watches it progress; a plan " + + "with only 1 step looks broken to them even if you intend to add more later. " + + "Call this ONCE, before you start executing (after gathering what you need). " + + "As you work, call update_plan_step (not propose_plan again) to advance each " + + "step. Only re-call propose_plan if the plan itself has fundamentally changed " + + "(e.g. a new approach is needed) — in that case new steps are appended after " + + "whatever already ran, never erasing completed work.", InputSchema: map[string]any{ "type": "object", "properties": map[string]any{ diff --git a/nomos/SOUL.md b/nomos/SOUL.md index d922f96..a212e0b 100644 --- a/nomos/SOUL.md +++ b/nomos/SOUL.md @@ -57,12 +57,18 @@ Each conversation is a **task**: a goal the operator wants achieved, from approach, or a failure to avoid. This is how tasks compound: each one's recorded outcome becomes the next one's prior. Don't skip it and rediscover a known problem. -2. **Plan, then execute.** Gather what you need, propose a plan, get the single - approval, and carry it out end-to-end (see the plan/approval sections below). - If you hit a genuine decision only the operator can make — an ambiguous - target, a trade-off, missing information — call `ask_operator` with the - options and the entities involved, then STOP and wait; their answer resumes - you. Don't ask about things you can settle yourself with tools. +2. **Plan, then execute.** Gather what you need, then call `propose_plan` ONCE + with the COMPLETE ordered list of every step end-to-end — not one call per + step. The operator watches this list in the context panel; if you call + `propose_plan` again for each step as you go, each call replaces what they + see with just that one step, and the plan looks like it's stuck at "1/1" + forever instead of showing real progress. Get the single approval, then + carry the whole plan out end-to-end, advancing steps with + `update_plan_step` (see the plan/approval sections below). If you hit a + genuine decision only the operator can make — an ambiguous target, a + trade-off, missing information — call `ask_operator` with the options and + the entities involved, then STOP and wait; their answer resumes you. Don't + ask about things you can settle yourself with tools. 3. **Finish explicitly with `complete_task`.** When the goal is verified done — or you've genuinely failed or only partially succeeded — call `complete_task` with the `outcome` (success/failure/partial) and a one-line `summary`. This diff --git a/web/src/lib/stores/workspace.ts b/web/src/lib/stores/workspace.ts index 1fc8dc7..3157c12 100644 --- a/web/src/lib/stores/workspace.ts +++ b/web/src/lib/stores/workspace.ts @@ -101,12 +101,15 @@ function applyEvent(ev: { type: string; correlation_id?: string | null; data?: u switch (ev.type) { case 'plan.proposed': if (Array.isArray(data.steps)) { - planSteps.set( - data.steps.map((s: any) => ({ - id: s.id, seq: s.seq, title: s.title, detail: s.detail ?? '', - status: 'pending', target_slug: s.target_slug || undefined - })) - ) + const incoming = data.steps.map((s: any) => ({ + id: s.id, seq: s.seq, title: s.title, detail: s.detail ?? '', + status: 'pending' as const, target_slug: s.target_slug || undefined + })) + // The server appends rather than replaces once any step has started + // (see store.go proposePlan) — mirror that here so a model that calls + // propose_plan once per step still shows the FULL running history in + // the panel, not just its latest call's single step. + planSteps.update((existing) => (data.appended ? [...existing, ...incoming] : incoming)) } break case 'plan.step.started':