Skip to content

test(e2e): restore workspace-aware P0 coverage - #6445

Merged
AmyShang-alt merged 6 commits into
mainfrom
codex/cleanup-workspace-p0-e2e
Aug 6, 2026
Merged

test(e2e): restore workspace-aware P0 coverage#6445
AmyShang-alt merged 6 commits into
mainfrom
codex/cleanup-workspace-p0-e2e

Conversation

@AmyShang-alt

@AmyShang-alt AmyShang-alt commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Why

Workspace-aware project creation made several P0 suites depend on the runner's real Vela session, while the Automations P0 cases were absent from the merge-gated matrix. The multi-client collaboration shard also paid for redundant and serial runtime startup, pushing the runner past its 12-minute hard limit. This cleanup restores hermetic, complete P0 coverage and keeps the collaboration gate inside the runner budget.

After the collaboration optimization, project-workspace became the remaining long UI P0 path: the slowest observed job took 13m12s, while a later merge-queue run still took 9m26s with one retry. The workspace suite is now split into two runner-isolated, balanced jobs without enabling unsafe in-job client parallelism.

What users will see

No product UI changes. Contributors get deterministic local/BYOK project-creation tests, CI runs the Automations P0 scenarios in their own shard, and both collaboration and workspace gates finish substantially faster.

Surface area

  • None — internal tests and CI coverage only.

What changed

  • Make the standard E2E mock return a signed-out Vela status so local account state cannot trigger Workspace authority checks and block Local CLI or BYOK project creation.
  • Consolidate duplicated setup in the app, Design Files, Manual Edit, and Restoration suites onto the standard mock.
  • Remove an obsolete P0 test that depended on the retired Home starters gallery and was permanently skipped.
  • Add the Automations P0 tests to the CI matrix as a dedicated entry-automations shard, and update the coverage guard accordingly.
  • Give cluster-owned Playwright specs a lifecycle that does not boot an unused default worker runtime.
  • Start and stop isolated collaboration clients concurrently while preserving complete partial-acquisition cleanup and failure diagnostics.
  • Pin each client's active Workspace before its first navigation and wait for the exact browser-side Workspace event stream before emitting live invalidations.
  • Keep the merge-gated multi-client browser flow focused on the cross-layer content, presence, unshare, and role-transition witnesses; narrower daemon/web tests retain the lower-level catch-up, lease, comment-relay, metadata, and membership contracts.
  • Keep project-collab limited to the cluster-owned spec and move the ordinary keyboard-flow spec back to project-workspace.
  • Split project-workspace into core and editor jobs using historical file timings while retaining workers: 1 inside each isolated runner.
  • Replace a raw iframe CSS evaluate() poll with locator-native toHaveCSS, so navigation does not leave a stale execution context.
  • Remove a redundant global upload-filename assertion that could match both the design-file row and staged attachment and trigger a retry.

Root cause

After Workspace support was merged, a signed-in Vela account requires an exact Workspace authority. Several older E2E suites did not mock the login status, so their behavior depended on the runner's real account state. Project creation could therefore be rejected before POST /api/projects was sent.

In addition, automations-page.test.ts contained P0 tests but was not registered in the UI P0 coverage list or CI matrix, so CI could not detect regressions in those scenarios.

The collaboration spec owned two isolated daemon/web clients but still inherited the default Playwright worker runtime, started and stopped both clients serially, navigated both clients through redundant Home reloads, and repeated lower-layer scenarios inside the longest browser chain. That made a passing shard exceed the runner's 12-minute wall-clock limit. The optimized lifecycle removes the unused runtime, starts the real clients concurrently, performs one readiness-gated navigation per browser witness, and keeps the P0 chain at the cross-layer boundary.

The workspace P0 job then ran all editor, core project, management, keyboard, and design-system scenarios serially in one runner. Directly increasing Playwright workers breaks Workspace/API authority isolation, so the safe optimization is runner-level file sharding with one worker per job. Two navigation-sensitive assertions also caused avoidable whole-worker retries.

Validation

  • Hosted UI P0 (project-collab): 2/2 passed; test domain completed in 2m11s and the full job in 3m34s.
  • Exact integrated project-collab entry: 2/2 passed in 1.8 minutes locally.
  • Optimized collaboration entry repeated on the same machine: 2/2 passed in 97.82 seconds and 94.94 seconds before the final main integration.
  • Collaboration cluster lifecycle tests: 4/4 passed.
  • Focused workflow/scope topology tests: 108/108 passed (56 workflow + 52 scope).
  • Hosted run 31064947330 passed the full workflow and both workspace jobs without retries:
    • project-workspace: 25/25 tests in 4.0m; full job 5m32s.
    • project-workspace-editor: 13/13 tests in 4.4m; full job 5m51s.
  • The longest workspace path is 38% below the 9m26s merge-queue baseline and 56% below the 13m12s slow sample.
  • pnpm guard passed.
  • pnpm typecheck passed.
  • pnpm -C e2e typecheck passed.

Hosted runner wall-clock acceptance and the complete required matrix passed on the exact optimization commit now at this PR's head.

@AmyShang-alt
AmyShang-alt marked this pull request as ready for review August 5, 2026 04:38
@AmyShang-alt
AmyShang-alt requested a review from a team as a code owner August 5, 2026 04:39
@lefarcen
lefarcen requested a review from mrcfps August 5, 2026 04:40
@lefarcen lefarcen added size/M PR changes 100-300 lines risk/medium Medium risk: regular code changes type/chore CI / build / config / tooling skip-validation Maintainer override: bot will not auto-add needs-validation on this PR. labels Aug 5, 2026

@mrcfps mrcfps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AmyShang-alt thank you for this careful E2E cleanup — it is a real reliability win.

I reviewed the full changed range on 01e8647c:

  • Hermetic Vela statusrouteSignedOutVelaStatus in applyStandardMocks (and the targeted use in api-empty-response) correctly isolates local/BYOK project-create flows from a host machine’s signed-in AMR session, while still letting suite-level signed-in routes register later and win under Playwright’s LIFO route matching.
  • Suite consolidation — moving app / design-files / manual-edit / restoration onto applyStandardMocks preserves the prior standard config and adds the missing authority witness without changing those specs’ agent fixtures.
  • Coverage matrixentry-automations is wired consistently through uiP0Groups, uiP0CiMatrix, coverage files, the six-domain scope guard, and the packaged-smoke / scopes expectations, so the six Automations P0 cases actually run in CI.
  • Obsolete P0 removal — dropping the permanently skipped Home starters plugin-input P0 is appropriate now that plugins-home-section is gone after #5517.

No correctness, safety, or maintainability blockers in the changed ranges. Nice work making the P0 surface workspace-aware and less environment-dependent. 🙌

🔁 Powered by Looper · runner=reviewer · agent=grok-build · An autonomous AI dev team for your GitHub repos.

@lefarcen

lefarcen commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

The E2E coverage cleanup itself looks solid, and the root-cause + validation sections already make the test intent easy to follow.

One quick PR-body follow-up: could you add a short Why section (the user/problem framing), a What users will see section, and tick the Surface area box — likely None here if this is strictly test/coverage work? That should make the template complete for the next pass.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Visual regression review

Head: 843fd1f · Base: 5f5a589

6 changed · 41 unchanged · 0 new without baseline · 0 failed

Changed cases

