fix(agent): move plan step replacement from reopenSession to setGoal
reopenSession was replacing plan steps on every follow-up message —
including approvals ('go ahead') — which destroyed the plan the operator
just approved, leaving the agent unable to track step progress and looping
run calls until maxIterations.
Fix: setGoal is the explicit signal for 'new sub-task' (the agent calls
it at the start of each follow-up direction). Step replacement now happens
there, not in reopenSession. An approval ('go ahead') does NOT call
set_goal, so the plan stays intact and the agent can execute + complete
it.
This commit is contained in:
@@ -452,10 +452,24 @@ func (s *store) taskEntityPtr(ctx context.Context, sessionID string) *uuid.UUID
|
||||
// intermediate state was removed (2026-07-14) — it was indistinguishable from
|
||||
// `active` to the operator and caused sessions to appear stuck when the agent
|
||||
// called set_goal but never propose_plan (observed in production).
|
||||
//
|
||||
// P2 (2026-07-15): setGoal also replaces any prior plan steps (from a
|
||||
// previous sub-task or an incomplete first turn) as `replaced`, clearing the
|
||||
// way for a fresh propose_plan. This is the ONLY place step replacement
|
||||
// happens — not in reopenSession — because set_goal is the explicit signal
|
||||
// for "new sub-task." An approval ("go ahead") does NOT call set_goal, so it
|
||||
// won't destroy the plan the operator just approved.
|
||||
func (s *store) setGoal(ctx context.Context, sessionID, goal string) error {
|
||||
if s == nil || sessionID == "" || sessionID == "ephemeral" {
|
||||
return nil
|
||||
}
|
||||
// Replace any prior plan steps (done/running/pending/...) as `replaced`.
|
||||
// The rows are kept for the generation counter + audit trail; proposePlan
|
||||
// excludes `replaced` from its in-flight check, so the next propose_plan
|
||||
// takes the fresh-generation path.
|
||||
s.pool.Exec(ctx,
|
||||
`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`,
|
||||
sessionID, goal); err != nil {
|
||||
@@ -469,25 +483,17 @@ func (s *store) setGoal(ctx context.Context, sessionID, goal string) error {
|
||||
// reopenSession flips a terminal (done/failed) session back to `executing`
|
||||
// so a follow-up message can start a new sub-task — the iteration path
|
||||
// (P2, 2026-07-15). Without this, a completed session stays `done` forever
|
||||
// and propose_plan refuses the new sub-task with errPlanInFlight because the
|
||||
// prior plan's steps are all `done` (status <> 'pending'). reopenSession:
|
||||
// and the panel shows a stale result.
|
||||
//
|
||||
// 1. Marks all existing session_plan_steps as `replaced` (a status already
|
||||
// recognized by updatePlanStep's stamp switch). The rows are KEPT — the
|
||||
// generation column preserves which plan they belonged to, and the
|
||||
// audit trail survives. proposePlan's anyStarted check excludes
|
||||
// `replaced` (see proposePlan), so the next propose_plan takes the
|
||||
// fresh-generation path rather than being refused with errPlanInFlight.
|
||||
// This runs for BOTH terminal and executing sessions — a follow-up on
|
||||
// an executing session (first turn didn't complete_task) is still a
|
||||
// new direction, and the old plan's steps must not block the new one.
|
||||
// 2. If the session was terminal, clears outcome/summary and flips status
|
||||
// to `executing` so the panel doesn't show the old result.
|
||||
// 3. Stamps last_active_at.
|
||||
// reopenSession ONLY flips the status + clears outcome/summary. It does NOT
|
||||
// touch plan steps — that's `setGoal`'s job (see below). The reason: not
|
||||
// every follow-up is a new sub-task. An approval ("go ahead") is a
|
||||
// continuation of the current plan, and replacing its steps would destroy
|
||||
// the plan the operator just approved. `set_goal` is the explicit signal for
|
||||
// "new sub-task," so step replacement happens there, not here.
|
||||
//
|
||||
// Returns true if the session was actually reopened (was terminal), false if
|
||||
// it was already active (the plan steps are still replaced — the new
|
||||
// direction needs a fresh plan — but the status doesn't change).
|
||||
// it was already active (no-op).
|
||||
func (s *store) reopenSession(ctx context.Context, sessionID string) bool {
|
||||
if s == nil || sessionID == "" || sessionID == "ephemeral" {
|
||||
return false
|
||||
@@ -497,14 +503,9 @@ func (s *store) reopenSession(ctx context.Context, sessionID string) bool {
|
||||
`SELECT status FROM agent_sessions WHERE id = $1`, sessionID).Scan(¤tStatus); err != nil {
|
||||
return false
|
||||
}
|
||||
// Always replace prior plan steps — the follow-up is a new direction
|
||||
// regardless of whether the prior turn completed. Without this, an
|
||||
// executing session with done steps would block proposePlan with
|
||||
// errPlanInFlight.
|
||||
s.pool.Exec(ctx,
|
||||
`UPDATE session_plan_steps SET status = 'replaced', finished_at = COALESCE(finished_at, now()) WHERE session_id = $1 AND status <> 'replaced'`,
|
||||
sessionID)
|
||||
if currentStatus == "done" || currentStatus == "failed" {
|
||||
if currentStatus != "done" && currentStatus != "failed" {
|
||||
return false
|
||||
}
|
||||
s.pool.Exec(ctx,
|
||||
`UPDATE agent_sessions SET status = 'executing', outcome = NULL, summary = NULL, last_active_at = now() WHERE id = $1`,
|
||||
sessionID)
|
||||
@@ -512,9 +513,6 @@ func (s *store) reopenSession(ctx context.Context, sessionID string) bool {
|
||||
"info", "nomos", sessionID, map[string]any{"prior_status": currentStatus})
|
||||
return true
|
||||
}
|
||||
s.pool.Exec(ctx, `UPDATE agent_sessions SET last_active_at = now() WHERE id = $1`, sessionID)
|
||||
return false
|
||||
}
|
||||
|
||||
// planStepInput is one step as the agent proposes it.
|
||||
type planStepInput struct {
|
||||
|
||||
Reference in New Issue
Block a user