Skip to content

fix(commands): keep a failed command's output in its error - #90

Open
sathpal wants to merge 1 commit into
kagent-dev:mainfrom
sathpal:fix/keep-command-output-on-failure
Open

sathpal wants to merge 1 commit into
kagent-dev:mainfrom
sathpal:fix/keep-command-output-on-failure

Conversation

@sathpal

@sathpal sathpal commented Oct 21, 2026

Copy link
Copy Markdown

Fixes #89

What this changes

When a command fails, CommandBuilder.executeCommand now puts the command's trimmed output into the error's cause:

if commandOutput := strings.TrimSpace(string(output)); commandOutput != "" {
    err = fmt.Errorf("%w: %s", err, commandOutput)
}

so a failed kubectl get reaches the agent as

[Kubernetes] get deployment does-not-exist -o yaml -n default failed: exit status 1: Error from server (NotFound): deployments.apps "does-not-exist" not found

instead of ending at exit status 1.

  • One place, every CLI: kubectl, helm, istioctl, cilium and the rest all go through executeCommand, cached or not.
  • The original error stays wrapped (%w), so nothing that inspects it changes.
  • When the command printed nothing, the error is exactly as before.
  • The error creators now see the real message, so their existing branches (forbidden, not found, already exists, connection refused) pick the matching suggestions and error code instead of the generic fallback.
  • The output is the same text the shell executor already logs, and the same kind of text a successful command returns, so this adds no new exposure.

Tests

internal/commands/builder_errors_test.go adds a table-driven test with the mock shell executor: an admission webhook denial, an RBAC denial (now K8S_PERMISSION_ERROR) and a failure with no output. The first two fail on main. internal/commands coverage goes from 69.2% to 79.8%.

  • go test -tags=test -cover ./pkg/... ./internal/... ./cmd/...: all packages pass
  • golangci-lint run --build-tags=test (v2.13.2, the Makefile's version): 0 issues
  • go fmt ./...: no changes

Checked on a cluster

kagent v0.10.3 on kind, with this build of the tool server mounted over /tool-server and an admission webhook that holds risky changes. Asked to scale a deployment in a protected namespace, kagent's unchanged k8s-agent now receives the webhook's full message. It explained the hold, quoted the approval command and stopped after one tool call. Without the fix it got exit status 1 and tried the change three different ways.


Prepared with an AI coding assistant; I reviewed the change and ran the tests and the cluster check above.

When kubectl, helm, istioctl or cilium exits non-zero, executeCommand
wraps only the exit error, so the agent is told "exit status 1" and
never sees the command's own message, such as kubectl's
"Error from server (Forbidden): ...". The output is logged by the shell
executor but not returned.

Include the trimmed output in the error's cause. The original error
stays wrapped, and the error creators now see the real message, so the
suggestions they pick from it (permission, not found, already exists,
connection refused) apply instead of the generic fallback.

Signed-off-by: Sathpal-Singh <sathpal.gnec@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Failed commands return only "exit status 1"; the command's error never reaches the agent

1 participant