mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
perf(renderer): keep the repos array identity across no-op refetches (#13744)
* perf(renderer): keep the repos array identity across no-op refetches reconcileFetchedRepos deliberately returns the previous array when a refetch changes nothing, so identity-keyed memos can skip work. Two later steps in the same chain copied unconditionally and threw that identity away: - reconcileReadoptedSshRepoRows spread the input on its no-prune path, which is the common case. - applyManualRepoOrder allocated a fresh array even when the saved order moved nothing. Both now return the input when they change nothing, so state.repos stays referentially stable through fetchRepos, fetchRuntimeEnvironmentRepos, fetchReposForAllHosts, and hydratePersistedUI. Return type stays Repo[] with the same cast reconcileFetchedRepos already uses; all four call sites only read the result. Co-authored-by: Orca <help@stably.ai> * refactor(renderer): make the store repos array readonly at the type level Preserving the repos array identity means handing callers the same array that is live store state, which previously relied on `as Repo[]` casts to launder readonly inputs back into a mutable field. A cast is a footgun: the next person to add a .push or .sort corrupts store state with no type error. Widen RepoSlice['repos'] to readonly Repo[] and propagate honestly. Consumers that only read take readonly Repo[]; genuine local accumulators are annotated Repo[] and built from copies. Removes all four pre-existing `as Repo[]` casts in the reconcile chain (repo-identity-reconcile, superseded-ssh-repo-rows, manual-repo-order x2) — this lands with fewer casts than main has today. Type-only change; no runtime behavior differs. Co-authored-by: Orca <help@stably.ai> * fix(renderer): compare nested repo fields so reconciliation actually fires Preserving the repos array identity was inert. reconcileFetchedRepos compares repo fields with !==, but every repo the renderer receives carries nested records that are new objects on every fetch: - main's hydrateRepo unconditionally rebuilds hookSettings for every repo, even a pristine one (persistence.ts:5109) - IPC structured-clone (and the JSON hop for SSH/runtime hosts) reclones gitRemoteIdentity, upstream, repoIcon, and the path arrays So every repo compared unequal, `identical` went false on every refresh, and the array was rebuilt regardless of the copies removed in the parent commit. Compare nested plain records structurally instead. They are small sanitized values; anything non-plain falls back to reference equality. The end-to-end test previously passed for a fixture-only reason: its repo had five scalar fields and the mock returned the same object both calls, the one shape production never produces. It now uses a production-shaped repo and fails without this change. Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -149,7 +149,7 @@ function PreflightBanner({
|
||||
repos
|
||||
}: {
|
||||
issues: PreflightIssue[]
|
||||
repos: Repo[]
|
||||
repos: readonly Repo[]
|
||||
}): React.JSX.Element | null {
|
||||
// Why: keying the seed on the current GitHub project set means adding a new
|
||||
// GitHub project (which changes the key) re-evaluates dismissals, so a lapsed
|
||||
|
||||
@@ -84,7 +84,7 @@ type NewWorkspaceComposerCardProps = {
|
||||
nameInputRef?: React.RefObject<HTMLInputElement | null>
|
||||
quickAgent: TuiAgent | null
|
||||
onQuickAgentChange: (agent: TuiAgent | null) => void
|
||||
eligibleRepos: RepoOption[]
|
||||
eligibleRepos: readonly RepoOption[]
|
||||
repoId: string
|
||||
projectOptions?: NewWorkspaceProjectOption[]
|
||||
selectedProjectId?: string | null
|
||||
@@ -98,7 +98,7 @@ type NewWorkspaceComposerCardProps = {
|
||||
selectedEphemeralVmRecipeId?: string | null
|
||||
onEphemeralVmRecipeChange?: (recipeId: string | null) => void
|
||||
ephemeralVmRecipeError?: string | null
|
||||
repoBackedSearchRepos?: RepoOption[]
|
||||
repoBackedSearchRepos?: readonly RepoOption[]
|
||||
repoBackedSourcesDisabled?: boolean
|
||||
allowSmartNameAddProject?: boolean
|
||||
smartNameRepoSwitchTarget?: 'project' | 'task-source'
|
||||
|
||||
@@ -61,7 +61,7 @@ type AutomationEditorDialogProps = {
|
||||
isSaving: boolean
|
||||
canSave: boolean
|
||||
createTarget: AutomationCreateTarget
|
||||
repos: Repo[]
|
||||
repos: readonly Repo[]
|
||||
projectHostSetups: ProjectHostSetup[]
|
||||
automationYamlHooksByRepoKey: Record<string, OrcaHooks | null>
|
||||
getAutomationHooksCacheKey: (repoId: string) => string
|
||||
|
||||
@@ -31,7 +31,7 @@ type AutomationEditorDialogFooterProps = {
|
||||
isHermesCreate: boolean
|
||||
isSaving: boolean
|
||||
canSave: boolean
|
||||
repos: Repo[]
|
||||
repos: readonly Repo[]
|
||||
projectHostSetups: ProjectHostSetup[]
|
||||
automationYamlHooksByRepoKey: Record<string, OrcaHooks | null>
|
||||
getAutomationHooksCacheKey: (repoId: string) => string
|
||||
|
||||
@@ -19,7 +19,7 @@ import {
|
||||
} from './automation-project-groups'
|
||||
|
||||
type AutomationProjectComboboxProps = {
|
||||
repos: Repo[]
|
||||
repos: readonly Repo[]
|
||||
value: string
|
||||
onValueChange: (repoId: string) => void
|
||||
placeholder?: string
|
||||
|
||||
@@ -94,8 +94,22 @@ describe('buildDashboardWorktreeLaunchOptions', () => {
|
||||
const options = buildDashboardWorktreeLaunchOptions(
|
||||
state({
|
||||
repos: [
|
||||
{ id: 'repo-ssh', connectionId: 'ssh-1' },
|
||||
{ id: 'repo-runtime', executionHostId: 'runtime:hub-1' }
|
||||
{
|
||||
id: 'repo-ssh',
|
||||
path: '/repo-ssh',
|
||||
displayName: 'repo-ssh',
|
||||
badgeColor: 'blue',
|
||||
addedAt: 1,
|
||||
connectionId: 'ssh-1'
|
||||
},
|
||||
{
|
||||
id: 'repo-runtime',
|
||||
path: '/repo-runtime',
|
||||
displayName: 'repo-runtime',
|
||||
badgeColor: 'blue',
|
||||
addedAt: 1,
|
||||
executionHostId: 'runtime:hub-1'
|
||||
}
|
||||
] as LaunchState['repos'],
|
||||
worktreesByRepo: {
|
||||
'repo-ssh': [{ id: 'ssh-worktree', repoId: 'repo-ssh' }],
|
||||
|
||||
@@ -22,7 +22,7 @@ import { parsePaneKey } from '../../../../shared/stable-pane-id'
|
||||
type RetainedAgentSnapshot = Map<string, { row: DashboardAgentRow; worktreeId: string }>
|
||||
|
||||
type RetainedAgentsSyncInputs = {
|
||||
repos: Repo[]
|
||||
repos: readonly Repo[]
|
||||
worktreesByRepo: Record<string, Worktree[]>
|
||||
folderWorkspaces: FolderWorkspace[]
|
||||
tabsByWorktree: Record<string, TerminalTab[]>
|
||||
@@ -38,7 +38,7 @@ function paneKeyTabId(paneKey: string): string | null {
|
||||
}
|
||||
|
||||
function buildLiveTabIndex(args: {
|
||||
repos: Repo[]
|
||||
repos: readonly Repo[]
|
||||
worktreesByRepo: Record<string, Worktree[]>
|
||||
folderWorkspaces: FolderWorkspace[]
|
||||
tabsByWorktree: Record<string, TerminalTab[]>
|
||||
|
||||
@@ -23,7 +23,7 @@ function storageKey(issueId: string): string {
|
||||
|
||||
/** GitHub-backed project identity keys for the current repo set, de-duped so
|
||||
* the same GitHub project added twice doesn't read as two distinct projects. */
|
||||
export function githubProjectKeys(repos: Repo[]): string[] {
|
||||
export function githubProjectKeys(repos: readonly Repo[]): string[] {
|
||||
const keys = repos
|
||||
.filter((repo) => isGitHubBackedRepo(repo))
|
||||
.map((repo) => getProjectIdentityKey(repo))
|
||||
@@ -45,7 +45,7 @@ function readRecord(issueId: string): DismissalRecord | null {
|
||||
|
||||
/** True when the issue was dismissed and no new GitHub project has appeared
|
||||
* since. A GitHub key present now but absent from the snapshot re-surfaces it. */
|
||||
export function isPreflightIssueDismissed(issueId: string, repos: Repo[]): boolean {
|
||||
export function isPreflightIssueDismissed(issueId: string, repos: readonly Repo[]): boolean {
|
||||
const record = readRecord(issueId)
|
||||
if (!record) {
|
||||
return false
|
||||
@@ -55,7 +55,7 @@ export function isPreflightIssueDismissed(issueId: string, repos: Repo[]): boole
|
||||
return !hasNewGithubProject
|
||||
}
|
||||
|
||||
export function dismissPreflightIssue(issueId: string, repos: Repo[]): void {
|
||||
export function dismissPreflightIssue(issueId: string, repos: readonly Repo[]): void {
|
||||
try {
|
||||
const record: DismissalRecord = { githubKeys: githubProjectKeys(repos) }
|
||||
localStorage.setItem(storageKey(issueId), JSON.stringify(record))
|
||||
|
||||
@@ -114,7 +114,7 @@ type RepoOption = ReturnType<typeof useAppStore.getState>['repos'][number]
|
||||
const EMPTY_REPO_SEARCH_REPOS: readonly RepoOption[] = []
|
||||
|
||||
type SmartWorkspaceNameFieldProps = {
|
||||
repos: RepoOption[]
|
||||
repos: readonly RepoOption[]
|
||||
repoId: string
|
||||
onRepoChange: (repoId: string) => void
|
||||
value: string
|
||||
|
||||
@@ -19,7 +19,7 @@ import RepoBadgeLabel from './RepoBadgeLabel'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
|
||||
type RepoComboboxProps = {
|
||||
repos: Repo[]
|
||||
repos: readonly Repo[]
|
||||
value: string
|
||||
onValueChange: (repoId: string) => void
|
||||
onValueSelected?: (repoId: string) => void
|
||||
|
||||
@@ -217,7 +217,7 @@ describe('useGitStatusPolling rerender stability', () => {
|
||||
useAppStore.setState({
|
||||
repos: [{ ...repo, connectionId: 'ssh-1' }],
|
||||
sshConnectionStates: new Map([
|
||||
['ssh-1', { status: 'disconnected', error: null, reconnectAttempt: 0 }]
|
||||
['ssh-1', { targetId: 'ssh-1', status: 'disconnected', error: null, reconnectAttempt: 0 }]
|
||||
])
|
||||
} as Partial<AppState>)
|
||||
await renderHook()
|
||||
|
||||
@@ -21,7 +21,7 @@ function ScopeTriggerLabel({
|
||||
}: {
|
||||
showAll: boolean
|
||||
effectiveSelection: ReadonlySet<string>
|
||||
repos: Repo[]
|
||||
repos: readonly Repo[]
|
||||
}): React.JSX.Element {
|
||||
if (showAll) {
|
||||
return (
|
||||
@@ -57,7 +57,7 @@ export function QuickCommandsScopeFilter({
|
||||
handleSelectAll,
|
||||
toggleScope
|
||||
}: {
|
||||
repos: Repo[]
|
||||
repos: readonly Repo[]
|
||||
effectiveSelection: ReadonlySet<string>
|
||||
showAll: boolean
|
||||
scopePopoverOpen: boolean
|
||||
|
||||
@@ -117,7 +117,16 @@ describe('resource usage open slices', () => {
|
||||
})
|
||||
|
||||
it('gates repo and worktree slices only while closed', () => {
|
||||
const repos = [{ id: 'repo-1', path: '/repo', kind: 'git' }] as AppState['repos']
|
||||
const repos = [
|
||||
{
|
||||
id: 'repo-1',
|
||||
path: '/repo',
|
||||
displayName: 'repo',
|
||||
badgeColor: 'blue',
|
||||
addedAt: 1,
|
||||
kind: 'git'
|
||||
}
|
||||
] as AppState['repos']
|
||||
const row = worktree()
|
||||
const worktreesByRepo = {
|
||||
'repo-1': [row]
|
||||
|
||||
+1
-1
@@ -11,7 +11,7 @@ import { TerminalQuickCommandScopeField } from './TerminalQuickCommandScopeField
|
||||
|
||||
type TerminalQuickCommandAdvancedSectionProps = {
|
||||
draft: TerminalQuickCommand
|
||||
repos: Pick<Repo, 'id' | 'displayName' | 'path' | 'badgeColor'>[]
|
||||
repos: readonly Pick<Repo, 'id' | 'displayName' | 'path' | 'badgeColor'>[]
|
||||
advancedOpen: boolean
|
||||
selectedScope: ReturnType<typeof getTerminalQuickCommandScope>
|
||||
selectedRepoId: string
|
||||
|
||||
@@ -39,7 +39,7 @@ type TerminalQuickCommandDialogProps = {
|
||||
open: boolean
|
||||
mode: TerminalQuickCommandDialogMode
|
||||
command: TerminalQuickCommand
|
||||
repos?: Pick<Repo, 'id' | 'displayName' | 'path' | 'badgeColor'>[]
|
||||
repos?: readonly Pick<Repo, 'id' | 'displayName' | 'path' | 'badgeColor'>[]
|
||||
onOpenChange: (open: boolean) => void
|
||||
onSave: (command: TerminalQuickCommand) => void
|
||||
}
|
||||
|
||||
+2
-2
@@ -18,7 +18,7 @@ import { QUICK_COMMAND_TOGGLE_ITEM_CLASS } from './terminal-quick-command-toggle
|
||||
import { translate } from '@/i18n/i18n'
|
||||
|
||||
type TerminalQuickCommandScopeFieldProps = {
|
||||
repos: Pick<Repo, 'id' | 'displayName' | 'path' | 'badgeColor'>[]
|
||||
repos: readonly Pick<Repo, 'id' | 'displayName' | 'path' | 'badgeColor'>[]
|
||||
selectedScope: TerminalQuickCommandScope
|
||||
selectedRepoId: string
|
||||
selectedRepoMissing: boolean
|
||||
@@ -32,7 +32,7 @@ function getRepoLabel(repo: Pick<Repo, 'displayName' | 'path'>): string {
|
||||
}
|
||||
|
||||
export function getQuickCommandProjectScopeRepoId(
|
||||
repos: Pick<Repo, 'id'>[],
|
||||
repos: readonly Pick<Repo, 'id'>[],
|
||||
lastRepoScopeId: string | null
|
||||
): string | null {
|
||||
return lastRepoScopeId ?? repos[0]?.id ?? null
|
||||
|
||||
@@ -16,7 +16,7 @@ import RepoBadgeLabel from '@/components/repo/RepoBadgeLabel'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
|
||||
type RepoMultiComboboxProps = {
|
||||
repos: Repo[]
|
||||
repos: readonly Repo[]
|
||||
/** Currently selected repo ids. The component enforces `selected.size >= 1`
|
||||
* by disabling the last-selected checkbox. */
|
||||
selected: ReadonlySet<string>
|
||||
@@ -32,7 +32,10 @@ type RepoMultiComboboxProps = {
|
||||
triggerClassName?: string
|
||||
}
|
||||
|
||||
function renderTriggerLabel(repos: Repo[], selected: ReadonlySet<string>): React.JSX.Element {
|
||||
function renderTriggerLabel(
|
||||
repos: readonly Repo[],
|
||||
selected: ReadonlySet<string>
|
||||
): React.JSX.Element {
|
||||
if (repos.length === 0) {
|
||||
return (
|
||||
<span className="text-muted-foreground">
|
||||
|
||||
@@ -1,16 +1,20 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { Repo } from '../../../shared/types'
|
||||
import { folderWorkspaceKey } from '../../../shared/workspace-scope'
|
||||
import { getAgentDetectionTargetKeyForWorktree } from './useAgentDetectionTarget'
|
||||
|
||||
describe('getAgentDetectionTargetKeyForWorktree', () => {
|
||||
it('uses an explicit runtime owner without scanning ambiguous child SSH repos', () => {
|
||||
let projectGroupReads = 0
|
||||
const repos = Array.from({ length: 100 }, (_, index) => {
|
||||
const repo = {
|
||||
const repos: readonly Repo[] = Array.from({ length: 100 }, (_, index) => {
|
||||
const repo: Repo = {
|
||||
id: `repo-${index}`,
|
||||
connectionId: `ssh-${index}`,
|
||||
executionHostId: `ssh:ssh-${index}`,
|
||||
path: `/workspace/repo-${index}`
|
||||
path: `/workspace/repo-${index}`,
|
||||
displayName: `repo-${index}`,
|
||||
badgeColor: 'blue',
|
||||
addedAt: 1
|
||||
}
|
||||
Object.defineProperty(repo, 'projectGroupId', {
|
||||
enumerable: true,
|
||||
|
||||
@@ -291,7 +291,10 @@ describe('ensureHooksConfirmed', () => {
|
||||
repos: [
|
||||
{
|
||||
id: 'repo-1',
|
||||
path: '/repo-1',
|
||||
displayName: 'Repo One',
|
||||
badgeColor: 'blue',
|
||||
addedAt: 1,
|
||||
hookSettings: {
|
||||
mode: 'auto',
|
||||
commandSourcePolicy: 'local-only',
|
||||
@@ -318,7 +321,10 @@ describe('ensureHooksConfirmed', () => {
|
||||
repos: [
|
||||
{
|
||||
id: 'repo-1',
|
||||
path: '/repo-1',
|
||||
displayName: 'Repo One',
|
||||
badgeColor: 'blue',
|
||||
addedAt: 1,
|
||||
hookSettings: {
|
||||
mode: 'auto',
|
||||
scripts: { setup: 'echo local', archive: '' }
|
||||
|
||||
@@ -6,7 +6,7 @@ import { parseExecutionHostId } from '../../../shared/execution-host'
|
||||
export type FolderWorkspaceConnectionState = {
|
||||
folderWorkspaces: FolderWorkspace[]
|
||||
projectGroups: ProjectGroup[]
|
||||
repos: Repo[]
|
||||
repos: readonly Repo[]
|
||||
}
|
||||
|
||||
function getFolderScopeCandidateRepos(args: {
|
||||
|
||||
@@ -35,7 +35,7 @@ type StoreAccessor = () => {
|
||||
> | null
|
||||
setActiveWorktree: (worktreeId: string) => void
|
||||
createBrowserTab: (worktreeId: string, url: string, opts: { activate: boolean }) => unknown
|
||||
repos?: LocalhostLinkRepo[]
|
||||
repos?: readonly LocalhostLinkRepo[]
|
||||
projects?: LocalhostLinkProject[]
|
||||
worktreesByRepo?: Record<string, LocalhostLinkWorktree[]>
|
||||
allWorktrees?: () => LocalhostLinkWorktree[]
|
||||
|
||||
@@ -16,11 +16,15 @@ afterEach(() => {
|
||||
resetLocalPreflightContextCachesForTests()
|
||||
})
|
||||
|
||||
function makeRepos(id: string, path: string): AppState['repos'] {
|
||||
return [{ id, path, displayName: id, badgeColor: 'blue', addedAt: 1 }]
|
||||
}
|
||||
|
||||
function makeWslState(distro: string): AppState {
|
||||
return {
|
||||
activeRepoId: 'repo-1',
|
||||
activeWorktreeId: null,
|
||||
repos: [{ id: 'repo-1', path: `\\\\wsl.localhost\\${distro}\\home\\alice\\repo` }],
|
||||
repos: makeRepos('repo-1', `\\\\wsl.localhost\\${distro}\\home\\alice\\repo`),
|
||||
worktreesByRepo: {}
|
||||
} as AppState
|
||||
}
|
||||
@@ -29,7 +33,7 @@ function makeWindowsProjectState(projectId: string): AppState {
|
||||
return {
|
||||
activeRepoId: projectId,
|
||||
activeWorktreeId: null,
|
||||
repos: [{ id: projectId, path: `C:\\Users\\alice\\${projectId}` }],
|
||||
repos: makeRepos(projectId, `C:\\Users\\alice\\${projectId}`),
|
||||
settings: {},
|
||||
worktreesByRepo: {}
|
||||
} as AppState
|
||||
|
||||
@@ -11,26 +11,30 @@ import {
|
||||
} from './local-preflight-context'
|
||||
|
||||
function makeState(args: {
|
||||
repoPath?: string | null
|
||||
repoPath?: string
|
||||
worktreePath?: string | null
|
||||
repo?: Partial<Repo>
|
||||
worktree?: Partial<Worktree>
|
||||
}): AppState {
|
||||
const repoId = 'repo-1'
|
||||
const worktreeId = `${repoId}::worktree-1`
|
||||
const repos: AppState['repos'] =
|
||||
args.repoPath === undefined
|
||||
? []
|
||||
: [
|
||||
{
|
||||
id: repoId,
|
||||
path: args.repoPath,
|
||||
displayName: repoId,
|
||||
badgeColor: 'blue',
|
||||
addedAt: 1,
|
||||
...args.repo
|
||||
}
|
||||
]
|
||||
return {
|
||||
activeRepoId: repoId,
|
||||
activeWorktreeId: args.worktreePath === undefined ? null : worktreeId,
|
||||
repos:
|
||||
args.repoPath === undefined
|
||||
? []
|
||||
: [
|
||||
{
|
||||
id: repoId,
|
||||
path: args.repoPath,
|
||||
...args.repo
|
||||
}
|
||||
],
|
||||
repos,
|
||||
worktreesByRepo:
|
||||
args.worktreePath === undefined
|
||||
? {}
|
||||
|
||||
@@ -38,7 +38,7 @@ function matchScore(repo: Repo, query: string): number | null {
|
||||
return null
|
||||
}
|
||||
|
||||
export function searchRepos(repos: Repo[], rawQuery: string): Repo[] {
|
||||
export function searchRepos(repos: readonly Repo[], rawQuery: string): readonly Repo[] {
|
||||
if (isRepoSearchQueryTooLarge(rawQuery)) {
|
||||
return []
|
||||
}
|
||||
|
||||
@@ -126,7 +126,7 @@ async function resolveRepoSlug(
|
||||
}
|
||||
|
||||
async function buildIndex(
|
||||
repos: Repo[],
|
||||
repos: readonly Repo[],
|
||||
settings: Pick<GlobalSettings, 'activeRuntimeEnvironmentId'> | null | undefined
|
||||
): Promise<{ index: SlugIndex; upstreamIndex: SlugIndex; retryDelayMs: number | null }> {
|
||||
// Why: evict cached entries for repos that no longer exist in state so
|
||||
|
||||
@@ -9,8 +9,8 @@ import { localhostWorktreeLabelRouteForPort } from './workspace-port-localhost-l
|
||||
// this is the single source for both reactive and imperative call sites.
|
||||
type LocalhostLabelLookupState = {
|
||||
settings?: Pick<GlobalSettings, 'localhostWorktreeLabelsEnabled'> | null
|
||||
repos?: Repo[]
|
||||
projects?: Project[]
|
||||
repos?: readonly Repo[]
|
||||
projects?: readonly Project[]
|
||||
getKnownWorktreeById?: (worktreeId: string) => { projectId?: string | null } | null | undefined
|
||||
}
|
||||
|
||||
|
||||
@@ -327,6 +327,10 @@ describe('createDetectedAgentsSlice WSL context', () => {
|
||||
projects: [
|
||||
{
|
||||
id: 'repo-1',
|
||||
displayName: 'repo-1',
|
||||
badgeColor: 'blue',
|
||||
createdAt: 1,
|
||||
updatedAt: 1,
|
||||
sourceRepoIds: ['repo-1'],
|
||||
localWindowsRuntimePreference: { kind: 'windows-host' }
|
||||
}
|
||||
|
||||
@@ -6534,17 +6534,18 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => {
|
||||
_meta: { runtimeId: 'remote-runtime' }
|
||||
})
|
||||
const store = createTestStore()
|
||||
const repos: AppState['repos'] = [
|
||||
{
|
||||
id: 'runtime-repo-id',
|
||||
path: '/server/repo',
|
||||
displayName: 'repo',
|
||||
badgeColor: 'blue',
|
||||
addedAt: 1
|
||||
}
|
||||
]
|
||||
store.setState({
|
||||
settings: { activeRuntimeEnvironmentId: 'env-1' },
|
||||
repos: [
|
||||
{
|
||||
id: 'runtime-repo-id',
|
||||
path: '/server/repo',
|
||||
displayName: 'repo',
|
||||
badgeColor: 'blue',
|
||||
addedAt: 1
|
||||
}
|
||||
]
|
||||
repos
|
||||
} as Partial<AppState>)
|
||||
|
||||
await store.getState().fetchWorkItems('caller-repo-id', '/server/repo', 24, 'is:open', {
|
||||
@@ -6634,17 +6635,18 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => {
|
||||
_meta: { runtimeId: 'source-runtime' }
|
||||
})
|
||||
const store = createTestStore()
|
||||
const repos: AppState['repos'] = [
|
||||
{
|
||||
id: 'local-repo-id',
|
||||
path: '/server/repo',
|
||||
displayName: 'repo',
|
||||
badgeColor: 'blue',
|
||||
addedAt: 1
|
||||
}
|
||||
]
|
||||
store.setState({
|
||||
settings: { activeRuntimeEnvironmentId: 'focused-runtime' },
|
||||
repos: [
|
||||
{
|
||||
id: 'local-repo-id',
|
||||
path: '/server/repo',
|
||||
displayName: 'repo',
|
||||
badgeColor: 'blue',
|
||||
addedAt: 1
|
||||
}
|
||||
]
|
||||
repos
|
||||
} as Partial<AppState>)
|
||||
|
||||
const sourceContext = {
|
||||
|
||||
@@ -265,11 +265,12 @@ describe('createPreflightSlice', () => {
|
||||
new Error('Project runtime requires repair before preflight: wsl-distro-required')
|
||||
)
|
||||
const store = createTestStore()
|
||||
const repos: AppState['repos'] = [makeRepo({ id: 'repo-1', path: 'C:\\repo' })]
|
||||
store.setState({
|
||||
settings: {
|
||||
localWindowsRuntimeDefault: { kind: 'wsl', distro: null }
|
||||
},
|
||||
repos: [makeRepo({ id: 'repo-1', path: 'C:\\repo' })],
|
||||
repos,
|
||||
worktreesByRepo: {},
|
||||
activeRepoId: 'repo-1',
|
||||
activeWorktreeId: null
|
||||
|
||||
@@ -14,6 +14,50 @@ describe('reconcileFetchedRepos', () => {
|
||||
expect(reconcileFetchedRepos(previous, next)).toBe(previous)
|
||||
})
|
||||
|
||||
it('reconciles repos whose nested records were rebuilt by hydration and IPC', () => {
|
||||
// Why: main's hydrateRepo reconstructs hookSettings on every list and structured-clone
|
||||
// rebuilds the rest, so a reference compare would report every real repo as changed.
|
||||
const nested = (): Partial<Repo> => ({
|
||||
hookSettings: { mode: 'auto', scripts: { setup: 'echo hi', archive: '' } },
|
||||
gitRemoteIdentity: {
|
||||
canonicalKey: 'github.com/o/n',
|
||||
remoteName: 'origin',
|
||||
remoteUrl: 'git@github.com:o/n.git'
|
||||
},
|
||||
importedExternalWorktreePaths: ['/a', '/b']
|
||||
})
|
||||
const previous = [makeRepo('a', nested())]
|
||||
const next = structuredClone([makeRepo('a', nested())]) as Repo[]
|
||||
|
||||
expect(reconcileFetchedRepos(previous, next)).toBe(previous)
|
||||
})
|
||||
|
||||
it('treats a changed nested field as a real change', () => {
|
||||
const previous = [makeRepo('a', { importedExternalWorktreePaths: ['/a'] })]
|
||||
const next = [makeRepo('a', { importedExternalWorktreePaths: ['/b'] })]
|
||||
const result = reconcileFetchedRepos(previous, next)
|
||||
|
||||
expect(result).not.toBe(previous)
|
||||
expect(result[0].importedExternalWorktreePaths).toEqual(['/b'])
|
||||
})
|
||||
|
||||
it('treats a nested field gaining a key as a real change', () => {
|
||||
const previous = [
|
||||
makeRepo('a', { hookSettings: { mode: 'auto', scripts: { setup: '', archive: '' } } })
|
||||
]
|
||||
const next = [
|
||||
makeRepo('a', {
|
||||
hookSettings: {
|
||||
mode: 'auto',
|
||||
setupRunPolicy: 'run-by-default',
|
||||
scripts: { setup: '', archive: '' }
|
||||
}
|
||||
})
|
||||
]
|
||||
|
||||
expect(reconcileFetchedRepos(previous, next)).not.toBe(previous)
|
||||
})
|
||||
|
||||
it('reuses unchanged repo objects while reflecting a reorder', () => {
|
||||
const previous = [makeRepo('a'), makeRepo('b')]
|
||||
const next = [makeRepo('b'), makeRepo('a')]
|
||||
|
||||
@@ -8,6 +8,45 @@ import { getRepoHostIdentity } from './repo-host-identity'
|
||||
// virtualizer to rebuild + re-measure a tick after the drop — the visible jump.
|
||||
// Reusing equal objects (and the whole array when nothing moved) makes the echo
|
||||
// a no-op render.
|
||||
// Why: `Repo` carries nested records (hookSettings, upstream, gitRemoteIdentity, repoIcon, path
|
||||
// arrays). IPC structured-clone rebuilds those every fetch, and main's hydrateRepo always
|
||||
// reconstructs hookSettings — so a reference compare reports every repo as changed and no repo
|
||||
// ever reconciles. Compare nested plain values structurally; they are small sanitized records.
|
||||
function areValuesEqual(a: unknown, b: unknown): boolean {
|
||||
if (a === b) {
|
||||
return true
|
||||
}
|
||||
if (typeof a !== 'object' || typeof b !== 'object' || a === null || b === null) {
|
||||
return false
|
||||
}
|
||||
if (Array.isArray(a) || Array.isArray(b)) {
|
||||
return (
|
||||
Array.isArray(a) &&
|
||||
Array.isArray(b) &&
|
||||
a.length === b.length &&
|
||||
a.every((item, index) => areValuesEqual(item, b[index]))
|
||||
)
|
||||
}
|
||||
// Why: only plain records are safe to walk — anything exotic falls back to reference equality.
|
||||
if (
|
||||
Object.getPrototypeOf(a) !== Object.prototype ||
|
||||
Object.getPrototypeOf(b) !== Object.prototype
|
||||
) {
|
||||
return false
|
||||
}
|
||||
const aRecord = a as Record<string, unknown>
|
||||
const bRecord = b as Record<string, unknown>
|
||||
const keys = Object.keys(aRecord)
|
||||
if (keys.length !== Object.keys(bRecord).length) {
|
||||
return false
|
||||
}
|
||||
return keys.every(
|
||||
(key) =>
|
||||
Object.prototype.hasOwnProperty.call(bRecord, key) &&
|
||||
areValuesEqual(aRecord[key], bRecord[key])
|
||||
)
|
||||
}
|
||||
|
||||
function areReposEqual(a: Repo, b: Repo): boolean {
|
||||
if (a === b) {
|
||||
return true
|
||||
@@ -20,14 +59,17 @@ function areReposEqual(a: Repo, b: Repo): boolean {
|
||||
if (!Object.prototype.hasOwnProperty.call(b, key)) {
|
||||
return false
|
||||
}
|
||||
if (a[key] !== b[key]) {
|
||||
if (!areValuesEqual(a[key], b[key])) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
export function reconcileFetchedRepos(previous: readonly Repo[], next: Repo[]): Repo[] {
|
||||
export function reconcileFetchedRepos(
|
||||
previous: readonly Repo[],
|
||||
next: readonly Repo[]
|
||||
): readonly Repo[] {
|
||||
const previousById = new Map(previous.map((repo) => [getRepoHostIdentity(repo), repo]))
|
||||
let identical = next.length === previous.length
|
||||
const reconciled = next.map((repo, index) => {
|
||||
@@ -41,5 +83,5 @@ export function reconcileFetchedRepos(previous: readonly Repo[], next: Repo[]):
|
||||
identical = false
|
||||
return repo
|
||||
})
|
||||
return identical ? (previous as Repo[]) : reconciled
|
||||
return identical ? previous : reconciled
|
||||
}
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { createTestStore, makeWorktree } from './store-test-helpers'
|
||||
import { workItemsCacheKey } from './github'
|
||||
import type { Project, ProjectHostSetup } from '../../../../shared/types'
|
||||
import type { Project, ProjectHostSetup, Repo } from '../../../../shared/types'
|
||||
import { toast } from 'sonner'
|
||||
import {
|
||||
installReposRuntimeRoutingHarness,
|
||||
@@ -52,6 +52,46 @@ describe('repo slice runtime routing', () => {
|
||||
expect(runtimeEnvironmentCall).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps the repos array identity across a refetch that changes nothing', async () => {
|
||||
// Why: main rebuilds nested records (hookSettings et al) per list and IPC clones them, so a
|
||||
// production-shaped repo — not a scalar-only fixture — is what proves reconciliation works.
|
||||
const hydrated = (): Repo => ({
|
||||
...localRepo,
|
||||
kind: 'git',
|
||||
gitUsername: 'octocat',
|
||||
hookSettings: { mode: 'auto', scripts: { setup: 'echo hi', archive: '' } },
|
||||
gitRemoteIdentity: {
|
||||
canonicalKey: 'github.com/octocat/local',
|
||||
remoteName: 'origin',
|
||||
remoteUrl: 'git@github.com:octocat/local.git'
|
||||
},
|
||||
importedExternalWorktreePaths: ['/local/wt']
|
||||
})
|
||||
reposList.mockImplementation(async () => [hydrated()])
|
||||
const store = createTestStore()
|
||||
await store.getState().fetchRepos()
|
||||
const reposRef = store.getState().repos
|
||||
|
||||
await store.getState().fetchRepos()
|
||||
|
||||
// Why: identity-keyed renderer memos (repo lookup index, selectors) rebuild on a new array.
|
||||
expect(store.getState().repos).toBe(reposRef)
|
||||
expect(store.getState().repos[0]).toBe(reposRef[0])
|
||||
})
|
||||
|
||||
it('replaces the repos array identity when a refetch adds a repo', async () => {
|
||||
reposList.mockResolvedValue([localRepo])
|
||||
const store = createTestStore()
|
||||
await store.getState().fetchRepos()
|
||||
const reposRef = store.getState().repos
|
||||
reposList.mockResolvedValue([localRepo, { ...localRepo, id: 'second', path: '/second' }])
|
||||
|
||||
await store.getState().fetchRepos()
|
||||
|
||||
expect(store.getState().repos).not.toBe(reposRef)
|
||||
expect(store.getState().repos).toHaveLength(2)
|
||||
})
|
||||
|
||||
it('fetches repos from the active remote runtime environment', async () => {
|
||||
runtimeEnvironmentCall.mockResolvedValue({
|
||||
id: 'rpc-1',
|
||||
|
||||
@@ -881,9 +881,9 @@ function mergeByIdentity<T>(
|
||||
|
||||
function mergeFetchedReposForHost(
|
||||
previous: readonly Repo[],
|
||||
fetched: Repo[],
|
||||
fetched: readonly Repo[],
|
||||
hostId: string
|
||||
): Repo[] {
|
||||
): readonly Repo[] {
|
||||
const fetchedWithProjectGroups = applyInheritedProjectGroups(previous, fetched)
|
||||
const fetchedIdentities = new Set(fetchedWithProjectGroups.map(getRepoHostIdentity))
|
||||
const preserved = previous.filter((repo) => {
|
||||
@@ -1050,7 +1050,7 @@ function mergeFetchedFolderWorkspacesForHost({
|
||||
}
|
||||
|
||||
type FetchedRepoCatalog = {
|
||||
repos: Repo[]
|
||||
repos: readonly Repo[]
|
||||
projectHostSetupCompatibility: ProjectHostSetupProjection
|
||||
hostId: ReturnType<typeof getRuntimeTargetHostId>
|
||||
}
|
||||
@@ -1111,7 +1111,7 @@ function mergeFetchedRepoCatalog(
|
||||
catalog: FetchedRepoCatalog,
|
||||
currentRepos: readonly Repo[]
|
||||
): {
|
||||
repos: Repo[]
|
||||
repos: readonly Repo[]
|
||||
projectHostSetupCompatibility: ProjectHostSetupProjection
|
||||
hostId: ReturnType<typeof getRuntimeTargetHostId>
|
||||
} {
|
||||
@@ -1620,7 +1620,7 @@ function getFolderWorkspacePathStatusRequestSnapshotForRead(
|
||||
}
|
||||
|
||||
export type RepoSlice = {
|
||||
repos: Repo[]
|
||||
repos: readonly Repo[]
|
||||
projects: Project[]
|
||||
projectHostSetups: ProjectHostSetup[]
|
||||
projectGroups: ProjectGroup[]
|
||||
|
||||
@@ -31,6 +31,12 @@ describe('reconcileReadoptedSshRepoRows', () => {
|
||||
expect(result.pendingReadoptions).toEqual([])
|
||||
})
|
||||
|
||||
it('returns the input array when nothing is pruned', () => {
|
||||
const repos = [repo({ id: 'shared', path: '/local' })]
|
||||
|
||||
expect(reconcileReadoptedSshRepoRows(repos, []).repos).toBe(repos)
|
||||
})
|
||||
|
||||
it('keeps evidence pending when repos:changed has not delivered the new row yet', () => {
|
||||
const oldSsh = repo({ id: 'shared', connectionId: 'ssh-old' })
|
||||
|
||||
|
||||
@@ -3,7 +3,7 @@ import type { Repo } from '../../../../shared/types'
|
||||
import { getRepoExecutionHostId, toSshExecutionHostId } from '../../../../shared/execution-host'
|
||||
|
||||
export type SshRepoReconciliation = {
|
||||
repos: Repo[]
|
||||
repos: readonly Repo[]
|
||||
pendingReadoptions: SshRepoReadoption[]
|
||||
}
|
||||
|
||||
@@ -53,8 +53,10 @@ export function reconcileReadoptedSshRepoRows(
|
||||
}
|
||||
}
|
||||
|
||||
// Why: pruning nothing must hand back the input array, or the copy alone would defeat the
|
||||
// referential stability reconcileFetchedRepos just established upstream.
|
||||
if (prunedOwners.size === 0) {
|
||||
return { repos: [...repos], pendingReadoptions }
|
||||
return { repos, pendingReadoptions }
|
||||
}
|
||||
return {
|
||||
repos: repos.filter(
|
||||
|
||||
@@ -14,6 +14,7 @@ import type {
|
||||
WorktreeLineage,
|
||||
WorkspaceLineage,
|
||||
ProjectHostSetup,
|
||||
Repo,
|
||||
WorktreeMeta
|
||||
} from '../../../../shared/types'
|
||||
import type { RuntimeWorktreeListResult } from '../../../../shared/runtime-types'
|
||||
@@ -6066,7 +6067,7 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
|
||||
const repoIdsWithRemovedOwners = new Set<string>()
|
||||
const survivingRepoIds = new Set<string>()
|
||||
const repoIdsWithSurvivingOwners = new Set<string>()
|
||||
const survivingRepos: AppState['repos'] = []
|
||||
const survivingRepos: Repo[] = []
|
||||
for (const repo of s.repos) {
|
||||
if (isRemovedRuntimeHostId(getRepoExecutionHostId(repo), removed)) {
|
||||
repoIdsWithRemovedOwners.add(repo.id)
|
||||
|
||||
@@ -27,6 +27,26 @@ describe('manual repo order', () => {
|
||||
expect(applyManualRepoOrder([localBravo, localAlpha], [])).toEqual([localBravo, localAlpha])
|
||||
})
|
||||
|
||||
it('returns the input array when no overlay exists', () => {
|
||||
const repos = [localBravo, localAlpha]
|
||||
|
||||
expect(applyManualRepoOrder(repos, [])).toBe(repos)
|
||||
})
|
||||
|
||||
it('returns the input array when the saved order moves nothing', () => {
|
||||
const repos = [localAlpha, remoteCharlie, localBravo, remoteDelta]
|
||||
|
||||
expect(applyManualRepoOrder(repos, getManualRepoOrder(repos))).toBe(repos)
|
||||
})
|
||||
|
||||
it('returns a new array when the saved order actually reorders', () => {
|
||||
const repos = [localBravo, localAlpha]
|
||||
const reordered = applyManualRepoOrder(repos, getManualRepoOrder([localAlpha, localBravo]))
|
||||
|
||||
expect(reordered).not.toBe(repos)
|
||||
expect(reordered).toEqual([localAlpha, localBravo])
|
||||
})
|
||||
|
||||
it('restores a host-qualified cross-host interleaving', () => {
|
||||
const order = getManualRepoOrder([localAlpha, remoteCharlie, localBravo, remoteDelta])
|
||||
|
||||
|
||||
@@ -41,13 +41,15 @@ export function getManualRepoOrder(repos: readonly Repo[]): ManualRepoOrderEntry
|
||||
export function applyManualRepoOrder(
|
||||
repos: readonly Repo[],
|
||||
order: readonly ManualRepoOrderEntry[] | null | undefined
|
||||
): Repo[] {
|
||||
): readonly Repo[] {
|
||||
const normalized = normalizeManualRepoOrder(order)
|
||||
// Why: results flow straight back into `repos`, so a reorder that moves nothing must return
|
||||
// the input — a fresh array would invalidate every identity-keyed repo memo downstream.
|
||||
if (normalized.length === 0) {
|
||||
return [...repos]
|
||||
return repos
|
||||
}
|
||||
const rankByKey = new Map(normalized.map((entry, index) => [getEntryKey(entry), index]))
|
||||
return repos
|
||||
const ordered = repos
|
||||
.map((repo, index) => ({
|
||||
repo,
|
||||
index,
|
||||
@@ -66,4 +68,5 @@ export function applyManualRepoOrder(
|
||||
return a.rank - b.rank || a.index - b.index
|
||||
})
|
||||
.map(({ repo }) => repo)
|
||||
return ordered.every((repo, index) => repo === repos[index]) ? repos : ordered
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user