-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add in-sandbox orchestrator binary with watch mode #85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,8 @@ func NewApplyCmd(harnessDir, cli string) *cobra.Command { | |
| sandboxName string | ||
| task string | ||
| entrypoint string | ||
| mode string | ||
| watch bool | ||
| attach bool | ||
| providerRefresh bool | ||
| dryRun bool | ||
|
|
@@ -50,6 +52,12 @@ then deploy a sandbox. Use --dry-run to validate without deploying, or | |
| agentPath := resolveAgentPath(harnessDir, agentName, file) | ||
|
|
||
| // CLI overrides | ||
| if watch { | ||
| mode = "watch" | ||
| } | ||
| if mode != "" { | ||
| agentCfg.Mode = mode | ||
| } | ||
|
Comment on lines
+55
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Two issues here:
As per coding guidelines, "Use flag resolution order: Explicit flag > 🔧 Proposed fix- cmd.Flags().StringVar(&mode, "mode", "", "Orchestrator mode: once or watch")
+ cmd.Flags().StringVar(&mode, "mode", envOr("OPENSHELL_MODE", ""), "Orchestrator mode: once or watch")
cmd.Flags().BoolVar(&watch, "watch", false, "Shorthand for --mode=watch") if watch {
- mode = "watch"
+ if mode != "" && mode != "watch" {
+ return fmt.Errorf("--mode=%s conflicts with --watch", mode)
+ }
+ mode = "watch"
}
if mode != "" {
+ if mode != "once" && mode != "watch" {
+ return fmt.Errorf("--mode must be \"once\" or \"watch\", got %q", mode)
+ }
agentCfg.Mode = mode
}Also applies to: 154-155 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| if entrypoint != "" { | ||
| agentCfg.Entrypoint = entrypoint | ||
| } | ||
|
|
@@ -143,6 +151,8 @@ then deploy a sandbox. Use --dry-run to validate without deploying, or | |
| cmd.Flags().StringVar(&sandboxName, "name", "", "Sandbox name (overrides agent config)") | ||
| cmd.Flags().StringVar(&task, "task", "", "Task to pass to the agent (inline text or @filepath)") | ||
| cmd.Flags().StringVar(&entrypoint, "entrypoint", "", "Override agent entrypoint (claude, opencode, bash)") | ||
| cmd.Flags().StringVar(&mode, "mode", "", "Orchestrator mode: once or watch") | ||
| cmd.Flags().BoolVar(&watch, "watch", false, "Shorthand for --mode=watch") | ||
| cmd.Flags().BoolVar(&attach, "attach", false, "Attach TTY after creation (interactive mode)") | ||
| cmd.Flags().BoolVar(&providerRefresh, "provider-refresh", false, "Delete and recreate all providers") | ||
| cmd.Flags().BoolVar(&dryRun, "dry-run", false, "Validate configuration without deploying") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "context" | ||
| "flag" | ||
| "fmt" | ||
| "os" | ||
| "os/signal" | ||
| "path/filepath" | ||
| "syscall" | ||
|
|
||
| "github.com/stackrox/harness-openshell/internal/orchestrator" | ||
| ) | ||
|
|
||
| var version = "dev" | ||
|
|
||
| func main() { | ||
| configPath := flag.String("config", "/sandbox/.config/openshell/orchestrator.yaml", "path to orchestrator config") | ||
| showVersion := flag.Bool("version", false, "print version and exit") | ||
| flag.Parse() | ||
|
|
||
| if *showVersion { | ||
| fmt.Println("harness-orchestrator", version) | ||
| os.Exit(0) | ||
| } | ||
|
|
||
| cfg, err := orchestrator.LoadConfig(*configPath) | ||
| if err != nil { | ||
| fmt.Fprintf(os.Stderr, "harness-orchestrator: %v\n", err) | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| configDir := filepath.Dir(*configPath) | ||
| if cfg.Task != "" && !filepath.IsAbs(cfg.Task) { | ||
| cfg.Task = filepath.Join(configDir, cfg.Task) | ||
| } | ||
|
|
||
| ctx, cancel := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) | ||
| defer cancel() | ||
|
|
||
| orch, err := orchestrator.New(cfg, configDir) | ||
| if err != nil { | ||
| fmt.Fprintf(os.Stderr, "harness-orchestrator: %v\n", err) | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| if err := orch.Run(ctx); err != nil { | ||
| fmt.Fprintf(os.Stderr, "harness-orchestrator: %v\n", err) | ||
| os.Exit(1) | ||
| } | ||
|
Comment on lines
+38
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
ast-grep run --pattern 'func (o *Orchestrator) runWatch($_) $_' --lang go internal/orchestrator/orchestrator.go
rg -n -A20 'func \(o \*Orchestrator\) runWatch' internal/orchestrator/orchestrator.goRepository: stackrox/harness-openshell Length of output: 4110 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the orchestrator entrypoint and Run implementation.
grep -n -A40 -B10 'func (o \*Orchestrator) Run' internal/orchestrator/orchestrator.go
printf '\n---\n'
sed -n '1,180p' internal/orchestrator/orchestrator.go
printf '\n---\n'
sed -n '1,120p' cmd/orchestrator/main.goRepository: stackrox/harness-openshell Length of output: 6619 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Check whether any code already treats context cancellation specially at the CLI boundary.
rg -n 'context\.Canceled|context\.DeadlineExceeded|errors\.Is\(.*ctx|signal\.NotifyContext|os\.Exit\(1\)' cmd internalRepository: stackrox/harness-openshell Length of output: 407 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Run a small read-only probe to show the exact error propagated by runWatch on cancellation.
python3 - <<'PY'
from pathlib import Path
p = Path('internal/orchestrator/orchestrator.go')
text = p.read_text()
for needle in [
'func (o *Orchestrator) Run',
'func (o *Orchestrator) runWatch',
'if ctx.Err() != nil {',
'return ctx.Err()',
]:
print(f'== {needle} ==')
for i, line in enumerate(text.splitlines(), 1):
if needle in line:
start = max(1, i-3)
end = min(len(text.splitlines()), i+6)
for j in range(start, end+1):
print(f'{j}:{text.splitlines()[j-1]}')
break
PYRepository: stackrox/harness-openshell Length of output: 1456 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Show the relevant CLI exit path in main.go with line numbers.
nl -ba cmd/orchestrator/main.go | sed -n '1,120p'Repository: stackrox/harness-openshell Length of output: 204 Treat SIGINT/SIGTERM as a clean shutdown.
🤖 Prompt for AI Agents |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| package orchestrator | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "os/exec" | ||
| ) | ||
|
|
||
| type HarnessAdapter interface { | ||
| Name() string | ||
| BuildCommand(task string, headless bool) *exec.Cmd | ||
| } | ||
|
|
||
| func NewAdapter(name string) (HarnessAdapter, error) { | ||
| switch name { | ||
| case "claude": | ||
| return ClaudeAdapter{}, nil | ||
| case "codex": | ||
| return CodexAdapter{}, nil | ||
| case "opencode": | ||
| return OpenCodeAdapter{}, nil | ||
| default: | ||
| return nil, fmt.Errorf("unknown adapter %q", name) | ||
| } | ||
| } | ||
|
|
||
| type ClaudeAdapter struct{} | ||
|
|
||
| func (ClaudeAdapter) Name() string { return "claude" } | ||
|
|
||
| func (ClaudeAdapter) BuildCommand(task string, headless bool) *exec.Cmd { | ||
| if task == "" { | ||
| return exec.Command("claude") | ||
| } | ||
| if headless { | ||
| return exec.Command("claude", "--print", task) | ||
| } | ||
| return exec.Command("claude", "-p", task) | ||
| } | ||
|
|
||
| type CodexAdapter struct{} | ||
|
|
||
| func (CodexAdapter) Name() string { return "codex" } | ||
|
|
||
| func (CodexAdapter) BuildCommand(task string, headless bool) *exec.Cmd { | ||
| if task == "" { | ||
| return exec.Command("codex") | ||
| } | ||
| if headless { | ||
| return exec.Command("codex", "--print", task) | ||
| } | ||
| return exec.Command("codex", "-p", task) | ||
| } | ||
|
Comment on lines
+40
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n## Files mentioning codex / adapter\n'
git ls-files | rg 'internal/orchestrator/adapter\.go|codex|orchestrator' || true
printf '\n## Search for codex references\n'
rg -n --hidden --glob '!**/.git/**' '\bcodex\b|--print|\-p\b|exec subcommand|codex exec|codex e' .
printf '\n## Read target file with line numbers\n'
sed -n '1,140p' internal/orchestrator/adapter.goRepository: stackrox/harness-openshell Length of output: 5751 🌐 Web query:
💡 Result: The OpenAI Codex CLI (a Rust-based coding agent) uses the Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,220p' internal/orchestrator/adapter_test.go
printf '\n---\n'
sed -n '1,220p' internal/orchestrator/orchestrator.goRepository: stackrox/harness-openshell Length of output: 7065 internal/orchestrator/adapter.go:46-51 — Use 🤖 Prompt for AI Agents |
||
|
|
||
| type OpenCodeAdapter struct{} | ||
|
|
||
| func (OpenCodeAdapter) Name() string { return "opencode" } | ||
|
|
||
| func (OpenCodeAdapter) BuildCommand(task string, headless bool) *exec.Cmd { | ||
| if task == "" { | ||
| return exec.Command("opencode") | ||
| } | ||
| return exec.Command("opencode", "run", task) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| package orchestrator | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
| ) | ||
|
|
||
| func TestNewAdapterValid(t *testing.T) { | ||
| for _, name := range []string{"claude", "codex", "opencode"} { | ||
| a, err := NewAdapter(name) | ||
| if err != nil { | ||
| t.Errorf("NewAdapter(%q): %v", name, err) | ||
| } | ||
| if a.Name() != name { | ||
| t.Errorf("Name() = %q, want %q", a.Name(), name) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func TestNewAdapterInvalid(t *testing.T) { | ||
| _, err := NewAdapter("unknown") | ||
| if err == nil { | ||
| t.Error("expected error for unknown adapter") | ||
| } | ||
| } | ||
|
|
||
| func TestClaudeAdapterCommands(t *testing.T) { | ||
| a := ClaudeAdapter{} | ||
|
|
||
| cmd := a.BuildCommand("do stuff", true) | ||
| args := strings.Join(cmd.Args, " ") | ||
| if args != "claude --print do stuff" { | ||
| t.Errorf("headless+task = %q", args) | ||
| } | ||
|
|
||
| cmd = a.BuildCommand("do stuff", false) | ||
| args = strings.Join(cmd.Args, " ") | ||
| if args != "claude -p do stuff" { | ||
| t.Errorf("tty+task = %q", args) | ||
| } | ||
|
|
||
| cmd = a.BuildCommand("", false) | ||
| args = strings.Join(cmd.Args, " ") | ||
| if args != "claude" { | ||
| t.Errorf("no task = %q", args) | ||
| } | ||
| } | ||
|
|
||
| func TestCodexAdapterCommands(t *testing.T) { | ||
| a := CodexAdapter{} | ||
|
|
||
| cmd := a.BuildCommand("review code", true) | ||
| args := strings.Join(cmd.Args, " ") | ||
| if args != "codex --print review code" { | ||
| t.Errorf("headless+task = %q", args) | ||
| } | ||
|
|
||
| cmd = a.BuildCommand("review code", false) | ||
| args = strings.Join(cmd.Args, " ") | ||
| if args != "codex -p review code" { | ||
| t.Errorf("tty+task = %q", args) | ||
| } | ||
| } | ||
|
|
||
| func TestOpenCodeAdapterCommands(t *testing.T) { | ||
| a := OpenCodeAdapter{} | ||
|
|
||
| cmd := a.BuildCommand("fix bugs", true) | ||
| args := strings.Join(cmd.Args, " ") | ||
| if args != "opencode run fix bugs" { | ||
| t.Errorf("headless+task = %q", args) | ||
| } | ||
|
|
||
| cmd = a.BuildCommand("fix bugs", false) | ||
| args = strings.Join(cmd.Args, " ") | ||
| if args != "opencode run fix bugs" { | ||
| t.Errorf("tty+task (opencode always uses run) = %q", args) | ||
| } | ||
|
|
||
| cmd = a.BuildCommand("", false) | ||
| args = strings.Join(cmd.Args, " ") | ||
| if args != "opencode" { | ||
| t.Errorf("no task = %q", args) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
orchestratortarget hardcodesGOARCH=amd64, conflicting with native-arch dev builds.dev-sandboxis documented as "native arch only" (line 98) and the Dockerfile downloads arch-specific binaries viauname -m(e.g. lines 52-54), implying the sandbox image is built for the host's native architecture. But this target unconditionally cross-compilesharness-orchestratorforlinux/amd64. On an arm64 dev host (e.g. Apple Silicon),make dev-sandboxwill bake an amd64 binary into an arm64 image, causing an exec-format failure at container runtime — and sinceexecutor.goonly checkscommand -v harness-orchestrator(existence, not architecture), there is no fallback torun.shin that case.🔧 Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents