feat: automaticly load claude.md files and settings json globally / per project #267
Conversation
- Introduced a new setting to enable automatic loading of CLAUDE.md files from project-specific directories. - Updated relevant services and components to support the new setting, including the AgentService and AutoModeService. - Added UI controls for managing the auto-load setting in the settings view. - Enhanced SDK options to incorporate settingSources for CLAUDE.md loading. - Updated global and project settings interfaces to include autoLoadClaudeMd property.
- Updated API routes to accept an optional settings service for loading the autoLoadClaudeMd setting. - Introduced a new settings helper utility for retrieving project-specific settings. - Enhanced feature generation and spec generation processes to utilize the autoLoadClaudeMd setting. - Refactored relevant route handlers to support the new settings integration across various endpoints.
Shironex
added
Enhancement
|
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. 📝 WalkthroughWalkthroughWires an opt-in autoLoadClaudeMd setting and SettingsService through types, SDK option factories, services, route handlers, provider options, and UI; adds a preset systemPrompt shape and settingSources to ExecuteOptions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Router as Route Handler
participant Settings as SettingsService
participant Helper as getAutoLoadClaudeMdSetting
participant Service as Agent/AutoModeService
participant Provider as Claude Provider / SDK
Client->>Router: HTTP request (e.g., /generate, /describe)
Router->>Settings: (injected) optional SettingsService reference
Router->>Helper: getAutoLoadClaudeMdSetting(projectPath, settingsService)
Helper-->>Router: autoLoadClaudeMd (true|false)
Router->>Service: start job (includes autoLoadClaudeMd, settingSources, systemPrompt)
Service->>Provider: create*Options(autoLoadClaudeMd, settingSources, systemPrompt)
Provider->>Provider: initialize or resume session (may auto-load CLAUDE.md)
Provider-->>Service: stream/result
Service-->>Router: emit events / persist outputs
Router-->>Client: 200 Accepted (background) / eventual events
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
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 enhances the Claude Agent's ability to understand and adhere to project-specific guidelines by introducing automatic loading of 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 great new feature for automatically loading CLAUDE.md files and settings from .claude/ directories at both a global and per-project level. The implementation is comprehensive, involving changes across the server and UI to support the new autoLoadClaudeMd setting. The refactoring of SDK option creation is clean and centralizes the new logic well.
My review includes a few suggestions to improve maintainability by removing duplicated code, enhancing error handling, and cleaning up some development logging. I also noticed a small typo in the PR title ("automaticly" should be "automatically"). Overall, this is a solid contribution.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apps/server/src/lib/settings-helpers.ts (1)
16-45: Consider usingcreateLoggerfor consistency.The helper uses
console.log/console.errordirectly, while other server modules (e.g.,describe-image.ts,generate.ts) usecreateLoggerfrom@automaker/utils. For consistency and structured logging:🔎 Suggested refactor using createLogger
+import { createLogger } from '@automaker/utils'; import type { SettingsService } from '../services/settings-service.js'; +const logger = createLogger('SettingsHelper'); + export async function getAutoLoadClaudeMdSetting( projectPath: string, settingsService?: SettingsService, logPrefix = '[SettingsHelper]' ): Promise<boolean> { if (!settingsService) { - console.log(`${logPrefix} SettingsService not available, autoLoadClaudeMd disabled`); + logger.info(`${logPrefix} SettingsService not available, autoLoadClaudeMd disabled`); return false; } try { const projectSettings = await settingsService.getProjectSettings(projectPath); if (projectSettings.autoLoadClaudeMd !== undefined) { - console.log( + logger.info( `${logPrefix} autoLoadClaudeMd from project settings: ${projectSettings.autoLoadClaudeMd}` ); return projectSettings.autoLoadClaudeMd; } const globalSettings = await settingsService.getGlobalSettings(); const result = globalSettings.autoLoadClaudeMd ?? false; - console.log(`${logPrefix} autoLoadClaudeMd from global settings: ${result}`); + logger.info(`${logPrefix} autoLoadClaudeMd from global settings: ${result}`); return result; } catch (error) { - console.error(`${logPrefix} Failed to load autoLoadClaudeMd setting:`, error); + logger.error(`${logPrefix} Failed to load autoLoadClaudeMd setting:`, error); return false; } }apps/ui/src/store/app-store.ts (1)
1557-1563: Consider error handling for server sync failure.The action updates local state and then syncs to server, but doesn't handle sync failures. If
syncSettingsToServerfails, the UI state will be out of sync with the server. WhilesyncSettingsToServerlogs errors internally, the UI won't provide feedback to users.This may be acceptable for a non-critical setting, but consider whether you want to:
- Show a toast/notification on sync failure
- Roll back the local state on failure
🔎 Example with error feedback (optional)
setAutoLoadClaudeMd: async (enabled) => { set({ autoLoadClaudeMd: enabled }); - // Sync to server settings file const { syncSettingsToServer } = await import('@/hooks/use-settings-migration'); - await syncSettingsToServer(); + const success = await syncSettingsToServer(); + if (!success) { + // Optionally: roll back or notify user + console.warn('[AppStore] Failed to sync autoLoadClaudeMd to server'); + } },apps/server/src/lib/sdk-options.ts (1)
176-186: Consider using structured logging instead of console.log.The codebase appears to use
createLoggerfrom@automaker/utilselsewhere. Usingconsole.loghere is inconsistent with the logging patterns in other files (e.g.,generate-suggestions.tsusescreateLogger).🔎 Suggested improvement
+import { createLogger } from '@automaker/utils'; + +const logger = createLogger('SdkOptions'); + function buildClaudeMdOptions(config: CreateSdkOptionsConfig): { // ... - console.log( - '[SDK Options] CLAUDE.md auto-loading enabled:', - JSON.stringify( - { - systemPrompt: result.systemPrompt, - settingSources: result.settingSources, - }, - null, - 2 - ) - ); + logger.debug('CLAUDE.md auto-loading enabled:', { + systemPrompt: result.systemPrompt, + settingSources: result.settingSources, + });apps/server/src/services/agent-service.ts (1)
607-636: LGTM! Well-structured settings retrieval with proper precedence.The method correctly implements project-over-global precedence with safe fallbacks. Error handling ensures the feature fails closed (disabled) if settings cannot be loaded.
Optional consideration: The
console.logstatements on lines 621-623 and 630 will emit on every message. If high message throughput is expected, consider using debug-level logging or removing these after the feature stabilizes.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
.gitignoreapps/server/src/index.tsapps/server/src/lib/sdk-options.tsapps/server/src/lib/settings-helpers.tsapps/server/src/providers/claude-provider.tsapps/server/src/providers/types.tsapps/server/src/routes/app-spec/generate-features-from-spec.tsapps/server/src/routes/app-spec/generate-spec.tsapps/server/src/routes/app-spec/index.tsapps/server/src/routes/app-spec/routes/generate-features.tsapps/server/src/routes/app-spec/routes/generate.tsapps/server/src/routes/context/index.tsapps/server/src/routes/context/routes/describe-file.tsapps/server/src/routes/context/routes/describe-image.tsapps/server/src/routes/github/index.tsapps/server/src/routes/github/routes/validate-issue.tsapps/server/src/routes/suggestions/generate-suggestions.tsapps/server/src/routes/suggestions/index.tsapps/server/src/routes/suggestions/routes/generate.tsapps/server/src/services/agent-service.tsapps/server/src/services/auto-mode-service.tsapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/views/settings-view/claude/claude-md-settings.tsxapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/store/app-store.tslibs/types/src/provider.tslibs/types/src/settings.ts
🧰 Additional context used
🧬 Code graph analysis (12)
apps/server/src/routes/suggestions/routes/generate.ts (4)
apps/server/src/routes/templates/common.ts (1)
logger(8-8)apps/server/src/routes/app-spec/routes/generate.ts (1)
createGenerateHandler(20-94)apps/server/src/lib/events.ts (1)
EventEmitter(10-13)apps/server/src/routes/suggestions/generate-suggestions.ts (1)
generateSuggestions(126-265)
apps/server/src/routes/suggestions/index.ts (3)
apps/server/src/lib/events.ts (1)
EventEmitter(10-13)apps/server/src/services/settings-service.ts (1)
SettingsService(104-583)apps/server/src/routes/suggestions/routes/generate.ts (1)
createGenerateHandler(14-59)
apps/server/src/routes/suggestions/generate-suggestions.ts (1)
apps/server/src/lib/sdk-options.ts (1)
createSuggestionsOptions(303-320)
apps/server/src/routes/app-spec/routes/generate.ts (2)
apps/server/src/routes/suggestions/routes/generate.ts (1)
createGenerateHandler(14-59)apps/server/src/services/settings-service.ts (1)
SettingsService(104-583)
apps/ui/src/components/views/settings-view/claude/claude-md-settings.tsx (1)
apps/ui/src/lib/utils.ts (1)
cn(5-7)
apps/ui/src/components/views/settings-view.tsx (1)
apps/ui/src/components/views/settings-view/claude/claude-md-settings.tsx (1)
ClaudeMdSettings(25-82)
apps/server/src/lib/settings-helpers.ts (2)
apps/server/src/services/auto-mode-service.ts (1)
getAutoLoadClaudeMdSetting(2523-2548)apps/server/src/services/settings-service.ts (1)
SettingsService(104-583)
apps/server/src/routes/app-spec/index.ts (4)
apps/server/src/lib/events.ts (1)
EventEmitter(10-13)apps/server/src/services/settings-service.ts (1)
SettingsService(104-583)apps/server/src/routes/app-spec/routes/generate.ts (1)
createGenerateHandler(20-94)apps/server/src/routes/app-spec/routes/generate-features.ts (1)
createGenerateFeaturesHandler(20-76)
apps/server/src/services/auto-mode-service.ts (4)
apps/server/src/lib/events.ts (1)
EventEmitter(10-13)apps/server/src/lib/sdk-options.ts (2)
createCustomOptions(395-418)createAutoModeOptions(367-387)libs/types/src/provider.ts (1)
ExecuteOptions(34-46)apps/server/src/providers/types.ts (1)
ExecuteOptions(25-37)
apps/ui/src/store/app-store.ts (1)
apps/ui/src/hooks/use-settings-migration.ts (1)
syncSettingsToServer(195-242)
apps/server/src/routes/context/routes/describe-file.ts (5)
apps/server/src/services/settings-service.ts (1)
SettingsService(104-583)apps/server/src/lib/settings-helpers.ts (1)
getAutoLoadClaudeMdSetting(16-45)apps/server/src/services/auto-mode-service.ts (1)
getAutoLoadClaudeMdSetting(2523-2548)apps/server/src/lib/sdk-options.ts (1)
createCustomOptions(395-418)libs/types/src/model.ts (1)
CLAUDE_MODEL_MAP(4-8)
apps/server/src/routes/app-spec/routes/generate-features.ts (3)
apps/server/src/lib/events.ts (1)
EventEmitter(10-13)apps/server/src/services/settings-service.ts (1)
SettingsService(104-583)apps/server/src/routes/app-spec/generate-features-from-spec.ts (1)
generateFeaturesFromSpec(20-173)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (31)
.gitignore (1)
82-83: Appropriate ignore entries for the CLAUDE.md feature.Both additions follow the existing patterns and conventions:
- Line 82 (
docker-compose.override.yml): Standard Docker override file that should not be committed.- Line 83 (
.claude/): New directory for local CLAUDE.md files and settings, which aligns with the per-project configuration scope of this PR.apps/ui/src/hooks/use-settings-migration.ts (1)
226-226: LGTM! Consistent integration of the new setting.The addition of
autoLoadClaudeMdto the settings sync payload follows the established pattern for other boolean settings in this function.apps/server/src/routes/suggestions/routes/generate.ts (1)
10-10: LGTM! Proper dependency injection pattern.The SettingsService is correctly threaded through the handler and passed to
generateSuggestions. The optional parameter maintains backward compatibility while enabling the new CLAUDE.md auto-loading feature.Also applies to: 14-14, 41-41
apps/server/src/routes/context/index.ts (1)
11-11: LGTM! Consistent dependency injection.The SettingsService is properly threaded through to both image and file description handlers, with clear JSDoc documentation of the new parameter.
Also applies to: 16-19, 22-23
apps/server/src/routes/github/index.ts (1)
19-19: LGTM! Proper selective dependency injection.The SettingsService is correctly passed only to the validate-issue handler that requires it, avoiding unnecessary coupling for other GitHub endpoints.
Also applies to: 21-24, 33-33
apps/server/src/providers/types.ts (1)
29-29: LGTM! Type extensions enable the new functionality.The union type for
systemPromptmaintains backward compatibility (existing string usage still works) while enabling preset-based configuration. ThesettingSourcesfield is properly typed with specific literal values that align with the CLAUDE.md loading mechanism.Also applies to: 36-36
apps/ui/src/components/views/settings-view.tsx (1)
13-13: LGTM! Clean UI integration.The ClaudeMdSettings component is properly integrated into the Claude settings section with correct state wiring from the app store. The placement alongside other Claude-related settings is logical and maintains UI consistency.
Also applies to: 51-52, 108-111
libs/types/src/settings.ts (1)
301-303: LGTM! Well-structured type definitions.The
autoLoadClaudeMdsetting is properly defined in both GlobalSettings and ProjectSettings with clear JSDoc documentation. The default value offalseprovides safe opt-in behavior, and the optional field in ProjectSettings enables proper cascading from global to project-specific configuration.Also applies to: 400-402, 461-461
apps/server/src/providers/claude-provider.ts (1)
58-59: The implementation is correct and the Claude SDK version supportssettingSources.The dependency confirms
@anthropic-ai/claude-agent-sdkversion 0.1.72 is used, which supports thesettingSourcesoption (type:SettingSource[]whereSettingSource = 'user' | 'project' | 'local'). The code correctly forwards this option using a conditional spread operator, which properly handles cases wheresettingSourcesis undefined.apps/server/src/routes/suggestions/index.ts (1)
11-23: LGTM! Clean dependency injection for settings service.The optional
settingsServiceparameter follows a consistent pattern with other route factories in this PR, correctly wiring it through to only the generate handler where it's needed for autoLoadClaudeMd configuration.apps/server/src/routes/app-spec/routes/generate.ts (1)
16-73: LGTM! Correct propagation of settings service to spec generation.The
settingsServiceparameter is properly threaded through togenerateSpec, enabling autoLoadClaudeMd configuration to be applied during spec generation workflows.apps/server/src/routes/context/routes/describe-image.ts (1)
330-348: LGTM! Settings integration follows the established pattern.The handler correctly loads the
autoLoadClaudeMdsetting and passes it to SDK options. Note thatcwd(the image's parent directory) is used as the project path for settings lookup, which should work correctly when images are within the project directory structure.apps/server/src/routes/app-spec/index.ts (1)
12-23: LGTM! Consistent settings service wiring for generation routes.The
settingsServiceis appropriately passed only to the handlers that perform AI generation (/generateand/generate-features), while the create handler remains unchanged.libs/types/src/provider.ts (1)
22-46: LGTM! Well-designed type extensions for CLAUDE.md auto-loading.The
SystemPromptPresetinterface provides a clean abstraction for preset-based system prompts, and the union type forsystemPromptmaintains backward compatibility. ThesettingSourcesarray offers granular control over which CLAUDE.md sources to load.apps/server/src/routes/context/routes/describe-file.ts (1)
20-21: LGTM! Consistent implementation with describe-image handler.The settings integration follows the same pattern as
describe-image.ts, correctly loading theautoLoadClaudeMdsetting and passing it to SDK options.Also applies to: 77-82, 173-188
apps/ui/src/store/app-store.ts (2)
481-483: LGTM! Clean state and action definitions.The
autoLoadClaudeMdstate field and asyncsetAutoLoadClaudeMdaction are well-defined, following the existing patterns in the store.Also applies to: 757-759
2708-2708: LGTM! Setting correctly persisted.The
autoLoadClaudeMdfield is properly included in thepartializefunction, ensuring it persists across sessions.apps/ui/src/components/views/settings-view/claude/claude-md-settings.tsx (1)
1-82: LGTM! Well-structured settings component.The component follows React best practices with proper controlled input handling, accessibility attributes (
htmlFor,data-testid), and clear documentation. TheonCheckedChangecallback correctly handles thechecked === truecoercion for theCheckedStatetype from Radix UI.apps/server/src/lib/sdk-options.ts (2)
246-262: LGTM! Consistent integration of CLAUDE.md options.The
claudeMdOptionsspread pattern is correctly applied after base options, ensuring the CLAUDE.md configuration takes precedence. ThebuildClaudeMdOptionshelper properly handles the case where a customsystemPromptexists by appending it to the preset rather than overwriting.
195-202: SystemPromptConfig structure correctly matches @anthropic-ai/claude-agent-sdk v0.1.72.The interface definition with
type: 'preset',preset: 'claude_code', and optionalappendfield aligns precisely with the SDK's expected shape. The implementation inbuildClaudeMdOptions()properly combines this preset withsettingSources: ['user', 'project']to enable CLAUDE.md auto-loading as documented.apps/server/src/routes/suggestions/generate-suggestions.ts (1)
126-175: LGTM! Properly integrated settings service for CLAUDE.md auto-loading.The
settingsServiceparameter is correctly added as optional, thegetAutoLoadClaudeMdSettinghelper is properly awaited, and the setting is passed through to SDK options. This follows the same pattern established across other routes in this PR.apps/server/src/routes/app-spec/generate-features-from-spec.ts (1)
20-108: LGTM! Consistent integration pattern.The changes follow the same established pattern for
SettingsServiceintegration with proper optional parameter handling and correct async flow.apps/server/src/routes/app-spec/generate-spec.ts (1)
25-104: LGTM! Proper settings propagation.The
settingsServiceis correctly integrated and notably propagated togenerateFeaturesFromSpec(line 288), ensuring consistent CLAUDE.md behavior throughout the spec generation workflow.apps/server/src/routes/app-spec/routes/generate-features.ts (1)
20-56: LGTM! Clean factory pattern update.The handler factory correctly accepts the optional
SettingsServiceand propagates it to the underlyinggenerateFeaturesFromSpecfunction.apps/server/src/services/auto-mode-service.ts (2)
1102-1123: LGTM! Good integration of CLAUDE.md options into analysis flow.The
analyzeProjectmethod correctly usescreateCustomOptionswith theautoLoadClaudeMdsetting and properly extracts/propagatessettingSourcesto theExecuteOptions.
1799-1848: LGTM! Proper propagation of CLAUDE.md settings to agent execution.The
runAgentmethod correctly loads theautoLoadClaudeMdsetting, passes it tocreateAutoModeOptions, and extracts the resultingsystemPromptandsettingSourcesfor theExecuteOptions.apps/server/src/routes/github/routes/validate-issue.ts (1)
51-100: LGTM! Consistent SettingsService integration.The changes follow the same established pattern for integrating
SettingsServiceandgetAutoLoadClaudeMdSettinginto the validation workflow.apps/server/src/index.ts (2)
115-119: LGTM! Clean dependency injection ordering.The
SettingsServiceis correctly instantiated first and injected into dependent services (AgentService,AutoModeService). The comment on line 115 clarifies the ordering rationale.
152-162: LGTM! Consistent route wiring.The
settingsServiceis consistently injected into all routes that require access toautoLoadClaudeMdsettings. The pattern is uniform and follows the existing dependency injection style used elsewhere in the file.apps/server/src/services/agent-service.ts (2)
61-68: LGTM! Good backward-compatible optional dependency.The optional
SettingsServiceparameter with nullable field allows the service to function gracefully when the dependency is not provided, maintaining backward compatibility.
231-242: LGTM! SDK options correctly propagated to provider.The
systemPromptfromsdkOptionsandsettingSourcesare correctly passed to the provider options, enabling the SDK to handle CLAUDE.md loading when configured.
- Removed the private method for getting the autoLoadClaudeMd setting from AgentService and AutoModeService. - Updated both services to utilize the new settings helper for retrieving the autoLoadClaudeMd setting, improving code reusability and clarity. - Adjusted error handling in the settings helper to throw errors instead of returning false when the settings service is unavailable.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/server/src/services/auto-mode-service.ts (1)
565-568: CLAUDE.md duplication confirmed across executeFeature and followUpFeature flows when autoLoadClaudeMd is enabled.Both flows unconditionally load context files (including CLAUDE.md) via
loadContextFiles()and pass the result assystemPrompt. However, whenautoLoadClaudeMdis enabled, the SDK'sbuildClaudeMdOptionsfunction:
- Uses preset mode with 'claude_code' to auto-load CLAUDE.md via
settingSources- Appends the manually-loaded
systemPrompt(containing CLAUDE.md) to the presetThis results in CLAUDE.md content being sent to the agent twice.
Solution: Either:
- Conditionally skip loading in
loadContextFileswhenautoLoadClaudeMdis enabled (requires adding a skip parameter)- Only call
loadContextFileswhenautoLoadClaudeMdis false, letting the SDK handle it exclusively when enabledAffects: executeFeature (lines 565-568, 614), followUpFeature (lines 757-761, 877-890), and the plan analysis flow (lines 1104-1118).
apps/server/src/services/agent-service.ts (1)
193-221: Clarify CLAUDE.md handling when autoLoadClaudeMd is enabled.The duplication concern is valid. When
autoLoadClaudeMdis true:
loadContextFilesloads all files from.automaker/context/(lines 202-205), which can include CLAUDE.md- SDK's
settingSources: ['user', 'project']loads CLAUDE.md from the standard.claude/location- Both are appended to the final system prompt (line 218 → buildClaudeMdOptions)
If a user has CLAUDE.md in both locations, it will be included twice. Consider:
- Option 1: Modify
loadContextFilesto skip CLAUDE.md whenautoLoadClaudeMdis enabled- Option 2: Document that having CLAUDE.md in both locations is unsupported or intentional
- Option 3: Add logic to SDK to deduplicate CLAUDE.md content at the prompt level
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/server/src/lib/sdk-options.tsapps/server/src/lib/settings-helpers.tsapps/server/src/services/agent-service.tsapps/server/src/services/auto-mode-service.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/server/src/services/agent-service.ts (2)
apps/server/src/services/settings-service.ts (1)
SettingsService(104-583)apps/server/src/lib/settings-helpers.ts (1)
getAutoLoadClaudeMdSetting(16-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (5)
apps/server/src/lib/settings-helpers.ts (1)
16-45: LGTM! Clean implementation of settings precedence.The function correctly implements project-over-global settings precedence with appropriate error handling. The change to rethrow errors (instead of returning false) provides better visibility for debugging and aligns with the commit message.
apps/server/src/lib/sdk-options.ts (2)
139-177: LGTM! Well-structured CLAUDE.md auto-loading implementation.The
buildClaudeMdOptionsfunction cleanly separates standard mode from auto-loading mode:
- Standard mode passes through the systemPrompt unchanged
- Auto-load mode uses the 'claude_code' preset with settingSources to enable SDK-driven CLAUDE.md loading
- Custom systemPrompts are properly appended to the preset
234-235: LGTM! Consistent integration across all option builders.The
buildClaudeMdOptionshelper is correctly integrated into all create*Options functions, ensuring consistent CLAUDE.md loading behavior across spec generation, feature generation, suggestions, chat, auto mode, and custom options.Also applies to: 266-267, 295-296, 327-328, 359-360, 393-394
apps/server/src/services/auto-mode-service.ts (2)
1103-1118: LGTM! Clean integration of autoLoadClaudeMd in analyzeProject.The analysis flow correctly:
- Loads the autoLoadClaudeMd setting using the shared helper
- Creates SDK options via createCustomOptions with the setting
- Propagates settingSources to ExecuteOptions
1804-1817: LGTM! Proper settings loading and SDK options construction.The runAgent method correctly:
- Loads autoLoadClaudeMd with project precedence over global settings
- Passes the setting to createAutoModeOptions
- Propagates the resulting systemPrompt and settingSources to ExecuteOptions
Shironex
added
Ready-To-Merge
…deService - Updated both services to conditionally load CLAUDE.md based on the autoLoadClaudeMd setting, preventing duplication. - Improved clarity in comments regarding the loading process of context files. - Ensured consistent retrieval of the autoLoadClaudeMd setting across different methods.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/server/src/services/auto-mode-service.ts (2)
562-579: Verify: Same concern about non-CLAUDE.md context files.This has the same implementation as AgentService (lines 193-208): all context files are skipped when
autoLoadClaudeMdis enabled, not justCLAUDE.md. Please verify this doesn't unintentionally excludeCODE_QUALITY.mdand other context files.See the verification script in the agent-service.ts review comment for the same concern.
769-784: Same context loading concern applies here.This follows the identical pattern of conditionally skipping all context files. Same verification needed as flagged in the
executeFeatureandAgentService.sendMessagemethods.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/server/src/services/agent-service.tsapps/server/src/services/auto-mode-service.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/server/src/services/agent-service.ts (2)
apps/server/src/lib/settings-helpers.ts (1)
getAutoLoadClaudeMdSetting(16-45)libs/utils/src/context-loader.ts (1)
loadContextFiles(148-202)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (6)
apps/server/src/services/agent-service.ts (3)
62-62: LGTM: Clean dependency injection pattern.The SettingsService integration follows best practices with optional injection and explicit null handling.
Also applies to: 64-64, 68-68
217-248: LGTM: SDK options correctly propagated.The centralized SDK options flow is well-structured:
createChatOptionsreceives the combined system promptautoLoadClaudeMdcontrols the loading strategysystemPromptandsettingSourcesare properly forwarded to ExecuteOptions
193-198: LGTM: Shared helper function eliminates duplication.The use of
getAutoLoadClaudeMdSettingfromsettings-helpers.tsproperly addresses the code duplication concern raised in previous reviews. The helper correctly handles project/global setting precedence and null service handling.Based on past review comments, this successfully resolves the duplication issue.
apps/server/src/services/auto-mode-service.ts (3)
350-350: LGTM: Consistent dependency injection.The SettingsService integration follows the same clean pattern as AgentService.
Also applies to: 352-352, 354-354
1127-1152: LGTM: Clean SDK options usage.The
analyzeProjectmethod correctly uses centralized SDK configuration viacreateCustomOptions. By relying entirely on SDK options without manual context file loading, this avoids potential duplication and maintains consistency.
1756-1756: LGTM: Efficient setting reuse with proper fallback.The pattern of using a provided
autoLoadClaudeMdvalue (lines 1831-1834) before loading from settings is efficient and avoids redundant calls. The SDK options are correctly constructed and propagated.Also applies to: 1829-1842, 1880-1881
Shironex
added
Do Not Merge
…ervice - Updated both services to conditionally load context files while excluding CLAUDE.md when autoLoadClaudeMd is enabled, preventing duplication. - Improved the structure and clarity of the context files prompt, emphasizing the importance of following project-specific rules and conventions. - Ensured consistent handling of context file loading across different methods in both services.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/server/src/services/auto-mode-service.ts (2)
577-617: Extract duplicated CLAUDE.md filtering logic to a shared helper.This is the same CLAUDE.md filtering and prompt reconstruction logic flagged in
agent-service.tslines 206-246. It's also duplicated again in this file at lines 823-860. All instances should use a shared helper function to reduce maintenance burden.
820-860: Extract duplicated CLAUDE.md filtering logic to a shared helper.Another instance of the duplicated CLAUDE.md filtering and prompt reconstruction logic. Should use the same shared helper function recommended for the other occurrences.
🧹 Nitpick comments (1)
apps/server/src/services/agent-service.ts (1)
206-246: Extract duplicated CLAUDE.md filtering logic to a shared helper.This CLAUDE.md filtering and prompt reconstruction logic is duplicated in at least 4 locations:
agent-service.tslines 209-246 (here)auto-mode-service.tslines 580-617auto-mode-service.tslines 823-860The identical template and filtering logic creates maintenance burden when the prompt format needs updating.
🔎 Refactor suggestion
Create a shared helper in
apps/server/src/lib/context-helpers.ts:import type { ContextFilesResult } from '@automaker/utils'; export function filterAndRebuildContextPrompt( contextResult: ContextFilesResult, excludeClaude: boolean ): string { if (!excludeClaude || contextResult.files.length === 0) { return contextResult.formattedPrompt; } const nonClaudeFiles = contextResult.files.filter( (f) => f.name.toLowerCase() !== 'claude.md' ); if (nonClaudeFiles.length === 0) { return ''; } const formattedFiles = nonClaudeFiles.map((file) => { const header = `## ${file.name}`; const pathInfo = `**Path:** \`${file.path}\``; const descriptionInfo = file.description ? `\n**Purpose:** ${file.description}` : ''; return `${header}\n${pathInfo}${descriptionInfo}\n\n${file.content}`; }); return `# Project Context Files The following context files provide project-specific rules, conventions, and guidelines. Each file serves a specific purpose - use the description to understand when to reference it. If you need more details about a context file, you can read the full file at the path provided. **IMPORTANT**: You MUST follow the rules and conventions specified in these files. - Follow ALL commands exactly as shown (e.g., if the project uses \`pnpm\`, NEVER use \`npm\` or \`npx\`) - Follow ALL coding conventions, commit message formats, and architectural patterns specified - Reference these rules before running ANY shell commands or making commits --- ${formattedFiles.join('\n\n---\n\n')} --- **REMINDER**: Before taking any action, verify you are following the conventions specified above. `; }Then replace all occurrences with:
- // When autoLoadClaudeMd is enabled, filter out CLAUDE.md to avoid duplication - // (SDK handles CLAUDE.md via settingSources), but keep other context files like CODE_QUALITY.md - let contextFilesPrompt = contextResult.formattedPrompt; - if (autoLoadClaudeMd && contextResult.files.length > 0) { - const nonClaudeFiles = contextResult.files.filter( - (f) => f.name.toLowerCase() !== 'claude.md' - ); - - // Rebuild prompt without CLAUDE.md - if (nonClaudeFiles.length > 0) { - const formattedFiles = nonClaudeFiles.map((file) => { - const header = `## ${file.name}`; - const pathInfo = `**Path:** \`${file.path}\``; - const descriptionInfo = file.description ? `\n**Purpose:** ${file.description}` : ''; - return `${header}\n${pathInfo}${descriptionInfo}\n\n${file.content}`; - }); - - contextFilesPrompt = `# Project Context Files - -The following context files provide project-specific rules, conventions, and guidelines. -Each file serves a specific purpose - use the description to understand when to reference it. -If you need more details about a context file, you can read the full file at the path provided. - -**IMPORTANT**: You MUST follow the rules and conventions specified in these files. -- Follow ALL commands exactly as shown (e.g., if the project uses \`pnpm\`, NEVER use \`npm\` or \`npx\`) -- Follow ALL coding conventions, commit message formats, and architectural patterns specified -- Reference these rules before running ANY shell commands or making commits - ---- - -${formattedFiles.join('\n\n---\n\n')} - ---- - -**REMINDER**: Before taking any action, verify you are following the conventions specified above. -`; - } else { - // All files were CLAUDE.md, so no context to add - contextFilesPrompt = ''; - } - } + // When autoLoadClaudeMd is enabled, filter out CLAUDE.md to avoid duplication + // (SDK handles CLAUDE.md via settingSources), but keep other context files like CODE_QUALITY.md + const contextFilesPrompt = filterAndRebuildContextPrompt(contextResult, autoLoadClaudeMd);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/server/src/services/agent-service.tsapps/server/src/services/auto-mode-service.ts
🧰 Additional context used
🧬 Code graph analysis (2)
apps/server/src/services/auto-mode-service.ts (4)
apps/server/src/lib/settings-helpers.ts (1)
getAutoLoadClaudeMdSetting(16-45)libs/utils/src/context-loader.ts (1)
loadContextFiles(148-202)apps/server/src/lib/sdk-options.ts (2)
createCustomOptions(383-406)createAutoModeOptions(355-375)libs/types/src/provider.ts (1)
ExecuteOptions(34-46)
apps/server/src/services/agent-service.ts (3)
apps/server/src/services/settings-service.ts (1)
SettingsService(104-583)apps/server/src/lib/settings-helpers.ts (1)
getAutoLoadClaudeMdSetting(16-45)libs/utils/src/context-loader.ts (1)
loadContextFiles(148-202)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (10)
apps/server/src/services/agent-service.ts (4)
64-68: LGTM! Clean dependency injection.The optional
settingsServiceparameter maintains backward compatibility while enabling project-specific settings support. The nullish coalescing pattern is appropriate for handling the optional dependency.
193-198: LGTM! Correct usage of centralized settings helper.The
getAutoLoadClaudeMdSettingcall properly handles project/global precedence and provides clear logging context.
277-288: LGTM! Proper propagation of SDK options to provider.The
ExecuteOptionscorrectly includessystemPromptandsettingSourcesfrom the SDK options, enabling CLAUDE.md auto-loading at the provider level.
248-262: The system prompt is correctly preserved. ThecombinedSystemPromptflows throughcreateChatOptions→buildClaudeMdOptions(which returns{ systemPrompt }) → spread into the returnedOptionsobject, makingsdkOptions.systemPromptavailable for use inExecuteOptions.apps/server/src/services/auto-mode-service.ts (6)
352-354: LGTM! Consistent dependency injection pattern.The optional
settingsServiceparameter mirrors the implementation inagent-service.tsand maintains backward compatibility.
562-567: LGTM! Correct settings loading for feature execution.The
getAutoLoadClaudeMdSettingcall properly loads the setting with project/global precedence before feature execution begins.
807-812: LGTM! Correct settings loading for follow-up execution.The
getAutoLoadClaudeMdSettingcall appropriately loads the setting before follow-up feature execution.
1203-1228: LGTM! Clean SDK options integration for project analysis.The flow correctly:
- Loads
autoLoadClaudeMdsetting (lines 1204-1208)- Builds SDK options with
createCustomOptions(lines 1211-1218)- Propagates
settingSourcestoExecuteOptions(line 1227)This enables CLAUDE.md auto-loading during project analysis.
1905-1910: LGTM! Efficient settings handling with fallback.The conditional logic correctly uses the provided
autoLoadClaudeMdvalue when available, falling back to loading from settings only when needed. This avoids redundant setting loads when the value was already determined by the caller.
1912-1958: LGTM! Proper SDK options flow for agent execution.The implementation correctly:
- Builds SDK options with
createAutoModeOptionsincludingautoLoadClaudeMd(lines 1913-1918)- Extracts and uses fields from SDK options (lines 1921-1923)
- Propagates
systemPromptandsettingSourcestoExecuteOptions(lines 1956-1957)This ensures CLAUDE.md auto-loading works correctly during feature implementation.
…e handling - Introduced a new utility function to filter out CLAUDE.md from context files when autoLoadClaudeMd is enabled, enhancing clarity and preventing duplication. - Updated AgentService and AutoModeService to utilize the new filtering function, streamlining context file management. - Improved documentation for the new utility, detailing its purpose and usage in context file handling.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/server/src/lib/settings-helpers.ts (1)
77-98: Duplicated prompt template may drift from source.The comment on line 103 notes this format should match
@automaker/utils/context-loader.ts, but the template is hardcoded here. If the original format changes, this copy won't be updated automatically.Consider extracting a shared constant or function for the prompt header/footer in
@automaker/utilsthat both locations can import.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/server/src/lib/settings-helpers.tsapps/server/src/services/agent-service.tsapps/server/src/services/auto-mode-service.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (9)
apps/server/src/lib/settings-helpers.ts (2)
1-46: Well-structured helper for settings resolution.The implementation correctly follows the precedence pattern (project settings → global settings) and proper error handling with re-throw. The logging with customizable prefix aids debugging.
60-75: LGTM - edge cases properly handled.The function correctly handles:
autoLoadClaudeMd = false→ returns original prompt unchanged- Empty files array → returns original prompt
- All files are CLAUDE.md → returns empty string
- Case-insensitive filtering with
.toLowerCase()apps/server/src/services/agent-service.ts (2)
193-224: Good integration - addresses previous review concerns.The implementation now:
- Uses the shared
getAutoLoadClaudeMdSettinghelper (eliminating code duplication)- Always loads context files via
loadContextFiles()- Filters out only CLAUDE.md when auto-loading is enabled, preserving other context files like CODE_QUALITY.md
This correctly addresses the previous review concern about non-CLAUDE.md context files being lost.
62-68: Clean dependency injection pattern.Using optional
settingsServicewith null default allows backward compatibility while enabling the new auto-loading feature when the service is available.apps/server/src/services/auto-mode-service.ts (5)
350-354: Clean constructor update for settings service injection.Follows the same pattern as AgentService for backward compatibility.
562-579: Consistent context loading pattern.The implementation correctly:
- Loads
autoLoadClaudeMdsetting using the shared helper- Loads all context files
- Filters CLAUDE.md when auto-loading is enabled
This follows the same pattern as AgentService, maintaining consistency across the codebase.
1829-1841: Smart optimization to avoid redundant settings load.The code checks if
autoLoadClaudeMdwas already passed in options before callinggetAutoLoadClaudeMdSetting. This avoids redundant async settings lookups when the caller has already determined the value.
1127-1151: Good use of createCustomOptions for SDK configuration.The
analyzeProjectmethod now properly integratesautoLoadClaudeMdthrough the centralized SDK options, ensuring consistent behavior with other execution paths.
769-784: Consistent pattern in followUpFeature.Same context loading and filtering pattern is applied here, maintaining consistency across all feature execution paths.
Shironex
added
Ready-To-Merge
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.