-
Notifications
You must be signed in to change notification settings - Fork 2
fix: harden plugin loading and add pack test #14
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.
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
Conversation
Summary of ChangesHello @iHildy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the stability and testability of the "opencode-synced" plugin. It introduces robust error handling for plugin asset loading, improves module export flexibility for broader compatibility, and adds a new local testing workflow that simulates a production installation. These changes collectively aim to make the plugin more resilient and easier to verify before release. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request hardens the plugin loading mechanism, improves module compatibility with additional exports, and introduces a new local packing test script. The changes are generally positive and improve the robustness of the project. My feedback focuses on making the new test script more reliable and ensuring that potential errors during plugin loading are not silently ignored, which will improve debuggability.
| import json, re | ||
| from pathlib import Path | ||
| version = "${VERSION}" | ||
| path = Path("${CACHE_DIR}") / "package.json" | ||
| text = path.read_text() | ||
| deps = dict(re.findall(r'\"([^\\\"]+)\"\\s*:\\s*\"([^\\\"]+)\"', text)) | ||
| deps["opencode-synced"] = version | ||
| path.write_text(json.dumps({"dependencies": deps}, indent=2) + "\\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using regular expressions to parse and modify a JSON file is brittle and can lead to unexpected behavior. This implementation overwrites the entire package.json with only the dependencies field, which would remove other important fields like name, version, or scripts. A much safer and more robust approach is to use Python's built-in json module to load, modify, and then write back the JSON data. This preserves the original structure of the file while updating the necessary dependency.
import json
from pathlib import Path
version = "${VERSION}"
path = Path("${CACHE_DIR}") / "package.json"
try:
data = json.loads(path.read_text())
except json.JSONDecodeError:
data = {}
if "dependencies" not in data:
data["dependencies"] = {}
data["dependencies"]["opencode-synced"] = version
path.write_text(json.dumps(data, indent=2) + "\n")
| frontmatter, | ||
| template: body, | ||
| }); | ||
| } catch {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Swallowing errors with an empty catch block can make debugging very difficult. If a command file fails to load for any reason (e.g., malformed content, read permissions), the error will be silently ignored, and there will be no indication of what went wrong. It is a best practice to at least log the error to provide visibility into such issues.
} catch (error) {
console.error(`[opencode-synced] Failed to load command from ${file}:`, error);
}
Summary
Testing