refactor(mobile): parse raw HTTP and infer review contracts - #5460
Merged
Conversation
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryIncremental review of the strict PR-number validation fix; the previous parseInt prefix issue is resolved and no new changed-code issues were found. Files Reviewed (4 files)
Previous Review Summaries (2 snapshots, latest commit 12df0a0)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 12df0a0)Status: No Issues Found | Recommendation: Merge Files Reviewed (8 files)
Previous review (commit bb0c285)Status: No Issues Found | Recommendation: Merge Files Reviewed (18 files)
Reviewed by grok-4.6 · Input: 53.9K · Output: 7.5K · Cached: 331.6K Review guidance: REVIEW.md from base branch |
This was referenced Aug 25, 2026
iscekic
marked this pull request as draft
August 25, 2026 17:09
…cts-dffd # Conflicts: # apps/mobile/src/lib/hooks/use-code-reviews.ts # apps/mobile/src/lib/pr-review/merge/merge-blocked-reasons.ts
iscekic
marked this pull request as ready for review
August 26, 2026 13:32
pandemicsyn
approved these changes
Aug 26, 2026
| const parsedOwner = parseParam(owner); | ||
| const parsedRepo = parseParam(repo); | ||
| const rawNumber = parseParam(number); | ||
| const numberValue = rawNumber ? Number.parseInt(rawNumber, 10) : Number.NaN; |
Contributor
There was a problem hiding this comment.
bot: The PR number is not strictly validated: Number.parseInt accepts numeric prefixes, so /pr-review/acme/repo/12abc or /1.5 passes Number.isInteger and opens PR 12 or 1 rather than the invalid-route state.
Suggested fix: Validate the full segment before converting it, for example require /^[1-9]\d*$/ and then use Number(rawNumber) with Number.isSafeInteger(numberValue). Add route tests covering 12abc, 1.5, and oversized values.
Number.parseInt stops at the first non-digit, so /pr-review/acme/repo/12abc and /1.5 opened PR 12 and PR 1 instead of the invalid-route state. Require the whole segment to be digits and stay exactly representable.
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
3 participants
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
Reviewer Notes
The scope-plus-platform reviewer contract is now a discriminated union. Its personal variant allows only GitHub and GitLab, so a personal Bitbucket route cannot exist, and
toPersonalPlatformthrows on Bitbucket instead of rewriting it to GitHub. The review-config typesReviewConfigDataandConfigPatchnow derive from the tRPC router outputs instead of hand-copied shapes.Files
apps/mobile/src/lib/code-reviewer-config.ts— adds theReviewerScopePlatformunion,parseReviewerPlatform(returns the union or null), andtoPersonalPlatform(throws on Bitbucket); derivesReviewConfigDataandConfigPatchfrom thegetReviewConfigrouter outputs.apps/mobile/src/lib/hooks/use-reviewer-route-params.ts— returnsReviewerScopePlatform | nullthroughparseReviewerPlatform.apps/mobile/src/lib/hooks/use-code-reviewer.ts— guardstoPersonalPlatformbehindisPersonalso the disabled org query never throws, re-exports it, and consumes the inferred config types.Four raw HTTP responses are now Zod-parsed instead of cast. Each endpoint gets an exported schema, so an invalid JSON shape throws a parse error rather than silently casting to the expected type. A malformed server reply now surfaces as an error instead of being swallowed.
Files
apps/mobile/src/lib/auth/admission.ts— addsAdmissionChallengeResponseSchemaand parses the admission-challenge reply.apps/mobile/src/components/agents/mobile-session-manager.ts— addsStreamTicketResponseSchemaand parses the stream-ticket reply; keeps the required-field checks forticketandexpiresAtafter parse.apps/mobile/src/lib/hooks/use-available-models.ts— addsOpenRouterModelsResponseSchemaandOrganizationDefaultsResponseSchema; parses the models-list and org-defaults replies.The pull-request review route now validates its path segments. Owner, repository, and number are strictly parsed, and a missing or malformed value renders the invalid-route state instead of forwarding a broken value into the screen.
Files
apps/mobile/src/app/(app)/pr-review/[owner]/[repo]/[number]/index.tsx— parses owner and repository viaparseParamand the number as a positive integer; rendersInvalidRouteStateon a bad value.The copied merge and review data shapes now derive from the tRPC router.
PrOverviewDto, the merge result, the merge and review-comment inputs, and the check-run type useinferRouterOutputsorinferRouterInputs, so hand-copied shapes cannot drift from the server procedures. The repo-settings type drops the unusedviewerLoginfield.Files
apps/mobile/src/lib/pr-review/merge/merge-blocked-reasons.ts— derivesPrOverviewDtoandPrOverviewRepoSettingsfromgetPullRequest; drops the copied DTO and theviewerLoginfield.apps/mobile/src/lib/pr-review/merge/merge-result-gate.ts— basesMergePullRequestResulton the inferredmergePullRequestoutput and re-adds thebranchDeleteErrorvariant.apps/mobile/src/lib/pr-review/merge/use-pr-merge-mutations.ts— infersMergePullRequestInputfrom themergePullRequestprocedure.apps/mobile/src/lib/pr-review/use-pr-review-mutations.ts— infers the comment and submit inputs from thegithubPrReviewprocedures.apps/mobile/src/components/pr-review/merge/pr-merge-sheet.tsx— infers the merge and auto-merge inputs from the router.apps/mobile/src/components/pr-review/pr-review-checks-section.tsx— derivesCheckRunfromlistChecks.apps/mobile/src/lib/hooks/use-code-reviews.ts— infersCreateManualReviewInputfromcreateManualReviewJob.Tests: 4 files updated —
code-reviewer-config.test.ts,admission.test.ts,use-available-models.test.ts,mobile-session-manager.test.ts.Generated: none.
Verification
Human steps: none. The merged code needs no new environment values, secrets, migrations, or flag flips.
Visual Changes
Visual Changes: N/A
Stacked PRs — merge bottom to top. Each level shows only its own diff.
Full verification (E2E, user advocacy, simplify, bot review) runs on the tip PR over every level.
A finding on a level is repaired on that level, then carried upward with stack.sh forward.
audit-w7b-data-contracts-dffd— refactor(mobile): parse raw HTTP and infer review contracts #5460 ← this PRaudit-w7b-data-contracts-dffd-s2— feat(web): add org credit and invoice page procedures #5462audit-w7b-data-contracts-dffd-s3— feat(mobile): owner-keyed credits, ledger paging, list freshness #5468audit-w7b-data-contracts-dffd-s4— refactor: route-scoped registry and live custom mode #5472audit-w7b-data-contracts-dffd-s5— feat(mobile): provider-aware new-session and layout extract #5479 (tip)