feat: Add Support Login Screen Recognition Over Serial over LAN by DawidBerk · Pull Request #51 · intel/mfd-connect

DawidBerk · 2026-10-05T15:40:17Z

Add Support Login Screen Recognition Over Serial over LAN

Copilot

Copilot review overview

🟡 Changes recommended

Prompt matching has several paths that miss standard login flows or produce false positives.

Review effort: Balanced
Findings: 4

Open (4)

Support qualified password prompts · New
Detect login prompts when stdout is discarded · New
Detect login prompts after sending Enter · New
Preserve Unix-first OS detection order · New

What changed in this PR

Adds Serial over LAN login-screen recognition to prevent OS probes from being sent to interactive authentication prompts.

Changes:

Adds login-prompt patterns and detection paths.
Introduces a dedicated compatibility exception.
Expands unit coverage for prompt detection and retries.

File	Description
`mfd_connect/sol.py`	Implements prompt detection in SOL command and OS-discovery flows.
`mfd_connect/exceptions.py`	Adds `SolLoginPromptDetected`.
`tests/unit/test_mfd_connect/test_sol.py`	Tests prompt recognition and command retry behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

coveralls-official · 2026-10-05T15:44:58Z

coverage: 81.047% (+0.2%) from 80.829% — DawidBerk:task_MFD_9418 into intel:main

tjankows-itp · 2026-10-09T12:10:26Z

    """Raises when OS is not supported."""


+class SolLoginPromptDetected(OsNotSupported):


The Serial-over-LAN (SoL) is only a connection, by which the users gets to the text console of the SUT.
I would name this exception TextConsoleLoginPromptDetected because for graphic-mode log-in there is no output on SoL whatsoever...

tjankows-itp · 2026-10-09T12:56:18Z

        expected_return_codes: Optional[Iterable] = frozenset({0}),
        shell: bool = True,
        custom_exception: Type[CalledProcessError] = None,
+        detect_login: bool = False,


I am not sure if enhancing SolConnection.execute_command(...) to do two distinctively different things is a good idea. After all this method handles sending a text command over SoL, and returns the response without any interpretation.

In a test-framework you do need method like this. After all you never know, if the output would include strings like "log-in" or "password" by pure coincidence.

I would create a thin-wrapper for SolConnection.execute_command(...) method that would use the execute_command internally, but it would also raise exception if the output was anything but SUCCESS. A detection of the log-in prompt screen would be one of these possible exceptions.

E.g. SolConnection.execute_command_expect_success(...)

Implemented

Copilot

🟡 Changes recommended

Prompt fragmentation can evade detection, and the success wrapper cannot validate exit status.

2 open findings

4 resolved since last review

🧠 Review effort: Balanced

Copilot

🟡 Changes recommended

Prompt matching can falsely detect historical output and miss prompts fragmented across serial reads.

1 open finding

Anchor prompt matching to the end of the complete buffer · New

2 resolved since last review

🧠 Review effort: Balanced

tjankows-itp · 2026-10-14T14:04:02Z

+        :raises ConnectionCalledProcessError: if program exits with an unexpected return code (shell=True only)
+        :raises TextConsoleLoginPromptDetected: if an interactive login prompt is detected in the response
+        """
+        if discard_stdout:


The user may not be interested in the output of the command, despite the fact that the method processes it internally.

In this case, simply discard it instead of returning. There is no need for raising an exception.

Without the output there is nothing to check the login prompt against - discard_stdout=True would make this method silently skip its whole purpose (login-prompt detection) without telling the caller. Raising an exception here prevents a false positive.

tjankows-itp · 2026-10-14T14:20:34Z

                raise custom_exception(returncode=returncode, cmd=command, output=output)
            raise ConnectionCalledProcessError(returncode=returncode, cmd=command, output=output)

+    def execute_command_expect_success(


The name suggest that this method runs a command and expects it to be completed successfully. This is not the case, as SolConnection.execute_command(...) is also used for sending commands that are expected to fail.

I would rename it to SolConnection.execute_command_detect_login(...)

One more thing:

The reason for the execute_command(...) method to maintain the same interface (same parameters) despite being a method of various classes (e.g. SolConnection, SSHConnection or RShellConnection among others) is to allow the user to seamlessly replace the connection type (class) without the need for changing a single method call.

If you introduce SolConnecition.execute_command_detect_login(...), you break this symmetry anyway, as no other class has a counterpart for it, and there is no abstract method in Connection class either.

To fix this you would need to add an abstract method to the Connection base class to signal the need for implementing it for other types of connection as well.

We could make this a private method so that we don't have to implement an abstract method in Connection/base.py

adrianlasota

The best should be adding support for login to that system. SerialConnection handles that, so it can be added here too.

adrianlasota · 2026-10-15T09:45:11Z

+        custom_exception: Type[CalledProcessError] = None,
+    ) -> "ConnectionCompletedProcess":
+        """
+        Run program and wait for its completion, raising if the outcome is not a SUCCESS.


I don't get that method. before each execute command there is _raise_if_login_prompt means it won't go through the login prompt. What's the case where you are checking login again?

_raise_if_login_prompt() only reads what is already in the buffer - it doesn't send anything. If the login prompt is not printed again, the buffer can be empty and the check passes, even if the prompt is there.

To detect it for sure, we need to send a command and check the response. That's why execute_command_detect_login() checks the output after sending the command - it's the only way to reliably catch the prompt.

We could make this a private method so that we don't have to implement an abstract method in Connection/base.py, as Tomasz suggested above.

Signed-off-by: Berk, Dawid <dawidx.berk@intel.com>

adrianlasota · 2026-10-15T14:52:53Z

                raise custom_exception(returncode=returncode, cmd=command, output=output)
            raise ConnectionCalledProcessError(returncode=returncode, cmd=command, output=output)

+    def execute_command_detect_login(


I suggest to do it always on the start of connection. I don't see the purpose to always "detect" login. If you need to do it should be done in your logic.

I don't see response for login feature request.

The flow should be:
connect, check if need to login, login or raise exception (your case?), execute command.

but it should be done in logic, here you should give just implementation of each part

Copilot AI balanced review requested due to automatic review settings October 5, 2026 15:40

Copilot started reviewing on behalf of DawidBerk October 5, 2026 15:40

View session

Copilot AI reviewed Oct 5, 2026

View reviewed changes

Comment thread mfd_connect/sol.py Resolved

Comment thread mfd_connect/sol.py Resolved

Comment thread mfd_connect/sol.py Resolved

Comment thread mfd_connect/sol.py Resolved

DawidBerk force-pushed the task_MFD_9418 branch from 74677da to c714f5c Compare October 5, 2026 15:44

DawidBerk marked this pull request as draft October 5, 2026 15:51

DawidBerk force-pushed the task_MFD_9418 branch from c714f5c to 8f668af Compare October 6, 2026 09:51

DawidBerk marked this pull request as ready for review October 6, 2026 10:16

tjankows-itp suggested changes Oct 9, 2026

View reviewed changes

DawidBerk force-pushed the task_MFD_9418 branch from 8f668af to 15b3644 Compare October 13, 2026 09:49

DawidBerk marked this pull request as draft October 13, 2026 09:49

DawidBerk force-pushed the task_MFD_9418 branch 2 times, most recently from 7cc65b0 to e38ba90 Compare October 14, 2026 08:37

DawidBerk marked this pull request as ready for review October 14, 2026 08:38

DawidBerk requested review from tjankows-itp and a balanced review from Copilot October 14, 2026 08:54

Copilot started reviewing on behalf of DawidBerk October 14, 2026 08:55

View session

Copilot AI reviewed Oct 14, 2026

View reviewed changes

Comment thread mfd_connect/sol.py Resolved

Comment thread mfd_connect/sol.py Resolved

DawidBerk force-pushed the task_MFD_9418 branch from e38ba90 to 133c42d Compare October 14, 2026 09:24

DawidBerk requested a balanced review from Copilot October 14, 2026 09:27

Copilot started reviewing on behalf of DawidBerk October 14, 2026 09:27

View session

Copilot AI reviewed Oct 14, 2026

View reviewed changes

Comment thread mfd_connect/sol.py Resolved

DawidBerk force-pushed the task_MFD_9418 branch from 133c42d to 2a704a7 Compare October 14, 2026 09:35

tjankows-itp suggested changes Oct 14, 2026

View reviewed changes

abaczek previously approved these changes Oct 15, 2026

View reviewed changes

adrianlasota requested changes Oct 15, 2026

View reviewed changes

feat: Add Support Login Screen Recognition Over Serial over LAN

d128525

Signed-off-by: Berk, Dawid <dawidx.berk@intel.com>

DawidBerk dismissed abaczek’s stale review via d128525 October 15, 2026 13:21

DawidBerk force-pushed the task_MFD_9418 branch from 2a704a7 to d128525 Compare October 15, 2026 13:21

DawidBerk requested a review from adrianlasota October 15, 2026 13:22

DawidBerk requested a review from tjankows-itp October 15, 2026 13:22

adrianlasota requested changes Oct 15, 2026

View reviewed changes

		"""Raises when OS is not supported."""


		class SolLoginPromptDetected(OsNotSupported):

Conversation

DawidBerk commented Oct 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coveralls-official Bot commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

adrianlasota 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!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

coveralls-official Bot commented Oct 5, 2026 •

edited

Loading