refactor(renderer): no rebound-settings owner pattern (#27607)

* refactor(renderer): no rebound-settings owner pattern

* chore: lower focus-read baseline for composer owner scalar

---------

Co-authored-by: m4air <m4air@Mac.localdomain>
This commit is contained in:
OrcaWin
2026-10-11 15:34:24 -07:00
committed by GitHub
co-authored by m4air
parent 5e9fb931a1
commit c9730df4c8
22 changed files with 141 additions and 445 deletions
+3 -6
View File
@@ -277,12 +277,9 @@
2 src/renderer/src/components/use-worktree-jump-palette-store-state.ts
1 src/renderer/src/components/use-worktree-jump-palette-task-url.ts
1 src/renderer/src/components/worktree-jump-palette-create-worktree.ts
1 src/renderer/src/hooks/composer-state/attachment-drop-state.ts
1 src/renderer/src/hooks/composer-state/composer-attachment-drop-state.ts
1 src/renderer/src/hooks/composer-state/composer-navigation-actions.ts
2 src/renderer/src/hooks/composer-state/composer-source-state.ts
1 src/renderer/src/hooks/composer-state/composer-target-state.ts
1 src/renderer/src/hooks/composer-state/workspace-identity-state.ts
1 src/renderer/src/hooks/composer-state/composer-source-state.ts
1 src/renderer/src/hooks/composer-state/runtime-target-selection.ts
1 src/renderer/src/hooks/editor-external-watch-target-rows.ts
3 src/renderer/src/hooks/editor-external-watch-targets.ts
1 src/renderer/src/hooks/ipc-events-close-routing-test-harness.ts
@@ -345,7 +342,7 @@
2 src/renderer/src/lib/provider-runtime-context.ts
2 src/renderer/src/lib/remap-open-editor-tabs-for-path-change.ts
1 src/renderer/src/lib/remote-workspace-drop-test-fixtures.ts
3 src/renderer/src/lib/repo-runtime-owner.ts
2 src/renderer/src/lib/repo-runtime-owner.ts
2 src/renderer/src/lib/resume-sleeping-agent-session.ts
1 src/renderer/src/lib/runtime-session-mirror-owners.ts
1 src/renderer/src/lib/runtime-session-mirror-targets.ts
@@ -1,207 +0,0 @@
#!/usr/bin/env node
// Benchmark: cost of the owner-routed settings selector per store write.
//
// getSettingsForRepoRuntimeOwner() is called from useShallow selectors at ~43
// sites across PullRequestPage and TaskPage. Zustand re-runs every subscribed
// selector on every store write, so before the fix each unrelated write
// allocated one settings-sized object per row and then shallow-compared every
// field to conclude nothing had changed.
//
// The fix caches by repo id and reuses the reference while the settings object,
// repo list, and resolved owner are unchanged, so useShallow's equality check
// short-circuits on Object.is.
//
// The selector body is mirrored here (node cannot import the .ts source, matching
// the sibling benchmarks). The settings field count is read from the real
// GlobalSettings type so a drifted shape fails loudly instead of flattering the
// result with a stale, smaller object.
import { readFileSync } from 'node:fs'
import { performance } from 'node:perf_hooks'
import { fileURLToPath } from 'node:url'
const TYPES_SOURCE = readFileSync(
fileURLToPath(new URL('../../src/shared/types.ts', import.meta.url)),
'utf8'
)
function countGlobalSettingsFields(source) {
const block = source.match(/export type GlobalSettings = \{([\s\S]*?)\n\}/)
if (!block) {
throw new Error('types.ts no longer declares GlobalSettings in the expected shape')
}
const fields = block[1].match(/^\s{2}\w+\??:/gm) ?? []
if (fields.length < 50) {
throw new Error(`GlobalSettings parsed as only ${fields.length} fields; re-sync this benchmark`)
}
return fields.length
}
const SETTINGS_FIELDS = countGlobalSettingsFields(TYPES_SOURCE)
const ROWS = Number.parseInt(process.env.ORCA_OWNER_SETTINGS_BENCH_ROWS ?? '43', 10)
const WRITES = Number.parseInt(process.env.ORCA_OWNER_SETTINGS_BENCH_WRITES ?? '2000', 10)
const WARMUP = Number.parseInt(process.env.ORCA_OWNER_SETTINGS_BENCH_WARMUP ?? '200', 10)
for (const [name, value] of [
['ORCA_OWNER_SETTINGS_BENCH_ROWS', ROWS],
['ORCA_OWNER_SETTINGS_BENCH_WRITES', WRITES],
['ORCA_OWNER_SETTINGS_BENCH_WARMUP', WARMUP]
]) {
if (!Number.isInteger(value) || value <= 0) {
throw new Error(`${name} must be a positive integer, received ${value}`)
}
}
function makeSettings() {
const settings = { activeRuntimeEnvironmentId: 'focused-runtime' }
for (let index = 0; index < SETTINGS_FIELDS - 1; index += 1) {
settings[`field${index}`] = index % 3 === 0 ? `value-${index}` : index % 3 === 1 ? index : true
}
return settings
}
function makeRepos(rows) {
return Array.from({ length: rows }, (_value, index) => ({
id: `repo-${index}`,
connectionId: null,
executionHostId: `runtime:env-${index % 4}`
}))
}
function resolveEnvironmentId(state, repoId) {
if (!repoId) {
return null
}
const matching = state.repos.filter((entry) => entry.id === repoId)
const repo = matching.length === 1 ? matching[0] : null
const hasOwner = Boolean(repo?.executionHostId?.trim() || repo?.connectionId?.trim())
if (repo && hasOwner) {
const hostId = repo.executionHostId ?? ''
return hostId.startsWith('runtime:') ? hostId.slice('runtime:'.length) : null
}
return state.settings?.activeRuntimeEnvironmentId?.trim() || null
}
// Pre-fix: a fresh object every call.
function selectorBefore(state, repoId) {
return { ...state.settings, activeRuntimeEnvironmentId: resolveEnvironmentId(state, repoId) }
}
// Post-fix: reuse the reference while nothing it derives from changed.
function makeCachedSelector() {
const cache = new Map()
return (state, repoId) => {
const environmentId = resolveEnvironmentId(state, repoId)
const cacheKey = repoId ?? ''
const cached = cache.get(cacheKey)
if (
cached &&
cached.settingsSource === state.settings &&
cached.reposSource === state.repos &&
cached.environmentId === environmentId
) {
return cached.value
}
const value = { ...state.settings, activeRuntimeEnvironmentId: environmentId }
cache.set(cacheKey, {
settingsSource: state.settings,
reposSource: state.repos,
environmentId,
value
})
if (cache.size > 256) {
const oldest = cache.keys().next()
if (!oldest.done) {
cache.delete(oldest.value)
}
}
return value
}
}
// Mirror of zustand's useShallow comparison.
function shallowEqual(a, b) {
if (Object.is(a, b)) {
return true
}
if (a === null || b === null || a === undefined || b === undefined) {
return false
}
const keysA = Object.keys(a)
const keysB = Object.keys(b)
if (keysA.length !== keysB.length) {
return false
}
for (const key of keysA) {
if (!Object.is(a[key], b[key])) {
return false
}
}
return true
}
// One unrelated store write: every subscribed row re-runs its selector and the
// result is compared against the previous one to decide whether to re-render.
function simulateWrite(selector, state, rows, previous) {
let changed = 0
for (let row = 0; row < rows; row += 1) {
const next = selector(state, `repo-${row}`)
if (!shallowEqual(previous[row], next)) {
changed += 1
}
previous[row] = next
}
return changed
}
function measure(selector, state, rows) {
const previous = Array.from({ length: rows }, () => null)
for (let index = 0; index < WARMUP; index += 1) {
simulateWrite(selector, state, rows, previous)
}
const samples = []
for (let round = 0; round < 5; round += 1) {
const start = performance.now()
for (let index = 0; index < WRITES; index += 1) {
simulateWrite(selector, state, rows, previous)
}
samples.push((performance.now() - start) / WRITES)
}
samples.sort((a, b) => a - b)
return samples[2]
}
const settings = makeSettings()
const rowCounts = [1, 10, ROWS, 100]
const rows = []
for (const rowCount of rowCounts) {
const state = { repos: makeRepos(Math.max(rowCount, ROWS)), settings }
const cached = makeCachedSelector()
// Equivalence: both selectors must produce the same value for every row.
for (let row = 0; row < rowCount; row += 1) {
const before = selectorBefore(state, `repo-${row}`)
const after = cached(state, `repo-${row}`)
if (!shallowEqual(before, after)) {
throw new Error(`selector mismatch at repo-${row}`)
}
}
rows.push({
rowCount,
beforeMs: measure(selectorBefore, state, rowCount),
afterMs: measure(makeCachedSelector(), state, rowCount)
})
}
const pad = (value, width) => String(value).padStart(width)
console.log(`Owner-routed settings selector, per unrelated store write`)
console.log(
`GlobalSettings fields=${SETTINGS_FIELDS} writes=${WRITES} warmup=${WARMUP} (median of 5 rounds)`
)
console.log(`${pad('rows', 6)} ${pad('before ms', 11)} ${pad('after ms', 10)} ${pad('speedup', 9)}`)
for (const row of rows) {
console.log(
`${pad(row.rowCount, 6)} ${pad(row.beforeMs.toFixed(4), 11)} ${pad(row.afterMs.toFixed(4), 10)} ${pad(`${(row.beforeMs / row.afterMs).toFixed(0)}x`, 9)}`
)
}
console.log(
`\nrows = subscribed selector call sites on screen (~${ROWS} across PullRequestPage/TaskPage).\nThe cost is paid on EVERY store write, including writes that touch nothing\nthese selectors read.`
)
@@ -136,7 +136,6 @@ describe('new workspace card file drop ownership', () => {
selectedRepo: { id: 'source-repo' },
selectedRepoPath: source,
selectedRepoExecutionHostId: 'ssh:ssh-a',
selectedRepoSettings: {},
connectionId: 'ssh-a'
})
const dropState = useAttachmentDropState({
@@ -146,7 +145,7 @@ describe('new workspace card file drop ownership', () => {
promptTextareaRef: createRef<HTMLTextAreaElement>(),
connectionId: target.connectionId,
selectedRepoPath: target.path ?? undefined,
selectedRepoSettings: target.settings,
selectedRepoRuntimeEnvironmentId: target.runtimeEnvironmentId,
selectedWorktreeId: target.worktreeId,
setAgentPrompt: () => {},
setAttachmentPaths: attach
@@ -0,0 +1,64 @@
// @vitest-environment happy-dom
import { renderHook } from '@testing-library/react'
import { describe, expect, it } from 'vitest'
import type { Repo } from '../../../../shared/repo-types'
import { useAutomationsPageSetupState } from './use-automations-page-setup-state'
type HookInput = Parameters<typeof useAutomationsPageSetupState>[0]
function cacheKeyFor(repos: Partial<Repo>[], focused: string | null, repoId = 'repo-1'): string {
const { result } = renderHook(() =>
useAutomationsPageSetupState({
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the cache key reads only repos and settings.
store: {
repos,
settings: { activeRuntimeEnvironmentId: focused },
projectHostSetups: [],
worktreesByRepo: {}
} as unknown as HookInput['store'],
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: inert for the cache key.
local: {
draft: { projectId: repoId },
automationHostTargetKey: null,
selectedAutomationRuns: { rowKey: null, ownerKey: null, notice: null },
selectedAutomationRunPageId: null
} as unknown as HookInput['local'],
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: inert for the cache key.
list: {
selected: null,
selectedRow: null,
selectedAutomationRunsWithWorkspaceNames: [],
capturedAutomationOwners: {},
hostCatalog: { entries: [] }
} as unknown as HookInput['list']
})
)
return result.current.getAutomationHooksCacheKey(repoId)
}
describe('automation hooks cache key', () => {
it.each<[string, Partial<Repo>[], string | null, string]>([
[
'runtime owner',
[{ id: 'repo-1', executionHostId: 'runtime:owner' }],
'focused',
'owner:repo-1'
],
['local owner', [{ id: 'repo-1', executionHostId: 'local' }], 'focused', 'local:repo-1'],
['unstamped row follows focus', [{ id: 'repo-1' }], 'focused', 'focused:repo-1'],
['missing row follows focus', [], 'focused', 'focused:repo-1'],
['missing row, local focus', [], null, 'local:repo-1'],
[
'duplicates fall back to focus',
[
{ id: 'repo-1', executionHostId: 'local' },
{ id: 'repo-1', executionHostId: 'runtime:owner' }
],
'other',
'other:repo-1'
]
])('%s', (_name, repos, focused, expected) => {
expect(cacheKeyFor(repos, focused)).toBe(expected)
})
})
@@ -1,6 +1,6 @@
import { useCallback, useMemo } from 'react'
import type { OrcaHooks } from '../../../../shared/orca-yaml-hook-types'
import { getSettingsForRepoRuntimeOwner } from '@/lib/repo-runtime-owner'
import { getRuntimeEnvironmentIdForRepo } from '@/lib/repo-runtime-owner'
import { checkRuntimeHooks } from '@/runtime/runtime-hooks-client'
import type { AutomationDraft } from './AutomationEditorDialog'
import { getAutomationHostTargetFromKey, type AutomationHostTarget } from './automation-host-client'
@@ -72,14 +72,7 @@ export function useAutomationsPageSetupState({
(setup) => setup.repoId !== candidate.projectId || setup.hostId === setupHostId
)
: projectHostSetups
const settingsForRepo = setupHostId
? {
...settings,
activeRuntimeEnvironmentId:
selectedEditAuthority?.kind === 'runtime' ? selectedEditAuthority.environmentId : null
}
: getSettingsForRepoRuntimeOwner({ repos, settings }, candidate.projectId)
const hookKey = `${setupHostId ?? settingsForRepo.activeRuntimeEnvironmentId ?? 'local'}:${candidate.projectId}`
const hookKey = `${setupHostId ?? getRuntimeEnvironmentIdForRepo({ repos, settings }, candidate.projectId) ?? 'local'}:${candidate.projectId}`
return getVisibleAutomationSetupDecision({
createTarget,
workspaceMode: candidate.workspaceMode,
@@ -105,8 +98,7 @@ export function useAutomationsPageSetupState({
if (hostId) {
return `${hostId}:${repoId}`
}
const settingsForRepo = getSettingsForRepoRuntimeOwner({ repos, settings }, repoId)
return `${settingsForRepo.activeRuntimeEnvironmentId ?? 'local'}:${repoId}`
return `${getRuntimeEnvironmentIdForRepo({ repos, settings }, repoId) ?? 'local'}:${repoId}`
},
[repos, settings]
)
@@ -19,7 +19,6 @@ type ComposerAsyncStateInput = Pick<
| 'selectedRepoConnectionId'
| 'selectedRepoHookContextKey'
| 'selectedRepoIsGit'
| 'selectedRepoSettings'
| 'setName'
>
@@ -55,7 +54,6 @@ export function useComposerAsyncState(input: ComposerAsyncStateInput) {
selectedRepoConnectionId,
selectedRepoHookContextKey,
selectedRepoIsGit,
selectedRepoSettings,
setName
} = input
const { getInitialAutoManagedWorkspaceName, getInitialGitHubPrStartPointSelection } = decisions
@@ -209,15 +207,12 @@ export function useComposerAsyncState(input: ComposerAsyncStateInput) {
const selectedRepoPathRef = useRef<string | undefined>(selectedRepoPath)
const selectedRepoSettingsRef = useRef(selectedRepoSettings)
useEffect(() => {
agentPromptRef.current = agentPrompt
connectionIdRef.current = connectionId
selectedRepoConnectionIdRef.current = selectedRepoConnectionId
selectedRepoPathRef.current = selectedRepoPath
selectedRepoSettingsRef.current = selectedRepoSettings
}, [agentPrompt, connectionId, selectedRepoConnectionId, selectedRepoPath, selectedRepoSettings])
}, [agentPrompt, connectionId, selectedRepoConnectionId, selectedRepoPath])
return {
yamlHooks,
@@ -279,7 +274,6 @@ export function useComposerAsyncState(input: ComposerAsyncStateInput) {
selectedRepoSlug,
setSelectedRepoSlug,
selectedRepoPath,
selectedRepoPathRef,
selectedRepoSettingsRef
selectedRepoPathRef
}
}
@@ -2,7 +2,6 @@ import type * as React from 'react'
import type { RefObject } from 'react'
import type { GitHubWorkItem } from '../../../../shared/github/work-item-types'
import type { OrcaHooks, SetupAgentStartupPolicy } from '../../../../shared/orca-yaml-hook-types'
import type { GlobalSettings } from '../../../../shared/global-settings-types'
import type { GitHubRepositoryIdentity } from '../../../../shared/github/pull-request-types'
import type { WorkspaceCreateErrorDisplay } from '@/lib/workspace-create-error-format'
import type { IssueCommandReadResult } from '@/runtime/runtime-hooks-client'
@@ -78,5 +77,4 @@ export type ComposerAsyncModel = {
setSelectedRepoSlug: React.Dispatch<React.SetStateAction<GitHubRepositoryIdentity | null>>
selectedRepoPath: string | undefined
selectedRepoPathRef: RefObject<string | undefined>
selectedRepoSettingsRef: RefObject<Pick<GlobalSettings, 'activeRuntimeEnvironmentId'> | null>
}
@@ -52,7 +52,7 @@ function renderDropState(setAttachmentPaths: Dispatch<SetStateAction<string[]>>)
promptCaretFrameRef: { current: null },
promptTextareaRef: createRef<HTMLTextAreaElement>(),
selectedRepoPath: '/repo',
selectedRepoSettings: null,
selectedRepoRuntimeEnvironmentId: null,
selectedWorktreeId: null,
setAgentPrompt: () => {},
setAttachmentPaths
@@ -106,7 +106,7 @@ describe('local composer drop failures', () => {
})
// Why: the upload branch returns early unless a runtime environment or connection is resolved.
const RUNTIME_SETTINGS = { activeRuntimeEnvironmentId: 'env-1' }
const RUNTIME_ENVIRONMENT_ID = 'env-1'
describe('composer upload failures', () => {
it('aggregates a mixed runtime import into one toast, and withholds a reason that is not shared', async () => {
@@ -128,7 +128,7 @@ describe('composer upload failures', () => {
await act(async () => {
await result.current.uploadComposerPaths(
['/a.png', '/b.png', '/c.png'],
RUNTIME_SETTINGS,
RUNTIME_ENVIRONMENT_ID,
null,
'/repo'
)
@@ -155,7 +155,7 @@ describe('composer upload failures', () => {
const { result } = renderDropState(() => {})
await act(async () => {
await result.current.uploadComposerPaths(['/a.png'], RUNTIME_SETTINGS, null, '/repo')
await result.current.uploadComposerPaths(['/a.png'], RUNTIME_ENVIRONMENT_ID, null, '/repo')
})
expect(mocks.toastError).not.toHaveBeenCalled()
@@ -170,7 +170,7 @@ describe('composer upload failures', () => {
await act(async () => {
await result.current.uploadComposerPaths(
['/b.png'],
RUNTIME_SETTINGS,
RUNTIME_ENVIRONMENT_ID,
null,
'/repo',
null,
@@ -193,7 +193,7 @@ describe('composer upload failures', () => {
await act(async () => {
await result.current.uploadComposerPaths(
['/b.png', '/c.png'],
RUNTIME_SETTINGS,
RUNTIME_ENVIRONMENT_ID,
null,
'/repo'
)
@@ -25,7 +25,7 @@ function renderDrop(strict = false) {
promptCaretFrameRef: { current: null },
promptTextareaRef: createRef<HTMLTextAreaElement>(),
selectedRepoPath: '/folder-workspace',
selectedRepoSettings: null,
selectedRepoRuntimeEnvironmentId: null,
selectedWorktreeId: null,
setAgentPrompt: prompt,
setAttachmentPaths: attach
@@ -8,7 +8,7 @@ type AttachmentDropStateInput = Pick<
| 'promptCaretFrameRef'
| 'promptTextareaRef'
| 'selectedRepoPath'
| 'selectedRepoSettings'
| 'selectedRepoRuntimeEnvironmentId'
| 'setAgentPrompt'
| 'setAttachmentPaths'
> & {
@@ -54,7 +54,7 @@ export function useAttachmentDropState(input: AttachmentDropStateInput) {
promptCaretFrameRef,
promptTextareaRef,
selectedRepoPath,
selectedRepoSettings,
selectedRepoRuntimeEnvironmentId,
selectedWorktreeId,
setAgentPrompt,
setAttachmentPaths
@@ -126,14 +126,14 @@ export function useAttachmentDropState(input: AttachmentDropStateInput) {
const uploadComposerPaths = useCallback(
async (
sourcePaths: string[],
targetSettings = selectedRepoSettings,
targetEnvironmentId = selectedRepoRuntimeEnvironmentId,
targetConnectionId: string | null | undefined = connectionId,
targetRepoPath: string | null | undefined = selectedRepoPath,
targetWorktreeId: string | null | undefined = selectedWorktreeId,
canReportFailure: () => boolean = () => true
): Promise<{ filePaths: string[]; folderPaths: string[] } | null> => {
// Repo-owner settings: the selected repo's host, not the focused server.
const targetRuntimeEnvironmentId = targetSettings?.activeRuntimeEnvironmentId?.trim() || null
// The selected repo's host, not the focused server.
const targetRuntimeEnvironmentId = targetEnvironmentId?.trim() || null
if (!targetRuntimeEnvironmentId && !targetConnectionId) {
return null
}
@@ -198,7 +198,7 @@ export function useAttachmentDropState(input: AttachmentDropStateInput) {
}
return { filePaths: uploadResult.filePaths, folderPaths: uploadResult.folderPaths }
},
[connectionId, selectedRepoPath, selectedRepoSettings, selectedWorktreeId]
[connectionId, selectedRepoPath, selectedRepoRuntimeEnvironmentId, selectedWorktreeId]
)
const handleAddAttachment = useCallback(async (): Promise<void> => {
@@ -264,7 +264,7 @@ export function useAttachmentDropState(input: AttachmentDropStateInput) {
uploadPaths: (sourcePaths) =>
uploadComposerPaths(
sourcePaths,
selectedRepoSettings,
selectedRepoRuntimeEnvironmentId,
connectionId,
selectedRepoPath,
selectedWorktreeId,
@@ -283,7 +283,7 @@ export function useAttachmentDropState(input: AttachmentDropStateInput) {
connectionId,
insertComposerFolderPaths,
selectedRepoPath,
selectedRepoSettings,
selectedRepoRuntimeEnvironmentId,
selectedWorktreeId,
uploadComposerPaths
]
@@ -8,7 +8,6 @@ export function useComposerAttachmentDropState(target: ComposerTargetState) {
selectedRepoPath: target.asyncComposerState.selectedRepoPath,
selectedRepo: target.runtimeTargetSelection.selectedRepo,
selectedRepoExecutionHostId: target.runtimeTargetSelection.selectedRepoExecutionHostId,
selectedRepoSettings: target.runtimeTargetSelection.selectedRepoSettings,
connectionId: target.workspaceIdentityState.connectionId
})
return useAttachmentDropState({
@@ -18,7 +17,7 @@ export function useComposerAttachmentDropState(target: ComposerTargetState) {
promptCaretFrameRef: target.asyncComposerState.promptCaretFrameRef,
promptTextareaRef: target.asyncComposerState.promptTextareaRef,
selectedRepoPath: destination.path ?? undefined,
selectedRepoSettings: destination.settings,
selectedRepoRuntimeEnvironmentId: destination.runtimeEnvironmentId,
selectedWorktreeId: destination.worktreeId,
setAgentPrompt: target.sourceContextState.setAgentPrompt,
setAttachmentPaths: target.sourceContextState.setAttachmentPaths
@@ -19,25 +19,24 @@ const selected = {
selectedRepo: { id: 'repo-1' },
selectedRepoPath: '/repo',
selectedRepoExecutionHostId: 'local' as const,
selectedRepoSettings: { activeRuntimeEnvironmentId: 'focused-runtime' },
connectionId: null
}
describe('composer attachment destination', () => {
it('uses the selected local host even when a runtime is focused', () => {
it('uses the selected local host', () => {
expect(resolveComposerAttachmentTarget(selected)).toEqual({
hostId: 'local',
path: '/repo',
worktreeId: 'repo-1::/repo',
connectionId: null,
settings: { activeRuntimeEnvironmentId: null }
runtimeEnvironmentId: null
})
})
it('uses the selected runtime without borrowing the focused runtime', () => {
it('uses the selected runtime', () => {
expect(
resolveComposerAttachmentTarget({
...selected,
selectedRepoExecutionHostId: 'runtime:selected'
}).settings.activeRuntimeEnvironmentId
}).runtimeEnvironmentId
).toBe('selected')
})
it('uses a folder project path and recorded SSH host', () => {
@@ -52,7 +51,7 @@ describe('composer attachment destination', () => {
path: '/folder',
worktreeId: null,
connectionId: 'ssh-a',
settings: { activeRuntimeEnvironmentId: null }
runtimeEnvironmentId: null
})
})
it('uses a folder project runtime owner', () => {
@@ -67,7 +66,7 @@ describe('composer attachment destination', () => {
path: '/folder',
worktreeId: null,
connectionId: null,
settings: { activeRuntimeEnvironmentId: 'folder-host' }
runtimeEnvironmentId: 'folder-host'
})
})
it.each(['/folder/source-repo', '/outside/source-repo'])(
@@ -5,11 +5,7 @@ import type { ComposerModel } from './composer-model'
type TargetInput = Pick<
ComposerModel,
| 'selectedProjectGroup'
| 'selectedRepoPath'
| 'selectedRepoExecutionHostId'
| 'selectedRepoSettings'
| 'connectionId'
'selectedProjectGroup' | 'selectedRepoPath' | 'selectedRepoExecutionHostId' | 'connectionId'
> & { selectedRepo: Pick<Repo, 'id'> | undefined }
export function resolveComposerAttachmentTarget(input: TargetInput) {
@@ -30,9 +26,6 @@ export function resolveComposerAttachmentTarget(input: TargetInput) {
? null
: (group.connectionId ?? null)
: input.connectionId,
settings: {
...input.selectedRepoSettings,
activeRuntimeEnvironmentId: host?.kind === 'runtime' ? host.environmentId : null
}
runtimeEnvironmentId: host?.kind === 'runtime' ? host.environmentId : null
}
}
@@ -118,7 +118,6 @@ function renderComposerDrop(attached: string[]) {
selectedRepo: REPO,
selectedRepoPath: REPO.path,
selectedRepoExecutionHostId: `runtime:${ENVIRONMENT_ID}`,
selectedRepoSettings: { activeRuntimeEnvironmentId: null },
connectionId: null
})
return useAttachmentDropState({
@@ -128,7 +127,7 @@ function renderComposerDrop(attached: string[]) {
promptCaretFrameRef: { current: null },
promptTextareaRef: createRef<HTMLTextAreaElement>(),
selectedRepoPath: target.path ?? undefined,
selectedRepoSettings: target.settings,
selectedRepoRuntimeEnvironmentId: target.runtimeEnvironmentId,
selectedWorktreeId: target.worktreeId,
setAgentPrompt: () => {},
setAttachmentPaths: (next) => {
@@ -5,7 +5,6 @@ import type { JiraIssue } from '../../../../shared/jira-types'
import type { TaskSourceContext } from '../../../../shared/task-source-context'
import type { LinearIssue } from '../../../../shared/linear/issue-types'
import type { SparsePreset } from '../../../../shared/worktree/create-types'
import type { GlobalSettings } from '../../../../shared/global-settings-types'
import type { PendingSmartGitHubSubmitResolution } from './source-selection-decisions'
import type { SmartWorkspaceNameSelection } from '@/components/new-workspace/smart-workspace-name-field-model'
export type ComposerSourceModel = {
@@ -68,7 +67,7 @@ export type ComposerSourceModel = {
smartNameSelection: SmartWorkspaceNameSelection | null
uploadComposerPaths: (
sourcePaths: string[],
targetSettings?: Pick<GlobalSettings, 'activeRuntimeEnvironmentId'> | null,
targetEnvironmentId?: string | null,
targetConnectionId?: string | null,
targetRepoPath?: string | null,
targetWorktreeId?: string | null,
@@ -83,7 +83,7 @@ export function useComposerTargetState(
newWorkspaceDraft: composerTargetStore.newWorkspaceDraft,
persistDraft: composerTargetStore.persistDraft,
selectedRepoConnectionId: runtimeTargetSelection.selectedRepoConnectionId,
selectedRepoSettings: runtimeTargetSelection.selectedRepoSettings,
selectedRepoRuntimeEnvironmentId: runtimeTargetSelection.selectedRepoRuntimeEnvironmentId,
settings: composerTargetStore.settings
})
const asyncComposerState = useComposerAsyncState({
@@ -104,7 +104,6 @@ export function useComposerTargetState(
selectedRepoConnectionId: runtimeTargetSelection.selectedRepoConnectionId,
selectedRepoHookContextKey: runtimeTargetSelection.selectedRepoHookContextKey,
selectedRepoIsGit: runtimeTargetSelection.selectedRepoIsGit,
selectedRepoSettings: runtimeTargetSelection.selectedRepoSettings,
setName: sourceContextState.setName
})
const providerRuntimeSync = useComposerProviderRuntimeSync({
@@ -5,7 +5,6 @@ import type { Repo } from '../../../../shared/repo-types'
import type { SshConnectionState, SshConnectionStatus } from '../../../../shared/ssh-types'
import type { TuiAgent } from '../../../../shared/tui-agent'
import type { OrcaVmRecipe } from '../../../../shared/orca-yaml-hook-types'
import type { GlobalSettings } from '../../../../shared/global-settings-types'
import type { AgentStartupShell } from '../../../../shared/tui-agent-startup-shell'
import type { ExecutionHostPlatformFact } from '@/lib/execution-host-facts'
import type { NewWorkspaceProjectOption } from '@/lib/new-workspace-project-options'
@@ -45,7 +44,8 @@ export type ComposerRuntimeTargetModel = {
selectedProjectHostSetupId: string | null
projectHostSetupOptions: ProjectHostSetupOption[]
projectOptions: NewWorkspaceProjectOption[]
selectedRepoSettings: Pick<GlobalSettings, 'activeRuntimeEnvironmentId'> | null
/** Runtime that owns the selected repo; an unstamped repo follows focus. */
selectedRepoRuntimeEnvironmentId: string | null
/** Transport to the selected repo's owner; `null` with no repo selected. */
selectedRepoTarget: RuntimeClientTarget | null
selectedRecipeRepoId: string | null
@@ -22,7 +22,7 @@ import { useAppStore } from '@/store'
import { repoIsRemote } from '../../../../shared/agent-launch-remote'
import { buildProjectHostSetupOptions } from '@/lib/project-host-setup-options'
import { buildNewWorkspaceCreateTargetOptions } from '@/lib/new-workspace-project-options'
import { getSettingsForRepoRuntimeOwner } from '@/lib/repo-runtime-owner'
import { getRuntimeEnvironmentIdForRepo } from '@/lib/repo-runtime-owner'
import { useEphemeralVmRecipeOptions } from '@/hooks/useEphemeralVmRecipeOptions'
export function useComposerRuntimeTargetSelection(input: ComposerRuntimeTargetSelectionInput) {
@@ -212,16 +212,13 @@ export function useComposerRuntimeTargetSelection(input: ComposerRuntimeTargetSe
[eligibleRepos, hostOptions, projectGroups, projectHostSetups, projects]
)
const selectedRepoSettings = useMemo(() => {
if (!settings) {
return settings
}
// Why: probes and attachment uploads inspect the selected repo, even though creation defaults still follow host scope.
return getSettingsForRepoRuntimeOwner(
{ repos: selectedRepo ? [selectedRepo] : [], settings },
selectedRepo?.id ?? null
)
}, [selectedRepo, settings])
// Why: probes and attachment uploads inspect the selected repo, even though creation defaults still follow host scope.
const selectedRepoRuntimeEnvironmentId = settings
? getRuntimeEnvironmentIdForRepo(
{ repos: selectedRepo ? [selectedRepo] : [], settings },
selectedRepo?.id ?? null
)
: null
const selectedRepoTarget = useMemo(
() => (selectedRepo ? runtimeTargetForOwnerHostId(getRepoExecutionHostId(selectedRepo)) : null),
@@ -304,7 +301,7 @@ export function useComposerRuntimeTargetSelection(input: ComposerRuntimeTargetSe
selectedProjectHostSetupId,
projectHostSetupOptions,
projectOptions,
selectedRepoSettings,
selectedRepoRuntimeEnvironmentId,
selectedRepoTarget,
selectedRecipeRepoId,
selectedRecipeRepoConnectionId,
@@ -109,7 +109,7 @@ describe("agents for a project on a paired server's SSH host", () => {
persistDraft: false,
newWorkspaceDraft: null,
selectedRepoConnectionId: 'ssh-dike',
selectedRepoSettings: { activeRuntimeEnvironmentId: 'env-1' },
selectedRepoRuntimeEnvironmentId: 'env-1',
settings: null
} as unknown as Parameters<typeof useWorkspaceIdentityState>[0])
)
@@ -11,7 +11,7 @@ type WorkspaceIdentityStateInput = Pick<
| 'newWorkspaceDraft'
| 'persistDraft'
| 'selectedRepoConnectionId'
| 'selectedRepoSettings'
| 'selectedRepoRuntimeEnvironmentId'
| 'settings'
>
@@ -51,7 +51,7 @@ export function useWorkspaceIdentityState(input: WorkspaceIdentityStateInput) {
newWorkspaceDraft,
persistDraft,
selectedRepoConnectionId,
selectedRepoSettings,
selectedRepoRuntimeEnvironmentId,
settings
} = input
@@ -176,7 +176,7 @@ export function useWorkspaceIdentityState(input: WorkspaceIdentityStateInput) {
)
// Why: for a repo on an SSH host or runtime env, read the per-host agent list so the dialog shows the host's installed agents, not local.
const runtimeEnvironmentId = selectedRepoSettings?.activeRuntimeEnvironmentId?.trim() || null
const runtimeEnvironmentId = selectedRepoRuntimeEnvironmentId?.trim() || null
// Kept as-is: uploads fence on the worktree's real host, which for a server repo is its target.
const connectionId = selectedRepoConnectionId
+26 -101
View File
@@ -1,10 +1,8 @@
import { describe, expect, it } from 'vitest'
import type { GlobalSettings } from '../../../shared/global-settings-types'
import type { RepoRuntimeOwnerState } from './repo-runtime-owner'
import {
getExplicitRuntimeOwnerEnvironmentId,
getRuntimeEnvironmentIdForRepo,
getSettingsForRepoRuntimeOwner,
runtimeTargetForRepoOwner
} from './repo-runtime-owner'
@@ -86,17 +84,33 @@ describe('getRuntimeEnvironmentIdForRepo', () => {
)
).toBe('other-runtime')
})
})
it('returns settings scoped to an explicit local repo owner', () => {
expect(
getSettingsForRepoRuntimeOwner(
{
settings: { activeRuntimeEnvironmentId: 'focused-runtime' },
repos: [{ id: 'repo-1', connectionId: null, executionHostId: 'local' }]
},
'repo-1'
)
).toEqual({ activeRuntimeEnvironmentId: null })
type OwnerRow = NonNullable<RepoRuntimeOwnerState['repos']>[number]
const row = (
executionHostId: OwnerRow['executionHostId'],
connectionId: string | null = null
): OwnerRow => ({
id: 'repo-1',
connectionId,
executionHostId
})
// Composer passes a singleton or empty list; automations pass the whole catalog.
describe('getRuntimeEnvironmentIdForRepo owner matrix', () => {
it.each<[string, OwnerRow[], string | null, string | null]>([
['local row', [row('local')], 'focused', null],
['direct SSH row', [row(null, 'ssh-1')], 'focused', null],
['nested runtime row', [row('runtime:owner', 'ssh-1')], 'focused', 'owner'],
['unstamped row', [row(null)], 'focused', 'focused'],
['missing row', [], 'focused', 'focused'],
['duplicates, local focus', [row('local'), row('runtime:owner')], null, null],
['duplicates, matching focus', [row('local'), row('runtime:owner')], 'owner', 'owner'],
['duplicates, other focus', [row('local'), row('runtime:owner')], 'other', 'other'],
['duplicates, deleted focus', [row('runtime:a'), row('runtime:b')], 'deleted', 'deleted']
])('%s', (_name, repos, focused, expected) => {
const settings = { activeRuntimeEnvironmentId: focused }
expect(getRuntimeEnvironmentIdForRepo({ repos, settings }, 'repo-1')).toBe(expected)
})
})
@@ -194,95 +208,6 @@ describe('getExplicitRuntimeOwnerEnvironmentId', () => {
})
})
describe('getSettingsForRepoRuntimeOwner identity', () => {
type OwnerRepo = NonNullable<RepoRuntimeOwnerState['repos']>[number]
const repos: OwnerRepo[] = [
{ id: 'repo-1', connectionId: null, executionHostId: 'runtime:env-a' }
]
const settings = {
activeRuntimeEnvironmentId: 'focused',
sourceControlViewMode: 'list'
} as unknown as GlobalSettings
// Why identity matters: these run inside useShallow selectors, so a fresh
// object per store write costs a full settings-wide compare on every row.
it('returns the same reference while settings and repos are unchanged', () => {
const state = { repos, settings }
expect(getSettingsForRepoRuntimeOwner(state, 'repo-1')).toBe(
getSettingsForRepoRuntimeOwner(state, 'repo-1')
)
})
it('keeps separate identities per repo id', () => {
const state = { repos, settings }
const first = getSettingsForRepoRuntimeOwner(state, 'repo-1')
const second = getSettingsForRepoRuntimeOwner(state, 'repo-2')
expect(first).not.toBe(second)
expect(first.activeRuntimeEnvironmentId).toBe('env-a')
expect(second.activeRuntimeEnvironmentId).toBe('focused')
// Interleaving must not evict either entry.
expect(getSettingsForRepoRuntimeOwner(state, 'repo-1')).toBe(first)
expect(getSettingsForRepoRuntimeOwner(state, 'repo-2')).toBe(second)
})
it('returns a new value when the settings object changes', () => {
const first = getSettingsForRepoRuntimeOwner({ repos, settings }, 'repo-1')
const nextSettings = { ...settings, sourceControlViewMode: 'tree' } as unknown as GlobalSettings
const second = getSettingsForRepoRuntimeOwner({ repos, settings: nextSettings }, 'repo-1')
expect(second).not.toBe(first)
expect((second as { sourceControlViewMode?: string }).sourceControlViewMode).toBe('tree')
})
it('returns a new value when the repo owner changes', () => {
const first = getSettingsForRepoRuntimeOwner({ repos, settings }, 'repo-1')
const movedRepos: OwnerRepo[] = [
{ id: 'repo-1', connectionId: null, executionHostId: 'runtime:env-b' }
]
const second = getSettingsForRepoRuntimeOwner({ repos: movedRepos, settings }, 'repo-1')
expect(second).not.toBe(first)
expect(second.activeRuntimeEnvironmentId).toBe('env-b')
})
// Why isolate each source: a fixture that changes the repo list AND the
// resolved owner together still passes when only one of the two checks
// survives, so each gets a case that holds the other constant.
it('returns a new value when the repo list changes but the owner does not', () => {
const first = getSettingsForRepoRuntimeOwner({ repos, settings }, 'repo-1')
// Same owner for repo-1; only the array identity and an unrelated row differ.
const grownRepos: OwnerRepo[] = [
...repos,
{ id: 'repo-9', connectionId: null, executionHostId: 'local' }
]
const second = getSettingsForRepoRuntimeOwner({ repos: grownRepos, settings }, 'repo-1')
expect(second.activeRuntimeEnvironmentId).toBe('env-a')
expect(second).not.toBe(first)
})
it('returns a new value when the owner changes under a stable repo list identity', () => {
// Mutate in place so the array reference and settings both stay identical
// and only the resolved environment id differs.
const mutableRepos: OwnerRepo[] = [
{ id: 'repo-1', connectionId: null, executionHostId: 'runtime:env-a' }
]
const state = { repos: mutableRepos, settings }
const first = getSettingsForRepoRuntimeOwner(state, 'repo-1')
expect(first.activeRuntimeEnvironmentId).toBe('env-a')
mutableRepos[0]!.executionHostId = 'runtime:env-c'
const second = getSettingsForRepoRuntimeOwner(state, 'repo-1')
expect(second.activeRuntimeEnvironmentId).toBe('env-c')
expect(second).not.toBe(first)
})
it('bounds the cache so unbounded repo ids cannot leak', () => {
const state = { repos, settings }
const first = getSettingsForRepoRuntimeOwner(state, 'repo-evictable')
for (let index = 0; index < 300; index += 1) {
getSettingsForRepoRuntimeOwner(state, `repo-filler-${index}`)
}
expect(getSettingsForRepoRuntimeOwner(state, 'repo-evictable')).not.toBe(first)
})
})
describe('runtimeTargetForRepoOwner', () => {
const focused = { activeRuntimeEnvironmentId: 'focused-runtime' }
@@ -95,54 +95,3 @@ export function getHostlessSourceRepoOwnerEnvironmentId(
const parsed = parseExecutionHostId(getRepoExecutionHostId(matches[0]))
return parsed?.kind === 'runtime' ? parsed.environmentId : null
}
type OwnerRoutedSettingsCacheEntry = {
settingsSource: RepoRuntimeOwnerState['settings']
reposSource: RepoRuntimeOwnerState['repos']
environmentId: string | null
value: Pick<GlobalSettings, 'activeRuntimeEnvironmentId'>
}
// Why keyed by repo id: every PR/task row calls this with its own repo while
// sharing one settings object, so a single-entry cache would thrash.
const ownerRoutedSettingsCache = new Map<string, OwnerRoutedSettingsCacheEntry>()
// Why bounded: repo ids come from remote listings, so the key space is not closed.
const MAX_OWNER_ROUTED_SETTINGS_CACHE_KEYS = 256
export function getSettingsForRepoRuntimeOwner(
state: RepoRuntimeOwnerState,
repoId: string | null | undefined
): Pick<GlobalSettings, 'activeRuntimeEnvironmentId'> {
const environmentId = getRuntimeEnvironmentIdForRepo(state, repoId)
// Why a stable identity: this runs inside useShallow selectors on every store
// write, and spreading the whole settings object per row made unrelated writes
// allocate and re-compare every field. Reusing the reference when nothing it
// derives from changed lets useShallow settle on the Object.is fast path.
const cacheKey = repoId ?? ''
const cached = ownerRoutedSettingsCache.get(cacheKey)
if (
cached &&
cached.settingsSource === state.settings &&
cached.reposSource === state.repos &&
cached.environmentId === environmentId
) {
return cached.value
}
const value = {
...state.settings,
activeRuntimeEnvironmentId: environmentId
}
ownerRoutedSettingsCache.set(cacheKey, {
settingsSource: state.settings,
reposSource: state.repos,
environmentId,
value
})
if (ownerRoutedSettingsCache.size > MAX_OWNER_ROUTED_SETTINGS_CACHE_KEYS) {
const oldest = ownerRoutedSettingsCache.keys().next()
if (!oldest.done) {
ownerRoutedSettingsCache.delete(oldest.value)
}
}
return value
}