fix: system backup history queries use wrong column names

[Yeraze]

Summary

Fixes "View Saved Backups" failing with SqliteError: no such column: backupPath on all database backends.

Root cause: The v3.7 baseline migration creates system_backup_history with camelCase columns (backupPath, backupType, totalSize, tableCount, appVersion, schemaVersion), but during the async refactoring the INSERT and SELECT queries were rewritten to use snake_case names (dirname, type, size, table_count, meshmonitor_version, schema_version). This meant:

• INSERTs silently failed — no backup history was ever recorded
• SELECTs threw "no such column" when listing backups

Why tests didn't catch it: The backup system test only checks HTTP 200 and that a backup dirname appears in the list. Since INSERTs were failing, listBackups returned empty arrays (no SQL error). The API exercise test only checks status codes.

Fixes #2419

Changes

File	Change
src/server/services/systemBackupService.ts	Fixed all INSERT, SELECT, DELETE queries to use baseline camelCase column names with proper PG quoting

Test plan

• npx vitest run — all tests pass
• npm run build — no TypeScript errors
• Create a backup, verify it appears in "View Saved Backups"
• Verify on PostgreSQL backend

🤖 Generated with Claude Code

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Code Review

Reviewing PR #2421: fix: system backup history queries use wrong column names

Tasks

• Read and analyze the changed file
• Review column naming fixes and database compatibility
• Check for potential bugs or edge cases
• Assess performance and security implications
• Evaluate test coverage and suggest improvements
• Provide comprehensive review feedback

---

Review Summary ✅

This PR successfully fixes a critical bug where backup history operations were failing due to column name mismatches. The fix is well-implemented and addresses the root cause effectively.

Code Quality Assessment

Excellent Fix Quality ✅

• Root Cause Analysis: Correctly identified that v3.7 migration created camelCase columns but queries used snake_case
• Database Schema Verification: Confirmed against schema definitions in src/db/schema/misc.ts - all column names match the baseline camelCase convention:
  • backupPath, backupType, totalSize, tableCount, appVersion, schemaVersion
• Multi-Database Support: Properly handles PostgreSQL quoted identifiers with "backupPath" syntax
• Consistency: All query locations updated uniformly (INSERT at lines 236-239, SELECT at line 368, DELETE at lines 472-497)

Technical Analysis

Database Compatibility ✅
The fix correctly handles all three database backends:

• SQLite: Uses unquoted camelCase column names
• PostgreSQL: Uses quoted camelCase identifiers ("backupPath")
• MySQL: Uses unquoted camelCase column names

Query Analysis ✅

• recordBackupInDatabase() lines 236-239: Fixed INSERT columns to match schema
• listBackups() line 368: Fixed SELECT columns with proper PG quoting
• deleteBackup() lines 472-497: Fixed WHERE clause column references

Performance & Security

Performance: ✅ No performance impact - same query patterns, just corrected column names
Security: ✅ No security implications - fix maintains existing parameterized query approach

Test Coverage Gaps 🔍

The PR description identifies a critical testing gap:

Why tests didn't catch it: The backup system test only checks HTTP 200 and that a backup dirname appears in the list. Since INSERTs were failing, listBackups returned empty arrays (no SQL error).

Recommendations for Better Test Coverage:

1. Database Integration Tests: Add tests that verify actual data persistence, not just API responses
2. Multi-Backend Testing: Test the same operations against SQLite, PostgreSQL, and MySQL
3. Backup History Validation: Assert that backup records are actually created and retrievable
4. Column Name Validation: Add schema validation tests to catch column name mismatches

Minor Suggestions

1. Line 365: Consider extracting the column quoting logic into a helper method since it's used multiple times:

   private getColumnName(name: string, dbType: string): string {
     return dbType === 'postgres' ? `"${name}"` : name;
   }

2. Error Handling: The current approach is solid, but consider adding specific error messages that mention column name issues for future debugging.

Verification Checklist

Before merging, please verify:

• Create a backup and confirm it appears in "View Saved Backups"
• Test on PostgreSQL backend (as mentioned in PR description)
• Verify backup deletion functionality works
• Run full backup/restore cycle to ensure end-to-end functionality

Conclusion

This is a well-executed bug fix that:

• ✅ Correctly identifies and fixes the root cause
• ✅ Maintains database compatibility across all backends
• ✅ Uses proper PostgreSQL identifier quoting
• ✅ Updates all affected query locations consistently
• ✅ Follows existing code patterns and conventions

Recommendation: APPROVE - This fix resolves a critical functionality bug with no apparent risks.

--- | Branch: fix/backup-history-column-names