mirror of
https://github.com/stablyai/orca.git
synced 2026-10-24 00:04:01 +00:00
Harden document preview lifecycle and permissions
This commit is contained in:
@@ -3,7 +3,9 @@ import type { BrowserSessionProfile } from '../../shared/browser-workspace-types
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
handleGuestWillDownload: vi.fn(),
|
||||
noticeDocPreviewDownloadBlocked: vi.fn()
|
||||
noticeDocPreviewDownloadBlocked: vi.fn(),
|
||||
clearBrowserWebAuthnAccessHandlers: vi.fn(),
|
||||
installBrowserWebAuthnAccessHandlers: vi.fn()
|
||||
}))
|
||||
|
||||
type WillDownloadListener = (
|
||||
@@ -88,13 +90,13 @@ vi.mock('./browser-session-user-agent-mode', () => ({
|
||||
}))
|
||||
vi.mock('./browser-webauthn-access', () => ({
|
||||
allowsBrowserWebAuthnPermission: () => false,
|
||||
clearBrowserWebAuthnAccessHandlers: vi.fn(),
|
||||
installBrowserWebAuthnAccessHandlers: vi.fn()
|
||||
clearBrowserWebAuthnAccessHandlers: mocks.clearBrowserWebAuthnAccessHandlers,
|
||||
installBrowserWebAuthnAccessHandlers: mocks.installBrowserWebAuthnAccessHandlers
|
||||
}))
|
||||
|
||||
type PartitionPolicyInstaller = (
|
||||
profile: BrowserSessionProfile,
|
||||
options?: { downloads?: 'route' | 'deny' }
|
||||
options?: { downloads?: 'route' | 'deny'; permissions?: 'browser' | 'deny' }
|
||||
) => void
|
||||
|
||||
// Why imported per test rather than at the top: the installer remembers which partitions it has
|
||||
@@ -183,3 +185,51 @@ describe('partition download policy', () => {
|
||||
expect(mocks.handleGuestWillDownload).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
|
||||
describe('partition permission policy', () => {
|
||||
it('keeps ordinary browser partitions on the browser permission policy', async () => {
|
||||
const install = await loadInstaller()
|
||||
install(profileFor('persist:browsing-1'))
|
||||
|
||||
expect(mocks.installBrowserWebAuthnAccessHandlers).toHaveBeenCalledWith(
|
||||
sessionsByPartition.get('persist:browsing-1')
|
||||
)
|
||||
expect(mocks.clearBrowserWebAuthnAccessHandlers).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('denies every request and check on a strict partition without WebAuthn handlers', async () => {
|
||||
const install = await loadInstaller()
|
||||
install(profileFor('orca-doc-preview'), { permissions: 'deny' })
|
||||
const sess = sessionsByPartition.get('orca-doc-preview')
|
||||
if (!sess) {
|
||||
throw new Error('Expected the preview session')
|
||||
}
|
||||
const requestHandler = sess.setPermissionRequestHandler.mock.calls[0]?.[0] as (
|
||||
webContents: Electron.WebContents,
|
||||
permission: string,
|
||||
callback: (allowed: boolean) => void
|
||||
) => void
|
||||
const checkHandler = sess.setPermissionCheckHandler.mock.calls[0]?.[0] as (
|
||||
webContents: Electron.WebContents,
|
||||
permission: string
|
||||
) => boolean
|
||||
const displayMediaHandler = sess.setDisplayMediaRequestHandler.mock.calls[0]?.[0] as (
|
||||
request: Electron.DisplayMediaRequestHandlerHandlerRequest,
|
||||
callback: (streams: { video?: Electron.WebFrameMain; audio?: 'loopback' }) => void
|
||||
) => void
|
||||
|
||||
for (const permission of ['media', 'clipboard-read', 'notifications', 'fullscreen']) {
|
||||
let decision: boolean | null = null
|
||||
requestHandler({} as Electron.WebContents, permission, (allowed) => (decision = allowed))
|
||||
expect(decision).toBe(false)
|
||||
expect(checkHandler({} as Electron.WebContents, permission)).toBe(false)
|
||||
}
|
||||
expect(mocks.installBrowserWebAuthnAccessHandlers).not.toHaveBeenCalled()
|
||||
expect(mocks.clearBrowserWebAuthnAccessHandlers).toHaveBeenCalledWith(sess)
|
||||
let displayMediaDecision: { video?: Electron.WebFrameMain; audio?: 'loopback' } | null = null
|
||||
displayMediaHandler({} as Electron.DisplayMediaRequestHandlerHandlerRequest, (decision) => {
|
||||
displayMediaDecision = decision
|
||||
})
|
||||
expect(displayMediaDecision).toEqual({ video: undefined, audio: undefined })
|
||||
})
|
||||
})
|
||||
|
||||
@@ -57,10 +57,14 @@ function resolvePermissionNoticeUrl(
|
||||
|
||||
/** `route` hands the item to the owning page's download flow; `deny` cancels it before it starts. */
|
||||
export type BrowserPartitionDownloadPolicy = 'route' | 'deny'
|
||||
export type BrowserPartitionPermissionPolicy = 'browser' | 'deny'
|
||||
|
||||
export function installBrowserSessionPartitionPolicies(
|
||||
profile: BrowserSessionProfile,
|
||||
options?: { downloads?: BrowserPartitionDownloadPolicy }
|
||||
options?: {
|
||||
downloads?: BrowserPartitionDownloadPolicy
|
||||
permissions?: BrowserPartitionPermissionPolicy
|
||||
}
|
||||
): void {
|
||||
const { partition } = profile
|
||||
const sess = session.fromPartition(partition)
|
||||
@@ -75,57 +79,63 @@ export function installBrowserSessionPartitionPolicies(
|
||||
sess.setUserAgent(cleanUA)
|
||||
setupClientHintsOverride(sess, cleanUA)
|
||||
}
|
||||
sess.setPermissionRequestHandler((webContents, permission, callback, details) => {
|
||||
// Why: defer media to macOS TCC; denying at the session layer throws NotAllowedError even after the user granted Camera/Mic to the OS.
|
||||
if (permission === 'media') {
|
||||
// Capture before async handling; opaque frames cannot be attributed to a named site.
|
||||
const rawUrl = resolvePermissionNoticeUrl(webContents, details)
|
||||
void requestSystemMediaAccess(
|
||||
details as Electron.MediaAccessPermissionRequest | undefined
|
||||
).then(
|
||||
(granted) => {
|
||||
if (!granted) {
|
||||
if (options?.permissions === 'deny') {
|
||||
sess.setPermissionRequestHandler((_webContents, _permission, callback) => callback(false))
|
||||
sess.setPermissionCheckHandler(() => false)
|
||||
clearBrowserWebAuthnAccessHandlers(sess)
|
||||
} else {
|
||||
sess.setPermissionRequestHandler((webContents, permission, callback, details) => {
|
||||
// Why: defer media to macOS TCC; denying at the session layer throws NotAllowedError even after the user granted Camera/Mic to the OS.
|
||||
if (permission === 'media') {
|
||||
// Capture before async handling; opaque frames cannot be attributed to a named site.
|
||||
const rawUrl = resolvePermissionNoticeUrl(webContents, details)
|
||||
void requestSystemMediaAccess(
|
||||
details as Electron.MediaAccessPermissionRequest | undefined
|
||||
).then(
|
||||
(granted) => {
|
||||
if (!granted) {
|
||||
browserManager.notifyPermissionDenied({
|
||||
guestWebContentsId: webContents.id,
|
||||
permission,
|
||||
rawUrl
|
||||
})
|
||||
}
|
||||
callback(granted)
|
||||
},
|
||||
(error: unknown) => {
|
||||
console.error('[permissions] Browser media access failed:', error)
|
||||
browserManager.notifyPermissionDenied({
|
||||
guestWebContentsId: webContents.id,
|
||||
permission,
|
||||
rawUrl
|
||||
})
|
||||
callback(false)
|
||||
}
|
||||
callback(granted)
|
||||
},
|
||||
(error: unknown) => {
|
||||
console.error('[permissions] Browser media access failed:', error)
|
||||
browserManager.notifyPermissionDenied({
|
||||
guestWebContentsId: webContents.id,
|
||||
permission,
|
||||
rawUrl
|
||||
})
|
||||
callback(false)
|
||||
}
|
||||
)
|
||||
return
|
||||
}
|
||||
const allowed = isAutoGrantedBrowserSessionPermission(permission)
|
||||
if (!allowed) {
|
||||
const rawUrl = resolvePermissionNoticeUrl(webContents, details)
|
||||
browserManager.notifyPermissionDenied({
|
||||
guestWebContentsId: webContents.id,
|
||||
permission,
|
||||
rawUrl
|
||||
})
|
||||
}
|
||||
callback(allowed)
|
||||
})
|
||||
sess.setPermissionCheckHandler((_webContents, permission, _origin, details) => {
|
||||
if (permission === 'media') {
|
||||
return hasSystemMediaAccess(details?.mediaType)
|
||||
}
|
||||
if (allowsBrowserWebAuthnPermission(permission, details)) {
|
||||
return true
|
||||
}
|
||||
return isAutoGrantedBrowserSessionPermission(permission)
|
||||
})
|
||||
installBrowserWebAuthnAccessHandlers(sess)
|
||||
)
|
||||
return
|
||||
}
|
||||
const allowed = isAutoGrantedBrowserSessionPermission(permission)
|
||||
if (!allowed) {
|
||||
const rawUrl = resolvePermissionNoticeUrl(webContents, details)
|
||||
browserManager.notifyPermissionDenied({
|
||||
guestWebContentsId: webContents.id,
|
||||
permission,
|
||||
rawUrl
|
||||
})
|
||||
}
|
||||
callback(allowed)
|
||||
})
|
||||
sess.setPermissionCheckHandler((_webContents, permission, _origin, details) => {
|
||||
if (permission === 'media') {
|
||||
return hasSystemMediaAccess(details?.mediaType)
|
||||
}
|
||||
if (allowsBrowserWebAuthnPermission(permission, details)) {
|
||||
return true
|
||||
}
|
||||
return isAutoGrantedBrowserSessionPermission(permission)
|
||||
})
|
||||
installBrowserWebAuthnAccessHandlers(sess)
|
||||
}
|
||||
sess.setDisplayMediaRequestHandler((_request, callback) => {
|
||||
callback({ video: undefined, audio: undefined })
|
||||
})
|
||||
|
||||
@@ -2,7 +2,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
publishDocPreviewFailure: vi.fn(),
|
||||
boundGrantIdByGuest: new Map<object, string>()
|
||||
boundGrantIdByGuest: new Map<object, string>(),
|
||||
revocationListener: null as null | ((grant: { id: string }) => void)
|
||||
}))
|
||||
|
||||
vi.mock('./doc-preview-failure-notice', () => ({
|
||||
@@ -11,6 +12,12 @@ vi.mock('./doc-preview-failure-notice', () => ({
|
||||
vi.mock('./doc-preview-guest-policy', () => ({
|
||||
readDocPreviewGuestBoundGrantId: (guest: object) => mocks.boundGrantIdByGuest.get(guest) ?? null
|
||||
}))
|
||||
vi.mock('./doc-preview-grant-registry', () => ({
|
||||
onDocPreviewGrantRevoked: (listener: (grant: { id: string }) => void) => {
|
||||
mocks.revocationListener = listener
|
||||
return vi.fn()
|
||||
}
|
||||
}))
|
||||
|
||||
const GRANT_ID = 'a'.repeat(32)
|
||||
const OTHER_GRANT_ID = 'b'.repeat(32)
|
||||
@@ -32,6 +39,7 @@ async function loadNotifier(): Promise<(guest: Electron.WebContents) => void> {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mocks.boundGrantIdByGuest.clear()
|
||||
mocks.revocationListener = null
|
||||
// Why per test: the module remembers which grants it has already told the reader about.
|
||||
vi.resetModules()
|
||||
vi.useFakeTimers()
|
||||
@@ -94,6 +102,17 @@ describe('noticeDocPreviewDownloadBlocked', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('forgets a preview as soon as its grant is revoked', async () => {
|
||||
const notice = await loadNotifier()
|
||||
const guest = guestBoundTo(GRANT_ID)
|
||||
|
||||
notice(guest)
|
||||
mocks.revocationListener?.({ id: GRANT_ID })
|
||||
notice(guest)
|
||||
|
||||
expect(mocks.publishDocPreviewFailure).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
// The absence half of the first test: no shell is showing this contents, so there is no preview
|
||||
// to put a notice on. Without the presence tests above, this would pass on a module that never
|
||||
// published anything at all.
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { publishDocPreviewFailure } from './doc-preview-failure-notice'
|
||||
import { onDocPreviewGrantRevoked } from './doc-preview-grant-registry'
|
||||
import { readDocPreviewGuestBoundGrantId } from './doc-preview-guest-policy'
|
||||
|
||||
/**
|
||||
@@ -12,6 +13,8 @@ import { readDocPreviewGuestBoundGrantId } from './doc-preview-guest-policy'
|
||||
const NOTICE_MIN_INTERVAL_MS = 2_000
|
||||
const noticedAtByGrantId = new Map<string, number>()
|
||||
|
||||
onDocPreviewGrantRevoked((grant) => noticedAtByGrantId.delete(grant.id))
|
||||
|
||||
export function noticeDocPreviewDownloadBlocked(guest: Electron.WebContents): void {
|
||||
const grantId = readDocPreviewGuestBoundGrantId(guest)
|
||||
// Nothing to route a notice to: no shell is showing this contents as a preview.
|
||||
|
||||
@@ -261,8 +261,8 @@ describe('installDocPreviewProtocolHandler', () => {
|
||||
expect(cancelled(`orca-preview://${'a'.repeat(32)}/index.html`)).toBe(false)
|
||||
})
|
||||
|
||||
// Why: preview guests are webviews like any other, so they must not skip the deny-by-default
|
||||
// permission and display-media policy every browser partition gets.
|
||||
// Why: preview guests still use the shared installer for certificate, UA, permission and
|
||||
// download hooks, with a stricter decision than ordinary browsing partitions.
|
||||
it('applies the shared browser partition policies to the preview session', () => {
|
||||
installDocPreviewProtocolHandler()
|
||||
|
||||
@@ -272,14 +272,12 @@ describe('installDocPreviewProtocolHandler', () => {
|
||||
)
|
||||
})
|
||||
|
||||
// Why downloads and nothing else: the browser download flow attributes a file to the page that
|
||||
// asked for it, and a previewed document is no page. Routed, it would write remote-authored bytes
|
||||
// into this desktop's Downloads folder with no prompt and no tab to name as the source.
|
||||
it('asks for downloads to be denied on the preview partition', () => {
|
||||
it('denies downloads and ambient browser permissions on the preview partition', () => {
|
||||
installDocPreviewProtocolHandler()
|
||||
|
||||
expect(mocks.installBrowserSessionPartitionPolicies).toHaveBeenCalledWith(expect.anything(), {
|
||||
downloads: 'deny'
|
||||
downloads: 'deny',
|
||||
permissions: 'deny'
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -124,8 +124,8 @@ export function installDocPreviewProtocolHandler(): void {
|
||||
previewSession.webRequest.onBeforeRequest((details, callback) => {
|
||||
callback({ cancel: !isAllowedDocPreviewRequestUrl(details.url) })
|
||||
})
|
||||
// Why: preview guests are webviews like any other, so they inherit the same deny-by-default
|
||||
// permission, display-media and user-agent policy every browser partition gets.
|
||||
// Why: the shared installer still owns certificate, UA, download and permission hooks, while
|
||||
// preview content receives no ambient browser/device permissions.
|
||||
installBrowserSessionPartitionPolicies(
|
||||
{
|
||||
id: DOC_PREVIEW_PARTITION,
|
||||
@@ -139,6 +139,6 @@ export function installDocPreviewProtocolHandler(): void {
|
||||
// page to attribute the file to, and a previewed document is not one. Routed here it would
|
||||
// reserve a name in this desktop's Downloads folder and write remote-authored bytes into it
|
||||
// with nothing in the UI naming the tab that asked, and no prompt in front of it.
|
||||
{ downloads: 'deny' }
|
||||
{ downloads: 'deny', permissions: 'deny' }
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { getDefaultWorkspaceSession } from '../../shared/constants'
|
||||
import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types'
|
||||
import {
|
||||
mergeWorkspaceSessions,
|
||||
removeRepoFromWorkspaceSession
|
||||
@@ -84,6 +85,57 @@ describe('profile project session state', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('rekeys document-preview ownership during project transfer', () => {
|
||||
const session = {
|
||||
...getDefaultWorkspaceSession(),
|
||||
browserTabsByWorktree: {
|
||||
[REMOVED_WORKTREE_ID]: [
|
||||
{
|
||||
id: 'browser-1',
|
||||
worktreeId: REMOVED_WORKTREE_ID,
|
||||
docLocation: {
|
||||
kind: 'workspace-doc',
|
||||
worktreeId: REMOVED_WORKTREE_ID,
|
||||
filePath: '/removed/docs/report.html'
|
||||
}
|
||||
}
|
||||
]
|
||||
},
|
||||
browserPagesByWorkspace: {
|
||||
'browser-1': [
|
||||
{
|
||||
id: 'page-1',
|
||||
workspaceId: 'browser-1',
|
||||
worktreeId: REMOVED_WORKTREE_ID,
|
||||
docLocation: {
|
||||
kind: 'workspace-doc',
|
||||
worktreeId: REMOVED_WORKTREE_ID,
|
||||
filePath: '/removed/docs/report.html'
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
const result = extractSessionForTransfer(
|
||||
session as unknown as WorkspaceSessionState,
|
||||
REMOVED_REPO_ID,
|
||||
'repo-c'
|
||||
)
|
||||
const transferredWorktreeId = 'repo-c::/removed'
|
||||
|
||||
expect(result.browserTabsByWorktree?.[transferredWorktreeId]?.[0]?.docLocation).toEqual({
|
||||
kind: 'workspace-doc',
|
||||
worktreeId: transferredWorktreeId,
|
||||
filePath: '/removed/docs/report.html'
|
||||
})
|
||||
expect(result.browserPagesByWorkspace?.['browser-1']?.[0]?.docLocation).toEqual({
|
||||
kind: 'workspace-doc',
|
||||
worktreeId: transferredWorktreeId,
|
||||
filePath: '/removed/docs/report.html'
|
||||
})
|
||||
})
|
||||
|
||||
it('prunes terminal membership authority records with a removed repo', () => {
|
||||
const session = {
|
||||
...getDefaultWorkspaceSession(),
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
import { getDefaultWorkspaceSession } from '../../shared/constants'
|
||||
import type { ExecutionHostId } from '../../shared/execution-host'
|
||||
import type { BrowserPage, BrowserWorkspace } from '../../shared/browser-workspace-types'
|
||||
import type {
|
||||
BrowserPage,
|
||||
BrowserPageDocLocation,
|
||||
BrowserWorkspace
|
||||
} from '../../shared/browser-workspace-types'
|
||||
import { remapBrowserPageDocLocation } from '../../shared/browser-page-doc-location'
|
||||
import type { Tab, TabGroup } from '../../shared/tab-types'
|
||||
import type { TerminalTab } from '../../shared/terminal-tab-types'
|
||||
import type {
|
||||
@@ -175,6 +180,9 @@ function rekeyBrowserWorkspace(
|
||||
return {
|
||||
...structuredClone(workspace),
|
||||
worktreeId: rekeyWorktreeId(oldRepoId, newRepoId, workspace.worktreeId),
|
||||
...(workspace.docLocation
|
||||
? { docLocation: rekeyBrowserDocLocation(workspace.docLocation, oldRepoId, newRepoId) }
|
||||
: {}),
|
||||
// Why: both the session profile and the resolved partition string are
|
||||
// source-profile-scoped; carrying either across would point the restored
|
||||
// pane at a partition the target profile's allowlist rejects.
|
||||
@@ -186,10 +194,22 @@ function rekeyBrowserWorkspace(
|
||||
function rekeyBrowserPage(page: BrowserPage, oldRepoId: string, newRepoId: string): BrowserPage {
|
||||
return {
|
||||
...structuredClone(page),
|
||||
worktreeId: rekeyWorktreeId(oldRepoId, newRepoId, page.worktreeId)
|
||||
worktreeId: rekeyWorktreeId(oldRepoId, newRepoId, page.worktreeId),
|
||||
...(page.docLocation
|
||||
? { docLocation: rekeyBrowserDocLocation(page.docLocation, oldRepoId, newRepoId) }
|
||||
: {})
|
||||
}
|
||||
}
|
||||
|
||||
function rekeyBrowserDocLocation(
|
||||
location: BrowserPageDocLocation,
|
||||
oldRepoId: string,
|
||||
newRepoId: string
|
||||
): BrowserPageDocLocation {
|
||||
const nextWorktreeId = rekeyWorktreeId(oldRepoId, newRepoId, location.worktreeId)
|
||||
return remapBrowserPageDocLocation(location, location.worktreeId, nextWorktreeId)
|
||||
}
|
||||
|
||||
function copyBrowserPages(
|
||||
pagesByWorkspace: Record<string, BrowserPage[]> | undefined,
|
||||
workspaceIds: ReadonlySet<string>,
|
||||
|
||||
@@ -243,10 +243,33 @@ describe('Store.migrateWorktreeIdentity', () => {
|
||||
},
|
||||
activeFileIdByWorktree: { [OLD]: '/ws/cunner/a.ts' },
|
||||
browserTabsByWorktree: {
|
||||
[OLD]: [{ id: 'browser1', worktreeId: OLD, title: 'Browser', url: 'about:blank' }]
|
||||
[OLD]: [
|
||||
{
|
||||
id: 'browser1',
|
||||
worktreeId: OLD,
|
||||
title: 'Browser',
|
||||
url: 'about:blank',
|
||||
docLocation: {
|
||||
kind: 'workspace-doc',
|
||||
worktreeId: OLD,
|
||||
filePath: '/ws/cunner/docs/report.html'
|
||||
}
|
||||
}
|
||||
]
|
||||
},
|
||||
browserPagesByWorkspace: {
|
||||
browser1: [{ id: 'page1', workspaceId: 'browser1', worktreeId: OLD }]
|
||||
browser1: [
|
||||
{
|
||||
id: 'page1',
|
||||
workspaceId: 'browser1',
|
||||
worktreeId: OLD,
|
||||
docLocation: {
|
||||
kind: 'workspace-doc',
|
||||
worktreeId: OLD,
|
||||
filePath: '/ws/cunner/docs/report.html'
|
||||
}
|
||||
}
|
||||
]
|
||||
},
|
||||
activeBrowserTabIdByWorktree: { [OLD]: 'browser1' },
|
||||
activeTabTypeByWorktree: { [OLD]: 'browser' },
|
||||
@@ -327,6 +350,16 @@ describe('Store.migrateWorktreeIdentity', () => {
|
||||
expect(session.browserTabsByWorktree?.[OLD]).toBeUndefined()
|
||||
expect(session.browserTabsByWorktree?.[NEW]?.[0]?.worktreeId).toBe(NEW)
|
||||
expect(session.browserPagesByWorkspace?.browser1?.[0]?.worktreeId).toBe(NEW)
|
||||
expect(session.browserTabsByWorktree?.[NEW]?.[0]?.docLocation).toEqual({
|
||||
kind: 'workspace-doc',
|
||||
worktreeId: NEW,
|
||||
filePath: '/ws/worktree-creation-spinner/docs/report.html'
|
||||
})
|
||||
expect(session.browserPagesByWorkspace?.browser1?.[0]?.docLocation).toEqual({
|
||||
kind: 'workspace-doc',
|
||||
worktreeId: NEW,
|
||||
filePath: '/ws/worktree-creation-spinner/docs/report.html'
|
||||
})
|
||||
expect(session.activeBrowserTabIdByWorktree?.[NEW]).toBe('browser1')
|
||||
expect(session.activeTabTypeByWorktree?.[NEW]).toBe('browser')
|
||||
expect(session.activeWorktreeId).toBe(NEW)
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import type { WorkspaceKey } from '../../../shared/folder-workspace-types'
|
||||
import type { BrowserPage, BrowserWorkspace } from '../../../shared/browser-workspace-types'
|
||||
import { remapBrowserPageDocLocation } from '../../../shared/browser-page-doc-location'
|
||||
import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types'
|
||||
import { worktreeWorkspaceKey } from '../../../shared/workspace-scope'
|
||||
import type { PersistedState } from '../../../shared/persisted-state-types'
|
||||
@@ -6,6 +8,7 @@ import {
|
||||
getWorktreeIdFromHostIdentity,
|
||||
isWorktreeHostIdentity
|
||||
} from '../../../shared/worktree/host-qualified-identity'
|
||||
import { splitWorktreeIdForFilesystem } from '../../../shared/worktree/id'
|
||||
|
||||
/**
|
||||
* Re-keys every worktreeId-keyed record in `state` from `oldWorktreeId` to `newWorktreeId`. Mutates `state` in place;
|
||||
@@ -35,6 +38,23 @@ export function migrateWorktreeIdentity(
|
||||
}
|
||||
const withNewWorktreeId = <T extends { worktreeId: string }>(value: T): T =>
|
||||
value.worktreeId === oldWorktreeId ? { ...value, worktreeId: newWorktreeId } : value
|
||||
const oldWorktreePath = splitWorktreeIdForFilesystem(oldWorktreeId)?.worktreePath
|
||||
const newWorktreePath = splitWorktreeIdForFilesystem(newWorktreeId)?.worktreePath
|
||||
const withNewBrowserWorktreeId = <T extends BrowserPage | BrowserWorkspace>(value: T): T => {
|
||||
const renamedValue = withNewWorktreeId(value)
|
||||
return value.docLocation?.worktreeId === oldWorktreeId
|
||||
? {
|
||||
...renamedValue,
|
||||
docLocation: remapBrowserPageDocLocation(
|
||||
value.docLocation,
|
||||
oldWorktreeId,
|
||||
newWorktreeId,
|
||||
oldWorktreePath,
|
||||
newWorktreePath
|
||||
)
|
||||
}
|
||||
: renamedValue
|
||||
}
|
||||
const migrateSession = (session: WorkspaceSessionState | undefined): boolean => {
|
||||
if (!session) {
|
||||
return false
|
||||
@@ -72,16 +92,21 @@ export function migrateWorktreeIdentity(
|
||||
sessionChanged = moveSessionKey(session.activeFileIdByWorktree) || sessionChanged
|
||||
sessionChanged =
|
||||
moveSessionKey(session.browserTabsByWorktree, (workspaces) =>
|
||||
workspaces.map(withNewWorktreeId)
|
||||
workspaces.map(withNewBrowserWorktreeId)
|
||||
) || sessionChanged
|
||||
if (session.browserPagesByWorkspace) {
|
||||
let pagesChanged = false
|
||||
const nextPagesByWorkspace = { ...session.browserPagesByWorkspace }
|
||||
for (const [workspaceId, pages] of Object.entries(nextPagesByWorkspace)) {
|
||||
if (!pages.some((page) => page.worktreeId === oldWorktreeId)) {
|
||||
if (
|
||||
!pages.some(
|
||||
(page) =>
|
||||
page.worktreeId === oldWorktreeId || page.docLocation?.worktreeId === oldWorktreeId
|
||||
)
|
||||
) {
|
||||
continue
|
||||
}
|
||||
nextPagesByWorkspace[workspaceId] = pages.map(withNewWorktreeId)
|
||||
nextPagesByWorkspace[workspaceId] = pages.map(withNewBrowserWorktreeId)
|
||||
pagesChanged = true
|
||||
}
|
||||
if (pagesChanged) {
|
||||
|
||||
@@ -280,6 +280,40 @@ describe('a browser page that shows a workspace document', () => {
|
||||
expect(store.getState().browserPagesByWorkspace[tab.id]).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('reopens a closed document tab with its document identity', () => {
|
||||
const store = createStoreWithWorktree()
|
||||
const tab = store.getState().createBrowserTab(WORKTREE_ID, LIVE_GRANT_URL, {
|
||||
docLocation: DOC_LOCATION,
|
||||
browserRuntimeEnvironmentId: null
|
||||
})
|
||||
|
||||
store.getState().closeBrowserTab(tab.id)
|
||||
const reopened = store.getState().reopenClosedBrowserTab(WORKTREE_ID)
|
||||
const page = reopened ? store.getState().browserPagesByWorkspace[reopened.id]?.[0] : undefined
|
||||
|
||||
expect(page?.docLocation).toEqual(DOC_LOCATION)
|
||||
expect(page?.url).toBe(ORCA_BROWSER_BLANK_URL)
|
||||
expect(reopened?.docLocation).toEqual(DOC_LOCATION)
|
||||
})
|
||||
|
||||
it('reopens a closed document page with its document identity', () => {
|
||||
const store = createStoreWithWorktree()
|
||||
const tab = store.getState().createBrowserTab(WORKTREE_ID, 'https://example.com/')
|
||||
const documentPage = store.getState().createBrowserPage(tab.id, LIVE_GRANT_URL, {
|
||||
docLocation: DOC_LOCATION,
|
||||
browserRuntimeEnvironmentId: null
|
||||
})
|
||||
if (!documentPage) {
|
||||
throw new Error('Expected a document page')
|
||||
}
|
||||
|
||||
store.getState().closeBrowserPage(documentPage.id)
|
||||
const reopened = store.getState().reopenClosedBrowserPage(tab.id)
|
||||
|
||||
expect(reopened?.docLocation).toEqual(DOC_LOCATION)
|
||||
expect(reopened?.url).toBe(ORCA_BROWSER_BLANK_URL)
|
||||
})
|
||||
|
||||
it('writes the document and not the grant to the session', () => {
|
||||
const store = createStoreWithWorktree()
|
||||
const tab = store.getState().createBrowserTab(WORKTREE_ID, LIVE_GRANT_URL, {
|
||||
|
||||
@@ -97,6 +97,8 @@ type CreateBrowserPageOptions = {
|
||||
activate?: boolean
|
||||
title?: string
|
||||
browserRuntimeEnvironmentId?: string | null
|
||||
/** Creates a page that shows a workspace document instead of a URL. */
|
||||
docLocation?: BrowserPageDocLocation
|
||||
}
|
||||
|
||||
type BrowserTabPageState = {
|
||||
@@ -1187,6 +1189,7 @@ export const createBrowserSlice: StateCreator<AppState, [], [], BrowserSlice> =
|
||||
activate: true,
|
||||
sessionProfileId,
|
||||
sessionPartition,
|
||||
...(snap.docLocation ? { docLocation: snap.docLocation } : {}),
|
||||
targetGroupId: entryToRestore.position?.groupId
|
||||
})
|
||||
restoreRecentlyClosedTabPosition(get, worktreeId, restored.id, entryToRestore.position)
|
||||
@@ -1201,14 +1204,16 @@ export const createBrowserSlice: StateCreator<AppState, [], [], BrowserSlice> =
|
||||
sessionProfileId,
|
||||
sessionPartition,
|
||||
targetGroupId: entryToRestore.position?.groupId,
|
||||
browserRuntimeEnvironmentId: firstPage.browserRuntimeEnvironmentId
|
||||
browserRuntimeEnvironmentId: firstPage.browserRuntimeEnvironmentId,
|
||||
...(firstPage.docLocation ? { docLocation: firstPage.docLocation } : {})
|
||||
})
|
||||
|
||||
for (const p of restPages) {
|
||||
get().createBrowserPage(restored.id, p.url, {
|
||||
activate: false,
|
||||
title: p.title,
|
||||
browserRuntimeEnvironmentId: p.browserRuntimeEnvironmentId
|
||||
browserRuntimeEnvironmentId: p.browserRuntimeEnvironmentId,
|
||||
...(p.docLocation ? { docLocation: p.docLocation } : {})
|
||||
})
|
||||
}
|
||||
|
||||
@@ -1288,7 +1293,9 @@ export const createBrowserSlice: StateCreator<AppState, [], [], BrowserSlice> =
|
||||
workspace.worktreeId,
|
||||
url,
|
||||
options?.title,
|
||||
options?.browserRuntimeEnvironmentId
|
||||
options?.browserRuntimeEnvironmentId,
|
||||
undefined,
|
||||
options?.docLocation
|
||||
)
|
||||
|
||||
set((s) => {
|
||||
@@ -1309,6 +1316,7 @@ export const createBrowserSlice: StateCreator<AppState, [], [], BrowserSlice> =
|
||||
s.activeBrowserTabIdByWorktree[workspace.worktreeId] === workspaceId
|
||||
const shouldFocusAddressBar =
|
||||
shouldUpdateGlobalActiveSurface &&
|
||||
!page.docLocation &&
|
||||
(page.url === 'about:blank' || page.url === ORCA_BROWSER_BLANK_URL)
|
||||
|
||||
return {
|
||||
@@ -1476,7 +1484,8 @@ export const createBrowserSlice: StateCreator<AppState, [], [], BrowserSlice> =
|
||||
return get().createBrowserPage(workspaceId, pageToRestore.url, {
|
||||
title: pageToRestore.title,
|
||||
activate: true,
|
||||
browserRuntimeEnvironmentId: pageToRestore.browserRuntimeEnvironmentId
|
||||
browserRuntimeEnvironmentId: pageToRestore.browserRuntimeEnvironmentId,
|
||||
...(pageToRestore.docLocation ? { docLocation: pageToRestore.docLocation } : {})
|
||||
})
|
||||
},
|
||||
|
||||
|
||||
@@ -74,8 +74,32 @@ describe('migrateWorktreeIdentity', () => {
|
||||
[OLD]: { resultOwner: { worktreeId: OLD, runtimeEnvironmentId: 'runtime-a' } }
|
||||
},
|
||||
activeTabIdByWorktree: { [OLD]: 'tab1' },
|
||||
browserTabsByWorktree: { [OLD]: [{ id: 'browser1', worktreeId: OLD }] },
|
||||
browserPagesByWorkspace: { browser1: [{ id: 'page1', worktreeId: OLD }] },
|
||||
browserTabsByWorktree: {
|
||||
[OLD]: [
|
||||
{
|
||||
id: 'browser1',
|
||||
worktreeId: OLD,
|
||||
docLocation: {
|
||||
kind: 'workspace-doc',
|
||||
worktreeId: OLD,
|
||||
filePath: '/ws/cunner/docs/report.html'
|
||||
}
|
||||
}
|
||||
]
|
||||
},
|
||||
browserPagesByWorkspace: {
|
||||
browser1: [
|
||||
{
|
||||
id: 'page1',
|
||||
worktreeId: OLD,
|
||||
docLocation: {
|
||||
kind: 'workspace-doc',
|
||||
worktreeId: OLD,
|
||||
filePath: '/ws/cunner/docs/report.html'
|
||||
}
|
||||
}
|
||||
]
|
||||
},
|
||||
recentlyClosedBrowserTabsByWorktree: {
|
||||
[OLD]: [
|
||||
{ workspace: { id: 'closed-browser', worktreeId: OLD }, pages: [{ worktreeId: OLD }] }
|
||||
@@ -127,6 +151,16 @@ describe('migrateWorktreeIdentity', () => {
|
||||
expect(s.activeTabIdByWorktree[NEW]).toBe('tab1')
|
||||
expect(s.browserTabsByWorktree[NEW]?.[0]?.worktreeId).toBe(NEW)
|
||||
expect(s.browserPagesByWorkspace.browser1?.[0]?.worktreeId).toBe(NEW)
|
||||
expect(s.browserTabsByWorktree[NEW]?.[0]?.docLocation).toEqual({
|
||||
kind: 'workspace-doc',
|
||||
worktreeId: NEW,
|
||||
filePath: '/ws/worktree-creation-spinner/docs/report.html'
|
||||
})
|
||||
expect(s.browserPagesByWorkspace.browser1?.[0]?.docLocation).toEqual({
|
||||
kind: 'workspace-doc',
|
||||
worktreeId: NEW,
|
||||
filePath: '/ws/worktree-creation-spinner/docs/report.html'
|
||||
})
|
||||
expect(s.recentlyClosedBrowserTabsByWorktree[NEW]?.[0]?.workspace.worktreeId).toBe(NEW)
|
||||
expect(s.recentlyClosedBrowserTabsByWorktree[NEW]?.[0]?.pages[0]?.worktreeId).toBe(NEW)
|
||||
expect(s.recentlyClosedBrowserPagesByWorkspace.browser1?.[0]?.worktreeId).toBe(NEW)
|
||||
|
||||
@@ -1,4 +1,9 @@
|
||||
import type { AppState } from '../../../types'
|
||||
import type {
|
||||
BrowserPage,
|
||||
BrowserWorkspace
|
||||
} from '../../../../../../shared/browser-workspace-types'
|
||||
import { remapBrowserPageDocLocation } from '../../../../../../shared/browser-page-doc-location'
|
||||
import { splitWorktreeIdForFilesystem } from '../../../../../../shared/worktree/id'
|
||||
import { worktreeWorkspaceKey } from '../../../../../../shared/workspace-scope'
|
||||
import { getWorktreeIdFromVisitKey } from '@/lib/worktree-visit-recency'
|
||||
@@ -76,17 +81,34 @@ export function buildWorktreeRenameState(
|
||||
}
|
||||
const withNewWorktreeId = <T extends { worktreeId: string }>(value: T): T =>
|
||||
value.worktreeId === oldWorktreeId ? { ...value, worktreeId: newWorktreeId } : value
|
||||
const oldWorktreePath = splitWorktreeIdForFilesystem(oldWorktreeId)?.worktreePath
|
||||
const newWorktreePath = splitWorktreeIdForFilesystem(newWorktreeId)?.worktreePath
|
||||
const withNewBrowserWorktreeId = <T extends BrowserPage | BrowserWorkspace>(value: T): T => {
|
||||
const renamedValue = withNewWorktreeId(value)
|
||||
return value.docLocation?.worktreeId === oldWorktreeId
|
||||
? {
|
||||
...renamedValue,
|
||||
docLocation: remapBrowserPageDocLocation(
|
||||
value.docLocation,
|
||||
oldWorktreeId,
|
||||
newWorktreeId,
|
||||
oldWorktreePath,
|
||||
newWorktreePath
|
||||
)
|
||||
}
|
||||
: renamedValue
|
||||
}
|
||||
const renameValueByKey: Partial<Record<(typeof WORKTREE_ID_KEYED_MAP_KEYS)[number], unknown>> = {
|
||||
tabsByWorktree: (tabs: { worktreeId: string }[]) => tabs.map(withNewWorktreeId),
|
||||
browserTabsByWorktree: (workspaces: { worktreeId: string }[]) =>
|
||||
workspaces.map(withNewWorktreeId),
|
||||
browserTabsByWorktree: (workspaces: BrowserWorkspace[]) =>
|
||||
workspaces.map(withNewBrowserWorktreeId),
|
||||
recentlyClosedBrowserTabsByWorktree: (
|
||||
snapshots: { workspace: { worktreeId: string }; pages: { worktreeId: string }[] }[]
|
||||
snapshots: { workspace: BrowserWorkspace; pages: BrowserPage[] }[]
|
||||
) =>
|
||||
snapshots.map((snapshot) => ({
|
||||
...snapshot,
|
||||
workspace: withNewWorktreeId(snapshot.workspace),
|
||||
pages: snapshot.pages.map(withNewWorktreeId)
|
||||
workspace: withNewBrowserWorktreeId(snapshot.workspace),
|
||||
pages: snapshot.pages.map(withNewBrowserWorktreeId)
|
||||
})),
|
||||
fileSearchStateByWorktree: (searchState: AppState['fileSearchStateByWorktree'][string]) => ({
|
||||
...searchState,
|
||||
@@ -121,8 +143,6 @@ export function buildWorktreeRenameState(
|
||||
files.map(withNewWorktreeId)
|
||||
)
|
||||
// Why: terminal reopen snapshots hold absolute startupCwd paths under the old folder; remap or Cmd+Shift+T respawns into a directory that no longer exists after the rename.
|
||||
const oldWorktreePath = splitWorktreeIdForFilesystem(oldWorktreeId)?.worktreePath
|
||||
const newWorktreePath = splitWorktreeIdForFilesystem(newWorktreeId)?.worktreePath
|
||||
renameKey('recentlyClosedTerminalTabsByWorktree', (snapshots: ClosedTerminalTabSnapshot[]) =>
|
||||
oldWorktreePath && newWorktreePath
|
||||
? remapClosedTerminalTabSnapshotCwds(snapshots, oldWorktreePath, newWorktreePath)
|
||||
@@ -137,23 +157,29 @@ export function buildWorktreeRenameState(
|
||||
: s.openFiles
|
||||
const currentBrowserPagesByWorkspace = s.browserPagesByWorkspace ?? {}
|
||||
const browserPagesByWorkspace = Object.values(currentBrowserPagesByWorkspace).some((pages) =>
|
||||
pages.some((page) => page.worktreeId === oldWorktreeId)
|
||||
pages.some(
|
||||
(page) => page.worktreeId === oldWorktreeId || page.docLocation?.worktreeId === oldWorktreeId
|
||||
)
|
||||
)
|
||||
? Object.fromEntries(
|
||||
Object.entries(currentBrowserPagesByWorkspace).map(([workspaceId, pages]) => [
|
||||
workspaceId,
|
||||
pages.map(withNewWorktreeId)
|
||||
pages.map(withNewBrowserWorktreeId)
|
||||
])
|
||||
)
|
||||
: s.browserPagesByWorkspace
|
||||
const currentRecentlyClosedBrowserPagesByWorkspace = s.recentlyClosedBrowserPagesByWorkspace ?? {}
|
||||
const recentlyClosedBrowserPagesByWorkspace = Object.values(
|
||||
currentRecentlyClosedBrowserPagesByWorkspace
|
||||
).some((pages) => pages.some((page) => page.worktreeId === oldWorktreeId))
|
||||
).some((pages) =>
|
||||
pages.some(
|
||||
(page) => page.worktreeId === oldWorktreeId || page.docLocation?.worktreeId === oldWorktreeId
|
||||
)
|
||||
)
|
||||
? Object.fromEntries(
|
||||
Object.entries(currentRecentlyClosedBrowserPagesByWorkspace).map(([workspaceId, pages]) => [
|
||||
workspaceId,
|
||||
pages.map(withNewWorktreeId)
|
||||
pages.map(withNewBrowserWorktreeId)
|
||||
])
|
||||
)
|
||||
: s.recentlyClosedBrowserPagesByWorkspace
|
||||
|
||||
@@ -3,6 +3,7 @@ import type {
|
||||
BrowserPageDocLocation,
|
||||
BrowserWorkspace
|
||||
} from './browser-workspace-types'
|
||||
import { relativePathInsideRoot, resolveRuntimePath } from './cross-platform-path'
|
||||
|
||||
/**
|
||||
* Why an explicit comparison and not object identity: the mirror rebuilds the workspace's copy of
|
||||
@@ -25,3 +26,26 @@ export function isWorkspaceDocSurface(
|
||||
): boolean {
|
||||
return Boolean(surface.docLocation)
|
||||
}
|
||||
|
||||
export function remapBrowserPageDocLocation(
|
||||
location: BrowserPageDocLocation,
|
||||
oldWorktreeId: string,
|
||||
newWorktreeId: string,
|
||||
oldWorktreePath?: string,
|
||||
newWorktreePath?: string
|
||||
): BrowserPageDocLocation {
|
||||
if (location.worktreeId !== oldWorktreeId) {
|
||||
return location
|
||||
}
|
||||
const relativePath =
|
||||
oldWorktreePath && newWorktreePath
|
||||
? relativePathInsideRoot(oldWorktreePath, location.filePath)
|
||||
: null
|
||||
return {
|
||||
...location,
|
||||
worktreeId: newWorktreeId,
|
||||
...(relativePath !== null && newWorktreePath
|
||||
? { filePath: resolveRuntimePath(newWorktreePath, relativePath) }
|
||||
: {})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user