feat(ai-visibility): dual-mode Semrush credential provider + PR-3a hardening (inert)

[cwjwisse]

1. Abstract

Makes the flag-gated per-brand Semrush auth seam (shipped inert in #3064) a seamless drop-in for the eventual S2S wiring, without turning it on. Adds a decision-independent dual-mode credential provider and lands the three PR-3a hardening items deferred from that review. Still inert by default — no behavior change in any environment.

2. Reasoning

The seam from #3064 is provider-less and flag-off. The upstream technical-account (TA) decision is still contested along two axes (spec §6.2), and the S2S path itself is blocked (LLMO-6836). Rather than wait, this PR pre-builds both contested shapes behind config so the eventual decision is a config flip, not a rewrite, and closes the correctness gaps the #3064 review flagged as required-before-flip. Everything remains gated: no provider is registered, the flag defaults off, and the shared client_credentials path is byte-identical to today.

3. High-level overview of the changes

Part A — dual-mode credential provider (semrush-credential-provider.js, new)

• createDualModeSemrushCredentialProvider({ config, resolveOrgId, resolveRequestWorkspace, lookupWorkspaceForOrg, mintToken }) returns a provider with the exact (brand, env) => SemrushCredential | null shape the resolver expects.
• Implements both contested knobs (spec §6.2) so landing the decision is a config flip:
  • Knob A — credential granularity: SHARED (one TA) vs PER_ORG (one TA per org). Controls only the credential scope/cache key.
  • Knob B — workspace selection: REQUEST (workspaceId from the request) vs TOKEN_ORG_MAPPING (ims_org_id → workspace via an injected lookup). Controls only the descriptor's workspaceHint.
• Default-to-code: PER_ORG + TOKEN_ORG_MAPPING (spec §6.2 — most likely to survive a security review).
• The org resolver, request-workspace resolver, ims_org_id → workspace mapping (candidate source: serenity-docs INDEX.md), and token minter are all injected seams. No brand/org/workspace value is hardcoded; the built-in extractors only read fields off the passed context.
• Registers nothing — it is an available implementation, installed only by a future PR (3b) via setSemrushCredentialProvider.

Part B — hardening deferred from the #3064 (PR-3a) review (semrush-credential-resolver.js, grpc-transport.js)

1. In-flight token coalescing: getCachedToken now shares a single outstanding mint across concurrent cold/expired callers for one key (in-flight Promise map), instead of each minting its own token. Directly addresses the thundering-herd suggestion in the feat(ai-visibility): flag-gated per-brand Semrush auth seam skeleton (inert) #3064 review.
2. Transport teardown on eviction: the per-credential client pool now owns an Http2SessionManager per entry and abort()s it on eviction/reset, so the evicted transport's HTTP/2 session is closed rather than leaked.
3. TTL skew margin: getCachedToken refreshes a real (provider-supplied expiry) token a few seconds before its true expiry, avoiding a mid-flight 401. The conservative default TTL is unaffected, and a real TTL shorter than the skew keeps its own expiry (never born-stale).

getGrpcClients returns the same client set as before; the pool entry now internally carries its session manager, and the external return shape is unchanged.

4. Required information

• Jira / issue: LLMO-7029 (PR-3b preparation; upstream blocker LLMO-6836)
• Spec / design of record: adobe/mysticat-architecture#248 — products/llmo/spec-brand-claims-semrush-ingestion-feed.md §6.2 / §6.3 (the two pluggable knobs; default-to-code)
• Predecessor: feat(ai-visibility): flag-gated per-brand Semrush auth seam skeleton (inert) #3064 (inert seam skeleton)

Related Issues

• LLMO-7029 (this PR), LLMO-6836 (upstream S2S blocker — gates the live flip), spec adobe/mysticat-architecture#248 §6.3.

7. Test plan

(a) Local unit tests:

• test/support/ai-visibility/semrush-credential-provider.test.js (new): inert-by-default (importing/building the provider registers nothing — resolver still returns null); config validation; default mode (per-org key + org→workspace mapping); Knob A (shared key, no org needed); Knob B (request workspaceId vs mapping); both contested shapes from one seam set via config alone; getAuthToken mints for the resolved scope; no hardcoded mapping; and an end-to-end install into the resolver seam that shares one mint per credential key via getCachedToken.
• test/support/ai-visibility/semrush-credential-resolver.test.js: added B1 coalescing (single mint shared across concurrent callers; shared failure caches nothing) and B3 skew (early refresh for real tokens; default TTL unskewed; short-TTL guard).
• test/support/ai-visibility/grpc-transport-per-brand.test.js: added B2 teardown assertions (evicted entry and reset both abort() the HTTP/2 session) and mocked Http2SessionManager; the flag-off byte-identical path is unchanged.
• Commands + results are in the PR conversation; npm run lint, npm run type-check, and the full npm test suite pass.

(b) Per environment: nothing to verify — the flag defaults off everywhere, no provider is registered, and the shared client_credentials path is byte-identical. Existing GET /llmo/ai-visibility/* responses are unchanged. Enabling AI_VISIBILITY_PER_BRAND_AUTH_ENABLED with a provider installed is the first live check, and that lands in PR-3b once the contested TA decision (Knob A + Knob B) and LLMO-6836 resolve.

8. Deployment & merge order

• Seam remains OFF / inert. No behavior change. The live flip is future work (PR-3b) and is gated on LLMO-6836 (upstream S2S) plus the Knob A / Knob B decision.
• adobe/mysticat-architecture#248 — related (design of record, §6.2/§6.3); docs-only, no deploy coupling.
• Stacks on feat(ai-visibility): flag-gated per-brand Semrush auth seam skeleton (inert) #3064 (already merged to main).

🤖 Generated with Claude Code

[github-actions]

This PR will trigger a minor release when merged.

[codecov]

Codecov Report

❌ Patch coverage is 98.31933% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...pport/ai-visibility/semrush-credential-provider.js	98.23%	4 Missing ⚠️
...pport/ai-visibility/semrush-credential-resolver.js	97.67%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[MysticatBot]

Hey @cwjwisse,

Verdict: Request changes - two hardening gaps undermine the resilience this PR is building.
Complexity: HIGH - large diff (805 lines across 6 files).
Changes: Adds a dual-mode Semrush credential provider implementing both contested auth knobs behind config, plus three hardening items (in-flight token coalescing, HTTP/2 session teardown on eviction, TTL skew margin) to the flag-gated per-brand auth seam (6 files).

Must fix before merge

1. [Important] Stale nowMs used for expiry calculation after mint delay - src/support/ai-visibility/semrush-credential-resolver.js:160 (details inline)
2. [Important] teardownClientPoolEntry can throw in forEach and prevent remaining entries from being torn down - src/support/ai-visibility/grpc-transport.js:149 (details inline)

Non-blocking (4): minor issues and suggestions

• nit: Provider returns null on two distinct early-exit paths (missing org vs unmapped workspace) with no way for callers to distinguish the failure mode - src/support/ai-visibility/semrush-credential-provider.js:110 - consider a debug-level log or typed result when this goes live.
• nit: TOKEN_TTL_SKEW_MS = 5000 assumes gRPC calls complete within 5s but the assumption is not documented relative to transport timeout - src/support/ai-visibility/semrush-credential-resolver.js:56
• suggestion: Add a test verifying TTL skew applies to the expiresAtMs (absolute) path, not just the expiresInMs (relative) path - the branch sets providerSuppliedExpiry = true identically but has no dedicated skew assertion.
• suggestion: Add a test verifying resetGrpcClients calls abort() on the flag-OFF singleton cachedEntry - this is new production code on the default live path with no dedicated coverage.

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 2m 57s | Cost: $7.49 | Commit: 812bf358fa9420f82d73736703c263615a360de5
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

issue (blocking): Stale nowMs undermines the TTL skew this PR introduces.

nowMs is captured at function entry (line 134) but reused inside the mint promise (here and line 153) after await mintFn(). If the mint call takes significant time (network round-trip to an S2S token endpoint), nowMs is stale by the time the token is cached, and the effective TTL is longer than intended.

For a 5-minute default TTL this is benign, but for a provider-supplied short-lived token (say 60s), a 3s mint latency means the cache overestimates remaining life by 3s - directly eroding the 5s skew margin this PR adds to prevent mid-flight 401s.

Fix: Capture Date.now() fresh inside the IIFE after the mint resolves:

const mintedAt = Date.now();
// then use mintedAt for relative expiry:
expiresAtMs = mintedAt + minted.expiresInMs;

Keep the outer nowMs for the cache-hit check at the top.

[MysticatBot]

issue (blocking): Unguarded abort() call can break the teardown loop.

teardownClientPoolEntry is called inside perCredentialEntries.forEach(teardownClientPoolEntry) in resetGrpcClients. If Http2SessionManager.abort() throws (e.g. the session is already in an error state, or a future library version changes behavior), the exception propagates out of the forEach and remaining entries are never torn down - leaking the connections this fix was designed to close.

Fix: Wrap the abort in a try/catch:

function teardownClientPoolEntry(entry) {
  try {
    entry?.sessionManager?.abort?.();
  } catch { /* session already closed */ }
}