fix(app): surface exhausted auto-approve permission requests (#61584)

packages/app/src/runtime/server/runtime.tsx

@@ -249,7 +249,7 @@ function createServerController(
   })
 
   const sync = createServerSyncContext(sdk, data)
-  createPermissionAutoApprover({ sdk, data })
+  const autoApprove = createPermissionAutoApprover({ sdk, data })
   const notification = createServerNotificationState({ sdk, data, key: connKey, coordinator: notificationCoordinator })
 
   // Projects whose repository has no root commit share the `global` ID; their name and icon stay local to the
@@ -356,6 +356,7 @@ function createServerController(
       },
     },
     notification,
+    autoApprove,
   }
 }
 

packages/app/src/session/requests/auto-approve-machine.test.ts

@@ -0,0 +1,160 @@
+import { describe, expect, test } from "bun:test"
+import {
+  INITIAL_AUTO_APPROVE_STATE,
+  isDockVisible,
+  transitionAutoApprove,
+  type AutoApproveEffect,
+  type AutoApproveEvent,
+  type AutoApproveState,
+  type AutoApproveTransition,
+} from "./auto-approve-machine"
+
+const approve = (runId: string): AutoApproveEvent => ({
+  type: "approve",
+  sessionID: "ses_1",
+  projectID: "proj_1",
+  runId,
+})
+
+const replying = (runId: string, attempt: number): AutoApproveState => ({
+  status: "replying",
+  sessionID: "ses_1",
+  projectID: "proj_1",
+  runId,
+  attempt,
+})
+
+const waiting = (runId: string, attempt: number): AutoApproveState => ({
+  status: "waiting-retry",
+  sessionID: "ses_1",
+  projectID: "proj_1",
+  runId,
+  attempt,
+})
+
+const gaveUp: AutoApproveState = { status: "gave-up", projectID: "proj_1" }
+
+function replay(events: AutoApproveEvent[], from = INITIAL_AUTO_APPROVE_STATE) {
+  return events.reduce<AutoApproveTransition[]>((steps, event) => {
+    steps.push(transitionAutoApprove(steps.at(-1)?.state ?? from, event))
+
+    return steps
+  }, [])
+}
+
+describe("transitionAutoApprove", () => {
+  test("approves once and settles on a successful reply", () => {
+    const steps = replay([approve("r1"), { type: "reply.succeeded", runId: "r1", attempt: 0 }])
+
+    expect(steps.map((step) => step.state)).toEqual([replying("r1", 0), { status: "replied" }])
+    expect(steps.map((step) => step.effects)).toEqual([
+      [{ type: "reply", sessionID: "ses_1", runId: "r1", attempt: 0 }],
+      [],
+    ])
+  })
+
+  test("surfaces the request in the dock once every retry fails", () => {
+    const steps = replay([
+      approve("r1"),
+      { type: "reply.failed", runId: "r1", attempt: 0 },
+      { type: "retry.due", runId: "r1", attempt: 1 },
+      { type: "reply.failed", runId: "r1", attempt: 1 },
+      { type: "retry.due", runId: "r1", attempt: 2 },
+      { type: "reply.failed", runId: "r1", attempt: 2 },
+    ])
+
+    expect(steps.map((step) => step.state)).toEqual([
+      replying("r1", 0),
+      waiting("r1", 1),
+      replying("r1", 1),
+      waiting("r1", 2),
+      replying("r1", 2),
+      gaveUp,
+    ])
+    expect(steps.map((step) => step.effects)).toEqual([
+      [{ type: "reply", sessionID: "ses_1", runId: "r1", attempt: 0 }],
+      [{ type: "schedule-retry", runId: "r1", attempt: 1, delayMs: 1000 }],
+      [{ type: "reply", sessionID: "ses_1", runId: "r1", attempt: 1 }],
+      [{ type: "schedule-retry", runId: "r1", attempt: 2, delayMs: 2000 }],
+      [{ type: "reply", sessionID: "ses_1", runId: "r1", attempt: 2 }],
+      [],
+    ])
+    expect(steps.map((step) => isDockVisible(step.state, true))).toEqual([false, false, false, false, false, true])
+  })
+
+  test("recovers on a later attempt", () => {
+    const state = replay([
+      approve("r1"),
+      { type: "reply.failed", runId: "r1", attempt: 0 },
+      { type: "retry.due", runId: "r1", attempt: 1 },
+      { type: "reply.succeeded", runId: "r1", attempt: 1 },
+    ]).at(-1)?.state
+
+    expect(state).toEqual({ status: "replied" })
+  })
+
+  test.each<{ name: string; from: AutoApproveState }>([
+    { name: "replying", from: replying("r1", 0) },
+    { name: "waiting for a retry", from: waiting("r1", 1) },
+    { name: "exhausted", from: gaveUp },
+    { name: "replied", from: { status: "replied" } },
+  ])("ignores another approval while $name", (input) => {
+    const result = transitionAutoApprove(input.from, approve("r2"))
+
+    expect(result.state).toBe(input.from)
+    expect(result.effects).toEqual([])
+  })
+
+  test("rejects results and timers from a superseded cycle or attempt", () => {
+    const state = replay([approve("r1"), { type: "cancel" }, approve("r2")]).at(-1)?.state ?? INITIAL_AUTO_APPROVE_STATE
+
+    expect(state).toEqual(replying("r2", 0))
+
+    const stale: AutoApproveEvent[] = [
+      { type: "reply.failed", runId: "r1", attempt: 0 },
+      { type: "reply.succeeded", runId: "r1", attempt: 0 },
+      { type: "reply.failed", runId: "r2", attempt: 1 },
+      { type: "retry.due", runId: "r2", attempt: 0 },
+    ]
+
+    stale.forEach((event) => {
+      const result = transitionAutoApprove(state, event)
+
+      expect(result.state).toBe(state)
+      expect(result.effects).toEqual([])
+    })
+
+    const pausing = waiting("r2", 1)
+
+    expect(transitionAutoApprove(pausing, { type: "retry.due", runId: "r1", attempt: 1 }).state).toBe(pausing)
+  })
+
+  test.each<{ from: AutoApproveState; cancel: AutoApproveState; resolved: AutoApproveState; clears: boolean }>([
+    {
+      from: INITIAL_AUTO_APPROVE_STATE,
+      cancel: INITIAL_AUTO_APPROVE_STATE,
+      resolved: INITIAL_AUTO_APPROVE_STATE,
+      clears: false,
+    },
+    { from: replying("r1", 0), cancel: INITIAL_AUTO_APPROVE_STATE, resolved: { status: "replied" }, clears: false },
+    { from: waiting("r1", 1), cancel: INITIAL_AUTO_APPROVE_STATE, resolved: { status: "replied" }, clears: true },
+    { from: gaveUp, cancel: INITIAL_AUTO_APPROVE_STATE, resolved: { status: "replied" }, clears: false },
+    { from: { status: "replied" }, cancel: { status: "replied" }, resolved: { status: "replied" }, clears: false },
+  ])("cancel and resolved from $from.status", (input) => {
+    const effects: AutoApproveEffect[] = input.clears ? [{ type: "cancel-retry" }] : []
+
+    expect(transitionAutoApprove(input.from, { type: "cancel" })).toEqual({ state: input.cancel, effects })
+    expect(transitionAutoApprove(input.from, { type: "resolved" })).toEqual({ state: input.resolved, effects })
+  })
+})
+
+test.each<{ state: AutoApproveState; on: boolean; off: boolean }>([
+  { state: INITIAL_AUTO_APPROVE_STATE, on: false, off: true },
+  { state: replying("r1", 0), on: false, off: true },
+  { state: waiting("r1", 1), on: false, off: true },
+  { state: gaveUp, on: true, off: true },
+  { state: { status: "replied" }, on: false, off: false },
+])("isDockVisible for $state.status", (input) => {
+  expect(isDockVisible(input.state, true)).toBe(input.on)
+  expect(isDockVisible(input.state, false)).toBe(input.off)
+})

