fix: derive agent phase from container status in runtime List methods

[meatballs]

Summary

• Podman, Docker, and Apple container runtimes hardcode Phase: "created" for all containers in List()
• When agent-info.json loses its phase field (e.g. harness overwrites it with only the activity), the heartbeat sends created to the hub, reverting running agents
• Derive phase from container status instead: Up ... → running, Exited ... → stopped, else created
• Matches the hub's own legacy derivation logic at pkg/hub/handlers.go:5280-5293

Fixes #123

Test plan

• Start an agent, verify hub shows running
• Interact with agent terminal (triggering harness to overwrite agent-info.json)
• Verify phase stays running through subsequent heartbeats
• Stop agent, verify phase becomes stopped

[ptone]

PR #124 Review: fix: derive agent phase from container status in runtime List methods

Executive Summary

Low risk. This is a small, well-scoped bug fix that replaces hardcoded "created" phase values with a helper function that derives phase from container status strings. The logic exactly mirrors the hub's existing legacy derivation at pkg/hub/handlers.go:5280-5293, reducing the chance of state regression during heartbeats.

---

Critical Issues

None.

---

Observations

1. Slight behavioral divergence from hub logic (Minor)

Location: pkg/runtime/common.go:748-758

The hub's legacy path (handlers.go:5286-5292) treats containerStatusLower == "created" specially — it maps to PhaseProvisioning (not "created") and includes a guard to avoid downgrading a running agent. The new phaseFromContainerStatus function maps the "created" status string to the "created" phase via the default branch.

This is likely intentional and correct for the List() context: a container that genuinely reports "created" status hasn't started yet, so "created" phase is appropriate. The hub-side guard against downgrading is a server-side concern. However, it's worth noting the difference explicitly, especially if the hub ever starts trusting the client-reported phase without its own derivation.

Severity: Informational — no action needed.

2. No unit tests for phaseFromContainerStatus

The function is pure and deterministic — an ideal candidate for a small table-driven test covering edge cases like:

• "Up 5 minutes" → "running"
• "running" → "running"
• "Exited (0) 3 hours ago" → "stopped"
• "stopped" → "stopped"
• "Created" → "created"
• "" (empty string) → "created"
• "Paused" → "created" (Docker supports paused containers)
• "Restarting" → "created" (Docker restarting state)

Severity: Low — the function is simple and the logic is proven by the hub's existing equivalent, but a test would add confidence for future modifications.

3. "Paused" and "Restarting" container states fall to default

Docker containers can be in Paused or Restarting states. Both will map to "created" via the default branch. For Restarting, this seems reasonable. For Paused, the container is technically still running (just suspended) — mapping to "created" could be surprising, though it's unlikely to occur in practice for Scion agents.

Severity: Informational.

---

Positive Feedback

• Consistency with existing patterns: The function at line 249 of apple_container.go already uses the exact same strings.HasPrefix(status, "up") && status != "running" pattern for status checking, confirming the status string formats are well-established.
• Clean separation: Extracting the logic into a shared phaseFromContainerStatus function in common.go avoids duplication across three runtime implementations and centralizes the mapping for future updates.
• Good documentation: The function comment clearly explains the purpose and the status string formats it handles.
• Minimal diff: The change is surgical — 17 lines added, 3 lines modified, no unnecessary refactoring.

---

Final Verdict

Approve. The fix is correct, minimal, and directly addresses the bug described in #123. The logic mirrors the hub's proven derivation. The only suggestion is adding a table-driven unit test for phaseFromContainerStatus to guard against regressions, but this is not blocking.