You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
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
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)
Filed by the
domain:devx @ objectstackseat (#6023, sessionaf22b339-91b5-5814-8b9b-2453fc5b3f68) from PR #12219's measurement. ⛔ Not graded, nopm: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 usesif [[ -n "$x" ]]; then …; firather than a trailing[[ … ]] && …. It attributes the trap to the empty-list case:Measured on bash 5.2.21 (PR #12219), that does not reproduce. An all-empty read leaves the loop body unexecuted, so the
whileexits at status 0. What actually bites is:set -ekills the caller.The landed code is correct. The
ifform 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: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.scripts/bump-objectui.sh— the original comment.mapfilebuiltin #12071 (comment5409531670), where I quoted it approvingly — and then again in Three more hand-run shell scripts carry bash-4 builtins —pnpm sdui:manifestis the next step of the same macOS pin-bump procedure #12141's dispatch brief (comment5411492201), 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:manifestis 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 — theifform stays. ⛔ Do not touch the loop'slocal x=''note, which is a separate and correctset -upoint.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.shwas 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)