refactor(internal): extract shared file data loading into internal/filedata

[kinyoklion]

ldfiledata and ldfiledatav2 each carried their own copies of the file reading, JSON/YAML detection, flag-value expansion, absolute-path resolution, and multi-file merge logic. This extracts one implementation into a new internal/filedata package and ports both data sources onto it. A file-based override source (OVERRIDE spec) is about to become a third consumer of the same logic.

The shared Merge produces a consumer-neutral result (ordered flag and segment item lists); each data source adapts it to its own output in a few lines — v1 into store collections for Init, v2 into a full-transfer change set. Merge order is now deterministic for both (v1 previously emitted map-iteration order, which nothing depended on). The duplicate-key error now says flag/segment in both packages; the two previously disagreed (features vs flag).

Two ldfiledatav2 fixes ride along:

• closeReloaderCh was declared and passed to the reloader factory but never assigned, so Close() never stopped the file watcher goroutine, and the reload log line was unreachable. It is now created before the reloader starts, matching ldfiledata.
• Fetch spun up a goroutine and two broadcaster listeners that were never removed (a leak on every call), with a select/default that could silently return an empty no-changes basis instead of the file data. It now loads the files synchronously and returns the change set or an error directly. Read/parse failures still report error kind UNKNOWN and merge failures INVALID_DATA, as before.

No public API changes. All existing ldfiledata/ldfiledatav2/ldfilewatch tests pass unchanged; the shared package has its own unit tests.

SDK-2654

---

Note

Medium Risk
Touches core file-based flag loading for v1 and v2 data sources and changes Fetch/reload behavior in v2, though behavior is intended to match prior semantics with bug fixes.

Overview
Duplicates file read/parse/merge logic from ldfiledata and ldfiledatav2 move into new internal/filedata (ReadFile, AbsFilePaths, Merge, flag-value expansion). Each data source maps the neutral MergeResult into its own store init or full-transfer change set. Merge now keeps first-seen document order (v1 previously used map iteration); duplicate-key errors consistently say flag / segment.

ldfiledatav2 also fixes: closeReloaderCh is created when the reloader starts so Close stops the watcher; Fetch loads files synchronously via load() instead of leaking broadcaster listeners and sometimes returning an empty basis.

^{Reviewed by Cursor Bugbot for commit 5433c04. Bugbot is set up for automated code reviews on this repo. Configure here.}

[kinyoklion]

Addressed the multi-agent review findings in 1c754e7:

• Finding 1 (regression coverage): added TestCloseStopsReloader to both ldfiledata and ldfiledatav2 — a reloader factory captures the close channel and the test asserts Close() closes it. Reverting either lifecycle fix now fails a test.
• Finding 2 (closeReloaderCh race): the channel is now created in the constructor rather than lazily in Sync/Start, so Close never races an assignment; an explicit flag gates the change-driven reload log instead of the channel's nil-ness. Applied to ldfiledata as well, which had the same (pre-existing) shape.
• Finding 3 (version race): the counter is atomic.Int64 now that both Fetch and the reload path go through the same load code.
• Finding 4 (ordering doc): MergeResult's doc now states the guarantee precisely — deterministic at document granularity only, within-document order unspecified, consumers must key by Key.

Findings 5–7 are informational/pre-existing and intentionally left alone. Fixes propagated through the stacked branches (#406, #407) and the rlamb/overrides integration branch; all file-source tests pass under -race, and the flag-overrides contract suite still passes end-to-end.

[kinyoklion]

Inline notes on the behavior fixes in this PR (vs. the original code).

[kinyoklion]

Fix: this channel was declared and passed to the reloader factory but never assigned, so the factory always received nil and Close() never stopped the watcher goroutine. Creating it up front (rather than lazily in Sync) also means Close cannot race an assignment.

[kinyoklion]

Fix: this log was previously gated on closeReloaderCh != nil, which was never true because the channel was never assigned — the line was unreachable. Now gated on an explicit flag so the initial load still does not log it.

[kinyoklion]

Fix: Fetch previously spawned a goroutine and added two broadcaster listeners it never removed (a leak on every call), and its select/default could return an empty no-changes basis instead of the file data, marking an initializer initialized with no data. It now reads and merges synchronously and returns the change set or an error.

[kinyoklion]

Fix: was a plain int incremented from both Fetch (caller goroutine) and the reload path — a data race now that both share one load routine. Atomic preserves the Payload.Target progression.

[kinyoklion]

Hardening, same shape as the v2 fix: previously assigned in Start, which Close could race. No observed bug here (Start/Close are sequential in practice), but the channel is now created up front for the same reason.