diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index a653b3e21f..28ede7e476 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -176,6 +176,10 @@ jobs: replicas: 0 kagent-tools: enabled: false + # The lifecycle suite applies its own Harness named kagent. + builtins: + harness: + enabled: false substrateWorkerPool: create: true replicas: 4 diff --git a/Makefile b/Makefile index 115b436248..56d1575df5 100644 --- a/Makefile +++ b/Makefile @@ -449,6 +449,11 @@ helm-version: helm-cleanup helm package -d "$(HELM_DIST_FOLDER)" "$(HELM_SOURCE_DIR)/kagent-crds" helm package -d "$(HELM_DIST_FOLDER)" "$(HELM_SOURCE_DIR)/kagent" +.PHONY: builtin-skills-catalog +builtin-skills-catalog: ## Regenerate the built-in skills catalog from the kagent-tools image + bash scripts/builtin-skills-catalog.sh > "$(HELM_SOURCE_DIR)/kagent/files/builtin-skills.json.tmp" + mv "$(HELM_SOURCE_DIR)/kagent/files/builtin-skills.json.tmp" "$(HELM_SOURCE_DIR)/kagent/files/builtin-skills.json" + .PHONY: helm-dev helm-dev: ## Prepare development charts with the images published by local builds @test -d "$(RUNTIME_BUILD_METADATA_DIR)" || { echo "Missing runtime image metadata. Run make build before preparing development charts." >&2; exit 1; } diff --git a/helm/kagent/Chart-template.yaml b/helm/kagent/Chart-template.yaml index a887f8c28b..d94fdee3e6 100644 --- a/helm/kagent/Chart-template.yaml +++ b/helm/kagent/Chart-template.yaml @@ -7,7 +7,7 @@ annotations: "org.opencontainers.image.source": "https://github.com/kagent-dev/kagent" dependencies: - name: kagent-tools - version: 0.3.0 + version: 0.3.1 repository: oci://ghcr.io/kagent-dev/tools/helm condition: kagent-tools.enabled - name: oauth2-proxy diff --git a/helm/kagent/files/builtin-skills.json b/helm/kagent/files/builtin-skills.json new file mode 100644 index 0000000000..79474b359e --- /dev/null +++ b/helm/kagent/files/builtin-skills.json @@ -0,0 +1,20 @@ +{ + "image": "ghcr.io/kagent-dev/kagent/tools@sha256:607cb0ac95c9905529ad1cfebeff3aa2fbf68aa81861ed3bbba9d71a7477d640", + "skills": [ + { + "name": "helm-release-management", + "description": "Inspect, upgrade, roll forward and uninstall Helm releases with the kagent-tools helm_* MCP tools. Use for questions about installed charts, release status, values, failed upgrades, or adding chart repositories.", + "path": "opt/kagent-tools-skills/skills/helm-release-management" + }, + { + "name": "istio-traffic-debugging", + "description": "Debug Istio service-mesh traffic (503s, mTLS failures, routing not applied, sidecar or ambient/waypoint issues) using the kagent-tools istio_* MCP tools. Use when requests between mesh services fail or routing rules do not take effect.", + "path": "opt/kagent-tools-skills/skills/istio-traffic-debugging" + }, + { + "name": "k8s-troubleshooting", + "description": "Diagnose unhealthy Kubernetes workloads (CrashLoopBackOff, Pending, ImagePullBackOff, OOMKilled, failing readiness, unreachable Services) using the kagent-tools k8s_* MCP tools. Use when a pod, deployment or service is not working as expected.", + "path": "opt/kagent-tools-skills/skills/k8s-troubleshooting" + } + ] +} diff --git a/helm/kagent/templates/_helpers.tpl b/helm/kagent/templates/_helpers.tpl index 8774839610..28d910765b 100644 --- a/helm/kagent/templates/_helpers.tpl +++ b/helm/kagent/templates/_helpers.tpl @@ -13,6 +13,29 @@ {{- $images | mustToJson -}} {{- end -}} +{{/* +The built-in skills catalog as JSON. builtins.skills.image and builtins.skills.list +replace the packaged catalog. Empty when turned off. +*/}} +{{- define "kagent.builtinSkillsCatalog" -}} +{{- if and (dig "builtins" "enabled" false .Values.AsMap) (dig "builtins" "skills" "enabled" false .Values.AsMap) -}} + {{- $catalog := .Files.Get "files/builtin-skills.json" | mustFromJson -}} + {{- with .Values.builtins.skills.image -}} + {{- $catalog = dict "image" . "skills" ($.Values.builtins.skills.list | default (list)) -}} + {{- end -}} + {{- if not (regexMatch "^[^[:space:]@]+@sha256:[0-9a-f]{64}$" $catalog.image) -}} + {{- fail (printf "builtins.skills.image must be pinned by sha256 digest: %q" $catalog.image) -}} + {{- end -}} + {{- range $catalog.skills -}} + {{- if not (and .name .path) -}} + {{- fail (printf "builtins.skills.list: each skill needs a name and a path, got %s" (mustToJson .)) -}} + {{- end -}} + {{- end -}} + {{- $_ := set $catalog "image" (include "kagent.mirroredImage" (dict "root" $ "image" $catalog.image)) -}} + {{- $catalog | mustToJson -}} +{{- end -}} +{{- end -}} + {{/* Create a default fully qualified app name. */}} diff --git a/helm/kagent/templates/default-harness.yaml b/helm/kagent/templates/default-harness.yaml new file mode 100644 index 0000000000..f53266be90 --- /dev/null +++ b/helm/kagent/templates/default-harness.yaml @@ -0,0 +1,19 @@ +{{- if and (dig "builtins" "enabled" false .Values.AsMap) (dig "builtins" "harness" "enabled" false .Values.AsMap) .Values.substrateWorkerPool.create }} +{{- if not .Values.builtins.harness.snapshotLocation }} +{{- fail "builtins.harness.snapshotLocation is required: set the snapshot storage location for the default Harness" }} +{{- end }} +apiVersion: api.kagent.dev/v1alpha3 +kind: Harness +metadata: + name: {{ .Values.builtins.harness.name | quote }} + namespace: {{ include "kagent.namespace" . }} + labels: + {{- include "kagent.labels" . | nindent 4 }} +spec: + kagent: {} + substrate: + workerPoolRef: + name: {{ .Values.substrateWorkerPool.name | quote }} + snapshotPolicy: + location: {{ .Values.builtins.harness.snapshotLocation | quote }} +{{- end }} diff --git a/helm/kagent/templates/k8s-agent.yaml b/helm/kagent/templates/k8s-agent.yaml new file mode 100644 index 0000000000..77aa3fe25d --- /dev/null +++ b/helm/kagent/templates/k8s-agent.yaml @@ -0,0 +1,80 @@ +{{- $catalog := include "kagent.builtinSkillsCatalog" . | default "{}" | mustFromJson }} +{{- $skill := dict }} +{{- range $catalog.skills | default (list) }} +{{- if eq .name "k8s-troubleshooting" }}{{ $skill = . }}{{ end }} +{{- end }} +{{- if and (dig "builtins" "enabled" false .Values.AsMap) (dig "builtins" "k8sAgent" "enabled" false .Values.AsMap) (dig "builtins" "harness" "enabled" false .Values.AsMap) .Values.substrateWorkerPool.create (index .Values "kagent-tools" "enabled") $skill }} +{{- $toolServer := printf "%s-tool-server" (include "kagent.fullname" .) }} +apiVersion: api.kagent.dev/v1alpha3 +kind: Agent +metadata: + name: {{ .Values.builtins.k8sAgent.name | quote }} + namespace: {{ include "kagent.namespace" . }} + labels: + {{- include "kagent.labels" . | nindent 4 }} +spec: + harnessRef: + name: {{ .Values.builtins.harness.name | quote }} + template: + inline: + description: "Kubernetes assistant: finds and fixes problems with workloads in the cluster where kagent runs." + modelConfig: + name: {{ include "kagent.defaultModelConfigName" . }} + promptTemplate: + dataSources: + - name: kagent-builtin-prompts + alias: builtin + systemPrompt: | + You help users run and troubleshoot workloads in the Kubernetes cluster + where kagent runs. When a question matches one of your skills, load that + skill and follow it. + + {{`{{include "builtin/skills-usage"}}`}} + + {{`{{include "builtin/kubernetes-context"}}`}} + + {{`{{include "builtin/tool-usage-best-practices"}}`}} + + {{`{{include "builtin/safety-guardrails"}}`}} + tools: + - mcp: + server: + kind: RemoteMCPServer + name: {{ $toolServer }} + tools: + - k8s_describe_resource + - k8s_generate_resource + - k8s_get_available_api_resources + - k8s_get_cluster_configuration + - k8s_get_events + - k8s_get_pod_logs + - k8s_get_resource_yaml + - k8s_get_resources + - k8s_wait + # These tools change the cluster, so each call waits for the user's approval. + - mcp: + server: + kind: RemoteMCPServer + name: {{ $toolServer }} + requireApproval: true + tools: + - k8s_annotate_resource + - k8s_apply_manifest + - k8s_check_service_connectivity + - k8s_create_resource + - k8s_create_resource_from_url + - k8s_delete_resource + - k8s_execute_command + - k8s_label_resource + - k8s_patch_resource + - k8s_patch_status + - k8s_remove_annotation + - k8s_remove_label + - k8s_rollout + - k8s_scale + skills: + - name: {{ $skill.name | quote }} + source: + oci: {{ $catalog.image | quote }} + path: {{ $skill.path | quote }} +{{- end }} diff --git a/helm/kagent/templates/ui-configmap.yaml b/helm/kagent/templates/ui-configmap.yaml index 52b61880bd..acd7cb725a 100644 --- a/helm/kagent/templates/ui-configmap.yaml +++ b/helm/kagent/templates/ui-configmap.yaml @@ -19,3 +19,6 @@ data: {{- if .Values.ui.streamTimeoutSeconds }} KAGENT_UI_STREAM_TIMEOUT_MS: {{ mul (int .Values.ui.streamTimeoutSeconds) 1000 | quote }} {{- end }} + {{- with include "kagent.builtinSkillsCatalog" . }} + KAGENT_UI_BUILTIN_SKILLS: {{ . | quote }} + {{- end }} diff --git a/helm/kagent/tests/default-harness_test.yaml b/helm/kagent/tests/default-harness_test.yaml new file mode 100644 index 0000000000..30a2e134b7 --- /dev/null +++ b/helm/kagent/tests/default-harness_test.yaml @@ -0,0 +1,78 @@ +suite: test the default Harness +templates: + - default-harness.yaml +release: + namespace: kagent +set: + substrateWorkerPool.create: true + substrateWorkerPool.workerImage: ghcr.io/kagent-dev/substrate/ateworker-gvisor:v1 +tests: + - it: creates a Go ADK Harness on the chart's worker pool + asserts: + - isKind: + of: Harness + - equal: + path: metadata.name + value: kagent + - equal: + path: metadata.namespace + value: kagent + - exists: + path: spec.kagent + - equal: + path: spec.substrate.workerPoolRef.name + value: kagent-default + - equal: + path: spec.substrate.snapshotPolicy.location + value: s3://ate-snapshots/kagent/ + + - it: uses the configured name, pool and snapshot location + set: + builtins.harness.name: team + builtins.harness.snapshotLocation: gs://bucket/team/ + substrateWorkerPool.name: other-pool + asserts: + - equal: + path: metadata.name + value: team + - equal: + path: spec.substrate.workerPoolRef.name + value: other-pool + - equal: + path: spec.substrate.snapshotPolicy.location + value: gs://bucket/team/ + + - it: renders nothing when turned off + set: + builtins.harness.enabled: false + asserts: + - hasDocuments: + count: 0 + + - it: renders nothing when all built-ins are turned off + set: + builtins.enabled: false + asserts: + - hasDocuments: + count: 0 + + - it: renders nothing when the built-ins values are missing + set: + builtins: null + asserts: + - hasDocuments: + count: 0 + + - it: requires a snapshot location + set: + builtins.harness.snapshotLocation: "" + asserts: + - failedTemplate: + errorMessage: "builtins.harness.snapshotLocation is required: set the snapshot storage location for the default Harness" + + - it: renders nothing without the chart's worker pool + set: + substrateWorkerPool.create: false + asserts: + - hasDocuments: + count: 0 diff --git a/helm/kagent/tests/k8s-agent_test.yaml b/helm/kagent/tests/k8s-agent_test.yaml new file mode 100644 index 0000000000..c22b89f950 --- /dev/null +++ b/helm/kagent/tests/k8s-agent_test.yaml @@ -0,0 +1,191 @@ +suite: test the k8s Agent +templates: + - k8s-agent.yaml +release: + namespace: kagent +set: + substrateWorkerPool.create: true + substrateWorkerPool.workerImage: ghcr.io/kagent-dev/substrate/ateworker-gvisor:v1 +tests: + - it: runs on the default Harness with the default ModelConfig + asserts: + - isKind: + of: Agent + - equal: + path: metadata.name + value: k8s-agent + - equal: + path: metadata.namespace + value: kagent + - equal: + path: spec.harnessRef.name + value: kagent + - equal: + path: spec.template.inline.modelConfig.name + value: default-model-config + + - it: binds only the kagent-tools Kubernetes tools, and cluster changes need approval + asserts: + - lengthEqual: + path: spec.template.inline.tools + count: 2 + - equal: + path: spec.template.inline.tools[0].mcp.server + value: + kind: RemoteMCPServer + name: RELEASE-NAME-tool-server + - notExists: + path: spec.template.inline.tools[0].mcp.requireApproval + - equal: + path: spec.template.inline.tools[0].mcp.tools + value: + - k8s_describe_resource + - k8s_generate_resource + - k8s_get_available_api_resources + - k8s_get_cluster_configuration + - k8s_get_events + - k8s_get_pod_logs + - k8s_get_resource_yaml + - k8s_get_resources + - k8s_wait + - equal: + path: spec.template.inline.tools[1].mcp.requireApproval + value: true + - equal: + path: spec.template.inline.tools[1].mcp.tools + value: + - k8s_annotate_resource + - k8s_apply_manifest + - k8s_check_service_connectivity + - k8s_create_resource + - k8s_create_resource_from_url + - k8s_delete_resource + - k8s_execute_command + - k8s_label_resource + - k8s_patch_resource + - k8s_patch_status + - k8s_remove_annotation + - k8s_remove_label + - k8s_rollout + - k8s_scale + + - it: attaches k8s-troubleshooting from the packaged kagent-tools image + asserts: + - lengthEqual: + path: spec.template.inline.skills + count: 1 + - equal: + path: spec.template.inline.skills[0].name + value: k8s-troubleshooting + - matchRegex: + path: spec.template.inline.skills[0].source.oci + pattern: ^ghcr\.io/kagent-dev/kagent/tools@sha256:[0-9a-f]{64}$ + - equal: + path: spec.template.inline.skills[0].source.path + value: opt/kagent-tools-skills/skills/k8s-troubleshooting + + - it: mirrors the packaged image through global.imageRegistry + set: + global.imageRegistry: mirror.example + asserts: + - matchRegex: + path: spec.template.inline.skills[0].source.oci + pattern: ^mirror\.example/kagent-dev/kagent/tools@sha256:[0-9a-f]{64}$ + + - it: attaches the skill from a configured catalog + set: + builtins.skills.image: registry.example/team/skills@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + builtins.skills.list: + - name: helm-release-management + description: Inspect, upgrade and uninstall Helm releases. + path: skills/helm-release-management + - name: k8s-troubleshooting + description: Diagnose unhealthy workloads. + path: catalog/k8s-troubleshooting + asserts: + - equal: + path: spec.template.inline.skills + value: + - name: k8s-troubleshooting + source: + oci: registry.example/team/skills@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + path: catalog/k8s-troubleshooting + + - it: rejects a skills image without a digest + set: + builtins.skills.image: registry.example/team/skills:v1 + asserts: + - failedTemplate: + errorMessage: 'builtins.skills.image must be pinned by sha256 digest: "registry.example/team/skills:v1"' + + - it: uses the configured Agent and Harness names + set: + builtins.k8sAgent.name: ops + builtins.harness.name: team + asserts: + - equal: + path: metadata.name + value: ops + - equal: + path: spec.harnessRef.name + value: team + + - it: is skipped when turned off + set: + builtins.k8sAgent.enabled: false + asserts: + - hasDocuments: + count: 0 + + - it: is skipped when all built-ins are turned off + set: + builtins.enabled: false + asserts: + - hasDocuments: + count: 0 + + - it: is skipped when the built-ins values are missing + set: + builtins: null + asserts: + - hasDocuments: + count: 0 + + - it: is skipped without the default Harness + set: + builtins.harness.enabled: false + asserts: + - hasDocuments: + count: 0 + + - it: is skipped without kagent-tools + set: + kagent-tools.enabled: false + asserts: + - hasDocuments: + count: 0 + + - it: is skipped without the chart's worker pool + set: + substrateWorkerPool.create: false + asserts: + - hasDocuments: + count: 0 + + - it: is skipped when built-in skills are turned off + set: + builtins.skills.enabled: false + asserts: + - hasDocuments: + count: 0 + + - it: is skipped when the catalog lacks k8s-troubleshooting + set: + builtins.skills.image: registry.example/team/skills@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + builtins.skills.list: + - name: helm-release-management + description: Inspect, upgrade and uninstall Helm releases. + path: skills/helm-release-management + asserts: + - hasDocuments: + count: 0 diff --git a/helm/kagent/tests/ui-configmap_test.yaml b/helm/kagent/tests/ui-configmap_test.yaml new file mode 100644 index 0000000000..51cf97c0e3 --- /dev/null +++ b/helm/kagent/tests/ui-configmap_test.yaml @@ -0,0 +1,78 @@ +suite: test ui configmap +templates: + - ui-configmap.yaml +tests: + - it: offers the skills of the packaged kagent-tools image + asserts: + - matchRegex: + path: data.KAGENT_UI_BUILTIN_SKILLS + pattern: ^\{"image":"ghcr\.io/kagent-dev/kagent/tools@sha256:[0-9a-f]{64}","skills":\[ + - matchRegex: + path: data.KAGENT_UI_BUILTIN_SKILLS + pattern: '"name":"k8s-troubleshooting","path":"opt/kagent-tools-skills/skills/k8s-troubleshooting"' + + - it: mirrors the packaged image through global.imageRegistry + set: + global.imageRegistry: mirror.example + asserts: + - matchRegex: + path: data.KAGENT_UI_BUILTIN_SKILLS + pattern: ^\{"image":"mirror\.example/kagent-dev/kagent/tools@sha256:[0-9a-f]{64}" + + - it: offers the skills of the configured image, not the packaged ones + set: + builtins.skills.image: registry.example/team/skills@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + builtins.skills.list: + - name: k8s-troubleshooting + description: Diagnose unhealthy workloads. + path: skills/k8s-troubleshooting + asserts: + - equal: + path: data.KAGENT_UI_BUILTIN_SKILLS + value: '{"image":"registry.example/team/skills@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","skills":[{"description":"Diagnose unhealthy workloads.","name":"k8s-troubleshooting","path":"skills/k8s-troubleshooting"}]}' + + - it: offers no skills for a configured image that lists none + set: + builtins.skills.image: registry.example/team/skills@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + asserts: + - equal: + path: data.KAGENT_UI_BUILTIN_SKILLS + value: '{"image":"registry.example/team/skills@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","skills":[]}' + + - it: offers no built-in skills when turned off + set: + builtins.skills.enabled: false + asserts: + - notExists: + path: data.KAGENT_UI_BUILTIN_SKILLS + + - it: offers no built-in skills when all built-ins are turned off + set: + builtins.enabled: false + asserts: + - notExists: + path: data.KAGENT_UI_BUILTIN_SKILLS + + - it: offers no built-in skills when the built-ins values are missing + set: + builtins: null + asserts: + - notExists: + path: data.KAGENT_UI_BUILTIN_SKILLS + + - it: rejects a skills image without a digest + set: + builtins.skills.image: registry.example/team/skills:v1 + asserts: + - failedTemplate: + errorMessage: 'builtins.skills.image must be pinned by sha256 digest: "registry.example/team/skills:v1"' + + - it: rejects a configured skill without a path + set: + builtins.skills.image: registry.example/team/skills@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + builtins.skills.list: + - name: k8s-troubleshooting + description: Diagnose unhealthy workloads. + asserts: + - failedTemplate: + errorMessage: 'builtins.skills.list: each skill needs a name and a path, got {"description":"Diagnose unhealthy workloads.","name":"k8s-troubleshooting"}' diff --git a/helm/kagent/values.yaml b/helm/kagent/values.yaml index 69c5b77417..ebcba92f48 100644 --- a/helm/kagent/values.yaml +++ b/helm/kagent/values.yaml @@ -23,8 +23,9 @@ global: # air-gap mirror knob: one value redirects all images. Repository and tag stay # per-image. For per-image control, leave this unset and set the per-image # registry keys. Covers this chart's own images: controller, sandbox guest, ui, - # substrateWorkerPool.workerImage, and built-in Harness images - # (both packaged defaults and config.harnessImages overrides). The vendored subcharts + # substrateWorkerPool.workerImage, built-in Harness images + # (both packaged defaults and config.harnessImages overrides), and the built-in + # skills image (builtins.skills). The vendored subcharts # kagent-tools adopts it when its pinned version bumps. imageRegistry: "" # -- Pull secrets for every pod. The chart merges this list (union) into each @@ -772,6 +773,53 @@ kagent-tools: metrics: port: 8085 # Use a different port than the controller service port to avoid duplicate port definitions +# ============================================================================== +# BUILT-INS +# ============================================================================== +# A starting point for a new install: a Harness to run Agents on, a ready-made +# Kubernetes Agent to chat with, and skills you can add to your own Agents. +# The Harness and the Agent run on the chart's worker pool, so they need +# substrateWorkerPool.create (kagent install sets it). + +builtins: + # -- Set to false to skip every built-in below, for example when you manage + # your own Harnesses and Agents. + enabled: true + harness: + # -- Create a Harness, the runtime your Agents run on. Agents name it in + # spec.harnessRef. + enabled: true + # -- Name of the Harness. + name: kagent + # -- Where the Harness stores Agent snapshots. The default bucket comes + # with the Substrate chart's built-in object store. If you turned that off, + # set your own bucket. + snapshotLocation: s3://ate-snapshots/kagent/ + k8sAgent: + # -- Create k8s-agent, an Agent that helps you troubleshoot workloads in + # this cluster. It runs on builtins.harness, uses the kagent-tools + # Kubernetes tools and the k8s-troubleshooting skill, and asks for your + # approval before it changes anything. Needs kagent-tools.enabled. + enabled: true + # -- Name of the Agent. + name: k8s-agent + skills: + # -- Offer the built-in skills. A skill is a set of instructions an Agent + # loads when a task needs it. The UI lists these skills when you build an + # Agent, and k8s-agent uses one of them. + enabled: true + # -- Leave empty to use the skills that ship in the kagent-tools image. + # To offer skills from your own image instead, set its digest-pinned + # reference here, for example + # registry.example.com/team/skills@sha256:, and list the skills in + # builtins.skills.list. + image: "" + # -- The skills in builtins.skills.image, used only when that is set. Each + # entry has a name, a one-line description and the skill's directory in + # the image, for example: + # {name: k8s-troubleshooting, description: Diagnose unhealthy workloads., path: skills/k8s-troubleshooting} + list: [] + # ============================================================================== # PROXY CONFIGURATION # ============================================================================== diff --git a/scripts/builtin-skills-catalog.sh b/scripts/builtin-skills-catalog.sh new file mode 100644 index 0000000000..057e59325f --- /dev/null +++ b/scripts/builtin-skills-catalog.sh @@ -0,0 +1,55 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Prints the built-in skills catalog: the kagent-tools image that the chart +# deploys, pinned by digest, and the name, description and path of every skill +# in it. Rerun with make builtin-skills-catalog after you bump kagent-tools in +# helm/kagent/Chart-template.yaml. +root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +repository="ghcr.io/kagent-dev/kagent/tools" +skills_root="opt/kagent-tools-skills/skills" +# The kagent-tools skills the UI offers. The others need tools or services +# (Argo Rollouts, Cilium, Kubescape, Prometheus) that most clusters lack. +skills=(helm-release-management istio-traffic-debugging k8s-troubleshooting) + +version="$(awk '/- name: kagent-tools$/ { found = 1; next } found && /version:/ { print $2; exit }' \ + "$root/helm/kagent/Chart-template.yaml")" +[[ -n "$version" ]] || { echo "kagent-tools version not found in helm/kagent/Chart-template.yaml" >&2; exit 1; } + +digest="$(docker buildx imagetools inspect "$repository:$version" --format '{{json .Manifest}}' | jq -r .digest)" +[[ "$digest" =~ ^sha256:[0-9a-f]{64}$ ]] || { echo "invalid digest for $repository:$version: $digest" >&2; exit 1; } +image="$repository@$digest" + +work="$(mktemp -d)" +container="" +cleanup() { + [[ -z "$container" ]] || docker rm "$container" > /dev/null + rm -rf "$work" +} +trap cleanup EXIT +container="$(docker create "$image" true)" +docker cp "$container:/$skills_root" - | tar -x -C "$work" + +for dir in "${skills[@]}"; do + file="$work/skills/$dir/SKILL.md" + [[ -f "$file" ]] || { echo "$image has no $skills_root/$dir/SKILL.md" >&2; exit 1; } + awk -v dir="$dir" -v file="$dir/SKILL.md" ' + function fail(message) { printf "%s: %s\n", file, message > "/dev/stderr"; failed = 1; exit 1 } + function plain(field, value) { + if (value ~ /^["\047]/ || value ~ /^[|>]/) { + fail(field " must be one line of unquoted text, not a quoted or block value: " value) + } + return value + } + NR == 1 && $0 != "---" { exit 1 } + NR > 1 && $0 == "---" { closed = 1; exit } + NR > 1 && /^name:/ { name = plain("name", substr($0, 7)) } + NR > 1 && /^description:/ { description = plain("description", substr($0, 14)) } + END { + if (failed) exit 1 + if (!closed || name == "" || description == "") fail("front matter needs a name and a description") + if (name != dir) fail("name " name " must match its directory " dir) + print name; print description + }' "$file" | jq -Rn --arg path "$skills_root/$dir" '[inputs] | {name: .[0], description: .[1], path: $path}' +done | jq -es --arg image "$image" ' + if length == 0 then error("no skills found") else {image: $image, skills: sort_by(.name)} end' diff --git a/ui/entrypoint/main.go b/ui/entrypoint/main.go index 9ac07b3834..8af5057eab 100644 --- a/ui/entrypoint/main.go +++ b/ui/entrypoint/main.go @@ -79,6 +79,9 @@ func render(environ []string) (string, string) { fmt.Fprintf(&b, " \"API_BASE_URL\": \"%s\",\n", jsonEscape(get("KAGENT_UI_API_BASE_URL", "/api"))) fmt.Fprintf(&b, " \"SSO_REDIRECT_PATH\": \"%s\",\n", jsonEscape(get("KAGENT_UI_SSO_REDIRECT_PATH", "/oauth2/start"))) fmt.Fprintf(&b, " \"STREAM_TIMEOUT_MS\": \"%s\",\n", timeout) + if skills := env["KAGENT_UI_BUILTIN_SKILLS"]; skills != "" { + fmt.Fprintf(&b, " \"BUILTIN_SKILLS\": \"%s\",\n", jsonEscape(skills)) + } for _, kv := range extensions { name, value, _ := strings.Cut(kv, "=") fmt.Fprintf(&b, " \"%s\": \"%s\",\n", jsonEscape(name), jsonEscape(value)) diff --git a/ui/entrypoint/main_test.go b/ui/entrypoint/main_test.go index 9416f07259..1d8dcc93f4 100644 --- a/ui/entrypoint/main_test.go +++ b/ui/entrypoint/main_test.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "encoding/json" "os" "os/exec" "path/filepath" @@ -97,3 +98,27 @@ func TestMatchesInitScript(t *testing.T) { }) } } + +func TestBuiltinSkills(t *testing.T) { + settings := func(environ ...string) map[string]string { + t.Helper() + config, _ := render(environ) + body := strings.TrimSuffix(strings.TrimPrefix(config, "window.environmentVariables = "), ";\n") + var got map[string]string + if err := json.Unmarshal([]byte(body), &got); err != nil { + t.Fatalf("env-config.js is not a JSON object: %v\n%s", err, config) + } + if strings.Contains(config, " element:\n%s", config) + } + return got + } + + catalog := `{"image":"ghcr.io/kagent-dev/kagent/tools@sha256:aaaa","skills":[{"name":"a","description":"Says \"hi\" \\ then "}]}` + if got := settings("KAGENT_UI_BUILTIN_SKILLS=" + catalog)["BUILTIN_SKILLS"]; got != catalog { + t.Errorf("BUILTIN_SKILLS = %q, want %q", got, catalog) + } + if got, ok := settings()["BUILTIN_SKILLS"]; ok { + t.Errorf("BUILTIN_SKILLS = %q without KAGENT_UI_BUILTIN_SKILLS, want it absent", got) + } +} diff --git a/ui/playwright/live/agent-template-skills.spec.ts b/ui/playwright/live/agent-template-skills.spec.ts new file mode 100644 index 0000000000..8a46af547d --- /dev/null +++ b/ui/playwright/live/agent-template-skills.spec.ts @@ -0,0 +1,259 @@ +import { test, expect, type Page } from "@playwright/test"; +import { createClient } from "@connectrpc/connect"; +import { createGrpcWebTransport } from "@connectrpc/connect-web"; +import { AgentTemplateService } from "../../src/generated/kagent/api/v1/agent_templates_pb"; +import { expectNoLoadFailure, liveRoutes, loadLive, throwawayName } from "./helpers/live"; +import { liveApi } from "./helpers/api"; +import { pressOnce } from "../helpers/resource"; + +/** + * Skills in the agent template form, against a real controller. + * + * Needs the chart's built-in skills catalog in `KAGENT_UI_BUILTIN_SKILLS` (the value the + * chart writes to the UI ConfigMap), the `kagent` Harness and `default-model-config` in + * `kagent`. The chat test also needs a working model behind `default-model-config`. + */ + +const NAMESPACE = "kagent"; +const HARNESS = "kagent"; +const MODEL = `${NAMESPACE}/default-model-config`; +const BUILTIN = "k8s-troubleshooting"; +const READY_TIMEOUT = 180_000; + +// A real public repository and commit. A template does not fetch its skills, so these +// only have to be well formed; an Agent would need them to exist. +const GIT_URL = "https://github.com/kagent-dev/kagent.git"; +const GIT_COMMIT = "8cc31cfe7b9ea951bf3d21478e5a9b00f55025f6"; +const S3 = { + endpoint: "https://s3.amazonaws.com", + bucket: "e2e-skills", + key: "skills.tar.gz", + versionId: "3HL4kqtJlcpXroDTDmJ.rmSpXd3dIbrHY", +}; + +const RAW_CATALOG = process.env.KAGENT_UI_BUILTIN_SKILLS ?? ""; +const catalog = RAW_CATALOG + ? (JSON.parse(RAW_CATALOG) as { image: string; skills: { name: string; path: string }[] }) + : undefined; + +const templates = new Set(); +const agents = new Set(); + +test.skip(!catalog, "KAGENT_UI_BUILTIN_SKILLS is not set to the chart's skills catalog"); + +// Through the API, not the UI: teardown runs after failures, when the page may be anywhere. +test.afterAll(async ({ baseURL }) => { + const api = liveApi(baseURL!); + for (const name of agents) await api.removeAgent(NAMESPACE, name); + const client = templateClient(baseURL!); + for (const name of templates) { + await client.deleteAgentTemplate({ ref: { namespace: NAMESPACE, name } }).catch(() => {}); + } +}); + +function templateClient(baseURL: string) { + return createClient(AgentTemplateService, createGrpcWebTransport({ baseUrl: `${baseURL}/api` })); +} + +/** The template's spec as the controller stores it. */ +async function storedSpec(baseURL: string, name: string) { + const { agentTemplate } = await templateClient(baseURL).getAgentTemplate({ ref: { namespace: NAMESPACE, name } }); + return (agentTemplate?.resource?.value as { spec?: { description?: string; skills?: unknown[] } } | undefined)?.spec; +} + +/** + * Picks an option from the dropdown just opened. Each skill row's kind select has its own + * dropdown with the same titles, and the last one can still be fading out; antd appends a + * dropdown to the body the first time it opens, so the newest open one is the last. + */ +async function choose(page: Page, testId: string, title: string) { + await page.getByTestId(testId).click(); + const open = page.locator(".ant-select-dropdown:not(.ant-select-dropdown-hidden)").last(); + await pressOnce(open.locator(`.ant-select-item-option[title="${title}"]`)); +} + +async function addBuiltinSkill(page: Page, name: string) { + await choose(page, "template-form-add-builtin-skill", name); +} + +async function addOwnSkill(page: Page, index: number, name: string, kind: "OCI image" | "Git" | "S3") { + await page.getByTestId("template-form-add-skill").click(); + await page.getByTestId(`template-form-skill-name-${index}`).fill(name); + if (kind !== "OCI image") await choose(page, `template-form-skill-kind-${index}`, kind); +} + +test("live: agent template skills are saved, read back and kept through an edit", async ({ page, baseURL }) => { + test.setTimeout(240_000); + const name = throwawayName("skills"); + const helmPath = catalog!.skills.find((skill) => skill.name === "helm-release-management")!.path; + const builtinPath = catalog!.skills.find((skill) => skill.name === BUILTIN)!.path; + const expectedSkills = [ + { name: BUILTIN, source: { oci: catalog!.image, path: builtinPath } }, + { name: "own-helm", source: { oci: catalog!.image, path: helmPath } }, + { name: "own-git", source: { git: { url: GIT_URL, commit: GIT_COMMIT }, path: "skills/k8s-troubleshooting" } }, + { name: "own-s3", source: { bucket: { s3: S3 }, path: "skills/incident-review" } }, + ]; + + await test.step("1. the page carries the chart's catalog", async () => { + await loadLive(page, "/agent-templates/new"); + await expectNoLoadFailure(page); + const served = await page.evaluate( + () => (window as { environmentVariables?: Record }).environmentVariables?.BUILTIN_SKILLS, + ); + expect(served).toBe(RAW_CATALOG); + }); + + await test.step("2. create a template with a kagent skill and an OCI, a Git and an S3 skill", async () => { + await page.getByTestId("template-form-name").fill(name); + await choose(page, "template-form-model", MODEL); + await page.getByTestId("template-form-description").fill(`Live ${name}`); + await page.getByTestId("template-form-prompt").fill("You are a concise assistant."); + + await addBuiltinSkill(page, BUILTIN); + await expect(page.getByTestId("template-form-skill-0")).toContainText("kagent"); + await expect(page.getByTestId("template-form-skill-0")).toContainText(BUILTIN); + + await addOwnSkill(page, 1, "own-helm", "OCI image"); + await page.getByTestId("template-form-skill-oci-1").fill(catalog!.image); + await page.getByTestId("template-form-skill-path-1").fill(helmPath); + + await addOwnSkill(page, 2, "own-git", "Git"); + await page.getByTestId("template-form-skill-git-url-2").fill(GIT_URL); + await page.getByTestId("template-form-skill-git-commit-2").fill(GIT_COMMIT); + await page.getByTestId("template-form-skill-path-2").fill("skills/k8s-troubleshooting"); + + await addOwnSkill(page, 3, "own-s3", "S3"); + await page.getByTestId("template-form-skill-s3-endpoint-3").fill(S3.endpoint); + await page.getByTestId("template-form-skill-s3-bucket-3").fill(S3.bucket); + await page.getByTestId("template-form-skill-s3-key-3").fill(S3.key); + await page.getByTestId("template-form-skill-s3-version-3").fill(S3.versionId); + await page.getByTestId("template-form-skill-path-3").fill("skills/incident-review"); + + await expect(page.getByTestId("template-form-problems")).toHaveCount(0); + templates.add(name); + await page.getByTestId("template-submit").click(); + await expect(page).toHaveURL(/\/agents\?tab=templates/, { timeout: 60_000 }); + }); + + await test.step("3. the controller stored exactly those four skills", async () => { + expect((await storedSpec(baseURL!, name))?.skills).toEqual(expectedSkills); + }); + + await test.step("4. reading it back shows the kagent skill as one and the others' sources", async () => { + await loadLive(page, `/agent-templates/${NAMESPACE}/${name}`); + await expectNoLoadFailure(page); + await expect(page.getByTestId("template-edit")).toBeVisible({ timeout: 60_000 }); + await expect(page.getByTestId("template-form-add-skill")).toHaveCount(0); + + const builtin = page.getByTestId("template-form-skill-0"); + await expect(builtin.locator(".ant-tag")).toHaveText("kagent"); + await expect(builtin).toContainText(BUILTIN); + + await expect(page.getByTestId("template-form-skill-name-1")).toHaveValue("own-helm"); + await expect(page.getByTestId("template-form-skill-oci-1")).toHaveValue(catalog!.image); + await expect(page.getByTestId("template-form-skill-path-1")).toHaveValue(helmPath); + + await expect(page.getByTestId("template-form-skill-name-2")).toHaveValue("own-git"); + await expect(page.getByTestId("template-form-skill-kind-2")).toContainText("Git"); + await expect(page.getByTestId("template-form-skill-git-url-2")).toHaveValue(GIT_URL); + await expect(page.getByTestId("template-form-skill-git-commit-2")).toHaveValue(GIT_COMMIT); + + await expect(page.getByTestId("template-form-skill-name-3")).toHaveValue("own-s3"); + await expect(page.getByTestId("template-form-skill-kind-3")).toContainText("S3"); + await expect(page.getByTestId("template-form-skill-s3-endpoint-3")).toHaveValue(S3.endpoint); + await expect(page.getByTestId("template-form-skill-s3-bucket-3")).toHaveValue(S3.bucket); + await expect(page.getByTestId("template-form-skill-s3-key-3")).toHaveValue(S3.key); + await expect(page.getByTestId("template-form-skill-s3-version-3")).toHaveValue(S3.versionId); + }); + + await test.step("5. an edit to the description keeps the skills", async () => { + await page.getByTestId("template-edit").click(); + await page.getByTestId("template-form-description").fill(`Edited ${name}`); + await page.getByTestId("template-submit").click(); + await expect(page.getByTestId("template-edit")).toBeVisible({ timeout: 60_000 }); + const spec = await storedSpec(baseURL!, name); + expect(spec?.description).toBe(`Edited ${name}`); + expect(spec?.skills).toEqual(expectedSkills); + }); + + await test.step("6. a short Git commit and a name with a slash are refused before saving", async () => { + await loadLive(page, "/agent-templates/new"); + await page.getByTestId("template-form-name").fill(throwawayName("refused")); + await choose(page, "template-form-model", MODEL); + await page.getByTestId("template-form-prompt").fill("You are a concise assistant."); + await addOwnSkill(page, 0, "team/runbooks", "Git"); + await page.getByTestId("template-form-skill-git-url-0").fill(GIT_URL); + await page.getByTestId("template-form-skill-git-commit-0").fill("abc"); + + const problems = page.getByTestId("template-form-problems"); + await expect(problems).toContainText('The skill "team/runbooks" needs a name that is one path segment'); + await expect(problems).toContainText('The skill "team/runbooks" needs a full Git commit ID: 40 or 64 hex characters.'); + await expect(page.getByTestId("template-submit")).toBeDisabled(); + }); +}); + +test("live: an agent built with a kagent skill loads it in chat", async ({ page }, testInfo) => { + test.setTimeout(READY_TIMEOUT + 300_000); + const name = throwawayName("skill-chat"); + + await test.step("1. create an Agent whose inline template has only the kagent skill", async () => { + await loadLive(page, liveRoutes.agentNew); + await expectNoLoadFailure(page); + await page.getByTestId("agent-form-name").fill(name); + await page.getByTestId("agent-form-template-source").getByText("Inline", { exact: true }).click(); + await choose(page, "template-form-model", MODEL); + await page.getByTestId("template-form-description").fill(`Live ${name}`); + await page + .getByTestId("template-form-prompt") + .fill( + "You are a Kubernetes assistant. Before you answer, find the skill whose description matches the task and load it with your skill tool. When you cite a skill, put its text in quotes.", + ); + await addBuiltinSkill(page, BUILTIN); + await expect(page.getByTestId("template-form-skill-0")).toContainText(BUILTIN); + await choose(page, "agent-form-harness-ref", HARNESS); + agents.add(name); + await page.getByTestId("agent-form-submit").click(); + await expect(page).toHaveURL(/\/agents\?tab=agents$/, { timeout: 60_000 }); + }); + + await test.step("2. the Agent becomes Ready, so the kagent-tools image was pulled", async () => { + const status = page.getByTestId(`agent-revision-${NAMESPACE}/${name}`); + await expect + .poll( + async () => { + await page.getByRole("button", { name: /Refresh/ }).click(); + return status.getAttribute("data-revision-state"); + }, + { timeout: READY_TIMEOUT, message: `${name} never became Ready` }, + ) + .toBe("ready"); + }); + + await test.step("3. asked about a crashing pod, it loads the skill", async () => { + await page.getByTestId(`agent-link-${NAMESPACE}-${name}`).click(); + await page + .getByTestId("chat-input") + .fill( + // Not "word for word": Azure OpenAI's content filter reads that as a jailbreak and refuses the request. + "A pod in namespace default is in CrashLoopBackOff. Which of your skills applies? Load it, then tell me the opening line of its instructions in quotes.", + ); + await page.getByTestId("chat-send").click(); + await expect(page).toHaveURL(/\/agents\/[0-9a-f-]{36}\/chat$/, { timeout: 60_000 }); + // The turn has started once the agent says something, and has ended once it can no longer be cancelled. + await expect(page.locator('[data-testid="chat-message"][data-role="agent"]').first()).toBeVisible({ timeout: 240_000 }); + await expect(page.getByTestId("chat-cancel")).toHaveCount(0, { timeout: 240_000 }); + + const call = page.locator('[data-testid="chat-tool-call"][data-tool-name="load_skill"]'); + await expect(call.first()).toBeVisible(); + await expect(call.first().getByTestId("chat-tool-payload")).toContainText(BUILTIN); + // The skill's own text, from opt/kagent-tools-skills/skills/k8s-troubleshooting/SKILL.md in the kagent-tools image. + const loaded = page.locator('[data-testid="chat-tool-result"][data-tool-name="load_skill"]'); + await expect(loaded.first().getByTestId("chat-tool-payload")).toContainText("Work from symptoms to cause."); + // The reply's wording varies between runs (it sometimes quotes the heading), so the + // skill's text is checked in the tool result above, not in the reply. + await expect(page.locator('[data-testid="chat-message"][data-role="agent"]').last()).not.toBeEmpty(); + await page.screenshot({ path: testInfo.outputPath("chat-reply.png"), fullPage: true }); + testInfo.annotations.push({ type: "session", description: new URL(page.url()).pathname.split("/")[2] }); + }); +}); + diff --git a/ui/playwright/tests/agent-templates/agent-templates.spec.ts b/ui/playwright/tests/agent-templates/agent-templates.spec.ts index d0ec66a587..09fd1d1908 100644 --- a/ui/playwright/tests/agent-templates/agent-templates.spec.ts +++ b/ui/playwright/tests/agent-templates/agent-templates.spec.ts @@ -4,12 +4,15 @@ import { LIFECYCLE_TIMEOUT, confirmation, expectRequired, + optionNamed, pressOnce, selectOption, } from "../../helpers/resource"; /** A reusable template can be authored without choosing an Agent or Harness. */ const CREATED = "browser-made"; +const GIT_URL = "https://github.com/example/runbooks.git"; +const GIT_COMMIT = "0123456789abcdef0123456789abcdef01234567"; /* * A lifecycle is longer than a journey, so it gets its own budget — see @@ -87,6 +90,34 @@ test("agent templates: a template is created, read, edited and deleted", async ( await page.getByTestId("template-form-mcp-remove-0").click(); }); + await test.step("a kagent skill and the reader's own Git skill can be added", async () => { + const builtin = page.getByTestId("template-form-add-builtin-skill"); + await builtin.click(); + await pressOnce(optionNamed(page, "k8s-troubleshooting")); + const added = page.getByTestId("template-form-skill-0"); + await expect(added).toContainText("k8s-troubleshooting"); + await expect(added).toContainText("Diagnose unhealthy Kubernetes workloads"); + + // Offered once: a second copy would be a duplicate name the CRD refuses. + await builtin.click(); + await expect(optionNamed(page, "k8s-troubleshooting")).toHaveClass( + /ant-select-item-option-disabled/, + ); + await page.keyboard.press("Escape"); + + await page.getByTestId("template-form-add-skill").click(); + await page.getByTestId("template-form-skill-name-1").fill("incident-review"); + await selectOption(page, "template-form-skill-kind-1", "Git"); + await page.getByTestId("template-form-skill-git-url-1").fill(GIT_URL); + await page.getByTestId("template-form-skill-git-commit-1").fill("main"); + await expect(page.getByTestId("template-form-problems")).toContainText( + 'The skill "incident-review" needs a full Git commit ID', + ); + await page.getByTestId("template-form-skill-git-commit-1").fill(GIT_COMMIT); + await page.getByTestId("template-form-skill-path-1").fill("skills/incident-review"); + await expect(page.getByTestId("template-form-problems")).not.toContainText("skill"); + }); + await test.step("a namespace can select a visible model from another namespace", async () => { await selectOption(page, "template-form-namespace", "default"); await selectOption(page, "template-form-model", "kagent/default-model-config"); @@ -153,6 +184,22 @@ test("agent templates: a template is created, read, edited and deleted", async ( await expect(page.getByTestId("template-form-name")).toHaveCount(0); // And nothing that authors: the add buttons only exist when something can be added. await expect(page.getByTestId("template-form-add-label")).toHaveCount(0); + await expect(page.getByTestId("template-form-add-skill")).toHaveCount(0); + + // The skills as the backend stored them: the kagent skill is still recognized as + // one, so it was saved as the kagent-tools image and its directory. + await expect(page.getByTestId("template-form-skill-0")).toContainText("kagent"); + await expect(page.getByTestId("template-form-skill-0")).toContainText("k8s-troubleshooting"); + await expect(page.getByTestId("template-form-skill-name-1")).toHaveValue("incident-review"); + await expect(page.getByTestId("template-form-skill-kind-1")).toContainText("Git"); + await expect(page.getByTestId("template-form-skill-git-url-1")).toHaveValue(GIT_URL); + await expect(page.getByTestId("template-form-skill-git-commit-1")).toHaveValue(GIT_COMMIT); + await expect(page.getByTestId("template-form-skill-path-1")).toHaveValue("skills/incident-review"); + // Each read-only source box shows its whole value, not the start of it. + for (const testId of ["git-url", "git-commit", "path"]) { + const box = page.getByTestId(`template-form-skill-${testId}-1`); + expect(await box.evaluate((el) => el.scrollWidth <= el.clientWidth), testId).toBe(true); + } // Nor does it mark anything required. Read-only is not a form, so it asks for // nothing — an asterisk here would be demanding a reader supply something they are @@ -218,6 +265,8 @@ test("agent templates: a template is created, read, edited and deleted", async ( await expect(page.getByTestId("template-form-description")).toHaveValue( "Edited by the suite.", ); + await expect(page.getByTestId("template-form-skill-0")).toContainText("k8s-troubleshooting"); + await expect(page.getByTestId("template-form-skill-name-1")).toHaveValue("incident-review"); }); await test.step("11. deleting says what it costs, in the confirmation", async () => { @@ -276,9 +325,29 @@ test("agent templates: a template is created, read, edited and deleted", async ( "Answers questions about workloads in the cluster.", ); - // `k8s-agent-7f3a91c` carries a `skills` entry, which this form does not author. A - // save built from the fields it shows would remove it, and the API would accept that - // without a word — so the notice standing here is what tells a reader it survives. + await expect(page.getByTestId("template-form-skill-0")).toContainText("k8s-troubleshooting"); + await expect(page.getByTestId("template-form-skill-git-url-1")).toHaveValue( + "https://github.com/example/runbooks.git", + ); + + // Saved under an earlier kagent-tools image digest, as after a chart upgrade: still a + // kagent skill, marked as older. Reading offers no Update; editing does, and the + // saved row then pins the current image. + const older = page.getByTestId("template-form-skill-2"); + await expect(older).toContainText("helm-release-management"); + await expect(page.getByTestId("template-form-skill-outdated-2")).toBeVisible(); + await expect(page.getByTestId("template-form-skill-update-2")).toHaveCount(0); + await page.getByTestId("template-edit").click(); + await page.getByTestId("template-form-skill-update-2").click(); + await expect(page.getByTestId("template-form-skill-outdated-2")).toHaveCount(0); + await page.getByTestId("template-submit").click(); + await expect(page.getByTestId("template-edit")).toBeVisible({ timeout: 30_000 }); + await expect(older).toContainText("helm-release-management"); + await expect(page.getByTestId("template-form-skill-outdated-2")).toHaveCount(0); + + // `k8s-agent-7f3a91c` carries prompt data sources, which this form does not author. A + // save built from the fields it shows would remove them, and the API would accept that + // without a word — so the notice standing here is what tells a reader they survive. // Asserted on this template rather than on the one the journey made, because the // form cannot create a template that has anything to lose. await expect(page.getByTestId("template-form-unshown")).toContainText( diff --git a/ui/src/api/domain/agentTemplates.ts b/ui/src/api/domain/agentTemplates.ts index 29257121fc..f0be9c862a 100644 --- a/ui/src/api/domain/agentTemplates.ts +++ b/ui/src/api/domain/agentTemplates.ts @@ -54,8 +54,7 @@ export interface ToolBinding { /** * One immutable artifact, as `skills` and `plugins` reference content. * - * Exactly one of `oci`, `git` or `bucket`. Left as-is by this build's form, which - * does not author skills or plugins — see `AgentTemplateSpec.skills`. + * Exactly one of `oci`, `git` or `bucket`, plus an optional relative `path` inside it. */ export interface ArtifactSource { oci?: string; diff --git a/ui/src/api/runtimeConfig.test.ts b/ui/src/api/runtimeConfig.test.ts new file mode 100644 index 0000000000..a27266b3cb --- /dev/null +++ b/ui/src/api/runtimeConfig.test.ts @@ -0,0 +1,46 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { runtimeConfig } from "./runtimeConfig"; + +const IMAGE = "ghcr.io/kagent-dev/kagent/tools@sha256:" + "a".repeat(64); + +afterEach(() => { + delete window.environmentVariables; + vi.restoreAllMocks(); +}); + +describe("runtimeConfig builtinSkills", () => { + it("reads the catalog the chart passes", () => { + window.environmentVariables = { + BUILTIN_SKILLS: JSON.stringify({ + image: IMAGE, + skills: [{ name: "k8s-troubleshooting", description: "Diagnose unhealthy workloads.", path: "opt/kagent-tools-skills/skills/k8s-troubleshooting" }], + }), + }; + expect(runtimeConfig().builtinSkills).toEqual({ + image: IMAGE, + skills: [{ name: "k8s-troubleshooting", description: "Diagnose unhealthy workloads.", path: "opt/kagent-tools-skills/skills/k8s-troubleshooting" }], + }); + }); + + it("offers nothing when the chart passes no catalog", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + expect(runtimeConfig().builtinSkills).toBeUndefined(); + window.environmentVariables = { BUILTIN_SKILLS: "" }; + expect(runtimeConfig().builtinSkills).toBeUndefined(); + expect(warn).not.toHaveBeenCalled(); + }); + + it.each([ + ["not JSON", "{"], + ["not an object", "[]"], + ["no image", JSON.stringify({ skills: [] })], + ["no skills list", JSON.stringify({ image: IMAGE })], + ["a skill without a name", JSON.stringify({ image: IMAGE, skills: [{ description: "x", path: "opt/kagent-tools-skills/skills/x" }] })], + ["a skill without a path", JSON.stringify({ image: IMAGE, skills: [{ name: "x", description: "x" }] })], + ])("warns and offers nothing for %s", (_, value) => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + window.environmentVariables = { BUILTIN_SKILLS: value }; + expect(runtimeConfig().builtinSkills).toBeUndefined(); + expect(warn).toHaveBeenCalledOnce(); + }); +}); diff --git a/ui/src/api/runtimeConfig.ts b/ui/src/api/runtimeConfig.ts index e6d17462f3..79d0581b52 100644 --- a/ui/src/api/runtimeConfig.ts +++ b/ui/src/api/runtimeConfig.ts @@ -12,17 +12,33 @@ import { ENV_DEFAULTS, env, withBasePath } from "@/env"; +export interface BuiltinSkill { + name: string; + description: string; + /** The skill's directory inside the catalog image. */ + path: string; +} + +/** The skills kagent ships, all in one digest-pinned image. */ +export interface BuiltinSkillsCatalog { + image: string; + skills: BuiltinSkill[]; +} + export interface RuntimeConfig { /** Where "Sign in with SSO" sends the browser. */ ssoRedirectPath: string; /** Chat stream inactivity timeout before the client aborts. */ streamTimeoutMs: number; + /** Absent when the chart turns built-in skills off, so the UI offers none. */ + builtinSkills: BuiltinSkillsCatalog | undefined; } /** What the app runs on when nothing is configured. */ export const RUNTIME_CONFIG_DEFAULTS: RuntimeConfig = { ssoRedirectPath: ENV_DEFAULTS.SSO_REDIRECT_PATH, streamTimeoutMs: Number(ENV_DEFAULTS.STREAM_TIMEOUT_MS), + builtinSkills: undefined, }; /** @@ -36,6 +52,7 @@ export function runtimeConfig(): RuntimeConfig { return { ssoRedirectPath: withBasePath(env("SSO_REDIRECT_PATH")), streamTimeoutMs: streamTimeoutMs(), + builtinSkills: builtinSkills(), }; } @@ -52,3 +69,40 @@ function streamTimeoutMs(): number { ? parsed : RUNTIME_CONFIG_DEFAULTS.streamTimeoutMs; } + +function builtinSkills(): BuiltinSkillsCatalog | undefined { + const text = env("BUILTIN_SKILLS"); + if (text === "") return undefined; + const catalog = parseBuiltinSkills(text); + if (!catalog) console.warn("BUILTIN_SKILLS is not a valid skills catalog, so no built-in skills are offered."); + return catalog; +} + +function parseBuiltinSkills(text: string): BuiltinSkillsCatalog | undefined { + let value: unknown; + try { + value = JSON.parse(text); + } catch { + return undefined; + } + if (!isRecord(value) || !isText(value.image)) return undefined; + if (!Array.isArray(value.skills)) return undefined; + const skills: BuiltinSkill[] = []; + for (const skill of value.skills) { + if (!isRecord(skill) || !isText(skill.name) || !isText(skill.path)) return undefined; + skills.push({ + name: skill.name, + description: typeof skill.description === "string" ? skill.description : "", + path: skill.path, + }); + } + return { image: value.image, skills }; +} + +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +function isText(value: unknown): value is string { + return typeof value === "string" && value !== ""; +} diff --git a/ui/src/components/agent-template-form/AgentTemplateForm.tsx b/ui/src/components/agent-template-form/AgentTemplateForm.tsx index 9961b9518b..1ee7e82ba1 100644 --- a/ui/src/components/agent-template-form/AgentTemplateForm.tsx +++ b/ui/src/components/agent-template-form/AgentTemplateForm.tsx @@ -22,6 +22,7 @@ import { type AgentTemplateDraft, type OutputSource, } from "./agentTemplateDraft"; +import { SkillsField } from "./SkillsField"; import { referenceLabel, type ObjectReference } from "@/api/domain/agentTemplates"; @@ -67,9 +68,7 @@ function referenceFromPicker(value: string): ObjectReference { * * ## What it does not author, and why that is safe * - * `skills`, `plugins` and `promptTemplate` are not on this form. Each is a rich - * shape of its own — three artifact-source variants with strict CEL patterns — and - * authoring them properly is its own piece of work. What matters is that an edit + * `plugins` and `promptTemplate` are not on this form. What matters is that an edit * does not *lose* them: `specFromDraft` merges onto the spec it was given rather * than building a fresh one, and `agentTemplateDraft.test.ts` pins that. * @@ -600,6 +599,8 @@ export function AgentTemplateForm({ + set("skills", next)} readOnly={readOnly} /> + {embedded ? null : ( @@ -661,14 +662,14 @@ export function AgentTemplateForm({ )} {/* Said rather than left to be assumed. A reader who knows their template has - skills and cannot see them here would reasonably conclude a save will + plugins and cannot see them here would reasonably conclude a save will drop them. */} {hasUnshownFields ? ( diff --git a/ui/src/components/agent-template-form/SkillsField.tsx b/ui/src/components/agent-template-form/SkillsField.tsx new file mode 100644 index 0000000000..d6d3554a36 --- /dev/null +++ b/ui/src/components/agent-template-form/SkillsField.tsx @@ -0,0 +1,190 @@ +import { useMemo } from "react"; +import { Button, Form, Input, Select, Space, Tag, Typography } from "antd"; +import { useTheme } from "@emotion/react"; +import { Plus, Trash } from "lucide-react"; +import { runtimeConfig } from "@/api/runtimeConfig"; +import { + builtinSkillDraft, + emptySkillDraft, + matchBuiltinSkill, + type SkillDraft, + type SkillSourceKind, +} from "./agentTemplateDraft"; + +const { Text } = Typography; + +/** + * The template's `spec.skills`: kagent's built-in skills, offered from the catalog + * the chart passes, and the reader's own skills by OCI, Git or S3 source. + */ +export function SkillsField({ + skills, + onChange, + readOnly, +}: { + skills: SkillDraft[]; + onChange: (next: SkillDraft[]) => void; + readOnly: boolean; +}) { + const theme = useTheme(); + const catalog = useMemo(() => runtimeConfig().builtinSkills, []); + + const setSkill = (index: number, patch: Partial) => { + const next = [...skills]; + next[index] = { ...next[index], ...patch }; + onChange(next); + }; + const remove = (index: number) => onChange(skills.filter((_, at) => at !== index)); + + const placeholder = (example: string) => (readOnly ? "Not set" : example); + const readOnlyInput = readOnly ? ({ readOnly: true, variant: "borderless" } as const) : {}; + const readOnlySelect = readOnly + ? ({ open: false, variant: "borderless", suffixIcon: null } as const) + : {}; + const muted = { color: theme.color.textMuted, fontSize: 12 }; + + return ( + + + {readOnly && skills.length === 0 ? No skills. : null} + + {skills.map((skill, index) => { + const removeButton = readOnly ? null : ( + + )} + {builtin.skill.description} + {removeButton} + + ); + } + + const input = (field: Exclude, testId: string, example: string, width: number) => + readOnly && skill[field] === "" ? null : ( + setSkill(index, { [field]: event.target.value })} + {...readOnlyInput} + /> + ); + return ( + + {input("name", "name", "Skill name", 180)} +
+ { + const skill = catalog.skills.find((entry) => entry.name === name); + if (skill) onChange([...skills, builtinSkillDraft(catalog, skill)]); + }} + options={catalog.skills.map((skill) => ({ + value: skill.name, + title: skill.name, + disabled: skills.some((row) => row.name.trim() === skill.name), + label: ( + + {skill.name} + {skill.description} + + ), + }))} + /> +
+ ) : null} + + + )} + + + ); +} diff --git a/ui/src/components/agent-template-form/agentTemplateDraft.test.ts b/ui/src/components/agent-template-form/agentTemplateDraft.test.ts index 3b23406ae2..5055f1aca7 100644 --- a/ui/src/components/agent-template-form/agentTemplateDraft.test.ts +++ b/ui/src/components/agent-template-form/agentTemplateDraft.test.ts @@ -1,20 +1,24 @@ import { describe, expect, it } from "vitest"; import { + builtinSkillDraft, draftFromTemplate, draftFromSpec, draftProblems, emptyDraft, + emptySkillDraft, labelsFromDraft, + matchBuiltinSkill, specFromDraft, + type AgentTemplateDraft, } from "./agentTemplateDraft"; import type { AgentTemplate } from "@/api/domain/agentTemplates"; /** * The agent-template draft, and the one property that is invisible on screen. * - * An edit form still omits three rich `AgentTemplateSpec` fields. Building the update - * out of only what it shows deletes those fields — `skills`, `plugins`, - * `promptTemplate` — and the API accepts that happily. Nothing on the page changes, + * An edit form still omits two `AgentTemplateSpec` fields. Building the update out of + * only what it shows deletes those fields — `plugins`, `promptTemplate` — and the API + * accepts that happily. Nothing on the page changes, * no error is raised, and the loss only appears on the cluster. * * This repository has made that mistake once already, on the agent form. So the @@ -76,11 +80,7 @@ describe("the agent template draft", () => { // A save with nothing edited at all. const spec = specFromDraft(draft, template.resource.spec); - // The three the form never displays, still there. - expect(spec.skills, "an edit must not delete skills it never showed").toEqual( - template.resource.spec.skills, - ); - expect(spec.plugins).toEqual(template.resource.spec.plugins); + expect(spec.plugins, "an edit must not delete plugins it never showed").toEqual(template.resource.spec.plugins); expect(spec.promptTemplate).toEqual(template.resource.spec.promptTemplate); }); @@ -95,6 +95,7 @@ describe("the agent template draft", () => { expect(spec.systemPrompt).toBe("Be brief."); expect(spec.outputSchema).toEqual(template.resource.spec.outputSchema); expect(spec.tools).toEqual(template.resource.spec.tools); + expect(spec.skills).toEqual(template.resource.spec.skills); }); it("drops what it does show when the draft empties it", () => { @@ -110,7 +111,7 @@ describe("the agent template draft", () => { expect(spec).not.toHaveProperty("systemPrompt"); expect(spec).not.toHaveProperty("tools"); // And still not at the cost of the fields it does not model. - expect(spec.skills).toHaveLength(1); + expect(spec.plugins).toHaveLength(1); }); it("never sends both prompt sources, because the CRD rejects both", () => { @@ -368,3 +369,159 @@ describe("cross-namespace references", () => { expect(cleared.tools).toHaveLength(1); }); }); + +const DIGEST = "sha256:" + "0".repeat(64); +const CATALOG = { + image: `ghcr.io/kagent-dev/kagent/tools@${DIGEST}`, + skills: [{ name: "k8s-troubleshooting", description: "Diagnose unhealthy workloads.", path: "opt/kagent-tools-skills/skills/k8s-troubleshooting" }], +}; + +function draftWithSkills(...skills: Partial[]): AgentTemplateDraft { + const draft = emptyDraft("kagent"); + draft.name = "t"; + draft.skills = skills.map((skill) => ({ ...emptySkillDraft(), ...skill })); + return draft; +} + +describe("skills", () => { + it("round-trips every source kind, with and without the optional fields", () => { + const skills = [ + { name: "oci", source: { oci: `ghcr.io/org/skills@${DIGEST}` } }, + { name: "git", source: { git: { url: "https://github.com/org/skills.git", commit: "a".repeat(40) }, path: "skills/git" } }, + { + name: "s3", + source: { + bucket: { s3: { endpoint: "https://s3.example.com", bucket: "b", key: "k.tgz", versionId: "v1", region: "eu-west-1" } }, + }, + }, + { + name: "s3-no-region", + source: { bucket: { s3: { endpoint: "https://s3.example.com", bucket: "b", key: "k.tgz", versionId: "v2" } }, path: "x" }, + }, + ]; + const draft = draftFromSpec({ skills }, "kagent"); + + expect(draft.skills.map((skill) => skill.sourceKind)).toEqual(["oci", "git", "s3", "s3"]); + expect(specFromDraft(draft, { skills }).skills).toEqual(skills); + }); + + it("trims what it sends, omits an empty path and region, and drops blank rows", () => { + const draft = draftWithSkills( + { name: " review ", sourceKind: "s3", s3Endpoint: " https://s3.example.com ", s3Bucket: "b", s3Key: "k", s3VersionId: "v", path: " " }, + { sourceKind: "git" }, + ); + + expect(specFromDraft(draft).skills).toEqual([ + { name: "review", source: { bucket: { s3: { endpoint: "https://s3.example.com", bucket: "b", key: "k", versionId: "v" } } } }, + ]); + expect(draftProblems(draft, { isCreate: true })).toEqual([]); + }); + + it("removes spec.skills when the draft has none", () => { + const spec = { skills: [{ name: "a", source: { oci: `ghcr.io/org/skills@${DIGEST}` } }] }; + const draft = draftFromSpec(spec, "kagent"); + draft.skills = []; + + expect(specFromDraft(draft, spec)).not.toHaveProperty("skills"); + }); + + it("keeps the other kinds' text when the source kind changes", () => { + const draft = draftWithSkills({ name: "a", sourceKind: "git", gitUrl: "https://x.test/r.git", gitCommit: "b".repeat(40) }); + draft.skills[0].sourceKind = "oci"; + draft.skills[0].sourceKind = "git"; + + expect(specFromDraft(draft).skills?.[0].source).toEqual({ git: { url: "https://x.test/r.git", commit: "b".repeat(40) } }); + }); + + it("writes a kagent skill as the catalog image and path, and recognizes it again", () => { + const draft = draftWithSkills(); + draft.skills = [builtinSkillDraft(CATALOG, CATALOG.skills[0])]; + const spec = specFromDraft(draft); + + expect(spec.skills).toEqual([ + { name: "k8s-troubleshooting", source: { oci: CATALOG.image, path: "opt/kagent-tools-skills/skills/k8s-troubleshooting" } }, + ]); + const reopened = draftFromSpec(spec, "kagent").skills[0]; + expect(matchBuiltinSkill(reopened, CATALOG)).toEqual({ skill: CATALOG.skills[0], current: true }); + expect(matchBuiltinSkill({ ...reopened, path: "opt/kagent-tools-skills/skills/other" }, CATALOG)).toBeUndefined(); + expect(matchBuiltinSkill({ ...reopened, name: "my-copy" }, CATALOG)).toBeUndefined(); + expect(matchBuiltinSkill({ ...reopened, oci: `ghcr.io/org/skills@${DIGEST}` }, CATALOG)).toBeUndefined(); + expect(matchBuiltinSkill(reopened, undefined)).toBeUndefined(); + }); + + it("recognizes a kagent skill from an earlier image of the same repository as an older version", () => { + const older = draftWithSkills({ + name: "k8s-troubleshooting", + oci: `ghcr.io/kagent-dev/kagent/tools@sha256:${"1".repeat(64)}`, + path: "opt/kagent-tools-skills/skills/k8s-troubleshooting", + }).skills[0]; + + expect(matchBuiltinSkill(older, CATALOG)).toEqual({ skill: CATALOG.skills[0], current: false }); + }); + + it("reports what the CRD would refuse, naming the skill", () => { + const draft = draftWithSkills( + { name: "", oci: `ghcr.io/org/skills@${DIGEST}` }, + { name: "tagged", oci: "ghcr.io/org/skills:v1" }, + { name: "git", sourceKind: "git", gitUrl: "git@github.com:org/r.git", gitCommit: "abc123" }, + { name: "s3", sourceKind: "s3", s3Endpoint: "s3.example.com", s3Bucket: "b" }, + { name: "up", oci: `ghcr.io/org/skills@${DIGEST}`, path: "skills/../secrets" }, + { name: "abs", oci: `ghcr.io/org/skills@${DIGEST}`, path: "/skills" }, + { name: "up", oci: `ghcr.io/org/skills@${DIGEST}` }, + { name: "..", oci: `ghcr.io/org/skills@${DIGEST}` }, + { name: "a/b", oci: `ghcr.io/org/skills@${DIGEST}` }, + { name: "a\\b", oci: `ghcr.io/org/skills@${DIGEST}` }, + { name: "from-plugin", oci: `ghcr.io/org/skills@${DIGEST}` }, + ); + draft.pluginSkills = ["from-plugin"]; + + expect(draftProblems(draft, { isCreate: true })).toEqual([ + "Skill 1 needs a name.", + 'The skill "tagged" needs an OCI image pinned by digest, like ghcr.io/org/skills@sha256:<64 hex characters>.', + 'The skill "git" needs a Git URL that starts with http:// or https://.', + 'The skill "git" needs a full Git commit ID: 40 or 64 hex characters.', + 'The skill "s3" needs an S3 endpoint that starts with http:// or https://.', + 'The skill "s3" needs an S3 key and version ID.', + 'The skill "up" needs a path relative to the artifact, without ".." segments.', + 'The skill "abs" needs a path relative to the artifact, without ".." segments.', + 'The skill ".." needs a name that is one path segment: not "." or "..", and without "/" or "\\".', + 'The skill "a/b" needs a name that is one path segment: not "." or "..", and without "/" or "\\".', + 'The skill "a\\b" needs a name that is one path segment: not "." or "..", and without "/" or "\\".', + 'More than one skill is named "up". Skill names must be unique.', + 'A plugin on this template already selects a skill named "from-plugin". Skill names must be unique.', + ]); + }); + + it("checks names against the skills the existing spec's plugins select", () => { + const spec = { + skills: [{ name: "review", source: { oci: `ghcr.io/org/skills@${DIGEST}` } }], + plugins: [{ source: { oci: `ghcr.io/org/plugin@${DIGEST}` }, skills: ["review"] }], + }; + + expect(draftProblems(draftFromSpec(spec, "kagent"), { isCreate: false })).toEqual([ + 'A plugin on this template already selects a skill named "review". Skill names must be unique.', + ]); + }); + + it("numbers a skill by its row, counting blank rows", () => { + const draft = draftWithSkills({}, { oci: `ghcr.io/org/skills@${DIGEST}` }); + + expect(draftProblems(draft, { isCreate: true })).toEqual(["Skill 2 needs a name."]); + }); + + it("treats a row as blank from its name, path and active source, and saves only that source", () => { + const hidden = { sourceKind: "oci" as const, gitUrl: "https://x.test/r.git", s3Bucket: "b" }; + expect(specFromDraft(draftWithSkills(hidden))).not.toHaveProperty("skills"); + expect(draftProblems(draftWithSkills(hidden), { isCreate: true })).toEqual([]); + + const draft = draftWithSkills({ ...hidden, name: "a", oci: `ghcr.io/org/skills@${DIGEST}` }); + expect(specFromDraft(draft).skills).toEqual([{ name: "a", source: { oci: `ghcr.io/org/skills@${DIGEST}` } }]); + }); + + it("allows at most 50 skills", () => { + const skills = Array.from({ length: 51 }, (_, index) => ({ name: `s${index}`, oci: `ghcr.io/org/skills@${DIGEST}` })); + + expect(draftProblems(draftWithSkills(...skills.slice(0, 50)), { isCreate: true })).toEqual([]); + expect(draftProblems(draftWithSkills(...skills), { isCreate: true })).toEqual(["A template can have at most 50 skills."]); + }); +}); diff --git a/ui/src/components/agent-template-form/agentTemplateDraft.ts b/ui/src/components/agent-template-form/agentTemplateDraft.ts index 1440fa4e7f..d1a424594d 100644 --- a/ui/src/components/agent-template-form/agentTemplateDraft.ts +++ b/ui/src/components/agent-template-form/agentTemplateDraft.ts @@ -4,14 +4,13 @@ * ## The property that matters most here * * **An edit must not delete what the form does not show.** This form authors the - * common interactive fields; `skills`, `plugins` and - * `promptTemplate` are rich enough — three artifact-source shapes, each with its own - * strict CEL pattern — that authoring them is its own piece of work. + * common interactive fields and `skills`; `plugins` and `promptTemplate` are not on + * it yet. * * Building an update out of the fields a form displays silently drops the rest. That * has already happened in this repository once. `specFromDraft` therefore merges * into the spec it was given rather than constructing a fresh one, so a template carrying - * skills survives an edit that never mentioned them. + * plugins survives an edit that never mentioned them. * * `agentTemplateDraft.test.ts` pins that, because it is invisible on screen — the * form looks identical either way and the loss only shows up on the cluster. @@ -19,10 +18,13 @@ import type { AgentTemplate, + AgentTemplateSkill, AgentTemplateSpec, + ArtifactSource, ToolBinding, ObjectReference, } from "@/api/domain/agentTemplates"; +import type { BuiltinSkill, BuiltinSkillsCatalog } from "@/api/runtimeConfig"; /** One MCP tool selection, flattened for a form to hold. */ export interface McpToolDraft { @@ -44,6 +46,35 @@ export interface SubAgentToolDraft { refName: string; } +export type SkillSourceKind = "oci" | "git" | "s3"; + +/** + * One skill, flattened for a form to hold. Every source kind keeps its own fields, + * so switching kind and back does not lose what was typed. + */ +export interface SkillDraft { + name: string; + sourceKind: SkillSourceKind; + oci: string; + gitUrl: string; + gitCommit: string; + s3Endpoint: string; + s3Bucket: string; + s3Key: string; + s3VersionId: string; + s3Region: string; + path: string; +} + +type SkillTextField = Exclude; + +/** The fields each source kind writes. Text in another kind's fields is ignored. */ +const SKILL_SOURCE_FIELDS: Record = { + oci: ["oci"], + git: ["gitUrl", "gitCommit"], + s3: ["s3Endpoint", "s3Bucket", "s3Key", "s3VersionId", "s3Region"], +}; + /** Where the system prompt comes from. The CRD rejects both at once. */ export type PromptSource = "inline" | "configMap"; @@ -68,6 +99,12 @@ export interface AgentTemplateDraft { outputStrict: boolean; mcpTools: McpToolDraft[]; subAgentTools: SubAgentToolDraft[]; + skills: SkillDraft[]; + /** + * The skill names `spec.plugins` selects. Not authored here, but the controller + * rejects a skill named like one of them. + */ + pluginSkills: string[]; labels: { key: string; value: string }[]; } @@ -89,10 +126,57 @@ export function emptyDraft(namespace: string): AgentTemplateDraft { outputStrict: false, mcpTools: [], subAgentTools: [], + skills: [], + pluginSkills: [], labels: [], }; } +export function emptySkillDraft(): SkillDraft { + return { + name: "", + sourceKind: "oci", + oci: "", + gitUrl: "", + gitCommit: "", + s3Endpoint: "", + s3Bucket: "", + s3Key: "", + s3VersionId: "", + s3Region: "", + path: "", + }; +} + +/** One of kagent's built-in skills, as a row of the form. */ +export function builtinSkillDraft(catalog: BuiltinSkillsCatalog, skill: BuiltinSkill): SkillDraft { + return { ...emptySkillDraft(), name: skill.name, oci: catalog.image, path: skill.path }; +} + +/** + * The built-in skill a row holds: the catalog image's repository and a catalog path. + * `current` is false when the row pins another digest, as a row saved before a + * chart upgrade does. + */ +export function matchBuiltinSkill( + draft: SkillDraft, + catalog: BuiltinSkillsCatalog | undefined, +): { skill: BuiltinSkill; current: boolean } | undefined { + if (!catalog || draft.sourceKind !== "oci") return undefined; + const image = draft.oci.trim(); + if (imageRepository(image) !== imageRepository(catalog.image)) return undefined; + // The name must match too: a user may point their own skill at a catalog path under + // another name, and that row must keep its editable name and source. + const skill = catalog.skills.find( + (entry) => entry.path === draft.path.trim() && entry.name === draft.name.trim(), + ); + return skill && { skill, current: image === catalog.image }; +} + +function imageRepository(image: string): string { + return image.split("@")[0]; +} + /** The draft a form opens with when editing an existing template. */ export function draftFromTemplate(template: AgentTemplate): AgentTemplateDraft { return { @@ -145,6 +229,8 @@ export function draftFromSpec(spec: AgentTemplateSpec, namespace: string): Agent description: binding.subAgent?.description ?? "", refName: binding.subAgent?.templateRef.name ?? "", })), + skills: (spec.skills ?? []).map(skillDraftFromSpec), + pluginSkills: (spec.plugins ?? []).flatMap((plugin) => plugin.skills ?? []), labels: [], }; } @@ -153,8 +239,8 @@ export function draftFromSpec(spec: AgentTemplateSpec, namespace: string): Agent * The spec a draft describes, merged onto the one it came from. * * `existing` is the whole reason this takes an argument. Omit it and every field - * this form does not model — `skills`, `plugins`, `promptTemplate` — is dropped from - * the resource, which the API accepts happily and the reader never sees. + * this form does not model — `plugins`, `promptTemplate` — is dropped from the + * resource, which the API accepts happily and the reader never sees. */ export function specFromDraft( draft: AgentTemplateDraft, @@ -241,6 +327,10 @@ export function specFromDraft( if (tools.length > 0) spec.tools = tools; else delete spec.tools; + const skills = draft.skills.filter((skill) => !isBlankSkill(skill)).map(skillFromDraft); + if (skills.length > 0) spec.skills = skills; + else delete spec.skills; + return spec; } @@ -310,9 +400,130 @@ export function draftProblems( ); } } + problems.push(...skillProblems(draft.skills, draft.pluginSkills)); return problems; } +/** Mirrors the CRD's `AgentTemplateSkill` and `ArtifactSource` validation. */ +const MAX_SKILLS = 50; +const MAX_PATH_LENGTH = 1024; +const OCI_DIGEST_REF = /^[^\s@]+@sha256:[0-9a-f]{64}$/; +const HTTP_URL = /^https?:\/\/\S+$/; +const GIT_COMMIT = /^([0-9a-fA-F]{40}|[0-9a-fA-F]{64})$/; + +function skillProblems(drafts: SkillDraft[], pluginSkills: string[]): string[] { + const problems: string[] = []; + if (drafts.filter((skill) => !isBlankSkill(skill)).length > MAX_SKILLS) { + problems.push(`A template can have at most ${MAX_SKILLS} skills.`); + } + + const seen = new Set(); + const repeated = new Set(); + const fromPlugins = new Set(); + drafts.forEach((draft, index) => { + if (isBlankSkill(draft)) return; + const skill = skillFromDraft(draft); + const label = skill.name ? `The skill "${skill.name}"` : `Skill ${index + 1}`; + if (!skill.name) problems.push(`${label} needs a name.`); + else if (skill.name === "." || skill.name === ".." || /[/\\]/.test(skill.name)) { + problems.push(`${label} needs a name that is one path segment: not "." or "..", and without "/" or "\\".`); + } else if (seen.has(skill.name)) repeated.add(skill.name); + else if (pluginSkills.includes(skill.name)) fromPlugins.add(skill.name); + seen.add(skill.name); + + const { source } = skill; + if (draft.sourceKind === "oci" && !OCI_DIGEST_REF.test(source.oci ?? "")) { + problems.push(`${label} needs an OCI image pinned by digest, like ghcr.io/org/skills@sha256:<64 hex characters>.`); + } + if (draft.sourceKind === "git") { + if (!HTTP_URL.test(source.git?.url ?? "")) { + problems.push(`${label} needs a Git URL that starts with http:// or https://.`); + } + if (!GIT_COMMIT.test(source.git?.commit ?? "")) { + problems.push(`${label} needs a full Git commit ID: 40 or 64 hex characters.`); + } + } + if (draft.sourceKind === "s3") { + const s3 = source.bucket?.s3; + if (!HTTP_URL.test(s3?.endpoint ?? "")) { + problems.push(`${label} needs an S3 endpoint that starts with http:// or https://.`); + } + const missing = [ + s3?.bucket ? "" : "bucket", + s3?.key ? "" : "key", + s3?.versionId ? "" : "version ID", + ].filter(Boolean); + if (missing.length > 0) { + const fields = missing.length === 1 ? missing[0] : `${missing.slice(0, -1).join(", ")} and ${missing.at(-1)}`; + problems.push(`${label} needs an S3 ${fields}.`); + } + } + const path = source.path ?? ""; + if (path.startsWith("/") || path.split("/").includes("..")) { + problems.push(`${label} needs a path relative to the artifact, without ".." segments.`); + } + if (path.length > MAX_PATH_LENGTH) { + problems.push(`${label} needs a path of at most ${MAX_PATH_LENGTH} characters.`); + } + }); + for (const name of repeated) { + problems.push(`More than one skill is named "${name}". Skill names must be unique.`); + } + for (const name of fromPlugins) { + problems.push(`A plugin on this template already selects a skill named "${name}". Skill names must be unique.`); + } + return problems; +} + +function skillDraftFromSpec(skill: AgentTemplateSkill): SkillDraft { + const { source } = skill; + const s3 = source.bucket?.s3; + return { + name: skill.name, + sourceKind: source.git ? "git" : s3 ? "s3" : "oci", + oci: source.oci ?? "", + gitUrl: source.git?.url ?? "", + gitCommit: source.git?.commit ?? "", + s3Endpoint: s3?.endpoint ?? "", + s3Bucket: s3?.bucket ?? "", + s3Key: s3?.key ?? "", + s3VersionId: s3?.versionId ?? "", + s3Region: s3?.region ?? "", + path: source.path ?? "", + }; +} + +function skillFromDraft(draft: SkillDraft): AgentTemplateSkill { + let source: ArtifactSource; + if (draft.sourceKind === "git") { + source = { git: { url: draft.gitUrl.trim(), commit: draft.gitCommit.trim() } }; + } else if (draft.sourceKind === "s3") { + const region = draft.s3Region.trim(); + source = { + bucket: { + s3: { + endpoint: draft.s3Endpoint.trim(), + bucket: draft.s3Bucket.trim(), + key: draft.s3Key.trim(), + versionId: draft.s3VersionId.trim(), + ...(region ? { region } : {}), + }, + }, + }; + } else { + source = { oci: draft.oci.trim() }; + } + const path = draft.path.trim(); + if (path) source.path = path; + return { name: draft.name.trim(), source }; +} + +/** A row the reader added and never filled in, which a save drops. */ +function isBlankSkill(draft: SkillDraft): boolean { + const fields: SkillTextField[] = ["name", "path", ...SKILL_SOURCE_FIELDS[draft.sourceKind]]; + return fields.every((field) => draft[field].trim() === ""); +} + /** Keep local manifests concise while preserving shared resource identity. */ function normalizedReference(ref: ObjectReference, ownerNamespace: string): ObjectReference { const name = ref.name.trim(); diff --git a/ui/src/components/agent-template-form/unshownFields.ts b/ui/src/components/agent-template-form/unshownFields.ts index ef6fc8a1fe..7630733e2b 100644 --- a/ui/src/components/agent-template-form/unshownFields.ts +++ b/ui/src/components/agent-template-form/unshownFields.ts @@ -3,22 +3,16 @@ import type { AgentTemplateSpec } from "@/api/domain/agentTemplates"; /** * Whether a template carries spec fields the form does not author. * - * `skills`, `plugins` and `promptTemplate` are not on the form — each is a rich - * shape of its own, with three artifact-source variants and strict CEL patterns — - * and an edit deliberately carries them through untouched rather than dropping - * them. + * `plugins` and `promptTemplate` are not on the form, and an edit deliberately + * carries them through untouched rather than dropping them. * * This exists so the form can *say* that. A reader who knows their template has - * skills and cannot see them would reasonably conclude a save will remove them, + * plugins and cannot see them would reasonably conclude a save will remove them, * and the only way to know otherwise is to be told. * * In its own module rather than beside the component so the form file exports a * component and nothing else — which is what keeps fast refresh working on it. */ export function hasUnshownSpecFields(spec: AgentTemplateSpec): boolean { - return Boolean( - (spec.skills?.length ?? 0) > 0 || - (spec.plugins?.length ?? 0) > 0 || - spec.promptTemplate, - ); + return Boolean((spec.plugins?.length ?? 0) > 0 || spec.promptTemplate); } diff --git a/ui/src/env.ts b/ui/src/env.ts index 2a764393db..edb85d56f4 100644 --- a/ui/src/env.ts +++ b/ui/src/env.ts @@ -26,6 +26,8 @@ export interface EnvironmentVariables { ENABLE_MOCK_UI: string; /** Public path prefix the UI is served under, e.g. `/ui`; empty at the root. */ BASE_PATH: string; + /** JSON naming the skills image and the skills in it; read through `runtimeConfig`. */ + BUILTIN_SKILLS: string; } /** @@ -46,6 +48,7 @@ export const CORE_ENV_VARS = { STREAM_TIMEOUT_MS: "KAGENT_UI_STREAM_TIMEOUT_MS", ENABLE_MOCK_UI: "KAGENT_UI_ENABLE_MOCK", BASE_PATH: "KAGENT_UI_BASE_PATH", + BUILTIN_SKILLS: "KAGENT_UI_BUILTIN_SKILLS", } as const satisfies Record; /** @@ -59,6 +62,7 @@ export const ENV_DEFAULTS: EnvironmentVariables = { STREAM_TIMEOUT_MS: "1800000", ENABLE_MOCK_UI: "false", BASE_PATH: "", + BUILTIN_SKILLS: "", }; declare global { diff --git a/ui/src/mocks/fixtures.ts b/ui/src/mocks/fixtures.ts index 41c068e0c5..fb144c8fc7 100644 --- a/ui/src/mocks/fixtures.ts +++ b/ui/src/mocks/fixtures.ts @@ -26,6 +26,7 @@ import type { Agent, AgentSpec, AgentStatus } from "@/api/domain/agents"; import type { AgentTemplate } from "@/api/domain/agentTemplates"; import type { EgressPolicy, EgressPolicySpec } from "@/api/domain/egressPolicies"; import type { SandboxTemplate } from "@/api/domain/sandboxTemplates"; +import type { BuiltinSkillsCatalog } from "@/api/runtimeConfig"; export const mockModels: ModelConfig[] = [ { @@ -698,6 +699,28 @@ export const mockHarnesses: Harness[] = [ ]; +/** Served as `BUILTIN_SKILLS`, which a deployment gets from the chart. */ +export const mockBuiltinSkills: BuiltinSkillsCatalog = { + image: "ghcr.io/kagent-dev/kagent/tools@sha256:0f1e2d3c4b5a69788796a5b4c3d2e1f00f1e2d3c4b5a69788796a5b4c3d2e1f0", + skills: [ + { + name: "helm-release-management", + path: "opt/kagent-tools-skills/skills/helm-release-management", + description: "Inspect, upgrade and uninstall Helm releases.", + }, + { + name: "istio-traffic-debugging", + path: "opt/kagent-tools-skills/skills/istio-traffic-debugging", + description: "Debug Istio service mesh traffic: 503s, mTLS failures, routing not applied.", + }, + { + name: "k8s-troubleshooting", + path: "opt/kagent-tools-skills/skills/k8s-troubleshooting", + description: "Diagnose unhealthy Kubernetes workloads: CrashLoopBackOff, Pending, ImagePullBackOff.", + }, + ], +}; + export const mockAgentTemplates: AgentTemplate[] = [ { ref: "kagent/k8s-agent-7f3a91c", @@ -723,16 +746,33 @@ export const mockAgentTemplates: AgentTemplate[] = [ }, }, ], - // Not authored by the form, and here on purpose: an edit that dropped it - // would be invisible on screen, so this is what the round-trip test guards. skills: [ + { + name: "k8s-troubleshooting", + source: { oci: mockBuiltinSkills.image, path: "opt/kagent-tools-skills/skills/k8s-troubleshooting" }, + }, { name: "incident-review", source: { - oci: "ghcr.io/kagent-dev/skills@sha256:0f1e2d3c4b5a69788796a5b4c3d2e1f00f1e2d3c4b5a69788796a5b4c3d2e1f0", + git: { + url: "https://github.com/example/runbooks.git", + commit: "3f1c9d2e5b7a48e0a1c6d9f2b4e8a7c30d5f6e9b", + }, + path: "skills/incident-review", + }, + }, + // Saved before an upgrade changed the kagent-tools image's digest. + { + name: "helm-release-management", + source: { + oci: "ghcr.io/kagent-dev/kagent/tools@sha256:1a2b3c4d5e6f708192a3b4c5d6e7f8091a2b3c4d5e6f708192a3b4c5d6e7f809", + path: "opt/kagent-tools-skills/skills/helm-release-management", }, }, ], + // Not authored by the form, and here on purpose: an edit that dropped it + // would be invisible on screen. + promptTemplate: { dataSources: [{ name: "kagent-builtin-prompts", alias: "builtin" }] }, }, }, }, diff --git a/ui/src/mocks/startMockBackend.ts b/ui/src/mocks/startMockBackend.ts index 335e61ac7b..c677857318 100644 --- a/ui/src/mocks/startMockBackend.ts +++ b/ui/src/mocks/startMockBackend.ts @@ -17,6 +17,7 @@ import { MOCK_API_BASE_URL } from "@/api/config"; import { setApiTransport } from "@/api/transport"; import { mockTransport } from "./transport"; +import { mockBuiltinSkills } from "./fixtures"; import { AUTH_SCENARIOS, AUTH_SCENARIO_PARAM, @@ -30,6 +31,13 @@ import { export async function startMockBackend(): Promise { setApiTransport(mockTransport); + if (!window.environmentVariables?.BUILTIN_SKILLS) { + window.environmentVariables = { + ...window.environmentVariables, + BUILTIN_SKILLS: JSON.stringify(mockBuiltinSkills), + }; + } + const { worker } = await import("./browser"); await worker.start({ diff --git a/ui/src/pages/AgentTemplateDetailsPage.tsx b/ui/src/pages/AgentTemplateDetailsPage.tsx index 4278d2b908..edc5af0b86 100644 --- a/ui/src/pages/AgentTemplateDetailsPage.tsx +++ b/ui/src/pages/AgentTemplateDetailsPage.tsx @@ -146,8 +146,8 @@ export function AgentTemplateDetailsPage() { namespace: template.data.namespace, labels: labelsFromDraft(draft), }, - // Merged onto the spec that was read, so skills, plugins and prompt data - // sources survive an edit that never mentioned them. + // Merged onto the spec that was read, so plugins and prompt data sources + // survive an edit that never mentioned them. spec: specFromDraft(draft, template.data.resource.spec), }, });