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,