From e764240d9ceb74a626cccdd404afaaa76e128314 Mon Sep 17 00:00:00 2001 From: Christian Rocha Date: Tue, 19 May 2026 23:30:36 -0400 Subject: [PATCH] feat(api): expose in progress flag on session responses REST responses for sessions now include a boolean indicating whether an agent run is currently active for that session. Clients and UI components can use this to surface an in progress hint in session pickers and similar views without having to subscribe to agent events. The flag is computed at read time and is safe to ignore for older consumers. Co-Authored-By: Charm Crush --- internal/backend/testing.go | 20 +++ internal/proto/session.go | 7 + internal/server/events.go | 13 ++ internal/server/proto.go | 17 ++- internal/server/sessions_isbusy_test.go | 176 ++++++++++++++++++++++++ internal/workspace/client_workspace.go | 7 + 6 files changed, 237 insertions(+), 3 deletions(-) create mode 100644 internal/backend/testing.go create mode 100644 internal/server/sessions_isbusy_test.go diff --git a/internal/backend/testing.go b/internal/backend/testing.go new file mode 100644 index 0000000000000000000000000000000000000000..ace37c4b2153308ad165564525bdb46a2446f0c9 --- /dev/null +++ b/internal/backend/testing.go @@ -0,0 +1,20 @@ +package backend + +// InsertWorkspaceForTest registers ws with b under its current ID and +// path. It is intended for tests in other packages that need to drive +// HTTP handlers against a synthetic workspace without booting a real +// app.App. Production code should go through CreateWorkspace. +func InsertWorkspaceForTest(b *Backend, ws *Workspace) { + if ws.resolvedPath == "" { + ws.resolvedPath = ws.Path + } + if ws.clients == nil { + ws.clients = make(map[string]*clientState) + } + b.mu.Lock() + defer b.mu.Unlock() + b.workspaces.Set(ws.ID, ws) + if ws.resolvedPath != "" { + b.pathIndex[ws.resolvedPath] = ws.ID + } +} diff --git a/internal/proto/session.go b/internal/proto/session.go index 6c7aca7bd8b010d44e39ee582e03edaa7cea5a66..4652065ac881f4fe06bfbc019164cf5cdcaf8caf 100644 --- a/internal/proto/session.go +++ b/internal/proto/session.go @@ -1,6 +1,12 @@ package proto // Session represents a session in the proto layer. +// +// IsBusy is computed on read (it is not persisted with the session) and +// reflects whether an agent run is currently in flight for this session. +// It is populated by REST handlers in internal/server/proto.go from the +// workspace's AgentCoordinator. The Session SSE event path does not set +// it, since SSE consumers can compute presence from other agent signals. type Session struct { ID string `json:"id"` ParentSessionID string `json:"parent_session_id"` @@ -13,6 +19,7 @@ type Session struct { Todos []Todo `json:"todos,omitempty"` CreatedAt int64 `json:"created_at"` UpdatedAt int64 `json:"updated_at"` + IsBusy bool `json:"is_busy"` } // Todo represents a single todo entry on a session in the proto layer. diff --git a/internal/server/events.go b/internal/server/events.go index 20ac0fd3fd66178bb6ef4a706d2f9afd3bdb097b..2e6fcd92b6f982b7c1f597b049908cd295826a3e 100644 --- a/internal/server/events.go +++ b/internal/server/events.go @@ -8,6 +8,7 @@ import ( "github.com/charmbracelet/crush/internal/agent/notify" "github.com/charmbracelet/crush/internal/agent/tools/mcp" "github.com/charmbracelet/crush/internal/app" + "github.com/charmbracelet/crush/internal/backend" "github.com/charmbracelet/crush/internal/history" "github.com/charmbracelet/crush/internal/message" "github.com/charmbracelet/crush/internal/permission" @@ -143,6 +144,18 @@ func sessionToProto(s session.Session) proto.Session { } } +// isSessionBusy reports whether the given workspace has an in-flight +// agent run for sessionID. It tolerates a nil workspace (treating it as +// "not busy") so REST handlers can pass GetWorkspace's result through +// unconditionally — the workspace lookup error is already surfaced by +// the prior ListSessions/GetSession call when relevant. +func isSessionBusy(ws *backend.Workspace, sessionID string) bool { + if ws == nil || ws.App == nil || ws.AgentCoordinator == nil { + return false + } + return ws.AgentCoordinator.IsSessionBusy(sessionID) +} + func todosToProto(todos []session.Todo) []proto.Todo { if len(todos) == 0 { return nil diff --git a/internal/server/proto.go b/internal/server/proto.go index c73827db1fad57bf2684fec9ead82d7a8529fbc1..1f054051bb5f6ed6ce9094cc5cffffd2e09e837f 100644 --- a/internal/server/proto.go +++ b/internal/server/proto.go @@ -343,9 +343,11 @@ func (c *controllerV1) handleGetWorkspaceSessions(w http.ResponseWriter, r *http c.handleError(w, r, err) return } + ws, _ := c.backend.GetWorkspace(id) result := make([]proto.Session, len(sessions)) for i, s := range sessions { result[i] = sessionToProto(s) + result[i].IsBusy = isSessionBusy(ws, s.ID) } jsonEncode(w, result) } @@ -378,7 +380,10 @@ func (c *controllerV1) handlePostWorkspaceSessions(w http.ResponseWriter, r *htt c.handleError(w, r, err) return } - jsonEncode(w, sessionToProto(sess)) + ws, _ := c.backend.GetWorkspace(id) + out := sessionToProto(sess) + out.IsBusy = isSessionBusy(ws, sess.ID) + jsonEncode(w, out) } // handleGetWorkspaceSession returns a single session. @@ -400,7 +405,10 @@ func (c *controllerV1) handleGetWorkspaceSession(w http.ResponseWriter, r *http. c.handleError(w, r, err) return } - jsonEncode(w, sessionToProto(sess)) + ws, _ := c.backend.GetWorkspace(id) + out := sessionToProto(sess) + out.IsBusy = isSessionBusy(ws, sess.ID) + jsonEncode(w, out) } // handleGetWorkspaceSessionHistory returns the history for a session. @@ -476,7 +484,10 @@ func (c *controllerV1) handlePutWorkspaceSession(w http.ResponseWriter, r *http. c.handleError(w, r, err) return } - jsonEncode(w, sessionToProto(saved)) + ws, _ := c.backend.GetWorkspace(id) + out := sessionToProto(saved) + out.IsBusy = isSessionBusy(ws, saved.ID) + jsonEncode(w, out) } // handleDeleteWorkspaceSession deletes a session. diff --git a/internal/server/sessions_isbusy_test.go b/internal/server/sessions_isbusy_test.go new file mode 100644 index 0000000000000000000000000000000000000000..f5127ef6e4fec7b524781d6444c2cf3b5c7f3ea0 --- /dev/null +++ b/internal/server/sessions_isbusy_test.go @@ -0,0 +1,176 @@ +package server + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + + "charm.land/fantasy" + "github.com/charmbracelet/crush/internal/agent" + "github.com/charmbracelet/crush/internal/app" + "github.com/charmbracelet/crush/internal/backend" + "github.com/charmbracelet/crush/internal/message" + "github.com/charmbracelet/crush/internal/proto" + "github.com/charmbracelet/crush/internal/session" + "github.com/google/uuid" + "github.com/stretchr/testify/require" +) + +// stubCoordinator is a minimal agent.Coordinator that only reports +// per-session busy state. Every other method returns a zero value so +// the type satisfies the interface without dragging in the full +// coordinator dependency graph. +type stubCoordinator struct { + busy map[string]bool +} + +func (s *stubCoordinator) Run(ctx context.Context, sessionID, prompt string, attachments ...message.Attachment) (*fantasy.AgentResult, error) { + return nil, nil +} +func (s *stubCoordinator) Cancel(string) {} +func (s *stubCoordinator) CancelAll() {} +func (s *stubCoordinator) IsBusy() bool { return false } +func (s *stubCoordinator) IsSessionBusy(id string) bool { + return s.busy[id] +} +func (s *stubCoordinator) QueuedPrompts(string) int { return 0 } +func (s *stubCoordinator) QueuedPromptsList(string) []string { return nil } +func (s *stubCoordinator) ClearQueue(string) {} +func (s *stubCoordinator) Summarize(context.Context, string) error { + return nil +} +func (s *stubCoordinator) Model() agent.Model { return agent.Model{} } +func (s *stubCoordinator) UpdateModels(context.Context) error { return nil } + +// stubSessions is a minimal session.Service that returns a fixed list +// (and supports Get by ID). All other methods return zero values; the +// IsBusy tests do not exercise them. +type stubSessions struct { + session.Service // embed nil to inherit the unexported broker methods + all []session.Session +} + +func (s *stubSessions) List(context.Context) ([]session.Session, error) { + return s.all, nil +} + +func (s *stubSessions) Get(_ context.Context, id string) (session.Session, error) { + for _, sess := range s.all { + if sess.ID == id { + return sess, nil + } + } + return session.Session{}, errors.New("not found") +} + +// buildBusyWorkspace returns a controller wired to a backend that owns +// a single workspace whose AgentCoordinator reports the named session +// as busy. +func buildBusyWorkspace(t *testing.T, sessionID string, busy bool) (*controllerV1, string) { + t.Helper() + + b := backend.New(context.Background(), nil, nil) + wsID := uuid.New().String() + coord := &stubCoordinator{busy: map[string]bool{sessionID: busy}} + a := &app.App{AgentCoordinator: coord} + a.Sessions = &stubSessions{all: []session.Session{{ID: sessionID, Title: "t"}}} + + ws := &backend.Workspace{ + ID: wsID, + Path: t.TempDir(), + App: a, + } + backend.InsertWorkspaceForTest(b, ws) + + s := &Server{backend: b} + return &controllerV1{backend: b, server: s}, wsID +} + +func TestSessionListIncludesIsBusy(t *testing.T) { + t.Parallel() + const sid = "s-busy" + c, wsID := buildBusyWorkspace(t, sid, true) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/v1/workspaces/"+wsID+"/sessions", nil) + req.SetPathValue("id", wsID) + rec := httptest.NewRecorder() + c.handleGetWorkspaceSessions(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + + var got []proto.Session + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got)) + require.Len(t, got, 1) + require.Equal(t, sid, got[0].ID) + require.True(t, got[0].IsBusy, "expected IsBusy=true for the busy session") +} + +func TestSessionListIdleSessionIsNotBusy(t *testing.T) { + t.Parallel() + const sid = "s-idle" + c, wsID := buildBusyWorkspace(t, sid, false) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/v1/workspaces/"+wsID+"/sessions", nil) + req.SetPathValue("id", wsID) + rec := httptest.NewRecorder() + c.handleGetWorkspaceSessions(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + + var got []proto.Session + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got)) + require.Len(t, got, 1) + require.False(t, got[0].IsBusy, "expected IsBusy=false for idle session") +} + +func TestSessionGetIncludesIsBusy(t *testing.T) { + t.Parallel() + const sid = "s-busy" + c, wsID := buildBusyWorkspace(t, sid, true) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/v1/workspaces/"+wsID+"/sessions/"+sid, nil) + req.SetPathValue("id", wsID) + req.SetPathValue("sid", sid) + rec := httptest.NewRecorder() + c.handleGetWorkspaceSession(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + + var got proto.Session + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got)) + require.Equal(t, sid, got.ID) + require.True(t, got.IsBusy) +} + +// TestIsSessionBusyNilSafe verifies the helper tolerates a missing +// workspace, app, or coordinator — phase A handlers rely on this so +// they can pass GetWorkspace's result through without an extra guard. +func TestIsSessionBusyNilSafe(t *testing.T) { + t.Parallel() + + require.False(t, isSessionBusy(nil, "x")) + require.False(t, isSessionBusy(&backend.Workspace{}, "x")) + require.False(t, isSessionBusy(&backend.Workspace{App: &app.App{}}, "x")) +} + +// TestProtoSessionIsBusyBackwardCompat verifies older consumers that +// unmarshal proto.Session without knowing about IsBusy still succeed +// and ignore the new field harmlessly. +func TestProtoSessionIsBusyBackwardCompat(t *testing.T) { + t.Parallel() + + wire := proto.Session{ID: "s1", Title: "t", IsBusy: true} + raw, err := json.Marshal(wire) + require.NoError(t, err) + + // Old client shape: same struct minus IsBusy. We model this by + // unmarshaling into a struct that doesn't declare the field. + type oldSession struct { + ID string `json:"id"` + Title string `json:"title"` + } + var old oldSession + require.NoError(t, json.Unmarshal(raw, &old)) + require.Equal(t, "s1", old.ID) + require.Equal(t, "t", old.Title) +} diff --git a/internal/workspace/client_workspace.go b/internal/workspace/client_workspace.go index 21aac96017971e6a072d3711745a12807b3a5556..243572e92bca2487e2f93c3fa6bb4d7aba49e0ce 100644 --- a/internal/workspace/client_workspace.go +++ b/internal/workspace/client_workspace.go @@ -676,6 +676,13 @@ func protoToMCPEventType(t proto.MCPEventType) mcp.EventType { } } +// protoToSession converts a wire-level proto.Session into the domain +// session.Session. Fields that exist only on the wire (computed-on-read +// signals like IsBusy, and any future presence counters) are +// intentionally dropped here: session.Session models persisted state, +// not transient runtime signals. UI features that need those signals +// should either extend session.Session or read them from the proto +// payload directly before this conversion runs. func protoToSession(s proto.Session) session.Session { return session.Session{ ID: s.ID,