diff --git a/internal/server/server_test.go b/internal/server/server_test.go index a8098bf..3bf589e 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -583,3 +583,80 @@ func TestConnectionHeaderListIsAnUpgrade(t *testing.T) { } } } + +// A known verb still exits immediately. Every existing client depends on this, +// including chant's Fly activities, so the interactive path below must not +// change it. +func TestKnownVerbStillExitsImmediately(t *testing.T) { + h := newHarness(t) + if code, body := h.do(http.MethodPost, "/v1/sprites", map[string]any{"name": "known"}); code != http.StatusCreated { + t.Fatalf("create sprite: %d %s", code, body) + } + + stdout, _, exit := h.execWS("known", "echo hi") + if stdout != "hi\n" || exit != 0 { + t.Fatalf("echo hi: stdout=%q exit=%d, want \"hi\\n\" and 0", stdout, exit) + } +} + +// An unrecognised command holds the session open and echoes stdin back, so a +// client that writes after opening exec finds something on the other end. +// +// That is the difference between a fountain turn completing and being orphaned +// on a write to a process that already exited (#18). +func TestUnrecognisedCommandEchoesStdinUntilEOF(t *testing.T) { + h := newHarness(t) + if code, body := h.do(http.MethodPost, "/v1/sprites", map[string]any{"name": "interactive"}); code != http.StatusCreated { + t.Fatalf("create sprite: %d %s", code, body) + } + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + u := "ws" + strings.TrimPrefix(h.ts.URL, "http") + + "/v1/sprites/interactive/exec?cmd=" + url.QueryEscape("claude --print") + c, _, err := websocket.Dial(ctx, u, nil) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer func() { _ = c.CloseNow() }() + + // The command itself comes back first, as it always has. + if got := readFrameOfStream(t, ctx, c, streamStdout); got != "claude --print\n" { + t.Fatalf("command echo: got %q", got) + } + + // Now write stdin the way a client does after opening exec. Before this + // change the session had already closed and this write went nowhere. + if err := c.Write(ctx, websocket.MessageBinary, append([]byte{streamStdin}, []byte("say hello")...)); err != nil { + t.Fatalf("write stdin: %v", err) + } + if got := readFrameOfStream(t, ctx, c, streamStdout); got != "say hello" { + t.Fatalf("stdin echo: got %q, want %q", got, "say hello") + } + + // EOF ends the turn, and only then does the exit frame arrive. + if err := c.Write(ctx, websocket.MessageBinary, []byte{streamStdinEOF}); err != nil { + t.Fatalf("write eof: %v", err) + } + typ, data, err := c.Read(ctx) + if err != nil { + t.Fatalf("read exit: %v", err) + } + if typ != websocket.MessageBinary || len(data) != 2 || data[0] != streamExit || data[1] != 0 { + t.Fatalf("exit frame: typ=%v data=%v", typ, data) + } +} + +// readFrameOfStream reads one binary frame and asserts its stream id. +func readFrameOfStream(t *testing.T, ctx context.Context, c *websocket.Conn, want byte) string { + t.Helper() + _, data, err := c.Read(ctx) + if err != nil { + t.Fatalf("read: %v", err) + } + if len(data) == 0 || data[0] != want { + t.Fatalf("frame stream id: got %v, want %d", data, want) + } + return string(data[1:]) +} diff --git a/internal/server/ws.go b/internal/server/ws.go index eb60e3e..fd33e56 100644 --- a/internal/server/ws.go +++ b/internal/server/ws.go @@ -63,14 +63,6 @@ func (s *Server) execSpriteWS(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - // Drain any client stdin frames in the background. The interpreter does not - // read stdin, so the bytes are discarded, but reading keeps the connection - // responsive and honors a client that streams stdin then a StreamStdinEOF - // frame. Draining stops on EOF, read error, or connection close. - if r.URL.Query().Get("stdin") != "false" { - go drainStdin(ctx, c) - } - result, err := s.store.Exec(id, cmd) if err != nil { if errors.Is(err, sprite.ErrNotFound) { @@ -91,6 +83,27 @@ func (s *Server) execSpriteWS(w http.ResponseWriter, r *http.Request) { return } } + // A known verb has said everything it is going to say, so exit and close — + // unchanged behaviour, and what every existing client depends on. + // + // An unknown command is a different situation. A real sprite would have + // started a process the caller can now write to, and callers do: fountain + // opens exec for its runtime command and then writes the prompt in as + // stdin. Exiting immediately means that write lands on a process that has + // already gone, and the caller's conversation server crashes on a call to + // a dead pid rather than getting an error back (INTENTIUS/spritzer#18). + // + // So an unrecognised command holds the session open and echoes stdin back + // on stdout until the caller says it is done. Still not execution — it is + // the same echo the interpreter already does, extended over time. + if result.Unrecognised && r.URL.Query().Get("stdin") != "false" { + echoStdinUntilEOF(ctx, c) + } else if r.URL.Query().Get("stdin") != "false" { + // Known verb: drain and discard, so a client that speaks the full + // framing does not stall on a write nobody is reading. + go drainStdin(ctx, c) + } + if err := writeFrame(ctx, c, streamExit, []byte{byte(result.ExitCode)}); err != nil { return } @@ -98,6 +111,36 @@ func (s *Server) execSpriteWS(w http.ResponseWriter, r *http.Request) { _ = c.Close(websocket.StatusNormalClosure, "") } +// echoStdinUntilEOF keeps an unrecognised command's session alive, echoing each +// stdin frame back on stdout, until the client sends StreamStdinEOF or the +// connection ends. +// +// This is the whole of "holding the session open": no process exists, and the +// bytes come straight back. What it buys is that a client which writes after +// opening exec finds something on the other end, which is the difference +// between a turn that completes and one that is orphaned. +func echoStdinUntilEOF(ctx context.Context, c *websocket.Conn) { + for { + _, data, err := c.Read(ctx) + if err != nil { + return + } + if len(data) == 0 { + continue + } + switch data[0] { + case streamStdinEOF: + return + case streamStdin: + if len(data) > 1 { + if err := writeFrame(ctx, c, streamStdout, data[1:]); err != nil { + return + } + } + } + } +} + // reconstructCmd rebuilds the command line from the exec query params. Repeated // cmd params are argv elements joined with spaces; a single cmd param is the // whole command line (joining a one-element slice is a no-op). When no cmd param diff --git a/internal/sprite/exec.go b/internal/sprite/exec.go index 33359e1..bdb0c45 100644 --- a/internal/sprite/exec.go +++ b/internal/sprite/exec.go @@ -23,6 +23,7 @@ var ( func execInto(sp *Sprite, cmd string) ExecResult { var stdout, stderr strings.Builder exitCode := 0 + unrecognised := false for _, raw := range strings.Split(cmd, ";") { seg := strings.TrimSpace(raw) @@ -68,10 +69,16 @@ func execInto(sp *Sprite, cmd string) ExecResult { // execution. stdout.WriteString(seg + "\n") exitCode = 0 + unrecognised = true } } - return ExecResult{Stdout: stdout.String(), Stderr: stderr.String(), ExitCode: exitCode} + return ExecResult{ + Stdout: stdout.String(), + Stderr: stderr.String(), + ExitCode: exitCode, + Unrecognised: unrecognised, + } } // unquote strips a single pair of matching single or double quotes. diff --git a/internal/sprite/sprite.go b/internal/sprite/sprite.go index eca50de..624ba30 100644 --- a/internal/sprite/sprite.go +++ b/internal/sprite/sprite.go @@ -95,6 +95,15 @@ type ExecResult struct { Stdout string Stderr string ExitCode int + // Unrecognised reports that at least one segment fell through to the + // echo-back default rather than matching a scripted verb. + // + // The interpreter answers a known verb and is done. An unknown command is + // a different situation: the caller asked for something this emulator has + // no script for, and a real sprite would have started a process the caller + // can now talk to. The server uses this to decide whether the exec session + // is one-shot or stays open for stdin — see internal/server/ws.go. + Unrecognised bool } // View is the read-only projection returned by GET /v1/sprites/{id}: the