Conversation
|
Removed the PR template file (.github/pull_request_template.md) per preference; CodeRabbit + CI remain. |
|
CI was failing at npm ci with Node 24.13.0 / npm 11.6.2 due to a lockfile sync check (missing node-gyp-build@4.8.4). Updated package-lock.json so npm ci succeeds on npm 11.6.2. |
omarespejel
force-pushed
the
chore/coderabbit
branch
from
d7ab953 to
424d7e0
Compare
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes introduce CodeRabbit and Greptile configuration files for automated code review, along with GitHub Actions CI workflow updates. These modifications establish automated PR reviews with tool configurations, path-based artifact exclusions, review policies, and deterministic CI checks (typecheck, lint, build, unit tests) to enforce security and code quality standards. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.coderabbit.yaml:
- Around line 18-26: The config lists "main" under base_branches which per the
schema is only for non-default branches; check the repository's default branch
and if it is "main" remove "main" from the base_branches array (or leave a short
comment explaining why you intentionally listed it), otherwise keep it; update
the base_branches key accordingly and ensure enabled: true remains as-is so
CodeRabbit continues reviewing default-branch PRs automatically.
In @.greptile.yaml:
- Around line 1-25: The repository contains a Greptile config in YAML format
with top-level keys like "version", "review", "severity", and "categories"
(currently in .greptile.yaml); Greptile requires JSON, so migrate this YAML into
the supported JSON layout by creating either a greptile.json at repo root or
splitting into .greptile/config.json, .greptile/rules.md and
.greptile/files.json, preserving the same settings (version,
review.comprehensive, review.context_files, review.severity, review.categories)
and validate against the greptile.json schema from the Greptile docs to ensure
the format is correct and will be picked up by the tool.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.json
📒 Files selected for processing (3)
.coderabbit.yaml.github/workflows/ci.yml.greptile.yaml
📜 Review details
🔇 Additional comments (11)
.coderabbit.yaml (4)
1-16: LGTM! Configuration structure is valid and well-documented.The top-level settings and review options are correctly configured:
language: en-USandearly_access: falseare valid per schemaprofile: assertivealigns with PR objectives for thorough reviews- Disabling
poemandin_progress_fortunereduces noise as intended- Summary settings (
high_level_summary_in_walkthrough: true) avoid editing PR bodies
28-35: LGTM! Path filters correctly exclude generated artifacts.The exclusion patterns align with PR objectives:
- Build outputs (
dist/**,**/*.tgz)- Documentation artifacts (
docs/api/**,docs/export/**)- Dependencies and lockfiles (
node_modules/**,**/package-lock.json)
37-57: LGTM! Path instructions provide targeted, security-focused guidance.The instructions appropriately prioritize:
- MCP server: Defense-in-depth for fund-handling code (write gating, amount caps, secret protection)
- SDK: API stability and deterministic behavior
- Tests: Preference for stable unit tests over flaky integration tests
This aligns well with the PR objective to "standardize review focus for security-sensitive areas."
59-79: LGTM! Tool configuration is well-reasoned.Enabled tools provide good coverage:
github-checkswith 5-minute timeout for CI integrationeslintfor JS/TS linting (complements CI)actionlintfor workflow validationgitleaksandtrufflehogfor secret scanning (critical for MCP server security)Disabling
biomeavoids duplicate linting sinceeslintis already enabled. Thetimeout_ms: 300000(5 minutes) is reasonable and within the schema maximum..github/workflows/ci.yml (4)
3-15: LGTM! Workflow triggers are well-configured.The trigger configuration:
- Includes
ready_for_reviewtype to re-run CI when a draft PR becomes ready- Enables
workflow_dispatchfor manual runs- Targets
mainbranch for both PRs and pushesThis aligns with the
auto_review.drafts: falsesetting in.coderabbit.yaml.
20-22: LGTM! Concurrency configuration prevents redundant CI runs.Using
cancel-in-progress: truewith a ref-based group ensures that new pushes cancel stale runs, reducing CI resource waste.
24-31: LGTM! Job configuration with draft PR handling.The condition
github.event_name != 'pull_request' || github.event.pull_request.draft == falsecorrectly:
- Skips CI for draft PRs (aligns with CodeRabbit's
drafts: false)- Always runs on
pushandworkflow_dispatcheventsThe 15-minute timeout provides a reasonable ceiling for the CI steps.
32-59: LGTM! CI steps follow a logical order with proper caching.The workflow:
- Uses
.nvmrcfor consistent Node version- Enables npm caching for faster installs
- Runs checks in a sensible order: typecheck → lint → format → build → test
- Uses
-sflag for cleaner outputThis fulfills the PR objective for "deterministic CI checks (typecheck, lint, build, unit tests)."
.greptile.yaml (3)
26-32: LGTM! Strict TypeScript settings enforce type safety.These settings align with the project's emphasis on correctness for fund-handling code:
strict_null_checksandno_anyprevent common type errorsrequire_error_handlingandno_empty_catchensure errors aren't silently swallowed
34-51: LGTM! Context section aligns with CodeRabbit path instructions.The
key_fileslist appropriately highlights critical entry points for the SDK and MCP server. Thepatternsreinforce the same security principles from.coderabbit.yaml:
- Address validation
- Opt-in writes for state changes
- Amount caps and sanitized errors
This provides consistent guidance across both review tools.
53-64: LGTM! Security and PR settings provide strong guardrails.The security section covers critical concerns for fund-handling code:
- Secret/key detection (
no_hardcoded_secrets,no_api_keys_in_code)- Input validation requirements
- Code injection prevention (
no_eval,no_shell_injection)
block_on_critical: trueadds enforcement beyond CodeRabbit's advisory reviews, though note that actual blocking depends on branch protection rules.
| auto_review: | ||
| enabled: true | ||
| drafts: false | ||
| base_branches: | ||
| - main | ||
| ignore_title_keywords: | ||
| - wip | ||
| - draft | ||
| - do not review |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider whether main needs to be in base_branches.
Per the schema, base_branches is for "base branches (other than the default branch) to review." If main is already the repository's default branch, this entry is redundant (CodeRabbit reviews PRs targeting the default branch automatically when enabled: true).
However, explicitly listing it doesn't cause harm and documents intent clearly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.coderabbit.yaml around lines 18 - 26, The config lists "main" under
base_branches which per the schema is only for non-default branches; check the
repository's default branch and if it is "main" remove "main" from the
base_branches array (or leave a short comment explaining why you intentionally
listed it), otherwise keep it; update the base_branches key accordingly and
ensure enabled: true remains as-is so CodeRabbit continues reviewing
default-branch PRs automatically.
| # Greptile configuration for StarkZap SDK + MCP server | ||
| # https://docs.greptile.com/config | ||
|
|
||
| version: 1 | ||
|
|
||
| review: | ||
| comprehensive: true | ||
| context_files: true | ||
|
|
||
| severity: | ||
| - critical | ||
| - high | ||
| - medium | ||
| - low | ||
|
|
||
| categories: | ||
| - security | ||
| - correctness | ||
| - performance | ||
| - maintainability | ||
| - best-practices | ||
| - error-handling | ||
| - type-safety | ||
| - documentation | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Greptile yaml configuration schema version 1 documentation
💡 Result:
Greptile does not publish a “YAML configuration schema version 1” for its code-review configuration. Greptile’s repo-based configuration is JSON-based, via either:
greptile.json(single file, repo root) — full reference documentation is here. (greptile.com)- A
.greptile/folder using:.greptile/config.json.greptile/rules.md.greptile/files.json— complete schema/reference is here. (greptile.com)
If you meant a different Greptile YAML (e.g., self-hosting/Helm values.yaml, or another product area), tell me the filename you’re looking at and I’ll track down the matching docs.
Citations:
- 1: https://www.greptile.com/docs/code-review/greptile-json-reference
- 2: https://www.greptile.com/docs/code-review/greptile-config-reference
Greptile requires JSON configuration, not YAML.
Greptile does not support .greptile.yaml. According to the official documentation, Greptile's code-review configuration must be in JSON format using either:
greptile.jsonin the repository root, or.greptile/config.json,.greptile/rules.md, and.greptile/files.json
The current .greptile.yaml file will not be recognized or used by Greptile. Migrate the configuration to the proper JSON format referenced in the greptile.json documentation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.greptile.yaml around lines 1 - 25, The repository contains a Greptile
config in YAML format with top-level keys like "version", "review", "severity",
and "categories" (currently in .greptile.yaml); Greptile requires JSON, so
migrate this YAML into the supported JSON layout by creating either a
greptile.json at repo root or splitting into .greptile/config.json,
.greptile/rules.md and .greptile/files.json, preserving the same settings
(version, review.comprehensive, review.context_files, review.severity,
review.categories) and validate against the greptile.json schema from the
Greptile docs to ensure the format is correct and will be picked up by the tool.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Checked the docs and greptile doesn't support yaml
| - name: Unit Tests | ||
| run: npm run -s test | ||
|
|
||
| examples: |
There was a problem hiding this comment.
Why aren't we checking the examples in the ci?
| - name: Lint | ||
| run: npm run -s lint | ||
|
|
||
| - name: Build Flappy-Bird Example | ||
| run: npm run -s build --workspace examples/flappy-bird | ||
| - name: Format check | ||
| run: npm run -s prettier:check | ||
|
|
There was a problem hiding this comment.
Why are we changing the order of the lint, format, and unit test?
3 participants
Closes #31.
This PR adds a repo-tuned CodeRabbit configuration plus baseline CI checks so automated reviews can rely on deterministic results.
What changed
.coderabbit.yamlmain.dist,docs/api, lockfiles, etc.).packages/mcp-server/**security..github/workflows/ci.ymltypecheck,lint,build, and unit tests on PRs andmainpushes..nvmrc..github/pull_request_template.md@coderabbitai summaryand a minimal checklist.Notes
docs.json+ Mintlify json configs (these were failingprettier --checklocally).