Fix: run plugin hooks on command failure, not just success

[derekmisler]

This PR is part of a trilogy:

1. docker/cli <-- you are here
2. docker/ai
3. docker/pinata

---

Closes https://github.com/docker/gordon/issues/125
- What I did

Enabled CLI plugin hooks to fire on command failure, not just success. Previously, when a plugin-delegated command like docker build (buildx) or docker compose up failed, the hooks system was skipped entirely. This meant plugins like ai, scout, and debug could never show "What's next:" hints for failed builds or compose errors — exactly when users would benefit most from a suggestion like docker ai "fix my problem".

Additionally, introduced a new error-hooks plugin configuration key that lets plugins opt in to error-only hooks, separate from the existing hooks key that fires on every invocation.

- How I did it

Three coordinated changes:

1. cli-plugins/manager/hooks.go — Extended RunPluginHooks to accept a cmdErrorMessage string and forward it to runHooks (previously hardcoded to ""). Refactored pluginMatch to check both hooks (always fires) and error-hooks (fires only when cmdErrorMessage is non-empty). Extracted a new matchHookConfig helper to deduplicate the comma-separated hook matching logic.

2. cmd/docker/docker.go — Moved the RunPluginHooks call out of the if err == nil block so hooks fire regardless of plugin exit status. The error message is extracted the same way the native command path (RunCLICommandHooks) already does. Hooks are still skipped for "not found" errors (i.e., unknown commands) to avoid false triggers.

3. *_test.go — Added comprehensive tests for:

   • pluginMatch with the new cmdErrorMessage parameter and error-hooks configuration
   • The new matchHookConfig helper
   • RunPluginHooks integration tests verifying both success and failure paths
   • invokeAndCollectHooks edge cases (no plugins, cancelled context, error-hooks skipped on success)
   • getExitCode covering StatusError, wrapped errors, and signal termination

---

- How to verify it

These two PRs work together:

• docker/cli#6794 — Makes plugin hooks fire on command failure and adds error-hooks config
• docker/ai#384 — Registers the ai plugin's hook handler to show "ask Gordon" hints when commands fail

1. Build the custom CLI binary

git clone https://github.com/docker/cli.git
cd cli
gh pr checkout https://github.com/docker/cli/pull/6794
make -f docker.Makefile build
# Binary: ./build/docker

2. Build and install the custom docker-ai plugin

Note: task cli:install is currently broken due to a dotenv issue in an unrelated Taskfile. Run the build steps directly instead.

git clone https://github.com/docker/ai.git
cd ai
gh pr checkout https://github.com/docker/ai/pull/384
cd cli
go build -trimpath -ldflags="-s -w" -o docker-ai .
ln -sf "$PWD/docker-ai" ~/.docker/cli-plugins/docker-ai

3. Configure hooks in ~/.docker/config.json

Add the features and plugins keys (merge with your existing config):

{
  "features": {
    "hooks": "true"
  },
  "plugins": {
    "ai": {
      "hooks": "run",
      "error-hooks": "build,buildx build,compose"
    }
  }
}

Important: error-hooks must include both "build" and "buildx build". docker build and docker buildx build produce different command strings internally, and the hook matching is a prefix token match — "buildx build" won't match a bare docker build. Use "compose" (not "compose up") because flags like -f appear between compose and up in the args, breaking the positional match.

4. Test the full flow

Failing build (the main use case):

echo "FROM nonexistent:image" > /tmp/Dockerfile
~/path/to/cli/build/docker build /tmp
# Expected output after the build error:
#   What's next:
#     Debug this build failure with Gordon → docker ai "help me fix this build failure"

Failing run:

~/path/to/cli/build/docker run nonexistent:image
# Expected:
#   What's next:
#     Debug this container error with Gordon → docker ai "help me fix this container error"

Failing compose:

cat > /tmp/docker-compose.yml <<'EOF'
services:
  broken:
    image: nonexistent:image
EOF
~/path/to/cli/build/docker compose -f /tmp/docker-compose.yml up
# Expected:
#   What's next:
#     Debug this Compose error with Gordon → docker ai "help me fix this compose error"

Successful build (error-hooks should NOT fire):

~/path/to/cli/build/docker build -t test .
# Expected: build succeeds, NO Gordon hints shown
# (error-hooks only fire on failure)

Successful run (regular hooks fire, but plugin returns nothing):

~/path/to/cli/build/docker run --rm hello-world
# Expected: run succeeds, no hints shown
# (ai plugin returns nothing when CommandError is empty)

5. Run the tests

# CLI plugin hooks tests
cd cli
go test ./cli-plugins/manager/... ./cmd/docker/...

