Skip to content

Fix cross-repo inbound deeplinks and iOS HTTPS send-to-Mac#383

Merged
arul28 merged 3 commits into
mainfrom
cursor/critical-correctness-bugs-13a3
May 28, 2026
Merged

Fix cross-repo inbound deeplinks and iOS HTTPS send-to-Mac#383
arul28 merged 3 commits into
mainfrom
cursor/critical-correctness-bugs-13a3

Conversation

@cursor
Copy link
Copy Markdown
Contributor

@cursor cursor Bot commented May 28, 2026

Bug and impact

Wrong-project lane creation from inbound branch/PR deeplinks

When multiple projects are open, OS-routed deeplinks (ade:// / second-instance) dispatch to the focused window. InboundDeeplinkModal can create a lane from a PR in repo A while the active project’s origin points at repo B. If the wrong checkout has a colliding branch name on origin, preflight could pass and import the wrong branch into the wrong workspace.

iOS “Send to Mac” broken for HTTPS open links

iOS forwards both ade:// and https://ade.app/open?... URLs, but deeplinks.open rejected non-ade: protocols. Users saw “Sent” on iOS while desktop never navigated.

Local runtime stale connection after child exit

When an app-owned ade serve child exited without a socket disconnect callback, activeClient and projectsByRoot were left populated while connection was cleared, so status could read “connected” while RPC failed.

Root cause

  • createLaneFromPrBranch preflight fetched PR metadata for the deeplink repo but ran git checks against the active projectRoot without verifying the project’s GitHub remote matched.
  • syncRemoteCommandService used a raw URL.protocol === "ade:" gate instead of shared parseDeeplink (which already accepts HTTPS open URLs).
  • localRuntimeConnectionPool child exit handler only nulled connection.

Fix and validation

  • Block preflight with new project_repo_mismatch when getRepoOrThrow() disagrees with explicit deeplink repo args.
  • Validate deeplinks.open URLs via parseDeeplink; pass the original URL through to desktop dispatch.
  • Clear activeClient and projectsByRoot on owned-child exit/error.
  • Tests: prService.createLaneFromPrBranch cross-repo block case; existing pool tests pass.
  • npm run typecheck in apps/desktop and apps/ade-cli.
Open in Web View Automation 

ADE   Open in ADE  ·  cursor/critical-correctness-bugs-13a3 branch  ·  PR #383

Greptile Summary

This PR fixes three independent bugs: a cross-repo lane import guard, iOS HTTPS deeplink support, and stale connection state after a child-process exit. The fixes are self-contained and well-tested.

  • Cross-repo guard (prService.ts, prs.ts): buildCreateLaneFromPrBranchPreflight now calls getRepoOrThrow() before fetching PR data and returns a project_repo_mismatch block when the deeplink's owner/repo differs from the active project's GitHub remote; case-insensitive comparison prevents false positives on GitHub's case-preserving but case-insensitive namespace.
  • iOS HTTPS deeplinks (syncRemoteCommandService.ts): The raw URL.protocol === \"ade:\" gate is replaced with parseDeeplink, and the original URL string is forwarded to dispatchDeeplinkUrl rather than a canonicalized .toString(), preserving HTTPS form for downstream handlers.
  • Stale connection cleanup (localRuntimeConnectionPool.ts): A new ownedRuntimeChild field tracks the currently-owned ChildProcess; the new clearCurrentChildState closure guards against superseded-child exits by identity-checking against child, then atomically nulls connection/activeClient/ownedRuntimeChild, clears projectsByRoot, and calls closeRuntimeClient.

Confidence Score: 5/5

Safe to merge — all three fixes are narrowly scoped, identity-guarded against state races, and backed by new integration tests.

The cross-repo guard short-circuits before any network call and falls back gracefully when the project remote is unresolvable. The ownedRuntimeChild identity check in clearCurrentChildState correctly prevents a superseded child exit from wiping a live replacement connection, and closeRuntimeClient wraps client.close() in try/catch so the event handler cannot throw into Node uncaught-exception path. The deeplink URL validation delegates to the already-tested parseDeeplink shared parser, and the original URL string is forwarded rather than a canonicalized form. No pre-existing contracts are broken.

No files require special attention.

Important Files Changed

Filename Overview
apps/desktop/src/main/services/prs/prService.ts Adds cross-repo mismatch guard before PR fetch; uses case-insensitive comparison, silently falls back on getRepoOrThrow failure with a clear comment.
apps/desktop/src/shared/types/prs.ts Adds project_repo_mismatch to the CreateLaneFromPrBranchBlockCode union type; minimal, correct change.
apps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.ts Adds ownedRuntimeChild field and clearCurrentChildState closure; identity guard prevents superseded-child exits from clearing replacement connections; closeRuntimeClient is already wrapped in try/catch.
apps/ade-cli/src/services/sync/syncRemoteCommandService.ts Replaces raw protocol check with parseDeeplink; adds formatDeeplinkParseError helper covering all ParseError variants; passes original URL string to dispatcher to preserve HTTPS form.
apps/desktop/src/main/services/prs/prService.test.ts Adds cross-repo block test; verifies blockingConflict.code === project_repo_mismatch and confirms no GitHub API or branch import calls are made.
apps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.test.ts Two new integration tests: one verifies state is cleared and client closed on child exit; the other verifies a superseded child exit leaves the replacement connection intact.
apps/desktop/src/main/services/sync/syncRemoteCommandService.test.ts Adds deeplinks.open to IOS_REMOTE_COMMAND_ACTIONS and adds tests for HTTPS URL acceptance and unsupported URL rejection.

Reviews (2): Last reviewed commit: "fix: address review cleanup details" | Re-trigger Greptile

@vercel
Copy link
Copy Markdown

vercel Bot commented May 28, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
ade Ignored Ignored Preview May 28, 2026 11:14pm

cursoragent and others added 2 commits May 28, 2026 18:44
Block createLaneFromPrBranch when the requested GitHub repo does not match
the active project's origin remote, preventing lane imports into the wrong
checkout when a deeplink targets another repository.

Accept https://ade.app/open URLs in sync deeplinks.open (iOS Send to Mac)
by validating with parseDeeplink instead of requiring ade:// only.

Clear local runtime client state when an owned child process exits.

Co-authored-by: Arul Sharma <arul28@users.noreply.github.com>
@arul28 arul28 force-pushed the cursor/critical-correctness-bugs-13a3 branch from d24cbdf to 418fc50 Compare May 28, 2026 22:46
@arul28
Copy link
Copy Markdown
Owner

arul28 commented May 28, 2026

@copilot review but do not make fixes

@arul28 arul28 marked this pull request as ready for review May 28, 2026 22:46
@arul28 arul28 self-requested a review as a code owner May 28, 2026 22:46
Copy link
Copy Markdown
Contributor Author

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Scope: 7 file(s), +250 / −12
Verdict: Looks good

This PR fixes cross-repo inbound deeplinks and iOS “Send to Mac” by accepting validated https://ade.app/open URLs over sync, blocks createLaneFromPrBranch when the deeplink repo does not match the active project’s GitHub remote, and clears local-runtime client state only when the exiting child is still the pool’s owned runtime.


Notes

  • Deeplink validation: deeplinks.open now uses shared parseDeeplink, which allowlists ade.app, enforces /open, and validates target shape before dispatch — a tighter boundary than the previous ade://-only check and a good reuse of existing parser tests.
  • Cross-repo lane guard: project_repo_mismatch is enforced in buildCreateLaneFromPrBranchPreflight and therefore applies to both preflight and createLaneFromPrBranch (canCreate: false → throw). UI surfaces via blockingConflict.message in InboundDeeplinkModal / GitHubTab.
  • Runtime cleanup: ownedRuntimeChild identity guard correctly prevents a superseded spawn’s exit handler from wiping a replacement connection; tests cover both stale-clear and no-clear paths.
  • Intentional gap (unchanged): When githubService.getRepoOrThrow() fails, projectRepo stays null and the repo-mismatch block is skipped — same as pre-PR behavior for projects without a resolvable GitHub remote; navigation UX still relies on CrossRepoPrBanner for the view-only case.
Open in Web View Automation 

Sent by Cursor Automation: BUGBOT in Versic

Comment thread apps/ade-cli/src/services/sync/syncRemoteCommandService.ts
Comment thread apps/desktop/src/main/services/prs/prService.ts
@capy-ai
Copy link
Copy Markdown

capy-ai Bot commented May 28, 2026

Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

@arul28
Copy link
Copy Markdown
Owner

arul28 commented May 28, 2026

@copilot review but do not make fixes

@arul28 arul28 merged commit 9429c35 into main May 28, 2026
27 of 28 checks passed
@arul28 arul28 deleted the cursor/critical-correctness-bugs-13a3 branch May 29, 2026 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants