[DarknessKiller]

Description

Upstream yusing/main (0fc5a6a) already fixed the shared-dependency guard for watchers (hasRunningDependent) plus the concurrency rework (lifecycle gate, delayed-event reconciliation). This branch is rebased on top of it and closes the remaining gap.

Always-on routes were still broken. An always-on route (no idle timeout, container label proxy.depends_on: koito_backend:service_started) has no watcher in watcherMap, so hasRunningDependent never sees it. Production symptom (podman-compose, verified in logs): when multi_scrobbler (idle timeout 1m, depends on koito_backend) idled out, it stopped koito_backend even though the always-on route koito was still up, breaking that route.

The fix

hasRunningDependent now consults two sources:

return w.hasRunningWatcherDependent() || w.hasRunningRouteDependent()

• hasRunningWatcherDependent is the original watcherMap scan. The map lock is now released before the route scan runs, so provider lookups and route iteration no longer happen while holding it.
• hasRunningRouteDependent walks ep.IterRoutes and asks isNeededBy whether a route is up and declares this container as a dependency. It reads r.HealthMonitor() per route instead of calling GetHealthInfoSimple(), which allocated a map of every route keyed by route name on each stop.

Dependency edge resolution

findDependencyRoute is the single place that resolves a dependency entry, and NewWatcher calls it too, so both sides behave identically:

• the :service_started / :service_healthy condition suffix is stripped;
• a route that has container info resolves through FindService inside the same Compose project first;
• otherwise it falls back to the entrypoint route lookup by alias.

NewWatcher now resolves the entrypoint once instead of once per dependency, and reports a missing dependency instead of dereferencing a nil entrypoint.

Pinning rule

A route pins its dependency while it is not napping.

An unhealthy route still pins. The dependency may be exactly what brings the route back, and only a sleeping route is guaranteed to rebuild the dependency chain on its next wake. Unknown health — no health monitor configured, the common always-on case — pins as well.

Why the label survives route construction

proxy.depends_on is consumed by getDeleteLabel during route construction (internal/docker/container.go); Dependencies() reads ActualLabels (cloned before consumption), so the edge is still visible at stop time. That fallback is already exercised by TestIdlewatcherDependsOnLabel, which consumes the label and then reads Dependencies() through IdlewatcherConfig.DependsOn, so this branch does not add a separate regression test for it.

Delta on upstream: 3 files, +177/-15.

Test Plan

• go test -ldflags=-checklinkname=0 ./internal/...
• New tests in internal/idlewatcher/watcher_dependencies_test.go:
  • TestStopSharedDependencyKeptForAlwaysOnRoute — an idle route stops while an always-on route still depends on the same container through container labels. Subtests cover a bare service name, both condition suffixes, and unknown / unhealthy health. The shared dependency must stay up in every case.
  • TestStopSharedDependencyWhenAlwaysOnRouteNaps — a napping route must not pin its dependency.
  • The helper declares the edge through proxy.depends_on on the route's docker.Container, so the label path is exercised end to end.
• Both new tests are mutation-checked: dropping the route scan fails all five pin subtests, and disabling the napping release fails the nap test.

Test Result

• gofmt clean; go vet clean.
• go test -ldflags=-checklinkname=0 -race -count=2 ./internal/idlewatcher/ ./internal/docker/ ./internal/entrypoint/ — pass.
• go test -ldflags=-checklinkname=0 -count=1 ./internal/... — 48 packages pass. The only failures are 4 pre-existing ones in internal/config and internal/routeimpl, verified identical on clean upstream main (the webui dist is not built for the embedded-UI smoke tests, and the config tests are environment-dependent).

Links

• Upstream commit this builds on: 0fc5a6a (retain dependencies needed by running routes).

Summary by CodeRabbit

• Bug Fixes
  • Dependencies shared with always-on routes now remain running according to their container dependency settings, even when the route has no idle watcher.
  • Routes that are napping no longer keep shared dependencies running; unhealthy routes still do.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 2c19809c-eb9f-4106-b2da-993d8b6573b4

📥 Commits

Reviewing files that changed from the base of the PR and between 8d91e1f and 9572e55.

📒 Files selected for processing: 3

• internal/idlewatcher/README.md
• internal/idlewatcher/watcher.go
• internal/idlewatcher/watcher_dependencies_test.go

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

The idle watcher now resolves Docker dependency names through a shared helper and checks entrypoint routes when deciding whether a watched container has running dependents. It excludes napping routes from that check, while unhealthy routes remain eligible. Tests and README documentation cover the behavior.

Changes: Always-on route dependencies

Layer / File(s)	Summary
Resolve dependency names internal/idlewatcher/watcher.go	A shared helper resolves dependency names through the same Compose project first, then the entrypoint. NewWatcher uses this helper and still reports unresolved dependencies.
Scan always-on route dependencies internal/idlewatcher/watcher.go, internal/idlewatcher/watcher_dependencies_test.go, internal/idlewatcher/README.md	The dependent check scans entrypoint routes for Docker dependencies on the watched container. It skips napping routes but includes unhealthy routes. Tests cover dependency labels and health statuses; the README documents the behavior.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: yusing

Merge Risk: ⚪ Minimal · up to 9572e

The change keeps shared dependencies running for always-on routes that have no watcher. No actionable merge-blocking risk remains in the supplied review context.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: preserving dependencies required by always-on routes in the idle watcher.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

✨ Simplify code

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks each route at night,
The watcher keeps its links in sight.
If napping, dependencies may rest,
Unhealthy routes still pass the test.
The burrow hums; the code is right.}

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

[DarknessKiller]

Changes has been verified -

[DarknessKiller]

validated with multiple idle watcher cases.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[DarknessKiller]

Pushed 9572e557, which supersedes the three guard helpers from the previous commits (entrypointRouteDependsOn, routeDependsOnContainer, resolveDepRoute) with one shared resolver and a tighter scan. The net diff against main is the same shape as before — 3 files, +177/-15 — but the implementation and one behavior changed:

1. Dependency edges resolve in one place. findDependencyRoute now owns both the entry parsing and the route resolution, and NewWatcher calls the same function, so the guard and the watcher constructor can no longer drift apart. Previously the entrypoint lookup was duplicated.

2. The route scan no longer runs under watcherMapMu. The old hasRunningDependent used a deferred RUnlock, so GetHealthInfoSimple, route iteration and FindService all ran while the global watcher map read lock was held. The lock is now scoped to the watcher scan, which removes a lock-order dependency between watcherMapMu and each provider's route lock, and stops blocking watcher registration during the scan.

3. No health map allocation per stop. GetHealthInfoSimple() built a map of every route keyed by route name on every stop, and two routes sharing a name would collide in it. The guard reads r.HealthMonitor() for the route it is looking at instead.

4. Behavior change: an unhealthy route now pins its dependency. Previously both unhealthy and napping released the dependency. For unhealthy that is backwards — the dependency is often what brings the route back, so releasing it can strand a running container behind a permanently unhealthy route. Only napping releases now, because a sleeping route is the case that is guaranteed to rebuild the chain on its next wake. unknown (no health monitor, the usual always-on case) and healthy still pin, and the watcher path already behaved this way via other.running(), so the two paths now agree.

5. NewWatcher resolves the entrypoint once instead of once per dependency, and reports dependency %q not found instead of dereferencing a nil entrypoint when the context does not carry one.

6. Dropped TestDependenciesAfterLabelConsumption. TestIdlewatcherDependsOnLabel already covers it: loadDeleteIdlewatcherLabels calls getDeleteLabel(LabelDependsOn) (which deletes the label) before Dependencies(c), and then asserts IdlewatcherConfig.DependsOn. The ActualLabels fallback is therefore already under test.

Tests

TestStopSharedDependencyKeptForAlwaysOnRoute is table-driven over a bare service name, :service_started, :service_healthy, and unknown / unhealthy health; TestStopSharedDependencyWhenAlwaysOnRouteNaps covers the release. Both are mutation-checked: dropping the route scan fails all five pin subtests, and disabling the napping check fails the nap test. Full runs: gofmt/go vet clean, -race -count=2 on internal/idlewatcher, internal/docker and internal/entrypoint pass, and ./internal/... is 48 packages green with only the 4 pre-existing internal/config / internal/routeimpl failures that also reproduce on clean main.