[generator] add recursive component discovery for GitHub repositories - #909
[generator] add recursive component discovery for GitHub repositories#909ShigrafS wants to merge 11 commits into
Conversation
|
Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, you can find updates in the #github-notifications channel in the community Slack. |
Summary of ChangesHello @ShigrafS, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves Meshery's ability to process component definitions from GitHub repositories by implementing recursive directory traversal. Users can now specify a root directory, and the system will automatically discover and process relevant files nested within subdirectories, overcoming previous limitations with deeply structured repositories. The new functionality is highly configurable, allowing fine-grained control over the depth of the recursive search, and has been integrated seamlessly while maintaining compatibility with existing operations. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces recursive component discovery for GitHub repositories, a valuable enhancement. The implementation adds options for recursion and depth control, and updates relevant parts of the generator logic. My review has identified a couple of high-severity issues: one is a bug in the recursion handling logic that can cause incorrect behavior, and the other is an off-by-one error in the max depth calculation which leads to counter-intuitive results. Additionally, the new tests for the recursive functionality are currently quite minimal and should be expanded to ensure the feature is robust. Overall, the changes are in the right direction, but these key issues should be addressed before merging.
e177431 to
444c8fc
Compare
There was a problem hiding this comment.
Pull request overview
Adds configurable recursive component discovery for GitHub-based generators by propagating recursion/depth/extension options down into the git walker, and introducing tests around recursive traversal behavior.
Changes:
- Introduces
GeneratorOptionsand threads recursive options intoGitHubPackageManager/downloaders. - Updates GitHub git downloader (
GitRepo) and git walker to support max depth and extension filtering during recursive traversal. - Adds unit tests for recursive walk behavior and option propagation.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
utils/walker/git.go |
Adds MaxDepth and extension filtering, and applies max-depth logic during WalkDir traversal. |
generators/github/scheme_interface.go |
Passes recursive options into the git downloader path via GitHubPackageManager. |
generators/github/package_manager.go |
Stores recursion options on the package manager and passes them into downloader creation. |
generators/github/git_repo.go |
Propagates recursion/maxDepth/extensions into the walker and adjusts root handling for recursive mode. |
generators/generator.go |
Adds GeneratorOptions and a new constructor that forwards options to GitHub generator. |
generators/generator_test.go |
Verifies option propagation through NewGeneratorWithOptions. |
generators/github/recursive_test.go |
Adds functional tests for recursive walking, max depth, and extension filtering. |
Comments suppressed due to low confidence (1)
utils/walker/git.go:242
- After calling
d.Info(), the code checksif err != nilbuterris not the error returned byd.Info()(it’s an outer variable). This meanserrInfocan be non-nil and the code will still proceed tog.readFile(f, path)with a potentially nilFileInfo, which can panic. CheckerrInfoinstead and return it when non-nil.
f, errInfo := d.Info()
if err != nil {
return errInfo
}
return g.readFile(f, path)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Strong first PR, @ShigrafS — the recursive discovery feature with depth/extension control is well-architected. Several important issues to address:
The functional test with a real git repo is excellent — well done on that. |
a31d4f6 to
2e1dfaf
Compare
|
Thank you for the detailed review @leecalcote The PR now fully resolves the issues identified in the review while maintaining backward compatibility. |
3d97e47 to
fb4177c
Compare
f0abf53 to
a85d12d
Compare
e2e73bf to
d951716
Compare
180b584 to
ac285f0
Compare
📝 WalkthroughWalkthroughThe change adds generator options for recursive Git traversal, maximum depth, and file extensions. GitHub downloaders pass these settings to ChangesRecursive generation configuration and traversal
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Generator
participant GitHubPackageManager
participant GitRepo
participant Git
Generator->>GitHubPackageManager: configure recursive, depth, and extensions
GitHubPackageManager->>GitRepo: pass repository configuration
GitRepo->>Git: apply traversal settings
Git->>GitRepo: return matching repository files
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@generators/generator_test.go`:
- Around line 10-17: Extend the table-driven cases in the generator tests with
expected extension values through GeneratorOptions.Extensions, then assert the
resulting ghpm.Extensions matches tt.opts.Extensions after
NewGeneratorWithOptions. Ensure the test fails if Extensions is no longer
forwarded, while preserving existing assertions for the other generator fields.
In `@utils/walker/git.go`:
- Around line 92-104: The non-recursive file traversal must honor extension
filtering just like the recursive path. In the non-recursive loop, apply
Git.isAllowedFile to each filename before calling g.readFile, while preserving
the existing behavior when no extensions are configured; add a test covering
non-recursive traversal with an allowed extension that excludes other files.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c1472fe-6953-42c7-8a73-40d167904af4
📒 Files selected for processing (7)
generators/generator.gogenerators/generator_test.gogenerators/github/git_repo.gogenerators/github/package_manager.gogenerators/github/recursive_test.gogenerators/github/scheme_interface.goutils/walker/git.go
| tests := []struct { | ||
| name string | ||
| registrant string | ||
| url string | ||
| packageName string | ||
| opts GeneratorOptions | ||
| wantRec bool | ||
| wantDepth int |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Test Extensions propagation.
Add expected extensions to each test case. Assert that ghpm.Extensions equals tt.opts.Extensions.
The current test can pass if NewGeneratorWithOptions stops forwarding Extensions.
As per coding guidelines, “Every behavioral change must include accompanying tests.”
Also applies to: 52-58
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@generators/generator_test.go` around lines 10 - 17, Extend the table-driven
cases in the generator tests with expected extension values through
GeneratorOptions.Extensions, then assert the resulting ghpm.Extensions matches
tt.opts.Extensions after NewGeneratorWithOptions. Ensure the test fails if
Extensions is no longer forwarded, while preserving existing assertions for the
other generator fields.
Source: Coding guidelines
| func (g *Git) isAllowedFile(name string) bool { | ||
| if len(g.allowedExtensions) == 0 { | ||
| return true // no filtering | ||
| } | ||
|
|
||
| ext := strings.ToLower(filepath.Ext(name)) | ||
| for _, allowed := range g.allowedExtensions { | ||
| if ext == allowed { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Apply extension filtering in non-recursive mode.
The recursive branch calls g.isAllowedFile. The non-recursive loop reads every file without this check.
As a result, AllowedExtensions([]string{".yaml"}) still returns JSON and other files when recursion is disabled. Apply the same check before g.readFile in the non-recursive loop. Add a non-recursive filtered test case.
Proposed fix
if f.IsDir() {
continue
}
+ if !g.isAllowedFile(f.Name()) {
+ continue
+ }
err := g.readFile(f, fPath)Also applies to: 249-250
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@utils/walker/git.go` around lines 92 - 104, The non-recursive file traversal
must honor extension filtering just like the recursive path. In the
non-recursive loop, apply Git.isAllowedFile to each filename before calling
g.readFile, while preserving the existing behavior when no extensions are
configured; add a test covering non-recursive traversal with an allowed
extension that excludes other files.
Signed-off-by: ShigrafS <shigrafsalik@proton.me>
Signed-off-by: ShigrafS <shigrafsalik@proton.me>
Signed-off-by: ShigrafS <shigrafsalik@proton.me>
Signed-off-by: ShigrafS <shigrafsalik@proton.me>
Signed-off-by: ShigrafS <shigrafsalik@proton.me> # Conflicts: # generators/github/git_repo.go
Signed-off-by: ShigrafS <shigrafsalik@proton.me>
Signed-off-by: ShigrafS <shigrafsalik@proton.me> # Conflicts: # generators/github/recursive_test.go
Signed-off-by: ShigrafS <shigrafsalik@proton.me>
Signed-off-by: ShigrafS <shigrafsalik@proton.me>
Signed-off-by: ShigrafS <shigrafsalik@proton.me>
…ions - Computed effectiveRoot before calling Root() to prevent recursive mutation bugs - Added er != nil check in WalkDir to prevent panic on nil DirEntry - Normalized extension inputs in AllowedExtensions to guarantee accurate file filtering - Corrected the error variable check from err to errInfo after d.Info() - Replaced deprecated ioutil functions with os equivalents in tests Signed-off-by: ShigrafS <shigrafsalik@proton.me>
Closes #726
Description
This PR implements recursive component discovery for Meshery’s generator logic, addressing the limitation where only files directly under a specified directory were processed. With this enhancement:
Users can point to any root directory in a repository, and Meshery will automatically discover all component definition files in subdirectories.
Recursive search can be enabled via options (
GeneratorOptions). By default, recursion is disabled for backward compatibility.MaxDepth)*.yaml,*.yml,*.json)RecursiveoptionBackward compatibility is maintained for existing workflows.
Relevant CLI commands and API endpoints have been updated to support these options.
Changes include updates to:
generators/generator.go&generator_test.go– AddedGeneratorOptionsfor recursion and depth.generators/github/git_repo.go&package_manager.go– GitRepo and GitHubPackageManager updated to handle recursive directory traversal.generators/github/scheme_interface.go– Downloader updated to pass recursive options.utils/walker/git.go– File walker now respects recursion and max depth.recursive_test.go).This fixes the configuration fragility for repositories with deeply nested component files and aligns with the desired behavior in #726 .
Notes for Reviewers
filepath.WalkDirwith max depth checks inutils/walker/git.go.Recursive: falseensures backward compatibility; recursion is only enabled if explicitly set inGeneratorOptions.MaxDepthlogic and that recursion is disabled whenRecursive: false.[Signed commits](../CONTRIBUTING.md#signing-off-on-commits-developer-certificate-of-origin)
Summary by CodeRabbit
New Features
Bug Fixes