Retire the persistentgroups plugin - #35
Merged
Merged
Conversation
Core now does what it existed to fake, and does it without the copying. A group holds a snapin grant of its own (fogproject #1604, #1611, #1612), so a host added to a group later is covered by it and a host removed stops being covered -- which is the thing this plugin could never manage, because it acted once, at the moment of joining, and left rows behind. The plugin was already substantially non-functional on 1.6 data. Its printer and module copies carry no ON DUPLICATE KEY UPDATE, so a collision raised a duplicate-key error inside an AFTER INSERT trigger and rolled back the groupMembers row that fired it: the host was not added to the group at all. DELETING THIS DIRECTORY DOES NOT REMOVE THE PLUGIN FROM A SERVER, and that is the whole reason the retirement is a schema step rather than a file removal. The work is a database TRIGGER, which outlives the files completely -- sites that deleted this plugin years ago have had it firing ever since, copying 13 hosts columns including hostADPass onto member hosts, below the PHP layer and outside the audit trail. fogproject schema step 402 drops the trigger and removes the registration row; this commit only stops shipping the source that would re-create it. Removed alongside the directory: the README row, and a comment in tests/plugin-layout.test.php that named this plugin as the example of one with models but no pages. capone still carries the other half of that comment's point, so the check itself is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Core now does what this plugin existed to fake, and does it without the copying. A group holds a snapin grant of its own (fogproject #1604, #1611, #1612), so a host added to a group later is covered by it and a host removed stops being covered — the thing this plugin could never manage, because it acted once, at the moment of joining, and left rows behind.
Deleting this directory does not remove the plugin from a server
That is the whole reason the retirement is a schema step in fogproject rather than a file removal here. The work is a database TRIGGER, which outlives the files completely: sites that deleted this plugin years ago have had it firing ever since, copying 13
hostscolumns —hostADPassamong them — onto member hosts, below the PHP layer and outside the audit trail.fogproject schema step 402 drops the trigger and removes the registration row. This commit only stops shipping the source that would re-create it.
It was already substantially broken on 1.6 data
The printer and module copies carry no
ON DUPLICATE KEY UPDATE, so a collision raised a duplicate-key error inside anAFTER INSERTtrigger and rolled back thegroupMembersrow that fired it. The host was not added to the group at all. Anyone who has seen group additions fail with a database error was seeing this.Also removed
The README row, and a comment in
tests/plugin-layout.test.phpnaming this plugin as the example of one with models but no pages.caponestill carries the other half of that comment's point, so the check itself is unchanged.tests/run-all.sh: 15 passed, 0 failed.Background: ADR 0038 decisions 14 and 15.
🤖 Generated with Claude Code