fix(discovery): honour type_source='manual' in the UniFi reconcile path (#3011)

[ToddHebebrand]

Closes #3011

Root cause

reconcileDiscoveredAsset in apps/api/src/services/unifi/unifiSyncService.ts wrote asset_type unconditionally on both the UPDATE branch and the ON CONFLICT DO UPDATE branch, and never consulted type_source.

The manual write was never lost — PATCH /discovery/assets/:id correctly stamped type_source='manual', and the next UniFi sync overwrote asset_type on top of it. That is exactly the reporter's evidence: type_source still reads manual, asset_type has reverted, and last_seen_at / unifi_devices.updated_at bump together on the revert.

The agent-scan path (discoveryWorker.processResults) already implements this precedence. The UniFi path was never given it — the original 2026-06-28-discovered-asset-type-source.sql design doc has no UniFi coverage at all.

unifiTelemetryService (the agent-collector path) never writes asset_type, so it was not a contributor. This is the only write site, verified by grep across services/unifi/, jobs/unifi*.ts and routes/unifi/.

The fix

1. A manual type outranks telemetry classification — enforced in SQL on both write paths.

asset_type = case when "discovered_assets"."type_source" = 'manual'
                  then "discovered_assets"."asset_type"
                  else <proposed> end

<proposed> is a bind parameter on the UPDATE path and excluded.asset_type on the conflict path. The guard is deliberately not a JS decision: on the conflict path we never read the colliding row, and on the update path type_source can change between the SELECT and the UPDATE — a user saving a manual type mid-sync is precisely the race this issue is about. type_source is written only on the INSERT side, so a collision can never reset an existing row to 'auto'.

2. An unclassified device no longer writes either type column.

assetType() only maps UniFi network gear (usw/uap/ugw/udm/ufg) and returns 'unknown' for everything else — including the G6 180, G4 Doorbell Pro and G3 Pro named in the issue. That return means "this sync has no opinion", not "the device is of unknown type", so stamping it back would erase a classification made by another source. This is why the reporter saw unknown specifically.

This is slightly beyond the literal ask — flagging it explicitly. It is required for the correctness of the change itself: without it, the newly-added detected_asset_type write would clobber a better detection from the agent-scan path. It does not implement camera auto-identification, which stays with #2199.

3. detected_asset_type is now recorded whenever the sync does recognise a device, mirroring discoveryWorker. Previously a UniFi-created asset had it NULL, so "reset to auto" (coalesce(detected_asset_type, asset_type)) had nothing to restore.

Supporting hardening from review: write sets are typed PgUpdateSetSource<typeof discoveredAssets> (drizzle silently drops set keys that don't name a column, and DbExecutor is deliberately loose — a mis-keyed column previously compiled, passed the mocked tests, and no-opped at runtime), and excluded.asset_type is derived from the column definition so a rename cannot desync the CASE.

Relationship to #2199

#2199 notes a "manual overrides still win" requirement. This PR establishes that precedence rule in the UniFi path so #2199 builds on it rather than reimplementing it.

Tenancy

No schema or migration change — type_source and detected_asset_type already exist (2026-06-28-discovered-asset-type-source.sql). discovered_assets is already registered in CORE_ORG_CASCADE_DELETE_ORDER, CORE_DEVICE_CASCADE_DELETE_TABLES and CORE_TENANT_EXPORT_POLICY (both columns already classified included), and is RLS shape 1 (direct org_id, auto-discovered). No registration work applies.

The UniFi ingest runs under withSystemDbAccessContext, so RLS is bypassed on this path — the override guard is necessarily application-level, which is what this PR adds.

Verification

Real Postgres — apps/api/src/__tests__/integration/unifiAssetTypeSource.integration.test.ts (new, 5 cases). A mock cannot verify what makes this work: that a table-qualified reference inside DO UPDATE SET reads the pre-update row, that the bind parameter unifies with the discovered_asset_type enum, that excluded resolves, that the (org_id, ip_address) arbiter is inferred. Covers manual-preserved and auto-applied on both the UPDATE path and the ON CONFLICT path (the latter driven through a simulated race that makes the row invisible to the lookups but present for the arbiter), plus the unclassified-device case. 4 of the 5 fail against origin/main; the fifth is a positive control.

Unit — 6 new cases in unifiSyncService.test.ts. The guard assertions render through PgDialect rather than flattening sql template chunks; the earlier chunk-flattening version passed an inverted guard, and both mutants (inverted branches, wrong guard column) now fail. Conflict arbiter and the mac-less IP-fallback path are covered.

Suites — vitest run src/services/unifi src/jobs/unifiTelemetryWorker.test.ts src/routes/unifi src/routes/discovery.test.ts src/jobs/discoveryWorker.test.ts → 11 files, 186 passed. Integration: unifiAssetTypeSource + unifiCollectorUpsert → 7 passed. tsc --noEmit clean.

Known follow-ups (not in this PR)

• discoveryWorker still stamps 'unknown' and, worse, inferAssetTypeFromVendor maps ubiquiti → access_point while this sync sets manufacturer: 'Ubiquiti' on every asset it touches — so an agent scan can rewrite a correctly-classified UniFi switch to access_point and flap detected_asset_type between runs. Pre-existing and orthogonal to [Discovery] Manual Asset Type is overwritten on the next UniFi ingest — type_source='manual' is not honoured by the reconcile path #3011 (that path does honour type_source='manual'), but the "no opinion" semantics is only half-adopted across the two writers.
• "Reset to auto" is a visible no-op for unclassified devices (detected_asset_type stays NULL, coalesce falls back to the current value). Pre-existing; naturally resolved by [Discovery] Auto-identify cameras and UniFi devices (hostname heuristics, OUI coverage, UniFi Protect sync) #2199 adding camera mappings.
• No observability for unrecognised UniFi device-type strings. If UniFi renames a type, classification silently stops updating. Worth pairing with [Discovery] Auto-identify cameras and UniFi devices (hostname heuristics, OUI coverage, UniFi Protect sync) #2199, which will shrink the unmapped set to something genuinely anomalous and therefore loggable without spamming every camera on every sync.
• discovered_assets.updated_at is not bumped by the UniFi sync (no $onUpdate, not in the write set), unlike discoveryWorker. Pre-existing.

🤖 Generated with Claude Code

[cloudflare-workers-and-pages]

Deploying breeze with    Cloudflare Pages

Latest commit:	95f85be
Status:	✅  Deploy successful!
Preview URL:	https://d20a286a.breeze-9te.pages.dev
Branch Preview URL:	https://fix-3011-unifi-manual-asset.breeze-9te.pages.dev

View logs

[ToddHebebrand]

Review run: /pr-review-toolkit:review-pr — code-reviewer, pr-test-analyzer, silent-failure-hunter (type-design-analyzer skipped: no new types in the original diff).

Findings: 5 raised as real and consequential → all addressed in 072dc8f; 0 outstanding. 4 further findings triaged as pre-existing/out-of-scope and recorded as follow-ups in the PR body rather than fixed here.

Addressed:

• TOCTOU on the UPDATE branch. The guard read type_source in a SELECT and decided in JS, so a manual save landing between the SELECT and the UPDATE was still clobbered — the same bug in a narrower window. Both branches now share one SQL expression evaluated at write time; the lookups no longer project type_source.
• Record<string, unknown> write sets. Drizzle iterates the table's columns and silently drops set keys that don't name one, and DbExecutor is deliberately loose — a mis-keyed column compiled, passed the mocked tests and no-opped at runtime. Now PgUpdateSetSource<typeof discoveredAssets>; verified it rejects a deliberate typo.
• The guard assertion was theatre. It flattened the sql template's literal chunks, which drops every column reference — an inverted guard rendered byte-identical and passed. Assertions now render through PgDialect; both mutants (inverted branches, wrong guard column) fail.
• No real-DB coverage of the conflict branch. Added unifiAssetTypeSource.integration.test.ts — arbiter inference, excluded resolution and enum/bind unification only exist inside the planner.
• Uncovered paths: conflict arbiter and the mac-less IP-fallback lookup now asserted.

Triaged as follow-ups (detail in the PR body): discoveryWorker still stamps 'unknown' and can flap a UniFi switch to access_point via the Ubiquiti OUI heuristic; "reset to auto" is a no-op for unclassified devices; no observability for unrecognised UniFi type strings; updated_at not bumped by this sync. All pre-existing and orthogonal to #3011.

Tests: tsc --noEmit clean. Unit — 11 files / 186 passed (src/services/unifi, src/jobs/unifiTelemetryWorker.test.ts, src/routes/unifi, src/routes/discovery.test.ts, src/jobs/discoveryWorker.test.ts), single-fork. Integration — unifiAssetTypeSource + unifiCollectorUpsert → 7 passed against real Postgres on a worktree-private stack. Non-vacuity checked both ways: 4 of the 5 integration cases and 6 of the unit cases fail against origin/main.

Status: review-clean, awaiting maintainer merge. Not merging and not closing #3011 — the reporter should confirm the manual type sticks across a sync first.