Skip to content

fix(session): stop the background activity indicator going stale #431

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chriswritescode-dev merged 3 commits into from
Oct 15, 2026
Merged

fix(session): stop the background activity indicator going stale #431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
37b4ab8
fix(background-activity): reconcile shells for directories subscribed…
Oct 15, 2026
74a2f6f
fix(session): follow a moved session's new directory without a reload
Oct 15, 2026
1b2665b
fix(background-activity): address review on resync scope and stale su…
chriswritescode-dev Oct 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 113 additions & 1 deletion e2e/background-activity.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { execFileSync } from 'node:child_process'
import fs from 'node:fs'
import path from 'node:path'
import type { APIRequestContext, Page } from '@playwright/test'
import { test, expect } from './fixtures'
import { E2E_TMP } from './env'

const SHELL_COMMAND = 'sleep 300'

Expand All @@ -19,7 +23,7 @@ async function requireOk(response: Awaited<ReturnType<APIRequestContext['post']>
if (!response.ok()) {
throw new Error(`${label} failed: ${response.status()} ${await response.text()}`)
}
return response.json()
return response.status() === 204 ? null : response.json()
}

async function createSession(request: APIRequestContext, directory: string, parentID?: string): Promise<string> {
Expand Down Expand Up @@ -54,12 +58,44 @@ async function removeShell(request: APIRequestContext, directory: string, shellI
}

const createdShells: Array<{ directory: string; id: string }> = []
const createdDirectories: string[] = []
let directoryCounter = 0

function createUnregisteredDirectory(name: string): string {
directoryCounter += 1
const directory = path.join(E2E_TMP, `background-${name}-${process.pid}-${directoryCounter}`)
fs.mkdirSync(directory, { recursive: true })
fs.writeFileSync(path.join(directory, 'README.md'), '# background activity fixture\n')

const git = (...args: string[]) => execFileSync('git', args, { cwd: directory, stdio: 'pipe' })
git('-c', 'init.defaultBranch=main', 'init', '-q')
git('-c', 'user.name=e2e', '-c', 'user.email=e2e@example.com', 'add', '-A')
git('-c', 'user.name=e2e', '-c', 'user.email=e2e@example.com', 'commit', '-q', '-m', 'init')

createdDirectories.push(directory)
return directory
}

function waitForDirectorySubscription(page: Page, directory: string, endpoint = '/api/sse/subscribe') {
return page.waitForResponse((response) =>
response.url().includes(endpoint) && (response.request().postData() ?? '').includes(directory),
)
}

async function navigateWithinApp(page: Page, url: string): Promise<void> {
await page.evaluate((target) => {
window.history.pushState({}, '', target)
window.dispatchEvent(new PopStateEvent('popstate'))
}, url)
}

test.afterEach(async ({ request }) => {
const shells = createdShells.splice(0)
const results = await Promise.allSettled(shells.map(({ directory, id }) => removeShell(request, directory, id)))
const failures = results.flatMap((result) => (result.status === 'rejected' ? [String(result.reason)] : []))
expect(failures, 'background shell cleanup failed').toEqual([])

createdDirectories.splice(0).forEach((directory) => fs.rmSync(directory, { recursive: true, force: true }))
})

test.describe('session background activity', () => {
Expand Down Expand Up @@ -138,4 +174,80 @@ test.describe('session background activity', () => {
await expect(backgroundIndicator(page)).toHaveCount(0)
await expect(backgroundStatus(page)).toHaveCount(0)
})

test('shows the indicator for a shell started in an unregistered session directory before page load', async ({
page,
request,
repo,
}) => {
const directory = createUnregisteredDirectory('preload')
const sessionId = await createSession(request, directory)
const shellId = await startBackgroundShell(request, directory, sessionId)
createdShells.push({ directory, id: shellId })

await page.goto(`/repos/${repo.id}/sessions/${sessionId}`)
await expect(page.getByPlaceholder('Send a message...')).toBeVisible()

await expect(backgroundIndicator(page)).toBeVisible()

await removeShell(request, directory, shellId)

await expect(backgroundIndicator(page)).toBeHidden()
})

test('clears the indicator for a shell that exits while an unregistered session directory is unsubscribed', async ({
page,
request,
repo,
}) => {
const directory = createUnregisteredDirectory('stale')
const sessionId = await createSession(request, directory)
const subscribed = waitForDirectorySubscription(page, directory)

await page.goto(`/repos/${repo.id}/sessions/${sessionId}`)
await expect(page.getByPlaceholder('Send a message...')).toBeVisible()
expect((await subscribed).ok()).toBe(true)

const shellId = await startBackgroundShell(request, directory, sessionId)
createdShells.push({ directory, id: shellId })

await expect(backgroundIndicator(page)).toBeVisible()

const unsubscribed = waitForDirectorySubscription(page, directory, '/api/sse/unsubscribe')
await navigateWithinApp(page, '/')
expect((await unsubscribed).ok()).toBe(true)
await removeShell(request, directory, shellId)
await page.waitForTimeout(3_000)
await navigateWithinApp(page, `/repos/${repo.id}/sessions/${sessionId}`)

await expect(page.getByPlaceholder('Send a message...')).toBeVisible()
await expect(backgroundIndicator(page)).toBeHidden({ timeout: 20_000 })
})

test('follows a session moved to another directory without a reload', async ({ page, request, repo }) => {
const source = createUnregisteredDirectory('move-source')
const target = createUnregisteredDirectory('move-target')
const sessionId = await createSession(request, source)
const subscribedToSource = waitForDirectorySubscription(page, source)

await page.goto(`/repos/${repo.id}/sessions/${sessionId}`)
await expect(page.getByPlaceholder('Send a message...')).toBeVisible()
expect((await subscribedToSource).ok()).toBe(true)

const subscribedToTarget = waitForDirectorySubscription(page, target)
await requireOk(
await request.post(`/api/opencode/api/session/${sessionId}/move`, { data: { directory: target } }),
'move session',
)
expect((await subscribedToTarget).ok()).toBe(true)

const shellId = await startBackgroundShell(request, target, sessionId)
createdShells.push({ directory: target, id: shellId })

await expect(backgroundIndicator(page)).toBeVisible()

await removeShell(request, target, shellId)

await expect(backgroundIndicator(page)).toBeHidden()
})
})
119 changes: 104 additions & 15 deletions frontend/src/contexts/EventContext.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ describe('EventProvider permissions and forms', () => {

const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1]
const onEvent = lastSubscribeCall[0].onEvent as (data: unknown) => void
const onResync = lastSubscribeCall[0].onResync as (() => void) | undefined
const onResync = lastSubscribeCall[0].onResync as ((directories: string[]) => void) | undefined

act(() => {
onEvent({ type: 'permission.asked', data: pendingPermission, directory: '/repo' })
Expand All @@ -470,7 +470,7 @@ describe('EventProvider permissions and forms', () => {
})

act(() => {
onResync?.()
onResync?.([])
})

await waitFor(() => {
Expand All @@ -492,7 +492,7 @@ describe('EventProvider permissions and forms', () => {

const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1]
const onEvent = lastSubscribeCall[0].onEvent as (data: unknown) => void
const onResync = lastSubscribeCall[0].onResync as (() => void) | undefined
const onResync = lastSubscribeCall[0].onResync as ((directories: string[]) => void) | undefined

act(() => {
onEvent({ type: 'permission.asked', data: pendingPermission, directory: '/repo' })
Expand All @@ -505,7 +505,7 @@ describe('EventProvider permissions and forms', () => {
})

act(() => {
onResync?.()
onResync?.([])
})

await waitFor(() => {
Expand All @@ -531,7 +531,7 @@ describe('EventProvider permissions and forms', () => {

const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1]
const onEvent = lastSubscribeCall[0].onEvent as (data: unknown) => void
const onResync = lastSubscribeCall[0].onResync as (() => void) | undefined
const onResync = lastSubscribeCall[0].onResync as ((directories: string[]) => void) | undefined

act(() => {
onEvent({ type: 'permission.asked', data: pendingPermission, directory: '/worktrees/wt-1' })
Expand All @@ -542,7 +542,7 @@ describe('EventProvider permissions and forms', () => {
})

act(() => {
onResync?.()
onResync?.([])
})

await waitFor(() => {
Expand Down Expand Up @@ -852,10 +852,10 @@ describe('EventProvider permissions and forms', () => {
invalidateQueries.mockClear()

const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1]
const onResync = lastSubscribeCall[0].onResync as (() => void) | undefined
const onResync = lastSubscribeCall[0].onResync as ((directories: string[]) => void) | undefined

act(() => {
onResync?.()
onResync?.([])
})

await waitFor(() => {
Expand Down Expand Up @@ -939,10 +939,10 @@ describe('EventProvider permissions and forms', () => {
mocks.listPendingForms.mockClear()

const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1]
const onResync = lastSubscribeCall[0].onResync as (() => void) | undefined
const onResync = lastSubscribeCall[0].onResync as ((directories: string[]) => void) | undefined

act(() => {
onResync?.()
onResync?.([])
})

expect(mocks.listPendingPermissions).toHaveBeenCalledWith('/repo-a')
Expand Down Expand Up @@ -1210,15 +1210,15 @@ describe('EventProvider permissions and forms', () => {

const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1]
const onEvent = lastSubscribeCall[0].onEvent as (data: unknown) => void
const onResync = lastSubscribeCall[0].onResync as (() => void) | undefined
const onResync = lastSubscribeCall[0].onResync as ((directories: string[]) => void) | undefined

act(() => {
onEvent({ type: 'session.execution.started', data: { sessionID: 'child-1' }, directory: '/repo' })
})
expect(useSessionStatus.getState().getStatus('child-1')).toEqual({ type: 'busy' })

act(() => {
onResync?.()
onResync?.([])
})

await waitFor(() => {
Expand Down Expand Up @@ -1865,11 +1865,18 @@ describe('EventProvider permissions and forms', () => {

render(<Harness />, { wrapper: createWrapper() })

await waitFor(() => expect(mocks.listShells).toHaveBeenCalledWith('/repo'))
await waitFor(() => expect(mocks.subscribeGlobalMonitor).toHaveBeenCalled())

const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1]
const onDirectoriesSubscribed = lastSubscribeCall[0].onDirectoriesSubscribed as (directories: string[]) => void
const onEvent = lastSubscribeCall[0].onEvent as (data: unknown) => void

act(() => {
onDirectoriesSubscribed(['/repo'])
})

await waitFor(() => expect(mocks.listShells).toHaveBeenCalledWith('/repo'))

act(() => {
onEvent({ type: 'shell.exited', data: { id: 'shell-1', status: 'exited' }, directory: '/repo' })
})
Expand All @@ -1889,11 +1896,18 @@ describe('EventProvider permissions and forms', () => {

render(<Harness />, { wrapper: createWrapper() })

await waitFor(() => expect(mocks.listShells).toHaveBeenCalledWith('/repo'))
await waitFor(() => expect(mocks.subscribeGlobalMonitor).toHaveBeenCalled())

const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1]
const onDirectoriesSubscribed = lastSubscribeCall[0].onDirectoriesSubscribed as (directories: string[]) => void
const onEvent = lastSubscribeCall[0].onEvent as (data: unknown) => void

act(() => {
onDirectoriesSubscribed(['/repo'])
})

await waitFor(() => expect(mocks.listShells).toHaveBeenCalledWith('/repo'))

act(() => {
onEvent({
type: 'shell.created',
Expand All @@ -1913,7 +1927,7 @@ describe('EventProvider permissions and forms', () => {
})
})

it('resolves ancestry for a shell owner from the directory snapshot', async () => {
it('resolves ancestry for a shell owner from a newly subscribed directory snapshot', async () => {
mocks.listRepos.mockResolvedValue([{ id: 123, fullPath: '/repo' }])
mocks.listShells.mockResolvedValue([
{ id: 'shell-1', status: 'running', metadata: { sessionID: 'child-1' } },
Expand All @@ -1926,6 +1940,15 @@ describe('EventProvider permissions and forms', () => {

render(<Harness />, { wrapper: createWrapper() })

await waitFor(() => expect(mocks.subscribeGlobalMonitor).toHaveBeenCalled())

const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1]
const onDirectoriesSubscribed = lastSubscribeCall[0].onDirectoriesSubscribed as (directories: string[]) => void

act(() => {
onDirectoriesSubscribed(['/repo'])
})

await waitFor(() => {
expect(useSessionStatus.getState().parents.get('child-1')).toBe('parent-1')
})
Expand Down Expand Up @@ -1967,6 +1990,12 @@ describe('EventProvider permissions and forms', () => {
it('syncs running shells for a directory whose repo arrives after connect', async () => {
const reposDeferred = createDeferred<Array<{ id: number; fullPath: string }>>()
mocks.listRepos.mockReturnValueOnce(reposDeferred.promise)
mocks.subscribeGlobalMonitor.mockImplementation((input: { onDirectoriesSubscribed?: (directories: string[]) => void }) => ({
dispose: vi.fn(),
updateDirectories: (directories: string[]) => input.onDirectoriesSubscribed?.(directories),
reconnect: vi.fn(),
reportVisibility: vi.fn(),
}))

render(<Harness />, { wrapper: createWrapper() })

Expand All @@ -1988,4 +2017,64 @@ describe('EventProvider permissions and forms', () => {

await waitFor(() => expect(mocks.listShells).toHaveBeenCalledWith('/repo'))
})

it('reconciles newly subscribed directories reported by the stream', async () => {
mocks.listPendingPermissions.mockResolvedValue([pendingPermission])
mocks.listPendingForms.mockResolvedValue([pendingForm])

render(<Harness />, { wrapper: createWrapper() })

await waitFor(() => expect(mocks.subscribeGlobalMonitor).toHaveBeenCalled())

const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1]
const onDirectoriesSubscribed = lastSubscribeCall[0].onDirectoriesSubscribed as (directories: string[]) => void

act(() => {
useSessionStatus.getState().setShellRunning('shell-stale', 'session-1', '/session-dir')
})
expect(useSessionStatus.getState().runningShells.has('shell-stale')).toBe(true)

await act(async () => {
onDirectoriesSubscribed(['/session-dir'])
})

await waitFor(() => {
expect(mocks.listShells).toHaveBeenCalledWith('/session-dir')
expect(mocks.listPendingPermissions).toHaveBeenCalledWith('/session-dir')
expect(mocks.listPendingForms).toHaveBeenCalledWith('/session-dir')
})
expect(useSessionStatus.getState().runningShells.has('shell-stale')).toBe(false)
expect(screen.getByTestId('permission-count').textContent).toBe('1')
expect(screen.getByTestId('count').textContent).toBe('1')
})

it('reconciles subscribed directories reported by a resync even when they are not tracked', async () => {
mocks.listPendingPermissions.mockResolvedValue([pendingPermission])
mocks.listPendingForms.mockResolvedValue([pendingForm])

render(<Harness />, { wrapper: createWrapper() })

await waitFor(() => expect(mocks.subscribeGlobalMonitor).toHaveBeenCalled())

const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1]
const onResync = lastSubscribeCall[0].onResync as ((directories: string[]) => void) | undefined

act(() => {
useSessionStatus.getState().setShellRunning('shell-stale', 'session-1', '/unregistered-dir')
})
expect(useSessionStatus.getState().runningShells.has('shell-stale')).toBe(true)

await act(async () => {
onResync?.(['/unregistered-dir'])
})

await waitFor(() => {
expect(mocks.listShells).toHaveBeenCalledWith('/unregistered-dir')
expect(mocks.listPendingPermissions).toHaveBeenCalledWith('/unregistered-dir')
expect(mocks.listPendingForms).toHaveBeenCalledWith('/unregistered-dir')
})
expect(useSessionStatus.getState().runningShells.has('shell-stale')).toBe(false)
expect(screen.getByTestId('permission-count').textContent).toBe('1')
expect(screen.getByTestId('count').textContent).toBe('1')
})
})
Loading