Add Herdr plugin health check to doctor - #185
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's GuideAdds a Herdr-aware Sequence diagram for Herdr plugin health doctor checksequenceDiagram
participant Doctor
participant HerdrCLI
participant PluginInventory
participant CommandLogs
participant HealthAssessment
Doctor->>Doctor: checkHerdrPluginHealth()
alt HERDR_ENV is not 1
Doctor-->>Doctor: pass: not running inside Herdr, skipped
else Herdr session
Doctor->>HerdrCLI: plugin list --json
HerdrCLI-->>PluginInventory: installed plugin manifests
Doctor->>HerdrCLI: plugin log list
HerdrCLI-->>CommandLogs: command execution logs
Doctor->>HealthAssessment: assessHerdrPluginHealth(plugins, logs, lookup)
HealthAssessment->>HealthAssessment: herdrCommandApplies()
HealthAssessment->>HealthAssessment: missingHerdrCommandPath()
HealthAssessment-->>Doctor: pass or actionable warning
end
Flow diagram for Herdr plugin health assessmentflowchart TD
A["Herdr plugin health check"] --> B{HERDR_ENV = 1?}
B -- No --> C["Pass: skipped outside Herdr"]
B -- Yes --> D["Read plugin manifests and command logs"]
D --> E["Select latest log per plugin and command"]
E --> F{"Platform applies?"}
F -- No --> G["Ignore command"]
F -- Yes --> H{"Declared command file missing?"}
H -- Yes --> I["Warn: reinstall or update plugin"]
H -- No --> J{"Latest command log failed?"}
J -- No --> K["Command is healthy"]
J -- Yes --> L{"Herdr server PATH failure?"}
L -- Yes --> M["Warn with resolved path or PATH guidance"]
L -- No --> N["Warn with log ID and error/stderr detail"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="internal/app/doctor_herdr.go" line_range="163-177" />
<code_context>
+}
+
+func missingHerdrCommandPath(pluginRoot string, command []string) string {
+ for i, token := range command {
+ if i == 0 && !strings.ContainsRune(token, filepath.Separator) {
+ continue
+ }
+ if strings.HasPrefix(token, "-") || strings.ContainsAny(token, " \t\n\r$\"'`") || (!strings.HasPrefix(token, ".") && !strings.ContainsRune(token, filepath.Separator)) {
+ continue
+ }
+ candidate := token
+ if !filepath.IsAbs(candidate) {
+ candidate = filepath.Join(pluginRoot, candidate)
+ }
+ if _, err := os.Stat(candidate); os.IsNotExist(err) {
+ return candidate
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** On Windows, a command path using forward slashes, such as `C:/plugin/bin/hook.exe` or `bin/hook.exe`, is not recognized as a path because the code searches only for `filepath.Separator` (`\\`). The first token is skipped and missing files are therefore reported as healthy.
**Triggers:** When a Windows manifest uses `/` as the path separator.
**Suggested fix:** Use path-aware checks such as `filepath.IsAbs` and `filepath.Clean`, or recognize both slash forms when identifying command paths.
```suggestion
for i, token := range command {
if i == 0 && !strings.ContainsAny(token, `/\`) {
continue
}
if strings.HasPrefix(token, "-") || strings.ContainsAny(token, " \t\n\r$\"'`") || (!strings.HasPrefix(token, ".") && !strings.ContainsAny(token, `/\`)) {
continue
}
candidate := token
if !filepath.IsAbs(candidate) {
candidate = filepath.Join(pluginRoot, candidate)
}
if _, err := os.Stat(candidate); os.IsNotExist(err) {
return candidate
}
}
```
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: internal/app/doctor_herdr.go:177
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Adds a
herdr pluginscheck todotagents doctorand ports the in-progressdoctor_herdrwork into the newinternal/applayout (it was still sitting incmd/dotagentswith the pre-refactorpackage main, which no longer compiled after #184).The check runs only inside a Herdr session (
HERDR_ENV=1) and inspects the installed plugin manifests and their command logs:failedwarns with the log id and error/stderr detailManifest platform names are matched against
runtime.GOOSwithmacosmapped todarwin.Verification
go test ./internal/...passes, including 5 tests for the new logic (server PATH mismatch, recovered failure, platform mapping, inline shell program, missing hook file)go build ./...andgo vet ./...are cleangolangci-lint v2.12.2on a cold cache with the Go 1.24.2 toolchain reports 0 issuesherdr plugins .................. pass (5 installed plugins have valid commands and no current command failures)No behavior change to existing checks;
cmd/dotagentsstays a thin entrypoint.Summary by Sourcery
Add Herdr plugin health diagnostics to
dotagents doctor.New Features:
dotagents doctorthat validates installed plugin commands and reports actionable issues.Bug Fixes:
Enhancements:
macosto Darwin.Tests: