Cyclic service-dependency detection in service.start #63

New issue

Open

opened 2026-04-29 04:22:26 +00:00 by despiegk · 0 comments

despiegk commented

2026-04-29 04:22:26 +00:00

Owner

Context

When a user creates services with circular requires (A→B→A), the supervisor should refuse to start them with a clear error. Today there is no cycle check: starting either side of the cycle leaves the system in an unspecified state.

A test for this already exists but is #[ignore]d:

tests/integration/tests/dependencies.rs:91 — test_circular_dependency_error — #[ignore = "cyclic dependency detection not yet implemented"]

Acceptance

service.start (and probably service.set or both — see Notes) returns an Err when the dependency graph reachable from the target service contains a cycle.
Error message names the cycle: e.g. cyclic dependency: svc-a -> svc-b -> svc-a.
Existing test test_circular_dependency_error is un-ignored and passes.
No regression in test_chain_starts, test_conflict_blocking, or other dependency tests.

Notes / scope

Cycle detection should be cheap — DFS with a visiting set, returns the cycle path on hit. Implement once and call from the supervisor's start-with-deps path.
Decide between checking at service.set (refuse the bad config up front) vs service.start (allow temporarily-broken graphs and only refuse when actually starting). The existing test only asserts service.start fails, so start-time is the minimum bar.
The other 20 ignored tests are unrelated (env-gated bare-metal/stress) and stay ignored.

Follow-up to #56 — found while auditing why 21 tests are skipped.

## Context When a user creates services with circular `requires` (A→B→A), the supervisor should refuse to start them with a clear error. Today there is no cycle check: starting either side of the cycle leaves the system in an unspecified state. A test for this already exists but is `#[ignore]`d: - `tests/integration/tests/dependencies.rs:91` — `test_circular_dependency_error` — `#[ignore = "cyclic dependency detection not yet implemented"]` ## Acceptance - `service.start` (and probably `service.set` or both — see Notes) returns an `Err` when the dependency graph reachable from the target service contains a cycle. - Error message names the cycle: e.g. `cyclic dependency: svc-a -> svc-b -> svc-a`. - Existing test `test_circular_dependency_error` is un-ignored and passes. - No regression in `test_chain_starts`, `test_conflict_blocking`, or other dependency tests. ## Notes / scope - Cycle detection should be cheap — DFS with a visiting set, returns the cycle path on hit. Implement once and call from the supervisor's start-with-deps path. - Decide between checking at `service.set` (refuse the bad config up front) vs `service.start` (allow temporarily-broken graphs and only refuse when actually starting). The existing test only asserts `service.start` fails, so start-time is the minimum bar. - The other 20 ignored tests are unrelated (env-gated bare-metal/stress) and stay ignored. Follow-up to #56 — found while auditing why 21 tests are skipped.