PLG+DOCS: plugins dev docs for temp files + adguard export cleanup

.claude/skills/plugin-development/SKILL.md

@@ -0,0 +1,76 @@
+---
+name: plugin-development
+description: Create and run NetAlertX plugins. Use this when asked to create a plugin, run a plugin, test a plugin, or develop plugin functionality.
+---
+
+# Plugin Development
+
+## Expected Workflow
+
+1. Read this skill and `docs/PLUGINS_DEV.md` for full context.
+2. Find or create the plugin in `server/plugins/<code_name>/`.
+3. Read the plugin's `config.json` and script to understand its functionality and settings.
+4. Run: `python3 server/plugins/<code_name>/script.py`
+5. Retrieve the result from `/tmp/log/plugins/last_result.<PREF>.log` quickly — the backend processes and deletes it almost immediately.
+
+## Plugin Structure
+
+```text
+server/plugins/<code_name>/
+├── config.json      # Manifest: settings, data contract, DB column mapping
+├── script.py         # Main script (or equivalent, depending on data_source)
+└── README.md         # Setup/usage docs
+```
+
+- `code_name` must match the folder name.
+- `unique_prefix` drives every setting key and filename (e.g. `ARPSCAN` → `ARPSCAN_RUN`, `last_result.ARPSCAN.log`). Uppercase letters only, no underscores/numbers, must be unique across all plugins.
+- Ensure `sys.path` includes `/app/server/plugins` and `/app/server` (as in `server/plugins/__template/rename_me.py`).
+
+## Settings Pattern
+
+- `<PREF>_RUN`: execution phase (see below). Should default to `"disabled"` for any non-core plugin.
+- `<PREF>_RUN_SCHD`: cron-like schedule — check a similar existing plugin for precedent (e.g. `pihole_api_scan` uses `*/5 * * * *`) rather than inventing a new cadence.
+- `<PREF>_CMD`: script path.
+- `<PREF>_RUN_TIMEOUT`: timeout in seconds — **enforced by the core plugin runner as the whole script's kill-timeout** (`server/plugin.py` passes it straight to `subprocess(..., timeout=...)`). Not a safe per-HTTP-call timeout — don't reuse it for individual network calls in a loop, or one slow call can burn the whole budget and get the process killed before it writes its result file. Two correct alternatives: `config.json`'s `"timeoutMultiplier": true` on a `params[]` entry for a config-declared, known-length loop (see `arp_scan`); `plugin_helper.per_item_timeout()` for a runtime-variable-length loop (see the `_publisher_*` plugins).
+- `<PREF>_WATCH`: columns to watch for changes.
+
+## Data Contract
+
+```python
+from plugin_helper import Plugin_Objects
+
+plugin_objects = Plugin_Objects(RESULT_FILE)
+plugin_objects.add_object(...)       # once per discovered item
+plugin_objects.write_result_file()   # exactly once, at the end
+```
+
+Full column spec: `docs/PLUGINS_DEV_DATA_CONTRACT.md`. Note `helpVal1-4`/`watchedValue1-4` both preserve a real `0`/`False` you pass explicitly — only an omitted (`None`) value defaults to `""`.
+
+## Execution Phases
+
+| Phase | Trigger |
+|-------|---------|
+| `once` | Once at startup |
+| `schedule` | On cron schedule |
+| `always_after_scan` | After every scan |
+| `before_name_updates` | Before name resolution |
+| `on_new_device` | When new device detected |
+| `on_notification` | When notification triggered |
+
+## Plugin Formats
+
+| Format | Purpose | Phase |
+|--------|---------|-------|
+| publisher | Send notifications | `on_notification` |
+| dev scanner | Create/manage devices | `schedule` |
+| name discovery | Discover device names | `before_name_updates` |
+| importer | Import from services | `schedule` |
+| system | Core functionality | `schedule` |
+
+## Before Opening a PR
+
+Check the plugin against the [Conventions Checklist](../../../docs/PLUGINS_DEV.md#conventions-checklist) — `RUN` default, schedule precedent, `RUN_TIMEOUT` semantics, reusing core settings instead of duplicating them, description length (renders in the Settings UI — keep it short), and the multi-instance settings pattern (nested array + popup-form, see `rest_import`, not a hardcoded "primary"/"secondary" pair). Most plugin PR review comments trace back to one of these, and `test/plugins/test_plugin_conventions.py` mechanically enforces the RUN-default, description-length, hardcoded-default-drift, and RUN_TIMEOUT-reuse-in-loop items — run it after touching a plugin.
+
+## Starting Point
+
+Copy `server/plugins/__template/` and customize. Read `docs/PLUGINS_DEV.md` for the full development guide.

.claude/skills/pr-analysis/SKILL.md

@@ -0,0 +1,62 @@
+---
+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 *new* tests under a subdirectory of `test/` that mirrors the source path (e.g. `test/scan/` for `server/scan/`). Don't add new files directly in `test/` root - a handful of existing ones there (e.g. `test_plugin_helper.py`, `test_wol_validation.py`) predate this convention; that's not license to add more, but don't migrate them unprompted either.
+4. **No inline imports:** All imports at the top of the file.
+
+## Before Acting on Any PR Comment
+
+1. Load the `code-standards` skill — all code changes must comply with it before replying.
+2. Load the `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. 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 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.

.claude/skills/testing-workflow/SKILL.md

@@ -0,0 +1,133 @@
+---
+name: testing-workflow
+description: Read before running tests. Detailed instructions for single tests, full suites, authentication, obtaining the API Token, and a real cross-test pollution pitfall. Use this when asked to run tests, check failures, or debug failing tests.
+---
+
+# Testing Workflow
+
+**Crucial:** Tests MUST be run inside the devcontainer to access the correct runtime environment (DB, config, dependencies).
+
+## 0. Pre-requisites: Environment Check
+
+Before running any tests, verify you are inside the development container:
+
+```bash
+ls -d /workspaces/NetAlertX
+```
+
+If this directory does not exist, you are likely on the host machine — load the `devcontainer-management` skill (or its `.github`/`.gemini` equivalents) to enter the container or run commands inside it.
+
+## 1. Check for Pre-Existing Failures First
+
+Before attributing any failure to your own changes, see what was already broken:
+
+```bash
+cd /workspaces/NetAlertX; pytest test/ --tb=no -q 2>&1 | tail -20
+```
+
+Do not fix pre-existing failures unless that is the explicit goal.
+
+## 2. Full Test Suite (default)
+
+Unless the user explicitly asks for "fast"/"quick" tests, run the full suite. Don't optimize for time — comprehensive coverage is the priority.
+
+```bash
+cd /workspaces/NetAlertX; pytest test/
+```
+
+## 3. Fast Unit Tests (only when explicitly requested)
+
+Excludes tests marked `docker` or `feature_complete`:
+
+```bash
+cd /workspaces/NetAlertX; pytest test/ -m 'not docker and not feature_complete'
+```
+
+## 4. Running Specific Tests
+
+```bash
+cd /workspaces/NetAlertX; pytest test/<path_to_test>
+# e.g. pytest test/api_endpoints/test_mcp_extended_endpoints.py
+# or a single test: pytest test/plugins/test_adguard_export.py::TestManagedNames::test_round_trip
+```
+
+## PYTHONPATH
+
+Pre-configured with:
+- `/app` — primary location where Python runs in production
+- `/app/server`, `/app/server/plugins` — symlinks to `/workspaces/NetAlertX/server[/plugins]`
+- `/opt/venv/lib/pythonX.Y/site-packages`, `/usr/lib/pythonX.Y/site-packages`
+- `/workspaces/NetAlertX`, `/workspaces/NetAlertX/server`, `/workspaces/NetAlertX/test`
+
+## Authentication & Environment Reset
+
+After making code changes, reset the environment to pick up the new code and get a fresh `API_TOKEN`:
+
+```bash
+bash /workspaces/NetAlertX/.devcontainer/scripts/setup.sh
+sleep 5   # let nginx/python server/etc. stabilize
+python3 -c "from helper import get_setting_value; print(get_setting_value('API_TOKEN'))"
+```
+
+Use the retrieved token for any subsequent authenticated API/test calls.
+
+### Troubleshooting 403 Forbidden / empty token
+
+1. Confirm the server is running; re-run `setup.sh` if needed.
+2. Verify config loaded: `cat /data/config/app.conf`, or `get_setting_value("API_TOKEN")` returns non-empty.
+
+## Docker Test Image
+
+If the Dockerfile or dependencies changed, rebuild before running tests:
+
+```bash
+docker buildx build -t netalertx-test .
+```
+
+~30 seconds normally, ~90 seconds if the venv stage changed.
+
+## Pitfall: `sys.modules` Stubbing Leaks Across Test Files
+
+Some plugin tests (e.g. `test/plugins/test_ntfy_custom_headers.py`) stub NetAlertX
+modules (`conf`, `helper`, `models.notification_instance`, etc.) via
+`sys.modules[name] = fake_module` so the plugin script can be imported standalone,
+outside the container. Because `sys.modules` is a single process-wide cache shared
+by the whole pytest session, a fake module inserted by one test file silently
+shadows the real module for every other test file collected afterwards — pytest
+imports all test files during collection, before any test runs, so this can happen
+regardless of alphabetical/directory order.
+
+Symptom: `AttributeError: <module 'models.notification_instance'> does not have
+the attribute 'get_setting_value'` (or similar) in an unrelated test file, where
+the module repr has no `from '<path>'` suffix — a giveaway that a stub, not the
+real module, was resolved.
+
+Fix pattern: track which module names your stub actually inserted, and pop them
+back out of `sys.modules` immediately after the one-time import that needed them
+(the already-imported script keeps its bound names regardless):
+
+```python
+_stubbed_module_names = []
+
+def _stub(name, **attrs):
+    if name not in sys.modules:
+        mod = types.ModuleType(name)
+        for k, v in attrs.items():
+            setattr(mod, k, v)
+        sys.modules[name] = mod
+        _stubbed_module_names.append(name)
+
+# ... _stub(...) calls, then the one-time import ...
+import ntfy
+
+for _name in _stubbed_module_names:
+    sys.modules.pop(_name, None)
+```
+
+Reproduce cross-file pollution locally by running the suspect file together with
+the affected one in a single pytest invocation (order matters less than you'd
+think — collection happens for all files first):
+
+```bash
+pytest test/plugins/test_ntfy_custom_headers.py test/backend/test_notification_templates.py -v
+```

.gemini/skills/devcontainer-management/SKILL.md

@@ -26,6 +26,6 @@ Prefix commands with `docker exec <CONTAINER_ID>` to run them inside the environ
 docker exec <CONTAINER_ID> bash /workspaces/NetAlertX/.devcontainer/scripts/setup.sh
 ```
 
-*Note: This script wipes `/tmp` ramdisks, resets DBs, and restarts services (python server, cron,php-fpm, nginx).*
+*Note: This script wipes `/tmp` ramdisks, ensures `/data`, `/data/config`, `/data/db` exist, and restarts services (python server, cron, php-fpm, nginx) by symlinking and running `/entrypoint.sh` (`install/production-filesystem/entrypoint.d/`). By default it does **not** delete existing database or config content - `entrypoint.d/25-first-run-db.sh` and `20-first-run-config.sh` only wipe them when `ALWAYS_FRESH_INSTALL=true` is set in the environment (default: unset/`false`, so content is preserved).*
 
 ```

.gemini/skills/plugin-development/plugin-skill.md

@@ -41,7 +41,7 @@ server/plugins/<code_name>/
 - `<PREF>_RUN`: execution phase
 - `<PREF>_RUN_SCHD`: cron-like schedule
 - `<PREF>_CMD`: script path
-- `<PREF>_RUN_TIMEOUT`: timeout in seconds
+- `<PREF>_RUN_TIMEOUT`: timeout in seconds — **this is enforced by the core plugin runner as the whole script's kill-timeout** (`server/plugin.py` passes it straight to `subprocess(..., timeout=...)`). It is not a safe per-HTTP-call timeout — don't reuse it for individual network calls in a loop, or one slow call can burn the whole budget and get the process killed before it writes its result file.
 - `<PREF>_WATCH`: columns to watch for changes
 
 ## Data Contract
@@ -79,6 +79,10 @@ plugin_objects.write_result_file()  # Exactly once at end
 | importer | Import from services | `schedule` |
 | system | Core functionality | `schedule` |
 
+## Before Opening a PR
+
+Check the plugin against the [Conventions Checklist](../../../docs/PLUGINS_DEV.md#conventions-checklist) in `docs/PLUGINS_DEV.md` — `RUN` default, schedule precedent, `RUN_TIMEOUT` semantics, reusing core settings, description length, and the multi-instance settings pattern. Most plugin PR review comments trace back to one of these.
+
 ## Starting Point
 
 Copy from `server/plugins/__template` and customize. Read `docs/PLUGINS_DEV.md` for the full development guide.

.gemini/skills/pr-analysis/SKILL.md

@@ -11,7 +11,7 @@ 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/`.
+3. **Test file location:** Place *new* tests under a subdirectory of `test/` that mirrors the source path (e.g. `test/scan/` for `server/scan/`). Don't add new files directly in `test/` root - a handful of existing ones there (e.g. `test_plugin_helper.py`, `test_wol_validation.py`) predate this convention; that's not license to add more, but don't migrate them unprompted either.
 4. **No inline imports:** All imports at the top of the file.
 
 ## Before Acting on Any PR Comment
@@ -53,6 +53,7 @@ For each comment, determine:
 - **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