fix(actions): emit trailing newline from node-stdout, unblocks read#29
Conversation
The diagnostic dispatch (stella/stdnum#run-25810975562, debug branch)
finally captured the actual failure path. The script reaches:
read -r PACKAGE_NAME PACKAGE_VERSION < <(node -e '
... process.stdout.write(name + "\t" + version);
' "${PKG_JSON_FILE}")
and then exits 1 via the EXIT trap — meaning `read` returned non-zero
and `set -e` killed the script.
Per `man bash`, `read` "returns 0 unless end-of-file is encountered."
The node script wrote `name\tversion` with no trailing newline, so
`read` hit EOF before finding a delimiter and returned 1 — even
though it correctly assigned both variables. `set -e` then exited
the script before reaching the subsequent `[[ -z PACKAGE_NAME ]]`
check that would have emitted a meaningful error.
A four-byte fix: append "\n" to the node-emitted string. Verified
locally — script now reaches the next gate (npm version check)
instead of silent-exiting.
The diagnostic instrumentation that surfaced this is NOT included
in this commit — it lives on a separate `debug/publish-trace` branch
and will be cleaned up after this lands.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the publish.sh script to ensure a trailing newline is included when outputting the package name and version from Node.js. This change prevents the bash read command from returning a non-zero exit code, which would otherwise terminate the script under set -e. Additionally, a ShellCheck suppression was added for template literals. The reviewer suggested using console.log with a template literal as a more idiomatic and readable alternative to process.stdout.write, providing a code suggestion to implement this improvement.
| // Trailing newline is required: bash `read` returns non-zero on EOF | ||
| // without a delimiter even when the variables were assigned. Under | ||
| // `set -e` that non-zero kills the script silently right here. | ||
| process.stdout.write((j.name ?? "") + "\t" + (j.version ?? "") + "\n"); |
There was a problem hiding this comment.
For writing line-based output to stdout in Node.js, it's more idiomatic and slightly more concise to use console.log(), which automatically appends a newline. You could also use a template literal to improve readability. This would also make the shellcheck disable comment on line 75 more accurate, as it mentions template literals.
| process.stdout.write((j.name ?? "") + "\t" + (j.version ?? "") + "\n"); | |
| console.log(`${j.name ?? ""}\t${j.version ?? ""}`); |
Replaces `process.stdout.write(... + "\\n")` with
`console.log(`${name}\\t${version}`)`. console.log appends the
required newline automatically, and the template literal makes the
shellcheck disable directive (which mentions template literals)
accurate. No functional change.
Verified locally — script still reaches the next gate.
Addresses gemini medium on PR #29.
|
CC on behalf of @jan-kubica Applied in latest commit. `console.log` + template literal is cleaner and now the shellcheck disable directive accurately matches what's in the line. Verified locally that the script still reaches the next gate. |
1 participant
Root cause (finally)
Three consecutive dispatches of stella/stdnum's release against `v0.0.1` exited 1 with no script output. After ruling out file-mode (#28), `NODE_AUTH_TOKEN` placeholder handling (#27), and a few other hypotheses, I instrumented `publish.sh` with `set -x` on a temporary branch. The trace from that diagnostic dispatch (stdnum run 25810975562) showed the script reaching:
```
++ node -e '...process.stdout.write(name + "\t" + version);' ${PKG_JSON_FILE}
##[error]Process completed with exit code 1.
```
`rm -f` is the EXIT trap — so the script exited cleanly via trap, but with code 1.
Per `man bash`, `read` returns 0 "unless end-of-file is encountered". The node script wrote `name\tversion` with no trailing newline, so `read` hit EOF before finding a delimiter and returned 1 — even though it correctly assigned both variables. `set -e` then exited the script before reaching the subsequent `[[ -z PACKAGE_NAME ]]` check that would have emitted a meaningful error.
Fix
A 4-byte change: append `"\n"` to the node-emitted string.
```diff
```
Test plan
The diagnostic instrumentation that surfaced this lives on a separate `debug/publish-trace` branch and will be cleaned up after this lands.
Apologies
Three failed dispatches in public. The earlier fixes (#26, #27, #28) were all real concerns, just not THIS concern. Should have instrumented earlier instead of iterating on hypotheses.