Merged
Conversation
… bar if the client version is out of date
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughRemoves developerMode usage from PluginManager and loads sideloaded plugins unconditionally. Updates PluginManager constructor and corresponding tests. Adds MicrobotVersionChecker class and integrates it into MicrobotPlugin to periodically check a remote version and update the window title. Adds startup/shutdown hooks for the checker. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant MP as MicrobotPlugin
participant MVC as MicrobotVersionChecker
participant SE as ScheduledExecutor
participant Remote as Version Endpoint
participant EDT as Swing EDT
participant UI as Window Title
MP->>MVC: checkForUpdate()
MVC->>SE: scheduleAtFixedRate(every 10 min)
loop Every 10 minutes
SE->>MVC: run() if not already running
MVC->>Remote: GET current-version.txt
alt Newer version available
MVC-->>MVC: set flag to avoid duplicate title
MVC-->>EDT: invokeLater(update title)
EDT->>UI: append "(NEW CLIENT AVAILABLE)"
MVC-->>MP: log info
else Up-to-date
MVC-->>MP: log debug
end
end
MP->>MVC: shutdown()
MVC->>SE: shutdownNow()
MVC-->>MVC: reset running flag
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Contributor
Author
|
@CodeRabbit review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotVersionChecker.java (2)
71-88: Guard against null frame/title when updating the UI and reuse a constant marker.
ClientUI.getFrame()orgetTitle()could be null. Also, centralize the marker string.-private void notifyNewVersionAvailable(String remoteVersion, String localVersion) +private void notifyNewVersionAvailable(String remoteVersion, String localVersion) { SwingUtilities.invokeLater(() -> { try { - String oldTitle = ClientUI.getFrame().getTitle(); - if (!oldTitle.contains("(NEW CLIENT AVAILABLE)")) + var frame = ClientUI.getFrame(); + if (frame == null) + { + log.debug("Client frame not yet available; will retry later."); + return; + } + String oldTitle = String.valueOf(frame.getTitle()); + if (!oldTitle.contains(NEW_CLIENT_MARKER)) { log.info("New Microbot client version available: {} (current: {})", remoteVersion, localVersion); - ClientUI.getFrame().setTitle(oldTitle + " (NEW CLIENT AVAILABLE)"); + frame.setTitle(oldTitle + " " + NEW_CLIENT_MARKER); } } catch (Exception e) { log.warn("Failed to update client title", e); } }); }Add constant if not added in previous suggestion:
private static final String NEW_CLIENT_MARKER = "(NEW CLIENT AVAILABLE)";
95-98: Ensure proper cancellation and reset state on shutdown.Calling
shutdownNow()is fine, but cancel the scheduled task explicitly and clear the idempotent guard.public void shutdown() { - scheduledExecutorService.shutdownNow(); - newVersionAvailable.set(false); + try { + if (future != null) { + future.cancel(true); + } + } finally { + scheduledExecutorService.shutdownNow(); + newVersionAvailable.set(false); + scheduled.set(false); + } }runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java (1)
131-132: Kickoff timing: consider minor delay to ensure UI is ready.If the initial version check happens before the frame is initialized, the checker’s first UI update can fail. With the checker fix in place (retry on subsequent ticks) this is mitigated. Alternatively, schedule the first check with a small delay here.
- microbotVersionChecker.checkForUpdate(); + microbotVersionChecker.checkForUpdate(); // initial scheduling handled inside; retries covered by checker
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
runelite-client/src/main/java/net/runelite/client/plugins/PluginManager.java(0 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java(3 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotVersionChecker.java(1 hunks)runelite-client/src/test/java/net/runelite/client/plugins/PluginManagerTest.java(2 hunks)
💤 Files with no reviewable changes (1)
- runelite-client/src/main/java/net/runelite/client/plugins/PluginManager.java
🔇 Additional comments (3)
runelite-client/src/test/java/net/runelite/client/plugins/PluginManagerTest.java (1)
142-142: PluginManager constructor change verified, tests remain hermetic
- Ran a workspace-wide search for
new PluginManager(and confirmed only two callsites inPluginManagerTest.java(lines 142 and 164), both updated to the 5-arg signature. No lingering 6-arg invocations were found.- Inspected
PluginManager’s VisibleForTesting constructor (5 parameters: safeMode, EventBus, Scheduler, ConfigManager, GameEventManager provider); it only assigns fields and does not trigger filesystem scanning on instantiation.- Tests invoke
loadCorePlugins(), which loads only from the application’s classpath and never touches theSIDELOADED_PLUGINSdirectory—so passingnulldependencies cannot cause nondeterministic sideloaded-JAR pickup.No further action required.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java (2)
114-116: DI wiring for MicrobotVersionChecker looks good.Singleton injection is appropriate for a periodic checker tied to plugin lifecycle.
203-204: Shutdown hook is correct.Cleanly stopping the scheduler on plugin shutdown prevents thread leaks.
Comment on lines
24
to
33
| private final ScheduledExecutorService scheduledExecutorService = Executors.newSingleThreadScheduledExecutor(new ThreadFactory() | ||
| { | ||
| @Override | ||
| public Thread newThread(@NotNull Runnable r) | ||
| { | ||
| Thread t = new Thread(r); | ||
| t.setName(this.getClass().getSimpleName()); | ||
| return t; | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
ThreadFactory sets an empty thread name and creates non‑daemon threads.
this.getClass().getSimpleName()inside an anonymous ThreadFactory returns an empty string, so the thread name is blank.- The thread is non‑daemon, which can prolong shutdown if
shutdown()isn’t called.
Apply:
- private final ScheduledExecutorService scheduledExecutorService = Executors.newSingleThreadScheduledExecutor(new ThreadFactory()
- {
- @Override
- public Thread newThread(@NotNull Runnable r)
- {
- Thread t = new Thread(r);
- t.setName(this.getClass().getSimpleName());
- return t;
- }
- });
+ private final ScheduledExecutorService scheduledExecutorService =
+ Executors.newSingleThreadScheduledExecutor(r -> {
+ Thread t = new Thread(r, MicrobotVersionChecker.class.getSimpleName() + "-scheduler");
+ t.setDaemon(true);
+ t.setUncaughtExceptionHandler((th, e) -> log.warn("Version checker thread error", e));
+ return t;
+ });📝 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
| private final ScheduledExecutorService scheduledExecutorService = Executors.newSingleThreadScheduledExecutor(new ThreadFactory() | |
| { | |
| @Override | |
| public Thread newThread(@NotNull Runnable r) | |
| { | |
| Thread t = new Thread(r); | |
| t.setName(this.getClass().getSimpleName()); | |
| return t; | |
| } | |
| }); | |
| private final ScheduledExecutorService scheduledExecutorService = | |
| Executors.newSingleThreadScheduledExecutor(r -> { | |
| Thread t = new Thread(r, MicrobotVersionChecker.class.getSimpleName() + "-scheduler"); | |
| t.setDaemon(true); | |
| t.setUncaughtExceptionHandler((th, e) -> log.warn("Version checker thread error", e)); | |
| return t; | |
| }); |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotVersionChecker.java
around lines 24 to 33, the anonymous ThreadFactory sets the thread name to
this.getClass().getSimpleName() (which is empty for an anonymous class) and
creates non-daemon threads; change the factory to assign a meaningful name (e.g.
MicrobotVersionChecker.class.getSimpleName() + "-thread" or
"MicrobotVersionChecker-%d") and call t.setDaemon(true) before returning the
thread so the executor threads are identifiable and won’t block JVM shutdown.
Comment on lines
35
to
41
| private void runVersionCheck() | ||
| { | ||
| if (newVersionAvailable.get()) | ||
| { | ||
| return; | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Bug: Once a new version is detected, future UI updates may never occur if the first notify fails.
If the initial notifyNewVersionAvailable runs before the frame is ready (or throws), newVersionAvailable stays true and runVersionCheck() returns early forever, so the title is never updated.
Refactor to retry the announcement while newVersionAvailable is true:
private void runVersionCheck()
{
- if (newVersionAvailable.get())
- {
- return;
- }
+ if (newVersionAvailable.get())
+ {
+ // We already know an update exists; ensure the title gets tagged even if an earlier attempt failed.
+ ensureTitleMarked();
+ return;
+ }Add helper:
+ private static final String NEW_CLIENT_MARKER = "(NEW CLIENT AVAILABLE)";
+
+ private void ensureTitleMarked()
+ {
+ SwingUtilities.invokeLater(() -> {
+ try {
+ var frame = ClientUI.getFrame();
+ if (frame == null) {
+ return;
+ }
+ String oldTitle = String.valueOf(frame.getTitle());
+ if (!oldTitle.contains(NEW_CLIENT_MARKER)) {
+ frame.setTitle(oldTitle + " " + NEW_CLIENT_MARKER);
+ }
+ } catch (Exception e) {
+ log.debug("Retrying to update client title failed", e);
+ }
+ });
+ }
Comment on lines
42
to
55
| try | ||
| { | ||
| String remoteVersion = fetchRemoteVersion(); | ||
| String localVersion = RuneLiteProperties.getMicrobotVersion(); | ||
| if (remoteVersion != null && !remoteVersion.trim().equals(localVersion)) | ||
| { | ||
| newVersionAvailable.set(true); | ||
| notifyNewVersionAvailable(remoteVersion, localVersion); | ||
| } | ||
| else | ||
| { | ||
| log.debug("Microbot client is up to date: {}", localVersion); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden version comparison to avoid false positives on blank/whitespace responses.
A blank non‑null response will currently trigger an “update available.” Normalize both versions and require a non‑empty remote version.
- String remoteVersion = fetchRemoteVersion();
- String localVersion = RuneLiteProperties.getMicrobotVersion();
- if (remoteVersion != null && !remoteVersion.trim().equals(localVersion))
+ String remoteVersion = fetchRemoteVersion();
+ String localVersion = RuneLiteProperties.getMicrobotVersion();
+ String remote = remoteVersion == null ? null : remoteVersion.trim();
+ String local = localVersion == null ? "" : localVersion.trim();
+ if (remote != null && !remote.isEmpty() && !remote.equals(local))
{
newVersionAvailable.set(true);
- notifyNewVersionAvailable(remoteVersion, localVersion);
+ notifyNewVersionAvailable(remote, local);
}
else
{
- log.debug("Microbot client is up to date: {}", localVersion);
+ log.debug("Microbot client is up to date: {}", local);
}📝 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
| try | |
| { | |
| String remoteVersion = fetchRemoteVersion(); | |
| String localVersion = RuneLiteProperties.getMicrobotVersion(); | |
| if (remoteVersion != null && !remoteVersion.trim().equals(localVersion)) | |
| { | |
| newVersionAvailable.set(true); | |
| notifyNewVersionAvailable(remoteVersion, localVersion); | |
| } | |
| else | |
| { | |
| log.debug("Microbot client is up to date: {}", localVersion); | |
| } | |
| } | |
| try | |
| { | |
| String remoteVersion = fetchRemoteVersion(); | |
| String localVersion = RuneLiteProperties.getMicrobotVersion(); | |
| String remote = remoteVersion == null ? null : remoteVersion.trim(); | |
| String local = localVersion == null ? "" : localVersion.trim(); | |
| if (remote != null && !remote.isEmpty() && !remote.equals(local)) | |
| { | |
| newVersionAvailable.set(true); | |
| notifyNewVersionAvailable(remote, local); | |
| } | |
| else | |
| { | |
| log.debug("Microbot client is up to date: {}", local); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotVersionChecker.java
around lines 42 to 55, the code treats a blank (whitespace) remoteVersion as a
valid update and can produce false positives; normalize both versions by
trimming (or otherwise normalizing) and require the remote version to be
non-null and non-empty after trimming before considering it an update, then
compare the normalized remote and local versions for inequality; only set
newVersionAvailable and call notifyNewVersionAvailable when remoteNormalized is
not empty and differs from localNormalized, otherwise log that the client is up
to date with the normalized local version.
Comment on lines
62
to
69
| private String fetchRemoteVersion() throws Exception | ||
| { | ||
| URL url = new URL(REMOTE_VERSION_URL); | ||
| try (BufferedReader in = new BufferedReader(new InputStreamReader(url.openStream()))) | ||
| { | ||
| return in.readLine(); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Network call lacks timeouts and status handling; URL#openStream can hang indefinitely.
Use HttpURLConnection with connect/read timeouts and basic status checks to avoid hanging the scheduler.
-private String fetchRemoteVersion() throws Exception
-{
- URL url = new URL(REMOTE_VERSION_URL);
- try (BufferedReader in = new BufferedReader(new InputStreamReader(url.openStream())))
- {
- return in.readLine();
- }
-}
+private String fetchRemoteVersion() throws Exception
+{
+ var url = new URL(REMOTE_VERSION_URL);
+ var conn = (java.net.HttpURLConnection) url.openConnection();
+ conn.setConnectTimeout(5_000);
+ conn.setReadTimeout(5_000);
+ conn.setRequestMethod("GET");
+ conn.setInstanceFollowRedirects(true);
+ try (var reader = new BufferedReader(new InputStreamReader(conn.getInputStream(), java.nio.charset.StandardCharsets.UTF_8)))
+ {
+ if (conn.getResponseCode() != 200)
+ {
+ log.debug("Version check responded with HTTP {}", conn.getResponseCode());
+ return null;
+ }
+ String line = reader.readLine();
+ return line != null ? line.trim() : null;
+ }
+ finally
+ {
+ conn.disconnect();
+ }
+}Add imports:
// add if missing
import java.net.HttpURLConnection;
import java.nio.charset.StandardCharsets;🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotVersionChecker.java
around lines 62 to 69, replace the URL#openStream usage with an
HttpURLConnection: open a connection to REMOTE_VERSION_URL, call
conn.setConnectTimeout(5000) and conn.setReadTimeout(5000), set request method
to "GET", call conn.connect(), verify conn.getResponseCode() ==
HttpURLConnection.HTTP_OK and throw an exception otherwise, then read the
response using a try-with-resources InputStreamReader wrapped in BufferedReader
using StandardCharsets.UTF_8 to get the version (e.g., first line) and finally
disconnect the connection; also add imports for java.net.HttpURLConnection and
java.nio.charset.StandardCharsets.
Comment on lines
90
to
93
| public void checkForUpdate() | ||
| { | ||
| scheduledExecutorService.scheduleWithFixedDelay(this::runVersionCheck, 0, 10, TimeUnit.MINUTES); | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make scheduling idempotent to avoid multiple tasks if checkForUpdate() is called more than once.
Repeated calls schedule multiple fixed‑delay tasks on the same executor.
-public void checkForUpdate()
-{
- scheduledExecutorService.scheduleWithFixedDelay(this::runVersionCheck, 0, 10, TimeUnit.MINUTES);
-}
+private final java.util.concurrent.atomic.AtomicBoolean scheduled = new java.util.concurrent.atomic.AtomicBoolean(false);
+private volatile java.util.concurrent.ScheduledFuture<?> future;
+
+public void checkForUpdate()
+{
+ if (scheduled.compareAndSet(false, true))
+ {
+ future = scheduledExecutorService.scheduleWithFixedDelay(this::runVersionCheck, 0, 10, TimeUnit.MINUTES);
+ }
+}Add import if desired:
import java.util.concurrent.ScheduledFuture;🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotVersionChecker.java
around lines 90 to 93, scheduling is not idempotent and repeated calls to
checkForUpdate() will create multiple scheduled tasks; add a private
ScheduledFuture<?> field to hold the scheduled task, check that field before
scheduling (only schedule if null or isDone/isCancelled), or cancel the existing
future and reschedule as desired, and assign the ScheduledFuture returned by
scheduleWithFixedDelay to that field; also add the import
java.util.concurrent.ScheduledFuture if missing.
Merged
This was referenced Aug 27, 2025
Merged
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.
Features
Summary by CodeRabbit