[fix] Scope web-test cleanup and render messages once - #6581
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 SummarySummary by CodeRabbit
WalkthroughThe changes update member acceptance tests, record ephemeral project ownership, and make teardown delete only owned ephemeral projects. Teardown tests cover success and failure paths. The layout no longer mounts ChangesMember acceptance tests
Ephemeral project lifecycle
Layout message provider removal
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to Cleanup now preserves fallback persistent projects and deletes only explicitly owned ephemeral projects. A transient setup failure before successful project creation can still leave an ephemeral test project undeleted on retry, creating bounded test-environment resource leakage. Sequence Diagram(s)sequenceDiagram
participant PlaywrightTeardown
participant ProjectMetadata
participant API
participant Filesystem
PlaywrightTeardown->>ProjectMetadata: Read ephemeral ownership
PlaywrightTeardown->>API: Restore the original default project
PlaywrightTeardown->>API: Delete the ephemeral project
PlaywrightTeardown->>Filesystem: Remove metadata after successful deletion
PlaywrightTeardown->>Filesystem: Retain metadata after failed deletion
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
9623c13 to
64902a6
Compare
|
🤖 The AI agent says: @coderabbitai review Please focus on teardown project ownership and whether each Members locator proves the intended action. |
|
🤖 The AI agent says: Updated the branch after review feedback: the resend step now waits for the exact non-redirected @coderabbitai review |
64902a6 to
ce33c4e
Compare
mmabrouk
changed the title
|
🤖 The AI agent says: The focused browser rerun exposed an application defect rather than another locator issue: one resend produced two success toasts, and one removal produced two identical Radix alert dialogs. The shared This revision removes the newer @coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/tests/playwright/global-teardown.ts (2)
41-41: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winGuard deletion with an explicit ephemeral marker.
writeProjectMetadatarecordsdefaultProject.project_idwhen ephemeral creation is disabled or fails, anddeleteEphemeralProjectdeletes every recorded ID; record only successfully created ephemeral projects.
41-41: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winSensitive Data Exposure (CWE-459)
Reachability: Internal · Exploitability: Difficult
Retain teardown metadata when project deletion fails.
Project deletion cascades project secrets atomically on success. However, teardown removes the metadata after any failed
DELETErequest or fetch error, leaving retained secrets without a recovery handle. Remove the metadata only after successful deletion, and retry or explicitly clean up secrets on failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: 545c9a65-82ca-452d-b2a9-6e8acff56471
📒 Files selected for processing (3)
web/ee/tests/playwright/acceptance/members/index.tsweb/tests/README.mdweb/tests/playwright/global-teardown.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review.
|
✅ Action performedReview finished.
|
ce33c4e to
ce1be09
Compare
|
🤖 The AI agent says: The completed CodeRabbit review covered the previous Added focused regressions for all three paths: fallback/default sends no DELETE, owned-project success deletes and removes metadata, and failed deletion retains metadata. All 3 pass. Full frontend lint still passes. @coderabbitai review |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: 4d5c99b4-11dc-4b9e-90a5-88752a4f6950
📒 Files selected for processing (2)
web/ee/tests/playwright/acceptance/members/index.tsweb/oss/src/components/Layout/Layout.tsx
💤 Files with no reviewable changes (1)
- web/oss/src/components/Layout/Layout.tsx
Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review.
|
|
ce1be09 to
f03d6e8
Compare
|
🤖 The AI agent says: Resolved the current-head Members finding in @coderabbitai review |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: 0692f9d9-3b8f-4109-9bd9-5246bb154f3d
📒 Files selected for processing (4)
web/ee/tests/playwright/acceptance/members/index.tsweb/tests/playwright/global-setup.tsweb/tests/playwright/global-teardown.test.tsweb/tests/playwright/global-teardown.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 2 remain after this review.
|
|
|
|
|
🤖 The AI agent says: Resolved the final exact-head finding in @coderabbitai review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR status and diff ==='
git status --short
git diff --stat HEAD^
git diff --unified=80 HEAD^ -- \
web/tests/playwright/global-setup.ts \
web/tests/playwright/global-teardown.ts \
web/tests/playwright/global-teardown.test.ts \
web/ee/tests/playwright/acceptance/members/index.ts \
web/oss/src/components/Layout/Layout.tsx \
web/tests/README.md
echo '=== Candidate file outlines ==='
ast-grep outline web/tests/playwright/global-setup.ts --items all
ast-grep outline web/tests/playwright/global-teardown.ts --items all
ast-grep outline web/tests/playwright/global-teardown.test.ts --items all
ast-grep outline web/ee/tests/playwright/acceptance/members/index.ts --items all
echo '=== Project metadata definitions and usages ==='
rg -n -C 4 --glob '*.ts' --glob '*.tsx' \
'(writeProjectMetadata\s*\(|ephemeral\s*:|projectPath|statePath|deleteEphemeralProject\s*\()' \
web/tests web/ee/tests
echo '=== Member UI and acceptance patterns ==='
rg -n -C 5 --glob '*.ts' --glob '*.tsx' \
'(invite/resend|Resend|Remove member|Pending|alertdialog|successfully|AppMessageContext)' \
web/ee web/oss/src web/tests/playwrightLength of output: 201660 #!/bin/bash
set -euo pipefail
echo '=== global-setup.ts ==='
cat -n web/tests/playwright/global-setup.ts
echo '=== global-teardown.ts ==='
cat -n web/tests/playwright/global-teardown.ts
echo '=== global-teardown.test.ts ==='
cat -n web/tests/playwright/global-teardown.test.ts
echo '=== Members acceptance test ==='
cat -n web/ee/tests/playwright/acceptance/members/index.ts
echo '=== Narrow Members implementation search ==='
rg -n -C 8 --glob '*.ts' --glob '*.tsx' \
'(resendInvite|invite/resend|removeMember|Remove member|Resend invitation|Resend invite|Pending)' \
web/ee web/oss/src/components/pages web/oss/src/pages
echo '=== Metadata file names and all setup/teardown call sites ==='
rg -n -C 6 --glob '*.ts' \
'(PROJECT.*(?:STATE|METADATA)|project.*(?:state|metadata)|writeProjectMetadata|readFileSync|unlinkSync|rmSync)' \
web/tests/playwrightYou are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== PR metadata =="
gh pr view 6581 --repo Agenta-AI/agenta --json state,isDraft,headRefOid,baseRefOid,commits,files \
--jq '{state, isDraft, headRefOid, baseRefOid, commits: [.commits[] | {oid: .oid, messageHeadline: .messageHeadline}], files: [.files[].path]}'
echo "== Target commit =="
git show --no-ext-diff --format=fuller --stat 64350bf3
git show --no-ext-diff --format= --find-renames 64350bf3 -- \
web/tests/playwright/global-setup.ts \
web/tests/playwright/global-teardown.ts \
web/tests/playwright/global-teardown.test.ts
echo "== Relevant file outlines =="
ast-grep outline web/tests/playwright/global-setup.ts --items all
ast-grep outline web/tests/playwright/global-teardown.ts --items all
ast-grep outline web/tests/playwright/global-teardown.test.ts --items all
echo "== Relevant implementation references =="
rg -n -C 5 \
'maybeCreateEphemeralProject|writeProjectMetadata|ephemeral|originalDefault|defaultProject|deleteEphemeralProject|restore' \
web/tests/playwright/global-setup.ts \
web/tests/playwright/global-teardown.ts \
web/tests/playwright/global-teardown.test.tsLength of output: 2222 🏁 Script executed: #!/bin/bash
set -euo pipefail
head="f03d6e8fc77f56ddf9d9f5d2ad1fe8e35b2c2fa5"
base="7496c66dce30494abc5373e267a686f44e73bfb9"
echo "== PR diff at merged head =="
git diff --no-ext-diff --find-renames "${base}...${head}" -- \
web/tests/playwright/global-setup.ts \
web/tests/playwright/global-teardown.ts \
web/tests/playwright/global-teardown.test.ts
echo "== global setup: metadata and ephemeral-project control flow =="
rg -n -C 18 \
'async function maybeCreateEphemeralProject|function writeProjectMetadata|const writeProjectMetadata|ephemeral|default.*project|project.*default' \
web/tests/playwright/global-setup.ts
echo "== global teardown: cleanup control flow =="
rg -n -C 18 \
'interface DeleteEphemeralProjectOptions|async function deleteEphemeralProject|export async function deleteEphemeralProject|ephemeral|default.*project|project.*default|unlink|rm\(' \
web/tests/playwright/global-teardown.ts
echo "== teardown regression tests =="
rg -n -C 14 \
'describe\(|test\(|deleteEphemeralProject|ephemeral|default.*project|project.*default|metadata' \
web/tests/playwright/global-teardown.test.ts
echo "== Calls and metadata types across Playwright tests =="
rg -n -C 4 \
'maybeCreateEphemeralProject\s*\(|writeProjectMetadata\s*\(|deleteEphemeralProject\s*\(|ephemeral\s*:' \
web/tests/playwrightLength of output: 41530
In In Add a guard before the The available PR checkout does not contain You are interacting with an AI system. |
Railway Preview Environment
|
1 participant
Context
The web acceptance teardown restored the account's persistent default project, deleted its ephemeral test project, then listed secrets without a project scope and deleted every provider whose name contained
openai. A release QA run therefore deleted a persistent browser QA credential after the test project was already gone. Setup could also record a fallback default project in the same metadata shape as an ephemeral project, while teardown treated every recorded ID as test-owned.The Members acceptance cases depended on removed Ant Design classes. Once those selectors were corrected, the focused browser run exposed two identical success toasts and two identical destructive confirmation dialogs. The desktop app mounted the global app-message outlet in both
_appandLayout, although the outlet contract requires one mount.Changes
Setup now records whether a project is ephemeral. Teardown sends DELETE only for an explicitly owned ephemeral project, relies on project deletion to cascade its secrets, and retains metadata after a failed deletion so a later run can retry. It no longer scans or deletes credentials in the restored project.
The Members cases select rows and actions by accessible roles, wait for the exact resend and remove responses, and require one toast and one alert dialog. The app keeps the original
_appmessage outlet and removes the newer duplicate fromLayout.Before, teardown could issue
DELETE /secrets/<persistent-id>after restoring the original project, and onemessageormodalcall rendered twice. After this change, its only remote cleanup is an ownedDELETE /projects/<ephemeral-id>, and each app-message action has one renderer.How to review
ephemeral: true._appremains the sole app-message outlet andLayoutno longer adds another.Tests
pnpm exec tsx --test tests/playwright/global-teardown.test.ts(3/3 passed)pnpm lint-fix(25/25 tasks passed)git diff --check