fix: improve startup reliability, repo validation, and toasts#4
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 significantly improves the application's startup reliability and user experience by making the initial synchronization process more robust. It ensures that startup tasks are deferred until the UI is ready, validates repository configurations upfront, and provides clearer, non-blocking feedback through enhanced toast notifications when issues occur. These changes contribute to a more stable and informative application launch. 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
|
![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 significantly improves the startup reliability and user experience of the plugin. The changes to defer startup tasks, validate repository configuration, and make toast notifications more resilient are all excellent additions. My review focuses on further enhancing this reliability. I've identified a potential race condition with the use of setTimeout and suggest exploring a more deterministic, event-based approach for startup synchronization. Additionally, I've provided suggestions to make error messages more specific for better usability and to improve debuggability by logging errors that are intentionally caught and suppressed.
| setTimeout(() => { | ||
| void service.startupSync(); | ||
| }, 1000); |
There was a problem hiding this comment.
Using a fixed setTimeout to delay an operation is often a sign of a race condition and can be unreliable. On a slow system or under heavy load, the TUI might take longer than 1 second to connect, causing startupSync to fail. Conversely, on a fast system, this introduces an unnecessary delay.
A more robust solution would be to use an event-driven approach. Please investigate if the plugin's context or lifecycle provides a mechanism to know when the TUI is ready, such as an onReady callback or an event listener. This would eliminate the guesswork and make the startup process more reliable.
There was a problem hiding this comment.
Addressed in 11d417b. Replaced the fixed setTimeout with an event-driven approach using the server.connected event listener. Added a guard flag and a fallback timer to ensure reliable, single-execution startup.
| if (!config.repo || (!config.repo.url && (!config.repo.owner || !config.repo.name))) { | ||
| await showToast(ctx, 'Run /sync-init to set up config sync.', 'info'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The validation checks for a missing configuration file and a malformed repository configuration both display the same generic toast message. For a better user experience, consider providing a more specific message for this case to inform the user precisely what is wrong with their configuration.
await showToast(ctx, 'Repository details are missing or incomplete in config. Run /sync-init to fix.', 'info');
return;There was a problem hiding this comment.
Fixed in 11d417b. Added a specific error message for malformed repository configurations to provide better guidance to the user.
| } catch { | ||
| // Toast failed - TUI might not be connected yet | ||
| } |
There was a problem hiding this comment.
This empty catch block swallows errors, which can make debugging difficult if toasts fail for reasons other than the TUI not being connected. While it's correct not to block the startup process for a failed toast, logging the error to the console would be beneficial for development and troubleshooting without impacting the user.
} catch (error) {
// Toast failed - TUI might not be connected yet
console.error('[opencode-synced] Failed to show toast:', error);
}There was a problem hiding this comment.
Done in 11d417b. Added console error logging to the catch block to improve debuggability while maintaining non-blocking behavior.
- Replace fixed setTimeout with event-driven 'server.connected' listener - Add guard flag to ensure startupSync runs exactly once - Provide specific toast message for malformed repo config - Log showToast errors to console for better debuggability
iHildy
changed the title
1 participant
Summary
Improves startup reliability by deferring startup tasks until the UI is ready, validating repository configuration before syncing, and enhancing toast notifications for resilience.
Changes
src/index.tsby 1 second to ensure the TUI is connectedrunStartupto determine if the local repo is cloned; if not cloned, verify repo exists on GitHub before attempting to cloneshowToastto support an optional title and handle UI toast failures gracefully