feat(compose): stream output lines and fall back to stderr

[strausmann]

What this changes

Two things in the compose path, both needed before Dockhand can show a live console view for agent-managed environments (Finsys/dockhand#506, Finsys/dockhand#1499).

1. Execute() can report lines as they arrive

internal/docker/compose.go buffered stdout/stderr into bytes.Buffer and returned one block when the process exited. It now takes an optional line callback. Without a callback the behaviour is unchanged — same buffering, same return value.

internal/edge/client.go's handleComposeRequest sets that callback when the caller asks for it (new streamOutput field in the payload) and sends one message per line, modelled on handleStreamingRequest in the same file.

2. result.Output falls back to stderr

result.Output was stdout.String() with no stderr fallback. Compose writes to stderr, so on the agent path a successful run returned an effectively empty output and Dockhand fell back to its placeholder text almost every time. The local path in Dockhand already does stdout || stderr || placeholder; this brings the agent path in line.

This is a small fix with a visible effect on its own — even without any Dockhand change, agent-side compose results stop coming back blank.

Compatibility

streamOutput is a new optional payload field. Agents built before this change ignore it (json.Unmarshal without DisallowUnknownFields), and Dockhand reacts to whatever arrives — either line messages followed by the result, or just the result. No version negotiation.

Conversely, an agent with this change talking to a Dockhand without it simply never gets asked to stream.

Related

• feat(stacks): live compose output and deploy history dockhand#1499 — the Dockhand side. It stands alone without this PR (local environments stream immediately); this PR is what extends it to agent-managed environments.
• fix: give compose operations their own timeout instead of RequestTimeout #96 and fix: explain compose failures caused by our own timeout, not the compose run itself #98 — compose timeout. Without those, a long compose run is still killed by the generic RequestTimeout mid-stream, which with live output becomes very visible: you watch lines arrive and then it stops with no explanation. They are independent of this PR but land in the same area.

Testing

go build ./..., go vet ./..., go test ./... all clean.

Added coverage for: teeLines flushing a final unterminated line on close, build output landing on stdout, and the stderr fallback. The docker-dependent build test is gated behind an env switch so it does not require a daemon in CI.

[jotka]

Really nice work on this, the streaming path and the back-compat story (opt-in streamOutput, base64 []byte matching Dockhand's decode) all check out on our side.

One thing before this goes in though: teeLines can deadlock the whole compose run. The scanner is capped at 1MB (sc.Buffer(..., 1024*1024)), so a single output line longer than that with no newline makes Scan() return ErrTooLong and the scanner goroutine exits — but cmd.Stdout is still the io.MultiWriter(dst, pw), so the subprocess's next write to pw blocks forever with nobody draining the pipe. cmd.Run() then hangs in Wait() and COMPOSE_TIMEOUT doesn't rescue it (the kill goes through the same wedged Wait). Net effect: the deploy stalls indefinitely and the goroutine + subprocess leak.

It's not just theoretical — a build: step that echoes a large blob, or any tool drawing a \r-only progress bar (no \n), is one giant token. Reproduced it locally: 2MB unbroken line → bufio.Scanner: token too long → all goroutines are asleep - deadlock.

We'll take care of the fix on our end. Simplest thing that worked for us is dropping bufio.Scanner for a bufio.Reader + ReadString('\n') — no line-length cap, returns the trailing partial on EOF, so the pipe never wedges (verified the 2MB case goes through clean). Happy to push that onto the branch or leave it to you, whichever you prefer.

The rest is good to go from our review — #96 and #98 both look solid too. Heads-up on the merge: #99 and #96/#98 both add a fresh internal/edge/client_test.go, so whichever lands second hits an add/add conflict there (production client.go merges clean; the test symbols are disjoint so it's just a union).

[strausmann]

Thanks for the careful read — you're right, and I reproduced it independently before touching anything.

I lifted teeLines verbatim out of 409e87c into a standalone harness and pushed a 2 MB unbroken line through it, followed by a normal \n-terminated line:

BEFORE (409e87c): DEADLOCK CONFIRMED — Write blocks >5s, the process would never finish
FIX (bufio.Reader + ReadString): no deadlock; close() returns
                                 1 line delivered (2,097,177 chars), dst buffer 2,097,178 bytes

Every link in your chain holds up:

1. sc.Buffer(…, 1024*1024) caps at 1 MB → a longer line without \n yields ErrTooLong and the scanner goroutine returns.
2. It never closes pr (no defer pr.Close()), and io.Pipe is unbuffered and synchronous, so the subprocess's next write to pw blocks forever with no reader.
3. cmd.Stdout is an io.Writer, not an *os.File, so os/exec copies through its own goroutine — and cmd.Wait() waits on exactly that goroutine, which is stuck in the blocked write.
4. Which is why the context timeout doesn't rescue it: exec.CommandContext kills the process, but Wait() still waits on the copier.

Please go ahead and push the fix — you've already verified the 2 MB case, and bufio.Reader + ReadString('\n') is the right call: no length cap, and the trailing partial comes back on EOF. Three small things from our findings we'd ask you to fold in while you're in there:

• defer pr.Close() in the reader goroutine. ReadString removes the one known reason for an early exit; the close covers any future one (a panic in onLine, say) by handing the writer ErrClosedPipe instead of a permanent block. It's what our harness above runs with.
• cmd.WaitDelay. It's currently unset, and it's the structural guard for this whole failure class — "Wait() hangs on an io goroutine" — rather than for this one trigger. With it, the timeout would actually bite no matter which writer wedges. Possibly out of scope here; happy for it to be a separate PR.
• A regression test that goes red against the unfixed teeLines. Worth the extra minute: a test that only pushes a 2 MB line and asserts "no deadlock" also passes against the broken version if the harness happens to drain the pipe. Ours asserts on the delivered line and the buffer contents after close() returns, which the broken version cannot satisfy. Say the word and I'll post the harness or open it as a test-only PR against your branch — whichever fits better.

Noted on the merge order too: #99 and #96/#98 both add internal/edge/client_test.go fresh, so the second one in takes an add/add conflict there. Agreed the symbols are disjoint and it resolves as a union — happy to take that on our branch whenever it comes up.

[strausmann]

@jotka — small correction to my comment above, and since an edit doesn't notify: I originally wrote that we'd take the fix ourselves. Scratch that, please go ahead and push it — you have it verified already, and there's no sense in us rebuilding it.

I've edited that comment to list the three things from our findings we'd like folded in: defer pr.Close() in the reader goroutine, cmd.WaitDelay (fine as a separate PR if you'd rather keep #99 narrow), and a regression test that actually goes red against the unfixed teeLines rather than just asserting "no deadlock". Happy to contribute the last one as a test-only PR against your branch if that's useful — just say which you prefer.

[jotka]

Thanks — we will have all three folded in:

• defer pr.Close() in the reader goroutine.
• cmd.WaitDelay (10s) on the compose command, so a wedged copier can't outlast the context.
• rgression test rewritten: the write runs off the test goroutine behind a 5s deadline, and it asserts on the delivered line (the 2MB line whole, then the trailing short line) and the dst buffer after close(). It fails fast and clean
  against the unfixed teeLines (~5s, clear message) instead of hanging to the package timeout, and the broken version can't satisfy the content assertions.

running the full ci/cd suite now (takes ~3hrs) with new agents, with all of these PRs and fixes on top. will release soon.

[Copilot]

🟡 Changes recommended

New tests can fail or race in environments without docker compose installed and under concurrent callbacks, and the streamed line handling should trim CRLF correctly to avoid incorrect output.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR extends the agent-side compose execution path to support live, per-line output streaming (when requested) and fixes “blank output” results by falling back to stderr when stdout is empty, aligning agent behavior with Dockhand’s local behavior.

Changes:

• Updated ComposeClient.Execute to optionally stream stdout/stderr lines via a callback while preserving buffered output behavior when no callback is provided.
• Added Edge compose request support for optional streaming (streamOutput) and introduced a composeExecutor interface to enable non-shelling test doubles.
• Added tests covering line streaming behavior, stderr fallback, and teeLines edge cases.

File summaries

File	Description
internal/server/http.go	Updates REST compose execution call to pass the new onLine parameter (nil for REST).
internal/edge/client.go	Introduces composeExecutor and streams compose output lines over WebSocket only when streamOutput is requested.
internal/edge/client_test.go	Adds WebSocket-based tests validating that compose streaming messages are only sent when requested.
internal/docker/compose.go	Implements teeLines, streams output lines, adds WaitDelay, and falls back result.Output to stderr when stdout is empty.
internal/docker/compose_test.go	Adds tests for streaming, stderr fallback, build-output-on-stdout behavior (gated), and teeLines edge cases.

Review details

Suppressed comments (1)

internal/docker/compose_test.go:48

• TestOutputFallsBackToStderr assumes docker compose is installed; if detectComposeCommand fails, Execute returns early and the assertion fails even though the behavior under test isn't reachable. Skipping when compose isn't available keeps this unit test environment-independent (it doesn't otherwise require a Docker daemon).

	c := NewComposeClient("", t.TempDir())
	res, err := c.Execute(context.Background(), &ComposeOperation{Operation: "ps"}, nil)

• Files reviewed: 5/5 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.