Repository navigation
test(desktop): read only marked lines from the real-shell PATH probe - #6851
Conversation
The ignored real_shell_selects_desktop_shim_on_temp_home test required exactly two stdout lines from `bash -l -i`. On Ubuntu, /etc/bash.bashrc prints the sudo hint to stdout for any sudo-group user without ~/.hushlogin, so the release-QA evidence test failed before it checked anything. The shell script now prints sentinel-prefixed result lines and exits non-zero if the lookup or the CLI fails; the assertions read only those lines.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
added
the
chore
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe real-shell integration test now extracts marked command results instead of assuming stdout contains exactly two lines. It fails on lookup or CLI errors and validates the marked executable path and output. ChangesPOSIX real-shell test
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~8 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to This change makes an ignored integration test more robust against system shell startup output. It has no production impact and no merge-blocking risk remains. Pre-merge checks |
 |
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Description Check | |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title accurately describes the main change: the POSIX real-shell test now reads marked lines from the PATH probe and ignores unrelated shell output. |
✨ Finishing Touches
📝 Generate docstrings
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
- Commit to this branch
- Create a new PR
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e54779cdef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
End the marked output line with :OCX-END so a CLI that prints the expected line followed by another line no longer passes; verified by temporarily adding a second echo to the fixture CLI (zsh: wrong CLI output).
…#6855) * docs(devlog): record the cross-OS probe of the desktop-owned CLI path Plan, per-host execution notes and outcomes for probing #6802, #6807, #6809, #6812, #6816 and #6818 on Ubuntu 24.04 and Windows 11. Follow-ups: #6851, #6852, #6853, #6854. * docs(devlog): tighten evidence citations and the F1 hypothesis * docs(devlog): qualify unretained evidence in the probe outcome * docs(devlog): record exact-head CI for the probe follow-ups * docs(devlog): close the cross-OS probe unit and fold review fixes * docs(devlog): align the P2 escape note and guard the probe cleanup paths
Summary
The ignored real-shell evidence test for the Desktop terminal command (#6816),
cli_command_posix::tests::real_shell_selects_desktop_shim_on_temp_home, fails on stock Ubuntu before it checks anything. It required exactly two stdout lines frombash -l -i -c 'command -v ocx; ocx hello', but Ubuntu's/etc/bash.bashrcprints its "To run a command as administrator…" hint to stdout for every sudo-group user whose HOME has no.hushlogin— and the test's temporary HOME never has one.Before (Ubuntu 24.04, sudo-group user):
zsh: skipped (shell unavailable), thenbash: expected exactly two stdout lines, exit 101, although the shell had resolved/usr/bin/ocxand printednpm-ocxcorrectly.After: the shell script captures both results, exits 11/12 if the lookup or the CLI fails (so
status.success()still catches a missing command), and printsOCX-PATH:/OCX-OUT:lines; the assertions read only those marked lines and still require exactly one of each with the exact expected path and output. The output line ends with:OCX-END, so a CLI that printed the expected line followed by another non-empty line still fails (command substitution drops trailing blank lines, which the test does not try to detect). Unrelated rc output no longer counts.Found while probing the desktop-owned CLI path on real Linux and Windows hosts (Linux fish/bash login results for the release-QA list are recorded in the probe devlog).
Verification
lidge, sudo-group user, the failing case), freshdevclone + this file, CI's sidecar/resource stubs:cargo test --manifest-path desktop/src-tauri/Cargo.toml cli_command_posix::tests::real_shell_selects_desktop_shim_on_temp_home -- --ignored --exact --nocapture→test result: ok. 1 passed(bash; zsh not installed there). Same treecargo test ... cli_command→46 passed; 0 failed; 2 ignored. Both runs repeated at the current headd785fb1b02.1 passed; with the fixture CLI temporarily printing a second line it fails withzsh: wrong CLI output(mutation check, reverted);cargo fmt --checkandcargo clippy --all-targets -- -D warningsfordesktop/src-tauriclean.bun run testsuite — this PR changes one Rust test body only and touches no TypeScript; the ignored test is not part of CI, so the host runs above are its only evidence.Checklist
Summary by CodeRabbit