chore: more tweaks to hackathon project - #1935
Merged
daniel-graham-amplitude merged 3 commits intoSep 22, 2026
Merged
Conversation
size-limit report 📦
|
![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 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Leftover unused install scaffolding
- Removed the unused checkout-install constants, CodeBlock import, commands style, and the prism-bash import that only existed for those setup commands.
- ✅ Fixed: Tab swap skips SDK injection
- onReplaced now injects into an already-committed replacement tab, and runOnUrl treats a prerender swap as success instead of a missing-tab error.
Or push these changes by commenting:
@cursor push 2f3cd503df
Preview (2f3cd503df)
diff --git a/test-server/configurator-extension/background.js b/test-server/configurator-extension/background.js
--- a/test-server/configurator-extension/background.js
+++ b/test-server/configurator-extension/background.js
@@ -19,6 +19,10 @@
const TABS_KEY = 'instrumentedTabs';
const LAST_PAYLOAD_KEY = 'lastPayload';
+// Filled synchronously at the start of onReplaced so runOnUrl can tell a prerender swap from a close
+// after tabs.update fails on the old id. Dropped on the next turn, once that catch has had a look.
+const replacedTabs = new Map();
+
// What the configurator sends when no API key has been typed in — PLACEHOLDER_API_KEY in its snippet.js.
const PLACEHOLDER_API_KEY = 'YOUR_API_KEY';
@@ -110,44 +114,51 @@
};
}
+async function injectInto(tabId, payload, url) {
+ if (!url?.startsWith('http')) {
+ return;
+ }
+ // Read before injecting: by the time a navigation commits the response headers have arrived, which is
+ // where the policy the page was sent is still visible.
+ const csp = cspReport(tabId, payload);
+ if (csp) {
+ await ignoreMissingTab(chrome.action.setTitle({ tabId, title: csp.summary }));
+ }
+ const target = { tabId };
+ const inject = (options) =>
+ chrome.scripting.executeScript({ target, world: 'MAIN', injectImmediately: true, ...options });
+ try {
+ await inject({ func: handOver, args: [payload, csp] });
+ await inject({ files: [SDK_BUNDLE] });
+ if (payload.sessionReplay) {
+ // Its own call: a plugin bundle that won't load shouldn't stop analytics from running, and
+ // inject.js reports the gap when the global it expects isn't there.
+ try {
+ await inject({ files: [SESSION_REPLAY_BUNDLE] });
+ } catch (error) {
+ console.warn('[amplitude-configurator] session replay bundle failed to load', error);
+ }
+ }
+ await inject({ files: ['inject.js'] });
+ } catch (error) {
+ if (isMissingTab(error)) {
+ throw error;
+ }
+ console.error('[amplitude-configurator] injection failed', error);
+ await ignoreMissingTab(chrome.action.setBadgeText({ tabId, text: 'err' }));
+ }
+}
+
chrome.webNavigation.onCommitted.addListener(
guard('injection', async ({ tabId, frameId, url }) => {
- if (frameId !== 0 || !url.startsWith('http')) {
+ if (frameId !== 0) {
return;
}
const payload = (await instrumentedTabs())[tabId];
if (!payload) {
return;
}
- // Read before injecting: by the time a navigation commits the response headers have arrived, which is
- // where the policy the page was sent is still visible.
- const csp = cspReport(tabId, payload);
- if (csp) {
- await ignoreMissingTab(chrome.action.setTitle({ tabId, title: csp.summary }));
- }
- const target = { tabId };
- const inject = (options) =>
- chrome.scripting.executeScript({ target, world: 'MAIN', injectImmediately: true, ...options });
- try {
- await inject({ func: handOver, args: [payload, csp] });
- await inject({ files: [SDK_BUNDLE] });
- if (payload.sessionReplay) {
- // Its own call: a plugin bundle that won't load shouldn't stop analytics from running, and
- // inject.js reports the gap when the global it expects isn't there.
- try {
- await inject({ files: [SESSION_REPLAY_BUNDLE] });
- } catch (error) {
- console.warn('[amplitude-configurator] session replay bundle failed to load', error);
- }
- }
- await inject({ files: ['inject.js'] });
- } catch (error) {
- if (isMissingTab(error)) {
- throw error;
- }
- console.error('[amplitude-configurator] injection failed', error);
- await ignoreMissingTab(chrome.action.setBadgeText({ tabId, text: 'err' }));
- }
+ await injectInto(tabId, payload, url);
}),
);
@@ -178,8 +189,9 @@
}
// The tab opens blank so it can be marked for instrumentation before it commits anything; navigating
// afterwards is what makes the ordering reliable. It also means there is a moment where the run depends
- // on a tab nobody is looking at yet, and anything that closes it — a click, a tab-tidying extension,
- // Chrome swapping in a prerender — leaves the steps below with nothing to work on.
+ // on a tab nobody is looking at yet, and anything that closes it — a click, a tab-tidying extension —
+ // leaves the steps below with nothing to work on. A prerender swap is different: onReplaced moves the
+ // mark, and the catch below treats that as the run continuing rather than as a failure.
let tab;
try {
tab = await chrome.tabs.create({ url: 'about:blank', active: true });
@@ -190,6 +202,11 @@
if (!isMissingTab(error)) {
throw error;
}
+ // Chrome can swap the blank tab for a prerender of the destination; onReplaced records that
+ // synchronously, and has already moved the mark to the surviving id.
+ if (tab && replacedTabs.has(tab.id)) {
+ return { message: describe(payload) };
+ }
if (tab) {
// The mark and the CSP rule are both keyed by tab id, and Chrome reuses ids, so leaving them behind
// would take the policy off whichever tab inherits this one's.
@@ -234,11 +251,21 @@
// mark and the CSP rule across keeps the run alive, and keeps a rule from outliving the tab it was for.
chrome.tabs.onReplaced.addListener(
guard('tab replacement', async (addedTabId, removedTabId) => {
+ replacedTabs.set(removedTabId, addedTabId);
+ setTimeout(() => replacedTabs.delete(removedTabId), 0);
const payload = (await instrumentedTabs())[removedTabId];
if (!payload) {
return;
}
await forget(removedTabId);
await instrument(addedTabId, payload);
+ // The prerendered document committed under this id before it was marked, so onCommitted will not
+ // run again for this load. Inject into whatever is already there; a document that hasn't committed
+ // yet is left for the forthcoming onCommitted.
+ const frames = await chrome.webNavigation.getAllFrames({ tabId: addedTabId });
+ const url = frames?.find((frame) => frame.frameId === 0)?.url;
+ if (url) {
+ await injectInto(addedTabId, payload, url);
+ }
}),
);
diff --git a/test-server/configurator/components.jsx b/test-server/configurator/components.jsx
--- a/test-server/configurator/components.jsx
+++ b/test-server/configurator/components.jsx
@@ -1,8 +1,5 @@
import React from 'react';
-// Prism's default build already registers the javascript and markup grammars, so only the shell one the
-// extension's setup commands are shown in has to be pulled in.
import Prism from 'prismjs';
-import 'prismjs/components/prism-bash';
import './syntax-theme.css';
const styles = {
diff --git a/test-server/configurator/runner-extension-panel.jsx b/test-server/configurator/runner-extension-panel.jsx
--- a/test-server/configurator/runner-extension-panel.jsx
+++ b/test-server/configurator/runner-extension-panel.jsx
@@ -2,12 +2,8 @@
// install link to point at: the steps are the download this server builds, and Load unpacked. They mirror
// test-server/configurator-extension/README.md, which is the fuller account.
import React from 'react';
-import { CodeBlock, Panel } from './components.jsx';
+import { Panel } from './components.jsx';
-const EXTENSION_DIRECTORY = 'test-server/configurator-extension';
-
-const REPOSITORY_URL = `https://github.com/amplitude/Amplitude-TypeScript/tree/main/${EXTENSION_DIRECTORY}`;
-
// Built by test-server/extension-archive.js, which owns this path, out of the extension directory as it
// stands in whatever checkout is serving this page.
const ARCHIVE_URL = '/configurator-extension.zip';
@@ -16,18 +12,11 @@
// read the same either way.
const UNPACKED_FOLDER = 'configurator-extension';
-// The bundles the extension injects aren't checked in. The archive carries them already; a checkout has
-// to build them before Chrome will accept the folder.
-const SETUP_COMMANDS = `pnpm --dir packages/analytics-browser build
-pnpm --dir packages/plugin-session-replay-browser build
-node ${EXTENSION_DIRECTORY}/sync-vendor.mjs`;
-
const styles = {
wrapper: { maxWidth: 760, margin: '0 0 20px' },
note: { color: '#888', fontSize: 12, margin: '0 0 10px' },
steps: { margin: '0 0 10px', paddingLeft: 20, fontSize: 13, color: '#444', lineHeight: 1.6 },
step: { marginBottom: 6 },
- commands: { margin: '8px 0 4px' },
};
export function RunnerExtensionPanel({ version }) {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 3 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Tab storage update race
- Serialized instrument, forget, and takePayload so overlapping session-storage read-modify-writes can no longer drop another tab's mark.
Or push these changes by commenting:
@cursor push 255428c300
Preview (255428c300)
diff --git a/test-server/configurator-extension/background.js b/test-server/configurator-extension/background.js
--- a/test-server/configurator-extension/background.js
+++ b/test-server/configurator-extension/background.js
@@ -55,6 +55,21 @@
return tabs;
}
+// chrome.storage.session has no atomic update, so a get that yields and a later set of the whole map
+// would write a snapshot that no longer has every tab. instrument, forget and takePayload all do
+// that — takePayload on nearly every first commit, because clearSession defaults to true — so they
+// share a queue, and each re-reads after the previous write has landed.
+let tabsQueue = Promise.resolve();
+
+function withTabs(work) {
+ const run = tabsQueue.then(work);
+ tabsQueue = run.then(
+ () => {},
+ () => {},
+ );
+ return run;
+}
+
// A tab is not a thing that stays put: it can be closed, and Chrome can swap it for a prerendered one
// mid-navigation. Every call below that names a tab id can therefore find nothing there, and tabs, action
// and scripting all say so the same way — "No tab with id: 1234.", a message that says nothing about what
@@ -92,16 +107,20 @@
// Both transitions carry the CSP rule with them, so no path can mark a tab and forget to clear the way for
// what the SDK is about to do — or leave a tab unprotected after instrumentation stops.
async function instrument(tabId, payload) {
- const tabs = await instrumentedTabs();
- await chrome.storage.session.set({ [TABS_KEY]: { ...tabs, [tabId]: payload } });
+ await withTabs(async () => {
+ const tabs = await instrumentedTabs();
+ await chrome.storage.session.set({ [TABS_KEY]: { ...tabs, [tabId]: payload } });
+ });
await relaxCsp(tabId);
await ignoreMissingTab(chrome.action.setBadgeText({ tabId, text: 'on' }));
}
async function forget(tabId) {
- const tabs = await instrumentedTabs();
- delete tabs[tabId];
- await chrome.storage.session.set({ [TABS_KEY]: tabs });
+ await withTabs(async () => {
+ const tabs = await instrumentedTabs();
+ delete tabs[tabId];
+ await chrome.storage.session.set({ [TABS_KEY]: tabs });
+ });
await restoreCsp(tabId);
}
@@ -118,16 +137,18 @@
// tab and the navigation it opened, which would otherwise make "first commit" mean "first since the worker
// last woke up".
async function takePayload(tabId) {
- const tabs = await instrumentedTabs();
- const payload = tabs[tabId];
- if (!payload) {
- return undefined;
- }
- const { clearSession, mockReferrer, ...rest } = payload;
- if (clearSession || mockReferrer) {
- await chrome.storage.session.set({ [TABS_KEY]: { ...tabs, [tabId]: rest } });
- }
- return payload;
+ return withTabs(async () => {
+ const tabs = await instrumentedTabs();
+ const payload = tabs[tabId];
+ if (!payload) {
+ return undefined;
+ }
+ const { clearSession, mockReferrer, ...rest } = payload;
+ if (clearSession || mockReferrer) {
+ await chrome.storage.session.set({ [TABS_KEY]: { ...tabs, [tabId]: rest } });
+ }
+ return payload;
+ });
}
// Runs in the page before the SDK bundle: saves what the page had under window.amplitude, since theYou can send follow-ups to the cloud agent here.
Mercy811
approved these changes
Sep 22, 2026
daniel-graham-amplitude
force-pushed
the
hackathon-client-configurator-hosted
branch
from
September 22, 2026 17:17
9b728c0 to
64460fa
Compare
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: Storage access skips its try/catch
- Moved the localStorage and sessionStorage lookups inside the existing try so a blocked-store SecurityError is caught per store and no longer aborts session clearing or the follow-on SDK inject.
Or push these changes by commenting:
@cursor push c835c12cc1
Preview (c835c12cc1)
diff --git a/test-server/configurator-extension/background.js b/test-server/configurator-extension/background.js
--- a/test-server/configurator-extension/background.js
+++ b/test-server/configurator-extension/background.js
@@ -177,13 +177,12 @@
removed.push(`cookie ${name}`);
}
- for (const [label, store] of [
- ['localStorage', localStorage],
- ['sessionStorage', sessionStorage],
- ]) {
+ for (const label of ['localStorage', 'sessionStorage']) {
// Reading either throws outright where the site's cookie policy forbids it, which says nothing about
- // the other or about the cookies above.
+ // the other or about the cookies above. The lookup itself has to sit inside the try: the getter throws
+ // before the loop body when storage is blocked.
try {
+ const store = window[label];
for (const key of Object.keys(store).filter(isAmplitude)) {
store.removeItem(key);
removed.push(`${label} ${key}`);You can send follow-ups to the cloud agent here.
Base automatically changed from
hackathon-client-configurator-extension
to
main
September 22, 2026 17:46
daniel-graham-amplitude
changed the title
daniel-graham-amplitude
force-pushed
the
hackathon-client-configurator-hosted
branch
from
September 22, 2026 17:51
64460fa to
546de49
Compare
daniel-graham-amplitude
disabled the stack merge
September 22, 2026 17:51
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue. You can view the agent here.
Reviewed by Cursor Bugbot for commit 546de49. Configure here.
daniel-graham-amplitude
deleted the
hackathon-client-configurator-hosted
branch
September 22, 2026 20:11
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
Changes affect a dev-only Chrome extension with broad host permissions and page-main-world injection, including storage wiping on target origins; scope is test-server/configurator tooling rather than published SDK packages.
Overview
Expands the configurator Run on URL flow and the Chrome runner extension so hosted and local setups can install the extension without building SDK bundles manually, and so attribution testing behaves more like a fresh visit.
The test server (and
vite build) now servesconfigurator-extension.zipplus a shipped version JSON viaextension-archive.js, withRunnerExtensionPanelguiding download/unpack/load and warning when the unpacked runner is older than the server. Vendor sync also copies source maps, and the manifest adds a hosted Netlify configurator origin plusweb_accessible_resourcesfor maps.The configurator adds a dedicated Run on URL panel (target URL, optional mock referrer, Clean Session default on) persisted in share links, a Blades picker that gates Session Replay / Guides sections, and disables Run on URL until the extension is detected (with clearer errors after extension reload).
In
background.js, the extension applies mock referrer and Amplitude storage cleanup only on the first navigation of a run (takePayload), shadowsdocument.referrerbefore SDK init, clearsAMP_/amp_cookies and web storage when requested, and hardens tab lifecycle (missing tab handling,tabs.onReplacedmigration, guarded async listeners). The configurator bridge returns an explicit error when a page’s content script is orphaned after an extension reload.Reviewed by Cursor Bugbot for commit 546de49. Bugbot is set up for automated code reviews on this repo. Configure here.