Handle already-running containers and port conflicts

[carole-lavillonniere]

Before pulling images or validating licenses, lstk start now checks the state of each configured container:

• If a container is already running, it logs "<name> is already running" and skips it. If all containers are already running, the command exits successfully without doing any work.
• If the configured port is already bound by another process, it returns an error "port <N> already in use" with a hint pointing to the config file.

[carole-lavillonniere]

@coderabbitai review

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors the container startup flow into a staged pipeline (select, pull images, validate licenses, start) with new port availability validation, while consolidating Docker test helpers into a shared test infrastructure module.

Changes

Cohort / File(s)	Summary
Container Startup Pipeline internal/container/start.go	Replaces sequential startup with staged pipeline: selectContainersToStart (filters running containers, checks port availability), pullImages (pre-pulls container images), validateLicenses (license validation), and startContainers (starts filtered containers). Shifts from config-based field references to container-scoped naming (Name, Image, Port, HealthPath). Introduces checkPortAvailable helper for TCP port collision detection.
Test Infrastructure Consolidation test/integration/main_test.go	Adds shared test scaffolding: containerName and testImage constants, and startTestContainer helper function to manage Docker container lifecycle for integration tests.
Integration Test Coverage test/integration/start_test.go, test/integration/license_test.go, test/integration/stop_test.go	Adds two new test cases to start_test.go (already-running and port-in-use scenarios); consolidates test utilities by removing testImage, startTestContainer, and containerName from license_test.go and stop_test.go to reference shared helpers from main_test.go.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main changes: handling already-running containers and detecting port conflicts, which are the primary features added to the lstk start command.
Description check	✅ Passed	The description clearly explains the key behavioral changes to the lstk start command, covering already-running container detection, appropriate logging, and port conflict handling.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch carole/drg-357

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/container/start.go`:
- Around line 57-75: The token is currently retrieved before
selectContainersToStart which prevents early-return when no containers need
starting; move the token retrieval to after selectContainersToStart and the
len(containers)==0 check so lstk can fast-exit without failing on missing token.
Update the flow in start.go: call selectContainersToStart(...) first, if
len(containers)==0 return nil, then fetch the token (the existing token
retrieval logic), then ensure the filtered containers (the returned containers
value) are injected/associated with the token before calling pullImages(...),
validateLicenses(..., token) and startContainers(...). Adjust any code that
passes cfg.Containers or containers to validateLicenses to use the token-aware
container set.

[silv-io]

LGTM! Just some thoughts that popped up

[silv-io]

thought: that's a long line. Wild that gofmt doesn't split it up :) Maybe gofumpt would?

[silv-io]

thought: I wonder if we could somehow compose those together so that we don't have to handle them separately, like having runtime.ContainerConfig contain config.ContainerConfig, somewhat like this:

type ContainerConfig struct {
	Image      string
	Name       string
	Port       string
	HealthPath string
	Env        []string // e.g., ["KEY=value", "FOO=bar"]
	*config.ContainerConfig
}

Not sure if it wouldn't make things annoying somewhere else though :)

[silv-io]

thought: I wonder if it makes sense to have that somewhere in a util/x, but we might only need it here.