fix(desktop): honor minimizeOnLaunch on auto-start (#115)#121
Conversation
The renderer stored minimizeOnLaunch only in zustand persist (localStorage), but the main process reads it from the SQLite settings table at startup — so the value was never actually visible to main. The main process always saw the default (false) and opened the window even when the user had enabled 'Minimize on Launch' in the UI. Fix: - Sync minimizeOnLaunch (and launchAtStartup, for symmetry) to the main process DB on toggle and on every store rehydrate. This also migrates existing users whose value currently lives only in localStorage. - On Windows, also pass '--hidden' as the auto-launch argument so the main process can detect hidden startup independently of the DB (the macOS 'openAsHidden' flag is a no-op on Windows). - On startup, respect an OS-level hidden signal (wasOpenedAsHidden or '--hidden' arg) in addition to the persisted setting, so the window stays hidden reliably even if the DB sync has not yet happened. Tests: - tests/unit/main/settings-startup.test.ts: verifies getMinimizeOnLaunchSetting reads the persisted SQLite value correctly and tolerates missing / malformed rows. - tests/unit/stores/settings-startup.test.ts: verifies the renderer store pushes minimizeOnLaunch and launchAtStartup to window.api.settings.set() so the main process can read them. Fixes legeling#115
|
@TuTouPower is attempting to deploy a commit to the legeling's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough本 PR 添加启动时最小化设置:扩展 Settings 类型、主进程在 ready-to-show 根据 OS 隐藏信号与持久化设置决定窗口可见性,渲染器同步设置到主进程,并补充单元测试覆盖主进程与渲染器行为。 启动时最小化功能
sequenceDiagram
participant User
participant AppMain as MainProcess
participant Store as RendererStore
participant DB as SettingsDB
participant Window as mainWindow
User->>Store: 切换启动设置
Store->>Store: 更新本地状态
Store->>AppMain: syncSettingsToMain via IPC
AppMain->>DB: 持久化设置
AppMain->>Window: 启动时检测 OS hidden / args / DB
alt shouldMinimize
Window->>Window: 隐藏/最小化
else
Window->>Window: 显示
end
🎯 3 (中等) | ⏱️ ~25 分钟 可能相关的 PRs:
🚥 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 unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Review Summary by QodoFix minimize-on-launch not honored on auto-start by syncing to main DB
WalkthroughsDescription• Sync minimizeOnLaunch and launchAtStartup to main process DB on toggle • Honor OS-level hidden signals (--hidden arg, wasOpenedAsHidden) at startup • Add comprehensive tests for settings persistence and startup behavior • Migrate existing users whose settings live only in localStorage Diagramflowchart LR
A["Renderer toggles<br/>minimizeOnLaunch"] -->|syncSettingsToMain| B["Main process<br/>SQLite DB"]
C["App auto-starts"] -->|check OS signal| D["--hidden arg or<br/>wasOpenedAsHidden"]
D -->|if present| E["Start minimized"]
D -->|if absent| F["Check DB setting"]
F -->|minimizeOnLaunch=true| E
F -->|minimizeOnLaunch=false| G["Show window"]
B -->|persisted| F
File Changes1. apps/desktop/src/main/index.ts
|
Code Review by Qodo
1.
|
There was a problem hiding this comment.
🧹 Nitpick comments (7)
apps/desktop/tests/unit/stores/settings-startup.test.ts (4)
82-91: ⚡ Quick win移除冗余的
.toBeDefined()断言同上,
expect(payload).toBeDefined()是多余的。♻️ 建议的修复
const payload = lastPayloadWithKey(setSpy, "launchAtStartup"); - expect(payload).toBeDefined(); - expect(payload?.launchAtStartup).toBe(true); + expect(payload?.launchAtStartup).toBe(true);🤖 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 `@apps/desktop/tests/unit/stores/settings-startup.test.ts` around lines 82 - 91, The test "syncs launchAtStartup to the main process when toggled" contains a redundant assertion; remove the unnecessary expect(payload).toBeDefined() in the test that imports setSpy and uses lastPayloadWithKey, leaving the existing checks that assert useSettingsStore.getState().launchAtStartup is true and that payload?.launchAtStartup is true after calling useSettingsStore.getState().setLaunchAtStartup(true).
71-80: ⚡ Quick win移除冗余的
.toBeDefined()断言同上,
expect(payload).toBeDefined()是多余的。♻️ 建议的修复
const payload = lastPayloadWithKey(setSpy, "minimizeOnLaunch"); - expect(payload).toBeDefined(); - expect(payload?.minimizeOnLaunch).toBe(false); + expect(payload?.minimizeOnLaunch).toBe(false);🤖 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 `@apps/desktop/tests/unit/stores/settings-startup.test.ts` around lines 71 - 80, Remove the redundant assertion expect(payload).toBeDefined() in the test "syncs minimizeOnLaunch=false to the main process when toggled off"; keep the existing checks using lastPayloadWithKey(setSpy, "minimizeOnLaunch") and the subsequent expect(payload?.minimizeOnLaunch).toBe(false) so the test still verifies the payload value, referencing useSettingsStore.getState().setMinimizeOnLaunch, lastPayloadWithKey, and setSpy.
60-80: 💤 Low value考虑使用参数化测试减少重复
两个
minimizeOnLaunch测试仅在布尔值上有差异,可以使用it.each()合并以符合编码规范:"禁止复制粘贴仅有细微差异的测试块;使用it.each()或参数化测试"。♻️ 可选的重构建议
it.each([ { value: true, label: "true" }, { value: false, label: "false" }, ])( "syncs minimizeOnLaunch=$label to the main process when toggled", async ({ value }) => { const { useSettingsStore, setSpy } = await importStoreWithSpy(); useSettingsStore.getState().setMinimizeOnLaunch(value); expect(useSettingsStore.getState().minimizeOnLaunch).toBe(value); const payload = lastPayloadWithKey(setSpy, "minimizeOnLaunch"); expect(payload?.minimizeOnLaunch).toBe(value); } );🤖 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 `@apps/desktop/tests/unit/stores/settings-startup.test.ts` around lines 60 - 80, Replace the two nearly-identical tests for setMinimizeOnLaunch with a parameterized test using Jest's it.each to remove duplication: write an it.each over [{value:true,label:"true"},{value:false,label:"false"}] that imports importStoreWithSpy, calls useSettingsStore.getState().setMinimizeOnLaunch(value), asserts useSettingsStore.getState().minimizeOnLaunch === value, and checks lastPayloadWithKey(setSpy, "minimizeOnLaunch")?.minimizeOnLaunch === value; keep the existing helpers (importStoreWithSpy, useSettingsStore, setMinimizeOnLaunch, lastPayloadWithKey) but consolidate into one parameterized test named like "syncs minimizeOnLaunch=$label to the main process when toggled".
60-69: ⚡ Quick win移除冗余的
.toBeDefined()断言下一行已经使用可选链
payload?.minimizeOnLaunch检查实际值,因此expect(payload).toBeDefined()是多余的懒惰断言。如果payload为undefined,实际值检查会提供更明确的错误信息。根据编码规范:"禁止使用懒惰断言如
expect(result).toBeDefined(),应直接检查实际值"。♻️ 建议的修复
const payload = lastPayloadWithKey(setSpy, "minimizeOnLaunch"); - expect(payload).toBeDefined(); - expect(payload?.minimizeOnLaunch).toBe(true); + expect(payload?.minimizeOnLaunch).toBe(true);🤖 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 `@apps/desktop/tests/unit/stores/settings-startup.test.ts` around lines 60 - 69, Remove the redundant lazy assertion expect(payload).toBeDefined() in the test "syncs minimizeOnLaunch=true to the main process when toggled" and rely on the direct check expect(payload?.minimizeOnLaunch).toBe(true); specifically, delete the line that calls expect(payload).toBeDefined() in apps/desktop/tests/unit/stores/settings-startup.test.ts where payload is obtained from lastPayloadWithKey(setSpy, "minimizeOnLaunch") and keep the remaining assertions using useSettingsStore.getState(), setMinimizeOnLaunch, and lastPayloadWithKey.apps/desktop/src/renderer/stores/settings.store.ts (3)
812-815: ⚡ Quick win移除不必要的类型断言
syncSettingsToMain的参数类型是Partial<Settings>,对象字面量{ launchAtStartup: enabled }应该可以直接赋值,无需as Partial<Settings>断言。根据编码规范,类型断言必须有注释说明必要性,或者应该修复底层类型错误而非使用断言。♻️ 建议的修复
- syncSettingsToMain({ launchAtStartup: enabled } as Partial<Settings>); + syncSettingsToMain({ launchAtStartup: enabled });🤖 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 `@apps/desktop/src/renderer/stores/settings.store.ts` around lines 812 - 815, Remove the unnecessary type assertion on the object passed to syncSettingsToMain: change the call that currently uses "{ launchAtStartup: enabled } as Partial<Settings>" to pass the object literal directly, e.g. syncSettingsToMain({ launchAtStartup: enabled }); ensure the property name matches the Settings interface (launchAtStartup) and that enabled is the correct boolean, and only add a typed cast if a real type mismatch in the Settings definition is discovered and cannot be fixed.
1452-1460: ⚡ Quick win移除不必要的类型断言
rehydration 回调中的对象字面量也可以直接满足
Partial<Settings>类型,无需断言。♻️ 建议的修复
launchAtStartup: state?.launchAtStartup ?? false, minimizeOnLaunch: state?.minimizeOnLaunch ?? false, - } as Partial<Settings>); + });🤖 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 `@apps/desktop/src/renderer/stores/settings.store.ts` around lines 1452 - 1460, Remove the unnecessary type assertion "as Partial<Settings>" from the object literal returned in the rehydration callback around the properties launchAtStartup and minimizeOnLaunch; the literal already conforms to Partial<Settings>, so update the rehydrate/restore block (the object that sets launchAtStartup and minimizeOnLaunch) to return the object without the type cast, leaving TypeScript to infer the Partial<Settings> type.
828-834: ⚡ Quick win移除不必要的类型断言
同上,
{ minimizeOnLaunch: enabled }可以直接传递给syncSettingsToMain,无需类型断言。♻️ 建议的修复
- syncSettingsToMain({ minimizeOnLaunch: enabled } as Partial<Settings>); + syncSettingsToMain({ minimizeOnLaunch: enabled });🤖 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 `@apps/desktop/src/renderer/stores/settings.store.ts` around lines 828 - 834, The code is using an unnecessary type assertion when calling syncSettingsToMain; remove the "as Partial<Settings>" so you pass the object literal { minimizeOnLaunch: enabled } directly to syncSettingsToMain (update the call site where syncSettingsToMain({ minimizeOnLaunch: enabled } as Partial<Settings}) is used to simply syncSettingsToMain({ minimizeOnLaunch: enabled })). Ensure the call still type-checks against syncSettingsToMain's parameter type.
🤖 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.
Nitpick comments:
In `@apps/desktop/src/renderer/stores/settings.store.ts`:
- Around line 812-815: Remove the unnecessary type assertion on the object
passed to syncSettingsToMain: change the call that currently uses "{
launchAtStartup: enabled } as Partial<Settings>" to pass the object literal
directly, e.g. syncSettingsToMain({ launchAtStartup: enabled }); ensure the
property name matches the Settings interface (launchAtStartup) and that enabled
is the correct boolean, and only add a typed cast if a real type mismatch in the
Settings definition is discovered and cannot be fixed.
- Around line 1452-1460: Remove the unnecessary type assertion "as
Partial<Settings>" from the object literal returned in the rehydration callback
around the properties launchAtStartup and minimizeOnLaunch; the literal already
conforms to Partial<Settings>, so update the rehydrate/restore block (the object
that sets launchAtStartup and minimizeOnLaunch) to return the object without the
type cast, leaving TypeScript to infer the Partial<Settings> type.
- Around line 828-834: The code is using an unnecessary type assertion when
calling syncSettingsToMain; remove the "as Partial<Settings>" so you pass the
object literal { minimizeOnLaunch: enabled } directly to syncSettingsToMain
(update the call site where syncSettingsToMain({ minimizeOnLaunch: enabled } as
Partial<Settings}) is used to simply syncSettingsToMain({ minimizeOnLaunch:
enabled })). Ensure the call still type-checks against syncSettingsToMain's
parameter type.
In `@apps/desktop/tests/unit/stores/settings-startup.test.ts`:
- Around line 82-91: The test "syncs launchAtStartup to the main process when
toggled" contains a redundant assertion; remove the unnecessary
expect(payload).toBeDefined() in the test that imports setSpy and uses
lastPayloadWithKey, leaving the existing checks that assert
useSettingsStore.getState().launchAtStartup is true and that
payload?.launchAtStartup is true after calling
useSettingsStore.getState().setLaunchAtStartup(true).
- Around line 71-80: Remove the redundant assertion
expect(payload).toBeDefined() in the test "syncs minimizeOnLaunch=false to the
main process when toggled off"; keep the existing checks using
lastPayloadWithKey(setSpy, "minimizeOnLaunch") and the subsequent
expect(payload?.minimizeOnLaunch).toBe(false) so the test still verifies the
payload value, referencing useSettingsStore.getState().setMinimizeOnLaunch,
lastPayloadWithKey, and setSpy.
- Around line 60-80: Replace the two nearly-identical tests for
setMinimizeOnLaunch with a parameterized test using Jest's it.each to remove
duplication: write an it.each over
[{value:true,label:"true"},{value:false,label:"false"}] that imports
importStoreWithSpy, calls
useSettingsStore.getState().setMinimizeOnLaunch(value), asserts
useSettingsStore.getState().minimizeOnLaunch === value, and checks
lastPayloadWithKey(setSpy, "minimizeOnLaunch")?.minimizeOnLaunch === value; keep
the existing helpers (importStoreWithSpy, useSettingsStore, setMinimizeOnLaunch,
lastPayloadWithKey) but consolidate into one parameterized test named like
"syncs minimizeOnLaunch=$label to the main process when toggled".
- Around line 60-69: Remove the redundant lazy assertion
expect(payload).toBeDefined() in the test "syncs minimizeOnLaunch=true to the
main process when toggled" and rely on the direct check
expect(payload?.minimizeOnLaunch).toBe(true); specifically, delete the line that
calls expect(payload).toBeDefined() in
apps/desktop/tests/unit/stores/settings-startup.test.ts where payload is
obtained from lastPayloadWithKey(setSpy, "minimizeOnLaunch") and keep the
remaining assertions using useSettingsStore.getState(), setMinimizeOnLaunch, and
lastPayloadWithKey.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8dbfe0fc-8cac-4bef-a6f6-326260957032
📒 Files selected for processing (5)
apps/desktop/src/main/index.tsapps/desktop/src/renderer/stores/settings.store.tsapps/desktop/tests/unit/main/settings-startup.test.tsapps/desktop/tests/unit/stores/settings-startup.test.tspackages/shared/types/settings.ts
- Drop Chinese-only comments introduced by this PR (AGENTS.md rule) - Drop unnecessary `as Partial<Settings>` assertions — the object literals already satisfy the parameter type - Log `app.getLoginItemSettings()` failures instead of silently falling back; electron can throw on systems without a login-items service, and a silent false caused issue legeling#115 all over again in those paths - Wrap `app:setAutoLaunch` handler's `setLoginItemSettings` call in a try/catch so OS-level policy denials don't crash the main process - Parameterize the two renderer store tests with `it.each` instead of copy-paste, and drop redundant `.toBeDefined()` assertions in favor of the more specific `toBe(value)` check
Summary
Fixes #115 — when a user enables both "launch at startup" and "minimize on launch" on Windows (and by extension, all platforms), the window still pops up on auto-start instead of staying hidden.
Root cause
The renderer stored `minimizeOnLaunch` only in zustand `persist` (localStorage), but the main process reads it from the SQLite `settings` table on startup. The value was never synced to SQLite, so main always saw the default (`false`) and showed the window — even when the UI displayed the toggle as enabled.
On Windows, there is a second issue: electron's `openAsHidden` flag on `app.setLoginItemSettings` is a macOS-only hint. On Windows the OS has no way to tell the app it was auto-started hidden.
Fix
Persist to main DB
OS-level signal on Windows
Shared types
Tests
Verification
Notes
Summary by CodeRabbit
新功能
Bug 修复
测试
其他