fix: swap stat -f %m to Linux-first order for cross-platform compat#1492
fix: swap stat -f %m to Linux-first order for cross-platform compat#1492marcusquinn merged 1 commit intomainfrom
Conversation
…1491) On Linux, stat -f %m interprets -f as 'filesystem' mode and dumps multi-line filesystem metadata to stdout before exiting non-zero. Since 2>/dev/null only suppresses stderr, the stdout pollution is captured by $(), corrupting the variable and causing arithmetic evaluation failures ('unbound variable' errors). Fix: swap all 10 macOS-first || chains to Linux-first: stat -c %Y "$file" 2>/dev/null || stat -f %m "$file" 2>/dev/null stat -c on macOS fails cleanly (no stdout, exit 1) so the fallback to stat -f works correctly. Also simplified subagent-index-helper.sh from a 5-line if/else to the standard || chain pattern. Verified on Linux via OrbStack (Ubuntu 22.04): - stat -c %Y returns clean numeric timestamp - Arithmetic age calculation passes - All 10 fixed scripts confirmed Linux-first Closes #1491
Summary of ChangesHello @marcusquinn, 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 significantly enhances the cross-platform compatibility of several shell scripts by rectifying an issue with the 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
|
WalkthroughThis pull request fixes a cross-platform compatibility bug in shell scripts where Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
🔍 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 Feb 15 15:36:51 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![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 effectively addresses a cross-platform compatibility issue with the stat command by prioritizing the Linux-compatible version (-c %Y) over the macOS-specific one (-f %m). The changes are applied consistently across 10 files, which will prevent stdout pollution and "unbound variable" errors on Linux systems. The simplification in subagent-index-helper.sh, replacing a verbose if/else block with the concise || chain, is a great improvement for readability and consistency. I have one minor suggestion to improve stylistic consistency regarding arithmetic expansion.
| local lock_pid lock_age | ||
| lock_pid=$(head -1 "$escalation_lock" 2>/dev/null || echo "") | ||
| lock_age=$(($(date +%s) - $(stat -f %m "$escalation_lock" 2>/dev/null || stat -c %Y "$escalation_lock" 2>/dev/null || echo "0"))) | ||
| lock_age=$(($(date +%s) - $(stat -c %Y "$escalation_lock" 2>/dev/null || stat -f %m "$escalation_lock" 2>/dev/null || echo "0"))) |
There was a problem hiding this comment.
For consistency with other scripts in this pull request and for cleaner syntax, it's better to avoid the redundant nested parentheses for arithmetic expansion. The $(($(...))) pattern can be simplified to $((...)). This improves readability and aligns with the common practice seen in the other modified files.
| lock_age=$(($(date +%s) - $(stat -c %Y "$escalation_lock" 2>/dev/null || stat -f %m "$escalation_lock" 2>/dev/null || echo "0"))) | |
| lock_age=$(( $(date +%s) - $(stat -c %Y "$escalation_lock" 2>/dev/null || stat -f %m "$escalation_lock" 2>/dev/null || echo "0") )) |
1 participant
Summary
Fixes #1491 —
stat -f %m(macOS-first) breaks on Linux, causing stdout pollution and "unbound variable" errors.Root Cause
On Linux,
stat -f %minterprets-fas "filesystem" mode and dumps multi-line filesystem metadata to stdout (not stderr). Since2>/dev/nullonly suppresses stderr, the garbage is captured by$(), corrupting the variable. Arithmetic evaluation then fails on the garbage text.Fix
Swap all 10 macOS-first
||chains to Linux-first:stat -con macOS fails cleanly (no stdout, exit 1) so the fallback tostat -fworks correctly.Also simplified
subagent-index-helper.shfrom a 5-line if/else to the standard||chain.Files Changed (10)
All
||chain patterns swapped to Linux-first. The 4 files usingunameguards were already safe and left unchanged.Verification
Tested on Linux via OrbStack (Ubuntu 22.04):
stat -c %Yreturns clean numeric timestampSummary by CodeRabbit