Skip to content

feat: e2e tests with playwright - #7961

Open
shoom3301 wants to merge 37 commits into
developfrom
feat/e2e-playwright-2
Open

feat: e2e tests with playwright#7961
shoom3301 wants to merge 37 commits into
developfrom
feat/e2e-playwright-2

Conversation

@shoom3301

@shoom3301 shoom3301 commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Fixes:

What changed

  • Bootstraps a new Playwright + Synpress e2e suite at apps/cowswap-e2e-tests, replacing the removed legacy e2e tests.
  • Two wallet fixtures, picked deliberately per spec:
    • fixtures/mockWallet — an in-process viem account (EIP-6963 injected), instant signing, no extension. Used by every current spec.
    • fixtures (Synpress) — a real MetaMask extension, for connect/approve/network-switch UI itself.
  • A per-worker local JSON-RPC proxy (src/support/rpcProxy.ts) sits between the app and Sepolia so each test gets isolated chain-state stubs.
  • A fixture-driven mock layer for the CoW Protocol API (quote, postOrder, accountOrders, orderStatus, order, ...), ERC-20 allowances, and the balances-watcher SSE endpoint. Any unmocked CoW API call fails the test at teardown by default.
  • Page-object model (src/pages/) for Swap, Limit, TWAP, Account, TokenSelector, ConfirmModal, Header, Trade.
  • Initial spec coverage: market orders, limit orders, and network switching (7 tests across 3 spec files; 2 files tagged @smoke).

Why

  • The legacy e2e tests were flaky and slow to run in CI; this suite mocks the CoW Protocol API, balances, and allowances so runs are deterministic and don't depend on live backend state.
  • Splitting mock-wallet vs. Synpress/MetaMask lets most specs run fast (no extension boot) while still reserving real-extension coverage for the flows that actually need it
    (connect/approve/network-switch popups).
  • A PR-gating smoke subset keeps the merge queue fast; the nightly full run (sharded, fail-fast: false) catches regressions and Synpress/MetaMask version drift without blocking PRs.

Testing

Developer verification:

  • pnpm e2e:build-cache once, then pnpm --filter @cowprotocol/cowswap-e2e-pw exec playwright test runs the full suite locally.
  • pnpm e2e:ui runs Playwright app where you can manually run specific tests.
  • npx tsx --test "src/**/*.test.ts" covers the mocks/support code in isolation (no browser).

Reviewer note:

  • This PR ships test infrastructure, not a product change, so there's no app preview URL to click through.
  • Running E2E tests in CI is implemented in ci: run smoke and nightly e2e #7965. There also might be some leftovers of cypress which are removed in the mentioned PR.

Summary by CodeRabbit

  • New Features

    • Added a comprehensive Playwright/Synpress end-to-end testing suite for swaps, limit orders, network switching, wallets, balances, allowances, and API interactions.
    • Added reusable test tools for mocked wallets, token lists, prices, balances, quotes, routing, and order fulfillment.
    • Added support for Sepolia and Gnosis test scenarios.
  • Documentation

    • Added setup, configuration, execution, troubleshooting, and contribution guidance for the E2E test suite.
  • Tests

    • Added extensive coverage for wallet behavior, RPC handling, mocks, token resolution, and core trading flows.

@vercel

vercel Bot commented Aug 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cowfi Ready Ready Preview Aug 12, 2026 10:59am
explorer-dev Ready Ready Preview Aug 12, 2026 10:59am
storybook Ready Ready Preview Aug 12, 2026 10:59am
swap-dev Ready Ready Preview Aug 12, 2026 10:59am
widget-configurator Ready Ready Preview Aug 12, 2026 10:59am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
cosmos Ignored Ignored Aug 12, 2026 10:59am
sdk-tools Ignored Ignored Preview Aug 12, 2026 10:59am

Request Review

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Changes

The PR adds a complete Playwright/Synpress E2E application for CoW Swap. It includes wallet engines, RPC services, API and balance mocks, page objects, test fixtures, documentation, Nx targets, and smoke tests.

Playwright E2E application

