feat: add profile-based config for multi-client workflows#20
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds profile-based configuration support to enable multi-client workflows where different clients can have isolated config settings. The feature allows users to specify a --profile flag on commands to load client-specific configuration from ~/.rune/profiles/<name>.yaml which is merged with the base config.
Changes:
- Added profile loading infrastructure with
LoadWithProfile()function and profile merge logic - Added
--profilepersistent flag to root command and integrated it into start, stop, and ritual commands - Added comprehensive test coverage for profile loading, merging, validation, and error handling
- Added mise.toml for Go version management
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| mise.toml | Defines Go toolchain version for the project |
| internal/config/config.go | Implements profile loading, merging logic, and utility functions (GetProfilePath, ProfileExists) |
| internal/config/config_test.go | Adds comprehensive tests for profile functionality including load, merge, validation, and error cases |
| internal/commands/root.go | Adds --profile persistent flag and binds it to viper |
| internal/commands/start.go | Integrates profile flag to load profile-specific config |
| internal/commands/stop.go | Integrates profile flag to load profile-specific config |
| internal/commands/ritual.go | Integrates profile flag for ritual list, test, and run commands |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/config/config.go
Outdated
| func mergeNotificationSettings(base *NotificationSettings, profile *NotificationSettings) { | ||
| // Only override fields that are explicitly set | ||
| // For booleans, we assume they're set if they're true | ||
| if profile.Enabled { | ||
| base.Enabled = profile.Enabled | ||
| } | ||
| if profile.BreakReminders { | ||
| base.BreakReminders = profile.BreakReminders | ||
| } | ||
| if profile.EndOfDayReminders { | ||
| base.EndOfDayReminders = profile.EndOfDayReminders | ||
| } | ||
| if profile.SessionComplete { | ||
| base.SessionComplete = profile.SessionComplete | ||
| } | ||
| if profile.IdleDetection { | ||
| base.IdleDetection = profile.IdleDetection | ||
| } | ||
| if profile.Sound { | ||
| base.Sound = profile.Sound | ||
| } | ||
| } |
There was a problem hiding this comment.
The boolean merge logic cannot distinguish between explicitly set false values and unset values in profiles. For example, if a profile wants to disable telemetry (set Enabled to false), the current logic (lines 242-259, 302-322) will only merge if the value is true, meaning you cannot use profiles to disable boolean settings that are enabled in the base config. Consider using pointers (*bool) for boolean fields that need to support explicit false values in profiles, or track which fields are explicitly set in the profile YAML.
internal/config/config.go
Outdated
| func mergeIntegrations(base *Integrations, profile *Integrations) { | ||
| // Git integration | ||
| if profile.Git.Enabled || profile.Git.AutoDetectProject { | ||
| base.Git = profile.Git | ||
| } | ||
|
|
||
| // Slack integration | ||
| if profile.Slack.Workspace != "" || profile.Slack.DNDOnStart { | ||
| base.Slack = profile.Slack | ||
| } | ||
|
|
||
| // Calendar integration | ||
| if profile.Calendar.Provider != "" || profile.Calendar.BlockCalendar { | ||
| base.Calendar = profile.Calendar | ||
| } | ||
|
|
||
| // Telemetry integration | ||
| if profile.Telemetry.SentryDSN != "" { | ||
| base.Telemetry.SentryDSN = profile.Telemetry.SentryDSN | ||
| } | ||
| if profile.Telemetry.Enabled { | ||
| base.Telemetry.Enabled = profile.Telemetry.Enabled | ||
| } | ||
| } |
There was a problem hiding this comment.
The integration merge logic has issues with boolean fields. For Git integration (lines 302-304), the condition "if profile.Git.Enabled || profile.Git.AutoDetectProject" means that if both are false in the profile, the entire Git struct won't be merged, which may not be the intended behavior. Similar issues exist for Slack (lines 307-309) and Calendar (lines 312-314). If a profile wants to explicitly disable Git or change other Git settings while keeping Enabled=false, this won't work correctly. Consider checking if any field in the struct is non-zero instead of just the boolean fields.
internal/config/config.go
Outdated
| func mergeRituals(base *Rituals, profile *Rituals) { | ||
| // Merge templates | ||
| if len(profile.Templates) > 0 { | ||
| if base.Templates == nil { | ||
| base.Templates = make(map[string]TmuxTemplate) | ||
| } | ||
| for name, template := range profile.Templates { | ||
| base.Templates[name] = template | ||
| } | ||
| } | ||
|
|
||
| // Merge start rituals | ||
| if len(profile.Start.Global) > 0 { | ||
| base.Start.Global = profile.Start.Global | ||
| } | ||
| if len(profile.Start.PerProject) > 0 { | ||
| if base.Start.PerProject == nil { | ||
| base.Start.PerProject = make(map[string][]Command) | ||
| } | ||
| for project, commands := range profile.Start.PerProject { | ||
| base.Start.PerProject[project] = commands | ||
| } | ||
| } | ||
|
|
||
| // Merge stop rituals | ||
| if len(profile.Stop.Global) > 0 { | ||
| base.Stop.Global = profile.Stop.Global | ||
| } | ||
| if len(profile.Stop.PerProject) > 0 { | ||
| if base.Stop.PerProject == nil { | ||
| base.Stop.PerProject = make(map[string][]Command) | ||
| } | ||
| for project, commands := range profile.Stop.PerProject { | ||
| base.Stop.PerProject[project] = commands | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The rituals merge logic replaces entire arrays instead of merging them. For example, if base config has global start rituals and a profile also defines global start rituals, the profile completely replaces the base rituals (line 275). This means you cannot add to base rituals in a profile - you can only replace them entirely. Consider whether this is the intended behavior or if rituals should be merged/appended. The same applies to stop rituals (line 288).
Implement profile-based configuration system that allows users to maintain
multiple config variants (e.g., work, home, personal) and switch between
them using the --profile flag.
Features:
- LoadWithProfile() function to load and merge profile configs
- --profile global flag for all commands
- Profile configs stored in ~/.rune/profiles/{name}.yaml
- Safe merge behavior: profiles override base config where specified
- Projects are appended, templates are merged, other fields override
- Clear error messages for missing profiles
- Full validation of merged configs
Implementation:
- Added config.LoadWithProfile() with profile merging logic
- Added config.GetProfilePath() to locate profile files
- Added config.mergeConfigs() with safe merge behavior
- Added --profile flag to root command as persistent flag
- Created loadConfigWithProfile() helper in commands/root.go
- Updated all commands to use loadConfigWithProfile()
- Comprehensive test coverage for all profile operations
Tests:
- TestLoadWithProfile_MissingProfile: validates error handling
- TestLoadWithProfile_EmptyProfile: validates fallback to base config
- TestLoadWithProfile_ValidProfile: validates merge behavior
- TestMergeConfigs: validates merge logic in detail
- TestGetProfilePath: validates profile path resolution
All existing tests continue to pass.
v1truv1us
force-pushed
the
agent/issue-11-subagent
branch
from
9720b79 to
6b863cf
Compare
Closes #11
Adds profile-based config support using --profile to load and merge client-specific config from ~/.rune/profiles/.yaml.
Includes