fix: detect externally stopped and removed containers in up monitor

[glours]

What I did
Since 2.39.3 the monitor relies solely on die events to stop tracking containers, but a container stopped while in restart backoff emits only stop and destroy events, so an attached up hangs forever after an external stop/down. Handle destroy as terminal, and on stop inspect the container to distinguish a definitive stop from the transient one emitted during a ContainerRestart, which keeps watch sync+restart (#13161) working.

Related issue
Fixes #13985

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

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

Two medium-confidence findings introduced by this PR's new ActionStop handling — both relate to how the monitor responds when ContainerInspect does or doesn't find the container.

Lower-confidence findings (not posted inline)

• [low] pkg/compose/monitor.go:122 — ActionStart handler does not clear the restarting set (confidence: weak 10/100) — not a real bug given the current code flow; noted for future maintainability

[codecov]

Codecov Report

❌ Patch coverage is 80.76923% with 5 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pkg/compose/monitor.go	80.76%	2 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

[docker-agent]

Assessment: 🟢 APPROVE

The fix is well-structured and correct. The new ActionStop handler correctly distinguishes a definitive stop from a transient one by inspecting the container state, and ActionDestroy unconditionally terminates tracking without requiring an inspect (appropriate since there is nothing left to inspect). The isRestarting helper cleanly handles the NotFound case (container already removed) by returning false, nil, which is the right terminal disposition. The test suite covers the four key scenarios: destroy-only, die+stop canceling a restart backoff (#13985 repro), die+stop+start across a ContainerRestart cycle (#13161 guard), and stop racing a removal (inspect returns NotFound).

[ndeloof]

Considering the impact event management had in the past 😓 I'd like this new corner case is covered by an e2e test, based on reproduction scenario documented by #13985

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

This PR correctly fixes the core #13985 issue — a container in restart backoff that is externally stopped no longer hangs up forever. Two semantic edge cases in the new event-handling logic are worth reviewing:

1. The final event consumers receive in the stop-of-backoff scenario carries Restarting: true with no corrective follow-up
2. A stop-before-die race on a normal container stop can leave a stale entry in the restarting set

[docker-agent]

[medium] Final event consumers receive after external stop-of-backoff has Restarting: true, with no corrective follow-up

In the #13985 fix scenario (container in restart backoff, externally stopped), the event sequence is:

1. die → onContainerDie: isRestarting returns true → emits ContainerEventExited{Restarting: true}, keeps container tracked
2. stop → onContainerStop: isRestarting returns false → calls restarting.Remove + containers.Remove, emits no event, monitor loop ends

The only ContainerEventExited consumers ever see for this scenario carries Restarting: true. No follow-up event corrects it to Restarting: false. The code comment ("the exit was already reported to listeners by the preceding die event") is accurate, but that prior event's flag was set optimistically at die-time and is now stale.

The test TestMonitorExitsWhenRestartingContainerStopped encodes this shape (got[0].Restarting == true), which means callers that check the flag to decide whether to wait for a restart may behave incorrectly — they see "exiting, restart expected" and then nothing. For callers that rely solely on monitor termination (not on the event flag) to detect the definitive stop this is harmless, but it is a semantic mismatch worth noting.

Fix: pass the stop-event timestamp into onContainerStop and emit a corrective ContainerEventExited{Restarting: false} from the definitive-stop branch before evicting the container:

// in the else branch of onContainerStop:
c.notify(newContainerEvent(stopTimeNano, ctr, api.ContainerEventExited))
restarting.Remove(ctr.ID)
containers.Remove(ctr.ID)

This also means the test assertion should flip to got[1].Restarting == false (two events: the die-time Restarting: true and the stop-time Restarting: false).

Confidence	Score
🟡 moderate	67/100

[docker-agent]

[low] onContainerDie's non-restart path leaves a stale restarting entry when onContainerStop ran first

The new onContainerStop handler (added by this PR) can add a container to restarting when isRestarting returns true (i.e., State.Running == true, mid-exit during a normal docker stop). If this stop event arrives before the corresponding die event:

1. stop → onContainerStop: Running=true seen → restarting.Add(ctr.ID)
2. die → onContainerDie: Exited seen → willRestart=false → calls containers.Remove(ctr.ID) but NOT restarting.Remove(ctr.ID)

The container is removed from containers (so the monitor eventually terminates), but the restarting set retains a stale entry. In the current session this is harmless — once the container leaves containers, no future start event for it is expected. However if the container is externally re-created with the same ID before the monitor session ends, onContainerStart would see restarting.Has(ctr.ID) == true and emit ContainerEventStarted{Restarting: true} incorrectly (suggesting it is recovering from a monitored crash rather than being a fresh start).

Fix: add restarting.Remove(ctr.ID) to onContainerDie's non-restart branch (alongside the existing containers.Remove):

// in onContainerDie, willRestart=false branch:
c.notify(newContainerEvent(event.TimeNano, ctr, api.ContainerEventExited))
restarting.Remove(ctr.ID)   // clean up any entry added by a preceding stop event
containers.Remove(ctr.ID)
return nil

This is a defensive cleanup that makes the two paths (stop-first and die-first) symmetric.

Confidence	Score
🟡 moderate	57/100