Conversation
paulr34
requested review from
brdandu,
dhchandw,
ethanzhouyc and
tecimovic
as code owners
July 31, 2026 16:33
paulr34
force-pushed
the
cursor/zap-edit-cli-b5b9
branch
from
July 31, 2026 16:34
f8e9c8f to
c11aa5b
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive command-line interface (zap edit) that allows users to perform configuration edits—such as managing endpoints, device types, clusters, attributes, commands, events, and features—directly from the terminal or via batch scripts. The review feedback highlights a critical bug in cli-operations.js where using e.parentRef instead of e.parentEndpointRef bypasses the safety check when deleting parent endpoints. Additionally, improvements were suggested to allow global options to precede the edit command in the argument parser and to make the package category matching case-insensitive.
| let endpoint = await resolver.resolveEndpoint(ctx, params.endpoint) | ||
| let all = await queryEndpoint.selectAllEndpoints(ctx.db, ctx.sessionId) | ||
|
|
||
| let children = all.filter((e) => e.parentRef === endpoint.id) |
Contributor
There was a problem hiding this comment.
The endpoint object e returned by selectAllEndpoints has its parent endpoint reference mapped to parentEndpointRef (corresponding to the database column PARENT_ENDPOINT_REF), not parentRef. Using e.parentRef will evaluate to undefined, causing the children check to always be empty and allowing deletion of parent endpoints without --force (which silently orphans children).
Please update this to use e.parentEndpointRef instead.
Suggested change
| let children = all.filter((e) => e.parentRef === endpoint.id) | |
| let children = all.filter((e) => e.parentEndpointRef === endpoint.id) |
| for (let i = 0; i < argv.length; i++) { | ||
| let arg = argv[i] | ||
| if (typeof arg !== 'string') continue | ||
| if (arg.startsWith('-')) return -1 |
Contributor
There was a problem hiding this comment.
Returning -1 immediately when an argument starting with - is encountered prevents users from placing global options (such as --logToStdout or --stateDirectory) before the edit command.
Consider skipping option flags and their values or using a more flexible detection mechanism to allow global options to precede the edit subcommand.
Comment on lines
+165
to
+168
| let filtered = pkgs.filter( | ||
| (p) => | ||
| p.category === wanted || p.type === dbEnum.packageType.zclXmlStandalone | ||
| ) |
Contributor
There was a problem hiding this comment.
The category comparison p.category === wanted is case-sensitive. Since users might pass mixed-case category names (e.g., --category Zigbee or --category Matter), it is more robust to perform a case-insensitive comparison and guard against null/undefined categories.
let wantedLower = wanted.toLowerCase()
let filtered = pkgs.filter(
(p) =>
(p.category && p.category.toLowerCase() === wantedLower) ||
p.type === dbEnum.packageType.zclXmlStandalone
)
cursor
Bot
force-pushed
the
cursor/zap-edit-cli-b5b9
branch
4 times, most recently
from
August 1, 2026 13:22
830a5fd to
da463ff
Compare
paulr34
force-pushed
the
cursor/zap-edit-cli-b5b9
branch
from
August 1, 2026 14:53
80d143c to
9b25dff
Compare
cursor
Bot
force-pushed
the
cursor/zap-edit-cli-b5b9
branch
from
August 1, 2026 16:02
dec864f to
23c8cb2
Compare
paulr34
force-pushed
the
cursor/zap-edit-cli-b5b9
branch
2 times, most recently
from
August 1, 2026 16:16
139bb55 to
dc5bed3
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1725 +/- ##
==========================================
+ Coverage 75.07% 77.12% +2.04%
==========================================
Files 204 219 +15
Lines 23892 26552 +2660
Branches 5469 6243 +774
==========================================
+ Hits 17938 20477 +2539
- Misses 5954 6075 +121 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
paulr34
force-pushed
the
cursor/zap-edit-cli-b5b9
branch
3 times, most recently
from
August 3, 2026 15:27
311d66e to
aa06ef3
Compare
brdandu
reviewed
Aug 3, 2026
paulr34
force-pushed
the
cursor/zap-edit-cli-b5b9
branch
2 times, most recently
from
August 3, 2026 20:11
99d9de1 to
cd4755f
Compare
paulr34
force-pushed
the
cursor/zap-edit-cli-b5b9
branch
4 times, most recently
from
August 6, 2026 20:09
6daea04 to
54f162f
Compare
|
|
paulr34
force-pushed
the
cursor/zap-edit-cli-b5b9
branch
3 times, most recently
from
September 9, 2026 17:59
491525a to
fac54ab
Compare
Provides a discoverable CLI to create and edit .zap files: endpoints, clusters, attributes, commands, events, and Matter features, with ranked lookup suggestions, multiprotocol support, GUI-aligned guardrails, and generation/check coverage. Shares Zigbee cluster state across endpoints the same way the UI does (via util/shared-cluster-state.js), with protocol rules driven by the data model rather than protocol name checks. Ships zap-cli with bundled edit modules and test generation defaults, short --zcl/--gen names, release smoke coverage, and a QA quick-start. When --studioHttpPort and --ideProjectPath are both given, cluster enable/disable and endpoint create ask Studio to install or remove the matching UC components, matching the GUI path. Includes a SKILL.md so agents use zap-cli (with SDK-provided --zcl) instead of rewriting .zap JSON by hand. Includes architecture docs, layering tests, and the step-by-step edit guide.
Adds what an agent could not work out from the command surface alone: where zap-cli and the .zap files live, that clusters need their UC component installed and the project regenerated afterwards, that batching through apply avoids paying the metadata load per command, and that concurrent jobs need their own state directory. Corrects two things. --strict also applies to edits, where it refuses to save when that edit introduced errors, and --gen is not only for code generation: the cluster-to-component mapping is a gen-templates package extension, so component integration needs it too.
Configuring a cluster is half of what the GUI checkbox does; installing the component that implements it is the other half, and only Studio can write the project file. Two ways of getting that wrong were silent. Asking for Studio integration when nothing is answering saved the edit, installed nothing and looked like success, because failures were reported one 404 per component after the fact. The port is now checked once, up front, against the same endpoint isProjectActive uses, and a request that cannot be served is refused. Without Studio the cluster-to-component mapping is still known locally, since it is a package extension the generation templates carry. It was being thrown away. An enable now names the components it could not install, so a headless caller can hand them to slc instead of discovering later that nothing implements the cluster it configured. Also guards isProjectActive against a missing response, which threw a TypeError rather than returning false when nothing was listening.
The test copied a resource .zap and passed the generation templates that carry the cluster-to-component mapping. A resource names its own templates, and fuzzy package matching prefers those, so the session only used the templates the test asked for when the state database happened to resolve the file's reference to them by file name. It did on a machine that had run the suite before, and did not on a fresh one, which is every build agent. Starting from an empty configuration means the file names the templates the test loaded, so what is under test no longer depends on what the database has already seen.
Omitting them can rewrite Matter .zap packages to Zigbee test templates and leave slc generate without Matter zap-generated output.
Strict package matching stops new rewrites but does not fix a .zap that already points at Zigbee test gen-templates.
So a bare zap-cli edit against an SLC project uses the same SDK packages Studio and generate already inject from apack properties, instead of the bundled test templates that corrupt Matter .zap files on save.
Studio 6's Jetty 12 rejects %2F in a path segment, so encodeURIComponent of a filesystem .slcp never reaches the UC servlets. Match the GUI's %→_ mangling and document the Studio-vs-slc component install routes in the skill.
paulr34
force-pushed
the
cursor/zap-edit-cli-b5b9
branch
from
September 17, 2026 13:19
fac54ab to
31a8f57
Compare
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
4 participants
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.
No description provided.