Skip to content

test(mobile): pairing link test runs the app's real link parsing - #26326

Merged
juliusmarminge merged 1 commit into
mainfrom
t3/mobile-pairing-link-test-real-pipeline
Oct 28, 2026
Merged

juliusmarminge merged 1 commit into
mainfrom
t3/mobile-pairing-link-test-real-pipeline

Conversation

@juliusmarminge

Copy link
Copy Markdown
Member

Follow-up to https://git.cafe/pingdotgg/t3code/pulls/26318 (c0f9e36), which strips routeFor from pairing deep links so a link can't turn an untargeted pairing into an "Add route" on a saved environment.

Problem. The test for that guard built its own route config and called withoutPairingTarget directly. It stayed green if someone removed the strip from the link-parsing pipeline in App.tsx. Its last assertion (toHaveBeenLastCalledWith(PAIRING_URL)) also didn't check that the strip happened.

Fix. getAppStateFromPath moves out of App.tsx into apps/mobile/src/lib/appLinkState.ts. Both App.tsx and the new test import it, because App.tsx itself can't load under Node. The new test sends connections/new?routeFor=victim&pairingUrl=…&autoConnect=1 and settings/environment-new?… through the real pipeline and React Navigation's real getStateFromPath. It asserts that the pairing form's params have no routeFor and that auto-connect pairs exactly once with the URL. The old helper-only test in devAutoConnect.test.ts is removed.

Mocks stay at the native boundary: root navigation, the pairing call, the workspace store, and the stores that prompt links read. The path config is written out to match Stack.tsx (settings/environment-new and connections/new are root routes), because RootStack imports every screen and can't load in a Node test.

Proved the test. I replaced the withoutPairingTarget(...) call in the pipeline with an identity function. The test then failed with routeFor: "victim" in the received params. After restoring the call, it passed. vp run check:changed passes.

Done by Claude Opus 5.5 (claude-opus-5-5) in Claude Code, running in T3 Code.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 28, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 80c939f

Macroscope's review found this PR approvable — The mobile link parser moves into a shared module while retaining its existing processing order and application callers. The assessed change introduces no material behavior or product-default change.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between c0f9e36 and 80c939f.

Files selected for processing: 4
  • apps/mobile/src/App.tsx
  • apps/mobile/src/features/connection/devAutoConnect.test.ts
  • apps/mobile/src/lib/appLinkState.test.ts
  • apps/mobile/src/lib/appLinkState.ts
Files with no reviewable changes: 1
  • apps/mobile/src/features/connection/devAutoConnect.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

Walkthrough

The mobile app now parses deep links through getAppStateFromPath in appLinkState.ts. The helper applies the existing link-state handlers in sequence. Pairing-link tests verify that routeFor is omitted from linked route parameters while pairingUrl and autoConnect remain.

Changes: Mobile app links

Layer / File(s) Summary
Shared app-link parser and integration
apps/mobile/src/lib/appLinkState.ts, apps/mobile/src/App.tsx
getAppStateFromPath parses paths and applies the link-state handlers. App.tsx imports this helper instead of defining the parser locally.
Pairing-link route validation
apps/mobile/src/lib/appLinkState.test.ts, apps/mobile/src/features/connection/devAutoConnect.test.ts
The app-link-state test checks pairing parameters for both supported routes. The corresponding test is removed from devAutoConnect.test.ts.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Possibly related PRs

Suggested reviewers: macroscopeapp

Merge Risk: Minimal · up to 80c93

The shared parser test covers both pairing routes. No merge-blocking risk was identified.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title names the main change: a pairing link test now runs the app's real link parsing. It matches the changeset, which moves getAppStateFromPath into lib/appLinkState.ts and adds a test that sends…
Description check Passed The description follows the template. It has a Problem section that states the gap in the old test. It has a Change section that explains the move of getAppStateFromPath and the new test. It names the…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the main change: a pairing link test now runs the app's real link parsing. It matches the changeset, which moves getAppStateFromPath into lib/appLinkState.ts and adds a test that sends pairing links through the real pipeline. The title is in conventional commit form and is short, clear, and specific.

Full details: Description check

Explanation

The description follows the template. It has a Problem section that states the gap in the old test. It has a Change section that explains the move of getAppStateFromPath and the new test. It names the related PR #26318 in the Scope context, which links the work to prior approved direction. The Verification section gives concrete checks: the mutation test (identity replacement made the test fail with routeFor: "victim"), and vp run check:changed passing. The template's Scope and approval section is not filled in with an explicit approval link or exemption rationale. The author cites a follow-up to PR #26318 but does not state a triaged issue or maintainer approval, and the description does not say why this fix qualifies without a prior issue. The description is mostly complete, so the check passes, but the author should add the scope and approval justification.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@juliusmarminge
juliusmarminge merged commit 0deb383 into main Oct 28, 2026
31 checks passed
@juliusmarminge
juliusmarminge deleted the t3/mobile-pairing-link-test-real-pipeline branch October 28, 2026 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant