diff --git a/.claude/skills/plugin-development/SKILL.md b/.claude/skills/plugin-development/SKILL.md new file mode 100644 index 00000000..eccc9741 --- /dev/null +++ b/.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//`. +3. Read the plugin's `config.json` and script to understand its functionality and settings. +4. Run: `python3 server/plugins//script.py` +5. Retrieve the result from `/tmp/log/plugins/last_result..log` quickly — the backend processes and deletes it almost immediately. + +## Plugin Structure + +```text +server/plugins// +├── 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 + +- `_RUN`: execution phase (see below). Should default to `"disabled"` for any non-core plugin. +- `_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. +- `_CMD`: script path. +- `_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). +- `_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. diff --git a/.claude/skills/pr-analysis/SKILL.md b/.claude/skills/pr-analysis/SKILL.md new file mode 100644 index 00000000..a411eae8 --- /dev/null +++ b/.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 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 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. diff --git a/.claude/skills/testing-workflow/SKILL.md b/.claude/skills/testing-workflow/SKILL.md new file mode 100644 index 00000000..346a272f --- /dev/null +++ b/.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/ +# 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: does not have +the attribute 'get_setting_value'` (or similar) in an unrelated test file, where +the module repr has no `from ''` 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 +``` diff --git a/.gemini/skills/devcontainer-management/SKILL.md b/.gemini/skills/devcontainer-management/SKILL.md index 596aa037..cd77b1fa 100644 --- a/.gemini/skills/devcontainer-management/SKILL.md +++ b/.gemini/skills/devcontainer-management/SKILL.md @@ -26,6 +26,6 @@ Prefix commands with `docker exec ` to run them inside the environ docker exec bash /workspaces/NetAlertX/.devcontainer/scripts/setup.sh ``` -*Note: This script wipes `/tmp` ramdisks, ensures `/data`, `/data/config`, `/data/db` exist, and restarts services (python server, cron, php-fpm, nginx). It does **not** reset or delete any existing database content - it only creates the DB directory if missing.* +*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).* ``` diff --git a/.gemini/skills/skills-index/SKILL.md b/.gemini/skills/skills-index/SKILL.md index b6130205..45ffc23c 100644 --- a/.gemini/skills/skills-index/SKILL.md +++ b/.gemini/skills/skills-index/SKILL.md @@ -1,31 +1,32 @@ --- name: skills-index -description: Index of all available skills across both Gemini CLI (.gemini/skills/) and GitHub Copilot (.github/skills/). Load this to find the right skill for a task, or to locate the counterpart skill in the other AI system. +description: Index of all available skills across Gemini CLI (.gemini/skills/), GitHub Copilot (.github/skills/), and Claude Code (.claude/skills/). Load this to find the right skill for a task, or to locate the counterpart skill in another assistant's tree. --- # Skills Index — Cross-Reference -Two AI assistants are configured for this project, each with their own skill directory: +Three AI assistants are configured for this project, each with their own skill directory: - **Gemini CLI** → `.gemini/skills/` - **GitHub Copilot** → `.github/skills/` +- **Claude Code** → `.claude/skills/` (currently mirrors only the 3 highest-value skills below, not the full set) -Skills with the same purpose exist in both, sometimes under different names and with different depth. This index maps them so you can find the richer version when needed. +Skills with the same purpose exist in more than one, sometimes under different names and with different depth. This index maps them so you can find the richer version when needed. A CI check (`scripts/check_skill_pairs.py`, run as `check-skill-pairs` in `.github/workflows/code-checks.yml`) flags PRs that touch some but not all files in a mirrored group - non-blocking, since some divergence is intentional. --- -## Shared Skills (exist in both) +## Shared Skills (exist in more than one tree) -| Topic | Gemini Skill | Copilot Skill | Notes | -|-------|-------------|--------------|-------| -| Testing | `testing-workflow` | `testing-workflow` | Gemini version emphasises full suite preference and container detection; Copilot version covers `testFailure` tool and PYTHONPATH | -| Settings & config | `settings` | `settings-management` | Gemini version is more comprehensive (22-point guide + PR checklist); Copilot version covers `ccd()` and `get_setting_value()` usage | -| MCP activation | `mcp-activation` | `mcp-activation` | Gemini version covers Gemini CLI session restart; Copilot version covers VS Code window reload | -| Project navigation | `project-navigation` | `project-navigation` | Copilot version has full path tables and env vars; Gemini version is a brief reference | -| Plugin dev | `plugin-development` | `plugin-run-development` | Both cover data contract, phases, formats, the `RUN_TIMEOUT` kill-timer gotcha, and a pre-PR pointer to the Conventions Checklist in `docs/PLUGINS_DEV.md`; kept in sync manually | -| Devcontainer | `devcontainer-management` | `devcontainer-services` + `devcontainer-setup` + `devcontainer-configs` | Gemini combines into one (uses `docker exec`); Copilot splits into 3 focused skills | -| PR review | `pr-analysis` | `pr-analysis` | How to classify and respond to PR comments; pre-flight skill loading checklist | -| Logging | `logging-standards` | `logging-standards` | `mylog` levels, message format, what not to log | +| Topic | Gemini Skill | Copilot Skill | Claude Skill | Notes | +|-------|-------------|--------------|--------------|-------| +| Testing | `testing-workflow` | `testing-workflow` | `testing-workflow` | All three cover the full-suite-by-default rule, PYTHONPATH, auth/token retrieval, and the `sys.modules` stubbing pitfall | +| Settings & config | `settings` | `settings-management` | — | Gemini version is more comprehensive (22-point guide + PR checklist); Copilot version covers `ccd()` and `get_setting_value()` usage | +| MCP activation | `mcp-activation` | `mcp-activation` | — | Gemini version covers Gemini CLI session restart; Copilot version covers VS Code window reload | +| Project navigation | `project-navigation` | `project-navigation` | — | Copilot version has full path tables and env vars; Gemini version is a brief reference | +| Plugin dev | `plugin-development` | `plugin-run-development` | `plugin-development` | All three cover data contract, phases, formats, the `RUN_TIMEOUT` kill-timer gotcha (`timeoutMultiplier`/`per_item_timeout()`), and a pre-PR pointer to the Conventions Checklist in `docs/PLUGINS_DEV.md` | +| Devcontainer | `devcontainer-management` | `devcontainer-services` + `devcontainer-setup` + `devcontainer-configs` | — | Gemini combines into one (uses `docker exec`); Copilot splits into 3 focused skills | +| PR review | `pr-analysis` | `pr-analysis` | `pr-analysis` | How to classify and respond to PR comments; pre-flight skill loading checklist | +| Logging | `logging-standards` | `logging-standards` | — | `mylog` levels, message format, what not to log | --- @@ -64,3 +65,5 @@ When adding a skill, create it in **both** directories to keep both AI systems c - `.github/skills//SKILL.md` — add an entry to the skills table in `.github/copilot-instructions.md` Keep the body content identical between both files. Only the frontmatter `name`/`description` may differ slightly to match each system's discovery heuristics. + +If the skill is high-value enough to also mirror to Claude Code, add `.claude/skills//SKILL.md` too, and add the group to `GROUPS` in `scripts/check_skill_pairs.py` so drift gets flagged. Claude Code has no `activate_skill()`/`testFailure`/`runTests`/`report_progress` equivalents - adapt any such tool references to plain `Bash` commands instead of copying them verbatim. diff --git a/.github/skills/devcontainer-setup/SKILL.md b/.github/skills/devcontainer-setup/SKILL.md index 9b554d62..ac6c661a 100644 --- a/.github/skills/devcontainer-setup/SKILL.md +++ b/.github/skills/devcontainer-setup/SKILL.md @@ -21,7 +21,7 @@ The setup script forcefully resets all runtime state. It is idempotent—every r 4. Links `/entrypoint.d` and `/app` symlinks 5. Creates `/data`, `/data/config`, `/data/db` directories 6. Creates all log files -7. Runs `/entrypoint.sh` to start services +7. Runs `/entrypoint.sh` to start services - by default this **preserves** existing DB/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 8. Writes version to `.VERSION` ## When to Use diff --git a/.github/skills/skills-overview/SKILL.md b/.github/skills/skills-overview/SKILL.md index c116e5a7..7045a0fd 100644 --- a/.github/skills/skills-overview/SKILL.md +++ b/.github/skills/skills-overview/SKILL.md @@ -1,31 +1,32 @@ --- name: skills-overview -description: Index of all available skills across both GitHub Copilot (.github/skills/) and Gemini CLI (.gemini/skills/). Load this to find the right skill for a task, or to locate the counterpart skill in the other AI system. +description: Index of all available skills across GitHub Copilot (.github/skills/), Gemini CLI (.gemini/skills/), and Claude Code (.claude/skills/). Load this to find the right skill for a task, or to locate the counterpart skill in another assistant's tree. --- # Skills Index — Cross-Reference -Two AI assistants are configured for this project, each with their own skill directory: +Three AI assistants are configured for this project, each with their own skill directory: - **GitHub Copilot** → `.github/skills/` - **Gemini CLI** → `.gemini/skills/` +- **Claude Code** → `.claude/skills/` (currently mirrors only the 3 highest-value skills below, not the full set) -Skills with the same purpose exist in both, sometimes under different names and with different depth. This index maps them so you can find the richer version when needed. +Skills with the same purpose exist in more than one, sometimes under different names and with different depth. This index maps them so you can find the richer version when needed. A CI check (`scripts/check_skill_pairs.py`, run as `check-skill-pairs` in `.github/workflows/code-checks.yml`) flags PRs that touch some but not all files in a mirrored group - non-blocking, since some divergence is intentional. --- -## Shared Skills (exist in both) +## Shared Skills (exist in more than one tree) -| Topic | Copilot Skill | Gemini Skill | Notes | -|-------|--------------|--------------|-------| -| Testing | `testing-workflow` | `testing-workflow` | Copilot version covers `testFailure` tool and PYTHONPATH; Gemini version emphasises full suite preference and container detection | -| Settings & config | `settings-management` | `settings` | Gemini version is more comprehensive (22-point guide + PR checklist); Copilot version covers `ccd()` and `get_setting_value()` usage | -| MCP activation | `mcp-activation` | `mcp-activation` | Copilot version covers VS Code window reload; Gemini version covers Gemini CLI session restart | -| Project navigation | `project-navigation` | `project-navigation` | Copilot version has full path tables and env vars; Gemini version is a brief reference | -| Plugin dev | `plugin-run-development` | `plugin-development` | Both cover data contract, phases, formats, the `RUN_TIMEOUT` kill-timer gotcha, and a pre-PR pointer to the Conventions Checklist in `docs/PLUGINS_DEV.md`; kept in sync manually | -| Devcontainer | `devcontainer-services` + `devcontainer-setup` + `devcontainer-configs` | `devcontainer-management` | Copilot splits into 3 focused skills; Gemini combines into one (uses `docker exec`) | -| PR review | `pr-analysis` | `pr-analysis` | How to classify and respond to PR comments; pre-flight skill loading checklist | -| Logging | `logging-standards` | `logging-standards` | `mylog` levels, message format, what not to log | +| Topic | Copilot Skill | Gemini Skill | Claude Skill | Notes | +|-------|--------------|--------------|--------------|-------| +| Testing | `testing-workflow` | `testing-workflow` | `testing-workflow` | All three cover the full-suite-by-default rule, PYTHONPATH, auth/token retrieval, and the `sys.modules` stubbing pitfall | +| Settings & config | `settings-management` | `settings` | — | Gemini version is more comprehensive (22-point guide + PR checklist); Copilot version covers `ccd()` and `get_setting_value()` usage | +| MCP activation | `mcp-activation` | `mcp-activation` | — | Copilot version covers VS Code window reload; Gemini version covers Gemini CLI session restart | +| Project navigation | `project-navigation` | `project-navigation` | — | Copilot version has full path tables and env vars; Gemini version is a brief reference | +| Plugin dev | `plugin-run-development` | `plugin-development` | `plugin-development` | All three cover data contract, phases, formats, the `RUN_TIMEOUT` kill-timer gotcha (`timeoutMultiplier`/`per_item_timeout()`), and a pre-PR pointer to the Conventions Checklist in `docs/PLUGINS_DEV.md` | +| Devcontainer | `devcontainer-services` + `devcontainer-setup` + `devcontainer-configs` | `devcontainer-management` | — | Copilot splits into 3 focused skills; Gemini combines into one (uses `docker exec`) | +| PR review | `pr-analysis` | `pr-analysis` | `pr-analysis` | How to classify and respond to PR comments; pre-flight skill loading checklist | +| Logging | `logging-standards` | `logging-standards` | — | `mylog` levels, message format, what not to log | --- @@ -64,3 +65,5 @@ When adding a skill, create it in **both** directories to keep both AI systems c - `.gemini/skills//SKILL.md` — auto-discovered by Gemini CLI via YAML frontmatter Keep the body content identical between both files. Only the frontmatter `name`/`description` may differ slightly to match each system's discovery heuristics. + +If the skill is high-value enough to also mirror to Claude Code, add `.claude/skills//SKILL.md` too, and add the group to `GROUPS` in `scripts/check_skill_pairs.py` so drift gets flagged. Claude Code has no `activate_skill()`/`testFailure`/`runTests`/`report_progress` equivalents - adapt any such tool references to plain `Bash` commands instead of copying them verbatim. diff --git a/.gitignore b/.gitignore index eb332d93..9cb71482 100755 --- a/.gitignore +++ b/.gitignore @@ -31,10 +31,12 @@ front/api/* **/%40eaDir/ **/@eaDir/ .claude/settings.local.json +.claude/scheduled_tasks.lock __pycache__/ *.py[cod] *$py.class +.pytest_cache/ **/last_result.log **/script.log diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..ee3c3d72 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,91 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Project + +NetAlertX is a network visibility / asset-intelligence platform: continuous device discovery, presence/intruder detection, IPAM drift tracking, notifications, and multi-site sync, aimed at homelabs, MSPs, and NOCs. Backend is Python, frontend is PHP/JS served by Nginx, data lives in SQLite plus flat config files. + +## Commands + +Almost everything must run **inside the devcontainer** (Docker) — the host machine lacks the runtime environment (DB, `/data/config`, dependencies). Check with `ls -d /workspaces/NetAlertX`; if absent, you're on the host. + +```bash +# Full test suite (default — comprehensive coverage over speed, don't optimize for time unless asked) +cd /workspaces/NetAlertX; pytest test/ + +# One file or directory +pytest test/plugins/test_adguard_export.py +pytest test/plugins/ + +# Fast/unit-only (only when explicitly asked for "fast"/"quick" tests) +pytest test/ -m 'not docker and not feature_complete' + +# Reset the environment / pick up code changes / get a fresh API_TOKEN +bash /workspaces/NetAlertX/.devcontainer/scripts/setup.sh +sleep 5 +python3 -c "from helper import get_setting_value; print(get_setting_value('API_TOKEN'))" + +# Lint (matches CI in .github/workflows/code-checks.yml) +flake8 . --max-line-length=180 --ignore=E221,E222,E251,E203 + +# Full CI-equivalent run (regenerates devcontainer Dockerfile, rebuilds, runs everything) +./scripts/run_tests_in_docker_environment.sh +``` + +Rebuild the test image (`docker buildx build -t netalertx-test .`) only if the Dockerfile or dependencies changed — otherwise skip it, it's slow. + +Outside the container, most plugin unit tests (`test/plugins/test_*.py`) still run standalone — they stub NetAlertX modules into `sys.modules` before importing the plugin script. See the stubbing pitfall below before adding one. + +## Architecture + +### Backend layout + +- `server/__main__.py` — entry point. `server/plugin.py` — plugin runner/scheduler. `server/api_server/` — Flask + GraphQL API. +- `server/const.py` / `server/config_paths.py` — resolve the three runtime path roots. `server/conf.py` — process-wide config variables (a deliberate workaround for cross-module globals). +- `server/db/` — the only layer allowed to touch SQLite directly (`db_helper.py`). `server/models/` — domain handlers on top of it (e.g. `DeviceInstance` in `models/device_instance.py`). Never query the DB from elsewhere — go through a model or `db_helper.py`. +- `server/scan/`, `server/messaging/`, `server/workflows/`, `server/utils/` — scanning pipeline, notification dispatch, the workflow-automation engine, and shared utilities (`utils/datetime_utils.py`'s `timeNowUTC()` is the *only* place `datetime.now()` should be called — everything is stored in UTC). + +### Frontend + +`front/` is PHP + vanilla JS served by Nginx — no build step, no bundler, no `package.json`. Pages are top-level `.php` files; shared logic under `front/php/`. + +### Data & path conventions + +Three distinct roots, each with a different persistence contract — get this wrong and data silently disappears on restart: +- `dbFolderPath` (`/data/db`) — durable. Plugin-internal state (caches, "what did I already do" trackers) belongs here. +- `configPath` (`/data/config`) — durable, user-facing. `app.conf` lives here; config-like plugin artifacts (exports, backups) belong here too. +- `logPath` (`/tmp/log`, plus `/tmp/api`, `/tmp/db_is_locked`, nginx state) — **ephemeral tmpfs**, wiped on every container restart. Never put anything here you need to survive a restart. (`server/plugins/adguard_export`, `unifi_import` were both fixed this way after shipping with state files rooted in `logPath` — check any plugin that opens a file outside its `RESULT_FILE` against this before assuming it's fine.) + +All three are exported from `server/const.py` (`dbFolderPath`, `configPath`, `dataPath`, `logPath`) and importable by any plugin. + +### Plugin system (`server/plugins/*/`) + +Every plugin is a folder with `config.json` (manifest: settings, data contract, DB column mapping), an optional `script.py`-equivalent, and a `README.md`. Start from `server/plugins/__template/`. Full reference: `docs/PLUGINS_DEV.md` (its "Conventions Checklist" section is CI-enforced — see below). + +Non-obvious things that have caused real, shipped bugs in this codebase: + +- **`RUN_TIMEOUT` is the whole subprocess's kill-timeout, enforced by `server/plugin.py`, not a safe per-call HTTP/subprocess timeout.** A plugin that loops over N things and reuses `RUN_TIMEOUT` as each individual call's timeout can have one slow call burn the whole budget and get SIGKILLed before it writes its result file — silently losing the entire run. Two correct answers depending on the loop shape: + - Looping over a **config-declared, known-length list** (e.g. a subnets setting) → mark that `params[]` entry `"timeoutMultiplier": true` in `config.json`; the framework scales the *outer* kill-timeout by the list length. See `arp_scan/config.json`. + - Looping over a **runtime-variable-length collection** (e.g. a notification queue) → `plugin_helper.per_item_timeout(run_timeout, item_count)` divides the *inner* per-call budget instead. See `server/plugins/_publisher_ntfy/ntfy.py`. + - `test/plugins/test_plugin_conventions.py` mechanically checks for the unguarded reuse pattern (AST-based, including the case where the loop calls a helper function that does the risky call) — run it after touching any plugin that makes network/subprocess calls in a loop. +- **`plugin_helper.Plugin_Object`**: `helpVal1-4` and `watchedValue1-4` both preserve a real `0`/`False` you pass explicitly (checked via `is not None`) — only an actually-omitted (`None`) value defaults to `""`. Don't reintroduce a bare `x or ""` coercion here; it silently discards legitimate falsy values (this was a real, if narrowly-triggered, bug). +- A plugin's hardcoded Python fallback (`get_setting_value("X") or `) must match that setting's `config.json` `default_value` — `test_plugin_conventions.py` checks this too. `RUN` should default to `"disabled"` for every non-core plugin; description strings render directly in the Settings UI and should stay short (README is for implementation detail). +- Plugin unit tests that stub NetAlertX modules into `sys.modules` (so a script imports standalone outside the container) **must pop every stubbed name back out immediately after the one-time import** — otherwise the fake module leaks and shadows the real one for every other test file collected in the same pytest session, regardless of file/alphabetical order. See `test/plugins/test_ntfy_custom_headers.py` for the pattern, or `docs/PLUGINS_DEV.md` / the `testing-workflow` skill for the full writeup. + +### Data contract (plugin → DB) + +Plugins write pipe-delimited rows to `RESULT_FILE` via `plugin_helper.Plugin_Objects`/`Plugin_Object` — 9 required columns, 4 optional `helpVal*` ones. Full column spec and validation rules: `docs/PLUGINS_DEV_DATA_CONTRACT.md`. + +### Skills + +Procedural/how-to knowledge (running tests, resetting the DB, devcontainer management, PR analysis, etc.) lives as paired files in `.gemini/skills//` and `.github/skills//` (see `.gemini/skills/skills-index/SKILL.md` for the pairing map) — Claude Code should treat both as equally authoritative sources for the same procedures. The pairing convention is "keep body content identical"; a CI job (`check-skill-pairs` in `.github/workflows/code-checks.yml`) flags PRs that edit one side of a pair without the other, but it only checks *presence*, not content — if you edit one side, check whether the other needs the same update. + +## Code conventions + +- DB columns are camelCase, never snake_case (`deviceInstanceId`, not `device_instance_id`). +- Every `subprocess` call needs an explicit timeout; a nested subprocess call needs its own — an outer timeout doesn't propagate. +- Always run MACs through `normalize_mac()` (`plugin_helper.py`) before writing to DB; MAC literals in tests must be lowercase. +- No inline imports — everything at module top level. +- Reuse `test/db_test_helpers.py` for DB mocks/fixtures in tests rather than redefining `DummyDB`/`make_db` locally. +- Keep files under ~500 lines; split rather than grow. diff --git a/scripts/check_skill_pairs.py b/scripts/check_skill_pairs.py index fa39e122..ae58234c 100644 --- a/scripts/check_skill_pairs.py +++ b/scripts/check_skill_pairs.py @@ -1,15 +1,18 @@ #!/usr/bin/env python3 """ -Flag PRs that touch one half of a paired .gemini/.github skill file without -touching the other. `.gemini/skills/skills-index/SKILL.md` documents these -pairs and says to "keep body content identical between both files" - but -nothing previously enforced that, and the plugin-development pair had -already drifted apart before this check existed. +Flag PRs that touch some but not all files in a group of mirrored skill +files (`.gemini/skills/`, `.github/skills/`, `.claude/skills/`) without +touching the others. `.gemini/skills/skills-index/SKILL.md` documents these +groups and says to "keep body content identical" across them - but nothing +previously enforced that, and the plugin-development pair had already +drifted apart before this check existed. -This can't verify the two files still say the *same thing* (that needs -judgment - some pairs are intentionally different in depth), only that a -change to one side didn't forget the other exists. Exit non-zero (but the -CI step calling this is non-blocking) when a pair looks one-sided. +This can't verify the files still say the *same thing* (that needs +judgment - some groups are intentionally different in depth, and the three +devcontainer-management targets each cover only part of the Gemini file), +only that a change to one file didn't forget the others exist. Exit +non-zero (but the CI step calling this is non-blocking) when a group looks +one-sided. python3 scripts/check_skill_pairs.py origin/main """ @@ -18,18 +21,19 @@ import subprocess import sys # Kept in sync with the tables in .gemini/skills/skills-index/SKILL.md and -# .github/skills/skills-overview/SKILL.md. -PAIRS = [ - (".gemini/skills/plugin-development/plugin-skill.md", ".github/skills/plugin-run-development/SKILL.md"), - (".gemini/skills/testing-workflow/SKILL.md", ".github/skills/testing-workflow/SKILL.md"), - (".gemini/skills/settings/SKILL.md", ".github/skills/settings-management/SKILL.md"), - (".gemini/skills/mcp-activation/SKILL.md", ".github/skills/mcp-activation/SKILL.md"), - (".gemini/skills/project-navigation/SKILL.md", ".github/skills/project-navigation/SKILL.md"), - (".gemini/skills/pr-analysis/SKILL.md", ".github/skills/pr-analysis/SKILL.md"), - (".gemini/skills/logging-standards/SKILL.md", ".github/skills/logging-standards/SKILL.md"), - (".gemini/skills/devcontainer-management/SKILL.md", ".github/skills/devcontainer-services/SKILL.md"), - (".gemini/skills/devcontainer-management/SKILL.md", ".github/skills/devcontainer-setup/SKILL.md"), - (".gemini/skills/devcontainer-management/SKILL.md", ".github/skills/devcontainer-configs/SKILL.md"), +# .github/skills/skills-overview/SKILL.md. Most groups are 2 files (Gemini + +# Copilot); a few skills are also mirrored to .claude/skills/ as a 3rd member. +GROUPS = [ + [".gemini/skills/plugin-development/plugin-skill.md", ".github/skills/plugin-run-development/SKILL.md", ".claude/skills/plugin-development/SKILL.md"], + [".gemini/skills/testing-workflow/SKILL.md", ".github/skills/testing-workflow/SKILL.md", ".claude/skills/testing-workflow/SKILL.md"], + [".gemini/skills/pr-analysis/SKILL.md", ".github/skills/pr-analysis/SKILL.md", ".claude/skills/pr-analysis/SKILL.md"], + [".gemini/skills/settings/SKILL.md", ".github/skills/settings-management/SKILL.md"], + [".gemini/skills/mcp-activation/SKILL.md", ".github/skills/mcp-activation/SKILL.md"], + [".gemini/skills/project-navigation/SKILL.md", ".github/skills/project-navigation/SKILL.md"], + [".gemini/skills/logging-standards/SKILL.md", ".github/skills/logging-standards/SKILL.md"], + [".gemini/skills/devcontainer-management/SKILL.md", ".github/skills/devcontainer-services/SKILL.md"], + [".gemini/skills/devcontainer-management/SKILL.md", ".github/skills/devcontainer-setup/SKILL.md"], + [".gemini/skills/devcontainer-management/SKILL.md", ".github/skills/devcontainer-configs/SKILL.md"], ] @@ -48,21 +52,22 @@ def main(): changed = changed_files(sys.argv[1]) problems = [] - for gemini_path, github_path in PAIRS: - gemini_changed = gemini_path in changed - github_changed = github_path in changed - if gemini_changed != github_changed: - touched, untouched = (gemini_path, github_path) if gemini_changed else (github_path, gemini_path) - problems.append(f"- {touched} changed but its pair {untouched} wasn't.") + for group in GROUPS: + touched = [path for path in group if path in changed] + untouched = [path for path in group if path not in changed] + if touched and untouched: + problems.append( + f"- touched {', '.join(touched)} but not {', '.join(untouched)}." + ) if problems: - print("Possible skill-pair drift (only one side of a pair was touched):") + print("Possible skill-group drift (only some mirrored files were touched):") print("\n".join(problems)) - print("\nIf the change is Gemini/Copilot-specific on purpose, ignore this. " - "Otherwise update both sides - see .gemini/skills/skills-index/SKILL.md.") + print("\nIf the change is genuinely tool-specific, ignore this. " + "Otherwise update the other file(s) too - see .gemini/skills/skills-index/SKILL.md.") return 1 - print("No skill-pair drift detected.") + print("No skill-group drift detected.") return 0