Layer / File(s) Summary
Application setup and documentation
apps/cowswap-e2e-tests/*, AGENTS.md, package.json, eslint.config.js
Adds project configuration, dependencies, environment examples, commands, documentation, and lint rules.
Wallet engine and fixtures
apps/cowswap-e2e-tests/src/mockWallet/*, apps/cowswap-e2e-tests/src/fixtures/*, apps/cowswap-e2e-tests/src/support/wallet*
Adds mock and MetaMask wallet fixtures, EIP-1193 injection, auto-connect state, signing, chain switching, RPC stubbing, and wallet-cache support.
Allowance interception and resolution
apps/cowswap-e2e-tests/src/mocks/allowances/*
Adds validated allowance fixtures, override resolution, Multicall3 decoding, nested result patching, RPC URL handling, and Playwright interception.
Balance and CoW API mocks
apps/cowswap-e2e-tests/src/mocks/balances/*, apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/*
Adds fixture-backed balance SSE responses, CoW API endpoint matching, normalization, overrides, recording, posted-order tracking, and teardown assertions.
RPC support, setup helpers, and page objects
apps/cowswap-e2e-tests/src/support/*, apps/cowswap-e2e-tests/src/pages/*, apps/cowswap-e2e-tests/src/mocks/{bungee,nearIntents,safeSdk,tokenLists,usdPrices}.ts
Adds the RPC proxy, token and trade setup utilities, page objects, global lifecycle hooks, and auxiliary service mocks.
E2E scenarios and Synpress integration
apps/cowswap-e2e-tests/src/tests/*, patches/@synthetixio__synpress-metamask@0.0.11.patch
Adds limit-order, market-order, and network-switching tests. Updates Synpress MetaMask selectors, click handling, and popup cleanup.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant PlaywrightTest
  participant WalletFixture
  participant RpcProxy
  participant CowProtocolApiMock
  participant BalancesMock
  participant CowSwapUI

  PlaywrightTest->>WalletFixture: Configure wallet and chain
  WalletFixture->>RpcProxy: Send wallet RPC requests
  PlaywrightTest->>CowProtocolApiMock: Configure API responses
  PlaywrightTest->>BalancesMock: Configure balances and SSE snapshots
  CowSwapUI->>CowProtocolApiMock: Request quotes and orders
  CowSwapUI->>BalancesMock: Request balance sessions and updates
  CowSwapUI->>WalletFixture: Sign and submit trade operations
  WalletFixture->>RpcProxy: Forward or stub chain calls
  PlaywrightTest->>CowSwapUI: Assert trade and network states
Loading

Possibly related PRs

Suggested reviewers: danziger, kernelwhisperer

Poem

A rabbit checks the quote with care,
Then hops through wallets everywhere.
Mocked calls and balances gleam,
Tests flow neatly through the stream.
“Green paws!” it says, and bounds away.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: adding Playwright end-to-end tests.
Description check ✅ Passed The description explains the scope, motivation, test commands, and related issues, but does not include the template's self-checklist.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/e2e-playwright-2

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.

@socket-security

socket-security Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Added@​synthetixio/​synpress-metamask@​0.0.11901007291100
Addedtsx@​4.22.0881008291100
Addedexceljs@​4.4.08710010082100
Added@​synthetixio/​synpress@​4.0.10911008491100
Added@​playwright/​test@​1.49.110010010099100

View full report

@socket-security

socket-security Bot commented Aug 5, 2026

Copy link
Copy Markdown

Warning

Review the following alerts detected in dependencies.

According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.

Action Severity Alert  (click "▶" to expand/collapse)
Warn High
Obfuscated code: npm @esbuild/aix-ppc64 is 90.0% likely obfuscated

Confidence: 0.90

Location: Package overview

From: pnpm-lock.yamlnpm/@synthetixio/synpress@4.0.10npm/@synthetixio/synpress-metamask@0.0.11npm/@esbuild/aix-ppc64@0.19.12

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/@esbuild/aix-ppc64@0.19.12. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Warn High
Obfuscated code: npm exceljs is 90.0% likely obfuscated

Confidence: 0.90

Location: Package overview

From: apps/cowswap-e2e-tests/package.jsonnpm/exceljs@4.4.0

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/exceljs@4.4.0. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

@shoom3301
shoom3301 requested a review from a team August 5, 2026 12:56
@shoom3301
shoom3301 marked this pull request as ready for review August 5, 2026 12:56
@shoom3301 shoom3301 self-assigned this Aug 5, 2026
@shoom3301 shoom3301 added the e2e label Aug 5, 2026
… feat/e2e-playwright-1

# Conflicts:
#	apps/cowswap-frontend/src/modules/balancesAndAllowances/updaters/CommonPriorityBalancesAndAllowancesUpdater.tsx
Comment thread apps/cowswap-e2e-tests/src/fixtures/index.ts
@azebuado

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 20

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

🟡 Minor comments (13)
apps/cowswap-e2e-tests/src/tests/market-orders.spec.ts-27-27 (1)

27-27: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the expected quote value.

Both assertions accept any non-empty output. A stale default quote can satisfy them. Assert the configured quote value, or poll until the expected value is stable.

  • apps/cowswap-e2e-tests/src/tests/market-orders.spec.ts#L27-L27: replace the non-empty assertion with the expected output amount.
  • apps/cowswap-e2e-tests/src/tests/market-orders.spec.ts#L46-L46: replace the non-empty assertion with the expected output amount.

As per coding guidelines, tests must assert a specific expected value or poll for stability because stale default quotes must not satisfy assertions.

🤖 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 `@apps/cowswap-e2e-tests/src/tests/market-orders.spec.ts` at line 27, Replace
the non-empty output assertions in market-orders.spec.ts at lines 27-27 and
46-46 with assertions against the configured expected output amount, or poll
until that exact value is stable; both sites require the same change so stale
default quotes cannot satisfy the tests.

Source: Coding guidelines

apps/cowswap-e2e-tests/README.md-191-192 (1)

191-192: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Limit the cache prerequisite to Synpress tests.

The MetaMask cache is required only for Synpress fixtures. Mock-wallet tests do not use the extension or .cache-synpress. Update this command description so developers do not build the cache before mock-wallet-only runs.

🤖 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 `@apps/cowswap-e2e-tests/README.md` around lines 191 - 192, Update the
e2e:build-cache description in the README so it states that the Synpress
MetaMask profile cache is required only for Synpress tests, not all test runs;
leave the command itself and the full-suite entry unchanged.
apps/cowswap-e2e-tests/README.md-151-153 (1)

151-153: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Do not state a USDC amount without fixture decimals.

"5000000" is not always 5 USDC. The committed Sepolia fixture token labelled USDC has 18 decimals. State that the displayed amount depends on the token fixture decimals.

🤖 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 `@apps/cowswap-e2e-tests/README.md` around lines 151 - 153, Update the README
guidance around raw atom values to state that the displayed token amount depends
on the fixture token’s decimals, specifically noting that the committed Sepolia
fixture labeled USDC uses 18 decimals; avoid presenting “5000000” as universally
equal to 5 USDC, while preserving the existing guidance about stringifying
values above 2^53.
apps/cowswap-e2e-tests/src/mockWallet/injectedShim.ts-77-80 (1)

77-80: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Normalize the rejection value before the callback.

Promise rejection values can be non-Error values. Type the parameter as unknown. Normalize it once before passing it to callback.

Because injectedShim is serialized into the page, keep the normalization helper inside the serialized function.

Proposed fix
+  function normalizeError(err: unknown): Error {
+    return err instanceof Error ? err : new Error(String(err))
+  }
+
   const provider = {
     // ...
     send(
       // ...
     ): Promise<unknown> | void {
       if (typeof methodOrRequest === 'object' && typeof paramsOrCallback === 'function') {
         const callback = paramsOrCallback
         request(methodOrRequest).then(
           (result) => callback(null, { result }),
-          (error: Error) => callback(error, null),
+          (err: unknown) => {
+            const error = normalizeError(err)
+            callback(error, null)
+          },
         )

As per coding guidelines, promise rejection values must use (err: unknown) and const error = normalizeError(err) before use.

🤖 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 `@apps/cowswap-e2e-tests/src/mockWallet/injectedShim.ts` around lines 77 - 80,
Update the rejection handler in the serialized injectedShim function to accept
`(err: unknown)`, normalize it once with the local normalization helper, and
pass the resulting error to callback. Keep the helper inside the serialized
function and preserve the existing callback arguments.

Source: Coding guidelines

apps/cowswap-e2e-tests/src/support/rpcProxy.test.ts-89-93 (1)

89-93: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the /admin/reset postcondition.

The test invokes /admin/reset but does not verify that it cleared the worker stub. The test passes if the endpoint returns success without changing proxy state. Query the same balance after reset and assert the dummy upstream error.

🤖 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 `@apps/cowswap-e2e-tests/src/support/rpcProxy.test.ts` around lines 89 - 93,
Update the test around the /admin/reset request to query the same worker balance
again after the POST and assert that the dummy upstream error is returned,
verifying the reset cleared the worker stub rather than only relying on the
endpoint response.
apps/cowswap-e2e-tests/src/support/rpcProxy.ts-123-126 (1)

123-126: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Normalize rejected values at both catch sites.

Production TypeScript must type the rejection value as unknown and normalize it once before use. Replace each catch parameter with err: unknown, then create const error = normalizeError(err) before constructing the response.

  • apps/cowswap-e2e-tests/src/support/rpcProxy.ts#L123-L126: normalize the request-handler rejection before writing the HTTP error.
  • apps/cowswap-e2e-tests/src/support/rpcProxy.ts#L321-L325: normalize the forwarding rejection before creating the JSON-RPC error.
🤖 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 `@apps/cowswap-e2e-tests/src/support/rpcProxy.ts` around lines 123 - 126,
Normalize rejected values at both catch sites in
apps/cowswap-e2e-tests/src/support/rpcProxy.ts:123-126 and
apps/cowswap-e2e-tests/src/support/rpcProxy.ts:321-325 by typing each catch
parameter as unknown, calling normalizeError once to create error, and using
that normalized value for the HTTP response and JSON-RPC error respectively.

Source: Coding guidelines

apps/cowswap-e2e-tests/src/support/rpcProxy.ts-104-108 (1)

104-108: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Use getAddressKey for EVM address map keys.

Manual toLowerCase() normalization violates the address identity rule and can miss equivalent inputs.

  • apps/cowswap-e2e-tests/src/support/rpcProxy.ts#L104-L108: use getAddressKey for the address part of both balance and call stub keys.
  • apps/cowswap-e2e-tests/src/mocks/usdPrices.ts#L23-L25, apps/cowswap-e2e-tests/src/mocks/usdPrices.ts#L55-L61: use getAddressKey for price storage and lookup keys.
🤖 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 `@apps/cowswap-e2e-tests/src/support/rpcProxy.ts` around lines 104 - 108,
Replace manual toLowerCase() address normalization with getAddressKey in
balanceKey and callKey in apps/cowswap-e2e-tests/src/support/rpcProxy.ts (lines
104-108), applying it to each address component. Also update price storage in
apps/cowswap-e2e-tests/src/mocks/usdPrices.ts (lines 23-25) and price lookup in
lines 55-61 to use getAddressKey for their EVM address keys.

Source: Coding guidelines

apps/cowswap-e2e-tests/src/support/buildWalletCache.ts-99-101 (1)

99-101: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Normalize the rejected value before logging it.

Declare the handler parameter as err: unknown. Set const error = normalizeError(err) once. Log error instead of the raw rejection value.

As per coding guidelines, “Promise rejection values MUST be typed as (err: unknown) and normalized exactly once with const error = normalizeError(err) before use.”

🤖 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 `@apps/cowswap-e2e-tests/src/support/buildWalletCache.ts` around lines 99 -
101, Update the catch handler in buildWalletCache to type its rejection
parameter as err: unknown, normalize it exactly once with const error =
normalizeError(err), and pass the normalized error to console.error before
exiting.

Source: Coding guidelines

apps/cowswap-e2e-tests/src/mocks/allowances/fixture.ts-30-40 (1)

30-40: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Reject unsafe chain IDs before creating the lookup key.

Line 33 rounds decimal values above Number.MAX_SAFE_INTEGER. This can store an allowance under a different chain ID. Validate chainId with Number.isSafeInteger() after conversion.

Proposed fix
       const chainId = Number(chainKey)
+      if (!Number.isSafeInteger(chainId)) {
+        throw new Error(`${source}["${owner}"]: "${chainKey}" is not a safe integer chain id`)
+      }
 
       for (const [token, value] of entriesOf(byToken, `${source}["${owner}"]["${chainKey}"]`, 'chain entry')) {
🤖 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 `@apps/cowswap-e2e-tests/src/mocks/allowances/fixture.ts` around lines 30 - 40,
Update the chain ID validation in the allowance fixture before calling
allowanceKey: after converting chainKey to chainId, require
Number.isSafeInteger(chainId) and reject unsafe values with the existing
invalid-chain error path. Keep the decimal-format validation and lookup behavior
unchanged for safe integer IDs.
apps/cowswap-e2e-tests/src/mocks/allowances/rpcUrls.ts-10-13 (1)

10-13: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Normalize the pathname before appending the query string.

Line 12 does not normalize https://rpc.example/path/?key=value to the same value as https://rpc.example/path?key=value. This contradicts the trailing-slash contract and can prevent route interception.

Proposed fix
 export function normalizeRpcUrl(url: string): string {
   const parsed = new URL(url)
-  return `${parsed.origin}${parsed.pathname}${parsed.search}`.replace(/\/$/, '')
+  return `${parsed.origin}${parsed.pathname.replace(/\/$/, '')}${parsed.search}`
 }

Add a test that covers a trailing slash before a query string.

🤖 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 `@apps/cowswap-e2e-tests/src/mocks/allowances/rpcUrls.ts` around lines 10 - 13,
The normalizeRpcUrl function currently removes a trailing slash only after
appending the query string, so paths ending in "/?" are not normalized
correctly. Normalize parsed.pathname before concatenating parsed.search,
preserving the query string, and add a test covering a trailing slash before a
query string.
apps/cowswap-e2e-tests/src/mocks/allowances/index.ts-120-124 (1)

120-124: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Normalize the caught rejection value once.

Bind this catch handler to (err: unknown), then use const error = normalizeError(err) before recording the problem.

🤖 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 `@apps/cowswap-e2e-tests/src/mocks/allowances/index.ts` around lines 120 - 124,
Update the catch handler in the allowance mock route to bind the rejection as
err: unknown, normalize it once with normalizeError(err), and use the resulting
error when pushing the problem message. Preserve the existing route.continue()
fallback behavior.

Source: Coding guidelines

apps/cowswap-e2e-tests/src/mocks/balances/fixture.ts-30-33 (1)

30-33: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject chain IDs that cannot be represented safely.

DECIMAL_RE accepts values that Number() rounds or converts to Infinity. This can create incorrect lookup keys. Validate chainId with Number.isSafeInteger() before using it. Reject non-positive values if chain ID zero is not supported.

Proposed fix
       if (!DECIMAL_RE.test(chainKey)) {
         throw new Error(`${source}["${owner}"]: "${chainKey}" is not a valid chain id`)
       }
       const chainId = Number(chainKey)
+      if (!Number.isSafeInteger(chainId) || chainId <= 0) {
+        throw new Error(`${source}["${owner}"]: "${chainKey}" is not a valid chain id`)
+      }
🤖 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 `@apps/cowswap-e2e-tests/src/mocks/balances/fixture.ts` around lines 30 - 33,
Validate the `chainId` produced in the chain-key parsing flow with
`Number.isSafeInteger()` and require it to be positive before using it as a
lookup key. Extend the existing error path near `DECIMAL_RE` and
`Number(chainKey)` to reject unsafe, infinite, fractional, or non-positive
values while preserving the current valid-chain behavior.
apps/cowswap-e2e-tests/src/mocks/balances/types.ts-21-27 (1)

21-27: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Use getAddressKey for the owner balance key.

balanceKey and balanceOwnerChainPrefix lowercase the owner manually. Replace those owner normalizations with getAddressKey() so the balance mock uses the repository address-key contract.

🤖 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 `@apps/cowswap-e2e-tests/src/mocks/balances/types.ts` around lines 21 - 27,
Update balanceKey and balanceOwnerChainPrefix to normalize the owner with
getAddressKey() instead of manually calling toLowerCase(), while preserving the
existing chain and token formatting.

Source: Coding guidelines

🧹 Nitpick comments (3)
apps/cowswap-e2e-tests/src/mocks/allowances/index.ts (1)

45-45: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Handle route registration asynchronously before returning the mock.

BrowserContext.route() returns a Promise, and installAllowances() returns synchronously. Other mock installers discard the same Promise, but this API’s caller path also has no await surface, so tests can assume allowance interception is installed immediately. Make the registration awaitable where the mock is installed, or otherwise document that interception is asynchronous.

🤖 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 `@apps/cowswap-e2e-tests/src/mocks/allowances/index.ts` at line 45, Update
installAllowances to await BrowserContext.route before resolving, changing its
return type as needed to expose an awaitable installation; update its callers to
await the returned mock before issuing requests. Ensure allowance interception
is registered before the mock is used rather than relying on synchronous return
timing.
apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/types.ts (1)

47-47: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Simplify the factory return type.

unknown | Promise<unknown> collapses to unknown in TypeScript. The union reads as if it constrains the return value, but it does not. resolveResponse already awaits the result, so unknown alone is accurate.

♻️ Proposed change
-export type CowApiOverrideFactory = (req: CowApiRequest) => unknown | Promise<unknown>
+/** May return a value or a promise; `resolveResponse` awaits the result. */
+export type CowApiOverrideFactory = (req: CowApiRequest) => unknown
🤖 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 `@apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/types.ts` at line 47, Update
the CowApiOverrideFactory return type to unknown instead of unknown |
Promise<unknown>; resolveResponse already awaits the factory result, so preserve
its existing behavior while simplifying the type declaration.
apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/record.ts (1)

