Skip to content

Conversation

@adamoutler
Copy link
Contributor

@adamoutler adamoutler commented Jan 25, 2026

This PR introduces modular agent skills for Gemini and Copilot while streamlining context and documentation.

Skills Docs

  • Implemented modular Agent Skills for Gemini to replace the monolithic GEMINI.md.
  • Added comprehensive GitHub Copilot skills to improve IDE-based agent performance.
  • Reduced the initial context load of copilot-instructions.md by offloading logic to skills.
  • Updated project documentation structure to support dynamic skill discovery.

Environmental variable Docs

  • Added dedicated documentation for Environmental Variables.

Environmental Variable Unit Test

  • Included a new unit test to verify environmental variable handling.

Agent Skills act like specialized cheat sheets for the AI. Instead of forcing a small model to memorize a massive manual, it only looks up the specific instructions it needs for the current task. This keeps the AI from getting confused by too much information, allowing it to work faster and give better answers.

Summary by CodeRabbit

  • Documentation

    • Added extensive developer and operational guides (devcontainer, testing, API, auth, services, plugins, DB reset, build/prune, sample data, settings, code standards); removed an older CLI agent instruction document.
    • Updated API docs with CORS_ORIGINS details and Portainer startup options.
  • Chores

    • Enabled VS Code AI agent skills integration and introduced startup skip flags (SKIP_TESTS, SKIP_STARTUP_CHECKS).
  • Tests

    • Added a test covering startup behavior when skipping specified checks.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 25, 2026

📝 Walkthrough

Walkthrough

This PR removes legacy Gemini CLI docs, adds numerous SKILL documentation files for devcontainer, testing, API, auth, and operational workflows, updates docs/config (CORS, Portainer, VSCode), and adds a test validating SKIP_STARTUP_CHECKS behavior. No runtime code changes beyond tests.

Changes

Cohort / File(s) Summary
Gemini docs removed
./.gemini/GEMINI.md
Deleted legacy Gemini-CLI Agent instructions and examples.
.gemini skill guides (new)
./.gemini/skills/devcontainer-management/SKILL.md, ./.gemini/skills/project-navigation/SKILL.md, ./.gemini/skills/testing-workflow/SKILL.md
Added devcontainer discovery/exec, project layout/navigation, and testing workflow guides for containerized development.
GitHub copilot & skills (new/updated)
./.github/copilot-instructions.md, ./.github/skills/...
Rewrote copilot instructions; added 14 new SKILL.md files under .github/skills/ covering API development, auth, code standards, DB reset, devcontainer setup/services/configs, docker build/prune, plugin dev, sample data, settings, and testing workflows.
Docs & config updates
docs/API.md, docs/DOCKER_PORTAINER.md, ./.vscode/settings.json
Documented CORS_ORIGINS format, added SKIP_TESTS and SKIP_STARTUP_CHECKS env var docs in Portainer guide, and enabled chat.useAgentSkills in VS Code settings.
Tests
test/docker_tests/test_entrypoint.py
Added test_skip_startup_checks_env_var() to assert entrypoint exits 0 and skips specified startup-check output when SKIP_STARTUP_CHECKS is set.

Possibly related PRs

Poem

🐰
I hopped through docs with nimble paws,
New SKILLs for builders, neat and laws.
Containers, tests, and API delight,
Gone the old GEMINI from my sight.
Hop on—dev paths now snug and bright!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'Agentic Skills & Environmental Vars docs' accurately and concisely describes the main changes: introduction of modular agent skills and environmental variable documentation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🤖 Fix all issues with AI agents
In @.gemini/skills/devcontainer-management/SKILL.md:
- Around line 21-26: The README sentence referring to "Use the scripts in
`/services/`" is misleading compared to the example path
`/workspaces/NetAlertX/.devcontainer/scripts/setup.sh`; update the text in
SKILL.md to either correct the directory name or mention both locations
explicitly (e.g., "Use the scripts in
`/workspaces/NetAlertX/.devcontainer/scripts/` (or `/services/` if present)"),
and adjust the example or wording so `/services/` is not presented as the sole
scripts directory unless it actually exists.

In @.github/copilot-instructions.md:
- Around line 25-43: Update the skills table to fix incorrect/missing skill
references: change the `plugin-development` entry to `plugin-run-development`,
and for `pr-analysis` and `logging-standards` either create matching SKILL.md
directories named exactly `pr-analysis` and `logging-standards` under
.github/skills/ or remove those two rows from the table if those skills are not
intended to exist; ensure the table entries exactly match the directory names so
the listed skills correspond to actual SKILL.md files.
- Around line 18-20: Update the repository README snippet in
.github/copilot-instructions.md to correct the source repo database path: change
the "Data (SQLite): db/app.db; helpers in server/db/*" entry to "Data (SQLite):
back/app.db; helpers in server/db/*" (and ensure any mention of /data/db/app.db
is kept only in deployment/runtime docs); locate and edit the list block
containing the Backend (Python), Data (SQLite), and Frontend entries to make
this single-line replacement.

In @.github/skills/api-development/SKILL.md:
- Around line 37-51: Add a documented exception to the Response Contract noting
that the legacy endpoint /device/<mac> GET does not follow the standard
{"success": true|false} contract; update the Response Contract section in
SKILL.md by appending a short "**Exception:** The legacy /device/<mac> GET
endpoint does not follow this contract to maintain backward compatibility with
the UI." so developers are aware not to refactor that endpoint to the standard
contract.

In @.github/skills/devcontainer-setup/SKILL.md:
- Around line 1-3: The skill name uses an em dash which can break discovery;
replace the em dash in the name value "netalertx-idempotent—setup" with a
standard hyphen so it reads "netalertx-idempotent-setup", ensuring all
references use the updated name (look for the name field in SKILL.md and any
code or configs that reference "netalertx-idempotent—setup").

In @.github/skills/plugin-run-development/SKILL.md:
- Around line 26-31: The fenced code block containing the plugin directory
listing (the block starting with "front/plugins/<code_name>/") is missing a
language tag and trips markdownlint rule MD040; update that opening fence to
include a language (e.g., use "text" so it becomes ```text) in the SKILL.md file
so the block is properly marked as plain text while preserving the directory
listing content.

In @.github/skills/project-navigation/SKILL.md:
- Around line 33-51: Update the Data/Path table in SKILL.md to show the
corresponding NETALERTX_* environment variables alongside the hardcoded runtime
paths (e.g., add a second column or an extra row per item) so readers see both
the default path and the env var that should be used (reference the table header
"Data | Path" and the entries for Config, Database, API JSON cache, Logs, Plugin
logs). Ensure the listed env vars NETALERTX_DB, NETALERTX_LOG, NETALERTX_CONFIG,
NETALERTX_DATA, and NETALERTX_APP are clearly paired with the appropriate path
entries and add a short note under "Environment Variables" directing users to
prefer those vars at runtime instead of hardcoding paths.

In @.github/skills/testing-workflow/SKILL.md:
- Around line 28-31: The PYTHONPATH note currently hardcodes the minor Python
version (`/opt/venv/lib/python3.12/site-packages`); update the text to avoid
pinning the minor version by replacing that entry with a generic pattern like
`/opt/venv/lib/pythonX.Y/site-packages` or remove the versioned path entirely
and reference `/opt/venv/lib/pythonX.Y/site-packages` (or an unversioned venv
path) so the documentation remains correct if the base image Python minor
version changes; ensure you update the line that lists PYTHONPATH entries where
`/opt/venv/lib/python3.12/site-packages` appears.

In `@test/docker_tests/test_entrypoint.py`:
- Around line 86-92: The test function test_skip_startup_checks_env_var is
missing the standard pytest markers used across this file; add the decorators
`@pytest.mark.docker` and `@pytest.mark.feature_complete` immediately above the
test_skip_startup_checks_env_var definition so it runs under the same test
selection as the other tests in this module and behaves consistently with the
test suite.
🧹 Nitpick comments (4)
.github/skills/devcontainer-services/SKILL.md (1)

1-4: Consider aligning skill name with folder name.

The skill name is restarting-netalertx-services but the containing folder is devcontainer-services. Consider using netalertx-devcontainer-services for consistency with other skills (e.g., netalertx-docker-prune, netalertx-devcontainer-configs).

Suggested fix
 ---
-name: restarting-netalertx-services
+name: netalertx-devcontainer-services
 description: Control NetAlertX services inside the devcontainer. Use this when asked to start backend, start frontend, start nginx, start php-fpm, start crond, stop services, restart services, or check if services are running.
 ---
test/docker_tests/test_entrypoint.py (1)

34-34: Consider increasing subprocess timeout.

The _run_entrypoint helper uses timeout=30, while coding guidelines recommend a minimum of 60 seconds for subprocess calls. For quick entrypoint checks this may be acceptable, but could cause flaky failures in slower CI environments.

Suggested adjustment
-    return subprocess.run(cmd, capture_output=True, text=True, timeout=30)
+    return subprocess.run(cmd, capture_output=True, text=True, timeout=60)

Based on coding guidelines: "Always set explicit timeouts for subprocess calls; minimum 60 seconds unless plugin config specifies otherwise."

.github/skills/code-standards/SKILL.md (1)

8-11: Clarify 500-line rule exemption for tests.

Repo guidance exempts test/ and test_*.py files from the 500-line limit; consider documenting that exception to prevent confusion. Based on learnings, ...

.github/skills/api-development/SKILL.md (1)

53-58: Consider clarifying "pipeline" reference.

Line 58 mentions "don't bypass pipeline" which could be clearer for new contributors. Consider briefly explaining what the pipeline refers to (e.g., data processing flow, caching layer, etc.).

📝 Optional clarification
 4. Update UI to read/write JSON cache (don't bypass pipeline)
+   - Ensure data flows through the standard processing pipeline rather than direct database access

Comment on lines +21 to +26
## Running Commands in the Container

Prefix commands with `docker exec <CONTAINER_ID>` to run them inside the environment. Use the scripts in `/services/` to control backend and other processes.

```bash
docker exec <CONTAINER_ID> bash /workspaces/NetAlertX/.devcontainer/scripts/setup.sh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Clarify the scripts location path.

This section says “Use the scripts in /services/” but the example uses /workspaces/NetAlertX/.devcontainer/scripts/setup.sh. If /services/ isn’t a real scripts directory, update the text to avoid misleading instructions.

🤖 Prompt for AI Agents
In @.gemini/skills/devcontainer-management/SKILL.md around lines 21 - 26, The
README sentence referring to "Use the scripts in `/services/`" is misleading
compared to the example path
`/workspaces/NetAlertX/.devcontainer/scripts/setup.sh`; update the text in
SKILL.md to either correct the directory name or mention both locations
explicitly (e.g., "Use the scripts in
`/workspaces/NetAlertX/.devcontainer/scripts/` (or `/services/` if present)"),
and adjust the example or wording so `/services/` is not presented as the sole
scripts directory unless it actually exists.

Comment on lines 18 to 20
- **Backend (Python):** `server/__main__.py`, `server/plugin.py`, `server/api_server/api_server_start.py`
- **Data (SQLite):** `db/app.db`; helpers in `server/db/*`
- **Frontend (Nginx + PHP + JS):** `front/`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Find SQLite database files and verify path references

echo "=== Searching for app.db files ==="
fd -t f 'app\.db$'

echo -e "\n=== Searching for database directories ==="
fd -t d '^db$|^database$'

echo -e "\n=== Checking for DB references in documentation ==="
rg -n "app\.db|/data/db" --type md

echo -e "\n=== Checking for DB references in source code ==="
rg -n "app\.db|/data/db" --type py --type sh

Repository: netalertx/NetAlertX

Length of output: 9136


Correct database file path in source repository structure.

Line 19 references the database as db/app.db, but the actual database file in the repository is located at back/app.db. The server/db/ directory contains helper code, not the database file itself. Update to: **Data (SQLite):** back/app.db; helpers in server/db/*``

(The runtime location /data/db/app.db is relevant only for deployed container documentation, not source architecture.)

🤖 Prompt for AI Agents
In @.github/copilot-instructions.md around lines 18 - 20, Update the repository
README snippet in .github/copilot-instructions.md to correct the source repo
database path: change the "Data (SQLite): db/app.db; helpers in server/db/*"
entry to "Data (SQLite): back/app.db; helpers in server/db/*" (and ensure any
mention of /data/db/app.db is kept only in deployment/runtime docs); locate and
edit the list block containing the Backend (Python), Data (SQLite), and Frontend
entries to make this single-line replacement.

Comment on lines 25 to 43
Procedural knowledge lives in `.github/skills/`. Load the appropriate skill when performing these tasks:

## API/Endpoints quick map
- Flask app: `server/api_server/api_server_start.py` exposes routes like `/device/<mac>`, `/devices`, `/devices/export/{csv,json}`, `/devices/import`, `/devices/totals`, `/devices/by-status`, plus `nettools`, `events`, `sessions`, `dbquery`, `metrics`, `sync`.
- Authorization: all routes expect header `Authorization: Bearer <API_TOKEN>` via `get_setting_value('API_TOKEN')`.
- All responses need to return `"success":<False:True>` and if `False` an "error" message needs to be returned, e.g. `{"success": False, "error": f"No stored open ports for Device"}`
| Task | Skill |
|------|-------|
| Run tests, check failures | `testing-workflow` |
| Start/stop/restart services | `devcontainer-services` |
| Wipe database, fresh start | `database-reset` |
| Load sample devices | `sample-data` |
| Build Docker images | `docker-build` |
| Reprovision devcontainer | `devcontainer-setup` |
| Create or run plugins | `plugin-development` |
| Analyze PR comments | `pr-analysis` |
| Clean Docker resources | `docker-prune` |
| Generate devcontainer configs | `devcontainer-configs` |
| Create API endpoints | `api-development` |
| Logging conventions | `logging-standards` |
| Settings and config | `settings-management` |
| Find files and paths | `project-navigation` |
| Coding standards | `code-standards` |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find .github -type d -name "skills" 2>/dev/null | head -20

Repository: netalertx/NetAlertX

Length of output: 77


🏁 Script executed:

ls -la .github/ 2>/dev/null || echo "Directory not found"

Repository: netalertx/NetAlertX

Length of output: 565


🏁 Script executed:

if [ -d ".github/skills" ]; then
  echo "=== Contents of .github/skills ==="
  find .github/skills -type f -o -type d | sort
else
  echo ".github/skills directory does not exist"
fi

Repository: netalertx/NetAlertX

Length of output: 1138


🏁 Script executed:

# Check if .gemini/skills exists as alternative
if [ -d ".gemini/skills" ]; then
  echo "=== Contents of .gemini/skills ==="
  find .gemini/skills -type f -o -type d | sort
else
  echo ".gemini/skills directory does not exist"
fi

Repository: netalertx/NetAlertX

Length of output: 349


🏁 Script executed:

# Search for any mention of skills in the copilot instructions file
cat -n .github/copilot-instructions.md 2>/dev/null | head -60

Repository: netalertx/NetAlertX

Length of output: 2501


Fix skill references: three skills in the table don't exist or have incorrect names.

The following skills referenced in the table (lines 25-43) are missing or incorrectly named in .github/skills/:

  • plugin-development → should be plugin-run-development (actual directory name)
  • pr-analysis → does not exist
  • logging-standards → does not exist

Update the table to either remove non-existent skills or create the corresponding SKILL.md files. If pr-analysis and logging-standards are planned skills, create the directories under .github/skills/; otherwise, remove those rows from the table.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~25-~25: The official name of this software platform is spelled with a capital “H”.
Context: ...# Skills Procedural knowledge lives in .github/skills/. Load the appropriate skill wh...

(GITHUB)

🤖 Prompt for AI Agents
In @.github/copilot-instructions.md around lines 25 - 43, Update the skills
table to fix incorrect/missing skill references: change the `plugin-development`
entry to `plugin-run-development`, and for `pr-analysis` and `logging-standards`
either create matching SKILL.md directories named exactly `pr-analysis` and
`logging-standards` under .github/skills/ or remove those two rows from the
table if those skills are not intended to exist; ensure the table entries
exactly match the directory names so the listed skills correspond to actual
SKILL.md files.

Comment on lines 33 to 51
| Data | Path |
|------|------|
| Config (runtime) | `/data/config/app.conf` |
| Config (default) | `back/app.conf` |
| Database | `/data/db/app.db` |
| API JSON cache | `/tmp/api/*.json` |
| Logs | `/tmp/log/` |
| Plugin logs | `/tmp/log/plugins/` |

## Environment Variables

Use these instead of hardcoding paths:

- `NETALERTX_DB`
- `NETALERTX_LOG`
- `NETALERTX_CONFIG`
- `NETALERTX_DATA`
- `NETALERTX_APP`

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Prefer env vars over hardcoded runtime paths in the table.
Consider listing NETALERTX_* variables alongside the /data/* paths to avoid encouraging hardcoded paths in docs. Based on learnings, ...

🤖 Prompt for AI Agents
In @.github/skills/project-navigation/SKILL.md around lines 33 - 51, Update the
Data/Path table in SKILL.md to show the corresponding NETALERTX_* environment
variables alongside the hardcoded runtime paths (e.g., add a second column or an
extra row per item) so readers see both the default path and the env var that
should be used (reference the table header "Data | Path" and the entries for
Config, Database, API JSON cache, Logs, Plugin logs). Ensure the listed env vars
NETALERTX_DB, NETALERTX_LOG, NETALERTX_CONFIG, NETALERTX_DATA, and NETALERTX_APP
are clearly paired with the appropriate path entries and add a short note under
"Environment Variables" directing users to prefer those vars at runtime instead
of hardcoding paths.

Comment on lines +86 to +92
def test_skip_startup_checks_env_var():
# If SKIP_STARTUP_CHECKS contains the human-readable name of a check (e.g. "mandatory folders"),
# the entrypoint should skip that specific check. We check that the "Creating NetAlertX log directory."
# message (from the mandatory folders check) is not printed when skipped.
result = _run_entrypoint(env={"SKIP_STARTUP_CHECKS": "mandatory folders"}, check_only=True)
assert "Creating NetAlertX log directory" not in result.stdout
assert result.returncode == 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing pytest markers.

The new test test_skip_startup_checks_env_var is missing the @pytest.mark.docker and @pytest.mark.feature_complete decorators that all other tests in this file have. This may cause the test to be skipped or run in unexpected contexts.

Proposed fix
+@pytest.mark.docker
+@pytest.mark.feature_complete
 def test_skip_startup_checks_env_var():
     # If SKIP_STARTUP_CHECKS contains the human-readable name of a check (e.g. "mandatory folders"),
     # the entrypoint should skip that specific check. We check that the "Creating NetAlertX log directory." 
     # message (from the mandatory folders check) is not printed when skipped.
     result = _run_entrypoint(env={"SKIP_STARTUP_CHECKS": "mandatory folders"}, check_only=True)
     assert "Creating NetAlertX log directory" not in result.stdout
     assert result.returncode == 0
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def test_skip_startup_checks_env_var():
# If SKIP_STARTUP_CHECKS contains the human-readable name of a check (e.g. "mandatory folders"),
# the entrypoint should skip that specific check. We check that the "Creating NetAlertX log directory."
# message (from the mandatory folders check) is not printed when skipped.
result = _run_entrypoint(env={"SKIP_STARTUP_CHECKS": "mandatory folders"}, check_only=True)
assert "Creating NetAlertX log directory" not in result.stdout
assert result.returncode == 0
`@pytest.mark.docker`
`@pytest.mark.feature_complete`
def test_skip_startup_checks_env_var():
# If SKIP_STARTUP_CHECKS contains the human-readable name of a check (e.g. "mandatory folders"),
# the entrypoint should skip that specific check. We check that the "Creating NetAlertX log directory."
# message (from the mandatory folders check) is not printed when skipped.
result = _run_entrypoint(env={"SKIP_STARTUP_CHECKS": "mandatory folders"}, check_only=True)
assert "Creating NetAlertX log directory" not in result.stdout
assert result.returncode == 0
🤖 Prompt for AI Agents
In `@test/docker_tests/test_entrypoint.py` around lines 86 - 92, The test function
test_skip_startup_checks_env_var is missing the standard pytest markers used
across this file; add the decorators `@pytest.mark.docker` and
`@pytest.mark.feature_complete` immediately above the
test_skip_startup_checks_env_var definition so it runs under the same test
selection as the other tests in this module and behaves consistently with the
test suite.

@jokob-sk jokob-sk merged commit 6cfc5ef into netalertx:main Jan 25, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants