Feature/plugin dev sdk - #2
Conversation
Provide offline scaffolding, validation, packaging, local install helpers, and a local LauncherClient so plugin authors can develop without hand-editing manifests or manually copying files.
Remove accidental bytecode artifacts from the repository so the plugin SDK working tree stays clean.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an offline plugin SDK and CLI tools for Bloret Launcher, enabling users to scaffold, validate, inspect, build, and install plugins, as well as a LauncherClient for local HTTP API communication. The review feedback suggests three key robustness improvements: handling existing directories as build output paths in build_plugin, safely normalizing the permissions field in normalize_manifest to prevent type errors, and ensuring scaffold_plugin correctly handles file paths as destinations without raising a NotADirectoryError.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| LOGGER.error("Plugin build refused due to %d validation error(s)", len(errors)) | ||
| raise ValidationError(errors) | ||
| manifest = load_manifest(root) | ||
| archive = Path(output) if output is not None else root.parent / "{}-{}.zip".format(manifest.get("id", root.name), manifest.get("version", "dev") or "dev") |
There was a problem hiding this comment.
If the output argument is an existing directory, build_plugin will attempt to open that directory as a zip file (e.g., zipfile.ZipFile("dist", "w")), which will raise an IsADirectoryError or PermissionError. Since the README explicitly suggests BLAPI plugin build my-plugin -o dist as a standard usage, we should handle the case where output is a directory by appending the default zip filename inside it.
| archive = Path(output) if output is not None else root.parent / "{}-{}.zip".format(manifest.get("id", root.name), manifest.get("version", "dev") or "dev") | |
| default_filename = "{}-{}.zip".format(manifest.get("id", root.name), manifest.get("version", "dev") or "dev") | |
| if output is not None: | |
| out_path = Path(output) | |
| archive = out_path / default_filename if out_path.is_dir() else out_path | |
| else: | |
| archive = root.parent / default_filename |
| entry["process"] = str(raw_entry.get("process") or "") | ||
| entry["qml_page"] = str(raw_entry.get("qml_page") or raw_entry.get("qml") or "") | ||
| result["entry"] = entry | ||
| result["permissions"] = sorted(set(result.get("permissions") or [])) |
There was a problem hiding this comment.
If permissions in the manifest is not a list (for example, if it is an integer or a string), normalize_manifest will either crash with a TypeError or split the string into individual characters. We should safely normalize permissions only if it is a list, set, or tuple, and default to an empty list otherwise.
| result["permissions"] = sorted(set(result.get("permissions") or [])) | |
| raw_permissions = result.get("permissions") | |
| if isinstance(raw_permissions, (list, set, tuple)): | |
| result["permissions"] = sorted(set(p for p in raw_permissions if isinstance(p, str))) | |
| else: | |
| result["permissions"] = [] |
| if target.exists() and any(target.iterdir()) and not force: | ||
| raise FileExistsError("destination is not empty: {!s}".format(target)) |
There was a problem hiding this comment.
If destination exists but is a file, calling any(target.iterdir()) will raise a NotADirectoryError instead of a FileExistsError. We should check if target is a directory before calling iterdir(), or raise FileExistsError if it is a file.
| if target.exists() and any(target.iterdir()) and not force: | |
| raise FileExistsError("destination is not empty: {!s}".format(target)) | |
| if target.exists() and not force: | |
| if not target.is_dir() or any(target.iterdir()): | |
| raise FileExistsError("destination already exists or is not empty: {!s}".format(target)) |
1 participant
No description provided.