From 443878def0a289ed523a57470567ac4b995df5bd Mon Sep 17 00:00:00 2001 From: Luke Parker <10430890+Hona@users.noreply.github.com> Date: Wed, 4 Nov 2026 13:24:56 -0600 Subject: [PATCH] fix(app): badge permission requests that auto-approve gave up on (#61736) --- packages/app/e2e/regression/tabs.spec.ts | 77 +++++++++++++++++++ .../requests/auto-approve-machine.test.ts | 10 +-- .../session/requests/auto-approve-machine.ts | 5 +- packages/app/src/session/requests/model.ts | 4 +- .../src/shell/layout/project-avatar-state.ts | 25 +++--- .../app/src/shell/titlebar/inbox-model.ts | 21 +++-- 6 files changed, 112 insertions(+), 30 deletions(-) diff --git a/packages/app/e2e/regression/tabs.spec.ts b/packages/app/e2e/regression/tabs.spec.ts index a9565d61389..bef7c8638d9 100644 --- a/packages/app/e2e/regression/tabs.spec.ts +++ b/packages/app/e2e/regression/tabs.spec.ts @@ -1964,3 +1964,80 @@ test("the inbox waits for initial requests and orders attention by waiting age r await expect(inbox.getByRole("status")).toHaveText("New live question has a question.") await expect(inbox.getByRole("status")).toHaveCount(1) }) + +test("an inactive tab shows attention once auto-accept gives up on its permission", async ({ page }) => { + const reply = await mockGaveUpPermission(page, "horizontal") + await page.goto(sessionHref(a.id)) + const tab = page.locator(`[data-titlebar-tab-slot]:has(a[href="${sessionHref(b.id)}"])`) + const dot = tab.locator('[data-mark="attention"]') + + // The first reply proves the client holds the request; auto-accept still owns it, so the tab only shows work. + await reply.arrived + await expect(tab.locator('[data-mark="busy"]')).toBeVisible() + await expect(dot).toBeHidden() + + reply.release() + await expect(dot).toBeVisible() + await expect(tab.locator('[data-mark="busy"]')).toBeHidden() + expect(reply.attempts()).toBe(3) +}) + +test("the inbox moves a session to Needs attention once auto-accept gives up on its permission", async ({ page }) => { + const reply = await mockGaveUpPermission(page, "inbox") + await page.goto(sessionHref(a.id)) + + const inbox = page.locator('[data-slot="sidebar-inbox"]') + + const row = inbox.locator('[data-slot="sidebar-inbox-item"]').filter({ hasText: b.title }) + + const needs = inbox + .getByRole("group", { name: "Needs attention", exact: true }) + .locator('[data-slot="sidebar-inbox-item"]') + + await reply.arrived + await expect(row).toHaveAttribute("data-status", "working") + await expect(needs).toHaveCount(0) + + reply.release() + await expect(needs).toHaveCount(1) + await expect(needs).toContainText(b.title) + await expect(needs).toHaveAttribute("data-status", "approval") + expect(reply.attempts()).toBe(3) +}) + +// Auto-accept is on and every reply to session B's permission fails. The first reply waits for `release()`; then it +// and both retries fail, and auto-accept gives up. +async function mockGaveUpPermission(page: Page, tabLayout: "horizontal" | "inbox") { + const permissionID = "permission-gave-up" + const released = Promise.withResolvers() + const arrived = Promise.withResolvers() + const attempts = { count: 0 } + + await mockWorkspace(page, { + name: "GaveUp", + sessions: [a, b], + sessionPermissions: { + [b.id]: [ + { id: permissionID, sessionID: b.id, action: "shell", resources: ["git status"], metadata: {}, save: [] }, + ], + }, + seed: { + settings: { appearance: { tabLayout }, permissions: { autoApprove: true } }, + storage: { "opencode.global.dat:sidebar.inbox": { workingExpanded: true } }, + }, + }) + await page.route(new RegExp(`/api/session/${b.id}/permission/${permissionID}/reply(\\?|$)`), async (route) => { + if (route.request().method() !== "POST") return route.fallback() + attempts.count++ + arrived.resolve() + + if (attempts.count === 1) await released.promise + await route.fulfill({ status: 500, json: { message: "Reply failed" } }) + }) + + return { + arrived: arrived.promise, + release: () => released.resolve(), + attempts: () => attempts.count, + } +} diff --git a/packages/app/src/session/requests/auto-approve-machine.test.ts b/packages/app/src/session/requests/auto-approve-machine.test.ts index afc359159d2..de8e5b3d3fb 100644 --- a/packages/app/src/session/requests/auto-approve-machine.test.ts +++ b/packages/app/src/session/requests/auto-approve-machine.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from "bun:test" import { INITIAL_AUTO_APPROVE_STATE, - isDockVisible, + needsUser, transitionAutoApprove, type AutoApproveEffect, type AutoApproveEvent, @@ -79,7 +79,7 @@ describe("transitionAutoApprove", () => { [{ type: "reply", sessionID: "ses_1", runId: "r1", attempt: 2 }], [], ]) - expect(steps.map((step) => isDockVisible(step.state, true))).toEqual([false, false, false, false, false, true]) + expect(steps.map((step) => needsUser(step.state, true))).toEqual([false, false, false, false, false, true]) }) test("recovers on a later attempt", () => { @@ -154,7 +154,7 @@ test.each<{ state: AutoApproveState; on: boolean; off: boolean }>([ { 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) +])("needsUser for $state.status", (input) => { + expect(needsUser(input.state, true)).toBe(input.on) + expect(needsUser(input.state, false)).toBe(input.off) }) diff --git a/packages/app/src/session/requests/auto-approve-machine.ts b/packages/app/src/session/requests/auto-approve-machine.ts index c3bffcf82de..2457f5ffbb0 100644 --- a/packages/app/src/session/requests/auto-approve-machine.ts +++ b/packages/app/src/session/requests/auto-approve-machine.ts @@ -97,8 +97,9 @@ export function transitionAutoApprove(state: AutoApproveState, event: AutoApprov ]) } -// 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) { +// Whether a pending request waits on the user, so the permission dock shows it and every badge counts it. +// Auto-approve hides it only while a cycle can still answer it. +export function needsUser(state: AutoApproveState, autoApprove: boolean) { if (state.status === "replied") return false if (state.status === "gave-up") return true diff --git a/packages/app/src/session/requests/model.ts b/packages/app/src/session/requests/model.ts index f6b42e2726d..9939a428616 100644 --- a/packages/app/src/session/requests/model.ts +++ b/packages/app/src/session/requests/model.ts @@ -12,7 +12,7 @@ import { createWebSearchRequest } from "./websearch" import { createSessionBackground } from "@/session/requests/background" import { createSteerBackground } from "@/session/requests/steer-background" import { useData, useServer } from "@/runtime/server/current" -import { isDockVisible } from "./auto-approve-machine" +import { needsUser } from "./auto-approve-machine" export function createSessionRequestModel() { const params = useParams() @@ -114,7 +114,7 @@ export function createSessionRequestModel() { ) return sessionPermissionRequest(data.session.list(), data.session.permission.list, id, (item) => - isDockVisible(autoApprove.state(item.id), enabled), + needsUser(autoApprove.state(item.id), enabled), ) }) diff --git a/packages/app/src/shell/layout/project-avatar-state.ts b/packages/app/src/shell/layout/project-avatar-state.ts index ec310cf3a83..7a6010229c3 100644 --- a/packages/app/src/shell/layout/project-avatar-state.ts +++ b/packages/app/src/shell/layout/project-avatar-state.ts @@ -2,6 +2,7 @@ import { createMemo, type Accessor } from "solid-js" import type { SessionInfo } from "@opencode/client/promise" import { useGlobal, useServerCtx, type ServerCtx } from "@/runtime/server/runtime" import { sessionPermissionRequest, sessionFormRequest } from "@/session/requests/session-request-tree" +import { needsUser } from "@/session/requests/auto-approve-machine" import { ServerConnection } from "@/runtime/server/registry" import { projectSettingsKey, useSettings } from "@/settings/model" @@ -19,18 +20,17 @@ export function useSessionTabAvatarState( const connection = createMemo(() => global.servers.list().find((item) => ServerConnection.key(item) === server())) const serverCtx = useServerCtx(connection) - const requests = createMemo(() => { + const needsAttention = createMemo(() => { const ctx = serverCtx() - if (!ctx) return { permission: false, form: false } + if (!ctx) return false const project = projectSettingsKey(ctx.sdk.scope, ctx.data.session.get(sessionId())?.projectID) + const requests = sessionRequests(ctx, sessionId(), !!root?.(), settings.permissions.autoApproves(project)) - return sessionRequests(ctx.data, sessionId(), !!root?.(), settings.permissions.autoApproves(project)) + return !!requests.permission || !!requests.form }) - const needsAttention = createMemo(() => requests().permission || requests().form) - const unread = createMemo( () => needsAttention() || (serverCtx()?.notification.session.unread(sessionId(), snapshot?.()) ?? false), ) @@ -53,8 +53,11 @@ export function useSessionTabAvatarState( return { unread, loading, attention: needsAttention } } -// Pending permission and form requests anywhere in the session (or its family, for roots). -export function sessionRequests(data: ServerData, sessionId: string, root: boolean, autoApprove: boolean) { +// The pending permission and form request that waits on the user anywhere in the session (or its family, for roots). +// A permission counts by the same rule as the dock, so one auto-approve gave up on still marks the session. +export function sessionRequests(ctx: ServerCtx, sessionId: string, root: boolean, autoApprove: boolean) { + const data = ctx.data + const sessions = root ? [...new Set([sessionId, ...data.session.family(sessionId)])].flatMap((id) => { const info = data.session.get(id) @@ -64,11 +67,15 @@ export function sessionRequests(data: ServerData, sessionId: string, root: boole : data.session.list() return { - permission: !autoApprove && !!sessionPermissionRequest(sessions, data.session.permission.list, sessionId), - form: !!sessionFormRequest(sessions, data.session.form.list, sessionId), + permission: sessionPermissionRequest(sessions, data.session.permission.list, sessionId, (item) => + needsUser(ctx.autoApprove.state(item.id), autoApprove), + ), + form: sessionFormRequest(sessions, data.session.form.list, sessionId), } } +export type SessionRequests = ReturnType + // Like the TUI, work waiting in the inbox counts as busy; parked synthetic context does not. export function sessionBusy(data: ServerData, sessionId: string) { return ( diff --git a/packages/app/src/shell/titlebar/inbox-model.ts b/packages/app/src/shell/titlebar/inbox-model.ts index 2e2e00a189f..257c360fdd0 100644 --- a/packages/app/src/shell/titlebar/inbox-model.ts +++ b/packages/app/src/shell/titlebar/inbox-model.ts @@ -8,7 +8,7 @@ import { useLanguage } from "@/runtime/i18n/language" import { projectSettingsKey, useSettings } from "@/settings/model" import { useLayout } from "@/shell/state/layout" import { useTabs } from "@/shell/tabs/tabs" -import { sessionBusy, sessionRequests } from "@/shell/layout/project-avatar-state" +import { sessionBusy, sessionRequests, type SessionRequests } from "@/shell/layout/project-avatar-state" import { pathKey } from "@/workspaces/path-key" import { parseHomeSessionIndex } from "@/home/sessions/index" import { sessionFormRequest, sessionPermissionRequest, sessionTreeIDs } from "@/session/requests/session-request-tree" @@ -153,10 +153,7 @@ export function createSessionInbox(options: { archived?: boolean } = {}) { ), ) - const status = (session: SessionInfo): InboxStatus => { - const project = projectSettingsKey(item.ctx.sdk.scope, session.projectID) - const requests = sessionRequests(data, session.id, true, settings.permissions.autoApproves(project)) - + const status = (session: SessionInfo, requests: SessionRequests): InboxStatus => { if (requests.permission) return "approval" if (requests.form) return "input" @@ -168,15 +165,13 @@ export function createSessionInbox(options: { archived?: boolean } = {}) { return session.outcome === "failed" ? "failed" : "done" } - const since = (value: InboxStatus, session: SessionInfo) => { + // A waiting session ages from the request it shows, not from one auto-approve still answers. + const since = (value: InboxStatus, session: SessionInfo, requests: SessionRequests) => { if (value === "working") return workingSince(session) if (!inboxBusy(value)) return session.time.idle ?? session.time.updated - const request = - value === "approval" - ? sessionPermissionRequest(data.session.list(), data.session.permission.list, session.id) - : sessionFormRequest(data.session.list(), data.session.form.list, session.id) + const request = value === "approval" ? requests.permission : requests.form return (request && requestTimes[`${server}\0${request.id}`]) ?? workingSince(session) ?? session.time.updated } @@ -193,7 +188,9 @@ export function createSessionInbox(options: { archived?: boolean } = {}) { return data.session .apply([...sessions.values()].filter((session) => !session.parentID)) .map((session): InboxEntry => { - const value = status(session) + const project = projectSettingsKey(item.ctx.sdk.scope, session.projectID) + const requests = sessionRequests(item.ctx, session.id, true, settings.permissions.autoApproves(project)) + const value = status(session, requests) return { key: `${server}\0${session.id}`, @@ -202,7 +199,7 @@ export function createSessionInbox(options: { archived?: boolean } = {}) { ctx: item.ctx, session, status: value, - since: since(value, session), + since: since(value, session, requests), } }) })