test: add playwright script that proxies Amplitude scripts to test customer pages#1682
Open
daniel-graham-amplitude wants to merge 11 commits into
Open
test: add playwright script that proxies Amplitude scripts to test customer pages#1682daniel-graham-amplitude wants to merge 11 commits into
daniel-graham-amplitude wants to merge 11 commits into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Playwright added as production dependency instead of dev
- Moved
playwrightfromdependenciestodevDependenciesso it is not installed in production contexts.
- Moved
- ✅ Fixed: Playwright version mismatch with @playwright/test dependency
- Pinned
playwrightto1.55.0to match the existing@playwright/testversion and updated lockfile entries accordingly.
- Pinned
- ✅ Fixed: ESM import syntax in .js without "type": "module"
- Converted
e2e/manual-test.jsto CommonJS (require) and corrected usage text/comments so it runs with plain Node.js.
- Converted
Or push these changes by commenting:
@cursor push 7665094b95
Preview (7665094b95)
diff --git a/e2e/manual-test.js b/e2e/manual-test.js
--- a/e2e/manual-test.js
+++ b/e2e/manual-test.js
@@ -1,12 +1,12 @@
-// playwright-proxy.ts
-import { chromium } from 'playwright';
+// playwright-proxy.js
+const { chromium } = require('playwright');
// Get URL from command line args
const targetUrl = process.argv[2];
if (!targetUrl) {
- console.error('Usage: npx ts-node playwright-proxy.ts <url>');
- console.error('Example: npx ts-node playwright-proxy.ts https://example.com');
+ console.error('Usage: node e2e/manual-test.js <url>');
+ console.error('Example: node e2e/manual-test.js https://example.com');
process.exit(1);
}
diff --git a/package.json b/package.json
--- a/package.json
+++ b/package.json
@@ -78,6 +78,7 @@
"morgan": "^1.10.0",
"nodemon": "^3.0.1",
"nx": "^21.2.1",
+ "playwright": "1.55.0",
"prettier": "^2.8.1",
"rimraf": "^3.0.2",
"source-map": "^0.7.4",
@@ -95,7 +96,6 @@
]
},
"dependencies": {
- "playwright": "^1.59.1",
"tslib": "^2.4.1"
},
"packageManager": "pnpm@10.26.1+sha512.664074abc367d2c9324fdc18037097ce0a8f126034160f709928e9e9f95d98714347044e5c3164d65bd5da6c59c6be362b107546292a8eecb7999196e5ce58fa"
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
--- a/pnpm-lock.yaml
+++ b/pnpm-lock.yaml
@@ -8,9 +8,6 @@
.:
dependencies:
- playwright:
- specifier: ^1.59.1
- version: 1.59.1
tslib:
specifier: ^2.4.1
version: 2.8.1
@@ -111,6 +108,9 @@
nx:
specifier: ^21.2.1
version: 21.6.8
+ playwright:
+ specifier: 1.55.0
+ version: 1.55.0
prettier:
specifier: ^2.8.1
version: 2.8.8You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Usage error message references wrong filename and command
- Updated
e2e/manual-test.jsto reference the correct script filename and invocation command in both the header comment and missing-argument usage/example messages.
- Updated
Or push these changes by commenting:
@cursor push d8388b0d1d
Preview (d8388b0d1d)
diff --git a/e2e/manual-test.js b/e2e/manual-test.js
--- a/e2e/manual-test.js
+++ b/e2e/manual-test.js
@@ -1,12 +1,12 @@
-// playwright-proxy.ts
+// manual-test.js
import { chromium } from 'playwright';
// Get URL from command line args
const targetUrl = process.argv[2];
if (!targetUrl) {
- console.error('Usage: npx ts-node playwright-proxy.ts <url>');
- console.error('Example: npx ts-node playwright-proxy.ts https://example.com');
+ console.error('Usage: node ./e2e/manual-test.js <website-url>');
+ console.error('Example: node ./e2e/manual-test.js https://example.com');
process.exit(1);
}You can send follow-ups to the cloud agent here.
daniel-graham-amplitude
changed the title
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
There are 4 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed:
replaceonly strips first occurrence of each hash- Replaced hash stripping calls with replaceAll so every occurrence of each integrity hash is removed in GTM and document rewrite paths.
- ✅ Fixed: Redundant duplicate hash stripping for HTML documents
- Removed the second HTML-specific integrity stripping loop and fulfilled HTML responses directly from the already-stripped content.
- ✅ Fixed: Non-HTML fulfill path missing encoding header removal
- Updated the non-HTML fulfill branch to set status and dropped encoding/length headers before returning the decompressed text body.
Or push these changes by commenting:
@cursor push 8a47623b97
Preview (8a47623b97)
diff --git a/e2e/manual-test.js b/e2e/manual-test.js
--- a/e2e/manual-test.js
+++ b/e2e/manual-test.js
@@ -71,7 +71,7 @@
const response = await route.fetch();
let body = await response.text();
for (const hash of INTEGRITY_HASHES) {
- body = body.replace(hash, '');
+ body = body.replaceAll(hash, '');
}
await route.fulfill({
status: response.status(),
@@ -102,23 +102,22 @@
}
let content = await response.text();
for (const hash of INTEGRITY_HASHES) {
- content = content.replace(hash, '');
+ content = content.replaceAll(hash, '');
}
const ct = (response.headers()['content-type'] || '').toLowerCase();
if (!ct.includes('text/html')) {
- return route.fulfill({ response, body: content });
+ return route.fulfill({
+ status: response.status(),
+ headers: dropEncodingHeaders(response.headers()),
+ body: content,
+ });
}
- let html = content;
- for (const hash of INTEGRITY_HASHES) {
- html = html.replace(hash, '');
- }
-
await route.fulfill({
status: response.status(),
headers: dropEncodingHeaders(response.headers()),
- body: html,
+ body: content,
});
});You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 5 total unresolved issues (including 4 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Debug console.log accidentally committed in production SDK
- Removed the accidental
console.log('AI WEEK CODING CHANGES')fromAmplitudeBrowser.initso SDK initialization no longer emits debug output.
- Removed the accidental
Or push these changes by commenting:
@cursor push 337b19c541
Preview (337b19c541)
diff --git a/packages/analytics-browser/src/browser-client.ts b/packages/analytics-browser/src/browser-client.ts
--- a/packages/analytics-browser/src/browser-client.ts
+++ b/packages/analytics-browser/src/browser-client.ts
@@ -97,7 +97,6 @@
init(apiKey = '', userIdOrOptions?: string | BrowserOptions, maybeOptions?: BrowserOptions) {
let userId: string | undefined;
let options: BrowserOptions | undefined;
- console.log('AI WEEK CODING CHANGES');
if (arguments.length > 2) {
userId = userIdOrOptions as string | undefined;
options = maybeOptions;You can send follow-ups to the cloud agent here.
daniel-graham-amplitude
force-pushed
the
ai-week-playwright-mcp
branch
from
12033d2 to
e825749
Compare
daniel-graham-amplitude
force-pushed
the
ai-week-playwright-mcp
branch
from
e825749 to
f7f5ca8
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Production file rewritten with "dummy patch" comment
- Removed the accidental analytics-browser production source files, including the dummy-patch default instance and unused precision client file.
Or push these changes by commenting:
@cursor push f853e39994
Preview (f853e39994)
diff --git a/packages/analytics-browser/src/browser-client-precision.ts b/packages/analytics-browser/src/browser-client-precision.ts
deleted file mode 100644
--- a/packages/analytics-browser/src/browser-client-precision.ts
+++ /dev/null
@@ -1,356 +1,0 @@
-import {
- AmplitudeCore,
- Identify,
- returnWrapper,
- Revenue,
- UUID,
- getAnalyticsConnector,
- setConnectorDeviceId,
- setConnectorUserId,
- isNewSession,
- getQueryParams,
- Event,
- EventOptions,
- IIdentify,
- IRevenue,
- TransportTypeOrOptions,
- Result,
- BrowserOptions,
- BrowserConfig,
- BrowserClient,
- AnalyticsClient,
- AnalyticsIdentity,
- createIdentifyEvent,
- Logger,
- safeJsonStringify,
- LogLevel,
-} from '@amplitude/analytics-core';
-import { convertProxyObjectToRealObject, isInstanceProxy } from './utils/snippet-helper';
-import { useBrowserConfig, createTransport } from './config';
-import { DEFAULT_SERVER_ZONE } from './constants';
-
-const UNSPECIFIED_SESSION_ID = -1;
-
-/**
- * Exported for `@amplitude/unified` or integration with blade plugins.
- * If you only use `@amplitude/analytics-browser`, use `amplitude.init()` or `amplitude.createInstance()` instead.
- */
-export class AmplitudeBrowser extends AmplitudeCore implements BrowserClient, AnalyticsClient {
- // eslint-disable-next-line @typescript-eslint/ban-ts-comment
- // @ts-ignore
- config: BrowserConfig;
- previousSessionDeviceId: string | undefined;
- previousSessionUserId: string | undefined;
-
- // Backdoor to set diagnostics sample rate
- // by calling amplitude._setDiagnosticsSampleRate(1); before amplitude.init()
- _diagnosticsSampleRate = 0;
-
- // Backdoor to test request body compression
- // by calling amplitude._enableRequestBodyCompressionExperimental(true); before amplitude.init()
- _enableRequestBodyCompressionExperimentalValue = false;
-
- init(apiKey = '', userIdOrOptions?: string | BrowserOptions, maybeOptions?: BrowserOptions) {
- let userId: string | undefined;
- let options: BrowserOptions | undefined;
- if (arguments.length > 2) {
- userId = userIdOrOptions as string | undefined;
- options = maybeOptions;
- } else {
- if (typeof userIdOrOptions === 'string') {
- userId = userIdOrOptions;
- options = undefined;
- } else {
- userId = userIdOrOptions?.userId;
- options = userIdOrOptions;
- }
- }
- return returnWrapper(this._init({ ...options, userId, apiKey }));
- }
- protected async _init(options: BrowserOptions & { apiKey: string }) {
- // Step 1: Block concurrent initialization
- if (this.initializing) {
- return;
- }
- this.initializing = true;
-
- // Set up early dependencies needed for remote config client
- // These use options directly or fall back to defaults
- const loggerProvider = options.loggerProvider ?? new Logger();
- if (!options.loggerProvider) {
- loggerProvider.enable(options.logLevel ?? LogLevel.Warn);
- }
- const serverZone = options.serverZone ?? DEFAULT_SERVER_ZONE;
-
- // Step 2.4: Create browser config with diagnosticsClient and earlyConfig
- // earlyConfig ensures consistent logger/serverZone/diagnostics settings across all components
- const browserOptions = await useBrowserConfig(options.apiKey, options, this, {
- loggerProvider,
- serverZone,
- });
-
- await super._init(browserOptions);
- this.logBrowserOptions(browserOptions);
-
- // Step 3: Set session ID
- // Priority 1: `options.sessionId`
- // Priority 2: sessionId from url if it's Number and ampTimestamp is valid
- // Priority 3: last known sessionId from user identity storage
- // Default: `Date.now()`
- // Session ID is handled differently than device ID and user ID due to session events
- const queryParams = getQueryParams();
-
- // Check if ampTimestamp is present and valid
- const ampTimestamp = queryParams.ampTimestamp ? Number(queryParams.ampTimestamp) : undefined;
- const isWithinTimeLimit = ampTimestamp ? Date.now() < ampTimestamp : true;
-
- // check if we need to set the sessionId
- const querySessionId =
- isWithinTimeLimit && !Number.isNaN(Number(queryParams.ampSessionId))
- ? Number(queryParams.ampSessionId)
- : undefined;
-
- let deferredSessionId = this.config.deferredSessionId;
- if (deferredSessionId === UNSPECIFIED_SESSION_ID && !this.config.optOut) {
- deferredSessionId = Date.now();
- }
-
- this.setSessionId(options.sessionId ?? querySessionId ?? deferredSessionId ?? this.config.sessionId);
-
- if (this.config.optOut) {
- this.timeline.addOptOutListener(async (optOut) => {
- if (!optOut && this.config.deferredSessionId) {
- if (this.config.deferredSessionId === UNSPECIFIED_SESSION_ID) {
- this.setSessionId(undefined);
- } else {
- this.setSessionId(this.config.deferredSessionId);
- }
- }
- });
- }
-
- // Set up the analytics connector to integrate with the experiment SDK.
- // Send events from the experiment SDK and forward identifies to the
- // identity store.
- const connector = getAnalyticsConnector(options.instanceName);
- connector.identityStore.setIdentity({
- userId: this.config.userId,
- deviceId: this.config.deviceId,
- });
-
- // no plugins because precision
-
- this.initializing = false;
-
- // Step 6: Run queued dispatch functions
- await this.runQueuedFunctions('dispatchQ');
-
- // Step 7: Add the event receiver after running remaining queued functions.
- connector.eventBridge.setEventReceiver((event) => {
- const { time, ...cleanEventProperties } = event.eventProperties || {};
- const eventOptions = typeof time === 'number' ? { time } : undefined;
- void this.track(event.eventType, cleanEventProperties, eventOptions);
- });
- }
-
- getUserId() {
- return this.config?.userId;
- }
-
- setUserId(userId: string | undefined) {
- if (!this.config) {
- this.q.push(this.setUserId.bind(this, userId));
- return;
- }
- this.config.loggerProvider.debug('function setUserId: ', userId);
- if (userId !== this.config.userId || userId === undefined) {
- this.config.userId = userId;
- // eslint-disable-next-line @typescript-eslint/no-unsafe-call
- this.timeline.onIdentityChanged({ userId: userId });
- setConnectorUserId(userId, this.config.instanceName);
- }
- }
-
- getDeviceId() {
- return this.config?.deviceId;
- }
-
- setDeviceId(deviceId: string) {
- if (!this.config) {
- this.q.push(this.setDeviceId.bind(this, deviceId));
- return;
- }
- this.config.loggerProvider.debug('function setDeviceId: ', deviceId);
- if (deviceId !== this.config.deviceId) {
- this.config.deviceId = deviceId;
- // eslint-disable-next-line @typescript-eslint/no-unsafe-call
- this.timeline.onIdentityChanged({ deviceId: deviceId });
- setConnectorDeviceId(deviceId, this.config.instanceName);
- }
- }
-
- reset() {
- this.setDeviceId(UUID());
- this.setUserId(undefined);
- this.timeline.onReset();
- }
-
- getIdentity() {
- return {
- deviceId: this.config?.deviceId,
- userId: this.config?.userId,
- userProperties: this.userProperties,
- };
- }
-
- setIdentity(identity: Partial<AnalyticsIdentity>) {
- // Handle userId change
- if ('userId' in identity) {
- this.setUserId(identity.userId);
- }
-
- // Handle deviceId change
- if ('deviceId' in identity && identity.deviceId) {
- this.setDeviceId(identity.deviceId);
- }
-
- // Handle userProperties change - auto-send identify
- if ('userProperties' in identity) {
- this.userProperties = identity.userProperties;
- // Auto-send identify event with $set operations
- const identifyObj = new Identify();
- // istanbul ignore next
- const userProperties = identity.userProperties ?? {};
- for (const [key, value] of Object.entries(userProperties)) {
- // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
- // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
- identifyObj.set(key, value);
- }
- // The identify event processing in core-client already calls onIdentityChanged,
- // so we don't need to call it explicitly here to avoid duplicate notifications.
- this.identify(identifyObj);
- }
- }
-
- getOptOut(): boolean | undefined {
- return this.config?.optOut;
- }
-
- getSessionId() {
- return this.config?.sessionId;
- }
-
- setSessionId(sessionId: number | undefined) {
- const promises: Promise<Result>[] = [];
- if (!this.config) {
- this.q.push(this.setSessionId.bind(this, sessionId));
- return returnWrapper(Promise.resolve());
- }
- // do not start a new session if optOut is true
- if (this.config.optOut) {
- // save the sessionId to storage to be used when optOut is false
- this.config.deferredSessionId = sessionId ?? UNSPECIFIED_SESSION_ID;
- return returnWrapper(Promise.resolve());
- }
-
- // default sessionId to current time
- if (sessionId === undefined) {
- sessionId = Date.now();
- }
-
- // Prevents starting a new session with the same session ID
- if (sessionId === this.config.sessionId) {
- return returnWrapper(Promise.resolve());
- }
-
- this.config.loggerProvider.debug('function setSessionId: ', sessionId);
-
- const previousSessionId = this.getSessionId();
- if (previousSessionId !== sessionId) {
- // eslint-disable-next-line @typescript-eslint/no-unsafe-call
- this.timeline.onSessionIdChanged(sessionId);
- }
-
- this.config.sessionId = sessionId;
- this.config.lastEventTime = undefined;
- this.config.pageCounter = 0;
-
- // track the identify event if an Identify object is provided in the config
- if (this.config.identify) {
- promises.push(this.track(createIdentifyEvent(this.config.identify)).promise);
- }
-
- this.previousSessionDeviceId = this.config.deviceId;
- this.previousSessionUserId = this.config.userId;
- return returnWrapper(Promise.all(promises));
- }
-
- extendSession() {
- if (!this.config) {
- this.q.push(this.extendSession.bind(this));
- return;
- }
- this.config.lastEventTime = Date.now();
- }
-
- setTransport(transport: TransportTypeOrOptions) {
- if (!this.config) {
- this.q.push(this.setTransport.bind(this, transport));
- return;
- }
- this.config.transportProvider = createTransport(transport);
- }
-
- identify(identify: IIdentify, eventOptions?: EventOptions) {
- if (isInstanceProxy(identify)) {
- const queue = identify._q;
- identify._q = [];
- identify = convertProxyObjectToRealObject(new Identify(), queue);
- }
- if (eventOptions?.user_id) {
- this.setUserId(eventOptions.user_id);
- }
- if (eventOptions?.device_id) {
- this.setDeviceId(eventOptions.device_id);
- }
- return super.identify(identify, eventOptions);
- }
-
- groupIdentify(groupType: string, groupName: string | string[], identify: IIdentify, eventOptions?: EventOptions) {
- if (isInstanceProxy(identify)) {
- const queue = identify._q;
- identify._q = [];
- identify = convertProxyObjectToRealObject(new Identify(), queue);
- }
- return super.groupIdentify(groupType, groupName, identify, eventOptions);
- }
-
- revenue(revenue: IRevenue, eventOptions?: EventOptions) {
- if (isInstanceProxy(revenue)) {
- const queue = revenue._q;
- revenue._q = [];
- revenue = convertProxyObjectToRealObject(new Revenue(), queue);
- }
- return super.revenue(revenue, eventOptions);
- }
-
- async process(event: Event) {
- return super.process(event);
- }
-
- private logBrowserOptions(browserConfig: BrowserOptions & { apiKey: string }) {
- try {
- const browserConfigCopy = {
- ...browserConfig,
- apiKey: browserConfig.apiKey.substring(0, 10) + '********',
- };
- this.config.loggerProvider.debug(
- 'Initialized Amplitude with BrowserConfig:',
- safeJsonStringify(browserConfigCopy),
- );
- } catch (e) {
- /* istanbul ignore next */
- this.config.loggerProvider.error('Error logging browser config', e);
- }
- }
-}
\ No newline at end of file
diff --git a/packages/analytics-browser/src/default-instance.ts b/packages/analytics-browser/src/default-instance.ts
deleted file mode 100644
--- a/packages/analytics-browser/src/default-instance.ts
+++ /dev/null
@@ -1,30 +1,0 @@
-/* eslint-disable @typescript-eslint/unbound-method */
-// dummy patch analytics-browser to force GTM deploy
-import client from './browser-client-factory';
-
-export const {
- add,
- extendSession,
- flush,
- getDeviceId,
- getIdentity,
- getOptOut,
- getSessionId,
- getUserId,
- groupIdentify,
- identify,
- init,
- logEvent,
- remove,
- reset,
- revenue,
- setDeviceId,
- setGroup,
- setIdentity,
- setOptOut,
- setSessionId,
- setTransport,
- setUserId,
- track,
- _setDiagnosticsSampleRate,
-} = client;
\ No newline at end of fileYou can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit f7f5ca8. Configure here.
| setUserId, | ||
| track, | ||
| _setDiagnosticsSampleRate, | ||
| } = client; |
There was a problem hiding this comment.
Production file rewritten with "dummy patch" comment
High Severity
The default-instance.ts file — a production SDK export file — is completely rewritten with the comment "dummy patch analytics-browser to force GTM deploy." This appears to be an unrelated deployment workaround accidentally included in a test-only PR. Similarly, browser-client-precision.ts is added but never imported anywhere in the codebase (no references found via grep), making it dead code. Both changes modify the published analytics-browser package and don't belong in a PR titled "test: add playwright script."
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f7f5ca8. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Checklist
Note
Medium Risk
Primarily adds dev/test tooling (Playwright proxy + Vite middleware) but also introduces new
analytics-browsersource files that could affect packaging/bundling if inadvertently wired into production exports.Overview
Adds a new headed Playwright script (
e2e/manual-test.js) that proxies a target website to swap Amplitude CDN script URLs to a local Vite dev server, strips SRI integrity hashes (including inside GTM scripts), and optionally rewrites event ingestion requests with a providedAMPLITUDE_API_KEY.Updates the local
test-serverto better support this workflow by adding Private Network Access CORS handling (Access-Control-Allow-Private-Network), adjusting Vite plugin ordering so this middleware runs first, and fixing the gzip middleware to append (not overwrite) theVaryheader.Includes local unified-script artifacts (
test-server/unified-script-local*.js), addsplaywrightas a dev dependency, extends.gitignorefor Playwright MCP traces, and adds newanalytics-browsersource stubs (browser-client-precision.ts,default-instance.ts).Reviewed by Cursor Bugbot for commit f7f5ca8. Bugbot is set up for automated code reviews on this repo. Configure here.