fix: the actual root bug — assent-window auto-approve never dispatched work at all

The previous commit fixed a context-cancellation bug in the auto-approve path
and appeared to fix things, but re-testing end-to-end after deploy showed the
execution STILL never completed — just via a different symptom
("no pending execution found for approval" in the logs). Dug further and
found the real, deeper bug underneath: this whole mechanism has never
actually worked.

autoApprove() directly flipped BOTH approvals.status and executions.status to
'approved' via raw SQL, then called executeApprovedViaAPI to POST to the
decision endpoint. But DecideApproval's own logic specifically looks for the
execution still at status='pending_approval' to find and dispatch the real
SSH work (executeApprovedAction) — autoApprove's premature flip meant that
lookup always found zero rows. DecideApproval's UpdateApprovalStatus call
also silently no-ops the same way (sqlc :exec doesn't surface "0 rows
affected" as an error). Every assent-window auto-approved pct_create/
apt_upgrade has been sitting at 'approved' forever with the real work never
triggered — indistinguishable from "still running" until you check.

Fix: remove autoApprove() entirely. Call executeApprovedViaAPI directly
against the untouched pending_approval row from createApproval — identical
to the manual Approve-button path, just without the human click. DecideApproval
is now the single place that transitions status and dispatches, for both the
manual and auto-approved paths, closing the class of bug where two code paths
raced to do the same state transition.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

internal/mcp/server.go

@@ -403,24 +403,29 @@ func newServer(pool *db.Pool, agentID uuid.UUID) *mcp.Server {
 			if assentWindowActive(ctx, pool, agentID) {
 				pool.Exec(ctx, `UPDATE executions SET status='pending_approval', risk_class='config_mutation' WHERE entity_id=$1`, id)
 				createApproval(ctx, pool, id, targetID, "apt_upgrade", params, "config_mutation")
-				if approved := autoApprove(ctx, pool, id); approved {
+				// Do NOT pre-flip approvals/executions status here (that was
-					// context.Background(), NOT ctx: ctx is scoped to this
+				// the previous, broken "autoApprove" helper). DecideApproval
-					// MCP tool call, which ends (and cancels) as soon as the
+				// (invoked below) is the ONE place that transitions
-					// chat turn's HTTP response completes — normal, expected,
+				// pending_approval -> approved and dispatches the real SSH
-					// happens on every turn. A goroutine meant to outlive the
+				// work — it specifically looks for status='pending_approval'
-					// request must not inherit its context, or the async work
+				// to find what to run. Pre-flipping the status past that
-					// dies silently the instant the turn ends. Found live:
+				// state meant DecideApproval's own lookup found nothing,
-					// every assent-window auto-approved pct_create failed
+				// silently no-opped, and the execution sat at 'approved'
-					// with "context canceled" the moment the triggering
+				// forever with nothing actually running. Found live: every
-					// /chat request finished — exactly the same context-
+				// assent-window auto-approved pct_create/apt_upgrade has
-					// lifetime class of bug, in the one place it had been
+				// never actually executed, via this exact bug. Calling
-					// missed (httpapi's own approval goroutine already used
+				// executeApprovedViaAPI directly against the untouched
-					// context.Background() correctly).
+				// pending_approval row makes this identical to the manual
+				// Approve-button path, just without a human click.
+				//
+				// context.Background(), NOT ctx: ctx is scoped to this MCP
+				// tool call, cancelled the instant the chat turn's HTTP
+				// response completes (every normal turn) — a goroutine
+				// meant to outlive the request must not inherit its context.
 				go executeApprovedViaAPI(context.Background(), id, targetSlug, "apt_upgrade:"+params)
 				slog.Info("mcp: apt_upgrade auto-approved via assent window", "execution_id", id)
 				return textResult(fmt.Sprintf("apt_upgrade on %s auto-approved via assent window — execution %s running.", targetSlug, id)), nil
 			}
-			}
 			// upgrade requires approval — queue
 			pool.Exec(ctx, `UPDATE executions SET status='pending_approval', risk_class='config_mutation' WHERE entity_id=$1`, id)
 			createApproval(ctx, pool, id, targetID, "apt_upgrade", params, "config_mutation")
@@ -432,12 +437,13 @@ func newServer(pool *db.Pool, agentID uuid.UUID) *mcp.Server {
 			if assentWindowActive(ctx, pool, agentID) {
 				pool.Exec(ctx, `UPDATE executions SET status='pending_approval', risk_class='config_mutation' WHERE entity_id=$1`, id)
 				createApproval(ctx, pool, id, targetID, "pct_create", params, "config_mutation")
-				if approved := autoApprove(ctx, pool, id); approved {
+				// See the apt_upgrade case above for why there's no
-					go executeApprovedViaAPI(context.Background(), id, targetSlug, "pct_create:"+params) // see context.Background() comment above (apt_upgrade case) — same bug, same fix
+				// pre-flip-status "autoApprove" step here anymore, and why
+				// this uses context.Background().
+				go executeApprovedViaAPI(context.Background(), id, targetSlug, "pct_create:"+params)
 				slog.Info("mcp: pct_create auto-approved via assent window", "execution_id", id)
 				return textResult(fmt.Sprintf("pct_create on %s auto-approved via assent window — execution %s running. The LXC is being provisioned now.", targetSlug, id)), nil
 			}
-			}
 			pool.Exec(ctx, `UPDATE executions SET status='pending_approval', risk_class='config_mutation' WHERE entity_id=$1`, id)
 			createApproval(ctx, pool, id, targetID, "pct_create", params, "config_mutation")
 			return textResult(fmt.Sprintf("pct_create on %s requires approval — execution %s queued. The LXC will be provisioned once approved.", targetSlug, id)), nil
@@ -1345,23 +1351,6 @@ func resolveExecTarget(ctx context.Context, pool *db.Pool, targetSlug string) (h
 // mirroring what DecideApproval does. Returns true on success. This is used
 // by the assent-window path to skip the operator-approval queue when the
 // operator already approved the overall plan via chat assent.
-func autoApprove(ctx context.Context, pool *db.Pool, execID uuid.UUID) bool {
-	_, err := pool.Exec(ctx, `
-		UPDATE approvals SET status='approved', decided_at=now(), decided_by=$1
-		WHERE entity_id=$2 AND status='pending'`,
-		execID, execID)
-	if err != nil {
-		slog.Error("mcp: autoApprove update approval", "error", err, "execution", execID)
-		return false
-	}
-	_, err = pool.Exec(ctx, `UPDATE executions SET status='approved' WHERE entity_id=$1`, execID)
-	if err != nil {
-		slog.Error("mcp: autoApprove update execution", "error", err, "execution", execID)
-		return false
-	}
-	return true
-}
-
 // executeApprovedViaAPI calls the HTTP API's approval-decision endpoint to
 // trigger the actual execution. The API server (phase3.executeApprovedAction)
 // handles the real SSH work (pct create, apt upgrade, etc.) in a goroutine.
@@ -1388,10 +1377,14 @@ func executeApprovedViaAPI(ctx context.Context, execID uuid.UUID, targetSlug, ac
 	}
 	defer resp.Body.Close()
 	if resp.StatusCode != http.StatusOK {
-		// The autoApprove DB update may have already marked it approved,
+		// A non-200 here means the real SSH work was never dispatched — this
-		// and the API found no pending approval to decide — that's fine,
+		// is the call that actually triggers executeApprovedAction via
-		// the execution was already triggered by the DB state change.
+		// DecideApproval. (A previous version of this comment claimed a
-		slog.Info("mcp: executeApprovedViaAPI non-200 (likely already decided)", "status", resp.StatusCode, "execution", execID)
+		// non-200 was fine because a since-removed "autoApprove" step had
+		// already triggered execution via a raw DB update — it hadn't; that
+		// was the bug where auto-approved pct_create/apt_upgrade never
+		// actually ran. There is no other path that dispatches the work.)
+		slog.Error("mcp: executeApprovedViaAPI non-200 — execution was NOT dispatched", "status", resp.StatusCode, "execution", execID)
 	}
 }