# AI plugin hooks tests
cd ai/cli
go test ./commands/... -run Hook

6. Restore original plugin when done

ln -sf /Applications/Docker.app/Contents/Resources/cli-plugins/docker-ai \
  ~/.docker/cli-plugins/docker-ai

And remove the features/plugins keys from ~/.docker/config.json if you don't want hooks active.

---

- Human readable description for the release notes

CLI plugin hooks now fire on command failure (not just success), and plugins can use "error-hooks" to show hints only when commands fail.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 56.52174% with 10 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
cmd/docker/docker.go	0.00%	10 Missing ⚠️

📢 Thoughts on this report? Let us know!

[thaJeztah]

cc @vvoland @laurazard looks related to what was implemented in;

• hooks: pass command execution error to plugins #5033

[derekmisler]

cc @vvoland @laurazard looks related to what was implemented in;

• hooks: pass command execution error to plugins #5033

Yeah, that PR was helpful when working on this one. I saw we were hard-coding an empty "" in place of the error here: https://github.com/docker/cli/pull/5033/changes#diff-7e76b3d50db2ee696d1cdbaa3f1e429b179da962bb8f308960ef85bc0ba835a2R43

[derekmisler]

I'm not sure about that e2e test, @thaJeztah, it passes sometimes and fails others, and I can't figure out why.

[thaJeztah]

Yeah, that test is rather flaky; not an issue with this PR; I restarted CI.

Yeah, that PR was helpful when working on this one. I saw we were hard-coding an empty "" in place of the error here

Yup; I mostly pinged @vvoland and @laurazard to check if they recalled there was a specific reason; there was this line in the PR description;

This is only available for CLI command executions.

And I do recall there was some chatter at the the about some risks with hook being triggered recursively (with a growing number of plugins, there's also a growing amount of overhead).

[thaJeztah]

Just so I don't forget; this function is exported (and may be in use elsewhere), so we probably can't change the signature.

[derekmisler]

This was the only implementation site I could find: https://github.com/docker/cli/pull/6794/changes#diff-3dcd6325b70a5f5db393a3ecc08060b69007fcef371fb4dabf3164da95646fb6R488

[thaJeztah]

Ah, yes, maybe we're fine; at least nothing in a public repository AFAICS; https://grep.app/search?q=.RunPluginHooks

We should probably consider to move some of this to internal/, and make the "public" (cli-plugins/xxxx) wrappers for the internal implementation so that we have a better separation between "these are only to be used by the CLI itself" and "these are "ok(ish)" for external consumers". We've been painted in a corner too many times because things were meant for the CLI itself, but happened to be exported, and now ... got used by other projects.

[thaJeztah]

Forgot to post this, but in case it's useful information; I was curious what the performance impact would be. For that I chose a "minimal" call to some plugins; I picked <plugin> version, which should be very lightweight, and (I expect) not be making API calls.

First, timing when running that command as "plugin" (docker <plugin-name> version); note that some of the overhead here is in the CLI, which may still needs optimization (it currently scans for CLI plugin candidates that can provide a subcommand, so effectively, all plugins may get invoked here for the "metadata" subcommand). I also set a remote context (-c remote1), pointing ao a ssh:// connection;

hyperfine --runs 5 --warmup 2 'docker -c remote1 --version'
Benchmark 1: docker -c remote1 --version
  Time (mean ± σ):      20.5 ms ±   2.0 ms    [User: 10.7 ms, System: 8.5 ms]
  Range (min … max):    17.5 ms …  23.2 ms    5 runs

hyperfine --runs 5 --warmup 2 'docker -c remote1 compose version'
Benchmark 1: docker -c remote1 compose version
  Time (mean ± σ):      53.1 ms ±   2.4 ms    [User: 19.4 ms, System: 13.2 ms]
  Range (min … max):    50.0 ms …  55.5 ms    5 runs

hyperfine --runs 5 --warmup 2 'docker -c remote1 ai version'
Benchmark 1: docker -c remote1 ai version
  Time (mean ± σ):      72.6 ms ±   5.1 ms    [User: 31.8 ms, System: 17.0 ms]
  Range (min … max):    65.2 ms …  78.4 ms    5 runs

hyperfine --runs 5 --warmup 2 'docker -c remote1 buildx version'
Benchmark 1: docker -c remote1 buildx version
  Time (mean ± σ):      76.7 ms ±   3.7 ms    [User: 37.4 ms, System: 19.8 ms]
  Range (min … max):    71.7 ms …  81.2 ms    5 runs

Also looking at running the same command, but invoking the plugins directly;

hyperfine --runs 5 --warmup 2 'docker --version'
Benchmark 1: docker --version
  Time (mean ± σ):      13.6 ms ±   1.9 ms    [User: 7.1 ms, System: 5.3 ms]
  Range (min … max):    11.8 ms …  16.7 ms    5 runs

hyperfine --runs 5 --warmup 2 '~/.docker/cli-plugins/docker-compose version'
Benchmark 1: ~/.docker/cli-plugins/docker-compose version
  Time (mean ± σ):      17.9 ms ±   3.1 ms    [User: 6.5 ms, System: 3.4 ms]
  Range (min … max):    15.7 ms …  23.3 ms    5 runs

hyperfine --runs 5 --warmup 2 '~/.docker/cli-plugins/docker-ai version'
Benchmark 1: ~/.docker/cli-plugins/docker-ai version
  Time (mean ± σ):      28.4 ms ±   1.6 ms    [User: 12.7 ms, System: 5.3 ms]
  Range (min … max):    25.9 ms …  29.8 ms    5 runs

hyperfine --runs 5 --warmup 2 '~/.docker/cli-plugins/docker-buildx version'
Benchmark 1: ~/.docker/cli-plugins/docker-buildx version
  Time (mean ± σ):      54.1 ms ±  13.0 ms    [User: 20.3 ms, System: 13.4 ms]
  Range (min … max):    38.6 ms …  74.6 ms    5 runs

Looks like the ai plugin is not the slowest, but still takes twice as much as the CLI itself (not sure why buildx is so slow though; that's surprising!)

[thaJeztah]

This extra overhead may be less problematic in the "unhappy" path (something failed), although it's possible there's scripts that run a command to detect errors; I THINK (but must double-check) we skip hooks if there's no TTY attached, but it's possible we invoke the hook, but don't print the output.

The overhead is of course also "relative" to the command executed; 50ms of extra time probably doesn't matter on a failing docker pull;

hyperfine --runs 5 --warmup 2 --ignore-failure 'docker pull no-sucn-image'
Benchmark 1: docker pull no-sucn-image
  Time (mean ± σ):     822.0 ms ±  62.9 ms    [User: 25.6 ms, System: 28.1 ms]
  Range (min … max):   765.2 ms … 930.0 ms    5 runs

  Warning: Ignoring non-zero exit code.
  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.

However the way hooks currently work is that they must be registered for each command they should be called on. This means that if we want the ai plugin to be called for assistance on any command, it would be invoked after every command; regardless if the command was successful or failed. That means that setting a hook on every command would add at least 50ms extra; that's based on running the version subcommand, so it'll probably be more.

One thing we could consider is to introduce a separate config for "error-hooks"; i.e., only invoke the hook for errors, but not for the "happy path"; currently hooks are defined like this;

        "plugins": {
                "debug": {
                        "hooks": "exec"
                },
                "scout": {
                        "hooks": "pull,image pull,buildx build"
                }
        },
        "features": {
                "hooks": "true"
        }

We could add a similar config (error-hooks, failure-hooks ?); note that adding hooks for all commands would be ... slightly tedious (every command has to be enumerated in the list); not sure what's best for that; we could have a "error-handler" or wildcard support, but that may become a sliding slope.

There's another thing we must verify (just thinking out loud here, haven't thought it through, and haven't looked at the logic);

• docker compose build nowadays also hands off builds to buildx, and (similar to the CLI itself) invokes buildx as a plugin (docker compose build -> invokes ~/.docker/cli-plugins/docker-compose build -> invokes ~/.docker/cli-plugins/docker-buildx bake). I wonder how hooks are called in that scenario (could it invoke hooks multiple times, recurse?)
• similar to the above; how do we currently handle failures when invoking a hook? (we must make sure we don't trigger a "error-hook" if a "hook" or "error-hook" failed 😂)

[vvoland]

+1 on error hooks for this one

[derekmisler]

Thanks for the thorough review and the detailed benchmarks, @thaJeztah, that's all super helpful context. And thanks @vvoland for the +1 on the direction.

I've reworked this to use the error-hooks approach. The change is contained to pluginMatch in hooks.go:

• "hooks" behavior is unchanged — fires on every invocation
• New "error-hooks" config key — only fires when the command exits with an error

Example config:

{
  "plugins": {
    "ai": {
      "error-hooks": "build,compose,pull"
    }
  }
}

The docker.go changes to move RunPluginHooks outside the if err == nil block are still needed, without that, error-hooks would never get a chance to be evaluated. But the performance impact is negligible: pluginMatch is just string comparison, and when a plugin only declares error-hooks, no subprocess is spawned on success.

Re: the longer-term response header approach (WithResponseHook / X-Docker-Hint) — that sounds like the right eventual architecture, but it's beyond the scope of what I can tackle here. Happy to leave that for y'all to drive.