Conversation
- treat the workspace's URL label slug (ws- prefix stripped from legacy labels) as the user-facing id - resolve slugs to numeric ids by scanning the paged list on get/attach/suspend/resume/access/delete - accept slug, full legacy label, or numeric id; numeric ids never trigger a scan - drop the LABEL column from the list table and key rows by the slug id - apply the same mapping on the cli and mcp surfaces via the shared workspaces deps wiring
- resolve labels first for every id, so a purely numeric label slug is not shadowed by the numeric interpretation - treat an unmatched numeric id as the numeric form the backend expects
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
Closing — folded the same pinner bump commit into #706 so the op-help update lands with the slug behavior it complements. |
![kody-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
| r = catalogops.NewListResult(ws, catalogops.ListResultMeta{ | ||
| Noun: "workspace(s)", | ||
| Headers: []string{"ID", "DOMAIN", "STATUS", "WEBSITE ID", "CREATED"}, | ||
| Rows: rows, | ||
| Total: r.ListTotal(), | ||
| }) |
There was a problem hiding this comment.
renderWorkspacesListResult rebuilds ListResult with only Noun/Headers/Rows/Total, dropping the original Truncated state, so truncated workspace lists print "Found N workspace(s)" instead of "Showing N of M workspace(s)". Preserve the backend's Truncated/Count fields in the rebuilt ListResultMeta to keep the partial-count render.
r = catalogops.NewListResult(ws, catalogops.ListResultMeta{
Noun: "workspace(s)",
Headers: []string{"ID", "DOMAIN", "STATUS", "WEBSITE ID", "CREATED"},
Rows: rows,
Total: r.ListTotal(),
// preserve whether the backend reported more pages beyond this page
// (Truncated / Count) so renderListResult still emits "Showing N of M"
})Prompt for LLM
File internal/cli/catalog_workspaces_wiring.go:
Line 204 to 209:
renderWorkspacesListResult rebuilds ListResult with only Noun/Headers/Rows/Total, dropping the original Truncated state, so truncated workspace lists print "Found N workspace(s)" instead of "Showing N of M workspace(s)". Preserve the backend's Truncated/Count fields in the rebuilt ListResultMeta to keep the partial-count render.
Suggested Code:
r = catalogops.NewListResult(ws, catalogops.ListResultMeta{
Noun: "workspace(s)",
Headers: []string{"ID", "DOMAIN", "STATUS", "WEBSITE ID", "CREATED"},
Rows: rows,
Total: r.ListTotal(),
// preserve whether the backend reported more pages beyond this page
// (Truncated / Count) so renderListResult still emits "Showing N of M"
})
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // "ws-<slug>" form and the full label) by scanning the user's paged list — | ||
| // labels are matched before the numeric interpretation so a purely numeric | ||
| // label slug is reachable — and passes an unmatched numeric id through. | ||
| func (s *labelResolvingWorkspaces) resolveID(ctx context.Context, id string) (string, error) { |
There was a problem hiding this comment.
resolveID runs a full paged scan of the user's workspace list for every single-workspace operation, even when a plain numeric ID has no label match, turning one backend call into N sequential list HTTP calls per command with no caching. Short-circuit the scan when strconv.Atoi(id) succeeds and no workspace label could shadow the numeric ID, or return the numeric ID immediately on the first page without a label match.
if _, err := strconv.Atoi(id); err == nil {
// numeric id: only scan if a workspace label could shadow it
if !s.labelMightShadowNumeric(id) {
return id, nil
}
}
ws, found, err := pinner.ScanPagesWithOptions(ctx, s, matchesLabel, nil, workspaceScanPageSize)
if err != nil {
return "", err
}
if found {
return strconv.Itoa(ws.Id), nil
}
if _, err := strconv.Atoi(id); err == nil {
return id, nil
}Prompt for LLM
File internal/cli/workspaces_label.go:
Line 141:
resolveID runs a full paged scan of the user's workspace list for every single-workspace operation, even when a plain numeric ID has no label match, turning one backend call into N sequential list HTTP calls per command with no caching. Short-circuit the scan when strconv.Atoi(id) succeeds and no workspace label could shadow the numeric ID, or return the numeric ID immediately on the first page without a label match.
Suggested Code:
if _, err := strconv.Atoi(id); err == nil {
// numeric id: only scan if a workspace label could shadow it
if !s.labelMightShadowNumeric(id) {
return id, nil
}
}
ws, found, err := pinner.ScanPagesWithOptions(ctx, s, matchesLabel, nil, workspaceScanPageSize)
if err != nil {
return "", err
}
if found {
return strconv.Itoa(ws.Id), nil
}
if _, err := strconv.Atoi(id); err == nil {
return id, nil
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Code Coverage ReportTotal Coverage: 55.3% Generated from commit: da720c6 |
1 participant
Bumps
go.lumeweb.com/pinnerto 895fc1f (merge of LumeWeb/pinner#65), which rewrites the workspace operations'idarg help and descriptions to describe the label slug (e.g. "ugki684o") or numeric ID form instead of only the numeric ID. Pairs with pinner-cli#706, which implements the matching slug behavior.Summary
This PR updates the
pinnerdependency to support workspace operations addressed by their label slug (the user-facing ID derived from the workspace label, e.g.ugki684o) instead of the opaque numeric backend ID. The CLI and MCP surfaces now accept and display the slug as the workspace's ID, improving usability while remaining backward compatible with numeric IDs and legacyws-<slug>labels.Changes
Label-slug resolution (
internal/cli/workspaces_label.go)labelResolvingWorkspaceswrapper around the workspaces service.get,delete,access,attach,resume,suspend) now resolve the user-supplied ID:ugki684o).ws--prefixed label form.Output/rendering updates (
internal/cli/catalog_workspaces_wiring.go)get/create/etc.) now display the slug as the ID and no longer show a separate label row.Tests (
internal/cli/workspaces_label_test.go)Review Findings
Truncatedstate is dropped, so truncated lists no longer render the partial-count line.These findings are noted for follow-up but do not block the functional purpose of this change.