ROCMAI-859: split rocmd watchers.rs into a watchers/ directory by jussielo-amd · Pull Request #885 · ROCm/rocm-cli

jussielo-amd · 2026-11-04T15:02:06Z

Summary

apps/rocmd/src/watchers.rs had grown to 3,537 lines (about 1,575 of them production code, the rest one inline test module with 40 tests). It held the watcher registry, the event collector and dispatcher, and the handlers of all six built-in watchers with their policy, which made every change to one watcher a change to a file shared with all of the others, and put the file over the 1,500 production-line limit that the planned size gate will enforce. This PR splits it into a module root plus one file per watcher under apps/rocmd/src/watchers/, the same layout apps/rocm/src/comfyui.rs plus comfyui/ uses. It is a pure, behaviour-preserving move: function bodies are unchanged, and only visibility (pub(super), and only for what the root or a sibling file uses) and import paths differ. No user-visible behaviour changes.

Completes ROCMAI-859.

What moved

watchers.rs stays the module root and keeps the registry and the collector seam. Everything else moved, together with the tests that call it directly. The shared test builders moved first so that the later commits move tests without touching them.

Symbols	Destination
`test_watcher_snapshot`, `test_runtime_state`, `proposals_beyond_a_recent_window` (the last one is a helper two sibling test modules share)	`test_support.rs`
`WatcherPolicyAction`, `watcher_policy_action`, `queue_proposal`, `queue_proposal_with_arguments`, `proposal_tool_for_action`, `proposal_arguments_for_action`, `pending_proposal_exists`	`watchers/policy.rs`
the server-recover handler and its wrapper, `find_recoverable_service`, `stop_requested`, the manifest, endpoint and healthcheck recovery classification, `service_recovery_event_kind`, `display_recovery_reason`, `server_recover_due`, `pending_restart_proposal_exists`, the test-only `evaluate_server_recover` helper, the three backoff and health-timeout constants	`watchers/server_recover.rs`
`handle_gpu_metrics_event`, `gpu_metrics_due`, `gpu_snapshot_summary`, `gpu_pressure_events`, `gpu_pressure_event` and the amd-smi monitor-entry readers, `GPU_METRICS_INTERVAL_MS` and the three pressure thresholds	`watchers/gpu_metrics.rs`
`handle_gpu_thermal_protect_event`, `gpu_thermal_protect_due`, the pressure-target selection, `pending_stop_proposal_exists`	`watchers/gpu_thermal_protect.rs`
`handle_therock_update_event` and its runner variant, the restricted check-updates result parsing, the update-available notification, `therock_update_due`, `THEROCK_UPDATE_INTERVAL_MS`	`watchers/therock_update.rs`
`handle_driver_upgrade_event` and its runner variant, the restricted driver-plan result parsing	`watchers/driver_upgrade.rs`
`handle_cache_warm_event` and its resolver variant	`watchers/cache_warm.rs`

watchers.rs keeps reconcile_watcher_snapshots, evaluate_watchers, collect_automation_events_with_gpu_snapshot, evaluate_watchers_for_events and the two event filters, plus the eleven tests that drive the collector or the dispatcher. find_recoverable_service and stop_requested are re-exported from watchers.rs, so the paths supervise.rs and service.rs use in their tests (crate::watchers::find_recoverable_service, crate::watchers::stop_requested) are unchanged. Only tests use stop_requested outside the watcher code, so its re-export is #[cfg(test)] rather than an unused import in the normal build.

Where this differs from the plan, and why:

