@@ -0,0 +1,24 @@
+---
+name: git-workflow
+description: Read before running any git command that changes branch state (checkout -b, branch, push) in this repo. The default workflow is commit-and-push directly to next_release, not a feature-branch/PR flow - never create a branch without asking first.
+---
+
+# Git Workflow
+
+## Never create a branch without asking first
+
+Don't run `git checkout -b <new-branch>`, `git branch <new-branch>`, or anything else that creates a new branch, unless the user has explicitly said to. This applies even when a task sounds like it implies a branch-and-PR flow (e.g. "prep a PR") — ask first rather than assuming that's the intended workflow here.
+
+**Why:** this repo's working tree is not necessarily an isolated checkout. The user may have their own terminal open on the exact same repo (e.g. a NAS/server shell alongside this session's working directory) at the same time. Switching branches changes shared repository state — a `git checkout -b` run from one place silently changes what `git push`/`git status` does from every other place touching the same repo, which has caused real, confusing failures (a `git push` from the user's own terminal failing with "no upstream branch" because this assistant had switched branches without saying so).
+
+## Default: commit and push directly to `next_release`
+
+Absent other instructions, work lands on `next_release` directly — commit there, `git push` targets `origin next_release`. Don't invent a feature-branch/PR workflow unless asked for one.
+
+## Confirm before every push
+
+Ask for explicit confirmation immediately before running `git push`, even to the default `next_release` target. Don't fold a push into a larger task silently — surface it as its own step and wait for a go-ahead.
+
+## Before any git command that changes shared state
+
+Run `git status` and `git branch --show-current` first, and don't assume the branch you last left the repo on is still checked out — another process/terminal may have changed it.
@@ -0,0 +1,28 @@
+---
+name: install-scripts
+description: Read before editing anything under install/ (production-filesystem/, ubuntu24/, etc.) or reviewing a PR that touches it. Covers uninstall safety, why check-*.sh scripts don't all exit the same way, and what's legacy vs. active.
+---
+
+# Install Scripts
+
+Conventions mined from real review comments on `install/*` PRs (#1214, #1230, #1235) — not derived from reading the scripts alone, so check current behavior before relying on any of these if the surrounding code has since changed.
+
+## Uninstall safety
+
+`install/*/uninstall.sh` must only remove NetAlertX's own files (`/app` and its own data) — never touch system packages it depends on (PHP, nginx, avahi, etc.) or other shared system components. A hardware/bare-metal install shares the host with other software; removing a system package on uninstall can break things NetAlertX didn't install.
+
+## `check-*.sh` scripts don't all exit the same way — by design
+
+`install/production-filesystem/services/scripts/check-*.sh` scripts are **not uniform** in whether a failure is fatal:
+- Some are fail-fast on purpose: `check-first-run-config.sh` exits non-zero on a critical failure (can't create the config dir, can't copy default config) so the container doesn't start in a broken state.
+- Some are warning-only on purpose: `check-ramdisk.sh` exits 0 even when it detects a real issue, letting the app start anyway with suboptimal config.
+
+Don't assume one check's exit-code convention applies to another — check what that specific script is actually guarding before suggesting a change to its exit behavior.
+
+## `/back` is legacy, not the active code path
+
+`back/cron_script.sh` (and the rest of `/back`) is legacy, kept only for compatibility with older/external components. The active, maintained version is `install/production-filesystem/services/scripts/cron_script.sh`. Changes to `/back` are out of scope for most PRs unless the PR is specifically about compatibility with whatever still depends on it.
+
+## Build-time vs. runtime, and generated files
+
+See the `devcontainer-configs`/`devcontainer-management` skills for the Dockerfile-generation and build-vs-runtime-file-availability rules — the same "source files aren't present at Docker build time" constraint applies to `install/production-filesystem/build/*` scripts, not just the devcontainer image.
@@ -5,6 +5,14 @@ description: How to analyze and respond to GitHub PR review comments in NetAlert
 
 # PR Analysis
 
+## Standing Rule: A Repeated Comment Becomes a Skill Update
+
+If a review comment corrects something a skill *should* already cover, don't just fix that one instance — update the relevant skill as part of addressing the comment, same PR, same turn. If no skill covers it yet, that's the signal to create one. Check the skill first, though — sometimes it already covers the point and just wasn't consulted; the fix is in *applying* it, not in the skill being incomplete.
+
+## Code Style
+
+- **Prefer explicit, readable logic over compact-but-opaque expressions.** When a built-in/clever expression (e.g. a `max()`-based one-liner) saves a line or two but a reader has to reverse-engineer *why* it produces the right answer, write it out as plain conditional logic instead.
+
 ## Before Writing Any Test Code — Non-Negotiable Checklist
 
 Run through this before creating or editing any file under `test/`:
 
@@ -0,0 +1,24 @@
+---
+name: git-workflow
+description: Read before running any git command that changes branch state (checkout -b, branch, push) in this repo. The default workflow is commit-and-push directly to next_release, not a feature-branch/PR flow - never create a branch without asking first.
+---
+
+# Git Workflow
+
+## Never create a branch without asking first
+
+Don't run `git checkout -b <new-branch>`, `git branch <new-branch>`, or anything else that creates a new branch, unless the user has explicitly said to. This applies even when a task sounds like it implies a branch-and-PR flow (e.g. "prep a PR") — ask first rather than assuming that's the intended workflow here.
+
+**Why:** this repo's working tree is not necessarily an isolated checkout. The user may have their own terminal open on the exact same repo (e.g. a NAS/server shell alongside this session's working directory) at the same time. Switching branches changes shared repository state — a `git checkout -b` run from one place silently changes what `git push`/`git status` does from every other place touching the same repo, which has caused real, confusing failures (a `git push` from the user's own terminal failing with "no upstream branch" because this assistant had switched branches without saying so).
+
+## Default: commit and push directly to `next_release`
+
+Absent other instructions, work lands on `next_release` directly — commit there, `git push` targets `origin next_release`. Don't invent a feature-branch/PR workflow unless asked for one.
+
+## Confirm before every push
+
+Ask for explicit confirmation immediately before running `git push`, even to the default `next_release` target. Don't fold a push into a larger task silently — surface it as its own step and wait for a go-ahead.
+
+## Before any git command that changes shared state
+
+Run `git status` and `git branch --show-current` first, and don't assume the branch you last left the repo on is still checked out — another process/terminal may have changed it.
@@ -0,0 +1,28 @@
+---
+name: install-scripts
+description: Read before editing anything under install/ (production-filesystem/, ubuntu24/, etc.) or reviewing a PR that touches it. Covers uninstall safety, why check-*.sh scripts don't all exit the same way, and what's legacy vs. active.
+---
+
+# Install Scripts
+
+Conventions mined from real review comments on `install/*` PRs (#1214, #1230, #1235) — not derived from reading the scripts alone, so check current behavior before relying on any of these if the surrounding code has since changed.
+
+## Uninstall safety
+
+`install/*/uninstall.sh` must only remove NetAlertX's own files (`/app` and its own data) — never touch system packages it depends on (PHP, nginx, avahi, etc.) or other shared system components. A hardware/bare-metal install shares the host with other software; removing a system package on uninstall can break things NetAlertX didn't install.
+
+## `check-*.sh` scripts don't all exit the same way — by design
+
+`install/production-filesystem/services/scripts/check-*.sh` scripts are **not uniform** in whether a failure is fatal:
+- Some are fail-fast on purpose: `check-first-run-config.sh` exits non-zero on a critical failure (can't create the config dir, can't copy default config) so the container doesn't start in a broken state.
+- Some are warning-only on purpose: `check-ramdisk.sh` exits 0 even when it detects a real issue, letting the app start anyway with suboptimal config.
+
+Don't assume one check's exit-code convention applies to another — check what that specific script is actually guarding before suggesting a change to its exit behavior.
+
+## `/back` is legacy, not the active code path
+
+`back/cron_script.sh` (and the rest of `/back`) is legacy, kept only for compatibility with older/external components. The active, maintained version is `install/production-filesystem/services/scripts/cron_script.sh`. Changes to `/back` are out of scope for most PRs unless the PR is specifically about compatibility with whatever still depends on it.
+
+## Build-time vs. runtime, and generated files
+
+See the `devcontainer-configs`/`devcontainer-management` skills for the Dockerfile-generation and build-vs-runtime-file-availability rules — the same "source files aren't present at Docker build time" constraint applies to `install/production-filesystem/build/*` scripts, not just the devcontainer image.
@@ -5,6 +5,14 @@ description: How to analyze and respond to GitHub PR review comments in NetAlert
 
 # PR Analysis
 
+## Standing Rule: A Repeated Comment Becomes a Skill Update
+
+If a review comment corrects something a skill *should* already cover, don't just fix that one instance — update the relevant skill as part of addressing the comment, same PR, same turn. If no skill covers it yet, that's the signal to create one. Check the skill first, though — sometimes it already covers the point and just wasn't consulted; the fix is in *applying* it, not in the skill being incomplete.
+
+## Code Style
+
+- **Prefer explicit, readable logic over compact-but-opaque expressions.** When a built-in/clever expression (e.g. a `max()`-based one-liner) saves a line or two but a reader has to reverse-engineer *why* it produces the right answer, write it out as plain conditional logic instead.
+
 ## Before Writing Any Test Code — Non-Negotiable Checklist
 
 Run through this before creating or editing any file under `test/`:
 
@@ -25,8 +25,9 @@ Skills with the same purpose exist in more than one, sometimes under different n
 | 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` |
 | Plugin README docs | `plugin-readme` | `plugin-readme` | `plugin-readme` | All three cover README structure, the "don't re-document settings" rule, the `docs.netalertx.com` cross-linking convention, and common defects (template leftovers, copy-paste errors) found during a full-repo audit |
-| 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 |
+| Devcontainer | `devcontainer-management` | `devcontainer-services` + `devcontainer-setup` + `devcontainer-configs` | — | Gemini combines into one (uses `docker exec`); Copilot splits into 3 focused skills. `devcontainer-configs` also covers Dockerfile generation (root `Dockerfile` → `generate-dockerfile.sh` → `.devcontainer/Dockerfile`, source files not present at build time) - mined from real review comments on PR #1184/#1230. |
+| Install scripts | `install-scripts` | `install-scripts` | `install-scripts` | `install/*` conventions mined from real review comments (#1214/#1230/#1235): uninstall must only remove NetAlertX's own files, never system packages; `check-*.sh` scripts intentionally differ on fail-fast vs. warning-only; `/back` is legacy, not the active code path. |
+| PR review | `pr-analysis` | `pr-analysis` | `pr-analysis` | How to classify and respond to PR comments; pre-flight skill loading checklist; the standing rule that a repeated review comment becomes a skill update (sourced from jokob's own real PR comments, #1739/#1744); a readability-over-cleverness code-style note from the same source. |
 | Logging | `logging-standards` | `logging-standards` | — | `mylog` levels, message format, what not to log |
 | Scan pipeline internals | `scan-pipeline` | `scan-pipeline` | `scan-pipeline` | `process_scan()` call order and why it's load-bearing, `CurrentScan`/`Events`/`Sessions`/`DevicesView` relationships, how a session actually closes (no `close_session()` exists), and the `FIELD_SPECS` field-write authority mechanism. Complements `database-patterns` (Devices write-path/`*Source` attribution) rather than duplicating it. |
 | Database patterns | `database-patterns` | `database-patterns` | `database-patterns` | Devices table write-path inventory, the `FIELD_SOURCE_MAP`/`*Source` attribution system in `server/db/authoritative_handler.py`, SQLite trigger vs. Python-hook tradeoffs, and event-sourced vs. snapshot audit logging. |
 
@@ -26,3 +26,11 @@ Combines and merges template configurations into the final config used by VS Cod
 ## Note
 
 This affects only the devcontainer configuration. It has no bearing on the production or test Docker image.
+
+## Dockerfile Generation (separate from the config generation above)
+
+`.devcontainer/Dockerfile` is itself a **generated file** — `generate-dockerfile.sh` combines the root `Dockerfile` with `.devcontainer/resources/devcontainer-Dockerfile`. Never review or edit `.devcontainer/Dockerfile` directly; dependency/build-step changes belong in the root `Dockerfile`, regenerate after.
+
+Two build-time gotchas that have caused real review confusion:
+- **Source repo files are not available during the Docker build phase** — they're mounted into the container after it starts. A `COPY` referencing a source file will fail at build time; config that depends on repo content has to be handled at runtime (a setup script), not a build step.
+- Under modern BuildKit, `COPY` with a glob pattern that matches nothing (e.g. `.[V]ERSION`) succeeds silently — copies nothing, doesn't fail the build. Don't assume a missing optional file will surface as a build error.
@@ -0,0 +1,24 @@
+---
+name: netalertx-git-workflow
+description: Read before running any git command that changes branch state (checkout -b, branch, push) in this repo. The default workflow is commit-and-push directly to next_release, not a feature-branch/PR flow - never create a branch without asking first.
+---
+
+# Git Workflow
+
+## Never create a branch without asking first
+
+Don't run `git checkout -b <new-branch>`, `git branch <new-branch>`, or anything else that creates a new branch, unless the user has explicitly said to. This applies even when a task sounds like it implies a branch-and-PR flow (e.g. "prep a PR") — ask first rather than assuming that's the intended workflow here.
+
+**Why:** this repo's working tree is not necessarily an isolated checkout. The user may have their own terminal open on the exact same repo (e.g. a NAS/server shell alongside this session's working directory) at the same time. Switching branches changes shared repository state — a `git checkout -b` run from one place silently changes what `git push`/`git status` does from every other place touching the same repo, which has caused real, confusing failures (a `git push` from the user's own terminal failing with "no upstream branch" because this assistant had switched branches without saying so).
+
+## Default: commit and push directly to `next_release`
+
+Absent other instructions, work lands on `next_release` directly — commit there, `git push` targets `origin next_release`. Don't invent a feature-branch/PR workflow unless asked for one.
+
+## Confirm before every push
+
+Ask for explicit confirmation immediately before running `git push`, even to the default `next_release` target. Don't fold a push into a larger task silently — surface it as its own step and wait for a go-ahead.
+
+## Before any git command that changes shared state
+
+Run `git status` and `git branch --show-current` first, and don't assume the branch you last left the repo on is still checked out — another process/terminal may have changed it.
@@ -0,0 +1,28 @@
+---
+name: netalertx-install-scripts
+description: Read before editing anything under install/ (production-filesystem/, ubuntu24/, etc.) or reviewing a PR that touches it. Covers uninstall safety, why check-*.sh scripts don't all exit the same way, and what's legacy vs. active.
+---
+
+# Install Scripts
+
+Conventions mined from real review comments on `install/*` PRs (#1214, #1230, #1235) — not derived from reading the scripts alone, so check current behavior before relying on any of these if the surrounding code has since changed.
+
+## Uninstall safety
+
+`install/*/uninstall.sh` must only remove NetAlertX's own files (`/app` and its own data) — never touch system packages it depends on (PHP, nginx, avahi, etc.) or other shared system components. A hardware/bare-metal install shares the host with other software; removing a system package on uninstall can break things NetAlertX didn't install.
+
+## `check-*.sh` scripts don't all exit the same way — by design
+
+`install/production-filesystem/services/scripts/check-*.sh` scripts are **not uniform** in whether a failure is fatal:
+- Some are fail-fast on purpose: `check-first-run-config.sh` exits non-zero on a critical failure (can't create the config dir, can't copy default config) so the container doesn't start in a broken state.
+- Some are warning-only on purpose: `check-ramdisk.sh` exits 0 even when it detects a real issue, letting the app start anyway with suboptimal config.
+
+Don't assume one check's exit-code convention applies to another — check what that specific script is actually guarding before suggesting a change to its exit behavior.
+
+## `/back` is legacy, not the active code path
+
+`back/cron_script.sh` (and the rest of `/back`) is legacy, kept only for compatibility with older/external components. The active, maintained version is `install/production-filesystem/services/scripts/cron_script.sh`. Changes to `/back` are out of scope for most PRs unless the PR is specifically about compatibility with whatever still depends on it.
+
+## Build-time vs. runtime, and generated files
+
+See the `devcontainer-configs`/`devcontainer-management` skills for the Dockerfile-generation and build-vs-runtime-file-availability rules — the same "source files aren't present at Docker build time" constraint applies to `install/production-filesystem/build/*` scripts, not just the devcontainer image.