test(playwright): add functional UI flow coverage#3257
Merged
senamakel merged 2 commits intoJun 3, 2026
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 35 minutes and 9 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR expands Playwright functional coverage across chat, connectors, memory, onboarding, settings, workflows, invites, and top-level routes, while adding one E2E build flag for chat attachments, one mock invite-code response path, and one accessibility label in the agent editor. ChangesFunctional E2E coverage and support updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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. Comment |
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
coderabbitai
Bot
added
feature
![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: 3
🧹 Nitpick comments (6)
app/test/playwright/specs/top-level-functional-flows.spec.ts (1)
12-24: ⚡ Quick winExtract
mockFetch/setMockBehaviorinto a shared E2E helper.These two helpers are duplicated verbatim in
app/test/playwright/specs/connector-session-guard-matrix.spec.ts(see context snippets 1 and 2), with only themockFetchreturn type differing (unknownhere vs{ data?: unknown }there). Consider hoisting them next tocallCoreRpcinapp/test/playwright/helpers/core-rpc.ts(or a newmock-admin.tshelper) to keep the mock-admin contract in one place.🤖 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/playwright/specs/top-level-functional-flows.spec.ts` around lines 12 - 24, The two duplicated helpers mockFetch and setMockBehavior should be extracted into a shared E2E helper so the mock-admin contract is centralized; create a new helper (or add next to the existing callCoreRpc helper) named e.g. mock-admin.ts and move the implementations there, standardizing the mockFetch return type (pick a single return shape such as unknown or { data?: unknown } and update both specs), then import and use mockFetch and setMockBehavior from that helper in top-level-functional-flows.spec.ts and connector-session-guard-matrix.spec.ts to remove the duplication.scripts/mock-api/routes/invites.mjs (1)
15-25: ⚡ Quick winReuse the existing
parseBehaviorJsonhelper.
scripts/mock-api/state.mjsalready exportsparseBehaviorJson(key, fallback)(context snippet 1), which performs the same missing/invalid-JSON fallback. Reusing it keeps behavior-key parsing consistent across routes; just retain the array guard for safety.♻️ Proposed refactor
-import { behavior } from "../state.mjs"; +import { parseBehaviorJson } from "../state.mjs"; @@ - const rawCodes = behavior().inviteCodes; - let codes = []; - if (typeof rawCodes === "string" && rawCodes.length > 0) { - try { - const parsed = JSON.parse(rawCodes); - if (Array.isArray(parsed)) codes = parsed; - } catch { - codes = []; - } - } - json(res, 200, { success: true, data: codes }); + const parsed = parseBehaviorJson("inviteCodes", []); + const codes = Array.isArray(parsed) ? parsed : []; + json(res, 200, { success: true, data: codes });🤖 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 `@scripts/mock-api/routes/invites.mjs` around lines 15 - 25, Replace the manual JSON.parse logic for invite codes with the existing helper parseBehaviorJson: import and call parseBehaviorJson('inviteCodes', []) instead of using behavior() + JSON.parse; assign its result to rawCodes (or directly to codes), then keep the Array.isArray guard (if not an array, fallback to []). Remove the try/catch block and ensure the route still returns json(res, 200, { success: true, data: codes }).app/test/playwright/specs/onboarding-config-functional.spec.ts (1)
62-70: 💤 Low valueReuse
chooseConfigureinstead of re-inlining it.This block duplicates the
chooseConfigure(page)helper defined at Line 20-25.♻️ Call the existing helper
await expect(page.getByTestId(id)).toBeVisible({ timeout: 20_000 }); - const configure = page.getByRole('button', { name: /Configure/ }); - if ( - await configure - .first() - .isVisible() - .catch(() => false) - ) { - await configure.first().click(); - } + await chooseConfigure(page); await page.getByTestId('onboarding-next-button').click();🤖 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/playwright/specs/onboarding-config-functional.spec.ts` around lines 62 - 70, The duplicated inline logic that queries the Configure button and conditionally clicks it should be replaced with the existing helper chooseConfigure(page); locate the block using the configure constant and getByRole call (the if-await-isVisible-catch pattern) and remove it, then invoke chooseConfigure(page) so the shared helper from lines ~20-25 is reused instead of reimplementing the visibility-check-and-click behavior.app/test/playwright/specs/intelligence-memory-ui-functional.spec.ts (3)
13-13: 💤 Low value
MemorySourceis unused; preferinterfaceif retained.This type alias does not appear to be referenced anywhere in the file. Remove it as dead code, or, if it's intended for use, declare it as an
interfacefor object shapes.♻️ Remove unused type
-type MemorySource = { id: string; kind: string; label: string; enabled: boolean }; -As per coding guidelines: "Prefer interface for defining object shapes in TypeScript".
🤖 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/playwright/specs/intelligence-memory-ui-functional.spec.ts` at line 13, The type alias MemorySource is unused; either remove the declaration entirely to eliminate dead code, or if it’s intended to be used elsewhere convert it to an interface (declare interface MemorySource { id: string; kind: string; label: string; enabled: boolean }) and export it if needed so other modules can reference it. Locate the MemorySource declaration in the spec and apply one of these two fixes, then run TypeScript/linters to confirm no unused symbol or missing import errors remain.
15-24: 💤 Low valueRedundant
waitForAppReadycall.
bootAuthenticatedPagealready navigates to the hash and awaitswaitForAppReadyinternally (seehelpers/core-rpc.ts:135-146), so the explicit call on Line 17 is redundant.♻️ Drop the duplicate readiness wait
await bootAuthenticatedPage(page, 'pw-intelligence-memory-ui', '/intelligence'); - await waitForAppReady(page); await dismissWalkthroughIfPresent(page);🤖 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/playwright/specs/intelligence-memory-ui-functional.spec.ts` around lines 15 - 24, The openMemory function contains a redundant call to waitForAppReady because bootAuthenticatedPage already navigates and awaits readiness; remove the explicit await waitForAppReady(page) line inside openMemory so the sequence relies on bootAuthenticatedPage's internal readiness check (refer to openMemory and bootAuthenticatedPage and the waitForAppReady helper).
76-77: ⚡ Quick winConsider asserting the backend effect of removal.
The removal flow asserts only the UI row disappears. Per the E2E guidance to assert backend/mock effects when relevant, consider confirming the source is gone via
openhuman.memory_sources_list(and addingdumpAccessibilityTree()on failure) to harden the test against UI-only regressions.Based on learnings: "Assert both UI outcomes and backend/mock effects when relevant in E2E tests. Add failure diagnostics (request logs, dumpAccessibilityTree()) for faster debugging by agents."
🤖 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/playwright/specs/intelligence-memory-ui-functional.spec.ts` around lines 76 - 77, After clicking the remove button (row.getByTitle('Remove').click()) add an assertion that the memory source is actually removed from the backend/mock by calling openhuman.memory_sources_list and asserting the removed source id/name is not present; wrap this check so on failure you dump diagnostics (call page.dumpAccessibilityTree() and capture request logs) before failing the test to aid debugging. Ensure you keep the existing UI assertion (await expect(row).toHaveCount(0)) but augment it with the backend check against openhuman.memory_sources_list and include the diagnostic calls in the failure path.
🤖 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/playwright/specs/chat-management-functional.spec.ts`:
- Around line 97-98: The rename/delete test ("thread rename and delete remain
usable from the conversation UI") is inheriting mock server state from prior
tests; call resetMock() at the start of that test (before openChat(page,
'pw-chat-rename-delete')) to clear the /__admin/behavior state so the test runs
in isolation — do not rely on bootAuthenticatedPage() since it only resets core
mocks.
In `@app/test/playwright/specs/intelligence-memory-ui-functional.spec.ts`:
- Around line 45-46: The call to addFolderSource(label) is happening before
openMemory/bootAuthenticatedPage which triggers resetCoreForWebUser →
openhuman.auth_clear_session and can change the config directory used for
Config.memory_sources; move the addFolderSource(label) invocation to after
openMemory (or after bootAuthenticatedPage) so memory_sources_add runs in the
authenticated user context, or alternatively ensure openhuman.auth_store_session
for the same userId is called before memory_sources_add; update test to call
addFolderSource only after openMemory (or add an auth_store_session step) and
reference addFolderSource, openMemory, bootAuthenticatedPage,
resetCoreForWebUser, auth_clear_session, auth_store_session, memory_sources_add,
and Config.memory_sources when making the change.
In `@app/test/playwright/specs/onboarding-config-functional.spec.ts`:
- Around line 53-75: The loop iterates over the list including
'onboarding-custom-vault-step' but always clicks
page.getByTestId('onboarding-next-button'), which conflicts with subsequent
assertions that the vault step remains visible and the Next button is enabled;
update the loop in onboarding-config-functional.spec.ts to exclude
'onboarding-custom-vault-step' (or add a conditional guard around the
page.getByTestId('onboarding-next-button').click() so it skips clicking when id
=== 'onboarding-custom-vault-step'), then keep the final assertions that await
expect(page.getByTestId('onboarding-custom-vault-step')).toBeVisible() and
expect(page.getByTestId('onboarding-next-button')).toBeEnabled(); confirm the
actual Next behavior in the vault step component before applying the change.
---
Nitpick comments:
In `@app/test/playwright/specs/intelligence-memory-ui-functional.spec.ts`:
- Line 13: The type alias MemorySource is unused; either remove the declaration
entirely to eliminate dead code, or if it’s intended to be used elsewhere
convert it to an interface (declare interface MemorySource { id: string; kind:
string; label: string; enabled: boolean }) and export it if needed so other
modules can reference it. Locate the MemorySource declaration in the spec and
apply one of these two fixes, then run TypeScript/linters to confirm no unused
symbol or missing import errors remain.
- Around line 15-24: The openMemory function contains a redundant call to
waitForAppReady because bootAuthenticatedPage already navigates and awaits
readiness; remove the explicit await waitForAppReady(page) line inside
openMemory so the sequence relies on bootAuthenticatedPage's internal readiness
check (refer to openMemory and bootAuthenticatedPage and the waitForAppReady
helper).
- Around line 76-77: After clicking the remove button
(row.getByTitle('Remove').click()) add an assertion that the memory source is
actually removed from the backend/mock by calling openhuman.memory_sources_list
and asserting the removed source id/name is not present; wrap this check so on
failure you dump diagnostics (call page.dumpAccessibilityTree() and capture
request logs) before failing the test to aid debugging. Ensure you keep the
existing UI assertion (await expect(row).toHaveCount(0)) but augment it with the
backend check against openhuman.memory_sources_list and include the diagnostic
calls in the failure path.
In `@app/test/playwright/specs/onboarding-config-functional.spec.ts`:
- Around line 62-70: The duplicated inline logic that queries the Configure
button and conditionally clicks it should be replaced with the existing helper
chooseConfigure(page); locate the block using the configure constant and
getByRole call (the if-await-isVisible-catch pattern) and remove it, then invoke
chooseConfigure(page) so the shared helper from lines ~20-25 is reused instead
of reimplementing the visibility-check-and-click behavior.
In `@app/test/playwright/specs/top-level-functional-flows.spec.ts`:
- Around line 12-24: The two duplicated helpers mockFetch and setMockBehavior
should be extracted into a shared E2E helper so the mock-admin contract is
centralized; create a new helper (or add next to the existing callCoreRpc
helper) named e.g. mock-admin.ts and move the implementations there,
standardizing the mockFetch return type (pick a single return shape such as
unknown or { data?: unknown } and update both specs), then import and use
mockFetch and setMockBehavior from that helper in
top-level-functional-flows.spec.ts and connector-session-guard-matrix.spec.ts to
remove the duplication.
In `@scripts/mock-api/routes/invites.mjs`:
- Around line 15-25: Replace the manual JSON.parse logic for invite codes with
the existing helper parseBehaviorJson: import and call
parseBehaviorJson('inviteCodes', []) instead of using behavior() + JSON.parse;
assign its result to rawCodes (or directly to codes), then keep the
Array.isArray guard (if not an array, fallback to []). Remove the try/catch
block and ensure the route still returns json(res, 200, { success: true, data:
codes }).
🪄 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: 790dbdce-d851-4fd5-89d6-56345f982331
📒 Files selected for processing (9)
app/scripts/e2e-web-build.shapp/src/components/settings/panels/AgentEditorPage.tsxapp/test/playwright/specs/chat-management-functional.spec.tsapp/test/playwright/specs/connector-session-guard-matrix.spec.tsapp/test/playwright/specs/intelligence-memory-ui-functional.spec.tsapp/test/playwright/specs/onboarding-config-functional.spec.tsapp/test/playwright/specs/settings-leaf-workflows.spec.tsapp/test/playwright/specs/top-level-functional-flows.spec.tsscripts/mock-api/routes/invites.mjs
senamakel
added a commit
to senamakel/openhuman
that referenced
this pull request
Jun 6, 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
1 participant
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
Problem
The existing Playwright suite covered broad smoke and harness paths, but several UX-critical flows could regress without an end-to-end signal: settings persistence, agent tool permissions, connector session preservation, chat attachment recovery, memory-source controls, invite redemption, workflow create/delete, and onboarding configuration navigation.
Solution
This PR adds focused functional Playwright specs that drive those flows through the UI and assert persisted state, mock-backend effects, or RPC-visible outcomes where relevant. The only product-code adjustment is an accessible label on the agent editor tool-picker button so the test and users of assistive tech can target it by name.
Submission Checklist
## Related— N/A: no matrix feature IDs changed.docs/RELEASE-MANUAL-SMOKE.md) — N/A: test-only coverage expansion with no release manual workflow change.Closes #NNNin the## Relatedsection — N/A: user-requested test audit follow-up without a GitHub/Linear issue.Impact
Runtime behavior is unchanged except for a more descriptive accessible name on the agent tool-picker button. The mock API can now seed invite codes for deterministic Playwright coverage, and the Playwright web build explicitly enables chat attachments to cover attachment UI behavior.
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:check— passed in pre-push hook on second run.pnpm typecheck— passed via pre-pushpnpm compile/tsc --noEmit.pnpm --filter openhuman-app test:e2e:web:build— passed.bash app/scripts/e2e-web-session.sh test/playwright/specs/settings-leaf-workflows.spec.ts test/playwright/specs/top-level-functional-flows.spec.ts test/playwright/specs/connector-session-guard-matrix.spec.ts test/playwright/specs/chat-management-functional.spec.ts test/playwright/specs/intelligence-memory-ui-functional.spec.ts test/playwright/specs/onboarding-config-functional.spec.ts— 17 passed.pnpm debug unit src/components/settings/panels/AgentEditorPage.test.tsx src/components/settings/panels/TaskSourcesPanel.test.tsx— passed, 16 tests.pnpm debug unit src/components/intelligence/__tests__/MemoryTextAndDetail.test.tsx src/components/intelligence/MemoryGraph.test.tsx— passed, 18 tests.pnpm --filter openhuman-app exec prettier --check src/components/settings/panels/AgentEditorPage.tsx test/playwright/specs/settings-leaf-workflows.spec.ts test/playwright/specs/top-level-functional-flows.spec.ts test/playwright/specs/connector-session-guard-matrix.spec.ts test/playwright/specs/chat-management-functional.spec.ts test/playwright/specs/intelligence-memory-ui-functional.spec.ts test/playwright/specs/onboarding-config-functional.spec.ts ../scripts/mock-api/routes/invites.mjs— passed.pnpm rust:checkpassed via pre-push hook with existing warnings only; no Tauri source changed.Validation Blocked
command:node scripts/codex-pr-preflight.mjs --strict-path --lightweighterror:[FAIL] expected repo path :: expected /workspace/openhuman, got /Users/enamakel/work/tinyhumansai/openhuman-5impact:Local checkout path differs from the Codex web path expected by the remote-agent preflight; repository files, branch naming, remotes, and changed-file readability passed after renaming the branch.Behavior Changes
Parity Contract
Duplicate / Superseded PR Handling
senamakel:codex/QA-0-playwright-functional.Summary by CodeRabbit
Release Notes
Accessibility
Tests
Chores