Upstream sync: v2 sidebar, recently viewed, workspace fixes#204
Merged
Upstream sync: v2 sidebar, recently viewed, workspace fixes#204
Conversation
…create (superset-sh#3494) * fix(desktop): prevent "workspace not found" flash after v2 workspace create The pending page previously navigated to /v2-workspace/$id after 3s even if the workspace row hadn't synced into the local Electric collection. The destination route's live query resolved empty and flashed WorkspaceNotFoundState. Hard-gate navigation on sync completion, bump the stall timeout to 10s, and on stall show a recoverable error with Keep waiting / Open anyway / Dismiss instead of silently navigating into a broken page. * fix(desktop): use styled WorkspaceNotFoundState in v2-workspace layout The layout had its own bare unstyled "Workspace not found" text that fired before the page-level component could render, so users saw a raw centered string instead of the styled empty state. Promote WorkspaceNotFoundState to the shared v2-workspace/components/ folder and use it in both the layout and the page. * Lint * fix(desktop): re-arm sync timeout on "Keep waiting" and reset on pendingId change Clicking "Keep waiting" cleared syncTimedOut but the effect's dep array didn't include it, so no new timer was scheduled. User would land on the "Workspace ready — opening..." branch (no buttons) with no way out if sync still didn't arrive. Add syncTimedOut as a dep and an early-return guard so the effect re-arms cleanly. Also reset syncTimedOut when pendingId changes so a new pending page doesn't inherit the prior one's stall UI. * chore(desktop): trim comments in pending-page sync timeout
…#3492) * fix(desktop): persist v2 sidebar base branch across creation and display The v2 right sidebar always rendered the repo default branch because the BaseBranchSelector was hardcoded to display defaultBranchName, and ensureSidebarWorkspaceRecord never seeded sidebarState.baseBranch from the pending workspace's selection. Thread the persisted value into the selector and seed it at creation time from pending.baseBranch. * refactor(desktop): derive v2 base branch from git config instead of storing Replace sidebarState.baseBranch in v2WorkspaceLocalState with a host-service git.getBaseBranch query backed by git config (branch.<name>.base, the same key v1 uses). Writes go through a new git.setBaseBranch mutation that invalidates the dependent queries. Removes the duplicate store (and the seed-at-creation workaround) so the v2 sidebar reflects whatever git has, which is already per-branch and persists across worktree recreates. React Query's cache is the only cache we need — base branch changes rarely enough that an extra roundtrip on focus is imperceptible. * fix(host-service): persist v2 fork base branch to git config The v2 `create` mutation used the selected base branch only as the `git worktree add` start point and never wrote it to `branch.<newBranch>.base`. Combined with the v2 sidebar deriving the base branch from that config, a freshly forked workspace's Changes tab always compared against the repo default — the user's fork-time selection was effectively dropped the moment creation finished. Record `startPoint.shortName` into the config right after the worktree is created. Skipped for "head" start points where no meaningful base exists. * Lint * fix(host-service): enable push.autoSetupRemote on v2 worktree creation V2's create and checkout mutations produced worktrees without push.autoSetupRemote set, so the first terminal `git push` failed with "no upstream branch" unless the user had the config globally. V1's equivalent path already writes this per-worktree; match that. Safe against wrong upstream targeting: --no-track (create) guarantees no upstream at first push, so auto-create always wins and always sets <remote>/<branch-name>, never the base branch. Checkout from a remote branch already has upstream via --track, so the config is a no-op there. Also rewrite the --no-track comment at the create site — it had incorrectly tied --no-track to push targeting, when --no-track is actually about suppressing `git pull` / ahead-behind counts against the start point; push targeting is governed independently by push.autoSetupRemote. * fix(desktop): invalidate getBaseBranch on git:changed and clarify config scope Address PR review: 1. P1 (CodeRabbit / cubic): getBaseBranch was cached with staleTime: Infinity but never invalidated on git:changed. An external branch switch (terminal `git checkout`) would leave the sidebar comparing against the previous branch's base. Add invalidation alongside getStatus in useGitStatus — since useGitStatus is always mounted while the Changes UI is visible, this refreshes the shared cache for all consumers (useChangesTab, useSidebarDiffRef). 2. P2 (cubic): my earlier comment claimed push.autoSetupRemote was set "per-worktree" but `git config --local` in a linked worktree writes to the shared repo config, not a worktree-specific one. Rewrite the comments to note the repo-wide scope is intentional — every superset-managed worktree wants the same ergonomics.
…-sh#3488) Pins recently-opened files at the top of the v2 workspace command palette, with currently-open tabs preferred. List is persisted per-workspace via the existing v2WorkspaceLocalState localStorage collection (cap 25 stored, 10 displayed). Empty query shows only the section; non-empty query filters recents by substring match and dedupes them out of the fuzzy results. Extracts the duplicated row markup into a shared FileResultItem component.
…ache (superset-sh#3495) * fix(desktop): bump electron-updater and clear cache on errors Bumps electron-updater from 6.7.3 to 6.8.3 and wires a defensive cache clear into the error handler. Addresses reports of users stuck on an old version until they manually reinstall: electron-updater's internal cache only self-invalidates on remote sha512 mismatch, so a silently corrupt cached download (failed install, signature error, Squirrel ShipIt failure) would be retried indefinitely. Non-network errors now call DownloadedUpdateHelper.clear() so the next 4-hour check re-downloads from scratch. * chore(desktop): tighten auto-updater cache recovery
…-sh#3480) * fix(desktop): remove viewed checkboxes from v2 diff sidebar The sidebar file list no longer shows checkboxes for marking files as viewed. Removes the Checkbox component, viewed/onSetViewed props, and the partitionByViewed utility from the changes sidebar pipeline. * feat(desktop): add icons to v2 sidebar tabs and nest Review under Changes Match the v1 sidebar layout: top-level tabs are Changes (GitCompareArrows icon) and Files (FileText icon). Review is now a subtab within Changes alongside Diffs, using the same Tabs/TabsTrigger pattern as v1. * fix(desktop): use v1 icons and styles for v2 sidebar Use LuGitCompareArrows and LuFile from react-icons/lu (same as v1), getSidebarHeaderTabButtonClassName for the top-level tab buttons, and sidebarHeaderTabTriggerClassName for the Diffs/Review subtabs — matching the v1 sidebar layout and styling exactly. * fix(desktop): center empty review tab content vertically Make the tab content wrapper a flex column so TabsContent can fill the available height, allowing the "Open a pull request" message to center vertically instead of sticking to the top. * feat(desktop): compact icon-only tabs when v2 sidebar is narrow When the sidebar width drops below 250px, the Changes and Files tabs collapse to icon-only buttons with tooltips, matching v1 behavior. Uses the same getSidebarHeaderTabButtonClassName compact mode. * fix(desktop): lower compact sidebar breakpoint to 200px * feat(desktop): persist v2 sidebar active tab and subtab across sessions Store activeTab (Changes/Files) and changesSubtab (Diffs/Review) in the workspace local state collection so the sidebar reopens to the same tab the user last had open. * fix(desktop): code review fixes for v2 sidebar - Remove duplicate React key on compact tab button (SidebarHeader) - Stabilize setActiveTab/setChangesSubtab callbacks via refs so they don't bust the combinedChangesTab useMemo on every collection update - Narrow activeTab schema from z.string() to z.enum(["changes","files"]) with a runtime guard in the setter * refactor(desktop): remove unnecessary useMemo/useCallback/refs from WorkspaceSidebar No memo boundaries downstream benefit from stable references, so plain functions and plain objects are simpler and equivalent.
# Conflicts: # apps/desktop/package.json # apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsx # apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx # apps/desktop/src/renderer/screens/main/components/CommandPalette/CommandPalette.tsx # bun.lock # packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 55 minutes and 38 seconds. ⌛ 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 selected for processing (5)
📝 WalkthroughWalkthroughelectron-updaterの依存関係を更新し、自動更新エラーハンドリング時のキャッシュ削除ロジックを追加。サイドバータブ状態とベースブランチをTRPCクエリと永続化されたローカル状態に移行。ファイル表示追跡機能を削除し、最近表示したファイル追跡機能を実装。コマンドパレットに最近使用したファイル表示機能を追加。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e5eab73c0
ℹ️ 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".
upstream (superset-sh#3488) adds Recently Viewed to the CommandPrimitive-based palette, but fork ships a SearchDialog-based rewrite. Wire upstream's useRecentlyViewedFiles hook and FileResultItem into the fork's dialog so the feature actually works instead of being dead-imported. SearchDialog gets two new optional props: - preResultsSection: ReactNode rendered inside CommandList before results (for the "Recently Viewed" group + separator) - hasPreResults: suppresses the empty state when the recent section has matches but the main results are empty CommandPalette now computes orderedRecent / filteredRecent / dedupedResults (same logic as upstream) and wires them through to SearchDialog. Only active in workspace scope — global scope shows only the flat cross-workspace result list. v2 page passes recentFiles and openFilePaths (already being computed for upstream's abandoned integration) into the palette. Also rewrite FileResultItem to use the fork's wrapped CommandItem instead of raw CommandPrimitive.Item — fork's @superset/ui/command doesn't re-export CommandPrimitive. SidebarHeader non-compact path now reuses the shared `btn` variable via Fragment instead of duplicating the button element.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx (1)
128-130:Tabsの値をasでそのまま永続化しない方が安全です
onValueChangeはstringを返すので、ここだけactiveTabと違って実行時チェックがありません。保存前に"diffs"/"review"を明示判定してください。🛡️ 修正案
<Tabs value={changesSubtab} - onValueChange={(v) => setChangesSubtab(v as "diffs" | "review")} + onValueChange={(v) => { + if (v === "diffs" || v === "review") { + setChangesSubtab(v); + } + }} className="flex min-h-0 flex-1 flex-col gap-0" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx around lines 128 - 130, The current onValueChange handler passes the raw string cast with "as" into setChangesSubtab (Tabs -> onValueChange -> setChangesSubtab), which is unsafe because onValueChange yields a generic string; instead, validate the incoming value explicitly before persisting: check whether the value equals "diffs" or "review" and only then call setChangesSubtab with that literal, otherwise ignore or set a safe default; update the handler where Tabs value and setChangesSubtab are used to perform this runtime guard.apps/desktop/src/main/lib/auto-updater.ts (1)
14-27: private API の shape チェックを実行時に強化してください。
downloadedUpdateHelperの形状は type casting だけでは保証されず、electron-updaterの内部実装変更によってclearメソッドが消失または型が変わる可能性があります。現在のコードは null チェックのみで、呼び出し時にclearが実際に function であることを確認していないため、TypeScript の型安全性の次層である runtime guard を追加することで、将来のライブラリ更新に対する耐性が向上します。
if (!helper || typeof helper.clear !== "function") return;で呼び出し前に shape を確認するか、interface の properties を optional にして同様に guard するアプローチが推奨されます。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/auto-updater.ts` around lines 14 - 27, The runtime shape of the private downloadedUpdateHelper isn't verified — update clearCachedUpdate to guard before calling helper.clear: in the clearCachedUpdate function (and the AppUpdaterInternals usage) check that the resolved helper is non-null and that helper.clear is a function (e.g., if (!helper || typeof helper.clear !== "function") return;) so you only await helper.clear() when it actually exists and is callable; adjust the AppUpdaterInternals/interface usage as needed to mark downloadedUpdateHelper optional and rely on the runtime guard before invoking helper.clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/package.json`:
- Line 174: Change the "electron-updater" dependency from a caret range to an
exact version in package.json (replace ^6.8.3 with 6.8.3) because your
auto-updater code touches the private API downloadedUpdateHelper; after updating
the dependency spec, regenerate the lockfile (run install) so the lockfile pins
the exact electron-updater release to avoid accidental private-API breaks.
In `@apps/desktop/src/main/lib/auto-updater.ts`:
- Line 428: clearCachedUpdate is being called too broadly on any non-network
error; restrict cache clearing to when a download is actually in progress by
checking the updater phase/state first. Update the call site that currently
invokes clearCachedUpdate(`error: ${error?.message ?? "unknown"}`) so it only
runs when the updater's phase/state equals DOWNLOADING (or the equivalent
variable/enum used in this module, e.g. updatePhase, currentState, or
UpdateState.DOWNLOADING); otherwise skip clearing the cached update so
CHECKING/FEED validation errors don't discard a valid downloaded update.
In `@apps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.ts`:
- Around line 19-27: getStatus is running before getBaseBranch resolves because
baseBranchQuery.data is initially undefined; update the
workspaceTrpc.git.getStatus.useQuery call to only enable when the base-branch
query has completed. Change the enabled flag from Boolean(workspaceId) to
Boolean(workspaceId) && baseBranchQuery.isFetched (or
baseBranchQuery.isSuccess), so getStatus (workspaceTrpc.git.getStatus.useQuery)
waits for baseBranchQuery (workspaceTrpc.git.getBaseBranch.useQuery) to resolve
before executing.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/SidebarHeader/SidebarHeader.tsx:
- Around line 27-37: The compact-mode tab button loses its visible label so it
needs an accessible name: update the <button> in SidebarHeader to include
aria-label={tab.label} (or aria-label={tab.label ?? 'tab'} if you want a
fallback) so screen readers can announce the control; keep the existing
onClick={onTabChange(tab.id)}, className from
getSidebarHeaderTabButtonClassName, and icon rendering unchanged and ensure this
uses the same tab.label property referenced where tab.id, tab.icon, compact are
used.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx:
- Around line 52-59: The setBaseBranchMutation currently only handles onSuccess
and is silent on failure; add an onError handler to setBaseBranchMutation (and
the similar mutation around lines 61-65) that displays a user-facing toast/error
notification containing the error message so users know the change failed;
locate setBaseBranchMutation and the other branch-changing mutation and call the
app's existing toast utility (e.g., toast or showToast) inside onError with a
clear message and the error details, and keep invalidation in onSuccess as-is.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx:
- Around line 63-80: The component currently reads v2WorkspaceLocalState via
collections.v2WorkspaceLocalState.get(workspaceId), which is a non-reactive
snapshot so updates via collections.v2WorkspaceLocalState.update(...) (used in
setActiveTab and setChangesSubtab) won't re-render the component; replace the
snapshot read with the reactive hook (useLiveQuery) from the collection so
localState updates cause re-renders — e.g., switch the localState initialization
to use collections.v2WorkspaceLocalState.useLiveQuery(workspaceId) (and adjust
the subsequent null/undefined checks in setActiveTab and setChangesSubtab to
reference the live localState or guard against it being undefined).
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/useRecentlyViewedFiles/useRecentlyViewedFiles.ts:
- Around line 37-55: The recordView callback currently returns early if
collections.v2WorkspaceLocalState.get(workspaceId) is falsy which drops the
first view; instead ensure the per-workspace state is created/initialized inside
recordView (using collections.v2WorkspaceLocalState.insert(...) or an upsert
pattern) before updating so the first event is recorded; modify recordView (the
callback using collections.v2WorkspaceLocalState.update and RECENT_STORE_LIMIT)
to create or upsert the workspace entry when missing and then perform the same
dedupe/ prepend/ slice logic.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/page.tsx:
- Around line 179-180: The recently-viewed list is only updated by recordView
returned from useRecentlyViewedFiles in the workspace page, but files opened via
Quick Open / openFilePane bypass that call; update the recently-viewed state by
invoking recordView whenever a file is opened through openFilePane (and any
other centralized open handlers), or emit a shared event from openFilePane and
subscribe to it in useRecentlyViewedFiles/page.tsx to call recordView; locate
the useRecentlyViewedFiles hook and the openFilePane function and either add a
call to recordView after successful open or wire an event emission/handler so
all open paths update recents consistently.
In `@packages/host-service/src/trpc/router/git/git.ts`:
- Around line 321-370: 保存している baseBranch がプレーンなブランチ名だと origin/${baseBranch}
に組み立て直されローカル専用ブランチが失われる問題があるので、setBaseBranch と getBaseBranch を修正して baseBranch
を「fullRef（例 refs/heads/xxx や refs/remotes/origin/xxx）」または { kind:
'local'|'remote', name: string } のいずれかの明示的フォーマットで永続化するように変更し、getBaseBranch
は現在の文字列返却をやめてその新フォーマットを返すようにすること（関数名: setBaseBranch, getBaseBranch）。さらに
workspace 作成コード（workspace-creation.ts）での直接書き込み処理も同じフォーマットに統一し、参照側の getStatus /
listCommits / getDiff が読み出した値を単に origin/${baseBranch} と組み直すのではなく、fullRef または
kind/name を基に適切なリモート/ref 解決ロジックを使うように合わせて更新すること。
---
Nitpick comments:
In `@apps/desktop/src/main/lib/auto-updater.ts`:
- Around line 14-27: The runtime shape of the private downloadedUpdateHelper
isn't verified — update clearCachedUpdate to guard before calling helper.clear:
in the clearCachedUpdate function (and the AppUpdaterInternals usage) check that
the resolved helper is non-null and that helper.clear is a function (e.g., if
(!helper || typeof helper.clear !== "function") return;) so you only await
helper.clear() when it actually exists and is callable; adjust the
AppUpdaterInternals/interface usage as needed to mark downloadedUpdateHelper
optional and rely on the runtime guard before invoking helper.clear.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx:
- Around line 128-130: The current onValueChange handler passes the raw string
cast with "as" into setChangesSubtab (Tabs -> onValueChange ->
setChangesSubtab), which is unsafe because onValueChange yields a generic
string; instead, validate the incoming value explicitly before persisting: check
whether the value equals "diffs" or "review" and only then call setChangesSubtab
with that literal, otherwise ignore or set a safe default; update the handler
where Tabs value and setChangesSubtab are used to perform this runtime guard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c4014981-f274-43cd-849a-915476018d93
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
apps/desktop/package.jsonapps/desktop/src/main/lib/auto-updater.tsapps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/SidebarHeader/SidebarHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/ChangesFileList.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/FileRow/FileRow.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/utils/partitionByViewed/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/utils/partitionByViewed/partitionByViewed.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesTabContent/ChangesTabContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/types.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useRecentlyViewedFiles/constants.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useRecentlyViewedFiles/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useRecentlyViewedFiles/useRecentlyViewedFiles.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useSidebarDiffRef/useSidebarDiffRef.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/components/WorkspaceNotFoundState/WorkspaceNotFoundState.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/components/WorkspaceNotFoundState/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsxapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.tsapps/desktop/src/renderer/screens/main/components/CommandPalette/CommandPalette.tsxapps/desktop/src/renderer/screens/main/components/CommandPalette/components/FileResultItem/FileResultItem.tsxapps/desktop/src/renderer/screens/main/components/CommandPalette/components/FileResultItem/index.tsapps/desktop/src/renderer/screens/main/components/SearchDialog/SearchDialog.tsxpackages/host-service/src/trpc/router/git/git.tspackages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts
💤 Files with no reviewable changes (2)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/utils/partitionByViewed/partitionByViewed.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/utils/partitionByViewed/index.ts
| "dotenv": "^17.3.1", | ||
| "drizzle-orm": "0.45.1", | ||
| "electron-updater": "^6.7.3", | ||
| "electron-updater": "^6.8.3", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "# package.json の version range"
sed -n '172,175p' apps/desktop/package.json
echo
echo "# repo 内の private electron-updater 依存箇所"
rg -n "downloadedUpdateHelper|clearCachedUpdate" apps/desktop/src/main/lib/auto-updater.ts
echo
echo "# electron-updater 6.8.3 tarball 内に private symbol が存在するか"
TMP="$(mktemp -d)"
trap 'rm -rf "$TMP"' EXIT
curl -Ls https://registry.npmjs.org/electron-updater/-/electron-updater-6.8.3.tgz | tar -xz -C "$TMP"
rg -n "downloadedUpdateHelper" "$TMP/package"Repository: MocA-Love/superset
Length of output: 50375
非公開 internals に依存するなら electron-updater は固定した方が安全です。
apps/desktop/src/main/lib/auto-updater.ts が downloadedUpdateHelper に直接触る前提になったので、ここが ^6.8.3 のままだと lockfile 再生成だけで private API 変更の影響を受けます。少なくともこの依存だけは exact pin にしておく方が事故りにくいです。
🔧 提案差分
- "electron-updater": "^6.8.3",
+ "electron-updater": "6.8.3",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "electron-updater": "^6.8.3", | |
| "electron-updater": "6.8.3", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/package.json` at line 174, Change the "electron-updater"
dependency from a caret range to an exact version in package.json (replace
^6.8.3 with 6.8.3) because your auto-updater code touches the private API
downloadedUpdateHelper; after updating the dependency spec, regenerate the
lockfile (run install) so the lockfile pins the exact electron-updater release
to avoid accidental private-API breaks.
Owner
Author
There was a problem hiding this comment.
保留: この PR は upstream からの取り込みが主目的で、exact pin にすると将来の upstream sync で ^6.8.3 と衝突します。upstream 側で pin すべきか議論したいので、このフォークでは caret を維持します。
There was a problem hiding this comment.
@MocA-Love、ご説明ありがとうございます。upstream sync との互換性を優先してキャレット範囲を維持するという判断、承知しました。この点は今後のレビューで指摘しないよう記録しておきます。
( ´･ω･｀)ﾉ
✏️ Learnings added
Learnt from: MocA-Love
Repo: MocA-Love/superset PR: 204
File: apps/desktop/package.json:174-174
Timestamp: 2026-04-16T09:32:50.995Z
Learning: In MocA-Love/superset (fork of upstream), `electron-updater` in `apps/desktop/package.json` intentionally uses a caret range (e.g. `^6.8.3`) rather than an exact pin. Exact pinning is avoided because this fork closely tracks upstream and a pinned version would conflict during future upstream sync merges. Do not suggest exact pinning for `electron-updater` in this repository unless upstream itself adopts exact pinning.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: MocA-Love/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-14T09:00:06.303Z
Learning: Use published upstream `mastracode` and `mastra/*` packages - do not add fork tarball overrides or custom patch steps unless explicitly requested
| `[auto-updater] Error during update (currentVersion=${app.getVersion()}):`, | ||
| error?.message || error, | ||
| ); | ||
| void clearCachedUpdate(`error: ${error?.message ?? "unknown"}`); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "# currentStatus の更新箇所"
sed -n '97,110p' apps/desktop/src/main/lib/auto-updater.ts
echo
echo "# error handler の現在実装"
sed -n '418,430p' apps/desktop/src/main/lib/auto-updater.tsRepository: MocA-Love/superset
Length of output: 863
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# ファイル全体の行数確認
wc -l apps/desktop/src/main/lib/auto-updater.ts
# clearCachedUpdate の定義を確認
echo "=== clearCachedUpdate 関数 ==="
rg -A 8 "function clearCachedUpdate|const clearCachedUpdate" apps/desktop/src/main/lib/auto-updater.ts
# currentStatus の定義と使用方法
echo ""
echo "=== currentStatus 定義 ==="
rg -B 2 "currentStatus\s*=" apps/desktop/src/main/lib/auto-updater.ts | head -30
# error handler 周辺の full context
echo ""
echo "=== error handler 完全コンテキスト (lines 415-435) ==="
sed -n '415,435p' apps/desktop/src/main/lib/auto-updater.ts
# AUTO_UPDATE_STATUS の定義確認
echo ""
echo "=== AUTO_UPDATE_STATUS 定義 ==="
rg -A 10 "enum AUTO_UPDATE_STATUS|const AUTO_UPDATE_STATUS" apps/desktop/src/main/lib/auto-updater.ts | head -20Repository: MocA-Love/superset
Length of output: 1577
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# AUTO_UPDATE_STATUS 定義を見つける
echo "=== AUTO_UPDATE_STATUS 定義 ==="
sed -n '1,100p' apps/desktop/src/main/lib/auto-updater.ts | grep -A 15 "AUTO_UPDATE_STATUS"
# イベントハンドラ一覧で状態遷移を理解
echo ""
echo "=== autoUpdater イベントハンドラ一覧 ==="
rg "autoUpdater\.on\(" apps/desktop/src/main/lib/auto-updater.ts | head -20
# download-progress, update-downloaded などのイベントハンドラ
echo ""
echo "=== update-downloaded ハンドラ ==="
rg -A 6 'autoUpdater\.on\("update-downloaded"' apps/desktop/src/main/lib/auto-updater.ts
# download-progress ハンドラ
echo ""
echo "=== download-progress ハンドラ ==="
rg -A 6 'autoUpdater\.on\("download-progress"' apps/desktop/src/main/lib/auto-updater.ts
# checking-for-update ハンドラ
echo ""
echo "=== checking-for-update ハンドラ ==="
rg -A 6 'autoUpdater\.on\("checking-for-update"' apps/desktop/src/main/lib/auto-updater.tsRepository: MocA-Love/superset
Length of output: 2178
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# AUTO_UPDATE_STATUS enum in shared package
echo "=== AUTO_UPDATE_STATUS enum definition ==="
find apps -name "auto-update.ts" -o -name "auto-update.tsx" | while read f; do
if [[ -f "$f" ]]; then
echo "File: $f"
cat -n "$f" | head -50
fi
done
# Also check in packages
echo ""
echo "=== shared/auto-update search ==="
fd "auto-update" --type f | grep -E "(shared|packages)" | head -10
# Full file listing to find where it's defined
echo ""
echo "=== searching for AUTO_UPDATE_STATUS enum ==="
rg "enum AUTO_UPDATE_STATUS|export.*AUTO_UPDATE_STATUS" --type ts --type tsxRepository: MocA-Love/superset
Length of output: 724
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check update-available handler to see when DOWNLOADING state is set
echo "=== update-available ハンドラ ==="
rg -A 10 'autoUpdater\.on\("update-available"' apps/desktop/src/main/lib/auto-updater.ts
# Check if there's any explicit DOWNLOADING state setting
echo ""
echo "=== DOWNLOADING state transitions ==="
rg "DOWNLOADING" apps/desktop/src/main/lib/auto-updater.ts
# Check the full sequence of state changes
echo ""
echo "=== 状態遷移フロー ==="
sed -n '420,480p' apps/desktop/src/main/lib/auto-updater.ts
# Check if isNetworkError is defined and what it matches
echo ""
echo "=== isNetworkError 関数 ==="
rg -B 2 -A 10 "function isNetworkError|const isNetworkError" apps/desktop/src/main/lib/auto-updater.tsRepository: MocA-Love/superset
Length of output: 3079
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check SILENT_ERROR_PATTERNS to understand what errors are considered non-network
echo "=== SILENT_ERROR_PATTERNS 定義 ==="
rg -B 5 "SILENT_ERROR_PATTERNS" apps/desktop/src/main/lib/auto-updater.ts
# Look for all places where clearCachedUpdate is called
echo ""
echo "=== clearCachedUpdate の呼び出し全体 ==="
rg "clearCachedUpdate" apps/desktop/src/main/lib/auto-updater.ts -B 2 -A 1
# Check download-progress to see if errors can occur during download
echo ""
echo "=== ダウンロード進行中のエラーハンドリング ==="
sed -n '460,475p' apps/desktop/src/main/lib/auto-updater.ts
# Check if there are any other state-dependent error handling
echo ""
echo "=== state を条件にした処理がないか確認 ==="
rg "currentStatus.*==|currentStatus.*===|switch.*currentStatus" apps/desktop/src/main/lib/auto-updater.tsRepository: MocA-Love/superset
Length of output: 1392
🏁 Script executed:
#!/bin/bash
set -euo typefail
# Get full SILENT_ERROR_PATTERNS list to understand what non-network errors include
echo "=== SILENT_ERROR_PATTERNS 完全リスト ==="
sed -n '1,100p' apps/desktop/src/main/lib/auto-updater.ts | grep -A 20 "SILENT_ERROR_PATTERNS"
# Check if error handler can be triggered during CHECKING phase (before download starts)
echo ""
echo "=== checkForUpdates 関数 ==="
rg -A 20 "async function checkForUpdates|function checkForUpdates" apps/desktop/src/main/lib/auto-updater.tsRepository: MocA-Love/superset
Length of output: 115
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get full SILENT_ERROR_PATTERNS list to understand what non-network errors include
echo "=== SILENT_ERROR_PATTERNS 完全リスト ==="
sed -n '1,100p' apps/desktop/src/main/lib/auto-updater.ts | grep -A 20 "SILENT_ERROR_PATTERNS"
# Check if error handler can be triggered during CHECKING phase (before download starts)
echo ""
echo "=== checkForUpdates 関数 ==="
rg -A 20 "async function checkForUpdates|function checkForUpdates" apps/desktop/src/main/lib/auto-updater.tsRepository: MocA-Love/superset
Length of output: 1820
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check what errors can occur during the CHECKING phase
# Look at the broader context around checkForUpdates and error handling
echo "=== checkForUpdates と error handler の全コンテキスト ==="
sed -n '340,410p' apps/desktop/src/main/lib/auto-updater.ts
# Check if there are metadata/feed validation errors that could occur
echo ""
echo "=== UPDATE_FEED_URL と feed 検証 ==="
rg "UPDATE_FEED_URL|sha512|checksum" apps/desktop/src/main/lib/auto-updater.ts | head -20
# Verify the state flow: when can error be triggered?
echo ""
echo "=== Error が発生しうるフェーズ分析 ==="
# Get lines around line 415-435 to see state when error fires
sed -n '410,440p' apps/desktop/src/main/lib/auto-updater.tsRepository: MocA-Love/superset
Length of output: 3375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if there are any feed parsing or metadata validation errors that could occur
# during CHECKING phase
echo "=== electron-updater の error イベント発生条件 ==="
# Look for comments or documentation about when error is emitted
sed -n '1,50p' apps/desktop/src/main/lib/auto-updater.ts
# Verify: can error occur during CHECKING phase (before DOWNLOADING)?
# If yes, then the concern is valid
echo ""
echo "=== CHECKING フェーズ中のエラー可能性 ==="
# The autoUpdater.on("error") is global and fires for ANY error
# CHECKING phase: autoUpdater.checkForUpdates() could fail on feed validation
# This would trigger error handler while currentStatus === CHECKING
# Final verification: confirm the flow
echo "フロー:"
echo "1. checkForUpdates() -> CHECKING 状態に変更"
echo "2. autoUpdater.checkForUpdates() 実行"
echo "3a. 成功: update-available または update-not-available イベント"
echo "3b. 失敗: error イベント（currentStatus はまだ CHECKING）"
echo ""
echo "現在の実装では 3b の時点で clearCachedUpdate() が呼ばれるため、"
echo "CHECKING フェーズのエラー（feed 検証失敗など）でもキャッシュが削除される"
echo ""
echo "proposed fix では currentStatus === DOWNLOADING の時のみ削除するので、"
echo "CHECKING フェーズのエラーではキャッシュを残す"Repository: MocA-Love/superset
Length of output: 2525
キャッシュ削除の条件が広すぎます。
ここだと「非ネットワーク系エラー」なら全部キャッシュを消すので、CHECKING フェーズの feed 検証エラーなど、キャッシュ破損と無関係なエラーでも、既に落とせていた更新を捨てる可能性があります。clearCachedUpdate() は corrupt 済みファイルのクリアが目的なので、実際にダウンロードが進行中の DOWNLOADING フェーズに限定すべきです。
🔧 提案差分
+function shouldClearCachedUpdate(status: AutoUpdateStatus): boolean {
+ return status === AUTO_UPDATE_STATUS.DOWNLOADING;
+}
+
autoUpdater.on("error", (error) => {
if (isNetworkError(error)) {
console.info("[auto-updater] Network unavailable, will retry later");
emitStatus(AUTO_UPDATE_STATUS.IDLE);
return;
@@
- void clearCachedUpdate(`error: ${error?.message ?? "unknown"}`);
+ if (shouldClearCachedUpdate(currentStatus)) {
+ void clearCachedUpdate(`error: ${error?.message ?? "unknown"}`);
+ }
emitStatus(AUTO_UPDATE_STATUS.ERROR, undefined, error.message);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| void clearCachedUpdate(`error: ${error?.message ?? "unknown"}`); | |
| function shouldClearCachedUpdate(status: AutoUpdateStatus): boolean { | |
| return status === AUTO_UPDATE_STATUS.DOWNLOADING; | |
| } | |
| autoUpdater.on("error", (error) => { | |
| if (isNetworkError(error)) { | |
| console.info("[auto-updater] Network unavailable, will retry later"); | |
| emitStatus(AUTO_UPDATE_STATUS.IDLE); | |
| return; | |
| } | |
| if (shouldClearCachedUpdate(currentStatus)) { | |
| void clearCachedUpdate(`error: ${error?.message ?? "unknown"}`); | |
| } | |
| emitStatus(AUTO_UPDATE_STATUS.ERROR, undefined, error.message); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/lib/auto-updater.ts` at line 428, clearCachedUpdate is
being called too broadly on any non-network error; restrict cache clearing to
when a download is actually in progress by checking the updater phase/state
first. Update the call site that currently invokes clearCachedUpdate(`error:
${error?.message ?? "unknown"}`) so it only runs when the updater's phase/state
equals DOWNLOADING (or the equivalent variable/enum used in this module, e.g.
updatePhase, currentState, or UpdateState.DOWNLOADING); otherwise skip clearing
the cached update so CHECKING/FEED validation errors don't discard a valid
downloaded update.
Owner
Author
There was a problem hiding this comment.
保留: upstream 共通の挙動なので、この PR ではそのまま取り込みます。upstream 側で修正されたら次回同期で取り込む方針。
There was a problem hiding this comment.
@MocA-Love 了解しました。upstream 共通の挙動として本 PR はそのまま取り込み、upstream 側で修正が入った際に次回同期で対応する方針ですね。追跡のため、必要であれば後でフォローアップ Issue を作成することもできますのでお知らせください。
🧠 Learnings used
Learnt from: CR
Repo: MocA-Love/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-14T09:00:06.303Z
Learning: Use published upstream `mastracode` and `mastra/*` packages - do not add fork tarball overrides or custom patch steps unless explicitly requested
Comment on lines
+19
to
27
| const baseBranchQuery = workspaceTrpc.git.getBaseBranch.useQuery( | ||
| { workspaceId }, | ||
| { staleTime: Number.POSITIVE_INFINITY, enabled: Boolean(workspaceId) }, | ||
| ); | ||
| const baseBranch = baseBranchQuery.data?.baseBranch ?? null; | ||
|
|
||
| const query = workspaceTrpc.git.getStatus.useQuery( | ||
| { workspaceId, baseBranch: baseBranch ?? undefined }, | ||
| { refetchOnWindowFocus: true, enabled: Boolean(workspaceId) }, |
There was a problem hiding this comment.
保存済み base branch の取得前に getStatus が既定ブランチで走ります。
初回レンダーでは baseBranchQuery.data がまだ無いので、このクエリは一度 undefined で実行されます。保存済み base branch が既定と異なる workspace では、Changes 一覧が既定ブランチ基準 → 保存済み base branch 基準に一瞬ジャンプします。getBaseBranch が一度解決するまでは getStatus を待たせた方が安定します。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.ts`
around lines 19 - 27, getStatus is running before getBaseBranch resolves because
baseBranchQuery.data is initially undefined; update the
workspaceTrpc.git.getStatus.useQuery call to only enable when the base-branch
query has completed. Change the enabled flag from Boolean(workspaceId) to
Boolean(workspaceId) && baseBranchQuery.isFetched (or
baseBranchQuery.isSuccess), so getStatus (workspaceTrpc.git.getStatus.useQuery)
waits for baseBranchQuery (workspaceTrpc.git.getBaseBranch.useQuery) to resolve
before executing.
Owner
Author
There was a problem hiding this comment.
保留: upstream 共通の挙動で、軽微な UX ちらつき。upstream 側の判断を待つ方針です。
There was a problem hiding this comment.
@MocA-Love、了解しました。upstream 共通の挙動であり、軽微な UX ちらつきとのことですので、upstream 側の対応を待つ方針を尊重します。必要であれば後続 PR でフォローアップしてください。
Comment on lines
+37
to
+55
| const recordView = useCallback( | ||
| (file: RecentFileInput) => { | ||
| if (!collections.v2WorkspaceLocalState.get(workspaceId)) return; | ||
| collections.v2WorkspaceLocalState.update(workspaceId, (draft) => { | ||
| const current = draft.recentlyViewedFiles ?? []; | ||
| const withoutDup = current.filter( | ||
| (f) => f.relativePath !== file.relativePath, | ||
| ); | ||
| draft.recentlyViewedFiles = [ | ||
| { | ||
| relativePath: file.relativePath, | ||
| absolutePath: file.absolutePath, | ||
| lastAccessedAt: Date.now(), | ||
| }, | ||
| ...withoutDup, | ||
| ].slice(0, RECENT_STORE_LIMIT); | ||
| }); | ||
| }, | ||
| [collections, workspaceId], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# `v2WorkspaceLocalState` が、ファイル閲覧より前に必ず作成されるかを確認します。
# 期待結果: workspace bootstrap/create path のどこかで insert/upsert が見つかること。
rg -n -C3 'v2WorkspaceLocalState\.(get|insert|upsert|update)\(' apps/desktop/src packagesRepository: MocA-Love/superset
Length of output: 26354
🏁 Script executed:
# Find where useRecentlyViewedFiles is imported/used
rg -n 'useRecentlyViewedFiles' apps/desktop/src --type ts --type tsxRepository: MocA-Love/superset
Length of output: 89
🏁 Script executed:
# Check where recordView is called (if accessible from the hook export)
rg -n 'recordView' apps/desktop/src --type ts --type tsx -A 2 -B 2Repository: MocA-Love/superset
Length of output: 89
🏁 Script executed:
# Verify ensureWorkspaceInSidebar is defined and called at the right time
rg -n 'ensureWorkspaceInSidebar' apps/desktop/src --type ts --type tsx -B 3 -A 3Repository: MocA-Love/superset
Length of output: 89
🏁 Script executed:
rg -n 'useRecentlyViewedFiles' apps/desktop/src -A 2 -B 2Repository: MocA-Love/superset
Length of output: 3621
🏁 Script executed:
rg -n 'recordView' apps/desktop/src -A 2 -B 2Repository: MocA-Love/superset
Length of output: 4377
🏁 Script executed:
# Find the file and check its full context around recordView
cat apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/\$workspaceId/hooks/useRecentlyViewedFiles/useRecentlyViewedFiles.tsRepository: MocA-Love/superset
Length of output: 1722
🏁 Script executed:
# Search for ensureWorkspaceInSidebar definition and all calls
rg -n 'ensureWorkspaceInSidebar' apps/desktop/src -B 3 -A 3Repository: MocA-Love/superset
Length of output: 14505
🏁 Script executed:
# Check the ensureSidebarWorkspaceRecord implementation
rg -n 'ensureSidebarWorkspaceRecord' apps/desktop/src -B 5 -A 15Repository: MocA-Love/superset
Length of output: 5653
🏁 Script executed:
# Check the layout.tsx to understand initialization order
head -n 100 apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsxRepository: MocA-Love/superset
Length of output: 2968
🏁 Script executed:
# Check if there are any upsert patterns used elsewhere for reference
rg -n '\.upsert' apps/desktop/src -B 2 -A 2 | head -50Repository: MocA-Love/superset
Length of output: 6901
recordView 内で状態の初期化を保証する必要があります
Line 39 の早期リターンにより、v2WorkspaceLocalState がまだ初期化されていないタイミングで recordView が呼び出された場合、最初のファイル閲覧イベントが無言で破棄されます。親コンポーネント (layout.tsx) の初期化に依存する現在の設計は脆弱です。
代わりに、recordView 内で以下のパターンを使用して状態を確保してください:
- 状態が存在しない場合は
.insert()で初期化、または .upsert()パターンを使用 (コードベースの他の部分で使用例あり:upsertDocument,browserHistory.upsert)
これにより、初期化順序への暗黙的な依存を排除し、データ損失を防げます。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/useRecentlyViewedFiles/useRecentlyViewedFiles.ts
around lines 37 - 55, The recordView callback currently returns early if
collections.v2WorkspaceLocalState.get(workspaceId) is falsy which drops the
first view; instead ensure the per-workspace state is created/initialized inside
recordView (using collections.v2WorkspaceLocalState.insert(...) or an upsert
pattern) before updating so the first event is recorded; modify recordView (the
callback using collections.v2WorkspaceLocalState.update and RECENT_STORE_LIMIT)
to create or upsert the workspace entry when missing and then perform the same
dedupe/ prepend/ slice logic.
Owner
Author
There was a problem hiding this comment.
保留: upsert パターンを使うには projectId が必要で、このフックのシグネチャを変更する必要があります (upstream API 変更)。実際には ensureWorkspaceInSidebar が workspace 表示前に呼ばれるため初回 drop はほぼ発生しない想定。根本修正は upstream 側で対応依頼したい。
There was a problem hiding this comment.
@MocA-Love、制約のご説明ありがとうございます。upsert には projectId が必要でシグネチャ変更を伴うという点、また ensureWorkspaceInSidebar が事前に呼ばれるため実用上の影響が小さい点、理解しました。
upstream 側での根本対応が適切と判断されたこと、了解です。もし後続タスクとして GitHub Issue を作成して追跡したい場合はお声がけください。
ᓚᘏᗢ
Comment on lines
+321
to
+370
| getBaseBranch: protectedProcedure | ||
| .input(z.object({ workspaceId: z.string() })) | ||
| .query(async ({ ctx, input }) => { | ||
| const worktreePath = resolveWorktreePath(ctx, input.workspaceId); | ||
| const git = await ctx.git(worktreePath); | ||
| const currentBranch = ( | ||
| await git.revparse(["--abbrev-ref", "HEAD"]).catch(() => "") | ||
| ).trim(); | ||
| if (!currentBranch || currentBranch === "HEAD") { | ||
| return { baseBranch: null as string | null }; | ||
| } | ||
| const configured = ( | ||
| await git | ||
| .raw(["config", `branch.${currentBranch}.base`]) | ||
| .catch(() => "") | ||
| ).trim(); | ||
| return { baseBranch: (configured || null) as string | null }; | ||
| }), | ||
|
|
||
| setBaseBranch: protectedProcedure | ||
| .input( | ||
| z.object({ | ||
| workspaceId: z.string(), | ||
| baseBranch: z.string().nullable(), | ||
| }), | ||
| ) | ||
| .mutation(async ({ ctx, input }) => { | ||
| const worktreePath = resolveWorktreePath(ctx, input.workspaceId); | ||
| const git = await ctx.git(worktreePath); | ||
| const currentBranch = ( | ||
| await git.revparse(["--abbrev-ref", "HEAD"]).catch(() => "") | ||
| ).trim(); | ||
| if (!currentBranch || currentBranch === "HEAD") { | ||
| throw new TRPCError({ | ||
| code: "PRECONDITION_FAILED", | ||
| message: "Cannot set base branch on detached HEAD", | ||
| }); | ||
| } | ||
| if (input.baseBranch) { | ||
| await git.raw([ | ||
| "config", | ||
| `branch.${currentBranch}.base`, | ||
| input.baseBranch, | ||
| ]); | ||
| } else { | ||
| await git | ||
| .raw(["config", "--unset", `branch.${currentBranch}.base`]) | ||
| .catch(() => {}); | ||
| } | ||
| return { baseBranch: input.baseBranch }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n# Writers of branch.<name>.base\n'
sed -n '694,706p' packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts
printf '\n'
sed -n '359,370p' packages/host-service/src/trpc/router/git/git.ts
printf '\n# Readers that always rebuild compare refs as origin/<baseBranch>\n'
sed -n '170,175p' packages/host-service/src/trpc/router/git/git.ts
printf '\n'
sed -n '274,279p' packages/host-service/src/trpc/router/git/git.ts
printf '\n'
sed -n '426,430p' packages/host-service/src/trpc/router/git/git.tsRepository: MocA-Love/superset
Length of output: 1585
baseBranch を文字列だけで永続化すると、ローカル基点を復元できません。
setBaseBranch (およびワークスペース作成時) でプレーンな branch 名を保存していますが、getStatus / listCommits / getDiff では読み込んだ値を常に origin/${baseBranch} に組み立て直しています。ローカル専用ブランチを base に選んだ場合、保存後の比較先は存在しない origin/<name> に化けて Changes 系クエリが失敗します。
baseBranch は fullRef または { kind, name } 形式で保存し、workspace-creation.ts での直接書き込みも同じフォーマットに統一する必要があります。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/host-service/src/trpc/router/git/git.ts` around lines 321 - 370,
保存している baseBranch がプレーンなブランチ名だと origin/${baseBranch}
に組み立て直されローカル専用ブランチが失われる問題があるので、setBaseBranch と getBaseBranch を修正して baseBranch
を「fullRef（例 refs/heads/xxx や refs/remotes/origin/xxx）」または { kind:
'local'|'remote', name: string } のいずれかの明示的フォーマットで永続化するように変更し、getBaseBranch
は現在の文字列返却をやめてその新フォーマットを返すようにすること（関数名: setBaseBranch, getBaseBranch）。さらに
workspace 作成コード（workspace-creation.ts）での直接書き込み処理も同じフォーマットに統一し、参照側の getStatus /
listCommits / getDiff が読み出した値を単に origin/${baseBranch} と組み直すのではなく、fullRef または
kind/name を基に適切なリモート/ref 解決ロジックを使うように合わせて更新すること。
Owner
Author
There was a problem hiding this comment.
保留: 根本的な format 再設計 (fullRef / {kind, name}) はスコープが大きいため follow-up PR で対応します。この PR では workspace-creation.ts 側で local-only branch の config 書き込みをスキップすることで P1 の実害を回避しました。
There was a problem hiding this comment.
@MocA-Love ご説明ありがとうございます。暫定対応の方針（ローカル専用ブランチの場合は branch.<name>.base への書き込みをスキップ）は理にかなっています。getStatus / listCommits / getDiff が origin/${baseBranch} で解決を試みる現状では、ローカル専用ブランチの設定が書き込まれないことで P1 の実害を確実に回避できます。
フォーマット再設計（fullRef / { kind, name }）の follow-up PR 用に Issue を作成しましょうか？
✏️ Learnings added
Learnt from: MocA-Love
Repo: MocA-Love/superset PR: 204
File: packages/host-service/src/trpc/router/git/git.ts:321-370
Timestamp: 2026-04-16T09:32:59.998Z
Learning: In the MocA-Love/superset fork, `getBaseBranch`/`setBaseBranch` in `packages/host-service/src/trpc/router/git/git.ts` store baseBranch as a plain branch name string. Consumers (`getStatus`, `listCommits`, `getDiff`) always reconstruct the ref as `origin/${baseBranch}`. Local-only branches are intentionally excluded from `branch.<name>.base` config writes (in `workspace-creation.ts`) as an interim mitigation. A full redesign to store `fullRef` or `{ kind: 'local'|'remote', name: string }` is deferred to a follow-up PR.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
- SidebarHeader: add aria-label to compact tab buttons (a11y, CodeRabbit)
- WorkspaceSidebar: use useLiveQuery instead of non-reactive collection.get()
so tab switches re-render immediately (CodeRabbit — confirmed by TanStack
docs: get() does not subscribe)
- v2 page: call recordView from openFilePane too, not only openSidebarFilePane
so Cmd+P / Quick Open / other centralized open paths update recents
(CodeRabbit)
- useChangesTab: surface setBaseBranch mutation failures via toast instead
of silently reverting the selection (CodeRabbit)
- workspace-creation: only write branch.<name>.base for remote-tracking
start points. Downstream (getStatus/listCommits/getDiff) rebuilds the
compare ref as `origin/${baseBranch}`, so a local-only branch name
would resolve to a non-existent `origin/<local-name>` and break the
Changes tab (Codex P1)
Deferred to follow-up:
- setBaseBranch plain-string persistence (full fullRef/{kind,name} format
refactor, CodeRabbit git.ts:370)
- useRecentlyViewedFiles.ts first-view drop edge case (upsert pattern
needs projectId redesign, CodeRabbit)
- auto-updater clearCachedUpdate scope narrowing (upstream common)
- useGitStatus enabled gating on baseBranchQuery (upstream common)
- electron-updater exact pin (would drift from upstream)
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
This was referenced Apr 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
upstream/main から 5 コミットを取り込み。すべて desktop アプリ (v2 workspace) の変更。
取り込みコミット
ec45c78b3c2e96c37886e0170a7e4c1365f81b7cb9c96コンフリクト解消
package.jsonworkspace-creation.tsfullRef維持 + upstream のautoSetupRemote/baseブロック追加pending/$pendingId/page.tsxv2-workspace/$workspaceId/page.tsxCommandPalette.tsxbun.lockbun installで再生成フォーク適応
workspace-creation.ts: FORK NOTE 維持 —fullRef使用でリモートブランチ名の曖昧さを回避pending page: フォーク独自のsyncTimedOutリセット useEffect を upstream のprevPendingIdRefパターンに置換CommandPalette: フォーク側の SearchDialog アーキテクチャを維持。upstream の Recently Viewed UI は旧 CommandPrimitive ベースのため未統合（別タスクで対応予定）注意事項
useRecentlyViewedFileshook とFileResultItemコンポーネントは取り込み済みだが、SearchDialog への UI 統合は別タスクテストチェックリスト
Summary by CodeRabbit
New Features
Bug Fixes
Improvements