fix(db): persist the Keep-latest-backups retention setting (#3834) - #3867
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Code Review
This pull request implements persistence for the "Keep latest backups" database retention setting, resolving an issue where the setting would reset to 20 upon page refresh. The chosen value is now saved to a dedicated key_value store namespace (dbBackup) and retrieved with fallback precedence (env override → persisted value → default). Unit tests have been added to verify this behavior. Feedback on the changes suggests removing a redundant Number.parseInt call on a JSON-parsed number and wrapping the database write operation in a try/catch block to prevent potential write failures from crashing the cleanup process.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| .prepare("SELECT value FROM key_value WHERE namespace = ? AND key = ?") | ||
| .get(DB_BACKUP_SETTINGS_NAMESPACE, DB_BACKUP_MAX_FILES_KEY) as { value?: string } | undefined; | ||
| if (!row?.value) return undefined; | ||
| const parsed = Number.parseInt(JSON.parse(row.value), 10); |
There was a problem hiding this comment.
The JSON.parse(row.value) call already returns a parsed number since the value was stored using JSON.stringify(value) where value is a number. Wrapping it in Number.parseInt(..., 10) is redundant and can be safely removed.
| const parsed = Number.parseInt(JSON.parse(row.value), 10); | |
| const parsed = JSON.parse(row.value); |
| const db = getDbInstance(); | ||
| db.prepare( | ||
| "INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES (?, ?, ?)" | ||
| ).run(DB_BACKUP_SETTINGS_NAMESPACE, DB_BACKUP_MAX_FILES_KEY, JSON.stringify(value)); |
There was a problem hiding this comment.
Database write operations can fail due to various reasons (e.g., database locked, read-only database, or disk full). Since persisting this setting is a non-critical side effect of the backup cleanup process, wrapping the database write in a try/catch block ensures that a write failure does not crash the entire cleanup operation.
| const db = getDbInstance(); | |
| db.prepare( | |
| "INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES (?, ?, ?)" | |
| ).run(DB_BACKUP_SETTINGS_NAMESPACE, DB_BACKUP_MAX_FILES_KEY, JSON.stringify(value)); | |
| try { | |
| const db = getDbInstance(); | |
| db.prepare( | |
| "INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES (?, ?, ?)" | |
| ).run(DB_BACKUP_SETTINGS_NAMESPACE, DB_BACKUP_MAX_FILES_KEY, JSON.stringify(value)); | |
| } catch (error) { | |
| console.error("Failed to persist DB backup max files setting:", error); | |
| } |
* chore(release): continue v3.8.25 development cycle after main code-sync (r5) main fast-forwarded to release/v3.8.25 (#3863): unblocked Build+Docker via #3864, plus #3837 (mimocode proxy) and #3862 (trivy bump). This marker re-opens the umbrella PR for further v3.8.25 work. No version bump. * fix(db): persist the Keep-latest-backups retention setting (#3834) (#3867) * fix(oauth): clear GitLab Duo setup message instead of 500 (#3861) (#3868) * test(oauth): prove refresh_token preserved on real gemini-cli/antigravity dispatch (#3850) (#3869) * feat(compression-ui): unified compression config UI — per-engine pages + combos editor + menu + WS default-on (#3860) Integrated into release/v3.8.25 — feat(compression-ui): unified compression configuration UI (Compression Hub + per-engine Lite/Aggressive/Ultra pages + combos editor + sidebar entry + live-WS default-on). File-size re-baselined for sidebarVisibility.ts/chatCore.ts growth; orphan ws test relocated to a collected path. * docs(changelog): complete the v3.8.25 release notes + credit all contributors Audited every commit since v3.8.24 and filled the gaps the [3.8.25] section was missing: a New Features section (compression engines + Compression Studios #3848, compression UI #3860, injection-guard #3857, kiro discovery #3836, Veo #3839, mimocode proxy #3837, Arena ELO flag #3821), 9 more Fixed entries (#3811/#3807/#3759/#3849/#3838/#3835/#3814/#3820/#3819), a Security section (CCR IDOR #3859, supply-chain #3824), and an Internal/Quality section. Every contributor and issue reporter is now credited. * docs(changelog): restore + complete the v3.8.25 release notes Re-adds CHANGELOG.md (a prior server-side commit accidentally dropped it) with the complete, audited [3.8.25] section: New Features, the full Fixed list, Security & Hardening, and Internal/Quality — every contributor and issue reporter credited. * chore(release): finalize v3.8.25 — reconcile CHANGELOG + i18n mirrors, document OMNIROUTE_MAX_PENDING_MIGRATIONS, green the unit suite Release-gate reconciliation for v3.8.25: - CHANGELOG: dated 2026-06-14, linked #3826, rolled up file-size re-baselines (#3823/#3833), recorded the test-greening; re-synced all 41 i18n CHANGELOG mirrors. - Documented OMNIROUTE_MAX_PENDING_MIGRATIONS (#3416) in .env.example + ENVIRONMENT.md. - Greened the unit suite (was merged red on 4 CI shards): aligned 10 stale tests to this cycle's intended behavior (#3838/#3822/#3501/SOCKS5/Vertex-Express/Antigravity) and the same-provider 503 fall-through test; de-flaked the compression benchmark reproducibility and ServiceSupervisor crash tests. No production code changed. * ci(security): clear OpenSSF Scorecard code-scanning noise + harden workflow token permissions The Security tab held 155 open alerts, ALL from the advisory OpenSSF Scorecard tool (#3824) — supply-chain/posture scores, not code vulnerabilities — which drowned out real CodeQL findings. - scorecard.yml: stop uploading SARIF to the code-scanning tab (drop the upload-sarif step + the now-unused security-events: write). The run still produces the OpenSSF badge (publish_results) and a downloadable SARIF artifact. - TokenPermissions hardening (the high-severity, genuinely-valuable subset): set each workflow's top-level token to read-only and grant the exact writes at the job level that needs them — npm-publish (id-token/packages on publish jobs), docker-publish (packages on build), electron-release (contents on build/release, id-token/packages on publish-npm), build-fork (packages on build), claude (empty top-level; job grants its own). The 155 existing alerts were dismissed. Not adopting repo-wide SHA-pinning (143 PinnedDependencies advisories) — declined. * test(integration): align stale wiring/socks5 integration tests to this cycle's behavior These were red on the CI Integration job (pre-existing). No production code changed: - integration-wiring: the combos page no longer renders a per-page EmailPrivacyToggle (#3822 consolidated it into Settings → Appearance); the provider-detail test-result masking and upstream-proxy copy moved to decomposed components (#3501 BatchTestResultsModal / UpstreamProxyCard) — assertions now read the owning files. - api-routes-critical: SOCKS5 is now enabled by default (opt-out), so the disabled- rejection test must set ENABLE_SOCKS5_PROXY=false explicitly (an unset env now means enabled). (The ~32 live-Gemini integration tests are gated on OMNIROUTE_API_KEY and skip in CI; they only 'fail' locally when that key is present without a running server.)
* chore(release): continue v3.8.25 development cycle after main code-sync (r5) main fast-forwarded to release/v3.8.25 (diegosouzapw#3863): unblocked Build+Docker via diegosouzapw#3864, plus diegosouzapw#3837 (mimocode proxy) and diegosouzapw#3862 (trivy bump). This marker re-opens the umbrella PR for further v3.8.25 work. No version bump. * fix(db): persist the Keep-latest-backups retention setting (diegosouzapw#3834) (diegosouzapw#3867) * fix(oauth): clear GitLab Duo setup message instead of 500 (diegosouzapw#3861) (diegosouzapw#3868) * test(oauth): prove refresh_token preserved on real gemini-cli/antigravity dispatch (diegosouzapw#3850) (diegosouzapw#3869) * feat(compression-ui): unified compression config UI — per-engine pages + combos editor + menu + WS default-on (diegosouzapw#3860) Integrated into release/v3.8.25 — feat(compression-ui): unified compression configuration UI (Compression Hub + per-engine Lite/Aggressive/Ultra pages + combos editor + sidebar entry + live-WS default-on). File-size re-baselined for sidebarVisibility.ts/chatCore.ts growth; orphan ws test relocated to a collected path. * docs(changelog): complete the v3.8.25 release notes + credit all contributors Audited every commit since v3.8.24 and filled the gaps the [3.8.25] section was missing: a New Features section (compression engines + Compression Studios diegosouzapw#3848, compression UI diegosouzapw#3860, injection-guard diegosouzapw#3857, kiro discovery diegosouzapw#3836, Veo diegosouzapw#3839, mimocode proxy diegosouzapw#3837, Arena ELO flag diegosouzapw#3821), 9 more Fixed entries (diegosouzapw#3811/diegosouzapw#3807/diegosouzapw#3759/diegosouzapw#3849/diegosouzapw#3838/diegosouzapw#3835/diegosouzapw#3814/diegosouzapw#3820/diegosouzapw#3819), a Security section (CCR IDOR diegosouzapw#3859, supply-chain diegosouzapw#3824), and an Internal/Quality section. Every contributor and issue reporter is now credited. * docs(changelog): restore + complete the v3.8.25 release notes Re-adds CHANGELOG.md (a prior server-side commit accidentally dropped it) with the complete, audited [3.8.25] section: New Features, the full Fixed list, Security & Hardening, and Internal/Quality — every contributor and issue reporter credited. * chore(release): finalize v3.8.25 — reconcile CHANGELOG + i18n mirrors, document OMNIROUTE_MAX_PENDING_MIGRATIONS, green the unit suite Release-gate reconciliation for v3.8.25: - CHANGELOG: dated 2026-06-14, linked diegosouzapw#3826, rolled up file-size re-baselines (diegosouzapw#3823/diegosouzapw#3833), recorded the test-greening; re-synced all 41 i18n CHANGELOG mirrors. - Documented OMNIROUTE_MAX_PENDING_MIGRATIONS (diegosouzapw#3416) in .env.example + ENVIRONMENT.md. - Greened the unit suite (was merged red on 4 CI shards): aligned 10 stale tests to this cycle's intended behavior (diegosouzapw#3838/diegosouzapw#3822/diegosouzapw#3501/SOCKS5/Vertex-Express/Antigravity) and the same-provider 503 fall-through test; de-flaked the compression benchmark reproducibility and ServiceSupervisor crash tests. No production code changed. * ci(security): clear OpenSSF Scorecard code-scanning noise + harden workflow token permissions The Security tab held 155 open alerts, ALL from the advisory OpenSSF Scorecard tool (diegosouzapw#3824) — supply-chain/posture scores, not code vulnerabilities — which drowned out real CodeQL findings. - scorecard.yml: stop uploading SARIF to the code-scanning tab (drop the upload-sarif step + the now-unused security-events: write). The run still produces the OpenSSF badge (publish_results) and a downloadable SARIF artifact. - TokenPermissions hardening (the high-severity, genuinely-valuable subset): set each workflow's top-level token to read-only and grant the exact writes at the job level that needs them — npm-publish (id-token/packages on publish jobs), docker-publish (packages on build), electron-release (contents on build/release, id-token/packages on publish-npm), build-fork (packages on build), claude (empty top-level; job grants its own). The 155 existing alerts were dismissed. Not adopting repo-wide SHA-pinning (143 PinnedDependencies advisories) — declined. * test(integration): align stale wiring/socks5 integration tests to this cycle's behavior These were red on the CI Integration job (pre-existing). No production code changed: - integration-wiring: the combos page no longer renders a per-page EmailPrivacyToggle (diegosouzapw#3822 consolidated it into Settings → Appearance); the provider-detail test-result masking and upstream-proxy copy moved to decomposed components (diegosouzapw#3501 BatchTestResultsModal / UpstreamProxyCard) — assertions now read the owning files. - api-routes-critical: SOCKS5 is now enabled by default (opt-out), so the disabled- rejection test must set ENABLE_SOCKS5_PROXY=false explicitly (an unset env now means enabled). (The ~32 live-Gemini integration tests are gated on OMNIROUTE_API_KEY and skip in CI; they only 'fail' locally when that key is present without a running server.)
* chore(release): continue v3.8.25 development cycle after main code-sync (r5) main fast-forwarded to release/v3.8.25 (diegosouzapw#3863): unblocked Build+Docker via diegosouzapw#3864, plus diegosouzapw#3837 (mimocode proxy) and diegosouzapw#3862 (trivy bump). This marker re-opens the umbrella PR for further v3.8.25 work. No version bump. * fix(db): persist the Keep-latest-backups retention setting (diegosouzapw#3834) (diegosouzapw#3867) * fix(oauth): clear GitLab Duo setup message instead of 500 (diegosouzapw#3861) (diegosouzapw#3868) * test(oauth): prove refresh_token preserved on real gemini-cli/antigravity dispatch (diegosouzapw#3850) (diegosouzapw#3869) * feat(compression-ui): unified compression config UI — per-engine pages + combos editor + menu + WS default-on (diegosouzapw#3860) Integrated into release/v3.8.25 — feat(compression-ui): unified compression configuration UI (Compression Hub + per-engine Lite/Aggressive/Ultra pages + combos editor + sidebar entry + live-WS default-on). File-size re-baselined for sidebarVisibility.ts/chatCore.ts growth; orphan ws test relocated to a collected path. * docs(changelog): complete the v3.8.25 release notes + credit all contributors Audited every commit since v3.8.24 and filled the gaps the [3.8.25] section was missing: a New Features section (compression engines + Compression Studios diegosouzapw#3848, compression UI diegosouzapw#3860, injection-guard diegosouzapw#3857, kiro discovery diegosouzapw#3836, Veo diegosouzapw#3839, mimocode proxy diegosouzapw#3837, Arena ELO flag diegosouzapw#3821), 9 more Fixed entries (diegosouzapw#3811/diegosouzapw#3807/diegosouzapw#3759/diegosouzapw#3849/diegosouzapw#3838/diegosouzapw#3835/diegosouzapw#3814/diegosouzapw#3820/diegosouzapw#3819), a Security section (CCR IDOR diegosouzapw#3859, supply-chain diegosouzapw#3824), and an Internal/Quality section. Every contributor and issue reporter is now credited. * docs(changelog): restore + complete the v3.8.25 release notes Re-adds CHANGELOG.md (a prior server-side commit accidentally dropped it) with the complete, audited [3.8.25] section: New Features, the full Fixed list, Security & Hardening, and Internal/Quality — every contributor and issue reporter credited. * chore(release): finalize v3.8.25 — reconcile CHANGELOG + i18n mirrors, document OMNIROUTE_MAX_PENDING_MIGRATIONS, green the unit suite Release-gate reconciliation for v3.8.25: - CHANGELOG: dated 2026-06-14, linked diegosouzapw#3826, rolled up file-size re-baselines (diegosouzapw#3823/diegosouzapw#3833), recorded the test-greening; re-synced all 41 i18n CHANGELOG mirrors. - Documented OMNIROUTE_MAX_PENDING_MIGRATIONS (diegosouzapw#3416) in .env.example + ENVIRONMENT.md. - Greened the unit suite (was merged red on 4 CI shards): aligned 10 stale tests to this cycle's intended behavior (diegosouzapw#3838/diegosouzapw#3822/diegosouzapw#3501/SOCKS5/Vertex-Express/Antigravity) and the same-provider 503 fall-through test; de-flaked the compression benchmark reproducibility and ServiceSupervisor crash tests. No production code changed. * ci(security): clear OpenSSF Scorecard code-scanning noise + harden workflow token permissions The Security tab held 155 open alerts, ALL from the advisory OpenSSF Scorecard tool (diegosouzapw#3824) — supply-chain/posture scores, not code vulnerabilities — which drowned out real CodeQL findings. - scorecard.yml: stop uploading SARIF to the code-scanning tab (drop the upload-sarif step + the now-unused security-events: write). The run still produces the OpenSSF badge (publish_results) and a downloadable SARIF artifact. - TokenPermissions hardening (the high-severity, genuinely-valuable subset): set each workflow's top-level token to read-only and grant the exact writes at the job level that needs them — npm-publish (id-token/packages on publish jobs), docker-publish (packages on build), electron-release (contents on build/release, id-token/packages on publish-npm), build-fork (packages on build), claude (empty top-level; job grants its own). The 155 existing alerts were dismissed. Not adopting repo-wide SHA-pinning (143 PinnedDependencies advisories) — declined. * test(integration): align stale wiring/socks5 integration tests to this cycle's behavior These were red on the CI Integration job (pre-existing). No production code changed: - integration-wiring: the combos page no longer renders a per-page EmailPrivacyToggle (diegosouzapw#3822 consolidated it into Settings → Appearance); the provider-detail test-result masking and upstream-proxy copy moved to decomposed components (diegosouzapw#3501 BatchTestResultsModal / UpstreamProxyCard) — assertions now read the owning files. - api-routes-critical: SOCKS5 is now enabled by default (opt-out), so the disabled- rejection test must set ENABLE_SOCKS5_PROXY=false explicitly (an unset env now means enabled). (The ~32 live-Gemini integration tests are gated on OMNIROUTE_API_KEY and skip in CI; they only 'fail' locally when that key is present without a running server.)
* chore(release): continue v3.8.25 development cycle after main code-sync (r5) main fast-forwarded to release/v3.8.25 (diegosouzapw#3863): unblocked Build+Docker via diegosouzapw#3864, plus diegosouzapw#3837 (mimocode proxy) and diegosouzapw#3862 (trivy bump). This marker re-opens the umbrella PR for further v3.8.25 work. No version bump. * fix(db): persist the Keep-latest-backups retention setting (diegosouzapw#3834) (diegosouzapw#3867) * fix(oauth): clear GitLab Duo setup message instead of 500 (diegosouzapw#3861) (diegosouzapw#3868) * test(oauth): prove refresh_token preserved on real gemini-cli/antigravity dispatch (diegosouzapw#3850) (diegosouzapw#3869) * feat(compression-ui): unified compression config UI — per-engine pages + combos editor + menu + WS default-on (diegosouzapw#3860) Integrated into release/v3.8.25 — feat(compression-ui): unified compression configuration UI (Compression Hub + per-engine Lite/Aggressive/Ultra pages + combos editor + sidebar entry + live-WS default-on). File-size re-baselined for sidebarVisibility.ts/chatCore.ts growth; orphan ws test relocated to a collected path. * docs(changelog): complete the v3.8.25 release notes + credit all contributors Audited every commit since v3.8.24 and filled the gaps the [3.8.25] section was missing: a New Features section (compression engines + Compression Studios diegosouzapw#3848, compression UI diegosouzapw#3860, injection-guard diegosouzapw#3857, kiro discovery diegosouzapw#3836, Veo diegosouzapw#3839, mimocode proxy diegosouzapw#3837, Arena ELO flag diegosouzapw#3821), 9 more Fixed entries (diegosouzapw#3811/diegosouzapw#3807/diegosouzapw#3759/diegosouzapw#3849/diegosouzapw#3838/diegosouzapw#3835/diegosouzapw#3814/diegosouzapw#3820/diegosouzapw#3819), a Security section (CCR IDOR diegosouzapw#3859, supply-chain diegosouzapw#3824), and an Internal/Quality section. Every contributor and issue reporter is now credited. * docs(changelog): restore + complete the v3.8.25 release notes Re-adds CHANGELOG.md (a prior server-side commit accidentally dropped it) with the complete, audited [3.8.25] section: New Features, the full Fixed list, Security & Hardening, and Internal/Quality — every contributor and issue reporter credited. * chore(release): finalize v3.8.25 — reconcile CHANGELOG + i18n mirrors, document OMNIROUTE_MAX_PENDING_MIGRATIONS, green the unit suite Release-gate reconciliation for v3.8.25: - CHANGELOG: dated 2026-06-14, linked diegosouzapw#3826, rolled up file-size re-baselines (diegosouzapw#3823/diegosouzapw#3833), recorded the test-greening; re-synced all 41 i18n CHANGELOG mirrors. - Documented OMNIROUTE_MAX_PENDING_MIGRATIONS (diegosouzapw#3416) in .env.example + ENVIRONMENT.md. - Greened the unit suite (was merged red on 4 CI shards): aligned 10 stale tests to this cycle's intended behavior (diegosouzapw#3838/diegosouzapw#3822/diegosouzapw#3501/SOCKS5/Vertex-Express/Antigravity) and the same-provider 503 fall-through test; de-flaked the compression benchmark reproducibility and ServiceSupervisor crash tests. No production code changed. * ci(security): clear OpenSSF Scorecard code-scanning noise + harden workflow token permissions The Security tab held 155 open alerts, ALL from the advisory OpenSSF Scorecard tool (diegosouzapw#3824) — supply-chain/posture scores, not code vulnerabilities — which drowned out real CodeQL findings. - scorecard.yml: stop uploading SARIF to the code-scanning tab (drop the upload-sarif step + the now-unused security-events: write). The run still produces the OpenSSF badge (publish_results) and a downloadable SARIF artifact. - TokenPermissions hardening (the high-severity, genuinely-valuable subset): set each workflow's top-level token to read-only and grant the exact writes at the job level that needs them — npm-publish (id-token/packages on publish jobs), docker-publish (packages on build), electron-release (contents on build/release, id-token/packages on publish-npm), build-fork (packages on build), claude (empty top-level; job grants its own). The 155 existing alerts were dismissed. Not adopting repo-wide SHA-pinning (143 PinnedDependencies advisories) — declined. * test(integration): align stale wiring/socks5 integration tests to this cycle's behavior These were red on the CI Integration job (pre-existing). No production code changed: - integration-wiring: the combos page no longer renders a per-page EmailPrivacyToggle (diegosouzapw#3822 consolidated it into Settings → Appearance); the provider-detail test-result masking and upstream-proxy copy moved to decomposed components (diegosouzapw#3501 BatchTestResultsModal / UpstreamProxyCard) — assertions now read the owning files. - api-routes-critical: SOCKS5 is now enabled by default (opt-out), so the disabled- rejection test must set ENABLE_SOCKS5_PROXY=false explicitly (an unset env now means enabled). (The ~32 live-Gemini integration tests are gated on OMNIROUTE_API_KEY and skip in CI; they only 'fail' locally when that key is present without a running server.)
Closes #3834
Problem
In Settings → Database backup retention, changing "Keep latest backups" to anything other than 20 had no effect — it snapped back to 20 on refresh / after "Clean old backups". Editing
.envwas ignored too (Next does not reloadprocess.envafter start).Root cause:
getDbBackupMaxFiles()read onlyprocess.env.DB_BACKUP_MAX_FILES ?? 20. There was no setter and no persisted store, so the UI value was never saved. (A separatedatabaseSettings.backup.keepLastNBackups, default 5, exists but is never consumed by the cleanup logic — left untouched here to avoid surprising existing installs with a 5-backup default.)Fix
src/lib/db/backup.ts:getDbBackupMaxFiles()precedence is now env override → persisted value → default 20, backed by a dedicatedkey_valuenamespace (dbBackup/maxFiles). NewsetDbBackupMaxFiles()persists it. The dedicated namespace avoids cross-talk with thedatabaseSettingsstore (which rewrites all of its own keys on every update).src/app/api/db-backups/route.ts: the DELETE ("Clean old backups") handler persists the chosenkeepLatestso it survives the subsequentloadStorageHealth()refetch.src/lib/localDb.ts: re-exportsetDbBackupMaxFiles(re-export only).Existing installs keep the historical default of 20 until an operator explicitly changes it.
Tests (TDD)
tests/unit/db-backup-extended.test.ts: round-trip (set 5 → get 5 → set 12 → get 12), default-20-when-unset, and env-override-wins. The round-trip fails on the unfixed code (no setter; getter ignored any stored value).typecheck:coreclean, lint 0 errors, file-size clean.