[no-release] feat(menu): group Plugins ▸ Matter into five sections (#134) - #144
Merged
Conversation
The menu was sixteen flat items in creation order. "Install/update
matter-server" sat first and "Install/update the Matter export bridge"
twelfth, and the wrong one was clicked live on 2026-08-06 — reinstalling
the inbound controller and bouncing it when the outbound bridge was
meant. They are separately versioned, separately installed npm packages.
Separators ARE available, contrary to the canonical MenuItems.xml
reference, which lists <Name> as required and documents neither
separators nor submenus: an EMPTY, self-closing <MenuItem id="…"/>
renders as one. That is not a hack of ours — Perceptive Automation's own
bundled plugins do it (Timers and Pesters, Timed Devices, Virtual
Devices, Alexa), Actions.xml uses the same form, and fifteen of the
plugins installed on jarvis rely on it.
Five sections, everyday work first and the destructive things fenced off
last: devices Indigo controls · devices Indigo publishes · matter-server
plumbing · export-bridge plumbing · backup and recovery. The two
Install/update items now sit in different sections.
Renamed, because "Export" meant save-to-disk in a menu where it means the
outbound bridge everywhere else:
Export fabric backup… → Back up the Matter fabric…
Restore fabric backup… → Restore a fabric backup…
Callback methods, menu ids and every ConfigUI are unchanged — verified
field-by-field against HEAD, dialogs byte-identical.
The ordering IS the change, so the tests assert the order and the
separator positions, not membership: a set-based check would pass with
all sixteen items back in one flat list. Mutation-verified — deleting the
separators, and merging the two install sections, each fail three tests.
2256 Python tests pass (from 2252), pylint 9.42 unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Records the finding that outlives the change: the canonical MenuItems.xml reference is the authority on what is supported, not on what works — an empty self-closing <MenuItem/> is a separator, evidenced by four bundled Perceptive Automation plugins and 15 installs on jarvis. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe plugin menu is reorganized into five sections with new recovery actions and ordering tests. Documentation uses updated backup terminology. The handover and plugin version records reflect current project status. ChangesMatter menu and documentation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with 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.
Inline comments:
In `@docs/HANDOVER.md`:
- Around line 3-12: Update the top status block in HANDOVER.md to reconcile it
with the 2026.8.5 entry: revise the current branch/release status and
plugin/deployed-version fields so they consistently identify 2026.8.5 as the
current menu-work release, while preserving the existing bridge-node version and
test/deployment details unless they are affected.
In `@docs/INSTALL.md`:
- Around line 99-112: Update the obsolete package-status statements near the
document introduction and the Matter menu description to remove claims that the
Matter export bridge is unpublished or cannot be installed. Replace them with
the current installation flow, referencing the “Install/update the Matter export
bridge” menu item and its supported installation behavior.
In `@indigo-matter.indigoPlugin/Contents/Server` Plugin/MenuItems.xml:
- Around line 13-14: Update the referenced test path in the menu-order
documentation from tests/test_export_menu.py to tests/test_plugin_module.py,
which contains the order and separator-position contract tests.
In `@tests/test_plugin_module.py`:
- Around line 138-139: Update _menu_items to add a targeted Ruff S314
suppression on the ET.parse call, with an inline comment documenting that
MenuItems.xml is a repository-controlled trusted test fixture. Keep the
suppression scoped only to this parsing statement.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7199c0ed-8bee-4ed2-9fa2-12bcd7baab30
📒 Files selected for processing (7)
docs/HANDOVER.mddocs/INSTALL.mddocs/MATTER.mddocs/matter.htmlindigo-matter.indigoPlugin/Contents/Info.plistindigo-matter.indigoPlugin/Contents/Server Plugin/MenuItems.xmltests/test_plugin_module.py
Comment on lines
+138
to
+139
| def _menu_items(): | ||
| return ET.parse(SERVER_PLUGIN / "MenuItems.xml").getroot().findall("MenuItem") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Suppress the false-positive XML security finding.
Line 139 parses a repository-controlled test fixture. Ruff reports S314 on this line, which can fail linting even though no untrusted XML enters this test. Add a targeted suppression and document the trust boundary.
Proposed fix
def _menu_items():
- return ET.parse(SERVER_PLUGIN / "MenuItems.xml").getroot().findall("MenuItem")
+ # MenuItems.xml is a repository-controlled test fixture.
+ return ET.parse(SERVER_PLUGIN / "MenuItems.xml").getroot().findall("MenuItem") # noqa: S314📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _menu_items(): | |
| return ET.parse(SERVER_PLUGIN / "MenuItems.xml").getroot().findall("MenuItem") | |
| def _menu_items(): | |
| # MenuItems.xml is a repository-controlled test fixture. | |
| return ET.parse(SERVER_PLUGIN / "MenuItems.xml").getroot().findall("MenuItem") # noqa: S314 |
🧰 Tools
🪛 Ruff (0.16.1)
[error] 139-139: Using xml to parse untrusted data is known to be vulnerable to XML attacks; use defusedxml equivalents
(S314)
🤖 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 `@tests/test_plugin_module.py` around lines 138 - 139, Update _menu_items to
add a targeted Ruff S314 suppression on the ET.parse call, with an inline
comment documenting that MenuItems.xml is a repository-controlled trusted test
fixture. Keep the suppression scoped only to this parsing statement.
Source: Linters/SAST tools
Three of CodeRabbit's four findings, verified before acting: * The comment in MenuItems.xml pointed at tests/test_export_menu.py for the ordering contract; those tests live in tests/test_plugin_module.py (test_export_menu.py owns the Manage Matter Exports dialog's shape). * INSTALL.md ×2 and README.md ×1 still said indigo-matter-bridge "is not on the npm registry yet, so this half cannot be installed today". `npm view indigo-matter-bridge` reports 0.7.0 as `latest`, and Step E1 is already written as an install you perform — so the claim was false and, after this PR, sat ten lines above a menu table naming the very install item it denied. Called out-of-scope in the PR body; reversed because it is three sentences and provably wrong. * HANDOVER's header described a clean tree on `main` at 2026.8.4 while its own new section described 2026.8.5 on an open branch. Not done: a `noqa: S314` on the new ET.parse. No ruff or bandit config exists in pyproject.toml, and this file already has six unsuppressed ET.parse calls (zero S314 suppressions across tests/) — suppressing a rule nothing runs, on one call out of six, is noise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
simons-plugins
changed the title
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.
Closes #134.
The problem
Sixteen flat menu items in creation order.
Install/update matter-serverwasfirst,
Install/update the Matter export bridgetwelfth — and on 2026-08-06 thewrong one was clicked live, reinstalling the inbound controller and bouncing it
when the outbound bridge was meant. They are separately versioned, separately
installed npm packages.
Separators exist — the docs just don't say so
The canonical
MenuItems.xmlreference lists<Name>as required anddocuments neither separators nor submenus. It is incomplete: an empty,
self-closing
<MenuItem id="…"/>renders as a separator.This is not a hack of ours. Perceptive Automation's own bundled plugins use it
(Timers and Pesters, Timed Devices, Virtual Devices, Alexa),
Actions.xmltakes the same form (<Action id="sep1" uiPath="DeviceActions"/>),and 15 of the plugins installed on jarvis rely on it.
The layout
Everyday work first, destructive things fenced off last:
The two
Install/updateitems are now in different sections, not merelyadjacent.
Renames
Only one pair, because "Export" meant save-to-disk in a menu where it means the
outbound bridge everywhere else:
Export fabric backup…Back up the Matter fabric…Restore fabric backup…Restore a fabric backup…Menu ids and callback methods are unchanged, so no Python moved.
spec.install_menu_namestill resolves to a real<Name>—test_bridge_agent.pyalready pinned that and still passes.
Nothing else changed
Every
ConfigUIwas compared field-by-field againstHEAD— attributes, labels,descriptions, titles, list methods, callbacks. Byte-identical. The only
content edits are the two names above.
Tests
2256 Python (from 2252), pylint 9.42 unchanged. The four new tests assert
the order and the separator positions, not membership — a set-based check
would pass with all sixteen items back in one flat list, which is the state they
exist to prevent.
Mutation-verified: deleting the four separators, and deleting only
sepServerFromBridgeso the two install sections merge, each fail exactly threelayout tests and nothing else.
Docs
INSTALL.mdgains a five-section menu table under Overview (plus a Contentsentry); the renamed items are updated there and in
MATTER.md/matter.html.Deliberately out of scope
Same file, but it is dialog text and a separate bug; combining them would
make this diff unreviewable.
INSTALL.md§Overview still saysthe bridge package "is not on the npm registry yet".
indigo-matter-bridge@0.7.0is published. That belongs with the E8 docs pass (Docs site has no route to the export half (matter.html / index.html) #137/Screenshots wanted for the export walkthrough #138).
Version bumped
2026.8.4→2026.8.5(patch: user-visible, but not a feature).🤖 Generated with Claude Code
Summary by CodeRabbit