[marconetsf]

Problem

The Weidmuller UC20 VPP, and any future VPP that follows the same model,
needs an opaque archive next to its JSON config (for Weidmuller:
conf/<name>.station.bin, which the plugin forwards to /var/run/devices.sock
as a DeployRequest.data payload for the u-OS systembus service). The editor
already emits that archive in the uploaded zip, but the runtime copies only
conf/<name>.json and the .license sibling (hardcoded carve-out) to
VPP_DATA_DIR. The archive is extracted into core/generated/conf/ and left
there, so wm_deploy_apply cannot find it and the station is never deployed.

Issue: RTOP-392

Change

apply_vpp_plugin_conf (webserver/plcapp_management.py) now copies every
regular conf/<name>.* sibling, other than .json (handled above) and
.license (kept on its own path through derive_license_path), to
VPP_DATA_DIR/<name>.<ext>. The containment guard that already protects
dest_config (is_inside_root) applies to every sibling destination.
Discovery stays vpp_plugins.conf-anchored: an orphan sibling with no
matching VPP entry is ignored.

The ABI docstring on plugin_specific_config_file_path in
core/src/drivers/plugin_types.h is updated to document the sibling
convention. The struct layout, field count and offsets are unchanged.

Scope is VPP-only. The built-in plugin path
(plugin_config_model.update_plugins_from_config_dir) is a separate code
path, skips VPP explicitly, and no built-in plugin ships siblings today, so
it is left untouched.

Verification

Unit tests (new tests/pytest/plugins/test_vpp_sibling_delivery.py,
following the test_vpp_license_delivery.py pattern):

• test_station_bin_sibling_lands_in_persistent_dir -- AC1
• test_station_bin_and_license_coexist -- AC2
• test_no_sibling_leaves_persistent_dir_at_the_json -- AC3
• test_orphan_sibling_without_vpp_entry_is_ignored -- AC4
• test_two_vpp_plugins_each_get_only_their_own_siblings
• test_subdirectory_under_conf_is_not_copied
• test_second_upload_updates_sibling_content

pytest tests/pytest/plugins/test_vpp_sibling_delivery.py tests/pytest/plugins/test_vpp_license_delivery.py -> 18 passed. The .license regression suite (test_vpp_license_delivery.py) is untouched and keeps passing.

Full pytest tests/pytest (excluding opcua/modbus suites blocked by
pre-existing dev-env dep gaps): 241 passed, 3 pre-existing failures in
test_etherdog_manager.py and test_ethercat_upload.py from a permission
issue on a generated c_blocks.h, unrelated to this change.

AC5 (ABI docs) satisfied by the plugin_types.h comment.

Out of scope

• Editor-side VPP packaging. The editor already emits
  conf/<plugin>.station.bin as a carve-out in
  build-vpp-plugin-files.ts; kept as-is.
• The VPP schema or openplc-packages manifests.
• The Weidmuller plugin itself (com.weidmuller.ucontrol/hal/**).
• plugin_config_model.update_plugins_from_config_dir (built-in path).
• Support for a conf/<plugin>/ directory. Flat siblings only.
• Transport-layer changes. Zip ingestion and safe_extract are unchanged.

Attack surface

No attack surface touched. safe_extract and its path-traversal/size/
extension guards are unchanged; the generalization operates only on files
already normalized by safe_extract, and copied siblings are placed on
disk for the plugin to read, never executed. The same
is_inside_root(dest, VPP_DATA_DIR) guard that protects dest_config
covers every sibling destination.

No requirements document for this task: small change, task description is
the record. The task was re-read against what shipped and brought in line
(the plugin_config_model.py bullet dropped, built-in path marked out of
scope); ACs stay satisfied by the test file above.

Contributor License Agreement

• I have read and agree to the Contributor License Agreement, version 1.0

🤖 Generated with Claude Code

[JulioSergioFS]

Review — RTOP-392

Reviewed against the task's acceptance criteria and the real code paths (apply_vpp_plugin_conf, safe_extract, derive_license_path, the pytest workflow). Verdict: approve once the CI wiring is fixed. Everything else is a nit.

Findings

#	Severity	Location	Finding	Confidence
1	required	.github/workflows/tests.yml:98-101 and :117-119	The new spec never runs in CI. The pytest job ignores tests/pytest/plugins wholesale, and the "plugin suites known green" step allowlists only the two opcua files — so test_vpp_sibling_delivery.py (and the test_vpp_license_delivery.py regression it leans on) execute nowhere. AC6 is proven on your machine only, and nothing guards this on the next PR. Add both files to that step.	high
2	nit	webserver/plcapp_management.py:509	os.listdir(conf_dir) sits inside the for p in vpp_conf_plugins.plugins loop, so the directory is re-read once per VPP entry. Hoist it above the loop.	high
3	nit	core/src/drivers/plugin_types.h:257	AC5 asks the ABI doc to state that the directory of plugin_specific_config_file_path is where ride-along files land, so a plugin resolves them with dirname(). The new one-liner dropped the field's own label (/* Plugin configuration */) and says "next to this file" without naming the dirname() contract. Suggest keeping both.	high
4	nit	webserver/plcapp_management.py:512	Anchoring is startswith(f"{p.name}."). A plugin name containing a dot breaks the "vpp_plugins.conf-anchored" claim: with a declared and a.b not, conf/a.b.json is copied as a sibling of a and overwrites a previously installed VPP_DATA_DIR/a.b.json. No current name has a dot, so this is theoretical.	medium
5	nit	webserver/plcapp_management.py:507-530	Stale siblings are not pruned. The task accepts this explicitly, but it is not quite symmetric with the .json: the JSON is rewritten on every upload, whereas a .station.bin removed from the project stays in VPP_DATA_DIR and keeps being deployed. Follow-up ticket rather than a change here.	high