105-105: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add a timeout to the recording fetch call.

If the CoW API stops responding, the unbounded request can let record.mocks run for an entry without output or progress. The declared Node range (>=20.19 <23) supports AbortSignal.timeout, so attach signal: AbortSignal.timeout(30_000) to both GET and POST options.

🤖 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 `@apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/record.ts` at line 105, Add a
30-second timeout to the fetch request in the recording flow by attaching
AbortSignal.timeout(30_000) to the request options used for both GET and POST
calls. Update the shared options around the fetch invocation so both methods
receive the signal without changing their existing behavior.
🤖 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 `@apps/cowswap-e2e-tests/playwright.config.ts`:
- Around line 31-33: Update the webServer command in the Playwright
configuration to use pnpm start:cowswap instead of pnpm preview, while
preserving the existing cwd and other web server settings.

In `@apps/cowswap-e2e-tests/src/fixtures/mockWallet.ts`:
- Around line 110-114: Update the initialization flow around openApp() and the
injectedShim setup so the initial chain is derived from the URL hash, falling
back to chainIdHex when no chain is specified. Pass that resolved chain to the
shim instead of always using toHex(CHAIN_IDS.SEPOLIA), keeping
window.ethereum.chainId consistent with eth_chainId.

In `@apps/cowswap-e2e-tests/src/fixtures/rpcProxy.ts`:
- Around line 22-41: Update the administrative requests in setBalance, stubCall,
and reset to retain each fetch response, validate response.ok, and throw an
error containing the specific endpoint and HTTP status when a request fails.
Preserve the existing request payloads and methods for successful responses.

In `@apps/cowswap-e2e-tests/src/mocks/allowances/codec.ts`:
- Around line 122-130: Replace manual toLowerCase() address normalization in
classifyAllowance with getAddressKey from `@cowprotocol/cow-sdk` for token, owner,
and spender. Update the matching expected allowance identities in
apps/cowswap-e2e-tests/src/mocks/allowances/codec.test.ts at lines 69-77,
95-101, 154-163, and 173-183 to use the same getAddressKey representation.

In `@apps/cowswap-e2e-tests/src/mocks/allowances/types.ts`:
- Around line 22-28: Update allowanceKey() and ownerKeyPrefix() to normalize
addresses through getAddressKey() instead of calling toLowerCase() directly for
owner and token values. Preserve the existing key format and ensure both fixture
and runtime allowance lookups use the canonical normalization path.

In `@apps/cowswap-e2e-tests/src/mocks/balances/index.ts`:
- Around line 116-121: Update the SSE response body in the balance mock route
around route.fulfill to remove the 500 ms retry configuration or replace it with
a retry interval longer than the browser’s default reconnect delay, while
preserving the existing balance_update snapshot payload.

In `@apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/index.ts`:
- Line 77: Update installCowProtocolApi to return the context.route(...)
promise, and await that installer from the shared mocks.cowApi fixture before
exposing the mock. Keep CowProtocolApiMock methods synchronous by resolving the
registration promise internally in set, while preserving the existing mock API.

In `@apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/normalize.ts`:
- Around line 163-166: Replace the toLowerCase() normalization in subjectAddress
with getAddressKey from `@cowprotocol/cow-sdk` while preserving undefined when no
address is provided. Update install.test.ts lines 104-107 to compare order.owner
and order.receiver with getAddressKey(ADDRESS), and resolve.test.ts lines 41-46
to compare them with getAddressKey(TRADER).

In `@apps/cowswap-e2e-tests/src/mocks/safeSdk.ts`:
- Around line 24-35: Normalize the Safe RPC method name before the response
selection in the mock handler: strip the `safe_` prefix from `msg.method` or
compare against the full prefixed names so `safe_getSafeInfo` returns the safe
details and `safe_getEnvironmentInfo` returns the environment data, while
preserving the empty object fallback for other Safe methods.

In `@apps/cowswap-e2e-tests/src/mockWallet/walletEngine.ts`:
- Line 79: Replace the cast-only RPC response handling near the `res.json()`
call and in `eth_signTypedData` with `unknown` values followed by runtime
validation of the required response and typed-data fields. Narrow validated
values before forwarding them to viem, and reject malformed or missing RPC
payloads rather than relying on TypeScript assertions.
- Around line 166-167: Update the catch block in handle() to catch the rejection
as err: unknown and normalize it exactly once with normalizeError(err). Preserve
the normalized EIP-1193 code and data when constructing the RpcError, then
return the failure envelope as { ok: false, error }.

In `@apps/cowswap-e2e-tests/src/pages/HeaderPage.ts`:
- Around line 5-19: In apps/cowswap-e2e-tests/src/pages/HeaderPage.ts lines
5-19, add readonly constructor properties for the header and network-dialog
Locators, and scope the dynamic currentNetworkLabel and targetNetworkLabel
locators from those properties within switchNetwork. In
apps/cowswap-e2e-tests/src/pages/TokenSelector.ts lines 4-17, add readonly
constructor properties for the input selector, output selector, search input,
and currency-list Locators, then reuse them in the page-object action methods.

In `@apps/cowswap-e2e-tests/src/pages/SwapPage.ts`:
- Around line 135-138: Update the mockSwapFulfillment flow to accept and use
buyTokenBalanceBefore when setting balances. In the balances.set call, preserve
the existing buy-token balance by storing buyTokenBalanceBefore +
BigInt(body.buyAmount) instead of replacing it with body.buyAmount, while
keeping the sell-token deduction unchanged.

In `@apps/cowswap-e2e-tests/src/support/rpcProxy.ts`:
- Around line 114-121: Update forward() to use an AbortController with a bounded
timeout when calling fetch(opts.sepoliaRpcUrl, ...), pass its signal to fetch,
and clear the timeout in cleanup after resolution or rejection so stalled
upstream requests abort and tryForward() can return its JSON-RPC error.

In `@apps/cowswap-e2e-tests/src/support/setupTestConditions.test.ts`:
- Around line 91-98: Update the expected USDC balance in the balances.calls
assertion to use the committed Sepolia fixture’s 18-decimal value,
56_000_000_000_000_000_000_000n, while leaving the WETH and allowance
expectations unchanged.

In `@apps/cowswap-e2e-tests/src/support/tokens.ts`:
- Around line 10-13: Update the Sepolia USDC metadata in the CHAIN_IDS.SEPOLIA
token configuration to use 18 decimals instead of 6, and add a matching
assertion in the token metadata tests confirming the Sepolia USDC fixture has 18
decimals.

In `@apps/cowswap-e2e-tests/src/support/wallet.setup.ts`:
- Around line 25-27: Update the button-click flow around button.isVisible() so
clicked is set to true only when button.click succeeds; do not suppress a failed
click while marking it completed. If the locator remains visible after failure,
apply a bounded retry or propagate the normalized error so the quiet counter
cannot reset indefinitely.

In `@apps/cowswap-e2e-tests/src/tests/limit-orders.spec.ts`:
- Line 27: Move the specified interactions behind page-object APIs: in
apps/cowswap-e2e-tests/src/tests/limit-orders.spec.ts at line 27, add and call a
LimitPage action that clicks placeOrderButton; in
apps/cowswap-e2e-tests/src/tests/market-orders.spec.ts at line 135, add and call
a ConfirmModal action that confirms the trade; and at line 179, add a readonly
warning locator to SwapPage and assert it from the spec. Keep selectors and
interactions out of the specs.
- Around line 11-30: Add a test.beforeEach fixture with default mocked token
balances to the test.describe blocks in
apps/cowswap-e2e-tests/src/tests/limit-orders.spec.ts (11-30),
apps/cowswap-e2e-tests/src/tests/market-orders.spec.ts (16-181), and
apps/cowswap-e2e-tests/src/tests/network.spec.ts (8-23). Keep each test’s
scenario-specific balances in setupTestConditions as overrides of those shared
defaults.

In `@package.json`:
- Line 143: Align the patched dependency key with the installed
`@synthetixio/synpress-metamask` version: regenerate and reference the patch for
version 0.0.14, updating related package metadata or lockfile entries as needed,
or consistently pin the dependency to patched version 0.0.11.

---

Minor comments:
In `@apps/cowswap-e2e-tests/README.md`:
- Around line 191-192: Update the e2e:build-cache description in the README so
it states that the Synpress MetaMask profile cache is required only for Synpress
tests, not all test runs; leave the command itself and the full-suite entry
unchanged.
- Around line 151-153: Update the README guidance around raw atom values to
state that the displayed token amount depends on the fixture token’s decimals,
specifically noting that the committed Sepolia fixture labeled USDC uses 18
decimals; avoid presenting “5000000” as universally equal to 5 USDC, while
preserving the existing guidance about stringifying values above 2^53.

In `@apps/cowswap-e2e-tests/src/mocks/allowances/fixture.ts`:
- Around line 30-40: Update the chain ID validation in the allowance fixture
before calling allowanceKey: after converting chainKey to chainId, require
Number.isSafeInteger(chainId) and reject unsafe values with the existing
invalid-chain error path. Keep the decimal-format validation and lookup behavior
unchanged for safe integer IDs.

In `@apps/cowswap-e2e-tests/src/mocks/allowances/index.ts`:
- Around line 120-124: Update the catch handler in the allowance mock route to
bind the rejection as err: unknown, normalize it once with normalizeError(err),
and use the resulting error when pushing the problem message. Preserve the
existing route.continue() fallback behavior.

In `@apps/cowswap-e2e-tests/src/mocks/allowances/rpcUrls.ts`:
- Around line 10-13: The normalizeRpcUrl function currently removes a trailing
slash only after appending the query string, so paths ending in "/?" are not
normalized correctly. Normalize parsed.pathname before concatenating
parsed.search, preserving the query string, and add a test covering a trailing
slash before a query string.

In `@apps/cowswap-e2e-tests/src/mocks/balances/fixture.ts`:
- Around line 30-33: Validate the `chainId` produced in the chain-key parsing
flow with `Number.isSafeInteger()` and require it to be positive before using it
as a lookup key. Extend the existing error path near `DECIMAL_RE` and
`Number(chainKey)` to reject unsafe, infinite, fractional, or non-positive
values while preserving the current valid-chain behavior.

In `@apps/cowswap-e2e-tests/src/mocks/balances/types.ts`:
- Around line 21-27: Update balanceKey and balanceOwnerChainPrefix to normalize
the owner with getAddressKey() instead of manually calling toLowerCase(), while
preserving the existing chain and token formatting.

In `@apps/cowswap-e2e-tests/src/mockWallet/injectedShim.ts`:
- Around line 77-80: Update the rejection handler in the serialized injectedShim
function to accept `(err: unknown)`, normalize it once with the local
normalization helper, and pass the resulting error to callback. Keep the helper
inside the serialized function and preserve the existing callback arguments.

In `@apps/cowswap-e2e-tests/src/support/buildWalletCache.ts`:
- Around line 99-101: Update the catch handler in buildWalletCache to type its
rejection parameter as err: unknown, normalize it exactly once with const error
= normalizeError(err), and pass the normalized error to console.error before
exiting.

In `@apps/cowswap-e2e-tests/src/support/rpcProxy.test.ts`:
- Around line 89-93: Update the test around the /admin/reset request to query
the same worker balance again after the POST and assert that the dummy upstream
error is returned, verifying the reset cleared the worker stub rather than only
relying on the endpoint response.

In `@apps/cowswap-e2e-tests/src/support/rpcProxy.ts`:
- Around line 123-126: Normalize rejected values at both catch sites in
apps/cowswap-e2e-tests/src/support/rpcProxy.ts:123-126 and
apps/cowswap-e2e-tests/src/support/rpcProxy.ts:321-325 by typing each catch
parameter as unknown, calling normalizeError once to create error, and using
that normalized value for the HTTP response and JSON-RPC error respectively.
- Around line 104-108: Replace manual toLowerCase() address normalization with
getAddressKey in balanceKey and callKey in
apps/cowswap-e2e-tests/src/support/rpcProxy.ts (lines 104-108), applying it to
each address component. Also update price storage in
apps/cowswap-e2e-tests/src/mocks/usdPrices.ts (lines 23-25) and price lookup in
lines 55-61 to use getAddressKey for their EVM address keys.

In `@apps/cowswap-e2e-tests/src/tests/market-orders.spec.ts`:
- Line 27: Replace the non-empty output assertions in market-orders.spec.ts at
lines 27-27 and 46-46 with assertions against the configured expected output
amount, or poll until that exact value is stable; both sites require the same
change so stale default quotes cannot satisfy the tests.

---

Nitpick comments:
In `@apps/cowswap-e2e-tests/src/mocks/allowances/index.ts`:
- Line 45: Update installAllowances to await BrowserContext.route before
resolving, changing its return type as needed to expose an awaitable
installation; update its callers to await the returned mock before issuing
requests. Ensure allowance interception is registered before the mock is used
rather than relying on synchronous return timing.

In `@apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/record.ts`:
- Line 105: Add a 30-second timeout to the fetch request in the recording flow
by attaching AbortSignal.timeout(30_000) to the request options used for both
GET and POST calls. Update the shared options around the fetch invocation so
both methods receive the signal without changing their existing behavior.

In `@apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/types.ts`:
- Line 47: Update the CowApiOverrideFactory return type to unknown instead of
unknown | Promise<unknown>; resolveResponse already awaits the factory result,
so preserve its existing behavior while simplifying the type declaration.
🪄 Autofix

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: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4df44055-ca9f-43cf-8ac3-7ceaab0370ee

📥 Commits

