fix(agent-readiness): WebMCP uses registerTool + static import (#3316) (#3361)

* fix(agent-readiness): WebMCP uses registerTool + static import (#3316)

isitagentready.com reported "No WebMCP tools detected on page load"
on prod. Two compounding bugs in PR #3356:

1) API shape mismatch. Deployed code calls
   navigator.modelContext.provideContext({ tools }), but the scanner
   SKILL and shipping Chrome implementation use
   navigator.modelContext.registerTool(tool, { signal }) per tool with
   AbortController-driven teardown. The older provideContext form is
   kept as a fallback.

2) Dynamic-import timing. The webmcp module was lazy-loaded from a
   deep init phase, so the chunk resolved after the scanner probe
   window elapsed.

Fix:

- Rewrite registerWebMcpTools to prefer registerTool with an
  AbortController. provideContext becomes a legacy fallback. Returns
  the AbortController so teardown paths exist.
- Static-import webmcp in App.ts and call registerWebMcpTools
  synchronously at the start of init, before any await. Bindings
  close over lazy refs so throw-on-null guards still fire correctly
  when a tool is invoked later.

Test additions lock in registerTool-precedes-provideContext ordering,
AbortController pattern, static import, and call-before-first-await.

* fix(agent-readiness): WebMCP readiness wait + teardown on destroy (#3316)

Addresses three findings on PR #3361.

P1 — startup race. Early registration is required for scanner probes,
but a tool invoked during the window between register and Phase-4 UI
init threw "Search modal is not initialised yet." Both scanners and
agents that probe-and-invoke hit this. Bindings now await a uiReady
promise that resolves after searchManager.init and countryIntel.init.
A 10s timeout keeps a broken init from hanging the caller. After
readiness, a still-null target is a real failure and still throws.

Mechanics: App constructor builds uiReady as a Promise with its
resolve stored on the instance; Phase-4 end calls resolveUiReady;
waitForUiReady races uiReady against a timeout; both bindings await it.

P2 — AbortController was returned and dropped. registerWebMcpTools
returns a controller so callers can unregister on teardown, but App
discarded it. Stored on App now and aborted in destroy, so test
harnesses and SPA re-inits don't accumulate stale registrations.

P2 — test coverage. Added assertions for: bindings await
waitForUiReady before accessing state; resolveUiReady fires after
countryIntel.init; waitForUiReady uses Promise.race with a timeout;
destroy aborts the stored controller. Kept silent-success guard
assertions so bindings still throw when state is absent post-readiness.

Tests: 16 webmcp, 6682 full suite, all green.

* test(webmcp): tighten init()/destroy() regex anchoring (#3316)

Addresses P2 from PR #3361 review. The init() and destroy() body
captures used lazy `[\s\S]+?\n  }` which stops at the first
2-space-indent close brace. An intermediate `}` inside init (e.g.
some exotic scope block) would truncate the slice; the downstream
`.split(/\n\s+await\s/)` would then operate on a smaller string and
could let a refactor slip by without tripping the assertion.

Both regexes now end with a lookahead for the next class member
(`\n\n  (?:public|private) `), so the capture spans the whole method
body regardless of internal braces. If the next-member anchor ever
breaks, the match returns null and the `assert.ok` guard fails
loudly instead of silently accepting a short capture.

P1 (AbortController silently dropped) was already addressed in
f3bbd2170 — `this.webMcpController` is stored and destroy() aborts
it. Greptile reviewed the first push.

tests/webmcp.test.mjs

@@ -15,18 +15,29 @@ const WEBMCP_PATH = resolve(ROOT, 'src/services/webmcp.ts');
 const src = readFileSync(WEBMCP_PATH, 'utf-8');
 
 describe('webmcp.ts: draft-spec contract', () => {
-  it('feature-detects navigator.modelContext before calling provideContext', () => {
-    // The detection gate must run before any call. If a future refactor
-    // inverts the order, this regex stops matching and fails.
+  it('prefers registerTool (Chrome-implemented form) over provideContext (legacy)', () => {
+    // isitagentready.com scans for navigator.modelContext.registerTool calls.
+    // The registerTool branch must come first; provideContext is a legacy
+    // fallback. If a future refactor inverts order, the scanner will miss us.
+    const registerIdx = src.search(/typeof provider\.registerTool === 'function'/);
+    const provideIdx = src.search(/typeof provider\.provideContext === 'function'/);
+    assert.ok(registerIdx >= 0, 'registerTool branch missing');
+    assert.ok(provideIdx >= 0, 'provideContext fallback missing');
+    assert.ok(
+      registerIdx < provideIdx,
+      'registerTool must be checked before provideContext (Chrome-impl form is the primary target)',
+    );
+  });
+
+  it('uses AbortController for registerTool teardown (draft-spec pattern)', () => {
     assert.match(
       src,
-      /typeof provider\.provideContext !== 'function'\) return false[\s\S]+?provider\.provideContext\(/,
-      'feature detection must short-circuit before provideContext is invoked',
+      /const controller = new AbortController\(\)[\s\S]+?provider\.registerTool\(tool, \{ signal: controller\.signal \}\)/,
     );
   });
 
   it('guards against non-browser runtimes (navigator undefined)', () => {
-    assert.match(src, /typeof navigator === 'undefined'\) return false/);
+    assert.match(src, /typeof navigator === 'undefined'\) return null/);
   });
 
   it('ships at least two tools (acceptance criterion: >=2 tools)', () => {
@@ -76,31 +87,116 @@ describe('webmcp.ts: tool behaviour (source-level invariants)', () => {
   });
 });
 
-// App.ts wiring — guards against silent-success bugs where a binding
-// forwards to a nullable UI target whose no-op the tool then falsely
-// reports as success. Bindings MUST throw when the target is absent
-// so withInvocationLogging's catch path can return isError:true.
-describe('webmcp App.ts binding: guard against silent success', () => {
+// App.ts wiring — guards against two classes of bug:
+//   (1) Silent success when a binding forwards to a nullable UI target.
+//   (2) Startup race when a tool is invoked during the window between
+//       early registration (needed for scanners) and Phase-4 UI init.
+// Bindings await a readiness signal before touching UI state and fall
+// through to a throw if the signal never resolves; withInvocationLogging
+// converts that throw into isError:true.
+describe('webmcp App.ts binding: readiness + teardown', () => {
   const appSrc = readFileSync(resolve(ROOT, 'src/App.ts'), 'utf-8');
   const bindingBlock = appSrc.match(
-    /registerWebMcpTools\(\{[\s\S]+?\}\);\s*\}\);/,
+    /registerWebMcpTools\(\{[\s\S]+?\}\);/,
   );
 
   it('the WebMCP binding block exists in App.ts init', () => {
     assert.ok(bindingBlock, 'could not locate registerWebMcpTools(...) in App.ts');
   });
 
-  it('openSearch binding throws when searchModal is absent', () => {
+  it('is imported statically (not via dynamic import)', () => {
+    // Scanner timing: dynamic import defers registration past the probe
+    // window. A static import lets the synchronous call at init-start run
+    // before any await in init(), catching the first scanner probe.
+    assert.match(
+      appSrc,
+      /^import \{ registerWebMcpTools \} from '@\/services\/webmcp';$/m,
+      'registerWebMcpTools must be imported statically',
+    );
+    assert.doesNotMatch(
+      appSrc,
+      /import\(['"]@\/services\/webmcp['"]\)/,
+      "no dynamic import('@/services/webmcp') — defers past scanner probe window",
+    );
+  });
+
+  it('is called before the first await in init()', () => {
+    // Anchor the end of the capture to the NEXT class-level member
+    // (public/private) so an intermediate 2-space-indent `}` inside
+    // init() can't truncate the body. A lazy `[\s\S]+?\n  }` match
+    // would stop at the first such closing brace and silently shrink
+    // the slice we search for the pre-await pattern.
+    const initBody = appSrc.match(
+      /public async init\(\): Promise<void> \{([\s\S]*?)\n  \}(?=\n\n  (?:public|private) )/,
+    );
+    assert.ok(initBody, 'could not locate init() body (anchor to next class member missing)');
+    const preAwait = initBody[1].split(/\n\s+await\s/, 2)[0];
+    assert.match(
+      preAwait,
+      /registerWebMcpTools\(/,
+      'registerWebMcpTools must be invoked before the first await in init()',
+    );
+  });
+
+  it('both bindings await the UI-readiness signal before touching state', () => {
+    // Before-fix regression: openSearch threw immediately on first
+    // invocation during startup. Both bindings must wait for Phase-4
+    // UI init to complete, then check the state, then dispatch.
+    assert.match(
+      bindingBlock[0],
+      /openSearch:[\s\S]+?await this\.waitForUiReady\(\)[\s\S]+?this\.state\.searchModal/,
+      'openSearch must await waitForUiReady() before accessing searchModal',
+    );
+    assert.match(
+      bindingBlock[0],
+      /openCountryBriefByCode:[\s\S]+?await this\.waitForUiReady\(\)[\s\S]+?this\.state\.countryBriefPage/,
+      'openCountryBriefByCode must await waitForUiReady() before accessing countryBriefPage',
+    );
+  });
+
+  it('bindings still throw (not silently succeed) when state is absent after readiness', () => {
+    // The silent-success guard from PR #3356 review must survive the
+    // readiness refactor. After awaiting readiness, a missing target is
+    // a real failure — throw so withInvocationLogging returns isError.
     assert.match(
       bindingBlock[0],
       /openSearch:[\s\S]+?if \(!this\.state\.searchModal\)[\s\S]+?throw new Error/,
     );
-  });
-
-  it('openCountryBriefByCode binding throws when countryBriefPage is absent', () => {
     assert.match(
       bindingBlock[0],
       /openCountryBriefByCode:[\s\S]+?if \(!this\.state\.countryBriefPage\)[\s\S]+?throw new Error/,
     );
   });
+
+  it('uiReady is resolved after Phase-4 UI modules initialise', () => {
+    // waitForUiReady() hangs forever if nothing ever resolves uiReady.
+    // The resolve must live right after countryIntel.init() so that all
+    // Phase-4 modules are ready by the time waiters unblock.
+    assert.match(
+      appSrc,
+      /this\.countryIntel\.init\(\);[\s\S]{0,200}this\.resolveUiReady\(\)/,
+      'resolveUiReady() must fire after countryIntel.init() in Phase 4',
+    );
+  });
+
+  it('waitForUiReady enforces a timeout so a broken init cannot hang the agent', () => {
+    assert.match(
+      appSrc,
+      /private async waitForUiReady\(timeoutMs = [\d_]+\)[\s\S]+?Promise\.race\(\[this\.uiReady/,
+    );
+  });
+
+  it('destroy() aborts the WebMCP controller so re-inits do not duplicate registrations', () => {
+    // Same anchoring as init() — end at the next class member so an
+    // intermediate 2-space-indent close brace can't truncate the capture.
+    const destroyBody = appSrc.match(
+      /public destroy\(\): void \{([\s\S]*?)\n  \}(?=\n\n  (?:public|private) )/,
+    );
+    assert.ok(destroyBody, 'could not locate destroy() body (anchor to next class member missing)');
+    assert.match(
+      destroyBody[1],
+      /this\.webMcpController\?\.abort\(\)/,
+      'destroy() must abort the stored WebMCP AbortController',
+    );
+  });
 });