From 36ed2363945a305b50722d76991306335c19f6dd Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 19 Oct 2026 00:33:50 -0700 Subject: [PATCH] fix(persistence): repo writes take the row's identity, never a bare id (#30616) * fix(persistence): repo writes take the row's identity, never a bare id updateRepo(target, updates) now takes Pick (RepoHostTarget) and resolves the host from it, so a caller that holds the row can no longer drop its host and write a same-id sibling on another host. The id-only callers (project host setup controller and IPC handler, folder-to-Git upgrade watch, both clone paths, registration adoption, fork backfill) pass the row they resolved. Co-Authored-By: Claude * test: migrate the remaining Store.updateRepo callers to row targets Co-Authored-By: Claude --------- Co-authored-by: Claude --- .../automation-worker-durability.test.ts | 2 +- src/main/folder-repo-git-conversion.ts | 2 +- .../folder-repo-git-upgrade-lifetime.test.ts | 2 +- src/main/ipc/folder-repo-git-upgrade.test.ts | 19 +-- src/main/ipc/folder-repo-git-upgrade.ts | 2 +- .../repos-local-add-and-project-setup.test.ts | 14 +- .../ipc/repos-local-clone-lifecycle.test.ts | 3 +- src/main/ipc/repos-remote-test-harness.ts | 2 +- .../ipc/repos/project-host-setup-handlers.ts | 4 +- src/main/ipc/repos/repo-clone-lifecycle.ts | 2 +- src/main/ipc/repos/repo-update-handler.ts | 24 ++-- .../orcad-runtime-conversion.test.ts | 2 +- src/main/persistence-initial-load.test.ts | 15 ++- src/main/persistence-jira-project.test.ts | 8 +- ...ce-orcad-repository-cache-reimport.test.ts | 10 +- src/main/persistence-repo-lifecycle.test.ts | 13 +- src/main/persistence-update-repo.test.ts | 120 +++++++++++------- .../persistence-worktree-visibility.test.ts | 95 ++++++++------ .../repo-lifecycle-operations.ts | 10 +- .../repo-host-target-matcher.test-fixture.ts | 12 ++ .../project-host-setup-update.ts | 2 +- .../repo-update-host-guard.test.ts | 45 +++++-- .../tracking-repos/repo-update-operations.ts | 19 +-- src/main/repo-backfill-wsl-move.test.ts | 9 +- .../repo-git-remote-avatar-refresh.test.ts | 31 ++--- ...epo-git-remote-identity-enrichment.test.ts | 56 ++++---- .../repo-git-remote-identity-enrichment.ts | 13 +- ...knowledged-terminal-tab-retirement.test.ts | 2 +- .../orca-runtime-test-fixtures.spec.ts | 2 +- ...ository-project-operations-part-02.spec.ts | 6 +- .../repository-project-operations.spec.ts | 14 +- .../runtime/repo-icon-fork-backfill.test.ts | 59 ++++----- ...time-project-host-setup-controller.test.ts | 2 +- .../runtime-project-host-setup-controller.ts | 4 +- .../runtime-remove-project-host-scope.spec.ts | 7 +- .../runtime-repository-clone-controller.ts | 2 +- .../runtime-repository-fork-backfill.ts | 2 +- ...time-repository-registration-controller.ts | 2 +- .../runtime-repository-settings-controller.ts | 13 +- .../runtime-rpc-worktree-store-fixtures.ts | 2 +- .../terminal-list-payload-size.test.ts | 2 +- ...rcad-migration-cutover-coordinator.test.ts | 4 +- .../ssh/orcad-migration-delta-move.test.ts | 2 +- .../orcad-migration-repository-link.test.ts | 2 +- src/shared/repo-host-identity.ts | 5 + ...-upgrade-identity-persistence.unit.test.ts | 6 +- 46 files changed, 380 insertions(+), 294 deletions(-) create mode 100644 src/main/persistence/repo-host-target-matcher.test-fixture.ts diff --git a/src/main/automations/automation-worker-durability.test.ts b/src/main/automations/automation-worker-durability.test.ts index 1cd77cd8518..1bf497a71a4 100644 --- a/src/main/automations/automation-worker-durability.test.ts +++ b/src/main/automations/automation-worker-durability.test.ts @@ -210,7 +210,7 @@ describe('automation background writer durability', () => { try { const pending = service.runNow(automation.id) await acknowledgement.blocked - store.updateRepo('repo-local', { executionHostId: 'runtime:other-host' }) + store.updateRepo({ id: 'repo-local' }, { executionHostId: 'runtime:other-host' }) acknowledgement.release() await expect(pending).resolves.toMatchObject({ status: 'skipped_unavailable' }) expect(dispatcher).not.toHaveBeenCalled() diff --git a/src/main/folder-repo-git-conversion.ts b/src/main/folder-repo-git-conversion.ts index b23a0ad4bd8..ac464f4413f 100644 --- a/src/main/folder-repo-git-conversion.ts +++ b/src/main/folder-repo-git-conversion.ts @@ -141,7 +141,7 @@ export async function updateProjectHostSetupWithProvenKind( if (!result?.repo || !proven.folderUpgradeGitRootPath) { return result } - const stamped = store.updateRepo(result.repo.id, proven, getRepoExecutionHostId(result.repo)) + const stamped = store.updateRepo(result.repo, proven) return stamped ? { ...result, repo: stamped } : result }, allowRuntimeHost diff --git a/src/main/ipc/folder-repo-git-upgrade-lifetime.test.ts b/src/main/ipc/folder-repo-git-upgrade-lifetime.test.ts index bf3cb64e258..c0fc97c5c9b 100644 --- a/src/main/ipc/folder-repo-git-upgrade-lifetime.test.ts +++ b/src/main/ipc/folder-repo-git-upgrade-lifetime.test.ts @@ -60,7 +60,7 @@ function begin(repos = [folder()], meta: ReturnType getRepos: () => repos, getRepo: (id: string) => repos.find((repo) => repo.id === id), getAllWorktreeMeta: () => meta, - updateRepo: vi.fn((id: string, patch: Partial) => { + updateRepo: vi.fn(({ id }: Pick, patch: Partial) => { const repo = repos.find((row) => row.id === id) return repo ? Object.assign(repo, patch) : null }) diff --git a/src/main/ipc/folder-repo-git-upgrade.test.ts b/src/main/ipc/folder-repo-git-upgrade.test.ts index ebc58b92b72..7f8e8607d67 100644 --- a/src/main/ipc/folder-repo-git-upgrade.test.ts +++ b/src/main/ipc/folder-repo-git-upgrade.test.ts @@ -82,6 +82,7 @@ import { stopFolderRepoGitUpgradeWatch } from './folder-repo-git-upgrade' import { wakeFolderRepoGitUpgradeWatch } from './folder-repo-git-upgrade-wake' +import { repoTarget } from '../persistence/repo-host-target-matcher.test-fixture' type TestWindow = { destroyed: boolean @@ -128,7 +129,7 @@ function makeStore( getAllWorktreeMeta: () => worktreeMeta, getRepos: () => repos, getRepo: vi.fn((id: string) => repos.find((repo) => repo.id === id)), - updateRepo: vi.fn((id: string, updates: Partial) => { + updateRepo: vi.fn(({ id }: Pick, updates: Partial) => { const repo = repos.find((candidate) => candidate.id === id) if (!repo) { return null @@ -199,7 +200,7 @@ describe('folder repo git upgrade watch', () => { gitInit(repoPath) await tick() - expect(store.updateRepo).toHaveBeenCalledWith('folder-repo', { + expect(store.updateRepo).toHaveBeenCalledWith(repoTarget('folder-repo'), { kind: 'git', folderUpgradeGitRootPath: repoPath.replaceAll('\\', '/'), externalWorktreeVisibility: 'hide' @@ -226,7 +227,7 @@ describe('folder repo git upgrade watch', () => { // A fixed tick wait can finish before the marker stat resolves under load. await vi.waitFor( () => { - expect(store.updateRepo).toHaveBeenCalledWith('folder-repo', { + expect(store.updateRepo).toHaveBeenCalledWith(repoTarget('folder-repo'), { kind: 'git', folderUpgradeGitRootPath: join(root, 'symlinked-project').replaceAll('\\', '/') }) @@ -257,7 +258,7 @@ describe('folder repo git upgrade watch', () => { await tick(2) expect(store.updateRepo).toHaveBeenCalledWith( - 'folder-repo', + repoTarget('folder-repo'), expect.objectContaining({ kind: 'git' }) ) }) @@ -316,7 +317,7 @@ describe('folder repo git upgrade watch', () => { await tick(2) expect(store.updateRepo).toHaveBeenCalledWith( - 'folder-repo', + repoTarget('folder-repo'), expect.objectContaining({ kind: 'git' }) ) }) @@ -349,7 +350,7 @@ describe('folder repo git upgrade watch', () => { await tick(2) expect(store.updateRepo).toHaveBeenCalledWith( - 'folder-repo', + repoTarget('folder-repo'), expect.objectContaining({ kind: 'git' }) ) expect(notifyReposChanged).toHaveBeenCalledWith(nextWindow) @@ -374,7 +375,7 @@ describe('folder repo git upgrade watch', () => { expect(store.updateRepo).toHaveBeenCalledTimes(1) expect(store.updateRepo).toHaveBeenCalledWith( - 'repo-a', + repoTarget('repo-a'), expect.objectContaining({ kind: 'git' }) ) }) @@ -434,7 +435,7 @@ describe('folder repo git upgrade watch', () => { await tick(2) expect(store.updateRepo).toHaveBeenCalledWith( - 'late-repo', + repoTarget('late-repo'), expect.objectContaining({ kind: 'git' }) ) }) @@ -463,7 +464,7 @@ describe('folder repo git upgrade watch', () => { await tick() expect(store.updateRepo).toHaveBeenCalledWith( - 'folder-repo', + repoTarget('folder-repo'), expect.objectContaining({ kind: 'git' }) diff --git a/src/main/ipc/folder-repo-git-upgrade.ts b/src/main/ipc/folder-repo-git-upgrade.ts index a91bb5afdc3..68cdb213569 100644 --- a/src/main/ipc/folder-repo-git-upgrade.ts +++ b/src/main/ipc/folder-repo-git-upgrade.ts @@ -117,7 +117,7 @@ async function upgradeFolderRepo(watch: UpgradeWatch, repoId: string): Promise { setupMethod: 'imported-existing-folder' }) - expect(mockStore.updateRepo).toHaveBeenNthCalledWith(1, existing.id, { + expect(mockStore.updateRepo).toHaveBeenNthCalledWith(1, repoTarget(existing.id), { upstream: { owner: 'acme', repo: 'orca', @@ -275,7 +276,7 @@ describe('repos:add + repos:clone', () => { const added: Record[] = [] mockStore.getRepos.mockImplementation(() => added) mockStore.addRepo.mockImplementation((repo: Record) => added.push(repo)) - mockStore.updateRepo.mockImplementation((id, updates) => { + mockStore.updateRepo.mockImplementation(({ id }: { id: string }, updates) => { const repo = added.find((entry) => entry.id === id) if (!repo) { return null @@ -363,6 +364,7 @@ describe('repos:add + repos:clone', () => { badgeColor: '#22c55e', worktreeBasePath: '../worktrees' } + mockStore.getRepo.mockReturnValue(updated) mockStore.updateRepo.mockReturnValue(updated) const result = await handlers.get('repos:update')!(null, { @@ -371,7 +373,7 @@ describe('repos:add + repos:clone', () => { }) expect(result).toBe(updated) - expect(mockStore.updateRepo).toHaveBeenCalledWith(updated.id, { + expect(mockStore.updateRepo).toHaveBeenCalledWith(repoTarget(updated.id), { worktreeBasePath: '../worktrees' }) expect(mockStore.getRepos).not.toHaveBeenCalled() @@ -388,6 +390,7 @@ describe('repos:add + repos:clone', () => { badgeColor: '#22c55e', agentWorktreeVisibility: 'show' } + mockStore.getRepo.mockReturnValue(updated) mockStore.updateRepo.mockReturnValue(updated) const result = await handlers.get('repos:update')!(null, { @@ -396,7 +399,7 @@ describe('repos:add + repos:clone', () => { }) expect(result).toBe(updated) - expect(mockStore.updateRepo).toHaveBeenCalledWith(updated.id, { + expect(mockStore.updateRepo).toHaveBeenCalledWith(repoTarget(updated.id), { agentWorktreeVisibility: 'show' }) }) @@ -409,6 +412,7 @@ describe('repos:add + repos:clone', () => { kind: 'git', badgeColor: '#22c55e' } + mockStore.getRepo.mockReturnValue(updated) mockStore.updateRepo.mockReturnValue(updated) await handlers.get('repos:update')!(null, { @@ -425,7 +429,7 @@ describe('repos:add + repos:clone', () => { } }) - expect(mockStore.updateRepo).toHaveBeenCalledWith(updated.id, { + expect(mockStore.updateRepo).toHaveBeenCalledWith(repoTarget(updated.id), { customWorktreeVisibilitySources: [{ id: 'team', rootPath: '/srv/team' }], worktreeVisibilitySourcePreferences: { builtIn: { claude: 'show' }, diff --git a/src/main/ipc/repos-local-clone-lifecycle.test.ts b/src/main/ipc/repos-local-clone-lifecycle.test.ts index b60bff50181..b279049a904 100644 --- a/src/main/ipc/repos-local-clone-lifecycle.test.ts +++ b/src/main/ipc/repos-local-clone-lifecycle.test.ts @@ -38,6 +38,7 @@ import { resetLocalRepoMocks, waitForAssertion } from './repos-remote-test-harness' +import { repoTarget } from '../persistence/repo-host-target-matcher.test-fixture' const { handleMock, @@ -197,7 +198,7 @@ describe('repos:add + repos:clone', () => { expect(mockStore.updateRepo).not.toHaveBeenCalled() expect(result).toEqual(current) } else { - expect(mockStore.updateRepo).toHaveBeenCalledWith(current.id, { + expect(mockStore.updateRepo).toHaveBeenCalledWith(repoTarget(current.id), { kind: 'git', folderUpgradeGitRootPath: clonePath, projectHostSetupMethod: 'cloned' diff --git a/src/main/ipc/repos-remote-test-harness.ts b/src/main/ipc/repos-remote-test-harness.ts index 383c3300ee1..0ae09ccb998 100644 --- a/src/main/ipc/repos-remote-test-harness.ts +++ b/src/main/ipc/repos-remote-test-harness.ts @@ -30,7 +30,7 @@ export type ReposIpcMocks = { | 'getSshTargets', ReposIpcSpy > & { - updateRepo: Mock<(repoId: string, updates: Record) => unknown> + updateRepo: Mock<(target: { id: string }, updates: Record) => unknown> } gitSpawnMock: ReposIpcSpy gitSpawnAfterWindowsEnvironmentReadyMock: ReposIpcSpy diff --git a/src/main/ipc/repos/project-host-setup-handlers.ts b/src/main/ipc/repos/project-host-setup-handlers.ts index efc995cfd8f..7ba1bb0cf09 100644 --- a/src/main/ipc/repos/project-host-setup-handlers.ts +++ b/src/main/ipc/repos/project-host-setup-handlers.ts @@ -64,7 +64,7 @@ function alignRepoWithRequestedProject( throw new Error('Imported folder does not match the selected project identity.') } // Why: stamp the selected project's provider identity when the folder lacks upstream, so projection can merge it. - const updated = store.updateRepo(repo.id, { + const updated = store.updateRepo(repo, { upstream: { owner: identity.owner, repo: identity.repo, @@ -77,7 +77,7 @@ function alignRepoWithRequestedProject( repo = updated setup = getProjectHostSetupForRepo(store.getProjectHostSetups(), repo) } - const updated = store.updateRepo(repo.id, { projectHostSetupMethod: setupMethod }) + const updated = store.updateRepo(repo, { projectHostSetupMethod: setupMethod }) if (!updated) { throw new Error( `Project setup repo disappeared before setup metadata could be linked: ${repo.id}` diff --git a/src/main/ipc/repos/repo-clone-lifecycle.ts b/src/main/ipc/repos/repo-clone-lifecycle.ts index b70f5a0fc0f..66d83eb0097 100644 --- a/src/main/ipc/repos/repo-clone-lifecycle.ts +++ b/src/main/ipc/repos/repo-clone-lifecycle.ts @@ -264,7 +264,7 @@ export function registerRepoCloneHandlers(mainWindow: BrowserWindow, store: Stor isFolderRepo(current) && getRepoExecutionHostId(current) === getRepoExecutionHostId(existing) ) { - const updated = store.updateRepo(current.id, { + const updated = store.updateRepo(current, { kind: 'git', ...conversion, projectHostSetupMethod: 'cloned' diff --git a/src/main/ipc/repos/repo-update-handler.ts b/src/main/ipc/repos/repo-update-handler.ts index 53b5fe0ff60..ba95477f644 100644 --- a/src/main/ipc/repos/repo-update-handler.ts +++ b/src/main/ipc/repos/repo-update-handler.ts @@ -2,6 +2,7 @@ import type { BrowserWindow } from 'electron' import { ipcMain } from 'electron' import type { Store } from '../../persistence' import type { Repo } from '../../../shared/repo-types' +import type { RepoHostTarget } from '../../../shared/repo-host-identity' import { isFolderRepo } from '../../../shared/repo-kind' import type { ExecutionHostId } from '../../../shared/execution-host' import { getRepoExecutionHostId, normalizeExecutionHostId } from '../../../shared/execution-host' @@ -227,26 +228,27 @@ export function registerRepoUpdateHandler(mainWindow: BrowserWindow, store: Stor if (args.hostId && !hostId) { return null } + // Why: some renderer callers send only the id; they keep the first row with it, as before. + const target: RepoHostTarget | undefined = hostId + ? { id: args.repoId, executionHostId: hostId } + : store.getRepo(args.repoId) + if (!target) { + return null + } const write = (proven: Pick): Repo | null => { if (isWslMoveSourceHost(args.hostId)) { assertWslMoveProjectWritable(store, { repoId: args.repoId }) } - return hostId - ? store.updateRepo(args.repoId, { ...updates, ...proven }, hostId) - : store.updateRepo(args.repoId, { ...updates, ...proven }) + return store.updateRepo(target, { ...updates, ...proven }) } let updated: Repo | null if (updates.kind === 'git') { - const target = store + const targetHostId = getRepoExecutionHostId(target) + const row = store .getRepos() - .find( - (repo) => - repo.id === args.repoId && (!hostId || getRepoExecutionHostId(repo) === hostId) - ) + .find((repo) => repo.id === target.id && getRepoExecutionHostId(repo) === targetHostId) updated = - target && isFolderRepo(target) - ? await commitProvenRepoKindChange(store, target, write) - : write({}) + row && isFolderRepo(row) ? await commitProvenRepoKindChange(store, row, write) : write({}) } else { updated = write({}) } diff --git a/src/main/managed-server/lifecycle/orcad-runtime-conversion.test.ts b/src/main/managed-server/lifecycle/orcad-runtime-conversion.test.ts index cc8ea55398e..d280bc0cfaa 100644 --- a/src/main/managed-server/lifecycle/orcad-runtime-conversion.test.ts +++ b/src/main/managed-server/lifecycle/orcad-runtime-conversion.test.ts @@ -245,7 +245,7 @@ describe('the source frozen during a conversion', () => { it('still refuses a catalog change mid-conversion', async () => { const stage = destination.stage.getMockImplementation()! destination.stage.mockImplementationOnce(async (manifest) => { - store.updateRepo('repo-1', { displayName: 'Renamed' }) + store.updateRepo(store.getRepo('repo-1')!, { displayName: 'Renamed' }) return stage(manifest) }) await expect(convert()).rejects.toThrow('orcad_migration_source_changed') diff --git a/src/main/persistence-initial-load.test.ts b/src/main/persistence-initial-load.test.ts index ee663d302a2..f5c62bc3f43 100644 --- a/src/main/persistence-initial-load.test.ts +++ b/src/main/persistence-initial-load.test.ts @@ -391,13 +391,16 @@ describe('Store', () => { const store = await createStore() // A re-probe that now prefers the `upstream` remote rewrites the derived project id. - store.updateRepo('r1', { - gitRemoteIdentity: { - canonicalKey: 'git.example.com/acme/app-upstream', - remoteName: 'upstream', - remoteUrl: 'git@git.example.com:acme/app-upstream.git' + store.updateRepo( + { id: 'r1' }, + { + gitRemoteIdentity: { + canonicalKey: 'git.example.com/acme/app-upstream', + remoteName: 'upstream', + remoteUrl: 'git@git.example.com:acme/app-upstream.git' + } } - }) + ) const upstreamProjectId = 'git:git.example.com/acme/app-upstream' expect(store.getProjects().map((project) => project.id)).toEqual([upstreamProjectId]) diff --git a/src/main/persistence-jira-project.test.ts b/src/main/persistence-jira-project.test.ts index 4346e73c06a..659cd4e0eb7 100644 --- a/src/main/persistence-jira-project.test.ts +++ b/src/main/persistence-jira-project.test.ts @@ -18,13 +18,13 @@ describe('persisted repository Jira defaults', () => { it('persists and clears the project default across restart without requiring Git', () => { const store = createStore() store.addRepo(makeRepo({ kind: 'folder' })) - expect(store.updateRepo('r1', { defaultJiraProject: project })?.defaultJiraProject).toEqual( - project - ) + expect( + store.updateRepo({ id: 'r1' }, { defaultJiraProject: project })?.defaultJiraProject + ).toEqual(project) store.flush() const reloaded = createStore() expect(reloaded.getRepo('r1')?.defaultJiraProject).toEqual(project) - reloaded.updateRepo('r1', { defaultJiraProject: null }) + reloaded.updateRepo({ id: 'r1' }, { defaultJiraProject: null }) reloaded.flush() expect(createStore().getRepo('r1')?.defaultJiraProject).toBeNull() }) diff --git a/src/main/persistence-orcad-repository-cache-reimport.test.ts b/src/main/persistence-orcad-repository-cache-reimport.test.ts index c37d988e432..5525b431ffb 100644 --- a/src/main/persistence-orcad-repository-cache-reimport.test.ts +++ b/src/main/persistence-orcad-repository-cache-reimport.test.ts @@ -34,7 +34,7 @@ describe('reimporting a repository after the destination resolves its Git identi async (identity) => { const store = createStore() commit(store, manifest()) - store.updateRepo(REPOSITORY.id, { gitRemoteIdentity: identity }) + store.updateRepo(store.getRepos()[0]!, { gitRemoteIdentity: identity }) await store.flushPendingOrThrowAsync() await closeTestStores() const restored = createStore() @@ -50,7 +50,7 @@ describe('reimporting a repository after the destination resolves its Git identi it('preserves the current cache when the incoming cache is stale', () => { const store = createStore() commit(store, manifest()) - store.updateRepo(REPOSITORY.id, { gitRemoteIdentity: remoteIdentity }) + store.updateRepo(store.getRepos()[0]!, { gitRemoteIdentity: remoteIdentity }) const next = manifest({ migrationId: 'migration-stale-cache', payload: { ...manifest().payload, repositories: [{ ...REPOSITORY, gitRemoteIdentity: null }] } @@ -64,7 +64,7 @@ describe('reimporting a repository after the destination resolves its Git identi (changes) => { const store = createStore() commit(store, manifest()) - store.updateRepo(REPOSITORY.id, { gitRemoteIdentity: remoteIdentity }) + store.updateRepo(store.getRepos()[0]!, { gitRemoteIdentity: remoteIdentity }) const next = manifest({ migrationId: 'migration-conflict', payload: { ...manifest().payload, repositories: [{ ...REPOSITORY, ...changes }] } @@ -91,7 +91,7 @@ describe('reimporting a repository after the destination resolves its Git identi manifest({ migrationId, payload: { ...manifest().payload, repositories: [source] } }) const store = createStore() commit(store, withSource('m1')) - store.updateRepo(REPOSITORY.id, { gitRemoteIdentity: remoteIdentity, repoIcon: newIcon }) + store.updateRepo(store.getRepos()[0]!, { gitRemoteIdentity: remoteIdentity, repoIcon: newIcon }) expect(commit(store, withSource('m2'))).toMatchObject({ state: 'committed' }) expect(store.getRepos()[0]).toMatchObject({ @@ -103,7 +103,7 @@ describe('reimporting a repository after the destination resolves its Git identi it('still refuses a changed icon the user chose', () => { const store = createStore() commit(store, manifest()) - store.updateRepo(REPOSITORY.id, { + store.updateRepo(store.getRepos()[0]!, { repoIcon: githubAvatarIcon({ owner: 'example', repo: 'project' }) }) const next = manifest({ diff --git a/src/main/persistence-repo-lifecycle.test.ts b/src/main/persistence-repo-lifecycle.test.ts index 78b6ebee23d..e9e58d05343 100644 --- a/src/main/persistence-repo-lifecycle.test.ts +++ b/src/main/persistence-repo-lifecycle.test.ts @@ -255,10 +255,13 @@ describe('Store', () => { const group = store.createProjectGroup({ name: 'Platform', createdFrom: 'manual' }) store.addRepo(makeRepo({ id: 'r1', projectGroupId: group.id, projectGroupOrder: 1 })) - const updated = store.updateRepo('r1', { - projectGroupId: '', - projectGroupOrder: Number.POSITIVE_INFINITY - } as never) + const updated = store.updateRepo( + { id: 'r1' }, + { + projectGroupId: '', + projectGroupOrder: Number.POSITIVE_INFINITY + } + ) expect(updated?.projectGroupId).toBeNull() expect(updated?.projectGroupOrder).toBe(1) @@ -268,7 +271,7 @@ describe('Store', () => { const store = await createStore() store.addRepo(makeRepo({ id: 'r1' })) - const updated = store.updateRepo('r1', { executionHostId: 'runtime:env-1' }) + const updated = store.updateRepo({ id: 'r1' }, { executionHostId: 'runtime:env-1' }) expect(updated?.executionHostId).toBe('runtime:env-1') expect(store.getRepo('r1')?.executionHostId).toBe('runtime:env-1') diff --git a/src/main/persistence-update-repo.test.ts b/src/main/persistence-update-repo.test.ts index fe8096bcb91..89ea94e9598 100644 --- a/src/main/persistence-update-repo.test.ts +++ b/src/main/persistence-update-repo.test.ts @@ -82,7 +82,7 @@ describe('Store', () => { const store = await createStore() store.addRepo(makeRepo()) - const updated = store.updateRepo('r1', { displayName: 'renamed' }) + const updated = store.updateRepo({ id: 'r1' }, { displayName: 'renamed' }) expect(updated).not.toBeNull() expect(updated!.displayName).toBe('renamed') expect(store.getRepo('r1')!.displayName).toBe('renamed') @@ -95,14 +95,14 @@ describe('Store', () => { const initialGeneration = getLocalWorktreeScanGeneration('scan-round-trip') expect(isLocalWorktreeScanGenerationCurrent('scan-round-trip', initialGeneration)).toBe(true) - const folder = store.updateRepo('scan-round-trip', { kind: 'folder' }) + const folder = store.updateRepo({ id: 'scan-round-trip' }, { kind: 'folder' }) expect(folder?.kind).toBe('folder') expect(isLocalWorktreeScanGenerationCurrent('scan-round-trip', initialGeneration)).toBe(false) const folderGeneration = getLocalWorktreeScanGeneration('scan-round-trip') expect(isLocalWorktreeScanGenerationCurrent('scan-round-trip', folderGeneration)).toBe(true) - const git = store.updateRepo('scan-round-trip', { kind: 'git' }) + const git = store.updateRepo({ id: 'scan-round-trip' }, { kind: 'git' }) expect(git?.kind).toBe('git') expect(isLocalWorktreeScanGenerationCurrent('scan-round-trip', folderGeneration)).toBe(false) }) @@ -119,7 +119,10 @@ describe('Store', () => { }) ) - const updated = store.updateRepo('shared', { displayName: 'Remote renamed' }, 'ssh:server') + const updated = store.updateRepo( + { id: 'shared', executionHostId: 'ssh:server' }, + { displayName: 'Remote renamed' } + ) expect(updated?.path).toBe('/remote/repo') expect(store.getRepos().filter((repo) => repo.id === 'shared')).toMatchObject([ @@ -128,15 +131,30 @@ describe('Store', () => { ]) }) + it('updateRepo writes the row it was handed, not the first row with that id', async () => { + const store = await createStore() + store.addRepo(makeRepo({ id: 'shared', path: '/remote/repo', connectionId: 'server' })) + store.addRepo(makeRepo({ id: 'shared', path: '/local/repo' })) + const local = store.getRepos().find((repo) => repo.path === '/local/repo')! + + const updated = store.updateRepo(local, { displayName: 'Local renamed' }) + + expect(updated?.path).toBe('/local/repo') + expect(store.getRepos().map((repo) => repo.displayName)).toEqual(['test', 'Local renamed']) + }) + it('updateRepo keeps project host setup compatibility records in sync', async () => { const store = await createStore() store.addRepo(makeRepo({ worktreeBasePath: '../worktrees' })) - store.updateRepo('r1', { - displayName: 'renamed', - worktreeBasePath: '../new-worktrees', - upstream: { owner: 'stablyai', repo: 'orca' } - }) + store.updateRepo( + { id: 'r1' }, + { + displayName: 'renamed', + worktreeBasePath: '../new-worktrees', + upstream: { owner: 'stablyai', repo: 'orca' } + } + ) expect(store.getProjects()).toEqual([ expect.objectContaining({ @@ -176,7 +194,7 @@ describe('Store', () => { }) const store = await createStore() - store.updateRepo('r1', { displayName: 'renamed' }) + store.updateRepo({ id: 'r1' }, { displayName: 'renamed' }) store.reorderRepos(['r1']) expect(store.getProjects().map((project) => project.id)).toEqual(['repo:r1', 'cloud-project']) @@ -459,7 +477,7 @@ describe('Store', () => { const store = await createStore() store.addRepo(makeRepo()) - store.updateRepo('r1', { projectHostSetupMethod: 'cloned' }) + store.updateRepo({ id: 'r1' }, { projectHostSetupMethod: 'cloned' }) expect(store.getRepo('r1')?.projectHostSetupMethod).toBe('cloned') expect(store.getProjectHostSetups()).toEqual([ @@ -474,13 +492,16 @@ describe('Store', () => { const store = await createStore() store.addRepo(makeRepo()) - const updated = store.updateRepo('r1', { - repoIcon: { - type: 'image', - source: 'upload', - src: 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4=' - } as never - }) + const updated = store.updateRepo( + { id: 'r1' }, + { + repoIcon: { + type: 'image', + source: 'upload', + src: 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4=' + } as never + } + ) expect(updated).not.toBeNull() expect(updated!.repoIcon).toBeUndefined() @@ -491,7 +512,7 @@ describe('Store', () => { const store = await createStore() store.addRepo(makeRepo()) - const updated = store.updateRepo('r1', { badgeColor: ' ABCDEF ' }) + const updated = store.updateRepo({ id: 'r1' }, { badgeColor: ' ABCDEF ' }) expect(updated!.badgeColor).toBe('#abcdef') expect(store.getRepo('r1')!.badgeColor).toBe('#abcdef') @@ -501,7 +522,7 @@ describe('Store', () => { const store = await createStore() store.addRepo(makeRepo({ badgeColor: '#123456' })) - const updated = store.updateRepo('r1', { badgeColor: 'blue' }) + const updated = store.updateRepo({ id: 'r1' }, { badgeColor: 'blue' }) expect(updated!.badgeColor).toBe('#123456') expect(store.getRepo('r1')!.badgeColor).toBe('#123456') @@ -527,12 +548,15 @@ describe('Store', () => { const store = await createStore() store.addRepo(makeRepo()) - const updated = store.updateRepo('r1', { - upstream: { owner: ' stablyai ', repo: ' orca ' } - }) + const updated = store.updateRepo( + { id: 'r1' }, + { + upstream: { owner: ' stablyai ', repo: ' orca ' } + } + ) expect(updated!.upstream).toEqual({ owner: 'stablyai', repo: 'orca' }) - store.updateRepo('r1', { upstream: null }) + store.updateRepo({ id: 'r1' }, { upstream: null }) store.flush() const reloaded = await createStore() expect(reloaded.getRepo('r1')!.upstream).toBeNull() @@ -542,20 +566,23 @@ describe('Store', () => { const store = await createStore() store.addRepo(makeRepo()) - const updated = store.updateRepo('r1', { - gitRemoteIdentity: { - canonicalKey: 'gitlab.example.com/team/orca', - remoteName: 'origin', - remoteUrl: 'git@gitlab.example.com:team/orca.git' + const updated = store.updateRepo( + { id: 'r1' }, + { + gitRemoteIdentity: { + canonicalKey: 'gitlab.example.com/team/orca', + remoteName: 'origin', + remoteUrl: 'git@gitlab.example.com:team/orca.git' + } } - }) + ) expect(updated!.gitRemoteIdentity).toEqual({ canonicalKey: 'gitlab.example.com/team/orca', remoteName: 'origin', remoteUrl: 'git@gitlab.example.com:team/orca.git' }) - store.updateRepo('r1', { gitRemoteIdentity: null }) + store.updateRepo({ id: 'r1' }, { gitRemoteIdentity: null }) store.flush() const reloaded = await createStore() expect(reloaded.getRepo('r1')!.gitRemoteIdentity).toBeNull() @@ -573,9 +600,12 @@ describe('Store', () => { const store = await createStore() store.addRepo(makeRepo()) - const updated = store.updateRepo('r1', { - upstream: { owner: ' acme ', repo: ' widgets ', host: ' GHE.example:8443 ' } - }) + const updated = store.updateRepo( + { id: 'r1' }, + { + upstream: { owner: ' acme ', repo: ' widgets ', host: ' GHE.example:8443 ' } + } + ) expect(updated!.upstream).toEqual({ owner: 'acme', repo: 'widgets', @@ -608,14 +638,14 @@ describe('Store', () => { it('updateRepo returns null for nonexistent id', async () => { const store = await createStore() - expect(store.updateRepo('nope', { displayName: 'x' })).toBeNull() + expect(store.updateRepo({ id: 'nope' }, { displayName: 'x' })).toBeNull() }) it('updateRepo persists issueSourcePreference across reloads', async () => { const store = await createStore() store.addRepo(makeRepo()) - const updated = store.updateRepo('r1', { issueSourcePreference: 'upstream' }) + const updated = store.updateRepo({ id: 'r1' }, { issueSourcePreference: 'upstream' }) expect(updated!.issueSourcePreference).toBe('upstream') store.flush() @@ -627,16 +657,19 @@ describe('Store', () => { const store = await createStore() store.addRepo(makeRepo()) - const updated = store.updateRepo('r1', { - ghAccount: { host: ' GitHub.COM ', user: ' Alice ' } - }) + const updated = store.updateRepo( + { id: 'r1' }, + { + ghAccount: { host: ' GitHub.COM ', user: ' Alice ' } + } + ) expect(updated!.ghAccount).toEqual({ host: 'github.com', user: 'Alice' }) store.flush() const reloaded = await createStore() expect(reloaded.getRepo('r1')!.ghAccount).toEqual({ host: 'github.com', user: 'Alice' }) - const cleared = reloaded.updateRepo('r1', { ghAccount: null }) + const cleared = reloaded.updateRepo({ id: 'r1' }, { ghAccount: null }) expect(cleared!.ghAccount).toBeUndefined() }) @@ -644,7 +677,7 @@ describe('Store', () => { const store = await createStore() store.addRepo(makeRepo()) - const updated = store.updateRepo('r1', { forkSyncMode: 'safe-auto' }) + const updated = store.updateRepo({ id: 'r1' }, { forkSyncMode: 'safe-auto' }) expect(updated!.forkSyncMode).toBe('safe-auto') store.flush() @@ -656,7 +689,8 @@ describe('Store', () => { const store = await createStore() store.addRepo(makeRepo({ forkSyncMode: 'ask' })) - const updated = store.updateRepo('r1', { forkSyncMode: 'always' as never }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: feeds the sanitizer a value its type forbids, on purpose. + const updated = store.updateRepo({ id: 'r1' }, { forkSyncMode: 'always' as never }) expect(updated!.forkSyncMode).toBe('ask') @@ -683,7 +717,7 @@ describe('Store', () => { expect(store.getRepo('r1')!.issueSourcePreference).toBe('origin') // Why: `Object.assign` skips undefined, so updateRepo needs an explicit delete branch or the key with value undefined wouldn't clear. - store.updateRepo('r1', { issueSourcePreference: undefined }) + store.updateRepo({ id: 'r1' }, { issueSourcePreference: undefined }) expect(store.getRepo('r1')!.issueSourcePreference).toBeUndefined() store.flush() diff --git a/src/main/persistence-worktree-visibility.test.ts b/src/main/persistence-worktree-visibility.test.ts index 600a76effb2..3fc49ab1336 100644 --- a/src/main/persistence-worktree-visibility.test.ts +++ b/src/main/persistence-worktree-visibility.test.ts @@ -75,7 +75,7 @@ describe('Store', () => { }) ) - const updated = store.updateRepo('r1', { externalWorktreeVisibility: 'hide' }) + const updated = store.updateRepo({ id: 'r1' }, { externalWorktreeVisibility: 'hide' }) expect(updated!.externalWorktreeVisibility).toBe('hide') expect(updated!.externalWorktreeVisibilityLegacy).toBe(true) @@ -172,7 +172,7 @@ describe('Store', () => { const store = await createStore() store.addRepo(makeRepo({ externalWorktreeVisibility: 'hide' })) - const updated = store.updateRepo('r1', { agentWorktreeVisibility: 'show' }) + const updated = store.updateRepo({ id: 'r1' }, { agentWorktreeVisibility: 'show' }) expect(updated).toMatchObject({ externalWorktreeVisibility: 'hide', @@ -197,16 +197,19 @@ describe('Store', () => { const store = await createStore() store.addRepo(makeRepo()) - const updated = store.updateRepo('r1', { - customWorktreeVisibilitySources: [ - { id: 'team', rootPath: ' /srv/team-worktrees ' }, - { id: 'invalid', rootPath: '../relative' } - ], - worktreeVisibilitySourcePreferences: { - builtIn: { claude: 'show', gsd: 'hide' }, - custom: { team: 'show' } + const updated = store.updateRepo( + { id: 'r1' }, + { + customWorktreeVisibilitySources: [ + { id: 'team', rootPath: ' /srv/team-worktrees ' }, + { id: 'invalid', rootPath: '../relative' } + ], + worktreeVisibilitySourcePreferences: { + builtIn: { claude: 'show', gsd: 'hide' }, + custom: { team: 'show' } + } } - }) + ) expect(updated).toMatchObject({ customWorktreeVisibilitySources: [{ id: 'team', rootPath: '/srv/team-worktrees' }], @@ -232,7 +235,7 @@ describe('Store', () => { }) ) - const updated = store.updateRepo('r1', { externalWorktreeVisibility: null }) + const updated = store.updateRepo({ id: 'r1' }, { externalWorktreeVisibility: null }) expect(updated?.externalWorktreeVisibility).toBeUndefined() expect(updated?.externalWorktreeVisibilityLegacy).toBe(false) @@ -281,10 +284,13 @@ describe('Store', () => { }) ) - store.updateRepo('r1', { - issueSourcePreference: undefined, - sourceControlAi: undefined - }) + store.updateRepo( + { id: 'r1' }, + { + issueSourcePreference: undefined, + sourceControlAi: undefined + } + ) expect(store.getRepo('r1')!.issueSourcePreference).toBeUndefined() expect(store.getRepo('r1')!.sourceControlAi).toBeUndefined() @@ -306,9 +312,12 @@ describe('Store', () => { }) ) - store.updateRepo('r1', { - sourceControlAi: null - }) + store.updateRepo( + { id: 'r1' }, + { + sourceControlAi: null + } + ) expect(store.getRepo('r1')!.sourceControlAi).toBeUndefined() @@ -321,29 +330,32 @@ describe('Store', () => { const store = await createStore() store.addRepo(makeRepo()) - const updated = store.updateRepo('r1', { - sourceControlAi: { - instructionsByOperation: { - commitMessage: 'Repo style', - pullRequest: 42, - unknown: 'ignored' - }, - prCreationDefaults: { - draft: true, - useTemplate: null, - openAfterCreate: 'yes' - }, - modelOverridesByOperation: { - commitMessage: { - selectedModelByAgent: { codex: 'gpt-5.4', claude: false }, - selectedThinkingByModel: { 'gpt-5.4': 'high', bad: true } + const updated = store.updateRepo( + { id: 'r1' }, + { + sourceControlAi: { + instructionsByOperation: { + commitMessage: 'Repo style', + pullRequest: 42, + unknown: 'ignored' }, - unknown: { - selectedModelByAgent: { codex: 'ignored' } + prCreationDefaults: { + draft: true, + useTemplate: null, + openAfterCreate: 'yes' + }, + modelOverridesByOperation: { + commitMessage: { + selectedModelByAgent: { codex: 'gpt-5.4', claude: false }, + selectedThinkingByModel: { 'gpt-5.4': 'high', bad: true } + }, + unknown: { + selectedModelByAgent: { codex: 'ignored' } + } } - } - } as never - }) + } as never + } + ) expect(updated!.sourceControlAi).toEqual({ instructionsByOperation: { @@ -377,7 +389,8 @@ describe('Store', () => { }) ) - const updated = store.updateRepo('r1', { sourceControlAi: 'bad' as never }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: feeds the sanitizer a value its type forbids, on purpose. + const updated = store.updateRepo({ id: 'r1' }, { sourceControlAi: 'bad' as never }) expect(updated!.sourceControlAi).toEqual({ instructionsByOperation: { commitMessage: 'Keep me' }, diff --git a/src/main/persistence/loading-store/repo-lifecycle-operations.ts b/src/main/persistence/loading-store/repo-lifecycle-operations.ts index 439b2349c66..29a697cda59 100644 --- a/src/main/persistence/loading-store/repo-lifecycle-operations.ts +++ b/src/main/persistence/loading-store/repo-lifecycle-operations.ts @@ -1,6 +1,7 @@ import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' import type { ProjectHostSetup, ProjectHostSetupUpdateArgs } from '../../../shared/project-types' import type { Repo } from '../../../shared/repo-types' +import type { RepoHostTarget } from '../../../shared/repo-host-identity' import type { GhAccountBinding } from '../../../shared/github/account-binding' import { removeRepoFromHostWorkspaceSessions, @@ -151,7 +152,7 @@ export class RepoLifecycleOperations { } updateRepo( - id: string, + target: RepoHostTarget, updates: Partial< Pick< Repo, @@ -185,10 +186,9 @@ export class RepoLifecycleOperations { sourceControlAi?: Repo['sourceControlAi'] | null externalWorktreeDiscoverySuppressedAt?: Repo['externalWorktreeDiscoverySuppressedAt'] | null ghAccount?: GhAccountBinding | null - }, - hostId?: ExecutionHostId + } ): Repo | null { - return getRepoUpdateOperations(this).updateRepo(id, updates, hostId) + return getRepoUpdateOperations(this).updateRepo(target, updates) } } @@ -274,7 +274,7 @@ export function getProjectHostSetupOperations( owner[repoLifecycleOperationsContext].runtime.projectHostSetupOperations ??= new ProjectHostSetupPersistenceOperations({ state: owner[repoLifecycleOperationsContext].runtime.state, - updateRepo: (id, updates, hostId) => owner.updateRepo(id, updates, hostId), + updateRepo: (target, updates) => owner.updateRepo(target, updates), scheduleSave: () => scheduleSave(owner[repoLifecycleOperationsContext].scheduling) }) return owner[repoLifecycleOperationsContext].runtime.projectHostSetupOperations diff --git a/src/main/persistence/repo-host-target-matcher.test-fixture.ts b/src/main/persistence/repo-host-target-matcher.test-fixture.ts new file mode 100644 index 00000000000..c182c45bf67 --- /dev/null +++ b/src/main/persistence/repo-host-target-matcher.test-fixture.ts @@ -0,0 +1,12 @@ +import { getRepoExecutionHostId, type ExecutionHostId } from '../../shared/execution-host' +import type { RepoHostTarget } from '../../shared/repo-host-identity' + +/** Matches an `updateRepo` target for `id` on `hostId`, however the target spells its host. */ +export function repoTarget(id: string, hostId: ExecutionHostId = 'local') { + return { + asymmetricMatch: (target: RepoHostTarget) => + target?.id === id && getRepoExecutionHostId(target) === hostId, + toString: () => `repoTarget(${id}, ${hostId})`, + toAsymmetricMatcher: () => `repoTarget(${id}, ${hostId})` + } +} diff --git a/src/main/persistence/tracking-repos/project-host-setup-update.ts b/src/main/persistence/tracking-repos/project-host-setup-update.ts index 05d291bd171..0da32d7eba6 100644 --- a/src/main/persistence/tracking-repos/project-host-setup-update.ts +++ b/src/main/persistence/tracking-repos/project-host-setup-update.ts @@ -56,7 +56,7 @@ export class ProjectHostSetupPersistenceOperations { repoUpdates.projectHostSetupMethod = updates.setupMethod } const updatedRepo = - Object.keys(repoUpdates).length > 0 ? this.updateRepo(repo.id, repoUpdates) : repo + Object.keys(repoUpdates).length > 0 ? this.updateRepo(repo, repoUpdates) : repo if (!updatedRepo) { return null } diff --git a/src/main/persistence/tracking-repos/repo-update-host-guard.test.ts b/src/main/persistence/tracking-repos/repo-update-host-guard.test.ts index a83f0cbdf76..1452ad1a881 100644 --- a/src/main/persistence/tracking-repos/repo-update-host-guard.test.ts +++ b/src/main/persistence/tracking-repos/repo-update-host-guard.test.ts @@ -42,7 +42,10 @@ describe('updateRepo host guard', () => { makeRepo({ id: 'repo-1', executionHostId: 'runtime:env-1' }) ]) - const result = operations.updateRepo('repo-1', { displayName: 'Renamed' }, 'local') + const result = operations.updateRepo( + { id: 'repo-1', executionHostId: 'local' }, + { displayName: 'Renamed' } + ) expect(result).toBeNull() expect(scheduleSave).not.toHaveBeenCalled() @@ -57,7 +60,12 @@ describe('updateRepo host guard', () => { makeRepo({ id: 'repo-1', executionHostId: 'runtime:env-1' }) ]) - expect(operations.updateRepo('repo-missing', { displayName: 'Renamed' }, 'local')).toBeNull() + expect( + operations.updateRepo( + { id: 'repo-missing', executionHostId: 'local' }, + { displayName: 'Renamed' } + ) + ).toBeNull() expect(errors).toEqual([]) }) @@ -66,7 +74,10 @@ describe('updateRepo host guard', () => { makeRepo({ id: 'repo-1', executionHostId: 'runtime:env-1' }) ]) - const result = operations.updateRepo('repo-1', { displayName: 'Renamed' }, 'runtime:env-1') + const result = operations.updateRepo( + { id: 'repo-1', executionHostId: 'runtime:env-1' }, + { displayName: 'Renamed' } + ) expect(result?.displayName).toBe('Renamed') expect(state.repos[0].displayName).toBe('Renamed') @@ -79,20 +90,34 @@ describe('updateRepo host guard', () => { const peer = makeRepo({ id: 'repo-1', displayName: 'Peer', executionHostId: 'ssh:peer' }) const { operations, state } = makeOperations([local, peer]) - const result = operations.updateRepo('repo-1', { displayName: 'Local edit' }, 'local') + const result = operations.updateRepo( + { id: 'repo-1', executionHostId: 'local' }, + { displayName: 'Local edit' } + ) expect(result?.displayName).toBe('Local edit') // The peer's row is untouched: a client-local write must never repair another host's metadata. expect(state.repos[1].displayName).toBe('Peer') }) - it('writes without a host argument exactly as before', () => { - const { operations, state } = makeOperations([ - makeRepo({ id: 'repo-1', executionHostId: 'runtime:env-1' }) - ]) + it('reads a target without host fields as the local row, never the first same-id row', () => { + const peer = makeRepo({ id: 'repo-1', displayName: 'Peer', connectionId: 'peer' }) + const local = makeRepo({ id: 'repo-1', displayName: 'Local' }) + const { operations, state } = makeOperations([peer, local]) - expect(operations.updateRepo('repo-1', { displayName: 'Renamed' })?.displayName).toBe('Renamed') - expect(state.repos[0].displayName).toBe('Renamed') + operations.updateRepo({ id: 'repo-1' }, { displayName: 'Renamed' }) + + expect(state.repos.map((repo) => repo.displayName)).toEqual(['Peer', 'Renamed']) + }) + + it('addresses an SSH row through its connectionId alone', () => { + const local = makeRepo({ id: 'repo-1', displayName: 'Local' }) + const peer = makeRepo({ id: 'repo-1', displayName: 'Peer', connectionId: 'peer' }) + const { operations, state } = makeOperations([local, peer]) + + operations.updateRepo(peer, { displayName: 'Peer edit' }) + + expect(state.repos.map((repo) => repo.displayName)).toEqual(['Local', 'Peer edit']) expect(errors).toEqual([]) }) }) diff --git a/src/main/persistence/tracking-repos/repo-update-operations.ts b/src/main/persistence/tracking-repos/repo-update-operations.ts index 8cffb741c0c..25d4eff9d70 100644 --- a/src/main/persistence/tracking-repos/repo-update-operations.ts +++ b/src/main/persistence/tracking-repos/repo-update-operations.ts @@ -1,6 +1,7 @@ import type { PersistedState } from '../../../shared/persisted-state-types' import type { Repo } from '../../../shared/repo-types' import type { ExecutionHostId } from '../../../shared/execution-host' +import type { RepoHostTarget } from '../../../shared/repo-host-identity' import { getRepoExecutionHostId } from '../../../shared/execution-host' import { isLegacyRepoForExternalWorktreeVisibility } from '../../../shared/external-worktree-visibility' import { normalizeRepoSourceControlAiOverrides } from '../../../shared/source-control-ai' @@ -30,11 +31,8 @@ export type RepoUpdateMutationOperations = { export function findRepoRowForHostScopedWrite( repos: readonly Repo[], id: string, - hostId: ExecutionHostId | undefined + hostId: ExecutionHostId ): Repo | undefined { - if (!hostId) { - return repos.find((candidate) => candidate.id === id) - } const matched = repos.find( (candidate) => candidate.id === id && getRepoExecutionHostId(candidate) === hostId ) @@ -97,7 +95,7 @@ export class RepoUpdatePersistenceOperations { } updateRepo( - id: string, + target: RepoHostTarget, updates: Partial< Pick< Repo, @@ -131,10 +129,13 @@ export class RepoUpdatePersistenceOperations { sourceControlAi?: Repo['sourceControlAi'] | null externalWorktreeDiscoverySuppressedAt?: Repo['externalWorktreeDiscoverySuppressedAt'] | null ghAccount?: GhAccountBinding | null - }, - hostId?: ExecutionHostId + } ): Repo | null { - const repo = findRepoRowForHostScopedWrite(this.state.repos, id, hostId) + const repo = findRepoRowForHostScopedWrite( + this.state.repos, + target.id, + getRepoExecutionHostId(target) + ) if (!repo) { return null } @@ -260,7 +261,7 @@ export class RepoUpdatePersistenceOperations { } } Object.assign(repo, sanitizedUpdates) - this.bumpLocalWorktreeScanGeneration(id) + this.bumpLocalWorktreeScanGeneration(repo.id) this.syncProjectHostSetupCompatibilityState() this.scheduleSave() return this.hydrateRepo(repo) diff --git a/src/main/repo-backfill-wsl-move.test.ts b/src/main/repo-backfill-wsl-move.test.ts index 4c18b5c3659..07711f5b7d5 100644 --- a/src/main/repo-backfill-wsl-move.test.ts +++ b/src/main/repo-backfill-wsl-move.test.ts @@ -2,8 +2,9 @@ import { mkdtempSync, rmSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { getRepoExecutionHostId, type ExecutionHostId } from '../shared/execution-host' +import { getRepoExecutionHostId } from '../shared/execution-host' import type { Repo } from '../shared/repo-types' +import type { RepoHostTarget } from '../shared/repo-host-identity' import { closeTestStores, createStore, testState } from './persistence-test-harness' import { wslMoveCutoverFixture as cutover } from './persistence/__fixtures__/wsl-mover/wsl-move-cutover-fixture' import { seedWslMoveCutover } from './persistence/__fixtures__/wsl-mover/wsl-move-journal-seeding' @@ -69,9 +70,11 @@ describe('git remote identity enrichment under a WSL move', () => { ] const store = { getRepos: () => rows, - updateRepo: (id: string, updates: Partial, hostId?: ExecutionHostId) => { + updateRepo: (target: RepoHostTarget, updates: Partial) => { const row = rows.find( - (candidate) => candidate.id === id && getRepoExecutionHostId(candidate) === hostId + (candidate) => + candidate.id === target.id && + getRepoExecutionHostId(candidate) === getRepoExecutionHostId(target) ) return row ? Object.assign(row, updates) : null } diff --git a/src/main/repo-git-remote-avatar-refresh.test.ts b/src/main/repo-git-remote-avatar-refresh.test.ts index a787f5501bf..7280ad0def0 100644 --- a/src/main/repo-git-remote-avatar-refresh.test.ts +++ b/src/main/repo-git-remote-avatar-refresh.test.ts @@ -4,6 +4,8 @@ import { deriveGitRemoteIdentity } from '../shared/git-remote-identity' import { projectHostSetupProjectionFromRepos } from '../shared/project-host-setup-projection' import { githubAvatarIcon, type RepoIcon } from '../shared/repo-icon' import type { Repo } from '../shared/repo-types' +import type { RepoHostTarget } from '../shared/repo-host-identity' +import { repoTarget } from './persistence/repo-host-target-matcher.test-fixture' import { probeGitRemoteIdentity, type GitRemoteIdentityProbe } from './repo-git-remote-identity' import { enrichMissingRepoGitRemoteIdentities, @@ -39,8 +41,8 @@ function repo(overrides: Partial = {}): Repo { function storeFor(repos: Repo[]) { const getRepo = (id: string, hostId?: ExecutionHostId) => repos.find((row) => row.id === id && (!hostId || getRepoExecutionHostId(row) === hostId)) - const updateRepo = vi.fn((id: string, updates: Partial, hostId?: ExecutionHostId) => { - const current = getRepo(id, hostId) + const updateRepo = vi.fn((target: RepoHostTarget, updates: Partial) => { + const current = getRepo(target.id, getRepoExecutionHostId(target)) if (!current) { return null } @@ -223,11 +225,9 @@ it('repairs only the matching owner when repo IDs and paths collide across hosts const store = storeFor([local, ssh]) vi.mocked(probeGitRemoteIdentity).mockResolvedValue({ status: 'resolved', identity: identity() }) await refresh(store) - expect(store.updateRepo).toHaveBeenCalledWith( - 'app', - { repoIcon: githubAvatarIcon({ owner: 'org-b', repo: 'app' }) }, - 'ssh:build' - ) + expect(store.updateRepo).toHaveBeenCalledWith(repoTarget('app', 'ssh:build'), { + repoIcon: githubAvatarIcon({ owner: 'org-b', repo: 'app' }) + }) expect(projectHostSetupProjectionFromRepos([local, ssh]).projects).toHaveLength(1) }) @@ -250,11 +250,10 @@ it('keeps local metadata writes scoped when a same-id ssh:build row comes first' expect(foreign.repoIcon).toBe(foreignIcon) expect(local.gitRemoteIdentity).toEqual(identity()) expect(local.repoIcon).toEqual(githubAvatarIcon({ owner: 'org-b', repo: 'app' })) - expect(store.updateRepo).toHaveBeenCalledExactlyOnceWith( - 'app', - { gitRemoteIdentity: identity(), repoIcon: local.repoIcon }, - 'local' - ) + expect(store.updateRepo).toHaveBeenCalledExactlyOnceWith(repoTarget('app', 'local'), { + gitRemoteIdentity: identity(), + repoIcon: local.repoIcon + }) }) // Why this row is repaired where the `ssh:build` one above is not: a `runtime:` stamp reaches this @@ -282,8 +281,8 @@ it('repairs a same-id runtime-addressed row under its own stamp', async () => { } expect(runtimeRow).toMatchObject(repaired) expect(local).toMatchObject(repaired) - expect(store.updateRepo).toHaveBeenCalledWith('app', repaired, 'runtime:env-a') - expect(store.updateRepo).toHaveBeenCalledWith('app', repaired, 'local') + expect(store.updateRepo).toHaveBeenCalledWith(repoTarget('app', 'runtime:env-a'), repaired) + expect(store.updateRepo).toHaveBeenCalledWith(repoTarget('app', 'local'), repaired) }) it('does not write after a pending local probe becomes peer-owned', async () => { @@ -328,7 +327,9 @@ it('does not overwrite a custom icon selected while the probe is pending', async answer({ status: 'resolved', identity: identity() }) await flushRepoGitRemoteIdentityEnrichmentForTests() expect(row.repoIcon).toBe(selected) - expect(store.updateRepo).toHaveBeenCalledWith('app', { gitRemoteIdentity: identity() }, 'local') + expect(store.updateRepo).toHaveBeenCalledWith(repoTarget('app', 'local'), { + gitRemoteIdentity: identity() + }) }) it('never probes folder workspaces for an avatar repair', async () => { diff --git a/src/main/repo-git-remote-identity-enrichment.test.ts b/src/main/repo-git-remote-identity-enrichment.test.ts index 54350dda3d4..a820b8bf817 100644 --- a/src/main/repo-git-remote-identity-enrichment.test.ts +++ b/src/main/repo-git-remote-identity-enrichment.test.ts @@ -1,7 +1,9 @@ import { afterEach, describe, expect, it, vi } from 'vitest' -import { getRepoExecutionHostId, type ExecutionHostId } from '../shared/execution-host' +import { getRepoExecutionHostId } from '../shared/execution-host' import type { GitRemoteIdentity } from '../shared/git-remote-identity' import type { Repo } from '../shared/repo-types' +import type { RepoHostTarget } from '../shared/repo-host-identity' +import { repoTarget } from './persistence/repo-host-target-matcher.test-fixture' import { type GitRemoteIdentityProbe, probeGitRemoteIdentity } from './repo-git-remote-identity' import { enrichMissingRepoGitRemoteIdentities, @@ -17,9 +19,8 @@ type RepoIdentityStore = { getRepos: () => Repo[] getRepo: (id: string) => Repo | undefined updateRepo: ( - id: string, - updates: Pick, 'gitRemoteIdentity'>, - hostId?: ExecutionHostId + target: RepoHostTarget, + updates: Pick, 'gitRemoteIdentity'> ) => Repo | null } @@ -47,18 +48,19 @@ function makeStore(...repos: Repo[]): RepoIdentityStore & { updateRepo: ReturnTy return { getRepos: () => repos, getRepo: (id) => repos.find((candidate) => candidate.id === id), - // Mirrors the real store: `hostId` is matched against the row's own stamp, so a write - // addressed to the wrong host finds no row (src/main/persistence/tracking-repos). - updateRepo: vi.fn((id, updates, hostId) => { - const target = repos.find( + // Mirrors the real store: the target's host is matched against the row's own stamp, so a + // write addressed to the wrong host finds no row (src/main/persistence/tracking-repos). + updateRepo: vi.fn((target: RepoHostTarget, updates) => { + const row = repos.find( (candidate) => - candidate.id === id && (!hostId || getRepoExecutionHostId(candidate) === hostId) + candidate.id === target.id && + getRepoExecutionHostId(candidate) === getRepoExecutionHostId(target) ) - if (!target) { + if (!row) { return null } - Object.assign(target, updates) - return target + Object.assign(row, updates) + return row }) } } @@ -170,11 +172,9 @@ describe('enrichMissingRepoGitRemoteIdentities', () => { 'local', expect.objectContaining({ signal: expect.any(AbortSignal) }) ) - expect(store.updateRepo).toHaveBeenCalledWith( - 'repo-1', - { gitRemoteIdentity: remoteIdentity }, - 'runtime:env-a' - ) + expect(store.updateRepo).toHaveBeenCalledWith(repoTarget('repo-1', 'runtime:env-a'), { + gitRemoteIdentity: remoteIdentity + }) expect(repo.gitRemoteIdentity).toEqual(remoteIdentity) }) @@ -238,7 +238,9 @@ describe('enrichMissingRepoGitRemoteIdentities', () => { enrichMissingRepoGitRemoteIdentities(store) await flushRepoGitRemoteIdentityEnrichmentForTests() - expect(store.updateRepo).toHaveBeenCalledWith('repo-1', { gitRemoteIdentity: null }, 'local') + expect(store.updateRepo).toHaveBeenCalledWith(repoTarget('repo-1', 'local'), { + gitRemoteIdentity: null + }) expect(repo.gitRemoteIdentity).toBeNull() }) @@ -273,11 +275,9 @@ describe('enrichMissingRepoGitRemoteIdentities', () => { enrichMissingRepoGitRemoteIdentities(store) await flushRepoGitRemoteIdentityEnrichmentForTests() - expect(store.updateRepo).toHaveBeenCalledWith( - 'repo-1', - { gitRemoteIdentity: remoteIdentity }, - 'local' - ) + expect(store.updateRepo).toHaveBeenCalledWith(repoTarget('repo-1', 'local'), { + gitRemoteIdentity: remoteIdentity + }) }) it('does not re-probe a resolved identity before the refresh window elapses', async () => { @@ -330,11 +330,9 @@ describe('enrichMissingRepoGitRemoteIdentities', () => { enrichMissingRepoGitRemoteIdentities(store, { onChanged }) await drainEnrichmentSweep() - expect(store.updateRepo).toHaveBeenCalledWith( - 'repo-1', - { gitRemoteIdentity: movedIdentity }, - 'local' - ) + expect(store.updateRepo).toHaveBeenCalledWith(repoTarget('repo-1', 'local'), { + gitRemoteIdentity: movedIdentity + }) expect(repo.gitRemoteIdentity).toEqual(movedIdentity) expect(onChanged).toHaveBeenCalledTimes(1) }) @@ -502,7 +500,7 @@ describe('retiring probes for removed repos', () => { return { getRepos: () => live, getRepo: (id) => live.find((candidate) => candidate.id === id), - updateRepo: vi.fn((id, updates) => { + updateRepo: vi.fn(({ id }: RepoHostTarget, updates) => { const target = live.find((candidate) => candidate.id === id) if (!target) { return null diff --git a/src/main/repo-git-remote-identity-enrichment.ts b/src/main/repo-git-remote-identity-enrichment.ts index e222c33d9a9..6769b89d8f0 100644 --- a/src/main/repo-git-remote-identity-enrichment.ts +++ b/src/main/repo-git-remote-identity-enrichment.ts @@ -5,6 +5,7 @@ import { type ExecutionHostId } from '../shared/execution-host' import type { Repo } from '../shared/repo-types' +import type { RepoHostTarget } from '../shared/repo-host-identity' import { githubAvatarIcon, type RepoIcon } from '../shared/repo-icon' import { isUnresolvedSshHostAlias } from '../shared/git-remote-host-alias' import { getProjectProviderIdentity } from '../shared/project-host-setup-projection' @@ -28,9 +29,8 @@ type RepoIdentityStore = { getRepos(): Repo[] getRepo?(id: string, hostId?: ExecutionHostId): Repo | undefined updateRepo( - id: string, - updates: Pick, 'gitRemoteIdentity' | 'repoIcon'>, - hostId?: ExecutionHostId + target: RepoHostTarget, + updates: Pick, 'gitRemoteIdentity' | 'repoIcon'> ): Repo | null } @@ -145,11 +145,10 @@ function writeIdentity( const icon = gitRemoteIdentity ? getAutomaticGitHubIconRefresh(current, gitRemoteIdentity) : undefined - // The row's own host, not the probe's: the store matches this argument against the row's stamp, - // so a `runtime:` row probed locally is only addressable here under that stamp. - const storeHostId = getRepoExecutionHostId(snapshot) + // The row's own identity, not the probe's host: a `runtime:` row probed locally is only + // addressable under its own stamp. const update = (updates: Pick, 'gitRemoteIdentity' | 'repoIcon'>): Repo | null => { - return store.updateRepo(snapshot.id, updates, storeHostId) + return store.updateRepo(snapshot, updates) } if (icon) { return !!update({ ...(writeRemote ? { gitRemoteIdentity } : {}), repoIcon: icon }) diff --git a/src/main/runtime/acknowledged-terminal-tab-retirement.test.ts b/src/main/runtime/acknowledged-terminal-tab-retirement.test.ts index 6e55edb32df..813dbfae064 100644 --- a/src/main/runtime/acknowledged-terminal-tab-retirement.test.ts +++ b/src/main/runtime/acknowledged-terminal-tab-retirement.test.ts @@ -139,7 +139,7 @@ it('rechecks current pins after renderer acknowledgement', async () => { it('keeps persistence writable when worktree teardown finds remaining terminal rows', async () => { const f = fixture() - f.store.updateRepo('repo1', { executionHostId: 'ssh:target' }) + f.store.updateRepo({ id: 'repo1' }, { executionHostId: 'ssh:target' }) f.store.setWorktreeMeta(ACK_WORKTREE, { hostId: 'ssh:target' }) f.store.setWorkspaceSession(f.store.getWorkspaceSession(), 'ssh:target') vi.spyOn(f.runtime, 'closeMobileSessionTab').mockResolvedValue(delegatedMobileSessionTabClose()) diff --git a/src/main/runtime/orca-runtime-test-fixtures.spec.ts b/src/main/runtime/orca-runtime-test-fixtures.spec.ts index 597edda24d7..22944b86cfe 100644 --- a/src/main/runtime/orca-runtime-test-fixtures.spec.ts +++ b/src/main/runtime/orca-runtime-test-fixtures.spec.ts @@ -406,7 +406,7 @@ const store = { getRetiredWorktreeNameRegistry: () => ({ exhaustedTiers: 0, names: [] }), mergeRetiredWorktreeNames: () => false, addRepo: () => {}, - updateRepo: (id: string, updates: Record) => + updateRepo: ({ id }: { id: string }, updates: Record) => ({ ...store.getRepo(id), ...updates diff --git a/src/main/runtime/orca-runtime-tests/repository-project-operations-part-02.spec.ts b/src/main/runtime/orca-runtime-tests/repository-project-operations-part-02.spec.ts index 81c9e856da7..9da1ae1583a 100644 --- a/src/main/runtime/orca-runtime-tests/repository-project-operations-part-02.spec.ts +++ b/src/main/runtime/orca-runtime-tests/repository-project-operations-part-02.spec.ts @@ -43,7 +43,7 @@ describe('OrcaRuntimeService', () => { repos.push(repo) }, getRepo: (id: string) => repos.find((repo) => repo.id === id) as never, - updateRepo: (id: string, updates: Record) => { + updateRepo: ({ id }: { id: string }, updates: Record) => { const index = repos.findIndex((repo) => repo.id === id) if (index === -1) { return null @@ -123,7 +123,7 @@ describe('OrcaRuntimeService', () => { repos.push(repo) }, getRepo: (id: string) => repos.find((repo) => repo.id === id) as never, - updateRepo: (id: string, updates: Record) => { + updateRepo: ({ id }: { id: string }, updates: Record) => { const index = repos.findIndex((repo) => repo.id === id) if (index === -1) { return null @@ -379,7 +379,7 @@ describe('OrcaRuntimeService', () => { const colorStore = { ...store, getRepos: () => [existing], - updateRepo: (id: string, repoUpdates: Record) => { + updateRepo: ({ id }: { id: string }, repoUpdates: Record) => { updates.push({ id, updates: repoUpdates }) return upgraded as never } diff --git a/src/main/runtime/orca-runtime-tests/repository-project-operations.spec.ts b/src/main/runtime/orca-runtime-tests/repository-project-operations.spec.ts index babf21eac28..c0e313c2e93 100644 --- a/src/main/runtime/orca-runtime-tests/repository-project-operations.spec.ts +++ b/src/main/runtime/orca-runtime-tests/repository-project-operations.spec.ts @@ -130,7 +130,7 @@ describe('OrcaRuntimeService', () => { repos.push(repo) }, getRepo: (id: string) => repos.find((repo) => repo.id === id) as never, - updateRepo: (id: string, updates: Record) => { + updateRepo: ({ id }: { id: string }, updates: Record) => { const index = repos.findIndex((repo) => repo.id === id) if (index === -1) { return null @@ -207,7 +207,7 @@ describe('OrcaRuntimeService', () => { getRepos: () => [...repos] as never, addRepo: (repo: Record) => repos.push(repo), getRepo: (id: string) => repos.find((repo) => repo.id === id) as never, - updateRepo: (id: string, updates: Record) => { + updateRepo: ({ id }: { id: string }, updates: Record) => { const repo = repos.find((entry) => entry.id === id) if (!repo) { return null @@ -259,7 +259,7 @@ describe('OrcaRuntimeService', () => { getRepos: () => [...repos] as never, addRepo: (repo: Record) => repos.push(repo), getRepo: (id: string) => repos.find((repo) => repo.id === id) as never, - updateRepo: (id: string, updates: Record) => { + updateRepo: ({ id }: { id: string }, updates: Record) => { const repo = repos.find((entry) => entry.id === id) if (!repo) { return null @@ -348,7 +348,7 @@ describe('OrcaRuntimeService', () => { repos.push(repo) }, getRepo: (id: string) => repos.find((repo) => repo.id === id) as never, - updateRepo: (id: string, updates: Record) => { + updateRepo: ({ id }: { id: string }, updates: Record) => { const index = repos.findIndex((repo) => repo.id === id) if (index === -1) { return null @@ -465,7 +465,7 @@ describe('OrcaRuntimeService', () => { repos.push(repo) }, getRepo: (id: string) => repos.find((repo) => repo.id === id) as never, - updateRepo: (id: string, updates: Record) => { + updateRepo: ({ id }: { id: string }, updates: Record) => { const index = repos.findIndex((repo) => repo.id === id) if (index === -1) { return null @@ -506,7 +506,7 @@ describe('OrcaRuntimeService', () => { repos.push(repo) }, getRepo: (id: string) => repos.find((repo) => repo.id === id) as never, - updateRepo: (id: string, updates: Record) => { + updateRepo: ({ id }: { id: string }, updates: Record) => { const index = repos.findIndex((repo) => repo.id === id) if (index === -1) { return null @@ -539,7 +539,7 @@ describe('OrcaRuntimeService', () => { repos.push(repo) }, getRepo: (id: string) => repos.find((repo) => repo.id === id) as never, - updateRepo: (id: string, updates: Record) => { + updateRepo: ({ id }: { id: string }, updates: Record) => { const index = repos.findIndex((repo) => repo.id === id) if (index === -1) { return null diff --git a/src/main/runtime/repo-icon-fork-backfill.test.ts b/src/main/runtime/repo-icon-fork-backfill.test.ts index 20a4475b9d8..20317f65dbb 100644 --- a/src/main/runtime/repo-icon-fork-backfill.test.ts +++ b/src/main/runtime/repo-icon-fork-backfill.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import type { Repo } from '../../shared/repo-types' import * as client from '../github/client' import { OrcaRuntimeService } from './orca-runtime' +import { repoTarget } from '../persistence/repo-host-target-matcher.test-fixture' vi.mock('../github/client', async (importOriginal) => ({ ...(await importOriginal>()), @@ -33,8 +34,8 @@ function makeRepo(overrides: Partial = {}): Repo { } function attachStore(runtime: OrcaRuntimeService, repos: Repo[]) { - const updateRepo = vi.fn((repoId: string, updates: Partial) => { - const index = repos.findIndex((repo) => repo.id === repoId) + const updateRepo = vi.fn((target: Pick, updates: Partial) => { + const index = repos.findIndex((repo) => repo.id === target.id) repos[index] = { ...repos[index], ...updates } }) Object.assign(runtime, { store: { getRepos: () => repos, updateRepo } }) @@ -54,19 +55,15 @@ describe('startup fork-upstream backfill', () => { await (runtime as unknown as BackfillInternals).repositoryForkBackfill.run() - expect(updateRepo).toHaveBeenCalledExactlyOnceWith( - 'repo-1', - { - upstream: { owner: 'upstream-org', repo: 'rocket' }, - repoIcon: { - type: 'image', - src: 'https://github.com/acme.png?size=64', - source: 'github', - label: 'acme/rocket-pro' - } - }, - 'local' - ) + expect(updateRepo).toHaveBeenCalledExactlyOnceWith(repoTarget('repo-1'), { + upstream: { owner: 'upstream-org', repo: 'rocket' }, + repoIcon: { + type: 'image', + src: 'https://github.com/acme.png?size=64', + source: 'github', + label: 'acme/rocket-pro' + } + }) }) it('migrates a same-name fork to the upstream owner avatar', async () => { @@ -87,19 +84,15 @@ describe('startup fork-upstream backfill', () => { await (runtime as unknown as BackfillInternals).repositoryForkBackfill.run() - expect(updateRepo).toHaveBeenCalledExactlyOnceWith( - 'repo-1', - { - upstream: { owner: 'upstream-org', repo: 'rocket' }, - repoIcon: { - type: 'image', - src: 'https://github.com/upstream-org.png?size=64', - source: 'github', - label: 'upstream-org/rocket' - } - }, - 'local' - ) + expect(updateRepo).toHaveBeenCalledExactlyOnceWith(repoTarget('repo-1'), { + upstream: { owner: 'upstream-org', repo: 'rocket' }, + repoIcon: { + type: 'image', + src: 'https://github.com/upstream-org.png?size=64', + source: 'github', + label: 'upstream-org/rocket' + } + }) }) it('skips every row whose files sit on an SSH host', async () => { @@ -145,12 +138,8 @@ describe('startup fork-upstream backfill', () => { resolveSlug({ owner: 'acme', repo: 'rocket-pro' }) await backfill - expect(updateRepo).toHaveBeenCalledExactlyOnceWith( - 'repo-1', - { - upstream: { owner: 'upstream-org', repo: 'rocket' } - }, - 'local' - ) + expect(updateRepo).toHaveBeenCalledExactlyOnceWith(repoTarget('repo-1'), { + upstream: { owner: 'upstream-org', repo: 'rocket' } + }) }) }) diff --git a/src/main/runtime/runtime-project-host-setup-controller.test.ts b/src/main/runtime/runtime-project-host-setup-controller.test.ts index d2ecb757276..7e03490f037 100644 --- a/src/main/runtime/runtime-project-host-setup-controller.test.ts +++ b/src/main/runtime/runtime-project-host-setup-controller.test.ts @@ -29,7 +29,7 @@ function makeController(): { const store = { getProjects: () => projectHostSetupProjectionFromRepos([repo]).projects, getProjectHostSetups: () => [], - updateRepo: (_id: string, updates: Record) => ({ ...repo, ...updates }) + updateRepo: (_target: unknown, updates: Record) => ({ ...repo, ...updates }) } const addRepo = vi.fn().mockResolvedValue(repo) const cloneRepo = vi.fn().mockResolvedValue(repo) diff --git a/src/main/runtime/runtime-project-host-setup-controller.ts b/src/main/runtime/runtime-project-host-setup-controller.ts index 548f4636fd0..2ce0e4c950b 100644 --- a/src/main/runtime/runtime-project-host-setup-controller.ts +++ b/src/main/runtime/runtime-project-host-setup-controller.ts @@ -286,7 +286,7 @@ export class RuntimeProjectHostSetupController { if (!identity || getProjectIdForProviderIdentity(identity) !== args.projectId) { throw new Error('Imported folder does not match the selected project identity.') } - const updated = store.updateRepo(repo.id, { + const updated = store.updateRepo(repo, { upstream: { owner: identity.owner, repo: identity.repo, @@ -300,7 +300,7 @@ export class RuntimeProjectHostSetupController { setup = getProjectHostSetupForRepo(this.listSetups(), repo) } const setupMethod = args.setupMethod ?? 'imported-existing-folder' - const updated = store.updateRepo(repo.id, { projectHostSetupMethod: setupMethod }) + const updated = store.updateRepo(repo, { projectHostSetupMethod: setupMethod }) if (!updated) { throw new Error( `Project setup repo disappeared before setup metadata could be linked: ${repo.id}` diff --git a/src/main/runtime/runtime-remove-project-host-scope.spec.ts b/src/main/runtime/runtime-remove-project-host-scope.spec.ts index fabf93c1be1..51891522070 100644 --- a/src/main/runtime/runtime-remove-project-host-scope.spec.ts +++ b/src/main/runtime/runtime-remove-project-host-scope.spec.ts @@ -9,7 +9,8 @@ import { describe, expect, it, vi } from 'vitest' import type { Repo } from '../../shared/repo-types' import { OrcaRuntimeService } from './orca-runtime' import { findRepoRowForHostScopedWrite } from '../persistence/tracking-repos/repo-update-operations' -import type { ExecutionHostId } from '../../shared/execution-host' +import { getRepoExecutionHostId } from '../../shared/execution-host' +import type { RepoHostTarget } from '../../shared/repo-host-identity' export function registerRuntimeRemoveProjectHostScopeTests(): void { function makeRepos(): Repo[] { @@ -55,8 +56,8 @@ export function registerRuntimeRemoveProjectHostScopeTests(): void { removeWorktreeMeta: vi.fn(), getGitHubCache: () => null, // Same row lookup the real store's host-scoped write uses. - updateRepo: (id: string, updates: Partial, hostId?: ExecutionHostId) => { - const row = findRepoRowForHostScopedWrite(repos, id, hostId) + updateRepo: (target: RepoHostTarget, updates: Partial) => { + const row = findRepoRowForHostScopedWrite(repos, target.id, getRepoExecutionHostId(target)) return row ? Object.assign(row, updates) : null }, removeProjectForHost diff --git a/src/main/runtime/runtime-repository-clone-controller.ts b/src/main/runtime/runtime-repository-clone-controller.ts index d56b0615178..7323aad06b0 100644 --- a/src/main/runtime/runtime-repository-clone-controller.ts +++ b/src/main/runtime/runtime-repository-clone-controller.ts @@ -166,7 +166,7 @@ export class RuntimeRepositoryCloneController { isFolderRepo(current) && runtimeRepoMatchesExecutionHost(current, executionHostId) ) { - const updated = store.updateRepo(current.id, { kind: 'git', ...conversion }) + const updated = store.updateRepo(current, { kind: 'git', ...conversion }) if (updated) { await prepareLocalWorktreeRootForRepo(store, updated) invalidateAuthorizedRootsCache() diff --git a/src/main/runtime/runtime-repository-fork-backfill.ts b/src/main/runtime/runtime-repository-fork-backfill.ts index 4bbee92e640..19548f0614c 100644 --- a/src/main/runtime/runtime-repository-fork-backfill.ts +++ b/src/main/runtime/runtime-repository-fork-backfill.ts @@ -74,7 +74,7 @@ export class RuntimeRepositoryForkBackfill { ) { updates.repoIcon = repoIcon } - store.updateRepo(repo.id, updates, hostId) + store.updateRepo(current, updates) changed = true } if (changed) { diff --git a/src/main/runtime/runtime-repository-registration-controller.ts b/src/main/runtime/runtime-repository-registration-controller.ts index d7a0cff081b..5595c4bb3b1 100644 --- a/src/main/runtime/runtime-repository-registration-controller.ts +++ b/src/main/runtime/runtime-repository-registration-controller.ts @@ -66,7 +66,7 @@ export class RuntimeRepositoryRegistrationController { parseExecutionHostId(executionHostId)?.kind === 'runtime' ) { const adopted = - store.updateRepo(existing.id, { executionHostId }) ?? + store.updateRepo(existing, { executionHostId }) ?? ({ ...existing, executionHostId } as Repo) this.invalidate(existing.id) return adopted diff --git a/src/main/runtime/runtime-repository-settings-controller.ts b/src/main/runtime/runtime-repository-settings-controller.ts index c742b36ee7a..bd84bd3571a 100644 --- a/src/main/runtime/runtime-repository-settings-controller.ts +++ b/src/main/runtime/runtime-repository-settings-controller.ts @@ -73,11 +73,7 @@ export class RuntimeRepositorySettingsController { if (isFolderRepo(repo)) { throw new Error('Folder mode does not support base refs.') } - const updated = store.updateRepo( - repo.id, - { worktreeBaseRef: baseRef }, - getRepoExecutionHostId(repo) - ) + const updated = store.updateRepo(repo, { worktreeBaseRef: baseRef }) if (!updated) { throw new Error('repo_not_found') } @@ -105,12 +101,7 @@ export class RuntimeRepositorySettingsController { } const write = (proven: Pick): Repo | null => { assertWslMoveRepoWritable(store, repo) - // Why the host: a same-id sibling on another host would take an id-only write. - return store.updateRepo( - repo.id, - { ...sanitizedUpdates, ...proven }, - getRepoExecutionHostId(repo) - ) + return store.updateRepo(repo, { ...sanitizedUpdates, ...proven }) } const updated = sanitizedUpdates.kind === 'git' && isFolderRepo(repo) diff --git a/src/main/runtime/runtime-rpc-worktree-store-fixtures.ts b/src/main/runtime/runtime-rpc-worktree-store-fixtures.ts index 34619a68c88..d075672984f 100644 --- a/src/main/runtime/runtime-rpc-worktree-store-fixtures.ts +++ b/src/main/runtime/runtime-rpc-worktree-store-fixtures.ts @@ -13,7 +13,7 @@ export const makeStore = (overrides?: { isUnread?: boolean }) => ({ } ], addRepo: () => {}, - updateRepo: (id: string, updates: Record) => + updateRepo: ({ id }: { id: string }, updates: Record) => ({ ...makeStore(overrides).getRepo(id), ...updates diff --git a/src/main/runtime/terminal-list-payload-size.test.ts b/src/main/runtime/terminal-list-payload-size.test.ts index 98ddb3a00cb..e90e7dac132 100644 --- a/src/main/runtime/terminal-list-payload-size.test.ts +++ b/src/main/runtime/terminal-list-payload-size.test.ts @@ -30,7 +30,7 @@ const makeStore = () => ({ { id: REPO_ID, path: '/tmp/repo', displayName: 'repo', badgeColor: 'blue', addedAt: 1 } ], addRepo: () => {}, - updateRepo: (id: string) => makeStore().getRepo(id) as never, + updateRepo: () => null, getAllWorktreeMeta: () => ({ [WORKTREE_ID]: { displayName: 'foo', diff --git a/src/main/ssh/orcad-migration-cutover-coordinator.test.ts b/src/main/ssh/orcad-migration-cutover-coordinator.test.ts index cca170c90fd..38ebb2400be 100644 --- a/src/main/ssh/orcad-migration-cutover-coordinator.test.ts +++ b/src/main/ssh/orcad-migration-cutover-coordinator.test.ts @@ -142,7 +142,7 @@ describe('migration cutover coordinator', () => { it.each([ [ 'a source row changed', - (store: Store) => store.updateRepo('repo-1', { displayName: 'Renamed' }), + (store: Store) => store.updateRepo(store.getRepo('repo-1')!, { displayName: 'Renamed' }), 'orcad_migration_source_changed' ], [ @@ -165,7 +165,7 @@ describe('migration cutover coordinator', () => { it('refuses a commit whose source changed after staging', async () => { const h = await setup() await stageOrcadMigrationDestination(h.context, h.migrationId) - h.store.updateRepo('repo-1', { displayName: 'Renamed' }) + h.store.updateRepo(h.store.getRepo('repo-1')!, { displayName: 'Renamed' }) await expect(commitOrcadMigrationDestination(h.context, h.migrationId)).rejects.toThrow( 'orcad_migration_source_changed' ) diff --git a/src/main/ssh/orcad-migration-delta-move.test.ts b/src/main/ssh/orcad-migration-delta-move.test.ts index d02d9cb53ec..346b9b0b4d3 100644 --- a/src/main/ssh/orcad-migration-delta-move.test.ts +++ b/src/main/ssh/orcad-migration-delta-move.test.ts @@ -136,7 +136,7 @@ async function convertedThenChangedOnOlderBuild(): Promise { }) ).resolves.toMatchObject({ outcome: 'converted' }) store.addRepo(repo('repo-2', '/srv/tool')) - store.updateRepo('repo-1', { displayName: 'app-renamed' }) + store.updateRepo(store.getRepo('repo-1')!, { displayName: 'app-renamed' }) reconcileManagedOrcadSshTargets(userDataPath, store, now) expect(store.getSshTarget(TARGET.id)?.orcadFence?.sourceChangedAt).toBeDefined() } diff --git a/src/main/ssh/orcad-migration-repository-link.test.ts b/src/main/ssh/orcad-migration-repository-link.test.ts index d6370bd77a3..66e9b60f834 100644 --- a/src/main/ssh/orcad-migration-repository-link.test.ts +++ b/src/main/ssh/orcad-migration-repository-link.test.ts @@ -345,7 +345,7 @@ describe('moving onto a server another desktop already added the same project to }) await expect(convert()).rejects.toThrow('socket closed') otherDesktopAddsSameCheckout() - store.updateRepo('desktop-only', { displayName: 'Renamed' }) + store.updateRepo(store.getRepo('desktop-only')!, { displayName: 'Renamed' }) await expect(convert()).rejects.toThrow('orcad_migration_source_changed') expect(listOrcadMigrationSourceCutovers(userDataPath)[0]?.linkedRepositoryIds).toBeUndefined() }) diff --git a/src/shared/repo-host-identity.ts b/src/shared/repo-host-identity.ts index d85c49be81c..f4c148b31ef 100644 --- a/src/shared/repo-host-identity.ts +++ b/src/shared/repo-host-identity.ts @@ -1,8 +1,13 @@ +import type { Repo } from './repo-types' + declare const hostScopedRepoKeyBrand: unique symbol // Why: for new code only; existing raw-id stores stay as they are (D7). export type HostScopedRepoKey = string & { readonly [hostScopedRepoKeyBrand]: true } +/** A repo write's target: the row's own identity fields, so its host can't be dropped or guessed. */ +export type RepoHostTarget = Pick + export function getRepoHostIdentityForParts(repoId: string, hostId: string): HostScopedRepoKey { // Why: host ids and repo ids can contain punctuation; NUL keeps the composite // key collision-free without escaping user/provider-owned strings. diff --git a/tests/e2e/folder-upgrade-identity-persistence.unit.test.ts b/tests/e2e/folder-upgrade-identity-persistence.unit.test.ts index 417133b7c43..ea55aa7ff98 100644 --- a/tests/e2e/folder-upgrade-identity-persistence.unit.test.ts +++ b/tests/e2e/folder-upgrade-identity-persistence.unit.test.ts @@ -37,7 +37,7 @@ it('retains the folder instance and metadata through upgrade, listing, and Store activeTabId: 'omp-tab', tabsByWorktree: { [id]: [makeTerminalTab({ id: 'omp-tab', worktreeId: id })] } }) - store.updateRepo(owner.id, { kind: 'git', folderUpgradeGitRootPath: 'C:/projects/draft' }) + store.updateRepo(owner, { kind: 'git', folderUpgradeGitRootPath: 'C:/projects/draft' }) store.flush() const reloaded = createStore() const repo = reloaded.getRepo(owner.id) @@ -95,7 +95,7 @@ it('retains the folder instance and metadata through upgrade, listing, and Store it('drops upgrade path evidence when execution ownership changes', () => { const store = createStore() store.addRepo(makeRepo({ id: 'folder', path: 'C:\\projects\\draft', kind: 'git' })) - store.updateRepo('folder', { folderUpgradeGitRootPath: 'C:/projects/draft' }) - store.updateRepo('folder', { executionHostId: 'ssh:builder' }) + store.updateRepo({ id: 'folder' }, { folderUpgradeGitRootPath: 'C:/projects/draft' }) + store.updateRepo({ id: 'folder' }, { executionHostId: 'ssh:builder' }) expect(store.getRepo('folder')?.folderUpgradeGitRootPath).toBeUndefined() })