fix(ui): deny open permission prompts on send (#2445)

Sending a message while a permission prompt is open now denies every pending permission in the session subtree (optimistically, then formally via permission.reply reject) and queues the message for next-turn delivery, mirroring the question-dismiss path from #1740. - Add dismissOpenPermissionsForSession plus isPermissionRequestNotFoundError and removePermissionRequestFromChildStores helpers to session-actions - Extend dismissPermission with not-found cleanup, parallel to rejectQuestion - Wire handleSubmit to deny permissions and dismiss questions together, queueing once if either prompt type was open - Add unit tests mirroring the dismissOpenQuestionsForSession suite Closes #1958
2026-07-28 17:36:00 +03:00
parent 92abdefea8
commit d50cb5becc
3 changed files with 303 additions and 9 deletions
@@ -978,8 +978,20 @@ const ChatInputComponent: React.FC<ChatInputProps> = ({ onOpenSettings, scrollTo
        }

        if (currentSessionId && !queuedOnly) {
-            const dismissedQuestions = await sessionActions.dismissOpenQuestionsForSession(currentSessionId);
-            if (dismissedQuestions) {
+            // Sending is authoritative for blocking prompts: deny pending
+            // permissions and dismiss open questions for the session subtree,
+            // then queue the message once if either was open. The deny/clear
+            // vanishes the card instantly (optimistic); rejecting unblocks the
+            // agent's tool but does NOT end its turn, so a direct send would
+            // race with the still-active run and be silently discarded by the
+            // OpenCode runner. Instead we queue; the queued-message auto-send
+            // hook delivers it as the next turn once the rejected turn winds
+            // down and the session returns to idle (parity with #1740).
+            const [deniedPermissions, dismissedQuestions] = await Promise.all([
+                sessionActions.dismissOpenPermissionsForSession(currentSessionId),
+                sessionActions.dismissOpenQuestionsForSession(currentSessionId),
+            ]);
+            if (deniedPermissions || dismissedQuestions) {
                handleQueueMessage();
                return;
            }
@@ -9,6 +9,7 @@ const registeredSessionDirectories: Array<{ sessionID: string; directory: string
 let sessionRevertResult: { data?: unknown; error?: unknown; response?: { status?: number } } = {}
 let questionReplyError: unknown | null = null
 let questionRejectError: unknown | null = null
+let permissionReplyError: unknown | null = null
 let sessionShareResult: { data?: unknown; error?: unknown; response?: { status?: number } } = {}
 let sessionUpdateResult: { data?: unknown; error?: unknown; response?: { status?: number } } = {}
 let sessionMessagesResult: { data?: unknown; error?: unknown; response?: { status?: number } } = { data: [] }
@@ -22,6 +23,10 @@ const mockScopedClient = {
  permission: {
    reply: mock((params: Record<string, unknown>) => {
      replyCalls.push({ method: "permission.reply", params })
+      if (permissionReplyError) {
+        const status = (permissionReplyError as { status?: number })?.status ?? 404
+        return Promise.resolve({ error: permissionReplyError, response: { status } })
+      }
      return Promise.resolve({ data: true })
    }),
  },
@@ -85,6 +90,10 @@ const mockSdk = {
  permission: {
    reply: mock((params: Record<string, unknown>) => {
      replyCalls.push({ method: "permission.reply", params })
+      if (permissionReplyError) {
+        const status = (permissionReplyError as { status?: number })?.status ?? 404
+        return Promise.resolve({ error: permissionReplyError, response: { status } })
+      }
      return Promise.resolve({ data: true })
    }),
  },
@@ -970,6 +979,7 @@ describe("dismissPermission passes directory", () => {
    replyCalls.length = 0
    scopedClientDirectories.length = 0
    questionReplyError = null
+    permissionReplyError = null
  })

  test("passes directory and reply=reject", async () => {
@@ -1084,6 +1094,17 @@ function buildQuestion(id: string, sessionId: string): QuestionRequest {
  }
 }

+function buildPermission(id: string, sessionId: string): PermissionRequest {
+  return {
+    id,
+    sessionID: sessionId,
+    permission: "edit",
+    patterns: [],
+    metadata: {},
+    always: [],
+  }
+}
+
 describe("dismissOpenQuestionsForSession", () => {
  beforeEach(() => {
    replyCalls.length = 0
@@ -1157,3 +1178,141 @@ describe("dismissOpenQuestionsForSession", () => {
    expect(store.getState().question["session-a"]).toBe(undefined)
  })
 })
+
+describe("dismissPermission not-found handling", () => {
+  beforeEach(() => {
+    replyCalls.length = 0
+    scopedClientDirectories.length = 0
+    permissionReplyError = null
+  })
+
+  test("clears the stale permission and rethrows on PermissionNotFoundError", async () => {
+    const permission = buildPermission("perm-stale", "session-a")
+    const store = createStore({ "session-a": [permission] })
+    const childStores = createChildStores([["/test/project", store]])
+    permissionReplyError = Object.assign(new Error("permission.reply failed (404): PermissionNotFoundError"), { status: 404 })
+
+    const { setActionRefs, dismissPermission } = await import("./session-actions")
+    setActionRefs(mockSdk as unknown as OpencodeClient, childStores, () => "/test/project")
+
+    await expect(dismissPermission("session-a", "perm-stale")).rejects.toThrow()
+    expect(replyCalls.filter((call) => call.method === "permission.reply")).toHaveLength(1)
+    // The stale entry is cleared from the store even though the server reported not-found.
+    expect(store.getState().permission["session-a"]).toBe(undefined)
+  })
+
+  test("does not clear the store on a non-not-found failure (rethrow only)", async () => {
+    const permission = buildPermission("perm-500", "session-a")
+    const store = createStore({ "session-a": [permission] })
+    const childStores = createChildStores([["/test/project", store]])
+    permissionReplyError = Object.assign(new Error("permission.reply failed (500)"), { status: 500 })
+
+    const { setActionRefs, dismissPermission } = await import("./session-actions")
+    setActionRefs(mockSdk as unknown as OpencodeClient, childStores, () => "/test/project")
+
+    await expect(dismissPermission("session-a", "perm-500")).rejects.toThrow()
+    // A non-not-found failure leaves store reconciliation to the next server event.
+    expect(store.getState().permission["session-a"]).toHaveLength(1)
+  })
+})
+
+describe("dismissOpenPermissionsForSession", () => {
+  beforeEach(() => {
+    replyCalls.length = 0
+    scopedClientDirectories.length = 0
+    permissionReplyError = null
+  })
+
+  test("returns false and rejects nothing when no permissions are pending", async () => {
+    const store = createStore({}, { session: [{ id: "session-a", time: { created: 1 } } as Session] })
+    const childStores = createChildStores([["/test/project", store]])
+
+    const { setActionRefs, dismissOpenPermissionsForSession } = await import("./session-actions")
+    setActionRefs(mockSdk as unknown as OpencodeClient, childStores, () => "/test/project")
+
+    const dismissed = await dismissOpenPermissionsForSession("session-a")
+
+    expect(dismissed).toBe(false)
+    expect(replyCalls.filter((call) => call.method === "permission.reply")).toHaveLength(0)
+  })
+
+  test("rejects every pending permission in the session subtree (root + subagent child)", async () => {
+    const rootPermission = buildPermission("perm-root", "session-a")
+    const childPermission = buildPermission("perm-child", "session-child")
+    const store = createStore({
+      "session-a": [rootPermission],
+      "session-child": [childPermission],
+    }, {
+      session: [
+        { id: "session-a", time: { created: 1 } } as Session,
+        { id: "session-child", parentID: "session-a", time: { created: 2 } } as Session,
+      ],
+    })
+    const childStores = createChildStores([["/test/project", store]])
+
+    const { setActionRefs, dismissOpenPermissionsForSession } = await import("./session-actions")
+    setActionRefs(mockSdk as unknown as OpencodeClient, childStores, () => "/test/project")
+
+    const dismissed = await dismissOpenPermissionsForSession("session-a")
+
+    expect(dismissed).toBe(true)
+    const replyCallsForPermissions = replyCalls.filter((call) => call.method === "permission.reply")
+    expect(replyCallsForPermissions).toHaveLength(2)
+    const rejectedIds = replyCallsForPermissions.map((call) => call.params.requestID).sort()
+    expect(rejectedIds).toEqual(["perm-child", "perm-root"])
+    expect(replyCallsForPermissions.every((call) => call.params.reply === "reject")).toBe(true)
+    // Optimistic clear: the permissions are removed from the local store so the
+    // prompt disappears instantly, without waiting for the reject round-trip.
+    expect(store.getState().permission["session-a"]).toBe(undefined)
+    expect(store.getState().permission["session-child"]).toBe(undefined)
+  })
+
+  test("swallows PermissionNotFoundError so a stranded permission never blocks the send", async () => {
+    const stalePermission = buildPermission("perm-stale", "session-a")
+    const store = createStore({ "session-a": [stalePermission] }, {
+      session: [{ id: "session-a", time: { created: 1 } } as Session],
+    })
+    const childStores = createChildStores([["/test/project", store]])
+    permissionReplyError = Object.assign(new Error("permission.reply failed (404): PermissionNotFoundError"), { status: 404 })
+
+    const { setActionRefs, dismissOpenPermissionsForSession } = await import("./session-actions")
+    setActionRefs(mockSdk as unknown as OpencodeClient, childStores, () => "/test/project")
+
+    const dismissed = await dismissOpenPermissionsForSession("session-a")
+
+    expect(dismissed).toBe(true)
+    const replyCallsForPermissions = replyCalls.filter((call) => call.method === "permission.reply")
+    expect(replyCallsForPermissions).toHaveLength(1)
+    expect(replyCallsForPermissions[0].params.requestID).toBe("perm-stale")
+    // The stale entry is cleared from the store even though the server reported not-found.
+    expect(store.getState().permission["session-a"]).toBe(undefined)
+  })
+
+  test("swallows and logs a non-not-found reject failure so the send is never blocked", async () => {
+    const permission = buildPermission("perm-500", "session-a")
+    const store = createStore({ "session-a": [permission] }, {
+      session: [{ id: "session-a", time: { created: 1 } } as Session],
+    })
+    const childStores = createChildStores([["/test/project", store]])
+    permissionReplyError = Object.assign(new Error("permission.reply failed (500)"), { status: 500 })
+
+    const { setActionRefs, dismissOpenPermissionsForSession } = await import("./session-actions")
+    setActionRefs(mockSdk as unknown as OpencodeClient, childStores, () => "/test/project")
+
+    const errors: unknown[][] = []
+    const originalError = console.error
+    console.error = (...args: unknown[]) => { errors.push(args) }
+    try {
+      const dismissed = await dismissOpenPermissionsForSession("session-a")
+
+      expect(dismissed).toBe(true)
+      const replyCallsForPermissions = replyCalls.filter((call) => call.method === "permission.reply")
+      expect(replyCallsForPermissions).toHaveLength(1)
+      expect(replyCallsForPermissions[0].params.requestID).toBe("perm-500")
+      expect(errors).toHaveLength(1)
+      expect(String(errors[0]?.[0])).toContain("[session-actions]")
+    } finally {
+      console.error = originalError
+    }
+  })
+})
@@ -553,6 +553,56 @@ function removeQuestionRequestFromChildStores(sessionId: string, requestId: stri
  return removed
 }

+function isPermissionRequestNotFoundError(error: unknown): boolean {
+  if (error && typeof error === "object") {
+    const status = (error as { status?: unknown }).status
+    if (status === 404) return true
+  }
+
+  let message = ""
+  if (error instanceof Error) {
+    message = error.message
+  } else if (typeof error === "string") {
+    message = error
+  }
+
+  return /Permission(?:\.)?NotFoundError|Permission request not found/i.test(message)
+}
+
+function removePermissionRequestFromChildStores(sessionId: string, requestId: string): boolean {
+  const stores = _childStores
+  if (!stores || !requestId) return false
+
+  let removed = false
+  for (const [, store] of stores.children) {
+    const current = store.getState().permission ?? {}
+    let nextPermission: typeof current | null = null
+    const sessionIds = new Set([sessionId, ...Object.keys(current)].filter(Boolean))
+
+    for (const candidateSessionId of sessionIds) {
+      const requests = current[candidateSessionId]
+      if (!requests?.length) continue
+
+      const nextRequests = requests.filter((request) => request.id !== requestId)
+      if (nextRequests.length === requests.length) continue
+
+      nextPermission ??= { ...current }
+      if (nextRequests.length > 0) {
+        nextPermission[candidateSessionId] = nextRequests
+      } else {
+        delete nextPermission[candidateSessionId]
+      }
+      removed = true
+    }
+
+    if (nextPermission) {
+      store.setState({ permission: nextPermission })
+    }
+  }
+
+  return removed
+}
+
 function getRequestReplyClient(
  type: "permission" | "question",
  sessionId: string,
@@ -1093,16 +1143,89 @@ export async function dismissPermission(
  const directory = resolveDirectoryForBlockingRequest("permission", sessionId, requestId)
    || getSessionDirectory(sessionId)
    || dir()
-  const result = await getRequestReplyClient("permission", sessionId, requestId).permission.reply({
-    requestID: requestId,
-    reply: "reject",
-    ...(directory ? { directory } : {}),
-  })
-  if (assertSdkData(result, "permission.reply") !== true) {
-    throw new Error("Permission dismissal failed")
+  try {
+    const result = await getRequestReplyClient("permission", sessionId, requestId).permission.reply({
+      requestID: requestId,
+      reply: "reject",
+      ...(directory ? { directory } : {}),
+    })
+    if (assertSdkData(result, "permission.reply") !== true) {
+      throw new Error("Permission dismissal failed")
+    }
+  } catch (error) {
+    if (isPermissionRequestNotFoundError(error)) {
+      removePermissionRequestFromChildStores(sessionId, requestId)
+    }
+    throw error
  }
 }

+/**
+ * Dismiss every pending permission for the session subtree rooted at `sessionId`
+ * (the session itself plus any subagent children). Used by the chat send path:
+ * sending a message while a permission prompt is open must cancel/supersede the
+ * open permission so it cannot linger or block the new turn.
+ *
+ * The permissions are removed from the local store OPTIMISTICALLY (before any
+ * network call) so the prompt disappears instantly instead of waiting on the
+ * `permission.reply` round-trip. Each permission is then formally rejected on
+ * the backend via `permission.reply` with `reply: "reject"`, which fires
+ * `permission.replied` for reconciliation.
+ *
+ * Returns true when at least one permission was dismissed. Rejection failures are
+ * swallowed (a stranded permission must never block the send);
+ * PermissionNotFoundError also clears the stale entry from the child store via
+ * {@link dismissPermission}.
+ *
+ * NOTE: rejecting unblocks the agent's tool but does NOT end its turn. Callers
+ * that need to send the next message right away (the chat send path) must also
+ * queue the message so the OpenCode runner reaches `idle` — otherwise the new
+ * prompt arrives while the run is still active and is discarded by the runner's
+ * `ensureRunning`.
+ */
+export async function dismissOpenPermissionsForSession(sessionId: string): Promise<boolean> {
+  if (!sessionId) return false
+  const stores = _childStores
+  if (!stores) return false
+
+  const toDismiss: Array<{ sessionId: string; requestId: string }> = []
+  for (const [, store] of stores.children) {
+    const state = store.getState()
+    const scopedIds = computeSubtreeIds(state.session, sessionId)
+    if (scopedIds.size === 0) continue
+    const permissionsBySession = state.permission ?? {}
+    for (const scopedId of scopedIds) {
+      const requests = permissionsBySession[scopedId]
+      if (!requests) continue
+      for (const request of requests) {
+        toDismiss.push({ sessionId: scopedId, requestId: request.id })
+      }
+    }
+  }
+
+  if (toDismiss.length === 0) return false
+
+  // Optimistically clear the permissions from the local store so the prompt
+  // disappears immediately, before the reject round-trip.
+  for (const { sessionId: scopedSessionId, requestId } of toDismiss) {
+    removePermissionRequestFromChildStores(scopedSessionId, requestId)
+  }
+
+  await Promise.all(
+    toDismiss.map(async ({ sessionId: scopedSessionId, requestId }) => {
+      try {
+        await dismissPermission(scopedSessionId, requestId)
+      } catch (error) {
+        if (isPermissionRequestNotFoundError(error)) return
+        // Swallow: a failed dismissal must not block the send. The next
+        // permission.asked / permission.replied event reconciles the store.
+        console.error("[session-actions] Failed to dismiss open permission on send:", error)
+      }
+    }),
+  )
+  return true
+}
+
 // ---------------------------------------------------------------------------
 // Questions
 // ---------------------------------------------------------------------------