Support for the .agents/skills standard #1609
Conversation
Signed-off-by: Christopher Petito <chrisjpetito@gmail.com>
There was a problem hiding this comment.
Review Summary
Reviewed the .agents/skills standard support implementation. Found one minor inconsistency in error handling for path resolution. The code is well-tested with comprehensive test coverage. The new features work correctly overall.
Found Issues: 1 medium severity issue (inconsistency in return value types)
| // Not in a git repo, just return cwd | ||
| return []string{absPath} | ||
| } | ||
|
|
There was a problem hiding this comment.
Inconsistent return value types - may return relative path in error case while returning absolute path in success case
The projectSearchDirs function has an inconsistency: when filepath.Abs fails (line 98), it returns []string{cwd} which could be a relative path, but when gitRoot is empty (line 105), it returns []string{absPath} which is an absolute path.
While this doesn't cause immediate functional failures because filepath.Join and os.ReadDir both handle relative and absolute paths, it represents a logic inconsistency that violates the function's implicit contract. The comment on lines 89-92 says "returns directories" without specifying absolute vs relative.
This inconsistency could cause subtle issues with path handling, logging, or if the paths are later used in contexts that expect absolute paths.
Suggestion: Return []string{absPath} in the error case on line 98 as well, or return an empty slice []string{} if Abs fails, rather than returning the potentially relative cwd. This would maintain consistency that all returned paths are absolute.
No description provided.