Reviewing files that changed from the base of the PR and between fcbfb4e and 0d64709.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (92)
  • AGENTS.md
  • apps/cowswap-e2e-tests/.env.example
  • apps/cowswap-e2e-tests/.gitignore
  • apps/cowswap-e2e-tests/AGENTS.md
  • apps/cowswap-e2e-tests/README.md
  • apps/cowswap-e2e-tests/package.json
  • apps/cowswap-e2e-tests/playwright.config.ts
  • apps/cowswap-e2e-tests/project.json
  • apps/cowswap-e2e-tests/src/fixtures/index.ts
  • apps/cowswap-e2e-tests/src/fixtures/mockWallet.ts
  • apps/cowswap-e2e-tests/src/fixtures/rpcProxy.ts
  • apps/cowswap-e2e-tests/src/fixtures/shared.ts
  • apps/cowswap-e2e-tests/src/fixtures/synpress.ts
  • apps/cowswap-e2e-tests/src/fixtures/wallet.ts
  • apps/cowswap-e2e-tests/src/mockWallet/injectedShim.ts
  • apps/cowswap-e2e-tests/src/mockWallet/seedAutoConnect.ts
  • apps/cowswap-e2e-tests/src/mockWallet/walletEngine.test.ts
  • apps/cowswap-e2e-tests/src/mockWallet/walletEngine.ts
  • apps/cowswap-e2e-tests/src/mocks/allowances/codec.test.ts
  • apps/cowswap-e2e-tests/src/mocks/allowances/codec.ts
  • apps/cowswap-e2e-tests/src/mocks/allowances/fixture.test.ts
  • apps/cowswap-e2e-tests/src/mocks/allowances/fixture.ts
  • apps/cowswap-e2e-tests/src/mocks/allowances/fixtures/allowances.json
  • apps/cowswap-e2e-tests/src/mocks/allowances/index.ts
  • apps/cowswap-e2e-tests/src/mocks/allowances/resolve.test.ts
  • apps/cowswap-e2e-tests/src/mocks/allowances/resolve.ts
  • apps/cowswap-e2e-tests/src/mocks/allowances/rpcUrls.test.ts
  • apps/cowswap-e2e-tests/src/mocks/allowances/rpcUrls.ts
  • apps/cowswap-e2e-tests/src/mocks/allowances/types.ts
  • apps/cowswap-e2e-tests/src/mocks/balances/fixture.test.ts
  • apps/cowswap-e2e-tests/src/mocks/balances/fixture.ts
  • apps/cowswap-e2e-tests/src/mocks/balances/fixtures/balances.json
  • apps/cowswap-e2e-tests/src/mocks/balances/index.ts
  • apps/cowswap-e2e-tests/src/mocks/balances/resolve.test.ts
  • apps/cowswap-e2e-tests/src/mocks/balances/resolve.ts
  • apps/cowswap-e2e-tests/src/mocks/balances/types.ts
  • apps/cowswap-e2e-tests/src/mocks/bungee.ts
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/endpoints.test.ts
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/endpoints.ts
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/fixtures/accountOrders.json
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/fixtures/appData.json
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/fixtures/nativePrice.json
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/fixtures/order.json
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/fixtures/orderStatus.json
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/fixtures/quote.json
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/fixtures/solverCompetition.json
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/fixtures/solverCompetitionByTx.json
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/fixtures/totalSurplus.json
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/fixtures/trades.json
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/fixtures/transactionOrders.json
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/fixtures/version.json
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/index.ts
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/install.test.ts
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/networks.ts
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/normalize.ts
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/record.ts
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/resolve.test.ts
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/resolve.ts
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/types.ts
  • apps/cowswap-e2e-tests/src/mocks/nearIntents.ts
  • apps/cowswap-e2e-tests/src/mocks/safeSdk.ts
  • apps/cowswap-e2e-tests/src/mocks/tokenLists.ts
  • apps/cowswap-e2e-tests/src/mocks/usdPrices.ts
  • apps/cowswap-e2e-tests/src/pages/AccountPage.ts
  • apps/cowswap-e2e-tests/src/pages/ConfirmModal.ts
  • apps/cowswap-e2e-tests/src/pages/HeaderPage.ts
  • apps/cowswap-e2e-tests/src/pages/LimitPage.ts
  • apps/cowswap-e2e-tests/src/pages/SwapPage.ts
  • apps/cowswap-e2e-tests/src/pages/TokenSelector.ts
  • apps/cowswap-e2e-tests/src/pages/TradePage.ts
  • apps/cowswap-e2e-tests/src/pages/TwapPage.ts
  • apps/cowswap-e2e-tests/src/support/buildWalletCache.ts
  • apps/cowswap-e2e-tests/src/support/constants.ts
  • apps/cowswap-e2e-tests/src/support/globalSetup.ts
  • apps/cowswap-e2e-tests/src/support/globalTeardown.ts
  • apps/cowswap-e2e-tests/src/support/rpcProxy.test.ts
  • apps/cowswap-e2e-tests/src/support/rpcProxy.ts
  • apps/cowswap-e2e-tests/src/support/setupTestConditions.test.ts
  • apps/cowswap-e2e-tests/src/support/setupTestConditions.ts
  • apps/cowswap-e2e-tests/src/support/synpress.ts
  • apps/cowswap-e2e-tests/src/support/tokens.test.ts
  • apps/cowswap-e2e-tests/src/support/tokens.ts
  • apps/cowswap-e2e-tests/src/support/wallet.setup.ts
  • apps/cowswap-e2e-tests/src/support/walletSetupHashProbe.spec.ts
  • apps/cowswap-e2e-tests/src/tests/limit-orders.spec.ts
  • apps/cowswap-e2e-tests/src/tests/market-orders.spec.ts
  • apps/cowswap-e2e-tests/src/tests/network.spec.ts
  • apps/cowswap-e2e-tests/tsconfig.json
  • bundle-size.jsonc
  • eslint.config.js
  • package.json
  • patches/@synthetixio__synpress-metamask@0.0.11.patch

Comment thread apps/cowswap-e2e-tests/playwright.config.ts
Comment thread apps/cowswap-e2e-tests/src/fixtures/mockWallet.ts
Comment thread apps/cowswap-e2e-tests/src/fixtures/rpcProxy.ts Outdated
Comment thread apps/cowswap-e2e-tests/src/mocks/allowances/codec.ts
Comment thread apps/cowswap-e2e-tests/src/mocks/allowances/types.ts Outdated
Comment thread apps/cowswap-e2e-tests/src/support/tokens.ts
Comment thread apps/cowswap-e2e-tests/src/support/wallet.setup.ts Outdated
Comment thread apps/cowswap-e2e-tests/src/tests/limit-orders.spec.ts
Comment thread apps/cowswap-e2e-tests/src/tests/limit-orders.spec.ts Outdated
Comment thread package.json
@shoom3301
shoom3301 requested a review from Danziger August 11, 2026 07:39

@shoom3301 shoom3301 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

AI Review (Claude Sonnet 5, worked ~10m): follow-up addressed, no new findings

Rechecked

  • WalletConnect downgrade finding: fixed in b6a1aa9c9. package.json resolutions/dependencies/pnpm.peerDependencyRules.allowedVersions for @walletconnect/* are back to 2.23.10, matching develop. bundle-size.jsonc (which still carried the pre-fix numbers when this was first fixed) has since been corrected by the automatic bundle-size job in 87961b30f and now matches develop exactly — no diff remains.
  • Two additional issues found in a follow-up pass over this PR's own new code, fixed in bee89ccc1:
    • apps/cowswap-e2e-tests/src/mocks/allowances/resolve.test.ts: two tests ('matches regardless of address case', 'isOwnerConfigured sees owners from either map') called .toUpperCase() on the whole address including the 0x prefix, which @cowprotocol/cow-sdk's isEvmAddress doesn't recognize as an EVM address (it requires a literal lowercase 0x), so getAddressKey skipped normalization and the lookup missed. Fixed to uppercase only the hex body.
    • apps/cowswap-e2e-tests/AGENTS.md: the "Known issues" section referenced a bwEnabledPercentage LaunchDarkly flag already removed from CommonPriorityBalancesAndAllowancesUpdater.tsx in #7985 (already in this branch's history) — updated to describe the current isWatcherActive-based gating.

Result: All fixed. Re-ran npx tsx --test "src/**/*.test.ts" (123/123 pass), tsc --noEmit, and nx run cowswap-e2e-tests:lint against current HEAD (da89b12ca) — clean.

