Skip to content

[5573] fix(frontend): Prevent agent app leaks in E2E tests - #5600

Open
Jonesxq wants to merge 2 commits into
Agenta-AI:mainfrom
Jonesxq:fix/5573-skill-upload-cleanup
Open

[5573] fix(frontend): Prevent agent app leaks in E2E tests#5600
Jonesxq wants to merge 2 commits into
Agenta-AI:mainfrom
Jonesxq:fix/5573-skill-upload-cleanup

Conversation

@Jonesxq

@Jonesxq Jonesxq commented Jul 31, 2026

Copy link
Copy Markdown

Summary

The delete-app acceptance test could select an agent app left behind by the skill-folder-upload test. Agent apps do not render on /prompts, so the test timed out while waiting to hover a row that could never appear.

This change isolates both sides of that failure:

  • The skill upload spec registers its created workflow with fixture teardown and archives it even when a later assertion fails.
  • getApp() queries each workflow's latest non-v0 revision and merges its flags before classifying completion, chat, and custom apps. Agent workflows are rejected first, so stale agents cannot enter prompt-only tests.

Closes #5573.

Testing

Verified locally

  • pnpm lint-fix
  • Prettier check for all four changed files
  • Focused Playwright unit spec: 4 tests passed
  • Targeted Playwright acceptance collection: 4 tests collected across the skill upload and app-management specs

Added or updated tests

The existing skill upload E2E now includes automatic workflow teardown. A focused unit spec verifies that revision-derived agent flags take precedence over completion, chat, and custom matching, covers prompt-app classification from latest revision flags, ignores v0 revisions, and retains valid revisions whose version is omitted.

QA follow-up

  • Run the OSS post-deploy acceptance suite with skill-folder-upload before delete-app. Both should pass, and no e2e-skill-upload-* workflow should remain active afterward.
  • Full acceptance execution was not run locally because it requires a deployed Agenta instance and authentication credentials.
  • The standalone tests TypeScript configuration still reports existing path alias, .ts extension, and DOM library errors unrelated to this change.

Demo

N/A. This changes test infrastructure only.

Checklist

  • I have included a video or screen recording for UI changes, or marked Demo as N/A
  • Relevant tests pass locally
  • Relevant linting and formatting pass locally
  • I have signed the CLA, or I will sign it when the bot prompts me

Contributor Resources

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@dosubot dosubot Bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Jul 31, 2026
@vercel

vercel Bot commented Jul 31, 2026

Copy link
Copy Markdown

@Jonesxq is attempting to deploy a commit to the agenta projects Team on Vercel.

A member of the Team first needs to authorize it.

@dosubot dosubot Bot added the tests label Jul 31, 2026
@CLAassistant

CLAassistant commented Jul 31, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved app revision selection and classification for more accurate app matching.
    • Excluded agent revisions from prompt app matching.
    • Ignored version 0 records when identifying the latest revision.
    • Correctly recognizes completion, chat, and custom app types based on revision status.
  • Test Improvements

    • Added coverage for revision selection and classification.
    • Test-created agent apps are now archived automatically after completion.

Walkthrough

The Playwright API helpers now select current app revisions, exclude agent apps from app matching, and archive workflows. The skill-folder upload acceptance test registers created workflows for cleanup. Unit tests cover revision classification and selection.

Changes

App fixture hygiene

Layer / File(s) Summary
Revision-aware app matching
web/tests/tests/fixtures/base.fixture/apiHelpers/types.d.ts, web/tests/tests/fixtures/base.fixture/apiHelpers/index.ts, web/oss/tests/playwright/unit/api-helpers.spec.ts
Exports shared app types, selects latest non-zero revisions, combines revision flags during app matching, and excludes agent apps. Unit tests cover these rules.
App archive fixture API
web/tests/tests/fixtures/base.fixture/apiHelpers/types.d.ts, web/tests/tests/fixtures/base.fixture/apiHelpers/index.ts
Adds the archiveApp helper and exposes it through the fixture API.
Acceptance test cleanup
web/oss/tests/playwright/acceptance/agent-skills/skill-folder-upload.spec.ts
Archives the workflow created by the skill-folder upload test after execution.

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

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes implement both issue requirements: fixture teardown archives agent apps and getApp excludes agent-flagged workflows.
Out of Scope Changes check ✅ Passed All changes support agent cleanup, revision classification, and regression coverage described in issue #5573.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly summarizes the main change: preventing agent app leaks in end-to-end tests.
Description check ✅ Passed The description directly explains the agent app leak, fixture teardown, revision filtering, tests, and issue resolution.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6d83ad4e-31d9-4078-b43f-94325739f110

📥 Commits

Reviewing files that changed from the base of the PR and between 53aaf6f and 9aeb396.

📒 Files selected for processing (4)
  • web/oss/tests/playwright/acceptance/agent-skills/skill-folder-upload.spec.ts
  • web/oss/tests/playwright/unit/api-helpers.spec.ts
  • web/tests/tests/fixtures/base.fixture/apiHelpers/index.ts
  • web/tests/tests/fixtures/base.fixture/apiHelpers/types.d.ts

Comment thread web/tests/tests/fixtures/base.fixture/apiHelpers/index.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS This PR changes 0-9 lines, ignoring generated files. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(bug) delete-app E2E test times out when a leftover skill-upload agent app leaks into the picker

2 participants