Skip to content

feat(instances): support workspace access by name or ID#166

Merged
feloy merged 1 commit intokortex-hub:mainfrom
feloy:by-name
Apr 2, 2026
Merged

feat(instances): support workspace access by name or ID#166
feloy merged 1 commit intokortex-hub:mainfrom
feloy:by-name

Conversation

@feloy
Copy link
Copy Markdown
Contributor

@feloy feloy commented Apr 2, 2026

Workspaces can now be accessed using either their human-readable name or unique ID in all commands (start, stop, remove, terminal). The Manager.Get() method accepts name or ID and tries ID match first (backward compatible), then falls back to name match. Commands always output the workspace ID for consistency.

Closes #32

Workspaces can now be accessed using either their human-readable
name or unique ID in all commands (start, stop, remove, terminal).
The Manager.Get() method accepts name or ID and tries ID match
first (backward compatible), then falls back to name match.
Commands always output the workspace ID for consistency.

Closes kortex-hub#32

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 2, 2026

📝 Walkthrough

Walkthrough

This PR enables workspaces to be accessed by either name or ID across all operational commands. The Manager.Get() method now accepts a name-or-ID parameter and resolves by ID first, then by name. Command usage strings are updated to show NAME|ID format. All commands that interact with workspaces are updated to resolve the user-provided identifier and use the resolved instance ID for operations.

Changes

Cohort / File(s) Summary
Documentation
README.md, skills/implementing-command-patterns/SKILL.md, skills/working-with-instances-manager/SKILL.md
Updated examples and command reference to show workspace operations using human-readable names instead of IDs; documented the NAME|ID input format and resolution behavior (ID-first matching, then name matching).
Command Aliases
pkg/cmd/start.go, pkg/cmd/stop.go, pkg/cmd/terminal.go, pkg/cmd/remove.go
Updated Cobra command Use field from ID-only format (e.g., "start ID") to "start NAME|ID" format; delegated behavior remains unchanged.
Workspace Command Implementations
pkg/cmd/workspace_start.go, pkg/cmd/workspace_stop.go, pkg/cmd/workspace_remove.go, pkg/cmd/workspace_terminal.go
Changed internal field from id to nameOrID to capture user-supplied identifier; updated run methods to resolve the identifier via manager.Get(nameOrID), then use resolved instance.GetID() for actual operations; adjusted error messages and output to reference the resolved ID.
Manager Core
pkg/instances/manager.go
Updated Get() method signature from Get(id string) to Get(nameOrID string) and implementation to resolve by ID match first, then by name match; same error behavior when no match found.
Autocomplete
pkg/cmd/autocomplete.go
Updated getFilteredWorkspaceIDs to return both workspace ID and name for each matched instance in completion candidates.
Command Tests
pkg/cmd/..._test.go (start, stop, terminal, remove, workspace_start, workspace_stop, workspace_terminal, workspace_remove)
Updated command Use field assertions to expect NAME|ID format; adjusted field name assertions from c.id to c.nameOrID; updated expected example command counts where applicable.
Manager Tests
pkg/instances/manager_test.go
Added three new subtests to TestManager_Get: name-based retrieval, not-found error for invalid name, and ID-precedence validation (ensuring ID resolution takes priority over name).

Sequence Diagram

sequenceDiagram
    actor User
    participant Command
    participant Manager
    participant Instance
    
    User->>Command: start my-project<br/>(NAME|ID argument)
    activate Command
    Command->>Manager: Get("my-project")
    activate Manager
    Manager->>Manager: scan by ID match
    Manager->>Manager: scan by name match
    Manager-->>Command: Instance (resolved)
    deactivate Manager
    Command->>Instance: GetID()
    Instance-->>Command: workspace-id-123
    deactivate Command
    Command->>Manager: Start(ctx, workspace-id-123)
    activate Manager
    Manager-->>User: success, outputs workspace-id-123
    deactivate Manager
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 3.70% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main feature: adding support for accessing workspaces by name or ID across commands.
Description check ✅ Passed The description clearly explains the implementation: name or ID support in Manager.Get(), ID-first matching for backward compatibility, and consistent ID output.
Linked Issues check ✅ Passed All requirements from issue #32 are met: workspaces accessible by name/ID in all commands (start/stop/remove/terminal), Manager.Get() resolves name or ID with ID-first precedence, and commands output workspace ID consistently.
Out of Scope Changes check ✅ Passed All changes are directly related to enabling workspace access by name or ID: command implementations, Manager.Get() interface/logic, documentation updates, and corresponding test updates are all in scope.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
pkg/cmd/autocomplete.go (1)

33-34: Optional: rename helper to reflect returned values.

getFilteredWorkspaceIDs now returns IDs and names. Renaming it to something like getFilteredWorkspaceCompletions would reduce ambiguity.

♻️ Proposed rename
-// getFilteredWorkspaceIDs retrieves workspace IDs and names, optionally filtered by state
-func getFilteredWorkspaceIDs(cmd *cobra.Command, filter stateFilter) ([]string, cobra.ShellCompDirective) {
+// getFilteredWorkspaceCompletions retrieves workspace completion values (IDs and names), optionally filtered by state
+func getFilteredWorkspaceCompletions(cmd *cobra.Command, filter stateFilter) ([]string, cobra.ShellCompDirective) {
@@
 func completeNonRunningWorkspaceID(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-	return getFilteredWorkspaceIDs(cmd, func(state string) bool {
+	return getFilteredWorkspaceCompletions(cmd, func(state string) bool {
 		return state != "running"
 	})
 }
@@
 func completeRunningWorkspaceID(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-	return getFilteredWorkspaceIDs(cmd, func(state string) bool {
+	return getFilteredWorkspaceCompletions(cmd, func(state string) bool {
 		return state == "running"
 	})
 }

Also applies to: 65-77

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/cmd/autocomplete.go` around lines 33 - 34, The helper
getFilteredWorkspaceIDs is misleading because it returns both IDs and display
names; rename the function (e.g., to getFilteredWorkspaceCompletions) and update
its signature and all call sites (including uses referenced around the previous
implementation) to reflect that it returns completion entries (IDs and names)
along with the cobra.ShellCompDirective; ensure you update any variable names
and documentation/comments (the function comment and any callers expecting only
IDs) to match the new name and return semantics so callers handle both ID and
label correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/instances/manager_test.go`:
- Around line 637-674: The test "ID takes precedence over name" doesn't create a
name collision because inst2 is named "workspace-two"; change inst2's Name to
the same value as inst1 (e.g., "workspace-one") so a name match exists and
retrieving by id1 via manager.Get(id1) exercises ID-vs-name precedence; update
the inst2 creation (in the test function using
newFakeInstance/newFakeInstanceParams) to use the same Name as inst1 and keep
Accessible true, then assert that manager.Get(id1) returns the instance with
GetID() == id1 and GetName() == "workspace-one".

---

Nitpick comments:
In `@pkg/cmd/autocomplete.go`:
- Around line 33-34: The helper getFilteredWorkspaceIDs is misleading because it
returns both IDs and display names; rename the function (e.g., to
getFilteredWorkspaceCompletions) and update its signature and all call sites
(including uses referenced around the previous implementation) to reflect that
it returns completion entries (IDs and names) along with the
cobra.ShellCompDirective; ensure you update any variable names and
documentation/comments (the function comment and any callers expecting only IDs)
to match the new name and return semantics so callers handle both ID and label
correctly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 74970e31-1f9b-42cf-b339-74063f095b2e

📥 Commits

Reviewing files that changed from the base of the PR and between 0868243 and 1c211d8.

📒 Files selected for processing (21)
  • README.md
  • pkg/cmd/autocomplete.go
  • pkg/cmd/autocomplete_test.go
  • pkg/cmd/remove.go
  • pkg/cmd/remove_test.go
  • pkg/cmd/start.go
  • pkg/cmd/stop.go
  • pkg/cmd/terminal.go
  • pkg/cmd/terminal_test.go
  • pkg/cmd/workspace_remove.go
  • pkg/cmd/workspace_remove_test.go
  • pkg/cmd/workspace_start.go
  • pkg/cmd/workspace_start_test.go
  • pkg/cmd/workspace_stop.go
  • pkg/cmd/workspace_stop_test.go
  • pkg/cmd/workspace_terminal.go
  • pkg/cmd/workspace_terminal_test.go
  • pkg/instances/manager.go
  • pkg/instances/manager_test.go
  • skills/implementing-command-patterns/SKILL.md
  • skills/working-with-instances-manager/SKILL.md

@feloy feloy merged commit 360cffc into kortex-hub:main Apr 2, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

access workspaces by name

2 participants