feat(telemetry): record external-datastore-configured flag per invocation

[keeb]

Summary

• Adds externalDatastoreConfigured: boolean to InvocationContext / InvocationContextData, populated at telemetry-service init.
• True only when the repo marker declares a datastore whose type is not filesystem — custom providers like S3. Unset markers and local filesystem datastores both report false.
• Logic lives in a small isExternalDatastoreConfigured(datastore) helper next to buildInvocationContext, with unit coverage for the three cases.
• Wire shape lands at properties.invocationContext.externalDatastoreConfigured — http_telemetry_sender_test locks the contract so the swamp-club consumer can query it without a server-side change.

Test Plan

• deno fmt --check
• deno lint
• deno check
• Targeted: deno run test src/domain/telemetry/... src/cli/telemetry_integration_test.ts src/infrastructure/persistence/json_telemetry_repository_test.ts src/infrastructure/telemetry/http_telemetry_sender_test.ts — 102 passed

[github-actions]

Code Review

Clean, well-scoped PR that adds a single boolean telemetry signal for whether the repo uses a non-filesystem datastore. The implementation follows established patterns throughout.

Blocking Issues

None.

Suggestions

1. The isExternalDatastoreConfigured helper is a pure domain predicate (operates on DatastoreConfigData, no CLI/infra dependencies). It could live closer to the domain — e.g., next to isCustomDatastoreConfig in src/domain/datastore/datastore_config.ts — for discoverability and reuse. Not blocking since it's only used at the telemetry-integration boundary today and the placement is consistent with how buildInvocationContext composes other signals.

Looks good — types are consistent across the InvocationContext/InvocationContextData interfaces, the wire-shape contract test locks the new field for the consumer, all three predicate cases (undefined, filesystem, custom) are covered, and the marker.datastore call site types align correctly. No import-boundary or security concerns.

[github-actions]

Adversarial Review

Critical / High

None found.

Medium

1. Backward compatibility on deserialization (src/domain/telemetry/invocation_context.ts:75): When invocationContextFromData is called on JSON written by the previous version (which has invocationContext present but no externalDatastoreConfigured key), props.externalDatastoreConfigured is undefined at runtime despite the boolean type. The value undefined is stored as-is and is falsy, so downstream checks like if (ctx.externalDatastoreConfigured) work correctly. However, a strict === false comparison would fail. This matches the precedent set by isInteractive (same pattern, no default fallback), and since the field is only consumed server-side via the wire-shape test (which always sends true/false), the risk is low. A defensive ?? false in createInvocationContext would harden this:

   externalDatastoreConfigured: props.externalDatastoreConfigured ?? false,

Low

1. Empty-string type from malformed YAML (src/cli/telemetry_integration.ts:41): If a .swamp.yaml has datastore: { type: "" }, isExternalDatastoreConfigured returns true because "" !== "filesystem". This is arguably correct (an empty type isn't "filesystem"), but it could produce a misleading telemetry signal. Very unlikely in practice since the marker is normally validated upstream.

Verdict

PASS — Clean, well-tested additive change. The new boolean flag is constructed atomically at the call site, round-trip tested through all persistence layers, and the wire-shape contract test locks the downstream schema. The backward-compat note (Medium #1) is a hardening suggestion, not a blocker — the existing isInteractive field has the same exposure and has shipped without issues.