Case Main PR Diff
visual-home
4,455 px (0.34%)
main pr diff
visual-home-plugin-use-staged
5,025 px (0.39%)
main pr diff
visual-home-plugin-use-with-query
756 px (0.06%)
main pr diff
visual-home-staged-attachment
3,636 px (0.28%)
main pr diff
visual-projects
1,886 px (0.15%)
main pr diff
visual-topbar-local-cli-model-list
4,493 px (0.35%)
main pr diff
Unchanged cases
Case Main PR Diff
visual-avatar-local-agent-list
0 px (0.00%)
main pr diff
visual-avatar-local-agent-list-panel
0 px (0.00%)
main pr diff
visual-avatar-menu
17 px (0.00%)
main pr diff
visual-avatar-menu-panel
17 px (0.05%)
main pr diff
visual-avatar-open-design-model-picker
0 px (0.00%)
main pr diff
visual-critical-settings
51 px (0.00%)
main pr diff
visual-critical-workspace
0 px (0.00%)
main pr diff
visual-critical-workspace-preview
0 px (0.00%)
main pr diff
visual-design-system-detail
48 px (0.00%)
main pr diff
visual-design-systems
48 px (0.00%)
main pr diff
visual-home-catalog
48 px (0.00%)
main pr diff
visual-home-context-picker
197 px (0.02%)
main pr diff
visual-home-context-picker-popover
117 px (0.03%)
main pr diff
visual-home-plugin-filter
48 px (0.00%)
main pr diff
visual-integrations-use-everywhere
51 px (0.00%)
main pr diff
visual-new-project-modal
0 px (0.00%)
main pr diff
visual-onboarding-cloud
0 px (0.00%)
main pr diff
visual-plugin-details
48 px (0.00%)
main pr diff
visual-plugin-share-menu
48 px (0.00%)
main pr diff
visual-plugin-share-menu-popover
0 px (0.00%)
main pr diff

Visual diff is advisory only and does not block merging.

Generated-By: looper 0.11.7 (runner=fixer, agent=codex)
@lefarcen
lefarcen requested a review from mrcfps August 5, 2026 05:35
@lefarcen lefarcen added size/L PR changes 300-700 lines and removed size/M PR changes 100-300 lines labels Aug 5, 2026

@mrcfps mrcfps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AmyShang-alt thank you for this careful E2E reliability cleanup — the second commit makes the collab waits as deliberate as the hermetic mock work.

I re-reviewed the full changed range on 589e66f6:

  • Hermetic Vela statusrouteSignedOutVelaStatus in applyStandardMocks (and the targeted use in api-empty-response) keeps local/BYOK project-create flows independent of a host machine’s signed-in AMR session, while suite-level signed-in routes registered later still win under Playwright’s LIFO matching.
  • Suite consolidation — app / design-files / manual-edit / restoration now share applyStandardMocks, so they pick up the authority witness without changing their agent fixtures or standard config shape.
  • Coverage matrixentry-automations is wired consistently through uiP0Groups, uiP0CiMatrix, coverage files, the six-domain scope guard, and the packaged-smoke / scopes expectations; CI already shows the new shard green.
  • Obsolete P0 removal — dropping the permanently skipped Home starters plugin-input case is appropriate after the gallery retirement, with remaining P2 coverage still exercising plugin input binding.
  • Collab race hardening — the replacement client now dispatches pagehide (matching useCollableaveBeacon) and awaits the daemon leave before page.close(), and openHomeAndPinWorkspace waits for /api/workspace/events after reload so live invalidations are not emitted against a shell that has not finished hop 2.

No correctness, safety, or maintainability blockers in the changed ranges. Nice work making the P0 surface workspace-aware and less flaky. 🙌

🔁 Powered by Looper · runner=reviewer · agent=grok-build · An autonomous AI dev team for your GitHub repos.

@lefarcen
lefarcen requested a review from mrcfps August 5, 2026 05:35
@lefarcen lefarcen added size/XL PR changes 700-1500 lines and removed size/L PR changes 300-700 lines labels Aug 5, 2026

@mrcfps mrcfps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AmyShang-alt thank you for this careful E2E reliability cleanup — the collab slim-down is a real CI-budget win without abandoning the cross-layer witness.

I re-reviewed the full changed range on 1af2d83b:

  • Hermetic Vela statusrouteSignedOutVelaStatus in applyStandardMocks (and the targeted use in api-empty-response) keeps local/BYOK project-create flows independent of a host machine’s signed-in AMR session, while suite-level signed-in routes registered later still win under Playwright’s LIFO matching.
  • Suite consolidation — app / design-files / manual-edit / restoration now share applyStandardMocks, so they pick up the authority witness without changing their agent fixtures or standard config shape.
  • Coverage matrixentry-automations is wired consistently through uiP0Groups, uiP0CiMatrix, coverage files, the six-domain scope guard, and the packaged-smoke / scopes expectations, so the Automations P0 cases actually run in CI.
  • Obsolete P0 removal — dropping the permanently skipped Home starters plugin-input case is appropriate after the gallery retirement.
  • Collab cluster lifecycle — concurrent Promise.allSettled start with partial-acquisition cleanup, parallel close, and the new concurrent start/stop unit coverage look correct; port allocation still holds sockets until start and retries on conflict.
  • clusterTest + pin-before-navigate — skipping the unused worker runtime for the cluster-owned shard, pinning active Workspace before first navigation, waiting for /api/workspace/events, and keeping the P0 chain on content / presence / unshare / role transitions is a coherent cross-layer gate; lower-layer contracts remain owned elsewhere as the PR describes.

No correctness, safety, or maintainability blockers in the changed ranges. Nice work making the P0 surface workspace-aware, hermetic, and runnable under the 12-minute fleet budget. 🙌

🔁 Powered by Looper · runner=reviewer · agent=grok-build · An autonomous AI dev team for your GitHub repos.

@AmyShang-alt
AmyShang-alt added this pull request to the merge queue Aug 5, 2026
@AmyShang-alt
AmyShang-alt removed this pull request from the merge queue due to a manual request Aug 5, 2026
@AmyShang-alt
AmyShang-alt added this pull request to the merge queue Aug 5, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 5, 2026

@mrcfps mrcfps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AmyShang-alt thank you for this careful E2E reliability cleanup — the workspace split and upload assertion fix are a real CI-budget and flake-reduction win.

I re-reviewed the full changed range on 843fd1fd:

  • Hermetic Vela statusrouteSignedOutVelaStatus in applyStandardMocks (and the targeted use in api-empty-response) keeps local/BYOK project-create flows independent of a host machine’s signed-in AMR session, while suite-level signed-in routes registered later still win under Playwright’s LIFO matching.
  • Suite consolidation — app / design-files / manual-edit / restoration now share applyStandardMocks, so they pick up the authority witness without changing their agent fixtures or standard config shape.
  • Coverage matrixentry-automations and project-workspace-editor are wired consistently through uiP0Groups, uiP0CiMatrix, uiP0CoverageFiles, the guard’s seven-domain contract, and the scope/workflow topology tests; no file falls out of the merge gate.
  • Collab lifecycle — concurrent start/stop with partial-acquisition cleanup, clusterTest (no unused default worker runtime), pin-before-navigate + workspace event readiness, and the slimmed cross-layer witness remain sound; the new concurrency unit test pins the parallel lifecycle intent.
  • Assertion hardening — locator-native toHaveCSS avoids stale iframe execution contexts; removing the global reference.txt text match leaves the staged-attachment and post-send .user-attachments witnesses intact.

No correctness, safety, or maintainability blockers in the changed ranges. Nice work getting P0 coverage hermetic and keeping the long shards under the runner budget.

🔁 Powered by Looper · runner=reviewer · agent=grok-build · An autonomous AI dev team for your GitHub repos.

@AmyShang-alt
AmyShang-alt added this pull request to the merge queue Aug 6, 2026
Merged via the queue into main with commit 5a2e561 Aug 6, 2026
72 of 74 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk/medium Medium risk: regular code changes size/XL PR changes 700-1500 lines skip-validation Maintainer override: bot will not auto-add needs-validation on this PR. type/chore CI / build / config / tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants