Skills and Agent file - #147
Conversation
…can use lola to adopt it to your coding agnet
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
📝 WalkthroughWalkthroughAdds FORGE coding guidance, a pre-commit review skill, a diff-gathering script, and documentation for installing and updating the module through Lola or manually. ChangesFORGE review workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant gather_diff_sh
participant AGENTS_md
Agent->>gather_diff_sh: Gather selected diff and status
gather_diff_sh-->>Agent: Return structured diff sections
Agent->>AGENTS_md: Load rule categories
AGENTS_md-->>Agent: Return review rules
Agent->>Agent: Analyze changes and produce verdict
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@module/README.md`:
- Around line 111-118: Update the Cursor and Claude Code subsection headings
under Manual installation from level four to level three, preserving the
existing heading text and document hierarchy.
In `@module/skills/forge-review/scripts/gather-diff.sh`:
- Around line 39-55: Update the remote-branch detection flow around HAS_REMOTE
and BRANCH_DIFF to capture the git ls-remote exit status without allowing set -e
to abort, and distinguish probe failures from a genuinely absent remote branch.
Resolve and use the validated remote reference for the branch diff, tracking its
exit status separately; only emit DIFF_TYPE: none when the diff command succeeds
with empty output, while preserving failure information for unsuccessful probes
or diff resolution.
In `@module/skills/forge-review/SKILL.md`:
- Around line 19-25: Update Step 1 in the forge-review skill instructions to
invoke gather-diff.sh from the installed skill directory under
.cursor/skills/forge-review/scripts or .claude/skills/forge-review/scripts,
rather than assuming a project-root ./scripts path. Keep the documented
installation locations consistent with the command so diff gathering works after
installation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 83b3bad0-cc2d-4745-85d8-f61a877d1bbc
📒 Files selected for processing (4)
AGENTS.mdmodule/README.mdmodule/skills/forge-review/SKILL.mdmodule/skills/forge-review/scripts/gather-diff.sh
| #### Cursor | ||
|
|
||
| ```bash | ||
| mkdir -p .cursor/skills/forge-review | ||
| cp -r module/skills/forge-review/* .cursor/skills/forge-review/ | ||
| ``` | ||
|
|
||
| #### Claude Code |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use level-three headings under manual installation.
## Manual installation is followed by #### Cursor and #### Claude Code, skipping ###. Change both subsections to level-three headings to preserve document navigation and resolve MD001.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 111-111: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🤖 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 `@module/README.md` around lines 111 - 118, Update the Cursor and Claude Code
subsection headings under Manual installation from level four to level three,
preserving the existing heading text and document hierarchy.
Source: Linters/SAST tools
| HAS_REMOTE=$(git ls-remote --heads origin "${BRANCH}" 2>/dev/null | head -1) | ||
| if [[ -n "${HAS_REMOTE}" ]]; then | ||
| BRANCH_DIFF=$(git diff "${REMOTE_BRANCH}...HEAD" 2>/dev/null || true) | ||
| if [[ -n "${BRANCH_DIFF}" ]]; then | ||
| echo "" | ||
| echo "=== DIFF_TYPE: branch ===" | ||
| echo "${BRANCH_DIFF}" | ||
| else | ||
| echo "" | ||
| echo "=== DIFF_TYPE: none ===" | ||
| echo "No changes found between local and remote." | ||
| fi | ||
| else | ||
| echo "" | ||
| echo "=== DIFF_TYPE: none ===" | ||
| echo "No remote branch and no local changes." | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'gather-diff\.sh$' . || true
echo "== script excerpt =="
if [ -f module/skills/forge-review/scripts/gather-diff.sh ]; then
wc -l module/skills/forge-review/scripts/gather-hdiff.sh module/skills/forge-review/scripts/gather-diff.sh 2>/dev/null || true
sed -n '1,90p' module/skills/forge-review/scripts/gather-diff.sh | cat -n
fi
echo "== git commands in script =="
rg -n "git (ls-remote|diff|checkout|show)" module/skills/forge-review/scripts/gather-diff.sh || true
echo "== changed files/stat =="
git diff --stat HEAD~1..HEAD 2>/dev/null || true
git diff -- module/skills/forge-review/scripts/gather-diff.sh 2>/dev/null | head -120 || trueRepository: openshift-psap/forge
Length of output: 3178
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Behavioral probe for bash -euo pipefail assignment with pipeline command failure =="
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
cat > "$tmpdir/probe.sh" <<'BASH'
#!/usr/bin/env bash
set -euo pipefail
echo "pre ls-remote"
HAS_REMOTE=$(git ls-remote --heads origin nonexistent-branch-1234 2>/dev/null | head -1)
echo "after ls-remote assigned: ${HAS_REMOTE-<unset>}"
echo "pre diff"
BRANCH_DIFF=$(git diff nonexistent-unborn-branch-5678...HEAD 2>/dev/null || true)
echo "after diff assigned: ${BRANCH_DIFF-<empty>}"
echo "end"
BASH
chmod +x "$tmpdir/probe.sh"
bash "$tmpdir/probe.sh"
status=$?
echo "probe status=$status"
echo "== inspect git availability and repo gitdir =="
git --version || true
git rev-parse --git-dir || true
git ls-remote --heads origin nonexistent-branch-1234 || true
git diff nonexistent-unborn-branch-5678...HEAD || trueRepository: openshift-psap/forge
Length of output: 625
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp=$(mktemp -d)
cd "$tmp"
git init -q
git commit --allow-empty -q -m base
cat > probe.sh <<'BASH'
#!/usr/bin/env bash
set -euo pipefail
echo "pre ls-remote"
HAS_REMOTE=$(git ls-remote --heads origin nonexistent-branch-1234 2>/dev/null | head -1)
echo "after ls-remote assigned: ${HAS_REMOTE-<unset>}"
if [[ "${HAS_REMOTE-}" != "" ]]; then
echo "has remote"
fi
echo "pre diff"
BRANCH_DIFF=$(git diff nonexistent-unborn-branch-5678...HEAD 2>/dev/null || true)
echo "after diff assigned: ${BRANCH_DIFF-<empty>}"
echo "end"
BASH
chmod +x probe.sh
bash probe.sh
status=$?
echo "status=$status"Repository: openshift-psap/forge
Length of output: 496
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp=$(mktemp -d)
cd "$tmp"
git init -q
git config --local user.email test@example.com
git config --local user.name Test
git commit --allow-empty -q -m base
cat > probe.sh <<'BASH'
#!/usr/bin/env bash
set -euo pipefail
echo "pre ls-remote"
HAS_REMOTE=$(git ls-remote --heads origin nonexistent-branch-1234 2>/dev/null | head -1)
echo "after ls-remote assigned: '${HAS_REMOTE-<unset>}'"
if [[ -n "${HAS_REMOTE}" ]]; then
echo "has remote"
fi
if [[ -z "${HAS_REMOTE}" ]]; then
echo "no remote fallback taken"
fi
echo "pre diff"
BRANCH_DIFF=$(git diff nonexistent-unborn-branch-5678...HEAD 2>/dev/null || true)
echo "after diff assigned: '${BRANCH_DIFF-<empty>}'"
echo "end"
BASH
chmod +x probe.sh
bash probe.sh
status=$?
echo "status=$status"Repository: openshift-psap/forge
Length of output: 176
Distinguish remote probe failures from an empty branch diff.
Under set -euo pipefail, a failed git ls-remote exits during the assignment before taking the documented no-remote fallback. Conversely, missing remote references or stale refs can cause git diff origin/${BRANCH}...HEAD to fail and produce DIFF_TYPE: none via || true. Track the probe exit status separately and only report “none” after the branch diff succeeds with no output.
🤖 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 `@module/skills/forge-review/scripts/gather-diff.sh` around lines 39 - 55,
Update the remote-branch detection flow around HAS_REMOTE and BRANCH_DIFF to
capture the git ls-remote exit status without allowing set -e to abort, and
distinguish probe failures from a genuinely absent remote branch. Resolve and
use the validated remote reference for the branch diff, tracking its exit status
separately; only emit DIFF_TYPE: none when the diff command succeeds with empty
output, while preserving failure information for unsuccessful probes or diff
resolution.
| ### Step 1: Gather the diff | ||
|
|
||
| Run the diff gathering script: | ||
|
|
||
| ```bash | ||
| ./scripts/gather-diff.sh | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Resolve the diff script from the installed skill directory.
The documented installs place the script under .cursor/skills/forge-review/scripts/ or .claude/skills/forge-review/scripts/, but this command resolves ./scripts/gather-diff.sh from the project root. That path is not created by either installation method, so the skill cannot gather diffs after installation. Invoke the skill’s sibling script or install it at the project-level path consistently.
🤖 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 `@module/skills/forge-review/SKILL.md` around lines 19 - 25, Update Step 1 in
the forge-review skill instructions to invoke gather-diff.sh from the installed
skill directory under .cursor/skills/forge-review/scripts or
.claude/skills/forge-review/scripts, rather than assuming a project-root
./scripts path. Keep the documented installation locations consistent with the
command so diff gathering works after installation.
1 participant
add more instructions in the agent.md file and a reviewer skill, you can use lola to adopt it to your coding agnet
Summary by CodeRabbit
New Features
Documentation