fix(opencode): accept leading # in opencode pr <number>#32261
Conversation
`opencode pr anomalyco#992` printed "Fetching and checking out PR #NaN" and failed because the `number` positional is typed `number`, so yargs coerces `anomalyco#992` to NaN. `gh pr checkout` accepts a leading `#`, so opencode should too. Type the positional as a string and parse it with `parsePrNumber`, which strips one optional leading `#`, accepts a positive integer, and fails with a clear message otherwise.
github-actions
Bot
added
the
needs:compliance
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds robust parsing/validation for PR numbers in the pr CLI command (including #123 and whitespace), and verifies behavior with new tests.
Changes:
- Introduced
parsePrNumber()helper to normalize and validate PR number input. - Updated
prcommand argument type fromnumbertostringand added a validation failure path. - Added Bun tests covering accepted/rejected PR number formats.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/opencode/test/cli/cmd/pr.test.ts | Adds unit tests for PR number parsing behavior. |
| packages/opencode/src/cli/cmd/pr.ts | Implements PR number parsing/validation and updates CLI arg handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const prNumber = args.number | ||
| const prNumber = parsePrNumber(args.number) | ||
| if (prNumber === undefined) { | ||
| return yield* fail(`Invalid PR number: ${args.number}. Pass a positive integer, optionally prefixed with '#'.`) |
| type: "string", | ||
| describe: "PR number to checkout", | ||
| demandOption: true, |
| test("rejects non-numeric, empty, zero, negative, decimal, and double-hash input", () => { | ||
| expect(parsePrNumber("#")).toBeUndefined() | ||
| expect(parsePrNumber("")).toBeUndefined() | ||
| expect(parsePrNumber("abc")).toBeUndefined() | ||
| expect(parsePrNumber("0")).toBeUndefined() | ||
| expect(parsePrNumber("-3")).toBeUndefined() | ||
| expect(parsePrNumber("12.5")).toBeUndefined() | ||
| expect(parsePrNumber("99x")).toBeUndefined() | ||
| expect(parsePrNumber("##992")).toBeUndefined() | ||
| }) |
|
This pull request has been automatically closed because it was not updated to meet our contributing guidelines within the 2-hour window. Feel free to open a new pull request that follows our guidelines. |
github-actions
Bot
removed
the
needs:compliance
Fixes #32251
opencode pr #992printedFetching and checking out PR #NaNand failed, because thenumberpositional is typednumberso yargs coerces#992toNaN.gh pr checkoutaccepts a leading#, soopencode prshould too.This types the positional as a string and adds
parsePrNumber, which strips one optional leading#, accepts a positive integer, and fails with a clear message otherwise.opencode pr 992andopencode pr #992now both resolve to992.How I verified
packages/opencode/test/cli/cmd/pr.test.tscovering bare,#-prefixed, and whitespace input plus rejected cases (#, empty, non-numeric,0, negative, decimal, double-#).bun test test/cli/cmd/pr.test.ts→ 4 pass.bun run typecheckpasses;oxlintreports no new findings.