Repository navigation
refactor(projects): repo, project group and folder catalog calls route by their owner - #27288
Conversation
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🔵 Low · up to The project-added dialog can close prematurely during overlapping catalog refreshes. This is a bounded issue, but the hydration result should be made authoritative before merge or accepted for follow-up. Pre-merge checks |
|
…e by their owner Catalog fetches, path-status probes, project and group mutations, nested scans and imports now take the owner's host explicitly instead of falling back to the Active Server setting. A new project with no picked host still goes to the default host, through defaultCreationHost.
4253520 to
575c55e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
20a296aa-2dba-4888-8b7a-51c3414cab14
📒 Files selected for processing (66)
config/focus-setting-read-baseline.txtconfig/owner-routing-baseline.txtsrc/renderer/src/components/settings/settings-project-section-renderer.tsxsrc/renderer/src/components/sidebar/AddRepoSteps.tsxsrc/renderer/src/components/sidebar/ProjectAddedDialog.test.tsxsrc/renderer/src/components/sidebar/ProjectAddedDialog.tsxsrc/renderer/src/components/sidebar/add-repo-runtime-owner.tssrc/renderer/src/components/sidebar/folder-workspace-composer-path-status.test.tsxsrc/renderer/src/components/sidebar/folder-workspace-composer-path-status.tssrc/renderer/src/components/sidebar/use-worktree-context-menu-commands.tssrc/renderer/src/components/sidebar/useAddRepoCloneFlow.tssrc/renderer/src/components/sidebar/useAddRepoNestedImportFlow.test.tssrc/renderer/src/components/sidebar/useAddRepoNestedImportFlow.tssrc/renderer/src/components/sidebar/useAddRepoNestedReviewController.tssrc/renderer/src/components/sidebar/useAddRepoNestedReviewState.tssrc/renderer/src/components/sidebar/useAddRepoServerPathFlow.tssrc/renderer/src/components/sidebar/useCreateRepo.tssrc/renderer/src/components/sidebar/worktree-list/listing/use-folder-path-statuses.tssrc/renderer/src/components/sidebar/worktree-list/rows/use-project-group-dialogs.tssrc/renderer/src/hooks/composer-state/composer-store-actions.tssrc/renderer/src/hooks/ipc-events/project-catalog-ipc-bridge.tssrc/renderer/src/lib/workspace-activation-path-gate.test.tssrc/renderer/src/store/folder-workspaces/folder-path-status-actions.tssrc/renderer/src/store/folder-workspaces/folder-workspace-catalog-actions.tssrc/renderer/src/store/folder-workspaces/folder-workspace-mutations.tssrc/renderer/src/store/folder-workspaces/folder-workspace-routing.tssrc/renderer/src/store/github/work-item-mutation-actions.tssrc/renderer/src/store/project-groups/nested-repository-operations.tssrc/renderer/src/store/project-groups/project-group-catalog-actions.tssrc/renderer/src/store/project-groups/project-group-mutations.tssrc/renderer/src/store/projects/project-host-routing.tssrc/renderer/src/store/projects/project-update.tssrc/renderer/src/store/repos/all-host-repo-catalog-actions.tssrc/renderer/src/store/repos/owner-routing.tssrc/renderer/src/store/repos/repo-add-actions.tssrc/renderer/src/store/repos/repo-catalog-actions.tssrc/renderer/src/store/repos/repo-removal.tssrc/renderer/src/store/repos/repo-state.tssrc/renderer/src/store/repos/repo-update.tssrc/renderer/src/store/repos/runtime-repo-catalog-actions.tssrc/renderer/src/store/slices/folder-workspace-owner-routed-mutations.test.tssrc/renderer/src/store/slices/project-group-owner-routing.tssrc/renderer/src/store/slices/repos-add-races.test.tssrc/renderer/src/store/slices/repos-all-hosts-generation.test.tssrc/renderer/src/store/slices/repos-all-hosts.test.tssrc/renderer/src/store/slices/repos-catalog-merge-identity.test.tssrc/renderer/src/store/slices/repos-catalog-notifications.test.tssrc/renderer/src/store/slices/repos-cross-host-refresh-identity.test.tssrc/renderer/src/store/slices/repos-module-lifetime-coordinators.test.tssrc/renderer/src/store/slices/repos-nested-import-refresh-failures.test.tssrc/renderer/src/store/slices/repos-nested-ssh-projection.test.tssrc/renderer/src/store/slices/repos-project-group-create-race.test.tssrc/renderer/src/store/slices/repos-project-groups-delete.test.tssrc/renderer/src/store/slices/repos-project-groups-owner-routing.test.tssrc/renderer/src/store/slices/repos-project-groups.test.tssrc/renderer/src/store/slices/repos-project-host-capability.test.tssrc/renderer/src/store/slices/repos-project-host-lifecycle.test.tssrc/renderer/src/store/slices/repos-project-runtime.test.tssrc/renderer/src/store/slices/repos-refresh-identity.test.tssrc/renderer/src/store/slices/repos-runtime-project-groups.test.tssrc/renderer/src/store/slices/repos-selected-owner-routing.test.tssrc/renderer/src/store/slices/repos-setup-script-dismissals.test.tssrc/renderer/src/store/slices/repos-stale-fetch.test.tssrc/renderer/src/store/slices/repos.test.tssrc/renderer/src/store/slices/runtime-catalog-merge-removed-env-guard.test.tssrc/renderer/src/store/slices/settings.ts
💤 Files with no reviewable changes (3)
- config/owner-routing-baseline.txt
- src/renderer/src/store/projects/project-host-routing.ts
- src/renderer/src/store/repos/owner-routing.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.
| void (async () => { | ||
| await fetchRepos() | ||
| // Why: no row yet names the new project's host, so look on every host. | ||
| await fetchReposForAllHosts() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not close the dialog after a superseded catalog load.
If another all-host load supersedes this call before remote catalogs are fetched, fetchReposForAllHosts() returns early. The subsequent lookup can miss a remote repo that the newer load is still fetching, so the dialog closes before its project handoff. Wait for an authoritative hydration result before treating the repo as absent.
Upstream now routes project group catalog calls by their owner host (stablyai#27288): createProjectGroup takes the seeding project's host, and rows are adopted through adoptFromEndpoint. Subgroup creation moves to its own createProjectSubgroup action, routed by the parent group's owner like moveProjectGroup, and the update/move helper uses the new owner routing. protocol-version.ts re-exports the attention capabilities with a star, as stablyai#27493 did for the resume ones, to stay within the line limit.
ELI5
Orca keeps a list of projects, project groups and folder workspaces from every computer it knows about: this computer, plus any paired Orca servers. Some actions on that list still asked "which server is picked in the Active Server setting?" instead of "which computer owns this project?". If you picked a different server, renaming, moving or deleting a project or group could therefore be sent to the wrong computer. Now each of these actions goes to the computer that owns the row. Only adding a brand-new project, where nothing has an owner yet, still uses that setting as its default.
What Changed
fetchRepos,fetchProjectGroupsandfetchFolderWorkspacesrequire{ runtimeEnvironmentId: string | null }, wherenullis this app. Every caller already knew the host, except:repos:changedevent, which now refreshes this app's catalog;runtimeTargetForProjectGroupOwner, which replacessettingsForProjectGroupOwner. An id that two hosts both own, with no host given, is refused instead of following focus.createProjectGroup(name, hostId)andmoveProjectToGroup(…, hostId)take the project's host.updateProject(projectId, updates, hostId)updates the project row on the host the caller names.runtimeTargetForWorkspaceOwner. Nested-repo scan, cancel and import take the host picked in Add Project.addRepoPathwithout a host or the OS folder picker, go throughdefaultCreationHost, the one approved reader of the setting.settingsForProjectGroupOwner,getAddRepoPathRouteSettings,getFolderWorkspacePathStatusRouteSettings,getRuntimeTargetCachePrefix,getProjectUpdateRuntimeTarget, and the settings parameter ofresolveAddRepoRuntimeTarget, which always resolved to this app anyway.Why
This is one slice of making the owner-routing count reach zero. The setting becomes "Default host for new projects", and only creation flows that have no source row may read it.
We considered keeping the optional
runtimeEnvironmentIdand treating "missing" as this app. That would compile without the caller ever saying which host it meant, which is the bug this work removes. Making the field required lets the compiler find every caller that hadn't decided.An unresolved or ambiguous owner is refused, rather than run on this computer or on the focused host. Running it either way would be a guess.
Linked Issue
None. This is part of the host-ownership routing stack (D4 G1).
Visual Proof
N/A. The visible UI is unchanged; only the host each call reaches changes. That is covered by unit tests, and the machine is load-capped, so the app wasn't launched.
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
repos-project-runtime.test.tshas a new case. A local Windows project's runtime preference stays on this computer even while a server with a setup for it is the default host; the old code sent it to that server.repos-project-groups-owner-routing.test.ts: a group id that two hosts both own, with no host given, is now refused. It used to follow focus.Several
repos.test.tsandrepos-project-groups-delete.test.tscases relied on an unstamped row following focus to a server. They now stamp the row the way ingest does, so they assert owner routing.Other tests pass the owner explicitly.
Ran:
pnpm tc:web, focusedpnpm test(store, sidebar, settings, hooks, lib: 1853 files),oxlint,oxfmt,pnpm run check:code-quality:changed,pnpm check:owner-routing-ratchet, andcheck:max-lines-ratchet.Review
Agent skill upstream boundary
Notes
runtimeEnvironmentId.connectionId. Server-owned SSH targets route to their server.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)