No new findings this round.

Review scope and related context

All 20 coderabbitai threads on this PR are resolved and hold up against current code (spot-checked safeSdk.ts, walletEngine.ts, SwapPage.mockSwapFulfillment, tokens.ts, wallet.setup.ts). This comment closes out the one thread (discussion_r3750735358) that was still open, plus reports two issues found and fixed outside any existing thread.

Generated using the pr-review skill from the CoW Protocol skills repo.

Base automatically changed from feat/e2e-playwright-1 to develop August 11, 2026 12:00

@coderabbitai coderabbitai Bot 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.

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)
apps/cowswap-e2e-tests/src/pages/SwapPage.ts (1)

83-89: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Wait for the current quote before submitting orders.

data-isLoading is absent when no request is active, so the predicate can pass before the request for the current input starts. Apply a request-specific completion check or assert the expected output value in SwapPage.ts, LimitPage.ts, and TwapPage.ts.

🤖 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 `@apps/cowswap-e2e-tests/src/pages/SwapPage.ts` around lines 83 - 89, The quote
wait logic in waitForQuote must not complete while no request is active; replace
the data-isLoading-only predicate with a request-specific completion check or
validation of the expected output value. Apply the same correction in
SwapPage.ts (waitForQuote), LimitPage.ts (its quote-wait flow), and TwapPage.ts
so orders are submitted only after the current input’s quote has completed.
🤖 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 `@apps/cowswap-e2e-tests/src/mocks/balances/index.ts`:
- Around line 116-121: Update handleSse() so balances.set() invoked by postOrder
delivers the updated snapshot to the active EventSource response
deterministically, rather than relying on retry: 800 reconnects. Add a direct
SSE push or explicit synchronization between the balance update and response
delivery, while preserving the existing initial snapshot behavior.

In `@apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/index.ts`:
- Line 77: Await every installCowProtocolApi invocation, including the calls in
shared.ts and install.test.ts, so route registration completes before exposing
CowProtocolApiMock or starting navigation. Propagate async setup through the
surrounding fixture and test functions as needed while preserving existing mock
behavior.

---

Outside diff comments:
In `@apps/cowswap-e2e-tests/src/pages/SwapPage.ts`:
- Around line 83-89: The quote wait logic in waitForQuote must not complete
while no request is active; replace the data-isLoading-only predicate with a
request-specific completion check or validation of the expected output value.
Apply the same correction in SwapPage.ts (waitForQuote), LimitPage.ts (its
quote-wait flow), and TwapPage.ts so orders are submitted only after the current
input’s quote has completed.
🪄 Autofix

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: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d9ae55b2-f057-41a2-9bc8-e2917da78716

📥 Commits

Reviewing files that changed from the base of the PR and between 0d64709 and 1617843.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (29)
  • apps/cowswap-e2e-tests/AGENTS.md
  • apps/cowswap-e2e-tests/package.json
  • apps/cowswap-e2e-tests/playwright.config.ts
  • apps/cowswap-e2e-tests/src/fixtures/rpcProxy.ts
  • apps/cowswap-e2e-tests/src/fixtures/shared.ts
  • apps/cowswap-e2e-tests/src/mockWallet/walletEngine.ts
  • apps/cowswap-e2e-tests/src/mocks/allowances/codec.test.ts
  • apps/cowswap-e2e-tests/src/mocks/allowances/codec.ts
  • apps/cowswap-e2e-tests/src/mocks/allowances/resolve.test.ts
  • apps/cowswap-e2e-tests/src/mocks/allowances/types.ts
  • apps/cowswap-e2e-tests/src/mocks/balances/index.ts
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/index.ts
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/install.test.ts
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/normalize.ts
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/resolve.test.ts
  • apps/cowswap-e2e-tests/src/mocks/safeSdk.ts
  • apps/cowswap-e2e-tests/src/pages/ConfirmModal.ts
  • apps/cowswap-e2e-tests/src/pages/HeaderPage.ts
  • apps/cowswap-e2e-tests/src/pages/LimitPage.ts
  • apps/cowswap-e2e-tests/src/pages/SwapPage.ts
  • apps/cowswap-e2e-tests/src/pages/TokenSelector.ts
  • apps/cowswap-e2e-tests/src/support/rpcProxy.ts
  • apps/cowswap-e2e-tests/src/support/setupTestConditions.test.ts
  • apps/cowswap-e2e-tests/src/support/tokens.test.ts
  • apps/cowswap-e2e-tests/src/support/tokens.ts
  • apps/cowswap-e2e-tests/src/support/wallet.setup.ts
  • apps/cowswap-e2e-tests/src/tests/limit-orders.spec.ts
  • apps/cowswap-e2e-tests/src/tests/market-orders.spec.ts
  • package.json
🚧 Files skipped from review as they are similar to previous changes (19)
  • apps/cowswap-e2e-tests/playwright.config.ts
  • apps/cowswap-e2e-tests/src/fixtures/rpcProxy.ts
  • apps/cowswap-e2e-tests/src/pages/HeaderPage.ts
  • apps/cowswap-e2e-tests/src/pages/ConfirmModal.ts
  • apps/cowswap-e2e-tests/src/tests/limit-orders.spec.ts
  • apps/cowswap-e2e-tests/src/support/setupTestConditions.test.ts
  • apps/cowswap-e2e-tests/src/mocks/allowances/types.ts
  • apps/cowswap-e2e-tests/src/support/tokens.test.ts
  • apps/cowswap-e2e-tests/package.json
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/install.test.ts
  • apps/cowswap-e2e-tests/src/support/tokens.ts
  • apps/cowswap-e2e-tests/src/tests/market-orders.spec.ts
  • apps/cowswap-e2e-tests/src/mocks/safeSdk.ts
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/normalize.ts
  • apps/cowswap-e2e-tests/src/mockWallet/walletEngine.ts
  • apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/resolve.test.ts
  • apps/cowswap-e2e-tests/src/mocks/allowances/codec.test.ts
  • apps/cowswap-e2e-tests/src/mocks/allowances/codec.ts
  • apps/cowswap-e2e-tests/src/support/rpcProxy.ts

Comment thread apps/cowswap-e2e-tests/src/mocks/balances/index.ts
Comment thread apps/cowswap-e2e-tests/src/mocks/cowProtocolApi/index.ts
… feat/e2e-playwright-2

# Conflicts:
#	pnpm-lock.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants