fix: use transaction context for database update in start command

[kangju2000]

• Fixed a bug where database update was using direct db instead of transaction context tx
• This ensures proper atomicity when cancelling active intents before starting new ones

[gemini-code-assist]

Summary of Changes

Hello @kangju2000, I'm Gemini Code Assist¹! 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 resolves a critical bug in the start command's database interaction. It ensures that all database operations intended to be part of a single transaction are correctly executed within that transaction's context, thereby guaranteeing atomicity for related updates.

Highlights

• Bug Fix: Corrected a bug in the start command where a database update for cancelling active intents was incorrectly using the global db object instead of the transaction context (tx).
• Improved Atomicity: By ensuring the database update occurs within the transaction context, the operation of cancelling active intents before starting new ones is now properly atomic, preventing potential data inconsistencies.

Using Gemini Code Assist

The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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 or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request correctly fixes a bug where a database update was performed outside of a transaction, ensuring atomicity for the operation. The change from db.update to tx.update is essential for data integrity.

My review includes one high-severity comment regarding a potential race condition that still exists in the logic. While the current change is a step in the right direction for transactional safety, I've pointed out how the application is still vulnerable to a TOCTOU (Time-of-check to time-of-use) issue and suggested a way to resolve it by moving the data read into the transaction.

[gemini-code-assist]

This change correctly uses the transaction context tx, which is a great fix for ensuring the update is part of the transaction.

However, a race condition (TOCTOU - Time-of-check to time-of-use) remains because activeIntent is fetched before the transaction begins.

Here's a possible scenario that leads to data inconsistency:

1. Process A calls findActiveIntent and finds no active intent.
2. Process B calls findActiveIntent, also finds no active intent, then creates a new active intent and commits its transaction.
3. Process A starts its transaction. It is unaware of the intent created by Process B and proceeds to create another active intent.

This results in two active intents for the same branchId, which likely violates the application's invariants.

To resolve this, the check for an active intent should be moved inside the transaction. Since drizzle-orm/better-sqlite3 transactions are synchronous, you would need to use the synchronous query builder API instead of the async findActiveIntent function.

For example, the query inside the transaction would look like this:

const activeIntent = tx
  .select()
  .from(intents)
  .where(and(eq(intents.branchId, branchId), eq(intents.status, "active")))
  .get();

This would require refactoring the start function. Given the importance of data integrity, addressing this race condition is highly recommended.