feat/shelly

safeviewing older commit

6c2d095 · fullpre-PRreviewed 2026-07-07 21:09 UTC0H · 0M · 2L · 7I

The branch

Purpose

Lay the shared foundation seams for porting 12 legacy device integrations (hoymiles, shelly-cloud, solis, huawei-portal, solark, fronius, apsystems, abb, victron, enphase, sma, powerradar) into the v2 metrics engine so per-brand branches can implement their workers independently.

Goal

Provision manifests, catalog seeds, CDK path contracts, ARN-seed registry entries, and D2/D3 migration scripts — everything a brand branch needs pre-declared before it writes a single line of worker code.

Sub-goals

SG-1: 12 device-integration manifests in packages/integration-manifests
SG-2: catalog seed rows (Make / Integration / MetricType) for all 12 brands
SG-3: ARN-seed REGISTRY entries for all 12 brands (with --only guard)
SG-4: CDK paths.ts handler pre-declarations for all 12 vendor directories
SG-5: D3 legacy-registry migration script + pure mapping core + tests
SG-6: D2 legacy-credential copy script + SecretProvider widening + tests
SG-N: retire stale manifest co-location docs; update INTEGRATION_STANDARDS.md and services/metrics/integrations/CLAUDE.md

The changes (whole branch)

What: 12 new IntegrationManifest objects (pure data); widened SecretProvider union + DB enum (TS-only, no migration); additive Make/Integration/MetricType seeds; CDK paths.ts vendor path pre-declarations; D2 copy-legacy-secrets script + D3 migrate-legacy-registry script with supporting pure-core libraries; comprehensive coherence tests for all 12 manifests; updated porting docs.
Why: The legacy mgmt-account registry holds 12 device brands whose data plane work was deferred. Provisioning the shared seams on a single branch prevents M brand branches from duplicating (and diverging on) catalog rows, CDK path naming, and ARN conventions.
Areas: packages/integration-manifests+1039−6packages/database+257−14infra/cdk/src/lib/paths.ts+185−0scripts/metrics+1182−0domains/core/src/secret-configuration+16−1services/metrics/integrations/docs+38−6domains/metrics/src/metric-source/__tests__+5−4
Blast: 35 files, +2727/−187. All changes are additive (new manifests, new seed rows, new scripts, new CDK path entries). No handler logic, no DB migrations, no schema deletions. Brand branches are unblocked.

Migration scripts are dry-run by default — --apply gate must be explicit ARN-seed REGISTRY now includes undeployed brands — always scope with --only SMA has a non-standard legacy secret path — requires --sma-oid flag

➖ci· No PR open — no CI rollup available➖coderabbit· No .coderabbit.yaml in repo

Findings · 9

correctness3

low

paths.ts comment claims 'explore' mirrors REGISTRY jobTypes, but REGISTRY uses 'exploration'

infra/cdk/src/lib/paths.ts:234

The comment at line 234 reads "Handler keys mirror the ARN-seed REGISTRY jobTypes … `metrics`/`connection`/`explore`", but `seed-integration-arns.ts` consistently uses `exploration` as the job type key (growatt, solis, huawei). The paths.ts TypeScript keys (`explore`) and the REGISTRY job type keys (`exploration`) serve different purposes and don't need to match, but the comment implies they do. A brand-branch author reading paths.ts first will infer the wrong jobType key for the DB seed — they should use `exploration`, not `explore`.

info

drizzle cast bypasses Database type in emitDbRows

scripts/metrics/copy-legacy-secrets.ts:186

`drizzle(connection) as unknown as Database` is a forced cast needed because drizzle's inferred return type doesn't match the project's Database alias. This is a known pattern in scripts and poses no runtime risk today, but would silently miss any future Database type constraints. Acceptable for a migration script.

info

TS-only widening assertion for secret_configurations provider enum is correct

packages/database/src/schema/secret-configurations.ts:65

Comment correctly notes Drizzle's `text()` with `{ enum }` does NOT generate a PostgreSQL CHECK constraint — the widening is truly migration-free. Verified against Drizzle behaviour.

security2

low

actorId: null loses audit attribution for bulk-migrated IntegrationSecrets

scripts/metrics/copy-legacy-secrets.ts:259

The `createIntegrationSecretShell` call passes `actorId: null, actorType: 'system'`. This is consistent with other system-initiated shells but means IntegrationSecret rows created via D2 will have no human actor in the audit trail. Acceptable for a one-time migration script — worth noting if compliance/audit requirements surface later.

info

Secret values held only in-memory between SM calls — design is sound

scripts/metrics/copy-legacy-secrets.ts

GetSecretValue → CreateSecret/PutSecretValue copies the SecretString in-process with no intermediate logging, disk write, or env var. Reports deal in names/ARNs only. The security model matches the intent documented in the file header.

conventions1

info

Manifest JSDoc blocks are verbose but content is genuinely non-obvious

packages/integration-manifests/src/manifests/sma.ts

Several manifest files (ABB, APsystems, Enphase, SMA) contain multi-paragraph JSDoc blocks that exceed the project's 'one short line max' comment convention. The content captures genuinely non-obvious vendor constraints (legacy channel maps, secret naming deviations, auth-cache patterns). Given these are integration manifests where the WHY is opaque without vendor docs, the convention's intent is satisfied even if the form is verbose. Low priority to trim.

tests2

info

device-integrations.test.ts coherence suite is thorough

packages/integration-manifests/src/__tests__/device-integrations.test.ts

The 9-test coherence suite enforces: exact 12 manifest IDs, registry registration, requiresAsset/supportedMakes shape, requiresAuth↔credentialFields parity, canonical variable key → metricTypeVariable match, externalVariableId spot-checks against prod registry snapshot, SMA battery+load channels, PowerRadar dynamic-variables path. This is an unusually complete structural lock for a foundation seam.

info

egauge.test.ts granularity update correctly matches already-updated manifest

packages/integration-manifests/src/__tests__/egauge.test.ts:27

Test updated from expecting `['5m']` to `['5m', '15m']`. Confirmed the egauge manifest is already at `['5m', '15m']` on all channels — the test was stale and this is a correct fix, not a test-ahead-of-code situation.

improvement1

low

--brands CLI filter operates on integrationId but is named 'brands' — potential operator confusion

scripts/metrics/copy-legacy-secrets.ts:284

The `--brands` flag is documented as 'filter by manifest id (new integration code)', meaning operators must pass `--brands=shelly-cloud`, NOT `--brands=shelly`. The parameter name `brands` suggests hardware brand names. CLAUDE.md covers this but the CLI usage example in the file header only shows `[--brands=victron,sma]` (which happen to be unambiguous). Adding `[--brands=shelly-cloud]` to the example would prevent a common operator mistake.

History · 2 commits

4350d25blockedfull2H · 11M · 7L2026-07-08 02:36
6c2d095safefull0H · 0M · 2L2026-07-07 21:09current