chore: add AGENT.md and tune CodeRabbit review config#2065
chore: add AGENT.md and tune CodeRabbit review config#2065Adi-204 wants to merge 6 commits intoasyncapi:masterfrom
Conversation
Adi-204
requested review from
Florence-Njeri,
asyncapi-bot-eve,
derberg,
jonaslagoni and
magicmatatjahu
as code owners
|
What reviewer looks at during PR reviewThe following are ideal points maintainers look for during review. Reviewing these points yourself beforehand can help streamline the review process and reduce time to merge.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds repository-wide Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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)
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 the current code and only fix it if needed.
Inline comments:
In `@AGENT.md`:
- Around line 13-24: Update the fenced code blocks in AGENT.md that show the
directory trees so they include a language identifier (e.g., change ``` to
```text); specifically modify the block containing the "apps/ ... packages/ ..."
tree and the block containing the "<client>/ ... template/ components/ test/
..." tree (also apply same change to the other instance around the "<client>/"
example mentioned in lines 180-189) so both opening fences read ```text to
satisfy markdownlint MD040.
- Around line 92-97: The taxonomy text uses "client" but the repo convention and
existing dirs use "clients"; update the sentence and the type set so they match
the established naming (e.g., change "type ∈ {client, sdk, docs, scripts}" to
"type ∈ {clients, sdk, docs, scripts}" or otherwise consistently refer to
"packages/templates/clients/..."), and update the explanatory bullets
(references to `client` and "client template") to the plural `clients` (or
choose the singular and rename directories across the repo with an accompanying
issue/discussion), ensuring consistency for the Template-type taxonomy block and
any mentions of `packages/templates/<type>/...`.
In `@packages/templates/clients/websocket/test/README.md`:
- Line 138: Replace the shell-specific example "TEST_CLIENT=python jest" with a
cross-platform instruction: either reference the npm script "npm run
test:python" (ensure package.json contains a "test:python" script that sets
TEST_CLIENT and runs jest) or recommend using cross-env (e.g., a "test:python"
script that uses cross-env to set TEST_CLIENT before invoking jest) so Windows
users can run the same command; update the README.md line to show the
cross-platform approach and mention adding "-u" for updating snapshots.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ffcecab0-5e30-4c79-91d0-b2dbc870968e
📒 Files selected for processing (3)
.coderabbit.yamlAGENT.mdpackages/templates/clients/websocket/test/README.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENT.md`:
- Around line 53-55: Update the release-prefix guidance in AGENT.md so it
matches section 2.5: replace the current sentence that limits triggers to only
`feat:`/`fix:` with the full, unified set of release-triggering prefixes
(`feat:`, `feat!:`, `fix:`, `fix!:`, and `chore(release):`), and ensure the note
about requiring a `.changeset/*.md` for `feat:`/`fix:` diffs in publishable
packages remains intact; update any example or wording that conflicts (search
for the current line mentioning `feat:` / `fix:` and the later mention of
`feat!:/fix!:/chore(release):`) so both sections use identical wording.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENT.md`:
- Line 54: Update the rule sentence that currently reads "Any `feat:` or `fix:`
in the diff of a publishable package..." to also require the breaking-change
variants by including backtick-marked tokens `feat!:`, `fix!:` (i.e., change the
phrase to "Any `feat:`, `fix:`, `feat!:`, or `fix!:` in the diff of a
publishable package ... must be accompanied by a `.changeset/*.md` file");
ensure the wording clearly ties `!`-marked commits to major-bump changesets so
it matches the release/major-bump rules elsewhere in the document.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENT.md`:
- Line 79: Update the sentence on Line 79 of AGENT.md to explicitly list all
five patterns the release workflow checks (startsWith(..., 'feat:'),
startsWith(..., 'feat!:'), startsWith(..., 'fix:'), startsWith(..., 'fix!:'),
and startsWith(..., 'chore(release):')) and note that the workflow uses separate
startsWith() calls for each, so scoped prefixes like 'feat(generator):' will not
trigger a release; keep the rest of the paragraph unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
| Hard requirements from the root config (non-exhaustive — see `.eslintrc` for the full list): | ||
| - **`--max-warnings 0`** in every lint script. A warning is a blocker. | ||
| - `eqeqeq: error`, `no-var: error`, `prefer-const: error`, `prefer-template: error`, `prefer-arrow-callback: error`, `object-shorthand: error`. | ||
| - `quotes: single`, `semi: always`, `indent: 2 spaces`, `brace-style: 1tbs`, `no-multiple-empty-lines: max 1`. | ||
| - `sonarjs/cognitive-complexity: warn` — treat as a signal to split a function, not to add an `// eslint-disable` line. | ||
| - `camelcase` is disabled (templates generate snake_case / kebab-case identifiers for foreign languages), but JS source should still use camelCase by convention. | ||
| - `no-unused-vars: error` except for function args — unused function args are allowed because they document signatures. |
There was a problem hiding this comment.
remove, let's make sure agent follows .eslintrc and that is it
| Conventional Commits are enforced by the repo's `pre_merge_checks.title` rule. | ||
| - PR titles start **lowercase**, imperative mood: `add`, `fix`, `refactor`, `document` — not `added`, `fixed`. | ||
| - The prefixes that trigger a changeset release are exactly `feat:`, `feat!:`, `fix:`, `fix!:`, and `chore(release):` (see §2.5 for workflow details). Use `chore:`, `docs:`, `test:`, `refactor:`, `ci:` for non-releasing changes. | ||
| - Any `feat:`, `fix:`, `feat!:`, or `fix!:` in the diff of a publishable package (`apps/*`, `packages/components`, `packages/helpers`) **must be accompanied by a `.changeset/*.md` file** naming the affected packages and the bump level — `!`-marked commits must pair with a `major` changeset (see §2.5). Flag missing changesets on review. |
There was a problem hiding this comment.
better reference Contributing.md or development - I don't remember but there is a section about conventional commits
| - Changesets live in `.changeset/*.md` with frontmatter naming each affected `@asyncapi/*` package and the bump level (`patch`/`minor`/`major`). | ||
| - When a PR touches both an app and a consumed package (e.g. `packages/helpers` + `apps/generator`), both must appear in the changeset if either's public behaviour changes. | ||
| - Dependency bumps done by `dependabot`/`asyncapi-bot` are exempt from the changeset rule. | ||
| - **Release-triggering prefixes are narrower than Conventional Commits.** The release workflow (`.github/workflows/release-with-changesets.yml`) only fires on master-push commits whose message **exactly starts with** `feat:`, `feat!:`, `fix:`, `fix!:`, or `chore(release):`. The workflow's `if:` uses five separate calls — `startsWith(..., 'feat:')`, `startsWith(..., 'feat!:')`, `startsWith(..., 'fix:')`, `startsWith(..., 'fix!:')`, and `startsWith(..., 'chore(release):')` — each matching its literal prefix, so **scoped prefixes like `feat(generator):` do NOT trigger a release** — squash/rebase into an unscoped `feat:` / `fix:` at merge time if a release is intended. `refactor:`/`perf:`/`docs:`/`chore:` (without `(release)`) also do not trigger a release. | ||
| - **Major bumps use `!`.** `feat!:` or `fix!:` signals a breaking change and must be paired with a `major` changeset. Don't accept `major` changesets unless the PR title also carries the `!`. | ||
| - **The `chore(release): release and bump versions of packages` PR is bot-authored** by `asyncapi-bot` via `changesets/action`. Do not rewrite its title or squash it under a different prefix — the exact `chore(release):` prefix is what re-fires the workflow to publish to npm. |
There was a problem hiding this comment.
reference release guide from dev guide
| 1. **Parser-API, not shape-poking.** When extracting data from an AsyncAPI document, use the Parser API. Never do `binding.json()["query"]["properties"]` or equivalent shape-reaching. If the Parser lacks what you need, add a helper in `packages/helpers` instead of inlining. | ||
| 2. **Tested helpers over components over inline logic.** Template code should depend on **well-tested** `@asyncapi/generator-helpers` first, then shared components in `@asyncapi/generator-components`, and only drop into template-local components as a last resort. Templates must not reach into the Parser API directly when an equivalent helper exists (or could exist). | ||
| 3. **Reusability budget.** Every new template or template feature must be built with reusability in mind — custom helpers and template-local components are kept to the minimum. A new shared helper/component is justified when used in templates, or when the logic is too gnarly to test inline. Otherwise, keep it in the template. | ||
| 4. **Template-type taxonomy (provisional, opinionated).** Templates live under `packages/templates/<type>/<protocol>/<language>` where `type ∈ {clients, sdk, docs, scripts}`. This split is intentionally opinionated and expected to evolve as the types are exercised in practice — treat it as a hard constraint today, but flag friction rather than forking the taxonomy silently. Semantics: | ||
| - `clients` — generates a client library consumable from a server, a standalone app, **or** a server under development (there is deliberately no `server` type — a client used during server development covers that case). | ||
| - `sdk` — richer project generation; an `sdk` template **should extend an existing `clients` template** rather than fork it. If extension isn't feasible, that's a discussion, not a drop-in. | ||
| - `docs` — wraps standard doc generators (HTML, Markdown) that often already exist outside this repo. | ||
| - `scripts` — generates operational scripts (e.g. broker topic setup). | ||
| Do not introduce a new top-level type without an issue + discussion. | ||
| 5. **Microcks acceptance tests** are the gate for "production-ready" status. A new client template without a Microcks-backed acceptance test (mocks + runtime) is experimental and must be labelled as such in its README. See `packages/templates/clients/websocket/test` for the reference implementation. | ||
| 6. **Spec-first feature work.** Every new template feature must be cross-referenced against the AsyncAPI 3.0 spec **and** the Parser API before implementation. Link the spec section (raw docs or visualizer) in the PR description, and confirm the data is reachable via Parser API capabilities rather than a workaround. |
| - Async I/O uses promisified `fs` wrappers in `lib/utils.js`. Do not use sync `fs` calls in new code. | ||
| - Error handling: validate inputs in constructors (see `GENERATOR_OPTIONS` whitelist in `lib/generator.js`); reject with contextual messages; log at `log.debug`/`log.warn` via `loglevel` — never `console.log`. | ||
| - User-facing strings live in `lib/logMessages.js` as functions returning strings. Do not inline user-facing strings at call sites — it breaks i18n/consistency. | ||
| - Template configuration is loaded from `.ageneratorrc` → `package.json` in `lib/templates/config/loader.js`. |
There was a problem hiding this comment.
not sure it is giving any value to the agent
|
|
||
| ### 4.2 `apps/hooks` — `@asyncapi/generator-hooks` | ||
|
|
||
| **Role:** built-in generator lifecycle hooks (currently a `generate:after` hook that writes the source AsyncAPI doc alongside output). |
There was a problem hiding this comment.
do not mention what hooks we have
|
|
||
| **Conventions:** | ||
| - Exports the hook map shape `{ 'generate:after': fn, ... }` — do not rename or restructure this object without coordinating with `apps/generator/lib/hooksRegistry.js`. | ||
| - YAML vs JSON detection is by try/catch on `JSON.parse`. Keep this simple; do not introduce a format-detection dependency. |
There was a problem hiding this comment.
implementation details of specific hook - remove please
3 participants
Description
Introduces AGENT.md as the source of truth for code review standards across the asyncapi/generator monorepo, consumed by both human reviewers and AI agents (CodeRabbit). Also tightens CodeRabbit review configuration and refreshes the WebSocket integration-test README to reflect the per-client isolation model landed in #2042.
Related issue(s)
Fixes #2064
Summary by CodeRabbit
Documentation
Chores