Add CI/CD pipeline, bundle dist, and SARIF + job-summary features#82
Add CI/CD pipeline, bundle dist, and SARIF + job-summary features#82shahrryyar wants to merge 1 commit into
Conversation
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
There was a problem hiding this comment.
Verdict
needs changes
Primary Goal
Add CI/CD pipeline, bundle dist, and SARIF + job-summary features
Overview
This PR introduces a comprehensive CI/CD workflow (.github/workflows/ci.yml), bundles the action using @vercel/ncc with committed dist/, adds SARIF output for GitHub code scanning (src/sarif.ts), renders job summaries (src/summary.ts), updates tooling (ESLint 9, Prettier 3, Vitest 4), fixes Dependabot configuration, and corrects workflow references. The changes touch multiple areas including CI, bundling, security reporting, and developer experience.
Scope Assessment
The PR stays focused on its stated goal of adding CI/CD, bundling, SARIF, and job-summary features. All changes are either core to these features (new workflows, new source files) or support files) or necessary adjustments (.gitignore, action.yml, package.json). No significant scope drift detected.
Risk Assessment
Main risks include: 1) Potential breakage from updating action versions (checkout@v4, gh-release@v2) without testing in this PR's context; 2) The SARIF generation assumes all comments have valid type and path/line fields - missing validation could cause runtime errors; 3) Bundling with ncc and committing dist/ requires the check-dist step to prevent drift - this relies on the git diff --exit-code dist command which may fail in edge cases (e.g., line endings).
Reuse Notes
- Existing
src/reviewer.tsalready returns aReviewResponsestructure - the SARIF and summary modules reuse this pattern. - The
runReviewfunction insrc/reviewer.tswas modified to returnReviewResponseinstead ofvoid- this aligns with existing TypeScript patterns in the codebase. - ESLint configuration reuses
typescript-eslintrecommended rules with overrides forno-explicit-any. - Test setup uses Vitest with V8 coverage - consistent with existing test patterns.
Action Items
- Add validation in
buildSarifto handle comments missingtype,path, orlinefields to prevent runtime errors. - Consider adding a step in the CI workflow to verify the bundled
dist/index.mjsruns correctly (e.g.,node dist/index.mjs --help). - Ensure the
check-distscript inpackage.jsonworks correctly in forked repositories where the action might be run from a fork. - Update the README to document the new
sarif-fileoutput and how to use it with GitHub code scanning upload action.
| fullDescription: { text: `OpenRabbit flagged this as a ${type} issue.` }, | ||
| defaultConfiguration: { level: LEVEL_BY_TYPE[type] }, | ||
| helpUri: 'https://github.com/aryanbrite/openrabbit', | ||
| })) |
There was a problem hiding this comment.
bug: The SARIF builder assumes all comments have valid type, path, and line fields. If a comment lacks these (e.g., from a bug in the reviewer), this will throw a runtime error when accessing comment.type or comment.line. Add a guard to skip invalid comments.
| })) | |
| const results = response.comments | |
| .filter((comment): comment is ReviewComment & { type: ReviewCommentType; path: string; line: number } => | |
| Boolean(comment.type) && typeof comment.path === 'string' && typeof comment.line === 'number') | |
| .map((comment) => { | |
| const type = comment.type as ReviewCommentType | |
| return { | |
| ruleId: `openrabbit/${type}`, | |
| ruleIndex: ALL_TYPES.indexOf(type), | |
| level: LEVEL_BY_TYPE[type], | |
| message: { text: comment.body }, | |
| locations: [ | |
| { | |
| physicalLocation: { | |
| artifactLocation: { uri: comment.path }, | |
| region: { startLine: comment.line }, | |
| }, | |
| }, | |
| ], | |
| } | |
| }) |
| name: Bundle & verify dist | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
question: The test job uses a matrix for Node 20 and 22, but only uploads the coverage artifact when matrix.node == 20. Is this intentional? If so, consider adding a comment explaining why coverage is only uploaded from one matrix job.
There was a problem hiding this comment.
Maybe add a short comment explaining why only Node 20 uploads coverage. Good Catch
| run: npm run build | ||
| - name: Verify committed dist is up to date | ||
| run: git diff --exit-code dist | ||
|
|
There was a problem hiding this comment.
suggestion: The actionlint step uses reviewdog/action-actionlint@v1. Consider pinning to a specific version (e.g., @v1) is already done, but verify this is the latest stable version. Alternatively, use actions/setup-actionlint if available for better integration.
| - uses: reviewdog/action-actionlint@v1 |
There was a problem hiding this comment.
v1 is acceptable so its fine. Ignore this
| @@ -19,8 +25,13 @@ | |||
| "undici": "^5.0.0" | |||
| }, | |||
| "devDependencies": { | |||
There was a problem hiding this comment.
suggestion: The check-dist script runs npm build && git diff --exit-code dist. If the build fails, the git diff won't run, but the error message might be unclear. Consider splitting into two steps or adding a custom message: npm run build && echo 'Checking dist is up to date...' && git diff --exit-code dist || (echo 'Dist is out of date. Run npm run build to update.' && exit 1)
There was a problem hiding this comment.
The proposed shell command is more complicated than necessary.
It doesn't improve debugging much. ignore this
| core.setOutput('sarif-file', sarifPath) | ||
| } | ||
|
|
||
| run().catch((error) => core.setFailed(`${error}`)); |
There was a problem hiding this comment.
question: The SARIF file is written to openrabbit.sarif in the root of the workspace. Since actions run in $GITHUB_WORKSPACE, this is acceptable, but consider making the path configurable or using a subdirectory to avoid potential conflicts with other actions in the same workflow.
There was a problem hiding this comment.
This is a real robustness issue.
|
SARIF is probably the coolest addition :) |
|
Please look into the comments, Fix them and ill merge this. This is a very cool PR. OpenRabbit is now a GitHub Security Scanner. The Beautiful Workflow Summary will actually make this look like a polished product. Man I can see 10 new features. |
There was a problem hiding this comment.
Was this intentional? If so, was there a compatibility issue with v5?
| core.setOutput('sarif-file', sarifPath) | ||
| } | ||
|
|
||
| run().catch((error) => core.setFailed(`${error}`)); |
There was a problem hiding this comment.
This is a real robustness issue.
| name: Bundle & verify dist | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
Maybe add a short comment explaining why only Node 20 uploads coverage. Good Catch
| run: npm run build | ||
| - name: Verify committed dist is up to date | ||
| run: git diff --exit-code dist | ||
|
|
There was a problem hiding this comment.
v1 is acceptable so its fine. Ignore this
| @@ -19,8 +25,13 @@ | |||
| "undici": "^5.0.0" | |||
| }, | |||
| "devDependencies": { | |||
There was a problem hiding this comment.
The proposed shell command is more complicated than necessary.
It doesn't improve debugging much. ignore this
| fullDescription: { text: `OpenRabbit flagged this as a ${type} issue.` }, | ||
| defaultConfiguration: { level: LEVEL_BY_TYPE[type] }, | ||
| helpUri: 'https://github.com/aryanbrite/openrabbit', | ||
| })) |
|
hey Aryanbrite/Openrabbit, there are a little more surprises can be done
and be near the level of coderabit and you can do it
…On Thu, Jul 9, 2026 at 6:41 PM Aryan Brite ***@***.***> wrote:
***@***.**** requested changes on this pull request.
------------------------------
On .github/workflows/auto-version.yml
<#82 (comment)>:
Was this intentional? If so, was there a compatibility issue with v5?
------------------------------
In src/sarif.ts
<#82 (comment)>:
> + 'scope-drift',
+ 'reuse',
+ 'suggestion',
+ 'style',
+ 'question',
+]
+
+export function buildSarif(response: ReviewResponse, toolName = 'OpenRabbit'): string {
+ const rules = ALL_TYPES.map((type) => ({
+ id: `openrabbit/${type}`,
+ name: `openrabbit/${type}`,
+ shortDescription: { text: `OpenRabbit ${type} finding` },
+ fullDescription: { text: `OpenRabbit flagged this as a ${type} issue.` },
+ defaultConfiguration: { level: LEVEL_BY_TYPE[type] },
+ helpUri: 'https://github.com/aryanbrite/openrabbit',
+ }))
Please look into this.
------------------------------
In .github/workflows/ci.yml
<#82 (comment)>:
> + - run: npm ci
+ - name: Run tests with coverage
+ run: npm run test:coverage
+ - name: Upload coverage report
+ if: ${{ matrix.node == 20 }}
+ uses: ***@***.***
+ with:
+ name: coverage
+ path: coverage/
+ if-no-files-found: ignore
+
+ build:
+ name: Bundle & verify dist
+ runs-on: ubuntu-latest
+ steps:
+ - uses: ***@***.***
Maybe add a short comment explaining why only Node 20 uploads coverage.
Good Catch
------------------------------
In .github/workflows/ci.yml
<#82 (comment)>:
> +
+ build:
+ name: Bundle & verify dist
+ runs-on: ubuntu-latest
+ steps:
+ - uses: ***@***.***
+ - uses: ***@***.***
+ with:
+ node-version-file: .nvmrc
+ cache: npm
+ - run: npm ci
+ - name: Build bundle
+ run: npm run build
+ - name: Verify committed dist is up to date
+ run: git diff --exit-code dist
+
v1 is acceptable so its fine. Ignore this
------------------------------
In package.json
<#82 (comment)>:
> @@ -19,8 +25,13 @@
"undici": "^5.0.0"
},
"devDependencies": {
The proposed shell command is more complicated than necessary.
It doesn't improve debugging much. ignore this
------------------------------
In src/action.ts
<#82 (comment)>:
> }
-run().catch((error) => core.setFailed(`${error}`));
This is a real robustness issue.
—
Reply to this email directly, view it on GitHub
<#82?email_source=notifications&email_token=A3L735MKRPVTBLABEBTNL6D5D64LDA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINRWGQZTCMJYHA22M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-4664311885>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A3L735KLBLRB6VXXADBI77L5D64LDAVCNFSNUABGKJSXA33TNF2G64TZHMYTEMRTGU3TSNJWHA5US43TOVSTWNBYGQ3DOOJYGE2DNILWAI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
I have tested this by myself. Its working good. Would appreciate if you fix some fixes pointed above |
91800cd to
a48696e
Compare
There was a problem hiding this comment.
Verdict
ready to merge
Primary Goal
Implement a comprehensive CI/CD pipeline with linting, testing, builder, SARIF reporting, and job-summary rendering for the OpenRabbit action.
Overview
This PR adds a robust CI/CD pipeline for the OpenRabbit action, including ESLint/Prettier/typechecking, Node 20/22 testing with coverage, @vercel/ncc bundling, SARIF output for GitHub code scanning, and job-summary rendering. It addresses prior gaps in automated quality checks, self-contained distribution, and security/quality visibility.
Scope Assessment
All changes align with the PR's stated goal of establishing DevOps infrastructure. No detectable scope drift; all modified files (.github/workflows/*.yml, src/action.ts, tooling configs) directly support the CI/CD enhancements.
Risk Assessment
Low risk: Secrets are properly hidden via GitHub Actions secrets, SARIF enables security/quality scanning, and dependency management is now automated. No critical vulnerabilities identified in the changes.
Reuse Notes
- Reuses existing ESLint/Prettier configurations
- Leverages
@vercel/nccfor bundling aligned with existing tooling - SARIF schema follows standard format for GitHub integration
Action Items
- Ensure
LLM_API_KEYsecret is configured in repository settings - Verify all workflow jobs pass locally with
npm test
|
right now I am doing the edits and I have a question. beside grok and
openrouter API does it accept other providers? Anthropic, openai, deepseek?
…On Thu, Jul 9, 2026 at 7:00 PM Aryan Brite ***@***.***> wrote:
*aryanbrite* left a comment (aryanbrite/openrabbit#82)
<#82 (comment)>
I have tested this by myself. Its working good. Would appreciate if you
fix some fixes pointed above
—
Reply to this email directly, view it on GitHub
<#82?email_source=notifications&email_token=A3L735PPAAFZW7NYVLI3EOL5D66S5A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJSG4YDQNRUG4YKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4927086470>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A3L735OIN4AW3MISMEMJIB35D66S5AVCNFSNUABGKJSXA33TNF2G64TZHMYTEMRTGU3TSNJWHA5US43TOVSTWNBYGQ3DOOJYGE2DNILWAI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
No, I wanna add compatibility for all models. Let me know if you can do that. Otherwise, ill do this weekend |
a48696e to
f13a55b
Compare
|
I could do actually but do you have other improvements in mind beside the
compabillities to all providers? I could take a look to see if its easy to
be done and do it also
…On Thu, Jul 9, 2026 at 7:03 PM Aryan Brite ***@***.***> wrote:
*aryanbrite* left a comment (aryanbrite/openrabbit#82)
<#82 (comment)>
right now I am doing the edits and I have a question. beside grok and
openrouter API does it accept other providers? Anthropic, openai, deepseek?
… <#m_3522859429509242873_>
On Thu, Jul 9, 2026 at 7:00 PM Aryan Brite *@*.*> wrote: aryanbrite left
a comment (aryanbrite/openrabbit#82
<#82>) <#82 (comment)
<#82 (comment)>>
I have tested this by myself. Its working good. Would appreciate if you fix
some fixes pointed above — Reply to this email directly, view it on GitHub
<#82
<#82>?email_source=notifications&email_token=A3L735PPAAFZW7NYVLI3EOL5D66S5A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJSG4YDQNRUG4YKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4927086470>,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/A3L735OIN4AW3MISMEMJIB35D66S5AVCNFSNUABGKJSXA33TNF2G64TZHMYTEMRTGU3TSNJWHA5US43TOVSTWNBYGQ3DOOJYGE2DNILWAI
<https://github.com/notifications/unsubscribe-auth/A3L735OIN4AW3MISMEMJIB35D66S5AVCNFSNUABGKJSXA33TNF2G64TZHMYTEMRTGU3TSNJWHA5US43TOVSTWNBYGQ3DOOJYGE2DNILWAI>
. You are receiving this because you authored the thread.Message ID: @.*>
No, I wanna add compatibility for all models. Let me know if you can do
that. Otherwise, ill do this weekend
—
Reply to this email directly, view it on GitHub
<#82?email_source=notifications&email_token=A3L735J336II2L6VVZZQ5ED5D6665A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJSG4YTCNBWHAYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4927114680>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A3L735OB2BNVNUFI4OG77DD5D6665AVCNFSNUABGKJSXA33TNF2G64TZHMYTEMRTGU3TSNJWHA5US43TOVSTWNBYGQ3DOOJYGE2DNILWAI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Verdict
needs changes
Primary Goal
Hook up CI/CD – bundle the action, emit SARIF and run‑summary, and make the action self‑contained.
Overview
The PR adds robust CI tooling and surface finding data via SARIF and a workflow run summary. Core logic for generating SARIF and markdown exists, though the action entrant (src/action.ts) never produces those artifacts. Consequently, the action will finish without setting the declared sarif-file output or writing the summary to the workflow UI.
Key consequences:
- The action appears to run successfully (no errors), but the SARIF file is never written.
- coupe for downstream tooling (GitHub Code Scanning, workflow summaries) is broken.
- Unused imports hint at incomplete integration.
Fixing this will make the CI pipeline useful.
f13a55b to
a7f5cc3
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
There was a problem hiding this comment.
Verdict
ready to merge
Primary Goal
Establish CI/CD pipeline with linting, testing, bundling, and SARIF output for OpenRabbit action
Overview
This PR implements a comprehensive CI/CD workflow that automates linting (ESLint/Prettier), testing (Node 20/22 with coverage), and action bundle generation. It adds SARIF Secure Analysis output for code scanning and job-summary rendering. Key additions include distributed workflow files, updated package.json with new dependencies, and enhanced action.ts/reviewer.ts logic for CI integration.
Scope Assessment
All changes directly implement the stated goal of establishing devops infrastructure. The workflow definitions and code additions align with the PR's stated purpose without scope drift.
Risk Assessment
Low risk. Proper secret handling via GitHub Actions Secrets, deterministic dist bundle verification, and comprehensive test coverage. No security vulnerabilities detected in the implementation.
Reuse Notes
- Standard GitHub Actions workflow structure reused from existing patterns
- Action.ts logic follows established OpenRabbit patterns
Action Items
- Verify LLM_API_KEY secret is configured in repository
Separate PR Suggestions
- Extract config cleanup suggestions into separate PR if found
Inline comments (could not be placed directly)
-
.github/workflows/pr-review.yml#L22: Ensure LLM_PROVIDER variable is properly sanitized before use
-
src/action.ts#L58: Why is
esModuleInteroprequired in tsconfig.json if all modules are ESM?
| jobs: | ||
| quality: | ||
| name: Lint, format & typecheck | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
suggestion: Consider adding node:18 matrix for broader compatibility testing
| runs-on: ubuntu-latest | |
| Add `node:18` to the matrix strategy in the test job |
a7f5cc3 to
108fc96
Compare
|
LGTM !!! |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
I will remove comments like this. I will work on this tmw. I was thinking to work on the language of the reviews and was thinking to add some look features like visual infographic generation. but that would be a completely different this. Now this project pretty much holds the basic features it needs. I would love to give you the maintainer access if you are willing to :)
|
|
Yes of course I will gladly accept it
…On Thu, Jul 9, 2026 at 7:25 PM Aryan Brite ***@***.***> wrote:
*aryanbrite* left a comment (aryanbrite/openrabbit#82)
<#82 (comment)>
I will remove comments like this. I will work on this tmw. I was thinking
to work on the language of the reviews and was thinking to add some look
features like visual infographic generation. but that would be a completely
different this. Now this project pretty much holds the basic features it
needs. I would love to give you the maintainer access if you are willing to
:)
Overview
User Safety: safe
—
Reply to this email directly, view it on GitHub
<#82?email_source=notifications&email_token=A3L735IQ6EIX2FX2JPV5MLT5D7BN3A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJSG4ZDSNZQGIZKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4927297022>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A3L735LX7JZEITHOJRQ6WIT5D7BN3AVCNFSNUABGKJSXA33TNF2G64TZHMYTEMRTGU3TSNJWHA5US43TOVSTWNBYGQ3DOOJYGE2DNILWAI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
hey why dont you take a look at https://github.com/shahrryyar/Reguant its
an alternative to coolify which I think is better for the reason it needs
least RAM memory
…On Thu, Jul 9, 2026 at 7:25 PM Shada Sabir ***@***.***> wrote:
Yes of course I will gladly accept it
On Thu, Jul 9, 2026 at 7:25 PM Aryan Brite ***@***.***>
wrote:
> *aryanbrite* left a comment (aryanbrite/openrabbit#82)
> <#82 (comment)>
>
> I will remove comments like this. I will work on this tmw. I was thinking
> to work on the language of the reviews and was thinking to add some look
> features like visual infographic generation. but that would be a completely
> different this. Now this project pretty much holds the basic features it
> needs. I would love to give you the maintainer access if you are willing to
> :)
>
> Overview
>
> User Safety: safe
>
> —
> Reply to this email directly, view it on GitHub
> <#82?email_source=notifications&email_token=A3L735IQ6EIX2FX2JPV5MLT5D7BN3A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJSG4ZDSNZQGIZKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4927297022>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/A3L735LX7JZEITHOJRQ6WIT5D7BN3AVCNFSNUABGKJSXA33TNF2G64TZHMYTEMRTGU3TSNJWHA5US43TOVSTWNBYGQ3DOOJYGE2DNILWAI>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
|
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
There was a problem hiding this comment.
Verdict
needs changes
Primary Goal
Add CI/CD pipeline, bundle dist, and SARIF + job-summary features
Overview
This PR introduces a comprehensive CI workflow that covers linting, formatting, type‑checking, unit tests, and a build job that bundles the action with ncc. It also adds a new sarif.ts module that serializes review results into a SARIF 2.1.0 file and a summary.ts formatter that writes a Markdown summary to GITHUB_STEP_SUMMARY. The action entrypoint has been switched from a composite action to a direct node20 action using the committed dist/index.mjs. New testing harnesses validate the SARIF generation and summary rendering.
The intent is to have the action be fully reproducible without a build step, expose machine‑readable findings, and surface a concise run‑summary for operators.
108fc96 to
acc298a
Compare
|
hey may I know which LLM is configured to review the code?
…On Thu, Jul 9, 2026 at 7:34 PM github-actions[bot] ***@***.***> wrote:
***@***.***[bot]* commented on this pull request.
Verdict
*needs changes*
Primary Goal
Add CI/CD pipeline, bundle dist, and SARIF + job-summary features
Overview
This PR introduces a comprehensive CI workflow that covers linting,
formatting, type‑checking, unit tests, and a build job that bundles the
action with ncc. It also adds a new sarif.ts module that serializes
review results into a SARIF 2.1.0 file and a summary.ts formatter that
writes a Markdown summary to GITHUB_STEP_SUMMARY. The action entrypoint
has been switched from a composite action to a direct node20 action using
the committed dist/index.mjs. New testing harnesses validate the SARIF
generation and summary rendering.
The intent is to have the action be fully reproducible without a build
step, expose machine‑readable findings, and surface a concise run‑summary
for operators.
—
Reply to this email directly, view it on GitHub
<#82?email_source=notifications&email_token=A3L735N2CYUIVGSLZDVVRHD5D7CSBA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINRWGQ4TENBWGQZ2M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-4664924643>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A3L735MZ37LCAOH3KTYWJFT5D7CSBAVCNFSNUABGKJSXA33TNF2G64TZHMYTEMRTGU3TSNJWHA5US43TOVSTWNBYGQ3DOOJYGE2DNILWAI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Verdict
needs changes
Primary Goal
Add a CI pipeline, commit a prebuilt ncc bundle, and emit SARIF + job-summary output for the OpenRabbit reviewer action.
Overview
What this PR does
- Introduces a full CI workflow (
.github/workflows/ci.yml) with quality, test (Node 20/22), build/check-dist, and actionlint jobs. - Switches
action.ymlfrom a composite action to a self-containednode20action using a committeddist/index.mjsbundle produced by@vercel/ncc. - Adds
src/sarif.ts(SARIF 2.1.0 generation) andsrc/summary.ts(GitHub step-summary markdown) plus unit tests. - Fixes dependabot config, pins action versions, and corrects a typo in
pr-review.yml.
Concerns
.prettierignoreexcludes all source files (src/reviewer.ts,src/cli.ts,src/types.ts, etc.). This meansprettier --checkwill never validate the actual code, defeating the purpose of the format gate in CI.- Minor scope drift: whitespace/quote tweaks and
actions/checkout@v3 → v4bumps inacknowledge-contributors.ymlandauto-version.ymlare unrelated to the CI/SARIF feature. - CLI bin removed:
package.jsonno longer declares thebinentry forsrc/cli.ts, which may silently break the previously documented CLI usage. - Entrypoint execution:
src/action.tsnowexport async function run(), but the bundleddist/index.mjsmust actually invokerun()(the CI smoke-test expects a non-zero exit without inputs, which would catch a no-op bundle, but worth confirming).
Verification
Tests for SARIF and summary look solid; the mocked action test asserts SARIF write and summary append correctly.
Scope Assessment
Core changes (CI, bundling, SARIF, summary) are coherent. However, the PR also touches unrelated workflow files for cosmetic reasons and inexplicably ignores source files in Prettier.
Risk Assessment
The Prettier misconfiguration is the main correctness risk: formatted code will not be enforced, allowing drift. Removing the CLI bin may be an unintended breaking change for consumers using the CLI. The switched action runtime relies on a committed bundle being up to date (guarded by check-dist).
Reuse Notes
- Reuses existing
ReviewResponse/ReviewCommenttypes fromsrc/types.tsfor SARIF and summary rendering. - Leverages
@actions/coreinput/output conventions already used insrc/action.ts.
Action Items
- Fix
.prettierignoreso source files are not excluded from formatting checks. - Confirm
dist/index.mjsactually callsrun()(or relies on the existing bottom-level invocation). - Decide whether removing the
binentry forsrc/cli.tsis intentional; if CLI is still supported, restore it. - Consider moving unrelated workflow whitespace/version bumps to a separate maintenance PR.
Separate PR Suggestions
- Extract the cosmetic workflow changes (
.github/workflows/acknowledge-contributors.yml,.github/workflows/auto-version.ymlwhitespace/checkout version bumps) and thepr-review.ymlowner typo fix into a separate 'repo maintenance' PR, keeping this PR focused on CI + SARIF + summary.
| node_modules | ||
| coverage | ||
| package-lock.json | ||
| src/reviewer.ts |
There was a problem hiding this comment.
bug: Why are all src/*.ts files listed in .prettierignore? This excludes the actual source from prettier --check, so the CI format gate will never catch unformatted code. Only ignore build/output artifacts and vendored docs, not the source you want to enforce style on.
| src/reviewer.ts | |
| dist | |
| node_modules | |
| coverage | |
| package-lock.json | |
| .github/CONTRIBUTING.md | |
| .github/FUNDING.YML | |
| .github/copilot-instructions.md |
| }, | ||
| "bin": { | ||
| "coderabbit-pr-reviewer": "dist/cli.js" | ||
| "test:coverage": "vitest run --coverage", |
There was a problem hiding this comment.
question: The bin entry for src/cli.ts was removed. Is the CLI no longer supported? If src/cli.ts is still shipped, removing bin silently breaks npx openrabbit / local CLI usage. Either restore it or confirm the CLI is deprecated.
|
|
||
| permissions: | ||
| contents: write # Allows git push | ||
| contents: write # Allows git push |
There was a problem hiding this comment.
scope-drift: This whitespace fix and checkout@v3 → v4 bump is unrelated to the CI/SARIF feature. It’s harmless but expands the PR’s blast radius. Consider splitting pure maintenance tweaks into a separate PR.
OpenRouter works good with this. |
|
no this action bot reviews I meant, some of his concerns are false
…On Thu, Jul 9, 2026 at 7:38 PM Aryan Brite ***@***.***> wrote:
*aryanbrite* left a comment (aryanbrite/openrabbit#82)
<#82 (comment)>
hey may I know which LLM is configured to review the code?
… <#m_1548319844074632706_>
On Thu, Jul 9, 2026 at 7:34 PM github-actions[bot] *@*.*> wrote: @.*[bot]*
commented on this pull request. Verdict *needs changes* Primary Goal Add
CI/CD pipeline, bundle dist, and SARIF + job-summary features Overview This
PR introduces a comprehensive CI workflow that covers linting, formatting,
type‑checking, unit tests, and a build job that bundles the action with
ncc. It also adds a new sarif.ts module that serializes review results into
a SARIF 2.1.0 file and a summary.ts formatter that writes a Markdown
summary to GITHUB_STEP_SUMMARY. The action entrypoint has been switched
from a composite action to a direct node20 action using the committed
dist/index.mjs. New testing harnesses validate the SARIF generation and
summary rendering. The intent is to have the action be fully reproducible
without a build step, expose machine‑readable findings, and surface a
concise run‑summary for operators. — Reply to this email directly, view it
on GitHub <#82 <#82>?email_source=notifications&email_token=A3L735N2CYUIVGSLZDVVRHD5D7CSBA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINRWGQ4TENBWGQZ2M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-4664924643>,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/A3L735MZ37LCAOH3KTYWJFT5D7CSBAVCNFSNUABGKJSXA33TNF2G64TZHMYTEMRTGU3TSNJWHA5US43TOVSTWNBYGQ3DOOJYGE2DNILWAI
. You are receiving this because you authored the thread.Message ID: *@*
.***>
OpenRouter works good with this.
—
Reply to this email directly, view it on GitHub
<#82?email_source=notifications&email_token=A3L735IPVGY4WM4SAE2DF3T5D7DAFA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJSG42DEMJSGA2KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4927421204>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A3L735MDHUW3GS6FYBNMLL35D7DAFAVCNFSNUABGKJSXA33TNF2G64TZHMYTEMRTGU3TSNJWHA5US43TOVSTWNBYGQ3DOOJYGE2DNILWAI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
oh wait, sorry for misunderstanding. This repo is using openrouter/free cause i wanted to reach the free audiance. feel free to ignore some code reviews by this |
|
I think i should change this to a paid one. ill change this dw :) |
|
okay so I did my job everything passes locally and green on gh hope you
review very soon
…On Thu, Jul 9, 2026 at 7:40 PM Aryan Brite ***@***.***> wrote:
*aryanbrite* left a comment (aryanbrite/openrabbit#82)
<#82 (comment)>
oh wait, sorry for misunderstanding. This repo is using openrouter/free
cause i wanted to reach the free audiance. feel free to ignore some code
reviews by this
openrouter/free sometimes picks up some dumb models
—
Reply to this email directly, view it on GitHub
<#82?email_source=notifications&email_token=A3L735LGOQYITF6IMUMPHCD5D7DJNA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJSG42DIMZZGQ42M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4927443949>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A3L735ORVNMCC2FJTHQTECL5D7DJNAVCNFSNUABGKJSXA33TNF2G64TZHMYTEMRTGU3TSNJWHA5US43TOVSTWNBYGQ3DOOJYGE2DNILWAI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
- Add GitHub Actions CI workflow (lint, format, typecheck, test matrix, build/check-dist, actionlint) and Dependabot + .nvmrc - Switch action.yml from composite to node20 using ncc-bundled dist/index.mjs - Add SARIF 2.1.0 output (security/code-scanning) and GitHub job-summary rendering from the review response - Make runReview return the response so action.ts can emit both artifacts - Fix broken workflow refs (owner, action versions, checkout/dependabot) - Add ESLint 9 + Prettier + Vitest configs and unit tests for new features
acc298a to
d32cad7
Compare
There was a problem hiding this comment.
Verdict
needs changes
Primary Goal
Add a comprehensive CI/CD pipeline, bundle the action distribution, and emit SARIF + job-summary for review results.
Overview
This PR introduces a new CI workflow that lint, test, type‑check, and build the action, verifies the bundled dist/ is up‑to‑date, and generates a SARIF file and a workflow run summary. The action itself was upgraded from a composite action to a node20 node module (dist/index.mjs), and new inputs/outputs (sarif_path, sarif-file) were added to allow callers to consume the findings.
Key changes:
- New GitHub Actions workflow files (
.github/workflows/ci.yml,pr-review.ymladjustments, dependabot config). - Action packaging rewritten using @vercel/ncc and committed
dist/. src/sarif.tsimplements SARIF 2.1.0 generation andsrc/summary.tsrenders a Markdown summary.- Tests updated to validate SARIF output and summary appending.
- Miscellaneous tooling updates (ESLint 9, Prettier 3, Vitest 4).
|
hi, I am sorry. This PR is in my mind. I am just a bit occupied a bit. ill review this in a few days :) |
What we added
.github/workflows/ci.yml) — runs on push/PR tomainwith four jobs:quality: ESLint + Prettier (--check) +tsc --noEmittest: Node 20 & 22 matrix with coverage uploadbuild:@vercel/nccbundle +check-dist(fails if committeddistis out of date)actionlint: validates all workflow YAMLdist/—action.ymlswitched from a composite action tousing: node20withdist/index.mjsproduced byncc. The self-contained bundle is committed, so the action works without a build step at release time.src/sarif.ts) — the review now emits a SARIF 2.1.0 file (openrabbit.sarif) with 7 rules (security/bug→error;scope-drift/reuse/suggestion/style/question→warning).action.ymlexposes asarif-fileoutput so it can be uploaded to GitHub code scanning.src/summary.ts) — the review is rendered into the workflow run summary (GITHUB_STEP_SUMMARY), so results are readable without opening the PR.typescript-eslint, Prettier 3, Vitest 4 +v8coverage,.nvmrc, and unit tests for the new features.dependabot.yml(waspackage-ecosystem: ""), corrected thepr-review.ymlowner typo (aryan6673→aryanbrite), and pinned broken action versions (checkout@v5/v3→v4,action-gh-release@v3→v2).Why we added it
The repo had no DevOps/CI/CD: nothing built, linted, type-checked, or tested the action's own source, and the composite action relied on a runtime build that wasn't part of the repo. Reviews also produced no machine-readable security signal and no visible run summary.
What it solves
distcommitted +check-distguard).Verification (proven locally)
lint,format:check,typecheck, andtest(11 tests; new modules at 100% coverage) all pass.check-distpasses: thenccbundle is deterministic and matches the committeddist.buildSarif+formatSummaryMarkdownexecuted against a realistic 7-type payload: valid SARIF 2.1.0 (correctruleId/ruleIndex/level/locations, untyped comments skipped) and a correctly rendered summary.Notes
pr-review.ymlworkflow referencesaryanbrite/openrabbit@mainand needsLLM_API_KEY(andGITHUB_TOKEN) configured as repo secrets to run on PRs.shahrryyar/openrabbit) because direct push toaryanbrite/openrabbitis denied for this account.