Skip to content

fix(e2e): dispatch mobile PR E2E against default-branch workflow - #95

Closed
yasserfaraazkhan wants to merge 1 commit into
masterfrom
matterwick/mobile-e2e-dispatch-from-default
Closed

fix(e2e): dispatch mobile PR E2E against default-branch workflow#95
yasserfaraazkhan wants to merge 1 commit into
masterfrom
matterwick/mobile-e2e-dispatch-from-default

Conversation

@yasserfaraazkhan

@yasserfaraazkhan yasserfaraazkhan commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Closed: release-* PRs should behave like any other PR (dispatch with ref=<PR head>). No Matterwick special-case. The fix is backporting current e2e-detox-pr.yml / E2E CI onto the release line so the head accepts five-server inputs.

Release cherry-pick heads often lack five-server workflow inputs, so
dispatching with ref=PR head returns 422. Use ref=main for mobile PR
dispatches (MOBILE_VERSION stays the PR SHA) and cancel in-flight runs
by run-name "E2E PR <n>" instead of head_branch.

Co-authored-by: Cursor <cursoragent@cursor.com>
@mm-cloud-bot

Copy link
Copy Markdown

@yasserfaraazkhan: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

I understand the commands that are listed here

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Mobile E2E dispatches now use the main workflow ref while preserving the PR branch in version_name. Workflow cancellation matches mobile runs by display_title and desktop runs by branch.

Changes

Mobile E2E workflow handling

Layer / File(s) Summary
Mobile workflow dispatch inputs
server/e2e_tests.go, server/e2e_dryrun_test.go
Mobile dispatches use main as the workflow ref and include version_name, pr_number, and run_type. Tests verify these values.
Workflow-run cancellation matching
server/e2e_tests.go
Cancellation reads display_title, skips non-running runs, matches mobile runs by E2E PR <number>, and retains branch matching for desktop runs.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Matterwick
  participant GitHubAPI
  participant MobileWorkflow
  Matterwick->>GitHubAPI: Dispatch mobile workflow from main
  Matterwick->>GitHubAPI: Send version_name, pr_number, and run_type
  GitHubAPI->>MobileWorkflow: Start E2E PR workflow
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the mobile PR E2E workflow dispatch change.
Description check ✅ Passed The description directly explains the mobile E2E dispatch, cancellation changes, dependency, and test plan.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch matterwick/mobile-e2e-dispatch-from-default

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@server/e2e_tests.go`:
- Around line 1722-1734: Update the mobile workflow matching in the loop using
prTitlePrefix so PR numbers match exactly rather than sharing a prefix; require
the complete display title or its valid separator. Preserve desktop branch
matching, and add coverage for PR 1 not matching PR 10.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 54b9cf1d-60de-4613-9006-5cd7400489db

📥 Commits

Reviewing files that changed from the base of the PR and between 799a23b and 351c37b.

📒 Files selected for processing (2)
  • server/e2e_dryrun_test.go
  • server/e2e_tests.go

Comment thread server/e2e_tests.go
Comment on lines +1722 to +1734
// Mobile PR runs are dispatched with ref=main and run-name "E2E PR <n>", so
// match display_title (not head_branch == pr.Ref). Desktop still matches branch.
prTitlePrefix := fmt.Sprintf("E2E PR %d", pr.Number)
cancelCount := 0
for _, run := range workflowRuns.WorkflowRuns {
// Check if this run is for the PR's branch
if run.HeadBranch == pr.Ref && run.Status == "in_progress" {
cancelURL := fmt.Sprintf("/repos/%s/%s/actions/runs/%d/cancel",
pr.RepoOwner, pr.RepoName, run.ID)
if run.Status != "in_progress" {
continue
}
matches := run.HeadBranch == pr.Ref
if strings.Contains(pr.RepoName, "mobile") {
matches = strings.HasPrefix(run.DisplayTitle, prTitlePrefix)
}
if !matches {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Prevent cancellation of runs for a different PR.

strings.HasPrefix(run.DisplayTitle, "E2E PR 1") also matches "E2E PR 10". The cancellation can stop an in-progress workflow for another PR.

Match the complete title, or require the title separator that follows the PR number. Add a case for PR 1 and PR 10.

Proposed fix
  prTitlePrefix := fmt.Sprintf("E2E PR %d", pr.Number)
  cancelCount := 0
  for _, run := range workflowRuns.WorkflowRuns {
      if run.Status != "in_progress" {
          continue
      }
      matches := run.HeadBranch == pr.Ref
      if strings.Contains(pr.RepoName, "mobile") {
-         matches = strings.HasPrefix(run.DisplayTitle, prTitlePrefix)
+         matches = run.DisplayTitle == prTitlePrefix ||
+             strings.HasPrefix(run.DisplayTitle, prTitlePrefix+" ")
      }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Mobile PR runs are dispatched with ref=main and run-name "E2E PR <n>", so
// match display_title (not head_branch == pr.Ref). Desktop still matches branch.
prTitlePrefix := fmt.Sprintf("E2E PR %d", pr.Number)
cancelCount := 0
for _, run := range workflowRuns.WorkflowRuns {
// Check if this run is for the PR's branch
if run.HeadBranch == pr.Ref && run.Status == "in_progress" {
cancelURL := fmt.Sprintf("/repos/%s/%s/actions/runs/%d/cancel",
pr.RepoOwner, pr.RepoName, run.ID)
if run.Status != "in_progress" {
continue
}
matches := run.HeadBranch == pr.Ref
if strings.Contains(pr.RepoName, "mobile") {
matches = strings.HasPrefix(run.DisplayTitle, prTitlePrefix)
}
if !matches {
// Mobile PR runs are dispatched with ref=main and run-name "E2E PR <n>", so
// match display_title (not head_branch == pr.Ref). Desktop still matches branch.
prTitlePrefix := fmt.Sprintf("E2E PR %d", pr.Number)
cancelCount := 0
for _, run := range workflowRuns.WorkflowRuns {
if run.Status != "in_progress" {
continue
}
matches := run.HeadBranch == pr.Ref
if strings.Contains(pr.RepoName, "mobile") {
matches = run.DisplayTitle == prTitlePrefix ||
strings.HasPrefix(run.DisplayTitle, prTitlePrefix+" ")
}
if !matches {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/e2e_tests.go` around lines 1722 - 1734, Update the mobile workflow
matching in the loop using prTitlePrefix so PR numbers match exactly rather than
sharing a prefix; require the complete display title or its valid separator.
Preserve desktop branch matching, and add coverage for PR 1 not matching PR 10.

@yasserfaraazkhan

Copy link
Copy Markdown
Contributor Author

Closing: no Matterwick change needed — release cherry-pick PRs are normal PRs; dispatch stays ref=<PR head>. Fix is E2E CI on the release branch, not dispatching from main.

@yasserfaraazkhan
yasserfaraazkhan deleted the matterwick/mobile-e2e-dispatch-from-default branch August 4, 2026 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants