refactor: exit 64 for ambiguous partial_match input (#237)#239
Merged
refactor: exit 64 for ambiguous partial_match input (#237)#239
Conversation
issue move, issue link, and issue unlink reported ambiguous-substring errors via anyhow::bail! which doesn't inject a JrError into the cause chain, so main.rs fell back to exit 1. issue list --status already used JrError::UserError (exit 64) for the same failure class, producing an inconsistency that broke scripts keying off $? == 64. Change the three Ambiguous + no_input branches to return JrError::UserError so all four handlers surface exit 64 uniformly. Error-message strings are byte-identical — no script-visible wording changes. Updates the two tests added in PR #238 to pin Some(64). Closes #237
There was a problem hiding this comment.
Pull request overview
This PR standardizes exit behavior for partial_match ambiguity errors across issue move, issue link, and issue unlink so they consistently surface as JrError::UserError (exit code 64) rather than anyhow::bail! (exit code 1), aligning with existing behavior in issue list --status.
Changes:
- Switch ambiguous-substring
--no-inputbranches fromanyhow::bail!toJrError::UserError(...)in move/link/unlink handlers. - Update integration tests to assert exit code
64for the ambiguous-substring cases inmoveandlink.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/cli/issue/workflow.rs |
Return JrError::UserError on ambiguous transition under --no-input to produce exit 64. |
src/cli/issue/links.rs |
Return JrError::UserError on ambiguous link type under --no-input for both link and unlink. |
tests/issue_commands.rs |
Update pinned exit-code assertions from 1 to 64 for the relevant integration tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Zious11
added a commit
that referenced
this pull request
Apr 21, 2026
#236) (#241) Adds handler-level (or library-level where plumbing outweighs signal) coverage for three more partial_match callers identified in #236: - issue unlink (tests/issue_commands.rs — binary test, exit 64) - queue resolve (tests/queue.rs — library test on resolve_queue_by_name) - assets schema (tests/assets.rs — binary test, --schema ambiguity) Each uses a single-hit substring input (per the #193 regression surface) and asserts the Ambiguous branch fires with the expected disambiguation message. The binary tests pin exit 64 to complement PR #239's bail!→UserError unification. The unlink test mounts no DELETE mock so wiremock 404s any state-change attempt; the assets schema test mounts both objecttypes/flat endpoints with .expect(0) to lock the short-circuit. The remaining 5 call sites from #236 (helpers.rs team/user/asset, assets search --status, assets schema <type_name>) need CMDB/user/ team plumbing or await a parallel bail!→UserError refactor of helpers.rs. Tracked in #240. Refs #236
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
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.
Summary
partial_matchhandlers: all ambiguous-substring errors now exit 64 (JrError::UserError).move/link/unlinkusedanyhow::bail!→ exit 1.list --statusalready usedUserError→ exit 64.UserError→ exit 64."Ambiguous transition"or"Ambiguous link type"keep working.Why it matters
An AI agent or shell pipeline checking
\$? == 64for "bad user input" previously missed move/link/unlink ambiguity and treated them as transient errors. Now the contract is consistent.Changes
src/cli/issue/workflow.rs:157—bail!→JrError::UserError(Ambiguous transition,--no-input)src/cli/issue/links.rs:67— same (link, Ambiguous link type)src/cli/issue/links.rs:136— same (unlink, Ambiguous link type)tests/issue_commands.rs— flip pinnedSome(1)→Some(64)for the two tests added in PR test: integration tests for partial_match substring rejection (#193) #238, remove stale "currently exits 1" comments.Scope notes
bail!remains used elsewhere in both files (for non-user errors andNonebranches); import is still needed.MatchResult::None(_)branches (no-match-at-all) also exit 1 today viabail!. Out of scope here — refactor: unify exit codes for ambiguous input — move/link currently exit 1, list exits 64 #237 is specifically about ambiguity. Can be revisited if the same consistency concern applies.unlinkintegration test in this PR; that gap is tracked in test: extend substring-rejection handler tests to helpers/assets/queue call sites #236.Test plan
cargo fmt --all -- --checkcleancargo clippy --all-targets -- -D warningscleancargo test— 803 passed, 0 failedSome(64):test_move_single_substring_rejected_no_inputtest_link_single_substring_rejected_no_inputCloses #237