git.iko.soy/crush
on main help sign in
feat(server): share one workspace per directory across clients
1ce40dfChristian Rocha
internal/backend/backend.go+414−31
compare with main
unifiedsplit
@@ -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"
@@ -28,19 +31,59 @@ var (
28 ErrPathRequired = errors.New("path is required")
31 ErrPathRequired = errors.New("path is required")
29 ErrInvalidPermissionAction = errors.New("invalid permission action")
32 ErrInvalidPermissionAction = errors.New("invalid permission action")
30 ErrUnknownCommand = errors.New("unknown command")
33 ErrUnknownCommand = errors.New("unknown command")
34+ ErrInvalidClientID = errors.New("invalid client_id")
31)
35)
32
36
37+// DefaultCreateGrace is the window in which a client must open an SSE
38+// stream after creating a workspace before its creation hold is
39+// released. Exposed as a package variable so tests can shorten it.
40+var DefaultCreateGrace = 30 * time.Second
41+
33// ShutdownFunc is called when the backend needs to trigger a server
42// ShutdownFunc is called when the backend needs to trigger a server
34// shutdown (e.g. when the last workspace is removed).
43// shutdown (e.g. when the last workspace is removed).
35type ShutdownFunc func()
44type ShutdownFunc func()
36
45
37// Backend provides transport-agnostic business logic for the Crush
46// Backend provides transport-agnostic business logic for the Crush
38// server. It manages workspaces and delegates to [app.App] services.
47// server. It manages workspaces and delegates to [app.App] services.
48+//
49+// Locking order: when both [Backend.mu] and [Workspace.clientsMu] are
50+// held at once, [Backend.mu] is acquired first. Detach paths
51+// ([detachStream], [releaseHoldLocked], [expireHold]) only hold
52+// [Workspace.clientsMu] briefly, drop it, then call [teardown] which
53+// takes [Backend.mu] (and then re-takes [Workspace.clientsMu] to
54+// re-check that the workspace has not been re-claimed). This avoids
55+// the AB/BA hazard with [CreateWorkspace], which holds [Backend.mu]
56+// while calling [registerClient] so that a workspace cannot be torn
57+// down beneath it.
39type Backend struct {
58type Backend struct {
40 workspaces *csync.Map[string, *Workspace]
59 workspaces *csync.Map[string, *Workspace]
60+ // pathIndex maps a resolved absolute workspace path to its
61+ // workspace ID. Reads and writes are serialised via mu so
62+ // concurrent CreateWorkspace calls at the same path deduplicate
63+ // deterministically.
64+ pathIndex map[string]string
65+ mu sync.Mutex
66+
41− cfg *config.ConfigStore
67+ cfg *config.ConfigStore
42− ctx context.Context
68+ ctx context.Context
43− shutdownFn ShutdownFunc
69+ shutdownFn ShutdownFunc
70+ createGrace time.Duration
71+}
72+
73+// clientState tracks one client's claim on a workspace.
74+//
75+// - streams counts the number of live SSE event streams the client
76+// currently has open against the workspace.
77+// - holdTimer is non-nil iff the client created the workspace but has
78+// not yet attached an SSE stream; it fires after createGrace and
79+// releases the hold.
80+//
81+// The two are mutually exclusive in practice (the hold timer is stopped
82+// the moment an SSE stream attaches), but both being zero/nil means the
83+// entry has been released and should be removed.
84+type clientState struct {
85+ streams int
86+ holdTimer *time.Timer
44}
87}
45
88
46// Workspace represents a running [app.App] workspace with its
89// Workspace represents a running [app.App] workspace with its
@@ -51,18 +94,57 @@ type Workspace struct {
51 Path string
94 Path string
52 Cfg *config.ConfigStore
95 Cfg *config.ConfigStore
53 Env []string
96 Env []string
97+
98+ // resolvedPath is the path used as the dedup key in
99+ // Backend.pathIndex. It is filepath.EvalSymlinks(filepath.Abs(Path))
100+ // with fallback to the cleaned absolute path.
101+ resolvedPath string
102+
103+ // clientsMu guards clients. It is held only briefly (no IO).
104+ clientsMu sync.Mutex
105+ // clients tracks each client's claim on this workspace. Refcount
106+ // is a derived value: len(clients).
107+ clients map[string]*clientState
108+
109+ // shutdownFn is the function invoked by [Backend.teardown] to
110+ // release the workspace's underlying resources. It defaults to the
111+ // embedded [app.App.Shutdown]; tests may override it to avoid
112+ // driving a full [app.App] through shutdown.
113+ shutdownFn func()
114+}
115+
116+// invokeShutdown calls the workspace shutdown hook if set, falling
117+// back to the embedded [app.App.Shutdown] when not.
118+func (w *Workspace) invokeShutdown() {
119+ if w.shutdownFn != nil {
120+ w.shutdownFn()
121+ return
122+ }
123+ if w.App != nil {
124+ w.Shutdown()
125+ }
54}
126}
55
127
56// New creates a new [Backend].
128// New creates a new [Backend].
57func New(ctx context.Context, cfg *config.ConfigStore, shutdownFn ShutdownFunc) *Backend {
129func New(ctx context.Context, cfg *config.ConfigStore, shutdownFn ShutdownFunc) *Backend {
58 return &Backend{
130 return &Backend{
59− workspaces: csync.NewMap[string, *Workspace](),
131+ workspaces: csync.NewMap[string, *Workspace](),
132+ pathIndex: make(map[string]string),
60− cfg: cfg,
133+ cfg: cfg,
61− ctx: ctx,
134+ ctx: ctx,
62− shutdownFn: shutdownFn,
135+ shutdownFn: shutdownFn,
136+ createGrace: DefaultCreateGrace,
63 }
137 }
64}
138}
65
139
140+// SetCreateGrace overrides the create-grace window. Intended for tests
141+// that need short timeouts.
142+func (b *Backend) SetCreateGrace(d time.Duration) {
143+ b.mu.Lock()
144+ defer b.mu.Unlock()
145+ b.createGrace = d
146+}
147+
66// GetWorkspace retrieves a workspace by ID.
148// GetWorkspace retrieves a workspace by ID.
67func (b *Backend) GetWorkspace(id string) (*Workspace, error) {
149func (b *Backend) GetWorkspace(id string) (*Workspace, error) {
68 ws, ok := b.workspaces.Get(id)
150 ws, ok := b.workspaces.Get(id)
@@ -82,12 +164,46 @@ func (b *Backend) ListWorkspaces() []proto.Workspace {
82}
164}
83
165
84// CreateWorkspace initializes a new workspace from the given
166// CreateWorkspace initializes a new workspace from the given
85−// parameters. It creates the config, database connection, and
167+// parameters, or returns an existing workspace if one already exists at
86−// [app.App] instance.
168+// the same resolved path (first-wins semantics).
169+//
170+// args.ClientID must be a valid UUID identifying the calling client;
171+// the resulting workspace registers a creation hold on behalf of that
172+// client which is released either by the first SSE attach (which
173+// converts it into a stream claim) or by the grace window expiring.
87func (b *Backend) CreateWorkspace(args proto.Workspace) (*Workspace, proto.Workspace, error) {
174func (b *Backend) CreateWorkspace(args proto.Workspace) (*Workspace, proto.Workspace, error) {
88 if args.Path == "" {
175 if args.Path == "" {
89 return nil, proto.Workspace{}, ErrPathRequired
176 return nil, proto.Workspace{}, ErrPathRequired
90 }
177 }
178+ clientID, err := validateClientID(args.ClientID)
179+ if err != nil {
180+ return nil, proto.Workspace{}, err
181+ }
182+
183+ key, err := resolveWorkspaceKey(args.Path)
184+ if err != nil {
185+ return nil, proto.Workspace{}, fmt.Errorf("failed to resolve workspace path: %w", err)
186+ }
187+
188+ b.mu.Lock()
189+ if existingID, ok := b.pathIndex[key]; ok {
190+ if ws, found := b.workspaces.Get(existingID); found {
191+ // Hold b.mu while registering: teardown also
192+ // acquires b.mu before tearing the workspace
193+ // down, so this guarantees the workspace we
194+ // return cannot be torn out from under us
195+ // between lookup and registerClient. Lock order
196+ // here is b.mu -> ws.clientsMu.
197+ logFirstWinsMismatch(ws, args)
198+ b.registerClient(ws, clientID)
199+ b.mu.Unlock()
200+ return ws, workspaceToProto(ws), nil
201+ }
202+ // pathIndex referenced a workspace that has since been
203+ // removed; clean the stale entry and fall through.
204+ delete(b.pathIndex, key)
205+ }
206+ b.mu.Unlock()
91
207
92 id := uuid.New().String()
208 id := uuid.New().String()
93 cfg, err := config.Init(args.Path, args.DataDir, args.Debug)
209 cfg, err := config.Init(args.Path, args.DataDir, args.Debug)
@@ -112,14 +228,38 @@ func (b *Backend) CreateWorkspace(args proto.Workspace) (*Workspace, proto.Works
112 }
228 }
113
229
114 ws := &Workspace{
230 ws := &Workspace{
115− App: appWorkspace,
231+ App: appWorkspace,
116− ID: id,
232+ ID: id,
117− Path: args.Path,
233+ Path: args.Path,
118− Cfg: cfg,
234+ Cfg: cfg,
119− Env: args.Env,
235+ Env: args.Env,
236+ resolvedPath: key,
237+ clients: make(map[string]*clientState),
120 }
238 }
121
239
240+ b.mu.Lock()
241+ // Re-check the index under the lock: a concurrent caller may have
242+ // won the race between the initial unlock and here.
243+ if existingID, ok := b.pathIndex[key]; ok {
244+ if existing, found := b.workspaces.Get(existingID); found {
245+ // Register under b.mu so teardown cannot run
246+ // between lookup and registerClient. Lock order
247+ // is b.mu -> ws.clientsMu.
248+ logFirstWinsMismatch(existing, args)
249+ b.registerClient(existing, clientID)
250+ b.mu.Unlock()
251+ ws.invokeShutdown()
252+ return existing, workspaceToProto(existing), nil
253+ }
254+ delete(b.pathIndex, key)
255+ }
122 b.workspaces.Set(id, ws)
256 b.workspaces.Set(id, ws)
257+ b.pathIndex[key] = id
258+ // Register the originating client's hold while still holding
259+ // b.mu so the workspace is observable with its claim from the
260+ // moment it appears in the index.
261+ b.registerClient(ws, clientID)
262+ b.mu.Unlock()
123
263
124 if args.Version != "" && args.Version != version.Version {
264 if args.Version != "" && args.Version != version.Version {
125 slog.Warn(
265 slog.Warn(
@@ -133,34 +273,201 @@ func (b *Backend) CreateWorkspace(args proto.Workspace) (*Workspace, proto.Works
133 )))
273 )))
134 }
274 }
135
275
136− result := proto.Workspace{
276+ return ws, workspaceToProto(ws), nil
137− ID: id,
277+}
138− Path: args.Path,
278+
139− DataDir: cfg.Config().Options.DataDirectory,
279+// AttachClient registers a new SSE stream for the given client on the
140− Debug: cfg.Config().Options.Debug,
280+// workspace. The stream's deferred cleanup must call DetachClient with
141− YOLO: cfg.Overrides().SkipPermissionRequests,
281+// the same arguments to release the claim.
142− Config: cfg.Config(),
282+//
143− Env: args.Env,
283+// The lookup and the clients-map mutation are performed under
284+// [Backend.mu] so that AttachClient cannot race with [Backend.teardown]:
285+// teardown also holds [Backend.mu] while removing the workspace from
286+// b.workspaces, so once AttachClient observes the workspace and takes
287+// ws.clientsMu (under b.mu), no concurrent teardown can succeed without
288+// re-checking the (now non-empty) clients map. Lock order is the
289+// canonical b.mu -> ws.clientsMu.
290+func (b *Backend) AttachClient(workspaceID, clientID string) error {
291+ if _, err := validateClientID(clientID); err != nil {
292+ return err
293+ }
294+
295+ b.mu.Lock()
296+ defer b.mu.Unlock()
297+ ws, ok := b.workspaces.Get(workspaceID)
298+ if !ok {
299+ return ErrWorkspaceNotFound
144 }
300 }
145
301
146− return ws, result, nil
302+ ws.clientsMu.Lock()
303+ defer ws.clientsMu.Unlock()
304+ cs, ok := ws.clients[clientID]
305+ if !ok {
306+ // Defensive: SSE attach without a prior CreateWorkspace by
307+ // this client still installs a stream claim so the stream
308+ // stays alive for its duration.
309+ ws.clients[clientID] = &clientState{streams: 1}
310+ return nil
311+ }
312+ if cs.holdTimer != nil {
313+ cs.holdTimer.Stop()
314+ cs.holdTimer = nil
315+ }
316+ cs.streams++
317+ return nil
147}
318}
148
319
149−// DeleteWorkspace shuts down and removes a workspace. If it was the
320+// DetachClient releases one SSE stream's hold on the workspace. If the
321+// client has no other streams and no pending creation hold, its claim
322+// is removed and the workspace is torn down once refcount hits zero.
323+func (b *Backend) DetachClient(workspaceID, clientID string) {
324+ ws, ok := b.workspaces.Get(workspaceID)
325+ if !ok {
326+ return
327+ }
328+ b.detachStream(ws, clientID)
329+}
330+
331+// releaseHold releases the creation hold for a client, if any. Active
332+// stream claims are unaffected. Idempotent: returns nil if the
333+// workspace or the client's hold no longer exist.
334+func (b *Backend) releaseHold(workspaceID, clientID string) error {
335+ if _, err := validateClientID(clientID); err != nil {
336+ return err
337+ }
338+ ws, ok := b.workspaces.Get(workspaceID)
339+ if !ok {
340+ return nil
341+ }
342+ b.releaseHoldLocked(ws, clientID)
343+ return nil
344+}
345+
346+// registerClient installs (idempotently) the given client's claim on
347+// the workspace and starts a grace timer if the entry is fresh.
348+func (b *Backend) registerClient(ws *Workspace, clientID string) {
349+ ws.clientsMu.Lock()
350+ defer ws.clientsMu.Unlock()
351+ if _, ok := ws.clients[clientID]; ok {
352+ // Idempotent: a duplicate CreateWorkspace from the same
353+ // client does not add a second claim.
354+ return
355+ }
356+ cs := &clientState{}
357+ cs.holdTimer = time.AfterFunc(b.createGrace, func() {
358+ b.expireHold(ws, clientID, cs)
359+ })
360+ ws.clients[clientID] = cs
361+}
362+
363+// expireHold is the body of the grace timer. It runs in its own
364+// goroutine and races against AttachClient/releaseHold; the timer
365+// stays valid only while the entry's holdTimer still points at it.
366+func (b *Backend) expireHold(ws *Workspace, clientID string, timer *clientState) {
367+ ws.clientsMu.Lock()
368+ cs, ok := ws.clients[clientID]
369+ if !ok || cs != timer || cs.holdTimer == nil || cs.streams > 0 {
370+ ws.clientsMu.Unlock()
371+ return
372+ }
373+ cs.holdTimer = nil
374+ delete(ws.clients, clientID)
375+ teardown := len(ws.clients) == 0
376+ ws.clientsMu.Unlock()
377+ if teardown {
378+ b.teardown(ws)
379+ }
380+}
381+
382+func (b *Backend) releaseHoldLocked(ws *Workspace, clientID string) {
383+ ws.clientsMu.Lock()
384+ cs, ok := ws.clients[clientID]
385+ if !ok {
386+ ws.clientsMu.Unlock()
387+ return
388+ }
389+ if cs.holdTimer != nil {
390+ cs.holdTimer.Stop()
391+ cs.holdTimer = nil
392+ }
393+ teardown := false
394+ if cs.streams == 0 {
395+ delete(ws.clients, clientID)
396+ teardown = len(ws.clients) == 0
397+ }
398+ ws.clientsMu.Unlock()
399+ if teardown {
400+ b.teardown(ws)
401+ }
402+}
403+
404+func (b *Backend) detachStream(ws *Workspace, clientID string) {
405+ ws.clientsMu.Lock()
406+ cs, ok := ws.clients[clientID]
407+ if !ok {
408+ ws.clientsMu.Unlock()
409+ return
410+ }
411+ if cs.streams > 0 {
412+ cs.streams--
413+ }
414+ teardown := false
415+ if cs.streams == 0 && cs.holdTimer == nil {
416+ delete(ws.clients, clientID)
417+ teardown = len(ws.clients) == 0
418+ }
419+ ws.clientsMu.Unlock()
420+ if teardown {
421+ b.teardown(ws)
422+ }
423+}
424+
425+// teardown removes the workspace from the index, shuts down its
426+// underlying [app.App], and triggers a server shutdown if it was the
150−// last workspace, the shutdown callback is invoked.
427+// last workspace alive.
151−func (b *Backend) DeleteWorkspace(id string) {
428+//
152− ws, ok := b.workspaces.Get(id)
429+// Callers reach teardown after observing len(ws.clients) == 0 while
153− if ok {
430+// holding ws.clientsMu and then releasing it. Between that release
154− ws.Shutdown()
431+// and the b.mu.Lock below, a concurrent CreateWorkspace may have
432+// re-registered a client (CreateWorkspace holds b.mu while doing so,
433+// so it is mutually exclusive with this critical section). teardown
434+// re-checks under both locks (in the canonical b.mu -> ws.clientsMu
435+// order) and aborts if the workspace has been re-claimed.
436+func (b *Backend) teardown(ws *Workspace) {
437+ b.mu.Lock()
438+ ws.clientsMu.Lock()
439+ if len(ws.clients) > 0 {
440+ // Race: a CreateWorkspace re-registered a client
441+ // between the detach path dropping ws.clientsMu and us
442+ // taking b.mu. Abort: the workspace is still alive.
443+ ws.clientsMu.Unlock()
444+ b.mu.Unlock()
445+ return
155 }
446 }
447+ ws.clientsMu.Unlock()
448+ if existing, ok := b.pathIndex[ws.resolvedPath]; ok && existing == ws.ID {
449+ delete(b.pathIndex, ws.resolvedPath)
450+ }
156− b.workspaces.Del(id)
451+ b.workspaces.Del(ws.ID)
452+ remaining := b.workspaces.Len()
453+ b.mu.Unlock()
454+
455+ ws.invokeShutdown()
157
456
158− if b.workspaces.Len() == 0 && b.shutdownFn != nil {
457+ if remaining == 0 && b.shutdownFn != nil {
159 slog.Info("Last workspace removed, shutting down server...")
458 slog.Info("Last workspace removed, shutting down server...")
160 b.shutdownFn()
459 b.shutdownFn()
161 }
460 }
162}
461}
163
462
463+// DeleteWorkspace is the public entry point used by the HTTP DELETE
464+// handler. It releases the named client's creation hold; live streams
465+// from the same client remain attached and continue holding the
466+// workspace open until their own deferred DetachClient runs.
467+func (b *Backend) DeleteWorkspace(id, clientID string) error {
468+ return b.releaseHold(id, clientID)
469+}
470+
164// GetWorkspaceProto returns the proto representation of a workspace.
471// GetWorkspaceProto returns the proto representation of a workspace.
165func (b *Backend) GetWorkspaceProto(id string) (proto.Workspace, error) {
472func (b *Backend) GetWorkspaceProto(id string) (proto.Workspace, error) {
166 ws, err := b.GetWorkspace(id)
473 ws, err := b.GetWorkspace(id)
@@ -193,6 +500,33 @@ func (b *Backend) Shutdown() {
193 }
500 }
194}
501}
195
502
503+// resolveWorkspaceKey returns a stable canonical form of path suitable
504+// for use as a dedup key. It applies filepath.Abs, then attempts
505+// filepath.EvalSymlinks; because EvalSymlinks errors on non-existent
506+// paths, it falls back to the cleaned absolute path in that case.
507+func resolveWorkspaceKey(path string) (string, error) {
508+ abs, err := filepath.Abs(path)
509+ if err != nil {
510+ return "", err
511+ }
512+ if resolved, err := filepath.EvalSymlinks(abs); err == nil {
513+ return resolved, nil
514+ }
515+ return abs, nil
516+}
517+
518+// validateClientID returns the trimmed UUID string or an error if the
519+// input is empty or not a valid UUID.
520+func validateClientID(id string) (string, error) {
521+ if id == "" {
522+ return "", ErrInvalidClientID
523+ }
524+ if _, err := uuid.Parse(id); err != nil {
525+ return "", fmt.Errorf("%w: %v", ErrInvalidClientID, err)
526+ }
527+ return id, nil
528+}
529+
196func workspaceToProto(ws *Workspace) proto.Workspace {
530func workspaceToProto(ws *Workspace) proto.Workspace {
197 cfg := ws.Cfg.Config()
531 cfg := ws.Cfg.Config()
198 return proto.Workspace{
532 return proto.Workspace{
@@ -202,5 +536,54 @@ func workspaceToProto(ws *Workspace) proto.Workspace {
202 DataDir: cfg.Options.DataDirectory,
536 DataDir: cfg.Options.DataDirectory,
203 Debug: cfg.Options.Debug,
537 Debug: cfg.Options.Debug,
204 Config: cfg,
538 Config: cfg,
539+ Env: ws.Env,
540+ Version: version.Version,
541+ }
542+}
543+
544+// logFirstWinsMismatch emits a debug line whenever a second
545+// CreateWorkspace at the same resolved path arrives with flags that
546+// differ from the originating workspace. The existing workspace wins;
547+// the incoming flags are silently ignored.
548+//
549+// The comparison is done against the incoming args as the caller sent
550+// them — including empty/zero values — rather than after defaulting.
551+// This means that, for example, a second caller who omits DataDir
552+// while the first set one will still log the mismatch.
553+func logFirstWinsMismatch(existing *Workspace, args proto.Workspace) {
554+ existingCfg := existing.Cfg.Config()
555+ existingYOLO := existing.Cfg.Overrides().SkipPermissionRequests
556+ if existingYOLO == args.YOLO &&
557+ existingCfg.Options.Debug == args.Debug &&
558+ existingCfg.Options.DataDirectory == args.DataDir &&
559+ stringSlicesEqual(existing.Env, args.Env) {
560+ return
561+ }
562+ slog.Debug(
563+ "Workspace flag mismatch on duplicate create; first wins",
564+ "workspace_id", existing.ID,
565+ "path", existing.Path,
566+ "existing_yolo", existingYOLO,
567+ "requested_yolo", args.YOLO,
568+ "existing_debug", existingCfg.Options.Debug,
569+ "requested_debug", args.Debug,
570+ "existing_data_dir", existingCfg.Options.DataDirectory,
571+ "requested_data_dir", args.DataDir,
572+ "existing_env", existing.Env,
573+ "requested_env", args.Env,
574+ )
575+}
576+
577+// stringSlicesEqual reports whether a and b contain the same strings
578+// in the same order. nil and empty are treated as equal.
579+func stringSlicesEqual(a, b []string) bool {
580+ if len(a) != len(b) {
581+ return false
582+ }
583+ for i := range a {
584+ if a[i] != b[i] {
585+ return false
586+ }
205 }
587 }
588+ return true
206}
589}
internal/backend/backend_test.gonew file+953
@@ -0,0 +1,953 @@
1+package backend
2+
3+import (
4+ "bytes"
5+ "context"
6+ "errors"
7+ "log/slog"
8+ "os"
9+ "path/filepath"
10+ "runtime"
11+ "strings"
12+ "sync"
13+ "sync/atomic"
14+ "testing"
15+ "time"
16+
17+ "github.com/charmbracelet/crush/internal/csync"
18+ "github.com/charmbracelet/crush/internal/proto"
19+ "github.com/google/uuid"
20+ "github.com/stretchr/testify/require"
21+)
22+
23+// newTestBackend returns a Backend whose teardown path skips any
24+// real [app.App] shutdown work. Useful for state-machine tests that
25+// install synthetic workspaces directly via insertTestWorkspace.
26+func newTestBackend(t *testing.T) (*Backend, *atomic.Int32) {
27+ t.Helper()
28+ var shutdownCount atomic.Int32
29+ b := &Backend{
30+ workspaces: csync.NewMap[string, *Workspace](),
31+ pathIndex: make(map[string]string),
32+ ctx: context.Background(),
33+ createGrace: 50 * time.Millisecond,
34+ shutdownFn: func() { shutdownCount.Add(1) },
35+ }
36+ return b, &shutdownCount
37+}
38+
39+// insertTestWorkspace installs a synthetic workspace into b at the
40+// given resolved path. Its shutdownFn is recorded in the returned
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
57+}
58+
59+func newClientID(t *testing.T) string {
60+ t.Helper()
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)
72+ require.NoError(t, err)
73+ require.Equal(t, real, got)
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")
177+}
178+
179+func TestDetachClient_LastStreamTearsDown(t *testing.T) {
180+ t.Parallel()
181+
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)
353+ }
354+ }
355+}
356+
357+// TestConcurrentAttachDetach exercises the state machine under
358+// parallel attach/detach pressure with the race detector.
359+func TestConcurrentAttachDetach(t *testing.T) {
360+ t.Parallel()
361+
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) {
627+ t.Helper()
628+ for _, ws := range b.workspaces.Seq2() {
629+ ws.clientsMu.Lock()
630+ ids := make([]string, 0, len(ws.clients))
631+ for id := range ws.clients {
632+ ids = append(ids, id)
633+ }
634+ ws.clientsMu.Unlock()
635+ for _, cid := range ids {
636+ _ = b.releaseHold(ws.ID, cid)
637+ }
638+ }
639+}
640+
641+func protoWS(path, dataDir, clientID string) proto.Workspace {
642+ return proto.Workspace{Path: path, DataDir: dataDir, ClientID: clientID}
643+}
644+
645+// captureDebugLogs installs a buffer-backed slog handler at Debug
646+// level for the duration of the test, returning the buffer. The
647+// previous default handler is restored via t.Cleanup.
648+func captureDebugLogs(t *testing.T) *bytes.Buffer {
649+ t.Helper()
650+ var buf bytes.Buffer
651+ prev := slog.Default()
652+ handler := slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug})
653+ slog.SetDefault(slog.New(handler))
654+ t.Cleanup(func() { slog.SetDefault(prev) })
655+ return &buf
656+}
657+
658+// xdgIsolated points HOME and XDG_* variables at fresh tempdirs so
659+// CreateWorkspace's config loading does not interfere with the host
660+// machine's real config.
661+func xdgIsolated(t *testing.T) {
662+ t.Helper()
663+ t.Setenv("HOME", t.TempDir())
664+ t.Setenv("XDG_CACHE_HOME", t.TempDir())
665+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
666+ t.Setenv("XDG_DATA_HOME", t.TempDir())
667+}
668+
669+// TestFirstWinsMismatch_LogsOnFlagDifferences verifies that the
670+// debug mismatch line is emitted when any of YOLO, Debug, DataDir,
671+// or Env differs between the first and second CreateWorkspace at
672+// the same path, and that the existing workspace's Debug flag is
673+// not overwritten.
674+func TestFirstWinsMismatch_LogsOnFlagDifferences(t *testing.T) {
675+ tests := []struct {
676+ name string
677+ mutate func(*proto.Workspace)
678+ }{
679+ {
680+ name: "yolo",
681+ mutate: func(p *proto.Workspace) { p.YOLO = true },
682+ },
683+ {
684+ name: "debug",
685+ mutate: func(p *proto.Workspace) { p.Debug = true },
686+ },
687+ {
688+ name: "datadir",
689+ mutate: func(p *proto.Workspace) { p.DataDir = "" },
690+ },
691+ {
692+ name: "env",
693+ mutate: func(p *proto.Workspace) { p.Env = []string{"NEW=val"} },
694+ },
695+ }
696+
697+ for _, tc := range tests {
698+ t.Run(tc.name, func(t *testing.T) {
699+ xdgIsolated(t)
700+ cwd := t.TempDir()
701+ dataDir := t.TempDir()
702+
703+ buf := captureDebugLogs(t)
704+ b := New(context.Background(), nil, func() {})
705+ b.SetCreateGrace(2 * time.Second)
706+ t.Cleanup(func() { drainBackend(t, b) })
707+
708+ argsA := protoWS(cwd, dataDir, uuid.New().String())
709+ argsA.Env = []string{"FOO=bar"}
710+ wsA, _, err := b.CreateWorkspace(argsA)
711+ require.NoError(t, err)
712+ originalDebug := wsA.Cfg.Config().Options.Debug
713+ originalYOLO := wsA.Cfg.Overrides().SkipPermissionRequests
714+
715+ argsB := protoWS(cwd, dataDir, uuid.New().String())
716+ argsB.Env = []string{"FOO=bar"} // identical by default
717+ tc.mutate(&argsB)
718+ _, _, err = b.CreateWorkspace(argsB)
719+ require.NoError(t, err)
720+
721+ require.Contains(
722+ t, buf.String(),
723+ "Workspace flag mismatch on duplicate create",
724+ "expected debug log for mismatching %s", tc.name,
725+ )
726+ // Existing workspace's YOLO and Debug must not change.
727+ require.Equal(t, originalYOLO, wsA.Cfg.Overrides().SkipPermissionRequests, "YOLO must be immutable on first-wins")
728+ require.Equal(t, originalDebug, wsA.Cfg.Config().Options.Debug, "Debug must be immutable on first-wins")
729+ })
730+ }
731+}
732+
733+// TestFirstWinsMismatch_NoLogWhenIdentical confirms identical args
734+// do not emit the mismatch log line.
735+func TestFirstWinsMismatch_NoLogWhenIdentical(t *testing.T) {
736+ xdgIsolated(t)
737+ cwd := t.TempDir()
738+ dataDir := t.TempDir()
739+
740+ buf := captureDebugLogs(t)
741+ b := New(context.Background(), nil, func() {})
742+ b.SetCreateGrace(2 * time.Second)
743+ t.Cleanup(func() { drainBackend(t, b) })
744+
745+ argsA := protoWS(cwd, dataDir, uuid.New().String())
746+ argsA.Env = []string{"FOO=bar"}
747+ _, _, err := b.CreateWorkspace(argsA)
748+ require.NoError(t, err)
749+
750+ argsB := protoWS(cwd, dataDir, uuid.New().String())
751+ argsB.Env = []string{"FOO=bar"}
752+ _, _, err = b.CreateWorkspace(argsB)
753+ require.NoError(t, err)
754+
755+ require.False(t,
756+ strings.Contains(buf.String(), "Workspace flag mismatch on duplicate create"),
757+ "identical args must not log a mismatch: %s", buf.String())
758+}
759+
760+// TestRaceTwoClientsAttachOneDetaches exercises the PLAN-required
761+// race scenario: two clients attach concurrently, then one detaches.
762+// The workspace must remain alive with refcount==1 and the clients
763+// map must reflect the remaining client only.
764+func TestRaceTwoClientsAttachOneDetaches(t *testing.T) {
765+ t.Parallel()
766+
767+ b, _ := newTestBackend(t)
768+ ws, shutdowns := insertTestWorkspace(t, b, "/tmp/race-two")
769+
770+ cidA := newClientID(t)
771+ cidB := newClientID(t)
772+
773+ var wg sync.WaitGroup
774+ wg.Add(2)
775+ go func() {
776+ defer wg.Done()
777+ require.NoError(t, b.AttachClient(ws.ID, cidA))
778+ }()
779+ go func() {
780+ defer wg.Done()
781+ require.NoError(t, b.AttachClient(ws.ID, cidB))
782+ }()
783+ wg.Wait()
784+
785+ ws.clientsMu.Lock()
786+ require.Len(t, ws.clients, 2, "both clients must be attached")
787+ ws.clientsMu.Unlock()
788+
789+ b.DetachClient(ws.ID, cidA)
790+
791+ ws.clientsMu.Lock()
792+ require.Len(t, ws.clients, 1, "refcount must be 1 after one detach")
793+ require.Contains(t, ws.clients, cidB, "remaining client must be cidB")
794+ require.NotContains(t, ws.clients, cidA, "detached client must be removed")
795+ ws.clientsMu.Unlock()
796+ require.Equal(t, int32(0), shutdowns.Load(), "workspace must remain alive")
797+
798+ // Drain.
799+ b.DetachClient(ws.ID, cidB)
800+ require.Equal(t, int32(1), shutdowns.Load())
801+}
802+
803+// TestExplicitDeleteThenAttach reproduces the PLAN scenario: start
804+// with a real hold, releaseHold consumes it, AttachClient from the
805+// same clientID creates a fresh entry with streams==1, and calling
806+// releaseHold again is a no-op. A second client keeps the workspace
807+// alive so AttachClient can still resolve the workspace ID after the
808+// first client's hold is released.
809+func TestExplicitDeleteThenAttach(t *testing.T) {
810+ t.Parallel()
811+
812+ // Large grace window so timers cannot fire during the test
813+ // — we want to exercise the explicit releaseHold path.
814+ b, _ := newTestBackend(t)
815+ b.createGrace = time.Hour
816+ ws, shutdowns := insertTestWorkspace(t, b, "/tmp/delete-then-attach")
817+
818+ // Anchor client keeps the workspace registered in
819+ // b.workspaces across the cid's releaseHold below.
820+ anchor := newClientID(t)
821+ require.NoError(t, b.AttachClient(ws.ID, anchor))
822+
823+ cid := newClientID(t)
824+ // Real hold via registerClient (mirrors CreateWorkspace).
825+ b.registerClient(ws, cid)
826+ ws.clientsMu.Lock()
827+ require.Contains(t, ws.clients, cid)
828+ require.NotNil(t, ws.clients[cid].holdTimer, "hold must be live")
829+ require.Equal(t, 0, ws.clients[cid].streams)
830+ ws.clientsMu.Unlock()
831+
832+ // releaseHold: consumes the hold and removes the entry
833+ // (streams == 0). The anchor client keeps the workspace
834+ // alive.
835+ require.NoError(t, b.releaseHold(ws.ID, cid))
836+ require.Equal(t, int32(0), shutdowns.Load(), "anchor must keep workspace alive")
837+ ws.clientsMu.Lock()
838+ require.NotContains(t, ws.clients, cid, "entry must be removed by releaseHold")
839+ ws.clientsMu.Unlock()
840+
841+ // AttachClient creates a fresh entry with streams==1 and no
842+ // hold timer.
843+ require.NoError(t, b.AttachClient(ws.ID, cid))
844+ ws.clientsMu.Lock()
845+ require.Contains(t, ws.clients, cid, "fresh entry must be created")
846+ require.Equal(t, 1, ws.clients[cid].streams, "fresh attach must start at streams=1")
847+ require.Nil(t, ws.clients[cid].holdTimer, "fresh attach must have no hold timer")
848+ ws.clientsMu.Unlock()
849+
850+ // Calling releaseHold again is a no-op (no hold timer to
851+ // stop, streams > 0 so the entry stays).
852+ require.NoError(t, b.releaseHold(ws.ID, cid))
853+ ws.clientsMu.Lock()
854+ require.Contains(t, ws.clients, cid, "releaseHold must not touch a stream-only entry")
855+ require.Equal(t, 1, ws.clients[cid].streams)
856+ require.Nil(t, ws.clients[cid].holdTimer)
857+ ws.clientsMu.Unlock()
858+
859+ // Drain.
860+ b.DetachClient(ws.ID, cid)
861+ b.DetachClient(ws.ID, anchor)
862+ require.Equal(t, int32(1), shutdowns.Load())
863+}
864+
865+// TestAttachClient_RacesWithTeardown forces AttachClient to compete
866+// with the teardown path triggered by DetachClient. Before the fix,
867+// AttachClient could observe a workspace after teardown had already
868+// decided to remove it (because AttachClient did not synchronize with
869+// Backend.mu), leaving a live stream claim attached to a workspace
870+// that was then removed and shut down. With the fix, the outcome must
871+// be deterministic: either AttachClient won and the workspace is
872+// alive with the client registered, or teardown won and AttachClient
873+// returns ErrWorkspaceNotFound — never a half-state where the
874+// workspace is gone but ws.clients still contains the new client.
875+func TestAttachClient_RacesWithTeardown(t *testing.T) {
876+ t.Parallel()
877+
878+ for i := range 200 {
879+ b, _ := newTestBackend(t)
880+ // Keep the grace window long so it can't fire during the
881+ // test and confuse the bookkeeping.
882+ b.createGrace = time.Hour
883+ ws, shutdowns := insertTestWorkspace(t, b, "/tmp/race-teardown")
884+
885+ // Seed: cidA holds the workspace open via a stream. The
886+ // imminent DetachClient(cidA) will be the *only* claim
887+ // drop, so teardown will run.
888+ cidA := newClientID(t)
889+ require.NoError(t, b.AttachClient(ws.ID, cidA))
890+
891+ // cidB attempts to attach concurrently with the detach
892+ // that will tear the workspace down.
893+ cidB := newClientID(t)
894+ start := make(chan struct{})
895+ errCh := make(chan error, 1)
896+ detachDone := make(chan struct{})
897+ go func() {
898+ <-start
899+ errCh <- b.AttachClient(ws.ID, cidB)
900+ }()
901+ go func() {
902+ <-start
903+ b.DetachClient(ws.ID, cidA)
904+ close(detachDone)
905+ }()
906+ close(start)
907+
908+ // Wait for both goroutines so teardown (including
909+ // shutdownFn) has fully run before we read state.
910+ attachErr := <-errCh
911+ <-detachDone
912+
913+ _, wsStillRegistered := b.workspaces.Get(ws.ID)
914+ ws.clientsMu.Lock()
915+ _, hasA := ws.clients[cidA]
916+ _, hasB := ws.clients[cidB]
917+ clientCount := len(ws.clients)
918+ ws.clientsMu.Unlock()
919+ shutdownCount := shutdowns.Load()
920+
921+ switch {
922+ case attachErr == nil:
923+ // AttachClient won. The workspace must be alive
924+ // (registered) with cidB in its clients map. cidA
925+ // may or may not still be there depending on who
926+ // took clientsMu first, but the workspace must
927+ // not have been torn down.
928+ require.True(t, wsStillRegistered,
929+ "iter %d: attach succeeded but workspace was removed", i)
930+ require.True(t, hasB,
931+ "iter %d: attach succeeded but cidB missing from clients", i)
932+ require.Equal(t, int32(0), shutdownCount,
933+ "iter %d: attach succeeded but workspace was shut down", i)
934+ case errors.Is(attachErr, ErrWorkspaceNotFound):
935+ // Teardown won. The workspace must be removed,
936+ // shut down exactly once, and ws.clients must be
937+ // empty (no half-state with cidB inserted into a
938+ // dead workspace's clients map).
939+ require.False(t, wsStillRegistered,
940+ "iter %d: ErrWorkspaceNotFound but workspace still registered", i)
941+ require.Equal(t, int32(1), shutdownCount,
942+ "iter %d: ErrWorkspaceNotFound but shutdown count = %d", i, shutdownCount)
943+ require.False(t, hasA,
944+ "iter %d: teardown won but cidA still in clients", i)
945+ require.False(t, hasB,
946+ "iter %d: teardown won but cidB still in clients (would be the leaked attach)", i)
947+ require.Zero(t, clientCount,
948+ "iter %d: teardown won but clients map is non-empty", i)
949+ default:
950+ t.Fatalf("iter %d: unexpected AttachClient error: %v", i, attachErr)
951+ }
952+ }
953+}
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+5−2
@@ -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 }
@@ -87,8 +89,9 @@ func (c *Client) DeleteWorkspace(ctx context.Context, id string) error {
87// SubscribeEvents subscribes to server-sent events for a workspace.
89// SubscribeEvents subscribes to server-sent events for a workspace.
88func (c *Client) SubscribeEvents(ctx context.Context, id string) (<-chan any, error) {
90func (c *Client) SubscribeEvents(ctx context.Context, id string) (<-chan any, error) {
89 events := make(chan any, 100)
91 events := make(chan any, 100)
92+ q := url.Values{"client_id": []string{c.clientID}}
90 //nolint:bodyclose
93 //nolint:bodyclose
91− rsp, err := c.get(ctx, fmt.Sprintf("/workspaces/%s/events", id), nil, http.Header{
94+ rsp, err := c.get(ctx, fmt.Sprintf("/workspaces/%s/events", id), q, http.Header{
92 "Accept": []string{"text/event-stream"},
95 "Accept": []string{"text/event-stream"},
93 "Cache-Control": []string{"no-cache"},
96 "Cache-Control": []string{"no-cache"},
94 "Connection": []string{"keep-alive"},
97 "Connection": []string{"keep-alive"},
internal/proto/proto.go+9−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}
25}
25
26
26// Error represents an error response.
27// Error represents an error response.
internal/server/multiclient_test.gonew file+107
@@ -0,0 +1,107 @@
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/stretchr/testify/require"
14+)
15+
16+// newTestController builds a controllerV1 around a backend without a
17+// real config store, suitable for handler-level 400 tests.
18+func newTestController() *controllerV1 {
19+ s := &Server{}
20+ s.backend = backend.New(context.Background(), nil, nil)
21+ return &controllerV1{backend: s.backend, server: s}
22+}
23+
24+func TestPostWorkspaces_RejectsMissingClientID(t *testing.T) {
25+ t.Parallel()
26+ c := newTestController()
27+
28+ body, err := json.Marshal(proto.Workspace{Path: t.TempDir()})
29+ require.NoError(t, err)
30+ req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/v1/workspaces", bytes.NewReader(body))
31+ req.Header.Set("Content-Type", "application/json")
32+ rec := httptest.NewRecorder()
33+
34+ c.handlePostWorkspaces(rec, req)
35+
36+ require.Equal(t, http.StatusBadRequest, rec.Code)
37+ var perr proto.Error
38+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &perr))
39+ require.Contains(t, perr.Message, "client_id")
40+}
41+
42+func TestPostWorkspaces_RejectsMalformedClientID(t *testing.T) {
43+ t.Parallel()
44+ c := newTestController()
45+
46+ body, err := json.Marshal(proto.Workspace{Path: t.TempDir(), ClientID: "not-a-uuid"})
47+ require.NoError(t, err)
48+ req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/v1/workspaces", bytes.NewReader(body))
49+ req.Header.Set("Content-Type", "application/json")
50+ rec := httptest.NewRecorder()
51+
52+ c.handlePostWorkspaces(rec, req)
53+
54+ require.Equal(t, http.StatusBadRequest, rec.Code)
55+}
56+
57+func TestDeleteWorkspace_RejectsMissingClientID(t *testing.T) {
58+ t.Parallel()
59+ c := newTestController()
60+
61+ req := httptest.NewRequestWithContext(t.Context(), http.MethodDelete, "/v1/workspaces/abc", nil)
62+ req.SetPathValue("id", "abc")
63+ rec := httptest.NewRecorder()
64+
65+ c.handleDeleteWorkspaces(rec, req)
66+
67+ require.Equal(t, http.StatusBadRequest, rec.Code)
68+}
69+
70+func TestDeleteWorkspace_RejectsMalformedClientID(t *testing.T) {
71+ t.Parallel()
72+ c := newTestController()
73+
74+ req := httptest.NewRequestWithContext(t.Context(), http.MethodDelete, "/v1/workspaces/abc?client_id=nope", nil)
75+ req.SetPathValue("id", "abc")
76+ rec := httptest.NewRecorder()
77+
78+ c.handleDeleteWorkspaces(rec, req)
79+
80+ require.Equal(t, http.StatusBadRequest, rec.Code)
81+}
82+
83+func TestSubscribeEvents_RejectsMissingClientID(t *testing.T) {
84+ t.Parallel()
85+ c := newTestController()
86+
87+ req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/v1/workspaces/abc/events", nil)
88+ req.SetPathValue("id", "abc")
89+ rec := httptest.NewRecorder()
90+
91+ c.handleGetWorkspaceEvents(rec, req)
92+
93+ require.Equal(t, http.StatusBadRequest, rec.Code)
94+}
95+
96+func TestSubscribeEvents_RejectsMalformedClientID(t *testing.T) {
97+ t.Parallel()
98+ c := newTestController()
99+
100+ req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/v1/workspaces/abc/events?client_id=nope", nil)
101+ req.SetPathValue("id", "abc")
102+ rec := httptest.NewRecorder()
103+
104+ c.handleGetWorkspaceEvents(rec, req)
105+
106+ require.Equal(t, http.StatusBadRequest, rec.Code)
107+}
internal/server/proto.go+37−1
@@ -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,23 @@ 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+
136// handleDeleteWorkspaces deletes a workspace.
154// handleDeleteWorkspaces deletes a workspace.
137//
155//
138// @Summary Delete workspace
156// @Summary Delete workspace
@@ -143,7 +161,14 @@ func (c *controllerV1) handlePostWorkspaces(w http.ResponseWriter, r *http.Reque
143// @Router /workspaces/{id} [delete]
161// @Router /workspaces/{id} [delete]
144func (c *controllerV1) handleDeleteWorkspaces(w http.ResponseWriter, r *http.Request) {
162func (c *controllerV1) handleDeleteWorkspaces(w http.ResponseWriter, r *http.Request) {
145 id := r.PathValue("id")
163 id := r.PathValue("id")
164+ clientID, ok := c.requireClientID(w, r)
165+ if !ok {
166+ return
167+ }
146− c.backend.DeleteWorkspace(id)
168+ if err := c.backend.DeleteWorkspace(id, clientID); err != nil {
169+ c.handleError(w, r, err)
170+ return
171+ }
147}
172}
148
173
149// handleGetWorkspaceConfig returns workspace configuration.
174// handleGetWorkspaceConfig returns workspace configuration.
@@ -199,6 +224,15 @@ func (c *controllerV1) handleGetWorkspaceProviders(w http.ResponseWriter, r *htt
199func (c *controllerV1) handleGetWorkspaceEvents(w http.ResponseWriter, r *http.Request) {
224func (c *controllerV1) handleGetWorkspaceEvents(w http.ResponseWriter, r *http.Request) {
200 flusher := http.NewResponseController(w)
225 flusher := http.NewResponseController(w)
201 id := r.PathValue("id")
226 id := r.PathValue("id")
227+ clientID, ok := c.requireClientID(w, r)
228+ if !ok {
229+ return
230+ }
231+ if err := c.backend.AttachClient(id, clientID); err != nil {
232+ c.handleError(w, r, err)
233+ return
234+ }
235+ defer c.backend.DetachClient(id, clientID)
202 events, err := c.backend.SubscribeEvents(r.Context(), id)
236 events, err := c.backend.SubscribeEvents(r.Context(), id)
203 if err != nil {
237 if err != nil {
204 c.handleError(w, r, err)
238 c.handleError(w, r, err)
@@ -951,6 +985,8 @@ func (c *controllerV1) handleError(w http.ResponseWriter, r *http.Request, err e
951 status = http.StatusBadRequest
985 status = http.StatusBadRequest
952 case errors.Is(err, backend.ErrUnknownCommand):
986 case errors.Is(err, backend.ErrUnknownCommand):
953 status = http.StatusBadRequest
987 status = http.StatusBadRequest
988+ case errors.Is(err, backend.ErrInvalidClientID):
989+ status = http.StatusBadRequest
954 }
990 }
955 c.server.logError(r, err.Error())
991 c.server.logError(r, err.Error())
956 jsonError(w, status, err.Error())
992 jsonError(w, status, err.Error())