mirror of
https://github.com/jokob-sk/NetAlertX.git
synced 2026-09-14 23:32:57 -04:00
19 files changed
+447
-18
No files matched your search
@@ -0,0 +1,34 @@
|
||||
---
|
||||
name: plugin-review
|
||||
description: Read when reviewing a plugin PR or auditing an existing plugin script (server/plugins/*/script.py or equivalent). Covers the one check not already mechanically enforced by test_plugin_conventions.py - plugin scripts embedding their own raw SQL instead of an existing/new model method - plus a worked real-PR example.
|
||||
---
|
||||
|
||||
# Plugin Review
|
||||
|
||||
## Scope
|
||||
|
||||
This is a reviewer-facing checklist, complementary to [[plugin-development]] (which is author-facing). For `config.json` conventions already covered there and mechanically checked by `test/plugins/test_plugin_conventions.py` — `RUN` default, `RUN_TIMEOUT` reuse in a loop, `dataType`/`default_value` agreement, description length — defer to that skill's "Before Opening a PR" checklist rather than re-deriving them here.
|
||||
|
||||
## The check this skill adds: no raw SQL in a plugin script
|
||||
|
||||
Plugin scripts write their results to `RESULT_FILE` via `plugin_helper.Plugin_Objects` — the framework inserts those rows into the DB. A plugin that also runs its own `SELECT`/`INSERT`/`UPDATE` (via `sqlite3` directly or `database.get_temp_db_connection()`) is bypassing that contract, usually to read existing data before deciding what to write.
|
||||
|
||||
**Default for a new/contributed plugin: use an existing model method (`server/models/*.py`), or add one, instead of a raw SQL string in the plugin.** Not GraphQL — GraphQL (`/graphql`) is the frontend-facing API; no plugin in this codebase reaches it, and doing so would mean an HTTP round-trip (with an API token) from a subprocess that already has direct `server/` import access. Every plugin that currently touches the DB imports `server/` modules directly, matching how the rest of the backend is layered (`CLAUDE.md`: "Never query the DB from elsewhere — go through a model or `db_helper.py`" — this is that same rule, just also applying to plugins).
|
||||
|
||||
**Known exception — 5 existing core/infrastructure plugins legitimately use `get_temp_db_connection()` with raw SQL:** `db_cleanup` (retention `DELETE`s + `REINDEX` — no model method fits a multi-table retention sweep), `heartbeat`, `vendor_update`, `csv_backup`, `sync`. These predate this rule and do maintenance/schema-level work (bulk retention, `PRAGMA table_info`, full-table export) that doesn't map to a single-row model method. Don't treat their existence as precedent for a *new* plugin's simple lookup query — check whether an existing method already covers the new plugin's actual need first (it usually does, or is a one-line addition).
|
||||
|
||||
## Review flow for a raw SQL query in a plugin
|
||||
|
||||
1. **Does an existing model method already do this?** Check the relevant `server/models/*_instance.py` file (`DeviceInstance`, `EventInstance`, `PluginObjectInstance`, etc.) before assuming one needs to be added.
|
||||
2. **If not, is it worth adding one** (`server/models/device_instance.py` etc.), or is this genuinely a one-off maintenance/schema query that belongs in the core-plugin exception list above?
|
||||
3. **Check the collation the query relies on** against the column's actual schema (`server/db/schema/app.sql`) rather than assuming — `devMac`/`eveMac`/`sesMac`/`scanMac`/`devParentMAC` are declared `COLLATE NOCASE` at the column level, so an explicit `COLLATE NOCASE` against one of them in a new query is redundant (harmless, but a sign the author didn't check). `devName` has **no** column-level collation — an explicit `COLLATE NOCASE` there is genuinely necessary if case-insensitive name matching is intended, not a mistake.
|
||||
4. **Parameterization** — `?` placeholders, never string-formatted values into the query (this part is usually already fine; flag it if not).
|
||||
|
||||
## Worked example: PR #1788 (DOCKERDISC plugin)
|
||||
|
||||
Two raw queries in `server/plugins/dockerdisc/script.py`:
|
||||
|
||||
- `lookup_device_mac()`: `SELECT 1 FROM Devices WHERE devMac = ? COLLATE NOCASE LIMIT 1` — an existence check. `DeviceInstance.getByMac(mac)` already does this exact lookup (`server/models/device_instance.py:102-105`); its `COLLATE NOCASE` is redundant since `devMac` already carries that collation at the column level. **Fix: `DeviceInstance().getByMac(mac) is not None`, delete the raw SQL.**
|
||||
- `resolve_host_mac()`: `SELECT devMac FROM Devices WHERE devName = ? COLLATE NOCASE` — a name lookup with real 0/1/many-match handling (falls back to a manually-configured MAC on ambiguity or no match). No existing method covers this. `devName` has no column-level collation, so the explicit `COLLATE NOCASE` here is correct, not redundant. **Fix: add `DeviceInstance.getAllByName(name)` returning every match** (not just one — the plugin's own ambiguity detection needs the full set), and have the plugin call that instead.
|
||||
|
||||
This is the shape of the fix in general: an existence/single-row check usually already has a model method; a query with plugin-specific result handling (ambiguity, filtering) usually needs a small new method added rather than a workaround in the plugin itself.
|
||||
@@ -0,0 +1,34 @@
|
||||
---
|
||||
name: plugin-review
|
||||
description: Read when reviewing a plugin PR or auditing an existing plugin script (server/plugins/*/script.py or equivalent). Covers the one check not already mechanically enforced by test_plugin_conventions.py - plugin scripts embedding their own raw SQL instead of an existing/new model method - plus a worked real-PR example.
|
||||
---
|
||||
|
||||
# Plugin Review
|
||||
|
||||
## Scope
|
||||
|
||||
This is a reviewer-facing checklist, complementary to [[plugin-development]] (which is author-facing). For `config.json` conventions already covered there and mechanically checked by `test/plugins/test_plugin_conventions.py` — `RUN` default, `RUN_TIMEOUT` reuse in a loop, `dataType`/`default_value` agreement, description length — defer to that skill's "Before Opening a PR" checklist rather than re-deriving them here.
|
||||
|
||||
## The check this skill adds: no raw SQL in a plugin script
|
||||
|
||||
Plugin scripts write their results to `RESULT_FILE` via `plugin_helper.Plugin_Objects` — the framework inserts those rows into the DB. A plugin that also runs its own `SELECT`/`INSERT`/`UPDATE` (via `sqlite3` directly or `database.get_temp_db_connection()`) is bypassing that contract, usually to read existing data before deciding what to write.
|
||||
|
||||
**Default for a new/contributed plugin: use an existing model method (`server/models/*.py`), or add one, instead of a raw SQL string in the plugin.** Not GraphQL — GraphQL (`/graphql`) is the frontend-facing API; no plugin in this codebase reaches it, and doing so would mean an HTTP round-trip (with an API token) from a subprocess that already has direct `server/` import access. Every plugin that currently touches the DB imports `server/` modules directly, matching how the rest of the backend is layered (`CLAUDE.md`: "Never query the DB from elsewhere — go through a model or `db_helper.py`" — this is that same rule, just also applying to plugins).
|
||||
|
||||
**Known exception — 5 existing core/infrastructure plugins legitimately use `get_temp_db_connection()` with raw SQL:** `db_cleanup` (retention `DELETE`s + `REINDEX` — no model method fits a multi-table retention sweep), `heartbeat`, `vendor_update`, `csv_backup`, `sync`. These predate this rule and do maintenance/schema-level work (bulk retention, `PRAGMA table_info`, full-table export) that doesn't map to a single-row model method. Don't treat their existence as precedent for a *new* plugin's simple lookup query — check whether an existing method already covers the new plugin's actual need first (it usually does, or is a one-line addition).
|
||||
|
||||
## Review flow for a raw SQL query in a plugin
|
||||
|
||||
1. **Does an existing model method already do this?** Check the relevant `server/models/*_instance.py` file (`DeviceInstance`, `EventInstance`, `PluginObjectInstance`, etc.) before assuming one needs to be added.
|
||||
2. **If not, is it worth adding one** (`server/models/device_instance.py` etc.), or is this genuinely a one-off maintenance/schema query that belongs in the core-plugin exception list above?
|
||||
3. **Check the collation the query relies on** against the column's actual schema (`server/db/schema/app.sql`) rather than assuming — `devMac`/`eveMac`/`sesMac`/`scanMac`/`devParentMAC` are declared `COLLATE NOCASE` at the column level, so an explicit `COLLATE NOCASE` against one of them in a new query is redundant (harmless, but a sign the author didn't check). `devName` has **no** column-level collation — an explicit `COLLATE NOCASE` there is genuinely necessary if case-insensitive name matching is intended, not a mistake.
|
||||
4. **Parameterization** — `?` placeholders, never string-formatted values into the query (this part is usually already fine; flag it if not).
|
||||
|
||||
## Worked example: PR #1788 (DOCKERDISC plugin)
|
||||
|
||||
Two raw queries in `server/plugins/dockerdisc/script.py`:
|
||||
|
||||
- `lookup_device_mac()`: `SELECT 1 FROM Devices WHERE devMac = ? COLLATE NOCASE LIMIT 1` — an existence check. `DeviceInstance.getByMac(mac)` already does this exact lookup (`server/models/device_instance.py:102-105`); its `COLLATE NOCASE` is redundant since `devMac` already carries that collation at the column level. **Fix: `DeviceInstance().getByMac(mac) is not None`, delete the raw SQL.**
|
||||
- `resolve_host_mac()`: `SELECT devMac FROM Devices WHERE devName = ? COLLATE NOCASE` — a name lookup with real 0/1/many-match handling (falls back to a manually-configured MAC on ambiguity or no match). No existing method covers this. `devName` has no column-level collation, so the explicit `COLLATE NOCASE` here is correct, not redundant. **Fix: add `DeviceInstance.getAllByName(name)` returning every match** (not just one — the plugin's own ambiguity detection needs the full set), and have the plugin call that instead.
|
||||
|
||||
This is the shape of the fix in general: an existence/single-row check usually already has a model method; a query with plugin-specific result handling (ambiguity, filtering) usually needs a small new method added rather than a workaround in the plugin itself.
|
||||
@@ -33,6 +33,7 @@ Skills with the same purpose exist in more than one, sometimes under different n
|
||||
| PRD writing | `prd-writing` | `prd-writing` | `prd-writing` | Methodology for writing a design doc: challenge the idea, verify every claim against actual code, trace every downstream consumer of a new mechanism, evaluate performance impact against the real schema/indexes, record rejected alternatives and open-issue decisions explicitly, final-check pass before done. |
|
||||
| UX/frontend design | `ux-design-patterns` | `ux-design-patterns` | `ux-design-patterns` | Don't invent new UX behavior/visual patterns unless a PRD calls for it - search `front/` for an existing pattern first and reuse it. Priority order for design tradeoffs when several options are reasonable: existing behavior > intuitiveness > information density > usability > utility > uniqueness > industry practices > generic UI. |
|
||||
| Skill hygiene | `skill-hygiene` | `skill-hygiene` | `skill-hygiene` | Read before writing/editing any SKILL.md, or any research/audit doc in `.gemini/internal-docs/research/`. Two standing rules: state current behavior only (no "Correction:", no "as of <date>", no "caught in review" narration - that trail belongs in PRDs), and prefer plain, short wording. Includes the grep sweep to run before calling a doc clean. |
|
||||
| Plugin review | `plugin-review` | `plugin-review` | `plugin-review` | Reviewer-facing complement to Plugin dev above: no raw SQL in a plugin script - use an existing/new `server/models/*.py` method, not GraphQL (which no plugin reaches). Named exception list for 5 pre-existing core/infra plugins with legitimate direct DB access, a collation-checking step, and a worked example from a real PR (#1788, DOCKERDISC). |
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
---
|
||||
name: netalertx-plugin-review
|
||||
description: Read when reviewing a plugin PR or auditing an existing plugin script (server/plugins/*/script.py or equivalent). Covers the one check not already mechanically enforced by test_plugin_conventions.py - plugin scripts embedding their own raw SQL instead of an existing/new model method - plus a worked real-PR example.
|
||||
---
|
||||
|
||||
# Plugin Review
|
||||
|
||||
## Scope
|
||||
|
||||
This is a reviewer-facing checklist, complementary to [[plugin-development]] (which is author-facing). For `config.json` conventions already covered there and mechanically checked by `test/plugins/test_plugin_conventions.py` — `RUN` default, `RUN_TIMEOUT` reuse in a loop, `dataType`/`default_value` agreement, description length — defer to that skill's "Before Opening a PR" checklist rather than re-deriving them here.
|
||||
|
||||
## The check this skill adds: no raw SQL in a plugin script
|
||||
|
||||
Plugin scripts write their results to `RESULT_FILE` via `plugin_helper.Plugin_Objects` — the framework inserts those rows into the DB. A plugin that also runs its own `SELECT`/`INSERT`/`UPDATE` (via `sqlite3` directly or `database.get_temp_db_connection()`) is bypassing that contract, usually to read existing data before deciding what to write.
|
||||
|
||||
**Default for a new/contributed plugin: use an existing model method (`server/models/*.py`), or add one, instead of a raw SQL string in the plugin.** Not GraphQL — GraphQL (`/graphql`) is the frontend-facing API; no plugin in this codebase reaches it, and doing so would mean an HTTP round-trip (with an API token) from a subprocess that already has direct `server/` import access. Every plugin that currently touches the DB imports `server/` modules directly, matching how the rest of the backend is layered (`CLAUDE.md`: "Never query the DB from elsewhere — go through a model or `db_helper.py`" — this is that same rule, just also applying to plugins).
|
||||
|
||||
**Known exception — 5 existing core/infrastructure plugins legitimately use `get_temp_db_connection()` with raw SQL:** `db_cleanup` (retention `DELETE`s + `REINDEX` — no model method fits a multi-table retention sweep), `heartbeat`, `vendor_update`, `csv_backup`, `sync`. These predate this rule and do maintenance/schema-level work (bulk retention, `PRAGMA table_info`, full-table export) that doesn't map to a single-row model method. Don't treat their existence as precedent for a *new* plugin's simple lookup query — check whether an existing method already covers the new plugin's actual need first (it usually does, or is a one-line addition).
|
||||
|
||||
## Review flow for a raw SQL query in a plugin
|
||||
|
||||
1. **Does an existing model method already do this?** Check the relevant `server/models/*_instance.py` file (`DeviceInstance`, `EventInstance`, `PluginObjectInstance`, etc.) before assuming one needs to be added.
|
||||
2. **If not, is it worth adding one** (`server/models/device_instance.py` etc.), or is this genuinely a one-off maintenance/schema query that belongs in the core-plugin exception list above?
|
||||
3. **Check the collation the query relies on** against the column's actual schema (`server/db/schema/app.sql`) rather than assuming — `devMac`/`eveMac`/`sesMac`/`scanMac`/`devParentMAC` are declared `COLLATE NOCASE` at the column level, so an explicit `COLLATE NOCASE` against one of them in a new query is redundant (harmless, but a sign the author didn't check). `devName` has **no** column-level collation — an explicit `COLLATE NOCASE` there is genuinely necessary if case-insensitive name matching is intended, not a mistake.
|
||||
4. **Parameterization** — `?` placeholders, never string-formatted values into the query (this part is usually already fine; flag it if not).
|
||||
|
||||
## Worked example: PR #1788 (DOCKERDISC plugin)
|
||||
|
||||
Two raw queries in `server/plugins/dockerdisc/script.py`:
|
||||
|
||||
- `lookup_device_mac()`: `SELECT 1 FROM Devices WHERE devMac = ? COLLATE NOCASE LIMIT 1` — an existence check. `DeviceInstance.getByMac(mac)` already does this exact lookup (`server/models/device_instance.py:102-105`); its `COLLATE NOCASE` is redundant since `devMac` already carries that collation at the column level. **Fix: `DeviceInstance().getByMac(mac) is not None`, delete the raw SQL.**
|
||||
- `resolve_host_mac()`: `SELECT devMac FROM Devices WHERE devName = ? COLLATE NOCASE` — a name lookup with real 0/1/many-match handling (falls back to a manually-configured MAC on ambiguity or no match). No existing method covers this. `devName` has no column-level collation, so the explicit `COLLATE NOCASE` here is correct, not redundant. **Fix: add `DeviceInstance.getAllByName(name)` returning every match** (not just one — the plugin's own ambiguity detection needs the full set), and have the plugin call that instead.
|
||||
|
||||
This is the shape of the fix in general: an existence/single-row check usually already has a model method; a query with plugin-specific result handling (ambiguity, filtering) usually needs a small new method added rather than a workaround in the plugin itself.
|
||||
@@ -33,6 +33,7 @@ Skills with the same purpose exist in more than one, sometimes under different n
|
||||
| PRD writing | `prd-writing` | `prd-writing` | `prd-writing` | Methodology for writing a design doc: challenge the idea, verify every claim against actual code, trace every downstream consumer of a new mechanism, evaluate performance impact against the real schema/indexes, record rejected alternatives and open-issue decisions explicitly, final-check pass before done. |
|
||||
| UX/frontend design | `ux-design-patterns` | `ux-design-patterns` | `ux-design-patterns` | Don't invent new UX behavior/visual patterns unless a PRD calls for it - search `front/` for an existing pattern first and reuse it. Priority order for design tradeoffs when several options are reasonable: existing behavior > intuitiveness > information density > usability > utility > uniqueness > industry practices > generic UI. |
|
||||
| Skill hygiene | `skill-hygiene` | `skill-hygiene` | `skill-hygiene` | Read before writing/editing any SKILL.md, or any research/audit doc in `.gemini/internal-docs/research/`. Two standing rules: state current behavior only (no "Correction:", no "as of <date>", no "caught in review" narration - that trail belongs in PRDs), and prefer plain, short wording. Includes the grep sweep to run before calling a doc clean. |
|
||||
| Plugin review | `plugin-review` | `plugin-review` | `plugin-review` | Reviewer-facing complement to Plugin dev above: no raw SQL in a plugin script - use an existing/new `server/models/*.py` method, not GraphQL (which no plugin reaches). Named exception list for 5 pre-existing core/infra plugins with legitimate direct DB access, a collation-checking step, and a worked example from a real PR (#1788, DOCKERDISC). |
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -89,5 +89,5 @@ Procedural/how-to knowledge (running tests, resetting the DB, devcontainer manag
|
||||
- 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.
|
||||
- Every Python function/method gets a succinct docstring describing its current use and behavior — one or two sentences, not a changelog of what changed or why (that belongs in the commit/PR, not the docstring).
|
||||
- Every Python function/method gets a succinct docstring describing its current use and behavior — one or two sentences, not a changelog of what changed or why (that belongs in the commit/PR, not the docstring). Same rule for JS: a JSDoc `/** ... */` block, not a plain `//` line above the function. Whenever you touch a function that only has a plain description comment (Python or JS), convert it to a proper docstring as part of that edit rather than leaving the old style next to new code.
|
||||
- Before adding a new key to `front/php/templates/language/en_us.json`, search it for an existing key with the same text/purpose and reuse it — prefer generic `Gen_*` keys over page-scoped names for genuinely generic UI text (e.g. `Gen_Prev`/`Gen_Next`, not `Presence_Page_Prev`). Only the English file needs a new key; other locales fall back to it automatically.
|
||||
+6
-2
@@ -1098,12 +1098,16 @@ function hideSpinner() {
|
||||
|
||||
|
||||
// --------------------------------------------------------
|
||||
// Calls a backend function to add a front-end event to an execution queue
|
||||
/**
|
||||
* Queue an "update_api" ad-hoc event so the backend refreshes just the
|
||||
* given data sources (e.g. "devices,appevents") on its next tick.
|
||||
* @param {string} apiEndpoints - Comma-separated data source names.
|
||||
*/
|
||||
function updateApi(apiEndpoints)
|
||||
{
|
||||
|
||||
// value has to be in format event|param. e.g. run|ARPSCAN
|
||||
action = `${getGuid()}|update_api|${apiEndpoints}`
|
||||
action = `update_api|${apiEndpoints}`
|
||||
|
||||
const { token: apiToken, apiBase: apiBaseUrl, authHeader } = getAuthContext();
|
||||
const url = `${apiBaseUrl}/logs/add-to-execution-queue`;
|
||||
|
||||
@@ -620,6 +620,12 @@ function initializeDatatable (status) {
|
||||
|
||||
|
||||
// -----------------------------------------------------------------------------
|
||||
/**
|
||||
* Poll execution_queue.log for a pending "update_api|devices" entry and show
|
||||
* a spinner while it's queued; reload the page once it clears if a reload
|
||||
* was requested.
|
||||
* @param {boolean} [needsReload=false] - Reload the page once the queue clears.
|
||||
*/
|
||||
function handleLoadingDialog(needsReload = false)
|
||||
{
|
||||
// console.log(`needsReload: ${needsReload}`);
|
||||
@@ -630,7 +636,7 @@ function handleLoadingDialog(needsReload = false)
|
||||
{
|
||||
showSpinner("devices_old")
|
||||
|
||||
setTimeout(handleLoadingDialog(true), 1000);
|
||||
setTimeout(() => handleLoadingDialog(true), 1000);
|
||||
|
||||
} else if (needsReload)
|
||||
{
|
||||
|
||||
@@ -549,8 +549,14 @@ function goToDevice(mac, newtab = false) {
|
||||
|
||||
|
||||
// --------------------------------------------------------
|
||||
// Updating the execution queue in in modal pop-up
|
||||
function updateModalState() {
|
||||
/**
|
||||
* Poll execution_queue.log into the ad-hoc-event modal until it's empty or a
|
||||
* safety cap is hit (was: polled forever with no stop condition).
|
||||
* @param {number} [elapsedMs=0] - Total time already spent polling.
|
||||
*/
|
||||
function updateModalState(elapsedMs = 0) {
|
||||
const MAX_POLL_MS = 60000; // safety net - stop after 1 minute regardless
|
||||
|
||||
setTimeout(function() {
|
||||
// Fetch the content from the log file using an AJAX request
|
||||
$.ajax({
|
||||
@@ -560,7 +566,10 @@ function updateModalState() {
|
||||
// Update the content of the HTML element (e.g., a div with id 'logContent')
|
||||
$('#'+modalEventStatusId).html(data);
|
||||
|
||||
updateModalState();
|
||||
if (data.trim() === '' || elapsedMs + 2000 >= MAX_POLL_MS) {
|
||||
return; // queue drained (or safety net hit) - stop polling
|
||||
}
|
||||
updateModalState(elapsedMs + 2000);
|
||||
},
|
||||
error: function() {
|
||||
// Handle error, such as the file not being found
|
||||
|
||||
@@ -76,7 +76,13 @@ function SQLite3_connect($trytoreconnect = true, $retryCount = 0) {
|
||||
// Write unlock status to the locked file
|
||||
file_put_contents($DBFILE_LOCKED_FILE, '0');
|
||||
|
||||
return new SQLite3($DBFILE, SQLITE3_OPEN_READWRITE);
|
||||
$conn = new SQLite3($DBFILE, SQLITE3_OPEN_READWRITE);
|
||||
// Wait up to 5s for a lock before returning SQLITE_BUSY, matching the
|
||||
// Python main-loop connection's PRAGMA busy_timeout - without this,
|
||||
// any collision with a concurrent writer fails immediately instead
|
||||
// of retrying internally.
|
||||
$conn->busyTimeout(5000);
|
||||
return $conn;
|
||||
} catch (Exception $exception) {
|
||||
// sqlite3 throws an exception when it is unable to connect
|
||||
global $db_locked;
|
||||
|
||||
+38
-9
@@ -484,6 +484,11 @@ function autoHideEmptyTabs(counts, prefixes) {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the plugin tab headers/panes for every show_ui plugin with a
|
||||
* non-zero object/event/history count, and wire up (re-)initialization of
|
||||
* each one's DataTable(s) via shown.bs.tab.
|
||||
*/
|
||||
function generateTabs() {
|
||||
|
||||
// Reset the tabs by clearing previous headers and content
|
||||
@@ -515,7 +520,10 @@ function generateTabs() {
|
||||
|
||||
// Now that ALL DOM elements exist (both <a> headers and tab panes),
|
||||
// wire up DataTable initialization: immediate for the active tab,
|
||||
// deferred via shown.bs.tab for the rest.
|
||||
// on every subsequent shown.bs.tab for the rest - not just the first,
|
||||
// so revisiting a plugin tab refreshes its active sub-table instead of
|
||||
// leaving it stuck on whatever its one-time first fetch returned
|
||||
// (namespaced + off() first so repeat calls don't stack duplicate handlers).
|
||||
let firstVisible = true;
|
||||
visiblePlugins.forEach(pluginObj => {
|
||||
const prefix = pluginObj.unique_prefix;
|
||||
@@ -524,9 +532,11 @@ function generateTabs() {
|
||||
initializeDataTables(prefix, colDefinitions, pluginObj);
|
||||
firstVisible = false;
|
||||
} else {
|
||||
$(`a[href="#${prefix}"]`).one('shown.bs.tab', function() {
|
||||
initializeDataTables(prefix, colDefinitions, pluginObj);
|
||||
});
|
||||
$(`a[href="#${prefix}"]`)
|
||||
.off('shown.bs.tab.pluginsCore')
|
||||
.on('shown.bs.tab.pluginsCore', function() {
|
||||
initializeDataTables(prefix, colDefinitions, pluginObj);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
@@ -651,6 +661,14 @@ function generateDataTable(prefix, tableType, colDefinitions) {
|
||||
`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build (or, if already built, refresh) the Objects/Events/History
|
||||
* DataTables for one plugin - the active sub-tab immediately, the other two
|
||||
* on their own shown.bs.tab. Safe to call more than once per prefix.
|
||||
* @param {string} prefix - The plugin's unique_prefix.
|
||||
* @param {object[]} colDefinitions - This plugin's visible database_column_definitions.
|
||||
* @param {object} pluginObj - The full plugin definition from plugins.json.
|
||||
*/
|
||||
function initializeDataTables(prefix, colDefinitions, pluginObj) {
|
||||
const mac = $("#txtMacFilter").val();
|
||||
const foreignKey = (mac && mac !== "--") ? mac : null;
|
||||
@@ -661,9 +679,16 @@ function initializeDataTables(prefix, colDefinitions, pluginObj) {
|
||||
{ tableId: `historyTable_${prefix}`, gqlField: 'pluginsHistory', countId: `histCount_${prefix}`, badgeId: null },
|
||||
];
|
||||
|
||||
/**
|
||||
* Build tableId's DataTable on first call; on any later call, refresh its
|
||||
* data in place instead of no-op'ing (the whole point of this fix).
|
||||
*/
|
||||
function buildDT(tableId, gqlField, countId, badgeId) {
|
||||
if ($.fn.DataTable.isDataTable(`#${tableId}`)) {
|
||||
return; // already initialized
|
||||
// Already built - refresh in place (keep current page/sort) instead of
|
||||
// leaving it stuck on whatever its first-ever fetch returned.
|
||||
$(`#${tableId}`).DataTable().ajax.reload(null, false);
|
||||
return;
|
||||
}
|
||||
const skelId = `#skel-${tableId.replace('Table_', 'Target_')}`;
|
||||
$(`#${tableId}`).DataTable({
|
||||
@@ -719,10 +744,14 @@ function initializeDataTables(prefix, colDefinitions, pluginObj) {
|
||||
// This sub-tab is the currently active one — initialize immediately
|
||||
buildDT(cfg.tableId, cfg.gqlField, cfg.countId, cfg.badgeId);
|
||||
} else if ($subPane.closest('.tab-pane').length) {
|
||||
// Defer until shown
|
||||
$(`a[href="${href}"]`).one('shown.bs.tab', function() {
|
||||
buildDT(cfg.tableId, cfg.gqlField, cfg.countId, cfg.badgeId);
|
||||
});
|
||||
// Build on first shown, refresh (via buildDT's reload branch) on every
|
||||
// one after - namespaced + off() first since initializeDataTables()
|
||||
// itself can now run more than once (see generateTabs()).
|
||||
$(`a[href="${href}"]`)
|
||||
.off('shown.bs.tab.pluginsCore')
|
||||
.on('shown.bs.tab.pluginsCore', function() {
|
||||
buildDT(cfg.tableId, cfg.gqlField, cfg.countId, cfg.badgeId);
|
||||
});
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
@@ -33,6 +33,7 @@ GROUPS = [
|
||||
[".gemini/skills/prd-writing/SKILL.md", ".github/skills/prd-writing/SKILL.md", ".claude/skills/prd-writing/SKILL.md"],
|
||||
[".gemini/skills/ux-design-patterns/SKILL.md", ".github/skills/ux-design-patterns/SKILL.md", ".claude/skills/ux-design-patterns/SKILL.md"],
|
||||
[".gemini/skills/skill-hygiene/SKILL.md", ".github/skills/skill-hygiene/SKILL.md", ".claude/skills/skill-hygiene/SKILL.md"],
|
||||
[".gemini/skills/plugin-review/SKILL.md", ".github/skills/plugin-review/SKILL.md", ".claude/skills/plugin-review/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"],
|
||||
|
||||
+2
-1
@@ -189,7 +189,8 @@ class app_state_class:
|
||||
buildTimestamp=self.buildTimestamp,
|
||||
last_scan_run=self.last_scan_run,
|
||||
next_scan_time=self.next_scan_time,
|
||||
pause_until=self.pause_until
|
||||
pause_until=self.pause_until,
|
||||
pluginsStates=self.pluginsStates
|
||||
)
|
||||
except Exception as e:
|
||||
mylog("none", [f"[app_state] SSE broadcast: {e}"])
|
||||
|
||||
@@ -73,6 +73,11 @@ class DB:
|
||||
# The WAL journaling mode uses a write-ahead log instead of a
|
||||
# rollback journal to implement transactions.
|
||||
self.sql_connection.execute("pragma journal_mode=WAL;")
|
||||
# Wait up to 5s for a lock before raising "database is locked",
|
||||
# matching get_temp_db_connection()'s value - without this, any
|
||||
# collision with a concurrent writer (e.g. a PHP request) fails
|
||||
# immediately instead of retrying internally.
|
||||
self.sql_connection.execute("PRAGMA busy_timeout=5000;")
|
||||
# When synchronous is NORMAL (1), the SQLite database engine will
|
||||
# still sync at the most critical moments,
|
||||
# but less often than in FULL mode.
|
||||
|
||||
@@ -115,6 +115,13 @@ class DeviceInstance:
|
||||
SELECT * FROM Devices WHERE devLastIP = ?
|
||||
""", (ip,))
|
||||
|
||||
def getAllByName(self, name):
|
||||
"""Return every device whose devName matches (case-insensitive) - devName has
|
||||
no column-level collation, so COLLATE NOCASE is explicit here, unlike devMac."""
|
||||
return self._fetchall("""
|
||||
SELECT * FROM Devices WHERE devName = ? COLLATE NOCASE
|
||||
""", (name,))
|
||||
|
||||
def queryByConditions(self, conditions):
|
||||
"""Query Devices using a list of condition dicts.
|
||||
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
"""
|
||||
Unit tests for server/models/device_instance.py's DeviceInstance model methods.
|
||||
|
||||
Covers:
|
||||
- DeviceInstance.getAllByName()
|
||||
"""
|
||||
|
||||
import sys
|
||||
import os
|
||||
import unittest
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "..", "server"))
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||
|
||||
from db_test_helpers import make_db, make_device_dict, insert_device_from_dict
|
||||
|
||||
|
||||
class TestGetAllByName(unittest.TestCase):
|
||||
"""devName has no column-level collation (unlike devMac), so getAllByName()
|
||||
must apply COLLATE NOCASE itself, and must return every match rather than
|
||||
just one - callers (e.g. the dockerdisc plugin's resolve_host_mac()) rely
|
||||
on the full set to detect an ambiguous (multiple-match) name."""
|
||||
|
||||
def setUp(self):
|
||||
self.conn = make_db()
|
||||
devices = [
|
||||
make_device_dict("aa:bb:cc:dd:ee:01", devName="docker-host-1"),
|
||||
make_device_dict("aa:bb:cc:dd:ee:02", devName="Docker-Host-1"),
|
||||
make_device_dict("aa:bb:cc:dd:ee:03", devName="other-host"),
|
||||
]
|
||||
for d in devices:
|
||||
insert_device_from_dict(self.conn, d)
|
||||
self.conn.commit()
|
||||
|
||||
def _instance(self):
|
||||
from models.device_instance import DeviceInstance
|
||||
inst = DeviceInstance()
|
||||
|
||||
def _fetchall(q, p=()):
|
||||
rows = self.conn.execute(q, p).fetchall()
|
||||
return [dict(r) for r in rows]
|
||||
inst._fetchall = _fetchall
|
||||
return inst
|
||||
|
||||
def test_case_insensitive_match_returns_all_ambiguous_rows(self):
|
||||
inst = self._instance()
|
||||
results = inst.getAllByName("docker-host-1")
|
||||
macs = {r["devMac"] for r in results}
|
||||
self.assertEqual(macs, {"aa:bb:cc:dd:ee:01", "aa:bb:cc:dd:ee:02"})
|
||||
|
||||
def test_case_insensitive_match_different_case_query(self):
|
||||
inst = self._instance()
|
||||
results = inst.getAllByName("OTHER-HOST")
|
||||
self.assertEqual(len(results), 1)
|
||||
self.assertEqual(results[0]["devMac"], "aa:bb:cc:dd:ee:03")
|
||||
|
||||
def test_no_match_returns_empty_list(self):
|
||||
inst = self._instance()
|
||||
results = inst.getAllByName("does-not-exist")
|
||||
self.assertEqual(results, [])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,86 @@
|
||||
"""
|
||||
Unit tests for the execution_queue.log action-string format and dispatch
|
||||
(server/models/user_events_queue_instance.py, plugin.check_and_run_user_event()).
|
||||
|
||||
Covers the fix for updateApi() (front/js/common.js) prepending an extra
|
||||
client-side GUID onto its action string, which broke check_and_run_user_event()
|
||||
and finalize_event()'s shared "split('|')[2:4]" parsing - see
|
||||
.gemini/internal-docs/PRDs/execution-queue-fe-locking-fix.md, fix A.
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
import tempfile
|
||||
import shutil
|
||||
import unittest
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import patch
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "..", "server"))
|
||||
|
||||
from models.user_events_queue_instance import UserEventsQueueInstance # noqa: E402
|
||||
import plugin as plugin_module # noqa: E402
|
||||
|
||||
|
||||
class TestExecutionQueueDispatch(unittest.TestCase):
|
||||
"""Isolates execution_queue.log to a temp dir so tests don't touch the
|
||||
real logPath, and don't depend on plugin_manager's full __init__ (DB
|
||||
settings cache, schedules, logger) - check_and_run_user_event() only
|
||||
touches self.db/self.all_plugins, so a bare SimpleNamespace stands in."""
|
||||
|
||||
def setUp(self):
|
||||
self._tmpdir = tempfile.mkdtemp()
|
||||
self._patcher = patch("models.user_events_queue_instance.logPath", self._tmpdir)
|
||||
self._patcher.start()
|
||||
|
||||
def tearDown(self):
|
||||
self._patcher.stop()
|
||||
shutil.rmtree(self._tmpdir, ignore_errors=True)
|
||||
|
||||
def _fake_manager(self):
|
||||
return SimpleNamespace(db=None, all_plugins=[])
|
||||
|
||||
def test_check_and_run_user_event_dispatches_update_api(self):
|
||||
"""The fixed action format (no extra client GUID) must reach the
|
||||
elif event == "update_api" branch and call update_api() with the
|
||||
parsed params and is_ad_hoc_user_event=True. finalize_event("update_api")
|
||||
is called inside the real update_api()'s try_write() (api.py:205-208),
|
||||
not here - since update_api is mocked, the line is expected to still
|
||||
be in the log (verified separately in test_finalize_event_removes_update_api_line)."""
|
||||
q = UserEventsQueueInstance()
|
||||
q.add_event("update_api|devices,appevents")
|
||||
|
||||
with patch("plugin.update_api") as mock_update_api:
|
||||
plugin_module.plugin_manager.check_and_run_user_event(self._fake_manager())
|
||||
|
||||
mock_update_api.assert_called_once_with(None, [], False, ["devices", "appevents"], True)
|
||||
|
||||
def test_finalize_event_removes_update_api_line(self):
|
||||
q = UserEventsQueueInstance()
|
||||
q.add_event("update_api|devices,appevents")
|
||||
|
||||
removed = q.finalize_event("update_api")
|
||||
|
||||
self.assertTrue(removed)
|
||||
self.assertEqual(q.read_log(), [])
|
||||
|
||||
def test_malformed_action_falls_through_to_unhandled_branch(self):
|
||||
"""Regression guard for the exact bug fixed: an action string with an
|
||||
extra field before "update_api" (the old updateApi() format) must
|
||||
NOT reach the update_api() call - it's misrouted into the "else"
|
||||
unhandled-event branch instead. The line still gets removed (both
|
||||
check_and_run_user_event() and finalize_event() parse it the same
|
||||
wrong way and agree with each other), but the intended fast-refresh
|
||||
never fires."""
|
||||
q = UserEventsQueueInstance()
|
||||
q.add_event("11111111-1111-1111-1111-111111111111|update_api|devices,appevents")
|
||||
|
||||
with patch("plugin.update_api") as mock_update_api:
|
||||
plugin_module.plugin_manager.check_and_run_user_event(self._fake_manager())
|
||||
|
||||
mock_update_api.assert_not_called()
|
||||
self.assertEqual(q.read_log(), [])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,64 @@
|
||||
"""
|
||||
Regression guard for server/app_state.py's updateState()/broadcast_state_update()
|
||||
call - pluginsStates must reach the SSE broadcast payload, not just the
|
||||
persisted app_state.json. See
|
||||
.gemini/internal-docs/PRDs/to_review/execution-queue-fe-locking-fix.md's
|
||||
post-implementation addendum: the original broadcast_state_update() call
|
||||
never passed pluginsStates, so front/js/ui_components.js's watchPluginState()
|
||||
(fix C1) waited on an SSE event that could never arrive.
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
import tempfile
|
||||
import shutil
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "..", "server"))
|
||||
|
||||
import app_state # noqa: E402
|
||||
|
||||
|
||||
class TestAppStateSSEBroadcast(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._tmpdir = tempfile.mkdtemp()
|
||||
self._patchers = [
|
||||
patch("app_state.apiPath", self._tmpdir + os.sep),
|
||||
patch("app_state.checkNewVersion", lambda *a, **k: False),
|
||||
]
|
||||
for p in self._patchers:
|
||||
p.start()
|
||||
|
||||
def tearDown(self):
|
||||
for p in self._patchers:
|
||||
p.stop()
|
||||
shutil.rmtree(self._tmpdir, ignore_errors=True)
|
||||
|
||||
def test_update_state_broadcasts_plugins_states(self):
|
||||
with patch("app_state.broadcast_state_update") as mock_broadcast:
|
||||
app_state.updateState(
|
||||
pluginsStates={"INTRSPD": {"stateUpdated": "2026-09-14 12:00:00", "totalObjects": 1}}
|
||||
)
|
||||
|
||||
mock_broadcast.assert_called_once()
|
||||
_, kwargs = mock_broadcast.call_args
|
||||
self.assertIn("pluginsStates", kwargs)
|
||||
self.assertEqual(kwargs["pluginsStates"]["INTRSPD"]["totalObjects"], 1)
|
||||
|
||||
def test_plugins_states_persist_and_merge_across_calls(self):
|
||||
"""updateState() merges into the existing pluginsStates dict rather
|
||||
than replacing it - a second plugin's update must not drop the
|
||||
first's entry, and each broadcast must carry the full merged dict."""
|
||||
with patch("app_state.broadcast_state_update") as mock_broadcast:
|
||||
app_state.updateState(pluginsStates={"PLUGINA": {"stateUpdated": "2026-09-14 12:00:00"}})
|
||||
app_state.updateState(pluginsStates={"PLUGINB": {"stateUpdated": "2026-09-14 12:00:05"}})
|
||||
|
||||
self.assertEqual(mock_broadcast.call_count, 2)
|
||||
_, last_kwargs = mock_broadcast.call_args
|
||||
self.assertIn("PLUGINA", last_kwargs["pluginsStates"])
|
||||
self.assertIn("PLUGINB", last_kwargs["pluginsStates"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,43 @@
|
||||
"""
|
||||
Regression guard for the main-loop SQLite connection's busy_timeout
|
||||
(server/database.py's DB.open()) - see
|
||||
.gemini/internal-docs/PRDs/execution-queue-fe-locking-fix.md, fix D.
|
||||
|
||||
Without PRAGMA busy_timeout, a collision with a concurrent writer (e.g. a
|
||||
PHP request) fails immediately with "database is locked" instead of
|
||||
retrying internally for a bounded window.
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
import tempfile
|
||||
import shutil
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "..", "server"))
|
||||
|
||||
from database import DB # noqa: E402
|
||||
|
||||
|
||||
class TestDatabaseBusyTimeout(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._tmpdir = tempfile.mkdtemp()
|
||||
self._db_path = os.path.join(self._tmpdir, "test_app.db")
|
||||
|
||||
def tearDown(self):
|
||||
shutil.rmtree(self._tmpdir, ignore_errors=True)
|
||||
|
||||
def test_open_sets_busy_timeout_5000ms(self):
|
||||
db = DB()
|
||||
with patch("database.fullDbPath", self._db_path):
|
||||
db.open()
|
||||
try:
|
||||
value = db.sql_connection.execute("PRAGMA busy_timeout;").fetchone()[0]
|
||||
self.assertEqual(value, 5000)
|
||||
finally:
|
||||
db.sql_connection.close()
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in new issue
Block a user