From fcc7ceb8b627572b750130a94044288030d2b4c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Almeida?= <57680069+zeval@users.noreply.github.com> Date: Fri, 1 May 2026 00:25:51 +0100 Subject: [PATCH] fix(approvals): drop stuck pending permission_request messages (#723) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(web): exclude permission_request from turn-complete safety net The session.turn.completed handler iterated over messages and forced metadata.status="complete" on anything carrying tool_call_id. Permission prompts also carry tool_call_id (for FE pairing with their tool_call), so this loop overwrote the user's "approved" decision on every prompt in the turn — and the FE component re-rendered the Approve/Deny buttons. * fix(backend): protect permission_request rows from tool_call sweeps Three permission_request invariants the prior code violated, mirroring the FE bug fix: - CompletePendingToolCallsForTurn: SQL UPDATE filtered by tool_call_id presence, also catching permission_request rows. Add type filter so the safety net only touches actual tool messages. - GetMessageByToolCallID: returned the oldest row carrying that tool_call_id, which races to the permission_request when timing shifts. Exclude permission_request type so a tool_update can never land on the user's approve/reject row. - applyToolCallMessageUpdate: defensive guard that refuses to overwrite a permission_request, making the invariant explicit at the layer that does the writing. Adds a regression test for CompletePendingToolCallsForTurn covering approved/rejected/pending permission rows. * test(e2e): cover approval-reappear regression with mock-agent scenario Adds a multi-permission mock-agent scenario (three sequential bash tools, each requiring approval) and an E2E test that approves all three and asserts no permission action row remains visible after the agent's turn ends, and that the resolved state survives a reload. Supporting test infrastructure: - data-testid hooks on PermissionActionRow. - permissionActionRows() / permissionApproveButtons() on SessionPage. - backend fixture force-disables AGENTCTL_AUTO_APPROVE_PERMISSIONS=false so a developer's local AUTO_APPROVE=true env doesn't silently bypass the permission UI in tests that exercise the approve/reject flow. * test(backend): cover GetMessageByToolCallID permission_request exclusion Addresses claude-review feedback on the approvals fix: - Adds a regression test that seeds both a permission_request and a tool_call with the same tool_call_id (permission_request created first, so an ORDER BY created_at ASC sweep without the type filter would race to it) and asserts the lookup returns the tool_call row. - Bumps the applyToolCallMessageUpdate invariant guard from Warn to Error: reaching that branch means a caller bug, not a transient condition, and Warn would silently swallow the violation. * fix(e2e): scope auto-approve override to permission-approval test only The previous fixture force-disabled AGENTCTL_AUTO_APPROVE_PERMISSIONS globally, which broke session-isolation.spec.ts and other tests that use scenarios with permission-required tools (e.g. /e2e:read-and-edit) and rely on auto-approve to let the agent finish without UI clicks. Default the env to "true" in the fixture; let permission-approval.spec flip it to "false" via a backend.restart() override in beforeAll, then restore in afterAll. Adds an envOverrides parameter to backend.restart. --- apps/backend/cmd/mock-agent/scenarios.go | 29 ++++++ .../repository/session_repository_test.go | 99 ++++++++++++++++++- .../task/repository/sqlite/message.go | 13 ++- .../internal/task/service/service_messages.go | 15 +++ .../chat/messages/permission-action-row.tsx | 7 +- apps/web/e2e/fixtures/backend.ts | 22 ++++- apps/web/e2e/pages/session-page.ts | 10 ++ .../tests/chat/permission-approval.spec.ts | 81 +++++++++++++++ apps/web/lib/ws/handlers/turns.ts | 5 + 9 files changed, 274 insertions(+), 7 deletions(-) create mode 100644 apps/web/e2e/tests/chat/permission-approval.spec.ts diff --git a/apps/backend/cmd/mock-agent/scenarios.go b/apps/backend/cmd/mock-agent/scenarios.go index bdfc3ffb0d..067a4ec784 100644 --- a/apps/backend/cmd/mock-agent/scenarios.go +++ b/apps/backend/cmd/mock-agent/scenarios.go @@ -29,6 +29,7 @@ var scenarioRegistry = map[string]func(e *emitter){ "untracked-file-modify": scenarioUntrackedFileModify, "clarification": scenarioClarification, "clarification-timeout": scenarioClarificationTimeout, + "multi-permission": scenarioMultiPermission, "review-cumulative-setup": scenarioReviewCumulativeSetup, "symlink-file-setup": scenarioSymlinkFileSetup, } @@ -96,6 +97,34 @@ func scenarioReadAndEdit(e *emitter) { e.text("Read and edit scenario complete.") } +// scenarioMultiPermission: three sequential bash tools, each requiring permission. +// Reproduces the "approvals reappear at turn complete" bug — user approves all +// three, then when the turn ends the approval prompts must NOT come back. +func scenarioMultiPermission(e *emitter) { + fixedDelay(50) + e.text("Running three commands that need approval.") + + for i, label := range []string{"first", "second", "third"} { + fixedDelay(50) + id := nextToolID() + input := map[string]any{ + "command": fmt.Sprintf("echo %s", label), + "description": fmt.Sprintf("Step %d: %s", i+1, label), + } + e.startTool(id, fmt.Sprintf("Run %s command", label), acp.ToolKindExecute, input) + allowed := e.requestPermission(id, fmt.Sprintf("Run %s command", label), acp.ToolKindExecute, input) + fixedDelay(50) + if allowed { + e.completeTool(id, map[string]any{"output": label}) + } else { + e.completeTool(id, map[string]any{"output": "denied"}) + } + } + + fixedDelay(50) + e.text("Multi-permission scenario complete.") +} + // scenarioPermissionFlow: tool requiring permission with fixed delays. func scenarioPermissionFlow(e *emitter) { fixedDelay(50) diff --git a/apps/backend/internal/task/repository/session_repository_test.go b/apps/backend/internal/task/repository/session_repository_test.go index 741039f5a8..6a4aafa36b 100644 --- a/apps/backend/internal/task/repository/session_repository_test.go +++ b/apps/backend/internal/task/repository/session_repository_test.go @@ -372,8 +372,29 @@ func TestSQLiteRepository_CompletePendingToolCallsForTurn(t *testing.T) { Type: models.MessageTypeToolCall, Metadata: map[string]interface{}{"tool_call_id": "tc-6", "status": "error"}, } + // Permission_request messages share `tool_call_id` in metadata but their `status` + // is the user's approve/reject decision, not the tool call state. The sweep must + // leave them alone — otherwise an "approved" prompt would be reset and re-shown. + approvedPermission := &models.Message{ + ID: "msg-perm-approved-1", TaskSessionID: sessionID, TaskID: task.ID, TurnID: turnID, + AuthorType: models.MessageAuthorAgent, Content: "Approve this?", + Type: models.MessageTypePermissionRequest, + Metadata: map[string]interface{}{"tool_call_id": "tc-7", "pending_id": "p-1", "status": "approved"}, + } + rejectedPermission := &models.Message{ + ID: "msg-perm-rejected-1", TaskSessionID: sessionID, TaskID: task.ID, TurnID: turnID, + AuthorType: models.MessageAuthorAgent, Content: "Approve this?", + Type: models.MessageTypePermissionRequest, + Metadata: map[string]interface{}{"tool_call_id": "tc-8", "pending_id": "p-2", "status": "rejected"}, + } + pendingPermission := &models.Message{ + ID: "msg-perm-pending-1", TaskSessionID: sessionID, TaskID: task.ID, TurnID: turnID, + AuthorType: models.MessageAuthorAgent, Content: "Approve this?", + Type: models.MessageTypePermissionRequest, + Metadata: map[string]interface{}{"tool_call_id": "tc-9", "pending_id": "p-3", "status": "pending"}, + } - for _, msg := range []*models.Message{runningTool, completeTool, regularMsg, runningTool2, pendingTool, inProgressTool, errorTool} { + for _, msg := range []*models.Message{runningTool, completeTool, regularMsg, runningTool2, pendingTool, inProgressTool, errorTool, approvedPermission, rejectedPermission, pendingPermission} { if err := repo.CreateMessage(ctx, msg); err != nil { t.Fatalf("failed to create message %s: %v", msg.ID, err) } @@ -385,7 +406,8 @@ func TestSQLiteRepository_CompletePendingToolCallsForTurn(t *testing.T) { t.Fatalf("CompletePendingToolCallsForTurn failed: %v", err) } - // Should have updated 4 non-terminal tool call messages (running x2, pending, in_progress) + // Should have updated 4 non-terminal tool call messages (running x2, pending, in_progress). + // Permission_request messages must be excluded even though they carry tool_call_id. if affected != 4 { t.Errorf("expected 4 affected rows, got %d", affected) } @@ -430,6 +452,21 @@ func TestSQLiteRepository_CompletePendingToolCallsForTurn(t *testing.T) { t.Errorf("expected msg-error-1 status 'error' (unchanged), got %v", msg7.Metadata["status"]) } + // Verify permission_request messages were NOT affected — their status carries the + // user's decision (approve/reject) which the sweep must never overwrite. + msgPermApproved, _ := repo.GetMessage(ctx, "msg-perm-approved-1") + if msgPermApproved.Metadata["status"] != "approved" { + t.Errorf("expected msg-perm-approved-1 status 'approved' (unchanged), got %v", msgPermApproved.Metadata["status"]) + } + msgPermRejected, _ := repo.GetMessage(ctx, "msg-perm-rejected-1") + if msgPermRejected.Metadata["status"] != "rejected" { + t.Errorf("expected msg-perm-rejected-1 status 'rejected' (unchanged), got %v", msgPermRejected.Metadata["status"]) + } + msgPermPending, _ := repo.GetMessage(ctx, "msg-perm-pending-1") + if msgPermPending.Metadata["status"] != "pending" { + t.Errorf("expected msg-perm-pending-1 status 'pending' (unchanged), got %v", msgPermPending.Metadata["status"]) + } + // Running again should affect 0 rows affected2, err := repo.CompletePendingToolCallsForTurn(ctx, turnID) if err != nil { @@ -440,6 +477,64 @@ func TestSQLiteRepository_CompletePendingToolCallsForTurn(t *testing.T) { } } +// TestSQLiteRepository_GetMessageByToolCallID_ExcludesPermissionRequest locks +// in the second half of the approval-reappear fix: a tool_call message and a +// permission_request message can both carry the same tool_call_id (the +// permission_request stores it for FE pairing). The lookup must always return +// the tool_call row — otherwise a downstream tool_update would land on the +// permission_request and overwrite the user's approve/reject decision. +func TestSQLiteRepository_GetMessageByToolCallID_ExcludesPermissionRequest(t *testing.T) { + repo, cleanup := createTestSQLiteRepo(t) + defer cleanup() + ctx := context.Background() + + workflow := &models.Workflow{ID: "wf-1", Name: "Test Workflow"} + _ = repo.CreateWorkflow(ctx, workflow) + task := &models.Task{ID: "task-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", Title: "Test Task"} + _ = repo.CreateTask(ctx, task) + sessionID := setupSQLiteTestSession(ctx, repo, task.ID, "session-1") + turnID := setupSQLiteTestTurn(ctx, repo, sessionID, task.ID, "turn-1") + + // Permission_request created FIRST so an ORDER BY created_at ASC sweep + // without the type filter would race to it. The fix must ignore type + // ordering and always return the tool_call row. + permMsg := &models.Message{ + ID: "msg-perm-1", TaskSessionID: sessionID, TaskID: task.ID, TurnID: turnID, + AuthorType: models.MessageAuthorAgent, Content: "Approve?", + Type: models.MessageTypePermissionRequest, + Metadata: map[string]interface{}{"tool_call_id": "tc-1", "pending_id": "p-1", "status": "pending"}, + } + if err := repo.CreateMessage(ctx, permMsg); err != nil { + t.Fatalf("create permission message: %v", err) + } + toolMsg := &models.Message{ + ID: "msg-tool-1", TaskSessionID: sessionID, TaskID: task.ID, TurnID: turnID, + AuthorType: models.MessageAuthorAgent, Content: "Tool", + Type: models.MessageTypeToolCall, + Metadata: map[string]interface{}{"tool_call_id": "tc-1", "status": "running"}, + } + if err := repo.CreateMessage(ctx, toolMsg); err != nil { + t.Fatalf("create tool_call message: %v", err) + } + + got, err := repo.GetMessageByToolCallID(ctx, sessionID, "tc-1") + if err != nil { + t.Fatalf("GetMessageByToolCallID: %v", err) + } + if got.Type != models.MessageTypeToolCall { + t.Errorf("expected tool_call row, got type=%s id=%s", got.Type, got.ID) + } + if got.ID != toolMsg.ID { + t.Errorf("expected msg-tool-1, got %s", got.ID) + } + + // And when only the permission_request exists, the lookup must return an + // error (not match the permission_request as a fallback). + if _, err := repo.GetMessageByToolCallID(ctx, sessionID, "tc-not-here"); err == nil { + t.Error("expected error for missing tool_call_id, got nil") + } +} + // TestGetPrimarySessionInfoByTaskIDs_PopulatesID locks in the SQL fix: the // SELECT now includes ts.id so session.ID is populated on the returned // TaskSession. Before the fix, publishTaskEvent saw sessionInfo.ID == "" and diff --git a/apps/backend/internal/task/repository/sqlite/message.go b/apps/backend/internal/task/repository/sqlite/message.go index 815ebc1a3e..77db8a51db 100644 --- a/apps/backend/internal/task/repository/sqlite/message.go +++ b/apps/backend/internal/task/repository/sqlite/message.go @@ -297,8 +297,14 @@ func (r *Repository) getMessageByMetadataField(ctx context.Context, sessionID, f // GetMessageByToolCallID retrieves a tool message by session ID and tool_call_id in metadata. // Searches all message types that have a tool_call_id in metadata (tool_call, tool_read, tool_edit, // tool_execute, tool_search, todo, etc.). +// +// Permission_request messages also store tool_call_id but represent the user-approval card, +// not the tool call itself. Excluded so a tool_update never lands on a permission_request +// row — that would overwrite metadata.status (the user's approve/reject) and retype it to +// tool_execute, making the prompt buttons reappear after the turn ends. func (r *Repository) GetMessageByToolCallID(ctx context.Context, sessionID, toolCallID string) (*models.Message, error) { - return r.getMessageByMetadataField(ctx, sessionID, "tool_call_id", toolCallID, "ORDER BY created_at ASC LIMIT 1") + return r.getMessageByMetadataField(ctx, sessionID, "tool_call_id", toolCallID, + "AND type != 'permission_request' ORDER BY created_at ASC LIMIT 1") } // GetMessageByPendingID retrieves a message by session ID and pending_id in metadata @@ -335,12 +341,17 @@ func (r *Repository) FindMessageByPendingID(ctx context.Context, pendingID strin // CompletePendingToolCallsForTurn marks all non-terminal tool call messages for a turn as "complete". // This is a safety net to ensure no tool calls remain stuck in a non-terminal state (pending, // running, in_progress, etc.) after a turn completes. +// +// Excludes permission_request messages: they share `tool_call_id` in metadata but their +// `status` is the user's approve/reject decision, not the tool call state. Forcing them to +// "complete" wipes "approved"/"rejected" and re-shows the prompt buttons in the UI. func (r *Repository) CompletePendingToolCallsForTurn(ctx context.Context, turnID string) (int64, error) { drv := r.db.DriverName() query := fmt.Sprintf(` UPDATE task_session_messages SET metadata = %s WHERE turn_id = ? + AND type != 'permission_request' AND %s NOT IN ('complete', 'error') AND %s `, dialect.JSONSet(drv, "metadata", "status", "complete"), diff --git a/apps/backend/internal/task/service/service_messages.go b/apps/backend/internal/task/service/service_messages.go index 14d49e753b..8be14b9867 100644 --- a/apps/backend/internal/task/service/service_messages.go +++ b/apps/backend/internal/task/service/service_messages.go @@ -462,7 +462,22 @@ func (s *Service) createToolCallMessageFallback(ctx context.Context, sessionID, } // applyToolCallMessageUpdate applies status, result, normalized data, and title to a tool call message. +// +// Defensive guard: a permission_request row also carries `tool_call_id` in metadata, so +// any future code path that hands such a row in here would otherwise silently overwrite +// the user's approve/reject decision and retype it to tool_execute. The repository's +// GetMessageByToolCallID excludes permission_request, but the guard makes the invariant +// explicit at the layer that does the writing. func (s *Service) applyToolCallMessageUpdate(message *models.Message, status, result, title string, normalized *streams.NormalizedPayload) { + if message.Type == models.MessageTypePermissionRequest { + // Error severity: the repo-layer GetMessageByToolCallID filter is supposed + // to make this branch unreachable. Reaching it means a caller bug; surface + // it loudly so the invariant violation isn't silently swallowed. + s.logger.Error("applyToolCallMessageUpdate refusing to overwrite permission_request", + zap.String("message_id", message.ID), + zap.String("incoming_status", status)) + return + } if message.Metadata == nil { message.Metadata = make(map[string]interface{}) } diff --git a/apps/web/components/task/chat/messages/permission-action-row.tsx b/apps/web/components/task/chat/messages/permission-action-row.tsx index ee3b60501c..7236562e26 100644 --- a/apps/web/components/task/chat/messages/permission-action-row.tsx +++ b/apps/web/components/task/chat/messages/permission-action-row.tsx @@ -17,7 +17,10 @@ export const PermissionActionRow = memo(function PermissionActionRow({ isResponding = false, }: PermissionActionRowProps) { return ( -
+
Approve this action? @@ -26,6 +29,7 @@ export const PermissionActionRow = memo(function PermissionActionRow({ variant="outline" onClick={onReject} disabled={isResponding} + data-testid="permission-reject" className="h-6 px-3 text-foreground border-border bg-background hover:bg-muted hover:border-foreground/40 transition-colors cursor-pointer" > @@ -36,6 +40,7 @@ export const PermissionActionRow = memo(function PermissionActionRow({ variant="outline" onClick={onApprove} disabled={isResponding} + data-testid="permission-approve" className="h-6 px-3 text-foreground border-border bg-background hover:bg-muted hover:border-foreground/40 transition-colors cursor-pointer" > {isResponding ? ( diff --git a/apps/web/e2e/fixtures/backend.ts b/apps/web/e2e/fixtures/backend.ts index 2cc2c7b56f..e0c97e4889 100644 --- a/apps/web/e2e/fixtures/backend.ts +++ b/apps/web/e2e/fixtures/backend.ts @@ -28,8 +28,14 @@ export type BackendContext = { frontendPort: number; frontendUrl: string; tmpDir: string; - /** Kill the backend process and respawn with the same config (DB, ports, tmpDir persist). */ - restart: () => Promise; + /** + * Kill the backend process and respawn with the same config (DB, ports, + * tmpDir persist). Optional `envOverrides` mutates the captured env *for the + * lifetime of the worker*; pass them again on a subsequent restart to + * revert. Only in-memory execution state (running agents, WS connections) + * is lost. + */ + restart: (envOverrides?: Record) => Promise; }; async function waitForHealth(url: string, timeoutMs: number, proc?: ChildProcess): Promise { @@ -243,6 +249,11 @@ exec git "$@" KANDEV_LOG_LEVEL: process.env.KANDEV_LOG_LEVEL ?? "warn", AGENTCTL_INSTANCE_PORT_BASE: String(agentctlPortBase), AGENTCTL_INSTANCE_PORT_MAX: String(agentctlPortMax), + // Most E2E tests rely on auto-approve being on so tools that require + // permission (Edit, Bash) just complete without UI interaction. Tests + // that exercise the approve/reject flow itself opt in by spawning their + // own backend with this env var set to "false" — see permission-approval.spec.ts. + AGENTCTL_AUTO_APPROVE_PERMISSIONS: process.env.AGENTCTL_AUTO_APPROVE_PERMISSIONS ?? "true", // Short window makes coalesce-boundary tests practical without wall-clock waits. KANDEV_PLAN_COALESCE_WINDOW_MS: process.env.KANDEV_PLAN_COALESCE_WINDOW_MS ?? "2000", GIT_AUTHOR_NAME: "E2E Test", @@ -296,7 +307,12 @@ exec git "$@" * SQLite DB, tmpDir, and all persisted data survive the restart. * Only in-memory execution state (running agents, WS connections) is lost. */ - const restart = async () => { + const restart = async (envOverrides?: Record) => { + if (envOverrides) { + for (const [k, v] of Object.entries(envOverrides)) { + (backendEnv as Record)[k] = v; + } + } await killProcessGroup(backendProc); // Wait for OS to release the TCP port — Linux TIME_WAIT can exceed 500ms under load await new Promise((r) => setTimeout(r, 2_000)); diff --git a/apps/web/e2e/pages/session-page.ts b/apps/web/e2e/pages/session-page.ts index 3ca0eef77e..7a5293425c 100644 --- a/apps/web/e2e/pages/session-page.ts +++ b/apps/web/e2e/pages/session-page.ts @@ -242,6 +242,16 @@ export class SessionPage { return this.clarificationOverlay().getByTestId("clarification-option-description"); } + /** All visible "Approve / Deny" rows for pending permission requests. */ + permissionActionRows(): Locator { + return this.chat.getByTestId("permission-action-row"); + } + + /** All "Approve" buttons for pending permission requests. */ + permissionApproveButtons(): Locator { + return this.chat.getByTestId("permission-approve"); + } + /** Reset context button in the chat input toolbar. */ resetContextButton(): Locator { return this.page.getByTestId("reset-context-button"); diff --git a/apps/web/e2e/tests/chat/permission-approval.spec.ts b/apps/web/e2e/tests/chat/permission-approval.spec.ts new file mode 100644 index 0000000000..620482d259 --- /dev/null +++ b/apps/web/e2e/tests/chat/permission-approval.spec.ts @@ -0,0 +1,81 @@ +import { type Page } from "@playwright/test"; +import { test, expect } from "../../fixtures/test-base"; +import type { SeedData } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; +import { SessionPage } from "../../pages/session-page"; + +/** + * Seed a task that runs the multi-permission scenario, then navigate to it. + * The mock agent will request three permissions in sequence and block on each. + */ +async function seedMultiPermissionTask( + testPage: Page, + apiClient: ApiClient, + seedData: SeedData, +): Promise { + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Multi-permission approval", + seedData.agentProfileId, + { + description: "/e2e:multi-permission", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + if (!task.session_id) throw new Error("createTaskWithAgent did not return a session_id"); + + await testPage.goto(`/t/${task.id}`); + + const session = new SessionPage(testPage); + await session.waitForLoad(); + + return session; +} + +test.describe("Permission approval persistence", () => { + test.describe.configure({ retries: 1 }); + + // Most E2E tests run with auto-approve on so tools-needing-permission scenarios + // (e.g. /e2e:read-and-edit) don't block the agent. This test specifically + // exercises the approve UI, so it needs auto-approve OFF — restart the worker + // backend with the override before this file's tests, then restore. + test.beforeAll(async ({ backend }) => { + await backend.restart({ AGENTCTL_AUTO_APPROVE_PERMISSIONS: "false" }); + }); + test.afterAll(async ({ backend }) => { + await backend.restart({ AGENTCTL_AUTO_APPROVE_PERMISSIONS: "true" }); + }); + + test("approved prompts stay approved after the agent's turn ends", async ({ + testPage, + apiClient, + seedData, + }) => { + const session = await seedMultiPermissionTask(testPage, apiClient, seedData); + + // Approve all three permission prompts as they appear. Each click unblocks + // the agent which then emits the next prompt; the previous one's button + // detaches before the next one mounts, so wait for the count to be back + // at 1 between clicks. + for (let i = 0; i < 3; i++) { + await expect(session.permissionApproveButtons()).toHaveCount(1, { timeout: 30_000 }); + await session.permissionApproveButtons().first().click(); + } + + // After the agent finishes its turn, no permission action row should be + // visible — the previous bug had them re-appear at turn-complete because a + // safety-net loop overwrote the approved status with "complete". + await expect(session.idleInput()).toBeVisible({ timeout: 30_000 }); + await expect(session.permissionActionRows()).toHaveCount(0); + + // And the resolved state must survive a page reload — i.e. backend must + // have persisted the approve decisions, not "complete". + await testPage.reload(); + await session.waitForLoad(); + await expect(session.idleInput()).toBeVisible({ timeout: 30_000 }); + await expect(session.permissionActionRows()).toHaveCount(0); + }); +}); diff --git a/apps/web/lib/ws/handlers/turns.ts b/apps/web/lib/ws/handlers/turns.ts index 5d8d385614..042bf75f41 100644 --- a/apps/web/lib/ws/handlers/turns.ts +++ b/apps/web/lib/ws/handlers/turns.ts @@ -39,9 +39,14 @@ export function registerTurnsHandlers(store: StoreApi): WsHandlers { // Safety net: mark any tool calls still in a non-terminal state as "complete". // This handles edge cases where tool_update events were dropped or not processed. + // Permission_request messages also carry `tool_call_id` in metadata, but their + // `status` represents the user's approve/reject decision — not the tool call + // state — so they must be excluded from the sweep. Forcing them to "complete" + // wipes "approved"/"rejected" and re-shows the prompt buttons in the UI. const messages = store.getState().messages.bySession[payload.session_id]; if (messages) { for (const msg of messages) { + if (msg.type === "permission_request") continue; const meta = msg.metadata as Record | undefined; if (meta?.tool_call_id && meta?.status !== "complete" && meta?.status !== "error") { store.getState().updateMessage({