fix(telemetry): R5 review fixups — Vertex destination + ["*"] trim + docs

Self-review pass on commit 1c8528a56 (host-scoped session-id header):

1. **Vertex AI destination guessing**
   (geminiContentGenerator/index.ts)
   `@google/genai` routes to `{region}-aiplatform.googleapis.com` (not
   `generativelanguage.googleapis.com`) when `vertexai: true` and no
   `baseUrl`. The previous "guess generativelanguage" default would have
   mis-bucketed Vertex traffic under any operator-supplied allowlist that
   covered the public Gemini endpoint but not the Vertex one. Today
   invisible (both off the default allowlist), but a latent gotcha for
   operators tuning `telemetry.sessionIdHeaderHosts`.
   Fix: pass `undefined` when `config.baseUrl` is unset (fail-closed —
   no header). Operators who want correlation against Google endpoints
   must set `baseUrl` explicitly, which is also the SDK's input for
   destination resolution.

2. **`["*"]` broadcast escape hatch tolerates whitespace**
   (llm-correlation-fetch.ts)
   `[" * "]` (a settings.json hand-edit with a stray space) previously
   silently fell back to "no host matches" — the opposite of operator
   intent. Now `.trim()` before comparing, so common whitespace mistakes
   still trigger broadcast.

3. **Doc note on wrap-time allowlist snapshot**
   (llm-correlation-fetch.ts JSDoc)
   The session id is read live per-request, but `trustedHosts` is
   snapshotted once at `wrapFetchWithCorrelation` call time. Spell this
   out in the JSDoc so a future maintainer doesn't read the live
   `getSessionId()` and assume the allowlist is the same shape.

4. **Defensive test coverage**
   (trusted-llm-hosts.test.ts, llm-correlation-fetch.test.ts)
   Added: extractRequestHost with explicit port / userinfo / query /
   fragment / IPv6 bracket form. Whitespace `[" * "]` broadcast test.
   IPv6 case documents the "bracketed → never matches" behavior is
   intentional fail-closed for the named-host allowlist scope.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

packages/core/src/core/geminiContentGenerator/index.test.ts

@@ -161,17 +161,21 @@ describe('createGeminiContentGenerator', () => {
     );
   });
 
-  it('includes X-Qwen-Code-Session-Id when allowlist override covers googleapis.com', () => {
+  it('omits X-Qwen-Code-Session-Id when baseUrl is unset, even with allowlist override (factory fail-closed for unknown destination)', () => {
-    // Operator opts back in for Google's endpoint via settings override.
+    // The factory only passes a destinationUrl to staticCorrelationHeaders
+    // when `config.baseUrl` is set, because the Gemini SDK has two
+    // invisible defaults (generativelanguage.googleapis.com for public,
+    // {region}-aiplatform.googleapis.com for Vertex). Guessing one would
+    // mis-bucket the other under a googleapis-covering allowlist override.
+    // Operators who want correlation against Google endpoints must set
+    // `baseUrl` explicitly.
     mockConfig = {
       getUsageStatisticsEnabled: vi.fn().mockReturnValue(false),
       getContentGeneratorConfig: vi.fn().mockReturnValue({}),
       getCliVersion: vi.fn().mockReturnValue('1.0.0'),
       getTelemetryEnabled: vi.fn().mockReturnValue(true),
       getSessionId: vi.fn().mockReturnValue('sess-gemini'),
-      getTelemetrySessionIdHeaderHosts: vi
+      getTelemetrySessionIdHeaderHosts: vi.fn().mockReturnValue(['*']),
-        .fn()
-        .mockReturnValue(['*.googleapis.com']),
     } as unknown as Config;
     const config = {
       model: 'gemini-1.5-flash',
@@ -182,8 +186,8 @@ describe('createGeminiContentGenerator', () => {
     const callArgs = vi.mocked(GeminiContentGenerator).mock.calls[0]?.[0] as {
       httpOptions: { headers: Record<string, string> };
     };
-    expect(callArgs.httpOptions.headers['X-Qwen-Code-Session-Id']).toBe(
+    expect(callArgs.httpOptions.headers).not.toHaveProperty(
-      'sess-gemini',
+      'X-Qwen-Code-Session-Id',
     );
   });
 });

packages/core/src/core/geminiContentGenerator/index.ts

@@ -46,18 +46,19 @@ export function createGeminiContentGenerator(
   // SDK's cached headers retain the OLD session id until the contentGenerator
   // is recreated. See design doc §8.6 + #4384 follow-up sub-issue tracking.
   //
-  // Destination passed in is what the host-allowlist check uses. With the
+  // Destination resolution: only pass when we KNOW the destination — i.e.
-  // default `DEFAULT_SESSION_ID_HEADER_HOSTS` (Alibaba/DashScope-only),
+  // when the operator has explicitly set `config.baseUrl`. The SDK has two
-  // Google's default endpoint `generativelanguage.googleapis.com` is NOT
+  // different invisible defaults (`generativelanguage.googleapis.com` for
-  // on the list, so the header is naturally omitted for vanilla Gemini API
+  // public Gemini, `{region}-aiplatform.googleapis.com` for Vertex AI) and
-  // calls — matching the "first-party only" scope. Operators who deliberately
+  // guessing wrong here would mis-bucket Vertex traffic if the operator
-  // want correlation against a Google endpoint can add it via
+  // overrides `telemetry.sessionIdHeaderHosts` to include any googleapis
-  // `telemetry.sessionIdHeaderHosts` in settings.
+  // domain. Passing `undefined` makes `staticCorrelationHeaders` return
-  const destinationUrl =
+  // `{}` (no header) — the fail-closed default. Operators who want
-    config.baseUrl ?? 'https://generativelanguage.googleapis.com';
+  // correlation against a Google endpoint set `baseUrl` explicitly
+  // (which is the same input the SDK uses to resolve the destination).
   headers = {
     ...headers,
-    ...staticCorrelationHeaders(gcConfig, destinationUrl),
+    ...staticCorrelationHeaders(gcConfig, config.baseUrl),
   };
   const httpOptions = config.baseUrl
     ? {

packages/core/src/telemetry/llm-correlation-fetch.test.ts

@@ -498,6 +498,25 @@ describe('wrapFetchWithCorrelation — host allowlist gating', () => {
     );
   });
 
+  it('tolerates whitespace around "*" so [" * "] still enables broadcast', async () => {
+    // Defensive: settings.json hand-edits are easy to get a stray space
+    // wrong. Without trim() the operator would silently get "no host
+    // matches" instead of broadcast.
+    const m = makeFetchMock();
+    const wrapped = wrapFetchWithCorrelation(
+      m.fetch,
+      mockConfig({
+        enabled: true,
+        sessionId: 'sess-A',
+        hosts: [' * '],
+      }),
+    );
+    await wrapped('https://api.openai.com/v1/chat/completions');
+    expect((m.lastInit()?.headers as Headers).get(SESSION_ID_HEADER)).toBe(
+      'sess-A',
+    );
+  });
+
   it('respects [] override to fully disable injection', async () => {
     const m = makeFetchMock();
     const wrapped = wrapFetchWithCorrelation(

packages/core/src/telemetry/llm-correlation-fetch.ts

@@ -70,6 +70,15 @@ type FetchLikeLoose = (
  * Reading `config.getSessionId()` from inside the wrapper on every call
  * gives the live value.
  *
+ * Note on `trustedHosts`: snapshotted once at wrap time, not read per
+ * request. The session id is live-read but the allowlist is not — a
+ * mid-session change to `telemetry.sessionIdHeaderHosts` in settings.json
+ * takes effect at next content-generator init (any change that mutates
+ * Config snapshot the wrapper retains is by definition a Config-level
+ * concern, not a request-time concern). Operators tuning the scope live
+ * should restart, or call the openai/anthropic clients via a fresh
+ * provider after settings reload.
+ *
  * The caller is responsible for choosing the base fetch — usually
  * `runtimeOptions?.fetch ?? globalThis.fetch` so proxy-aware fetch (set up
  * by `buildRuntimeFetchOptions`) is preserved when ProxyAgent is in use.
@@ -102,7 +111,9 @@ export function wrapFetchWithCorrelation<TFetch extends FetchLikeLoose>(
   // Wildcard escape hatch so operators who want the old broadcast
   // behavior can opt in via `["*"]` without us extending the pattern
   // grammar in `matchesTrustedHost` (which would tempt other globbing).
-  const broadcastAll = trustedHosts.some((p) => p === '*');
+  // `.trim()` so `[" * "]` (whitespace from settings.json hand-edit) still
+  // triggers broadcast rather than silently degrading to "no host matches".
+  const broadcastAll = trustedHosts.some((p) => p.trim() === '*');
 
   const wrapped: FetchLikeLoose = async function correlationFetch(
     input: string | URL | Request,
@@ -185,7 +196,7 @@ export function staticCorrelationHeaders(
     const trustedHosts =
       config.getTelemetrySessionIdHeaderHosts?.() ??
       DEFAULT_SESSION_ID_HEADER_HOSTS;
-    const broadcastAll = trustedHosts.some((p) => p === '*');
+    const broadcastAll = trustedHosts.some((p) => p.trim() === '*');
     if (!broadcastAll) {
       let host: string;
       try {

packages/core/src/telemetry/trusted-llm-hosts.test.ts

@@ -99,6 +99,35 @@ describe('extractRequestHost', () => {
     ).toBe('api.anthropic.com');
   });
 
+  it('strips explicit port (URL.hostname does not include port)', () => {
+    expect(extractRequestHost('https://dashscope.aliyuncs.com:443/v1')).toBe(
+      'dashscope.aliyuncs.com',
+    );
+  });
+
+  it('strips userinfo (URL.hostname does not include user/password)', () => {
+    expect(
+      extractRequestHost('https://user:pw@dashscope.aliyuncs.com/v1'),
+    ).toBe('dashscope.aliyuncs.com');
+  });
+
+  it('ignores query string and fragment', () => {
+    expect(
+      extractRequestHost('https://dashscope.aliyuncs.com/v1?key=secret#frag'),
+    ).toBe('dashscope.aliyuncs.com');
+  });
+
+  it('returns bracketed form for IPv6 (will never match a string pattern, behaves as fail-closed)', () => {
+    // Document the IPv6 behavior so it's intentional rather than a
+    // surprise: URL.hostname returns `[::1]` (with brackets), our
+    // pattern grammar has no `[…]` form, so IPv6 destinations are
+    // effectively never on the allowlist. This is acceptable because
+    // qwen-code's allowlist is for named first-party endpoints, not
+    // raw IPs. If/when raw-IP correlation becomes a real ask, extend
+    // matchesTrustedHost — don't change extractRequestHost.
+    expect(extractRequestHost('http://[::1]:8080/x')).toBe('[::1]');
+  });
+
   it('returns undefined for unparseable string', () => {
     expect(extractRequestHost('not a url')).toBeUndefined();
   });