fix(db): throw instead of silently ignoring initDatabase with differe…#445
fix(db): throw instead of silently ignoring initDatabase with differe…#445HiddenPuppy wants to merge 1 commit intomainfrom
Conversation
…nt workspace path When initDatabase is called a second time with a different workspace path, the call was silently ignored (short-circuited by `if (db) return`) and writes would continue going to the originally-opened database with no feedback. Now initDatabase tracks the initialized workspace path and throws an explicit error when called with a different path, guiding callers to use reinitDatabase instead. reinitDatabase also updates the stored workspace path so that subsequent initDatabase calls use the new path as the baseline. Closes #392 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a silent failure issue where the database initialization would ignore subsequent calls with different workspace paths. By introducing path tracking, the system now enforces consistency and provides clear feedback to developers when an incorrect initialization sequence is attempted, ensuring that database operations are correctly routed to the intended workspace. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Hi @HiddenPuppy, DetailsInstructions for interacting with me using comments are available here. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces tracking for the initialized workspace path in the database module to prevent accidental re-initialization with a different path without an explicit reset. It also includes unit tests to verify this behavior. Feedback suggests normalizing file paths before comparison to handle variations in path strings and using a try...finally block in closeDatabase to ensure state variables are consistently reset even if the database closure fails.
| export function initDatabase(workspacePath: string): void { | ||
| if (db) return; | ||
| if (db) { | ||
| if (initializedWorkspacePath !== workspacePath) { |
There was a problem hiding this comment.
Comparing raw strings for file paths can be fragile. Differences in trailing slashes (e.g., /path/to/ws vs /path/to/ws/) or relative vs. absolute paths will cause this check to fail even if they point to the same directory. It is recommended to normalize the workspacePath (e.g., using path.resolve()) before storing and comparing it to ensure the check is robust.
| sqlite?.close(); | ||
| sqlite = null; | ||
| db = null; | ||
| initializedWorkspacePath = null; |
There was a problem hiding this comment.
If sqlite?.close() throws an error, the subsequent lines to reset sqlite, db, and initializedWorkspacePath will be skipped, leaving the module in an inconsistent state. Using a try...finally block ensures these variables are always reset regardless of whether the close operation succeeds.
try {
sqlite?.close();
} finally {
sqlite = null;
db = null;
initializedWorkspacePath = null;
}
Deploying cpwa with
|
| Latest commit: |
b457645
|
| Status: | 🚫 Build failed. |
|
Thanks @HiddenPuppy! Looks like this is superseded by #446, which bundles the same |
2 participants
Summary
initDatabasenow tracks the workspace path used on first initialization. Previously, calling it a second time with a different path was silently ignored (short-circuited byif (db) return), causingwrites to continue going to the original database with no feedback. Now it throws an explicit error guiding callers to use
reinitDatabaseinstead.Type of change
[Feat]new feature[Fix]bug fix[UI]UI or UX change[Docs]documentation-only change[Refactor]internal cleanup[Build]CI, packaging, or tooling change[Chore]maintenanceWhy is this needed?
Issue #392: After
initDatabase('/a'), callinginitDatabase('/b')was silently ignored. Writes kept going to/a's database, making it very painful to debug "why aren't my writes showing up."What changed?
packages/desktop/src/main/db/index.ts: AddedinitializedWorkspacePathvariable.initDatabasenow detects path mismatches and throws.reinitDatabaseandcloseDatabasealso sync reset thisvariable.
packages/desktop/test/db.test.ts: Added 5 unit tests covering all scenarios.Architecture impact
Linked issues
Closes #392
Validation
pnpm lintpnpm test— 97 passedpnpm typecheckpnpm buildScreenshots or recordings
No UI changes.
Release note