packages/app/src/session/requests/auto-approve-machine.ts

@@ -0,0 +1,119 @@
+export const AUTO_APPROVE_RETRY_LIMIT = 2
+
+export const AUTO_APPROVE_RETRY_DELAY_MS = 1000
+
+// One permission request's auto-approve lifecycle. `runId` names one approval cycle and `attempt` one reply within
+// it, so a reply or timer from a cancelled or resolved cycle can never move a later one.
+export type AutoApproveState =
+  | { readonly status: "pending" }
+  | {
+      readonly status: "replying"
+      readonly sessionID: string
+      readonly projectID: string | undefined
+      readonly runId: string
+      readonly attempt: number
+    }
+  | {
+      readonly status: "waiting-retry"
+      readonly sessionID: string
+      readonly projectID: string | undefined
+      readonly runId: string
+      readonly attempt: number
+    }
+  | { readonly status: "replied" }
+  | { readonly status: "gave-up"; readonly projectID: string | undefined }
+
+export type AutoApproveEvent =
+  | {
+      readonly type: "approve"
+      readonly sessionID: string
+      readonly projectID: string | undefined
+      readonly runId: string
+    }
+  | { readonly type: "reply.succeeded"; readonly runId: string; readonly attempt: number }
+  | { readonly type: "reply.failed"; readonly runId: string; readonly attempt: number }
+  | { readonly type: "retry.due"; readonly runId: string; readonly attempt: number }
+  | { readonly type: "cancel" }
+  | { readonly type: "resolved" }
+
+export type AutoApproveEffect =
+  | { readonly type: "reply"; readonly sessionID: string; readonly runId: string; readonly attempt: number }
+  | { readonly type: "schedule-retry"; readonly runId: string; readonly attempt: number; readonly delayMs: number }
+  | { readonly type: "cancel-retry" }
+
+export type AutoApproveTransition = {
+  readonly state: AutoApproveState
+  readonly effects: readonly AutoApproveEffect[]
+}
+
+export const INITIAL_AUTO_APPROVE_STATE: AutoApproveState = { status: "pending" }
+
+export function transitionAutoApprove(state: AutoApproveState, event: AutoApproveEvent): AutoApproveTransition {
+  // A request nobody tried to approve has nothing to settle when another client answers it.
+  if (event.type === "resolved") {
+    if (state.status === "pending" || state.status === "replied") return unchanged(state)
+
+    return changed({ status: "replied" }, state.status === "waiting-retry" ? [{ type: "cancel-retry" }] : [])
+  }
+
+  if (event.type === "cancel") {
+    if (state.status === "pending" || state.status === "replied") return unchanged(state)
+
+    return changed(INITIAL_AUTO_APPROVE_STATE, state.status === "waiting-retry" ? [{ type: "cancel-retry" }] : [])
+  }
+
+  // Store scans and sweeps approve repeatedly; only a request no cycle owns starts one. An exhausted request stays
+  // surfaced until someone answers it or auto-approve is turned off and on again.
+  if (event.type === "approve") {
+    if (state.status !== "pending") return unchanged(state)
+
+    return changed(
+      { status: "replying", sessionID: event.sessionID, projectID: event.projectID, runId: event.runId, attempt: 0 },
+      [{ type: "reply", sessionID: event.sessionID, runId: event.runId, attempt: 0 }],
+    )
+  }
+
+  if (event.type === "reply.succeeded") {
+    if (state.status !== "replying" || !current(state, event)) return unchanged(state)
+
+    return changed({ status: "replied" })
+  }
+
+  if (event.type === "reply.failed") {
+    if (state.status !== "replying" || !current(state, event)) return unchanged(state)
+
+    if (state.attempt >= AUTO_APPROVE_RETRY_LIMIT) return changed({ status: "gave-up", projectID: state.projectID })
+    const attempt = state.attempt + 1
+
+    return changed({ ...state, status: "waiting-retry", attempt }, [
+      { type: "schedule-retry", runId: state.runId, attempt, delayMs: AUTO_APPROVE_RETRY_DELAY_MS * attempt },
+    ])
+  }
+
+  if (state.status !== "waiting-retry" || !current(state, event)) return unchanged(state)
+
+  return changed({ ...state, status: "replying" }, [
+    { type: "reply", sessionID: state.sessionID, runId: state.runId, attempt: state.attempt },
+  ])
+}
+
+// Whether the permission dock shows a pending request. Auto-approve hides it only while a cycle can still answer it.
+export function isDockVisible(state: AutoApproveState, autoApprove: boolean) {
+  if (state.status === "replied") return false
+
+  if (state.status === "gave-up") return true
+
+  return !autoApprove
+}
+
+function current(state: { runId: string; attempt: number }, event: { runId: string; attempt: number }) {
+  return state.runId === event.runId && state.attempt === event.attempt
+}
+
+function changed(state: AutoApproveState, effects: readonly AutoApproveEffect[] = []): AutoApproveTransition {
+  return { state, effects }
+}
+
+function unchanged(state: AutoApproveState): AutoApproveTransition {
+  return { state, effects: [] }
+}

