-
Notifications
You must be signed in to change notification settings - Fork 35
Fix PR creation logic #920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment @cursor review or bugbot run to trigger another review on this PR
Replaces undefined return with OpenPrResult discriminated union type: - Distinguishes failure reasons: already_exists, validation_error, permission_denied, network_error, unknown - Enables caller to handle specific error cases appropriately - Provides detailed error information including validation details This foundational change enables better error handling in subsequent commits.
Previously, the socket fix command would only create a PR once and then never again because it checked for branch existence instead of PR existence. Since branches persist after PR creation, subsequent runs would skip PR creation entirely. Changes: - Check for open PR existence instead of branch existence - Add stale branch cleanup before creating new branches - Move GitHub token check before git operations for early validation - Integrate cleanup functions for proper branch lifecycle management - Add comprehensive error handling for all PR creation failure types - Remove unused gitDeleteRemoteBranch import This ensures PRs can be recreated after being closed/merged and prevents branch accumulation from failed PR attempts.
Extract branch cleanup operations into a separate module to improve code organization and testability. Each cleanup function has a specific purpose based on the branch lifecycle stage. Added functions: - cleanupStaleBranch: Delete both remote and local for branches without PRs - cleanupFailedPrBranches: Clean up after PR creation failure - cleanupSuccessfulPrLocalBranch: Remove only local branch after PR success - cleanupErrorBranches: Handle cleanup after unexpected errors Tests: - 8 unit tests with mocked git functions - 5 integration tests with real git repositories in tmpdir
db211c9 to
e1fbda4
Compare
|
@cursor review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no bugs!
Comment @cursor review or bugbot run to trigger another review on this PR
Fixes bug where PRs fail to be created on subsequent runs when branch exists from previous failed attempts.
Changes:
Note
Checks for existing open PRs, cleans up stale branches, adds structured PR creation errors, and integrates new branch cleanup utilities with tests; updates reach help snapshots.
src/commands/fix/coana-fix.mts):cleanupStaleBranch.openSocketFixPrresult for detailed failure handling (already exists, validation, permission, network, unknown) and clean up branches accordingly.cleanupSuccessfulPrLocalBranch.cleanupErrorBranches; reset back to base branch.src/commands/fix/branch-cleanup.mts):cleanupStaleBranch,cleanupFailedPrBranches,cleanupSuccessfulPrLocalBranch,cleanupErrorBrancheswith unit and integration tests.src/commands/fix/pull-request.mts):openSocketFixPrnow returns a discriminatedOpenPrResultwith explicit error reasons and logging.--reach-concurrencyand--reach-disable-analysis-splittingand ordering tweaks.spawnCoanaDlxlocal-path execution formatting/refactor insrc/utils/dlx.mts.Written by Cursor Bugbot for commit e1fbda4. Configure here.