fix(window): make a backgrounded window reachable again

[miadisabelle]

"Keep them alive in the background" hides the window instead of closing it, which is the point — the agents keep running. Getting back to it is where it breaks down, in two ways.

Nothing owns a second launch. Clicking the icon, or running the binary again, starts a whole second process. It reads the same state file, restores every persisted session, and spawns a duplicate agent for each one — on top of the PTYs the first instance is still holding. The hidden window is not raised, because nothing is listening for the launch. On Linux, where there is no dock to click, that launch is the only gesture a user has, so the app answers "come back" by cloning itself over its own running terminals.

show() is not enough on its own. It is a no-op on a minimized window, and does nothing for a window that is visible but buried behind another app. Both states reach the same handlers as the hidden one.

Changes

• Take the single-instance lock in packaged builds. A second launch now arrives at the running instance as second-instance and restores its window instead of starting a process. Dev runs skip the lock, so npm run dev still starts while an installed build is running.
• Add restoreWindow() in electron/window-restore.ts: show if hidden, restore if minimized, focus either way, no-op if the window is missing or destroyed. Typed structurally against a RestorableWindow shape rather than BrowserWindow, so it is testable without an Electron runtime — following the menu-template.ts pattern.
• Route all three call sites through it: the existing activate handler, the new second-instance, and the before-quit prompt.

On that third call site

before-quit currently calls mainWindow.show() and its comment already says a quit from the menu "must not prompt invisibly". A minimized window is exactly that case, so I included it. Happy to drop it to a separate PR if you'd rather keep this one to the lock.

Testing

7 unit tests covering hidden, minimized, both at once, visible-but-unfocused, destroyed, and missing. npm run check:static and the full suite pass.

Note

Found while running a fork of this app on a headless Linux box, where the icon-relaunch path is the whole story. Not fork-specific — the duplicate-spawn happens on any platform once the window is hidden.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MoJfWuU93BeL2W48mz9bC6

[johannesjo]

Verified locally: npm run typecheck, npm run check:static (tsc + eslint + knip + depcruise) and the 7 new unit tests all pass on this branch. The premise holds — before this, nothing listened for second-instance, and activate called only show(). The if (!gotTheLock) quit() else { on('second-instance'); whenReady() } shape matches Electron's canonical example, and adding show() on top of the documented isMinimized() → restore(); focus() is the right extension for an app that actually hides its window.

Four things, one of which I'd fix before merge.

1. fixEnv() runs before the lock is requested — main.ts:84 vs main.ts:240

The second, doomed process still executes fixEnv() at module scope: an interactive+login shell (-ilc) that sources .zshrc, so nvm/conda/compinit all run before it reaches line 240 and finds out it should quit. That's dead time sitting directly in the path this PR exists to fix — the user clicks the icon and nothing happens until a throwaway shell resolves.

I timed the exact probe from fixEnv() on my machine: ~0.53s, bounded at 5s by the existing timeout. Not fatal, but it's a half-second of "did my click register?" on the icon-relaunch path, which per your Note is the whole story on the headless Linux box.

Electron's guidance is to take the lock as early as possible. Hoisting the check above fixEnv() and moving fixEnv() into the primary branch would fix it — the resolved environment only matters for spawning PTYs, which only the primary does.

2. The visible-but-buried case may not be deliverable on Wayland (note, not a change request)

Electron's own docs for win.focus(): "On Wayland (Linux), the desktop environment may show a notification or flash the app icon if the window or app is not already focused." So of the three states the PR enumerates, the third one — visible but behind another app — may resolve to an icon flash rather than a raise on Wayland, since the compositor generally won't let a client raise itself.

I don't think there's a code fix here (show() isn't demonstrably better under Wayland, and deviating from the canonical pattern to chase it isn't worth it). But the restoreWindow doc comment states the buried case as solved, and on one of the two Linux display servers it likely isn't. Worth a sentence in the comment so the next person debugging "focus doesn't work on my desktop" doesn't go looking for a bug in this function.

I'd also drop the "show() is a no-op on a minimized window" claim in the header comment — Electron's docs don't actually say that either way, and the test file's comment ("minimized windows report themselves as not visible on some platforms") tells a different story from the source comment. The code calls both and is correct regardless; only the narration is over-specified.

3. restoreWindow's doc comment names a tray entry point that doesn't exist — window-restore.ts:8

