[AGI-550] - Tag tracking events with the library even on a customer's cioClient

[joesurf]

Summary

When a customer passes their own cioClient, our tracking events used to carry that client's c tag (e.g. ciojs-client-2.94.0). Those events then looked like a custom UI, so we couldn't tell which customers really use this library. This is the ASA version of Constructor-io/constructorio-ui-pia#177.

• Our tracking events always send c=cio-ui-asa-<version>. useAsaTracking sends through a view of the customer's tracker that changes only version. This covers every ASA event: submit, result load started/finished, result click/view, search submit and agent button click.
• Everything else still comes from the customer's client: API key, clientId, sessionId, userId, segments, test cells, service URL, request queue and event listeners. These are read when each event is sent, so later setClientOptions calls (e.g. login) still apply.
• The customer's own events keep their tag. Only the events this library sends are retagged, and their client isn't modified. A customer who also sends ASA events from their own code (e.g. Catalyst) still shows up as both library and custom.
• Only tracking changes. Agent API requests still go through the customer's client unchanged.
• The cio-ui-asa-<version> string is now one constant, LIBRARY_TAG, shared with useCioClient.

[constructor-claude-bedrock]

Code Review

This PR introduces withLibraryTag to ensure ASA tracking events always carry cio-ui-asa-<version> as the c tag, even when a customer supplies their own cioClient — a clean, well-tested solution with solid fallback guards.

Inline comments: 5 discussions added

Overall Assessment: ⚠️ Needs Work

[constructor-claude-bedrock]

Important Issue: The fallback retry silently swallows all errors, not just the narrowly-documented "method only works on the real instance" case. If a tracking call fails for any other reason (network error surfaced synchronously, bad arguments, a bug in a future ciojs version), the retry on the bare target will silently re-attempt it and also swallow that failure. Callers lose all visibility.

The PR description says "a failing call is retried on the customer's tracker" and lists this as a virtue, but it also says "the fallback retry could send an event twice if a future ciojs threw after queuing it". That double-fire risk is real. Consider only retrying on a specific, recognisable error (e.g. instanceof TypeError with a message matching a private-field access) and re-throwing everything else, or at least logging in non-production environments:

wrapped = (...args) => {
  try {
    return method.apply(wrapper, args);
  } catch (e) {
    // Only retry binding errors; everything else re-throws so callers aren't silenced.
    if (!(e instanceof TypeError)) throw e;
    return method.apply(target, args);
  }
};

[constructor-claude-bedrock]

Suggestion: canTag checks Object.isExtensible to guard against frozen objects, but the configurable !== false check on version's descriptor only inspects whether the property is currently non-configurable — it does not guard against the property becoming non-configurable after withLibraryTag is called (e.g. via Object.defineProperty later). The comment on isLocked notes "A proxy may only report a value other than the target's while that property could still change", but canTag is only called once at wrap-time for the initial options object, and then re-called inside currentOptions via !canTag(options) each time an event fires. This is fine for the options object itself, but worth a short comment explaining that canTag is intentionally re-evaluated per-send to catch post-wrap mutations.

[constructor-claude-bedrock]

Suggestion: The early-return check (initial.version === LIBRARY_TAG) uses the value at wrap time. This is correct for the library's own clients (constructed with version: LIBRARY_TAG), but the identity check is based on string equality rather than object identity. If a host somehow sets version to the same string as LIBRARY_TAG (highly unlikely, but possible), their tracker would be returned unwrapped. This is almost certainly fine, but a short comment noting "already tagged by us or matches our tag" would make the intent clearer for future maintainers.

[constructor-claude-bedrock]

Suggestion: This test covers the canTag(initial) guard, but there's no test for the case where options exists but version has a non-configurable descriptor (the configurable === false branch of canTag). That branch is exercised by an internal ciojs implementation detail; adding a test for it would protect against a future ciojs change hardening the descriptor silently removing tagging without a test failure. Something like:

it('returns a tracker with a non-configurable version property unchanged', () => {
  const tracker = { options: Object.defineProperty({}, 'version', {
    value: 'ciojs-client-host',
    configurable: false,
    writable: false,
    enumerable: true,
  }) };
  expect(withLibraryTag(tracker)).toBe(tracker);
});

[constructor-claude-bedrock]

Suggestion: withLibraryTag returns the tracker unchanged when it's already tagged (or can't be tagged), so the useMemo is always safe. However, the memoisation only keyed on hostTracker means that if a host replaces the options object on their tracker after mount (as tested in spec/libraryTag.test.ts: 'follows an options object the host replaces'), the useMemo won't re-run — which is fine because withLibraryTag's proxy already handles that dynamically.

This is correct behaviour, but the comment only says "Tagged even on a host-supplied client". A brief note that withLibraryTag is stable/idempotent — so the memo key need not include options — would save a future reader from questioning the missing options dependency.