Skip to content

Commit b0361cd

Browse files
Merge pull request #390 from chriswritescode-dev/fix/expired-permission-requests
fix(permissions): clear expired permission requests
2 parents bf99038 + fa0cde5 commit b0361cd

4 files changed

Lines changed: 148 additions & 8 deletions

File tree

‎frontend/src/api/opencode.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,21 @@ describe('OpenCode facade', () => {
392392
expect((failure as FetchError).message).toBe('Session not found')
393393
})
394394

395+
it('preserves the declared error tag as FetchError.code', async () => {
396+
fetchMock.mockResolvedValue(
397+
new Response(
398+
JSON.stringify({ _tag: 'PermissionNotFoundError', sessionID: 'ses_1', requestID: 'per_1', message: 'Permission request not found' }),
399+
{ status: 404, headers: { 'Content-Type': 'application/json' } },
400+
),
401+
)
402+
403+
const failure = await replyPermission('ses_1', 'per_1', 'once').catch((error: unknown) => error)
404+
405+
expect(failure).toBeInstanceOf(FetchError)
406+
expect((failure as FetchError).statusCode).toBe(404)
407+
expect((failure as FetchError).code).toBe('PermissionNotFoundError')
408+
})
409+
395410
it('maps a declared V2 conflict error to 409', async () => {
396411
fetchMock.mockResolvedValue(
397412
new Response(

‎frontend/src/components/repo/WorktreeSessionGroups.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ export function WorktreeSessionGroups({
179179
<div className="flex min-w-0 items-center gap-2">
180180
<span className="truncate text-sm font-medium">{label}</span>
181181
{sourceLabel && source && (
182-
<span className={`shrink-0 rounded border px-1.5 text-[10px] leading-4 ${OWNER_BADGE_CLASS[source]}`}>{sourceLabel}</span>
182+
<span className={`shrink-0 rounded-none border px-1.5 text-[10px] leading-4 ${OWNER_BADGE_CLASS[source]}`}>{sourceLabel}</span>
183183
)}
184184
{isInUse && (
185185
<span className="inline-flex shrink-0 items-center gap-1 rounded-full border border-success/40 bg-success/15 px-1.5 text-[10px] leading-4 text-success">

‎frontend/src/contexts/EventContext.test.tsx‎

Lines changed: 93 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
22
import { act, render, screen, waitFor } from '@testing-library/react'
33
import userEvent from '@testing-library/user-event'
4-
import type { ReactNode } from 'react'
4+
import { useState, type ReactNode } from 'react'
55
import { MemoryRouter, useLocation } from 'react-router-dom'
66
import { beforeEach, describe, expect, it, vi } from 'vitest'
77
import type { FormInfo, PermissionRequest } from '@opencode-manager/shared/opencode'
8+
import { FetchError } from '@opencode-manager/shared'
9+
import { showToast } from '@/lib/toast'
810
import { useSessionStatus } from '@/stores/sessionStatusStore'
911
import { changeWalkthroughQueryKey } from '@/hooks/useChangeWalkthrough'
1012
import { EventProvider, useEventContext, useForms, usePermissions, useSSEHealth } from './EventContext'
@@ -95,9 +97,14 @@ function Harness() {
9597
const { current, pendingCount, syncForSession, navigateToCurrent, cancel, reply, getForSession } = useForms()
9698
const permissions = usePermissions()
9799
const location = useLocation()
100+
const [rejection, setRejection] = useState('none')
101+
const recordRejection = (action: Promise<void>) => {
102+
action.catch((error: unknown) => setRejection(error instanceof FetchError ? error.code ?? 'unknown' : 'unknown'))
103+
}
98104

99105
return (
100106
<div>
107+
<div data-testid="rejection">{rejection}</div>
101108
<div data-testid="count">{pendingCount}</div>
102109
<div data-testid="current">{current?.id ?? 'none'}</div>
103110
<div data-testid="for-session-1">{getForSession('session-1')?.id ?? 'none'}</div>
@@ -116,9 +123,9 @@ function Harness() {
116123
<button onClick={() => syncForSession('/repo', 'session-1')}>Sync</button>
117124
<button onClick={() => permissions.syncForSession('/repo', 'session-1')}>Sync Permissions</button>
118125
<button onClick={navigateToCurrent}>Navigate</button>
119-
<button onClick={() => current && cancel(current.id)}>Dismiss</button>
120-
<button onClick={() => current && reply(current.id, { q0: 'Yes' })}>Reply</button>
121-
<button onClick={() => permissions.current && permissions.respond(permissions.current.id, permissions.current.sessionID, 'reject')}>Reject Permission</button>
126+
<button onClick={() => current && recordRejection(cancel(current.id))}>Dismiss</button>
127+
<button onClick={() => current && recordRejection(reply(current.id, { q0: 'Yes' }))}>Reply</button>
128+
<button onClick={() => permissions.current && recordRejection(permissions.respond(permissions.current.id, permissions.current.sessionID, 'reject'))}>Reject Permission</button>
122129
<button onClick={() => permissions.current && permissions.respond(permissions.current.id, permissions.current.sessionID, 'reject', 'not allowed')}>Reject Permission With Reason</button>
123130
</div>
124131
)
@@ -248,6 +255,88 @@ describe('EventProvider permissions and forms', () => {
248255
})
249256
})
250257

258+
it('removes a permission the server no longer knows about when replying', async () => {
259+
mocks.listPendingPermissions.mockResolvedValue([pendingPermission])
260+
mocks.replyPermission.mockRejectedValue(new FetchError('Permission request not found', 404, 'PermissionNotFoundError'))
261+
262+
render(<Harness />, { wrapper: createWrapper() })
263+
264+
await userEvent.click(screen.getByRole('button', { name: 'Sync Permissions' }))
265+
266+
await waitFor(() => expect(screen.getByTestId('permission-count')).toHaveTextContent('1'))
267+
268+
await userEvent.click(screen.getByRole('button', { name: 'Reject Permission' }))
269+
270+
await waitFor(() => {
271+
expect(screen.getByTestId('permission-count')).toHaveTextContent('0')
272+
expect(screen.getByTestId('permission-current')).toHaveTextContent('none')
273+
})
274+
expect(showToast.info).toHaveBeenCalledWith('Permission request expired')
275+
expect(screen.getByTestId('rejection')).toHaveTextContent('none')
276+
})
277+
278+
it('keeps a permission and rejects when the reply fails for another reason', async () => {
279+
mocks.listPendingPermissions.mockResolvedValue([pendingPermission])
280+
mocks.replyPermission.mockRejectedValue(new FetchError('Session not found', 404, 'SessionNotFoundError'))
281+
282+
render(<Harness />, { wrapper: createWrapper() })
283+
284+
await userEvent.click(screen.getByRole('button', { name: 'Sync Permissions' }))
285+
286+
await waitFor(() => expect(screen.getByTestId('permission-count')).toHaveTextContent('1'))
287+
288+
await userEvent.click(screen.getByRole('button', { name: 'Reject Permission' }))
289+
290+
await waitFor(() => expect(screen.getByTestId('rejection')).toHaveTextContent('SessionNotFoundError'))
291+
expect(screen.getByTestId('permission-count')).toHaveTextContent('1')
292+
expect(screen.getByTestId('permission-current')).toHaveTextContent('permission-1')
293+
expect(showToast.info).not.toHaveBeenCalled()
294+
})
295+
296+
it.each([
297+
['Reply', 'replyForm'],
298+
['Dismiss', 'cancelForm'],
299+
] as const)('removes a form the server no longer knows about on %s', async (button, mock) => {
300+
mocks.listPendingForms.mockResolvedValue([pendingForm])
301+
mocks[mock].mockRejectedValue(new FetchError('Form not found', 404, 'FormNotFoundError'))
302+
303+
render(<Harness />, { wrapper: createWrapper() })
304+
305+
await userEvent.click(screen.getByRole('button', { name: 'Sync' }))
306+
307+
await waitFor(() => expect(screen.getByTestId('count')).toHaveTextContent('1'))
308+
309+
await userEvent.click(screen.getByRole('button', { name: button }))
310+
311+
await waitFor(() => {
312+
expect(screen.getByTestId('count')).toHaveTextContent('0')
313+
expect(screen.getByTestId('current')).toHaveTextContent('none')
314+
})
315+
expect(showToast.info).toHaveBeenCalledWith('Form expired')
316+
expect(screen.getByTestId('rejection')).toHaveTextContent('none')
317+
})
318+
319+
it.each([
320+
['Reply', 'replyForm'],
321+
['Dismiss', 'cancelForm'],
322+
] as const)('keeps a form and rejects when %s fails for another reason', async (button, mock) => {
323+
mocks.listPendingForms.mockResolvedValue([pendingForm])
324+
mocks[mock].mockRejectedValue(new FetchError('Form already settled', 409, 'FormAlreadySettledError'))
325+
326+
render(<Harness />, { wrapper: createWrapper() })
327+
328+
await userEvent.click(screen.getByRole('button', { name: 'Sync' }))
329+
330+
await waitFor(() => expect(screen.getByTestId('count')).toHaveTextContent('1'))
331+
332+
await userEvent.click(screen.getByRole('button', { name: button }))
333+
334+
await waitFor(() => expect(screen.getByTestId('rejection')).toHaveTextContent('FormAlreadySettledError'))
335+
expect(screen.getByTestId('count')).toHaveTextContent('1')
336+
expect(screen.getByTestId('current')).toHaveTextContent('form-1')
337+
expect(showToast.info).not.toHaveBeenCalled()
338+
})
339+
251340
it('forwards an optional rejection message to the facade', async () => {
252341
mocks.listPendingPermissions.mockResolvedValue([pendingPermission])
253342

‎frontend/src/contexts/EventContext.tsx‎

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
replyPermission,
1212
} from '@/api/opencode'
1313
import { listRepos } from '@/api/repos'
14+
import { FetchError } from '@opencode-manager/shared'
1415
import type { FormAnswer, FormInfo, PermissionRequest, V2Event } from '@opencode-manager/shared/opencode'
1516
import type { PermissionResponse, SSHHostKeyRequest, Repo } from '@/api/types'
1617
import { showToast } from '@/lib/toast'
@@ -94,6 +95,11 @@ interface EventContextValue {
9495
permissions: {
9596
current: PermissionRequest | null
9697
pendingCount: number
98+
/**
99+
* Replies to a permission request and removes it from the queue. Resolves, after showing an
100+
* "expired" toast, when the server no longer knows the request (`PermissionNotFoundError`);
101+
* rejects on any other failure and keeps the request queued.
102+
*/
97103
respond: (
98104
permissionID: string,
99105
sessionID: string,
@@ -111,7 +117,15 @@ interface EventContextValue {
111117
forms: {
112118
current: FormInfo | null
113119
pendingCount: number
120+
/**
121+
* Answers a form and removes it from the queue. Resolves, after showing an "expired" toast,
122+
* when the server no longer knows the form (`FormNotFoundError`); rejects on any other failure.
123+
*/
114124
reply: (formID: string, answer: FormAnswer) => Promise<void>
125+
/**
126+
* Dismisses a form on the server and removes it from the queue, with the same
127+
* resolve-on-`FormNotFoundError` contract as `reply`.
128+
*/
115129
cancel: (formID: string) => Promise<void>
116130
dismiss: (formID: string, sessionID?: string) => void
117131
getForSession: (sessionID: string) => FormInfo | null
@@ -123,6 +137,24 @@ interface EventContextValue {
123137
getRepoIdForSession: (sessionID: string) => number | null
124138
}
125139

140+
/**
141+
* Sends a reply to a pending server request and treats the request as settled when the server
142+
* reports it no longer exists (`goneCode`), showing `expiredMessage` instead of rejecting.
143+
* Every other failure is rethrown.
144+
*/
145+
async function settlePendingReply(
146+
reply: () => Promise<void>,
147+
goneCode: string,
148+
expiredMessage: string,
149+
): Promise<void> {
150+
try {
151+
await reply()
152+
} catch (error) {
153+
if (!(error instanceof FetchError && error.code === goneCode)) throw error
154+
showToast.info(expiredMessage)
155+
}
156+
}
157+
126158
const EventContext = createContext<EventContextValue | null>(null)
127159

128160
export function EventProvider({ children }: { children: React.ReactNode }) {
@@ -284,21 +316,25 @@ export function EventProvider({ children }: { children: React.ReactNode }) {
284316
response: PermissionResponse,
285317
message?: string,
286318
) => {
287-
await replyPermission(sessionID, permissionID, response, message)
319+
await settlePendingReply(
320+
() => replyPermission(sessionID, permissionID, response, message),
321+
'PermissionNotFoundError',
322+
'Permission request expired',
323+
)
288324
removePermission(permissionID, sessionID)
289325
}, [removePermission])
290326

291327
const replyToForm = useCallback(async (formID: string, answer: FormAnswer) => {
292328
const form = Object.values(formsBySession).flat().find(f => f.id === formID)
293329
if (!form) throw new Error('Form not found')
294-
await replyForm(form.sessionID, formID, answer)
330+
await settlePendingReply(() => replyForm(form.sessionID, formID, answer), 'FormNotFoundError', 'Form expired')
295331
removeForm(formID, form.sessionID)
296332
}, [formsBySession, removeForm])
297333

298334
const cancelPendingForm = useCallback(async (formID: string) => {
299335
const form = Object.values(formsBySession).flat().find(f => f.id === formID)
300336
if (!form) throw new Error('Form not found')
301-
await cancelForm(form.sessionID, formID)
337+
await settlePendingReply(() => cancelForm(form.sessionID, formID), 'FormNotFoundError', 'Form expired')
302338
removeForm(formID, form.sessionID)
303339
}, [formsBySession, removeForm])
304340

0 commit comments

Comments
 (0)