packages/app/src/session/requests/auto-approve.ts

@@ -1,14 +1,22 @@
-import { createEffect, onCleanup } from "solid-js"
+import { createEffect, onCleanup, untrack } from "solid-js"
+import { createStore, produce } from "solid-js/store"
 import type { PermissionRequest } from "@opencode/client/promise"
 import type { Data } from "@opencode/client/solid"
+import { abort } from "@opencode/util/abort"
+import { uuid } from "@opencode/util/uuid"
 import type { ServerSDK } from "@/runtime/server/client"
 import { projectSettingsKey, useSettings } from "@/settings/model"
+import {
+  AUTO_APPROVE_RETRY_DELAY_MS,
+  AUTO_APPROVE_RETRY_LIMIT,
+  INITIAL_AUTO_APPROVE_STATE,
+  transitionAutoApprove,
+  type AutoApproveEffect,
+  type AutoApproveEvent,
+  type AutoApproveState,
+} from "./auto-approve-machine"
 
-const respondedLimit = 1000
-
-const retryLimit = 2
-
-const retryDelayMs = 1000
+const repliedLimit = 1000
 
 // Auto-approves permission requests on one server connection for the projects
 // the client-local settings cover: the app-wide default, unless a project overrides it.
@@ -19,24 +27,34 @@ export function createPermissionAutoApprover(input: { sdk: ServerSDK; data: Data
   const covers = (projectID: string | undefined) =>
     permissions.autoApproves(projectSettingsKey(input.sdk.scope, projectID))
 
-  const state = { disposed: false, generation: 0, responded: new Set<string>() }
+  // Requests in the initial `pending` state have no entry.
+  const [requests, setRequests] = createStore<Record<string, AutoApproveState | undefined>>({})
+  const timers = new Map<string, ReturnType<typeof setTimeout>>()
+  const scope = new AbortController()
 
-  const unsubscribe = input.sdk.event.on("permission.asked", (event) => {
-    if (enabled()) consider(event.data)
-  })
+  const unsubscribe = [
+    input.sdk.event.on("permission.asked", (event) => {
+      if (enabled()) consider(event.data, scope.signal)
+    }),
+    input.sdk.event.on("permission.replied", (event) => dispatch(event.data.requestID, { type: "resolved" })),
+  ]
 
   onCleanup(() => {
-    state.disposed = true
-    unsubscribe()
+    abort(scope)
+    unsubscribe.forEach((stop) => stop())
+    // Back to pending, so replies still in flight settle into nothing and no retry is scheduled.
+    untrack(() => Object.keys(requests)).forEach((id) => dispatch(id, { type: "cancel" }))
   })
 
   // The event stream does not replay requests asked while this client was
   // disconnected, and requests may already be pending before the setting turns
-  // on, so sweep on every connect while the setting is on.
+  // on, so sweep on every connect while the setting is on. A newer connect or
+  // a change of the setting aborts the sweep and its scheduled retries.
   createEffect(() => {
     if (!enabled() || input.sdk.connection.status() !== "connected") return
-    const generation = ++state.generation
-    void sweepWithRetry(generation, 0)
+    const controller = new AbortController()
+    onCleanup(() => abort(controller))
+    void sweepWithRetry(controller.signal, 0)
   })
 
   // Approves pending requests that reach the local store, which is how a
@@ -51,20 +69,78 @@ export function createPermissionAutoApprover(input: { sdk: ServerSDK; data: Data
     }
   })
 
-  // An incomplete sweep leaves pending requests hidden with no later trigger
-  // to recover them, so retry it a bounded number of times. A newer sweep
-  // supersedes scheduled retries.
-  async function sweepWithRetry(generation: number, attempt: number) {
-    const complete = await sweep()
+  // Turning auto-approve off for a request's project stops its cycle, so a
+  // scheduled retry never answers for the user and turning it on again starts over.
+  createEffect(() => {
+    Object.entries(requests).forEach(([id, state]) => {
+      if (!state || state.status === "pending" || state.status === "replied") return
 
-    if (complete || attempt >= retryLimit) return
-    setTimeout(() => {
-      if (state.disposed || !enabled() || generation !== state.generation) return
-      void sweepWithRetry(generation, attempt + 1)
-    }, retryDelayMs * (attempt + 1))
+      if (!covers(state.projectID)) dispatch(id, { type: "cancel" })
+    })
+  })
+
+  function dispatch(requestID: string, event: AutoApproveEvent) {
+    const state = untrack(() => requests[requestID]) ?? INITIAL_AUTO_APPROVE_STATE
+    const result = transitionAutoApprove(state, event)
+
+    if (result.state !== state) {
+      setRequests({ [requestID]: result.state.status === "pending" ? undefined : result.state })
+
+      if (result.state.status === "replied") pruneReplied()
+    }
+
+    result.effects.forEach((effect) => execute(requestID, effect))
   }
 
-  async function sweep() {
+  function execute(requestID: string, effect: AutoApproveEffect) {
+    if (effect.type === "reply") {
+      // A request another client already answered counts as replied.
+      void input.data.session.permission.reply({ sessionID: effect.sessionID, requestID, decision: "once" }).then(
+        () => dispatch(requestID, { type: "reply.succeeded", runId: effect.runId, attempt: effect.attempt }),
+        () => dispatch(requestID, { type: "reply.failed", runId: effect.runId, attempt: effect.attempt }),
+      )
+
+      return
+    }
+
+    clearTimeout(timers.get(requestID))
+    timers.delete(requestID)
+
+    if (effect.type === "cancel-retry") return
+    timers.set(
+      requestID,
+      setTimeout(() => {
+        timers.delete(requestID)
+        dispatch(requestID, { type: "retry.due", runId: effect.runId, attempt: effect.attempt })
+      }, effect.delayMs),
+    )
+  }
+
+  function pruneReplied() {
+    const replied = untrack(() =>
+      Object.entries(requests).flatMap(([id, state]) => (state?.status === "replied" ? [id] : [])),
+    )
+
+    if (replied.length <= repliedLimit) return
+    setRequests(produce((draft) => replied.slice(0, replied.length - repliedLimit).forEach((id) => delete draft[id])))
+  }
+
+  // An incomplete sweep leaves pending requests hidden with no later trigger
+  // to recover them, so retry it a bounded number of times.
+  async function sweepWithRetry(signal: AbortSignal, attempt: number) {
+    const complete = await sweep(signal)
+
+    if (complete || attempt >= AUTO_APPROVE_RETRY_LIMIT || signal.aborted) return
+
+    const timer = setTimeout(
+      () => void sweepWithRetry(signal, attempt + 1),
+      AUTO_APPROVE_RETRY_DELAY_MS * (attempt + 1),
+    )
+
+    signal.addEventListener("abort", () => clearTimeout(timer), { once: true })
+  }
+
+  async function sweep(signal: AbortSignal) {
     const inventory = await sweepLocations()
 
     const listed = await Promise.all(
@@ -72,7 +148,7 @@ export function createPermissionAutoApprover(input: { sdk: ServerSDK; data: Data
         input.sdk.api.permission.request
           .list({ location: { directory: location.directory } })
           .then((pending) => {
-            if (!state.disposed) pending.data.forEach((request) => consider(request))
+            if (!signal.aborted) pending.data.forEach((request) => consider(request, signal))
 
             return true
           })
@@ -128,42 +204,23 @@ export function createPermissionAutoApprover(input: { sdk: ServerSDK; data: Data
 
   // A request from a session this client has not loaded needs that session's
   // project before a per-project choice can apply to it.
-  function consider(permission: PermissionRequest) {
+  function consider(permission: PermissionRequest, signal: AbortSignal) {
     if (covers(undefined) || input.data.session.get(permission.sessionID)) return approve(permission)
     void input.sdk.api.session
       .get({ sessionID: permission.sessionID })
-      .then((session) => approve(permission, session.projectID))
+      .then((session) => {
+        if (!signal.aborted) approve(permission, session.projectID)
+      })
       .catch(() => undefined)
   }
 
-  function approve(
-    permission: PermissionRequest,
-    projectID = input.data.session.get(permission.sessionID)?.projectID,
-    attempt = 0,
-  ) {
-    // covers() guards the retry timer path: the user may disable the setting
-    // between a failed reply and its scheduled retry.
-    if (state.disposed || !covers(projectID) || state.responded.has(permission.id)) return
-    remember(permission.id)
-    input.sdk.api.permission
-      .reply({ sessionID: permission.sessionID, requestID: permission.id, decision: "once" })
-      .catch(() => {
-        // A reply failure leaves the request pending but invisible (the UI
-        // hides prompts while auto-approve is on), so retry a bounded number
-        // of times. Later sweeps retry it after that.
-        state.responded.delete(permission.id)
-
-        if (state.disposed || attempt >= retryLimit) return
-        setTimeout(() => approve(permission, projectID, attempt + 1), retryDelayMs * (attempt + 1))
-      })
+  function approve(permission: PermissionRequest, projectID = input.data.session.get(permission.sessionID)?.projectID) {
+    if (!covers(projectID)) return
+    dispatch(permission.id, { type: "approve", sessionID: permission.sessionID, projectID, runId: uuid() })
   }
 
-  function remember(id: string) {
-    state.responded.add(id)
-
-    for (const oldest of state.responded) {
-      if (state.responded.size <= respondedLimit) break
-      state.responded.delete(oldest)
-    }
+  return {
+    /** The auto-approve lifecycle of one permission request. Reactive. */
+    state: (requestID: string) => requests[requestID] ?? INITIAL_AUTO_APPROVE_STATE,
   }
 }

packages/app/src/session/requests/model.ts

@@ -11,13 +11,15 @@ import { sessionPermissionRequest, sessionFormRequest, sessionTreeIDs } from "@/
 import { createWebSearchRequest } from "./websearch"
 import { createSessionBackground } from "@/session/requests/background"
 import { createSteerBackground } from "@/session/requests/steer-background"
-import { useData } from "@/runtime/server/current"
+import { useData, useServer } from "@/runtime/server/current"
+import { isDockVisible } from "./auto-approve-machine"
 
 export function createSessionRequestModel() {
   const params = useParams()
   const sdk = useWorkspaceLocation()
   const serverSDK = useServerSDK()
   const data = useData()
+  const autoApprove = useServer().ctx.autoApprove
   const language = useLanguage()
   const settings = useSettings()
   createEffect(() => {
@@ -107,11 +109,13 @@ export function createSessionRequestModel() {
   const permissionRequest = createMemo((): PermissionRequest | undefined => {
     const id = params.id
 
-    const project = projectSettingsKey(serverSDK.scope, id ? data.session.get(id)?.projectID : undefined)
+    const enabled = settings.permissions.autoApproves(
+      projectSettingsKey(serverSDK.scope, id ? data.session.get(id)?.projectID : undefined),
+    )
 
-    if (settings.permissions.autoApproves(project)) return
-
-    return sessionPermissionRequest(data.session.list(), data.session.permission.list, id)
+    return sessionPermissionRequest(data.session.list(), data.session.permission.list, id, (item) =>
+      isDockVisible(autoApprove.state(item.id), enabled),
+    )
   })
 
   const blocked = createMemo(() => {