-
-
Notifications
You must be signed in to change notification settings - Fork 426
Next release #1740
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Next release #1740
Changes from all commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
16210a9
Initial plan
Copilot 2dc2207
fix: remove localtime modifier from skip_repeated_notifications coold…
Copilot 80f2b03
fix: use db_test_helpers and lowercase MACs in skip_repeated test
Copilot c0718d8
Initial plan
Copilot 283bea2
fix: NIC child presence no longer forces a directly-detected parent o…
Copilot f52cc50
fix: address review feedback on NIC presence logic and tests
Copilot 6208e00
Merge pull request #1738 from netalertx/copilot/fix-skip-repeated-not…
jokob-sk 66db9a4
chore: add missing pr-analysis and logging-standards skills
Copilot 4ed96e9
chore: strengthen test MAC and helper rules in code-standards and pr-…
Copilot f143643
Merge pull request #1739 from netalertx/copilot/fix-nic-child-relatio…
jokob-sk 208fa92
Initial plan
Copilot 0197e7c
fix: escape notification HTML device fields
Copilot 44ed53c
test: cover escaped notification html fallback
Copilot 13da2dd
fix: use valid notification fallback log level
Copilot 3196b80
Merge pull request #1744 from netalertx/copilot/fix-devcomments-xml-i…
jokob-sk f33800f
PLG: UNIFIAPI devVlan, devSite import #1741
jokob-sk 9adb39e
Merge branch 'next_release' of github.com:netalertx/NetAlertX into ne…
jokob-sk 383ab12
PLG: UNIFIAPI devVlan, devSite import #1741
jokob-sk 0e391eb
PLG: UNIFIAPI devVlan, devSite import #1741 + v bump
jokob-sk 85918cc
PLG:ADGUARDIMP add static_leases #1746 #1742
jokob-sk 2ec9033
PLG:ADGUARDIMP add static_leases #1746 #1742
jokob-sk 14a34df
Merge pull request #1748 from netalertx/main
jokob-sk 457281c
PLG: UNIFIAPI devVlan removal #1741 + v bump
jokob-sk 8ecf1ac
FE: Add devComments to columns selection #1751
jokob-sk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| --- | ||
| name: logging-standards | ||
| description: Logging conventions for NetAlertX backend Python code. Use this when adding, modifying, or reviewing log statements. | ||
| --- | ||
|
|
||
| # Logging Standards | ||
|
|
||
| ## Import | ||
|
|
||
| ```python | ||
| from logger import mylog | ||
| ``` | ||
|
|
||
| Never import `logging` directly in application code. Use `mylog` exclusively. | ||
|
|
||
| ## Function Signature | ||
|
|
||
| ```python | ||
| mylog(level, message_or_list) | ||
| ``` | ||
|
|
||
| `message_or_list` can be a plain string or a list of values — the logger joins them with spaces. | ||
|
|
||
| ## Log Levels | ||
|
|
||
| Levels from least to most verbose (higher number = more output): | ||
|
|
||
| | Level | Numeric | When to use | | ||
| |-------|---------|-------------| | ||
| | `"none"` | 0 | Always printed regardless of user setting. Reserve for startup, fatal errors, and one-time permission checks. | | ||
| | `"minimal"` | 1 | Important state transitions visible by default (scan start/end, plugin finish, restart). | | ||
| | `"verbose"` | 2 | Informational progress — what the system is doing without clutter (e.g. "No changes to report"). | | ||
| | `"debug"` | 3 | Developer-level detail — loop decisions, branch taken, counts. | | ||
| | `"trace"` | 4 | Granular per-item tracing — individual device rows, SQL queries, raw values. | | ||
|
|
||
| ## Message Format | ||
|
|
||
| Prefix every message with a `[Module]` tag matching the file/function context: | ||
|
|
||
| ```python | ||
| mylog("debug", [f"[device_handling] Processing MAC: {mac}"]) | ||
| mylog("verbose", ["[Scan] Scan complete — devices updated:", count]) | ||
| ``` | ||
|
|
||
| Use `f-strings` inside a list element, not string concatenation: | ||
|
|
||
| ```python | ||
| # Correct | ||
| mylog("debug", [f"[NIC] parent={parent_mac} nic_online={nic_online}"]) | ||
|
|
||
| # Avoid | ||
| mylog("debug", "[NIC] parent=" + parent_mac + " nic_online=" + str(nic_online)) | ||
| ``` | ||
|
|
||
| ## Timestamp | ||
|
|
||
| `mylog` / `file_print` prepend the current local-timezone time automatically via `timeNowTZ`. Do **not** add a timestamp manually inside the message. | ||
|
|
||
| ## What NOT to Log | ||
|
|
||
| - Do not log raw user input without sanitization. | ||
| - Do not log full SQL query strings at `"none"` or `"minimal"` — use `"trace"` at most. | ||
| - Do not use `print()` in server code — use `mylog`. `file_print` is an internal helper; do not call it directly. | ||
|
|
||
| ## Log File Location | ||
|
|
||
| Written to `{logPath}/app.log` (`logPath` from `const.py` → `/tmp/logs` at runtime). Do not hardcode this path. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| --- | ||
| name: pr-analysis | ||
| description: How to analyze and respond to GitHub PR review comments in NetAlertX. Use this whenever you are addressing PR feedback, review threads, or inline code comments. | ||
| --- | ||
|
|
||
| # PR Analysis | ||
|
|
||
| ## Before Writing Any Test Code — Non-Negotiable Checklist | ||
|
|
||
| Run through this before creating or editing any file under `test/`: | ||
|
|
||
| 1. **Helpers first:** Check `test/db_test_helpers.py` for existing factories (`make_db`, `make_device_dict`, `insert_device_from_dict`, `DummyDB`). Use them. If what you need doesn't exist, add it there — never define it locally in the test file. | ||
| 2. **MAC literals must be lowercase:** Every MAC string in fixtures, parametrize, assertions, docstrings, and comments must be lowercase hex (e.g. `aa:bb:cc:dd:ee:01`). No exceptions. | ||
| 3. **Test file location:** Place tests under a subdirectory of `test/` that mirrors the source path (e.g. `test/scan/` for `server/scan/`). Never put test files directly in `test/`. | ||
| 4. **No inline imports:** All imports at the top of the file. | ||
|
|
||
| ## Before Acting on Any PR Comment | ||
|
|
||
| 1. Load `code-standards` skill — all code changes must comply with it before replying. | ||
| 2. Load `testing-workflow` skill — any test additions or changes must follow it. | ||
| 3. Load any domain-specific skill relevant to the files being changed (e.g. `database-patterns` for DB writes, `settings` for config). | ||
|
|
||
| ## Comment Classification | ||
|
|
||
| For each comment, determine: | ||
|
|
||
| | Type | Action | | ||
| |------|--------| | ||
| | Request for code change | Make the change, validate it, then reply with the short commit hash | | ||
| | Question about code | Reply with a concise answer (no restatement of the question) | | ||
| | Suggestion / feedback | Decide if it is actionable. If yes, act and reply. If not, do not reply. | | ||
| | General / praise | Do not reply. | | ||
|
|
||
| ## Acting on Comments — Step by Step | ||
|
|
||
| 1. **Identify all actionable comments** before touching any file. | ||
| 2. **Load relevant skills** to understand conventions that apply. | ||
| 3. **Prepare a plan** — list each file and the exact change required. | ||
| 4. **Make changes one comment at a time** — keep commits focused. | ||
| 5. **Run targeted tests** after each change (`testing-workflow` skill). | ||
| 6. **Reply** only after the commit is pushed. Include the short SHA. | ||
|
|
||
| ## Reply Guidelines | ||
|
|
||
| - Be concise. Do not summarize or restate the original comment. | ||
| - State what was done and (optionally) why. | ||
| - Include the short commit hash when relevant. | ||
| - Do not thank or compliment the reviewer. | ||
|
|
||
| ## What to Check After Every Batch of Changes | ||
|
|
||
| - **MAC literals lowercase** — grep for uppercase hex in every changed test file: `grep -Pn '[0-9A-F]{2}:[0-9A-F]' test/` must be empty. | ||
|
Comment on lines
+50
to
+52
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Use the same complete MAC-case check in both PR-analysis skills.
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
| - **No local DB helpers** — no `DummyDB`, `make_db`, or inline DDL defined outside `test/db_test_helpers.py`. | ||
| - No inline imports — all imports at the top of the file. | ||
| - Tests live under a subdirectory of `test/` matching the source path, not in `test/` root. | ||
|
|
||
| ## Stacked / Base-Branch Issues | ||
|
|
||
| When a PR targets a non-default branch (e.g. `next_release`): | ||
| - Do **not** retarget the branch yourself; note it in a reply so the author can do it from the GitHub UI. | ||
| - Check CI failures on the **base branch** first before checking your branch. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| --- | ||
| name: netalertx-logging-standards | ||
| description: Logging conventions for NetAlertX backend Python code. Use this when adding, modifying, or reviewing log statements. | ||
| --- | ||
|
|
||
| # Logging Standards | ||
|
|
||
| ## Import | ||
|
|
||
| ```python | ||
| from logger import mylog | ||
| ``` | ||
|
|
||
| Never import `logging` directly in application code. Use `mylog` exclusively. | ||
|
|
||
| ## Function Signature | ||
|
|
||
| ```python | ||
| mylog(level, message_or_list) | ||
| ``` | ||
|
|
||
| `message_or_list` can be a plain string or a list of values — the logger joins them with spaces. | ||
|
|
||
| ## Log Levels | ||
|
|
||
| Levels from least to most verbose (higher number = more output): | ||
|
|
||
| | Level | Numeric | When to use | | ||
| |-------|---------|-------------| | ||
| | `"none"` | 0 | Always printed regardless of user setting. Reserve for startup, fatal errors, and one-time permission checks. | | ||
| | `"minimal"` | 1 | Important state transitions visible by default (scan start/end, plugin finish, restart). | | ||
| | `"verbose"` | 2 | Informational progress — what the system is doing without clutter (e.g. "No changes to report"). | | ||
| | `"debug"` | 3 | Developer-level detail — loop decisions, branch taken, counts. | | ||
| | `"trace"` | 4 | Granular per-item tracing — individual device rows, SQL queries, raw values. | | ||
|
|
||
| ## Message Format | ||
|
|
||
| Prefix every message with a `[Module]` tag matching the file/function context: | ||
|
|
||
| ```python | ||
| mylog("debug", [f"[device_handling] Processing MAC: {mac}"]) | ||
| mylog("verbose", ["[Scan] Scan complete — devices updated:", count]) | ||
| ``` | ||
|
|
||
| Use `f-strings` inside a list element, not string concatenation: | ||
|
|
||
| ```python | ||
| # Correct | ||
| mylog("debug", [f"[NIC] parent={parent_mac} nic_online={nic_online}"]) | ||
|
|
||
| # Avoid | ||
| mylog("debug", "[NIC] parent=" + parent_mac + " nic_online=" + str(nic_online)) | ||
| ``` | ||
|
|
||
| ## Timestamp | ||
|
|
||
| `mylog` / `file_print` prepend the current local-timezone time automatically via `timeNowTZ`. Do **not** add a timestamp manually inside the message. | ||
|
|
||
| ## What NOT to Log | ||
|
|
||
| - Do not log raw user input without sanitization. | ||
| - Do not log full SQL query strings at `"none"` or `"minimal"` — use `"trace"` at most. | ||
| - Do not use `print()` in server code — use `mylog`. `file_print` is an internal helper; do not call it directly. | ||
|
|
||
| ## Log File Location | ||
|
|
||
| Written to `{logPath}/app.log` (`logPath` from `const.py` → `/tmp/logs` at runtime). Do not hardcode this path. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| --- | ||
| name: netalertx-pr-analysis | ||
| description: How to analyze and respond to GitHub PR review comments in NetAlertX. Use this whenever you are addressing PR feedback, review threads, or inline code comments. | ||
| --- | ||
|
|
||
| # PR Analysis | ||
|
|
||
| ## Before Writing Any Test Code — Non-Negotiable Checklist | ||
|
|
||
| Run through this before creating or editing any file under `test/`: | ||
|
|
||
| 1. **Helpers first:** Check `test/db_test_helpers.py` for existing factories (`make_db`, `make_device_dict`, `insert_device_from_dict`, `DummyDB`). Use them. If what you need doesn't exist, add it there — never define it locally in the test file. | ||
| 2. **MAC literals must be lowercase:** Every MAC string in fixtures, `parametrize`, assertions, docstrings, and comments must be lowercase hex (e.g. `aa:bb:cc:dd:ee:01`). No exceptions. | ||
| 3. **Test file location:** Place tests under a subdirectory of `test/` that mirrors the source path (e.g. `test/scan/` for `server/scan/`). Never put test files directly in `test/`. | ||
| 4. **No inline imports:** All imports at the top of the file. | ||
|
|
||
| ## Before Acting on Any PR Comment | ||
|
|
||
| 1. Load `code-standards` skill — all code changes must comply with it before replying. | ||
| 2. Load `testing-workflow` skill — any test additions or changes must follow it. | ||
| 3. Load any domain-specific skill relevant to the files being changed (e.g. `database-patterns` for DB writes, `settings-management` for config). | ||
|
|
||
| ## Comment Classification | ||
|
|
||
| For each comment, determine: | ||
|
|
||
| | Type | Action | | ||
| |------|--------| | ||
| | Request for code change | Make the change, validate it, then reply with the short commit hash | | ||
| | Question about code | Reply with a concise answer (no restatement of the question) | | ||
| | Suggestion / feedback | Decide if it is actionable. If yes, act and reply. If not, do not reply. | | ||
| | General / praise | Do not reply. | | ||
|
|
||
| ## Acting on Comments — Step by Step | ||
|
|
||
| 1. **Identify all actionable comments** before touching any file. | ||
| 2. **Load relevant skills** to understand conventions that apply. | ||
| 3. **Prepare a plan** — list each file and the exact change required. | ||
| 4. **Make changes one comment at a time** — keep commits focused. | ||
| 5. **Run targeted tests** after each change (`testing-workflow` skill). | ||
| 6. **Reply** only after the commit is pushed via `report_progress`. Include the short SHA. | ||
|
|
||
| ## Reply Guidelines | ||
|
|
||
| - Be concise. Do not summarize or restate the original comment. | ||
| - State what was done and (optionally) why. | ||
| - Include the short commit hash when relevant. | ||
| - Do not thank or compliment the reviewer. | ||
|
|
||
| ## What to Check After Every Batch of Changes | ||
|
|
||
| - **MAC literals lowercase** — grep for uppercase hex in every changed test file: `grep -Pn '[0-9A-F]{2}:[0-9A-F]' test/` must be empty. | ||
| - **No local DB helpers** — no `DummyDB`, `make_db`, or inline DDL defined outside `test/db_test_helpers.py`. | ||
| - No inline imports — all imports at the top of the file. | ||
| - Tests live under a subdirectory of `test/` matching the source path, not in `test/` root. | ||
| - Secret scan (`runtime-tools-secret_scanning`) before committing. | ||
|
|
||
| ## Stacked / Base-Branch Issues | ||
|
|
||
| When a PR targets a non-default branch (e.g. `next_release`): | ||
| - Do **not** retarget the branch yourself; note it in a reply so the author can do it from the GitHub UI. | ||
| - Check CI failures on the **base branch** first before checking your branch. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Synchronize the paired PR-analysis workflow. The two documents have different mandatory instructions despite the shared rule requiring identical bodies.
.gemini/skills/pr-analysis/SKILL.md#L17-L21: align the settings reference, reply workflow, and post-batch checks with the Copilot document, or mark platform-specific steps explicitly..github/skills/pr-analysis/SKILL.md#L17-L21: apply the same shared-body policy and isolatereport_progressor secret scanning if those steps are platform-specific.🧰 Tools
🪛 LanguageTool
[style] ~21-~21: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...additions or changes must follow it. 3. Load any domain-specific skill relevant to t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
📍 Affects 2 files
.gemini/skills/pr-analysis/SKILL.md#L17-L21(this comment).github/skills/pr-analysis/SKILL.md#L17-L21🤖 Prompt for AI Agents