upstream merge 2026-05-08 PR 12: v1→v2 import flow rewrite#464
upstream merge 2026-05-08 PR 12: v1→v2 import flow rewrite#464
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (47)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 745264ce5b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * WebSocket route is attach-only by `terminalId`. Older host-services would | ||
| * reject the renderer's creation call and still expect socket-side startup. | ||
| */ | ||
| export const MIN_HOST_SERVICE_VERSION = "0.8.0"; |
There was a problem hiding this comment.
Keep host-service minimum version at supported API level
Raising MIN_HOST_SERVICE_VERSION to 0.8.0 blocks hosts that are actually compatible with this tree: packages/host-service/src/trpc/router/host/host.ts still reports HOST_SERVICE_VERSION = "0.3.0", and renderer code still uses terminal.ensureSession (not terminal.createSession). With this floor, every remote workspace is marked incompatible and adopted services are forcibly killed/restarted even though the required 0.8 API migration is not present here.
Useful? React with 👍 / 👎.
| return { | ||
| candidates: [], | ||
| cloudErrors: [ |
There was a problem hiding this comment.
Preserve cloud lookup failures in folder-first import flow
In the non-walkAllRemotes path, cloud lookup errors are now swallowed and converted to { candidates: [] }. Existing callers (e.g. useFolderFirstImport) only inspect candidates, so a transient cloud failure is interpreted as “no existing project” and the UI proceeds to create a new project, causing duplicate project records instead of surfacing an error as before.
Useful? React with 👍 / 👎.
| createdAt: Date; | ||
| updatedAt: Date; |
There was a problem hiding this comment.
Type workspace timestamps to match SDK wire decoding
The SDK now types workspace.createdAt/updatedAt as Date, but this client unwraps raw result.data.json from tRPC without SuperJSON deserialization metadata, so timestamp fields arrive as plain JSON values (typically strings). This makes the exported type unsound and can break consumers that call Date methods directly.
Useful? React with 👍 / 👎.
48e1a88 to
7cde849
Compare
…perset-sh#4122) * refactor(desktop): rewrite v1→v2 migration as pull-based importer Replaces the auto-running batch migration with a manual click-to-import flow across three pages (Projects / Workspaces / Presets) inside the welcome modal. v1MigrationState stays as an audit log; per-row status is derived live from host-service findByPath and the audit log so a re-run is naturally idempotent and cross-device dedup falls out for free. Removes ~3.4k LOC of batch orchestration: the auto-run hook, the heuristic findByPath fallbacks, the singleton concurrency guard, the localStorage event bus, the bulk sidebar writeback, and the separate preset auto-migration. * refactor(desktop): harden v1→v2 importer for forks, ghosts, and relocates Three real bugs the auto-batch v1→v2 migration silently mishandled, surfaced concretely while triaging a coworker's mangled import: - findByPath only ever read the local repo's `origin` (or first GitHub remote). Cloned-fork-as-origin contributors got linked to the wrong v2 project. Now reads every GitHub remote, accepts an `expectedRemoteUrl` hint from v1's recorded `githubOwner`, and tags matching candidates so the picker can recommend them. - Audit success was trusted blind. If another device or user deleted the v2 project, the row stayed flagged "Imported" forever pointing at a ghost. Adds `project.cloudList` + `workspace.cloudList` procedures; pages cross-check audit v2 ids against live cloud and demote ghosts back to "available". - Linking a v1 project whose folder differs from where v2 already had the project set up on this device threw a confusing CONFLICT. Now surfaces a confirm row ("Already set up at X. Link to Y instead?") with explicit Use-this-folder vs Cancel. Plus: surface findByPath cloud query failures (silent `console.warn` before, misleadingly fell through to "Import"); refresh button per page; stale-worktree NOT_FOUND retry-by-branch on adopt; listProjectWorktrees procedure for filtering v1 workspaces against the project's current `git worktree list` so guaranteed-to-fail rows don't surface; invalidate the cloud-list query after every successful import so the audit-ghost detector doesn't false-flag fresh imports. * fix(host-service): restore local-DB short-circuit in findByPath Hardening commit removed the short-circuit and started always walking all remotes + merging cloud results. That changed folder-first import behavior: if your org happens to have multiple v2 projects pointing at the same remote URL, re-importing a folder you'd already linked locally now triggers the multi-project picker instead of silently linking to the local row. /settings/projects has no setup-at-path affordance, so the multi-project toast was a dead end. Restore the short-circuit, gated by a cloud existence check on the local v2 id. Non-stale local hit → return that candidate only. Stale local hit (cloud project deleted) → fall through to the multi-remote walk so callers see real alternatives. Migration importer is unaffected: v1 paths typically have no local-DB hit, so it still takes the cloud-walk path and gets the picker for forks etc. * refactor(host-service): gate findByPath multi-remote walk behind opt-in flag Previous fix unconditionally short-circuited on local-DB hit which preserved folder-first behavior but also limited what the migration importer could see. Cleaner separation: add a `walkAllRemotes` input flag that gates the new discovery semantics (multi-remote walk + expected-URL hint + stale-local-link surfacing). Default false → folder-first sees the long-standing behavior unchanged. Importer passes true and gets the picker-eligible candidate set. No new behavior for any pre-existing caller. * fix(desktop): address PR review nits on v1→v2 importer Bug fixes from review: - findByPath staleness: only mark `staleLocalLink` on a confirmed NOT_FOUND. Transient cloud errors (network, auth, 5xx) used to drop the legitimate local candidate; now they leave it intact and surface via `cloudErrors` instead. - findByPath redundant round-trip: when the cloud-URL loop already saw the local id, skip the per-id `v2Project.get` staleness check (new internal `cloudConfirmed` flag, stripped from wire response). - ImportProjectsPage `linkToProjectId` fall-through: if the picker's candidate has gone stale between render and click, throw a clear error instead of silently calling `project.create` and duplicating the v2 project. - V1ImportModal: presets page used to render alongside the "host service not ready" fallback when activeHostUrl was missing — gated the fallback to projects/workspaces only since presets don't need the host service. Cheap wins: - V1ImportBanner: drop the redundant `useEffect` that re-read sessionStorage after mount (the `useState` initializer already does it synchronously). - ImportProjectsPage `expectedRemoteUrlFor`: split on `[\\/]` so a Windows-style v1 path doesn't silently produce undefined. - ImportPresetsPage: reuse `audit?.v2Id` on retry so a failed audit upsert after a successful collection insert doesn't leave an orphan v2 preset on next click. - Replace silent `.catch(() => {})` on audit-error writes with a warn that names the failing entity, so a real audit-write failure is greppable instead of invisible. * fix(desktop): more PR review nits on v1→v2 importer - ImportProjectsPage: drop `!projectsQuery.data` from isLoading. If readV1Projects errors, isPending flips false but data stays undefined — without this fix the page got stuck in a permanent loading spinner with no path to recovery. Falling through to the empty-state message is a cleaner dead-end. - ImportProjectsPage: replace ad-hoc `[\\/]` regex with the existing `getBaseName` util from `renderer/lib/pathBasename` — same POSIX/Windows/UNC handling, less code, already tested. - V1ImportBanner: scope the session-storage dismiss flag to organizationId. Dismissing in one org no longer hides the banner in every other org for the rest of the session. Re-sync local state on org change so flipping orgs reveals the banner again if it hasn't been dismissed there yet.
…-sh#4128) * refactor(desktop): drop v1_migration_state from importer UX, derive from real state The pull-based v1→v2 importer (d00b174) used `v1_migration_state` as the single source of truth for "is this row already imported?" and "did the last attempt fail?". That created a class of false-failure bugs where a stale audit row from a prior run survived a fix to the underlying state and kept the row showing "Retry — Project not found" forever, even when the new pull-based logic would have handled it cleanly. Removes the audit log as a UX driver for projects, workspaces, and presets. Each surface now derives state from real app state, self-healing across deletions and cross-device changes: - Projects: imported = host-service `findByPath` returns a `local-path` candidate (already cloud-staleness-filtered server-side). - Workspaces: v1→v2 project mapping via host-service `project.list` matched on repoPath; "imported workspace" via cloud workspace list keyed on (projectId, branch). - Presets: v1 preset.id is reused as the v2 collection row id, so "imported" is a single `v2TerminalPresets.has(preset.id)` check. - Banner: "remaining" count = v1 projects whose mainRepoPath isn't in host-service `project.list`. Wire-shape change: `workspace.cloudList` now returns `{id, projectId, branch, hostId}` instead of `{id}`. Only the importer consumes it. Removed: `migration.listState`, `migration.upsertState`, and `project.cloudList` (no callers). Errors are no longer persisted; they live in component state for the modal session and `console.error` for the log file. Also folds in a small V1ImportModal page-transition simplification (`key={page}` + Tailwind `animate-in` instead of an explicit isTransitioning fade) and a `findByPath` integration test fix that was missed in d00b174 — wire shape grew but the assertions didn't loosen. The `v1_migration_state` table and migration 0041 stay in place, inert. Drop in a follow-up after the rollout settles. * test(desktop): manual test fixtures for v1→v2 importer Pair of bash scripts that seed and tear down the full set of fixtures needed to walk the importer modal by hand: dev neon branch v2 projects across two orgs (covering same-URL and fork ambiguity), v1 local.db projects + worktrees + workspaces (clean adopt, stale-path-fallback, orphan-branch-hidden, ghost), on-disk repos with the right remote configurations, and a host.db row that triggers the relocate-confirm flow. setup.sh refuses to run if .env points at the prod neon branch. Useful for QA on every importer change; previously lived untracked in a separate worktree. * fix(desktop): address v1→v2 importer review nits - ImportWorkspacesPage: drop the dead-code IIFE for `alreadyImported` — the visibility loop already filters out workspaces without a v2 project, so the recompute and null guard at the WorkspaceRow call site were unreachable. Carry `(v2ProjectId, alreadyImported)` through the visible-workspace tuple instead. Drops a now-unreachable "blocked" branch in the action computation too. - ImportWorkspacesPage: pass `adoptedV2Id` back as `existingWorkspaceId` on adopt retry. If a previous adopt succeeded in cloud but the local follow-up threw, the retry now hints the workspace id so adopt's idempotency hits instead of provisioning a duplicate workspace. - ImportProjectsPage: invalidate the workspaces tab's host-project-list cache after a successful import. Without it, switching to Workspaces immediately after Import showed an empty list until the staleTime-0 background refetch finished.
) The revert of superset-sh#4120 (superset-sh#4135) restored a hardcoded `tabOrder: 0` in the v2 workspace.create path. Existing rows default to `tabOrder: 0` too, so new workspaces tied with them and ordering was non-deterministic. Restores the `getPrependTabOrder` helper from superset-sh#4120 as a shared util in `dashboardSidebarLocal/`, and uses it from `useWorkspaceCreates` so new rows land strictly above every existing top-level item — matching what the pending-row injection in `useDashboardSidebarData` already assumes.
…opt (superset-sh#4137) * fix(desktop): prefer cloud-confirmed v2 project in v1→v2 workspace adopt When multiple host.db rows share a repo_path, the importer's repoPath→v2id map was last-write-wins, so an orphan local project (e.g. left over from a manual repair) could shadow the canonical id. Workspace adopt then called v2Workspace.create with the orphan id and tripped the org-scope check with "Project not found in this organization". Prefer ids that appear as a projectId on the org's cloud workspace list, falling back to first-seen. * fix(desktop): gate v1 import modal on cloud workspace list cloudWorkspacesQuery was missing from isLoading, so a slow cloud response let the modal render Adopt buttons before the preference logic had the signal it needs to skip orphan host.db rows. The preference would then fall back to first-seen and could still pick the orphan, hitting the exact error this PR is meant to prevent.
…rset-sh#4141) After workspaces.create resolves end-to-end on the host service, the detail page would sit on WorkspaceCreatingState until Electric pushed the synced row into collections.v2Workspaces. If Electric was slow or disconnected, the page perma-loaded even though the workspace was fully usable on the cloud and host. Cache the cloud row returned by the mutation on the in-flight entry and have the layout fall back to it while the live query is empty. Manager.tsx still cleans up the entry once Electric delivers, at which point the live query takes over seamlessly. No optimistic rollback machinery — cloud is the source of truth. Threads the full SelectV2Workspace through workspaces.create (host-service) and the SDK's WorkspaceCreateResult.workspace.
…4148) Drops the "you have N v1 projects you can bring over to v2" banner and its component directory.
…-sh#4151) * feat(desktop): add v1 import intro page + fix preset import Splits the v1 import welcome screen into two pages: the existing "Welcome to Superset v2" hero (now copy-free) and a new intro page that sets expectations — workspaces and projects port over, terminal sessions don't, and v1 stays accessible. Also fixes the preset import flow, which threw "Invalid input - path: id" for builtin agent presets ("claude", "codex"). v1 stores those with the agent name as the preset id, but v2's schema requires id to be a UUID. The import now generates a fresh UUID for the v2 row, sets agentId for the link, and dedups on agentId (builtins) or name (custom presets) instead of v1's id. * fix(desktop): align import row label with imported name + tighten dedup ImportRow showed the raw v1 key ("claude") while the imported v2 row landed under the display label ("Claude Code"). Pass v2Name so the picker matches the post-import name. importedNames also now excludes builtin-linked v2 rows so a custom v1 preset whose name happens to match a builtin display label isn't falsely flagged as already imported.
…ces (superset-sh#4153) * feat(desktop): show host-offline + version-mismatch screens for v2 workspaces Opening a v2 workspace whose remote host has no live relay tunnel previously rendered the workspace UI normally — every query would 503 and every button would silently fail. Gate WorkspaceProvider on a new useRemoteHostStatus hook: read v2Hosts.isOnline from the live collection for the offline check, fire host.info via the workspace tRPC URL for the version check, and swap in WorkspaceHostOfflineState or WorkspaceHostIncompatibleState before mounting the workspace. Move MIN_HOST_SERVICE_VERSION from the desktop main process into @superset/shared/host-version so the renderer reads the same floor the coordinator already enforces locally. * fix(desktop): polish host-offline + version-mismatch screens Frame the icon with a bordered tile and overlay a status pin (muted dot for offline, amber up-arrow for needs-update) so the state reads at a glance. For incompatible, show host name + running/required versions in a single two-row card with the same emerald online dot used in HostHeader, instead of two separate borderless rows. * fix(desktop): handle prerelease host versions and missing v2Hosts row Two edge cases flagged in review: - semver.satisfies excludes prereleases from range comparisons by default, so a host on 0.9.0-dev.1 fails >=0.8.0 and gets the incompatible screen even though it's functionally newer. Pass includePrerelease so dev/staging builds aren't rejected. - If v2Hosts has no row for workspace.hostId once the collection is ready (deregistered host, race with sync), the hook used to return loading forever and the layout rendered a blank div. Fall back to the offline screen so the user gets the Browse workspaces link. * revert(desktop): drop includePrerelease from host-version check Greptile flagged this as a P1, but HOST_SERVICE_VERSION is a hardcoded constant in packages/host-service/src/trpc/router/host/host.ts — hand-bumped on breaking-change PRs (per DAEMON_SUPERVISION.md). The -canary.<timestamp> suffix only goes onto apps/desktop/package.json in the canary workflow; nothing rewrites the host-service constant. So host.info always returns a clean X.Y.Z and includePrerelease has no real value to cover. Worse, it would diverge from host-service-coordinator.ts:295 (plain satisfies, no options) and silently accept e.g. "0.8.0-rc.1" against ">=0.8.0".
superset-sh#4176) * fix(desktop): resolve v2 default from local db, not stale localStorage The v2 default introduced in superset-sh#4115 used `localStorage.getItem("tabs-storage")` to detect returning users, but tabs state was migrated off localStorage to app-state.json (lowdb) in #303 (2025-12-10). The key has been null for every install since, so `!hasPriorSupersetUsage()` evaluated to true for all users and force-flipped anyone without an explicit toggle into v2 on first launch. Resolve the default from the canonical signal instead: ≥1 row in `projects` or `workspaces` in localDb means a returning user. `optInV2` starts `null`, `V2DefaultResolver` runs both `*.hasAny` queries once and persists the result. Users who got force-pulled but never toggled have no persisted override yet, so they self-heal back to v1 on next launch. * fix(desktop): only fresh installs are forced into onboarding The onboarding gate at _authenticated/layout.tsx only checked isV2CloudEnabled && !requiredComplete, so any existing v1 user who explicitly toggled v2 ON was force-walked through /setup/* — wrong: they already have projects/workspaces and should land on the dashboard. Split the v2 default into two flags on the override store: - optInV2: user-controllable (resolver default OR experimental toggle). - isFreshInstall: sticky one-shot detection from the resolver, never changed by user toggles. Onboarding gate now requires isFreshInstall === true. Resolver runs whenever either flag is null, so users with persisted optInV2 from the broken release also get isFreshInstall backfilled on next launch.
…h#4177) * fix(desktop): stop auto-opting users into v2 + onboarding Two recurring complaints on top of each other: fresh installs were being flipped to v2 without explicit opt-in, and v2 users were then force-walked through the experimental onboarding flow on first launch. v2 + onboarding aren't ready to be defaults — only users who explicitly opt in via Settings → Experimental should land there. - Delete V2DefaultResolver. Its only job was to auto-set optInV2=true on detected-fresh installs. With it gone, optInV2 stays null until the user toggles it, and useIsV2CloudEnabled returns false. - Remove the /setup/* redirect in _authenticated/layout.tsx so fresh v2 users land on the dashboard like v1 users. - Drop isFreshInstall from the store + the useIsFreshInstall hook (only consumer was the redirect gate). - Drop the projects.hasAny / workspaces.hasAny tRPC procedures (only consumer was V2DefaultResolver). Preserved: the optInV2 toggle and "Restart onboarding" action in Settings → Experimental, the full /setup/* route tree + onboarding store, and any persisted optInV2=true from explicit toggles. * fix(desktop): bump v2-local-override persist key to clear stale auto-opt-ins V2DefaultResolver wrote optInV2=true to v2-local-override-v2 for every fresh-detected install in the ~24h window since superset-sh#4176 shipped. Those writes are indistinguishable from explicit user toggles in the store, so without bumping the key those users would silently stay on v2 even though they never made a real choice. Bumping to v2-local-override-v3 clears the field for everyone: - existing v1 users: no change (their key was already null/absent) - recently auto-flipped users: land back on v1, can opt in if they want - explicit opt-ins: have to toggle once more Acceptable trade-off vs leaving a small group force-stuck on v2. * Revert "fix(desktop): bump v2-local-override persist key to clear stale auto-opt-ins" This reverts 3ca8703. The bump also clears optInV2 for users who explicitly toggled v2 on and went through onboarding — they'd silently land back on v1 with no projects visible (v2 workspaces live in cloud, not v1 local), looking broken. Worse trade-off than leaving the small ~24h auto-flip cohort stuck on v2.
…#4179) - Add an always-visible close button to the v1→v2 import modal so users can dismiss it from any page. - Clamp modal to viewport in both axes (h-[min(540px,...)] / w-[min(744px,...)]) so it never overflows narrow or short windows and the close/back/next buttons stay reachable. - Truncate long workspace, project, and group names with hover tooltips so a long name can't push the row's action button off-screen. - Reserve header gutter for the X so it no longer overlaps the refresh button.
The v1→v2 importer cherry-picks (superset-sh#4122 et al.) introduced two missing dependencies that need backporting: - packages/host-service/.../shared/worktree-list.ts: required by the new list-project-worktrees procedure. Originally added in upstream superset-sh#4074 (not in this batch). Brought in verbatim from 8ae0b3e. - packages/trpc/.../v2-workspace.ts: workspace.cloudList calls ctx.api.v2Workspace.list which lands in upstream superset-sh#3889. Backported just the list procedure here so the cherry-picked importer has the shape it expects.
Auto-merge of superset-sh#4122 brought useV2AgentConfigs + buildAgentLaunchCommand imports for the agent-aware preset launch flow, but the fork's V2PresetsSection doesn't use that flow yet (depends on agents/PR3). Lint flagged them as unused; remove until the agents PR lands.
70dd124 to
c3883b5
Compare
Recorded as integrated via -s ours after batch PRs #455-#464. Taken via individual PRs: - PR 1 (#455): v2 polish 前半 safe set (9 commits) - PR 2 (#456): v2/host-service polish 中盤 (12 commits) - PR 3 (#457): sidebar polish + jwt API (5 commits) - PR 4 (#458): host-service tRPC retry/cache/timeout (3 commits) - PR 5 (#459): v2 diff pane / file pane polish (2 commits) - PR 7 (#462): host-service v2 canonical workspace.create + attachment store (PR1 superset-sh#3893 + PR2 superset-sh#3916) - PR 11 (#463): agents API + onboarding (7 commits + 1 cleanup) - PR 12 (#464): v1→v2 import flow rewrite (11 commits + 2 follow-ups) - PR 13 (#460): host-service shell env probe + typo (2 commits) - PR 16 (#461): marketplace 19 themes (1 commit) Skipped / deferred (recorded as integrated for behind=0): - PR 6: CLI v1 launch (superset-sh#3898 + 30+ CLI/SDK followups) — defer to dedicated migration - PR 9: v2 PR3 (superset-sh#3940) + revert (superset-sh#4017) — net-zero pair - PR 10: pty-daemon (superset-sh#3896, superset-sh#3971, superset-sh#4054) — fork keeps its terminal-host - PR 14: Slack MCP-v2 (superset-sh#4197, superset-sh#4208) — depends on mcp-v2/sdk divergence - PR 15: onboarding remaining (superset-sh#4115, superset-sh#4125, superset-sh#4214, superset-sh#4213, superset-sh#4222, superset-sh#4225) — depends on fork's deleted setup pages Behind: 0 after this merge.
Summary
upstream 同期バッチ第 12 弾。v1→v2 import 系 (11 commits, sub-agent committed)。
進捗: PR 1-5 + 7 + 13 + 16 マージ済み (合計 36) + PR 11 (#463) merge 待ち、本 PR で +13 commits = 49+。
取り込み内容 (sub-agent委譲で処理)
Plus 2 follow-up commits:
Skipped / Removed
agentIdがない / fork にuseFinalizeProjectSetupがない / workspace 型のrepoPathが無い — UI 側だけドロップして core importer logic のみ採用Fork 側の主な調整
_dashboard/layout.tsx: fork の tearoff/scratch +<KeepAliveWorkspaces />維持。<V1ImportBanner />撤去_authenticated/layout.tsx: fork の{ isV2CloudEnabled }維持、V1MigrationSummaryModalを上流の 不採用 に置き換え(V1ImportModal自体を削除したため、layout からのインポート/レンダリング除去)useIsV2CloudEnabled.ts: fork の object-shape 戻り値 +IS_DEVshort-circuit + upstream のoptInV2 === true厳格チェックv2-local-override.ts: nullableoptInV2+setOptInV2setter + fork のtoggle()を維持useRemoteHostStatus.ts: 引数型をPick<SelectV2Workspace, "id" \| "organizationId" \| "hostId">に narrow して fork の Electric collection 型と互換にするhost-service-coordinator.ts: fork のMIN_HOST_SERVICE_VERSION = "0.3.0"削除 → upstream の0.8.0floor (@superset/shared/host-version) を採用Fork 固有機能ヘルスチェック
githubExtendedprocedure 健在v1MigrationStateテーブル維持 (audit log として残す)Test plan
bun install成功 (lockfile origin/main から再生成)