fix(dashboard): give the settings page the launcher's run-option rules - #1098
Merged
Conversation
Adding and removing a device already worked, but only from the composer's "Run on" menu, and the composer only exists on a project launcher: from the Overview or the settings page there was no way to manage the roster at all. Adds a Devices section to the settings page listing each saved device with its origin and online/offline status, an Add device button (the existing dialog), and a remove per row. The "Run on" picker still lists devices, because choosing a run target is a per-run act; which devices exist is configuration. Removing from settings applies the same guard the composer already did: a device that was the selected run target clears the selection, so a run can never point at a device that is no longer saved. The presentation is new but the state is not: it reads the existing useConnectionProfiles / useDeviceStatus hooks and profiles.ts, so there is one device store, not two. The section says devices are saved in this browser because, unlike every other setting on the page, a device carries a token and so never reaches the daemon.
The settings page rendered the run options as flat, independently checkable boxes while the launcher has real rules between them, so the page could show an option checked that the launcher shows off, and allowed combinations that mean nothing: Eco under Vanilla, Browser on Codex, Auto maintenance without Post-merge cleanup, and anything at all under Transparent. Move the option table and its rules out of Composer into lib/run-option-rows.ts, rendered by both surfaces, so a rule cannot hold in one place and not the other. It is pure data, so the launcher still renders dropdown items and the settings page renders page rows; neither owns the rules. `checked` is the effective value, so nothing claims an option is on while the run ignores it. A disabled row is greyed with its reason rather than hidden: the settings page is where you go to look for a setting, so a vanished row would be worse than a greyed one. The launcher is unchanged (it already hid the Eco drops when Eco is off), and its tests pass untouched. Two smaller cases of the same mistake, fixed here too: - the notification rows now show the delivery capability the bell already did (permission blocked, DISCORD_WEBHOOK / DISCORD_BOT_TOKEN unset) - the spend offset is bounded to the same range as its slider and the sanitizer, which used to clamp a typed 9999 to 50 while the box kept 9999
#1097 was squash-merged, so its commits are not the ones on main and the shared SettingsPage.tsx conflicted. Resolved in favour of this branch's shared run-option table, keeping the Devices section main brought in.
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.
Stacked on #1097 (Devices) — both touch
SettingsPage.tsx, so this targets that branch. Base retargets tomainonce #1097 merges.The bug
The settings page shipped in #958/#1096 rendered the run options as flat, independently checkable boxes, while the launcher has real rules between them. That is not just noise, it is wrong:
transparentoverrides autopilot / technical / vanilla / post-merge / browser, and forces them to read offecoDisabled = vanilla || transparent— "nothing to trim while the system prompt is off"browseris Claude-only (inert on Codex — it rides Claude Code's MCP config)ecoMaintenancerequiresonBeforeMergeableQualitySo the page could show a box checked that the launcher shows off.
The fix: one table, two renderers
The option table and its rules move out of
Composer.tsxintolib/run-option-rows.ts, which both surfaces render. It is pure data (no JSX), so the launcher still renders dropdown items and the settings page renders page rows — neither owns the rules, and a rule cannot hold in one place and not the other.checkedis now the effective value, so no surface claims an option is on while the run ignores it.Two smaller cases of the same mistake, also fixed
DISCORD_WEBHOOK/DISCORD_BOT_TOKENare unset.MAX_SPEND_OFFSET, the same range as its slider and the sanitizer. It used to accept a typed9999that the sanitizer silently clamped to 50 while the box kept showing 9999.Verification
pnpm typecheck22/22,pnpm build12/129999into the spend offset lands on50.One behaviour I carried over rather than "fixed", flagged for a decision: an unrecognised stored agent falls back to Claude Code for the label but is not treated as Claude for the Browser rule, so Browser stays disabled. That is what the launcher already did; treating an unknown agent as Claude would offer a browser its driver cannot wire up. Pinned by a test so it is now a decision rather than an accident.