kitenite/halved position#2102
Merged
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a small auto-create-PR utility with tests, introduces GitHub PR URL utilities and tests, updates backend PR creation flow to build PR compare URLs (handling upstreams/forks), and adjusts frontend components/hooks to use the new utilities and open returned PR URLs. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Router as TRPC Router
participant Git as Git (local)
participant RepoMeta as Repo Metadata
participant Browser
Client->>Router: request create/open PR
Router->>Git: get current branch, tracking, config (gh-merge-base)
Router->>RepoMeta: query remote repo metadata (owner, default branch)
Router->>Router: parse upstream ref / normalize repo URL
Router->>Router: build compare URL (baseRepo, baseBranch, headOwner, headBranch)
Router->>Client: return { success:true, url }
Client->>Browser: open url in new tab
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (3 warnings)
✏️ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx:367">
P1: Bug: `hasGitHubRepo` check was dropped when extracting `shouldAutoCreatePRAfterPublish` into a utility. The original inline logic was `hasGitHubRepo && !isDefaultBranch && !hasExistingPR`, but the new function only checks `!hasExistingPR && !isDefaultBranch`. This means workspaces without a GitHub repo will now incorrectly trigger auto-PR creation after a branch publish.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| }); | ||
| const shouldAutoCreatePRAfterPublish = | ||
| hasGitHubRepo && !isDefaultBranch && !hasExistingPR; | ||
| const shouldAutoCreatePR = shouldAutoCreatePRAfterPublish({ |
Contributor
There was a problem hiding this comment.
P1: Bug: hasGitHubRepo check was dropped when extracting shouldAutoCreatePRAfterPublish into a utility. The original inline logic was hasGitHubRepo && !isDefaultBranch && !hasExistingPR, but the new function only checks !hasExistingPR && !isDefaultBranch. This means workspaces without a GitHub repo will now incorrectly trigger auto-PR creation after a branch publish.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx, line 367:
<comment>Bug: `hasGitHubRepo` check was dropped when extracting `shouldAutoCreatePRAfterPublish` into a utility. The original inline logic was `hasGitHubRepo && !isDefaultBranch && !hasExistingPR`, but the new function only checks `!hasExistingPR && !isDefaultBranch`. This means workspaces without a GitHub repo will now incorrectly trigger auto-PR creation after a branch publish.</comment>
<file context>
@@ -364,8 +364,10 @@ export function ChangesView({ onFileOpen, isExpandedView }: ChangesViewProps) {
});
- const shouldAutoCreatePRAfterPublish =
- hasGitHubRepo && !isDefaultBranch && !hasExistingPR;
+ const shouldAutoCreatePR = shouldAutoCreatePRAfterPublish({
+ hasExistingPR,
+ isDefaultBranch,
</file context>
Suggested change
| const shouldAutoCreatePR = shouldAutoCreatePRAfterPublish({ | |
| const shouldAutoCreatePR = hasGitHubRepo && shouldAutoCreatePRAfterPublish({ |
Contributor
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/lib/trpc/routers/changes/git-operations.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/changes/git-operations.ts:269">
P2: Empty catch block silently swallows errors from multiple git operations (upstream ref resolution and remote URL lookup). At minimum, log a warning with context so fork/upstream debugging is possible.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| `branch.${branch}.gh-merge-base`, | ||
| ]); | ||
| return configuredBaseBranch.trim() || null; | ||
| } catch { |
Contributor
There was a problem hiding this comment.
P2: Empty catch block silently swallows errors from multiple git operations (upstream ref resolution and remote URL lookup). At minimum, log a warning with context so fork/upstream debugging is possible.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/changes/git-operations.ts, line 269:
<comment>Empty catch block silently swallows errors from multiple git operations (upstream ref resolution and remote URL lookup). At minimum, log a warning with context so fork/upstream debugging is possible.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.) </comment>
<file context>
@@ -242,22 +242,92 @@ async function findOpenPRByHeadCommit(
+ `branch.${branch}.gh-merge-base`,
+ ]);
+ return configuredBaseBranch.trim() || null;
+ } catch {
+ return null;
+ }
</file context>
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.
Auto create PR on publish branch
Summary by CodeRabbit
New Features
Bug Fixes