Skip to content

Interactive verbs explain non-interactive consoles instead of silent no-ops (#2097) - #2098

Merged
erikdarlingdata merged 1 commit into
devfrom
noninteractive-guidance-2097
Aug 7, 2026
Merged

Interactive verbs explain non-interactive consoles instead of silent no-ops (#2097)#2098
erikdarlingdata merged 1 commit into
devfrom
noninteractive-guidance-2097

Conversation

@erikdarlingdata

Copy link
Copy Markdown
Owner

Closes #2097 — implemented essentially as the reporter suggested.

Root cause (as diagnosed in the report, verified)

On non-interactive hosts (ISE, PSRemoting, redirected stdin), Console.ReadLine() returns null instantly, and both verbs put their prompts and failure messages on stderr — which those hosts don't surface. --encrypt-password (the very first setup step) read as a hung tool; --configure-network printed a misleading "No changes made."

Fix

  • Shared WriteNonInteractiveGuidance writes the explanation to stdout — cause + both remedies (real console, or Read-Host -Prompt 'password' | … for single-value verbs).
  • --encrypt-password: null/empty read → guidance + exit 1 (stderr line kept for real consoles).
  • Wizard menu: EOF ≠ quit — null → guidance + exit 1; explicit q/empty keeps the quiet "No changes made." + exit 0. (Mid-wizard EOFs already print visible "Cancelled" lines on stdout.)
  • Blob-on-stdout piping (> blob.txt) unaffected — guidance only appears on the failure path where stdout carries no blob.

Test

Drives the wizard with an exhausted reader (the exact ISE shape): exit 1 + guidance on stdout; and pins that explicit q stays quiet with exit 0.

🤖 Generated with Claude Code

…no-ops (#2097)

In the PowerShell ISE, remote sessions, and redirected stdin,
ReadLine() returns null immediately and stderr -- where these verbs put
their prompts and failure lines -- is not surfaced at all. So
--encrypt-password (the FIRST setup step) read as a hung tool, and the
--configure-network wizard bailed with a misleading 'No changes made.'

Both now write actionable guidance to STDOUT naming the cause and the
paths forward (real console, or pipe the value in). The wizard tells
EOF apart from an explicit quit: guidance + exit 1 vs the quiet
'No changes made.' + exit 0. Test drives the wizard with an exhausted
reader (the exact ISE shape) and pins both behaviors.

Closes #2097

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment on lines 57 to 65
var plaintext = Console.ReadLine();
if (string.IsNullOrEmpty(plaintext))
{
/* #2097: on a non-interactive host (ISE/remote/redirected stdin) ReadLine returns null instantly
and stderr is invisible — the guidance must ride STDOUT, the stream every host shows. */
Console.Error.WriteLine("No password read from stdin.");
DarlingCliCommands.WriteNonInteractiveGuidance(Console.Out);
return 1;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness: --encrypt-password conflates EOF with an empty interactive answer, so the new guidance can lie to a real console user.

var plaintext = Console.ReadLine();
if (string.IsNullOrEmpty(plaintext))
{
    Console.Error.WriteLine("No password read from stdin.");
    DarlingCliCommands.WriteNonInteractiveGuidance(Console.Out);
    return 1;
}

Console.ReadLine() returns null on EOF (the actual non-interactive signal) but returns "" when an interactive user just presses Enter at the Password: prompt without typing anything. string.IsNullOrEmpty treats both the same, so a real, interactive operator who hits Enter by mistake now gets told:

"This console appears to be non-interactive (PowerShell ISE, a remote/PSRemoting session, or redirected stdin)..."

which is false in that case and exactly the kind of misdiagnosis this PR is fixing elsewhere.

This PR gets the distinction right in ConfigureNetworkAsync (DarlingCliCommands.cs), which explicitly checks choice is null for EOF separately from choice.Length == 0 for a deliberate empty/quit answer — and the Prompt helper's own doc comment says as much: "Returns null on EOF (input exhausted); an empty line yields defaultValue (or "")." This verb should follow the same split, e.g.:

if (plaintext is null)
{
    Console.Error.WriteLine("No password read from stdin.");
    DarlingCliCommands.WriteNonInteractiveGuidance(Console.Out);
    return 1;
}
if (plaintext.Length == 0)
{
    Console.Error.WriteLine("No password read from stdin.");
    return 1;
}

Worth noting this path also isn't covered by the new test in DarlingCliCommandsTests.cs — that test only drives ConfigureNetworkAsync through its injectable TextReader/TextWriter, while this verb reads Console.ReadLine() directly in Program.cs and has no equivalent regression test.

@claude

claude Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review summary

Reviewed the diff (CHANGELOG.md, Darling/PerformanceMonitor.Darling.Service/{Program.cs,DarlingCliCommands.cs}, Darling/Darling.Tests/DarlingCliCommandsTests.cs).

Scope check: this only touches Darling (interactive CLI wizard for darling.json/network config). Lite has no equivalent interactive verb/wizard, so there's no Lite/Darling parity drift here — nothing to mirror on the Lite side.

What's solid:

  • ConfigureNetworkAsync's EOF-vs-explicit-quit split (choice is null → guidance + exit 1, vs choice.Length == 0 || "q" → quiet "No changes made." + exit 0) is correct and well-tested.
  • WriteNonInteractiveGuidance writing to stdout instead of stderr correctly targets the actual problem (ISE/PSRemoting/redirected-stdin hosts don't surface stderr).
  • CHANGELOG entry is accurate and follows the existing format/attribution convention.

One correctness bug found (see inline comment on Program.cs): the --encrypt-password verb uses string.IsNullOrEmpty(plaintext) to gate the new non-interactive guidance, which conflates ReadLine() returning null (true EOF/non-interactive) with it returning "" (a real interactive user who just pressed Enter at the prompt). That means an interactive operator who accidentally submits a blank password now gets told their console "appears to be non-interactive," which is false — the same misdiagnosis class this PR is fixing for the wizard, just left unfixed on this path. The wizard code and the Prompt() helper's own doc comment already establish the null-vs-empty convention this verb should follow. This path also has no regression test (unlike the wizard, Program.cs's Console.ReadLine() call isn't behind an injectable TextReader), so the gap wouldn't be caught automatically.

@erikdarlingdata
erikdarlingdata merged commit 3022f3a into dev Aug 7, 2026
5 checks passed
@erikdarlingdata
erikdarlingdata deleted the noninteractive-guidance-2097 branch August 7, 2026 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant