Thanks for the review!

I updated the ssh connection logic to avoid disabling pipelining when use_tty is true (default). _is_tty_requested() now returns True only when the active become plugin requires a TTY (become.require_tty) or when explicit SSH args request a TTY (-t / RequestTTY=yes|force).

I also updated/added unit tests:

• has_tty respects become.require_tty
• regression: pipelining is not disabled solely due to default use_tty

So the reason use_tty was not inspected was due to the assumption that it would already be factored in the resulting arguments. If that assumption is incorrect and the arguments are not properly constructed, the whole check is problematic, it should only occur on finalized arguments.

So I don't think this fix goes in the right direction, as it will hide the real underlying problem.

@bcoca The PR is unrelated to use_tty now, I updated the title to reflect the current branch.

The underlying issue is specific to the ssh connection, and occurs here: https://github.com/ansible/ansible/blob/devel/lib/ansible/executor/task_executor.py#L1035-L1039. The ssh connection overrides has_tty and does not check whether the become plugin requires a tty.

I'm not sure what the issue is then, that code in TE just makes sure the configurations are in agreement.

If I'm reading the change correctly this basically overrides the user's conflicting configuration and gives priority to the become plugin?

Prior to 7290959, has_tty was true for the ssh connection. The changelog suggests the only reason this was done, was to disable pipelining by default:

• ssh connection plugin now overrides pipelining when a tty is requested.

If the become plugin requires a TTY, it should do the same thing imo. An alternative fix would be to remove the has_tty property (so it's always True again), so _is_tty_requested only used for automatically disabling pipelining. I think this might be slightly better, since become plugins can configure pipelining separately.

talked with @s-hertel and realized that we have several issues:

• ssh defaults to using tty (use_tty defaults to true), even though it should only do so if explicitly set on ssh connection or required by become plugin
• _is_tty_requesxted does not match the actual command build for it's detection, why adding check on use_tty 'worked'TM, but was still incorrect.
• missing integration tests for these paths

use_tty should be factored in _build_command and then switch _is_tty_requested to use _build_command, so both paths match. Also make the default for has_tty 'smart' and depend on pipelining and become plugin settings

@bcoca

I’ve dropped the become.require_tty check from _is_tty_requested() and instead aligned tty detection with the actual ssh command generation.