feat: adds name to workspace

[feloy]

• Adds a --name flag to the init command
• refactor NewInstance (and similar tests factories) to accept options as a struct
• document the init command

Fixes #18

[coderabbitai]

Warning

Rate limit exceeded

@feloy has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 12 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bca5b7ce-f982-4a13-a276-6ac431115c2a

📥 Commits

Reviewing files that changed from the base of the PR and between 7795f06 and 246fd37.

📒 Files selected for processing (1)

• pkg/instances/manager_test.go

📝 Walkthrough

Walkthrough

This pull request adds workspace naming functionality to the Kortex CLI. It introduces a Name field to instances, replaces positional constructor arguments with a NewInstanceParams struct, implements name generation and uniqueness validation in the manager, adds a --name flag to the init command, and updates all affected tests accordingly.

Changes

Cohort / File(s)	Summary
Documentation README.md	Adds a "Workspace" section documenting the init command, including syntax, arguments, flags, examples, and configuration notes.
Instance Core pkg/instances/instance.go	Introduces Name field to Instance interface and instance struct; adds NewInstanceParams struct with SourceDir, ConfigDir, and Name fields; updates NewInstance() signature to accept NewInstanceParams; extends InstanceData to include Name; adds GetName() method; updates NewInstanceFromData() to validate and assign Name.
Instance Tests pkg/instances/instance_test.go	Migrates all NewInstance() calls from two-argument form to struct-based NewInstanceParams form; adds test coverage for custom names via NewInstanceParams.Name; verifies GetName() behavior.
Manager Logic pkg/instances/manager.go	Adds generateUniqueName() and ensureUniqueName() helper functions to derive and validate unique workspace names; updates Add() method to generate default names from source directory or enforce uniqueness of provided names; populates Name field in stored instances.
Manager Tests pkg/instances/manager_test.go	Introduces newFakeInstanceParams struct and refactors fakeInstance to include name field with GetName() accessor; updates Dump() to emit Name; migrates all test constructor calls to use newFakeInstanceParams; enforces non-empty Name in factory validation.
Init Command pkg/cmd/init.go	Adds name field to initCmd struct; introduces --name / -n command-line flag; passes name to instance creation via NewInstanceParams; includes Name in verbose output.
Init Command Tests pkg/cmd/init_test.go	Adds E2E test cases verifying: default name generation from source directory, custom name via --name flag, unique name increments (project, project-2, project-3) for duplicates, and verbose output including workspace name; validates non-empty Name in initial test assertion.
Workspace List Tests pkg/cmd/workspace_list_test.go	Updates NewInstance() calls to use NewInstanceParams struct instead of positional arguments.

Sequence Diagram

sequenceDiagram
    actor User
    participant CLI as Init Command
    participant Instance as Instance Factory
    participant Manager as Instance Manager
    participant Storage as Instance Storage

    User->>CLI: kortex-cli init [--name NAME] [DIR]
    CLI->>CLI: Parse flags & arguments
    alt Name provided
        CLI->>Instance: NewInstance(NewInstanceParams{<br/>SourceDir, ConfigDir, Name})
    else Name not provided
        CLI->>Manager: generateUniqueName(SourceDir)
        Manager->>Storage: List existing instances
        Manager->>Manager: ensureUniqueName(baseName)
        Manager-->>CLI: unique name
        CLI->>Instance: NewInstance(NewInstanceParams{<br/>SourceDir, ConfigDir, Name})
    end
    Instance->>Instance: Validate parameters
    Instance->>Manager: Add(instance)
    Manager->>Manager: Validate Name uniqueness<br/>(if needed, append suffix)
    Manager->>Storage: Store instance with Name & ID
    Manager-->>CLI: Created instance
    CLI-->>User: Initialized agent [ID] with name [NAME]

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

