Add AGENTS.md with Docker E2E setup + fix E2E login state race#3773
Open
yasserfaraazkhan wants to merge 36 commits into
Open
Add AGENTS.md with Docker E2E setup + fix E2E login state race#3773yasserfaraazkhan wants to merge 36 commits into
yasserfaraazkhan wants to merge 36 commits into
Conversation
Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds workflows and utilities to publish E2E server info and launch Cursor agents for failing E2E runs; disables legacy label-cleanup workflow; tightens E2E test synchronization on post-login renderer state; adds AGENTS.md with Cursor Cloud run and workflow security guidance. Changes
Sequence DiagramsequenceDiagram
participant WF as Workflow
participant Util as e2e/utils/github-actions.js
participant GH as GitHub API
participant Cursor as Cursor API
participant PR as Pull Request
WF->>Util: findPrNumber(context, prNumberInput)
Util->>GH: inspect workflow run / list PRs / lookup by head branch & sha
GH-->>Util: PR metadata or null
Util-->>WF: resolved prNumber
WF->>Util: postServerInfoComment(platforms, prNumber, server metadata)
Util->>GH: list PR comments (search marker)
GH-->>Util: existing comments
alt marker found
Util->>GH: update existing comment
else
Util->>GH: create new comment
end
GH-->>PR: comment posted/updated
WF->>GH: read workflow_run -> gather job conclusions
WF->>GH: read PR comment with server URLs (marker)
WF->>Cursor: POST launch-agent (prompt + server URLs + PR URL)
Cursor-->>WF: response (agentId / agentUrl)
WF->>GH: post PR comment announcing agent and CI link
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
When Matterwick provisions cloud Mattermost instances for a PR, the server URLs are now surfaced back to the PR as a comment immediately after the matrix is ready — while tests are still running. This lets developers connect to the exact same servers that are running the CI suite to reproduce and fix failing tests without waiting for the run to finish. Changes: - Add findPrNumber() helper to e2e/utils/github-actions.js, extracting the three-step PR resolution logic (explicit input → run.pull_requests → branch/SHA lookup) that was previously duplicated across removeE2ELabel and the remove-e2e-label job. - Add postServerInfoComment() to e2e/utils/github-actions.js. It builds a markdown table of per-platform server URLs, includes the admin username and server version, and adds a ready-to-run shell snippet. The comment is idempotent: a hidden HTML marker (<!-- e2e-server-info -->) is used to find and update an existing comment on re-runs rather than appending a new one each time. The admin password is intentionally omitted. - Add post-server-info job to e2e-functional.yml. It runs in parallel with update-initial-status after prepare-matrix succeeds, is skipped for nightly runs (no PR to comment on), and requires issues:write / pull-requests:write permissions. The top-level workflow permissions block is extended to include those two scopes. Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
When Matterwick provisions cloud Mattermost instances for a PR, the server URLs are now surfaced back to the PR as a comment immediately after the matrix is ready — while tests are still running. This lets developers connect to the exact same servers that are running the CI suite to reproduce and fix failing tests without waiting for the run to finish. Changes: - Add findPrNumber() helper to e2e/utils/github-actions.js, extracting the three-step PR resolution logic (explicit input → run.pull_requests → branch/SHA lookup) that was previously duplicated across removeE2ELabel and the remove-e2e-label job. - Add postServerInfoComment() to e2e/utils/github-actions.js. It builds a markdown table of per-platform server URLs, includes the admin username and server version, and adds a ready-to-run shell snippet. The comment is idempotent: a hidden HTML marker (<!-- e2e-server-info -->) is used to find and update an existing comment on re-runs rather than appending a new one each time. The admin password is intentionally omitted. - Add post-server-info job to e2e-functional.yml. It runs in parallel with update-initial-status after prepare-matrix succeeds, is skipped for nightly runs (no PR to comment on), and requires issues:write / pull-requests:write permissions. The top-level workflow permissions block is extended to include those two scopes. Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
Inputs interpolated directly into single-quoted JS string literals inside
an actions/github-script step could be escaped by a malicious dispatcher
(e.g. a value containing a single quote) to break out of the string and
run arbitrary JavaScript on the runner, which holds issues:write and
pull-requests:write permissions.
Fix: move all three user-controlled inputs (pr_number, MM_TEST_USER_NAME,
MM_SERVER_VERSION) out of the ${{ }} expression context and into env:
variables on the step, then read them via process.env.* inside the script.
The platforms JSON comes from a trusted internal job output (not a raw
workflow input) so its interpolation is safe and is left as-is.
Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
cursor
Bot
changed the title
Three issues raised in the CodeRabbit review of PR #3774: 1. Overly broad top-level permissions: remove issues:write and pull-requests:write from the workflow-level permissions block. Those scopes are already granted at job level on post-server-info and remove-e2e-label which are the only jobs that need them. 2. Comment posting not fault-tolerant: wrap postServerInfoComment in a try/catch inside the github-script step so a GitHub API error (e.g. rate limit, permissions) logs a warning but does not fail the post-server-info job and block the overall workflow run. 3. Loose PR number parsing in findPrNumber: replace parseInt which accepts '123abc' and negative values with strict validation — trim the input, test against /^\d+$/, then confirm the result is a positive integer before returning. Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
…iven fixes Matterwick destroys the provisioned cloud Mattermost servers as soon as the E2E/Run label is removed from a PR. To allow agents to connect to those servers and fix failing tests within the same PR run, both label removal paths are disabled: - e2e-functional.yml: remove-e2e-label job is commented out in full. - e2e-label-cleanup.yml: removal logic replaced with a no-op job that logs a message; the workflow trigger and permissions block are preserved so the file remains valid YAML and the workflow still runs (harmlessly). Re-enable both when automated fix-and-rerun is no longer needed. Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
Label removal:
- Disable remove-e2e-label job in e2e-functional.yml (commented out) so
Matterwick keeps provisioned servers alive after tests finish.
- Replace the remove-e2e-label job in e2e-label-cleanup.yml with a no-op
job for the same reason.
Servers are destroyed when the label is removed; keeping them alive lets
agents connect and fix failing tests in the same PR run.
Test fix — post-login tabsDisabled race (5 failing tests across all 3 OSes):
Root cause: MainPage.tabsDisabled is set to !currentServer.isLoggedIn.
After loginToMattermost() returns (web app shell ready), the
SERVER_LOGGED_IN_CHANGED IPC event still needs to travel from the server
WebContentsView through the main process ServerManager to the renderer
MainPage, where it triggers updateServers() which fetches the updated
currentServer.isLoggedIn value. Tests that called mainWindow.click('#newTabButton')
or mainWindow.waitForSelector('#newTabButton') immediately after
loginToMattermost() were racing this propagation and timing out because the
button was still disabled (tabsDisabled=true).
Fix: change the post-login wait in each affected beforeAll from
waitForSelector('#newTabButton') // present but possibly disabled
to
waitForSelector('#newTabButton:not([disabled])') // present AND enabled
Affected specs:
- e2e/specs/server_management/drag_and_drop.test.ts
- e2e/specs/server_management/popout_windows.test.ts
- e2e/specs/server_management/tab_management.test.ts
- e2e/specs/menu_bar/window_menu.test.ts
Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
Lint (Expected line before comment): - Add blank line before each explanatory comment added in the previous commit across drag_and_drop, popout_windows, tab_management, and window_menu test files. CodeRabbit review items (carried over from PR #3774 review of the original commit, now applied to this branch): - Remove issues:write / pull-requests:write from the workflow-level permissions block; those scopes already exist at job level on the post-server-info job. - Add continue-on-error: true to the Post provisioned server URLs step so a comment API failure never blocks the overall workflow. - Move workflow inputs (pr_number, MM_TEST_USER_NAME, MM_SERVER_VERSION) into env: vars and read them via process.env.* to prevent script injection (security fix from DryRun Security report). - Replace parseInt() in findPrNumber with strict /^\d+$/ + positive integer validation to reject partial strings and negative values. Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
Security (DryRun Security findings on automation-changed files only):
drs_40c108a1 — Code injection via expression interpolation:
`${{ needs.prepare-matrix.outputs.platforms }}` was interpolated
directly into a JS string literal inside actions/github-script. A
poisoned instance_details input could escape the shell command in
prepare-matrix, control the platforms output, and inject arbitrary JS.
Fix: pass PLATFORMS via env: and parse with JSON.parse(process.env.PLATFORMS)
so the value is never textually interpolated into the script body.
drs_e5221983 — Markdown injection in PR comments:
platform and url values from the platforms array were inserted raw into a
Markdown table in postServerInfoComment(). An attacker controlling
instance_details could inject pipe characters to break the table or embed
malicious links/mentions.
Fix: add sanitizeMd() helper in github-actions.js that escapes |, `,
[ and ] before inserting any platform-provided value into the comment body.
CodeRabbit review (on automation-changed files only):
- Remove inputs.pr_number gate from post-server-info job condition so
findPrNumber fallback resolution can run when pr_number is absent.
New condition: if: ${{ !inputs.nightly }}
- Remove ~60 lines of commented-out remove-e2e-label implementation from
e2e-functional.yml; replaced with a single explanatory comment line.
Git history preserves the full implementation.
- Fix e2e-label-cleanup.yml noop job to never allocate a runner:
change if condition to ${{ false }} and add permissions: {} so the job
is skipped entirely and consumes no permissions or compute.
Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
…gents-md-c5d4 Merges PR #3774 into the combined branch. Conflict resolution: e2e-functional.yml (4 conflicts): - post-server-info job condition: took PR #3774 side (no pr_number gate, allows findPrNumber fallback resolution) - Post step: took PR #3774 side (continue-on-error, PLATFORMS env var, JSON.parse instead of direct expression interpolation — security fix) - remove-e2e-label block: took PR #3774 side (single comment line instead of ~60 lines of commented-out dead code) e2e-label-cleanup.yml: took PR #3774 side entirely (if: false + permissions: {}) e2e/utils/github-actions.js: took PR #3774 side entirely (sanitizeMd injection protection, strict findPrNumber validation) Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
🧪 E2E Test Servers ReadyMatterwick has provisioned the following Mattermost instances for this PR. Admin username: Run a single spec against one of these servers: MM_TEST_SERVER_URL=<url above> \
MM_TEST_USER_NAME=<username above> \
MM_TEST_PASSWORD=<MM_DESKTOP_E2E_USER_CREDENTIALS secret> \
npx playwright test <spec-file> --reporter=list --workers=1
Workflow run: https://github.com/mattermost/desktop/actions/runs/24454531995 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Around line 13-15: Update the fenced code block in AGENTS.md to include the
bash language tag so markdown linter and readers recognize the snippet;
specifically change the block that contains `source ~/.nvm/nvm.sh && nvm use
20.15.0` to use ```bash as the opening fence (i.e., replace the current opening
``` with ```bash).
In `@e2e/utils/github-actions.js`:
- Line 194: Update the misleading description string "'> Servers are active for
the duration of this workflow run and destroyed afterwards.'" in
e2e/utils/github-actions.js to reflect that label cleanup is disabled and
servers may be retained after the run; replace the sentence with text like
"Servers are active for the duration of this workflow run and may be retained
afterwards (label cleanup disabled)" so the message matches current workflow
behavior.
- Around line 158-162: The current sanitizeMd function allows newline characters
and raw HTML-like content which can break GitHub comment tables; update
sanitizeMd (used by platformRows) to also escape or remove newline characters
(replace \n and \r with a space or escaped sequence) and escape HTML-sensitive
characters (&, <, >) in addition to pipes, backticks and brackets so neither
platform names nor URLs can inject table breaks or raw HTML; ensure platformRows
continues to call the updated sanitizeMd for both p.platform and p.url.
- Around line 121-123: The current logic finds a PR by matching
run.data.head_sha but falls back to prs.data[0], which can cause posting to the
wrong PR; change the return to only return the matched PR number (e.g., return
matching ? matching.number : null) instead of (matching || prs.data[0]).number,
and update any callers of this function to check for a null/undefined PR number
and skip posting comments when null; reference the variables matching,
prs.data[0], and run.data.head_sha to locate the code to change.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d686c302-d133-4228-8d51-9be4268513a4
📒 Files selected for processing (8)
.github/workflows/e2e-functional.yml.github/workflows/e2e-label-cleanup.ymlAGENTS.mde2e/specs/menu_bar/window_menu.test.tse2e/specs/server_management/drag_and_drop.test.tse2e/specs/server_management/popout_windows.test.tse2e/specs/server_management/tab_management.test.tse2e/utils/github-actions.js
CodeRabbit review fixes (automation-changed files only): AGENTS.md: - Add 'bash' language tag to the fenced code block containing the nvm command so markdown linters and readers recognize it as a shell snippet. e2e/utils/github-actions.js: 1. Comment out removeE2ELabel function body — entire implementation is commented out so the GitHub Actions master-branch cleanup job no longer removes the E2E/Run label after tests complete. Matterwick keeps the provisioned servers alive, allowing agents to connect and fix failures in the same run. Function signature is preserved; all callers continue to compile and run without errors. 2. sanitizeMd — extend to also strip/replace newline characters (\r, \n → space) and escape HTML-sensitive chars (&, <, >) in addition to the existing pipe/backtick/bracket escaping, preventing table-breaking and raw HTML injection in PR comments. 3. findPrNumber — remove fallback to prs.data[0] when no PR matches the run's head SHA; return null instead so the caller skips posting a comment rather than posting to the wrong PR. 4. postServerInfoComment — update the server lifetime note in the PR comment to reflect that label cleanup is disabled and servers may be retained after the run ends. Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
AGENTS.md (1)
26-26: Avoid hardcoded test suite/test counts in docs.
(73 suites, 1118 tests)will drift quickly and become stale; consider keeping this generic or linking to CI status instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` at line 26, Remove the hardcoded test counts from the documentation line that references the npm test command (the string "npm run test:unit" and its parent table row) and replace the parenthetical "(73 suites, 1118 tests)" with a generic description like "Run unit tests" or a link to the CI/build status badge; update the AGENTS.md table row so it references the test command only or points to CI for live counts.e2e/utils/github-actions.js (1)
239-309: Delete the commented-out label-removal implementation.The function is already a noop. Keeping the old body in comments leaves stale logic around, including code paths you intentionally backed away from, and makes future re-enablement easier to copy/paste incorrectly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/utils/github-actions.js` around lines 239 - 309, Remove the entire commented-out removeE2ELabel implementation block (the multi-line comment that defines async function removeE2ELabel and its try/catch logic) so the file only contains the noop behavior (the existing console.log('removeE2ELabel: commented out for testing purposes — label removal is disabled.')); ensure no leftover commented code referencing removeE2ELabel, github, context, run, prs, or related label-removal logic remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Line 3: Change the header text "Cursor Cloud specific instructions" to use a
hyphenated compound adjective: update it to "Cursor Cloud-specific instructions"
(replace the existing heading string) so the grammar and style are correct.
In `@e2e/utils/github-actions.js`:
- Around line 209-215: The current lookup uses github.rest.issues.listComments
with per_page:100 which only fetches a single page and can miss an existing
MARKER comment; update the logic that retrieves comments (the call to
github.rest.issues.listComments and the subsequent existing =
comments.find(...)) to paginate through all pages (e.g., use github.paginate or
iterate pages) for the given owner, repo, and prNumber, then search the
aggregated comments array for the MARKER to ensure you never create duplicate
marker comments.
---
Nitpick comments:
In `@AGENTS.md`:
- Line 26: Remove the hardcoded test counts from the documentation line that
references the npm test command (the string "npm run test:unit" and its parent
table row) and replace the parenthetical "(73 suites, 1118 tests)" with a
generic description like "Run unit tests" or a link to the CI/build status
badge; update the AGENTS.md table row so it references the test command only or
points to CI for live counts.
In `@e2e/utils/github-actions.js`:
- Around line 239-309: Remove the entire commented-out removeE2ELabel
implementation block (the multi-line comment that defines async function
removeE2ELabel and its try/catch logic) so the file only contains the noop
behavior (the existing console.log('removeE2ELabel: commented out for testing
purposes — label removal is disabled.')); ensure no leftover commented code
referencing removeE2ELabel, github, context, run, prs, or related label-removal
logic remains.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cf8de842-29f3-4f85-acc2-b7c96bbdb30e
📒 Files selected for processing (2)
AGENTS.mde2e/utils/github-actions.js
New file: .github/workflows/e2e-fix-trigger.yml
Fires on workflow_run:completed for 'Electron Playwright Tests'.
Reads failure counts, applies triage logic, then either:
- Skips with a PR comment if mass failure (all platforms failed or
total >= MASS_FAILURE_THRESHOLD=15) — prevents wasting tokens on
systemic build/infra breakages.
- Launches a Cursor cloud agent (POST /v0/agents) with source.prUrl
and target.autoBranch:false so fixes push to the PR's head branch
and re-trigger a new E2E run automatically.
The prompt tells the agent to:
- Fix test bugs (selector changes, race conditions, wrong assertions)
in e2e/ only, capped at 8 files per run.
- Post a PR comment for product bugs instead of modifying tests.
- Run each fixed spec against the live server before committing.
Requires CURSOR_API_KEY repository secret.
e2e/utils/github-actions.js (CodeRabbit fixes):
- removeE2ELabel: entire function body is commented out so the
GitHub Actions master-branch cleanup job no longer removes E2E/Run.
- sanitizeMd: also escapes \r/\n (→ space) and & < > (HTML entities)
to prevent table-breaking and raw HTML injection in PR comments.
- findPrNumber: return null instead of falling back to prs.data[0]
when no PR matches the run's head SHA.
- postServerInfoComment: update server lifetime note to reflect that
label cleanup is disabled and servers may be retained after the run.
AGENTS.md (CodeRabbit fix):
- Add 'bash' language tag to the nvm fenced code block.
Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
github-actions
Bot
added
E2E/Run
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/e2e-fix-trigger.yml:
- Around line 232-240: The "Launch Cursor agent to fix E2E failures" step emits
AGENT_URL via $GITHUB_OUTPUT but has no id and the downstream "Post agent launch
notice on PR" step incorrectly reads ${ env.AGENT_URL }; add an id (e.g., id:
launch_agent) to the launcher step and expose its output, then wire that output
into the PR-comment step by referencing steps.launch_agent.outputs.AGENT_URL (or
set env.AGENT_URL: ${{ steps.launch_agent.outputs.AGENT_URL }}) so the comment
uses the actual AGENT_URL instead of remaining "(pending)".
- Around line 243-306: The sed substitution breaks when SERVER_BLOCK contains
pipe characters because sed uses | as the delimiter; replace the two-step
write+sed flow with an unquoted heredoc that expands variables directly into
PROMPT to avoid delimiter issues. Specifically, stop writing
/tmp/agent-prompt.txt then running PROMPT=$(sed ... /tmp/agent-prompt.txt);
instead construct PROMPT with an unquoted here-doc (e.g., PROMPT=$(cat
<<PROMPT_EOF
...body...
PROMPT_EOF
) ) so ${FAILING_SUMMARY}, ${SERVER_BLOCK}, and ${WORKFLOW_RUN_URL} are expanded
in-place and you can remove the sed invocation and its use of | as a delimiter.
- Around line 160-164: The mass-failure early return path (condition using
allPlatformsFailed or totalFailures >= threshold) returns before the
workflow_run_url output is set, leaving workflow_run_url empty for later steps;
update the mass-failure branch so that before calling core.setOutput('skip',
'true') and returning you also call core.setOutput('workflow_run_url', <the same
URL value used later>) (ensure the same variable or expression used to set
workflow_run_url later is invoked here), so the "Post skip notice for mass
failures" step can read a valid URL even when returning early.
- Around line 120-158: The code currently hard-codes platform buckets
(platformFailures, platformJobIds) and classifies jobs by substring checks on
job.name (in the loop over jobs.data.jobs), which drops any non-matching
platform names; instead, change the logic to use dynamic platform keys derived
from the actual job names or the matrix input: build platformFailures and
platformJobIds as maps keyed by the normalized job.name or matrix.platform
value, increment/set entries for each jobs.data.jobs item (use job.id and
job.conclusion), and replace the fixed-output lines and the allPlatformsFailed
check with logic that iterates over Object.keys(platformFailures) (e.g., check
all values > 0) and emits outputs for each dynamic key (or aggregate them)
rather than only linux/macos/windows; update any references to platformFailures,
platformJobIds, and allPlatformsFailed accordingly.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ccb113ac-29db-49dc-8225-e7c61f593dc3
📒 Files selected for processing (1)
.github/workflows/e2e-fix-trigger.yml
New file: .github/workflows/e2e-cursor-commands.yml Adds two @cursor commands that any PR contributor can invoke via a PR comment. Both trigger immediately (issue_comment event) and reply with a reaction and a confirmation comment so the user knows the command landed. Supported commands (case-insensitive): @cursor fix e2e @cursor fix e2e failures Launched after a large change caused mass E2E failures that the automatic fix trigger skipped. The agent looks for a shared root cause first (renamed selector, changed IPC channel, modified config), fixes it at the shared layer, then handles remaining individual test bugs. Product bugs are reported via PR comment, not test modification. Hard limit: 10 test files per run. @cursor add e2e tests @cursor add e2e tests for pr Generates new E2E test cases covering the behavioral changes in the PR. The agent reads each changed non-e2e file, writes Playwright tests in e2e/specs/, and runs them against the live server before committing. Hard limit: 3 new spec files per run. Both commands: - React to the triggering comment with 👀 immediately - Use source.prUrl + target.autoBranch:false so the agent pushes directly to the PR branch (triggering a new CI run) - Include server URLs from the existing PR comment so the agent can reproduce and validate against the exact same servers - Post a summary comment when done (or a failure notice if the API key is missing/invalid) Requires: CURSOR_API_KEY repository secret. Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
github-actions
Bot
added
E2E/Run
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/e2e-cursor-commands.yml:
- Around line 40-45: The job's if condition that currently checks for
pull_request presence and comment body containing '@cursor fix e2e' or '@cursor
add e2e' must also enforce commenter authorization; modify the multiline if
block (the "if: |" conditional that checks contains(github.event.comment.body,
...)) to require github.event.comment.author_association be one of trusted
values (e.g., OWNER, MEMBER, COLLABORATOR) before proceeding, so combine the
existing comment-body checks with an additional membership check for
github.event.comment.author_association equals OWNER or MEMBER or COLLABORATOR.
- Around line 67-70: The workflow currently injects github.event.comment.body
directly into the inline script via template interpolation (the commentBody
variable) which risks script injection; change the job to set an environment
variable (e.g., COMMENT_BODY) using github.event.comment.body and in the inline
script read and normalize it from process.env (e.g., const commentBody =
process.env.COMMENT_BODY?.toLowerCase()) instead of interpolating into the
script source; keep existing uses of context.repo (owner, repo) and prNumber but
replace direct interpolation of github.event.comment.body with the env-based
process.env access.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 77af3938-d612-4542-9dd2-f1a77c82392f
📒 Files selected for processing (1)
.github/workflows/e2e-cursor-commands.yml
yasserfaraazkhan
added
E2E/Reset-Servers
- Remove --no-zygote: was preventing renderer child processes from spawning, causing all window URLs to be empty. The zygote process is essential for Chromium's renderer process model. - Add --in-process-gpu and --enable-features=NetworkServiceInProcess2: prevents network service subprocess crashes in Firecracker/Docker environments where utility process isolation fails. - Fix waitForAppReady to force-show the main window in headless/VNC environments where BrowserWindow visibility isn't triggered automatically, which blocked __e2eAppReady from being set. Before: only tests using emptyConfig (welcome screen) passed in Cloud Agent VMs. After: 28+ tests pass including config, settings, menu bar, and downloads. Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
github-actions
Bot
added
E2E/Run
- Centralize test Electron Chromium flags and process env in config - Harden login helpers (origin wait, shell detection, waitForLoggedIn) - Add WebContentsManager.markTabLoginForE2e for missed desktop onLogin IPC - Pass electronApp into loginToMattermost across specs Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
github-actions
Bot
added
E2E/Run
Playwright ElectronApplication.evaluate injects the electron module as the first parameter; the serialized webContentsId is the second. Passing (id) alone treated the module as the id, so markTabLoginForE2e never ran and waitForLoggedIn timed out. Also extend waitForAppShell selectors/timeouts for already-logged-in paths on slower Matterwick servers. Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
github-actions
Bot
added
E2E/Run
- Canonical paste-ready prompt: Stages 1–4, frozen test plan, evidence rules - Hard scope: no src/ or e2e/ edits; report-only deliverable - Pointer to keep test-fix as separate automation; optional 3-attempt cap note Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
github-actions
Bot
added
E2E/Run
AGENTS.md: default to xvfb-run -a for Playwright/Electron when no verified X server; document DISPLAY only after xdpy-info works. Update E2E example, e2e/AGENTS.md, cursor QA prompt, and e2e-agents.mdc so agents do not rediscover DISPLAY=:1 failures each run. Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
github-actions
Bot
added
E2E/Run
…e spec - Add qa_demo spec that saves PNGs under docs/qa-artifacts when MM_TEST_* is set - Include exploration report and README for demo/automation runs Co-authored-by: yasser khan <attitude3cena.yf@gmail.com>
github-actions
Bot
added
E2E/Run
devinbinnie
approved these changes
May 8, 2026
Member
devinbinnie
left a comment
devinbinnie
left a comment
There was a problem hiding this comment.
Thanks @yasserfaraazkhan :)
yasserfaraazkhan
added
E2E/Run
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
5 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
AGENTS.md — Cursor Cloud-specific instructions for the desktop repo. Documents Node version, headless Linux launch, and most importantly: how to spin up a local Mattermost server via Docker to run and verify E2E test fixes. No GitHub Actions workflow files needed — agents use
@cursoragentPR comments or Cursor Cloud's CI monitoring.E2E login state race fix — Added
waitForLoggedIn()helper that pollsServerManager.isLoggedIndirectly in the Electron main process (the source of truth), bypassing the slow multi-hop IPC chain that caused 30s timeouts on all three CI platforms. Applied towindow_menu,drag_and_drop,popout_windows,tab_managementtests. Also added retry-reload forbad_serversexpired certificate test.Approach for Cursor automation: Instead of adding workflow files to the repo, agents are triggered via:
@cursoragent fix e2ecomments on PRs (works today, zero setup)Agents follow
AGENTS.mdinstructions to start a Docker Mattermost server, reproduce failures, fix tests, and verify fixes locally before pushing.Ticket Link
N/A — developer experience improvements.
Checklist
npm run lint:jsfor proper code formattingDevice Information
This PR was tested on: Linux (Cloud Agent VM, Ubuntu)
Release Note
Change Impact: 🟡 Medium
Regression Risk: Changes primarily affect CI/workflows and E2E infrastructure utilities (findPrNumber, postServerInfoComment), plus GitHub Actions that create/update PR comments and launch Cursor agents. These introduce moderate risk of CI-side regressions (mis-posted/unsanitized comments, agent-launch failures, retained cloud instances) and increase blast radius across workflows. Application code changes are limited to test synchronization (waiting for enabled UI state) and are low-risk.
QA Recommendation: Execute several full E2E CI runs (PR-triggered and workflow_dispatch) to verify idempotent/sanitized post-server-info comments, correct mass-failure skipping and agent launches, and acceptable resource retention with label cleanup disabled. Validate that the new enabled-state waits reduce flakiness. Focus manual QA on CI/infrastructure behaviors; minimal app-level manual testing required.
Generated by CodeRabbitAI