Skip to content

feat(desktop): unified Git operation dialog across Changes sidebar#153

Merged
MocA-Love merged 8 commits intomainfrom
feat/git-dialog-unification
Apr 12, 2026
Merged

feat(desktop): unified Git operation dialog across Changes sidebar#153
MocA-Love merged 8 commits intomainfrom
feat/git-dialog-unification

Conversation

@MocA-Love
Copy link
Copy Markdown
Owner

@MocA-Love MocA-Love commented Apr 12, 2026

Summary

Right Sidebar の Changes タブで発生する Git 関連のエラーとユーザー判断を、統一された GitOperationDialog に集約します。これまで raw stderr の toast で流れていたケースをすべて構造化ダイアログ化し、rebase/stash/force unlock/retry など次のアクションを UI クリックだけで完結できるようにします。

なにが変わったか

基盤 (新規)

  • useGitOperationDialogStore (zustand) — アプリ全体で 1 つのダイアログ状態を共有
  • GitOperationDialog — 汎用的な spec ベースの alert dialog コンポーネント
  • classifyGitError(error, context) — git stderr をコンテキスト付きで分類するピュア関数
  • showGitErrorDialog / showGitConfirmDialog / showGitWarningDialog — 呼び出し側ヘルパー

エラー分類 kind

push-rejected / push-protected-branch / push-no-remote-for-pr / pull-conflict / pull-overwrite / pull-upstream-missing / commit-hook-failed / commit-gpg-failed / commit-identity-missing / nothing-to-commit / auth-failed / network-error / no-remote / stash-pop-conflict / nothing-to-stash / pr-not-mergeable / pr-already-done / pr-not-found / index-lock / detached-head / permission-denied / branch-name-collision / branch-behind-upstream / non-git-repo / generic-error

Confirm ダイアログ (破壊的 / 不可逆操作)

  • merge-pr-confirm — PRButton の squash/merge/rebase は全て確認ダイアログ必須
  • bulk-stage-all-confirm / bulk-unstage-all-confirm — 10 件以上で確認
  • pr-draft-state-confirm — open → draft 変換は確認
  • rerun-all-checks-confirm — 全 job 再実行は確認 (failed のみのオプションも提示)
  • workflow-dispatch-confirm — GitHub Actions manual dispatch は確認
  • create-pr-open-existing — 既存 PR があれば確認してから開く
  • discard-unsaved-editor — エディタに未保存があれば bulk discard 前に警告

Silent auto-repair 通知

Backend の push / sync が裏で自動復旧した場合、ユーザーに見える通知を出します。

  • auto-published-upstream — sync 中に upstream が無く publish した
  • post-push-fetch-failed — push は成功したが fetch だけ失敗 (これまで "Push failed" と誤表示)
  • push-retargeted — 既存 PR の head へ自動リターゲット
  • post-checkout-hook-failed — post-checkout フック失敗の警告

Backend 変更

  • push / sync は warnings[] を返すよう変更 (スローではなく)
  • sync の pull/push stage 失敗を GitSyncStageError で区別可能に
  • createPRisExisting: boolean を返す
  • commitskipHooks?: boolean--no-verify 対応
  • 新 procedure forceUnlockIndex.git/index.lock 等の stale lock を削除

既存ダイアログ統一

  • DiscardConfirmDialog とバルク discard の文言を日本語化
  • window.confirm(behind upstream) を GitOperationDialog に置き換え
  • ChangesHeader の switch-branch / create-branch / compare base 変更のエラーパスを統一
  • BranchActionDialog の git-busy のうち、index.lock 検出時は強制解除アクション付きの統一ダイアログに分岐

設計メモ

  • BranchActionDialog は既存のブランチ操作 dirty/conflicted 等に特化した責務のまま残し、拡張せず。新ダイアログは完全に独立させることで god component 化を回避。
  • ダイアログ呼び出しは zustand store 経由で、CommitInput / PRButton / ChangesHeader / ReviewPanel / RepositoryPanel / useCreateOrOpenPR どこからでも同じ store に書き込み、ChangesView 末尾の単一インスタンスがレンダリング。
  • 破壊的アクション (force push, force unlock, discard) は既定ボタンにせず、safe action (retry, stash, rebase) を既定に。

スコープ外 (次回以降)

  • submodule-detected / lfs-missing / case-rename — backend に検知経路が無いため detection 実装後に結線予定
  • shallow-clone — banner 表示の方が適切
  • github-data-partial-or-stale — modal より banner 向き

Test plan

  • 意図的に git push を rejected にさせて push-rejected ダイアログ → pull--rebase 再試行が動くか
  • Commit 時に husky で落として hook-failed ダイアログを確認
  • Merge PR の確認ダイアログでキャンセルできること
  • bulk stage (11 件以上) で確認ダイアログが出ること
  • sync で upstream missing → auto-publish 通知が出ること
  • post-push で fetch が落ちた時に "push は成功" 通知が出ること (ネットワーク切断で再現)
  • create-pr が既存 PR を検知した時に confirm が出ること
  • Convert to draft / rerun all が確認必須になっていること
  • DiscardConfirmDialog が日本語になっていること
  • bulk discard 時に editor で未保存ファイルがあれば警告が出ること
  • index.lock 状態で switch-branch して強制解除できること

Summary by CodeRabbit

  • 新機能

    • 統一された Git 操作ダイアログ基盤(確認/エラー/警告)を追加し、アプリ上部で常時利用可能に。
    • インデックスロック強制解除の公的操作を追加。
    • コミットにフック無視オプションを追加。
    • プッシュ/同期で警告配列を返し、PR作成で既存PR判定を返す仕様を追加。
  • 改善

    • 文脈別のGitエラー分類に基づくダイアログと復旧アクション/リトライを導入。
    • 大量操作(閾値10件)で確認フローを追加。
    • 各種ダイアログやボタン文言を日本語化。

- New zustand store (useGitOperationDialogStore) for cross-component dialog state
- New GitOperationDialog component rendered once at ChangesView level
- classifyGitError helper maps stderr/error-message to structured kinds
- showGitErrorDialog / showGitConfirmDialog helpers for call sites
- CommitInput: commit/push/pull/sync/fetch errors now open the unified dialog
  with context-aware next actions (rebase+retry, stash+retry, copy, etc.)
- PRButton: merge PR now requires a confirmation dialog (irreversible action);
  merge errors route through classifyGitError
- ChangesView: bulk stage/unstage of >10 files now requires confirmation
- useCreateOrOpenPR: behind-upstream flow replaces window.confirm with unified
  dialog; base-repo and reset errors route through showGitErrorDialog
…nd P2 errors

Backend (apps/desktop/src/lib/trpc/routers/changes):
- git-operation-types: new GitOperationWarning union + GitSyncStageError class
- push/sync now return warnings[] instead of throwing for non-fatal follow-ups
  (auto-published-upstream, post-push-fetch-failed); sync wraps pull/push
  failures in GitSyncStageError so frontend can distinguish stages
- createPR returns isExisting flag so frontend can show "open existing PR?"
- commit accepts skipHooks to bypass pre-commit / commit-msg hooks
- New forceUnlockIndex procedure to remove stale .git/index.lock

Frontend:
- gitWarningDialog: renders auto-repair notifications from response warnings
- CommitInput push/sync onSuccess now show warning dialogs
- ChangesHeader switchBranch routes index.lock through the unified dialog with
  forceUnlockIndex action; other non-BranchActionDialog branch errors route to
  showGitErrorDialog; fetch RefreshButton error too
- ReviewPanel: Convert to draft now requires confirmation; Re-run all jobs
  requires confirmation (keeps a secondary "failed only" action)
- RepositoryPanel: workflow_dispatch requires confirmation
- ChangesView bulk discard now detects editor unsaved state and shows a
  warning dialog listing dirty document count
- DiscardConfirmDialog + bulk discard prompts translated to Japanese for
  consistency with the rest of the git dialog surface
- useCreateOrOpenPR: existing-PR detection opens a confirm dialog with an
  explicit "open existing" action instead of silently navigating
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 12, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Git操作の警告型とステージ別エラーを導入し、エラー分類ロジック、Zustandベースの汎用Git操作ダイアログ、ダイアログヘルパー群を追加してChanges系UIへ統合しました(50語以内)。

Changes

コホート / ファイル 説明
型定義とコア構造
apps/desktop/src/lib/trpc/routers/changes/git-operation-types.ts
GitOperationWarning(4バリアント)とGitSyncStageErrorを追加。
バックエンドルータ更新
apps/desktop/src/lib/trpc/routers/changes/git-operations.ts
commitskipHooks?: boolean追加。push/syncwarnings: GitOperationWarning[]を返すよう変更。syncエラーをGitSyncStageErrorでステージ別にラップ。createPRisExistingを追加。forceUnlockIndexミューテーション追加。
エラー分類ロジック
apps/desktop/src/renderer/lib/git/classifyGitError.ts
文脈依存のclassifyGitError(error, context)を追加。多数のGitErrorKindと補助データ(conflict/overwriteファイル等)を抽出。
ダイアログビルダ/ハンドラ
apps/desktop/src/renderer/lib/git/gitConfirmDialog.ts, apps/desktop/src/renderer/lib/git/gitErrorDialog.ts, apps/desktop/src/renderer/lib/git/gitWarningDialog.ts
確認・エラー・警告ダイアログ用ビルダーとshow*Dialog関数群を追加。バックエンド警告→ユーザーダイアログ優先選定と条件付きアクションを実装。
ダイアログストアとUIコンポーネント
apps/desktop/src/renderer/stores/git-operation-dialog.ts, apps/desktop/src/renderer/screens/main/components/GitOperationDialog/...
Zustandストア(spec/dialogId/isPending)とopenGitOperationDialog/closeGitOperationDialogGitOperationDialogコンポーネントを追加。アクション実行時のペンディング管理と例外ハンドリングを導入。
Changes系UI統合
apps/desktop/src/renderer/screens/main/components/WorkspaceView/.../ChangesView.tsx, .../ChangesHeader.tsx, CommitInput.tsx, PRButton.tsx, RepositoryPanel.tsx, ReviewPanel.tsx
既存のtoast/confirmをダイアログベースに差し替え。bulk操作の閾値判定で確認ダイアログを挟む変更、エラーリカバリ用ハンドラ(再試行/force-unlock/stash等)を追加。日本語文言の一部更新。
PR作成フロー統合
apps/desktop/src/renderer/screens/main/hooks/useCreateOrOpenPR/useCreateOrOpenPR.ts
PR作成の成功/失敗処理をダイアログ化(既存PR検出やbehind-upstreamの再試行フロー)。
ローカライズ・文言調整
apps/desktop/src/renderer/react-query/InitGitDialog.tsx, .../CreatePullRequestBaseRepoDialog.tsx, .../DiscardConfirmDialog.tsx
複数ダイアログの文言を日本語化、ボタンラベルを更新。
ルート統合
apps/desktop/src/renderer/routes/_authenticated/layout.tsx
認証レイアウトにGitOperationDialogを追加して常駐表示。

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant UI as UIコンポーネント
    participant Store as Git操作ストア
    participant Backend as バックエンドルータ
    participant Classify as classifyGitError
    participant Dialog as GitOperationDialog

    User->>UI: コミット/プッシュ/同期 等を実行
    UI->>Backend: ミューテーション呼び出し
    alt 成功かつ警告あり
        Backend-->>UI: { success, warnings }
        UI->>Dialog: showGitWarningDialog(warnings, handlers)
        Dialog->>Store: openGitOperationDialog(spec)
        Store-->>Dialog: spec を公開
        Dialog-->>User: 警告ダイアログ表示
        User->>Dialog: アクション選択(例: PR を開く / 再フェッチ)
        Dialog->>UI: 指定ハンドラを実行
    else エラー
        Backend-->>UI: error
        UI->>Classify: classifyGitError(error, context)
        Classify-->>UI: ClassifiedGitError
        UI->>Dialog: showGitErrorDialog(classified, handlers)
        Dialog->>Store: openGitOperationDialog(spec)
        Dialog-->>User: エラーダイアログ表示(再試行/フォース解除等)
        User->>Dialog: 再試行等の選択
        Dialog->>UI: 選択されたハンドラを起動
    end
Loading
sequenceDiagram
    participant ChangesView
    participant ConfirmHelper as showGitConfirmDialog
    participant Store as git-operation-dialog
    participant Dialog as GitOperationDialog
    participant Action as ActionButton
    participant Backend as バックエンド

    ChangesView->>ChangesView: bulk 件数判定 (BULK_STAGE_CONFIRM_THRESHOLD)
    alt 閾値以下
        ChangesView->>Backend: 即時ミューテーション実行
    else 閾値以上
        ChangesView->>ConfirmHelper: showGitConfirmDialog(kind="bulk-stage-all-confirm")
        ConfirmHelper->>Store: openGitOperationDialog(spec)
        Store-->>Dialog: spec 更新
        Dialog-->>User: 確認ダイアログ表示
        User->>Action: 確認クリック
        Action->>ChangesView: ミューテーション実行(ペンディング管理)
        Action->>Store: isPending 更新 / close
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐇
ダイアログで問いかけ 道を示すよ、
エラーは分類して 一つずつ直す、
再試行ボタンで また進み出す、
ストアと僕で 君の操作守る、
にんじん片手に 今日もGitを見守るよ 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.82% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed PR タイトルは主要な変更内容(Changes サイドバー全体で Git 操作ダイアログの統一)を的確に要約しており、変更セットの中心を明確に表現しています。
Description check ✅ Passed PR 説明は詳細で構造化されており、基盤の変更、エラー分類、新規ダイアログ、バックエンド変更、既存ダイアログの統一、設計メモ、スコープ外の項目、テストプランを網羅しています。リポジトリのテンプレートに対しては「Description」セクションは非常に詳細ですが、「Related Issues」、「Type of Change」、「Testing」、「Screenshots」などのセクションは明示的に記載されていません。

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/git-dialog-unification

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@MocA-Love MocA-Love self-assigned this Apr 12, 2026
@MocA-Love MocA-Love marked this pull request as ready for review April 12, 2026 18:29
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6f1895a4c7

ℹ️ 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".

Comment thread apps/desktop/src/lib/trpc/routers/changes/git-operations.ts Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (11)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/DiscardConfirmDialog/DiscardConfirmDialog.tsx (1)

28-28: 日本語テキストのハードコード化について

デフォルト値として日本語テキストがハードコードされています。これは PR 目的に沿っていますが、将来的に多言語対応が必要になった場合、i18n フレームワークへの移行が必要になります。

現時点では問題ありませんが、他のコンポーネントでも同様のパターンが使用されている場合は、将来の保守性のために統一的な国際化戦略を検討することをお勧めします。

Also applies to: 45-45

🤖 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/DiscardConfirmDialog/DiscardConfirmDialog.tsx`
at line 28, The component DiscardConfirmDialog currently hardcodes Japanese text
for the confirmLabel prop ("破棄") (also present at the other occurrence), which
hinders future i18n; update DiscardConfirmDialog to obtain its default confirm
label from the app's i18n utility (e.g., use t('discard') or equivalent) or fall
back to a prop so the default is language-neutral, replacing the literal "破棄" in
the confirmLabel default and the other occurrence with a call to the translation
function or a configurable prop value.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitInput/CommitInput.tsx (1)

82-85: クリップボード API の使用が一貫していません

ReviewPanel.tsx では electronTrpc.external.copyText.useMutation() を使用していますが、ここでは navigator.clipboard.writeText を直接使用しています。Electron 環境では動作しますが、コードベースの一貫性のため、tRPC mutation の使用を検討してください。

♻️ tRPC mutation を使用する提案
+	const copyToClipboardMutation = electronTrpc.external.copyText.useMutation();
+
 	const commitMutation = electronTrpc.changes.commit.useMutation({
 		// ...
 		onError: (error) => {
 			showGitErrorDialog(error, "commit", {
 				retry: () => {
 					// ...
 				},
 				copyDetails: () => {
-					void navigator.clipboard.writeText(error.message);
-					toast.success("Copied");
+					copyToClipboardMutation.mutate(error.message, {
+						onSuccess: () => toast.success("Copied"),
+						onError: () => toast.error("Failed to copy"),
+					});
 				},
 			});
 		},
 	});
🤖 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/CommitInput/CommitInput.tsx`
around lines 82 - 85, Replace the direct navigator.clipboard.writeText call in
the copyDetails callback inside CommitInput.tsx with the existing tRPC mutation
used elsewhere (electronTrpc.external.copyText.useMutation). Import/use the
copyText mutation hook in the CommitInput component, call its mutate (or
mutateAsync) with error.message instead of navigator.clipboard.writeText, and
show toast.success on mutation success (or in the mutation's onSuccess). Ensure
you remove the direct navigator call and keep the same “Copied” toast behavior
and error.message payload.
apps/desktop/src/renderer/lib/git/classifyGitError.ts (1)

187-188: 冗長な return 文

この return 文は常に null を返すため、単に return null; で十分です。

♻️ 修正案
-	return context === "push" || context === "sync" ? null : null;
+	return null;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/lib/git/classifyGitError.ts` around lines 187 -
188, The return expression in classifyGitError (in
apps/desktop/src/renderer/lib/git/classifyGitError.ts) uses a conditional that
always yields null; simplify it by replacing the conditional return (return
context === "push" || context === "sync" ? null : null;) with a single return
null; to remove redundant logic and clarify intent.
apps/desktop/src/renderer/screens/main/hooks/useCreateOrOpenPR/useCreateOrOpenPR.ts (1)

232-236: エラーハンドリングが統一されていません

onConfirm 内の updatePullRequestBaseRepo 失敗時は toast.error を使用していますが、他の箇所は showGitErrorDialog に移行しています。一貫性のために統一を検討してください。

♻️ 修正案
 					} catch (error) {
-						const message =
-							error instanceof Error ? error.message : String(error);
-						toast.error(`Failed: ${message}`);
+						showGitErrorDialog(error, "create-pr");
 					}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/screens/main/hooks/useCreateOrOpenPR/useCreateOrOpenPR.ts`
around lines 232 - 236, The error handling for updatePullRequestBaseRepo inside
onConfirm is inconsistent with the rest of the hook: replace the toast.error
call with the same showGitErrorDialog usage used elsewhere so all git errors use
the unified dialog; capture the error as currently done (const message = error
instanceof Error ? error.message : String(error)) and pass the original error
(or message and error) and a short context string to showGitErrorDialog so
behavior and user messaging match other failure branches that call
showGitErrorDialog.
apps/desktop/src/renderer/lib/git/gitErrorDialog.ts (1)

143-161: オプショナルチェーンの重複使用

handlers.openConflictFiles の存在チェックが条件部分と onClick 内で重複しています。条件で既にチェック済みなので、onClick 内では不要です。

♻️ 修正案
 				primaryAction:
 					handlers.openConflictFiles &&
 					data.conflictFiles &&
 					data.conflictFiles.length > 0
 						? {
 								label: "競合ファイルを開く",
 								variant: "accent",
 								onClick: () =>
-									handlers.openConflictFiles?.(data.conflictFiles ?? []),
+									handlers.openConflictFiles!(data.conflictFiles!),
 							}
 						: undefined,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/lib/git/gitErrorDialog.ts` around lines 143 - 161,
The primaryAction currently double-checks handlers.openConflictFiles using
optional chaining even though the outer condition already ensures it exists;
update the primaryAction's onClick to call handlers.openConflictFiles directly
(no optional chaining) and pass the existing data.conflictFiles fallback as
before (e.g., data.conflictFiles ?? []) so you avoid redundant checks while
preserving the nullish fallback for conflictFiles.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx (2)

54-56: import 文の配置が不自然です

RepositoryPanelReviewPanel などの import が関数定義の後に配置されています。通常、import 文はファイルの先頭にまとめるべきです。lint で自動修正されない場合は手動で移動を検討してください。

🤖 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/ChangesView.tsx`
around lines 54 - 56, 現在のファイルで
RepositoryPanel/ReviewPanel/VerticalResizablePanels の import
が関数定義の後にあり不自然なので、これらの import 文をファイル先頭の既存 import
ブロックと同じ場所に移動してください(関数やコンポーネント定義より前)。移動後に ESLint/TypeScript のエラーや副作用(サイドエフェクト
import の順序依存)を確認し、必要ならば関連する import をまとめて並べ替えてください。

785-800: 重複コードパターン

onShowDiscardStagedDialogonShowDiscardUnstagedDialog のロジックがほぼ同一です。共通のヘルパー関数に抽出することで保守性が向上します。

♻️ 抽出案
const showDiscardDialogWithUnsavedCheck = (
  openDialog: () => void,
) => {
  const dirtyCount = countDirtyDocumentsForWorkspace(workspaceId);
  if (dirtyCount > 0) {
    showGitConfirmDialog({
      kind: "discard-unsaved-editor",
      tone: "warn",
      title: "エディタに保存していない変更があります",
      description: `${dirtyCount} 件のファイルがエディタで未保存です。discard を続行すると保存前の編集内容が失われます。`,
      confirmLabel: "続行して discard",
      confirmVariant: "danger",
      onConfirm: openDialog,
    });
    return;
  }
  openDialog();
};

Also applies to: 842-857

🤖 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/ChangesView.tsx`
around lines 785 - 800, Extract the duplicated logic in
onShowDiscardStagedDialog and onShowDiscardUnstagedDialog into a helper function
(e.g., showDiscardDialogWithUnsavedCheck) that calls
countDirtyDocumentsForWorkspace(workspaceId), shows the warn confirmation via
showGitConfirmDialog when dirtyCount > 0, and otherwise invokes the provided
opener; replace the current bodies to call showDiscardDialogWithUnsavedCheck(()
=> setShowDiscardStagedDialog(true)) and showDiscardDialogWithUnsavedCheck(() =>
setShowDiscardUnstagedDialog(true)) respectively, keeping the same dialog props
and onConfirm behavior.
apps/desktop/src/lib/trpc/routers/changes/git-operations.ts (2)

759-768: エラーハンドリングが広すぎる可能性

catch ブロックがすべてのエラーを無視してファイルが存在しないものとして扱っています。stat が ENOENT 以外のエラー(例: EACCES)を返した場合も無視されます。権限エラーの場合はユーザーに通知した方が良いかもしれません。

♻️ ENOENT のみを無視する案
 				for (const candidate of candidates) {
 					try {
 						await stat(candidate);
 						await unlink(candidate);
 						clearStatusCacheForWorktree(input.worktreePath);
 						return { removed: true, path: candidate };
-					} catch {
-						// file not present; try next
+					} catch (err) {
+						// ENOENT means file not present; try next
+						if ((err as NodeJS.ErrnoException).code !== "ENOENT") {
+							console.warn(`[forceUnlockIndex] Failed to remove ${candidate}:`, err);
+						}
 					}
 				}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/lib/trpc/routers/changes/git-operations.ts` around lines 759
- 768, The current loop over candidates swallows all errors in the try { await
stat(candidate); await unlink(candidate);
clearStatusCacheForWorktree(input.worktreePath); return { removed: true, path:
candidate }; } catch { ... } which treats permission and other errors like
ENOENT; change the catch to only ignore ENOENT (error.code === 'ENOENT') and
continue to the next candidate, but rethrow or surface any other errors (e.g.,
EACCES) so the caller can handle/report them; keep references to stat, unlink,
clearStatusCacheForWorktree, candidates and input.worktreePath when implementing
this change.

751-752: 動的インポートの使用

node:pathnode:fs/promises が関数内で動的にインポートされています。これらはバックエンドコードなので、ファイル先頭で静的にインポートする方がパフォーマンスと可読性の面で良いです。

♻️ 静的インポートへの変更案

ファイル先頭に追加:

import { join } from "node:path";
import { stat, unlink } from "node:fs/promises";

関数内から動的インポートを削除:

-				const { join } = await import("node:path");
-				const { stat, unlink } = await import("node:fs/promises");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/lib/trpc/routers/changes/git-operations.ts` around lines 751
- 752, Replace the in-function dynamic imports for node:path and
node:fs/promises by adding static imports at the top of the module and removing
the await import(...) calls; specifically, add top-level imports for join, stat,
and unlink and remove the dynamic import lines that currently load "node:path"
and "node:fs/promises" so existing uses of join, stat, and unlink in the
git-operations code continue to work with the statically imported symbols.
apps/desktop/src/lib/trpc/routers/changes/git-operation-types.ts (1)

35-44: Error.cause 標準プロパティのシャドウイングを回避する

ES2022 以降、Error クラスには標準の cause プロパティがあります。プロジェクトの TypeScript 5.9.3 と ES2022 設定はこの機能を完全にサポートしているため、カスタムプロパティとして定義するのではなく、Error コンストラクタの options パラメータを通じて標準方式で設定することをお勧めします。

♻️ 標準の cause を使用する案
 export class GitSyncStageError extends Error {
 	readonly stage: "pull" | "push";
-	readonly cause: unknown;
 	constructor(stage: "pull" | "push", cause: unknown) {
 		const message = cause instanceof Error ? cause.message : String(cause);
-		super(`[sync:${stage}] ${message}`);
+		super(`[sync:${stage}] ${message}`, { cause });
 		this.name = "GitSyncStageError";
 		this.stage = stage;
-		this.cause = cause;
 	}
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/lib/trpc/routers/changes/git-operation-types.ts` around
lines 35 - 44, Replace the custom shadowing of the error "cause" by using the
standard ES2022 Error cause option in the GitSyncStageError class: remove the
readonly cause property declaration from GitSyncStageError, keep the stage
field, and in the constructor (GitSyncStageError(stage: "pull" | "push", cause:
unknown)) compute the message as before but pass the cause via super(message, {
cause }) instead of assigning this.cause; retain setting this.name and
this.stage. Ensure TypeScript relies on the built-in Error.cause type (do not
redeclare cause).
apps/desktop/src/renderer/stores/git-operation-dialog.ts (1)

30-30: kind: string は型安全性が弱いです

Line 30 は任意文字列なので、識別子の typo をコンパイル時に検知できません。kind は union 型(as const 配列由来など)へ寄せると、分岐・計測・テストの安定性が上がります。

差分案
+export const GIT_OPERATION_DIALOG_KINDS = [
+	"push-rejected",
+	"pull-conflict",
+	"index-lock",
+	"detached-head",
+] as const;
+
+export type GitOperationDialogKind = (typeof GIT_OPERATION_DIALOG_KINDS)[number];
+
 export interface GitOperationDialogSpec {
 	/** Identifier for telemetry and tests (e.g. "push-rejected"). */
-	kind: string;
+	kind: GitOperationDialogKind;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/stores/git-operation-dialog.ts` at line 30, The
property declared as "kind: string" is too permissive; replace it with a string
literal union type derived from a const array (e.g., define a const
GIT_OPERATION_KINDS = [...] as const and use type GitOperationKind = typeof
GIT_OPERATION_KINDS[number]) and change the "kind" declaration to use that
GitOperationKind type; update any code that constructs or narrows on "kind"
(switch/if, tests) to use the defined constants/union so typos are caught at
compile time and enable exhaustive checks where appropriate.
🤖 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/lib/git/classifyGitError.ts`:
- Around line 118-121: The code is incorrectly excluding valid file paths that
contain spaces by checking file.includes(" ") before adding to the set; update
the logic in classifyGitError.ts (the block handling match[1] and files.add) to
accept filenames with spaces (keep trimming but remove the file.includes(" ")
guard) so it matches the behavior of extractOverwriteFiles; ensure you still
guard against empty or undefined match[1] values and add the trimmed file string
to files.

In `@apps/desktop/src/renderer/lib/git/gitWarningDialog.ts`:
- Around line 11-15: The local GitOperationWarning type duplicates the backend
definition; remove this local union type and import the canonical
GitOperationWarning from the backend router type instead (use the exported type
name GitOperationWarning from lib/trpc/routers/changes/git-operation-types),
replacing usages in this file to reference the imported type; ensure any
exported symbols relying on the local type now re-export or type-alias the
imported type so callers remain consistent.

In
`@apps/desktop/src/renderer/screens/main/components/GitOperationDialog/GitOperationDialog.tsx`:
- Around line 84-95: The onClick handler currently swallows errors from
action.onClick() because there is no catch; update the onClick function (the
async onClick defined in GitOperationDialog.tsx that calls action.onClick(),
toggles useGitOperationDialogStore.getState().setPending, and calls close()) to
catch errors from both synchronous throws and rejected Promises, then either
rethrow the error or surface it to the UI (e.g., call your notification/error
handler) before the finally block runs; keep the existing setPending(false) and
close() in finally so state is always cleared, but ensure the catch
handles/logs/propagates the error instead of silently ignoring it.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx`:
- Around line 479-495: The forceUnlockIndex callback passed to
showGitErrorDialog lacks error handling: wrap the call to
forceUnlockIndexMutation.mutateAsync({ worktreePath }) in an async try/catch and
await it (remove the void), and on success call switchBranch.mutate; on failure
show an error dialog/notification (e.g., via showGitErrorDialog or other
existing UI error handler). Also handle errors from switchBranch.mutate by
providing an onError callback or by catching its promise result and surfacing a
user-facing error; update the forceUnlockIndex function closure accordingly to
ensure both unlock and switch failures produce visible feedback to the user
(reference: forceUnlockIndexMutation.mutateAsync, switchBranch.mutate,
showGitErrorDialog, forceUnlockIndex).

In `@apps/desktop/src/renderer/stores/git-operation-dialog.ts`:
- Around line 61-64: The open/close handlers in the git-operation-dialog store
lack a dialog identifier so a trailing async finally can close a different
dialog; add a dialogId field to the store state and have open(spec, dialogId)
set { spec, isPending: false, dialogId }, setPending(isPending) keep dialogId,
and change close() to accept an optional dialogId and only clear { spec: null,
isPending: false, dialogId: null } when the stored dialogId matches the supplied
id (or when no id is supplied, require a match). Update callers that call open
and finally/close to pass the same dialogId so only the intended dialog is
closed.

---

Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/changes/git-operation-types.ts`:
- Around line 35-44: Replace the custom shadowing of the error "cause" by using
the standard ES2022 Error cause option in the GitSyncStageError class: remove
the readonly cause property declaration from GitSyncStageError, keep the stage
field, and in the constructor (GitSyncStageError(stage: "pull" | "push", cause:
unknown)) compute the message as before but pass the cause via super(message, {
cause }) instead of assigning this.cause; retain setting this.name and
this.stage. Ensure TypeScript relies on the built-in Error.cause type (do not
redeclare cause).

In `@apps/desktop/src/lib/trpc/routers/changes/git-operations.ts`:
- Around line 759-768: The current loop over candidates swallows all errors in
the try { await stat(candidate); await unlink(candidate);
clearStatusCacheForWorktree(input.worktreePath); return { removed: true, path:
candidate }; } catch { ... } which treats permission and other errors like
ENOENT; change the catch to only ignore ENOENT (error.code === 'ENOENT') and
continue to the next candidate, but rethrow or surface any other errors (e.g.,
EACCES) so the caller can handle/report them; keep references to stat, unlink,
clearStatusCacheForWorktree, candidates and input.worktreePath when implementing
this change.
- Around line 751-752: Replace the in-function dynamic imports for node:path and
node:fs/promises by adding static imports at the top of the module and removing
the await import(...) calls; specifically, add top-level imports for join, stat,
and unlink and remove the dynamic import lines that currently load "node:path"
and "node:fs/promises" so existing uses of join, stat, and unlink in the
git-operations code continue to work with the statically imported symbols.

In `@apps/desktop/src/renderer/lib/git/classifyGitError.ts`:
- Around line 187-188: The return expression in classifyGitError (in
apps/desktop/src/renderer/lib/git/classifyGitError.ts) uses a conditional that
always yields null; simplify it by replacing the conditional return (return
context === "push" || context === "sync" ? null : null;) with a single return
null; to remove redundant logic and clarify intent.

In `@apps/desktop/src/renderer/lib/git/gitErrorDialog.ts`:
- Around line 143-161: The primaryAction currently double-checks
handlers.openConflictFiles using optional chaining even though the outer
condition already ensures it exists; update the primaryAction's onClick to call
handlers.openConflictFiles directly (no optional chaining) and pass the existing
data.conflictFiles fallback as before (e.g., data.conflictFiles ?? []) so you
avoid redundant checks while preserving the nullish fallback for conflictFiles.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx`:
- Around line 54-56: 現在のファイルで
RepositoryPanel/ReviewPanel/VerticalResizablePanels の import
が関数定義の後にあり不自然なので、これらの import 文をファイル先頭の既存 import
ブロックと同じ場所に移動してください(関数やコンポーネント定義より前)。移動後に ESLint/TypeScript のエラーや副作用(サイドエフェクト
import の順序依存)を確認し、必要ならば関連する import をまとめて並べ替えてください。
- Around line 785-800: Extract the duplicated logic in onShowDiscardStagedDialog
and onShowDiscardUnstagedDialog into a helper function (e.g.,
showDiscardDialogWithUnsavedCheck) that calls
countDirtyDocumentsForWorkspace(workspaceId), shows the warn confirmation via
showGitConfirmDialog when dirtyCount > 0, and otherwise invokes the provided
opener; replace the current bodies to call showDiscardDialogWithUnsavedCheck(()
=> setShowDiscardStagedDialog(true)) and showDiscardDialogWithUnsavedCheck(() =>
setShowDiscardUnstagedDialog(true)) respectively, keeping the same dialog props
and onConfirm behavior.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitInput/CommitInput.tsx`:
- Around line 82-85: Replace the direct navigator.clipboard.writeText call in
the copyDetails callback inside CommitInput.tsx with the existing tRPC mutation
used elsewhere (electronTrpc.external.copyText.useMutation). Import/use the
copyText mutation hook in the CommitInput component, call its mutate (or
mutateAsync) with error.message instead of navigator.clipboard.writeText, and
show toast.success on mutation success (or in the mutation's onSuccess). Ensure
you remove the direct navigator call and keep the same “Copied” toast behavior
and error.message payload.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/DiscardConfirmDialog/DiscardConfirmDialog.tsx`:
- Line 28: The component DiscardConfirmDialog currently hardcodes Japanese text
for the confirmLabel prop ("破棄") (also present at the other occurrence), which
hinders future i18n; update DiscardConfirmDialog to obtain its default confirm
label from the app's i18n utility (e.g., use t('discard') or equivalent) or fall
back to a prop so the default is language-neutral, replacing the literal "破棄" in
the confirmLabel default and the other occurrence with a call to the translation
function or a configurable prop value.

In
`@apps/desktop/src/renderer/screens/main/hooks/useCreateOrOpenPR/useCreateOrOpenPR.ts`:
- Around line 232-236: The error handling for updatePullRequestBaseRepo inside
onConfirm is inconsistent with the rest of the hook: replace the toast.error
call with the same showGitErrorDialog usage used elsewhere so all git errors use
the unified dialog; capture the error as currently done (const message = error
instanceof Error ? error.message : String(error)) and pass the original error
(or message and error) and a short context string to showGitErrorDialog so
behavior and user messaging match other failure branches that call
showGitErrorDialog.

In `@apps/desktop/src/renderer/stores/git-operation-dialog.ts`:
- Line 30: The property declared as "kind: string" is too permissive; replace it
with a string literal union type derived from a const array (e.g., define a
const GIT_OPERATION_KINDS = [...] as const and use type GitOperationKind =
typeof GIT_OPERATION_KINDS[number]) and change the "kind" declaration to use
that GitOperationKind type; update any code that constructs or narrows on "kind"
(switch/if, tests) to use the defined constants/union so typos are caught at
compile time and enable exhaustive checks where appropriate.
🪄 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: 3c5d4fe5-111a-4a77-97a6-a384417900f6

📥 Commits

Reviewing files that changed from the base of the PR and between dcb96d6 and 6f1895a.

📒 Files selected for processing (17)
  • apps/desktop/src/lib/trpc/routers/changes/git-operation-types.ts
  • apps/desktop/src/lib/trpc/routers/changes/git-operations.ts
  • apps/desktop/src/renderer/lib/git/classifyGitError.ts
  • apps/desktop/src/renderer/lib/git/gitConfirmDialog.ts
  • apps/desktop/src/renderer/lib/git/gitErrorDialog.ts
  • apps/desktop/src/renderer/lib/git/gitWarningDialog.ts
  • apps/desktop/src/renderer/screens/main/components/GitOperationDialog/GitOperationDialog.tsx
  • apps/desktop/src/renderer/screens/main/components/GitOperationDialog/index.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/components/PRButton/PRButton.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitInput/CommitInput.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/DiscardConfirmDialog/DiscardConfirmDialog.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/RepositoryPanel/RepositoryPanel.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx
  • apps/desktop/src/renderer/screens/main/hooks/useCreateOrOpenPR/useCreateOrOpenPR.ts
  • apps/desktop/src/renderer/stores/git-operation-dialog.ts

Comment thread apps/desktop/src/renderer/lib/git/classifyGitError.ts
Comment thread apps/desktop/src/renderer/lib/git/gitWarningDialog.ts Outdated
Comment thread apps/desktop/src/renderer/stores/git-operation-dialog.ts Outdated
…ions

Visual consistency fixes based on review feedback.

- GitOperationDialog icon box now always uses bg-muted/text-muted-foreground,
  matching BranchActionDialog. Removed the amber/emerald/sky/red tone colors
  that were out of character for the rest of the app. Tone still drives the
  icon glyph (alert-triangle / circle-alert / circle-check / info /
  shield-alert) but no longer the background color.
- Button variants narrowed to shadcn's primary / outline / ghost /
  destructive. Removed the custom "warn" / "ok" / "accent" variants that
  produced amber/emerald/sky buttons. "destructive" is reserved for truly
  irreversible actions (force push, force unlock, discard, abort rebase).
- Removed flex-wrap on the footer so all dialogs render on a single row,
  matching the actual BranchActionDialog behavior. Button labels trimmed to
  fit: "修正して再 commit" -> "再試行", "フックを無視して commit" ->
  "フック無視で commit", "強制的に解除" -> "強制解除", "force push (lease)"
  -> "force push", "署名なしでこの commit を作る" -> "署名なしで commit".
- Dropped the tertiary "出力をコピー" action on commit-hook-failed. The raw
  stderr is already shown in the details block inside the dialog, so a
  separate copy button was redundant and only contributed to the 4-button
  overflow.
- Translated CreatePullRequestBaseRepoDialog (Choose -> "Pull Request の base
  リポジトリを選択"), its call sites in PRButton (Update / Save -> "PR base
  リポジトリを変更" / "保存"), and InitGitDialog (all Cancel / Initialize
  Git strings) to Japanese so the git dialog surface is uniform.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx (1)

293-344: ⚠️ Potential issue | 🟡 Minor

runToggleDraftState に多重実行ガードを追加してください。

確認ダイアログ経由の再入や連打時に、同一 mutation が重複送信される余地があります。isDraftTogglePending の早期 return を入れておくと安全です。

差分案
 const runToggleDraftState = () => {
-	if (!resolvedWorkspaceId || !pr) {
+	if (!resolvedWorkspaceId || !pr || isDraftTogglePending) {
 		return;
 	}
🤖 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/ReviewPanel/ReviewPanel.tsx`
around lines 293 - 344, The runToggleDraftState function can be invoked multiple
times causing duplicate mutations; add an early-return guard that checks the
isDraftTogglePending flag at the start of runToggleDraftState to prevent
re-entrancy (return immediately if isDraftTogglePending is true), so the
function sets setIsDraftTogglePending(true) only when no toggle is in flight and
then proceeds to call setPullRequestDraftStateMutation.mutateAsync and the
existing finally block clears the flag.
🤖 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/ReviewPanel/ReviewPanel.tsx`:
- Around line 592-613: The confirm dialog currently makes "all" the primary
high-cost action; update the dialog in handleRerunChecks so the primary action
runs failed checks and the secondary runs all checks: swap confirmLabel and
secondaryLabel meanings, change confirmVariant from "primary" to a
neutral/default and make the secondary (all) action destructive if supported,
and wire onConfirm to call runRerunChecks("failed") while onSecondary calls
runRerunChecks("all"); keep showGitConfirmDialog, handleRerunChecks and
runRerunChecks as the referenced symbols to locate the change.

---

Outside diff comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx`:
- Around line 293-344: The runToggleDraftState function can be invoked multiple
times causing duplicate mutations; add an early-return guard that checks the
isDraftTogglePending flag at the start of runToggleDraftState to prevent
re-entrancy (return immediately if isDraftTogglePending is true), so the
function sets setIsDraftTogglePending(true) only when no toggle is in flight and
then proceeds to call setPullRequestDraftStateMutation.mutateAsync and the
existing finally block clears the flag.
🪄 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: ea12945b-7756-406e-b2e9-58da04bd8b5d

📥 Commits

Reviewing files that changed from the base of the PR and between 6f1895a and f5fecfc.

📒 Files selected for processing (11)
  • apps/desktop/src/renderer/lib/git/gitConfirmDialog.ts
  • apps/desktop/src/renderer/lib/git/gitErrorDialog.ts
  • apps/desktop/src/renderer/react-query/projects/InitGitDialog.tsx
  • apps/desktop/src/renderer/screens/main/components/CreatePullRequestBaseRepoDialog/CreatePullRequestBaseRepoDialog.tsx
  • apps/desktop/src/renderer/screens/main/components/GitOperationDialog/GitOperationDialog.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/components/PRButton/PRButton.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitInput/CommitInput.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx
  • apps/desktop/src/renderer/screens/main/hooks/useCreateOrOpenPR/useCreateOrOpenPR.ts
  • apps/desktop/src/renderer/stores/git-operation-dialog.ts
✅ Files skipped from review due to trivial changes (4)
  • apps/desktop/src/renderer/react-query/projects/InitGitDialog.tsx
  • apps/desktop/src/renderer/screens/main/components/CreatePullRequestBaseRepoDialog/CreatePullRequestBaseRepoDialog.tsx
  • apps/desktop/src/renderer/lib/git/gitErrorDialog.ts
  • apps/desktop/src/renderer/stores/git-operation-dialog.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • apps/desktop/src/renderer/screens/main/hooks/useCreateOrOpenPR/useCreateOrOpenPR.ts
  • apps/desktop/src/renderer/lib/git/gitConfirmDialog.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitInput/CommitInput.tsx

8 fixes from codex/coderabbit review on feat/git-dialog-unification.

P1 — forceUnlockIndex now resolves the real git-dir via git rev-parse
  --git-dir before probing index.lock / HEAD.lock / shallow.lock. Linked
  worktrees have .git as a file pointing at .git/worktrees/<name>, so the
  old worktreePath/.git/*.lock probing always returned removed: false on
  that common layout.

Critical — GitOperationDialog ActionButton now catches errors thrown by
  action.onClick instead of swallowing them via the unhandled try/finally.
  Mutation errors still route through their own onError handlers; this only
  catches unexpected throws so they land in the console rather than vanishing.

Major — New dialogId token in the zustand store so that a late-running
  action's finally can only clear the dialog it originally opened. open()
  increments and returns the id, close(id?) no-ops when the current dialog
  has already been replaced. GitOperationDialog threads the id into
  ActionButton and the dismiss button.

Major — GitOperationWarning is now re-exported from
  lib/trpc/routers/changes/git-operation-types instead of being duplicated
  in gitWarningDialog.ts; backend and renderer share a single source.

Major — rerun-all-checks-confirm now makes "失敗分だけ再実行" the primary
  action and "全て再実行" the secondary, so a stray Enter keypress can no
  longer re-queue every job on the PR.

P2 — CommitInput now passes retryWithoutHooks to showGitErrorDialog so
  the commit-hook-failed dialog actually exposes the "フック無視で commit"
  button that the backend's new skipHooks flag was added for.

Minor — extractConflictFiles no longer drops filenames that contain spaces
  (git emits them literally in merge conflict output).

Minor — ChangesHeader's forceUnlockIndex handler now awaits and catches
  the unlock mutation failure, surfacing it through showGitErrorDialog so
  a failed unlock is no longer silent.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/CommitInput/CommitInput.tsx`:
- Around line 72-90: The onError closure is reading the live commitMessage state
(via trimmed) so retry uses whatever the user types later; capture the message
actually submitted and use that captured value in the retry callbacks.
Concretely: in the commit submission flow in CommitInput (the code that calls
commitMutation.mutate), compute const submittedMessage = commitMessage.trim()
and pass/close over submittedMessage (not commitMessage/trimmed) into the
showGitErrorDialog onError handler so both retry and retryWithoutHooks call
commitMutation.mutate with message: submittedMessage.

In `@apps/desktop/src/renderer/stores/git-operation-dialog.ts`:
- Around line 63-65: The setPending call must be guarded by dialogId like close
is: modify the store API and its implementation so setPending accepts an
optional id (e.g., setPending(pending: boolean, id?: number)) and only changes
state when id is undefined or matches the current dialogId; update all callers
(including the ActionButton usages mentioned around the other diff block) to
pass the dialogId they hold so older dialogs' finally handlers cannot flip
pending on a newer dialog.
🪄 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: 014c67b3-686e-4a01-89ee-2e0c85666705

📥 Commits

Reviewing files that changed from the base of the PR and between f5fecfc and aeebac1.

📒 Files selected for processing (8)
  • apps/desktop/src/lib/trpc/routers/changes/git-operations.ts
  • apps/desktop/src/renderer/lib/git/classifyGitError.ts
  • apps/desktop/src/renderer/lib/git/gitWarningDialog.ts
  • apps/desktop/src/renderer/screens/main/components/GitOperationDialog/GitOperationDialog.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitInput/CommitInput.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx
  • apps/desktop/src/renderer/stores/git-operation-dialog.ts
✅ Files skipped from review due to trivial changes (1)
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/desktop/src/renderer/lib/git/gitWarningDialog.ts
  • apps/desktop/src/lib/trpc/routers/changes/git-operations.ts

Comment thread apps/desktop/src/renderer/stores/git-operation-dialog.ts Outdated
Two follow-ups to the previous review batch.

- CommitInput retry now reads the commit message from the mutation
  variables argument (the value that was actually submitted) instead of
  closing over the current textarea state. Editing the input after a
  failed commit previously silently changed what "retry" / "フック無視で
  commit" would send.
- git-operation-dialog's setPending now takes an optional dialogId and
  only updates the store when it matches the current dialog, mirroring
  the close() guard. Without this, a late-running action's finally could
  flip pending=false on a dialog the user had opened in the meantime (for
  example when the action itself opens a subsequent error dialog).
  ActionButton now passes its captured dialogId to both setPending and
  close so a stale action cannot stomp on a newer dialog.
@MocA-Love
Copy link
Copy Markdown
Owner Author

@codex review

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/GitOperationDialog/GitOperationDialog.tsx (1)

69-87: instanceof Promise チェックをより堅牢な方法に改善する

行 73 の instanceof Promise は、realm 差異のある Promise やテナブル(thenable)オブジェクトを検出できません。特に Electron のように複数のコンテキストがある環境では、別のコンテキストで作成された Promise は instanceof チェックをパスしません。また、Promise/A+ 仕様で定義されているテナブルなオブジェクト(.then メソッドを持つオブジェクト)も同様に検出されません。

あわせて、finally ブロック内で既に取得した store 変数を再利用することで、不要な getState() 呼び出しを削減できます。

♻️ 提案差分
 	const onClick = async () => {
 		const store = useGitOperationDialogStore.getState();
 		try {
 			const result = action.onClick();
-			if (result instanceof Promise) {
+			const isPromiseLike =
+				(typeof result === "object" || typeof result === "function") &&
+				result !== null &&
+				"then" in result &&
+				typeof (result as { then: unknown }).then === "function";
+			if (isPromiseLike) {
 				store.setPending(true, dialogId);
-				await result;
+				await (result as PromiseLike<unknown>);
 			}
 		} catch (err) {
 			// Actions normally delegate error reporting to their own mutation
 			// onError handlers. Anything reaching here is an unexpected throw —
 			// surface it to the console instead of silently eating it.
 			console.error("[GitOperationDialog] action threw", err);
 		} finally {
 			// Both setPending and close are scoped to this button's dialogId so
 			// that a late-running action cannot clobber a dialog the user has
 			// opened in the meantime (e.g. if the action opens another dialog).
-			useGitOperationDialogStore.getState().setPending(false, dialogId);
-			useGitOperationDialogStore.getState().close(dialogId);
+			store.setPending(false, dialogId);
+			store.close(dialogId);
 		}
 	};
🤖 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/GitOperationDialog/GitOperationDialog.tsx`
around lines 69 - 87, Replace the brittle "result instanceof Promise" check in
the onClick handler with a robust thenable detection (e.g., check result is
non-null and has a typeof then === 'function') so cross-realm Promises and
Promise-like objects are correctly detected for async handling of
action.onClick(); also reuse the previously captured store variable (from
useGitOperationDialogStore.getState()) inside the finally block when calling
setPending(false, dialogId) and close(dialogId) to avoid extra getState() calls.
🤖 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/GitOperationDialog/GitOperationDialog.tsx`:
- Around line 69-87: Replace the brittle "result instanceof Promise" check in
the onClick handler with a robust thenable detection (e.g., check result is
non-null and has a typeof then === 'function') so cross-realm Promises and
Promise-like objects are correctly detected for async handling of
action.onClick(); also reuse the previously captured store variable (from
useGitOperationDialogStore.getState()) inside the finally block when calling
setPending(false, dialogId) and close(dialogId) to avoid extra getState() calls.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e7d03d80-5ed7-4d00-8ecc-dc1498a80edf

📥 Commits

Reviewing files that changed from the base of the PR and between aeebac1 and 265c7b2.

📒 Files selected for processing (3)
  • apps/desktop/src/renderer/screens/main/components/GitOperationDialog/GitOperationDialog.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitInput/CommitInput.tsx
  • apps/desktop/src/renderer/stores/git-operation-dialog.ts
✅ Files skipped from review due to trivial changes (1)
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitInput/CommitInput.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/src/renderer/stores/git-operation-dialog.ts

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 265c7b208f

ℹ️ 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".

Comment thread apps/desktop/src/lib/trpc/routers/changes/git-operations.ts Outdated
- Move GitOperationDialog mount from ChangesView to the authenticated
  layout (next to InitGitDialog). The dialog was previously only rendered
  while the Changes sidebar was open, so useCreateOrOpenPR prompts
  (behind-upstream confirm, error dialogs) invoked from workspace-level
  hotkey paths were silently invisible when the sidebar was closed.
- forceUnlockIndex no longer swallows stat/unlink failures. Only ENOENT
  from the stat probe is treated as "file not present"; any other stat
  error and every unlink failure is surfaced through a TRPCError so the
  user can see EACCES/EPERM etc. instead of seeing removed: false with
  no explanation.
Six findings from the parallel rv-pr review (HIGH 2, MED 4).

HIGH — rerun-all-checks confirm had a conflicting title/primary pair
  ("全ての CI jobs を再実行しますか?" + primary "失敗分だけ再実行")
  so a stray Enter press did the opposite of what the title suggested.
  The title is now neutral ("CI jobs を再実行しますか?") and the copy
  names the two options explicitly; the safer "failed only" stays as
  the primary action.

HIGH — CommitInput's stashAndRetry recovery path now chains a stashPop
  after the successful pull. Previously picking "stash してから pull"
  from the pull-overwrite dialog stashed the user's local changes and
  never popped them back, silently leaving them on the stash stack.

MED — classifyPushError's trailing `return context === "push" || ...
  ? null : null` was a no-op ternary; collapsed to `return null` and
  dropped the now-unused context parameter.

MED — GitOperationDialog's ActionButton now forwards the mapped variant
  directly to shadcn's <Button variant={...}> instead of hardcoding
  variant="outline" and overriding with className. The previous approach
  let outline borders and shadows bleed into the primary/destructive
  renders.

MED — classifyGitError now consumes the [sync:pull] / [sync:push]
  prefixes that GitSyncStageError plants in the message, so sync
  failures are routed to the matching stage classifier (pull stage
  errors no longer get tried against push patterns and vice versa).

MED — forceUnlockIndex now walks every candidate (index.lock /
  HEAD.lock / shallow.lock) instead of returning after the first
  successful removal, so a worktree with multiple stale locks can be
  cleared in a single invocation.
CI lint caught five biome violations:
- import sort in _authenticated/layout.tsx (my added GitOperationDialog
  import landed in the wrong alphabetical position)
- import sort in _authenticated/_dashboard/layout.tsx (pre-existing)
- trailing blank line in ChangesView.tsx after DiscardConfirmDialog
- formatter whitespace in git-operations.ts (around forceUnlockIndex)
- formatter line wrap in InitGitDialog.tsx Japanese text

All auto-fixed by bun run lint:fix; no behavior change.
@MocA-Love MocA-Love merged commit d13e23c into main Apr 12, 2026
3 of 6 checks passed
MocA-Love added a commit that referenced this pull request Apr 13, 2026
The `commit-identity-missing` dialog that PR #153 introduced told the
user to run `git config` in a terminal, which meant leaving the app
mid-commit. Replace that instruction with an inline form inside the
existing unified GitOperationDialog so the user can set user.name /
user.email and retry the commit without context switching.

- `getGitAuthorEmail` helper added next to the existing
  `getGitAuthorName`; `settings.getGitInfo` now returns it too.
- `settings.setGlobalGitUserConfig` mutation: writes `user.name` and
  `user.email` to the global git config via `simple-git.addConfig(...,
  "global")`.
- `MissingGitUserConfigForm` component: two inputs + a "保存して再試行"
  button. On success it fires the caller-provided `onSaved` callback
  so the commit retry runs immediately with the new identity.
- `gitErrorDialog` is now a `.tsx` so it can render the form as the
  dialog's `extraContent`. The old "設定後に再試行" primary button is
  removed — the form's own save button owns the retry flow to avoid
  a stale retry that would hit the same error again.
- Dialog is closed before firing retry so that any subsequent failure
  (`commit-identity-missing` classifier hit a second time, network
  error, etc) can open a fresh dialog from its own onError.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant