# Issue 839: Multi-picklist Canonicalization

## Problem

Issue 839 reports that multi-select picklist writes can persist as a
JSON-string-of-JSON-array instead of the platform's intended storage shape.
Example:

- client sends `["A","B"]` as a string, or a prior surface forwards that value
- current `NormalizeSObject` treats every string as semicolon-delimited text
- storage becomes `["[\"A\",\"B\"]"]`
- SOQL and formula/validation surfaces then see a wrapper value instead of the
  selected values

The fix must be generic for every `multipicklist` field. It must not branch on
Opportunity, CRM objects, pack names, or tenant-specific field names.
The Salesforce metadata spelling `MultiselectPicklist` normalizes to the same
field type as NexusCRM's canonical `multipicklist` spelling.

Review follow-up: metadata validation allows multi-picklist option values that
look like JSON arrays or JSON string scalars, such as `["A","B"]` or
`"Closed"`. A generic L0 canonicalizer must not treat those legal option
values as encoded envelopes and rewrite user data into different selections.

## Current Paths

Writes:

- REST `PATCH /services/data/v63.0/sobjects/<Object>/<Id>` calls
  `SObjectHandler.UpdateRecord`.
- The handler validates field existence and delegates to
  `PersistenceService.Update`.
- `prepareUpdateRequest`, before-save flow re-normalization, insert,
  prepared updates, and bulk CSV normalization all call
  `persistence.NormalizeSObject`.
- `NormalizeSObject` is therefore the correct write-side SSoT for generic
  field-type coercion.

Reads:

- Single-record reads use `RecordRepository.normalizeRecord`, which calls
  `persistence.FromStorageRecord`.
- SOQL reads use `QueryService.hydrateVirtualFields`, which also calls
  `persistence.FromStorageRecord`.
- Therefore `FromStorageRecord` is the correct read-side SSoT for turning
  storage values into the public wire value.

Repair:

- New writes can be fixed by `NormalizeSObject`.
- Existing rows need a bounded repair path because semantically equal rewrites
  can be treated as no-ops by the records diffing logic.
- Example: stored `["[\"A\",\"B\"]"]` and new PATCH value `"A;B"` represent
  the same selected set, so the normal field-diff check may skip the physical
  update. Repair must not depend on users changing the selection.
- `setup repair` already owns one-time compatibility fixes, is serialized by
  the control-plane lock, and is invoked by `setup apply`.

## Canonical Contract

Storage contract:

- A multi-select picklist field is stored as a JSON array string in its TiDB
  JSON column, for example `["A","B"]`.
- Empty string normalizes to `nil`; an explicit empty array storage value
  remains `[]` so an already-normalized write is idempotent.
- Given the same selected items in the same first-seen order, all accepted input
  shapes produce byte-identical storage.
- Repair closure treats database JSON serialization whitespace as equivalent to
  that canonical storage, because TiDB/MySQL may read `["A","B"]` back as
  `["A", "B"]`.
- Accepted write inputs are:
  - scalar string: `"A"` -> `["A"]`
  - semicolon string: `"A; B"` -> `["A","B"]`
  - JSON array string: `"[\"A\",\"B\"]"` -> `["A","B"]` only when static
    `Options.Value` metadata proves every decoded value is a declared option
    and the exact scalar string is not itself a declared option value
  - array values: `[]string{"A","B"}` or `[]interface{}{"A","B"}` ->
    `["A","B"]`
  - legacy double wrapper: `["[\"A\",\"B\"]"]` -> `["A","B"]` only when static
    `Options.Value` metadata proves the nested values are declared selections

Disambiguation rules:

- Write-side scalar strings are checked against the field's declared
  `Options.Value` set before JSON compatibility decoding. If a string equals a
  declared option value, it is stored as one selected item.
- A valid JSON-array string from an external scalar decodes into selected items
  only when static `Options.Value` metadata can prove the decoded items are
  selections. Optionless and runtime-scoped multipicklists are freeform at this
  layer, so scalar strings such as `["A","B"]` remain one literal selected
  value unless the caller supplies an actual array or an in-process provenance
  marker.
- Already-normalized in-process values use the `NormalizedMultiPicklistJSON`
  provenance marker to remain idempotent without making external REST strings
  ambiguous.
- Storage-side strings are treated as storage envelopes when they are valid JSON
  arrays. Option matching applies to decoded array items, not the whole storage
  string, because canonical storage for two selections `A` and `B` is itself
  `["A","B"]`.
- Storage repair unwraps a nested JSON-looking item only when static options
  prove it is a legacy wrapper. Without that proof, a stored value like
  `["[\"A\",\"B\"]"]` may be one legal literal option and is left untouched.
- JSON string scalars are never unquoted by the multipicklist helper. A value
  like `"Closed"` is data and stores as `["\"Closed\""]`.
- The repair migration reads `_System_Field.Options` and preserves canonical
  single-option storage such as `["[\"A\",\"B\"]"]` when `["A","B"]` is a
  declared option value.

Wire/read contract:

- `FromStorageRecord` returns Salesforce-compatible semicolon text for
  selected values, for example `A;B`.
- Existing double wrappers read as `A;B`, not `["A","B"]`, when static options
  can disambiguate them from one literal JSON-looking option.
- Malformed values do not silently become empty. If a string starts with `[` but
  is not a valid JSON array, it falls back to semicolon splitting so bad storage
  remains observable.

Normalization rules:

- Trim whitespace around item values.
- Drop blank items.
- De-duplicate while preserving first-seen order. Multi-picklist selections are
  a set; stable first-seen order avoids surprising UI reorder while still
  converging duplicate payloads.
- Keep this at the field-type layer. Do not validate active picklist option
  membership in this PR; that belongs to existing/future metadata validators.

## Implementation Plan

1. Add multi-picklist helpers in `backend/internal/infrastructure/persistence`
   and use them from `NormalizeSObject` and `FromStorageRecord`.
2. Add a one-time `setup repair` migration that:
   - discovers `multipicklist` fields from `_System_Object` and `_System_Field`
   - reads `_System_Field.Options` for metadata-aware disambiguation
   - uses `_System_Field.PhysicalName` when present, with `ApiName` as the
     fallback column name
   - validates object and column identifiers before dynamic SQL
   - scans only those columns
   - rewrites rows whose stored value normalizes to a different canonical JSON
     array string
   - performs a second scan for closure before recording the
     `_System_Migration` stamp
3. Wire the repair into `controlplane.RunRepair`, which also covers
   `controlplane.ApplyAll`.
4. Add focused tests for:
   - scalar, semicolon, array, JSON-array-string, blank, and double-wrapper
     write normalization
   - double-wrapper read normalization
   - repair discovery and update behavior
   - stamp guard idempotency

## Boundary Check

This is L0 substrate work. The implementation keys only on
`FieldTypeMultiPicklist`, metadata-declared object and field identifiers, and
generic record storage. It does not encode CRM object names, tenant custom
fields, picklist values, profiles, record types, or approval process names.

## Confidence Gates

Implementation is ready only when all of these are true:

- A write of `"A;B"`, `["A","B"]`, and `"[\"A\",\"B\"]"` produces the same
  byte-identical storage shape.
- A repair scan over TiDB/MySQL formatted JSON arrays such as `["A", "B"]`
  does not loop or fail closure.
- Declared option values that look like JSON arrays or quoted JSON scalars are
  preserved as single selected values on write, read, and repair.
- A read of canonical storage and legacy double-wrapper storage produces the
  same semicolon wire value.
- The repair path is stamp-guarded and only touches metadata-declared
  multi-picklist fields.
- No REST handler response envelope or error shape changes are required.
- Targeted Go tests pass, then the relevant package tests pass.