fix(cli): detect orphaned containers on build, flag unusable healthchecks (G-014) - #409
Merged
Merged
Conversation
…ecks G-014: `nself build` only ever generated files — it never looked at the live daemon, so a service dropped from the generated compose left its container running forever with no service definition, no nginx vhost, and no traffic. Measured live: nself-claw/notify/mux/cron survived this way, two of them dead a full month before anyone noticed. - internal/docker: DetectOrphans lists containers scoped strictly to this project's compose-project label (never touches unrelated workloads on the host) and compares their service label against the union of services in the freshly generated compose files (base + plugin fragments). RemoveOrphans force-removes what's found. - cmd/commands: `nself build` now reports orphans by default and adds `--remove-orphans` to remove them (opt-in). `nself status` gets the same read-only detection, since a container can drift orphaned between builds. - internal/doctor: a second, compounding defect made these containers undetectable — their healthchecks ran a binary (curl) not installed in the image, so they reported "unhealthy" permanently regardless of whether the service worked, and `doctor --deep` suggested a useless `docker restart`. diagnoseUnhealthyContainer now confirms whether the healthcheck command actually exists in the image before trusting the status, and gives an honest fix (install the binary or change the test) instead of a restart that can never help. Regenerated .github/command-inventory.json for the new --remove-orphans flag; SPORT F02 refreshed to match.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes CLI gap G-014.
nself buildonly ever generated files — it never looked at the live Docker daemon, so a service dropped from the generated compose (removed fromnself.yaml, an uninstalled plugin, a rename) left its container running forever: no service definition, no nginx vhost, no traffic, and nothing reported it. Measured live on prod: four containers (nself-claw,nself-notify,nself-mux,nself-cron) were in this state; two had been dead a full month (a database DNS failure) before anyone noticed — because a second, compounding defect made their healthchecks permanently meaningless too (curlwas never installed in their image, so the healthcheck always errored and the container reported "unhealthy" whether or not it actually worked).1. Orphan detection + removal
internal/docker/orphans.go(new):DetectOrphanslists containers filtered strictly by this project'scom.docker.compose.projectlabel (the exact label Docker Compose stamps from the generated compose file'sname:field, i.e.cfg.ProjectName) and flags any whosecom.docker.compose.servicelabel isn't in the union of services from the freshly generated compose files (base + plugin fragments, read viabuild.ReadComposeManifest).RemoveOrphansforce-removes what's found via the samedocker rm -fprimitivecleanup.goalready uses for init/zombie containers.nself buildnow runs this detection by default and reports orphans prominently; a new--remove-orphansflag makes removal opt-in. Detection is best-effort — any docker-level failure (daemon unreachable, docker not installed) is logged at debug level and swallowed, so a build in a Docker-less CI image still succeeds.nself statusgets the same read-only detection (a container can drift orphaned between builds, not only during one) — it never removes anything itself.Scoping guarantee: every container is created by
docker composewithcom.docker.compose.projectset to the compose file's top-levelname:, which nSelf sets tocfg.ProjectName(internal/compose/generator.go).DetectOrphansfilters on that exact label server-side in thedocker pscall itself — a container from a different project (different label value) or one Docker didn't create via compose (no label) can never be selected. This mirrors the label filtercleanup.go'scleanupZombieContainersalready uses for the same reason.2. Healthcheck validity
internal/doctor/deep_docker_healthcheck.go(new): given a containerdoctor --deepalready found "unhealthy",diagnoseUnhealthyContainerinspects its healthcheckTestcommand, extracts the binary it would invoke, and confirms viadocker exec ... command -v <binary>whether that binary actually exists in the image. If it's confirmed absent, the check reports a distinct, honest message ("healthcheck command %q is not installed... status carries no information") with a real fix (install the binary or change the test), instead of the pre-existing genericdocker restartsuggestion that can never help this case. Any ambiguity (healthcheck unset, can't verify) falls back to the original generic message — this never downgrades a real failure.nself doctor --deep, notnself build.internal/buildis a pure generator — nothing in it touches the Docker daemon — and confirming a binary is on a container'sPATHrequires a live container to exec into, which only exists once something is running.doctor --deep'sDockerDeepChecksalready inspects live container health and is exactly where the false "unhealthy"/"docker restart" signal was produced.Other
.github/command-inventory.jsonfor the new--remove-orphansflag (verified viainternal/repoqa'sTestCommandInventoryIsCurrent); refreshed SPORTF02-COMMAND-INVENTORY.mdto match (unaffected content-wise — that table lists commands/subcommands, not flags — but timestamp/regeneration kept current).Test plan
go build ./...— cleango vet ./...— cleangolangci-lint run ./...— 0 issuesgo test ./...— 5154 passed, 97 packages, no regressionsinternal/docker/orphans_test.go:buildOrphanPsArgsscoping,parseOrphanPsOutputorphan/non-orphan/blank-label/malformed-line cases,ComposeServiceNamesunion/missing-file/invalid-YAML casesinternal/doctor/deep_docker_healthcheck_test.go:extractHealthcheckBinaryacross CMD/CMD-SHELL/NONE/malformed shapes,classifyBinaryProbefound/confirmed-missing/inconclusive cases