Skip to content

Fix script output dropping last line without trailing newline#4995

Open
denik wants to merge 6 commits into
mainfrom
denik/random-bugfixes-4
Open

Fix script output dropping last line without trailing newline#4995
denik wants to merge 6 commits into
mainfrom
denik/random-bugfixes-4

Conversation

@denik
Copy link
Copy Markdown
Contributor

@denik denik commented Apr 16, 2026

Changes

Fix experimental.scripts hooks silently dropping the last line of output when it doesn't end with a trailing newline (e.g. printf "done").

Tests

Added acceptance test at acceptance/bundle/scripts/no-trailing-newline/ and unit tests for the script executor.

Running the acceptance test against the previous release demonstrates the bug:

$ go test ./acceptance -run 'TestAccept/bundle/scripts/no-trailing-newline' -useversion 0.296.0

--- Expected
+++ Actual (v0.296.0)
@@ -5,3 +5,2 @@
 line 2
-line without newline
 Name: scripts_no_trailing_newline

v0.296.0 drops "line without newline" because the last line has no trailing \n. The fix correctly includes it.

denik added a commit that referenced this pull request Apr 16, 2026
denik added a commit that referenced this pull request Apr 22, 2026
@denik denik force-pushed the denik/random-bugfixes-4 branch from a86395c to f43f4d2 Compare April 22, 2026 16:10
denik added a commit that referenced this pull request Apr 29, 2026
denik added a commit that referenced this pull request Apr 29, 2026
Drop the placeholder Bundles changelog line that was never replaced when
the PR-linked entry was added, leaving a single entry referencing #4995.
Also remove TestExecuteNoScript which only re-exercises the trivial
getCommand early-return without adding coverage for the fix.

Task: 007.md
Task-review: /home/denis.bilenko/work/prompts-features/cli/random-bugfixes-4/006.SUMMARY.md

Co-authored-by: Isaac
denik added a commit that referenced this pull request Apr 29, 2026
@denik denik force-pushed the denik/random-bugfixes-4 branch from f43f4d2 to 7802668 Compare April 29, 2026 09:50
@denik denik temporarily deployed to test-trigger-is April 29, 2026 09:50 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is April 29, 2026 09:50 — with GitHub Actions Inactive
@denik denik enabled auto-merge May 22, 2026 13:52
@denik denik temporarily deployed to test-trigger-is May 22, 2026 13:52 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is May 22, 2026 13:52 — with GitHub Actions Inactive
@eng-dev-ecosystem-bot
Copy link
Copy Markdown
Collaborator

Commit: 966f67f

Run: 26291825165

denik added 5 commits May 26, 2026 10:38
bufio.Reader.ReadString returns both data and io.EOF when the stream
ends without a delimiter. The old loop condition `for err == nil` caused
the last line of output to be silently dropped when it lacked a trailing
newline. This affected any bundle script (experimental.scripts hooks)
whose output didn't end with `\n`.

Restructure the loop to always process data before checking for errors,
which is the idiomatic Go pattern for ReadString.

Task: 001.md

Co-authored-by: Isaac
This test demonstrates the bug where the last line of script output
was dropped when it didn't end with a trailing newline. Running this
test with -useversion 0.296.0 shows the bug: "line without newline"
is missing from the output.

Task: 002.md

Co-authored-by: Isaac
Drop the placeholder Bundles changelog line that was never replaced when
the PR-linked entry was added, leaving a single entry referencing #4995.
Also remove TestExecuteNoScript which only re-exercises the trivial
getCommand early-return without adding coverage for the fix.

Task: 007.md
Task-review: /home/denis.bilenko/work/prompts-features/cli/random-bugfixes-4/006.SUMMARY.md

Co-authored-by: Isaac
When a test script is killed by the timeout, the test directory holds
partial state: any "out.*" files reflect an incomplete recording and
output.txt contains only the prefix that streamed before the kill.
Previously `make test-update` would overwrite committed reference files
with this partial state and emit untracked "out.requests.txt" files
across cluster, deploy, and template tests.

Wrap the timeout error with the errScriptTimedOut sentinel and gate
OverwriteMode on errors.Is matching it. Also short-circuit the
unexpected-files loop so leftover partial files are not flagged or
written. Existing comparisons still run, so selftest/timeout keeps
passing.

Task: 009.md

Co-authored-by: Isaac
@denik denik force-pushed the denik/random-bugfixes-4 branch from 966f67f to e0c91f9 Compare May 26, 2026 10:40
@denik denik requested a deployment to test-trigger-is May 26, 2026 10:41 — with GitHub Actions Abandoned
@denik denik requested a deployment to test-trigger-is May 26, 2026 10:41 — with GitHub Actions Abandoned
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.

3 participants