Add resource name prefix matching in command mode

[yimsk]

Summary

• Resource prefix match: :ec2/in → ec2/instances
• Fix alias resource lost in prefix match (e.g., :log → cloudwatch/log-groups)

Test plan

• Type :ec2/in and verify preview shows ec2/instances
• Type :log (alias) and verify preview shows cloudwatch/log-groups
• Type :ec2/xyz (invalid) and verify error displayed

[claude]

PR Review: Add resource name prefix matching in command mode

Summary

This PR adds prefix matching for resource names in command mode (e.g., :ec2/in → ec2/instances) and fixes a bug where alias resources were being lost during prefix matching. The implementation is well-tested with comprehensive test coverage.

✅ Code Quality & Best Practices

Strengths:

• Clean refactoring: The resolvePrefixMatch function is well-structured with clear separation of concerns (service matching, alias resolution, resource matching)
• Good documentation: Function comments clearly explain the purpose and return values (lines 304-305 in command_input.go)
• Consistent naming: Variable names like matchedService, aliasResource, resourcePart are descriptive and follow Go conventions
• Proper error handling: Returns early with ok=false when no match is found, avoiding unnecessary processing
• Test coverage: Excellent test coverage with two new test functions covering both features

Minor suggestions:

• Line 279-283: Consider extracting the parts logic into a helper function since it's used in multiple places (also in resolvePrefixMatch)
• Line 348: The comment "sorted, so first match = alphabetically first" assumes ListResources returns sorted results - this is correct per registry.go:522, but could be more explicit

🐛 Potential Issues

1. Prefix match behavior with multiple matches (Medium priority)
In resolvePrefixMatch at line 348-351, the function returns the first alphabetically matching resource when there are multiple prefix matches. For example, if a user types :ec2/i and both images and instances exist, they'll always get images.

for _, res := range c.registry.ListResources(matchedService) {
    if strings.HasPrefix(res, resourcePart) {
        return matchedService, res, true  // Returns first match only
    }
}

Impact: This is acceptable for the current use case since the preview shows what will be selected, but users might expect to see suggestions for all matches.

Recommendation: Document this "first match wins" behavior or consider showing multiple suggestions when there are ambiguous prefixes.

2. Empty resource part handling (Low priority)
At line 343, when resourcePart == "" (user types :ec2/), the function returns with the alias resource. The calling code in resolveDestination (line 286) correctly shows only the service name in this case. However, in executeCommand (line 501-502), it sets a default resource.

This creates slightly inconsistent behavior:

• Preview shows: ec2 (no resource)
• Execution navigates to: ec2/instances (default resource)

Recommendation: Consider showing the default resource in the preview to match execution behavior.

⚡ Performance Considerations

Positive aspects:

• Uses strings.SplitN(input, "/", 2) instead of Split - efficient for splitting on first occurrence only
• Early returns prevent unnecessary iterations
• ListResources is already sorted, avoiding redundant sorting

No performance concerns identified - the code is efficient for interactive use cases.

🔒 Security Concerns

No security issues identified.

• All string operations use safe standard library functions
• No user input is used in unsafe contexts (SQL, shell commands, file paths)
• The registry lookup is read-only and properly mutex-protected (registry.go:509-511)

🧪 Test Coverage

Excellent test coverage:

1. TestCommandInput_ResourcePrefixMatching (lines 694-724):

   • Tests prefix matching for multiple resources
   • Tests exact matching as a control case
   • Covers the happy path thoroughly

2. TestCommandInput_AliasResourcePreservation (lines 726-748):

   • Tests both exact alias match (logs)
   • Tests prefix match on alias (log)
   • Validates the bug fix directly

Test quality:

• Clear test names and structure
• Good use of table-driven tests
• Tests cover both the bug fix and the new feature

Missing test cases (optional enhancements):

• Edge case: What happens with :ec2/xyz (invalid resource prefix)?
• Edge case: Multiple prefix matches (e.g., :ec2/i with both images and instances)
• Integration test: Verify the preview display matches execution behavior

📝 Additional Observations

1. Binary file change: The demo.gif update is expected for showcasing the new feature - no concerns.

2. Tape file cleanup (docs/tapes/demo.tape): Removed 3 lines including a Type "s" command - appears to be cleaning up the demo script, which is appropriate.

3. Code organization: The separation between resolveDestination (for preview) and executeCommand (for navigation) is clean, though it does create the minor inconsistency mentioned earlier.

🎯 Recommendations

Priority: Medium

• Document the "first match wins" behavior for resource prefix matching
• Consider aligning preview and execution behavior for :service/ (empty resource part)

Priority: Low

• Add test case for invalid resource prefix (e.g., :ec2/xyz)
• Add test case demonstrating behavior with multiple prefix matches
• Extract repeated strings.SplitN(input, "/", 2) logic into a helper function

✨ Conclusion

This is a high-quality PR that adds useful functionality with proper testing. The code is clean, well-documented, and follows Go best practices. The bug fix for alias resource preservation is correctly implemented and tested.

The identified issues are minor and mostly relate to edge cases and documentation. The PR is ready to merge with the above suggestions noted for potential future improvements.

Approval: ✅ Approved with minor suggestions

---

Review generated with assistance from Claude Code