fix(issue): ensure --into flag is resolved with multiple positional args - #1256
Conversation
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bccec76. Configure here.
There was a problem hiding this comment.
This change is regressing correct behavior — I'd recommend closing it rather than merging.
The premise doesn't match --into's design. The flag's own brief says it "must match one of the provided IDs": --into selects which of the provided issues becomes the canonical parent. It is not a way to add another issue to the merge set. So sentry issue merge CLI-A CLI-B --into CLI-C (with CLI-C not among the positionals) is meant to error — the fix is merge CLI-A CLI-B CLI-C --into CLI-C.
It breaks 4 existing tests in test/commands/issue/merge.func.test.ts (this is the failing Unit Tests check):
--into rejects a value that doesn't match any provided issue— this test intentionally asserts that--into CLI-XYZwith positionalsCLI-A CLI-Bis rejected. This PR appendsCLI-XYZintoeffectiveArgs, so it's no longer rejected.1 positional + --into same issue triggers dedupe guard—merge CLI-A --into CLI-Ashould reach the clearer "at least 2 distinct issues" error. The!args.includes(flags.into)guard suppresses the append here, so it falls back to the generic "needs at least 2 issue IDs" message (this is also what Bugbot flagged above).--into swallows ResolutionError as clean not-foundandfast-path matches short IDs case-insensitively— both break becauseargs.includes(flags.into)is a strict string compare that misses case/alias/org-qualified forms, changing how many issues get resolved.
main already handles the single-issue convenience form correctly with args.length === 1 && flags.into ? [...args, flags.into] : args, and it also correctly produces the distinct-issues error for merge CLI-A --into CLI-A. The change here (flags.into && !args.includes(flags.into)) is what introduces the regressions.
If the underlying Sentry issue (CLI-1AF) was a user running merge A B --into C and getting a confusing error, the right fix would be improving that error message to suggest adding C as a positional — not changing --into to silently pull in an unlisted issue. Closing this in favor of that approach.
|
Jared, we want this new behavior so let's get the build fixed |
|
Jared, listen to @BYK 😅 |
--into now designates an issue that always participates in the merge (as the preferred parent), even when it is not one of the positional args. Previously merge CLI-A CLI-B --into CLI-C dropped CLI-C whenever 2+ positionals were given, so orderForMerge could not find it and threw a 'did not match any of the provided issues' error (CLI-1AF). The target is appended to the arg list unconditionally; duplicate forms of the same issue (positional + --into) collapse via numeric-ID dedupe in resolveAllIssues. Updated the merge tests to cover the new semantics. Fixes CLI-1AF
bccec76 to
512bf6a
Compare
|
Done. The build was failing on I reworked the change to match the behavior @BYK wants: Also rebased onto |
Codecov Results 📊✅ Patch coverage is 100.00%. Project has 5492 uncovered lines. Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 81.56% 81.50% -0.06%
==========================================
Files 428 428 —
Lines 29661 29694 +33
Branches 19429 19435 +6
==========================================
+ Hits 24194 24202 +8
- Misses 5467 5492 +25
- Partials 2024 2029 +5Generated by Codecov Action |

Previously, the
--intoflag was only appended to the list of effective arguments if exactly one positional argument was provided tosentry issue merge. This meant that if a user ran a command likesentry issue merge CLI-A CLI-B --into CLI-C, theCLI-Cissue specified by--intowould not be included in the issues resolved for merging.This omission caused
orderForMergeto fail, as it would attempt to findCLI-Camong onlyCLI-AandCLI-B, leading to aValidationErrorstating that `--into 'CLI-C' did not match any of the provided issues.This change modifies the logic for constructing
effectiveArgsto always appendflags.intoif it is present and not already included in the positional arguments, regardless of the number of positional arguments. This ensures that the--intotarget is correctly resolved and considered during the merge operation.Fixes CLI-1AF
Comment
@sentry <feedback>on this PR to have Autofix iterate on the changes.