git.iko.soy/crush
on main help sign in
Merge pull request #3010 from charmbracelet/multi-client
5e05c50Christian Rocha
README.md+34
compare with main
unifiedsplit
@@ -368,6 +368,40 @@ which do expand.
368Crush has preliminary support for hooks. For details, see
368Crush has preliminary support for hooks. For details, see
369[the hook guide](./docs/hooks/).
369[the hook guide](./docs/hooks/).
370
370
371+### Sharing a workspace across clients
372+
373+When Crush is run against a shared backend (for example two TUIs talking to
374+the same `crush serve`), clients are grouped into **workspaces** keyed by
375+their resolved `--cwd`. Two clients with the same `--cwd` join the same
376+underlying workspace, so they share the session list, message history,
377+permission queue, LSP, and MCP state.
378+
379+Joining is implicit: pointing a second client at the same working directory
380+attaches it to the existing workspace. Each new invocation, however, starts
381+in its own fresh session by default. To pick up the conversation another
382+client already has open, use the session manager (the session picker) and
383+select it. Sessions surface two signals there:
384+
385+- `IsBusy` is set while an agent turn is in flight for that session.
386+- `AttachedClients` reports how many clients are currently viewing it.
387+
388+A non-zero `AttachedClients` (often combined with `IsBusy`) is the cue that a
389+session is "in progress" on another client and joining it will mirror that
390+view live.
391+
392+The first client to create a workspace fixes its process-wide flags. In
393+particular, `--yolo` and `--debug` follow a **first-wins** rule: later
394+clients that arrive at the same `--cwd` with different values for those
395+flags do not change the running workspace. A debug log line is emitted
396+recording the mismatch, and the workspace keeps the flags it was created
397+with.
398+
399+A workspace lives as long as at least one client has an SSE event stream
400+open against it. When the last stream disconnects, the workspace is torn
401+down. There is a short grace window right after `POST /v1/workspaces` so a
402+client that has created the workspace but not yet opened its event stream
403+does not get reaped before it can attach.
404+
371### Ignoring Files
405### Ignoring Files
372
406
373Crush respects `.gitignore` files by default, but you can also create a
407Crush respects `.gitignore` files by default, but you can also create a
internal/agent/tools/bash_test.go+10−6
@@ -21,11 +21,13 @@ func (m *mockBashPermissionService) Request(ctx context.Context, req permission.
21 return true, nil
21 return true, nil
22}
22}
23
23
24−func (m *mockBashPermissionService) Grant(req permission.PermissionRequest) {}
24+func (m *mockBashPermissionService) Grant(req permission.PermissionRequest) bool { return true }
25
25
26−func (m *mockBashPermissionService) Deny(req permission.PermissionRequest) {}
26+func (m *mockBashPermissionService) Deny(req permission.PermissionRequest) bool { return true }
27
27
28−func (m *mockBashPermissionService) GrantPersistent(req permission.PermissionRequest) {}
28+func (m *mockBashPermissionService) GrantPersistent(req permission.PermissionRequest) bool {
29+ return true
30+}
29
31
30func (m *mockBashPermissionService) AutoApproveSession(sessionID string) {}
32func (m *mockBashPermissionService) AutoApproveSession(sessionID string) {}
31
33
@@ -90,11 +92,13 @@ func (m *recordingPermissionService) Request(ctx context.Context, req permission
90 return m.allow, nil
92 return m.allow, nil
91}
93}
92
94
93−func (m *recordingPermissionService) Grant(req permission.PermissionRequest) {}
95+func (m *recordingPermissionService) Grant(req permission.PermissionRequest) bool { return true }
94
96
95−func (m *recordingPermissionService) Deny(req permission.PermissionRequest) {}
97+func (m *recordingPermissionService) Deny(req permission.PermissionRequest) bool { return true }
96
98
97−func (m *recordingPermissionService) GrantPersistent(req permission.PermissionRequest) {}
99+func (m *recordingPermissionService) GrantPersistent(req permission.PermissionRequest) bool {
100+ return true
101+}
98
102
99func (m *recordingPermissionService) AutoApproveSession(sessionID string) {}
103func (m *recordingPermissionService) AutoApproveSession(sessionID string) {}
100
104
internal/agent/tools/multiedit_test.go+5−3
@@ -20,11 +20,13 @@ func (m *mockPermissionService) Request(ctx context.Context, req permission.Crea
20 return true, nil
20 return true, nil
21}
21}
22
22
23−func (m *mockPermissionService) Grant(req permission.PermissionRequest) {}
23+func (m *mockPermissionService) Grant(req permission.PermissionRequest) bool { return true }
24
24
25−func (m *mockPermissionService) Deny(req permission.PermissionRequest) {}
25+func (m *mockPermissionService) Deny(req permission.PermissionRequest) bool { return true }
26
26
27−func (m *mockPermissionService) GrantPersistent(req permission.PermissionRequest) {}
27+func (m *mockPermissionService) GrantPersistent(req permission.PermissionRequest) bool {
28+ return true
29+}
28
30
29func (m *mockPermissionService) AutoApproveSession(sessionID string) {}
31func (m *mockPermissionService) AutoApproveSession(sessionID string) {}
30
32
internal/agent/tools/view_test.go+5−3
@@ -216,11 +216,13 @@ func (m *mockViewPermissionService) Request(ctx context.Context, req permission.
216 return true, nil
216 return true, nil
217}
217}
218
218
219−func (m *mockViewPermissionService) Grant(req permission.PermissionRequest) {}
219+func (m *mockViewPermissionService) Grant(req permission.PermissionRequest) bool { return true }
220
220
221−func (m *mockViewPermissionService) Deny(req permission.PermissionRequest) {}
221+func (m *mockViewPermissionService) Deny(req permission.PermissionRequest) bool { return true }
222
222
223−func (m *mockViewPermissionService) GrantPersistent(req permission.PermissionRequest) {}
223+func (m *mockViewPermissionService) GrantPersistent(req permission.PermissionRequest) bool {
224+ return true
225+}
224
226
225func (m *mockViewPermissionService) AutoApproveSession(sessionID string) {}
227func (m *mockViewPermissionService) AutoApproveSession(sessionID string) {}
226
228
internal/app/testing.gonew file+69
@@ -0,0 +1,69 @@
1+package app
2+
3+import (
4+ "context"
5+ "sync"
6+
7+ tea "charm.land/bubbletea/v2"
8+ "github.com/charmbracelet/crush/internal/agent/notify"
9+ "github.com/charmbracelet/crush/internal/permission"
10+ "github.com/charmbracelet/crush/internal/pubsub"
11+)
12+
13+// NewForTest constructs a minimal [App] suitable for in-process tests
14+// that need a working event broker and permission service without
15+// booting a real config, database, LSP, MCP, or agent coordinator.
16+//
17+// The returned App has:
18+//
19+// - A live `events` broker that [App.SendEvent] publishes to and
20+// [App.Events] subscribes from.
21+// - A real [permission.Service] whose request and notification
22+// brokers are fanned into the events broker, so subscribers to
23+// [App.Events] observe the same permission events the production
24+// wiring would deliver to SSE clients.
25+// - An [App.agentNotifications] broker.
26+//
27+// The caller owns lifetime: cancel ctx (or call [App.Shutdown]) to
28+// tear down the fan-in goroutines and the events broker.
29+func NewForTest(ctx context.Context) *App {
30+ app := &App{
31+ Permissions: permission.NewPermissionService("", false, nil),
32+ globalCtx: ctx,
33+ events: pubsub.NewBroker[tea.Msg](),
34+ serviceEventsWG: &sync.WaitGroup{},
35+ tuiWG: &sync.WaitGroup{},
36+ agentNotifications: pubsub.NewBroker[notify.Notification](),
37+ }
38+
39+ eventsCtx, cancel := context.WithCancel(ctx)
40+ app.eventsCtx = eventsCtx
41+ setupSubscriber(eventsCtx, app.serviceEventsWG, "permissions",
42+ app.Permissions.Subscribe, app.events)
43+ setupSubscriber(eventsCtx, app.serviceEventsWG, "permissions-notifications",
44+ app.Permissions.SubscribeNotifications, app.events)
45+ setupSubscriber(eventsCtx, app.serviceEventsWG, "agent-notifications",
46+ app.agentNotifications.Subscribe, app.events)
47+ app.cleanupFuncs = append(app.cleanupFuncs, func(context.Context) error {
48+ cancel()
49+ app.serviceEventsWG.Wait()
50+ app.events.Shutdown()
51+ return nil
52+ })
53+ return app
54+}
55+
56+// ShutdownForTest tears down the App's event broker and fan-in
57+// goroutines. It is safe to call multiple times.
58+//
59+// Use this in tests instead of [App.Shutdown], which drives a full
60+// production shutdown path (database release, LSP teardown, MCP
61+// shutdown) that synthetic test apps cannot satisfy.
62+func (app *App) ShutdownForTest() {
63+ for _, cleanup := range app.cleanupFuncs {
64+ if cleanup != nil {
65+ _ = cleanup(context.Background())
66+ }
67+ }
68+ app.cleanupFuncs = nil
69+}
internal/backend/backend.go+487−36
@@ -8,7 +8,10 @@ import (
8 "errors"
8 "errors"
9 "fmt"
9 "fmt"
10 "log/slog"
10 "log/slog"
11+ "path/filepath"
11 "runtime"
12 "runtime"
13+ "sync"
14+ "time"
12
15
13 "github.com/charmbracelet/crush/internal/app"
16 "github.com/charmbracelet/crush/internal/app"
14 "github.com/charmbracelet/crush/internal/config"
17 "github.com/charmbracelet/crush/internal/config"
@@ -29,19 +32,65 @@ var (
29 ErrPathRequired = errors.New("path is required")
32 ErrPathRequired = errors.New("path is required")
30 ErrInvalidPermissionAction = errors.New("invalid permission action")
33 ErrInvalidPermissionAction = errors.New("invalid permission action")
31 ErrUnknownCommand = errors.New("unknown command")
34 ErrUnknownCommand = errors.New("unknown command")
35+ ErrInvalidClientID = errors.New("invalid client_id")
36+ ErrClientNotAttached = errors.New("client not attached")
32)
37)
33
38
39+// DefaultCreateGrace is the window in which a client must open an SSE
40+// stream after creating a workspace before its creation hold is
41+// released. Exposed as a package variable so tests can shorten it.
42+var DefaultCreateGrace = 30 * time.Second
43+
34// ShutdownFunc is called when the backend needs to trigger a server
44// ShutdownFunc is called when the backend needs to trigger a server
35// shutdown (e.g. when the last workspace is removed).
45// shutdown (e.g. when the last workspace is removed).
36type ShutdownFunc func()
46type ShutdownFunc func()
37
47
38// Backend provides transport-agnostic business logic for the Crush
48// Backend provides transport-agnostic business logic for the Crush
39// server. It manages workspaces and delegates to [app.App] services.
49// server. It manages workspaces and delegates to [app.App] services.
50+//
51+// Locking order: when both [Backend.mu] and [Workspace.clientsMu] are
52+// held at once, [Backend.mu] is acquired first. Detach paths
53+// ([detachStream], [releaseHoldLocked], [expireHold]) only hold
54+// [Workspace.clientsMu] briefly, drop it, then call [teardown] which
55+// takes [Backend.mu] (and then re-takes [Workspace.clientsMu] to
56+// re-check that the workspace has not been re-claimed). This avoids
57+// the AB/BA hazard with [CreateWorkspace], which holds [Backend.mu]
58+// while calling [registerClient] so that a workspace cannot be torn
59+// down beneath it.
40type Backend struct {
60type Backend struct {
41 workspaces *csync.Map[string, *Workspace]
61 workspaces *csync.Map[string, *Workspace]
62+ // pathIndex maps a resolved absolute workspace path to its
63+ // workspace ID. Reads and writes are serialised via mu so
64+ // concurrent CreateWorkspace calls at the same path deduplicate
65+ // deterministically.
66+ pathIndex map[string]string
67+ mu sync.Mutex
68+
42− cfg *config.ConfigStore
69+ cfg *config.ConfigStore
43− ctx context.Context
70+ ctx context.Context
44− shutdownFn ShutdownFunc
71+ shutdownFn ShutdownFunc
72+ createGrace time.Duration
73+}
74+
75+// clientState tracks one client's claim on a workspace.
76+//
77+// - streams counts the number of live SSE event streams the client
78+// currently has open against the workspace.
79+// - holdTimer is non-nil iff the client created the workspace but has
80+// not yet attached an SSE stream; it fires after createGrace and
81+// releases the hold.
82+// - currentSessionID records which session this client is currently
83+// viewing. Empty string means the client has no session selected
84+// (e.g. the landing screen). Cleared automatically when the
85+// clientState entry is removed.
86+//
87+// streams and holdTimer are mutually exclusive in practice (the hold
88+// timer is stopped the moment an SSE stream attaches), but both being
89+// zero/nil means the entry has been released and should be removed.
90+type clientState struct {
91+ streams int
92+ holdTimer *time.Timer
93+ currentSessionID string
45}
94}
46
95
47// Workspace represents a running [app.App] workspace with its
96// Workspace represents a running [app.App] workspace with its
@@ -53,18 +102,57 @@ type Workspace struct {
53 Cfg *config.ConfigStore
102 Cfg *config.ConfigStore
54 Env []string
103 Env []string
55 Skills *skills.Manager
104 Skills *skills.Manager
105+
106+ // resolvedPath is the path used as the dedup key in
107+ // Backend.pathIndex. It is filepath.EvalSymlinks(filepath.Abs(Path))
108+ // with fallback to the cleaned absolute path.
109+ resolvedPath string
110+
111+ // clientsMu guards clients. It is held only briefly (no IO).
112+ clientsMu sync.Mutex
113+ // clients tracks each client's claim on this workspace. Refcount
114+ // is a derived value: len(clients).
115+ clients map[string]*clientState
116+
117+ // shutdownFn is the function invoked by [Backend.teardown] to
118+ // release the workspace's underlying resources. It defaults to the
119+ // embedded [app.App.Shutdown]; tests may override it to avoid
120+ // driving a full [app.App] through shutdown.
121+ shutdownFn func()
122+}
123+
124+// invokeShutdown calls the workspace shutdown hook if set, falling
125+// back to the embedded [app.App.Shutdown] when not.
126+func (w *Workspace) invokeShutdown() {
127+ if w.shutdownFn != nil {
128+ w.shutdownFn()
129+ return
130+ }
131+ if w.App != nil {
132+ w.Shutdown()
133+ }
56}
134}
57
135
58// New creates a new [Backend].
136// New creates a new [Backend].
59func New(ctx context.Context, cfg *config.ConfigStore, shutdownFn ShutdownFunc) *Backend {
137func New(ctx context.Context, cfg *config.ConfigStore, shutdownFn ShutdownFunc) *Backend {
60 return &Backend{
138 return &Backend{
61− workspaces: csync.NewMap[string, *Workspace](),
139+ workspaces: csync.NewMap[string, *Workspace](),
140+ pathIndex: make(map[string]string),
62− cfg: cfg,
141+ cfg: cfg,
63− ctx: ctx,
142+ ctx: ctx,
64− shutdownFn: shutdownFn,
143+ shutdownFn: shutdownFn,
144+ createGrace: DefaultCreateGrace,
65 }
145 }
66}
146}
67
147
148+// SetCreateGrace overrides the create-grace window. Intended for tests
149+// that need short timeouts.
150+func (b *Backend) SetCreateGrace(d time.Duration) {
151+ b.mu.Lock()
152+ defer b.mu.Unlock()
153+ b.createGrace = d
154+}
155+
68// GetWorkspace retrieves a workspace by ID.
156// GetWorkspace retrieves a workspace by ID.
69func (b *Backend) GetWorkspace(id string) (*Workspace, error) {
157func (b *Backend) GetWorkspace(id string) (*Workspace, error) {
70 ws, ok := b.workspaces.Get(id)
158 ws, ok := b.workspaces.Get(id)
@@ -84,12 +172,46 @@ func (b *Backend) ListWorkspaces() []proto.Workspace {
84}
172}
85
173
86// CreateWorkspace initializes a new workspace from the given
174// CreateWorkspace initializes a new workspace from the given
87−// parameters. It creates the config, database connection, and
175+// parameters, or returns an existing workspace if one already exists at
88−// [app.App] instance.
176+// the same resolved path (first-wins semantics).
177+//
178+// args.ClientID must be a valid UUID identifying the calling client;
179+// the resulting workspace registers a creation hold on behalf of that
180+// client which is released either by the first SSE attach (which
181+// converts it into a stream claim) or by the grace window expiring.
89func (b *Backend) CreateWorkspace(args proto.Workspace) (*Workspace, proto.Workspace, error) {
182func (b *Backend) CreateWorkspace(args proto.Workspace) (*Workspace, proto.Workspace, error) {
90 if args.Path == "" {
183 if args.Path == "" {
91 return nil, proto.Workspace{}, ErrPathRequired
184 return nil, proto.Workspace{}, ErrPathRequired
92 }
185 }
186+ clientID, err := validateClientID(args.ClientID)
187+ if err != nil {
188+ return nil, proto.Workspace{}, err
189+ }
190+
191+ key, err := resolveWorkspaceKey(args.Path)
192+ if err != nil {
193+ return nil, proto.Workspace{}, fmt.Errorf("failed to resolve workspace path: %w", err)
194+ }
195+
196+ b.mu.Lock()
197+ if existingID, ok := b.pathIndex[key]; ok {
198+ if ws, found := b.workspaces.Get(existingID); found {
199+ // Hold b.mu while registering: teardown also
200+ // acquires b.mu before tearing the workspace
201+ // down, so this guarantees the workspace we
202+ // return cannot be torn out from under us
203+ // between lookup and registerClient. Lock order
204+ // here is b.mu -> ws.clientsMu.
205+ logFirstWinsMismatch(ws, args)
206+ b.registerClient(ws, clientID)
207+ b.mu.Unlock()
208+ return ws, workspaceToProto(ws), nil
209+ }
210+ // pathIndex referenced a workspace that has since been
211+ // removed; clean the stale entry and fall through.
212+ delete(b.pathIndex, key)
213+ }
214+ b.mu.Unlock()
93
215
94 id := uuid.New().String()
216 id := uuid.New().String()
95 cfg, err := config.Init(args.Path, args.DataDir, args.Debug)
217 cfg, err := config.Init(args.Path, args.DataDir, args.Debug)
@@ -103,7 +225,7 @@ func (b *Backend) CreateWorkspace(args proto.Workspace) (*Workspace, proto.Works
103 return nil, proto.Workspace{}, fmt.Errorf("failed to create data directory: %w", err)
225 return nil, proto.Workspace{}, fmt.Errorf("failed to create data directory: %w", err)
104 }
226 }
105
227
106− conn, err := db.Connect(b.ctx, cfg.Config().Options.DataDirectory)
228+ conn, err := db.Connect(b.ctx, cfg.Config().Options.DataDirectory, db.WithDataDirLock(true))
107 if err != nil {
229 if err != nil {
108 return nil, proto.Workspace{}, fmt.Errorf("failed to connect to database: %w", err)
230 return nil, proto.Workspace{}, fmt.Errorf("failed to connect to database: %w", err)
109 }
231 }
@@ -125,15 +247,39 @@ func (b *Backend) CreateWorkspace(args proto.Workspace) (*Workspace, proto.Works
125 }
247 }
126
248
127 ws := &Workspace{
249 ws := &Workspace{
128− App: appWorkspace,
250+ App: appWorkspace,
129− ID: id,
251+ ID: id,
130− Path: args.Path,
252+ Path: args.Path,
131− Cfg: cfg,
253+ Cfg: cfg,
132− Env: args.Env,
254+ Env: args.Env,
133− Skills: skillsMgr,
255+ Skills: skillsMgr,
256+ resolvedPath: key,
257+ clients: make(map[string]*clientState),
134 }
258 }
135
259
260+ b.mu.Lock()
261+ // Re-check the index under the lock: a concurrent caller may have
262+ // won the race between the initial unlock and here.
263+ if existingID, ok := b.pathIndex[key]; ok {
264+ if existing, found := b.workspaces.Get(existingID); found {
265+ // Register under b.mu so teardown cannot run
266+ // between lookup and registerClient. Lock order
267+ // is b.mu -> ws.clientsMu.
268+ logFirstWinsMismatch(existing, args)
269+ b.registerClient(existing, clientID)
270+ b.mu.Unlock()
271+ ws.invokeShutdown()
272+ return existing, workspaceToProto(existing), nil
273+ }
274+ delete(b.pathIndex, key)
275+ }
136 b.workspaces.Set(id, ws)
276 b.workspaces.Set(id, ws)
277+ b.pathIndex[key] = id
278+ // Register the originating client's hold while still holding
279+ // b.mu so the workspace is observable with its claim from the
280+ // moment it appears in the index.
281+ b.registerClient(ws, clientID)
282+ b.mu.Unlock()
137
283
138 if args.Version != "" && args.Version != version.Version {
284 if args.Version != "" && args.Version != version.Version {
139 slog.Warn(
285 slog.Warn(
@@ -147,18 +293,7 @@ func (b *Backend) CreateWorkspace(args proto.Workspace) (*Workspace, proto.Works
147 )))
293 )))
148 }
294 }
149
295
150− result := proto.Workspace{
151− ID: id,
152− Path: args.Path,
153− DataDir: cfg.Config().Options.DataDirectory,
154− Debug: cfg.Config().Options.Debug,
155− YOLO: cfg.Overrides().SkipPermissionRequests,
156− Config: cfg.Config(),
157− Env: args.Env,
158− Skills: skillStatesToProto(skillStates),
159− }
160−
161− return ws, result, nil
296+ return ws, workspaceToProto(ws), nil
162}
297}
163
298
164// skillsDiscoveryConfig adapts a *config.ConfigStore to the
299// skillsDiscoveryConfig adapts a *config.ConfigStore to the
@@ -203,21 +338,261 @@ func skillStatesToProto(states []*skills.SkillState) []proto.SkillState {
203 return out
338 return out
204}
339}
205
340
206−// DeleteWorkspace shuts down and removes a workspace. If it was the
341+// AttachClient registers a new SSE stream for the given client on the
207−// last workspace, the shutdown callback is invoked.
342+// workspace. The stream's deferred cleanup must call DetachClient with
343+// the same arguments to release the claim.
344+//
345+// The lookup and the clients-map mutation are performed under
346+// [Backend.mu] so that AttachClient cannot race with [Backend.teardown]:
347+// teardown also holds [Backend.mu] while removing the workspace from
348+// b.workspaces, so once AttachClient observes the workspace and takes
349+// ws.clientsMu (under b.mu), no concurrent teardown can succeed without
350+// re-checking the (now non-empty) clients map. Lock order is the
351+// canonical b.mu -> ws.clientsMu.
352+func (b *Backend) AttachClient(workspaceID, clientID string) error {
353+ if _, err := validateClientID(clientID); err != nil {
354+ return err
355+ }
356+
357+ b.mu.Lock()
358+ defer b.mu.Unlock()
359+ ws, ok := b.workspaces.Get(workspaceID)
360+ if !ok {
361+ return ErrWorkspaceNotFound
362+ }
363+
364+ ws.clientsMu.Lock()
365+ defer ws.clientsMu.Unlock()
366+ cs, ok := ws.clients[clientID]
367+ if !ok {
368+ // Defensive: SSE attach without a prior CreateWorkspace by
369+ // this client still installs a stream claim so the stream
370+ // stays alive for its duration.
371+ ws.clients[clientID] = &clientState{streams: 1}
372+ return nil
373+ }
374+ if cs.holdTimer != nil {
375+ cs.holdTimer.Stop()
376+ cs.holdTimer = nil
377+ }
378+ cs.streams++
379+ return nil
380+}
381+
382+// DetachClient releases one SSE stream's hold on the workspace. If the
383+// client has no other streams and no pending creation hold, its claim
384+// is removed and the workspace is torn down once refcount hits zero.
385+func (b *Backend) DetachClient(workspaceID, clientID string) {
386+ ws, ok := b.workspaces.Get(workspaceID)
387+ if !ok {
388+ return
389+ }
390+ b.detachStream(ws, clientID)
391+}
392+
393+// releaseHold releases the creation hold for a client, if any. Active
394+// stream claims are unaffected. Idempotent: returns nil if the
395+// workspace or the client's hold no longer exist.
396+func (b *Backend) releaseHold(workspaceID, clientID string) error {
397+ if _, err := validateClientID(clientID); err != nil {
398+ return err
399+ }
400+ ws, ok := b.workspaces.Get(workspaceID)
401+ if !ok {
402+ return nil
403+ }
404+ b.releaseHoldLocked(ws, clientID)
405+ return nil
406+}
407+
408+// registerClient installs (idempotently) the given client's claim on
409+// the workspace and starts a grace timer if the entry is fresh.
410+func (b *Backend) registerClient(ws *Workspace, clientID string) {
411+ ws.clientsMu.Lock()
412+ defer ws.clientsMu.Unlock()
413+ if _, ok := ws.clients[clientID]; ok {
414+ // Idempotent: a duplicate CreateWorkspace from the same
415+ // client does not add a second claim.
416+ return
417+ }
418+ cs := &clientState{}
419+ cs.holdTimer = time.AfterFunc(b.createGrace, func() {
420+ b.expireHold(ws, clientID, cs)
421+ })
422+ ws.clients[clientID] = cs
423+}
424+
425+// expireHold is the body of the grace timer. It runs in its own
426+// goroutine and races against AttachClient/releaseHold; the timer
427+// stays valid only while the entry's holdTimer still points at it.
428+func (b *Backend) expireHold(ws *Workspace, clientID string, timer *clientState) {
429+ ws.clientsMu.Lock()
430+ cs, ok := ws.clients[clientID]
431+ if !ok || cs != timer || cs.holdTimer == nil || cs.streams > 0 {
432+ ws.clientsMu.Unlock()
433+ return
434+ }
435+ cs.holdTimer = nil
436+ delete(ws.clients, clientID)
437+ teardown := len(ws.clients) == 0
438+ ws.clientsMu.Unlock()
439+ if teardown {
440+ b.teardown(ws)
441+ }
442+}
443+
208−func (b *Backend) DeleteWorkspace(id string) {
444+func (b *Backend) releaseHoldLocked(ws *Workspace, clientID string) {
209− ws, ok := b.workspaces.Get(id)
445+ ws.clientsMu.Lock()
446+ cs, ok := ws.clients[clientID]
447+ if !ok {
448+ ws.clientsMu.Unlock()
449+ return
450+ }
451+ if cs.holdTimer != nil {
452+ cs.holdTimer.Stop()
453+ cs.holdTimer = nil
454+ }
455+ teardown := false
456+ if cs.streams == 0 {
457+ delete(ws.clients, clientID)
458+ teardown = len(ws.clients) == 0
459+ }
460+ ws.clientsMu.Unlock()
461+ if teardown {
462+ b.teardown(ws)
463+ }
464+}
465+
466+func (b *Backend) detachStream(ws *Workspace, clientID string) {
467+ ws.clientsMu.Lock()
468+ cs, ok := ws.clients[clientID]
210− if ok {
469+ if !ok {
211− ws.Shutdown()
470+ ws.clientsMu.Unlock()
471+ return
472+ }
473+ if cs.streams > 0 {
474+ cs.streams--
475+ }
476+ teardown := false
477+ if cs.streams == 0 && cs.holdTimer == nil {
478+ delete(ws.clients, clientID)
479+ teardown = len(ws.clients) == 0
480+ }
481+ ws.clientsMu.Unlock()
482+ if teardown {
483+ b.teardown(ws)
212 }
484 }
485+}
486+
487+// teardown removes the workspace from the index, shuts down its
488+// underlying [app.App], and triggers a server shutdown if it was the
489+// last workspace alive.
490+//
491+// Callers reach teardown after observing len(ws.clients) == 0 while
492+// holding ws.clientsMu and then releasing it. Between that release
493+// and the b.mu.Lock below, a concurrent CreateWorkspace may have
494+// re-registered a client (CreateWorkspace holds b.mu while doing so,
495+// so it is mutually exclusive with this critical section). teardown
496+// re-checks under both locks (in the canonical b.mu -> ws.clientsMu
497+// order) and aborts if the workspace has been re-claimed.
498+func (b *Backend) teardown(ws *Workspace) {
499+ b.mu.Lock()
500+ ws.clientsMu.Lock()
501+ if len(ws.clients) > 0 {
502+ // Race: a CreateWorkspace re-registered a client
503+ // between the detach path dropping ws.clientsMu and us
504+ // taking b.mu. Abort: the workspace is still alive.
505+ ws.clientsMu.Unlock()
506+ b.mu.Unlock()
507+ return
508+ }
509+ ws.clientsMu.Unlock()
510+ if existing, ok := b.pathIndex[ws.resolvedPath]; ok && existing == ws.ID {
511+ delete(b.pathIndex, ws.resolvedPath)
512+ }
213− b.workspaces.Del(id)
513+ b.workspaces.Del(ws.ID)
514+ remaining := b.workspaces.Len()
515+ b.mu.Unlock()
214
516
517+ ws.invokeShutdown()
518+
215− if b.workspaces.Len() == 0 && b.shutdownFn != nil {
519+ if remaining == 0 && b.shutdownFn != nil {
216 slog.Info("Last workspace removed, shutting down server...")
520 slog.Info("Last workspace removed, shutting down server...")
217 b.shutdownFn()
521 b.shutdownFn()
218 }
522 }
219}
523}
220
524
525+// DeleteWorkspace is the public entry point used by the HTTP DELETE
526+// handler. It releases the named client's creation hold; live streams
527+// from the same client remain attached and continue holding the
528+// workspace open until their own deferred DetachClient runs.
529+func (b *Backend) DeleteWorkspace(id, clientID string) error {
530+ return b.releaseHold(id, clientID)
531+}
532+
533+// SetCurrentSession records which session the given client is
534+// currently viewing within the workspace. Passing an empty sessionID
535+// clears the client's current-session entry (e.g. the client has
536+// returned to the landing screen).
537+//
538+// The client must be actually attached — i.e. its [clientState] entry
539+// must exist and have at least one live stream. A bare creation hold
540+// (streams == 0) is rejected with [ErrClientNotAttached]. This
541+// guards against zombie writes from a client that has detached and
542+// against ghost presence from a hold-only client that never opened an
543+// SSE stream.
544+func (b *Backend) SetCurrentSession(workspaceID, clientID, sessionID string) error {
545+ if _, err := validateClientID(clientID); err != nil {
546+ return err
547+ }
548+ ws, ok := b.workspaces.Get(workspaceID)
549+ if !ok {
550+ return ErrWorkspaceNotFound
551+ }
552+ ws.clientsMu.Lock()
553+ defer ws.clientsMu.Unlock()
554+ cs, ok := ws.clients[clientID]
555+ if !ok || cs.streams == 0 {
556+ // No entry, or hold-only (no live stream): refuse the
557+ // write. The presence record this is meant to feed
558+ // should only reflect clients that can actually observe
559+ // session events.
560+ return ErrClientNotAttached
561+ }
562+ cs.currentSessionID = sessionID
563+ return nil
564+}
565+
566+// AttachedClients returns the number of clients currently viewing
567+// sessionID in the given workspace. Only clients with at least one live
568+// SSE stream (streams > 0) AND a matching currentSessionID are counted;
569+// pure creation holds do not contribute. Returns [ErrWorkspaceNotFound]
570+// if the workspace is unknown.
571+func (b *Backend) AttachedClients(workspaceID, sessionID string) (int, error) {
572+ ws, ok := b.workspaces.Get(workspaceID)
573+ if !ok {
574+ return 0, ErrWorkspaceNotFound
575+ }
576+ return ws.AttachedClientsForSession(sessionID), nil
577+}
578+
579+// AttachedClientsForSession returns the number of clients in this
580+// workspace whose currentSessionID equals sessionID and which have at
581+// least one live SSE stream. Hold-only clients (streams == 0) do not
582+// contribute. Acquires the workspace's [clientsMu] briefly; the
583+// returned count is a point-in-time snapshot.
584+func (w *Workspace) AttachedClientsForSession(sessionID string) int {
585+ w.clientsMu.Lock()
586+ defer w.clientsMu.Unlock()
587+ n := 0
588+ for _, cs := range w.clients {
589+ if cs.streams > 0 && cs.currentSessionID == sessionID {
590+ n++
591+ }
592+ }
593+ return n
594+}
595+
221// GetWorkspaceProto returns the proto representation of a workspace.
596// GetWorkspaceProto returns the proto representation of a workspace.
222func (b *Backend) GetWorkspaceProto(id string) (proto.Workspace, error) {
597func (b *Backend) GetWorkspaceProto(id string) (proto.Workspace, error) {
223 ws, err := b.GetWorkspace(id)
598 ws, err := b.GetWorkspace(id)
@@ -250,6 +625,33 @@ func (b *Backend) Shutdown() {
250 }
625 }
251}
626}
252
627
628+// resolveWorkspaceKey returns a stable canonical form of path suitable
629+// for use as a dedup key. It applies filepath.Abs, then attempts
630+// filepath.EvalSymlinks; because EvalSymlinks errors on non-existent
631+// paths, it falls back to the cleaned absolute path in that case.
632+func resolveWorkspaceKey(path string) (string, error) {
633+ abs, err := filepath.Abs(path)
634+ if err != nil {
635+ return "", err
636+ }
637+ if resolved, err := filepath.EvalSymlinks(abs); err == nil {
638+ return resolved, nil
639+ }
640+ return abs, nil
641+}
642+
643+// validateClientID returns the trimmed UUID string or an error if the
644+// input is empty or not a valid UUID.
645+func validateClientID(id string) (string, error) {
646+ if id == "" {
647+ return "", ErrInvalidClientID
648+ }
649+ if _, err := uuid.Parse(id); err != nil {
650+ return "", fmt.Errorf("%w: %v", ErrInvalidClientID, err)
651+ }
652+ return id, nil
653+}
654+
253func workspaceToProto(ws *Workspace) proto.Workspace {
655func workspaceToProto(ws *Workspace) proto.Workspace {
254 cfg := ws.Cfg.Config()
656 cfg := ws.Cfg.Config()
255 out := proto.Workspace{
657 out := proto.Workspace{
@@ -259,9 +661,58 @@ func workspaceToProto(ws *Workspace) proto.Workspace {
259 DataDir: cfg.Options.DataDirectory,
661 DataDir: cfg.Options.DataDirectory,
260 Debug: cfg.Options.Debug,
662 Debug: cfg.Options.Debug,
261 Config: cfg,
663 Config: cfg,
664+ Env: ws.Env,
665+ Version: version.Version,
262 }
666 }
263 if ws.Skills != nil {
667 if ws.Skills != nil {
264 out.Skills = skillStatesToProto(ws.Skills.States())
668 out.Skills = skillStatesToProto(ws.Skills.States())
265 }
669 }
266 return out
670 return out
267}
671}
672+
673+// logFirstWinsMismatch emits a debug line whenever a second
674+// CreateWorkspace at the same resolved path arrives with flags that
675+// differ from the originating workspace. The existing workspace wins;
676+// the incoming flags are silently ignored.
677+//
678+// The comparison is done against the incoming args as the caller sent
679+// them — including empty/zero values — rather than after defaulting.
680+// This means that, for example, a second caller who omits DataDir
681+// while the first set one will still log the mismatch.
682+func logFirstWinsMismatch(existing *Workspace, args proto.Workspace) {
683+ existingCfg := existing.Cfg.Config()
684+ existingYOLO := existing.Cfg.Overrides().SkipPermissionRequests
685+ if existingYOLO == args.YOLO &&
686+ existingCfg.Options.Debug == args.Debug &&
687+ existingCfg.Options.DataDirectory == args.DataDir &&
688+ stringSlicesEqual(existing.Env, args.Env) {
689+ return
690+ }
691+ slog.Debug(
692+ "Workspace flag mismatch on duplicate create; first wins",
693+ "workspace_id", existing.ID,
694+ "path", existing.Path,
695+ "existing_yolo", existingYOLO,
696+ "requested_yolo", args.YOLO,
697+ "existing_debug", existingCfg.Options.Debug,
698+ "requested_debug", args.Debug,
699+ "existing_data_dir", existingCfg.Options.DataDirectory,
700+ "requested_data_dir", args.DataDir,
701+ "existing_env", existing.Env,
702+ "requested_env", args.Env,
703+ )
704+}
705+
706+// stringSlicesEqual reports whether a and b contain the same strings
707+// in the same order. nil and empty are treated as equal.
708+func stringSlicesEqual(a, b []string) bool {
709+ if len(a) != len(b) {
710+ return false
711+ }
712+ for i := range a {
713+ if a[i] != b[i] {
714+ return false
715+ }
716+ }
717+ return true
718+}
internal/backend/backend_skills_test.gonew file+167
@@ -0,0 +1,167 @@
1+package backend_test
2+
3+import (
4+ "context"
5+ "fmt"
6+ "os"
7+ "path/filepath"
8+ "testing"
9+ "time"
10+
11+ tea "charm.land/bubbletea/v2"
12+ "github.com/charmbracelet/crush/internal/backend"
13+ "github.com/charmbracelet/crush/internal/config"
14+ "github.com/charmbracelet/crush/internal/proto"
15+ "github.com/charmbracelet/crush/internal/pubsub"
16+ "github.com/charmbracelet/crush/internal/skills"
17+ "github.com/google/uuid"
18+ "github.com/stretchr/testify/require"
19+)
20+
21+// TestBackend_WorkspaceSkillsIsolation verifies that skill discovery
22+// state and SSE events are per-workspace, not process-global. Two
23+// workspaces in the same backend process must not see each other's
24+// discoveries (either in their initial snapshot or in subsequent
25+// PublishStates events).
26+func TestBackend_WorkspaceSkillsIsolation(t *testing.T) {
27+ // Isolate all of config.Init's filesystem reads from the host. The
28+ // project-local .agents/skills/<name>/SKILL.md per working dir is
29+ // what we actually want each workspace to see; everything else
30+ // (global skills, XDG dirs, etc.) must be empty/deterministic.
31+ hostHome := t.TempDir()
32+ t.Setenv("HOME", hostHome)
33+ t.Setenv("XDG_CONFIG_HOME", filepath.Join(hostHome, ".config"))
34+ t.Setenv("XDG_DATA_HOME", filepath.Join(hostHome, ".local", "share"))
35+ t.Setenv("XDG_CACHE_HOME", filepath.Join(hostHome, ".cache"))
36+ t.Setenv("CRUSH_SKILLS_DIR", t.TempDir())
37+
38+ // Each workspace gets its own working directory containing a
39+ // distinct project-local skill so the discovery output differs.
40+ wdA := t.TempDir()
41+ wdB := t.TempDir()
42+ writeSkill(t, wdA, "wsa-only-skill", "Workspace A only skill.")
43+ writeSkill(t, wdB, "wsb-only-skill", "Workspace B only skill.")
44+
45+ srvCfg, err := config.Init(wdA, "", false)
46+ require.NoError(t, err)
47+ b := backend.New(t.Context(), srvCfg, nil)
48+
49+ cidA := uuid.New().String()
50+ cidB := uuid.New().String()
51+
52+ wsA, _, err := b.CreateWorkspace(proto.Workspace{
53+ ClientID: cidA,
54+ Path: wdA,
55+ DataDir: filepath.Join(wdA, ".crush"),
56+ })
57+ require.NoError(t, err)
58+ t.Cleanup(func() { _ = b.DeleteWorkspace(wsA.ID, cidA) })
59+
60+ wsB, _, err := b.CreateWorkspace(proto.Workspace{
61+ ClientID: cidB,
62+ Path: wdB,
63+ DataDir: filepath.Join(wdB, ".crush"),
64+ })
65+ require.NoError(t, err)
66+ t.Cleanup(func() { _ = b.DeleteWorkspace(wsB.ID, cidB) })
67+
68+ require.NotNil(t, wsA.Skills, "workspace A must have its own skills.Manager")
69+ require.NotNil(t, wsB.Skills, "workspace B must have its own skills.Manager")
70+ require.NotSame(t, wsA.Skills, wsB.Skills, "managers must be distinct instances per workspace")
71+
72+ // Initial snapshots see each workspace's own filesystem skill, and
73+ // neither sees the other's.
74+ statesA := wsA.Skills.States()
75+ statesB := wsB.Skills.States()
76+ require.True(t, containsSkillName(statesA, "wsa-only-skill"),
77+ "workspace A snapshot missing its own skill")
78+ require.False(t, containsSkillName(statesA, "wsb-only-skill"),
79+ "workspace A snapshot leaked workspace B's skill")
80+ require.True(t, containsSkillName(statesB, "wsb-only-skill"),
81+ "workspace B snapshot missing its own skill")
82+ require.False(t, containsSkillName(statesB, "wsa-only-skill"),
83+ "workspace B snapshot leaked workspace A's skill")
84+
85+ // Subscribe to each workspace's SSE event stream.
86+ ctxA, cancelA := context.WithCancel(t.Context())
87+ t.Cleanup(cancelA)
88+ chA, err := b.SubscribeEvents(ctxA, wsA.ID)
89+ require.NoError(t, err)
90+
91+ ctxB, cancelB := context.WithCancel(t.Context())
92+ t.Cleanup(cancelB)
93+ chB, err := b.SubscribeEvents(ctxB, wsB.ID)
94+ require.NoError(t, err)
95+
96+ // Trigger a republish on workspace A only. The marker name lets us
97+ // distinguish this event from any incidental backend activity.
98+ const marker = "wsa-republish-marker"
99+ wsA.Skills.PublishStates([]*skills.SkillState{
100+ {Name: marker, State: skills.StateNormal},
101+ })
102+
103+ // Workspace A must receive its own event.
104+ require.True(t,
105+ waitForSkillsEvent(t, chA, marker, 2*time.Second),
106+ "workspace A never received its own skills event")
107+
108+ // Workspace B must NOT receive workspace A's event.
109+ require.False(t,
110+ waitForSkillsEvent(t, chB, marker, 250*time.Millisecond),
111+ "workspace B leaked workspace A's skills event")
112+
113+ // And A's published states are now visible on its manager's
114+ // snapshot (verifies PublishStates updates the cache, not just the
115+ // broker).
116+ updatedA := wsA.Skills.States()
117+ require.True(t, containsSkillName(updatedA, marker),
118+ "PublishStates must update Manager.States()")
119+
120+ // B's manager snapshot is untouched.
121+ require.False(t, containsSkillName(wsB.Skills.States(), marker),
122+ "workspace B's Manager.States() leaked workspace A's republish")
123+}
124+
125+func writeSkill(t *testing.T, workingDir, name, desc string) {
126+ t.Helper()
127+ skillDir := filepath.Join(workingDir, ".agents", "skills", name)
128+ require.NoError(t, os.MkdirAll(skillDir, 0o755))
129+ content := fmt.Sprintf("---\nname: %s\ndescription: %s\n---\n%s\n", name, desc, desc)
130+ require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(content), 0o644))
131+}
132+
133+func containsSkillName(states []*skills.SkillState, name string) bool {
134+ for _, s := range states {
135+ if s.Name == name {
136+ return true
137+ }
138+ }
139+ return false
140+}
141+
142+// waitForSkillsEvent drains the given event channel until either a
143+// pubsub.Event[skills.Event] containing a state named marker arrives or
144+// the timeout elapses. Non-skills events on the channel are silently
145+// skipped — the backend fans in many event types and we only care
146+// about skills here.
147+func waitForSkillsEvent(t *testing.T, ch <-chan pubsub.Event[tea.Msg], marker string, timeout time.Duration) bool {
148+ t.Helper()
149+ deadline := time.After(timeout)
150+ for {
151+ select {
152+ case ev, ok := <-ch:
153+ if !ok {
154+ return false
155+ }
156+ se, ok := ev.Payload.(pubsub.Event[skills.Event])
157+ if !ok {
158+ continue
159+ }
160+ if containsSkillName(se.Payload.States, marker) {
161+ return true
162+ }
163+ case <-deadline:
164+ return false
165+ }
166+ }
167+}
internal/backend/backend_test.go+1215−144
@@ -1,170 +1,1241 @@
1−package backend_test
1+package backend
2
2
3import (
3import (
4+ "bytes"
4 "context"
5 "context"
5− "fmt"
6+ "errors"
7+ "log/slog"
6 "os"
8 "os"
7 "path/filepath"
9 "path/filepath"
10+ "runtime"
11+ "strings"
12+ "sync"
13+ "sync/atomic"
8 "testing"
14 "testing"
9 "time"
15 "time"
10
16
11− tea "charm.land/bubbletea/v2"
12− "github.com/charmbracelet/crush/internal/backend"
17+ "github.com/charmbracelet/crush/internal/csync"
13− "github.com/charmbracelet/crush/internal/config"
14 "github.com/charmbracelet/crush/internal/proto"
18 "github.com/charmbracelet/crush/internal/proto"
15− "github.com/charmbracelet/crush/internal/pubsub"
19+ "github.com/google/uuid"
16− "github.com/charmbracelet/crush/internal/skills"
17 "github.com/stretchr/testify/require"
20 "github.com/stretchr/testify/require"
18)
21)
19
22
20−// TestBackend_WorkspaceSkillsIsolation verifies that skill discovery
23+// newTestBackend returns a Backend whose teardown path skips any
21−// state and SSE events are per-workspace, not process-global. This
24+// real [app.App] shutdown work. Useful for state-machine tests that
22−// guards the structural change in §5 of the plan: two workspaces in the
25+// install synthetic workspaces directly via insertTestWorkspace.
23−// same backend process must not see each other's discoveries (either in
26+func newTestBackend(t *testing.T) (*Backend, *atomic.Int32) {
24−// their initial snapshot or in subsequent PublishStates events).
27+ t.Helper()
25−//
28+ var shutdownCount atomic.Int32
26−// Before that change landed, the package-level latestStates cache and
29+ b := &Backend{
27−// the package-level broker meant that:
30+ workspaces: csync.NewMap[string, *Workspace](),
28−// - workspace A's PublishStates would arrive on workspace B's SSE
31+ pathIndex: make(map[string]string),
29−// stream, and
32+ ctx: context.Background(),
30−// - the most recent SetLatestStates would silently overwrite the
31−// other workspace's cache.
32−//
33−// This test fails on either scenario.
34−func TestBackend_WorkspaceSkillsIsolation(t *testing.T) {
35− // Isolate all of config.Init's filesystem reads from the host. The
36− // project-local .agents/skills/<name>/SKILL.md per working dir is
37− // what we actually want each workspace to see; everything else
38− // (global skills, XDG dirs, etc.) must be empty/deterministic.
39− hostHome := t.TempDir()
40− t.Setenv("HOME", hostHome)
41− t.Setenv("XDG_CONFIG_HOME", filepath.Join(hostHome, ".config"))
42− t.Setenv("XDG_DATA_HOME", filepath.Join(hostHome, ".local", "share"))
43− t.Setenv("XDG_CACHE_HOME", filepath.Join(hostHome, ".cache"))
44− t.Setenv("CRUSH_SKILLS_DIR", t.TempDir())
45−
46− // Each workspace gets its own working directory containing a
47− // distinct project-local skill so the discovery output differs.
48− wdA := t.TempDir()
49− wdB := t.TempDir()
50− writeSkill(t, wdA, "wsa-only-skill", "Workspace A only skill.")
51− writeSkill(t, wdB, "wsb-only-skill", "Workspace B only skill.")
52−
53− srvCfg, err := config.Init(wdA, "", false)
54− require.NoError(t, err)
55− b := backend.New(t.Context(), srvCfg, nil)
56−
57− wsA, _, err := b.CreateWorkspace(proto.Workspace{
58− Path: wdA,
59− DataDir: filepath.Join(wdA, ".crush"),
60− })
61− require.NoError(t, err)
62− t.Cleanup(func() { b.DeleteWorkspace(wsA.ID) })
63−
64− wsB, _, err := b.CreateWorkspace(proto.Workspace{
65− Path: wdB,
66− DataDir: filepath.Join(wdB, ".crush"),
67− })
68− require.NoError(t, err)
69− t.Cleanup(func() { b.DeleteWorkspace(wsB.ID) })
70−
71− require.NotNil(t, wsA.Skills, "workspace A must have its own skills.Manager")
72− require.NotNil(t, wsB.Skills, "workspace B must have its own skills.Manager")
73− require.NotSame(t, wsA.Skills, wsB.Skills, "managers must be distinct instances per workspace")
74−
75− // Initial snapshots see each workspace's own filesystem skill, and
76− // neither sees the other's.
77− statesA := wsA.Skills.States()
78− statesB := wsB.Skills.States()
79− require.True(t, containsSkillName(statesA, "wsa-only-skill"),
80− "workspace A snapshot missing its own skill")
81− require.False(t, containsSkillName(statesA, "wsb-only-skill"),
82− "workspace A snapshot leaked workspace B's skill")
83− require.True(t, containsSkillName(statesB, "wsb-only-skill"),
84− "workspace B snapshot missing its own skill")
85− require.False(t, containsSkillName(statesB, "wsa-only-skill"),
86− "workspace B snapshot leaked workspace A's skill")
87−
88− // Subscribe to each workspace's SSE event stream.
89− ctxA, cancelA := context.WithCancel(t.Context())
90− t.Cleanup(cancelA)
91− chA, err := b.SubscribeEvents(ctxA, wsA.ID)
92− require.NoError(t, err)
93−
94− ctxB, cancelB := context.WithCancel(t.Context())
95− t.Cleanup(cancelB)
96− chB, err := b.SubscribeEvents(ctxB, wsB.ID)
97− require.NoError(t, err)
98−
99− // Trigger a republish on workspace A only. The marker name lets us
100− // distinguish this event from any incidental backend activity.
101− const marker = "wsa-republish-marker"
102− wsA.Skills.PublishStates([]*skills.SkillState{
103− {Name: marker, State: skills.StateNormal},
104− })
105−
106− // Workspace A must receive its own event.
107− require.True(t,
108− waitForSkillsEvent(t, chA, marker, 2*time.Second),
109− "workspace A never received its own skills event")
110−
111− // Workspace B must NOT receive workspace A's event.
112− require.False(t,
113− waitForSkillsEvent(t, chB, marker, 250*time.Millisecond),
33+ createGrace: 50 * time.Millisecond,
114− "workspace B leaked workspace A's skills event")
34+ shutdownFn: func() { shutdownCount.Add(1) },
115−
35+ }
116− // And A's published states are now visible on its manager's
36+ return b, &shutdownCount
117− // snapshot (verifies PublishStates updates the cache, not just the
37+}
118− // broker).
119− updatedA := wsA.Skills.States()
120− require.True(t, containsSkillName(updatedA, marker),
121− "PublishStates must update Manager.States()")
122
38
123− // B's manager snapshot is untouched.
39+// insertTestWorkspace installs a synthetic workspace into b at the
124− require.False(t, containsSkillName(wsB.Skills.States(), marker),
40+// given resolved path. Its shutdownFn is recorded in the returned
125− "workspace B's Manager.States() leaked workspace A's republish")
41+// counter so tests can assert it ran exactly once.
42+func insertTestWorkspace(t *testing.T, b *Backend, key string) (*Workspace, *atomic.Int32) {
43+ t.Helper()
44+ var shutdowns atomic.Int32
45+ ws := &Workspace{
46+ ID: uuid.New().String(),
47+ Path: key,
48+ resolvedPath: key,
49+ clients: make(map[string]*clientState),
50+ shutdownFn: func() { shutdowns.Add(1) },
51+ }
52+ b.mu.Lock()
53+ b.workspaces.Set(ws.ID, ws)
54+ b.pathIndex[key] = ws.ID
55+ b.mu.Unlock()
56+ return ws, &shutdowns
126}
57}
127
58
128−func writeSkill(t *testing.T, workingDir, name, desc string) {
59+func newClientID(t *testing.T) string {
129 t.Helper()
60 t.Helper()
130− skillDir := filepath.Join(workingDir, ".agents", "skills", name)
61+ return uuid.New().String()
62+}
63+
64+func TestResolveWorkspaceKey_AbsoluteAndSymlink(t *testing.T) {
65+ t.Parallel()
66+
67+ tmp := t.TempDir()
68+ real, err := filepath.EvalSymlinks(tmp)
69+ require.NoError(t, err)
70+
71+ got, err := resolveWorkspaceKey(tmp)
131− require.NoError(t, os.MkdirAll(skillDir, 0o755))
72+ require.NoError(t, err)
132− content := fmt.Sprintf("---\nname: %s\ndescription: %s\n---\n%s\n", name, desc, desc)
73+ require.Equal(t, real, got)
133− require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(content), 0o644))
74+}
75+
76+func TestResolveWorkspaceKey_NonExistentFallback(t *testing.T) {
77+ t.Parallel()
78+
79+ missing := filepath.Join(t.TempDir(), "does", "not", "exist")
80+ got, err := resolveWorkspaceKey(missing)
81+ require.NoError(t, err)
82+ abs, err := filepath.Abs(missing)
83+ require.NoError(t, err)
84+ require.Equal(t, abs, got)
85+}
86+
87+func TestValidateClientID(t *testing.T) {
88+ t.Parallel()
89+
90+ _, err := validateClientID("")
91+ require.ErrorIs(t, err, ErrInvalidClientID)
92+ _, err = validateClientID("not-a-uuid")
93+ require.ErrorIs(t, err, ErrInvalidClientID)
94+
95+ id := uuid.New().String()
96+ got, err := validateClientID(id)
97+ require.NoError(t, err)
98+ require.Equal(t, id, got)
99+}
100+
101+func TestRegisterClient_Idempotent(t *testing.T) {
102+ t.Parallel()
103+
104+ b, _ := newTestBackend(t)
105+ ws, _ := insertTestWorkspace(t, b, "/tmp/a")
106+
107+ cid := newClientID(t)
108+ b.registerClient(ws, cid)
109+ b.registerClient(ws, cid)
110+
111+ ws.clientsMu.Lock()
112+ defer ws.clientsMu.Unlock()
113+ require.Len(t, ws.clients, 1)
114+ require.NotNil(t, ws.clients[cid].holdTimer)
115+ require.Equal(t, 0, ws.clients[cid].streams)
116+}
117+
118+func TestAttachClient_ConsumesHold(t *testing.T) {
119+ t.Parallel()
120+
121+ b, _ := newTestBackend(t)
122+ ws, shutdowns := insertTestWorkspace(t, b, "/tmp/a")
123+
124+ cid := newClientID(t)
125+ b.registerClient(ws, cid)
126+ require.NoError(t, b.AttachClient(ws.ID, cid))
127+
128+ ws.clientsMu.Lock()
129+ require.Len(t, ws.clients, 1)
130+ require.Nil(t, ws.clients[cid].holdTimer, "attach must stop the grace timer")
131+ require.Equal(t, 1, ws.clients[cid].streams)
132+ ws.clientsMu.Unlock()
133+
134+ // Wait past the grace window: a stopped timer must not fire.
135+ time.Sleep(150 * time.Millisecond)
136+ require.Equal(t, int32(0), shutdowns.Load(), "workspace must not be torn down while attached")
137+}
138+
139+func TestAttachClient_WithoutPriorCreate(t *testing.T) {
140+ t.Parallel()
141+
142+ b, _ := newTestBackend(t)
143+ ws, _ := insertTestWorkspace(t, b, "/tmp/a")
144+
145+ cid := newClientID(t)
146+ require.NoError(t, b.AttachClient(ws.ID, cid))
147+
148+ ws.clientsMu.Lock()
149+ defer ws.clientsMu.Unlock()
150+ require.Len(t, ws.clients, 1)
151+ require.Equal(t, 1, ws.clients[cid].streams)
152+ require.Nil(t, ws.clients[cid].holdTimer)
153+}
154+
155+func TestAttachClient_DuplicateStreams(t *testing.T) {
156+ t.Parallel()
157+
158+ b, _ := newTestBackend(t)
159+ ws, shutdowns := insertTestWorkspace(t, b, "/tmp/a")
160+
161+ cid := newClientID(t)
162+ require.NoError(t, b.AttachClient(ws.ID, cid))
163+ require.NoError(t, b.AttachClient(ws.ID, cid))
164+
165+ ws.clientsMu.Lock()
166+ require.Equal(t, 2, ws.clients[cid].streams)
167+ ws.clientsMu.Unlock()
168+
169+ b.DetachClient(ws.ID, cid)
170+ ws.clientsMu.Lock()
171+ require.Equal(t, 1, ws.clients[cid].streams)
172+ ws.clientsMu.Unlock()
173+ require.Equal(t, int32(0), shutdowns.Load())
174+
175+ b.DetachClient(ws.ID, cid)
176+ require.Equal(t, int32(1), shutdowns.Load(), "second detach tears down the workspace")
134}
177}
135
178
136−func containsSkillName(states []*skills.SkillState, name string) bool {
179+func TestDetachClient_LastStreamTearsDown(t *testing.T) {
137− for _, s := range states {
180+ t.Parallel()
138− if s.Name == name {
181+
139− return true
182+ b, srvShutdowns := newTestBackend(t)
183+ ws, wsShutdowns := insertTestWorkspace(t, b, "/tmp/a")
184+
185+ cid := newClientID(t)
186+ b.registerClient(ws, cid)
187+ require.NoError(t, b.AttachClient(ws.ID, cid))
188+ b.DetachClient(ws.ID, cid)
189+
190+ require.Equal(t, int32(1), wsShutdowns.Load())
191+ require.Equal(t, int32(1), srvShutdowns.Load(), "last workspace shut down must trigger server shutdown")
192+ _, err := b.GetWorkspace(ws.ID)
193+ require.ErrorIs(t, err, ErrWorkspaceNotFound)
194+}
195+
196+func TestHoldExpiry_TearsDown(t *testing.T) {
197+ t.Parallel()
198+
199+ b, srvShutdowns := newTestBackend(t)
200+ ws, wsShutdowns := insertTestWorkspace(t, b, "/tmp/a")
201+
202+ cid := newClientID(t)
203+ b.registerClient(ws, cid)
204+
205+ require.Eventually(t, func() bool {
206+ return wsShutdowns.Load() == 1 && srvShutdowns.Load() == 1
207+ }, 1*time.Second, 5*time.Millisecond)
208+}
209+
210+func TestReleaseHold_NoStreams(t *testing.T) {
211+ t.Parallel()
212+
213+ b, _ := newTestBackend(t)
214+ ws, shutdowns := insertTestWorkspace(t, b, "/tmp/a")
215+
216+ cid := newClientID(t)
217+ b.registerClient(ws, cid)
218+ require.NoError(t, b.releaseHold(ws.ID, cid))
219+
220+ require.Equal(t, int32(1), shutdowns.Load())
221+ // Idempotent.
222+ require.NoError(t, b.releaseHold(ws.ID, cid))
223+ require.Equal(t, int32(1), shutdowns.Load())
224+}
225+
226+func TestReleaseHold_WithActiveStream(t *testing.T) {
227+ t.Parallel()
228+
229+ b, _ := newTestBackend(t)
230+ ws, shutdowns := insertTestWorkspace(t, b, "/tmp/a")
231+
232+ cid := newClientID(t)
233+ b.registerClient(ws, cid)
234+ require.NoError(t, b.AttachClient(ws.ID, cid))
235+ require.NoError(t, b.releaseHold(ws.ID, cid))
236+
237+ ws.clientsMu.Lock()
238+ require.Equal(t, 1, ws.clients[cid].streams)
239+ require.Nil(t, ws.clients[cid].holdTimer)
240+ ws.clientsMu.Unlock()
241+ require.Equal(t, int32(0), shutdowns.Load())
242+
243+ b.DetachClient(ws.ID, cid)
244+ require.Equal(t, int32(1), shutdowns.Load())
245+}
246+
247+func TestReleaseHoldThenAttach(t *testing.T) {
248+ t.Parallel()
249+
250+ b, _ := newTestBackend(t)
251+ ws, shutdowns := insertTestWorkspace(t, b, "/tmp/a")
252+
253+ cid := newClientID(t)
254+ require.NoError(t, b.releaseHold(ws.ID, cid)) // no entry yet — no-op.
255+ require.NoError(t, b.AttachClient(ws.ID, cid))
256+ ws.clientsMu.Lock()
257+ require.Equal(t, 1, ws.clients[cid].streams)
258+ ws.clientsMu.Unlock()
259+ require.NoError(t, b.releaseHold(ws.ID, cid)) // hold-only no-op (no hold timer).
260+ require.Equal(t, int32(0), shutdowns.Load())
261+ b.DetachClient(ws.ID, cid)
262+ require.Equal(t, int32(1), shutdowns.Load())
263+}
264+
265+func TestRefcountWithSecondClient(t *testing.T) {
266+ t.Parallel()
267+
268+ b, _ := newTestBackend(t)
269+ ws, shutdowns := insertTestWorkspace(t, b, "/tmp/a")
270+
271+ cidA := newClientID(t)
272+ cidB := newClientID(t)
273+ b.registerClient(ws, cidA)
274+ require.NoError(t, b.AttachClient(ws.ID, cidA))
275+ b.registerClient(ws, cidB)
276+ require.NoError(t, b.AttachClient(ws.ID, cidB))
277+
278+ b.DetachClient(ws.ID, cidA)
279+ ws.clientsMu.Lock()
280+ require.Contains(t, ws.clients, cidB)
281+ require.NotContains(t, ws.clients, cidA)
282+ ws.clientsMu.Unlock()
283+ require.Equal(t, int32(0), shutdowns.Load(), "workspace survives while second client attached")
284+
285+ b.DetachClient(ws.ID, cidB)
286+ require.Equal(t, int32(1), shutdowns.Load())
287+}
288+
289+func TestAttachClient_InvalidID(t *testing.T) {
290+ t.Parallel()
291+
292+ b, _ := newTestBackend(t)
293+ ws, _ := insertTestWorkspace(t, b, "/tmp/a")
294+
295+ require.ErrorIs(t, b.AttachClient(ws.ID, ""), ErrInvalidClientID)
296+ require.ErrorIs(t, b.AttachClient(ws.ID, "not-a-uuid"), ErrInvalidClientID)
297+}
298+
299+func TestDeleteWorkspace_RejectsBadClientID(t *testing.T) {
300+ t.Parallel()
301+
302+ b, _ := newTestBackend(t)
303+ ws, _ := insertTestWorkspace(t, b, "/tmp/a")
304+
305+ require.ErrorIs(t, b.DeleteWorkspace(ws.ID, ""), ErrInvalidClientID)
306+ require.ErrorIs(t, b.DeleteWorkspace(ws.ID, "not-a-uuid"), ErrInvalidClientID)
307+}
308+
309+// TestHoldExpiry_RaceWithAttach checks that, when the grace timer fires
310+// while a concurrent AttachClient call is in flight, the workspace ends
311+// up either fully attached or fully torn down — never in a half-state.
312+func TestHoldExpiry_RaceWithAttach(t *testing.T) {
313+ t.Parallel()
314+
315+ for i := range 50 {
316+ b, _ := newTestBackend(t)
317+ // Tighten the grace window further to force the race.
318+ b.createGrace = 1 * time.Millisecond
319+ ws, shutdowns := insertTestWorkspace(t, b, "/tmp/race")
320+
321+ cid := newClientID(t)
322+ b.registerClient(ws, cid)
323+ // Attach concurrently with the very short grace timer.
324+ errCh := make(chan error, 1)
325+ go func() { errCh <- b.AttachClient(ws.ID, cid) }()
326+ <-errCh
327+
328+ // Wait for any pending timer to settle.
329+ time.Sleep(10 * time.Millisecond)
330+
331+ ws.clientsMu.Lock()
332+ gotShutdown := shutdowns.Load() == 1
333+ cs, present := ws.clients[cid]
334+ var (
335+ gotStreams int
336+ gotHoldTimer *time.Timer
337+ )
338+ if present {
339+ gotStreams = cs.streams
340+ gotHoldTimer = cs.holdTimer
341+ }
342+ ws.clientsMu.Unlock()
343+ // Either the workspace was torn down OR the client is
344+ // attached with streams==1 and the hold timer cleared.
345+ // The state must be consistent: if shutdown, client is
346+ // gone; if attached, no teardown and streams==1.
347+ if gotShutdown {
348+ require.False(t, present, "iter %d: shutdown but client still present", i)
349+ } else {
350+ require.True(t, present, "iter %d: not shutdown but client missing", i)
351+ require.Equal(t, 1, gotStreams, "iter %d: attach winner must leave streams=1", i)
352+ require.Nil(t, gotHoldTimer, "iter %d: attach winner must clear holdTimer", i)
140 }
353 }
141 }
354 }
142− return false
143}
355}
144
356
145−// waitForSkillsEvent drains the given event channel until either a
357+// TestConcurrentAttachDetach exercises the state machine under
146−// pubsub.Event[skills.Event] containing a state named marker arrives or
358+// parallel attach/detach pressure with the race detector.
147−// the timeout elapses. Non-skills events on the channel are silently
359+func TestConcurrentAttachDetach(t *testing.T) {
148−// skipped — the backend fans in many event types and we only care
360+ t.Parallel()
149−// about skills here.
361+
150−func waitForSkillsEvent(t *testing.T, ch <-chan pubsub.Event[tea.Msg], marker string, timeout time.Duration) bool {
362+ b, _ := newTestBackend(t)
363+ ws, _ := insertTestWorkspace(t, b, "/tmp/a")
364+
365+ cid := newClientID(t)
366+ b.registerClient(ws, cid)
367+ require.NoError(t, b.AttachClient(ws.ID, cid)) // ensure refcount stays > 0.
368+
369+ const n = 50
370+ var wg sync.WaitGroup
371+ wg.Add(n)
372+ for range n {
373+ go func() {
374+ defer wg.Done()
375+ cid2 := newClientID(t)
376+ _ = b.AttachClient(ws.ID, cid2)
377+ b.DetachClient(ws.ID, cid2)
378+ }()
379+ }
380+ wg.Wait()
381+
382+ ws.clientsMu.Lock()
383+ defer ws.clientsMu.Unlock()
384+ require.Len(t, ws.clients, 1)
385+ require.Contains(t, ws.clients, cid)
386+}
387+
388+// TestPathDedupe_FullCreate exercises CreateWorkspace end-to-end
389+// (config init, real app.App). Two CreateWorkspace calls at the same
390+// path return the same workspace ID and share the clients map.
391+func TestPathDedupe_FullCreate(t *testing.T) {
392+ t.Setenv("HOME", t.TempDir())
393+ t.Setenv("XDG_CACHE_HOME", t.TempDir())
394+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
395+ t.Setenv("XDG_DATA_HOME", t.TempDir())
396+
397+ cwd := t.TempDir()
398+ dataDir := t.TempDir()
399+
400+ b := New(context.Background(), nil, func() {})
401+ b.SetCreateGrace(2 * time.Second)
402+ t.Cleanup(func() { drainBackend(t, b) })
403+
404+ cidA := uuid.New().String()
405+ cidB := uuid.New().String()
406+
407+ wsA, protoA, err := b.CreateWorkspace(protoWS(cwd, dataDir, cidA))
408+ require.NoError(t, err)
409+ require.NotEmpty(t, protoA.ID)
410+ require.Equal(t, protoA.DataDir, wsA.Cfg.Config().Options.DataDirectory)
411+
412+ wsB, protoB, err := b.CreateWorkspace(protoWS(cwd, dataDir, cidB))
413+ require.NoError(t, err)
414+ require.Equal(t, wsA.ID, wsB.ID, "second create at same path must return existing workspace")
415+ require.Equal(t, protoA.ID, protoB.ID)
416+
417+ wsA.clientsMu.Lock()
418+ require.Contains(t, wsA.clients, cidA)
419+ require.Contains(t, wsA.clients, cidB)
420+ wsA.clientsMu.Unlock()
421+}
422+
423+// TestPathDedupe_DifferentPaths_DifferentWorkspaces confirms that two
424+// CreateWorkspace calls at distinct paths produce distinct workspaces.
425+func TestPathDedupe_DifferentPaths_DifferentWorkspaces(t *testing.T) {
426+ t.Setenv("HOME", t.TempDir())
427+ t.Setenv("XDG_CACHE_HOME", t.TempDir())
428+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
429+ t.Setenv("XDG_DATA_HOME", t.TempDir())
430+
431+ cwdA := t.TempDir()
432+ cwdB := t.TempDir()
433+ dataA := t.TempDir()
434+ dataB := t.TempDir()
435+
436+ b := New(context.Background(), nil, func() {})
437+ b.SetCreateGrace(2 * time.Second)
438+ t.Cleanup(func() { drainBackend(t, b) })
439+
440+ wsA, _, err := b.CreateWorkspace(protoWS(cwdA, dataA, uuid.New().String()))
441+ require.NoError(t, err)
442+ wsB, _, err := b.CreateWorkspace(protoWS(cwdB, dataB, uuid.New().String()))
443+ require.NoError(t, err)
444+ require.NotEqual(t, wsA.ID, wsB.ID)
445+}
446+
447+// TestPathDedupe_FirstWinsKeepsOriginalEnv verifies that the second
448+// create at the same path returns the *originating* client's Env in
449+// its proto and does not mutate the existing workspace's YOLO/Debug
450+// flags.
451+func TestPathDedupe_FirstWinsKeepsOriginalEnv(t *testing.T) {
452+ t.Setenv("HOME", t.TempDir())
453+ t.Setenv("XDG_CACHE_HOME", t.TempDir())
454+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
455+ t.Setenv("XDG_DATA_HOME", t.TempDir())
456+
457+ cwd := t.TempDir()
458+ dataDir := t.TempDir()
459+
460+ b := New(context.Background(), nil, func() {})
461+ b.SetCreateGrace(2 * time.Second)
462+ t.Cleanup(func() { drainBackend(t, b) })
463+
464+ originalEnv := []string{"FOO=bar"}
465+ argsA := protoWS(cwd, dataDir, uuid.New().String())
466+ argsA.YOLO = true
467+ argsA.Env = originalEnv
468+ wsA, protoA, err := b.CreateWorkspace(argsA)
469+ require.NoError(t, err)
470+ require.True(t, protoA.YOLO)
471+ require.Equal(t, originalEnv, protoA.Env)
472+
473+ argsB := protoWS(cwd, dataDir, uuid.New().String())
474+ argsB.YOLO = false
475+ argsB.Debug = true
476+ argsB.Env = []string{"BAZ=qux"}
477+ _, protoB, err := b.CreateWorkspace(argsB)
478+ require.NoError(t, err)
479+ require.Equal(t, protoA.ID, protoB.ID)
480+ require.True(t, protoB.YOLO, "first wins: YOLO must remain true")
481+ require.Equal(t, originalEnv, protoB.Env, "proto must carry the originating client's Env")
482+ require.Equal(t, wsA.Cfg.Overrides().SkipPermissionRequests, true)
483+}
484+
485+// TestPathDedupe_Symlink confirms two paths that resolve to the same
486+// target share a workspace.
487+func TestPathDedupe_Symlink(t *testing.T) {
488+ if runtime.GOOS == "windows" {
489+ t.Skip("symlink semantics differ on windows")
490+ }
491+ t.Setenv("HOME", t.TempDir())
492+ t.Setenv("XDG_CACHE_HOME", t.TempDir())
493+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
494+ t.Setenv("XDG_DATA_HOME", t.TempDir())
495+
496+ real := t.TempDir()
497+ link := filepath.Join(t.TempDir(), "link")
498+ require.NoError(t, os.Symlink(real, link))
499+ dataDir := t.TempDir()
500+
501+ b := New(context.Background(), nil, func() {})
502+ b.SetCreateGrace(2 * time.Second)
503+ t.Cleanup(func() { drainBackend(t, b) })
504+
505+ wsA, _, err := b.CreateWorkspace(protoWS(real, dataDir, uuid.New().String()))
506+ require.NoError(t, err)
507+ wsB, _, err := b.CreateWorkspace(protoWS(link, dataDir, uuid.New().String()))
508+ require.NoError(t, err)
509+ require.Equal(t, wsA.ID, wsB.ID)
510+}
511+
512+// TestPathDedupe_NonExistentPath ensures CreateWorkspace tolerates a
513+// path that does not yet exist (EvalSymlinks falls back to Abs).
514+func TestPathDedupe_NonExistentPath(t *testing.T) {
515+ t.Setenv("HOME", t.TempDir())
516+ t.Setenv("XDG_CACHE_HOME", t.TempDir())
517+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
518+ t.Setenv("XDG_DATA_HOME", t.TempDir())
519+
520+ parent := t.TempDir()
521+ missing := filepath.Join(parent, "does-not-exist")
522+ dataDir := t.TempDir()
523+
524+ b := New(context.Background(), nil, func() {})
525+ b.SetCreateGrace(2 * time.Second)
526+ t.Cleanup(func() { drainBackend(t, b) })
527+
528+ _, p, err := b.CreateWorkspace(protoWS(missing, dataDir, uuid.New().String()))
529+ require.NoError(t, err)
530+ require.NotEmpty(t, p.ID)
531+}
532+
533+// TestCreateWorkspace_IdempotentSameClient checks that a duplicate
534+// create from the same client at the same path does not produce a
535+// second claim.
536+func TestCreateWorkspace_IdempotentSameClient(t *testing.T) {
537+ t.Setenv("HOME", t.TempDir())
538+ t.Setenv("XDG_CACHE_HOME", t.TempDir())
539+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
540+ t.Setenv("XDG_DATA_HOME", t.TempDir())
541+
542+ cwd := t.TempDir()
543+ dataDir := t.TempDir()
544+ b := New(context.Background(), nil, func() {})
545+ b.SetCreateGrace(2 * time.Second)
546+ t.Cleanup(func() { drainBackend(t, b) })
547+
548+ cid := uuid.New().String()
549+ ws1, _, err := b.CreateWorkspace(protoWS(cwd, dataDir, cid))
550+ require.NoError(t, err)
551+ ws2, _, err := b.CreateWorkspace(protoWS(cwd, dataDir, cid))
552+ require.NoError(t, err)
553+ require.Equal(t, ws1.ID, ws2.ID)
554+
555+ ws1.clientsMu.Lock()
556+ require.Len(t, ws1.clients, 1, "duplicate create from same client must not double the claim")
557+ ws1.clientsMu.Unlock()
558+}
559+
560+// TestPathDedupe_ParallelCreates ensures two simultaneous CreateWorkspace
561+// calls at the same path produce the same workspace and the clients map
562+// contains both client IDs.
563+func TestPathDedupe_ParallelCreates(t *testing.T) {
564+ t.Setenv("HOME", t.TempDir())
565+ t.Setenv("XDG_CACHE_HOME", t.TempDir())
566+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
567+ t.Setenv("XDG_DATA_HOME", t.TempDir())
568+
569+ cwd := t.TempDir()
570+ dataDir := t.TempDir()
571+
572+ b := New(context.Background(), nil, func() {})
573+ b.SetCreateGrace(2 * time.Second)
574+ t.Cleanup(func() { drainBackend(t, b) })
575+
576+ cidA := uuid.New().String()
577+ cidB := uuid.New().String()
578+
579+ type result struct {
580+ ws *Workspace
581+ proto proto.Workspace
582+ err error
583+ }
584+ ch := make(chan result, 2)
585+ start := make(chan struct{})
586+ go func() {
587+ <-start
588+ ws, p, err := b.CreateWorkspace(protoWS(cwd, dataDir, cidA))
589+ ch <- result{ws, p, err}
590+ }()
591+ go func() {
592+ <-start
593+ ws, p, err := b.CreateWorkspace(protoWS(cwd, dataDir, cidB))
594+ ch <- result{ws, p, err}
595+ }()
596+ close(start)
597+ r1 := <-ch
598+ r2 := <-ch
599+ require.NoError(t, r1.err)
600+ require.NoError(t, r2.err)
601+ require.Equal(t, r1.ws.ID, r2.ws.ID, "both creates must converge on one workspace ID")
602+
603+ ws := r1.ws
604+ ws.clientsMu.Lock()
605+ defer ws.clientsMu.Unlock()
606+ require.Contains(t, ws.clients, cidA)
607+ require.Contains(t, ws.clients, cidB)
608+}
609+
610+// TestCreateWorkspace_RejectsBadClientID covers the 400 path from the
611+// backend side.
612+func TestCreateWorkspace_RejectsBadClientID(t *testing.T) {
613+ t.Parallel()
614+
615+ b := New(context.Background(), nil, func() {})
616+
617+ _, _, err := b.CreateWorkspace(protoWS("/tmp/x", t.TempDir(), ""))
618+ require.ErrorIs(t, err, ErrInvalidClientID)
619+ _, _, err = b.CreateWorkspace(protoWS("/tmp/x", t.TempDir(), "not-a-uuid"))
620+ require.ErrorIs(t, err, ErrInvalidClientID)
621+}
622+
623+// drainBackend tears the backend down at the end of a test by deleting
624+// every remaining workspace. Necessary so the test process doesn't
625+// leak goroutines or DB handles from the embedded [app.App] instances.
626+func drainBackend(t *testing.T, b *Backend) {
151 t.Helper()
627 t.Helper()
152− deadline := time.After(timeout)
628+ for _, ws := range b.workspaces.Seq2() {
153− for {
629+ ws.clientsMu.Lock()
154− select {
630+ ids := make([]string, 0, len(ws.clients))
155− case ev, ok := <-ch:
631+ for id := range ws.clients {
156− if !ok {
632+ ids = append(ids, id)
157− return false
158− }
633+ }
159− se, ok := ev.Payload.(pubsub.Event[skills.Event])
634+ ws.clientsMu.Unlock()
160− if !ok {
635+ for _, cid := range ids {
161− continue
636+ _ = b.releaseHold(ws.ID, cid)
637+ }
638+ }
162− }
639+}
163− if containsSkillName(se.Payload.States, marker) {
640+
164− return true
641+func protoWS(path, dataDir, clientID string) proto.Workspace {
642+ return proto.Workspace{Path: path, DataDir: dataDir, ClientID: clientID}
165− }
643+}
166− case <-deadline:
644+
167− return false
645+// syncBuffer is a thread-safe buffer that can be safely read and written
646+// from multiple goroutines.
647+type syncBuffer struct {
648+ mu sync.Mutex
649+ buf bytes.Buffer
650+}
651+
652+func (sb *syncBuffer) Write(p []byte) (n int, err error) {
653+ sb.mu.Lock()
654+ defer sb.mu.Unlock()
655+ return sb.buf.Write(p)
656+}
657+
658+func (sb *syncBuffer) String() string {
659+ sb.mu.Lock()
660+ defer sb.mu.Unlock()
661+ return sb.buf.String()
662+}
663+
664+// captureDebugLogs installs a buffer-backed slog handler at Debug
665+// level for the duration of the test, returning the buffer. The
666+// previous default handler is restored via t.Cleanup.
667+func captureDebugLogs(t *testing.T) *syncBuffer {
668+ t.Helper()
669+ var sb syncBuffer
670+ prev := slog.Default()
671+ handler := slog.NewTextHandler(&sb, &slog.HandlerOptions{Level: slog.LevelDebug})
672+ slog.SetDefault(slog.New(handler))
673+ t.Cleanup(func() { slog.SetDefault(prev) })
674+ return &sb
675+}
676+
677+// xdgIsolated points HOME and XDG_* variables at fresh tempdirs so
678+// CreateWorkspace's config loading does not interfere with the host
679+// machine's real config.
680+func xdgIsolated(t *testing.T) {
681+ t.Helper()
682+ t.Setenv("HOME", t.TempDir())
683+ t.Setenv("XDG_CACHE_HOME", t.TempDir())
684+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
685+ t.Setenv("XDG_DATA_HOME", t.TempDir())
686+}
687+
688+// TestFirstWinsMismatch_LogsOnFlagDifferences verifies that the
689+// debug mismatch line is emitted when any of YOLO, Debug, DataDir,
690+// or Env differs between the first and second CreateWorkspace at
691+// the same path, and that the existing workspace's Debug flag is
692+// not overwritten.
693+func TestFirstWinsMismatch_LogsOnFlagDifferences(t *testing.T) {
694+ tests := []struct {
695+ name string
696+ mutate func(*proto.Workspace)
697+ }{
698+ {
699+ name: "yolo",
700+ mutate: func(p *proto.Workspace) { p.YOLO = true },
701+ },
702+ {
703+ name: "debug",
704+ mutate: func(p *proto.Workspace) { p.Debug = true },
705+ },
706+ {
707+ name: "datadir",
708+ mutate: func(p *proto.Workspace) { p.DataDir = "" },
709+ },
710+ {
711+ name: "env",
712+ mutate: func(p *proto.Workspace) { p.Env = []string{"NEW=val"} },
713+ },
714+ }
715+
716+ for _, tc := range tests {
717+ t.Run(tc.name, func(t *testing.T) {
718+ xdgIsolated(t)
719+ cwd := t.TempDir()
720+ dataDir := t.TempDir()
721+
722+ buf := captureDebugLogs(t)
723+ b := New(context.Background(), nil, func() {})
724+ b.SetCreateGrace(2 * time.Second)
725+ t.Cleanup(func() { drainBackend(t, b) })
726+
727+ argsA := protoWS(cwd, dataDir, uuid.New().String())
728+ argsA.Env = []string{"FOO=bar"}
729+ wsA, _, err := b.CreateWorkspace(argsA)
730+ require.NoError(t, err)
731+ originalDebug := wsA.Cfg.Config().Options.Debug
732+ originalYOLO := wsA.Cfg.Overrides().SkipPermissionRequests
733+
734+ argsB := protoWS(cwd, dataDir, uuid.New().String())
735+ argsB.Env = []string{"FOO=bar"} // identical by default
736+ tc.mutate(&argsB)
737+ _, _, err = b.CreateWorkspace(argsB)
738+ require.NoError(t, err)
739+
740+ require.Contains(
741+ t, buf.String(),
742+ "Workspace flag mismatch on duplicate create",
743+ "expected debug log for mismatching %s", tc.name,
744+ )
745+ // Existing workspace's YOLO and Debug must not change.
746+ require.Equal(t, originalYOLO, wsA.Cfg.Overrides().SkipPermissionRequests, "YOLO must be immutable on first-wins")
747+ require.Equal(t, originalDebug, wsA.Cfg.Config().Options.Debug, "Debug must be immutable on first-wins")
748+ })
749+ }
750+}
751+
752+// TestFirstWinsMismatch_NoLogWhenIdentical confirms identical args
753+// do not emit the mismatch log line.
754+func TestFirstWinsMismatch_NoLogWhenIdentical(t *testing.T) {
755+ xdgIsolated(t)
756+ cwd := t.TempDir()
757+ dataDir := t.TempDir()
758+
759+ buf := captureDebugLogs(t)
760+ b := New(context.Background(), nil, func() {})
761+ b.SetCreateGrace(2 * time.Second)
762+ t.Cleanup(func() { drainBackend(t, b) })
763+
764+ argsA := protoWS(cwd, dataDir, uuid.New().String())
765+ argsA.Env = []string{"FOO=bar"}
766+ _, _, err := b.CreateWorkspace(argsA)
767+ require.NoError(t, err)
768+
769+ argsB := protoWS(cwd, dataDir, uuid.New().String())
770+ argsB.Env = []string{"FOO=bar"}
771+ _, _, err = b.CreateWorkspace(argsB)
772+ require.NoError(t, err)
773+
774+ require.False(t,
775+ strings.Contains(buf.String(), "Workspace flag mismatch on duplicate create"),
776+ "identical args must not log a mismatch: %s", buf.String())
777+}
778+
779+// TestRaceTwoClientsAttachOneDetaches exercises the PLAN-required
780+// race scenario: two clients attach concurrently, then one detaches.
781+// The workspace must remain alive with refcount==1 and the clients
782+// map must reflect the remaining client only.
783+func TestRaceTwoClientsAttachOneDetaches(t *testing.T) {
784+ t.Parallel()
785+
786+ b, _ := newTestBackend(t)
787+ ws, shutdowns := insertTestWorkspace(t, b, "/tmp/race-two")
788+
789+ cidA := newClientID(t)
790+ cidB := newClientID(t)
791+
792+ var wg sync.WaitGroup
793+ wg.Add(2)
794+ go func() {
795+ defer wg.Done()
796+ require.NoError(t, b.AttachClient(ws.ID, cidA))
797+ }()
798+ go func() {
799+ defer wg.Done()
800+ require.NoError(t, b.AttachClient(ws.ID, cidB))
801+ }()
802+ wg.Wait()
803+
804+ ws.clientsMu.Lock()
805+ require.Len(t, ws.clients, 2, "both clients must be attached")
806+ ws.clientsMu.Unlock()
807+
808+ b.DetachClient(ws.ID, cidA)
809+
810+ ws.clientsMu.Lock()
811+ require.Len(t, ws.clients, 1, "refcount must be 1 after one detach")
812+ require.Contains(t, ws.clients, cidB, "remaining client must be cidB")
813+ require.NotContains(t, ws.clients, cidA, "detached client must be removed")
814+ ws.clientsMu.Unlock()
815+ require.Equal(t, int32(0), shutdowns.Load(), "workspace must remain alive")
816+
817+ // Drain.
818+ b.DetachClient(ws.ID, cidB)
819+ require.Equal(t, int32(1), shutdowns.Load())
820+}
821+
822+// TestExplicitDeleteThenAttach reproduces the PLAN scenario: start
823+// with a real hold, releaseHold consumes it, AttachClient from the
824+// same clientID creates a fresh entry with streams==1, and calling
825+// releaseHold again is a no-op. A second client keeps the workspace
826+// alive so AttachClient can still resolve the workspace ID after the
827+// first client's hold is released.
828+func TestExplicitDeleteThenAttach(t *testing.T) {
829+ t.Parallel()
830+
831+ // Large grace window so timers cannot fire during the test
832+ // — we want to exercise the explicit releaseHold path.
833+ b, _ := newTestBackend(t)
834+ b.createGrace = time.Hour
835+ ws, shutdowns := insertTestWorkspace(t, b, "/tmp/delete-then-attach")
836+
837+ // Anchor client keeps the workspace registered in
838+ // b.workspaces across the cid's releaseHold below.
839+ anchor := newClientID(t)
840+ require.NoError(t, b.AttachClient(ws.ID, anchor))
841+
842+ cid := newClientID(t)
843+ // Real hold via registerClient (mirrors CreateWorkspace).
844+ b.registerClient(ws, cid)
845+ ws.clientsMu.Lock()
846+ require.Contains(t, ws.clients, cid)
847+ require.NotNil(t, ws.clients[cid].holdTimer, "hold must be live")
848+ require.Equal(t, 0, ws.clients[cid].streams)
849+ ws.clientsMu.Unlock()
850+
851+ // releaseHold: consumes the hold and removes the entry
852+ // (streams == 0). The anchor client keeps the workspace
853+ // alive.
854+ require.NoError(t, b.releaseHold(ws.ID, cid))
855+ require.Equal(t, int32(0), shutdowns.Load(), "anchor must keep workspace alive")
856+ ws.clientsMu.Lock()
857+ require.NotContains(t, ws.clients, cid, "entry must be removed by releaseHold")
858+ ws.clientsMu.Unlock()
859+
860+ // AttachClient creates a fresh entry with streams==1 and no
861+ // hold timer.
862+ require.NoError(t, b.AttachClient(ws.ID, cid))
863+ ws.clientsMu.Lock()
864+ require.Contains(t, ws.clients, cid, "fresh entry must be created")
865+ require.Equal(t, 1, ws.clients[cid].streams, "fresh attach must start at streams=1")
866+ require.Nil(t, ws.clients[cid].holdTimer, "fresh attach must have no hold timer")
867+ ws.clientsMu.Unlock()
868+
869+ // Calling releaseHold again is a no-op (no hold timer to
870+ // stop, streams > 0 so the entry stays).
871+ require.NoError(t, b.releaseHold(ws.ID, cid))
872+ ws.clientsMu.Lock()
873+ require.Contains(t, ws.clients, cid, "releaseHold must not touch a stream-only entry")
874+ require.Equal(t, 1, ws.clients[cid].streams)
875+ require.Nil(t, ws.clients[cid].holdTimer)
876+ ws.clientsMu.Unlock()
877+
878+ // Drain.
879+ b.DetachClient(ws.ID, cid)
880+ b.DetachClient(ws.ID, anchor)
881+ require.Equal(t, int32(1), shutdowns.Load())
882+}
883+
884+// TestAttachClient_RacesWithTeardown forces AttachClient to compete
885+// with the teardown path triggered by DetachClient. Before the fix,
886+// AttachClient could observe a workspace after teardown had already
887+// decided to remove it (because AttachClient did not synchronize with
888+// Backend.mu), leaving a live stream claim attached to a workspace
889+// that was then removed and shut down. With the fix, the outcome must
890+// be deterministic: either AttachClient won and the workspace is
891+// alive with the client registered, or teardown won and AttachClient
892+// returns ErrWorkspaceNotFound — never a half-state where the
893+// workspace is gone but ws.clients still contains the new client.
894+func TestAttachClient_RacesWithTeardown(t *testing.T) {
895+ t.Parallel()
896+
897+ for i := range 200 {
898+ b, _ := newTestBackend(t)
899+ // Keep the grace window long so it can't fire during the
900+ // test and confuse the bookkeeping.
901+ b.createGrace = time.Hour
902+ ws, shutdowns := insertTestWorkspace(t, b, "/tmp/race-teardown")
903+
904+ // Seed: cidA holds the workspace open via a stream. The
905+ // imminent DetachClient(cidA) will be the *only* claim
906+ // drop, so teardown will run.
907+ cidA := newClientID(t)
908+ require.NoError(t, b.AttachClient(ws.ID, cidA))
909+
910+ // cidB attempts to attach concurrently with the detach
911+ // that will tear the workspace down.
912+ cidB := newClientID(t)
913+ start := make(chan struct{})
914+ errCh := make(chan error, 1)
915+ detachDone := make(chan struct{})
916+ go func() {
917+ <-start
918+ errCh <- b.AttachClient(ws.ID, cidB)
919+ }()
920+ go func() {
921+ <-start
922+ b.DetachClient(ws.ID, cidA)
923+ close(detachDone)
924+ }()
925+ close(start)
926+
927+ // Wait for both goroutines so teardown (including
928+ // shutdownFn) has fully run before we read state.
929+ attachErr := <-errCh
930+ <-detachDone
931+
932+ _, wsStillRegistered := b.workspaces.Get(ws.ID)
933+ ws.clientsMu.Lock()
934+ _, hasA := ws.clients[cidA]
935+ _, hasB := ws.clients[cidB]
936+ clientCount := len(ws.clients)
937+ ws.clientsMu.Unlock()
938+ shutdownCount := shutdowns.Load()
939+
940+ switch {
941+ case attachErr == nil:
942+ // AttachClient won. The workspace must be alive
943+ // (registered) with cidB in its clients map. cidA
944+ // may or may not still be there depending on who
945+ // took clientsMu first, but the workspace must
946+ // not have been torn down.
947+ require.True(t, wsStillRegistered,
948+ "iter %d: attach succeeded but workspace was removed", i)
949+ require.True(t, hasB,
950+ "iter %d: attach succeeded but cidB missing from clients", i)
951+ require.Equal(t, int32(0), shutdownCount,
952+ "iter %d: attach succeeded but workspace was shut down", i)
953+ case errors.Is(attachErr, ErrWorkspaceNotFound):
954+ // Teardown won. The workspace must be removed,
955+ // shut down exactly once, and ws.clients must be
956+ // empty (no half-state with cidB inserted into a
957+ // dead workspace's clients map).
958+ require.False(t, wsStillRegistered,
959+ "iter %d: ErrWorkspaceNotFound but workspace still registered", i)
960+ require.Equal(t, int32(1), shutdownCount,
961+ "iter %d: ErrWorkspaceNotFound but shutdown count = %d", i, shutdownCount)
962+ require.False(t, hasA,
963+ "iter %d: teardown won but cidA still in clients", i)
964+ require.False(t, hasB,
965+ "iter %d: teardown won but cidB still in clients (would be the leaked attach)", i)
966+ require.Zero(t, clientCount,
967+ "iter %d: teardown won but clients map is non-empty", i)
968+ default:
969+ t.Fatalf("iter %d: unexpected AttachClient error: %v", i, attachErr)
168 }
970 }
169 }
971 }
170}
972}
973+
974+// TestSetCurrentSession_BasicAttachAndSwitch verifies the happy path:
975+// an attached client can set its current session, a second attached
976+// client can target the same session, and one of them can switch to a
977+// different session without disturbing the other's record.
978+func TestSetCurrentSession_BasicAttachAndSwitch(t *testing.T) {
979+ t.Parallel()
980+
981+ b, _ := newTestBackend(t)
982+ ws, _ := insertTestWorkspace(t, b, "/tmp/current-session-basic")
983+
984+ cidA := newClientID(t)
985+ cidB := newClientID(t)
986+ require.NoError(t, b.AttachClient(ws.ID, cidA))
987+ require.NoError(t, b.AttachClient(ws.ID, cidB))
988+
989+ require.NoError(t, b.SetCurrentSession(ws.ID, cidA, "S1"))
990+ ws.clientsMu.Lock()
991+ require.Equal(t, "S1", ws.clients[cidA].currentSessionID)
992+ ws.clientsMu.Unlock()
993+
994+ require.NoError(t, b.SetCurrentSession(ws.ID, cidB, "S1"))
995+ ws.clientsMu.Lock()
996+ require.Equal(t, "S1", ws.clients[cidA].currentSessionID)
997+ require.Equal(t, "S1", ws.clients[cidB].currentSessionID)
998+ ws.clientsMu.Unlock()
999+
1000+ // B switches to S2; counts redistribute.
1001+ require.NoError(t, b.SetCurrentSession(ws.ID, cidB, "S2"))
1002+ ws.clientsMu.Lock()
1003+ require.Equal(t, "S1", ws.clients[cidA].currentSessionID)
1004+ require.Equal(t, "S2", ws.clients[cidB].currentSessionID)
1005+ ws.clientsMu.Unlock()
1006+
1007+ // A clears its selection.
1008+ require.NoError(t, b.SetCurrentSession(ws.ID, cidA, ""))
1009+ ws.clientsMu.Lock()
1010+ require.Empty(t, ws.clients[cidA].currentSessionID)
1011+ require.Equal(t, "S2", ws.clients[cidB].currentSessionID)
1012+ ws.clientsMu.Unlock()
1013+
1014+ // Drain to release the workspace.
1015+ b.DetachClient(ws.ID, cidA)
1016+ b.DetachClient(ws.ID, cidB)
1017+}
1018+
1019+// TestSetCurrentSession_DetachClearsEntry verifies the implicit
1020+// cleanup: once a client's [clientState] entry is removed (last
1021+// stream closed), its currentSessionID is gone with it.
1022+func TestSetCurrentSession_DetachClearsEntry(t *testing.T) {
1023+ t.Parallel()
1024+
1025+ b, _ := newTestBackend(t)
1026+ ws, _ := insertTestWorkspace(t, b, "/tmp/current-session-detach")
1027+
1028+ // Anchor client so the workspace is not torn down when cid
1029+ // detaches.
1030+ anchor := newClientID(t)
1031+ require.NoError(t, b.AttachClient(ws.ID, anchor))
1032+
1033+ cid := newClientID(t)
1034+ require.NoError(t, b.AttachClient(ws.ID, cid))
1035+ require.NoError(t, b.SetCurrentSession(ws.ID, cid, "S2"))
1036+
1037+ b.DetachClient(ws.ID, cid)
1038+
1039+ ws.clientsMu.Lock()
1040+ _, present := ws.clients[cid]
1041+ ws.clientsMu.Unlock()
1042+ require.False(t, present, "detach must remove the clientState entry along with its currentSessionID")
1043+
1044+ // A follow-up SetCurrentSession on the gone client must be
1045+ // rejected with ErrClientNotAttached.
1046+ require.ErrorIs(t, b.SetCurrentSession(ws.ID, cid, "S3"), ErrClientNotAttached)
1047+
1048+ b.DetachClient(ws.ID, anchor)
1049+}
1050+
1051+// TestSetCurrentSession_RejectsHoldOnly verifies that a registered
1052+// client whose only claim is a creation hold (streams == 0) cannot
1053+// influence presence: SetCurrentSession returns ErrClientNotAttached
1054+// and the entry's currentSessionID stays empty.
1055+func TestSetCurrentSession_RejectsHoldOnly(t *testing.T) {
1056+ t.Parallel()
1057+
1058+ b, _ := newTestBackend(t)
1059+ // Keep the grace window large so the hold survives the test.
1060+ b.createGrace = time.Hour
1061+ ws, _ := insertTestWorkspace(t, b, "/tmp/current-session-hold")
1062+
1063+ cid := newClientID(t)
1064+ b.registerClient(ws, cid)
1065+
1066+ require.ErrorIs(t, b.SetCurrentSession(ws.ID, cid, "S1"), ErrClientNotAttached)
1067+
1068+ ws.clientsMu.Lock()
1069+ require.Empty(t, ws.clients[cid].currentSessionID, "hold-only client must not write a session id")
1070+ ws.clientsMu.Unlock()
1071+
1072+ // Drain.
1073+ require.NoError(t, b.releaseHold(ws.ID, cid))
1074+}
1075+
1076+// TestSetCurrentSession_UnknownClient verifies that a client with no
1077+// entry at all is rejected with ErrClientNotAttached.
1078+func TestSetCurrentSession_UnknownClient(t *testing.T) {
1079+ t.Parallel()
1080+
1081+ b, _ := newTestBackend(t)
1082+ ws, _ := insertTestWorkspace(t, b, "/tmp/current-session-unknown")
1083+
1084+ require.ErrorIs(t, b.SetCurrentSession(ws.ID, newClientID(t), "S1"), ErrClientNotAttached)
1085+}
1086+
1087+// TestSetCurrentSession_RejectsBadInputs covers the validation
1088+// branches: empty/malformed client_id and unknown workspace.
1089+func TestSetCurrentSession_RejectsBadInputs(t *testing.T) {
1090+ t.Parallel()
1091+
1092+ b, _ := newTestBackend(t)
1093+ ws, _ := insertTestWorkspace(t, b, "/tmp/current-session-bad")
1094+
1095+ require.ErrorIs(t, b.SetCurrentSession(ws.ID, "", "S1"), ErrInvalidClientID)
1096+ require.ErrorIs(t, b.SetCurrentSession(ws.ID, "not-a-uuid", "S1"), ErrInvalidClientID)
1097+
1098+ require.ErrorIs(
1099+ t,
1100+ b.SetCurrentSession("00000000-0000-0000-0000-000000000000", newClientID(t), "S1"),
1101+ ErrWorkspaceNotFound,
1102+ )
1103+}
1104+
1105+// TestSetCurrentSession_RaceWithDetach exercises concurrent
1106+// SetCurrentSession updates from one client racing against detach
1107+// on a second client. The final state must be self-consistent: any
1108+// remaining clientState entries reflect a coherent
1109+// (streams, currentSessionID) pair.
1110+func TestSetCurrentSession_RaceWithDetach(t *testing.T) {
1111+ t.Parallel()
1112+
1113+ b, _ := newTestBackend(t)
1114+ ws, _ := insertTestWorkspace(t, b, "/tmp/current-session-race")
1115+
1116+ cidA := newClientID(t)
1117+ cidB := newClientID(t)
1118+ require.NoError(t, b.AttachClient(ws.ID, cidA))
1119+ require.NoError(t, b.AttachClient(ws.ID, cidB))
1120+
1121+ var wg sync.WaitGroup
1122+ const updates = 200
1123+ wg.Add(3)
1124+ go func() {
1125+ defer wg.Done()
1126+ for i := range updates {
1127+ // Errors are tolerated: once cidA detaches,
1128+ // further updates against cidA must return
1129+ // ErrClientNotAttached but never panic.
1130+ _ = b.SetCurrentSession(ws.ID, cidA, "SA")
1131+ _ = i
1132+ }
1133+ }()
1134+ go func() {
1135+ defer wg.Done()
1136+ for i := range updates {
1137+ _ = b.SetCurrentSession(ws.ID, cidB, "SB")
1138+ _ = i
1139+ }
1140+ }()
1141+ go func() {
1142+ defer wg.Done()
1143+ // Single concurrent detach of cidA partway through.
1144+ b.DetachClient(ws.ID, cidA)
1145+ }()
1146+ wg.Wait()
1147+
1148+ ws.clientsMu.Lock()
1149+ defer ws.clientsMu.Unlock()
1150+ require.NotContains(t, ws.clients, cidA, "detached client must be gone")
1151+ require.Contains(t, ws.clients, cidB, "remaining client must still be present")
1152+ require.Equal(t, "SB", ws.clients[cidB].currentSessionID, "remaining client must keep its last set session")
1153+}
1154+
1155+// TestAttachedClients_BasicLifecycle walks one session's count through
1156+// attach -> set -> second client joins -> switch -> detach. It also
1157+// confirms hold-only and unselected clients do not contribute.
1158+func TestAttachedClients_BasicLifecycle(t *testing.T) {
1159+ t.Parallel()
1160+
1161+ b, _ := newTestBackend(t)
1162+ // Keep the grace window long so the hold-only client survives.
1163+ b.createGrace = time.Hour
1164+ ws, _ := insertTestWorkspace(t, b, "/tmp/attached-clients-basic")
1165+
1166+ // No clients yet.
1167+ n, err := b.AttachedClients(ws.ID, "S1")
1168+ require.NoError(t, err)
1169+ require.Zero(t, n)
1170+
1171+ // Attach A, set to S1. Count for S1 is 1; count for S2 is 0.
1172+ cidA := newClientID(t)
1173+ require.NoError(t, b.AttachClient(ws.ID, cidA))
1174+ require.NoError(t, b.SetCurrentSession(ws.ID, cidA, "S1"))
1175+
1176+ n, err = b.AttachedClients(ws.ID, "S1")
1177+ require.NoError(t, err)
1178+ require.Equal(t, 1, n)
1179+ n, err = b.AttachedClients(ws.ID, "S2")
1180+ require.NoError(t, err)
1181+ require.Zero(t, n)
1182+
1183+ // Attach B, set to S1. Count for S1 is 2.
1184+ cidB := newClientID(t)
1185+ require.NoError(t, b.AttachClient(ws.ID, cidB))
1186+ require.NoError(t, b.SetCurrentSession(ws.ID, cidB, "S1"))
1187+
1188+ n, _ = b.AttachedClients(ws.ID, "S1")
1189+ require.Equal(t, 2, n)
1190+
1191+ // B switches to S2; counts redistribute.
1192+ require.NoError(t, b.SetCurrentSession(ws.ID, cidB, "S2"))
1193+ n, _ = b.AttachedClients(ws.ID, "S1")
1194+ require.Equal(t, 1, n)
1195+ n, _ = b.AttachedClients(ws.ID, "S2")
1196+ require.Equal(t, 1, n)
1197+
1198+ // A hold-only client must NOT be counted, even if we were able to
1199+ // imagine a currentSessionID on it. registerClient leaves
1200+ // currentSessionID empty by construction, and SetCurrentSession
1201+ // rejects hold-only writers — so the contract holds two ways.
1202+ cidHold := newClientID(t)
1203+ b.registerClient(ws, cidHold)
1204+ t.Cleanup(func() { _ = b.releaseHold(ws.ID, cidHold) })
1205+ n, _ = b.AttachedClients(ws.ID, "S1")
1206+ require.Equal(t, 1, n, "hold-only client must not contribute")
1207+ n, _ = b.AttachedClients(ws.ID, "")
1208+ require.Equal(t, 0, n,
1209+ "empty sessionID must not match the hold-only entry (streams==0)")
1210+
1211+ // A client with streams > 0 but currentSessionID == "" is NOT
1212+ // counted toward any non-empty session, and is matched only
1213+ // against the empty session id (which represents the landing
1214+ // screen).
1215+ cidC := newClientID(t)
1216+ require.NoError(t, b.AttachClient(ws.ID, cidC))
1217+ n, _ = b.AttachedClients(ws.ID, "S1")
1218+ require.Equal(t, 1, n, "stream-only client with empty currentSessionID must not be counted toward S1")
1219+ n, _ = b.AttachedClients(ws.ID, "")
1220+ require.Equal(t, 1, n, "stream-only client with empty currentSessionID matches the empty session id")
1221+
1222+ // B detaches: count for S2 drops to 0.
1223+ b.DetachClient(ws.ID, cidB)
1224+ n, _ = b.AttachedClients(ws.ID, "S2")
1225+ require.Zero(t, n)
1226+ n, _ = b.AttachedClients(ws.ID, "S1")
1227+ require.Equal(t, 1, n, "A still on S1")
1228+
1229+ // Final cleanup.
1230+ b.DetachClient(ws.ID, cidA)
1231+ b.DetachClient(ws.ID, cidC)
1232+}
1233+
1234+// TestAttachedClients_UnknownWorkspace verifies the error surface.
1235+func TestAttachedClients_UnknownWorkspace(t *testing.T) {
1236+ t.Parallel()
1237+
1238+ b, _ := newTestBackend(t)
1239+ _, err := b.AttachedClients("00000000-0000-0000-0000-000000000000", "S1")
1240+ require.ErrorIs(t, err, ErrWorkspaceNotFound)
1241+}
internal/backend/config.go+54−7
@@ -11,9 +11,23 @@ import (
11 "github.com/charmbracelet/crush/internal/config"
11 "github.com/charmbracelet/crush/internal/config"
12 "github.com/charmbracelet/crush/internal/oauth"
12 "github.com/charmbracelet/crush/internal/oauth"
13 "github.com/charmbracelet/crush/internal/proto"
13 "github.com/charmbracelet/crush/internal/proto"
14+ "github.com/charmbracelet/crush/internal/pubsub"
14 "github.com/charmbracelet/crush/internal/skills"
15 "github.com/charmbracelet/crush/internal/skills"
15)
16)
16
17
18+// publishConfigChanged publishes a ConfigChanged event on the workspace's
19+// event broker so all subscribers (e.g. remote clients) refresh their
20+// cached config snapshot.
21+func publishConfigChanged(ws *Workspace) {
22+ if ws == nil || ws.App == nil {
23+ return
24+ }
25+ ws.SendEvent(pubsub.Event[proto.ConfigChanged]{
26+ Type: pubsub.UpdatedEvent,
27+ Payload: proto.ConfigChanged{WorkspaceID: ws.ID},
28+ })
29+}
30+
17// MCPResourceContents holds the contents of an MCP resource returned
31// MCPResourceContents holds the contents of an MCP resource returned
18// by the backend.
32// by the backend.
19type MCPResourceContents struct {
33type MCPResourceContents struct {
@@ -30,7 +44,11 @@ func (b *Backend) SetConfigField(workspaceID string, scope config.Scope, key str
30 if err != nil {
44 if err != nil {
31 return err
45 return err
32 }
46 }
33− return ws.Cfg.SetConfigField(scope, key, value)
47+ if err := ws.Cfg.SetConfigField(scope, key, value); err != nil {
48+ return err
49+ }
50+ publishConfigChanged(ws)
51+ return nil
34}
52}
35
53
36// RemoveConfigField removes a key from the config file for the given
54// RemoveConfigField removes a key from the config file for the given
@@ -40,7 +58,11 @@ func (b *Backend) RemoveConfigField(workspaceID string, scope config.Scope, key
40 if err != nil {
58 if err != nil {
41 return err
59 return err
42 }
60 }
43− return ws.Cfg.RemoveConfigField(scope, key)
61+ if err := ws.Cfg.RemoveConfigField(scope, key); err != nil {
62+ return err
63+ }
64+ publishConfigChanged(ws)
65+ return nil
44}
66}
45
67
46// UpdatePreferredModel updates the preferred model for the given type
68// UpdatePreferredModel updates the preferred model for the given type
@@ -50,7 +72,11 @@ func (b *Backend) UpdatePreferredModel(workspaceID string, scope config.Scope, m
50 if err != nil {
72 if err != nil {
51 return err
73 return err
52 }
74 }
53− return ws.Cfg.UpdatePreferredModel(scope, modelType, model)
75+ if err := ws.Cfg.UpdatePreferredModel(scope, modelType, model); err != nil {
76+ return err
77+ }
78+ publishConfigChanged(ws)
79+ return nil
54}
80}
55
81
56// SetCompactMode sets the compact mode setting and persists it.
82// SetCompactMode sets the compact mode setting and persists it.
@@ -59,7 +85,11 @@ func (b *Backend) SetCompactMode(workspaceID string, scope config.Scope, enabled
59 if err != nil {
85 if err != nil {
60 return err
86 return err
61 }
87 }
62− return ws.Cfg.SetCompactMode(scope, enabled)
88+ if err := ws.Cfg.SetCompactMode(scope, enabled); err != nil {
89+ return err
90+ }
91+ publishConfigChanged(ws)
92+ return nil
63}
93}
64
94
65// SetProviderAPIKey sets the API key for a provider and persists it.
95// SetProviderAPIKey sets the API key for a provider and persists it.
@@ -68,7 +98,11 @@ func (b *Backend) SetProviderAPIKey(workspaceID string, scope config.Scope, prov
68 if err != nil {
98 if err != nil {
69 return err
99 return err
70 }
100 }
71− return ws.Cfg.SetProviderAPIKey(scope, providerID, apiKey)
101+ if err := ws.Cfg.SetProviderAPIKey(scope, providerID, apiKey); err != nil {
102+ return err
103+ }
104+ publishConfigChanged(ws)
105+ return nil
72}
106}
73
107
74// ImportCopilot attempts to import a GitHub Copilot token from disk.
108// ImportCopilot attempts to import a GitHub Copilot token from disk.
@@ -78,6 +112,9 @@ func (b *Backend) ImportCopilot(workspaceID string) (*oauth.Token, bool, error)
78 return nil, false, err
112 return nil, false, err
79 }
113 }
80 token, ok := ws.Cfg.ImportCopilot()
114 token, ok := ws.Cfg.ImportCopilot()
115+ if ok {
116+ publishConfigChanged(ws)
117+ }
81 return token, ok, nil
118 return token, ok, nil
82}
119}
83
120
@@ -87,7 +124,11 @@ func (b *Backend) RefreshOAuthToken(ctx context.Context, workspaceID string, sco
87 if err != nil {
124 if err != nil {
88 return err
125 return err
89 }
126 }
90− return ws.Cfg.RefreshOAuthToken(ctx, scope, providerID)
127+ if err := ws.Cfg.RefreshOAuthToken(ctx, scope, providerID); err != nil {
128+ return err
129+ }
130+ publishConfigChanged(ws)
131+ return nil
91}
132}
92
133
93// ProjectNeedsInitialization checks whether the project in this
134// ProjectNeedsInitialization checks whether the project in this
@@ -106,7 +147,11 @@ func (b *Backend) MarkProjectInitialized(workspaceID string) error {
106 if err != nil {
147 if err != nil {
107 return err
148 return err
108 }
149 }
109− return config.MarkProjectInitialized(ws.Cfg)
150+ if err := config.MarkProjectInitialized(ws.Cfg); err != nil {
151+ return err
152+ }
153+ publishConfigChanged(ws)
154+ return nil
110}
155}
111
156
112// InitializePrompt builds the initialization prompt for the workspace.
157// InitializePrompt builds the initialization prompt for the workspace.
@@ -186,6 +231,7 @@ func (b *Backend) EnableDockerMCP(ctx context.Context, workspaceID string) error
186 return fmt.Errorf("docker MCP started but failed to persist configuration: %w", errors.Join(err, disableErr))
231 return fmt.Errorf("docker MCP started but failed to persist configuration: %w", errors.Join(err, disableErr))
187 }
232 }
188
233
234+ publishConfigChanged(ws)
189 return nil
235 return nil
190}
236}
191
237
@@ -205,6 +251,7 @@ func (b *Backend) DisableDockerMCP(workspaceID string) error {
205 return err
251 return err
206 }
252 }
207
253
254+ publishConfigChanged(ws)
208 return nil
255 return nil
209}
256}
210
257
internal/backend/config_test.gonew file+207
@@ -0,0 +1,207 @@
1+package backend
2+
3+import (
4+ "context"
5+ "testing"
6+ "time"
7+
8+ tea "charm.land/bubbletea/v2"
9+ "github.com/charmbracelet/crush/internal/config"
10+ "github.com/charmbracelet/crush/internal/proto"
11+ "github.com/charmbracelet/crush/internal/pubsub"
12+ "github.com/google/uuid"
13+ "github.com/stretchr/testify/require"
14+)
15+
16+// awaitConfigChanged drains events until a ConfigChanged is received
17+// for the given workspace ID, or fails the test on timeout. Other
18+// event types are ignored.
19+func awaitConfigChanged(t *testing.T, evc <-chan pubsub.Event[tea.Msg], workspaceID string) {
20+ t.Helper()
21+ deadline := time.After(2 * time.Second)
22+ for {
23+ select {
24+ case ev, ok := <-evc:
25+ if !ok {
26+ t.Fatal("event channel closed before ConfigChanged arrived")
27+ }
28+ cc, ok := ev.Payload.(pubsub.Event[proto.ConfigChanged])
29+ if !ok {
30+ continue
31+ }
32+ require.Equal(t, workspaceID, cc.Payload.WorkspaceID)
33+ return
34+ case <-deadline:
35+ t.Fatal("timed out waiting for ConfigChanged event")
36+ }
37+ }
38+}
39+
40+// newPublishingWorkspace creates a real workspace through the backend
41+// so its embedded *app.App is wired up and SendEvent works. It returns
42+// the backend, the workspace, and a fresh event subscription.
43+func newPublishingWorkspace(t *testing.T) (*Backend, *Workspace, <-chan pubsub.Event[tea.Msg]) {
44+ t.Helper()
45+ xdgIsolated(t)
46+
47+ cwd := t.TempDir()
48+ dataDir := t.TempDir()
49+
50+ b := New(context.Background(), nil, func() {})
51+ b.SetCreateGrace(2 * time.Second)
52+ t.Cleanup(func() { drainBackend(t, b) })
53+
54+ cid := uuid.New().String()
55+ ws, _, err := b.CreateWorkspace(protoWS(cwd, dataDir, cid))
56+ require.NoError(t, err)
57+
58+ ctx, cancel := context.WithCancel(context.Background())
59+ t.Cleanup(cancel)
60+ return b, ws, ws.Events(ctx)
61+}
62+
63+func TestSetConfigField_PublishesConfigChanged(t *testing.T) {
64+ b, ws, evc := newPublishingWorkspace(t)
65+
66+ require.NoError(t, b.SetConfigField(ws.ID, config.ScopeGlobal, "options.debug", true))
67+ awaitConfigChanged(t, evc, ws.ID)
68+}
69+
70+func TestRemoveConfigField_PublishesConfigChanged(t *testing.T) {
71+ b, ws, evc := newPublishingWorkspace(t)
72+
73+ // Seed a field we can then remove. Setting also publishes, so
74+ // drain the resulting event before testing remove.
75+ require.NoError(t, b.SetConfigField(ws.ID, config.ScopeGlobal, "options.debug", true))
76+ awaitConfigChanged(t, evc, ws.ID)
77+
78+ require.NoError(t, b.RemoveConfigField(ws.ID, config.ScopeGlobal, "options.debug"))
79+ awaitConfigChanged(t, evc, ws.ID)
80+}
81+
82+func TestUpdatePreferredModel_PublishesConfigChanged(t *testing.T) {
83+ if raceEnabled {
84+ // UpdatePreferredModel writes config.Models concurrently
85+ // with the agent coordinator's async sub-agent builder
86+ // that reads it via buildAgentModels. That race is
87+ // pre-existing in the codebase and unrelated to this
88+ // item; ConfigStore mutations are not currently
89+ // synchronized against background readers in [app.App].
90+ // The mutator → publish wiring is unit-tested via
91+ // publishConfigChanged regardless.
92+ t.Skip("skipped under -race: pre-existing race between ConfigStore writes and agent coordinator startup")
93+ }
94+ b, ws, evc := newPublishingWorkspace(t)
95+
96+ model := config.SelectedModel{Provider: "openai", Model: "gpt-4"}
97+ require.NoError(t, b.UpdatePreferredModel(ws.ID, config.ScopeGlobal, config.SelectedModelTypeLarge, model))
98+ awaitConfigChanged(t, evc, ws.ID)
99+}
100+
101+func TestSetCompactMode_PublishesConfigChanged(t *testing.T) {
102+ b, ws, evc := newPublishingWorkspace(t)
103+
104+ require.NoError(t, b.SetCompactMode(ws.ID, config.ScopeGlobal, true))
105+ awaitConfigChanged(t, evc, ws.ID)
106+}
107+
108+func TestSetProviderAPIKey_PublishesConfigChanged(t *testing.T) {
109+ b, ws, evc := newPublishingWorkspace(t)
110+
111+ require.NoError(t, b.SetProviderAPIKey(ws.ID, config.ScopeGlobal, "openai", "test-key"))
112+ awaitConfigChanged(t, evc, ws.ID)
113+}
114+
115+func TestMarkProjectInitialized_PublishesConfigChanged(t *testing.T) {
116+ b, ws, evc := newPublishingWorkspace(t)
117+
118+ require.NoError(t, b.MarkProjectInitialized(ws.ID))
119+ awaitConfigChanged(t, evc, ws.ID)
120+}
121+
122+// TestImportCopilot_PublishesConfigChanged exercises the success path
123+// by seeding a token file in the location ImportCopilot scans, then
124+// asserting the event fires only when ok==true.
125+func TestImportCopilot_PublishesConfigChanged(t *testing.T) {
126+ // ImportCopilot reads from external user-state directories that
127+ // vary by OS. Rather than recreate that setup, drive the
128+ // publishing helper directly and assert ImportCopilot's
129+ // no-event-on-not-found semantics are preserved.
130+ b, ws, evc := newPublishingWorkspace(t)
131+
132+ // Not-found path: no token exists, so no event must fire.
133+ _, ok, err := b.ImportCopilot(ws.ID)
134+ require.NoError(t, err)
135+ require.False(t, ok, "ImportCopilot should return ok=false when no token is present")
136+
137+ select {
138+ case ev := <-evc:
139+ if _, isCC := ev.Payload.(pubsub.Event[proto.ConfigChanged]); isCC {
140+ t.Fatal("ImportCopilot must not publish ConfigChanged when nothing was imported")
141+ }
142+ case <-time.After(100 * time.Millisecond):
143+ // Expected: no ConfigChanged.
144+ }
145+
146+ // Helper sanity: publishing manually does fire the event.
147+ publishConfigChanged(ws)
148+ awaitConfigChanged(t, evc, ws.ID)
149+}
150+
151+// TestRefreshOAuthToken_PublishesConfigChangedOnError verifies that
152+// the unhappy path does not publish (mutator returned an error). The
153+// happy path requires a real OAuth-capable provider configured with a
154+// refreshable token, which is beyond an isolated unit test's scope.
155+func TestRefreshOAuthToken_NoEventOnError(t *testing.T) {
156+ b, ws, evc := newPublishingWorkspace(t)
157+
158+ // Provider does not exist → store returns an error → no event.
159+ err := b.RefreshOAuthToken(context.Background(), ws.ID, config.ScopeGlobal, "no-such-provider")
160+ require.Error(t, err)
161+
162+ select {
163+ case ev := <-evc:
164+ if _, isCC := ev.Payload.(pubsub.Event[proto.ConfigChanged]); isCC {
165+ t.Fatal("RefreshOAuthToken must not publish ConfigChanged when it errors")
166+ }
167+ case <-time.After(100 * time.Millisecond):
168+ }
169+}
170+
171+// TestDisableDockerMCP_PublishesConfigChanged seeds a Docker MCP entry
172+// directly so DisableDockerMCP has something to remove without needing
173+// a running Docker daemon for PrepareDockerMCPConfig's availability
174+// probe.
175+func TestDisableDockerMCP_PublishesConfigChanged(t *testing.T) {
176+ b, ws, evc := newPublishingWorkspace(t)
177+
178+ // Persist a Docker MCP entry directly via the store so the
179+ // downstream DisableDockerMCP path has something to remove.
180+ require.NoError(t, ws.Cfg.PersistDockerMCPConfig(config.DockerMCPConfig()))
181+ drainEvents(evc, 100*time.Millisecond)
182+
183+ require.NoError(t, b.DisableDockerMCP(ws.ID))
184+ awaitConfigChanged(t, evc, ws.ID)
185+}
186+
187+// drainEvents reads from evc until quiet for the given window. Used
188+// to flush events emitted by setup steps so the assertion can target
189+// the event from the action under test.
190+func drainEvents(evc <-chan pubsub.Event[tea.Msg], quiet time.Duration) {
191+ for {
192+ select {
193+ case <-evc:
194+ case <-time.After(quiet):
195+ return
196+ }
197+ }
198+}
199+
200+// TestPublishConfigChanged_NilWorkspaceSafe documents that the helper
201+// is safe to call on workspaces without an *app.App (e.g. synthetic
202+// test workspaces).
203+func TestPublishConfigChanged_NilWorkspaceSafe(t *testing.T) {
204+ t.Parallel()
205+ require.NotPanics(t, func() { publishConfigChanged(nil) })
206+ require.NotPanics(t, func() { publishConfigChanged(&Workspace{}) })
207+}
internal/backend/permission.go+9−8
@@ -6,11 +6,13 @@ import (
6)
6)
7
7
8// GrantPermission grants, denies, or persistently grants a permission
8// GrantPermission grants, denies, or persistently grants a permission
9−// request.
9+// request. The returned bool reports whether this call resolved the
10+// pending request (true) or found it already resolved by a previous
11+// caller (false). A false return is not an error.
10−func (b *Backend) GrantPermission(workspaceID string, req proto.PermissionGrant) error {
12+func (b *Backend) GrantPermission(workspaceID string, req proto.PermissionGrant) (bool, error) {
11 ws, err := b.GetWorkspace(workspaceID)
13 ws, err := b.GetWorkspace(workspaceID)
12 if err != nil {
14 if err != nil {
13− return err
15+ return false, err
14 }
16 }
15
17
16 perm := permission.PermissionRequest{
18 perm := permission.PermissionRequest{
@@ -26,15 +28,14 @@ func (b *Backend) GrantPermission(workspaceID string, req proto.PermissionGrant)
26
28
27 switch req.Action {
29 switch req.Action {
28 case proto.PermissionAllow:
30 case proto.PermissionAllow:
29− ws.Permissions.Grant(perm)
31+ return ws.Permissions.Grant(perm), nil
30 case proto.PermissionAllowForSession:
32 case proto.PermissionAllowForSession:
31− ws.Permissions.GrantPersistent(perm)
33+ return ws.Permissions.GrantPersistent(perm), nil
32 case proto.PermissionDeny:
34 case proto.PermissionDeny:
33− ws.Permissions.Deny(perm)
35+ return ws.Permissions.Deny(perm), nil
34 default:
36 default:
35− return ErrInvalidPermissionAction
37+ return false, ErrInvalidPermissionAction
36 }
38 }
37− return nil
38}
39}
39
40
40// SetPermissionsSkip sets whether permission prompts are skipped.
41// SetPermissionsSkip sets whether permission prompts are skipped.
internal/backend/race_off_test.gonew file+5
@@ -0,0 +1,5 @@
1+//go:build !race
2+
3+package backend
4+
5+const raceEnabled = false
internal/backend/race_on_test.gonew file+5
@@ -0,0 +1,5 @@
1+//go:build race
2+
3+package backend
4+
5+const raceEnabled = true
internal/backend/testing.gonew file+55
@@ -0,0 +1,55 @@
1+package backend
2+
3+// InsertWorkspaceForTest registers ws with b under its current ID and
4+// path. It is intended for tests in other packages that need to drive
5+// HTTP handlers against a synthetic workspace without booting a real
6+// app.App. Production code should go through CreateWorkspace.
7+func InsertWorkspaceForTest(b *Backend, ws *Workspace) {
8+ if ws.resolvedPath == "" {
9+ ws.resolvedPath = ws.Path
10+ }
11+ if ws.clients == nil {
12+ ws.clients = make(map[string]*clientState)
13+ }
14+ b.mu.Lock()
15+ defer b.mu.Unlock()
16+ b.workspaces.Set(ws.ID, ws)
17+ if ws.resolvedPath != "" {
18+ b.pathIndex[ws.resolvedPath] = ws.ID
19+ }
20+}
21+
22+// RegisterClientForTesting installs a creation hold for clientID on
23+// ws using the backend's normal registerClient path. Intended for
24+// tests in other packages that need to drive a hold-only client
25+// (streams == 0) without booting a real CreateWorkspace flow.
26+func RegisterClientForTesting(b *Backend, ws *Workspace, clientID string) error {
27+ if _, err := validateClientID(clientID); err != nil {
28+ return err
29+ }
30+ b.registerClient(ws, clientID)
31+ return nil
32+}
33+
34+// SetWorkspaceShutdownFnForTest overrides the workspace teardown
35+// callback. Useful for tests in other packages that drive synthetic
36+// workspaces (where the embedded [app.App] is incomplete) through
37+// detach paths that would otherwise crash inside App.Shutdown.
38+func SetWorkspaceShutdownFnForTest(ws *Workspace, fn func()) {
39+ ws.shutdownFn = fn
40+}
41+
42+// WorkspaceLiveStreamCountForTest returns the number of clients on ws
43+// that have at least one live SSE stream. Used by integration tests
44+// in other packages to wait for SSE attaches before publishing events.
45+func WorkspaceLiveStreamCountForTest(ws *Workspace) int {
46+ ws.clientsMu.Lock()
47+ defer ws.clientsMu.Unlock()
48+ n := 0
49+ for _, cs := range ws.clients {
50+ if cs.streams > 0 {
51+ n++
52+ }
53+ }
54+ return n
55+}
internal/client/client.go+13−4
@@ -15,6 +15,7 @@ import (
15 "github.com/charmbracelet/crush/internal/config"
15 "github.com/charmbracelet/crush/internal/config"
16 "github.com/charmbracelet/crush/internal/proto"
16 "github.com/charmbracelet/crush/internal/proto"
17 "github.com/charmbracelet/crush/internal/server"
17 "github.com/charmbracelet/crush/internal/server"
18+ "github.com/google/uuid"
18)
19)
19
20
20// DummyHost is used to satisfy the http.Client's requirement for a URL.
21// DummyHost is used to satisfy the http.Client's requirement for a URL.
@@ -22,10 +23,11 @@ const DummyHost = "api.crush.localhost"
22
23
23// Client represents an RPC client connected to a Crush server.
24// Client represents an RPC client connected to a Crush server.
24type Client struct {
25type Client struct {
25− h *http.Client
26+ h *http.Client
26− path string
27+ path string
27− network string
28+ network string
29+ addr string
28− addr string
30+ clientID string
29}
31}
30
32
31// DefaultClient creates a new [Client] connected to the default server address.
33// DefaultClient creates a new [Client] connected to the default server address.
@@ -44,6 +46,7 @@ func NewClient(path, network, address string) (*Client, error) {
44 c.path = filepath.Clean(path)
46 c.path = filepath.Clean(path)
45 c.network = network
47 c.network = network
46 c.addr = address
48 c.addr = address
49+ c.clientID = uuid.New().String()
47 p := &http.Protocols{}
50 p := &http.Protocols{}
48 p.SetHTTP1(true)
51 p.SetHTTP1(true)
49 p.SetUnencryptedHTTP2(true)
52 p.SetUnencryptedHTTP2(true)
@@ -65,6 +68,12 @@ func (c *Client) Path() string {
65 return c.path
68 return c.path
66}
69}
67
70
71+// ClientID returns the per-process client ID minted in [NewClient].
72+// The server uses it as a presence/coordination handle.
73+func (c *Client) ClientID() string {
74+ return c.clientID
75+}
76+
68// GetGlobalConfig retrieves the server's configuration.
77// GetGlobalConfig retrieves the server's configuration.
69func (c *Client) GetGlobalConfig(ctx context.Context) (*config.Config, error) {
78func (c *Client) GetGlobalConfig(ctx context.Context) (*config.Config, error) {
70 var cfg config.Config
79 var cfg config.Config
internal/client/proto.go+46−7
@@ -39,6 +39,7 @@ func (c *Client) ListWorkspaces(ctx context.Context) ([]proto.Workspace, error)
39
39
40// CreateWorkspace creates a new workspace on the server.
40// CreateWorkspace creates a new workspace on the server.
41func (c *Client) CreateWorkspace(ctx context.Context, ws proto.Workspace) (*proto.Workspace, error) {
41func (c *Client) CreateWorkspace(ctx context.Context, ws proto.Workspace) (*proto.Workspace, error) {
42+ ws.ClientID = c.clientID
42 rsp, err := c.post(ctx, "/workspaces", nil, jsonBody(ws), http.Header{"Content-Type": []string{"application/json"}})
43 rsp, err := c.post(ctx, "/workspaces", nil, jsonBody(ws), http.Header{"Content-Type": []string{"application/json"}})
43 if err != nil {
44 if err != nil {
44 return nil, fmt.Errorf("failed to create workspace: %w", err)
45 return nil, fmt.Errorf("failed to create workspace: %w", err)
@@ -73,7 +74,8 @@ func (c *Client) GetWorkspace(ctx context.Context, id string) (*proto.Workspace,
73
74
74// DeleteWorkspace deletes a workspace on the server.
75// DeleteWorkspace deletes a workspace on the server.
75func (c *Client) DeleteWorkspace(ctx context.Context, id string) error {
76func (c *Client) DeleteWorkspace(ctx context.Context, id string) error {
77+ q := url.Values{"client_id": []string{c.clientID}}
76− rsp, err := c.delete(ctx, fmt.Sprintf("/workspaces/%s", id), nil, nil)
78+ rsp, err := c.delete(ctx, fmt.Sprintf("/workspaces/%s", id), q, nil)
77 if err != nil {
79 if err != nil {
78 return fmt.Errorf("failed to delete workspace: %w", err)
80 return fmt.Errorf("failed to delete workspace: %w", err)
79 }
81 }
@@ -84,11 +86,36 @@ func (c *Client) DeleteWorkspace(ctx context.Context, id string) error {
84 return nil
86 return nil
85}
87}
86
88
89+// SetCurrentSession reports the client's current-session selection
90+// for the named workspace. An empty sessionID clears the entry. The
91+// request carries the process-scoped client ID minted in [NewClient]
92+// as a query parameter so the server can route the update to the
93+// correct [clientState] entry.
94+func (c *Client) SetCurrentSession(ctx context.Context, workspaceID, sessionID string) error {
95+ q := url.Values{"client_id": []string{c.clientID}}
96+ rsp, err := c.post(
97+ ctx,
98+ fmt.Sprintf("/workspaces/%s/current-session", workspaceID),
99+ q,
100+ jsonBody(proto.CurrentSession{SessionID: sessionID}),
101+ http.Header{"Content-Type": []string{"application/json"}},
102+ )
103+ if err != nil {
104+ return fmt.Errorf("failed to set current session: %w", err)
105+ }
106+ defer rsp.Body.Close()
107+ if rsp.StatusCode != http.StatusOK {
108+ return fmt.Errorf("failed to set current session: status code %d", rsp.StatusCode)
109+ }
110+ return nil
111+}
112+
87// SubscribeEvents subscribes to server-sent events for a workspace.
113// SubscribeEvents subscribes to server-sent events for a workspace.
88func (c *Client) SubscribeEvents(ctx context.Context, id string) (<-chan any, error) {
114func (c *Client) SubscribeEvents(ctx context.Context, id string) (<-chan any, error) {
89 events := make(chan any, 100)
115 events := make(chan any, 100)
116+ q := url.Values{"client_id": []string{c.clientID}}
90 //nolint:bodyclose
117 //nolint:bodyclose
91− rsp, err := c.get(ctx, fmt.Sprintf("/workspaces/%s/events", id), nil, http.Header{
118+ rsp, err := c.get(ctx, fmt.Sprintf("/workspaces/%s/events", id), q, http.Header{
92 "Accept": []string{"text/event-stream"},
119 "Accept": []string{"text/event-stream"},
93 "Cache-Control": []string{"no-cache"},
120 "Cache-Control": []string{"no-cache"},
94 "Connection": []string{"keep-alive"},
121 "Connection": []string{"keep-alive"},
@@ -168,6 +195,10 @@ func (c *Client) SubscribeEvents(ctx context.Context, id string) (<-chan any, er
168 var e pubsub.Event[proto.AgentEvent]
195 var e pubsub.Event[proto.AgentEvent]
169 _ = json.Unmarshal(p.Payload, &e)
196 _ = json.Unmarshal(p.Payload, &e)
170 sendEvent(ctx, events, e)
197 sendEvent(ctx, events, e)
198+ case pubsub.PayloadTypeConfigChanged:
199+ var e pubsub.Event[proto.ConfigChanged]
200+ _ = json.Unmarshal(p.Payload, &e)
201+ sendEvent(ctx, events, e)
171 case pubsub.PayloadTypeSkillsEvent:
202 case pubsub.PayloadTypeSkillsEvent:
172 var e pubsub.Event[proto.SkillsEvent]
203 var e pubsub.Event[proto.SkillsEvent]
173 _ = json.Unmarshal(p.Payload, &e)
204 _ = json.Unmarshal(p.Payload, &e)
@@ -482,17 +513,25 @@ func (c *Client) ListSessions(ctx context.Context, id string) ([]proto.Session,
482 return sessions, nil
513 return sessions, nil
483}
514}
484
515
485−// GrantPermission grants a permission on a workspace.
516+// GrantPermission grants a permission on a workspace. The returned
517+// bool reports whether this call resolved the pending request (true)
518+// or found it already resolved by a previous caller (false). A false
519+// value is not an error — it just means another subscriber resolved
520+// the same request first.
486−func (c *Client) GrantPermission(ctx context.Context, id string, req proto.PermissionGrant) error {
521+func (c *Client) GrantPermission(ctx context.Context, id string, req proto.PermissionGrant) (bool, error) {
487 rsp, err := c.post(ctx, fmt.Sprintf("/workspaces/%s/permissions/grant", id), nil, jsonBody(req), http.Header{"Content-Type": []string{"application/json"}})
522 rsp, err := c.post(ctx, fmt.Sprintf("/workspaces/%s/permissions/grant", id), nil, jsonBody(req), http.Header{"Content-Type": []string{"application/json"}})
488 if err != nil {
523 if err != nil {
489− return fmt.Errorf("failed to grant permission: %w", err)
524+ return false, fmt.Errorf("failed to grant permission: %w", err)
490 }
525 }
491 defer rsp.Body.Close()
526 defer rsp.Body.Close()
492 if rsp.StatusCode != http.StatusOK {
527 if rsp.StatusCode != http.StatusOK {
493− return fmt.Errorf("failed to grant permission: status code %d", rsp.StatusCode)
528+ return false, fmt.Errorf("failed to grant permission: status code %d", rsp.StatusCode)
494 }
529 }
495− return nil
530+ var resp proto.PermissionGrantResponse
531+ if err := json.NewDecoder(rsp.Body).Decode(&resp); err != nil {
532+ return false, fmt.Errorf("failed to decode grant permission response: %w", err)
533+ }
534+ return resp.Resolved, nil
496}
535}
497
536
498// SetPermissionsSkipRequests sets the skip-requests flag for a workspace.
537// SetPermissionsSkipRequests sets the skip-requests flag for a workspace.
internal/db/connect.go+72−4
@@ -6,6 +6,7 @@ import (
6 "embed"
6 "embed"
7 "fmt"
7 "fmt"
8 "log/slog"
8 "log/slog"
9+ "os"
9 "path/filepath"
10 "path/filepath"
10 "sync"
11 "sync"
11 "testing"
12 "testing"
@@ -39,10 +40,16 @@ func init() {
39 }
40 }
40}
41}
41
42
42−// connEntry holds a shared database connection and its reference count.
43+// connEntry holds a shared database connection, its reference count,
44+// and the data-directory lock that gates access to this entry. The
45+// lock is acquired exactly once when the entry is created and released
46+// when the last reference is dropped, which lets the same process open
47+// the same data directory concurrently while still blocking a second
48+// crush process from racing the storage.
43type connEntry struct {
49type connEntry struct {
44 db *sql.DB
50 db *sql.DB
45 refCount int
51 refCount int
52+ lock *dataDirLock
46}
53}
47
54
48var (
55var (
@@ -50,16 +57,39 @@ var (
50 poolMu sync.Mutex
57 poolMu sync.Mutex
51)
58)
52
59
60+// ConnectOption configures a Connect call. Options are applied in
61+// order; later options override earlier ones for the same field.
62+type ConnectOption func(*connectOptions)
63+
64+// connectOptions holds the resolved configuration for a Connect call.
65+type connectOptions struct {
66+ lockDataDir bool
67+}
68+
69+// WithDataDirLock toggles acquisition of the per-data-directory lock
70+// for this Connect call. The lock is off by default so local-mode
71+// invocations do not regress today's behavior; the server's
72+// workspace-bootstrap path opts in. CRUSH_SKIP_DATADIR_LOCK still
73+// bypasses acquisition even when this option is set.
74+func WithDataDirLock(enable bool) ConnectOption {
75+ return func(o *connectOptions) { o.lockDataDir = enable }
76+}
77+
53// Connect opens a SQLite database connection for the given data
78// Connect opens a SQLite database connection for the given data
54// directory and runs migrations. If a connection to the same database
79// directory and runs migrations. If a connection to the same database
55// file already exists, the existing connection is returned with its
80// file already exists, the existing connection is returned with its
56// reference count incremented. Callers must pair each Connect with a
81// reference count incremented. Callers must pair each Connect with a
57// [Release] when they no longer need the connection.
82// [Release] when they no longer need the connection.
58−func Connect(ctx context.Context, dataDir string) (*sql.DB, error) {
83+func Connect(ctx context.Context, dataDir string, opts ...ConnectOption) (*sql.DB, error) {
59 if dataDir == "" {
84 if dataDir == "" {
60 return nil, fmt.Errorf("data.dir is not set")
85 return nil, fmt.Errorf("data.dir is not set")
61 }
86 }
62
87
88+ var cfg connectOptions
89+ for _, opt := range opts {
90+ opt(&cfg)
91+ }
92+
63 dbPath := filepath.Join(dataDir, "crush.db")
93 dbPath := filepath.Join(dataDir, "crush.db")
64
94
65 // Resolve to an absolute path so that different relative paths to
95 // Resolve to an absolute path so that different relative paths to
@@ -77,8 +107,30 @@ func Connect(ctx context.Context, dataDir string) (*sql.DB, error) {
77 return entry.db, nil
107 return entry.db, nil
78 }
108 }
79
109
110+ // Take the per-data-directory lock before opening the database so
111+ // we fail fast and with a clear error rather than racing another
112+ // crush process on the same SQLite file. The lock is released when
113+ // the matching Release call drops the refcount to zero. Ensuring
114+ // the data directory exists is required because the lock file
115+ // lives inside it. Locking is opt-in via WithDataDirLock so that
116+ // local-mode invocations do not refuse a second crush against the
117+ // same data dir until client/server becomes the default.
118+ if err := os.MkdirAll(dataDir, 0o700); err != nil {
119+ return nil, fmt.Errorf("failed to create data directory %q: %w", dataDir, err)
120+ }
121+ var lock *dataDirLock
122+ if cfg.lockDataDir && !skipDataDirLock() {
123+ lock, err = acquireDataDirLock(dataDir)
124+ if err != nil {
125+ return nil, err
126+ }
127+ }
128+
80 conn, err := openDB(dbPath)
129 conn, err := openDB(dbPath)
81 if err != nil {
130 if err != nil {
131+ if lock != nil {
132+ lock.release()
133+ }
82 return nil, err
134 return nil, err
83 }
135 }
84
136
@@ -89,24 +141,33 @@ func Connect(ctx context.Context, dataDir string) (*sql.DB, error) {
89 // resulting in SQLITE_NOTADB (26) on the next open.
141 // resulting in SQLITE_NOTADB (26) on the next open.
90 conn.SetMaxOpenConns(1)
142 conn.SetMaxOpenConns(1)
91
143
144+ releaseLock := func() {
145+ if lock != nil {
146+ lock.release()
147+ }
148+ }
149+
92 if err = conn.PingContext(ctx); err != nil {
150 if err = conn.PingContext(ctx); err != nil {
93 conn.Close()
151 conn.Close()
152+ releaseLock()
94 return nil, fmt.Errorf("failed to connect to database: %w", err)
153 return nil, fmt.Errorf("failed to connect to database: %w", err)
95 }
154 }
96
155
97 if err := initGoose(); err != nil {
156 if err := initGoose(); err != nil {
98 conn.Close()
157 conn.Close()
158+ releaseLock()
99 slog.Error("Failed to initialize goose", "error", err)
159 slog.Error("Failed to initialize goose", "error", err)
100 return nil, fmt.Errorf("failed to initialize goose: %w", err)
160 return nil, fmt.Errorf("failed to initialize goose: %w", err)
101 }
161 }
102
162
103 if err := goose.Up(conn, "migrations"); err != nil {
163 if err := goose.Up(conn, "migrations"); err != nil {
104 conn.Close()
164 conn.Close()
165+ releaseLock()
105 slog.Error("Failed to apply migrations", "error", err)
166 slog.Error("Failed to apply migrations", "error", err)
106 return nil, fmt.Errorf("failed to apply migrations: %w", err)
167 return nil, fmt.Errorf("failed to apply migrations: %w", err)
107 }
168 }
108
169
109− pool[absPath] = &connEntry{db: conn, refCount: 1}
170+ pool[absPath] = &connEntry{db: conn, refCount: 1, lock: lock}
110 return conn, nil
171 return conn, nil
111}
172}
112
173
@@ -134,7 +195,11 @@ func Release(dataDir string) error {
134 }
195 }
135
196
136 delete(pool, absPath)
197 delete(pool, absPath)
137− return entry.db.Close()
198+ closeErr := entry.db.Close()
199+ if entry.lock != nil {
200+ entry.lock.release()
201+ }
202+ return closeErr
138}
203}
139
204
140// ResetPool closes all pooled connections and clears the pool. This is
205// ResetPool closes all pooled connections and clears the pool. This is
@@ -144,6 +209,9 @@ func ResetPool() {
144 defer poolMu.Unlock()
209 defer poolMu.Unlock()
145 for path, entry := range pool {
210 for path, entry := range pool {
146 entry.db.Close()
211 entry.db.Close()
212+ if entry.lock != nil {
213+ entry.lock.release()
214+ }
147 delete(pool, path)
215 delete(pool, path)
148 }
216 }
149}
217}
internal/db/connect_test.go+155
@@ -2,6 +2,8 @@ package db
2
2
3import (
3import (
4 "context"
4 "context"
5+ "errors"
6+ "path/filepath"
5 "testing"
7 "testing"
6
8
7 "github.com/stretchr/testify/require"
9 "github.com/stretchr/testify/require"
@@ -52,3 +54,156 @@ func TestRelease_NoopForUnknownDataDir(t *testing.T) {
52
54
53 require.NoError(t, Release("/nonexistent/path"), "releasing unknown data dir should not error")
55 require.NoError(t, Release("/nonexistent/path"), "releasing unknown data dir should not error")
54}
56}
57+
58+// TestConnect_FailsWhenDataDirLocked simulates a second crush process by
59+// taking the data-dir lock directly via the OS primitive on a separate
60+// file descriptor and then asserting that Connect surfaces a clean
61+// ErrDataDirLocked instead of opening the database under contention.
62+func TestConnect_FailsWhenDataDirLocked(t *testing.T) {
63+ t.Cleanup(ResetPool)
64+
65+ dataDir := t.TempDir()
66+ lockPath := filepath.Join(dataDir, dataDirLockFile)
67+
68+ release, err := tryFileLock(lockPath)
69+ require.NoError(t, err, "expected to take the data-dir lock for the first time")
70+ t.Cleanup(release)
71+
72+ _, err = Connect(context.Background(), dataDir, WithDataDirLock(true))
73+ require.Error(t, err, "Connect must refuse to open a locked data dir")
74+ require.ErrorIs(t, err, ErrDataDirLocked)
75+}
76+
77+// TestConnect_SucceedsAfterContenderReleases ensures the lock is purely
78+// advisory and that a clean release lets the next Connect proceed.
79+func TestConnect_SucceedsAfterContenderReleases(t *testing.T) {
80+ t.Cleanup(ResetPool)
81+
82+ dataDir := t.TempDir()
83+ lockPath := filepath.Join(dataDir, dataDirLockFile)
84+
85+ release, err := tryFileLock(lockPath)
86+ require.NoError(t, err)
87+
88+ _, err = Connect(context.Background(), dataDir, WithDataDirLock(true))
89+ require.ErrorIs(t, err, ErrDataDirLocked)
90+
91+ release()
92+
93+ conn, err := Connect(context.Background(), dataDir, WithDataDirLock(true))
94+ require.NoError(t, err, "Connect should succeed once the contender releases the lock")
95+ require.NoError(t, conn.PingContext(context.Background()))
96+ require.NoError(t, Release(dataDir))
97+}
98+
99+// TestConnect_LockReleasedOnFinalRelease confirms that closing the last
100+// reference to a pool entry also drops the OS lock, so subsequent
101+// processes can take the data dir.
102+func TestConnect_LockReleasedOnFinalRelease(t *testing.T) {
103+ t.Cleanup(ResetPool)
104+
105+ dataDir := t.TempDir()
106+ lockPath := filepath.Join(dataDir, dataDirLockFile)
107+
108+ conn, err := Connect(context.Background(), dataDir, WithDataDirLock(true))
109+ require.NoError(t, err)
110+ require.NoError(t, conn.PingContext(context.Background()))
111+
112+ // Holding the in-process entry must keep the OS lock held so a
113+ // "second process" (simulated by a fresh tryFileLock call) is
114+ // rejected.
115+ _, lockErr := tryFileLock(lockPath)
116+ require.Error(t, lockErr)
117+ require.True(t, errors.Is(lockErr, errLockContended), "expected contended lock while pool entry is live")
118+
119+ require.NoError(t, Release(dataDir))
120+
121+ // After the final release the lock is free again.
122+ release, err := tryFileLock(lockPath)
123+ require.NoError(t, err, "expected lock to be released after final Release")
124+ release()
125+}
126+
127+// TestConnect_SharedPoolDoesNotReacquireLock makes sure that subsequent
128+// in-process Connect calls reuse the existing OS lock through refcount,
129+// not by re-acquiring it. The simplest observable signal of correctness
130+// is that the second Connect does not error and the lock is still held
131+// after a single Release.
132+func TestConnect_SharedPoolDoesNotReacquireLock(t *testing.T) {
133+ t.Cleanup(ResetPool)
134+
135+ dataDir := t.TempDir()
136+ lockPath := filepath.Join(dataDir, dataDirLockFile)
137+
138+ _, err := Connect(context.Background(), dataDir, WithDataDirLock(true))
139+ require.NoError(t, err)
140+
141+ _, err = Connect(context.Background(), dataDir, WithDataDirLock(true))
142+ require.NoError(t, err)
143+
144+ // Drop one reference; lock must still be held.
145+ require.NoError(t, Release(dataDir))
146+ _, lockErr := tryFileLock(lockPath)
147+ require.ErrorIs(t, lockErr, errLockContended)
148+
149+ require.NoError(t, Release(dataDir))
150+}
151+
152+// TestConnect_SkipLockEnvBypassesAcquisition exercises the escape
153+// hatch used by users on filesystems where flock is unreliable.
154+func TestConnect_SkipLockEnvBypassesAcquisition(t *testing.T) {
155+ t.Cleanup(ResetPool)
156+
157+ dataDir := t.TempDir()
158+ lockPath := filepath.Join(dataDir, dataDirLockFile)
159+
160+ release, err := tryFileLock(lockPath)
161+ require.NoError(t, err)
162+ t.Cleanup(release)
163+
164+ t.Setenv("CRUSH_SKIP_DATADIR_LOCK", "1")
165+
166+ conn, err := Connect(context.Background(), dataDir, WithDataDirLock(true))
167+ require.NoError(t, err, "skip-lock env should bypass contention")
168+ require.NoError(t, conn.PingContext(context.Background()))
169+ require.NoError(t, Release(dataDir))
170+}
171+
172+// TestConnect_DefaultIgnoresContendedLock confirms that without
173+// WithDataDirLock(true) the lock file is irrelevant: a contender can
174+// hold tryFileLock and Connect still succeeds. This pins the
175+// local-mode default to its pre-lock behavior.
176+func TestConnect_DefaultIgnoresContendedLock(t *testing.T) {
177+ t.Cleanup(ResetPool)
178+
179+ dataDir := t.TempDir()
180+ lockPath := filepath.Join(dataDir, dataDirLockFile)
181+
182+ release, err := tryFileLock(lockPath)
183+ require.NoError(t, err, "expected to take the data-dir lock for the first time")
184+ t.Cleanup(release)
185+
186+ conn, err := Connect(context.Background(), dataDir)
187+ require.NoError(t, err, "default Connect must not take the lock and must succeed under contention")
188+ require.NoError(t, conn.PingContext(context.Background()))
189+ require.NoError(t, Release(dataDir))
190+}
191+
192+// TestConnect_ServerPathFailsWhenDataDirLocked is the server's
193+// workspace-bootstrap analogue of TestConnect_FailsWhenDataDirLocked:
194+// passing WithDataDirLock(true) must surface ErrDataDirLocked when a
195+// contender already holds the lock.
196+func TestConnect_ServerPathFailsWhenDataDirLocked(t *testing.T) {
197+ t.Cleanup(ResetPool)
198+
199+ dataDir := t.TempDir()
200+ lockPath := filepath.Join(dataDir, dataDirLockFile)
201+
202+ release, err := tryFileLock(lockPath)
203+ require.NoError(t, err, "expected to take the data-dir lock for the first time")
204+ t.Cleanup(release)
205+
206+ _, err = Connect(context.Background(), dataDir, WithDataDirLock(true))
207+ require.Error(t, err, "server-path Connect must refuse to open a locked data dir")
208+ require.ErrorIs(t, err, ErrDataDirLocked)
209+}
internal/db/datadirlock.gonew file+130
@@ -0,0 +1,130 @@
1+package db
2+
3+import (
4+ "encoding/json"
5+ "errors"
6+ "fmt"
7+ "log/slog"
8+ "os"
9+ "path/filepath"
10+ "strconv"
11+ "time"
12+
13+ "github.com/charmbracelet/crush/internal/version"
14+)
15+
16+// ErrDataDirLocked is returned by Connect when the data directory is
17+// already in use by another crush process.
18+var ErrDataDirLocked = errors.New("data directory already in use by another crush process")
19+
20+// dataDirLockFile is the name of the lock file inside the data
21+// directory. It lives next to crush.db so users can `ls` and find it.
22+const dataDirLockFile = "crush.lock"
23+
24+// dataDirOwnerInfo is the JSON payload written into the lock file by
25+// the process that currently owns it. It is purely informational; the
26+// authoritative state of ownership is the operating system flock on
27+// the file descriptor.
28+type dataDirOwnerInfo struct {
29+ PID int `json:"pid"`
30+ Version string `json:"version,omitempty"`
31+ StartedAt string `json:"started_at,omitempty"`
32+}
33+
34+// dataDirLock represents an acquired exclusive lock on a data
35+// directory. release closes the underlying file descriptor which the
36+// kernel uses to drop the OS-level lock.
37+type dataDirLock struct {
38+ release func()
39+}
40+
41+// acquireDataDirLock takes an exclusive non-blocking lock on
42+// {dataDir}/crush.lock. If the lock is already held by another
43+// process, it returns ErrDataDirLocked wrapped with a diagnostic that
44+// includes whatever owner info that process wrote.
45+//
46+// Acquisition is skipped (returning a no-op lock) when
47+// CRUSH_SKIP_DATADIR_LOCK is set to a truthy value. This is intended
48+// as an escape hatch for hostile filesystems that do not implement
49+// advisory locking; it should not be used in normal operation.
50+func acquireDataDirLock(dataDir string) (*dataDirLock, error) {
51+ if skipDataDirLock() {
52+ return &dataDirLock{release: func() {}}, nil
53+ }
54+
55+ path := filepath.Join(dataDir, dataDirLockFile)
56+ release, err := tryFileLock(path)
57+ if err != nil {
58+ if errors.Is(err, errLockContended) {
59+ return nil, contendedLockError(dataDir, path)
60+ }
61+ return nil, fmt.Errorf("failed to lock data directory %q: %w", dataDir, err)
62+ }
63+
64+ // Record ownership metadata so a contending process can identify
65+ // us. Failures here are non-fatal: the OS-level lock is what
66+ // actually guarantees mutual exclusion, and a missing/partial JSON
67+ // payload only degrades the diagnostic a contender prints.
68+ if err := writeOwnerInfo(path); err != nil {
69+ slog.Debug("Failed to write data-dir owner info", "path", path, "error", err)
70+ }
71+
72+ // The lock file itself is intentionally never unlinked. flock is
73+ // keyed by inode, not by path, and any close-then-unlink (or
74+ // unlink-then-close) ordering opens a window where two processes
75+ // can each hold a flock on a different inode that lives at the
76+ // same path. Leaving the file in place lets every acquirer see
77+ // the same inode and lets the kernel arbitrate correctly.
78+ return &dataDirLock{release: release}, nil
79+}
80+
81+// skipDataDirLock reports whether the data-dir lock should be bypassed.
82+func skipDataDirLock() bool {
83+ v, _ := strconv.ParseBool(os.Getenv("CRUSH_SKIP_DATADIR_LOCK"))
84+ return v
85+}
86+
87+// writeOwnerInfo truncates and rewrites the lock file with the current
88+// process's identifying information. It is called only after the lock
89+// is held.
90+func writeOwnerInfo(path string) error {
91+ info := dataDirOwnerInfo{
92+ PID: os.Getpid(),
93+ Version: version.Version,
94+ StartedAt: time.Now().UTC().Format(time.RFC3339),
95+ }
96+ payload, err := json.MarshalIndent(info, "", " ")
97+ if err != nil {
98+ return err
99+ }
100+ payload = append(payload, '\n')
101+ return os.WriteFile(path, payload, 0o600)
102+}
103+
104+// readOwnerInfo returns the lock file's recorded owner, if it parses.
105+// A missing or malformed file yields an empty struct and no error;
106+// the caller decides what to surface to the user.
107+func readOwnerInfo(path string) dataDirOwnerInfo {
108+ raw, err := os.ReadFile(path)
109+ if err != nil || len(raw) == 0 {
110+ return dataDirOwnerInfo{}
111+ }
112+ var info dataDirOwnerInfo
113+ _ = json.Unmarshal(raw, &info)
114+ return info
115+}
116+
117+// contendedLockError builds a wrapped ErrDataDirLocked annotated with
118+// whatever owner metadata is currently in the lock file.
119+func contendedLockError(dataDir, lockPath string) error {
120+ info := readOwnerInfo(lockPath)
121+ details := ""
122+ switch {
123+ case info.PID != 0 && info.StartedAt != "":
124+ details = fmt.Sprintf(" (owner pid=%d version=%s started_at=%s)",
125+ info.PID, info.Version, info.StartedAt)
126+ case info.PID != 0:
127+ details = fmt.Sprintf(" (owner pid=%d)", info.PID)
128+ }
129+ return fmt.Errorf("%w: %s%s", ErrDataDirLocked, dataDir, details)
130+}
internal/db/datadirlock_unix.gonew file+45
@@ -0,0 +1,45 @@
1+//go:build !windows
2+
3+package db
4+
5+import (
6+ "errors"
7+ "fmt"
8+ "os"
9+
10+ "golang.org/x/sys/unix"
11+)
12+
13+// errLockContended is returned by tryFileLock when the lock is already
14+// held by another open file description (typically another process).
15+var errLockContended = errors.New("file lock is held by another process")
16+
17+// tryFileLock takes an exclusive non-blocking BSD flock on path,
18+// creating the file if necessary. On success it returns a release
19+// function that drops the lock and closes the descriptor. When the
20+// lock is contended it returns errLockContended.
21+//
22+// BSD flock is advisory and per-open-file-description, so it does not
23+// interfere with the byte-range locks SQLite itself uses on the same
24+// file's siblings (crush.db, crush.db-wal, crush.db-shm). The lock is
25+// also released automatically by the kernel when the file descriptor
26+// is closed, including on process crash, so we do not need any
27+// explicit stale-lock recovery.
28+func tryFileLock(path string) (func(), error) {
29+ f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0o600)
30+ if err != nil {
31+ return nil, fmt.Errorf("open lock file: %w", err)
32+ }
33+ if err := unix.Flock(int(f.Fd()), unix.LOCK_EX|unix.LOCK_NB); err != nil {
34+ _ = f.Close()
35+ if errors.Is(err, unix.EWOULDBLOCK) {
36+ return nil, errLockContended
37+ }
38+ return nil, fmt.Errorf("flock: %w", err)
39+ }
40+ return func() {
41+ // Closing the descriptor releases the flock atomically.
42+ _ = unix.Flock(int(f.Fd()), unix.LOCK_UN)
43+ _ = f.Close()
44+ }, nil
45+}
internal/db/datadirlock_windows.gonew file+46
@@ -0,0 +1,46 @@
1+//go:build windows
2+
3+package db
4+
5+import (
6+ "errors"
7+ "fmt"
8+ "math"
9+ "os"
10+
11+ "golang.org/x/sys/windows"
12+)
13+
14+// errLockContended is returned by tryFileLock when the lock is held
15+// by another process.
16+var errLockContended = errors.New("file lock is held by another process")
17+
18+// tryFileLock takes an exclusive non-blocking lock on path via
19+// LockFileEx. On success it returns a release function that unlocks
20+// and closes the descriptor.
21+//
22+// The flags combine LOCKFILE_EXCLUSIVE_LOCK with LOCKFILE_FAIL_IMMEDIATELY
23+// to mirror the BSD LOCK_EX|LOCK_NB semantics used on POSIX. The lock
24+// is released when the file handle closes, including on process exit,
25+// which gives us automatic stale-lock recovery without any bookkeeping.
26+func tryFileLock(path string) (func(), error) {
27+ f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0o600)
28+ if err != nil {
29+ return nil, fmt.Errorf("open lock file: %w", err)
30+ }
31+ h := windows.Handle(f.Fd())
32+ ol := new(windows.Overlapped)
33+ flags := uint32(windows.LOCKFILE_EXCLUSIVE_LOCK | windows.LOCKFILE_FAIL_IMMEDIATELY)
34+ if err := windows.LockFileEx(h, flags, 0, math.MaxUint32, math.MaxUint32, ol); err != nil {
35+ _ = f.Close()
36+ if errors.Is(err, windows.ERROR_LOCK_VIOLATION) || errors.Is(err, windows.ERROR_IO_PENDING) {
37+ return nil, errLockContended
38+ }
39+ return nil, fmt.Errorf("LockFileEx: %w", err)
40+ }
41+ return func() {
42+ ol := new(windows.Overlapped)
43+ _ = windows.UnlockFileEx(windows.Handle(f.Fd()), 0, math.MaxUint32, math.MaxUint32, ol)
44+ _ = f.Close()
45+ }, nil
46+}
internal/permission/permission.go+62−43
@@ -64,9 +64,19 @@ type PermissionRequest struct {
64
64
65type Service interface {
65type Service interface {
66 pubsub.Subscriber[PermissionRequest]
66 pubsub.Subscriber[PermissionRequest]
67+ // GrantPersistent grants a permission request and remembers the grant
68+ // for the session. It returns true if this call actually resolved the
69+ // pending request; false if the request had already been resolved
70+ // (e.g., by another concurrent caller) or is unknown.
67− GrantPersistent(permission PermissionRequest)
71+ GrantPersistent(permission PermissionRequest) bool
72+ // Grant grants a permission request. It returns true if this call
73+ // actually resolved the pending request; false if the request had
74+ // already been resolved or is unknown.
68− Grant(permission PermissionRequest)
75+ Grant(permission PermissionRequest) bool
76+ // Deny denies a permission request. It returns true if this call
77+ // actually resolved the pending request; false if the request had
78+ // already been resolved or is unknown.
69− Deny(permission PermissionRequest)
79+ Deny(permission PermissionRequest) bool
70 Request(ctx context.Context, opts CreatePermissionRequest) (bool, error)
80 Request(ctx context.Context, opts CreatePermissionRequest) (bool, error)
71 AutoApproveSession(sessionID string)
81 AutoApproveSession(sessionID string)
72 SetSkipRequests(skip bool)
82 SetSkipRequests(skip bool)
@@ -100,63 +110,72 @@ type permissionService struct {
100 activeRequestMu sync.Mutex
110 activeRequestMu sync.Mutex
101}
111}
102
112
113+// resolve atomically removes the pending request entry for the given
114+// permission and, if it was still pending, publishes exactly one
115+// PermissionNotification and forwards the outcome to the waiter on
116+// respCh. It returns true if this call resolved the request, false if
117+// it had already been resolved (e.g., by another concurrent caller) or
118+// the request ID is unknown.
119+//
120+// If onResolve is non-nil it runs after the pending entry has been
121+// taken but before the notification is published or the waiter is
122+// unblocked. This lets GrantPersistent record the session permission
123+// only when it actually wins the race, so a losing GrantPersistent
124+// that lost to a Deny does not leak an auto-approve entry.
125+//
126+// All three public resolution methods (Grant, GrantPersistent, Deny)
127+// route through this helper so multi-subscriber UIs can race safely:
128+// the first caller wins, the rest become no-ops.
103−func (s *permissionService) GrantPersistent(permission PermissionRequest) {
129+func (s *permissionService) resolve(permission PermissionRequest, granted, denied bool, onResolve func()) bool {
104− s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{
105− ToolCallID: permission.ToolCallID,
106− Granted: true,
107− })
108− respCh, ok := s.pendingRequests.Get(permission.ID)
130+ respCh, ok := s.pendingRequests.Take(permission.ID)
109− if ok {
131+ if !ok {
110− respCh <- true
132+ return false
111 }
133 }
112
134
113− s.sessionPermissions.Set(PermissionKey{
135+ if onResolve != nil {
114− SessionID: permission.SessionID,
136+ onResolve()
115− ToolName: permission.ToolName,
116− Action: permission.Action,
117− Path: permission.Path,
118− }, true)
119−
120− s.activeRequestMu.Lock()
121− if s.activeRequest != nil && s.activeRequest.ID == permission.ID {
122− s.activeRequest = nil
123 }
137 }
124− s.activeRequestMu.Unlock()
125−}
126
138
127−func (s *permissionService) Grant(permission PermissionRequest) {
128 s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{
139 s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{
129 ToolCallID: permission.ToolCallID,
140 ToolCallID: permission.ToolCallID,
130− Granted: true,
141+ Granted: granted,
142+ Denied: denied,
131 })
143 })
132− respCh, ok := s.pendingRequests.Get(permission.ID)
144+
133− if ok {
145+ // respCh is buffered (cap 1) and only ever has at most one sender
146+ // per request because Take removes the entry under the map lock,
147+ // so this send never blocks.
134− respCh <- true
148+ respCh <- granted
135− }
136
149
137 s.activeRequestMu.Lock()
150 s.activeRequestMu.Lock()
138 if s.activeRequest != nil && s.activeRequest.ID == permission.ID {
151 if s.activeRequest != nil && s.activeRequest.ID == permission.ID {
139 s.activeRequest = nil
152 s.activeRequest = nil
140 }
153 }
141 s.activeRequestMu.Unlock()
154 s.activeRequestMu.Unlock()
155+ return true
142}
156}
143
157
144−func (s *permissionService) Deny(permission PermissionRequest) {
158+func (s *permissionService) GrantPersistent(permission PermissionRequest) bool {
145− s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{
159+ // Record the persistent grant only if this call wins the
146− ToolCallID: permission.ToolCallID,
160+ // pending-request race. Otherwise a losing GrantPersistent that
147− Granted: false,
161+ // lost to a Deny would still leave an auto-approve entry behind,
148− Denied: true,
162+ // silently flipping later denied calls to allowed.
163+ return s.resolve(permission, true, false, func() {
164+ s.sessionPermissions.Set(PermissionKey{
165+ SessionID: permission.SessionID,
166+ ToolName: permission.ToolName,
167+ Action: permission.Action,
168+ Path: permission.Path,
169+ }, true)
149 })
170 })
150− respCh, ok := s.pendingRequests.Get(permission.ID)
151− if ok {
152− respCh <- false
153− }
171+}
154
172
155− s.activeRequestMu.Lock()
173+func (s *permissionService) Grant(permission PermissionRequest) bool {
156− if s.activeRequest != nil && s.activeRequest.ID == permission.ID {
174+ return s.resolve(permission, true, false, nil)
157− s.activeRequest = nil
158− }
175+}
159− s.activeRequestMu.Unlock()
176+
177+func (s *permissionService) Deny(permission PermissionRequest) bool {
178+ return s.resolve(permission, false, true, nil)
160}
179}
161
180
162func (s *permissionService) Request(ctx context.Context, opts CreatePermissionRequest) (bool, error) {
181func (s *permissionService) Request(ctx context.Context, opts CreatePermissionRequest) (bool, error) {
internal/permission/permission_test.go+271
@@ -2,7 +2,9 @@ package permission
2
2
3import (
3import (
4 "sync"
4 "sync"
5+ "sync/atomic"
5 "testing"
6 "testing"
7+ "time"
6
8
7 "github.com/stretchr/testify/assert"
9 "github.com/stretchr/testify/assert"
8 "github.com/stretchr/testify/require"
10 "github.com/stretchr/testify/require"
@@ -342,3 +344,272 @@ func TestPermissionService_SequentialProperties(t *testing.T) {
342 assert.True(t, result, "Repeated request should be auto-approved due to persistent permission")
344 assert.True(t, result, "Repeated request should be auto-approved due to persistent permission")
343 })
345 })
344}
346}
347+
348+// TestPermissionService_ResolveIdempotency covers the multi-subscriber
349+// resolve guarantees added for client/server mode: exactly one
350+// notification per resolution, racing callers see "already resolved",
351+// and stray Grant/Deny calls for unknown IDs are safe no-ops.
352+func TestPermissionService_ResolveIdempotency(t *testing.T) {
353+ t.Parallel()
354+
355+ t.Run("concurrent grants resolve exactly once", func(t *testing.T) {
356+ t.Parallel()
357+ service := NewPermissionService("/tmp", false, nil)
358+
359+ events := service.Subscribe(t.Context())
360+ notifications := service.SubscribeNotifications(t.Context())
361+
362+ req := CreatePermissionRequest{
363+ SessionID: "race-session",
364+ ToolCallID: "race-call",
365+ ToolName: "tool",
366+ Action: "act",
367+ Path: "/tmp/race",
368+ }
369+
370+ var (
371+ wg sync.WaitGroup
372+ granted bool
373+ requestErr error
374+ )
375+ wg.Go(func() {
376+ granted, requestErr = service.Request(t.Context(), req)
377+ })
378+
379+ // Wait for the request to be published so we have a real
380+ // PermissionRequest (with its server-side ID) to race on.
381+ var pending PermissionRequest
382+ select {
383+ case ev := <-events:
384+ pending = ev.Payload
385+ case <-time.After(2 * time.Second):
386+ t.Fatal("permission request was never published")
387+ }
388+
389+ // Drain the initial "request opened" notification (Granted ==
390+ // false && Denied == false) so the next read is the resolution
391+ // itself.
392+ select {
393+ case ev := <-notifications:
394+ require.False(t, ev.Payload.Granted, "initial notification must not be granted")
395+ require.False(t, ev.Payload.Denied, "initial notification must not be denied")
396+ case <-time.After(2 * time.Second):
397+ t.Fatal("initial notification was never published")
398+ }
399+
400+ // Race two grants from two goroutines.
401+ var (
402+ resolvedCount atomic.Int32
403+ start = make(chan struct{})
404+ racers sync.WaitGroup
405+ )
406+ for range 2 {
407+ racers.Go(func() {
408+ <-start
409+ if service.Grant(pending) {
410+ resolvedCount.Add(1)
411+ }
412+ })
413+ }
414+ close(start)
415+ racers.Wait()
416+
417+ // Original Request must return granted exactly once.
418+ wg.Wait()
419+ require.NoError(t, requestErr)
420+ assert.True(t, granted, "request should observe its grant")
421+
422+ // Exactly one of the two grants resolved the request.
423+ assert.Equal(t, int32(1), resolvedCount.Load(),
424+ "exactly one Grant should report it resolved the request")
425+
426+ // Exactly one resolution notification, and no further ones.
427+ select {
428+ case ev := <-notifications:
429+ assert.True(t, ev.Payload.Granted, "resolution notification should be granted")
430+ assert.Equal(t, "race-call", ev.Payload.ToolCallID)
431+ case <-time.After(2 * time.Second):
432+ t.Fatal("resolution notification was never published")
433+ }
434+ select {
435+ case ev := <-notifications:
436+ t.Fatalf("unexpected duplicate notification: %+v", ev.Payload)
437+ case <-time.After(50 * time.Millisecond):
438+ // good: no duplicate.
439+ }
440+
441+ // pendingRequests must be empty: no goroutine is left blocked
442+ // on a send, and a future Grant for the same ID is a no-op.
443+ ps := service.(*permissionService)
444+ assert.Equal(t, 0, ps.pendingRequests.Len(),
445+ "pendingRequests must be empty after resolution")
446+
447+ assert.False(t, service.Grant(pending),
448+ "a third Grant should report already-resolved")
449+ })
450+
451+ t.Run("grant after deny is a no-op", func(t *testing.T) {
452+ t.Parallel()
453+ service := NewPermissionService("/tmp", false, nil)
454+
455+ events := service.Subscribe(t.Context())
456+ notifications := service.SubscribeNotifications(t.Context())
457+
458+ req := CreatePermissionRequest{
459+ SessionID: "deny-first",
460+ ToolCallID: "df-call",
461+ ToolName: "tool",
462+ Action: "act",
463+ Path: "/tmp/df",
464+ }
465+
466+ var (
467+ wg sync.WaitGroup
468+ granted bool
469+ requestErr error
470+ )
471+ wg.Go(func() {
472+ granted, requestErr = service.Request(t.Context(), req)
473+ })
474+
475+ var pending PermissionRequest
476+ select {
477+ case ev := <-events:
478+ pending = ev.Payload
479+ case <-time.After(2 * time.Second):
480+ t.Fatal("permission request was never published")
481+ }
482+
483+ // Drain the initial neither-granted-nor-denied notification.
484+ <-notifications
485+
486+ assert.True(t, service.Deny(pending), "Deny should resolve the request")
487+ wg.Wait()
488+ require.NoError(t, requestErr)
489+ assert.False(t, granted, "request should observe denial")
490+
491+ // A follow-up Grant must be a no-op and must not flip the
492+ // outcome or publish anything new.
493+ assert.False(t, service.Grant(pending),
494+ "Grant after Deny should report already-resolved")
495+
496+ select {
497+ case ev := <-notifications:
498+ // The first resolution notification (denial) is expected;
499+ // anything after that is a bug.
500+ require.True(t, ev.Payload.Denied,
501+ "the only post-initial notification must be the denial")
502+ case <-time.After(2 * time.Second):
503+ t.Fatal("denial notification was never published")
504+ }
505+ select {
506+ case ev := <-notifications:
507+ t.Fatalf("Grant after Deny must not publish: %+v", ev.Payload)
508+ case <-time.After(50 * time.Millisecond):
509+ // good.
510+ }
511+ })
512+
513+ t.Run("losing GrantPersistent does not record session permission", func(t *testing.T) {
514+ t.Parallel()
515+ service := NewPermissionService("/tmp", false, nil)
516+
517+ events := service.Subscribe(t.Context())
518+ notifications := service.SubscribeNotifications(t.Context())
519+
520+ req := CreatePermissionRequest{
521+ SessionID: "race-persist",
522+ ToolCallID: "rp-call",
523+ ToolName: "tool",
524+ Action: "act",
525+ Path: "/tmp/rp",
526+ }
527+
528+ var (
529+ wg sync.WaitGroup
530+ granted bool
531+ requestErr error
532+ )
533+ wg.Go(func() {
534+ granted, requestErr = service.Request(t.Context(), req)
535+ })
536+
537+ // Wait for the request to be published so we have the real
538+ // pending PermissionRequest to race on.
539+ var pending PermissionRequest
540+ select {
541+ case ev := <-events:
542+ pending = ev.Payload
543+ case <-time.After(2 * time.Second):
544+ t.Fatal("permission request was never published")
545+ }
546+
547+ // Drain the initial neither-granted-nor-denied notification.
548+ <-notifications
549+
550+ // Deny wins, then a competing GrantPersistent loses.
551+ assert.True(t, service.Deny(pending), "Deny should resolve the request")
552+ assert.False(t, service.GrantPersistent(pending),
553+ "GrantPersistent after Deny should report already-resolved")
554+
555+ wg.Wait()
556+ require.NoError(t, requestErr)
557+ assert.False(t, granted, "request should observe denial")
558+
559+ // The losing GrantPersistent must not have inserted an
560+ // auto-approve entry. Issue a matching follow-up request and
561+ // confirm the service still publishes a pending request (i.e.
562+ // not auto-approved). We then Deny it to drain the goroutine.
563+ var (
564+ wg2 sync.WaitGroup
565+ granted2 bool
566+ requestErr2 error
567+ )
568+ wg2.Go(func() {
569+ granted2, requestErr2 = service.Request(t.Context(), req)
570+ })
571+
572+ select {
573+ case ev := <-events:
574+ assert.Equal(t, pending.SessionID, ev.Payload.SessionID)
575+ service.Deny(ev.Payload)
576+ case <-time.After(2 * time.Second):
577+ t.Fatal("follow-up request was auto-approved; persistent grant leaked")
578+ }
579+
580+ wg2.Wait()
581+ require.NoError(t, requestErr2)
582+ assert.False(t, granted2, "follow-up request should be denied, not auto-approved")
583+ })
584+
585+ t.Run("grant for unknown id is a safe no-op", func(t *testing.T) {
586+ t.Parallel()
587+ service := NewPermissionService("/tmp", false, nil)
588+
589+ notifications := service.SubscribeNotifications(t.Context())
590+
591+ bogus := PermissionRequest{
592+ ID: "does-not-exist",
593+ ToolCallID: "ghost",
594+ ToolName: "tool",
595+ Action: "act",
596+ Path: "/tmp/ghost",
597+ }
598+
599+ assert.NotPanics(t, func() {
600+ assert.False(t, service.Grant(bogus),
601+ "Grant for unknown ID should report already-resolved")
602+ assert.False(t, service.GrantPersistent(bogus),
603+ "GrantPersistent for unknown ID should report already-resolved")
604+ assert.False(t, service.Deny(bogus),
605+ "Deny for unknown ID should report already-resolved")
606+ })
607+
608+ select {
609+ case ev := <-notifications:
610+ t.Fatalf("unknown-ID resolution must not publish: %+v", ev.Payload)
611+ case <-time.After(50 * time.Millisecond):
612+ // good: no notification.
613+ }
614+ })
615+}
internal/proto/proto.go+31−8
@@ -13,14 +13,15 @@ import (
13// Workspace represents a running app.App workspace with its associated
13// Workspace represents a running app.App workspace with its associated
14// resources and state.
14// resources and state.
15type Workspace struct {
15type Workspace struct {
16− ID string `json:"id"`
16+ ID string `json:"id"`
17− Path string `json:"path"`
17+ Path string `json:"path"`
18− YOLO bool `json:"yolo,omitempty"`
18+ YOLO bool `json:"yolo,omitempty"`
19− Debug bool `json:"debug,omitempty"`
19+ Debug bool `json:"debug,omitempty"`
20− DataDir string `json:"data_dir,omitempty"`
20+ DataDir string `json:"data_dir,omitempty"`
21− Version string `json:"version,omitempty"`
21+ Version string `json:"version,omitempty"`
22+ ClientID string `json:"client_id,omitempty"`
22− Config *config.Config `json:"config,omitempty"`
23+ Config *config.Config `json:"config,omitempty"`
23− Env []string `json:"env,omitempty"`
24+ Env []string `json:"env,omitempty"`
24 // Skills carries the snapshot of skill discovery state at workspace
25 // Skills carries the snapshot of skill discovery state at workspace
25 // creation time. Subsequent updates flow through the SSE event
26 // creation time. Subsequent updates flow through the SSE event
26 // stream.
27 // stream.
@@ -32,6 +33,19 @@ type Error struct {
32 Message string `json:"message"`
33 Message string `json:"message"`
33}
34}
34
35
36+// ConfigChanged is published whenever the workspace's configuration is
37+// mutated by a backend operation. Clients react by re-fetching the
38+// workspace snapshot so cached config stays in sync across subscribers.
39+type ConfigChanged struct {
40+ WorkspaceID string `json:"workspace_id"`
41+}
42+
43+// CurrentSession is the request body for the per-client
44+// current-session endpoint. An empty SessionID clears the entry.
45+type CurrentSession struct {
46+ SessionID string `json:"session_id"`
47+}
48+
35// SkillInfo describes a visible skill exposed to a frontend.
49// SkillInfo describes a visible skill exposed to a frontend.
36type SkillInfo struct {
50type SkillInfo struct {
37 ID string `json:"id"`
51 ID string `json:"id"`
@@ -118,6 +132,15 @@ type PermissionGrant struct {
118 Action PermissionAction `json:"action"`
132 Action PermissionAction `json:"action"`
119}
133}
120
134
135+// PermissionGrantResponse is the server's response to a permission
136+// grant call. Resolved is true when this call resolved the pending
137+// request, and false when the request had already been resolved by a
138+// previous caller (e.g., another client in a multi-subscriber UI). A
139+// false value is not an error.
140+type PermissionGrantResponse struct {
141+ Resolved bool `json:"resolved"`
142+}
143+
121// PermissionSkipRequest represents a request to skip permission prompts.
144// PermissionSkipRequest represents a request to skip permission prompts.
122type PermissionSkipRequest struct {
145type PermissionSkipRequest struct {
123 Skip bool `json:"skip"`
146 Skip bool `json:"skip"`
internal/proto/session.go+14
@@ -1,6 +1,18 @@
1package proto
1package proto
2
2
3// Session represents a session in the proto layer.
3// Session represents a session in the proto layer.
4+//
5+// IsBusy is computed on read (it is not persisted with the session) and
6+// reflects whether an agent run is currently in flight for this session.
7+// It is populated by REST handlers in internal/server/proto.go from the
8+// workspace's AgentCoordinator. The Session SSE event path does not set
9+// it, since SSE consumers can compute presence from other agent signals.
10+//
11+// AttachedClients counts the number of clients currently viewing this
12+// session — i.e. entries in the workspace's clients map whose
13+// currentSessionID equals this session's ID and which have at least one
14+// live SSE stream. Hold-only clients (streams == 0) do not contribute.
15+// Like IsBusy, it is computed on read by REST handlers.
4type Session struct {
16type Session struct {
5 ID string `json:"id"`
17 ID string `json:"id"`
6 ParentSessionID string `json:"parent_session_id"`
18 ParentSessionID string `json:"parent_session_id"`
@@ -13,6 +25,8 @@ type Session struct {
13 Todos []Todo `json:"todos,omitempty"`
25 Todos []Todo `json:"todos,omitempty"`
14 CreatedAt int64 `json:"created_at"`
26 CreatedAt int64 `json:"created_at"`
15 UpdatedAt int64 `json:"updated_at"`
27 UpdatedAt int64 `json:"updated_at"`
28+ IsBusy bool `json:"is_busy"`
29+ AttachedClients int `json:"attached_clients"`
16}
30}
17
31
18// Todo represents a single todo entry on a session in the proto layer.
32// Todo represents a single todo entry on a session in the proto layer.
internal/pubsub/events.go+1
@@ -24,6 +24,7 @@ const (
24 PayloadTypeSession PayloadType = "session"
24 PayloadTypeSession PayloadType = "session"
25 PayloadTypeFile PayloadType = "file"
25 PayloadTypeFile PayloadType = "file"
26 PayloadTypeAgentEvent PayloadType = "agent_event"
26 PayloadTypeAgentEvent PayloadType = "agent_event"
27+ PayloadTypeConfigChanged PayloadType = "config_changed"
27 PayloadTypeSkillsEvent PayloadType = "skills_event"
28 PayloadTypeSkillsEvent PayloadType = "skills_event"
28)
29)
29
30
internal/server/e2e_test.gonew file+600
@@ -0,0 +1,600 @@
1+package server
2+
3+import (
4+ "bufio"
5+ "bytes"
6+ "context"
7+ "encoding/json"
8+ "io"
9+ "net/http"
10+ "net/http/httptest"
11+ "net/url"
12+ "sync"
13+ "sync/atomic"
14+ "testing"
15+ "time"
16+
17+ "github.com/charmbracelet/crush/internal/app"
18+ "github.com/charmbracelet/crush/internal/backend"
19+ "github.com/charmbracelet/crush/internal/db"
20+ "github.com/charmbracelet/crush/internal/message"
21+ "github.com/charmbracelet/crush/internal/permission"
22+ "github.com/charmbracelet/crush/internal/proto"
23+ "github.com/charmbracelet/crush/internal/pubsub"
24+ "github.com/google/uuid"
25+ "github.com/stretchr/testify/require"
26+)
27+
28+// e2eHarness wires a Server, its Backend (with a custom shutdownFn we
29+// can observe), an httptest.NewServer, and a synthetic Workspace whose
30+// embedded App has a live event broker. It is the minimum scaffolding
31+// the multi-client end-to-end scenarios in PLAN item 6 need.
32+type e2eHarness struct {
33+ httpSrv *httptest.Server
34+ srv *Server
35+ backend *backend.Backend
36+ workspace *backend.Workspace
37+ app *app.App
38+ shutdownHit atomic.Bool
39+
40+ // sseWG tracks every SSE reader goroutine spawned by
41+ // [e2eHarness.subscribeSSE]. The harness's cleanup hook waits on
42+ // it after the httptest server has been closed so that the test
43+ // cannot leave behind background readers (and therefore unclosed
44+ // response bodies) after returning.
45+ sseWG sync.WaitGroup
46+}
47+
48+// installServer attaches a fresh Server (with a custom shutdown
49+// callback that flips [e2eHarness.shutdownHit]) wrapped in an
50+// [httptest.Server] onto h. It registers the cleanup hooks for the
51+// httptest server and the SSE reader WaitGroup in the order required
52+// by the LIFO contract documented on [newE2EHarness].
53+//
54+// Callers that want a fully synthetic workspace use [newE2EHarness];
55+// callers that want to drive the real CreateWorkspace HTTP path use
56+// [newRealCreateHarness] and then [e2eHarness.postWorkspace].
57+func (h *e2eHarness) installServer(t *testing.T) {
58+ t.Helper()
59+ srv := &Server{}
60+ srv.backend = backend.New(context.Background(), nil, func() {
61+ h.shutdownHit.Store(true)
62+ })
63+ srv.installHandler()
64+
65+ hs := httptest.NewServer(srv.Handler())
66+ // Order matters: t.Cleanup is LIFO and the test's own per-
67+ // stream cancels (cancelA/cancelB) run first. After those, we
68+ // want hs.Close to fire first (so any handler still parked in
69+ // its `select` returns), THEN sseWG.Wait so every reader
70+ // goroutine exits and closes its response body. Any caller-
71+ // owned cleanups registered *before* installServer (e.g. App
72+ // teardown for the synthetic harness) therefore run LAST,
73+ // after the readers have drained.
74+ t.Cleanup(h.sseWG.Wait)
75+ t.Cleanup(hs.Close)
76+
77+ h.httpSrv = hs
78+ h.srv = srv
79+ h.backend = srv.backend
80+}
81+
82+// newE2EHarness builds an in-process server + a synthetic Workspace
83+// whose embedded App is a real [app.App] constructed via
84+// [app.NewForTest], so its event broker delivers everything the SSE
85+// pipeline expects. Used by the scenarios that do not need to
86+// exercise the path-dedupe behavior of [backend.CreateWorkspace].
87+//
88+// Cleanup tears down the App's broker only after sseWG.Wait and
89+// hs.Close have run, so SSE readers cannot observe a dead broker.
90+func newE2EHarness(t *testing.T) *e2eHarness {
91+ t.Helper()
92+
93+ h := &e2eHarness{}
94+
95+ // Register the App teardown FIRST so LIFO order puts it AFTER
96+ // the cleanups that installServer registers below (hs.Close +
97+ // sseWG.Wait).
98+ appCtx, cancel := context.WithCancel(context.Background())
99+ a := app.NewForTest(appCtx)
100+ t.Cleanup(func() {
101+ cancel()
102+ a.ShutdownForTest()
103+ })
104+
105+ h.installServer(t)
106+
107+ ws := &backend.Workspace{
108+ ID: uuid.New().String(),
109+ Path: t.TempDir(),
110+ App: a,
111+ }
112+ // Synthetic workspaces have an incomplete App; bypass the
113+ // default teardown so the "last workspace removed" path can run
114+ // without panicking inside [app.App.Shutdown].
115+ backend.SetWorkspaceShutdownFnForTest(ws, func() {})
116+ backend.InsertWorkspaceForTest(h.backend, ws)
117+
118+ h.workspace = ws
119+ h.app = a
120+ return h
121+}
122+
123+// newRealCreateHarness builds an in-process server WITHOUT any
124+// pre-inserted workspace, intended for tests that drive the real
125+// [backend.CreateWorkspace] HTTP path (path-dedupe scenario). It
126+// isolates HOME/XDG_* via [t.Setenv] so [config.Init] doesn't read
127+// the host machine's config, which means callers MUST NOT mark the
128+// test as parallel.
129+func newRealCreateHarness(t *testing.T) *e2eHarness {
130+ t.Helper()
131+ t.Setenv("HOME", t.TempDir())
132+ t.Setenv("XDG_CACHE_HOME", t.TempDir())
133+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
134+ t.Setenv("XDG_DATA_HOME", t.TempDir())
135+
136+ h := &e2eHarness{}
137+ h.installServer(t)
138+ return h
139+}
140+
141+// postWorkspace drives the real POST /v1/workspaces handler and
142+// returns the resolved workspace proto. This is how scenario 1
143+// exercises the path-dedupe behavior from PLAN item 1: two calls
144+// with the same Path and distinct ClientIDs must return the same
145+// workspace ID.
146+func (h *e2eHarness) postWorkspace(t *testing.T, args proto.Workspace) proto.Workspace {
147+ t.Helper()
148+ body, err := json.Marshal(args)
149+ require.NoError(t, err)
150+ req, err := http.NewRequestWithContext(t.Context(), http.MethodPost,
151+ h.httpSrv.URL+"/v1/workspaces", bytes.NewReader(body))
152+ require.NoError(t, err)
153+ req.Header.Set("Content-Type", "application/json")
154+ resp, err := h.httpSrv.Client().Do(req)
155+ require.NoError(t, err)
156+ defer resp.Body.Close()
157+ require.Equal(t, http.StatusOK, resp.StatusCode, "POST /v1/workspaces must succeed")
158+ var out proto.Workspace
159+ require.NoError(t, json.NewDecoder(resp.Body).Decode(&out))
160+ require.NotEmpty(t, out.ID, "server must return a workspace id")
161+ return out
162+}
163+
164+// subscribeSSE opens an SSE stream against the test server for the
165+// given workspace and client ID. It returns a channel of decoded
166+// envelopes plus a cancel function that closes the stream. The
167+// returned channel is closed when the stream ends.
168+func (h *e2eHarness) subscribeSSE(t *testing.T, ctx context.Context, workspaceID, clientID string) (<-chan any, context.CancelFunc) {
169+ t.Helper()
170+ streamCtx, cancel := context.WithCancel(ctx)
171+
172+ q := url.Values{"client_id": []string{clientID}}
173+ reqURL := h.httpSrv.URL + "/v1/workspaces/" + workspaceID + "/events?" + q.Encode()
174+ req, err := http.NewRequestWithContext(streamCtx, http.MethodGet, reqURL, nil)
175+ require.NoError(t, err)
176+ req.Header.Set("Accept", "text/event-stream")
177+
178+ resp, err := h.httpSrv.Client().Do(req)
179+ require.NoError(t, err)
180+ require.Equal(t, http.StatusOK, resp.StatusCode, "SSE subscribe should return 200")
181+
182+ out := make(chan any, 64)
183+ h.sseWG.Go(func() {
184+ defer resp.Body.Close()
185+ defer close(out)
186+ reader := bufio.NewReader(resp.Body)
187+ for {
188+ line, err := reader.ReadBytes('\n')
189+ if err != nil {
190+ return
191+ }
192+ line = bytes.TrimSpace(line)
193+ if len(line) == 0 {
194+ continue
195+ }
196+ data, ok := bytes.CutPrefix(line, []byte("data:"))
197+ if !ok {
198+ continue
199+ }
200+ data = bytes.TrimSpace(data)
201+ var p pubsub.Payload
202+ if err := json.Unmarshal(data, &p); err != nil {
203+ continue
204+ }
205+ ev, decoded := decodeSSEEnvelope(p)
206+ if !decoded {
207+ continue
208+ }
209+ select {
210+ case out <- ev:
211+ case <-streamCtx.Done():
212+ return
213+ }
214+ }
215+ })
216+ return out, cancel
217+}
218+
219+// decodeSSEEnvelope decodes the discriminated SSE envelope into the
220+// concrete pubsub.Event[proto.X] payload the e2e tests care about.
221+// Unknown payload types are skipped so tests can match on type
222+// assertions without worrying about envelope noise.
223+func decodeSSEEnvelope(p pubsub.Payload) (any, bool) {
224+ switch p.Type {
225+ case pubsub.PayloadTypePermissionRequest:
226+ var e pubsub.Event[proto.PermissionRequest]
227+ if err := json.Unmarshal(p.Payload, &e); err != nil {
228+ return nil, false
229+ }
230+ return e, true
231+ case pubsub.PayloadTypePermissionNotification:
232+ var e pubsub.Event[proto.PermissionNotification]
233+ if err := json.Unmarshal(p.Payload, &e); err != nil {
234+ return nil, false
235+ }
236+ return e, true
237+ case pubsub.PayloadTypeMessage:
238+ var e pubsub.Event[proto.Message]
239+ if err := json.Unmarshal(p.Payload, &e); err != nil {
240+ return nil, false
241+ }
242+ return e, true
243+ }
244+ return nil, false
245+}
246+
247+// grantPermission posts a permission grant via the HTTP surface and
248+// returns the server's "resolved" verdict. Mirrors the client-side
249+// GrantPermission flow without importing internal/client (which
250+// would create an import cycle from this in-package test).
251+func (h *e2eHarness) grantPermission(t *testing.T, ctx context.Context, workspaceID string, req proto.PermissionGrant) bool {
252+ t.Helper()
253+ body, err := json.Marshal(req)
254+ require.NoError(t, err)
255+ httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost,
256+ h.httpSrv.URL+"/v1/workspaces/"+workspaceID+"/permissions/grant",
257+ bytes.NewReader(body))
258+ require.NoError(t, err)
259+ httpReq.Header.Set("Content-Type", "application/json")
260+ resp, err := h.httpSrv.Client().Do(httpReq)
261+ require.NoError(t, err)
262+ defer resp.Body.Close()
263+ require.Equal(t, http.StatusOK, resp.StatusCode)
264+ var out proto.PermissionGrantResponse
265+ require.NoError(t, json.NewDecoder(resp.Body).Decode(&out))
266+ return out.Resolved
267+}
268+
269+// waitForAttached spins until the workspace's clients map reports at
270+// least n entries with streams > 0. Catches the race where a test
271+// publishes events before the server-side AttachClient has completed.
272+func (h *e2eHarness) waitForAttached(t *testing.T, n int) {
273+ t.Helper()
274+ h.waitForAttachedOn(t, h.workspace, n)
275+}
276+
277+// waitForAttachedOn is the workspace-explicit form of waitForAttached.
278+// Tests that drive a workspace whose pointer is not stored on the
279+// harness (e.g. the real CreateWorkspace path) pass the workspace in.
280+func (h *e2eHarness) waitForAttachedOn(t *testing.T, ws *backend.Workspace, n int) {
281+ t.Helper()
282+ deadline := time.Now().Add(2 * time.Second)
283+ for time.Now().Before(deadline) {
284+ if backend.WorkspaceLiveStreamCountForTest(ws) >= n {
285+ return
286+ }
287+ time.Sleep(5 * time.Millisecond)
288+ }
289+ t.Fatalf("expected %d attached streams, have %d", n,
290+ backend.WorkspaceLiveStreamCountForTest(ws))
291+}
292+
293+// drainUntil reads from evc until it sees an event of type T that
294+// satisfies match, or ctx expires. Returns the matching event and
295+// ok=true, or the zero value and ok=false on timeout.
296+func drainUntil[T any](ctx context.Context, evc <-chan any, match func(T) bool) (T, bool) {
297+ var zero T
298+ for {
299+ select {
300+ case <-ctx.Done():
301+ return zero, false
302+ case ev, ok := <-evc:
303+ if !ok {
304+ return zero, false
305+ }
306+ typed, isT := ev.(T)
307+ if !isT {
308+ continue
309+ }
310+ if match == nil || match(typed) {
311+ return typed, true
312+ }
313+ }
314+ }
315+}
316+
317+// TestE2E_TwoClientsReceiveSameMessage covers PLAN item 6 scenario 1:
318+// two clients POST /v1/workspaces with the same Path and observe
319+// that the server returns a single workspace (path-dedupe from PLAN
320+// item 1) and that an event published on that workspace fans out to
321+// both SSE streams.
322+//
323+// Cannot run in parallel: it isolates HOME/XDG_* via t.Setenv so
324+// config.Init does not read the host machine's real config.
325+func TestE2E_TwoClientsReceiveSameMessage(t *testing.T) {
326+ h := newRealCreateHarness(t)
327+ // Shorten the create-grace window so the workspace's pending
328+ // creation holds release quickly during test cleanup once both
329+ // SSE streams have been detached.
330+ h.backend.SetCreateGrace(200 * time.Millisecond)
331+
332+ ctx, cancel := context.WithCancel(t.Context())
333+ t.Cleanup(cancel)
334+
335+ cidA := uuid.New().String()
336+ cidB := uuid.New().String()
337+
338+ // Shared workspace path. Two POSTs with this path must
339+ // deduplicate at the backend's pathIndex and return the same
340+ // workspace id.
341+ wsPath := t.TempDir()
342+ dataDir := t.TempDir()
343+ args := proto.Workspace{Path: wsPath, DataDir: dataDir}
344+
345+ argsA := args
346+ argsA.ClientID = cidA
347+ wsRespA := h.postWorkspace(t, argsA)
348+
349+ argsB := args
350+ argsB.ClientID = cidB
351+ wsRespB := h.postWorkspace(t, argsB)
352+
353+ require.Equal(t, wsRespA.ID, wsRespB.ID,
354+ "POST /v1/workspaces with the same Path must return the same workspace id")
355+
356+ // Look up the resulting workspace on the backend so the test
357+ // can publish events through its real [app.App] event broker.
358+ ws, err := h.backend.GetWorkspace(wsRespA.ID)
359+ require.NoError(t, err)
360+ // Override the shutdown callback so test cleanup doesn't run
361+ // the full app.Shutdown path (which would tear down LSP/MCP
362+ // resources the test doesn't need to exercise), but still
363+ // release the pooled DB connection so Windows can clean up
364+ // the temp data directory.
365+ wsDataDir := ws.Cfg.Config().Options.DataDirectory
366+ backend.SetWorkspaceShutdownFnForTest(ws, func() {
367+ _ = db.Release(wsDataDir)
368+ })
369+
370+ evcA, cancelA := h.subscribeSSE(t, ctx, ws.ID, cidA)
371+ t.Cleanup(cancelA)
372+ evcB, cancelB := h.subscribeSSE(t, ctx, ws.ID, cidB)
373+ t.Cleanup(cancelB)
374+
375+ h.waitForAttachedOn(t, ws, 2)
376+
377+ const sessionID = "s-e2e-1"
378+ msg := message.Message{
379+ ID: "m-1",
380+ SessionID: sessionID,
381+ Role: message.Assistant,
382+ Parts: []message.ContentPart{message.TextContent{Text: "hello multi-client"}},
383+ }
384+ ws.SendEvent(pubsub.Event[message.Message]{
385+ Type: pubsub.CreatedEvent,
386+ Payload: msg,
387+ })
388+
389+ pickCtx, pickCancel := context.WithTimeout(ctx, 3*time.Second)
390+ defer pickCancel()
391+ gotA, okA := drainUntil(pickCtx, evcA, func(e pubsub.Event[proto.Message]) bool {
392+ return e.Payload.ID == "m-1"
393+ })
394+ require.True(t, okA, "client A must receive the MessageEvent")
395+ require.Equal(t, sessionID, gotA.Payload.SessionID)
396+
397+ gotB, okB := drainUntil(pickCtx, evcB, func(e pubsub.Event[proto.Message]) bool {
398+ return e.Payload.ID == "m-1"
399+ })
400+ require.True(t, okB, "client B must receive the same MessageEvent")
401+ require.Equal(t, sessionID, gotB.Payload.SessionID)
402+}
403+
404+// TestE2E_PermissionFlowCrossClient covers PLAN item 6 scenario 2:
405+// a tool-driven permission request is granted by client A; client B
406+// observes a PermissionNotification; a redundant grant from B
407+// returns the "already resolved" indicator (resolved=false from the
408+// bool plumbing landed in item 3).
409+func TestE2E_PermissionFlowCrossClient(t *testing.T) {
410+ t.Parallel()
411+ h := newE2EHarness(t)
412+ ctx, cancel := context.WithCancel(t.Context())
413+ t.Cleanup(cancel)
414+
415+ cidA := uuid.New().String()
416+ cidB := uuid.New().String()
417+
418+ evcA, cancelA := h.subscribeSSE(t, ctx, h.workspace.ID, cidA)
419+ t.Cleanup(cancelA)
420+ evcB, cancelB := h.subscribeSSE(t, ctx, h.workspace.ID, cidB)
421+ t.Cleanup(cancelB)
422+
423+ h.waitForAttached(t, 2)
424+
425+ // Drive the permission request from a goroutine simulating the
426+ // tool path. Request blocks until resolved; capture the outcome.
427+ const sessionID = "s-perm"
428+ const toolCallID = "tc-1"
429+ type result struct {
430+ granted bool
431+ err error
432+ }
433+ done := make(chan result, 1)
434+ go func() {
435+ granted, err := h.app.Permissions.Request(ctx, permission.CreatePermissionRequest{
436+ SessionID: sessionID,
437+ ToolCallID: toolCallID,
438+ ToolName: "view",
439+ Description: "read a file",
440+ Action: "read",
441+ Path: h.workspace.Path,
442+ })
443+ done <- result{granted: granted, err: err}
444+ }()
445+
446+ // Wait for the PermissionRequest to arrive on client A's SSE
447+ // stream. We need its ID to drive the grant.
448+ pickCtx, pickCancel := context.WithTimeout(ctx, 3*time.Second)
449+ defer pickCancel()
450+ reqEv, ok := drainUntil(pickCtx, evcA, func(e pubsub.Event[proto.PermissionRequest]) bool {
451+ return e.Payload.ToolCallID == toolCallID
452+ })
453+ require.True(t, ok, "client A must receive the PermissionRequest")
454+
455+ // Client A grants — first grant must report resolved=true.
456+ resolvedA := h.grantPermission(t, ctx, h.workspace.ID, proto.PermissionGrant{
457+ Permission: reqEv.Payload,
458+ Action: proto.PermissionAllow,
459+ })
460+ require.True(t, resolvedA, "client A's grant must resolve the pending request")
461+
462+ // The blocked Request call must now return granted=true.
463+ select {
464+ case r := <-done:
465+ require.NoError(t, r.err)
466+ require.True(t, r.granted)
467+ case <-pickCtx.Done():
468+ t.Fatal("permission Request did not return after grant")
469+ }
470+
471+ // Client B must receive a PermissionNotification with
472+ // Granted=true for the same ToolCallID. The initial neither-
473+ // granted-nor-denied notification published at the start of
474+ // Request also lands on B's stream — match on the granted one.
475+ notif, ok := drainUntil(pickCtx, evcB, func(e pubsub.Event[proto.PermissionNotification]) bool {
476+ return e.Payload.ToolCallID == toolCallID && e.Payload.Granted
477+ })
478+ require.True(t, ok, "client B must receive a granting PermissionNotification")
479+ require.True(t, notif.Payload.Granted)
480+ require.False(t, notif.Payload.Denied)
481+
482+ // A follow-up grant from client B must report resolved=false
483+ // (the request was already resolved by A).
484+ resolvedB := h.grantPermission(t, ctx, h.workspace.ID, proto.PermissionGrant{
485+ Permission: reqEv.Payload,
486+ Action: proto.PermissionAllow,
487+ })
488+ require.False(t, resolvedB, "client B's follow-up grant must report already resolved")
489+}
490+
491+// TestE2E_KillingClientASSEDoesNotBreakClientB covers PLAN item 6
492+// scenario 3: terminating client A's SSE stream does not affect
493+// client B's stream; client B continues to receive events.
494+func TestE2E_KillingClientASSEDoesNotBreakClientB(t *testing.T) {
495+ t.Parallel()
496+ h := newE2EHarness(t)
497+ ctxB, cancelB := context.WithCancel(t.Context())
498+ t.Cleanup(cancelB)
499+ ctxA, cancelA := context.WithCancel(t.Context())
500+
501+ cidA := uuid.New().String()
502+ cidB := uuid.New().String()
503+
504+ _, killA := h.subscribeSSE(t, ctxA, h.workspace.ID, cidA)
505+ t.Cleanup(killA)
506+ evcB, killB := h.subscribeSSE(t, ctxB, h.workspace.ID, cidB)
507+ t.Cleanup(killB)
508+
509+ h.waitForAttached(t, 2)
510+
511+ // Kill A's stream. The server's deferred DetachClient should
512+ // drop A's claim, leaving B as the sole attached client.
513+ cancelA()
514+ killA()
515+
516+ require.Eventually(t, func() bool {
517+ return backend.WorkspaceLiveStreamCountForTest(h.workspace) == 1
518+ }, 3*time.Second, 10*time.Millisecond,
519+ "expected client A's stream to drop the attached count to 1")
520+
521+ // Workspace must still exist (B is holding it open) and
522+ // shutdown callback must not have fired yet.
523+ _, err := h.backend.GetWorkspace(h.workspace.ID)
524+ require.NoError(t, err, "workspace must still exist while B is attached")
525+ require.False(t, h.shutdownHit.Load(),
526+ "shutdown callback must not fire while B is still attached")
527+
528+ // Publish a fresh event; B must still receive it.
529+ const sessionID = "s-after-a-died"
530+ msg := message.Message{
531+ ID: "m-after",
532+ SessionID: sessionID,
533+ Role: message.Assistant,
534+ Parts: []message.ContentPart{message.TextContent{Text: "still alive"}},
535+ }
536+ h.app.SendEvent(pubsub.Event[message.Message]{
537+ Type: pubsub.CreatedEvent,
538+ Payload: msg,
539+ })
540+
541+ pickCtx, pickCancel := context.WithTimeout(ctxB, 3*time.Second)
542+ defer pickCancel()
543+ got, ok := drainUntil(pickCtx, evcB, func(e pubsub.Event[proto.Message]) bool {
544+ return e.Payload.ID == "m-after"
545+ })
546+ require.True(t, ok, "client B must still receive events after A's stream is killed")
547+ require.Equal(t, sessionID, got.Payload.SessionID)
548+}
549+
550+// TestE2E_ShutdownCallbackFiresWhenLastClientLeaves covers PLAN
551+// item 6 scenario 4: once both clients disconnect, the backend
552+// runs its "last workspace removed -> server shutdown" path.
553+func TestE2E_ShutdownCallbackFiresWhenLastClientLeaves(t *testing.T) {
554+ t.Parallel()
555+ h := newE2EHarness(t)
556+
557+ ctxA, cancelA := context.WithCancel(t.Context())
558+ ctxB, cancelB := context.WithCancel(t.Context())
559+ t.Cleanup(cancelA)
560+ t.Cleanup(cancelB)
561+
562+ cidA := uuid.New().String()
563+ cidB := uuid.New().String()
564+ _, killA := h.subscribeSSE(t, ctxA, h.workspace.ID, cidA)
565+ t.Cleanup(killA)
566+ _, killB := h.subscribeSSE(t, ctxB, h.workspace.ID, cidB)
567+ t.Cleanup(killB)
568+
569+ h.waitForAttached(t, 2)
570+ require.False(t, h.shutdownHit.Load(), "shutdown must not fire while clients are attached")
571+
572+ cancelA()
573+ killA()
574+ require.Eventually(t, func() bool {
575+ return backend.WorkspaceLiveStreamCountForTest(h.workspace) == 1
576+ }, 3*time.Second, 10*time.Millisecond)
577+ require.False(t, h.shutdownHit.Load(),
578+ "shutdown must not fire after only one client disconnects")
579+
580+ cancelB()
581+ killB()
582+ require.Eventually(t, h.shutdownHit.Load,
583+ 3*time.Second, 10*time.Millisecond,
584+ "shutdown callback must fire once the last client disconnects")
585+
586+ // Workspace must be gone from the index.
587+ _, err := h.backend.GetWorkspace(h.workspace.ID)
588+ require.ErrorIs(t, err, backend.ErrWorkspaceNotFound)
589+
590+ // Subsequent GETs against the now-defunct workspace return
591+ // 404, confirming the http surface still reflects the teardown.
592+ req, err := http.NewRequestWithContext(t.Context(), http.MethodGet,
593+ h.httpSrv.URL+"/v1/workspaces/"+h.workspace.ID, nil)
594+ require.NoError(t, err)
595+ r, err := h.httpSrv.Client().Do(req)
596+ require.NoError(t, err)
597+ _, _ = io.Copy(io.Discard, r.Body)
598+ r.Body.Close()
599+ require.Equal(t, http.StatusNotFound, r.StatusCode)
600+}
internal/server/events.go+26
@@ -8,6 +8,7 @@ import (
8 "github.com/charmbracelet/crush/internal/agent/notify"
8 "github.com/charmbracelet/crush/internal/agent/notify"
9 "github.com/charmbracelet/crush/internal/agent/tools/mcp"
9 "github.com/charmbracelet/crush/internal/agent/tools/mcp"
10 "github.com/charmbracelet/crush/internal/app"
10 "github.com/charmbracelet/crush/internal/app"
11+ "github.com/charmbracelet/crush/internal/backend"
11 "github.com/charmbracelet/crush/internal/history"
12 "github.com/charmbracelet/crush/internal/history"
12 "github.com/charmbracelet/crush/internal/message"
13 "github.com/charmbracelet/crush/internal/message"
13 "github.com/charmbracelet/crush/internal/permission"
14 "github.com/charmbracelet/crush/internal/permission"
@@ -92,6 +93,8 @@ func wrapEvent(ev any) *pubsub.Payload {
92 Type: proto.AgentEventType(e.Payload.Type),
93 Type: proto.AgentEventType(e.Payload.Type),
93 },
94 },
94 })
95 })
96+ case pubsub.Event[proto.ConfigChanged]:
97+ return envelope(pubsub.PayloadTypeConfigChanged, e)
95 case pubsub.Event[skills.Event]:
98 case pubsub.Event[skills.Event]:
96 return envelope(pubsub.PayloadTypeSkillsEvent, pubsub.Event[proto.SkillsEvent]{
99 return envelope(pubsub.PayloadTypeSkillsEvent, pubsub.Event[proto.SkillsEvent]{
97 Type: e.Type,
100 Type: e.Type,
@@ -147,6 +150,29 @@ func sessionToProto(s session.Session) proto.Session {
147 }
150 }
148}
151}
149
152
153+// isSessionBusy reports whether the given workspace has an in-flight
154+// agent run for sessionID. It tolerates a nil workspace (treating it as
155+// "not busy") so REST handlers can pass GetWorkspace's result through
156+// unconditionally — the workspace lookup error is already surfaced by
157+// the prior ListSessions/GetSession call when relevant.
158+func isSessionBusy(ws *backend.Workspace, sessionID string) bool {
159+ if ws == nil || ws.App == nil || ws.AgentCoordinator == nil {
160+ return false
161+ }
162+ return ws.AgentCoordinator.IsSessionBusy(sessionID)
163+}
164+
165+// attachedClients returns the number of clients currently viewing
166+// sessionID in ws. Hold-only clients (streams == 0) do not contribute.
167+// A nil workspace is treated as zero so handlers can pass GetWorkspace's
168+// result through without an extra guard.
169+func attachedClients(ws *backend.Workspace, sessionID string) int {
170+ if ws == nil {
171+ return 0
172+ }
173+ return ws.AttachedClientsForSession(sessionID)
174+}
175+
150func todosToProto(todos []session.Todo) []proto.Todo {
176func todosToProto(todos []session.Todo) []proto.Todo {
151 if len(todos) == 0 {
177 if len(todos) == 0 {
152 return nil
178 return nil
internal/server/multiclient_test.gonew file+230
@@ -0,0 +1,230 @@
1+package server
2+
3+import (
4+ "bytes"
5+ "context"
6+ "encoding/json"
7+ "net/http"
8+ "net/http/httptest"
9+ "testing"
10+
11+ "github.com/charmbracelet/crush/internal/backend"
12+ "github.com/charmbracelet/crush/internal/proto"
13+ "github.com/google/uuid"
14+ "github.com/stretchr/testify/require"
15+)
16+
17+// installSyntheticWorkspace creates a synthetic [backend.Workspace]
18+// registered with the controller's backend, suitable for handler-level
19+// tests that do not need a real [app.App]. The workspace's ID is a
20+// fresh UUID and its path is a tempdir; teardown is the caller's
21+// responsibility (handlers should not rely on synthetic workspaces
22+// disappearing automatically).
23+func installSyntheticWorkspace(t *testing.T, c *controllerV1) *backend.Workspace {
24+ t.Helper()
25+ ws := &backend.Workspace{
26+ ID: uuid.New().String(),
27+ Path: t.TempDir(),
28+ }
29+ backend.InsertWorkspaceForTest(c.backend, ws)
30+ return ws
31+}
32+
33+// newTestController builds a controllerV1 around a backend without a
34+// real config store, suitable for handler-level 400 tests.
35+func newTestController() *controllerV1 {
36+ s := &Server{}
37+ s.backend = backend.New(context.Background(), nil, nil)
38+ return &controllerV1{backend: s.backend, server: s}
39+}
40+
41+func TestPostWorkspaces_RejectsMissingClientID(t *testing.T) {
42+ t.Parallel()
43+ c := newTestController()
44+
45+ body, err := json.Marshal(proto.Workspace{Path: t.TempDir()})
46+ require.NoError(t, err)
47+ req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/v1/workspaces", bytes.NewReader(body))
48+ req.Header.Set("Content-Type", "application/json")
49+ rec := httptest.NewRecorder()
50+
51+ c.handlePostWorkspaces(rec, req)
52+
53+ require.Equal(t, http.StatusBadRequest, rec.Code)
54+ var perr proto.Error
55+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &perr))
56+ require.Contains(t, perr.Message, "client_id")
57+}
58+
59+func TestPostWorkspaces_RejectsMalformedClientID(t *testing.T) {
60+ t.Parallel()
61+ c := newTestController()
62+
63+ body, err := json.Marshal(proto.Workspace{Path: t.TempDir(), ClientID: "not-a-uuid"})
64+ require.NoError(t, err)
65+ req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/v1/workspaces", bytes.NewReader(body))
66+ req.Header.Set("Content-Type", "application/json")
67+ rec := httptest.NewRecorder()
68+
69+ c.handlePostWorkspaces(rec, req)
70+
71+ require.Equal(t, http.StatusBadRequest, rec.Code)
72+}
73+
74+func TestDeleteWorkspace_RejectsMissingClientID(t *testing.T) {
75+ t.Parallel()
76+ c := newTestController()
77+
78+ req := httptest.NewRequestWithContext(t.Context(), http.MethodDelete, "/v1/workspaces/abc", nil)
79+ req.SetPathValue("id", "abc")
80+ rec := httptest.NewRecorder()
81+
82+ c.handleDeleteWorkspaces(rec, req)
83+
84+ require.Equal(t, http.StatusBadRequest, rec.Code)
85+}
86+
87+func TestDeleteWorkspace_RejectsMalformedClientID(t *testing.T) {
88+ t.Parallel()
89+ c := newTestController()
90+
91+ req := httptest.NewRequestWithContext(t.Context(), http.MethodDelete, "/v1/workspaces/abc?client_id=nope", nil)
92+ req.SetPathValue("id", "abc")
93+ rec := httptest.NewRecorder()
94+
95+ c.handleDeleteWorkspaces(rec, req)
96+
97+ require.Equal(t, http.StatusBadRequest, rec.Code)
98+}
99+
100+func TestSubscribeEvents_RejectsMissingClientID(t *testing.T) {
101+ t.Parallel()
102+ c := newTestController()
103+
104+ req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/v1/workspaces/abc/events", nil)
105+ req.SetPathValue("id", "abc")
106+ rec := httptest.NewRecorder()
107+
108+ c.handleGetWorkspaceEvents(rec, req)
109+
110+ require.Equal(t, http.StatusBadRequest, rec.Code)
111+}
112+
113+func TestSubscribeEvents_RejectsMalformedClientID(t *testing.T) {
114+ t.Parallel()
115+ c := newTestController()
116+
117+ req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/v1/workspaces/abc/events?client_id=nope", nil)
118+ req.SetPathValue("id", "abc")
119+ rec := httptest.NewRecorder()
120+
121+ c.handleGetWorkspaceEvents(rec, req)
122+
123+ require.Equal(t, http.StatusBadRequest, rec.Code)
124+}
125+
126+// postCurrentSession is a small helper that POSTs the JSON body to
127+// /v1/workspaces/{id}/current-session?client_id=cid and returns the
128+// recorder. It does not require a real listener.
129+func postCurrentSession(t *testing.T, c *controllerV1, wsID, clientID, sessionID string) *httptest.ResponseRecorder {
130+ t.Helper()
131+ body, err := json.Marshal(proto.CurrentSession{SessionID: sessionID})
132+ require.NoError(t, err)
133+ url := "/v1/workspaces/" + wsID + "/current-session"
134+ if clientID != "" {
135+ url += "?client_id=" + clientID
136+ }
137+ req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, url, bytes.NewReader(body))
138+ req.SetPathValue("id", wsID)
139+ req.Header.Set("Content-Type", "application/json")
140+ rec := httptest.NewRecorder()
141+ c.handlePostWorkspaceCurrentSession(rec, req)
142+ return rec
143+}
144+
145+func TestPostCurrentSession_RejectsMissingClientID(t *testing.T) {
146+ t.Parallel()
147+ c := newTestController()
148+
149+ body, err := json.Marshal(proto.CurrentSession{SessionID: "S1"})
150+ require.NoError(t, err)
151+ req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/v1/workspaces/abc/current-session", bytes.NewReader(body))
152+ req.SetPathValue("id", "abc")
153+ req.Header.Set("Content-Type", "application/json")
154+ rec := httptest.NewRecorder()
155+
156+ c.handlePostWorkspaceCurrentSession(rec, req)
157+
158+ require.Equal(t, http.StatusBadRequest, rec.Code)
159+}
160+
161+func TestPostCurrentSession_RejectsMalformedClientID(t *testing.T) {
162+ t.Parallel()
163+ c := newTestController()
164+
165+ rec := postCurrentSession(t, c, "abc", "not-a-uuid", "S1")
166+ require.Equal(t, http.StatusBadRequest, rec.Code)
167+}
168+
169+func TestPostCurrentSession_RejectsBadBody(t *testing.T) {
170+ t.Parallel()
171+ c := newTestController()
172+
173+ cid := uuid.New().String()
174+ url := "/v1/workspaces/abc/current-session?client_id=" + cid
175+ req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, url, bytes.NewReader([]byte("not-json")))
176+ req.SetPathValue("id", "abc")
177+ req.Header.Set("Content-Type", "application/json")
178+ rec := httptest.NewRecorder()
179+
180+ c.handlePostWorkspaceCurrentSession(rec, req)
181+
182+ require.Equal(t, http.StatusBadRequest, rec.Code)
183+}
184+
185+func TestPostCurrentSession_UnknownWorkspace(t *testing.T) {
186+ t.Parallel()
187+ c := newTestController()
188+
189+ rec := postCurrentSession(t, c, uuid.New().String(), uuid.New().String(), "S1")
190+ require.Equal(t, http.StatusNotFound, rec.Code)
191+}
192+
193+func TestPostCurrentSession_UnknownClient(t *testing.T) {
194+ t.Parallel()
195+ c := newTestController()
196+ ws := installSyntheticWorkspace(t, c)
197+
198+ rec := postCurrentSession(t, c, ws.ID, uuid.New().String(), "S1")
199+ require.Equal(t, http.StatusNotFound, rec.Code)
200+}
201+
202+func TestPostCurrentSession_HoldOnly(t *testing.T) {
203+ t.Parallel()
204+ c := newTestController()
205+ ws := installSyntheticWorkspace(t, c)
206+
207+ cid := uuid.New().String()
208+ require.NoError(t, backend.RegisterClientForTesting(c.backend, ws, cid))
209+ t.Cleanup(func() { _ = c.backend.DeleteWorkspace(ws.ID, cid) })
210+
211+ rec := postCurrentSession(t, c, ws.ID, cid, "S1")
212+ require.Equal(t, http.StatusNotFound, rec.Code, "hold-only client must be rejected")
213+}
214+
215+func TestPostCurrentSession_AttachedClientSucceeds(t *testing.T) {
216+ t.Parallel()
217+ c := newTestController()
218+ ws := installSyntheticWorkspace(t, c)
219+
220+ cid := uuid.New().String()
221+ require.NoError(t, c.backend.AttachClient(ws.ID, cid))
222+ t.Cleanup(func() { c.backend.DetachClient(ws.ID, cid) })
223+
224+ rec := postCurrentSession(t, c, ws.ID, cid, "S1")
225+ require.Equal(t, http.StatusOK, rec.Code)
226+
227+ // Clearing also returns 200.
228+ rec = postCurrentSession(t, c, ws.ID, cid, "")
229+ require.Equal(t, http.StatusOK, rec.Code)
230+}
internal/server/proto.go+100−7
@@ -9,6 +9,7 @@ import (
9 "github.com/charmbracelet/crush/internal/backend"
9 "github.com/charmbracelet/crush/internal/backend"
10 "github.com/charmbracelet/crush/internal/proto"
10 "github.com/charmbracelet/crush/internal/proto"
11 "github.com/charmbracelet/crush/internal/session"
11 "github.com/charmbracelet/crush/internal/session"
12+ "github.com/google/uuid"
12)
13)
13
14
14type controllerV1 struct {
15type controllerV1 struct {
@@ -133,6 +134,56 @@ func (c *controllerV1) handlePostWorkspaces(w http.ResponseWriter, r *http.Reque
133 jsonEncode(w, result)
134 jsonEncode(w, result)
134}
135}
135
136
137+// requireClientID reads the client_id query parameter and validates it
138+// as a UUID. On failure it writes a 400 and returns false.
139+func (c *controllerV1) requireClientID(w http.ResponseWriter, r *http.Request) (string, bool) {
140+ cid := r.URL.Query().Get("client_id")
141+ if cid == "" {
142+ c.server.logError(r, "Missing client_id query parameter")
143+ jsonError(w, http.StatusBadRequest, "client_id is required")
144+ return "", false
145+ }
146+ if _, err := uuid.Parse(cid); err != nil {
147+ c.server.logError(r, "Invalid client_id", "error", err)
148+ jsonError(w, http.StatusBadRequest, "client_id is not a valid UUID")
149+ return "", false
150+ }
151+ return cid, true
152+}
153+
154+// handlePostWorkspaceCurrentSession records the calling client's
155+// current session selection for the workspace. An empty session_id
156+// clears the entry (e.g. the client is on the landing screen).
157+//
158+// @Summary Set current session for a client
159+// @Tags workspaces
160+// @Accept json
161+// @Produce json
162+// @Param id path string true "Workspace ID"
163+// @Param client_id query string true "Client ID (UUID)"
164+// @Param request body proto.CurrentSession true "Current session selection"
165+// @Success 200
166+// @Failure 400 {object} proto.Error
167+// @Failure 404 {object} proto.Error
168+// @Router /workspaces/{id}/current-session [post]
169+func (c *controllerV1) handlePostWorkspaceCurrentSession(w http.ResponseWriter, r *http.Request) {
170+ id := r.PathValue("id")
171+ clientID, ok := c.requireClientID(w, r)
172+ if !ok {
173+ return
174+ }
175+ var req proto.CurrentSession
176+ if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
177+ c.server.logError(r, "Failed to decode request", "error", err)
178+ jsonError(w, http.StatusBadRequest, "failed to decode request")
179+ return
180+ }
181+ if err := c.backend.SetCurrentSession(id, clientID, req.SessionID); err != nil {
182+ c.handleError(w, r, err)
183+ return
184+ }
185+}
186+
136// handleDeleteWorkspaces deletes a workspace.
187// handleDeleteWorkspaces deletes a workspace.
137//
188//
138// @Summary Delete workspace
189// @Summary Delete workspace
@@ -143,7 +194,14 @@ func (c *controllerV1) handlePostWorkspaces(w http.ResponseWriter, r *http.Reque
143// @Router /workspaces/{id} [delete]
194// @Router /workspaces/{id} [delete]
144func (c *controllerV1) handleDeleteWorkspaces(w http.ResponseWriter, r *http.Request) {
195func (c *controllerV1) handleDeleteWorkspaces(w http.ResponseWriter, r *http.Request) {
145 id := r.PathValue("id")
196 id := r.PathValue("id")
197+ clientID, ok := c.requireClientID(w, r)
198+ if !ok {
199+ return
200+ }
146− c.backend.DeleteWorkspace(id)
201+ if err := c.backend.DeleteWorkspace(id, clientID); err != nil {
202+ c.handleError(w, r, err)
203+ return
204+ }
147}
205}
148
206
149// handleGetWorkspaceConfig returns workspace configuration.
207// handleGetWorkspaceConfig returns workspace configuration.
@@ -199,6 +257,15 @@ func (c *controllerV1) handleGetWorkspaceProviders(w http.ResponseWriter, r *htt
199func (c *controllerV1) handleGetWorkspaceEvents(w http.ResponseWriter, r *http.Request) {
257func (c *controllerV1) handleGetWorkspaceEvents(w http.ResponseWriter, r *http.Request) {
200 flusher := http.NewResponseController(w)
258 flusher := http.NewResponseController(w)
201 id := r.PathValue("id")
259 id := r.PathValue("id")
260+ clientID, ok := c.requireClientID(w, r)
261+ if !ok {
262+ return
263+ }
264+ if err := c.backend.AttachClient(id, clientID); err != nil {
265+ c.handleError(w, r, err)
266+ return
267+ }
268+ defer c.backend.DetachClient(id, clientID)
202 events, err := c.backend.SubscribeEvents(r.Context(), id)
269 events, err := c.backend.SubscribeEvents(r.Context(), id)
203 if err != nil {
270 if err != nil {
204 c.handleError(w, r, err)
271 c.handleError(w, r, err)
@@ -208,6 +275,12 @@ func (c *controllerV1) handleGetWorkspaceEvents(w http.ResponseWriter, r *http.R
208 w.Header().Set("Content-Type", "text/event-stream")
275 w.Header().Set("Content-Type", "text/event-stream")
209 w.Header().Set("Cache-Control", "no-cache")
276 w.Header().Set("Cache-Control", "no-cache")
210 w.Header().Set("Connection", "keep-alive")
277 w.Header().Set("Connection", "keep-alive")
278+ // Flush headers immediately so clients see the 200 response
279+ // before any events arrive. Without this, a quiet workspace
280+ // keeps the client's SubscribeEvents call blocked on the
281+ // initial RoundTrip.
282+ w.WriteHeader(http.StatusOK)
283+ flusher.Flush()
211
284
212 for {
285 for {
213 select {
286 select {
@@ -303,9 +376,12 @@ func (c *controllerV1) handleGetWorkspaceSessions(w http.ResponseWriter, r *http
303 c.handleError(w, r, err)
376 c.handleError(w, r, err)
304 return
377 return
305 }
378 }
379+ ws, _ := c.backend.GetWorkspace(id)
306 result := make([]proto.Session, len(sessions))
380 result := make([]proto.Session, len(sessions))
307 for i, s := range sessions {
381 for i, s := range sessions {
308 result[i] = sessionToProto(s)
382 result[i] = sessionToProto(s)
383+ result[i].IsBusy = isSessionBusy(ws, s.ID)
384+ result[i].AttachedClients = attachedClients(ws, s.ID)
309 }
385 }
310 jsonEncode(w, result)
386 jsonEncode(w, result)
311}
387}
@@ -338,7 +414,11 @@ func (c *controllerV1) handlePostWorkspaceSessions(w http.ResponseWriter, r *htt
338 c.handleError(w, r, err)
414 c.handleError(w, r, err)
339 return
415 return
340 }
416 }
417+ ws, _ := c.backend.GetWorkspace(id)
418+ out := sessionToProto(sess)
419+ out.IsBusy = isSessionBusy(ws, sess.ID)
420+ out.AttachedClients = attachedClients(ws, sess.ID)
341− jsonEncode(w, sessionToProto(sess))
421+ jsonEncode(w, out)
342}
422}
343
423
344// handleGetWorkspaceSession returns a single session.
424// handleGetWorkspaceSession returns a single session.
@@ -360,7 +440,11 @@ func (c *controllerV1) handleGetWorkspaceSession(w http.ResponseWriter, r *http.
360 c.handleError(w, r, err)
440 c.handleError(w, r, err)
361 return
441 return
362 }
442 }
443+ ws, _ := c.backend.GetWorkspace(id)
444+ out := sessionToProto(sess)
445+ out.IsBusy = isSessionBusy(ws, sess.ID)
446+ out.AttachedClients = attachedClients(ws, sess.ID)
363− jsonEncode(w, sessionToProto(sess))
447+ jsonEncode(w, out)
364}
448}
365
449
366// handleGetWorkspaceSessionHistory returns the history for a session.
450// handleGetWorkspaceSessionHistory returns the history for a session.
@@ -436,7 +520,11 @@ func (c *controllerV1) handlePutWorkspaceSession(w http.ResponseWriter, r *http.
436 c.handleError(w, r, err)
520 c.handleError(w, r, err)
437 return
521 return
438 }
522 }
523+ ws, _ := c.backend.GetWorkspace(id)
524+ out := sessionToProto(saved)
525+ out.IsBusy = isSessionBusy(ws, saved.ID)
526+ out.AttachedClients = attachedClients(ws, saved.ID)
439− jsonEncode(w, sessionToProto(saved))
527+ jsonEncode(w, out)
440}
528}
441
529
442// handleDeleteWorkspaceSession deletes a session.
530// handleDeleteWorkspaceSession deletes a session.
@@ -864,7 +952,7 @@ func (c *controllerV1) handleGetWorkspaceAgentDefaultSmallModel(w http.ResponseW
864// @Accept json
952// @Accept json
865// @Param id path string true "Workspace ID"
953// @Param id path string true "Workspace ID"
866// @Param request body proto.PermissionGrant true "Permission grant"
954// @Param request body proto.PermissionGrant true "Permission grant"
867−// @Success 200
955+// @Success 200 {object} proto.PermissionGrantResponse
868// @Failure 400 {object} proto.Error
956// @Failure 400 {object} proto.Error
869// @Failure 404 {object} proto.Error
957// @Failure 404 {object} proto.Error
870// @Failure 500 {object} proto.Error
958// @Failure 500 {object} proto.Error
@@ -879,11 +967,12 @@ func (c *controllerV1) handlePostWorkspacePermissionsGrant(w http.ResponseWriter
879 return
967 return
880 }
968 }
881
969
882− if err := c.backend.GrantPermission(id, req); err != nil {
970+ resolved, err := c.backend.GrantPermission(id, req)
971+ if err != nil {
883 c.handleError(w, r, err)
972 c.handleError(w, r, err)
884 return
973 return
885 }
974 }
886− w.WriteHeader(http.StatusOK)
975+ jsonEncode(w, proto.PermissionGrantResponse{Resolved: resolved})
887}
976}
888
977
889// handlePostWorkspacePermissionsSkip sets whether to skip permission prompts.
978// handlePostWorkspacePermissionsSkip sets whether to skip permission prompts.
@@ -951,6 +1040,10 @@ func (c *controllerV1) handleError(w http.ResponseWriter, r *http.Request, err e
951 status = http.StatusBadRequest
1040 status = http.StatusBadRequest
952 case errors.Is(err, backend.ErrUnknownCommand):
1041 case errors.Is(err, backend.ErrUnknownCommand):
953 status = http.StatusBadRequest
1042 status = http.StatusBadRequest
1043+ case errors.Is(err, backend.ErrInvalidClientID):
1044+ status = http.StatusBadRequest
1045+ case errors.Is(err, backend.ErrClientNotAttached):
1046+ status = http.StatusNotFound
954 }
1047 }
955 c.server.logError(r, err.Error())
1048 c.server.logError(r, err.Error())
956 jsonError(w, status, err.Error())
1049 jsonError(w, status, err.Error())
internal/server/server.go+19−4
@@ -100,7 +100,18 @@ func NewServer(cfg *config.ConfigStore, network, address string) *Server {
100 }
100 }
101 }()
101 }()
102 })
102 })
103+ s.installHandler()
104+ if network == "tcp" {
105+ s.h.Addr = address
106+ }
107+ return s
108+}
103
109
110+// installHandler builds the protocol/router around s.backend and
111+// assigns the resulting http.Server to s.h. Extracted from
112+// [NewServer] so test harnesses can wire a Server around a
113+// pre-constructed backend.
114+func (s *Server) installHandler() {
104 var p http.Protocols
115 var p http.Protocols
105 p.SetHTTP1(true)
116 p.SetHTTP1(true)
106 p.SetUnencryptedHTTP2(true)
117 p.SetUnencryptedHTTP2(true)
@@ -113,6 +124,7 @@ func NewServer(cfg *config.ConfigStore, network, address string) *Server {
113 mux.HandleFunc("GET /v1/workspaces", c.handleGetWorkspaces)
124 mux.HandleFunc("GET /v1/workspaces", c.handleGetWorkspaces)
114 mux.HandleFunc("POST /v1/workspaces", c.handlePostWorkspaces)
125 mux.HandleFunc("POST /v1/workspaces", c.handlePostWorkspaces)
115 mux.HandleFunc("DELETE /v1/workspaces/{id}", c.handleDeleteWorkspaces)
126 mux.HandleFunc("DELETE /v1/workspaces/{id}", c.handleDeleteWorkspaces)
127+ mux.HandleFunc("POST /v1/workspaces/{id}/current-session", c.handlePostWorkspaceCurrentSession)
116 mux.HandleFunc("GET /v1/workspaces/{id}", c.handleGetWorkspace)
128 mux.HandleFunc("GET /v1/workspaces/{id}", c.handleGetWorkspace)
117 mux.HandleFunc("GET /v1/workspaces/{id}/config", c.handleGetWorkspaceConfig)
129 mux.HandleFunc("GET /v1/workspaces/{id}/config", c.handleGetWorkspaceConfig)
118 mux.HandleFunc("GET /v1/workspaces/{id}/events", c.handleGetWorkspaceEvents)
130 mux.HandleFunc("GET /v1/workspaces/{id}/events", c.handleGetWorkspaceEvents)
@@ -172,10 +184,13 @@ func NewServer(cfg *config.ConfigStore, network, address string) *Server {
172 Protocols: &p,
184 Protocols: &p,
173 Handler: s.recoverHandler(s.loggingHandler(mux)),
185 Handler: s.recoverHandler(s.loggingHandler(mux)),
174 }
186 }
175− if network == "tcp" {
176− s.h.Addr = address
177− }
187+}
188+
189+// Handler returns the server's HTTP handler. Exposed so test harnesses
190+// can wrap it in an httptest.Server without going through the
191+// production listener setup.
192+func (s *Server) Handler() http.Handler {
178− return s
193+ return s.h.Handler
179}
194}
180
195
181// Serve accepts incoming connections on the listener.
196// Serve accepts incoming connections on the listener.
internal/server/sessions_isbusy_test.gonew file+325
@@ -0,0 +1,325 @@
1+package server
2+
3+import (
4+ "context"
5+ "encoding/json"
6+ "errors"
7+ "net/http"
8+ "net/http/httptest"
9+ "testing"
10+
11+ "charm.land/fantasy"
12+ "github.com/charmbracelet/crush/internal/agent"
13+ "github.com/charmbracelet/crush/internal/app"
14+ "github.com/charmbracelet/crush/internal/backend"
15+ "github.com/charmbracelet/crush/internal/message"
16+ "github.com/charmbracelet/crush/internal/proto"
17+ "github.com/charmbracelet/crush/internal/session"
18+ "github.com/google/uuid"
19+ "github.com/stretchr/testify/require"
20+)
21+
22+// stubCoordinator is a minimal agent.Coordinator that only reports
23+// per-session busy state. Every other method returns a zero value so
24+// the type satisfies the interface without dragging in the full
25+// coordinator dependency graph.
26+type stubCoordinator struct {
27+ busy map[string]bool
28+}
29+
30+func (s *stubCoordinator) Run(ctx context.Context, sessionID, prompt string, attachments ...message.Attachment) (*fantasy.AgentResult, error) {
31+ return nil, nil
32+}
33+func (s *stubCoordinator) Cancel(string) {}
34+func (s *stubCoordinator) CancelAll() {}
35+func (s *stubCoordinator) IsBusy() bool { return false }
36+func (s *stubCoordinator) IsSessionBusy(id string) bool {
37+ return s.busy[id]
38+}
39+func (s *stubCoordinator) QueuedPrompts(string) int { return 0 }
40+func (s *stubCoordinator) QueuedPromptsList(string) []string { return nil }
41+func (s *stubCoordinator) ClearQueue(string) {}
42+func (s *stubCoordinator) Summarize(context.Context, string) error {
43+ return nil
44+}
45+func (s *stubCoordinator) Model() agent.Model { return agent.Model{} }
46+func (s *stubCoordinator) UpdateModels(context.Context) error { return nil }
47+
48+// stubSessions is a minimal session.Service that returns a fixed list
49+// (and supports Get by ID). All other methods return zero values; the
50+// IsBusy tests do not exercise them.
51+type stubSessions struct {
52+ session.Service // embed nil to inherit the unexported broker methods
53+ all []session.Session
54+}
55+
56+func (s *stubSessions) List(context.Context) ([]session.Session, error) {
57+ return s.all, nil
58+}
59+
60+func (s *stubSessions) Get(_ context.Context, id string) (session.Session, error) {
61+ for _, sess := range s.all {
62+ if sess.ID == id {
63+ return sess, nil
64+ }
65+ }
66+ return session.Session{}, errors.New("not found")
67+}
68+
69+// buildBusyWorkspace returns a controller wired to a backend that owns
70+// a single workspace whose AgentCoordinator reports the named session
71+// as busy.
72+func buildBusyWorkspace(t *testing.T, sessionID string, busy bool) (*controllerV1, string) {
73+ t.Helper()
74+
75+ b := backend.New(context.Background(), nil, nil)
76+ wsID := uuid.New().String()
77+ coord := &stubCoordinator{busy: map[string]bool{sessionID: busy}}
78+ a := &app.App{AgentCoordinator: coord}
79+ a.Sessions = &stubSessions{all: []session.Session{{ID: sessionID, Title: "t"}}}
80+
81+ ws := &backend.Workspace{
82+ ID: wsID,
83+ Path: t.TempDir(),
84+ App: a,
85+ }
86+ backend.InsertWorkspaceForTest(b, ws)
87+
88+ s := &Server{backend: b}
89+ return &controllerV1{backend: b, server: s}, wsID
90+}
91+
92+func TestSessionListIncludesIsBusy(t *testing.T) {
93+ t.Parallel()
94+ const sid = "s-busy"
95+ c, wsID := buildBusyWorkspace(t, sid, true)
96+
97+ req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/v1/workspaces/"+wsID+"/sessions", nil)
98+ req.SetPathValue("id", wsID)
99+ rec := httptest.NewRecorder()
100+ c.handleGetWorkspaceSessions(rec, req)
101+ require.Equal(t, http.StatusOK, rec.Code)
102+
103+ var got []proto.Session
104+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got))
105+ require.Len(t, got, 1)
106+ require.Equal(t, sid, got[0].ID)
107+ require.True(t, got[0].IsBusy, "expected IsBusy=true for the busy session")
108+}
109+
110+func TestSessionListIdleSessionIsNotBusy(t *testing.T) {
111+ t.Parallel()
112+ const sid = "s-idle"
113+ c, wsID := buildBusyWorkspace(t, sid, false)
114+
115+ req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/v1/workspaces/"+wsID+"/sessions", nil)
116+ req.SetPathValue("id", wsID)
117+ rec := httptest.NewRecorder()
118+ c.handleGetWorkspaceSessions(rec, req)
119+ require.Equal(t, http.StatusOK, rec.Code)
120+
121+ var got []proto.Session
122+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got))
123+ require.Len(t, got, 1)
124+ require.False(t, got[0].IsBusy, "expected IsBusy=false for idle session")
125+}
126+
127+func TestSessionGetIncludesIsBusy(t *testing.T) {
128+ t.Parallel()
129+ const sid = "s-busy"
130+ c, wsID := buildBusyWorkspace(t, sid, true)
131+
132+ req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/v1/workspaces/"+wsID+"/sessions/"+sid, nil)
133+ req.SetPathValue("id", wsID)
134+ req.SetPathValue("sid", sid)
135+ rec := httptest.NewRecorder()
136+ c.handleGetWorkspaceSession(rec, req)
137+ require.Equal(t, http.StatusOK, rec.Code)
138+
139+ var got proto.Session
140+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got))
141+ require.Equal(t, sid, got.ID)
142+ require.True(t, got.IsBusy)
143+}
144+
145+// TestIsSessionBusyNilSafe verifies the helper tolerates a missing
146+// workspace, app, or coordinator — phase A handlers rely on this so
147+// they can pass GetWorkspace's result through without an extra guard.
148+func TestIsSessionBusyNilSafe(t *testing.T) {
149+ t.Parallel()
150+
151+ require.False(t, isSessionBusy(nil, "x"))
152+ require.False(t, isSessionBusy(&backend.Workspace{}, "x"))
153+ require.False(t, isSessionBusy(&backend.Workspace{App: &app.App{}}, "x"))
154+}
155+
156+// TestProtoSessionIsBusyBackwardCompat verifies older consumers that
157+// unmarshal proto.Session without knowing about IsBusy still succeed
158+// and ignore the new field harmlessly.
159+func TestProtoSessionIsBusyBackwardCompat(t *testing.T) {
160+ t.Parallel()
161+
162+ wire := proto.Session{ID: "s1", Title: "t", IsBusy: true}
163+ raw, err := json.Marshal(wire)
164+ require.NoError(t, err)
165+
166+ // Old client shape: same struct minus IsBusy. We model this by
167+ // unmarshaling into a struct that doesn't declare the field.
168+ type oldSession struct {
169+ ID string `json:"id"`
170+ Title string `json:"title"`
171+ }
172+ var old oldSession
173+ require.NoError(t, json.Unmarshal(raw, &old))
174+ require.Equal(t, "s1", old.ID)
175+ require.Equal(t, "t", old.Title)
176+}
177+
178+// buildMultiSessionWorkspace returns a controller wired to a backend
179+// that owns a workspace with the given session IDs. Used to exercise
180+// AttachedClients counts across more than one session.
181+func buildMultiSessionWorkspace(t *testing.T, sessionIDs ...string) (*controllerV1, *backend.Workspace) {
182+ t.Helper()
183+
184+ b := backend.New(context.Background(), nil, nil)
185+ a := &app.App{AgentCoordinator: &stubCoordinator{}}
186+ sessions := make([]session.Session, len(sessionIDs))
187+ for i, sid := range sessionIDs {
188+ sessions[i] = session.Session{ID: sid, Title: sid}
189+ }
190+ a.Sessions = &stubSessions{all: sessions}
191+
192+ ws := &backend.Workspace{
193+ ID: uuid.New().String(),
194+ Path: t.TempDir(),
195+ App: a,
196+ }
197+ backend.InsertWorkspaceForTest(b, ws)
198+ // Synthetic workspaces have an incomplete App; bypass the
199+ // default teardown to avoid panics when the last client detaches.
200+ backend.SetWorkspaceShutdownFnForTest(ws, func() {})
201+
202+ s := &Server{backend: b}
203+ return &controllerV1{backend: b, server: s}, ws
204+}
205+
206+// listSessions invokes handleGetWorkspaceSessions and returns the
207+// decoded response so tests can assert per-session counts.
208+func listSessions(t *testing.T, c *controllerV1, wsID string) []proto.Session {
209+ t.Helper()
210+ req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/v1/workspaces/"+wsID+"/sessions", nil)
211+ req.SetPathValue("id", wsID)
212+ rec := httptest.NewRecorder()
213+ c.handleGetWorkspaceSessions(rec, req)
214+ require.Equal(t, http.StatusOK, rec.Code)
215+ var got []proto.Session
216+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got))
217+ return got
218+}
219+
220+func countsBySessionID(sessions []proto.Session) map[string]int {
221+ out := make(map[string]int, len(sessions))
222+ for _, s := range sessions {
223+ out[s.ID] = s.AttachedClients
224+ }
225+ return out
226+}
227+
228+// TestSessionListIncludesAttachedClients walks two sessions through
229+// the same lifecycle covered by TestAttachedClients_BasicLifecycle in
230+// the backend package, but observed at the handler boundary.
231+func TestSessionListIncludesAttachedClients(t *testing.T) {
232+ t.Parallel()
233+ c, ws := buildMultiSessionWorkspace(t, "S1", "S2")
234+
235+ // No attached clients yet.
236+ counts := countsBySessionID(listSessions(t, c, ws.ID))
237+ require.Equal(t, 0, counts["S1"])
238+ require.Equal(t, 0, counts["S2"])
239+
240+ // Attach A, set to S1: S1=1.
241+ cidA := uuid.New().String()
242+ require.NoError(t, c.backend.AttachClient(ws.ID, cidA))
243+ t.Cleanup(func() { c.backend.DetachClient(ws.ID, cidA) })
244+ require.NoError(t, c.backend.SetCurrentSession(ws.ID, cidA, "S1"))
245+ counts = countsBySessionID(listSessions(t, c, ws.ID))
246+ require.Equal(t, 1, counts["S1"])
247+ require.Equal(t, 0, counts["S2"])
248+
249+ // Attach B, set to S1: S1=2.
250+ cidB := uuid.New().String()
251+ require.NoError(t, c.backend.AttachClient(ws.ID, cidB))
252+ require.NoError(t, c.backend.SetCurrentSession(ws.ID, cidB, "S1"))
253+ counts = countsBySessionID(listSessions(t, c, ws.ID))
254+ require.Equal(t, 2, counts["S1"])
255+ require.Equal(t, 0, counts["S2"])
256+
257+ // B switches to S2: counts redistribute.
258+ require.NoError(t, c.backend.SetCurrentSession(ws.ID, cidB, "S2"))
259+ counts = countsBySessionID(listSessions(t, c, ws.ID))
260+ require.Equal(t, 1, counts["S1"])
261+ require.Equal(t, 1, counts["S2"])
262+
263+ // B detaches: S2 drops to 0.
264+ c.backend.DetachClient(ws.ID, cidB)
265+ counts = countsBySessionID(listSessions(t, c, ws.ID))
266+ require.Equal(t, 1, counts["S1"])
267+ require.Equal(t, 0, counts["S2"])
268+}
269+
270+// TestSessionListExcludesHoldOnlyClient verifies that a registered
271+// client without an SSE stream (streams == 0) does not contribute to
272+// AttachedClients, even though it has an entry in the workspace's
273+// clients map.
274+func TestSessionListExcludesHoldOnlyClient(t *testing.T) {
275+ t.Parallel()
276+ c, ws := buildMultiSessionWorkspace(t, "S1")
277+
278+ cid := uuid.New().String()
279+ require.NoError(t, backend.RegisterClientForTesting(c.backend, ws, cid))
280+ t.Cleanup(func() { _ = c.backend.DeleteWorkspace(ws.ID, cid) })
281+
282+ counts := countsBySessionID(listSessions(t, c, ws.ID))
283+ require.Equal(t, 0, counts["S1"], "hold-only client must not be counted")
284+}
285+
286+// TestSessionListExcludesUnselectedAttachedClient verifies that a
287+// client with a live SSE stream but no current session
288+// (currentSessionID == "") does not show up under any session's count.
289+func TestSessionListExcludesUnselectedAttachedClient(t *testing.T) {
290+ t.Parallel()
291+ c, ws := buildMultiSessionWorkspace(t, "S1")
292+
293+ cid := uuid.New().String()
294+ require.NoError(t, c.backend.AttachClient(ws.ID, cid))
295+ t.Cleanup(func() { c.backend.DetachClient(ws.ID, cid) })
296+ // Intentionally do NOT call SetCurrentSession.
297+
298+ counts := countsBySessionID(listSessions(t, c, ws.ID))
299+ require.Equal(t, 0, counts["S1"],
300+ "attached client with no current session must not contribute to S1")
301+}
302+
303+// TestSessionGetIncludesAttachedClients verifies the single-session
304+// handler also populates AttachedClients.
305+func TestSessionGetIncludesAttachedClients(t *testing.T) {
306+ t.Parallel()
307+ c, ws := buildMultiSessionWorkspace(t, "S1")
308+
309+ cid := uuid.New().String()
310+ require.NoError(t, c.backend.AttachClient(ws.ID, cid))
311+ t.Cleanup(func() { c.backend.DetachClient(ws.ID, cid) })
312+ require.NoError(t, c.backend.SetCurrentSession(ws.ID, cid, "S1"))
313+
314+ req := httptest.NewRequestWithContext(t.Context(), http.MethodGet,
315+ "/v1/workspaces/"+ws.ID+"/sessions/S1", nil)
316+ req.SetPathValue("id", ws.ID)
317+ req.SetPathValue("sid", "S1")
318+ rec := httptest.NewRecorder()
319+ c.handleGetWorkspaceSession(rec, req)
320+ require.Equal(t, http.StatusOK, rec.Code)
321+
322+ var got proto.Session
323+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got))
324+ require.Equal(t, 1, got.AttachedClients)
325+}
internal/ui/dialog/permissions.go+6
@@ -224,6 +224,12 @@ func (*Permissions) ID() string {
224 return PermissionsID
224 return PermissionsID
225}
225}
226
226
227+// ToolCallID returns the tool call ID associated with this dialog's
228+// permission request.
229+func (p *Permissions) ToolCallID() string {
230+ return p.permission.ToolCallID
231+}
232+
227// HandleMsg implements [Dialog].
233// HandleMsg implements [Dialog].
228func (p *Permissions) HandleMsg(msg tea.Msg) Action {
234func (p *Permissions) HandleMsg(msg tea.Msg) Action {
229 switch msg := msg.(type) {
235 switch msg := msg.(type) {
internal/ui/model/permission_test.gonew file+96
@@ -0,0 +1,96 @@
1+package model
2+
3+import (
4+ "testing"
5+
6+ "github.com/charmbracelet/crush/internal/permission"
7+ "github.com/charmbracelet/crush/internal/ui/dialog"
8+ "github.com/stretchr/testify/require"
9+)
10+
11+// newTestUIForPermissions builds a UI with a chat, dialog overlay, and
12+// common context sufficient to exercise handlePermissionNotification.
13+func newTestUIForPermissions() *UI {
14+ u := newTestUI()
15+ u.dialog = dialog.NewOverlay()
16+ return u
17+}
18+
19+func TestHandlePermissionNotification_RemoteGrantClosesDialog(t *testing.T) {
20+ t.Parallel()
21+
22+ u := newTestUIForPermissions()
23+ perm := permission.PermissionRequest{
24+ ID: "perm-1",
25+ ToolCallID: "tool-call-X",
26+ ToolName: "bash",
27+ }
28+ u.dialog.OpenDialog(dialog.NewPermissions(u.com, perm))
29+ require.True(t, u.dialog.ContainsDialog(dialog.PermissionsID))
30+
31+ u.handlePermissionNotification(permission.PermissionNotification{
32+ ToolCallID: "tool-call-X",
33+ Granted: true,
34+ })
35+
36+ require.False(t, u.dialog.ContainsDialog(dialog.PermissionsID),
37+ "granted notification should close matching permissions dialog")
38+}
39+
40+func TestHandlePermissionNotification_RemoteDenyClosesDialog(t *testing.T) {
41+ t.Parallel()
42+
43+ u := newTestUIForPermissions()
44+ perm := permission.PermissionRequest{
45+ ID: "perm-2",
46+ ToolCallID: "tool-call-Y",
47+ }
48+ u.dialog.OpenDialog(dialog.NewPermissions(u.com, perm))
49+
50+ u.handlePermissionNotification(permission.PermissionNotification{
51+ ToolCallID: "tool-call-Y",
52+ Denied: true,
53+ })
54+
55+ require.False(t, u.dialog.ContainsDialog(dialog.PermissionsID),
56+ "denied notification should close matching permissions dialog")
57+}
58+
59+func TestHandlePermissionNotification_InitialPendingDoesNotClose(t *testing.T) {
60+ t.Parallel()
61+
62+ u := newTestUIForPermissions()
63+ perm := permission.PermissionRequest{
64+ ID: "perm-3",
65+ ToolCallID: "tool-call-Z",
66+ }
67+ u.dialog.OpenDialog(dialog.NewPermissions(u.com, perm))
68+
69+ // The initial notification published by permission.Request is
70+ // neither granted nor denied; it must not dismiss the dialog.
71+ u.handlePermissionNotification(permission.PermissionNotification{
72+ ToolCallID: "tool-call-Z",
73+ })
74+
75+ require.True(t, u.dialog.ContainsDialog(dialog.PermissionsID),
76+ "initial pending notification must not close the dialog")
77+}
78+
79+func TestHandlePermissionNotification_DifferentToolCallIDDoesNotClose(t *testing.T) {
80+ t.Parallel()
81+
82+ u := newTestUIForPermissions()
83+ perm := permission.PermissionRequest{
84+ ID: "perm-4",
85+ ToolCallID: "tool-call-A",
86+ }
87+ u.dialog.OpenDialog(dialog.NewPermissions(u.com, perm))
88+
89+ u.handlePermissionNotification(permission.PermissionNotification{
90+ ToolCallID: "tool-call-B",
91+ Granted: true,
92+ })
93+
94+ require.True(t, u.dialog.ContainsDialog(dialog.PermissionsID),
95+ "notification for unrelated tool call must not close the dialog")
96+}
internal/ui/model/session.go+21−1
@@ -64,8 +64,13 @@ type SessionFile struct {
64// the diff statistics (additions and deletions) for each file in the session.
64// the diff statistics (additions and deletions) for each file in the session.
65// It returns a tea.Cmd that, when executed, fetches the session data and
65// It returns a tea.Cmd that, when executed, fetches the session data and
66// returns a sessionFilesLoadedMsg containing the processed session files.
66// returns a sessionFilesLoadedMsg containing the processed session files.
67+//
68+// The returned batch also reports the new current-session selection to
69+// the workspace so the server can update its per-client presence map.
70+// That report is fire-and-forget: errors are logged at debug and the
71+// UI never blocks on the call.
67func (m *UI) loadSession(sessionID string) tea.Cmd {
72func (m *UI) loadSession(sessionID string) tea.Cmd {
68− return func() tea.Msg {
73+ load := func() tea.Msg {
69 session, err := m.com.Workspace.GetSession(context.Background(), sessionID)
74 session, err := m.com.Workspace.GetSession(context.Background(), sessionID)
70 if err != nil {
75 if err != nil {
71 return util.ReportError(err)
76 return util.ReportError(err)
@@ -87,6 +92,21 @@ func (m *UI) loadSession(sessionID string) tea.Cmd {
87 readFiles: readFiles,
92 readFiles: readFiles,
88 }
93 }
89 }
94 }
95+ return tea.Batch(load, m.reportCurrentSession(sessionID))
96+}
97+
98+// reportCurrentSession returns a fire-and-forget tea.Cmd that
99+// informs the workspace which session this client is currently
100+// viewing. Errors are logged at debug only; the call is a hint
101+// for server-side presence tracking, not correctness-critical
102+// state.
103+func (m *UI) reportCurrentSession(sessionID string) tea.Cmd {
104+ return func() tea.Msg {
105+ if err := m.com.Workspace.SetCurrentSession(context.Background(), sessionID); err != nil {
106+ slog.Debug("Failed to report current session", "session_id", sessionID, "error", err)
107+ }
108+ return nil
109+ }
90}
110}
91
111
92func (m *UI) loadSessionFiles(sessionID string) ([]SessionFile, error) {
112func (m *UI) loadSessionFiles(sessionID string) ([]SessionFile, error) {
internal/ui/model/ui.go+18−8
@@ -3523,16 +3523,25 @@ func (m *UI) openPermissionsDialog(perm permission.PermissionRequest) tea.Cmd {
3523
3523
3524// handlePermissionNotification updates tool items when permission state changes.
3524// handlePermissionNotification updates tool items when permission state changes.
3525func (m *UI) handlePermissionNotification(notification permission.PermissionNotification) {
3525func (m *UI) handlePermissionNotification(notification permission.PermissionNotification) {
3526− toolItem := m.chat.MessageItem(notification.ToolCallID)
3526+ if toolItem := m.chat.MessageItem(notification.ToolCallID); toolItem != nil {
3527− if toolItem == nil {
3527+ if permItem, ok := toolItem.(chat.ToolMessageItem); ok {
3528− return
3528+ if notification.Granted {
3529+ permItem.SetStatus(chat.ToolStatusRunning)
3530+ } else {
3531+ permItem.SetStatus(chat.ToolStatusAwaitingPermission)
3532+ }
3533+ }
3529 }
3534 }
3530
3535
3531− if permItem, ok := toolItem.(chat.ToolMessageItem); ok {
3536+ // If this notification reflects a final resolution (granted or denied),
3537+ // dismiss any open permissions dialog whose tool call ID matches. This
3538+ // covers the case where another client resolved the request remotely.
3532− if notification.Granted {
3539+ if !notification.Granted && !notification.Denied {
3533− permItem.SetStatus(chat.ToolStatusRunning)
3540+ return
3534− } else {
3541+ }
3535− permItem.SetStatus(chat.ToolStatusAwaitingPermission)
3542+ if d := m.dialog.Dialog(dialog.PermissionsID); d != nil {
3543+ if perm, ok := d.(*dialog.Permissions); ok && perm.ToolCallID() == notification.ToolCallID {
3544+ m.dialog.CloseDialog(dialog.PermissionsID)
3536 }
3545 }
3537 }
3546 }
3538}
3547}
@@ -3601,6 +3610,7 @@ func (m *UI) newSession() tea.Cmd {
3601 return nil
3610 return nil
3602 },
3611 },
3603 m.loadPromptHistory(),
3612 m.loadPromptHistory(),
3613+ m.reportCurrentSession(""),
3604 )
3614 )
3605}
3615}
3606
3616
internal/workspace/app_workspace.go+13−6
@@ -68,6 +68,13 @@ func (w *AppWorkspace) ParseAgentToolSessionID(sessionID string) (string, string
68 return w.app.Sessions.ParseAgentToolSessionID(sessionID)
68 return w.app.Sessions.ParseAgentToolSessionID(sessionID)
69}
69}
70
70
71+// SetCurrentSession is a no-op in single-client local mode. The
72+// presence concept only matters when multiple clients can share a
73+// workspace via the HTTP server.
74+func (w *AppWorkspace) SetCurrentSession(ctx context.Context, sessionID string) error {
75+ return nil
76+}
77+
71// -- Messages --
78// -- Messages --
72
79
73func (w *AppWorkspace) ListMessages(ctx context.Context, sessionID string) ([]message.Message, error) {
80func (w *AppWorkspace) ListMessages(ctx context.Context, sessionID string) ([]message.Message, error) {
@@ -174,16 +181,16 @@ func (w *AppWorkspace) GetDefaultSmallModel(providerID string) config.SelectedMo
174
181
175// -- Permissions --
182// -- Permissions --
176
183
177−func (w *AppWorkspace) PermissionGrant(perm permission.PermissionRequest) {
184+func (w *AppWorkspace) PermissionGrant(perm permission.PermissionRequest) bool {
178− w.app.Permissions.Grant(perm)
185+ return w.app.Permissions.Grant(perm)
179}
186}
180
187
181−func (w *AppWorkspace) PermissionGrantPersistent(perm permission.PermissionRequest) {
188+func (w *AppWorkspace) PermissionGrantPersistent(perm permission.PermissionRequest) bool {
182− w.app.Permissions.GrantPersistent(perm)
189+ return w.app.Permissions.GrantPersistent(perm)
183}
190}
184
191
185−func (w *AppWorkspace) PermissionDeny(perm permission.PermissionRequest) {
192+func (w *AppWorkspace) PermissionDeny(perm permission.PermissionRequest) bool {
186− w.app.Permissions.Deny(perm)
193+ return w.app.Permissions.Deny(perm)
187}
194}
188
195
189func (w *AppWorkspace) PermissionSkipRequests() bool {
196func (w *AppWorkspace) PermissionSkipRequests() bool {
internal/workspace/client_workspace.go+41−11
@@ -63,7 +63,7 @@ func NewClientWorkspace(c *client.Client, ws proto.Workspace) *ClientWorkspace {
63// refreshWorkspace re-fetches the workspace from the server, updating
63// refreshWorkspace re-fetches the workspace from the server, updating
64// the cached snapshot. Called after config-mutating operations.
64// the cached snapshot. Called after config-mutating operations.
65func (w *ClientWorkspace) refreshWorkspace() {
65func (w *ClientWorkspace) refreshWorkspace() {
66− updated, err := w.client.GetWorkspace(context.Background(), w.ws.ID)
66+ updated, err := w.client.GetWorkspace(context.Background(), w.workspaceID())
67 if err != nil {
67 if err != nil {
68 slog.Error("Failed to refresh workspace", "error", err)
68 slog.Error("Failed to refresh workspace", "error", err)
69 return
69 return
@@ -142,6 +142,14 @@ func (w *ClientWorkspace) ParseAgentToolSessionID(sessionID string) (string, str
142 return parts[0], parts[1], true
142 return parts[0], parts[1], true
143}
143}
144
144
145+// SetCurrentSession reports the session this client is currently
146+// viewing to the server. Empty sessionID clears the entry. Errors
147+// are propagated to the caller; the TUI logs and ignores them since
148+// the presence record is a hint, not correctness-critical state.
149+func (w *ClientWorkspace) SetCurrentSession(ctx context.Context, sessionID string) error {
150+ return w.client.SetCurrentSession(ctx, w.workspaceID(), sessionID)
151+}
152+
145// -- Messages --
153// -- Messages --
146
154
147func (w *ClientWorkspace) ListMessages(ctx context.Context, sessionID string) ([]message.Message, error) {
155func (w *ClientWorkspace) ListMessages(ctx context.Context, sessionID string) ([]message.Message, error) {
@@ -255,8 +263,8 @@ func (w *ClientWorkspace) GetDefaultSmallModel(providerID string) config.Selecte
255
263
256// -- Permissions --
264// -- Permissions --
257
265
258−func (w *ClientWorkspace) PermissionGrant(perm permission.PermissionRequest) {
266+func (w *ClientWorkspace) PermissionGrant(perm permission.PermissionRequest) bool {
259− _ = w.client.GrantPermission(context.Background(), w.workspaceID(), proto.PermissionGrant{
267+ resolved, _ := w.client.GrantPermission(context.Background(), w.workspaceID(), proto.PermissionGrant{
260 Permission: proto.PermissionRequest{
268 Permission: proto.PermissionRequest{
261 ID: perm.ID,
269 ID: perm.ID,
262 SessionID: perm.SessionID,
270 SessionID: perm.SessionID,
@@ -267,12 +275,13 @@ func (w *ClientWorkspace) PermissionGrant(perm permission.PermissionRequest) {
267 Path: perm.Path,
275 Path: perm.Path,
268 Params: perm.Params,
276 Params: perm.Params,
269 },
277 },
270− Action: proto.PermissionAllowForSession,
278+ Action: proto.PermissionAllow,
271 })
279 })
280+ return resolved
272}
281}
273
282
274−func (w *ClientWorkspace) PermissionGrantPersistent(perm permission.PermissionRequest) {
283+func (w *ClientWorkspace) PermissionGrantPersistent(perm permission.PermissionRequest) bool {
275− _ = w.client.GrantPermission(context.Background(), w.workspaceID(), proto.PermissionGrant{
284+ resolved, _ := w.client.GrantPermission(context.Background(), w.workspaceID(), proto.PermissionGrant{
276 Permission: proto.PermissionRequest{
285 Permission: proto.PermissionRequest{
277 ID: perm.ID,
286 ID: perm.ID,
278 SessionID: perm.SessionID,
287 SessionID: perm.SessionID,
@@ -283,12 +292,13 @@ func (w *ClientWorkspace) PermissionGrantPersistent(perm permission.PermissionRe
283 Path: perm.Path,
292 Path: perm.Path,
284 Params: perm.Params,
293 Params: perm.Params,
285 },
294 },
286− Action: proto.PermissionAllow,
295+ Action: proto.PermissionAllowForSession,
287 })
296 })
297+ return resolved
288}
298}
289
299
290−func (w *ClientWorkspace) PermissionDeny(perm permission.PermissionRequest) {
300+func (w *ClientWorkspace) PermissionDeny(perm permission.PermissionRequest) bool {
291− _ = w.client.GrantPermission(context.Background(), w.workspaceID(), proto.PermissionGrant{
301+ resolved, _ := w.client.GrantPermission(context.Background(), w.workspaceID(), proto.PermissionGrant{
292 Permission: proto.PermissionRequest{
302 Permission: proto.PermissionRequest{
293 ID: perm.ID,
303 ID: perm.ID,
294 SessionID: perm.SessionID,
304 SessionID: perm.SessionID,
@@ -301,6 +311,7 @@ func (w *ClientWorkspace) PermissionDeny(perm permission.PermissionRequest) {
301 },
311 },
302 Action: proto.PermissionDeny,
312 Action: proto.PermissionDeny,
303 })
313 })
314+ return resolved
304}
315}
305
316
306func (w *ClientWorkspace) PermissionSkipRequests() bool {
317func (w *ClientWorkspace) PermissionSkipRequests() bool {
@@ -593,10 +604,22 @@ func (w *ClientWorkspace) Subscribe(program *tea.Program) {
593 return
604 return
594 }
605 }
595
606
607+ w.consumeEvents(evc, program.Send)
608+}
609+
610+// consumeEvents drives the workspace event loop. It is split out from
611+// Subscribe so tests can drive it without a real *tea.Program.
612+// ConfigChanged events trigger a workspace refresh; all other events
613+// are translated into domain types and forwarded to send.
614+func (w *ClientWorkspace) consumeEvents(evc <-chan any, send func(tea.Msg)) {
596 for ev := range evc {
615 for ev := range evc {
616+ if _, ok := ev.(pubsub.Event[proto.ConfigChanged]); ok {
617+ w.refreshWorkspace()
618+ continue
619+ }
597 translated := w.translateEvent(ev)
620 translated := w.translateEvent(ev)
598− if translated != nil {
621+ if translated != nil && send != nil {
599− program.Send(translated)
622+ send(translated)
600 }
623 }
601 }
624 }
602}
625}
@@ -714,6 +737,13 @@ func protoToMCPEventType(t proto.MCPEventType) mcp.EventType {
714 }
737 }
715}
738}
716
739
740+// protoToSession converts a wire-level proto.Session into the domain
741+// session.Session. Fields that exist only on the wire (computed-on-read
742+// signals like IsBusy, and any future presence counters) are
743+// intentionally dropped here: session.Session models persisted state,
744+// not transient runtime signals. UI features that need those signals
745+// should either extend session.Session or read them from the proto
746+// payload directly before this conversion runs.
717func protoToSession(s proto.Session) session.Session {
747func protoToSession(s proto.Session) session.Session {
718 return session.Session{
748 return session.Session{
719 ID: s.ID,
749 ID: s.ID,
internal/workspace/client_workspace_test.go+88
@@ -1,9 +1,16 @@
1package workspace
1package workspace
2
2
3import (
3import (
4+ "encoding/json"
5+ "io"
6+ "net/http"
7+ "net/http/httptest"
8+ "net/url"
4 "testing"
9 "testing"
5
10
11+ "github.com/charmbracelet/crush/internal/client"
6 "github.com/charmbracelet/crush/internal/message"
12 "github.com/charmbracelet/crush/internal/message"
13+ "github.com/charmbracelet/crush/internal/permission"
7 "github.com/charmbracelet/crush/internal/proto"
14 "github.com/charmbracelet/crush/internal/proto"
8 "github.com/charmbracelet/crush/internal/pubsub"
15 "github.com/charmbracelet/crush/internal/pubsub"
9 "github.com/charmbracelet/crush/internal/skills"
16 "github.com/charmbracelet/crush/internal/skills"
@@ -47,6 +54,87 @@ func TestProtoToMessageToolResult(t *testing.T) {
47 require.False(t, tr.IsError)
54 require.False(t, tr.IsError)
48}
55}
49
56
57+// TestClientWorkspace_PermissionGrantMapping verifies that
58+// PermissionGrant on the ClientWorkspace serializes a one-time grant
59+// (proto.PermissionAllow) and PermissionGrantPersistent serializes a
60+// persistent grant (proto.PermissionAllowForSession). A swap between
61+// these two would silently flip "allow once" into "remember for the
62+// session", and vice versa, so we pin the wire mapping here.
63+func TestClientWorkspace_PermissionGrantMapping(t *testing.T) {
64+ t.Parallel()
65+
66+ cases := []struct {
67+ name string
68+ call func(*ClientWorkspace, permission.PermissionRequest)
69+ want proto.PermissionAction
70+ }{
71+ {
72+ name: "Grant -> PermissionAllow",
73+ call: func(w *ClientWorkspace, p permission.PermissionRequest) {
74+ w.PermissionGrant(p)
75+ },
76+ want: proto.PermissionAllow,
77+ },
78+ {
79+ name: "GrantPersistent -> PermissionAllowForSession",
80+ call: func(w *ClientWorkspace, p permission.PermissionRequest) {
81+ w.PermissionGrantPersistent(p)
82+ },
83+ want: proto.PermissionAllowForSession,
84+ },
85+ {
86+ name: "Deny -> PermissionDeny",
87+ call: func(w *ClientWorkspace, p permission.PermissionRequest) {
88+ w.PermissionDeny(p)
89+ },
90+ want: proto.PermissionDeny,
91+ },
92+ }
93+
94+ for _, tc := range cases {
95+ t.Run(tc.name, func(t *testing.T) {
96+ t.Parallel()
97+
98+ var got proto.PermissionGrant
99+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
100+ require.Equal(t, http.MethodPost, r.Method)
101+ require.Equal(t, "/v1/workspaces/ws-1/permissions/grant", r.URL.Path)
102+ body, err := io.ReadAll(r.Body)
103+ require.NoError(t, err)
104+ require.NoError(t, json.Unmarshal(body, &got))
105+ require.NoError(t, json.NewEncoder(w).Encode(proto.PermissionGrantResponse{Resolved: true}))
106+ }))
107+ defer srv.Close()
108+
109+ u, err := url.Parse(srv.URL)
110+ require.NoError(t, err)
111+ c, err := client.NewClient(t.TempDir(), "tcp", u.Host)
112+ require.NoError(t, err)
113+
114+ ws := NewClientWorkspace(c, proto.Workspace{ID: "ws-1"})
115+
116+ perm := permission.PermissionRequest{
117+ ID: "req-1",
118+ SessionID: "sess-1",
119+ ToolCallID: "tc-1",
120+ ToolName: "tool",
121+ Description: "do thing",
122+ Action: "act",
123+ Path: "/tmp/p",
124+ }
125+ tc.call(ws, perm)
126+
127+ require.Equal(t, tc.want, got.Action)
128+ require.Equal(t, "req-1", got.Permission.ID)
129+ require.Equal(t, "sess-1", got.Permission.SessionID)
130+ require.Equal(t, "tc-1", got.Permission.ToolCallID)
131+ require.Equal(t, "tool", got.Permission.ToolName)
132+ require.Equal(t, "act", got.Permission.Action)
133+ require.Equal(t, "/tmp/p", got.Permission.Path)
134+ })
135+ }
136+}
137+
50// TestProtoToSkillStates verifies that the wire representation of skill
138// TestProtoToSkillStates verifies that the wire representation of skill
51// discovery states reconstructs identical values on the client,
139// discovery states reconstructs identical values on the client,
52// including synthetic errors derived from Error strings.
140// including synthetic errors derived from Error strings.
internal/workspace/export_test.gonew file+14
@@ -0,0 +1,14 @@
1+package workspace
2+
3+import (
4+ tea "charm.land/bubbletea/v2"
5+)
6+
7+// ConsumeEventsForTest runs the event-handling loop on the given
8+// channel, invoking send for translated domain messages and refreshing
9+// the cached workspace snapshot on ConfigChanged. Exposed for
10+// cross-package integration tests that cannot rely on a real
11+// *tea.Program. It returns when evc is closed.
12+func (w *ClientWorkspace) ConsumeEventsForTest(evc <-chan any, send func(tea.Msg)) {
13+ w.consumeEvents(evc, send)
14+}
internal/workspace/multiclient_integration_test.gonew file+176
@@ -0,0 +1,176 @@
1+package workspace_test
2+
3+import (
4+ "context"
5+ "net/http/httptest"
6+ "net/url"
7+ "testing"
8+ "time"
9+
10+ tea "charm.land/bubbletea/v2"
11+ "github.com/charmbracelet/crush/internal/client"
12+ "github.com/charmbracelet/crush/internal/config"
13+ "github.com/charmbracelet/crush/internal/proto"
14+ "github.com/charmbracelet/crush/internal/pubsub"
15+ "github.com/charmbracelet/crush/internal/server"
16+ "github.com/charmbracelet/crush/internal/workspace"
17+ "github.com/stretchr/testify/require"
18+)
19+
20+// xdgIsolate redirects HOME and XDG_* to fresh temp dirs so config
21+// loading does not touch the host's real config.
22+func xdgIsolate(t *testing.T) {
23+ t.Helper()
24+ t.Setenv("HOME", t.TempDir())
25+ t.Setenv("XDG_CACHE_HOME", t.TempDir())
26+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
27+ t.Setenv("XDG_DATA_HOME", t.TempDir())
28+}
29+
30+// runtimeServer wires the production server handler around an
31+// httptest.NewServer for integration testing.
32+type runtimeServer struct {
33+ httpSrv *httptest.Server
34+ host string
35+}
36+
37+func newRuntimeServer(t *testing.T) *runtimeServer {
38+ t.Helper()
39+ s := server.NewServer(nil, "tcp", "127.0.0.1:0")
40+ hs := httptest.NewServer(s.Handler())
41+ t.Cleanup(hs.Close)
42+
43+ u, err := url.Parse(hs.URL)
44+ require.NoError(t, err)
45+ return &runtimeServer{httpSrv: hs, host: u.Host}
46+}
47+
48+func (r *runtimeServer) newClient(t *testing.T, path string) *client.Client {
49+ t.Helper()
50+ c, err := client.NewClient(path, "tcp", r.host)
51+ require.NoError(t, err)
52+ return c
53+}
54+
55+// TestClientWorkspace_ConfigChangedRefreshesSiblingCache is the
56+// cross-client refresh end-to-end test required by PLAN item 4. Two
57+// ClientWorkspace instances pointed at the same backend workspace
58+// subscribe to events; when one mutates configuration via the server,
59+// the other's cached Config snapshot reflects the new value without
60+// a manual refresh.
61+func TestClientWorkspace_ConfigChangedRefreshesSiblingCache(t *testing.T) {
62+ xdgIsolate(t)
63+ rt := newRuntimeServer(t)
64+
65+ cwd := t.TempDir()
66+ dataDir := t.TempDir()
67+
68+ cA := rt.newClient(t, cwd)
69+ cB := rt.newClient(t, cwd)
70+ ctx, cancel := context.WithCancel(context.Background())
71+ t.Cleanup(cancel)
72+
73+ wsProto, err := cA.CreateWorkspace(ctx, proto.Workspace{Path: cwd, DataDir: dataDir})
74+ require.NoError(t, err)
75+ // Client B joins the same workspace by path; the server
76+ // deduplicates and returns the existing workspace.
77+ wsProtoB, err := cB.CreateWorkspace(ctx, proto.Workspace{Path: cwd, DataDir: dataDir})
78+ require.NoError(t, err)
79+ require.Equal(t, wsProto.ID, wsProtoB.ID)
80+
81+ wsA := workspace.NewClientWorkspace(cA, *wsProto)
82+ wsB := workspace.NewClientWorkspace(cB, *wsProtoB)
83+
84+ // Both clients attach event streams. They run for the
85+ // lifetime of the test; cancelling via context tears them
86+ // down. consumeEvents is exercised by Subscribe in production;
87+ // here we run it inline so we don't need a real *tea.Program.
88+ evcA, err := cA.SubscribeEvents(ctx, wsProto.ID)
89+ require.NoError(t, err)
90+ evcB, err := cB.SubscribeEvents(ctx, wsProto.ID)
91+ require.NoError(t, err)
92+
93+ go wsA.ConsumeEventsForTest(evcA, func(tea.Msg) {})
94+ go wsB.ConsumeEventsForTest(evcB, func(tea.Msg) {})
95+
96+ // Pre-condition: neither cache has compact mode enabled yet.
97+ require.NotNil(t, wsA.Config())
98+ require.NotNil(t, wsB.Config())
99+ require.False(t, compactMode(wsA.Config()), "compact mode must start disabled on client A")
100+ require.False(t, compactMode(wsB.Config()), "compact mode must start disabled on client B")
101+
102+ // Client A flips a real config-mutating workspace operation
103+ // (SetCompactMode) via the server. PLAN item 4 acceptance:
104+ // B's cached ws.Config must reflect this change without restart.
105+ // SetCompactMode is used over UpdatePreferredModel because the
106+ // latter's autoReload reverts unknown-provider models back to
107+ // defaults during configureSelectedModels, which would make the
108+ // assertion test infrastructure rather than the cache wiring.
109+ require.NoError(t, wsA.SetCompactMode(config.ScopeGlobal, true))
110+
111+ // Client A writes and refreshes synchronously inside
112+ // SetCompactMode, so its cache must already reflect the change.
113+ // Eventually here absorbs any background work but should pass
114+ // immediately.
115+ require.Eventually(t, func() bool { return compactMode(wsA.Config()) },
116+ 3*time.Second, 25*time.Millisecond,
117+ "client A cache must reflect its own compact-mode mutation")
118+
119+ // Client B must see the same change via the ConfigChanged SSE
120+ // event triggering its own cached refresh.
121+ require.Eventually(t, func() bool { return compactMode(wsB.Config()) },
122+ 3*time.Second, 25*time.Millisecond,
123+ "client B cache must reflect A's compact-mode mutation via SSE")
124+}
125+
126+// compactMode is a tiny accessor that survives nil intermediates so
127+// the Eventually polling loop can call it on a transient cache state.
128+func compactMode(cfg *config.Config) bool {
129+ if cfg == nil || cfg.Options == nil {
130+ return false
131+ }
132+ return cfg.Options.TUI.CompactMode
133+}
134+
135+// TestClientWorkspace_ConfigChangedSignalArrives is a smaller test
136+// that asserts the SSE wiring delivers a ConfigChanged event to the
137+// raw client subscription. It catches breakage in the
138+// wrapEvent/decoder bridge independent of the workspace cache.
139+func TestClientWorkspace_ConfigChangedSignalArrives(t *testing.T) {
140+ xdgIsolate(t)
141+ rt := newRuntimeServer(t)
142+
143+ cwd := t.TempDir()
144+ dataDir := t.TempDir()
145+
146+ c := rt.newClient(t, cwd)
147+ ctx, cancel := context.WithCancel(context.Background())
148+ t.Cleanup(cancel)
149+
150+ wsProto, err := c.CreateWorkspace(ctx, proto.Workspace{Path: cwd, DataDir: dataDir})
151+ require.NoError(t, err)
152+
153+ evc, err := c.SubscribeEvents(ctx, wsProto.ID)
154+ require.NoError(t, err)
155+
156+ require.NoError(t, c.SetConfigField(ctx, wsProto.ID, config.ScopeGlobal, "options.debug", true))
157+
158+ gotConfigChanged := false
159+ deadline := time.After(3 * time.Second)
160+loop:
161+ for !gotConfigChanged {
162+ select {
163+ case ev, ok := <-evc:
164+ if !ok {
165+ break loop
166+ }
167+ if cc, isCC := ev.(pubsub.Event[proto.ConfigChanged]); isCC {
168+ require.Equal(t, wsProto.ID, cc.Payload.WorkspaceID)
169+ gotConfigChanged = true
170+ }
171+ case <-deadline:
172+ break loop
173+ }
174+ }
175+ require.True(t, gotConfigChanged, "expected ConfigChanged event over SSE")
176+}
internal/workspace/workspace.go+17−3
@@ -68,6 +68,12 @@ type Workspace interface {
68 DeleteSession(ctx context.Context, sessionID string) error
68 DeleteSession(ctx context.Context, sessionID string) error
69 CreateAgentToolSessionID(messageID, toolCallID string) string
69 CreateAgentToolSessionID(messageID, toolCallID string) string
70 ParseAgentToolSessionID(sessionID string) (messageID string, toolCallID string, ok bool)
70 ParseAgentToolSessionID(sessionID string) (messageID string, toolCallID string, ok bool)
71+ // SetCurrentSession reports the session this client is currently
72+ // viewing. Empty sessionID clears the entry (e.g. landing screen).
73+ // In single-client local mode this is a no-op. In client/server
74+ // mode it informs the server's per-client presence map so other
75+ // observers can compute attached-client counts per session.
76+ SetCurrentSession(ctx context.Context, sessionID string) error
71
77
72 // Messages
78 // Messages
73 ListMessages(ctx context.Context, sessionID string) ([]message.Message, error)
79 ListMessages(ctx context.Context, sessionID string) ([]message.Message, error)
@@ -90,9 +96,17 @@ type Workspace interface {
90 GetDefaultSmallModel(providerID string) config.SelectedModel
96 GetDefaultSmallModel(providerID string) config.SelectedModel
91
97
92 // Permissions
98 // Permissions
99+ //
100+ // PermissionGrant, PermissionGrantPersistent, and PermissionDeny
101+ // return true if the call resolved the pending request and false if
102+ // it had already been resolved by another subscriber (or is no
103+ // longer pending). A false return is not an error; the modal can
104+ // still close locally because the resolution will arrive via the
105+ // PermissionNotification event stream regardless of which client
106+ // won the race.
93− PermissionGrant(perm permission.PermissionRequest)
107+ PermissionGrant(perm permission.PermissionRequest) bool
94− PermissionGrantPersistent(perm permission.PermissionRequest)
108+ PermissionGrantPersistent(perm permission.PermissionRequest) bool
95− PermissionDeny(perm permission.PermissionRequest)
109+ PermissionDeny(perm permission.PermissionRequest) bool
96 PermissionSkipRequests() bool
110 PermissionSkipRequests() bool
97 PermissionSetSkipRequests(skip bool)
111 PermissionSetSkipRequests(skip bool)
98
112