fix(events): update container health status on all transitions

[amir20]

Summary

• Container health icon was stuck on "starting" because the starting → healthy transition event was never handled.
• Docker emits health events with actions like health_status: healthy, health_status: starting, health_status: unhealthy. The match only covered healthy/unhealthy, dropping the rest.
• The handler also tried to read actor.attributes["health_status"], which Docker doesn't populate, so even matched events failed to parse.

Fix

• Match any action starting with health_status and pass the action string to the handler.
• Parse the status directly from the action string (works with the existing FromStr substring matcher), falling back to attributes and then inspect_container.

Test plan

• cargo fmt --check
• cargo clippy
• cargo test
• Manually verify a container with a healthcheck transitions starting → healthy in the UI

🤖 Generated with Claude Code

[claude]

Code Review

Overview

This PR fixes a real bug: health status was stuck on "starting" because the match arm didn't include health_status: starting, and the attribute lookup (actor.attributes["health_status"]) was never populated by Docker anyway. The fix is correct and the approach is sound.

---

Issues

1. Minor: match health { Some(h) => Some(h), None => ... } can be simplified

// current
let health = match health {
    Some(h) => Some(h),
    None => { ... }
};

This is just Option::or_else:

let health = health.or_else(|| {
    if let Ok(inspect) = self.docker.inspect_container(...).await {
        ...
    } else {
        None
    }
});

2. Minor: format!("{:?}", status).parse().ok() is a fragile inspect fallback

The inspect fallback (unchanged from original) relies on bollard::models::HealthStatusEnum's Debug output containing the substrings "healthy" / "unhealthy" / "starting". If bollard ever changes its enum variant names or debug formatting, this silently breaks. Since the action string is now parsed first and the attributes fallback is second, this path is rarely hit — but it's worth a note. A match on the enum variants would be more robust.

3. Edge case: health_status: none not handled

Docker can emit health_status: none when a healthcheck is removed from a running container. The new starts_with("health_status") arm will match it, but "health_status: none".parse::<HealthStatus>() returns Err(()) (no None variant exists). This falls through to the inspect fallback, which may return stale state. The correct behavior would be to clear the health status from the container entirely (None). Low-priority since this is an unusual case, but worth tracking.

---

What looks good

• starts_with("health_status") is more robust and forward-compatible than the exhaustive string list.
• Parsing directly from the action string is the right primary strategy — the action like "health_status: healthy" directly encodes the new state.
• The HealthStatus::from_str substring matcher (contains("healthy") && !contains("unhealthy")) correctly handles the action format without any changes needed there.
• The attribute fallback is retained as a belt-and-suspenders measure without cost.
• Pre-commit checks (fmt, clippy, test) all pass per the PR description.

---

Verdict

The core fix is correct and an improvement. Items 1 and 2 are minor style/robustness issues; item 3 is a low-priority edge case. Good to merge as-is with optional follow-up on the edge case.

[claude]

Code Review

Overview

This PR fixes a real bug: Docker health events use actions like health_status: healthy (with a space and status suffix), not the bare health_status string. The old literal match missed all transitions, and even matched events tried to read actor.attributes["health_status"] which Docker does not actually populate.

What works well

• The starts_with("health_status") guard is the correct fix for the event matching.
• Parsing from the action string first is the right order of priority — the existing FromStr for HealthStatus uses substring matching (contains), so "health_status: healthy", "health_status: unhealthy", and "health_status: starting" all parse correctly.
• The three-level fallback (action string → attributes → inspect) is a sensible safety net.

Issues

Style: Some(h) => Some(h) is redundant

// current
let health = match health {
    Some(h) => Some(h),
    None => { ... }
};

// simpler
let health = match health {
    Some(_) => health,
    None => { ... }
};

Minor nit, but wrapping h in Some(h) reads as an identity transform and could confuse a reader.

No automated tests added

The bug is in handle_health_status_change, but there are no new unit tests verifying the parsing logic. Given the project already tests stats/log parsing, a test for the action-string parsing path would lock in the contract this fix depends on:

assert_eq!("health_status: healthy".parse(), Ok(HealthStatus::Healthy));
assert_eq!("health_status: unhealthy".parse(), Ok(HealthStatus::Unhealthy));
assert_eq!("health_status: starting".parse(), Ok(HealthStatus::Starting));

Manual verification unchecked

The test plan has the starting → healthy transition check unchecked. This is the core user-visible behavior being fixed — should be confirmed before merge.

Verdict

The fix is correct and the logic is sound. Two things before merging: confirm the manual test, and optionally add the unit test above to document the FromStr contract this change relies on.

[github-actions]

Docker Image Built Successfully

docker pull ghcr.io/amir20/dtop:pr-297