pending_proposal_exists lives in policy.rs. The thermal-protect and server-recover handlers both call it, and putting it in either file would have made the other import its sibling.
proposals_beyond_a_recent_window (a test helper both dedupe-window tests call) moved to test_support.rs in the server-recover commit, for the same reason.
The GPU pressure thresholds and GPU_METRICS_INTERVAL_MS live in gpu_metrics.rs, because the pressure-event derivation uses the thresholds, and the thermal-protect handler imports them from there. gpu_metrics.rs imports nothing from gpu_thermal_protect.rs.
Tests that call evaluate_watchers_for_events or the collector stay in watchers.rs, because a child test importing the root would create a cycle. For the same reason gpu_metrics.rs has no test module of its own: every test of it goes through the collector or the dispatcher.
Two imports in server_recover.rs are cfg(test)-scoped (json! and queue_proposal_with_arguments), since only the test-only helper and one test use them.
docs/architecture.md is updated in the commit that moves each file rather than in a trailing docs commit, so every commit leaves the doc matching the code. Its entries for common.rs, engine_ipc.rs and supervise.rs, which named watchers.rs as a caller, now name the child files that call them.

Module dependencies

No child imports the root (watchers.rs), no child imports another child except as listed, and rocmd still does not depend on rocm (cargo xtask check-crate-edges passes). Production use crate:: and use super:: edges per file, found by grep on the final tree (test-only imports excluded):

File	Edges
`watchers.rs`	the six children, `crate::host_snapshot`, `crate::persistence`
`policy.rs`	none
`server_recover.rs`	`super::policy`, `crate::engine_ipc`, `crate::persistence`, `crate::supervise`
`gpu_metrics.rs`	`crate::persistence`
`gpu_thermal_protect.rs`	`super::gpu_metrics`, `super::policy`, `crate::common`, `crate::persistence`
`therock_update.rs`	`super::policy`, `crate::sandbox`, `crate::sandbox_tool`, `crate::common`, `crate::persistence`
`driver_upgrade.rs`	`super::policy`, `crate::sandbox`, `crate::sandbox_tool`, `crate::common`, `crate::persistence`
`cache_warm.rs`	`super::policy`, `crate::common`, `crate::persistence`

Among the children the only sibling edges are everything to policy and gpu_thermal_protect to gpu_metrics, which is a DAG. The crate-level edges are the same ones watchers.rs already had; the only edges pointing back at watchers are the two #[cfg(test)] imports in supervise.rs and service.rs, which existed before.

Size

Production lines are all lines outside brace-matched #[cfg(test)] mod blocks, measured on the final tree. Every file is far under the 1,500-line limit, and none of them needs an allowance.

File	Total lines	Production lines
`watchers.rs`	730	237
`watchers/policy.rs`	151	127
`watchers/server_recover.rs`	934	382
`watchers/gpu_metrics.rs`	292	292
`watchers/gpu_thermal_protect.rs`	445	173
`watchers/therock_update.rs`	475	211
`watchers/driver_upgrade.rs`	376	157
`watchers/cache_warm.rs`	277	124

The production total rose from about 1,575 to 1,703 lines, which is the per-file license header, module comment and import block.

Verification

Ran locally, on each of the six commits separately (via git rebase -x), so that every commit compiles and is green on its own: cargo fmt --all --check, cargo clippy -p rocmd --all-targets -- -D warnings, and cargo test -p rocmd --no-fail-fast (168 passed, 1 ignored, the same on every commit and on origin/main).

Ran locally on the final tree: the leaf-name test diff (cargo test -p rocmd -- --list and --list --ignored, keeping lines ending in : test, stripping the module path, sorted without -u) is empty before versus after, 169 names and 1 ignored. The watcher tests are still 40 (11 in watchers, 11 in server_recover, 5 each in driver_upgrade and gpu_thermal_protect, 4 in therock_update, 3 in cache_warm, 1 in policy). cargo xtask check-crate-edges and cargo xtask manifest --check pass. A grep for watchers.rs, watchers:: and watchers/ across docs, AGENTS.md, CONTRIBUTING.md, README.md, xtask/src, tests, .github, scripts and the pre-commit config finds only docs/architecture.md, which this PR updates. One doc comment in crates/rocm-core/src/automation.rs still names apps/rocmd/src/watchers.rs as the watcher plumbing that uses AutomationTriggerEvent; that stays true for the module root, so it is left alone.

Not run locally: a Windows clippy cross-check (--target x86_64-pc-windows-msvc) cannot finish here, because the aws-lc-sys build script needs a Windows C toolchain (lib.exe). None of the moved code is platform-gated (the only cfg attributes in these files are cfg(test)), so the Windows CI lane is the check for the imports and visibility. Also not run, because CI carries them: the full workspace test and clippy gate, cargo xtask e2e and scripts/smoke_local.py.

The new test modules are existing tests relocated unchanged, so there is no new test to mutate. The one new piece of logic, the cfg(test) re-export of stop_requested, is exercised by the service.rs test that imports it.

User-visible behaviour

None. The watchers' recorded events, proposals, messages and timings are produced by the same code, so there is no new or changed Gherkin scenario. This is a purely internal refactor; the existing end-to-end suite runs unmodified.

Notes for reviewers

Reading order, one commit per step, each compiling and passing on its own:

refactor(rocmd): move the watcher test fixtures to test_support.rs: the two builders, pub(crate), no other change.
... watchers/policy.rs: the policy and proposal queue, with pending_proposal_exists; the two action tables stay private to the file.
... watchers/server_recover.rs: the largest move (a file of 934 lines with its eleven tests). This is also where proposals_beyond_a_recent_window moves to test_support.rs, and where the two re-exports are added.
... gpu_metrics and thermal-protect: two files.
... watchers/therock_update.rs.
... driver_upgrade and cache_warm: two files.

A quick way to see that nothing but position changed: git diff --color-moved=dimmed-zebra per commit shows the moved blocks as moved, and what remains is the import lines, the pub(super) keywords and the module declarations.

Open pull requests that also touch apps/rocmd/src/watchers.rs (read-only check against current origin/main, using a merge-tree of each PR head with each commit here):

fix(serve): keep startup progress until the server is ready or stalled #817 (changes requested) edits find_recoverable_service and stop_requested, adds a new helper next to stop_requested, adds two recovery tests (a 138-line hunk, which would move to server_recover.rs), and touches the import block, so it would conflict with the server-recover commit and with every commit that rewrites the imports. It already conflicts with origin/main in watchers.rs and docs/architecture.md, so it needs a rebase either way.
fix(uninstall): stop managed services before removing the tooling that stops them (EAI-8014) #299 (changes requested) adds one daemon_start_ticks: None line in six test fixtures: test_runtime_state (first commit), the AutomationRuntimeState literal in a server-recover test (third commit) and four in the TheRock update tests (fifth commit). It merges cleanly with origin/main today and conflicts from the first commit here on; after this lands the six lines go to test_support.rs, server_recover.rs and therock_update.rs.
fix(serve): make model serving ports collision-safe #243 (changes requested) edits the server-recover handler, find_recoverable_service and the manifest recovery classification (third commit), plus many hunks in the tests. It also touches test_support.rs, but no overlap with the lines edited here was found. It already conflicts with origin/main in watchers.rs, common.rs and docs/architecture.md.

Landing this before any of them means each author rebases onto the new layout; landing them first means a rebase of this branch, which is mechanical because every commit is a move.

test_watcher_snapshot and test_runtime_state are the builders almost every watcher test uses. Moving them to the crate's shared test fixtures first lets the following commits move the handler tests into their own files without touching the builders. Pure move; the function bodies are unchanged and only widened to pub(crate). Signed-off-by: Jussi Elo <jussi.elo@amd.com>

…rs/policy.rs WatcherPolicyAction, watcher_policy_action, queue_proposal, queue_proposal_with_arguments and the action-to-tool and action-to-arguments tables move out of watchers.rs, with their test. pending_proposal_exists, the pending-proposal check shared by the thermal-protect and server-recover handlers, moves with them so the later handler files can reach it without importing each other. Pure move: bodies are unchanged, visibility is pub(super) for what the other watcher files use (the two action tables stay private to policy.rs), and the only other edit is that the one test that builds an AutomationProposalRecord directly now imports it in the test module. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

