feat(agent): close all post-fix remainders + golden eval harness (F.1-F.2, C.1-C.2, B.4-B.6, E.1-E.2)
Ships the 9 remaining post-fix items and a golden-conversation eval harness that validates them against the live agent. All 4 evals pass. SOUL.md (F.1, C.2, E.1): - Consolidated three overlapping task-flow sections (MANDATORY TASK FLOW, 'Every chat is a task', 'AFTER EVERY TASK: WRITE BACK') into one. ~50 lines shorter. The operator's 'be more crisp' feedback. - Added anti-patterns: don't re-execute on UI/sidebar complaints (C.2); don't re-run fleet-wide audits when same-day knowledge exists (E.1). - Updated approval vocabulary in step 4 to match tasks.go (approved/yes/ go/proceed/continue/ok/go ahead). Tool-result strings (F.2): - set_goal: tightened to 'Goal set. NEXT: pre-plan (read-only tools only). Then propose_plan. Do not call run.' - update_plan_step: added '(Advance with update_plan_step + run; do not re-propose.)' C.1 — completeTask rejects re-completion of a terminal session: - Returns errTaskAlreadyComplete when status is already done/failed. - The tool result directs: 'Task is already complete. Do not call complete_task again. If the operator pointed out a UI/sidebar inconsistency, fix it with update_plan_step...' B.4 — Surface real model error text: - chatWith's error event now includes finish_reason + refusal text: 'Nomos returned an empty or unusable response (finish_reason=length). Retry or rephrase.' instead of generic 'empty response'. - The resume-failed note already carried errText (B.3), which now has the real context. B.5 — Back off between resume retries (4s, 8s): - resumeSession now sleeps before attempts 1 and 2 (exponential backoff). A transient provider issue gets time to clear instead of 3 identical calls in 3 seconds. B.6 — Don't persist the empty placeholder as a visible bubble: - If a chat turn ends with no text and no tool calls (model empty-response'd and all retries failed), delete the placeholder row instead of persisting an empty bubble. The error was already streamed via done+error=true. E.2 — list_lxcs last-audited hint: - The list_lxcs result now includes last_audited_at — the most recent knowledge entry (tagged audit/update, or titled audit/update) linked via an 'about' edge. The agent can see 'nextcloud — last audited today' and skip re-running it. Tool-call doubling bug fix (found by the eval harness): - main.go + continue.go: the tool_use and tool_result events were both appending separate entries to the persisted tool_calls array, doubling every tool call in the transcript. Confirmed pre-existing (d9cdcee1, v0.3.x era). Fixed: tool_use creates the entry, tool_result merges the result into the same entry (matched by id). One entry per tool call. Golden eval harness (cmd/nomos/eval/): - A standalone Go program that loads YAML manifests of golden conversations + assertions, sends prompts to the chat endpoint, drains the SSE stream (keeping the agent's context alive), and scores structural assertions against the persisted transcript. - 4 golden conversations covering: trivial read-only (degenerate case), plan + proceed (the original duplication bug), UI complaint (no re-exec), fleet audit (knowledge preferred over re-execution). - Structural assertions only (tool-call sequences, plan steps, writeback, completion) — text quality is model-dependent and not scored. - Run: go run ./cmd/nomos/eval -gateway http://localhost:8092 -manifest cmd/nomos/eval/evals/*.yaml (~$0.10/run in OpenRouter credits). Eval results (4/4 passed): trivial_readonly: 2 tool calls, no plan, no run plan_advances_on_proceed: 13 tool calls, propose_plan x1, writes back ui_complaint_no_rerun: 12 tool calls, propose_plan x1, writes back knowledge_preferred_over_rerun: 7 tool calls, search_knowledge x1, 0 run Version 0.5.2 -> 0.5.3 (minor: eval harness + structural hardening).
This commit is contained in:
@@ -158,6 +158,18 @@ func (s *store) updateMessage(ctx context.Context, id uuid.UUID, content json.Ra
|
||||
return err
|
||||
}
|
||||
|
||||
// deleteMessage removes a message row. Used by B.6: when a chat turn ends
|
||||
// with no text and no tool calls (the model empty-response'd and all
|
||||
// retries failed), the placeholder row is deleted instead of persisting an
|
||||
// empty assistant bubble — the error was already streamed to the frontend
|
||||
// via the 'done with error=true' event, so the operator sees it inline.
|
||||
func (s *store) deleteMessage(ctx context.Context, id uuid.UUID) {
|
||||
if s == nil || id == uuid.Nil {
|
||||
return
|
||||
}
|
||||
s.pool.Exec(ctx, `DELETE FROM agent_messages WHERE id = $1`, id)
|
||||
}
|
||||
|
||||
// lastUserMessage returns the most recent user message text for a session,
|
||||
// or "" if none. Used to build a context-rich reconnect/resume note: instead
|
||||
// of a generic "report your state," the note can say "the operator's last
|
||||
@@ -647,15 +659,38 @@ func (s *store) updatePlanStep(ctx context.Context, sessionID string, seq int, s
|
||||
return nil
|
||||
}
|
||||
|
||||
// errTaskAlreadyComplete is returned by completeTask when the session is
|
||||
// already in a terminal state (done/failed/partial). The agent sometimes
|
||||
// re-calls complete_task after a UI clarification (operator-reported
|
||||
// 2026-07-14) — without this guard, the re-completion produces duplicate
|
||||
// knowledge entries and erodes audit-log clarity. The caller translates this
|
||||
// into a directive tool result.
|
||||
var errTaskAlreadyComplete = errors.New("task already complete")
|
||||
|
||||
// completeTask sets a task's terminal state, outcome, and one-line summary,
|
||||
// mirrors the outcome onto the task entity's attributes (so the board/graph
|
||||
// show it), and publishes task.status for the live context panel. outcome is
|
||||
// success|failure|partial; status is derived (failure → failed, else done).
|
||||
// Returns errTaskAlreadyComplete if the session is already terminal — the
|
||||
// agent must not re-complete a finished task.
|
||||
func (s *store) completeTask(ctx context.Context, sessionID, outcome, summary string) error {
|
||||
if s == nil || sessionID == "" || sessionID == "ephemeral" {
|
||||
return nil
|
||||
}
|
||||
|
||||
// C.1: reject re-completion of an already-terminal session. The agent
|
||||
// sometimes re-calls complete_task after a UI clarification ("the sidebar
|
||||
// differs") — without this guard, the re-completion duplicates knowledge
|
||||
// entries and produces a confusing audit trail.
|
||||
var currentStatus string
|
||||
if err := s.pool.QueryRow(ctx,
|
||||
`SELECT status FROM agent_sessions WHERE id = $1`, sessionID).Scan(¤tStatus); err != nil {
|
||||
// Session doesn't exist or query failed — let the rest of the
|
||||
// function proceed; it'll fail safely on the UPDATE below.
|
||||
} else if currentStatus == "done" || currentStatus == "failed" {
|
||||
return errTaskAlreadyComplete
|
||||
}
|
||||
|
||||
// Auto-cancel any executions still in pending_approval/approved/queued
|
||||
// state for this session — preventing orphaned approvals (observed in
|
||||
// production: 4 approvals left open after session completed).
|
||||
|
||||
Reference in New Issue
Block a user