Skip to content

Commit f976bb6

Browse files
ryanbas21claude
andcommitted
feat: harden reliability, remove global mutable state, fix test flakiness
- Remove configureDevtools global singleton; pass DevtoolsOptions through call sites to eliminate shared mutable state - Cap __PING_DEVTOOLS_STATE__ at 500 entries to prevent memory leaks - Guard localStorage/cookie access with try-catch for privacy modes - Pin GitHub Actions to commit SHAs for supply-chain security - Replace waitForTimeout with Playwright toPass/expect retries in e2e tests - Fix CORS credentials-mismatch false positive when no credentials sent - Remove hard-coded PKCE challengeMethod from token annotator - Add origin check to content relay for defense-in-depth - Fix service worker rehydration (module-eval vs activate lifecycle) - Add defensive guards for snapshot loading and event-store hydration - Clean up CDP client disconnect (reject pending calls, clear state) - Properly dispose ManagedRuntime on VS Code extension deactivate - Add devtools-core and vscode-extension to root tsconfig references - Fix devtools-ui ports export to use .ts source Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 8b3a644 commit f976bb6

25 files changed

Lines changed: 271 additions & 207 deletions

File tree

.github/workflows/publish-extension.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ jobs:
2727
run: cd dist && zip -r ../extension-chrome.zip .
2828

2929
- name: Upload to Chrome Web Store
30-
uses: mnao305/chrome-extension-upload@v5.0.0
30+
uses: mnao305/chrome-extension-upload@4008e29e13c144d0f6725462cbd49b7c291b4928 # v5.0.0
3131
with:
3232
file-path: packages/devtools-extension/extension-chrome.zip
3333
extension-id: ${{ secrets.CHROME_EXTENSION_ID }}
@@ -65,7 +65,7 @@ jobs:
6565
run: zip -r source.zip . -x 'node_modules/*' '*/node_modules/*' '*/dist/*' '.git/*' '*.zip'
6666

6767
- name: Upload to Firefox Add-ons
68-
uses: trmcnvn/firefox-addon@v1
68+
uses: trmcnvn/firefox-addon@0d05671269b82c69c3f22ed86d8e772e89d47cf4 # v1
6969
with:
7070
uuid: oidc-devtool@wolfcola
7171
xpi: packages/devtools-extension/extension-firefox.zip

.github/workflows/release.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ jobs:
5151

5252
- name: Publish Chrome extension to testers
5353
if: inputs.extension
54-
uses: mnao305/chrome-extension-upload@v5.0.0
54+
uses: mnao305/chrome-extension-upload@4008e29e13c144d0f6725462cbd49b7c291b4928 # v5.0.0
5555
with:
5656
file-path: packages/devtools-extension/extension-chrome.zip
5757
extension-id: ${{ secrets.CHROME_EXTENSION_ID }}
@@ -77,7 +77,7 @@ jobs:
7777

7878
- name: Publish Firefox extension (unlisted)
7979
if: inputs.extension
80-
uses: trmcnvn/firefox-addon@v1
80+
uses: trmcnvn/firefox-addon@0d05671269b82c69c3f22ed86d8e772e89d47cf4 # v1
8181
with:
8282
uuid: oidc-devtool@wolfcola
8383
xpi: packages/devtools-extension/extension-firefox.zip
@@ -108,7 +108,7 @@ jobs:
108108
run: pnpm build
109109

110110
- name: Create release PR or publish
111-
uses: changesets/action@v1
111+
uses: changesets/action@63a615b9cd06ba9a3e6d13796c7fbcb080a60a0b # v1.8.0
112112
with:
113113
publish: pnpm release
114114
version: pnpm run version

e2e/fixtures/extension.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ export const test = base.extend<TestFixtures>({
7272
mockServer: async ({}, use) => {
7373
const result = await createMockOidcServer(0);
7474
await use(result);
75-
result.server.close();
75+
await new Promise<void>((resolve) => result.server.close(() => resolve()));
7676
},
7777
});
7878

e2e/tests/network-capture.test.ts

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,13 @@ test.describe('network capture pipeline', () => {
2929
});
3030
}, `${mockServer.baseUrl}/.well-known/openid-configuration`);
3131

32-
await panelPage.waitForTimeout(1000);
33-
34-
await panelPage.reload();
35-
await panelPage.waitForSelector('.toolbar', { state: 'visible' });
36-
await panelPage.waitForTimeout(500);
37-
38-
const eventCount = await getEventCount(panelPage);
39-
expect(eventCount).toBeGreaterThanOrEqual(1);
32+
// Wait for the service worker to persist the event, then reload to verify
33+
await expect(async () => {
34+
await panelPage.reload();
35+
await panelPage.waitForSelector('.toolbar', { state: 'visible' });
36+
const eventCount = await getEventCount(panelPage);
37+
expect(eventCount).toBeGreaterThanOrEqual(1);
38+
}).toPass({ timeout: 5000 });
4039

4140
await panelPage.close();
4241
});
@@ -75,8 +74,6 @@ test.describe('network capture pipeline', () => {
7574
});
7675
}, `${mockServer.baseUrl}/.well-known/openid-configuration`);
7776

78-
await panelPage.waitForTimeout(500);
79-
8077
await panelPage.evaluate((url) => {
8178
chrome.runtime.sendMessage({
8279
type: 'NETWORK_EVENT',
@@ -104,13 +101,13 @@ test.describe('network capture pipeline', () => {
104101
});
105102
}, `${mockServer.baseUrl}/token`);
106103

107-
await panelPage.waitForTimeout(1000);
108-
await panelPage.reload();
109-
await panelPage.waitForSelector('.toolbar', { state: 'visible' });
110-
await panelPage.waitForTimeout(500);
111-
112-
const eventCount = await getEventCount(panelPage);
113-
expect(eventCount).toBeGreaterThanOrEqual(2);
104+
// Wait for both events to be persisted, then reload to verify
105+
await expect(async () => {
106+
await panelPage.reload();
107+
await panelPage.waitForSelector('.toolbar', { state: 'visible' });
108+
const eventCount = await getEventCount(panelPage);
109+
expect(eventCount).toBeGreaterThanOrEqual(2);
110+
}).toPass({ timeout: 5000 });
114111

115112
await panelPage.close();
116113
});

e2e/tests/panel-renders-events.test.ts

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,13 @@ test.describe('panel renders events', () => {
3434
makeSdkEvent('test-sdk-1', 'test-flow-sdk-1'),
3535
);
3636

37-
await panelPage.waitForTimeout(500);
38-
await panelPage.reload();
39-
await panelPage.waitForSelector('.toolbar', { state: 'visible' });
40-
await panelPage.waitForTimeout(500);
41-
42-
const after = await panelPage.locator('.tl-row').count();
43-
expect(after).toBeGreaterThan(before);
37+
// Wait for the event to be persisted, then reload to verify
38+
await expect(async () => {
39+
await panelPage.reload();
40+
await panelPage.waitForSelector('.toolbar', { state: 'visible' });
41+
const after = await panelPage.locator('.tl-row').count();
42+
expect(after).toBeGreaterThan(before);
43+
}).toPass({ timeout: 5000 });
4444

4545
await panelPage.close();
4646
});
@@ -56,22 +56,19 @@ test.describe('panel renders events', () => {
5656
makeSdkEvent('test-sdk-clear', 'test-flow-clear'),
5757
);
5858

59-
await panelPage.waitForTimeout(500);
60-
await panelPage.reload();
61-
await panelPage.waitForSelector('.toolbar', { state: 'visible' });
62-
await panelPage.waitForTimeout(500);
63-
64-
let rows = await panelPage.locator('.tl-row').count();
65-
expect(rows).toBeGreaterThan(0);
59+
// Wait for event to appear
60+
await expect(async () => {
61+
await panelPage.reload();
62+
await panelPage.waitForSelector('.toolbar', { state: 'visible' });
63+
const rows = await panelPage.locator('.tl-row').count();
64+
expect(rows).toBeGreaterThan(0);
65+
}).toPass({ timeout: 5000 });
6666

6767
const clearBtn = panelPage.locator('.tb-btn', { hasText: 'Clear' });
68-
if (await clearBtn.isVisible()) {
69-
await clearBtn.click();
70-
await panelPage.waitForTimeout(500);
68+
await expect(clearBtn).toBeVisible();
69+
await clearBtn.click();
7170

72-
rows = await panelPage.locator('.tl-row').count();
73-
expect(rows).toBe(0);
74-
}
71+
await expect(panelPage.locator('.tl-row')).toHaveCount(0, { timeout: 3000 });
7572

7673
await panelPage.close();
7774
});

packages/devtools-bridge/src/index.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,5 @@ export { attachJourneyBridge } from './lib/journey-bridge.js';
44
export type { JourneyBridgeHandle } from './lib/journey-bridge.js';
55
export { attachOidcBridge } from './lib/oidc-bridge.js';
66
export type { OidcBridgeHandle } from './lib/oidc-bridge.js';
7-
export {
8-
DEVTOOLS_EVENT_NAME,
9-
emitAuthEvent,
10-
emitConfigEvent,
11-
configureDevtools,
12-
} from './lib/emit.js';
7+
export { DEVTOOLS_EVENT_NAME, emitAuthEvent, emitConfigEvent } from './lib/emit.js';
138
export type { DevtoolsOptions } from './lib/emit.js';

packages/devtools-bridge/src/lib/bridge.ts

Lines changed: 71 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { Schema, Option, pipe } from 'effect';
22
import type { SdkData } from '@wolfcola/devtools-types';
33
import { SdkErrorSchema, SdkAuthorizationSchema } from '@wolfcola/devtools-types';
4-
import { emitAuthEvent, emitConfigEvent, configureDevtools } from './emit.js';
4+
import { emitAuthEvent, emitConfigEvent } from './emit.js';
55
import type { DevtoolsOptions } from './emit.js';
66

