Repository navigation
fix(db): degrade getSettings() to defaults instead of crashing Home on corrupted SQLite (#14060) - #14421
Merged
Conversation
…n corrupted SQLite (#14060) Root cause: getSettings() (src/lib/db/settings.ts) read the key_value table without try/catch, and src/app/(dashboard)/home/page.tsx awaited it unguarded during server-side rendering. When the key_value table's page is corrupted (SQLITE_CORRUPT / 'database disk image is malformed'), that unguarded read threw synchronously and crashed the Home Server Component render, producing the generic Next.js Internal Server Error the reporter saw. Fix: getSettings() now catches the read error, warns via console.warn (mirroring the pattern already used by optimizationSettings.ts, proxyLogger.ts and memory/index.ts), and falls through to the existing in-memory defaults. The Home page also wraps getSettings() in a .catch() as defense-in-depth against a future unguarded read anywhere in its dependency chain. Regression test: tests/unit/settings-14060-getsettings-corrupt-db.test.ts corrupts only the on-disk page backing key_value on an otherwise-valid storage.sqlite, then asserts getSettings() degrades to defaults instead of throwing.
…er (#14060) Swallowing the key_value read error inside getSettings() returned password-less defaults, so isAuthRequired() stopped failing closed and disabled auth for loopback requests (including the first-password bootstrap write). Revert that and move the degradation into a display-only loadHomeSettings() helper used by the Home page. Regression test proves isAuthRequired() stays true on a corrupted key_value table and that Home degrades without throwing.
diegosouzapw
added a commit
that referenced
this pull request
Sep 24, 2026
All three fail on the release tip (fast-path shards 3/4 and 4/4 of #14718, a PR that touches none of their subjects) and no open base-red PR covers them. Each follows an intentional product change that merged without its guard: - responses-handler: #14572 (#14330) gave the synthesized response.in_progress keepalive a sequence_number and response object; the assertion now pins the full compliant frame instead of the bare one. - check-vitest-exclusions: #14493 repaired every quarantined suite and emptied the inventory; the live-config test still fails on any exclusion the inventory does not list, so the non-empty check was the only stale part. - perf-waterfall-elimination: #14421 routes the Home settings read through loadHomeSettings() (defaults to getSettings); the test still requires it in the same Promise.all batch, before getMachineId, with no serial await. 28/28 on the idle .113. Refs #14547
diegosouzapw
added a commit
that referenced
this pull request
Sep 24, 2026
…s loader; rebaseline two test files - responses-handler: the synthesized in_progress frame now carries sequence_number 1 and a response object (#14572/#14330) — pin that prefix. - perf-waterfall A1: Home batches loadHomeSettings() (#14421/#14060) with getMachineId(); assert the loader still reads getSettings() by default. - file-size: documented rebaseline for chatcore-translation-paths (+12) and account-fallback-service (+14, 11 of them Prettier reflow of tip lines). Refs #14496
diegosouzapw
added a commit
that referenced
this pull request
Sep 24, 2026
All three fail on the release tip (fast-path shards 3/4 and 4/4 of #14718, a PR that touches none of their subjects) and no open base-red PR covers them. Each follows an intentional product change that merged without its guard: - responses-handler: #14572 (#14330) gave the synthesized response.in_progress keepalive a sequence_number and response object; the assertion now pins the full compliant frame instead of the bare one. - check-vitest-exclusions: #14493 repaired every quarantined suite and emptied the inventory; the live-config test still fails on any exclusion the inventory does not list, so the non-empty check was the only stale part. - perf-waterfall-elimination: #14421 routes the Home settings read through loadHomeSettings() (defaults to getSettings); the test still requires it in the same Promise.all batch, before getMachineId, with no serial await. 28/28 on the idle .113. Refs #14547
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #14060
Root cause
src/app/(dashboard)/home/page.tsxawaitsgetSettings()(src/lib/db/settings.ts) with no try/catch during server-side rendering. When the SQLite pager reports corruption (SQLITE_CORRUPT/ "database disk image is malformed") localized to thekey_valuetable, that unguarded read throws synchronously and crashes the Home Server Component render, producing the generic Next.js "Internal Server Error" the reporter saw — while every other DB-backed startup consumer (optimizationSettings.ts,proxyLogger.ts,memory/index.ts, etc.) already catches the identical error and falls back to defaults.Fix
src/lib/db/settings.ts: wrapped thekey_valueread ingetSettings()in try/catch; on error,console.warn(mirroringoptimizationSettings.ts's pattern) and fall through to the existing in-memory defaults object instead of throwing.src/app/(dashboard)/home/page.tsx: defense-in-depth — wrapsgetSettings()in.catch()so a future unguarded read anywhere in its dependency chain can't regress this same crash.Regression test
tests/unit/settings-14060-getsettings-corrupt-db.test.ts— corrupts only the on-disk page backing thekey_valuetable (located viasqlite_master.rootpage) on an otherwise-valid, migratedstorage.sqlite, reopens the DB (boot probe / other tables stay readable), then callsgetSettings().RED (on unfixed base, per the plan-file's own proof — verbatim log match to the reporter's):
GREEN (with the fix, inverted assertion per the analysis audit note):
Gates run
node --import tsx/esm --test tests/unit/settings-14060-getsettings-corrupt-db.test.ts→ pass 1/1npm run typecheck:core→ exit 0node scripts/check/check-dashboard-typecheck.mjs→ OK, 206 pre-existing errors within frozen baselinenpx eslint --suppressions-location config/quality/eslint-suppressions.json <changed files>→ cleannode scripts/check/check-file-size.mjs→ no violations on touched filesnode scripts/check/check-complexity.mjs→ OK (2904 violations, baseline 3218)node scripts/check/check-cognitive-complexity.mjs→ OK (1316 violations, baseline 1437)node scripts/check/check-changelog-integrity.mjs→ OKtests/unit/{db-settings-crud,db-settings-split,db-settings-extended,settings-api,settings-debugmode-default,db-settings-debug-mode-default-10312,database-settings-maintenance}.test.ts→ 108/108 passExisting tests aligned
None — no pre-existing test encoded the old (crashing) behavior; only the new plan-file repro did, and its assertion was inverted per the analysis audit note before use.
Plan-file:
_tasks/pipeline/bugs/2-implementing/14060-bug-dashboard-internal-server-error-with-malformed-sqlite-data.plan.mdRework (merge-batch 2026-09-23)
Defect found in review: the try/catch added inside
getSettings()replaced its fail-closed behavior with defaults.isAuthRequired()(src/shared/utils/apiAuth.ts) relies ongetSettings()throwing so it lands in itscatchand requires auth (the safe default, also onSQLITE_BUSY). With the swallow, a read error returned password-less defaults, which disabled auth for loopback requests and opened the first-password bootstrap write (POST /api/settings/require-login).Correction:
src/lib/db/settings.ts: reverted to the base version, sogetSettings()rejects on a read error again. It is no longer in this PR's diff.src/app/(dashboard)/home/loadHomeSettings.ts: a display-only helper that catches the error, logs a warning and returns{ setupComplete: false }.home/page.tsxuses it, so Home still degrades instead of returning 500, and no auth/authz path sees defaults.tests/unit/settings-14060-getsettings-corrupt-db.test.tsrewritten. It corrupts onlykey_value's root page and asserts:isAuthRequired(loopbackRequest, { loopback: true })staystrueon a protected path and on the bootstrap write;getSettings()still rejects;loadHomeSettings()resolves to defaults without throwing, plus a healthy-settings pass-through case.Red → green:
settings.ts(7064cca), the test fails withAssertionError: isAuthRequired() must fail closed when settings are unreadable — actual: false, expected: true.Gates (after merging
origin/release/v3.8.51):settings-14060+api-auth).typecheck:corehas only the inheritedcliproxyAccountHealth.ts(157,5) TS2322.check-file-size: OK.check:open-sse-typecheckis red withopen-sse/executors/auggie.tsTS2769/TS18047. That error is inherited from the release tip (fix(auggie): resolve spawn EINVAL when running the CLI shim on Windowsfix(auggie): resolve spawn EINVAL when running the CLI shim on Windows #14215); this PR changes nothing underopen-sse/.check-dashboard-typecheckis red withsrc/lib/combos/intelligentRouting.tsTS2698. That error is also inherited from the release tip (last touched by fix(routing): honor manual auto weights and weight weekly reset urgency higher #14176); this PR does not touch the file.