From 03d3ceed6ba402542d56875e92d21cf12def3e58 Mon Sep 17 00:00:00 2001 From: lex00 <121451605+lex00@users.noreply.github.com> Date: Mon, 3 Aug 2026 19:04:03 -0600 Subject: [PATCH] an unrecognised command holds the exec session open for stdin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #18. The interpreter answers a known verb and is done, which is right — echo, cat and rm have said everything they are going to say. 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, so exiting immediately meant that write landed on a process already gone, and its conversation server crashed on a call to a dead pid rather than getting an error back. So an unrecognised command now stays open and echoes stdin back on stdout until StreamStdinEOF or close. Still not execution — the same echo the interpreter already does, extended over time. Deliberately narrow, per the discussion on #18: known verbs keep exiting immediately and every existing client, including chant's Fly activities, sees exactly today's semantics. A test asserts that separately from the new path so the two cannot drift. Verified against a real fountain deployment on k3d. Before, every turn ended reattach interrupted {"reason":"no_active_session","outcome":"turn_orphaned"} Now the turn runs to completion: stage turn started output claude --dangerously-skip-permissions --print --verbose ... output say hello stage turn done {"exit_code":0} The reply is the prompt echoed back, which is what an emulator should produce and what the downstream docs already say a local conversation proves. It is the difference between fountain-ops provisioning a sandbox and fountain-ops holding a conversation offline. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WPvVBaE97cfmvzytEvJCBr --- internal/server/server_test.go | 77 ++++++++++++++++++++++++++++++++++ internal/server/ws.go | 59 ++++++++++++++++++++++---- internal/sprite/exec.go | 9 +++- internal/sprite/sprite.go | 9 ++++ 4 files changed, 145 insertions(+), 9 deletions(-) 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