77
interface Subscribable {
@@ -95,51 +95,68 @@ interface SessionSnapshot {
9595

9696
function snapshotSession(): SessionSnapshot {
9797
const storage: Record<string, string> = {};
98-
for (let i = 0; i < localStorage.length; i++) {
99-
const k = localStorage.key(i);
100-
if (k) storage[k] = localStorage.getItem(k) ?? '';
98+
try {
99+
for (let i = 0; i < localStorage.length; i++) {
100+
const k = localStorage.key(i);
101+
if (k) storage[k] = localStorage.getItem(k) ?? '';
102+
}
103+
} catch {
104+
// localStorage access blocked (e.g. privacy mode, opaque origin)
105+
}
106+
let cookie = '';
107+
try {
108+
cookie = document.cookie;
109+
} catch {
110+
// cookie access blocked
101111
}
102-
return { cookie: document.cookie, storage };
112+
return { cookie, storage };
103113
}
104114

105115
function emitSessionDiffs(
106116
before: SessionSnapshot,
107117
after: SessionSnapshot,
108118
flowId: string | null,
119+
options?: DevtoolsOptions,
109120
): void {
110121
if (before.cookie !== after.cookie) {
111-
emitAuthEvent({
112-
id: crypto.randomUUID(),
113-
timestamp: performance.now(),
114-
type: 'session:cookie',
115-
source: 'session',
116-
flowId,
117-
causedBy: null,
118-
data: {
119-
_tag: 'session',
120-
key: 'document.cookie',
121-
before: before.cookie || undefined,
122-
after: after.cookie || undefined,
122+
emitAuthEvent(
123+
{
124+
id: crypto.randomUUID(),
125+
timestamp: performance.now(),
126+
type: 'session:cookie',
127+
source: 'session',
128+
flowId,
129+
causedBy: null,
130+
data: {
131+
_tag: 'session',
132+
key: 'document.cookie',
133+
before: before.cookie || undefined,
134+
after: after.cookie || undefined,
135+
},
136+
flags: { isCors: false, isError: false, isAuthRelated: true },
123137
},
124-
flags: { isCors: false, isError: false, isAuthRelated: true },
125-
});
138+
options,
139+
);
126140
}
127141

128142
const allKeys = new Set([...Object.keys(before.storage), ...Object.keys(after.storage)]);
129143
for (const key of allKeys) {
130144
const beforeVal = before.storage[key];
131145
const afterVal = after.storage[key];
132146
if (beforeVal !== afterVal) {
133-
emitAuthEvent({
134-
id: crypto.randomUUID(),
135-
timestamp: performance.now(),
136-
type: 'session:storage',
137-
source: 'session',
138-
flowId,
139-
causedBy: null,
140-
data: { _tag: 'session', key, before: beforeVal, after: afterVal },
141-
flags: { isCors: false, isError: false, isAuthRelated: true },
142-
});
147+
emitAuthEvent(
148+
{
149+
id: crypto.randomUUID(),
150+
timestamp: performance.now(),
151+
type: 'session:storage',
152+
source: 'session',
153+
flowId,
154+
causedBy: null,
155+
data: { _tag: 'session', key, before: beforeVal, after: afterVal },
156+
flags: { isCors: false, isError: false, isAuthRelated: true },
157+
},
158+
options,
159+
);
143160
}
144161
}
145162
}
@@ -148,21 +165,24 @@ function emitSessionDiffs(
148165
// Event builders
149166
// ---------------------------------------------------------------------------
150167

151-
function emitNodeChange(data: SdkData): void {
152-
emitAuthEvent({
153-
id: crypto.randomUUID(),
154-
timestamp: performance.now(),
155-
type: 'sdk:node-change',
156-
source: 'sdk',
157-
flowId: data.interactionId ?? null,
158-
causedBy: null,
159-
data,
160-
flags: {
161-
isCors: false,
162-
isError: data.nodeStatus === 'error' || data.nodeStatus === 'failure',
163-
isAuthRelated: true,
168+
function emitNodeChange(data: SdkData, options?: DevtoolsOptions): void {
169+
emitAuthEvent(
170+
{
171+
id: crypto.randomUUID(),
172+
timestamp: performance.now(),
173+
type: 'sdk:node-change',
174+
source: 'sdk',
175+
flowId: data.interactionId ?? null,
176+
causedBy: null,
177+
data,
178+
flags: {
179+
isCors: false,
180+
isError: data.nodeStatus === 'error' || data.nodeStatus === 'failure',
181+
isAuthRelated: true,
182+
},
164183
},
165-
});
184+
options,
185+
);
166186
}
167187

168188
// ---------------------------------------------------------------------------
@@ -187,10 +207,6 @@ export function attachDevToolsBridge(
187207
return { detach: () => undefined };
188208
}
189209

190-
if (devtoolsOptions) {
191-
configureDevtools(devtoolsOptions);
192-
}
193-
194210
let previousStatus: string | undefined;
195211
let configEmitted = false;
196212
let lastSnapshot: SessionSnapshot = snapshotSession();
@@ -212,14 +228,19 @@ export function attachDevToolsBridge(
212228
Option.map((data) => {
213229
if (config && !configEmitted) {
214230
configEmitted = true;
215-
emitConfigEvent(config);
231+
emitConfigEvent(config, devtoolsOptions);
216232
}
217-
emitNodeChange(data);
233+
emitNodeChange(data, devtoolsOptions);
218234
// Snapshot before deferring so mutations in the same call stack are captured.
219235
const snapshotBefore = lastSnapshot;
220236
setTimeout(() => {
221237
const snapshotAfter = snapshotSession();
222-
emitSessionDiffs(snapshotBefore, snapshotAfter, data.interactionId ?? null);
238+
emitSessionDiffs(
239+
snapshotBefore,
240+
snapshotAfter,
241+
data.interactionId ?? null,
242+
devtoolsOptions,
243+
);
223244
lastSnapshot = snapshotAfter;
224245
}, 0);
225246
}),

0 commit comments

Comments
 (0)