feat(cli): complete command coverage for all GoClaw backend APIs

[mrgoonie]

Summary

• Add 2 new command modules and 7 new command files extending existing modules
• Fills remaining API coverage gaps after PR feat(cli): multi-tenant support, 12 new command groups, modularization #1's modularization

New Modules

Module	Commands	Transport
devices	list, revoke, pairing-status	WS RPC
export/import	agent, team, skills, mcp (+ previews)	HTTP

New Commands in Existing Modules

File	Commands Added
agents_wake.go	agents wake
agents_files.go	agents files list/get/set
knowledge_graph_dedup.go	kg dedup scan/list/dismiss, kg merge
memory_index.go	memory index, memory index-all, memory chunks
teams_tasks_delete.go	teams tasks delete
mcp_reconnect.go	mcp servers reconnect
providers_verify_embedding.go	providers verify-embedding

Test plan

• go build ./... passes
• go vet ./... passes
• go test ./... all tests pass (including cmd package)
• All new commands visible in --help output
• No conflicts with main

[mrgoonie]

@claude review

[claude]

Claude finished @mrgoonie's task in 1m 33s —— View job

---

Code Review

• Read changed files
• Review code quality and patterns
• Check for bugs and security issues
• Verify consistency with codebase conventions
• Post review feedback

---

Overall the PR is well-structured and follows the established patterns. The new commands are clean and composable. I found a few issues worth addressing before merging.

---

Issues

Bug: --ids flag sends a raw string instead of an array (knowledge_graph_dedup.go)

cmd/knowledge_graph_dedup.go:50-58, 81

The dismiss command accepts --ids as a plain String flag and passes it directly to the API:

ids, _ := cmd.Flags().GetString("ids")
data, err := c.Post("/v1/agents/"+url.PathEscape(args[0])+"/kg/dedup/dismiss",
    map[string]any{"ids": ids})

This sends "ids": "id1,id2" (a string) to the backend. REST APIs that accept a list of IDs typically expect a JSON array (["id1","id2"]). This should use StringSlice so the flag accepts --ids id1 --ids id2 or --ids id1,id2 and sends an actual []string.

// init()
kgDedupDismissCmd.Flags().StringSlice("ids", nil, "Duplicate pair IDs (comma-separated or repeated)")

// RunE
ids, _ := cmd.Flags().GetStringSlice("ids")
c.Post(..., map[string]any{"ids": ids})

Fix this →

---

Missing table output in agentsFilesListCmd (agents_files.go)

cmd/agents_files.go:16-31

Every other list command in this codebase (e.g., devicesListCmd, memoryListCmd, agentsListCmd) checks cfg.OutputFormat != "table" and renders a formatted table in the default mode. agentsFilesListCmd always calls printer.Print(unmarshalList(data)) regardless of output format. This breaks the consistent UX for --output table (the default).

Fix this →

---

defer f.Close() on write path silently drops flush errors (export_import.go)

cmd/export_import.go:270-285

func writeExportFile(body io.Reader, outFile, defaultName string) error {
    f, err := os.Create(outFile)
    ...
    defer f.Close()   // <-- close errors after io.Copy are silently discarded
    n, err := io.Copy(f, body)
    ...
}

On a write path (unlike reads), Close() can fail if the OS hasn't flushed all buffered data to disk. This could produce a silently-corrupted export file with no error reported. The standard pattern is:

n, err := io.Copy(f, body)
if err != nil {
    return err
}
if err := f.Close(); err != nil {
    return fmt.Errorf("close export file: %w", err)
}

Fix this →

---

Missing importTeamPreviewCmd (export_import.go)

cmd/export_import.go:300-308

The export side has both exportAgentPreviewCmd + exportTeamPreviewCmd. The import side has importAgentPreviewCmd but no importTeamPreviewCmd. This looks like an accidental gap — if a preview endpoint exists for teams on the backend, it should be wired up for symmetry.

---

Minor / Style

-f shorthand for --output is non-idiomatic (export_import.go:301):
The conventional shorthand for an output file path is -o, not -f (which usually means "force"). This is a UX inconsistency.

Tense mismatch in success message (mcp_reconnect.go:21):
All other success messages in the codebase use past tense: "Agent deleted", "Device revoked", "MCP server updated". The reconnect command says "MCP server reconnecting" (present progressive), which is inconsistent. Consider "MCP server reconnect triggered" or simply "Reconnecting".

---

What looks good

• All new commands correctly use url.PathEscape() on path parameters — no path injection risk.
• Destructive operations (revoke, delete) consistently require confirmation via tui.Confirm(..., cfg.Yes).
• readJSONFile properly validates JSON before sending it to the API — good guard against silent payload errors.
• The writeExportFile / readJSONFile helpers are cleanly scoped to export_import.go and don't pollute helpers.go.
• WS commands consistently pair ws.Connect() with defer ws.Close().
• export/import commands correctly use url.PathEscape on agent/team IDs.
• New modules (devices, export/import) are correctly wired to rootCmd; new subcommands extend the right parent commands.

[mrgoonie]

@claude review

[claude]

Claude finished @mrgoonie's task in 1m 46s —— View job

---

Claude finished @mrgoonie's task ——

---

Code Review

• Read changed files
• Review code quality and patterns
• Check for bugs and security issues
• Verify consistency with codebase conventions
• Post review feedback

---

All issues from the previous review have been addressed. The code is clean and follows codebase conventions well. I found three new issues to address before merging.

---

Issues

kgMergeCmd is a destructive operation without confirmation (knowledge_graph_dedup.go:61-78)

Every other destructive or irreversible operation in the codebase calls tui.Confirm before proceeding — kgEntityDeleteCmd, devicesRevokeCmd, teamsTasksDeleteCmd, etc. kgMergeCmd merges KG entities (not easily undoable) but has no confirmation gate.

// Add before the HTTP call in RunE:
if !tui.Confirm("Merge these KG entities?", cfg.Yes) {
    return nil
}

Fix this →

---

kgDedupListCmd and memoryChunksCmd missing table output (knowledge_graph_dedup.go:27-41, memory_index.go:42-56)

Both commands return lists but skip the cfg.OutputFormat != "table" branch present in every other list command (devicesListCmd, agentsFilesListCmd, memoryListCmd, teamsTasksCmd, etc.). In default mode users get a raw dump instead of a formatted table.

// kgDedupListCmd RunE — replace printer.Print(unmarshalList(data)) with:
if cfg.OutputFormat != "table" {
    printer.Print(unmarshalList(data))
    return nil
}
tbl := output.NewTable("ID", "ENTITY_A", "ENTITY_B", "SCORE")
for _, r := range unmarshalList(data) {
    tbl.AddRow(str(r, "id"), str(r, "entity_a"), str(r, "entity_b"), str(r, "score"))
}
printer.Print(tbl)

Apply the same pattern to memoryChunksCmd with appropriate columns (e.g. ID, CONTENT, CREATED_AT).

Fix this →

---

Missing importSkillsPreviewCmd and importMCPPreviewCmd (export_import.go:323-332)

The export side has preview commands for all four resource types (agent, team, skills, MCP). The import side has previews for agent and team but not for skills or MCP, despite exportSkillsPreviewCmd and exportMCPPreviewCmd existing. If the backend exposes /v1/skills/import/preview and /v1/mcp/import/preview, these commands should be wired up for parity. If the endpoints don't exist, a brief comment in the code noting this would prevent future confusion.

---

What looks good

• All six issues from the previous review are resolved: --ids uses StringSlice, agentsFilesListCmd has table output, writeExportFile handles close errors explicitly, importTeamPreviewCmd is implemented, the export flag uses -o, and the reconnect message uses past tense.
• kgDedupDismissCmd correctly marks --ids as required — no silent no-op possible.
• readJSONFile validates JSON before posting — prevents silent corrupt-payload errors.
• All HTTP path parameters consistently use url.PathEscape across every new command.
• WS commands all pair ws.Connect() with defer ws.Close().
• All new destructive commands (devicesRevokeCmd, import commands) correctly use tui.Confirm.
• New modules (devices, export, import) are correctly wired to rootCmd; new subcommands extend the right parent.

[mrgoonie]

@claude review

[claude]

Claude finished @mrgoonie's task in 2m 26s —— View job

---

Code Review

• Read changed files
• Review code quality and patterns
• Check for bugs and security issues
• Verify consistency with codebase conventions
• Post review feedback

---

All issues from the previous two review rounds have been resolved. The code is clean and well-structured. I found one real gap to address before merging, plus two minor notes.

---

Issue

cmd_test.go doesn't cover new root-level commands (devices, export, import)

cmd/cmd_test.go:18-52, 89-122

The PR adds three new root-level commands (devices, export, import) wired via rootCmd.AddCommand(...) in devices.go:85 and export_import.go:372. But neither TestAllCommandsRegistered nor TestCommandUseFields includes them in the expected list. If a future refactor accidentally removes those AddCommand calls, the tests will still pass.

Add all three to both expected slices:

// TestAllCommandsRegistered expected list (alphabetical):
"devices",
"export",
"import",

// TestCommandUseFields cases:
{"devices"},
{"export"},
{"import"},

Fix this →

---

Minor / Style

memoryChunksCmd CONTENT column is unbounded (memory_index.go:58-60):
The table column CONTENT shows raw memory chunk content, which is typically hundreds of words of prose. In table mode this will overflow terminal width and make the output unreadable. A truncation — or dropping the column from table mode in favor of ID, DOCUMENT, CREATED only — would be safer. Other commands in the codebase that might show long text (e.g. memoryListCmd, agentsFilesListCmd) only show metadata in table mode, not content.

Import responses uniformly use unmarshalMap (export_import.go:88, 166, 246, 324):
All four import commands call printer.Print(unmarshalMap(data)). If the backend returns a JSON array (e.g. a list of imported resource IDs), unmarshalMap will silently give an empty {}. This is low-risk since all four are treated the same way and the backend contract is consistent — but worth confirming that each import endpoint returns an object rather than an array.

---

What looks good

All six issues from round 1 and all three issues from round 2 are cleanly resolved:

• --ids uses StringSlice — API receives a JSON array ✓
• agentsFilesListCmd, kgDedupListCmd, memoryChunksCmd all have table output branches ✓
• writeExportFile calls f.Close() explicitly and returns the error ✓
• importTeamPreviewCmd, importSkillsPreviewCmd, importMCPPreviewCmd all implemented ✓
• Export output flag uses -o shorthand ✓
• Reconnect success message uses past tense ✓
• kgMergeCmd gates behind tui.Confirm ✓
• All HTTP path parameters consistently use url.PathEscape ✓
• WS commands all pair ws.Connect() with defer ws.Close() ✓
• All destructive ops (revoke, delete, import) correctly use tui.Confirm ✓

[mrgoonie]

@claude review

[claude]

Claude finished @mrgoonie's task in 1m 57s —— View job

---

Code Review

• Read changed files
• Review code quality and patterns
• Check for bugs and security issues
• Verify consistency with codebase conventions
• Post review feedback

---

All issues from the three previous review rounds have been resolved. This PR is clean and ready to merge.

---

Verification of Prior Issues

Round	Issue	Status
R1	--ids flag sends string instead of array	✓ Fixed — StringSlice, marked required
R1	agentsFilesListCmd missing table output	✓ Fixed — cfg.OutputFormat != "table" branch present
R1	defer f.Close() silently drops flush errors	✓ Fixed — explicit f.Close() with error return
R1	Missing importTeamPreviewCmd	✓ Fixed — implemented at export_import.go:130
R1	-f shorthand for output flag	✓ Fixed — uses -o
R1	Tense mismatch in reconnect message	✓ Fixed — "MCP server reconnect triggered"
R2	kgMergeCmd missing tui.Confirm	✓ Fixed — guard at knowledge_graph_dedup.go:74
R2	kgDedupListCmd / memoryChunksCmd missing table output	✓ Fixed — both have table branches
R2	Missing importSkillsPreviewCmd / importMCPPreviewCmd	✓ Fixed — both implemented
R3	cmd_test.go missing devices, export, import	✓ Fixed — present in both expected and cases slices
R3	memoryChunksCmd CONTENT column unbounded	✓ Fixed — columns are ID, DOCUMENT, SIZE, CREATED (no raw content)

---

What Looks Good (Full Pass)

• writeExportFile (export_import.go:330-348): explicit f.Close() after io.Copy, error returned — write-flush failures surface correctly.
• kgDedupDismissCmd (knowledge_graph_dedup.go:53-68): StringSlice flag marked required — no silent no-op.
• kgMergeCmd (knowledge_graph_dedup.go:71-91): tui.Confirm before HTTP call, both --source and --target required.
• agentsFilesListCmd (agents_files.go:15-41): table output branch present; WS correctly paired with defer ws.Close().
• memoryChunksCmd (memory_index.go:43-65): table shows path metadata only — no unbounded text columns.
• devicesRevokeCmd (devices.go:40-61): confirmation gate, WS properly closed.
• All HTTP path parameters use url.PathEscape across every new command — no path injection risk.
• All destructive commands (revoke, delete, merge, all imports) gate on tui.Confirm.
• All WS commands pair ws.Connect() with defer ws.Close().
• readJSONFile validates JSON before posting — prevents silent corrupt-payload errors.
• cmd_test.go now covers all three new root-level commands (devices, export, import) in both TestAllCommandsRegistered and TestCommandUseFields.
• New modules correctly wired: devices.go:85, export_import.go:372 both call rootCmd.AddCommand.

---

No issues found. The PR is ready to merge.