v0.12.1

[yimsk]

Summary

• Resource prefix match in command mode (:ec2/in → ec2/instances)
• Fix alias resource lost in prefix match (:logs → cloudwatch/log-groups)

[claude]

Code Review - PR #134

Summary

This PR adds resource prefix matching in command mode (e.g., :ec2/in → ec2/instances) and fixes an issue where alias resources were being lost during prefix matching (e.g., :logs → cloudwatch/log-groups). Overall, this is a solid improvement with good test coverage.

---

✅ Strengths

1. Excellent test coverage: Two new test functions comprehensively cover the new functionality

   • TestCommandInput_ResourcePrefixMatching validates resource prefix matching
   • TestCommandInput_AliasResourcePreservation ensures alias resources are preserved

2. Clear bug fix: The PR correctly addresses the issue where resolvePrefixMatch was discarding the resource part from ResolveAlias

3. Better code clarity: Variable naming improved (matched → matchedService, added aliasResource)

4. Simplified logic: Removed the indirect path through ParseServiceResource in favor of direct prefix matching on resources

---

🔍 Code Quality & Best Practices

Good:

• Comment updates accurately reflect the new behavior
• Code follows Go conventions
• Proper use of early returns for error cases

Minor suggestions:

1. Inconsistent behavior in resolveDestination (internal/view/command_input.go:285-290)

   • The check if resourcePart != "" && resourceType != "" might hide issues where ParseServiceResource returns a service but no resource
   • Consider whether returning just the service is the intended behavior when user types service/ (with trailing slash but no resource text)

2. Potential ambiguity with prefix matching (internal/view/command_input.go:347-351)

   • The comment says "first match = alphabetically first", but this assumes ListResources returns sorted results
   • Consider adding a comment about the ordering guarantee or explicitly sorting to ensure deterministic behavior

---

🐛 Potential Issues

1. Edge case: Multiple prefix matches (internal/view/command_input.go:316-320)

   • The code breaks on the first match when multiple services share a prefix
   • Example: If you have both ec2 and ec2-instance-connect, input ec2 will always match ec2, but what if services are not in the desired order?
   • This is likely acceptable since ListServices() should return deterministic results, but worth noting

2. Missing validation in executeCommand (internal/view/command_input.go:501-503)

   • When falling back to prefix match, the code calls DefaultResource(svc) without checking if it returns an empty string
   • If no default resource exists, this could create a browser with an empty resource type
   • Recommendation: Add validation for empty default resources

---

⚡ Performance Considerations

• Acceptable: The changes do not introduce performance concerns
• Prefix matching is O(n) but operates on small lists (services, aliases, resources per service)
• Breaking on first match prevents unnecessary iterations

---

🔒 Security Concerns

• None identified: The code deals with user input matching against a registry, no security issues detected
• Proper use of strings.SplitN with limit prevents potential DoS from excessive splits

---

📋 Test Coverage

Strong coverage for happy paths:

• ✅ Resource prefix matching with multiple resources
• ✅ Alias resolution preserving resources
• ✅ Exact matches

Missing edge case tests:

• ❌ What happens with service/ (trailing slash, no resource)?
• ❌ What happens when prefix matches multiple resources (e.g., ec2/i matching both instances and images)?
• ❌ What happens when an alias prefix matches but the service has no default resource?
• ❌ What happens with invalid input like // or /resource?

---

📝 Recommendations

High Priority:

1. Add validation in executeCommand for empty default resources (see issue GIFデモ #2 above)

Medium Priority:

2. Add test cases for edge cases mentioned in Test Coverage section
3. Consider documenting the prefix matching behavior (first alphabetical match) in user-facing docs

Low Priority (Optional):

4. Consider adding explicit sorting of ListResources result if not already guaranteed
5. Consider logging when prefix matching is used vs exact matching for debugging purposes

---

✨ Conclusion

This is a well-implemented feature with good test coverage. The code is clean and addresses the stated issues effectively. The main concern is the missing validation for empty default resources in executeCommand, which could lead to unexpected behavior.

Recommendation: Approve with minor changes - specifically adding the validation for empty default resources.

Great work overall! 🎉