Skip to content

Commit 3116b16

Browse files
authored
fix(cf): scope-bound solvedCFTargetIds lifecycle and Docker build fix (#125)
* build: add commitlint for structured commit message validation * build: upgrade vite 8 + vitest 4.1, add prom-client as explicit dep - Upgrade vite ^7.3.1 → ^8.0.0 and vitest ^4.0.18 → ^4.1.0 - Add prom-client ^15.1.3 as explicit dependency (was phantom dep that got de-hoisted when commitlint was added, causing silent server crash on startup) * fix(cf): scope-bound solvedCFTargetIds lifecycle and Docker build fix - Replace raw Set.add with scope-bound addSolvedCFTarget that auto-cleans entries when owning page is destroyed, preventing unbounded growth - Add page cleanup scopes to CloudflareStateTracker with proper finalizers - Add startup failure diagnostics to vitest integration globalSetup - Fix Docker build: add --ignore-scripts to npm clean-install (lefthook)
1 parent a943e78 commit 3116b16

5 files changed

Lines changed: 116 additions & 16 deletions

File tree

docker/base/Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ RUN curl -fsSL https://bun.com/install | bash && \
9494

9595
COPY package.json package-lock.json ./
9696

97-
RUN npm clean-install
97+
RUN npm clean-install --ignore-scripts
9898

9999
COPY assets assets
100100
COPY bin bin

src/session/cf/cloudflare-detector.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -897,7 +897,7 @@ export class CloudflareDetector {
897897
);
898898
if (postSolveSnapshot._tag === 'detected') {
899899
for (const t of postSolveSnapshot.targets) {
900-
self.state.solvedCFTargetIds.add(t.targetId);
900+
yield* self.state.addSolvedCFTarget(t.targetId, targetId);
901901
}
902902
}
903903
})(),
@@ -989,9 +989,9 @@ export class CloudflareDetector {
989989
break;
990990
}
991991

992-
// Common cleanup
992+
// Common cleanup — scope-bound so entries are removed when page is destroyed
993993
for (const t of filteredDetection.targets) {
994-
self.state.solvedCFTargetIds.add(t.targetId);
994+
yield* self.state.addSolvedCFTarget(t.targetId, targetId);
995995
}
996996
return;
997997
}

src/session/cf/cloudflare-state-tracker.ts

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Effect, Schedule } from 'effect';
1+
import { Effect, Exit, Schedule, Scope } from 'effect';
22

33
import { runForkInServer } from '../../otel-runtime.js';
44
import {
@@ -107,6 +107,8 @@ export class CloudflareStateTracker {
107107
readonly pendingRechallengeCount = new Map<TargetId, number>();
108108
/** Per-page reload count for widget-not-rendered recovery. Reset on solve. */
109109
readonly widgetReloadCount = new Map<TargetId, number>();
110+
/** Per-page cleanup scopes — finalizers remove solvedCFTargetIds entries when a page is destroyed. */
111+
private readonly pageCleanupScopes = new Map<TargetId, Scope.Closeable>();
110112
config: Required<CloudflareConfig> = { maxAttempts: 3, attemptTimeout: 30000, recordingMarkers: true };
111113
destroyed = false;
112114
/** Per-page accumulator of solved/failed phases for compound summary labels. */
@@ -355,6 +357,36 @@ export class CloudflareStateTracker {
355357
return this.registry.destroyAll();
356358
}
357359

360+
/**
361+
* Add an OOPIF target ID to solvedCFTargetIds with scope-bound cleanup.
362+
* When the owning page's cleanup scope is closed (via unregisterPage),
363+
* the finalizer atomically removes the entry — preventing unbounded growth.
364+
*/
365+
addSolvedCFTarget(oopifId: string, pageTargetId: TargetId): Effect.Effect<void> {
366+
const tracker = this;
367+
return Effect.suspend(() => {
368+
tracker.solvedCFTargetIds.add(oopifId);
369+
const scope = tracker.pageCleanupScopes.get(pageTargetId);
370+
if (!scope) return Effect.void; // safety: no scope = cleanup falls to destroy()
371+
return Scope.addFinalizer(scope, Effect.sync(() => {
372+
tracker.solvedCFTargetIds.delete(oopifId);
373+
}));
374+
});
375+
}
376+
377+
/**
378+
* Synchronous variant of addSolvedCFTarget — used in stopTargetDetection
379+
* where we're outside an Effect generator context.
380+
*/
381+
addSolvedCFTargetSync(oopifId: string, pageTargetId: TargetId): void {
382+
this.solvedCFTargetIds.add(oopifId);
383+
const scope = this.pageCleanupScopes.get(pageTargetId);
384+
if (!scope) return; // safety: no scope = cleanup falls to destroy()
385+
Effect.runSync(Scope.addFinalizer(scope, Effect.sync(() => {
386+
this.solvedCFTargetIds.delete(oopifId);
387+
})));
388+
}
389+
358390
/**
359391
* Resolve an active detection as auto-solved (token appeared without navigation).
360392
* Token is provided by the CF bridge push event — no Runtime.evaluate fallback.
@@ -487,9 +519,12 @@ export class CloudflareStateTracker {
487519
);
488520
}
489521

490-
/** Register a page target → CDP session mapping. */
522+
/** Register a page target → CDP session mapping. Creates a cleanup scope for scope-bound solvedCFTargetIds entries. */
491523
registerPage(targetId: TargetId, cdpSessionId: CdpSessionId): void {
492524
this.knownPages.set(targetId, cdpSessionId);
525+
if (!this.pageCleanupScopes.has(targetId)) {
526+
this.pageCleanupScopes.set(targetId, Scope.makeUnsafe());
527+
}
493528
}
494529

495530
/**
@@ -514,7 +549,16 @@ export class CloudflareStateTracker {
514549
tracker.solvedPages.delete(targetId);
515550
tracker.pendingIframes.delete(targetId);
516551
tracker.pendingRechallengeCount.delete(targetId);
552+
tracker.widgetReloadCount.delete(targetId);
517553
tracker.summaryPhases.delete(targetId);
554+
555+
// Close the page's cleanup scope — atomically fires all finalizers
556+
// that remove this page's entries from solvedCFTargetIds.
557+
const cleanupScope = tracker.pageCleanupScopes.get(targetId);
558+
if (cleanupScope) {
559+
yield* Scope.close(cleanupScope, Exit.void);
560+
tracker.pageCleanupScopes.delete(targetId);
561+
}
518562
})();
519563
}
520564

@@ -554,9 +598,18 @@ export class CloudflareStateTracker {
554598
this.iframeToPage.clear();
555599
this.knownPages.clear();
556600
this.bindingSolvedTargets.clear();
601+
602+
// Close all remaining page cleanup scopes before clearing solvedCFTargetIds.
603+
// Finalizers fire first (deleting entries), then clear() sweeps any stragglers.
604+
for (const scope of this.pageCleanupScopes.values()) {
605+
yield* Scope.close(scope, Exit.void);
606+
}
607+
this.pageCleanupScopes.clear();
608+
557609
this.solvedCFTargetIds.clear();
558610
this.solvedPages.clear();
559611
this.pendingIframes.clear();
612+
this.widgetReloadCount.clear();
560613
this.summaryPhases.clear();
561614
}).bind(this));
562615
}

src/session/cloudflare-solver.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -355,14 +355,14 @@ export class CloudflareSolver {
355355
// it from a different runtime. JS single-threadedness makes this safe.
356356
this.destroyedTargets.add(targetId);
357357

358-
// Record destroyed target — prevents stale OOPIF re-detection if Chrome
359-
// fires targetDestroyed after our detection poll but before cleanup
360-
this.stateTracker.solvedCFTargetIds.add(targetId as unknown as string);
361-
362-
// Check if this target is an OOPIF child of a page detection.
363-
// If so, DON'T kill the parent page's detection fiber — it needs to
364-
// continue to detect navigation/token. Only abort if click was delivered.
358+
// Hoist: determine owning page for scope-based cleanup
365359
const parentCtx = this.stateTracker.registry.findByIframeTarget(targetId);
360+
const owningPageId = parentCtx?.active.pageTargetId ?? targetId;
361+
362+
// Race guard: prevents stale OOPIF re-detection if Chrome fires
363+
// targetDestroyed after our detection poll but before cleanup.
364+
// Scope-bound — entry is removed when owning page is destroyed.
365+
this.stateTracker.addSolvedCFTargetSync(targetId as unknown as string, owningPageId);
366366
if (parentCtx) {
367367
if (parentCtx.oopif && parentCtx.active.clickDelivered) {
368368
// Post-click: close OOPIF scope — finalizer propagates abort

vitest.integration.setup.ts

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,43 @@ async function waitForReady(timeoutMs: number): Promise<void> {
102102
if (await isRunning()) return;
103103
await new Promise((r) => setTimeout(r, POLL_INTERVAL_MS));
104104
}
105-
throw new Error(`Browserless did not start within ${timeoutMs}ms`);
105+
106+
// ── Diagnostic dump on startup failure ─────────────────────────────
107+
// Collect everything needed to diagnose WHY the server didn't start.
108+
// Without this, vitest shows "No test files found" — zero signal.
109+
const diagnostics: string[] = [`Browserless did not start within ${timeoutMs}ms`];
110+
111+
// Server process state
112+
if (serverProcess) {
113+
diagnostics.push(` PID: ${serverProcess.pid ?? 'unknown'}`);
114+
diagnostics.push(` exitCode: ${serverProcess.exitCode}`);
115+
diagnostics.push(` killed: ${serverProcess.killed}`);
116+
diagnostics.push(` signalCode: ${serverProcess.signalCode}`);
117+
}
118+
119+
// Port ownership
120+
try {
121+
const lsofOut = execFileSync('lsof', ['-ti', `:${PORT}`], { encoding: 'utf-8' });
122+
diagnostics.push(` Port ${PORT} owners: ${lsofOut.trim().replace(/\n/g, ', ')}`);
123+
} catch {
124+
diagnostics.push(` Port ${PORT}: no process listening`);
125+
}
126+
127+
// Server log tail — last 20 lines
128+
const logPath = path.join(BROWSERLESS_DIR, 'test-server.log');
129+
if (existsSync(logPath)) {
130+
const log = readFileSync(logPath, 'utf-8');
131+
const lines = log.trim().split('\n');
132+
const tail = lines.slice(-20).join('\n');
133+
diagnostics.push(` Server log (last ${Math.min(20, lines.length)} lines):\n${tail}`);
134+
} else {
135+
diagnostics.push(' Server log: file not found');
136+
}
137+
138+
const message = diagnostics.join('\n');
139+
// Print to stderr so it's visible even if vitest swallows the error
140+
console.error(`\n[globalSetup] STARTUP FAILURE DIAGNOSTICS:\n${message}\n`);
141+
throw new Error(message);
106142
}
107143

108144
let serverProcess: ChildProcess | null = null;
@@ -182,9 +218,20 @@ export async function setup() {
182218
throw new Error(`Failed to spawn browserless: ${err.message}`);
183219
});
184220

185-
serverProcess.on('exit', (code) => {
221+
serverProcess.on('exit', (code, signal) => {
186222
if (code && code !== 0) {
187-
console.error(`[globalSetup] Browserless exited with code ${code}`);
223+
// Dump server log on crash — visible to any agent reading stdout
224+
const logPath = path.join(BROWSERLESS_DIR, 'test-server.log');
225+
let logTail = '';
226+
if (existsSync(logPath)) {
227+
const lines = readFileSync(logPath, 'utf-8').trim().split('\n');
228+
logTail = lines.slice(-10).join('\n');
229+
}
230+
console.error(
231+
`[globalSetup] Browserless crashed!\n` +
232+
` exit code: ${code}, signal: ${signal}\n` +
233+
` Server log (last 10 lines):\n${logTail}`,
234+
);
188235
}
189236
});
190237

0 commit comments

Comments
 (0)