[jussielo-amd]

Summary

apps/rocm/src/services_text.rs is named for rendering (the logs browser, the rocm services status text), but about a third of its production code stops and restarts managed local servers: the sandbox-tool runner, the stop path with its verified process termination, and the restart path with its record bookkeeping. Anyone looking for how rocm services stop|restart actually works has to find it inside a text-rendering module of about 1,570 production lines. This PR moves that process-control cluster into its own apps/rocm/src/service_control.rs. It is a pure move: no behaviour changes, no renames, and every moved line is byte-identical. This is part of the Phase 8.2-G follow-up work.

This is part of the ongoing apps/rocm module split; further moves follow.

The importers switch to the new path instead of going through a services_text re-export, because a re-export would make services_text and service_control import each other.

What moved

Everything below went from services_text.rs to service_control.rs, with its cfg gates intact (cfg(windows), cfg(not(windows)), cfg(feature = "e2e-test-hooks"), and cfg(unix) on the process tests).

Symbol	Destination
run_internal_sandbox_tool (pub(crate))	service_control.rs
MANAGED_STOP_GRACE, terminate_recorded_service_pids	service_control.rs
RecordedPidOutcome, RecordedStopTermination (private types the stop path returns; not in the ticket's list, but the moved functions cannot be separated from them)	service_control.rs
scripted_stop_outcome (both e2e-test-hooks variants)	service_control.rs
stop_internal_managed_service, stop_internal_managed_service_with, stop_internal_managed_service_confirmed_with, unload_lemonade_service_model	service_control.rs
apply_restart_runtime_pin, load_service_for_restart, restart_child_argv	service_control.rs
restart_internal_managed_service (pub(crate)), restart_internal_managed_service_with	service_control.rs
update_restarting_record, record_restarted_child, record_restart_readiness	service_control.rs
26 tests that drive those functions (the lemonade unload tests, the restart refusal and pin tests, the stop marker/record/key tests, the services_stop_* report tests, the managed_stop_* process tests) and their helpers	service_control.rs mod tests

Importers now use crate::service_control::... directly: main.rs and services_cmd.rs and service_launch.rs (run_internal_sandbox_tool), and runtime_services.rs (restart_internal_managed_service). The ticket named three importers; services_cmd.rs is a fourth that the earlier services-family split created.

What stays in services_text.rs: the logs browser, status rendering, render_service_action_result, render_unconfirmed_service_stop, read_managed_service_manifest, load_managed_service, validate_service_id, render_internal_status_text, and also internal_stop_server_value, stop_report_confirmed and service_stop_unconfirmed. The last three are not in the ticket's list; I left them with the renderer that reads the same stop_unconfirmed status rather than widen the move, and service_control.rs imports internal_stop_server_value from there. Two items are widened from private to pub(crate) because the new module reaches them: read_managed_service_manifest, and process_check_command (used only by one moved test). Nothing else changed visibility.

Documentation updated in the same commit: docs/architecture.md (a new entry for service_control.rs, the services_text.rs entry narrowed, and the entries for engine_runtime_select.rs, gpu_policy.rs, engines_cmd.rs, service_launch.rs and services_cmd.rs that named services_text.rs as the caller of moved code), the //! docs of services_text.rs, engine_runtime_select.rs, gpu_policy.rs and services_cmd.rs, two doc comments in services_text.rs that linked to moved functions, and the comment in main.rs's use block that named services_text.rs as home of restart_internal_managed_service. A grep of docs, AGENTS.md, CONTRIBUTING.md, xtask, tests, .github and .pre-commit-config.yaml found no path pin (in xtask/src/ascii_output_contract.rs, advised_commands.rs or elsewhere) that names the moved code.

Verification

Ran, on Linux (-j 4, private target dir):

• cargo fmt --all --check: clean.
• cargo clippy -p rocm --all-targets -- -D warnings: clean. Also clean with --features e2e-test-hooks, which compiles the scripted_stop_outcome hook variant.
• cargo test -p rocm --no-fail-fast: 1,473 passed, 0 failed, 8 ignored across all rocm test binaries.
• Leaf-name test-list diff of cargo test -p rocm -- --list and -- --list --ignored, origin/main against this branch: empty for both (1,480 and 8 names; module paths change for the moved tests, leaf names do not).
• Pure-move check: every line of the old services_text.rs appears in the new services_text.rs or service_control.rs except doc-comment and import lines and the two pub(crate) widenings listed above.
• cargo test -p rocm advised_commands, cargo xtask manifest --check, cargo xtask check-crate-edges, cargo xtask lychee and cargo test -p xtask: all pass (advised_commands 20 passed and 1 ignored; xtask 398 passed).
• Leak scan of git diff origin/main...HEAD: every hit is an existing identifier (internal_* function names, Rust private visibility) or a pre-existing sentence in docs/architecture.md; no internal names, hosts, trackers or addresses.

Not run:

• Windows clippy. cargo clippy -p rocm --target x86_64-pc-windows-msvc cannot finish on this host: the ring and aws-lc-sys build scripts fail first (no lib.exe, MSVC C headers missing), before rocm is reached. The cfg(windows) code moved verbatim (the restart's Windows spawn block and its two Windows-only imports), and the Windows lane's "Clippy (Windows targets)" step in windows-debug is the check for it; please treat that lane as the authority for this PR.
• cargo xtask e2e, scripts/smoke_local.py and the full workspace gate (CI carries them).
• New-test mutation checks: not applicable, no test was added or edited.

User-visible behaviour

None. This is a pure move of internal process-control code; nothing a user of the CLI can observe changes (output, exit codes, files, engine selection), so no new scenario is needed. The existing scenarios that exercise stop and restart keep covering it: service-stop-yes-stops-a-running-service, service-stop-confirms-a-stop-and-cleans-up, service-stop-unconfirmed-fails-and-keeps-the-record, service-stop-unverified-fails-without-a-retry and service-stop-unconfirmed-keyless-keeps-only-the-record (stop; the unconfirmed ones use the e2e-test-hooks ROCM_E2E_STOP_OUTCOME seam that moved here), service-unknown-stop and service-unknown-restart (the shared not-found error through both paths), and runtime-lifecycle-failed-service-restart-is-reported (restart; runtime-lifecycle-activate-restart-services-succeeds needs a GPU and runs on the GPU lane). All of these run on the mock lane on every PR except the GPU-gated one. The Windows lane runs only the @lifecycle scenarios plus a short list of named ones, so it does not exercise stop or restart end to end; on Windows the moved code is covered by its clippy step and the workspace unit tests (the managed_stop_* process tests are cfg(unix)).

Notes for reviewers

Reading order: service_control.rs first (the module //! doc lists what it owns and who calls it), then the shrunk services_text.rs, then the one-line import changes in main.rs, services_cmd.rs, service_launch.rs and runtime_services.rs, then docs/architecture.md.

Import cycles. The ticket expected this move to carry the existing services_text <-> service_launch cycle over to service_control <-> service_launch, neither adding nor removing one. On current main that is not quite what happens, and I would rather say so than let the PR text claim it. service_launch.rs imports run_internal_sandbox_tool from the new module and the new module imports ensure_public_service_has_endpoint_key and the spawn helpers from service_launch.rs, so service_control <-> service_launch is a pair. services_text.rs also still reaches crate::service_launch::background_helper_note (in render_service_action_result_with_helper_note, added after the audit) and service_launch.rs still imports render_service_action_result, read_optional_tail_lines and others from services_text.rs, so that pair remains too. Sibling pairs that import each other go from one to two. Breaking either is a structure change, not a move, and stays out of scope. There is no cycle between service_control and services_text: service_control reads records and internal_stop_server_value from services_text, and services_text imports nothing from service_control outside its tests.

The new module's use crate:: edges (production): endpoint_keys; engine_ipc (builtin_engine_serve_http_args, engine_request); engine_runtime_select (http_post_local_service_json, managed_service_launcher_path, parse_engine_recipe_json_arg, wait_for_service_http_ready, plus apply_app_path_env off Windows and app_path_env_var_refs/app_path_env_var_values on Windows); engines_cmd (env_root_for_service); gpu_policy (parse_device_policy); managed_service_status (load_managed_services); service_launch (ensure_public_service_has_endpoint_key, plus attach_background_stdio, detach_background_command, managed_engine_startup_failure_detail and managed_service_process_command off Windows); services_text (internal_stop_server_value, load_managed_service, read_managed_service_manifest); and the crate root (SandboxToolArg, status_for_readiness). Test-only edges: runtime_services, services_cmd (finish_service_action), services_text::tests::assert_no_retry_line, services_text (process_check_command, render_service_action_result) and crate::tests fixtures. check-crate-edges covers crate-level edges only, so none of this is checked mechanically.

Test assignment. The services_stop_* tests assert both the rendered unconfirmed-stop text and the record, key and audit state on disk, through finish_service_action. I moved them with the stop path because each drives stop_internal_managed_service_with with a stand-in termination, which needs the private RecordedStopTermination types; keeping them in services_text.rs would have widened those types and their fields to pub(crate). assert_no_retry_line stays in services_text::tests because service_launch.rs's tests import it there. service_commands_report_an_unknown_id_without_an_os_cause stays in services_text.rs: its subject is load_managed_service's not-found error, and it merely calls the moved runner.

The ticket's line numbers and some assumptions were stale: services_text.rs is about 1,573 lines before its test module on main (885 after this PR), and the moved production code is about 650 lines including doc comments and blanks (465 non-comment code lines in the new file) rather than 565.

Open PRs that touch the same ground. Four open PRs edit the moved functions and will need their hunks re-applied onto service_control.rs (git cannot follow the move): #859 (restart_internal_managed_service_with, record_restarted_child, record_restart_readiness), #854 (same functions), #299 (run_internal_sandbox_tool, stop_internal_managed_service, stop_internal_managed_service_with, MANAGED_STOP_GRACE) and #243 (stop_internal_managed_service, restart_internal_managed_service and _with). #889 (the clap command tree move into cli.rs, a proposal) edits main.rs, runtime_services.rs, service_launch.rs, services_cmd.rs and docs/architecture.md, including the service_launch.rs and services_cmd.rs entries this PR also edits; expect import-line and docs/architecture.md conflicts that are rebase-only. #883, #857, #723, #678, #563, #437, #631 and #888 touch services_text.rs outside the moved production code (rendering, and test-module hunks that may need to follow a moved test); #891 and #892 edit the main.rs use block and docs/architecture.md only.

[Copilot]

🟢 Approval recommended

No active review issues remain for the process-control module extraction.

No open issues 🎉

What changed in this PR

Separates managed-service process control from text rendering by moving the stop/restart implementation into service_control.rs, continuing the CLI’s module split without intended user-visible changes.

Changes:

• Relocates process-control helpers and their 26 tests, preserving platform and feature gates.
• Updates callers to import directly from service_control.
• Revises module and architecture documentation to reflect the new ownership.

File	Description
docs/​architecture.md	Documents the process-control module and revised dependencies.
apps/​rocm/​src/​services_text.rs	Removes process control and its tests; retains rendering and record helpers.
apps/​rocm/​src/​services_cmd.rs	Imports the sandbox runner from service_control and updates module docs.
apps/​rocm/​src/​service_launch.rs	Redirects the sandbox-runner import.
apps/​rocm/​src/​runtime_services.rs	Imports restart control from its new module.
apps/​rocm/​src/​main.rs	Registers service_control and updates the runner import.
apps/​rocm/​src/​gpu_policy.rs	Updates documentation naming the restart helper’s module.
apps/​rocm/​src/​engine_runtime_select.rs	Updates documentation identifying process-control callers.

_{Review effort: Balanced}

[juhovainio]

Reviewed the diff and found nothing blocking. The move looks behavior-preserving: the relocated code matches what was removed, apart from imports, the mod service_control; declaration, the tests wrapper, and the two pub(crate) widenings (process_check_command, read_managed_service_manifest), both needed and called out in the description. cfg gates and tests travel with the code.

Non-blocking notes:

• The branch currently conflicts with main and needs a rebase; the other open PRs listed in the description will need re-applying afterwards, so merge ordering matters.
• The two Strix Halo Ubuntu E2E lanes (release and nightly) were red when I looked, and MI300X/W7900 were still pending. I couldn't see the logs, so please confirm those are hardware-lane flakes (and not something this PR causes) before merging.
• Please cite the tracking ticket key in the description.

This is a diff-only read; I did not build or run the tests locally.

[jussielo-amd]

Thanks for the approval. Merged main into the branch (4992adf and a198b3f: plain merge commits, no history rewrite, so your approval stays attached to the existing commits). Conflicts were imports, a module doc and docs/architecture.md entries only: main's shell_quote hint changes in services_text.rs and runtime_services.rs, and the chat/planner cycle break (#888) merged cleanly or were taken from main's side, and nothing main changed touched code that this PR moved into service_control.rs. cargo test -p rocm --bin rocm passes (1499), clippy, fmt, manifest --check, check-crate-edges and lychee are clean, and the same 26 moved tests are present. The two Strix Halo Ubuntu lanes that were red are the known serve-40 xfail issue (#898), unrelated to this move. On the ticket key: I leave it out of the description on purpose, since naming it in a merged PR body closes the ticket, and this move is one of several that continue the split; the body now says it is part of the ongoing apps/rocm module split.