[jussielo-amd]

Summary

Fixes ROCMAI-871. The managed-service modules in apps/rocm imported each other in production code: services_text.rs and service_launch.rs were a pair (the stop-result text needed the background-helper note from the launcher, and the launcher needed the stop-result renderers), and since the stop path moved into service_control.rs, service_launch.rs and service_control.rs were a second pair (the launcher took run_internal_sandbox_tool from the control module for the attached-serve Ctrl-C, and the control module took the endpoint-key guard and the spawn helpers from the launcher). The two pairs also closed a three-module loop, so none of the three could be read, tested or moved without the other two. This change makes the graph one-way with two small commits and no behaviour change.

The dependency order is now services_text.rs below service_launch.rs below service_control.rs, with services_cmd.rs, runtime_services.rs and serve_cmd.rs above them. Only two edges had to go. The endpoint-key guard and the spawn helpers stay where they are, because the control module is the natural caller of them and the launcher does not import it any more.

What moved

Item	From	To
background_helper_already_running, HelperNote, helper_note_for, background_helper_note (and their five tests plus the runtime_state and set_helper_facts fixtures)	service_launch.rs	new leaf background_helper.rs
Ctrl-C stop of an attached serve	service_launch.rs called service_control::run_internal_sandbox_tool	run_attached_service takes the stop as a parameter, serve_cmd.rs passes the same call

The first commit is a verbatim move. The leaf imports nothing from the service modules, so services_text.rs and uninstall_plan.rs read the note from it, and service_launch.rs reads the liveness check from it for its autostart decision (spawning the helper stays in service_launch.rs). The two fixtures became pub(crate) because the autostart and stop-output tests that stay in service_launch.rs use them too.

The second commit is not a move, and I could not make it one: run_internal_sandbox_tool calls the stop and restart paths that live in service_control.rs, so it cannot be pushed down below the launcher. Its one call in service_launch.rs is the Ctrl-C arm of an attached serve, so the dependency is cut there: run_attached_service gets a final stop_service: impl FnOnce(&AppPaths, &str) -> Result<serde_json::Value> argument, and serve_cmd.rs, which already sits above both modules and is the only caller, passes run_internal_sandbox_tool(paths, SandboxToolArg::StopServer, Some(service_id), true), the exact call that was made before. The stop runs at the same point with the same arguments and its answer still goes to finish_attached_stop unchanged.

Production import edges among the seven service modules, before and after (test-only code ignored; service_launch.rs -> service_hints.rs and services_text.rs -> service_hints.rs are unchanged and omitted below where nothing changed). Removed edges are marked.

Edge	Before	After
services_text to service_launch (background_helper_note)	yes	removed
service_launch to service_control (run_internal_sandbox_tool)	yes	removed
service_launch to services_text (render_service_action_result, render_service_action_result_with_helper_note, service_stop_unconfirmed, read_optional_tail_lines)	yes	yes
service_control to service_launch (ensure_public_service_has_endpoint_key, attach_background_stdio, detach_background_command, managed_engine_startup_failure_detail, managed_service_process_command)	yes	yes
service_control to services_text (internal_stop_server_value, load_managed_service, read_managed_service_manifest)	yes	yes
service_records to services_text (load_managed_service, validate_service_id)	yes	yes
services_cmd to service_control, service_records, service_hints, services_text	yes	yes
runtime_services to service_control (restart_internal_managed_service), services_text (load_managed_service)	yes	yes
serve_cmd to service_control (run_internal_sandbox_tool)	no	new, one-way
services_text, service_launch, uninstall_plan to background_helper	no	new, one-way

Before the change a script over the production use crate::... statements and inline crate::... paths of the seven files found three cycles: services_text and service_launch, service_launch and service_control, and services_text, service_launch, service_control. After it the same script reports none.

Verification

Ran on this branch: cargo fmt --all --check; cargo clippy -p rocm --all-targets -- -D warnings; cargo test -p rocm --bin rocm --no-fail-fast (1509 passed, 8 ignored); the sorted leaf-name test list from cargo test -p rocm --bin rocm -- --list is identical before and after (1517 tests, including the five tests that moved to background_helper.rs); cargo xtask manifest --check; cargo xtask check-crate-edges; the lychee hook on docs/architecture.md (runs in the commit hooks); the leak scan of the diff against the base (the only keyword hits are existing identifiers such as run_internal_sandbox_tool and Rust's "private types"); and the end-to-end scenarios that traverse the changed paths against a debug build with the test hooks (ROCM_CLI_BINARY and ROCM_CLI_ROCMD_BINARY pointing at it): service-stop-01 to service-stop-05, automations-11 (the helper exiting once the last watcher is off, which the note's text names) and the seven service-cleanup scenarios, all green.

Not run: the Windows type-check of this crate (cargo clippy -p rocm --target x86_64-pc-windows-msvc), which cannot build here because the C toolchain for aws-lc-sys is missing for that target. Nothing gated on cfg(windows) was edited and the moved items carry no cfg gates, so the risk is limited to import lists. The full workspace gate, scripts/smoke_local.py and the whole e2e suite are left to CI.

No test was added, so there is no new test to break. The one changed call site (the attached-serve Ctrl-C stop in run_attached_service) has no unit or scenario coverage before or after, because it needs an interactive terminal; finish_attached_stop, which handles the answer, keeps its existing tests. I checked the call by reading it against the removed one rather than by a test.

User-visible behaviour

None, so no new scenario. This is a structure change inside apps/rocm: no command output, exit code, file or engine selection changes. The scenarios listed above are the ones that already cover the stop path, the stop text and the helper note.

Notes for reviewers

Read the commits in order. The first is mechanical; the second is the only place a signature changes. docs/architecture.md is updated in each commit (a new entry for background_helper.rs, and the service_control.rs and service_launch.rs entries now describe the one-way edges instead of the cycles). Two of the touched lines still carry the older "Phase 6b" wording because I edited them in place rather than rewriting them.

Open pull requests that touch the same files: the one that moves the stop routine into rocm-core (#929) edits service_control.rs and the same long service_control.rs entry in docs/architecture.md, so expect a one-line conflict there that resolves by keeping both changes; it merges cleanly in the code. The endpoint-key guard pull request (#926) also edits service_launch.rs; its code hunks do not overlap this change, and it already conflicts with the current base in main.rs and services_text.rs independently of this change, since it predates the move of the stop path.

[Copilot]

🟢 Approval recommended

No active review issues are recorded for the managed-service dependency refactor.

No open issues 🎉

What changed in this PR

Removes circular dependencies among the managed-service modules in apps/rocm, establishing one-way dependencies without intended user-visible behavior changes.

Changes:

• Extracts background-helper liveness checks, notification text, and existing tests into background_helper.rs.
• Passes the attached-serve Ctrl-C stop operation from serve_cmd.rs into the launcher as a callback.
• Updates architecture documentation and module comments to describe the new dependency structure.

File	Description
docs/​architecture.md	Documents the helper module and one-way service dependencies.
apps/​rocm/​src/​uninstall_plan.rs	Reads helper warnings from the extracted module.
apps/​rocm/​src/​services_text.rs	Reads helper notes without depending on the launcher.
apps/​rocm/​src/​service_launch.rs	Uses extracted helper checks and accepts a stop callback.
apps/​rocm/​src/​service_control.rs	Updates comments describing callers and dependencies.
apps/​rocm/​src/​serve_cmd.rs	Supplies the existing managed-stop operation as a callback.
apps/​rocm/​src/​main.rs	Registers the new helper module.
apps/​rocm/​src/​background_helper.rs	Houses relocated helper logic, tests, and shared test fixtures.

_{Review effort: Balanced}

[jussielo-amd]

Superseded by #936: this pull request's commits are included in that branch (head a920d07), and #936's description covers this change. Closing so the review happens in one place.