Skip to content

feat: [#859] Generate Agent Skills#1412

Open
krishankumar01 wants to merge 3 commits intomasterfrom
kkumar-gcc/#859
Open

feat: [#859] Generate Agent Skills#1412
krishankumar01 wants to merge 3 commits intomasterfrom
kkumar-gcc/#859

Conversation

@krishankumar01
Copy link
Copy Markdown
Member

@krishankumar01 krishankumar01 commented Mar 15, 2026

📑 Description

Closes goravel/goravel#859

Summary by CodeRabbit

  • New Features
    • Added agents:install command to install AI agent skill files for your Goravel version with selective facade installation or bulk installation options
    • Added agents:update command to keep installed agents current, with conflict detection and resolution handling
    • Automatic version detection and branch resolution for seamless agent file management

✅ Checks

  • Added test cases for my code

@krishankumar01 krishankumar01 requested a review from a team as a code owner March 15, 2026 13:36
Comment thread ai/console/helpers.go Fixed
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 15, 2026

Codecov Report

❌ Patch coverage is 45.80153% with 213 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.86%. Comparing base (0cb7658) to head (baccd1b).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
ai/console/update_command.go 42.63% 62 Missing and 12 partials ⚠️
ai/console/helpers.go 41.22% 60 Missing and 7 partials ⚠️
ai/console/install_command.go 55.71% 56 Missing and 6 partials ⚠️
ai/service_provider.go 0.00% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1412      +/-   ##
==========================================
- Coverage   67.18%   66.86%   -0.33%     
==========================================
  Files         348      351       +3     
  Lines       25609    26001     +392     
==========================================
+ Hits        17206    17386     +180     
- Misses       7674     7861     +187     
- Partials      729      754      +25     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hwbrzzl
Copy link
Copy Markdown
Contributor

hwbrzzl commented Mar 16, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 16, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 16, 2026

📝 Walkthrough

Walkthrough

Introduces an AI agent skills system with helpers for version tracking, manifest fetching, and branch resolution. Adds two new console commands: AgentsInstallCommand to install agent skill files from goravel/docs with version and facade selection, and AgentsUpdateCommand to update installed files with conflict detection. Integrates both commands into the service provider.

Changes

Cohort / File(s) Summary
Version & Manifest Helpers
ai/console/helpers.go, ai/console/helpers_test.go
Introduces VersionFile and ManifestEntry types for tracking agent versions and manifest metadata. Adds utilities for Goravel version detection from go.mod, branch resolution, manifest fetching with HTTP retry logic, SHA256 hashing, and file I/O with automatic directory creation.
Install Command
ai/console/install_command.go, ai/console/install_command_test.go
Implements command to install agent skill files with version/branch resolution (auto-detect or explicit), manifest retrieval with fallback to master branch, concurrent file downloads, selective installation by facade or --all flag, and local version tracking via .version file.
Update Command
ai/console/update_command.go, ai/console/update_command_test.go
Implements command to update agent files with SHA256-based change detection, conflict handling (local modified vs upstream), --force override flag, and support for selective updates by facade or --all. Tracks up-to-date, updated, skipped, and conflicted entries.
Service Provider Integration
ai/service_provider.go
Registers both new commands via a helper function during service provider boot.

Sequence Diagram(s)

sequenceDiagram
    participant User as User/CLI
    participant Install as Install Command
    participant Resolver as Version Resolver
    participant Manifest as Manifest Fetcher
    participant File as File Fetcher
    participant Local as Local Storage

    User->>Install: Execute with version/facade args
    Install->>Resolver: resolveVersionAndBranch()
    alt Version provided
        Resolver->>Resolver: Validate & resolve branch
    else Auto-detect
        Resolver->>Resolver: Parse go.mod
        alt go.mod parse fails
            Resolver->>Manifest: Fetch available branches
            Manifest-->>Resolver: Branch list
            Resolver->>User: Prompt for selection
            User-->>Resolver: Selected version
        end
    end
    Resolver-->>Install: version, branch
    
    Install->>Manifest: Fetch manifest for branch
    alt Empty manifest & not fallback branch
        Manifest->>Manifest: Try fallback (master)
    end
    Manifest-->>Install: ManifestEntry list
    
    Install->>Install: Filter by facades/--all/defaults
    Install->>File: Download files concurrently
    File-->>Install: File contents ([]byte)
    
    Install->>Local: Read existing .version file
    Install->>Local: Merge metadata & update SHA256s
    Install->>Local: Write files to disk
    Install->>Local: Write updated .version
    Install->>User: Report success summary
Loading
sequenceDiagram
    participant User as User/CLI
    participant Update as Update Command
    participant Local as Local Storage
    participant Manifest as Manifest Fetcher
    participant File as File Fetcher
    participant Conflict as Conflict Detector

    User->>Update: Execute with facade/--all args
    Update->>Local: Read .version file
    alt No .version
        Update->>User: Error: not installed
    else Version exists
        Update->>Update: Validate version support
        alt Unsupported
            Update->>User: Error: upgrade Goravel
        else Supported
            Update->>Manifest: Fetch manifest
            Manifest-->>Update: ManifestEntry list
            Update->>Update: Filter installed entries
            
            loop For each target entry
                Update->>File: Fetch upstream file
                File-->>Update: Content
                Update->>Local: Read local file
                Local-->>Update: Content + SHA256
                Update->>Conflict: Compare hashes
                
                alt Hashes match
                    Conflict-->>Update: Up-to-date
                else Local modified, upstream same
                    Conflict-->>Update: Skipped
                else Both modified & no --force
                    Conflict-->>Update: Conflict
                else Can update
                    Update->>Local: Write file
                    Update->>Local: Update SHA256
                    Conflict-->>Update: Updated
                end
            end
            
            Update->>Local: Write merged .version
            Update->>User: Report summary (updated/skipped/conflicts/up-to-date)
        end
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

  • fix: PrepareForValidation Context fixes [#589] #859: Generate Agent Skills — This PR directly implements the core features outlined in the linked issue: the install command pulls agent files from goravel/docs, the update command maintains version synchronization with conflict detection, and local version tracking via .version file with SHA256 hashing enables change detection.

Suggested labels

🚀 Review Ready

Poem

🐰 Hops through versions with glee,
Manifests fetch'd from docs with ease,
Skills installed, updates bloom,
Conflicts fade in goravel's room,
AI agents now hop with thee! 🎯✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary feature: generating agent skills via linked issue #859, directly mapping to the PR's main changes.
Linked Issues check ✅ Passed The PR successfully implements all core requirements from #859: install command (with version selection), update command (with version and facade selection), version file tracking, and manifest integration from goravel/docs.
Out of Scope Changes check ✅ Passed All changes directly support agent skills functionality: helpers for version/manifest management, two new CLI commands, integration with service provider, and corresponding tests. No unrelated changes detected.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch kkumar-gcc/#859
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ai/console/helpers_test.go`:
- Around line 38-42: The test currently ignores errors from os.Remove and
f.Close which triggers linter failures; update the teardown to check those
returns (e.g., replace defer os.Remove(f.Name()) with a deferred closure that
captures and asserts the error, and change f.Close() to capture its error and
call assert.NoError(t, err) or require.NoError(t, err)); reference the test
helper where the file is created (the variable f and the calls os.Remove and
f.Close in helpers_test.go) and ensure both removal and close errors are
asserted so the linter is satisfied and failures surface in the test runner.

In `@ai/console/helpers.go`:
- Line 156: The deferred call "defer f.Close()" leaves the Close error
unchecked; replace it with a deferred closure that captures and handles the
error (e.g., defer func() { if err := f.Close(); err != nil { /* log or
propagate the error */ } }()) so the Close() failure is either logged via the
package logger or returned to the caller; locate the occurrence of "defer
f.Close()" in helpers.go and update it to use the deferred anonymous function
handling f.Close()'s error.
- Line 176: The deferred resp.Body.Close() call currently ignores its returned
error; change the defer to wrap Close in an inline function that checks the
error (e.g., defer func() { if err := resp.Body.Close(); err != nil { /* log or
handle error */ } }()), and either log the error via the package logger or
propagate it if the surrounding function returns an error. Locate the defer
usage where resp.Body.Close() is called (the resp variable) and replace it with
this checked-close pattern.
- Line 109: The deferred resp.Body.Close() call currently ignores its returned
error; modify the function containing that line to capture and handle Close()'s
error (e.g., assign the result of resp.Body.Close(), check if err != nil and
either log it via the existing logger or return/wrap it up the call chain).
Locate the resp variable and the function that performs the HTTP request in
ai/console/helpers.go, replace the bare defer resp.Body.Close() with a deferred
closure or explicit check that captures the error and handles it consistently
with the function's error-handling strategy (log/return/wrap) so the linter no
longer flags an unchecked error.

In `@ai/console/install_command_test.go`:
- Around line 24-27: The cleanup function currently ignores errors from
os.RemoveAll(".ai") and os.Remove("AGENTS.md"), causing static analysis
failures; update the cleanup closure to capture and handle those errors (e.g.,
assign err := os.RemoveAll(".ai") and err := os.Remove("AGENTS.md") and either
ignore NotExist via if err != nil && !os.IsNotExist(err) { t.Fatalf(...) } or
explicitly discard with _ = os.RemoveAll(".ai") / _ = os.Remove("AGENTS.md")
depending on desired behavior) so the unchecked-return lint is resolved while
keeping references to the cleanup function and the two remove calls.
- Around line 160-161: The test setup is ignoring errors from os.MkdirAll and
os.WriteFile; update the test to check those returns (e.g., use require.NoError
or assert.NoError from testify) and fail the test on error. Capture the
testing.T instance in the closure or move the setup into the top-level test
function so you can call require.NoError(t, err) for the os.MkdirAll and
os.WriteFile calls that create ".ai" and write versionFilePath
(`{"version":"v1.16","files":{}}`). Ensure both calls propagate or assert
failures so pipeline static analysis and CI will detect filesystem errors.

In `@ai/console/update_command_test.go`:
- Line 14: The os.RemoveAll(".ai") call has an unchecked error; capture its
return (e.g., err := os.RemoveAll(".ai")) and handle it in the test by failing
the test on error (e.g., use t.Fatalf/t.Fatal or your test helper like
require.NoError) so the linter is satisfied and the pipeline won't fail; update
the occurrence in update_command_test.go where os.RemoveAll(".ai") is invoked.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b9043236-8a39-4ae6-b6bf-51b147ae8a59

📥 Commits

Reviewing files that changed from the base of the PR and between 0cb7658 and baccd1b.

📒 Files selected for processing (7)
  • ai/console/helpers.go
  • ai/console/helpers_test.go
  • ai/console/install_command.go
  • ai/console/install_command_test.go
  • ai/console/update_command.go
  • ai/console/update_command_test.go
  • ai/service_provider.go

Comment thread ai/console/helpers_test.go Outdated
Comment thread ai/console/helpers.go
Comment thread ai/console/helpers.go Outdated
Comment thread ai/console/helpers.go Outdated
Comment thread ai/console/ai_docs_install_command_test.go
Comment thread ai/console/install_command_test.go Outdated
Comment thread ai/console/update_command_test.go Outdated
Copy link
Copy Markdown
Contributor

@hwbrzzl hwbrzzl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, awesome. I haven't reviewed this PR carefully. I'll re-review it after the document PR is merged. And please add more details in the PR description after that.

Comment thread ai/console/install_command.go Outdated
Comment thread ai/console/install_command.go Outdated
Comment thread ai/console/install_command.go Outdated
Comment thread ai/console/update_command.go Outdated
Comment thread ai/console/update_command.go Outdated
Comment on lines +49 to +52
&command.StringFlag{
Name: "version",
Usage: "Force a specific version (e.g. v1.17)",
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This flag may be unnecessary. We should check the version according to the go.mod file.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is also there, but in case the user wants to get something specific, the priority is: first from the version, then auto-detect from the mod file, and then the interactive setup.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, I'd suggest adding the flag when users actually need it.

Copy link
Copy Markdown
Member Author

@krishankumar01 krishankumar01 Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this for my testing purpose for now as we don't publish v1.17 docs as separate branch. So I had to add so that I can pull the master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generate Agent Skills

3 participants