• PR #19: Introduces the initial instances module structure that this PR extends by adding the Name field, NewInstanceParams struct, and updating the NewInstance signature.
• PR #23: Implements the init command plumbing; this PR extends it with the Name field, parameter struct, and name-generation logic in the manager.
• PR #24: Adds the workspace list command that depends on the instances.Manager and Instance interface; this PR modifies the Instance/InstanceData shape and Manager behavior by adding Name support.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 21.05% 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 and concisely summarizes the main change: adding a name feature to workspaces.
Description check	✅ Passed	The description is related to the changeset, listing three main changes: adding --name flag, refactoring NewInstance, and documenting init.
Linked Issues check	✅ Passed	Code changes comprehensively implement issue #18 requirements: --name flag with auto-generation logic, unique name validation, and UID creation all present.
Out of Scope Changes check	✅ Passed	All changes are directly aligned with issue #18 objectives. The refactoring of NewInstance and test factories is foundational work supporting the core name feature.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

pkg/instances/manager_test.go (1)

638-651: Factory missing Name field handling may cause test failures.

The inaccessibleFactory in this test creates instances without setting the Name field. Since the manager's Add() method now generates a name and persists it, but when loading via this factory (after reconcile), instances without a name will fail validation.

However, reviewing more carefully: the Add() method generates and sets the Name before saving, so reloaded instances via inaccessibleFactory would have a Name from the JSON. The factory reads data.Name but doesn't validate it's non-empty like fakeInstanceFactory does. This inconsistency could mask issues.

Consider adding Name validation to inaccessibleFactory for consistency:

 inaccessibleFactory := func(data InstanceData) (Instance, error) {
     if data.ID == "" {
         return nil, errors.New("instance ID cannot be empty")
     }
+    if data.Name == "" {
+        return nil, errors.New("instance name cannot be empty")
+    }
     if data.Paths.Source == "" || data.Paths.Configuration == "" {
         return nil, ErrInvalidPath
     }
     return &fakeInstance{
         id:         data.ID,
+        name:       data.Name,
         sourceDir:  data.Paths.Source,
         configDir:  data.Paths.Configuration,
         accessible: false,
     }, nil
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/instances/manager_test.go` around lines 638 - 651, The test factory
inaccessibleFactory is not validating the Name field which is inconsistent with
fakeInstanceFactory and may allow creating instances that fail manager
Add()/reconcile checks; update inaccessibleFactory to check that data.Name is
non-empty (similar to fakeInstanceFactory) and return an error (e.g.,
errors.New("instance name cannot be empty")) when it's empty, while still
preserving the existing ID and path validations and returning the fakeInstance
with accessible:false when valid.

🤖 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.go`:
- Around line 240-274: When loading persisted instances in loadInstances(),
detect entries with an empty Name and populate a derived unique name (use
SourceDir's base) before calling NewInstanceFromData so legacy records can be
migrated; implement this by computing m.generateUniqueName(sourceDir,
instancesSoFar) or calling m.ensureUniqueName(baseName, instancesSoFar) for each
raw record with an empty Name, set the Name field on the parsed/raw data, and
then proceed to call NewInstanceFromData and append to instancesSoFar to keep
uniqueness checks correct.

In `@README.md`:
- Around line 88-94: Update the fenced code block in README.md that shows the
"Registered workspace" verbose output to include a language specifier (for
example "text") after the opening ``` so the snippet is rendered correctly;
locate the exact block containing "Registered workspace:" and add the language
tag to the opening fence.

---

Nitpick comments:
In `@pkg/instances/manager_test.go`:
- Around line 638-651: The test factory inaccessibleFactory is not validating
the Name field which is inconsistent with fakeInstanceFactory and may allow
creating instances that fail manager Add()/reconcile checks; update
inaccessibleFactory to check that data.Name is non-empty (similar to
fakeInstanceFactory) and return an error (e.g., errors.New("instance name cannot
be empty")) when it's empty, while still preserving the existing ID and path
validations and returning the fakeInstance with accessible:false when valid.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 27e8d968-9239-4cf4-8f4c-90b0fe6115ae

📥 Commits

Reviewing files that changed from the base of the PR and between da03481 and 7795f06.

📒 Files selected for processing (8)

• README.md
• pkg/cmd/init.go
• pkg/cmd/init_test.go
• pkg/cmd/workspace_list_test.go
• pkg/instances/instance.go
• pkg/instances/instance_test.go
• pkg/instances/manager.go
• pkg/instances/manager_test.go