Skip to content

fix(sudo): keep line endings when replaying captured command output - #49

Merged
johnny4young merged 4 commits into
mainfrom
fix/sudo-output-line-endings
Oct 8, 2026
Merged

johnny4young merged 4 commits into
mainfrom
fix/sudo-output-line-endings

Conversation

@johnny4young

@johnny4young johnny4young commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner

Summary

_gos_sudo_for_target (gos.sh) runs a privileged command by capturing its stdout and stderr with command substitution, which strips trailing newlines. It then replays them with printf '%s', so the command's last line lost its line ending and the next progress or error line was glued onto it. For example:

mv: cannot move …: Permission deniedError: …
sudo mv /usr/local/go /usr/local/go.gos-backup.24418Activating go from ~/.gos/versions/go1.21.6...

This affected all four replay paths: success, permission-retry success, permission-retry failure (the original error and the sudo error were also concatenated), and plain failure.

What changed: one small helper, _gos_sudo_replay, prints each non-empty stream followed by a newline. All four paths use it.

Why it matters beyond cosmetics: this is also why tests/install-transaction.bash failed when the suite ran as root. As root, the protected-parent side-by-side case goes through the permission-retry path, and the glued sudo mv … go.gos-backup.NActivating go from <versions dir> line made the "no sudo inside the versions dir" grep match a line that never existed. With this fix, the full suite passes as root as well.

Behavior note: a command whose output lacked a trailing newline now gets one when replayed. That matches what a terminal shows for a directly executed command. Callers that capture _gos_sudo output with $(…) are unaffected, since command substitution strips it again.

Type of Change

  • Bug fix
  • Feature
  • Documentation
  • Release or packaging
  • Maintenance

Validation

  • bash -n gos.sh install.sh
  • shfmt -d -i 2 -ci -bn . (shfmt 3.13.1, as pinned in CI)
  • shellcheck gos.sh install.sh completions/gos.bash scripts/*.bash scripts/*.sh tests/*.bash (ShellCheck 0.11.0, as pinned in CI)
  • scripts/sync-command-surfaces.bash --check
  • bash tests/install-transaction.bash: passes as root (it failed on main as root) and as non-root
  • scripts/run-tests.bash --jobs 4: 31/31 passed as a non-root user and as root (Linux, Bash 5.2)
  • Other: new regression case in tests/unit-versions.bash drives the sourced _gos_sudo_for_target through the retry-success, retry-failure and plain-failure paths. It asserts the exact stdout/stderr line sequence, and it fails without the fix (movedafter-retry).
  • Hosted CI on 824fb14: all 10 jobs and all 11 checks green, including native macOS Bash 3.2, Linux Fish and Windows Bash/PowerShell.

Notes

🤖 Generated with Claude Code

https://claude.ai/code/session_01ExmnfUngrDYjAheGvYwMnn

_gos_sudo_for_target captures a command's stdout and stderr with command
substitution, which strips trailing newlines, then replayed them with
printf '%s'. The next progress or error line was glued onto the
command's last line, e.g. "mv: ...: Permission deniedError: ..." or
"sudo mv ... go.gos-backup.1234Activating go from ..." (which is also why
tests/install-transaction.bash failed when run as root). Replay each
non-empty stream with its line ending through one helper.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExmnfUngrDYjAheGvYwMnn
claude added 3 commits October 8, 2026 05:27
- add a first-try success case with stdout and stderr to the replay regression
- describe the fix as covering every replay path, not only retries

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExmnfUngrDYjAheGvYwMnn
@johnny4young
johnny4young merged commit 62dd280 into main Oct 8, 2026
11 checks passed
@johnny4young
johnny4young deleted the fix/sudo-output-line-endings branch October 8, 2026 06:37
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.

2 participants