fix(desktop): Databaseサイドバーをワークスペースごとにスコープ化#76
Merged
Conversation
- YAMLパースでworkflow_dispatch inputsを抽出（choice/boolean/string/number対応） - dispatch APIにinputsパラメータを送信 - WorkflowRunCardにView logsボタンを追加し既存ActionLogsPaneでログ表示 - Configure/Runボタンを横並びレイアウトに改善
ActionLogsPaneStateにrunIdを追加し、workflow dispatchから開いた場合は getWorkflowRunJobsを3秒ポーリングしてjobリストを動的に更新する。 途中でjobが増えるワークフローでも再オープン不要になる。
DB接続・アクティブ接続・クエリ履歴がグローバル共有されていた問題を修正。 ストアをRecord<workspaceId, State>構造に再設計し、v5→v6マイグレーションで 既存データを_unassignedキーに移行。 Closes #74
📝 WalkthroughWalkthroughGitHub Actions ワークフローの YAML 解析で Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as RepositoryPanel UI
participant tRPC as tRPC Client
participant Backend as Git Status Router
participant GitHub as GitHub API
participant Store as Tabs Store
User->>UI: Click "Run" / provide inputs / Click "View logs"
UI->>tRPC: dispatchGitHubWorkflow(workspaceId, workflowId, ref?, inputs?)
tRPC->>Backend: dispatch request (form inputs)
Backend->>GitHub: gh api .../dispatches
GitHub-->>Backend: 204 accepted
Backend-->>tRPC: dispatch response
alt View logs / open run
UI->>tRPC: getWorkflowRunJobs.fetch(workspaceId, runId)
tRPC->>Backend: request run jobs
Backend->>GitHub: gh api .../runs/{runId}/jobs
GitHub-->>Backend: jobs payload
Backend-->>tRPC: parsed jobs
tRPC-->>UI: jobs
UI->>Store: addActionLogsTab(workspaceId, jobs, failedIdx?, runId)
end
sequenceDiagram
actor ActionLogs as ActionLogsPane
participant tRPC as tRPC Client
participant Backend as Git Status Router
participant GitHub as GitHub API
ActionLogs->>tRPC: getWorkflowRunJobs (enabled when runId)
loop every 3s while any job.status == "pending"
tRPC->>Backend: fetch run jobs
Backend->>GitHub: gh api .../runs/{runId}/jobs
GitHub-->>Backend: updated jobs
Backend-->>tRPC: parsed jobs
tRPC-->>ActionLogs: updated jobs
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
配列を返すセレクタが毎回新しい参照を生成し、zustandの等値チェックが 常にfalseとなりMaximum update depth exceededが発生していた。 shallow comparatorと安定した空配列参照で修正。
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
apps/desktop/src/renderer/stores/database-sidebar.ts (1)
151-156:getWorkspaceヘルパーで毎回新しいオブジェクト参照を返す可能性があります。
emptyWorkspaceStateをそのまま返すと、Zustand のセレクタ最適化において毎回同じ参照が返されるため問題ありませんが、将来的にこのヘルパーが拡張される場合は注意が必要です。現状は問題ありません。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/stores/database-sidebar.ts` around lines 151 - 156, getWorkspace currently returns state.workspaces[workspaceId] ?? emptyWorkspaceState; ensure you keep returning the shared emptyWorkspaceState constant (do not create a new object or clone) so selectors keep stable references; if you later need to extend or mutate the returned workspace, change call sites to create a copy only when modifying (or explicitly clone inside a new function) to preserve selector optimization for getWorkspace and avoid returning a fresh reference each call.
🤖 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/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/RepositoryPanel/RepositoryPanel.tsx`:
- Around line 361-366: workflowInputValues is a single shared Record so inputs
bleed between workflows; change its shape to map by workflow id (e.g.,
useState<Record<number, Record<string,string>>>({})) and update
setWorkflowInputValues usage to read/write under workflow.id, and when
dispatching or calling the Run handler (check usages that reference
expandedWorkflowId and any run/dispatch functions), only send the payload from
workflowInputValues[workflow.id] and guard dispatches with expandedWorkflowId
=== workflow.id to avoid sending another workflow's inputs.
- Around line 591-598: The code currently sends inputs with empty-string values
(inputs -> filteredInputs -> dispatchWorkflowMutation.mutateAsync), which
prevents distinguishing "omitted" vs empty; change the filtering so that before
calling dispatchWorkflowMutation.mutateAsync you construct filteredInputs by
removing any key whose value is exactly "" (and also drop null/undefined), e.g.
transform inputs into an object that only includes entries with non-empty
values, then pass that as inputs; keep existing workflowRef.trim() logic and
ensure required-field validation is performed separately (i.e., do not rely on
this filter to enforce requiredness).
- Around line 629-643: The current logic prevents opening the ActionLogs tab for
runs with no jobs because the jobs.length === 0 check returns early; instead,
allow creating the tab when a runId exists so ActionLogsPane can poll for jobs
itself. Remove or change the early-return branch around getWorkflowRunJobs.fetch
so addActionLogsTab(workspaceId, jobs, failedIdx >= 0 ? failedIdx : undefined,
runId) is called even if jobs is an empty array (you may still show a
non-blocking toast but do not abort creating the tab). This touches
getWorkflowRunJobs.fetch result handling and the call to addActionLogsTab and
relies on ActionLogsPane to handle runId-based polling.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx`:
- Around line 856-862: The addConnection callback currently returns a forced
cast undefined as SavedDatabaseConnection when workspaceId is missing, which is
unsafe; update addConnection (and similarly addQueryHistoryItem) to either throw
an explicit error early when workspaceId is undefined (so callers never receive
an invalid SavedDatabaseConnection) or change the callback signature/return type
to SavedDatabaseConnection | undefined and propagate that type to callers;
locate the functions addConnection and addQueryHistoryItem and the call to
storeAddConnection and adjust the control flow and types accordingly so no
undefined is cast to SavedDatabaseConnection.
---
Nitpick comments:
In `@apps/desktop/src/renderer/stores/database-sidebar.ts`:
- Around line 151-156: getWorkspace currently returns
state.workspaces[workspaceId] ?? emptyWorkspaceState; ensure you keep returning
the shared emptyWorkspaceState constant (do not create a new object or clone) so
selectors keep stable references; if you later need to extend or mutate the
returned workspace, change call sites to create a copy only when modifying (or
explicitly clone inside a new function) to preserve selector optimization for
getWorkspace and avoid returning a fresh reference each call.
🪄 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: 4517d24a-e939-48c0-a5d9-011ef2d66a65
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
apps/desktop/package.jsonapps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ActionLogsPane/ActionLogsPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/DatabaseExplorerPane/DatabaseExplorerPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/RepositoryPanel/RepositoryPanel.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsxapps/desktop/src/renderer/stores/database-sidebar.tsapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/stores/tabs/types.tsapps/desktop/src/renderer/stores/tabs/utils.tsapps/desktop/src/shared/tabs-types.ts
Comment on lines
+361
to
+366
| const [expandedWorkflowId, setExpandedWorkflowId] = useState<number | null>( | ||
| null, | ||
| ); | ||
| const [workflowInputValues, setWorkflowInputValues] = useState< | ||
| Record<string, string> | ||
| >({}); |
There was a problem hiding this comment.
ワークフロー入力が別 workflow 間で混線します。
workflowInputValues が 1 つの共有 Record<string, string> のままなので、A の Conf を開いた状態で B の Run を押すと、B にも A の入力が渡ります。入力値は workflow.id ごとに保持するか、少なくとも expandedWorkflowId === workflow.id のときだけその workflow の値を dispatch しないと、別 workflow に不正な payload を送ります。
Also applies to: 1251-1258
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/RepositoryPanel/RepositoryPanel.tsx`
around lines 361 - 366, workflowInputValues is a single shared Record so inputs
bleed between workflows; change its shape to map by workflow id (e.g.,
useState<Record<number, Record<string,string>>>({})) and update
setWorkflowInputValues usage to read/write under workflow.id, and when
dispatching or calling the Run handler (check usages that reference
expandedWorkflowId and any run/dispatch functions), only send the payload from
workflowInputValues[workflow.id] and guard dispatches with expandedWorkflowId
=== workflow.id to avoid sending another workflow's inputs.
Comment on lines
+591
to
598
| const filteredInputs = | ||
| inputs && Object.keys(inputs).length > 0 ? inputs : undefined; | ||
| const result = await dispatchWorkflowMutation.mutateAsync({ | ||
| workspaceId, | ||
| workflowId, | ||
| ref: workflowRef.trim() || undefined, | ||
| inputs: filteredInputs, | ||
| }); |
There was a problem hiding this comment.
空欄 input をそのまま送ると「未指定」を表現できません。
この分岐は key 数しか見ていないので、設定 UI を一度開いた workflow は未入力の optional field まで "" で送られます。空欄は payload から落として、required だけ別途バリデーションしないと、省略と空文字が区別できません。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/RepositoryPanel/RepositoryPanel.tsx`
around lines 591 - 598, The code currently sends inputs with empty-string values
(inputs -> filteredInputs -> dispatchWorkflowMutation.mutateAsync), which
prevents distinguishing "omitted" vs empty; change the filtering so that before
calling dispatchWorkflowMutation.mutateAsync you construct filteredInputs by
removing any key whose value is exactly "" (and also drop null/undefined), e.g.
transform inputs into an object that only includes entries with non-empty
values, then pass that as inputs; keep existing workflowRef.trim() logic and
ensure required-field validation is performed separately (i.e., do not rely on
this filter to enforce requiredness).
Comment on lines
+629
to
+643
| const jobs = await trpcUtils.workspaces.getWorkflowRunJobs.fetch({ | ||
| workspaceId, | ||
| runId, | ||
| }); | ||
| if (jobs.length === 0) { | ||
| toast.error("No jobs found for this workflow run"); | ||
| return; | ||
| } | ||
| const failedIdx = jobs.findIndex((j) => j.status === "failure"); | ||
| addActionLogsTab( | ||
| workspaceId, | ||
| jobs, | ||
| failedIdx >= 0 ? failedIdx : undefined, | ||
| runId, | ||
| ); |
There was a problem hiding this comment.
ジョブ未生成の run で View logs が失敗します。
ActionLogsPane は runId があれば自前でジョブをポーリングできますが、ここで jobs.length === 0 を弾いているせいで queued 直後の run を開けません。空配列でも runId 付きでタブを作れば、そのままジョブ出現を待てます。
💡 修正例
try {
const jobs = await trpcUtils.workspaces.getWorkflowRunJobs.fetch({
workspaceId,
runId,
});
- if (jobs.length === 0) {
- toast.error("No jobs found for this workflow run");
- return;
- }
const failedIdx = jobs.findIndex((j) => j.status === "failure");
addActionLogsTab(
workspaceId,
jobs,
failedIdx >= 0 ? failedIdx : undefined,
runId,
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/RepositoryPanel/RepositoryPanel.tsx`
around lines 629 - 643, The current logic prevents opening the ActionLogs tab
for runs with no jobs because the jobs.length === 0 check returns early;
instead, allow creating the tab when a runId exists so ActionLogsPane can poll
for jobs itself. Remove or change the early-return branch around
getWorkflowRunJobs.fetch so addActionLogsTab(workspaceId, jobs, failedIdx >= 0 ?
failedIdx : undefined, runId) is called even if jobs is an empty array (you may
still show a non-blocking toast but do not abort creating the tab). This touches
getWorkflowRunJobs.fetch result handling and the call to addActionLogsTab and
relies on ActionLogsPane to handle runId-based polling.
マイグレーション後の_unassigned内の接続を削除・更新できない問題を修正。 removeConnection, updateConnection, removeQueryHistoryItem, clearQueryHistoryForConnectionが_unassignedバケットも操作するように変更。 addConnectionの重複チェックも_unassignedを含めるよう修正。
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx (1)
856-900: エラーハンドリングの一貫性がありません。
addConnectionとaddQueryHistoryItemはworkspaceIdが未定義の場合に例外をスローしますが、他のラッパー関数（updateConnection,removeConnection,removeQueryHistoryItem,clearQueryHistoryForConnection,setActiveConnectionId）はnullまたはundefinedを返して静かに失敗します。例えば、
removeConnection（3001行目で使用）が静かに失敗すると、ユーザーが「削除」ボタンをクリックしても何も起きず、混乱を招く可能性があります。♻️ 一貫したエラーハンドリングの提案
const removeConnection = useCallback( - (id: string) => - workspaceId ? storeRemoveConnection(workspaceId, id) : undefined, + (id: string) => { + if (!workspaceId) { + console.warn("removeConnection called without workspaceId"); + return; + } + return storeRemoveConnection(workspaceId, id); + }, [workspaceId, storeRemoveConnection], );同様のパターンを
updateConnection,removeQueryHistoryItem,clearQueryHistoryForConnection,setActiveConnectionIdにも適用し、少なくとも警告をログに出力することで、デバッグを容易にすることを検討してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx` around lines 856 - 900, 他のラッパー関数（updateConnection, removeConnection, removeQueryHistoryItem, clearQueryHistoryForConnection, setActiveConnectionId）が workspaceId 不在時に静かに null/undefined を返すため一貫性がないので、addConnection / addQueryHistoryItem と同様に workspaceId が無い場合は明示的に例外を投げするか、少なくとも processLogger.warn で警告を出すよう修正してください；該当するラッパー（updateConnection → storeUpdateConnection, removeConnection → storeRemoveConnection, removeQueryHistoryItem → storeRemoveQueryHistoryItem, clearQueryHistoryForConnection → storeClearQueryHistoryForConnection, setActiveConnectionId → storeSetActiveConnectionId）内で workspaceId チェックを追加し、存在しなければ throw new Error("... called without workspaceId") もしくは logger を呼ぶ形に揃えてください。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx`:
- Around line 856-900: 他のラッパー関数（updateConnection, removeConnection,
removeQueryHistoryItem, clearQueryHistoryForConnection, setActiveConnectionId）が
workspaceId 不在時に静かに null/undefined を返すため一貫性がないので、addConnection /
addQueryHistoryItem と同様に workspaceId が無い場合は明示的に例外を投げするか、少なくとも processLogger.warn
で警告を出すよう修正してください；該当するラッパー（updateConnection → storeUpdateConnection,
removeConnection → storeRemoveConnection, removeQueryHistoryItem →
storeRemoveQueryHistoryItem, clearQueryHistoryForConnection →
storeClearQueryHistoryForConnection, setActiveConnectionId →
storeSetActiveConnectionId）内で workspaceId チェックを追加し、存在しなければ throw new Error("...
called without workspaceId") もしくは logger を呼ぶ形に揃えてください。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b54ec52f-bb2d-4d34-b4b1-cba1228719fb
📒 Files selected for processing (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsxapps/desktop/src/renderer/stores/database-sidebar.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/renderer/stores/database-sidebar.ts
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
1 participant
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.
Summary
database-sidebar.tsのストアをフラット構造からRecord<workspaceId, WorkspaceDatabaseState>に再設計し、DB接続・アクティブ接続・クエリ履歴をワークスペース単位で分離workspaceIdを第一引数として追加_unassignedキーに移行し、後方互換性を維持DatabasesView.tsxとDatabaseExplorerPane.tsxを新しいワークスペーススコープAPIに適合Closes #74
Test plan
_unassignedとして全ワークスペースで表示されることを確認Summary by CodeRabbit
リリースノート
New Features
Refactor