[docs] Harmonized AGENTS.md with @openwisp guidelines - #596
Conversation
Aligned this repository's AGENTS.md with shared @openwisp contributing guidelines and its technology-family conventions. Removed redundant generic instructions while preserving repository-specific requirements, commands, and safety guidance.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous Review Summaries (3 snapshots, latest commit ad08e5d)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit ad08e5d)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous review (commit 91b41b9)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous review (commit d69c0fe)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
AGENTS.md with shared OpenWISP contributing guidelines and aligning the CI pip install command with the development-version guidance already referenced in AGENTS.md. No application code or test changes, so no regression risk. The YAML change is valid and consistent with the documented guidance. |
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR expands Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
Suggested labels: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (7 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 @.github/workflows/ci.yml:
- Line 45: Leave the openwisp-utils installation in the CI workflow unchanged;
the review explicitly accepts tracking the master branch for now. Only consider
replacing the branch tarball with a tagged release or pinned commit SHA in a
future reliability-focused change.
In `@AGENTS.md`:
- Around line 51-52: Consolidate the duplicate testing requirements in the
AGENTS.md file by removing the redundant test execution and verification
instructions from the Contributing Guidelines section (which repeats content
about regression tests and ./run-qa-checks already defined in the Testing and QA
section). Instead, update the Contributing Guidelines section to reference the
Testing and QA section for those specific requirements, keeping implementation
details in a single authoritative location and avoiding documentation
duplication.
- Around line 27-28: Update the generated-artifact guidance lines describing
dist/ and src/css/ to also explicitly include lib/ as a generated artifact,
consistent with the mention of lib/ and dist/ as built artifacts elsewhere in
the document. Clarify that lib/ must not be edited directly and should be
treated the same as dist/ in terms of source-of-truth rules, rebuilding via the
documented build command when generated output is required.
- Line 55: The pip install command in the troubleshooting section for
openwisp-utils references the mutable master branch in the GitHub archive URL.
Replace the master branch reference in the URL with an immutable alternative
such as a specific release tag, commit hash, or version constraint, or add
explicit documentation stating that a floating development branch is
intentionally required. Ensure the installation instruction remains reproducible
across different time periods and environments.
- Line 30: Update the comment guidance in AGENTS.md at the section about opaque
syntax and domain-specific code to change the permissive language from "may also
explain" to mandatory language that requires a concise comment for cryptic or
non-obvious code. The rule must state that complex or opaque code, including
shell scripts and complex commands, must include a comment that explains the
purpose and justifies why the complexity is acceptable, rather than making such
comments optional.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9f3c45be-58e8-4d12-958b-496b025dde80
📒 Files selected for processing (2)
.github/workflows/ci.ymlAGENTS.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: If a pull request is flagged as invalid, immediately stop reviewing it.
Cryptic or non-obvious code, including complex regular expressions or shell commands, must have a concise comment explaining its purpose and why the complexity is acceptable; avoid comments or docstrings when the code is already clear.
Keep code formatting compact and readable, avoiding excessive blank lines, especially inside function or method bodies.
Flag unused or redundant code, and outdated or incorrect documentation, comments, or docstrings.
Use descriptive and consistent names for variables, functions, classes, and files.
Files:
AGENTS.md
**/AGENTS.md
📄 CodeRabbit inference engine (Custom checks)
When AGENTS.md is present, verify that the changed code follows the rules defined in it.
Files:
AGENTS.md
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T15:02:20.447Z
Learning: Prefer short, precise names that rely on their nearest meaningful scope; avoid repeating context already supplied by the containing module, class, or function.
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T15:02:20.447Z
Learning: Add comments or docstrings only when they explain non-obvious reasons, constraints, compatibility or security requirements, side effects, unavoidable complexity, or opaque/domain-specific code; do not merely restate adjacent code.
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T15:02:20.447Z
Learning: Use `yarn test`, `yarn coverage`, the documented browser test flow, `openwisp-qa-format`, `yarn lint:fix`, and `./run-qa-checks` when available; treat unrelated failures as blocking until confirmed and reported.
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T15:02:20.447Z
Learning: Inspect relevant implementation, tests, documentation, and configuration before editing; follow existing patterns and avoid inventing behavior.
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T15:02:20.447Z
Learning: Keep contributions focused and change only necessary lines; avoid unrelated refactors, formatting churn, generated-file changes, and dependency-file changes unless required.
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T15:02:20.447Z
Learning: When requirements, intended behavior, or unexpected failures are unclear, stop and seek clarification instead of making speculative changes.
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T15:02:20.447Z
Learning: When starting work on a new issue, create a branch from `master`; use `issues/<issue-number>-<short-title>` for issue work or a short descriptive name otherwise.
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T15:02:20.447Z
Learning: Commit messages must be descriptive and use past tense; issue commits must use an allowed prefix, a capitalized past-tense subject ending in `#<issue-number>`, and an appropriate issue reference in the body.
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T15:02:20.447Z
Learning: Add an explanatory commit body only for substantial changes, new features, or non-obvious bug fixes; write releasable commit subjects clearly and in user-friendly language.
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T15:02:20.447Z
Learning: Send new commits in response to review feedback instead of amending existing commits.
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T15:02:20.447Z
Learning: If documentation and CI commands differ, use CI for verification, report both paths and commands, and do not change documentation without explicit user direction.
🔇 Additional comments (6)
AGENTS.md (6)
29-29: LGTM!
45-45: LGTM!
47-50: LGTM!
53-54: LGTM!
56-56: 📐 Maintainability & Code QualityVerify the QA and release-tool contract.
Confirm that
openwisp-commit --check,cz -n cz_openwisp info, the listed commit prefixes, and the changelog behavior match the QA dependency and release configuration changed by this pull request. Otherwise, contributors may follow commands that fail or produce commit subjects that the releaser ignores.Based on the PR objectives, this pull request also changes the QA dependency declaration, so verify this cross-file contract before merge.
57-57: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
AGENTS.md (3)
55-55: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSecurity Misconfiguration (CWE-494): Download of Code Without Integrity Check
Reachability: External
Do not install QA tooling from mutable
master.The documented command downloads and installs
openwisp-utilsfrom a mutable branch. An upstream change can alter the installed code without a change in this repository. Pin the archive to an immutable release or commit. If the development branch is required, document that exception and define how to verify the revision.🤖 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 `@AGENTS.md` at line 55, Update the QA dependency installation guidance in AGENTS.md to avoid the mutable openwisp-utils master archive: pin the package to an immutable release or commit. If master is necessary, document the exception and specify how contributors can verify the exact revision installed, while preserving the existing commit-validation instructions.
30-30: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMake the opaque-code comment rule mandatory.
Line 30 says a comment “may also explain” opaque code. The repository rule requires a concise comment for cryptic or non-obvious code, including complex shell commands. The comment must explain the purpose and why the complexity is acceptable.
As per coding guidelines, cryptic or non-obvious code requires a concise explanatory comment.
Proposed clarification
-- In opaque syntax or domain-specific code, especially shell scripts, a comment may also explain what the code does. +- In opaque syntax or domain-specific code, especially complex shell scripts, add a concise comment that explains its purpose and why the complexity is acceptable.🤖 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 `@AGENTS.md` at line 30, Update the comment guidance in AGENTS.md so comments are required, not optional, for cryptic or non-obvious code, including complex shell commands. Require each such comment to concisely explain the code’s purpose and why its complexity is necessary or acceptable, while retaining the prohibition on comments that merely restate clear code.Source: Coding guidelines
27-28: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winInclude
lib/in the generated-artifact rule.Line 10 identifies both
lib/anddist/as built artifacts. Lines 27-28 only restrict direct edits todist/and one CSS copy. This leaveslib/ambiguous and can make generated output inconsistent with its source. State thatlib/must not be edited directly.As per coding guidelines, flag inconsistent or redundant documentation.
Proposed clarification
-- Treat `dist/` and its built CSS, JavaScript, and assets as generated artifacts. Make source changes under `src/`, `public/`, or other tracked build inputs, not in `dist/`, other built assets, or dependency directories such as `node_modules/`. +- Treat `lib/`, `dist/`, and their built CSS, JavaScript, and assets as generated artifacts. Make source changes under `src/`, `public/`, or other tracked build inputs, not in `lib/`, `dist/`, other built assets, or dependency directories such as `node_modules/`.🤖 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 `@AGENTS.md` around lines 27 - 28, Update the generated-artifact guidance in AGENTS.md to explicitly prohibit direct edits to lib/ alongside dist/ and dependency directories. Clarify that changes belong in tracked source/build inputs and that lib/ output must be regenerated through the appropriate build process, while consolidating overlapping wording to avoid redundant documentation.Source: Coding guidelines
🤖 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 `@AGENTS.md`:
- Around line 51-52: Update the contribution guidance in AGENTS.md to remove
repeated QA command details, including ./run-qa-checks, while retaining only the
requirement to report verification failures or blockers. Keep the authoritative
test and QA command instructions in the existing “Testing and QA” section.
---
Duplicate comments:
In `@AGENTS.md`:
- Line 55: Update the QA dependency installation guidance in AGENTS.md to avoid
the mutable openwisp-utils master archive: pin the package to an immutable
release or commit. If master is necessary, document the exception and specify
how contributors can verify the exact revision installed, while preserving the
existing commit-validation instructions.
- Line 30: Update the comment guidance in AGENTS.md so comments are required,
not optional, for cryptic or non-obvious code, including complex shell commands.
Require each such comment to concisely explain the code’s purpose and why its
complexity is necessary or acceptable, while retaining the prohibition on
comments that merely restate clear code.
- Around line 27-28: Update the generated-artifact guidance in AGENTS.md to
explicitly prohibit direct edits to lib/ alongside dist/ and dependency
directories. Clarify that changes belong in tracked source/build inputs and that
lib/ output must be regenerated through the appropriate build process, while
consolidating overlapping wording to avoid redundant documentation.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8a1467b1-98cb-489f-ba19-4125b28209ff
📒 Files selected for processing (1)
AGENTS.md
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Kilo Code Review
🧰 Additional context used
📓 Path-based instructions (2)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: If a pull request is flagged as invalid, immediately stop reviewing it.
Cryptic or non-obvious code, including complex regular expressions or shell commands, must have a concise comment explaining its purpose and why the complexity is acceptable; avoid comments or docstrings when the code is already clear.
Keep code formatting compact and readable, avoiding excessive blank lines, especially inside function or method bodies.
Flag unused or redundant code, and outdated or incorrect documentation, comments, or docstrings.
Use descriptive and consistent names for variables, functions, classes, and files.
Files:
AGENTS.md
**/AGENTS.md
📄 CodeRabbit inference engine (Custom checks)
When AGENTS.md is present, verify that the changed code follows the rules defined in it.
Files:
AGENTS.md
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T19:06:02.122Z
Learning: Use the repository's documented commands for testing and QA, including yarn test, yarn coverage, browser tests, openwisp-qa-format, yarn lint:fix, and ./run-qa-checks when available; treat unrelated failures as blockers only after confirming and reporting them.
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T19:06:02.122Z
Learning: Update documentation or examples when behavior, options, public APIs, setup steps, or supported versions change.
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T19:06:02.122Z
Learning: Before editing, inspect the relevant implementation, tests, documentation, and configuration; follow existing repository patterns and avoid inventing behavior or requirements.
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T19:06:02.122Z
Learning: Keep each contribution focused and change only lines necessary for its goal; avoid unrelated refactors, formatting churn, generated-file changes, and dependency-file changes unless explicitly required.
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T19:06:02.122Z
Learning: When requirements, intended behavior, or unexpected failures are unclear, stop and seek clarification instead of making speculative changes.
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T19:06:02.122Z
Learning: Create a new branch from master when starting an issue; use issues/<issue-number>-<short-title> for issue work or a short descriptive branch name otherwise.
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T19:06:02.122Z
Learning: Use descriptive, past-tense commit messages; issue commits must use an allowed prefix, a capitalized past-tense subject ending with the issue number, and an appropriate issue reference in the body.
Learnt from: CR
Repo: openwisp/netjsongraph.js
Timestamp: 2026-08-04T19:06:02.122Z
Learning: Send new commits in response to review feedback instead of amending existing commits.
🔇 Additional comments (2)
AGENTS.md (2)
23-26: LGTM!Also applies to: 29-29
42-50: LGTM!Also applies to: 53-54, 56-57
- Listed "Contributing Guidelines" rules early on, some rules have been adapted to this repository to avoid ambiguity. - Ensured following rules do not repeat nor contradict the rules listed in "Contributing Guideliens".
|
@coderabbitai do you see any issue? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="openwisp/netjsongraph.js"
pr_number="596"
echo '=== Pull request metadata ==='
gh pr view "$pr_number" --repo "$repo" --json title,state,baseRefName,headRefName,changedFiles,additions,deletions,reviews
echo '=== Pull request diff ==='
gh pr diff "$pr_number" --repo "$repo" --patch
echo '=== AGENTS.md on the pull request branch ==='
if [ -f AGENTS.md ]; then
cat -n AGENTS.md
else
echo 'AGENTS.md is not present in the checked-out pull request branch.'
fi
echo '=== CodeRabbit review threads ==='
gh api graphql \
-F owner="openwisp" \
-F name="netjsongraph.js" \
-F number="$pr_number" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
isResolved
isOutdated
comments(first: 20) {
nodes {
author { login }
body
path
line
}
}
}
}
}
}
}' \
--jq '.data.repository.pullRequest.reviewThreads.nodes[]
| select(any(.comments.nodes[]?; .author.login == "coderabbitai"))
| {isResolved, isOutdated, comments: [.comments.nodes[] | {author: .author.login, path, line, body}]}'Length of output: 64616
The mutable You are interacting with an AI system. |
|
@coderabbitai review |
|
|
@coderabbitai resolve |
✅ Action performedComments resolved and changes approved. |
Checklist
Reference to Existing Issue
N/A: Organization-wide maintenance change.
Description of Changes
AGENTS.mdwith the shared OpenWISP contributor guidance while retaining repository-specific JavaScript and build instructions.Screenshot
N/A: No user interface changes.