feat(config): add multi-level workspace configuration system#99
feat(config): add multi-level workspace configuration system#99feloy merged 2 commits intokortex-hub:mainfrom
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThis PR implements a multi-level workspace configuration system allowing users to define configurations at four precedence layers: workspace-level ( Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI/init
participant Manager as Manager
participant PLoader as ProjectConfigLoader
participant ALoader as AgentConfigLoader
participant Merger as Merger
participant Runtime as Runtime
CLI->>Manager: Add(opts: Agent, Project, WorkspaceConfig)
Manager->>Manager: mergeConfigurations(project, workspace, agent)
Manager->>PLoader: Load(projectID)
PLoader->>PLoader: Read projects.json
PLoader-->>Manager: project config
Manager->>ALoader: Load(agentName)
ALoader->>ALoader: Read agents.json
ALoader-->>Manager: agent config
Manager->>Merger: Merge(workspace, project config)
Merger-->>Manager: merged config
Manager->>Merger: Merge(merged, agent config)
Merger-->>Manager: final config
Manager->>Runtime: CreateParams.WorkspaceConfig = final
Manager-->>CLI: instance registered
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/plans/88.md`:
- Around line 113-118: The fenced code block containing the pseudocode lines
starting with "merged = workspace_config" should include a language tag to
satisfy markdownlint MD040; update the opening fence from ``` to ```text (or
another appropriate language identifier) so the block reads like ```text
followed by the existing lines and the closing ```.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c55854ee-d47f-4a78-ac18-bddbbeb00041
📒 Files selected for processing (13)
AGENTS.mdREADME.mddocs/plans/88.mdpkg/cmd/init.gopkg/cmd/init_test.gopkg/config/agents.gopkg/config/agents_test.gopkg/config/merger.gopkg/config/merger_test.gopkg/config/projects.gopkg/config/projects_test.gopkg/instances/manager.gopkg/instances/manager_test.go
👮 Files not reviewed due to content moderation or server errors (8)
- pkg/cmd/init.go
- pkg/cmd/init_test.go
- README.md
- pkg/config/merger.go
- pkg/instances/manager.go
- pkg/instances/manager_test.go
- pkg/config/merger_test.go
- pkg/config/projects_test.go
|
hello, I've a question about the empty string and the configuration I think I'm lost on how to configure it if I see I don't understand where is that relative path (I don't think we should use relative path as you never know where it is) I don't see the difference between a dependencies or a configs mounts but it seems unrelated to this PR allowing to have multi-level but I think I'm lost |
The The I tried to document it here: https://github.com/kortex-hub/kortex-cli#mount-paths. Please tell me if it is not clear enough, I can add some more details |
|
I think it's adding a layer of complexity and you're not free to mount things where you want I could want to mount my when I use docker/podman/compose (all things mounting volumes) I say the source and the dest and the type (rw or ro) so I would expect something where I say the folder that I have locally and where I want it inside my agent but there is no 'dependency' or 'config' meaning. |
benoitf
left a comment
benoitf
left a comment
There was a problem hiding this comment.
this PR is unrelated to the mount definition, so it should not be hold
OK, I get your point. This will need to introduce two special variables $HOME and $SOURCES, for the user to be able to indicate that it wants to mount directories relative to these points (which are not known by the user, and probably different from one runtime to another). And in this case, we can remove the configs/dependencies and have only a |
Implements a three-level configuration system allowing users to customize workspace settings at different contexts with automatic merging and proper precedence: - Workspace config (.kortex/workspace.json) - shared, committed - Project config (~/.kortex-cli/config/projects.json) - user-specific - Agent config (~/.kortex-cli/config/agents.json) - per-agent overrides Configuration merging is handled by Manager for better separation of concerns. Added global project config support via empty string key. Closes kortex-hub#88 Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/instances/manager.go (1)
563-571: Consider caching config loaders for efficiency.New
ProjectConfigLoaderandAgentConfigLoaderinstances are created on eachAdd()call. While acceptable for CLI usage patterns, caching these loaders in themanagerstruct would avoid repeated allocations if the manager is reused.💡 Optional: Cache loaders in manager struct
type manager struct { storageFile string storageDir string mu sync.RWMutex factory InstanceFactory generator generator.Generator runtimeRegistry runtime.Registry gitDetector git.Detector + projectLoader config.ProjectConfigLoader + agentLoader config.AgentConfigLoader }Then initialize in
newManagerWithFactoryand reuse inmergeConfigurations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/instances/manager.go` around lines 563 - 571, The Add() path recreates ProjectConfigLoader and AgentConfigLoader on each call (see NewProjectConfigLoader/NewAgentConfigLoader usage in mergeConfigurations), causing repeated allocations; modify the manager struct to hold cached loader instances (e.g., projectLoader and agentLoader), initialize them in newManagerWithFactory using m.storageDir, and then reuse those fields inside mergeConfigurations/Add() instead of calling config.NewProjectConfigLoader/config.NewAgentConfigLoader on every invocation.pkg/config/projects.go (1)
121-129: Potential issue: Returning pointer to local map value.Lines 123 and 128 return pointers to values stored in the local
projectsConfigmap (&projectCfg,&globalCfg). While this works in Go (the map values are copied when extracted), it's clearer and safer to explicitly copy the configuration. TheMerger.Mergemethod on line 132 already handles this correctly by returning a new object.Consider using
copyConfig(from merger.go) for consistency, or this pattern is acceptable since the map values are already copies.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/config/projects.go` around lines 121 - 129, The function currently returns pointers to locally scoped map-extracted values (&projectCfg and &globalCfg), which can be confusing; change those returns to return explicit copies using the existing copy helper (copyConfig from merger.go) or the same copy logic Merger.Merge uses so callers receive a fresh object (e.g., replace returning &projectCfg and &globalCfg with copyConfig(projectCfg) / copyConfig(globalCfg) or equivalent) and keep Merger.Merge usage unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/config/projects.go`:
- Around line 121-129: The function currently returns pointers to locally scoped
map-extracted values (&projectCfg and &globalCfg), which can be confusing;
change those returns to return explicit copies using the existing copy helper
(copyConfig from merger.go) or the same copy logic Merger.Merge uses so callers
receive a fresh object (e.g., replace returning &projectCfg and &globalCfg with
copyConfig(projectCfg) / copyConfig(globalCfg) or equivalent) and keep
Merger.Merge usage unchanged.
In `@pkg/instances/manager.go`:
- Around line 563-571: The Add() path recreates ProjectConfigLoader and
AgentConfigLoader on each call (see NewProjectConfigLoader/NewAgentConfigLoader
usage in mergeConfigurations), causing repeated allocations; modify the manager
struct to hold cached loader instances (e.g., projectLoader and agentLoader),
initialize them in newManagerWithFactory using m.storageDir, and then reuse
those fields inside mergeConfigurations/Add() instead of calling
config.NewProjectConfigLoader/config.NewAgentConfigLoader on every invocation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2897accb-8314-4174-8546-29529aea44e0
📒 Files selected for processing (12)
AGENTS.mdREADME.mdpkg/cmd/init.gopkg/cmd/init_test.gopkg/config/agents.gopkg/config/agents_test.gopkg/config/merger.gopkg/config/merger_test.gopkg/config/projects.gopkg/config/projects_test.gopkg/instances/manager.gopkg/instances/manager_test.go
✅ Files skipped from review due to trivial changes (5)
- pkg/cmd/init.go
- pkg/cmd/init_test.go
- README.md
- AGENTS.md
- pkg/config/merger_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/instances/manager_test.go
- pkg/config/merger.go
- pkg/config/agents.go
- pkg/config/agents_test.go
3 participants
Implements a three-level configuration system allowing users to customize workspace settings at different contexts with automatic merging and proper precedence:
Configuration merging is handled by Manager for better separation of concerns. Added global project config support via empty string key.
Closes #88
Implementation Plan: Multi-Level Workspace Configuration
Issue: #88 - Workspace configuration at different levels
Branch:
config-levelsDate: 2026-03-24
Problem Statement
Currently, workspace configuration is only available via the
.kortexdirectory (stored with sources in the repository). This configuration is shared with all developers of the project and used with all agents.Users need the ability to:
.kortexSolution Overview
Implement a three-level configuration system with proper precedence rules:
.kortex/workspace.json) - Shared project configuration~/.kortex-cli/config/projects.json) - User's custom config for specific projects~/.kortex-cli/config/agents.json) - Per-agent overridesPrecedence: Agent-specific > Project-specific > Workspace-level
Configuration File Formats
Agent Configuration
Location:
~/.kortex-cli/config/agents.json(or$KORTEX_CLI_STORAGE/config/agents.jsonif set){ "claude": { "environment": [ { "name": "DEBUG", "value": "true" } ], "mounts": { "dependencies": ["../shared-libs"], "configs": [".claude-config"] } }, "goose": { "environment": [ { "name": "GOOSE_MODE", "value": "verbose" } ] } }Project Configuration
Location:
~/.kortex-cli/config/projects.json(or$KORTEX_CLI_STORAGE/config/projects.jsonif set){ "": { "mounts": { "configs": [".gitconfig", ".ssh"] } }, "github.com/kortex-hub/kortex-cli": { "environment": [ { "name": "API_KEY", "secret": "my-api-key-secret" } ], "mounts": { "dependencies": ["../kortex-common"] } }, "/home/user/my/project": { "environment": [ { "name": "LOCAL_DEV", "value": "true" } ] } }Special Key - Empty String
"":"") represents a global/default configuration that applies to all projects"") → project-specific → agent-specificNote: Each configuration uses the same
WorkspaceConfigurationstructure defined in the kortex-cli-api package.Implementation Tasks
Phase 1: Core Infrastructure
Task 1: Configuration Merger
File:
pkg/config/merger.goConfigMergerinterface for mergingWorkspaceConfigurationobjectsMerging Algorithm:
Task 2: Agent Configuration Loader
File:
pkg/config/agents.goAgentConfiginterface for loading agent-specific configurationLoadAgentConfig(storageDir string, agentName string) (*WorkspaceConfiguration, error)<storageDir>/config/agents.jsonstorageDirparameter comes from the--storageflag orKORTEX_CLI_STORAGEenvironment variableTask 3: Project Configuration Loader
File:
pkg/config/projects.goProjectConfiginterface for loading project-specific configurationLoadProjectConfig(storageDir string, projectID string) (*WorkspaceConfiguration, error)<storageDir>/config/projects.jsonstorageDirparameter comes from the--storageflag orKORTEX_CLI_STORAGEenvironment variable""key and merge with project-specific config""key)/home/user/project/subdirovergithub.com/user/repo)Phase 2: Integration
Task 4: Manager merges Configs
Files:
pkg/instances/manager.go,pkg/cmd/init.goArchitecture Decision: Merging logic by the Manager. This provides:
Changes to
pkg/instances/manager.go:Agentfield toAddOptionsstructstorageDirin manager struct (needed for config loaders)Add()method to:Changes to
pkg/cmd/init.go:--agentflag.kortex/workspace.json(existing)manager.Add()New AddOptions structure:
Command logic:
Phase 3: Testing
Task 5: Merger Tests
File:
pkg/config/merger_test.goTest scenarios:
Task 6: Agent Config Tests
File:
pkg/config/agents_test.goTest scenarios:
Task 7: Project Config Tests
File:
pkg/config/projects_test.goTest scenarios:
""key)Task 8: Init Command Tests
File:
pkg/cmd/init_test.goTest scenarios:
Phase 4: Documentation
Task 9: Update Documentation
Files:
CLAUDE.md,README.mdCLAUDE.md - Add new section: "Multi-Level Configuration System"
Document:
agents.jsonandprojects.jsonREADME.md - Add subsection to "Workspace Configuration" section
Document:
~/.kortex-clican be overridden withKORTEX_CLI_STORAGEenvironment variable or--storageflagagents.jsonandprojects.json""key inprojects.jsonfor settings that apply to all projectsinitDesign Decisions
File Storage Locations
~/.kortex-cli/config/agents.json(default) or$KORTEX_CLI_STORAGE/config/agents.json(if environment variable is set)~/.kortex-cli/config/projects.json(default) or$KORTEX_CLI_STORAGE/config/projects.json(if environment variable is set).kortex/workspace.json(existing, stored with sources)Note: The storage directory can be customized using the
--storageflag orKORTEX_CLI_STORAGEenvironment variable (see README.md for details). Agent and project configurations are stored under<storage-dir>/config/.Precedence Order (highest to lowest)
projects.jsonusing exact project ID match)projects.jsonusing empty string""key).kortex/workspace.json)Project ID Matching
pkg/instances/manager.go""key inprojects.jsonapplies to all projectsMerging Behavior
Environment Variables:
namefieldMount Paths:
Error Handling
pkg/config/config.goCross-Platform Compatibility
filepath.Join()for all path operationst.TempDir()in tests (never hardcode paths)Testing Strategy
Success Criteria
Future Enhancements
Potential future work (not in scope for this issue):
config set,config get)References
pkg/config/config.gopkg/cmd/init.go