Skip to content

setup: summarize remote probe errors#56

Merged
flyingrobots merged 3 commits intomainfrom
chore/setup-sanitize
Oct 22, 2025
Merged

setup: summarize remote probe errors#56
flyingrobots merged 3 commits intomainfrom
chore/setup-sanitize

Conversation

@flyingrobots
Copy link
Owner

@flyingrobots flyingrobots commented Oct 22, 2025

Summary

git shiplog setup now surfaces remote-probe failures without drowning users in raw git ls-remote stderr. The new sanitize_remote_error helper normalizes CRLF output, trims leading/trailing whitespace, and keeps only the first two meaningful lines (summarising the remainder as (+N more lines)). When we do trim lines we append a hint telling the user exactly which command to re-run for the full output.

Before: multi-line Git “help” blobs and verbose transport traces spilled into the wizard, forcing users to scroll to find the actionable line.

After: the wizard shows a one-line summary (e.g. fatal: could not read Username for ...; (+3 more lines) (trimmed; run 'git ls-remote ...' for full output)), with the original command included so the user can re-run it manually if they need detail.

No behavioral change to setup logic—just cleaner, higher-signal error messaging.

Tests

  • make test (Dockerized)

Deployment / Docs

  • Docs updated when applicable
  • Progress bars updated (make progress)

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 22, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced error reporting for failed remote operations with improved message formatting, normalization, and readability.
    • Remote error output is now summarized and condensed for better clarity and usability.
    • Added helpful guidance to access complete error details when error messages are truncated or summarized.

Walkthrough

Added sanitize_remote_error() to normalize remote command output (strip CRs, trim lines, extract first two non-empty lines, count extras) and integrated it into remote probe failure handling so sanitized messages or a standardized exit-code fallback are returned with reproduction hints when applicable.

Changes

Cohort / File(s) Summary
Error sanitization & probe handling
lib/commands.sh
Added sanitize_remote_error(raw_input) to normalize remote error output (remove CRs, trim lines, produce compact summary and expose SANITIZE_REMOTE_EXTRA_LINES / SANITIZE_REMOTE_TOTAL_LINES). Updated remote probe failure path to use sanitized output when present, fall back to a "command failed with exit code X" message when empty, append reproduction guidance if extra lines exist, and unset global sanitation variables after use.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant remote_probe_error
    participant sanitize_remote_error
    participant ErrorHandler

    Caller->>remote_probe_error: run git ls-remote (fails)
    remote_probe_error->>sanitize_remote_error: pass raw probe_output
    sanitize_remote_error->>sanitize_remote_error: remove CRs, trim lines, pick first two non-empty, count extras
    sanitize_remote_error-->>remote_probe_error: return sanitized_summary + extras metadata
    alt sanitized_summary non-empty
        remote_probe_error->>ErrorHandler: deliver sanitized_summary (append "reproduce full output" if extras > 0)
    else sanitized_summary empty
        remote_probe_error->>ErrorHandler: deliver "command failed with exit code X" (+ trimmed-output hint if available)
    end
    ErrorHandler-->>Caller: final error message
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

CLEAN THE NOISE, TRIM THE SPRAWL,
LINES REDUCED TO TWO — CONCISE AND SMALL.
CARRIAGE RETURNS VANISH, EXTRAS COUNTED TOO,
DIAGNOSTICS KEPT TIDY, REPRODUCTION HINTS IN VIEW. ✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description has excellent Summary and Deployment/Docs sections that are complete and detailed. However, the Tests section is incomplete—it shows only the "make test (Dockerized)" checkbox but omits the "Lint CI green (shellcheck/markdownlint/yamllint)" checkbox. More critically, the Approval section is entirely missing, including the mandatory "Label approved-by-owner applied (required)" checkbox that the template explicitly specifies. This is a structural violation of the required template that breaks the checklist contract. Add the missing checkboxes to match the template exactly: include the "Lint CI green" checkbox in the Tests section (even if unchecked) and restore the entire Approval section with the approved-by-owner label checkbox. Templates exist for a reason—follow them rigorously.
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title "setup: summarize remote probe errors" directly and accurately captures the core change: the PR adds error sanitization to present remote-probe failures more concisely in the setup wizard. The title is specific, action-oriented, and immediately conveys the primary purpose of the changeset without vagueness or excessive detail.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/setup-sanitize

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 83c53fa and 6eb5a72.

📒 Files selected for processing (1)
  • lib/commands.sh (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
{scripts/bosun,lib/**/*.sh}

📄 CodeRabbit inference engine (AGENTS.md)

Construct Bosun table output column-by-column using $'\t' concatenation; avoid embedding literal tabs in a single string

Files:

  • lib/commands.sh
{scripts/**/*.sh,lib/**/*.sh,test/**/*.bats}

📄 CodeRabbit inference engine (AGENTS.md)

Keep runtime and test dependencies to stock POSIX tools plus jq; avoid introducing other external binaries

Files:

  • lib/commands.sh
**/*.sh

📄 CodeRabbit inference engine (AGENTS.md)

Shell scripts should pass ShellCheck in CI (fix issues rather than suppress unless documented)

Files:

  • lib/commands.sh
🔇 Additional comments (2)
lib/commands.sh (2)

20-58: Solid function. Past concerns addressed, defensive coding in place, and POSIX-clean.

The implementation consolidates all local declarations into a single statement (line 21) and properly validates input with an early return for empty strings (lines 28–31). The line-by-line normalization correctly:

  • Strips carriage returns (line 33, using POSIX tr)
  • Filters non-empty lines via character class matching (line 37)
  • Trims leading/trailing whitespace with sed (line 39)
  • Counts excess lines beyond the first two (lines 40–44)
  • Exposes line counts via globals for caller introspection (lines 49–50)

Output formatting (lines 52–57) handles all cases: 0 lines → empty, 1 line → first_line, 2 lines → "first; second", 3+ lines → "first; second; (+N more lines)".

All external commands are POSIX-standard (tr, sed, printf). Here-string syntax (<<<"$normalized_input" at line 47) is a bash idiom matching the shebang; acceptable.


1763-1776: Integration is clean and properly scoped.

The error handler correctly:

  • Calls the sanitizer on raw probe output (line 1764)
  • Falls back to a standardized exit-code message if sanitized output is empty (lines 1766–1767)
  • Appends a reproduction hint with the actual command only when lines were trimmed (lines 1769–1774)
  • Cleans up global state after use (line 1776)

Variable expansion is defensive: ${SANITIZE_REMOTE_EXTRA_LINES:-0} defaults to 0 if unset, and the hint condition checks > 0 (line 1769).

Command display using printf '%q ' (line 1771) correctly shell-quotes each array element for user re-execution.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@flyingrobots flyingrobots added the approved-by-owner Required for PR merge label Oct 22, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6ac68be and e882278.

📒 Files selected for processing (1)
  • lib/commands.sh (2 hunks)
🔇 Additional comments (1)
lib/commands.sh (1)

1752-1755: Integration is clean and handles the fallback correctly.

The call at line 1752 properly sanitizes probe_output (which contains git stderr), and the fallback at line 1754 only triggers when stderr was pure whitespace—exactly what you want. The exit-code check at line 1749 correctly gates timeout handling separately before attempting sanitization. No issues here.

coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 22, 2025
@flyingrobots flyingrobots merged commit 114a17b into main Oct 22, 2025
10 checks passed
@flyingrobots flyingrobots deleted the chore/setup-sanitize branch October 22, 2025 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved-by-owner Required for PR merge

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant

Comments