feat: add stop lifecycle hook for external providers by glours · Pull Request #13779 · docker/compose

glours · 2026-05-07T16:41:45Z

What I did
Provider-backed services were silently skipped on docker compose stop, leaving external resources running after the user expected the stack to be paused (e.g. after Ctrl+C during up --watch).

Compose now invokes <provider> compose stop <service> for providers that advertise a stop block in their metadata subcommand output. Providers that do not advertise stop (or do not implement metadata at all) are silently skipped, preserving backward compatibility with existing providers that pre-date this hook.

Related issue
Closes #13772

(not mandatory) A picture of a cute animal, if possible in relation to what you did

Provider-backed services were silently skipped on `docker compose stop`, leaving external resources running after the user expected the stack to be paused (e.g. after Ctrl+C during `up --watch`). Compose now invokes `<provider> compose stop <service>` for providers that advertise a `stop` block in their `metadata` subcommand output. Providers that do not advertise stop (or do not implement metadata at all) are silently skipped, preserving backward compatibility with existing providers that pre-date this hook. Closes #13772 Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>

docker-agent

Assessment: 🟡 NEEDS ATTENTION

This PR correctly adds the stop lifecycle hook for provider-backed services and preserves backward compatibility. One medium-severity efficiency issue was identified in the new code path.

Copilot

Pull request overview

This PR adds support for a provider “stop” lifecycle hook so provider-backed services are no longer skipped when users run docker compose stop (e.g., after Ctrl+C from up --watch). The hook is opt-in via provider compose metadata advertising a stop block, preserving backward compatibility.

Changes:

Invoke provider compose stop <service> during docker compose stop when the provider advertises stop in metadata.
Extend provider metadata parsing to include an optional stop command block and emit stop-related progress events.
Add unit + e2e coverage and update provider extension docs + example provider implementation.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/compose/stop.go	Adds stop-path logic to call provider stop hook when advertised by metadata.
pkg/compose/plugins.go	Adds stop handling to plugin execution flow and provider metadata model.
pkg/compose/plugins_test.go	Adds unit tests for `ProviderMetadata.Stop` parsing and `IsEmpty()` semantics.
pkg/e2e/providers_test.go	Adds e2e test asserting `docker compose stop` triggers provider stop hook.
pkg/e2e/fixtures/providers/provider-stop.yaml	New fixture to exercise provider-backed service stop behavior.
docs/extension.md	Documents the new stop lifecycle hook and metadata schema.
docs/examples/provider.go	Updates example provider to implement and advertise the stop hook.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+		if serv.Provider != nil {
+			path, err := s.getPluginBinaryPath(serv.Provider.Type)
+			if err != nil {
+				return err
+			}
+			metadata := s.getPluginMetadata(path, serv.Provider.Type, project)
+			if metadata.Stop == nil {
+				return nil
+			}
+			return s.runPlugin(ctx, project, serv, "stop")


+`docker compose start` or `docker compose up` can resume it. Any `setenv` JSON message returned during `stop` is ignored,
+since dependent services are also stopping.


The previous implementation fetched the provider binary path and metadata twice per service during `compose stop`: once in stop.go to gate on the `stop` capability, and again inside runPlugin via setupPluginCommand. setupPluginCommand now signals "skip" by returning (nil, nil) when the requested command is absent from the provider's metadata. stop.go calls runPlugin directly; the skip-when-unadvertised check moves into runPlugin. Addresses PR #13779 review feedback. Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>

glours · 2026-05-07T16:58:15Z

/review

docker-agent

Assessment: 🟡 NEEDS ATTENTION

This PR adds a stop lifecycle hook for external provider plugins. The core design is solid — providers that do not advertise a stop block in their metadata are silently skipped (preserving backward compatibility), the runPlugin early-return after executing stop correctly avoids injecting environment variables that are only relevant to up, and the unit/e2e test coverage is appropriate.

One medium-severity issue was found in the fallback project-derivation path for provider-only stacks when options.Project is nil.

The previous implementation fetched the provider binary path and metadata twice per service during `compose stop`: once in stop.go to gate on the `stop` capability, and again inside runPlugin via setupPluginCommand. setupPluginCommand now signals "skip" by returning (nil, nil) when the requested command is absent from the provider's metadata. stop.go calls runPlugin directly; the skip-when-unadvertised check moves into runPlugin. Addresses PR #13779 review feedback. Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>

glours · 2026-05-07T17:55:07Z

/review

docker-agent

Assessment: 🟡 NEEDS ATTENTION

A previous change extended IsEmpty() to include `Stop == nil`. For a provider that advertises only a `stop` block, this caused commandMetadataIsEmpty to be false in setupPluginCommand, which made the option-forwarding filter reject every key — silently dropping all provider options on `up` and `down`. The Stop-presence signal lives independently in stop.go and the "stop" case of setupPluginCommand, so reverting the IsEmpty check to its original semantics is sufficient. Addresses PR #13779 review feedback. Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>

glours · 2026-05-07T20:28:52Z

/review

docker-agent

Assessment: 🟢 APPROVE

Reviewed feat: add stop lifecycle hook for external providers (PR #13779).

Summary: The implementation is clean and correct. The stop lifecycle hook is well-integrated into the existing provider plugin infrastructure with proper backward compatibility.

What was reviewed:

pkg/compose/stop.go: Provider-backed services are now correctly routed to runPlugin(..., "stop") instead of stopContainers. The guard is correctly placed before the container stop path.
pkg/compose/plugins.go: The setupPluginCommand returns (nil, nil) when a provider's metadata has no stop block, and runPlugin guards against a nil cmd with an early return — clean opt-in design preserving backward compatibility.
executePlugin: Correctly fires stoppingEvent/stoppedEvent for the stop case, and early-returns before the setenv variable propagation (which is intentionally skipped for stop, as documented).
plugins_test.go: Good coverage of IsEmpty() edge cases and JSON unmarshaling of the new Stop field, including the {"stop":{"parameters":null}} corner case.
providers_test.go: TestProviderStopHook uses a sentinel file to verify the provider binary's stop subcommand is invoked end-to-end.
docs/examples/provider.go: Example provider correctly advertises stop in metadata and uses PROVIDER_STOP_MARKER env var for test verification.
docs/extension.md: Documentation accurately describes the opt-in nature, the ignored setenv messages during stop, and the --timeout flag scope limitation.

No bugs, race conditions, or error-handling issues found.

glours requested a review from a team as a code owner May 7, 2026 16:41

glours requested review from Copilot and ndeloof May 7, 2026 16:41

Copilot started reviewing on behalf of glours May 7, 2026 16:42 View session

docker-agent Bot reviewed May 7, 2026

View reviewed changes

Comment thread pkg/compose/stop.go Outdated

Copilot AI reviewed May 7, 2026

View reviewed changes

glours self-assigned this May 7, 2026

docker-agent Bot reviewed May 7, 2026

View reviewed changes

Comment thread pkg/compose/stop.go

glours force-pushed the feat/provider-stop-hook branch from a9cc6bb to 38a12cf Compare May 7, 2026 17:49

docker-agent Bot reviewed May 7, 2026

View reviewed changes

Comment thread pkg/compose/plugins.go Outdated

docker-agent Bot reviewed May 7, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add stop lifecycle hook for external providers#13779

feat: add stop lifecycle hook for external providers#13779
glours wants to merge 3 commits intomainfrom
feat/provider-stop-hook

glours commented May 7, 2026

Uh oh!

docker-agent Bot left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

glours commented May 7, 2026

Uh oh!

docker-agent Bot left a comment

Uh oh!

Uh oh!

glours commented May 7, 2026

Uh oh!

docker-agent Bot left a comment

Uh oh!

Uh oh!

glours commented May 7, 2026

Uh oh!

docker-agent Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		`docker compose start` or `docker compose up` can resume it. Any `setenv` JSON message returned during `stop` is ignored,
		since dependent services are also stopping.

Conversation

glours commented May 7, 2026

Uh oh!

docker-agent Bot left a comment

Choose a reason for hiding this comment

Assessment: 🟡 NEEDS ATTENTION

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

glours commented May 7, 2026

Uh oh!

docker-agent Bot left a comment

Choose a reason for hiding this comment

Assessment: 🟡 NEEDS ATTENTION

Uh oh!

Uh oh!

glours commented May 7, 2026

Uh oh!

docker-agent Bot left a comment

Choose a reason for hiding this comment

Assessment: 🟡 NEEDS ATTENTION

Uh oh!

Uh oh!

glours commented May 7, 2026

Uh oh!

docker-agent Bot left a comment

Choose a reason for hiding this comment

Assessment: 🟢 APPROVE

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants