Add AGENTS.md, shellcheck automation, and version-check.sh#1
Conversation
- AGENTS.md documents LFS build conventions, environment variables, build discipline, and verified host tool versions - .ona/automations.yaml adds two tasks: installShellcheck (runs on postDevcontainerStart) and lintScripts (manual, runs shellcheck against all .sh files) - version-check.sh verifies required LFS host tool versions Co-authored-by: Ona <no-reply@ona.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR establishes a build environment with automated shell script linting, comprehensive documentation, and version verification. It adds Ona automation to install and run shellcheck over repository scripts, a detailed AGENTS.md guide describing the LFS environment and agent practices, and a version-check utility for toolchain verification. ChangesBuild Environment Setup
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![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: 28d0252a35
ℹ️ 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".
| export LC_ALL=C | ||
| bash --version | head -n1 | cut -d" " -f2-4 | ||
| echo -n "Binutils: "; ld --version | head -n1 | cut -d" " -f3- | ||
| bison --version | head -n1 |
There was a problem hiding this comment.
version-check.sh is intended to verify tool availability, but each check is piped through head/cut without pipefail, so a missing tool can still produce a successful pipeline status (e.g., bison --version | head -n1 returns 0 even when bison is not installed). In that case the script can exit successfully and report a false pass, which can let an invalid host environment proceed into later LFS steps and fail much later.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
AGENTS.md (1)
50-50: 💤 Low valueConsider quoting the find output or using -exec for robustness.
The current command will fail if any
.shfilename contains whitespace. While this is just example documentation, a robust alternative would help users avoid issues.♻️ More robust alternative
-find . -name "*.sh" -not -path "./.git/*" | sort | xargs shellcheck -x +find . -name "*.sh" -not -path "./.git/*" -print0 | sort -z | xargs -0 shellcheck -xUsing
-print0,sort -z, andxargs -0handles filenames with spaces, newlines, and special characters correctly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@AGENTS.md` at line 50, The find + xargs pipeline in the example is unsafe for filenames with whitespace; update the example command (the line starting with find . -name "*.sh" -not -path "./.git/*" | sort | xargs shellcheck -x) to a robust variant that either uses null-separated output (use find with -print0, sort -z and xargs -0 to pass filenames safely to shellcheck) or replaces xargs with find -exec to run shellcheck per file safely; pick one approach and update the documentation string accordingly..ona/automations.yaml (1)
21-34: ⚡ Quick winUse robust filename iteration to handle paths with whitespace.
The unquoted
$SCRIPTSexpansion on line 26 will word-split on any whitespace in filenames, causing the loop to break or skip files if a.shfile contains spaces or special characters.♻️ Proposed fix using while-read loop
- SCRIPTS=$(find . -name "*.sh" -not -path "./.git/*" | sort) TOTAL=0 FAILED=0 PASSED=0 - for f in $SCRIPTS; do + while IFS= read -r f; do TOTAL=$((TOTAL + 1)) echo "Checking: $f" if shellcheck -x "$f"; then @@ -32,7 +30,7 @@ FAILED=$((FAILED + 1)) fi echo "" - done + done < <(find . -name "*.sh" -not -path "./.git/*" | sort)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.ona/automations.yaml around lines 21 - 34, The loop over SCRIPTS uses an unquoted expansion of $SCRIPTS which breaks on filenames with whitespace; replace the `SCRIPTS=$(find ...)` + `for f in $SCRIPTS; do` pattern with a null-delimited reader: run `find` with `-print0` and iterate using `while IFS= read -r -d '' f; do` so filenames with spaces/newlines are preserved; keep the same counters TOTAL/FAILED/PASSED and the `shellcheck -x "$f"` check inside the new `while` loop and remove the intermediate SCRIPTS variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.ona/automations.yaml:
- Around line 21-34: The loop over SCRIPTS uses an unquoted expansion of
$SCRIPTS which breaks on filenames with whitespace; replace the `SCRIPTS=$(find
...)` + `for f in $SCRIPTS; do` pattern with a null-delimited reader: run `find`
with `-print0` and iterate using `while IFS= read -r -d '' f; do` so filenames
with spaces/newlines are preserved; keep the same counters TOTAL/FAILED/PASSED
and the `shellcheck -x "$f"` check inside the new `while` loop and remove the
intermediate SCRIPTS variable.
In `@AGENTS.md`:
- Line 50: The find + xargs pipeline in the example is unsafe for filenames with
whitespace; update the example command (the line starting with find . -name
"*.sh" -not -path "./.git/*" | sort | xargs shellcheck -x) to a robust variant
that either uses null-separated output (use find with -print0, sort -z and xargs
-0 to pass filenames safely to shellcheck) or replaces xargs with find -exec to
run shellcheck per file safely; pick one approach and update the documentation
string accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4293f26-efb5-4a61-93fb-1cc6022ac3c0
📒 Files selected for processing (3)
.ona/automations.yamlAGENTS.mdversion-check.sh
1 participant
What
AGENTS.md— documents LFS build conventions, required environment variables, build discipline rules, verified host tool versions, and how to use opencode..ona/automations.yaml— two automation tasks:installShellcheck— installs shellcheck automatically onpostDevcontainerStartlintScripts— manually triggered; runsshellcheck -xagainst every.shfile in the repo and reports a pass/fail summaryversion-check.sh— script to verify all required LFS host tools are present and print their versionsWhy
The repo had no documented conventions for agents or contributors, no automated code quality checks, and no way to quickly verify the host build environment. This adds all three.
Notes
lintScriptsfound 7 of 20 scripts with issues — those are pre-existing and can be fixed in follow-up commits.shellcheck(v0.9.0) is installed in the devcontainer via the automation; no manual setup needed.Summary by CodeRabbit
Release Notes
New Features
Documentation