mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-09-25 21:43:42 +00:00
fa15ab0a30
Partial fix for #1809. Red commit:c9c782b5(CI runs on PR open; standalone red-branch CI not configured — local repro proves the gate, see below). ## Problem Issue #1809: at startup the background fill loader logged `background load FAILED` within seconds and `backgroundLoadFailed=true` was set, leaving the coverage gate tripped even though LoadChunked itself completed normally. ## Root cause `main.go:225-245` spawned `go store.loadBackgroundChunks()` as soon as `FirstChunkReady` fired (chunk #1 = 10000 tx). But `s.oldestLoaded` is only assigned at the end of `LoadChunked` (`chunked_load.go:329-333`), ~tens of seconds later. The bg loader read `oldestLoaded==""` at `store.go:1462-1466`, broke out immediately, walked zero chunks, and the coverage gate at `store.go:1543-1554` flipped `backgroundLoadFailed=true`. ## Fix (initial) Introduce `PacketStore.RunStartupLoad(chunkSize)` (`chunked_load.go`). It runs `LoadChunked` first; only on success and only when `hotStartupHours > 0` does it call `loadBackgroundChunks`. `main.go` invokes `RunStartupLoad` in the same goroutine pattern as before, so `FirstChunkReady` still unblocks the HTTP listener bind at chunk #1 — only the bg loader is gated. ## Round-1 followups (this push) Reviewer-driven hardening on top of the initial fix: ### Production behavior (commitdb5592f6) - **Steady-state semantics tightened.** `RunStartupLoad` now picks a terminal state on every branch: - LoadChunked error → `backgroundLoadFailed=true` with captured error (was: `done=false, failed=false` indefinite). - `hotStartupHours == 0` → `backgroundLoadDone=true` immediately, `progress=100` (was: `done=false` forever → healthz stuck on `backgroundLoadComplete=false`). - Successful hot-window path → terminal state is whatever `loadBackgroundChunks` sets (#1690 semantics, unchanged). - **Runtime invariant assertion (A7).** `loadBackgroundChunks` panics when `oldestLoaded==""` and packets exist — a future refactor that re-introduces the parallel-spawn race fails loudly instead of silently shipping the same coverage regression. - **`RunStartupLoad` cleanup.** Inlined the superfluous goroutine + channel that wrapped `LoadChunked` (direct call is equivalent). - **Logging.** Added an INFO line between `LoadChunked` completion and bg-loader start (the #1809 post-mortem needed exactly this signal). Fixed the lying `"background load will start"` log that fired even on the `hotStartupHours==0` branch. - **Immutability documented.** `hotStartupHours` is now explicitly documented as immutable post-construction, so the lock-free reads in `LoadChunked` / `RunStartupLoad` / `loadBackgroundChunks` are sound. ### Test coverage (commitsdb5592f6+e9e12acf) - **Tautology fix (B1, commite9e12acf).** The original `Test1809_StartupLoad_BgLoaderSeesOldestLoaded` fixture seeded all 100 rows inside the 1h hot window, so `LoadChunked` alone produced coverage=1.0 — the test passed even if `loadBackgroundChunks` was a no-op. Rewrote the fixture to spread 100 rows over 14 days with `hotStartupHours=24`, so only ~7 rows are hot and the remaining ~93 MUST be loaded by the bg loader for the assertions to hold. Original red-commit assertions kept intact; added `len(packets) > hot-only cap` and `oldestLoaded < hot-cutoff - 12h` assertions on top. - **New tests (commitdb5592f6, `runstartup_load_test.go`)** codify the new contracts: - `TestRunStartupLoad_HotStartupHoursZero_SetsDoneImmediately` - `TestRunStartupLoad_LoadChunkedError_SetsFailedTerminal` - `TestRunStartupLoad_EmptyDB_SetsDoneTerminal` - `TestRunStartupLoad_BgLoaderRunsAfterLoadChunkedSets_OldestLoaded` - `TestLoadBackgroundChunks_PanicsOnOldestLoadedEmpty_Invariant` ### Docs (commit70fa16f7) - Package-level doc in `chunked_load.go` now documents `RunStartupLoad` as the orchestrator entry point alongside `LoadChunked` / `loadStatusMiddleware` / `OnChunkLoaded`. ### Preflight (commiteec1b48c) - Test-fixture DDL annotated with `// PREFLIGHT: async=true reason="unit-test fixture"` so the async-migration gate distinguishes ephemeral test schema from prod migration paths. ## What's still NOT fixed (left intentionally open) This PR addresses the startup race specifically. Issue #1809 will be closed by the operator after observing healthy startup logs (`background load complete: ... coverage=100.0%`) in prod for a full restart cycle. Do not auto-close — leave open until the operator verifies. ## Test Local repro (red branch state, before green commit): `go test ./cmd/server/ -run Test1809_StartupLoad_BgLoaderSeesOldestLoaded` → FAIL on assertion `backgroundLoadFailed=true ... oldest="2026-06-30T18:56:09Z"`. After green commit + round-1 followups: PASS. Full `cmd/server/...` suite: ok in ~70s. ## Risk Low — startup path only. New behavior gates surfaced by the new tests; coverage-gate semantics unchanged. Runtime panic is a new failure mode but only fires on a state (`oldestLoaded=="" && len(packets)>0`) that is unreachable on the current code path — it exists solely as a refactor tripwire. --------- Co-authored-by: mc-bot <bot@corescope.local> Co-authored-by: openclaw-bot <bot@openclaw.local> Co-authored-by: meshcore-bot <bot@meshcore>