diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 7351888c..73182e6f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -50,3 +50,31 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Pushes the Homebrew formula to spinloop-ai/homebrew-tap. HOMEBREW_TAP_GITHUB_TOKEN: ${{ secrets.HOMEBREW_TAP_GITHUB_TOKEN }} + + agent-image: + needs: test + runs-on: ubuntu-latest + permissions: + contents: read + packages: write + steps: + - uses: actions/checkout@v7 + + # The tag names the image the same way it names the release — GoReleaser + # stamps main.version from it in the binaries job above, and + # DefaultDockerImage builds --dispatch docker's default from the same + # value at runtime, so the two never drift apart. + - name: Resolve the image tag + id: tag + run: echo "version=${GITHUB_REF_NAME#v}" >> "$GITHUB_OUTPUT" + + - name: Log in to ghcr.io + run: echo "${{ secrets.GITHUB_TOKEN }}" | docker login ghcr.io -u "${{ github.actor }}" --password-stdin + + - name: Build and push + run: | + docker buildx build --push \ + --platform linux/amd64,linux/arm64 \ + -t "ghcr.io/spinloop-ai/agent:${{ steps.tag.outputs.version }}" \ + -t ghcr.io/spinloop-ai/agent:latest \ + images/agent diff --git a/cmd/spinloop/complete.go b/cmd/spinloop/complete.go index f66d1cf2..e295168d 100644 --- a/cmd/spinloop/complete.go +++ b/cmd/spinloop/complete.go @@ -176,6 +176,12 @@ func compLogLevel(_ *cobra.Command, _ []string, _ string) ([]string, cobra.Shell return daemon.LevelNames(), cobra.ShellCompDirectiveNoFileComp } +// compDispatchBackends offers the dispatch backends spinloop orchestrator +// accepts. +func compDispatchBackends(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { + return []string{"bare", "docker"}, cobra.ShellCompDirectiveNoFileComp +} + // compFiles offers no static candidates and lets the shell fall back to // filesystem paths. func compFiles(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { diff --git a/cmd/spinloop/complete_test.go b/cmd/spinloop/complete_test.go index a0973b70..4ff9666b 100644 --- a/cmd/spinloop/complete_test.go +++ b/cmd/spinloop/complete_test.go @@ -409,6 +409,14 @@ func TestComplete_FlagValues(t *testing.T) { if _, directive := complete(t, "orchestrator", "--fleet", ""); directive != directiveFile { t.Errorf("orchestrator --fleet should complete paths, got %q", directive) } + // --harness-config names a file too. + if _, directive := complete(t, "orchestrator", "--harness-config", ""); directive != directiveFile { + t.Errorf("orchestrator --harness-config should complete paths, got %q", directive) + } + // --dispatch offers the two backends. + if got, _ := complete(t, "orchestrator", "--dispatch", ""); !hasAll(got, "bare", "docker") { + t.Errorf("orchestrator --dispatch: bare/docker missing from %v", got) + } if got, _ := complete(t, "completion", ""); !hasAll(got, "bash", "zsh", "powershell") { t.Errorf("shells missing from %v", got) } diff --git a/cmd/spinloop/orchestrator.go b/cmd/spinloop/orchestrator.go index baa25047..64d7fa5d 100644 --- a/cmd/spinloop/orchestrator.go +++ b/cmd/spinloop/orchestrator.go @@ -31,6 +31,7 @@ func orchestratorCmd() *cobra.Command { var createItemDirs bool var listen, apiToken, apiTokenFile string var loopback bool + var dispatchBackend, dispatchImage, harnessConfigPath string c := &cobra.Command{ Use: "orchestrator", Short: "work a backlog of items against the fleet, at the fleet's pace", @@ -81,7 +82,7 @@ without a separate call to spinloop work list.`, if err != nil { return err } - return runOrchestratorCommand(gateway, itemsPath, tokenVar, harnessName, logLevel, createItemDirs, listenAddr, apiToken, apiTokenFile) + return runOrchestratorCommand(gateway, itemsPath, tokenVar, harnessName, c.Flags().Changed("harness"), logLevel, createItemDirs, listenAddr, apiToken, apiTokenFile, dispatchBackend, c.Flags().Changed("dispatch"), dispatchImage, harnessConfigPath) }, } @@ -98,9 +99,14 @@ without a separate call to spinloop work list.`, fs.BoolVarP(&loopback, "loopback", "l", false, "serve the work list API on loopback on the default port ("+orchestrator.LoopbackListen+"); needs no token") fs.StringVar(&apiTokenFile, "api-token-file", "", "read the work list API's bearer token from this file") fs.StringVar(&apiToken, "api-token", "", "the work list API's bearer token") + fs.StringVar(&dispatchBackend, "dispatch", "bare", "how an admitted item's agent runs: bare (a host process) or docker (a container)") + fs.StringVar(&dispatchImage, "dispatch-image", "", "the agent image the docker backend runs (default: ghcr.io/spinloop-ai/agent:); only meaningful with --dispatch docker") + fs.StringVar(&harnessConfigPath, "harness-config", "", "the harness.yaml to read (default: harness.yaml beside the items file, where one exists)") compRegister(c, "items", compFiles) compRegister(c, "harness", compHarnessNames) compRegister(c, "log-level", compLogLevel) + compRegister(c, "dispatch", compDispatchBackends) + compRegister(c, "harness-config", compFiles) return c } @@ -164,8 +170,23 @@ func orchestratorListenAddr(listen string, listenExplicit, loopback bool) (strin // and then the work list API standing before the run's first pass, the way // the gateway has its handler in before a signal can arrive, held until the // signal ends the run and the server goes down with it. -func runOrchestratorCommand(gatewayAddr, itemsPath, token, harnessName, logLevel string, createItemDirs bool, listenAddr, apiToken, apiTokenFile string) error { - h, _, err := harness.Resolve(harnessName) +func runOrchestratorCommand(gatewayAddr, itemsPath, token, harnessName string, harnessChanged bool, logLevel string, createItemDirs bool, listenAddr, apiToken, apiTokenFile, dispatchBackend string, dispatchChanged bool, dispatchImage, harnessConfigPath string) error { + hc, err := orchestrator.LoadHarnessConfig(harnessConfigPath, itemsPath) + if err != nil { + return err + } + if err := orchestrator.CheckHarnessEnvCollision(hc.Env); err != nil { + return err + } + + // --harness, given explicitly, wins outright; otherwise harness.yaml's + // own harness: is tried before harness.Resolve's own env-var/stored- + // preference/default chain. + resolveName := harnessName + if !harnessChanged && hc.Harness != "" { + resolveName = hc.Harness + } + h, _, err := harness.Resolve(resolveName) if err != nil { return err } @@ -190,7 +211,31 @@ func runOrchestratorCommand(gatewayAddr, itemsPath, token, harnessName, logLevel } defer ln.Close() - dispatch := orchestrator.NewDispatcher(h, gatewayAddr, token, createItemDirs) + // --dispatch, given explicitly, wins outright; otherwise harness.yaml's + // own dispatch: is the run's choice, and the flag's default ("bare") + // only where neither says anything. + backend, backendSource := dispatchBackend, "--dispatch" + if !dispatchChanged && hc.Dispatch != "" { + backend, backendSource = hc.Dispatch, "harness.yaml's dispatch" + } + + var dispatch orchestrator.Launcher + switch backend { + case "", "bare": + dispatch = orchestrator.NewDispatcher(h, gatewayAddr, token, createItemDirs).WithHarnessConfig(hc).WithBaseDir(hc.BaseDir) + case "docker": + if err := orchestrator.CheckDockerReachable(); err != nil { + return err + } + image := dispatchImage + if image == "" { + image = orchestrator.DefaultDockerImage(version) + } + dispatch = orchestrator.NewDockerLauncher(h, gatewayAddr, token, image, createItemDirs).WithHarnessConfig(hc).WithBaseDir(hc.BaseDir) + default: + return fmt.Errorf("unknown %s %q: expected bare or docker", backendSource, backend) + } + store, err := orchestrator.OpenStore(itemsPath) if err != nil { return err @@ -199,6 +244,7 @@ func runOrchestratorCommand(gatewayAddr, itemsPath, token, harnessName, logLevel if err != nil { return err } + wl = wl.WithBaseDir(hc.BaseDir) // The store's lock goes with the process, after the server has gone // down and with it any request that could still touch the store. defer wl.Close() diff --git a/cmd/spinloop/orchestrator_test.go b/cmd/spinloop/orchestrator_test.go index a5bf90a9..a7137de2 100644 --- a/cmd/spinloop/orchestrator_test.go +++ b/cmd/spinloop/orchestrator_test.go @@ -543,3 +543,158 @@ func TestCmdOrchestrator_StartupShowsARestartsRecoveredState(t *testing.T) { t.Fatal("the interrupt did not end the run") } } + +// An unrecognised --dispatch stops the command before it works an item, +// naming the flag, the value, and the accepted set. +func TestCmdOrchestrator_AnUnrecognisedDispatchBackendNamesIt(t *testing.T) { + isolateConfig(t) + t.Setenv("OPENAI_API_KEY", "the-token") + dir := t.TempDir() + t.Chdir(dir) + mustWrite(t, "work.yaml", "- id: a\n instructions: do\n dir: .\n") + err := cmdOrchestrator([]string{"--gateway", "http://gw:4000", "-l", "--dispatch", "vm"}) + if err == nil { + t.Fatal("an unrecognised --dispatch should stop the command") + } + if !strings.Contains(err.Error(), "--dispatch") || !strings.Contains(err.Error(), "vm") || + !strings.Contains(err.Error(), "bare") || !strings.Contains(err.Error(), "docker") { + t.Errorf("the failure should name the flag, the value, and the accepted set, got %v", err) + } + if _, err := os.Stat("work.yaml.state.json"); !os.IsNotExist(err) { + t.Errorf("a run that worked no item records no state, got %v", err) + } +} + +// harness.yaml's dispatch: is the run's choice where --dispatch is not +// given: naming a value the command does not recognise stops it, naming +// harness.yaml's own field rather than the flag. +func TestCmdOrchestrator_HarnessYamlDispatchIsUsedWhenFlagNotGiven(t *testing.T) { + isolateConfig(t) + t.Setenv("OPENAI_API_KEY", "the-token") + dir := t.TempDir() + t.Chdir(dir) + mustWrite(t, "work.yaml", "- id: a\n instructions: do\n dir: .\n") + mustWrite(t, "harness.yaml", "dispatch: vm\n") + err := cmdOrchestrator([]string{"--gateway", "http://gw:4000", "-l"}) + if err == nil { + t.Fatal("an unrecognised harness.yaml dispatch should stop the command") + } + if !strings.Contains(err.Error(), "harness.yaml") || !strings.Contains(err.Error(), "vm") { + t.Errorf("the failure should name harness.yaml's own field and the value, got %v", err) + } +} + +// An explicit --dispatch wins over harness.yaml's dispatch:, even where +// harness.yaml names a value the command would otherwise refuse. +func TestCmdOrchestrator_ExplicitDispatchFlagWinsOverHarnessYaml(t *testing.T) { + isolateConfig(t) + t.Setenv("OPENAI_API_KEY", "the-token") + dir := t.TempDir() + t.Chdir(dir) + mustWrite(t, "work.yaml", "- id: a\n instructions: do\n dir: .\n") + mustWrite(t, "harness.yaml", "dispatch: vm\n") + // bare, explicitly, reaches the (unreachable) gateway's own failure — + // not harness.yaml's unrecognised value, which --dispatch bare never + // consults. + err := cmdOrchestrator([]string{"--gateway", "http://127.0.0.1:1", "-l", "--dispatch", "bare"}) + if err == nil || !strings.Contains(err.Error(), "http://127.0.0.1:1") { + t.Errorf("an explicit --dispatch should win over harness.yaml's, got %v", err) + } +} + +// --dispatch docker checks the daemon once, at startup: an unreachable one +// stops the command before it works an item, naming the fix. +func TestCmdOrchestrator_DispatchDockerChecksTheDaemonAtStartup(t *testing.T) { + isolateConfig(t) + t.Setenv("OPENAI_API_KEY", "the-token") + // No `docker` on PATH stands in for a daemon that does not answer: both + // are the docker backend's own reachability check failing before any + // item is worked. + t.Setenv("PATH", t.TempDir()) + dir := t.TempDir() + t.Chdir(dir) + mustWrite(t, "work.yaml", "- id: a\n instructions: do\n dir: .\n") + err := cmdOrchestrator([]string{"--gateway", "http://gw:4000", "-l", "--dispatch", "docker"}) + if err == nil { + t.Fatal("an unreachable docker daemon should stop the command") + } + if !strings.Contains(err.Error(), "docker") { + t.Errorf("the failure should name the fix, got %v", err) + } + if _, err := os.Stat("work.yaml.state.json"); !os.IsNotExist(err) { + t.Errorf("a run that worked no item records no state, got %v", err) + } +} + +// A harness.yaml whose env collides with the token's own variable stops +// the command before it works an item, whether the file is the default +// beside the items file or one --harness-config names. +func TestCmdOrchestrator_HarnessYamlEnvCollisionNamesTheVariable(t *testing.T) { + isolateConfig(t) + t.Setenv("OPENAI_API_KEY", "the-token") + dir := t.TempDir() + t.Chdir(dir) + mustWrite(t, "work.yaml", "- id: a\n instructions: do\n dir: .\n") + mustWrite(t, "harness.yaml", "env:\n OPENAI_API_KEY: nope\n") + err := cmdOrchestrator([]string{"--gateway", "http://gw:4000", "-l"}) + if err == nil || !strings.Contains(err.Error(), "OPENAI_API_KEY") { + t.Errorf("the failure should name the colliding variable, got %v", err) + } + if _, err := os.Stat("work.yaml.state.json"); !os.IsNotExist(err) { + t.Errorf("a run that worked no item records no state, got %v", err) + } +} + +// --harness-config names a file read in preference to harness.yaml beside +// the items file. +func TestCmdOrchestrator_HarnessConfigFlagOverridesTheDefault(t *testing.T) { + isolateConfig(t) + t.Setenv("OPENAI_API_KEY", "the-token") + dir := t.TempDir() + t.Chdir(dir) + mustWrite(t, "work.yaml", "- id: a\n instructions: do\n dir: .\n") + mustWrite(t, "harness.yaml", "env:\n OPENAI_API_KEY: nope\n") + mustWrite(t, "other.yaml", "env:\n FOO: bar\n") + err := cmdOrchestrator([]string{"--gateway", "http://127.0.0.1:1", "-l", "--harness-config", "other.yaml"}) + // The default harness.yaml's collision would have stopped the command + // before it ever reached the gateway; reaching the (unreachable) + // gateway's own failure instead shows the flag's file was the one read. + if err == nil || !strings.Contains(err.Error(), "http://127.0.0.1:1") { + t.Errorf("the flag's file should be read instead of the colliding default, got %v", err) + } +} + +// harness.yaml's own harness: is used where --harness is not given. +func TestCmdOrchestrator_HarnessYamlHarnessIsUsedWhenFlagNotGiven(t *testing.T) { + isolateConfig(t) + t.Setenv("OPENAI_API_KEY", "the-token") + dir := t.TempDir() + t.Chdir(dir) + mustWrite(t, "work.yaml", "- id: a\n instructions: do\n dir: .\n") + mustWrite(t, "harness.yaml", "harness: not-a-real-harness\n") + err := cmdOrchestrator([]string{"--gateway", "http://gw:4000", "-l"}) + if err == nil { + t.Fatal("an unrecognised harness.yaml harness should stop the command") + } + if !strings.Contains(err.Error(), "not-a-real-harness") { + t.Errorf("the failure should name harness.yaml's own value, got %v", err) + } +} + +// An explicit --harness wins over harness.yaml's harness:, even where +// harness.yaml names a value the command would otherwise refuse. +func TestCmdOrchestrator_ExplicitHarnessFlagWinsOverHarnessYaml(t *testing.T) { + isolateConfig(t) + t.Setenv("OPENAI_API_KEY", "the-token") + dir := t.TempDir() + t.Chdir(dir) + mustWrite(t, "work.yaml", "- id: a\n instructions: do\n dir: .\n") + mustWrite(t, "harness.yaml", "harness: not-a-real-harness\n") + // opencode, explicitly, reaches the (unreachable) gateway's own + // failure — not harness.yaml's unrecognised value, which an explicit + // --harness never consults. + err := cmdOrchestrator([]string{"--gateway", "http://127.0.0.1:1", "-l", "--harness", "opencode"}) + if err == nil || !strings.Contains(err.Error(), "http://127.0.0.1:1") { + t.Errorf("an explicit --harness should win over harness.yaml's, got %v", err) + } +} diff --git a/docs/commands/orchestrator.md b/docs/commands/orchestrator.md index 24d6c1d7..631507ba 100644 --- a/docs/commands/orchestrator.md +++ b/docs/commands/orchestrator.md @@ -5,7 +5,7 @@ absorb. The orchestrator holds the backlog, reads the fleet's topology from the fleet's [gateway](gateway.md), admits an item while the fleet's declared [concurrency](fleet.md#concurrency) limits allow, and runs each admitted item as a one-shot agent of the [active harness](harness.md) in the item's own -directory — the agent's inference going through that same gateway. +workspace directory — the agent's inference going through that same gateway. ```sh spinloop orchestrator @@ -36,8 +36,8 @@ reaches each agent as its key. ## The items file -A list of items, each with an id, the instructions its agent is given, and the -directory the agent works in: +A list of items, each with an id, the instructions its agent is given, and its +own directory: ```yaml - id: fix-the-parser @@ -54,9 +54,11 @@ directory the agent works in: - **`id`** — unique in the file; the state and the per-item log are keyed by it. - **`instructions`** — the task the agent is given, whole. -- **`dir`** — the working directory the agent runs in. A missing directory - fails that item and only that item; the rest of the backlog goes on. With - `--create-item-dirs`, the orchestrator creates a missing directory instead. +- **`dir`** — the item's own directory: everything the orchestrator keeps + about that item's launch lives under it (see [What it + keeps](#what-it-keeps)). A missing directory fails that item and only that + item; the rest of the backlog goes on. With `--create-item-dirs`, the + orchestrator creates a missing directory instead. - **`priority`** — an integer, higher first; items of one rank go in file order. - **`tags`** — `key=value` pairs naming the [tags](fleet.md#tags) of the nodes @@ -69,7 +71,7 @@ backlog, and an item dropped from the file keeps its recorded end. ## What it keeps -Beside the items file, and only beside it: +Beside the items file: ``` work.yaml the items @@ -79,6 +81,25 @@ work.yaml.logs/ one log per item, its agent's output work.yaml.aborts/ the abort markers left beside the file, while they stand ``` +And inside each item's own directory (`dir` in the items file): + +``` +/workspace/ the harness's own working directory +/config/ under --dispatch docker: one item's scoped config, while it runs +/log a second copy of the item's kept output, written once the agent ends +``` + +`workspace/` is created on every launch, under either backend, if it is not +already there; the operator's own files elsewhere in `dir` are left alone. +`config/` only appears under the docker backend, and only while the item +runs. `log` is a static copy, not a live one: `work logs`/the work list API +read the canonical copy beside the items file, which streams while the +agent runs; `/log` is written once the agent ends, so the item's own +directory carries everything about its launch on its own. `work remove` +takes both `config/` and `log` out with the item's kept output — +`workspace/` is left alone, since it can hold real output an operator +wants to keep. + The state holds each item's `running`/`done`/`failed` record — the node a running one is on, the reason a failed one failed — and an item with no entry is in the backlog. A run that ends in a crash leaves its in-flight items @@ -164,13 +185,20 @@ API's token never reaches a node or an agent. ## How an item runs An admitted item is launched as the harness's non-interactive single-task -form — `opencode run` or `pi --print` — in the item's directory. The model it +form — `opencode run` or `pi --print` — in the item's workspace directory +(`/workspace/`, see [What it keeps](#what-it-keeps)). The model it runs against is the gateway and the chosen node: the node's model, under a provider the orchestrator writes into the harness config for the run, with the gateway's OpenAI-compatible address as the base URL — its address plus `/v1` where it names none — and the gateway's token as the key. A harness with no single-task form — lucinate — is refused at startup, naming it. +opencode's launch carries `--auto`: with no terminal for a permission prompt +to reach, one otherwise blocks the item forever, or is auto-rejected off a +terminal, silently stopping the agent from doing the item's own work. Every +one-shot launch trusts every tool the item's instructions call for — there is +no way, at dispatch time, to know which ones it will need. + The node an item takes is the fleet's own routing, read through the gateway: an item matches a node only where every tag it names is one the node carries; a node already running and answering is offered before a node the run would @@ -178,6 +206,90 @@ have to start, and among a tier the fleet file's `prefer` ranks them. A stopped node is an option only where the fleet file [wakes](fleet.md#waking). +### Where an item's agent runs: `--dispatch` + +`--dispatch` chooses how an admitted item's agent actually runs, for the +whole run — not per node, not per item: + +- **`bare`** (the default) — a process on the orchestrator's own host, in + its own process group, the harness's config the host's own — exactly + what running spinloop directly has always done. +- **`docker`** — a container from the official spinloop agent image + (opencode, Pi and `gh`), on its own network. A gateway bound to the + orchestrator host's own loopback (`localhost`, `127.0.0.1` — what a + gateway run on the same machine usually is) reaches the container by + `host.docker.internal` instead: nothing to change in the fleet file or + `--gateway`, and no reliance on a Docker Desktop setting most operators + do not have on for `--network host` to reach the real host on macOS or + Windows. A gateway already on a routable address is unaffected. The + container mounts the item's own `workspace/` and `config/` directories + (see [What it keeps](#what-it-keeps)) — the config scoped to that one + launch alone, never the host's own harness config, carrying just the + provider this launch needs. `docker version` is checked once, at + startup: an unreachable daemon stops the command before it works an + item, naming the fix. `--dispatch-image` names the image to run, + defaulting to `ghcr.io/spinloop-ai/agent:` — the + image built alongside that release. + +[`harness.yaml`](#harnessyaml-environment-and-lifecycle-scripts) can also +name the backend, with `dispatch:` — useful for keeping the choice with +the rest of an item's environment rather than in a wrapper script that +adds the flag. An explicit `--dispatch` on the command line wins over it. + +Stopping an item — an abort, or the run's own clean interrupt — stops +either the same way: the polite signal first, then, where the grace runs +out, the hard end. See [`images/agent/README.md`](../../images/agent/README.md) +for what the official image carries. + +## `harness.yaml`: environment and lifecycle scripts + +Beyond the fleet file and the flags, an operator can shape the environment +an item's agent runs in without building a new image: `harness.yaml`, +found beside the items file by default, or named with +`--harness-config `. Where neither is present, nothing changes. + +```yaml +dispatch: docker +harness: opencode +baseDir: ../work +env: + GH_TOKEN: ghp_... + SOME_TOOL_FLAG: "1" +startup: | + git config --global user.email "agent@example.com" + git config --global user.name "Agent" +shutdown: | + echo "item finished" >> /tmp/agent-activity.log +``` + +- **`dispatch`** — the backend the run uses, `bare` or `docker`, the way + `--dispatch` does. An explicit `--dispatch` wins over it; where the flag + is not given, this is the run's choice. +- **`harness`** — the harness the run uses, the way `--harness`/`-H` does. + An explicit `--harness` wins over it; where the flag is not given, this + is tried before the `HARNESS` environment variable and the stored + preference (`spinloop harness use`). +- **`baseDir`** — the directory an item's own relative `dir` (see [The + items file](#the-items-file)) resolves against, in place of the + directory the orchestrator command happens to be started from. An + item's `dir` that is already absolute is unaffected. A relative + `baseDir` resolves against `harness.yaml`'s own directory, so the file + stays portable together with the items it describes. +- **`env`** — a map added to every launch's environment, under either + backend. An entry naming the same variable the gateway's token is + presented under is refused, naming it, before the command works an item. +- **`startup`** — a shell script run before the harness, in the item's + workspace directory, under the launch's full environment. A startup + script that exits non-zero fails the item, naming the script, before the + harness ever runs. +- **`shutdown`** — a shell script run once the harness has ended — + cleanly, failed, or aborted alike — provided a startup script ran at + all. An abort's own answer waits for it, bound by the same grace the + harness's own stop already has. + +Both scripts' output joins the harness's own in the item's kept log, in +the order they ran: startup's, then the harness's, then shutdown's. + ## What it does not do - It does not start or stop anything on a node. A stopped node appears in the @@ -210,6 +322,9 @@ stopped node is an option only where the fleet file | `--api-token ` | The work list API's bearer token | | `-H`, `--harness ` | Which harness to run the agents with (default the resolved one) | | `--log-level` | `debug`, `info`, `warn`, or `error` — overrides `SPINLOOP_LOG_LEVEL` (default `info`) | +| `--dispatch ` | How an admitted item's agent runs: `bare` (default) or `docker` | +| `--dispatch-image ` | The agent image the docker backend runs (default `ghcr.io/spinloop-ai/agent:`); only meaningful with `--dispatch docker` | +| `--harness-config ` | The `harness.yaml` to read (default: one beside the items file, where it exists) | ## See also diff --git a/examples/gateway-docker/run-tests.sh b/examples/gateway-docker/run-tests.sh index f2483e20..39fb25fe 100755 --- a/examples/gateway-docker/run-tests.sh +++ b/examples/gateway-docker/run-tests.sh @@ -678,13 +678,13 @@ EOF assert_equals "the items ended done" "$(state_count done)" "3" assert_equals "the fleet's total held the in-flight count" "${max_inflight}" "2" assert_contains "the tagged item took the node carrying its tag" \ - "$(cat "${sandbox}/work/a/args.txt")" "spinloop-orchestrator-node-a" + "$(cat "${sandbox}/work/a/workspace/args.txt")" "spinloop-orchestrator-node-a" assert_contains "an untagged item ran too" \ - "$(cat "${sandbox}/work/b/args.txt")" "spinloop-orchestrator-" + "$(cat "${sandbox}/work/b/workspace/args.txt")" "spinloop-orchestrator-" assert_contains "the agent was given the gateway's token as its key" \ - "$(cat "${sandbox}/work/a/key.txt")" "key=${GATEWAY_TOKEN}" + "$(cat "${sandbox}/work/a/workspace/key.txt")" "key=${GATEWAY_TOKEN}" assert_contains "the agent's inference reached the gateway, at the address its config named" \ - "$(cat "${sandbox}/work/a/reply.txt" 2>/dev/null)" "Hello from the fake engine" + "$(cat "${sandbox}/work/a/workspace/reply.txt" 2>/dev/null)" "Hello from the fake engine" kill -INT "${orch_pid}" 2>/dev/null || true local rc=0 @@ -827,13 +827,13 @@ EOF assert_contains "the abort is answered for the item it stopped" "${body}" '"id":"c"' deadline=$((SECONDS + 60)) while (( SECONDS < deadline )); do - if [[ "$(wc -l < "${sandbox}/work/c/launches.txt" 2>/dev/null | tr -d ' ')" == "2" ]]; then + if [[ "$(wc -l < "${sandbox}/work/c/workspace/launches.txt" 2>/dev/null | tr -d ' ')" == "2" ]]; then break fi sleep 0.2 done assert_equals "the aborted item is admitted again, launched a second time" \ - "$(wc -l < "${sandbox}/work/c/launches.txt" 2>/dev/null | tr -d ' ')" "2" + "$(wc -l < "${sandbox}/work/c/workspace/launches.txt" 2>/dev/null | tr -d ' ')" "2" # A remove of an ended item takes the item, its record, and its output. body="$(curl -s -X DELETE "${base}/v1/items/a")" diff --git a/examples/orchestrator/README.md b/examples/orchestrator/README.md new file mode 100644 index 00000000..9f7e467e --- /dev/null +++ b/examples/orchestrator/README.md @@ -0,0 +1,74 @@ +# A backlog you can actually run + +[`spinloop orchestrator`](../../docs/commands/orchestrator.md) working a +one-item backlog against a real (if fake-engined) gateway, with a +[`harness.yaml`](harness.yaml) shaping the launch — so you can see what an +item's environment and lifecycle hooks actually do before pointing this at +real work. + +This example holds no gateway of its own: it points at +[`examples/gateway-docker`](../gateway-docker/), which is real enough for +this — a real `spinloop gateway` in front of two real daemons, answering +OpenAI-shaped chat completions from a fake engine. An opencode or Pi launch +through it gets back a real (canned) reply, not a connection refused. + +```sh +# once, in examples/gateway-docker/ +cp .env.example .env +docker compose up -d --build +set -a && . ./.env && set +a + +# from this directory — picks up ./work.yaml and ./harness.yaml, both by +# being the defaults beside the command +spinloop orchestrator --fleet ../gateway-docker/fleet.yaml -l +``` + +In another terminal: + +```sh +spinloop work list --url http://127.0.0.1:4010 +``` + +`describe-repo` runs once admitted — its actual working directory is +`workspace/`, created fresh under `dir` on launch, hence `work.yaml`'s +instructions reading `../README.md` rather than one already in its cwd. +`harness.yaml`'s `startup` sets a throwaway git identity before the agent +starts, and its `shutdown` line lands in the kept log after the agent's own +output: + +```sh +cat work.yaml.logs/describe-repo.log +``` + +## Trying `--dispatch docker` + +The same backlog, the same `harness.yaml`, the agent in a container instead +of a bare process — build the image once, from the repository root: + +```sh +docker build -t spinloop-agent:local -f ../../images/agent/Dockerfile ../../images/agent + +spinloop orchestrator --fleet ../gateway-docker/fleet.yaml -l \ + --dispatch docker --dispatch-image spinloop-agent:local +``` + +`docker ps` shows the container while `describe-repo` runs. Everything else — +the work list, the log, `harness.yaml`'s hooks — is the same either way; that +sameness is the point of `--dispatch` being one flag rather than two +different things to learn. + +## The files + +| File | What it is | +| --- | --- | +| [`work.yaml`](work.yaml) | One item: read this directory's own README and summarise it. Change `dir` to point anywhere you actually want an agent's storage — its real working directory is `/workspace/`. | +| [`harness.yaml`](harness.yaml) | The `--dispatch` backend, the harness, a base directory for items' own `dir`, environment variables, and `startup`/`shutdown` scripts — a flag on the command line still wins over its `dispatch:`/`harness:`. Found automatically, beside `work.yaml`. | + +## See also + +- [`spinloop orchestrator`](../../docs/commands/orchestrator.md) — the + command this example drives, `--dispatch` and `harness.yaml` included +- [`examples/gateway-docker/`](../gateway-docker/) — the gateway this points + at, and what is real and fake about it +- [`images/agent/`](../../images/agent/) — the official image `--dispatch + docker` runs diff --git a/examples/orchestrator/harness.yaml b/examples/orchestrator/harness.yaml new file mode 100644 index 00000000..76fdeccc --- /dev/null +++ b/examples/orchestrator/harness.yaml @@ -0,0 +1,57 @@ +# spinloop orchestrator's own way to shape an item's environment and +# lifecycle without building a new docker image — see +# ../../docs/commands/orchestrator.md. +# +# Read automatically, because it sits beside work.yaml — the default +# --items — in this directory; --harness-config names one explicitly +# elsewhere. Applies to every launch, under either --dispatch backend +# (bare or docker) alike. + +# The backend, the way --dispatch names one. Set here so the choice lives +# with the rest of the item's environment rather than in a wrapper script +# that adds the flag — but an explicit --dispatch on the command line +# still wins over it. bare here is what the README's first walkthrough +# runs; its "Trying --dispatch docker" section passes the flag explicitly, +# overriding this, which is the precedence rule in action, not a special +# case. +dispatch: bare + +# The harness the run uses, the way --harness/-H does — an explicit +# --harness still wins over it. Set here so the walkthrough runs against +# opencode regardless of the HARNESS environment variable or a stored +# preference (`spinloop harness use`) on whatever machine runs it. +harness: opencode + +# The directory an item's own relative dir (work.yaml's dir: .) resolves +# against, in place of wherever the orchestrator command happens to be +# started from. `.` here is a no-op — harness.yaml already lives in this +# directory, which is where dir: . already resolves with no baseDir at +# all — but the field earns its keep once this file and work.yaml are +# copied elsewhere and run from a different working directory: baseDir +# keeps dir: . pointing at this directory regardless. +baseDir: . + +# Environment variables added to every launch. Values are literal — there +# is no ${VAR} expansion — so a real secret here is a real secret on disk; +# write the value your shell would otherwise export. An entry naming the +# same variable the gateway's token is presented under is refused, naming +# it, before the command works an item, so the token itself can't be +# shadowed by accident. +env: + GH_TOKEN: ghp_your_token_here # so an item's `gh pr create` works + GIT_AUTHOR_NAME: spinloop-agent + GIT_AUTHOR_EMAIL: agent@example.com + +# Runs before the harness, in the item's workspace directory, under the +# launch's full environment. A non-zero exit fails the item, naming the +# script, before the harness ever runs. +startup: | + git config --global user.name "$GIT_AUTHOR_NAME" + git config --global user.email "$GIT_AUTHOR_EMAIL" + git config --global --add safe.directory "$PWD" + +# Runs once the harness has ended — done, failed, or aborted alike — +# provided startup ran at all. Both scripts' output joins the harness's +# own in the item's kept log, in the order they ran. +shutdown: | + echo "item finished at $(date -u +%FT%TZ)" diff --git a/examples/orchestrator/work.yaml b/examples/orchestrator/work.yaml new file mode 100644 index 00000000..0c69f896 --- /dev/null +++ b/examples/orchestrator/work.yaml @@ -0,0 +1,15 @@ +# A minimal backlog `spinloop orchestrator` works. One item, one directory +# — pointed at wherever you actually want an agent to work. `dir` is the +# item's own directory: the agent's actual working directory is its +# workspace/ subdirectory (see docs/commands/orchestrator.md's "What it +# keeps"), created fresh on every launch — hence reading the README one +# level up, rather than one already sitting in the agent's own cwd. +# +# spinloop orchestrator --fleet ../fleet-docker/fleet.yaml -l +# spinloop work add --url http://127.0.0.1:4010 --id describe-repo \ +# --instructions "read ../README.md and summarise what this project does" \ +# --dir . + +- id: describe-repo + instructions: read ../README.md and summarise what this project does + dir: . diff --git a/images/agent/Dockerfile b/images/agent/Dockerfile new file mode 100644 index 00000000..9a622beb --- /dev/null +++ b/images/agent/Dockerfile @@ -0,0 +1,47 @@ +# The official spinloop agent image: opencode, Pi and gh, nothing of any +# operator's own harness configuration baked in — the docker dispatch +# backend mounts a scoped, per-launch config in at runtime (see +# internal/orchestrator/dispatch_docker.go and images/agent/README.md). +# +# Build args pin the harness versions; the CI workflow that publishes this +# image passes the versions current at release time. +FROM node:22-bookworm-slim + +ARG OPENCODE_VERSION=1.18.31 +ARG PI_VERSION=0.85.1 + +RUN apt-get update && apt-get install -y --no-install-recommends \ + ca-certificates curl gnupg ripgrep \ + && mkdir -p -m 755 /etc/apt/keyrings \ + && curl -fsSL https://cli.github.com/packages/githubcli-archive-keyring.gpg \ + -o /etc/apt/keyrings/githubcli-archive-keyring.gpg \ + && chmod go+r /etc/apt/keyrings/githubcli-archive-keyring.gpg \ + && echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/githubcli-archive-keyring.gpg] https://cli.github.com/packages stable main" \ + > /etc/apt/sources.list.d/github-cli.list \ + && apt-get update && apt-get install -y --no-install-recommends gh \ + && rm -rf /var/lib/apt/lists/* + +# opencode: a single binary off its GitHub releases — no npm package. +RUN arch="$(dpkg --print-architecture)" \ + && case "$arch" in \ + amd64) oc_arch=x64 ;; \ + arm64) oc_arch=arm64 ;; \ + *) echo "opencode has no release for architecture $arch" >&2; exit 1 ;; \ + esac \ + && curl -fsSL -o /tmp/opencode.tar.gz \ + "https://github.com/anomalyco/opencode/releases/download/v${OPENCODE_VERSION}/opencode-linux-${oc_arch}.tar.gz" \ + && tar -xzf /tmp/opencode.tar.gz -C /usr/local/bin opencode \ + && chmod +x /usr/local/bin/opencode \ + && rm /tmp/opencode.tar.gz + +# Pi: npm's global bin directory is already on the node image's PATH. +RUN npm install -g "@earendil-works/pi-coding-agent@${PI_VERSION}" \ + && npm cache clean --force + +# A fixed, unprivileged user and home: the docker dispatch backend mounts a +# launch's scoped config directly at a path under it (opencode: +# $HOME/.config/opencode/opencode.json, Pi: $HOME/.pi/agent/models.json). +RUN useradd --create-home --home-dir /home/agent --shell /bin/sh agent +USER agent +ENV HOME=/home/agent +WORKDIR /workspace diff --git a/images/agent/README.md b/images/agent/README.md new file mode 100644 index 00000000..f7277b1a --- /dev/null +++ b/images/agent/README.md @@ -0,0 +1,50 @@ +# The spinloop agent image + +What `spinloop orchestrator --dispatch docker` runs an item's agent in. +Built from [`Dockerfile`](Dockerfile). + +## What it carries + +- [opencode](https://github.com/anomalyco/opencode) and + [Pi](https://github.com/earendil-works/pi), the two harnesses the + orchestrator can dispatch as a one-shot agent (lucinate has no + single-task form, so it is never dispatched under any backend, and is + not in this image) +- `ripgrep`, opencode's own runtime dependency +- [`gh`](https://cli.github.com/), the GitHub CLI — an item's instructions + routinely want it: opening a pull request, reading an issue, checking a + run's status +- A fixed, unprivileged user, `agent`, with `HOME=/home/agent` + +It carries none of any operator's own harness configuration: a container +started from it with nothing else mounted in has no provider configured +for either harness. The docker dispatch backend mounts an item's own +`workspace/` and `config/` subdirectories in at `/item/workspace` and +`/item/config`, redirecting each harness's own config resolution there +with its own environment variable (`XDG_CONFIG_HOME` for opencode, +`PI_CODING_AGENT_DIR` for Pi) rather than into this image's fixed +`/home/agent` — see +[`internal/orchestrator/dispatch_docker.go`](../../internal/orchestrator/dispatch_docker.go) +and the `agent-dispatch` spec (`openspec/specs/agent-dispatch/spec.md` +once this change is archived). + +## Tag + +Published as `ghcr.io/spinloop-ai/agent:`, one tag per spinloop +release — the same version `spinloop version` reports — so a given +spinloop binary's default `--dispatch-image` always names the image built +alongside it. `--dispatch-image` overrides the default, for testing an +unreleased image or pinning one. + +## Building it locally + +```sh +docker build -t spinloop-agent:local images/agent +docker run --rm spinloop-agent:local opencode --version +docker run --rm spinloop-agent:local pi --version +docker run --rm spinloop-agent:local gh --version +``` + +`OPENCODE_VERSION` and `PI_VERSION` build args pin the harness versions +the image installs; the CI workflow that publishes a release's image +passes the versions current at that time. diff --git a/internal/harness/adapters.go b/internal/harness/adapters.go index dfe3efaa..cc8c1176 100644 --- a/internal/harness/adapters.go +++ b/internal/harness/adapters.go @@ -2,6 +2,8 @@ package harness import ( "fmt" + "os" + "path/filepath" "github.com/spinloop-ai/spinloop/internal/catalog" "github.com/spinloop-ai/spinloop/internal/contextsize" @@ -31,10 +33,13 @@ func (opencodeHarness) Command() string { return "opencode" } func (opencodeHarness) ConfigPath() (string, error) { return opencode.ResolveConfigFile() } -func (opencodeHarness) Apply(p *catalog.Provider, sel spinloop.Selection, contextWindow, outputTokens int, setDefaultModel bool, resolve func(string) string) (Summary, error) { +// opencodeBlock builds the provider block and default model Apply and +// RenderProviderConfig both write, applying the display name, context +// limits and discovered models a selection carries. +func opencodeBlock(p *catalog.Provider, sel spinloop.Selection, contextWindow, outputTokens int, setDefaultModel bool, resolve func(string) string) (map[string]any, string, error) { block, defaultModel, err := catalog.BuildProviderBlock(sel.Provider, p, modelKey(sel), sel.BaseURL, resolve) if err != nil { - return Summary{}, err + return nil, "", err } if !setDefaultModel { defaultModel = "" @@ -67,6 +72,14 @@ func (opencodeHarness) Apply(p *catalog.Provider, sel spinloop.Selection, contex } block["models"] = models } + return block, defaultModel, nil +} + +func (opencodeHarness) Apply(p *catalog.Provider, sel spinloop.Selection, contextWindow, outputTokens int, setDefaultModel bool, resolve func(string) string) (Summary, error) { + block, defaultModel, err := opencodeBlock(p, sel, contextWindow, outputTokens, setDefaultModel, resolve) + if err != nil { + return Summary{}, err + } configFile, err := opencode.ResolveConfigFile() if err != nil { @@ -117,6 +130,27 @@ func (opencodeHarness) State() (map[string]ProviderState, string, error) { return out, defaultModel, nil } +// RenderProviderConfig builds the same block Apply would and writes it to a +// path inside a fresh temp directory rather than ConfigPath() — a path +// WriteConfig has never seen merges into nothing, so the bytes read back +// carry this provider alone. +func (opencodeHarness) RenderProviderConfig(p *catalog.Provider, sel spinloop.Selection, resolve func(string) string) ([]byte, error) { + block, defaultModel, err := opencodeBlock(p, sel, 0, 0, false, resolve) + if err != nil { + return nil, err + } + dir, err := os.MkdirTemp("", "spinloop-opencode-render-*") + if err != nil { + return nil, err + } + defer os.RemoveAll(dir) + path := filepath.Join(dir, "opencode.json") + if err := opencode.WriteConfig(path, sel.Provider, block, defaultModel); err != nil { + return nil, err + } + return os.ReadFile(path) +} + // piHarness configures the Pi coding agent via ~/.pi/agent/models.json. type piHarness struct{} @@ -126,10 +160,12 @@ func (piHarness) Command() string { return "pi" } func (piHarness) ConfigPath() (string, error) { return pi.ConfigPath() } -func (piHarness) Apply(p *catalog.Provider, sel spinloop.Selection, contextWindow, outputTokens int, setDefaultModel bool, resolve func(string) string) (Summary, error) { +// piProvider builds the provider Apply and RenderProviderConfig both write, +// adding the discovered models a selection with no model or alias carries. +func piProvider(p *catalog.Provider, sel spinloop.Selection, resolve func(string) string) (catalog.PiProvider, string, error) { prov, defaultModel, err := catalog.BuildPiProvider(sel.Provider, p, modelKey(sel), sel.BaseURL, resolve) if err != nil { - return Summary{}, err + return catalog.PiProvider{}, "", err } // DiscoveredModels is set only when the selection named no model or alias // to route by, in which case BuildPiProvider above added no model at all — @@ -145,6 +181,14 @@ func (piHarness) Apply(p *catalog.Provider, sel spinloop.Selection, contextWindo } } } + return prov, defaultModel, nil +} + +func (piHarness) Apply(p *catalog.Provider, sel spinloop.Selection, contextWindow, outputTokens int, setDefaultModel bool, resolve func(string) string) (Summary, error) { + prov, defaultModel, err := piProvider(p, sel, resolve) + if err != nil { + return Summary{}, err + } if err := pi.Write(sel.Provider, prov, contextWindow, outputTokens); err != nil { return Summary{}, err } @@ -187,6 +231,30 @@ func (piHarness) State() (map[string]ProviderState, string, error) { return out, "", nil } +// RenderProviderConfig builds the same provider Apply would and writes it +// to a path inside a fresh temp directory rather than ConfigPath() — a +// path Write has never seen merges into nothing, so the bytes read back +// carry this provider alone. +func (piHarness) RenderProviderConfig(p *catalog.Provider, sel spinloop.Selection, resolve func(string) string) ([]byte, error) { + prov, _, err := piProvider(p, sel, resolve) + if err != nil { + return nil, err + } + dir, err := os.MkdirTemp("", "spinloop-pi-render-*") + if err != nil { + return nil, err + } + defer os.RemoveAll(dir) + path := filepath.Join(dir, "models.json") + // contextWindow/outputTokens are 0 here the way a dispatch's Apply call + // already passes 0: a one-shot launch names its model, not a context + // override. + if err := pi.WriteTo(path, sel.Provider, prov, 0, 0); err != nil { + return nil, err + } + return os.ReadFile(path) +} + // lucinateHarness configures the lucinate chat client via // ~/.lucinate/connections.json, writing one managed OpenAI-compatible // connection per provider. diff --git a/internal/harness/harness.go b/internal/harness/harness.go index 0660e715..001939e3 100644 --- a/internal/harness/harness.go +++ b/internal/harness/harness.go @@ -74,6 +74,20 @@ type Harness interface { State() (providers map[string]ProviderState, defaultModel string, err error) } +// ConfigRenderer is a Harness whose provider config can be rendered fresh — +// carrying one provider and nothing merged in from any existing file. It is +// what a scoped, per-launch config (the docker dispatch backend's) is built +// from, so it is a separate interface from Harness rather than a method +// every harness must have: lucinate has no one-shot form (see +// dispatch.oneShot), so the orchestrator never dispatches it under any +// backend, and there is nothing for it to render. +type ConfigRenderer interface { + // RenderProviderConfig returns a fresh config file's bytes, carrying + // only this one provider selection — the same content Apply would + // write were ConfigPath() empty, without writing it there. + RenderProviderConfig(p *catalog.Provider, sel spinloop.Selection, resolve func(string) string) ([]byte, error) +} + // registry holds the available harnesses by name. var registry = map[string]Harness{ "opencode": opencodeHarness{}, diff --git a/internal/harness/harness_test.go b/internal/harness/harness_test.go index 16c66e14..4a711b65 100644 --- a/internal/harness/harness_test.go +++ b/internal/harness/harness_test.go @@ -1,9 +1,11 @@ package harness import ( + "encoding/json" "github.com/spinloop-ai/spinloop/internal/catalog" "os" "path/filepath" + "reflect" "strings" "testing" @@ -308,6 +310,64 @@ func TestOpencodeApplyStateRemove(t *testing.T) { } } +// TestOpencodeRenderProviderConfig checks that RenderProviderConfig's bytes +// carry the same provider block Apply would write, and that rendering +// touches nothing at ConfigPath() — the host's real config stays absent +// until Apply itself is the one that writes it. +func TestOpencodeRenderProviderConfig(t *testing.T) { + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + + cat, err := catalog.Load() + if err != nil { + t.Fatal(err) + } + h, _ := Lookup("opencode") + renderer, ok := h.(ConfigRenderer) + if !ok { + t.Fatal("opencode should implement ConfigRenderer") + } + resolve := func(k string) string { + if k == "DEEPSEEK_API_KEY" { + return "sk-or-v1-test" + } + return "" + } + sel := spinloop.Selection{Provider: "openrouter", Model: "deepseek/deepseek-v4-pro"} + + configFile, err := h.ConfigPath() + if err != nil { + t.Fatal(err) + } + rendered, err := renderer.RenderProviderConfig(cat.Providers["openrouter"], sel, resolve) + if err != nil { + t.Fatalf("RenderProviderConfig: %v", err) + } + if _, err := os.Stat(configFile); !os.IsNotExist(err) { + t.Errorf("rendering should not touch the host's config, got %v", err) + } + + if _, err := h.Apply(cat.Providers["openrouter"], sel, 0, 0, true, resolve); err != nil { + t.Fatalf("Apply: %v", err) + } + applied, err := os.ReadFile(configFile) + if err != nil { + t.Fatal(err) + } + + var renderedRoot, appliedRoot map[string]any + if err := json.Unmarshal(rendered, &renderedRoot); err != nil { + t.Fatalf("rendered bytes are not valid JSON: %v", err) + } + if err := json.Unmarshal(applied, &appliedRoot); err != nil { + t.Fatalf("applied bytes are not valid JSON: %v", err) + } + renderedProvider := renderedRoot["provider"].(map[string]any)["openrouter"] + appliedProvider := appliedRoot["provider"].(map[string]any)["openrouter"] + if !reflect.DeepEqual(renderedProvider, appliedProvider) { + t.Errorf("rendered provider block = %+v, want the same as Apply's %+v", renderedProvider, appliedProvider) + } +} + // TestOpencodeApplyWithContextSize verifies that context and output limits are // propagated through the adapter into the opencode config and readable back. func TestOpencodeApplyWithContextSize(t *testing.T) { @@ -476,6 +536,64 @@ func TestPiApplyStateRemove(t *testing.T) { } } +// TestPiRenderProviderConfig checks that RenderProviderConfig's bytes carry +// the same provider Apply would write, and that rendering touches nothing +// at ConfigPath(). +func TestPiRenderProviderConfig(t *testing.T) { + tmp := t.TempDir() + t.Setenv("HOME", tmp) + + cat, err := catalog.Load() + if err != nil { + t.Fatal(err) + } + h, _ := Lookup("pi") + renderer, ok := h.(ConfigRenderer) + if !ok { + t.Fatal("pi should implement ConfigRenderer") + } + resolve := func(k string) string { + if k == "DEEPSEEK_API_KEY" { + return "sk-or-v1-test" + } + return "" + } + sel := spinloop.Selection{Provider: "openrouter", Model: "deepseek/deepseek-v4-pro"} + + configFile, err := h.ConfigPath() + if err != nil { + t.Fatal(err) + } + rendered, err := renderer.RenderProviderConfig(cat.Providers["openrouter"], sel, resolve) + if err != nil { + t.Fatalf("RenderProviderConfig: %v", err) + } + if _, err := os.Stat(configFile); !os.IsNotExist(err) { + t.Errorf("rendering should not touch the host's config, got %v", err) + } + + if _, err := h.Apply(cat.Providers["openrouter"], sel, 0, 0, true, resolve); err != nil { + t.Fatalf("Apply: %v", err) + } + applied, err := os.ReadFile(configFile) + if err != nil { + t.Fatal(err) + } + + var renderedRoot, appliedRoot map[string]any + if err := json.Unmarshal(rendered, &renderedRoot); err != nil { + t.Fatalf("rendered bytes are not valid JSON: %v", err) + } + if err := json.Unmarshal(applied, &appliedRoot); err != nil { + t.Fatalf("applied bytes are not valid JSON: %v", err) + } + renderedProvider := renderedRoot["providers"].(map[string]any)["openrouter"] + appliedProvider := appliedRoot["providers"].(map[string]any)["openrouter"] + if !reflect.DeepEqual(renderedProvider, appliedProvider) { + t.Errorf("rendered provider = %+v, want the same as Apply's %+v", renderedProvider, appliedProvider) + } +} + // TestPiApplyWithDiscoveredModels merges a gateway-routed selection's // DiscoveredModels into the provider's models array, the way the opencode // adapter merges them into its models map. diff --git a/internal/orchestrator/api_test.go b/internal/orchestrator/api_test.go index 35dc8065..5f9aadfa 100644 --- a/internal/orchestrator/api_test.go +++ b/internal/orchestrator/api_test.go @@ -446,6 +446,78 @@ func TestAPI_ARemovalTakesTheItemItsRecordAndItsOutput(t *testing.T) { } } +// TestAPI_ARemovalTakesTheDockerBackendsScopedConfigToo checks that a +// removal takes the docker backend's per-launch config directory out with +// the item's kept output — the same cleanup, one more directory. +func TestAPI_ARemovalTakesTheDockerBackendsScopedConfigToo(t *testing.T) { + itemDir := filepath.Join(t.TempDir(), "a") + wl, _, _ := testWorkList(t, itemsFile(itemSpec{id: "a", instr: "do a", dir: itemDir}), nil) + configDir := ItemConfigDir(itemDir) + if err := os.MkdirAll(configDir, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(configDir, "opencode.json"), []byte("{}"), 0o600); err != nil { + t.Fatal(err) + } + srv := workListServer(t, wl, "") + defer srv.Close() + + code, raw := apiDo(t, srv, "", http.MethodDelete, "/v1/items/a", "") + if code != http.StatusOK { + t.Fatalf("a removal the work list accepts is answered, got %d: %s", code, raw) + } + if _, err := os.Stat(configDir); !os.IsNotExist(err) { + t.Errorf("the docker backend's scoped config directory should be gone with the item, got %v", err) + } +} + +// TestAPI_ARemovalResolvesTheConfigDirAgainstBaseDir checks that Remove's +// own cleanup resolves a relative item dir against WithBaseDir the same +// way a launch already does — not the raw dir the items file carries. +func TestAPI_ARemovalResolvesTheConfigDirAgainstBaseDir(t *testing.T) { + base := t.TempDir() + wl, _, _ := testWorkList(t, itemsFile(itemSpec{id: "a", instr: "do a", dir: "a"}), nil) + wl = wl.WithBaseDir(base) + configDir := ItemConfigDir(filepath.Join(base, "a")) + if err := os.MkdirAll(configDir, 0o700); err != nil { + t.Fatal(err) + } + srv := workListServer(t, wl, "") + defer srv.Close() + + code, raw := apiDo(t, srv, "", http.MethodDelete, "/v1/items/a", "") + if code != http.StatusOK { + t.Fatalf("a removal the work list accepts is answered, got %d: %s", code, raw) + } + if _, err := os.Stat(configDir); !os.IsNotExist(err) { + t.Errorf("the config directory under baseDir should be gone with the item, got %v", err) + } +} + +// TestAPI_ARemovalTakesTheItemsOwnLogCopyToo checks that a removal takes +// the item's own copy of its log out with the canonical one. +func TestAPI_ARemovalTakesTheItemsOwnLogCopyToo(t *testing.T) { + itemDir := filepath.Join(t.TempDir(), "a") + if err := os.MkdirAll(itemDir, 0o755); err != nil { + t.Fatal(err) + } + wl, _, _ := testWorkList(t, itemsFile(itemSpec{id: "a", instr: "do a", dir: itemDir}), nil) + logFile := ItemLogFile(itemDir) + if err := os.WriteFile(logFile, []byte("the agent's output"), 0o600); err != nil { + t.Fatal(err) + } + srv := workListServer(t, wl, "") + defer srv.Close() + + code, raw := apiDo(t, srv, "", http.MethodDelete, "/v1/items/a", "") + if code != http.StatusOK { + t.Fatalf("a removal the work list accepts is answered, got %d: %s", code, raw) + } + if _, err := os.Stat(logFile); !os.IsNotExist(err) { + t.Errorf("the item's own log copy should be gone with the item, got %v", err) + } +} + func TestAPI_ARemovalItCannotMakeIsRefused(t *testing.T) { wl, _, _ := testWorkList(t, itemsFile(itemSpec{id: "a", instr: "do a", dir: "./a"}), func(s *memStore) { diff --git a/internal/orchestrator/dispatch.go b/internal/orchestrator/dispatch.go index 2f15b8dd..468c0071 100644 --- a/internal/orchestrator/dispatch.go +++ b/internal/orchestrator/dispatch.go @@ -5,6 +5,7 @@ import ( "os" "os/exec" "path/filepath" + "sort" "strings" "sync" @@ -42,7 +43,13 @@ func HasOneShotForm(harnessName string) bool { // naming it. var oneShot = map[string]func(providerKey, model, instructions string) []string{ "opencode": func(key, model, instructions string) []string { - return []string{"run", "-m", key + "/" + model, instructions} + // --auto: with no terminal to answer one, a permission prompt + // opencode raises otherwise blocks forever, or — off a terminal — + // is auto-rejected, which silently stops the agent from doing the + // item's own work rather than failing loudly. There is no + // dispatch-time way to know which tools an item's instructions + // will need, so every one-shot launch trusts them all. + return []string{"run", "-m", key + "/" + model, "--auto", instructions} }, "pi": func(key, model, instructions string) []string { return []string{"--print", "--model", key + "/" + model, instructions} @@ -61,11 +68,21 @@ type Child interface { Kill() } -// Dispatcher launches admitted items as one-shot agents: it verifies the -// item's directory, applies the node's provider into the harness config, -// and runs the active harness in its single-task form in the item's -// directory, the agent's output kept per item beside the items file. -type Dispatcher struct { +// Launcher starts an admitted item's harness and returns the Child that +// tracks it: what the loop holds on to, reaps, and stops. Each backend +// chooses how and where that process actually runs — a bare host process, +// a container — behind the one method; everything above it (admission, +// matching, abort, state) works the same regardless. +type Launcher interface { + Launch(item Item, node Node, logPath string) (Child, error) +} + +// dispatchConfig is what every launcher backend needs to resolve a launch +// plan: the harness, the gateway a launch's inference points at, the +// token the harness's own bearer needs, whether a missing item directory +// is created rather than failing the item, and the base an item's own +// relative directory resolves against. +type dispatchConfig struct { h harness.Harness gateway string // the gateway's address token string // the gateway's token, resolved by the caller @@ -74,58 +91,70 @@ type Dispatcher struct { // failing the item. createItemDirs bool - mu sync.Mutex // serialises the applies into the shared harness config + // baseDir is harness.yaml's own baseDir: an item's relative dir + // resolves against it (see ResolveItemDir) rather than the + // orchestrator's own working directory. Empty means unchanged + // behavior — an item's dir is used exactly as the items file gives it. + baseDir string - // start begins a child process; a test seam standing in for exec. - start func(bin string, args []string, dir, logPath string, env []string) (Child, error) + // harnessConfig is harness.yaml's own: env added to every launch, and + // the startup/shutdown scripts wrapping it. Its env has already been + // checked against the token's own variable (CheckHarnessEnvCollision) + // before the run ever admits an item, so resolvePlan's caller does not + // check it again per launch. + harnessConfig HarnessConfig } -// NewDispatcher builds a dispatcher for the active harness, pointed at the -// gateway, holding the token its caller presents. Where createItemDirs is -// set, a missing item directory is created rather than failing the item. -func NewDispatcher(h harness.Harness, gateway, token string, createItemDirs bool) *Dispatcher { - d := &Dispatcher{h: h, gateway: gateway, token: token, createItemDirs: createItemDirs} - d.start = startChild - return d +// launchPlan is what every backend needs before it diverges into how the +// process actually runs: the item's resolved directory (see +// ResolveItemDir), the args a one-shot harness runs with, the catalogue +// provider its config carries, and the selection that provider is written +// under. +type launchPlan struct { + dir string + args []string + provider *catalog.Provider + sel spinloop.Selection } -// Launch runs the item against the node. A failure names the item and the -// cause — a missing directory, a harness without a single-task form, an -// apply the harness refused — and the caller records it failed rather than -// retrying it. -func (d *Dispatcher) Launch(item Item, node Node, logPath string) (Child, error) { - if fi, err := os.Stat(item.Dir); err != nil || !fi.IsDir() { - if !d.createItemDirs { - return nil, fmt.Errorf("item %q's working directory %s does not exist", item.ID, item.Dir) +// resolvePlan resolves the item's directory (see ResolveItemDir), checks +// it (creating it where cfg allows), ensures its workspace subdirectory +// exists, resolves the harness's one-shot form and the node's model, and +// builds the catalogue provider and selection a launch's config is +// written under. A failure names the item and the cause — a missing +// directory, a harness without a single-task form — and the caller +// records it failed rather than retrying it. +func (cfg dispatchConfig) resolvePlan(item Item, node Node) (launchPlan, error) { + dir := ResolveItemDir(cfg.baseDir, item.Dir) + if fi, err := os.Stat(dir); err != nil || !fi.IsDir() { + if !cfg.createItemDirs { + return launchPlan{}, fmt.Errorf("item %q's directory %s does not exist", item.ID, dir) } - if err := os.MkdirAll(item.Dir, 0o755); err != nil { - return nil, fmt.Errorf("item %q's working directory %s: creating it: %v", item.ID, item.Dir, err) + if err := os.MkdirAll(dir, 0o755); err != nil { + return launchPlan{}, fmt.Errorf("item %q's directory %s: creating it: %v", item.ID, dir, err) } } - form, ok := oneShot[d.h.Name()] + // The workspace subdirectory is the orchestrator's own, unlike dir + // itself: always created if missing, regardless of createItemDirs. + if err := os.MkdirAll(ItemWorkspaceDir(dir), 0o755); err != nil { + return launchPlan{}, fmt.Errorf("item %q's workspace directory: %v", item.ID, err) + } + form, ok := oneShot[cfg.h.Name()] if !ok { - return nil, fmt.Errorf("the %q harness has no single-task form the orchestrator can run", d.h.Name()) + return launchPlan{}, fmt.Errorf("the %q harness has no single-task form the orchestrator can run", cfg.h.Name()) } model := node.ModelName() if model == "" { - return nil, fmt.Errorf("node %s reports no model to run item %q against", node.Name, item.ID) + return launchPlan{}, fmt.Errorf("node %s reports no model to run item %q against", node.Name, item.ID) } cat, err := catalog.LoadFrom(catalog.ResolveCatalogPath("")) if err != nil { - return nil, err + return launchPlan{}, err } p := cat.Providers[plumbingProvider] if p == nil { - return nil, fmt.Errorf("the catalogue names no %q provider for the dispatch", plumbingProvider) - } - resolve := func(name string) string { - // The config carries the variable's name, never the token's value: - // the value reaches the agent through its environment. - if name == p.APIKeyEnv { - return d.token - } - return os.Getenv(name) + return launchPlan{}, fmt.Errorf("the catalogue names no %q provider for the dispatch", plumbingProvider) } sel := spinloop.Selection{ Provider: providerKey(node), @@ -133,25 +162,118 @@ func (d *Dispatcher) Launch(item Item, node Node, logPath string) (Child, error) // The agent's address for the gateway is the OpenAI-compatible // prefix the way a gateway-routed launch gives it: a gateway at // http://gw:4000 is handed to the agent as http://gw:4000/v1. - BaseURL: fleet.EndpointBaseURL(d.gateway), + BaseURL: fleet.EndpointBaseURL(cfg.gateway), DisplayName: "Spinloop fleet (" + node.Name + ")", } + return launchPlan{dir: dir, args: form(providerKey(node), model, item.Instructions), provider: p, sel: sel}, nil +} + +// sortedEnvPairs returns m as NAME=VALUE pairs in a stable, sorted order — +// deterministic for a test to assert on, unlike a bare map range. +func sortedEnvPairs(m map[string]string) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + out := make([]string, 0, len(keys)) + for _, k := range keys { + out = append(out, k+"="+m[k]) + } + return out +} + +// providerEnv is the token's own entry (where the provider takes one) and +// harness.yaml's env, as NAME=VALUE pairs — what a launch that starts from +// nothing (the docker backend's container) needs in full; the bare +// backend adds these to its inherited host environment instead. +func (cfg dispatchConfig) providerEnv(plan launchPlan) []string { + var out []string + if plan.provider.APIKeyEnv != "" { + out = append(out, plan.provider.APIKeyEnv+"="+cfg.token) + } + return append(out, sortedEnvPairs(cfg.harnessConfig.Env)...) +} + +// Dispatcher is the bare-process Launcher: it applies the node's provider +// into the harness's own host config, and runs the active harness in its +// single-task form in the item's workspace directory (see +// ItemWorkspaceDir), the agent's output kept per item beside the items +// file. +type Dispatcher struct { + dispatchConfig + + mu sync.Mutex // serialises the applies into the shared harness config + + // start begins a child process; a test seam standing in for exec. + start func(bin string, args []string, dir, logPath string, env []string) (Child, error) +} + +// NewDispatcher builds a dispatcher for the active harness, pointed at the +// gateway, holding the token its caller presents. Where createItemDirs is +// set, a missing item directory is created rather than failing the item. +func NewDispatcher(h harness.Harness, gateway, token string, createItemDirs bool) *Dispatcher { + d := &Dispatcher{dispatchConfig: dispatchConfig{h: h, gateway: gateway, token: token, createItemDirs: createItemDirs}} + d.start = startChild + return d +} + +// WithHarnessConfig sets harness.yaml's own config — its env, and the +// startup/shutdown scripts a launch runs under. Unset (the zero value) +// means no harness.yaml at all: every launch proceeds exactly as it did +// before this existed. +func (d *Dispatcher) WithHarnessConfig(hc HarnessConfig) *Dispatcher { + d.harnessConfig = hc + return d +} + +// WithBaseDir sets harness.yaml's own baseDir — see dispatchConfig's +// baseDir field. Unset (empty) means unchanged behavior. +func (d *Dispatcher) WithBaseDir(dir string) *Dispatcher { + d.baseDir = dir + return d +} + +// Launch runs the item against the node as a bare process. A failure names +// the item and the cause — a missing directory, a harness without a +// single-task form, an apply the harness refused — and the caller records +// it failed rather than retrying it. +func (d *Dispatcher) Launch(item Item, node Node, logPath string) (Child, error) { + plan, err := d.resolvePlan(item, node) + if err != nil { + return nil, err + } + resolve := func(name string) string { + // The config carries the variable's name, never the token's value: + // the value reaches the agent through its environment. + if name == plan.provider.APIKeyEnv { + return d.token + } + return os.Getenv(name) + } // Under the lock, with the exec inside it: the next dispatch's apply // must not rewrite this node's block before this agent has read it. d.mu.Lock() - _, err = d.h.Apply(p, sel, 0, 0, false, resolve) + _, err = d.h.Apply(plan.provider, plan.sel, 0, 0, false, resolve) d.mu.Unlock() if err != nil { return nil, fmt.Errorf("applying node %s's provider for item %q: %v", node.Name, item.ID, err) } var env []string - if p.APIKeyEnv != "" && os.Getenv(p.APIKeyEnv) == "" { - env = append(os.Environ(), p.APIKeyEnv+"="+d.token) + if plan.provider.APIKeyEnv != "" && os.Getenv(plan.provider.APIKeyEnv) == "" { + env = append(os.Environ(), plan.provider.APIKeyEnv+"="+d.token) } else { env = os.Environ() } - return d.start(d.h.Command(), form(providerKey(node), model, item.Instructions), item.Dir, logPath, env) + env = append(env, sortedEnvPairs(d.harnessConfig.Env)...) + + bin, args, extraEnv := wrapCommand(d.h.Command(), plan.args, d.harnessConfig) + child, err := d.start(bin, args, ItemWorkspaceDir(plan.dir), logPath, append(env, extraEnv...)) + if err != nil { + return nil, err + } + return withItemLogCopy(wrapChild(child, d.harnessConfig), logPath, ItemLogFile(plan.dir)), nil } // startChild begins the agent as this process's child: its own process diff --git a/internal/orchestrator/dispatch_docker.go b/internal/orchestrator/dispatch_docker.go new file mode 100644 index 00000000..7a5fb637 --- /dev/null +++ b/internal/orchestrator/dispatch_docker.go @@ -0,0 +1,303 @@ +package orchestrator + +import ( + "bytes" + "fmt" + "net/url" + "os" + "os/exec" + "path/filepath" + "strings" + + "github.com/spinloop-ai/spinloop/internal/harness" +) + +// dockerHarnessRedirect is how a dispatchable harness's own config +// resolution is pointed at dockerConfigRoot instead of the image's fixed +// $HOME: the env var the harness reads in place of its default config +// directory, and the path its rendered config lands at, relative to that +// directory. A harness with a one-shot form but no entry here cannot run +// under the docker backend; lucinate has neither an entry here nor a +// one-shot form, so it never reaches this table at all. +type dockerHarnessRedirect struct { + envVar string // overrides the harness's own default config directory + file string // the harness's config file, relative to envVar's value +} + +var dockerHarnessRedirects = map[string]dockerHarnessRedirect{ + // opencode reads $XDG_CONFIG_HOME/opencode/opencode.json ahead of + // $HOME/.config/opencode/opencode.json. + "opencode": {envVar: "XDG_CONFIG_HOME", file: "opencode/opencode.json"}, + // Pi reads $PI_CODING_AGENT_DIR/models.json ahead of + // $HOME/.pi/agent/models.json. + "pi": {envVar: "PI_CODING_AGENT_DIR", file: "models.json"}, +} + +// dockerItemRoot is where one launch's own directory tree lands inside the +// container: config/ (the rendered, per-launch provider config, from +// ItemConfigDir) and workspace/ (from ItemWorkspaceDir, the harness's +// working directory) sit side by side under it, mirroring the same two +// subdirectories of the item's own directory on the host, rather than +// scattering config under the image's fixed $HOME while workspace sits at +// the container's root. +const ( + dockerItemRoot = "/item" + dockerConfigRoot = dockerItemRoot + "/config" + dockerWorkspaceRoot = dockerItemRoot + "/workspace" +) + +// DefaultDockerImage builds the docker backend's default image reference +// for spinloop's own version, so a given binary defaults to the image +// built alongside it: ghcr.io/spinloop-ai/agent:. +func DefaultDockerImage(version string) string { + if version == "" || version == "dev" { + version = "latest" + } + return "ghcr.io/spinloop-ai/agent:" + version +} + +// CheckDockerReachable runs `docker version` once, the way HasOneShotForm +// checks the chosen harness once: an unreachable daemon is refused before +// any item is worked, naming the fix, rather than failing the first item +// and leaving the rest to fail the same way one at a time. +func CheckDockerReachable() error { + cmd := exec.Command("docker", "version") + var stderr bytes.Buffer + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + msg := strings.TrimSpace(stderr.String()) + if msg == "" { + msg = err.Error() + } + return fmt.Errorf("the docker backend needs a reachable docker daemon: %s", msg) + } + return nil +} + +// dockerReachableGateway is the address a container reaches the gateway +// at, given the address the orchestrator itself — a bare process — reaches +// it at. A loopback-bound gateway is the orchestrator host's own loopback, +// which is not the container's: `--network host` would put it in the same +// namespace on Linux, but Docker Desktop's Mac and Windows builds only +// honour that with a setting most operators do not have on, so it cannot +// be relied on. `host.docker.internal` is the one address that reaches +// back to the real host on every platform docker runs on — Docker Desktop +// resolves it out of the box, and `--add-host +// host.docker.internal:host-gateway` (see the docker run invocation below) +// makes it resolve on a native Linux docker host too, where it is not +// automatic. A gateway already on a routable, non-loopback address is +// reachable from the container's own network exactly as it is from the +// host, unchanged. +func dockerReachableGateway(gateway string) string { + u, err := url.Parse(gateway) + if err != nil || u.Hostname() == "" { + return gateway + } + switch u.Hostname() { + case "localhost", "127.0.0.1", "::1": + default: + return gateway + } + host := "host.docker.internal" + if port := u.Port(); port != "" { + host += ":" + port + } + u.Host = host + return u.String() +} + +// dockerLauncher is the docker Launcher: it renders a scoped, per-launch +// config carrying one provider (harness.ConfigRenderer), mounts it and the +// item's workspace directory into a container from the official agent +// image, and runs the harness — wrapped where harness.yaml names a +// lifecycle script, see wrapCommand — as the container's own command, on +// the default bridge network, reaching a loopback-bound gateway via +// host.docker.internal (dockerReachableGateway) rather than the host's own +// network namespace. +type dockerLauncher struct { + dispatchConfig + + image string // the agent image reference + + // run begins `docker run` for one launch as this process's own child, + // given the full argv and the log path its output is kept at; a test + // seam standing in for exec, mirroring Dispatcher.start. + run func(args []string, containerName, logPath string) (Child, error) +} + +// NewDockerLauncher builds a docker launcher for the active harness, +// pointed at the gateway and the agent image, holding the token its +// caller presents. Where createItemDirs is set, a missing item directory +// is created rather than failing the item, the same as the bare backend. +func NewDockerLauncher(h harness.Harness, gateway, token, image string, createItemDirs bool) *dockerLauncher { + l := &dockerLauncher{ + dispatchConfig: dispatchConfig{h: h, gateway: gateway, token: token, createItemDirs: createItemDirs}, + image: image, + } + l.run = runDockerContainer + return l +} + +// WithHarnessConfig sets harness.yaml's own config, the way Dispatcher's +// does — see its doc comment. +func (l *dockerLauncher) WithHarnessConfig(hc HarnessConfig) *dockerLauncher { + l.harnessConfig = hc + return l +} + +// WithBaseDir sets harness.yaml's own baseDir, the way Dispatcher's does — +// see its doc comment. +func (l *dockerLauncher) WithBaseDir(dir string) *dockerLauncher { + l.baseDir = dir + return l +} + +// Launch runs the item against the node inside a container. A failure +// names the item and the cause — a missing directory, a harness without a +// single-task form or a docker config mount, a docker failure — and the +// caller records it failed rather than retrying it. +func (l *dockerLauncher) Launch(item Item, node Node, logPath string) (Child, error) { + // resolvePlan is shared with the bare backend, which needs the + // orchestrator host's own gateway address unchanged — so the + // substitution happens on a local copy of the config, not on + // l.dispatchConfig itself. + cfg := l.dispatchConfig + cfg.gateway = dockerReachableGateway(l.gateway) + plan, err := cfg.resolvePlan(item, node) + if err != nil { + return nil, err + } + renderer, ok := l.h.(harness.ConfigRenderer) + if !ok { + return nil, fmt.Errorf("the %q harness cannot render a config for the docker backend", l.h.Name()) + } + redirect, ok := dockerHarnessRedirects[l.h.Name()] + if !ok { + return nil, fmt.Errorf("the %q harness has no docker config mount known to the docker backend", l.h.Name()) + } + resolve := func(name string) string { + if name == plan.provider.APIKeyEnv { + return l.token + } + return os.Getenv(name) + } + rendered, err := renderer.RenderProviderConfig(plan.provider, plan.sel, resolve) + if err != nil { + return nil, fmt.Errorf("rendering the docker config for item %q: %v", item.ID, err) + } + + configDir := ItemConfigDir(plan.dir) + if err := os.MkdirAll(configDir, 0o700); err != nil { + return nil, fmt.Errorf("item %q's docker config directory %s: %v", item.ID, configDir, err) + } + configFile := filepath.Join(configDir, filepath.Base(redirect.file)) + if err := os.WriteFile(configFile, rendered, 0o600); err != nil { + return nil, fmt.Errorf("item %q's docker config %s: %v", item.ID, configFile, err) + } + + // resolvePlan (above) has already created the workspace subdirectory. + workspace, err := filepath.Abs(ItemWorkspaceDir(plan.dir)) + if err != nil { + return nil, fmt.Errorf("resolving item %q's workspace directory: %v", item.ID, err) + } + absConfigFile, err := filepath.Abs(configFile) + if err != nil { + return nil, fmt.Errorf("resolving item %q's docker config %s: %v", item.ID, configFile, err) + } + + bin, runArgs, wrapEnv := wrapCommand(l.h.Command(), plan.args, l.harnessConfig) + name := dockerContainerName(item.ID) + args := []string{ + "run", "--rm", "--name", name, + "--add-host", "host.docker.internal:host-gateway", + "-v", workspace + ":" + dockerWorkspaceRoot, + "-w", dockerWorkspaceRoot, + // A file mount, not a directory: only the rendered config lands + // at the harness's config path. Mounting the whole directory + // would put whatever else the harness keeps there — opencode + // installs its plugin's node_modules into its config directory — + // on the host, growing without bound across launches. The rest + // of that directory stays ordinary container filesystem, gone + // with the container on exit. + "-v", absConfigFile + ":" + dockerConfigRoot + "/" + redirect.file, + // Redirects the harness's own config resolution to dockerConfigRoot + // rather than the image's fixed $HOME, so the file mount above lands + // where the harness actually looks. + "-e", redirect.envVar + "=" + dockerConfigRoot, + } + for _, e := range l.providerEnv(plan) { + args = append(args, "-e", e) + } + for _, e := range wrapEnv { + args = append(args, "-e", e) + } + args = append(args, l.image, bin) + args = append(args, runArgs...) + + child, err := l.run(args, name, logPath) + if err != nil { + return nil, fmt.Errorf("item %q: %v", item.ID, err) + } + return withItemLogCopy(wrapChild(child, l.harnessConfig), logPath, ItemLogFile(plan.dir)), nil +} + +// dockerContainerName is the container name a launch runs under: unique +// per item, and stable, so a second launch for the same id under the same +// run cannot collide with one docker has not yet finished tearing down. +func dockerContainerName(itemID string) string { + return "spinloop-" + itemID +} + +// runDockerContainer begins `docker run` as this process's own child, in +// the foreground rather than detached, so its own exit is the container's +// exit — the same contract startChild gives for a bare process — and its +// output, whatever the outcome, is written to the item's log the same way. +func runDockerContainer(args []string, containerName, logPath string) (Child, error) { + log, err := os.OpenFile(logPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o600) + if err != nil { + return nil, fmt.Errorf("opening the item's log: %v", err) + } + cmd := exec.Command("docker", args...) + cmd.Stdout = log + cmd.Stderr = log + if err := cmd.Start(); err != nil { + log.Close() + return nil, fmt.Errorf("starting the container: %v", err) + } + log.Close() // the child holds its own copy of the handle + return &dockerChild{cmd: cmd, name: containerName}, nil +} + +// dockerChild is the container a docker launch starts. Stop and Kill act +// on the container by name — a separate `docker stop`/`docker kill` +// invocation each — rather than a signal to the local `docker run` +// client: that client process simply exits, taking Wait with it, once the +// container it is attached to does. +type dockerChild struct { + cmd *exec.Cmd + name string +} + +// Wait blocks until the container ends, reporting its exit as the error — +// an *exec.ExitError carrying the container's own exit code, the same +// shape procChild's Wait already gives, so sentinelChild's exitCoder +// check works unchanged for either backend. +func (c *dockerChild) Wait() error { return c.cmd.Wait() } + +// Stop asks the container to end politely, with no grace of docker's own: +// the orchestrator's own stopGrace already bounds the wait between Stop +// and Kill, and a second, stacked grace inside `docker stop` would only +// make an abort take longer for no benefit. +func (c *dockerChild) Stop() { + dockerCLI("stop", "--time", "0", c.name) +} + +// Kill ends the container hard. +func (c *dockerChild) Kill() { + dockerCLI("kill", c.name) +} + +// dockerCLI runs `docker `, its outcome not reported — Stop and +// Kill are already best-effort, matching the bare backend's own silent +// Stop/Kill. A test seam standing in for exec. +var dockerCLI = func(args ...string) { exec.Command("docker", args...).Run() } diff --git a/internal/orchestrator/dispatch_docker_test.go b/internal/orchestrator/dispatch_docker_test.go new file mode 100644 index 00000000..f5b699f1 --- /dev/null +++ b/internal/orchestrator/dispatch_docker_test.go @@ -0,0 +1,341 @@ +package orchestrator + +import ( + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/spinloop-ai/spinloop/internal/harness" +) + +// dockerRunRecorder is the fake `docker run` seam dockerLauncher tests +// swap in: it records the argv each launch built, and answers with a +// fakeChild (or a launch failure) a test configures. +type dockerRunRecorder struct { + calls []dockerRunCall + err error // returned in place of a Child where set +} + +type dockerRunCall struct { + args []string + containerName string + logPath string +} + +func (r *dockerRunRecorder) run(args []string, containerName, logPath string) (Child, error) { + r.calls = append(r.calls, dockerRunCall{args: args, containerName: containerName, logPath: logPath}) + if r.err != nil { + return nil, r.err + } + return newFakeChild(nil, nil), nil +} + +// testDockerHarness is the real opencode adapter: RenderProviderConfig +// never touches ConfigPath() (see harness_test.go's own coverage of +// that), so no env sandboxing is needed for it here the way Apply's tests +// need XDG_CONFIG_HOME. +func testDockerHarness(t *testing.T) harness.Harness { + t.Helper() + h, ok := harness.Lookup("opencode") + if !ok { + t.Fatal("opencode should be registered") + } + return h +} + +func testDockerLauncher(t *testing.T, h harness.Harness, image string) (*dockerLauncher, *dockerRunRecorder, string) { + t.Helper() + work := t.TempDir() + l := NewDockerLauncher(h, "http://gateway:4000", "the-token", image, false) + rec := &dockerRunRecorder{} + l.run = rec.run + return l, rec, work +} + +func TestDockerLaunch_RendersAScopedConfigPerItem(t *testing.T) { + h := testDockerHarness(t) + l, _, work := testDockerLauncher(t, h, "spinloop/agent:test") + + dirA := filepath.Join(work, "a") + dirB := filepath.Join(work, "b") + if err := os.MkdirAll(dirA, 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(dirB, 0o755); err != nil { + t.Fatal(err) + } + itemA := Item{ID: "a", Instructions: "do a", Dir: dirA} + itemB := Item{ID: "b", Instructions: "do b", Dir: dirB} + node := runningNode("n", "org/model", nil) + + if _, err := l.Launch(itemA, node, filepath.Join(work, "a.log")); err != nil { + t.Fatalf("Launch a: %v", err) + } + if _, err := l.Launch(itemB, node, filepath.Join(work, "b.log")); err != nil { + t.Fatalf("Launch b: %v", err) + } + + dataA, err := os.ReadFile(filepath.Join(ItemConfigDir(dirA), "opencode.json")) + if err != nil { + t.Fatalf("item a's config should be on disk: %v", err) + } + if !strings.Contains(string(dataA), "spinloop-orchestrator-n") { + t.Errorf("the rendered config should carry the node's provider, got %s", dataA) + } + if _, err := os.Stat(filepath.Join(ItemConfigDir(dirB), "opencode.json")); err != nil { + t.Errorf("item b's config should be on disk: %v", err) + } +} + +func TestDockerLaunch_TheKeptLogIsAlsoCopiedToTheItemsOwnDirectory(t *testing.T) { + h := testDockerHarness(t) + l, _, work := testDockerLauncher(t, h, "spinloop/agent:test") + + itemDir := filepath.Join(work, "a") + if err := os.MkdirAll(itemDir, 0o755); err != nil { + t.Fatal(err) + } + logPath := filepath.Join(work, "a.log") + // runDockerContainer itself opens and writes logPath; the fake run + // seam does not, so this stands in for the container's own output. + if err := os.WriteFile(logPath, []byte("the container's output"), 0o600); err != nil { + t.Fatal(err) + } + + item := Item{ID: "a", Instructions: "do a", Dir: itemDir} + child, err := l.Launch(item, runningNode("n", "org/model", nil), logPath) + if err != nil { + t.Fatalf("Launch: %v", err) + } + if err := child.Wait(); err != nil { + t.Fatalf("Wait: %v", err) + } + + copied, err := os.ReadFile(ItemLogFile(itemDir)) + if err != nil { + t.Fatalf("the item's own directory should carry a copy of the log: %v", err) + } + if string(copied) != "the container's output" { + t.Errorf("the copy should match the canonical log, got %q", copied) + } +} + +func TestDockerLaunch_ABaseDirResolvesTheItemsRelativeDirectory(t *testing.T) { + h := testDockerHarness(t) + l, rec, work := testDockerLauncher(t, h, "spinloop/agent:test") + l.WithBaseDir(work) + if err := os.MkdirAll(filepath.Join(work, "parser"), 0o755); err != nil { + t.Fatal(err) + } + + item := Item{ID: "a", Instructions: "do a", Dir: "parser"} + if _, err := l.Launch(item, runningNode("n", "org/model", nil), filepath.Join(work, "a.log")); err != nil { + t.Fatalf("Launch: %v", err) + } + + itemDir := filepath.Join(work, "parser") + if _, err := os.Stat(filepath.Join(ItemWorkspaceDir(itemDir))); err != nil { + t.Errorf("the workspace should be under baseDir/parser, got: %v", err) + } + if _, err := os.Stat(filepath.Join(ItemConfigDir(itemDir), "opencode.json")); err != nil { + t.Errorf("the config should be under baseDir/parser, got: %v", err) + } + call := rec.calls[0] + if !strings.Contains(strings.Join(call.args, " "), mustAbs(t, itemDir)+"/workspace:/item/workspace") { + t.Errorf("the mount should use baseDir/parser, got:\n%s", strings.Join(call.args, " ")) + } +} + +func TestDockerLaunch_BuildsTheExpectedInvocation(t *testing.T) { + h := testDockerHarness(t) + l, rec, work := testDockerLauncher(t, h, "spinloop/agent:test") + l.harnessConfig = HarnessConfig{Env: map[string]string{"FOO": "bar"}} + + itemDir := filepath.Join(work, "a") + if err := os.MkdirAll(itemDir, 0o755); err != nil { + t.Fatal(err) + } + item := Item{ID: "a", Instructions: "fix it", Dir: itemDir} + node := runningNode("gpu-1", "org/model", nil) + logPath := filepath.Join(work, "a.log") + + if _, err := l.Launch(item, node, logPath); err != nil { + t.Fatalf("Launch: %v", err) + } + if len(rec.calls) != 1 { + t.Fatalf("one docker run per launch, got %d", len(rec.calls)) + } + call := rec.calls[0] + joined := strings.Join(call.args, " ") + + for _, want := range []string{ + "run", "--rm", + "--add-host host.docker.internal:host-gateway", + "-v " + mustAbs(t, filepath.Join(itemDir, "workspace")) + ":/item/workspace", + "-w /item/workspace", + "-v " + mustAbs(t, filepath.Join(itemDir, "config")) + "/opencode.json:/item/config/opencode/opencode.json", + "-e XDG_CONFIG_HOME=/item/config", + "-e OPENAI_API_KEY=the-token", + "-e FOO=bar", + "spinloop/agent:test", + "opencode run -m spinloop-orchestrator-gpu-1/org/model --auto fix it", + } { + if !strings.Contains(joined, want) { + t.Errorf("the invocation should carry %q, got:\n%s", want, joined) + } + } + if strings.Contains(joined, "--network") { + t.Errorf("the invocation should not set --network, got:\n%s", joined) + } + if call.containerName != "spinloop-a" { + t.Errorf("containerName = %q, want spinloop-a", call.containerName) + } + if call.logPath != logPath { + t.Errorf("logPath = %q, want %q", call.logPath, logPath) + } +} + +// TestDockerLaunch_ALoopbackGatewayReachesTheContainerViaDockerInternal is +// the bug a fleet.yaml naming a loopback gateway (http://localhost:4000, +// http://127.0.0.1:4000) hits under the docker backend: the container is +// not the host, and --network host does not reliably put it there on +// Docker Desktop. The rendered config's base URL SHALL name +// host.docker.internal instead, never asking the operator to change their +// fleet file. +func TestDockerLaunch_ALoopbackGatewayReachesTheContainerViaDockerInternal(t *testing.T) { + h := testDockerHarness(t) + work := t.TempDir() + l := NewDockerLauncher(h, "http://localhost:4000", "the-token", "spinloop/agent:test", false) + rec := &dockerRunRecorder{} + l.run = rec.run + + item := Item{ID: "a", Instructions: "do", Dir: work} + if _, err := l.Launch(item, runningNode("n", "org/model", nil), filepath.Join(work, "a.log")); err != nil { + t.Fatalf("Launch: %v", err) + } + + rendered, err := os.ReadFile(filepath.Join(ItemConfigDir(work), "opencode.json")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(rendered), "host.docker.internal:4000") { + t.Errorf("the rendered config should reach the gateway via host.docker.internal, got:\n%s", rendered) + } + if strings.Contains(string(rendered), "localhost:4000") || strings.Contains(string(rendered), "127.0.0.1:4000") { + t.Errorf("the rendered config should not carry the host's own loopback address, got:\n%s", rendered) + } +} + +// TestDockerLaunch_ARoutableGatewayIsUnchanged checks that a gateway +// already reachable from the container's own network — anything but the +// host's own loopback — is not rewritten. +func TestDockerLaunch_ARoutableGatewayIsUnchanged(t *testing.T) { + h := testDockerHarness(t) + l, _, work := testDockerLauncher(t, h, "spinloop/agent:test") // gateway: http://gateway:4000 + + item := Item{ID: "a", Instructions: "do", Dir: work} + if _, err := l.Launch(item, runningNode("n", "org/model", nil), filepath.Join(work, "a.log")); err != nil { + t.Fatalf("Launch: %v", err) + } + + rendered, err := os.ReadFile(filepath.Join(ItemConfigDir(item.Dir), "opencode.json")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(rendered), "gateway:4000") { + t.Errorf("a routable gateway should reach the config unchanged, got:\n%s", rendered) + } +} + +func TestDockerReachableGateway(t *testing.T) { + for _, tc := range []struct{ in, want string }{ + {"http://localhost:4000", "http://host.docker.internal:4000"}, + {"http://127.0.0.1:4000/v1", "http://host.docker.internal:4000/v1"}, + {"http://[::1]:4000", "http://host.docker.internal:4000"}, + {"http://gateway.internal:4000", "http://gateway.internal:4000"}, + {"http://gateway:4000", "http://gateway:4000"}, + } { + if got := dockerReachableGateway(tc.in); got != tc.want { + t.Errorf("dockerReachableGateway(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} + +func TestDefaultDockerImage(t *testing.T) { + if got, want := DefaultDockerImage("v1.42.0"), "ghcr.io/spinloop-ai/agent:v1.42.0"; got != want { + t.Errorf("DefaultDockerImage(v1.42.0) = %q, want %q", got, want) + } + // "dev" is what main.version defaults to off a release build; there is + // no matching image tag for it, so the default falls back to latest. + if got, want := DefaultDockerImage("dev"), "ghcr.io/spinloop-ai/agent:latest"; got != want { + t.Errorf("DefaultDockerImage(dev) = %q, want %q", got, want) + } + if got, want := DefaultDockerImage(""), "ghcr.io/spinloop-ai/agent:latest"; got != want { + t.Errorf("DefaultDockerImage(\"\") = %q, want %q", got, want) + } +} + +func TestDockerLaunch_AFailureFailsTheItemNamingIt(t *testing.T) { + h := testDockerHarness(t) + l, rec, work := testDockerLauncher(t, h, "spinloop/agent:test") + rec.err = fmt.Errorf("starting the container: exec: \"docker\": executable file not found in $PATH") + + item := Item{ID: "a", Instructions: "do", Dir: work} + _, err := l.Launch(item, runningNode("n", "org/m", nil), filepath.Join(work, "a.log")) + if err == nil { + t.Fatal("a docker failure should fail the launch") + } + if !strings.Contains(err.Error(), `item "a"`) || !strings.Contains(err.Error(), "docker") { + t.Errorf("the failure should name the item and the docker command's own message, got %v", err) + } +} + +func TestDockerChild_WaitReturnsTheContainersExitCode(t *testing.T) { + cmd := exec.Command("sh", "-c", "exit 3") + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + c := &dockerChild{cmd: cmd, name: "irrelevant"} + err := c.Wait() + if err == nil { + t.Fatal("a nonzero exit should be reported as an error") + } + var ec exitCoder + if !errors.As(err, &ec) || ec.ExitCode() != 3 { + t.Errorf("Wait's error should carry the exit code the way procChild's does, got %v", err) + } +} + +func TestDockerChild_StopAndKill(t *testing.T) { + var calls [][]string + orig := dockerCLI + dockerCLI = func(args ...string) { calls = append(calls, append([]string{}, args...)) } + defer func() { dockerCLI = orig }() + + c := &dockerChild{name: "spinloop-a"} + c.Stop() + c.Kill() + + if len(calls) != 2 { + t.Fatalf("Stop and Kill should each run one docker command, got %d: %v", len(calls), calls) + } + if got := strings.Join(calls[0], " "); got != "stop --time 0 spinloop-a" { + t.Errorf("Stop's argv = %q", got) + } + if got := strings.Join(calls[1], " "); got != "kill spinloop-a" { + t.Errorf("Kill's argv = %q", got) + } +} + +func mustAbs(t *testing.T, p string) string { + t.Helper() + abs, err := filepath.Abs(p) + if err != nil { + t.Fatal(err) + } + return abs +} diff --git a/internal/orchestrator/harnessconfig.go b/internal/orchestrator/harnessconfig.go new file mode 100644 index 00000000..e8ea92a4 --- /dev/null +++ b/internal/orchestrator/harnessconfig.go @@ -0,0 +1,95 @@ +package orchestrator + +import ( + "fmt" + "os" + "path/filepath" + + "gopkg.in/yaml.v3" + + "github.com/spinloop-ai/spinloop/internal/catalog" +) + +// HarnessConfig is what an operator's harness.yaml carries: environment +// variables added to every launch under either dispatch backend, and shell +// scripts bracketing the harness — see the agent-dispatch spec's +// "Configuring the harness with harness.yaml" and "The startup and +// shutdown scripts" requirements. +type HarnessConfig struct { + Env map[string]string `yaml:"env"` + // Dispatch names the backend the run uses — "bare" or "docker" — the + // way --dispatch does. An explicit --dispatch wins over it; where the + // flag is not given, this is the run's choice. + Dispatch string `yaml:"dispatch"` + // Harness names the harness the run uses — the way --harness/-H does. + // An explicit --harness wins over it; where the flag is not given, + // this is tried before the HARNESS environment variable and the + // stored preference (see harness.Resolve). + Harness string `yaml:"harness"` + // BaseDir is the directory an item's own relative dir (the items + // file's `dir`) resolves against, in place of the orchestrator's own + // working directory when the command starts. An item's dir that is + // already absolute is unaffected. Where BaseDir itself is relative, + // it resolves against harness.yaml's own directory — LoadHarnessConfig + // resolves it once, at load time, so every other use of it is already + // absolute. + BaseDir string `yaml:"baseDir"` + Startup string `yaml:"startup"` + Shutdown string `yaml:"shutdown"` +} + +// hasLifecycle reports whether hc names a startup or shutdown script — the +// only condition under which a launch runs the lifecycle wrapper instead +// of the harness directly. +func (hc HarnessConfig) hasLifecycle() bool { + return hc.Startup != "" || hc.Shutdown != "" +} + +// LoadHarnessConfig reads flagPath where it is given, else harness.yaml +// beside itemsPath; where neither is present, it returns a zero +// HarnessConfig — every launch proceeds exactly as it would with no +// harness.yaml at all. A present file that does not parse is an error. +func LoadHarnessConfig(flagPath, itemsPath string) (HarnessConfig, error) { + path := flagPath + if path == "" { + path = filepath.Join(filepath.Dir(itemsPath), "harness.yaml") + } + data, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return HarnessConfig{}, nil + } + return HarnessConfig{}, fmt.Errorf("reading %s: %v", path, err) + } + var cfg HarnessConfig + if err := yaml.Unmarshal(data, &cfg); err != nil { + return HarnessConfig{}, fmt.Errorf("parsing %s: %v", path, err) + } + if cfg.BaseDir != "" && !filepath.IsAbs(cfg.BaseDir) { + cfg.BaseDir = filepath.Join(filepath.Dir(path), cfg.BaseDir) + } + return cfg, nil +} + +// CheckHarnessEnvCollision refuses a harness.yaml env entry that names the +// same variable the work list's provider token is presented under. Every +// launch under every backend resolves the same catalogue provider (see +// plumbingProvider), so this is checked once, before the command works an +// item, rather than per launch. +func CheckHarnessEnvCollision(env map[string]string) error { + cat, err := catalog.LoadFrom(catalog.ResolveCatalogPath("")) + if err != nil { + return err + } + p := cat.Providers[plumbingProvider] + if p == nil { + return fmt.Errorf("the catalogue names no %q provider for the dispatch", plumbingProvider) + } + if p.APIKeyEnv == "" { + return nil + } + if _, collide := env[p.APIKeyEnv]; collide { + return fmt.Errorf("harness.yaml's env names %q, the variable the resolved token is presented under", p.APIKeyEnv) + } + return nil +} diff --git a/internal/orchestrator/harnessconfig_test.go b/internal/orchestrator/harnessconfig_test.go new file mode 100644 index 00000000..9b91f37c --- /dev/null +++ b/internal/orchestrator/harnessconfig_test.go @@ -0,0 +1,130 @@ +package orchestrator + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestLoadHarnessConfig_DefaultBesideItemsFile(t *testing.T) { + dir := t.TempDir() + itemsPath := filepath.Join(dir, "work.yaml") + if err := os.WriteFile(filepath.Join(dir, "harness.yaml"), + []byte("env:\n FOO: bar\nstartup: echo hi\n"), 0o600); err != nil { + t.Fatal(err) + } + hc, err := LoadHarnessConfig("", itemsPath) + if err != nil { + t.Fatalf("LoadHarnessConfig: %v", err) + } + if hc.Env["FOO"] != "bar" || hc.Startup != "echo hi" { + t.Errorf("the default harness.yaml beside the items file should be read, got %+v", hc) + } +} + +func TestLoadHarnessConfig_ExplicitFlagOverridesDefault(t *testing.T) { + dir := t.TempDir() + itemsPath := filepath.Join(dir, "work.yaml") + if err := os.WriteFile(filepath.Join(dir, "harness.yaml"), + []byte("env:\n FOO: default\n"), 0o600); err != nil { + t.Fatal(err) + } + named := filepath.Join(dir, "other.yaml") + if err := os.WriteFile(named, []byte("env:\n FOO: named\n"), 0o600); err != nil { + t.Fatal(err) + } + hc, err := LoadHarnessConfig(named, itemsPath) + if err != nil { + t.Fatalf("LoadHarnessConfig: %v", err) + } + if hc.Env["FOO"] != "named" { + t.Errorf("the flag's file should be read in preference to the default, got %+v", hc) + } +} + +func TestLoadHarnessConfig_NeitherPresentChangesNothing(t *testing.T) { + dir := t.TempDir() + itemsPath := filepath.Join(dir, "work.yaml") + hc, err := LoadHarnessConfig("", itemsPath) + if err != nil { + t.Fatalf("no harness.yaml at all should not be an error: %v", err) + } + if hc.hasLifecycle() || len(hc.Env) != 0 { + t.Errorf("a zero HarnessConfig was expected, got %+v", hc) + } +} + +func TestLoadHarnessConfig_UnparsableFileFails(t *testing.T) { + dir := t.TempDir() + itemsPath := filepath.Join(dir, "work.yaml") + if err := os.WriteFile(filepath.Join(dir, "harness.yaml"), []byte("not: [valid: yaml"), 0o600); err != nil { + t.Fatal(err) + } + _, err := LoadHarnessConfig("", itemsPath) + if err == nil || !strings.Contains(err.Error(), "harness.yaml") { + t.Errorf("an unparsable harness.yaml should fail naming it, got %v", err) + } +} + +func TestLoadHarnessConfig_ARelativeBaseDirResolvesAgainstHarnessYamlsOwnDirectory(t *testing.T) { + dir := t.TempDir() + itemsPath := filepath.Join(dir, "sub", "work.yaml") + if err := os.MkdirAll(filepath.Dir(itemsPath), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(filepath.Dir(itemsPath), "harness.yaml"), + []byte("baseDir: ../items\n"), 0o600); err != nil { + t.Fatal(err) + } + hc, err := LoadHarnessConfig("", itemsPath) + if err != nil { + t.Fatalf("LoadHarnessConfig: %v", err) + } + want := filepath.Join(dir, "items") + if hc.BaseDir != want { + t.Errorf("baseDir = %q, want %q (resolved against harness.yaml's own directory)", hc.BaseDir, want) + } +} + +func TestLoadHarnessConfig_AnAbsoluteBaseDirIsUnchanged(t *testing.T) { + dir := t.TempDir() + itemsPath := filepath.Join(dir, "work.yaml") + abs := filepath.Join(dir, "elsewhere") + if err := os.WriteFile(filepath.Join(dir, "harness.yaml"), + []byte("baseDir: "+abs+"\n"), 0o600); err != nil { + t.Fatal(err) + } + hc, err := LoadHarnessConfig("", itemsPath) + if err != nil { + t.Fatalf("LoadHarnessConfig: %v", err) + } + if hc.BaseDir != abs { + t.Errorf("an absolute baseDir should be unchanged, got %q, want %q", hc.BaseDir, abs) + } +} + +func TestResolveItemDir(t *testing.T) { + for _, tc := range []struct { + name, baseDir, dir, want string + }{ + {"no baseDir", "", "./parser", "./parser"}, + {"relative dir joins baseDir", "/base", "parser", "/base/parser"}, + {"absolute dir is unaffected", "/base", "/elsewhere/parser", "/elsewhere/parser"}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := ResolveItemDir(tc.baseDir, tc.dir); got != tc.want { + t.Errorf("ResolveItemDir(%q, %q) = %q, want %q", tc.baseDir, tc.dir, got, tc.want) + } + }) + } +} + +func TestCheckHarnessEnvCollision(t *testing.T) { + if err := CheckHarnessEnvCollision(map[string]string{"OPENAI_API_KEY": "x"}); err == nil { + t.Error("an entry naming the token's own variable should be refused") + } + if err := CheckHarnessEnvCollision(map[string]string{"SOME_OTHER_VAR": "x"}); err != nil { + t.Errorf("an unrelated entry should not be refused: %v", err) + } +} diff --git a/internal/orchestrator/integration_test.go b/internal/orchestrator/integration_test.go index a1a8cb7a..70d8b631 100644 --- a/internal/orchestrator/integration_test.go +++ b/internal/orchestrator/integration_test.go @@ -251,7 +251,7 @@ func TestIntegration_ItemsTakeTheNodeTheFleetShapes(t *testing.T) { t.Fatalf("the clean interrupt should end the run without an error, got %v", err) } - args, err := os.ReadFile(filepath.Join(workdir, "args.txt")) + args, err := os.ReadFile(filepath.Join(ItemWorkspaceDir(workdir), "args.txt")) if err != nil { t.Fatal(err) } @@ -285,7 +285,7 @@ func TestIntegration_ItemsTakeTheNodeTheFleetShapes(t *testing.T) { t.Fatalf("the clean interrupt should end the run without an error, got %v", err) } - args, err := os.ReadFile(filepath.Join(workdir, "args.txt")) + args, err := os.ReadFile(filepath.Join(ItemWorkspaceDir(workdir), "args.txt")) if err != nil { t.Fatal(err) } @@ -320,7 +320,7 @@ func TestIntegration_ItemsTakeTheNodeTheFleetShapes(t *testing.T) { t.Fatalf("the clean interrupt should end the run without an error, got %v", err) } - args, err := os.ReadFile(filepath.Join(workdir, "args.txt")) + args, err := os.ReadFile(filepath.Join(ItemWorkspaceDir(workdir), "args.txt")) if err != nil { t.Fatal(err) } @@ -348,7 +348,7 @@ func TestIntegration_ItemsTakeTheNodeTheFleetShapes(t *testing.T) { if st := readState(t, itemsPath).Items["a"]; st.State != "" { t.Fatalf("with the fleet's wake off the item should wait, got %s", st.State) } - if _, err := os.ReadFile(filepath.Join(workdir, "args.txt")); !os.IsNotExist(err) { + if _, err := os.ReadFile(filepath.Join(ItemWorkspaceDir(workdir), "args.txt")); !os.IsNotExist(err) { t.Errorf("with the fleet's wake off the item should not be launched") } cancel() @@ -387,7 +387,7 @@ func TestIntegration_ItemsTakeTheNodeTheFleetShapes(t *testing.T) { if err := <-done; err != nil { t.Fatalf("the clean interrupt should end the run without an error, got %v", err) } - args, err := os.ReadFile(filepath.Join(workdir, "args.txt")) + args, err := os.ReadFile(filepath.Join(ItemWorkspaceDir(workdir), "args.txt")) if err != nil { t.Fatal(err) } @@ -488,7 +488,7 @@ func TestIntegration_TheLifecycleEndsTheSpecSays(t *testing.T) { if err := <-done; err != nil { t.Fatalf("the clean interrupt should end the run without an error, got %v", err) } - if _, err := os.ReadFile(filepath.Join(workdir, "args.txt")); err != nil { + if _, err := os.ReadFile(filepath.Join(ItemWorkspaceDir(workdir), "args.txt")); err != nil { t.Errorf("the restarted run should have launched the item, got %v", err) } }) @@ -549,8 +549,8 @@ func TestIntegration_TheLifecycleEndsTheSpecSays(t *testing.T) { if fi, err := os.Stat(missing); err != nil || !fi.IsDir() { t.Errorf("the item's directory should have been created: %v", err) } - if _, err := os.ReadFile(filepath.Join(missing, "args.txt")); err != nil { - t.Errorf("the agent should have worked in the created directory, got %v", err) + if _, err := os.ReadFile(filepath.Join(ItemWorkspaceDir(missing), "args.txt")); err != nil { + t.Errorf("the agent should have worked in the created directory's workspace, got %v", err) } }) } diff --git a/internal/orchestrator/items.go b/internal/orchestrator/items.go index af0293ee..166b7d7d 100644 --- a/internal/orchestrator/items.go +++ b/internal/orchestrator/items.go @@ -1,6 +1,6 @@ // The work items file: the backlog the orchestrator works. A file is a list -// of items in file order; each names the work its agent is given, the -// directory the agent works in, and — where the item is bound to a kind of +// of items in file order; each names the work its agent is given, its own +// directory (see Item.Dir), and — where the item is bound to a kind of // node — the tags of the nodes it can run on. package orchestrator @@ -20,7 +20,11 @@ type Item struct { ID string // Instructions is what the item's agent is told to do. Instructions string - // Dir is the directory the agent works in. + // Dir is the item's own directory: the storage location for everything + // the orchestrator keeps about that item's launch — the harness's own + // working directory under a `workspace` subdirectory (ItemWorkspaceDir), + // and, under the docker backend, its scoped config under a `config` + // subdirectory (ItemConfigDir). Dir string // Tags are the nodes the item can run on, named the way node tags are // named — a key and a value joined by =. An item that names none diff --git a/internal/orchestrator/orchestrator_test.go b/internal/orchestrator/orchestrator_test.go index 9cdc1b77..e31736d3 100644 --- a/internal/orchestrator/orchestrator_test.go +++ b/internal/orchestrator/orchestrator_test.go @@ -676,7 +676,7 @@ func readRecord(t *testing.T, path string) string { return string(data) } -func TestDispatch_RunsTheAgentOneShotInTheItemsDirectory(t *testing.T) { +func TestDispatch_RunsTheAgentOneShotInTheItemsWorkspaceDirectory(t *testing.T) { work := t.TempDir() record := filepath.Join(work, "record") t.Setenv("RECORD_FILE", record) @@ -697,10 +697,11 @@ func TestDispatch_RunsTheAgentOneShotInTheItemsDirectory(t *testing.T) { t.Fatalf("the agent should end cleanly: %v", err) } - // The kernel resolves the item's directory for the child, so the - // reported working directory is the resolved one. - canonical := work - if resolved, err := filepath.EvalSymlinks(work); err == nil { + // The kernel resolves the item's workspace directory for the child, so + // the reported working directory is the resolved one. + workspace := ItemWorkspaceDir(work) + canonical := workspace + if resolved, err := filepath.EvalSymlinks(workspace); err == nil { canonical = resolved } rec := readRecord(t, record) @@ -709,6 +710,7 @@ func TestDispatch_RunsTheAgentOneShotInTheItemsDirectory(t *testing.T) { "arg:run", "arg:-m", "arg:spinloop-orchestrator-gpu-a/org/model", + "arg:--auto", "arg:fix the parser", "key:the-token", } { @@ -741,7 +743,109 @@ func TestDispatch_RunsTheAgentOneShotInTheItemsDirectory(t *testing.T) { } } -func TestDispatch_TheChildsPWDVariableCarriesTheItemsDirectory(t *testing.T) { +func TestDispatch_TheKeptLogIsAlsoCopiedToTheItemsOwnDirectory(t *testing.T) { + work := t.TempDir() + t.Setenv("OPENAI_API_KEY", "") + bin := stubAgent(t) + + h := &fakeHarness{name: "opencode", bin: bin} + d := NewDispatcher(h, "http://gateway:4000", "the-token", false) + logPath := filepath.Join(work, "a.log") + child, err := d.Launch( + Item{ID: "a", Instructions: "fix the parser", Dir: work}, + runningNode("gpu-a", "org/model", nil), + logPath, + ) + if err != nil { + t.Fatalf("the launch should succeed: %v", err) + } + if err := child.Wait(); err != nil { + t.Fatalf("the agent should end cleanly: %v", err) + } + + canonical, err := os.ReadFile(logPath) + if err != nil { + t.Fatal(err) + } + copied, err := os.ReadFile(ItemLogFile(work)) + if err != nil { + t.Fatalf("the item's own directory should carry a copy of the log: %v", err) + } + if string(copied) != string(canonical) { + t.Errorf("the item directory's copy should match the canonical log, got %q, want %q", copied, canonical) + } +} + +func TestDispatch_ABaseDirResolvesTheItemsRelativeDirectory(t *testing.T) { + base := t.TempDir() + if err := os.MkdirAll(filepath.Join(base, "parser"), 0o755); err != nil { + t.Fatal(err) + } + work := t.TempDir() + record := filepath.Join(work, "record") + t.Setenv("RECORD_FILE", record) + t.Setenv("OPENAI_API_KEY", "") + bin := stubAgent(t) + + h := &fakeHarness{name: "opencode", bin: bin} + d := NewDispatcher(h, "http://gateway:4000", "the-token", false).WithBaseDir(base) + child, err := d.Launch( + Item{ID: "a", Instructions: "fix the parser", Dir: "parser"}, + runningNode("gpu-a", "org/model", nil), + filepath.Join(work, "a.log"), + ) + if err != nil { + t.Fatalf("the launch should succeed: %v", err) + } + if err := child.Wait(); err != nil { + t.Fatalf("the agent should end cleanly: %v", err) + } + + workspace := ItemWorkspaceDir(filepath.Join(base, "parser")) + canonical := workspace + if resolved, err := filepath.EvalSymlinks(workspace); err == nil { + canonical = resolved + } + rec := readRecord(t, record) + if !strings.Contains(rec, "cwd:"+canonical) { + t.Errorf("a relative item dir should resolve against baseDir, record:\n%s", rec) + } +} + +func TestDispatch_ABaseDirLeavesAnAbsoluteItemDirUnaffected(t *testing.T) { + base := t.TempDir() + work := t.TempDir() + record := filepath.Join(work, "record") + t.Setenv("RECORD_FILE", record) + t.Setenv("OPENAI_API_KEY", "") + bin := stubAgent(t) + + h := &fakeHarness{name: "opencode", bin: bin} + d := NewDispatcher(h, "http://gateway:4000", "the-token", false).WithBaseDir(base) + child, err := d.Launch( + Item{ID: "a", Instructions: "fix the parser", Dir: work}, + runningNode("gpu-a", "org/model", nil), + filepath.Join(work, "a.log"), + ) + if err != nil { + t.Fatalf("the launch should succeed: %v", err) + } + if err := child.Wait(); err != nil { + t.Fatalf("the agent should end cleanly: %v", err) + } + + workspace := ItemWorkspaceDir(work) + canonical := workspace + if resolved, err := filepath.EvalSymlinks(workspace); err == nil { + canonical = resolved + } + rec := readRecord(t, record) + if !strings.Contains(rec, "cwd:"+canonical) { + t.Errorf("an absolute item dir should ignore baseDir, record:\n%s", rec) + } +} + +func TestDispatch_TheChildsPWDVariableCarriesTheItemsWorkspaceDirectory(t *testing.T) { work := t.TempDir() itemDir := filepath.Join(work, "item") if err := os.MkdirAll(itemDir, 0o755); err != nil { @@ -773,8 +877,8 @@ func TestDispatch_TheChildsPWDVariableCarriesTheItemsDirectory(t *testing.T) { // The variable carries the directory resolved the way filepath.Abs // gives it — the symlinked form, not the kernel's resolved cwd. rec := readRecord(t, record) - if !strings.Contains(rec, "pwd:"+itemDir) { - t.Errorf("the child's PWD variable should carry the item's directory, record:\n%s", rec) + if !strings.Contains(rec, "pwd:"+ItemWorkspaceDir(itemDir)) { + t.Errorf("the child's PWD variable should carry the item's workspace directory, record:\n%s", rec) } } @@ -822,12 +926,13 @@ func TestDispatch_AMissingDirectoryIsCreatedWhereTheCommandSaysTo(t *testing.T) t.Fatalf("the directory should have been created: %v", err) } rec := readRecord(t, record) - canonical := missing - if resolved, err := filepath.EvalSymlinks(missing); err == nil { + workspace := ItemWorkspaceDir(missing) + canonical := workspace + if resolved, err := filepath.EvalSymlinks(workspace); err == nil { canonical = resolved } if !strings.Contains(rec, "cwd:"+canonical) { - t.Errorf("the agent should have worked in the created directory, record:\n%s", rec) + t.Errorf("the agent should have worked in the created directory's workspace, record:\n%s", rec) } } diff --git a/internal/orchestrator/run.go b/internal/orchestrator/run.go index 93fa4d3a..0021d381 100644 --- a/internal/orchestrator/run.go +++ b/internal/orchestrator/run.go @@ -12,12 +12,12 @@ var stopGrace = 5 * time.Second // Config is what the loop runs on: the gateway it names in its failures, // the items file it works, the gateway it reads the fleet from, and the -// dispatcher that launches admitted items. +// launcher that starts an admitted item's harness. type Config struct { Gateway string ItemsPath string Topologist Topologist - Dispatcher *Dispatcher + Dispatcher Launcher // Tick is how often the loop re-reads the topology and the items file; // zero takes the default. Tick time.Duration diff --git a/internal/orchestrator/state.go b/internal/orchestrator/state.go index 454ef848..87391aa0 100644 --- a/internal/orchestrator/state.go +++ b/internal/orchestrator/state.go @@ -63,6 +63,75 @@ func logDirFor(itemsPath string) string { return itemsPath + ".logs" } func abortsDirFor(itemsPath string) string { return itemsPath + ".aborts" } +// ResolveItemDir is the directory the orchestrator actually operates on +// for an item: dir unchanged where it is already absolute, or where +// baseDir is empty; otherwise dir resolved against baseDir. baseDir comes +// from harness.yaml's own baseDir, letting an operator keep the items +// file's dir fields short and portable, independent of the directory the +// orchestrator command happens to start from. +func ResolveItemDir(baseDir, dir string) string { + if baseDir == "" || filepath.IsAbs(dir) { + return dir + } + return filepath.Join(baseDir, dir) +} + +// ItemWorkspaceDir is the harness's own working directory for an item: a +// `workspace` subdirectory of the item's own directory (the `dir` the items +// file names), bind-mounted into the container under the docker backend, +// and the bare backend's own process working directory. +func ItemWorkspaceDir(itemDir string) string { + return filepath.Join(itemDir, "workspace") +} + +// ItemConfigDir is where the docker backend renders one item's scoped, +// per-launch harness config: a `config` subdirectory of the item's own +// directory, alongside `workspace` — the item's own directory is the +// storage location for everything the orchestrator keeps about that one +// item's launch. +func ItemConfigDir(itemDir string) string { + return filepath.Join(itemDir, "config") +} + +// ItemLogFile is a second copy of an item's kept output, written under +// the item's own directory alongside workspace/ and config/. LogPathFor +// is the canonical copy, kept beside the items file — `work logs` and the +// work list API both read from that one, live, while the agent runs; +// this one is written once the agent has ended, for an item's own +// directory to carry everything about its launch on its own. +func ItemLogFile(itemDir string) string { + return filepath.Join(itemDir, "log") +} + +// logCopyChild wraps a Child so that once it ends, whatever the outcome, +// its kept log is copied into the item's own directory too. The copy runs +// after Wait returns, not live alongside the agent's own output: the +// canonical log (logPath) is what streams while the agent runs, and by +// the time Wait returns the backend that started the child has already +// finished writing to it, so a plain read-then-write is enough. +type logCopyChild struct { + Child + logPath string + itemLogPath string +} + +func (c logCopyChild) Wait() error { + err := c.Child.Wait() + // Best-effort: a failure here — the item's own directory gone, a + // permissions problem — does not change the item's own recorded + // outcome, the way a failing shutdown script already does not either. + if data, rerr := os.ReadFile(c.logPath); rerr == nil { + _ = os.WriteFile(c.itemLogPath, data, 0o600) + } + return err +} + +// withItemLogCopy wraps child so its kept log is copied into the item's +// own directory once it ends — see logCopyChild. +func withItemLogCopy(child Child, logPath, itemLogPath string) Child { + return logCopyChild{Child: child, logPath: logPath, itemLogPath: itemLogPath} +} + // LogPathFor is where one item's kept output stands beside the items file. func LogPathFor(itemsPath, id string) string { return filepath.Join(logDirFor(itemsPath), id+".log") diff --git a/internal/orchestrator/worklist.go b/internal/orchestrator/worklist.go index b8281a15..175153e7 100644 --- a/internal/orchestrator/worklist.go +++ b/internal/orchestrator/worklist.go @@ -32,10 +32,15 @@ type WorkList struct { itemsPath string store Store - dispatch *Dispatcher + dispatch Launcher gateway string log *slog.Logger + // baseDir is harness.yaml's own baseDir — see dispatchConfig's field of + // the same name. Remove's own cleanup needs it to resolve an item's + // directory the same way a launch already does. + baseDir string + items []Item records map[string]ItemState inflight map[string]*flight @@ -61,7 +66,7 @@ type WorkList struct { // NewWorkList reads the work a run works from: the items file's items, the // record the store keeps, and the file's last-modified facts the re-read // checks against. The store's lock is the one it took opening. -func NewWorkList(itemsPath string, store Store, dispatch *Dispatcher, gateway string, log *slog.Logger) (*WorkList, error) { +func NewWorkList(itemsPath string, store Store, dispatch Launcher, gateway string, log *slog.Logger) (*WorkList, error) { if log == nil { log = slog.New(slog.DiscardHandler) } @@ -91,6 +96,13 @@ func NewWorkList(itemsPath string, store Store, dispatch *Dispatcher, gateway st return wl, nil } +// WithBaseDir sets harness.yaml's own baseDir — see dispatchConfig's +// baseDir field. Unset (empty) means unchanged behavior. +func (w *WorkList) WithBaseDir(dir string) *WorkList { + w.baseDir = dir + return w +} + // Close releases the store's hold on the items file. func (w *WorkList) Close() { w.store.Close() } @@ -469,10 +481,12 @@ func (w *WorkList) Remove(id string) error { return &errConflict{msg: fmt.Sprintf("item %q is running: abort it first, then remove it", id)} } kept := make([]Item, 0, len(w.items)) + var removed Item found := false for _, it := range w.items { if it.ID == id { found = true + removed = it continue } kept = append(kept, it) @@ -493,6 +507,17 @@ func (w *WorkList) Remove(id string) error { w.log.Info("item removed", slog.String("item", id)) // The kept output goes with it; an item that wrote none has no file. os.Remove(w.store.LogPath(id)) + dir := ResolveItemDir(w.baseDir, removed.Dir) + // The docker backend's scoped config goes with it too — a config + // subdirectory of the item's own directory, left behind by the item's + // workspace subdirectory, which is not the orchestrator's to remove. A + // bare-backend item, or one the docker backend never launched, has no + // config directory — RemoveAll of one that is not there is a silent + // no-op. + os.RemoveAll(ItemConfigDir(dir)) + // The item's own copy of its log goes with it too, the same as the + // canonical one beside the items file; an item never launched has none. + os.Remove(ItemLogFile(dir)) return nil } diff --git a/internal/orchestrator/wrapper.go b/internal/orchestrator/wrapper.go new file mode 100644 index 00000000..c8760ddf --- /dev/null +++ b/internal/orchestrator/wrapper.go @@ -0,0 +1,92 @@ +package orchestrator + +import ( + "errors" + "fmt" + "strconv" +) + +// wrapperSentinelExit is the lifecycle wrapper's own signal that the +// startup script failed before the harness ever ran — read only where the +// wrapper actually ran (HarnessConfig.hasLifecycle), so it is never +// confused with a harness's own exit code on a run with no harness.yaml, +// or one naming neither script. +const wrapperSentinelExit = 97 + +// wrapperScript is the POSIX shell run in place of the harness whenever +// HarnessConfig names a startup or shutdown script: it runs STARTUP first, +// the harness itself (given as its own arguments) only where that +// succeeds, then SHUTDOWN once the harness has ended — whatever ended it. +// term forwards an incoming stop signal to the harness child rather than +// letting the wrapper's own default disposition take it down before +// shutdown gets to run; see design.md's "The wrapper" decision. Both +// backends run this same script — the bare backend as a local process, the +// docker backend as the container's command — assuming a POSIX shell is on +// the PATH either way. +var wrapperScript = `set -u +term() { [ -n "${child:-}" ] && kill -TERM "$child" 2>/dev/null; } +trap term TERM INT + +sh -c "$STARTUP"; startup_rc=$? +if [ "$startup_rc" -ne 0 ]; then + sh -c "$SHUTDOWN" + exit ` + strconv.Itoa(wrapperSentinelExit) + ` +fi + +"$@" & child=$! +wait "$child"; harness_rc=$? +# A trap firing while wait was blocked can make it return early with +# 128+signal rather than the child's real status (a POSIX/bash quirk, not +# the child's own outcome); wait again for the same pid, now already +# reaped, to read the real one. +if [ "$harness_rc" -ge 128 ]; then + wait "$child" 2>/dev/null; harness_rc=$? +fi + +sh -c "$SHUTDOWN" +exit "$harness_rc" +` + +// wrapCommand returns the bin, args and extra environment a launch should +// actually run: the harness directly where hc names neither script, or the +// lifecycle wrapper around it, carrying STARTUP/SHUTDOWN in its +// environment, otherwise. +func wrapCommand(bin string, args []string, hc HarnessConfig) (runBin string, runArgs []string, extraEnv []string) { + if !hc.hasLifecycle() { + return bin, args, nil + } + runArgs = append([]string{"-c", wrapperScript, "spinloop-wrapper", bin}, args...) + return "/bin/sh", runArgs, []string{"STARTUP=" + hc.Startup, "SHUTDOWN=" + hc.Shutdown} +} + +// exitCoder is what both *exec.ExitError and dockerExitError implement: +// the numeric exit code an ended process left, however the backend that +// ran it reports one. +type exitCoder interface{ ExitCode() int } + +// sentinelChild wraps a Child running under the lifecycle wrapper: its +// Wait turns the wrapper's own sentinel exit into a startup failure, +// naming the script, rather than reporting it as if it were the harness's +// own outcome. Every other outcome — the harness's own exit, a clean +// finish — passes through unchanged. +type sentinelChild struct{ Child } + +func (c sentinelChild) Wait() error { + err := c.Child.Wait() + var ec exitCoder + if errors.As(err, &ec) && ec.ExitCode() == wrapperSentinelExit { + return fmt.Errorf("the startup script failed before the harness ran") + } + return err +} + +// wrapChild wraps child in a sentinelChild where hc names a startup or +// shutdown script, so its exit is read the wrapper's way; where hc names +// neither, child is returned unchanged — a run with no harness.yaml, or +// one naming no script, works exactly as it always has. +func wrapChild(child Child, hc HarnessConfig) Child { + if !hc.hasLifecycle() { + return child + } + return sentinelChild{child} +} diff --git a/internal/orchestrator/wrapper_test.go b/internal/orchestrator/wrapper_test.go new file mode 100644 index 00000000..1840af24 --- /dev/null +++ b/internal/orchestrator/wrapper_test.go @@ -0,0 +1,176 @@ +package orchestrator + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +func TestWrapCommand_NoLifecycleRunsTheHarnessDirectly(t *testing.T) { + bin, args, env := wrapCommand("opencode", []string{"run", "-m", "x"}, HarnessConfig{}) + if bin != "opencode" || len(args) != 3 || env != nil { + t.Errorf("no startup or shutdown named should run the harness directly, got bin=%q args=%v env=%v", bin, args, env) + } +} + +func TestWrapCommand_ALifecycleScriptWrapsTheHarness(t *testing.T) { + bin, args, env := wrapCommand("opencode", []string{"run", "-m", "x"}, HarnessConfig{Startup: "echo hi"}) + if bin != "/bin/sh" { + t.Errorf("a lifecycle script should run the wrapper, got bin=%q", bin) + } + found := false + for _, e := range env { + if e == "STARTUP=echo hi" { + found = true + } + } + if !found { + t.Errorf("STARTUP should carry the script, got env=%v", env) + } + // The harness bin and its args trail the wrapper's own -c arguments, + // unchanged, so "$@" inside the script sees them. + want := []string{"-c", wrapperScript, "spinloop-wrapper", "opencode", "run", "-m", "x"} + if len(args) != len(want) { + t.Fatalf("args = %v, want %v", args, want) + } + for i, w := range want { + if args[i] != w { + t.Errorf("args[%d] = %q, want %q", i, args[i], w) + } + } +} + +func TestDispatch_WrapperRunsStartupThenHarnessThenShutdown(t *testing.T) { + work := t.TempDir() + record := filepath.Join(work, "record") + t.Setenv("RECORD_FILE", record) + t.Setenv("OPENAI_API_KEY", "") + bin := stubAgent(t) + + h := &fakeHarness{name: "opencode", bin: bin} + d := NewDispatcher(h, "http://gateway:4000", "the-token", false). + WithHarnessConfig(HarnessConfig{Startup: "echo from-startup", Shutdown: "echo from-shutdown"}) + + logPath := filepath.Join(work, "a.log") + child, err := d.Launch( + Item{ID: "a", Instructions: "fix the parser", Dir: work}, + runningNode("gpu-a", "org/model", nil), + logPath, + ) + if err != nil { + t.Fatalf("the launch should succeed: %v", err) + } + if err := child.Wait(); err != nil { + t.Fatalf("the agent should end cleanly: %v", err) + } + + log, err := os.ReadFile(logPath) + if err != nil { + t.Fatal(err) + } + out := string(log) + iStart := strings.Index(out, "from-startup") + iAgent := strings.Index(out, "the agent's output") + iShutdown := strings.Index(out, "from-shutdown") + if iStart == -1 || iAgent == -1 || iShutdown == -1 { + t.Fatalf("all three outputs should be in the log, got:\n%s", out) + } + if !(iStart < iAgent && iAgent < iShutdown) { + t.Errorf("the outputs should be in order startup, harness, shutdown, got:\n%s", out) + } +} + +func TestDispatch_AFailingStartupScriptFailsBeforeTheHarnessRuns(t *testing.T) { + work := t.TempDir() + record := filepath.Join(work, "record") + t.Setenv("RECORD_FILE", record) + t.Setenv("OPENAI_API_KEY", "") + bin := stubAgent(t) + + h := &fakeHarness{name: "opencode", bin: bin} + d := NewDispatcher(h, "http://gateway:4000", "the-token", false). + WithHarnessConfig(HarnessConfig{Startup: "exit 3", Shutdown: "echo from-shutdown"}) + + logPath := filepath.Join(work, "a.log") + child, err := d.Launch( + Item{ID: "a", Instructions: "fix the parser", Dir: work}, + runningNode("gpu-a", "org/model", nil), + logPath, + ) + if err != nil { + t.Fatalf("the launch itself should succeed — the failure is the wrapper's own exit: %v", err) + } + if err := child.Wait(); err == nil || !strings.Contains(err.Error(), "startup script") { + t.Errorf("a failing startup script should fail naming it, got %v", err) + } + if _, err := os.Stat(record); !os.IsNotExist(err) { + t.Error("the harness should never have run") + } + log, err := os.ReadFile(logPath) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(log), "from-shutdown") { + t.Errorf("shutdown should still run even though startup failed, got:\n%s", log) + } +} + +func TestDispatch_AbortRunsShutdownAfterStoppingTheHarness(t *testing.T) { + work := t.TempDir() + t.Setenv("OPENAI_API_KEY", "") + // A harness stand-in that ignores the default TERM disposition, traps + // it, records that it arrived, and then exits — so the wrapper's + // forwarded signal, not a default kill, is what ends it. + bin := filepath.Join(work, "agent") + script := "#!/bin/sh\n" + + "trap 'echo got-term; exit 0' TERM\n" + + "echo started\n" + + "while true; do sleep 0.05; done\n" + if err := os.WriteFile(bin, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + + h := &fakeHarness{name: "opencode", bin: bin} + d := NewDispatcher(h, "http://gateway:4000", "the-token", false). + WithHarnessConfig(HarnessConfig{Shutdown: "echo from-shutdown"}) + + logPath := filepath.Join(work, "a.log") + child, err := d.Launch( + Item{ID: "a", Instructions: "do", Dir: work}, + runningNode("gpu-a", "org/model", nil), + logPath, + ) + if err != nil { + t.Fatalf("the launch should succeed: %v", err) + } + + deadline := time.Now().Add(5 * time.Second) + for { + if data, _ := os.ReadFile(logPath); strings.Contains(string(data), "started") { + break + } + if time.Now().After(deadline) { + t.Fatal("the harness never started") + } + time.Sleep(5 * time.Millisecond) + } + + child.Stop() + if err := child.Wait(); err != nil { + t.Fatalf("a clean stop should not be reported as a failure: %v", err) + } + + log, err := os.ReadFile(logPath) + if err != nil { + t.Fatal(err) + } + out := string(log) + if !strings.Contains(out, "got-term") { + t.Errorf("the stop signal should reach the harness, got:\n%s", out) + } + if !strings.Contains(out, "from-shutdown") { + t.Errorf("shutdown should still run after the stop, got:\n%s", out) + } +} diff --git a/internal/pi/pi.go b/internal/pi/pi.go index 8c19b8c6..80f8e62b 100644 --- a/internal/pi/pi.go +++ b/internal/pi/pi.go @@ -84,16 +84,25 @@ func providersMap(root map[string]any) map[string]any { return pv } -// Write deep-merges a provider entry into models.json. An existing provider's -// unknown fields (headers, compat, modelOverrides, …) are preserved; baseUrl, -// api and apiKey are overwritten when set, and models are merged by id with the -// new entries winning. contextWindow and outputTokens, when > 0, are applied to -// every model the selection writes (as contextWindow and maxTokens). +// Write deep-merges a provider entry into models.json at ConfigPath(). An +// existing provider's unknown fields (headers, compat, modelOverrides, …) +// are preserved; baseUrl, api and apiKey are overwritten when set, and +// models are merged by id with the new entries winning. contextWindow and +// outputTokens, when > 0, are applied to every model the selection writes +// (as contextWindow and maxTokens). func Write(id string, prov catalog.PiProvider, contextWindow, outputTokens int) error { path, err := ConfigPath() if err != nil { return err } + return WriteTo(path, id, prov, contextWindow, outputTokens) +} + +// WriteTo is Write against an explicit path rather than ConfigPath() — a +// path that does not yet exist merges into nothing, the way models.json +// itself does the first time Write runs, so a caller renders a fresh, +// scoped file by pointing this at an empty temp path. +func WriteTo(path, id string, prov catalog.PiProvider, contextWindow, outputTokens int) error { root, err := load(path) if err != nil { return err diff --git a/openspec/changes/archive/2026-09-18-docker-agent-dispatch/.openspec.yaml b/openspec/changes/archive/2026-09-18-docker-agent-dispatch/.openspec.yaml new file mode 100644 index 00000000..d28e909f --- /dev/null +++ b/openspec/changes/archive/2026-09-18-docker-agent-dispatch/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-17 diff --git a/openspec/changes/archive/2026-09-18-docker-agent-dispatch/design.md b/openspec/changes/archive/2026-09-18-docker-agent-dispatch/design.md new file mode 100644 index 00000000..f44705dc --- /dev/null +++ b/openspec/changes/archive/2026-09-18-docker-agent-dispatch/design.md @@ -0,0 +1,382 @@ +## Context + +`Dispatcher.Launch` (`internal/orchestrator/dispatch.go`) today does five +things in one method: checks the item's directory, resolves the harness's +one-shot form and the node's model, resolves the catalogue's +`openai-compatible` provider, calls `harness.Harness.Apply(...)` — which +deep-merges a provider block into the harness's own host config file, +`ConfigPath()` resolved from `$HOME`/`XDG_CONFIG_HOME` — and finally starts +the process (`d.start`, normally `startChild`, a thin `exec.Command` +wrapper). The result satisfies `Child` (`Wait`/`Stop`/`Kill`), which +`pass()` and `Abort()` hold onto without caring how it runs. + +`pass()`'s failure handling already treats a `Launch` error as one item's +problem: it records the item failed, naming the error, and goes on to the +rest of the backlog (`internal/orchestrator/worklist.go`). Nothing above +`Launch` needs to change for a new backend to slot in, provided a backend's +own failures surface the same way — as an `error` from `Launch`. + +`harness.Harness.Apply` is built to merge into an existing file: it loads +whatever is there, adds or updates one provider's block, and writes the +whole thing back, preserving everything else the operator's own host +config carries. That is the wrong tool for a docker launch's config: the +scoped, per-launch directory starts empty, and mounting the host's real +config into a container to solve it would work against the isolation the +whole change is for. + +## Goals / Non-Goals + +**Goals:** +- A `Launcher` interface behind which `Dispatcher.Launch`'s five steps + split into a shared plan (directory, form, provider, selection) and a + backend-specific execution (host process vs. container) +- The bare backend's observable behavior is unchanged — same process + group, same host config file, same env +- The docker backend: host networking, the two bind mounts, a scoped + config carrying one provider, the official image +- `docker` unreachable is caught once, at startup, when `--dispatch + docker` is given — not per item +- `harness.yaml`'s `env` reaches a launch under either backend the same + way; its `startup`/`shutdown` scripts bracket the harness under either + backend, and shutdown's "always runs, even aborted" guarantee holds + without `Abort`, `pass`, or the `Child` interface changing shape + +**Non-Goals:** see proposal.md; restated only where it shapes a decision +below. + +## Decisions + +### The interface sits at `Launch`, not at `exec.Command` + +`d.start` is already a seam (`func(bin string, args []string, dir, +logPath string, env []string) (Child, error)`), but it is too narrow: the +docker backend does not run a bin/args/dir/env tuple the host way — it +needs to build mounts and choose an image, and it does not call +`harness.Apply` against the host at all. The seam moves up to the whole +`Launch(item Item, node Node, logPath string) (Child, error)` call. + +```go +type Launcher interface { + Launch(item Item, node Node, logPath string) (Child, error) +} +``` + +`Dispatcher` itself becomes the bare `Launcher` — it already resolves the +shared plan and starts the process, so it needs no separate wrapper type — +and a new `dockerLauncher` (`dispatch_docker.go`) is the second, chosen +once at construction from `--dispatch`. `WorkList` keeps holding a +`Launcher` where it holds a `*Dispatcher` today; the field's type is the +only call-site change outside `internal/orchestrator/dispatch*.go`. + +### The shared plan + +Both backends need: the item's directory (checked/created the way +`Launch` already does), the harness's one-shot form and args +(`oneShot[d.h.Name()]`), and the resolved `spinloop.Selection` (provider +key, model, the gateway's base URL, display name). This becomes one +function, `resolvePlan`, called by both launchers before they diverge — +extracted from today's `Launch` body verbatim, no behavior change. + +### Rendering a provider block without touching the host + +`harness.Harness` gains one method, implemented by opencode and Pi (the +two harnesses with a one-shot form; lucinate has neither): + +```go +// RenderProviderConfig returns a fresh config file's bytes, carrying only +// this one provider — no merge with anything already on disk. It is what +// Apply's merge starts from when the file does not yet exist, factored out +// so a caller can have the content without writing it to ConfigPath(). +RenderProviderConfig(p *catalog.Provider, sel spinloop.Selection, resolve func(string) string) ([]byte, error) +``` + +`Apply` is refactored to call it for the "file does not exist yet" case it +already has, rather than duplicating the provider-block construction. The +docker launcher calls it directly and writes the bytes to a file under the +item's own `config/` subdirectory (see "Where an item's workspace and +config live" below) — never the host's real `ConfigPath()`. + +### Where an item's workspace and config live + +An item's own directory (`dir` in the items file) is the storage location +for everything the orchestrator keeps about that item's launch, not the +harness's working directory directly: + +``` +/workspace/ the harness's own working directory, both backends +/config/ docker only: this item's scoped provider config +``` + +`resolvePlan` (shared by both backends) creates `workspace/` on every +launch if it is not already there — unconditionally, unlike `` +itself, which is only created where `--create-item-dirs` allows it. The +bare backend runs the harness with `workspace/` as its working directory, +exactly where `` itself was the working directory before this +decision; the docker backend bind-mounts it. + +Inside the container, both live under one root, `/item`: +`/item/workspace` (bind-mounted from `/workspace`, and the +container's `-w`) and `/item/config` (the rendered provider config's +parent). A harness resolves its own config from a fixed, `$HOME`-relative +path, not an arbitrary one — opencode from `$XDG_CONFIG_HOME/opencode/ +opencode.json` (falling back to `$HOME/.config/opencode/opencode.json`), +Pi from `$PI_CODING_AGENT_DIR/models.json` (falling back to `$HOME/ +.pi/agent/models.json`) — so the docker launcher sets that variable to +`/item/config` for the harness in use, rather than mounting into the +image's fixed `$HOME` directly. A small table (`dockerHarnessRedirects`, +parallel to, and no bigger than, the existing `oneShot` map) carries each +harness's variable name and its config file's path relative to it; a +harness added to `oneShot` later needs one line here, the same as it needs +one line there. + +The config mount is a file, not a directory: only the rendered config +lands inside the container, never whatever else the harness keeps +alongside it on a real host — opencode installs its plugin's +`node_modules` into its config directory on first run, which a directory +mount would otherwise leave growing, unbounded, on the host across +launches. A file mount confines that growth to the container's own +filesystem, gone with the container on exit. + +### Mounts and network + +- `/workspace` → `/item/workspace`, and the container's `-w` +- `/config/` → `/item/config/`, a file mount (see + above), with the harness's own redirect variable set to `/item/config` +- The default bridge network, not `--network host` — see "A loopback + gateway reaches the container by host.docker.internal, not host + networking" below for why +- The token reaches the container as an environment variable + (`-e =`), the same variable `Apply`'s `resolve` would have + read from the host's own environment for a bare launch + +### A loopback gateway reaches the container by host.docker.internal, not host networking + +The original design used `--network host` on the theory that it puts the +container on the same network the orchestrator itself is, so a +loopback-bound gateway just works. That holds on native Linux docker, but +not reliably anywhere else: Docker Desktop's Mac and Windows builds run +containers inside a VM, and `--network host` only reaches the real host's +loopback when an operator has turned on a specific, off-by-default Docker +Desktop setting. Without it, `localhost`/`127.0.0.1` inside the container +is the VM's own loopback — not the machine running spinloop, and not +reachable at all. A `fleet.yaml` naming `http://localhost:4000` — the +common case for a gateway run on the same machine — hits exactly this. + +The fix does not touch the fleet file: `dockerReachableGateway` rewrites +the address the *container's own rendered config* points its inference +at, substituting `host.docker.internal` for a `localhost`/`127.0.0.1`/`::1` +host, leaving a routable address (anything else) unchanged. This runs on +a copy of `dispatchConfig` local to the docker launcher's `Launch` — the +bare backend, sharing the same `resolvePlan`, keeps the real host address +it actually needs. `host.docker.internal` resolves on Docker Desktop out +of the box; `--add-host host.docker.internal:host-gateway` on the `docker +run` invocation makes it resolve on native Linux docker too, where it is +not automatic — a no-op where Docker Desktop already provides it. + +Dropping `--network host` also undoes the one deliberate trade-off the +original design named (see Risks/Trade-offs below): the container is back +on its own network namespace, so the isolation this whole change is for +is no longer given up for the network dimension specifically. A service +on the host's loopback *other than the gateway* is no longer transparently +reachable as plain `localhost` from inside the container — an item that +needs one reaches it via `host.docker.internal` itself, the same way the +gateway now does; that is a real, narrower trade than before, not a +regression from `--network host`'s own unreliability. + +### Running and stopping the container + +Shells out to the `docker` CLI (`docker run`, `docker stop`, `docker +kill`), the way `startChild` shells out to `exec.Command` — consistent +with the codebase's existing preference for thin process wrappers over +vendoring a client SDK (the only real external dependency elsewhere is +`aws-sdk-go-v2`, scoped to `internal/remote`). `docker run` itself runs in +the foreground, not detached, as this process's own child — the same +contract `startChild` gives for a bare launch — so `dockerChild.Wait` is +that local `docker` client process's own `Wait`, its exit mirroring the +container's, with no separate `docker wait` call; `Stop` is `docker stop +--time 0` (an immediate `SIGTERM`, no separate Docker-side grace — the +orchestrator's own `stopGrace` already bounds the wait between `Stop` and +`Kill` in `worklist.go`, and a second, stacked grace inside `docker stop` +would just make an abort take longer for no benefit); `Kill` is `docker +kill`. + +### `--dispatch docker` checks `docker` once, at startup + +The way `HasOneShotForm` fails the command before any item is worked +where the chosen harness cannot run one-shot, `--dispatch docker` runs +`docker version` once when the command starts (not per item): an +unreachable daemon is refused there, naming the fix, rather than failing +the first item and leaving the rest to fail the same way one at a time. + +### Image tag follows the spinloop version + +The docker launcher's default image reference is +`ghcr.io/spinloop-ai/agent:` (the same version +`main.version` already carries), so a given spinloop binary defaults to +the image built alongside it. An `--dispatch-image` flag overrides it — +useful for testing an unreleased image, or pinning one — deferred to +tasks.md as a small addition once the default path works. + +### `harness.yaml`: one loader, read once at startup + +A small struct (`Dispatch, Harness, BaseDir, Startup, Shutdown string`, +`Env map[string]string`), loaded once when the command starts — the way +the items file and the fleet file already are — from `--harness-config`, +or `harness.yaml` beside the items file, or neither. `resolvePlan` folds +`Env` into the launch's environment for every item; an entry naming the +token's own variable is refused there, before any item runs, the way an +unrecognised `--dispatch` value already is. A missing file is not an +error; a present-but-unparsable one is, the way a malformed items file +already is. + +`Dispatch`'s and `Harness`'s precedence against `--dispatch` and +`--harness` are each resolved with the same `cmd.Flags().Changed(...)` +check `orchestratorListenAddr` already uses for `--listen`/`--loopback`: +an explicit flag beats the file outright, so `harness.yaml` never has to +know whether the flag's value it might be overridden by is the flag's own +default or a real choice — the command already tracks that distinction, +and `HarnessConfig` does not need to. `Harness`'s own fallback, where +neither the flag nor harness.yaml says anything, is `harness.Resolve`'s +existing chain (the `HARNESS` environment variable, the stored +preference, the default) — harness.yaml slots in ahead of that chain, +behind the flag, rather than replacing it. + +`BaseDir` is resolved once, inside `LoadHarnessConfig`, against +harness.yaml's own directory where it is itself relative — before +`dispatchConfig` or `WorkList` ever see it, so every later use +(`ResolveItemDir`) just joins, never re-resolves. Both `Dispatcher` and +`dockerLauncher` gain a `WithBaseDir` builder, mirroring `WithHarnessConfig`; +`WorkList` gains one too, since `Remove`'s own cleanup needs to resolve an +item's directory the same way a launch already does. + +### The wrapper: how shutdown gets to always run + +Neither backend's `Child` is naturally in a position to run a second +script *after* an abort's `Stop`/`Kill` — those act directly on the +harness's own process (group) or container. Guaranteeing shutdown runs +regardless of why the harness stopped means the thing `Stop`/`Kill` act on +has to be able to run shutdown itself once the harness underneath it is +done, not the orchestrator racing to run it separately after `Wait` +returns. + +Both backends launch a small generated shell script instead of the +harness directly, whenever `harness.yaml` names a startup or shutdown +script (never, otherwise — the common case still launches the harness +directly exactly as today, no wrapper, no behavior change): + +```sh +#!/bin/sh +set -u +term() { [ -n "${child:-}" ] && kill -TERM "$child" 2>/dev/null; } +trap term TERM INT + +sh -c "$STARTUP" ; startup_rc=$? +if [ "$startup_rc" -ne 0 ]; then + sh -c "$SHUTDOWN" + exit 97 # sentinel: startup failed, the harness never ran +fi + +"$@" & child=$! +wait "$child"; harness_rc=$? + +sh -c "$SHUTDOWN" +exit "$harness_rc" +``` + +(`sh -c ""` is a no-op where a script is absent, so the same wrapper +serves startup-only, shutdown-only, and both.) The wrapper is what +`Stop`/`Kill` now act on — its own process group for bare, the container +for docker — so a `SIGTERM` reaches it first: it forwards the signal to +the harness child, waits for that child the same as the clean-finish path +does, then still runs shutdown before the wrapper itself exits. `Abort`'s +existing wait on `fl.done` therefore already covers shutdown; nothing +about `stopGrace`'s bound, or where `Abort` waits, needs to change — it is +now bounding a slightly longer sequence inside the same process. + +`Wait`'s exit code 97 is how `resolvePlan`'s caller tells a startup +failure apart from the harness's own: it fails the item naming the +startup script, rather than whatever the harness itself would have meant +by exiting 97 — a harness's own exit code never reaches this path where +there is no wrapper (no startup/shutdown named), so the sentinel only +matters, and only ever appears, when the wrapper is the one running. + +Startup's and shutdown's own stdout/stderr go to the same `logPath` the +harness's already does — the wrapper never redirects internally; whatever +opens `logPath` for the wrapper process (bare: `startChild`'s existing +redirect; docker: the container's own stdout, already what `docker run`'s +log capture points at) catches all three in the order they actually +wrote, with no separate plumbing. + +A failing shutdown script does not change the item's own recorded +outcome — that is the harness's exit code (or the sentinel), not +shutdown's — but is visible in the kept log the way shutdown's output +always is, so a broken teardown is not silent. + +### A second copy of the log, written once, not streamed + +`Child` gains a decorator, `logCopyChild` (`state.go`, alongside +`ItemLogFile`), the same shape as `sentinelChild`: its `Wait` calls +through, then copies the canonical log (`logPath`) into +`ItemLogFile(plan.dir)` — a plain read then write, not a live mirror. Both +`Dispatcher.Launch` and `dockerLauncher.Launch` wrap the child in it, +outside `wrapChild`, so the copy runs regardless of whether a lifecycle +wrapper is in play. + +A live mirror — writing both files as the agent's own output arrives — +was rejected: `startChild` and `runDockerContainer` currently pass the +opened log file straight to `cmd.Stdout`/`cmd.Stderr`, letting `exec` +`dup` the descriptor into the child and let the parent close its own copy +immediately (the "the child holds its own copy of the handle" comment +each already carries). Writing to two files at once means an +`io.MultiWriter` instead, which `exec` cannot `dup` — it falls back to a +pipe and a copying goroutine per stream, keeping both file handles open +for the launch's own lifetime rather than closed the moment the child +starts. That is a real complication — tracking the handles on `procChild` +and `dockerChild`, closing them only after `Wait` returns — for a copy +`work logs`/the API never read from live in the first place; a +post-`Wait` copy gets the same end state with none of it. + +### gh in the image, opencode and Pi's own tool config untouched + +`gh` is installed alongside opencode and Pi in the Dockerfile +(task 4.1); it needs no config of its own to be useful for the common +cases (`gh pr create` against the public API), and `GH_TOKEN` is exactly +the kind of thing `harness.yaml`'s `env` now exists for an operator to +set without a new image. + +## Risks / Trade-offs + +- **A host-loopback service other than the gateway is no longer plain + `localhost` from inside the container**, now that `--network host` is + gone — see "A loopback gateway reaches the container by + host.docker.internal, not host networking" above. An item that needs + one reaches it via `host.docker.internal` itself. +- **A second image to build, publish and version.** Mitigated by tying + its tag to spinloop's own version, so there is one place a given binary + looks, not a moving target. +- **Per-launch config directories need cleanup.** `/config/` is + found from the item's own `Dir` the way its `workspace/` sibling is, so + `work remove` takes it out the same way it already removes an item's + kept output; `workspace/` itself is left alone — it can hold real + output an operator wants to keep, unlike the config `work remove` + itself rendered. +- **`docker stop --time 0` skips Docker's own grace entirely.** This is + deliberate (see above), but means a harness that needs more than the + orchestrator's own `stopGrace` to flush anything on `SIGTERM` gets less + time under docker than it might expect from `docker stop`'s normal + default. No different in kind from the bare backend's own + `stopGrace`-bounded `SIGTERM`-then-`SIGKILL`, which already applies the + same bound. +- **The wrapper adds a shell script to the process tree whenever + startup/shutdown are used**, and a hard `Kill` (the grace already run + out) still ends everything underneath it at once — shutdown does not + get to run to completion against a `SIGKILL`, only against the polite + signal. This matches what "hard end" already means for the bare + backend's own process group; it is a real limit on the "always runs" + guarantee, not a gap specific to this change. +- **Exit code 97 is a sentinel, not a reserved range.** A harness whose + own one-shot form happens to exit 97 for an unrelated reason cannot be + told apart from a startup failure — mitigated by the sentinel only ever + being read where the wrapper actually ran (i.e. only when + startup/shutdown are named at all), so a run with no harness.yaml, or + one naming neither script, is completely unaffected. diff --git a/openspec/changes/archive/2026-09-18-docker-agent-dispatch/proposal.md b/openspec/changes/archive/2026-09-18-docker-agent-dispatch/proposal.md new file mode 100644 index 00000000..73d2d2ad --- /dev/null +++ b/openspec/changes/archive/2026-09-18-docker-agent-dispatch/proposal.md @@ -0,0 +1,109 @@ +## Why + +The orchestrator's dispatch has exactly one path today: `Dispatcher.Launch` +runs the harness as a bare `exec.Command` on the same machine as the +orchestrator, in its own process group, with no isolation beyond that. An +item's agent reads and writes the item's directory, and whatever else its +process can reach, directly on the orchestrator's host. Operators want +stronger isolation for less-trusted work — running the agent inside a +container rather than bare on the host — and the dispatch path has to +become a choice rather than a given for that to be possible. + +## What Changes + +- `Dispatcher` gains a pluggable backend: the launch mechanics (how a + harness process actually starts, and what it runs inside) move behind an + interface, with the existing bare-process behavior as the first backend + and a new docker backend as the second. +- `spinloop orchestrator` takes a `--dispatch` flag choosing the backend + for every launch the run makes (`bare`, the default, or `docker`). This + is a run-wide choice, not per-node or per-item. +- Under `docker`, an item's agent runs inside a container from an official + spinloop image, on the container's own network — a loopback-bound + gateway address is rewritten to `host.docker.internal` for the + container's own rendered config, with no change to the fleet file or + the gateway flag, and no reliance on a Docker Desktop setting that + makes `--network host` behave the way it does on native Linux docker — + with two bind mounts, both subdirectories of the item's own directory + (created fresh where not already there, under either backend): a + `workspace` subdirectory as the harness's working directory, and, under + docker, a `config` subdirectory holding a config generated fresh for + that one launch, mounted where the harness resolves its own config to + inside the container — redirected there with the harness's own + environment variable for the purpose (`XDG_CONFIG_HOME` for opencode, + `PI_CODING_AGENT_DIR` for Pi) rather than mounted into the image's fixed + `$HOME` — and no mount of the host's real harness config. +- The official image is a new artifact this change publishes: opencode and + Pi's one-shot forms, their runtimes, the `gh` CLI (an item's instructions + routinely want it — opening a PR, reading an issue), nothing of the + host's own configuration baked in. +- An operator's `harness.yaml` — found beside the items file by default, or + named with `--harness-config` — configures the run's harness beyond what + a fleet file or flag reaches: a `dispatch`, naming the backend the way + `--dispatch` does (an explicit flag still wins), a `harness`, naming the + harness the way `--harness`/`-H` does (an explicit flag still wins, + otherwise tried ahead of the `HARNESS` environment variable and the + stored preference), a `baseDir` an item's own relative `dir` resolves + against in place of the orchestrator's own working directory, an `env` + map added to the launch's environment, and `startup`/`shutdown` shell + scripts bracketing it. Both backends honour it: the scripts run in the + item's own `workspace` subdirectory — on the host for bare, inside the + container for docker. A failing startup script + fails the item before the harness ever runs; shutdown always runs once + the harness has ended, whatever ended it — done, failed, or aborted — + since it exists to tear down whatever startup set up. Both scripts' + output joins the harness's own in the item's kept log, in order. +- Once an admitted item's agent ends, under either backend, its kept log + is also copied into the item's own directory (`/log`), alongside + `workspace`/`config` — a static copy, written once, not a live mirror: + `work logs` and the work list API still read the canonical copy beside + the items file, which streams while the agent runs. `work remove` takes + the copy out with the rest of the item's kept output. + +## Capabilities + +### New Capabilities + +- `agent-dispatch`: how the orchestrator runs an admitted item's harness + process — the backend it chooses, and what each backend guarantees. The + bare backend is today's behavior, restated as a backend rather than the + only path; docker is the first alternative. + +### Modified Capabilities + +- `fleet-orchestrator`: "Running an item" now says the agent works in a + `workspace` subdirectory of the item's own directory, not the item's + directory directly. The outcome it already states — one-shot, inference + pointed at the gateway, output kept — still holds under either backend + by design; only where, precisely, the agent runs changed, and the rest + of that mechanics-level detail belongs to `agent-dispatch`. + +## Impact + +- `internal/orchestrator/dispatch.go`: the `Dispatcher`/`Child` split + becomes a launcher interface with two implementations +- `cmd/spinloop/orchestrator.go`: the `--dispatch` flag +- A new `internal/orchestrator` (or sibling) package for the docker + backend, using the Docker Engine API or CLI to run and stop containers +- A new image build (Dockerfile, and its publishing) under, e.g., + `images/agent/` — exact layout decided in design.md +- `internal/orchestrator`: a `harness.yaml` loader, and the wrapper that + bare and docker launches alike run in place of the harness directly, + when there is a startup or shutdown script to bracket it with +- `openspec/specs/agent-dispatch/spec.md`: new capability +- `openspec/specs/fleet-orchestrator/spec.md`: "Running an item" updated + for where the agent actually runs + +## Non-Goals + +- Docker's stronger, VM-based sandbox product — a future backend behind + the same interface, not built now +- Remote dispatch — running an item's agent on a machine other than the + orchestrator's own host — also a future backend; `--dispatch` chooses + among local backends only for now +- Per-node or per-item backend selection: one backend for the whole run +- A host-loopback service *other than the gateway* being reachable as + plain `localhost` from inside the container: the container is on its + own network, and only the gateway address is rewritten to reach the + host; an item needing another host-loopback service reaches it via + `host.docker.internal` itself diff --git a/openspec/changes/archive/2026-09-18-docker-agent-dispatch/specs/agent-dispatch/spec.md b/openspec/changes/archive/2026-09-18-docker-agent-dispatch/specs/agent-dispatch/spec.md new file mode 100644 index 00000000..41497723 --- /dev/null +++ b/openspec/changes/archive/2026-09-18-docker-agent-dispatch/specs/agent-dispatch/spec.md @@ -0,0 +1,291 @@ +## Purpose + +How the orchestrator runs an admitted item's harness process: the backend +it chooses, and what each backend guarantees the agent and its isolation +from the host. A bare process on the orchestrator's own host is one +backend; a docker container is another, with a stronger sandbox and a +remote host as later additions behind the same choice. + +## ADDED Requirements + +### Requirement: Choosing the dispatch backend + +`spinloop orchestrator` SHALL take a `--dispatch` flag naming the backend +every launch the run makes uses, one of `bare` or `docker`. Where the flag +is not given, harness.yaml's own `dispatch` (see "Configuring the harness +with harness.yaml") SHALL be the run's choice where it names one; where +neither says anything, the run SHALL use `bare`. A name neither the flag +nor harness.yaml's field recognises SHALL fail the command before it works +an item, naming whichever of the two named it, the value, and the accepted +set. The choice is the run's: every item the run launches goes through the +same backend, not a choice per node or per item. + +#### Scenario: The default is the bare backend + +- **WHEN** the operator runs the command with no `--dispatch` flag and no + harness.yaml naming a dispatch +- **THEN** every item it admits runs through the bare backend + +#### Scenario: Docker is named + +- **WHEN** the operator runs the command with `--dispatch docker` +- **THEN** every item it admits runs through the docker backend + +#### Scenario: harness.yaml names the backend + +- **WHEN** the operator runs the command with no `--dispatch` flag, and + harness.yaml carries a `dispatch` +- **THEN** every item it admits runs through the backend harness.yaml + names + +#### Scenario: An explicit flag wins over harness.yaml's + +- **WHEN** the operator runs the command with `--dispatch`, and + harness.yaml also carries a `dispatch` naming a different backend +- **THEN** every item it admits runs through the flag's backend + +#### Scenario: An unrecognised backend stops the command + +- **WHEN** the operator runs the command with `--dispatch`, or harness.yaml + with no `--dispatch` given, naming anything but `bare` or `docker` +- **THEN** the command fails before it works an item, naming whichever of + the two named it, the value, and the accepted set + +### Requirement: The bare backend + +The bare backend SHALL run an admitted item's harness as a process on the +orchestrator's own host, in its own process group, working in a +`workspace` subdirectory of the item's own directory (created fresh on +every launch, if it is not already there), its config the harness's own +host config file, written with the node's provider before the launch the +way it always has been. Stopping the item SHALL stop the process group: +the polite signal first, then, where the grace runs out, the hard end. + +#### Scenario: An item runs as a bare process + +- **WHEN** the run admits an item under the bare backend +- **THEN** the harness runs as a process on the orchestrator's host, + working in the item's own `workspace` subdirectory, its inference + pointed at the gateway + +### Requirement: The docker backend + +The docker backend SHALL run an admitted item's harness inside a container +from the official spinloop agent image. A gateway address bound to the +orchestrator host's own loopback SHALL reach the container by +`host.docker.internal` instead — never a change to the fleet file or the +gateway flag — so it answers from inside the container the way it does a +bare process; a gateway already on a routable address is reachable from +the container's own network unchanged. The container SHALL carry two +mounts, both from subdirectories of the item's own directory (created +fresh where not already there): a `workspace` subdirectory as the +harness's working directory, and a `config` subdirectory holding a +config generated fresh for that one launch, carrying only that launch's +provider — never the orchestrator host's own harness configuration — +mounted where the harness resolves its own config to inside the +container. The token the provider needs SHALL reach the container through +its environment, the way the bare backend's does. Stopping the item SHALL +stop the container: the polite signal first, then, where the grace runs +out, the hard end. A docker failure — the daemon unreachable, the image +missing, the container refusing to start — SHALL fail the item alone, +naming the item and the cause, the rest of the backlog going on. + +#### Scenario: An item runs inside a container + +- **WHEN** the run admits an item under the docker backend +- **THEN** a container starts from the official agent image, the item's + own `workspace` subdirectory mounted as the harness's working + directory, and the harness's inference reaches the gateway + +#### Scenario: A loopback gateway reaches the container + +- **WHEN** the gateway is bound to the orchestrator host's own loopback, + and the run admits an item under the docker backend +- **THEN** the container's harness reaches it by `host.docker.internal`, + with nothing changed in the fleet file or the gateway flag + +#### Scenario: The container's config carries only this launch + +- **WHEN** the run admits an item under the docker backend +- **THEN** the container's mounted config carries this launch's provider + alone, and no other configuration the orchestrator host's own harness + config carries + +#### Scenario: Stopping a containerized item + +- **WHEN** a containerized item is aborted, or the run's own shutdown + stops it +- **THEN** the container is stopped the polite way first, and killed where + the grace runs out before it has + +#### Scenario: A docker failure fails the item alone + +- **WHEN** the docker daemon does not answer, or the agent image is not + present, when the run tries to launch an item under the docker backend +- **THEN** the item is failed, naming the item and the cause, and the rest + of the backlog goes on + +### Requirement: The official agent image + +Spinloop SHALL publish an image the docker backend's containers run from, +carrying opencode and Pi's one-shot forms and the runtimes they need, the +`gh` CLI, and none of any operator's own harness configuration: a +container from it, before the docker backend mounts a launch's config in, +has no provider configured. + +#### Scenario: The image carries no baked-in configuration + +- **WHEN** a container starts from the official agent image with nothing + else mounted in +- **THEN** the harnesses it carries have no provider configured + +#### Scenario: gh is on the image + +- **WHEN** a container starts from the official agent image +- **THEN** `gh` runs in it, for an item's instructions that want it + +### Requirement: Configuring the harness with harness.yaml + +`spinloop orchestrator` SHALL read `harness.yaml` beside the items file by +default, or the file `--harness-config` names where the flag is given; +where neither the named file nor the default is present, the run SHALL +proceed exactly as it does without one. The file MAY carry a `dispatch`, +naming the backend the way `--dispatch` does — see "Choosing the dispatch +backend" for how the two are reconciled. The file MAY carry a `harness`, +naming the harness the way `--harness`/`-H` does; an explicit `--harness` +wins over it outright, and where neither says anything the harness is +resolved the way it always has been (the `HARNESS` environment variable, +then the stored preference, then the default). The file MAY carry a +`baseDir`: an item's own `dir`, where it is not already absolute, SHALL +resolve against it rather than the directory the orchestrator command +happens to be started from; a relative `baseDir` itself resolves against +`harness.yaml`'s own directory. The file MAY carry an `env` map, each +entry added to every launch's environment, under both backends alike. An +entry naming the variable the resolved token is presented under SHALL be +refused, naming it, before the command works an item. The file MAY also +carry `startup` and `shutdown`, each a multiline shell script — see "The +startup and shutdown scripts". + +#### Scenario: No harness.yaml changes nothing + +- **WHEN** the operator runs the command with no `--harness-config` flag + and no `harness.yaml` beside the items file +- **THEN** every launch proceeds exactly as it would with no harness.yaml + at all + +#### Scenario: harness.yaml names the harness + +- **WHEN** the operator runs the command with no `--harness` flag, and + harness.yaml carries a `harness` +- **THEN** every item it admits runs against the harness harness.yaml + names + +#### Scenario: An explicit harness flag wins over harness.yaml's + +- **WHEN** the operator runs the command with `--harness`, and + harness.yaml also carries a `harness` naming a different one +- **THEN** every item it admits runs against the flag's harness + +#### Scenario: harness.yaml's baseDir resolves a relative item dir + +- **WHEN** harness.yaml carries a `baseDir`, and an admitted item's `dir` + is relative +- **THEN** the item's directory is that `dir` resolved against `baseDir`, + not the directory the orchestrator command was started from + +#### Scenario: baseDir does not affect an absolute item dir + +- **WHEN** harness.yaml carries a `baseDir`, and an admitted item's `dir` + is already absolute +- **THEN** the item's directory is that `dir`, unchanged + +#### Scenario: harness.yaml's env reaches the launch + +- **WHEN** harness.yaml carries an `env` entry +- **THEN** every item's launch, under either backend, carries that + variable in its environment + +#### Scenario: An env entry colliding with the token is refused + +- **WHEN** harness.yaml's `env` names the same variable the resolved + token is presented under +- **THEN** the command fails before it works an item, naming the variable + +#### Scenario: An explicit file is read instead of the default + +- **WHEN** the operator runs the command with `--harness-config` naming a + file +- **THEN** that file is read, whether or not `harness.yaml` exists beside + the items file + +### Requirement: The startup and shutdown scripts + +A `startup` script SHALL run before the harness, under both backends +alike, in the item's own `workspace` subdirectory — the container's +`/item/workspace` for the docker backend — under the launch's full +environment — harness.yaml's `env` and everything the launch already +sets. A `startup` script that exits non-zero SHALL fail the item, naming +the script's failure, before the harness ever runs. + +A `shutdown` script SHALL run once the harness has ended — a clean finish, +a failure, or an abort alike — provided a `startup` script ran at all, +whether or not that script itself succeeded: shutdown exists to tear down +whatever startup set up, which a failed startup may have partly done. +Where the item is aborted, the run's own answer to the abort SHALL wait +for the shutdown script the way it already waits for the harness itself, +bound by the same grace. + +Both scripts' output SHALL join the harness's own in the item's kept log, +in the order they ran: startup's, then the harness's, then shutdown's. + +#### Scenario: Startup runs before the harness + +- **WHEN** harness.yaml names a startup script and the run admits an item +- **THEN** the script runs before the harness, in the launch's environment + +#### Scenario: A failing startup script fails the item + +- **WHEN** the startup script exits non-zero +- **THEN** the item is failed, naming the script's failure, and the + harness never runs + +#### Scenario: Shutdown runs after a clean finish + +- **WHEN** harness.yaml names a shutdown script and the harness ends on + its own +- **THEN** the shutdown script runs before the item is recorded done or + failed + +#### Scenario: Shutdown runs after an abort + +- **WHEN** harness.yaml names a shutdown script and a running item is + aborted +- **THEN** the harness is stopped, the shutdown script then runs, and the + abort is answered once it has, bound by the same grace the harness's own + stop already has + +#### Scenario: Shutdown runs even where startup failed + +- **WHEN** harness.yaml names both scripts and startup exits non-zero +- **THEN** the shutdown script still runs before the item is recorded + failed + +#### Scenario: Script output joins the kept log + +- **WHEN** harness.yaml names a startup or shutdown script +- **THEN** the script's own output is in the item's kept log, in the order + it ran relative to the harness's own output + +### Requirement: An item's own copy of its log + +Once an admitted item's agent has ended — whatever the outcome — its kept +log SHALL also be copied into the item's own directory, under both +backends alike. This copy is not live: while the agent runs, the item's +kept log beside the items file is the one that streams; the item +directory's own copy is written once, after the agent ends. + +#### Scenario: The log is copied once the agent ends + +- **WHEN** an admitted item's agent ends, under either backend +- **THEN** the item's own directory carries a copy of the item's kept + log, matching it diff --git a/openspec/changes/archive/2026-09-18-docker-agent-dispatch/specs/fleet-orchestrator/spec.md b/openspec/changes/archive/2026-09-18-docker-agent-dispatch/specs/fleet-orchestrator/spec.md new file mode 100644 index 00000000..c9da0099 --- /dev/null +++ b/openspec/changes/archive/2026-09-18-docker-agent-dispatch/specs/fleet-orchestrator/spec.md @@ -0,0 +1,40 @@ +## MODIFIED Requirements + +### Requirement: Running an item + +An admitted item SHALL run as a one-shot agent: the active harness, in its +non-interactive single-task form, given the item's instructions, working in +a `workspace` subdirectory of the item's own directory, with its inference +pointed at the gateway and the model of the node the item was matched to. +The agent's output SHALL be kept, per item, beside the items file, so a +finished or failed item can be read after the fact. The orchestrator SHALL +verify the item's directory exists before it launches the agent; an item +whose directory is missing SHALL be failed, naming the item, and the rest +of the backlog SHALL go on. Where the command was given +`--create-item-dirs`, the orchestrator SHALL create the missing directory +instead, and the item SHALL go on to launch; a directory it cannot create +SHALL fail the item, naming the item and the cause. + +#### Scenario: An admitted item runs against the gateway + +- **WHEN** the orchestrator admits an item matched to a node +- **THEN** its agent runs in a `workspace` subdirectory of the item's + directory, one-shot, with its inference pointed at the gateway and the + node's model + +#### Scenario: A missing directory fails the item alone + +- **WHEN** an admitted item's directory does not exist +- **THEN** the item is failed, naming it, and other items still run + +#### Scenario: A missing directory is created where the command says to + +- **WHEN** the command is given `--create-item-dirs` and an admitted item's + directory does not exist +- **THEN** the orchestrator creates the directory, and the item's agent + runs in its workspace subdirectory + +#### Scenario: An item's output is kept + +- **WHEN** an item's agent finishes, whatever its outcome +- **THEN** what the agent said is kept, per item, beside the items file diff --git a/openspec/changes/archive/2026-09-18-docker-agent-dispatch/tasks.md b/openspec/changes/archive/2026-09-18-docker-agent-dispatch/tasks.md new file mode 100644 index 00000000..bb7ef7e8 --- /dev/null +++ b/openspec/changes/archive/2026-09-18-docker-agent-dispatch/tasks.md @@ -0,0 +1,184 @@ +## 1. Extract the shared plan, no behavior change + +- [x] 1.1 Extract `resolvePlan` from `Dispatcher.Launch`: the directory + check/create, the one-shot form lookup, the catalogue provider, and the + `spinloop.Selection` — verify with `go test ./internal/orchestrator/...` + passing unchanged (existing `Launch` tests still hold against the + refactor alone) +- [x] 1.2 Introduce the `Launcher` interface (`Launch(Item, Node, string) + (Child, error)`) and have `Dispatcher` itself implement it — its + `Launch` calls `resolvePlan` then does exactly what it does today, no + separate wrapper type — verify with the existing dispatch/worklist test + suite passing with no changes to its assertions +- [x] 1.3 Change `WorkList`'s dispatcher field to hold a `Launcher`, `NewWorkList` and `NewDispatcher`'s + callers updated — verify with `go build ./...` and the orchestrator + suite passing + +## 2. Render a provider config without touching the host + +- [x] 2.1 Add `RenderProviderConfig` to `harness.Harness`, implemented for + opencode and Pi by factoring their `Apply`'s "no existing file" branch + out into it, `Apply` calling the same function for that case — verify + with a test asserting `Apply` on an empty host config and + `RenderProviderConfig`'s bytes parse to the identical provider block +- [x] 2.2 Lucinate's harness is left without the method (it has no + one-shot form, so the docker backend never dispatches it) — verify + `internal/harness` still builds without a lucinate implementation, and + document why in a comment beside the interface method + +## 3. `harness.yaml` and the wrapper + +- [x] 3.1 Add a `harness.yaml` loader: `--harness-config`, else + `harness.yaml` beside the items file, else nothing, read once at + startup — verify with a test for each of the three cases, and one for a + present-but-unparsable file failing the command before it works an item +- [x] 3.2 Fold `harness.yaml`'s `env` into `resolvePlan`'s environment; an + entry naming the resolved token's variable fails the command before any + item is worked, naming the variable — verify with a test for the merge + and one for the refusal +- [x] 3.2a Add `dispatch` to `HarnessConfig`: harness.yaml's choice of + backend where `--dispatch` is not given, an explicit flag still winning + outright — verify with a test for harness.yaml's value being used, one + for the flag overriding it, and one for an unrecognised value from + either source failing the command before it works an item +- [x] 3.2b Add `harness` to `HarnessConfig`: harness.yaml's choice of + harness where `--harness`/`-H` is not given, tried ahead of + `harness.Resolve`'s own env-var/stored-preference/default chain, an + explicit flag still winning outright — verify with a test for + harness.yaml's value being used and one for the flag overriding it +- [x] 3.2c Add `baseDir` to `HarnessConfig`, resolved once against + harness.yaml's own directory inside `LoadHarnessConfig`; `ResolveItemDir` + joins an item's relative `dir` under it, leaving an absolute `dir` + unaffected; `Dispatcher`, `dockerLauncher` and `WorkList` each gain a + `WithBaseDir` builder, `resolvePlan` and `Remove`'s own cleanup both + resolving through it — verify with a test for a relative `baseDir` + resolving against harness.yaml's directory, one for an absolute + `baseDir` unchanged, one for a relative item `dir` joining it, one for + an absolute item `dir` unaffected, and one for `Remove`'s cleanup + resolving the same way a launch does +- [x] 3.3 Add the wrapper: a generated POSIX shell script (design.md's + "The wrapper" decision) run in place of the harness directly whenever + `harness.yaml` names a `startup` or `shutdown` script; where neither is + named, launch the harness directly exactly as today — verify with a + unit test rendering the script for startup-only, shutdown-only, both, + and neither, and shell-checking the rendered output +- [x] 3.4 Wire the wrapper into the bare backend: `startChild` runs the + wrapper (`/bin/sh -c