mirror of
https://github.com/stablyai/orca.git
synced 2026-10-17 08:03:14 +00:00
fix(mobile): hydrate settings/trust before availability probes settle
Review fixes for #7985: setTrustedOrcaHooks/setRuntimeSettings no longer wait on status.get/preflight.check/linear.status (a first-open preflight.check can take seconds, widening the spurious setup-trust re-prompt window). Also adds param-parity tests for createBlankWorkspace and a GitLab MR base-resolve test.
This commit is contained in:
@@ -12,7 +12,7 @@ import {
|
||||
} from 'react-native'
|
||||
import { ChevronDown, ChevronUp } from 'lucide-react-native'
|
||||
import type { RpcClient } from '../transport/rpc-client'
|
||||
import type { RpcSuccess } from '../transport/types'
|
||||
import type { RpcResponse, RpcSuccess } from '../transport/types'
|
||||
import { colors, spacing, radii, typography } from '../theme/mobile-theme'
|
||||
import { BottomDrawer } from './BottomDrawer'
|
||||
import { PickerListDrawer } from './PickerListDrawer'
|
||||
@@ -313,19 +313,23 @@ function NewWorktreeModalContent({
|
||||
// Why: settle each RPC independently so a flaky availability probe (e.g. a
|
||||
// linear.status timeout, which rejects rather than resolving {ok:false})
|
||||
// can't discard the already-resolved critical settings/ui results.
|
||||
const settled = await Promise.allSettled([
|
||||
client.sendRequest('settings.get'),
|
||||
client.sendRequest('ui.get'),
|
||||
const probes = Promise.allSettled([
|
||||
client.sendRequest('status.get'),
|
||||
client.sendRequest('preflight.check'),
|
||||
client.sendRequest('linear.status')
|
||||
])
|
||||
const okResult = (entry: PromiseSettledResult<RpcResponse>): RpcSuccess | null =>
|
||||
entry.status === 'fulfilled' && entry.value.ok ? (entry.value as RpcSuccess) : null
|
||||
// Why: hydrate settings/trust the moment their own RPCs settle — gating them
|
||||
// on the probes (a first-open preflight.check can take seconds) widens the
|
||||
// window where an already-trusted setup hook spuriously re-prompts on create.
|
||||
const [settingsRes, uiRes] = await Promise.allSettled([
|
||||
client.sendRequest('settings.get'),
|
||||
client.sendRequest('ui.get')
|
||||
])
|
||||
if (stale) {
|
||||
return
|
||||
}
|
||||
const okResult = (entry: (typeof settled)[number]): RpcSuccess | null =>
|
||||
entry.status === 'fulfilled' && entry.value.ok ? (entry.value as RpcSuccess) : null
|
||||
const [settingsRes, uiRes, statusRes, preflightRes, linearRes] = settled
|
||||
|
||||
const settingsResult = okResult(settingsRes)
|
||||
const settingsValue = settingsResult
|
||||
@@ -343,6 +347,11 @@ function NewWorktreeModalContent({
|
||||
const ui = (uiResult.result as { ui?: { trustedOrcaHooks?: PersistedTrustedOrcaHooks } }).ui
|
||||
setTrustedOrcaHooks(ui?.trustedOrcaHooks ?? {})
|
||||
}
|
||||
|
||||
const [statusRes, preflightRes, linearRes] = await probes
|
||||
if (stale) {
|
||||
return
|
||||
}
|
||||
// Tasks is an additive RPC surface, so older paired desktops without the
|
||||
// capability fall back to branch + blank sources only.
|
||||
const statusResult = okResult(statusRes)
|
||||
|
||||
@@ -0,0 +1,145 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { RpcClient } from '../transport/rpc-client'
|
||||
import { createBlankWorkspace } from './blank-workspace-create'
|
||||
|
||||
type Call = { method: string; params: unknown }
|
||||
|
||||
function fakeClient(script: (method: string, call: number) => unknown, calls: Call[]): RpcClient {
|
||||
return {
|
||||
sendRequest: async (method: string, params?: unknown) => {
|
||||
calls.push({ method, params })
|
||||
const result = script(method, calls.length)
|
||||
if (result instanceof Error) {
|
||||
return {
|
||||
id: '1',
|
||||
ok: false,
|
||||
error: { code: 'x', message: result.message },
|
||||
_meta: { runtimeId: 'r' }
|
||||
}
|
||||
}
|
||||
return { id: '1', ok: true, result, _meta: { runtimeId: 'r' } }
|
||||
}
|
||||
} as unknown as RpcClient
|
||||
}
|
||||
|
||||
describe('createBlankWorkspace', () => {
|
||||
it('assembles exactly the params the modal historically sent, omitting empty extras', async () => {
|
||||
const calls: Call[] = []
|
||||
const client = fakeClient(() => ({ worktree: { id: 'wt-1' } }), calls)
|
||||
|
||||
const result = await createBlankWorkspace({
|
||||
client,
|
||||
repoId: 'repo-1',
|
||||
baseName: 'octopus',
|
||||
startupCommand: undefined,
|
||||
createdWithAgentId: undefined,
|
||||
comment: undefined,
|
||||
setupDecision: 'inherit'
|
||||
})
|
||||
|
||||
expect(result).toEqual({ worktreeId: 'wt-1', name: 'octopus' })
|
||||
expect(calls).toHaveLength(1)
|
||||
expect(calls[0]).toEqual({
|
||||
method: 'worktree.create',
|
||||
params: {
|
||||
repo: 'id:repo-1',
|
||||
startupCommand: undefined,
|
||||
setupDecision: 'inherit',
|
||||
name: 'octopus'
|
||||
}
|
||||
})
|
||||
const params = calls[0]?.params as Record<string, unknown>
|
||||
expect('createdWithAgent' in params).toBe(false)
|
||||
expect('comment' in params).toBe(false)
|
||||
})
|
||||
|
||||
it('includes createdWithAgent and comment only when provided', async () => {
|
||||
const calls: Call[] = []
|
||||
const client = fakeClient(() => ({ worktree: { id: 'wt-2' } }), calls)
|
||||
|
||||
await createBlankWorkspace({
|
||||
client,
|
||||
repoId: 'repo-2',
|
||||
baseName: 'manatee',
|
||||
startupCommand: 'claude',
|
||||
createdWithAgentId: 'claude',
|
||||
comment: 'spike',
|
||||
setupDecision: 'run'
|
||||
})
|
||||
|
||||
expect(calls[0]?.params).toMatchObject({
|
||||
repo: 'id:repo-2',
|
||||
name: 'manatee',
|
||||
startupCommand: 'claude',
|
||||
setupDecision: 'run',
|
||||
createdWithAgent: 'claude',
|
||||
comment: 'spike'
|
||||
})
|
||||
})
|
||||
|
||||
it('retries with a numeric suffix on a branch-collision error', async () => {
|
||||
const calls: Call[] = []
|
||||
const client = fakeClient((_method, call) => {
|
||||
if (call === 1) {
|
||||
return new Error('Branch "octopus" already exists locally. Pick a different branch name.')
|
||||
}
|
||||
return { worktree: { id: 'wt-3' } }
|
||||
}, calls)
|
||||
|
||||
const result = await createBlankWorkspace({
|
||||
client,
|
||||
repoId: 'repo-1',
|
||||
baseName: 'octopus',
|
||||
startupCommand: undefined,
|
||||
createdWithAgentId: undefined,
|
||||
comment: undefined,
|
||||
setupDecision: 'inherit'
|
||||
})
|
||||
|
||||
expect(result).toEqual({ worktreeId: 'wt-3', name: 'octopus-2' })
|
||||
expect(calls).toHaveLength(2)
|
||||
const retryParams = calls[1]?.params as Record<string, unknown>
|
||||
expect(retryParams.name).toBe('octopus-2')
|
||||
})
|
||||
|
||||
it('retries on the bare older-runtime collision message', async () => {
|
||||
const calls: Call[] = []
|
||||
const client = fakeClient((_method, call) => {
|
||||
if (call === 1) {
|
||||
return new Error('Branch "octopus" already exists.')
|
||||
}
|
||||
return { worktree: { id: 'wt-4' } }
|
||||
}, calls)
|
||||
|
||||
const result = await createBlankWorkspace({
|
||||
client,
|
||||
repoId: 'repo-1',
|
||||
baseName: 'octopus',
|
||||
startupCommand: undefined,
|
||||
createdWithAgentId: undefined,
|
||||
comment: undefined,
|
||||
setupDecision: 'inherit'
|
||||
})
|
||||
|
||||
expect(result).toEqual({ worktreeId: 'wt-4', name: 'octopus-2' })
|
||||
expect(calls).toHaveLength(2)
|
||||
})
|
||||
|
||||
it('surfaces a non-collision error without retrying', async () => {
|
||||
const calls: Call[] = []
|
||||
const client = fakeClient(() => new Error('SSH connection is not available'), calls)
|
||||
|
||||
const result = await createBlankWorkspace({
|
||||
client,
|
||||
repoId: 'repo-1',
|
||||
baseName: 'octopus',
|
||||
startupCommand: undefined,
|
||||
createdWithAgentId: undefined,
|
||||
comment: undefined,
|
||||
setupDecision: 'skip'
|
||||
})
|
||||
|
||||
expect(result).toEqual({ error: 'SSH connection is not available' })
|
||||
expect(calls).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
@@ -4,6 +4,7 @@ import { createWorkspaceFromSource } from './source-workspace-create'
|
||||
import {
|
||||
buildBranchSource,
|
||||
buildGitHubTaskSource,
|
||||
buildGitLabTaskSource,
|
||||
buildLinearTaskSource,
|
||||
buildNewBranchSource
|
||||
} from './workspace-source-selection'
|
||||
@@ -69,6 +70,44 @@ describe('createWorkspaceFromSource', () => {
|
||||
expect(createParams.pushTarget).toEqual({ remoteName: 'origin', branchName: 'feat' })
|
||||
})
|
||||
|
||||
it('resolves the MR base then creates a workspace linked to the GitLab MR', async () => {
|
||||
const calls: Call[] = []
|
||||
const client = fakeClient((method) => {
|
||||
if (method === 'worktree.resolveMrBase') {
|
||||
return { baseBranch: 'develop', pushTarget: { remoteName: 'origin', branchName: 'fix' } }
|
||||
}
|
||||
return { worktree: { id: 'wt-mr' } }
|
||||
}, calls)
|
||||
|
||||
const result = await createWorkspaceFromSource({
|
||||
client,
|
||||
source: buildGitLabTaskSource('repo-2', {
|
||||
type: 'mr',
|
||||
number: 7,
|
||||
title: 'Fix logout',
|
||||
url: 'https://gitlab.com/acme/app/-/merge_requests/7',
|
||||
branchName: 'fix',
|
||||
isCrossRepository: true
|
||||
}) as never,
|
||||
targetRepoId: 'repo-2',
|
||||
setupDecision: 'inherit',
|
||||
agent: { choice: 'blank', startupCommand: undefined },
|
||||
workspaceName: undefined,
|
||||
note: undefined
|
||||
})
|
||||
|
||||
expect(result).toEqual({ worktreeId: 'wt-mr', name: 'mr-7' })
|
||||
expect(calls[0]).toEqual({
|
||||
method: 'worktree.resolveMrBase',
|
||||
params: { repo: 'id:repo-2', mrIid: 7, sourceBranch: 'fix', isCrossRepository: true }
|
||||
})
|
||||
expect(calls[1]?.method).toBe('worktree.create')
|
||||
const createParams = calls[1]?.params as Record<string, unknown>
|
||||
expect(createParams.linkedGitLabMR).toBe(7)
|
||||
expect(createParams.baseBranch).toBe('develop')
|
||||
expect(createParams.pushTarget).toEqual({ remoteName: 'origin', branchName: 'fix' })
|
||||
})
|
||||
|
||||
it('creates a branch workspace with baseBranch + branchNameOverride and no resolve call', async () => {
|
||||
const calls: Call[] = []
|
||||
const client = fakeClient(() => ({ worktree: { id: 'wt-2' } }), calls)
|
||||
|
||||
Reference in New Issue
Block a user