mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 00:57:45 +00:00
fix(notebook): run cells only when the resolved owner is this computer (#30187)
A managed SSH or paired-server workspace has no SSH connection id, so the notebook read it as local and ran its cells in the desktop's own Python against a same-named local file. Derive the execution host from resolveOwner; refuse remote, ambiguous and unloaded owners.
This commit is contained in:
@@ -17,7 +17,6 @@ import { translate } from '@/i18n/i18n'
|
||||
import { editorShortcutMatches } from './editor-shortcuts'
|
||||
import { getShortcutPlatform } from '@/lib/shortcut-platform'
|
||||
import { findWorktreeById } from '@/store/slices/worktree-helpers'
|
||||
import { getConnectionId } from '@/lib/connection-context'
|
||||
import { IpynbCellToolbar, IpynbToolbarButton } from './IpynbCellToolbar'
|
||||
import { IpynbCellSource } from './IpynbCellEditor'
|
||||
import { IpynbCellRunOutputs, IpynbCellRunPrompt } from './IpynbCellRun'
|
||||
@@ -29,6 +28,7 @@ import {
|
||||
useIpynbDocumentEditing
|
||||
} from './useIpynbDocumentEditing'
|
||||
import { useIpynbCellExecution } from './useIpynbCellExecution'
|
||||
import { resolveNotebookExecutionHost } from './ipynb-execution-host'
|
||||
import { useIpynbScrollRestoration } from './useIpynbScrollRestoration'
|
||||
|
||||
type IpynbViewerProps = {
|
||||
@@ -54,6 +54,7 @@ export default function IpynbViewer({
|
||||
}: IpynbViewerProps): React.JSX.Element {
|
||||
const editorFontZoomLevel = useAppStore((s) => s.editorFontZoomLevel)
|
||||
const rootPath = useAppStore((s) => findWorktreeById(s.worktreesByRepo, worktreeId)?.path ?? null)
|
||||
const executionHost = useAppStore((s) => resolveNotebookExecutionHost(s, worktreeId))
|
||||
const [editingCellKey, setEditingCellKey] = useState<string | null>(null)
|
||||
const parsed = useMemo(() => {
|
||||
try {
|
||||
@@ -158,7 +159,7 @@ export default function IpynbViewer({
|
||||
|
||||
const { notebook } = parsed
|
||||
// Kernels are local Python only; other notebooks still render and edit.
|
||||
const canRun = notebook.language === 'python' && !getConnectionId(worktreeId)
|
||||
const canRun = notebook.language === 'python' && executionHost === 'local'
|
||||
return (
|
||||
<div
|
||||
ref={setRootRef}
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { resolveNotebookExecutionHost } from './ipynb-execution-host'
|
||||
import type { WorktreeOperationRouteState } from '@/lib/worktree-operation-route'
|
||||
|
||||
const WORKTREE_ID = 'repo-1::/srv/w'
|
||||
|
||||
function rows(
|
||||
repo: Record<string, string>,
|
||||
worktree: {
|
||||
hostId?: `runtime:${string}` | `ssh:${string}` | 'local'
|
||||
runtimeOwnerEnvironmentId?: string
|
||||
}
|
||||
): WorktreeOperationRouteState {
|
||||
return {
|
||||
repos: [{ id: 'repo-1', ...repo }],
|
||||
worktreesByRepo: { 'repo-1': [{ id: WORKTREE_ID, repoId: 'repo-1', ...worktree }] }
|
||||
}
|
||||
}
|
||||
|
||||
describe('resolveNotebookExecutionHost', () => {
|
||||
it.each([
|
||||
['a local workspace', rows({}, { hostId: 'local' }), 'local'],
|
||||
['a legacy unstamped local workspace', rows({}, {}), 'local'],
|
||||
['a direct SSH workspace', rows({ connectionId: 'box' }, { hostId: 'ssh:box' }), 'remote'],
|
||||
[
|
||||
'a managed SSH workspace, which has no connection id',
|
||||
rows(
|
||||
{ executionHostId: 'runtime:env-orcad' },
|
||||
{ hostId: 'runtime:env-orcad', runtimeOwnerEnvironmentId: 'env-orcad' }
|
||||
),
|
||||
'remote'
|
||||
],
|
||||
['unloaded rows', {}, 'unresolved']
|
||||
] as const)('reads %s as %s', (_name, state, expected) => {
|
||||
expect(resolveNotebookExecutionHost(state, WORKTREE_ID)).toBe(expected)
|
||||
})
|
||||
|
||||
it('treats a local folder workspace as local and a server-owned one as remote', () => {
|
||||
const folder = (
|
||||
executionHostId: 'local' | `runtime:${string}`
|
||||
): WorktreeOperationRouteState => ({
|
||||
folderWorkspaces: [
|
||||
{ id: 'f1', projectGroupId: 'g1', connectionId: null, executionHostId, diffComments: [] }
|
||||
]
|
||||
})
|
||||
expect(resolveNotebookExecutionHost(folder('local'), 'folder:f1')).toBe('local')
|
||||
expect(resolveNotebookExecutionHost(folder('runtime:env-a'), 'folder:f1')).toBe('remote')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,22 @@
|
||||
import { LOCAL_EXECUTION_HOST_ID } from '../../../../shared/execution-host'
|
||||
import { resolveOwner } from '@/lib/resolve-owner'
|
||||
import type { WorktreeOperationRouteState } from '@/lib/worktree-operation-route'
|
||||
|
||||
/** Where a notebook's kernel would run: kernels exist only on this computer. */
|
||||
export type NotebookExecutionHost = 'local' | 'remote' | 'unresolved'
|
||||
|
||||
/**
|
||||
* Why: a managed SSH or paired-server workspace has no SSH connection id, so "no connection id"
|
||||
* cannot mean local; running its cells here would execute a same-named local file.
|
||||
*/
|
||||
export function resolveNotebookExecutionHost(
|
||||
state: WorktreeOperationRouteState,
|
||||
worktreeId: string
|
||||
): NotebookExecutionHost {
|
||||
const match = resolveOwner(state, { workspaceId: worktreeId })
|
||||
if (match.kind !== 'resolved') {
|
||||
return 'unresolved'
|
||||
}
|
||||
const { owner } = match
|
||||
return owner.endpoint.kind === 'self' && owner.at === LOCAL_EXECUTION_HOST_ID ? 'local' : 'remote'
|
||||
}
|
||||
@@ -3,8 +3,12 @@ import { act, renderHook, waitFor } from '@testing-library/react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { parseIpynb } from './ipynb-parse'
|
||||
import type { KernelFrameEvent } from '../../../../shared/notebook-kernel-types'
|
||||
import type {
|
||||
WorktreeOperationOwnerRecord,
|
||||
WorktreeOperationRouteState
|
||||
} from '@/lib/worktree-operation-route'
|
||||
|
||||
const { getConnectionIdMock, notebookApi, toastError, frameListener } = vi.hoisted(() => {
|
||||
const { storeState, notebookApi, toastError, frameListener } = vi.hoisted(() => {
|
||||
const frameListener = { current: (_event: KernelFrameEvent): void => {} }
|
||||
const notebookApi = {
|
||||
listPythonEnvironments: vi.fn(),
|
||||
@@ -18,23 +22,42 @@ const { getConnectionIdMock, notebookApi, toastError, frameListener } = vi.hoist
|
||||
}
|
||||
// The kernel session subscribes to kernel frames when it loads.
|
||||
Object.defineProperty(window, 'api', { configurable: true, value: { notebook: notebookApi } })
|
||||
const storeState: { current: WorktreeOperationRouteState } = { current: {} }
|
||||
return {
|
||||
getConnectionIdMock: vi.fn((): string | null => null),
|
||||
storeState,
|
||||
notebookApi,
|
||||
toastError: vi.fn(),
|
||||
frameListener
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('@/lib/connection-context', () => ({ getConnectionId: getConnectionIdMock }))
|
||||
vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback }))
|
||||
vi.mock('sonner', () => ({ toast: { error: toastError } }))
|
||||
vi.mock('@/store', () => ({ useAppStore: { subscribe: () => () => {} } }))
|
||||
vi.mock('@/store', () => ({
|
||||
useAppStore: { subscribe: () => () => {}, getState: () => storeState.current }
|
||||
}))
|
||||
|
||||
import { useIpynbCellExecution } from './useIpynbCellExecution'
|
||||
import { runCells, trustNotebook } from './ipynb-kernel-session'
|
||||
import { getSession } from './ipynb-kernel-store'
|
||||
|
||||
const WORKTREE_ID = 'repo-1::/repo'
|
||||
|
||||
function ownerRows(
|
||||
repo: Pick<
|
||||
NonNullable<WorktreeOperationRouteState['repos']>[number],
|
||||
'connectionId' | 'executionHostId'
|
||||
>,
|
||||
worktree: Pick<WorktreeOperationOwnerRecord, 'hostId' | 'runtimeOwnerEnvironmentId'> = {}
|
||||
): WorktreeOperationRouteState {
|
||||
return {
|
||||
repos: [{ id: 'repo-1', ...repo }],
|
||||
worktreesByRepo: { 'repo-1': [{ id: WORKTREE_ID, repoId: 'repo-1', ...worktree }] }
|
||||
}
|
||||
}
|
||||
|
||||
const LOCAL_ROWS = ownerRows({}, { hostId: 'local' })
|
||||
|
||||
function notebookContent(withIds: boolean): string {
|
||||
return JSON.stringify({
|
||||
nbformat: 4,
|
||||
@@ -62,7 +85,7 @@ function renderExecution(filePath: string, applyContent = vi.fn(), withIds = tru
|
||||
const hook = renderHook(() =>
|
||||
useIpynbCellExecution({
|
||||
filePath,
|
||||
worktreeId: 'worktree-a',
|
||||
worktreeId: WORKTREE_ID,
|
||||
rootPath: '/repo',
|
||||
flushSourceDrafts: () => content,
|
||||
applyContent
|
||||
@@ -73,7 +96,7 @@ function renderExecution(filePath: string, applyContent = vi.fn(), withIds = tru
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
getConnectionIdMock.mockReturnValue(null)
|
||||
storeState.current = LOCAL_ROWS
|
||||
notebookApi.listPythonEnvironments.mockResolvedValue({
|
||||
workspace: [{ path: '/repo/.venv/bin/python', name: '.venv', version: '3.12.1' }],
|
||||
path: []
|
||||
@@ -106,9 +129,22 @@ describe('notebook cell execution', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('refuses to run in SSH workspaces without touching the notebook or starting a kernel', () => {
|
||||
getConnectionIdMock.mockReturnValue('ssh-connection')
|
||||
const { hook, applyContent } = renderExecution('/remote/notebook.ipynb')
|
||||
it.each([
|
||||
['a direct SSH workspace', ownerRows({ connectionId: 'box' }, { hostId: 'ssh:box' })],
|
||||
[
|
||||
'a managed SSH workspace owned by its server',
|
||||
ownerRows(
|
||||
{ executionHostId: 'runtime:env-orcad' },
|
||||
{ hostId: 'runtime:env-orcad', runtimeOwnerEnvironmentId: 'env-orcad' }
|
||||
)
|
||||
],
|
||||
[
|
||||
'an SSH target behind a paired server',
|
||||
ownerRows({}, { hostId: 'ssh:t', runtimeOwnerEnvironmentId: 'env-a' })
|
||||
]
|
||||
])('refuses to run %s without touching the notebook or starting a kernel', (_name, rows) => {
|
||||
storeState.current = rows
|
||||
const { hook, applyContent } = renderExecution('/repo/notebook.ipynb')
|
||||
|
||||
act(() => hook.result.current.runCell(1))
|
||||
expect(toastError).toHaveBeenCalledWith(
|
||||
@@ -119,6 +155,32 @@ describe('notebook cell execution', () => {
|
||||
expect(notebookApi.startKernel).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it.each<[string, WorktreeOperationRouteState]>([
|
||||
['rows that have not loaded', {}],
|
||||
[
|
||||
'two hosts publishing the same workspace',
|
||||
{
|
||||
repos: [{ id: 'repo-1' }, { id: 'repo-1', executionHostId: 'runtime:env-a' }],
|
||||
worktreesByRepo: {
|
||||
'repo-1': [
|
||||
{ id: WORKTREE_ID, repoId: 'repo-1', hostId: 'local' },
|
||||
{ id: WORKTREE_ID, repoId: 'repo-1', hostId: 'runtime:env-a' }
|
||||
]
|
||||
}
|
||||
}
|
||||
]
|
||||
])('refuses to run with %s instead of reading them as local', (_name, rows) => {
|
||||
storeState.current = rows
|
||||
const { hook, applyContent } = renderExecution('/repo/notebook.ipynb')
|
||||
|
||||
act(() => hook.result.current.runCell(1))
|
||||
expect(toastError).toHaveBeenCalledWith(
|
||||
"Orca can't tell which computer this notebook is on yet, so its cells can't run."
|
||||
)
|
||||
expect(applyContent).not.toHaveBeenCalled()
|
||||
expect(hook.result.current.pendingRun).toBeNull()
|
||||
})
|
||||
|
||||
it('gives id-less cells ids before queueing, so output follows a moved cell', () => {
|
||||
const { hook, content } = renderExecution('/repo/legacy.ipynb', vi.fn(), false)
|
||||
|
||||
@@ -185,7 +247,7 @@ function mountFinishedNotebook(filePath: string, content: string) {
|
||||
const hook = renderHook(() =>
|
||||
useIpynbCellExecution({
|
||||
filePath,
|
||||
worktreeId: 'worktree-a',
|
||||
worktreeId: WORKTREE_ID,
|
||||
rootPath: '/repo',
|
||||
flushSourceDrafts,
|
||||
applyContent
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
import { useEffect, useState } from 'react'
|
||||
import { toast } from 'sonner'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import { getConnectionId } from '@/lib/connection-context'
|
||||
import { useAppStore } from '@/store'
|
||||
import { resolveNotebookExecutionHost, type NotebookExecutionHost } from './ipynb-execution-host'
|
||||
import { clearIpynbOutputs, updateIpynbCellRuns, withIpynbCellIds } from './ipynb-cell-mutations'
|
||||
import { toStoredOutputs } from './ipynb-kernel-outputs'
|
||||
import {
|
||||
@@ -19,6 +20,28 @@ import {
|
||||
import { parseIpynb } from './ipynb-parse'
|
||||
import { getIpynbCellKey } from './useIpynbDocumentEditing'
|
||||
|
||||
function executionRefusal(host: NotebookExecutionHost, language: string): string | null {
|
||||
switch (host) {
|
||||
case 'remote':
|
||||
return translate(
|
||||
'auto.components.editor.IpynbViewer.localOnly',
|
||||
'Notebook cells can only run for files on this computer.'
|
||||
)
|
||||
case 'unresolved':
|
||||
return translate(
|
||||
'auto.components.editor.IpynbViewer.ownerUnresolved',
|
||||
"Orca can't tell which computer this notebook is on yet, so its cells can't run."
|
||||
)
|
||||
case 'local':
|
||||
return language === 'python'
|
||||
? null
|
||||
: translate(
|
||||
'auto.components.editor.IpynbViewer.pythonOnly',
|
||||
'Only Python notebooks can run in Orca.'
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
type UseIpynbCellExecutionArgs = {
|
||||
filePath: string
|
||||
worktreeId: string
|
||||
@@ -83,18 +106,12 @@ export function useIpynbCellExecution({
|
||||
if (codeIndexes.length === 0) {
|
||||
return
|
||||
}
|
||||
if (getConnectionId(worktreeId) || notebook.language !== 'python') {
|
||||
toast.error(
|
||||
getConnectionId(worktreeId)
|
||||
? translate(
|
||||
'auto.components.editor.IpynbViewer.localOnly',
|
||||
'Notebook cells can only run for files on this computer.'
|
||||
)
|
||||
: translate(
|
||||
'auto.components.editor.IpynbViewer.pythonOnly',
|
||||
'Only Python notebooks can run in Orca.'
|
||||
)
|
||||
)
|
||||
const refusal = executionRefusal(
|
||||
resolveNotebookExecutionHost(useAppStore.getState(), worktreeId),
|
||||
notebook.language
|
||||
)
|
||||
if (refusal) {
|
||||
toast.error(refusal)
|
||||
return
|
||||
}
|
||||
// Why: id-less (nbformat 4.4) cells are keyed by index, so a move mid-run would misroute output.
|
||||
|
||||
@@ -15866,7 +15866,8 @@
|
||||
"noPython": "Python was not found on this computer. Install it from [python.org](https://www.python.org/downloads/), then run the cell again.",
|
||||
"kernelDied": "The kernel died.",
|
||||
"localOnly": "Notebook cells can only run for files on this computer.",
|
||||
"pythonOnly": "Only Python notebooks can run in Orca."
|
||||
"pythonOnly": "Only Python notebooks can run in Orca.",
|
||||
"ownerUnresolved": "Orca can't tell which computer this notebook is on yet, so its cells can't run."
|
||||
},
|
||||
"MarkdownPreview": {
|
||||
"e4683f70c4": "Cancel",
|
||||
|
||||
Reference in New Issue
Block a user