diff --git a/cmd/wsh/cmd/wshcmd-blocks.go b/cmd/wsh/cmd/wshcmd-blocks.go index 7e4b935ee3..3414084da9 100644 --- a/cmd/wsh/cmd/wshcmd-blocks.go +++ b/cmd/wsh/cmd/wshcmd-blocks.go @@ -106,7 +106,7 @@ func blocksListRun(cmd *cobra.Command, args []string) error { var allBlocks []BlockDetails - workspaces, err := wshclient.WorkspaceListCommand(RpcClient, &wshrpc.RpcOpts{Timeout: int64(blocksTimeout)}) + workspaces, err := wshclient.WorkspaceListAllCommand(RpcClient, &wshrpc.RpcOpts{Timeout: int64(blocksTimeout)}) if err != nil { return fmt.Errorf("failed to list workspaces: %v", err) } diff --git a/cmd/wsh/cmd/wshcmd-workspace.go b/cmd/wsh/cmd/wshcmd-workspace.go index 6a793d68cf..b6ec198e6b 100644 --- a/cmd/wsh/cmd/wshcmd-workspace.go +++ b/cmd/wsh/cmd/wshcmd-workspace.go @@ -28,7 +28,7 @@ var workspaceListCommand = &cobra.Command{ } func workspaceListRun(cmd *cobra.Command, args []string) { - workspaces, err := wshclient.WorkspaceListCommand(RpcClient, &wshrpc.RpcOpts{Timeout: 2000}) + workspaces, err := wshclient.WorkspaceListAllCommand(RpcClient, &wshrpc.RpcOpts{Timeout: 2000}) if err != nil { WriteStderr("Unable to list workspaces: %v\n", err) return diff --git a/frontend/app/store/wshclientapi.ts b/frontend/app/store/wshclientapi.ts index 8482be260d..5dece08473 100644 --- a/frontend/app/store/wshclientapi.ts +++ b/frontend/app/store/wshclientapi.ts @@ -1032,6 +1032,12 @@ export class RpcApiType { return client.wshRpcCall("workspacelist", null, opts); } + // command "workspacelistall" [call] + WorkspaceListAllCommand(client: WshClient, opts?: RpcOpts): Promise { + if (this.mockClient) return this.mockClient.mockWshRpcCall(client, "workspacelistall", null, opts); + return client.wshRpcCall("workspacelistall", null, opts); + } + // command "writeappfile" [call] WriteAppFileCommand(client: WshClient, data: CommandWriteAppFileData, opts?: RpcOpts): Promise { if (this.mockClient) return this.mockClient.mockWshRpcCall(client, "writeappfile", data, opts); diff --git a/pkg/wcore/workspace.go b/pkg/wcore/workspace.go index c01e509a13..daccd2ec7f 100644 --- a/pkg/wcore/workspace.go +++ b/pkg/wcore/workspace.go @@ -383,7 +383,32 @@ func UpdateWorkspaceTabIds(ctx context.Context, workspaceId string, tabIds []str return nil } +// ListWorkspaces returns only "saved" workspaces (Name, Icon, and Color all +// set) - the ones worth offering in the cross-window switcher. CreateWindow +// deliberately creates a blank, unsaved scratch workspace for a new window +// (CreateWorkspace with applyDefaults=false; see CreateWindow), and +// DeleteWorkspace auto-cleans one of those up on close unless it's since +// been named. Excluding unsaved workspaces here is that same intentional +// lifecycle, not a data-completeness bug - do not backfill or persist +// defaults into them, that would silently and permanently "save" scratch +// workspaces the user never asked to keep, defeating DeleteWorkspace's +// cleanup and orphaning them. func ListWorkspaces(ctx context.Context) (waveobj.WorkspaceList, error) { + return listWorkspacesInternal(ctx, false) +} + +// ListAllWorkspaces includes unsaved (scratch) workspaces too - CLI tooling +// (wsh workspace list, wsh blocks list) needs visibility into every live +// workspace's tabs and blocks regardless of whether the user has gotten +// around to naming it, unlike the switcher's "workspaces you'd want to +// jump to" framing. A confirmed real case: a workspace holding a +// continuously-used session's own tab, never named, was completely +// invisible to wsh blocks list because of this filter. +func ListAllWorkspaces(ctx context.Context) (waveobj.WorkspaceList, error) { + return listWorkspacesInternal(ctx, true) +} + +func listWorkspacesInternal(ctx context.Context, includeUnsaved bool) (waveobj.WorkspaceList, error) { workspaces, err := wstore.DBGetAllObjsByType[*waveobj.Workspace](ctx, waveobj.OType_Workspace) if err != nil { return nil, err @@ -399,7 +424,7 @@ func ListWorkspaces(ctx context.Context) (waveobj.WorkspaceList, error) { var wl waveobj.WorkspaceList for _, workspace := range workspaces { - if workspace.Name == "" || workspace.Icon == "" || workspace.Color == "" { + if !includeUnsaved && (workspace.Name == "" || workspace.Icon == "" || workspace.Color == "") { continue } windowId, ok := workspaceToWindow[workspace.OID] diff --git a/pkg/wcore/workspace_test.go b/pkg/wcore/workspace_test.go new file mode 100644 index 0000000000..643efa67b8 --- /dev/null +++ b/pkg/wcore/workspace_test.go @@ -0,0 +1,164 @@ +// Copyright 2026, Command Line Inc. +// SPDX-License-Identifier: Apache-2.0 +package wcore + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/google/uuid" + "github.com/wavetermdev/waveterm/pkg/wavebase" + "github.com/wavetermdev/waveterm/pkg/waveobj" + "github.com/wavetermdev/waveterm/pkg/wstore" +) + +// initTestWStore points wstore at a fresh, real sqlite DB (real migrations, +// real driver - not mocked) under t.TempDir() so each test gets an isolated +// store. Creates the db subdirectory directly rather than via +// wavebase.EnsureWaveDBDir(), which caches success process-wide by a fixed +// key - fine for a real single-lifetime process, but it would silently +// no-op for every test after the first, each pointed at its own fresh +// (not-yet-existing) temp dir. +func initTestWStore(t *testing.T) context.Context { + t.Helper() + dataDir := t.TempDir() + wavebase.DataHome_VarCache = dataDir + if err := os.MkdirAll(filepath.Join(dataDir, wavebase.WaveDBDir), 0700); err != nil { + t.Fatalf("failed to create wave db dir: %v", err) + } + if err := wstore.InitWStore(); err != nil { + t.Fatalf("failed to init wstore: %v", err) + } + return context.Background() +} + +func containsWorkspaceId(list waveobj.WorkspaceList, id string) bool { + for _, entry := range list { + if entry.WorkspaceId == id { + return true + } + } + return false +} + +// TestListWorkspaces_ExcludesUnsavedWithoutMutating confirms ListWorkspaces +// keeps excluding an unsaved (blank Name/Icon/Color) workspace - this is the +// intentional CreateWindow/DeleteWorkspace scratch-workspace lifecycle +// (blank workspaces are deliberately unnamed until a user "saves" them, and +// auto-cleaned up on window close otherwise), not the bug. It must also +// leave the unsaved workspace's stored fields completely untouched - no +// backfill, no persistence - since writing defaults into it would silently +// convert it into a "saved" workspace and break that cleanup lifecycle. +func TestListWorkspaces_ExcludesUnsavedWithoutMutating(t *testing.T) { + ctx := initTestWStore(t) + + saved := &waveobj.Workspace{ + OID: uuid.NewString(), + Name: "Workspace1", + Icon: "flask", + Color: "#FF453A", + TabIds: []string{uuid.NewString()}, + } + if err := wstore.DBInsert(ctx, saved); err != nil { + t.Fatalf("failed to insert saved workspace: %v", err) + } + + unsavedTabId := uuid.NewString() + unsaved := &waveobj.Workspace{ + OID: uuid.NewString(), + Name: "", + Icon: "", + Color: "", + TabIds: []string{unsavedTabId}, + } + if err := wstore.DBInsert(ctx, unsaved); err != nil { + t.Fatalf("failed to insert unsaved workspace: %v", err) + } + + list, err := ListWorkspaces(ctx) + if err != nil { + t.Fatalf("ListWorkspaces failed: %v", err) + } + if len(list) != 1 || !containsWorkspaceId(list, saved.OID) { + t.Fatalf("expected only the saved workspace, got %+v", list) + } + if containsWorkspaceId(list, unsaved.OID) { + t.Fatalf("unsaved workspace must stay excluded from ListWorkspaces (switcher-facing)") + } + + // The unsaved workspace's stored fields must be byte-for-byte untouched + // by the call - no backfill, no persisted mutation of any kind. + refetched, err := wstore.DBMustGet[*waveobj.Workspace](ctx, unsaved.OID) + if err != nil { + t.Fatalf("failed to refetch unsaved workspace: %v", err) + } + if refetched.Name != "" || refetched.Icon != "" || refetched.Color != "" { + t.Fatalf("ListWorkspaces must not persist any backfill into an unsaved workspace, got name=%q icon=%q color=%q", + refetched.Name, refetched.Icon, refetched.Color) + } + if len(refetched.TabIds) != 1 || refetched.TabIds[0] != unsavedTabId { + t.Fatalf("ListWorkspaces must not disturb existing fields, got TabIds=%v", refetched.TabIds) + } +} + +// TestListAllWorkspaces_IncludesUnsavedWithoutMutating is the actual fix: +// CLI tooling (wsh workspace list, wsh blocks list) needs visibility into +// every live workspace regardless of saved status - confirmed against a +// real user's database, where an unsaved workspace held a continuously-used +// session's own tab, completely invisible to those commands. It must +// include the unsaved workspace, but - just like ListWorkspaces - must +// never write anything into it; only the caller decides whether/how to +// display an unnamed workspace. +func TestListAllWorkspaces_IncludesUnsavedWithoutMutating(t *testing.T) { + ctx := initTestWStore(t) + + saved := &waveobj.Workspace{ + OID: uuid.NewString(), + Name: "Workspace1", + Icon: "flask", + Color: "#FF453A", + TabIds: []string{uuid.NewString()}, + } + if err := wstore.DBInsert(ctx, saved); err != nil { + t.Fatalf("failed to insert saved workspace: %v", err) + } + + unsavedTabId := uuid.NewString() + unsaved := &waveobj.Workspace{ + OID: uuid.NewString(), + Name: "", + Icon: "", + Color: "", + TabIds: []string{unsavedTabId}, + } + if err := wstore.DBInsert(ctx, unsaved); err != nil { + t.Fatalf("failed to insert unsaved workspace: %v", err) + } + + list, err := ListAllWorkspaces(ctx) + if err != nil { + t.Fatalf("ListAllWorkspaces failed: %v", err) + } + if len(list) != 2 { + t.Fatalf("expected both workspaces (the unsaved one must now be visible to CLI tooling), got %d: %+v", len(list), list) + } + if !containsWorkspaceId(list, saved.OID) || !containsWorkspaceId(list, unsaved.OID) { + t.Fatalf("expected both workspace IDs present, got %+v", list) + } + + // No mutation, same as ListWorkspaces - this function only changes what + // gets included, never what gets written. + refetched, err := wstore.DBMustGet[*waveobj.Workspace](ctx, unsaved.OID) + if err != nil { + t.Fatalf("failed to refetch unsaved workspace: %v", err) + } + if refetched.Name != "" || refetched.Icon != "" || refetched.Color != "" { + t.Fatalf("ListAllWorkspaces must not persist any backfill either, got name=%q icon=%q color=%q", + refetched.Name, refetched.Icon, refetched.Color) + } + if len(refetched.TabIds) != 1 || refetched.TabIds[0] != unsavedTabId { + t.Fatalf("ListAllWorkspaces must not disturb existing fields, got TabIds=%v", refetched.TabIds) + } +} diff --git a/pkg/wshrpc/wshclient/wshclient.go b/pkg/wshrpc/wshclient/wshclient.go index d5333aec2b..f2b426db52 100644 --- a/pkg/wshrpc/wshclient/wshclient.go +++ b/pkg/wshrpc/wshclient/wshclient.go @@ -1024,6 +1024,12 @@ func WorkspaceListCommand(w *wshutil.WshRpc, opts *wshrpc.RpcOpts) ([]wshrpc.Wor return resp, err } +// command "workspacelistall", wshserver.WorkspaceListAllCommand +func WorkspaceListAllCommand(w *wshutil.WshRpc, opts *wshrpc.RpcOpts) ([]wshrpc.WorkspaceInfoData, error) { + resp, err := sendRpcRequestCallHelper[[]wshrpc.WorkspaceInfoData](w, "workspacelistall", nil, opts) + return resp, err +} + // command "writeappfile", wshserver.WriteAppFileCommand func WriteAppFileCommand(w *wshutil.WshRpc, data wshrpc.CommandWriteAppFileData, opts *wshrpc.RpcOpts) error { _, err := sendRpcRequestCallHelper[any](w, "writeappfile", data, opts) diff --git a/pkg/wshrpc/wshrpctypes.go b/pkg/wshrpc/wshrpctypes.go index 51e2338ba8..db2c709f71 100644 --- a/pkg/wshrpc/wshrpctypes.go +++ b/pkg/wshrpc/wshrpctypes.go @@ -146,6 +146,13 @@ type WshRpcInterface interface { GetSecretsLinuxStorageBackendCommand(ctx context.Context) (string, error) WorkspaceListCommand(ctx context.Context) ([]WorkspaceInfoData, error) + // WorkspaceListAllCommand is the CLI-only counterpart to + // WorkspaceListCommand: it includes unsaved (scratch) workspaces too. + // WorkspaceListCommand itself must keep excluding them - it's also + // called from emain (Electron Workspace menu, Alt+Ctrl+N workspace + // switching), which relies on that filtering to avoid blank menu + // entries and shortcut slots for scratch workspaces. + WorkspaceListAllCommand(ctx context.Context) ([]WorkspaceInfoData, error) GetUpdateChannelCommand(ctx context.Context) (string, error) // terminal diff --git a/pkg/wshrpc/wshserver/wshserver.go b/pkg/wshrpc/wshserver/wshserver.go index 38006fd9a8..fd5cd50162 100644 --- a/pkg/wshrpc/wshserver/wshserver.go +++ b/pkg/wshrpc/wshserver/wshserver.go @@ -945,7 +945,29 @@ func (ws *WshServer) BlocksListCommand( } func (ws *WshServer) WorkspaceListCommand(ctx context.Context) ([]wshrpc.WorkspaceInfoData, error) { - workspaceList, err := wcore.ListWorkspaces(ctx) + return workspaceListInternal(ctx, false) +} + +// WorkspaceListAllCommand is the CLI-only counterpart backing wsh workspace +// list / wsh blocks list: it includes unsaved (scratch) workspaces too, +// since CLI tooling needs visibility into every live workspace's tabs and +// blocks regardless of whether the user has named it. WorkspaceListCommand +// itself is also called from emain (Electron Workspace menu, Alt+Ctrl+N +// workspace switching), which relies on unsaved workspaces staying +// excluded there to avoid blank menu entries and shortcut slots - so it +// must keep its original behavior unchanged. +func (ws *WshServer) WorkspaceListAllCommand(ctx context.Context) ([]wshrpc.WorkspaceInfoData, error) { + return workspaceListInternal(ctx, true) +} + +func workspaceListInternal(ctx context.Context, includeUnsaved bool) ([]wshrpc.WorkspaceInfoData, error) { + var workspaceList waveobj.WorkspaceList + var err error + if includeUnsaved { + workspaceList, err = wcore.ListAllWorkspaces(ctx) + } else { + workspaceList, err = wcore.ListWorkspaces(ctx) + } if err != nil { return nil, fmt.Errorf("error listing workspaces: %w", err) }