No Electron Tray anywhere in the repo (the only hit is a renderer component, NeedsInputTray). Either drop it or the next reader goes hunting for a tray handler.

4. The dev exemption sanctions a config that cross-wires agent hooks (pre-existing; probably its own PR)

Not introduced here, but the new comment promotes "dev alongside an installed build" to a documented, supported configuration, so it's worth knowing what that configuration does today: persistence.ts:6-12 gives dev its own -dev state dir, but agent-hooks/runtime.ts:18 uses raw userData. Both instances write agent-hooks/endpoint (port + token) to the same path, and the hook script re-sources that file on every event (per hook-script.test.ts:20) — so whichever instance started last receives hook status events for both instances' agents. A one-line fix would mirror the dev suffix from persistence.ts.

Minor

• second-instance drops commandLine/workingDirectory. Correct today — main.ts reads no argv (the only process.argv is in electron/mcp/server.ts, a separate process). Flagging it because second-instance is also where deep links land on Linux/Windows, so this is the hook to remember if a parallel-code <dir> or protocol handler ever ships.
• 'asks whether the window is destroyed before touching it' asserts a mock was called, which the no-op test already covers. Harmless, adds nothing.

On your question about the third call site

Keep it here. One line, same failure mode, and the existing comment already asserted a property the code didn't have — splitting it would leave that comment lying for another cycle.

Caveat on coverage

The lock only engages in packaged builds, so neither CI nor npm run dev exercises the second-instance path at all — the unit tests cover restoreWindow in isolation, not the wiring. Worth one manual packaged-build check (launch, hide, relaunch) before merge.

[miadisabelle]

Thanks — the fixEnv() catch is the good one. Pushed 64123b3.

1. Lock before fixEnv() — fixed

The lock decision now happens before fixEnv(), and fixEnv() moved into the primary branch on your reasoning: only the primary spawns PTYs, so only the primary needs the resolved environment.

I measured it on a packaged build rather than trusting the shape of the fix. Xvfb + kwin, a throwaway --user-data-dir, 3 runs each, timing the doomed second instance end to end:

	second instance	login-shell probes observed while it ran
before	7.86s (7.80 / 7.75 / 8.04)	63
after	1.25s (1.24 / 1.23 / 1.27)	0

Your 0.53s is the floor, not the ceiling. This box is the headless Linux one from the Note, and its rc file has nvm and conda in it — so the probe costs 6.6s here, and every bit of it landed on the click.

2. Wayland — noted in the comment, no code change

Agreed on both halves. The doc comment now says the visible-but-buried case may end at an icon flash rather than a raise on Wayland, with the reason (a client generally cannot raise itself; Electron's docs say focus() may notify or flash instead), and that hidden and minimized are unaffected.

I also dropped the "show() is a no-op on a minimized window" claim. You were right that the source comment and the test comment were telling different stories, and the docs back neither. The comment now says only what the code relies on: the two states are not exclusive, so both questions get asked and each answer gets acted on.

3. tray — dropped

No Electron Tray in the repo. Gone.

4. agent-hooks/endpoint cross-wiring — separate PR, ready when you want it

Confirmed the shape you describe: persistence.ts:6-12 gives dev its own -dev state dir, agent-hooks/runtime.ts:18 uses raw userData, and the hook script re-sources the endpoint file per event — so the last instance to start collects hook status for both. Happy to send the one-line mirror of the dev suffix as its own PR, since as you say it predates this one. Say the word and it's up.

Minor

• Dropped the isDestroyed() mock assertion. You're right, the no-op test already covers it.
• commandLine/workingDirectory: understood, leaving the parameters off rather than accepting and ignoring them. Noted that this is where deep links would land.

Third call site

Kept, per your call.

Packaged-build check

Done, since neither CI nor npm run dev reaches this path. Packaged build, Xvfb + kwin, both instances on the same --user-data-dir:

A launched            → window state: Normal, 2 processes on the profile
xdotool windowminimize → window state: Iconic
B launched            → exit 0 after 1.14s
A still running       → YES
A window state        → Normal        (restored by second-instance)
processes on profile  → 2 (unchanged)

So the wiring is exercised, not just restoreWindow in isolation: B declines to become a second app, and the minimized window comes back. I used minimize rather than "Keep in Background" because driving the UI headless is awkward, and minimize is the stricter of the two — it needs restore(), where hide only needs show().

npm run check:static and the full suite (2219 passing) are green on the branch.