test(e2e): opt-in live Reserve/zapper API validation mode (zrs1) - #1088
test(e2e): opt-in live Reserve/zapper API validation mode (zrs1)#1088TheFrozenFire wants to merge 1 commit into
Conversation
Adds a `live` Playwright project (`pnpm e2e:live`) that swaps the API mock boundary for a recording, contract-validating passthrough. Reserve and zapper/planner surfaces are configured independently via E2E_LIVE_RESERVE_API / E2E_LIVE_ZAPPER_API (production, staging, zrs1, or an absolute URL); everything else (RPC, subgraph, wallet, receipts, chain state) stays mocked and the offline suites are unchanged. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
📝 WalkthroughWalkthroughThe E2E suite adds an opt-in live project for Reserve and Zapper APIs. It routes configured requests to live targets, validates responses against contracts, records violations, and adds API, pricing, and zap widget coverage. ChangesLive API validation
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant Test as Live Playwright test
participant Routes as mockApiRoutes
participant Live as fulfillFromLive
participant API as Reserve or Zapper API
participant Contracts as validateLiveResponse
Test->>Routes: Send API request
Routes->>Live: Route configured surface
Live->>API: Fetch rewritten live URL
API-->>Live: Return response
Live->>Contracts: Validate status, schema, and invariants
Contracts-->>Live: Return violations or valid result
Live-->>Test: Fulfill browser response
Suggested reviewers: 🚥 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 |
Deploying register-app with
|
| Latest commit: |
78327db
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://90d7e52d.register-app.pages.dev |
| Branch Preview URL: | https://devin-1786473311-e2e-live-ap.register-app.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (4)
e2e/helpers/tests/live.test.ts (2)
319-326: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the teardown-race test out of the basket-drift block.
This test asserts
isTeardownRacebehavior. It sits insidedescribe('basket drift between the snapshot and the live API'), which describes an unrelated concern. A reader scanning for teardown-race coverage will not find it here.Move it to its own
describe('teardown race detection')block.🤖 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 `@e2e/helpers/tests/live.test.ts` around lines 319 - 326, Move the test covering isTeardownRace out of the basket-drift describe block and place it in a separate describe('teardown race detection') block. Keep all existing assertions and test behavior unchanged.
255-289: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd negative coverage for
candleInvariants.
e2e/helpers/live-contracts.tsLines 185-202 add thehistorical dtf candlescontract and its bracket invariant (high >= low, and open/close inside the high/low range). No test in this file exercises that path. A regression in the comparison at Lines 193-196 would ship undetected.Add one accepting case and one violating case for
/v2/historical/dtf/candles.As per path instructions, "every new central mock branch must include a negative unit test".
💚 Proposed additional coverage
expect( validate('/historical/dtf', { timeseries: [{ timestamp: 1, price: 5.2 }] }, reserve) ).toEqual([]) + // Candles are the overview's DEFAULT chart type and a different shape. + const candle = { timestamp: 1, open: 5, high: 6, low: 4, close: 5.5 } + expect( + validate('/v2/historical/dtf/candles', { candles: [candle] }, reserve) + ).toEqual([]) + // A body that cannot be drawn inside its wick. + expect( + validate( + '/v2/historical/dtf/candles', + { candles: [{ ...candle, close: 9 }] }, + reserve + )[0] + ).toContain('open/close outside the high/low range') + expect( + validate( + '/v2/historical/dtf/candles', + { candles: [{ ...candle, high: 3 }] }, + reserve + )[0] + ).toContain('< low')🤖 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 `@e2e/helpers/tests/live.test.ts` around lines 255 - 289, Add coverage in the live contract tests for `/v2/historical/dtf/candles`: add one valid candle payload that `validate` accepts and one payload violating the `candleInvariants` bracket rules (`high >= low` and open/close within the high-low range) that reports a shape drift. Keep both cases scoped to the existing test setup and use the established reserve surface where appropriate.Source: Path instructions
e2e/helpers/live-contracts.ts (1)
365-370: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider contracting
/healthon the reserve surface too.The
healthcontract is registered for thezappersurface only.e2e/helpers/api.tsLine 404 shows the mock answers/healthon the reserve host as well, so a live reserve run reaches an uncontracted endpoint andvalidateLiveResponsereturns no violations for it.If reserve
/healthis in scope, setsurfaceper entry or add a second registry 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 `@e2e/helpers/live-contracts.ts` around lines 365 - 370, Extend the health contract registration around the health entry to cover the reserve surface as well as zapper, using a separate registry entry or the supported per-entry surface configuration. Preserve the existing /health path match and healthSchema validation for both surfaces.e2e/helpers/live.ts (1)
123-129: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTwo option fields are declared and populated but never read. The live transport declares option fields that no consumer consumes, so call sites do work whose result is discarded.
e2e/helpers/live.ts#L123-L129:LivePassthroughOptions.logis declared, but thefulfillFromLivebody at Lines 158-227 never readsoptions.log.e2e/helpers/api.tsLine 192 ande2e/helpers/zapper.tsLine 204 both pass it;e2e/helpers/zapper.tsLine 284 omits it. Either remove the field and the two passing call sites, or calllogon the transport-failure path at Lines 189-192 so live request failures reach the unmocked-calls report.e2e/helpers/live-contracts.ts#L383-L391:LiveValidationInput.postDatais declared, butvalidateLiveResponsedestructures onlysurface,targetName,method,url,status, andbodyat Line 404.e2e/helpers/live.tsLine 358 runsJSON.stringify(payload)on every deploy-zap request to fill it. Either remove the field and both call sites, or forward it intoLiveContract.invariantsso a contract can assert request/response correlation.🤖 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 `@e2e/helpers/live.ts` around lines 123 - 129, Remove the unused LivePassthroughOptions.log field from e2e/helpers/live.ts:123-129 and remove the corresponding arguments in e2e/helpers/api.ts:192 and e2e/helpers/zapper.ts:204. Remove the unused LiveValidationInput.postData field from e2e/helpers/live-contracts.ts:383-391 and stop constructing/passing it from e2e/helpers/live.ts:358; leave validateLiveResponse and request behavior otherwise unchanged.
🤖 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 `@docs/wiki/domains/e2e.md`:
- Around line 64-66: Update the documentation for E2E_LIVE_RESERVE_API and
E2E_LIVE_ZAPPER_API in docs/wiki/domains/e2e.md and e2e/README.md to state that
custom absolute URL targets must be origins without path prefixes. Make clear
that values such as https://host/prefix are not supported because
resolveLiveTarget stores only new URL(raw).origin, and document the expected
accepted format consistently in both locations.
In `@e2e/fixtures/base.ts`:
- Around line 254-265: Move the live-contract attachment logic from the
liveViolations throw block to before the unmocked-calls throw in the surrounding
fixture flow, so testInfo.attach('live-contract-violations', ...) executes
whenever violations are recorded even when unmocked calls also exist. Keep the
existing liveViolations error throw and attachment payload unchanged.
In `@e2e/helpers/api.ts`:
- Around line 187-194: Update the live-target branch in the API request handler
to call fulfillFromLive without passing requests, since this handler already
records the BoundaryRequest and the helper would duplicate it. Also enforce that
live cannot be configured without liveViolations: validate and throw before
falling back to snapshots, or make the options type require both values
together.
In `@e2e/helpers/zapper.ts`:
- Around line 145-157: Reduce the retry budget in fillAmountAwaitLiveQuote’s
toPass call from 150,000 ms to 90,000 ms, matching fillAmountAwaitQuote, so the
live spec retains time for its subsequent assertions within the project timeout.
- Around line 202-215: Align the live-mode guard in the zapper route setup so
the route registration and early return use the same condition, requiring
liveViolations whenever live.zapper is enabled. Move the snapshots computation
below the live-mode early return so live mode does not load unused snapshot
files; preserve snapshot routing for non-live mode.
In `@e2e/TEST_MAP.md`:
- Around line 14-18: Update the inventory statement in e2e/TEST_MAP.md to
clarify that the 73-spec, five-directory count applies only to the offline
suite, or revise the counts to include tests/live/. Keep the documented
Playwright project breakdown consistent with the chosen scope.
In `@e2e/tests/live/pricing-live.spec.ts`:
- Around line 39-101: The live pricing test in
e2e/tests/live/pricing-live.spec.ts:39-101 must add controlled-boundary L0–L3
lifecycle assertions for the overview price and chart, repeat the same coverage
under a phone viewport tagged `@mobile`, call freezeTime before navigation, and
call advanceTime after user actions. Apply equivalent lifecycle, mobile, and
clock coverage to the zap widget test in
e2e/tests/live/zap-widget-live.spec.ts:40-103, using that spec’s existing test
symbols and preserving its current live API assertions.
In `@e2e/tests/live/zap-widget-live.spec.ts`:
- Around line 29-30: Replace the hardcoded DTF_ADDRESS in the live Zap widget
test with an LCAP lookup from REGISTRY, bind the selected entry as dtf, and use
dtf.address for both seedZapSurface and navigation while preserving the existing
amount and test flow.
- Around line 79-81: Update the live quote assertion in fillAmountAwaitLiveQuote
to avoid Number conversion; parse quoted using Amount or convert it to bigint
with the known token decimals, then assert the exact token amount is greater
than zero while preserving the existing executable-quote check.
- Around line 52-61: Update the quote-capture flow around the page response
listener so assertions wait until response body parsing and quotes.push
complete, using an awaited response/route capture or polling quotes.length.
Parse the JSON result as unknown and validate its shape before reading
result.tx, preserving capture only for matching Zapper swap responses.
In `@e2e/tests/live/zapper-api-contract.spec.ts`:
- Around line 106-112: Update the test named “fetchZapperTokens can read the
live token list” to invoke fetchZapperTokens against the configured live zapper
target and assert that it returns a non-empty Set. Remove the unconditional
test.fail(), while keeping the existing liveProbe raw-response envelope
assertion as a separate check.
---
Nitpick comments:
In `@e2e/helpers/live-contracts.ts`:
- Around line 365-370: Extend the health contract registration around the health
entry to cover the reserve surface as well as zapper, using a separate registry
entry or the supported per-entry surface configuration. Preserve the existing
/health path match and healthSchema validation for both surfaces.
In `@e2e/helpers/live.ts`:
- Around line 123-129: Remove the unused LivePassthroughOptions.log field from
e2e/helpers/live.ts:123-129 and remove the corresponding arguments in
e2e/helpers/api.ts:192 and e2e/helpers/zapper.ts:204. Remove the unused
LiveValidationInput.postData field from e2e/helpers/live-contracts.ts:383-391
and stop constructing/passing it from e2e/helpers/live.ts:358; leave
validateLiveResponse and request behavior otherwise unchanged.
In `@e2e/helpers/tests/live.test.ts`:
- Around line 319-326: Move the test covering isTeardownRace out of the
basket-drift describe block and place it in a separate describe('teardown race
detection') block. Keep all existing assertions and test behavior unchanged.
- Around line 255-289: Add coverage in the live contract tests for
`/v2/historical/dtf/candles`: add one valid candle payload that `validate`
accepts and one payload violating the `candleInvariants` bracket rules (`high >=
low` and open/close within the high-low range) that reports a shape drift. Keep
both cases scoped to the existing test setup and use the established reserve
surface where appropriate.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bf46166f-ea86-4c64-8209-38825bf76759
📒 Files selected for processing (17)
docs/wiki/domains/e2e.mddocs/wiki/progress.mde2e/CLAUDE.mde2e/README.mde2e/TEST_MAP.mde2e/fixtures/base.tse2e/helpers/api.tse2e/helpers/live-contracts.tse2e/helpers/live.tse2e/helpers/tests/live.test.tse2e/helpers/zapper.tse2e/tests/live/pricing-live.spec.tse2e/tests/live/reserve-api-contract.spec.tse2e/tests/live/zap-widget-live.spec.tse2e/tests/live/zapper-api-contract.spec.tspackage.jsonplaywright.config.ts
| `E2E_LIVE_RESERVE_API` and `E2E_LIVE_ZAPPER_API`, resolved independently by | ||
| `helpers/live.ts` (`production` / `staging` / `zrs1` / an absolute URL; an | ||
| unknown value throws rather than silently falling back offline). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Specify that custom targets are origins.
resolveLiveTarget in e2e/helpers/live.ts stores new URL(raw).origin. Therefore, https://host/prefix silently loses /prefix. Either preserve the path in the helper or document and reject path-prefixed values. Apply the same rule in e2e/README.md Lines 78-84.
🤖 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 `@docs/wiki/domains/e2e.md` around lines 64 - 66, Update the documentation for
E2E_LIVE_RESERVE_API and E2E_LIVE_ZAPPER_API in docs/wiki/domains/e2e.md and
e2e/README.md to state that custom absolute URL targets must be origins without
path prefixes. Make clear that values such as https://host/prefix are not
supported because resolveLiveTarget stores only new URL(raw).origin, and
document the expected accepted format consistently in both locations.
|
|
||
| // Live contract drift is a failure of the deployment under validation — | ||
| // never a soft warning, and never silenced by allowUnmocked. | ||
| if (liveViolations.length) { | ||
| await testInfo.attach('live-contract-violations', { | ||
| body: liveViolations.join('\n'), | ||
| contentType: 'text/plain', | ||
| }) | ||
| throw new Error( | ||
| `live API returned ${liveViolations.length} contract violation(s):\n${liveViolations.join('\n')}` | ||
| ) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Attach live violations before the unmocked-calls throw.
The unmocked-calls check at Lines 249-253 throws before this block runs. If a test records both an unmocked call and a live contract violation, testInfo.attach('live-contract-violations', ...) never executes and the violation report is missing from the trace.
Move the attachment above the first throw so both artifacts always reach the report.
🐛 Proposed fix for the attachment ordering
if (calls.length) {
await testInfo.attach('unmocked-calls', {
body: calls.join('\n'),
contentType: 'text/plain',
})
}
+ if (liveViolations.length) {
+ await testInfo.attach('live-contract-violations', {
+ body: liveViolations.join('\n'),
+ contentType: 'text/plain',
+ })
+ }
// Every committed test is strict by default. Exploratory work must opt out
// explicitly with test.use({ allowUnmocked: true }).
if (!allowUnmocked && calls.length) {
throw new Error(
`test hit ${calls.length} unmocked call(s):\n${calls.join('\n')}`
)
}
// Live contract drift is a failure of the deployment under validation —
// never a soft warning, and never silenced by allowUnmocked.
if (liveViolations.length) {
- await testInfo.attach('live-contract-violations', {
- body: liveViolations.join('\n'),
- contentType: 'text/plain',
- })
throw new Error(
`live API returned ${liveViolations.length} contract violation(s):\n${liveViolations.join('\n')}`
)
}🤖 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 `@e2e/fixtures/base.ts` around lines 254 - 265, Move the live-contract
attachment logic from the liveViolations throw block to before the
unmocked-calls throw in the surrounding fixture flow, so
testInfo.attach('live-contract-violations', ...) executes whenever violations
are recorded even when unmocked calls also exist. Keep the existing
liveViolations error throw and attachment payload unchanged.
| // Live passthrough wins over BOTH snapshots and per-test overrides: a live | ||
| // run must report what the deployment actually returns, never a local | ||
| // substitute. Only the surfaces explicitly pointed at a target are live. | ||
| const liveTarget = live?.[surfaceForPath(path)] | ||
| if (liveTarget && liveViolations) { | ||
| return fulfillFromLive(route, liveTarget, { violations: liveViolations, log, requests }) | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Live-routed requests are recorded twice on boundaryRequests.
Line 179 already pushes a BoundaryRequest for every API request that reaches this handler. fulfillFromLive then pushes a second, byte-identical entry (e2e/helpers/live.ts Lines 215-220): same boundary: 'api', same method, same pathname, same search.
Every live-routed API request therefore appears twice in boundaryRequests. e2e/fixtures/base.ts Lines 27-28 document that log as the oracle for request counts, so any live spec asserting a count or an exact request list gets doubled entries.
Do not pass requests from this call site. e2e/helpers/zapper.ts registers its planner routes on a separate page.route that never reaches this handler, so its fulfillFromLive calls correctly rely on the helper's own push.
Second point on the same guard: if a caller supplies live but omits liveViolations, the request falls through to snapshots silently. That is the "reported as live, actually offline" failure e2e/helpers/live.ts Lines 63-65 says must never occur. Consider making the two options a single object so the invalid state is unrepresentable, or throw when live is set without liveViolations.
🐛 Proposed fix for the duplicate boundary record
// Live passthrough wins over BOTH snapshots and per-test overrides: a live
// run must report what the deployment actually returns, never a local
// substitute. Only the surfaces explicitly pointed at a target are live.
+ // The request is already recorded above, so `requests` is NOT forwarded —
+ // fulfillFromLive would append a duplicate entry.
const liveTarget = live?.[surfaceForPath(path)]
if (liveTarget && liveViolations) {
- return fulfillFromLive(route, liveTarget, { violations: liveViolations, log, requests })
+ return fulfillFromLive(route, liveTarget, { violations: liveViolations, log })
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Live passthrough wins over BOTH snapshots and per-test overrides: a live | |
| // run must report what the deployment actually returns, never a local | |
| // substitute. Only the surfaces explicitly pointed at a target are live. | |
| const liveTarget = live?.[surfaceForPath(path)] | |
| if (liveTarget && liveViolations) { | |
| return fulfillFromLive(route, liveTarget, { violations: liveViolations, log, requests }) | |
| } | |
| // Live passthrough wins over BOTH snapshots and per-test overrides: a live | |
| // run must report what the deployment actually returns, never a local | |
| // substitute. Only the surfaces explicitly pointed at a target are live. | |
| // The request is already recorded above, so `requests` is NOT forwarded — | |
| // fulfillFromLive would append a duplicate entry. | |
| const liveTarget = live?.[surfaceForPath(path)] | |
| if (liveTarget && liveViolations) { | |
| return fulfillFromLive(route, liveTarget, { violations: liveViolations, log }) | |
| } |
🤖 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 `@e2e/helpers/api.ts` around lines 187 - 194, Update the live-target branch in
the API request handler to call fulfillFromLive without passing requests, since
this handler already records the BoundaryRequest and the helper would duplicate
it. Also enforce that live cannot be configured without liveViolations: validate
and throw before falling back to snapshots, or make the options type require
both values together.
| export async function fillAmountAwaitLiveQuote( | ||
| panel: Locator, | ||
| amount: string | ||
| ): Promise<string> { | ||
| const input = panel.locator('input[inputmode="decimal"]:not([disabled])') | ||
| const output = panel.locator('input[inputmode="decimal"][disabled]') | ||
| await expect(async () => { | ||
| await input.fill(amount) | ||
| // Anything but empty/zero — a live quote's magnitude is not predictable. | ||
| await expect(output).toHaveValue(/^(?!0(\.0*)?$)\d*\.?\d+$/, { timeout: 30_000 }) | ||
| }).toPass({ timeout: 150_000 }) | ||
| return (await output.inputValue()) ?? '' | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
The 150s retry budget nearly consumes the whole live test timeout.
playwright.config.ts Line 68 sets the live project timeout to 180_000 ms. This helper alone can spend 150_000 ms in toPass. e2e/tests/live/zap-widget-live.spec.ts calls it after page.goto and connectWallet, then still has to wait for the submit button, click, and assert the tx link with two further 30_000 ms expectations.
On a slow planner the spec fails on the project timeout rather than on this helper's own assertion, which hides the actual cause.
Lower the toPass budget (90_000 ms matches fillAmountAwaitQuote at Line 137) or raise the live project timeout.
🤖 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 `@e2e/helpers/zapper.ts` around lines 145 - 157, Reduce the retry budget in
fillAmountAwaitLiveQuote’s toPass call from 150,000 ms to 90,000 ms, matching
fillAmountAwaitQuote, so the live spec retains time for its subsequent
assertions within the project timeout.
| if (live?.zapper && liveViolations) { | ||
| await page.route('**/api.reserve.org/api/zapper/**', (route) => | ||
| fulfillFromLive(route, live.zapper!, { violations: liveViolations, log }) | ||
| ) | ||
| } | ||
|
|
||
| const snapshots = ZAP_FIXTURES.filter((fixture) => { | ||
| const dtf = findDtfByAddress(dtfAddress) | ||
| return dtf && snapshotExists(`${dtf.snapshotDir}/zap-${fixture}.json`) | ||
| }).map((fixture) => loadZapSnapshot(dtfAddress, fixture)) | ||
|
|
||
| // Live mode owns the planner endpoints outright — never register the snapshot | ||
| // route alongside it, so no request can fall back to a pinned quote. | ||
| if (live?.zapper) return installAggregatorRoutes(page, live, liveViolations) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
The two live guards disagree, and one path installs no route at all.
Line 202 registers the live planner route only when live?.zapper && liveViolations are both present. Line 215 returns early on live?.zapper alone.
If a caller passes live.zapper without liveViolations, neither branch installs a handler for **/api.reserve.org/api/zapper/**. Every planner request then reaches the default-deny catch-all in e2e/fixtures/base.ts Line 109 and receives a 502 logged as unmocked egress. The failure is a 502 storm, not a clear configuration error.
Use one guard for both decisions, or require liveViolations whenever live is set.
Separately, Lines 208-211 load and parse every zap snapshot before the Line 215 early return. In live mode that file I/O is discarded. Move the snapshots computation below the early return.
🐛 Proposed fix for the guard mismatch and the discarded snapshot load
const { live, liveViolations } = options
- if (live?.zapper && liveViolations) {
+ // Live mode owns the planner endpoints outright — never register the snapshot
+ // route alongside it, so no request can fall back to a pinned quote.
+ if (live?.zapper) {
+ if (!liveViolations) {
+ throw new Error(
+ 'mockZapperRoutes: live.zapper is configured without liveViolations — ' +
+ 'contract drift would go unrecorded'
+ )
+ }
await page.route('**/api.reserve.org/api/zapper/**', (route) =>
- fulfillFromLive(route, live.zapper!, { violations: liveViolations, log })
+ fulfillFromLive(route, live.zapper!, { violations: liveViolations, log })
)
+ return installAggregatorRoutes(page, live, liveViolations)
}
const snapshots = ZAP_FIXTURES.filter((fixture) => {
const dtf = findDtfByAddress(dtfAddress)
return dtf && snapshotExists(`${dtf.snapshotDir}/zap-${fixture}.json`)
}).map((fixture) => loadZapSnapshot(dtfAddress, fixture))
- // Live mode owns the planner endpoints outright — never register the snapshot
- // route alongside it, so no request can fall back to a pinned quote.
- if (live?.zapper) return installAggregatorRoutes(page, live, liveViolations)
-
// Native zap quotes: pinned-input snapshot or fail-loud 500.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (live?.zapper && liveViolations) { | |
| await page.route('**/api.reserve.org/api/zapper/**', (route) => | |
| fulfillFromLive(route, live.zapper!, { violations: liveViolations, log }) | |
| ) | |
| } | |
| const snapshots = ZAP_FIXTURES.filter((fixture) => { | |
| const dtf = findDtfByAddress(dtfAddress) | |
| return dtf && snapshotExists(`${dtf.snapshotDir}/zap-${fixture}.json`) | |
| }).map((fixture) => loadZapSnapshot(dtfAddress, fixture)) | |
| // Live mode owns the planner endpoints outright — never register the snapshot | |
| // route alongside it, so no request can fall back to a pinned quote. | |
| if (live?.zapper) return installAggregatorRoutes(page, live, liveViolations) | |
| if (live?.zapper) { | |
| if (!liveViolations) { | |
| throw new Error( | |
| 'mockZapperRoutes: live.zapper is configured without liveViolations — ' + | |
| 'contract drift would go unrecorded' | |
| ) | |
| } | |
| await page.route('**/api.reserve.org/api/zapper/**', (route) => | |
| fulfillFromLive(route, live.zapper!, { violations: liveViolations, log }) | |
| ) | |
| return installAggregatorRoutes(page, live, liveViolations) | |
| } | |
| const snapshots = ZAP_FIXTURES.filter((fixture) => { | |
| const dtf = findDtfByAddress(dtfAddress) | |
| return dtf && snapshotExists(`${dtf.snapshotDir}/zap-${fixture}.json`) | |
| }).map((fixture) => loadZapSnapshot(dtfAddress, fixture)) |
🤖 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 `@e2e/helpers/zapper.ts` around lines 202 - 215, Align the live-mode guard in
the zapper route setup so the route registration and early return use the same
condition, requiring liveViolations whenever live.zapper is enabled. Move the
snapshots computation below the live-mode early return so live mode does not
load unused snapshot files; preserve snapshot routing for non-live mode.
| test(`overview renders a live price for ${dtf.slug} (${dtf.chain})`, async ({ | ||
| page, | ||
| request, | ||
| unmockedCalls, | ||
| liveViolations, | ||
| boundaryRequests, | ||
| }) => { | ||
| // Precondition, not a product assertion: the SDK joins the API basket with | ||
| // the (mocked, pinned) chain state by address, so a basket that moved since | ||
| // the last capture can never render. Say so explicitly. | ||
| const { data } = await liveGet( | ||
| request, | ||
| config.reserve!, | ||
| `/current/dtf?address=${dtf.address.toLowerCase()}&chainId=${dtf.chainId}`, | ||
| liveViolations | ||
| ) | ||
| const drift = describeBasketDrift( | ||
| basketDrift(loadSnapshot(`${dtf.snapshotDir}/current-price.json`), data) | ||
| ) | ||
| test.skip( | ||
| drift.length > 0, | ||
| `${dtf.slug}: live basket ${drift.join(' ')} vs the pinned chain-state ` + | ||
| `snapshot — the SDK cannot join them, run \`pnpm e2e:capture\` to ` + | ||
| `validate this DTF's pricing UI (docs/wiki/progress.md § E2E coverage debt)` | ||
| ) | ||
|
|
||
| await page.goto(dtfPath(dtf, 'overview')) | ||
|
|
||
| // Header price: skeleton -> value. `$0` would mean the response landed but | ||
| // carried no usable price, so the money format is asserted, not just text. | ||
| const price = page.getByTestId('overview-dtf-price') | ||
| await expect(price).toHaveText(/^\$[\d,]+(\.\d+)?$/, { timeout: 60_000 }) | ||
| await expect(price).not.toHaveText(/^\$0(\.0+)?$/) | ||
|
|
||
| // Chart: plotted geometry, not just the container — proves the live | ||
| // series was consumed and is plottable. The default chart type is | ||
| // candlestick (recharts bars); the line type draws an area curve. | ||
| const chart = page.getByTestId('overview-price-chart') | ||
| await expect(chart.locator('svg').first()).toBeVisible({ timeout: 60_000 }) | ||
| const plotted = chart.locator( | ||
| 'svg .recharts-bar-rectangle, svg .recharts-area-curve' | ||
| ) | ||
| await expect | ||
| .poll(() => plotted.count(), { timeout: 60_000, message: 'chart plotted nothing' }) | ||
| .toBeGreaterThan(0) | ||
|
|
||
| // The price came from the live API, with the identity register is | ||
| // expected to send. | ||
| const priceRequests = boundaryRequests.filter( | ||
| (entry) => entry.boundary === 'api' && entry.pathname.includes('/current/dtf') | ||
| ) | ||
| expect(priceRequests.length, 'no /current/dtf request recorded').toBeGreaterThan(0) | ||
| expect( | ||
| priceRequests.some( | ||
| (entry) => | ||
| entry.boundary === 'api' && | ||
| entry.search.address?.toLowerCase() === dtf.address.toLowerCase() | ||
| ), | ||
| '/current/dtf must be requested for this DTF address' | ||
| ).toBe(true) | ||
|
|
||
| expect(unmockedCalls).toEqual([]) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Add required lifecycle, mobile, and clock coverage.
Both specs assert desktop L3 behavior only. Add L0-L3 assertions with controlled boundary responses. Add @mobile assertions for the same lifecycle. Call freezeTime before navigation and advanceTime after user actions.
e2e/tests/live/pricing-live.spec.ts#L39-L101: Cover the overview price and chart through L0-L3 and mobile states.e2e/tests/live/zap-widget-live.spec.ts#L40-L103: Cover the zap widget through L0-L3 and mobile states.
As per coding guidelines, “Every page spec must include the same L0–L3 coverage at a phone viewport” and “Call freezeTime(page, seconds) before navigation.”
📍 Affects 2 files
e2e/tests/live/pricing-live.spec.ts#L39-L101(this comment)e2e/tests/live/zap-widget-live.spec.ts#L40-L103
🤖 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 `@e2e/tests/live/pricing-live.spec.ts` around lines 39 - 101, The live pricing
test in e2e/tests/live/pricing-live.spec.ts:39-101 must add controlled-boundary
L0–L3 lifecycle assertions for the overview price and chart, repeat the same
coverage under a phone viewport tagged `@mobile`, call freezeTime before
navigation, and call advanceTime after user actions. Apply equivalent lifecycle,
mobile, and clock coverage to the zap widget test in
e2e/tests/live/zap-widget-live.spec.ts:40-103, using that spec’s existing test
symbols and preserving its current live API assertions.
Source: Coding guidelines
| const DTF_ADDRESS = '0x4dA9A0f397dB1397902070f93a4D6ddBC0E0E6e8' // base/lcap | ||
| const AMOUNT = '0.05' // ETH in; same trade the offline suite pins |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Resolve the DTF from REGISTRY.
Line 29 hardcodes a DTF address. A registry address update can make this test target a stale asset. Select LCAP from REGISTRY, then use dtf.address for seedZapSurface and navigation.
As per coding guidelines, “Render DTF pages using a DTF from helpers/registry.ts” and “never hardcode addresses or chains outside the registry.”
🤖 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 `@e2e/tests/live/zap-widget-live.spec.ts` around lines 29 - 30, Replace the
hardcoded DTF_ADDRESS in the live Zap widget test with an LCAP lookup from
REGISTRY, bind the selected entry as dtf, and use dtf.address for both
seedZapSurface and navigation while preserving the existing amount and test
flow.
Source: Coding guidelines
| page.on('response', async (response) => { | ||
| const url = response.url() | ||
| if (!url.includes('/api/zapper/') || !url.includes('/swap')) return | ||
| const body = await response.text().catch(() => '') | ||
| if (!body) return | ||
| const parsed = JSON.parse(body) as { | ||
| result?: { tx?: { to: string; data: string; value: string } | null } | ||
| } | ||
| if (parsed.result?.tx) quotes.push(parsed.result.tx) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline e2e/helpers/zapper.ts --items all
rg -n -C 4 'mockZapperRoutes|fillAmountAwaitLiveQuote|safeParse|\.parse\(' e2e/helpers/zapper.ts
rg -n -C 4 'page\.on\(.response.|JSON\.parse|quotes' e2e/tests/live/zap-widget-live.spec.tsRepository: reserve-protocol/register
Length of output: 5359
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- live spec ---'
cat -n e2e/tests/live/zap-widget-live.spec.ts | sed -n '1,150p'
printf '%s\n' '--- zapper helper ---'
cat -n e2e/helpers/zapper.ts | sed -n '120,280p'
printf '%s\n' '--- registry references ---'
rg -n -C 3 'DTF_ADDRESS|findDtfByAddress|dtfPath|registry' e2e/tests/live/zap-widget-live.spec.ts e2e/helpers/registry.ts e2e/helpers -g '*.ts'
printf '%s\n' '--- response schemas and quote contracts ---'
rg -n -C 4 'ZapQuoteResponse|tx:.*to|zapper|swap' e2e/helpers e2e/tests -g '*.{ts,tsx}' | head -n 300Repository: reserve-protocol/register
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- live response fulfillment ---'
rg -n -C 12 'function fulfillFromLive|export.*fulfillFromLive|validateLiveResponse' e2e/helpers/live.ts e2e/helpers/live-contracts.ts
printf '%s\n' '--- swap contract ---'
rg -n -C 10 'swap|ZapQuote|tx' e2e/helpers/live-contracts.ts | head -n 220
printf '%s\n' '--- awaited response capture patterns ---'
rg -n -C 5 'page\.waitForResponse|expect\.poll|response\.text\(\)|page\.on\(' e2e -g '*.{ts,tsx}' | head -n 260
printf '%s\n' '--- standalone async event timing probe ---'
node - <<'JS'
const listeners = []
const on = (fn) => listeners.push(fn)
const emit = (value) => {
for (const fn of listeners) fn(value)
}
const quotes = []
on(async (value) => {
await Promise.resolve()
quotes.push(value)
})
emit('quote')
console.log(JSON.stringify({
immediatelyAfterEmit: quotes.length,
afterMicrotask: awaitMicrotask()
}))
function awaitMicrotask() {
return undefined
}
JSRepository: reserve-protocol/register
Length of output: 32975
Await the quote-capture task before asserting quotes.
page.on('response', async ...) does not await its callback. The output assertion can run before response.text() and quotes.push(...), causing a false failure. Use an awaited response or route capture, or poll quotes.length. Parse the body as unknown; do not rely on JSON.parse(body) as ... for validation.
🤖 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 `@e2e/tests/live/zap-widget-live.spec.ts` around lines 52 - 61, Update the
quote-capture flow around the page response listener so assertions wait until
response body parsing and quotes.push complete, using an awaited response/route
capture or polling quotes.length. Parse the JSON result as unknown and validate
its shape before reading result.tx, preserving capture only for matching Zapper
swap responses.
Source: Coding guidelines
| const quoted = await fillAmountAwaitLiveQuote(panel, AMOUNT) | ||
| expect(Number(quoted), 'live quote output').toBeGreaterThan(0) | ||
| expect(quotes.length, 'live planner returned an executable quote').toBeGreaterThan(0) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not convert the quote amount with Number.
Line 80 loses exact token-amount semantics. Parse the quote with Amount, or scale it to bigint with known token decimals before the positive-value assertion.
As per coding guidelines, “Represent money using Amount or bigint, never Number, for on-chain math.”
🤖 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 `@e2e/tests/live/zap-widget-live.spec.ts` around lines 79 - 81, Update the live
quote assertion in fillAmountAwaitLiveQuote to avoid Number conversion; parse
quoted using Amount or convert it to bigint with the known token decimals, then
assert the exact token amount is greater than zero while preserving the existing
executable-quote check.
Source: Coding guidelines
| test('fetchZapperTokens can read the live token list', async () => { | ||
| test.fail() | ||
| const { head } = await liveProbe(zapper, `/api/zapper/${CHAINS.base.chainId}/tokens`) | ||
| expect(head, 'response has no `tokens` key for fetchZapperTokens to read').toContain( | ||
| '"tokens":[' | ||
| ) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the client seam and its consumers.
fd -a -t f 'zapper.ts' . | while IFS= read -r file; do
rg -n -C 8 '\bfetchZapperTokens\b|data\.tokens|\.result' "$file"
done
# Locate existing tests or fixture routing that can exercise the client seam.
rg -n -C 6 --glob '*.{test,spec}.{ts,tsx}' '\bfetchZapperTokens\s*\(' .Repository: reserve-protocol/register
Length of output: 1622
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Identify the client implementation and inspect the live test helpers and cases.
fd -a -t f . | rg '(^|/)(zapper(\.ts|.*)|zapper-api-contract\.spec\.ts)$'
rg -n -C 12 'fetchZapperTokens|liveProbe|test\.fail|tokens|result' e2e/tests/live/zapper-api-contract.spec.ts .Repository: reserve-protocol/register
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
spec=$(rg --files | rg '(^|/)e2e/tests/live/zapper-api-contract\.spec\.ts$' | head -n 1)
client=$(rg --files | rg '(^|/)(zapper|zapper-api)\.ts$' | head -n 1)
printf '%s\n' "SPEC=$spec" "CLIENT=$client"
printf '\n--- spec ---\n'
sed -n '1,135p' "$spec"
printf '\n--- client ---\n'
sed -n '1,90p' "$client"Repository: reserve-protocol/register
Length of output: 9985
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- fetchZapperTokens definitions and uses ---'
rg -l -g '*.ts' -g '*.tsx' '\bfetchZapperTokens\b' . | while IFS= read -r file; do
printf '\nFILE=%s\n' "$file"
rg -n -C 10 '\bfetchZapperTokens\b' "$file"
done
printf '%s\n' '--- live helper configuration and probe implementation ---'
live=$(rg --files | rg '(^|/)e2e/helpers/live\.ts$' | head -n 1)
printf 'LIVE=%s\n' "$live"
rg -n -C 12 'export function liveProbe|export const liveConfig|zapper|E2E_LIVE_ZAPPER_API' "$live"Repository: reserve-protocol/register
Length of output: 12553
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- liveProbe implementation ---'
live=e2e/helpers/live.ts
rg -n -A 80 -B 10 'export async function liveProbe|function liveProbe|liveProbe\s*=' "$live"
printf '%s\n' '--- ZAPPER_API definition and test/runtime configuration ---'
rg -n -C 8 'ZAPPER_API|RESERVE_API' src/utils/constants.ts src e2e | head -n 240Repository: reserve-protocol/register
Length of output: 21405
Exercise fetchZapperTokens against the configured live target.
liveProbe reads only the target response head. It never calls src/utils/zapper.ts.
A client-only fix from data.tokens to data.result cannot make this test.fail() pass. Invoke fetchZapperTokens through the same live target and assert a non-empty Set. Keep the raw envelope probe separate.
🤖 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 `@e2e/tests/live/zapper-api-contract.spec.ts` around lines 106 - 112, Update
the test named “fetchZapperTokens can read the live token list” to invoke
fetchZapperTokens against the configured live zapper target and assert that it
returns a non-empty Set. Remove the unconditional test.fail(), while keeping the
existing liveProbe raw-response envelope assertion as a separate check.
Source: Coding guidelines
1 participant
Summary
The e2e suite had no way to hit the real APIs:
fixtures/base.tsdefault-denies egress and always serves the Reserve/zapper boundaries from snapshots (allowUnmockedonly silences the teardown failure — the mocks still answer). This adds a separate, opt-inliveproject so the same specs can be used as a validation suite for Register's usage of the Reserve and zapper/planner APIs, againstzrs1in particular. The offline suites are untouched and stay deterministic.Two independent surfaces, one classifier:
Live mode is a boundary swap, not an escape hatch —
helpers/api.tsconsults the live target before snapshots/overrides, and default-deny egress,boundaryRequestsrecording and strict teardown all still apply:Live responses are additionally checked by
helpers/live-contracts.ts(per-endpoint zod shape + invariants — quoteminAmountOut <= amountOut, non-zeroamountOut, atxwhenever funds suffice, candlehigh >= lowwith open/close in range). Drift or a broken invariant fails the test.Coverage in
e2e/tests/live/: request-level contracts for the Reserve API (prices, current/historical DTF + v2 candles, compliance, discover, portfolio, exposure, rebalance,POST /rebalance/liquidity) and the planner (health, bounded token-list probe,/api/prices/{chain}, buy quotes, ungoverned deploy quote), plus UI specs driving live pricing and the zap widget (live quote in, submitted tx compared to the quote'stx).Non-obvious bits found while running it against zrs1/production, all documented in
docs/wiki/progress.md§ E2E coverage debt ande2e/README.md§ Live API mode:/api/zapper/{chain}/tokensis ~500 MB →liveProbevalidates a bounded prefix, never buffers.fetchZapperTokenscannot read any deployment's token list: it readsdata.tokens[], deployments answer{status, result[]}, and itscatchturns that into an empty Set — "nothing is zappable", silently. Pinned withtest.fail()rather than hidden; needs a client-or-API decision./api/prices/56→result: []; the route doesn't exist on api.reserve.org at all). Register reads/current/prices, which covers them, so the spec asserts exactly that cross-surface guarantee: a planner gap must be covered by the Reserve API, else the basket renders $0.basketDriftdetects it and the spec skips with apnpm e2e:captureinstruction instead of a fake pass/fail (CMC20 today: live basket added one token).Verification:
pnpm typecheck·pnpm lint· 95 helper units (23 new,e2e/helpers/tests/live.test.ts) ·pnpm e2e:smoke58 ·pnpm e2e:check· live run against reserve=production + zapper=zrs1 → 39 passed / 1 skipped (CMC20 basket drift).Engineer review required: deploy coverage is quote-level only — the planner's deploy tx is contract-validated, never submitted on-chain — and the token-list parser drift above needs a decision on which side moves.
Link to Devin session: https://app.devin.ai/sessions/9d5a13fae07b4191b08efd0fe2040631
Requested by: @TheFrozenFire
Summary by CodeRabbit
New Features
pnpm e2e:livecommand for running live tests.Documentation