Capture backend identity for Lemonade managed-serve failures by Copilot · Pull Request #899 · ROCm/rocm-cli

Copilot · 2026-11-05T08:27:18Z

Strix Halo Ubuntu managed serves return success without exposing the model, with logs showing argument rejection, backend-ready/router-unavailable, or download failure. Existing xfails attribute all three to an immediate shutdown.

Diagnostics: Capture runtime version, configured pins, candidate backend paths, SHA-256 fingerprints, and version/help output alongside archived service logs before cleanup. Bound probe duration and retained output; kill and reap timed-out children.
Expectations: Replace shutdown-only attribution with the observed failure signatures and cover serve-40’s shared serve precondition. Retain intermittent-failure handling and existing platform scope.
Scope: Diagnostic coverage, not a confirmed backend fix. Preserve the runtime pin and asynchronous managed-launch contract; distinguish configured pins and candidate binaries from proof of what actually ran.
If this PR fixes a bug, searched tests/e2e-cucumber/expectations.toml for the fixed ticket ID and removed/narrowed any now-stale xfail rows.
If this PR adds a new subcommand or subsystem, its domain implementation lives in its own file per docs/architecture.md (the clap declaration and dispatch wiring staying in main.rs/lib.rs is expected, not a violation).
Every new or changed user-facing message was read against the code path that runs after it, and its test asserts the resulting state — not only the wording — per AGENTS.md §3. Covers claims of an outcome, remediation advice naming a command, and promises that something will not happen.

Refs E2E: lemonade serving fails on the Strix Halo Ubuntu lanes, with a different log signature depending on the run #897

jussielo-amd

Thanks for this, the diagnostics are well thought out: the backend list and manifest path match what the engine uses, and probes are read-only with good "not proof of selection" wording. One blocker, plus a few small things.

Blocker

The "Commit signatures + sign-off" check is red: the first commit ("Initial plan") has no Signed-off-by trailer. Please add the sign-off (git rebase --signoff origin/main) or squash that commit away, then re-push.

Non-blocking

The description says Fixes #897, which will auto-close the issue on merge, while the PR (and docs/testing.md) states this is diagnostics only and the new xfail rows point at #897 as the open bug. Refs #897 / Part of #897 would keep it open.
tests/e2e-cucumber/features/model_serving.feature:78 still mentions the old ticket id in a comment; worth pointing it at #897.
docs/testing.md (around the "This change only adds harness diagnostics..." sentence) reads like a changelog and hardcodes current signatures and scenario ids. Describing the standing behaviour and linking #897 should age better.
A few new branches have no unit coverage: the 512 MiB hash limit, MAX_CANDIDATES truncation, and the 20 s budget early return.

More detail in the inline comments. Happy to re-check once the sign-off is sorted.

🤖 by agenthub on AMD AgentHub

jussielo-amd · 2026-11-05T09:04:22Z

 when = { effective_engine = "lemonade", os = "linux" }
-bug = "EAI-7423"
-reason = "Lemonade managed serve reaches ready then shuts down immediately, so the endpoint never becomes reachable."
+bug = "ROCm/rocm-cli#897"


Non-blocking: the stale-xfail hint script (scripts/xfail_expectations_hint.py) matches bug strings verbatim against refs found in PR text, and its issue pattern is (?<![\w/])#\d+, so ROCm/rocm-cli#897 can never match a later "Fixes #897". Its self-test uses the plain bug = "#123" form. Using bug = "#897" (here, in the other rows, and in the new unit test in expectation.rs) keeps the hint working for these rows; the repo name can stay in the comment text.

Addressed in 1577abe: all eight xfail rows and the regression assertion use plain #897. The hint script now recognizes the actual rows when run with Refs #897.

jussielo-amd · 2026-11-05T09:04:24Z

        stop_status: &stop_status,
-    })
+    });
+    format!("{report}\n--- Lemonade backend evidence ---\n{backend_evidence}")


Non-blocking: this appends a "Lemonade backend evidence" header to every failed serve report, so vLLM/AIM failures end with "not a Lemonade serve", which is noise and could suggest Lemonade was involved. Could we append the section only when engine == "lemonade" and return the report unchanged otherwise?

Addressed in 1577abe: only Lemonade attempts supply optional backend evidence. Other engines retain the base report unchanged. The report regression covers both forms and fails if the section becomes unconditional.

jussielo-amd · 2026-11-05T09:04:27Z

            "{}",
            serve_failure_evidence(
                world,
+                selected_engine(&stdout),


Non-blocking: selected_engine panics when there is no engine: line, and it is evaluated here before serve_failure_evidence runs. In that case the panic hides the evidence this PR is meant to capture and skips stopping the service. A non-panicking lookup (e.g. stdout.lines().find_map(|l| l.trim().strip_prefix("engine:")).map_or("", str::trim)) would keep the failure path safe.

Addressed in 1577abe: the failure path uses a non-panicking engine lookup with an empty fallback, allowing evidence collection and service cleanup without an engine: line. Added missing, blank, and valid-plan lookup coverage.

jussielo-amd · 2026-11-05T09:04:29Z

+        "{status}\nstdout:\n{}\nstderr:\n{}{}",
+        String::from_utf8_lossy(&out),
+        String::from_utf8_lossy(&err),
+        if out.len() == MAX_EVIDENCE_BYTES as usize / 2


Non-blocking: the retained limit per stream is MAX_EVIDENCE_BYTES / 2 (32 KiB), but the marker (and docs/testing.md) say "64 KiB", and the len == 32768 check also fires for output that is exactly 32 KiB without being truncated. Having drain_output return whether it actually truncated, and naming the stream and real limit in the marker, would make this accurate. Worth noting in the docs too that the per-report total is larger (up to 8 candidates x 2 probes).

Addressed in 1577abe: capture tracks discarded bytes per stream, including when cancelled by a timeout. Markers name stdout/stderr and the 32 KiB limit; exact-boundary output is not marked. Tests cover empty/below/exact/over-limit output and cancellation. Docs describe the aggregate sixteen-probe allowance.

jussielo-amd · 2026-11-05T09:04:31Z

+            "candidate (not proof of selection): {}",
+            server.display()
+        );
+        match fingerprint(server) {


Non-blocking: fingerprint does synchronous file reads and hashing (up to 512 MiB per candidate, up to 8 candidates) inside an async fn. That blocks the executor thread and sits outside the "time-bounded (20 s total)" claim in the docs. Consider spawn_blocking under the remaining budget and/or a much smaller limit (a real llama-server is far below 512 MiB), and wording the docs to say what is actually bounded.

Addressed in 1577abe: hashing runs in spawn_blocking, rejects files over 32 MiB, and checks the shared deadline between 64 KiB reads inside the worker. The worker is awaited rather than abandoned on a JoinHandle timeout. Docs explicitly exclude non-preemptible filesystem operations and cleanup from a hard wall-clock guarantee; cap and deadline regressions are covered.

jussielo-amd · 2026-11-05T09:04:33Z

+        )
+        .unwrap();
+        let start = std::time::Instant::now();
+        let report = probe(&server, "--help", Duration::from_millis(50)).await;


Non-blocking: with a 50 ms timeout this test needs the shell to start and print its PID before the timeout fires. On a loaded runner stdout: can be empty, and the .lines().next().unwrap().parse::<u32>().unwrap() chain below then panics. A longer timeout (e.g. 1 s; the < 5 s assertion still holds) or having the script write its PID to a file and polling for it would make it robust.

Addressed in 1577abe: increased the timeout to one second while retaining the under-five-second assertion and verification that the printed child PID is no longer running.

Copilot · 2026-11-05T09:25:08Z

Thanks for this, the diagnostics are well thought out: the backend list and manifest path match what the engine uses, and probes are read-only with good "not proof of selection" wording. One blocker, ...

1577abe updates the feature reference and standing docs, adds hash/candidate/budget boundary coverage, and addresses all six inline comments. All 214 support tests, strict workspace/Cucumber clippy, formatting and CLI smoke pass. Full workspace tests hit ten unchanged checkout-permission failures; local CodeQL skipped analysis for database size. These are harness-only changes, with no CLI behavior change.

Two publication fixes remain: this environment cannot force-push rewritten history or edit the generated PR suffix. A maintainer must repair the initial commit's sign-off and replace the closing reference with Refs #897. A history rewrite may affect the standing review depending on repository rules. Current-head CI also requires workflow approval.

Signed-off-by: Jussi Elo <jussi.elo@amd.com>

Signed-off-by: GitHub <noreply@github.com> Co-authored-by: jussielo-amd <194791304+jussielo-amd@users.noreply.github.com> Signed-off-by: Jussi Elo <jussi.elo@amd.com>

jussielo-amd

Thanks, this addresses the earlier points. Verified on the current head:

The commit sign-off check is green and the description now says Refs #897, so the issue stays open.
The xfail rows use plain #897; the Lemonade evidence is only appended for Lemonade; the engine lookup no longer panics; hashing is bounded (32 MiB cap, off the executor, deadline checked per read); truncation markers name the stream and limit; the timeout test uses 1s.
CI is green on this head.

Nothing blocking. A few small non-blocking notes are inline (engine detection for --engine=..., probe-report noise, a comment on the serve-40 xfail row). Also, the "Initial plan" commit title won't tell anyone much, so a squash on merge or a reword would be nice.

🤖 by agenthub on AMD AgentHub

