feat(tui): wire Settings screen to AppSettings for live config #99
No reviewers
Labels
No labels
area:agents
area:ai
area:config
area:dashboard
area:design
area:design-review
area:devtools
area:entities
area:gallery
area:generate
area:image
area:infra
area:meta
area:model-browser
area:navigation
area:presets
area:security
area:sessions
area:settings
area:sharing
area:test
area:ux
area:webhook
area:workdir
type:bug
type:chore
type:meta
type:user-story
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
charles/loom!99
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "tui/settings-wire-92"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
AppSettingsvalues fromctx.settingson each render, populating all 9 section panes with live data (Backends, Paths, Generation Defaults, AI Features, NSFW, Appearance, Sharing, Key Bindings, System Prompts)AppSettingsand persists viasave_settings()Ctrl+Tin the Backends section to firebridge.health_check()and display the result as a notificationTest plan
cargo clippy -p loom-tui -- -D warningscleancargo test -p loom-tuipasses (122 tests)settings.jsonCtrl+Tin Backends section, verify health check notification appearsCloses charles/loom#92
🤖 Generated with Claude Code
Review — PR #99: Settings screen wired to AppSettings
Good work. Full two-pane settings editor with inline editing, boolean toggles, and health check. Tests are solid.
Concern:
blocking_read()in render pathrender()callsctx.settings.blocking_read(). This is atokio::sync::RwLock, andblocking_read()will park the current OS thread if a write is held. In practice settings writes are rare so this is fine, but if a background task ever holds the write lock for a network call (e.g. reconfiguring the backend), the TUI would freeze. Consider usingtry_read()with a fallback to "(loading…)" text instead.Concern:
apply_editwithtry_write()apply_editclones theArc, then callstry_write(). If it fails, the user gets "failed to acquire settings lock" with no retry path. This is OK for now, but a notification with "try again" would be friendlier.Nit
health_checkusesctx.bridge.clone()(field access) while other methods usectx.bridge()(accessor). Pick one for consistency.Ctrl+Thealth check hint shows in all field panes, even when not in the Backends section. The hint line should probably be conditional onself.section == SettingsSection::Backends.25c0a9907ato57982077c0