[jussielo-amd]

Closes ROCMAI-883 and ROCMAI-866.

Summary

Two defects in how the dashboard daemon keeps its list of instances in step with what is really running, fixed together because they sit in the same discovery pass of crates/rocm-dash-daemon/src/runner.rs. The first (ticket in the first section below) is that a managed-service registry that cannot be read no longer makes every served model vanish; the second is that when the Lemonade endpoint moves to a new host or port, the daemon no longer keeps the endpoint it left. Neither change alters what a healthy, unchanged system looks like.

ROCMAI-883: keep managed services when the registry cannot be read

The daemon follows the managed-service registry (the records rocm serve writes under the services directory) on every discovery tick. When that directory could not be read, or a record file could not be read, the loader quietly returned zero services and the run loop reconciled against that empty set, so a single transient read error announced every managed instance gone and dropped it from the snapshots. A dashboard user would see every served model vanish for a tick (or until the next successful read) with nothing to explain why. Docker discovery already avoids this: when its listing errors, the reconcile step is skipped and the known set is left alone. The registry had no way to say "I could not read" as opposed to "there is nothing here".

The fix makes that distinction in the registry reader and honours it in the loop. read_service_records returns a Result: a registry that reads in full (including one with no directory, or no records) is confirmed, and one that cannot be read in full is an error. A failed pass retires nothing, exactly as a Docker error does: the known services stay known, no gone or repeat discovered event is sent, the instances keep the data they hold and keep being scraped, and the header warns until a read succeeds. A confirmed-empty registry still retires the services it lacks. One difference from Docker, whose listing is all or nothing: a registry can fail on a single record while the others read, so the error carries the records that did read, and the loop still adds and refreshes those services (announced once, as ever). Retirement is the only thing a failed pass withholds.

Symbol	Change
registry::read_service_records (new)	Returns Result<RegistryRead, RegistryReadError>. RegistryRead holds the well-formed records (newest first) and the paths of records that read but are not valid JSON. The scan keeps going past a record it cannot open and returns the first failure together with what it did read.
registry::RegistryReadError (new)	The path that failed, the I/O error, and partial: the RegistryRead of everything the pass could read (empty when the directory itself could not be listed).
registry::load_service_records	Unchanged signature and best-effort contract: a directory that cannot be listed yields an empty list, and a record that cannot be read or parsed is skipped on its own, so one bad file never hides the services beside it. It is the strict scan with the failure replaced by its partial read, so the two cannot drift apart. It is still used by the one-shot readers in apps/rocm/src/dash.rs (launcher, exit prompt, services overlay), which have nothing to retire.
runner::run_loop managed block	Calls read_service_records. On Ok it reconciles and refreshes the non-vLLM set. On Err it upserts and announces the services the partial read saw, retires nothing, and sets a header warning.
runner::Reconcile::upsert_set (new)	The add-and-refresh half of reconcile_set, which now calls it and then retires.
runner::DiscoveryState::keep_through_partial_registry (new)	After a partial read, the services it saw join the known set (so a later complete read can retire them) and their non-vLLM marks follow the record; the known services it did not see stay known and keep their marks.
runner::RegistryHealth (new)	Remembers whether the previous pass failed and which malformed files were already reported, so each condition is logged once.

How each case is classified: a directory that does not exist is confirmed empty (it is created the first time something is served). A directory that exists but cannot be listed, or a regular file where it should be, is a read failure with nothing partial to offer. A record file that vanishes between the listing and the read was removed on purpose and is skipped. Any other failure to read a record (a permission or I/O error, a symlink loop) is a read failure for the pass, because that record's service cannot be told apart from one that is gone. A record that reads but does not parse is malformed: it is skipped and reported, and it no longer hides the other services. That last choice is safe because the rocm-cli writer replaces a record atomically (write_atomic in crates/rocm-core/src/service_record.rs), so a corrupt file is corrupt for good rather than half-written.

What a permanently unreadable record costs, since a review asked whether that is the intended trade-off: while one record stays unreadable (for example a root-owned file left by a privileged serve) the registry never reads in full, so retirement stays frozen for as long as it lasts, with no timeout. A service that has stopped, or whose record was removed, keeps its row and its last live fields until the file is fixed or removed, and the header warning names the unreadable path throughout. New services still appear on the next discovery tick, and a service whose record reads is refreshed. The freeze is deliberately not bounded by a deadline: retiring a service because a file stayed unreadable for long enough would bring back the wrongly vanishing row this change exists to prevent, so the cost is made visible (the warning and the log line name the file) rather than guessed away. The module docs of registry.rs state this.

ROCMAI-866: retire the old Lemonade instance when its host or port changes

The Lemonade instance id encodes host and port, so a moved endpoint is a new id. The daemon purged the old id's telemetry and announced the new id, but never retired the old instance: it stayed in the daemon state and in every snapshot afterwards, no gone event was sent for it, and the vLLM Prometheus scrape kept polling its port as if it were a vLLM server, because the scrape filter only skips the current Lemonade id. The result was two Lemonade rows (one dead) and a scrape warning on every instance cadence for as long as the daemon ran. If the endpoint later went quiet, only the current id was retired, so the dead one was never cleaned up.

Now, when the probe reports a different id, reconcile_lemonade retires the old one first (gone event, removed from the state, telemetry purged) through the same retire step an endpoint that goes quiet already used, and then upserts and announces the new one. A snapshot carries exactly one Lemonade instance, and the old endpoint is no longer in the set the vLLM scrape polls. An in-place refresh of the same id is unchanged and still carries live fields over. The vLLM scrape target selection moves out of run_loop into a small function, vllm_scrape_targets, with the filter unchanged, so the tests can ask which endpoints would be polled.

Verification

Run on the combined result, including the review-fix commits: cargo fmt --all --check; cargo clippy --locked -p rocm-dash-daemon --all-targets -- -D warnings; cargo clippy --locked -p e2e-cucumber --test e2e -- -D warnings; cargo test -p rocm-dash-daemon --no-fail-fast (80 tests on main: 56 unit, 4 + 9 + 11 integration; 117 after: 82 unit, 14 + 9 + 1 + 11 integration, none lost); the feature_naming test; cargo xtask e2e -- -n "dash-4[34]" (both scenarios pass); cargo xtask manifest --check; cargo xtask check-crate-edges; lychee in offline mode on docs/architecture.md; the Windows cross-check cargo clippy -p rocm-dash-daemon -p e2e-cucumber --all-targets --target x86_64-pc-windows-msvc -- -D warnings with stub C compilers (clean); and the leak scan over the lines this branch adds relative to main (no hits). On the merged tree I also broke the Lemonade retire step by hand (purge only, the old behaviour) and the host-change, port-change, move-away-and-back and lifecycle tests went red, then restored it. The per-change mutation checks described below were done on each branch before the two were combined.

Not run: the Windows tests themselves (only the cross-check compiles for that target), so CI's Windows job is the check; the full workspace test gate, coverage and the smoke script, which CI carries. The mode-000 tests, the symbolic-link tests and the scenarios' link steps are cfg(unix); the portable file-in-place-of-directory test and the injected-reader tests are the ones that run on the Windows job. The Lemonade change is not platform-gated.

Registry read tests. State matrix for the new health state, one test per row: first failure, repeated failure (quiet), recovery (once), a new streak (reported again), empty and populated reads (quiet), malformed set new, unchanged, grown, cleared, and malformed set unchanged across a read failure; these are the registry_health_* unit tests in runner.rs. The loop-level behaviour is driven through the real run_loop in crates/rocm-dash-daemon/tests/discovery_reconcile.rs, asserting the announcements and the snapshots together (and the header warning): confirmed-empty registry retires, absent directory retires, an unlistable directory (a file in its place; portable) keeps the instance, an unreadable directory (mode 000, Unix only and skipped when running as root) keeps it, recovery to the same service announces nothing and clears the warning, recovery into an empty registry retires it, and a malformed record beside a valid one hides nothing while a new valid record is still discovered. crates/rocm-dash-daemon/tests/registry_read_logging.rs captures the daemon's log across two outages and a recovery and asserts one warning per streak and one recovery line; it is its own test binary because tracing caches whether a callsite is enabled the first time any thread reaches it. registry.rs has unit tests for the classification and for both readers, including the unreadable-record behaviour and the partial read on every platform through the injected reader. Shown to fail when the code they cover is broken, by editing the branch and confirming red: reading an unlistable directory as empty; reconciling against the empty set on a failed read (four loop tests and scenario dash-43 fail); logging on every failed tick; treating a malformed record as a read failure; treating an unreadable record file as skipped; treating an absent directory as a failure; never clearing the header warning on recovery; and making the best-effort loader and the strict reader behave alike on an unreadable record.

Unreadable-record tests (the partial read). State matrix for a registry where some record cannot be opened, one test per row in discovery_reconcile.rs, each asserting the announcements, the ids a snapshot lists and the header warning together: one record unreadable and a new valid record added (the new service is announced once, the known one is never missing, the warning names the file); one record unreadable and a known record removed (nothing is announced and the removed service keeps its row, then the unreadable record is removed, the registry reads in full, the service is retired once and the warning clears); the known service's own record replaced by an unreadable one (it keeps its row with its live fields and nothing is announced, then it reads again and nothing is announced and the warning clears). runner.rs has two unit tests for the discovery state after a partial read (what joins the known set, which non-vLLM marks follow the record, what stays). Shown red by editing the branch: dropping the upsert on a failed pass fails the new-service test and scenario dash-44; retiring on a failed pass (reconciling the partial read as if it were complete) fails the removed-record test, the own-record test and three of the older outage tests (unlistable directory, unreadable directory, recovery into an empty registry); not adding the seen services to the known set fails the new-service test and the discovery-state test; not refreshing the non-vLLM marks fails the discovery-state test; and returning an empty partial read from the scan fails the four registry unit tests that read past an unreadable record.

Lemonade tests. State matrix for the Lemonade source, one test per row, each asserting the announcements, the instances a snapshot would carry, the per-id telemetry and the pairs the vLLM scrape would poll together:

State	Expected
Never answers	nothing announced, no instances, no telemetry, no scrape targets
First answer	discovered once, one instance, not a vLLM target
Same endpoint again	nothing announced, live fields and telemetry kept
Host changes	gone old, discovered new, one instance, old telemetry purged, no scrape target
Port changes	same as host change
Goes quiet	gone once, no instances, telemetry purged
Comes back elsewhere after quiet	discovered new only, one instance
Same answer twice in one tick	announced once, one instance
Moves away and back	gone and discovered on each move, one instance at the end

The existing lifecycle test had pinned the stale instance as a conscious pin; that assertion is now inverted. Shown to fail when the code it covers is broken: skipping the retire on an id change fails the host-change, port-change, move-away-and-back and lifecycle tests; retiring without purging telemetry fails five rows plus the lifecycle test; retiring the new id instead of the old one fails four; and dropping the Lemonade exclusion from the scrape target filter fails seven.

User-visible behaviour

Scenario dash-43 (@id:dash-managed-model-survives-unreadable-registry, tagged @requires-os:linux, so it runs on the Linux e2e lane and is not exercised on Windows) drives the real dashboard under a pseudo-terminal: with a managed model on screen, it adds one record to the registry that cannot be opened (a symbolic link to itself, which fails with a loop error and not with not-found), waits for the header warnings badge to grow (evidence that a frame assembled after the failed read is on screen), and then asserts the model is still on screen, sampling for a few seconds. The step is one atomic operation that leaves the model's own record and the directory in place, so the dashboard can never poll an empty or absent registry in between, and it works when the runner is privileged. It fails if the loop retires the model on a failed read. Scenario dash-44 (@id:dash-new-model-appears-beside-unreadable-record, also @requires-os:linux, on the same ordinary hermetic Linux lane) starts from the same unreadable record and then writes a second model's record into the registry: it asserts the second model appears on screen while the first stays. It fails if a failed pass adds nothing. The contrast (a confirmed-empty registry retires), the removed-record and recovery rows and the once-per-streak log line are asserted by the daemon's own tests, where the loop runs without a terminal. dash-07 (managed model visible) is unchanged and still applies. docs/testing.md and docs/manual-testing.md list no dash scenario individually (they name a scenario only where a special lane needs it), so neither gets an entry for dash-43 or dash-44.

On a registry read error, and only then (a readable registry, empty or populated, behaves as before):

• Managed instances no longer disappear from the dashboard. Before, each was announced gone and dropped from every snapshot for the tick. Now they stay with their last live fields (KV cache, request counts, throughput) and keep being scraped.
• When only some records cannot be opened, a service whose record can be read still appears (or refreshes) on the next discovery tick. Only retirement waits for the registry to read in full, for as long as the unreadable file stays.
• The header warnings badge gains one warning, managed registry: cannot read <path>: <error>; keeping the last known services, for as long as reads keep failing. It clears on the first successful read.
• The daemon log gets one warning on the first failed pass of a streak and one info line when reads work again. A second outage logs again; failed passes in between are silent.
• If the registry becomes readable and is empty after an outage, the services kept through it are retired at that point; likewise a service whose record was removed during the outage is retired when the registry reads in full.
• A malformed record is logged once, listing the files; the same set is not repeated every tick, and a changed set logs again. It does not trigger the header warning.

After the Lemonade host or port changes, wherever Lemonade discovery is enabled (the daemon's enable_lemonade option; the shipped rocm dash sets it off and fixes the host and port in runner_options in apps/rocm/src/dash.rs, so today this is reached by a daemon embedder, and by the Lemonade serving-discovery work that turns it on):

• The dashboard's instance list shows one Lemonade instance, the one at the new endpoint, instead of that one plus a stale row for the old endpoint. The Instances and Serving views render whatever the snapshot lists and carry no Lemonade-specific wording that changes.
• A subscriber to the daemon event stream (including a recorded session) sees a gone event for the old id followed by a discovered event for the new id; before, it saw only the discovered event.
• The spurious vLLM scrape warning for the dead Lemonade endpoint no longer appears in the header badge.
• If the endpoint then goes quiet, nothing is left behind; before, the old row stayed forever. If it moves away and back, there is always exactly one instance.
• Unchanged: a probe that answers with the same id is not announced again and keeps its live fields; an endpoint that goes quiet is announced gone once; nothing is announced when Lemonade never answers.

No Gherkin scenario is added for the Lemonade change, and none can reach it: the shipped dashboard never enables Lemonade discovery and its Lemonade host and port are fixed for the life of a run, so no end-to-end scenario can start a Lemonade endpoint and move it, and not even the integration test in crates/rocm-dash-daemon/tests/discovery_reconcile.rs can move one mid-session. The lane that covers it is cargo test -p rocm-dash-daemon in the ordinary Rust test job: the reconcile step and the scrape-target selection are covered at the unit level, and the existing Lemonade lifecycle integration test still passes.

Notes for reviewers

The two changes do not interact. Lemonade is a single probed endpoint with its own id and its own retire path, while the registry governs the managed-service source; a registry read failure touches known_services and managed_non_vllm only, and vllm_scrape_targets reads the latter, so a kept managed service is still scraped and a kept non-vLLM one is still skipped. The two sources treat silence differently on purpose: an unanswered Lemonade probe is a definite "not there" and retires the instance, whereas an unreadable registry is "unknown" and retires nothing. The branch merged cleanly, with no textual conflicts, because the Lemonade change edits the Lemonade reconcile step, the scrape-target selection and a test block placed before the lifecycle test, and the registry change edits the managed-service block, the discovery state and tests at the end of the module.

Read the registry-read commits first, in order: the first pins current behaviour on an unreadable directory, the second changes it and flips those pins, and a merge of origin/main only renumbers the new scenario (main added a dash-42 while this branch added one under the same number; main's keeps dash-42, this one is dash-43). The commit message of the second commit still calls the scenario dash-42. Two review-fix commits follow: the first makes the best-effort loader skip only an unreadable record (it had been discarding every readable service on one bad file), and the second changes how dash-43 makes the registry unreadable. Then the Lemonade commits: the first adds the Lemonade state matrix tests together with the behaviour-neutral vllm_scrape_targets extraction and pins the old behaviour, and the second changes the reconcile step and flips exactly those assertions. A merge commit joins the two. Two commits answer the approving review and come last: the first makes a failed pass keep adding and refreshing the services that did read (the partial read, with its state matrix and the registry module's trade-off note), and the second adds scenario dash-44 for it.

docs/architecture.md has one changed line, the rocm-dash-daemon crate description, which now covers the registry read distinction, including that a pass that cannot read in full retires nothing but still adds what it read. Open pull requests touching the same files: #631 changes registry.rs, runner.rs, telemetry_contract.rs, dash.feature, dash_steps.rs and docs/architecture.md (Lemonade serving discovery), so expect conflicts in the runner and registry modules there. #914, #856 and #913 append to or edit dash.feature and dash_steps.rs, so the scenario file tail and the step file may conflict textually.

Risk: a registry record that stays unreadable (for example a file with permissions removed) now keeps the dashboard from retiring any managed service until it is fixed, instead of silently dropping that service. That is deliberate (the alternative is guessing which service the file belonged to) and is visible through the header warning and the log line, which name the path. New and readable services are not held back by it.

[Copilot]

🟡 Changes recommended

The compatibility loader can hide readable services when one record fails, and the end-to-end test has a registry-replacement race that can fail even with a correct implementation.

2 open issues

• Preserve per-record error skipping in the compatibility loader · New
• Avoid exposing an empty registry during replacement · New

What changed in this PR

This PR distinguishes an unreadable managed-service registry from an empty one, allowing the dashboard daemon to retain known services during read failures.

Changes:

• Adds explicit registry read results and skips reconciliation when reads fail.
• Adds persistent header warnings and logs failure/recovery transitions.
• Adds reader, daemon, logging, and end-to-end regression coverage, plus architecture documentation.

Verification was limited to the supplied diffs; the repository snapshot was unavailable, and tests were not run.

File	Description
tests/​e2e-cucumber/​tests/​e2e/​dash_steps.rs	Adds registry-failure injection and screen assertions.
tests/​e2e-cucumber/​features/​dash.feature	Adds scenario dash-43 for retaining managed models during read failures.
docs/​architecture.md	Documents registry failure handling.
crates/​rocm-dash-daemon/​tests/​registry_read_logging.rs	Checks failure and recovery logging across outages.
crates/​rocm-dash-daemon/​tests/​discovery_reconcile.rs	Covers retention, retirement, recovery, and malformed records.
crates/​rocm-dash-daemon/​src/​runner.rs	Preserves known services on read errors and tracks registry health.
crates/​rocm-dash-daemon/​src/​registry.rs	Adds fallible registry reads and classification tests.

_{Review effort: Balanced}

[Copilot]

🟢 Approval recommended

No active review issues are recorded for the dashboard discovery and Lemonade lifecycle changes.

No open issues 🎉

2 resolved since last review

• Avoid exposing an empty registry during replacement
• Preserve per-record error skipping in the compatibility loader

_{Review effort: Balanced}

[juhovainio]

Approving. I read through the diff (no local build or test run) and found nothing blocking; CI is green. Both fixes do what the tickets ask: a failed registry read is now distinct from an empty registry and no longer wipes managed services, and the old Lemonade instance is retired when its host or port changes. The tests assert both the announcements and the resulting snapshot, which is what I wanted to see.

Three non-blocking points, two of them as inline comments:

1. A single permanently unreadable record fails every pass, so discovery would stay frozen. I understand this is a deliberate, documented trade-off; worth confirming it is the one you want.
2. The Lemonade retirement is user-visible but only covered by unit tests. Could the PR text say which lane covers it (or add a scenario), and confirm the Serving/Instances views and the Lemonade wording in docs/architecture.md need no change?
3. docs/testing.md and docs/manual-testing.md are not touched; please check whether the new scenario dash-43 needs an entry there.

[juhovainio]

One permanently unreadable record (a root-owned file, a symlink loop) now fails every pass, so discovery freezes: new services never appear and gone ones never retire. Is that the intended trade-off? Consider still adding newly readable records, or bounding how long the freeze can last.

[jussielo-amd]

It was a deliberate trade-off, but the freeze was wider than it needed to be, so it is narrower now (913a53e). A pass that cannot open one record still retires nothing, which is the rule this PR exists for, but the scan now keeps reading past the bad record and the error carries the records that did read. The run loop adds and refreshes those services and announces new ones once, so a new rocm serve appears on the next discovery tick even beside a stale root-owned record. What stays frozen is only retirement: a service that stopped, or whose record was removed, keeps its row and its last live fields until the registry reads in full, and the header warning names the unreadable path the whole time. I did not bound the freeze with a deadline, because retiring on a timer would bring back the wrongly vanishing row; the module docs of registry.rs and the PR body state this. Each row of the state matrix asserts the announcements and the snapshot together (new record beside an unreadable one, removed record then fixed, the known service's own record becoming unreadable then readable), each shown red by a targeted mutation, and scenario dash-44 (ea75b80) covers the new model appearing on screen.

[juhovainio]

Retiring the old id here is user-visible (the old row disappears) but only unit-tested. Please say in the PR text which lane covers it, or add a scenario, and confirm the Serving/Instances views and docs/architecture.md need no change.

[jussielo-amd]

The lane is cargo test -p rocm-dash-daemon in the ordinary Rust test job: the host-change, port-change, move-away-and-back, quiet and lifecycle rows are unit and integration tests there. I did not add a scenario because none can reach it: the shipped dashboard sets enable_lemonade to false and fixes the Lemonade host and port in runner_options in apps/rocm/src/dash.rs, so no end-to-end scenario can start a Lemonade endpoint and move it (the integration test cannot move one mid-session either). The row is user-visible only where Lemonade discovery is enabled, which today is a daemon embedder and the Lemonade serving-discovery work in #631. I also checked the views: the Instances and Serving views render whatever the snapshot lists, and the Lemonade wording in crates/rocm-dash-tui is about the engine picker and install labels, so nothing there changes. The Lemonade wording in docs/architecture.md is unaffected too, since the rocm-dash-daemon entry does not describe Lemonade discovery. The PR body now says all of this and no longer claims the shipped dashboard shows the change.

[jussielo-amd]

Answers to the three points of the approving review, for the push at ea75b80.

1. Confirmed that it was intended, and narrowed it in 913a53e: a failed pass still retires nothing, but the records that did read are now added and refreshed, so discovery of new services no longer freezes. Only retirement stays frozen while a record is unreadable, with no timeout on purpose, and the header warning and log line name the file; the trade-off is written into the registry.rs module docs and the PR body, and scenario dash-44 (ea75b80) plus the new state-matrix tests cover it.
2. The Lemonade retirement is covered by cargo test -p rocm-dash-daemon in the ordinary Rust test job. No scenario is possible because the shipped dashboard never enables Lemonade discovery and fixes its host and port, so it cannot be driven end to end; the PR body says so and now scopes the user-visible claim to where discovery is enabled. The Serving and Instances views and the Lemonade wording in docs/architecture.md need no change; the one docs/architecture.md line this round touches is the rocm-dash-daemon description, for the partial read.
3. Neither docs/testing.md nor docs/manual-testing.md lists dash scenarios individually (they name a scenario only where a special lane needs it, like serve-41), and docs/manual-testing.md has no section on the registry or the header warning, so dash-43 and dash-44 need no entry there and I left both files untouched.

[Copilot]

🟢 Approval recommended

No active review issues require further changes.

No open issues 🎉

_{Review effort: Balanced}