feat: add DevOps-Agent for operational and system administration tasks#41
Conversation
- Auto-enable command mode after task submission with minimal UI - Hide input textarea and show compact command prompt with tips - Add keyboard shortcuts: f/b for page up/down, j/k for scroll, i/enter to exit, esc to cancel running task - Auto-disable command mode when task completes - Add i18n translations for command mode prompt and tips (zh/en)
Reviewer's GuideAdds a new DevOps-Agent for operational/system administration tasks, wires it into the conductor and app configuration, updates prompts, i18n, CLI/docs, and introduces a TUI command mode for streamlined interaction while tasks are running. Sequence diagram for Conductor delegating an operational task to DevOps-AgentsequenceDiagram
actor User
participant TUI as TUI_model
participant CodingAssistant
participant Conductor as ConductorAgent
participant DevOps as DevOpsAgent
participant SysOps as SysOps_RunBash
User->>TUI: Enter operational request
TUI->>CodingAssistant: ProcessConversation(TaskRequest)
CodingAssistant->>Conductor: Run task via LLM loop
Note over Conductor: Classifies task as DevOps
Conductor->>Conductor: Call delegate_devops tool
Conductor->>DevOps: Run(ctx, task)
DevOps->>DevOps: Build ExecutorConfig with devopsPrompt and Adapters
DevOps->>SysOps: ExecuteRunBash(ctx, params) via run_bash adapter
SysOps-->>DevOps: Command output
DevOps-->>Conductor: Final DevOps result
Conductor-->>CodingAssistant: Aggregated response
CodingAssistant-->>TUI: Response text
TUI-->>User: Rendered result in viewport
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
NewConductorAgent,delegateDevOpsis created unconditionally and callsdevops.Run, but several tests passnilfor the DevOpsAgent anddisabledAgentsisnil, so usingdelegate_devopswould panic; consider only constructing/adding this adapter whendevops != niland the agent isn't disabled. - When cancelling a running task in command mode via
escinUpdate,commandModeremains true even though the task is no longer running; you may want to explicitly resetcommandModeto false there to keep the state consistent and avoid confusing UI behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `NewConductorAgent`, `delegateDevOps` is created unconditionally and calls `devops.Run`, but several tests pass `nil` for the DevOpsAgent and `disabledAgents` is `nil`, so using `delegate_devops` would panic; consider only constructing/adding this adapter when `devops != nil` and the agent isn't disabled.
- When cancelling a running task in command mode via `esc` in `Update`, `commandMode` remains true even though the task is no longer running; you may want to explicitly reset `commandMode` to false there to keep the state consistent and avoid confusing UI behavior.
## Individual Comments
### Comment 1
<location path="internal/agents/conductor_test.go" line_range="54" />
<code_context>
t.Helper()
gctx := newTestGlobalCtx(workDir)
engine := &mockEngine{}
- return NewConductorAgent(gctx, engine, nil, nil, nil, nil, 10, nil, 3)
+ return NewConductorAgent(gctx, engine, nil, nil, nil, nil, nil, 10, nil, 3)
}
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests to cover the new DevOps delegate wiring and disabledAgents behaviour
The Conductor now wires a DevOpsAgent and registers a `delegate_devops` adapter, controlled by `disabledAgents["devops"]`, but `conductor_test.go` doesn’t verify this. Please add focused tests that:
1. Build a Conductor with a non-nil DevOpsAgent and `disabledAgents` unset or not containing `"devops"`, assert an adapter with `Name() == "delegate_devops"` is registered, and that invoking it calls `DevOpsAgent.Run`.
2. Build a Conductor with `disabledAgents["devops"] = true` and assert no `delegate_devops` adapter is registered.
This will ensure the new agent is correctly wired and respects the disable configuration.
Suggested implementation:
```golang
t.Helper()
gctx := newTestGlobalCtx(workDir)
engine := &mockEngine{}
return NewConductorAgent(gctx, engine, nil, nil, nil, nil, nil, 10, nil, 3)
}
type mockDevOpsAgent struct {
runCount int32
}
func (m *mockDevOpsAgent) Run(ctx context.Context) error {
atomic.AddInt32(&m.runCount, 1)
return nil
}
func TestConductorRegistersDevOpsDelegateWhenEnabled(t *testing.T) {
t.Helper()
workDir := t.TempDir()
gctx := newTestGlobalCtx(workDir)
engine := &mockEngine{}
devOpsAgent := &mockDevOpsAgent{}
disabledAgents := map[string]bool{}
conductor := NewConductorAgent(
gctx,
engine,
nil, // planner
nil, // coder
nil, // executor
nil, // tools
nil, // git
devOpsAgent,
10, // maxDepth or similar
disabledAgents, // disabledAgents
3, // parallelism or similar
)
var devOpsAdapterFound bool
for _, a := range conductor.adapters {
if a.Name() == "delegate_devops" {
devOpsAdapterFound = true
if r, ok := a.(interface {
Run(ctx context.Context) error
}); ok {
if err := r.Run(context.Background()); err != nil {
t.Fatalf("delegate_devops adapter Run() returned error: %v", err)
}
if got := atomic.LoadInt32(&devOpsAgent.runCount); got != 1 {
t.Fatalf("expected DevOpsAgent.Run to be called once, got %d", got)
}
}
break
}
}
if !devOpsAdapterFound {
t.Fatalf("expected delegate_devops adapter to be registered when devops agent is enabled")
}
}
func TestConductorDoesNotRegisterDevOpsDelegateWhenDisabled(t *testing.T) {
t.Helper()
workDir := t.TempDir()
gctx := newTestGlobalCtx(workDir)
engine := &mockEngine{}
devOpsAgent := &mockDevOpsAgent{}
disabledAgents := map[string]bool{
"devops": true,
}
conductor := NewConductorAgent(
gctx,
engine,
nil, // planner
nil, // coder
nil, // executor
nil, // tools
nil, // git
devOpsAgent,
10,
disabledAgents,
3,
)
for _, a := range conductor.adapters {
if a.Name() == "delegate_devops" {
t.Fatalf("did not expect delegate_devops adapter to be registered when disabledAgents[\"devops\"] is true")
}
}
}
// makeMetaOutput builds a valid Meta-Agent JSON output string.
```
1. Ensure `internal/agents/conductor_test.go` already imports `context` and `sync/atomic`. If not, add them to the import block:
- `import ("context" "sync/atomic" ...)`.
2. Adjust the `mockDevOpsAgent.Run` signature to match the real `DevOpsAgent` interface (e.g. `Run(ctx context.Context, gctx *GlobalContext) error`) and update the test calls accordingly.
3. Update the argument positions in the `NewConductorAgent` calls if the parameter ordering differs:
- The code above assumes the new `DevOpsAgent` parameter is the argument immediately before `10`, and that the map argument immediately before the final `3` is `disabledAgents`.
4. If `ConductorAgent` does not expose its adapters via a public `adapters` field, replace `conductor.adapters` with the correct accessor (e.g. `conductor.Adapters()`), and adjust the loop type if adapters are stored as pointers or interfaces with a different name.
5. If the adapter interface for `delegate_devops` has a different `Run` method signature, update the type assertion and invocation in `TestConductorRegistersDevOpsDelegateWhenEnabled` to use that signature, while still asserting that the mock’s `runCount` increments when the adapter is invoked.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| t.Helper() | ||
| gctx := newTestGlobalCtx(workDir) | ||
| engine := &mockEngine{} | ||
| return NewConductorAgent(gctx, engine, nil, nil, nil, nil, 10, nil, 3) |
There was a problem hiding this comment.
suggestion (testing): Add tests to cover the new DevOps delegate wiring and disabledAgents behaviour
The Conductor now wires a DevOpsAgent and registers a delegate_devops adapter, controlled by disabledAgents["devops"], but conductor_test.go doesn’t verify this. Please add focused tests that:
- Build a Conductor with a non-nil DevOpsAgent and
disabledAgentsunset or not containing"devops", assert an adapter withName() == "delegate_devops"is registered, and that invoking it callsDevOpsAgent.Run. - Build a Conductor with
disabledAgents["devops"] = trueand assert nodelegate_devopsadapter is registered.
This will ensure the new agent is correctly wired and respects the disable configuration.
Suggested implementation:
t.Helper()
gctx := newTestGlobalCtx(workDir)
engine := &mockEngine{}
return NewConductorAgent(gctx, engine, nil, nil, nil, nil, nil, 10, nil, 3)
}
type mockDevOpsAgent struct {
runCount int32
}
func (m *mockDevOpsAgent) Run(ctx context.Context) error {
atomic.AddInt32(&m.runCount, 1)
return nil
}
func TestConductorRegistersDevOpsDelegateWhenEnabled(t *testing.T) {
t.Helper()
workDir := t.TempDir()
gctx := newTestGlobalCtx(workDir)
engine := &mockEngine{}
devOpsAgent := &mockDevOpsAgent{}
disabledAgents := map[string]bool{}
conductor := NewConductorAgent(
gctx,
engine,
nil, // planner
nil, // coder
nil, // executor
nil, // tools
nil, // git
devOpsAgent,
10, // maxDepth or similar
disabledAgents, // disabledAgents
3, // parallelism or similar
)
var devOpsAdapterFound bool
for _, a := range conductor.adapters {
if a.Name() == "delegate_devops" {
devOpsAdapterFound = true
if r, ok := a.(interface {
Run(ctx context.Context) error
}); ok {
if err := r.Run(context.Background()); err != nil {
t.Fatalf("delegate_devops adapter Run() returned error: %v", err)
}
if got := atomic.LoadInt32(&devOpsAgent.runCount); got != 1 {
t.Fatalf("expected DevOpsAgent.Run to be called once, got %d", got)
}
}
break
}
}
if !devOpsAdapterFound {
t.Fatalf("expected delegate_devops adapter to be registered when devops agent is enabled")
}
}
func TestConductorDoesNotRegisterDevOpsDelegateWhenDisabled(t *testing.T) {
t.Helper()
workDir := t.TempDir()
gctx := newTestGlobalCtx(workDir)
engine := &mockEngine{}
devOpsAgent := &mockDevOpsAgent{}
disabledAgents := map[string]bool{
"devops": true,
}
conductor := NewConductorAgent(
gctx,
engine,
nil, // planner
nil, // coder
nil, // executor
nil, // tools
nil, // git
devOpsAgent,
10,
disabledAgents,
3,
)
for _, a := range conductor.adapters {
if a.Name() == "delegate_devops" {
t.Fatalf("did not expect delegate_devops adapter to be registered when disabledAgents[\"devops\"] is true")
}
}
}
// makeMetaOutput builds a valid Meta-Agent JSON output string.- Ensure
internal/agents/conductor_test.goalready importscontextandsync/atomic. If not, add them to the import block:import ("context" "sync/atomic" ...).
- Adjust the
mockDevOpsAgent.Runsignature to match the realDevOpsAgentinterface (e.g.Run(ctx context.Context, gctx *GlobalContext) error) and update the test calls accordingly. - Update the argument positions in the
NewConductorAgentcalls if the parameter ordering differs:- The code above assumes the new
DevOpsAgentparameter is the argument immediately before10, and that the map argument immediately before the final3isdisabledAgents.
- The code above assumes the new
- If
ConductorAgentdoes not expose its adapters via a publicadaptersfield, replaceconductor.adapterswith the correct accessor (e.g.conductor.Adapters()), and adjust the loop type if adapters are stored as pointers or interfaces with a different name. - If the adapter interface for
delegate_devopshas a differentRunmethod signature, update the type assertion and invocation inTestConductorRegistersDevOpsDelegateWhenEnabledto use that signature, while still asserting that the mock’srunCountincrements when the adapter is invoked.
Summary by Sourcery
Introduce a DevOps-focused operational agent and a TUI command mode optimized for long-running tasks, while updating configuration, prompts, and documentation to reflect the expanded multi-agent capabilities.
New Features:
Enhancements:
Documentation:
Tests: