Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 28 additions & 11 deletions src/renderer/src/components/artifacts/artifact-publish-flow.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { ARTIFACT_MAX_CONTENT_BYTES } from '../../../../shared/artifacts'
import { publishArtifactFromSurface } from './artifact-publish-flow'
import {
ArtifactPublishPreparationError,
publishArtifactFromSurface
} from './artifact-publish-flow'

const mocks = vi.hoisted(() => ({
callRuntimeRpc: vi.fn(),
Expand Down Expand Up @@ -44,50 +47,64 @@ beforeEach(() => {
})

describe('artifact publish flow', () => {
it('signs in before preparing and publishing the request', async () => {
it('prepares the request once, before signing in and publishing it', async () => {
mocks.state.orcaProfileAuthStatus = { state: 'local' }
mocks.connect.mockResolvedValue({ status: 'connected' })
mocks.callRuntimeRpc.mockResolvedValue({ status: 'ok', value: published })
const createRequest = vi.fn().mockResolvedValue(request)

await expect(publishArtifactFromSurface(createRequest)).resolves.toBe(published)
expect(mocks.connect).toHaveBeenCalledOnce()
expect(createRequest).toHaveBeenCalledOnce()
expect(mocks.connect).toHaveBeenCalledOnce()
expect(createRequest.mock.invocationCallOrder[0]).toBeLessThan(
mocks.connect.mock.invocationCallOrder[0]
)
expect(mocks.callRuntimeRpc).toHaveBeenCalledWith(
{ kind: 'local' },
'artifacts.publish',
request
)
})

it('resumes with fresh content after reconnecting', async () => {
// Why: a re-read after sign-in could publish content the user never clicked Share on.
it('publishes the clicked request again after reconnecting, without re-reading', async () => {
mocks.connect.mockResolvedValue({ status: 'connected' })
mocks.callRuntimeRpc
.mockResolvedValueOnce({ status: 'reconnect-required' })
.mockResolvedValueOnce({ status: 'ok', value: published })
const createRequest = vi
.fn()
.mockResolvedValueOnce(request)
.mockResolvedValueOnce({ ...request, content: '<h1>Fresh</h1>' })
.mockResolvedValueOnce({ ...request, content: '<h1>Later</h1>' })

await expect(publishArtifactFromSurface(createRequest)).resolves.toBe(published)
expect(mocks.connect).toHaveBeenCalledOnce()
expect(createRequest).toHaveBeenCalledTimes(2)
expect(mocks.callRuntimeRpc.mock.calls[1]?.[2]).toMatchObject({
content: '<h1>Fresh</h1>'
})
expect(createRequest).toHaveBeenCalledOnce()
expect(mocks.callRuntimeRpc).toHaveBeenCalledTimes(2)
expect(mocks.callRuntimeRpc.mock.calls[1]?.[2]).toBe(request)
})

it('surfaces sign-in failures without preparing the file', async () => {
it('surfaces sign-in failures without publishing', async () => {
mocks.state.orcaProfileAuthStatus = { state: 'local' }
mocks.connect.mockRejectedValue(new Error('login failed'))
const createRequest = vi.fn().mockResolvedValue(request)

await expect(publishArtifactFromSurface(createRequest)).resolves.toBeNull()
expect(createRequest).not.toHaveBeenCalled()
expect(mocks.callRuntimeRpc).not.toHaveBeenCalled()
expect(mocks.toastError).toHaveBeenCalledWith('Could not share artifact', undefined)
})

it('reports an unreadable file without asking the user to sign in', async () => {
mocks.state.orcaProfileAuthStatus = { state: 'local' }
const createRequest = vi
.fn()
.mockRejectedValue(new ArtifactPublishPreparationError('unreadable'))

await expect(publishArtifactFromSurface(createRequest)).resolves.toBeNull()
expect(mocks.connect).not.toHaveBeenCalled()
expect(mocks.callRuntimeRpc).not.toHaveBeenCalled()
})

it('rejects an oversized request before RPC', async () => {
const createRequest = vi.fn().mockResolvedValue({
...request,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,12 @@ export async function publishArtifactFromSurface(
createRequest: () => Promise<ArtifactWriteRequest>
): Promise<ArtifactPublishResult | null> {
try {
// Why first and once: the click captures what is shared; a sign-in or retry must not re-read it.
const request = validateArtifactPublishRequest(await createRequest())
if (!(await ensureArtifactAccountConnected())) {
return null
}
for (let attempt = 0; attempt < 2; attempt += 1) {
const request = validateArtifactPublishRequest(await createRequest())
const result = await callRuntimeRpc<ArtifactCloudOperation<ArtifactPublishResult>>(
LOCAL_RUNTIME,
'artifacts.publish',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ import type { ArtifactWriteRequest } from '../../../../../shared/artifacts'
import { ARTIFACT_MAX_CONTENT_BYTES } from '../../../../../shared/artifacts'
import { getRuntimePathBasename } from '../../../../../shared/cross-platform-path'
import { ArtifactPublishPreparationError } from '@/components/artifacts/artifact-publish-flow'
import {
canonicalArtifactFilePath,
fileArtifactSourceKey,
LOCAL_FILE_ARTIFACT_OWNER
} from '@/lib/file-artifact-source-key'
import { userNamedFileAccess } from '@/lib/local-file-access'

export type ShareableBrowserArtifactFile = {
Expand All @@ -17,14 +22,10 @@ export function browserFileUrlToAbsolutePath(url: string): string | null {
}
const hostPrefix =
parsed.hostname && parsed.hostname !== 'localhost' ? `//${parsed.hostname}` : ''
let absolutePath = `${hostPrefix}${decodeURIComponent(parsed.pathname)}`
if (/^\/[A-Za-z]:\//.test(absolutePath)) {
absolutePath = absolutePath.slice(1)
}
if (/^[A-Za-z]:\//.test(absolutePath) || absolutePath.startsWith('//')) {
absolutePath = absolutePath.replaceAll('/', '\\')
}
return absolutePath
const absolutePath = `${hostPrefix}${decodeURIComponent(parsed.pathname)}`
return canonicalArtifactFilePath(
/^\/[A-Za-z]:\//.test(absolutePath) ? absolutePath.slice(1) : absolutePath
)
} catch {
return null
}
Expand Down Expand Up @@ -63,7 +64,7 @@ export async function readBrowserHtmlArtifactRequest(url: string): Promise<Artif
throw new ArtifactPublishPreparationError('binary')
}
return {
sourceKey: file.filePath,
sourceKey: fileArtifactSourceKey(LOCAL_FILE_ARTIFACT_OWNER, file.filePath),
content: result.content,
contentType: 'text/html',
fileName: file.fileName
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { act } from 'react'
import type { BrowserPage, BrowserWorkspace } from '../../../../../shared/browser-workspace-types'
import type * as WebviewRegistryModule from '../host-guest/webview-registry'
import type * as DocPreviewArtifactShareModule from './doc-preview-artifact-share'
import { createRoot, type Root } from 'react-dom/client'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { TooltipProvider } from '@/components/ui/tooltip'
Expand All @@ -22,25 +23,47 @@ const clipboard = vi.hoisted(() => ({ writes: [] as string[] }))
const grabCalls: { browserPageId: string; enabled: boolean }[] = []
const osOpens: string[] = []

vi.mock('@/lib/doc-preview-grants', () => ({
buildDocPreviewGrantRequest: () => ({
const grants = vi.hoisted(() => {
const mintedLocation = {
owner: {
kind: 'runtime' as const,
environmentId: 'env-1',
worktreeSelector: 'id:wt-1',
worktreeRoot: '/repo'
},
requestBase: '/repo',
root: '/repo',
entryRelativePath: ENTRY_RELATIVE_PATH
}),
entryRelativePath: 'docs/reports/index.html'
}
const shareControlCalls: unknown[][] = []
return { mintedLocation, requestLocation: mintedLocation, shareControlCalls }
})

vi.mock('@/lib/doc-preview-grants', () => ({
buildDocPreviewGrantRequest: () => grants.requestLocation,
// The real cache keeps the first mint's handle for the tab, whatever a later request says.
ensureDocPreviewGrant: () =>
Promise.resolve({
grantId: GRANT_ID,
url: `orca-preview://${GRANT_ID}/${ENTRY_RELATIVE_PATH}`
url: `orca-preview://${GRANT_ID}/${ENTRY_RELATIVE_PATH}`,
location: grants.mintedLocation
}),
releaseDocPreviewGrant: () => undefined
}))

vi.mock('./doc-preview-artifact-share', async (importOriginal) => {
const actual = await importOriginal<typeof DocPreviewArtifactShareModule>()
return {
...actual,
buildDocPreviewShareControl: (
...args: Parameters<typeof actual.buildDocPreviewShareControl>
) => {
grants.shareControlCalls.push(args)
return actual.buildDocPreviewShareControl(...args)
}
}
})

vi.mock('@/components/browser-pane/host-guest/webview-registry', async (importOriginal) => ({
...(await importOriginal<typeof WebviewRegistryModule>()),
moveFocusToRendererBeforeWebviewDetach: () => undefined
Expand Down Expand Up @@ -276,6 +299,27 @@ describe('HtmlDocPreview browser chrome', () => {
expect(hasLiveBrowserGuest('preview-1')).toBe(false)
})

// Why: Share must read and key from the host the grant was minted for, not the one selected now.
it('shares through the minted grant even after the workspace resolves to another host', async () => {
grants.shareControlCalls.length = 0
await renderPreview(container, root)
expect(grants.shareControlCalls.at(-1)).toEqual([ABSOLUTE_PATH, grants.mintedLocation])
expect(button(container, 'Share as artifact')).not.toBeNull()

grants.requestLocation = {
...grants.mintedLocation,
owner: { ...grants.mintedLocation.owner, environmentId: 'env-2' }
}
storeState.settings = { activeRuntimeEnvironmentId: 'env-2' }
try {
await renderPreview(container, root)
expect(grants.shareControlCalls.at(-1)).toEqual([ABSOLUTE_PATH, grants.mintedLocation])
} finally {
grants.requestLocation = grants.mintedLocation
storeState.settings = { activeRuntimeEnvironmentId: 'env-1' }
}
})

it('identifies the document by its workspace path and owning machine', async () => {
await renderPreview(container, root)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import { attachDocPreviewWebview } from './doc-preview-webview-attach'
import {
buildDocPreviewGrantRequest,
ensureDocPreviewGrant,
releaseDocPreviewGrant
releaseDocPreviewGrant,
type DocPreviewGrantHandle
} from '@/lib/doc-preview-grants'
import { selectWorktreeHostDisplayLabel } from '@/lib/execution-host-display-label'
import { translate } from '@/i18n/i18n'
Expand All @@ -31,6 +32,7 @@ import {
docPreviewFailureDetail
} from './doc-preview-failure-messages'
import { DocPreviewToolbar } from './doc-preview-toolbar'
import { buildDocPreviewShareControl } from './doc-preview-artifact-share'
import { useDocPreviewWebviewHistory } from './doc-preview-webview-history'
import { useDocPreviewGuestTools } from './use-doc-preview-guest-tools'

Expand Down Expand Up @@ -74,7 +76,8 @@ export function HtmlDocPreview({
const [assetFailures, setAssetFailures] = useState<DocPreviewFileFailure[]>([])
const [downloadBlocked, setDownloadBlocked] = useState(false)
const [remintCount, setRemintCount] = useState(0)
const [grantId, setGrantId] = useState<string | null>(null)
const [grant, setGrant] = useState<DocPreviewGrantHandle | null>(null)
const grantId = grant?.grantId ?? null
const {
requests: accessRequests,
busy: accessRequestBusy,
Expand Down Expand Up @@ -173,7 +176,7 @@ export function HtmlDocPreview({
setAssetFailures([])
resetDirectoryAccess()
setDownloadBlocked(false)
setGrantId(null)
setGrant(null)
resetHistory()
const request = buildDocPreviewGrantRequest(useAppStore.getState(), worktreeId, filePath)
if (!request) {
Expand Down Expand Up @@ -245,7 +248,7 @@ export function HtmlDocPreview({
webviewRef.current = attached.webview
// Why only now: main binds this grant to the guest on its first commit, so the tools have
// nothing to name until the webview exists and is pointed at it.
setGrantId(handle.grantId)
setGrant(handle)
})
.catch(() => {
if (!disposed) {
Expand Down Expand Up @@ -382,6 +385,7 @@ export function HtmlDocPreview({
onToggleMarkup={() => (markup.isActive ? markup.cancel() : void markup.start())}
// Nothing has painted yet on a loading or failed preview, so there is nothing to draw on.
markupDisabled={isUnavailable || state !== 'ready' || grab.state !== 'idle'}
shareControl={buildDocPreviewShareControl(filePath, grant?.location ?? null)}
/>
{notices.map((notice) => (
<div
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { ArtifactPublishPreparationError } from '@/components/artifacts/artifact-publish-flow'
import type { DocPreviewGrantLocation } from '@/lib/doc-preview-grants'
import {
buildDocPreviewShareControl,
readDocPreviewArtifactRequest
} from './doc-preview-artifact-share'

const mocks = vi.hoisted(() => ({ callRuntimeRpc: vi.fn(), readFile: vi.fn() }))

vi.mock('@/store', () => ({ useAppStore: { getState: () => ({}) } }))
vi.mock('@/runtime/runtime-rpc-client', () => ({ callRuntimeRpc: mocks.callRuntimeRpc }))

const FILE_PATH = '/srv/repo/reports/latency.html'

const pairedLocation: DocPreviewGrantLocation = {
owner: {
kind: 'runtime',
environmentId: 'server-1',
worktreeSelector: 'id:repo::/srv/repo',
worktreeRoot: '/srv/repo'
},
requestBase: '/srv/repo',
root: '/srv/repo/reports',
entryRelativePath: 'reports/latency.html'
}

const sshLocation: DocPreviewGrantLocation = {
owner: { kind: 'ssh', connectionId: 'ssh-1' },
requestBase: '/srv/repo',
root: '/srv/repo/reports',
entryRelativePath: 'reports/latency.html'
}

beforeEach(() => {
vi.clearAllMocks()
vi.stubGlobal('window', { api: { fs: { readFile: mocks.readFile } } })
})

describe('document preview sharing', () => {
it('reads the page from the server its grant was minted for', async () => {
mocks.callRuntimeRpc.mockResolvedValue({ content: '<h1>Latency</h1>', isBinary: false })

await expect(
readDocPreviewArtifactRequest(pairedLocation, FILE_PATH, 'key-1')
).resolves.toEqual({
sourceKey: 'key-1',
content: '<h1>Latency</h1>',
contentType: 'text/html',
fileName: 'latency.html'
})
expect(mocks.callRuntimeRpc).toHaveBeenCalledWith(
{ kind: 'environment', environmentId: 'server-1' },
'files.readPreview',
{ worktree: 'id:repo::/srv/repo', relativePath: 'reports/latency.html' },
expect.anything()
)
expect(mocks.readFile).not.toHaveBeenCalled()
})

// Why: a read without the connection id would share a local file instead.
it('keeps an SSH grant on its connection, so the read fails closed', async () => {
mocks.readFile.mockRejectedValue(new Error('SSH host is not served here'))

await expect(readDocPreviewArtifactRequest(sshLocation, FILE_PATH, 'key-1')).rejects.toEqual(
new ArtifactPublishPreparationError('unreadable')
)
expect(mocks.readFile).toHaveBeenCalledExactlyOnceWith({
filePath: FILE_PATH,
connectionId: 'ssh-1'
})
expect(mocks.callRuntimeRpc).not.toHaveBeenCalled()
})

it('refuses binary content and unreachable servers', async () => {
mocks.callRuntimeRpc.mockResolvedValueOnce({ content: '', isBinary: true })
await expect(readDocPreviewArtifactRequest(pairedLocation, FILE_PATH, 'key-1')).rejects.toEqual(
new ArtifactPublishPreparationError('binary')
)

mocks.callRuntimeRpc.mockRejectedValueOnce(new Error('offline'))
await expect(readDocPreviewArtifactRequest(pairedLocation, FILE_PATH, 'key-1')).rejects.toEqual(
new ArtifactPublishPreparationError('unreadable')
)
})

it('offers Share only for HTML previews with a minted grant', () => {
expect(buildDocPreviewShareControl(FILE_PATH, pairedLocation)).toBeTypeOf('function')
expect(buildDocPreviewShareControl(FILE_PATH, null)).toBeUndefined()
expect(buildDocPreviewShareControl('/srv/repo/report.pdf', pairedLocation)).toBeUndefined()
})
})
Loading
Loading