Skip to content

fix: match autostart membership to what the UI renders #54

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chodeus merged 3 commits into from
Aug 9, 2026
+38 −2
Merged

fix: match autostart membership to what the UI renders #54

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5ccd0b2
fix: match autostart membership to what the UI renders
chodeus Aug 9, 2026
2023d6c
fix: fail closed on label read, and type-check the regex
chodeus Aug 9, 2026
36ceaeb
Merge branch 'beta' into fix/autostart-membership-matches-ui
chodeus Aug 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 38 additions & 2 deletions src/folder.view3/usr/local/emhttp/plugins/folder.view3/server/lib.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,24 +314,60 @@ function syncContainerOrder(string $type): void {
return;
}

// `folder.view3: <name>` label claims, keyed by container name. getDockerContainers()
// carries no Labels, so read them from the same raw endpoint readInfo() uses.
$ctLabels = [];
$rawCts = $dockerClient->getDockerJSON("/containers/json?all=1");
// Fail closed. A failed or partial read yields no label claims, and the order below would
// then write label-assigned containers back out as unassigned — the same hazard $ctListComplete
// guards against above, so abort rather than fall through to the permissive path.
if (!is_array($rawCts) || count($rawCts) < count($allContainerNames)) {
fv3_debug_log("syncContainerOrder: label read unavailable or incomplete, aborting before write");
return;
}
foreach ($rawCts as $rc) {
$rcName = is_array($rc) ? ltrim($rc['Names'][0] ?? '', '/') : '';
if ($rcName === '') {
fv3_debug_log("syncContainerOrder: unnamed container in label read, aborting before write");
return;
}
$rcLabel = $rc['Labels']['folder.view3'] ?? '';
if (is_string($rcLabel) && $rcLabel !== '') { $ctLabels[$rcName] = $rcLabel; }
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
$folderNameSet = [];
foreach ($folders as $folder) {
if (isset($folder['name'])) { $folderNameSet[$folder['name']] = true; }
}

$folderContainers = [];
$folderNames = [];
$assignedContainers = [];
// Explicit members of any folder beat regex matches elsewhere (issue #46)
// Explicit members and label claims of any folder beat regex matches elsewhere (issue #46)
$explicitAssigned = [];
foreach ($folders as $folder) {
$explicitAssigned = array_merge($explicitAssigned, $folder['containers'] ?? []);
}
foreach ($ctLabels as $ctName => $ctLabel) {
if (isset($folderNameSet[$ctLabel])) { $explicitAssigned[] = $ctName; }
}
Comment on lines 346 to +352

@coderabbitai coderabbitai Bot Aug 9, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve explicit membership over label membership.

$explicitAssigned contains both explicit members and label claims. The label loop at Lines 356-358 does not distinguish them. The later $assignedContainers filter therefore makes the first folder in $folders win.

If container c is explicitly assigned to folder A but its label names folder B, the label wins when B appears before A. Keep a separate explicit-members set and skip label assignment for names in that set.

Proposed fix 
-        $explicitAssigned = [];
+        $explicitMembers = [];
        foreach ($folders as $folder) {
-            $explicitAssigned = array_merge($explicitAssigned, $folder['containers'] ?? []);
+            $explicitMembers = array_merge($explicitMembers, $folder['containers'] ?? []);
        }
+        $explicitAssigned = $explicitMembers;
        foreach ($ctLabels as $ctName => $ctLabel) {
-            if (isset($folderNameSet[$ctLabel])) { $explicitAssigned[] = $ctName; }
+            if (isset($folderNameSet[$ctLabel]) && !in_array($ctName, $explicitMembers, true)) {
+                $explicitAssigned[] = $ctName;
+            }
        }

-                if ($ctLabel === ($folder['name'] ?? null) && !in_array($ctName, $members)) {
+                if ($ctLabel === ($folder['name'] ?? null)
+                    && !in_array($ctName, $members, true)
+                    && !in_array($ctName, $explicitMembers, true)) {

Also applies to: 356-358

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/folder.view3/usr/local/emhttp/plugins/folder.view3/server/lib.php` around
lines 337 - 343, Update the folder-assignment setup around $explicitAssigned to
maintain a separate set of containers explicitly listed in
$folder['containers']; while collecting label claims from $ctLabels, skip any
container present in that explicit-members set. Keep $explicitAssigned available
for the later filtering, so explicit folder membership always takes precedence
over label-based membership regardless of $folders order.

@chodeus chodeus Aug 9, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Declining this one for the scope of this PR, and tracking it separately.

The conflict is real and reachable — folder.js:271-276 keeps containers labelled for other folders in choose, where they render as plain enabled checkboxes with nothing indicating they are already claimed, so ticking one into a second folder takes two clicks. Containers labelled for the folder being edited are correctly checked disabled and excluded from the save, but that protection does not extend across folders.

The reason not to fix it here: docker.js is order-dependent in exactly the same way. Label matching at docker.js:305 only tests !combinedContainers.includes(el), so whichever folder renders first takes the container — there is no explicit-wins rule on the client to mirror. This PR exists to remove divergence between the two membership calculations, and making the PHP deterministic while the JS stays arbitrary would introduce a new one.

Worth noting the two also iterate differently — PHP walks $folders in docker.json key order, the client renders in display order — so for any contested container they can already disagree, regardless of this change.

Explicit membership beating a label is the better rule; it just needs applying to both sides in one change. Filed separately.

@coderabbitai coderabbitai Bot Aug 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@chodeus, acknowledged. The finding remains valid, but the proposed PHP-only change would break the current client/server parity that this PR restores.

The follow-up should define the same explicit-membership precedence in both docker.js and syncContainerOrder(). It should also align the folder iteration order or make contested membership deterministic.

🐇

✏️ Learnings added 
Learnt from: chodeus
Repo: chodeus/folder.view3 PR: 54
File: src/folder.view3/usr/local/emhttp/plugins/folder.view3/server/lib.php:346-352
Timestamp: 2026-08-09T13:15:00.281Z
Learning: In Folder View 3 container folder membership, `src/folder.view3/usr/local/emhttp/plugins/folder.view3/docker.js` currently resolves conflicting label and explicit assignments by folder iteration order through `!combinedContainers.includes(el)`. `src/folder.view3/usr/local/emhttp/plugins/folder.view3/server/lib.php` `syncContainerOrder()` also resolves these conflicts by folder iteration order, but PHP uses `docker.json` key order and the client uses display order. A future change must implement explicit-membership precedence and deterministic matching on both client and server together.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

foreach ($folders as $folderId => $folder) {
$members = $folder['containers'] ?? [];
if (!empty($folder['regex'])) {
// is_string + trim, not empty(): empty("0") is true in PHP, so a regex of "0" was
// silently dropped while docker.js applied it. The type check mirrors docker.js and
// keeps a non-string regex from fataling trim() (TypeError on PHP 8).
if (is_string($folder['regex'] ?? null) && trim($folder['regex']) !== '') {
$regex = '/' . str_replace('/', '\/', $folder['regex']) . '/';
foreach ($allContainerNames as $name) {
if (@preg_match($regex, $name) && !in_array($name, $members) && !in_array($name, $explicitAssigned)) {
$members[] = $name;
}
}
}
foreach ($ctLabels as $ctName => $ctLabel) {
if ($ctLabel === ($folder['name'] ?? null) && !in_array($ctName, $members)) {
$members[] = $ctName;
}
}
$members = array_values(array_filter($members, function($m) use ($allContainerNames, $assignedContainers) {
return in_array($m, $allContainerNames) && !in_array($m, $assignedContainers);
}));
Expand Down