feat: dependency check#17
Merged
Merged
Conversation
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds optional dependency-health collection from direct go.mod requires, resolves GitHub module paths, enriches dependency repositories with release/commit/workflow/license/SBOM/PR metrics, refactors evaluation to run per dependency, and adds types, tests, README updates, and a design document. ChangesDependency Health Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds optional Go (go.mod) direct dependency parsing and per-dependency policy evaluation so the plugin can emit dependency health / supply-chain evidence (repository resolution, activity, workflows, PR signals, license, SBOM summary) when enabled.
Changes:
- Introduces a dependency data model (
RepositoryDependency+ health/supply-chain/status structs) and ago.modparser + GitHub-based dependency health collector. - Extends plugin configuration to enable/limit dependency health collection, and updates evaluation to route “dependency behavior” policies per dependency.
- Updates documentation and bumps
compliance-framework/agentplus addsgolang.org/x/mod.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
types.go |
Adds dependency-related types used as policy input/output. |
dependencies.go |
Implements go.mod parsing, GitHub repo resolution, and dependency health/supply-chain collection. |
dependencies_test.go |
Adds unit + end-to-end style HTTP-mocked tests for parsing/collection and dependency policy evaluation. |
main.go |
Adds dependency health config parsing and splits policy evaluation into repository vs dependency behavior. |
README.md |
Documents optional dependency collection, required permissions, and new config knobs. |
dependency-health-design.md |
Adds detailed design doc for dependency health evidence and policy model. |
go.mod / go.sum |
Bumps agent version and adds golang.org/x/mod. |
.gitignore |
Ignores built plugin-github-repositories artifact. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@dependencies_test.go`:
- Around line 391-394: The test populates data.PolicyData on the
SaturatedRepository but still calls EvaluatePolicies with nil (lines around the
EvaluatePolicies call), so the dependency-level policy_data path is never
exercised; update the test to pass the dependency policy data into
EvaluatePolicies (e.g., pass data.PolicyData or the specific dependency map
instead of nil) when invoking EvaluatePolicies in the test that builds
SaturatedRepository, and adjust assertions to verify that dependency-specific
thresholds from PolicyData are applied by the
EvaluatePolicies/ApplyDependencyPolicies code paths (refer to
SaturatedRepository, PolicyData, and EvaluatePolicies).
- Around line 420-453: Replace direct protobuf field accesses with protobuf
getter methods in the assertions: use ev.GetLabels() instead of ev.Labels (e.g.,
ev.GetLabels()["type"]), use foo.GetSubjects() and
foo.GetSubjects()[0].GetIdentifier() instead of foo.Subjects/Identifier and use
foo.GetLinks() when calling evidenceHasHref(foo, ...) (or adjust evidenceHasHref
to accept links), and similarly use bar.GetProps() and
bar.GetProps()[0].GetValue() and bar.GetLinks() instead of bar.Props and
bar.Links; update the length checks to len(foo.GetSubjects()) and any other
places referencing these fields to consistently use Get*() on proto.Evidence
(symbols: byDependency, evidence slice, foo, bar, evidenceHasHref).
In `@dependencies.go`:
- Around line 507-527: The listPullRequestIssues function currently only
requests Page:1 and ignores pagination, undercounting PRs; update it to paginate
by calling l.githubClient.Issues.ListByRepo repeatedly, collecting results
across pages: call ListByRepo with opts (including Since), append returned
issues to a single slice, inspect the returned *github.Response (resp.NextPage)
and loop setting opts.Page = resp.NextPage until NextPage == 0 or no more
results, preserving the same PR filter (issue.IsPullRequest()) and error
handling used in listPullRequestIssues.
In `@dependency-health-design.md`:
- Around line 577-584: The policy title string is inverted for stale-activity
violations; change the title variable (title := "...") to reflect a violation
(e.g., "Direct dependency has no recent upstream activity" or "Direct dependency
shows no recent upstream activity") and ensure the description (description :=
"...") remains the healthy/explanatory text already suggested so evidence
emitted uses the corrected title and matching description.
In `@main.go`:
- Around line 225-229: The code currently logs raw policy_data via
l.Logger.Debug which can leak secrets/PII; change all occurrences (e.g., the
l.Logger.Debug call using "policy_data" with config.policyData and any similar
logs in the evaluation loops at the referenced locations) to avoid dumping
values — log only mapKeys(config.policyData), counts (len), or a redacted
summary instead of the full map; update any places that pass config.policyData
into structured log fields to pass keys/count or a
redactPolicyData(config.policyData) summary and remove direct value exposure.
In `@README.md`:
- Around line 40-45: The README currently documents configuration keys
(dependency_health_enabled, dependency_health_max_dependencies,
dependency_health_closed_pr_lookback_days, dependency_health_include_unresolved,
dependency_health_collect_sbom, dependency_health_pr_interaction_sample_size)
using quoted strings; change these to typed YAML scalars (use unquoted booleans
true/false and unquoted integers for numeric values) so consumers copying the
snippet will have correct types and avoid parser coercion or decoding failures.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a15813a5-3529-43e2-b0d2-516927a77380
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
.gitignoreREADME.mddependencies.godependencies_test.godependency-health-design.mdgo.modmain.gotypes.go
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
dependency-health-design.md (1)
1034-1045:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign the identifier examples with the implementation in this PR.
The code now emits dependency identifiers as
github-repository-dependency/<repo>/<module>@<version>, but this section still documentsdependency/go/.... Leaving both formats in the same PR makes the evidence identifier contract ambiguous for policy authors and downstream tooling.🤖 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 `@dependency-health-design.md` around lines 1034 - 1045, Update the documentation examples to match the implementation that emits identifiers as github-repository-dependency/<repo>/<module>@<version> instead of the old dependency/go/... format; replace the recommended identifier example and the future inventory example with the actual contract used by the code (e.g., github-repository-dependency/github.com/example/lib@v1.2.3) and add a short note that these identifiers are stable and can be promoted to first-class inventory items, ensuring the doc strings exactly match the emitted identifier pattern used by the PR.main.go (1)
510-516:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBuffer dependency evidence until the whole repository succeeds.
CreateEvidencehappens inside the per-dependency callback. If a later dependency evaluation or write fails,EvalreturnsFAILUREafter earlier dependency findings have already been persisted. That leaves partial assessment state behind and can duplicate findings on retry. Accumulate the dependency evidences for this repository and submit them only aftergatherRepositoryDependenciescompletes without error.🤖 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 `@main.go` around lines 510 - 516, The current code calls apiHelper.CreateEvidence inside the per-dependency callback passed to l.gatherRepositoryDependencies (where EvaluatePolicies is invoked), causing evidence to be persisted incrementally and leaving partial state on later failure; instead, collect/append each dependency's evidences into a local slice (e.g., depsEvidences) defined outside the callback in main.go, have the callback return nil after appending, and after l.gatherRepositoryDependencies returns successfully call apiHelper.CreateEvidence once with the accumulated slice; ensure you only call CreateEvidence when gatherRepositoryDependencies returns no error so evidences are only persisted on complete success.dependencies.go (1)
400-408:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
workflows.TotalCountfor workflow count
collectDependencyWorkflowscallsActions.ListWorkflows(..., nil)and setssummary.Count = len(workflows.Workflows), which only reflects the number of workflows returned in that response page. Since go-github’sWorkflowsresponse includesTotalCount *int, use*workflows.TotalCount(with a nil fallback) to avoid undercounting repositories with more workflows than the returned slice size.🤖 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 `@dependencies.go` around lines 400 - 408, The code in collectDependencyWorkflows uses len(workflows.Workflows) which only reflects the current page; change assignment of summary.Count to use the TotalCount field from the go-github response instead: when workflows != nil set summary.Count to the dereferenced value of workflows.TotalCount with a safe nil fallback (e.g., 0) so repositories with more workflows than the returned slice are counted correctly.
🤖 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.
Outside diff comments:
In `@dependencies.go`:
- Around line 400-408: The code in collectDependencyWorkflows uses
len(workflows.Workflows) which only reflects the current page; change assignment
of summary.Count to use the TotalCount field from the go-github response
instead: when workflows != nil set summary.Count to the dereferenced value of
workflows.TotalCount with a safe nil fallback (e.g., 0) so repositories with
more workflows than the returned slice are counted correctly.
In `@dependency-health-design.md`:
- Around line 1034-1045: Update the documentation examples to match the
implementation that emits identifiers as
github-repository-dependency/<repo>/<module>@<version> instead of the old
dependency/go/... format; replace the recommended identifier example and the
future inventory example with the actual contract used by the code (e.g.,
github-repository-dependency/github.com/example/lib@v1.2.3) and add a short note
that these identifiers are stable and can be promoted to first-class inventory
items, ensuring the doc strings exactly match the emitted identifier pattern
used by the PR.
In `@main.go`:
- Around line 510-516: The current code calls apiHelper.CreateEvidence inside
the per-dependency callback passed to l.gatherRepositoryDependencies (where
EvaluatePolicies is invoked), causing evidence to be persisted incrementally and
leaving partial state on later failure; instead, collect/append each
dependency's evidences into a local slice (e.g., depsEvidences) defined outside
the callback in main.go, have the callback return nil after appending, and after
l.gatherRepositoryDependencies returns successfully call
apiHelper.CreateEvidence once with the accumulated slice; ensure you only call
CreateEvidence when gatherRepositoryDependencies returns no error so evidences
are only persisted on complete success.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 109b034e-3367-4a52-a362-1b15cdc22fdd
📒 Files selected for processing (4)
dependencies.godependencies_test.godependency-health-design.mdmain.go
Contributor
Author
|
lisa: done. no actionable review threads remaining. |
gusfcarvalho
commented
May 21, 2026
Contributor
Author
gusfcarvalho
left a comment
gusfcarvalho
left a comment
There was a problem hiding this comment.
I like submitting reviews. (this is a test)
There was a problem hiding this comment.
♻️ Duplicate comments (1)
README.md (1)
40-45:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep dependency-health config values as quoted strings to match the parser contract.
Line 40-45 now documents typed YAML booleans/integers, but these fields are parsed as strings in plugin config. This makes docs inconsistent with implementation and can break copied configs depending on decoding path.
Suggested README fix
- dependency_health_enabled: false - dependency_health_max_dependencies: 50 - dependency_health_closed_pr_lookback_days: 180 - dependency_health_include_unresolved: true - dependency_health_collect_sbom: true - dependency_health_pr_interaction_sample_size: 20 + dependency_health_enabled: "false" + dependency_health_max_dependencies: "50" + dependency_health_closed_pr_lookback_days: "180" + dependency_health_include_unresolved: "true" + dependency_health_collect_sbom: "true" + dependency_health_pr_interaction_sample_size: "20"Based on learnings: dependency health config fields in
PluginConfigare declared asstringand manually parsed, so the README example should use quoted string values.🤖 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 `@README.md` around lines 40 - 45, The README shows dependency_health fields as raw YAML booleans/integers but the PluginConfig expects string-typed values; update the example lines for dependency_health_enabled, dependency_health_max_dependencies, dependency_health_closed_pr_lookback_days, dependency_health_include_unresolved, dependency_health_collect_sbom, and dependency_health_pr_interaction_sample_size to use quoted string values (e.g. "false", "50", "180", "true", "true", "20") so the documented config matches the PluginConfig string contract and manual parsing.
🤖 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.
Duplicate comments:
In `@README.md`:
- Around line 40-45: The README shows dependency_health fields as raw YAML
booleans/integers but the PluginConfig expects string-typed values; update the
example lines for dependency_health_enabled, dependency_health_max_dependencies,
dependency_health_closed_pr_lookback_days, dependency_health_include_unresolved,
dependency_health_collect_sbom, and dependency_health_pr_interaction_sample_size
to use quoted string values (e.g. "false", "50", "180", "true", "true", "20") so
the documented config matches the PluginConfig string contract and manual
parsing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: daf2217a-2eea-48ba-a619-eff919a0c1be
📒 Files selected for processing (6)
README.mddependencies.godependencies_test.godependency-health-design.mdmain.gotypes.go
There was a problem hiding this comment.
♻️ Duplicate comments (1)
dependencies.go (1)
18-18:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIncrease PR page cap to avoid heavily biased health metrics.
Line 18 sets
dependencyPRMaxPagesto1, which truncates PR-derived metrics to at most 100 items per state and can significantly skewOpenCount,RecentClosedCount, and median calculations for active repositories.Suggested minimal fix
- dependencyPRMaxPages = 1 + dependencyPRMaxPages = 10🤖 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 `@dependencies.go` at line 18, The constant dependencyPRMaxPages is set to 1 which limits PR-derived metrics; update dependencyPRMaxPages to a higher sensible default (e.g., 5) or wire it to a configurable source (env/config) so PR pagination fetches more pages; locate the dependencyPRMaxPages definition and replace the literal 1 with the chosen default or a config lookup (e.g., read from config.GetInt/ENV) and ensure any callers that rely on this constant continue to accept an int.
🤖 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.
Duplicate comments:
In `@dependencies.go`:
- Line 18: The constant dependencyPRMaxPages is set to 1 which limits PR-derived
metrics; update dependencyPRMaxPages to a higher sensible default (e.g., 5) or
wire it to a configurable source (env/config) so PR pagination fetches more
pages; locate the dependencyPRMaxPages definition and replace the literal 1 with
the chosen default or a config lookup (e.g., read from config.GetInt/ENV) and
ensure any callers that rely on this constant continue to accept an int.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 61385e3b-9afd-4468-a081-49440e61aaa8
📒 Files selected for processing (4)
.github/workflows/test.ymlREADME.mddependencies.godependencies_test.go
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
main.go:189
- Renaming
policy_inputtopolicy_datain the repository evaluation payload is a breaking change for any existing policies that readinput.policy_input. Consider including a backward-compatible alias field (or populating both JSON keys) for at least one release cycle, or clearly documenting the breaking change and required policy updates.
RepositoryTeams []*RepositoryTeam `json:"repository_teams"`
Environments []*RepositoryEnvironment `json:"environments"`
EffectiveBranchRules map[string]*BranchRuleEvidence `json:"effective_branch_rules"`
PolicyData map[string]interface{} `json:"policy_data"`
}
Contributor
Author
|
lisa: done. all review threads addressed (via reply or fix). |
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
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
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.
Introduces the ability to parse a repository dependencies (for now golang apps only)
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
CI