feat(internal): add reload orchestration with debouncing and failure retry to file data loading

[kinyoklion]

Adds a Reloader to internal/filedata that owns the reload cycle for a set of data files, and routes ldfiledata and ldfiledatav2 through it. This is the robustness layer the file-based override source (OVERRIDE spec) requires, applied to the existing file data sources as well since the same failure modes (partial writes, rename-replace editors, event storms) affect them equally.

What the Reloader provides:

• Serialized reloads on a single goroutine, so overlapping change signals can never interleave two loads.
• Debouncing (100 ms): bursts of change notifications from a single file edit coalesce into one reload, which also reduces the chance of reading a mid-write file.
• All-or-nothing with last-good retention: any read, parse, or merge failure leaves the previously applied data in place (Apply simply isn't called).
• Automatic retry (1 s) after a failed reload, so a failure observed while a file was being written recovers even if no further filesystem notification ever arrives.
• Skip-unchanged: a reload whose raw file contents are byte-identical to the last applied contents doesn't re-apply, absorbing duplicate notifications and no-op edits.
• Log dedup: with automatic retries, a persistently broken file would otherwise log the same error every second; identical repeats are demoted to debug level.

Consumers keep their own success/failure semantics via Apply/OnError callbacks (v1: store init + status; v2: change-set broadcast + status, with read failures still mapped to UNKNOWN and merge failures to INVALID_DATA via the new filedata.ReadError). Sources configured without a reloader factory still load exactly once with no debounce or retry, as before.

Behavior notes for existing file data source users (changelog-worthy): change-driven reloads now settle for ~100 ms before applying; a reload that failed because a file was temporarily unreadable now recovers automatically instead of waiting for the next filesystem event; and byte-identical rewrites no longer re-initialize the store. One v2 test needed to tolerate the retry loop. ldfilewatch's public API is untouched — debouncing lives in the orchestrator so it applies to any trigger source.

Reloader unit tests run with millisecond-scale delays and observe behavior deterministically through the callbacks (coalescing, retry-without-trigger, retry-stops-after-success, skip-unchanged, close semantics), including under -race.

SDK-2654

---

Note

Medium Risk
Changes runtime behavior of file-based flag loading (timing, retries, fewer re-inits on identical files) on a core SDK data path, though semantics on success/failure are preserved via callbacks.

Overview
Introduces internal/filedata.Reloader to own multi-file load cycles: serialized reloads, ~100ms debounce on change signals, 1s automatic retry after failures, last-good retention (no Apply on error), skip-unchanged via content hashing, and deduplicated error logging on repeated failures.

ReadError is added in filedata (with parseDocument split out of ReadFile) so per-file read/parse failures are distinguishable from merge errors. ldfiledata and ldfiledatav2 replace inline reload logic with Reloader callbacks (applyData / handleError); file watchers now call Trigger instead of reloading directly. Debounce/retry apply only when a reloader factory is configured; one-shot sources still load once. v2 drops its local read error type in favor of filedata.ReadError (read → UNKNOWN, merge → INVALID_DATA). A v2 recovery test tolerates extra interrupted results from the retry loop.

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

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 18fc662. Configure here.}

[kinyoklion]

Addressed the multi-agent review findings in 41f0f21:

• Finding 1 (Close wedged by a blocked read): Close no longer waits for an in-flight reload — it closes the stop channel and returns, restoring the pre-feat(internal): add reload orchestration with debouncing and failure retry to file data loading #406 shutdown guarantee. A reload that was mid-read when Close happened re-checks the closed state before delivering through Apply/OnError, so a reload that starts after Close still never calls back; one already past that check may finish delivering, as before this PR. Regression test: Close returns while a reload is parked inside Apply.
• Finding 2 (per-retry OnError spam): OnError now fires once per distinct failure, gated by the same identical-error condition as the log demotion; a success re-arms it. Status listeners see one report per failure instead of one per second. Test asserts a single OnError and a single error-level log line across a stretch of identical retries, a new report on a different error, and re-arming after recovery.
• Findings 3+4 (re-Sync leak, unsynchronized fs.reloader): the Reloader is created in the data source constructors (both packages), like closeReloaderCh — no assignment in Sync/Start to race or to orphan a prior instance.
• Finding 5: Apply nil-guarded like OnError.
• Finding 11: a failed synchronous ReloadNow now arms the retry timer directly instead of routing through the change-trigger path, so the "detected a change" log no longer fires for it (and finding 2's dedup keeps the retries quiet).
• Findings 9, 12, 13: zero/negative delay semantics documented; a comment explains why the un-drained timer Stop+Reset is safe on this module's Go version; the read-error message is built in one place.
• Finding 7: the debounce-coalescing test's settle window is now 20× the trigger burst rather than ~1×.
• Finding 10 resolved structurally by the finding-1 change (a callback can no longer deadlock by calling Close), though the callback docs still say not to.

Finding 6 left as-is (unreachable today; not worth complicating the Apply seam), finding 8 covered by the new dedup test's log assertion, finding 14 pre-existing/out of scope. Propagated through #407 and the rlamb/overrides integration branch; all file-machinery tests pass under -race and the flag-overrides contract suite passes end-to-end.

[kinyoklion]

Round-2 finding N1 addressed in 54aeb92:

• N1 (goroutine leak per initializer build): NewReloader no longer starts the worker goroutine at construction; it starts on the first ReloadNow or Trigger. The initializer path only ever calls Fetch, which never touches the reloader, so building a file data source via AsInitializer() — an interface with no Close that the data system never tears down — now spawns nothing. Regression tests at both levels: constructing unused Reloaders accumulates no goroutines, and a loop of initializer Build+Fetch cycles in ldfiledatav2 holds the goroutine count flat. A Close racing the first use is benign: ensureStarted skips a closed reloader, and a worker that does start exits immediately on the closed channel.
• R2 (repeated v2 Sync accumulation): Sync is now single-use, guarded before any listener or watcher registration; a repeat (or Sync after Close) gets an Off result on a buffered channel, which also removes the pre-existing blocking send on that rejection path (round 1's finding 6).
• R4: the parked-Apply test now also asserts the worker goroutine exits once the callback releases (polled inline — spawning a polling goroutine would disturb the count being measured).
• R5: comment added at v1's Close noting the post-Close no-panic guarantee rests on the Broadcaster's lock discipline.

R1/R3/R6 left as noted (theoretical/unreachable/correct). All file-machinery suites pass under -race, and the flag-overrides contract suite passes end-to-end on the updated integration branch.

[kinyoklion]

Round-3 finding N1 addressed in 8893141:

• N1 (Sync deadlock on reloader-factory failure): the factory-error branch sent its Off result on the unbuffered result channel before the reader goroutine existed — the same anti-pattern the repeated-Sync guard already fixed, sitting three lines below it. A watcher-setup failure (inotify instance/watch limits) therefore hung Sync, and with it client initialization, with no fallback attempted. The branch now returns the same buffered single-result rejection, stops the already-started reload worker (nothing can ever trigger a reload without a watcher, and its broadcasts had no consumer), and detaches the two listeners it had registered. New test drives Sync with an erroring factory and asserts a prompt Off result on a closed channel.
• T1: added the missing repeated-Sync test (second Sync → single Off, closed channel, first Sync unaffected).
• T2: the unused-Reloader goroutine check now polls for the process-global count to settle instead of asserting one sample (and drops the no-op runtime.GC()); stable at -race -count=20.
• T3: the debounce-coalescing fixture enables SkipUnchanged, matching production configs and absorbing the documented tick-vs-trigger extra reload.

Propagated through #407, rlamb/overrides-base, #415/#416, and the rlamb/overrides end-to-end branch; -race -count=20 clean on the reloader suite and the flag-overrides contract suite passes end-to-end.

[kinyoklion]

Round 4 came back clean — approved for merge. The one non-blocking note (L1) is addressed in the latest commit: the debounce test had lost its discriminating power (with SkipUnchanged and identical contents, even a broken debounce collapses to one Apply). It now runs with SkipUnchanged off and counts applies across the settle window, tolerating the single documented tick-vs-trigger extra reload and failing on anything more; verified to fail with debouncing disabled and stable at -race -count=20. Propagated through the stack and the integration branches.

I1 noted as a pre-existing design choice (a source whose files loaded but whose watcher failed reports Off rather than serving statically — same intent as base); leaving it as-is for this PR.