Double-Check Mode #237
Double-Check Mode #237
Conversation
WalkthroughThis PR implements a double-check verification feature for the auto-mode workflow, including backend service updates to integrate SettingsService, new API endpoints for double-check operations, frontend UI for settings and board interactions, type definitions for double-check configurations, and styling updates across all themes. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User<br/>(Frontend)
participant UI as Board UI<br/>(React)
participant API as HTTP/Electron<br/>API Layer
participant Server as AutoModeService<br/>(Backend)
participant Settings as SettingsService
participant Agent as AgentProvider<br/>(AI Model)
participant Feature as Feature<br/>Storage
User->>UI: Click "Run Double-Check"<br/>on feature card
UI->>API: handleDoubleCheck(feature)
API->>Server: POST /api/auto-mode/double-check-feature<br/>{projectPath, featureId}
Server->>Settings: getGlobalSettings()
Settings-->>Server: doubleCheckMode config
Server->>Feature: loadFeature(featureId)
Feature-->>Server: feature data
rect rgb(135, 206, 250, 0.2)
Note over Server,Agent: Double-Check Verification Phase
Server->>Server: getDoubleCheckModel(strategy)
Server->>Server: buildDoubleCheckPrompt(feature)
Server->>Agent: Execute verification prompt<br/>with selected model
Agent-->>Server: [DOUBLE_CHECK_PASSED]<br/>or [DOUBLE_CHECK_FAILED]
end
Server->>Server: determineFinalStatus()<br/>parse results
Server->>Feature: updateFeatureWithDoubleCheckResult<br/>(passed/failed, summary, discrepancies)
Feature-->>Server: acknowledged
Server-->>API: {success, passed, summary, discrepancies}
API-->>UI: results
UI->>UI: Update feature status<br/>display discrepancies
UI-->>User: Show verification result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @NomadicDaddy, 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 introduces a significant new feature called 'Double-Check Mode,' designed to improve the quality and reliability of AI-implemented features. It provides an optional, configurable verification step where a secondary AI agent rigorously reviews completed work. This ensures that features meet specified criteria before proceeding to human approval, effectively acting as an automated quality gate. The implementation includes both front-end UI changes for configuration and status visualization, as well as robust back-end logic to manage the verification process and integrate seamlessly with the existing Auto Mode workflow. 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
|
![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 a significant new feature, "Double-Check Mode," which adds an optional AI-driven verification step for completed features. The implementation is comprehensive, with changes across the server, UI, and shared types. The overall approach is solid, but I've identified a critical bug in the server-side implementation that would prevent the double-check process from being stopped. Additionally, there are opportunities to improve code safety with better request validation and enhance maintainability by refactoring duplicated logic in the UI. I've also noted an unrelated functional change that disables model switching, which should be clarified. Expanding the new unit tests to cover the core doubleCheckFeature method would also be beneficial.
I am having trouble creating individual review comments. Click here to see my feedback.
apps/server/src/services/auto-mode-service.ts (555)
This abortController declaration shadows the one from the outer scope (line 521). The abortController from line 521 is stored in this.runningFeatures, but this new one is passed to executeQuery. This means stopFeature will not work for this operation as it will abort the wrong controller. You should remove this line and use the abortController from the outer scope.
apps/server/src/routes/agent/routes/model.ts (22-23)
This change disables the functionality to set the session model. The new implementation is a stub that always returns success. While the comment in the new code indicates this is intentional (// Model changes are not yet implemented), this is a significant functional change that seems unrelated to the Double-Check Mode feature. If this is a temporary measure, it might be better to handle it in a separate PR to avoid confusion. If it's permanent, the endpoint should probably be removed.
apps/server/src/routes/auto-mode/routes/double-check-feature.ts (12-15)
Using a type assertion as { ... } for the request body is not type-safe and can lead to runtime errors if the client sends an unexpected payload. It's better to use a validation library like Zod to parse and validate the request body. This ensures that projectPath and featureId are of the correct type and are present.
apps/server/tests/unit/services/auto-mode-service-double-check.test.ts (1-165)
It's great that you've added unit tests for the new double-check functionality. However, the tests currently cover helper methods but not the main doubleCheckFeature method itself. This method contains the core logic for the feature, including interactions with the provider, file system, and event emitters. Please consider adding tests for doubleCheckFeature to cover its success and failure paths, ensuring the logic is robust.
apps/ui/src/components/views/board-view.tsx (644-662)
The logic for filtering features by the current worktree branch is duplicated here and in other parts of this useEffect (lines 696-716 and 741-763). To improve maintainability and reduce code duplication, consider extracting this filtering logic into a reusable helper function. This would make the effect easier to read and ensure consistency across the different feature types (double-check, in-progress, backlog).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (12)
apps/server/src/routes/agent/routes/model.ts (1)
9-9: Consider removing the unused parameter.The
_agentServiceparameter is no longer used in the handler. If it's not required for interface compatibility, consider removing it entirely to clarify the intent.🔎 Proposed refactor
-export function createModelHandler(_agentService: AgentService) { +export function createModelHandler() { return async (req: Request, res: Response): Promise<void> => {If the parameter must remain for interface compatibility with other route handlers, add a comment explaining why:
+// Parameter kept for compatibility with route handler factory signature export function createModelHandler(_agentService: AgentService) {libs/types/src/feature.ts (1)
8-27: Well-structured type definition for double-check results.The
DoubleCheckResultinterface provides a clear schema for verification data. Consider the following for future enhancements:
- The
checkedAtfield is documented as an ISO timestamp but typed asstring. Consider using a branded type or adding runtime validation to ensure ISO 8601 format consistency.- The
discrepanciesarray uses plain strings. If more structured error reporting becomes necessary, consider defining a structured discrepancy type.These are optional improvements and don't block the current implementation.
apps/server/src/routes/auto-mode/routes/double-check-feature.ts (1)
32-35: Error message exposure in production.The current error handling returns the full error message to the client, which could leak sensitive implementation details in production. Consider using generic error messages for 500 errors.
🔎 Proposed improvement
} catch (error) { logError(error, 'Double-check feature failed'); - res.status(500).json({ success: false, error: getErrorMessage(error) }); + const isDevelopment = process.env.NODE_ENV === 'development'; + res.status(500).json({ + success: false, + error: isDevelopment ? getErrorMessage(error) : 'Internal server error' + }); }apps/server/tests/unit/services/auto-mode-service-double-check.test.ts (1)
67-148: Prefer exercising double-check behavior via public API instead of private methodsThese tests reach into
determineFinalStatus,getDoubleCheckModel,buildDoubleCheckPrompt, andextractTitleFromDescriptionvia(service as any), which makes them fragile against internal refactors. If there is (or will be) a public method that drives the double-check flow end-to-end, consider adding at least one higher-level test that asserts the same behavior through that entrypoint, keeping these private-method tests only where necessary.apps/ui/src/components/views/board-view/kanban-board.tsx (1)
89-103: Tighten store subscription and consider behavior when disabling Double-Check Mode
const { doubleCheckMode } = useAppStore();subscribes the board to the entire store and will re-render on any state change. You can avoid unnecessary renders by selecting just what you need, e.g.:Suggested selector usage
const doubleCheckMode = useAppStore((state) => state.doubleCheckMode);Also note that when
doubleCheckMode.enabledis turned off,visibleColumnsdrops thedouble_checkcolumn entirely. Any existing features indouble_checkwill effectively disappear from the Kanban view until the mode is re-enabled. If that’s not the desired UX, consider either:
- auto-transitioning such features out of
double_checkwhen disabling the mode, or- keeping the column visible but visually indicating that double-check is disabled.
apps/ui/src/components/views/settings-view/double-check/double-check-section.tsx (2)
37-48: Use a selector when reading fromuseAppStoreto limit re-renders
const { doubleCheckMode, setDoubleCheckMode } = useAppStore();will re-render this section on any store change. To keep settings UI snappy, it’s better to subscribe only to what you need, e.g.:Suggested selector
const { doubleCheckMode, setDoubleCheckMode } = useAppStore((state) => ({ doubleCheckMode: state.doubleCheckMode, setDoubleCheckMode: state.setDoubleCheckMode, }));
73-97: Settings description should match all model strategies, not just “different model”The help text under “Enable Double-Check Verification” says “A different model reviews the implementation…”, but users can also choose “Specific Model” or “Same Model”. Consider rephrasing to something strategy-agnostic like “A verification model reviews the implementation…” so the copy stays correct regardless of the selected strategy.
apps/ui/src/components/views/board-view/components/kanban-card/card-actions.tsx (2)
17-36: Remove or wire up unusedonMoveBackToInProgressprop
onMoveBackToInProgressis declared inCardActionsPropsand passed down fromKanbanCard, but it isn’t used anywhere in this component. Either hook it into an appropriate action (e.g., a “Send back to In Progress” control) or drop the prop to avoid confusion and keep the surface area lean.
293-330: Double-check action buttons are correctly integratedThe new “Run Double-Check” and “Skip” buttons for
feature.status === 'double_check'are consistent with the existing action patterns (guarded by handler presence, stop event propagation, and include test IDs). The behavior and styling look coherent with the rest of the card actions.If you expect double-checks to be long-running, you might optionally disable these buttons or show a busy state while a double-check is in progress to prevent duplicate requests.
apps/ui/src/components/views/board-view/hooks/use-board-actions.ts (2)
494-555: Double-check and skip handlers align well with existing board actions
handleDoubleChecksensibly guards against missing APIs, surfaces progress via toasts, and reloads features on success so status transitions driven by the server are reflected in the UI.handleSkipDoubleCheckperforms a straightforward transition towaiting_approvaland persists it.If you later attach more double-check-specific state (e.g.,
doubleCheckResultdisplay on cards), consider clearing or updating that alongside the status inhandleSkipDoubleCheckto keep the client view fully in sync with whatever the server stores.
808-937: Consider applying concurrency checks to double-checks triggered viahandleStartNextFeatures
handleStartNextFeaturesnow prioritizesdouble_checkfeatures ahead of backlog, which matches the intended flow. However, the new double-check branch runs beforeavailableSlotsis computed and the concurrency guard is applied, so pressing “Start Next” can launch a double-check even whenrunningAutoTasks.lengthis already atmaxConcurrency.If you want the UI behavior to respect concurrency limits uniformly (including verification work), you could:
- Compute
availableSlotsand bail early when<= 0before entering the double-check branch, and- Optionally reuse
availableSlotsto cap how many double-checks you start, mirroring what you already do later for backlog items.This would make manual “Start Next” behavior consistent with the auto-mode scheduler’s slot accounting.
apps/server/src/services/auto-mode-service.ts (1)
360-373: Consider differentiating error scenarios in logs.The method returns
nullfor both missing service and load failures. While the error is logged, callers cannot distinguish between "settings service unavailable" and "settings load failed."🔎 Optional: Add context to error logs
private async loadGlobalSettings(): Promise<GlobalSettings | null> { if (!this.settingsService) { + console.warn('[AutoMode] Settings service not available, double-check mode disabled'); return null; } try { return await this.settingsService.getGlobalSettings(); } catch (error) { - console.error('[AutoMode] Failed to load global settings:', error); + console.error('[AutoMode] Failed to load global settings (double-check may not work):', error); return null; } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (50)
apps/server/src/index.tsapps/server/src/routes/agent/routes/model.tsapps/server/src/routes/auto-mode/index.tsapps/server/src/routes/auto-mode/routes/double-check-feature.tsapps/server/src/services/auto-mode-service.tsapps/server/src/services/settings-service.tsapps/server/src/types/settings.tsapps/server/tests/unit/services/auto-mode-service-double-check.test.tsapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/components/kanban-card/card-actions.tsxapps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsxapps/ui/src/components/views/board-view/constants.tsapps/ui/src/components/views/board-view/hooks/use-board-actions.tsapps/ui/src/components/views/board-view/hooks/use-board-column-features.tsapps/ui/src/components/views/board-view/kanban-board.tsxapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/views/settings-view/config/navigation.tsapps/ui/src/components/views/settings-view/double-check/double-check-section.tsxapps/ui/src/components/views/settings-view/double-check/index.tsapps/ui/src/components/views/settings-view/hooks/use-settings-view.tsapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/lib/electron.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/store/app-store.tsapps/ui/src/styles/global.cssapps/ui/src/styles/themes/blossom.cssapps/ui/src/styles/themes/cream.cssapps/ui/src/styles/themes/dark.cssapps/ui/src/styles/themes/forest.cssapps/ui/src/styles/themes/github.cssapps/ui/src/styles/themes/gray.cssapps/ui/src/styles/themes/gruvboxlight.cssapps/ui/src/styles/themes/lavender.cssapps/ui/src/styles/themes/mint.cssapps/ui/src/styles/themes/nordlight.cssapps/ui/src/styles/themes/ocean.cssapps/ui/src/styles/themes/paper.cssapps/ui/src/styles/themes/peach.cssapps/ui/src/styles/themes/rose.cssapps/ui/src/styles/themes/sand.cssapps/ui/src/styles/themes/sepia.cssapps/ui/src/styles/themes/sky.cssapps/ui/src/styles/themes/snow.cssapps/ui/src/styles/themes/solarizedlight.cssapps/ui/src/styles/themes/sunset.cssapps/ui/src/types/electron.d.tslibs/types/src/event.tslibs/types/src/feature.tslibs/types/src/index.tslibs/types/src/settings.ts
🧰 Additional context used
🧬 Code graph analysis (12)
apps/ui/src/components/views/settings-view.tsx (2)
apps/ui/src/components/views/settings-view/double-check/double-check-section.tsx (1)
DoubleCheckSection(37-253)apps/ui/src/components/views/settings-view/double-check/index.ts (1)
DoubleCheckSection(1-1)
apps/server/tests/unit/services/auto-mode-service-double-check.test.ts (1)
apps/server/src/services/auto-mode-service.ts (1)
AutoModeService(337-2934)
apps/server/src/services/settings-service.ts (2)
libs/types/src/index.ts (1)
DEFAULT_GLOBAL_SETTINGS(76-76)libs/types/src/settings.ts (1)
DEFAULT_GLOBAL_SETTINGS(454-485)
libs/types/src/feature.ts (4)
libs/types/src/index.ts (3)
DoubleCheckResult(24-24)AgentModel(42-42)FeatureStatus(23-23)apps/server/src/types/settings.ts (1)
AgentModel(11-11)libs/types/src/settings.ts (1)
AgentModel(12-12)libs/types/src/model.ts (1)
AgentModel(23-23)
apps/server/src/index.ts (1)
apps/server/src/services/auto-mode-service.ts (1)
AutoModeService(337-2934)
apps/ui/src/components/views/settings-view/double-check/double-check-section.tsx (5)
libs/types/src/settings.ts (2)
DoubleCheckModelStrategy(80-80)DoubleCheckMode(88-97)apps/ui/src/components/views/settings-view/double-check/index.ts (1)
DoubleCheckSection(1-1)apps/ui/src/store/app-store.ts (1)
useAppStore(967-2735)apps/ui/src/hooks/use-settings-migration.ts (1)
syncSettingsToServer(195-241)apps/ui/src/lib/utils.ts (1)
cn(5-7)
apps/server/src/routes/auto-mode/index.ts (2)
apps/server/src/middleware/validate-paths.ts (1)
validatePathParams(22-69)apps/server/src/routes/auto-mode/routes/double-check-feature.ts (1)
createDoubleCheckFeatureHandler(9-37)
apps/ui/src/store/app-store.ts (3)
apps/server/src/types/settings.ts (1)
DoubleCheckMode(16-16)libs/types/src/index.ts (1)
DoubleCheckMode(62-62)libs/types/src/settings.ts (1)
DoubleCheckMode(88-97)
apps/ui/src/components/views/board-view/components/kanban-card/card-actions.tsx (1)
apps/ui/src/components/ui/button.tsx (1)
Button(108-108)
apps/ui/src/components/views/board-view.tsx (1)
apps/ui/src/store/app-store.ts (1)
useAppStore(967-2735)
apps/ui/src/components/views/board-view/hooks/use-board-actions.ts (4)
apps/ui/src/store/app-store.ts (1)
Feature(257-279)libs/types/src/feature.ts (1)
Feature(46-81)apps/ui/src/lib/electron.ts (1)
getElectronAPI(639-648)apps/ui/src/lib/utils.ts (1)
truncateDescription(32-37)
apps/ui/src/components/views/board-view/kanban-board.tsx (4)
apps/ui/src/store/app-store.ts (2)
Feature(257-279)useAppStore(967-2735)libs/types/src/feature.ts (1)
Feature(46-81)apps/ui/src/components/views/board-view/constants.ts (1)
COLUMNS(5-27)apps/ui/src/hooks/use-responsive-kanban.ts (1)
useResponsiveKanban(48-194)
🔇 Additional comments (62)
apps/server/src/routes/agent/routes/model.ts (2)
14-22: LGTM: Improved validation clarity.Splitting the validation into separate checks provides more specific error messages to API clients, making it easier to debug issues.
27-27: Error message updated for consistency.The error message now aligns with the updated endpoint description, though given that the endpoint no longer performs model changes, this catch block would only handle validation errors.
libs/types/src/event.ts (1)
22-22: LGTM!The new
'feature:updated'event type is appropriately placed among feature-related events and follows the existing naming convention. This enables the UI to react to feature state changes during the double-check workflow.apps/ui/src/types/electron.d.ts (1)
444-453: LGTM!The
doubleCheckFeaturemethod signature is well-designed and consistent with other AutoModeAPI methods. The return type appropriately includes verification-specific fields (passed,summary,discrepancies) alongside standard success/error handling.apps/ui/src/components/views/board-view/hooks/use-board-column-features.ts (2)
29-29: LGTM!The new
double_checkcolumn is correctly initialized in the column features map, consistent with the existing column structure.
83-89: LGTM!Good logic here — features in
double_checkstatus remain in their designated column even when running, while still showing the running indicator via theisCurrentAutoTaskprop. This correctly separates the visual status (column placement) from the execution state (running indicator).libs/types/src/settings.ts (3)
73-97: LGTM!Well-documented type definitions with clear JSDoc explanations. The
DoubleCheckModelStrategyunion type andDoubleCheckModeinterface comprehensively cover the configuration options described in the PR objectives.
291-293: LGTM!The
doubleCheckModefield is correctly added toGlobalSettingswith appropriate documentation.
470-474: LGTM!Sensible default values: feature disabled by default, uses a different model for verification (reducing bias), and auto-triggers during Auto Mode for seamless integration.
apps/ui/src/styles/themes/blossom.css (1)
73-73: LGTM!The purple/violet hue (280) provides good visual distinction from the in-progress (yellow) and waiting (orange) status colors. The lightness and chroma values are consistent with the theme's palette.
apps/ui/src/styles/themes/peach.css (1)
73-73: LGTM!Consistent with other light themes. The purple hue provides good contrast against the peach theme's warm color palette.
apps/ui/src/styles/themes/dark.css (1)
84-84: LGTM!The increased lightness (0.7) and chroma (0.18) appropriately adapt the double-check status color for dark mode visibility, following the same pattern as other status colors in this theme.
apps/ui/src/styles/themes/github.css (1)
73-73: LGTM!The slightly lower lightness (0.6) aligns with the GitHub theme's more muted color palette while maintaining the consistent purple hue for the double-check status.
apps/ui/src/lib/http-api-client.ts (1)
599-608: LGTM! Clean API wrapper implementation.The
doubleCheckFeaturemethod follows the established pattern for other Auto Mode API methods, with proper TypeScript typing and appropriate HTTP verb usage.apps/ui/src/components/views/settings-view.tsx (1)
19-19: LGTM! Consistent integration of the new settings section.The import and switch case follow the established pattern for other settings sections. The implementation is clean and maintainable.
Also applies to: 147-148
apps/ui/src/hooks/use-settings-migration.ts (1)
225-225: LGTM! Proper inclusion of doubleCheckMode in settings sync.The addition follows the established pattern for syncing global settings fields. The deep merge logic is correctly handled server-side in
settings-service.ts.apps/ui/src/styles/themes/mint.css (1)
73-73: LGTM! Consistent status color addition.The new
--status-double-checkcustom property is properly placed among other status colors and follows the theme's color palette conventions.apps/ui/src/styles/themes/gruvboxlight.css (1)
73-73: LGTM! Consistent theming across all CSS theme files.The
--status-double-checkcolor token has been consistently added across multiple theme files (gruvboxlight, sepia, rose, and mint), each with theme-appropriate color adjustments while maintaining the same hue family. This ensures the double-check status will have proper visual representation across all themes.apps/server/src/services/settings-service.ts (1)
137-140: LGTM! Proper deep merge implementation for nested settings.The deep merge logic for
doubleCheckModecorrectly follows the same pattern established forkeyboardShortcuts. This ensures that partial updates to the nesteddoubleCheckModeobject won't inadvertently remove unspecified fields, maintaining proper defaults and backward compatibility.Also applies to: 176-182
apps/ui/src/components/views/board-view/constants.ts (1)
12-16: LGTM! Well-positioned workflow column.The new
double_checkcolumn is correctly positioned in the workflow sequence and follows the established pattern for column definitions. The color variable reference aligns with the theme updates across the codebase.apps/ui/src/styles/themes/snow.css (1)
73-73: LGTM! Consistent theme color addition.The new status color variable follows the theme's color palette and format conventions.
apps/ui/src/components/views/settings-view/double-check/index.ts (1)
1-1: LGTM! Standard barrel export pattern.The export follows the established pattern for component modules.
apps/ui/src/styles/themes/sunset.css (1)
82-82: LGTM! Theme-appropriate color addition.The color value fits well within the Sunset theme's palette and maintains consistency with other status colors.
apps/ui/src/styles/themes/sand.css (1)
73-73: LGTM! Consistent theme integration.The status color addition maintains the Sand theme's color harmony.
apps/ui/src/styles/themes/lavender.css (1)
73-73: LGTM! Theme-consistent color value.The purple-hued status color aligns perfectly with the Lavender theme's soft purple/lavender palette.
apps/ui/src/styles/themes/paper.css (1)
73-73: LGTM! Clean minimal theme addition.The status color maintains the Paper theme's clean and minimal aesthetic.
apps/ui/src/components/views/settings-view/config/navigation.ts (2)
12-12: LGTM! Appropriate icon import.The
CheckCheckicon from lucide-react is semantically appropriate for the double-check feature.
32-32: LGTM! Well-structured navigation item.The new navigation item follows the established pattern and uses consistent kebab-case naming for the id. The positioning before the Danger Zone is logical for a feature-related setting.
apps/server/src/routes/auto-mode/index.ts (1)
20-20: LGTM! Clean integration of the double-check feature route.The new route follows the established pattern with appropriate path validation for
projectPath. The handler performs its own validation forfeatureId(as seen in the relevant code snippets), which is correct sincefeatureIdis not a file path and doesn't require path traversal protection.Also applies to: 67-71
apps/ui/src/styles/themes/forest.css (1)
73-73: LGTM! Consistent status color addition.The new
--status-double-checkvariable is properly placed within the status color group and uses an appropriate purple hue (280) that visually distinguishes it from other status colors in the Forest theme.apps/ui/src/components/views/settings-view/hooks/use-settings-view.ts (1)
12-12: LGTM! Clean type extension.The addition of
'double-check'to theSettingsViewIdunion type properly enables navigation to the new Double-Check settings section and follows the existing pattern.apps/ui/src/styles/themes/ocean.css (1)
73-73: LGTM! Consistent theme extension.The
--status-double-checkvariable addition matches the Ocean theme's color palette and maintains consistency with other status color definitions.apps/ui/src/lib/electron.ts (1)
326-335: LGTM! Well-structured API method addition.The
doubleCheckFeaturemethod follows the established pattern of other AutoModeAPI methods with appropriate parameter types and a comprehensive return type that includes both success and error states, along with optional verification details (passed,summary,discrepancies).apps/ui/src/styles/themes/nordlight.css (1)
73-73: LGTM! Theme-appropriate color adjustment.The
--status-double-checkvariable is properly adjusted for the Nord Light theme with slightly lower chroma and lightness values that harmonize with the theme's softer color palette.apps/ui/src/styles/themes/cream.css (1)
82-82: LGTM! Warm theme color harmony.The
--status-double-checkvariable is appropriately adjusted for the Cream theme's warm color palette while maintaining the purple hue (280) that distinguishes the double-check status across all themes.apps/ui/src/styles/themes/sky.css (1)
73-73: LGTM! Sky theme integration complete.The
--status-double-checkvariable is properly integrated into the Sky theme with appropriate color values that maintain visual consistency with the theme's light blue color scheme.apps/ui/src/styles/themes/solarizedlight.css (1)
73-73: LGTM!The new
--status-double-checkcolor token is well-chosen. The purple hue (280) provides good visual distinction from existing status colors and maintains consistency with the overall theme palette.apps/ui/src/styles/themes/gray.css (1)
82-82: LGTM!The color value is appropriately adjusted for the gray theme while maintaining the same purple hue (280) as other themes for consistency.
libs/types/src/feature.ts (2)
79-79: LGTM!The optional
doubleCheckResultfield integrates cleanly with the existing Feature interface without breaking changes.
83-89: LGTM!The new
double_checkstatus extends the workflow appropriately and aligns with the documented feature flow: implementation → double_check → waiting_approval/backlog.apps/server/src/types/settings.ts (1)
15-16: LGTM!Clean re-export of double-check types maintains consistency with the existing re-export pattern in this file.
apps/ui/src/styles/global.css (2)
115-115: LGTM!The color alias follows the established pattern for status tokens and integrates properly with the theme system.
193-193: LGTM!The light and dark mode values are well-balanced. The dark mode uses higher lightness (0.7 vs 0.6) which ensures proper contrast on dark backgrounds, following best practices for accessible color systems.
Also applies to: 292-292
apps/server/src/index.ts (1)
117-117: LGTM!Clean dependency injection of
SettingsServiceintoAutoModeService. This enables the double-check workflow to access global settings while maintaining testability and clear separation of concerns.libs/types/src/index.ts (2)
19-25: LGTM!The export of
DoubleCheckResultproperly expands the public type surface for the double-check feature.
55-62: LGTM!Clean addition of double-check configuration types to the public settings API surface.
apps/server/src/routes/auto-mode/routes/double-check-feature.ts (1)
12-23: Path validation is already in place via middleware; focus type guard addition on featureId.The
/double-check-featureroute already applies thevalidatePathParams('projectPath')middleware (line 69 in auto-mode/index.ts), which validates the projectPath using thevalidatePath()function from '@automaker/platform'. This middleware prevents path traversal attacks before the handler is invoked.However,
featureIdlacks runtime validation. Add a type guard to ensure it's a non-empty string:if (!projectPath || !featureId) { + if (!projectPath || !featureId || typeof featureId !== 'string') {Likely an incorrect or invalid review comment.
apps/ui/src/components/views/board-view/kanban-board.tsx (1)
45-46: Double-check callbacks are cleanly plumbed through KanbanBoard
onDoubleCheckandonSkipDoubleCheckare threaded into the props and down toKanbanCardconsistently; the board stays nicely declarative and keeps action logic in the hooks layer.Also applies to: 79-80, 203-204
apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx (1)
28-29: Draggability and double-check callbacks are wired correctlyIncluding
double_checkinisDraggableand threadingonDoubleCheck/onSkipDoubleCheckthrough toCardActionsmatches the updatedFeature['status']union and board workflows. No issues spotted with the prop surface or drag behavior.Also applies to: 50-58, 70-75, 168-188
apps/ui/src/store/app-store.ts (1)
10-11: Double-Check Mode state and persistence are cohesiveThe additions of
double_checktoFeature['status'],doubleCheckMode: DoubleCheckModeonAppState, thesetDoubleCheckModeaction, initial defaults, and inclusion inpartializeall line up. Given thepersistmerge behavior ({ ...current, ...persisted }), older persisted stores will naturally pick up the newdoubleCheckModedefaults without a dedicated migration. This looks ready to support the rest of the double-check workflow.Also applies to: 267-273, 485-487, 758-760, 931-935, 1561-1565, 2709-2710
apps/ui/src/components/views/board-view.tsx (3)
598-848: Auto-mode scheduler priority for double-checks and resumes matches the intended flowThe updated auto-mode loop now:
- Respects
maxConcurrencyviaavailableSlots.- Prioritizes
double_checkfeatures whendoubleCheckMode.enabled, honoringautoTriggerInAutoMode(auto-run vs. pause-until-manual).- Uses
featuresWithContextRefto resumein_progressfeatures with saved context before touching new backlog work.- Falls back to backlog selection with the same branch and dependency-filtering logic used elsewhere.
The use of refs (
autoModeRunningRef,hookFeaturesRef,runningAutoTasksRef,handleStartImplementationRef,handleDoubleCheckRef,handleResumeFeatureRef,featuresWithContextRef) keeps the effect stable while still always calling the latest handlers, which is a solid pattern for this polling loop.
1122-1155: BoardView correctly propagates double-check handlers into KanbanBoard
handleDoubleCheckandhandleSkipDoubleCheckare passed straight through toKanbanBoard, keeping BoardView focused on orchestration while the column/card components stay purely presentational. This aligns well with the existing separation of concerns.
560-582: No issue found. ThedoubleCheckFeatureimplementation in the AutoModeService properly emits the sameAutoModeEventtypes that the UI listener expects:
auto_mode_feature_startis emitted when double-check begins (cleanup removes from pending)auto_mode_feature_completeis emitted on both pass and fail outcomes (cleanup removes from pending)auto_mode_erroris emitted when an exception occurs (cleanup removes from pending)All three event types are already handled by the switch statement in
board-view.tsx(lines 577-591), and all code paths throughdoubleCheckFeatureemit one of these events. There is no risk of pending entries accumulating indefinitely.Likely an incorrect or invalid review comment.
apps/server/src/services/auto-mode-service.ts (9)
346-358: LGTM: Clean dependency injection pattern.The optional
settingsServiceparameter and setter method enable flexible initialization while maintaining backward compatibility.
375-395: LGTM: Clear status determination logic.The method correctly prioritizes automated testing, then double-check, then manual approval. The unused
_projectPathparameter follows the underscore convention for future extensibility.
423-493: LGTM: Well-structured verification prompt and title extraction.The double-check prompt provides a comprehensive checklist, and the title extraction handles truncation gracefully.
666-700: LGTM: Proper result persistence and event emission.The method correctly updates the feature with double-check results and emits the appropriate event.
1012-1030: LGTM: Proper integration of final status determination.The double-check workflow is correctly integrated into feature execution, with appropriate status messages for user feedback.
1294-1309: LGTM: Consistent status handling in follow-up flow.The follow-up feature completion correctly applies the same status determination logic.
1849-1862: LGTM: Proper status transition handling.The
justFinishedAttimestamp correctly marks when features complete implementation (enteringdouble_checkorwaiting_approval), anddoubleCheckResultis appropriately cleared when returning to the backlog for re-implementation.
1956-1987: LGTM: Clean helper for loading features by status.The method follows the same pattern as
loadPendingFeaturesand correctly filters features by status.
397-421: Add fallback for emptydifferentModelsarray.In the
'different'case (line 408-415), ifdifferentModelsis empty (edge case where all available models matchusedModel), line 414 would returnundefined, causing type issues.🔎 Proposed fix
case 'different': { // Select a different model from the one that implemented the feature const availableModels: AgentModel[] = ['opus', 'sonnet', 'haiku']; const usedModel = (feature.model as AgentModel) || 'sonnet'; const differentModels = availableModels.filter((m) => m !== usedModel); // Prefer sonnet if available, otherwise use first different model - return differentModels.includes('sonnet') ? 'sonnet' : differentModels[0]; + return differentModels.includes('sonnet') ? 'sonnet' : (differentModels[0] || 'sonnet'); }Likely an incorrect or invalid review comment.
| // Model changes are not yet implemented | ||
| res.json({ success: true, message: 'Model change acknowledged' }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the model.ts file
find . -path "*apps/server/src/routes/agent/routes/model.ts" -type f
# List the directory structure to understand the layout
ls -la apps/server/src/routes/agent/routes/ 2>/dev/null || echo "Directory not found, searching for model.ts files"
# Search for model.ts files
fd "model\.ts$" apps/server/src/routes/agent/ 2>/dev/null || fd "model\.ts$" . --max-depth 10 2>/dev/null | grep -E "(agent|model)"Repository: AutoMaker-Org/automaker
Length of output: 618
🏁 Script executed:
#!/bin/bash
# Read the model.ts file with line numbers
cat -n apps/server/src/routes/agent/routes/model.tsRepository: AutoMaker-Org/automaker
Length of output: 1280
🏁 Script executed:
#!/bin/bash
# Search for setSessionModel usage throughout codebase
echo "=== AgentService.setSessionModel usage ==="
rg -n "setSessionModel" --type=ts --type=tsx -B2 -A2
echo -e "\n=== Check AgentService implementation ==="
find . -name "*agent-service*" -type f | head -5
echo -e "\n=== Search for /model endpoint calls in client code ==="
rg -n "\/model|\.post.*model" apps/client apps/web --type=ts --type=tsx -C2 2>/dev/null | head -40
echo -e "\n=== Check route registration for model endpoint ==="
rg -n "model" apps/server/src/routes/agent --type=ts -B1 -A1 | head -30Repository: AutoMaker-Org/automaker
Length of output: 2225
🏁 Script executed:
#!/bin/bash
# Check AgentService implementation
echo "=== AgentService methods ==="
cat -n apps/server/src/services/agent-service.ts | head -100
echo -e "\n=== Search for setSession or model-related methods ==="
rg -n "setSession|getSession" apps/server/src/services/agent-service.ts -B1 -A5
echo -e "\n=== Check route index to see how model handler is registered ==="
cat -n apps/server/src/routes/agent/index.ts
echo -e "\n=== Check tests for model endpoint ==="
rg -n "model.*endpoint\|POST.*model" apps/server/tests --type=ts -C2Repository: AutoMaker-Org/automaker
Length of output: 5577
🏁 Script executed:
#!/bin/bash
# Look at the Session interface to understand model property
echo "=== Session interface definition ==="
cat -n apps/server/src/services/agent-service.ts | sed -n '33,53p'
# Check if setSessionModel was being called elsewhere before
echo -e "\n=== Full setSessionModel implementation ==="
rg -n "setSessionModel" apps/server/src/services/agent-service.ts -B1 -A10
# Check if there are any TODOs or FIXMEs related to model
echo -e "\n=== Search for TODOs/FIXMEs about model ==="
rg -n "TODO|FIXME" apps/server/src/routes/agent/routes/model.ts
# Check git log for recent changes to model.ts
echo -e "\n=== Recent git changes to model.ts ==="
git -C . log --oneline -10 -- "apps/server/src/routes/agent/routes/model.ts" 2>/dev/null || echo "Git log not available"
# Check if send.ts uses the model parameter
echo -e "\n=== How send.ts uses model parameter ==="
cat -n apps/server/src/routes/agent/routes/send.ts | sed -n '1,50p'Repository: AutoMaker-Org/automaker
Length of output: 3172
🏁 Script executed:
#!/bin/bash
# Check the full sendMessage implementation to see if it uses model
echo "=== AgentService.sendMessage with model parameter ==="
rg -n "sendMessage" apps/server/src/services/agent-service.ts -B2 -A20 | head -60
# Check how the model parameter affects the agent execution
echo -e "\n=== Search for where model is used in message processing ==="
rg -n "model" apps/server/src/services/agent-service.ts | grep -E "model\?" || echo "No model usage found"
# Check tests to see what behavior is expected
echo -e "\n=== Test files for agent routes ==="
find apps/server/tests -name "*model*" -o -name "*agent*route*" 2>/dev/null | head -10
# Check the commit message for Double-Check Mode
echo -e "\n=== Recent commits (last 5) ==="
git -C . log --oneline -5 2>/dev/null || echo "Git not available"Repository: AutoMaker-Org/automaker
Length of output: 1037
🏁 Script executed:
#!/bin/bash
# Check if there are any client calls to the /model endpoint
echo "=== Search client code for /model endpoint calls ==="
find apps/client apps/web -type f \( -name "*.ts" -o -name "*.js" \) 2>/dev/null | xargs grep -l "model" 2>/dev/null | head -5 | while read file; do
echo "File: $file"
grep -n "\/model\|setModel\|changeModel" "$file" 2>/dev/null | head -3
done
# Check the git diff for the model.ts file to see what was changed
echo -e "\n=== Git diff for model.ts in latest commit ==="
git -C . show 64c8623:apps/server/src/routes/agent/routes/model.ts 2>/dev/null || echo "Previous version not available"
echo -e "\n=== Check for any commented out code in model.ts ==="
cat -n apps/server/src/routes/agent/routes/model.ts | grep -i "agentService\|setSession"
# Check if there are integration tests
echo -e "\n=== Integration tests for agent routes ==="
find apps/server/tests -name "*.test.ts" -exec grep -l "model\|/model" {} \; 2>/dev/nullRepository: AutoMaker-Org/automaker
Length of output: 2069
🏁 Script executed:
#!/bin/bash
# Check the Double-Check Mode test to understand context
echo "=== Double-Check Mode test file (first 50 lines) ==="
head -50 apps/server/tests/unit/services/auto-mode-service-double-check.test.ts
# Check if there's any reference to model endpoint in tests
echo -e "\n=== All references to /model or model endpoint in tests ==="
rg -n "model.*endpoint\|POST.*model\|\/model" apps/server/tests --type=ts
# Check the PR description or commit message more fully
echo -e "\n=== Commit details for 64c8623 ==="
git -C . log --format=fuller 64c8623 2>/dev/null | head -30
# Check README or any documentation about model switching
echo -e "\n=== Check for documentation about model changes ==="
find . -name "README*" -o -name "*.md" 2>/dev/null | xargs grep -l "model" 2>/dev/null | head -3 | while read f; do
echo "=== File: $f ==="
grep -i "model.*change\|switch.*model\|POST.*model" "$f" 2>/dev/null | head -3
doneRepository: AutoMaker-Org/automaker
Length of output: 2258
🏁 Script executed:
#!/bin/bash
# Check the full commit message
echo "=== Full commit message ==="
git -C . log -1 --format="%B" 64c8623 2>/dev/null
# Check if model.ts was added or modified in this commit
echo -e "\n=== File status in commit ==="
git -C . diff-tree --no-commit-id --name-status -r 64c8623 2>/dev/null | grep -i model
# Check if there are any client API calls throughout the codebase
echo -e "\n=== All HTTP POST calls in client code ==="
find . -path ./node_modules -prune -o -type f \( -name "*.ts" -o -name "*.js" \) -exec grep -l "fetch.*POST\|axios.*post\|\.post(" {} \; 2>/dev/null | grep -E "(client|web)" | head -5
# Check if there's an API client or service that calls the model endpoint
echo -e "\n=== Search for API client files ==="
find apps/client apps/web -name "*api*" -o -name "*client*" 2>/dev/null | head -10
# Final check: see if send endpoint has any relation to model
echo -e "\n=== Full sendMessage implementation to see model usage ==="
rg -n "async sendMessage" apps/server/src/services/agent-service.ts -A 80 | head -100Repository: AutoMaker-Org/automaker
Length of output: 3043
🏁 Script executed:
#!/bin/bash
# Check the http-api-client to see if /model endpoint is used there
echo "=== HTTP API Client implementation ==="
cat apps/ui/src/lib/http-api-client.ts
# Verify if there are any UI components calling the model endpoint
echo -e "\n=== Search UI for model endpoint usage ==="
find apps/ui/src -type f -name "*.ts" 2>/dev/null | xargs grep -l "model" 2>/dev/null | head -5Repository: AutoMaker-Org/automaker
Length of output: 32541
🏁 Script executed:
#!/bin/bash
# Final verification - check if any code references the /model endpoint
echo "=== Direct references to /api/agent/model ==="
rg -r "api/agent/model" --type=ts --type=js . 2>/dev/null | head -20
# Check if model parameter in send is actually being used correctly
echo -e "\n=== How sendMessage uses the model parameter ==="
rg -n "async sendMessage" apps/server/src/services/agent-service.ts -A 50 | grep -E "model|session\.model" | head -10Repository: AutoMaker-Org/automaker
Length of output: 7607
Endpoint returns success without performing any action.
The /model endpoint accepts sessionId and model parameters but never uses them—the _agentService parameter (underscore-prefixed) is unused, and the handler returns { success: true, message: '...' } unconditionally without calling setSessionModel(). This is deceptive: clients will believe the model was changed when it wasn't.
The AgentService.setSessionModel() method exists and works correctly (confirmed in lines 502-510), but the endpoint never invokes it. The model parameter CAN be changed via the /send endpoint (lines 136-138), which properly updates the session model—but the dedicated /model endpoint is non-functional and should either be completed or removed.
🤖 Prompt for AI Agents
In apps/server/src/routes/agent/routes/model.ts around lines 24-25, the endpoint
currently returns success without using the provided sessionId/model or the
injected _agentService; update the handler to validate sessionId and model, call
agentService.setSessionModel(sessionId, model) (remove the underscore to use the
parameter), await its result, and return a success response only if the call
succeeds; also add error handling to return appropriate 4xx/5xx responses on
validation failure or if setSessionModel throws/fails.
| /** | ||
| * Execute double-check verification on a feature | ||
| */ | ||
| async doubleCheckFeature( | ||
| projectPath: string, | ||
| featureId: string | ||
| ): Promise<{ passed: boolean; summary?: string; discrepancies?: string[] }> { | ||
| const feature = await this.featureLoader.get(projectPath, featureId); | ||
| if (!feature) { | ||
| throw new Error(`Feature ${featureId} not found`); | ||
| } | ||
|
|
||
| if (feature.status !== 'double_check') { | ||
| throw new Error(`Feature ${featureId} is not in double_check status`); | ||
| } | ||
|
|
||
| const settings = await this.loadGlobalSettings(); | ||
| if (!settings) { | ||
| throw new Error('Unable to load settings'); | ||
| } | ||
|
|
||
| const model = this.getDoubleCheckModel(feature, settings); | ||
| const prompt = this.buildDoubleCheckPrompt(feature); | ||
| const modelString = resolveModelString(model, DEFAULT_MODELS.claude); | ||
|
|
||
| // Add to running features so UI shows it as active | ||
| const abortController = new AbortController(); | ||
| this.runningFeatures.set(featureId, { | ||
| featureId, | ||
| projectPath, | ||
| worktreePath: null, | ||
| branchName: feature.branchName || null, | ||
| abortController, | ||
| isAutoMode: false, // Manual double-check, not auto mode | ||
| startTime: Date.now(), | ||
| }); | ||
|
|
||
| // Emit start event so UI shows the running indicator | ||
| this.emitAutoModeEvent('auto_mode_feature_start', { | ||
| featureId, | ||
| projectPath, | ||
| }); | ||
|
|
||
| this.emitAutoModeEvent('auto_mode_progress', { | ||
| featureId, | ||
| projectPath, | ||
| content: `Starting double-check verification with model: ${model}`, | ||
| }); | ||
|
|
||
| try { | ||
| // Load context files for the project | ||
| const { formattedPrompt: contextFilesPrompt } = await loadContextFiles({ | ||
| projectPath, | ||
| fsModule: secureFs as Parameters<typeof loadContextFiles>[0]['fsModule'], | ||
| }); | ||
|
|
||
| // Get the provider for the model | ||
| const provider = ProviderFactory.getProviderForModel(modelString); | ||
|
|
||
| // Execute the double-check | ||
| const abortController = new AbortController(); | ||
| const executeOptions: ExecuteOptions = { | ||
| prompt, | ||
| model: modelString, | ||
| cwd: projectPath, | ||
| maxTurns: 50, // Limited turns for verification | ||
| abortController, | ||
| systemPrompt: contextFilesPrompt || undefined, | ||
| }; | ||
|
|
||
| const stream = provider.executeQuery(executeOptions); | ||
| let resultText = ''; | ||
|
|
||
| // Collect the streaming response | ||
| for await (const msg of stream) { | ||
| if (msg.type === 'assistant' && msg.message?.content) { | ||
| for (const block of msg.message.content) { | ||
| if (block.type === 'text') { | ||
| resultText += block.text || ''; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| const passed = resultText.includes('[DOUBLE_CHECK_PASSED]'); | ||
| const failed = resultText.includes('[DOUBLE_CHECK_FAILED]'); | ||
|
|
||
| // Extract summary and discrepancies | ||
| let summary: string | undefined; | ||
| let discrepancies: string[] | undefined; | ||
|
|
||
| if (passed) { | ||
| const passedIndex = resultText.indexOf('[DOUBLE_CHECK_PASSED]'); | ||
| summary = resultText.substring(passedIndex + '[DOUBLE_CHECK_PASSED]'.length).trim(); | ||
| } else if (failed) { | ||
| const failedIndex = resultText.indexOf('[DOUBLE_CHECK_FAILED]'); | ||
| const failedContent = resultText | ||
| .substring(failedIndex + '[DOUBLE_CHECK_FAILED]'.length) | ||
| .trim(); | ||
| // Try to extract discrepancies as bullet points | ||
| const lines = failedContent | ||
| .split('\n') | ||
| .filter((l) => l.trim().startsWith('-') || l.trim().match(/^\d+\./)); | ||
| discrepancies = lines | ||
| .map((l) => l.replace(/^[-\d.]+\s*/, '').trim()) | ||
| .filter((l) => l.length > 0); | ||
| summary = | ||
| discrepancies.length > 0 | ||
| ? `Found ${discrepancies.length} issue(s)` | ||
| : failedContent.substring(0, 200); | ||
| } | ||
|
|
||
| // Create the double-check result | ||
| const doubleCheckResult: DoubleCheckResult = { | ||
| status: passed ? 'passed' : 'failed', | ||
| discrepancies: discrepancies, | ||
| checkedAt: new Date().toISOString(), | ||
| checkedBy: model, | ||
| originalModel: (feature.model as AgentModel) || 'sonnet', | ||
| summary: summary, | ||
| }; | ||
|
|
||
| // Update feature with result and new status | ||
| // If double-check failed, send back to backlog so auto-mode can pick it up for re-implementation | ||
| const newStatus = passed ? 'waiting_approval' : 'backlog'; | ||
| await this.updateFeatureWithDoubleCheckResult( | ||
| projectPath, | ||
| featureId, | ||
| doubleCheckResult, | ||
| newStatus | ||
| ); | ||
|
|
||
| // Remove from running features | ||
| this.runningFeatures.delete(featureId); | ||
|
|
||
| // Emit appropriate event | ||
| if (passed) { | ||
| this.emitAutoModeEvent('auto_mode_feature_complete', { | ||
| featureId, | ||
| projectPath, | ||
| passes: true, | ||
| message: `Double-check passed: ${summary || 'No issues found'}`, | ||
| }); | ||
| } else { | ||
| this.emitAutoModeEvent('auto_mode_feature_complete', { | ||
| featureId, | ||
| projectPath, | ||
| passes: false, | ||
| message: `Double-check failed: ${summary || 'Issues found'}`, | ||
| }); | ||
| } | ||
|
|
||
| return { passed, summary, discrepancies }; | ||
| } catch (error) { | ||
| // Remove from running features on error | ||
| this.runningFeatures.delete(featureId); | ||
|
|
||
| console.error(`[AutoMode] Double-check failed for ${featureId}:`, error); | ||
| const errorInfo = classifyError(error); | ||
|
|
||
| // Keep in double_check status on error | ||
| this.emitAutoModeEvent('auto_mode_error', { | ||
| featureId, | ||
| error: `Double-check failed: ${errorInfo.message}`, | ||
| errorType: errorInfo.type, | ||
| projectPath, | ||
| }); | ||
|
|
||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: Abort controller shadowing prevents stopping double-check.
Line 555 creates a new AbortController that shadows the one created at line 521. The first controller is stored in runningFeatures (line 522-530), but the second is passed to executeOptions (line 562). As a result, calling stopFeature(featureId) would abort the wrong controller, and the double-check operation cannot be stopped.
🔎 Proposed fix
const model = this.getDoubleCheckModel(feature, settings);
const prompt = this.buildDoubleCheckPrompt(feature);
const modelString = resolveModelString(model, DEFAULT_MODELS.claude);
// Add to running features so UI shows it as active
const abortController = new AbortController();
this.runningFeatures.set(featureId, {
featureId,
projectPath,
worktreePath: null,
branchName: feature.branchName || null,
abortController,
isAutoMode: false, // Manual double-check, not auto mode
startTime: Date.now(),
});
// ... emit events ...
try {
// Load context files for the project
const { formattedPrompt: contextFilesPrompt } = await loadContextFiles({
projectPath,
fsModule: secureFs as Parameters<typeof loadContextFiles>[0]['fsModule'],
});
// Get the provider for the model
const provider = ProviderFactory.getProviderForModel(modelString);
// Execute the double-check
- const abortController = new AbortController();
const executeOptions: ExecuteOptions = {
prompt,
model: modelString,
cwd: projectPath,
maxTurns: 50, // Limited turns for verification
abortController,
systemPrompt: contextFilesPrompt || undefined,
};🤖 Prompt for AI Agents
In apps/server/src/services/auto-mode-service.ts around lines 495 to 664, the
AbortController created at line ~521 is shadowed by a second declaration around
line ~555, so the controller stored in runningFeatures is not the one passed to
provider.executeQuery; remove the second "const abortController = new
AbortController()" and reuse the earlier abortController when building
executeOptions so the same controller stored in runningFeatures is the one that
gets aborted by stopFeature(featureId); keep the rest of executeOptions the same
and ensure no other references expect the inner variable.
| // Check for features awaiting double-check (if auto-trigger is enabled) | ||
| const settings = await this.loadGlobalSettings(); | ||
| const doubleCheckEnabled = settings?.doubleCheckMode?.enabled; | ||
| const autoTrigger = settings?.doubleCheckMode?.autoTriggerInAutoMode; | ||
| console.log('[AutoMode] Double-check settings:', { | ||
| enabled: doubleCheckEnabled, | ||
| autoTriggerInAutoMode: autoTrigger, | ||
| }); | ||
|
|
||
| // Always check for double-check features first, regardless of auto-trigger setting | ||
| // This ensures double-check items are processed before new backlog items | ||
| const doubleCheckFeatures = await this.loadFeaturesWithStatus( | ||
| this.config!.projectPath, | ||
| 'double_check' | ||
| ); | ||
| console.log('[AutoMode] Found double-check features:', doubleCheckFeatures.length); | ||
|
|
||
| if (doubleCheckFeatures.length > 0 && doubleCheckEnabled) { | ||
| const nextDoubleCheck = doubleCheckFeatures.find( | ||
| (f: Feature) => !this.runningFeatures.has(f.id) | ||
| ); | ||
|
|
||
| if (nextDoubleCheck) { | ||
| if (autoTrigger) { | ||
| console.log('[AutoMode] Auto-processing double-check feature:', nextDoubleCheck.id); | ||
| // Run double-check in background | ||
| this.doubleCheckFeature(this.config!.projectPath, nextDoubleCheck.id).catch( | ||
| (error) => { | ||
| console.error( | ||
| `[AutoMode] Double-check feature ${nextDoubleCheck.id} error:`, | ||
| error | ||
| ); | ||
| } | ||
| ); | ||
| await this.sleep(2000); | ||
| continue; | ||
| } else { | ||
| // Double-check items exist but auto-trigger is off - wait for manual trigger | ||
| // Don't start new backlog items while double-check items are pending | ||
| console.log( | ||
| '[AutoMode] Double-check feature waiting for manual trigger:', | ||
| nextDoubleCheck.id | ||
| ); | ||
| await this.sleep(5000); | ||
| continue; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider handling features stuck in double_check status when mode is disabled.
Lines 758-764 always load double_check features but only process them if doubleCheckEnabled=true. If double-check mode is disabled after features are already in double_check status, those features will be skipped by the auto-loop and remain stuck.
🔎 Suggested handling
Consider either:
- Automatically moving
double_checkfeatures back towaiting_approvalwhen double-check mode is disabled, or - Processing them as
waiting_approvalregardless of the setting (since they're already in the queue).
Example approach:
// Always check for double-check features first, regardless of auto-trigger setting
// This ensures double-check items are processed before new backlog items
const doubleCheckFeatures = await this.loadFeaturesWithStatus(
this.config!.projectPath,
'double_check'
);
console.log('[AutoMode] Found double-check features:', doubleCheckFeatures.length);
- if (doubleCheckFeatures.length > 0 && doubleCheckEnabled) {
+ if (doubleCheckFeatures.length > 0) {
+ if (!doubleCheckEnabled) {
+ // Double-check disabled but features are stuck - move to waiting_approval
+ console.log('[AutoMode] Double-check disabled, moving features to waiting_approval');
+ for (const feature of doubleCheckFeatures) {
+ await this.updateFeatureStatus(this.config!.projectPath, feature.id, 'waiting_approval');
+ }
+ continue;
+ }
const nextDoubleCheck = doubleCheckFeatures.find(
(f: Feature) => !this.runningFeatures.has(f.id)
);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/server/src/services/auto-mode-service.ts around lines 747 to 794, the
loop currently always loads features with status "double_check" but only
processes them when doubleCheckEnabled is true, which leaves features stuck when
mode is later disabled; modify the logic so that when doubleCheckEnabled is
false and there are features in "double_check" you either (A) programmatically
move those features back to "waiting_approval" by calling the existing
status-update method (persist the change and log it) or (B) treat them as
"waiting_approval" for processing by the auto-loop (i.e., enqueue/process them
immediately as if their status were waiting_approval) — implement one of these
flows, add clear logs for the transition, and ensure any DB/feature-cache is
updated so they are not ignored on subsequent iterations.
Shironex
added
Enhancement
2 participants
Double-Check Mode
Adds an optional verification step where a second AI agent reviews completed features before they're approved. Useful for non-test driven modes or you just don't trust one model's take...
Features
Configurable in Settings - Enable/disable double-check mode with model selection options:
New "Double-Check" column on the board - Features pending verification appear here
Auto Mode Integration:
Flow
Changes
Summary by CodeRabbit
Release Notes
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.