feat/apsystems

splitDeviceId splits on first dash — valid but undocumented invariant (sids must be dash-free)

services/metrics/integrations/apsystems/src/handlers/metrics.lambda.ts:105

The comment says 'sids contain no dashes' — a legacy assumption about EMA system IDs being numeric. If a future device ever has a dash in sid, `indexOf('-')` would split incorrectly, silently producing wrong sid/eid values. The invariant should be asserted or validated against the manifest rather than assumed. Not a current bug.

`split('?')[0] ?? pathWithQuery` — nullish coalescing is unreachable dead code

services/metrics/integrations/apsystems/src/engine/ema-client.ts:100

`String.prototype.split()` always returns at least one element; `[0]` is never undefined. The `?? pathWithQuery` branch is unreachable. Harmless but adds noise.

IAM wildcard covers all orgs' APsystems secrets — no org-id scoping in ARN path

infra/cdk/src/stacks/services/metrics/integrations/apsystems/iam.ts:42

The policy grants GetSecretValue on `{env}/apsystems/*` and `batu/{env}/metrics/apsystems/*`. Neither pattern includes an org-id segment, so the Lambda can read ANY org's APsystems credential under those prefixes. This is the same design as eGauge, Growatt, and Helioscope (shared architectural decision, not new to this PR). The trust boundary rests on: (1) the coordinator building org-scoped `secretConfigArn` from a DB query filtered by orgId, and (2) the coordinator being the sole authorized invoker of the worker Lambda. The handler does not re-validate that `inv.secretConfigArn` belongs to `inv.orgPublicId` — a defense-in-depth gap that a crafted invocation could exploit if Lambda:InvokeFunction is ever widened. Recommend opening a backlog item to add an ARN prefix guard in the handler (shared fix across all integrations).

date_range query parameters not URL-encoded — defense-in-depth gap

services/metrics/integrations/apsystems/src/engine/ema-client.ts:177

`localDate` (YYYY-MM-DD) and `yearMonth` (YYYY-MM) are appended to the query string without `encodeURIComponent`. Values are always derived from Intl.DateTimeFormat so they contain only digits and hyphens (safe). Inconsistent with `encodeURIComponent` already applied to `sid`/`eid` path segments. No current attack vector; worth aligning for consistency.

Outer catch block includes `e.message` in returned InvocationError — may surface internal paths

services/metrics/integrations/apsystems/src/handlers/metrics.lambda.ts:293

The catch-all `e.message` is included in the `UpstreamUnavailable` error returned to the SFN state output and persisted in `metric_collection_jobs`. Node.js Error.message can contain internal Lambda file paths (ENOENT, etc.). Credentials cannot appear here (loadCredentialFromArn catches internally and returns null). Consider replacing with a fixed string for unknown errors.

HMAC nonce entropy, TLS validation, and credential non-logging are all correct

`crypto.randomBytes(16).toString('hex')` = 128 bits of CSPRNG entropy — adequate. Node.js fetch verifies TLS by default (no rejectUnauthorized override). BASE_URL is a hardcoded constant (no SSRF path). `loadCredentialFromArn` explicitly guards against logging the parsed JSON payload. No issues.

Coordinator enforces org-scoped secretConfigArn before passing to worker — normal SFN path is safe

The site-context-aggregator queries `integration_secrets JOIN secret_configurations WHERE orgId = site.orgId` before embedding `secretConfigArn` into the Invocation. The security boundary is the coordinator being the sole authorized invoker. The medium finding above is a defense-in-depth gap, not an active exploit on the normal path.

Unsupported granularity hard-fails entire invocation — violates §9 resilient skip semantics

services/metrics/integrations/apsystems/src/handlers/metrics.lambda.ts:172

INTEGRATION_STANDARDS.md §9 and CLAUDE.md §Multi-granularity porting rule #3 both say: 'Return a graceful unsupported-granularity SKIP (never throw/abort) for cadences your vendor doesn't serve.' The pre-flight guard at line 172-175 does `return failure(...)` on the WHOLE invocation when any channel carries an unsupported granularity (e.g. '1M'). If an operator ever provisions a `1M` deep-history source alongside a `5m` source, the entire collection aborts rather than skipping the unsupported grain and collecting the supported one. eGauge reference impl treats this as a per-channel skip (TranslationFailed in channelNotes), fails only when payloads.length===0. This pattern must be consistent before APsystems is used as a template for SG-7 (monthly granularity) or any other port.

Bespoke `wants()` predicate instead of `groupChannelsByGranularity` — undocumented deviation from standard

services/metrics/integrations/apsystems/src/handlers/metrics.lambda.ts:181

INTEGRATION_STANDARDS.md §9 and CLAUDE.md §Porting note 1 both prescribe `groupChannelsByGranularity` from `@batu/metrics-engine`, with eGauge as the reference impl. APsystems uses a bespoke `wants(granularity, isMeter)` Boolean predicate to split channels by both granularity AND endpoint type (ECU vs meter). The deviation is functionally justified (EMA uses distinct endpoints per measurement type, not just per granularity), but it is undocumented in the file. Future porters comparing to APsystems will see a structurally different handler without understanding why. Either collapse into `groupChannelsByGranularity` with post-hoc endpoint routing per channel, or add a block comment explaining the endpoint-type split invariant and why the standard utility doesn't apply here.

No descriptor.ts — correctly omitted, but INTEGRATION_STANDARDS.md §8 checklist hasn't been updated

APsystems correctly omits `descriptor.ts` (routing is DB-driven; descriptors are dead/legacy per metrics-pipeline.md). But INTEGRATION_STANDARDS.md §8 still lists it as required for new integrations. A documentation divergence, not a code problem — future ports will correctly skip the descriptor but the standards doc will mislead them.

No handler test for `auth-failed` / `rate-limited` transport error → correct InvocationError `_tag`

services/metrics/integrations/apsystems/src/__tests__/metrics-handler.test.ts:333

INTEGRATION_STANDARDS.md §9 rule 4: 'On total failure, surface the REAL transport error tag so the SFN retry/alerting policy can discriminate.' `mapFetchError` is tested in isolation (line 333-343) and `UpstreamTimeout` is tested via a mid-window-abort scenario (line 261-274). But there is no handler-level test asserting that a `fetchEcuMinutely` failure with `reason:'auth-failed'` surfaces as `_tag:'UpstreamAuthFailed'` in the `InvocationFailure`, or `reason:'rate-limited'` as `UpstreamRateLimited`. The mapping is only validated at the unit level, not through the handler path.

No multi-channel 'total silence' test (all channels skip for different reasons)

services/metrics/integrations/apsystems/src/__tests__/metrics-handler.test.ts:241

The dead-device test (line 241) covers ECU returning all-1001 → zero points → TranslationFailed. There is no test where MULTIPLE channels are provisioned, each skips for a different reason (cadence mismatch on solar + meterless 1001 on demand/import/export), producing a `channelNotes` array with multiple distinct entries in the final TranslationFailed message. This multi-reason failure format is tested at the unit level for single channels but not through the full handler integration.

Tinybird fixture lacks APsystems-representative rows for end-to-end SiteMetrics seam tests

infra/tinybird/fixtures/metric.ndjson

The 10 fixture rows use generic source IDs (src_a, src_b, src_c, src_m). No row is APsystems-sourced (5-min points at 300s intervals, W values from ×12000 formula, Mexico City wall-clock timezone). End-to-end SiteMetrics seam tests consuming APsystems data have no fixture baseline. Not blocking unit/handler tests.

Dangling cross-references to `hoymiles` and `shelly` files that don't exist in the repo

services/metrics/integrations/apsystems/src/handlers/metrics.lambda.ts:80

Handler comments cite `shelly L#4` (line 80), `hoymiles L#13` (line 309), `hoymiles L#14` (line 165); translation cites `hoymiles L#12` (points-to-batu.ts:211), `shelly L#7` (points-to-batu.ts:117). Neither hoymiles nor shelly integration packages exist in `services/metrics/integrations/`. A porter reading 'mirrors hoymiles L#14' cannot verify the referenced code. These rationales should be stated inline or attributed to INTEGRATION_STANDARDS.md §9 / CLAUDE.md learnings rather than to non-existent sibling files.

Timezone helpers block is 'Copied verbatim from the victron port' — unresolved duplication

services/metrics/integrations/apsystems/src/translation/points-to-batu.ts:60

Lines 60-62 explicitly say 'Copied verbatim from the victron port (feat/victron translation/points-to-batu.ts)'. Victron doesn't exist in the repo yet. When victron, hoymiles, and shelly land (all serve local-wall-clock labels requiring the same DST-correct two-pass fixed-point), this 80-line block will be duplicated N times. The appropriate home is `@batu/metrics-engine/src/lib/` alongside `groupChannelsByGranularity`. Leaving the 'copied verbatim' comment without a dedup plan means DST edge-case fixes must be applied in N places. Open a Linear item or add a TODO with the extraction target.

`computeTouBreakdown` is a pure function in a `.shells.ts` file — altitude violation

domains/cross-domain/src/site-energy-metrics.shells.ts:280

`computeTouBreakdown` is pure (no I/O, no async) and should live in `.decisions.ts` per canonical-form.md. It's in the shells file because it's tested via a direct import (`site-energy-metrics-breakdown.test.ts`). Extracting to `site-energy-metrics.decisions.ts` would fix the altitude violation without changing behavior. Not blocking, but it will confuse porters who read shells expecting I/O orchestration.

1M isMonthly branch in resolveSiteEnergyMetricsShell should be a named helper

domains/cross-domain/src/site-energy-metrics.shells.ts:173

The 15-line inline block that detects `isMonthly`, selects `MONTHLY_IDENTITY_INTERVAL`, and assembles the `calendarMonthly` InputSeries will need to be replicated when other coordinators read monthly streams. Extracting `toInputSeries(stream, points, timezone): InputSeries` (pure, ~8 lines) would make the loop body declarative and the 1M contract easier to audit. The `MONTHLY_IDENTITY_INTERVAL` constant is already a step toward this.