feat(agent): add full agent configuration support#115
Merged
feloy merged 3 commits intoopenkaiden:mainfrom Mar 26, 2026
Merged
Conversation
Agent is now a required field when registering workspaces, specified via --agent flag or KORTEX_CLI_DEFAULT_AGENT environment variable. Agent name is displayed in workspace list and init outputs. Terminal command uses the agent's configured terminal_command from runtime config, allowing different agents to have different default terminal commands. Podman runtime changed from hardcoded "claude" user/group to generic "agent" user/group, making the runtime more flexible for different agents. All documentation updated to reflect agent requirements and usage. Closes openkaiden#114 Co-Authored-By: Claude Code (Claude Sonnet 4.5) <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThreads an explicit agent identity through CLI, instance, manager, and Podman runtime layers: Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI User
participant InitCmd as init Command
participant Manager as Instance Manager
participant Runtime as Podman Runtime
participant AgentCfg as Agent Config
rect rgba(100,149,237,0.5)
CLI->>InitCmd: init --agent claude --runtime podman
InitCmd->>InitCmd: preRun: resolve agent (flag → KORTEX_CLI_DEFAULT_AGENT → error)
InitCmd->>Manager: Add(opts {Agent: "claude", ...})
Manager->>Runtime: Create(ctx, params {Agent: "claude", ...})
Runtime->>AgentCfg: LoadAgent("claude")
AgentCfg-->>Runtime: agent configuration
end
rect rgba(144,238,144,0.5)
CLI->>InitCmd: workspace terminal (no COMMAND)
InitCmd->>Manager: Terminal(instanceID, agent, [])
Manager->>Runtime: Terminal(ctx, instanceID, "claude", [])
Runtime->>AgentCfg: LoadAgent("claude")
AgentCfg-->>Runtime: TerminalCommand
Runtime->>Runtime: exec -it <instanceID> <agentTerminalCmd>
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
README.md (1)
175-187:⚠️ Potential issue | 🟡 MinorMissing
agentfield in JSON example.The "Step 8: Verify removal" JSON output example is missing the
"agent"field that should be present in workspace list output, unlike the other examples at lines 100-122 which correctly include it.📝 Suggested fix
{ "items": [ { "id": "f6e5d4c3b2a1098765432109876543210987654321098765432109876543210a", "name": "another-project", + "agent": "claude", "paths": { "source": "/absolute/path/to/another-project", "configuration": "/absolute/path/to/another-project/.kortex" } } ] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 175 - 187, The JSON example in the "Step 8: Verify removal" output is missing the required "agent" field; update the example object inside the "items" array (the entry with id "f6e5d4c3b2a1...a" and name "another-project") to include the same "agent" key/structure used by the other workspace examples so the workspace list output matches the expected schema.pkg/runtime/runtime.go (1)
93-96:⚠️ Potential issue | 🟡 MinorDocumentation example shows outdated method signature.
The example code in the docstring still uses the old 3-parameter signature
Terminal(ctx, instanceID string, command []string)but the interface now requires 4 parameters includingagent.📝 Suggested fix
// Example implementation: // // type myRuntime struct { // // ... other fields // } // -// func (r *myRuntime) Terminal(ctx context.Context, instanceID string, command []string) error { +// func (r *myRuntime) Terminal(ctx context.Context, instanceID string, agent string, command []string) error { // // Execute command interactively (stdin/stdout/stderr connected) // return r.exec.RunInteractive(ctx, "exec", "-it", instanceID, command...) // }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/runtime/runtime.go` around lines 93 - 96, The docstring/example uses the old Terminal(ctx, instanceID string, command []string) signature; update all examples and the method implementation to the current four-parameter signature including the agent (e.g., Terminal(ctx context.Context, agent string, instanceID string, command []string)), and adjust any calls to myRuntime.Terminal and other implementations to pass the agent through to the underlying exec.RunInteractive call (or equivalent) so the example and implementation match the interface.
🧹 Nitpick comments (1)
pkg/runtime/podman/config/defaults.go (1)
61-61: Hardcoded/home/agentpath creates coupling withContainerUserconstant.This path is hardcoded while
containerfile.godefinesContainerUser = "agent"as a constant. IfContainerUserchanges, this default will become inconsistent.However, since
RunCommandsare serialized to JSON config files that users can customize, and the config package shouldn't depend on the podman package (to avoid circular imports), this is acceptable as-is.Consider adding a comment noting the dependency on the container user being "agent".
📝 Suggested documentation
RunCommands: []string{ "curl -fsSL --proto-redir '-all,https' --tlsv1.3 https://claude.ai/install.sh | bash", + // Note: /home/agent must match ContainerUser in containerfile.go "mkdir -p /home/agent/.config", },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/runtime/podman/config/defaults.go` at line 61, The default RunCommands entry contains a hardcoded "/home/agent/.config" path which couples this package to the container user defined as ContainerUser = "agent" in containerfile.go; add a brief comment next to the RunCommands/defaults entry explaining that the path assumes the container user is "agent" (and that users can override the serialized JSON if they change the user), referencing ContainerUser and the literal "/home/agent/.config" so future readers know why the hardcode exists and to avoid introducing a podman package import.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@pkg/runtime/runtime.go`:
- Around line 93-96: The docstring/example uses the old Terminal(ctx, instanceID
string, command []string) signature; update all examples and the method
implementation to the current four-parameter signature including the agent
(e.g., Terminal(ctx context.Context, agent string, instanceID string, command
[]string)), and adjust any calls to myRuntime.Terminal and other implementations
to pass the agent through to the underlying exec.RunInteractive call (or
equivalent) so the example and implementation match the interface.
In `@README.md`:
- Around line 175-187: The JSON example in the "Step 8: Verify removal" output
is missing the required "agent" field; update the example object inside the
"items" array (the entry with id "f6e5d4c3b2a1...a" and name "another-project")
to include the same "agent" key/structure used by the other workspace examples
so the workspace list output matches the expected schema.
---
Nitpick comments:
In `@pkg/runtime/podman/config/defaults.go`:
- Line 61: The default RunCommands entry contains a hardcoded
"/home/agent/.config" path which couples this package to the container user
defined as ContainerUser = "agent" in containerfile.go; add a brief comment next
to the RunCommands/defaults entry explaining that the path assumes the container
user is "agent" (and that users can override the serialized JSON if they change
the user), referencing ContainerUser and the literal "/home/agent/.config" so
future readers know why the hardcode exists and to avoid introducing a podman
package import.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 504fad6b-11b0-4b9f-bfff-3e366139c557
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (24)
README.mdgo.modpkg/cmd/conversion.gopkg/cmd/conversion_test.gopkg/cmd/init.gopkg/cmd/init_test.gopkg/cmd/stdout_test.gopkg/cmd/terminal_test.gopkg/cmd/workspace_list.gopkg/cmd/workspace_terminal.gopkg/cmd/workspace_terminal_test.gopkg/instances/instance.gopkg/instances/manager.gopkg/instances/manager_terminal_test.gopkg/instances/manager_test.gopkg/runtime/podman/config/config.gopkg/runtime/podman/config/defaults.gopkg/runtime/podman/containerfile.gopkg/runtime/podman/containerfile_test.gopkg/runtime/podman/create.gopkg/runtime/podman/create_test.gopkg/runtime/podman/terminal.gopkg/runtime/podman/terminal_test.gopkg/runtime/runtime.go
Signed-off-by: Philippe Martin <phmartin@redhat.com> Co-Authored-By: Claude Code (Claude Sonnet 4.5) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/runtime/podman/create.go (1)
178-190:⚠️ Potential issue | 🔴 CriticalValidate and normalize config mount paths to prevent directory traversal attacks.
Config paths from
mounts.configscan escape the home directory root. While the validation inpkg/config/config.gocorrectly rejects absolute paths, it does not prevent traversal sequences like.., allowing entries such as../../etc/passwdto pass validation as "relative" paths. Whenfilepath.Join(homeDir, "../../etc/passwd")executes, it normalizes to/etc/passwd, mounting arbitrary host files. The same vulnerability exists for container-side paths viapath.Join().Validate by normalizing with
filepath.Clean()and rejecting paths that resolve to.,.., absolute paths, or contain leading traversal sequences before joining.Suggested fix
if params.WorkspaceConfig.Mounts.Configs != nil { homeDir, err := os.UserHomeDir() if err != nil { return nil, fmt.Errorf("failed to get home directory: %w", err) } for _, conf := range *params.WorkspaceConfig.Mounts.Configs { - // Convert config path from forward slashes to OS-specific separators - confOSPath := filepath.FromSlash(conf) - confAbsPath := filepath.Join(homeDir, confOSPath) + cleanConf := filepath.Clean(filepath.FromSlash(conf)) + if cleanConf == "." || cleanConf == ".." || filepath.IsAbs(cleanConf) || + strings.HasPrefix(cleanConf, ".."+string(filepath.Separator)) { + return nil, fmt.Errorf("%w: config path %q must stay within the user's home directory", runtime.ErrInvalidParams, conf) + } + confAbsPath := filepath.Join(homeDir, cleanConf) // HOME in container is /home/<user> for the image // Use path.Join for container paths to ensure forward slashes - confMountPoint := path.Join(fmt.Sprintf("/home/%s", constants.ContainerUser), conf) + confMountPoint := path.Join("/home", constants.ContainerUser, filepath.ToSlash(cleanConf)) args = append(args, "-v", fmt.Sprintf("%s:%s:Z", confAbsPath, confMountPoint)) } }Note: The same vulnerability exists in the dependencies mounting code (lines 165–174) and requires the same fix.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/runtime/podman/create.go` around lines 178 - 190, The config mount handling (params.WorkspaceConfig.Mounts.Configs) must reject paths that normalize outside the home directory: before joining to homeDir call filepath.Clean on conf (use confOSPath := filepath.Clean(filepath.FromSlash(conf))), then reject if filepath.IsAbs(confOSPath) or confOSPath == "." or confOSPath == ".." or any path segment equals ".." (split and check); only then compute confAbsPath := filepath.Join(homeDir, confOSPath). Similarly sanitize the container side mount point using path.Clean on the container-relative path and reject container paths that are absolute or contain ".."; apply the identical validation for the dependencies mount loop as well so neither host nor container mount targets can escape intended directories (refer to params.WorkspaceConfig.Mounts.Configs, constants.ContainerUser and the dependencies mount handling).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/runtime/runtime.go`:
- Around line 62-63: Update the documentation for CreateParams.Agent to declare
it as required and non-empty (do not state "optional/can be empty"); callers and
test doubles should treat CreateParams.Agent as a mandatory field. Specifically,
change the comment on the CreateParams.Agent field to state it must be a
non-empty agent name, and ensure any tests or mocks don't rely on "" as a valid
value; this aligns with the runtime/podman create logic that rejects empty
agents (see CreateParams.Agent and the validation in
pkg/runtime/podman/create.go).
In `@README.md`:
- Around line 1261-1270: The README contains several init examples that still
call the literal command "kortex-cli init ... --runtime ..." without the
now-required agent, which will fail; update each example that shows "kortex-cli
init ... --runtime ..." (the occurrences mentioned in the review) to include an
explicit agent flag (e.g., add "--agent <agent-name>") or show how to set
KORTEX_CLI_DEFAULT_AGENT in the environment before running the command, and
ensure the examples consistently demonstrate either the CLI flag approach or the
environment-variable approach so copy-pasted commands succeed.
- Around line 554-556: Replace the sample README command "mkdir
/home/agent/.config" with the idempotent form "mkdir -p /home/agent/.config" so
it matches the generated default that uses "mkdir -p /home/<user>/.config";
update the README sample agent config so copy-pasting preserves behavior and
avoids errors when the directory already exists.
---
Outside diff comments:
In `@pkg/runtime/podman/create.go`:
- Around line 178-190: The config mount handling
(params.WorkspaceConfig.Mounts.Configs) must reject paths that normalize outside
the home directory: before joining to homeDir call filepath.Clean on conf (use
confOSPath := filepath.Clean(filepath.FromSlash(conf))), then reject if
filepath.IsAbs(confOSPath) or confOSPath == "." or confOSPath == ".." or any
path segment equals ".." (split and check); only then compute confAbsPath :=
filepath.Join(homeDir, confOSPath). Similarly sanitize the container side mount
point using path.Clean on the container-relative path and reject container paths
that are absolute or contain ".."; apply the identical validation for the
dependencies mount loop as well so neither host nor container mount targets can
escape intended directories (refer to params.WorkspaceConfig.Mounts.Configs,
constants.ContainerUser and the dependencies mount handling).
🪄 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: c678ea4e-48b3-4e80-b71e-1c2f7a58ca01
📒 Files selected for processing (6)
README.mdpkg/runtime/podman/config/defaults.gopkg/runtime/podman/constants/constants.gopkg/runtime/podman/containerfile.gopkg/runtime/podman/create.gopkg/runtime/runtime.go
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/runtime/podman/config/defaults.go
- pkg/runtime/podman/containerfile.go
benoitf
approved these changes
Mar 26, 2026
Signed-off-by: Philippe Martin <phmartin@redhat.com>
feloy
changed the title
This was referenced Mar 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Agent is now a required field when registering workspaces, specified via --agent flag or KORTEX_CLI_DEFAULT_AGENT environment variable. Agent name is displayed in workspace list and init outputs. Terminal command uses the agent's configured terminal_command from runtime config, allowing different agents to have different default terminal commands.
Podman runtime changed from hardcoded "claude" user/group to generic "agent" user/group, making the runtime more flexible for different agents.
All documentation updated to reflect agent requirements and usage.
Closes #114