Skip to content

feat(settings): editable plugin config (Ctrl+,) + green CI and status refresh - #1

Merged
guyghost merged 4 commits into
mainfrom
finalisation/green-ci-and-status-refresh
Jul 5, 2026
Merged

feat(settings): editable plugin config (Ctrl+,) + green CI and status refresh#1
guyghost merged 4 commits into
mainfrom
finalisation/green-ci-and-status-refresh

Conversation

@guyghost

@guyghost guyghost commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Summary

Finalization branch that lands an editable in-app settings experience, restores a green CI pipeline, and refreshes project status docs. Behavior is additive; no existing flows are removed.

What & why

RightClick had no way to toggle plugin/UI options at runtime — configuration lived in config.json and required an editor restart. Meanwhile CI had drifted red (clippy -D warnings and the git-workspace tests failing), blocking merges. This branch closes both gaps and aligns the README/STATUS with the new UX.

Changes

feat(settings): editable plugin configuration (Ctrl+,)

Adds a settings modal (Ctrl+,) that edits the existing PluginsConfig / UIConfig fields at runtime, persists them to config.json, and pushes them into live plugins.

  • New settings module wrapping the generic modal::Modal framework (previously unused), mirroring checkbox state so toggles map back to the correct field.
  • Plugin trait gains apply_config(&Config) (default no-op) so the shell can push a saved config back into running plugins; implemented for gitstatus, filebrowser (show_hidden + refresh), and workers (paths).
  • App now owns the authoritative Config and shared event_bus: saves on settings confirm, calls apply_config on every plugin, and publishes Event::ConfigChanged.
  • Wires the app.* palette commands (quit / refresh / settings) in the shell and surfaces app.* entries in command search; the app.settings stub is now live.
  • Fixes a latent config_dir() closure arity error (returns Result, not Option) exposed once App started carrying the config.

fix(ci): make clippy and git workspace tests pass

  • Applies clippy suggestions (sort_bysort_by_key with Reverse, collapsible-match guards) to clear -D warnings.
  • Isolates the git-workspace test repos from the host GPG signing config so test commits succeed without gpg installed.
  • Mechanical refactors only — no behavior changes.

docs

  • docs(status): refreshed source/test counts, raised adapter count to 8, dropped already-shipped items from next steps.
  • docs(readme): documents the new Ctrl+, settings shortcut.

How it was validated

  • bash scripts/dev.sh ci stays green: cargo fmt + clippy -D warnings + 1581 tests passing.
  • Added settings unit tests and a config round-trip integration test (tests/settings_round_trip.rs).
  • CI is the focus of this branch (see fix(ci) commit); GitHub Actions will re-confirm on the PR.

Notes

  • Untracked local artifacts (.idea/, .rightclick/, a dev test-data helper script) were intentionally left out of this PR.

guyghost added 4 commits July 5, 2026 21:22
Apply clippy suggestions (sort_by -> sort_by_key with Reverse,
collapsible_match guards) to clear -D warnings, and isolate the
git workspace test repos from host GPG signing config so commits
succeed without gpg installed.

No behavior changes; refactorings are mechanical.
Update source/test counts, raise adapter count to 8, and drop
already-shipped items (extra adapters, global search) from the
next steps list.
Add a settings modal (Ctrl+,) that edits the existing PluginsConfig /
UIConfig fields at runtime, persists them to config.json, and applies
them to live plugins.

- New settings module wraps the generic modal::Modal framework
  (previously unused) and mirrors checkbox state so toggles map back
  to the right field.
- Plugin trait gains apply_config(&Config) (default no-op) so the shell
  can push a saved config back into running plugins; implemented for
  gitstatus, filebrowser (show_hidden + refresh), and workers (paths).
- App now owns the authoritative Config and shared event_bus, saves on
  settings confirm, calls apply_config on every plugin, and publishes
  Event::ConfigChanged.
- Route app.* palette commands (quit/refresh/settings) in the shell and
  surface the app.* entries in command search; the app.settings stub is
  now wired.
- Fix a latent config_dir() closure arity error (returns Result, not
  Option) exposed when App started carrying the config.
- Tests: settings unit tests, a config round-trip integration test.
  dev.sh ci stays green (fmt + clippy -D warnings + 1581 tests).
Copilot AI review requested due to automatic review settings July 5, 2026 20:07
@guyghost
guyghost merged commit 5207ee5 into main Jul 5, 2026
2 checks passed
@guyghost
guyghost deleted the finalisation/green-ci-and-status-refresh branch July 5, 2026 20:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds an in-app Settings modal (Ctrl+,) to edit/persist configuration at runtime, introduces a Plugin::apply_config hook for live reconfiguration, and includes a set of mechanical refactors/docs updates aimed at restoring a green CI pipeline.

Changes:

  • Adds src/settings module implementing a settings modal that maps checkbox toggles back into Config and supports save/cancel flows.
  • Extends the plugin system with apply_config(&Config) and wires the shell (App) to persist config changes, push them into live plugins, and publish a config-changed event.
  • Fixes CI issues via clippy-driven sort refactors and makes git-workspace tests independent of host GPG signing configuration; updates README/STATUS docs.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/settings_round_trip.rs Adds an integration test for config persistence through the settings modal.
tests/git_workspace_workflows.rs Disables GPG signing in test repos to avoid host config/test flakiness.
STATUS.md Updates project stats and next-steps documentation.
src/settings/mod.rs Implements the Settings modal and state mirroring for editable config booleans.
src/plugins/workspace/plugin.rs Small clippy-driven match-guard simplification.
src/plugins/workers/plugin.rs Adds apply_config to live-update worker paths (with a path-normalization issue noted).
src/plugins/gitstatus/plugin.rs Adds apply_config (but currently doesn’t apply settings to behavior).
src/plugins/filebrowser/plugin.rs Adds apply_config to refresh when show_hidden changes.
src/plugins/conversations/plugin.rs Clippy-driven sort and guard simplifications.
src/plugin/mod.rs Extends Plugin trait with a default apply_config hook.
src/palette/fuzzy.rs Refactors sorting to sort_by_key(Reverse(..)) for clippy.
src/main.rs App shell now owns config + shared event bus; adds settings modal flow and app.* palette routing.
src/lib.rs Exposes the new settings module.
src/adapters/opencode.rs Clippy-driven sort refactor.
src/adapters/gemini.rs Clippy-driven sort refactor.
src/adapters/codex.rs Clippy-driven sort refactor.
src/adapters/claudecode.rs Clippy-driven sort refactor.
README.md Documents the new Ctrl+, shortcut.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/settings/mod.rs
Comment on lines +138 to +142
let mut modal = Modal::new("Settings")
.with_variant(ModalVariant::Info)
.with_width(64)
.with_primary_action("Save")
.with_close_on_backdrop(true);
Comment on lines +1117 to +1119
fn apply_config(&mut self, config: &Config) {
self.config = Some(config.clone());
}
Comment on lines +1061 to +1073
fn apply_config(&mut self, config: &crate::core::models::Config) {
let new_config = config.plugins.workers.clone();
// Reflect path changes in state if they changed.
let intents_changed = self.config.intents_dir != new_config.intents_dir;
let logs_changed = self.config.logs_dir != new_config.logs_dir;
self.config = new_config;
if intents_changed {
self.state.intents_dir = PathBuf::from(&self.config.intents_dir);
}
if logs_changed {
self.state.logs_dir = PathBuf::from(&self.config.logs_dir);
}
}
Comment thread src/settings/mod.rs
Comment on lines +145 to +149
modal.add_section(section::text("Appearance"));
modal.add_section(section::checkbox(
ids::UI_SHOW_CLOCK,
"Show clock in header",
state.ui_show_clock,
Comment on lines +23 to +26
use crossterm::event::{KeyCode, KeyEvent, KeyModifiers};
for _ in 0..8 {
modal.handle_key(KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE));
}

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3a2a097a42

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/main.rs
};
let new_config = modal.into_config(&self.config);
// Persist to config.json.
if let Err(e) = config::save(&new_config) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Save settings to the loaded config file

When the app is launched with --config <path>, startup loads that file, but this settings save path always calls config::save(), which writes the default config path instead. Saving from the modal therefore leaves the selected config file unchanged and can unexpectedly create/overwrite the user's default config; carry the loaded path into App and use save_to for this case.

Useful? React with 👍 / 👎.

Comment thread src/main.rs
Comment on lines +357 to +358
for plugin in &mut self.plugins {
plugin.apply_config(&new_config);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reconcile enabled plugins when settings are saved

When a user toggles any plugin Enabled checkbox, this loop only calls apply_config on plugins that are already in self.plugins. The only code that constructs/skips plugins based on config.plugins.*.enabled runs during startup, so disabling a loaded plugin leaves its tab and commands active until restart, while enabling a plugin that was disabled at startup cannot make it appear. Either reconcile the plugin list on save or avoid exposing these flags as live-applied settings.

Useful? React with 👍 / 👎.

Comment thread src/settings/mod.rs
Comment on lines +222 to +225
modal.add_section(section::buttons(vec![
Button::primary(ids::SAVE, "Save"),
Button::secondary(ids::CANCEL, "Cancel"),
]));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep settings actions visible on 24-row terminals

On a common 80x24 terminal, the settings modal's content is taller than the available modal body: the modal clips content instead of scrolling, and these Save/Cancel buttons are appended after roughly 23 content lines. That leaves the only explicit save controls off-screen, so users must tab blindly to a hidden button to persist changes; reduce/collapse the sections or add scrolling/sticky actions.

Useful? React with 👍 / 👎.

Comment on lines +1117 to +1118
fn apply_config(&mut self, config: &Config) {
self.config = Some(config.clone());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply the Git show-untracked setting

When git.show_untracked is changed in the settings modal, this implementation only stores the new config. The git status refresh path still fetches all untracked files and PluginState::update_from_repo_status always appends status.untracked, and repo-wide search shows show_untracked is not read outside config/settings/tests, so saving Show untracked files = false leaves untracked files visible. Filter/refresh the status list based on the setting or remove the checkbox.

Useful? React with 👍 / 👎.

Comment thread src/settings/mod.rs
Comment on lines +184 to +187
modal.add_section(section::checkbox(
ids::CONVERSATIONS_SAVE_CONTEXT,
"Save conversation context",
state.conversations_save_context,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply the conversation retention setting

The new Save conversation context checkbox persists plugins.conversations.save_context, but the conversations plugin and adapters never read that field (repo-wide search only finds config/settings/tests), so users who turn it off still have conversation context handled exactly as before. Either wire this setting into the conversation-saving path or avoid exposing a privacy-looking toggle that has no effect.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants