feat(cli): add blocks list with filtering and JSON output #2337
Conversation
|
|
WalkthroughAdds a new "blocks list" feature across CLI, RPC, server, client, and frontend types. Introduces a public Go BlockDetails type and Cobra Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (13)
pkg/wshrpc/wshrpctypes.go (1)
683-686: Add optional TabId to BlocksListRequest and update callersAdd an optional TabId to enable server-side tab filtering and reduce over-fetching; update the request constructors, server handler, and generated frontend types.
Files to update:
- pkg/wshrpc/wshrpctypes.go — BlocksListRequest (type at ~683)
- pkg/wshrpc/wshclient/wshclient.go — BlocksListCommand usage
- pkg/wshrpc/wshserver/wshserver.go — BlocksListCommand implementation: apply TabId filter
- cmd/wsh/cmd/wshcmd-blocks.go — constructors creating BlocksListRequest
- frontend/types/gotypes.d.ts and frontend/app/store/wshclientapi.ts — regenerate/update TS types and ensure tabid is passed where needed
Apply this diff:
type BlocksListRequest struct { WindowId string `json:"windowid,omitempty"` WorkspaceId string `json:"workspaceid,omitempty"` + TabId string `json:"tabid,omitempty"` }frontend/types/gotypes.d.ts (1)
87-101: Add TabId to the request type if backend supports it.Keeps TS types aligned with the proposed Go change.
Apply this diff:
type BlocksListRequest = { windowid?: string; workspaceid?: string; + tabid?: string; };pkg/wshrpc/wshserver/wshserver.go (3)
855-859: Fill WindowId when lookup returns empty.DBFindWindowForWorkspaceId can return "", which yields entries with blank windowid. Prefer a fallback to the request’s WindowId if provided.
Apply this diff:
- windowId, err := wstore.DBFindWindowForWorkspaceId(ctx, wsID) + windowId, err := wstore.DBFindWindowForWorkspaceId(ctx, wsID) if err != nil { log.Printf("error finding window for workspace %s: %v", wsID, err) } + if windowId == "" && req.WindowId != "" { + windowId = req.WindowId + }Also applies to: 870-876
860-878: Be resilient: don’t abort the whole listing on a single missing tab/block.Current code returns error on first missing Tab/Block. Prefer log-and-continue to return partial results.
Apply this diff:
- for _, tabID := range append(wsData.PinnedTabIds, wsData.TabIds...) { + for _, tabID := range append(wsData.PinnedTabIds, wsData.TabIds...) { + // Optional server-side tab filter (if request gains TabId) + if req.TabId != "" && tabID != req.TabId { + continue + } tab, err := wstore.DBMustGet[*waveobj.Tab](ctx, tabID) - if err != nil { - return nil, err - } + if err != nil { + log.Printf("blockslist: skipping tab %s in workspace %s: %v", tabID, wsID, err) + continue + } for _, blkID := range tab.BlockIds { blk, err := wstore.DBMustGet[*waveobj.Block](ctx, blkID) - if err != nil { - return nil, err - } + if err != nil { + log.Printf("blockslist: skipping block %s in tab %s: %v", blkID, tabID, err) + continue + } results = append(results, wshrpc.BlocksListEntry{ WindowId: windowId, WorkspaceId: wsID, TabId: tabID, BlockId: blkID, Meta: blk.Meta, }) } }
825-847: Clarify precedence and “current workspace” resolution.Verified: when both WorkspaceId and WindowId are provided, WorkspaceId is effectively used; “current” is resolved as client.WindowIds[0]. WindowIds ordering is maintained in pkg/wcore/window.go (append at ~line 122, remove at ~156, MoveSliceIdxToFront at ~201–206) and relied on in pkg/wcore/layout.go (uses WindowIds[0]) and pkg/wshrpc/wshserver/wshserver.go (uses client.WindowIds[0]). Either document this precedence/“current” semantics or validate inputs (e.g., return an error when both WorkspaceId and WindowId are set or make the precedence explicit).
cmd/wsh/cmd/wshcmd-blocks.go (8)
54-60: Clarify view flag help to match supported aliases.Expose all accepted aliases so users don’t guess.
- blocksListCmd.Flags().StringVar(&blocksView, "view", "", "restrict to view type (term/terminal, web/browser, preview/edit, sysinfo, waveai)") + blocksListCmd.Flags().StringVar(&blocksView, "view", "", "restrict to view type (terminal|shell|console|term, browser|url|web, preview|edit, sysinfo|system, ai|waveai|assistant)")
123-131: Pass WindowId to RPC when provided to narrow server-side work.You resolve the window to a workspace locally, but the request supports WindowId. Forwarding it reduces ambiguity and helps future server logic.
- req := wshrpc.BlocksListRequest{ - WorkspaceId: wsId, - } + req := wshrpc.BlocksListRequest{ + WorkspaceId: wsId, + } + if blocksWindowId != "" { + req.WindowId = blocksWindowId + }
137-148: “--tab=current” is a no-op; either implement or remove the special-case.The code skips filtering when the value is “current”, but no “current tab” is derived. This silently broadens results.
Option A (remove special-case):
- if blocksTabId != "" && blocksTabId != "current" && b.TabId != blocksTabId { + if blocksTabId != "" && b.TabId != blocksTabId { continue }Option B: implement lookup of the current tab via RPC and filter accordingly.
25-30: Include WindowId in BlockDetails/JSON for completeness and parity with RPC.Helps downstream tooling correlate blocks to windows.
type BlockDetails struct { + WindowId string `json:"windowid"` BlockId string `json:"blockid"` WorkspaceId string `json:"workspaceid"` TabId string `json:"tabid"` Meta waveobj.MetaMapType `json:"meta"` }allBlocks = append(allBlocks, BlockDetails{ + WindowId: b.WindowId, BlockId: b.BlockId, WorkspaceId: b.WorkspaceId, TabId: b.TabId, Meta: b.Meta, })Also applies to: 150-156
171-201: Stabilize and tighten table output (sort, truncate id/content).
- Long IDs/content can break formatting.
- Stable sort improves determinism for tests and UX.
Add sort import and sort before printing:
@@ -import ( +import ( "encoding/json" "fmt" + "sort" "strings" @@ - if len(allBlocks) == 0 { + if len(allBlocks) == 0 { WriteStdout("No blocks found\n") return nil } + // Stable, deterministic order + sort.SliceStable(allBlocks, func(i, j int) bool { + if allBlocks[i].WorkspaceId != allBlocks[j].WorkspaceId { + return allBlocks[i].WorkspaceId < allBlocks[j].WorkspaceId + } + if allBlocks[i].TabId != allBlocks[j].TabId { + return allBlocks[i].TabId < allBlocks[j].TabId + } + return allBlocks[i].BlockId < allBlocks[j].BlockId + }) @@ - for _, b := range allBlocks { + for _, b := range allBlocks { view := b.Meta.GetString(waveobj.MetaKey_View, "<unknown>") var content string @@ - WriteStdout(format, b.BlockId, wsID, tabID, view, content) + blkID := b.BlockId + if len(blkID) > 36 { + blkID = blkID[0:34] + ".." + } + if len(content) > 80 { + content = content[0:78] + ".." + } + WriteStdout(format, blkID, wsID, tabID, view, content) }
38-49: Add a tab-filter example for completeness.Makes the --tab flag discoverable.
# List only terminal blocks wsh blocks list --view=term @@ # Output as JSON for scripting wsh blocks list --json`, + # + # Filter by tab ID + # wsh blocks list --tab=7b6d0e2a-...`,
159-164: Consider privacy: dumping full meta to JSON may leak sensitive fields.Default JSON includes entire meta. Offer a flag like --include-meta (off by default) or strip to a safe subset unless requested.
Also applies to: 171-201
97-121: Make --workspace and --window flags mutually exclusive (or document precedence).Cobra provides MarkFlagsMutuallyExclusive (added in v1.5.0); add this call or explicitly document the precedence in --help. Keep --tab combinable.
@@ blocksListCmd.Flags().BoolVar(&blocksJSON, "json", false, "output as JSON") + // Disambiguate flags to avoid surprising precedence + _ = blocksListCmd.MarkFlagsMutuallyExclusive("workspace", "window")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cmd/wsh/cmd/wshcmd-blocks.go(1 hunks)frontend/app/store/wshclientapi.ts(1 hunks)frontend/types/gotypes.d.ts(1 hunks)pkg/wshrpc/wshclient/wshclient.go(1 hunks)pkg/wshrpc/wshrpctypes.go(3 hunks)pkg/wshrpc/wshserver/wshserver.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
pkg/wshrpc/wshrpctypes.go (3)
frontend/app/store/wshclientapi.ts (1)
BlocksListCommand(36-38)pkg/wshrpc/wshclient/wshclient.go (1)
BlocksListCommand(50-53)pkg/waveobj/metamap.go (1)
MetaMapType(8-8)
frontend/app/store/wshclientapi.ts (1)
pkg/wshrpc/wshrpctypes.go (3)
BlocksListRequest(683-686)RpcOpts(271-277)BlocksListEntry(688-694)
frontend/types/gotypes.d.ts (1)
pkg/wshrpc/wshrpctypes.go (2)
BlocksListEntry(688-694)BlocksListRequest(683-686)
pkg/wshrpc/wshserver/wshserver.go (7)
frontend/app/store/wshclientapi.ts (1)
BlocksListCommand(36-38)pkg/wshrpc/wshclient/wshclient.go (1)
BlocksListCommand(50-53)pkg/wshrpc/wshrpctypes.go (2)
BlocksListRequest(683-686)BlocksListEntry(688-694)pkg/wcore/window.go (1)
GetWindow(72-79)pkg/wstore/wstore_dbops.go (3)
DBGetSingleton(102-105)DBFindWindowForWorkspaceId(396-403)DBMustGet(138-149)pkg/waveobj/wtype.go (6)
Client(125-133)Client(135-137)Tab(183-190)Tab(192-194)Block(279-287)Block(289-291)pkg/wcore/workspace.go (1)
GetWorkspace(186-188)
cmd/wsh/cmd/wshcmd-blocks.go (5)
pkg/waveobj/metamap.go (1)
MetaMapType(8-8)pkg/wshrpc/wshclient/wshclient.go (2)
WorkspaceListCommand(560-563)BlocksListCommand(50-53)pkg/wshrpc/wshrpctypes.go (2)
RpcOpts(271-277)BlocksListRequest(683-686)cmd/wsh/cmd/wshcmd-root.go (2)
WriteStderr(70-72)WriteStdout(74-76)pkg/waveobj/metaconsts.go (4)
MetaKey_View(9-9)MetaKey_File(13-13)MetaKey_Url(15-15)MetaKey_CmdCwd(56-56)
pkg/wshrpc/wshclient/wshclient.go (3)
frontend/app/store/wshclientapi.ts (1)
BlocksListCommand(36-38)pkg/wshutil/wshrpc.go (1)
WshRpc(46-60)pkg/wshrpc/wshrpctypes.go (3)
BlocksListRequest(683-686)RpcOpts(271-277)BlocksListEntry(688-694)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: merge-gatekeeper
🔇 Additional comments (6)
pkg/wshrpc/wshrpctypes.go (3)
70-70: Registering the new RPC command looks good.
200-201: Interface surface addition is consistent with the client and server.
688-695: Entry shape is fine; no changes needed.Meta as map is appropriate for downstream filtering (view, controller, etc.).
frontend/app/store/wshclientapi.ts (1)
35-38: API wiring is correct and matches the RPC name.pkg/wshrpc/wshclient/wshclient.go (1)
49-53: Client wrapper LGTM.Typed helper usage matches existing patterns.
pkg/wshrpc/wshserver/wshserver.go (1)
816-823: Nice addition.Handler signature and summary comment align with the interface and RPC name.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
cmd/wsh/cmd/wshcmd-blocks.go (2)
118-123: Nice: no longer assuming the first workspace is “current”Defaulting to “all workspaces” addresses prior feedback about workspaces[0] being incorrect.
162-169: Good fix: JSON marshal errors are now handledThis addresses earlier feedback about ignored errors during JSON output.
🧹 Nitpick comments (6)
cmd/wsh/cmd/wshcmd-blocks.go (6)
35-58: Add SilenceUsage to suppress noisy usage output on runtime errorsCobra prints the full usage on any returned error. Set SilenceUsage to true for cleaner UX.
var blocksListCmd = &cobra.Command{ Use: "list", Aliases: []string{"ls", "get"}, Short: "List blocks in workspaces/windows", Long: `List blocks with optional filtering by workspace, window, tab, or view type. @@ wsh blocks list --json`, RunE: blocksListRun, PreRunE: preRunSetupRpcClient, + SilenceUsage: true, }
102-124: Mutually exclusive flags: --workspace and --windowWhen both are provided, the code silently prefers --workspace. Fail fast with a clear message.
// Determine which workspaces to query +if blocksWorkspaceId != "" && blocksWindowId != "" { + return fmt.Errorf("--workspace and --window are mutually exclusive; specify only one") +} if blocksWorkspaceId != "":
125-133: Pass WindowId through to the RPC when filtering by windowBlocksListRequest supports WindowId; forwarding it lets the server-side apply the window filter directly and avoids relying on client-side mapping.
for _, wsId := range workspaceIdsToQuery { - req := wshrpc.BlocksListRequest{ - WorkspaceId: wsId, - } + req := wshrpc.BlocksListRequest{WorkspaceId: wsId} + if blocksWindowId != "" { + req.WindowId = blocksWindowId + } blocks, err := wshclient.BlocksListCommand(RpcClient, req, &wshrpc.RpcOpts{Timeout: 5000})
176-205: Truncate BlockId to keep columns aligned%-36s doesn’t truncate; long IDs will break alignment. Trim similarly to TabId.
- for _, b := range allBlocks { + for _, b := range allBlocks { + blockID := b.BlockId + if len(blockID) > 36 { + blockID = blockID[:34] + ".." + } view := b.Meta.GetString(waveobj.MetaKey_View, "<unknown>") @@ - WriteStdout(format, b.BlockId, wsID, tabID, view, content) + WriteStdout(format, blockID, wsID, tabID, view, content) }
176-178: Deterministic ordering for stable outputSort by workspace, tab, then block for reproducible CLI output and easier diffing.
- format := "%-36s %-10s %-36s %-15s %s\n" + // Stable ordering + sort.Slice(allBlocks, func(i, j int) bool { + if allBlocks[i].WorkspaceId != allBlocks[j].WorkspaceId { + return allBlocks[i].WorkspaceId < allBlocks[j].WorkspaceId + } + if allBlocks[i].TabId != allBlocks[j].TabId { + return allBlocks[i].TabId < allBlocks[j].TabId + } + return allBlocks[i].BlockId < allBlocks[j].BlockId + }) + format := "%-36s %-10s %-36s %-15s %s\n"Add the import:
import ( "encoding/json" "fmt" + "sort" "strings"
63-68: Consider a --timeout flag for slow backendsHardcoding 5s may be tight on busy instances. Surface it as a flag with a sane default.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cmd/wsh/cmd/wshcmd-blocks.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/wsh/cmd/wshcmd-blocks.go (6)
pkg/waveobj/metamap.go (1)
MetaMapType(8-8)frontend/app/store/wshclientapi.ts (2)
WorkspaceListCommand(471-473)BlocksListCommand(36-38)pkg/wshrpc/wshclient/wshclient.go (2)
WorkspaceListCommand(560-563)BlocksListCommand(50-53)pkg/wshrpc/wshrpctypes.go (2)
RpcOpts(271-277)BlocksListRequest(683-686)cmd/wsh/cmd/wshcmd-root.go (2)
WriteStderr(70-72)WriteStdout(74-76)pkg/waveobj/metaconsts.go (4)
MetaKey_View(9-9)MetaKey_File(13-13)MetaKey_Url(15-15)MetaKey_CmdCwd(56-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: merge-gatekeeper
🔇 Additional comments (1)
cmd/wsh/cmd/wshcmd-blocks.go (1)
136-139: --tab=current has no effect; either implement or warn--tab=current currently bypasses tab filtering and returns all tabs. Resolve the active tab from WorkspaceInfoData (waveobj.Workspace) if an active/focused-tab field exists; otherwise emit a warning before applying filters:
if blocksTabId == "current" { WriteStderr("Warning: --tab=current is not yet supported; ignoring tab filter.\n") }File: cmd/wsh/cmd/wshcmd-blocks.go — insert after BlocksListCommand return and before the filter loop.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
cmd/wsh/cmd/wshcmd-blocks.go (4)
132-167: Differentiate “no matches” from “all workspace RPCs failed.”Return a non-zero error if every workspace query failed instead of printing “No blocks found.”
@@ - // Query each selected workspace - for _, wsId := range workspaceIdsToQuery { + // Query each selected workspace + hadSuccess := false + for _, wsId := range workspaceIdsToQuery { req := wshrpc.BlocksListRequest{WorkspaceId: wsId} if blocksWindowId != "" { req.WindowId = blocksWindowId } blocks, err := wshclient.BlocksListCommand(RpcClient, req, &wshrpc.RpcOpts{Timeout: int64(blocksTimeout * 1000)}) if err != nil { WriteStderr("Warning: couldn't list blocks for workspace %s: %v\n", wsId, err) continue } + hadSuccess = true @@ - if len(allBlocks) == 0 { + if len(allBlocks) == 0 { + if !hadSuccess { + return fmt.Errorf("failed to list blocks from all %d workspace(s)", len(workspaceIdsToQuery)) + } WriteStdout("No blocks found\n") return nil }Also applies to: 179-183
28-34: Expose “view” as a top-level JSON field for easier automation.Clients shouldn’t have to know meta keys to get the view.
@@ type BlockDetails struct { BlockId string `json:"blockid"` // Unique identifier for the block WorkspaceId string `json:"workspaceid"` // ID of the workspace containing the block TabId string `json:"tabid"` // ID of the tab containing the block + View string `json:"view"` // Canonical view type (term, web, preview, edit, sysinfo, waveai) Meta waveobj.MetaMapType `json:"meta"` // Block metadata including view type } @@ - allBlocks = append(allBlocks, BlockDetails{ + v := b.Meta.GetString(waveobj.MetaKey_View, "") + allBlocks = append(allBlocks, BlockDetails{ BlockId: b.BlockId, WorkspaceId: b.WorkspaceId, TabId: b.TabId, + View: v, Meta: b.Meta, })Also applies to: 160-166
194-227: Minor: Consider text/tabwriter for nicer column alignment under mixed widths.It simplifies padding/truncation and stays readable on narrow terminals.
71-71: RpcOpts.Timeout is milliseconds — make CLI flag units consistentConfirmed: RpcOpts.Timeout is interpreted as milliseconds (pkg/wshutil/wshrpc.go uses DefaultTimeoutMs and constructs contexts with time.Duration(timeoutMs)*time.Millisecond). cmd/wsh/cmd/wshcmd-blocks.go documents --timeout as seconds and converts with blocksTimeout * 1000 when building RpcOpts (flag def + usages in that file) — the conversion is correct, but other CLI flags (e.g. cmd/wsh/cmd/wshcmd-file.go fileTimeout) accept milliseconds directly.
Action: standardize CLI units. Recommended: change the blocks timeout flag to accept milliseconds (or convert other flags to seconds) and update help text/usages accordingly. Fix locations: cmd/wsh/cmd/wshcmd-blocks.go (flag + RpcOpts uses), pkg/wshutil/wshrpc.go (timeout interpretation), cmd/wsh/cmd/wshcmd-file.go (example of ms-based flag).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cmd/wsh/cmd/wshcmd-blocks.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/wsh/cmd/wshcmd-blocks.go (6)
pkg/waveobj/metamap.go (1)
MetaMapType(8-8)frontend/app/store/wshclientapi.ts (2)
WorkspaceListCommand(471-473)BlocksListCommand(36-38)pkg/wshrpc/wshclient/wshclient.go (2)
WorkspaceListCommand(560-563)BlocksListCommand(50-53)pkg/wshrpc/wshrpctypes.go (2)
RpcOpts(271-277)BlocksListRequest(683-686)cmd/wsh/cmd/wshcmd-root.go (2)
WriteStderr(70-72)WriteStdout(74-76)pkg/waveobj/metaconsts.go (4)
MetaKey_View(9-9)MetaKey_File(13-13)MetaKey_Url(15-15)MetaKey_CmdCwd(56-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: merge-gatekeeper
🔇 Additional comments (3)
cmd/wsh/cmd/wshcmd-blocks.go (3)
169-176: Good: JSON marshalling error is handled and propagated.
106-125: Good: --workspace and --window are mutually exclusive with clear error messaging.
239-251: Good: View-type aliasing is comprehensive and case-insensitive.
cmd/wsh/cmd/wshcmd-blocks.go
Outdated
| if blocksTabId != "" && blocksTabId != "current" && b.TabId != blocksTabId { | ||
| continue | ||
| } | ||
|
|
||
| if blocksView != "" { | ||
| view := b.Meta.GetString(waveobj.MetaKey_View, "") | ||
|
|
||
| // Support view type aliases | ||
| if !matchesViewType(view, blocksView) { | ||
| continue | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix --tab=current: it’s silently ignored; either implement or reject explicitly.
Current logic treats "current" as no-op and returns all tabs, which is misleading. Reject it up-front for now and simplify the in-loop filter.
@@
- if len(workspaces) == 0 {
- return fmt.Errorf("no workspaces found")
- }
+ if len(workspaces) == 0 {
+ return fmt.Errorf("no workspaces found")
+ }
+
+ // TODO: implement resolving the active/current tab via RPC, then accept --tab=current
+ if strings.EqualFold(blocksTabId, "current") {
+ return fmt.Errorf("--tab=current is not supported yet; please specify a tab id")
+ }
@@
- if blocksTabId != "" && blocksTabId != "current" && b.TabId != blocksTabId {
+ if blocksTabId != "" && b.TabId != blocksTabId {
continue
}Also applies to: 100-106
🤖 Prompt for AI Agents
In cmd/wsh/cmd/wshcmd-blocks.go around lines 100-106 and 147-158, the code
treats the --tab=current value as a no-op and ends up returning all tabs;
instead, explicitly reject "current" at argument-parsing time and simplify the
in-loop filter. Add an upfront validation that if blocksTabId == "current"
return an error (or print a clear message and exit) so "current" is not silently
ignored, and then remove the special-case handling from the loop so the loop
only checks b.TabId != blocksTabId; leave view filtering unchanged.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
cmd/wsh/cmd/wshcmd-blocks.go (1)
113-121: Reject--tab=currentexplicitly (was previously flagged).As-is,
--tab=currentwill match nothing; better to fail fast with a clear error. This mirrors the earlier review suggestion.func blocksListRun(cmd *cobra.Command, args []string) error { @@ if len(workspaces) == 0 { return fmt.Errorf("no workspaces found") } + + // TODO: implement resolving the active/current tab via RPC, then accept --tab=current + if strings.EqualFold(blocksTabId, "current") { + return fmt.Errorf("--tab=current is not supported yet; please specify a tab id") + }
🧹 Nitpick comments (6)
cmd/wsh/cmd/wshcmd-blocks.go (6)
27-28: Fix timeout units in the comment (seconds vs milliseconds).Comment says “seconds” but the flag/help and default imply milliseconds.
- blocksTimeout int // Timeout in seconds for RPC calls + blocksTimeout int // Timeout in milliseconds for RPC calls
211-214: Write table output via WrappedStdout, not os.Stdout.Keeps output consistent with the rest of the CLI (capture/redirection).
- w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) + w := tabwriter.NewWriter(WrappedStdout, 0, 0, 2, ' ', 0)
104-112: Minor: skip listing workspaces when only --workspace is provided.You can avoid an RPC call if blocksWorkspaceId is set and --window is empty; directly query that workspace and let the server validate.
162-170: Validate--viewupfront to catch typos.A misspelled view silently yields no rows. Fail fast with a helpful message.
func blocksListRun(cmd *cobra.Command, args []string) error { + if v := strings.TrimSpace(blocksView); v != "" { + if !isKnownViewFilter(v) { + return fmt.Errorf("unknown --view %q; try one of: term, web, preview, edit, sysinfo, waveai", v) + } + }Add helper (outside the changed hunk):
// place near matchesViewType func isKnownViewFilter(f string) bool { switch strings.ToLower(strings.TrimSpace(f)) { case "term", "terminal", "shell", "console", "web", "browser", "url", "preview", "edit", "sysinfo", "sys", "system", "waveai", "ai", "assistant": return true default: return false } }
220-224: Normalize view string for table output.Cosmetic: lower-case for consistent display regardless of meta casing.
- view := b.View + view := strings.ToLower(b.View)
191-200: Optional: use SliceStable.Not required here, but a stable sort avoids reordering equal keys across Go versions.
- sort.Slice(allBlocks, func(i, j int) bool { + sort.SliceStable(allBlocks, func(i, j int) bool {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cmd/wsh/cmd/wshcmd-blocks.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/wsh/cmd/wshcmd-blocks.go (5)
pkg/waveobj/metamap.go (1)
MetaMapType(8-8)pkg/wshrpc/wshclient/wshclient.go (2)
WorkspaceListCommand(560-563)BlocksListCommand(50-53)pkg/wshrpc/wshrpctypes.go (2)
RpcOpts(271-277)BlocksListRequest(683-686)cmd/wsh/cmd/wshcmd-root.go (2)
WriteStderr(70-72)WriteStdout(74-76)pkg/waveobj/metaconsts.go (4)
MetaKey_View(9-9)MetaKey_File(13-13)MetaKey_Url(15-15)MetaKey_CmdCwd(56-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: merge-gatekeeper
🔇 Additional comments (3)
cmd/wsh/cmd/wshcmd-blocks.go (3)
30-37: Type shape and JSON tags look solid.BlockDetails is minimal, forward-compatible, and uses the project’s MetaMapType.
191-200: Deterministic ordering before both JSON and table.Nice touch; predictable outputs aid tests and automation.
202-210: Good: JSON errors are handled and surfaced.Prevents silent empty output on marshal failures.
|
ah this is cool! will check it out and see if i can get this merged |
|
@mister-x-ops so i'll need you to sign the CLA -- Contributor License Agreement (the first comment at the top), in order for me to proceed to try to get this merged. |
|
@sawka done |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/app/store/wshclientapi.ts(1 hunks)frontend/types/gotypes.d.ts(1 hunks)pkg/wshrpc/wshclient/wshclient.go(1 hunks)pkg/wshrpc/wshrpctypes.go(3 hunks)pkg/wshrpc/wshserver/wshserver.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/wshrpc/wshclient/wshclient.go
- pkg/wshrpc/wshrpctypes.go
🧰 Additional context used
🧬 Code graph analysis (3)
frontend/app/store/wshclientapi.ts (1)
pkg/wshrpc/wshrpctypes.go (3)
BlocksListRequest(708-711)RpcOpts(295-301)BlocksListEntry(713-719)
pkg/wshrpc/wshserver/wshserver.go (7)
frontend/app/store/wshclientapi.ts (1)
BlocksListCommand(36-38)pkg/wshrpc/wshclient/wshclient.go (1)
BlocksListCommand(51-54)pkg/wshrpc/wshrpctypes.go (2)
BlocksListRequest(708-711)BlocksListEntry(713-719)pkg/wcore/window.go (1)
GetWindow(72-79)pkg/wstore/wstore_dbops.go (3)
DBGetSingleton(102-105)DBFindWindowForWorkspaceId(396-403)DBMustGet(138-149)pkg/waveobj/wtype.go (6)
Client(125-133)Client(135-137)Tab(183-190)Tab(192-194)Block(280-288)Block(290-292)pkg/wcore/workspace.go (1)
GetWorkspace(186-188)
frontend/types/gotypes.d.ts (1)
pkg/wshrpc/wshrpctypes.go (2)
BlocksListEntry(713-719)BlocksListRequest(708-711)
🔇 Additional comments (3)
frontend/types/gotypes.d.ts (1)
90-103: LGTM! TypeScript declarations match Go types correctly.The
BlocksListEntryandBlocksListRequesttype declarations correctly mirror the Go structs frompkg/wshrpc/wshrpctypes.go, with proper optional field handling (the?onwindowidandworkspaceidcorresponds to theomitemptyJSON tags in Go).frontend/app/store/wshclientapi.ts (1)
35-38: LGTM! RPC client method follows established patterns.The
BlocksListCommandmethod correctly:
- Accepts
BlocksListRequestas input and returnsPromise<BlocksListEntry[]>- Delegates to
client.wshRpcCall("blockslist", ...)- Follows the same pattern as other commands in this file
pkg/wshrpc/wshserver/wshserver.go (1)
803-868: Implementation is correct and follows established patterns.The
BlocksListCommandproperly:
- Resolves scope (workspace/window/current)
- Handles the no-active-window edge case (Line 827)
- Iterates through both pinned and regular tabs (Line 847)
- Collects all required metadata in
BlocksListEntryNote: The RPC layer returns all blocks; view-type and per-tab filtering (mentioned in the PR objectives for the CLI
--viewand--tabflags) must be performed client-side sinceBlocksListRequestonly includesWindowIdandWorkspaceIdfields. This is a reasonable design for the RPC layer.
| windowId, err := wstore.DBFindWindowForWorkspaceId(ctx, wsID) | ||
| if err != nil { | ||
| log.Printf("error finding window for workspace %s: %v", wsID, err) | ||
| } |
There was a problem hiding this comment.
Inconsistent error handling could mask database issues.
Window lookup errors are logged but processing continues, whereas tab/block lookup errors immediately fail the entire command (Lines 850, 855). If DBFindWindowForWorkspaceId fails due to a database error (connection issue, corruption, etc.) rather than legitimately having no associated window, continuing silently could produce incomplete or inconsistent results.
Consider either:
- Returning an error if window lookup fails (consistent with tab/block errors)
- Explicitly distinguishing "window not found" (expected, continue) from "database error" (unexpected, fail)
windowId, err := wstore.DBFindWindowForWorkspaceId(ctx, wsID)
if err != nil {
- log.Printf("error finding window for workspace %s: %v", wsID, err)
+ return nil, fmt.Errorf("error finding window for workspace %s: %w", wsID, err)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| windowId, err := wstore.DBFindWindowForWorkspaceId(ctx, wsID) | |
| if err != nil { | |
| log.Printf("error finding window for workspace %s: %v", wsID, err) | |
| } | |
| windowId, err := wstore.DBFindWindowForWorkspaceId(ctx, wsID) | |
| if err != nil { | |
| return nil, fmt.Errorf("error finding window for workspace %s: %w", wsID, err) | |
| } |
🤖 Prompt for AI Agents
In pkg/wshrpc/wshserver/wshserver.go around lines 842-845, the call to
wstore.DBFindWindowForWorkspaceId only logs errors and continues while
subsequent tab/block lookups return on error; update handling so database errors
don't get masked: if the store returns a "not found" sentinel (e.g.
sql.ErrNoRows or the store's ErrNotFound) treat that as expected and continue,
but for any other non-nil error return that error from the handler (matching
behavior of the tab/block lookups); alternatively, if you prefer simpler change,
change the current log-only branch to return the error directly so failures stop
processing.
|
@mister-x-ops sorry about the delay here. was on a long running branch (for the new Wave AI feature, w/ 128 commits) and wanted to get that merged and stabilized before pulling other code. pulling now. will be in the next release. |
|
Thanks |
3 participants
What This Does
Adds a new
wsh blocks listsubcommand that lists all blocks in all or specified workspace, window, or tab. Includes filtering options and JSON output for automation.Motivation
Wave users had no simple way to programmatically discover block IDs for scripting and automation. This feature:
cdchanges.Usage
wsh blocks [list|ls|get] [--workspace=<workspace-id>] [--window=<window-id>] [--tab=<tab-id>] [--view=<view-type>] [--json]Where
<view-type>can be one of: term, terminal, shell, console, web, browser, url, preview, edit, sysinfo, sys, system, waveai, ai, or assistant.Notes