Skip to content

fix(ui): keep the saved sidebar tab and stdout intact while closing - #1954

Merged
chsami merged 1 commit into
developmentfrom
claude/console-shutdown-followups
Oct 13, 2026
Merged

chsami merged 1 commit into
developmentfrom
claude/console-shutdown-followups

Conversation

@chsami

@chsami chsami commented Oct 13, 2026

Copy link
Copy Markdown
Owner

Follow-ups from the review of #1951.

Problem

  1. Since fix(console): show each log line once and stop following on user input #1951, ClientUI.removeNavigation stores the fallback tab after it reopens one. On client close, MicrobotPluginManager.onClientShutdown stops the Hub plugins; their ClientToolbar.removeNavigation runs through invokeLater, after ConfigManager's final sendConfig (priority -100). The in-memory clientSidebarTab then flips to the fallback tab (or is unset), and a sendConfig during the exit window (the 5-minute periodic flush, or anything else that saves config) persists the wrong tab.
  2. ConsoleLogAppender.bindStdoutAppenders handed the real System.out PrintStream to logback. OutputStreamAppender.stop() and setOutputStream() close the current stream, so a stopped or reset LoggerContext (logback scan=true reconfiguration in dev) closed fd 1 and all later terminal output was lost.

Change

  • ClientUI: a closing flag is set by a ClientShutdown subscriber (priority 100, so before any plugin stops). removeNavigation still reopens the fallback tab but does not store it while closing. Removals while the client runs (user disables a plugin) still store the tab that is shown, as in fix(console): show each log line once and stop following on user input #1951. Diff to the upstream file: 11 lines added, 1 removed.
  • ConsoleLogAppender.bindStdoutAppenders wraps the stream in a private CloseShieldOutputStream (close() only flushes; bulk writes are passed through).

Testing

  • New ClientUITest (builds ClientUI through its private constructor with a mocked ConfigManager, a real EventBus and a plain JTabbedPane in place of the UI built by init()):
    • removingTheOpenTabWhileClosingKeepsTheSavedTab: fails on development (NeverWantedButInvoked: setConfiguration("runelite", "clientSidebarTab", ...)), passes with the fix.
    • removingTheOpenTabStoresTheTabShownInstead: guards the fix(console): show each log line once and stop following on user input #1951 behaviour while the client runs; passes before and after.
    • consoleInitializationKeepsStdoutLogLinesOutOfThePanel: calls the real initializeConsoleLogging and checks a root log line lands in the console panel once. With the bindStdoutAppenders call removed it fails with expected:<1> but was:<3>.
  • New ConsoleLogAppenderTest.stoppingTheLoggerContextLeavesStdoutOpen: fails on development (stream closed by context.stop()), passes with the fix.
  • Full ./gradlew :client:runUnitTests --offline: 2364 tests, 0 failures, 0 errors, 2 skipped (335 classes), including ClientThreadGuardrailTest with no baseline change.
  • Live (no login): a probe plugin marked isExternal = true added a sidebar panel and opened it (saved tab = the probe tab). The client was closed through the window close button path (WM close + the Exit confirmation), and the probe's shutDown called ConfigManager.sendConfig() after its removal to simulate a flush in the exit window.
    • Fixed build: after removal the saved tab was still the probe tab, and after a restart the persisted value was the probe tab.
    • Pre-fix build (same scenario): after removal the saved tab was null, and after a restart the persisted value was null.

Not verified: an installed Hub plugin restoring its own tab on restart (none of the locally installed Hub plugins adds a sidebar panel, and a deployed probe registers too late for the startup-only restore). The close-shield was not exercised live with a real logback rescan; it is covered by the unit test only. Profile switches that stop plugins still store the fallback tab, as before this PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LjtEMzQHGuq4M1Mpqk9pPV

When the client closes, MicrobotPluginManager stops the Hub plugins and
each removeNavigation runs from invokeLater after ConfigManager's final
sendConfig. Removing the open tab then stored the fallback tab, so the
in-memory value flipped and any later flush in the exit window persisted
the wrong tab. ClientUI now marks itself closing on ClientShutdown and
removeNavigation no longer stores the tab while closing.

bindStdoutAppenders handed the real System.out to logback, and
OutputStreamAppender.stop closes its stream, so a stopped or reset
LoggerContext closed fd 1. The stream is now wrapped in a close shield.

ClientUITest covers both tab paths and that console initialization binds
the stdout appenders; ConsoleLogAppenderTest covers the close shield.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LjtEMzQHGuq4M1Mpqk9pPV
@coderabbitai

coderabbitai Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 11 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9718975d-9911-4588-a656-63c67f06bba5
📥 Commits

Reviewing files that changed from the base of the PR and between d9ad291 and f534f83.

📒 Files selected for processing (4)
  • runelite-client/src/main/java/net/runelite/client/ui/ClientUI.java
  • runelite-client/src/main/java/net/runelite/client/ui/ConsoleLogAppender.java
  • runelite-client/src/test/java/net/runelite/client/ui/ClientUITest.java
  • runelite-client/src/test/java/net/runelite/client/ui/ConsoleLogAppenderTest.java
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chsami
chsami merged commit 805a221 into development Oct 13, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant