Implement separate icon and background image libraries

[alcatron]

Replace the image-manager placeholder with separate icon and background uploaders and existing-image libraries. Show previews, dimensions and map references; allow confirmed physical deletion of unreferenced managed images. Accept PNG, JPG/JPEG and GIF (first frame), subject to upload size and pixel limits. Preserve unmanaged current icon selections and accept case-insensitive raster extensions after saving and reloading.

Protect reference scans and deletion with the config-directory mutex. Configuration saves and repairs use checked atomic replacement with revision comparison, so stale data cannot overwrite concurrent edits. Create-only cloning reserves the file before creating map/auth/settings records or invoking the runner, so failed writes leave no broken registrations. Management creation also checks create-only writes. Loaded page revisions reject stale editor submissions. Controls and initial JavaScript metadata share one WeatherMap snapshot; saves reload the full editor state through a clean view URL rather than updating a token beside stale controls. A deletion race producing a null loaded revision returns HTTP 404. INCLUDE reference scanning follows literal renderer paths, including filenames with brackets; the symlink regression uses a temporary library root without renaming shared checkout assets. Clone registration binds both source and destination IDs in prepared statements. Clones retain the mutex through registration and clean their own reservation and partial records on false returns or exceptions. Raw config saves no longer report false post-save conflicts, and request CAS races return HTTP 409. Repair copies legacy assets without removing originals. Initial reference discovery scans .conf maps only, while literal INCLUDE files remain supported. All editor save actions propagate write failures to AJAX callers; stale move/delete targets return errors. The Close control is keyboard-focusable, and upgrade test stubs run in isolated processes.

Snapshot tokens include all loaded INCLUDE dependencies, and their hashes are rechecked at commit time under the appropriate directory mutexes. A newly selected managed image must still exist when the configuration is committed; a deletion-winning race returns HTTP 409 and leaves the configuration unchanged. Existing missing legacy references remain editable for repair. New-map failures retain their save status and show retry guidance, and registration supports readable read-only config directories.

Image validation inspects the proposed serialized ICON/BACKGROUND directives, including raw node overrides. Deletion locks logical and physical configuration sources and rescans references after acquiring those locks, protecting alias/direct writers together.

New map copies validate every inherited managed image reference at commit time. Legacy repair confines sources to the plugin tree and verifies raster decoding. Save and Enter submission restore the active dialog action.

Preserve tokenized icon paths during copy/raw editing, validate concrete inherited references in management cloning before registration, and reject corrupt or symlinked existing repair destinations.

Uploads publish only complete encoded images after flush, close and permission setup; staging files stay outside selectable library names. Create-only copies and management cloning validate effective image references inherited through INCLUDE files, with snapshot dependencies checked under their directory locks. Legacy sized raster icon repair preserves width and height.

Mutation routes require loaded revisions. Clone failures and cleanup errors log internal diagnostics server-side and show generic retry guidance.

New-destination copies bind the loaded source root revision and target identity at commit time. Cleanup attempts run independently and retain the complete config if map-row removal is uncertain; failed unlink is logged. Canonical destination identity preserves CAS across aliases, and logical/physical INCLUDE snapshots detect retargeting.

Validation: affected branch PHP 8.3 and browser regressions pass in isolated Cacti fixtures. The combined changes pass 292 PHP tests (5859 assertions) and 16 browser regression programs. Changed-line coverage passes at 100% of measured production lines; existing controller/entry-point exemptions remain. Syntax, whitespace, translation and manifest checks pass. These revisions have not been deployed to the live installation.

Based on current develop, including the merged icon picker and gallery resize. Shared editor files overlap with the other feature PRs.

[Copilot]

🟡 Changes recommended

Reference detection can permit deletion of templated image dependencies, and required dimensions and usage metadata are not displayed.

9 open findings

• Parse image directives for reliable reference checks · New
• Return image dimensions and reference metadata · New
• Use a larger random suffix to prevent filename collisions · New
• Use translated strings for dynamic dialog text · New
• Reindent the endpoint block with tabs · New
• Localize user-visible exception messages · New
• Add the required GPL v2 header to the support script · New
• Add the required GPL v2 header to the PHP test · New
• Localize new labels and buttons with gettext · New

What changed in this PR

Adds separate node-icon and background-image management libraries to the authenticated editor.

Changes:

• Adds upload, listing, preview, and deletion endpoints.
• Adds responsive image-management UI and styling.
• Adds image-format and deletion regression coverage.

File	Description
weathermap-cacti-plugin-editor.php	Registers image actions and dialog markup.
lib/​editor.assets.php	Implements image validation, storage, listing, and reference checks.
lib/​editor.actions.php	Adds JSON upload/delete request handling.
js/​editor.js	Implements library rendering and AJAX interactions.
css/​editor.css	Styles the responsive image libraries.
tests/​Unit/​EditorImageFormatsTest.php	Runs the image regression fixture.
tests/​Support/​EditorImageFormatsRegression.php	Tests formats, limits, references, and deletion.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Symlinked parent directories can escape deletion safeguards, missing includes disable the feature, and listing scales poorly.

6 open findings

• Reject symlinked images parent directories · New
• Skip unmatched optional configuration includes · New
• Cache configuration reference index per request · New
• Localize new labels and buttons with gettext
• Localize user-visible exception messages
• Use translated strings for dynamic dialog text

6 resolved since last review

• Parse image directives for reliable reference checks
• Use a larger random suffix to prevent filename collisions
• Return image dimensions and reference metadata
• Add the required GPL v2 header to the PHP test
• Add the required GPL v2 header to the support script
• Reindent the endpoint block with tabs

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Uploads can remain persisted after a reference-index failure is reported as an upload failure, and required PHPDoc is missing.

0 open findings

6 resolved since last review

• Reject symlinked images parent directories
• Cache configuration reference index per request
• Skip unmatched optional configuration includes
• Localize new labels and buttons with gettext
• Localize user-visible exception messages
• Use translated strings for dynamic dialog text

Previously missed (3)

In code that hasn't changed since last review

Build reference index before committing uploaded image

lib/​editor.actions.php:1151

Build the reference index before persisting the upload. As written, wm_asset_store_image() commits the file first, then wm_asset_image_list() can throw while scanning an unreadable configuration or when the 1,000-file limit is exceeded. The endpoint then reports a failed upload even though the image remains on disk, and retries create duplicates.

Document new asset functions with complete PHPDoc

lib/​editor.actions.php:1143

Both newly added functions need the repository-required PHPDoc blocks, including descriptions, parameters, and return values.

Add required PHPDoc structure to asset helpers

lib/​editor.assets.php:26

The new asset helpers do not follow the repository-required PHPDoc shape. Add a description, blank separator, @param entries, another blank separator, and @return to each function in this file; the one-line block here also lacks the parameters and return type.

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Image deletion can race with configuration writes, and the dimensions display does not satisfy the stated localization requirement.

1 open finding

• Lock configuration references during scan and image deletion · New

Previously missed (2)

In code that hasn't changed since last review

Use a translatable format for image dimensions

js/​editor.js:1679

The dimensions string hard-codes both the ordering and px, so translators cannot localize it, contrary to the PR's stated requirement that dimensions remain localizable. Add a translated dimensions format to wmEditorText, render through that format here, and regenerate cacti.pot.

Add required PHPDoc for the new helper

tests/​Support/​EditorImageFormatsRegression.php:27

This newly introduced helper lacks the PHPDoc block required for every non-vendored function. Document its purpose, parameters, and return value using the repository's standard shape.

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Selector refresh and .jpeg reload handling can discard valid existing image selections.

6 open findings

• Image pruning removes unmanaged raster paths · New
• Lock configuration references during scan and image deletion
• JPEG backgrounds are omitted after editor reload · New
• Use cacti_count() instead of raw count() · New
• Add PHPDoc and expand helper control flow · New
• Use cacti_count() in the PHP test · New

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Path portability and selector-refresh defects can corrupt uploads or remove valid selections and deletion safeguards.

9 open findings

• Normalize Windows paths before matching asset references · New
• Match referenced image paths case-insensitively · New
• Image pruning removes unmanaged raster paths
• Lock configuration references during scan and image deletion
• Open GD output streams in binary mode · New
• JPEG backgrounds are omitted after editor reload
• Use cacti_count() in the PHP test
• Add PHPDoc and expand helper control flow
• Use cacti_count() instead of raw count()

🧠 Review effort: Balanced

[alcatron]

Follow-up audit fixed the initial selector path as well as the AJAX library listing in f07f47a. get_imagelist now rejects non-files, symlinks, unreadable images, invalid image bytes and detected formats outside PNG/JPEG/GIF. Regression tests check that invalid data and BMP contents disguised as PNG are omitted from both the library list and initial selectors, in both icon and background categories.

Validation: 289 PHP tests and all browser regression programs pass. Syntax, whitespace, manifest and translation checks pass. Measured changed production PHP coverage is 100% (648/648 lines), with existing entry-point/controller exemptions.

[Copilot]

🟡 Changes recommended

Path handling and incomplete deletion locks can leave broken image references, while selector filtering can clear existing backgrounds.

2 open findings

• Lock and rescan missing INCLUDE destinations before deletion · New
• Keep readable symlinked backgrounds selectable · New

2 resolved since last review

• Unsupported image formats are added to the library
• Mutation routes bypass loaded-revision validation

Previously missed (2)

In code that hasn't changed since last review

Normalize managed-image paths before existence checks

lib/​WeatherMap.class.php:2459

Both managed-image collectors recognize only literal images/objects/ and images/backgrounds/ prefixes. Renderer-valid paths such as ./images/objects/router.png, images/./objects/router.png, or absolute library paths therefore bypass commit-time existence checks. Raw edits can save a deleted image, and copies or management clones can inherit missing images through INCLUDE files. Normalize path identities consistently in both collectors and the loaded-reference baseline, including missing files, while preserving the original serialized paths and renderer tokens.

Preserve symlink semantics when resolving relative INCLUDE paths

lib/​editor.assets.php:113

Lexically collapsing .. before resolving an INCLUDE changes its meaning when a preceding directory is a symlink. For INCLUDE linked/../include.conf, the renderer reads from the symlink target's parent, but this scan may check a different, nonexistent file and skip it. Images referenced only by that include can then be deleted. Make relative INCLUDE paths absolute without collapsing their components, leaving resolution to the filesystem.

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Regression cleanup leaves dangling image symlinks in the shared checkout after successful runs.

1 open finding

• Clean up dangling background symlinks after fixture deletion · New

2 resolved since last review

• Lock and rescan missing INCLUDE destinations before deletion
• Keep readable symlinked backgrounds selectable

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Cross-line directive matching can miss referenced images during deletion and bypass commit-time image validation.

1 open finding

• Whole-file regexes consume newlines and misparse image directives · New

1 resolved since last review

• Clean up dangling background symlinks after fixture deletion

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Coordinated configuration writes, dependency locking and physical image deletion warrant final human verification.

0 open findings

1 resolved since last review

• Whole-file regexes consume newlines and misparse image directives

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Reference scanning can read non-regular INCLUDE targets, blocking workers or exhausting memory.

1 open finding

• Reject non-regular INCLUDE targets before reading · New

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Concurrent image deletion, dependency locking and clone registration require final human integration review.

0 open findings

1 resolved since last review

• Reject non-regular INCLUDE targets before reading

🧠 Review effort: Balanced

[alcatron]

Merged current develop (41079aa, including #302) into this branch in 6c01c0f and resolved the editor CSS/JavaScript, dispatch, helper and translation conflicts while retaining both colour/pulsing and image-library functionality. The engine changes merged cleanly. Updated the standalone clone schema fixture for the effective-scale count helper used by the merged engine.

Combined validation: 292 PHP tests passed (5,894 assertions), plus all browser regression programs, including colours, glow, image libraries and notes. PHP/JavaScript syntax, whitespace, manifest and translation checks pass. Measured changed production PHP coverage against current develop is 100% (184/184 lines), with the existing controller/entry-point exemptions.