jussielo-amd · 2026-11-05T10:11:40Z

+        serve_failure_evidence(
+            world,
+            args.windows(2)
+                .find(|pair| pair[0] == "--engine")


Non-blocking: this only recognises the exact two-token form --engine X, so --engine=lemonade (or a serve that fails before printing its plan, for the default-engine path) yields an empty engine and the Lemonade evidence is silently skipped, with nothing in the report saying so. A small helper next to reported_engine that accepts both forms (with a unit test), and an "engine unknown: backend evidence skipped" line when it is empty, would make that visible.

Addressed in fc1a144. A new engine_from_serve_args next to reported_engine accepts both --engine X and --engine=X (and treats a trailing or empty value as absent), and serve_and_wait falls back to the engine in the printed serve plan when the arguments name none, which covers the default-engine path once the plan was printed. When neither source yields an engine, the failure report now carries engine unknown: backend evidence skipped instead of silently omitting the section. engine_is_read_from_both_flag_forms_and_absent_otherwise covers both flag forms plus the missing, trailing, empty and near-miss (--engine-version=1) cases, and goes red if the --engine=X branch is removed. The report line itself is built in serving_steps.rs, which has no unit-test harness, so that one line is not asserted by a test.

Read the engine from both `--engine X` and `--engine=X`, fall back to the printed serve plan, and say so in the failure report when no engine can be determined instead of silently skipping the Lemonade backend inventory. Print probe capture and kill/reap results only when they fail, and note on the serve-40 expectation row that xfail plus flaky leaves that assertion without signal until #897 is closed. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

jussielo-amd · 2026-11-05T11:27:45Z

Superseded by #912, which carries the same change rebased onto current main and squashed to one commit under the maintainer's authorship. Review history stays here for reference.

Copilot AI assigned Copilot and jussielo-amd Nov 5, 2026

Copilot started work on behalf of jussielo-amd November 5, 2026 08:28

View session

Copilot AI linked an issue Nov 5, 2026 that may be closed by this pull request

E2E: lemonade serving fails on the Strix Halo Ubuntu lanes, with a different log signature depending on the run #897

Closed

Copilot AI changed the title ~~[WIP] Fix lemonade serving failure on Strix Halo Ubuntu lanes~~ Capture backend identity for Lemonade managed-serve failures Nov 5, 2026

Copilot finished work on behalf of jussielo-amd November 5, 2026 08:52

Copilot AI requested a review from jussielo-amd November 5, 2026 08:52

jussielo-amd requested changes Nov 5, 2026

View reviewed changes

Copilot started work on behalf of jussielo-amd November 5, 2026 09:05

View session

Copilot finished work on behalf of jussielo-amd November 5, 2026 09:25

Copilot AI requested a review from jussielo-amd November 5, 2026 09:25

Copilot AI and others added 4 commits November 5, 2026 09:27

Initial plan

e38b064

Signed-off-by: Jussi Elo <jussi.elo@amd.com>

test(e2e): capture Lemonade backend identity on serve failures

62b2cd4

Signed-off-by: GitHub <noreply@github.com> Co-authored-by: jussielo-amd <194791304+jussielo-amd@users.noreply.github.com> Signed-off-by: Jussi Elo <jussi.elo@amd.com>

test(e2e): keep async serve evidence futures Send

55220d7

Signed-off-by: GitHub <noreply@github.com> Co-authored-by: jussielo-amd <194791304+jussielo-amd@users.noreply.github.com> Signed-off-by: Jussi Elo <jussi.elo@amd.com>

test(e2e): harden Lemonade failure evidence bounds

1e55894

Signed-off-by: GitHub <noreply@github.com> Co-authored-by: jussielo-amd <194791304+jussielo-amd@users.noreply.github.com> Signed-off-by: Jussi Elo <jussi.elo@amd.com>

jussielo-amd force-pushed the copilot/fix-lemonade-serving-failure branch from 1577abe to 1e55894 Compare November 5, 2026 09:53

jussielo-amd approved these changes Nov 5, 2026

View reviewed changes

jussielo-amd marked this pull request as ready for review November 5, 2026 10:20

jussielo-amd requested a review from a team as a code owner November 5, 2026 10:20

jussielo-amd requested a review from tomastola November 5, 2026 10:20

jussielo-amd mentioned this pull request Nov 5, 2026

Capture backend identity for Lemonade managed-serve failures #912

Open

3 tasks

jussielo-amd closed this Nov 5, 2026

Conversation

Copilot AI commented Nov 5, 2026 • edited by jussielo-amd Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jussielo-amd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Nov 5, 2026

Uh oh!

jussielo-amd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jussielo-amd commented Nov 5, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Nov 5, 2026 •

edited by jussielo-amd

Loading