Skip to content

Skills and Agent file - #147

Open
ashtarkb wants to merge 5 commits into
openshift-psap:mainfrom
ashtarkb:add-forge-skills
Open

Skills and Agent file #147
ashtarkb wants to merge 5 commits into
openshift-psap:mainfrom
ashtarkb:add-forge-skills

Conversation

@ashtarkb

@ashtarkb ashtarkb commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

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

    • Added a pre-commit review skill that checks changes against coding, security, and configuration guidelines.
    • Added automated diff collection for staged, unstaged, branch, and untracked changes.
    • Added installation and setup guidance for the FORGE AI Context Module across supported coding assistants.
  • Documentation

    • Expanded contributor guidance for error handling, layer isolation, polling, configuration types, framework behavior, and environment variables.
    • Documented review scope, findings, verdicts, and team installation and update workflows.

…can use lola to adopt it to your coding agnet
@openshift-ci

openshift-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kpouget for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds FORGE coding guidance, a pre-commit review skill, a diff-gathering script, and documentation for installing and updating the module through Lola or manually.

Changes

FORGE review workflow

Layer / File(s) Summary
Operational review rules
AGENTS.md
Adds rules for exception handling, toolbox isolation, retry polling, typed configuration, framework behavior, and environment-variable restoration.
Review skill and diff collection
module/skills/forge-review/SKILL.md, module/skills/forge-review/scripts/gather-diff.sh
Defines review checks, findings, verdicts, scope restrictions, and staged, unstaged, branch, and untracked change collection.
Module installation documentation
module/README.md
Documents Lola registration, agent installation, declarative synchronization, manual setup, and module updates.

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 repository status
  gather_diff_sh-->>Agent: Return structured diff and untracked-file sections
  Agent->>AGENTS_md: Load repository review rules
  AGENTS_md-->>Agent: Return categorized guidance
  Agent->>Agent: Analyze changes and produce verdict
Loading

Possibly related PRs

  • openshift-psap/forge#134: Adds secret-handling guidance to AGENTS.md, which overlaps with the rules enforced by the new review skill.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title identifies the added skill and agent guidance, which are the main changes in the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 68c3880 and 8172236.

📒 Files selected for processing (4)
  • AGENTS.md
  • module/README.md
  • module/skills/forge-review/SKILL.md
  • module/skills/forge-review/scripts/gather-diff.sh

Comment thread module/README.md Outdated
Comment thread module/skills/forge-review/scripts/gather-diff.sh Outdated
Comment thread module/skills/forge-review/SKILL.md
Comment thread module/skills/forge-review/scripts/gather-diff.sh Outdated
Comment thread module/skills/forge-review/SKILL.md
@kpouget

kpouget commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 6, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/skills/forge-review/scripts/gather-diff.sh`:
- Line 9: Update the base-resolution logic around BASE_BRANCH and MERGE_BASE to
try a verified local or remote-tracking reference before calling git merge-base,
and remove the fallback that silently converts failures into an empty base. When
no usable base ref can be resolved, emit DIFF_TYPE: error; only emit DIFF_TYPE:
none after successful base resolution confirms the diff is empty.
🪄 Autofix

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: 1d29c1de-0905-40a7-b672-c0a08f3b4c03

📥 Commits

Reviewing files that changed from the base of the PR and between 8172236 and 2714fda.

📒 Files selected for processing (4)
  • AGENTS.md
  • module/README.md
  • module/skills/forge-review/SKILL.md
  • module/skills/forge-review/scripts/gather-diff.sh
🚧 Files skipped from review as they are similar to previous changes (3)
  • AGENTS.md
  • module/README.md
  • module/skills/forge-review/SKILL.md

# Outputs structured sections so the reviewing agent can parse reliably.

BRANCH=$(git rev-parse --abbrev-ref HEAD 2>/dev/null || echo "HEAD")
BASE_BRANCH="main"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Report an unavailable base as an error.

At Line 39, git merge-base "${BASE_BRANCH}" HEAD can fail when the checkout has no local main ref. The || true then leaves MERGE_BASE empty, and Lines 40-43 emit DIFF_TYPE: none even when HEAD contains branch changes. This can skip the review.

Resolve a verified local or remote-tracking base ref. Emit DIFF_TYPE: error when base resolution fails. Reserve DIFF_TYPE: none for a successful empty diff.

Suggested fix
+    BASE_REF="${BASE_BRANCH}"
+    if ! git rev-parse --verify --quiet "${BASE_REF}^{commit}" >/dev/null; then
+        BASE_REF="origin/${BASE_BRANCH}"
+    fi
-    MERGE_BASE=$(git merge-base "${BASE_BRANCH}" HEAD 2>/dev/null || true)
-    if [[ -z "${MERGE_BASE}" ]]; then
+    if ! MERGE_BASE=$(git merge-base "${BASE_REF}" HEAD 2>/dev/null); then
         echo ""
-        echo "=== DIFF_TYPE: none ==="
+        echo "=== DIFF_TYPE: error ==="

Also applies to: 39-43

🤖 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` at line 9, Update the
base-resolution logic around BASE_BRANCH and MERGE_BASE to try a verified local
or remote-tracking reference before calling git merge-base, and remove the
fallback that silently converts failures into an empty base. When no usable base
ref can be resolved, emit DIFF_TYPE: error; only emit DIFF_TYPE: none after
successful base resolution confirms the diff is empty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants