feat: add adguard_export plugin#1649
Open
natecj wants to merge 2 commits into
Open
Conversation
Adds a new NetAlertX plugin that syncs known devices from the NetAlertX database to AdGuard Home as persistent clients, keeping names, MACs, IP addresses, and device-type tags in sync. Also fixes the adguard_import config.json description placeholder and a minor indentation inconsistency in that file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a new AdGuard Home device export plugin: configuration and README, Python implementation that syncs NetAlertX Devices to AdGuard Home persistent clients with managed-name state and optional deletions, and a comprehensive pytest suite; plus a minor description update to the AdGuard import plugin. ChangesAdGuard Export Plugin
Import Plugin Documentation Update
Sequence Diagram(s): sequenceDiagram
participant Main as main()
participant DB as get_netalertx_devices()
participant Builder as build_agrd_client()
participant Sync as sync_to_adguard()
participant AdGuard as AdGuardClient
participant State as managed_names
Main->>DB: query devices with filters
DB-->>Main: list of devices
Main->>Sync: pass devices, flags
Sync->>State: load_managed_names()
State-->>Sync: managed name set
Sync->>AdGuard: get_clients()
AdGuard-->>Sync: existing clients list
Sync->>Builder: build_agrd_client(device)
Builder-->>Sync: client payload (ids,tags,name)
Sync->>AdGuard: add_client()/update_client()/delete_client()
AdGuard-->>Sync: HTTP responses
Sync-->>Main: counts (added, updated, skipped, deleted)
Main->>State: save_managed_names()
Main-->>Main: write result file
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
front/plugins/adguard_export/README.md (1)
136-140: ⚡ Quick winStandardize the maintainer metadata block to
### Other infoformat.This README uses
## Notes; plugin READMEs are expected to use the standard### Other infoblock with canonical labels and date format.Suggested patch
-## Notes +### Other info - Version: 1.0.0 - Author: [natecj](https://github.com/natecj) -- Release Date: 2026-05-10 +- Release Date: 10-May-2026Based on learnings: For each NetAlertX plugin README at
front/plugins/**/README.md, include a### Other infosection with the exact standard field labels andDD-Mon-YYYYrelease date format.🤖 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 `@front/plugins/adguard_export/README.md` around lines 136 - 140, Replace the non-standard "## Notes" block with the canonical "### Other info" metadata block in this README: change the heading to "### Other info", use the exact standard field labels (Version, Maintainer, Release Date) and convert the Release Date to the DD-Mon-YYYY format (e.g., 10-May-2026); update the Author label to "Maintainer" and ensure the maintainer value uses the same notation as other plugins (e.g., [natecj](https://github.com/natecj)).test/plugins/test_adguard_export.py (1)
339-347: ⚡ Quick winAdd a regression test to ensure ID-matched manual clients are not adopted into managed state.
Current tests verify unmanaged clients aren’t deleted when state is empty, but not the case where a manual client is matched by ID during sync and must still remain unmanaged.
Suggested test addition
class TestSyncToAdguard: @@ def test_unmanaged_device_not_deleted(self, tmp_path): # State file is empty — we never added this client state = tmp_path / "state.json" state.write_text(json.dumps({"managed": []})) existing = [{"name": "Manual Client", "ids": ["10.0.0.50"], "tags": []}] agrd = _mock_agrd(existing=existing) with patch("script.STATE_FILE", str(state)): sync_to_adguard(agrd, [], use_mac=True, delete_missing=True) agrd.delete_client.assert_not_called() + + def test_id_matched_manual_client_is_not_marked_managed(self, tmp_path): + state = tmp_path / "state.json" + state.write_text(json.dumps({"managed": []})) + existing = [{"name": "Manual Client", "ids": ["aa:bb:cc:00:00:01", "10.0.0.1"], "tags": ["device_pc"]}] + agrd = _mock_agrd(existing=existing) + with patch("script.STATE_FILE", str(state)): + sync_to_adguard(agrd, [self._device(name="Manual Client")], use_mac=True, delete_missing=False) + managed = load_managed_names() + assert managed == set()🤖 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 `@test/plugins/test_adguard_export.py` around lines 339 - 347, Add a regression test ensuring manual clients matched by ID are not adopted: create a test (e.g. test_unmanaged_device_not_deleted_by_id) that writes an empty managed state to STATE_FILE, mocks existing AdGuard clients with an entry containing the matching ID (use _mock_agrd to return existing), then call sync_to_adguard(agrd, [], use_mac=True, delete_missing=True) and assert agrd.delete_client.assert_not_called(); this mirrors test_unmanaged_device_not_deleted but verifies ID-based matching doesn't convert a manual client into managed state during sync.
🤖 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 `@front/plugins/adguard_export/config.json`:
- Around line 340-533: The visible column objects in the config (the column
definitions mapped to cur_MAC, cur_IP, cur_Name, etc.) are missing show: true so
getColumnDefinitions() (which filters by colDef.show) hides them; update each
visible column object (e.g., the entries with "column": "Object_PrimaryID",
"Object_SecondaryID", "Watched_Value1", "Watched_Value2", "Watched_Value3",
"Watched_Value4", "Extra", "ForeignKey") to include show: true so they render in
the plugin output table.
In `@front/plugins/adguard_export/README.md`:
- Around line 96-114: Update the three fenced code blocks that show file paths
so they include a language identifier (e.g., "text") to satisfy markdownlint
MD040: change the blocks containing "/app/db/state.ADGUARDEXP.json",
"/tmp/log/plugins/script.ADGUARDEXP.log", and
"/tmp/log/plugins/last_result.ADGUARDEXP.log" to use triple backticks with a
language tag (text) instead of plain fences; keep the content unchanged and only
add the language identifier to each fence in README.md.
- Line 46: Update the README entry for ADGUARDEXP_URL to match the actual config
default: change the documented default URL from http://192.168.11.1:3000 to
http://localhost:3000 so it matches the default set for ADGUARDEXP_URL in the
plugin config (check the default value in config.json where ADGUARDEXP_URL is
defined).
In `@front/plugins/adguard_export/script.py`:
- Around line 320-342: The current logic marks any matched existing AdGuard
clients as plugin-managed by adding to managed_names inside the update and skip
branches (references: managed_names, agrd.update_client, existing, device,
client_data), which may cause deletion of manually-created clients later; change
it so we do NOT add or rename entries in managed_names when we merely match or
update an existing client (remove managed_names.add(old_name) /
managed_names.add(device["name"]) from the update and skip branches), and only
add names to managed_names at the single location where we actually create a new
client (the create_client/create branch) or when we can prove the plugin already
owned it; ensure rename logic still updates names for created-managed clients
but never converts a pre-existing manual client into managed-owned.
---
Nitpick comments:
In `@front/plugins/adguard_export/README.md`:
- Around line 136-140: Replace the non-standard "## Notes" block with the
canonical "### Other info" metadata block in this README: change the heading to
"### Other info", use the exact standard field labels (Version, Maintainer,
Release Date) and convert the Release Date to the DD-Mon-YYYY format (e.g.,
10-May-2026); update the Author label to "Maintainer" and ensure the maintainer
value uses the same notation as other plugins (e.g.,
[natecj](https://github.com/natecj)).
In `@test/plugins/test_adguard_export.py`:
- Around line 339-347: Add a regression test ensuring manual clients matched by
ID are not adopted: create a test (e.g. test_unmanaged_device_not_deleted_by_id)
that writes an empty managed state to STATE_FILE, mocks existing AdGuard clients
with an entry containing the matching ID (use _mock_agrd to return existing),
then call sync_to_adguard(agrd, [], use_mac=True, delete_missing=True) and
assert agrd.delete_client.assert_not_called(); this mirrors
test_unmanaged_device_not_deleted but verifies ID-based matching doesn't convert
a manual client into managed state during sync.
🪄 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: e0fd4190-ac85-4f3d-aa4f-7f0ae59be37c
📒 Files selected for processing (5)
front/plugins/adguard_export/README.mdfront/plugins/adguard_export/config.jsonfront/plugins/adguard_export/script.pyfront/plugins/adguard_import/config.jsontest/plugins/test_adguard_export.py
- config.json: add show:true to all visible column definitions so they render in the plugin output table - script.py: fix managed_names adoption bug — update/skip branches no longer add unowned clients to managed state; rename tracking now scoped to plugin-created clients only - README.md: fix ADGUARDEXP_URL default (localhost:3000, not local IP), add language tags to fenced code blocks, normalise metadata block to Other info / Maintainer / DD-Mon-YYYY format - test_adguard_export.py: add regression test for manual client matched by ID not being adopted into managed state Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Collaborator
|
Hi @natecj - thanks a lot for the PR. Let me know once you think it's ready to go and I'll do a close review. |
jokob-sk
reviewed
May 24, 2026
| devices = [] | ||
| conn = None | ||
| try: | ||
| conn = sqlite3.connect(db_path) |
Collaborator
There was a problem hiding this comment.
ideally use existing methods, don't query the DB directly see e.g. server/models/device_instance.py
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
adguard_exportplugin that syncs known devices from the NetAlertX database to AdGuard Home as persistent clients, keeping names, MAC addresses, IP addresses, and device-type tags in syncadguard_importconfig.json description placeholder and a minor indentation inconsistencyNew files
front/plugins/adguard_export/script.py— main sync scriptfront/plugins/adguard_export/config.json— plugin metadata and settingsfront/plugins/adguard_export/README.md— documentationtest/plugins/test_adguard_export.py— 31 pytest tests covering device-type tag mapping, client building, state file round-trips, SQLite queries, and the full sync logicTest plan
pytest test/plugins/test_adguard_export.py -vpasses (31 tests)ADGUARDEXP_DELETE, remove a device from NetAlertX, re-run, verify only that client is removed🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests