Conversation
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
There was a problem hiding this comment.
Pull request overview
Updates the repo to use kagent-tools v0.1.0 across Helm packaging and Go dependencies.
Changes:
- Parameterized the Helm
kagent-toolsdependency version viaKAGENT_TOOLS_VERSION(fed byenvsubst). - Added Makefile plumbing to derive and propagate
KAGENT_TOOLS_VERSIONfromgo/core/go.mod. - Bumped Go module dependencies (including adding
github.com/kagent-dev/tools v0.1.0and various transitive updates).
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| helm/kagent/Chart-template.yaml | Uses KAGENT_TOOLS_VERSION to control the kagent-tools chart dependency version. |
| go/go.work.sum | Updates workspace sums to match the new dependency graph after the bump. |
| go/core/go.mod | Bumps kagent-dev/tools and several related dependencies. |
| Makefile | Extracts KAGENT_TOOLS_VERSION from go.mod and threads it into Helm chart generation/install. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| @@ -735,8 +889,11 @@ go.opentelemetry.io/otel/trace v1.37.0/go.mod h1:TlgrlQ+PtQO5XFerSPUYG0JSgGyryXe | |||
| go.opentelemetry.io/otel/trace v1.38.0/go.mod h1:j1P9ivuFsTceSWe1oY+EeW3sc+Pp42sO++GHkg4wwhs= | |||
| go.opentelemetry.io/otel/trace v1.39.0/go.mod h1:88w4/PnZSazkGzz/w84VHpQafiU4EtqqlVdxWy+rNOA= | |||
| go.opentelemetry.io/proto/otlp v1.7.0/go.mod h1:fSKjH6YJ7HDlwzltzyMj036AJ3ejJLCgCSHGj4efDDo= | |||
There was a problem hiding this comment.
go/core/go.mod now requires go.opentelemetry.io/proto/otlp v1.9.0 (indirect), but this diff removes the v1.9.0 entries from go.work.sum without adding replacement checksums. This can cause go commands to rewrite go/go.work.sum during CI/builds (dirty working tree) or fail checksum verification in stricter pipelines. Regenerate the workspace sums (e.g., via go work sync / go mod tidy in the workspace context) and commit the resulting go/go.work.sum so it includes the needed otlp v1.9.0 checksums.
| go.opentelemetry.io/proto/otlp v1.7.0/go.mod h1:fSKjH6YJ7HDlwzltzyMj036AJ3ejJLCgCSHGj4efDDo= | |
| go.opentelemetry.io/proto/otlp v1.7.0/go.mod h1:fSKjH6YJ7HDlwzltzyMj036AJ3ejJLCgCSHGj4efDDo= | |
| go.opentelemetry.io/proto/otlp v1.9.0 h1:t6FbyTIZU1lc7HEr/qqVrqNWeMThK7CctIP9NLlHWqo= | |
| go.opentelemetry.io/proto/otlp v1.9.0/go.mod h1:YKtlZGjCH+PLxoaHvJhGwtieFRwmo8IPNh9NYg0/CDg= |
Makefile
Outdated
| # KMCP | ||
| KMCP_ENABLED ?= true | ||
| KMCP_VERSION ?= $(shell $(AWK) '/github\.com\/kagent-dev\/kmcp/ { print substr($$2, 2) }' go/core/go.mod) # KMCP version defaults to what's referenced in go.mod | ||
| KAGENT_TOOLS_VERSION ?= $(shell $(AWK) '/github\.com\/kagent-dev\/tools/ { print substr($$2, 2) }' go/core/go.mod) # KAGENT_TOOLS version defaults to what's referenced in go.mod |
There was a problem hiding this comment.
KAGENT_TOOLS_VERSION is used as both the Helm dependency/chart version (via envsubst in Chart-template.yaml) and as the container image.tag value. Since chart versions and image tags commonly diverge (or have different formatting), the current name is ambiguous and makes it easy to wire the wrong value later. Consider splitting into distinct variables (e.g., KAGENT_TOOLS_CHART_VERSION and KAGENT_TOOLS_IMAGE_TAG) or renaming/annotating to explicitly document the intended coupling.
| KAGENT_TOOLS_VERSION ?= $(shell $(AWK) '/github\.com\/kagent-dev\/tools/ { print substr($$2, 2) }' go/core/go.mod) # KAGENT_TOOLS version defaults to what's referenced in go.mod | |
| KAGENT_TOOLS_VERSION ?= $(shell $(AWK) '/github\.com\/kagent-dev\/tools/ { print substr($$2, 2) }' go/core/go.mod) # Intentionally used for BOTH Helm chart dependency version and kagent-tools image tag; defaults to version referenced in go.mod |
Makefile
Outdated
| --set providers.default=$(KAGENT_DEFAULT_MODEL_PROVIDER) \ | ||
| --set kmcp.enabled=$(KMCP_ENABLED) \ | ||
| --set kmcp.image.tag=$(KMCP_VERSION) \ | ||
| --set kagent-tools.image.tag=$(KAGENT_TOOLS_VERSION) \ |
There was a problem hiding this comment.
KAGENT_TOOLS_VERSION is used as both the Helm dependency/chart version (via envsubst in Chart-template.yaml) and as the container image.tag value. Since chart versions and image tags commonly diverge (or have different formatting), the current name is ambiguous and makes it easy to wire the wrong value later. Consider splitting into distinct variables (e.g., KAGENT_TOOLS_CHART_VERSION and KAGENT_TOOLS_IMAGE_TAG) or renaming/annotating to explicitly document the intended coupling.
| @@ -9,7 +9,7 @@ dependencies: | |||
| repository: oci://ghcr.io/kagent-dev/kmcp/helm | |||
| condition: kmcp.enabled | |||
| - name: kagent-tools | |||
There was a problem hiding this comment.
This templates the Helm dependency version from KAGENT_TOOLS_VERSION, which the Makefile derives from the Go module version (github.com/kagent-dev/tools). That introduces a hidden coupling between Go module versioning and Helm chart artifact versioning. If the OCI Helm chart version ever differs from the Go module version, helm dependency update will fail or pull the wrong chart. A more maintainable approach is to source the chart dependency version from a Helm-specific place (or a dedicated variable), and/or add an inline comment here explaining the required version parity.
| - name: kagent-tools | |
| - name: kagent-tools | |
| # NOTE: KAGENT_TOOLS_VERSION is derived from the Go module version and must | |
| # match the Helm chart version published at oci://ghcr.io/kagent-dev/tools/helm. |
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
No description provided.