[feiiiiii5]

Summary

• thv run prompts for a required, non-secret registry variable with fmt.Scanln, which reads one whitespace-delimited token. A value containing a space fails the read with expected newline, the caller logs a warning and continues, and the workload starts without a variable the registry declares required: true. The unread remainder stays in the stream, so the next prompt reads a fragment of the previous answer.
• The non-secret branch now reads a whole line, with one bufio.Reader shared by every prompt in a Validate call. A required variable that cannot be read or is answered empty now returns an error with a --env/--secret hint instead of a warning.

Fixes #6780

Type of change

• Bug fix

API Compatibility

• This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

Nothing here touches the operator CRD schema.

Test plan

• go test -count=1 ./pkg/runner/ — ok
• go test -count=1 ./pkg/runner/ -run 'TestDetachedEnvVarValidator_Validate|TestCLIEnvVarValidator_PromptsRequiredEnvVars|TestPromptForEnvironmentVariable' — ok
• go vet ./pkg/runner/ — clean
• gofmt -l pkg/runner/ — no output

Not run: golangci-lint is not installed here, and the e2e/Kind/chainsaw suites. The change touches only pkg/runner/env.go and its test file.

Changes

File	Change
pkg/runner/env.go	Non-secret prompt reads a line through one shared bufio.Reader; a read error or empty answer for a required variable returns missing required environment variable NAME (pass it with --env NAME=<value> or --secret <name>,target=NAME); the same helper now backs DetachedEnvVarValidator; promptForEnvironmentVariable keeps io.EOF only when a line was read
pkg/runner/env_test.go	Table cases for spaces, two prompts, empty answer and EOF; assertions on the error text and io.EOF; a direct test that a partial line followed by a non-EOF read error is rejected

Does this introduce a user-facing change?

Yes. In a non-interactive run (thv run ... </dev/null in CI), before this branch a missing required variable printed the prompt, logged failed to read input ... EOF and started the workload without it. Now it exits, for example:

failed to validate environment variables: missing required environment variable FOO (pass it with --env FOO=<value> or --secret <name>,target=FOO): failed to read input: EOF

The secret branch changed the same way: a failed term.ReadPassword (for example inappropriate ioctl for device when stdin is not a TTY) used to warn and continue, and now aborts with the same hint. The secret branch still reads a whole line through term.ReadPassword; only its failure handling changed. No TTY check was copied from upgrade.go into thv run, so piping an answer still works and a required secret that the secrets manager can supply is still resolved before the prompt.

Implementation plan

Plan followed

1. Replace fmt.Scanln with bufio.Reader.ReadString('\n'), one reader per Validate call so a buffered line is not discarded between prompts.
2. Keep a value that arrives without a trailing newline (io.EOF with data). Reject any other read error even when a partial line came back, so a truncated required value cannot start the workload.
3. Treat "no value for a required variable" as an error, matching DetachedEnvVarValidator (pkg/runner/env.go:57), and use one helper so both validators say how to pass the value without a prompt.
4. Cover spaces, two prompts in one call, an empty answer, EOF before a value, a value with no trailing newline, and a partial line followed by a read error.

Special notes for reviewers

promptForEnvironmentVariable now takes a *bufio.Reader instead of constructing one per call. It is package-private, so no exported signature changes.

The DetachedEnvVarValidator error text also gains the hint; its old missing required environment variable: NAME shape had no test pinning the exact string.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-22T23:38:32.522723Z We couldn’t complete this review. Comment @codex review to try again.	32746da	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7b921247c5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[jhrozek]

Fix looks right, a few things around the non-interactive path.

[feiiiiii5]

All four points are in at 32746daa.

• Hint on both errors: a required variable that cannot be read, or is answered empty, now returns missing required environment variable NAME (pass it with --env NAME=<value> or --secret <name>,target=NAME); the read error is appended with %w. DetachedEnvVarValidator uses the same helper, so the two validators no longer differ in wording. The secret branch is covered because it shares the same post-prompt error path.
• Only EOF is allowed with data: if readErr != nil && (!errors.Is(readErr, io.EOF) || line == ""). A partial line followed by a non-EOF error is rejected. The new direct test feeds io.MultiReader(strings.NewReader("partial"), iotest.ErrReader(errors.New("boom"))) and asserts the read fails.
• Test names and assertions: the empty-pipe case is now end of input before any value is an error, with require.ErrorIs(t, err, io.EOF) and require.ErrorContains for the hint; the empty-answer case asserts the new message. The other table cases assert the hint too.
• Description: the user-facing section now has the before/after for </dev/null, including the secret branch (ReadPassword failure now aborts instead of warning), and the "secret branch deliberately not in scope" line is gone. It also says no TTY check was copied into thv run, so piped answers still work.

Run from the worktree: go test -count=1 ./pkg/runner/ → ok (1.56s), go vet ./pkg/runner/ clean, gofmt -l pkg/runner/ empty. golangci-lint is not installed here, so that one still has not run.

[codecov]

Codecov Report

❌ Patch coverage is 90.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.08%. Comparing base (22f8487) to head (7f01ef0).

Files with missing lines	Patch %	Lines
pkg/runner/env.go	90.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6781      +/-   ##
==========================================
+ Coverage   79.98%   80.08%   +0.09%     
==========================================
  Files         834      834              
  Lines       85190    85197       +7     
==========================================
+ Hits        68140    68226      +86     
+ Misses      17045    16966      -79     
  Partials        5        5              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.