Conversation
…restart - Add graceful shutdown handler that marks running features as 'interrupted' before server exit (SIGTERM/SIGINT) - Add 30-second shutdown timeout to prevent hanging on exit - Add orphan detection to identify features with missing branches - Add isFeatureRunning() for idempotent resume checks - Improve resumeInterruptedFeatures() to handle features without saved context - Add 'interrupted' status to FeatureStatusWithPipeline type - Replace console.log with proper logger in auto-mode-service - Add comprehensive unit tests for all new functionality (15 new tests) Fixes #696 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @Shironex, 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 significantly improves the robustness of feature management by addressing scenarios where features could become unresumable after unexpected server interruptions. It introduces a comprehensive graceful shutdown process to correctly mark active features, implements a system to identify and log features whose underlying Git branches have been removed, and refines the feature resumption logic to be more resilient and prevent concurrent operations from causing issues. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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. 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
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThe PR introduces graceful shutdown handling with feature interruption detection, adds orphan feature detection on list requests, implements reference counting for running features, and expands the feature status type to include 'interrupted'. Comprehensive tests for new AutoModeService functionality are included. Changes
Sequence DiagramssequenceDiagram
participant Server as Server (Restart)
participant AMS as AutoModeService
participant FL as FeatureLoader
participant Git as Git Repository
participant FS as File System
Server->>AMS: Startup - discover running features
Note over AMS: Iterate through persisted features<br/>with 'in_progress' status
AMS->>FL: getAll(projectPath)
FL-->>AMS: [Feature with status='in_progress']
AMS->>Git: getExistingBranches(projectPath)
Git-->>AMS: Set of branch names
alt Branch exists
AMS->>AMS: Feature is valid
else Branch missing (orphaned)
AMS->>AMS: Mark as orphaned
end
AMS->>AMS: markAllRunningFeaturesInterrupted()
AMS->>FS: updateFeatureStatus(featureId, 'interrupted')
FS-->>AMS: Saved
AMS->>Server: Ready to serve (features resumable)
sequenceDiagram
participant Client as Client
participant API as /list endpoint
participant AMS as AutoModeService
participant FL as FeatureLoader
participant Git as Git
Client->>API: GET /list?projectPath=...
API->>API: Validate projectPath (new)
API->>FL: Load features
FL-->>API: Feature list
API->>AMS: detectOrphanedFeatures(projectPath)<br/>(background, non-blocking)
par Background Detection
AMS->>Git: getExistingBranches(projectPath)
Git-->>AMS: Branch set
AMS->>AMS: Find features with missing branches
AMS->>AMS: Log orphaned features
end
API-->>Client: Return feature list
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
There was a problem hiding this comment.
Code Review
This is an excellent pull request that addresses a critical issue of features getting stuck after a server restart. The implementation of a graceful shutdown handler, orphan detection for feature branches, and idempotent feature resumption logic significantly improves the application's robustness and reliability. The code quality has also been enhanced with better JSDoc documentation and consistent use of the logger. The addition of comprehensive unit tests for the new functionality is also a great practice. I've identified a couple of minor areas where the error handling could be simplified due to the robustness of the new methods. Overall, this is a high-quality contribution.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/server/src/services/auto-mode-service.ts`:
- Around line 3090-3152: markFeatureInterrupted currently unconditionally
overwrites a feature's status to 'interrupted', which clobbers pipeline_*
statuses and prevents resumePipelineFeature from picking up the correct step;
change markFeatureInterrupted (and the callers in
markAllRunningFeaturesInterrupted that use it) to detect pipeline statuses and
preserve them instead of overwriting — either skip calling updateFeatureStatus
for statuses that start with "pipeline_" or save the original status into a
metadata field (e.g., previousStatus or interruptedFlag) before marking
interrupted, using updateFeatureStatus only for non-pipeline statuses; refer to
markFeatureInterrupted, updateFeatureStatus, markAllRunningFeaturesInterrupted,
runningFeatures, and resumePipelineFeature when making the change.
… service - Simplified the graceful shutdown process by removing redundant error handling for marking features as interrupted, as it is now managed internally. - Updated orphan detection logging to streamline the process and enhance clarity. - Added logic to preserve specific pipeline statuses when marking features as interrupted, ensuring correct resumption of features after a server restart. - Enhanced unit tests to cover new behavior for preserving pipeline statuses and handling various feature states.
Summary
in_progressstatus after server restart, making them unresumableinterruptedbefore exitChanges
interruptedon SIGTERM/SIGINT with 30-second timeoutisFeatureRunning()check prevents duplicate resume attemptsinterruptedtoFeatureStatusWithPipelineFiles Modified
apps/server/src/index.ts- Graceful shutdown with timeoutapps/server/src/services/auto-mode-service.ts- Core feature resumption and orphan detection logicapps/server/src/routes/features/routes/list.ts- Orphan detection on project loadapps/server/src/routes/features/index.ts- Wire up autoModeService to list handlerapps/server/tests/unit/services/auto-mode-service.test.ts- 15 new unit testslibs/types/src/pipeline.ts- AddedinterruptedstatusTest plan
Fixes #696
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.