fix(ssh-console): fall back for Lenovo SOL activation

[williampnvidia]

Summary

• keep the existing Lenovo SOL activation command as the primary path
• detect the SR650 v4 Lenovo extraneous arguments response and fall back to console kill / console start
• add focused unit tests for the Lenovo fallback decision and existing non-Lenovo activation behavior

Fixes #2919

Test plan

• cargo +nightly-2026-06-16 fmt --all -- --check
• git diff --check fork/main..HEAD
• cargo test -p carbide-ssh-console --lib lenovo_primary_failure
• cargo test -p carbide-ssh-console --lib non_lenovo_activation
• cargo test -p carbide-ssh-console --lib

Note: William ran the cargo tests on Linux. Local macOS test execution is blocked by existing Linux-specific io_util.rs termios compile errors.

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[coderabbitai]

Summary by CodeRabbit

• New Features

  • Added support for Lenovo SR650 SSH-based serial console activation, including automatic fallback handling when the primary activation path fails.
  • Expanded SSH console behavior to recognize Lenovo-specific prompts and command flows.

• Bug Fixes

  • Improved console activation reliability by retrying with vendor-specific fallback commands and better handling of activation output timing.
  • Updated acceptance logic so the console can correctly detect when activation has succeeded across different vendor responses.

Walkthrough

Adds Lenovo SR650 SOL fallback handling in SSH activation, updates the mock BMC prompt flow for Lenovo, and extends test scaffolding plus unit and integration coverage for the new command sequence.

Changes

Lenovo SOL fallback activation flow

Layer / File(s)	Summary
Vendor constants and predicates crates/ssh-console/src/bmc/vendor.rs	Adds Lenovo SOL failure markers, fallback commands, byte-search support, and vendor methods for fallback selection and activation-output acceptance.
SSH activation loop and fallback sequencing crates/ssh-console/src/bmc/connection_impl/ssh.rs	Updates the SOL activation loop to track fallback state, send commands bytewise, retry with Lenovo fallback commands, and use vendor-specific activation-output acceptance.
Mock SSH server and test wiring crates/machine-a-tron/src/mock_ssh_server.rs, crates/ssh-console/tests/util/mod.rs, crates/ssh-console/tests/main.rs	Adds Lenovo mock prompt behavior, updates SSH test environment classification and host metadata, and adds an integration test for the Lenovo SR650 fallback path.
Vendor unit tests crates/ssh-console/src/bmc/vendor.rs	Adds unit-test fixtures and cases covering Lenovo fallback selection and activation-output acceptance.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the Lenovo SOL fallback change.
Description check	✅ Passed	The description matches the Lenovo fallback and test changes in the patch.
Linked Issues check	✅ Passed	The implementation preserves the primary Lenovo path, adds SR650 v4 fallback, and covers the required test cases.
Out of Scope Changes check	✅ Passed	The mock server and test harness updates support the Lenovo fallback flow and stay within the issue scope.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

crates/ssh-console/src/bmc/connection_impl/ssh.rs (2)

504-511: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the byte-wise command transmission to remove the duplicated send loop.

This block is a verbatim copy of the primary activation send at lines 474-481 (per-byte data() plus trailing newline and identical ConsoleActivateError::Request mapping). Consolidating into a small async helper keeps the two paths from drifting.

♻️ Proposed helper

