fix: guard initConfig against silent config reset on empty file load#36
fix: guard initConfig against silent config reset on empty file load#36bob56621517 wants to merge 1 commit into
Conversation
When global or project config files are transiently inaccessible
(I/O contention, race condition, missing files), loadConfigFromPaths()
returns {}. Calling buildConfig({}) fills all fields with hardcoded
defaults, then Object.assign(CONFIG, ...) overwrites the correctly
initialized CONFIG singleton — silently breaking user-configured
embedding model, dimensions, and API endpoint.
The Web UI migration detection (/api/migration/detect) reads
CONFIG.embeddingDimensions and CONFIG.embeddingModel to compare
against shard metadata, so the reset manifests as a false dimension
mismatch warning.
Fix: skip CONFIG rebuild entirely when both config sources are empty.
Closes ZeR020#35
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesConfig Guard Against Silent Reset
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| JavaScript | Jun 4, 2026 7:22a.m. | Review ↗ | |
| Secrets | Jun 4, 2026 7:22a.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
|
@ZeR020 This PR is ready for review. It fixes a bug where silently resets CONFIG to hardcoded defaults when the global config file is transiently inaccessible, causing false dimension mismatch warnings in the Web UI. Changes: Single-file fix in — adds a guard to skip CONFIG rebuild when config sources are empty. CI note: The DeepSource JavaScript failure appears pre-existing (not from this change — only 11 lines of guard logic added). CodeRabbit skipped. Secrets check passed. Please review when available. Thanks! |
|
Closing — the fix has been applied directly to main with a test added. Your guard approach was the right one: simplest fix, root cause, no new state. Thanks for the contribution! |
…rage comment Four fixes from GitHub triage: - Guard initConfig() against silent CONFIG reset when both global and project config sources return empty (transient I/O failure). Preserves the existing CONFIG singleton instead of rebuilding from defaults. Fixes #35. Closes #36, #37 (simple guard chosen over snapshot approach). - Expand ~ before URL parsing in secret-resolver.ts so file://~/path works on Linux. Fixes #34. - Update vitest + @vitest/coverage-v8 to 3.2.6 (fixes critical CVE GHSA-5xrq-8626-4rwp: arbitrary file read/execute via Vitest UI server). - Add ponytail comment on localStorage API key storage in web/app.js documenting the accepted risk (localhost-only server). Verified: typecheck, 663 tests, build, format:check, deepsource lint pass.
Version bump, changelog, and docs update for release. Added: - file://~/ path expansion in *ApiKey config fields (#34) - initConfig() empty-config guard preserving CONFIG on transient I/O failure (#35) Fixed: - initConfig() silent CONFIG reset to defaults (#35, #36) - file://~/ throws on Linux — ~ expanded before URL parsing (#34) - Vitest critical CVE GHSA-5xrq-8626-4rwp (dev-only, updated to 3.2.6) Removed (dead code, zero consumers): - NSWBackend + nsw/nsw-first factory branches (production-unreachable) - supportsSession() abstract method (always returned true) - profile-utils.ts (zero importers) - aiSessionManager backward-compatible Proxy export - getSupportedProviders(), startCleanupSchedule/stopCleanupSchedule - ConflictCheckLock class (inlined as Set) Changed: - iso-639-3 full dataset → Intl.DisplayNames - Dynamic imports → static in auto-capture.ts and handlers/memory.ts - .gitignore reorganized (280 → 73 lines) - localStorage API key storage documented (code scanning #7) Closed: #34, #35, #36, #37
- Release workflow now extracts the changelog section for the released version and uses it as the GitHub release body, with auto-generated notes appended (PR references, new contributors). This ensures consistent, detailed release notes even when changes are applied directly to main without PRs. - Added Contributors section to v2.17.0 changelog crediting @boyxil (issue #34), @bob56621517 (issue #35, PR #36), and @kingrubic (PR #37). - Updated the v2.17.0 GitHub release with full notes and credits.
Problem
initConfig()insrc/config.tscan silently reset the entire CONFIG to hardcoded defaults whenloadConfigFromPaths()returns an empty object{}. This happens when the global config file is transiently inaccessible (I/O contention, file lock, race condition).When triggered,
buildConfig({})fills all fields with defaults (768D Xenova/nomic-embed-text-v1), thenObject.assign(CONFIG, ...)overwrites the correctly initialized CONFIG — silently breaking user-configured embedding model, dimensions, and API endpoint.The Web UI migration detection then reports a false dimension mismatch because CONFIG values no longer match shard metadata.
Fix
Add a guard in
initConfig()— when both global and project config sources are empty, skip the CONFIG rebuild entirely and preserve the existing values from module-level initialization.Testing
node --checkpassesCloses #35
Summary by CodeRabbit