#4937: fix(wp-helper): replace bash -lc with bash -c to stop SSH stdout being swallowed#4938
Conversation
…g swallowed Login shell startup files (~/.bash_profile, ~/.bashrc) on remote servers (Cloudways, Hetzner, Cloudron, Hostinger, Closte) can redirect or close stdout before wp runs. Dropping the -l flag avoids sourcing those files while still executing the cd+wp command correctly. Also redirect the [INFO] diagnostic line in run_wp_command to stderr so callers can capture clean WP-CLI stdout without mixing it with log output. Fixes #4937
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where WP-CLI output was being suppressed on certain managed hosting providers when commands were executed via SSH. The primary fix involves modifying the SSH command execution to bypass login shell startup files, which were inadvertently interfering with standard output. Additionally, informational messages have been rerouted to stderr to maintain the integrity of WP-CLI's standard output. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sun Mar 15 17:35:12 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
![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 correctly resolves an issue where command output was being suppressed by remote login shell startup files by switching from bash -lc to bash -c. The change to redirect diagnostic information to stderr is also a good improvement for separating log output from command results. I have added one suggestion to further improve the diagnostic logging by properly quoting command arguments, which will make debugging easier, especially for commands with arguments containing spaces.
| site_type=$(echo "$site_config" | jq -r '.type') | ||
|
|
||
| print_info "Running on $site_name ($site_type): wp ${wp_args[*]}" | ||
| print_info "Running on $site_name ($site_type): wp ${wp_args[*]}" >&2 |
There was a problem hiding this comment.
Using ${wp_args[*]} can create ambiguous log output if any of the arguments contain spaces. For example, a command with an argument like 'John Doe' would be logged with the quotes removed, which is misleading.
To make the logged command accurate and easier to debug, I suggest quoting the arguments for display. You can use printf '%q ' to safely quote each argument.
| print_info "Running on $site_name ($site_type): wp ${wp_args[*]}" >&2 | |
| print_info "Running on $site_name ($site_type): wp $(printf '%q ' "${wp_args[@]}")" >&2 |
marcusquinn
added
the
review-feedback-scanned
…itter Root cause: multiple pulse runners evaluating the same issue simultaneously create duplicate PRs. Process-based dedup (has_worker_for_repo_issue, is-duplicate) only sees local processes — invisible across machines. Fix 1: Add is-assigned command to dispatch-dedup-helper.sh that queries GitHub assignees before dispatch. If another runner already self-assigned, skip the issue. This is the primary cross-machine dedup guard. Fix 2: Add 0-30s random startup jitter to pulse-wrapper.sh so concurrent launchd-triggered pulses don't evaluate issues at the same instant. Configurable via PULSE_JITTER_MAX (set to 0 to disable). Fix 3: Update pulse.md dispatch instructions to enforce the assignee check as a mandatory step alongside existing local process dedup. Observed: PR #4940 duplicated PR #4938 for issue #4937 because alex-solovyev's pulse dispatched 2 min after marcusquinn self-assigned, interpreting the in-progress worker as 'failed'.
…tter (#4948) * fix: prevent duplicate dispatch across runners via assignee check + jitter Root cause: multiple pulse runners evaluating the same issue simultaneously create duplicate PRs. Process-based dedup (has_worker_for_repo_issue, is-duplicate) only sees local processes — invisible across machines. Fix 1: Add is-assigned command to dispatch-dedup-helper.sh that queries GitHub assignees before dispatch. If another runner already self-assigned, skip the issue. This is the primary cross-machine dedup guard. Fix 2: Add 0-30s random startup jitter to pulse-wrapper.sh so concurrent launchd-triggered pulses don't evaluate issues at the same instant. Configurable via PULSE_JITTER_MAX (set to 0 to disable). Fix 3: Update pulse.md dispatch instructions to enforce the assignee check as a mandatory step alongside existing local process dedup. Observed: PR #4940 duplicated PR #4938 for issue #4937 because alex-solovyev's pulse dispatched 2 min after marcusquinn self-assigned, interpreting the in-progress worker as 'failed'. * fix: validate PULSE_JITTER_MAX as numeric, use read -ra for assignee parsing Address Gemini review feedback: - Validate PULSE_JITTER_MAX is numeric before arithmetic (prevents set -e failures from non-integer env var values) - Use read -ra for comma-separated assignee parsing instead of IFS word splitting (more robust against whitespace edge cases)
1 participant
Summary
bash -lcwithbash -cin both SSH execution paths (hostinger|closteandhetzner|cloudways|cloudron) so remote login shell startup files (~/.bash_profile,~/.bashrc) are not sourced — these files on managed hosting servers commonly redirect or close stdout beforewpruns, causing all WP-CLI output to be silently discarded.[INFO] Running on ...diagnostic line inrun_wp_commandto stderr, so callers can capture clean WP-CLI stdout without mixing it with log output.Root cause
bash -lcinvokes a login shell on the remote server. Cloudways (and other managed hosts) source startup files that include guards likemesg nortty -s && mesg n, or other interactive-shell redirections that close/redirect fd1 before thewpcommand runs. The-nSSH flag (stdin from/dev/null) is unrelated — the issue is the login shell, not stdin.Direct SSH without
bash -lc(as in the reporter's working example) bypasses this entirely.Changes
.agents/scripts/wp-helper.sh— twobash -lc→bash -csubstitutions (lines 340, 347 pre-patch); oneprint_inforedirected to stderr (line 382 pre-patch)Verification
ShellCheck: zero new violations (SC1091 info pre-existing, unrelated to this change).
Functional verification requires a live Cloudways/Hetzner/Cloudron site — the reporter can confirm with:
Closes #4937