test(e2e): repair desktop Appium suite after IA refactors#3649
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (33)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (27)
📝 WalkthroughWalkthroughAdds workflow_dispatch inputs, stable channel testids, a chat-harness mount helper, optional session clearing for test isolation, redirect-aware hash navigation, and updates many E2E specs to use new helpers and revised routes/labels. ChangesE2E Testing Infrastructure & Spec Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3330cee6fc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/test/e2e/specs/runtime-picker-login.spec.ts (1)
208-223:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the stale timeout comment to match the new deadline.
Line 208 still says “Polling up to 20s,” but Line 222 now waits 45_000ms. Please align the comment to avoid confusion during triage.
✏️ Proposed fix
- // Polling up to 20s for the connection result + potential accessibility tree dump. + // Polling up to 45s for the connection result + potential accessibility tree dump.🤖 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 `@app/test/e2e/specs/runtime-picker-login.spec.ts` around lines 208 - 223, Update the stale explanatory comment that currently reads "Polling up to 20s for the connection result + potential accessibility tree dump." to reflect the actual wait time used: replace it with a comment indicating the 45s deadline (e.g. "Polling up to 45s for the connection result + potential accessibility tree dump.") so it matches the Deadline/timeout logic (see this.timeout(60_000) and the deadline = Date.now() + 45_000 in runtime-picker-login.spec.ts).
🧹 Nitpick comments (1)
app/src/pages/Skills.tsx (1)
1-1176: ⚖️ Poor tradeoffConsider breaking this 1176-line file into smaller modules.
The coding guidelines recommend keeping frontend files to ≤ ~500 lines. This file exceeds that by more than 2×. Consider extracting the tab-specific rendering logic (composio grid, channels section, MCP, meetings, skills explorer) and the various tile/card components into separate focused modules.
Since this PR is test-infrastructure-focused, this refactor can be deferred to a future PR.
🤖 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 `@app/src/pages/Skills.tsx` around lines 1 - 1176, The file is too large (>1k LOC); extract tab-specific UI and tile components into smaller modules: move ComposioConnectorTile, ChannelTile, ComposioApiKeyEmptyState, BUILT_IN_SKILLS, renderGroup (or its JSX into a SkillsGroup component), and the composio/channels/mcp/meetings/skills explorer render blocks into their own files and import them from Skills (so Skills only orchestrates state and chooses which subcomponent to render); if you prefer to defer refactoring now, create a tracked issue and add a top-of-file TODO comment pointing to that issue and briefly listing the symbols to extract (ComposioConnectorTile, ChannelTile, ComposioApiKeyEmptyState, BUILT_IN_SKILLS, renderGroup, and the composio/channels/mcp/meetings/skills tab renderers) so future PRs can pick up the work.Source: Coding guidelines
🤖 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 `@app/test/e2e/helpers/shared-flows.ts`:
- Around line 214-224: The current settle check returns true for any non-empty
stabilized hash, causing false positives; update the logic in the block that
reads cur, expected, lastHash, and stableCount so that it only returns true when
the current hash equals the expected resolved target, or when there is no
expected target (e.g., expected is null/undefined) and the hash has stabilized
(cur && cur === lastHash && stableCount >= 2); remove the unconditional
"stableCount >= 2" return for non-matching hashes and ensure the initial
exact-match short-circuit (if (cur === expected) return true) remains.
In `@app/test/e2e/specs/conversations-web-channel-flow.spec.ts`:
- Around line 87-90: The timeout message is stale: update the timeoutMsg in the
browser.waitUntil call that uses chatMounted() to reflect the new mount
detection (replace "Conversations did not mount (Threads heading missing)" with
a message referencing the chat mount check, e.g. "Conversations did not mount
(chat component missing)"), so the error matches the chatMounted() logic.
---
Outside diff comments:
In `@app/test/e2e/specs/runtime-picker-login.spec.ts`:
- Around line 208-223: Update the stale explanatory comment that currently reads
"Polling up to 20s for the connection result + potential accessibility tree
dump." to reflect the actual wait time used: replace it with a comment
indicating the 45s deadline (e.g. "Polling up to 45s for the connection result +
potential accessibility tree dump.") so it matches the Deadline/timeout logic
(see this.timeout(60_000) and the deadline = Date.now() + 45_000 in
runtime-picker-login.spec.ts).
---
Nitpick comments:
In `@app/src/pages/Skills.tsx`:
- Around line 1-1176: The file is too large (>1k LOC); extract tab-specific UI
and tile components into smaller modules: move ComposioConnectorTile,
ChannelTile, ComposioApiKeyEmptyState, BUILT_IN_SKILLS, renderGroup (or its JSX
into a SkillsGroup component), and the composio/channels/mcp/meetings/skills
explorer render blocks into their own files and import them from Skills (so
Skills only orchestrates state and chooses which subcomponent to render); if you
prefer to defer refactoring now, create a tracked issue and add a top-of-file
TODO comment pointing to that issue and briefly listing the symbols to extract
(ComposioConnectorTile, ChannelTile, ComposioApiKeyEmptyState, BUILT_IN_SKILLS,
renderGroup, and the composio/channels/mcp/meetings/skills tab renderers) so
future PRs can pick up the work.
🪄 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: 7a74bcfc-a6f7-42f0-a238-ccb17008e26e
📒 Files selected for processing (33)
.github/workflows/e2e.ymlapp/src/components/channels/ChannelSelector.tsxapp/src/pages/Skills.tsxapp/test/e2e/helpers/chat-harness.tsapp/test/e2e/helpers/reset-app.tsapp/test/e2e/helpers/shared-flows.tsapp/test/e2e/specs/chat-conversation-history.spec.tsapp/test/e2e/specs/chat-harness-cancel.spec.tsapp/test/e2e/specs/chat-harness-scroll-render.spec.tsapp/test/e2e/specs/chat-harness-send-stream.spec.tsapp/test/e2e/specs/chat-harness-subagent-continue.spec.tsapp/test/e2e/specs/chat-harness-subagent.spec.tsapp/test/e2e/specs/chat-harness-wallet-flow.spec.tsapp/test/e2e/specs/chat-multi-tool-round.spec.tsapp/test/e2e/specs/chat-tool-call-flow.spec.tsapp/test/e2e/specs/chat-tool-error-recovery.spec.tsapp/test/e2e/specs/command-palette.spec.tsapp/test/e2e/specs/conversations-web-channel-flow.spec.tsapp/test/e2e/specs/harness-channel-bridge-flow.spec.tsapp/test/e2e/specs/harness-cron-prompt-flow.spec.tsapp/test/e2e/specs/harness-search-tool-flow.spec.tsapp/test/e2e/specs/insights-dashboard.spec.tsapp/test/e2e/specs/navigation-settings-panels.spec.tsapp/test/e2e/specs/navigation-smoothness.spec.tsapp/test/e2e/specs/onboarding-modes.spec.tsapp/test/e2e/specs/ptt-flow.spec.tsapp/test/e2e/specs/runtime-picker-login.spec.tsapp/test/e2e/specs/settings-account-preferences.spec.tsapp/test/e2e/specs/settings-advanced-config.spec.tsapp/test/e2e/specs/settings-channels-permissions.spec.tsapp/test/e2e/specs/settings-feature-preferences.spec.tsapp/test/e2e/specs/user-journey-full-task.spec.tsapp/test/e2e/specs/webhooks-tunnel-flow.spec.ts
senamakel-droid
force-pushed
the
e2e-suite-fixes
branch
2 times, most recently
from
091b69f to
e2caef8
Compare
…ai#3611/tinyhumansai#3632/tinyhumansai#3643/tinyhumansai#3646) Frontend IA refactors broke the WDIO/Appium E2E suite (baseline 83 failing specs). Test-only changes plus two one-line data-testid additions. Validated by running the full Linux E2E suite to green on a fork (see PR for RCAs).
senamakel-droid
force-pushed
the
e2e-suite-fixes
branch
from
e2caef8 to
f724c43
Compare
12 tasks
This was referenced Jun 25, 2026
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
2 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
clearAuthSessiontoresetAppso logged-out specs reliably reach Welcome (test_resetdoesn't clear the on-disk auth session token).data-testidadditions in app source (ChannelSelector,Skills)..github/workflows/e2e.ymldispatchable with per-OS +fullinputs.Problem
After the IA refactors, the desktop E2E suite failed broadly because specs/helpers targeted UI that moved or was renamed: the chat composer (button title, removed sidebar heading), navigation (routes now redirect; tabs renamed Chat/Brain/Human), and the Settings IA (panels split/merged/renamed). Two non-IA issues also surfaced: route-readiness relied on a root-innerText "signature changed" heuristic that the persistent TwoPanelLayout sidebar defeats, and
test_resetleaves the on-disk auth session token in place soskipAuthspecs running after a login stay authenticated.Solution
clickByTitletargets the stablenew-thread-buttontestid; newchatMounted()helper detects the composer instead of the removed "Threads" heading.HASH_TO_SIDEBAR_LABEL+ redirect resolution;waitForHashRouteReadyis now signature-independent and redirect/selector-tolerant.channel-select-<id>testid, insights/N2.4/command-palette repointed to/brain?tab=intelligence&itab=memory.activity/vaultcustom-wizard steps.clearAuthSession(callsauth_clear_session) used by runtime-picker + subagent/wallet chat-harness specs.Submission Checklist
data-testidattributes; not unit-covered surface.Impact
data-testidattributes. No perf/security/migration impact.Related
channels_connect); a few pre-existing flaky/edge specs (mid-stream cancel, autonomy save, onboarding-advanced Welcome→custom transition, Jira connect-modal CONNECTING seed).AI Authored PR Metadata
Linear Issue
Commit & Branch
senamakel-droid:e2e-suite-fixes3330cee6fcdcf67834e13070f3dc0b4e48c87a31Validation Run
pnpm --filter openhuman-app format:check— N/A (E2E specs; ESLint run on all changed files, clean)pnpm typecheck— passesBehavior Changes
data-testidattributes)Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Navigation & UI Updates
Chores / Stability