From b1f68cd590f32672f50c3e5adad70d2882c9f8a0 Mon Sep 17 00:00:00 2001 From: Ahimsa Jenar Bramsaifstyo Kusuma Date: Wed, 4 Nov 2026 14:44:08 +0700 Subject: [PATCH 1/6] Guard Chat delivery and prevent duplicate terminal input on retry Signed-off-by: Ahimsa Jenar Bramsaifstyo Kusuma (cherry picked from commit a30c62f49ae8f64c20f1d15254fe10cd0b1ed0ad) --- client/delivery.go | 11 ++ core/hostcontrol/hostcontrol.go | 1 + core/hostcontrol/hostcontrol_test.go | 2 + core/session/session.go | 31 ++++ daemon/delivery.go | 98 ++++++++++++ daemon/delivery_native_test.go | 120 +++++++++++++++ daemon/delivery_test.go | 217 +++++++++++++++++++++++++++ daemon/rpc.go | 2 +- daemon/rpc_session.go | 3 + daemon/server.go | 1 + docs/chat-view.md | 8 +- docs/protocol.md | 8 + proto/delivery.go | 21 +++ proto/testdata/api.txt | 6 + ui/chat.go | 47 +++--- ui/chatview/attach_test.go | 2 + ui/chatview/chatview.go | 11 +- ui/chatview/delivery_test.go | 141 +++++++++++++++++ ui/chatview/outgoing.go | 85 ++++++++--- ui/chatview/outgoing_test.go | 4 +- 20 files changed, 766 insertions(+), 53 deletions(-) create mode 100644 client/delivery.go create mode 100644 daemon/delivery.go create mode 100644 daemon/delivery_native_test.go create mode 100644 daemon/delivery_test.go create mode 100644 proto/delivery.go create mode 100644 ui/chatview/delivery_test.go diff --git a/client/delivery.go b/client/delivery.go new file mode 100644 index 00000000..e726cfe1 --- /dev/null +++ b/client/delivery.go @@ -0,0 +1,11 @@ +package client + +import "github.com/enowx/enowx/proto" + +// Deliver queues one message once. Keep Key and Run unchanged on retries. +// Success means terminal input was queued, not that the agent recorded a message. +func (c *Client) Deliver(p proto.DeliveryParams) (proto.DeliveryResult, error) { + var result proto.DeliveryResult + err := c.Call(proto.MethodSessionDeliver, p, &result) + return result, err +} diff --git a/core/hostcontrol/hostcontrol.go b/core/hostcontrol/hostcontrol.go index f9925476..37908a71 100644 --- a/core/hostcontrol/hostcontrol.go +++ b/core/hostcontrol/hostcontrol.go @@ -72,6 +72,7 @@ var methods = map[string]Level{ proto.MethodSessionCreate: Change, proto.MethodSessionFanOut: Change, proto.MethodSessionInput: Change, + proto.MethodSessionDeliver: Change, proto.MethodSessionInterrupt: Change, proto.MethodSessionAnswer: Change, proto.MethodSessionMode: Change, diff --git a/core/hostcontrol/hostcontrol_test.go b/core/hostcontrol/hostcontrol_test.go index 1b2d72ee..f2cba912 100644 --- a/core/hostcontrol/hostcontrol_test.go +++ b/core/hostcontrol/hostcontrol_test.go @@ -17,6 +17,8 @@ func TestRolesOnASharedHost(t *testing.T) { {proto.MethodSessionList, api.RoleViewer, true}, {proto.MethodSessionInput, api.RoleViewer, false}, {proto.MethodSessionInput, api.RoleMember, true}, + {proto.MethodSessionDeliver, api.RoleViewer, false}, + {proto.MethodSessionDeliver, api.RoleMember, true}, {proto.MethodSessionKill, api.RoleViewer, false}, {proto.MethodSessionKill, "", true}, {proto.MethodVaultGet, "", false}, diff --git a/core/session/session.go b/core/session/session.go index 29c37a93..3d7155c2 100644 --- a/core/session/session.go +++ b/core/session/session.go @@ -146,6 +146,8 @@ const EnvSession = "ENOWX_SESSION" // Session is one running (or exited) program. type Session struct { + inputMu sync.Mutex // orders queued input, independently of callbacks + inputSeq uint64 // started is when its program started; exitWhy why it ended, when it wasn't // clean; quickExits how many times in a row, over Replace too, it ended // right after starting, the last at lastExit. @@ -641,7 +643,35 @@ func (s *Session) noteScreen(now time.Time, changed bool) { // Input types p into the session and records it (see NoteInput). func (m *Manager) Input(s *Session, p []byte) { + m.input(s, p, nil) +} + +// InputSequence identifies the most recently queued input, independently of echo. +func (s *Session) InputSequence() uint64 { + s.inputMu.Lock() + defer s.inputMu.Unlock() + return s.inputSeq +} + +// InputIfUnchanged queues p only if no other input followed sequence after. +// It does not infer the contents of the program's composer from its screen. +func (m *Manager) InputIfUnchanged(s *Session, p []byte, after uint64) bool { + _, ok := m.input(s, p, &after) + return ok +} + +func (m *Manager) input(s *Session, p []byte, after *uint64) (uint64, bool) { + s.inputMu.Lock() + if after != nil && s.inputSeq != *after { + s.inputMu.Unlock() + return 0, false + } s.term.Send(p) + if len(p) > 0 { + s.inputSeq++ + } + seq := s.inputSeq + s.inputMu.Unlock() prompted := s.NoteInput(p) s.mu.Lock() changed := s.stateChanged(time.Now()) @@ -655,6 +685,7 @@ func (m *Manager) Input(s *Session, p []byte) { if prompted || changed { m.notify(s) } + return seq, true } // SetState records the state the agent's hooks report (proto.State*). diff --git a/daemon/delivery.go b/daemon/delivery.go new file mode 100644 index 00000000..89aca981 --- /dev/null +++ b/daemon/delivery.go @@ -0,0 +1,98 @@ +package daemon + +import ( + "crypto/sha256" + "fmt" + "sync" + "time" + + "github.com/enowx/enowx/core/agent" + "github.com/enowx/enowx/core/session" + "github.com/enowx/enowx/proto" +) + +// The ledger lives for the program run. Entries are never evicted within a run: +// a delayed retry must not become a new paste. No text is kept in the ledger. +type deliveryState struct { + mu sync.Mutex + runs map[*session.Session]*deliveryRun +} + +type deliveryRun struct { + sending bool + entries map[string]*deliveryAttempt +} + +type deliveryAttempt struct { + text [32]byte + done chan struct{} // publishes result to concurrent/reconnected retries + result proto.DeliveryResult +} + +const maxDeliveriesPerRun = 4096 + +func (s *Server) deliver(p proto.DeliveryParams) (proto.DeliveryResult, error) { + sess, err := s.sessions.Get(p.ID) + if err != nil { + return proto.DeliveryResult{}, err + } + info := sess.Info() + if p.Run == 0 || p.Run != info.Run || info.Exited || info.Queued { + return proto.DeliveryResult{}, fmt.Errorf("the session's program changed or isn't running; check its terminal") + } + if p.Key == "" || len(p.Key) > 128 || p.Text == "" { + return proto.DeliveryResult{}, fmt.Errorf("delivery needs a key and nonempty text") + } + seq := sess.InputSequence() + screen := sess.Terminal().Text() + ready := agent.PromptEmpty(screen) && agent.PromptFree(screen) + d := &s.deliveries + d.mu.Lock() + if d.runs == nil { + d.runs = make(map[*session.Session]*deliveryRun) + } + for old := range d.runs { + if current, err := s.sessions.Get(old.ID()); err != nil || current != old { + delete(d.runs, old) + } + } + r := d.runs[sess] + if r == nil { + r = &deliveryRun{entries: make(map[string]*deliveryAttempt)} + d.runs[sess] = r + } + hash := sha256.Sum256([]byte(p.Text)) + if a := r.entries[p.Key]; a != nil { + d.mu.Unlock() + if a.text != hash { + return proto.DeliveryResult{}, fmt.Errorf("delivery key was already used for different text") + } + <-a.done + return a.result, nil + } + if r.sending || len(r.entries) >= maxDeliveriesPerRun || working(info) || info.State == proto.StateWaiting || !ready { + d.mu.Unlock() + return proto.DeliveryResult{}, fmt.Errorf("the session isn't ready for a new chat delivery; check its terminal") + } + a := &deliveryAttempt{text: hash, done: make(chan struct{})} + r.entries[p.Key], r.sending = a, true + d.mu.Unlock() + + if !s.sessions.InputIfUnchanged(sess, []byte(pasteStart+p.Text+pasteEnd), seq) { + a.result.Interrupted = true + } else { + timer := time.NewTimer(pasteSettle) + defer timer.Stop() + select { + case <-sess.Done(): + a.result.Interrupted = true + case <-timer.C: + a.result.Interrupted = !s.sessions.InputIfUnchanged(sess, []byte("\r"), seq+1) + } + } + d.mu.Lock() + r.sending = false + close(a.done) + d.mu.Unlock() + return a.result, nil +} diff --git a/daemon/delivery_native_test.go b/daemon/delivery_native_test.go new file mode 100644 index 00000000..09dcb43a --- /dev/null +++ b/daemon/delivery_native_test.go @@ -0,0 +1,120 @@ +package daemon + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/enowx/enowx/proto" +) + +// Opt in with an installed Codex executable. No credentials are used: its only +// provider is a closed localhost port, and all of its files live in the fixture. +func TestDeliveryNativeCodex(t *testing.T) { + bin := os.Getenv("ENOWX_TEST_CODEX") + if bin == "" { + t.Skip("set ENOWX_TEST_CODEX for the isolated native regression") + } + for _, mode := range []string{"once", "draft", "menu"} { + t.Run(mode, func(t *testing.T) { + root := t.TempDir() + home := filepath.Join(root, "home") + os.MkdirAll(home, 0700) + codexHome := filepath.Join(root, "codex") + os.MkdirAll(codexHome, 0700) + srv := startServer(t) + c := connect(t, srv, nil) + info, err := c.CreateSession(proto.SessionCreateParams{Dir: home, Cols: 110, Rows: 32, Env: []string{"HOME=" + home, "USERPROFILE=" + home, "ENOWX_HOME=" + filepath.Join(root, "enowx"), "CODEX_HOME=" + codexHome, "LOCALAPPDATA=" + filepath.Join(home, "AppData", "Local"), "APPDATA=" + filepath.Join(home, "AppData", "Roaming")}, Command: []string{bin, "--no-daemon", "--no-alt-screen", "--dangerously-bypass-approvals-and-sandbox", "-c", `model_provider="fixture"`, "-c", `model="fixture"`, "-c", `model_providers.fixture={name="fixture",base_url="http://127.0.0.1:1/v1",wire_api="responses",requires_openai_auth=false,request_max_retries=0,stream_max_retries=0}`}}) + if err != nil { + t.Fatal(err) + } + sess, _ := srv.sessions.Get(info.ID) + t.Cleanup(func() { + if t.Failed() { + t.Logf("native screen: %s", sess.Terminal().Text()) + } + }) + waitDelivery(t, func() bool { return strings.Contains(sess.Terminal().Text(), "Ask Codex to do anything") }) + // Avoid mistaking initial drawing for a working agent. + waitDelivery(t, func() bool { return !sess.Info().Active }) + p := proto.DeliveryParams{ID: info.ID, Run: info.Run, Key: "native", Text: "NATIVE_DELIVERY_ONCE"} + if mode != "once" { + text := "MANUAL_DRAFT" + if mode == "menu" { + text = "/permissions" + } + srv.sessions.Input(sess, []byte(text)) + if mode == "menu" { + time.Sleep(300 * time.Millisecond) + srv.sessions.Input(sess, []byte("\r")) + } + waitDelivery(t, func() bool { + screen := sess.Terminal().Text() + if mode == "draft" { + return strings.Contains(screen, text) + } + return strings.Contains(screen, "Full Access") || strings.Contains(screen, "Full access") || strings.Contains(screen, "Read Only") || strings.Contains(screen, "Read-only") + }) + t.Logf("%s screen: %s", mode, sess.Terminal().Text()) + if _, err := c.Deliver(p); err == nil { + t.Fatal("typed into a manual draft or menu") + } + if len(nativeUserMessages(codexHome)) != 0 || strings.Contains(sess.Terminal().Text(), p.Text) { + t.Fatal("touched the draft/menu") + } + return + } + if r, err := c.Deliver(p); err != nil || r.Interrupted { + t.Fatalf("native send: %+v %v", r, err) + } + for range 5 { + if _, err := c.Deliver(p); err != nil { + t.Fatal(err) + } + } + waitDelivery(t, func() bool { return len(nativeUserMessages(codexHome)) > 0 }) + if got := nativeUserMessages(codexHome); len(got) != 1 || got[0] != p.Text { + t.Fatalf("recorded user messages: %q", got) + } + t.Logf("Codex recorded exactly one full UserMessage after delivery and five retries: %q", nativeUserMessages(codexHome)) + p.Key, p.Text = "busy", "BUSY_NOT_PASTED" + if _, err := c.Deliver(p); err == nil { + t.Fatal("pasted into the working native agent") + } + }) + } +} + +func nativeUserMessages(home string) []string { + var out []string + filepath.WalkDir(filepath.Join(home, "sessions"), func(path string, d os.DirEntry, err error) error { + if err != nil || d.IsDir() { + return nil + } + b, _ := os.ReadFile(path) + for _, line := range strings.Split(string(b), "\n") { + var r struct { + Type string + Payload struct { + Type string + Item struct { + Type string + Content []struct{ Text string } + } + } + } + if json.Unmarshal([]byte(line), &r) == nil && r.Type == "event_msg" && r.Payload.Type == "item_completed" && r.Payload.Item.Type == "UserMessage" { + var text strings.Builder + for _, c := range r.Payload.Item.Content { + text.WriteString(c.Text) + } + out = append(out, text.String()) + } + } + return nil + }) + return out +} diff --git a/daemon/delivery_test.go b/daemon/delivery_test.go new file mode 100644 index 00000000..95b60bef --- /dev/null +++ b/daemon/delivery_test.go @@ -0,0 +1,217 @@ +package daemon + +import ( + "bufio" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/enowx/enowx/client" + "github.com/enowx/enowx/core/session" + "github.com/enowx/enowx/proto" +) + +// This child process records lines read from its own PTY, rather than treating +// screen echo as evidence of receipt. Its parent closes only this fixture. +func TestDeliveryInputCapture(t *testing.T) { + path := os.Getenv("ENOWX_DELIVERY_CAPTURE") + if path == "" { + t.Skip("PTY helper") + } + f, err := os.Create(path) + if err != nil { + t.Fatal(err) + } + defer f.Close() + f.WriteString("ready\n") + fmt.Print("› ") + reader := bufio.NewReader(os.Stdin) + for { + line, err := reader.ReadString('\n') + if err != nil { + return + } + b, _ := json.Marshal(line) + fmt.Fprintln(f, string(b)) + } +} + +func deliveryFixture(t *testing.T) (*Server, *client.Client, proto.SessionInfo, string) { + t.Helper() + srv := startServer(t) + c := connect(t, srv, nil) + bin, err := os.Executable() + if err != nil { + t.Fatal(err) + } + path := filepath.Join(t.TempDir(), "received.jsonl") + info, err := c.CreateSession(proto.SessionCreateParams{Command: []string{bin, "-test.run=^TestDeliveryInputCapture$"}, Env: []string{"ENOWX_DELIVERY_CAPTURE=" + path}, Cols: 100, Rows: 24}) + if err != nil { + t.Fatal(err) + } + waitDelivery(t, func() bool { b, _ := os.ReadFile(path); return strings.HasPrefix(string(b), "ready\n") }) + sess, _ := srv.sessions.Get(info.ID) + srv.sessions.SetState(sess, proto.StateDone) + waitDelivery(t, func() bool { return strings.Contains(sess.Terminal().Text(), "›") }) + return srv, c, info, path +} + +func waitDelivery(t *testing.T, f func() bool) { + t.Helper() + for end := time.Now().Add(10 * time.Second); time.Now().Before(end); time.Sleep(10 * time.Millisecond) { + if f() { + return + } + } + t.Fatal("delivery fixture timed out") +} + +func capturedLines(path string) []string { + b, _ := os.ReadFile(path) + var lines []string + for _, row := range strings.Split(string(b), "\n") { + var line string + if json.Unmarshal([]byte(row), &line) == nil { + lines = append(lines, line) + } + } + return lines +} + +func TestDeliveryRetriesQueueInputOnce(t *testing.T) { + srv, c, info, path := deliveryFixture(t) + p := proto.DeliveryParams{ID: info.ID, Run: info.Run, Key: "once", Text: "a long\npasted message"} + // Separate connections retry even while the first request is settling. + var wg sync.WaitGroup + for range 4 { + other := connect(t, srv, nil) + wg.Add(1) + go func() { + defer wg.Done() + result, err := other.Deliver(p) + if err != nil || result.Interrupted { + t.Errorf("delivery: %+v %v", result, err) + } + }() + } + wg.Wait() + for range 4 { + if _, err := c.Deliver(p); err != nil { + t.Fatal(err) + } + } + waitDelivery(t, func() bool { return strings.Contains(strings.Join(capturedLines(path), ""), "pasted message") }) + // The multiline paste may be read as several lines by this deliberately + // simple helper; its text must occur once in the process's record. + lines := strings.Join(capturedLines(path), "") + if strings.Count(lines, "pasted message") != 1 { + t.Fatalf("input was repeated: %q", lines) + } + p.Text = "different text" + if _, err := c.Deliver(p); err == nil { + t.Fatal("reused a key for different text") + } +} + +func TestDeliveryManualInputCancelsEnterAndRetry(t *testing.T) { + srv, c, info, path := deliveryFixture(t) + p := proto.DeliveryParams{ID: info.ID, Run: info.Run, Key: "manual", Text: "APP_DRAFT"} + st, err := c.Attach(info.ID) + if err != nil { + t.Fatal(err) + } + defer st.Close() + result := make(chan proto.DeliveryResult, 1) + go func() { + r, e := c.Deliver(p) + if e != nil { + t.Error(e) + } + result <- r + }() + sess, _ := srv.sessions.Get(info.ID) + // Echo is used only to coordinate the intervention, never as an ack. + waitDelivery(t, func() bool { return strings.Contains(sess.Terminal().Text(), "APP_DRAFT") }) + if _, err := st.Write([]byte("MANUAL")); err != nil { + t.Fatal(err) + } + waitDelivery(t, func() bool { return strings.Contains(sess.Terminal().Text(), "MANUAL") }) + if r := <-result; !r.Interrupted { + t.Fatal("pressed Enter after manual input") + } + for range 4 { + r, e := c.Deliver(p) + if e != nil || !r.Interrupted { + t.Fatalf("retry: %+v %v", r, e) + } + } + if got := capturedLines(path); len(got) != 0 { + t.Fatalf("submitted a user's draft: %q", got) + } + // Only a deliberate manual Enter may submit that now-mixed draft. + srv.sessions.Input(sess, []byte("\r")) + waitDelivery(t, func() bool { return len(capturedLines(path)) > 0 }) + if got := strings.Join(capturedLines(path), ""); strings.Count(got, "APP_DRAFT") != 1 || !strings.Contains(got, "MANUAL") { + t.Fatalf("draft changed: %q", got) + } +} + +func TestDeliveryLostResponseDoesNotReplay(t *testing.T) { + srv, c, info, path := deliveryFixture(t) + p := proto.DeliveryParams{ID: info.ID, Run: info.Run, Key: "lost-response", Text: "LOST_RESPONSE_ONCE"} + done := make(chan error, 1) + go func() { _, err := c.Deliver(p); done <- err }() + sess, _ := srv.sessions.Get(info.ID) + waitDelivery(t, func() bool { + srv.deliveries.mu.Lock() + defer srv.deliveries.mu.Unlock() + r := srv.deliveries.runs[sess] + return r != nil && r.entries[p.Key] != nil + }) + c.Close() // the accepted request finishes independently of its response + if err := <-done; err == nil { + t.Fatal("fixture didn't lose the response") + } + other := connect(t, srv, nil) + if r, err := other.Deliver(p); err != nil || r.Interrupted { + t.Fatalf("reconnected retry: %+v %v", r, err) + } + waitDelivery(t, func() bool { return strings.Contains(strings.Join(capturedLines(path), ""), p.Text) }) + if got := strings.Join(capturedLines(path), ""); strings.Count(got, p.Text) != 1 { + t.Fatalf("replayed after losing response: %q", got) + } +} + +func TestDeliveryRefusesBusyWaitingAndDifferentRun(t *testing.T) { + srv, c, info, path := deliveryFixture(t) + sess, _ := srv.sessions.Get(info.ID) + p := proto.DeliveryParams{ID: info.ID, Run: info.Run, Key: "held", Text: "never pasted"} + for _, state := range []string{proto.StateWorking, proto.StateWaiting} { + srv.sessions.SetState(sess, state) + if _, err := c.Deliver(p); err == nil { + t.Fatal("pasted into busy/menu terminal", state) + } + } + srv.sessions.SetState(sess, proto.StateDone) + p.Run++ + if _, err := c.Deliver(p); err == nil { + t.Fatal("accepted a different run") + } + if got := capturedLines(path); len(got) != 0 { + t.Fatal(got) + } + // Replacing the program invalidates even the original token/run pair. + _, err := srv.sessions.Replace(session.Spec{ID: info.ID, Exited: true}) + if err != nil { + t.Fatal(err) + } + p.Run = info.Run + if _, err := c.Deliver(p); err == nil { + t.Fatal("replayed into a replacement program") + } +} diff --git a/daemon/rpc.go b/daemon/rpc.go index dae137a5..e2bc532e 100644 --- a/daemon/rpc.go +++ b/daemon/rpc.go @@ -70,7 +70,7 @@ var handlers = merge( var slowMethods = slow(methodsOf(reviewHandlers, tokenHandlers, cloudHandlers, libraryHandlers, libraryReuseHandlers, sessionPeekHandlers, visitorHandlers, teamChatHandlers, fileHandlers, hibernateHandlers, noteIOHandlers, projectSyncHandlers, trafficHandlers, androidHandlers, agentAPIHandlers), proto.MethodSessionPorts, proto.MethodSessionDisplay, proto.MethodSessionDisplayStop, proto.MethodTunnelOpen, proto.MethodTransferPack, proto.MethodTransferRead, proto.MethodTransferWrite, proto.MethodTransferUnpack, proto.MethodSetupScan, proto.MethodEnvMCPTest, proto.MethodEnvSkillFetch, proto.MethodEnvSkillInstall, proto.MethodEnvPluginRun, proto.MethodChecksList, proto.MethodChecksRun, proto.MethodChecksSet, - proto.MethodSessionRewind, proto.MethodAgentHistory, proto.MethodAutomationRun, + proto.MethodSessionRewind, proto.MethodSessionDeliver, proto.MethodAgentHistory, proto.MethodAutomationRun, proto.MethodSessionFanOut, proto.MethodBrowserAct, proto.MethodFeaturesRequest, proto.MethodNoteChat, proto.MethodNoteChatLog, proto.MethodCanvasRender, proto.MethodProjectCopyRequest, proto.MethodProjectSyncRequest, proto.MethodAutopushNow, proto.MethodAutopushSet, proto.MethodProjectKeepCopy, proto.MethodProjectFileRead, proto.MethodSessionCreate, proto.MethodSessionContinue, proto.MethodSessionFork, proto.MethodSessionMode, diff --git a/daemon/rpc_session.go b/daemon/rpc_session.go index 4f077ce1..ee4ef010 100644 --- a/daemon/rpc_session.go +++ b/daemon/rpc_session.go @@ -8,6 +8,9 @@ import ( ) var sessionHandlers = map[string]handler{ + proto.MethodSessionDeliver: with(func(c *conn, p proto.DeliveryParams) (any, error) { + return c.srv.deliver(p) + }), proto.MethodSessionCreate: with(func(c *conn, p proto.SessionCreateParams) (any, error) { sess, err := c.srv.createSession(p) if err != nil { diff --git a/daemon/server.go b/daemon/server.go index c2eb775b..05cbda9b 100644 --- a/daemon/server.go +++ b/daemon/server.go @@ -113,6 +113,7 @@ type Server struct { vault vaultState // apps lending credentials, and what they lent (vault.go) transcripts transcripts + deliveries deliveryState // controlAsks are the sessions waiting for the user's yes in the app, and // the method it's for (sessionAwaitsUser). controlAsks sync.Map diff --git a/docs/chat-view.md b/docs/chat-view.md index d5458428..cbbd1d18 100644 --- a/docs/chat-view.md +++ b/docs/chat-view.md @@ -26,7 +26,13 @@ Each message of the user's starts a turn. The agent's thoughts, tool calls and w Beside a wide conversation, **Your messages** lists the user's messages, the one read now lit, each a click away. It stays at the newest as messages come; scrolled up, it stays where it is left and new messages don't move it, until it is scrolled back to its end. The lit row doesn't move it. Each session keeps its own place, starting at the newest. -A message sent shows in the same frame, the composer emptied, without waiting for the host or the agent's file; the entry the agent records takes its row, with nothing doubled. While the agent is busy it says it waits for the agent. If it couldn't reach the session, or stays unrecorded for 45 seconds with the agent free, it says so and offers Retry and Edit, which puts it back in the composer with its files. A command (`/model`) shows no bubble: agents don't record those as messages. +A message sent shows immediately, with the composer emptied. Messages waiting for a busy agent, a permission question, or an earlier unconfirmed message stay **Queued in Chat**: their text has not been pasted into the terminal. The daemon accepts a new delivery only when the screen shows a free, empty composer. An existing draft or menu must be handled in the terminal. + +Chat uses `session.deliver`, with one key for the message and the program's run. The daemon queues its bracketed paste and a delayed Enter once; retries with that same key return the original result, including after a lost RPC response. Input typed into the terminal during the paste delay cancels the automatic Enter. It never clears a draft or presses Enter again to recover a delivery. A program restart rejects an old run's delivery. Older daemons ask the user to reconnect for an update; there is no fallback to unprotected input. The existing `session.input` RPC and terminal streams keep their behavior for older apps and the CLI. + +Transport success and screen echo are not acknowledgement. Only a newly recorded user entry matching the whole message and its attachment paths replaces its pending row. A message unrecorded for 45 seconds with the agent free is **unconfirmed**, not proven lost. A successful or interrupted terminal send offers **Show terminal**, without repasting or restoring an uncertain message to the Chat composer. **Retry** after a transport error uses the same key. **Stop tracking** removes only the pending row after the user has reviewed the terminal. A changed conversation never acknowledges a previous conversation's pending messages. Transcript formats that rewrite text or attachment paths may require manual review rather than automatic matching. + +A command (`/model`) shows no bubble: agents don't record those as messages. Slash commands and settings still use their existing terminal input path. The outline of the user's messages stands at the conversation's side when the column leaves room for it. A column running the frame's width (GOD of Agents') gives up that side; otherwise the outline folds to a button in the chat's corner. diff --git a/docs/protocol.md b/docs/protocol.md index 7400d748..9a00e044 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -55,4 +55,12 @@ Adding to the API means recording it: `go test ./proto -run API -update`, and ch ## Methods +### Chat delivery + +`session.deliver` takes `DeliveryParams` (`id`, `run`, `key`, `text`) and returns `DeliveryResult`. A key identifies one logical message within one live program run. Replaying it with the same text returns its original result without additional input; changing its text is refused. The daemon retains keys until the run is replaced or removed, with a limit of 4096 per run; reaching the limit refuses new keys rather than evicting replay protection. A daemon restart changes the run, so an old request cannot become a new paste. This ledger is not a durable message queue. + +The result describes terminal input queued, not a recorded user message. `interrupted` says other input or a replaced run prevented completing the paste/Enter sequence. A caller must not recover by pasting again or sending a bare Enter: ownership of the remaining draft is unknown. The request runs beside the connection reader so manual terminal input can cancel its Enter on the same connection. New apps receiving `unknown method` request a daemon update without falling back to `session.input`; old apps continue to use the unchanged methods. + +The Chat client distinguishes its own unsent queue, an uncertain terminal send, and a user entry recorded in the transcript. Only the latter is acknowledgement. This does not add a receipt ID to an agent's own transcript: matching uses the complete text and attachment paths, and a transcript reset invalidates pending matches. + The methods, by domain, are in `proto/*.go` (each file's constants say what they take and return) and are handled in `daemon/rpc_*.go`; `TestEveryMethodHasHandler` checks every one has a handler. `testdata/api.txt` lists them all. diff --git a/proto/delivery.go b/proto/delivery.go new file mode 100644 index 00000000..17e620db --- /dev/null +++ b/proto/delivery.go @@ -0,0 +1,21 @@ +package proto + +// MethodSessionDeliver queues one Chat message in a particular program run. +// Unlike session.input, replaying the same key never queues its text or Enter +// again. Older daemons refuse the method; callers must not fall back to input. +const MethodSessionDeliver = "session.deliver" + +// DeliveryParams identify one logical send, including across RPC retries. +type DeliveryParams struct { + ID string `json:"id"` + Run int64 `json:"run"` + Key string `json:"key"` + Text string `json:"text"` +} + +// DeliveryResult describes queued terminal input, never agent acknowledgement. +// Interrupted means other input or a run change prevented the delayed Enter. +// Neither state authorizes pasting or pressing Enter again: inspect the terminal. +type DeliveryResult struct { + Interrupted bool `json:"interrupted,omitempty"` +} diff --git a/proto/testdata/api.txt b/proto/testdata/api.txt index 9a10509d..c9880caa 100644 --- a/proto/testdata/api.txt +++ b/proto/testdata/api.txt @@ -950,6 +950,11 @@ field Decision.use string field Decision.verdict string field Decision.why string field Decision.wrong bool +field DeliveryParams.id string +field DeliveryParams.key string +field DeliveryParams.run int64 +field DeliveryParams.text string +field DeliveryResult.interrupted bool field DesignBrief.firstScreen string field DesignBrief.fixed string field DesignBrief.peers string @@ -3354,6 +3359,7 @@ method session.clear method session.commands method session.continue method session.create +method session.deliver method session.detach method session.display method session.displayStop diff --git a/ui/chat.go b/ui/chat.go index c6f04cb5..4498d97f 100644 --- a/ui/chat.go +++ b/ui/chat.go @@ -90,9 +90,11 @@ func (a *App) chatViewSending(c *ui.Context, s proto.SessionInfo, shown func() b if send == nil { send = func(text string, files []string) { a.sendWithFiles(s.ID, text, files) } } - var deliver func(string, []string, func(error)) + var deliver func(string, string, []string, func(error)) if tw.send == nil { - deliver = func(text string, files []string, done func(error)) { a.deliverToSession(s.ID, text, files, done) } + deliver = func(key, text string, files []string, done func(error)) { + a.deliverToSession(s, key, text, files, done) + } } spent, spentTip := "", "" if t, ok := a.sessionTokens(s.ID); ok { @@ -121,6 +123,7 @@ func (a *App) chatViewSending(c *ui.Context, s proto.SessionInfo, shown func() b Working: isWorking(s), Send: send, Deliver: deliver, + Run: s.Run, Interrupt: func() { a.interruptSession(s) }, Set: func(setting, id string) { a.setAgentSetting(s, setting, id) }, Paste: func(text string) bool { @@ -256,7 +259,12 @@ func (a *App) sendWithFiles(id, text string, files []string) { } // deliverToSession is sendWithFiles saying on the main thread how it went. -func (a *App) deliverToSession(id, text string, files []string, done func(error)) { +func (a *App) deliverToSession(s proto.SessionInfo, key, text string, files []string, done func(error)) { + cl := a.client + if cl == nil { + done(errors.New("not connected to the host")) + return + } msg := text if len(files) > 0 { msg = strings.Join(files, " ") @@ -264,28 +272,17 @@ func (a *App) deliverToSession(id, text string, files []string, done func(error) msg = text + " " + msg } } - paste := len(files) > 0 || strings.Contains(msg, "\n") - p := a.panes[id] - if p == nil { - a.inputThroughDaemon(id, msg, paste, done) - return - } - if paste { - msg = "\x1b[200~" + msg + "\x1b[201~" - } - a.withPane(id, func(p *pane) { - go func() { - _, err := p.stream.Write([]byte(msg)) - if err == nil { - time.Sleep(50 * time.Millisecond) // as writeThenEnter - _, err = p.stream.Write([]byte("\r")) - } - if err != nil { - err = fmt.Errorf("couldn't reach the session: %s", client.Explain(err)) - } - a.onMain(func() { done(err) }) - }() - }, func() { done(errNotAttached) }) + go func() { + result, err := cl.Deliver(proto.DeliveryParams{ID: s.ID, Run: s.Run, Key: key, Text: msg}) + if client.IsUnknownMethod(err) { + err = errors.New("reconnect to update this host before sending chat messages") + } else if err != nil { + err = fmt.Errorf("couldn't reach the session: %s", client.Explain(err)) + } else if result.Interrupted { + err = chatview.ErrDeliveryInterrupted + } + a.onMain(func() { done(err) }) + }() } // inputThroughDaemon types msg into session id, then Enter, for a session whose diff --git a/ui/chatview/attach_test.go b/ui/chatview/attach_test.go index 145e47d0..8bde046d 100644 --- a/ui/chatview/attach_test.go +++ b/ui/chatview/attach_test.go @@ -2,6 +2,7 @@ package chatview import ( "errors" + "github.com/enowx/enowx/proto" "slices" "testing" ) @@ -27,6 +28,7 @@ func TestMessageGoesWithItsAttachments(t *testing.T) { t.Fatal("attachments kept after sending") } // An image alone is a message too. + ch.apply(proto.Transcript{Supported: true, Rev: 1, Entries: []proto.TranscriptEntry{userEntry(0, "look at this /srv/shot.png")}}) ch.Attach("only.png", nil)("/srv/only.png", nil) if !ch.send(o, "") || !slices.Equal(files, []string{"/srv/only.png"}) { t.Fatalf("image alone: %q", files) diff --git a/ui/chatview/chatview.go b/ui/chatview/chatview.go index 216efba1..8002a643 100644 --- a/ui/chatview/chatview.go +++ b/ui/chatview/chatview.go @@ -283,10 +283,13 @@ func (ch *Chat) apply(t proto.Transcript) { ch.loaded, ch.supported, ch.reason = true, t.Supported, t.Reason if t.Reset { ch.entries, ch.base, ch.open = nil, 0, map[int]bool{} - first = true for _, m := range ch.out { - m.after = 0 + if !first { + m.changed = true + m.failed = "The conversation changed. Check the terminal." + } } + first = true } if len(ch.entries) == 0 && len(t.Entries) > 0 { ch.base = t.Entries[0].Index @@ -382,7 +385,9 @@ type Options struct { Send func(text string, files []string) // Deliver, when set, is Send that says on the main thread how it went, so that // a message that couldn't go offers Retry. - Deliver func(text string, files []string, done func(error)) + // Repeated calls with the same key must never repeat terminal input. + Deliver func(key, text string, files []string, done func(error)) + Run int64 // the program run; pending messages never move to another run Interrupt func() // Set changes a setting ("model", "mode" or "effort") to the agent's // value id. diff --git a/ui/chatview/delivery_test.go b/ui/chatview/delivery_test.go new file mode 100644 index 00000000..78647f37 --- /dev/null +++ b/ui/chatview/delivery_test.go @@ -0,0 +1,141 @@ +package chatview + +import ( + "errors" + "strings" + "testing" + "time" + + "github.com/enowx/enowx/proto" +) + +func TestDeliveryRetryKeepsKeyAndWaitsForAcknowledgement(t *testing.T) { + ch := New() + ch.apply(proto.Transcript{Supported: true, Rev: 1}) + var keys []string + var done func(error) + o := Options{Run: 7, Deliver: func(key, text string, files []string, d func(error)) { keys = append(keys, key); done = d }} + ch.outgoingSend(o, "hello", nil) + m := ch.out[0] + ch.outgoingGo(o, m) // clicks while the request is in flight + if len(keys) != 1 { + t.Fatal(keys) + } + done(errors.New("response lost")) + ch.outgoingGo(o, m) + if len(keys) != 2 || keys[0] == "" || keys[0] != keys[1] { + t.Fatal(keys) + } + done(nil) + if len(ch.out) != 1 { + t.Fatal("transport success acknowledged a user message") + } + ch.outgoingLook(o, time.Now().Add(2*outgoingLost)) + for range 5 { + ch.outgoingGo(o, m) + } + if len(keys) != 2 { + t.Fatal("unconfirmed terminal input was sent again", keys) + } + ch.apply(proto.Transcript{Supported: true, Rev: 1, Entries: []proto.TranscriptEntry{userEntry(0, "hello")}}) + if len(ch.out) != 0 { + t.Fatal("late acknowledgement was lost") + } +} + +func TestDeliveryQueuesWithoutTouchingBusyOrWaitingTerminal(t *testing.T) { + for _, busy := range []Options{{Working: true}, {Waiting: true}} { + ch := New() + sent := 0 + busy.Send = func(string, []string) { sent++ } + ch.outgoingSend(busy, "one", nil) + ch.outgoingSend(busy, "two", nil) + ch.outgoingLook(busy, time.Now().Add(10*outgoingLost)) + if sent != 0 || ch.out[0].failed != "" { + t.Fatal("touched a busy/menu terminal") + } + free := busy + free.Working, free.Waiting = false, false + ch.outgoingLook(free, time.Now()) + if sent != 1 { + t.Fatal("didn't send the oldest once", sent) + } + ch.apply(proto.Transcript{Supported: true, Rev: 1, Entries: []proto.TranscriptEntry{userEntry(0, "one")}}) + ch.outgoingLook(free, time.Now()) + if sent != 2 { + t.Fatal("didn't release the next message after acknowledgement", sent) + } + } +} + +func TestDeliveryAcknowledgementRequiresWholeTextAndFiles(t *testing.T) { + for _, text := range []string{strings.Repeat("a", 250) + " end", ""} { + ch := New() + o := Options{Send: func(string, []string) {}} + ch.outgoingSend(o, text, []string{"/fixture/image.png"}) + ch.apply(proto.Transcript{Supported: true, Rev: 1, Entries: []proto.TranscriptEntry{userEntry(0, strings.Repeat("a", 250)+" other")}}) + if len(ch.out) != 1 { + t.Fatal("unrelated user message acknowledged the send") + } + ch.apply(proto.Transcript{Supported: true, Rev: 2, Entries: []proto.TranscriptEntry{userEntry(1, ch.out[0].shown())}}) + if len(ch.out) != 0 { + t.Fatal("whole message didn't acknowledge the send") + } + } +} + +func TestDeliveryDoesNotMoveAcrossRunsOrConversations(t *testing.T) { + for _, reset := range []bool{false, true} { + ch := New() + calls := 0 + o := Options{Run: 1, Deliver: func(_, _ string, _ []string, done func(error)) { calls++; done(errors.New("uncertain")) }} + ch.apply(proto.Transcript{Supported: true, Rev: 1}) + ch.outgoingSend(o, "hello", nil) + m := ch.out[0] + if reset { + ch.apply(proto.Transcript{Supported: true, Reset: true, Rev: 2, Entries: []proto.TranscriptEntry{userEntry(0, "hello")}}) + } else { + o.Run = 2 + ch.outgoingLook(o, time.Now()) + } + ch.outgoingGo(o, m) + if len(ch.out) != 1 || !m.changed || calls != 1 { + t.Fatalf("reused a delivery across identity change: %+v, calls %d", m, calls) + } + } +} + +func TestInterruptedDeliveryRequiresTerminalReview(t *testing.T) { + ch := New() + ch.apply(proto.Transcript{Supported: true, Rev: 1}) + calls := 0 + o := Options{Deliver: func(_, _ string, _ []string, done func(error)) { calls++; done(ErrDeliveryInterrupted) }} + ch.outgoingSend(o, "hello", nil) + m := ch.out[0] + for range 5 { + ch.outgoingGo(o, m) + } + if calls != 1 || !m.changed || m.failed == "" { + t.Fatal("interrupted input was replayed") + } +} + +func TestDeliveryWaitsForTranscriptBaseline(t *testing.T) { + ch := New() + calls := 0 + o := Options{Deliver: func(_, _ string, _ []string, done func(error)) { calls++; done(nil) }} + ch.outgoingSend(o, "same", nil) + if calls != 0 { + t.Fatal("sent before reading the acknowledgement baseline") + } + ch.apply(proto.Transcript{Supported: true, Rev: 1, Reset: true, Entries: []proto.TranscriptEntry{userEntry(0, "same")}}) + ch.outgoingLook(o, time.Now()) + ch.outgoingSettle() + if calls != 1 || len(ch.out) != 1 { + t.Fatal("an existing message acknowledged the new send") + } + ch.apply(proto.Transcript{Supported: true, Rev: 2, Entries: []proto.TranscriptEntry{userEntry(1, "same")}}) + if len(ch.out) != 0 { + t.Fatal("new acknowledgement didn't settle") + } +} diff --git a/ui/chatview/outgoing.go b/ui/chatview/outgoing.go index 1c83a235..94ecc81d 100644 --- a/ui/chatview/outgoing.go +++ b/ui/chatview/outgoing.go @@ -1,6 +1,8 @@ package chatview import ( + "crypto/rand" + "errors" "path/filepath" "strings" "time" @@ -21,13 +23,20 @@ import ( // it before it counts as not taken. const outgoingLost = 45 * time.Second +// ErrDeliveryInterrupted means text may be a draft mixed with terminal input. +// Only the user can decide what to submit or remove in that terminal. +var ErrDeliveryInterrupted = errors.New("Terminal input changed during delivery. Check the terminal.") + // outgoing is a message on its way. It belongs to the main thread. type outgoing struct { - text string - files []string - at time.Time // sent, or last seen with the agent busy - after int // an entry from this index on may be it - failed string // why it didn't go, "" while on its way + key string // stable across transport retries + run int64 + started, inFlight, sent, changed bool + text string + files []string + at time.Time // sent, or last seen with the agent busy + after int // an entry from this index on may be it + failed string // why it didn't go, "" while on its way } // shown is the message as the user's entry will read: its files' paths after its text. @@ -50,19 +59,41 @@ func (ch *Chat) outgoingSend(o Options, text string, files []string) { o.Send(text, files) return } - m := &outgoing{text: text, files: files, after: ch.base + len(ch.entries)} + m := &outgoing{key: rand.Text(), run: o.Run, text: text, files: files, at: time.Now(), after: ch.base + len(ch.entries)} ch.out = append(ch.out, m) - ch.outgoingGo(o, m) + if len(ch.out) == 1 && !o.Working && !o.Waiting && (ch.loaded || o.Deliver == nil) { + ch.outgoingGo(o, m) + } } // outgoingGo sends m, or sends it again. func (ch *Chat) outgoingGo(o Options, m *outgoing) { + if m.inFlight || m.sent || m.changed { + return + } + if o.Run != m.run { + m.changed, m.failed = true, "The session restarted. Check the terminal." + return + } + if !m.started { + m.after = ch.base + len(ch.entries) + } m.failed, m.at = "", time.Now() + m.started, m.inFlight = true, true if o.Deliver == nil { + m.sent, m.inFlight = true, false o.Send(m.text, m.files) return } - o.Deliver(m.text, m.files, func(err error) { + o.Deliver(m.key, m.text, m.files, func(err error) { + m.inFlight = false + if m.changed { + return + } + m.sent = err == nil || errors.Is(err, ErrDeliveryInterrupted) + if errors.Is(err, ErrDeliveryInterrupted) { + m.changed = true + } if err != nil { m.failed = err.Error() } @@ -106,7 +137,7 @@ func (ch *Chat) outgoingSettle() { text = ch.showUser(text) } for j, m := range ch.out { - if e.Index < m.after || !sameMessage(text, m.text) { + if !m.started || m.changed || e.Index < m.after || !sameMessage(text, m.shown()) { continue } ch.out = append(ch.out[:j:j], ch.out[j+1:]...) @@ -119,30 +150,30 @@ func (ch *Chat) outgoingSettle() { } } -// sameMessage says an entry reading got is message want: agents add a picture's -// reference or a reminder of their own, and may keep a long paste apart. +// sameMessage compares the whole recorded message, including attachment paths. +// A shared prefix, screen echo, or a transport response is not acknowledgement. func sameMessage(got, want string) bool { want = strings.Join(strings.Fields(want), " ") - if want == "" { - return true // files alone - } got = strings.Join(strings.Fields(got), " ") - if r := []rune(want); len(r) > 200 { - want = string(r[:200]) - } - return strings.Contains(got, want) + return want != "" && got == want } // outgoingLook marks what the agent hasn't taken although free to. While it is busy // or asks, a message only waits its turn. func (ch *Chat) outgoingLook(o Options, now time.Time) { - for _, m := range ch.out { + for i, m := range ch.out { switch { + case o.Run != m.run: + m.changed, m.failed = true, "The session restarted. Check the terminal." case m.failed != "": + case !m.started: + if i == 0 && !o.Working && !o.Waiting && (ch.loaded || o.Deliver == nil) { + ch.outgoingGo(o, m) + } case o.Working || o.Waiting: m.at = now case now.Sub(m.at) > outgoingLost: - m.failed = "The agent hasn't taken this message." + m.failed = "Delivery isn't confirmed. Check the terminal before sending again." } } } @@ -203,12 +234,22 @@ func (ch *Chat) outgoingNote(c *ui.Context, m *outgoing, running bool) { ui.Row(c).MinWidth(0).Shrink(1).Children(func() { widget.Running(c, m.failed, widget.RunStyle{Color: theme.TextSoft}) }) - if widget.Button(c.Key("retry"), widget.Secondary, nil, "Retry", "").Clicked() { + if !m.sent && !m.changed && widget.Button(c.Key("retry"), widget.Secondary, nil, "Retry", "").Clicked() { ch.outgoingGo(ch.sending, m) } - if widget.Button(c.Key("edit"), widget.Secondary, nil, "Edit", "").Clicked() { + if !m.started && widget.Button(c.Key("edit"), widget.Secondary, nil, "Edit", "").Clicked() { ch.outgoingBack(m) } + if ch.sending.ShowTerminal != nil && widget.Button(c.Key("terminal"), widget.Secondary, nil, "Show terminal", "").Clicked() { + ch.sending.ShowTerminal() + } + if !m.inFlight && widget.Button(c.Key("dismiss"), widget.Secondary, nil, "Stop tracking", "Remove this pending row after reviewing the terminal; no terminal input is changed.").Clicked() { + ch.outgoingDrop(m) + } + }) + case !m.started: + ui.Row(c.Key("outgoing")).FillWidth().Justify(ui.End).Children(func() { + ui.Text(c, "Queued in Chat").FontSize(theme.TextLabel).TextColor(theme.TextSoft).SingleLine() }) case !running: ui.Row(c.Key("outgoing")).FillWidth().Justify(ui.End).Children(func() { diff --git a/ui/chatview/outgoing_test.go b/ui/chatview/outgoing_test.go index 852d1c9c..fb6c9166 100644 --- a/ui/chatview/outgoing_test.go +++ b/ui/chatview/outgoing_test.go @@ -26,6 +26,7 @@ func TestOutgoingSettles(t *testing.T) { if len(ch.out) != 1 { t.Fatalf("after the first: %d left", len(ch.out)) } + ch.outgoingLook(o, time.Now()) // the next message can now leave the local queue ch.apply(proto.Transcript{Supported: true, Rev: 3, Entries: []proto.TranscriptEntry{{Index: 3, Kind: proto.TranscriptAssistant, Text: "ok"}}}) if len(ch.out) != 1 { t.Fatalf("the same entry took the second: %d left", len(ch.out)) @@ -49,8 +50,9 @@ func TestCommandShowsNoBubble(t *testing.T) { // A send that fails keeps the message, to retry or take back with its files. func TestOutgoingFails(t *testing.T) { ch := New() + ch.apply(proto.Transcript{Supported: true, Rev: 1}) var done func(error) - o := Options{Deliver: func(_ string, _ []string, d func(error)) { done = d }} + o := Options{Deliver: func(_, _ string, _ []string, d func(error)) { done = d }} ch.outgoingSend(o, "hello", []string{"/tmp/a.png"}) done(errors.New("no connection")) m := ch.out[0] From 3da59cc339f688bde97451ee6b944d51109cd8ad Mon Sep 17 00:00:00 2001 From: Ahimsa Jenar Bramsaifstyo Kusuma Date: Wed, 4 Nov 2026 23:29:39 +0700 Subject: [PATCH 2/6] Cancel chat delivery on session transitions Validate input history, hook epoch, program identity and observed readiness while queueing paste and delayed Enter. Keep interrupted attempts in the delivery ledger so retries cannot repaste. Preserve done state across multiline bracketed paste. The terminal screen sample and Send still use separate MyGo operations. This fixes the reproduced hook race, but atomic exclusion of raw terminal redraws remains a release gate. Signed-off-by: Ahimsa Jenar Bramsaifstyo Kusuma (cherry picked from commit d815ff770d5a0733940bc3eaf0c172fdeebaa009) --- core/session/delivery.go | 181 +++++++++++++++++++++++++++++ core/session/session.go | 106 ++--------------- core/session/work.go | 3 + daemon/delivery.go | 8 +- daemon/delivery_transition_test.go | 76 ++++++++++++ daemon/review_regression_test.go | 32 +++++ 6 files changed, 306 insertions(+), 100 deletions(-) create mode 100644 core/session/delivery.go create mode 100644 daemon/delivery_transition_test.go create mode 100644 daemon/review_regression_test.go diff --git a/core/session/delivery.go b/core/session/delivery.go new file mode 100644 index 00000000..2f516409 --- /dev/null +++ b/core/session/delivery.go @@ -0,0 +1,181 @@ +package session + +import ( + "bytes" + "strings" + "time" + + "github.com/enowx/enowx/core/agent" + "github.com/enowx/enowx/proto" +) + +// DeliveryGuard binds a delivery to the input and hook history of one program. +// Its fields are private so only successful queueing can advance it. +type DeliveryGuard struct { + input uint64 + epoch uint64 +} + +// Retain observed unsafe screens even if a subsequent redraw hides them. +func (s *Session) noteDeliveryScreenLocked() { + if s.deliveryPending && !agent.PromptFree(s.term.Text()) { + s.deliveryEpoch++ + } +} + +// DeliveryGuard captures cancellation history before inspecting readiness. +func (s *Session) DeliveryGuard() DeliveryGuard { + s.inputMu.Lock() + defer s.inputMu.Unlock() + s.mu.Lock() + defer s.mu.Unlock() + return DeliveryGuard{input: s.inputSeq, epoch: s.deliveryEpoch} +} + +// DeliveryInput validates and queues input against the same session locks as +// manual input, hook reports, activity and program replacement. Notifications +// run only after those locks are released. Empty requires an untouched composer; +// the commit permits the delivery's own pasted draft. +func (m *Manager) DeliveryInput(s *Session, p []byte, guard DeliveryGuard, empty bool) (DeliveryGuard, bool) { + return m.input(s, p, nil, &guard, empty) +} + +// deliveryReadyLocked checks current readiness, not the snapshot before paste. +// The caller holds the manager, input and session locks through queueing. +// Terminal.Text releases its own lock before Send: this serializes known +// session transitions, but cannot exclude a concurrent terminal redraw. +func (s *Session) deliveryReadyLocked(guard DeliveryGuard, empty bool) bool { + if s.inputSeq != guard.input || s.deliveryEpoch != guard.epoch || s.exited || s.meta.Queued { + return false + } + state := s.stateLocked(time.Now()) + if state == proto.StateWaiting || s.workingLocked(time.Now()) { + return false + } + screen := s.term.Text() + return agent.PromptFree(screen) && (!empty || agent.PromptEmpty(screen)) +} + +// Input types p into the session and records it (see NoteInput). +func (m *Manager) Input(s *Session, p []byte) { + m.input(s, p, nil, nil, false) +} + +// InputSequence identifies the most recently queued input, independently of echo. +func (s *Session) InputSequence() uint64 { + s.inputMu.Lock() + defer s.inputMu.Unlock() + return s.inputSeq +} + +// InputIfUnchanged queues p only if no other input followed sequence after. +// It does not infer the contents of the program's composer from its screen. +func (m *Manager) InputIfUnchanged(s *Session, p []byte, after uint64) bool { + _, ok := m.input(s, p, &after, nil, false) + return ok +} + +func (m *Manager) input(s *Session, p []byte, after *uint64, guard *DeliveryGuard, empty bool) (DeliveryGuard, bool) { + // Lock order: manager (guarded delivery only), input, session. Send only + // enqueues bytes; prompt callbacks and notifications run after unlocking. + if guard != nil { + m.mu.Lock() + if m.sessions[s.id] != s { + m.mu.Unlock() + return DeliveryGuard{}, false + } + } + s.inputMu.Lock() + s.mu.Lock() + if after != nil && s.inputSeq != *after || guard != nil && !s.deliveryReadyLocked(*guard, empty) { + if guard != nil { + s.deliveryPending = false + } + s.mu.Unlock() + s.inputMu.Unlock() + if guard != nil { + m.mu.Unlock() + } + return DeliveryGuard{}, false + } + s.term.Send(p) + if guard != nil { + s.deliveryPending = empty + } + if len(p) > 0 { + s.inputSeq++ + } + prompted := s.noteInputLocked(p) + changed := s.stateChanged(time.Now()) + sent := s.lines.feed(p) + next := DeliveryGuard{input: s.inputSeq, epoch: s.deliveryEpoch} + s.mu.Unlock() + s.inputMu.Unlock() + if guard != nil { + m.mu.Unlock() + } + if m.OnPrompt != nil { + for _, line := range sent { + m.OnPrompt(s.id, line) + } + } + if prompted || changed { + m.notify(s) + } + return next, true +} + +// NoteInput records what the user typed: its echo must not count as +// activity, and its first line becomes the session's prompt when none was +// given. It reports whether the prompt was just set. +// +// Typing answers an agent that waits for the user, and Enter sends its +// next prompt after it finished, so the state the hooks reported no longer +// holds; agents with hooks report the next one themselves. +func (s *Session) NoteInput(p []byte) bool { + s.mu.Lock() + defer s.mu.Unlock() + return s.noteInputLocked(p) +} + +func (s *Session) noteInputLocked(p []byte) bool { + s.lastInput = time.Now() + // Newlines inside a bracketed paste edit the draft; only a separate + // Enter submits it. Delivery always queues its entire paste together. + paste := bytes.HasPrefix(p, []byte("\x1b[200~")) && bytes.HasSuffix(p, []byte("\x1b[201~")) + if s.state == proto.StateWaiting || s.state == proto.StateDone && !paste && bytes.ContainsAny(p, "\r\n") { + s.state = "" + s.deliveryEpoch++ + } + if s.prompt != "" { + return false + } + for _, r := range string(p) { + switch { + case s.inEscape: + // CSI and SS3 sequences end with a letter or ~; others after one rune. + if r >= '@' && r <= '~' && r != '[' && r != 'O' { + s.inEscape = false + } + case r == 0x1b: + s.inEscape = true + case r == '\r' || r == '\n': + if line := strings.TrimSpace(string(s.typed)); line != "" { + s.prompt, s.typed = truncate(line, 200), nil + return true + } + s.typed = nil + case r == 0x7f || r == 0x08: + if len(s.typed) > 0 { + s.typed = s.typed[:len(s.typed)-1] + } + case r == 0x15: // Ctrl-U + s.typed = nil + case r >= 0x20: + if len(s.typed) < 400 { + s.typed = append(s.typed, r) + } + } + } + return false +} diff --git a/core/session/session.go b/core/session/session.go index 3d7155c2..420b0f42 100644 --- a/core/session/session.go +++ b/core/session/session.go @@ -6,7 +6,6 @@ package session import ( - "bytes" "crypto/rand" "encoding/hex" "errors" @@ -146,8 +145,10 @@ const EnvSession = "ENOWX_SESSION" // Session is one running (or exited) program. type Session struct { - inputMu sync.Mutex // orders queued input, independently of callbacks - inputSeq uint64 + inputMu sync.Mutex // orders queued input, independently of callbacks + inputSeq uint64 + deliveryEpoch uint64 // guarded by mu; invalidates even a hook's waiting -> done round trip + deliveryPending bool // guarded by mu; inspect readiness on frames only while settling // started is when its program started; exitWhy why it ended, when it wasn't // clean; quickExits how many times in a row, over Replace too, it ended // right after starting, the last at lastExit. @@ -290,53 +291,6 @@ func (s *Session) ScreenChanged() time.Time { return s.screenAt } -// NoteInput records what the user typed: its echo must not count as -// activity, and its first line becomes the session's prompt when none was -// given. It reports whether the prompt was just set. -// -// Typing answers an agent that waits for the user, and Enter sends its -// next prompt after it finished, so the state the hooks reported no longer -// holds; agents with hooks report the next one themselves. -func (s *Session) NoteInput(p []byte) bool { - s.mu.Lock() - defer s.mu.Unlock() - s.lastInput = time.Now() - if s.state == proto.StateWaiting || s.state == proto.StateDone && bytes.ContainsAny(p, "\r\n") { - s.state = "" - } - if s.prompt != "" { - return false - } - for _, r := range string(p) { - switch { - case s.inEscape: - // CSI and SS3 sequences end with a letter or ~; others after one rune. - if r >= '@' && r <= '~' && r != '[' && r != 'O' { - s.inEscape = false - } - case r == 0x1b: - s.inEscape = true - case r == '\r' || r == '\n': - if line := strings.TrimSpace(string(s.typed)); line != "" { - s.prompt, s.typed = truncate(line, 200), nil - return true - } - s.typed = nil - case r == 0x7f || r == 0x08: - if len(s.typed) > 0 { - s.typed = s.typed[:len(s.typed)-1] - } - case r == 0x15: // Ctrl-U - s.typed = nil - case r >= 0x20: - if len(s.typed) < 400 { - s.typed = append(s.typed, r) - } - } - } - return false -} - func truncate(s string, n int) string { r := []rune(s) if len(r) <= n { @@ -605,6 +559,9 @@ func (m *Manager) watchActivity(s *Session) { s.mu.Lock() active := !s.exited && !s.lastChange.IsZero() && now.Sub(s.lastChange) < activeFor changed := active != s.active + if changed && active { + s.deliveryEpoch++ + } if s.active && !active && s.state != proto.StateDone && s.state != proto.StateWaiting { s.meta.FinishedAt = now.UnixMilli() // no hook says when: its screen stopped } @@ -634,6 +591,7 @@ func (s *Session) noteScreen(now time.Time, changed bool) { s.mu.Lock() if changed { s.screenAt = now + s.noteDeliveryScreenLocked() } if changed && now.Sub(s.lastInput) > echoFor { s.lastChange = now @@ -641,53 +599,6 @@ func (s *Session) noteScreen(now time.Time, changed bool) { s.mu.Unlock() } -// Input types p into the session and records it (see NoteInput). -func (m *Manager) Input(s *Session, p []byte) { - m.input(s, p, nil) -} - -// InputSequence identifies the most recently queued input, independently of echo. -func (s *Session) InputSequence() uint64 { - s.inputMu.Lock() - defer s.inputMu.Unlock() - return s.inputSeq -} - -// InputIfUnchanged queues p only if no other input followed sequence after. -// It does not infer the contents of the program's composer from its screen. -func (m *Manager) InputIfUnchanged(s *Session, p []byte, after uint64) bool { - _, ok := m.input(s, p, &after) - return ok -} - -func (m *Manager) input(s *Session, p []byte, after *uint64) (uint64, bool) { - s.inputMu.Lock() - if after != nil && s.inputSeq != *after { - s.inputMu.Unlock() - return 0, false - } - s.term.Send(p) - if len(p) > 0 { - s.inputSeq++ - } - seq := s.inputSeq - s.inputMu.Unlock() - prompted := s.NoteInput(p) - s.mu.Lock() - changed := s.stateChanged(time.Now()) - sent := s.lines.feed(p) - s.mu.Unlock() - if m.OnPrompt != nil { - for _, line := range sent { - m.OnPrompt(s.id, line) - } - } - if prompted || changed { - m.notify(s) - } - return seq, true -} - // SetState records the state the agent's hooks report (proto.State*). func (m *Manager) SetState(s *Session, state string) error { switch state { @@ -697,6 +608,7 @@ func (m *Manager) SetState(s *Session, state string) error { } now := time.Now() s.mu.Lock() + s.deliveryEpoch++ if state == proto.StateDone && s.state != proto.StateDone { s.meta.FinishedAt = now.UnixMilli() } diff --git a/core/session/work.go b/core/session/work.go index 9afe9ac7..67559cb0 100644 --- a/core/session/work.go +++ b/core/session/work.go @@ -192,6 +192,9 @@ func (s *Session) movedFromLocked() string { // SetMode records the permission mode session s's agent is in. func (m *Manager) SetMode(s *Session, mode string) { s.mu.Lock() + if s.meta.Mode != mode { + s.deliveryEpoch++ + } s.meta.Mode = mode s.mu.Unlock() m.notify(s) diff --git a/daemon/delivery.go b/daemon/delivery.go index 89aca981..5b7b224f 100644 --- a/daemon/delivery.go +++ b/daemon/delivery.go @@ -36,6 +36,7 @@ func (s *Server) deliver(p proto.DeliveryParams) (proto.DeliveryResult, error) { if err != nil { return proto.DeliveryResult{}, err } + guard := sess.DeliveryGuard() info := sess.Info() if p.Run == 0 || p.Run != info.Run || info.Exited || info.Queued { return proto.DeliveryResult{}, fmt.Errorf("the session's program changed or isn't running; check its terminal") @@ -43,7 +44,6 @@ func (s *Server) deliver(p proto.DeliveryParams) (proto.DeliveryResult, error) { if p.Key == "" || len(p.Key) > 128 || p.Text == "" { return proto.DeliveryResult{}, fmt.Errorf("delivery needs a key and nonempty text") } - seq := sess.InputSequence() screen := sess.Terminal().Text() ready := agent.PromptEmpty(screen) && agent.PromptFree(screen) d := &s.deliveries @@ -78,7 +78,8 @@ func (s *Server) deliver(p proto.DeliveryParams) (proto.DeliveryResult, error) { r.entries[p.Key], r.sending = a, true d.mu.Unlock() - if !s.sessions.InputIfUnchanged(sess, []byte(pasteStart+p.Text+pasteEnd), seq) { + next, pasted := s.sessions.DeliveryInput(sess, []byte(pasteStart+p.Text+pasteEnd), guard, true) + if !pasted { a.result.Interrupted = true } else { timer := time.NewTimer(pasteSettle) @@ -87,7 +88,8 @@ func (s *Server) deliver(p proto.DeliveryParams) (proto.DeliveryResult, error) { case <-sess.Done(): a.result.Interrupted = true case <-timer.C: - a.result.Interrupted = !s.sessions.InputIfUnchanged(sess, []byte("\r"), seq+1) + _, committed := s.sessions.DeliveryInput(sess, []byte("\r"), next, false) + a.result.Interrupted = !committed } } d.mu.Lock() diff --git a/daemon/delivery_transition_test.go b/daemon/delivery_transition_test.go new file mode 100644 index 00000000..4bf4e796 --- /dev/null +++ b/daemon/delivery_transition_test.go @@ -0,0 +1,76 @@ +package daemon + +import ( + "strings" + "testing" + + "github.com/enowx/enowx/core/session" + "github.com/enowx/enowx/proto" +) + +func TestDeliveryTransitionsDuringSettle(t *testing.T) { + for _, mode := range []string{"waiting", "working", "hook-round-trip", "menu", "menu-round-trip", "working-screen", "permission-mode", "replace", "kill"} { + t.Run(mode, func(t *testing.T) { + srv, c, info, path := deliveryFixture(t) + sess, _ := srv.sessions.Get(info.ID) + p := proto.DeliveryParams{ID: info.ID, Run: info.Run, Key: "transition", Text: "UNSUBMITTED_DRAFT"} + type answer struct { + result proto.DeliveryResult + err error + } + done := make(chan answer, 1) + go func() { r, e := c.Deliver(p); done <- answer{r, e} }() + waitDelivery(t, func() bool { return strings.Contains(sess.Terminal().Text(), p.Text) }) + switch mode { + case "waiting", "working": + if err := srv.sessions.SetState(sess, mode); err != nil { + t.Fatal(err) + } + case "hook-round-trip": + srv.sessions.SetState(sess, proto.StateWaiting) + srv.sessions.SetState(sess, proto.StateDone) + case "menu", "menu-round-trip": + sess.Terminal().Feed([]byte("\x1b[2J\x1b[HDo you want to proceed?\r\n > 1. Yes\r\n 2. No\r\n")) + if mode == "menu-round-trip" { + sess.NoteFrame(true) + sess.Terminal().Feed([]byte("\x1b[2J\x1b[H› " + p.Text)) + sess.NoteFrame(true) + } + case "working-screen": + sess.Terminal().Feed([]byte("\x1b[2J\x1b[H✶ Thinking… (esc to interrupt)\r\n")) + case "permission-mode": + srv.sessions.SetMode(sess, "review") + case "replace": + if _, err := srv.sessions.Replace(session.Spec{ID: info.ID, Exited: true}); err != nil { + t.Fatal(err) + } + case "kill": + if err := srv.sessions.Kill(info.ID); err != nil { + t.Fatal(err) + } + } + a := <-done + if a.err != nil || !a.result.Interrupted { + t.Fatalf("transition: %+v %v", a.result, a.err) + } + if mode == "replace" || mode == "kill" { + return + } + for range 4 { + r, err := c.Deliver(p) + if err != nil || !r.Interrupted { + t.Fatalf("retry: %+v %v", r, err) + } + } + if got := capturedLines(path); len(got) != 0 { + t.Fatalf("submitted draft: %q", got) + } + // Only an explicit manual Enter may submit the preserved single paste. + srv.sessions.Input(sess, []byte("\r")) + waitDelivery(t, func() bool { return len(capturedLines(path)) > 0 }) + if got := strings.Join(capturedLines(path), ""); strings.Count(got, p.Text) != 1 { + t.Fatalf("repasted draft: %q", got) + } + }) + } +} diff --git a/daemon/review_regression_test.go b/daemon/review_regression_test.go new file mode 100644 index 00000000..e2cf5c8e --- /dev/null +++ b/daemon/review_regression_test.go @@ -0,0 +1,32 @@ +package daemon + +import ( + "strings" + "testing" + + "github.com/enowx/enowx/proto" +) + +// A hook can announce a permission wait without any intervening terminal input. +func TestReviewDeliveryStopsEnterWhenStateChanges(t *testing.T) { + srv, c, info, path := deliveryFixture(t) + sess, _ := srv.sessions.Get(info.ID) + type answer struct { + result proto.DeliveryResult + err error + } + done := make(chan answer, 1) + go func() { + r, err := c.Deliver(proto.DeliveryParams{ID: info.ID, Run: info.Run, Key: "review-state-change", Text: "REVIEW_PENDING"}) + done <- answer{r, err} + }() + waitDelivery(t, func() bool { return strings.Contains(sess.Terminal().Text(), "REVIEW_PENDING") }) + srv.sessions.SetState(sess, proto.StateWaiting) + a := <-done + if a.err != nil { + t.Fatal(a.err) + } + if !a.result.Interrupted || len(capturedLines(path)) != 0 { + t.Fatalf("delayed Enter sent after state became waiting: result=%+v captured=%q", a.result, capturedLines(path)) + } +} From fe0aaf1b64d0f0a756fec5b957806ba6e975876a Mon Sep 17 00:00:00 2001 From: Ahimsa Jenar Bramsaifstyo Kusuma Date: Wed, 4 Nov 2026 23:52:28 +0700 Subject: [PATCH 3/6] Serialize delivery readiness with terminal input enqueue Add a 22-line terminal-owner SendChecked operation that holds the terminal and input queue locks through the pure readiness predicate and FIFO insertion. Keep the session cancellation epoch and delivery ledger from d815ff7. Apply the version- and source-hash-checked dependency addition from repository-owned source through a private modfile, without changing cached MyGo files or checking in a fork. Route build entry points through preparation and add a deterministic redraw-boundary regression. Signed-off-by: Ahimsa Jenar Bramsaifstyo Kusuma (cherry picked from commit 32a72e3ddb79a510d650fda047d30e1fb0c4bb4d) --- .github/workflows/ci.yml | 8 +- .github/workflows/release.yml | 4 +- Makefile | 5 +- core/session/delivery.go | 23 ++- scripts/daemons.sh | 7 +- scripts/package.sh | 2 +- scripts/release-local.sh | 4 + scripts/terminalpatch/README.md | 91 +++++++++++ scripts/terminalpatch/guarded_input.go.txt | 22 +++ .../terminalpatch/guarded_input_test.go.txt | 83 ++++++++++ scripts/terminalpatch/main.go | 146 ++++++++++++++++++ scripts/test-distros.sh | 2 + scripts/test-service.sh | 2 + 13 files changed, 384 insertions(+), 15 deletions(-) create mode 100644 scripts/terminalpatch/README.md create mode 100644 scripts/terminalpatch/guarded_input.go.txt create mode 100644 scripts/terminalpatch/guarded_input_test.go.txt create mode 100644 scripts/terminalpatch/main.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1c87b36d..4645612b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,7 +25,7 @@ jobs: shell: bash run: | status=0 - go test -json -timeout 15m ./... > test.json || status=$? + go run ./scripts/terminalpatch -- test -json -timeout 15m ./... > test.json || status=$? go run ./scripts/testreport test.json exit $status - uses: actions/upload-artifact@v4 @@ -35,6 +35,8 @@ jobs: path: test.json retention-days: 7 - run: make lint + - name: Terminal input ordering + run: go run ./scripts/terminalpatch -- test github.com/egoist/mygo/plugins/terminal -run '^TestEnowxCheckedInput' # The cloud's database code against Postgres, as the cloud runs in # production; the tests above run it on SQLite. @@ -73,4 +75,6 @@ jobs: # cmd.exe (docs/agents.md). - run: go test ./core/agent -run 'TestMoveToFiles|TestShortenGuide' - run: go test ./core/diag - - run: go test ./daemon -run 'TestPsQuote|TestPsThenShell|TestCommandsFit|TestLongPrompt|TestAgentThroughCmdShim' + - run: go run ./scripts/terminalpatch -- test ./daemon -run 'TestPsQuote|TestPsThenShell|TestCommandsFit|TestLongPrompt|TestAgentThroughCmdShim' + - name: Terminal input ordering + run: go run ./scripts/terminalpatch -- test github.com/egoist/mygo/plugins/terminal -run '^TestEnowxCheckedInput' diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index aa57f48d..237137d2 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -65,13 +65,13 @@ jobs: env: GH_TOKEN: ${{ github.token }} MYGO_UPDATER_PRIVATE_KEY: ${{ secrets.MYGO_UPDATER_PRIVATE_KEY }} - run: go tool mygo build -platform "${{ matrix.platform }}" -upload cmd/enowx + run: go run ./scripts/terminalpatch -- tool mygo build -platform "${{ matrix.platform }}" -upload cmd/enowx - name: Build if: ${{ !startsWith(github.ref, 'refs/tags/v') }} env: GH_TOKEN: ${{ github.token }} MYGO_UPDATER_PRIVATE_KEY: ${{ secrets.MYGO_UPDATER_PRIVATE_KEY }} - run: go tool mygo build -platform "${{ matrix.platform }}" cmd/enowx + run: go run ./scripts/terminalpatch -- tool mygo build -platform "${{ matrix.platform }}" cmd/enowx - uses: actions/upload-artifact@v4 if: ${{ !startsWith(github.ref, 'refs/tags/v') }} with: diff --git a/Makefile b/Makefile index 6645ac04..1f7ea0ba 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,5 @@ -GO ?= go +HOST_GO ?= go +GO ?= $(HOST_GO) run ./scripts/terminalpatch -- BIN := bin # Where `make dev` keeps its daemon, sessions, projects and hosts, apart # from the installed app's ~/.enowx-app. @@ -19,7 +20,7 @@ build: daemons # Linux builds of enowxd that the app installs on servers. daemons: - GO=$(GO) scripts/daemons.sh + HOST_GO=$(HOST_GO) scripts/daemons.sh test: $(GO) test ./... diff --git a/core/session/delivery.go b/core/session/delivery.go index 2f516409..84016e14 100644 --- a/core/session/delivery.go +++ b/core/session/delivery.go @@ -42,9 +42,7 @@ func (m *Manager) DeliveryInput(s *Session, p []byte, guard DeliveryGuard, empty // deliveryReadyLocked checks current readiness, not the snapshot before paste. // The caller holds the manager, input and session locks through queueing. -// Terminal.Text releases its own lock before Send: this serializes known -// session transitions, but cannot exclude a concurrent terminal redraw. -func (s *Session) deliveryReadyLocked(guard DeliveryGuard, empty bool) bool { +func (s *Session) deliveryReadyLocked(guard DeliveryGuard) bool { if s.inputSeq != guard.input || s.deliveryEpoch != guard.epoch || s.exited || s.meta.Queued { return false } @@ -52,8 +50,7 @@ func (s *Session) deliveryReadyLocked(guard DeliveryGuard, empty bool) bool { if state == proto.StateWaiting || s.workingLocked(time.Now()) { return false } - screen := s.term.Text() - return agent.PromptFree(screen) && (!empty || agent.PromptEmpty(screen)) + return true } // Input types p into the session and records it (see NoteInput). @@ -87,7 +84,7 @@ func (m *Manager) input(s *Session, p []byte, after *uint64, guard *DeliveryGuar } s.inputMu.Lock() s.mu.Lock() - if after != nil && s.inputSeq != *after || guard != nil && !s.deliveryReadyLocked(*guard, empty) { + if after != nil && s.inputSeq != *after || guard != nil && !s.deliveryReadyLocked(*guard) { if guard != nil { s.deliveryPending = false } @@ -98,7 +95,19 @@ func (m *Manager) input(s *Session, p []byte, after *uint64, guard *DeliveryGuar } return DeliveryGuard{}, false } - s.term.Send(p) + if guard != nil { + if !s.term.SendChecked(p, func(screen string) bool { + return agent.PromptFree(screen) && (!empty || agent.PromptEmpty(screen)) + }) { + s.deliveryPending = false + s.mu.Unlock() + s.inputMu.Unlock() + m.mu.Unlock() + return DeliveryGuard{}, false + } + } else { + s.term.Send(p) + } if guard != nil { s.deliveryPending = empty } diff --git a/scripts/daemons.sh b/scripts/daemons.sh index 7d8f0a46..aeb9832a 100755 --- a/scripts/daemons.sh +++ b/scripts/daemons.sh @@ -2,7 +2,12 @@ # Builds the Linux enowxd the app embeds, to install on servers. set -eu cd "$(dirname "$0")/.." -go=${GO:-go} +# Bootstrap on this host before applying target GOOS/GOARCH to the builds. +go=${HOST_GO:-go} +patch=$(mktemp -d "${TMPDIR:-/tmp}/enowx-terminalpatch.XXXXXX") +trap 'rm -rf "$patch"' EXIT +$go run ./scripts/terminalpatch -out "$patch/go.mod" +export GOFLAGS="${GOFLAGS:-} -modfile=$patch/go.mod" for arch in amd64 arm64; do GOOS=linux GOARCH=$arch CGO_ENABLED=0 $go build -trimpath -ldflags "-s -w" -o hostconn/daemons/enowxd-linux-$arch ./cmd/enowxd gzip -9f hostconn/daemons/enowxd-linux-$arch diff --git a/scripts/package.sh b/scripts/package.sh index 3b8ecb20..6afe0b09 100755 --- a/scripts/package.sh +++ b/scripts/package.sh @@ -15,7 +15,7 @@ mkdir -p "$HOME/.cache" work=$(mktemp -d "$HOME/.cache/enowx-package.XXXXXX") trap 'rm -rf "$work"' EXIT scripts/daemons.sh -go tool mygo build -o "$work" -platform "$platforms" cmd/enowx +go run ./scripts/terminalpatch -- tool mygo build -o "$work" -platform "$platforms" cmd/enowx mkdir -p "$dist" find "$work" -maxdepth 2 -type f \( -name '*.dmg' -o -name '*.deb' -o -name '*.tar.gz' -o -name '*Setup*.exe' -o -name install.sh \) | while read -r f; do case "$f" in diff --git a/scripts/release-local.sh b/scripts/release-local.sh index 9bb9c28b..43651d3b 100755 --- a/scripts/release-local.sh +++ b/scripts/release-local.sh @@ -11,6 +11,10 @@ mkdir -p "$dist" cd "$root" make daemons +patch=$(mktemp -d "${TMPDIR:-/tmp}/enowx-terminalpatch.XXXXXX") +trap 'rm -rf "$patch"' EXIT +go run ./scripts/terminalpatch -out "$patch/go.mod" +export GOFLAGS="${GOFLAGS:-} -modfile=$patch/go.mod" go tool mygo build -platform darwin/universal cmd/enowx app="$root/build/darwin-universal/enowX.app" diff --git a/scripts/terminalpatch/README.md b/scripts/terminalpatch/README.md new file mode 100644 index 00000000..aab740c0 --- /dev/null +++ b/scripts/terminalpatch/README.md @@ -0,0 +1,91 @@ +# Guarded terminal input + +MyGo v0.6.2 has separate `Text()` and `Send()` operations. A permission/menu +redraw can complete after the text sample but before Enter is enqueued. Its +public API has no operation that covers both. This directory adds one method, +`Terminal.SendChecked(data, allow) bool`, inside the terminal package. + +The addition is in `guarded_input.go.txt`, with tests in +`guarded_input_test.go.txt`. They are Go source stored as embedded resources so +they are compiled only in the dependency's package. No existing dependency file +is rewritten. No entire dependency fork is checked in. + +`SendChecked` holds `Terminal.mu`, then `inputQueue.mu`, through a pure screen +predicate and FIFO insertion. `Terminal.write`/`Feed` use the same terminal +mutex; terminal replies already use this lock order. The queue consumer releases +the queue lock before PTY I/O or completion callbacks. Data is copied, and a +closed terminal/queue refuses input. The predicate must not block or reenter +the terminal. The session holds manager -> input -> session locks before this +call, so known hook/state/program/manual-input transitions cannot invalidate +the guard between validation and enqueue. Notifications run after unlocking. + +Success means **enqueued**. It does not mean the PTY consumed Enter, nor that a +native permission was accepted. Output that reaches the terminal after enqueue +is later in this ordering. An unsafe screen that appeared and disappeared +before inspection is detected only if the session observed it during settling; +this API does not promise a historical classification of every raw redraw. + +## Reproducible use + +Use the wrapper from the repository root with Go 1.27.3+: + +```sh +go run ./scripts/terminalpatch -- test ./core/session ./daemon +go run ./scripts/terminalpatch -- test github.com/egoist/mygo/plugins/terminal -run '^TestEnowxCheckedInput' -count=10 +go run ./scripts/terminalpatch -- vet ./core/session ./daemon +``` + +`make` targets, the daemon/package scripts and applicable CI/release commands +use the patch. Direct `go test/build` of packages importing session requires +the prepared modfile; otherwise the missing `SendChecked` method fails compilation. +If overriding Make's `GO`, retain the wrapper or supply its prepared modfile. + +Go forbids overlays beneath `GOMODCACHE`. The tool therefore stages the pinned +module (about 16 MiB) in a unique temporary directory, adds these two files, and +selects it through a private modfile and `GOFLAGS`. Child Go commands inherit +that modfile; the wrapper removes its own temporary directory after the child +exits. The module cache and repository `go.mod`/`go.sum` are untouched. To retain +a fixture or cross compile with a host bootstrap: + +```sh +go run ./scripts/terminalpatch -out /absolute/new-fixture/go.mod +GOFLAGS='-modfile=/absolute/new-fixture/go.mod' go test ./core/session ./daemon +``` + +The output directory must be new; preparation refuses to overlay an existing +module copy. Use `TMPDIR` (Unix) or `TMP`/`TEMP` (Windows) to select the staging +drive. Prepare with host GOOS/GOARCH before applying cross compilation settings. +An existing unrelated `-modfile` is refused by the wrapper. + +The tool requires un-replaced MyGo v0.6.2 and verifies SHA-256 of the terminal +owner (`terminal.go`): +`7902afddc644b21ffcbc0d567a144b430ef579c8dd538f80499c99beb412738d`. +On a MyGo upgrade, review the mutex/queue layout, adopt a supported upstream +operation if available, and remove this patch and bootstrap when obsolete. + +## Deterministic boundary test + +`TestEnowxCheckedInputRedrawBoundary` pauses the readiness predicate with channels. +With no PTY, watcher, UI or queue-consumer goroutine, `TryLock` directly proves +whether a raw redraw can acquire the terminal mutex at that boundary. If it +can, the test completes an unsafe `Feed` before allowing enqueue and fails on +the queued Enter. With the patch, that lock remains held, Enter is enqueued +before the redraw can complete, and a later redraw-first attempt is refused. +No delay or timeout is used to manufacture the interleaving. + +For fail-before reproduction, in a fresh **private fixture copy only**, replace +the addition's implementation with the old split sequence: + +```go +func (t *Terminal) SendChecked(data []byte, allow func(string) bool) bool { + if !allow(t.Text()) { return false } + t.Send(data) + return true +} +``` + +Run the exact boundary test above. This is an instrumented extraction of the +old session's `Text`/predicate/`Send` sequence, not a native permission fixture. +The queue test separately checks FIFO order, copy ownership and refusal after +closure. Existing daemon-protocol and native fixture tests cover the delivery +ledger, interrupted retries, manual drafts and lost responses. diff --git a/scripts/terminalpatch/guarded_input.go.txt b/scripts/terminalpatch/guarded_input.go.txt new file mode 100644 index 00000000..5267ff50 --- /dev/null +++ b/scripts/terminalpatch/guarded_input.go.txt @@ -0,0 +1,22 @@ +package terminal + +// SendChecked queues data only if allow accepts the current screen. The +// predicate runs under the terminal mutex and must be a pure, nonblocking +// check that never reenters the terminal. Screen writes and queue insertion +// cannot interleave. Success means queued, not written to or read by the PTY. +func (t *Terminal) SendChecked(data []byte, allow func(string) bool) bool { + t.mu.Lock() + defer t.mu.Unlock() + // Same lock order as terminal replies: terminal -> input queue. + t.in.mu.Lock() + defer t.in.mu.Unlock() + if t.closed || t.term == nil || t.in.closed || allow == nil || !allow(t.term.Text()) { + return false + } + if len(data) > 0 { + t.in.init() + t.in.queue = append(t.in.queue, queued{p: append([]byte(nil), data...)}) + t.in.cond.Signal() + } + return true +} diff --git a/scripts/terminalpatch/guarded_input_test.go.txt b/scripts/terminalpatch/guarded_input_test.go.txt new file mode 100644 index 00000000..6b3fd4b1 --- /dev/null +++ b/scripts/terminalpatch/guarded_input_test.go.txt @@ -0,0 +1,83 @@ +package terminal + +import ( + "strings" + "testing" + + "github.com/egoist/mygo/plugins/terminal/internal/vt" +) + +func guardedFixture(t *testing.T) *Terminal { + t.Helper() + if err := Load(); err != nil { + t.Fatal(err) + } + v, err := vt.NewTerminal(80, 24, vt.Effects{}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(v.Free) + // No PTY, queue consumer, UI or activity goroutine can hold either lock. + term := &Terminal{term: v} + term.Feed([]byte("› DRAFT")) + return term +} + +func TestEnowxCheckedInputRedrawBoundary(t *testing.T) { + term := guardedFixture(t) + checked, resume := make(chan struct{}), make(chan struct{}) + done := make(chan bool, 1) + go func() { + done <- term.SendChecked([]byte("\r"), func(screen string) bool { + close(checked) + <-resume + return !strings.Contains(screen, "Proceed?") + }) + }() + <-checked + // This is the exact mutex acquired by Feed/write for a raw redraw. A + // failed TryLock proves exclusion without a sleep or scheduler guess. + redrawCanWin := term.mu.TryLock() + if redrawCanWin { + term.mu.Unlock() + term.Feed([]byte("\x1b[2J\x1b[HProceed?\r\n> Yes\r\n No")) + } + close(resume) + accepted := <-done + if redrawCanWin { + t.Fatalf("raw redraw completed between screen check and enqueue: accepted=%v queue=%+v", accepted, term.in.queue) + } + if !accepted || len(term.in.queue) != 1 || string(term.in.queue[0].p) != "\r" { + t.Fatalf("commit before redraw: accepted=%v queue=%+v", accepted, term.in.queue) + } + // The competing redraw can now complete, after the Enter was enqueued. + term.Feed([]byte("\x1b[2J\x1b[HProceed?\r\n> Yes\r\n No")) + if term.SendChecked([]byte("\r"), func(screen string) bool { return !strings.Contains(screen, "Proceed?") }) { + t.Fatal("accepted Enter after redraw won") + } + if len(term.in.queue) != 1 { + t.Fatalf("unexpected queue: %+v", term.in.queue) + } + t.Log("terminal mutex excludes raw redraw through predicate and enqueue; redraw-first refuses Enter") +} + +func TestEnowxCheckedInputQueueOwnership(t *testing.T) { + term := guardedFixture(t) + term.Send([]byte("MANUAL")) + data := []byte("PASTE") + if !term.SendChecked(data, func(string) bool { return true }) { + t.Fatal("refused ready screen") + } + data[0] = 'X' + if len(term.in.queue) != 2 || string(term.in.queue[0].p) != "MANUAL" || string(term.in.queue[1].p) != "PASTE" { + t.Fatalf("queue order or ownership lost: %+v", term.in.queue) + } + term.in.close() + called := false + if term.SendChecked([]byte("\r"), func(string) bool { called = true; return true }) { + t.Fatal("accepted input into closed queue") + } + if called { + t.Fatal("ran predicate after queue closed") + } +} diff --git a/scripts/terminalpatch/main.go b/scripts/terminalpatch/main.go new file mode 100644 index 00000000..a3458fb5 --- /dev/null +++ b/scripts/terminalpatch/main.go @@ -0,0 +1,146 @@ +// Command terminalpatch runs Go with the pinned terminal source addition in a +// private module copy. It never patches cached module source. +package main + +import ( + "crypto/sha256" + _ "embed" + "encoding/json" + "flag" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/enowx/enowx/internal/quietcmd" +) + +//go:embed guarded_input.go.txt +var source []byte + +//go:embed guarded_input_test.go.txt +var tests []byte + +func main() { + out := flag.String("out", "", "absolute private go.mod output path (required)") + flag.Parse() + if err := run(*out, flag.Args()); err != nil { + fmt.Fprintln(os.Stderr, "terminalpatch:", err) + if e, ok := err.(interface{ ExitCode() int }); ok { + os.Exit(e.ExitCode()) + } + os.Exit(1) + } +} + +func run(out string, args []string) error { + if out != "" { + return prepare(out) + } + if len(args) == 0 { + return fmt.Errorf("use -- , or -out /absolute/path/go.mod") + } + // Nested Go commands (MyGo, cross builds) inherit this same modfile. + if os.Getenv("ENOWX_TERMINAL_PATCH") == "v0.6.2" { + return runGo(args, "") + } + if strings.Contains(os.Getenv("GOFLAGS"), "-modfile") { + return fmt.Errorf("cannot compose with an existing -modfile") + } + dir, err := os.MkdirTemp("", "enowx-terminalpatch-") + if err != nil { + return err + } + defer os.RemoveAll(dir) + out = filepath.Join(dir, "go.mod") + if err := prepare(out); err != nil { + return err + } + return runGo(args, out) +} + +func runGo(args []string, modfile string) error { + cmd := quietcmd.Command("go", args...) + cmd.Stdin, cmd.Stdout, cmd.Stderr = os.Stdin, os.Stdout, os.Stderr + cmd.Env = os.Environ() + if modfile != "" { + cmd.Env = append(cmd.Env, "GOFLAGS="+os.Getenv("GOFLAGS")+fmt.Sprintf(" %q", "-modfile="+modfile), "ENOWX_TERMINAL_PATCH=v0.6.2") + } + return cmd.Run() +} + +func prepare(out string) error { + if !filepath.IsAbs(out) { + return fmt.Errorf("-out must be an absolute path") + } + cmd := quietcmd.Command("go", "list", "-m", "-json", "github.com/egoist/mygo") + b, err := cmd.Output() + if err != nil { + return fmt.Errorf("locate terminal module: %w", err) + } + var mod struct { + Dir, Version string + Replace *json.RawMessage + } + if err := json.Unmarshal(b, &mod); err != nil { + return err + } + if mod.Version != "v0.6.2" || mod.Replace != nil { + return fmt.Errorf("requires unmodified github.com/egoist/mygo v0.6.2; review patch when upgrading") + } + if mod.Dir == "" { + cmd = quietcmd.Command("go", "mod", "download", "-json", "github.com/egoist/mygo") + b, err = cmd.Output() + if err != nil { + return fmt.Errorf("download pinned terminal module: %w", err) + } + if err := json.Unmarshal(b, &mod); err != nil { + return err + } + if mod.Dir == "" { + return fmt.Errorf("pinned terminal module has no source directory") + } + } + b, err = os.ReadFile(filepath.Join(mod.Dir, "plugins", "terminal", "terminal.go")) + if err != nil { + return err + } + if fmt.Sprintf("%x", sha256.Sum256(b)) != "7902afddc644b21ffcbc0d567a144b430ef579c8dd538f80499c99beb412738d" { + return fmt.Errorf("terminal owner source differs from reviewed v0.6.2; review locking and queue layout") + } + if err := os.MkdirAll(filepath.Dir(out), 0700); err != nil { + return err + } + // Go forbids overlays beneath GOMODCACHE. Stage the pinned module in the + // output fixture, rather than modifying the cache or committing a fork. + private := filepath.Join(filepath.Dir(out), "mygo-v0.6.2") + if err := os.CopyFS(private, os.DirFS(mod.Dir)); err != nil { + return fmt.Errorf("stage private terminal dependency: %w", err) + } + for name, data := range map[string][]byte{ + "enowx_guarded_input.go": source, + "enowx_guarded_input_test.go": tests, + } { + dest := filepath.Join(private, "plugins", "terminal", name) + if err := os.WriteFile(dest, data, 0600); err != nil { + return err + } + } + b, err = os.ReadFile("go.mod") + if err != nil { + return err + } + b = append(b, []byte(fmt.Sprintf("\nreplace github.com/egoist/mygo => %q\n", filepath.ToSlash(private)))...) + if err := os.WriteFile(out, b, 0600); err != nil { + return err + } + b, err = os.ReadFile("go.sum") + if err != nil { + return err + } + if err := os.WriteFile(out[:len(out)-len(filepath.Ext(out))]+".sum", b, 0600); err != nil { + return err + } + fmt.Fprintf(os.Stderr, "terminalpatch: private MyGo v0.6.2 plus guarded input API at %s; module cache untouched\n", private) + return nil +} diff --git a/scripts/test-distros.sh b/scripts/test-distros.sh index 5381ecf6..ee9d5e62 100755 --- a/scripts/test-distros.sh +++ b/scripts/test-distros.sh @@ -15,6 +15,8 @@ esac mkdir -p "$HOME/.cache" out=$(mktemp -d "$HOME/.cache/enowx-distros.XXXXXX") trap 'rm -rf "$out"' EXIT +go run ./scripts/terminalpatch -out "$out/go.mod" +export GOFLAGS="${GOFLAGS:-} -modfile=$out/go.mod" CGO_ENABLED=0 GOOS=linux GOARCH=$goarch go build -o "$out/enowxd" ./cmd/enowxd CGO_ENABLED=0 GOOS=linux GOARCH=$goarch go test -c -o "$out/setup.test" ./core/setup cp hostconn/prepare.sh "$out/" diff --git a/scripts/test-service.sh b/scripts/test-service.sh index 330a6fae..e3b5b30a 100755 --- a/scripts/test-service.sh +++ b/scripts/test-service.sh @@ -13,6 +13,8 @@ mkdir -p "$HOME/.cache" out=$(mktemp -d "$HOME/.cache/enowx-service.XXXXXX") name=enowx-service-test trap 'docker rm -f $name >/dev/null 2>&1; rm -rf "$out"' EXIT +go run ./scripts/terminalpatch -out "$out/go.mod" +export GOFLAGS="${GOFLAGS:-} -modfile=$out/go.mod" CGO_ENABLED=0 GOOS=linux GOARCH=$goarch go build -o "$out/enowxd" ./cmd/enowxd cp hostconn/linger.sh "$out/" cat >"$out/Dockerfile" <<'DOCKERFILE' From 87d4568eb40748989ebb6eca605f83b179f74fef Mon Sep 17 00:00:00 2001 From: Ahimsa Jenar Bramsaifstyo Kusuma Date: Thu, 5 Nov 2026 01:10:05 +0700 Subject: [PATCH 4/6] Fix terminalpatch bootstrap across build targets Build the preparer for GOHOSTOS/GOHOSTARCH while preserving the child target and selected Go executable. Generate quoted modfile flags from native paths so spaced homes and Git Bash paths remain valid for Go. Signed-off-by: Ahimsa Jenar Bramsaifstyo Kusuma (cherry picked from commit 0788997ec2f57291092d94df466a25ea664e4d59) --- .github/workflows/ci.yml | 13 +- .github/workflows/release.yml | 4 +- Makefile | 4 +- scripts/daemons.sh | 6 +- scripts/package.sh | 2 +- scripts/release-local.sh | 8 +- scripts/terminalpatch.sh | 10 + scripts/terminalpatch/README.md | 29 ++- scripts/terminalpatch/bootstrap_test.go | 250 ++++++++++++++++++++++++ scripts/terminalpatch/main.go | 23 ++- scripts/test-distros.sh | 12 +- scripts/test-service.sh | 6 +- 12 files changed, 330 insertions(+), 37 deletions(-) create mode 100755 scripts/terminalpatch.sh create mode 100644 scripts/terminalpatch/bootstrap_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4645612b..cb44f9aa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,7 +25,7 @@ jobs: shell: bash run: | status=0 - go run ./scripts/terminalpatch -- test -json -timeout 15m ./... > test.json || status=$? + sh scripts/terminalpatch.sh -- test -json -timeout 15m ./... > test.json || status=$? go run ./scripts/testreport test.json exit $status - uses: actions/upload-artifact@v4 @@ -36,7 +36,7 @@ jobs: retention-days: 7 - run: make lint - name: Terminal input ordering - run: go run ./scripts/terminalpatch -- test github.com/egoist/mygo/plugins/terminal -run '^TestEnowxCheckedInput' + run: sh scripts/terminalpatch.sh -- test github.com/egoist/mygo/plugins/terminal -run '^TestEnowxCheckedInput' # The cloud's database code against Postgres, as the cloud runs in # production; the tests above run it on SQLite. @@ -75,6 +75,11 @@ jobs: # cmd.exe (docs/agents.md). - run: go test ./core/agent -run 'TestMoveToFiles|TestShortenGuide' - run: go test ./core/diag - - run: go run ./scripts/terminalpatch -- test ./daemon -run 'TestPsQuote|TestPsThenShell|TestCommandsFit|TestLongPrompt|TestAgentThroughCmdShim' + - shell: bash + run: sh scripts/terminalpatch.sh -- test ./daemon -run 'TestPsQuote|TestPsThenShell|TestCommandsFit|TestLongPrompt|TestAgentThroughCmdShim' - name: Terminal input ordering - run: go run ./scripts/terminalpatch -- test github.com/egoist/mygo/plugins/terminal -run '^TestEnowxCheckedInput' + shell: bash + run: sh scripts/terminalpatch.sh -- test github.com/egoist/mygo/plugins/terminal -run '^TestEnowxCheckedInput' + - name: Terminal bootstrap portability + shell: bash + run: ENOWX_TEST_SH="$(cygpath -m "$BASH")" go test ./scripts/terminalpatch diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 237137d2..01a42ae4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -65,13 +65,13 @@ jobs: env: GH_TOKEN: ${{ github.token }} MYGO_UPDATER_PRIVATE_KEY: ${{ secrets.MYGO_UPDATER_PRIVATE_KEY }} - run: go run ./scripts/terminalpatch -- tool mygo build -platform "${{ matrix.platform }}" -upload cmd/enowx + run: sh scripts/terminalpatch.sh -- tool mygo build -platform "${{ matrix.platform }}" -upload cmd/enowx - name: Build if: ${{ !startsWith(github.ref, 'refs/tags/v') }} env: GH_TOKEN: ${{ github.token }} MYGO_UPDATER_PRIVATE_KEY: ${{ secrets.MYGO_UPDATER_PRIVATE_KEY }} - run: go run ./scripts/terminalpatch -- tool mygo build -platform "${{ matrix.platform }}" cmd/enowx + run: sh scripts/terminalpatch.sh -- tool mygo build -platform "${{ matrix.platform }}" cmd/enowx - uses: actions/upload-artifact@v4 if: ${{ !startsWith(github.ref, 'refs/tags/v') }} with: diff --git a/Makefile b/Makefile index 1f7ea0ba..94982037 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,5 @@ HOST_GO ?= go -GO ?= $(HOST_GO) run ./scripts/terminalpatch -- +GO ?= HOST_GO="$(HOST_GO)" sh scripts/terminalpatch.sh -- BIN := bin # Where `make dev` keeps its daemon, sessions, projects and hosts, apart # from the installed app's ~/.enowx-app. @@ -20,7 +20,7 @@ build: daemons # Linux builds of enowxd that the app installs on servers. daemons: - HOST_GO=$(HOST_GO) scripts/daemons.sh + HOST_GO="$(HOST_GO)" scripts/daemons.sh test: $(GO) test ./... diff --git a/scripts/daemons.sh b/scripts/daemons.sh index aeb9832a..0e4cc90c 100755 --- a/scripts/daemons.sh +++ b/scripts/daemons.sh @@ -6,9 +6,9 @@ cd "$(dirname "$0")/.." go=${HOST_GO:-go} patch=$(mktemp -d "${TMPDIR:-/tmp}/enowx-terminalpatch.XXXXXX") trap 'rm -rf "$patch"' EXIT -$go run ./scripts/terminalpatch -out "$patch/go.mod" -export GOFLAGS="${GOFLAGS:-} -modfile=$patch/go.mod" +flags=$(sh scripts/terminalpatch.sh -out "$patch/go.mod" -print-go-flags) +export GOFLAGS="$flags" for arch in amd64 arm64; do - GOOS=linux GOARCH=$arch CGO_ENABLED=0 $go build -trimpath -ldflags "-s -w" -o hostconn/daemons/enowxd-linux-$arch ./cmd/enowxd + GOOS=linux GOARCH=$arch CGO_ENABLED=0 "$go" build -trimpath -ldflags "-s -w" -o hostconn/daemons/enowxd-linux-$arch ./cmd/enowxd gzip -9f hostconn/daemons/enowxd-linux-$arch done diff --git a/scripts/package.sh b/scripts/package.sh index 6afe0b09..dfa97576 100755 --- a/scripts/package.sh +++ b/scripts/package.sh @@ -15,7 +15,7 @@ mkdir -p "$HOME/.cache" work=$(mktemp -d "$HOME/.cache/enowx-package.XXXXXX") trap 'rm -rf "$work"' EXIT scripts/daemons.sh -go run ./scripts/terminalpatch -- tool mygo build -o "$work" -platform "$platforms" cmd/enowx +sh scripts/terminalpatch.sh -- tool mygo build -o "$work" -platform "$platforms" cmd/enowx mkdir -p "$dist" find "$work" -maxdepth 2 -type f \( -name '*.dmg' -o -name '*.deb' -o -name '*.tar.gz' -o -name '*Setup*.exe' -o -name install.sh \) | while read -r f; do case "$f" in diff --git a/scripts/release-local.sh b/scripts/release-local.sh index 43651d3b..927eae20 100755 --- a/scripts/release-local.sh +++ b/scripts/release-local.sh @@ -13,9 +13,9 @@ cd "$root" make daemons patch=$(mktemp -d "${TMPDIR:-/tmp}/enowx-terminalpatch.XXXXXX") trap 'rm -rf "$patch"' EXIT -go run ./scripts/terminalpatch -out "$patch/go.mod" -export GOFLAGS="${GOFLAGS:-} -modfile=$patch/go.mod" -go tool mygo build -platform darwin/universal cmd/enowx +flags=$(sh scripts/terminalpatch.sh -out "$patch/go.mod" -print-go-flags) +export GOFLAGS="$flags" +"${HOST_GO:-go}" tool mygo build -platform darwin/universal cmd/enowx app="$root/build/darwin-universal/enowX.app" if [[ "$(uname)" == "Darwin" ]]; then @@ -27,7 +27,7 @@ fi for arch in amd64 arm64; do mkdir -p "$dist/linux-$arch" - GOOS=linux GOARCH="$arch" CGO_ENABLED=0 go build -trimpath -ldflags "-s -w" -o "$dist/linux-$arch/enowxd" ./cmd/enowxd + GOOS=linux GOARCH="$arch" CGO_ENABLED=0 "${HOST_GO:-go}" build -trimpath -ldflags "-s -w" -o "$dist/linux-$arch/enowxd" ./cmd/enowxd tar -C "$dist/linux-$arch" -czf "$dist/enowxd-$version-linux-$arch.tar.gz" enowxd done diff --git a/scripts/terminalpatch.sh b/scripts/terminalpatch.sh new file mode 100755 index 00000000..0a289303 --- /dev/null +++ b/scripts/terminalpatch.sh @@ -0,0 +1,10 @@ +#!/bin/sh +# Compile the preparer for this host without changing its child's target. +set -eu +go=${HOST_GO:-go} +hostos=$("$go" env GOHOSTOS) +hostarch=$("$go" env GOHOSTARCH) +work=$(mktemp -d "${TMPDIR:-/tmp}/enowx-terminal-bootstrap.XXXXXX") +trap 'rm -rf "$work"' EXIT +GOOS=$hostos GOARCH=$hostarch "$go" build -o "$work/terminalpatch.exe" ./scripts/terminalpatch +"$work/terminalpatch.exe" -go "$go" "$@" diff --git a/scripts/terminalpatch/README.md b/scripts/terminalpatch/README.md index aab740c0..9da13a70 100644 --- a/scripts/terminalpatch/README.md +++ b/scripts/terminalpatch/README.md @@ -27,16 +27,26 @@ this API does not promise a historical classification of every raw redraw. ## Reproducible use -Use the wrapper from the repository root with Go 1.27.3+: +Use the shell entrypoint from the repository root with Go 1.27.3+ (Git Bash +on Windows): ```sh -go run ./scripts/terminalpatch -- test ./core/session ./daemon -go run ./scripts/terminalpatch -- test github.com/egoist/mygo/plugins/terminal -run '^TestEnowxCheckedInput' -count=10 -go run ./scripts/terminalpatch -- vet ./core/session ./daemon +sh scripts/terminalpatch.sh -- test ./core/session ./daemon +sh scripts/terminalpatch.sh -- test github.com/egoist/mygo/plugins/terminal -run '^TestEnowxCheckedInput' -count=10 +sh scripts/terminalpatch.sh -- vet ./core/session ./daemon ``` +`terminalpatch.sh` asks the selected Go toolchain for GOHOSTOS/GOHOSTARCH and +compiles the preparer for that host in a private temporary directory. It runs +the preparer with the original environment, so an inherited GOOS/GOARCH still +selects the child's build target. The bootstrap executable is removed when the +child finishes. `HOST_GO` selects an underlying Go executable, including a path +containing spaces, rather than another wrapper. Its ordinary GOTOOLCHAIN +selection still applies. Preparation and child Go commands use that same +executable, without recursive bootstrap. + `make` targets, the daemon/package scripts and applicable CI/release commands -use the patch. Direct `go test/build` of packages importing session requires +use this entrypoint. Direct `go test/build` of packages importing session requires the prepared modfile; otherwise the missing `SendChecked` method fails compilation. If overriding Make's `GO`, retain the wrapper or supply its prepared modfile. @@ -48,13 +58,16 @@ exits. The module cache and repository `go.mod`/`go.sum` are untouched. To retai a fixture or cross compile with a host bootstrap: ```sh -go run ./scripts/terminalpatch -out /absolute/new-fixture/go.mod -GOFLAGS='-modfile=/absolute/new-fixture/go.mod' go test ./core/session ./daemon +flags=$(sh scripts/terminalpatch.sh -out "/absolute/new fixture/go.mod" -print-go-flags) +GOFLAGS="$flags" go test ./core/session ./daemon ``` The output directory must be new; preparation refuses to overlay an existing module copy. Use `TMPDIR` (Unix) or `TMP`/`TEMP` (Windows) to select the staging -drive. Prepare with host GOOS/GOARCH before applying cross compilation settings. +drive. `TMPDIR` selects the shell bootstrap directory. `-print-go-flags` emits +the existing GOFLAGS plus a quoted modfile token using the preparer's native +path spelling. This also handles Git Bash's argument-path conversion on Windows; +putting a shell-only `/c/...` path inside GOFLAGS would not convert it for Go. An existing unrelated `-modfile` is refused by the wrapper. The tool requires un-replaced MyGo v0.6.2 and verifies SHA-256 of the terminal diff --git a/scripts/terminalpatch/bootstrap_test.go b/scripts/terminalpatch/bootstrap_test.go new file mode 100644 index 00000000..12b9b295 --- /dev/null +++ b/scripts/terminalpatch/bootstrap_test.go @@ -0,0 +1,250 @@ +package main + +import ( + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/enowx/enowx/internal/quietcmd" +) + +// Only the expensive build is replaced: bootstrap, preparation and Go's flag +// parser run for real. Exit 73 stops the scripts before packaging or Docker. +func TestMain(m *testing.M) { + if os.Getenv("ENOWX_BOOTSTRAP_FAKE_GO") == "1" { + args := os.Args[1:] + if len(args) > 0 && (args[0] == "tool" || args[0] == "build" && args[len(args)-1] == "./cmd/enowxd" || args[0] == "run" && len(args) > 1 && args[1] == "./cmd/enowxd") { + cmd := quietcmd.Command(os.Getenv("ENOWX_BOOTSTRAP_REAL_GO"), "list", "-m", "-json", "github.com/egoist/mygo") + b, err := cmd.CombinedOutput() + if err != nil { + fmt.Fprint(os.Stderr, string(b)) + os.Exit(1) + } + var module struct{ Replace *struct{ Dir string } } + if err := json.Unmarshal(b, &module); err != nil || module.Replace == nil { + fmt.Fprintln(os.Stderr, "child did not select the private module:", string(b), err) + os.Exit(1) + } + if _, err := os.Stat(filepath.Join(module.Replace.Dir, "plugins", "terminal", "enowx_guarded_input.go")); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + record, _ := json.Marshal(childBuild{Args: args, OS: os.Getenv("GOOS"), Arch: os.Getenv("GOARCH"), Flags: os.Getenv("GOFLAGS"), Module: module.Replace.Dir}) + if err := os.WriteFile(os.Getenv("ENOWX_BOOTSTRAP_RECORD"), record, 0600); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(73) + } + cmd := quietcmd.Command(os.Getenv("ENOWX_BOOTSTRAP_REAL_GO"), args...) + cmd.Stdin, cmd.Stdout, cmd.Stderr = os.Stdin, os.Stdout, os.Stderr + if err := cmd.Run(); err != nil { + if e, ok := err.(interface{ ExitCode() int }); ok { + os.Exit(e.ExitCode()) + } + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(0) + } + os.Exit(m.Run()) +} + +type childBuild struct { + Args []string + OS string + Arch string + Flags string + Module string +} + +func TestBuildScriptBootstrap(t *testing.T) { + for _, target := range []bool{false, true} { + for _, script := range []string{"daemons.sh", "release-local.sh", "test-distros.sh", "test-service.sh", "package.sh"} { + t.Run(fmt.Sprintf("%s/cross=%v", script, target), func(t *testing.T) { + root, shell := bootstrapFixture(t) + osName, arch := runtime.GOOS, runtime.GOARCH + if target { + osName, arch = "linux", "arm64" + if runtime.GOOS == osName && runtime.GOARCH == arch { + osName, arch = "windows", "amd64" + } + } + t.Setenv("GOOS", osName) + t.Setenv("GOARCH", arch) + cmd := quietcmd.Command(shell, "scripts/"+script, "test-version") + cmd.Dir = root + b, err := cmd.CombinedOutput() + if e, ok := err.(interface{ ExitCode() int }); !ok || e.ExitCode() != 73 { + t.Fatalf("wanted mocked build exit 73; got %v\n%s", err, b) + } + var child childBuild + b, err = os.ReadFile(os.Getenv("ENOWX_BOOTSTRAP_RECORD")) + if err != nil || json.Unmarshal(b, &child) != nil { + t.Fatalf("child record: %s %v", b, err) + } + // Daemon and Docker routes deliberately select Linux themselves. + wantOS, wantArch := osName, arch + if script == "daemons.sh" || script == "package.sh" || strings.HasPrefix(script, "test-") { + wantOS, wantArch = "linux", "amd64" + } + if child.OS != wantOS || child.Arch != wantArch || !strings.Contains(child.Flags, "-p=1") { + t.Fatalf("child target/flags changed: %+v; want %s/%s", child, wantOS, wantArch) + } + t.Logf("real preparer and Go parser selected %s; mocked child retained %s/%s and -p=1", child.Module, child.OS, child.Arch) + }) + } + } +} + +func TestBootstrapWrapperTarget(t *testing.T) { + root, shell := bootstrapFixture(t) + selected := filepath.Join(filepath.Dir(os.Getenv("HOST_GO")), "selected go.exe") + b, err := os.ReadFile(os.Getenv("HOST_GO")) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(selected, b, 0700); err != nil { + t.Fatal(err) + } + t.Setenv("HOST_GO", filepath.ToSlash(selected)) + t.Setenv("GOOS", "linux") + t.Setenv("GOARCH", "arm64") + cmd := quietcmd.Command(shell, "scripts/terminalpatch.sh", "--", "tool", "mygo", "build", "-platform", "darwin/universal", "cmd/enowx") + cmd.Dir = root + b, err = cmd.CombinedOutput() + if e, ok := err.(interface{ ExitCode() int }); !ok || e.ExitCode() != 73 { + t.Fatalf("wrapper: %v\n%s", err, b) + } + b, err = os.ReadFile(os.Getenv("ENOWX_BOOTSTRAP_RECORD")) + var child childBuild + if err != nil || json.Unmarshal(b, &child) != nil || child.OS != "linux" || child.Arch != "arm64" || strings.Join(child.Args, " ") != "tool mygo build -platform darwin/universal cmd/enowx" { + t.Fatalf("target or arguments changed: %s %v", b, err) + } + if _, err := os.Stat(child.Module); !os.IsNotExist(err) { + t.Fatalf("wrapper did not remove its private copy: %v", err) + } +} + +func TestMakeBootstrapTarget(t *testing.T) { + makeBin, err := exec.LookPath("make") + if err != nil { + t.Skip("GNU make is not installed") + } + root, _ := bootstrapFixture(t) + t.Setenv("GOOS", "linux") + t.Setenv("GOARCH", "arm64") + cmd := quietcmd.Command(makeBin, "run-daemon") + cmd.Dir = root + b, err := cmd.CombinedOutput() + // GNU make maps the child's sentinel to its own exit code 2. + if e, ok := err.(interface{ ExitCode() int }); !ok || e.ExitCode() != 2 { + t.Fatalf("make: %v\n%s", err, b) + } + b, err = os.ReadFile(os.Getenv("ENOWX_BOOTSTRAP_RECORD")) + var child childBuild + if err != nil || json.Unmarshal(b, &child) != nil || child.OS != "linux" || child.Arch != "arm64" { + t.Fatalf("make target lost: %s %v", b, err) + } +} + +func bootstrapFixture(t *testing.T) (string, string) { + t.Helper() + shell := os.Getenv("ENOWX_TEST_SH") + if shell == "" { + if runtime.GOOS == "windows" { + t.Skip("set ENOWX_TEST_SH to Git Bash for shell bootstrap checks") + } + shell = "/bin/sh" + } + // Every script is run in a disposable project, never in the checkout. + root := filepath.Join(t.TempDir(), "project with spaces") + if err := os.MkdirAll(filepath.Join(root, "scripts"), 0700); err != nil { + t.Fatal(err) + } + for _, dir := range []string{"scripts/terminalpatch", "internal/quietcmd"} { + if err := os.CopyFS(filepath.Join(root, dir), os.DirFS(filepath.Join("../..", dir))); err != nil { + t.Fatal(err) + } + } + for _, name := range []string{"go.mod", "go.sum", "Makefile", "scripts/daemons.sh", "scripts/release-local.sh", "scripts/test-distros.sh", "scripts/test-service.sh", "scripts/package.sh", "scripts/terminalpatch.sh"} { + b, err := os.ReadFile(filepath.Join("../..", name)) + if os.IsNotExist(err) && name == "scripts/terminalpatch.sh" { + continue // The regression also runs against the old source. + } + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, name), b, 0700); err != nil { + t.Fatal(err) + } + } + if err := os.MkdirAll(filepath.Join(root, "cmd/enowx"), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "cmd/enowx/mygo.json"), []byte("{\"version\": \"test\"}\n"), 0600); err != nil { + t.Fatal(err) + } + bin := filepath.Join(filepath.Dir(root), "bin") + if err := os.MkdirAll(bin, 0700); err != nil { + t.Fatal(err) + } + for name, body := range map[string]string{ + "docker": "#!/bin/sh\nif [ \"$1\" = info ]; then echo x86_64; fi\n", + "make": "#!/bin/sh\nexit 0\n", + } { + if err := os.WriteFile(filepath.Join(bin, name), []byte(body), 0700); err != nil { + t.Fatal(err) + } + } + // An executable fake Go can be invoked both by the shell and quietcmd + // on Windows; it forwards every inexpensive operation to the real Go. + exe, err := os.Executable() + if err != nil { + t.Fatal(err) + } + b, err := os.ReadFile(exe) + if err != nil { + t.Fatal(err) + } + fake := filepath.Join(bin, "go.exe") + if runtime.GOOS != "windows" { + fake = filepath.Join(bin, "go") + } + if err := os.WriteFile(fake, b, 0700); err != nil { + t.Fatal(err) + } + realGo := os.Getenv("ENOWX_TEST_GO") + if realGo == "" { + cmd := quietcmd.Command("go", "env", "GOROOT") + b, err := cmd.Output() + if err != nil { + t.Fatal(err) + } + realGo = filepath.Join(strings.TrimSpace(string(b)), "bin", "go") + if runtime.GOOS == "windows" { + realGo += ".exe" + } + } + // Forward slashes work for Go on Windows as well as the shell's PATH. + t.Setenv("HOST_GO", filepath.ToSlash(fake)) + t.Setenv("ENOWX_BOOTSTRAP_FAKE_GO", "1") + t.Setenv("ENOWX_BOOTSTRAP_REAL_GO", realGo) + t.Setenv("ENOWX_BOOTSTRAP_RECORD", filepath.Join(root, "child.json")) + t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv("GOFLAGS", "-p=1") + t.Setenv("ENOWX_TERMINAL_PATCH", "") + for _, name := range []string{"HOME", "USERPROFILE", "ENOWX_HOME", "CODEX_HOME", "APPDATA", "LOCALAPPDATA", "TMP", "TEMP", "TMPDIR", "GOTMPDIR"} { + dir := filepath.Join(root, name+" with spaces") + if err := os.MkdirAll(dir, 0700); err != nil { + t.Fatal(err) + } + t.Setenv(name, filepath.ToSlash(dir)) + } + return root, shell +} diff --git a/scripts/terminalpatch/main.go b/scripts/terminalpatch/main.go index a3458fb5..ca6e7d8c 100644 --- a/scripts/terminalpatch/main.go +++ b/scripts/terminalpatch/main.go @@ -21,9 +21,17 @@ var source []byte //go:embed guarded_input_test.go.txt var tests []byte +var goBinary = "go" + func main() { + flag.StringVar(&goBinary, "go", "go", "underlying Go executable (not a wrapper)") out := flag.String("out", "", "absolute private go.mod output path (required)") + printFlags := flag.Bool("print-go-flags", false, "print GOFLAGS for -out using native paths and Go token quoting") flag.Parse() + if *printFlags && *out == "" { + fmt.Fprintln(os.Stderr, "terminalpatch: -print-go-flags requires -out") + os.Exit(1) + } if err := run(*out, flag.Args()); err != nil { fmt.Fprintln(os.Stderr, "terminalpatch:", err) if e, ok := err.(interface{ ExitCode() int }); ok { @@ -31,6 +39,13 @@ func main() { } os.Exit(1) } + if *printFlags { + fmt.Println(modfileFlags(*out)) + } +} + +func modfileFlags(path string) string { + return os.Getenv("GOFLAGS") + fmt.Sprintf(" %q", "-modfile="+filepath.ToSlash(path)) } func run(out string, args []string) error { @@ -60,11 +75,11 @@ func run(out string, args []string) error { } func runGo(args []string, modfile string) error { - cmd := quietcmd.Command("go", args...) + cmd := quietcmd.Command(goBinary, args...) cmd.Stdin, cmd.Stdout, cmd.Stderr = os.Stdin, os.Stdout, os.Stderr cmd.Env = os.Environ() if modfile != "" { - cmd.Env = append(cmd.Env, "GOFLAGS="+os.Getenv("GOFLAGS")+fmt.Sprintf(" %q", "-modfile="+modfile), "ENOWX_TERMINAL_PATCH=v0.6.2") + cmd.Env = append(cmd.Env, "GOFLAGS="+modfileFlags(modfile), "ENOWX_TERMINAL_PATCH=v0.6.2") } return cmd.Run() } @@ -73,7 +88,7 @@ func prepare(out string) error { if !filepath.IsAbs(out) { return fmt.Errorf("-out must be an absolute path") } - cmd := quietcmd.Command("go", "list", "-m", "-json", "github.com/egoist/mygo") + cmd := quietcmd.Command(goBinary, "list", "-m", "-json", "github.com/egoist/mygo") b, err := cmd.Output() if err != nil { return fmt.Errorf("locate terminal module: %w", err) @@ -89,7 +104,7 @@ func prepare(out string) error { return fmt.Errorf("requires unmodified github.com/egoist/mygo v0.6.2; review patch when upgrading") } if mod.Dir == "" { - cmd = quietcmd.Command("go", "mod", "download", "-json", "github.com/egoist/mygo") + cmd = quietcmd.Command(goBinary, "mod", "download", "-json", "github.com/egoist/mygo") b, err = cmd.Output() if err != nil { return fmt.Errorf("download pinned terminal module: %w", err) diff --git a/scripts/test-distros.sh b/scripts/test-distros.sh index ee9d5e62..a89151d0 100755 --- a/scripts/test-distros.sh +++ b/scripts/test-distros.sh @@ -15,14 +15,14 @@ esac mkdir -p "$HOME/.cache" out=$(mktemp -d "$HOME/.cache/enowx-distros.XXXXXX") trap 'rm -rf "$out"' EXIT -go run ./scripts/terminalpatch -out "$out/go.mod" -export GOFLAGS="${GOFLAGS:-} -modfile=$out/go.mod" -CGO_ENABLED=0 GOOS=linux GOARCH=$goarch go build -o "$out/enowxd" ./cmd/enowxd -CGO_ENABLED=0 GOOS=linux GOARCH=$goarch go test -c -o "$out/setup.test" ./core/setup +flags=$(sh scripts/terminalpatch.sh -out "$out/go.mod" -print-go-flags) +export GOFLAGS="$flags" +CGO_ENABLED=0 GOOS=linux GOARCH=$goarch "${HOST_GO:-go}" build -o "$out/enowxd" ./cmd/enowxd +CGO_ENABLED=0 GOOS=linux GOARCH=$goarch "${HOST_GO:-go}" test -c -o "$out/setup.test" ./core/setup cp hostconn/prepare.sh "$out/" # Images there are no native build of run emulated. -CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o "$out/enowxd-amd64" ./cmd/enowxd -CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go test -c -o "$out/setup-amd64.test" ./core/setup +CGO_ENABLED=0 GOOS=linux GOARCH=amd64 "${HOST_GO:-go}" build -o "$out/enowxd-amd64" ./cmd/enowxd +CGO_ENABLED=0 GOOS=linux GOARCH=amd64 "${HOST_GO:-go}" test -c -o "$out/setup-amd64.test" ./core/setup failed="" for image in $images; do platform="" suffix="" diff --git a/scripts/test-service.sh b/scripts/test-service.sh index e3b5b30a..e197a452 100755 --- a/scripts/test-service.sh +++ b/scripts/test-service.sh @@ -13,9 +13,9 @@ mkdir -p "$HOME/.cache" out=$(mktemp -d "$HOME/.cache/enowx-service.XXXXXX") name=enowx-service-test trap 'docker rm -f $name >/dev/null 2>&1; rm -rf "$out"' EXIT -go run ./scripts/terminalpatch -out "$out/go.mod" -export GOFLAGS="${GOFLAGS:-} -modfile=$out/go.mod" -CGO_ENABLED=0 GOOS=linux GOARCH=$goarch go build -o "$out/enowxd" ./cmd/enowxd +flags=$(sh scripts/terminalpatch.sh -out "$out/go.mod" -print-go-flags) +export GOFLAGS="$flags" +CGO_ENABLED=0 GOOS=linux GOARCH=$goarch "${HOST_GO:-go}" build -o "$out/enowxd" ./cmd/enowxd cp hostconn/linger.sh "$out/" cat >"$out/Dockerfile" <<'DOCKERFILE' FROM ubuntu:24.04 From ca8639ebfd6bdf8de280a61ce08ec87499aa658f Mon Sep 17 00:00:00 2001 From: Ahimsa Jenar Bramsaifstyo Kusuma Date: Thu, 5 Nov 2026 02:26:39 +0700 Subject: [PATCH 5/6] Start delivery settling after the paste reaches the PTY A delayed input writer can drain paste and Enter back to back even though the enqueue timer expired 300ms apart. Wait for the FIFO write callback before starting that same interval, while retaining the final readiness guard and delivery ledger. Bound missing completion by program lifetime and a five-second write wait, and anchor delayed paste echo to its still-current input sequence. Signed-off-by: Ahimsa Jenar Bramsaifstyo Kusuma (cherry picked from commit 9009d02e17a54ffd3f2677551f3a51fc6196fe63) --- core/session/delivery.go | 38 +++++-- core/session/delivery_write_test.go | 101 ++++++++++++++++++ daemon/delivery.go | 21 +++- daemon/delivery_write_test.go | 38 +++++++ docs/chat-view.md | 2 +- scripts/terminalpatch/README.md | 20 +++- scripts/terminalpatch/guarded_input.go.txt | 12 ++- .../terminalpatch/guarded_input_test.go.txt | 69 ++++++++++++ 8 files changed, 288 insertions(+), 13 deletions(-) create mode 100644 core/session/delivery_write_test.go create mode 100644 daemon/delivery_write_test.go diff --git a/core/session/delivery.go b/core/session/delivery.go index 84016e14..6a5e0912 100644 --- a/core/session/delivery.go +++ b/core/session/delivery.go @@ -37,7 +37,15 @@ func (s *Session) DeliveryGuard() DeliveryGuard { // run only after those locks are released. Empty requires an untouched composer; // the commit permits the delivery's own pasted draft. func (m *Manager) DeliveryInput(s *Session, p []byte, guard DeliveryGuard, empty bool) (DeliveryGuard, bool) { - return m.input(s, p, nil, &guard, empty) + return m.input(s, p, nil, &guard, empty, nil) +} + +// DeliveryPaste also reports successful PTY completion, independently of queue +// acceptance. Failure or close need not signal written; the caller bounds it. +func (m *Manager) DeliveryPaste(s *Session, p []byte, guard DeliveryGuard) (DeliveryGuard, <-chan struct{}, bool) { + written := make(chan struct{}) + next, ok := m.input(s, p, nil, &guard, true, written) + return next, written, ok } // deliveryReadyLocked checks current readiness, not the snapshot before paste. @@ -55,7 +63,7 @@ func (s *Session) deliveryReadyLocked(guard DeliveryGuard) bool { // Input types p into the session and records it (see NoteInput). func (m *Manager) Input(s *Session, p []byte) { - m.input(s, p, nil, nil, false) + m.input(s, p, nil, nil, false, nil) } // InputSequence identifies the most recently queued input, independently of echo. @@ -68,11 +76,11 @@ func (s *Session) InputSequence() uint64 { // InputIfUnchanged queues p only if no other input followed sequence after. // It does not infer the contents of the program's composer from its screen. func (m *Manager) InputIfUnchanged(s *Session, p []byte, after uint64) bool { - _, ok := m.input(s, p, &after, nil, false) + _, ok := m.input(s, p, &after, nil, false, nil) return ok } -func (m *Manager) input(s *Session, p []byte, after *uint64, guard *DeliveryGuard, empty bool) (DeliveryGuard, bool) { +func (m *Manager) input(s *Session, p []byte, after *uint64, guard *DeliveryGuard, empty bool, written chan struct{}) (DeliveryGuard, bool) { // Lock order: manager (guarded delivery only), input, session. Send only // enqueues bytes; prompt callbacks and notifications run after unlocking. if guard != nil { @@ -95,10 +103,26 @@ func (m *Manager) input(s *Session, p []byte, after *uint64, guard *DeliveryGuar } return DeliveryGuard{}, false } + var next DeliveryGuard + var onWritten func() + if written != nil { + onWritten = func() { + // Delayed PTY writes echo after enqueue-time suppression expires. + // Anchor only this still-current input's echo window to its write. + s.inputMu.Lock() + s.mu.Lock() + if s.inputSeq == next.input && s.deliveryEpoch == next.epoch { + s.lastInput = time.Now() + } + s.mu.Unlock() + s.inputMu.Unlock() + close(written) + } + } if guard != nil { - if !s.term.SendChecked(p, func(screen string) bool { + if !s.term.SendCheckedWritten(p, func(screen string) bool { return agent.PromptFree(screen) && (!empty || agent.PromptEmpty(screen)) - }) { + }, onWritten) { s.deliveryPending = false s.mu.Unlock() s.inputMu.Unlock() @@ -117,7 +141,7 @@ func (m *Manager) input(s *Session, p []byte, after *uint64, guard *DeliveryGuar prompted := s.noteInputLocked(p) changed := s.stateChanged(time.Now()) sent := s.lines.feed(p) - next := DeliveryGuard{input: s.inputSeq, epoch: s.deliveryEpoch} + next = DeliveryGuard{input: s.inputSeq, epoch: s.deliveryEpoch} s.mu.Unlock() s.inputMu.Unlock() if guard != nil { diff --git a/core/session/delivery_write_test.go b/core/session/delivery_write_test.go new file mode 100644 index 00000000..c4758575 --- /dev/null +++ b/core/session/delivery_write_test.go @@ -0,0 +1,101 @@ +package session + +import ( + "io" + "sync" + "testing" + "time" + + "github.com/egoist/mygo/plugins/terminal" + "github.com/enowx/enowx/proto" +) + +type deliveryWriteConn struct { + entered, release, closed chan struct{} + enterOnce, releaseOnce, closeOnce sync.Once +} + +func (c *deliveryWriteConn) Read([]byte) (int, error) { <-c.closed; return 0, io.EOF } +func (c *deliveryWriteConn) Write(p []byte) (int, error) { + c.enterOnce.Do(func() { close(c.entered) }) + select { + case <-c.release: + return len(p), nil + case <-c.closed: + return 0, io.ErrClosedPipe + } +} +func (c *deliveryWriteConn) Close() error { c.closeOnce.Do(func() { close(c.closed) }); return nil } +func (c *deliveryWriteConn) resume() { c.releaseOnce.Do(func() { close(c.release) }) } + +func TestDeliveryPasteCompletionKeepsCancellationHistory(t *testing.T) { + for _, mode := range []string{"current", "manual", "hook"} { + t.Run(mode, func(t *testing.T) { + conn := &deliveryWriteConn{entered: make(chan struct{}), release: make(chan struct{}), closed: make(chan struct{})} + term, err := terminal.New(terminal.Options{Conn: conn}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { conn.resume(); term.Close() }) + term.Resize(80, 24) + term.Feed([]byte("› Ask Codex to do anything")) + s := &Session{id: "write", term: term, done: make(chan struct{}), state: proto.StateDone} + m := NewManager() + m.sessions[s.id] = s + next, written, ok := m.DeliveryPaste(s, []byte("\x1b[200~DRAFT\x1b[201~"), s.DeliveryGuard()) + if !ok { + t.Fatal("paste refused") + } + select { + case <-conn.entered: + case <-time.After(time.Second): + t.Fatal("writer not entered") + } + select { + case <-written: + t.Fatal("acknowledged pending write") + default: + } + s.mu.Lock() + s.lastInput = time.Now().Add(-time.Hour) + s.mu.Unlock() + switch mode { + case "manual": + m.Input(s, []byte("USER")) + case "hook": + m.SetState(s, proto.StateWaiting) + m.SetState(s, proto.StateDone) + } + s.mu.Lock() + before := s.lastInput + s.mu.Unlock() + conn.resume() + select { + case <-written: + case <-time.After(time.Second): + t.Fatal("write completion stranded") + } + s.mu.Lock() + after := s.lastInput + s.mu.Unlock() + if mode == "current" { + if !after.After(before) { + t.Fatal("echo window still anchored to enqueue") + } + s.noteScreen(time.Now(), true) + s.mu.Lock() + changed := !s.lastChange.IsZero() + s.mu.Unlock() + if changed { + t.Fatal("delayed paste echo counted as agent activity") + } + } else if !after.Equal(before) { + t.Fatal("write receipt overwrote cancellation history") + } + _, committed := m.DeliveryInput(s, []byte("\r"), next, false) + if committed != (mode == "current") { + t.Fatalf("Enter after %s: %v", mode, committed) + } + }) + } +} diff --git a/daemon/delivery.go b/daemon/delivery.go index 5b7b224f..4682fe79 100644 --- a/daemon/delivery.go +++ b/daemon/delivery.go @@ -31,6 +31,10 @@ type deliveryAttempt struct { const maxDeliveriesPerRun = 4096 +// A program that stops reading must not strand the ledger or a retry forever. +// This bounds only the new write-completion wait, not the paste settle delay. +const deliveryWriteLimit = 5 * time.Second + func (s *Server) deliver(p proto.DeliveryParams) (proto.DeliveryResult, error) { sess, err := s.sessions.Get(p.ID) if err != nil { @@ -78,7 +82,12 @@ func (s *Server) deliver(p proto.DeliveryParams) (proto.DeliveryResult, error) { r.entries[p.Key], r.sending = a, true d.mu.Unlock() - next, pasted := s.sessions.DeliveryInput(sess, []byte(pasteStart+p.Text+pasteEnd), guard, true) + next, written, pasted := s.sessions.DeliveryPaste(sess, []byte(pasteStart+p.Text+pasteEnd), guard) + if pasted { + limit := time.NewTimer(deliveryWriteLimit) + pasted = deliveryWritten(written, sess.Done(), limit.C) + limit.Stop() + } if !pasted { a.result.Interrupted = true } else { @@ -98,3 +107,13 @@ func (s *Server) deliver(p proto.DeliveryParams) (proto.DeliveryResult, error) { d.mu.Unlock() return a.result, nil } + +func deliveryWritten(written, stopped <-chan struct{}, expired <-chan time.Time) bool { + select { + case <-written: + return true + case <-stopped: + case <-expired: + } + return false +} diff --git a/daemon/delivery_write_test.go b/daemon/delivery_write_test.go new file mode 100644 index 00000000..a1ce24ef --- /dev/null +++ b/daemon/delivery_write_test.go @@ -0,0 +1,38 @@ +package daemon + +import ( + "testing" + "time" +) + +func TestDeliveryWriteCompletionOrCancellation(t *testing.T) { + for _, kind := range []string{"written", "closed", "deadline"} { + t.Run(kind, func(t *testing.T) { + written, stopped := make(chan struct{}), make(chan struct{}) + expired := make(chan time.Time, 1) + result := make(chan bool, 1) + go func() { result <- deliveryWritten(written, stopped, expired) }() + select { + case <-result: + t.Fatal("advanced before a write or cancellation") + default: + } + switch kind { + case "written": + close(written) + case "closed": + close(stopped) + case "deadline": + expired <- time.Now() + } + select { + case got := <-result: + if got != (kind == "written") { + t.Fatalf("write outcome: %v", got) + } + case <-time.After(time.Second): + t.Fatal("write wait stranded") + } + }) + } +} diff --git a/docs/chat-view.md b/docs/chat-view.md index cbbd1d18..56edb753 100644 --- a/docs/chat-view.md +++ b/docs/chat-view.md @@ -28,7 +28,7 @@ Beside a wide conversation, **Your messages** lists the user's messages, the one A message sent shows immediately, with the composer emptied. Messages waiting for a busy agent, a permission question, or an earlier unconfirmed message stay **Queued in Chat**: their text has not been pasted into the terminal. The daemon accepts a new delivery only when the screen shows a free, empty composer. An existing draft or menu must be handled in the terminal. -Chat uses `session.deliver`, with one key for the message and the program's run. The daemon queues its bracketed paste and a delayed Enter once; retries with that same key return the original result, including after a lost RPC response. Input typed into the terminal during the paste delay cancels the automatic Enter. It never clears a draft or presses Enter again to recover a delivery. A program restart rejects an old run's delivery. Older daemons ask the user to reconnect for an update; there is no fallback to unprotected input. The existing `session.input` RPC and terminal streams keep their behavior for older apps and the CLI. +Chat uses `session.deliver`, with one key for the message and the program's run. The daemon queues its bracketed paste once, waits for that paste's successful PTY write, then starts the 300ms settle interval before a guarded Enter. A closed program or a write still unconfirmed after five seconds interrupts delivery without another paste or Enter. Retries with that same key return the original result, including after a lost RPC response. Input typed into the terminal during the write or paste delay cancels the automatic Enter. It never clears a draft or presses Enter again to recover a delivery. A program restart rejects an old run's delivery. Older daemons ask the user to reconnect for an update; there is no fallback to unprotected input. The existing `session.input` RPC and terminal streams keep their behavior for older apps and the CLI. Transport success and screen echo are not acknowledgement. Only a newly recorded user entry matching the whole message and its attachment paths replaces its pending row. A message unrecorded for 45 seconds with the agent free is **unconfirmed**, not proven lost. A successful or interrupted terminal send offers **Show terminal**, without repasting or restoring an uncertain message to the Chat composer. **Retry** after a transport error uses the same key. **Stop tracking** removes only the pending row after the user has reviewed the terminal. A changed conversation never acknowledges a previous conversation's pending messages. Transcript formats that rewrite text or attachment paths may require manual review rather than automatic matching. diff --git a/scripts/terminalpatch/README.md b/scripts/terminalpatch/README.md index 9da13a70..cf544bf0 100644 --- a/scripts/terminalpatch/README.md +++ b/scripts/terminalpatch/README.md @@ -2,8 +2,9 @@ MyGo v0.6.2 has separate `Text()` and `Send()` operations. A permission/menu redraw can complete after the text sample but before Enter is enqueued. Its -public API has no operation that covers both. This directory adds one method, -`Terminal.SendChecked(data, allow) bool`, inside the terminal package. +public API has no operation that covers both. This directory adds +`Terminal.SendChecked(data, allow) bool` and its write-completion variant, +`Terminal.SendCheckedWritten(data, allow, written) bool`, inside that package. The addition is in `guarded_input.go.txt`, with tests in `guarded_input_test.go.txt`. They are Go source stored as embedded resources so @@ -25,6 +26,16 @@ is later in this ordering. An unsafe screen that appeared and disappeared before inspection is detected only if the session observed it during settling; this API does not promise a historical classification of every raw redraw. +`SendCheckedWritten` uses the existing FIFO's completion callback, invoked only +after a successful PTY write, outside terminal/queue locks. It does not fire for +discarded or failed writes. Chat delivery bounds the wait by program lifetime +and a five-second write deadline; failure is retained as an interrupted delivery +and never causes another paste or Enter. The unchanged 300ms settle interval +starts after paste write completion, and the original readiness/history guard +still covers the final Enter enqueue. The session anchors suppression of that +paste's echo to completion only while its input sequence and epoch still match. +A successful write still does not acknowledge native message consumption. + ## Reproducible use Use the shell entrypoint from the repository root with Go 1.27.3+ (Git Bash @@ -102,3 +113,8 @@ old session's `Text`/predicate/`Send` sequence, not a native permission fixture. The queue test separately checks FIFO order, copy ownership and refusal after closure. Existing daemon-protocol and native fixture tests cover the delivery ledger, interrupted retries, manual drafts and lost responses. + +`TestEnowxCheckedInputWriteCompletion` holds a writer with channels, proving +enqueue and queue close do not acknowledge a pending write, and failed writes +do not invoke completion. `TestDeliveryPasteCompletionKeepsCancellationHistory` +checks delayed echo suppression and preserves manual-input/hook invalidation. diff --git a/scripts/terminalpatch/guarded_input.go.txt b/scripts/terminalpatch/guarded_input.go.txt index 5267ff50..33f49617 100644 --- a/scripts/terminalpatch/guarded_input.go.txt +++ b/scripts/terminalpatch/guarded_input.go.txt @@ -5,6 +5,14 @@ package terminal // check that never reenters the terminal. Screen writes and queue insertion // cannot interleave. Success means queued, not written to or read by the PTY. func (t *Terminal) SendChecked(data []byte, allow func(string) bool) bool { + return t.SendCheckedWritten(data, allow, nil) +} + +// SendCheckedWritten also calls written after the PTY write succeeds. The +// callback runs without terminal or queue locks and must return promptly. +// It is not called for discarded input or failed writes; callers must +// separately bound their wait and handle program cancellation. +func (t *Terminal) SendCheckedWritten(data []byte, allow func(string) bool, written func()) bool { t.mu.Lock() defer t.mu.Unlock() // Same lock order as terminal replies: terminal -> input queue. @@ -13,9 +21,9 @@ func (t *Terminal) SendChecked(data []byte, allow func(string) bool) bool { if t.closed || t.term == nil || t.in.closed || allow == nil || !allow(t.term.Text()) { return false } - if len(data) > 0 { + if len(data) > 0 || written != nil { t.in.init() - t.in.queue = append(t.in.queue, queued{p: append([]byte(nil), data...)}) + t.in.queue = append(t.in.queue, queued{p: append([]byte(nil), data...), done: written}) t.in.cond.Signal() } return true diff --git a/scripts/terminalpatch/guarded_input_test.go.txt b/scripts/terminalpatch/guarded_input_test.go.txt index 6b3fd4b1..ecfaf44a 100644 --- a/scripts/terminalpatch/guarded_input_test.go.txt +++ b/scripts/terminalpatch/guarded_input_test.go.txt @@ -1,12 +1,81 @@ package terminal import ( + "io" "strings" "testing" + "time" "github.com/egoist/mygo/plugins/terminal/internal/vt" ) +type enowxGateWriter struct { + release <-chan struct{} + start chan<- struct{} + err error +} + +func (w enowxGateWriter) Write(p []byte) (int, error) { + close(w.start) + <-w.release + if w.err != nil { + return 0, w.err + } + return len(p), nil +} + +func TestEnowxCheckedInputWriteCompletion(t *testing.T) { + for _, fail := range []bool{false, true} { + t.Run(map[bool]string{false: "success", true: "failure"}[fail], func(t *testing.T) { + term := guardedFixture(t) + entered, release, written, stopped := make(chan struct{}), make(chan struct{}), make(chan struct{}), make(chan struct{}) + var writeErr error + if fail { + writeErr = io.ErrClosedPipe + } + if !term.SendCheckedWritten([]byte("PASTE"), func(string) bool { return true }, func() { close(written) }) { + t.Fatal("refused paste") + } + go func() { + defer close(stopped) + term.in.run(enowxGateWriter{start: entered, release: release, err: writeErr}) + }() + select { + case <-entered: + case <-time.After(time.Second): + t.Fatal("writer not entered") + } + select { + case <-written: + t.Fatal("acknowledged enqueue as write completion") + default: + } + term.in.close() // Also prove closing the queue doesn't acknowledge an in-flight write. + select { + case <-written: + t.Fatal("acknowledged queue close as a write") + default: + } + close(release) + select { + case <-stopped: + case <-time.After(time.Second): + t.Fatal("writer stranded") + } + select { + case <-written: + if fail { + t.Fatal("acknowledged failed write") + } + default: + if !fail { + t.Fatal("missing successful write completion") + } + } + }) + } +} + func guardedFixture(t *testing.T) *Terminal { t.Helper() if err := Load(); err != nil { From 1255f9468b2746f91c9a3194bea80267b2f9dc96 Mon Sep 17 00:00:00 2001 From: Ahimsa Jenar Bramsaifstyo Kusuma Date: Thu, 5 Nov 2026 08:12:00 +0700 Subject: [PATCH 6/6] Quarantine automatic delivery after an interrupted paste A timed-out paste can remain ahead of later messages in the terminal FIFO. Retain run-level uncertainty after any interrupted delivery that queued its paste, so a different key cannot send Enter behind those bytes. Preserve the original same-key result and bounded write wait; only a replacement program gets a fresh gate. Signed-off-by: Ahimsa Jenar Bramsaifstyo Kusuma (cherry picked from commit a3c4a57e7ce44cb55cd3590481ba8e2d47f7d131) --- daemon/delivery.go | 14 ++++- daemon/delivery_uncertainty_test.go | 91 +++++++++++++++++++++++++++++ docs/chat-view.md | 7 +++ scripts/terminalpatch/README.md | 7 +++ 4 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 daemon/delivery_uncertainty_test.go diff --git a/daemon/delivery.go b/daemon/delivery.go index 4682fe79..e05293d4 100644 --- a/daemon/delivery.go +++ b/daemon/delivery.go @@ -20,7 +20,9 @@ type deliveryState struct { type deliveryRun struct { sending bool - entries map[string]*deliveryAttempt + // Only program replacement resolves an interrupted paste's draft uncertainty. + uncertain bool + entries map[string]*deliveryAttempt } type deliveryAttempt struct { @@ -74,6 +76,10 @@ func (s *Server) deliver(p proto.DeliveryParams) (proto.DeliveryResult, error) { <-a.done return a.result, nil } + if r.uncertain { + d.mu.Unlock() + return proto.DeliveryResult{}, fmt.Errorf("a previous chat delivery left an unconfirmed paste; restart the session's program before sending another chat message") + } if r.sending || len(r.entries) >= maxDeliveriesPerRun || working(info) || info.State == proto.StateWaiting || !ready { d.mu.Unlock() return proto.DeliveryResult{}, fmt.Errorf("the session isn't ready for a new chat delivery; check its terminal") @@ -83,6 +89,7 @@ func (s *Server) deliver(p proto.DeliveryParams) (proto.DeliveryResult, error) { d.mu.Unlock() next, written, pasted := s.sessions.DeliveryPaste(sess, []byte(pasteStart+p.Text+pasteEnd), guard) + queued := pasted if pasted { limit := time.NewTimer(deliveryWriteLimit) pasted = deliveryWritten(written, sess.Done(), limit.C) @@ -102,6 +109,11 @@ func (s *Server) deliver(p proto.DeliveryParams) (proto.DeliveryResult, error) { } } d.mu.Lock() + // A missing receipt or refused Enter can leave bytes queued or a late draft. + // Neither a later receipt nor an empty screen proves that draft was resolved. + if queued && a.result.Interrupted { + r.uncertain = true + } r.sending = false close(a.done) d.mu.Unlock() diff --git a/daemon/delivery_uncertainty_test.go b/daemon/delivery_uncertainty_test.go new file mode 100644 index 00000000..75a28394 --- /dev/null +++ b/daemon/delivery_uncertainty_test.go @@ -0,0 +1,91 @@ +package daemon + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/enowx/enowx/core/session" + "github.com/enowx/enowx/proto" +) + +func TestDeliveryInterruptedPasteRequiresProgramReplacement(t *testing.T) { + for _, mode := range []string{"manual", "hook"} { + t.Run(mode, func(t *testing.T) { + srv, c, info, path := deliveryFixture(t) + sess, _ := srv.sessions.Get(info.ID) + p := proto.DeliveryParams{ID: info.ID, Run: info.Run, Key: "old", Text: "UNCERTAIN_DRAFT"} + type answer struct { + r proto.DeliveryResult + err error + } + done := make(chan answer, 1) + go func() { r, e := c.Deliver(p); done <- answer{r, e} }() + waitDelivery(t, func() bool { return strings.Contains(sess.Terminal().Text(), p.Text) }) + if mode == "manual" { + srv.sessions.Input(sess, []byte("USER")) + } else { + srv.sessions.SetState(sess, proto.StateWaiting) + srv.sessions.SetState(sess, proto.StateDone) + } + select { + case a := <-done: + if a.err != nil || !a.r.Interrupted { + t.Fatalf("delivery not interrupted: %+v %v", a.r, a.err) + } + case <-time.After(deliveryWriteLimit + time.Second): + t.Fatal("interrupted delivery did not finish") + } + for range 3 { + if r, e := c.Deliver(p); e != nil || !r.Interrupted { + t.Fatalf("original result lost: %+v %v", r, e) + } + } + assertBlocked := func(key string) { + t.Helper() + // A stale empty screen must not resolve a draft in the actual program. + sess.Terminal().Feed([]byte("\x1b[2J\x1b[H› Ask Codex to do anything")) + next := p + next.Key = key + next.Text = "NEVER_PASTED" + if _, e := c.Deliver(next); e == nil || !strings.Contains(e.Error(), "unconfirmed paste") { + t.Fatalf("new key allowed: %v", e) + } + } + assertBlocked("empty-screen") + // Deliberate terminal input remains available, but cannot prove safe + // automatic recovery of all older queued bytes for this program run. + srv.sessions.Input(sess, []byte("\r")) + waitDelivery(t, func() bool { return len(capturedLines(path)) > 0 }) + if got := strings.Join(capturedLines(path), ""); strings.Count(got, p.Text) != 1 || strings.Contains(got, "NEVER_PASTED") { + t.Fatalf("draft changed: %q", got) + } + srv.sessions.SetState(sess, proto.StateDone) + assertBlocked("manual-recovery-and-empty-screen") + + bin, err := os.Executable() + if err != nil { + t.Fatal(err) + } + newPath := filepath.Join(t.TempDir(), "replacement.jsonl") + replacement, err := srv.sessions.Replace(session.Spec{ID: info.ID, Command: []string{bin, "-test.run=^TestDeliveryInputCapture$"}, Env: []string{"ENOWX_DELIVERY_CAPTURE=" + newPath}, Cols: 100, Rows: 24}) + if err != nil { + t.Fatal(err) + } + waitDelivery(t, func() bool { b, _ := os.ReadFile(newPath); return strings.HasPrefix(string(b), "ready\n") }) + srv.sessions.SetState(replacement, proto.StateDone) + waitDelivery(t, func() bool { return strings.Contains(replacement.Terminal().Text(), "›") }) + if _, e := c.Deliver(p); e == nil { + t.Fatal("old run accepted after program replacement") + } + p.Run, p.Key, p.Text = replacement.Info().Run, "fresh", "FRESH_PROGRAM" + if r, e := c.Deliver(p); e != nil || r.Interrupted { + t.Fatalf("replacement still quarantined: %+v %v", r, e) + } + waitDelivery(t, func() bool { return strings.Contains(strings.Join(capturedLines(newPath), ""), p.Text) }) + t.Log("new keys refused after manual/hook cancellation and manual recovery; replacement accepts its own clean delivery") + }) + } +} diff --git a/docs/chat-view.md b/docs/chat-view.md index 56edb753..593fa084 100644 --- a/docs/chat-view.md +++ b/docs/chat-view.md @@ -30,6 +30,13 @@ A message sent shows immediately, with the composer emptied. Messages waiting fo Chat uses `session.deliver`, with one key for the message and the program's run. The daemon queues its bracketed paste once, waits for that paste's successful PTY write, then starts the 300ms settle interval before a guarded Enter. A closed program or a write still unconfirmed after five seconds interrupts delivery without another paste or Enter. Retries with that same key return the original result, including after a lost RPC response. Input typed into the terminal during the write or paste delay cancels the automatic Enter. It never clears a draft or presses Enter again to recover a delivery. A program restart rejects an old run's delivery. Older daemons ask the user to reconnect for an update; there is no fallback to unprotected input. The existing `session.input` RPC and terminal streams keep their behavior for older apps and the CLI. +If delivery is interrupted after its paste was queued, new chat delivery keys +are blocked for that program run: the paste may still arrive later or leave a +draft. A late write receipt, an empty screen, hooks, or manual terminal input +cannot establish that all earlier bytes are resolved. Manual input remains +available, but automatic delivery requires restarting the session's program. +The new program has its own delivery ledger and still checks readiness. + Transport success and screen echo are not acknowledgement. Only a newly recorded user entry matching the whole message and its attachment paths replaces its pending row. A message unrecorded for 45 seconds with the agent free is **unconfirmed**, not proven lost. A successful or interrupted terminal send offers **Show terminal**, without repasting or restoring an uncertain message to the Chat composer. **Retry** after a transport error uses the same key. **Stop tracking** removes only the pending row after the user has reviewed the terminal. A changed conversation never acknowledges a previous conversation's pending messages. Transcript formats that rewrite text or attachment paths may require manual review rather than automatic matching. A command (`/model`) shows no bubble: agents don't record those as messages. Slash commands and settings still use their existing terminal input path. diff --git a/scripts/terminalpatch/README.md b/scripts/terminalpatch/README.md index cf544bf0..62f907f6 100644 --- a/scripts/terminalpatch/README.md +++ b/scripts/terminalpatch/README.md @@ -36,6 +36,13 @@ still covers the final Enter enqueue. The session anchors suppression of that paste's echo to completion only while its input sequence and epoch still match. A successful write still does not acknowledge native message consumption. +An interrupted delivery after paste enqueue also quarantines new delivery keys +for that program run. The original key retains its interrupted result. Late +write completion, elapsed time, empty screens and manual/hook changes do not +clear uncertainty about queued bytes or a draft. Only replacement of the +program creates a fresh automatic-delivery ledger; manual terminal input is +still available without resetting that gate. + ## Reproducible use Use the shell entrypoint from the repository root with Go 1.27.3+ (Git Bash