chore: 全体レビュー後の follow-up (cache prefix 修正 + コメント改善)#424
Conversation
…ons) - `clearGitHubCachesForWorktree`: `githubStatusResource.invalidatePrefix` → `.invalidate` because the status cache is keyed by the exact worktree path (no suffix), so prefix matching could falsely invalidate sibling worktrees (e.g. `/foo/bar` vs `/foo/bar-2`). Caught by code-reviewer in the 6/6 review. - `applyAiWorkspaceRename`: clarify the no-op detection comment to explain *why* `cloudResult.branch !== deduped` is the correct signal (cloud returns the pre-update row when the atomic WHERE guard fires). - `updateNameFromHost`: clarify the early-return comment to spell out that we intentionally return the pre-update `workspace` row (not a fresh read) so the caller can observe the guard firing. - Move `plans/20260424-upstream-merge-strategy.md` → `plans/done/` now that the PR #398-#418 + #417 series is complete. Related: - #420 (electron-builder publish.owner residue — logged, not fixed in this PR)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughキャッシュ無効化ロジックがプレフィックスベースから完全一致ベースに変更され、複数のコメント説明が並行リネーム競争状態の意味論を明確化され、上流マージ戦略の包括的な計画ドキュメントが追加されました。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![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: d61a1a138e
ℹ️ 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".
| // githubStatusResource is keyed by the exact worktreePath (no suffix), so | ||
| // invalidate() rather than invalidatePrefix() avoids matching sibling | ||
| // worktrees whose paths share this prefix (e.g. /foo/bar vs /foo/bar-2). | ||
| githubStatusResource.invalidate(worktreePath); |
There was a problem hiding this comment.
githubStatusResource keys are not always the exact worktree path: fetchGitHubPRStatus stores branch workspace entries as "${worktreePath}::${branchName}". Switching clearGitHubCachesForWorktree to invalidate(worktreePath) only clears the exact key, so force-refresh and post-mutation clears can leave branch-scoped status entries untouched, causing stale GitHub status to persist until TTL expiry for branch workspaces. Please invalidate by worktree prefix (or explicitly clear both exact and branch-suffixed keys).
Useful? React with 👍 / 👎.
1 participant
概要
upstream 取り込み全体レビュー (6 並列 agent: Codex arch / Codex fork / code-reviewer / security-reviewer / regression-reviewer / general audit) で挙がった follow-up 事項のうち、即対応可能な 3 件を反映した PR です。
変更内容
1.
clearGitHubCachesForWorktreeの cache 無効化を exact match にファイル: `apps/desktop/src/lib/trpc/routers/workspaces/utils/github/cache.ts`
`githubStatusResource` はキーに worktreePath 完全一致を使う (suffix なし) ため、`invalidatePrefix(worktreePath)` では兄弟 worktree を誤マッチするリスクがあった (例: `/foo/bar` の invalidate が `/foo/bar-2::key` にもヒット)。`invalidate()` (exact) に変更。
2. `applyAiWorkspaceRename` の no-op 判定コメント改善
ファイル: `packages/host-service/src/trpc/router/workspace-creation/utils/ai-workspace-names.ts`
cloud 側の atomic WHERE guard が発火した時、`updateNameFromHost` は pre-update row を返すので `cloudResult.branch` が old branch のままになる。その挙動を前提にした `branch !== deduped` 判定が何を見ているか、なぜそれで正しいかを明文化。
3. `updateNameFromHost` の早期 return コメント改善
ファイル: `packages/trpc/src/router/v2-workspace/v2-workspace.ts`
WHERE guard 不一致時に返す row が「pre-update 時点で fetch した row で、fresh read ではない」ことを明示。Codex arch P2 の誤読余地指摘への対応。
4. plans/done への移動
レビュー結果サマリ (全 6 agent)
残 follow-up (別対応)
Test plan
Summary by CodeRabbit
バグ修正
内部改善