async fn send_command_bytewise(
    ssh_client_channel: &mut Channel<russh::client::Msg>,
    command: &[u8],
) -> Result<(), ConsoleActivateError> {
    for byte in command {
        ssh_client_channel
            .data([*byte].as_slice())
            .await
            .map_err(|error| ConsoleActivateError::Request {
                phase: "sending serial activate command to BMC",
                error,
            })?;
    }
    ssh_client_channel
        .data(b"\n".as_slice())
        .await
        .map_err(|error| ConsoleActivateError::Request {
            phase: "sending data to BMC",
            error,
        })
}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ssh-console/src/bmc/connection_impl/ssh.rs` around lines 504 - 511,
The byte-wise SSH command send logic is duplicated between the main activation
path and this serial activation block, so extract it into a shared async helper
like send_command_bytewise in ssh.rs and reuse it here. Keep the existing
per-byte data() behavior, trailing newline send, and
ConsoleActivateError::Request mapping, but centralize the loop so both
activation paths call the same helper and cannot drift.

---

658-707: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consolidate these predicate tests into a value_scenarios! table and add the missing prompt-wait case.

Two items here:

1. should_run_lenovo_sol_fallback and should_accept_sol_activation_output are total operations returning bool, which is precisely the shape the style guide targets with value_scenarios! / check_values. The three should_accept_sol_activation_output assertions (tests 1, 3, 4) in particular collapse cleanly into a single labeled table, making each branch one row. As per coding guidelines, "Prefer table-driven tests for any function that maps inputs to outputs, errors, or other observable results."
2. Issue #2919's acceptance criteria require a test covering "waiting for the prompt between console kill and console start." No test exercises that behaviour — which is consistent with the batched-command concern raised at lines 501-511. Once the prompt-wait question is settled, please add coverage for it.

Want me to draft the value_scenarios! table and the prompt-wait test?

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ssh-console/src/bmc/connection_impl/ssh.rs` around lines 658 - 707,
The predicate tests in the Lenovo SOL activation block should be consolidated
into a table-driven `value_scenarios!`/`check_values` style test, since
`should_run_lenovo_sol_fallback` and `should_accept_sol_activation_output` are
boolean-valued helpers. Move the existing `should_accept_sol_activation_output`
cases into labeled rows and keep `should_run_lenovo_sol_fallback` covered in the
same style if possible, using the existing function names to locate the
assertions. Also add the missing prompt-wait scenario for the Lenovo
batched-command flow, specifically covering the case where `console kill` is
followed by a prompt before `console start`, so the acceptance behavior is
exercised.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/ssh-console/src/bmc/connection_impl/ssh.rs`:
- Around line 501-511: The Lenovo fallback in ssh_client_channel activation is
batching console kill and console start together, which skips the required
re-sync on system> and can fail on SR650 v4. Update the Lenovo fallback path in
the ssh.rs activation flow to send console kill first, wait for the system>
prompt to be observed, then send console start as a separate prompt-synced
request. Also tighten the lenovo_fallback_sent success handling so it only
treats activation as successful when the expected prompt/state is reached and
the Lenovo failure marker is not present in the response.

---

Nitpick comments:
In `@crates/ssh-console/src/bmc/connection_impl/ssh.rs`:
- Around line 504-511: The byte-wise SSH command send logic is duplicated
between the main activation path and this serial activation block, so extract it
into a shared async helper like send_command_bytewise in ssh.rs and reuse it
here. Keep the existing per-byte data() behavior, trailing newline send, and
ConsoleActivateError::Request mapping, but centralize the loop so both
activation paths call the same helper and cannot drift.
- Around line 658-707: The predicate tests in the Lenovo SOL activation block
should be consolidated into a table-driven `value_scenarios!`/`check_values`
style test, since `should_run_lenovo_sol_fallback` and
`should_accept_sol_activation_output` are boolean-valued helpers. Move the
existing `should_accept_sol_activation_output` cases into labeled rows and keep
`should_run_lenovo_sol_fallback` covered in the same style if possible, using
the existing function names to locate the assertions. Also add the missing
prompt-wait scenario for the Lenovo batched-command flow, specifically covering
the case where `console kill` is followed by a prompt before `console start`, so
the acceptance behavior is exercised.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8077fa10-a8fb-4b4c-ae3e-d4b44c6f0b07

📥 Commits

Reviewing files that changed from the base of the PR and between 45cb7a1 and a0fbbbd.

📒 Files selected for processing (1)

• crates/ssh-console/src/bmc/connection_impl/ssh.rs

[github-actions]

🔍 Container Scan Summary

Service	Total	Critical	High	Medium	Low	Other
boot-artifacts-aarch64	3	0	0	3	0	0
boot-artifacts-x86_64	3	0	0	3	0	0
forge-admin-cli-x86_64	288	6	26	105	7	144
machine-validation-runner	751	30	190	274	36	221
machine_validation	751	30	190	274	36	221
machine_validation-aarch64	751	30	190	274	36	221
nvmetal-carbide	751	30	190	274	36	221
TOTAL	3298	126	786	1207	151	1028

Per-CVE detail lives in the per-service grype-* artifacts (JSON + SARIF). Severity counts only — no CVE IDs published here.

[kensimon]

Ideally we should be passing metadata about the model number in the BmcMetaDataGetResponse that lets ssh-console get this right the first time, but until that happens this is probably ok.

I think we ought to make the fallback logic more general-purpose though, with the lenovo-specific behavior placed in SshVendor.

Also, could you add an integration test to assert on this behavior? The tests in crates/ssh-console/tests/main.rs are at the level where they should be able to mock this. run_baseline_test_environment in crates/ssh-console/tests/util/mod.rs configures a mock SSH server from machine-a-tron with behavior from the PromptBehavior enum which currently has Dell and Dpu variants... you could create a third for LenovoSr650, and modify the data handler to check for that variant and override the behavior accordingly.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/machine-a-tron/src/mock_ssh_server.rs`:
- Around line 335-344: The Lenovo mock in mock_ssh_server.rs currently accepts
PromptBehavior::LenovoSr650 "console start" directly from ConsoleState::Bmc, so
the prompt-wait behavior is not actually enforced. Add an intermediate Lenovo
fallback state in the console state machine inside the SSH command handling so
"console start" is only accepted after the "console kill" response has been
emitted and the prompt has returned, and keep the state transition logic in the
console kill/start branches aligned with that flow. Update or add a focused test
around the LenovoSr650 path to verify that "console start" is rejected until the
prompt-return step has completed.

In `@crates/ssh-console/src/bmc/vendor.rs`:
- Around line 545-613: Add a positive Lenovo success path to the focused tests
so the SR650 fallback behavior is not the only Lenovo coverage. Update
fallback_serial_activate_commands_if_needed_detects_lenovo_failure and
should_accept_sol_activation_output_handles_fallback_cases to include a Lenovo
case where the primary serial activation output is successful and no fallback is
triggered, using SshBmcVendor::Lenovo and the existing helper methods like
serial_activate_command, bmc_prompt,
fallback_serial_activate_commands_if_needed, and
should_accept_sol_activation_output to keep the assertions aligned with the
current test structure.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: be89e23b-ca38-4cd0-83e7-6b27116dfe31

📥 Commits

Reviewing files that changed from the base of the PR and between a0fbbbd and 1ace7f7.

📒 Files selected for processing (5)

• crates/machine-a-tron/src/mock_ssh_server.rs
• crates/ssh-console/src/bmc/connection_impl/ssh.rs
• crates/ssh-console/src/bmc/vendor.rs
• crates/ssh-console/tests/main.rs
• crates/ssh-console/tests/util/mod.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• crates/ssh-console/src/bmc/connection_impl/ssh.rs

[kensimon]

Thanks for the updates! Looks good.