…ecover.rs The server-recover handler, the recoverable-service scan (find_recoverable_service, stop_requested and the manifest, endpoint and healthcheck classification), the backoff check and the pending-restart dedupe check move out of watchers.rs, with the eleven tests that call them. watchers.rs re-exports find_recoverable_service and stop_requested, the latter only for tests, so supervise.rs and service.rs keep resolving them by the crate::watchers path. Pure move. Two edits beyond relocation and import lines: the test helper proposals_beyond_a_recent_window, which the server-recover and thermal-protect dedupe tests both call, moves to test_support.rs so neither watcher file imports the other's tests; and the json import and the queue_proposal_with_arguments import are scoped to cfg(test), since only the test-only evaluate_server_recover helper and one test use them. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

…to watchers/ watchers/gpu_metrics.rs takes the gpu-metrics handler, the amd-smi snapshot summary, the pressure-event derivation from monitor entries, and the interval and pressure-threshold constants. watchers/gpu_thermal_protect.rs takes the thermal-protect handler, its target selection and its pending-stop check, with the five tests that call the handler directly. The thermal-protect file reads the interval and thresholds from gpu_metrics.rs, and gpu_metrics.rs imports nothing from it, so there is no cycle. The tests that reach these watchers through the dispatcher or the collector stay with them in watchers.rs. Pure move: bodies are unchanged and only visibility (pub(super)) and imports differ. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

…update.rs The therock-update handler and its read-only check runner, the restricted check_updates result parsing, the update-available notification, the schedule check and the interval constant move out of watchers.rs, with the four tests that call them. watchers.rs keeps the collector that reads the interval and the dispatcher that calls the handler. Pure move: bodies are unchanged and only visibility (pub(super)) and imports differ. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

… watchers/ watchers/driver_upgrade.rs takes the driver-upgrade handler, its read-only driver_plan runner and the restricted plan result parsing. watchers/cache_warm.rs takes the cache-warm handler and its registry lookup. The eight tests that call these handlers move with them. What stays in watchers.rs is the registry, the event collector and the dispatcher, plus the tests that drive them. Pure move: bodies are unchanged and only visibility (pub(super)) and imports differ. docs/architecture.md now describes watchers.rs as the module root and has one entry per file under watchers/, and the entries for common.rs, engine_ipc.rs and supervise.rs name the watcher files that call them. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

Copilot

🟢 Approval recommended

No active review issues were identified for the watcher-module refactor, although tests and builds were not run in the review environment.

No open issues 🎉

What changed in this PR

Refactors rocmd’s watcher implementation into focused modules, reducing file size and isolating watcher-specific changes without changing user-visible behavior.

Changes:

Moves six watcher implementations and shared proposal policy into watchers/, retaining collection and dispatch in watchers.rs.
Relocates existing tests alongside their handlers and shared fixtures into test_support.rs, preserving existing test-facing paths.
Updates the architecture documentation to reflect module responsibilities and dependencies.

Tests and builds were not run in this review environment.

File	Description
`docs/architecture.md`	Documents the watcher modules and updates caller references.
`apps/rocmd/src/watchers/therock_update.rs`	Houses scheduled update checks, notifications, and existing tests.
`apps/rocmd/src/watchers/server_recover.rs`	Houses recovery classification, restart handling, backoff, and existing tests.
`apps/rocmd/src/watchers/policy.rs`	Centralizes watcher policy, proposal queuing, and deduplication.
`apps/rocmd/src/watchers/gpu_thermal_protect.rs`	Houses pressure-target selection, stop proposals, and existing tests.
`apps/rocmd/src/watchers/gpu_metrics.rs`	Houses telemetry handling, pressure-event derivation, and thresholds.
`apps/rocmd/src/watchers/driver_upgrade.rs`	Houses driver-plan handling and existing tests.
`apps/rocmd/src/watchers/cache_warm.rs`	Houses artifact validation, prefetch proposals, and existing tests.
`apps/rocmd/src/watchers.rs`	Retains registry, collection, dispatch, re-exports, and integration-focused tests.
`apps/rocmd/src/test_support.rs`	Shares relocated watcher fixtures and the proposal-window helper.

