feat: remember desktop window size and position across launches#672
Merged
Conversation
Contributor
WalkthroughThe PR adds a WindowStateStore to persist window placement, position, and size to disk, restores validated state at startup, wires debounced runtime saves via snapshotFlow, ensures save on close/shutdown, and updates release notes to mention the new behavior. ChangesDesktop Window State Persistence
Sequence Diagram(s)sequenceDiagram
participant DesktopApp
participant WindowStateStore
participant Filesystem
participant DesktopWindow
Note over DesktopApp,Filesystem: Startup
DesktopApp->>WindowStateStore: load()
WindowStateStore->>Filesystem: read window-state.json
Filesystem-->>WindowStateStore: persisted data
WindowStateStore-->>DesktopApp: restored/centered WindowState
Note over DesktopWindow,DesktopApp: Runtime
DesktopWindow->>DesktopApp: placement/position/size changes
DesktopApp->>DesktopApp: snapshotFlow + distinctUntilChanged + debounce(500ms)
DesktopApp->>WindowStateStore: save(windowState)
WindowStateStore->>Filesystem: write window-state.json
Note over DesktopWindow,DesktopApp: Exit
DesktopWindow->>DesktopApp: onCloseRequest / JVM shutdown
DesktopApp->>WindowStateStore: save(windowState)
WindowStateStore->>Filesystem: write window-state.json
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@composeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/DesktopApp.kt`:
- Around line 94-100: The collector and the close handler are invoking the
blocking WindowStateStore.save(...) on the Compose/UI dispatcher; change both
sites to run the save on Dispatchers.IO so disk IO does not block UI or
shutdown. In the LaunchedEffect snapshotFlow collector replace the direct call
with a suspend offload (e.g., wrap the call in withContext(Dispatchers.IO) {
WindowStateStore.save(windowState) } or launch(Dispatchers.IO) {
WindowStateStore.save(windowState) }) and in the onCloseRequest handler launch
the save on Dispatchers.IO (e.g., CoroutineScope(Dispatchers.IO).launch {
WindowStateStore.save(windowState) }) instead of calling WindowStateStore.save
synchronously so the write happens off the UI thread and the close path is not
blocked.
In `@core/presentation/src/commonMain/composeResources/files/whatsnew/19.json`:
- Line 14: Update the release-note sentence in the whatsnew entry that currently
reads about "maximized state" so it also mentions fullscreen restoration; locate
the JSON string in the whatsnew/19 entry (the sentence starting "Desktop
remembers your last window size, position, and maximized state...") and change
the wording to include "fullscreen" (e.g., "maximized or fullscreen state" or
"maximized and fullscreen state") so the note accurately reflects that
fullscreen state is restored across launches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 62cfa5e9-c48b-4d82-ab2f-64438fb1d88c
📒 Files selected for processing (3)
composeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/DesktopApp.ktcomposeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/desktop/WindowStateStore.ktcore/presentation/src/commonMain/composeResources/files/whatsnew/19.json
Comment on lines
+94
to
+100
| LaunchedEffect(windowState) { | ||
| snapshotFlow { | ||
| Triple(windowState.placement, windowState.position, windowState.size) | ||
| }.distinctUntilChanged() | ||
| .debounce(500) | ||
| .collect { WindowStateStore.save(windowState) } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that save is collected without an IO dispatcher and performs blocking file IO.
set -euo pipefail
echo "== DesktopApp collection site =="
rg -n -C3 'snapshotFlow|debounce|collect|WindowStateStore\.save' composeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/DesktopApp.kt
echo
echo "== WindowStateStore blocking write site =="
rg -n -C3 'fun save|writeText|encodeToString' composeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/desktop/WindowStateStore.ktRepository: OpenHub-Store/GitHub-Store
Length of output: 2194
Move debounced persistence off the UI dispatcher (and fix close path).
WindowStateStore.save() performs blocking disk IO (configFile.writeText(...)) and is invoked directly from the Compose LaunchedEffect snapshotFlow collector (.collect { WindowStateStore.save(windowState) }), which can cause UI stutter during resize/drag. The same synchronous WindowStateStore.save(windowState) call also runs on onCloseRequest, risking delayed/blocked shutdown. Switch both call sites to Dispatchers.IO (e.g., wrap the save with withContext(Dispatchers.IO) or launch an IO coroutine) so the UI thread never performs the blocking write.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@composeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/DesktopApp.kt` around
lines 94 - 100, The collector and the close handler are invoking the blocking
WindowStateStore.save(...) on the Compose/UI dispatcher; change both sites to
run the save on Dispatchers.IO so disk IO does not block UI or shutdown. In the
LaunchedEffect snapshotFlow collector replace the direct call with a suspend
offload (e.g., wrap the call in withContext(Dispatchers.IO) {
WindowStateStore.save(windowState) } or launch(Dispatchers.IO) {
WindowStateStore.save(windowState) }) and in the onCloseRequest handler launch
the save on Dispatchers.IO (e.g., CoroutineScope(Dispatchers.IO).launch {
WindowStateStore.save(windowState) }) instead of calling WindowStateStore.save
synchronously so the write happens off the UI thread and the close path is not
blocked.
| "Inner Details pages — About and What's New now slide in as dedicated screens with their own back action, and stay inside the right pane on tablet.", | ||
| "Real Apple and Tux icons for the macOS and Linux platform indicators across cards and lists." | ||
| "Real Apple and Tux icons for the macOS and Linux platform indicators across cards and lists.", | ||
| "Desktop remembers your last window size, position, and maximized state across launches; new windows open centered instead of in the top-left corner." |
Contributor
There was a problem hiding this comment.
Include fullscreen restore in the release note for accuracy.
The PR scope includes restoring fullscreen state too, but this bullet currently mentions only maximized state.
Suggested wording
- "Desktop remembers your last window size, position, and maximized state across launches; new windows open centered instead of in the top-left corner."
+ "Desktop remembers your last window size, position, and maximized/fullscreen state across launches; new windows open centered instead of in the top-left corner."📝 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
| "Desktop remembers your last window size, position, and maximized state across launches; new windows open centered instead of in the top-left corner." | |
| "Desktop remembers your last window size, position, and maximized/fullscreen state across launches; new windows open centered instead of in the top-left corner." |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/presentation/src/commonMain/composeResources/files/whatsnew/19.json` at
line 14, Update the release-note sentence in the whatsnew entry that currently
reads about "maximized state" so it also mentions fullscreen restoration; locate
the JSON string in the whatsnew/19 entry (the sentence starting "Desktop
remembers your last window size, position, and maximized state...") and change
the wording to include "fullscreen" (e.g., "maximized or fullscreen state" or
"maximized and fullscreen state") so the note accurately reflects that
fullscreen state is restored across launches.
Greptile SummaryThis PR persists the desktop window's size, position, and maximized/fullscreen state to
Confidence Score: 5/5Safe to merge — the feature is well-contained, all IO paths are wrapped in runCatching, and any parse failure on load falls back to the default centered window. The change is narrowly scoped to window state persistence. Defensive coding throughout means a bad state file is silently ignored and defaults are used. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Main as main()
participant App as application {}
participant WSS as WindowStateStore
participant IO as Dispatchers.IO
participant FS as window-state.json
participant JVM as JVM Shutdown
Main->>WSS: load() [blocking, main thread]
WSS->>FS: readText()
FS-->>WSS: JSON / missing
WSS-->>App: WindowState (restored or default)
App->>App: DisposableEffect
App->>App: LaunchedEffect snapshotFlow debounce(500ms)
loop User drags / resizes
App->>App: snapshotFlow emits
App->>IO: withContext(IO) save()
IO->>FS: writeText(JSON)
end
alt Normal close
App->>WSS: save(windowState) EDT sync
WSS->>FS: writeText(JSON)
App->>App: exitApplication()
App->>App: onDispose removeShutdownHook
App-->>JVM: exits normally
else SIGTERM / hard kill
JVM->>WSS: shutdownHook thread save()
WSS->>FS: writeText(JSON)
note over IO,FS: concurrent write possible if IO dispatch in flight
end
Reviews (2): Last reviewed commit: "fix(desktop): move window state save off..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
composeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/desktop/WindowStateStore.kt (2)
75-89:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset the size when absolute coordinates are rejected.
If
isTitleBarVisible(...)fails, this falls back to centered positioning but still keeps the persistedwidth/height. A window saved on a larger monitor can reopen centered and still spill off-screen on a smaller display, which misses the documented1280×840fallback behavior.📐 Proposed fix
- val width = (saved?.width ?: DEFAULT_WIDTH).coerceAtLeast(MIN_WIDTH) - val height = (saved?.height ?: DEFAULT_HEIGHT).coerceAtLeast(MIN_HEIGHT) + val savedWidth = (saved?.width ?: DEFAULT_WIDTH).coerceAtLeast(MIN_WIDTH) + val savedHeight = (saved?.height ?: DEFAULT_HEIGHT).coerceAtLeast(MIN_HEIGHT) val placement = parsePlacement(saved?.placement) val savedX = saved?.x val savedY = saved?.y + val canRestoreAbsolutePosition = + savedX != null && + savedY != null && + isTitleBarVisible(savedX, savedY, savedWidth, savedHeight) + val width = if (canRestoreAbsolutePosition) savedWidth else DEFAULT_WIDTH + val height = if (canRestoreAbsolutePosition) savedHeight else DEFAULT_HEIGHT val position = - if ( - savedX != null && - savedY != null && - isTitleBarVisible(savedX, savedY, width, height) - ) { + if (canRestoreAbsolutePosition) { WindowPosition(savedX.dp, savedY.dp) } else { WindowPosition.Aligned(Alignment.Center) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@composeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/desktop/WindowStateStore.kt` around lines 75 - 89, The current logic keeps the persisted width/height even when saved absolute coordinates are rejected by isTitleBarVisible; change the fallback so when savedX/savedY are non-null but isTitleBarVisible(...) returns false you also reset width and height to their safe defaults (e.g., DEFAULT_WIDTH/DEFAULT_HEIGHT coerced with MIN_WIDTH/MIN_HEIGHT) before choosing WindowPosition.Aligned(Alignment.Center). Update the block around width/height/position calculation (references: width, height, savedX, savedY, isTitleBarVisible, WindowPosition.Aligned) to assign the default/coerced size in that else branch so centered windows never reopen larger than the documented fallback.
97-116:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSerialize and atomically replace
window-state.jsoninWindowStateStore.save()
WindowStateStore.save()is called from the debounced collector, the window close handler, and a shutdown hook, but it currently writesconfigFileviaconfigFile.writeText(...)with no synchronization—overlapping saves can truncate or partially writewindow-state.json(WindowStateStore.ktaround lines 97-116).🧵 Proposed fix
+import java.nio.file.Files +import java.nio.file.StandardCopyOption.ATOMIC_MOVE +import java.nio.file.StandardCopyOption.REPLACE_EXISTING ... - fun save(state: WindowState) { + `@Synchronized` + fun save(state: WindowState) { runCatching { val pos = state.position val (x, y) = if (pos is WindowPosition.Absolute) { pos.x.value to pos.y.value } else { null to null } val persisted = PersistedWindowState( x = x, y = y, width = state.size.width.value, height = state.size.height.value, placement = state.placement.name, ) val parent = configFile.parentFile if (parent != null && !parent.isDirectory) parent.mkdirs() - configFile.writeText(json.encodeToString(persisted)) + val tempFile = File(parent, "$FILE_NAME.tmp") + tempFile.writeText(json.encodeToString(persisted)) + Files.move(tempFile.toPath(), configFile.toPath(), REPLACE_EXISTING, ATOMIC_MOVE) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@composeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/desktop/WindowStateStore.kt` around lines 97 - 116, WindowStateStore.save currently calls configFile.writeText concurrently which can corrupt window-state.json; change save (in class WindowStateStore, function save) to perform an atomic replace: create a temp file in the same directory, write the serialized PersistedWindowState/json to that temp file, flush and force the file descriptor to disk, then atomically move/rename the temp file over configFile (use Files.move with REPLACE_EXISTING or File.renameTo) and ensure the parent dir exists before creating the temp file; also guard the whole write sequence with a brief synchronization (e.g. synchronized(this) or a private lock object) so concurrent callers of save cannot interleave writes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@composeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/desktop/WindowStateStore.kt`:
- Around line 75-89: The current logic keeps the persisted width/height even
when saved absolute coordinates are rejected by isTitleBarVisible; change the
fallback so when savedX/savedY are non-null but isTitleBarVisible(...) returns
false you also reset width and height to their safe defaults (e.g.,
DEFAULT_WIDTH/DEFAULT_HEIGHT coerced with MIN_WIDTH/MIN_HEIGHT) before choosing
WindowPosition.Aligned(Alignment.Center). Update the block around
width/height/position calculation (references: width, height, savedX, savedY,
isTitleBarVisible, WindowPosition.Aligned) to assign the default/coerced size in
that else branch so centered windows never reopen larger than the documented
fallback.
- Around line 97-116: WindowStateStore.save currently calls configFile.writeText
concurrently which can corrupt window-state.json; change save (in class
WindowStateStore, function save) to perform an atomic replace: create a temp
file in the same directory, write the serialized PersistedWindowState/json to
that temp file, flush and force the file descriptor to disk, then atomically
move/rename the temp file over configFile (use Files.move with REPLACE_EXISTING
or File.renameTo) and ensure the parent dir exists before creating the temp
file; also guard the whole write sequence with a brief synchronization (e.g.
synchronized(this) or a private lock object) so concurrent callers of save
cannot interleave writes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b8411a6e-7202-45ba-930a-49dc808da714
📒 Files selected for processing (2)
composeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/DesktopApp.ktcomposeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/desktop/WindowStateStore.kt
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
composeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/desktop/WindowStateStore.kt (1)
98-123:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSerialize writes to
window-state.json.
save()can run concurrently from the debounced collector, the shutdown hook, and the close path incomposeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/DesktopApp.kt. ConcurrentwriteText(...)calls against the same file can leave partially written JSON, which then makesload()fall back to defaults on the next launch.💡 Minimal fix
object WindowStateStore { + private val saveLock = Any() + private const val FILE_NAME = "window-state.json" private const val MIN_WIDTH = 480f private const val MIN_HEIGHT = 600f @@ fun save(state: WindowState) { if (!state.size.isSpecified) return val width = state.size.width.value val height = state.size.height.value if (width.isNaN() || height.isNaN()) return runCatching { @@ val persisted = PersistedWindowState( x = x, y = y, width = width, height = height, placement = state.placement.name, ) val parent = configFile.parentFile if (parent != null && !parent.isDirectory) parent.mkdirs() - configFile.writeText(json.encodeToString(persisted)) + val payload = json.encodeToString(persisted) + synchronized(saveLock) { + configFile.writeText(payload) + } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@composeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/desktop/WindowStateStore.kt` around lines 98 - 123, Concurrent writes to configFile via save() can produce partial JSON; fix by writing to a temporary file in the same directory and then atomically replacing configFile. In save(), after creating parent dirs, write the serialized PersistedWindowState to a temp file (e.g., configFile.name + ".tmp") in the same directory, flush/close it, then atomically move/rename the temp file over configFile using java.nio.file.Files.move with ATOMIC_MOVE (fall back to REPLACE_EXISTING if ATOMIC_MOVE not supported); keep the existing runCatching wrapper to log/ignore errors. Ensure references: save(), configFile, PersistedWindowState, and writeText (replace with write-to-temp + atomic move).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@composeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/desktop/WindowStateStore.kt`:
- Around line 98-123: Concurrent writes to configFile via save() can produce
partial JSON; fix by writing to a temporary file in the same directory and then
atomically replacing configFile. In save(), after creating parent dirs, write
the serialized PersistedWindowState to a temp file (e.g., configFile.name +
".tmp") in the same directory, flush/close it, then atomically move/rename the
temp file over configFile using java.nio.file.Files.move with ATOMIC_MOVE (fall
back to REPLACE_EXISTING if ATOMIC_MOVE not supported); keep the existing
runCatching wrapper to log/ignore errors. Ensure references: save(), configFile,
PersistedWindowState, and writeText (replace with write-to-temp + atomic move).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 99b945f1-bdaa-4f0d-9237-94ca1e1ec16f
📒 Files selected for processing (2)
composeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/DesktopApp.ktcomposeApp/src/jvmMain/kotlin/zed/rainxch/githubstore/desktop/WindowStateStore.kt
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.
Closes #664.
Desktop windows now restore their last size, position, and maximized/fullscreen state instead of always opening as a square in the top-left. First launch (and any time saved coordinates fall off-screen) opens centered at 1280x840.
State is persisted to
window-state.jsonunder the platform config dir (~/Library/Application Support/GitHub-Store/,%APPDATA%/GitHub-Store/,$XDG_CONFIG_HOME/GitHub-Store/). Writes are debounced 500ms while dragging/resizing and also flushed on close.Summary by CodeRabbit