mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-08 09:26:50 +08:00
* fix(agent): deliver the OpenCode prompt on stdin, not argv (#6538) The daemon appended the whole task prompt as an argv element, so on Windows every OpenCode task whose command line cleared CreateProcess's 32,767 character lpCommandLine limit failed to start at all. Go surfaces the resulting ERROR_FILENAME_EXCED_RANGE (206) as "The filename or extension is too long", which reads as an exe-path problem and is not one. A workspace with a realistic set of models and skills clears that ceiling on the prompt alone (39,629 bytes in the report), leaving no usable in-app workaround. `opencode run` merges its variadic [message..] positional with whatever is piped in, so passing no positional makes the piped text the entire run message. That path is present in the reported v1.18.14 and back to v1.4.10. Write the prompt from its own goroutine, as the Pi and Cursor backends already do: a prompt larger than the pipe buffer blocks mid-write until OpenCode drains it, and OpenCode cannot drain while nobody consumes its stdout. OpenCode reads stdin to EOF, so stdin is closed exactly once as the end-of-prompt signal — including on the cancellation path, where closing it releases a writer still blocked on a full pipe. The existing process-group SIGTERM -> SIGKILL ordering from #4533 is unchanged. Keeping the prompt off argv also stops it being echoed into the "agent command" log line; prompt_bytes replaces it there. Scoped to OpenCode. DevEco, OpenClaw (#6032), Qwen, Copilot and Antigravity inline their prompts the same way, but each CLI's input contract needs verifying on its own before it moves. Co-authored-by: multica-agent <[email protected]> * fix(agent): do not fail a completed run on a benign prompt-write EPIPE The backend CI run for this PR failed in TestPiExecuteRetainsOnlyLastTurnOutput with "pi prompt write failed: write |1: broken pipe". The cause is a latent race shared by pi and the OpenCode code added in the previous commit, which copied pi's error handling. A child that exits without draining stdin closes the read end while the parent is still writing the prompt, so the parent's write returns EPIPE. Both backends converted that into a failed result even when the agent had already emitted a complete, successful stream — making the outcome depend on whether the write landed before or after the child exited, i.e. on machine load. OpenCode reads stdin to EOF before it does any work, so a run that produced a complete stream necessarily received the whole prompt; an EPIPE recorded after that only means the pipe closed on the way out. Report the write error only for a run that did NOT complete, where it can actually explain the failure, and append rather than overwrite so the stream's own diagnosis survives. cursor.go already guards this correctly by ignoring writeErr once a result was seen. Test fakes now drain stdin, matching what the real CLIs do. A fake that exits without reading is what manufactures the spurious EPIPE, and that is what broke CI — pi's piEventStreamScript is the fixture that failed. Adds a deterministic regression test: a child that emits a full successful stream and never reads a 1.2 MB prompt. Against the previous commit it reproduces the CI signature exactly (status "failed", "broken pipe"); with this change it passes. pi.go's production handling has the same defect and still needs the equivalent fix, but that belongs to whoever owns #6485 — this change keeps to OpenCode plus the test fixture that was breaking the build. Co-authored-by: multica-agent <[email protected]> * fix(agent): gate prompt-write suppression on a positive terminal signal Review caught a false-success path opened by the previous commit. It suppressed a failed prompt write whenever status was "completed", on the reasoning that a completed run must have received the whole prompt. That reasoning does not hold: processEvents starts finalStatus at "completed" and only fails closed on structural evidence — an open step, or a step awaiting a continuation. A child that emits NOTHING and exits 0 sets neither, so the default survives. The full path: the parent writes a large prompt, the child exits 0 without reading a byte, the write returns EPIPE, stdout hits EOF with no events, and the run is reported as completed with empty output and no error. The prompt never reached the agent and nothing said so. Reproduced deterministically before the fix: status="completed" output="" error="". Absence of a failure signal is not proof of success, so record the positive one instead. eventResult.sawTerminalSignal is set only when a step_finish closed the last step with no continuation pending, and the write error is suppressed only on that evidence. It is deliberately not the negation of noTerminalSignal: an empty stream sets neither, because there is nothing to fail closed on and nothing proving completion either. When the signal is absent the write error now also flips the default "completed" to "failed". Adds the empty-stream regression alongside the existing complete-stream one, so both directions of the branch are pinned. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]>
308 lines
11 KiB
Go
308 lines
11 KiB
Go
//go:build unix
|
|
|
|
package agent
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"log/slog"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
)
|
|
|
|
// opencodeStreamTail is the minimal successful OpenCode event stream the fake
|
|
// CLIs below emit once they have recorded what the backend handed them.
|
|
const opencodeStreamTail = `printf '%s\n' '{"type":"step_start","timestamp":1,"sessionID":"ses_fake","part":{"type":"step-start"}}'
|
|
printf '%s\n' '{"type":"text","timestamp":2,"sessionID":"ses_fake","part":{"type":"text","text":"ok"}}'
|
|
printf '%s\n' '{"type":"step_finish","timestamp":3,"sessionID":"ses_fake","part":{"type":"step-finish"}}'
|
|
`
|
|
|
|
// opencodeStdinProbe runs the OpenCode backend against a fake CLI that records
|
|
// argv, drains stdin to EOF, and emits a successful OpenCode JSON stream.
|
|
func opencodeStdinProbe(t *testing.T, prompt string) ([]string, string, Result) {
|
|
t.Helper()
|
|
|
|
dir := t.TempDir()
|
|
argvPath := filepath.Join(dir, "argv.txt")
|
|
stdinPath := filepath.Join(dir, "stdin.txt")
|
|
|
|
script := fmt.Sprintf("#!/bin/sh\n"+
|
|
": > %[1]q\n"+
|
|
"for a in \"$@\"; do printf '%%s\\n' \"$a\" >> %[1]q; done\n"+
|
|
"cat > %[2]q\n", argvPath, stdinPath) + opencodeStreamTail
|
|
|
|
fakePath := filepath.Join(dir, "opencode")
|
|
writeTestExecutable(t, fakePath, []byte(script))
|
|
|
|
backend, err := New("opencode", Config{ExecutablePath: fakePath, Logger: slog.Default()})
|
|
if err != nil {
|
|
t.Fatalf("New(opencode): %v", err)
|
|
}
|
|
session, err := backend.Execute(t.Context(), prompt, ExecOptions{
|
|
Timeout: 30 * time.Second,
|
|
Model: "lanz/Lanz-Medium",
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("Execute: %v", err)
|
|
}
|
|
go func() {
|
|
for range session.Messages {
|
|
}
|
|
}()
|
|
result := <-session.Result
|
|
|
|
argvRaw, err := os.ReadFile(argvPath)
|
|
if err != nil {
|
|
t.Fatalf("read recorded argv: %v", err)
|
|
}
|
|
stdinRaw, err := os.ReadFile(stdinPath)
|
|
if err != nil {
|
|
t.Fatalf("read recorded stdin: %v", err)
|
|
}
|
|
argv := strings.Split(strings.TrimSuffix(string(argvRaw), "\n"), "\n")
|
|
return argv, string(stdinRaw), result
|
|
}
|
|
|
|
// TestOpencodeExecuteSendsPromptOnStdinNotArgv is the regression for #6538.
|
|
// The daemon used to inline the whole task prompt as an argv element, which
|
|
// Windows CreateProcess rejects once the command line clears 32,767 characters.
|
|
// The prompt must now reach OpenCode byte-for-byte on stdin while argv keeps
|
|
// only the daemon's own flags.
|
|
func TestOpencodeExecuteSendsPromptOnStdinNotArgv(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
prompt := "MULTICA_AGENT_BUILDER_INPUT\n" +
|
|
"{\n" +
|
|
` "user_request": "专注本机 CPA 有关的所有工作",` + "\n" +
|
|
` "current_draft": {"instructions": "Run go build -ldflags \"-X main.version=foo\""},` + "\n" +
|
|
` "available_workspace_skills": [{"description": "- inspect local changes"}]` + "\n" +
|
|
"}"
|
|
|
|
argv, stdinGot, result := opencodeStdinProbe(t, prompt)
|
|
|
|
if stdinGot != prompt {
|
|
t.Errorf("prompt did not arrive on stdin intact:\n got %q\n want %q", stdinGot, prompt)
|
|
}
|
|
for _, arg := range argv {
|
|
for _, needle := range []string{"MULTICA_AGENT_BUILDER_INPUT", "user_request", "-X", "inspect local"} {
|
|
if strings.Contains(arg, needle) {
|
|
t.Errorf("prompt fragment %q leaked into argv element %q; argv=%v", needle, arg, argv)
|
|
}
|
|
}
|
|
}
|
|
joined := strings.Join(argv, " ")
|
|
for _, want := range []string{"run", "--format json", "--dangerously-skip-permissions", "--model lanz/Lanz-Medium"} {
|
|
if !strings.Contains(joined, want) {
|
|
t.Errorf("expected %q in argv, got %v", want, argv)
|
|
}
|
|
}
|
|
if result.Status != "completed" || result.Output != "ok" {
|
|
t.Fatalf("result = %+v, want completed with output ok", result)
|
|
}
|
|
}
|
|
|
|
// TestOpencodeExecutePromptExceedingWindowsCommandLineLimit pins the exact
|
|
// shape reported in #6538: a prompt well past the 32,767-character Windows
|
|
// command-line ceiling. On the old argv path this could not even start the
|
|
// process on Windows; on stdin it is just bytes.
|
|
func TestOpencodeExecutePromptExceedingWindowsCommandLineLimit(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
prompt := strings.Repeat("multica opencode workspace skill catalogue 0123456789\n", 800)
|
|
if len(prompt) <= 32767 {
|
|
t.Fatalf("test prompt must exceed the Windows command-line limit, got %d bytes", len(prompt))
|
|
}
|
|
|
|
_, stdinGot, result := opencodeStdinProbe(t, prompt)
|
|
|
|
if stdinGot != prompt {
|
|
t.Errorf("oversized prompt corrupted on stdin: got %d bytes, want %d", len(stdinGot), len(prompt))
|
|
}
|
|
if result.Status != "completed" {
|
|
t.Fatalf("status = %q, want completed; error=%q", result.Status, result.Error)
|
|
}
|
|
}
|
|
|
|
// TestOpencodeExecuteLargePromptDoesNotDeadlock forces both process pipes past
|
|
// capacity: the child floods stdout before reading stdin, while the parent
|
|
// writes a large prompt. Only concurrent writing and scanning can advance.
|
|
func TestOpencodeExecuteLargePromptDoesNotDeadlock(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
dir := t.TempDir()
|
|
stdinPath := filepath.Join(dir, "stdin.txt")
|
|
script := fmt.Sprintf("#!/bin/sh\n"+
|
|
"yes '{\"type\":\"noise\",\"pad\":\"0123456789012345678901234567890123456789\"}' | head -n 8000\n"+
|
|
"cat > %[1]q\n", stdinPath) + opencodeStreamTail
|
|
|
|
fakePath := filepath.Join(dir, "opencode")
|
|
writeTestExecutable(t, fakePath, []byte(script))
|
|
|
|
prompt := strings.Repeat("multica opencode stdin payload 0123456789\n", 16_384)
|
|
if len(prompt) < 512*1024 {
|
|
t.Fatalf("test prompt too small: %d bytes", len(prompt))
|
|
}
|
|
backend, err := New("opencode", Config{ExecutablePath: fakePath, Logger: slog.Default()})
|
|
if err != nil {
|
|
t.Fatalf("New(opencode): %v", err)
|
|
}
|
|
session, err := backend.Execute(t.Context(), prompt, ExecOptions{Timeout: 30 * time.Second})
|
|
if err != nil {
|
|
t.Fatalf("Execute: %v", err)
|
|
}
|
|
go func() {
|
|
for range session.Messages {
|
|
}
|
|
}()
|
|
result := <-session.Result
|
|
|
|
stdinRaw, err := os.ReadFile(stdinPath)
|
|
if err != nil {
|
|
t.Fatalf("read recorded stdin: %v", err)
|
|
}
|
|
if string(stdinRaw) != prompt {
|
|
t.Errorf("large stdin prompt corrupted: got %d bytes, want %d", len(stdinRaw), len(prompt))
|
|
}
|
|
if result.Status != "completed" {
|
|
t.Fatalf("status = %q, want completed; error=%q", result.Status, result.Error)
|
|
}
|
|
}
|
|
|
|
// TestOpencodeExecuteIgnoresBenignPromptWriteEPIPE pins that a run which
|
|
// produced a complete, successful stream is not failed by a prompt write that
|
|
// broke on the way out. A child exiting without draining stdin closes the read
|
|
// end under the concurrent write, so the parent sees EPIPE — and whether that
|
|
// lands before or after the child exits is pure timing. Reporting it as a
|
|
// failure makes the result load-dependent and throws away a good run.
|
|
func TestOpencodeExecuteIgnoresBenignPromptWriteEPIPE(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
dir := t.TempDir()
|
|
// A complete, successful stream from a child that never reads stdin.
|
|
script := "#!/bin/sh\n" + opencodeStreamTail
|
|
fakePath := filepath.Join(dir, "opencode")
|
|
writeTestExecutable(t, fakePath, []byte(script))
|
|
|
|
// Far larger than any pipe buffer, so the write cannot slip through before
|
|
// the child exits: EPIPE here is deterministic, not incidental.
|
|
prompt := strings.Repeat("benign epipe payload 0123456789\n", 40_000)
|
|
|
|
backend, err := New("opencode", Config{ExecutablePath: fakePath, Logger: slog.Default()})
|
|
if err != nil {
|
|
t.Fatalf("New(opencode): %v", err)
|
|
}
|
|
session, err := backend.Execute(t.Context(), prompt, ExecOptions{Timeout: 30 * time.Second})
|
|
if err != nil {
|
|
t.Fatalf("Execute: %v", err)
|
|
}
|
|
go func() {
|
|
for range session.Messages {
|
|
}
|
|
}()
|
|
result := <-session.Result
|
|
|
|
if result.Status != "completed" {
|
|
t.Fatalf("status = %q, want completed; error=%q", result.Status, result.Error)
|
|
}
|
|
if result.Output != "ok" {
|
|
t.Errorf("Output = %q, want %q", result.Output, "ok")
|
|
}
|
|
}
|
|
|
|
// TestOpencodeExecuteFailsWhenPromptWriteBreaksAndStreamNeverStarted is the
|
|
// counterpart to the test above and guards the false-success it could
|
|
// otherwise create. processEvents starts at "completed" and only fails closed
|
|
// on structural evidence (an open step, or a step awaiting a continuation), so
|
|
// a child that emits NOTHING and exits 0 leaves that default untouched. If the
|
|
// prompt write also broke, suppressing it on the strength of that default
|
|
// would report a clean, empty success for a run whose prompt never landed.
|
|
//
|
|
// Suppression must therefore key on a positive terminal signal, not on the
|
|
// absence of a negative one.
|
|
func TestOpencodeExecuteFailsWhenPromptWriteBreaksAndStreamNeverStarted(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
dir := t.TempDir()
|
|
// Emits no events at all and exits 0 without reading stdin.
|
|
fakePath := filepath.Join(dir, "opencode")
|
|
writeTestExecutable(t, fakePath, []byte("#!/bin/sh\nexit 0\n"))
|
|
|
|
// Large enough that the write cannot complete before the child exits.
|
|
prompt := strings.Repeat("undelivered prompt payload 0123456789\n", 36_000)
|
|
|
|
backend, err := New("opencode", Config{ExecutablePath: fakePath, Logger: slog.Default()})
|
|
if err != nil {
|
|
t.Fatalf("New(opencode): %v", err)
|
|
}
|
|
session, err := backend.Execute(t.Context(), prompt, ExecOptions{Timeout: 30 * time.Second})
|
|
if err != nil {
|
|
t.Fatalf("Execute: %v", err)
|
|
}
|
|
go func() {
|
|
for range session.Messages {
|
|
}
|
|
}()
|
|
result := <-session.Result
|
|
|
|
if result.Status == "completed" {
|
|
t.Fatalf("undelivered prompt reported as success: status=%q output=%q error=%q",
|
|
result.Status, result.Output, result.Error)
|
|
}
|
|
if !strings.Contains(result.Error, "prompt write failed") {
|
|
t.Errorf("error = %q, want it to name the failed prompt write", result.Error)
|
|
}
|
|
}
|
|
|
|
// TestOpencodeExecuteCancelReleasesBlockedPromptWriter covers the cancellation
|
|
// half of the stdin contract: a child that never reads stdin leaves the prompt
|
|
// writer blocked on a full pipe. Cancelling must close stdin so that goroutine
|
|
// unwinds, and the run must still settle as aborted rather than hang.
|
|
func TestOpencodeExecuteCancelReleasesBlockedPromptWriter(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
dir := t.TempDir()
|
|
// Emit one event so the backend is streaming, then sleep without ever
|
|
// draining stdin — the parent's prompt write wedges on a full pipe.
|
|
script := "#!/bin/sh\n" +
|
|
"printf '%s\\n' '{\"type\":\"step_start\",\"timestamp\":1,\"sessionID\":\"ses_fake\",\"part\":{\"type\":\"step-start\"}}'\n" +
|
|
"sleep 60\n"
|
|
fakePath := filepath.Join(dir, "opencode")
|
|
writeTestExecutable(t, fakePath, []byte(script))
|
|
|
|
prompt := strings.Repeat("blocked prompt payload 0123456789\n", 32_768)
|
|
if len(prompt) < 1024*1024 {
|
|
t.Fatalf("test prompt too small to fill the pipe buffer: %d bytes", len(prompt))
|
|
}
|
|
|
|
backend, err := New("opencode", Config{ExecutablePath: fakePath, Logger: slog.Default()})
|
|
if err != nil {
|
|
t.Fatalf("New(opencode): %v", err)
|
|
}
|
|
// The fake CLI does not trap SIGTERM, so the group dies inside the grace
|
|
// window; this test deliberately leaves opencodeTerminateGraceNanos alone so
|
|
// it stays parallel-safe alongside the cancellation tests that do set it.
|
|
ctx, cancel := context.WithCancel(context.Background())
|
|
session, err := backend.Execute(ctx, prompt, ExecOptions{Timeout: 30 * time.Second})
|
|
if err != nil {
|
|
t.Fatalf("Execute: %v", err)
|
|
}
|
|
go func() {
|
|
for range session.Messages {
|
|
}
|
|
}()
|
|
|
|
cancel()
|
|
|
|
select {
|
|
case result := <-session.Result:
|
|
if result.Status != "aborted" {
|
|
t.Fatalf("status = %q, want aborted; error=%q", result.Status, result.Error)
|
|
}
|
|
case <-time.After(30 * time.Second):
|
|
t.Fatal("cancelled run never produced a result; the prompt writer likely deadlocked")
|
|
}
|
|
}
|