_{Review effort: Balanced}

nowycondro

🤖 Automated review (Claude Code) · 904ddb5

This automation never files a GitHub approval, so no approving review will appear here whatever the outcome — the merge decision stays with a human reviewer.

Review — no blocking findings

Full review of the whole change (a2048808...904ddb5, 10 files, +3410/-3216). Completes ROCMAI-859's acceptance criteria exactly: every symbol in the ticket's scope table lands in its stated destination, the re-exports supervise.rs/service.rs depend on resolve, the leaf-name test diff is empty, docs/architecture.md gets one entry per new file, and no module cycle is introduced.

Blocking

None

Non-blocking

None

Decisions for the author

None

Positive signals

Structural verification by function/const and by test name (automated diff, not eyeballing) shows all 54 non-test functions/consts and all 40 tests present exactly once each, pre- and post-split, with none duplicated or dropped.
The GPU pressure thresholds and GPU_METRICS_INTERVAL_MS stay defined exactly once in gpu_metrics.rs and are imported (not copy-pasted) into gpu_thermal_protect.rs, so the two files can't drift independently.
The restricted, read-only nature of the TheRock-update and driver-upgrade sandbox-tool calls (check_updates/driver_plan) survived the move intact — no broadening of what gets executed in contained mode.
docs/architecture.md is updated in the same commit as each move (not batched into a trailing docs commit), and its new links use this repo's required relative-citation style rather than bare paths.
The layout mirrors the existing apps/rocm/src/comfyui.rs + comfyui/ precedent and the AGENTS.md guardrail to extract a domain into its own file rather than growing it in-place.

Deployment notes

Pure internal refactor; no user-visible behavior change, confirmed by byte-for-byte-identical function bodies and unchanged test assertions across all seven moved watcher files. At review time, CI's build/test/clippy/fmt/license-header/lychee/commit-signature checks were all green on this head commit; a handful of hardware-gated E2E lanes (MI300X, W7900, Strix Halo nightly/release) were still queued rather than completed — worth a glance before merging, though nothing here suggests they'd behave differently than the Linux/Windows/WSL2 lanes that already passed.

What this covered

Read completely: both the pre-split apps/rocmd/src/watchers.rs (3,537 lines) and every post-split file — the new watchers.rs root, all seven watchers/*.rs children, test_support.rs (before and after), and the docs/architecture.md diff — plus a repo-wide grep for every crate::watchers::/mod watchers reference (lib.rs, daemon.rs, service.rs, supervise.rs, crates/rocm-core/src/automation.rs) to confirm nothing outside the moved files needed updating and none was missed. Three independent fan-out passes each covered a coupled file group (gpu-metrics+gpu-thermal-protect; therock-update+driver-upgrade; cache-warm plus the cross-cutting impact analysis) and converged on the same conclusion as the driver's own read of the root/policy/server-recover/test_support files. CI's check-run list was read directly rather than assumed.

Did not run: a local cargo build/cargo test of this PR's actual content — the PR branch was fetched read-only and never checked out into a working tree (per this review's no-checkout rule), so correctness relied on (a) textual/structural diffing against the pre-split source and (b) the PR's own green CI run on this exact head commit. History and prior-review passes were not run separately; this is a pure mechanical move with no adjacent history to mine. Ticket ROCMAI-859 was retrieved and compared against the diff rather than assumed; it matches. Not reconciled against this PR's existing review thread (only checked for a prior automated review under this name, which was absent).

🤖 Generated with Claude Code

juhovainio

I went through the whole diff line by line. Nothing blocking in the code itself: the moved bodies are identical to the originals (only imports, pub(super)/re-export visibility, wrapped signatures and module wrappers differ), all 40 tests are preserved by name, no cfg gates were dropped, and no new module cycle is introduced. Scope matches ROCMAI-859.

Requesting changes only so the items below get cleared before merge:

CI. Both E2E tests (Strix Halo, Ubuntu, nightly, current) and (Strix Halo, Ubuntu, release, current) fail at Run E2E tests on Strix Halo, and the MI300X and W7900 E2E jobs were still pending when I looked. For a pure move this looks like runner/infra trouble, but I couldn't confirm that from the logs. Please re-run or inspect, and get them green.
gpu-metrics tests (non-blocking, see inline). The two gpu-metrics tests stayed in the root watchers.rs, so gpu_metrics.rs has no test module, while the ticket says tests move with their code. I understand the cycle argument in the PR body. Either move them or leave a short note in gpu_metrics.rs saying where its coverage lives.
Doc nit (non-blocking). The docs/architecture.md entries for watchers/cache_warm.rs and watchers/driver_upgrade.rs list their crate:: edges but omit crate::webhook, which both use via local_webhook_event_from_request.

juhovainio · 2026-11-05T12:03:18Z

        Ok(())
    }

    #[test]
-    fn server_recover_local_webhook_does_not_restart_healthy_service() -> Result<()> {
-        let (root, paths) = temp_app_paths("server-recover-healthy-webhook");
+    fn gpu_metrics_event_records_read_only_status_without_proposal() -> Result<()> {


Non-blocking: this test drives handle_gpu_metrics_event but stays in the root, so gpu_metrics.rs has no tests; ticket says tests move with the code. If it goes through the dispatcher only, a one-line note in gpu_metrics.rs saying where its coverage lives would help the next reader.

Added a note to the module doc of gpu_metrics.rs (48150df) naming the four tests that cover it and why they stay in the root: moving them would make a child import the dispatcher, which is the cycle this split avoids, and calling handle_gpu_metrics_event directly would no longer be a pure move.

…nly webhook edge gpu_metrics.rs has no test module because its tests drive it through the collector and dispatcher in watchers.rs, which a child cannot import without a cycle; the module doc now names those tests. The architecture entries for the driver-upgrade and cache-warm watchers also list the webhook helper their tests use. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

jussielo-amd · 2026-11-05T12:29:33Z

Thanks for the line-by-line read. All three items are addressed in 48150df, which only touches a module doc comment and two lines of docs/architecture.md, so there is no code change since your review.

CI. Both red lanes are the two Strix Halo Ubuntu E2E jobs, and they are red on every PR and every main push since ROCMAI-827: start the background helper only while an automation needs it #838 merged, for a reason this PR cannot affect: serve-40 (serve-stop-leaves-no-background-helper, added by ROCMAI-827: start the background helper only while an automation needs it #838) has no xfail row, so the lane reports one unexpected failure. That is tracked in E2E: serve-40 has no xfail row for lemonade on Linux, so both Strix Halo Ubuntu lanes report an unexpected failure on every run #898, with the xfail row in test(e2e): expect serve-40 to fail on lemonade Linux hosts while #897 is open #900. I could not read the step output of this PR's own two runs (the job log API returned only the runner setup and teardown), so I am relying on E2E: serve-40 has no xfail row for lemonade on Linux, so both Strix Halo Ubuntu lanes report an unexpected failure on every run #898's analysis of the same two jobs, which names the identical failing step, and on this change being a move inside rocmd that no serve scenario reaches. The MI300X and W7900 jobs are still queued behind the runners. I will not re-run the two Strix lanes until test(e2e): expect serve-40 to fail on lemonade Linux hosts while #897 is open #900 or an equivalent merges, since they would fail the same way.
gpu-metrics tests. I kept the tests in the root, because moving them would make gpu_metrics.rs import the dispatcher in watchers.rs, which is the cycle this split avoids, and rewriting them to call handle_gpu_metrics_event directly would no longer be a pure move. As you suggested, gpu_metrics.rs now carries a note in its module doc naming the four tests that cover it and why they live in the root.
Doc nit. Correct, and the edge is test-only: local_webhook_event_from_request is called from the #[cfg(test)] modules of driver_upgrade.rs and cache_warm.rs. Both architecture entries now say their tests also use webhook.rs.

Copilot

🟢 Approval recommended

No active review issues block the watcher modularization.

No open issues 🎉

_{Review effort: Balanced}

fixed

juhovainio

Reviewed the split of rocmd watchers.rs into watchers/. Compared removed vs added lines across the diff: the only differences are imports, mod/use/re-exports, pub(super) visibility, one rustfmt reflow of the watcher_policy_action signature, module docs, license headers, and three test helpers made pub(crate) in test_support.rs. No function bodies, constants or cfg gates were dropped, and the test count is unchanged (34 removed, 34 added). architecture.md links are relative and point at files added here; new files carry license headers; no internal names in added lines. CI is green. Scope matches ROCMAI-859. No blocking or non-blocking findings.

jussielo-amd added 6 commits November 4, 2026 14:57

jussielo-amd requested a balanced review from Copilot November 4, 2026 15:02

Copilot started reviewing on behalf of jussielo-amd November 4, 2026 15:02

View session

Copilot AI reviewed Nov 4, 2026

View reviewed changes

jussielo-amd marked this pull request as ready for review November 4, 2026 15:06

jussielo-amd requested a review from a team as a code owner November 4, 2026 15:06

jussielo-amd requested a review from volen-silo November 4, 2026 15:06

nowycondro reviewed Nov 4, 2026

View reviewed changes

jussielo-amd mentioned this pull request Nov 5, 2026

refactor(rocmd): move the MCP argv builders and read-only allowlist out of mcp.rs into mcp_argv.rs #905

Merged

juhovainio previously requested changes Nov 5, 2026

View reviewed changes

jussielo-amd requested review from juhovainio and a balanced review from Copilot November 5, 2026 12:29

Copilot started reviewing on behalf of jussielo-amd November 5, 2026 12:30

View session

Copilot AI reviewed Nov 5, 2026

View reviewed changes

juhovainio approved these changes Nov 5, 2026

View reviewed changes

jussielo-amd added this pull request to the merge queue Nov 5, 2026

Merged via the queue into ROCm:main with commit 23324b1 Nov 5, 2026
31 checks passed

jussielo-amd deleted the refactor-rocmd-split-watchers branch November 5, 2026 15:04

Navigation Menu

Repository navigation

ROCMAI-859: split rocmd watchers.rs into a watchers/ directory - #885

ROCMAI-859: split rocmd watchers.rs into a watchers/ directory#885
jussielo-amd merged 7 commits into
ROCm:mainfrom
jussielo-amd:refactor-rocmd-split-watchers

jussielo-amd commented Nov 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

nowycondro left a comment

Uh oh!

juhovainio left a comment

Uh oh!

juhovainio Nov 5, 2026

Uh oh!

jussielo-amd Nov 5, 2026

Uh oh!

jussielo-amd commented Nov 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

juhovainio left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

jussielo-amd commented Nov 4, 2026

Summary

What moved

Module dependencies

Size

Verification

User-visible behaviour

Notes for reviewers

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

nowycondro left a comment

Choose a reason for hiding this comment

Review — no blocking findings

Blocking

Non-blocking

Decisions for the author

Positive signals

Deployment notes

What this covered

Uh oh!

juhovainio left a comment

Choose a reason for hiding this comment

Uh oh!

juhovainio Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

jussielo-amd Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

jussielo-amd commented Nov 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

juhovainio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants