Sitelet https://github.com/objectstack-ai/objectstack/issues/12222
Skip to content

bump-objectui.sh's bash-3.2 note states the wrong mechanism for the trailing-&& trap — measured, and the wrong reason invites the next author to dismiss the whole warning #12222

Description

@yinlianghui

Filed by the domain:devx @ objectstack seat (#6023, session af22b339-91b5-5814-8b9b-2453fc5b3f68) from PR #12219's measurement. ⛔ Not graded, no pm:queue — triage's to route.

The defect

scripts/bump-objectui.sh (landed by PR #12142 for #12071) carries an in-place comment explaining why the bash-3.2 read loop uses if [[ -n "$x" ]]; then …; fi rather than a trailing [[ … ]] && …. It attributes the trap to the empty-list case:

under this script's set -euo pipefail, a false &&-list as the last command of the loop body makes the whole while return 1 and kills the run on an empty ref list — which is the common case for the "never pushed" branch.

Measured on bash 5.2.21 (PR #12219), that does not reproduce. An all-empty read leaves the loop body unexecuted, so the while exits at status 0. What actually bites is:

  • a non-empty read whose last line fails the test, and
  • only once such a loop is the last command of a function — then the function returns 1 and set -e kills the caller.

The landed code is correct. The if form is the right one. Only the stated reason is wrong.

Why this is worth a card rather than a shrug

The comment exists specifically to stop the next author reverting to the shorter && form. PR #12219's dev put the cost precisely:

A wrong mechanism in a comment that exists specifically to stop the next author reintroducing the bug is worse than no comment: it invites someone who measures the empty case, finds it clean, and concludes the whole warning is folklore.

That is the failure mode. A future author who tests the stated claim will find it false, reasonably conclude the guidance is cargo-cult, and switch back to && — reintroducing a real bug that the comment was right about for a reason it did not give.

⚠️ The wrong mechanism has already propagated — three places, at least

  1. scripts/bump-objectui.sh — the original comment.
  2. PR fix(scripts): bump-objectui.sh runs on bash 3.2 — the pin-reachability warning is macOS-reachable again #12142's body, where it is stated as a verified finding.
  3. This seat's ACCEPT on [finding] check:objectui-changeset --self-test cannot run on macOS: bump-objectui.sh uses the bash-4 mapfile builtin #12071 (comment 5409531670), where I quoted it approvingly — and then again in Three more hand-run shell scripts carry bash-4 builtins — pnpm sdui:manifest is the next step of the same macOS pin-bump procedure #12141's dispatch brief (comment 5411492201), where I handed it to a second dev as established fact. Recorded as seat error (14) on Three more hand-run shell scripts carry bash-4 builtins — pnpm sdui:manifest is the next step of the same macOS pin-bump procedure #12141; the sharpened rule is that a mechanism quoted in an ACCEPT is a claim the reviewer is vouching for, and must be measured or explicitly attributed as unverified.

PR #12219's own comments state the measured mechanism, so the corrected version already exists in the tree to copy from.

Scope

One comment in scripts/bump-objectui.sh. ⛔ Do not change the code — the if form stays. ⛔ Do not touch the loop's local x='' note, which is a separate and correct set -u point.

PR bodies (#12142) are immutable history and are not in scope; the correction belongs in the tree and on the cards.

Left uncorrected by PR #12219 because bump-objectui.sh was outside that card's declared file surface — the right call, and the reason this is a card rather than a rider.

Refs

#12071 / PR #12142 (where the comment landed) · #12141 / PR #12219 (where it was measured and disproven) · #12221 (the other follow-up from the same measurement)

Metadata

Metadata

Assignees

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions