mirror of
https://github.com/anomalyco/opencode.git
synced 2026-11-14 12:10:45 +03:00
fix(app): badge permission requests that auto-approve gave up on (#61736)
This commit is contained in:
committed by
GitHub
Unverified
parent
8f5d192810
commit
443878def0
@@ -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<void>()
|
||||
const arrived = Promise.withResolvers<void>()
|
||||
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,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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),
|
||||
)
|
||||
})
|
||||
|
||||
|
||||
@@ -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<typeof sessionRequests>
|
||||
|
||||
// Like the TUI, work waiting in the inbox counts as busy; parked synthetic context does not.
|
||||
export function sessionBusy(data: ServerData, sessionId: string) {
|
||||
return (
|
||||
|
||||
@@ -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),
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user