repremium/mokosh
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(debug): race-tolerant offscreen target attach in UAT launch

Browse Source 
Plan-04-04 debug session-2 root cause: the offscreen-console capture
in tests/uat/lib/launch.ts:registerOffscreenConsoleAttach matched zero
offscreen targets across 4 spike runs, creating a critical observability
gap that prevented disambiguation of Plan 04-04 Wave 0 spike failure
mode.

Empirical investigation (tests/uat/spike-diagnose-offscreen-target.ts,
NEW): when chrome.offscreen.createDocument fires, Puppeteer's
`targetcreated` event fires with `type='other'` and `url=''` BEFORE the
CDP target metadata stabilizes. The previous filter (whether
`background_page` or `page`) never matched at event time. By the time
the metadata stabilizes (visible via `browser.targets()`), the
target's type is `'background_page'` (not `'page'` — MV2's
background_page type IS still used by Chrome's CDP for invisible
extension documents, despite MV3 abolishing classic background pages).

Fix:
  - Match the offscreen target by URL pattern (load-bearing criterion;
    type field is intentionally unchecked because it's unreliable at
    targetcreated time).
  - Bind to BOTH `targetcreated` AND `targetchanged` events (the latter
    fires when the URL stabilizes after navigation).
  - Add a `browser.targets()` enumeration race-free safety net for
    cases where the offscreen target exists at registration time.

Verification: tests/uat/spike-diagnose-offscreen-target.ts now emits
`(launch: offscreen console attached — url=chrome-extension://.../src/offscreen/index.html)`
followed by `[off:log] [OS:Recorder] Recording started ...` (zero such
lines in any prior spike run).

Test-infra correctness fix; ZERO production source changes. FORBIDDEN_HOOK_STRINGS
inventory unchanged at 12 entries. No new test-only `__MOKOSH_UAT__` symbols.

References:
  - .planning/debug/sw-offscreen-persistence-investigation-session-2.md
    (session-2 debug note documenting empirical root cause)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Mark
2026-05-22 07:16:27 +02:00
parent d614462694
commit 9ac580869d
2 changed files with 214 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

177
tests/uat/lib/launch.ts
View File

@@ -199,6 +199,36 @@ async function attachSwConsoleBestEffort(
*
* Idempotent — only the first matching offscreen target is attached.
*
* Target.type() discrimination: empirically verified via the
* `spike-diagnose-offscreen-target.ts` helper at debug session-2 time
* (2026-05-21). Despite MV3 abolishing classic MV2 background pages,
* Chrome's CDP STILL reports an extension's offscreen document with
* `targetInfo.type='background_page'` (Puppeteer's TargetType maps this
* to `TargetType.BACKGROUND_PAGE` verbatim per
* node_modules/puppeteer-core/lib/puppeteer/cdp/Target.js:107-108).
* Observed `browser.targets()` enumeration with active offscreen
* recording:
*
* type=background_page url=chrome-extension://{id}/src/offscreen/index.html
*
* Plan-04-04 debug session-2 root cause + fix:
* - The PREVIOUS implementation only registered a `targetcreated`
* listener — but Puppeteer fires `targetcreated` at the moment the
* target is initially created, with `type='other'` and `url=''`,
* BEFORE the CDP target metadata stabilizes. By the time the
* listener checks `target.type()` / `target.url()` they're still
* unset. The filter (regardless of which type it checked) never
* matched, producing 0 `[off:*]` log lines across all spike runs.
* - The new implementation adds an existing-targets enumeration via
* `browser.targets()` AFTER a brief settle — at that point all
* targets have finalized URLs + types and the offscreen is reliably
* discoverable.
* - The `targetcreated` listener is retained as a safety net for
* environments where the offscreen is created LATER (post-launch),
* but it uses the same URL-pattern predicate that works whether or
* not the type field has stabilized — the URL is the load-bearing
* match criterion; the type check is defense-in-depth.
*
* @param browser - Puppeteer browser handle.
* @param extensionId - The resolved extension id.
* @param offConsole - Accumulating string buffer for offscreen log lines.
@@ -209,44 +239,131 @@ function registerOffscreenConsoleAttach(
offConsole: string[],
): void {
let offscreenAttached = false;
/**
* Targetcreated handler — checks each new target for the offscreen
* extension URL pattern, attaches the console listener on the first
* match.
* Check whether a target matches the offscreen-document criteria.
*
* Match predicate: URL includes both the extension id AND the
* canonical offscreen path '/src/offscreen/' (which is
* `chrome.runtime.getURL('src/offscreen/index.html')` per
* src/background/index.ts:239). The URL is the load-bearing match
* criterion; the type field is intentionally NOT checked here
* because:
* - At `targetcreated` time, `type()` returns `'other'` and
* `url()` is empty; the listener may fire before metadata
* stabilizes, so type-based matching at event time is unreliable.
* - At `browser.targets()` enumeration time, the offscreen target's
* type is `'background_page'` (NOT `'page'`; see function docstring
* above for the empirical evidence).
* - Either way, the URL pattern is unique to the offscreen document
* and uncollidable with the harness page (whose URL ends with
* `/tests/uat/extension-page-harness.html`), the welcome page
* (`/src/welcome/welcome.html`), the victim page (`file://...`),
* or the SW (whose URL ends with `/service-worker-loader.js`).
*
* Both `url.includes` checks are needed:
* - url-includes-extension-id rules out the harness victim page
* (a file:// URL with no extension id);
* - url-includes-'/src/offscreen/' rules out every other
* extension-page realm (harness, welcome, popup).
*/
const onTargetCreated = async (
target: { type: () => string; url: () => string; asPage: () => Promise<Page> },
const isOffscreenTarget = (
target: { type: () => string; url: () => string },
): boolean => {
const url = target.url();
return url.includes(extensionId) && url.includes('/src/offscreen/');
};
/**
* Attach the console listener to a confirmed offscreen page target.
* Idempotent via the closed-over `offscreenAttached` flag.
*/
const attachToOffscreen = async (
target: { asPage: () => Promise<Page>; url: () => string },
): Promise<void> => {
if (offscreenAttached) {
return;
}
const url = target.url();
if (
target.type() === 'background_page' &&
url.includes(extensionId) &&
url.includes('offscreen')
) {
offscreenAttached = true;
try {
const offPage = await target.asPage();
/**
* Per-message callback — same tag format as the SW attach
* (`[off:<type>] <text>`).
*/
const onOffConsole = (msg: { type: () => string; text: () => string }): void => {
const line = `[off:${msg.type()}] ${msg.text()}`;
offConsole.push(line);
process.stderr.write(line + '\n');
};
offPage.on('console', onOffConsole);
} catch (offAttachErr) {
process.stderr.write(
`(launch: offscreen console attach skipped — ${String(offAttachErr)})\n`,
);
}
offscreenAttached = true;
try {
const offPage = await target.asPage();
/**
* Per-message callback — same tag format as the SW attach
* (`[off:<type>] <text>`).
*/
const onOffConsole = (msg: { type: () => string; text: () => string }): void => {
const line = `[off:${msg.type()}] ${msg.text()}`;
offConsole.push(line);
process.stderr.write(line + '\n');
};
offPage.on('console', onOffConsole);
process.stderr.write(
`(launch: offscreen console attached — url=${target.url()})\n`,
);
} catch (offAttachErr) {
process.stderr.write(
`(launch: offscreen console attach skipped — ${String(offAttachErr)})\n`,
);
}
};
browser.on('targetcreated', onTargetCreated);
/**
* Generic target handler — checks any target for the offscreen
* extension URL pattern, attaches the console listener on the first
* match. Idempotent via the closed-over `offscreenAttached` flag.
*
* Bound to BOTH `targetcreated` and `targetchanged` events:
* - `targetcreated` fires on initial target creation (but at that
* instant the target's url/type may not be stable — see function
* docstring above). We still listen here for the case where the
* target's metadata is ready synchronously.
* - `targetchanged` fires when the target's URL changes — which is
* the canonical signal that the offscreen document has finished
* navigating to its `chrome.runtime.getURL('src/offscreen/index.html')`
* URL after `chrome.offscreen.createDocument`.
* Either event firing the matching predicate triggers attach;
* subsequent firings are idempotent.
*/
const onTargetEvent = async (
target: { type: () => string; url: () => string; asPage: () => Promise<Page> },
): Promise<void> => {
if (isOffscreenTarget(target)) {
await attachToOffscreen(target);
}
};
browser.on('targetcreated', onTargetEvent);
browser.on('targetchanged', onTargetEvent);
// Race-condition guard: enumerate already-existing targets in case the
// offscreen was created BEFORE this listener was registered. Puppeteer
// only fires `targetcreated` for FUTURE targets — already-created ones
// are silent unless explicitly enumerated. In the harness flow the
// offscreen is created via chrome.offscreen.createDocument inside an
// assertion call (after this function returns), so the listener path
// is typically sufficient — but probe latency or pre-launch state can
// mean the offscreen target exists by the time we register. The check
// below is best-effort + idempotent (the `offscreenAttached` flag
// prevents double-attach if both code paths fire).
//
// Plan-04-04 debug session-2 fix: the prior implementation relied
// entirely on `targetcreated` and missed the offscreen in every spike
// run (zero `[off:*]` lines), creating the observability gap.
const enumerateExistingTargets = async (): Promise<void> => {
try {
const existing = browser.targets();
for (const target of existing) {
if (isOffscreenTarget(target)) {
await attachToOffscreen(target);
break;
}
}
} catch (enumErr) {
process.stderr.write(
`(launch: offscreen target enumeration skipped — ${String(enumErr)})\n`,
);
}
};
void enumerateExistingTargets();
}
/**