Fix --tags not being applied to integrated indexes, refactor /index/create.go for unit tests

[austin-denoble]

Problem

@bradumbaugh flagged a bug around the --tags argument not being applied when creating new integrated indexes through the CLI. The argument itself was not being passed to the Go SDK, which was an easy fix.

Because of our general lack of unit testing, and the importance of index creation and the business logic that's applied, we really needed unit tests for this command.

Solution

There were a few functional unit tests validating some of the logic around inferring index type, but ultimately we need more coverage around the core logic inside of the pc index create command.

• Fix --tags not being applied to integrated indexes.
• Refactor /index/create.go function signatures and add an interface to allow for dependency injection and unit testing. None of the logic itself has changed, but I added a new runCreateIndexWithService function and a CreateIndexService interface to allow injecting a mock Go SDK to allow asserting on what gets provided.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update
• Infrastructure change (CI configs, etc)
• Non-code change (docs, etc)
• None of the above: (explain here)

Test Plan

Test index creation flows for serverless, integrated, and pod indexes.

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1211763561664750

[claude]

Claude finished @austin-denoble's task —— View job

---

PR Review Progress

Reviewing the pull request that fixes --tags not being applied to integrated indexes and refactors for better unit testing.

Todo List:

• Read repository guidelines (CLAUDE.md)
• Analyze the bug fix for --tags not being applied
• Review the refactoring approach and interface design
• Examine new unit tests for coverage and quality
• Check for code quality and best practices
• Look for potential bugs or issues
• Assess performance considerations
• Review security concerns
• Provide comprehensive feedback

Code Review Summary

This is a solid refactoring that addresses a real bug and significantly improves testability. The changes are well-implemented with good test coverage.

✅ What's Good

Bug Fix: The core bug is properly fixed - Tags: indexTags was missing from the CreateIndexForModelRequest in create.go:244. This was the root cause of --tags not being applied to integrated indexes.

Clean Refactoring: The introduction of the CreateIndexService interface and runCreateIndexWithService() function is well-designed:

• Maintains single responsibility principle
• Enables dependency injection for testing
• No changes to business logic, only structure
• Clear separation between CLI concerns and SDK interactions

Comprehensive Test Coverage: The new tests are thorough and well-structured:

• Tests all three index types (serverless, pod, integrated)
• Validates that arguments are correctly passed to the SDK
• Includes edge cases and error scenarios
• Uses proper mocking with the mockIndexService

Code Quality: Clean, idiomatic Go code with good naming conventions and structure.

📝 Minor Observations

Test Organization: The consolidation from index_test.go to create_test.go makes sense - the new tests are more focused and comprehensive than the original validation-only tests.

Backwards Compatibility: The refactoring maintains full backwards compatibility - the original runCreateIndexCmd() function is unchanged and continues to work as before.

Error Handling: Consistent error handling patterns maintained throughout the refactor.

🔒 Security & Performance

Security: No security concerns identified. The refactoring doesn't introduce new attack vectors and follows secure coding practices.

Performance: Minimal performance impact. The additional function call layer is negligible, and the interface abstraction adds no runtime overhead in Go.

🧪 Test Coverage Assessment

The new tests provide excellent coverage of the critical path:

• Integration Tests: All three SDK methods are tested with comprehensive argument validation
• Unit Tests: deriveIndexType() and validate() methods have thorough test suites
• Edge Cases: Error conditions and invalid inputs are well covered

Suggestion: Consider adding a test that explicitly validates the bug fix - a test that creates an integrated index with tags and verifies they're included in the request.

📊 Overall Assessment

This PR successfully:

• ✅ Fixes the reported --tags bug for integrated indexes
• ✅ Improves code testability without breaking changes
• ✅ Adds comprehensive test coverage for critical functionality
• ✅ Maintains clean, readable code structure
• ✅ Uses appropriate Go patterns and conventions

Recommendation: Approve - This is a well-executed bug fix with solid testing improvements.

---

[austin-denoble]

Interface abstraction to allow passing in a mock service to represent the SDK in unit tests.

[austin-denoble]

This is the actual "fix" for the bug.

[austin-denoble]

These tests were moved here from the old index_test.go file since they're focused on index creation anyways.

[austin-denoble]

Pulling this in mainly to use the assert stuff for now, because I really don't want to bloat tests further with a bunch of if statements. Eventually would like to use testify for integration suites, etc.

[bradumbaugh]

@austin-denoble This approach makes sense to me, and I see the tags fix in there, too. Probably good to get someone more familiar than me with Go to take a look, though.