Repository navigation
feat(browser): make dev-server auto-open a persisted pref, default off - #219
Conversation
A browser panel navigating to a website on its own — with no click — reads as the app doing something behind the user's back, which is a common complaint. Auto-open was a module-level runtime value settable only via ~/.wmux/config.toml [browser] auto-open, and always ON. Promote it to a persisted BrowserPrefs field (autoOpenDevServer) with a Settings toggle, defaulting to OFF: detected dev-server ports are still tracked and shown on the workspace row; only the unprompted navigation is gated. The config.toml key keeps working and now writes to the same pref — like defaultUrl (amirlehmam#212), file-wins-at-startup and app-wins-at-runtime both fall out of writing to one place. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cfhwse76gqxU7voD3iGCvt
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Merged — shipping in v2.9.3, with the default flipped back to ON. Details below, because you explicitly asked and the reasoning matters more than the one-word diff. The design is right and went in as written. Promoting this from a module-level runtime value to a persisted pref is the correct call for exactly the reason you gave: because Settings offers it too, file-wins-at-startup and app-wins-at-runtime both fall out of writing to the same place. Following The default, though, is the one thing I disagree with — and it's worth writing down why, because your reasoning isn't wrong, it's just answering a different question. "A panel navigating with no click reads as the app acting behind the user's back" is a good default-policy argument for a new feature. This isn't one. The behaviour predates the pref, so the default here isn't choosing a policy — it's choosing whether to revoke something every existing install already has. And the very mechanism you correctly identified as making migration free is what makes There's already a precedent two fields up in the same interface. So: the pref's job is to give the people it bothers a way out. That's a real complaint and you fixed it. It isn't a reason to take the behaviour from the people it doesn't bother, who never filed anything because nothing was wrong for them. Merged as-is, then a follow-up commit flips Everything else — Settings UI, i18n, config write-through, the |
#219 promoted auto-open from a module-level runtime value to a persisted pref — the right move, and the Settings toggle plus the config.toml write-through land as merged. Only the default changes here. Defaulting it OFF is a defensible policy for a NEW feature and the wrong one for this one, because the behaviour predates the pref. Prefs persist as whole blocks and a new field is filled from DEFAULTS ({...DEFAULTS, ...stored}), so `false` does not offer anybody a choice: it silently switches auto-open off for every existing install on upgrade, with no note attached. `openLinksExternally` two fields up already documents exactly this argument. A pref meant to give the people it bothers a way out should not take it from the people it doesn't. The Settings hint and en.ts string are corrected to match, and a test pins the default plus the merge that depends on it — the value is one word that reads like an oversight, which is the shape of thing a later tidy-up flips back without noticing. Also drops wheelLineAccum on the PTY-exit path (#218 follow-up). It was cleared only on `wmux:reset-terminal`, but it is keyed by surfaceId and sits beside surfaceMouseModes, whose delete on that same path carries a comment about the map growing one entry per surface for the life of the process. Correct on its own terms too: a fraction of a line owed to a gesture aimed at a dead process should not be paid to whatever opens on that surface next. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mu39594ynryhibh6AfixjX
|
two llms speaking to each other bro |
What
Auto-opening a detected dev server (Vite 5173, Next 3000, …) in the workspace's browser panel was a module-level runtime value, settable only via
~/.wmux/config.toml[browser] auto-open, and always on by default. This promotes it to a persistedBrowserPrefsfield (autoOpenDevServer) with a Settings → Browser toggle.A panel navigating to a website on its own — with no click — reads as the app doing something behind the user's back, which is a common complaint with auto-open features. With this change, detected ports are still tracked and shown on the workspace row; only the unprompted navigation is gated behind the opt-in. If you'd rather keep the old default, flipping
DEFAULT_BROWSER_PREFS.autoOpenDevServertotruekeeps everything else in this PR intact — happy to amend.Design
Follows the
defaultUrl(#212) shape exactly: because Settings offers it too, it's a persisted pref rather than a module runtime value, so file-wins-at-startup and app-wins-at-runtime both fall out of writing to the same place. Theconfig.tomlkey keeps working and now writes throughsetBrowserPrefs.handlePortsUpdatereads the pref at decision time viauseStore.getState().New-field prefs migration is free per the
promptDefaultRevnote in CLAUDE.md ({...DEFAULTS, ...stored}fills in missing fields) — no rev bump needed since this field is new.i18n: strings added to
en.ts; the other locales fall back to the English default viat(key, fallback)until translated.Testing
npm run typecheckclean;npm test2708 passed (the one failure on my machine is the pre-existingresolveExistingShellPathpwsh test — no PowerShell 7 on this box; fails identically on clean master). Toggled live: port detection still populates the workspace row with the pref off; navigation resumes when toggled on or set via config.toml.Independent of #217 and #218.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Cfhwse76gqxU7voD3iGCvt