Minor style point: sibling_name == f"{p.name}.json" or sibling_name == f"{p.name}.license" (:516) reads better as a membership test against a set.

Checked, no finding

• ABI untouched — plugin_types.h changes a comment only; layout, field count and offsets identical, so no plugin_runtime_args.py ctypes mirror update is owed.
• Discovery is correctly double-anchored: the sibling loop runs after the conf/<name>.json existence check and after the basename guard on p.name, so AC4's orphan case never reaches the copy.
• No new attack surface: safe_extract writes regular files only (open(out_path, "wb")), never symlinks, and extract_dir is rmtree'd before each extraction, so os.path.isfile cannot be steered at a link target.
• sibling_name comes from os.listdir, so it carries no separator — is_inside_root here is defence in depth, not a live guard, and that is fine.
• The destination directory already exists when the sibling loop runs (os.makedirs on the JSON copy above).
• .license carve-out preserved: derive_license_path(dest_config) resolves to VPP_DATA_DIR/<name>.license and the sibling loop excludes that exact name, so no double copy and no clobber of the migration path.
• Failures are per-file and logged, never fatal: one unreadable sibling does not abort the upload.
• The tests map one-to-one onto the ACs and drive apply_vpp_plugin_conf end to end, as AC6 requires.

CI and validation

Green here does not mean this change was tested — the Webserver (pytest) job does not execute the new file (finding 1). CLA is red on both of your open PRs; that looks like an agreement/bot issue, not this change. Local validation was not run for this review; the findings above come from reading the code and the workflow.

[JulioSergioFS]

Requesting changes on finding 1 only: the new spec does not run in CI (tests.yml ignores tests/pytest/plugins and the known-green allowlist does not include it), so AC6 is unproven on the base branch and unguarded from here on. Add test_vpp_sibling_delivery.py and test_vpp_license_delivery.py to the "pytest (plugin suites known green)" step and this is good to go. The rest of the review is nits.

[marconetsf]

Thanks for the review — the CI catch in particular was a good one.

Addressed in edd9d34

• [RTOP-46] compile c program #1 (CI wiring): test_vpp_license_delivery.py and test_vpp_sibling_delivery.py are now in the "pytest (plugin suites known green)" step. I also added -rs: both files use pytest.importorskip("webserver.plcapp_management"), so a failed import would otherwise skip quietly and the step would still go green. Run 41045232737 shows 44 passed, no skips, with both VPP files executing.
• Fix core compilation #2: os.listdir(conf_dir) now runs once, before the plugin loop.
• Improve code structure #3: the comment keeps the field label and states the contract: /* Plugin configuration; conf/<plugin>.* ride-along files land in dirname() of this path */.
• Style point: the double comparison is now a set membership test (handled_siblings).

Not addressed here

• [RTOP-49] Implement base code to commands #4 (dotted plugin names): I agree the prefix match is not strictly anchored for dotted names, but I'd rather not change it in this PR:
  • The editor explicitly allows dots in plugin_name (PLUGIN_NAME_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$/ in build-vpp-plugin-files.ts). Rejecting them on the runtime side would break that contract.
  • Damage needs a declared a, an undeclared a.b, and a leftover a.b.json in VPP_DATA_DIR. The editor only emits conf/ files for declared plugins, and no current VPP name has a dot.
  • If we want it closed, the minimal fix is to assign each conf/ entry to the longest declared plugin name that prefixes it, plus a test. I can open that as a separate task.
• Task/rtop 57 implement driver contracts #5 (stale siblings): the runtime has never pruned VPP_DATA_DIR. On the base branch, apply_vpp_plugin_conf never deletes anything there: a removed plugin's .json stays, and .license is kept on purpose when an upload doesn't carry one (test_persistent_license_survives_an_upload_without_a_license). This PR extends that same lifecycle to the new siblings, as the task spec states.